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 3804e1041d2..04d58067353 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 @@ -24,17 +24,19 @@ function windowsSnapshot(): WindowsDescendantSnapshot { } describe('Claude child root identity', () => { - it('advances a re-walked row boundary to the refresh that re-derived it', () => { + 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) - // Every row carries the refresh boundary, so no per-pid map is needed. expect( mergeClaudeCapturedTrees( { platform: 'posix', tree: previous }, { platform: 'posix', tree: next } ) - ).toEqual({ platform: 'posix', tree: next }) + ).toEqual({ + platform: 'posix', + tree: { ...next, capturedAtMsByPid: { '200': previous.capturedAtMs } } + }) }) it('keeps the descendant verdict when a POSIX root probe is unavailable', async () => { 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 be75c6427ed..3f15f8e8fca 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.test.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.test.ts @@ -513,20 +513,17 @@ describe('claude child tree reaper', () => { expect(child.kill).toHaveBeenCalledWith('SIGKILL') }) - it('advances a re-walked POSIX row boundary but not one the refresh missed', async () => { + it('keeps the original capture boundary for retained POSIX rows', async () => { const child = mockChild() - const unseen = { pid: 4245, ppid: 424242, pgid: 1, startedAt: 'Mon Jan 1 00:00:00 2026' } - const base = snapshotOf(4243) const first = { - ...base, - capturedAtMs: 1_700_000_000_900, - descendants: [...base.descendants, unseen] + ...snapshotOf(4243), + capturedAtMs: 1_700_000_000_900 } const refreshed = { - ...base, + ...first, capturedAtMs: 1_700_000_002_100, descendants: [ - ...base.descendants, + ...first.descendants, { pid: 4244, ppid: 424242, @@ -549,13 +546,11 @@ describe('claude child tree reaper', () => { expect(terminateDescendants).toHaveBeenCalledWith({ ...refreshed, - descendants: [...base.descendants, unseen, refreshed.descendants[1]], + // The retained 4243 row was first observed in the earlier displayed + // second. Its per-row boundary must not advance with the refresh. capturedAtMsByPid: { - // Re-derived from the live root by this walk, so proved ours again at it. - '4243': refreshed.capturedAtMs, - '4244': refreshed.capturedAtMs, - // Absent from the refresh: nothing re-proved it, so it keeps its own. - '4245': first.capturedAtMs + '4243': first.capturedAtMs, + '4244': refreshed.capturedAtMs } }) }) 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 c6bcfc60180..9f843527e30 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 @@ -167,16 +167,17 @@ describe('Claude root kill fallback', () => { expect(child.kill).not.toHaveBeenCalled() }) - it('chains the boundary of a row dropped from later walks', () => { - const first: WindowsDescendantSnapshot = { ...windowsSnapshot(1_000) } + it('chains per-pid Windows boundaries across a second merge', async () => { + const first = windowsSnapshot(1_000) const second: WindowsDescendantSnapshot = { ...windowsSnapshot(2_000), - descendants: [{ pid: 4244, creationTimeMs: 1_700_000_000_002 }] + descendants: [ + { pid: 4243, creationTimeMs: 1_700_000_000_000 }, + { pid: 4244, creationTimeMs: 1_700_000_000_002 } + ] } const third: WindowsDescendantSnapshot = { ...second, capturedAtMs: 3_000 } - // 4243 is absent from both refreshes, so it keeps the boundary of the walk - // that did see it rather than inheriting the latest scan's scalar. const merged = mergeClaudeCapturedTrees( { platform: 'win32', tree: first }, { platform: 'win32', tree: second } @@ -184,6 +185,6 @@ describe('Claude root kill fallback', () => { 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': 3_000 }) + expect(rechained?.tree.capturedAtMsByPid).toEqual({ '4243': 1_000, '4244': 2_000 }) }) }) diff --git a/src/main/claude/claude-child-exit-proof-ladder.ts b/src/main/claude/claude-child-exit-proof-ladder.ts index 4c685adb494..85ed629f1b9 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.ts @@ -19,10 +19,6 @@ export async function proveClaudeChildExitWithReaper( const tree = input.tree ?? createTree() // Arm before stdin closes: only a live root can identify its descendants. await tree.capture() - // And re-walk it here, while the root certainly still lives: a descendant the - // arm first saw inside its own birth second is otherwise never eligible for a - // forced sweep, and the root may exit before any later walk gets the chance. - await tree.refresh?.() try { input.child.stdin?.end() } catch { diff --git a/src/main/claude/claude-child-tree-snapshot.ts b/src/main/claude/claude-child-tree-snapshot.ts index 1bd9f57cda3..e0955648b02 100644 --- a/src/main/claude/claude-child-tree-snapshot.ts +++ b/src/main/claude/claude-child-tree-snapshot.ts @@ -10,9 +10,6 @@ export type ClaudeCapturedTree = * Process-table reads are not atomic: a refresh can omit a still-live row, but * it can also observe a new process after the old row exited. Retain rows absent * from the refresh, but reject a PID whose identity changed between reads. - * - * Each row also carries the instant its membership of the tree was last proved, - * which is what the forced sweep fences on. */ function mergeRowsByPid( previous: readonly Row[], @@ -37,11 +34,9 @@ function mergeRowsByPid( if (prior && !sameIdentity(prior, row)) { return null } - // Why a retained row may take the later boundary: this walk re-derived it - // from the live root by ppid, which proves the process holding that pid is - // ours at this instant without appealing to its start time. A row only the - // earlier walk saw keeps the earlier boundary -- nothing re-proved it. - capturedAtMsByPid[String(row.pid)] = nextBoundary(row) + if (!prior) { + capturedAtMsByPid[String(row.pid)] = nextBoundary(row) + } merged.set(row.pid, row) } const boundaries = Object.values(capturedAtMsByPid) diff --git a/src/main/claude/claude-descendant-escalation-boundary.test.ts b/src/main/claude/claude-descendant-escalation-boundary.test.ts index 5ba1f9fedb0..55459df0c7f 100644 --- a/src/main/claude/claude-descendant-escalation-boundary.test.ts +++ b/src/main/claude/claude-descendant-escalation-boundary.test.ts @@ -1,62 +1,67 @@ import { describe, expect, it, vi } from 'vitest' -import type { DescendantTreeVerdict } from '../pty-descendant-exit-verification' import { terminateDescendantSnapshotWithVerdict } from '../pty-descendant-exit-verification' import { collectDescendantRows, type DescendantSnapshot, type ProcessTableRow } from '../pty-descendant-termination' -import { createClaudeChildTreeReaper, proveClaudeChildExit } from './claude-agent-sdk-exit-proof' +import { createClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' const ROOT_PID = 500 const ORCA_PGID = 400 -const ROOT_STARTED_AT = 'Thu Sep 3 16:37:20 2026' -const BIRTH_SECOND = 'Thu Sep 3 16:37:40 2026' -const BIRTH_MS = Date.parse(BIRTH_SECOND) -/** The measured shape: ten MCP-like children forked inside second :40. */ -const DESCENDANT_PIDS = [600, 601, 602, 603, 604, 605, 606, 607, 608, 609] +const ROOT_STARTED_AT = 'Thu Sep 3 18:04:50 2026' +/** The second both close-time walks land in. */ +const WALK_SECOND = 'Thu Sep 3 18:05:04 2026' +const WALK_MS = Date.parse(WALK_SECOND) +const EARLIER_SECOND = 'Thu Sep 3 18:05:03 2026' -function tableRows(descendantStartedAt: string): ProcessTableRow[] { +/** The measured split: `s20` at :03.946 died, `s21` at :04.042 leaked. */ +const EARLIER_BORN = [700, 701, 702] +const WALK_SECOND_BORN = [721, 722, 723, 724] + +type Cohort = { pids: number[]; startedAt: string } + +const LIVE_TREE: Cohort[] = [ + { pids: EARLIER_BORN, startedAt: EARLIER_SECOND }, + { pids: WALK_SECOND_BORN, startedAt: WALK_SECOND } +] + +function rowsFor(cohorts: Cohort[]): ProcessTableRow[] { return [ { pid: ROOT_PID, ppid: 1, pgid: ORCA_PGID, startedAt: ROOT_STARTED_AT }, - ...DESCENDANT_PIDS.map((pid) => ({ - pid, - ppid: ROOT_PID, - pgid: ORCA_PGID, - startedAt: descendantStartedAt - })) + ...cohorts.flatMap((cohort) => + cohort.pids.map((pid) => ({ + pid, + ppid: ROOT_PID, + pgid: ORCA_PGID, + startedAt: cohort.startedAt + })) + ) ] } /** A real ppid walk from the root, exactly as production captures one. */ -function walk(capturedAtMs: number, descendantStartedAt = BIRTH_SECOND): DescendantSnapshot { - return collectDescendantRows(ROOT_PID, tableRows(descendantStartedAt), capturedAtMs) -} - -/** - * Drives the real verifier against a process table where every descendant traps - * SIGTERM, so only a forced sweep can end them. - */ -function sigtermResistantVerifier(sendSignal: (pid: number, signal: NodeJS.Signals) => void) { - return (snapshot: DescendantSnapshot) => - terminateDescendantSnapshotWithVerdict(snapshot, { - requireIdentityBeforeSignal: true, - graceMs: 0, - verifyMs: 120, - sendSignal, - readTable: async () => ({ rows: tableRows(BIRTH_SECOND), capturedAtMs: Date.now() }) - }) +function walk(capturedAtMs: number, cohorts: Cohort[] = LIVE_TREE): DescendantSnapshot { + return collectDescendantRows(ROOT_PID, rowsFor(cohorts), capturedAtMs) } function killedPids(calls: [number, NodeJS.Signals][]): number[] { - return calls.flatMap(([pid, signal]) => (signal === 'SIGKILL' ? [pid] : [])) + return calls.flatMap(([pid, signal]) => (signal === 'SIGKILL' ? [pid] : [])).sort((a, b) => a - b) } function signalledPids(calls: [number, NodeJS.Signals][]): number[] { - return calls.flatMap(([pid, signal]) => (signal === 'SIGTERM' ? [pid] : [])) + return calls.flatMap(([pid, signal]) => (signal === 'SIGTERM' ? [pid] : [])).sort((a, b) => a - b) } -async function sweep(captures: DescendantSnapshot[]): Promise<[number, NodeJS.Signals][]> { +/** + * Drives the real reaper and the real verifier against a process table where + * every descendant traps SIGTERM, so only a forced sweep can end them. The root + * is alive for both walks and gone by the sweep, which is the measured teardown. + */ +async function sweep( + captures: DescendantSnapshot[], + liveTree: Cohort[] = LIVE_TREE +): Promise<[number, NodeJS.Signals][]> { const calls: [number, NodeJS.Signals][] = [] const captureDescendants = vi.fn() for (const capture of captures) { @@ -68,7 +73,14 @@ async function sweep(captures: DescendantSnapshot[]): Promise<[number, NodeJS.Si platform: 'linux', exited: () => false, captureDescendants, - terminateDescendants: sigtermResistantVerifier((pid, signal) => calls.push([pid, signal])) + terminateDescendants: (snapshot) => + terminateDescendantSnapshotWithVerdict(snapshot, { + requireIdentityBeforeSignal: true, + graceMs: 0, + verifyMs: 120, + sendSignal: (pid, signal) => calls.push([pid, signal]), + readTable: async () => ({ rows: rowsFor(liveTree), capturedAtMs: Date.now() }) + }) } ) // The close ladder's shape: arm, then re-walk the live root at the boundary. @@ -78,84 +90,35 @@ async function sweep(captures: DescendantSnapshot[]): Promise<[number, NodeJS.Si return calls } -describe('Claude descendant forced-sweep boundary', () => { - it('escalates descendants first seen in their own birth second', async () => { - // Arm lands inside second :40, the same second the children were forked in; - // the close-boundary walk re-derives them from the live root in second :41. - const calls = await sweep([walk(BIRTH_MS + 231), walk(BIRTH_MS + 1_200)]) +describe('Claude descendant forced-sweep fence', () => { + it('escalates a descendant forked in the same second as both close walks', async () => { + // Both walks land inside second :04, one ps duration apart, and the root is + // gone before a third could run. A descendant born at :04.042 is no less + // ours than its sibling born 96ms earlier at :03.946. + const calls = await sweep([walk(WALK_MS + 42), walk(WALK_MS + 140)]) - expect(signalledPids(calls)).toEqual(DESCENDANT_PIDS) - expect(killedPids(calls)).toEqual(DESCENDANT_PIDS) + expect(signalledPids(calls)).toEqual([...EARLIER_BORN, ...WALK_SECOND_BORN]) + expect(killedPids(calls)).toEqual([...EARLIER_BORN, ...WALK_SECOND_BORN]) }) - it('still escalates descendants born before the capture that first saw them', async () => { - const earlier = 'Thu Sep 3 16:37:17 2026' - const calls: [number, NodeJS.Signals][] = [] - const tree = createClaudeChildTreeReaper( - { pid: ROOT_PID, kill: vi.fn(() => true) }, - { - platform: 'linux', - exited: () => false, - captureDescendants: vi.fn(async () => - collectDescendantRows(ROOT_PID, tableRows(earlier), BIRTH_MS + 231) - ), - terminateDescendants: (snapshot) => - terminateDescendantSnapshotWithVerdict(snapshot, { - requireIdentityBeforeSignal: true, - graceMs: 0, - verifyMs: 120, - sendSignal: (pid, signal) => calls.push([pid, signal]), - readTable: async () => ({ rows: tableRows(earlier), capturedAtMs: Date.now() }) - }) - } - ) + it('still escalates descendants born before the walk that first saw them', async () => { + const onlyEarlier = [{ pids: EARLIER_BORN, startedAt: EARLIER_SECOND }] + const calls = await sweep([walk(WALK_MS + 42, onlyEarlier)], onlyEarlier) - await tree.capture() - await tree.reap() - - expect(killedPids(calls)).toEqual(DESCENDANT_PIDS) + expect(killedPids(calls)).toEqual(EARLIER_BORN) }) - it('withholds the sweep while no walk has re-proved the rows in a later second', async () => { - // Both walks land inside the birth second: nothing has disambiguated the - // rows, so the forced sweep must still stand down. - const calls = await sweep([walk(BIRTH_MS + 231), walk(BIRTH_MS + 640)]) + it('withholds the sweep from a row no walk re-derived, on its start second alone', async () => { + // 900 was seen once, in its own birth second, and the refresh did not find + // it. The merge retains the row, but nothing re-proved it belongs to us, so + // the second-resolution fence is all there is and it still says no. + const retained = { pids: [900], startedAt: WALK_SECOND } + const firstWalk = walk(WALK_MS + 42, [...LIVE_TREE, retained]) + const refresh = walk(WALK_MS + 140) - expect(signalledPids(calls)).toEqual(DESCENDANT_PIDS) - expect(killedPids(calls)).toEqual([]) - }) -}) + const calls = await sweep([firstWalk, refresh], [...LIVE_TREE, retained]) -describe('Claude close-boundary re-walk', () => { - it('re-walks the live root before stdin closes, not only after the grace window', async () => { - // The measured teardown: the root leaves on its own inside the grace window, - // so the post-timeout refresh never runs and this is the last walk that can - // happen while the root is still there to be walked from. - const order: string[] = [] - const tree = { - capture: vi.fn(async () => { - order.push('capture') - }), - refresh: vi.fn(async () => { - order.push('refresh') - }), - reap: vi.fn(async (): Promise => { - order.push('reap') - return 'exited' - }), - get treeVerdict(): DescendantTreeVerdict { - return 'unverifiable' - } - } - const stdin = { end: vi.fn(() => order.push('stdin-end')) } - - await proveClaudeChildExit({ - child: { pid: ROOT_PID, kill: vi.fn(() => true), stdin } as never, - exitPromise: Promise.resolve(), - exited: () => true, - tree - }) - - expect(order).toEqual(['capture', 'refresh', 'stdin-end', 'reap']) + expect(signalledPids(calls)).toEqual([...EARLIER_BORN, ...WALK_SECOND_BORN, 900]) + expect(killedPids(calls)).toEqual([...EARLIER_BORN, ...WALK_SECOND_BORN]) }) }) diff --git a/src/main/pty-descendant-exit-verification.ts b/src/main/pty-descendant-exit-verification.ts index 8d8903a3988..4c8471fa955 100644 --- a/src/main/pty-descendant-exit-verification.ts +++ b/src/main/pty-descendant-exit-verification.ts @@ -142,7 +142,15 @@ export async function terminateDescendantSnapshotWithVerdict( if (!forced && Date.now() >= deadline - verifyMs + graceMs) { forced = true for (const row of live) { + // A row a walk re-derived from a live root is ours whatever second it + // was born in, which start time alone can never establish for one born + // in its own capture second. Rows no walk re-derived still answer to + // the second-resolution fence, which is all the evidence they have. + // Scoped to the identity-revalidating callers; the same argument holds + // for the rest, but widening it is a deliberate change of its own. if ( + (deps.requireIdentityBeforeSignal === true && + snapshot.reDerivedPids?.has(row.pid) === true) || hasUnambiguousStartIdentity( row, snapshot.capturedAtMsByPid?.[String(row.pid)] ?? snapshot.capturedAtMs diff --git a/src/main/pty-descendant-termination.test.ts b/src/main/pty-descendant-termination.test.ts index fc4a41b8bc6..0c4bea81306 100644 --- a/src/main/pty-descendant-termination.test.ts +++ b/src/main/pty-descendant-termination.test.ts @@ -60,7 +60,9 @@ function snapshot( ...(rootPgid === null ? {} : { root: { pid: 10, startedAt: 'Mon Jul 13 12:54:47 2026' } }), rootPgid, descendants, - capturedAtMs + capturedAtMs, + // Everything a walk returns was re-derived by it. + ...(rootPgid === null ? {} : { reDerivedPids: new Set(descendants.map((row) => row.pid)) }) } } @@ -454,7 +456,9 @@ describe('terminateDescendantSnapshotAndWait', () => { const pending = terminateDescendantSnapshotWithVerdict( { ...snapshot([retained, fresh], 10, refreshBoundary), - capturedAtMsByPid: { '20': oldBoundary, '30': refreshBoundary } + capturedAtMsByPid: { '20': oldBoundary, '30': refreshBoundary }, + // What a merge produces: only the refresh re-derived 30; 20 is retained. + reDerivedPids: new Set([30]) }, { sendSignal, diff --git a/src/main/pty-descendant-termination.ts b/src/main/pty-descendant-termination.ts index 755c6b31da2..4f56c3e977b 100644 --- a/src/main/pty-descendant-termination.ts +++ b/src/main/pty-descendant-termination.ts @@ -32,6 +32,14 @@ export type DescendantSnapshot = { capturedAtMs: number /** Per-PID identity boundaries for merged captures. */ capturedAtMsByPid?: Readonly> + /** + * PIDs this walk re-derived from a live root. A ppid walk only reaches what + * the root actually parents, so membership is proof of ownership that owes + * nothing to `lstart`'s one-second resolution: a stranger would have to have + * been forked into our own tree, and then it is not a stranger. Rows a merge + * retained from an earlier walk are absent, and still answer to start time. + */ + reDerivedPids?: ReadonlySet } export type ProcessTableCapture = { @@ -204,7 +212,8 @@ export function collectDescendantRows( root: { pid: rootRow.pid, startedAt: rootRow.startedAt }, rootPgid: rootRow.pgid, descendants, - capturedAtMs + capturedAtMs, + reDerivedPids: new Set(descendants.map((row) => row.pid)) } }