From d76ee4ae79fb47208cfc3c38010b49ef8851f28a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:13:08 -0700 Subject: [PATCH 1/5] fix(native-chat): start Windows chats without reading process creation times Native chat on Windows refused to start ("Orca can't run this agent in a chat here") whenever the process-table addon could not report process creation times. Chat never needed them; only the bookkeeping around stopping the agent did. Windows now follows the common pattern: Stop ends the agent's tree with `taskkill /T /F` on the child Orca still holds, and reports the tree gone only when taskkill exits 0. A saved pid is never signalled after a restart. - Remove the Claude and Codex location gate and the Codex launch refusal. - A start time that cannot be read is recorded as unknown instead of refusing the session; recovery already releases such an owner without signalling it. - Delete Claude's Windows creation-time descendant snapshot and its verifier. - Codex's Windows teardown reports the real taskkill outcome. - The renderer no longer waits on the capability flag; the host keeps publishing it for older clients (temporary). --- config/scripts/ci-shard-timings.json | 1 - ...aude-agent-sdk-exit-proof-identity.test.ts | 54 +--- .../claude-agent-sdk-exit-proof.test.ts | 175 ++---------- .../claude/claude-agent-sdk-exit-proof.ts | 129 ++++----- ...laude-agent-sdk-root-kill-fallback.test.ts | 73 +---- .../claude/claude-child-root-termination.ts | 35 --- src/main/claude/claude-child-tree-snapshot.ts | 108 ++----- ...claude-structured-location-support.test.ts | 22 +- .../claude-structured-location-support.ts | 7 +- .../claude-structured-owner-identity.test.ts | 8 + .../claude-structured-owner-identity.ts | 7 +- ...-structured-acquisition-exit-proof.test.ts | 5 +- ...codex-structured-launch-resolution.test.ts | 29 +- .../codex-structured-launch-resolution.ts | 10 - .../codex-structured-location-support.test.ts | 15 +- .../codex-structured-location-support.ts | 13 +- .../codex-structured-owner-identity.test.ts | 11 +- .../codex/codex-structured-owner-identity.ts | 7 +- .../codex/codex-structured-session-adapter.ts | 3 +- .../codex/codex-structured-session-state.ts | 2 - ...-agent-session-recovery-resolution.test.ts | 25 ++ ...tured-agent-session-recovery-resolution.ts | 12 +- .../provider-process-teardown.test.ts | 22 +- .../provider-process-teardown.ts | 7 +- .../windows-foreground-process-rows.ts | 20 -- src/main/runtime/orca-runtime-get-status.ts | 5 +- .../structured-agent-session-runtime.test.ts | 6 +- ...ctured-agent-session-support-probe.test.ts | 18 +- ...ndows-descendant-exit-verification.test.ts | 270 ------------------ .../windows-descendant-exit-verification.ts | 186 ------------ src/main/windows-process-tree-kill.test.ts | 14 +- src/main/windows-process-tree-kill.ts | 13 +- ...windows-terminal-capabilities-race.test.ts | 41 +-- .../lib/windows-terminal-capabilities.test.ts | 9 +- .../src/lib/windows-terminal-capabilities.ts | 2 - .../lib/windows-terminal-capability-read.ts | 3 - ...indows-terminal-capability-reprobe.test.ts | 26 +- .../windows-terminal-capability-reprobe.ts | 8 +- src/shared/runtime-session-contracts.ts | 3 +- 39 files changed, 275 insertions(+), 1129 deletions(-) delete mode 100644 src/main/windows-descendant-exit-verification.test.ts delete mode 100644 src/main/windows-descendant-exit-verification.ts diff --git a/config/scripts/ci-shard-timings.json b/config/scripts/ci-shard-timings.json index 9ba114966a87..0f1442386ee4 100644 --- a/config/scripts/ci-shard-timings.json +++ b/config/scripts/ci-shard-timings.json @@ -4604,7 +4604,6 @@ "src/main/window/terminal-tab-close-request-relay.test.ts": 55, "src/main/window/updater-package-recovery-ipc.test.ts": 4430, "src/main/window/window-close-decision.test.ts": 41, - "src/main/windows-descendant-exit-verification.test.ts": 100, "src/main/windows-live-tree-kill.win32.test.ts": 46, "src/main/windows-process-tree-kill.test.ts": 61, "src/main/windows-pty-root-identity.test.ts": 94, diff --git a/src/main/claude/claude-agent-sdk-exit-proof-identity.test.ts b/src/main/claude/claude-agent-sdk-exit-proof-identity.test.ts index 04d58067353d..24ec5ba9bb0d 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof-identity.test.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof-identity.test.ts @@ -1,9 +1,8 @@ import { describe, expect, it, vi } from 'vitest' import type { DescendantSnapshot } from '../pty-descendant-termination' -import type { WindowsDescendantSnapshot } from '../windows-descendant-exit-verification' import { collectDescendantRows } from '../pty-descendant-termination' import { createClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' -import { mergeClaudeCapturedTrees } from './claude-child-tree-snapshot' +import { mergeClaudeDescendantSnapshots } from './claude-child-tree-snapshot' function posixSnapshot(capturedAtMs: number): DescendantSnapshot { return { @@ -14,43 +13,28 @@ function posixSnapshot(capturedAtMs: number): DescendantSnapshot { } } -function windowsSnapshot(): WindowsDescendantSnapshot { - return { - root: { pid: 100, creationTimeMs: 5 }, - descendants: [{ pid: 200, creationTimeMs: 7 }], - unidentifiedCount: 0, - capturedAtMs: 1 - } -} - describe('Claude child root identity', () => { it('keeps a retained row boundary when a refresh observes no new descendants', () => { const previous = posixSnapshot(1_700_000_000_900) const next = posixSnapshot(1_700_000_002_100) - expect( - mergeClaudeCapturedTrees( - { platform: 'posix', tree: previous }, - { platform: 'posix', tree: next } - ) - ).toEqual({ - platform: 'posix', - tree: { ...next, capturedAtMsByPid: { '200': previous.capturedAtMs } } + expect(mergeClaudeDescendantSnapshots(previous, next)).toEqual({ + ...next, + capturedAtMsByPid: { '200': previous.capturedAtMs } }) }) - it('keeps the descendant verdict when a POSIX root probe is unavailable', async () => { + it('keeps the descendant verdict alongside the POSIX root kill', async () => { const child = { pid: 100, kill: vi.fn(() => true) } const terminateDescendants = vi.fn(async () => 'exited' as const) const tree = createClaudeChildTreeReaper(child, { platform: 'linux', captureDescendants: vi.fn(async () => posixSnapshot(1)), - terminateDescendants, - verifyRootIdentity: vi.fn(async () => false) + terminateDescendants }) - // POSIX runs no bare-pid root operation, so a declined probe withholds - // nothing: the handle kill still lands and the verification still speaks. + // POSIX runs no bare-pid root operation: the handle kill lands and the + // verification still speaks. await expect(tree.reap()).resolves.toBe('exited') expect(terminateDescendants).toHaveBeenCalled() expect(child.kill).toHaveBeenCalledWith('SIGKILL') @@ -72,8 +56,7 @@ describe('Claude child root identity', () => { 1 ) ), - terminateDescendants, - verifyRootIdentity: vi.fn(async () => true) + terminateDescendants }) await expect(tree.reap()).resolves.toBe('unverifiable') @@ -82,23 +65,4 @@ describe('Claude child root identity', () => { expect(terminateDescendants).not.toHaveBeenCalled() expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - - it('fails closed when Windows root identity revalidation is unavailable', async () => { - const child = { pid: 100, kill: vi.fn(() => true) } - const terminateWindowsTree = vi.fn(async () => {}) - const terminateWindowsDescendants = vi.fn(async () => 'exited' as const) - const tree = createClaudeChildTreeReaper(child, { - platform: 'win32', - captureWindowsDescendants: vi.fn(async () => windowsSnapshot()), - terminateWindowsTree, - terminateWindowsDescendants, - verifyRootIdentity: vi.fn(async () => false) - }) - - await expect(tree.reap()).resolves.toBe('unverifiable') - // taskkill /T /F addresses a bare pid and stays gated; the handle does not. - expect(terminateWindowsTree).not.toHaveBeenCalled() - expect(terminateWindowsDescendants).not.toHaveBeenCalled() - expect(child.kill).toHaveBeenCalledWith('SIGKILL') - }) }) diff --git a/src/main/claude/claude-agent-sdk-exit-proof.test.ts b/src/main/claude/claude-agent-sdk-exit-proof.test.ts index 8ce2feb9ad54..d5ea5493f5e0 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.test.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.test.ts @@ -5,9 +5,8 @@ import { describe, expect, it, vi } from 'vitest' import { spawnProcess, type SpawnedProcess } from '../../shared/child-process/run-process' import type { DescendantTreeVerdict } from '../pty-descendant-exit-verification' import type { DescendantSnapshot } from '../pty-descendant-termination' -import type { WindowsDescendantSnapshot } from '../windows-descendant-exit-verification' import { - createClaudeChildTreeReaper as createClaudeChildTreeReaperImpl, + createClaudeChildTreeReaper, proveClaudeChildExit, type ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' @@ -154,15 +153,6 @@ function mockTree(verdicts: DescendantTreeVerdict[]): ClaudeChildTreeReaper & { } } -function windowsSnapshotOf(descendantPid: number): WindowsDescendantSnapshot { - return { - root: { pid: 424242, creationTimeMs: 1_700_000_000_001 }, - descendants: [{ pid: descendantPid, creationTimeMs: 1_700_000_000_000 }], - unidentifiedCount: 0, - capturedAtMs: 1 - } -} - function snapshotOf(descendantPid: number): DescendantSnapshot { return { root: { pid: 424242, startedAt: 'Mon Jan 1 00:00:00 2026' }, @@ -174,18 +164,6 @@ function snapshotOf(descendantPid: number): DescendantSnapshot { } } -// Unit tests use synthetic process ids; production always supplies the fresh -// identity probe, so the harness explicitly models a matching probe. -function createClaudeChildTreeReaper( - child: Parameters[0], - deps: Parameters[1] = {} -): ReturnType { - return createClaudeChildTreeReaperImpl(child, { - verifyRootIdentity: async () => true, - ...deps - }) -} - describe('claude child exit proof', () => { it.runIf(process.platform !== 'win32')( 'reports a proven exit only once a SIGTERM-resistant descendant is gone at the close boundary', @@ -605,35 +583,6 @@ describe('claude child tree reaper', () => { expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('fails closed when a Windows refresh reuses a PID with a new creation time', async () => { - const child = mockChild() - const first = windowsSnapshotOf(4243) - const replacement = { - ...first, - descendants: [{ pid: 4243, creationTimeMs: first.descendants[0].creationTimeMs + 1 }] - } - const captureWindowsDescendants = vi - .fn() - .mockResolvedValueOnce(first) - .mockResolvedValueOnce(replacement) - const terminateWindowsTree = vi.fn(async () => {}) - const terminateWindowsDescendants = vi.fn(async () => 'exited' as const) - const tree = createClaudeChildTreeReaper(child, { - platform: 'win32', - captureWindowsDescendants, - terminateWindowsTree, - terminateWindowsDescendants - }) - - await tree.capture() - await tree.refresh?.() - - await expect(tree.reap()).resolves.toBe('unverifiable') - expect(terminateWindowsTree).not.toHaveBeenCalled() - expect(terminateWindowsDescendants).not.toHaveBeenCalled() - expect(child.kill).toHaveBeenCalledWith('SIGKILL') - }) - it('queues a fresh boundary behind an output-triggered capture already in flight', async () => { const child = mockChild() const firstDone = Promise.withResolvers() @@ -698,39 +647,6 @@ describe('claude child tree reaper', () => { }) }) - it('retains a Windows replacement descendant while preserving unidentified rows', async () => { - const child = mockChild() - const first = windowsSnapshotOf(4243) - const replacement = { - ...windowsSnapshotOf(4244), - unidentifiedCount: 0 - } - const captureWindowsDescendants = vi - .fn() - .mockResolvedValueOnce({ ...first, unidentifiedCount: 1 }) - .mockResolvedValueOnce(replacement) - const terminateWindowsTree = vi.fn(async () => {}) - const terminateWindowsDescendants = vi.fn(async (snapshot: WindowsDescendantSnapshot) => - snapshot.descendants.some((row) => row.pid === 4244) ? ('live' as const) : ('exited' as const) - ) - const tree = createClaudeChildTreeReaper(child, { - platform: 'win32', - captureWindowsDescendants, - terminateWindowsTree, - terminateWindowsDescendants - }) - - await tree.capture() - await tree.refresh?.() - await expect(tree.reap()).resolves.toBe('live') - - expect(terminateWindowsDescendants).toHaveBeenCalledWith({ - ...replacement, - descendants: [...first.descendants, ...replacement.descendants], - unidentifiedCount: 1 - }) - }) - it('retains the prior identity-safe snapshot when a refresh is partial', async () => { const child = mockChild() const first = { @@ -840,104 +756,69 @@ describe('claude child tree reaper', () => { expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('waits for the Windows tree kill before releasing the root', async () => { + it('kills a held Windows root with one taskkill and never snapshots it', async () => { const child = mockChild() - const release = Promise.withResolvers() + const release = Promise.withResolvers() const terminateWindowsTree = vi.fn(() => release.promise) const captureDescendants = vi.fn() - const terminateWindowsDescendants = vi.fn(async () => 'exited' as const) const tree = createClaudeChildTreeReaper(child, { platform: 'win32', captureDescendants, - captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), - terminateWindowsTree, - terminateWindowsDescendants + terminateWindowsTree }) + await tree.capture() + await tree.refresh?.() const reap = tree.reap() - await vi.waitFor(() => - expect(terminateWindowsTree).toHaveBeenCalledWith({ - pid: 424242, - creationTimeMs: 1_700_000_000_001 - }) - ) + await vi.waitFor(() => expect(terminateWindowsTree).toHaveBeenCalledWith(424242)) expect(child.kill).not.toHaveBeenCalled() - expect(terminateWindowsDescendants).not.toHaveBeenCalled() - release.resolve() + release.resolve(true) + // taskkill's clean exit is the one outcome that reports every process in the tree terminated. await expect(reap).resolves.toBe('exited') expect(child.kill).toHaveBeenCalledWith('SIGKILL') - expect(terminateWindowsDescendants).toHaveBeenCalledWith(windowsSnapshotOf(4243)) + // No creation-time snapshot: a host whose table has no creation times reaps the same way. expect(captureDescendants).not.toHaveBeenCalled() }) - it('stays unproven on Windows when taskkill fails and a descendant is still observed', async () => { + it('reports a Windows tree unverifiable when taskkill does not exit cleanly', async () => { const child = mockChild() const tree = createClaudeChildTreeReaper(child, { platform: 'win32', - captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), - terminateWindowsTree: vi.fn(async () => { - throw new Error('taskkill: access denied') - }), - terminateWindowsDescendants: vi.fn(async () => 'live' as const) + terminateWindowsTree: vi.fn(async () => false) }) - // taskkill's own outcome is not the proof; the table read after it is. - await expect(tree.reap()).resolves.toBe('live') - expect(tree.treeVerdict).toBe('live') + await expect(tree.reap()).resolves.toBe('unverifiable') + expect(tree.treeVerdict).toBe('unverifiable') + // The held handle still takes the root down. expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('stays unproven on Windows when taskkill resolves but a descendant survives it', async () => { + it('reports a Windows tree unverifiable when taskkill could not run', async () => { const child = mockChild() - const terminateWindowsTree = vi.fn(async () => {}) const tree = createClaudeChildTreeReaper(child, { platform: 'win32', - captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), - terminateWindowsTree, - terminateWindowsDescendants: vi.fn(async () => 'live' as const) - }) - - await expect(tree.reap()).resolves.toBe('live') - expect(terminateWindowsTree).toHaveBeenCalledTimes(1) - expect(tree.treeVerdict).toBe('live') - }) - - it('never taskkills a Windows root that already exited, but still verifies its snapshot', async () => { - const child = mockChild() - let exited = false - const terminateWindowsTree = vi.fn(async () => {}) - const terminateWindowsDescendants = vi.fn(async () => 'exited' as const) - const tree = createClaudeChildTreeReaper(child, { - platform: 'win32', - exited: () => exited, - captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), - terminateWindowsTree, - terminateWindowsDescendants + terminateWindowsTree: vi.fn(async () => { + throw new Error('taskkill: spawn failed') + }) }) - await tree.capture() - exited = true - await expect(tree.reap()).resolves.toBe('exited') - // A dead root's pid may already belong to a stranger: taskkill /T /F on it - // would take down an unrelated tree. - expect(terminateWindowsTree).not.toHaveBeenCalled() - expect(terminateWindowsDescendants).toHaveBeenCalledWith(windowsSnapshotOf(4243)) + await expect(tree.reap()).resolves.toBe('unverifiable') + expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('treats an unreadable Windows table as unproven', async () => { + it('never taskkills a Windows root that already exited', async () => { const child = mockChild() - const terminateWindowsDescendants = vi.fn() + const terminateWindowsTree = vi.fn(async () => true) const tree = createClaudeChildTreeReaper(child, { platform: 'win32', - captureWindowsDescendants: vi.fn(async () => null), - terminateWindowsTree: vi.fn(async () => {}), - terminateWindowsDescendants + exited: () => true, + terminateWindowsTree }) + // Its pid left with it, so its tree can no longer be addressed. await expect(tree.reap()).resolves.toBe('unverifiable') - expect(terminateWindowsDescendants).not.toHaveBeenCalled() - // A host that cannot supply creation times blocks taskkill, not the root kill. - expect(child.kill).toHaveBeenCalledWith('SIGKILL') + expect(terminateWindowsTree).not.toHaveBeenCalled() + expect(child.kill).not.toHaveBeenCalled() }) it('has nothing to reap for a child that never spawned', async () => { diff --git a/src/main/claude/claude-agent-sdk-exit-proof.ts b/src/main/claude/claude-agent-sdk-exit-proof.ts index 883bf8efd6ac..6b808f050213 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.ts @@ -3,21 +3,10 @@ import { terminateDescendantSnapshotWithVerdict, type DescendantTreeVerdict } from '../pty-descendant-exit-verification' -import { - captureDescendantSnapshot, - type DescendantSnapshot, - type PosixProcessIdentity -} from '../pty-descendant-termination' -import { - captureWindowsDescendantSnapshot, - terminateIdentifiedWindowsProcessTree, - verifyWindowsDescendantSnapshotExit, - verifyWindowsProcessIdentity, - type WindowsDescendantSnapshot, - type WindowsProcessIdentity -} from '../windows-descendant-exit-verification' -import { mergeClaudeCapturedTrees, type ClaudeCapturedTree } from './claude-child-tree-snapshot' -import { terminateClaudeRoot, terminateClaudeWindowsRoot } from './claude-child-root-termination' +import { captureDescendantSnapshot, type DescendantSnapshot } from '../pty-descendant-termination' +import { terminateWindowsProcessTree } from '../windows-process-tree-kill' +import { mergeClaudeDescendantSnapshots } from './claude-child-tree-snapshot' +import { terminateClaudeRoot } from './claude-child-root-termination' import { proveClaudeChildExitWithReaper, type ClaudeChildExitProofInput @@ -37,24 +26,19 @@ const TREE_VERDICT_TRUST: Record = { type ReapableChild = Pick /** - * A walk is only admissible while the root it walked was alive. A POSIX walk - * that found no root says so with a null pgid; either platform's walk can also - * have raced the root's death. Both can only have missed descendants that - * already reparented away, so neither is evidence about the tree. + * A walk is only admissible while the root it walked was alive. A walk that + * found no root says so with a null pgid; it can also have raced the root's + * death. Both can only have missed descendants that already reparented away, + * so neither is evidence about the tree. */ function admissibleTree( - captured: DescendantSnapshot | WindowsDescendantSnapshot | null, - platform: NodeJS.Platform, + captured: DescendantSnapshot | null, exited: boolean -): ClaudeCapturedTree | null { +): DescendantSnapshot | null { if (!captured || exited) { return null } - if (platform === 'win32') { - return { platform: 'win32', tree: captured as WindowsDescendantSnapshot } - } - const tree = captured as DescendantSnapshot - return tree.rootPgid === null ? null : { platform: 'posix', tree } + return captured.rootPgid === null ? null : captured } export type ClaudeChildTreeReaperDeps = { @@ -63,13 +47,8 @@ export type ClaudeChildTreeReaperDeps = { exited?: () => boolean captureDescendants?: (rootPid: number) => Promise terminateDescendants?: (snapshot: DescendantSnapshot) => Promise - terminateWindowsTree?: (root: WindowsProcessIdentity) => Promise - captureWindowsDescendants?: (rootPid: number) => Promise - terminateWindowsDescendants?: ( - snapshot: WindowsDescendantSnapshot - ) => Promise - /** Identity probe for the bare-pid tree kill; only Windows has one to gate. */ - verifyRootIdentity?: (root: PosixProcessIdentity | WindowsProcessIdentity) => Promise + /** `taskkill /T /F` on the held root; true only when taskkill reports the tree terminated. */ + terminateWindowsTree?: (rootPid: number) => Promise } export type ClaudeChildTreeReaper = { @@ -110,24 +89,23 @@ export function createClaudeChildTreeReaper( ): ClaudeChildTreeReaper { const platform = deps.platform ?? process.platform const exited = deps.exited ?? (() => false) + // Windows reaches the tree through the held root instead, so it never snapshots. + const snapshots = platform !== 'win32' + const capture = deps.captureDescendants ?? captureDescendantSnapshot // Undefined until captured; null when no admissible snapshot exists — the root // was already gone, or the table could not be read while it was alive — which // no later read can make up for. - let snapshot: ClaudeCapturedTree | null | undefined + let snapshot: DescendantSnapshot | null | undefined let capturing: Promise | null = null let refreshing: Promise | null = null let queuedRefresh: Promise | null = null let inFlight: Promise | null = null let treeVerdict: DescendantTreeVerdict = 'unverifiable' - // Consulted only on win32: POSIX signals descendants by revalidated identity - // and reaches the root solely through Node's handle, so neither needs a probe. - const verifyRoot = - deps.verifyRootIdentity ?? - ((root: PosixProcessIdentity | WindowsProcessIdentity) => - verifyWindowsProcessIdentity(root as WindowsProcessIdentity)) - function captureOnce(): Promise { + if (!snapshots) { + return Promise.resolve() + } if (refreshing) { const pending = refreshing return pending.then(() => queuedRefresh ?? undefined) @@ -146,10 +124,6 @@ export function createClaudeChildTreeReaper( snapshot = exited() ? null : snapshot return Promise.resolve() } - const capture = - platform === 'win32' - ? (deps.captureWindowsDescendants ?? captureWindowsDescendantSnapshot) - : (deps.captureDescendants ?? captureDescendantSnapshot) capturing = capture(rootPid) .catch(() => null) .then((captured) => { @@ -159,7 +133,7 @@ export function createClaudeChildTreeReaper( // still lives the walk is simply retried, rather than latching a failed // read as proof that there was nothing to find. const rootExited = exited() - const tree = admissibleTree(captured, platform, rootExited) + const tree = admissibleTree(captured, rootExited) if (tree) { snapshot = tree } else if (rootExited) { @@ -186,16 +160,12 @@ export function createClaudeChildTreeReaper( if (!rootPid) { return Promise.resolve() } - const capture = - platform === 'win32' - ? (deps.captureWindowsDescendants ?? captureWindowsDescendantSnapshot) - : (deps.captureDescendants ?? captureDescendantSnapshot) const operation = (async () => { const captured = await capture(rootPid).catch(() => null) if (exited()) { return } - const tree = admissibleTree(captured, platform, false) + const tree = admissibleTree(captured, false) if (!tree) { return } @@ -208,7 +178,7 @@ export function createClaudeChildTreeReaper( // recycle/replace decision, not an absent descendant, so no row here may // be signalled from its number. Only the descendant evidence is lost — // the root still leaves through the handle no recycled pid can reach. - snapshot = mergeClaudeCapturedTrees(snapshot, tree) + snapshot = mergeClaudeDescendantSnapshots(snapshot, tree) } // Keep an earlier admissible snapshot when this close-boundary read fails; // it remains the only identity-safe evidence after root exit. @@ -244,6 +214,9 @@ export function createClaudeChildTreeReaper( } async function refresh(): Promise { + if (!snapshots) { + return + } const pending = capturing ?? refreshing if (pending) { await queueRefreshAfter(pending) @@ -260,6 +233,21 @@ export function createClaudeChildTreeReaper( } } + /** The common pattern: one `taskkill /T /F` addressed through the root Orca still holds. */ + async function judgeWindowsTree(rootPid: number): Promise { + if (exited()) { + // The root's pid left with it, so its tree can no longer be addressed. + return 'unverifiable' + } + const terminateTree = + deps.terminateWindowsTree ?? + ((pid: number) => terminateWindowsProcessTree(pid, { site: 'claude-structured-child-close' })) + const terminated = await terminateTree(rootPid).catch(() => false) + terminateClaudeRoot({ child, exited }) + // Only taskkill's clean exit reports the whole tree terminated. + return terminated ? 'exited' : 'unverifiable' + } + /** The only source of a tree verdict: every `exited` here is an observation. */ async function judgeTree(): Promise { const killRoot = (): boolean => terminateClaudeRoot({ child, exited }) @@ -268,36 +256,15 @@ export function createClaudeChildTreeReaper( // Never spawned, so the OS never created a tree to orphan. return 'exited' } - await captureOnce() - if (platform === 'win32') { - // Why taskkill's own outcome is never the verdict: it resolves identically - // on a timeout, an access denial, a recycled root and a real kill. - const { rootVerified } = await terminateClaudeWindowsRoot({ - snapshot: snapshot?.platform === 'win32' ? snapshot.tree : null, - exited, - verifyRoot: (root) => verifyRoot(root), - terminateTree: (root) => - deps.terminateWindowsTree - ? deps.terminateWindowsTree(root) - : terminateIdentifiedWindowsProcessTree(root, { - ownsRoot: () => !exited() - }).then(() => undefined), - killRoot - }) - if (!rootVerified && !exited()) { - return 'unverifiable' - } - return snapshot?.platform === 'win32' - ? await (deps.terminateWindowsDescendants ?? verifyWindowsDescendantSnapshotExit)( - snapshot.tree - ) - : 'unverifiable' + if (!snapshots) { + return judgeWindowsTree(rootPid) } - if (snapshot?.platform !== 'posix') { + await captureOnce() + if (!snapshot) { killRoot() return 'unverifiable' } - if (snapshot.tree.descendants.length === 0) { + if (snapshot.descendants.length === 0) { // Read while the root was alive and childless: a later table read has no // row it could match, so it would add nothing to this observation. killRoot() @@ -312,8 +279,8 @@ export function createClaudeChildTreeReaper( // reaps them. After a root exit the kill is a no-op: Node drops the handle // on exit and never signals a possibly recycled pid. const verdictPromise = deps.terminateDescendants - ? deps.terminateDescendants(snapshot.tree) - : terminateDescendantSnapshotWithVerdict(snapshot.tree, { + ? deps.terminateDescendants(snapshot) + : terminateDescendantSnapshotWithVerdict(snapshot, { requireIdentityBeforeSignal: true }) killRoot() diff --git a/src/main/claude/claude-agent-sdk-root-kill-fallback.test.ts b/src/main/claude/claude-agent-sdk-root-kill-fallback.test.ts index 9f843527e306..bb1f84bf5cd6 100644 --- a/src/main/claude/claude-agent-sdk-root-kill-fallback.test.ts +++ b/src/main/claude/claude-agent-sdk-root-kill-fallback.test.ts @@ -3,9 +3,7 @@ import { PassThrough } from 'node:stream' import { describe, expect, it, vi } from 'vitest' import type { SpawnedProcess } from '../../shared/child-process/run-process' import type { DescendantSnapshot } from '../pty-descendant-termination' -import type { WindowsDescendantSnapshot } from '../windows-descendant-exit-verification' import { createClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' -import { mergeClaudeCapturedTrees } from './claude-child-tree-snapshot' const ROOT_PID = 424242 const ROOT_STARTED_AT = 'Mon Jan 1 00:00:00 2026' @@ -32,15 +30,6 @@ function posixSnapshot(input: { } } -function windowsSnapshot(capturedAtMs = 1): WindowsDescendantSnapshot { - return { - root: { pid: ROOT_PID, creationTimeMs: 1_700_000_000_001 }, - descendants: [{ pid: 4243, creationTimeMs: 1_700_000_000_000 }], - unidentifiedCount: 0, - capturedAtMs - } -} - describe('Claude root kill fallback', () => { it('kills the root when the first capture landed in the fork second', async () => { // The production POSIX verifier declines a root born in its capture second, @@ -78,8 +67,7 @@ describe('Claude root kill fallback', () => { platform: 'linux', exited: () => false, captureDescendants, - terminateDescendants: vi.fn(async () => 'exited' as const), - verifyRootIdentity: vi.fn(async () => true) + terminateDescendants: vi.fn(async () => 'exited' as const) }) await tree.capture() @@ -89,7 +77,7 @@ describe('Claude root kill fallback', () => { expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('keeps an observed live descendant when the root identity probe declined', async () => { + it('keeps an observed live descendant through the root kill', async () => { const child = mockChild() const tree = createClaudeChildTreeReaper(child, { platform: 'linux', @@ -100,8 +88,7 @@ describe('Claude root kill fallback', () => { descendants: [{ pid: 100, ppid: ROOT_PID, pgid: ROOT_PID, startedAt: ROOT_STARTED_AT }] }) ), - terminateDescendants: vi.fn(async () => 'live' as const), - verifyRootIdentity: vi.fn(async () => false) + terminateDescendants: vi.fn(async () => 'live' as const) }) await expect(tree.reap()).resolves.toBe('live') @@ -109,22 +96,6 @@ describe('Claude root kill fallback', () => { expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('reports a Windows taskkill that worked as exited, not unverifiable', async () => { - const child = mockChild() - // Probe 1 gates taskkill; a later probe correctly finds the root already dead. - const verifyRootIdentity = vi.fn().mockResolvedValueOnce(true).mockResolvedValue(false) - const tree = createClaudeChildTreeReaper(child, { - platform: 'win32', - exited: () => false, - captureWindowsDescendants: vi.fn(async () => windowsSnapshot()), - terminateWindowsTree: vi.fn(async () => {}), - terminateWindowsDescendants: vi.fn(async () => 'exited' as const), - verifyRootIdentity - }) - - await expect(tree.reap()).resolves.toBe('exited') - }) - it('kills the root when no POSIX snapshot could be read', async () => { const child = mockChild() const tree = createClaudeChildTreeReaper(child, { @@ -138,23 +109,6 @@ describe('Claude root kill fallback', () => { expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('kills the root when the Windows process table is unreadable', async () => { - const child = mockChild() - const terminateWindowsTree = vi.fn(async () => {}) - const tree = createClaudeChildTreeReaper(child, { - platform: 'win32', - exited: () => false, - captureWindowsDescendants: vi.fn(async () => null), - terminateWindowsTree, - terminateWindowsDescendants: vi.fn() - }) - - await expect(tree.reap()).resolves.toBe('unverifiable') - // No identity means no bare-pid tree kill, but the owned handle is still ours. - expect(terminateWindowsTree).not.toHaveBeenCalled() - expect(child.kill).toHaveBeenCalledWith('SIGKILL') - }) - it('never signals a root the reaper already saw exit', async () => { const child = mockChild() const tree = createClaudeChildTreeReaper(child, { @@ -166,25 +120,4 @@ describe('Claude root kill fallback', () => { await expect(tree.reap()).resolves.toBe('unverifiable') expect(child.kill).not.toHaveBeenCalled() }) - - it('chains per-pid Windows boundaries across a second merge', async () => { - const first = windowsSnapshot(1_000) - const second: WindowsDescendantSnapshot = { - ...windowsSnapshot(2_000), - descendants: [ - { pid: 4243, creationTimeMs: 1_700_000_000_000 }, - { pid: 4244, creationTimeMs: 1_700_000_000_002 } - ] - } - const third: WindowsDescendantSnapshot = { ...second, capturedAtMs: 3_000 } - - const merged = mergeClaudeCapturedTrees( - { platform: 'win32', tree: first }, - { platform: 'win32', tree: second } - ) - expect(merged?.tree.capturedAtMsByPid).toEqual({ '4243': 1_000, '4244': 2_000 }) - const rechained = mergeClaudeCapturedTrees(merged!, { platform: 'win32', tree: third }) - - expect(rechained?.tree.capturedAtMsByPid).toEqual({ '4243': 1_000, '4244': 2_000 }) - }) }) diff --git a/src/main/claude/claude-child-root-termination.ts b/src/main/claude/claude-child-root-termination.ts index bba299195789..3b3f138cd547 100644 --- a/src/main/claude/claude-child-root-termination.ts +++ b/src/main/claude/claude-child-root-termination.ts @@ -1,11 +1,4 @@ import type { SpawnedProcess } from '../../shared/child-process/run-process' -import type { PosixProcessIdentity } from '../pty-descendant-termination' -import type { - WindowsDescendantSnapshot, - WindowsProcessIdentity -} from '../windows-descendant-exit-verification' - -export type ClaudeRootIdentity = PosixProcessIdentity | WindowsProcessIdentity type RootTerminationInput = { child: Pick @@ -27,31 +20,3 @@ type RootTerminationInput = { export function terminateClaudeRoot(input: RootTerminationInput): boolean { return input.exited() ? false : input.child.kill('SIGKILL') } - -type WindowsRootTerminationInput = { - snapshot: WindowsDescendantSnapshot | null - exited: () => boolean - verifyRoot: (root: WindowsProcessIdentity) => Promise - terminateTree: (root: WindowsProcessIdentity) => Promise - killRoot: () => boolean -} - -/** - * `taskkill /T /F` addresses a bare pid, so a dead root's pid may already belong - * to a stranger whose whole tree it would take down: that one is identity-gated. - * The direct root kill after it runs however the probe decided. - */ -export async function terminateClaudeWindowsRoot( - input: WindowsRootTerminationInput -): Promise<{ rootVerified: boolean }> { - const { snapshot, exited, verifyRoot, terminateTree, killRoot } = input - let rootVerified = false - if (!exited() && snapshot) { - rootVerified = await verifyRoot(snapshot.root).catch(() => false) - if (rootVerified && !exited()) { - await terminateTree(snapshot.root).catch(() => {}) - } - } - killRoot() - return { rootVerified } -} diff --git a/src/main/claude/claude-child-tree-snapshot.ts b/src/main/claude/claude-child-tree-snapshot.ts index e0955648b026..c8f517daac62 100644 --- a/src/main/claude/claude-child-tree-snapshot.ts +++ b/src/main/claude/claude-child-tree-snapshot.ts @@ -1,10 +1,4 @@ import type { DescendantSnapshot } from '../pty-descendant-termination' -import type { WindowsDescendantSnapshot } from '../windows-descendant-exit-verification' - -/** One platform's descendant tree, tagged so neither verifier can be handed the other's rows. */ -export type ClaudeCapturedTree = - | { platform: 'posix'; tree: DescendantSnapshot } - | { platform: 'win32'; tree: WindowsDescendantSnapshot } /** * Process-table reads are not atomic: a refresh can omit a still-live row, but @@ -48,81 +42,37 @@ function mergeRowsByPid( } } -export function mergeClaudeCapturedTrees( - previous: ClaudeCapturedTree, - next: ClaudeCapturedTree -): ClaudeCapturedTree | null { - if (previous.platform !== next.platform) { +export function mergeClaudeDescendantSnapshots( + previous: DescendantSnapshot, + next: DescendantSnapshot +): DescendantSnapshot | null { + if (previous.rootPgid !== next.rootPgid) { return null } - if (previous.platform === 'posix' && next.platform === 'posix') { - if (previous.tree.rootPgid !== next.tree.rootPgid) { - return null - } - // A refresh cannot repair an earlier capture that lacked root identity; - // retaining those rows would permit a later numeric-pid kill without proof. - if (!previous.tree.root || !next.tree.root) { - return null - } - if ( - previous.tree.root.pid !== next.tree.root.pid || - previous.tree.root.startedAt !== next.tree.root.startedAt - ) { - return null - } - const descendants = mergeRowsByPid( - previous.tree.descendants, - next.tree.descendants, - (left, right) => left.pgid === right.pgid && left.startedAt === right.startedAt, - (row) => previous.tree.capturedAtMsByPid?.[String(row.pid)] ?? previous.tree.capturedAtMs, - (row) => next.tree.capturedAtMsByPid?.[String(row.pid)] ?? next.tree.capturedAtMs, - next.tree.capturedAtMs - ) - if (!descendants) { - return null - } - return { - platform: 'posix', - tree: { - ...next.tree, - // Retained rows keep their earlier boundary; new rows use the refresh - // boundary. The scalar remains the latest scan for legacy consumers. - descendants: descendants.rows, - ...(descendants.capturedAtMsByPid - ? { capturedAtMsByPid: descendants.capturedAtMsByPid } - : {}) - } - } + // A refresh cannot repair an earlier capture that lacked root identity; + // retaining those rows would permit a later numeric-pid kill without proof. + if (!previous.root || !next.root) { + return null } - if (previous.platform === 'win32' && next.platform === 'win32') { - if ( - previous.tree.root.pid !== next.tree.root.pid || - previous.tree.root.creationTimeMs !== next.tree.root.creationTimeMs - ) { - return null - } - const descendants = mergeRowsByPid( - previous.tree.descendants, - next.tree.descendants, - (left, right) => left.creationTimeMs === right.creationTimeMs, - (row) => previous.tree.capturedAtMsByPid?.[String(row.pid)] ?? previous.tree.capturedAtMs, - (row) => next.tree.capturedAtMsByPid?.[String(row.pid)] ?? next.tree.capturedAtMs, - next.tree.capturedAtMs - ) - if (!descendants) { - return null - } - return { - platform: 'win32', - tree: { - ...next.tree, - descendants: descendants.rows, - ...(descendants.capturedAtMsByPid - ? { capturedAtMsByPid: descendants.capturedAtMsByPid } - : {}), - unidentifiedCount: Math.max(previous.tree.unidentifiedCount, next.tree.unidentifiedCount) - } - } + if (previous.root.pid !== next.root.pid || previous.root.startedAt !== next.root.startedAt) { + return null + } + const descendants = mergeRowsByPid( + previous.descendants, + next.descendants, + (left, right) => left.pgid === right.pgid && left.startedAt === right.startedAt, + (row) => previous.capturedAtMsByPid?.[String(row.pid)] ?? previous.capturedAtMs, + (row) => next.capturedAtMsByPid?.[String(row.pid)] ?? next.capturedAtMs, + next.capturedAtMs + ) + if (!descendants) { + return null + } + return { + ...next, + // Retained rows keep their earlier boundary; new rows use the refresh + // boundary. The scalar remains the latest scan for legacy consumers. + descendants: descendants.rows, + ...(descendants.capturedAtMsByPid ? { capturedAtMsByPid: descendants.capturedAtMsByPid } : {}) } - return null } diff --git a/src/main/claude/claude-structured-location-support.test.ts b/src/main/claude/claude-structured-location-support.test.ts index fe3820964e35..b17e284b4ff0 100644 --- a/src/main/claude/claude-structured-location-support.test.ts +++ b/src/main/claude/claude-structured-location-support.test.ts @@ -38,31 +38,13 @@ describe('supportsClaudeStructuredLocation', () => { ).toBe(true) }) - it('rejects Windows local locations until creation-time proof is available', () => { + it('accepts Windows local locations even when process creation times are unreadable', () => { previousPlatform = setPlatform('win32') + // An addon build without the creation-time flag used to refuse chat here. __setWindowsProcessTreeLoaderForTests(() => ({ ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 }, getAllProcesses: () => undefined })) - expect( - supportsClaudeStructuredLocation({ - executionHostId: 'local', - wslDistro: null, - workspaceId: 'workspace-1', - workspaceKind: 'git-worktree' - }) - ).toBe(false) - }) - - it('accepts Windows local locations once creation-time proof is available', () => { - previousPlatform = setPlatform('win32') - // supportedProcessDataFlags is the addon's own report; the enum alone is - // not proof, because pnpm patches the source over the tarball's prebuilt. - __setWindowsProcessTreeLoaderForTests(() => ({ - ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 }, - supportedProcessDataFlags: 7, - getAllProcesses: () => undefined - })) expect( supportsClaudeStructuredLocation({ executionHostId: 'local', diff --git a/src/main/claude/claude-structured-location-support.ts b/src/main/claude/claude-structured-location-support.ts index 9c784a91325d..41b456835468 100644 --- a/src/main/claude/claude-structured-location-support.ts +++ b/src/main/claude/claude-structured-location-support.ts @@ -1,11 +1,6 @@ import type { AgentSessionExecutionLocation } from '../../shared/agent-session-record' import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' -import { isWindowsProcessStartTimeAvailable } from '../windows/windows-process-table' export function supportsClaudeStructuredLocation(location: AgentSessionExecutionLocation): boolean { - return ( - location.executionHostId === LOCAL_EXECUTION_HOST_ID && - location.wslDistro === null && - (process.platform !== 'win32' || isWindowsProcessStartTimeAvailable()) - ) + return location.executionHostId === LOCAL_EXECUTION_HOST_ID && location.wslDistro === null } diff --git a/src/main/claude/claude-structured-owner-identity.test.ts b/src/main/claude/claude-structured-owner-identity.test.ts index 729aecbec877..376b9d83a497 100644 --- a/src/main/claude/claude-structured-owner-identity.test.ts +++ b/src/main/claude/claude-structured-owner-identity.test.ts @@ -37,4 +37,12 @@ describe('claude structured owner identity', () => { ).resolves.toMatchObject({ processStartTimeMs: 456 }) expect(readStartTime).toHaveBeenCalledTimes(3) }) + + it('records an owner whose start time is unreadable instead of refusing the session', async () => { + const readStartTime = vi.fn(async () => null) + await expect( + claudeProcessIdentity({ identity: IDENTITY, spawnToken: 'spawn-a', pid: 4242 }, readStartTime) + ).resolves.toMatchObject({ pid: 4242, processStartTimeMs: null, spawnToken: 'spawn-a' }) + expect(readStartTime).toHaveBeenCalledTimes(3) + }) }) diff --git a/src/main/claude/claude-structured-owner-identity.ts b/src/main/claude/claude-structured-owner-identity.ts index 80f6ca6977dc..65589b53aa4b 100644 --- a/src/main/claude/claude-structured-owner-identity.ts +++ b/src/main/claude/claude-structured-owner-identity.ts @@ -41,6 +41,8 @@ export async function claudeProcessIdentity( if (input.pid === undefined) { throw new Error('claude app-server started without a pid') } + // Best-effort: without it a later owner probe is indeterminate, and recovery releases such an + // owner without signalling it, so an unreadable start time must not refuse the session. let processStartTimeMs: number | null = null for ( let attempt = 0; @@ -49,11 +51,6 @@ export async function claudeProcessIdentity( ) { processStartTimeMs = await readStartTime(input.pid) } - if (processStartTimeMs === null) { - // Why: recording null makes every later owner probe indeterminate — a durable latch. - // Failing here reaps the child and leaves a retryable refusal instead. - throw new Error(`claude app-server start time for pid ${input.pid} could not be read`) - } return { hostId: input.identity.hostId, pid: input.pid, diff --git a/src/main/codex/codex-structured-acquisition-exit-proof.test.ts b/src/main/codex/codex-structured-acquisition-exit-proof.test.ts index f86d0aff6c1f..059c0f4851fa 100644 --- a/src/main/codex/codex-structured-acquisition-exit-proof.test.ts +++ b/src/main/codex/codex-structured-acquisition-exit-proof.test.ts @@ -79,7 +79,10 @@ describe('Codex failed-acquisition exit proof', () => { resumeThreadId: 'thread-1' }), openConnection: async () => connection, - readProcessStartTime: async () => null + // An unreadable start time no longer fails a start, so the identity read itself fails here. + readProcessStartTime: async () => { + throw new Error('process table unavailable') + } }) await expect( diff --git a/src/main/codex/codex-structured-launch-resolution.test.ts b/src/main/codex/codex-structured-launch-resolution.test.ts index 1e6a6b4805e5..e69f3b06fad1 100644 --- a/src/main/codex/codex-structured-launch-resolution.test.ts +++ b/src/main/codex/codex-structured-launch-resolution.test.ts @@ -7,6 +7,15 @@ import { createCodexStructuredLaunchResolver } from './codex-structured-launch-r import { codexStructuredPermissionPolicyForSettings } from './codex-structured-permission-policy' import { codexProviderHandle } from '../../shared/agent-session-provider-handle-encoding' +const { isWindowsProcessStartTimeAvailable } = vi.hoisted(() => ({ + isWindowsProcessStartTimeAvailable: vi.fn(() => true) +})) + +vi.mock('../windows/windows-process-table', async (importOriginal) => ({ + ...(await importOriginal()), + isWindowsProcessStartTimeAvailable +})) + const SESSION_ID = 'session-1' const IDENTITY = { sessionId: SESSION_ID } as Parameters< ReturnType @@ -49,7 +58,6 @@ function resolverFor( resolveWorkspacePath, resolveCommand: () => '/usr/local/bin/codex', resolveRollout, - isWindowsProcessStartTimeAvailable: () => true, resolvePermissionPolicy: () => codexStructuredPermissionPolicyForSettings({ agentDefaultArgs }) }) } @@ -76,8 +84,7 @@ describe('codex structured launch resolution', () => { const resolveLaunch = createCodexStructuredLaunchResolver({ store: { getRecord: () => record() } as unknown as AgentSessionRecordStore, resolveWorkspacePath: async () => String.raw`C:\workspaces\orca`, - resolveCommand: () => command, - isWindowsProcessStartTimeAvailable: () => true + resolveCommand: () => command }) await expect(resolveLaunch({ identity: IDENTITY })).resolves.toMatchObject({ @@ -87,19 +94,19 @@ describe('codex structured launch resolution', () => { }) }) - it('fails closed before resolving a Windows launch without creation-time proof', async () => { + it('resolves a Windows launch on a host that cannot read process creation times', async () => { + isWindowsProcessStartTimeAvailable.mockReturnValue(false) await withPlatform('win32', async () => { - const resolveWorkspacePath = vi.fn(async () => String.raw`C:\workspaces\orca`) const resolveLaunch = createCodexStructuredLaunchResolver({ store: { getRecord: () => record() } as unknown as AgentSessionRecordStore, - resolveWorkspacePath, - isWindowsProcessStartTimeAvailable: () => false + resolveWorkspacePath: async () => String.raw`C:\workspaces\orca`, + resolveCommand: () => 'codex.exe' }) - await expect(resolveLaunch({ identity: IDENTITY })).rejects.toThrow( - 'Windows process creation-time proof' - ) - expect(resolveWorkspacePath).not.toHaveBeenCalled() + await expect(resolveLaunch({ identity: IDENTITY })).resolves.toMatchObject({ + command: 'codex.exe', + args: ['app-server'] + }) }) }) diff --git a/src/main/codex/codex-structured-launch-resolution.ts b/src/main/codex/codex-structured-launch-resolution.ts index 19945c9a6c89..751b975bb848 100644 --- a/src/main/codex/codex-structured-launch-resolution.ts +++ b/src/main/codex/codex-structured-launch-resolution.ts @@ -14,7 +14,6 @@ import type { AgentSessionRecordStore } from '../runtime/agent-session-record-st import type { CodexStructuredLaunch } from './codex-structured-session-adapter' import type { CodexStructuredPermissionPolicy } from './codex-structured-permission-policy' import { resolvePinnedCodexRolloutProof } from './codex-pinned-rollout-proof' -import { isWindowsProcessStartTimeAvailable } from '../windows/windows-process-table' export type CodexStructuredLaunchResolverDeps = { store: AgentSessionRecordStore @@ -26,8 +25,6 @@ export type CodexStructuredLaunchResolverDeps = { /** Fresh shell/configured environment for this spawn; never written to the session record. */ resolveEnvironment?: () => Promise resolveRollout?: typeof resolvePinnedCodexRolloutProof - /** Test seam for the host capability; production uses the native process table. */ - isWindowsProcessStartTimeAvailable?: () => boolean /** The user's Agent Permissions setting as thread policy, re-read per acquisition. * States both postures outright — a resume inherits the last one for any field left absent. */ resolvePermissionPolicy?: () => CodexStructuredPermissionPolicy @@ -78,13 +75,6 @@ export function createCodexStructuredLaunchResolver( `codex structured sessions run on the local host, not ${location.executionHostId}` ) } - // Refuse before resolving launch data; a PID alone cannot prove Windows ownership. - if ( - process.platform === 'win32' && - !(deps.isWindowsProcessStartTimeAvailable ?? isWindowsProcessStartTimeAvailable)() - ) { - throw new Error('codex structured sessions require Windows process creation-time proof') - } if (accountHome.variable !== 'CODEX_HOME') { throw new Error(`codex sessions pin CODEX_HOME, not ${accountHome.variable}`) } diff --git a/src/main/codex/codex-structured-location-support.test.ts b/src/main/codex/codex-structured-location-support.test.ts index 324568956d86..ac8fb6c42884 100644 --- a/src/main/codex/codex-structured-location-support.test.ts +++ b/src/main/codex/codex-structured-location-support.test.ts @@ -25,23 +25,16 @@ function withPlatform(platform: NodeJS.Platform, run: () => T): T { } describe('Codex structured location support', () => { - it('uses the injected Windows identity capability for location admission', () => { - let proofAvailable = false + it('admits a native Windows location without consulting process creation times', () => { withPlatform('win32', () => { - expect(supportsCodexStructuredLocation(LOCAL_WINDOWS_LOCATION, () => proofAvailable)).toBe( - false - ) - proofAvailable = true - expect(supportsCodexStructuredLocation(LOCAL_WINDOWS_LOCATION, () => proofAvailable)).toBe( - true - ) + expect(supportsCodexStructuredLocation(LOCAL_WINDOWS_LOCATION)).toBe(true) }) }) it('rejects WSL locations while retaining native folder support on Windows', () => { withPlatform('win32', () => { - expect(supportsCodexStructuredLocation(WSL_WINDOWS_LOCATION, () => true)).toBe(false) - expect(supportsCodexStructuredLocation(LOCAL_WINDOWS_LOCATION, () => true)).toBe(true) + expect(supportsCodexStructuredLocation(WSL_WINDOWS_LOCATION)).toBe(false) + expect(supportsCodexStructuredLocation(LOCAL_WINDOWS_LOCATION)).toBe(true) }) }) }) diff --git a/src/main/codex/codex-structured-location-support.ts b/src/main/codex/codex-structured-location-support.ts index ad0bbefa4d33..618a415ebb26 100644 --- a/src/main/codex/codex-structured-location-support.ts +++ b/src/main/codex/codex-structured-location-support.ts @@ -1,15 +1,6 @@ import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' import type { AgentSessionExecutionLocation } from '../../shared/agent-session-record' -import { isWindowsProcessStartTimeAvailable } from '../windows/windows-process-table' -export function supportsCodexStructuredLocation( - location: AgentSessionExecutionLocation, - // Injected by the adapter, which owns this dep for every other Codex gate too. - hasWindowsProcessStartTimeProof: () => boolean = isWindowsProcessStartTimeAvailable -): boolean { - return ( - location.executionHostId === LOCAL_EXECUTION_HOST_ID && - location.wslDistro === null && - (process.platform !== 'win32' || hasWindowsProcessStartTimeProof()) - ) +export function supportsCodexStructuredLocation(location: AgentSessionExecutionLocation): boolean { + return location.executionHostId === LOCAL_EXECUTION_HOST_ID && location.wslDistro === null } diff --git a/src/main/codex/codex-structured-owner-identity.test.ts b/src/main/codex/codex-structured-owner-identity.test.ts index 229da5e00bd1..e5d8df1f9398 100644 --- a/src/main/codex/codex-structured-owner-identity.test.ts +++ b/src/main/codex/codex-structured-owner-identity.test.ts @@ -37,13 +37,16 @@ describe('codex process identity', () => { expect(readStartTime).toHaveBeenCalledTimes(3) }) - it('refuses an owner whose start time is unreadable rather than record one no probe can verify', async () => { - // A null start time guarantees every later owner probe answers indeterminate, which is - // a durable latch; refusing here is a retryable failure instead. + it('records an owner whose start time is unreadable instead of refusing the session', async () => { const readStartTime = vi.fn(async () => null) await expect( codexProcessIdentity({ identity: IDENTITY, spawnToken: 'spawn-a', pid: 4242 }, readStartTime) - ).rejects.toThrow('start time') + ).resolves.toEqual({ + hostId: 'local', + pid: 4242, + processStartTimeMs: null, + spawnToken: 'spawn-a' + }) expect(readStartTime).toHaveBeenCalledTimes(3) }) }) diff --git a/src/main/codex/codex-structured-owner-identity.ts b/src/main/codex/codex-structured-owner-identity.ts index 9b0d3e8e5488..00dd0bfb61f7 100644 --- a/src/main/codex/codex-structured-owner-identity.ts +++ b/src/main/codex/codex-structured-owner-identity.ts @@ -55,6 +55,8 @@ export async function codexProcessIdentity( if (input.pid === undefined) { throw new Error('codex app-server started without a pid') } + // Best-effort: without it a later owner probe is indeterminate, and recovery releases such an + // owner without signalling it, so an unreadable start time must not refuse the session. let processStartTimeMs: number | null = null for ( let attempt = 0; @@ -63,11 +65,6 @@ export async function codexProcessIdentity( ) { processStartTimeMs = await readStartTime(input.pid) } - if (processStartTimeMs === null) { - // Why: recording null makes every later owner probe indeterminate — a durable latch. - // Failing here reaps the child and leaves a retryable refusal instead. - throw new Error(`codex app-server start time for pid ${input.pid} could not be read`) - } return { hostId: input.identity.hostId, pid: input.pid, diff --git a/src/main/codex/codex-structured-session-adapter.ts b/src/main/codex/codex-structured-session-adapter.ts index 1c5a034fd63c..fdd9a1b29722 100644 --- a/src/main/codex/codex-structured-session-adapter.ts +++ b/src/main/codex/codex-structured-session-adapter.ts @@ -78,8 +78,7 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap }) } - supportsLocation = (location: Parameters[0]): boolean => - supportsCodexStructuredLocation(location, this.deps.isWindowsProcessStartTimeAvailable) + supportsLocation = supportsCodexStructuredLocation acquire = (input: StructuredAgentSessionAcquireInput): Promise => acquireCodexStructuredSession({ diff --git a/src/main/codex/codex-structured-session-state.ts b/src/main/codex/codex-structured-session-state.ts index 5773b659f5e1..bc273d5ebd77 100644 --- a/src/main/codex/codex-structured-session-state.ts +++ b/src/main/codex/codex-structured-session-state.ts @@ -76,8 +76,6 @@ export type CodexStructuredSessionAdapterDeps = { resolveLaunch: (input: { identity: AgentSessionJournalIdentity }) => Promise - /** Host capability seam; production uses the native Windows process table. */ - isWindowsProcessStartTimeAvailable?: () => boolean onEvent?: (event: CodexStructuredSessionEvent) => void /** Where bookkeeping a close or exit does after the child is gone reports a failure. */ logger?: StructuredAgentSessionLogger diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts index 93c9b2f569a6..d92eaddadc38 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts @@ -243,6 +243,31 @@ describe('structured session recovery resolution', () => { }) }) + it('releases a Windows owner the probe matches without signalling its saved pid', async () => { + const store = await openStore() + await liveOwner(store) + await latch(store) + const stopOwnerProcess = vi.fn() + + const result = await resolveStructuredSessionRecovery( + deps(store, () => ({ outcome: 'identity-matched', matchedOn: ['process-start-time'] }), { + stopOwnerProcess, + platform: 'win32' + }), + SESSION + ) + + expect(result).toBe('resolved') + // Windows stops a tree only through a child it still holds; a saved pid is never signalled. + expect(stopOwnerProcess).not.toHaveBeenCalled() + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'released', + handoffStage: null, + ownerProcess: null, + deathEvidence: null + }) + }) + it('waits out a terminal owner an older build recorded, and never stops it', async () => { const directory = await newStoreDirectory() await liveOwner(await openStore(directory)) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts index 5dbd9a4071b7..15e364fc7fed 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts @@ -4,9 +4,10 @@ * evicted on proof. A live one is stopped by identity and evicted once * proven gone. One that outlives the stop, or whose identity cannot be verified, is released * anyway: its transport died with the runtime that held it, so nothing can drive it, and no signal - * is sent to a pid that cannot be verified as the one recorded. Only a conflicted claim, which is - * how a terminal owner an older build recorded now loads, is waited out and never stopped: it is - * the user's own agent, and its exit is its way out. + * is sent to a pid that cannot be verified as the one recorded. Windows never signals a saved pid + * at all: Orca stops a Windows tree only through a child it still holds. Only a conflicted claim, + * which is how a terminal owner an older build recorded now loads, is waited out and never + * stopped: it is the user's own agent, and its exit is its way out. */ import { @@ -27,6 +28,7 @@ export type StructuredSessionRecoveryResolutionDeps = { now: () => number stopOwnerProcess?: (pid: number, signal: StructuredSessionRecoveryStopSignal) => void delay?: (ms: number) => Promise + platform?: NodeJS.Platform } const STOP_PROBE_INTERVAL_MS = 250 @@ -62,7 +64,9 @@ export async function resolveStructuredSessionRecovery( if (owner.hostId !== deps.store.hostId) { return 'unresolved' } - probe = await stopOwnerAndReprobe(deps, record, owner.pid) + if ((deps.platform ?? process.platform) !== 'win32') { + probe = await stopOwnerAndReprobe(deps, record, owner.pid) + } } try { await (owner && !isProvenDeadProbe(probe) diff --git a/src/main/provider-process/provider-process-teardown.test.ts b/src/main/provider-process/provider-process-teardown.test.ts index d5fdcce6afcf..89823f2dfd69 100644 --- a/src/main/provider-process/provider-process-teardown.test.ts +++ b/src/main/provider-process/provider-process-teardown.test.ts @@ -23,7 +23,7 @@ describe('terminateProviderProcessTree', () => { it('waits for the Windows tree kill before releasing the wrapper', async () => { const target = child() - const release = Promise.withResolvers() + const release = Promise.withResolvers() const terminateWindowsTree = vi.fn(() => release.promise) const teardown = terminateProviderProcessTree(target, { @@ -32,15 +32,29 @@ describe('terminateProviderProcessTree', () => { terminateWindowsTree }) expect(target.kill).not.toHaveBeenCalled() - release.resolve() - await teardown + release.resolve(true) + await expect(teardown).resolves.toBe(true) expect(terminateWindowsTree).toHaveBeenCalledWith(1234, { site: 'codex-app-server-teardown' }) expect(target.kill).toHaveBeenCalledWith('SIGKILL') }) + it('reports an unproven Windows tree when taskkill does not exit cleanly', async () => { + const target = child() + + await expect( + terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', + platform: 'win32', + terminateWindowsTree: async () => false + }) + ).resolves.toBe(false) + // The held child is still killed through its handle. + expect(target.kill).toHaveBeenCalledWith('SIGKILL') + }) + it('passes a non-Codex diagnostic label to Windows teardown', async () => { - const terminateWindowsTree = vi.fn(async () => undefined) + const terminateWindowsTree = vi.fn(async () => true) await terminateProviderProcessTree(child(), { site: 'provider-test-teardown', diff --git a/src/main/provider-process/provider-process-teardown.ts b/src/main/provider-process/provider-process-teardown.ts index 19e5b9b967f3..a7910f2b335c 100644 --- a/src/main/provider-process/provider-process-teardown.ts +++ b/src/main/provider-process/provider-process-teardown.ts @@ -14,7 +14,8 @@ export type ProviderProcessTeardownDeps = { dedicatedProcessGroup?: boolean captureDescendants?: (rootPid: number) => Promise terminateDescendants?: (snapshot: DescendantSnapshot) => Promise - terminateWindowsTree?: (rootPid: number, deps?: { site?: string }) => Promise + /** Resolves true only when taskkill reports the whole tree terminated. */ + terminateWindowsTree?: (rootPid: number, deps?: { site?: string }) => Promise signalProcessGroup?: (pgid: number, signal: NodeJS.Signals) => void } @@ -98,10 +99,10 @@ async function terminateOnce( } if ((deps.platform ?? process.platform) === 'win32') { const terminate = deps.terminateWindowsTree ?? terminateWindowsProcessTree - await terminate(rootPid, { site: deps.site }) + const treeTerminated = await terminate(rootPid, { site: deps.site }) // taskkill owns the tree; this preserves the prior direct-child fallback when it fails. child.kill('SIGKILL') - return true + return treeTerminated } if (deps.dedicatedProcessGroup) { return terminateDedicatedPosixGroup(rootPid, deps) diff --git a/src/main/providers/windows-foreground-process-rows.ts b/src/main/providers/windows-foreground-process-rows.ts index de24a2717129..8d93b4deb830 100644 --- a/src/main/providers/windows-foreground-process-rows.ts +++ b/src/main/providers/windows-foreground-process-rows.ts @@ -106,26 +106,6 @@ export async function queryWindowsPaneProcessInventory( } } -/** - * The descendant walk over rows the caller already read. - * - * Why exported: a caller that needs a field this module's projection drops — - * process creation time, for a PID-reuse-safe teardown snapshot — would - * otherwise read the whole table a second time to get it. - * Null when the root is absent, which is a stale or filtered snapshot rather - * than a root with no descendants. - */ -export function windowsDescendantsFromRows( - rows: Row[], - rootPid: number -): (Row & { depth: number })[] | null { - const index = getProcessTableIndex(rows) - if (!index.byPid.has(rootPid)) { - return null - } - return collectDescendantsFromIndex(index, rootPid).sort((a, b) => b.depth - a.depth) -} - /** Test-only: clear the shared snapshot so one case's rows never serve the next. */ export function resetWindowsProcessRowsSnapshotForTests(): void { resetWindowsProcessTableForTests() diff --git a/src/main/runtime/orca-runtime-get-status.ts b/src/main/runtime/orca-runtime-get-status.ts index 39b8dbf0dcfb..3451cd8af301 100644 --- a/src/main/runtime/orca-runtime-get-status.ts +++ b/src/main/runtime/orca-runtime-get-status.ts @@ -75,8 +75,9 @@ export class OrcaRuntimeWithGetStatus extends OrcaRuntimeWithGetRuntimeId { const hasOffscreen = !hasRenderer && Boolean(this.offscreenBrowserBackend) const hasHeadlessCommands = runtimeBrowserCommandsFactoryIsHeadless() const canBrowse = hasRenderer || hasOffscreen - // This field reports current Windows process-identity proof. Structured RPC - // support itself stays advertised; agentSession.createSupport owns current eligibility. + // TEMPORARY: nothing on this host reads it any more. Clients older than this build keep + // re-probing their WSL capabilities until it is true, so it is published for one + // compatibility window and then removed. const windowsProcessStartTimeAvailable = process.platform === 'win32' && isWindowsProcessStartTimeAvailable() const capabilities: RuntimeCapability[] = RUNTIME_CAPABILITIES.filter( diff --git a/src/main/runtime/structured-agent-session-runtime.test.ts b/src/main/runtime/structured-agent-session-runtime.test.ts index f572e8909833..eb0bb48c8a08 100644 --- a/src/main/runtime/structured-agent-session-runtime.test.ts +++ b/src/main/runtime/structured-agent-session-runtime.test.ts @@ -286,7 +286,7 @@ describe('structured agent-session runtime install', () => { expect(stopped).toBe(true) }) - it('does not infer Windows process identity support from an injected reader', async () => { + it('supports native Windows creation without the process-table addon', async () => { stateDirectory = await mkdtemp(join(tmpdir(), 'orca-structured-runtime-')) const originalPlatform = process.platform const location: AgentSessionExecutionLocation = { @@ -309,7 +309,9 @@ describe('structured agent-session runtime install', () => { readProcessStartTime: async () => 1_700_000_000_000 }) - expect(host.supportsCreate(location, 'codex')).toBe(false) + // No addon means no creation times; chat no longer depends on them. + expect(host.supportsCreate(location, 'codex')).toBe(true) + expect(host.supportsCreate(location, 'claude')).toBe(true) } finally { __setWindowsProcessTreeLoaderForTests() Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) 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 ea0ab3298d9b..6bfdcddba0f0 100644 --- a/src/main/runtime/structured-agent-session-support-probe.test.ts +++ b/src/main/runtime/structured-agent-session-support-probe.test.ts @@ -115,23 +115,21 @@ describe('structured agent-session create-support probe', () => { ) it.each([ - ['codex', true, { supported: true }], - ['codex', false, { supported: false, reason: 'agent' }], - ['claude', true, { supported: true }], - ['claude', false, { supported: false, reason: 'agent' }] + ['codex', true], + ['codex', false], + ['claude', true], + ['claude', false] ] as const)( - 'requires native Windows process identity proof before answering %s support (%s)', - async (agent, proofAvailable, expected) => { + 'answers native Windows %s support whether or not process creation times are readable (%s)', + async (agent, creationTimesReadable) => { setPlatform('win32') - isWindowsProcessStartTimeAvailable.mockReturnValue(proofAvailable) + isWindowsProcessStartTimeAvailable.mockReturnValue(creationTimesReadable) await expectSupportWithoutInstall({ agent, location: { executionHostId: 'local', wslDistro: null }, - expected + expected: { supported: true } }) - - expect(isWindowsProcessStartTimeAvailable).toHaveBeenCalled() } ) diff --git a/src/main/windows-descendant-exit-verification.test.ts b/src/main/windows-descendant-exit-verification.test.ts deleted file mode 100644 index d1944f5ef862..000000000000 --- a/src/main/windows-descendant-exit-verification.test.ts +++ /dev/null @@ -1,270 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' -import { - captureWindowsDescendantSnapshot, - terminateIdentifiedWindowsProcessTree, - verifyWindowsDescendantSnapshotExit, - type WindowsDescendantSnapshot -} from './windows-descendant-exit-verification' - -function snapshot( - descendants: { pid: number; creationTimeMs: number }[], - unidentifiedCount = 0 -): WindowsDescendantSnapshot { - return { - root: { pid: 100, creationTimeMs: 5 }, - descendants, - unidentifiedCount, - capturedAtMs: 1_700_000_000_000 - } -} - -describe('captureWindowsDescendantSnapshot', () => { - it('does not claim an older process whose former parent PID was reused by the root', async () => { - const olderProcess = { pid: 50244, ppid: 36084, creationTimeMs: 1788659167395 } - const captured = await captureWindowsDescendantSnapshot(36084, { - readTable: async () => [ - { pid: 36084, ppid: 60976, creationTimeMs: 1788733587893 }, - olderProcess - ] - }) - - expect(captured?.descendants).toEqual([]) - await expect( - verifyWindowsDescendantSnapshotExit(captured!, { readTable: async () => [olderProcess] }) - ).resolves.toBe('exited') - }) - - it('prunes a stale parent link and its subtree at any depth', async () => { - const captured = await captureWindowsDescendantSnapshot(100, { - readTable: async () => [ - { pid: 100, ppid: 1, creationTimeMs: 5 }, - { pid: 200, ppid: 100, creationTimeMs: 10 }, - { pid: 300, ppid: 200, creationTimeMs: 7 }, - { pid: 400, ppid: 300, creationTimeMs: 12 }, - { pid: 500, ppid: 100, creationTimeMs: 4 }, - { pid: 600, ppid: 500, creationTimeMs: 13 }, - { pid: 700, ppid: 200, creationTimeMs: 10 } - ] - }) - - expect(captured?.descendants).toEqual([ - { pid: 700, creationTimeMs: 10 }, - { pid: 200, creationTimeMs: 10 } - ]) - }) - - it('keeps the root when its own parent PID was reused by a newer process', async () => { - // The root's retained ppid now names a process created after it. Pruning the - // root drops the whole snapshot, so its own link is never evidence about it. - const captured = await captureWindowsDescendantSnapshot(100, { - readTable: async () => [ - { pid: 100, ppid: 900, creationTimeMs: 5 }, - { pid: 900, ppid: 1, creationTimeMs: 50 }, - { pid: 200, ppid: 100, creationTimeMs: 7 } - ], - now: () => 42 - }) - - expect(captured).toEqual({ - root: { pid: 100, creationTimeMs: 5 }, - descendants: [{ pid: 200, creationTimeMs: 7 }], - unidentifiedCount: 0, - capturedAtMs: 42 - }) - }) - - it('bounds a link by the root when the claimed parent denied its creation time', async () => { - // 300 has no creation time for a child to be compared against, so the root's - // start is the only bound left: 350 ties with it, which a same-millisecond - // spawn does routinely, while 360 predates the whole tree. - const captured = await captureWindowsDescendantSnapshot(100, { - readTable: async () => [ - { pid: 100, ppid: 1, creationTimeMs: 5 }, - { pid: 300, ppid: 100 }, - { pid: 350, ppid: 300, creationTimeMs: 5 }, - { pid: 360, ppid: 300, creationTimeMs: 2 } - ], - now: () => 42 - }) - - expect(captured).toEqual({ - root: { pid: 100, creationTimeMs: 5 }, - descendants: [{ pid: 350, creationTimeMs: 5 }], - unidentifiedCount: 1, - capturedAtMs: 42 - }) - }) - - it('drops an unidentified row whose parent link was pruned', async () => { - // 250 denied its creation time, but 200's claim on the root is impossible, so - // 250 was never in this tree: counting it would cap the verdict at - // unverifiable over a process the root does not own. - const captured = await captureWindowsDescendantSnapshot(100, { - readTable: async () => [ - { pid: 100, ppid: 1, creationTimeMs: 10 }, - { pid: 200, ppid: 100, creationTimeMs: 5 }, - { pid: 250, ppid: 200 } - ] - }) - - expect(captured?.descendants).toEqual([]) - expect(captured?.unidentifiedCount).toBe(0) - await expect( - verifyWindowsDescendantSnapshotExit(captured!, { readTable: async () => [] }) - ).resolves.toBe('exited') - }) - - it('walks the whole subtree and keeps only rows a later read can re-identify', async () => { - const captured = await captureWindowsDescendantSnapshot(100, { - // 400 is a grandchild; 300 denied a creation-time query, so no later read - // could tell it from a recycled pid and signalling it would risk a stranger. - readTable: vi.fn(async () => [ - { pid: 100, ppid: 1, creationTimeMs: 5 }, - { pid: 200, ppid: 100, creationTimeMs: 7 }, - { pid: 300, ppid: 100 }, - { pid: 400, ppid: 200, creationTimeMs: 9 }, - { pid: 500, ppid: 1, creationTimeMs: 11 } - ]), - now: () => 42 - }) - - expect(captured).toEqual({ - root: { pid: 100, creationTimeMs: 5 }, - descendants: [ - { pid: 400, creationTimeMs: 9 }, - { pid: 200, creationTimeMs: 7 } - ], - // Seen but not re-identifiable: counted, so no later read can prove it gone. - unidentifiedCount: 1, - capturedAtMs: 42 - }) - }) - - it('reports an unreadable or rootless table as no snapshot rather than an empty one', async () => { - await expect( - captureWindowsDescendantSnapshot(100, { - readTable: vi.fn(async () => { - throw new Error('table unavailable') - }) - }) - ).resolves.toBeNull() - // A snapshot without the root is stale or filtered; only an observed root - // can authoritatively have no descendants. - await expect( - captureWindowsDescendantSnapshot(100, { - readTable: vi.fn(async () => [{ pid: 999, ppid: 1, creationTimeMs: 5 }]) - }) - ).resolves.toBeNull() - }) - - it('refuses an invalid root pid', async () => { - const readTable = vi.fn() - await expect(captureWindowsDescendantSnapshot(0, { readTable })).resolves.toBeNull() - expect(readTable).not.toHaveBeenCalled() - }) -}) - -describe('verifyWindowsDescendantSnapshotExit', () => { - it('proves an empty tree without reading the table', async () => { - const readTable = vi.fn() - await expect(verifyWindowsDescendantSnapshotExit(snapshot([]), { readTable })).resolves.toBe( - 'exited' - ) - expect(readTable).not.toHaveBeenCalled() - }) - - it('never proves a tree that held a descendant it could not identify', async () => { - // A descendant that denied the creation-time query was seen in the table; - // being unable to re-identify it is "could not look", never "it is gone". - const readTable = vi.fn() - await expect(verifyWindowsDescendantSnapshotExit(snapshot([], 1), { readTable })).resolves.toBe( - 'unverifiable' - ) - expect(readTable).not.toHaveBeenCalled() - - // The identified sibling leaving proves nothing about the unidentified one. - await expect( - verifyWindowsDescendantSnapshotExit(snapshot([{ pid: 200, creationTimeMs: 7 }], 1), { - readTable: vi.fn(async () => []), - wait: async () => {}, - now: vi.fn().mockReturnValueOnce(0).mockReturnValue(1) - }) - ).resolves.toBe('unverifiable') - }) - - it('reports exited once no identity-matched row remains', async () => { - const readTable = vi - .fn() - .mockResolvedValueOnce([{ pid: 200, ppid: 100, creationTimeMs: 7 }]) - // The pid came back on a different process; that is a recycle, not a survivor. - .mockResolvedValueOnce([{ pid: 200, ppid: 100, creationTimeMs: 99 }]) - - await expect( - verifyWindowsDescendantSnapshotExit(snapshot([{ pid: 200, creationTimeMs: 7 }]), { - readTable, - wait: async () => {}, - now: vi.fn().mockReturnValueOnce(0).mockReturnValue(1) - }) - ).resolves.toBe('exited') - expect(readTable).toHaveBeenCalledTimes(2) - }) - - it('reports live for a descendant still matched at the deadline', async () => { - let clock = 0 - await expect( - verifyWindowsDescendantSnapshotExit(snapshot([{ pid: 200, creationTimeMs: 7 }]), { - readTable: vi.fn(async () => [{ pid: 200, ppid: 100, creationTimeMs: 7 }]), - wait: async () => { - clock += 100 - }, - now: () => clock, - verifyMs: 250 - }) - ).resolves.toBe('live') - }) - - it('reports unverifiable when the table cannot be read at the deadline', async () => { - await expect( - verifyWindowsDescendantSnapshotExit(snapshot([{ pid: 200, creationTimeMs: 7 }]), { - readTable: vi.fn(async () => { - throw new Error('table unavailable') - }), - wait: async () => {}, - now: vi.fn().mockReturnValueOnce(0).mockReturnValue(9_999) - }) - ).resolves.toBe('unverifiable') - }) -}) - -describe('terminateIdentifiedWindowsProcessTree', () => { - it('never taskkills a replacement that reused the captured root pid', async () => { - const terminateTree = vi.fn(async () => {}) - - await expect( - terminateIdentifiedWindowsProcessTree( - { pid: 100, creationTimeMs: 5 }, - { - readTable: vi.fn(async () => [{ pid: 100, ppid: 1, creationTimeMs: 99 }]), - terminateTree - } - ) - ).resolves.toBe(false) - expect(terminateTree).not.toHaveBeenCalled() - }) - - it('rechecks retained-child ownership after the identity read settles', async () => { - const terminateTree = vi.fn(async () => {}) - - await expect( - terminateIdentifiedWindowsProcessTree( - { pid: 100, creationTimeMs: 5 }, - { - readTable: vi.fn(async () => [{ pid: 100, ppid: 1, creationTimeMs: 5 }]), - ownsRoot: () => false, - terminateTree - } - ) - ).resolves.toBe(false) - expect(terminateTree).not.toHaveBeenCalled() - }) -}) diff --git a/src/main/windows-descendant-exit-verification.ts b/src/main/windows-descendant-exit-verification.ts deleted file mode 100644 index 5e365a57e5a3..000000000000 --- a/src/main/windows-descendant-exit-verification.ts +++ /dev/null @@ -1,186 +0,0 @@ -import { getProcessTableIndex } from '../shared/process-table-index' -import type { DescendantTreeVerdict } from './pty-descendant-exit-verification' -import { windowsDescendantsFromRows } from './providers/windows-foreground-process-rows' -import { readWindowsProcessTableFresh } from './windows/windows-process-table' -import { terminateWindowsProcessTree } from './windows-process-tree-kill' - -export const WINDOWS_DESCENDANT_KILL_VERIFY_MS = 3_500 -const WINDOWS_DESCENDANT_POLL_MS = 100 - -/** - * A Windows descendant tree captured while its root was alive, with the - * PID-reuse guard the POSIX snapshot gets from ps lstart: a row only counts as - * the same process when its creation time still matches. Rows without a - * creation time are never signalled, because a bare pid cannot be re-identified, - * but they are counted: a descendant that was seen and denied identification - * is one no later read can prove gone. - */ -export type WindowsProcessIdentity = { pid: number; creationTimeMs: number } - -export type WindowsDescendantSnapshot = { - root: WindowsProcessIdentity - descendants: WindowsProcessIdentity[] - /** Descendants seen in the walk that denied the creation-time query. */ - unidentifiedCount: number - capturedAtMs: number - /** Per-PID boundaries retained when close refreshes merge snapshots. */ - capturedAtMsByPid?: Readonly> -} - -export type WindowsDescendantVerificationDeps = { - readTable?: () => Promise<{ pid: number; ppid: number; creationTimeMs?: number }[]> - now?: () => number - wait?: (ms: number) => Promise - verifyMs?: number -} - -/** Revalidate a Windows PID/creation-time identity immediately before a kill. */ -export async function verifyWindowsProcessIdentity( - target: WindowsProcessIdentity, - deps: Pick = {} -): Promise { - if (!Number.isInteger(target.pid) || target.pid <= 0 || !Number.isFinite(target.creationTimeMs)) { - return false - } - const table = await (deps.readTable ?? readWindowsProcessTableFresh)().catch(() => null) - const current = table?.filter((row) => row.pid === target.pid) ?? [] - return current.length === 1 && current[0]?.creationTimeMs === target.creationTimeMs -} - -function delay(ms: number): Promise { - return new Promise((resolve) => { - const timer = setTimeout(resolve, ms) - timer.unref?.() - }) -} - -/** - * Snapshot a Windows root's descendants while it is still alive. Resolves null - * (never rejects) when the table is unreadable or the root is absent — the same - * contract as the POSIX walk, because "cannot see" is never "nothing is there". - * - * Stale parent links are pruned by creation time, so a backwards clock step - * between two spawns can drop a live descendant — accepted over a certain stall. - */ -export async function captureWindowsDescendantSnapshot( - rootPid: number, - deps: WindowsDescendantVerificationDeps = {} -): Promise { - if (!Number.isInteger(rootPid) || rootPid <= 0) { - return null - } - const capturedAtMs = (deps.now ?? Date.now)() - // One table read, not a walk plus an identity read: each is bounded in - // seconds, and this runs inside the close ladder's budget. - const table = await (deps.readTable ?? readWindowsProcessTableFresh)().catch(() => null) - if (!table) { - return null - } - // One index for both lookups, so a repeated pid resolves to the same row for - // the root and for a parent link: `byPid` is first-wins, a Map is not. - const rowsByPid = getProcessTableIndex(table).byPid - const root = rowsByPid.get(rootPid) - if (typeof root?.creationTimeMs !== 'number') { - return null - } - const rootCreationTimeMs = root.creationTimeMs - // Windows keeps a process's original parent PID after that parent exits, so a - // reused PID is not ancestry: no real child predates the parent it claims. - // The root's start backstops the undefined-time bypass, which admits a row - // unchecked and leaves its children no parent time to compare against. Ties - // pass -- FILETIMEs truncated to ms make a same-millisecond parent and child - // collide exactly, so `>` would drop true descendants. - const currentRows = table.filter((row) => { - const parentCreationTimeMs = rowsByPid.get(row.ppid)?.creationTimeMs - return ( - // Its own ppid can be recycled too, and a pruned root loses the snapshot. - row.pid === rootPid || - row.creationTimeMs === undefined || - (row.creationTimeMs >= rootCreationTimeMs && - (parentCreationTimeMs === undefined || row.creationTimeMs >= parentCreationTimeMs)) - ) - }) - const descendants = windowsDescendantsFromRows(currentRows, rootPid) - if (!descendants) { - return null - } - return { - root: { pid: root.pid, creationTimeMs: root.creationTimeMs }, - descendants: descendants.flatMap((row) => - // A descendant that denied a creation-time query cannot be told from a - // recycled pid later, so it is never signalled on a bare pid. - typeof row.creationTimeMs === 'number' - ? [{ pid: row.pid, creationTimeMs: row.creationTimeMs }] - : [] - ), - unidentifiedCount: descendants.filter((row) => typeof row.creationTimeMs !== 'number').length, - capturedAtMs - } -} - -export type IdentifiedWindowsTreeTerminationDeps = { - readTable?: WindowsDescendantVerificationDeps['readTable'] - terminateTree?: (target: WindowsProcessIdentity) => Promise - ownsRoot?: () => boolean -} - -/** Revalidate the captured root at the last async boundary before taskkill. */ -export async function terminateIdentifiedWindowsProcessTree( - target: WindowsProcessIdentity, - deps: IdentifiedWindowsTreeTerminationDeps = {} -): Promise { - if (!(await verifyWindowsProcessIdentity(target, { readTable: deps.readTable }))) { - return false - } - if (deps.ownsRoot?.() === false) { - return false - } - await ( - deps.terminateTree ?? - ((identified: WindowsProcessIdentity) => terminateWindowsProcessTree(identified.pid)) - )(target) - return true -} - -/** - * Whether a snapshotted Windows tree is gone, polled to a bounded deadline. - * - * Why a verification pass at all: `taskkill /T /F` resolves the same way on a - * timeout, an access denial and a recycled root as it does on a successful - * kill, so its completion is never evidence. Only a table read that no longer - * shows an identity-matched row is. - */ -export async function verifyWindowsDescendantSnapshotExit( - snapshot: WindowsDescendantSnapshot, - deps: WindowsDescendantVerificationDeps = {} -): Promise { - // The most a read can prove: a descendant that denied identification was seen - // and can never be matched gone, so "could not look" caps the verdict. - const proven: DescendantTreeVerdict = snapshot.unidentifiedCount > 0 ? 'unverifiable' : 'exited' - if (snapshot.descendants.length === 0) { - return proven - } - const now = deps.now ?? Date.now - const readTable = deps.readTable ?? readWindowsProcessTableFresh - const deadline = now() + (deps.verifyMs ?? WINDOWS_DESCENDANT_KILL_VERIFY_MS) - let verdict: DescendantTreeVerdict = 'unverifiable' - do { - const table = await readTable().catch(() => null) - if (!table) { - verdict = 'unverifiable' - } else { - const live = new Map(table.map((row) => [row.pid, row.creationTimeMs])) - verdict = snapshot.descendants.some((row) => live.get(row.pid) === row.creationTimeMs) - ? 'live' - : proven - if (verdict === proven) { - return verdict - } - } - if (now() >= deadline) { - return verdict - } - await (deps.wait ?? delay)(WINDOWS_DESCENDANT_POLL_MS) - } while (now() < deadline) - return verdict -} diff --git a/src/main/windows-process-tree-kill.test.ts b/src/main/windows-process-tree-kill.test.ts index 0e6cbb739c85..a2eb82a0d52d 100644 --- a/src/main/windows-process-tree-kill.test.ts +++ b/src/main/windows-process-tree-kill.test.ts @@ -16,9 +16,11 @@ describe('terminateWindowsProcessTree', () => { callback(null) } ) - await terminateWindowsProcessTree(1234, { - execFileImpl: execFileImpl as never - }) + await expect( + terminateWindowsProcessTree(1234, { + execFileImpl: execFileImpl as never + }) + ).resolves.toBe(true) expect(execFileImpl).toHaveBeenCalledWith( 'taskkill', ['/pid', '1234', '/T', '/F'], @@ -30,7 +32,7 @@ describe('terminateWindowsProcessTree', () => { ) }) - it('resolves even when taskkill reports failure (already dead)', async () => { + it('resolves false, never throws, when taskkill reports failure (already dead)', async () => { const execFileImpl = vi.fn( ( _cmd: string, @@ -43,7 +45,7 @@ describe('terminateWindowsProcessTree', () => { ) await expect( terminateWindowsProcessTree(55, { execFileImpl: execFileImpl as never }) - ).resolves.toBeUndefined() + ).resolves.toBe(false) }) it('skips taskkill for invalid pids', async () => { @@ -51,5 +53,7 @@ describe('terminateWindowsProcessTree', () => { await terminateWindowsProcessTree(0, { execFileImpl: execFileImpl as never }) await terminateWindowsProcessTree(-1, { execFileImpl: execFileImpl as never }) expect(execFileImpl).not.toHaveBeenCalled() + // No tree was addressed, so nothing reports it terminated. + await expect(terminateWindowsProcessTree(0)).resolves.toBe(false) }) }) diff --git a/src/main/windows-process-tree-kill.ts b/src/main/windows-process-tree-kill.ts index 31e6b18da2ab..8194948f419e 100644 --- a/src/main/windows-process-tree-kill.ts +++ b/src/main/windows-process-tree-kill.ts @@ -9,7 +9,8 @@ export const WINDOWS_PROCESS_TREE_KILL_TIMEOUT_MS = 5_000 /** * Force-kill a Windows process and every descendant (`taskkill /T /F`). * Best-effort: missing/already-dead roots still resolve so callers can finish - * their own handle cleanup via killRoot. + * their own handle cleanup via killRoot. Resolves true only when taskkill exited 0, + * the one outcome that reports every process in the tree terminated. * * Most main-process taskkills run through here; the families that keep their own * spawn (account-login teardowns, codex app-server deadline, git-command abort, @@ -19,13 +20,13 @@ export const WINDOWS_PROCESS_TREE_KILL_TIMEOUT_MS = 5_000 export function terminateWindowsProcessTree( rootPid: number, deps: { execFileImpl?: typeof execFile; site?: string } = {} -): Promise { +): Promise { if (!Number.isInteger(rootPid) || rootPid <= 0) { - return Promise.resolve() + return Promise.resolve(false) } const site = deps.site ?? 'windows-process-tree-kill' if (!admitSelfInitiatedTreeKill({ pid: rootPid, site, scope: 'win-taskkill-tree' })) { - return Promise.resolve() + return Promise.resolve(false) } const run = deps.execFileImpl ?? execFile return new Promise((resolve) => { @@ -37,8 +38,8 @@ export function terminateWindowsProcessTree( timeout: WINDOWS_PROCESS_TREE_KILL_TIMEOUT_MS, windowsHide: true }, - () => { - resolve() + (error) => { + resolve(error === null) } ) }) diff --git a/src/renderer/src/lib/windows-terminal-capabilities-race.test.ts b/src/renderer/src/lib/windows-terminal-capabilities-race.test.ts index 0439e5c76bfe..f5dd3e36afcf 100644 --- a/src/renderer/src/lib/windows-terminal-capabilities-race.test.ts +++ b/src/renderer/src/lib/windows-terminal-capabilities-race.test.ts @@ -15,19 +15,13 @@ describe('Windows terminal capability probe ordering', () => { vi.unstubAllGlobals() }) - it('does not let an older forced probe overwrite a newer identity proof', async () => { - let resolveOlderStatus!: (status: { hostPlatform: NodeJS.Platform }) => void - let resolveNewerStatus!: (status: { - hostPlatform: NodeJS.Platform - windowsProcessStartTimeAvailable: boolean - }) => void - const olderStatus = new Promise<{ hostPlatform: NodeJS.Platform }>((resolve) => { + it('does not let an older forced probe overwrite a newer answer', async () => { + let resolveOlderStatus!: (status: { hostPlatform: NodeJS.Platform | null }) => void + let resolveNewerStatus!: (status: { hostPlatform: NodeJS.Platform | null }) => void + const olderStatus = new Promise<{ hostPlatform: NodeJS.Platform | null }>((resolve) => { resolveOlderStatus = resolve }) - const newerStatus = new Promise<{ - hostPlatform: NodeJS.Platform - windowsProcessStartTimeAvailable: boolean - }>((resolve) => { + const newerStatus = new Promise<{ hostPlatform: NodeJS.Platform | null }>((resolve) => { resolveNewerStatus = resolve }) const runtimeGetStatus = vi @@ -57,24 +51,13 @@ describe('Windows terminal capability probe ordering', () => { now: 2_000 }) - resolveNewerStatus({ hostPlatform: 'win32', windowsProcessStartTimeAvailable: true }) - await expect(newerProbe).resolves.toMatchObject({ - hostPlatform: 'win32', - windowsProcessStartTimeAvailable: true - }) - expect(getCachedWindowsTerminalCapabilities('local')).toMatchObject({ - hostPlatform: 'win32', - windowsProcessStartTimeAvailable: true - }) + resolveNewerStatus({ hostPlatform: 'win32' }) + await expect(newerProbe).resolves.toMatchObject({ hostPlatform: 'win32' }) + expect(getCachedWindowsTerminalCapabilities('local')).toMatchObject({ hostPlatform: 'win32' }) - resolveOlderStatus({ hostPlatform: 'win32' }) - await expect(olderProbe).resolves.toMatchObject({ - hostPlatform: 'win32', - windowsProcessStartTimeAvailable: true - }) - expect(getCachedWindowsTerminalCapabilities('local')).toMatchObject({ - hostPlatform: 'win32', - windowsProcessStartTimeAvailable: true - }) + // The older probe started first; its late, emptier answer must not replace the newer one. + resolveOlderStatus({ hostPlatform: null }) + await expect(olderProbe).resolves.toMatchObject({ hostPlatform: 'win32' }) + expect(getCachedWindowsTerminalCapabilities('local')).toMatchObject({ hostPlatform: 'win32' }) }) }) diff --git a/src/renderer/src/lib/windows-terminal-capabilities.test.ts b/src/renderer/src/lib/windows-terminal-capabilities.test.ts index 7e7568f2e09a..85c2a9714436 100644 --- a/src/renderer/src/lib/windows-terminal-capabilities.test.ts +++ b/src/renderer/src/lib/windows-terminal-capabilities.test.ts @@ -69,7 +69,6 @@ function stubTerminalCapabilityApi(args: { wslDistros?: string[] gitBashAvailable?: boolean hostPlatform?: NodeJS.Platform | null - windowsProcessStartTimeAvailable?: boolean }): { wslIsAvailable: ReturnType wslListDistros: ReturnType @@ -82,10 +81,7 @@ function stubTerminalCapabilityApi(args: { const pwshIsAvailable = vi.fn().mockResolvedValue(args.pwshAvailable) const isGitBashAvailable = vi.fn().mockResolvedValue(args.gitBashAvailable ?? false) const runtimeGetStatus = vi.fn().mockResolvedValue({ - hostPlatform: 'hostPlatform' in args ? args.hostPlatform : 'win32', - ...(args.windowsProcessStartTimeAvailable !== undefined - ? { windowsProcessStartTimeAvailable: args.windowsProcessStartTimeAvailable } - : {}) + hostPlatform: 'hostPlatform' in args ? args.hostPlatform : 'win32' }) vi.stubGlobal('window', { @@ -590,8 +586,7 @@ describe('windows terminal capabilities', () => { const { wslIsAvailable, wslListDistros } = stubTerminalCapabilityApi({ wslAvailable: false, pwshAvailable: true, - wslDistros: [], - windowsProcessStartTimeAvailable: true + wslDistros: [] }) wslIsAvailable.mockResolvedValueOnce(false).mockResolvedValue(true) wslListDistros.mockResolvedValueOnce([]).mockResolvedValue(['Ubuntu']) diff --git a/src/renderer/src/lib/windows-terminal-capabilities.ts b/src/renderer/src/lib/windows-terminal-capabilities.ts index 8a2f468b134d..3818ce8733bb 100644 --- a/src/renderer/src/lib/windows-terminal-capabilities.ts +++ b/src/renderer/src/lib/windows-terminal-capabilities.ts @@ -11,8 +11,6 @@ export type WindowsTerminalCapabilities = { pwshAvailable: boolean gitBashAvailable: boolean hostPlatform: NodeJS.Platform | null - /** Host-owned PID-reuse proof; absent means the host did not advertise it. */ - windowsProcessStartTimeAvailable?: boolean isLoading: boolean } diff --git a/src/renderer/src/lib/windows-terminal-capability-read.ts b/src/renderer/src/lib/windows-terminal-capability-read.ts index 9a77538cefc5..345181452c01 100644 --- a/src/renderer/src/lib/windows-terminal-capability-read.ts +++ b/src/renderer/src/lib/windows-terminal-capability-read.ts @@ -66,9 +66,6 @@ export async function readWindowsTerminalCapabilities( pwshAvailable, gitBashAvailable, hostPlatform: runtimeStatus?.hostPlatform ?? null, - ...(runtimeStatus?.windowsProcessStartTimeAvailable !== undefined - ? { windowsProcessStartTimeAvailable: runtimeStatus.windowsProcessStartTimeAvailable } - : {}), isLoading: false } } diff --git a/src/renderer/src/lib/windows-terminal-capability-reprobe.test.ts b/src/renderer/src/lib/windows-terminal-capability-reprobe.test.ts index 3d3d476753e9..f2361c93076c 100644 --- a/src/renderer/src/lib/windows-terminal-capability-reprobe.test.ts +++ b/src/renderer/src/lib/windows-terminal-capability-reprobe.test.ts @@ -40,36 +40,18 @@ afterEach(() => { }) describe('windows terminal capability re-probe', () => { - it('reprobes usable WSL until Windows process identity is proved', async () => { + it('settles usable WSL on a Windows host without waiting for process identity', async () => { vi.useFakeTimers() - let current: WindowsTerminalCapabilities = USABLE_WSL - const probe = vi.fn(async () => { - current = { ...current, windowsProcessStartTimeAvailable: true } - return current - }) - const readCached = () => current + const { probe, readCached } = createWatcher([USABLE_WSL]) startWindowsTerminalCapabilityReprobe({ ownerKey: 'local', probe, readCached }) await vi.advanceTimersByTimeAsync(30_000) expect(probe).toHaveBeenCalledTimes(1) - expect(readCached().windowsProcessStartTimeAvailable).toBe(true) await vi.advanceTimersByTimeAsync(30 * 60_000) expect(probe).toHaveBeenCalledTimes(1) }) - it('resets the backoff when only process identity capability changes', async () => { - vi.useFakeTimers() - const identityAvailable = { ...ABSENT_WSL, windowsProcessStartTimeAvailable: true } - const { probe, readCached } = createWatcher([identityAvailable, identityAvailable]) - startWindowsTerminalCapabilityReprobe({ ownerKey: 'local', probe, readCached }) - - await vi.advanceTimersByTimeAsync(30_000) - expect(probe).toHaveBeenCalledTimes(1) - await vi.advanceTimersByTimeAsync(30_000) - expect(probe).toHaveBeenCalledTimes(2) - }) - it('backs off to a five-minute ceiling on a stable answer', async () => { vi.useFakeTimers() const { probe, readCached } = createWatcher() @@ -85,9 +67,7 @@ describe('windows terminal capability re-probe', () => { it('still re-checks a transient absent answer, then stops once WSL answers', async () => { vi.useFakeTimers() - const { probe, readCached } = createWatcher([ - { ...USABLE_WSL, windowsProcessStartTimeAvailable: true } - ]) + const { probe, readCached } = createWatcher([USABLE_WSL]) startWindowsTerminalCapabilityReprobe({ ownerKey: 'local', probe, readCached }) await vi.advanceTimersByTimeAsync(30_000) diff --git a/src/renderer/src/lib/windows-terminal-capability-reprobe.ts b/src/renderer/src/lib/windows-terminal-capability-reprobe.ts index f9b9d44b0256..4cda89d9bf82 100644 --- a/src/renderer/src/lib/windows-terminal-capability-reprobe.ts +++ b/src/renderer/src/lib/windows-terminal-capability-reprobe.ts @@ -31,19 +31,15 @@ function capabilitySignature(capabilities: WindowsTerminalCapabilities): string capabilities.wslDistros.join('\u0000'), capabilities.pwshAvailable, capabilities.gitBashAvailable, - capabilities.hostPlatform ?? '', - capabilities.windowsProcessStartTimeAvailable + capabilities.hostPlatform ?? '' ].join('|') } -/** A usable WSL is settled only after Windows hosts also prove PID identity. */ +/** A usable WSL is settled once the host's platform is known. */ function isSettled(capabilities: WindowsTerminalCapabilities): boolean { if (!capabilities.wslAvailable || capabilities.wslDistros.length === 0) { return false } - if (capabilities.hostPlatform === 'win32') { - return capabilities.windowsProcessStartTimeAvailable === true - } // A missing platform means the status probe may have failed; keep checking until it recovers. return capabilities.hostPlatform !== null } diff --git a/src/shared/runtime-session-contracts.ts b/src/shared/runtime-session-contracts.ts index c5b66426c62f..02c239b220bb 100644 --- a/src/shared/runtime-session-contracts.ts +++ b/src/shared/runtime-session-contracts.ts @@ -79,7 +79,8 @@ export type RuntimeStatus = { worktreeCreateIdempotency?: { dedupeTtlMs: number } - /** True only when this Windows host can prove process creation times for PID ownership. */ + /** True only when this Windows host can read process creation times. TEMPORARY: read only by + * older clients, which keep re-probing WSL until it is true; remove after their window. */ windowsProcessStartTimeAvailable?: boolean /** * Optional for mixed-version peers. Absence means the host predates structured From 6935e0de2dab3cf2933c18aaf6ea34fa595fd26f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:32:54 -0700 Subject: [PATCH 2/5] fix(native-chat): treat a Windows Claude exit after stdin end as a proven close On Windows an idle Claude leaves on its own once its stdin ends, so every Stop and close read as unproven: live background work settled as unknown and the trace logged a close that "did not finish cleanly". As with the Codex close, that exit is now the close and Orca makes no claim about processes Claude started; a forced close still rests on taskkill's own report. Also drop the Settings clause about Windows needing process start times, and fix comments and the tracked process-enumeration doc that still described the removed start-time gate and descendant snapshot. --- docs/reference/windows-process-enumeration.md | 13 +- src/main/agent-launch/agent-launch-mode.ts | 2 +- .../claude/claude-agent-sdk-exit-proof.ts | 20 ++-- .../claude-child-exit-proof-ladder.test.ts | 113 +++++++++++++++++- .../claude/claude-child-exit-proof-ladder.ts | 6 + src/main/claude/claude-child-work-decoder.ts | 4 +- .../claude-structured-acquisition-release.ts | 4 +- .../claude-structured-session-acquisition.ts | 2 +- .../claude/claude-structured-session-close.ts | 5 +- .../rpc/methods/structured-agent-session.ts | 4 +- src/main/windows/windows-process-table.ts | 5 +- .../settings/ExperimentalPane.test.tsx | 2 +- .../NativeChatExperimentalSetting.tsx | 2 +- src/renderer/src/i18n/locales/en.json | 2 +- src/renderer/src/i18n/locales/es.json | 2 +- src/renderer/src/i18n/locales/fr.json | 2 +- src/renderer/src/i18n/locales/ja.json | 2 +- src/renderer/src/i18n/locales/ko.json | 2 +- src/renderer/src/i18n/locales/zh.json | 2 +- .../lib/launch-structured-agent-session.ts | 5 +- 20 files changed, 158 insertions(+), 41 deletions(-) diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index affa11f7e4e8..432a1fe4690d 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -374,10 +374,9 @@ relay uses the scan and the WMI launch fallback. `node_addon_api.gyp` resolves outside the repo and hourly Windows builds die at configure. `node-pty` is patched the same way for the same reason. 4. **No PEB reads, no `PROCESS_VM_READ`.** See below. -5. **The `CreationTime` flag (4).** Upstream exposes no process start time, and - `isWindowsProcessStartTimeAvailable()` gates structured Claude and Codex - chat on it, so without this change win32 silently fell back to the legacy - transcript path. `GetProcessCreationTime` opens +5. **The `CreationTime` flag (4).** Upstream exposes no process start time. + Structured Claude and Codex chat no longer depend on it; the owner probe and + the orcad runtime preflight still read it. `GetProcessCreationTime` opens `PROCESS_QUERY_LIMITED_INFORMATION` and converts `GetProcessTimes`' FILETIME to Unix ms; a process that denies the handle is emitted with the field absent, never zero, because callers must be able to tell "cannot identify" @@ -395,9 +394,9 @@ relay uses the scan and the WMI launch fallback. so itself. Two readers depend on it. `isWindowsProcessStartTimeAvailable()` returns - false unless this bit is set, because claiming otherwise leaves - `captureWindowsDescendantSnapshot` returning null forever while structured - chat believes it has a reaper. And `windows-process-tree-creation-time.cjs` + false unless this bit is set, so the owner probe never scans the whole table + for times it cannot get (the Windows Claude descendant snapshot that once + relied on it is gone). And `windows-process-tree-creation-time.cjs` asserts it during install, which is what forces a from-source rebuild — the same role `node-pty-job-ownership.cjs` plays for node-pty's job exports. diff --git a/src/main/agent-launch/agent-launch-mode.ts b/src/main/agent-launch/agent-launch-mode.ts index 0ef01c01da06..bfa301aaa29b 100644 --- a/src/main/agent-launch/agent-launch-mode.ts +++ b/src/main/agent-launch/agent-launch-mode.ts @@ -213,7 +213,7 @@ async function readStructuredCreateSupport( /** * Applies the executing host's `agentSession.createSupport` answer, which is the authority on WSL, - * remoteness and the Windows process-start-time gate for the resolved workspace. + * remoteness and per-agent support for the resolved workspace. */ export function downgradeAgentLaunchModeForHost( receipt: AgentLaunchModeReceipt, diff --git a/src/main/claude/claude-agent-sdk-exit-proof.ts b/src/main/claude/claude-agent-sdk-exit-proof.ts index 6b808f050213..bc82d33bcfb9 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.ts @@ -67,9 +67,9 @@ export type ClaudeChildTreeReaper = { */ reap(): Promise /** - * `unverifiable` until a reap observes otherwise. `exited` is the only verdict - * that proves a close; `live` names a descendant that was seen still running, - * which no later caller may collapse into "unknown". + * `unverifiable` until a reap answers otherwise. `exited` is observed on POSIX + * and is taskkill's own report on Windows; `live` names a descendant that was + * seen still running, which no later caller may collapse into "unknown". */ readonly treeVerdict: DescendantTreeVerdict } @@ -248,7 +248,7 @@ export function createClaudeChildTreeReaper( return terminated ? 'exited' : 'unverifiable' } - /** The only source of a tree verdict: every `exited` here is an observation. */ + /** The only source of a tree verdict: `exited` is observed on POSIX, taskkill's report on Windows. */ async function judgeTree(): Promise { const killRoot = (): boolean => terminateClaudeRoot({ child, exited }) const rootPid = child.pid @@ -321,13 +321,15 @@ export function createClaudeChildTreeReaper( * Orca's own shutdown ladder on the child it spawned, kept because the SDK's * close path returns no proof and Orca never releases a lease on an assumed exit. * - * Resolves true only after the child actually emitted exit and its snapshotted - * descendants were observed gone; false is unproven. A root that left on its - * own before a snapshot could be armed stays unproven: its descendants had - * already reparented out of reach when the ladder first looked. + * Resolves true only after the child actually emitted exit and, on POSIX, its + * snapshotted descendants were observed gone; on Windows, after it left on its + * own once its stdin ended, or a forced `taskkill /T /F` reported its tree + * terminated. False is unproven. On POSIX a root that left on its own before a + * snapshot could be armed stays unproven: its descendants had already + * reparented out of reach when the ladder first looked. */ export function proveClaudeChildExit(input: ClaudeChildExitProofInput): Promise { return proveClaudeChildExitWithReaper(input, () => - createClaudeChildTreeReaper(input.child, { exited: input.exited }) + createClaudeChildTreeReaper(input.child, { exited: input.exited, platform: input.platform }) ) } diff --git a/src/main/claude/claude-child-exit-proof-ladder.test.ts b/src/main/claude/claude-child-exit-proof-ladder.test.ts index 6a74a0b0c6b6..b14965fba1c0 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.test.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' -import type { ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' +import { + createClaudeChildTreeReaper, + type ClaudeChildTreeReaper +} from './claude-agent-sdk-exit-proof' import { proveClaudeChildExitWithReaper } from './claude-child-exit-proof-ladder' function fakeTree(): ClaudeChildTreeReaper & { reap: ReturnType } { @@ -41,6 +44,53 @@ function rootStoppedBySigterm(stopMs: number) { } } +/** A Windows root that leaves on stdin end when `leavesOnStdinEnd`, otherwise only once killed. */ +function windowsRoot(leavesOnStdinEnd: boolean) { + let exited = false + let settle = (): void => {} + const exitPromise = new Promise((resolve) => { + settle = resolve + }) + const leave = (): void => { + exited = true + settle() + } + const kill = vi.fn(() => { + leave() + return true + }) + const stdin = { + end: vi.fn(() => { + if (leavesOnStdinEnd) { + leave() + } + }) + } + return { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The ladder reads only pid, kill and stdin.end from its child. + child: { pid: 4321, kill, stdin } as unknown as Parameters< + typeof proveClaudeChildExitWithReaper + >[0]['child'], + kill, + exitPromise, + exited: () => exited + } +} + +function windowsTree( + root: ReturnType, + terminateWindowsTree: (rootPid: number) => Promise +) { + const captureDescendants = vi.fn(async () => null) + const tree = createClaudeChildTreeReaper(root.child, { + platform: 'win32', + exited: root.exited, + captureDescendants, + terminateWindowsTree + }) + return { tree, reap: vi.spyOn(tree, 'reap'), captureDescendants } +} + describe('Claude child exit proof ladder', () => { it('stops a supervised child with SIGTERM and waits out the supervisor stop before forcing', async () => { // Slower than the unsupervised 1.5 s grace, still inside the supervisor's own bound. @@ -69,4 +119,65 @@ describe('Claude child exit proof ladder', () => { expect(root.kill).not.toHaveBeenCalledWith('SIGTERM') expect(tree.reap).toHaveBeenCalled() }, 10_000) + + it('on Windows proves a close when Claude leaves on its own after its stdin ends', async () => { + const root = windowsRoot(true) + const terminateWindowsTree = vi.fn(async () => true) + const { tree, reap, captureDescendants } = windowsTree(root, terminateWindowsTree) + + await expect( + proveClaudeChildExitWithReaper({ ...root, tree, platform: 'win32' }, () => tree) + ).resolves.toBe(true) + + // No claim about what Claude started: nothing is read, reaped or taskkilled after it left. + expect(reap).not.toHaveBeenCalled() + expect(terminateWindowsTree).not.toHaveBeenCalled() + expect(captureDescendants).not.toHaveBeenCalled() + expect(root.kill).not.toHaveBeenCalled() + expect(tree.treeVerdict).toBe('unverifiable') + }) + + it.each([ + { taskkill: true, proven: true }, + { taskkill: false, proven: false } + ])( + 'on Windows a forced close is proven only by taskkill: taskkill $taskkill', + async ({ taskkill, proven }) => { + const root = windowsRoot(false) + const terminateWindowsTree = vi.fn(async () => taskkill) + const { tree } = windowsTree(root, terminateWindowsTree) + + await expect( + proveClaudeChildExitWithReaper({ ...root, tree, platform: 'win32' }, () => tree) + ).resolves.toBe(proven) + + expect(terminateWindowsTree).toHaveBeenCalledWith(4321) + // The root still leaves through its held handle whatever taskkill reported. + expect(root.exited()).toBe(true) + }, + 10_000 + ) + + it('on POSIX still reaps a root that left on its own and keeps the tree verdict', async () => { + let exited = false + const stdin = { + end: vi.fn(() => { + exited = true + }) + } + const tree = { ...fakeTree(), treeVerdict: 'unverifiable' as const } + const input = { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The ladder reads only pid, kill and stdin.end from its child. + child: { pid: 4321, kill: vi.fn(), stdin } as unknown as Parameters< + typeof proveClaudeChildExitWithReaper + >[0]['child'], + exitPromise: Promise.resolve(), + exited: () => exited, + tree, + platform: 'linux' as const + } + + await expect(proveClaudeChildExitWithReaper(input, () => tree)).resolves.toBe(false) + expect(tree.reap).toHaveBeenCalledOnce() + }) }) diff --git a/src/main/claude/claude-child-exit-proof-ladder.ts b/src/main/claude/claude-child-exit-proof-ladder.ts index 41015b0192f0..0a27e725a50f 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.ts @@ -15,6 +15,7 @@ export type ClaudeChildExitProofInput = { tree?: ClaudeChildTreeReaper /** The child is the POSIX provider supervisor: SIGTERM stops Claude, which reaps its tools. */ supervised?: boolean + platform?: NodeJS.Platform } export async function proveClaudeChildExitWithReaper( @@ -47,6 +48,11 @@ export async function proveClaudeChildExitWithReaper( await waitForProcessExitUntil(input.exitPromise, FORCED_EXIT_MS) } } + // On Windows, as with the Codex close, Claude leaving on its own after its stdin ends is the + // close: Orca makes no claim about processes Claude started, and no reap could address them. + if (!reaped && input.exited() && (input.platform ?? process.platform) === 'win32') { + return true + } if (!reaped && input.exited() && tree.treeVerdict !== 'exited') { await tree.reap() } diff --git a/src/main/claude/claude-child-work-decoder.ts b/src/main/claude/claude-child-work-decoder.ts index e6af733b6fd4..ce975e8d39fa 100644 --- a/src/main/claude/claude-child-work-decoder.ts +++ b/src/main/claude/claude-child-work-decoder.ts @@ -4,7 +4,7 @@ // `task_notification`; nothing else ends it. A roster (`background_tasks_changed`), a turn ending // or a spawn call returning is the parent's view of the child, not the child's, and the CLI sends // every child its own terminal frame, so none of them settles one. When Orca ends the session and -// proves its tree gone, what is still live is stopped (`stopLive`); any other end leaves it for the +// proves the close, what is still live is stopped (`stopLive`); any other end leaves it for the // host to settle as unknown. A live child blocked on a permission request reads waiting; no task // frame says so, so the caller hands over which children a request blocks. Edges wait here until // the frame is journaled, then take the host clock. @@ -142,7 +142,7 @@ export class ClaudeChildWorkDecoder { } } - /** Orca ended the session and proved its process tree gone: what still ran is stopped. The + /** Orca ended the session and proved the close: what still ran is stopped. The * ending is Orca's, not the child's, so a frame of the child's own still replaces it. */ stopLive(): void { for (const id of this.live.keys()) { diff --git a/src/main/claude/claude-structured-acquisition-release.ts b/src/main/claude/claude-structured-acquisition-release.ts index d29879bb73ee..d8f0a1afd9f5 100644 --- a/src/main/claude/claude-structured-acquisition-release.ts +++ b/src/main/claude/claude-structured-acquisition-release.ts @@ -12,8 +12,8 @@ import type { /** * Cleanup for an acquisition the host could not commit or prove. A session that * a first-hand exit already removed is not an absence to report as proven: the - * ladder on its connection still answers, and that answer is classified exactly - * as a start-time failure would be. + * ladder on its connection still answers, and that answer is classified like + * any other unproven acquisition cleanup. */ export async function releaseClaudeAcquisition(input: { sessionId: string diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index 39d2e0cdbe2f..1943e80f0ef2 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -228,7 +228,7 @@ export async function acquireClaudeSession({ { ...input, pid: connection.pid }, deps.readProcessStartTime ).catch((error: unknown) => { - // A child that already ended explains why its start time could not be read. + // A child that already ended explains a missing pid or a failed read. throw childEnded ?? error }) acquisitions.assertCurrent(sessionId, attempt) diff --git a/src/main/claude/claude-structured-session-close.ts b/src/main/claude/claude-structured-session-close.ts index 903f49f052e7..4f0b3712f83e 100644 --- a/src/main/claude/claude-structured-session-close.ts +++ b/src/main/claude/claude-structured-session-close.ts @@ -118,8 +118,9 @@ async function finalizeClaudePublishedSession( rootExitVerdict = cleanupError } // Queues the session's ending for the host's child records; the adapter delivers it after close. - // A close that proved the whole tree gone stopped what still ran. One that saw a descendant - // survive, like an exit of the session's own, leaves how it ended unknown. + // A proven close stopped what still ran: on POSIX the whole tree was seen gone; on Windows Claude + // left after its stdin ended, or taskkill reported its tree terminated. Any other end, like an + // exit of the session's own, leaves how it ended unknown. if (connectionClosed === true) { session.childWork.stopLive() } diff --git a/src/main/runtime/rpc/methods/structured-agent-session.ts b/src/main/runtime/rpc/methods/structured-agent-session.ts index e0d24119086c..dada429977a1 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.ts @@ -71,8 +71,8 @@ import { sendStructuredAgentSessionForClient } from './structured-agent-session- /** * The attach-shaped entries take the location from the client instead of resolving it from a * worktree, so they never reach the worktree-resolving create-support check. Ask the executing - * host the same question directly: the answer includes host-measured facts the client cannot see - * or forge, such as whether this machine can read a provider child's process start time. + * host the same question directly: only it knows which agents and locations it runs, and a client + * cannot forge that answer. */ async function resolveClientSuppliedAttach(params: z.infer, ctx: RpcContext) { await ensureHostInstalled(ctx) diff --git a/src/main/windows/windows-process-table.ts b/src/main/windows/windows-process-table.ts index 20f1ca000c24..433771c81ff6 100644 --- a/src/main/windows/windows-process-table.ts +++ b/src/main/windows/windows-process-table.ts @@ -533,9 +533,8 @@ export function isWindowsProcessTableAvailable(): boolean { * `build/Release/` path, so a host can hold a patched `lib/index.js` — enum and * all — over a binary that ignores flag 4. CI produced exactly that: the enum * said available, and every row came back without `creationTimeMs`. Answering - * true there is worse than answering false: the descendant snapshot then - * returns null forever and the exit proof latches `unverifiable`, while - * structured chat believes it has a reaper. + * true there is worse than answering false: the owner probe would scan the + * whole table for nulls, and the published status would claim start times. */ export function isWindowsProcessStartTimeAvailable(): boolean { const native = moduleLoader() diff --git a/src/renderer/src/components/settings/ExperimentalPane.test.tsx b/src/renderer/src/components/settings/ExperimentalPane.test.tsx index 6513e9de22b7..8325045138da 100644 --- a/src/renderer/src/components/settings/ExperimentalPane.test.tsx +++ b/src/renderer/src/components/settings/ExperimentalPane.test.tsx @@ -265,7 +265,7 @@ describe('ExperimentalPane', () => { expect(container.textContent).toContain('Chats that already exist stay as they are.') // Paired Orca servers run structured chats too; only WSL and SSH stay on terminal chat. expect(container.textContent).toContain( - 'Runs on this machine and on paired Orca servers running a version that supports it; older servers keep terminal chat. WSL and SSH hosts continue to use terminal chat, and Windows falls back to it unless Orca can read process start times.' + 'Runs on this machine and on paired Orca servers running a version that supports it; older servers keep terminal chat. WSL and SSH hosts continue to use terminal chat.' ) expect(container.textContent).toContain('Default view') root.unmount() diff --git a/src/renderer/src/components/settings/NativeChatExperimentalSetting.tsx b/src/renderer/src/components/settings/NativeChatExperimentalSetting.tsx index 6c85a178965e..2e1fef30a217 100644 --- a/src/renderer/src/components/settings/NativeChatExperimentalSetting.tsx +++ b/src/renderer/src/components/settings/NativeChatExperimentalSetting.tsx @@ -141,7 +141,7 @@ export function NativeChatExperimentalSetting({

{translate( 'auto.components.settings.ExperimentalPane.nativeChat.structuredScope', - 'Runs on this machine and on paired Orca servers running a version that supports it; older servers keep terminal chat. WSL and SSH hosts continue to use terminal chat, and Windows falls back to it unless Orca can read process start times.' + 'Runs on this machine and on paired Orca servers running a version that supports it; older servers keep terminal chat. WSL and SSH hosts continue to use terminal chat.' )}

diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 4e69e42b6391..7b9ae20cf825 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -7349,7 +7349,7 @@ "defaultViewNative": "Chat UI", "structuredTitle": "Use updated structured native chat", "structuredCopy": "Open new Codex and Claude agents as structured chats. Off opens them in the terminal-backed chat. Chats that already exist stay as they are.", - "structuredScope": "Runs on this machine and on paired Orca servers running a version that supports it; older servers keep terminal chat. WSL and SSH hosts continue to use terminal chat, and Windows falls back to it unless Orca can read process start times.", + "structuredScope": "Runs on this machine and on paired Orca servers running a version that supports it; older servers keep terminal chat. WSL and SSH hosts continue to use terminal chat.", "structuredToggleLabel": "Toggle updated structured native chat", "resumeTitle": "Resume working chats automatically after a restart", "resumeCopy": "When Orca quits or installs an update, chats that were working are automatically resumed when Orca is reopened.", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 6d2694fdfce5..c44d834a9335 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -7136,7 +7136,7 @@ "resumeCopy": "Cuando Orca se cierra o instala una actualización, los chats que estaban trabajando se reanudan automáticamente al volver a abrir Orca.", "structuredTitle": "Utilice el chat nativo estructurado actualizado", "structuredCopy": "Abra nuevos agentes de Codex y Claude como chats estructurados. Off los abre en el chat respaldado por la terminal. Los chats que ya existen permanecen como están.", - "structuredScope": "Se ejecuta en esta máquina y en servidores Orca emparejados que ejecutan una versión que lo admita; Los servidores más antiguos mantienen el chat del terminal. Los hosts WSL y SSH continúan usando el chat de terminal y Windows recurre a él a menos que Orca pueda leer las horas de inicio del proceso.", + "structuredScope": "Se ejecuta en esta máquina y en servidores Orca emparejados que ejecutan una versión que lo admita; Los servidores más antiguos mantienen el chat del terminal. Los hosts WSL y SSH continúan usando el chat de terminal.", "structuredToggleLabel": "Alternar chat nativo estructurado actualizado", "resumeTitle": "Reanudar los chats de trabajo automáticamente después de reiniciar", "resumeToggleLabel": "Alternar reanudación automática después de un reinicio" diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 6a7ecc60d859..d230de030a4c 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -7238,7 +7238,7 @@ "shellEnvToggleLabel": "Activer l'utilisation de l'environnement de votre shell", "structuredTitle": "Utiliser le chat natif structuré mis à jour", "structuredCopy": "Ouvre les nouveaux agents Codex et Claude sous forme de discussions structurées. Désactivé, ils s'ouvrent dans le chat basé sur le terminal. Les discussions existantes restent telles quelles.", - "structuredScope": "Fonctionne sur cette machine et sur les serveurs Orca appairés dont la version le prend en charge ; les serveurs plus anciens conservent le chat du terminal. WSL et les hôtes SSH continuent d'utiliser le chat du terminal, et Windows y revient à moins qu'Orca ne puisse lire les heures de début des processus.", + "structuredScope": "Fonctionne sur cette machine et sur les serveurs Orca appairés dont la version le prend en charge ; les serveurs plus anciens conservent le chat du terminal. WSL et les hôtes SSH continuent d'utiliser le chat du terminal.", "structuredToggleLabel": "Basculer le chat natif structuré mis à jour", "resumeTitle": "Reprendre automatiquement les discussions de travail après un redémarrage", "resumeCopy": "Lorsqu'Orca quitte ou installe une mise à jour, les discussions en cours reprennent automatiquement à la réouverture d'Orca.", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 991435c31a65..dbb2a7a2fb55 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -7120,7 +7120,7 @@ "shellEnvToggleLabel": "シェル環境の使用の切り替え", "structuredTitle": "更新された構造化ネイティブチャットを使用する", "structuredCopy": "新規の Codex および Claude の Agent を構造化チャットとして開きます。オフにすると、ターミナルを使用したチャットで開きます。既存のチャットはそのまま残ります。", - "structuredScope": "このマシンと、対応するバージョンを実行しているペアリング済みの Orca サーバーで動作します。古いサーバーはターミナルチャットのままです。WSL および SSH ホストは引き続きターミナルチャットを使用し、Orca がプロセスの開始時間を読み取ることができない限り、Windows はターミナルチャットにフォールバックします。", + "structuredScope": "このマシンと、対応するバージョンを実行しているペアリング済みの Orca サーバーで動作します。古いサーバーはターミナルチャットのままです。WSL および SSH ホストは引き続きターミナルチャットを使用します。", "structuredToggleLabel": "更新された構造化ネイティブチャットの切り替え", "resumeTitle": "再起動後に作業中のチャットを自動的に再開する", "resumeCopy": "Orca が終了するかアップデートをインストールすると、Orca が再度開かれたときに作業中だったチャットが自動的に再開されます。", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 346317e1b171..3e5fd81570f1 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -7120,7 +7120,7 @@ "shellEnvToggleLabel": "셸 환경 사용 전환", "structuredTitle": "업데이트된 구조화된 기본 채팅 사용", "structuredCopy": "새 Codex 및 Claude 에이전트를 구조화된 채팅으로 엽니다. 끄면 터미널 지원 채팅으로 엽니다. 이미 있는 채팅은 그대로 유지됩니다.", - "structuredScope": "이 컴퓨터와 이를 지원하는 버전을 실행하는 페어링된 Orca 서버에서 실행됩니다. 이전 서버는 터미널 채팅을 유지합니다. WSL 및 SSH 호스트는 계속해서 터미널 채팅을 사용하며 Orca가 프로세스 시작 시간을 읽을 수 없으면 Windows는 이 채팅을 사용합니다.", + "structuredScope": "이 컴퓨터와 이를 지원하는 버전을 실행하는 페어링된 Orca 서버에서 실행됩니다. 이전 서버는 터미널 채팅을 유지합니다. WSL 및 SSH 호스트는 계속해서 터미널 채팅을 사용합니다.", "structuredToggleLabel": "업데이트된 구조화된 기본 채팅 전환", "resumeTitle": "다시 시작한 후 작업 중인 채팅을 자동으로 재개합니다.", "resumeCopy": "Orca가 종료되거나 업데이트를 설치하면 Orca가 다시 열릴 때 작업 중이던 채팅이 자동으로 재개됩니다.", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 31deaef11e10..83e3c0454ae9 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -7120,7 +7120,7 @@ "shellEnvToggleLabel": "切换使用 shell 环境", "structuredTitle": "使用更新的结构化本机聊天", "structuredCopy": "将新的 Codex 和 Claude 代理作为结构化聊天打开。关闭时会在终端支持的聊天中打开。已有的聊天保持不变。", - "structuredScope": "在本机和运行支持该功能版本的已配对 Orca 服务器上运行；较旧的服务器保留终端聊天。WSL 和 SSH 主机继续使用终端聊天，Windows 会回退到它，除非 Orca 可以读取进程启动时间。", + "structuredScope": "在本机和运行支持该功能版本的已配对 Orca 服务器上运行；较旧的服务器保留终端聊天。WSL 和 SSH 主机继续使用终端聊天。", "structuredToggleLabel": "切换更新的结构化本机聊天", "resumeTitle": "重启后自动恢复工作聊天", "resumeCopy": "当 Orca 退出或安装更新时，重新打开 Orca 后，正在工作的聊天会自动恢复。", diff --git a/src/renderer/src/lib/launch-structured-agent-session.ts b/src/renderer/src/lib/launch-structured-agent-session.ts index 0f807928d3d7..1762d88d20d8 100644 --- a/src/renderer/src/lib/launch-structured-agent-session.ts +++ b/src/renderer/src/lib/launch-structured-agent-session.ts @@ -232,9 +232,8 @@ export function abandonStructuredAgentSessionLaunchIntent( } /** - * Only the host that will execute the session can answer whether it supports creating one there — - * on Windows that means reading the provider child's process start time, which a client cannot - * observe. Both providers ask: the host classifies per agent, and Codex inherits the + * Only the host that will execute the session can answer whether it supports creating one there. + * Both providers ask: the host classifies per agent, and Codex inherits the * unresolvable-selector retry above along with the probe. The unknown branch stays on the chat for * reconciliation: a retry may follow a create whose reply was lost. Answers the seed create will use. */ From a9f2e8c4d57f104a038672c906324f9f6b677533 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:03:44 -0700 Subject: [PATCH 3/5] fix(native-chat): renew a held child's lease without a PID probe An owner recorded without a process start time could never renew: the renewer re-proves every live owner by PID identity, and with no start time and no spawn-token echo that probe is indeterminate. The lease's last renewal then stayed at the spawn, so a turn cut by an Orca crash was dated to its own start, and every tick logged a failed renewal and split the batch into one store transaction per chat. The runtime now renews a lease for a child it still holds at the record's fence and whose exit it has not received, recorded as a `held-child` match. Receipt of the exit ends that proof before the exit is settled, and a restart holds no child, so a dead owner's lease still expires and restart adjudication is unchanged. Records this runtime does not hold keep the PID probe. Also correct the identity probe's comment about shipped addons and note that recovery's stop ladder is POSIX-only. --- .../structured-agent-session-child-exit.ts | 14 +- ...t-session-held-child-lease-renewal.test.ts | 242 ++++++++++++++++++ ...ctured-agent-session-host-runtime-state.ts | 4 +- .../structured-agent-session-host-types.ts | 2 + .../structured-agent-session-host.ts | 11 +- .../structured-agent-session-lease-renewer.ts | 22 +- ...structured-agent-session-provider-child.ts | 21 ++ ...tured-agent-session-recovery-resolution.ts | 5 +- .../agent-session-lease-transitions.ts | 5 +- .../agent-session-process-identity-probe.ts | 10 +- .../agent-session-lease-adjudication.ts | 9 +- 11 files changed, 328 insertions(+), 17 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts index c606a73e310f..84b636da49b6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts @@ -12,7 +12,10 @@ import type { StructuredAgentSessionHostSession, StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' -import { endProviderChild } from './structured-agent-session-provider-child' +import { + endProviderChild, + markProviderChildExited +} from './structured-agent-session-provider-child' import { releaseStoredStructuredAgentSessionOwnerAfterExit, type StructuredAgentSessionLeaseStore @@ -71,6 +74,14 @@ export function settleStructuredAgentSessionChildExit< context: StructuredAgentSessionChildExitContext, event: StructuredAgentSessionEndedEvent ): Promise { + // Before the queue: lease renewal must not vouch for a child whose exit is only waiting its turn. + const exited = context.sessions.get(event.sessionId) + if (exited) { + markProviderChildExited(exited, { + generation: event.acquisitionGeneration, + fence: event.fence + }) + } return context.serialize(event.sessionId, async () => { const child = context.sessions.get(event.sessionId)?.child if (!child || child.fence !== event.fence || child.generation !== event.acquisitionGeneration) { @@ -99,6 +110,7 @@ export async function endExitedStructuredAgentSessionChildUnderSerialize< if (!session || session.child !== child) { return } + markProviderChildExited(session, child) const { expected } = exit const close = child.close // Receipt of the exit is the one end time the host may record for a running turn. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts new file mode 100644 index 000000000000..cabab0d2cc9d --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts @@ -0,0 +1,242 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { codexProviderHandle } from '../../../shared/agent-session-provider-handle-encoding' +import { probeAgentSessionProcessIdentity } from '../../runtime/agent-session-process-identity-probe' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { + settleStructuredAgentSessionChildExit, + type StructuredAgentSessionChildExitContext, + type StructuredAgentSessionChildExitSession +} from './structured-agent-session-child-exit' +import type { StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' +import { StructuredAgentSessionLeaseRenewer } from './structured-agent-session-lease-renewer' +import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' +import { + holdsLiveProviderChild, + markProviderChildExited +} from './structured-agent-session-provider-child' + +const NOW = 1_800_000_000_000 +const SESSION = 'session-held' +const roots: string[] = [] + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +/** A live owner recorded without a start time, as on a host that could not read one. */ +async function liveStoreWithoutStartTime(): Promise<{ + root: string + store: AgentSessionRecordStore + fence: number +}> { + const root = await mkdtemp(join(tmpdir(), 'orca-held-child-renewal-')) + roots.push(root) + const store = await openTestAgentSessionRecordStore(root) + const reserved = await store.reserveOwner({ + sessionId: SESSION, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'folder' + }, + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: root }, + expectedFence: null, + spawnToken: 'spawn-held', + claimKeyId: 'key-1', + handoffOperationId: null, + probe: { outcome: 'reservation-unused' }, + operation: { + callerKey: 'test', + operationId: `${NOW}-00000000000000000000000000000001`, + fingerprint: 'create' + }, + now: NOW + }) + const fence = reserved.record.lease.runtimeFence + await store.commitProcessIdentity({ + sessionId: SESSION, + fence, + process: { hostId: 'local', pid: 4242, processStartTimeMs: null, spawnToken: 'spawn-held' }, + now: NOW + }) + await store.proveOwner({ + sessionId: SESSION, + fence, + link: { + linkId: 'link-held', + handle: codexProviderHandle('thread-held'), + origin: 'created', + mintedAtFence: fence, + observedAt: NOW + }, + now: NOW + }) + return { root, store, fence } +} + +/** The real PID probe on `platform`, with the recorded pid present: no start time, no token echo. */ +function pidProbe(platform: NodeJS.Platform) { + return vi.fn((record: AgentSessionRecord) => + probeAgentSessionProcessIdentity({ + identity: record.lease.ownerProcess!, + deps: { platform, isPidPresent: () => true } + }) + ) +} + +function hostWith(child: StructuredAgentSessionProviderChild | null) { + const session: StructuredAgentSessionChildExitSession = { + child, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Only the exit-receipt mark runs here; the serialized settlement that reads the journal never does. + journal: {} as StructuredAgentSessionChildExitSession['journal'] + } + return { + session, + holdsLiveChild: (sessionId: string, fence: number) => + holdsLiveProviderChild(sessionId === SESSION ? session : undefined, fence) + } +} + +function renewerFor( + store: AgentSessionRecordStore, + probe: ReturnType, + clock: { now: number }, + holdsLiveChild: (sessionId: string, fence: number) => boolean +) { + const log = recordingStructuredAgentSessionLogger() + const renewer = new StructuredAgentSessionLeaseRenewer({ + store, + probe, + holdsLiveChild, + now: () => clock.now, + logger: log.logger + }) + return { renewer, log } +} + +describe.each(['darwin', 'win32'] as const)('lease renewal of a held child on %s', (platform) => { + it('keeps renewing a held child with no start time, without a PID probe', async () => { + const { store, fence } = await liveStoreWithoutStartTime() + const { holdsLiveChild } = hostWith({ generation: 'gen-1', fence, phase: 'ready' }) + const probe = pidProbe(platform) + const clock = { now: NOW + 10_000 } + const { renewer, log } = renewerFor(store, probe, clock, holdsLiveChild) + + await renewer.renewNow() + clock.now = NOW + 20_000 + await renewer.renewNow() + + expect(probe).not.toHaveBeenCalled() + expect(store.getRecord(SESSION)?.lease.lastRenewedAt).toBe(NOW + 20_000) + expect(log.entries).toEqual([]) + }) + + it('stops renewing once the held child exit is received', async () => { + const { store, fence } = await liveStoreWithoutStartTime() + const child: StructuredAgentSessionProviderChild = { + generation: 'gen-1', + fence, + phase: 'ready' + } + const { session, holdsLiveChild } = hostWith(child) + const probe = pidProbe(platform) + const clock = { now: NOW + 10_000 } + const { renewer } = renewerFor(store, probe, clock, holdsLiveChild) + await renewer.renewNow() + + markProviderChildExited(session, child) + clock.now = NOW + 20_000 + await renewer.renewNow() + + // Back on the PID probe, which cannot vouch for it: the lease keeps its last proof. + expect(probe).toHaveBeenCalledOnce() + expect(store.getRecord(SESSION)?.lease.lastRenewedAt).toBe(NOW + 10_000) + }) + + it('dates a crash death to the last held renewal, not to the spawn', async () => { + const { root, store, fence } = await liveStoreWithoutStartTime() + const { holdsLiveChild } = hostWith({ generation: 'gen-1', fence, phase: 'ready' }) + const clock = { now: NOW + 10_000 } + const { renewer } = renewerFor(store, pidProbe(platform), clock, holdsLiveChild) + await renewer.renewNow() + clock.now = NOW + 20_000 + await renewer.renewNow() + + // Orca crashed: the next runtime holds no child and finds the recorded pid gone. + const reopened = await openTestAgentSessionRecordStore(root) + await reopened.reconcileOnRestart({ + probe: async () => ({ outcome: 'pid-absent' }), + now: NOW + 600_000 + }) + + expect(reopened.getRecord(SESSION)?.lease.deathEvidence).toMatchObject({ + kind: 'pid-absent', + lastProvenAliveAt: NOW + 20_000 + }) + }) + + it('still probes a record this runtime does not hold at its fence', async () => { + const { store, fence } = await liveStoreWithoutStartTime() + // An older child on record, or none at all, holds nothing at the record's fence. + for (const child of [ + { generation: 'gen-0', fence: fence - 1, phase: 'ready' } as const, + null + ]) { + const { holdsLiveChild } = hostWith(child) + const probe = pidProbe(platform) + const { renewer, log } = renewerFor(store, probe, { now: NOW + 10_000 }, holdsLiveChild) + + await renewer.renewNow() + + expect(probe).toHaveBeenCalledOnce() + await expect(probe.mock.results[0]?.value).resolves.toMatchObject({ + outcome: 'indeterminate' + }) + expect(store.getRecord(SESSION)?.lease.lastRenewedAt).toBe(NOW) + expect(log.entries.map((entry) => entry.fields)).toContainEqual({ + scope: 'lease-renewal', + sessionId: SESSION, + error: expect.objectContaining({ message: 'agent_session_ownership_unknown' }) + }) + } + }) +}) + +describe('receipt of a child exit', () => { + it('ends the held child proof before the exit settlement gets its turn', async () => { + const child: StructuredAgentSessionProviderChild = { + generation: 'gen-1', + fence: 3, + phase: 'ready' + } + const { session, holdsLiveChild } = hostWith(child) + const context = { + sessions: new Map([[SESSION, session]]), + // The session's queue is busy: the settlement itself never starts in this test. + serialize: () => new Promise(() => {}) + } + + void settleStructuredAgentSessionChildExit( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The receipt mark reads only `sessions`; everything else runs inside `serialize`, which never starts here. + context as unknown as StructuredAgentSessionChildExitContext, + { + type: 'ended', + sessionId: SESSION, + reason: 'exited', + cause: 'unexpected-exit', + fence: 3, + acquisitionGeneration: 'gen-1' + } + ) + + expect(session.child).toBe(child) + expect(holdsLiveChild(SESSION, 3)).toBe(false) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts index fd0fd17b71e0..de3d8c00fb33 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts @@ -16,13 +16,15 @@ export class StructuredAgentSessionHostRuntimeState { constructor( private readonly deps: StructuredAgentSessionHostDeps, - onEventSinkFailure?: (sessionId: string, error: unknown) => void + onEventSinkFailure?: (sessionId: string, error: unknown) => void, + holdsLiveChild?: (sessionId: string, fence: number) => boolean ) { this.onEventSinkFailure = onEventSinkFailure this.leaseRenewer = new StructuredAgentSessionLeaseRenewer({ store: deps.store, probe: (record) => this.probeRecord(record), ...(deps.probeOwners ? { probeMany: deps.probeOwners } : {}), + ...(holdsLiveChild ? { holdsLiveChild } : {}), now: () => deps.now?.() ?? Date.now(), // Lease/ownership failures are transient and stay on the visible lease-error path. // Only deferred sink I/O failures are terminal and may force-close a provider. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts index 7016e91e17c6..b69d48ff162e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts @@ -68,6 +68,8 @@ export type StructuredAgentSessionProviderChild = StructuredAgentSessionProvider * for any other start. In memory only: it tells a restart offer its own start from another. */ readonly startedFor?: string close?: StructuredAgentSessionChildClose + /** The host received the child's exit; it stays on record only until that exit is settled. */ + exitObserved?: true } /** What ending a child established about its provider root. A stop's comes only from diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index 59599c5adfe6..fd2e5820928b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -51,7 +51,10 @@ import { } from './structured-agent-session-restart-resume-host' import { structuredAgentSessionRestartResumeSurfaces } from './structured-agent-session-restart-resume-wiring' import { createStructuredAgentSessionConversationDelivery } from './structured-agent-session-host-delivery' -import { structuredAgentSessionConversationFence } from './structured-agent-session-provider-child' +import { + holdsLiveProviderChild, + structuredAgentSessionConversationFence +} from './structured-agent-session-provider-child' import { wireStructuredAgentSessionQueuedMessages } from './structured-agent-session-queued-wiring' import * as sessionLogger from './structured-agent-session-logger' export type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' @@ -109,8 +112,10 @@ export class StructuredAgentSessionHost { (sessionId) => this.lifetime.conversation(sessionId), this.clientDelivery.readChildWork ) - this.runtimeState = new StructuredAgentSessionHostRuntimeState(deps, (sessionId, error) => - this.eventRecovery.recoverAfterSinkFailure(sessionId, error) + this.runtimeState = new StructuredAgentSessionHostRuntimeState( + deps, + (sessionId, error) => this.eventRecovery.recoverAfterSinkFailure(sessionId, error), + (sessionId, fence) => holdsLiveProviderChild(this.sessions.get(sessionId), fence) ) this.reconcileLeases = createRestartReconciler({ store: deps.store, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts index 7888d4867e23..3698ff1f3070 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts @@ -8,6 +8,11 @@ import type { StructuredAgentSessionLogger } from './structured-agent-session-lo const RENEW_INTERVAL_MS = Math.floor(AGENT_SESSION_LEASE_TTL_MS / 3) +const HELD_CHILD: AgentSessionOwnerProbe = { + outcome: 'identity-matched', + matchedOn: ['held-child'] +} + export class StructuredAgentSessionLeaseRenewer { private timer: ReturnType | null = null private running = false @@ -22,6 +27,8 @@ export class StructuredAgentSessionLeaseRenewer { probeMany?: ( records: readonly AgentSessionRecord[] ) => Promise> + /** Whether this runtime holds the session's child at `fence` and has not seen it exit. */ + holdsLiveChild?: (sessionId: string, fence: number) => boolean now: () => number logger: StructuredAgentSessionLogger intervalMs?: number @@ -72,7 +79,20 @@ export class StructuredAgentSessionLeaseRenewer { // keeps an orphan pid's lease reading as a healthy owner. record.lease.handoffStage !== 'recovering' ) - const probes = await this.probe(records) + // A child this runtime holds proves itself; a PID probe can be indeterminate for it (no start + // time, no token echo), which would freeze the lease at its last proof. + const holds = (record: AgentSessionRecord): boolean => + this.input.holdsLiveChild?.(record.sessionId, record.lease.runtimeFence) === true + const held = records.filter(holds) + const unheld = records.filter((record) => !held.includes(record)) + const probes = + unheld.length > 0 ? await this.probe(unheld) : new Map() + for (const record of held) { + // Re-read after the probes: an exit received meanwhile ends the child's proof. + if (holds(record)) { + probes.set(record.sessionId, HELD_CHILD) + } + } const renewals: { sessionId: string fence: number diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts index d4236f395da4..5288318dbcf4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts @@ -47,6 +47,27 @@ export function markProviderChildStarted( return child !== null } +/** From the moment the host receives the exit: the child stops vouching for its lease. */ +export function markProviderChildExited( + session: ChildBearer, + identity: StructuredAgentSessionProviderChildIdentity +): void { + const child = matchingChild(session, identity) + if (child) { + child.exitObserved = true + } +} + +/** This runtime holds a child at `fence` whose exit it has not received: first-hand proof of life + * that needs no PID probe. A previous runtime's child is never on record here. */ +export function holdsLiveProviderChild( + session: Pick | undefined, + fence: number +): boolean { + const child = session?.child + return !!child && child.fence === fence && child.exitObserved !== true +} + /** `endedAt` is a close's ask; an exit of the child's own ends where the journal stands. */ export function endProviderChild( session: ChildBearer, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts index 15e364fc7fed..257072f7c23b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts @@ -32,8 +32,9 @@ export type StructuredSessionRecoveryResolutionDeps = { } const STOP_PROBE_INTERVAL_MS = 250 -// A POSIX structured owner is its provider supervisor, which exits only after its provider -// group. A SIGKILL that lands first leaves the group running, so SIGTERM outlasts its stop. +// POSIX only: Windows never signals a recorded owner. The owner is its provider supervisor, which +// exits only after its provider group. A SIGKILL that lands first leaves the group running, so +// SIGTERM outlasts its stop. const STOP_PROBES: Record = { SIGTERM: Math.ceil(PROVIDER_SUPERVISOR_MAX_STOP_MS / STOP_PROBE_INTERVAL_MS) + 1, SIGKILL: 4 diff --git a/src/main/runtime/agent-session-lease-transitions.ts b/src/main/runtime/agent-session-lease-transitions.ts index 1737ad45d3fd..950e05a8e78e 100644 --- a/src/main/runtime/agent-session-lease-transitions.ts +++ b/src/main/runtime/agent-session-lease-transitions.ts @@ -178,8 +178,9 @@ export function proveAgentSessionOwner(args: { /** * A renewal asserts two things at once: the host is running its loop, and the child still matches - * the recorded identity. A host that cannot re-verify the child stops renewing rather than - * extending a lease it can no longer vouch for. + * the recorded identity — re-proven by a PID probe, or held by this runtime with no exit seen. A + * host that cannot re-verify the child stops renewing rather than extending a lease it can no + * longer vouch for. */ export function renewAgentSessionLease(args: { record: AgentSessionRecord diff --git a/src/main/runtime/agent-session-process-identity-probe.ts b/src/main/runtime/agent-session-process-identity-probe.ts index 0fe0a16f4ee0..2c7992af6f5e 100644 --- a/src/main/runtime/agent-session-process-identity-probe.ts +++ b/src/main/runtime/agent-session-process-identity-probe.ts @@ -10,8 +10,8 @@ import { readFile } from 'node:fs/promises' import type { - AgentSessionIdentityMatchField, - AgentSessionOwnerProbe + AgentSessionOwnerProbe, + AgentSessionProcessIdentityField } from '../../shared/agent-session-lease-adjudication' import type { AgentSessionProcessIdentity } from '../../shared/agent-session-record' import { runProcess } from '../../shared/child-process/run-process' @@ -114,8 +114,8 @@ async function readDarwinProcessStartTimesMs( } async function readWindowsProcessStartTimeMs(pid: number): Promise { - // No shipped addon build exposes the creation-time flag, so without this the - // whole table gets scanned to produce `null` every time. + // A binary without the creation-time flag (one built before Orca's patch) would + // otherwise scan the whole table to produce `null` every time. if (!isWindowsProcessStartTimeAvailable()) { return null } @@ -242,7 +242,7 @@ export async function probeAgentSessionProcessIdentity(args: { if (!isPidPresent(identity.pid)) { return { outcome: 'pid-absent' } } - const matchedOn: AgentSessionIdentityMatchField[] = [] + const matchedOn: AgentSessionProcessIdentityField[] = [] const echoedToken = await deps.readEchoedSpawnToken?.(identity).catch(() => null) if (echoedToken !== null && echoedToken !== undefined) { if (echoedToken !== identity.spawnToken) { diff --git a/src/shared/agent-session-lease-adjudication.ts b/src/shared/agent-session-lease-adjudication.ts index 271b136456f0..75de4a74c40d 100644 --- a/src/shared/agent-session-lease-adjudication.ts +++ b/src/shared/agent-session-lease-adjudication.ts @@ -20,7 +20,12 @@ import type { AgentSessionRefusalDetailsByCode } from './agent-session-wire-refusals' -export type AgentSessionIdentityMatchField = 'process-start-time' | 'spawn-token' +/** What a PID probe can compare against the recorded owner. */ +export type AgentSessionProcessIdentityField = 'process-start-time' | 'spawn-token' + +/** `held-child`: this runtime still holds the child at the record's fence and has not seen it exit. + * Only lease renewal in that runtime asserts it; restart and recovery probes never can. */ +export type AgentSessionIdentityMatchField = AgentSessionProcessIdentityField | 'held-child' export type AgentSessionOwnerProbe = /** Orca watched this exact process exit. */ @@ -28,7 +33,7 @@ export type AgentSessionOwnerProbe = /** The recorded pid is not present on the host. */ | { outcome: 'pid-absent' } /** The pid is present but is a different process. */ - | { outcome: 'identity-mismatch'; field: AgentSessionIdentityMatchField | 'command-line' } + | { outcome: 'identity-mismatch'; field: AgentSessionProcessIdentityField | 'command-line' } /** The pid is present and at least one identity element was verified. */ | { outcome: 'identity-matched'; matchedOn: readonly AgentSessionIdentityMatchField[] } /** No process carries the reserved spawn token and the provider saw no activity after it. */ From e1f5396a6d1b77456945ec9edcd2f30957d2f9d1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:16:59 -0700 Subject: [PATCH 4/5] fix(native-chat): keep a failed Windows taskkill unproven across a retried close A Windows close counts Claude leaving on its own after its stdin ends as the close. That shortcut also caught a retried close whose first attempt forced a taskkill that failed: once the root exited, the retry returned true and the failure read as a proven close. The tree reaper now records whether a reap ever reached the live root, here, on an earlier close or from a transport failure, and the shortcut applies only when none did; otherwise taskkill's verdict stands and no new taskkill runs against the exited root. Pin the platform on the existing tests that assume the POSIX close, and say what `exit-proven` means on Windows in the acquisition-failure docs. --- .../claude-agent-sdk-exit-proof.test.ts | 44 ++++++++++++++-- .../claude/claude-agent-sdk-exit-proof.ts | 13 ++++- .../claude-agent-sdk-process-spawn.test.ts | 3 +- .../claude-child-exit-proof-ladder.test.ts | 50 +++++++++++++++++-- .../claude/claude-child-exit-proof-ladder.ts | 10 +++- ...laude-stream-json-connection-close.test.ts | 3 +- .../claude-stream-json-connection.test.ts | 7 ++- .../structured-agent-session-adapter.ts | 5 +- ...-session-acquisition-failure-settlement.ts | 4 +- 9 files changed, 121 insertions(+), 18 deletions(-) diff --git a/src/main/claude/claude-agent-sdk-exit-proof.test.ts b/src/main/claude/claude-agent-sdk-exit-proof.test.ts index d5ea5493f5e0..1ccab648787a 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.test.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.test.ts @@ -136,12 +136,16 @@ function mockChild( } /** A tree whose verdict is scripted per reap, recording when it was armed. */ -function mockTree(verdicts: DescendantTreeVerdict[]): ClaudeChildTreeReaper & { +function mockTree( + verdicts: DescendantTreeVerdict[], + forcedReapAttempted = false +): ClaudeChildTreeReaper & { capture: ReturnType reap: ReturnType } { let treeVerdict: DescendantTreeVerdict = 'unverifiable' return { + forcedReapAttempted, capture: vi.fn(async () => {}), reap: vi.fn(async () => { treeVerdict = verdicts.shift() ?? treeVerdict @@ -267,7 +271,10 @@ describe('claude child exit proof', () => { exitedWhenArmed = exit.exited() }) - await expect(proveClaudeChildExit({ child, ...exit, tree })).resolves.toBe(true) + // POSIX: on Windows a root that leaves on its own after stdin end is the close itself. + await expect(proveClaudeChildExit({ child, ...exit, tree, platform: 'linux' })).resolves.toBe( + true + ) // The snapshot is the only proof that survives the root: taken while it lived, // verified once it left. A reap before the exit would have been the forced ladder. expect(exitedWhenArmed).toBe(false) @@ -333,11 +340,34 @@ describe('claude child exit proof', () => { const tree = mockTree(['exited']) await expect( - proveClaudeChildExit({ child, exitPromise: Promise.resolve(), exited: () => true, tree }) + proveClaudeChildExit({ + child, + exitPromise: Promise.resolve(), + exited: () => true, + tree, + platform: 'linux' + }) ).resolves.toBe(true) expect(tree.reap).toHaveBeenCalledTimes(1) }) + it('on Windows re-asks the tree on a retried close once a forced reap has run', async () => { + const child = mockChild() + // The earlier close forced a reap whose taskkill failed; the root has since exited. + const tree = mockTree(['unverifiable'], true) + + await expect( + proveClaudeChildExit({ + child, + exitPromise: Promise.resolve(), + exited: () => true, + tree, + platform: 'win32' + }) + ).resolves.toBe(false) + expect(tree.reap).toHaveBeenCalledTimes(1) + }) + it('stays unproven for a root that left before any snapshot could be armed', async () => { const child = mockChild() const captureDescendants = vi.fn(async () => snapshotOf(4243)) @@ -350,7 +380,13 @@ describe('claude child exit proof', () => { }) await expect( - proveClaudeChildExit({ child, exitPromise: Promise.resolve(), exited: () => true, tree }) + proveClaudeChildExit({ + child, + exitPromise: Promise.resolve(), + exited: () => true, + tree, + platform: 'darwin' + }) ).resolves.toBe(false) // A dead root's descendants have reparented: walking its pid now could only // sweep a stranger, so no walk is attempted and nothing is proven. diff --git a/src/main/claude/claude-agent-sdk-exit-proof.ts b/src/main/claude/claude-agent-sdk-exit-proof.ts index bc82d33bcfb9..7ae28f35b10f 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.ts @@ -72,6 +72,8 @@ export type ClaudeChildTreeReaper = { * seen still running, which no later caller may collapse into "unknown". */ readonly treeVerdict: DescendantTreeVerdict + /** A reap has reached the root while it lived: its exit since then is no longer its own. */ + readonly forcedReapAttempted: boolean } /** @@ -101,6 +103,7 @@ export function createClaudeChildTreeReaper( let queuedRefresh: Promise | null = null let inFlight: Promise | null = null let treeVerdict: DescendantTreeVerdict = 'unverifiable' + let forcedReapAttempted = false function captureOnce(): Promise { if (!snapshots) { @@ -256,6 +259,9 @@ export function createClaudeChildTreeReaper( // Never spawned, so the OS never created a tree to orphan. return 'exited' } + if (!exited()) { + forcedReapAttempted = true + } if (!snapshots) { return judgeWindowsTree(rootPid) } @@ -313,6 +319,9 @@ export function createClaudeChildTreeReaper( }, get treeVerdict() { return treeVerdict + }, + get forcedReapAttempted() { + return forcedReapAttempted } } } @@ -323,8 +332,8 @@ export function createClaudeChildTreeReaper( * * Resolves true only after the child actually emitted exit and, on POSIX, its * snapshotted descendants were observed gone; on Windows, after it left on its - * own once its stdin ended, or a forced `taskkill /T /F` reported its tree - * terminated. False is unproven. On POSIX a root that left on its own before a + * own once its stdin ended with no forced reap before, or a forced + * `taskkill /T /F` reported its tree terminated. False is unproven. On POSIX a root that left on its own before a * snapshot could be armed stays unproven: its descendants had already * reparented out of reach when the ladder first looked. */ diff --git a/src/main/claude/claude-agent-sdk-process-spawn.test.ts b/src/main/claude/claude-agent-sdk-process-spawn.test.ts index af1da7cd8bf0..ae90a8e569b6 100644 --- a/src/main/claude/claude-agent-sdk-process-spawn.test.ts +++ b/src/main/claude/claude-agent-sdk-process-spawn.test.ts @@ -136,7 +136,8 @@ describe('claude agent SDK process spawn', () => { const tree = { capture: vi.fn(async () => {}), reap: vi.fn(async () => 'exited' as const), - treeVerdict: 'exited' as const + treeVerdict: 'exited' as const, + forcedReapAttempted: false } await proveClaudeChildExitWithReaper( { diff --git a/src/main/claude/claude-child-exit-proof-ladder.test.ts b/src/main/claude/claude-child-exit-proof-ladder.test.ts index b14965fba1c0..5395c6eca084 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.test.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.test.ts @@ -11,7 +11,8 @@ function fakeTree(): ClaudeChildTreeReaper & { reap: ReturnType } capture: vi.fn(async () => {}), refresh: vi.fn(async () => {}), reap: vi.fn(async () => 'exited' as const), - treeVerdict: 'exited' + treeVerdict: 'exited', + forcedReapAttempted: false } } @@ -44,8 +45,9 @@ function rootStoppedBySigterm(stopMs: number) { } } -/** A Windows root that leaves on stdin end when `leavesOnStdinEnd`, otherwise only once killed. */ -function windowsRoot(leavesOnStdinEnd: boolean) { +/** A Windows root that leaves on stdin end when `leavesOnStdinEnd`, otherwise only once killed + * (or, with `leavesOnKill` false, only when the test calls `leave`). */ +function windowsRoot(leavesOnStdinEnd: boolean, leavesOnKill = true) { let exited = false let settle = (): void => {} const exitPromise = new Promise((resolve) => { @@ -56,7 +58,9 @@ function windowsRoot(leavesOnStdinEnd: boolean) { settle() } const kill = vi.fn(() => { - leave() + if (leavesOnKill) { + leave() + } return true }) const stdin = { @@ -72,6 +76,7 @@ function windowsRoot(leavesOnStdinEnd: boolean) { typeof proveClaudeChildExitWithReaper >[0]['child'], kill, + leave, exitPromise, exited: () => exited } @@ -180,4 +185,41 @@ describe('Claude child exit proof ladder', () => { await expect(proveClaudeChildExitWithReaper(input, () => tree)).resolves.toBe(false) expect(tree.reap).toHaveBeenCalledOnce() }) + + it('on Windows a retried close after a failed taskkill stays unproven once the root exits', async () => { + const root = windowsRoot(false, false) + const terminateWindowsTree = vi.fn(async () => false) + const { tree } = windowsTree(root, terminateWindowsTree) + const close = () => + proveClaudeChildExitWithReaper({ ...root, tree, platform: 'win32' }, () => tree) + + await expect(close()).resolves.toBe(false) + // The exit lands only after the forced wait: it is not Claude leaving on its own. + root.leave() + await expect(close()).resolves.toBe(false) + + expect(terminateWindowsTree).toHaveBeenCalledOnce() + expect(tree.forcedReapAttempted).toBe(true) + expect(tree.treeVerdict).toBe('unverifiable') + }, 10_000) + + it.each([ + { taskkill: true, proven: true }, + { taskkill: false, proven: false } + ])( + 'on Windows a reap outside a close keeps taskkill as the verdict: taskkill $taskkill', + async ({ taskkill, proven }) => { + // The transport-failure reap (stdin error, reader failure) kills the live tree itself. + const root = windowsRoot(false) + const terminateWindowsTree = vi.fn(async () => taskkill) + const { tree } = windowsTree(root, terminateWindowsTree) + await tree.reap() + expect(root.exited()).toBe(true) + + await expect( + proveClaudeChildExitWithReaper({ ...root, tree, platform: 'win32' }, () => tree) + ).resolves.toBe(proven) + expect(terminateWindowsTree).toHaveBeenCalledOnce() + } + ) }) diff --git a/src/main/claude/claude-child-exit-proof-ladder.ts b/src/main/claude/claude-child-exit-proof-ladder.ts index 0a27e725a50f..424a55bd5049 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.ts @@ -49,8 +49,14 @@ export async function proveClaudeChildExitWithReaper( } } // On Windows, as with the Codex close, Claude leaving on its own after its stdin ends is the - // close: Orca makes no claim about processes Claude started, and no reap could address them. - if (!reaped && input.exited() && (input.platform ?? process.platform) === 'win32') { + // close: Orca makes no claim about processes Claude started, and no reap could address them. An + // exit after any forced reap on this tree, here or on an earlier close, keeps taskkill's verdict. + if ( + !reaped && + !tree.forcedReapAttempted && + input.exited() && + (input.platform ?? process.platform) === 'win32' + ) { return true } if (!reaped && input.exited() && tree.treeVerdict !== 'exited') { diff --git a/src/main/claude/claude-stream-json-connection-close.test.ts b/src/main/claude/claude-stream-json-connection-close.test.ts index 348601bbd489..c1078a82a57a 100644 --- a/src/main/claude/claude-stream-json-connection-close.test.ts +++ b/src/main/claude/claude-stream-json-connection-close.test.ts @@ -16,7 +16,8 @@ const mocks = vi.hoisted(() => { capture: vi.fn(async () => {}), refresh: (...args: unknown[]) => refresh(...args), reap: vi.fn(async () => 'exited' as const), - treeVerdict: 'unverifiable' as const + treeVerdict: 'unverifiable' as const, + forcedReapAttempted: false } return { proveClaudeChildExit, refresh, tree } }) diff --git a/src/main/claude/claude-stream-json-connection.test.ts b/src/main/claude/claude-stream-json-connection.test.ts index bd79a4a2c9d2..d5806980f967 100644 --- a/src/main/claude/claude-stream-json-connection.test.ts +++ b/src/main/claude/claude-stream-json-connection.test.ts @@ -624,7 +624,12 @@ describe('Claude stream-json connection', () => { const closed = await connection.close() expect(connection.exitVerdict.root).toBe('exited') expect(['exited', 'unverifiable']).toContain(connection.exitVerdict.tree) - expect(closed).toBe(connection.exitVerdict.tree === 'exited') + if (process.platform === 'win32') { + // The self-exit is the close, unless a reap racing it already forced the tree. + expect(closed || connection.exitVerdict.tree === 'unverifiable').toBe(true) + } else { + expect(closed).toBe(connection.exitVerdict.tree === 'exited') + } }) it.runIf(process.platform !== 'win32')( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 9975701e7047..5a90d2cf25c2 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -97,8 +97,9 @@ export class AgentSessionAcquisitionRootExitObservedError extends Error { } } -/** The provider child failed and cleanup proved its whole tree gone. As with a root exit, the - * provider's own diagnostic is the message. */ +/** The provider child failed and cleanup proved its whole tree gone — on Windows, that its root + * left on its own after stdin end (descendants not addressed, as with Codex) or taskkill reported + * the tree terminated. As with a root exit, the provider's own diagnostic is the message. */ export class AgentSessionAcquisitionExitProvenError extends Error { constructor(cause: unknown) { super(cause instanceof Error ? cause.message : String(cause), { cause }) diff --git a/src/main/runtime/agent-session-acquisition-failure-settlement.ts b/src/main/runtime/agent-session-acquisition-failure-settlement.ts index f6efaa890c37..1be4974fc659 100644 --- a/src/main/runtime/agent-session-acquisition-failure-settlement.ts +++ b/src/main/runtime/agent-session-acquisition-failure-settlement.ts @@ -13,7 +13,9 @@ import type { AgentSessionStoreState } from './agent-session-record-store-file' /** * How the failed attempt's provider process was accounted for. - * - `exit-proven`: cleanup observed the whole tree gone. + * - `exit-proven`: cleanup observed the whole tree gone. On Windows it is the provider close's own + * proof: the root left on its own after its stdin ended, with its descendants not addressed (as + * with Codex), or a forced `taskkill /T` reported the tree terminated. * - `root-exit-observed`: the owner root's exit was observed first-hand, so the * identity this lease is keyed on is dead, but its descendants were not proven * gone. Releases the lease and says exactly that, claiming nothing more. From 989fc287973e8b66f3c51f722d23f00aeb7263e3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 5 Oct 2026 20:30:26 -0700 Subject: [PATCH 5/5] fix(native-chat): derive a held child's liveness from the adapter's own handle Lease renewal trusted a held child until the host settled its exit, and the host hears of an exit late: Claude runs its close ladder and a store write first, unexpected exits wait on one delivery chain shared by every chat, and a Claude close that cannot prove its tree publishes nothing at all. A dead root could keep renewing through that window, so a crash in it dated the cut turn late. Renewal now asks the adapter, which owns the process handle and sees the exit first: a child is held only while it is on record at the lease's fence and its adapter still runs that exact acquisition with no root exit seen. An adapter that cannot answer falls back to the PID probe. The stored exit-received mark is gone. The held-child read is now required by the runtime state and the renewer, and a host-level test drives renewal through the real host wiring. --- .../claude/claude-agent-sdk-exit-proof.ts | 10 +- ...e-structured-live-provider-process.test.ts | 28 +++++ .../claude-structured-session-adapter.ts | 8 ++ ...x-structured-live-provider-process.test.ts | 20 +++ .../codex/codex-structured-session-adapter.ts | 11 ++ ...structured-agent-session-adapter-router.ts | 4 + .../structured-agent-session-adapter.ts | 3 + .../structured-agent-session-child-exit.ts | 14 +-- ...red-agent-session-claude-root-exit.test.ts | 2 +- ...tructured-agent-session-event-sink.test.ts | 5 +- .../structured-agent-session-eviction.test.ts | 5 +- ...t-session-held-child-lease-renewal.test.ts | 75 +++--------- ...nt-session-host-held-child-renewal.test.ts | 114 ++++++++++++++++++ ...d-agent-session-host-runtime-state.test.ts | 4 +- ...ctured-agent-session-host-runtime-state.ts | 7 +- .../structured-agent-session-host-types.ts | 2 - .../structured-agent-session-host.ts | 4 +- ...ctured-agent-session-lease-renewer.test.ts | 7 ++ .../structured-agent-session-lease-renewer.ts | 4 +- ...structured-agent-session-provider-child.ts | 36 +++--- ...ured-agent-session-failure-logging.test.ts | 5 +- 21 files changed, 260 insertions(+), 108 deletions(-) create mode 100644 src/main/claude/claude-structured-live-provider-process.test.ts create mode 100644 src/main/codex/codex-structured-live-provider-process.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-host-held-child-renewal.test.ts diff --git a/src/main/claude/claude-agent-sdk-exit-proof.ts b/src/main/claude/claude-agent-sdk-exit-proof.ts index 7ae28f35b10f..a0db58343628 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.ts @@ -251,7 +251,8 @@ export function createClaudeChildTreeReaper( return terminated ? 'exited' : 'unverifiable' } - /** The only source of a tree verdict: `exited` is observed on POSIX, taskkill's report on Windows. */ + /** The only source of a tree verdict: `exited` is observed on POSIX and is taskkill's report + * on Windows. */ async function judgeTree(): Promise { const killRoot = (): boolean => terminateClaudeRoot({ child, exited }) const rootPid = child.pid @@ -333,9 +334,10 @@ export function createClaudeChildTreeReaper( * Resolves true only after the child actually emitted exit and, on POSIX, its * snapshotted descendants were observed gone; on Windows, after it left on its * own once its stdin ended with no forced reap before, or a forced - * `taskkill /T /F` reported its tree terminated. False is unproven. On POSIX a root that left on its own before a - * snapshot could be armed stays unproven: its descendants had already - * reparented out of reach when the ladder first looked. + * `taskkill /T /F` reported its tree terminated. False is unproven. On POSIX a + * root that left on its own before a snapshot could be armed stays unproven: + * its descendants had already reparented out of reach when the ladder first + * looked. */ export function proveClaudeChildExit(input: ClaudeChildExitProofInput): Promise { return proveClaudeChildExitWithReaper(input, () => diff --git a/src/main/claude/claude-structured-live-provider-process.test.ts b/src/main/claude/claude-structured-live-provider-process.test.ts new file mode 100644 index 000000000000..a829f0906a22 --- /dev/null +++ b/src/main/claude/claude-structured-live-provider-process.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest' +import { + adapterFor, + fakeClaude, + identityFor, + recordingJournalSink +} from './claude-structured-session-test-support' + +describe('Claude adapter liveness for lease renewal', () => { + it('holds its acquisition until the root exit is seen', async () => { + const claude = fakeClaude() + const adapter = adapterFor(claude) + const { acquisitionGeneration } = await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + events: recordingJournalSink() + }) + + expect(adapter.holdsLiveProviderProcess('session-1', acquisitionGeneration!)).toBe(true) + expect(adapter.holdsLiveProviderProcess('session-1', 'another-acquisition')).toBe(false) + expect(adapter.holdsLiveProviderProcess('session-2', acquisitionGeneration!)).toBe(false) + + // The connection's own observation, before any exit event reaches the host. + claude.connections[0]!.exitVerdict = { root: 'exited', tree: 'unverifiable' } + expect(adapter.holdsLiveProviderProcess('session-1', acquisitionGeneration!)).toBe(false) + }) +}) diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index 0e3176a1813c..2f778e610b62 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -252,6 +252,14 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda const session = this.sessions.get(sessionId) return session ? claudeHoldsDispatch(session) : false } + holdsLiveProviderProcess = (sessionId: string, acquisitionGeneration: string): boolean => { + const session = this.sessions.get(sessionId) + return ( + session?.acquisitionGeneration === acquisitionGeneration && + session.connection.pid !== undefined && + session.connection.exitVerdict.root === 'live' + ) + } answerPrompt: StructuredAgentSessionAdapter['answerPrompt'] = (request) => settleClaudePromptFreeingChild(this.asker(request), answerClaudeStructuredPrompt) setOption: StructuredAgentSessionAdapter['setOption'] = (input) => diff --git a/src/main/codex/codex-structured-live-provider-process.test.ts b/src/main/codex/codex-structured-live-provider-process.test.ts new file mode 100644 index 000000000000..f6163cf0558c --- /dev/null +++ b/src/main/codex/codex-structured-live-provider-process.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest' +import { adapterFor, fakeCodex, identityFor } from './codex-structured-session-adapter-fixture' + +describe('Codex adapter liveness for lease renewal', () => { + it('holds its acquisition until the connection reports the root exit', async () => { + const codex = fakeCodex() + const adapter = adapterFor(codex) + const { acquisitionGeneration } = await adapter.acquire({ + identity: identityFor('session-1'), + fence: 7, + spawnToken: 'spawn-9' + }) + + expect(adapter.holdsLiveProviderProcess('session-1', acquisitionGeneration!)).toBe(true) + expect(adapter.holdsLiveProviderProcess('session-1', 'another-acquisition')).toBe(false) + + codex.connections[0]!.handlers.onExit?.(new Error('codex app-server exited')) + expect(adapter.holdsLiveProviderProcess('session-1', acquisitionGeneration!)).toBe(false) + }) +}) diff --git a/src/main/codex/codex-structured-session-adapter.ts b/src/main/codex/codex-structured-session-adapter.ts index fdd9a1b29722..deca5901effd 100644 --- a/src/main/codex/codex-structured-session-adapter.ts +++ b/src/main/codex/codex-structured-session-adapter.ts @@ -166,6 +166,17 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap backgroundTaskState = (sessionId: string): AgentSessionBackgroundTaskState | null | undefined => this.sessions.get(sessionId)?.backgroundTasks.state + // `ended` is set in the same turn as the connection's own exit report. + holdsLiveProviderProcess = (sessionId: string, acquisitionGeneration: string): boolean => { + const session = this.sessions.get(sessionId) + return ( + session?.acquisitionGeneration === acquisitionGeneration && + session.connection.pid !== undefined && + !session.ended && + session.exitObservedAt === undefined + ) + } + // Codex exposes no honest stop for a child thread or a persistent command. backgroundTaskStops: NonNullable = ( sessionId diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts index 5edc3793d026..4e85826c3c9e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts @@ -116,6 +116,10 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi holdsDispatch = (sessionId: string): boolean => this.liveOwnerOrNull(sessionId)?.holdsDispatch?.(sessionId) ?? false + holdsLiveProviderProcess = (sessionId: string, acquisitionGeneration: string): boolean => + this.liveOwnerOrNull(sessionId)?.holdsLiveProviderProcess?.(sessionId, acquisitionGeneration) ?? + false + stopEndsSession = (sessionId: string): boolean => this.liveOwnerOrNull(sessionId)?.stopEndsSession?.(sessionId) ?? false diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 5a90d2cf25c2..b7add5fadbd5 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -341,6 +341,9 @@ export type StructuredAgentSessionAdapter = StructuredAgentSessionAdapterStop & /** The provider reported taking a send it has neither answered nor ended, as a queued follow-up * or a silent retry does. Derived from the live child; false with none. */ holdsDispatch?(sessionId: string): boolean + /** The adapter's own child for this exact acquisition has a pid and its root exit has not been + * seen: first-hand proof of life for lease renewal. Absent or false falls back to a PID probe. */ + holdsLiveProviderProcess?(sessionId: string, acquisitionGeneration: string): boolean /** The `/` surface the running provider reports for itself. Undefined when the * provider never reports one, which is what keeps the client on its catalog. */ readCommands?(sessionId: string): AgentSessionSlashCommand[] | undefined diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts index 84b636da49b6..c606a73e310f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts @@ -12,10 +12,7 @@ import type { StructuredAgentSessionHostSession, StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' -import { - endProviderChild, - markProviderChildExited -} from './structured-agent-session-provider-child' +import { endProviderChild } from './structured-agent-session-provider-child' import { releaseStoredStructuredAgentSessionOwnerAfterExit, type StructuredAgentSessionLeaseStore @@ -74,14 +71,6 @@ export function settleStructuredAgentSessionChildExit< context: StructuredAgentSessionChildExitContext, event: StructuredAgentSessionEndedEvent ): Promise { - // Before the queue: lease renewal must not vouch for a child whose exit is only waiting its turn. - const exited = context.sessions.get(event.sessionId) - if (exited) { - markProviderChildExited(exited, { - generation: event.acquisitionGeneration, - fence: event.fence - }) - } return context.serialize(event.sessionId, async () => { const child = context.sessions.get(event.sessionId)?.child if (!child || child.fence !== event.fence || child.generation !== event.acquisitionGeneration) { @@ -110,7 +99,6 @@ export async function endExitedStructuredAgentSessionChildUnderSerialize< if (!session || session.child !== child) { return } - markProviderChildExited(session, child) const { expected } = exit const close = child.close // Receipt of the exit is the one end time the host may record for a running turn. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts index bf473bfc6df0..a86a29b66cdf 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts @@ -126,7 +126,7 @@ describe('Claude root-exit stop', () => { claimKeyId: 'key-1', logger: createStructuredAgentSessionLogger() } - const runtimeState = new StructuredAgentSessionHostRuntimeState(deps) + const runtimeState = new StructuredAgentSessionHostRuntimeState(deps, undefined, () => false) claude.connections[0]!.handlers.onExit?.(new Error('provider exited')) await expect( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts index a2de3f338743..2c74b0fb77bc 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts @@ -318,10 +318,11 @@ describe('deferred structured agent-session event sink', () => { it('replaces a failed cached sink before recovery drain', async () => { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the cached-sink path reads only the logger, on the failed drain. - const runtime = new StructuredAgentSessionHostRuntimeState({ + const deps = { store: {}, logger: createStructuredAgentSessionLogger() - } as never) + } as never + const runtime = new StructuredAgentSessionHostRuntimeState(deps, undefined, () => false) const failed = runtime.eventSinkFor('session-1') failed.bind(target(1, [], 0)) failed.sink.appendItem(identity(0), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts index 4da10e213f6a..fe6c1dbe6ee2 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts @@ -38,11 +38,12 @@ function context(closeError?: Error): StructuredAgentSessionEvictionContext & { function runtimeState(): StructuredAgentSessionHostRuntimeState { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: eviction against the sink cache reads only the sinks and the logger; store and adapter are never reached. - return new StructuredAgentSessionHostRuntimeState({ + const deps = { store: {}, adapter: {}, logger: recordingStructuredAgentSessionLogger().logger - } as never) + } as never + return new StructuredAgentSessionHostRuntimeState(deps, undefined, () => false) } describe("the route release after a child's exit", () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts index cabab0d2cc9d..30e1399f91c1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-held-child-lease-renewal.test.ts @@ -7,18 +7,10 @@ import { codexProviderHandle } from '../../../shared/agent-session-provider-hand import { probeAgentSessionProcessIdentity } from '../../runtime/agent-session-process-identity-probe' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' -import { - settleStructuredAgentSessionChildExit, - type StructuredAgentSessionChildExitContext, - type StructuredAgentSessionChildExitSession -} from './structured-agent-session-child-exit' import type { StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' import { StructuredAgentSessionLeaseRenewer } from './structured-agent-session-lease-renewer' import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' -import { - holdsLiveProviderChild, - markProviderChildExited -} from './structured-agent-session-provider-child' +import { holdsLiveProviderChild } from './structured-agent-session-provider-child' const NOW = 1_800_000_000_000 const SESSION = 'session-held' @@ -91,16 +83,19 @@ function pidProbe(platform: NodeJS.Platform) { ) } +/** The host's child record, read against an adapter that owns the process for `gen-1` until the + * test says its root exited. */ function hostWith(child: StructuredAgentSessionProviderChild | null) { - const session: StructuredAgentSessionChildExitSession = { - child, - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Only the exit-receipt mark runs here; the serialized settlement that reads the journal never does. - journal: {} as StructuredAgentSessionChildExitSession['journal'] - } + const session = { child } + const adapter: { liveGeneration: string | null } = { liveGeneration: 'gen-1' } return { - session, + adapter, holdsLiveChild: (sessionId: string, fence: number) => - holdsLiveProviderChild(sessionId === SESSION ? session : undefined, fence) + holdsLiveProviderChild( + sessionId === SESSION ? session : undefined, + fence, + (generation) => generation === adapter.liveGeneration + ) } } @@ -138,20 +133,16 @@ describe.each(['darwin', 'win32'] as const)('lease renewal of a held child on %s expect(log.entries).toEqual([]) }) - it('stops renewing once the held child exit is received', async () => { + it('stops renewing the moment the adapter sees the root exit', async () => { const { store, fence } = await liveStoreWithoutStartTime() - const child: StructuredAgentSessionProviderChild = { - generation: 'gen-1', - fence, - phase: 'ready' - } - const { session, holdsLiveChild } = hostWith(child) + const { adapter, holdsLiveChild } = hostWith({ generation: 'gen-1', fence, phase: 'ready' }) const probe = pidProbe(platform) const clock = { now: NOW + 10_000 } const { renewer } = renewerFor(store, probe, clock, holdsLiveChild) await renewer.renewNow() - markProviderChildExited(session, child) + // The child is still on the host's record: only the adapter has seen the exit so far. + adapter.liveGeneration = null clock.now = NOW + 20_000 await renewer.renewNow() @@ -184,9 +175,11 @@ describe.each(['darwin', 'win32'] as const)('lease renewal of a held child on %s it('still probes a record this runtime does not hold at its fence', async () => { const { store, fence } = await liveStoreWithoutStartTime() - // An older child on record, or none at all, holds nothing at the record's fence. + // An older child, one whose acquisition the adapter does not run, or none at all. for (const child of [ { generation: 'gen-0', fence: fence - 1, phase: 'ready' } as const, + { generation: 'gen-2', fence, phase: 'ready' } as const, + { generation: null, fence, phase: 'ready' } as const, null ]) { const { holdsLiveChild } = hostWith(child) @@ -208,35 +201,3 @@ describe.each(['darwin', 'win32'] as const)('lease renewal of a held child on %s } }) }) - -describe('receipt of a child exit', () => { - it('ends the held child proof before the exit settlement gets its turn', async () => { - const child: StructuredAgentSessionProviderChild = { - generation: 'gen-1', - fence: 3, - phase: 'ready' - } - const { session, holdsLiveChild } = hostWith(child) - const context = { - sessions: new Map([[SESSION, session]]), - // The session's queue is busy: the settlement itself never starts in this test. - serialize: () => new Promise(() => {}) - } - - void settleStructuredAgentSessionChildExit( - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The receipt mark reads only `sessions`; everything else runs inside `serialize`, which never starts here. - context as unknown as StructuredAgentSessionChildExitContext, - { - type: 'ended', - sessionId: SESSION, - reason: 'exited', - cause: 'unexpected-exit', - fence: 3, - acquisitionGeneration: 'gen-1' - } - ) - - expect(session.child).toBe(child) - expect(holdsLiveChild(SESSION, 3)).toBe(false) - }) -}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-held-child-renewal.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-held-child-renewal.test.ts new file mode 100644 index 000000000000..4db7d146a068 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-held-child-renewal.test.ts @@ -0,0 +1,114 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { codexProviderHandle } from '../../../shared/agent-session-provider-handle-encoding' +import { probeAgentSessionProcessIdentity } from '../../runtime/agent-session-process-identity-probe' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { + closeTestJournalHostDatabase, + openTestJournalHostDatabase +} from '../agent-session-journal/journal-host-database-test-support' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams +} from './structured-agent-session-host-test-data' +import { recordingProductionStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' + +const RENEW_INTERVAL_MS = 10_000 +const roots: string[] = [] + +afterEach(async () => { + vi.useRealTimers() + for (const root of roots) { + closeTestJournalHostDatabase(root) + } + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +/** A real host whose adapter spawned a child it could not read a start time for. */ +async function hostWithStartTimeLessChild(platform: NodeJS.Platform) { + const root = await mkdtemp(join(tmpdir(), 'orca-host-held-renewal-')) + roots.push(root) + const store = await openTestAgentSessionRecordStore(root) + const clock = { now: NOW } + const process = { live: true } + const holdsLiveProviderProcess = vi.fn( + (_sessionId: string, generation: string) => process.live && generation === 'gen-1' + ) + const adapter: StructuredAgentSessionAdapter = { + acquire: async ({ fence }) => ({ + process: { + hostId: 'local', + pid: 4242, + processStartTimeMs: null, + spawnToken: store.getRecord(SESSION)?.lease.reservedSpawnToken ?? 'spawn-a' + }, + link: { + linkId: `link-${fence}`, + handle: codexProviderHandle(THREAD), + origin: 'created', + mintedAtFence: fence, + observedAt: NOW + }, + acquisitionGeneration: 'gen-1' + }), + holdsLiveProviderProcess, + dispatch: vi.fn(), + cancelTurn: vi.fn(), + answerPrompt: vi.fn(), + setOption: vi.fn() + } + // The real PID probe with the pid present: no start time and no token echo to match. + const probeOwner = vi.fn((record: AgentSessionRecord) => + probeAgentSessionProcessIdentity({ + identity: record.lease.ownerProcess!, + deps: { platform, isPidPresent: () => true } + }) + ) + const host = new StructuredAgentSessionHost({ + logger: recordingProductionStructuredAgentSessionLogger().logger, + store, + adapter, + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-a', + probeOwner, + now: () => clock.now + }) + return { host, store, clock, process, probeOwner, holdsLiveProviderProcess } +} + +describe.each(['darwin', 'win32'] as const)('host lease renewal on %s', (platform) => { + it('renews a held child without a PID probe until the adapter sees its root exit', async () => { + vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval'] }) + const { host, store, clock, process, probeOwner, holdsLiveProviderProcess } = + await hostWithStartTimeLessChild(platform) + const attached = await host.attach({ callerKey: 'client-1' }, hostTestAttachParams(null)) + expect(attached.ok).toBe(true) + const fence = store.getRecord(SESSION)!.lease.runtimeFence + + clock.now = NOW + RENEW_INTERVAL_MS + await vi.advanceTimersByTimeAsync(RENEW_INTERVAL_MS) + await vi.waitFor(() => + expect(store.getRecord(SESSION)?.lease.lastRenewedAt).toBe(NOW + RENEW_INTERVAL_MS) + ) + expect(probeOwner).not.toHaveBeenCalled() + expect(holdsLiveProviderProcess).toHaveBeenCalledWith(SESSION, 'gen-1') + + // The adapter saw the root exit; the host has not settled it, so its child is still on record. + process.live = false + clock.now = NOW + 2 * RENEW_INTERVAL_MS + await vi.advanceTimersByTimeAsync(RENEW_INTERVAL_MS) + await vi.waitFor(() => expect(probeOwner).toHaveBeenCalledOnce()) + + expect(store.getRecord(SESSION)?.lease.runtimeFence).toBe(fence) + expect(store.getRecord(SESSION)?.lease.lastRenewedAt).toBe(NOW + RENEW_INTERVAL_MS) + await host.flushAllStreamedEvents() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts index e8421b028933..9678a6d7cbae 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts @@ -73,7 +73,7 @@ function runtimeState( probeOwner, logger: createStructuredAgentSessionLogger() } as StructuredAgentSessionHostDeps - return new StructuredAgentSessionHostRuntimeState(deps) + return new StructuredAgentSessionHostRuntimeState(deps, undefined, () => false) } function liveRecord(): AgentSessionRecord { @@ -149,7 +149,7 @@ describe('host runtime-state owner probe', () => { probeOwner, logger: log.logger } as unknown as StructuredAgentSessionHostDeps - const state = new StructuredAgentSessionHostRuntimeState(deps, onEventSinkFailure) + const state = new StructuredAgentSessionHostRuntimeState(deps, onEventSinkFailure, () => false) await ( state as unknown as { leaseRenewer: { renewNow: () => Promise } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts index de3d8c00fb33..db5cd6587193 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts @@ -16,15 +16,16 @@ export class StructuredAgentSessionHostRuntimeState { constructor( private readonly deps: StructuredAgentSessionHostDeps, - onEventSinkFailure?: (sessionId: string, error: unknown) => void, - holdsLiveChild?: (sessionId: string, fence: number) => boolean + onEventSinkFailure: ((sessionId: string, error: unknown) => void) | undefined, + /** Required: dropping it silently sends every held child back to the PID probe. */ + holdsLiveChild: (sessionId: string, fence: number) => boolean ) { this.onEventSinkFailure = onEventSinkFailure this.leaseRenewer = new StructuredAgentSessionLeaseRenewer({ store: deps.store, probe: (record) => this.probeRecord(record), ...(deps.probeOwners ? { probeMany: deps.probeOwners } : {}), - ...(holdsLiveChild ? { holdsLiveChild } : {}), + holdsLiveChild, now: () => deps.now?.() ?? Date.now(), // Lease/ownership failures are transient and stay on the visible lease-error path. // Only deferred sink I/O failures are terminal and may force-close a provider. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts index b69d48ff162e..7016e91e17c6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts @@ -68,8 +68,6 @@ export type StructuredAgentSessionProviderChild = StructuredAgentSessionProvider * for any other start. In memory only: it tells a restart offer its own start from another. */ readonly startedFor?: string close?: StructuredAgentSessionChildClose - /** The host received the child's exit; it stays on record only until that exit is settled. */ - exitObserved?: true } /** What ending a child established about its provider root. A stop's comes only from diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index fd2e5820928b..f5a6a4f9a501 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -52,7 +52,7 @@ import { import { structuredAgentSessionRestartResumeSurfaces } from './structured-agent-session-restart-resume-wiring' import { createStructuredAgentSessionConversationDelivery } from './structured-agent-session-host-delivery' import { - holdsLiveProviderChild, + heldProviderChildReader, structuredAgentSessionConversationFence } from './structured-agent-session-provider-child' import { wireStructuredAgentSessionQueuedMessages } from './structured-agent-session-queued-wiring' @@ -115,7 +115,7 @@ export class StructuredAgentSessionHost { this.runtimeState = new StructuredAgentSessionHostRuntimeState( deps, (sessionId, error) => this.eventRecovery.recoverAfterSinkFailure(sessionId, error), - (sessionId, fence) => holdsLiveProviderChild(this.sessions.get(sessionId), fence) + heldProviderChildReader(this.sessions, deps.adapter) ) this.reconcileLeases = createRestartReconciler({ store: deps.store, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts index f428ab59a1bf..a937458f24b6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts @@ -105,6 +105,7 @@ describe('structured agent-session lease renewal', () => { ) ) const renewer = new StructuredAgentSessionLeaseRenewer({ + holdsLiveChild: () => false, logger: recordingStructuredAgentSessionLogger().logger, store: { listRecords: () => records, renewLeases } as unknown as AgentSessionRecordStore, probe: vi.fn(), @@ -149,6 +150,7 @@ describe('structured agent-session lease renewal', () => { }) const log = recordingStructuredAgentSessionLogger() const renewer = new StructuredAgentSessionLeaseRenewer({ + holdsLiveChild: () => false, store: { listRecords: () => records, renewLeases, @@ -178,6 +180,7 @@ describe('structured agent-session lease renewal', () => { const store = await liveStore() let now = NOW const renewer = new StructuredAgentSessionLeaseRenewer({ + holdsLiveChild: () => false, logger: recordingStructuredAgentSessionLogger().logger, store, probe: async () => ({ @@ -208,6 +211,7 @@ describe('structured agent-session lease renewal', () => { releaseProbe = resolve }) const renewer = new StructuredAgentSessionLeaseRenewer({ + holdsLiveChild: () => false, logger: recordingStructuredAgentSessionLogger().logger, store, probe: async () => { @@ -234,6 +238,7 @@ describe('structured agent-session lease renewal', () => { matchedOn: ['process-start-time' as const] })) const renewer = new StructuredAgentSessionLeaseRenewer({ + holdsLiveChild: () => false, logger: recordingStructuredAgentSessionLogger().logger, store, probe, @@ -250,6 +255,7 @@ describe('structured agent-session lease renewal', () => { const store = await liveStore() const log = recordingStructuredAgentSessionLogger() const renewer = new StructuredAgentSessionLeaseRenewer({ + holdsLiveChild: () => false, store, probe: async () => ({ outcome: 'indeterminate', reason: 'probe unavailable' }), now: () => NOW + 10_000, @@ -279,6 +285,7 @@ describe('structured agent-session lease renewal', () => { matchedOn: ['process-start-time' as const] })) const renewer = new StructuredAgentSessionLeaseRenewer({ + holdsLiveChild: () => false, logger: recordingStructuredAgentSessionLogger().logger, store, probe, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts index 3698ff1f3070..f969c65428f3 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts @@ -28,7 +28,7 @@ export class StructuredAgentSessionLeaseRenewer { records: readonly AgentSessionRecord[] ) => Promise> /** Whether this runtime holds the session's child at `fence` and has not seen it exit. */ - holdsLiveChild?: (sessionId: string, fence: number) => boolean + holdsLiveChild: (sessionId: string, fence: number) => boolean now: () => number logger: StructuredAgentSessionLogger intervalMs?: number @@ -82,7 +82,7 @@ export class StructuredAgentSessionLeaseRenewer { // A child this runtime holds proves itself; a PID probe can be indeterminate for it (no start // time, no token echo), which would freeze the lease at its last proof. const holds = (record: AgentSessionRecord): boolean => - this.input.holdsLiveChild?.(record.sessionId, record.lease.runtimeFence) === true + this.input.holdsLiveChild(record.sessionId, record.lease.runtimeFence) const held = records.filter(holds) const unheld = records.filter((record) => !held.includes(record)) const probes = diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts index 5288318dbcf4..7a5dc22c1c69 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts @@ -8,6 +8,7 @@ import type { AgentJournalCursor } from '../../../shared/agent-session-journal-types' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import type { StructuredAgentSessionEndedChild, StructuredAgentSessionHostSession, @@ -47,25 +48,30 @@ export function markProviderChildStarted( return child !== null } -/** From the moment the host receives the exit: the child stops vouching for its lease. */ -export function markProviderChildExited( - session: ChildBearer, - identity: StructuredAgentSessionProviderChildIdentity -): void { - const child = matchingChild(session, identity) - if (child) { - child.exitObserved = true - } -} - -/** This runtime holds a child at `fence` whose exit it has not received: first-hand proof of life - * that needs no PID probe. A previous runtime's child is never on record here. */ +/** This runtime holds a child at `fence` whose process its adapter still sees running: first-hand + * proof of life that needs no PID probe. A previous runtime's child is never on record here. */ export function holdsLiveProviderChild( session: Pick | undefined, - fence: number + fence: number, + processLive: (acquisitionGeneration: string) => boolean ): boolean { const child = session?.child - return !!child && child.fence === fence && child.exitObserved !== true + return ( + !!child && child.fence === fence && child.generation !== null && processLive(child.generation) + ) +} + +/** Lease renewal's held-child read, from the host's child record and the adapter's own handle. */ +export function heldProviderChildReader( + sessions: { get(sessionId: string): Pick | undefined }, + adapter: Pick +): (sessionId: string, fence: number) => boolean { + return (sessionId, fence) => + holdsLiveProviderChild( + sessions.get(sessionId), + fence, + (generation) => adapter.holdsLiveProviderProcess?.(sessionId, generation) === true + ) } /** `endedAt` is a close's ask; an exit of the child's own ends where the journal stands. */ diff --git a/src/main/runtime/structured-agent-session-failure-logging.test.ts b/src/main/runtime/structured-agent-session-failure-logging.test.ts index 14efe114f903..b3c28be73e96 100644 --- a/src/main/runtime/structured-agent-session-failure-logging.test.ts +++ b/src/main/runtime/structured-agent-session-failure-logging.test.ts @@ -132,9 +132,8 @@ describe('failures the desktop host used to drop', () => { it('logs provider events the chat journal would not take', async () => { const log = recordingStructuredAgentSessionLogger() // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: minting and binding a sink reads no other host dependency. - const runtime = new StructuredAgentSessionHostRuntimeState({ - logger: log.logger - } as unknown as StructuredAgentSessionHostDeps) + const deps = { logger: log.logger } as unknown as StructuredAgentSessionHostDeps + const runtime = new StructuredAgentSessionHostRuntimeState(deps, undefined, () => false) const sink = runtime.eventSinkFor(SESSION) const error = new Error('journal append failed') // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the sink calls only appendItem before it fails.