From b5e0fc1a57d6bc61dc160f4b0f82b0118e6276f2 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 22 Sep 2026 20:37:39 -0700 Subject: [PATCH 1/3] fix(runtime): budget explicit terminal close for the daemon's immediate-kill verdict Explicit terminal close (worker-release, worker-stop, `orca terminal close`) gave the daemon kill RPC a fixed 2s deadline. The daemon's immediate kill captures descendants, SIGTERMs them with a 2.5s verification window, then waits up to 8s for the root's physical exit. An agent that runs exit hooks after SIGTERM (Muse: ~3s) outlived main's 2s timeout, so close reported the PTY unverifiable and worker-release returned release_unknown even though the daemon confirmed the exit ~200ms later. Derive the close budget from the daemon's own immediate-kill reply budget (now in an import-free module) plus 2s for the post-kill inventory check. A process that exits within the daemon's budget is released; a wedged process or unreachable host still times out as unverifiable. --- .../daemon/immediate-kill-reply-budget.ts | 16 ++ .../daemon/session-termination-controller.ts | 3 +- .../daemon/terminal-descendant-shutdown.ts | 9 +- src/main/runtime/orca-runtime-core.ts | 5 +- .../terminal-close-slow-agent-exit.test.ts | 167 ++++++++++++++++++ 5 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 src/main/daemon/immediate-kill-reply-budget.ts create mode 100644 src/main/runtime/terminal-close-slow-agent-exit.test.ts 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..34da1a4fa05d --- /dev/null +++ b/src/main/daemon/immediate-kill-reply-budget.ts @@ -0,0 +1,16 @@ +// 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 +// 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, 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 + + 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..be9ac61da12a 100644 --- a/src/main/daemon/session-termination-controller.ts +++ b/src/main/daemon/session-termination-controller.ts @@ -2,9 +2,10 @@ 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 } from './immediate-kill-reply-budget' const KILL_TIMEOUT_MS = 5_000 -export const IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS = 8_000 +export { IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS } export const SESSION_FORCE_KILL_RETRY_MS = 250 const SESSION_FORCE_KILL_MAX_ATTEMPTS = 2 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..e7f95b2c9719 --- /dev/null +++ b/src/main/runtime/terminal-close-slow-agent-exit.test.ts @@ -0,0 +1,167 @@ +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?: (id: string, opts?: { deadlineMs?: number }) => Promise +) { + 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: stopAndWait ?? ((id, opts) => stopAndWaitPtyFromRuntimeController(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() + try { + let budgetMs = 0 + // Stands in for a kill RPC that runs out its deadline against a wedged process. + const stuckStop = vi.fn(async (_id: string, opts?: { deadlineMs?: number }) => { + budgetMs = (opts?.deadlineMs ?? 0) - Date.now() + return false + }) + const { receipt, fallbackKill } = await closeThroughDaemon(harness, stuckStop) + 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 { + await harness.dispose() + } + }) +}) From f52cf0910c355a345a85c239aa8aa9fc99f3b26c Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 22 Sep 2026 21:27:05 -0700 Subject: [PATCH 2/3] fix(runtime): budget the force-kill retry and exercise an expired close deadline --- .../daemon/immediate-kill-reply-budget.ts | 5 +++- .../daemon/session-termination-controller.ts | 8 ++++-- .../terminal-close-slow-agent-exit.test.ts | 28 +++++++++++++------ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/main/daemon/immediate-kill-reply-budget.ts b/src/main/daemon/immediate-kill-reply-budget.ts index 34da1a4fa05d..5c3f2667ccfa 100644 --- a/src/main/daemon/immediate-kill-reply-budget.ts +++ b/src/main/daemon/immediate-kill-reply-budget.ts @@ -4,13 +4,16 @@ 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, then the root's physical-exit wait. */ + * 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 be9ac61da12a..9ccee58af650 100644 --- a/src/main/daemon/session-termination-controller.ts +++ b/src/main/daemon/session-termination-controller.ts @@ -2,11 +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 } from './immediate-kill-reply-budget' +import { + IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS, + SESSION_FORCE_KILL_RETRY_MS +} from './immediate-kill-reply-budget' const KILL_TIMEOUT_MS = 5_000 -export { IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS } -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/runtime/terminal-close-slow-agent-exit.test.ts b/src/main/runtime/terminal-close-slow-agent-exit.test.ts index e7f95b2c9719..e3ab46e1f28d 100644 --- a/src/main/runtime/terminal-close-slow-agent-exit.test.ts +++ b/src/main/runtime/terminal-close-slow-agent-exit.test.ts @@ -22,7 +22,7 @@ type Harness = Awaited> async function closeThroughDaemon( harness: Harness, - stopAndWait?: (id: string, opts?: { deadlineMs?: number }) => Promise + stopAndWait: typeof stopAndWaitPtyFromRuntimeController = stopAndWaitPtyFromRuntimeController ) { const ports = { runtime: harness.runtime, @@ -39,7 +39,7 @@ async function closeThroughDaemon( write: () => true, getForegroundProcess: async () => null, kill: fallbackKill, - stopAndWait: stopAndWait ?? ((id, opts) => stopAndWaitPtyFromRuntimeController(deps, id, opts)) + stopAndWait: (id, opts) => stopAndWait(deps, id, opts) }) const terminal = (await harness.runtime.listTerminals()).terminals.find( (entry) => entry.ptyId === harness.id @@ -144,15 +144,25 @@ describe('worker release of an agent that is slow to exit after its terminal clo 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 - // Stands in for a kill RPC that runs out its deadline against a wedged process. - const stuckStop = vi.fn(async (_id: string, opts?: { deadlineMs?: number }) => { - budgetMs = (opts?.deadlineMs ?? 0) - Date.now() - return false - }) - const { receipt, fallbackKill } = await closeThroughDaemon(harness, stuckStop) + let expiredBy = -1 + 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) + const stopped = await stopAndWaitPtyFromRuntimeController(deps, id, opts) + expiredBy = Date.now() - deadlineMs + return stopped + } + const { receipt, fallbackKill } = await closeThroughDaemon(harness, expiringStop) expect(budgetMs).toBeGreaterThan(IMMEDIATE_KILL_REPLY_BUDGET_MS) + expect(expiredBy).toBeGreaterThanOrEqual(0) expect(receipt).toMatchObject({ state: 'release_unknown', processAction: 'closed_agent_terminal', @@ -161,6 +171,8 @@ describe('worker release of an agent that is slow to exit after its terminal clo expect(fallbackKill).toHaveBeenCalledTimes(1) expect(harness.runtime.captureState().liveness).toBe('unverifiable') } finally { + vi.useRealTimers() + harness.subprocess._simulateExit(0) await harness.dispose() } }) From b5559168dc7e228ef3c7df54fab1940293677d9f Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 22 Sep 2026 22:06:45 -0700 Subject: [PATCH 3/3] test(runtime): drop tautological deadline-expiry assertion --- src/main/runtime/terminal-close-slow-agent-exit.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/runtime/terminal-close-slow-agent-exit.test.ts b/src/main/runtime/terminal-close-slow-agent-exit.test.ts index e3ab46e1f28d..0158f3927daf 100644 --- a/src/main/runtime/terminal-close-slow-agent-exit.test.ts +++ b/src/main/runtime/terminal-close-slow-agent-exit.test.ts @@ -150,19 +150,15 @@ describe('worker release of an agent that is slow to exit after its terminal clo // A wedged agent ignores SIGKILL, so only the close deadline can end the kill. harness.subprocess.forceKill = () => {} let budgetMs = 0 - let expiredBy = -1 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) - const stopped = await stopAndWaitPtyFromRuntimeController(deps, id, opts) - expiredBy = Date.now() - deadlineMs - return stopped + return stopAndWaitPtyFromRuntimeController(deps, id, opts) } const { receipt, fallbackKill } = await closeThroughDaemon(harness, expiringStop) expect(budgetMs).toBeGreaterThan(IMMEDIATE_KILL_REPLY_BUDGET_MS) - expect(expiredBy).toBeGreaterThanOrEqual(0) expect(receipt).toMatchObject({ state: 'release_unknown', processAction: 'closed_agent_terminal',