Repository navigation
Fix native chat close and stale tab reopening - #27583
brennanb2025 wants to merge 3 commits into
Conversation
|
Closing a native chat could hide its tab while the agent continued running; an older create, reopen or startup restore could bring it back. The execution host now owns one close operation, cancels older publication attempts, and attempts stop despite bookkeeping failures. History is preserved, and reopening waits at most three seconds for an unfinished close. The PR stands on main after #27565 closed. Main's landed #28354 saved-layout write shares the cancellation guard with live tab publication; startup reconciliation stays inside the tracked restore lifetime. Other tab kinds follow main. Existing-client compatibility is preserved; the renderer command switch remains #27586. Requested correction complete at 262 focused tests across 24 explicit files and normal Node compilation passed. Tests/fixtures/configuration: +966/-66 versus production +980/-276 against pinned mainf31. Prepush passed 21/22, with only the documented local Bun wrapper limitation. The focused review is complete. The ONE new-head watch exited on static analysis. Both audit errors are from newer main: The complete historical inventory also includes the mainf31 compiler-cache workflow mismatch and TWO mainf31 openMobileFile arity failures (test expects3; owner has4). The coordinator reproduced these on main; root read actual source/blob evidence and an independent compiler-cache control. The prior standalone browser route/opaque timeout remains unexplained: no changed production modules are in its bundle, but a main browser reproduction and flakiness are unproven. None of those cases ran in this new CI. No further main merge, unrelated fix, rerun or replacement watch followed. M4Air production QA at Fresh SSH QA used an owned Ubuntu 20.04 host. The busy terminal's real X and “Stop and Close” removed its child before its natural deadline and left the next host list empty. A genuine empty chat was created without a first message, closed through its rendered X, and directly revealed as the same session. Busy SSH chat remains unverified; this does not establish SSH History Resume or retained nonempty messages. Setup scripts and large-history latency were not exercised in these UI scenarios. Uncropped fresh proof is in the PR body. Owned QA processes, listeners, containers, keys, isolated profiles, controller configuration, and the M4Air |
ce3ba42 to
ec36bdb
Compare
ELI5
Closing a native chat could hide its tab while the agent kept running. An older create, reopen, or startup restore could bring the tab back, and closing a saved conversation could fail before its owner was loaded.
The computer running the chat now handles one close operation. It cancels older attempts to show the chat, hides the tab, and attempts to stop execution even if saving visibility or updating the listing fails. History is preserved. An explicit reopen waits at most three seconds for an unfinished close, then returns the existing retry response if necessary.
What Changed
This PR stands on main after #27565 closed. Terminal, browser, and editor closing follows main; this diff concerns native chats. The renderer switch to the single close command remains #27586. Existing clients remain compatible through
session-tabs.close-agent-session-by-id.v1.Why
The execution host already owns the agent and conversation. Completing close there covers every caller; ordering commands in one renderer would leave other callers exposed. Cancellation belongs to pending operations rather than a permanent closed flag, and bounded waiting lets users retry when close never settles.
Intended differences from the common pattern: closing preserves history, a pending action can start before a saved owner is loaded, and trusted local window routing supports the existing confirmation flow. The desktop command change remains temporary until #27586 lands.
Linked Issue
Part of the tab-close lifecycle series beginning with #27561. No separate issue.
Visual Proof
Production QA captures on
9ec374103db, before and after each action on that same commit. Current0c9b3ee84cdchanges only a test expectation; the production code is identical. Hidden renderers; no desktop activation.M4Air macOS, real Claude
claude-opus-5-5[1m]. Newly generated assistant text grew before close. The original process was gone at the post-close check, and 40 observations over 59 seconds showed no returning chat tab.The real History Resume was clicked 1.4 seconds after starting close, with no history refresh between. It reopened the same conversation once with retained assistant text; 50 observations over 75 seconds showed no duplicate or later loss. The original Claude process was gone before Resume.
SSH control on a disposable Ubuntu 20.04 host: the identified busy terminal's real X and “Stop and Close” removed its
sleep 600child before its natural deadline, and the next full host list was empty.Testing
Current head is
0c9b3ee84cdafd71077fcea477151d70c0fbe7d0. Its only change after QA head9ec374103dbupdates the existing permission-create expectation to require the intended third attach argument containing an AbortSignal. The permission seed and two-argument ensure expectation are preserved. No production code changed. The scan of matching attach/create assertions found no additional proven mismatch.262 tests across 24 explicit files passed: the original 23-suite composition plus the permission-create suite. The three existing windowless-layout suites remain unchanged. The combined restore/close regression checks saved and live tabs, preserves the untouched sibling, and reopens once. Normal Node compilation passed. Prepush completed 21/22 checks; the only failure is the documented local Bun 1.3 compiler wrapper incompatibility, rather than a source diagnostic.
Counting fixtures and test configuration as tests, the own diff against pinned main
f31aada5is tests +966/-66 versus production +980/-276 (1,032 versus 1,256 changed lines). No lockfile or documentation changes are included.The one new-head fail-fast watch stopped on static analysis, at “Reject low-evidence patterns.” Both errors are in
mobile/src/tasks/use-mobile-jira-connection-state.test.ts: theclientargument at line 16 andnextClientat line 34 use the broadobjecttype.This is confirmed newer-main code: CI tested merge
2ab9e7b7, combining this PR with main0c05e6d3. The failing file is absent from this PR's head and byte-identical between the CI merge and that main parent (blob286bae38a23fb8e2122fd2bf95b623288674db7c); it was introduced by #28616. The exact CI diagnostics and both immutable sources were read and compared. No separate main lint run is claimed. Local prepush used this branch, where that file does not exist.The fail-fast snapshot had six passing checks, one failure, one queued aggregate and 24 skipped checks. Repository guards, Mobile verify, line count, and all three native smoke checks passed. Unit tests never ran in this CI attempt because static analysis failed; neither a new CI compiler pass nor an aggregate pass is claimed. The compiler step's success status was a guarded no-op. Local compilation did run and pass.
Historical CI inventory, superseding the earlier incomplete report:
0c9b3ee84cd: the permission-create assertion omitted the intended cancellation argument. It passed on main and failed on9ec; both create/ensure cases and the full focused set now pass locally.f31:headless-detector-compiler-cache.test.mjs:199readsworkflow.on.push.pathsafter ci: stop daily and post-merge headless-server checks #28664 removed the push trigger. An exact-file control reproduced it; test/workflow contents match main.f31:orca-runtime-files-artifact-facade.test.ts:125and:163expect three arguments foropenMobileFile, while the unchanged main owner takes four. The coordinator ran both main controls, and root verified the test/owner/facade byte identity.route/opaquenative-control probe timed out. Its unchanged bundle contains none of this PR's changed production modules. A main browser reproduction and flakiness remain unproven. It did not run in the new CI attempt.The requested assertion correction is complete. Required CI remains blocked by the newer-main audit errors above. No further main merge, CI rerun, replacement watch or unrelated source change was made.
Review
Focused adversarial review of the standalone diff found no proven P0/P1/P2 issue. It covered removing the closed parent, SSH, folder workspaces, startup membership, and older clients. The main integration also passed the combined regression and final-head QA above.
Agent skill upstream boundary
Notes
#28354 has landed on main; its saved-layout write now runs inside the same single
!input.signal?.abortedcheck as showing the tab.Busy SSH chat remains unverified: no installed, signed-in agent was used on that fixture. The SSH chat case created a genuine empty conversation without a first message, closed it through X, then used direct host reveal for the same session; it does not establish SSH History Resume or retained nonempty messages. Live Resume occurred after close completed; held-operation tests cover cancellation races. Setup scripts were not run in the UI scenarios, and large-history latency was not measured. If saving visibility fails, keeping the tab closed across a later restart remains best effort.
Owned QA apps, helpers, containers, temporary keys, isolated profiles, controller configuration, and the M4Air
qa-pr-27583worktree are removed. Raw evidence is preserved outside the checkouts. This PR remains draft; required CI is blocked by the newer-main audit errors above.Checklist