Repository navigation
fix(inspect): omit Darwin census rows reaped before the trailing identity read - #25
Conversation
…tity read Each Darwin census row brackets topology with birth-identity reads. The trailing read was collapsed with .ok().flatten(), so a proven absence (no kern.proc.pid record and kill ESRCH) looked like unreadable identity: a process reaped after its topology read stayed in the census with parent, group, and state but no identity, and lineage consumers that require identity failed closed with a permission error. Omit the row when the trailing read proves the PID gone, matching a topology read that observes the disappearance. Unreadable and changed identities keep the existing topology-only behavior, so denial handling and the PID-reuse bracket are unchanged. A cfg(test) hook reaps an unreaped zombie between the two reads for a deterministic real-OS regression test; macOS native CI lanes now run cargo test so the Darwin-only module's tests execute.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesOmit Darwin census rows when the trailing identity read proves the process has disappeared. Example: A selected zombie process is reaped after its topology is read but before its trailing identity read.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it fixes a current Darwin census race that main still contains, and review found no introduced correctness or security defect. Priority: P2 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe inspect subpath passes census requests to the native Darwin owner, which combines kernel topology and bracketed birth identities into process snapshots. flowchart TD
A[Inspect API] --> B[Darwin census]
B --> C[Initial birth identity]
C --> D[Topology and optional command]
D --> E[Trailing birth identity]
E --> F[Omit proven disappearance]
E --> G[Return verified or topology-only row]
Technical reviewBest possible solution: Land the focused native omission fix with its deterministic regression test and aligned documentation. Do we have a high-confidence way to reproduce the issue? Main visibly retains topology after a trailing Ok(None); the supplied real-OS test deterministically reaches that window, and the body reports five matching failures across 2,000 main censuses. Is this the best way to solve the issue? This fixes the distinction at the existing census owner without retries or consumer workarounds, preserving PID-reuse comparisons, denied observations, and deadline checks. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 2f01cbdc3bb8. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 18, 2026, 7:55 PM ET / 23:55 UTC. |
… processes exit (#176612) Pin @openclaw/proc-safe 0.4.2. Its Darwin census now reports parentage, process group, owner, and state for processes already exiting (openclaw/proc-safe#23) and omits rows proven gone by its trailing identity read (openclaw/proc-safe#25). OpenClaw's shared Darwin owner no longer fails a full census, and Codex app-server containment no longer reports cleanup "uncertain", whenever an unrelated process on the Mac is mid-exit. Adds an OpenClaw-boundary regression test that holds a child in XNU's exit window and requires readDarwinProcessTree and readDarwinProcessCensus to keep it with its topology. Release-age exclusions move to the 0.4.2 set (remove after 2026-10-26T00:51:31.079Z). Admin-merged over one inherited CI failure: run 39185036717 job 117845878737, an unhandled ERR_STREAM_WRITE_AFTER_END in the extensions/browser/src/browser/chrome-mcp.cleanup.test.ts fixture teardown. That test is unchanged by this PR. The failure reproduces on main with proc-safe 0.4.1 (5/5 runs at e2b6418, 3/6 at 9d013ba), and the job ran on Linux, where proc-safe 0.4.2 is unchanged.
… processes exit (openclaw#176612) Pin @openclaw/proc-safe 0.4.2. Its Darwin census now reports parentage, process group, owner, and state for processes already exiting (openclaw/proc-safe#23) and omits rows proven gone by its trailing identity read (openclaw/proc-safe#25). OpenClaw's shared Darwin owner no longer fails a full census, and Codex app-server containment no longer reports cleanup "uncertain", whenever an unrelated process on the Mac is mid-exit. Adds an OpenClaw-boundary regression test that holds a child in XNU's exit window and requires readDarwinProcessTree and readDarwinProcessCensus to keep it with its topology. Release-age exclusions move to the 0.4.2 set (remove after 2026-10-26T00:51:31.079Z). Admin-merged over one inherited CI failure: run 39185036717 job 117845878737, an unhandled ERR_STREAM_WRITE_AFTER_END in the extensions/browser/src/browser/chrome-mcp.cleanup.test.ts fixture teardown. That test is unchanged by this PR. The failure reproduces on main with proc-safe 0.4.1 (5/5 runs at 85afd2a, 3/6 at 11d9c97), and the job ran on Linux, where proc-safe 0.4.2 is unchanged.
Problem
On a loaded Mac, a Darwin census could return a row for a process that had already been reaped, with
parentPid,processGroupId, andstatebut noidentity. Consumers that require birth identity for every row in a lineage (OpenClaw'sreadDarwinProcessTree) then fail closed with a permission-style error even though the process simply exited.Each Darwin row is bracketed by birth-identity reads:
before, then topology, then optional command, thenafter. The trailing read was collapsed with.ok().flatten(), soOk(None)(proven gone:kern.proc.pidhas no record andkill(pid, 0)returnsESRCH) looked the same as an unreadable identity. The row kept bare topology instead of being omitted, which contradicts the documented contract that proven-gone PIDs are omitted.Fix
When the trailing identity read returns
Ok(None), skip the row, exactly as if topology had observed the disappearance. Everything else is unchanged:Errfrom the trailing read (unreadable/denied) still retains topology without identity; denial never establishes absence.before == Ok(None)is intentionally left alone: if the later topology read succeeds, some process holds that PID (only possible through PID reuse), so omitting it would falsely claim absence. If topology also finds it gone, the row is already omitted.Proof
Deterministic regression (Rust, real OS): a
#[cfg(test)]thread-local hook runs between topology and the trailing identity read. The test spawns/usr/bin/true, holds it as an unreaped zombie withwaitid(WEXITED | WNOWAIT), asserts the selected census returns onezombierow withidentity.exited, then reaps the child from inside the hook and asserts the census returns no row. With the old.ok().flatten()restored, the test fails with "a reaped PID must not keep bare topology". The hook is compiled only undercfg(test)and never reaches the addon. Becausedarwin_censusis macOS-only andcargo testpreviously ran only on Ubuntu, the native workflow now runscargo test --workspace --lockedon the darwin-x64 and darwin-arm64 lanes (Node 24 entry only).There is no external real-OS way to land a reap inside that window deterministically. The census is one synchronous native call, and the window is microseconds, so the existing pty/zombie fixture can hold a process in any state but cannot time a transition between two reads inside the call.
Statistical churn proof (Mac Studio, load average 73–98): 4 tight
/bin/sh -c 'while :; do /usr/bin/true; done'loops in the caller's lineage; each iteration takes a full census and requiresidentityon every descendant of the caller.The instrumented main build logged why each identity-less same-owner row lost identity: all 5 were
after == Ok(None)(3running, 2zombieat topology time), with zerobefore-error,after-error, or changed-identity cases. Every failing PID read back asnullimmediately after. At main's rate, about 15 failures were expected in 6,000 censuses (Poisson P(0) ≈ 3×10⁻⁷). Zombie lineage rows still carry identity after the fix (164–267 per 2,000-census batch).Checks:
PROC_SAFE_NATIVE=require pnpm check,cargo fmt --all --check,cargo clippy --workspace --locked -- -D warnings,cargo test --workspace --locked,actionlint,git diff --check.Related: #23, openclaw/openclaw#176612 (draft bump to 0.4.2). No release is tagged or published here.