Skip to content

fix(desktop): stop the agent's tmux runs a previous process left going - #8722

Merged
waleedlatif1 merged 6 commits into
stagingfrom
fix/desktop-terminal-run-ledger
Oct 7, 2026
Merged

waleedlatif1 merged 6 commits into
stagingfrom
fix/desktop-terminal-run-ledger

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A tmux run outlives the app, but until now only the process that started it knew about it. After a crash, a quit, or a crash part-way through sign-out, the next launch had nothing to stop, so the agent's command kept running in the user's tmux. A chat put away also took its runs out of reach of sign-out.

  • Each tagged run is recorded in userData/terminal-runs, outside account data. The record holds the run's tag, its pane, its tmux server's socket (#{socket_path}), its call id, and whether its result was handed back. It holds no command line and no output.
  • A record is removed once its run has ended, its pane is gone, or Sim has stopped it.
  • These stop every recorded run:
    • sign-out;
    • switching Terminal off;
    • the launch-time account recovery.
  • At every launch, a run a previous process left going is stopped only when nothing can collect it (see below), and none the new process has started is touched.
  • A recorded pane is only acted on through tmux -S <its socket>, and only while it still carries the run's tag. A pane that took the id after a tmux restart, or a different tmux server, is never touched, and its record is dropped.
  • A record tmux couldn't answer for is kept for the next sweep.

Restart semantics

A tmux run that outlives its wait hands its pane back to the model as running, and the model may come back to that pane with read, input, kill or close. So a restart must not end every leftover run.

  • Each record carries: the run's call id, and a delivered flag that is set once the run's result has been handed back as still going.

  • Identity ended or changed: sign-out, a crash during sign-out, or the launch-time account recovery. Every recorded run is stopped.

  • The same user at launch: a previous process's run is stopped only if:

    • its result was never handed back; or
    • the executor journal shows the call as claimed, started, not started or outcome unknown. The journal is read before the executor starts, because recovery rewrites it.

    Every other run is left going. Its record is dropped only once its pane is gone.

  • Quitting: stops no tmux run, as before.

  • A chat-view run: follows the same rule. Its result went to the chat view, so it counts as handed back once the terminal returns it.

Out of scope: a plain-shell agent command that ignores SIGHUP and outlives its shell after a crash. A stale process group can't be told from a reused one, so nothing could safely act on it.

Type of Change

  • Bug fix

Testing

  • Unit tests:
    • the ledger survives into a new process, and corrupt or misnamed records are dropped;
    • a run's record exists exactly while the run goes on;
    • a recorded run is stopped on its own server while it carries its tag;
    • a pane that took the id after a restart is never touched;
    • a different server is never asked;
    • a record tmux can't answer for is kept;
    • sign-out and launch sweep the records, and launch skips this process's own runs.
  • Same-user launch rule: a handed-back run is left going; a run never handed back, or handed back while the journal still shows its call unresolved, is stopped; a kept record whose pane is gone is dropped. The journal's unresolved calls are claimed, started, not started and outcome unknown.
  • Pinned guards:
    • a run whose server can't be named is never started;
    • the pane is checked once more right before it is closed;
    • a saved record is forgotten when its run then can't start;
    • records are forgotten on the done, reap and closed-tab paths;
    • only %N pane ids are accepted;
    • a record is kept when tmux couldn't confirm a finished pane closed.
  • Red checks: each guard was undone in turn (record on start, forget, tag check, -S, the sign-out sweep, the launch exclusion), and each makes its test fail.
  • Electron E2E with real tmux: in the follow-up terminal-cancel E2E PR.

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)

@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 6:16am 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 9 files

Turn on auto-fix | Re-trigger cubic

Comment thread apps/desktop/src/main/terminal/registry.ts
Comment thread apps/desktop/src/main/terminal/tmux.ts Outdated
Comment thread apps/desktop/src/main/terminal/run-ledger.ts Outdated
Comment thread apps/desktop/src/main/terminal/index.ts Outdated
Comment thread apps/desktop/src/main/terminal/run-ledger.ts Outdated
@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Adds process recovery for terminal runs across app restarts.

The changes since the previous review appear safe to merge.

What we checked:

  • Saved runs survive recovery: stopUncollectableRuns keeps calls listed in pendingResults. Recovery sends the saved completion for entries in the result state.
  • Active readers keep their files: The delayed cleanup calls releaseRun, which defers deletion while the handle remains in awaitedRuns.

Summary

This PR records agent tmux runs so a later app launch can find them and stop abandoned work.

  • The latest change preserves runs whose results are saved for recovery.
  • delivered is now set after Sim accepts the result, rather than after a local journal write.
  • Finished runs keep their files until pane cleanup finishes.
  • No new actionable issues were found in the changes since the previous review.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Launch[App launches] --> Read[Read saved results before executor recovery]
  Read --> Sweep[Check runs from the previous process]
  Sweep --> StopRequired{Marked mustStop?}
  StopRequired -->|Yes| Stop[Stop the matching tagged pane]
  StopRequired -->|No| Available{Result accepted or saved for recovery?}
  Available -->|Yes| Keep[Leave the run going]
  Available -->|No| Stop
  Read --> Recover[Start executor recovery]
  Recover --> Accepted[Sim accepts the result]
  Accepted --> Delivered[Mark the run delivered]
