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

Merged
WA
17d ago
+392-173

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 (#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)