fix(runner-e2e): treat macOS EPERM from a zombie-only process group as exited (#19470)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The runner E2E suite restarts the Paperclip server to prove that
agent work recovers after a controller restart
> - The restart supervisor in `tests/runner-e2e` owns the server process
tree and escalates to SIGKILL when graceful stop takes too long
> - On macOS, `kill(-pgid)` returns EPERM, not ESRCH, when every member
of the group has exited but is not reaped yet
> - The supervisor treated that EPERM as fatal, so a server that exited
at the same moment as the SIGKILL escalation aborted the restart with
"Owned tree cleanup incomplete: Error: kill EPERM"
> - This pull request makes the supervisor re-inspect the process table
on EPERM and skip the group only when no running member remains
> - The benefit is that restart recovery cases no longer fail on a
harmless exit race on macOS, while a real permission failure still fails
cleanup

## Linked Issues or Issue Description

No public issue exists. This describes the bug.

**What happened?**
The runner E2E cell
`everyday-workflows.runner-codex.local.recover-controller` failed on
macOS with failure class `cleanup_failure`: "Stopped waiting for
isolated server restart ...: Owned tree cleanup incomplete: Error: kill
EPERM". The server log shows that the graceful stop took 30 seconds, the
supervisor sent SIGKILL, and the server exited with code 0 in the same
instant. The restart did not start, so the case never checked recovery.

**Steps to reproduce**
1. On macOS, fork a child that calls `setpgid(0, 0)` and exits. Do not
reap it.
2. Call `killpg(child, SIGKILL)`. The kernel returns EPERM (errno 1),
not ESRCH. After the reap, it returns ESRCH.
3. In the runner E2E supervisor, any group that becomes zombie-only
between the `ps` inspection and the signal causes the same EPERM, which
the code rethrows.

**Expected behavior**
A group whose members have all exited counts as stopped, the same as
ESRCH. EPERM stays fatal while a running member remains.

**Environment**
macOS 26.5 (Darwin 25.5.0), arm64, Node 26.4. The behavior is the same
with and without the Claude Code sandbox.

## What Changed

- `tests/runner-e2e/process-tree-owner.ts`: on EPERM from a group
signal, read a fresh process table. Skip the group (as with ESRCH) only
when it has no running (non-zombie) member. Otherwise rethrow.
`signalSnapshot` is now async.
- `tests/runner-e2e/process-tree-owner.test.ts`: two regressions. One
covers a group that becomes zombie-only when signaled. One proves that
EPERM stays fatal while a running member remains.
- `tests/runner-e2e/README.md`: document the EPERM rule next to the
existing zombie rule.

## Verification

- Red before the fix: `node node_modules/vitest/vitest.mjs run --config
tests/runner-e2e/vitest.config.ts
tests/runner-e2e/process-tree-owner.test.ts` failed with `promise
rejected "Error: kill EPERM"`.
- Green after the fix: `process-tree-owner.test.ts`,
`server-stop.test.ts`, and `process-tree.test.ts` pass (35 tests). `pnpm
test:e2e:runner:typecheck` passes.
- Kernel behavior: a Python fork probe on this Mac shows that `killpg`
on an unreaped zombie group returns EPERM, both inside and outside the
sandbox.
- Live cell: `pnpm test:e2e:runner -- --max-automatic-retries 0 --id
everyday-workflows.runner-codex.local.recover-controller` on macOS. The
fixed revision passed. The server stop escalated to SIGKILL, the
replacement server started, and the run recovered. See the comment below
for the second rerun.

## Risks

- Low risk. The change is in the test harness only.
- EPERM is ignored only after a fresh inspection shows that no running
process holds that group ID. A process group ID cannot be reused while
zombie members still hold it, so the supervisor does not signal or skip
an unrelated group.
- The extra `ps` read runs only on EPERM.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, file edits). Default context window.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Opus 5.5 authored and GitHub committed 2026-11-06 09:48:52 -06:00
1 parent eb5c24b61c
commit a434edf991
3 files changed
+54 -5

No files matched your search

+4 -1
View File
@@ -2139,7 +2139,10 @@ retirement.
Launcher cancellation first signals its outer group, so Playwright and the wrapper Launcher cancellation first signals its outer group, so Playwright and the wrapper
own graceful server shutdown. It allows 45 seconds for that chain before bounded own graceful server shutdown. It allows 45 seconds for that chain before bounded
5-second forced cleanup of revalidated descendants. Normal launcher exit uses the 5-second forced cleanup of revalidated descendants. Normal launcher exit uses the
same retained descendant inventory. Zombies count as stopped; an unavailable or same retained descendant inventory. Zombies count as stopped, including when
macOS answers a group signal with EPERM because every remaining member is an
unreaped zombie; EPERM stays fatal while a fresh inspection shows a running
member. An unavailable or
uncertain identity inspection triggers bounded direct-child-only retirement and uncertain identity inspection triggers bounded direct-child-only retirement and
still fails the tree audit. Failed cleanup preserves the temporary state. Graceful still fails the tree audit. Failed cleanup preserves the temporary state. Graceful
owners are selected from the same validated table used for delivery; ESRCH allows owners are selected from the same validated table used for delivery; ESRCH allows
@@ -174,3 +174,38 @@ it.skipIf(process.platform === 'win32')('bounds fallback even when the known dir
expect(signals).toEqual(['SIGTERM', 'SIGKILL']); expect(signals).toEqual(['SIGTERM', 'SIGKILL']);
} finally { owner.stopObserving(); } } finally { owner.stopObserving(); }
}); });
function epermFixture(afterSignal: ProcessObservation[]) {
let table = [row(process.pid, 1, 10), row(100, process.pid, 100), row(200, 100, 200)];
const root = { pid: 100, exitCode: null as number | null, signalCode: null as NodeJS.Signals | null };
const attempts: Array<[number, NodeJS.Signals]> = [];
const owner = createProcessTreeOwner(root as ChildProcess, {
readTable: async () => table,
signalGroup: (pid, signal) => {
attempts.push([pid, signal]);
if (pid !== 200) return;
// Darwin killpg reports EPERM, not ESRCH, once every group member exited
// but is not reaped yet. The group exits between inspection and signal.
table = [row(process.pid, 1, 10), row(100, process.pid, 100), ...afterSignal];
throw Object.assign(new Error("kill EPERM"), { code: "EPERM" });
},
});
return { owner, attempts };
}
it.skipIf(process.platform === "win32")("treats EPERM from a group that is now only zombies as already exited", async () => {
const f = epermFixture([row(200, 100, 200, "start-200", "Z")]);
try {
await f.owner.observe();
await expect(f.owner.signal("SIGKILL")).resolves.toEqual([100]);
expect(f.attempts).toEqual([[200, "SIGKILL"], [100, "SIGKILL"]]);
} finally { f.owner.stopObserving(); }
});
it.skipIf(process.platform === "win32")("keeps EPERM fatal while the group still has a running member", async () => {
const f = epermFixture([row(200, 100, 200, "start-200", "Z"), row(201, 200, 200)]);
try {
await f.owner.observe();
await expect(f.owner.signal("SIGKILL")).rejects.toThrow("kill EPERM");
} finally { f.owner.stopObserving(); }
});
+15 -4
View File
@@ -138,7 +138,14 @@ export function createProcessTreeOwner(root: ChildProcess, options: {
const ids = new Set(preserved.map(group => group.processGroupId)); const ids = new Set(preserved.map(group => group.processGroupId));
return new Set(liveGroups().filter(group => !ids.has(group.processGroupId)).map(group => group.processGroupId)); return new Set(liveGroups().filter(group => !ids.has(group.processGroupId)).map(group => group.processGroupId));
} }
function signalSnapshot(signal: NodeJS.Signals, selected?: ReadonlySet<number>) { /** Darwin killpg reports EPERM, not ESRCH, when every remaining group member
* has exited but is not yet reaped. A zombie-only group needs no signal; any
* running member keeps EPERM a real failure. */
async function exitedSinceSnapshot(pid: number) {
const fresh = await readTable();
return !!fresh && !fresh.some(row => row.processGroupId === pid && running(row));
}
async function signalSnapshot(signal: NodeJS.Signals, selected?: ReadonlySet<number>) {
const currentProcessGroupId = table.find(row => row.pid === process.pid)?.processGroupId ?? null; const currentProcessGroupId = table.find(row => row.pid === process.pid)?.processGroupId ?? null;
if (currentProcessGroupId === null) throw new Error("Cleanup caller group identity is unavailable"); if (currentProcessGroupId === null) throw new Error("Cleanup caller group identity is unavailable");
const live = liveGroups(); const live = liveGroups();
@@ -147,14 +154,18 @@ export function createProcessTreeOwner(root: ChildProcess, options: {
for (const pid of ordered) { for (const pid of ordered) {
if (selected && !selected.has(pid)) continue; if (selected && !selected.has(pid)) continue;
try { signalGroup(pid, signal); } try { signalGroup(pid, signal); }
catch (error) { if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; else continue; } catch (error) {
const code = (error as NodeJS.ErrnoException).code;
if (code === "ESRCH" || (code === "EPERM" && await exitedSinceSnapshot(pid))) continue;
throw error;
}
signaled.push(pid); signaled.push(pid);
} }
return signaled; return signaled;
} }
async function signal(signal: NodeJS.Signals, selected?: ReadonlySet<number>) { async function signal(signal: NodeJS.Signals, selected?: ReadonlySet<number>) {
await observe(); await observe();
return signalSnapshot(signal, selected); return await signalSnapshot(signal, selected);
} }
function gracefulRoots() { function gracefulRoots() {
const live = liveGroups(); const live = liveGroups();
@@ -183,7 +194,7 @@ export function createProcessTreeOwner(root: ChildProcess, options: {
// Choose and deliver from one validated table. ESRCH grants no ownership // Choose and deliver from one validated table. ESRCH grants no ownership
// coverage: the next grace poll can select that branch's surviving owner. // coverage: the next grace poll can select that branch's surviving owner.
const selected = new Set([...gracefulRoots()].filter(pid => !covered.has(pid))); const selected = new Set([...gracefulRoots()].filter(pid => !covered.has(pid)));
const delivered = signalSnapshot(signal, selected); const delivered = await signalSnapshot(signal, selected);
for (const pid of delivered) { for (const pid of delivered) {
if (pid === root.pid) directGraceDelivered = true; if (pid === root.pid) directGraceDelivered = true;
covered.add(pid); covered.add(pid);