Loading

Reviews (6) · Last reviewed commit: "fix(desktop): keep a restarted user's tm..." · Reviewed by Greptile

Comment thread apps/desktop/src/main/terminal/tmux.ts Outdated
Comment thread apps/desktop/src/main/terminal/index.ts Outdated
Comment thread apps/desktop/src/main/terminal/run-ledger.ts
Comment thread apps/desktop/src/main/terminal/service.test.ts

@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 9 files

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

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

Comment thread apps/desktop/src/main/terminal/run-ledger.ts
Comment thread apps/desktop/src/main/terminal/index.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.

Comment thread apps/desktop/src/main/terminal/index.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.

No issues found across 10 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

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 10 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

A tmux run outlives the app, but only the process that started it knew about
it: after a crash, a quit, or a crash mid-sign-out, the next launch had nothing
to stop, and a chat put away took its runs out of reach of sign-out too.

- Each tagged run is recorded in userData (outside account data): its tag, its
  pane and its tmux server's socket, nothing else. The record goes once the run
  has ended, its pane is gone, or Sim has stopped it.
- Sign-out, switching Terminal off and the launch-time account recovery stop
  every recorded run; every launch stops the runs a previous process left
  going. A pane is only acted on, on the run's own server, while it still
  carries the run's tag.
- Restart semantics follow the executor's journal: a call claimed before the
  crash is settled as outcome unknown and nothing reattaches to its command, so
  a run still going belongs to a call no one can collect; a run that finished
  is only cleaned up.
…er lose a record tmux could not answer for

- A tagged run's record is saved before its start gate opens; a run whose
  record could not be saved (no socket, or the write failed) never starts.
- Records are written atomically; an unfinished write's temporary file is
  cleaned up, and a record that cannot be removed is logged and skipped
  rather than failing the sweep or the call.
- Stopping a recorded run reports `unknown` whenever tmux could not answer,
  so its record stays for the next sweep; its pane is closed only while it is
  confirmed the run's.
- A service rebuilt after a failed restore records its runs too, and a run
  that finished before its terminal closed is forgotten then.
…nd stop only those nothing can collect

A tmux `run` that outlives its wait hands its pane back to the model as
`running`, and the model may come back to it with read, input, kill or close.
The launch sweep stopped every previous process's run, so a relaunch, a crash
or an update restart killed a dev server or a build the model was still
watching.

- A record now names its call and notes when the run's result was handed back
  as still going.
- At launch, for the same user, a run is stopped only when its result was
  never handed back, or when the executor's journal, read before recovery
  rewrites it, shows the call as claimed, started, not started or outcome
  unknown. Every other run is left going and its record dropped only once its
  pane is gone. Sign-out and the launch-time account recovery still stop every
  run, and quitting still stops none.
- A record is kept when tmux could not confirm a finished run's pane closed,
  and forgotten when its run could not start after all.
@waleedlatif1
waleedlatif1 force-pushed the fix/desktop-terminal-run-ledger branch from 0f439dc to 10674d0 Compare October 7, 2026 05:31
@waleedlatif1
waleedlatif1 changed the base branch from fix/desktop-tmux-untagged-runs to staging October 7, 2026 05:31
@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.

All reported issues were addressed across 12 files

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

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

Comment thread apps/desktop/src/main/index.ts Outdated
Comment thread apps/desktop/src/main/terminal/tmux.ts Outdated
Comment thread apps/desktop/src/main/terminal/index.ts Outdated
… journaled, and check a pane's tag in the same tmux command that acts on it

- A run is marked handed back only once the executor has journaled the
  result that hands it back; a chat-view run cannot prove its result reached
  the model, so after a restart it is treated as orphaned and stopped.
- A run that a stop for everything (sign-out, Terminal off) could not confirm
  stays marked to stop, so a later launch for the same user never keeps it.
- Every tag-guarded action (Ctrl-C, closing the pane, for live and recorded
  runs) is one `if-shell -F` command that checks the tag and acts, so a tmux
  restart between a check and an action cannot hand it to another pane.
@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.

All reported issues were addressed across 14 files

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

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

Comment thread apps/desktop/src/main/index.ts
Comment thread apps/desktop/src/main/terminal/registry.ts Outdated
Comment thread apps/desktop/src/main/terminal/index.ts Outdated
Comment thread apps/desktop/src/main/terminal/tmux.ts
Comment thread apps/desktop/src/main/desktop-executor/executor.ts Outdated
…s, or will get, its pane

- A run is marked handed back when Sim takes the result that hands it back
  (recorded, or a duplicate of one it recorded), not when the journal is
  written: without OS encryption the journal write saves nothing.
- At launch, for the same user, a run is also kept when the journal holds its
  real result for recovery to send, so a crash between the journal write and
  Sim's answer no longer kills a pane the model is about to get. An unreadable
  journal counts as holding none.
- A run whose record could not be saved has its freshly tagged pane closed at
  once, rather than left for the start gate to time out.
- When a terminal closes, a finished run's files go only after its pane check,
  which an untracked run needs them for.
@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 14 files

Confidence score: 5/5

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

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 334b9f3 into staging Oct 7, 2026
36 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/desktop-terminal-run-ledger branch October 7, 2026 07:15

This branch was previously deployed

1 inactive deployment
Preview — 7e3fe3ad 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