From 5be7ba61d49e2f017772ab9edfdcfea6478466fe Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:56:14 -0700 Subject: [PATCH] fix(terminal): close descendant teardown races --- config/reliability-gates.jsonc | 20 +++- docs/reference/terminal-session-lifecycle.md | 8 +- src/main/daemon/session.ts | 19 +++- src/main/daemon/terminal-agent-teardown.ts | 94 +++++++++++++++++ src/main/daemon/terminal-host.test.ts | 101 +++++++++++++++++++ src/main/daemon/terminal-host.ts | 31 +++--- src/main/daemon/terminal-immediate-kill.ts | 34 ------- src/main/pty-descendant-termination.test.ts | 6 +- src/main/pty-descendant-termination.ts | 5 +- src/main/runtime/worktree-teardown.test.ts | 28 +++++ src/main/runtime/worktree-teardown.ts | 52 +++++----- 11 files changed, 310 insertions(+), 88 deletions(-) create mode 100644 src/main/daemon/terminal-agent-teardown.ts delete mode 100644 src/main/daemon/terminal-immediate-kill.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 2504aff9c20..1bc686393c0 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -711,16 +711,16 @@ "ssh", "runtime" ], - "coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Deterministic tests cover daemon and SSH routing, local/daemon pending-snapshot ownership, duplicate-kill completion sharing, bounded/fresh/coalesced process-table reads, source-scan timestamp identity, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, Windows, WSL, SSH, and remote-runtime process evidence remains pending.", + "coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Deterministic tests cover daemon and SSH routing, local/daemon pending-snapshot ownership across natural exit, graceful-to-immediate kill upgrades, duplicate-kill completion sharing, locale-stable bounded/fresh/coalesced process-table reads, concurrent bulk teardown, source-scan timestamp identity, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, Windows, WSL, SSH, and remote-runtime process evidence remains pending.", "motivatingLinks": [ "https://github.com/stablyai/orca/pull/8628", "https://github.com/stablyai/orca/pull/8706" ], - "invariant": "Close permanently removes the owned provider session, agent descendants, and resume authority even when no TerminalPane is mounted; a terminating id cannot reattach or start duplicate cleanup, and duplicate callers await the same completion; process-table work is bounded, fresh for each post-start request, and same-turn coalesced; detach and park preserve ownership; aliases prevent a detached agent's immutable physical pane key from being retired with its former tab.", - "oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; an agent-marked PTY's detached-pgid child is alive before close and absent afterward; unit tests reject pending-capture reattach, share duplicate teardown completion, coalesce 20 same-turn process-table requests, force a successor scan for later arrivals, retain the source scan's timestamp, bound both read phases, avoid ambiguous SIGKILL, and assert canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.", + "invariant": "Close permanently removes the owned provider session, agent descendants, and resume authority even when no TerminalPane is mounted; a terminating id remains reserved through natural exit, duplicate callers await the same completion, and immediate teardown upgrades any graceful request without signalling a recycled PID; process-table work is locale-stable, bounded, fresh for each post-start request, and same-turn coalesced, including bulk worktree cleanup; detach and park preserve ownership; aliases prevent a detached agent's immutable physical pane key from being retired with its former tab.", + "oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; an agent-marked PTY's detached-pgid child is alive before close and absent afterward; unit tests keep a naturally exited id reserved without re-killing its PID, upgrade pending and post-snapshot graceful kills to immediate, force ps into the C locale, begin bulk shutdowns together, share duplicate teardown completion, coalesce 20 same-turn process-table requests, force a successor scan for later arrivals, retain the source scan's timestamp, bound both read phases, avoid ambiguous SIGKILL, and assert canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.", "commands": [ "pnpm dlx node@24 ./node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/agent-hooks/server-pane-authority.test.ts src/main/ipc/agent-hooks.test.ts src/main/ipc/agent-pane-authority-ownership.test.ts src/main/ipc/pty-management.test.ts src/main/persistence.test.ts src/renderer/src/store/slices/agent-pane-authority.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/renderer/src/store/slices/terminal-tab-retirement.test.ts src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/main/pty-descendant-termination.test.ts src/main/daemon/session.test.ts src/main/daemon/terminal-host.test.ts src/main/providers/local-pty-provider.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/main/pty-descendant-termination.test.ts src/main/daemon/session.test.ts src/main/daemon/terminal-host.test.ts src/main/providers/local-pty-provider.test.ts src/main/runtime/worktree-teardown.test.ts", "pnpm run test:e2e -- tests/e2e/terminal-parked-close-retirement.spec.ts --workers=1", "pnpm run test:e2e -- tests/e2e/agent-descendant-process-kill.spec.ts --workers=1" ], @@ -734,6 +734,7 @@ "src/main/daemon/session.test.ts", "src/main/daemon/terminal-host.test.ts", "src/main/providers/local-pty-provider.test.ts", + "src/main/runtime/worktree-teardown.test.ts", "src/renderer/src/store/slices/agent-pane-authority.test.ts", "src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts", "src/renderer/src/store/slices/terminal-tab-retirement.test.ts", @@ -768,6 +769,7 @@ "assertions": [ "20 same-turn process-table requests execute one fresh scan while later arrivals receive a successor scan", "snapshot and escalation readers stop at their deadline", + "production ps reads force locale-independent C timestamps", "the source scan timestamp survives request resolution and capture-second identities are never escalated with SIGKILL" ] }, @@ -775,9 +777,17 @@ "file": "src/main/daemon/terminal-host.test.ts", "assertions": [ "agent immediate kill rejects reattach while descendant capture is pending", + "a naturally exited session id remains reserved until capture finishes without force-killing its retired PID", + "graceful teardown upgrades to immediate both during and after descendant capture", "duplicate immediate kill starts one descendant sweep" ] }, + { + "file": "src/main/runtime/worktree-teardown.test.ts", + "assertions": [ + "all owned provider shutdowns start before any completes so process-table snapshots can coalesce" + ] + }, { "file": "tests/e2e/agent-descendant-process-kill.spec.ts", "assertions": [ @@ -810,7 +820,7 @@ }, "performanceBudget": { "required": true, - "evidence": "The close path is user-triggered and bounded by canonical live-owner indexing. Production ps has a 1s kill timeout; 20 same-turn requests execute one fresh process-table read, while a request arriving after a scan starts queues one successor rather than consuming stale signal targets. Completed tables are never reused. Escalation uses the same bounded coordinator, and kill-all store scale remains covered by terminal-session.kill-all-surface-cleanup." + "evidence": "The close path is user-triggered and bounded by canonical live-owner indexing. Production ps has a 1s kill timeout; 20 same-turn requests execute one fresh process-table read, while a request arriving after a scan starts queues one successor rather than consuming stale signal targets. Completed tables are never reused. Bulk worktree shutdown begins owned sessions concurrently so the production coordinator can coalesce the initial scan instead of serializing one subprocess per session. Escalation uses the same bounded coordinator, and kill-all store scale remains covered by terminal-session.kill-all-surface-cleanup." }, "promotionCriteria": [ "Accumulate 100 clean runs or 14 days on required CI platforms.", diff --git a/docs/reference/terminal-session-lifecycle.md b/docs/reference/terminal-session-lifecycle.md index 4ab46df2325..2a9bab11789 100644 --- a/docs/reference/terminal-session-lifecycle.md +++ b/docs/reference/terminal-session-lifecycle.md @@ -81,9 +81,11 @@ could invalidate intentional long-lived `worktree-sleep` checkpoints. reach — via `pty-descendant-termination.ts` (bounded fresh snapshot with same-turn coalescing, SIGTERM, grace window, then identity-safe SIGKILL). Completed process tables are never reused as signal targets, and identity - checks use the source scan's timestamp. A session is marked as terminating - before capture so reattach cannot race teardown. Windows and SSH-hosted PTYs - keep the previous foreground-tree contract for now. + checks use C-locale timestamps from the source scan. A session is marked as + terminating before capture and keeps teardown ownership through natural exit, + so reattach, duplicate kill, and graceful-to-immediate upgrade cannot race the + snapshot. Windows and SSH-hosted PTYs keep the previous foreground-tree + contract for now. - Changing agent-provider resume commands or permission flags. - Making a UI close wait for a remote process to exit before the tab disappears. - Replacing worktree sleep with tab close. Sleep remains resumable by design. diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index 5d9898d8c5a..74b11f7af2f 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -284,12 +284,25 @@ export class Session { // signal; the kill timer below starts now, so force-dispose timing is // unaffected. void killWithDescendantSweep(this.subprocess.pid, () => { - if (this._state !== 'exited') { - this.subprocess.kill() - } + this.signalTerminationRoot() }) } + this.scheduleForceDisposeFallback() + } + /** Signals a root whose descendant snapshot has completed. */ + signalTerminationRoot(): void { + if (this._state !== 'exited') { + this.subprocess.kill() + } + } + + /** Starts the existing graceful-kill deadline when a coordinator owns the + * snapshot-first portion of teardown. */ + scheduleForceDisposeFallback(): void { + if (this.killTimer) { + return + } this.killTimer = setTimeout(() => { if (this._state !== 'exited') { this.forceDispose() diff --git a/src/main/daemon/terminal-agent-teardown.ts b/src/main/daemon/terminal-agent-teardown.ts new file mode 100644 index 00000000000..73bb9c88eb4 --- /dev/null +++ b/src/main/daemon/terminal-agent-teardown.ts @@ -0,0 +1,94 @@ +import { killWithDescendantSweep } from '../pty-descendant-termination' +import type { Session } from './session' + +type AgentTeardownOperation = { + promise: Promise + immediate: boolean +} + +/** Owns agent teardown by session id until descendant capture and root + * signalling finish, even when the root exits and its Session is reaped. */ +export class TerminalAgentTeardown { + private operations = new Map() + + constructor( + private sessions: ReadonlyMap, + private reapSession: (sessionId: string) => void + ) {} + + get(sessionId: string): Promise | undefined { + return this.operations.get(sessionId)?.promise + } + + requestImmediate(sessionId: string): Promise | undefined { + const pending = this.operations.get(sessionId) + if (pending) { + pending.immediate = true + } + return pending?.promise + } + + forceImmediate(sessionId: string, session: Session): void { + this.finishImmediate(sessionId, session) + } + + kill(sessionId: string, session: Session, immediate: boolean): void | Promise { + const pending = this.operations.get(sessionId) + if (pending) { + // Why: an immediate caller is a stronger teardown request and must not + // acknowledge a still-graceful root kill while capture is pending. + pending.immediate ||= immediate + return pending.promise + } + + if (!session.beginTermination()) { + // A completed graceful sweep can leave the root alive during its grace + // window. Immediate teardown may safely escalate once no scan is pending. + if (immediate && session.isAlive && session.isTerminating) { + this.finishImmediate(sessionId, session) + } + return + } + if (!immediate) { + session.scheduleForceDisposeFallback() + } + + const entry: AgentTeardownOperation = { + promise: Promise.resolve(), + immediate + } + const operation = Promise.resolve( + killWithDescendantSweep(session.pid, () => { + // Why: natural exit reaps the PID while ps is running. Never signal that + // stale numeric PID after the Session no longer represents a live root. + if (!session.isAlive) { + return + } + if (entry.immediate) { + this.finishImmediate(sessionId, session) + } else { + session.signalTerminationRoot() + } + }) + ) + entry.promise = operation + this.operations.set(sessionId, entry) + const clearOperation = (): void => { + if (this.operations.get(sessionId) === entry) { + this.operations.delete(sessionId) + } + } + void operation.then(clearOperation, clearOperation) + return operation + } + + private finishImmediate(sessionId: string, session: Session): void { + // Why: the old root may exit and a new same-id Session may appear after + // capture. Only this exact live Session is safe to force-kill and reap. + if (this.sessions.get(sessionId) !== session || !session.isAlive) { + return + } + session.forceKillAndDisposeSubprocess() + this.reapSession(sessionId) + } +} diff --git a/src/main/daemon/terminal-host.test.ts b/src/main/daemon/terminal-host.test.ts index d3c573b355d..24d255526f2 100644 --- a/src/main/daemon/terminal-host.test.ts +++ b/src/main/daemon/terminal-host.test.ts @@ -495,6 +495,107 @@ describe('TerminalHost', () => { expect(second).toBe(first) expect(lastSubprocess.forceKill).not.toHaveBeenCalled() }) + + it('keeps a naturally-exited id reserved until teardown finishes without re-killing its pid', async () => { + let completeSweep!: () => void + killWithDescendantSweepMock.mockImplementation( + (_pid: number, finish: () => void) => + new Promise((resolve) => { + completeSweep = () => { + finish() + resolve() + } + }) + ) + await host.createOrAttach({ + sessionId: 'agent-natural-exit', + cols: 80, + rows: 24, + launchAgent: 'claude', + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + const retiredSubprocess = lastSubprocess + + const killing = host.kill('agent-natural-exit', { immediate: true }) + retiredSubprocess._onExitCb?.(0) + await expect( + host.createOrAttach({ + sessionId: 'agent-natural-exit', + cols: 80, + rows: 24, + launchAgent: 'claude', + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + ).rejects.toThrow('Session not found') + + completeSweep() + await killing + expect(retiredSubprocess.forceKill).not.toHaveBeenCalled() + + await expect( + host.createOrAttach({ + sessionId: 'agent-natural-exit', + cols: 80, + rows: 24, + launchAgent: 'claude', + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + ).resolves.toEqual(expect.objectContaining({ isNew: true })) + expect(spawnFn).toHaveBeenCalledTimes(2) + }) + + it('upgrades a pending graceful agent teardown when immediate kill arrives', async () => { + let completeSweep!: () => void + killWithDescendantSweepMock.mockImplementation( + (_pid: number, finish: () => void) => + new Promise((resolve) => { + completeSweep = () => { + finish() + resolve() + } + }) + ) + await host.createOrAttach({ + sessionId: 'agent-upgrade-kill', + cols: 80, + rows: 24, + launchAgent: 'claude', + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + + const graceful = host.kill('agent-upgrade-kill') + const immediate = host.kill('agent-upgrade-kill', { immediate: true }) + expect(immediate).toBe(graceful) + expect(lastSubprocess.kill).not.toHaveBeenCalled() + expect(lastSubprocess.forceKill).not.toHaveBeenCalled() + + completeSweep() + await Promise.all([graceful, immediate]) + expect(lastSubprocess.kill).not.toHaveBeenCalled() + expect(lastSubprocess.forceKill).toHaveBeenCalledOnce() + expect(lastSubprocess.dispose).toHaveBeenCalledOnce() + }) + + it('force-kills when immediate teardown follows a completed graceful snapshot', async () => { + killWithDescendantSweepMock.mockImplementation(async (_pid: number, finish: () => void) => + finish() + ) + await host.createOrAttach({ + sessionId: 'agent-post-snapshot-upgrade', + cols: 80, + rows: 24, + launchAgent: 'claude', + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + + await host.kill('agent-post-snapshot-upgrade') + expect(lastSubprocess.kill).toHaveBeenCalledOnce() + expect(lastSubprocess.forceKill).not.toHaveBeenCalled() + + host.kill('agent-post-snapshot-upgrade', { immediate: true }) + expect(lastSubprocess.forceKill).toHaveBeenCalledOnce() + expect(lastSubprocess.dispose).toHaveBeenCalledOnce() + }) }) describe('signal', () => { diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index ff13ed67234..d7f0b6609a7 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -4,10 +4,14 @@ import { shellPathSupportsPtyStartupBarrier } from './shell-ready' import { resolveProcessCwd } from '../providers/process-cwd' import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' import { buildStartupCommandSubmission } from '../../shared/startup-command-submission' -import type { SessionInfo, TakePendingOutputResult, TerminalSnapshot } from './types' -import { SessionNotFoundError } from './types' +import { + SessionNotFoundError, + type SessionInfo, + type TakePendingOutputResult, + type TerminalSnapshot +} from './types' import type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract' -import { TerminalImmediateKillCoordinator } from './terminal-immediate-kill' +import { TerminalAgentTeardown } from './terminal-agent-teardown' export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract' @@ -42,7 +46,7 @@ export type TerminalHostOptions = { export class TerminalHost { private sessions = new Map() - private immediateKills = new TerminalImmediateKillCoordinator() + private agentTeardowns = new TerminalAgentTeardown(this.sessions, (id) => this.reapSession(id)) private killedTombstones = new Map() private spawnSubprocess: TerminalHostOptions['spawnSubprocess'] private onFinalCheckpoint: TerminalHostOptions['onFinalCheckpoint'] @@ -66,7 +70,7 @@ export class TerminalHost { // Why: async descendant capture must finish before anyone can attach or // dispose/recreate this id. Disposing here would kill the root before the // snapshot and reattaching would hand out a doomed session. - if (existing?.isTerminating) { + if (this.agentTeardowns.get(opts.sessionId) || existing?.isTerminating) { throw new SessionNotFoundError(opts.sessionId) } @@ -198,21 +202,18 @@ export class TerminalHost { } kill(sessionId: string, opts: { immediate?: boolean } = {}): void | Promise { - const pending = opts.immediate ? this.immediateKills.get(sessionId) : undefined + const pending = this.agentTeardowns.get(sessionId) if (pending) { - return pending + return opts.immediate ? this.agentTeardowns.requestImmediate(sessionId) : pending } const session = this.getAliveSession(sessionId) this.recordTombstone(sessionId) + if (session.launchAgent) { + return this.agentTeardowns.kill(sessionId, session, opts.immediate === true) + } if (opts.immediate) { - const finish = (): void => { - session.forceKillAndDisposeSubprocess() - // Why: the immediate path tears down synchronously without firing the - // session's onExit hook, so reap it here. The graceful path below - // funnels through Session.handleSubprocessExit -> onExit -> reapSession. - this.reapSession(sessionId) - } - return this.immediateKills.kill(sessionId, session, finish) + this.agentTeardowns.forceImmediate(sessionId, session) + return } session.kill() } diff --git a/src/main/daemon/terminal-immediate-kill.ts b/src/main/daemon/terminal-immediate-kill.ts deleted file mode 100644 index 52d1e2129f8..00000000000 --- a/src/main/daemon/terminal-immediate-kill.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { killWithDescendantSweep } from '../pty-descendant-termination' -import type { Session } from './session' - -/** Owns asynchronous agent teardown by session id so every duplicate caller - * observes the same completion, even if the root exits and is reaped first. */ -export class TerminalImmediateKillCoordinator { - private operations = new Map>() - - get(sessionId: string): Promise | undefined { - return this.operations.get(sessionId) - } - - kill(sessionId: string, session: Session, finish: () => void): void | Promise { - if (!session.launchAgent) { - finish() - return - } - if (!session.beginTermination()) { - return - } - - // Why: agent tool children live in detached process groups the shell's - // death never signals; the bounded snapshot defers teardown at most ~1s. - const operation = Promise.resolve(killWithDescendantSweep(session.pid, finish)) - this.operations.set(sessionId, operation) - const clearOperation = (): void => { - if (this.operations.get(sessionId) === operation) { - this.operations.delete(sessionId) - } - } - void operation.then(clearOperation, clearOperation) - return operation - } -} diff --git a/src/main/pty-descendant-termination.test.ts b/src/main/pty-descendant-termination.test.ts index d2e2733888c..f768bd4f37a 100644 --- a/src/main/pty-descendant-termination.test.ts +++ b/src/main/pty-descendant-termination.test.ts @@ -152,7 +152,11 @@ describe('captureDescendantSnapshot', () => { expect(execFileMock).toHaveBeenCalledWith( 'ps', ['-axo', 'pid=,ppid=,pgid=,lstart='], - expect.objectContaining({ timeout: 321, killSignal: 'SIGKILL' }), + expect.objectContaining({ + timeout: 321, + killSignal: 'SIGKILL', + env: expect.objectContaining({ LANG: 'C', LC_ALL: 'C' }) + }), expect.any(Function) ) }) diff --git a/src/main/pty-descendant-termination.ts b/src/main/pty-descendant-termination.ts index 6f8254cb9bd..366cbe9629c 100644 --- a/src/main/pty-descendant-termination.ts +++ b/src/main/pty-descendant-termination.ts @@ -64,7 +64,10 @@ function readFreshProcessTable( { maxBuffer: PS_MAX_BUFFER_BYTES, timeout: timeoutMs, - killSignal: 'SIGKILL' + killSignal: 'SIGKILL', + // Why: ps localizes lstart, but delayed identity checks must parse it + // identically for every user locale. + env: { ...process.env, LANG: 'C', LC_ALL: 'C' } }, (error, stdout) => { if (error) { diff --git a/src/main/runtime/worktree-teardown.test.ts b/src/main/runtime/worktree-teardown.test.ts index 63e207cd8da..698fb5e6858 100644 --- a/src/main/runtime/worktree-teardown.test.ts +++ b/src/main/runtime/worktree-teardown.test.ts @@ -146,6 +146,34 @@ describe('killAllProcessesForWorktree', () => { expect(r2.providerStopped).toBe(1) }) + it('starts owned provider shutdowns together so agent snapshots can coalesce', async () => { + const localProvider = createProviderStub(async () => [ + { id: 'w1@@aaaa', cwd: '/tmp', title: 'shell' }, + { id: 'w1@@bbbb', cwd: '/tmp', title: 'shell' } + ]) + listRegisteredPtysMock.mockReturnValue([]) + const releases: (() => void)[] = [] + ;(localProvider.shutdown as unknown as ReturnType).mockImplementation( + () => + new Promise((resolve) => { + releases.push(resolve) + }) + ) + + const teardown = killAllProcessesForWorktree('w1', { localProvider }) + await vi.waitFor(() => expect(localProvider.shutdown).toHaveBeenCalledTimes(2)) + expect(releases).toHaveLength(2) + + for (const release of releases) { + release() + } + await expect(teardown).resolves.toEqual({ + runtimeStopped: 0, + providerStopped: 2, + registryStopped: 0 + }) + }) + it('invokes runtime.stopTerminalsForWorktree when runtime is provided', async () => { const stopTerminalsForWorktree = vi.fn().mockResolvedValue({ stopped: 3 }) const runtime = { diff --git a/src/main/runtime/worktree-teardown.ts b/src/main/runtime/worktree-teardown.ts index 7572b9bbd77..fb105c22c72 100644 --- a/src/main/runtime/worktree-teardown.ts +++ b/src/main/runtime/worktree-teardown.ts @@ -72,21 +72,20 @@ async function sweepProviderByPrefix( ): Promise { const prefix = `${worktreeId}@@` const sessions = await provider.listProcesses().catch(() => []) - let killed = 0 - for (const s of sessions) { - if (!s.id.startsWith(prefix)) { - continue - } - try { - await provider.shutdown(s.id, { immediate: true }) - clearStoppedPtyState(s.id, onPtyStopped) - killed += 1 - } catch { - // Already dead, or the backend dropped the session — treat as success. - killed += 1 - } - } - return killed + const ownedSessions = sessions.filter((session) => session.id.startsWith(prefix)) + // Why: agent shutdown snapshots coalesce only when requests begin together; + // serial awaits multiply process-table scans and worktree-delete latency. + await Promise.all( + ownedSessions.map(async (session) => { + try { + await provider.shutdown(session.id, { immediate: true }) + clearStoppedPtyState(session.id, onPtyStopped) + } catch { + // Already dead, or the backend dropped the session — treat as success. + } + }) + ) + return ownedSessions.length } async function sweepRegistryForWorktree( @@ -95,17 +94,18 @@ async function sweepRegistryForWorktree( onPtyStopped?: (ptyId: string) => void ): Promise { const entries = listRegisteredPtys().filter((r) => r.worktreeId === worktreeId) - let killed = 0 - for (const entry of entries) { - try { - await localProvider.shutdown(entry.ptyId, { immediate: true }) - clearStoppedPtyState(entry.ptyId, onPtyStopped) - killed += 1 - } catch { - /* ignore — best-effort */ - } - } - return killed + const stopped = await Promise.all( + entries.map(async (entry) => { + try { + await localProvider.shutdown(entry.ptyId, { immediate: true }) + clearStoppedPtyState(entry.ptyId, onPtyStopped) + return 1 + } catch { + return 0 + } + }) + ) + return stopped.reduce((count, value) => count + value, 0) } function clearStoppedPtyState(ptyId: string, onPtyStopped?: (ptyId: string) => void): void {