From 294fd2ac06cfff31e5a7271bcefab040ae31e0de Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:01:41 -0700 Subject: [PATCH] fix(agent-hooks): bound retained agent exits (#29600) * fix(agent-hooks): bound retained agent exits Proven exits were only forgotten by a successor claim, so closed tabs and retired panes kept theirs forever and every save rewrote them. Drop an exit when its tab closes, its pane retires or its PTY is really torn down (not a reconnectable disconnect), and prune exits older than the hydrate window at save. * fix(agent-hooks): closing a moved pane's former tab keeps its exit * fix(agent-hooks): keep an aged exit while its retained resume row remains * fix(agent-hooks): launch-authority retirement keeps a proven exit --- .../agent-hooks/server-agent-presence.test.ts | 67 +++++++++++++++++++ .../agent-hooks/server/agent-exit-evidence.ts | 24 +++++++ .../agent-hooks/server/server-persistence.ts | 12 +++- src/main/agent-hooks/server/server-state.ts | 1 + .../agent-hooks/server/server-tab-cleanup.ts | 15 +++++ .../ipc/pty-restore-record-seeding.test.ts | 24 ++++++- src/main/ipc/pty/provider/state-cleanup.ts | 8 ++- 7 files changed, 147 insertions(+), 4 deletions(-) diff --git a/src/main/agent-hooks/server-agent-presence.test.ts b/src/main/agent-hooks/server-agent-presence.test.ts index 80d3b4a9a7c..1c674cd2185 100644 --- a/src/main/agent-hooks/server-agent-presence.test.ts +++ b/src/main/agent-hooks/server-agent-presence.test.ts @@ -628,3 +628,70 @@ describe('retained agent exit', () => { expect(await server.checkAgentPresenceSince(PANE, attachedAt)).toBe('live') }) }) + +describe('retained agent exit lifetime', () => { + async function serverWithExit(): Promise { + const server = await createServer() + await hook(server, 'SessionStart') + await hook(server, 'SessionEnd', 'session-a', 'prompt_input_exit') + expect(await server.checkAgentPresenceSince(PANE, 0)).toBe('exited') + return server + } + + it('keeps the exit through a plain pane cleanup but drops it when the PTY retires', async () => { + const server = await serverWithExit() + server.clearPaneState(PANE) + expect(await server.checkAgentPresenceSince(PANE, 0)).toBe('exited') + server.clearPaneState(PANE, { forgetAgentExit: true }) + expect(await server.checkAgentPresenceSince(PANE, 0)).toBeNull() + }) + + it('drops the exit when the pane tab closes', async () => { + const server = await serverWithExit() + server.dropStatusEntriesByTabPrefix('tab-1') + expect(await server.checkAgentPresenceSince(PANE, 0)).toBeNull() + }) + + it('keeps a moved pane exit when its former tab closes', async () => { + const server = await serverWithExit() + const moved = makePaneKey('tab-moved', LEAF_1) + server.transferPaneAuthority(PANE, moved) + server.dropStatusEntriesByTabPrefix('tab-1') + expect(await server.checkAgentPresenceSince(moved, 0)).toBe('exited') + server.dropStatusEntriesByTabPrefix('tab-moved') + expect(await server.checkAgentPresenceSince(moved, 0)).toBeNull() + }) + + // Why: a finished shell command retires launch authority while the PTY, and the exit's meaning, survive. + it('keeps the exit when only the pane launch authority retires', async () => { + const server = await serverWithExit() + server.retirePaneAuthority(PANE) + expect(await server.checkAgentPresenceSince(PANE, 0)).toBe('exited') + }) + + it('prunes an exit older than the hydrate window when saving', async () => { + const server = new AgentHookServer() + servers.push(server) + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-presence-exit-prune-')) + await server.start({ env: 'production', userDataPath }) + await hook(server, 'SessionStart') + await hook(server, 'SessionEnd', 'session-a', 'prompt_input_exit') + expect(await server.checkAgentPresenceSince(PANE, 0)).toBe('exited') + const saveEightDaysLater = (): void => { + const now = Date.now() + const clock = vi.spyOn(Date, 'now').mockReturnValue(now + 8 * 24 * 60 * 60 * 1000) + try { + server.flushStatusPersistSync() + } finally { + clock.mockRestore() + } + } + // The retained resume row still names the session, so its exit must stay with it. + expect(server.getStatusSnapshot().some((row) => row.paneKey === PANE)).toBe(true) + saveEightDaysLater() + expect(await server.checkAgentPresenceSince(PANE, 0)).toBe('exited') + server.clearPaneState(PANE) + saveEightDaysLater() + expect(await server.checkAgentPresenceSince(PANE, 0)).toBeNull() + }) +}) diff --git a/src/main/agent-hooks/server/agent-exit-evidence.ts b/src/main/agent-hooks/server/agent-exit-evidence.ts index 5c41a9e21d2..8dcc0e396ec 100644 --- a/src/main/agent-hooks/server/agent-exit-evidence.ts +++ b/src/main/agent-hooks/server/agent-exit-evidence.ts @@ -80,6 +80,30 @@ export function claimsPaneAfterExit( ) } +/** Drops one retired key; a pane that moved away keeps the exit under its surviving keys. */ +export function releaseAgentExitKey( + exits: Map, + paneKey: string +): boolean { + const exit = exits.get(paneKey) + exit?.paneKeys.delete(paneKey) + return exits.delete(paneKey) +} + +/** Drops exits proven before `notBefore` unless a retained row still names one of their panes. */ +export function pruneAgentExits( + exits: Map, + notBefore: number, + isPaneRowRetained: (paneKey: string) => boolean +): void { + for (const [key, exit] of exits) { + // Why: a kept resume row without its exit would read as a live session holder. + if (exit.at < notBefore && ![...exit.paneKeys].some(isPaneRowRetained)) { + exits.delete(key) + } + } +} + export function serializeAgentExits( exits: ReadonlyMap ): PersistedAgentExitEvidence[] { diff --git a/src/main/agent-hooks/server/server-persistence.ts b/src/main/agent-hooks/server/server-persistence.ts index 8dbeefe671d..66e58fb7bcc 100644 --- a/src/main/agent-hooks/server/server-persistence.ts +++ b/src/main/agent-hooks/server/server-persistence.ts @@ -1,7 +1,11 @@ import { createHash } from 'node:crypto' import { isValidPaneKey } from './server-status-identity' -import { LAST_STATUS_FILE_VERSION, STATUS_PERSIST_MIN_INTERVAL_MS } from './server-constants' +import { + HYDRATE_MAX_AGE_MS, + LAST_STATUS_FILE_VERSION, + STATUS_PERSIST_MIN_INTERVAL_MS +} from './server-constants' import type { EnrichedAgentHookEventPayload, LastStatusFile, @@ -10,7 +14,7 @@ import type { } from './server-types' import { authorityCommitmentsMatch } from './server-persistence-validation' import { AgentHookServerHydration } from './server-hydration' -import { serializeAgentExits } from './agent-exit-evidence' +import { pruneAgentExits, serializeAgentExits } from './agent-exit-evidence' import { AtomicSnapshotWriter } from '../../persistence/atomic-snapshot-writer' export abstract class AgentHookServerPersistence extends AgentHookServerHydration { @@ -74,6 +78,10 @@ export abstract class AgentHookServerPersistence extends AgentHookServerHydratio } } } + // Why: only a successor claim forgets an exit, so a pane that never runs another agent would keep it. + pruneAgentExits(this.agentExitedAtByPaneKey, Date.now() - HYDRATE_MAX_AGE_MS, (paneKey) => + this.state.lastStatusByPaneKey.has(paneKey) + ) const agentExits = serializeAgentExits(this.agentExitedAtByPaneKey) const file: LastStatusFile = { version: LAST_STATUS_FILE_VERSION, diff --git a/src/main/agent-hooks/server/server-state.ts b/src/main/agent-hooks/server/server-state.ts index f7ce65bb107..a417c70764f 100644 --- a/src/main/agent-hooks/server/server-state.ts +++ b/src/main/agent-hooks/server/server-state.ts @@ -310,6 +310,7 @@ export abstract class AgentHookServerState { emitStatusRowMutation?: boolean preserveTmuxInnerSubjects?: boolean statusUnavailable?: true + forgetAgentExit?: true } ): void protected abstract deleteStatusEntry( diff --git a/src/main/agent-hooks/server/server-tab-cleanup.ts b/src/main/agent-hooks/server/server-tab-cleanup.ts index bc1d481498b..fa2729770dd 100644 --- a/src/main/agent-hooks/server/server-tab-cleanup.ts +++ b/src/main/agent-hooks/server/server-tab-cleanup.ts @@ -2,6 +2,7 @@ import { clearPaneCacheState } from '../../../shared/agent-hook-listener/listene import { paneCacheKeyMatchesTab } from './server-status-identity' import { AgentHookServerCleanup } from './server-cleanup' import type { EnrichedAgentHookEventPayload } from './server-types' +import { releaseAgentExitKey } from './agent-exit-evidence' export abstract class AgentHookServerTabCleanup extends AgentHookServerCleanup { /** Drop every status/cache claim attributable to a closed tab prefix. */ @@ -85,6 +86,13 @@ export abstract class AgentHookServerTabCleanup extends AgentHookServerCleanup { this.restartedStatusLaunchTokenHashByPaneKey.delete(paneKey) this.evidenceObservedAtByPaneKey.delete(paneKey) } + // Why: an exit outlives its row, so it is matched by tab rather than by the keys cleared above. + let exitsChanged = false + for (const key of this.agentExitedAtByPaneKey.keys()) { + if (paneCacheKeyMatchesTab(key, tabId)) { + exitsChanged = releaseAgentExitKey(this.agentExitedAtByPaneKey, key) || exitsChanged + } + } if (aliasChanged) { this.notifyPaneKeyAliasPersistenceListener() } @@ -94,6 +102,8 @@ export abstract class AgentHookServerTabCleanup extends AgentHookServerCleanup { if (statusChanged || authorityChanged) { this.scheduleStatusPersist() this.notifyStatusChangeListeners() + } else if (exitsChanged) { + this.scheduleStatusPersist() } // Why: tab teardown must retire status subscribers' pane-scoped memo state too. for (const paneKey of statusPaneKeysToClear) { @@ -107,9 +117,14 @@ export abstract class AgentHookServerTabCleanup extends AgentHookServerCleanup { emitStatusRowMutation?: boolean preserveTmuxInnerSubjects?: boolean statusUnavailable?: true + /** The pane's PTY is retired, not just its agent. */ + forgetAgentExit?: true } ): void { const resolvedPaneKey = this.resolvePaneKeyAlias(paneKey) + if (options?.forgetAgentExit) { + this.forgetAgentExit(paneKey) + } if (!options?.preserveTmuxInnerSubjects) { this.clearTmuxInnerSubjects(resolvedPaneKey) } diff --git a/src/main/ipc/pty-restore-record-seeding.test.ts b/src/main/ipc/pty-restore-record-seeding.test.ts index 2a485a953f9..16018886c12 100644 --- a/src/main/ipc/pty-restore-record-seeding.test.ts +++ b/src/main/ipc/pty-restore-record-seeding.test.ts @@ -576,10 +576,32 @@ describe('registerPtyHandlers', () => { clearProviderPtyState(first.id) expect(getPtyIdForPaneKey(stablePaneKey)).toBe(second.id) - expect(clearAgentHookPaneStateMock).not.toHaveBeenCalledWith(stablePaneKey) + expect(clearAgentHookPaneStateMock).not.toHaveBeenCalled() clearProviderPtyState(second.id) expect(getPtyIdForPaneKey(stablePaneKey)).toBeUndefined() + expect(clearAgentHookPaneStateMock).toHaveBeenCalledWith(stablePaneKey, { + forgetAgentExit: true + }) + }) + // Why: a disconnect that can reconnect is no death, so the hook store keeps the agent's proven exit. + it('keeps the proven agent exit when a PTY is cleared for a disconnect', async () => { + registerPtyHandlers(mainWindow as never) + const stablePaneKey = makePaneKey('tab-1', '11111111-1111-4111-8111-111111111111') + await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: '11111111-1111-4111-8111-111111111111', + env: { ORCA_PANE_KEY: stablePaneKey } + }) + const ptyId = getPtyIdForPaneKey(stablePaneKey) + expect(ptyId).toBeDefined() + clearAgentHookPaneStateMock.mockClear() + + clearProviderPtyState(ptyId ?? '', { preserveAgentSessionOwners: true }) + expect(clearAgentHookPaneStateMock).toHaveBeenCalledWith(stablePaneKey) }) it('does not let restart-era alias cleanup clear a newer pane-key owner', async () => { diff --git a/src/main/ipc/pty/provider/state-cleanup.ts b/src/main/ipc/pty/provider/state-cleanup.ts index 0624fe38d16..df43c618084 100644 --- a/src/main/ipc/pty/provider/state-cleanup.ts +++ b/src/main/ipc/pty/provider/state-cleanup.ts @@ -88,7 +88,13 @@ export function clearProviderPtyState( // Why: clear the hook server's per-paneKey caches (via the spawn-time paneKey mapping, its only ptyId→paneKey correlation) so dead panes don't accumulate over process lifetime. if (paneKey) { if (stillOwnsPaneKey) { - agentHookServer.clearPaneState(paneKey) + // Why: a proven exit describes this PTY's agent, so only a real teardown (not a + // disconnect that can reconnect) may drop it. + if (opts.preserveAgentSessionOwners) { + agentHookServer.clearPaneState(paneKey) + } else { + agentHookServer.clearPaneState(paneKey, { forgetAgentExit: true }) + } paneKeyPtyId.delete(paneKey) } ptyPaneKey.delete(id)