Repository navigation
The window closes a chat with one command on hosts that support it - #27586
brennanb2025 wants to merge 12 commits into
Conversation
…osts that close it by id - a host advertising session-tabs.close-agent-session-by-id.v1 gets only session.tabs.close; older hosts keep both commands, chosen by the same capability lookup the create path uses - a reveal that answers success ends this window's earlier close of that chat, so the reopened tab is not hidden by it; a frame it may already have hidden is re-read - the local close owner is deleted: the window removes the chat before main relays the close, so the relay answers no such tab, which main reads as already closed
…the reveal behind its close
…nto brennanb2025/tab-close-chat-one-command # Conflicts: # src/renderer/src/runtime/structured-agent-session-close.ts # src/renderer/src/runtime/structured-agent-session-tab-retirement.test.ts # src/renderer/src/runtime/structured-agent-session-tab-retirement.ts
|
Closing a native chat previously sent overlapping commands to stop the agent and remove its tab. Reopening could race that close, and a replaced host could receive a close selected for its predecessor. This draft now sends one close to capable hosts, retains both commands for older hosts, preserves the host identity through delivery, and clears only the matching earlier close after a successful reopen. Review loop complete at
Owned remote QA rigs, sessions and worktree were cleaned up; evidence remains outside the checkout. The PR remains a draft. When composing #27583, retain its originating-window propagation and early close registration together with this change. |
ELI5
Closing a native chat sent separate commands to stop its agent and remove its tab. Those commands could overlap, and delayed host updates could make the closed tab return. This change lets a host that supports closing chats by ID handle both jobs with one close command. Older hosts still receive both commands so their agents stop too.
What Changed
The window keeps the host identity it used to choose the close command. For paired hosts, it checks the identity through the same encrypted connection that will deliver the close, then checks the current pairing again immediately before sending. A restarted or replaced host cannot inherit a close selected for its predecessor. If the capability lookup fails, the tab uses the existing failed-close recovery instead of sending commands without that proof.
Reopening waits up to five seconds for an earlier close to finish. A successful reopen clears only that earlier close record, so a newer close still wins and a second close sends a fresh command. An already confirmed local chat close also carries its originating window through main's existing request context; that window does not receive a second confirmation request when host cleanup finishes late.
Why
The execution host owns stopping the agent and removing it from published tab lists. Keeping those together removes overlapping destructive commands on capable hosts. Reusing the existing status check, request context and close ledger preserves ownership without a second close owner or new remote wire protocol. Sequencing the two old commands would retain overlapping close paths; clearing the close record at the reopen click would let older inventory show the tab too early.
Intended costs: paired closes include a read-only status check on their delivery connection. If the earlier close cannot be confirmed within five seconds, reopening asks the user to retry; repairing the pairing can also require a retry. These bounds keep a pending operation from holding the user indefinitely.
Linked Issue
Follow-up to #27205. Stacked on #27561; host counterpart #27583 is independent, and either side can land first.
Visual Proof
Hidden Electron checks on M4Air using renderer
37083530bc192ee0cd9775f8d58d8e0ffd519391, before the later parent/main sync, assembled locally with capable hostcc3975ac71ecd7ab0a457287d6b539522155ff52and legacy hostcfb63dc6ca72f33171261a5e7ec05f0e87859a0b. These are before-click and after-close views of the same build; they are not comparisons with an older window build.Testing
Current head:
046d493ff428b13bc2cce49ef9bf4e3bfbd0b193, incorporating parenta26946013e1ea2fb7c825ec178c63d56e46177b3. The parent includes 218 inherited main paths. The merge was conflict-free: all 42 non-locale paths changed by this PR remain byte-identical to the previously reviewed child, and all parent/child translation changes compose correctly. The earlier test-only parent update retains the asynchronous assertions and makes their expected host-identity options explicit.The real-agent screenshots above remain evidence from exact commit
37083530bc1. The later inherited-main composition was validated with automated checks; real-agent UI QA was not repeated.22 targeted files / 236 tests, fresh node and renderer typechecks and the scoped layout-writer check passed on
046d493ff428. The earlier independent connection-API merge review passed 55 targeted tests; the latest parent composition was separately checked for preservation.CI for current head
046d493ff428passed: 19 checks passed and 20 were skipped. The single blocking CI watch exited successfully. Localorca-ci-checkspassed 22 of 23 checks; the installed Bun cannot run the remainingcheckcommand. Hosted static analysis and typechecks passed.Real Claude (the 1M-context model) and Codex chats were closed while working through the rendered tab control on both assembled host versions. Each capable-host row sent exactly one
session.tabs.closeand zeroagentSession.close; each legacy-host row sent one of each. A scratch preload recorded method names while passing arguments and results through unchanged.Recorded provider processes exited before rig cleanup. Successful host inventory calls and rendered-tab observations kept each tab absent for 18.8–19.6 seconds, including beyond the 15-second pending-close interval. Full screenshots above were inspected by the coordinator.
Automated coverage exercises close/reopen ordering, capability rejection, host replacement, encrypted delivery connections, failed-close recovery and late confirmation. Actual paired/SSH transport, Windows and Linux UI, reopening after close, a second close after reopen and a delayed pinned-tab relay were not exercised in this rendered run. No measured performance claim.
Manually tested through the hidden rendered app on M4Air
Automated tests cover capability selection, close/reopen ordering, delayed confirmation and encrypted connection ownership
Review
The completed adversarial rounds and fresh recovery review found and verified fixes for host replacement during close, reopen/close races and delayed duplicate confirmation. The recovery review additionally reproduced a failed capability lookup being treated as legacy support without a host identity; that now follows failed-close recovery. The inherited store import cycle is resolved by the parent change.
Architecture verdict: keep the design. Readiness and dedicated performance review found no remaining proven blocker; a fresh narrow review of the final parent merge verified the new connection API and preservation of these fixes. No open correctness finding remains in the review ledger. The pre-existing paired starting-chat limitation below is deferred. The latest parent merge preserves the reviewed close logic; this composition check does not constitute a new review of all inherited main changes.
Agent skill upstream boundary
Notes
Host counterpart #27583 now includes the local originating-window propagation. When combining the branches, preserve that path together with its early close registration by chat ID and extracted close implementation. Missing origin, other windows, CLI and paired callers retain their existing confirmation path. The originating window is local request context, not a remote wire field.
Reopening a chat closed while it was still starting on a paired host remains a known, pre-existing limitation.
Windows UI QA is unverified: the high-spec Codex driver was signed out and OpenCode was unavailable; the optional low-spec driver refused requests because its accounts required re-authentication. No tools or account settings were changed. SSH and Linux UI QA were not run. Mixed-version and replacement-host behavior was tested at the encrypted connection layer, but the assembled M4Air rows do not establish live paired/SSH behavior. Folder/worktree ownership and supported git providers were assessed in source review; no platform-specific path, shortcut or Git command was introduced.
Checklist