From b91531eb2aa3ee1b0592e34e9e1e0101a01f57eb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:47:11 -0700 Subject: [PATCH] fix(terminal): preserve exact clean-exit authority --- .../parked-terminal-byte-watcher.ts | 6 ++- ...rminal-confirmed-agent-exit-resume.test.ts | 46 ++++++++++++++++ .../pty-connection-test-store-fixtures.ts | 1 + .../pty-connection/sleeping-record-access.ts | 48 +++-------------- .../remote-runtime-pty-transport.ts | 9 ++-- ...subscribe-failure-recovery-routing.test.ts | 33 +++++++++--- ...terminal-side-effect-facts-handler.test.ts | 15 ++++++ .../terminal-side-effect-facts-handler.ts | 26 ++++++--- .../confirmed-agent-exit-resume-retirement.ts | 53 +++++++++++++++++-- 9 files changed, 175 insertions(+), 62 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts index 9bd5a1d1ca8..769294f580a 100644 --- a/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts +++ b/src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts @@ -252,7 +252,11 @@ export function startParkedTerminalByteWatcher( ...sideEffectCallbacks, onAgentExited: (fact) => { if (fact.executionHostConfirmed === true) { - retireConfirmedAgentExitResumeAuthority(useAppStore.getState(), paneKey) + retireConfirmedAgentExitResumeAuthority(useAppStore.getState(), paneKey, { + tabId, + worktreeId, + paneId: options.paneId + }) } sideEffectCallbacks.onAgentExited?.() }, diff --git a/src/renderer/src/components/terminal-pane/parked-terminal-confirmed-agent-exit-resume.test.ts b/src/renderer/src/components/terminal-pane/parked-terminal-confirmed-agent-exit-resume.test.ts index 23a95ca4127..2ea72d3c4c5 100644 --- a/src/renderer/src/components/terminal-pane/parked-terminal-confirmed-agent-exit-resume.test.ts +++ b/src/renderer/src/components/terminal-pane/parked-terminal-confirmed-agent-exit-resume.test.ts @@ -144,4 +144,50 @@ describe('parked terminal confirmed agent-exit resume retirement', () => { expect(mockStoreState.sleepingAgentSessionsByPaneKey[PANE_KEY]).toBeUndefined() dispose() }) + + it('retires a legacy numeric-key resume record for the parked pane', async () => { + const { startParkedTerminalByteWatcher } = await import('./parked-terminal-byte-watcher') + const handler = await import('./terminal-side-effect-facts-handler') + const legacyPaneKey = `${TAB_ID}:1` + mockStoreState.sleepingAgentSessionsByPaneKey = { + [legacyPaneKey]: { + paneKey: legacyPaneKey, + tabId: TAB_ID, + worktreeId: WORKTREE_ID, + agent: 'claude', + providerSession: { key: 'session_id', id: 'parked-session' }, + prompt: 'finish the task', + state: 'working', + capturedAt: 1, + updatedAt: 1, + origin: 'live' + } + } + + const dispose = startParkedTerminalByteWatcher({ + ptyId: PTY_ID, + incarnationId: 'inc-parked-exit', + tabId: TAB_ID, + worktreeId: WORKTREE_ID, + leafId: LEAF_ID, + paneId: 1 + } satisfies ParkedTerminalByteWatcherOptions) + handler._dispatchTerminalSideEffectBatchForTest({ + ptyId: PTY_ID, + seq: 1, + paneKey: PANE_KEY, + tabId: TAB_ID, + worktreeId: WORKTREE_ID, + facts: [ + { + kind: 'agent-exited', + executionHostConfirmed: true, + incarnationId: 'inc-parked-exit' + } + ] + }) + + expect(mockStoreState.sleepingAgentSessionsByPaneKey[legacyPaneKey]).toBeUndefined() + dispose() + }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-test-store-fixtures.ts b/src/renderer/src/components/terminal-pane/pty-connection-test-store-fixtures.ts index d69c055d550..e41395e72f9 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-test-store-fixtures.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-test-store-fixtures.ts @@ -63,6 +63,7 @@ export function createInitialStoreState(getState: () => StoreState): StoreState ), clearSleepingAgentSession: vi.fn((paneKey: string) => { delete getState().sleepingAgentSessionsByPaneKey[paneKey] + delete getState().agentLaunchConfigByPaneKey[paneKey] }), clearSleepingAgentSessionsByPaneKey: vi.fn((paneKeys: readonly string[]) => { for (const paneKey of paneKeys) { diff --git a/src/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.ts b/src/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.ts index 6b725d7746c..ba7c312b56c 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.ts @@ -1,9 +1,8 @@ import { useAppStore } from '@/store' import type { PtyConnectResult } from '../pty-transport' import { createBrowserUuid } from '@/lib/browser-uuid' -import { parseLegacyNumericPaneKey } from '../../../../../shared/stable-pane-id' -import { getProviderSessionClaimKey } from '@/lib/sleeping-agent-pane-ownership' import type { SleepingAgentSessionRecord } from '../../../../../shared/agent-session-resume' +import { findSleepingAgentResumeRecordForPane } from '@/lib/confirmed-agent-exit-resume-retirement' import { recognizeAgentProcessFromCommandLine } from '../../../../../shared/agent-process-recognition' import type { TuiAgent } from '../../../../../shared/tui-agent' import { TUI_AGENT_CONFIG } from '../../../../../shared/tui-agent-config' @@ -19,46 +18,13 @@ import { installCommandInferredPaneAgent } from './command-inferred-pane-agent' export function installSleepingRecordAccess(session: ConnectPanePtySession): void { session.getSleepingRecordForPane = ( state: ReturnType - ): { paneKey: string; record: SleepingAgentSessionRecord } | null => { - const stableRecord = state.sleepingAgentSessionsByPaneKey[session.cacheKey] - if (stableRecord) { - return { paneKey: session.cacheKey, record: stableRecord } - } - const legacyMatches = Object.entries(state.sleepingAgentSessionsByPaneKey).filter( - ([paneKey, record]) => { - const legacy = parseLegacyNumericPaneKey(paneKey) - return ( - legacy?.tabId === session.deps.tabId && - record.worktreeId === session.deps.worktreeId && - (!record.tabId || record.tabId === session.deps.tabId) - ) - } - ) - const exactLegacyMatch = legacyMatches.find(([paneKey]) => { - const legacy = parseLegacyNumericPaneKey(paneKey) - return legacy?.numericPaneId === String(session.pane.id) + ): { paneKey: string; record: SleepingAgentSessionRecord } | null => + findSleepingAgentResumeRecordForPane(state.sleepingAgentSessionsByPaneKey, { + paneKey: session.cacheKey, + tabId: session.deps.tabId, + worktreeId: session.deps.worktreeId, + paneId: session.pane.id }) - const providerSessionKeys = new Set( - legacyMatches.map(([, record]) => getProviderSessionClaimKey(record)) - ) - const oldestLegacyMatch = legacyMatches - .slice() - .sort(([, a], [, b]) => a.capturedAt - b.capturedAt || a.updatedAt - b.updatedAt)[0] - // Why: duplicate legacy aliases can point at one provider session; consume - // the oldest capture as canonical and clear its aliases after resume. - const selectedLegacyMatch = - exactLegacyMatch ?? - (providerSessionKeys.size === 1 - ? legacyMatches.length === 1 - ? legacyMatches[0] - : oldestLegacyMatch - : null) - if (!selectedLegacyMatch) { - return null - } - const [paneKey, record] = selectedLegacyMatch - return { paneKey, record } - } session.isLegacyWorkerAutomaticResumeBlocked = (): boolean => session.getSleepingRecordForPane(useAppStore.getState())?.record.automaticResumeBlockedBy === 'legacy-orchestration-worker' diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index a9be9c37ce9..5d83e76c644 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -1458,7 +1458,10 @@ export function createRemoteRuntimePtyTransport( } } - function rebindRemoteTerminalHandle(nextHandle: string): void { + function rebindRemoteTerminalHandle( + nextHandle: string, + incarnationId?: RuntimeTerminalResolvePane['incarnationId'] + ): void { clearPublishedHandleWait() const replacedPtyId = remotePtyId unregisterShutdownHandlers(replacedPtyId) @@ -1472,7 +1475,7 @@ export function createRemoteRuntimePtyTransport( if (replacedPtyId) { replaceFitOverridePtyId(replacedPtyId, remotePtyId) replaceDriverPtyId(replacedPtyId, remotePtyId) - onPtyRebind?.(remotePtyId, replacedPtyId) + notifyPtyRebindForRemote(remotePtyId, replacedPtyId, incarnationId) } } @@ -1682,7 +1685,7 @@ export function createRemoteRuntimePtyTransport( return } if (resolved.handle !== previousHandle) { - rebindRemoteTerminalHandle(resolved.handle) + rebindRemoteTerminalHandle(resolved.handle, resolved.incarnationId) } } clearPublishedHandleWait() diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts index 7353e491ef1..cccabe6d8fa 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts @@ -31,7 +31,7 @@ import { } from '../../../../shared/terminal-stream-protocol' import type { PtyTransport } from './pty-transport-types' -type ResolvePaneOutcome = { handle: string } | { error: Error } +type ResolvePaneOutcome = { handle: string; incarnationId?: string } | { error: Error } const PANE_TAB_ID = 'tab-1' const PANE_LEAF_ID = 'pane:1' @@ -117,7 +117,12 @@ describe('remote runtime resubscribe failure: recovery routing', () => { return outcomes.length > 1 ? (outcomes.shift() as ResolvePaneOutcome) : outcomes[0] } - function paneResult(handle: string, paneKey: string, worktreeId: string): unknown { + function paneResult( + handle: string, + paneKey: string, + worktreeId: string, + incarnationId?: string + ): unknown { const separator = paneKey.indexOf(':') return { ok: true, @@ -126,7 +131,8 @@ describe('remote runtime resubscribe failure: recovery routing', () => { handle, tabId: paneKey.slice(0, separator), leafId: paneKey.slice(separator + 1), - worktreeId + worktreeId, + ...(incarnationId ? { incarnationId } : {}) } } } @@ -192,14 +198,19 @@ describe('remote runtime resubscribe failure: recovery routing', () => { if ('error' in outcome) { throw outcome.error } - return paneResult(outcome.handle, params.paneKey, params.worktreeId) + return paneResult(outcome.handle, params.paneKey, params.worktreeId, outcome.incarnationId) } if (request.method === 'terminal.recoverPane') { const params = request.params as { paneKey: string; worktreeId: string } if ('error' in recoverPaneOutcome) { throw recoverPaneOutcome.error } - return paneResult(recoverPaneOutcome.handle, params.paneKey, params.worktreeId) + return paneResult( + recoverPaneOutcome.handle, + params.paneKey, + params.worktreeId, + recoverPaneOutcome.incarnationId + ) } return { ok: true, result: { terminal: { handle: FIRST_HANDLE } } } }) @@ -231,13 +242,21 @@ describe('remote runtime resubscribe failure: recovery routing', () => { // The host fenced this handle during the outage and has already minted its // successor; only a require-replacement retry can reach it. - failNextResubscribeWith(new Error('terminal_handle_stale'), 'terminal-2') + resolvePaneOutcomes = [ + { error: new Error('terminal_handle_stale') }, + { handle: 'terminal-2', incarnationId: 'incarnation-2' } + ] + methodLog = [] dropMultiplexedStream() await vi.waitFor(() => expect(subscribedTerminalHandles()).toContain('terminal-2')) // Two resolvePane round-trips: the stale one, then the require-replacement retry. expect(methodLog.filter((method) => method === 'terminal.resolvePane')).toHaveLength(2) - expect(onPtyRebind).toHaveBeenCalledWith('remote:env-1@@terminal-2', FIRST_PTY_ID) + expect(onPtyRebind).toHaveBeenCalledWith( + 'remote:env-1@@terminal-2', + FIRST_PTY_ID, + 'incarnation-2' + ) expect(transport.getPtyId()).toBe('remote:env-1@@terminal-2') emitSnapshot(latestSubscribePayload().streamId, 'after replacement') diff --git a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts index af1e205fc39..8a857c0bf7c 100644 --- a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.ts @@ -462,6 +462,21 @@ describe('registerTerminalSideEffectFactConsumer', () => { expect(events).toEqual([]) }) + it('keeps host-confirmed exits fail-closed when attribution is unknown', () => { + const { callbacks, events } = createCallbackRecorder() + registerTerminalSideEffectFactConsumer({ + ptyId: PTY_ID, + incarnationId: 'inc-new', + callbacks + }) + + _dispatchTerminalSideEffectBatchForTest( + batch([{ kind: 'agent-exited', executionHostConfirmed: true, incarnationId: 'inc-new' }]) + ) + + expect(events).toEqual([]) + }) + it('keeps host-confirmed exits fail-closed when pane attribution is stale', () => { const { callbacks, events } = createCallbackRecorder() registerTerminalSideEffectFactConsumer({ diff --git a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts index 5deb086df5e..df523ff0b40 100644 --- a/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts +++ b/src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts @@ -151,10 +151,16 @@ function applyLiveFact( if ( !fact.incarnationId || !entry.incarnationId || + !entry.paneKey || + !entry.tabId || + !entry.worktreeId || + !batch.paneKey || + !batch.tabId || + !batch.worktreeId || fact.incarnationId !== entry.incarnationId || - entry.paneKey !== (batch.paneKey ?? null) || - entry.tabId !== (batch.tabId ?? null) || - entry.worktreeId !== (batch.worktreeId ?? null) + entry.paneKey !== batch.paneKey || + entry.tabId !== batch.tabId || + entry.worktreeId !== batch.worktreeId ) { return } @@ -221,14 +227,22 @@ export function dispatchTerminalSideEffectBatch(batch: TerminalSideEffectBatch): if ( !batch.replay && batch.paneKey && + batch.tabId && + batch.worktreeId && confirmedExit?.kind === 'agent-exited' && confirmedExit.incarnationId && + handoffAuthority?.paneKey && + handoffAuthority.tabId && + handoffAuthority.worktreeId && handoffAuthority?.incarnationId === confirmedExit.incarnationId && handoffAuthority.paneKey === batch.paneKey && - handoffAuthority.tabId === (batch.tabId ?? null) && - handoffAuthority.worktreeId === (batch.worktreeId ?? null) + handoffAuthority.tabId === batch.tabId && + handoffAuthority.worktreeId === batch.worktreeId ) { - retireConfirmedAgentExitResumeAuthority(useAppStore.getState(), batch.paneKey) + retireConfirmedAgentExitResumeAuthority(useAppStore.getState(), batch.paneKey, { + tabId: batch.tabId, + worktreeId: batch.worktreeId + }) } bufferTerminalSideEffectFactHandoff(batch) return diff --git a/src/renderer/src/lib/confirmed-agent-exit-resume-retirement.ts b/src/renderer/src/lib/confirmed-agent-exit-resume-retirement.ts index a68e56093fd..b0c7f4b549d 100644 --- a/src/renderer/src/lib/confirmed-agent-exit-resume-retirement.ts +++ b/src/renderer/src/lib/confirmed-agent-exit-resume-retirement.ts @@ -2,12 +2,49 @@ import { agentProviderSessionsEqual, type SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import { parseLegacyNumericPaneKey } from '../../../shared/stable-pane-id' +import { getProviderSessionClaimKey } from './sleeping-agent-pane-ownership' type ResumeRetirementStore = { sleepingAgentSessionsByPaneKey: Record clearSleepingAgentSessionsByPaneKey: (paneKeys: readonly string[]) => void } +export function findSleepingAgentResumeRecordForPane( + records: Record, + identity: { paneKey: string; tabId: string; worktreeId: string; paneId?: string | number } +): { paneKey: string; record: SleepingAgentSessionRecord } | null { + const stableRecord = records[identity.paneKey] + if (stableRecord) { + return { paneKey: identity.paneKey, record: stableRecord } + } + const legacyMatches = Object.entries(records).filter(([paneKey, record]) => { + const legacy = parseLegacyNumericPaneKey(paneKey) + return ( + legacy?.tabId === identity.tabId && + record.worktreeId === identity.worktreeId && + (!record.tabId || record.tabId === identity.tabId) + ) + }) + const exactLegacyMatch = legacyMatches.find(([paneKey]) => { + const legacy = parseLegacyNumericPaneKey(paneKey) + return identity.paneId !== undefined && legacy?.numericPaneId === String(identity.paneId) + }) + const providerSessionKeys = new Set( + legacyMatches.map(([, record]) => getProviderSessionClaimKey(record)) + ) + const selectedLegacyMatch = + exactLegacyMatch ?? + (providerSessionKeys.size === 1 + ? legacyMatches + .slice() + .sort(([, a], [, b]) => a.capturedAt - b.capturedAt || a.updatedAt - b.updatedAt)[0] + : undefined) + return selectedLegacyMatch + ? { paneKey: selectedLegacyMatch[0], record: selectedLegacyMatch[1] } + : null +} + export function retireConfirmedAgentExitResumeRecord( state: ResumeRetirementStore, consumed: { paneKey: string; record: SleepingAgentSessionRecord } @@ -32,11 +69,19 @@ export function retireConfirmedAgentExitResumeRecord( export function retireConfirmedAgentExitResumeAuthority( state: ResumeRetirementStore, - paneKey: string + paneKey: string, + legacyIdentity?: { tabId: string; worktreeId: string; paneId?: string | number } ): void { - const record = state.sleepingAgentSessionsByPaneKey[paneKey] - if (!record) { + const consumed = legacyIdentity + ? findSleepingAgentResumeRecordForPane(state.sleepingAgentSessionsByPaneKey, { + paneKey, + ...legacyIdentity + }) + : state.sleepingAgentSessionsByPaneKey[paneKey] + ? { paneKey, record: state.sleepingAgentSessionsByPaneKey[paneKey] } + : null + if (!consumed) { return } - retireConfirmedAgentExitResumeRecord(state, { paneKey, record }) + retireConfirmedAgentExitResumeRecord(state, consumed) }