Ignore stale terminal RPC failures after Cloud transcript reattachment

[teamleaderleo]

Closing or redirecting a terminal transcript while Send, Stop, or Focus Terminal is waiting for an RPC reply can append the old failure to the replacement view. Fence these callbacks by disposal generation, attachment, target identity, and captured target IDs. Active attachments still report their own failures and preserve failed-send prompts for recovery.

Depends on #16065; this PR targets fix/cloud-terminal-rpc-deadline so its diff contains only the lifetime fix. Focus failures now use the same guarded action path and are emitted once.

Validation

• Regression commit: 47d6d2975d5b841a2f24f7f5a815cb031bcefaff.
• Fix commit: 25677e569383a99a9fd3a898328b19b85b069797.
• From agent-chat, bun test/transcript-rpc-lifetime.test.ts: red with the regression commit's adapter (a disposed view must not receive a late terminal RPC failure), green with the fix. Covers Send, Stop, and Focus Terminal across disposal without a reader, redirection, mutable target IDs, and reattachment to identical targets; current failures still arrive.
• bun test/terminal-rpc-errors.test.ts: passed, including prompt recovery and all 20 locales.
• bun run check: passed (typecheck, 26 scripts, 44 tests across 13 bun:test files).
• bun build src/main.tsx src/gallery-main.tsx --target browser --minify --splitting --outdir scratch/build-check --define 'process.env.NODE_ENV="production"': passed.
• git diff --check 953efca947fb2f5901ec853da03acb04ed8ad49c..HEAD: passed.

No live Cloud instance or browser validation. The generic scripts/verify-local.py entrypoint is absent from this checkout; scoped Agent Chat checks ran instead.

Changelog

Fixed: Cloud Agent Chat ignores late terminal action failures after a transcript closes or switches attachments.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes Cloud Agent Chat terminal RPCs so they no longer hang past their deadline or surface stale failures after a transcript closes or switches attachments. Reopening a Cloud transcript now refreshes the terminal bindings when the agent session or surface changes.

Bug Fixes

• Send, Stop, and Focus callbacks are fenced by disposal generation, attachment identity, and target identity; Focus failures are guarded and emitted once.
• RPCs time out after 10 seconds even when the CLI ignores termination or keeps pipes open, and timeout errors render localized recovery copy across all 20 locales.
• Reopening a transcript that moved terminals pins the new binding and ignores stale replies without restarting history.

