Skip to content

refactor(desktop): one forward-only state per recorded tmux run, and one check for a delivered result - #8737

Merged
waleedlatif1 merged 2 commits into
stagingfrom
refactor/desktop-run-ledger-state
Oct 7, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
refactor/desktop-run-ledger-state

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A simplification of the tmux run ledger from #8722. The product rules stay the same: at launch, for the same user, runs the model can no longer collect are stopped, and every recorded run is stopped when the identity ends. The only behaviour changes are the correctness fixes listed under "Fixes".

Simpler

  • One forward-only state: state: 'started' | 'delivered' | 'stop' replaces delivered + mustStop, behind a single advance(runId, state).
    • Because a record never moves backwards, a late acknowledgement can't undo a stop. Before, that ordering rule lived in the launch check.
    • A record saved before state existed is read as the state it meant: mustStop: true as stop (this wins), then delivered: true as delivered, and otherwise started. A published staging build wrote that format. Any other record is dropped as unreadable.
  • One check for a delivered result: isDeliveredResult in the executor decides which results reach the model.
    • onResultDelivered fires only for those, and the journal's pending-results read uses the same check. Before, the check was duplicated in main/index.ts and the service.
    • The executor tests now cover the hook's filtering, closing the gap left by the untested wiring in main.
  • Journal ordering moves into the service: the service snapshots the journal's pending results once, before its own recovery can rewrite it.
    • main no longer has to await the snapshot before start().
    • A test drives recovery to empty the journal first and still gets the snapshot.
  • No abandon hook: a run that can't write its go file after tagging closes its tagged pane through ifTagged, like a refused run. The next sweep drops the saved record.
  • No directory scan per tool result: a delivery finds its run through an index from call id to run id.
    • The index is read from the directory once per process, then kept current.
    • It covers previous processes' runs too, so a result recovery delivers can still mark its run.

Fixes

  • Stand-in results no longer count as delivered. That covers a cancelled result and a 413 resultOmitted one, as well as the not-started and outcome-unknown ones already excluded. In none of these did the model get a pane.
  • A kept run whose command ended is closed at launch. Under remain-on-exit, a dead pane used to read as ours forever. recordedRunState now reads #{pane_dead} and reports such a pane as finished. The launch sweep then closes it through ifTagged and forgets the record, with no Ctrl-C sent.
  • A kept run whose recovered result comes back superseded: it's handled at the next launch, with no extra code.
    • The run was kept at this launch only because its result was pending in the journal. Its record stays started.
    • Recovery then drops the journal entry, so the next launch finds it neither delivered nor pending and stops it.
    • It goes on until then, like any run the model was handed back.
  • Text: stopRun's doc comment, which had drifted onto ifTagged, is back on stopRun.

#8725 follow-ups

  • Newline in the running command: a pane whose running command contains a newline is now covered. Before, the fixtures had a newline only in the window name and cwd, so reading pane_current_command raw stayed green.
  • UTF-8 split across reads: a fake tmux writes list-panes output in two pieces, split inside a multi-byte character. The test pins runTmux's setEncoding('utf8').

Corrections to #8722's description

  • delivered: it is set when Sim acknowledges a real result for the call. "Once the result has been handed back" was wrong: it waits for Sim's acknowledgement, and a stand-in result never sets it.
  • The same-user launch rule: a run is kept only when it is delivered, or when the journal holds its real result for recovery to send. Anything else is stopped. "Stopped when the journal shows the call claimed or started" undersold this, because a run with no journal entry at all is stopped too.
  • Chat-view runs: they never get an acknowledgement, so they are stopped at the next launch. They are not counted as handed back.
  • A run that can't start after its record is saved: its tagged pane is closed, and the next sweep drops the record. The record is no longer forgotten on the spot.

Type of Change

  • Bug fix

Testing

  • Unit tests: desktop src/main/terminal and src/main/desktop-executor (209 tests).
  • Red checks: each fix was removed in turn and its test fails. There are 16:
    • reading the old record format;
    • stop winning over delivered in an old record;
    • UTF-8 decoding;
    • command newline;
    • cancelled not delivered;
    • omitted not delivered;
    • the hook's result check;
    • snapshot before recovery;
    • an unreadable journal;
    • the state check;
    • forward-only;
    • lookup by call;
    • the index read from the directory;
    • a dead pane read as finished;
    • a kept dead pane stopped;
    • the go-file pane close.
  • Real tmux on EC2:
    • tmux 3.4 reads #{@sim-run-id} #{pane_dead} as run-x 0 while the pane is live and run-x 1 after its command ends under remain-on-exit, and the tagged kill-pane removes it;
    • tmux 2.9a has no pane options, so it never records a run.
  • Terminal-cancel Electron E2E (test(desktop): stop the agent's terminal commands in the real app, with real shells and real tmux #8733) on top of this change: 12/12 on EC2 with tmux 3.4.
  • EC2 gate and integration: green.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…one check for a delivered result

A recorded run is started, delivered or stop, and only moves forward, so a late acknowledgement can
never undo a stop. The executor alone decides which results reach the model (not a stop, nor one
that stands in for a run that did not start, has an unknown outcome or was too large to send), and
snapshots the journal before its own recovery. A delivery looks up its run by call instead of
reading every record. A run that cannot start after tagging closes its pane like a refused one. At
launch, a kept run whose pane outlived its command is closed and forgotten. Covers a newline in a
pane's running command and a character split across tmux's output.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@vercel

vercel Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 7, 2026 8:29am UTC

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 13 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Comment thread apps/desktop/src/main/terminal/run-ledger.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 13 files

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Turn on auto-fix | Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors tmux run state tracking and result delivery logic.

The PR appears safe to merge; the old-record cleanup issue is fixed.

What we checked:

  • Old runs remain visible: stateOf reads the old fields instead of rejecting their records. It maps mustStop: true to stop before checking delivered.

Summary

This PR gives recorded tmux runs one forward-only state and shares the check for results delivered to the model.

  • Recovery snapshots pending results before rewriting the journal.
  • Launch cleanup closes finished panes and stops runs the model cannot collect.
  • The latest change fixes the previous cleanup finding: old delivered and mustStop records remain readable, with mustStop taking priority.
  • No new actionable issues or repository-rule violations were found.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Started["started"] -->|Real result acknowledged| Delivered["delivered"]
  Started -->|Stop could not be confirmed| Stop["stop"]
  Delivered -->|Stop could not be confirmed| Stop
  Stop -->|Late acknowledgement| Stop
Loading

Reviews (2) · Last reviewed commit: "fix(desktop): read run records saved bef..." · Reviewed by Greptile

Comment thread apps/desktop/src/main/terminal/run-ledger.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 13 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 8f617da into staging Oct 7, 2026
37 checks passed
@waleedlatif1
waleedlatif1 deleted the refactor/desktop-run-ledger-state branch October 7, 2026 16:32

This branch was previously deployed

1 inactive deployment
Preview — 2a0fef2a Deployed Oct 7, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant