From a434edf991f22760973d3f84df343a10a0e685db Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:48:52 -0600 Subject: [PATCH] fix(runner-e2e): treat macOS EPERM from a zombie-only process group as exited (#19470) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- tests/runner-e2e/README.md | 5 ++- tests/runner-e2e/process-tree-owner.test.ts | 35 +++++++++++++++++++++ tests/runner-e2e/process-tree-owner.ts | 19 ++++++++--- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index 7e29d3539cf..2cdca967e73 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -2139,7 +2139,10 @@ retirement. 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 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 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 diff --git a/tests/runner-e2e/process-tree-owner.test.ts b/tests/runner-e2e/process-tree-owner.test.ts index f53b9d4340f..4f32f088ca7 100644 --- a/tests/runner-e2e/process-tree-owner.test.ts +++ b/tests/runner-e2e/process-tree-owner.test.ts @@ -174,3 +174,38 @@ it.skipIf(process.platform === 'win32')('bounds fallback even when the known dir expect(signals).toEqual(['SIGTERM', 'SIGKILL']); } 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(); } +}); diff --git a/tests/runner-e2e/process-tree-owner.ts b/tests/runner-e2e/process-tree-owner.ts index 3dcae17791f..ed12bf10d72 100644 --- a/tests/runner-e2e/process-tree-owner.ts +++ b/tests/runner-e2e/process-tree-owner.ts @@ -138,7 +138,14 @@ export function createProcessTreeOwner(root: ChildProcess, options: { const ids = new Set(preserved.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) { + /** 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) { const currentProcessGroupId = table.find(row => row.pid === process.pid)?.processGroupId ?? null; if (currentProcessGroupId === null) throw new Error("Cleanup caller group identity is unavailable"); const live = liveGroups(); @@ -147,14 +154,18 @@ export function createProcessTreeOwner(root: ChildProcess, options: { for (const pid of ordered) { if (selected && !selected.has(pid)) continue; 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); } return signaled; } async function signal(signal: NodeJS.Signals, selected?: ReadonlySet) { await observe(); - return signalSnapshot(signal, selected); + return await signalSnapshot(signal, selected); } function gracefulRoots() { const live = liveGroups(); @@ -183,7 +194,7 @@ export function createProcessTreeOwner(root: ChildProcess, options: { // Choose and deliver from one validated table. ESRCH grants no ownership // coverage: the next grace poll can select that branch's surviving owner. 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) { if (pid === root.pid) directGraceDelivered = true; covered.add(pid);