diff --git a/src/main/daemon/immediate-kill-reply-budget.ts b/src/main/daemon/immediate-kill-reply-budget.ts new file mode 100644 index 000000000000..5c3f2667ccfa --- /dev/null +++ b/src/main/daemon/immediate-kill-reply-budget.ts @@ -0,0 +1,19 @@ +// Import-free so runtime modules, and tests that mock the kill machinery, can read the budget. + +// Leave room for capture and root exit within daemon-entry's 5s shutdown budget. +export const SHUTDOWN_DESCENDANT_VERIFY_MS = 2_500 +export const SHUTDOWN_DESCENDANT_TABLE_TIMEOUT_MS = 250 +export const IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS = 8_000 +export const SESSION_FORCE_KILL_RETRY_MS = 250 +// Mirrors DESCENDANT_SNAPSHOT_TIMEOUT_MS, which immediate teardown's initial capture uses. +export const IMMEDIATE_KILL_CAPTURE_TIMEOUT_MS = 1_000 + +/** Longest the daemon takes to answer an immediate kill: descendant capture, the shutdown + * verification window plus its two overrunning table reads, one failed force-kill's retry wait, + * then the root's physical-exit wait. */ +export const IMMEDIATE_KILL_REPLY_BUDGET_MS = + IMMEDIATE_KILL_CAPTURE_TIMEOUT_MS + + SHUTDOWN_DESCENDANT_VERIFY_MS + + 2 * SHUTDOWN_DESCENDANT_TABLE_TIMEOUT_MS + + SESSION_FORCE_KILL_RETRY_MS + + IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS diff --git a/src/main/daemon/session-termination-controller.ts b/src/main/daemon/session-termination-controller.ts index 814be8228c3c..9ccee58af650 100644 --- a/src/main/daemon/session-termination-controller.ts +++ b/src/main/daemon/session-termination-controller.ts @@ -2,10 +2,13 @@ import { killWithDescendantSweep } from '../pty-descendant-termination' import { PhysicalExitTracker } from '../../shared/physical-exit-tracker' import type { SubprocessHandle } from './session-subprocess-handle' import type { TuiAgent } from '../../shared/tui-agent' +import { + IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS, + SESSION_FORCE_KILL_RETRY_MS +} from './immediate-kill-reply-budget' const KILL_TIMEOUT_MS = 5_000 -export const IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS = 8_000 -export const SESSION_FORCE_KILL_RETRY_MS = 250 +export { IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS, SESSION_FORCE_KILL_RETRY_MS } const SESSION_FORCE_KILL_MAX_ATTEMPTS = 2 export type SessionTerminationControllerDeps = { diff --git a/src/main/daemon/terminal-descendant-shutdown.ts b/src/main/daemon/terminal-descendant-shutdown.ts index 8a2f2fa43504..1cd13c6aeb39 100644 --- a/src/main/daemon/terminal-descendant-shutdown.ts +++ b/src/main/daemon/terminal-descendant-shutdown.ts @@ -4,6 +4,10 @@ import { terminateDescendantSnapshotWithVerdict, type DescendantTreeVerdict } from '../pty-descendant-exit-verification' +import { + SHUTDOWN_DESCENDANT_TABLE_TIMEOUT_MS, + SHUTDOWN_DESCENDANT_VERIFY_MS +} from './immediate-kill-reply-budget' // Keep one fresh table for the short burst of verifier polls that follows a shutdown signal. // This bounds process-table fanout without reusing a completed capture for a later polling round. @@ -40,9 +44,8 @@ export function terminateShutdownDescendants( return Promise.resolve('exited') } return terminateDescendantSnapshotWithVerdict(snapshot, { - // Leave room for capture and root exit within daemon-entry's 5s shutdown budget. - verifyMs: 2500, - timeoutMs: 250, + verifyMs: SHUTDOWN_DESCENDANT_VERIFY_MS, + timeoutMs: SHUTDOWN_DESCENDANT_TABLE_TIMEOUT_MS, keepAlive: true, requireIdentityBeforeSignal: true, readTable: readShutdownProcessTable diff --git a/src/main/runtime/orca-runtime-core.ts b/src/main/runtime/orca-runtime-core.ts index 8214f7506761..f2c7cf4405de 100644 --- a/src/main/runtime/orca-runtime-core.ts +++ b/src/main/runtime/orca-runtime-core.ts @@ -5,6 +5,7 @@ import type { ResolvedWorktree } from './runtime-worktree-path-identity' import type { RuntimeLeafRecord } from './runtime-terminal-state-records' import { isCursorAgentTitle } from '../../shared/agent-detection' import { isAbsolute, relative, resolve } from 'node:path' +import { IMMEDIATE_KILL_REPLY_BUDGET_MS } from '../daemon/immediate-kill-reply-budget' import type { RuntimeTerminalDriverState, RuntimeTerminalPresentation @@ -77,7 +78,9 @@ export const MOBILE_TERMINAL_SURFACE_TIMEOUT_MS = 10_000 // fallback kill is needed, so keep it short — an unreachable host must not stall the rejection. export const REJECTED_SPLIT_PTY_STOP_TIMEOUT_MS = 2_000 -export const EXPLICIT_TERMINAL_CLOSE_STOP_TIMEOUT_MS = 2_000 +// Why: covers the daemon's own immediate-kill verdict plus the follow-up inventory check. A +// shorter budget abandoned agents that run exit hooks mid-kill and reported them unverifiable. +export const EXPLICIT_TERMINAL_CLOSE_STOP_TIMEOUT_MS = IMMEDIATE_KILL_REPLY_BUDGET_MS + 2_000 export const CLAUDE_AGENT_PROMPT_RENDER_TIMEOUT_MS = 8000 diff --git a/src/main/runtime/terminal-close-slow-agent-exit.test.ts b/src/main/runtime/terminal-close-slow-agent-exit.test.ts new file mode 100644 index 000000000000..0158f3927daf --- /dev/null +++ b/src/main/runtime/terminal-close-slow-agent-exit.test.ts @@ -0,0 +1,175 @@ +import { describe, expect, it, vi } from 'vitest' +import { + IMMEDIATE_KILL_CAPTURE_TIMEOUT_MS, + IMMEDIATE_KILL_REPLY_BUDGET_MS +} from '../daemon/immediate-kill-reply-budget' +import { DESCENDANT_SNAPSHOT_TIMEOUT_MS } from '../pty-descendant-termination' +import { startLateExitHarness } from '../ipc/pty/daemon-late-exit-test-fixture' +import { finishPtyShutdown } from '../ipc/pty/provider/liveness' +import { shutdownProviderAndDetectExit } from '../ipc/pty/provider/shutdown-detect' +import type { PtyRuntimeControllerDeps } from '../ipc/pty/runtime/controller-deps' +import { stopAndWaitPtyFromRuntimeController } from '../ipc/pty/runtime/kill' +import type { OrchestrationDb } from './orchestration/db' +import type { WorkerTerminalResourceRow } from './orchestration/worker-terminal-ownership' +import type { OrcaRuntimeService } from './orca-runtime' +import { EXPLICIT_TERMINAL_CLOSE_STOP_TIMEOUT_MS } from './orca-runtime-core' +import { completeWorkerTerminalRelease } from './rpc/methods/orchestration/worker/worker-release-completion' + +// Longer than the old 2s close budget, like an agent running exit hooks after SIGTERM. +const SLOW_EXIT_MS = 2_300 + +type Harness = Awaited> + +async function closeThroughDaemon( + harness: Harness, + stopAndWait: typeof stopAndWaitPtyFromRuntimeController = stopAndWaitPtyFromRuntimeController +) { + const ports = { + runtime: harness.runtime, + getLocalPtyProviderStartupPromise: () => undefined, + shutdownProviderAndDetectExit, + rememberSyntheticKillExit: harness.session.rememberSyntheticKillExit, + sendPtyExitToRenderer: harness.session.sendPtyExitToRenderer, + finishPtyShutdown + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: exact-stop reads only these controller ports and optional store; spawn ports are unused. + const deps = ports as unknown as PtyRuntimeControllerDeps + const fallbackKill = vi.fn(() => true) + harness.runtime.setPtyController({ + write: () => true, + getForegroundProcess: async () => null, + kill: fallbackKill, + stopAndWait: (id, opts) => stopAndWait(deps, id, opts) + }) + const terminal = (await harness.runtime.listTerminals()).terminals.find( + (entry) => entry.ptyId === harness.id + ) + if (!terminal) { + throw new Error('Fixture terminal missing') + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: release reads only these resource columns. + const resource = { + id: 'resource-1', + terminal_handle: terminal.handle, + host_scope: JSON.stringify({ kind: 'local', hostId: 'local' }), + archive_source: 'terminal', + archive_status: 'captured', + ownership_state: 'owned', + release_state: 'requested' + } as WorkerTerminalResourceRow + // The release decision reads only the close result; everything before it is stubbed. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: release calls only these runtime methods on the local terminal path. + const runtime = { + showTerminal: vi.fn(async () => ({ handle: terminal.handle, connected: true })), + getTerminalPaneKey: vi.fn(() => 'tab-worker:leaf-worker'), + getTerminalProcessIncarnation: vi.fn(() => 'pty-worker:incarnation-1'), + getTerminalLivenessVerdict: vi.fn(() => null), + getOrchestrationDispatchAuthority: vi.fn(() => ({ + hostScope: { kind: 'local', hostId: 'local' } + })), + closeTerminal: (handle: string) => harness.runtime.closeTerminal(handle), + notifyMessageArrived: vi.fn() + } as unknown as OrcaRuntimeService + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: release calls only these db methods on the local terminal path. + const db = { + getWorkerDispatch: vi.fn(() => ({ + agent_terminal_handle: terminal.handle, + created_at: '2026-08-16T00:00:00.000Z' + })), + isDispatchProcessCurrent: vi.fn(() => true), + workerTerminalResourceHasIdentityConflict: vi.fn(() => false), + getWorkerTerminalArchive: vi.fn(() => ({ kind: 'transcript_pin' })), + commitWorkerTerminalArchiveForRelease: vi.fn(() => ({ + ...resource, + release_state: 'releasing' + })), + settleWorkerTerminalRelease: vi.fn(() => ({ ...resource, release_state: 'released' })), + markWorkerTerminalReleaseUnknown: vi.fn((_id: string, releaseError: string) => ({ + ...resource, + release_state: 'unknown', + release_error: releaseError + })), + recordWorkerTerminalRecoveryAttempt: vi.fn() + } as unknown as OrchestrationDb + const receipt = await completeWorkerTerminalRelease({ + runtime, + db, + dispatchId: 'ctx-worker', + resource + }) + return { receipt, fallbackKill } +} + +describe('worker release of an agent that is slow to exit after its terminal closes', () => { + it('budgets the close for the daemon immediate-kill verdict, not a fixed 2s', () => { + expect(EXPLICIT_TERMINAL_CLOSE_STOP_TIMEOUT_MS).toBeGreaterThan(IMMEDIATE_KILL_REPLY_BUDGET_MS) + expect(IMMEDIATE_KILL_CAPTURE_TIMEOUT_MS).toBe(DESCENDANT_SNAPSHOT_TIMEOUT_MS) + }) + + it('releases a worker whose process exits after the old 2s budget', async () => { + const harness = await startLateExitHarness() + try { + const subprocess = harness.subprocess + subprocess.forceKill = () => { + setTimeout(() => subprocess._simulateExit(0), SLOW_EXIT_MS) + } + const startedAt = Date.now() + const { receipt, fallbackKill } = await closeThroughDaemon(harness) + expect(Date.now() - startedAt).toBeGreaterThanOrEqual(SLOW_EXIT_MS) + expect(receipt).toMatchObject({ state: 'released', processAction: 'closed_agent_terminal' }) + expect(fallbackKill).not.toHaveBeenCalled() + expect(harness.runtime.captureState().liveness).toBe('exited') + } finally { + await harness.dispose() + } + }, 15_000) + + it('counts a SessionNotFoundError reply for an already-exited process as exited', async () => { + const harness = await startLateExitHarness() + try { + // The exit frame is held back, so main still believes the PTY is live when it closes it. + harness.pauseStream() + harness.subprocess._simulateExit(0) + await vi.waitFor(async () => expect(await harness.adapter.listProcesses()).toEqual([])) + const { receipt, fallbackKill } = await closeThroughDaemon(harness) + expect(receipt).toMatchObject({ state: 'released' }) + expect(fallbackKill).not.toHaveBeenCalled() + harness.resumeStream() + await harness.waitForExit() + expect(harness.runtime.captureState().liveness).toBe('exited') + } finally { + await harness.dispose() + } + }) + + it('keeps a process still running when the kill budget expires release_unknown', async () => { + const harness = await startLateExitHarness() + // Only Date is faked: the daemon socket and its timers stay real. + vi.useFakeTimers({ toFake: ['Date'] }) + try { + // A wedged agent ignores SIGKILL, so only the close deadline can end the kill. + harness.subprocess.forceKill = () => {} + let budgetMs = 0 + const expiringStop: typeof stopAndWaitPtyFromRuntimeController = async (deps, id, opts) => { + const deadlineMs = opts?.deadlineMs ?? 0 + budgetMs = deadlineMs - Date.now() + // Jump the clock to the deadline so the real stop issues its kill RPC with no time left. + vi.setSystemTime(deadlineMs) + return stopAndWaitPtyFromRuntimeController(deps, id, opts) + } + const { receipt, fallbackKill } = await closeThroughDaemon(harness, expiringStop) + expect(budgetMs).toBeGreaterThan(IMMEDIATE_KILL_REPLY_BUDGET_MS) + expect(receipt).toMatchObject({ + state: 'release_unknown', + processAction: 'closed_agent_terminal', + lastError: expect.stringContaining('could not be confirmed stopped') + }) + expect(fallbackKill).toHaveBeenCalledTimes(1) + expect(harness.runtime.captureState().liveness).toBe('unverifiable') + } finally { + vi.useRealTimers() + harness.subprocess._simulateExit(0) + await harness.dispose() + } + }) +})