diff --git a/config/scripts/ci-shard-timings.json b/config/scripts/ci-shard-timings.json index 2ca7094bdd24..1a187718dd23 100644 --- a/config/scripts/ci-shard-timings.json +++ b/config/scripts/ci-shard-timings.json @@ -3241,7 +3241,6 @@ "src/main/runtime/agent-session-launch-env-backfill.test.ts": 264, "src/main/runtime/agent-session-lease-renewal.test.ts": 242, "src/main/runtime/agent-session-operation-admission.test.ts": 165, - "src/main/runtime/agent-session-orphan-child-reaper.test.ts": 90, "src/main/runtime/agent-session-process-identity-probe-windows-batch.test.ts": 47, "src/main/runtime/agent-session-process-identity-probe.test.ts": 78, "src/main/runtime/agent-session-provider-handle-transition.test.ts": 104, diff --git a/src/main/codex/codex-app-server-connection.ts b/src/main/codex/codex-app-server-connection.ts index c0ac7f07400a..b0cf133e1e2a 100644 --- a/src/main/codex/codex-app-server-connection.ts +++ b/src/main/codex/codex-app-server-connection.ts @@ -5,7 +5,6 @@ import { buildCodexAppServerExitError } from './codex-app-server-exit-error' import { initializeCodexAppServerConnection } from './codex-app-server-handshake' import { CodexAppServerHandshakeExitUnprovenError } from './codex-app-server-handshake-exit-proof' import { terminateCodexAppServerProcessTree } from './codex-app-server-process-teardown' -import { CODEX_SPAWN_TOKEN_ENV } from './codex-structured-owner-identity' import { waitForProcessExitUntil } from './codex-process-exit-deadline' import { CodexAppServerTimeoutError, @@ -64,12 +63,11 @@ export async function openCodexAppServerConnection( } const spawnSpec = createProviderSpawnSpec(launch, childEnv, process.platform) const child = spawnImpl(spawnSpec) - const spawnToken = launch.env?.[CODEX_SPAWN_TOKEN_ENV] function terminateProcessTree(): Promise { // The supervisor and provider own separate POSIX groups so the supervisor can prove the // provider group empty before relaying its exit. Forced wrapper teardown uses descendant proof. - return terminateCodexAppServerProcessTree(child, spawnToken) + return terminateCodexAppServerProcessTree(child) } let stderrTail = '' diff --git a/src/main/codex/codex-app-server-process-teardown.test.ts b/src/main/codex/codex-app-server-process-teardown.test.ts index cec8f91d0815..295d780718d0 100644 --- a/src/main/codex/codex-app-server-process-teardown.test.ts +++ b/src/main/codex/codex-app-server-process-teardown.test.ts @@ -26,7 +26,7 @@ describe('terminateCodexAppServerProcessTree', () => { const release = Promise.withResolvers() const terminateWindowsTree = vi.fn(() => release.promise) - const teardown = terminateCodexAppServerProcessTree(target, undefined, { + const teardown = terminateCodexAppServerProcessTree(target, { platform: 'win32', terminateWindowsTree }) @@ -38,52 +38,12 @@ describe('terminateCodexAppServerProcessTree', () => { expect(target.kill).toHaveBeenCalledWith('SIGKILL') }) - it('kills exact Linux spawn-token PIDs before the recorded wrapper', async () => { - const target = child() - const findSpawnTokenProcesses = vi - .fn<() => Promise>() - .mockResolvedValueOnce([1234, 2345, 3456]) - .mockResolvedValueOnce([1234]) - .mockResolvedValueOnce([1234]) - const signalPid = vi.fn() - - await expect( - terminateCodexAppServerProcessTree(target, 'spawn-1', { - platform: 'linux', - findSpawnTokenProcesses, - signalPid, - isPidPresent: () => false, - wait: async () => undefined - }) - ).resolves.toBe(true) - - expect(signalPid.mock.calls).toEqual([ - [2345, 'SIGKILL'], - [3456, 'SIGKILL'] - ]) - expect(target.kill).toHaveBeenCalledTimes(1) - expect(target.kill).toHaveBeenCalledWith('SIGKILL') - }) - - it('keeps the wrapper reachable when Linux cannot prove descendant exit', async () => { - const target = child() - - await expect( - terminateCodexAppServerProcessTree(target, 'spawn-1', { - platform: 'linux', - findSpawnTokenProcesses: async () => null - }) - ).resolves.toBe(false) - - expect(target.kill).not.toHaveBeenCalled() - }) - it('waits for an owned POSIX snapshot before killing the wrapper', async () => { const target = child() const snapshot = { rootPgid: 1234, descendants: [], capturedAtMs: 1 } const release = Promise.withResolvers() - const teardown = terminateCodexAppServerProcessTree(target, undefined, { + const teardown = terminateCodexAppServerProcessTree(target, { platform: 'darwin', captureDescendants: async () => snapshot, terminateDescendants: () => release.promise @@ -102,7 +62,7 @@ describe('terminateCodexAppServerProcessTree', () => { const signalProcessGroup = vi.fn() await expect( - terminateCodexAppServerProcessTree(target, undefined, { + terminateCodexAppServerProcessTree(target, { platform: 'darwin', dedicatedProcessGroup: true, captureDescendants, @@ -119,7 +79,7 @@ describe('terminateCodexAppServerProcessTree', () => { const target = child() await expect( - terminateCodexAppServerProcessTree(target, undefined, { + terminateCodexAppServerProcessTree(target, { platform: 'linux', dedicatedProcessGroup: true, signalProcessGroup: () => { @@ -142,7 +102,7 @@ describe('terminateCodexAppServerProcessTree', () => { const target = { pid: UNREACHABLE_PGID, kill: vi.fn(() => true) as ChildProcess['kill'] } await expect( - terminateCodexAppServerProcessTree(target, undefined, { + terminateCodexAppServerProcessTree(target, { platform: 'darwin', captureDescendants: async () => ({ rootPgid: UNREACHABLE_PGID, @@ -162,7 +122,7 @@ describe('terminateCodexAppServerProcessTree', () => { const signalProcessGroup = vi.fn() await expect( - terminateCodexAppServerProcessTree(target, undefined, { + terminateCodexAppServerProcessTree(target, { platform: 'darwin', captureDescendants: async () => ({ rootPgid: 1234, descendants: [], capturedAtMs: 1 }), terminateDescendants: async () => true, @@ -191,7 +151,7 @@ describe('terminateCodexAppServerProcessTree', () => { const results = await Promise.all( targets.map((target) => - terminateCodexAppServerProcessTree(target, undefined, { + terminateCodexAppServerProcessTree(target, { platform: 'linux', dedicatedProcessGroup: true, captureDescendants, diff --git a/src/main/codex/codex-app-server-process-teardown.ts b/src/main/codex/codex-app-server-process-teardown.ts index 5a9c6e3574b4..10e625bef7e8 100644 --- a/src/main/codex/codex-app-server-process-teardown.ts +++ b/src/main/codex/codex-app-server-process-teardown.ts @@ -2,11 +2,8 @@ import type { ChildProcessHandle } from '../../shared/child-process/run-process' import { captureDescendantSnapshot, type DescendantSnapshot } from '../pty-descendant-termination' import { terminateDescendantSnapshotAndWait } from '../pty-descendant-exit-verification' import { terminateWindowsProcessTree } from '../windows-process-tree-kill' -import { findAgentSessionSpawnTokenProcesses } from '../runtime/agent-session-spawn-token-process-scan' import { recordSelfInitiatedTreeKill } from '../crash-reporting/self-initiated-tree-kill-log' -const TOKEN_PROCESS_EXIT_TIMEOUT_MS = 3_500 -const TOKEN_PROCESS_POLL_MS = 25 const activeTeardowns = new WeakMap>() type TeardownChild = Pick @@ -14,16 +11,10 @@ type TeardownChild = Pick export type CodexAppServerProcessTeardownDeps = { platform?: NodeJS.Platform dedicatedProcessGroup?: boolean - /** Diagnostic/recovery injection only; never used by the primary teardown. */ - findSpawnTokenProcesses?: (spawnToken: string) => Promise captureDescendants?: (rootPid: number) => Promise terminateDescendants?: (snapshot: DescendantSnapshot) => Promise terminateWindowsTree?: (rootPid: number, deps?: { site?: string }) => Promise - signalPid?: (pid: number, signal: NodeJS.Signals) => void signalProcessGroup?: (pgid: number, signal: NodeJS.Signals) => void - isPidPresent?: (pid: number) => boolean - wait?: (ms: number) => Promise - now?: () => number } function terminateDedicatedPosixGroup( @@ -47,70 +38,11 @@ function terminateDedicatedPosixGroup( return true } -function sendSignal(pid: number, signal: NodeJS.Signals): void { - try { - process.kill(pid, signal) - } catch { - // An already-gone exact PID is the desired outcome. - } -} - -function isPidPresent(pid: number): boolean { - try { - process.kill(pid, 0) - return true - } catch (error) { - return (error as NodeJS.ErrnoException).code !== 'ESRCH' - } -} - -async function diagnosticTokenFallback( - rootPid: number, - spawnToken: string, - deps: CodexAppServerProcessTeardownDeps -): Promise { - const find = deps.findSpawnTokenProcesses ?? findAgentSessionSpawnTokenProcesses - const signal = deps.signalPid ?? sendSignal - const pidPresent = deps.isPidPresent ?? isPidPresent - const delay = - deps.wait ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms))) - const now = deps.now ?? Date.now - const deadline = now() + TOKEN_PROCESS_EXIT_TIMEOUT_MS - const signalled = new Set() - while (now() < deadline) { - const pids = await find(spawnToken).catch(() => null) - if (pids === null) { - return false - } - for (const pid of pids.filter((candidate) => candidate !== rootPid)) { - signalled.add(pid) - signal(pid, 'SIGKILL') - } - if ([...signalled].every((pid) => !pidPresent(pid))) { - return true - } - await delay(TOKEN_PROCESS_POLL_MS) - } - return false -} - async function terminatePosixTree( child: TeardownChild, rootPid: number, - _spawnToken: string | undefined, deps: CodexAppServerProcessTeardownDeps ): Promise { - // Kept only for explicit recovery callers/tests. Production always follows - // the dedicated process-group path below; token enumeration is evidence, - // never the owner of orphan-reaping decisions. - if (_spawnToken && deps.findSpawnTokenProcesses) { - const reaped = await diagnosticTokenFallback(rootPid, _spawnToken, deps) - if (reaped) { - child.kill('SIGKILL') - return true - } - return false - } child.kill('SIGSTOP') const capture = deps.captureDescendants ?? captureDescendantSnapshot const snapshot = await capture(rootPid).catch(() => null) @@ -158,7 +90,6 @@ async function terminatePosixTree( /** Stops every process owned by one app-server launch before releasing its wrapper. */ async function terminateOnce( child: TeardownChild, - spawnToken: string | undefined, deps: CodexAppServerProcessTeardownDeps ): Promise { const rootPid = child.pid @@ -176,12 +107,11 @@ async function terminateOnce( if (deps.dedicatedProcessGroup) { return terminateDedicatedPosixGroup(rootPid, deps) } - return terminatePosixTree(child, rootPid, spawnToken, deps) + return terminatePosixTree(child, rootPid, deps) } export function terminateCodexAppServerProcessTree( child: TeardownChild, - spawnToken?: string, deps: CodexAppServerProcessTeardownDeps = {} ): Promise { const key = child as object @@ -189,7 +119,7 @@ export function terminateCodexAppServerProcessTree( if (active) { return active } - const attempt = terminateOnce(child, spawnToken, deps).catch(() => false) + const attempt = terminateOnce(child, deps).catch(() => false) activeTeardowns.set(key, attempt) void attempt.then(() => { if (activeTeardowns.get(key) === attempt) { diff --git a/src/main/runtime/agent-session-orphan-child-reaper.test.ts b/src/main/runtime/agent-session-orphan-child-reaper.test.ts deleted file mode 100644 index eb837ac4a223..000000000000 --- a/src/main/runtime/agent-session-orphan-child-reaper.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -// A child spawned under a reservation whose record was lost is invisible to the lease. Reaping -// is bounded cleanup only; it never replaces the lease proof required to grant another writer. - -import { describe, expect, it, vi } from 'vitest' -import type { AgentSessionRecordStore } from './agent-session-record-store' -import { stopOrphanAgentSessionChildren } from './agent-session-orphan-child-reaper' - -function storeWithLeasedTokens(tokens: readonly string[]) { - return { - listOrphanSpawnTokens: (observed: readonly string[]) => - observed.filter((token) => !tokens.includes(token)) - } as Pick -} - -describe('orphan agent-session child reaper', () => { - it('stops every process whose spawn token no lease claims', async () => { - const stop = vi.fn() - - const stopped = await stopOrphanAgentSessionChildren({ - store: storeWithLeasedTokens(['token-owned']), - scan: async () => - new Map([ - ['token-owned', [101]], - ['token-lost', [202, 203]] - ]), - stop - }) - - expect(stopped).toEqual([202, 203]) - expect(stop).toHaveBeenCalledWith(202, 'SIGTERM') - expect(stop).toHaveBeenCalledWith(203, 'SIGTERM') - expect(stop).not.toHaveBeenCalledWith(101, 'SIGTERM') - }) - - it('stops nothing on a host that cannot enumerate spawn tokens', async () => { - const stop = vi.fn() - - // Null is "cannot answer", never "no tokens" — treating it as an empty scan would be a - // license to signal nothing, but a future empty-map reading would be a license to signal - // whatever the caller guessed. - const stopped = await stopOrphanAgentSessionChildren({ - store: { - listOrphanSpawnTokens: () => { - throw new Error('the reaper must not ask when the host could not answer') - } - }, - scan: async () => null, - stop - }) - - expect(stopped).toEqual([]) - expect(stop).not.toHaveBeenCalled() - }) - - it('surfaces a failure to signal an observed orphan', async () => { - const failure = Object.assign(new Error('not permitted'), { code: 'EPERM' }) - - await expect( - stopOrphanAgentSessionChildren({ - store: storeWithLeasedTokens([]), - scan: async () => new Map([['token-lost', [202]]]), - stop: () => { - throw failure - } - }) - ).rejects.toBe(failure) - }) -}) diff --git a/src/main/runtime/agent-session-orphan-child-reaper.ts b/src/main/runtime/agent-session-orphan-child-reaper.ts deleted file mode 100644 index 68d9c8990240..000000000000 --- a/src/main/runtime/agent-session-orphan-child-reaper.ts +++ /dev/null @@ -1,48 +0,0 @@ -/** - * Best-effort stop for provider children that carry an Orca spawn token no lease claims. - * - * A child spawned under a reservation whose record was lost — the primary store file went with it, - * or the crash beat the durable write — is unreachable but still connected to the provider session. - * Only a token match justifies signalling a process; neither age nor CPU is evidence, and a host - * that cannot enumerate tokens stops nothing. This never proves process exit or licenses a new - * owner; lease adjudication remains the single-writer boundary. - */ - -import type { AgentSessionRecordStore } from './agent-session-record-store' -import { - scanAgentSessionSpawnTokenProcesses, - type AgentSessionSpawnTokenScan -} from './agent-session-spawn-token-process-scan' - -export type AgentSessionOrphanStopSignal = 'SIGTERM' | 'SIGKILL' - -function defaultStop(pid: number, signal: AgentSessionOrphanStopSignal): void { - try { - process.kill(pid, signal) - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ESRCH') { - throw error - } - } -} - -/** Returns the pids signalled, so the caller can report what it reaped. */ -export async function stopOrphanAgentSessionChildren(input: { - store: Pick - scan?: () => Promise - stop?: (pid: number, signal: AgentSessionOrphanStopSignal) => void -}): Promise { - const observed = await (input.scan ?? scanAgentSessionSpawnTokenProcesses)() - if (observed === null || observed.size === 0) { - return [] - } - const stop = input.stop ?? defaultStop - const stopped: number[] = [] - for (const token of input.store.listOrphanSpawnTokens([...observed.keys()])) { - for (const pid of observed.get(token) ?? []) { - stop(pid, 'SIGTERM') - stopped.push(pid) - } - } - return stopped -} diff --git a/src/main/runtime/agent-session-record-store.test.ts b/src/main/runtime/agent-session-record-store.test.ts index ce38e43d0ea3..5ee25d36813d 100644 --- a/src/main/runtime/agent-session-record-store.test.ts +++ b/src/main/runtime/agent-session-record-store.test.ts @@ -753,13 +753,7 @@ describe('host and workspace isolation', () => { }) }) -describe('orphans, claim keys, checkpoints, and unreadable rows', () => { - it('calls a spawn token with no lease an orphan', async () => { - const store = await open() - await establishOwner(store) - expect(store.listOrphanSpawnTokens(['spawn-a', 'spawn-z'])).toEqual(['spawn-z']) - }) - +describe('claim keys, checkpoints, and unreadable rows', () => { it('keeps a retired claim key verifiable for the retention window', async () => { const store = await open() await store.retireClaimKey('key-1', NOW) diff --git a/src/main/runtime/agent-session-record-store.ts b/src/main/runtime/agent-session-record-store.ts index b2bf36323015..924f879c1d85 100644 --- a/src/main/runtime/agent-session-record-store.ts +++ b/src/main/runtime/agent-session-record-store.ts @@ -24,7 +24,6 @@ import { retireAgentSessionClaimKey } from './agent-session-claim-key-retention' import type { AgentSessionOwnerProbe } from '../../shared/agent-session-lease-adjudication' -import { classifyObservedAgentSessionSpawnToken } from '../../shared/agent-session-lease-adjudication' import type { AgentSessionProviderHandleLink } from '../../shared/agent-session-provider-handle' import { agentSessionScopeKey, @@ -179,14 +178,6 @@ export class AgentSessionRecordStore { isClaimKeyVerifiable = (keyId: string, now: number): boolean => isAgentSessionClaimKeyVerifiable(this.state, keyId, now) - /** Spawn tokens observed on the host with no matching lease. Stop them; never adopt them. */ - listOrphanSpawnTokens(observedTokens: readonly string[]): string[] { - const leases = this.listRecords().map((record) => record.lease) - return observedTokens.filter( - (spawnToken) => classifyObservedAgentSessionSpawnToken({ spawnToken, leases }) === 'orphan' - ) - } - async reserveOwner(request: AgentSessionReserveRequest): Promise { return this.transact(() => commitAgentSessionReservation(this.state, request, AGENT_SESSION_LEASE_TTL_MS) diff --git a/src/main/runtime/agent-session-spawn-token-process-scan.ts b/src/main/runtime/agent-session-spawn-token-process-scan.ts index 6db1c069110f..55e860f93205 100644 --- a/src/main/runtime/agent-session-spawn-token-process-scan.ts +++ b/src/main/runtime/agent-session-spawn-token-process-scan.ts @@ -56,7 +56,7 @@ export async function scanAgentSessionSpawnTokenProcesses( /** * Diagnostic evidence only. A null result is deliberately typed as * `unverifiable`, not as an empty process set; callers must never use this - * Linux read-back as ownership or orphan-reaping proof. + * Linux read-back as ownership proof. */ export async function scanAgentSessionSpawnTokenEvidence( platform: NodeJS.Platform = process.platform, diff --git a/src/main/runtime/structured-agent-session-runtime.test.ts b/src/main/runtime/structured-agent-session-runtime.test.ts index 7749fc3c25b0..9f4f4e2474de 100644 --- a/src/main/runtime/structured-agent-session-runtime.test.ts +++ b/src/main/runtime/structured-agent-session-runtime.test.ts @@ -205,36 +205,6 @@ describe('structured agent-session runtime install', () => { vi.restoreAllMocks() }) - it('starts orphan reaping and reports failures without failing installation', async () => { - stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-')) - const failure = new Error('scan failed') - const reapOrphanChildren = vi.fn(async () => { - throw failure - }) - const onError = vi.fn() - - await expect( - ensureStructuredAgentSessionHost({ - stateDirectory, - hostId: HOST_ID, - claimKeyId: 'key-1', - resolveWorkspacePath: async () => stateDirectory!, - resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }), - resolveEnvironment: async () => ({}), - reapOrphanChildren, - onError - }) - ).resolves.toBeDefined() - - await vi.waitFor(() => - expect(onError).toHaveBeenCalledWith({ - scope: 'agent-session-orphan-child-reaper', - error: failure - }) - ) - expect(reapOrphanChildren).toHaveBeenCalledWith({ store: expect.anything() }) - }) - it('holds stop until the model catalog has written its coalesced save', async () => { stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-')) await ensureStructuredAgentSessionHost({ @@ -243,8 +213,7 @@ describe('structured agent-session runtime install', () => { claimKeyId: 'key-1', resolveWorkspacePath: async () => stateDirectory!, resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }), - resolveEnvironment: async () => ({}), - reapOrphanChildren: async () => [] + resolveEnvironment: async () => ({}) }) let finishWrite = (): void => {} vi.spyOn(agentModelCatalogStore, 'flushPersistence').mockReturnValue( @@ -265,33 +234,6 @@ describe('structured agent-session runtime install', () => { expect(stopped).toBe(true) }) - it('logs an orphan-reaper failure when no reporter is configured', async () => { - stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-')) - const failure = new Error('scan failed') - const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) - - await expect( - ensureStructuredAgentSessionHost({ - stateDirectory, - hostId: HOST_ID, - claimKeyId: 'key-1', - resolveWorkspacePath: async () => stateDirectory!, - resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }), - resolveEnvironment: async () => ({}), - reapOrphanChildren: async () => { - throw failure - } - }) - ).resolves.toBeDefined() - - await vi.waitFor(() => - expect(consoleError).toHaveBeenCalledWith( - '[structured-agent-session] orphan reaper failed', - failure - ) - ) - }) - it('does not infer Windows process identity support from an injected reader', async () => { stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-')) const originalPlatform = process.platform @@ -355,7 +297,6 @@ describe('a teardown that fails is retried by the next stop', () => { claimKeyId: 'key-1', resolveWorkspacePath: async () => directory!, resolveEnvironment: async () => ({}), - reapOrphanChildren: async () => [], resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }) }) diff --git a/src/main/runtime/structured-agent-session-runtime.ts b/src/main/runtime/structured-agent-session-runtime.ts index e6b458eb2602..76fb15fbc9ce 100644 --- a/src/main/runtime/structured-agent-session-runtime.ts +++ b/src/main/runtime/structured-agent-session-runtime.ts @@ -38,7 +38,6 @@ import { } from '../native-chat/claude-structured-managed-account-support' import { AgentSessionRecordStore } from './agent-session-record-store' import { agentSessionStorePath } from './agent-session-record-store-file' -import { stopOrphanAgentSessionChildren } from './agent-session-orphan-child-reaper' import { createStructuredAgentSessionOwnerProbe, createStructuredAgentSessionOwnerProbes @@ -106,7 +105,6 @@ export type StructuredAgentSessionRuntimeDeps = { statusSink?: StructuredAgentSessionHostDeps['statusSink'] /** See `StructuredAgentSessionHostDeps.hasOpenDispatch`. */ hasOpenDispatch?: StructuredAgentSessionHostDeps['hasOpenDispatch'] - reapOrphanChildren?: typeof stopOrphanAgentSessionChildren /** The account home a structured launch would pin right now, for catalog * reads with no session record. Absent disables the catalog surface. */ resolveAgentAccountHome?: RuntimeAgentAccountHomeResolver @@ -199,21 +197,6 @@ async function install(deps: StructuredAgentSessionRuntimeDeps): Promise { - try { - if (deps.onError) { - deps.onError({ scope: 'agent-session-orphan-child-reaper', error }) - } else { - console.error('[structured-agent-session] orphan reaper failed', error) - } - } catch (reportingError) { - console.error( - '[structured-agent-session] orphan reaper error reporting failed', - reportingError - ) - } - }) let host: StructuredAgentSessionHost | null = null const lifecycle = createStructuredAgentSessionLifecycleDelivery({ handle: (event) => host?.handleAdapterEvent(event), diff --git a/src/main/runtime/structured-agent-session-spawn-token-descendants.test.ts b/src/main/runtime/structured-agent-session-spawn-token-descendants.test.ts new file mode 100644 index 000000000000..d0650d94f495 --- /dev/null +++ b/src/main/runtime/structured-agent-session-spawn-token-descendants.test.ts @@ -0,0 +1,191 @@ +// The spawn token is an environment variable, so every descendant of a provider child inherits it: +// an editor the agent opened with `code .`, a tmux server, a detached dev server. Carrying the token +// proves descent, not being the provider child, so the host never signals a process on the token +// alone. The token scan still answers one question soundly: whether a reservation ever spawned. + +import type * as NodeFsPromises from 'node:fs/promises' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { CODEX_SPAWN_TOKEN_ENV } from '../codex/codex-structured-owner-identity' +import { AgentSessionRecordStore } from './agent-session-record-store' +import type { AgentSessionReserveRequest } from './agent-session-reservation-admission' +import { createStructuredAgentSessionOwnerProbe } from './structured-agent-session-owner-probe' +import { + ensureStructuredAgentSessionHost, + stopStructuredAgentSessionRuntime +} from './structured-agent-session-runtime' + +/** pid -> the NUL-separated environment block `/proc//environ` serves. */ +const fakeProc = vi.hoisted(() => ({ environs: new Map() })) + +vi.mock('node:fs/promises', async (importOriginal) => { + const fsp = await importOriginal() + const enoent = (path: string) => Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' }) + return { + ...fsp, + readdir: async (...args: Parameters) => + args[0] === '/proc' ? [...fakeProc.environs.keys()].map(String) : fsp.readdir(...args), + readFile: async (...args: Parameters) => { + const path = String(args[0]) + if (!path.startsWith('/proc/')) { + return fsp.readFile(...args) + } + const environ = fakeProc.environs.get(Number(path.split('/')[2])) + if (environ === undefined || !path.endsWith('/environ')) { + throw enoent(path) + } + return environ + } + } +}) + +const HOST_ID = 'local' +const NOW = 1_800_000_000_000 +const SESSION = 'session-alpha' +const MINTED_TOKEN = 'spawn-minted-by-this-store' + +function environ(token: string | null): string { + return ['PATH=/usr/bin', ...(token ? [`${CODEX_SPAWN_TOKEN_ENV}=${token}`] : [])].join('\0') +} + +function reserveRequest(): AgentSessionReserveRequest { + const operationId = `${NOW}-${'1'.padStart(32, '0')}` + return { + sessionId: SESSION, + location: { + executionHostId: HOST_ID, + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'git-worktree' + }, + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/home/dev/.codex' }, + expectedFence: null, + spawnToken: MINTED_TOKEN, + claimKeyId: 'key-1', + handoffOperationId: null, + probe: { outcome: 'indeterminate', reason: 'no previous owner' }, + operation: { callerKey: 'client-1', operationId, fingerprint: 'fp-1' }, + now: NOW + } +} + +const originalPlatform = process.platform +let stateDirectory: string + +beforeEach(async () => { + stateDirectory = await mkdtemp(join(tmpdir(), 'orca-spawn-token-descendants-')) + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) +}) + +afterEach(async () => { + await stopStructuredAgentSessionRuntime() + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + fakeProc.environs.clear() + vi.restoreAllMocks() + await rm(stateDirectory, { recursive: true, force: true }) +}) + +function openStore(): Promise { + return AgentSessionRecordStore.open({ + directory: join(stateDirectory, 'agent-sessions'), + hostId: HOST_ID + }) +} + +describe('a process that inherited a spawn token', () => { + it('is never signalled when the host installs and reconciles, even when this store minted the token', async () => { + // A chat ran and ended: its root exited and the lease was released, clearing the token. + const seed = await openStore() + const reserved = await seed.reserveOwner(reserveRequest()) + const fence = reserved.record.lease.runtimeFence + await seed.commitProcessIdentity({ + sessionId: SESSION, + fence, + process: { hostId: HOST_ID, pid: 4242, processStartTimeMs: NOW, spawnToken: MINTED_TOKEN }, + now: NOW + }) + await seed.proveOwner({ + sessionId: SESSION, + fence, + link: { + linkId: 'link-1', + handle: { provider: 'codex', threadId: 'thread-1' }, + origin: 'created', + mintedAtFence: fence, + observedAt: NOW + }, + now: NOW + }) + const released = await seed.evictProvenDeadOwner({ + sessionId: SESSION, + expectedFence: fence, + probe: { outcome: 'pid-absent' }, + now: NOW + }) + expect(released.lease).toMatchObject({ claimStatus: 'released', reservedSpawnToken: null }) + + // The editor the agent opened detached into its own group and outlived the chat, still + // carrying the token; another descendant carries a token no store here ever minted. + fakeProc.environs.set(1, environ(null)) + fakeProc.environs.set(5151, environ(MINTED_TOKEN)) + fakeProc.environs.set(6161, environ('spawn-foreign')) + const kill = vi.spyOn(process, 'kill').mockImplementation(() => true) + + const host = await ensureStructuredAgentSessionHost({ + stateDirectory, + hostId: HOST_ID, + claimKeyId: 'key-1', + resolveWorkspacePath: async () => stateDirectory, + resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }), + resolveEnvironment: async () => ({}) + }) + await host.reconcileRestartLeases() + // Install work that is not awaited would have run by now; nothing here waits on a timer. + await new Promise((resolve) => setTimeout(resolve, 50)) + + expect(kill.mock.calls.filter(([, signal]) => signal !== 0)).toEqual([]) + }) +}) + +describe('the reservation owner probe', () => { + async function reconcileReservation(): Promise { + const crashed = await openStore() + await crashed.reserveOwner(reserveRequest()) + // The restart after a crash between reservation and recorded identity. + const store = await openStore() + await store.reconcileOnRestart({ + probe: createStructuredAgentSessionOwnerProbe(HOST_ID), + now: NOW + }) + return store + } + + it('proves a reservation never spawned when the host scan finds no process carrying its token', async () => { + fakeProc.environs.set(1, environ(null)) + fakeProc.environs.set(6161, environ('spawn-foreign')) + + const store = await reconcileReservation() + + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'released', + deathEvidence: { kind: 'pid-absent', detail: 'reservation never spawned' } + }) + }) + + it('claims no absence when the host scan finds a process carrying the token', async () => { + fakeProc.environs.set(5151, environ(MINTED_TOKEN)) + const kill = vi.spyOn(process, 'kill').mockImplementation(() => true) + + const store = await reconcileReservation() + + // Released without evidence, and the process carrying the token is left alone. + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'released', + deathEvidence: null + }) + expect(kill.mock.calls.filter(([, signal]) => signal !== 0)).toEqual([]) + }) +}) diff --git a/src/main/runtime/structured-agent-session-support-probe.test.ts b/src/main/runtime/structured-agent-session-support-probe.test.ts index 07a2b671f0a7..ea0ab3298d9b 100644 --- a/src/main/runtime/structured-agent-session-support-probe.test.ts +++ b/src/main/runtime/structured-agent-session-support-probe.test.ts @@ -22,19 +22,15 @@ function setPlatform(platform: NodeJS.Platform): void { type InstallEffects = { storeOpened: boolean - reaperStarted: boolean } -/** Stands in for `install()` by performing the two effects it performs, so a probe that +/** Stands in for `install()` by performing the effect it performs, so a probe that * reinstalls the host is caught by what the install *does*, not by a call count alone. */ function stubStructuredHostInstall(runtime: OrcaRuntimeService): { effects: InstallEffects ensure: ReturnType } { - const effects: InstallEffects = { - storeOpened: false, - reaperStarted: false - } + const effects: InstallEffects = { storeOpened: false } // `supportsCreate` answers as the real Codex adapter would, so a probe that reinstalls the host // still returns the right answer and fails on the install effects alone. const host = { @@ -44,7 +40,6 @@ function stubStructuredHostInstall(runtime: OrcaRuntimeService): { } const ensure = vi.fn(async () => { effects.storeOpened = true - effects.reaperStarted = true setStructuredAgentSessionHost(host as never) }) vi.spyOn(runtime, 'ensureStructuredAgentSessionHost').mockImplementation(ensure) @@ -94,10 +89,7 @@ async function expectSupportWithoutInstall(input: { expect(answers).toEqual(Array(input.repetitions ?? 1).fill(input.expected)) expect(ensure).not.toHaveBeenCalled() - expect(effects).toEqual({ - storeOpened: false, - reaperStarted: false - }) + expect(effects).toEqual({ storeOpened: false }) expect(getStructuredAgentSessionHost()).toBeNull() } @@ -193,10 +185,7 @@ describe('structured agent-session create-support probe', () => { await runtime.prepareStructuredAgentSessionStartupRestoration() expect(ensure).toHaveBeenCalledTimes(1) - expect(effects).toEqual({ - storeOpened: true, - reaperStarted: true - }) + expect(effects).toEqual({ storeOpened: true }) expect( (getStructuredAgentSessionHost() as unknown as { reconcileRestartLeases: () => void }) .reconcileRestartLeases diff --git a/src/shared/agent-session-lease-adjudication.test.ts b/src/shared/agent-session-lease-adjudication.test.ts index 2216782b5e5c..eab7014b5fa9 100644 --- a/src/shared/agent-session-lease-adjudication.test.ts +++ b/src/shared/agent-session-lease-adjudication.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from 'vitest' import { adjudicateAgentSessionRestart, agentSessionLeaseAdmitsWriter, - classifyObservedAgentSessionSpawnToken, evaluateAgentSessionAcquisition, isProvenAliveProbe, isProvenDeadProbe, @@ -300,7 +299,7 @@ describe('restart reconciliation', () => { ) }) -describe('writer admission and orphan spawn tokens', () => { +describe('writer admission', () => { it('admits a writer only when reconciled, settled, live, and holding a process', () => { expect(agentSessionLeaseAdmitsWriter(lease())).toBe(true) expect(agentSessionLeaseAdmitsWriter(lease({ unreconciled: true }))).toBe(false) @@ -308,18 +307,4 @@ describe('writer admission and orphan spawn tokens', () => { expect(agentSessionLeaseAdmitsWriter(lease({ claimStatus: 'reserved' }))).toBe(false) expect(agentSessionLeaseAdmitsWriter(lease({ ownerProcess: null }))).toBe(false) }) - - it('calls a spawn token with no matching lease an orphan', () => { - const leases = [lease(), lease({ sessionId: 'session-beta-1', reservedSpawnToken: 'spawn-b' })] - expect(classifyObservedAgentSessionSpawnToken({ spawnToken: 'spawn-a', leases })).toBe('owned') - expect(classifyObservedAgentSessionSpawnToken({ spawnToken: 'spawn-b', leases })).toBe('owned') - expect(classifyObservedAgentSessionSpawnToken({ spawnToken: 'spawn-z', leases })).toBe('orphan') - }) - - it('still recognises an owner whose reservation token was cleared after proving', () => { - const proved = lease({ reservedSpawnToken: null }) - expect( - classifyObservedAgentSessionSpawnToken({ spawnToken: 'spawn-a', leases: [proved] }) - ).toBe('owned') - }) }) diff --git a/src/shared/agent-session-lease-adjudication.ts b/src/shared/agent-session-lease-adjudication.ts index ecdb1cecc3a7..99c3265b63b0 100644 --- a/src/shared/agent-session-lease-adjudication.ts +++ b/src/shared/agent-session-lease-adjudication.ts @@ -199,8 +199,8 @@ export function adjudicateAgentSessionRestart(args: { // Why: a child spawned before its identity was recorded lost its stdio with the runtime that // crashed, so nothing can drive it. A token scan that proves no child is the only evidence there // can be; without it the lease is released anyway. A live child still carrying the token is - // not signalled here, and the orphan reaper, which runs once at store open, may have seen this - // lease still claiming it. + // never signalled: the token is inherited by every descendant, so it cannot prove which one is + // the provider child. return { disposition: 'evicted', nextFence: nextAgentSessionFence(lease), @@ -231,19 +231,3 @@ export function adjudicateAgentSessionRestart(args: { probe.outcome === 'indeterminate' ? probe.reason : 'process identity could not be verified' } } - -/** - * A process carrying an Orca spawn token with no matching lease is an orphan: stop it, never - * adopt it. Neither age nor CPU is evidence — only a token match justifies acting on a process. - */ -export function classifyObservedAgentSessionSpawnToken(args: { - spawnToken: string - leases: readonly AgentSessionLease[] -}): 'owned' | 'orphan' { - const owned = args.leases.some( - (lease) => - lease.reservedSpawnToken === args.spawnToken || - lease.ownerProcess?.spawnToken === args.spawnToken - ) - return owned ? 'owned' : 'orphan' -}