From 6d05fb8a525a4e2d822836ee9b0bcf99fa9e85ec Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Wed, 9 Sep 2026 12:38:02 -0700 Subject: [PATCH] fix(worktrees): keep a proven-exited session from refusing removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The structured sweep re-observes after a close that reported `stopped: false`, but folded a proven `exited` into `unverifiable` — so a close that threw past its own observation, or one whose death evidence landed a beat later, refused a delete over a child that is demonstrably gone. That is the defect this sweep exists to remove, and the PTY gate it mirrors never refuses on a proven exit. Take the proof, and run the tab retirement the close skipped when it gave up: a chat tab left behind re-attaches a released session pointing at a workspace that is about to be deleted. --- ...ructured-session-worktree-teardown.test.ts | 27 +++++++++++++++++++ .../structured-session-worktree-teardown.ts | 12 ++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/main/runtime/structured-session-worktree-teardown.test.ts b/src/main/runtime/structured-session-worktree-teardown.test.ts index 8ec87f7f9c3..5918eec3a6c 100644 --- a/src/main/runtime/structured-session-worktree-teardown.test.ts +++ b/src/main/runtime/structured-session-worktree-teardown.test.ts @@ -50,6 +50,8 @@ function installHost(options: { stuck?: Set /** Sessions the host drops without death evidence, so the observation is `unverifiable`. */ unverifiable?: Set + /** Sessions whose child dies and is recorded dead, but whose close then fails past that point. */ + settledThenThrows?: Set /** Blocks every close, to exercise the shared sweep budget without fake timers. */ closeGate?: Promise }): { closed: string[] } { @@ -74,6 +76,9 @@ function installHost(options: { record.lease.claimStatus = 'released' record.lease.deathEvidence = { kind: 'exit-observed', detail: 'closed', observedAt: 1 } } + if (options.settledThenThrows?.has(sessionId)) { + throw new Error('the event sink could not be flushed') + } } } // `observeStructuredWorker` reads the record through the same host, so keep them consistent. @@ -201,6 +206,28 @@ describe('worktree teardown and structured agent sessions', () => { warn.mockRestore() }) + it('takes the proof when a failed close is re-observed as exited', async () => { + // `closeStructuredAgentSessionChild` reports `stopped: false` for anything that throws past its + // own observation, and for a record whose death evidence lands after it read. The re-read here + // can still PROVE the exit — refusing a delete over a child that is demonstrably gone is the + // defect this whole sweep exists to remove, so the proof has to win over the close's verdict. + const retired: string[] = [] + const runtime = { + stopTerminalsForWorktree: async () => ({ stopped: 0 }), + retireStructuredAgentSessionTabFromSnapshot: (sessionId: string) => { + retired.push(sessionId) + return true + } + } as never + installHost({ records: [record('s1', WORKTREE)], settledThenThrows: new Set(['s1']) }) + await expect( + killAllProcessesForWorktree(WORKTREE, { ...destructiveDeps(), runtime }) + ).resolves.toMatchObject({ structuredStopped: 1 }) + // Retired here because the close gave up before its own retirement step, and a chat tab left + // behind re-attaches a released session pointing at a workspace that is about to be deleted. + expect(retired).toEqual(['s1']) + }) + it('leaves the best-effort reconciliation paths alone', async () => { // Those callers repair state and delete nothing, so a refusal there would wedge a repair. installHost({ records: [record('s1', WORKTREE)] }) diff --git a/src/main/runtime/structured-session-worktree-teardown.ts b/src/main/runtime/structured-session-worktree-teardown.ts index 81876f46285..2996ff988dc 100644 --- a/src/main/runtime/structured-session-worktree-teardown.ts +++ b/src/main/runtime/structured-session-worktree-teardown.ts @@ -29,6 +29,7 @@ import { STILL_LIVE_DETAIL_PREFIX } from '../../shared/worktree/removal' import { getStructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-registry' import { observeStructuredWorker } from './structured-worker-authority' import { closeStructuredAgentSessionChild } from './structured-agent-session-close' +import { retireSettledStructuredWorkerTab } from './structured-agent-session-tab-retirement' import type { OrcaRuntimeService } from './orca-runtime' export type LiveStructuredSessionInWorkspace = { @@ -165,7 +166,16 @@ export async function closeStructuredSessionsForWorktree( // Re-observed rather than reusing the close's own reason string: what the user is asked to // waive is the state AFTER the attempt, and a close that threw never reached an observation. const status = observeStructuredWorker({ sessionId: session.sessionId }).status - unstopped.push({ ...session, status: status === 'live' ? 'live' : 'unverifiable' }) + if (status === 'exited') { + // The re-read can PROVE the exit a failed close could not — it threw past its own + // observation, or the record's death evidence landed after it read. Refusing on a child + // that is demonstrably gone is the defect this sweep exists to remove, so take the proof + // and run the retirement `closeStructuredAgentSessionChild` skipped when it gave up. + retireSettledStructuredWorkerTab(session.sessionId, runtime) + closed += 1 + continue + } + unstopped.push({ ...session, status }) } return { closed, unstopped } }