^{Written for commit b80327e. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Terminal requests now time out after 10 seconds instead of hanging indefinitely, with a localized message advising you to try again.
  • Failures from outdated or detached terminal sessions no longer appear as errors in the current transcript.
  • Terminal error messages now retain relevant prompt context, while ordinary failures continue to show their diagnostic message.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 96695c41-9d9a-4f02-b107-550cb7c53ac2

📥 Commits

Reviewing files that changed from the base of the PR and between 1681235 and b80327e.

📒 Files selected for processing (13)

• Resources/Localizable.xcstrings
• agent-chat/adapters/transcript.ts
• agent-chat/cmux-rpc.ts
• agent-chat/server.ts
• agent-chat/src/i18n.ts
• agent-chat/src/session.ts
• agent-chat/test/cmux-rpc-deadline.test.ts
• agent-chat/test/fake-cmux-rpc.ts
• agent-chat/test/i18n.test.ts
• agent-chat/test/terminal-rpc-errors.test.ts
• agent-chat/test/transcript-rpc-lifetime.test.ts
• agent-chat/test/transcript-terminal-rebind.test.ts
• agent-chat/types.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 26974be9-e600-43dc-9209-d3853c89c1bf

📥 Commits

Reviewing files that changed from the base of the PR and between 02b80fe and 1681235.

📒 Files selected for processing (12)

• Resources/Localizable.xcstrings
• agent-chat/adapters/transcript.ts
• agent-chat/cmux-rpc.ts
• agent-chat/server.ts
• agent-chat/src/i18n.ts
• agent-chat/src/session.ts
• agent-chat/test/cmux-rpc-deadline.test.ts
• agent-chat/test/fake-cmux-rpc.ts
• agent-chat/test/i18n.test.ts
• agent-chat/test/terminal-rpc-errors.test.ts
• agent-chat/test/transcript-rpc-lifetime.test.ts
• agent-chat/types.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Terminal RPC calls now time out after a deadline and clean up their process and output readers. Transcript operations suppress failures from stale requests. Timeout failures carry a specific event code and display localized text.

Changes

Terminal RPC handling

Layer / File(s)	Summary
Bounded cmux RPC execution agent-chat/cmux-rpc.ts, agent-chat/test/fake-cmux-rpc.ts, agent-chat/test/cmux-rpc-deadline.test.ts	RPC output is read incrementally. Calls race process and output completion against a deadline. Timeout and thrown-error paths retire the process and readers. Tests cover timeouts, JSON and text output, UTF-8 decoding, and controlled failures.
Transcript failure events agent-chat/adapters/transcript.ts, agent-chat/server.ts, agent-chat/test/transcript-rpc-lifetime.test.ts	Focus, send, and stop operations suppress failures from stale requests. Disposal invalidates outstanding requests. The server focus handler discards the operation result instead of emitting a separate error event.
Timeout event display agent-chat/types.ts, agent-chat/src/session.ts, agent-chat/src/i18n.ts, agent-chat/test/terminal-rpc-errors.test.ts, agent-chat/test/i18n.test.ts	Error events can carry a terminal timeout code. Event folding uses localized timeout text for that code and retains the existing message behavior for other errors. Locale records and tests cover the new text and event behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant TranscriptAdapter
  participant cmuxRpc
  participant RPCProcess
  participant Deadline
  participant foldEvent
  TranscriptAdapter->>cmuxRpc: issue terminal RPC
  cmuxRpc->>RPCProcess: start CLI process
  cmuxRpc->>Deadline: start RPC_TIMEOUT_MS timer
  Deadline->>cmuxRpc: return timeout failure
  cmuxRpc-->>TranscriptAdapter: return timeout result
  TranscriptAdapter->>TranscriptAdapter: create timeout-coded error event
  TranscriptAdapter->>foldEvent: fold error event
  foldEvent-->>TranscriptAdapter: use localized timeout text

Loading

Merge Risk: ⚪ Minimal · up to 16812

The change adds a deadline to terminal RPC calls, suppresses stale failure events, and shows localized timeout text. No concrete merge-blocking defect was established. The PR still depends on #16065 landing first, so merge order matters.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 16812

The attachment checks improve isolation of late failures without demonstrating broader terminal access or increased privileges. A terminal timeout can still leave delivery uncertain, so manually retrying a prompt may repeat an action. Attribution of that recovery risk to the lifetime fix versus its prerequisite remains partly unresolved.

Retained concerns

• Medium · reliability · inferred: A timeout retires the local CLI and readers but does not establish that the terminal rejected or cancelled the action. Send can mutate terminal state before its response arrives, while timeout copy encourages retry. A manual retry could therefore repeat an already-submitted prompt, weakening action failure containment. The deadline and retry guidance predate the isolated lifetime fix in the supplied validation baseline; their exact exposure delta against the broader reviewed base remains incompletely verified.

Security review details

Security Blast Radius

• inferred — The demonstrated scope is the selected terminal action and its owning session’s shared event stream. Other sockets subscribed to that same session receive its accepted events. This is session-scoped delivery evidence, not proof of tenant isolation or per-session authorization.

Trust Boundaries and Controls

• observed — Failure publication requires unchanged disposal generation, attachment identity, target-object identity, and captured target IDs. Server reattachment disposes the previous adapter before replacing its target and tail, so late failures cannot pass those checks merely because a replacement reuses the same target IDs.

Resilience and Maintainability Implications

• observed — The lifetime regression assertions cover Send, Stop, and Focus across disposal, redirection, mutable target IDs, and identical-target reattachment, while retaining reporting for a new active request. These assertions support event-ownership containment; they do not establish remote-action cancellation or safe retry.

Hardening Proposals

• proposed — Distinguish an unknown delivery outcome from a confirmed rejection. Before presenting retry as safe, use transcript or operation-status reconciliation, or an operation identity with server-side deduplication, where terminal actions must not be repeated.

🚥 Pre-merge checks | ✅ 23 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description provides a detailed summary, validation results, changelog entry, and known limitations. It does not use the required Summary and Testing headings, and it omits the required Demo Video…	Add the required Summary, Testing, Demo Video, and Checklist sections. Include a demo video or screenshots, state the localization and documentation checklist results, and record any remaining review or verification limitations.
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 11 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (23 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing stale terminal RPC failures after transcript reattachment.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input	✅ Passed	PASS. The PR changes timeout handling for the existing cmux rpc subprocess and adds transcript attachment-generation guards. Bun.spawn remains unchanged from the base, so the PR does not introduce…
Cmux Swift Actor Isolation	✅ Passed	The review-scoped diff contains no Swift files or Swift source changes. It changes TypeScript Agent Chat code, tests, and Resources/Localizable.xcstrings only. Therefore, this check is not applicabl…
Cmux Swift Blocking Runtime	✅ Passed	PASS: The pull request changes TypeScript test/runtime files and Resources/Localizable.xcstrings; it introduces no .swift files or other Swift source changes. Therefore the Swift blocking-runtime …
Cmux Browser Automation Off-Main	✅ Passed	PASS: The pull request does not modify browser socket automation. The policy target files, Sources/TerminalController.swift and `Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/Contr…
Cmux Expensive Synchronous Load	✅ Passed	The authoritative PR diff contains no Swift files. It changes TypeScript files and Resources/Localizable.xcstrings only, so it does not introduce or move a production Swift synchronous agent-history…
Cmux Cache Substitution Correctness	✅ Passed	The PR does not replace a fresh authoritative read with a cache in a persistence, history, undo, or snapshot path. The production changes only add terminal RPC timeout handling, transcript attachment/…
Cmux No Hacky Sleeps	✅ Passed	The production diff adds a 10-second cmuxRpc deadline, not a lifecycle sleep or race workaround. The deadline races RPC completion, kills the CLI with SIGKILL, cancels both output readers, and cle…
Cmux Algorithmic Complexity	✅ Passed	The production diff adds only constant-time transcript guards and linear RPC stream reads. transcriptRpcGuard performs identity and scalar comparisons, and cmux-rpc.ts reads each stream once befor…
Cmux Swift Concurrency	✅ Passed	The authoritative PR diff contains 12 non-Swift files: TypeScript and .xcstrings files only. It contains no .swift paths or Swift patch content. Therefore, the PR does not introduce or expand any …
Cmux Swift @Concurrent	✅ Passed	PASS: The authoritative pull-request diff changes TypeScript and localization files only. It contains no Swift files, Swift functions, or Swift call sites. Therefore it introduces no @concurrent or no…
Cmux Swift Package Boundaries	✅ Passed	The pull request changes TypeScript files and Resources/Localizable.xcstrings only. It does not change any Swift source, Swift package manifest, or Swift target boundary. The Swift package-boundaries …
Cmux Swiftpm Lockfiles	✅ Passed	The pull request changes only Agent Chat TypeScript/tests and localization files. It does not change a SwiftPM package, Package.swift, Package.resolved, Xcode project package references, .gitignore, w…
Cmux Swift Logging	✅ Passed	PASS: The authoritative PR diff contains no changed Swift files. The cmux Swift logging check is therefore not applicable, and the added TypeScript test console.log calls are not production Swift lo…
Cmux User-Facing Error Privacy	✅ Passed	PASS. The changed timeout recovery copy is generic and user-facing: it states that the terminal request timed out and tells the user to try again. The client reaches this copy through the WebSocket ev…
Cmux Full Internationalization	✅ Passed	No internationalization violation is introduced. The new user-facing timeout recovery copy is read through agentChatText("terminalRequestTimeout") and has a non-empty translation for all 20 locales …
Cmux Swiftui State Layout	✅ Passed	PASS: The reviewed diff contains no Swift or SwiftUI files and no SwiftUI state/layout constructs. The cmux Agent Chat changes are TypeScript, localization, and tests, so the SwiftUI-specific failure …
Cmux Architecture Rethink	✅ Passed	PASS: The reviewed diff contains no Swift files or Swift architectural changes. It changes TypeScript files, tests, and Resources/Localizable.xcstrings, so the Swift-specific failure conditions do n…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	The authoritative PR diff contains no changed Swift files. It adds or modifies Agent Chat, test, and localization files only. Therefore, the auxiliary-window close-shortcut rule is not applicable, and…
Cmux Source Artifacts	✅ Passed	All changed paths are source, localization, or deliberate test-fixture files. The new agent-chat/test/* files contain hand-written RPC tests and an isolated fake CLI fixture, not checked-in logs or …
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS: The reviewed diff changes only TypeScript, JavaScript test fixtures, and localization files. It contains no Swift file under a production **/Sources/** path, so the no-test-or-debug-seam-in-pr…

Full details: Description check

Explanation

The description provides a detailed summary, validation results, changelog entry, and known limitations. It does not use the required Summary and Testing headings, and it omits the required Demo Video and Checklist sections for this behavior change.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[teamleaderleo]

Review (subagent, parent→head so the diff matches the +92/-1 you wrote, not origin/main...HEAD; the merge base is 8e71daa1441 and main is 77 ahead).

The guard is the right mechanism and it is better than it looks: transcriptRpcGuard snapshots the generation, the TranscriptState and target object identities, and the agentSessionId/surfaceId values, so it catches in-place mutation of the target as well as replacement and re-attachment with an identical target and path. The test exercises all four. dispose() bumps the generation before its if (!st) return, so a teardown with no reader attached still fences. Load-bearing: reverting transcript.ts + server.ts to this PR's base makes the new test fail with actual: [ [Object] ], expected: [].

should-fix — the error-path net main has is gone and was not restored. Main wraps all three RPC call sites: void rpc(...).then(...).catch(err => sess.emit(...)) in stop, try/catch around send, try/catch in focusTranscriptTerminal. This branch has a bare .then() (adapters/transcript.ts:555-557), a bare await (:546-548) and no try/catch (:53-60), and all three are consumed as unhandled promises (server.ts:588, server.ts:2253). If the .then callback itself throws, say sess.emit throwing while broadcasting to a closing socket, there is nothing to catch it, and this file's own comment states the stakes: "an unhandled rejection ends the whole sidecar." Not reachable through production cmuxRpc today because #16065 gave it a top-level catch, but reachable through setTranscriptRpcForTest and through a throwing emit. Strictly it was #16065 that removed them, but this PR edits all three of those exact lines, and the merge into main forces the question anyway: transcript.ts has 4 conflict blocks against main and 3 of them are exactly main's try/catch versus this guard. The correct resolution there is keep both, so it is cheaper to put the net back here.

nit — dispose() now has a side effect on every call (monotonic generation bump), so it is no longer a no-op on an already-disposed session. Worth a comment.

Order: needs #16065 first (transcriptRpcErrorEvent, CmuxRpcResult.errorCode). Resolve server.ts:19 by keeping both imported names; #16069 adds transcriptTarget on that same line and it is the one conflict between these two PRs.

tsc exit 0, 44 pass / 0 fail, 26 scripts OK.

— Raindrop g2 🫧
Run: run_worker_20260930_3fc64ba6

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[github-actions]

CI failure attribution

CI failed on 0acefd1996 (run 36871209914 attempt 1): 1 code.

Job	Verdict	Why
macos / macOS compile admission	code	a compile error

Matched log lines

macos / macOS compile admission: /tmp/cmux-ci/src/Sources/Update/UpdateTitlebarAccessory.swift:989:49: error: invalid redeclaration of 'cmuxAccent'

Not re-run automatically: macos / macOS compile admission is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.