From 9999e548dc5b7e5539d68258abb36f20bcfad806 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 17 Aug 2026 18:49:56 -0700 Subject: [PATCH] fix(agent-status): retire panes whose agent process is gone (STA-4612) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent status can hold `working` on a pane where no work is outstanding, and nothing closes the gap. A pane's Claude state is a join of a lead turn and three latches — the subagent roster, the background-task gate and the session-cron gate — and each is set by a hook and cleared only by another hook. Claude Code emits no terminating hook on `/exit`, `/clear`, Ctrl+C, crash, SIGKILL or terminal close, so every one of those latches is a claim with no owner and no expiry. The join is also materialised at ingest time and persisted, so a stale `working` survives restart and blocks hibernation, which requires `done`. Registering `SessionEnd` is not the fix: it covers roughly a third of exit paths (measured on 2.1.231/2.1.233; upstream anthropics/claude-code#17885 and #6428 are both closed as not planned). Nor is a TTL — `AGENT_STATUS_STALE_AFTER_MS` only decays the sidebar dot at read time while the stored row stays non-terminal. So the backstop is built from evidence Orca already owns. A session id that changes means the conversation was replaced. On the first hook of the new session — whatever that hook is — the previous session's own claims are void: its session crons and its one-shot subagents. Deliberately not voided: the background-task gate (a background shell is an OS process that survives `/clear`, and the previous inventory is positive evidence it was running), and `confirmedTeammate` rows (persistent in-process teammates a lead swap cannot end). The lead record is left to the incoming event's own fold. A certified process exit retires the pane. Orca already does this on every attributable PTY exit — `clearProviderPtyState` resolves the pane key and calls `clearPaneState` — but that resolution depends on the spawn-time `ptyPaneKey` mapping, which a restored or reattached PTY may never rebuild. Those panes keep their row and latches for good. `onPtyExit` knows the keys teardown could not resolve, so it reconciles them from its own records. The certificate is `exitCode >= 0 || hostExitConfirmed || providerExitObserved`: a synthetic `-1` from a failed stop is not a death (the PTY can have survived it), while a real exit can also report `-1`, so neither the code nor the SSH surface predicate is sufficient alone. `providerExitObserved` is additive and separate from `hostExitConfirmed`, which also drives the liveness verdict and the SSH surface decision. A confirmed shell foreground is the `/exit` case: the agent died, the shell lived. That already dropped the row, but through `agentStatus:drop`, which by its own contract preserves a live pane's caches — so every latch survived and the next event resolved the pane back to `working`. It now routes through the reconciler instead, gated on a per-pane accepted-status generation rather than row identity: the confirming process read can take seconds, and `updatedAt` cannot order two writes inside one millisecond (the store deliberately admits equal timestamps). Cold start generalises the same way. The startup sweep required a restored subagent roster, so a stranded lead row, background-task gate or cron gate — the shapes with no child event left to reap them — were never candidates. Hibernation needs no change: with the above, those rows become genuinely `done` and the lockout resolves through the front door. A `restoredUnconfirmed` bypass in the planner would let it reclaim the heap of an agent that may be working. Not included: folding `background_tasks` from a child-attributed `SubagentStop`. Writing its test surfaced #11838's deliberate assertion that child inventories are not authoritative for lead-owned background work, and the listener says the same — "background_tasks is trusted only where unambiguous". An empty list on a `SubagentStop` does not prove the lead's shell ended, so the fold would have cleared a gate on evidence that establishes nothing. STA-4119's live-side question — whether a genuinely live background shell should hold the lead row after the lead turn ends — is untouched. This change extends gate-clearing to zero new triggers. --- .../ended-process-reconciliation.test.ts | 112 +++++++++++ .../restored-subagent-liveness-sweep.test.ts | 40 +++- src/main/agent-hooks/server.ts | 52 ++++- src/main/index.ts | 3 + src/main/ipc/agent-hooks.ts | 43 +---- src/main/ipc/agent-status-row-teardown-ipc.ts | 64 +++++++ .../pty-daemon-ssh-lease-lifecycle.test.ts | 8 +- .../pty-listener-teardown-and-orphans.test.ts | 4 +- .../ipc/pty-runtime-kill-and-exit.test.ts | 8 +- src/main/ipc/pty.ts | 10 +- src/main/runtime/orca-runtime.ts | 59 ++++-- ...y-exit-agent-status-reconciliation.test.ts | 71 +++++++ src/preload/api/agent-status-api.ts | 3 + src/preload/index.ts | 3 + ...onnection-command-finished-cleanup.test.ts | 37 ++++ .../pty-connection-test-environment.ts | 3 +- .../terminal-pane/pty-connection.ts | 46 ++++- src/renderer/src/store/slices/agent-status.ts | 27 +++ src/renderer/src/web/web-preload-api.ts | 1 + ...-hook-listener-session-replacement.test.ts | 179 ++++++++++++++++++ src/shared/agent-hook-listener.ts | 82 ++++++++ 21 files changed, 788 insertions(+), 67 deletions(-) create mode 100644 src/main/agent-hooks/ended-process-reconciliation.test.ts create mode 100644 src/main/ipc/agent-status-row-teardown-ipc.ts create mode 100644 src/main/runtime/pty-exit-agent-status-reconciliation.test.ts create mode 100644 src/shared/agent-hook-listener-session-replacement.test.ts diff --git a/src/main/agent-hooks/ended-process-reconciliation.test.ts b/src/main/agent-hooks/ended-process-reconciliation.test.ts new file mode 100644 index 00000000000..54f65e4ed0e --- /dev/null +++ b/src/main/agent-hooks/ended-process-reconciliation.test.ts @@ -0,0 +1,112 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { makePaneKey } from '../../shared/stable-pane-id' +import { AgentHookServer } from './server' + +const LEAF = '11111111-1111-4111-8111-111111111111' +const PANE = makePaneKey('tab-1', LEAF) + +let dir: string + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'orca-ended-process-')) +}) + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }) +}) + +async function startServer(): Promise { + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath: dir }) + return server +} + +function claudeRow(server: AgentHookServer, state: 'working' | 'waiting' | 'done'): void { + server.ingestTerminalStatus({ + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + connectionId: null, + payload: { state, prompt: 'review the PR', agentType: 'claude' } + }) +} + +function paneState(server: AgentHookServer): string { + return server.getStatusSnapshotForPane(PANE)[0]?.state ?? 'missing' +} + +// The teardown that normally clears a dead pane depends on the spawn-time ptyPaneKey mapping, which +// a restored PTY may never rebuild — leaving the row and its latches with nothing left to retire +// them. See PLAN-STA-4612 §4.2. +describe('reconcileEndedProcessForPaneKeys', () => { + it('retires a working row whose process is certifiably dead', async () => { + const server = await startServer() + try { + claudeRow(server, 'working') + expect(paneState(server)).toBe('working') + + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(1) + + expect(paneState(server)).toBe('missing') + } finally { + server.stop() + } + }) + + it('retires a waiting row — a wait whose owner is dead is not a wait', async () => { + const server = await startServer() + try { + claudeRow(server, 'waiting') + + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(1) + + expect(paneState(server)).toBe('missing') + } finally { + server.stop() + } + }) + + it('clears Claude latches even when the stored row already reads done', async () => { + // An interrupted lead suppresses the gate while leaving the latch set, so a row can read `done` + // with a latch that would re-gate `working` on the pane's very next event. + const server = await startServer() + try { + claudeRow(server, 'done') + server._getStateForTests().claudeRunningNonAgentTaskPaneKeys.add(PANE) + server._getStateForTests().claudeActiveSessionCronPaneKeys.add(PANE) + + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(1) + + expect(server._getStateForTests().claudeRunningNonAgentTaskPaneKeys.has(PANE)).toBe(false) + expect(server._getStateForTests().claudeActiveSessionCronPaneKeys.has(PANE)).toBe(false) + } finally { + server.stop() + } + }) + + it('is a no-op for a pane with nothing to retire', async () => { + const server = await startServer() + try { + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(0) + } finally { + server.stop() + } + }) + + it('drops the session owner so a rebound pane is not read as a session replacement', async () => { + const server = await startServer() + try { + claudeRow(server, 'working') + server._getStateForTests().claudeSessionOwnerByPaneKey.set(PANE, 'session-a') + + server.reconcileEndedProcessForPaneKeys([PANE]) + + expect(server._getStateForTests().claudeSessionOwnerByPaneKey.has(PANE)).toBe(false) + } finally { + server.stop() + } + }) +}) diff --git a/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts b/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts index 9811773eb07..b3257e6972c 100644 --- a/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts +++ b/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts @@ -211,7 +211,10 @@ describe('restored subagent liveness sweep', () => { } }) - it('does not probe a hydrated Claude pane without restored rows', async () => { + it('reaps a hydrated non-terminal row with no roster at all (STA-4612)', async () => { + // A restored roster is only one shape of stranded claim. A lead row left `working` with no + // children has nothing left to retire it — no child event, no inventory, no hook — so it holds + // the pane working for good and locks its agent out of hibernation. const server = await restartWithInFlightSubagent({ subagents: [] }) const probeLiveLocalPty = vi.fn(() => false) try { @@ -220,9 +223,42 @@ describe('restored subagent liveness sweep', () => { probeLiveLocalPty, persistedPtyIdByPaneKey: { [PANE]: PTY } }) + ).toBe(1) + + expect(probeLiveLocalPty).toHaveBeenCalled() + expect(paneStatus(server).state).toBe('missing') + } finally { + server.stop() + } + }) + + it('keeps a hydrated non-terminal row with no roster when its PTY is still live', async () => { + const server = await restartWithInFlightSubagent({ subagents: [] }) + try { + expect( + await sweepWith(server, { + probeLiveLocalPty: () => true, + persistedPtyIdByPaneKey: { [PANE]: PTY } + }) ).toBe(0) - expect(probeLiveLocalPty).not.toHaveBeenCalled() + expect(paneStatus(server).state).toBe('working') + } finally { + server.stop() + } + }) + + it('keeps a hydrated non-terminal row with no roster when the probe cannot prove either state', async () => { + const server = await restartWithInFlightSubagent({ subagents: [] }) + try { + expect( + await sweepWith(server, { + probeLiveLocalPty: () => null, + persistedPtyIdByPaneKey: { [PANE]: PTY } + }) + ).toBe(0) + + expect(paneStatus(server).state).toBe('working') } finally { server.stop() } diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 7bb86a34142..79a17719cf2 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -2606,6 +2606,40 @@ export class AgentHookServer { this.notifyStatusChangeListeners() } + /** Retire panes whose owning process is certifiably dead. + * + * The ordinary teardown already does this: every attributable PTY exit reaches + * `clearProviderPtyState`, which resolves the pane key and calls `clearPaneState`. But that + * resolution depends on the spawn-time `ptyPaneKey` mapping, which a restored/reattached PTY may + * never rebuild — so those panes keep a `working` row and its latches for good, with no hook left + * to retire them. This is the same operation reached from the runtime's own pane-key knowledge, + * so a dead pane is cleaned up identically however its keys were resolved. */ + reconcileEndedProcessForPaneKeys(paneKeys: Iterable): number { + let cleared = 0 + for (const paneKey of paneKeys) { + const resolvedPaneKey = this.resolvePaneKeyAlias(paneKey) + if (!this.hasLiveClaimsForPaneKey(resolvedPaneKey)) { + continue + } + this.clearPaneState(resolvedPaneKey) + cleared += 1 + } + return cleared + } + + /** Anything a dead pane could still be asserting: a row, or a Claude latch that would re-gate one + * through `resolveClaudePaneState` on the pane's next event even after the row reads `done`. */ + private hasLiveClaimsForPaneKey(paneKey: string): boolean { + return ( + this.state.lastStatusByPaneKey.has(paneKey) || + this.state.claudeSubagentRosterByPaneKey.has(paneKey) || + this.state.claudeRunningNonAgentTaskPaneKeys.has(paneKey) || + this.state.claudeActiveSessionCronPaneKeys.has(paneKey) || + this.state.claudeLeadStateByPaneKey.has(paneKey) || + this.state.claudeSessionOwnerByPaneKey.has(paneKey) + ) + } + /** Clear statuses proven to belong to one lost SSH transport. */ clearStatusEntriesForConnection(connectionId: string): void { const normalizedConnectionId = connectionId.trim() @@ -2636,6 +2670,7 @@ export class AgentHookServer { this.state.claudeLeadStateByPaneKey.delete(paneKey) this.state.claudeRunningNonAgentTaskPaneKeys.delete(paneKey) this.state.claudeActiveSessionCronPaneKeys.delete(paneKey) + this.state.claudeSessionOwnerByPaneKey.delete(paneKey) } } } @@ -2823,9 +2858,15 @@ export class AgentHookServer { enriched.payload.agentType === 'claude' && enriched.connectionId === null && isLocalExecutionHost(enriched.worktreeId) && - claudeRosterHasRestoredSnapshotSubagent( + // Why: a restored roster is only one shape of stranded claim. A lead row left non-terminal, + // or a background-task/cron latch nothing will refresh, strands the pane just as + // permanently — and unlike the roster case there is no child event left to reap it. + (claudeRosterHasRestoredSnapshotSubagent( this.state.claudeSubagentRosterByPaneKey.get(paneKey) - ) && + ) || + enriched.payload.state !== 'done' || + this.state.claudeRunningNonAgentTaskPaneKeys.has(paneKey) || + this.state.claudeActiveSessionCronPaneKeys.has(paneKey)) && !this.runtimeObservedStatusPaneKeys.has(paneKey) ) { candidates.push({ paneKey, entry: enriched }) @@ -2853,6 +2894,13 @@ export class AgentHookServer { continue } if (!reapRestoredClaudeSubagentsForDeadPane(this.state, paneKey)) { + // Why: the roster reap only speaks for restored child rows. A pane whose PTY is provably + // gone and whose claim is a lead row or a latch has nothing for it to reap, so retire the + // pane the same way an observed exit would — otherwise the widened candidate set is inert. + if (this.hasLiveClaimsForPaneKey(paneKey)) { + this.clearPaneState(paneKey) + changedPanes += 1 + } continue } changedPanes += 1 diff --git a/src/main/index.ts b/src/main/index.ts index 94c06be69f1..18006c207db 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2624,6 +2624,9 @@ void app.whenReady().then(async () => { agentHookServer.attestCompatibilityAuthority(candidate), retireAgentHookCompatibilityAuthority: (paneKey) => agentHookServer.retirePaneAuthority(paneKey), + reconcileAgentStatusForEndedProcess: (paneKeys) => { + agentHookServer.reconcileEndedProcessForPaneKeys(paneKeys) + }, canRecoverPersistentLocalPtys: () => getDaemonProvider() !== null, // Why: source codex-home here (runs in window AND serve) so aiVault.listSessions includes managed-Codex sessions; registerCoreHandlers is window-only. getAdditionalAiVaultCodexHomePaths: () => diff --git a/src/main/ipc/agent-hooks.ts b/src/main/ipc/agent-hooks.ts index 5a8e3f69ae4..5615fa38208 100644 --- a/src/main/ipc/agent-hooks.ts +++ b/src/main/ipc/agent-hooks.ts @@ -6,13 +6,9 @@ import type { } from '../../shared/agent-status-types' import type { AgentInterruptInferenceRequest } from '../../shared/agent-interrupt-intent' import type { AgentQuestionAnsweredInferenceRequest } from '../../shared/agent-question-answered-intent' -import { agentHookServer, isValidPaneKey } from '../agent-hooks/server' +import { agentHookServer } from '../agent-hooks/server' import { ampHookService } from '../amp/hook-service' -import { - clearMigrationUnsupportedPtysByTabPrefix, - clearMigrationUnsupportedPtysForPaneKey, - getMigrationUnsupportedPtySnapshot -} from '../agent-hooks/migration-unsupported-pty-state' +import { getMigrationUnsupportedPtySnapshot } from '../agent-hooks/migration-unsupported-pty-state' import { claudeHookService } from '../claude/hook-service' import { codexHookService } from '../codex/hook-service' import { geminiHookService } from '../gemini/hook-service' @@ -27,10 +23,10 @@ import { devinHookService } from '../devin/hook-service' import { kimiHookService } from '../kimi/hook-service' import { openClaudeHookService } from '../openclaude/hook-service' import { registerAgentPaneAuthorityIpcHandlers } from './agent-pane-authority-ipc' +import { registerAgentStatusRowTeardownIpcHandlers } from './agent-status-row-teardown-ipc' import { createAgentPaneAuthorityOwnership } from './agent-pane-authority-ownership' import { enrichAgentStatusIpcPayload, - isValidAgentStatusDropTabId, type AgentStatusRuntimeEnrichment } from './agent-status-ipc-boundary' @@ -70,38 +66,7 @@ export function registerAgentHookHandlers( ipcMain.removeHandler('agentStatus:inferInterrupt') ipcMain.removeHandler('agentStatus:inferQuestionAnswered') ipcMain.removeHandler('agentStatus:getMigrationUnsupportedSnapshot') - // Why: agentStatus:drop is sent fire-and-forget from the renderer via - // ipcRenderer.send(); we listen with ipcMain.on (not handle) so we don't - // round-trip a response. Removing first keeps re-registration safe even - // though the module-level registered guard already prevents re-entry today. - ipcMain.removeAllListeners('agentStatus:drop') - ipcMain.removeAllListeners('agentStatus:dropByTabPrefix') - ipcMain.on('agentStatus:drop', (_event, paneKey: unknown) => { - if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) { - return - } - try { - // Why: dropStatusEntry (not clearPaneState) is correct here — the user is - // dismissing a status row, not tearing down a PTY. clearPaneState would also - // wipe the per-pane prompt/tool caches, which the next hook event for that - // (still-alive) pane needs to render a coherent row. - agentHookServer.dropStatusEntry(paneKey) - clearMigrationUnsupportedPtysForPaneKey(paneKey) - } catch (err) { - console.warn('[agent-hooks] dropStatusEntry failed:', err) - } - }) - ipcMain.on('agentStatus:dropByTabPrefix', (_event, tabId: unknown) => { - if (!isValidAgentStatusDropTabId(tabId)) { - return - } - try { - agentHookServer.dropStatusEntriesByTabPrefix(tabId) - clearMigrationUnsupportedPtysByTabPrefix(tabId) - } catch (err) { - console.warn('[agent-hooks] dropStatusEntriesByTabPrefix failed:', err) - } - }) + registerAgentStatusRowTeardownIpcHandlers() registerAgentPaneAuthorityIpcHandlers({ ownsPty: createAgentPaneAuthorityOwnership({ getPtyIdForPaneKey: dependencies.getPtyIdForPaneKey, diff --git a/src/main/ipc/agent-status-row-teardown-ipc.ts b/src/main/ipc/agent-status-row-teardown-ipc.ts new file mode 100644 index 00000000000..b95cf2ee1fd --- /dev/null +++ b/src/main/ipc/agent-status-row-teardown-ipc.ts @@ -0,0 +1,64 @@ +import { ipcMain } from 'electron' +import { agentHookServer, isValidPaneKey } from '../agent-hooks/server' +import { + clearMigrationUnsupportedPtysByTabPrefix, + clearMigrationUnsupportedPtysForPaneKey +} from '../agent-hooks/migration-unsupported-pty-state' +import { isValidAgentStatusDropTabId } from './agent-status-ipc-boundary' + +/** + * The three renderer-initiated ways a status row goes away. All fire-and-forget + * (`ipcRenderer.send` → `ipcMain.on`), so none round-trips a response; removing the + * listeners first keeps re-registration safe. + * + * They are NOT interchangeable. A dismissal keeps the pane's per-pane caches because its agent may + * still be alive; a confirmed process exit must take them too, or a surviving Claude latch resolves + * the pane's next event straight back to `working`. + */ +export function registerAgentStatusRowTeardownIpcHandlers(): void { + ipcMain.removeAllListeners('agentStatus:drop') + ipcMain.removeAllListeners('agentStatus:reconcileEndedProcess') + ipcMain.removeAllListeners('agentStatus:dropByTabPrefix') + + ipcMain.on('agentStatus:drop', (_event, paneKey: unknown) => { + if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) { + return + } + try { + // Why: dropStatusEntry (not clearPaneState) is correct here — the user is + // dismissing a status row, not tearing down a PTY. clearPaneState would also + // wipe the per-pane prompt/tool caches, which the next hook event for that + // (still-alive) pane needs to render a coherent row. + agentHookServer.dropStatusEntry(paneKey) + clearMigrationUnsupportedPtysForPaneKey(paneKey) + } catch (err) { + console.warn('[agent-hooks] dropStatusEntry failed:', err) + } + }) + + ipcMain.on('agentStatus:reconcileEndedProcess', (_event, paneKey: unknown) => { + if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) { + return + } + try { + // Why: a process-table-confirmed agent exit is exactly the case the dismissal above excludes + // — the pane's agent is NOT still alive — so its latches must go with the row (STA-4612). + agentHookServer.reconcileEndedProcessForPaneKeys([paneKey]) + clearMigrationUnsupportedPtysForPaneKey(paneKey) + } catch (err) { + console.warn('[agent-hooks] reconcileEndedProcessForPaneKeys failed:', err) + } + }) + + ipcMain.on('agentStatus:dropByTabPrefix', (_event, tabId: unknown) => { + if (!isValidAgentStatusDropTabId(tabId)) { + return + } + try { + agentHookServer.dropStatusEntriesByTabPrefix(tabId) + clearMigrationUnsupportedPtysByTabPrefix(tabId) + } catch (err) { + console.warn('[agent-hooks] dropStatusEntriesByTabPrefix failed:', err) + } + }) +} diff --git a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts index d2e07b8458c..c99c7f7b93d 100644 --- a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts +++ b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts @@ -289,7 +289,9 @@ describe('registerPtyHandlers', () => { await Promise.resolve() expect(runtime.onPtyExit).toHaveBeenCalledTimes(1) - expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, { + providerExitObserved: true + }) expect( mainWindow.webContents.send.mock.calls.filter((call) => call[0] === 'pty:exit') ).toEqual([['pty:exit', { id: 'local-pty', code: 0 }]]) @@ -342,7 +344,9 @@ describe('registerPtyHandlers', () => { await expect(stopPromise).resolves.toBe(true) expect(runtime.onPtyExit).toHaveBeenCalledTimes(1) - expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, { + providerExitObserved: true + }) expect( mainWindow.webContents.send.mock.calls.filter((call) => call[0] === 'pty:exit') ).toEqual([['pty:exit', { id: 'local-pty', code: 0 }]]) diff --git a/src/main/ipc/pty-listener-teardown-and-orphans.test.ts b/src/main/ipc/pty-listener-teardown-and-orphans.test.ts index d148ebe3cff..f50a5be5012 100644 --- a/src/main/ipc/pty-listener-teardown-and-orphans.test.ts +++ b/src/main/ipc/pty-listener-teardown-and-orphans.test.ts @@ -436,7 +436,9 @@ describe('registerPtyHandlers', () => { await Promise.resolve() expect(killSpy).toHaveBeenCalled() - expect(runtime.onPtyExit).toHaveBeenCalledWith(spawnResult.id, -1, spawnResult.incarnationId) + expect(runtime.onPtyExit).toHaveBeenCalledWith(spawnResult.id, -1, spawnResult.incarnationId, { + providerExitObserved: true + }) const listed = await getLocalPtyProvider().listProcesses() expect(listed.some((info) => info.id === spawnResult.id)).toBe(false) }) diff --git a/src/main/ipc/pty-runtime-kill-and-exit.test.ts b/src/main/ipc/pty-runtime-kill-and-exit.test.ts index 2da8b80d4b8..dccd1a95318 100644 --- a/src/main/ipc/pty-runtime-kill-and-exit.test.ts +++ b/src/main/ipc/pty-runtime-kill-and-exit.test.ts @@ -284,7 +284,9 @@ describe('registerPtyHandlers', () => { await handlers.get('pty:kill')!(null, { id: 'local-pty' }) expect(runtime.onPtyExit).toHaveBeenCalledTimes(1) - expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, { + providerExitObserved: true + }) expect(mainWindow.webContents.send.mock.calls.filter((call) => call[0] === 'pty:exit')).toEqual( [['pty:exit', { id: 'local-pty', code: 0 }]] ) @@ -526,7 +528,9 @@ describe('registerPtyHandlers', () => { seq: 13, rawLength: 'daemon output'.length }) - expect(runtime.onPtyExit).toHaveBeenCalledWith(result.id, 0, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith(result.id, 0, undefined, { + providerExitObserved: true + }) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:exit', { id: result.id, code: 0 diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 0414cd514fa..a917ca588ba 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -2599,7 +2599,9 @@ export function registerPtyHandlers( clearProviderPtyState(id) ptyOwnership.delete(id) markClaudePtyExited(id) - runtime?.onPtyExit(id, code, incarnationId) + // Why: the provider's physical exit callback is a death certificate even when node-pty + // reports the code as -1; only the synthesized failed/unverified stop calls omit this. + runtime?.onPtyExit(id, code, incarnationId, { providerExitObserved: true }) }, onData: (id, data, timestamp, sequenceChars, transformed) => runtime?.onPtyData(id, data, timestamp, sequenceChars ?? data.length, transformed) @@ -4144,7 +4146,11 @@ export function registerPtyHandlers( clearProviderPtyState(payload.id) ptyOwnership.delete(payload.id) markClaudePtyExited(payload.id) - runtime?.onPtyExit(payload.id, payload.code, payload.incarnationId) + // Why: same physical-exit certificate as the LocalPtyProvider branch above — this is the + // daemon/WSL callback, and on macOS it is the branch that actually runs. + runtime?.onPtyExit(payload.id, payload.code, payload.incarnationId, { + providerExitObserved: true + }) } sendPtyExitToRenderer(payload) }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index a9e16ad00ff..ecce7b9f462 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -3594,6 +3594,9 @@ export class OrcaRuntimeService { }) => AgentHookAuthorityAttestation | null) | null private readonly retireAgentHookCompatibilityAuthorityFn: ((paneKey: string) => void) | null + private readonly reconcileAgentStatusForEndedProcessFn: + | ((paneKeys: Iterable) => void) + | null private readonly canRecoverPersistentLocalPtysFn: () => boolean private readonly buildAgentHookPtyEnv: (() => Record) | null private readonly getDesktopWindowStatusFn: () => RuntimeDesktopWindowStatus @@ -3674,6 +3677,7 @@ export class OrcaRuntimeService { terminalProvenance: 'current_runtime' | 'restored' }) => AgentHookAuthorityAttestation | null retireAgentHookCompatibilityAuthority?: (paneKey: string) => void + reconcileAgentStatusForEndedProcess?: (paneKeys: Iterable) => void canRecoverPersistentLocalPtys?: () => boolean // Why: codex-home paths for the Agent Session History scan must be sourced // here, not via the window-only registerCoreHandlers path — that path never @@ -3717,6 +3721,7 @@ export class OrcaRuntimeService { deps?.attestAgentHookCompatibilityAuthority ?? null this.retireAgentHookCompatibilityAuthorityFn = deps?.retireAgentHookCompatibilityAuthority ?? null + this.reconcileAgentStatusForEndedProcessFn = deps?.reconcileAgentStatusForEndedProcess ?? null this.canRecoverPersistentLocalPtysFn = deps?.canRecoverPersistentLocalPtys ?? (() => true) // Why: configure the shared AiVault scan cache from a serve-mode-reachable // seam so the aiVault.listSessions RPC includes managed-Codex + WSL sessions @@ -13781,6 +13786,27 @@ export class OrcaRuntimeService { } } + /** Every pane key this PTY could be addressed by. Independent of launch authority: an ordinary + * restored PTY has neither a launch token nor a receipt, and those are exactly the panes whose + * spawn-time `ptyPaneKey` mapping teardown could not resolve. */ + private collectPaneKeysForPty(ptyId: string): Set { + const paneKeys = new Set() + const pty = this.ptysById.get(ptyId) + if (pty?.paneKey && parsePaneKey(pty.paneKey)) { + paneKeys.add(pty.paneKey) + } + const receipt = this.restoredOrchestrationAuthorityByPtyId.get(ptyId) + if (receipt?.paneKey && parsePaneKey(receipt.paneKey)) { + paneKeys.add(receipt.paneKey) + } + for (const leaf of this.getLeavesForPty(ptyId)) { + if (isValidTerminalTabId(leaf.tabId) && isTerminalLeafId(leaf.leafId)) { + paneKeys.add(makePaneKey(leaf.tabId, leaf.leafId)) + } + } + return paneKeys + } + private retirePtyAgentLaunchAuthority(ptyId: string): void { const pty = this.ptysById.get(ptyId) if (!pty) { @@ -13790,21 +13816,10 @@ export class OrcaRuntimeService { if (!pty.launchToken && !receipt) { return } + const paneKeys = this.collectPaneKeysForPty(ptyId) this.restoredOrchestrationAuthorityByPtyId.delete(ptyId) pty.launchToken = null pty.launchIncarnationId = null - const paneKeys = new Set() - if (pty.paneKey && parsePaneKey(pty.paneKey)) { - paneKeys.add(pty.paneKey) - } - if (receipt?.paneKey && parsePaneKey(receipt.paneKey)) { - paneKeys.add(receipt.paneKey) - } - for (const leaf of this.getLeavesForPty(ptyId)) { - if (isValidTerminalTabId(leaf.tabId) && isTerminalLeafId(leaf.leafId)) { - paneKeys.add(makePaneKey(leaf.tabId, leaf.leafId)) - } - } for (const paneKey of paneKeys) { this.retireAgentHookCompatibilityAuthorityFn?.(paneKey) } @@ -15024,7 +15039,13 @@ export class OrcaRuntimeService { ptyId: string, exitCode: number, exitIncarnationId?: PtyIncarnationId, - options?: { hostExitConfirmed?: boolean } + options?: { + hostExitConfirmed?: boolean + /** The provider's own physical-exit callback fired. Separate from `hostExitConfirmed`, which + * also drives the SSH surface decision and the liveness verdict: node-pty reports real exits + * as -1, so the numeric code alone cannot tell a dead process from a failed stop. */ + providerExitObserved?: boolean + } ): void { const pty = this.ptysById.get(ptyId) if (exitIncarnationId && pty?.incarnationId && exitIncarnationId !== pty.incarnationId) { @@ -15035,6 +15056,9 @@ export class OrcaRuntimeService { pty?.connectionId != null && exitCode < 0 && options?.hostExitConfirmed !== true + // Why: collect before retirePtyAgentLaunchAuthority, which deletes the restored-authority + // receipt a receipt-only pane's key comes from. + const exitPaneKeys = this.collectPaneKeysForPty(ptyId) if (preservesAbnormalSshSurface) { if (this.getPtyLivenessVerdict(ptyId)?.status !== 'unverifiable') { this.markPtyLivenessUnverifiable(ptyId, SSH_EXIT_UNCONFIRMED_REASON) @@ -15043,6 +15067,15 @@ export class OrcaRuntimeService { } else { this.retirePtyAgentLaunchAuthority(ptyId) } + // Why: a synthetic -1 from a failed or unverified stop is not a death certificate (the PTY can + // have survived it), while a real exit can also report -1 — so neither the numeric code nor the + // SSH surface predicate is sufficient on its own. Decided separately from + // preservesAbnormalSshSurface, which a host-confirmed negative SSH exit would otherwise skip. + const processDeathCertified = + exitCode >= 0 || options?.hostExitConfirmed === true || options?.providerExitObserved === true + if (processDeathCertified && exitPaneKeys.size > 0) { + this.reconcileAgentStatusForEndedProcessFn?.(exitPaneKeys) + } const incarnationId = exitIncarnationId ?? pty?.incarnationId ?? diff --git a/src/main/runtime/pty-exit-agent-status-reconciliation.test.ts b/src/main/runtime/pty-exit-agent-status-reconciliation.test.ts new file mode 100644 index 00000000000..2067ecc46e4 --- /dev/null +++ b/src/main/runtime/pty-exit-agent-status-reconciliation.test.ts @@ -0,0 +1,71 @@ +/** + * A PTY that exits without a resolvable spawn-time pane key never reaches + * `agentHookServer.clearPaneState`, so its row and every Claude latch outlive the process with + * nothing left to retire them (STA-4612). `onPtyExit` knows the pane keys that teardown could not + * resolve, so it reconciles them — but only on a real death certificate. + */ +import { describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' +import { makePaneKey } from '../../shared/stable-pane-id' + +const LEAF = '11111111-1111-4111-8111-111111111111' +const PANE = makePaneKey('tab-1', LEAF) +const PTY = 'wt-1__pty-1' + +type RuntimeInternals = { + ptysById: Map +} + +function runtimeWithBoundPane( + reconcile: (paneKeys: Iterable) => void, + options: { connectionId?: string } = {} +): OrcaRuntimeService { + const runtime = new OrcaRuntimeService(null, undefined, { + reconcileAgentStatusForEndedProcess: reconcile + }) + const ptys = (runtime as unknown as RuntimeInternals).ptysById + ptys.set(PTY, { + ptyId: PTY, + paneKey: PANE, + worktreeId: 'wt-1', + tabId: 'tab-1', + ...(options.connectionId ? { connectionId: options.connectionId } : {}) + } as never) + return runtime +} + +function reconciledPaneKeys(calls: Iterable[]): string[] { + return calls.flatMap((keys) => [...keys]) +} + +describe('onPtyExit agent-status reconciliation', () => { + it('reconciles on a normal zero exit code', () => { + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, 0) + + expect(reconciledPaneKeys(reconcile.mock.calls.map(([keys]) => keys))).toContain(PANE) + }) + + it('reconciles a physical -1 exit reported by the provider callback', () => { + // node-pty forwards real exits as -1 (local-pty-provider-shutdown.test.ts pins this), so the + // numeric code alone cannot separate a dead process from a failed stop. + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, -1, undefined, { providerExitObserved: true }) + + expect(reconciledPaneKeys(reconcile.mock.calls.map(([keys]) => keys))).toContain(PANE) + }) + + it('reconciles a host-confirmed negative exit', () => { + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, -1, undefined, { hostExitConfirmed: true }) + + expect(reconciledPaneKeys(reconcile.mock.calls.map(([keys]) => keys))).toContain(PANE) + }) + + it('does NOT reconcile a synthetic -1 from a failed stop, whose PTY may have survived', () => { + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, -1) + + expect(reconcile).not.toHaveBeenCalled() + }) +}) diff --git a/src/preload/api/agent-status-api.ts b/src/preload/api/agent-status-api.ts index 900b3ea4490..fd6da88e351 100644 --- a/src/preload/api/agent-status-api.ts +++ b/src/preload/api/agent-status-api.ts @@ -30,6 +30,9 @@ export type AgentStatusApi = { getMigrationUnsupportedSnapshot: () => Promise /** Drop a paneKey from the main-process hook cache and on-disk last-status file. Fire-and-forget. */ drop: (paneKey: string) => void + /** Retire a pane whose agent process is proven gone — clears the row AND the per-pane caches a + * dismissal deliberately keeps. Not `drop`: that one is a user dismissal of a live pane's row. */ + reconcileEndedProcess: (paneKey: string) => void /** Drop every cached hook status under one terminal tab prefix. Fire-and-forget. */ dropByTabPrefix: (tabId: string) => void /** Permanently retire one pane's hook authority while siblings stay live. */ diff --git a/src/preload/index.ts b/src/preload/index.ts index f851ce726b7..77286f129ec 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -5025,6 +5025,9 @@ const api = { drop: (paneKey: string): void => { ipcRenderer.send('agentStatus:drop', paneKey) }, + reconcileEndedProcess: (paneKey: string): void => { + ipcRenderer.send('agentStatus:reconcileEndedProcess', paneKey) + }, /** Drop all cached hook statuses under one terminal tab prefix; fired on explicit tab close even without a local row. */ dropByTabPrefix: (tabId: string): void => { ipcRenderer.send('agentStatus:dropByTabPrefix', tabId) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts index dc3a6c4e2d5..58ab015b868 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts @@ -328,6 +328,43 @@ describe('connectPanePty', () => { shellForeground: true }) expect(resolveMockPaneWindowsShiftEnterEncoding(mockStoreState, paneKey)).toBe('alt-enter') + // Why: `agentStatus:drop` deliberately preserves a live pane's per-pane caches, so a + // process-confirmed agent exit must go through the reconcile route instead — otherwise a + // surviving Claude latch resolves the pane's next event straight back to 'working' (STA-4612). + expect(window.api.agentStatus.reconcileEndedProcess).toHaveBeenCalledWith(paneKey) + }) + + it('does NOT reconcile when the process check could not confirm a shell', async () => { + vi.useFakeTimers() + const { connectPanePty } = await import('./pty-connection') + vi.mocked(window.api.pty.confirmForegroundProcess).mockResolvedValue(null) + vi.mocked(window.api.agentStatus.reconcileEndedProcess).mockClear() + const dataCallbackRef: { current: ((data: string) => void) | null } = { current: null } + const ptyId = 'pty-unconfirmed-shell' + const transport = createMockTransport(ptyId) + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + dataCallbackRef.current = callbacks.onData ?? null + return { id: ptyId } + }) + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ isVisibleRef: { current: false } }) as never + ) + await vi.advanceTimersByTimeAsync(20) + await flushAsyncTicks() + mockStoreState.agentLaunchConfigByPaneKey[paneKey] = { + launchConfig: { agentArgs: '', agentEnv: {} }, + identity: { agentType: 'droid' } + } + + dataCallbackRef.current?.('\x1b]133;D;0\x07') + await vi.advanceTimersByTimeAsync(350) + + expect(window.api.agentStatus.reconcileEndedProcess).not.toHaveBeenCalled() }) it('disarms stale TUI modes in the emulator after a confirmed return to shell', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts b/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts index 6e9f24f3efe..eb3a40b6803 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts @@ -72,7 +72,8 @@ export function installTerminalTestGlobals(): void { restoreTerminalFit: vi.fn().mockResolvedValue({ restored: true }) }, agentStatus: { - inferInterrupt: vi.fn().mockResolvedValue(false) + inferInterrupt: vi.fn().mockResolvedValue(false), + reconcileEndedProcess: vi.fn() } }, dispatchEvent: vi.fn(), diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 5bebf788484..51028806477 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -6,6 +6,7 @@ import { resolveCursorAgentImeAnchor } from '@/lib/pane-manager/terminal-ime-anc import { installTerminalImeCompositionRoute } from './terminal-ime-composition-route' import { detectAgentStatusFromTitle, agentTypeToIconAgent, isClaudeAgent } from '@/lib/agent-status' import { reportWorkerTerminalUserInput } from '@/lib/worker-terminal-takeover-report' +import { getAcceptedAgentStatusGeneration } from '@/store/slices/agent-status' import { resolvePaneTitleDecision } from './terminal-title-evidence' import { blocksCodexPaneInput } from '../codex-restart-notice-state' import { resolveLiveAgentStatusConnectionRouting } from '@/lib/agent-status-connection-ownership' @@ -1930,6 +1931,25 @@ export function connectPanePty( return false }) }) + /** The pane's foreground was proven to be a shell, so its agent exited. Unlike a user dismissal, + * this must also retire the main-side per-pane caches — a surviving Claude latch resolves the + * next event straight back to `working`. + * + * Ordered by the pane's accepted-status generation rather than by row identity: the confirming + * process read can take seconds, and an unrelated field moving in that window is not evidence the + * agent is alive — while a genuinely newer row can share the anchor's millisecond, so a timestamp + * cannot separate the two. A relaunch inside the window is already handled by onCommandStarted, + * which discards the armed drop. */ + const reconcileEndedProcessIfPaneQuiet = (armedGeneration: number): void => { + if (getAcceptedAgentStatusGeneration(cacheKey) !== armedGeneration) { + return + } + // Why: main-side only. The renderer row and launch config are already owned by the deferred + // drop above; what that path cannot reach is the hook server's per-pane Claude latches, which + // `agentStatus:drop` deliberately preserves for a still-live pane. Main echoes its own clear + // back through the pane-status-cleared channel, so both sides stay consistent. + window.api?.agentStatus?.reconcileEndedProcess?.(cacheKey) + } const dropCommandFinishedStatusIfSameTurn = ( entry: AgentStatusEntry | undefined, options?: { allowInferredInterrupt?: boolean } @@ -2107,12 +2127,23 @@ export function connectPanePty( } } let deferredCommandFinishedStatusDrop: (() => void) | null = null + /** Armed alongside the deferred drop; runs only on a CONFIRMED shell foreground, where the + * process table proved the agent gone. The unavailable-inspection path deliberately does not + * reconcile — no proof, no teardown. */ + let deferredConfirmedShellReconcile: (() => void) | null = null let visibleForegroundSamplePending = false let visibleForegroundSampleSettled = false - const settleDeferredCommandFinishedStatusDrop = (): void => { + const settleDeferredCommandFinishedStatusDrop = ( + options: { confirmedShell?: boolean } = {} + ): void => { const dropStatus = deferredCommandFinishedStatusDrop + const reconcile = deferredConfirmedShellReconcile deferredCommandFinishedStatusDrop = null + deferredConfirmedShellReconcile = null dropStatus?.() + if (options.confirmedShell) { + reconcile?.() + } } const isForegroundTrackingAllowed = (id: string): boolean => { if (isRemoteRuntimePtyId(id) || parseAppSshPtyId(id) !== null) { @@ -2159,9 +2190,11 @@ export function connectPanePty( useAppStore.getState().clearAgentLaunchConfig(cacheKey) return } - settleDeferredCommandFinishedStatusDrop() + settleDeferredCommandFinishedStatusDrop({ confirmedShell: true }) }, - onCommandFinishedUnavailable: settleDeferredCommandFinishedStatusDrop, + // Why: inspection that could not answer is not shell proof — settle the drop the pane already + // armed, but never tear the pane's caches down on it. + onCommandFinishedUnavailable: () => settleDeferredCommandFinishedStatusDrop(), onVisibleForegroundSettled: (outcome) => { visibleForegroundSamplePending = false visibleForegroundSampleSettled = outcome !== 'inconclusive' @@ -2199,9 +2232,14 @@ export function connectPanePty( if (shouldDeferStatusDrop) { // Why: keep the concrete pane identity routable while the local process // check distinguishes a leaked nested-shell D from a genuine agent exit. + // Anchor the generation once, here — not per rung of the confirm ladder, which can span + // seconds — so the gate tolerates churn across the whole window. + const armedGeneration = getAcceptedAgentStatusGeneration(cacheKey) deferredCommandFinishedStatusDrop = dropStatus + deferredConfirmedShellReconcile = () => reconcileEndedProcessIfPaneQuiet(armedGeneration) return } + deferredConfirmedShellReconcile = null deferredCommandFinishedStatusDrop = null dropStatus() } @@ -2266,6 +2304,7 @@ export function connectPanePty( // Why: a new command invalidates cleanup waiting on the previous D; only // a later confirmed shell boundary may retire this pane's live identity. deferredCommandFinishedStatusDrop = null + deferredConfirmedShellReconcile = null visibleForegroundSamplePending = false visibleForegroundSampleSettled = false // Why: typed commands can be aliases, so they only widen the bounded @@ -9551,6 +9590,7 @@ export function connectPanePty( } commandLifecycle.dispose() deferredCommandFinishedStatusDrop = null + deferredConfirmedShellReconcile = null visibleForegroundSamplePending = false visibleForegroundSampleSettled = false paneForegroundAgentTracker.dispose() diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index e3b27077bf4..c6353341b56 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -1290,6 +1290,28 @@ function buildAgentStatusBatchPatch( return patch as Partial } +/** Per-pane counter of ACCEPTED status writes. Deliberately module-local, not store state: nothing + * renders from it, and it exists only so a deferred drop can ask "did this pane report again while + * I was confirming?" — a question `updatedAt` cannot answer, since the accept rule admits equal + * timestamps and a real event can share the anchor's millisecond. */ +const acceptedAgentStatusGenerationByPaneKey = new Map() + +function bumpAcceptedAgentStatusGeneration(paneKey: string): void { + acceptedAgentStatusGenerationByPaneKey.set( + paneKey, + (acceptedAgentStatusGenerationByPaneKey.get(paneKey) ?? 0) + 1 + ) +} + +/** Reads 0 for a pane that has never reported, so an armed drop compares equal rather than undefined. */ +export function getAcceptedAgentStatusGeneration(paneKey: string): number { + return acceptedAgentStatusGenerationByPaneKey.get(paneKey) ?? 0 +} + +export function forgetAcceptedAgentStatusGeneration(paneKey: string): void { + acceptedAgentStatusGenerationByPaneKey.delete(paneKey) +} + export const createAgentStatusSlice: StateCreator = ( storeSet, storeGet @@ -1974,6 +1996,10 @@ export const createAgentStatusSlice: StateCreator { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 57f5efacef3..ea655de8da8 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -940,6 +940,7 @@ function createWebPreloadApi(): Partial { onLegacyWorkerTerminalRecovery: () => noopUnsubscribe, getMigrationUnsupportedSnapshot: () => Promise.resolve([]), drop: () => {}, + reconcileEndedProcess: () => {}, dropByTabPrefix: () => {}, retirePaneAuthority: () => {}, restorePaneAuthority: () => {}, diff --git a/src/shared/agent-hook-listener-session-replacement.test.ts b/src/shared/agent-hook-listener-session-replacement.test.ts new file mode 100644 index 00000000000..1409fff8803 --- /dev/null +++ b/src/shared/agent-hook-listener-session-replacement.test.ts @@ -0,0 +1,179 @@ +import { describe, expect, it } from 'vitest' +import { + createHookListenerState, + normalizeHookPayload, + type HookListenerState +} from './agent-hook-listener' +import { makePaneKey } from './stable-pane-id' + +const LEAF_ID = '44444444-4444-4444-8444-444444444444' +const SESSION_A = 'session-a' +const SESSION_B = 'session-b' + +function claudeEvent( + state: HookListenerState, + paneKey: string, + payload: Record +): ReturnType { + return normalizeHookPayload(state, 'claude', { paneKey, payload }, 'production') +} + +/** Close a turn for `sessionId`, carrying whatever inventory the caller wants folded. */ +function stop( + state: HookListenerState, + paneKey: string, + sessionId: string, + extra: Record = {} +): ReturnType { + return claudeEvent(state, paneKey, { + hook_event_name: 'Stop', + session_id: sessionId, + ...extra + }) +} + +// A replacement session is the only evidence Orca gets for the exits Claude emits no terminating +// hook for (/clear, relaunch, resume). See PLAN-STA-4612 §4.1. +describe('Claude session replacement voids the replaced session claims', () => { + it('voids a session cron gate held by the previous session', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('cron-gate', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + + const replaced = stop(state, paneKey, SESSION_B) + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(false) + expect(replaced?.payload.state).toBe('done') + }) + + it('voids a one-shot subagent the previous session was still running', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('roster-void', LEAF_ID) + + claudeEvent(state, paneKey, { + hook_event_name: 'UserPromptSubmit', + session_id: SESSION_A, + prompt: 'go' + }) + claudeEvent(state, paneKey, { + hook_event_name: 'SubagentStart', + session_id: SESSION_A, + agent_id: 'achild-0000000000000001' + }) + expect(state.claudeSubagentRosterByPaneKey.get(paneKey)?.size).toBe(1) + + const replaced = stop(state, paneKey, SESSION_B) + + expect(state.claudeSubagentRosterByPaneKey.has(paneKey)).toBe(false) + expect(replaced?.payload.state).toBe('done') + expect(replaced?.payload.subagents ?? []).toEqual([]) + }) + + it('keeps a confirmed teammate across the replacement — a lead swap cannot end an in-process teammate', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('teammate-kept', LEAF_ID) + + claudeEvent(state, paneKey, { + hook_event_name: 'SubagentStart', + session_id: SESSION_A, + agent_id: 'arev-0000000000000001', + agent_type: 'rev' + }) + claudeEvent(state, paneKey, { + hook_event_name: 'TeammateIdle', + session_id: SESSION_A, + teammate_name: 'rev' + }) + claudeEvent(state, paneKey, { + hook_event_name: 'SubagentStart', + session_id: SESSION_A, + agent_id: 'arev-0000000000000001', + agent_type: 'rev' + }) + const tracked = state.claudeSubagentRosterByPaneKey.get(paneKey)?.get('arev-0000000000000001') + expect(tracked?.confirmedTeammate).toBe(true) + expect(tracked?.state).toBe('working') + + stop(state, paneKey, SESSION_B) + + expect( + state.claudeSubagentRosterByPaneKey.get(paneKey)?.get('arev-0000000000000001')?.state + ).toBe('working') + }) + + it('KEEPS a running background-shell gate across the replacement', () => { + // /clear keeps the Claude process alive, so a background shell can outlive the conversation and + // the previous inventory is positive evidence it was running. Retiring it here would decide the + // open live-shell product question inside this change. See PLAN-STA-4612 §6.3. + const state = createHookListenerState() + const paneKey = makePaneKey('bg-gate-kept', LEAF_ID) + + stop(state, paneKey, SESSION_A, { + background_tasks: [{ type: 'bash', status: 'running', id: 'bash_1' }] + }) + expect(state.claudeRunningNonAgentTaskPaneKeys.has(paneKey)).toBe(true) + + const replaced = claudeEvent(state, paneKey, { + hook_event_name: 'PostToolUse', + session_id: SESSION_B + }) + + expect(state.claudeRunningNonAgentTaskPaneKeys.has(paneKey)).toBe(true) + expect(replaced?.payload.state).toBe('working') + }) + + it('voids nothing when the session id is unchanged', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('same-session', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + const same = stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + expect(same?.payload.state).toBe('working') + }) + + it('voids nothing when a CHILD event carries a foreign session id', () => { + // Claude has no equivalent of the Codex child-session guard, so a child payload's own + // session_id reaches the same extraction path as the lead's. + const state = createHookListenerState() + const paneKey = makePaneKey('child-session', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + claudeEvent(state, paneKey, { + hook_event_name: 'PostToolUse', + session_id: 'child-session', + agent_id: 'achild-0000000000000009' + }) + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + expect(state.claudeSessionOwnerByPaneKey.get(paneKey)).toBe(SESSION_A) + }) + + it('voids nothing when the event carries no session id', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('no-session', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + stop(state, paneKey, '') + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + expect(state.claudeSessionOwnerByPaneKey.get(paneKey)).toBe(SESSION_A) + }) + + it('leaves the lead-state record to the incoming fold rather than deleting it', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('lead-untouched', LEAF_ID) + + stop(state, paneKey, SESSION_A) + claudeEvent(state, paneKey, { + hook_event_name: 'UserPromptSubmit', + session_id: SESSION_B, + prompt: 'new conversation' + }) + + expect(state.claudeLeadStateByPaneKey.get(paneKey)?.state).toBe('working') + }) +}) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 933e3de10d8..b527e46e571 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -142,6 +142,10 @@ export type HookListenerState = { claudeRunningNonAgentTaskPaneKeys: Set /** Panes whose latest authoritative Claude cron inventory still has a scheduled job. */ claudeActiveSessionCronPaneKeys: Set + /** Claude `session_id` that last reported on the pane from a LEAD event. A different id means the + * conversation was replaced (/clear, relaunch, resume), so claims the old session owned are void + * even when no SessionStart arrives — the backstop for the exits that emit no terminating hook. */ + claudeSessionOwnerByPaneKey: Map /** Live thread-spawn children per Codex pane. */ codexSubagentRosterByPaneKey: Map /** Incremental parent/child rollout cursors for Codex collaboration v2. */ @@ -182,6 +186,7 @@ export function createHookListenerState(): HookListenerState { claudeUnconfirmedRestoredStatusPaneKeys: new Set(), claudeRunningNonAgentTaskPaneKeys: new Set(), claudeActiveSessionCronPaneKeys: new Set(), + claudeSessionOwnerByPaneKey: new Map(), codexSubagentRosterByPaneKey: new Map(), codexSubagentTranscriptByPaneKey: new Map(), codexLeadStateByPaneKey: new Map() @@ -199,6 +204,7 @@ export function clearPaneCacheState(state: HookListenerState, paneKey: string): state.claudeUnconfirmedRestoredStatusPaneKeys.delete(paneKey) state.claudeRunningNonAgentTaskPaneKeys.delete(paneKey) state.claudeActiveSessionCronPaneKeys.delete(paneKey) + state.claudeSessionOwnerByPaneKey.delete(paneKey) state.codexSubagentRosterByPaneKey.delete(paneKey) state.codexSubagentTranscriptByPaneKey.delete(paneKey) state.codexLeadStateByPaneKey.delete(paneKey) @@ -246,6 +252,7 @@ export function movePaneCacheState( movePaneScopedSetEntries(state.claudeUnconfirmedRestoredStatusPaneKeys, fromPaneKey, toPaneKey) movePaneScopedSetEntries(state.claudeRunningNonAgentTaskPaneKeys, fromPaneKey, toPaneKey) movePaneScopedSetEntries(state.claudeActiveSessionCronPaneKeys, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.claudeSessionOwnerByPaneKey, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.codexSubagentRosterByPaneKey, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.codexSubagentTranscriptByPaneKey, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.codexLeadStateByPaneKey, fromPaneKey, toPaneKey) @@ -291,6 +298,7 @@ export function clearAllListenerCaches(state: HookListenerState): void { state.claudeUnconfirmedRestoredStatusPaneKeys.clear() state.claudeRunningNonAgentTaskPaneKeys.clear() state.claudeActiveSessionCronPaneKeys.clear() + state.claudeSessionOwnerByPaneKey.clear() state.codexSubagentRosterByPaneKey.clear() state.codexSubagentTranscriptByPaneKey.clear() state.codexLeadStateByPaneKey.clear() @@ -2627,6 +2635,9 @@ function updateClaudeRunningNonAgentTask( state: HookListenerState, paneKey: string, hasRunningNonAgentTask: boolean, + /** Lead-turn property. Pass `false` from any non-lead fold: an interrupt clears the gate even when + * the inventory positively reports a running shell, which is a live-shell judgement no new call + * site may inherit by copying this signature. */ interrupted: boolean ): void { if (hasRunningNonAgentTask && !interrupted) { @@ -2910,6 +2921,76 @@ function buildClaudeCachedLeadStatusPayload( }) } +/** Lead events that may re-anchor a pane's owning session. Allow-list, not a deny-list: a payload we + * can't attribute (unknown name, child event missing its agent_id) must void nothing. */ +const CLAUDE_SESSION_OWNER_EVENTS: ReadonlySet = new Set([ + 'SessionStart', + 'UserPromptSubmit', + 'Stop', + 'StopFailure', + 'PreToolUse', + 'PostToolUse', + 'PostToolUseFailure', + 'PermissionRequest' +]) + +/** A pane whose `session_id` changed is running a different conversation, so claims the previous one + * owned are void — the hook-independent backstop for /clear, relaunch and resume, which emit no + * terminating hook (SessionEnd covers about a third of exit paths). + * + * Voids only what the replaced session provably owned. Deliberately NOT voided: + * - `claudeRunningNonAgentTaskPaneKeys`: a background shell is an OS process that survives /clear, + * and the previous inventory is positive evidence it was running. Only a fresh inventory or a + * certified process death may retire it. + * - `confirmedTeammate` roster rows: persistent in-process teammates a lead replacement can't end. + * - `claudeLeadStateByPaneKey`: the caller's own fold overwrites it anyway. */ +function voidClaimsOfReplacedClaudeSession( + state: HookListenerState, + eventName: unknown, + eventAgentId: string | undefined, + paneKey: string, + hookPayload: Record +): void { + if ( + eventAgentId !== undefined || + typeof eventName !== 'string' || + !CLAUDE_SESSION_OWNER_EVENTS.has(eventName) + ) { + return + } + const sessionId = readString(hookPayload, 'session_id') + if (!sessionId) { + return + } + const previousOwner = state.claudeSessionOwnerByPaneKey.get(paneKey) + state.claudeSessionOwnerByPaneKey.set(paneKey, sessionId) + if (previousOwner === undefined || previousOwner === sessionId) { + return + } + // Why: a compact restart mints a SessionStart mid-turn under the same conversation; the existing + // handler already fails closed on non-idle sources, and this must not undercut it. + if ( + eventName === 'SessionStart' || + hookPayload.trigger === 'manual' || + hookPayload.trigger === 'auto' + ) { + return + } + state.claudeActiveSessionCronPaneKeys.delete(paneKey) + const roster = state.claudeSubagentRosterByPaneKey.get(paneKey) + if (!roster) { + return + } + for (const [id, tracked] of roster) { + if (tracked.confirmedTeammate !== true) { + roster.delete(id) + } + } + if (roster.size === 0) { + state.claudeSubagentRosterByPaneKey.delete(paneKey) + } +} + function normalizeClaudeEvent( state: HookListenerState, eventName: unknown, @@ -2925,6 +3006,7 @@ function normalizeClaudeEvent( ) { return normalizeClaudeSubagentLifecycleEvent(state, eventName, paneKey, hookPayload) } + voidClaimsOfReplacedClaudeSession(state, eventName, eventAgentId, paneKey, hookPayload) if (eventName === 'SessionStart') { // Why: SessionStart is the only signal a resumed session emits before its first prompt // (STA-3386). Land it as a session-boundary 'done' row: 'working' would show a phantom