From 14ec4701466ebb69726437e0807a62a1b7633eb8 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:01:38 -0700 Subject: [PATCH] fix(terminal-pane): a confirmed agent process exit removes the agent from the running list (#27742) In shells that send no command-finished mark (cmd.exe, Git Bash), quitting Claude or Codex left the pane's live agent row behind: the pane's process monitor confirmed the exit, but only the OSC 133;D path ever dropped the row. The monitor's confirmed exit now arms the same drop and main-side reconcile pair a confirmed 133;D uses, settled behind the existing host-verifiability gate: a pane whose owner process the host can verify keeps the host's decision; every other pane drops the row and keeps its resume identity. Skipped when a 133;D drop is already armed, for remote panes, and when the row belongs to another agent. --- ...connection-process-exit-row-retire.test.ts | 339 ++++++++++++++++++ .../confirmed-process-exit-row-retire.ts | 33 ++ .../pty-connection/pane-agent-identity.ts | 10 +- .../pty-connection/terminal-keydown-fit.ts | 8 +- 4 files changed, 385 insertions(+), 5 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/pty-connection-process-exit-row-retire.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pty-connection/confirmed-process-exit-row-retire.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection-process-exit-row-retire.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-process-exit-row-retire.test.ts new file mode 100644 index 00000000000..302ae5aac95 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-process-exit-row-retire.test.ts @@ -0,0 +1,339 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import { toAppSshPtyId } from '../../../../shared/ssh-pty-id' +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import type { TerminalAgent } from '../../../../shared/terminal-agent' +import type * as CoordinatorModule from './agent-completion-coordinator' +import type { AgentCompletionCoordinatorOptions } from './agent-completion-coordinator-types' +import { createDeferred, flushAsyncTicks } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + leafIdForPane, + type ConnectCallbacks, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { createInitialStoreState } from './pty-connection-test-store-fixtures' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { + coordinatorOptionsByPaneKey, + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep +} = vi.hoisted(() => ({ + coordinatorOptionsByPaneKey: new Map(), + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn() +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync +})) + +vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({ + ...(await importOriginal>()), + resetAndRefreshAllTerminalWebglAtlases +})) + +vi.mock('./terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ + shouldSeedCacheTimerOnInitialTitle +})) + +vi.mock('sonner', () => ({ + toast: { + info: toastInfo + } +})) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep +})) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn(() => { + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + getEagerPtyBufferHandle: vi.fn(() => undefined) + } +}) + +// Why: lets a test deliver the monitor's confirmed exit without driving a remote host's verdict. +vi.mock('./agent-completion-coordinator', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + createAgentCompletionCoordinator: (options: AgentCompletionCoordinatorOptions) => { + coordinatorOptionsByPaneKey.set(options.paneKey, options) + return actual.createAgentCompletionCoordinator(options) + } + } +}) + +async function connectAgentPane(args: { + ptyId: string + tabId?: string + paneId?: number + visible?: boolean +}): Promise<{ + paneKey: string + sendData: (data: string) => void + confirmExit: (agent: TerminalAgent) => void +}> { + const { connectPanePty } = await import('./pty-connection') + const dataCallbackRef: { current: ((data: string) => void) | null } = { current: null } + const transport = createMockTransport(args.ptyId) + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + dataCallbackRef.current = callbacks.onData ?? null + return { id: args.ptyId } + }) + transportFactoryQueue.push(transport) + const tabId = args.tabId ?? 'tab-1' + const paneId = args.paneId ?? 1 + const connectArgs = [ + createPane(paneId), + createManager(paneId), + buildPaneConnectionDeps(() => mockStoreState, { + tabId, + isVisibleRef: { current: args.visible ?? false } + }) + ] + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the shared pane fixtures implement the members connectPanePty touches. + connectPanePty(...(connectArgs as unknown as Parameters)) + await vi.advanceTimersByTimeAsync(20) + await flushAsyncTicks() + const paneKey = makePaneKey(tabId, leafIdForPane(paneId)) + return { + paneKey, + sendData: (data) => dataCallbackRef.current?.(data), + confirmExit: (agent) => + coordinatorOptionsByPaneKey.get(paneKey)?.onForegroundAgentExited?.({ + agent, + processName: agent + }) + } +} + +async function seedHookRow( + paneKey: string, + agentType: 'claude' | 'codex', + state: 'working' | 'done' = 'done' +): Promise { + // Why the real slice: the row must carry the acceptedStatusSeq a real accepted hook stamps. + const { createTestStore } = await import('@/store/slices/store-test-helpers') + const realStore = createTestStore() + realStore.getState().setAgentStatus(paneKey, { state, prompt: 'ship it', agentType }) + const row = realStore.getState().agentStatusByPaneKey[paneKey] + mockStoreState.agentStatusByPaneKey[paneKey] = row + return row +} + +function setHostCanVerifyOwner(answer: boolean | Promise): void { + window.api.agentStatus.hasVerifiableAgentProcess = vi.fn(() => Promise.resolve(answer)) +} + +describe('connectPanePty confirmed process exit retires the live agent row', () => { + beforeEach(async () => { + vi.resetModules() + vi.clearAllMocks() + vi.useFakeTimers() + coordinatorOptionsByPaneKey.clear() + transportFactoryQueue = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + await installTerminalTestGlobals() + mockStoreState.dropAgentStatus.mockImplementation((paneKey: string) => { + delete mockStoreState.agentStatusByPaneKey[paneKey] + }) + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + it('drops the row and retires main latches when Claude exits in a pane without command marks', async () => { + let foreground: string | null = 'claude' + let children = true + vi.mocked(window.api.pty.getForegroundProcess).mockImplementation(async () => foreground) + vi.mocked(window.api.pty.hasChildProcesses).mockImplementation(async () => children) + const { paneKey } = await connectAgentPane({ ptyId: 'pty-cmd-claude', visible: true }) + await seedHookRow(paneKey, 'claude') + await vi.advanceTimersByTimeAsync(3_000) + expect(mockStoreState.agentStatusByPaneKey[paneKey]).toBeDefined() + + // `/exit` in cmd.exe: the shell is back and no 133;D ever arrives. + foreground = 'cmd.exe' + children = false + await vi.advanceTimersByTimeAsync(10_000) + await flushAsyncTicks() + + expect(mockStoreState.dropAgentStatus).toHaveBeenCalledWith(paneKey) + // No `paneRemoved`: the pane outlived the agent, so main keeps the resume identity. + expect(mockStoreState.dropAgentStatus.mock.calls).toEqual([[paneKey]]) + expect(window.api.agentStatus.reconcileEndedProcess).toHaveBeenCalledWith(paneKey) + expect(mockStoreState.agentStatusByPaneKey[paneKey]).toBeUndefined() + }) + + it('retires a hook-identified row the foreground tracker never read', async () => { + const { paneKey, confirmExit } = await connectAgentPane({ ptyId: 'pty-cmd-hook-only' }) + await seedHookRow(paneKey, 'codex', 'working') + expect(mockStoreState.paneForegroundAgentByPaneKey[paneKey]?.agentEvidence).not.toBe( + 'process-read' + ) + + confirmExit('codex') + await flushAsyncTicks() + + expect(mockStoreState.dropAgentStatus).toHaveBeenCalledWith(paneKey) + expect(window.api.agentStatus.reconcileEndedProcess).toHaveBeenCalledWith(paneKey) + }) + + it('leaves a pane with an armed command-finished drop to its own confirming read', async () => { + vi.mocked(window.api.pty.confirmForegroundProcess).mockReturnValue(new Promise(() => {})) + const { paneKey, sendData, confirmExit } = await connectAgentPane({ ptyId: 'pty-zsh-claude' }) + // A live hook icon makes the 133;D defer its drop behind a confirming read. + await seedHookRow(paneKey, 'claude', 'working') + sendData('\x1b]133;D;0\x07') + + confirmExit('claude') + await flushAsyncTicks() + + expect(mockStoreState.dropAgentStatus).not.toHaveBeenCalled() + expect(window.api.agentStatus.reconcileEndedProcess).not.toHaveBeenCalled() + expect(mockStoreState.agentStatusByPaneKey[paneKey]).toBeDefined() + }) + + it('leaves a remote pane to its execution host', async () => { + const { paneKey, confirmExit } = await connectAgentPane({ + ptyId: toAppSshPtyId('conn-1', 'relay-pty-1') + }) + await seedHookRow(paneKey, 'claude') + + confirmExit('claude') + await flushAsyncTicks() + + expect(mockStoreState.dropAgentStatus).not.toHaveBeenCalled() + expect(window.api.agentStatus.reconcileEndedProcess).not.toHaveBeenCalled() + }) + + it('keeps the row when the host can verify the owner process itself', async () => { + setHostCanVerifyOwner(true) + const { paneKey, confirmExit } = await connectAgentPane({ ptyId: 'pty-mac-claude' }) + await seedHookRow(paneKey, 'claude') + + confirmExit('claude') + await flushAsyncTicks() + + expect(window.api.agentStatus.hasVerifiableAgentProcess).toHaveBeenCalledWith(paneKey) + expect(mockStoreState.dropAgentStatus).not.toHaveBeenCalled() + expect(window.api.agentStatus.reconcileEndedProcess).not.toHaveBeenCalled() + }) + + it('keeps the row when the agent is relaunched before the exit settles', async () => { + const hostAnswer = createDeferred() + setHostCanVerifyOwner(hostAnswer.promise) + const { paneKey, confirmExit } = await connectAgentPane({ ptyId: 'pty-cmd-relaunch' }) + const exitedRow = await seedHookRow(paneKey, 'claude') + + confirmExit('claude') + // The relaunched Claude's first hook lands while the host is still answering. + vi.setSystemTime(Date.now() + 1_000) + mockStoreState.agentStatusByPaneKey[paneKey] = { + ...exitedRow, + state: 'working', + prompt: 'next session', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + acceptedStatusSeq: (exitedRow.acceptedStatusSeq ?? 0) + 1 + } + hostAnswer.resolve(false) + await flushAsyncTicks() + + expect(mockStoreState.dropAgentStatus).not.toHaveBeenCalled() + expect(window.api.agentStatus.reconcileEndedProcess).not.toHaveBeenCalled() + }) + + it('keeps a row that belongs to a different agent', async () => { + const { paneKey, confirmExit } = await connectAgentPane({ ptyId: 'pty-cmd-other-agent' }) + await seedHookRow(paneKey, 'codex') + + confirmExit('claude') + await flushAsyncTicks() + + expect(mockStoreState.dropAgentStatus).not.toHaveBeenCalled() + expect(window.api.agentStatus.reconcileEndedProcess).not.toHaveBeenCalled() + }) + + it('retires only the split pane whose agent exited', async () => { + const left = await connectAgentPane({ ptyId: 'pty-split-left', paneId: 1 }) + const right = await connectAgentPane({ ptyId: 'pty-split-right', paneId: 2 }) + await seedHookRow(left.paneKey, 'claude') + await seedHookRow(right.paneKey, 'claude') + + left.confirmExit('claude') + await flushAsyncTicks() + + expect(mockStoreState.dropAgentStatus.mock.calls).toEqual([[left.paneKey]]) + expect(vi.mocked(window.api.agentStatus.reconcileEndedProcess).mock.calls).toEqual([ + [left.paneKey] + ]) + expect(mockStoreState.agentStatusByPaneKey[right.paneKey]).toBeDefined() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/confirmed-process-exit-row-retire.ts b/src/renderer/src/components/terminal-pane/pty-connection/confirmed-process-exit-row-retire.ts new file mode 100644 index 00000000000..47c0efafe8c --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection/confirmed-process-exit-row-retire.ts @@ -0,0 +1,33 @@ +import { useAppStore } from '@/store' +import type { RecognizedAgentProcess } from '../../../../../shared/agent-process-recognition' +import { resolveCompatibleAgentTypeForOwner } from '../../../../../shared/agent-title-owner' +import { isRemoteExecutionHostPtyId } from '../remote-execution-host-pty' + +import type { ConnectPanePtySession } from './connect-pane-pty-session' + +/** The pane's process monitor confirmed its agent exited: retire the live row the way a confirmed + * 133;D does, since shells without command marks (cmd.exe, Git Bash) never arm that drop. */ +export function retireLiveAgentRowOnConfirmedProcessExit( + session: ConnectPanePtySession, + exited: RecognizedAgentProcess +): void { + const ptyId = session.transport.getPtyId() + // Why: an armed 133;D drop owns its own confirming read, and a remote pane's host owns its rows. + if (session.deferredCommandFinishedStatusDrop || !ptyId || isRemoteExecutionHostPtyId(ptyId)) { + return + } + const entry = useAppStore.getState().agentStatusByPaneKey[session.cacheKey] + if ( + !entry || + resolveCompatibleAgentTypeForOwner(entry.agentType, exited.agent) !== exited.agent + ) { + return + } + const armedAcceptedStatusSeq = entry.acceptedStatusSeq + session.deferredCommandFinishedStatusDrop = () => + session.dropCommandFinishedStatusIfSameTurn(entry) + session.deferredConfirmedShellReconcile = () => + session.reconcileEndedProcessIfPaneQuiet(armedAcceptedStatusSeq) + // Why: a pane whose owner process the host can verify keeps the host's decision. + session.settleDeferredCommandFinishedStatusDrop({ processExitConfirmed: true }) +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts index dd433a42d00..0425194d893 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts @@ -110,7 +110,7 @@ export function installPaneAgentIdentity(session: ConnectPanePtySession): void { * pane armed at 1 that reports exactly once more compares equal. It needs a foreign drop inside * the confirm window; the relaunch case that would otherwise hit it is already discarded by * onCommandStarted, and the cost is retiring a pane the process table just proved is a shell. */ - const reconcileEndedProcessIfPaneQuiet = (armedAcceptedStatusSeq: number | undefined): void => { + session.reconcileEndedProcessIfPaneQuiet = (armedAcceptedStatusSeq: number | undefined): void => { const current = useAppStore.getState().agentStatusByPaneKey[session.cacheKey] if (current && current.acceptedStatusSeq !== armedAcceptedStatusSeq) { return @@ -124,7 +124,7 @@ export function installPaneAgentIdentity(session: ConnectPanePtySession): void { session.visibleForegroundSamplePending = false session.visibleForegroundSampleSettled = false session.settleDeferredCommandFinishedStatusDrop = ( - options: { confirmedShell?: boolean } = {} + options: { confirmedShell?: boolean; processExitConfirmed?: boolean } = {} ): void => { const dropStatus = session.deferredCommandFinishedStatusDrop const reconcile = session.deferredConfirmedShellReconcile @@ -147,6 +147,10 @@ export function installPaneAgentIdentity(session: ConnectPanePtySession): void { session.deferredCommandFinishedStatusDrop = null if (!verifiable) { dropStatus() + // Why: only a process-table-confirmed exit proves the agent's main-side latches are dead. + if (options.processExitConfirmed) { + reconcile?.() + } } } void Promise.resolve(window.api?.agentStatus?.hasVerifiableAgentProcess?.(session.cacheKey)) @@ -281,7 +285,7 @@ export function installPaneAgentIdentity(session: ConnectPanePtySession): void { const armedAcceptedStatusSeq = entry?.acceptedStatusSeq session.deferredCommandFinishedStatusDrop = dropStatus session.deferredConfirmedShellReconcile = () => - reconcileEndedProcessIfPaneQuiet(armedAcceptedStatusSeq) + session.reconcileEndedProcessIfPaneQuiet(armedAcceptedStatusSeq) return } session.deferredCommandFinishedStatusDrop = null diff --git a/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts b/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts index 2ba251c95ea..df9a8345da6 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts @@ -14,6 +14,7 @@ import { registerTerminalSideEffectFactConsumer } from '../terminal-side-effect- import { isAgentTaskCompleteTrackingEnabled } from './agent-task-complete-settings' import { isAgentProcessInspectionCostly } from '../agent-process-inspection-cost' import { isRemoteExecutionHostPtyId } from '../remote-execution-host-pty' +import { retireLiveAgentRowOnConfirmedProcessExit } from './confirmed-process-exit-row-retire' import type { ConnectPanePtySession } from './connect-pane-pty-session' @@ -213,8 +214,11 @@ export function installTerminalKeydownFit(session: ConnectPanePtySession): void ) }, // Why: the pane's only re-derivation of a process read where the shell emits no command marks. - onForegroundAgentExited: (exited) => - session.paneForegroundAgentTracker?.onProcessExitConfirmed(exited), + onForegroundAgentExited: (exited) => { + session.paneForegroundAgentTracker?.onProcessExitConfirmed(exited) + // Why: not behind the tracker's process-read guard, so hook-identified panes retire too. + retireLiveAgentRowOnConfirmedProcessExit(session, exited) + }, dispatchCompletion: (title, meta) => { if (meta?.source === 'process-exit') { session.clearSuppressedTitleSideEffects()