mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
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.
This commit is contained in:
+339
@@ -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<string, AgentCompletionCoordinatorOptions>(),
|
||||
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<Record<string, unknown>>()),
|
||||
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<Record<string, unknown>>())
|
||||
})
|
||||
|
||||
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<typeof React>()
|
||||
return {
|
||||
...actual,
|
||||
useCallback: <T extends (...args: unknown[]) => 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<Record<string, unknown>>()
|
||||
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<typeof CoordinatorModule>()
|
||||
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<typeof connectPanePty>))
|
||||
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<AgentStatusEntry> {
|
||||
// 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<boolean>): 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<boolean>()
|
||||
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()
|
||||
})
|
||||
})
|
||||
+33
@@ -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 })
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user