fix(claude): fence the forced sweep on re-derivation, not on lstart's second

A descendant forked in the same wall-clock second as every walk that sees it was
signalled with SIGTERM and then never escalated, so a SIGTERM-resistant child
survived tab close, app quit and a full relaunch. Two children of one parent
96ms apart across a second boundary took opposite paths. The leak predates this
branch: it reproduces with the change reverted.

`ps lstart` has one-second resolution, so a walk landing inside a row's birth
second can never rule out a pid recycled later in that same second. But a walk
is not a match: a ppid walk only reaches what the root actually parents, and the
root is pinned by Node's own handle, so a row the walk re-derived is ours
whatever second it was born in -- a stranger would have to have been forked into
our tree, and then it is not a stranger. Fence the escalation on that.

Rows a merge retained from an earlier walk are not re-derived and still answer
to the start-time fence, which remains correct for them.

Scoped to callers that revalidate identity before signalling, which is the
Claude close path. Codex teardown reaches this same verifier and is unchanged;
the argument holds there too, but widening it is its own deliberate change.

Also reverts two changes from the previous attempt at this leak. Advancing the
capture boundary on a later walk is inert once the sweep fences on re-derivation
-- both key on the same set of rows, so the new term short-circuits for exactly
the rows whose boundary it advanced. The extra ladder refresh was a duplicate
full process-table read: close() already awaits tree.refresh() immediately
before proveClaudeChildExit, on the only path that reaches it.

Known property: the kill lands roughly a grace window after the walk that proved
membership, so a pid recycled inside that gap could in principle be signalled.
It is bounded -- matchingSnapshotRows already requires the live row to carry the
same start-second and pgid, so an impostor must be born in the remainder of that
one second, land on that exact pid, and sit in the same process group, and it
has already received the unfenced SIGTERM from the same loop.
This commit is contained in:
Merge Sim
2026-09-03 18:36:21 -07:00
parent 4108e951e5
commit bafc314bee
9 changed files with 117 additions and 144 deletions
@@ -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 () => {
@@ -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
}
})
})
@@ -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 })
})
})
@@ -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 {
@@ -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<Row extends { pid: number }>(
previous: readonly Row[],
@@ -37,11 +34,9 @@ function mergeRowsByPid<Row extends { pid: number }>(
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)
@@ -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<DescendantTreeVerdict> => {
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])
})
})
@@ -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
+6 -2
View File
@@ -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,
+10 -1
View File
@@ -32,6 +32,14 @@ export type DescendantSnapshot = {
capturedAtMs: number
/** Per-PID identity boundaries for merged captures. */
capturedAtMsByPid?: Readonly<Record<string, number>>
/**
* 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<number>
}
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))
}
}