[BaaRai]

Related: #97616

What Problem This Solves

Fixes part of the zombie-reaping umbrella: Codex app-server close kills observed descendants by exact PID and treats a defunct (Z) process as terminated, but a zombie only clears when its parent consumes the exit status with waitpid. Descendants killed during close (hook relays, shells) can therefore linger as defunct children after the tracked root exits. In deployments where the gateway is PID 1 or the adoption point (fly.toml, Docker entrypoint overrides that drop tini), those zombies are adopted by the gateway and never waited, accumulating over hours into the hundreds and thousands reported in #97616 until the runtime degrades (spawn EAGAIN, uv_thread_create failures, connector silence).

User Impact

User impact: long-running Linux gateways that repeatedly start and stop Codex app-servers now consume the exit statuses of the descendants their close path killed, so those defunct children stop accumulating. Deployments with a reaping PID 1 (tini, docker --init) are unaffected either way, and non-Linux platforms see no change.

Why This Change Was Made

• terminateCodexAppServerDescendants now returns the exact descendant identities it proved and killed.
• Transport close schedules scheduleAdoptedDescendantReapAfterRootExit, an identity-scoped sibling of the group-scoped reaper landed in fix(process): clean up adopted zombies after command timeout #139548. After the tracked root exits it reaps only retained PIDs that are zombies parented to the current process, guarded by their <bootId>:<startTicks> procfs identity against PID reuse. It never waits the tracked root (libuv owns that status), never waitpids unscoped PIDs, and installs no signal handlers, preserving the spawner-owned scoped-reaper design this umbrella established (the gateway-wide SIGCHLD approach from fix(process): add zombie reaper to prevent child process leak #97731 stays rejected).
• Contained descendants may not share the root's process group (hook relays can lead their own groups), which is why the existing PGID-scoped reaper cannot see them.
• When containment cannot prove identities, the force-kill fallback group-kills the root's group, so close also schedules the existing group-scoped reaper for that path, with the hosted-gateway attached topology reported correctly.
• Both reaping helpers are exposed through openclaw/plugin-sdk/process-runtime so the extension reaches them through the supported boundary, with a docs paragraph in the SDK runtime guide. SDK direction: ClawSweeper recommended accepting these focused exports as narrow shared cleanup contracts; final public-API signoff belongs to the area owner.

Evidence

Real behavior proof runs through the changed production boundary (containment to transport close to scheduled reaper) with real adopted descendants across process groups, and demonstrates base failure versus head success with the same test file.

Real-process regression: extensions/codex/src/app-server/transport.close-reaping.process.test.ts. A Linux subreaper harness (mirroring gateway-as-PID-1 adoption) spawns a fixture app-server root whose holders and relays span independent and shared process groups, closes it through the real closeCodexAppServerTransportAndWait boundary, then observes /proc from outside the fixture tree. It asserts the close result is { exited: true, cleanup: "closed" } (containment actually ran), the tracked root completed with exit code 0, every retained descendant fully left /proc, no run-owned zombie stays parented to the harness, and an unrelated process outside the tree keeps running. A red control first proves the observer detects an intentionally leaked adopted zombie that no close owns, then reaps it through the exported scheduler. Unconditional teardown terminates, joins, and reaps every task-owned fixture process on success and failure alike, so a failing run cannot leak processes or adopted zombies into later tests. This replaces the earlier mocked scheduling test per review feedback.

Base failure (merge base bd23988e, same test file overlaid), Debian Linux, real processes:

FAIL ... > transport close consumes adopted descendants across process groups
AssertionError: holder-independent pid 1095 still present: {"state":"Z","ppid":1029,"startTicks":"23486"}: expected { state: 'Z', ppid: 1029, …(1) } to be undefined

The descendant killed by containment survives real transport close as a zombie parented to the harness (pid 1029), which is the reported leak shape. The control also fails on base because the exported scheduler does not exist there.

Head success (8429c35c): the regression passes in GitHub CI Linux (changed-extensions bundle shard, both tests green) and passed 3 consecutive local Debian runs:

Test Files  1 passed (1)
     Tests  2 passed (2)   x3 local runs, plus CI

Both tests pass: the contained close returns closed, the root exits 0, and all four descendants (independent and shared groups) are fully gone from /proc, with the unrelated process alive. The red control confirms the green is not vacuous: the same observer does flag a stuck adopted zombie when one is planted, and forced failing runs on the merge base leave no live fixture processes behind.

Unit coverage for the reaper itself stays in src/process/scoped-child-reaper.test.ts (19 passed: reaps only identity-matching adopted zombies after root exit, skips PID reuse under a new start identity, never waits the tracked root, retains live descendants until they become adopted zombies, stops at its deadline, no work on darwin/win32, single registration per child, no scheduling without usable identities).

Validation: CI fully green on 8429c35c (all 30 checks, including the check-test-types extensions-test lane after satisfying its test typecheck, and the security/boundary scans). Locally: pnpm tsgo:core, pnpm tsgo:extensions, pnpm tsgo:extensions:test, targeted oxlint and oxfmt --check, and the three extension import-boundary checks pass. One earlier CI round hit an unrelated crabbox-managed-binary 120 s acquisition timeout on a loaded 2-vCPU runner; the touched codex and process tests passed in that same round and the retry is green. This PR covers the containment-close owner; the umbrella stays open for other owners.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed October 8, 2026, 8:22 AM ET / 12:22 UTC (Revision 4).

ClawSweeper review

What this changes

The branch schedules scoped Linux zombie reaping after Codex app-server shutdown and exposes the cleanup schedulers through the Plugin SDK.

Example: A Linux Gateway that adopts orphaned processes closes a Codex app-server.

• Before: The recorded base run leaves holder-independent PID 1095 as a zombie parented to harness PID 1029.
• After: The recorded candidate runs remove all four descendants from /proc while the root exits with code 0 and unrelated work survives.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The useful repair has sufficient real-process boundary evidence and no remaining code finding, while harness-level proof, partial umbrella scope, and undecided public API direction limit the rating.
Proof confidence	🦐 gold shrimp (3/6)	Sufficient (terminal): The Debian subreaper harness exercises the changed containment-to-transport-close-to-native-reaper boundary with real processes: base failure leaves an adopted zombie, while candidate recovery removes four descendants, preserves root exit 0, and leaves unrelated work alive. This is sufficient focused production-boundary proof under the review contract. No stored-data contract changes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Needs a maintainer decision · Fix scope: Partial
User problem: Repeated Codex shutdowns can leave adopted zombie descendants consuming Linux process slots until the Gateway degrades.
Reason: The focused cleanup repair has clear value and correctly uses Related for the broader umbrella. Its new supported SDK APIs need area-owner direction.

Merge readiness

⛔ Blocked before merge - 2 items remain

This PR addresses a remaining Linux cleanup defect with sufficient real-process boundary proof and resolves the previous fixture-cleanup finding. No actionable patch defect was established; the public SDK additions still need an area-owner decision.

Priority: P2
Reviewed head: 8429c35c54e74fb8e803574cb618e237f0ad2a27
Owner decision: Required. See Decision needed.

Decision needed

• Question: Should process-runtime publicly support both cleanup schedulers and the adopted-child identity type?
• Recommendation: Accept the focused exports: Support the documented schedulers and identity type as narrow shared process-cleanup contracts.
• Why: The focused repair is justified, but selecting the permanent public plugin contract requires area-owner direction that neither the contributor nor previous automated reviews establish.

Before merge

• Complete next step (P2) - Record the SDK area owner’s choice of supported cleanup contract.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

None.

Agent review details

How this fits together

OpenClaw owns the local Codex app-server transport and terminates its descendant processes when retiring a client. Linux additionally requires the adopting parent to consume exited descendants’ statuses before their process slots disappear.

flowchart TD
 A[Codex client retirement] --> B[Transport shutdown]
 B --> C[Inspect and terminate descendants]
 C --> D[Retain exact process identities]
 D --> E[Tracked root exits]
 E --> F[Check identity and adopted zombie state]
 F --> G[Consume matching exit statuses]

Loading

Technical review

Best possible solution:

Keep cleanup with the existing transport and shared reaper owners through an explicitly accepted narrow SDK contract, while preserving the broader cross-owner umbrella.

Do we have a high-confidence way to reproduce the issue?

Yes: the recorded Linux subreaper regression exposes the failure through real transport close, and fetched-main source retains the same gap. This read-only review did not execute target code.

Is this the best way to solve the issue?

Yes for the repair boundary: retained-identity waiting complements existing containment without introducing a Gateway-wide reaper. The permanent SDK contract remains an owner choice.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b33494d0dfdf.

Provenance checked

• src/process/scoped-child-reaper.ts keeps the original intent (fix(process): clean up adopted zombies after command timeout #139548: Reap only adopted zombies in the terminated process group after Node consumes the tracked root, preserving unrelated children and bounded host shutdown; later budget integration retained that intent.)
• extensions/codex/src/app-server/transport-process-containment.ts keeps the original intent (fix(codex): reap app-server descendant processes #126285: Contain exact descendants across independent process groups before transport retirement so surviving process fleets cannot overlap replacement clients.)
• extensions/codex/src/app-server/transport.ts keeps the original intent (fix(runtime): require cleanup evidence before releasing owned work #139657: Keep physical transport exit separate from descendant cleanup evidence before releasing owned work.)

Testing

Proof path: in-process harness. Added test files: 2.

Security

None.

Evidence

What I checked:

• Pinned introduced changes: Read the introduced production and test changes against the verified merge base, including transport scheduling, retained containment identities, native reaping, SDK exports, and documentation. (8429c35c54e7)
• Current main still needs this repair: The four production paths have no changes between the merge base and fetched main. Main retains group-scoped reaping but does not schedule retained-identity reaping from Codex transport close; the merged command-timeout repair does not cover this distinct path. (extensions/codex/src/app-server/transport.ts:80, b33494d0dfdf)
• Real production-boundary proof: The fully supplied PR body and contributor comments record Debian subreaper runs through real containment, transport close, and native waiting. Base bd23988 leaves an adopted Z-state descendant; candidate runs remove four descendants, preserve root exit 0, and leave unrelated work alive. The deliberate leaked-zombie control validates the observer. This proves the focused process boundary, not a full Gateway soak. (extensions/codex/src/app-server/transport.close-reaping.process.test.ts:327, 8429c35c54e7)
• Previous finding resolved: Both tests now invoke unconditional fixture teardown before deleting their temporary files. Teardown terminates roster processes, joins tracked children, and schedules remaining adopted-zombie reaping. The contributor records successful Debian cleanup and forced base-failure cleanup. The earlier comparison blob could not be fetched because Git returned HTTP 403; current source and the recorded disposition establish resolution. (extensions/codex/src/app-server/transport.close-reaping.process.test.ts:366, 8429c35c54e7)
• Scoped native waiting: The new reaper excludes the tracked root and waits only retained PIDs with matching boot/start identity, zombie state, and current-process parentage. Its timers are paced, bounded, and unreferenced. Existing configuration and stored formats are unchanged. (src/process/scoped-child-reaper.ts:186, 8429c35c54e7)
• SDK direction remains undecided: The branch publishes two cleanup functions and one identity type in the existing process-runtime subpath. The contributor reserves public-API signoff for the area owner. REST discussion inspection found no human acceptance, and the reviews endpoint returned no reviews; a ClawSweeper recommendation does not establish owner approval. (src/plugin-sdk/process-runtime.ts:29, 8429c35c54e7)

Likely related people:

• Peter Steinberger: Raw commit bcef3b3 adds extensions/codex/src/app-server/transport.ts:97 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: bcef3b3526dd; files: extensions/codex/src/app-server/transport.ts)
• srikolagani: Raw commit e53b3de adds src/process/scoped-child-reaper.ts:101 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: e53b3dea864c; files: src/process/scoped-child-reaper.ts)

PR surface

Source +169, Tests +564, Docs +10. Total +743 across 7 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	4	173	4	+169
Tests	2	565	1	+564
Docs	1	10	0	+10
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	7	748	5	+743

Review metrics

Metric	Value	Why it matters
Public SDK additions	2 functions and 1 type	These additions establish a supported native-process cleanup contract requiring owner direction.

Root-cause cluster

Relationship: partial_overlap
Canonical: #97616
Summary: This candidate repairs Codex containment-close adoption within a broader cross-owner zombie report.

Members:

• canonical: [Bug]: OpenClaw leaks unreaped hook/tool child processes, causing zombie accumulation and runtime degradation #97616 - The open umbrella covers multiple process owners, and this PR explicitly addresses only the Codex close path.
• partial_overlap: fix(process): clean up adopted zombies after command timeout #139548 - The merged repair covers adopted zombies after command timeout rather than exact Codex descendants across independent groups.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Labels

Label changes:

No label changes.

Label justifications:

• P2: This is a focused Linux cleanup repair for Gateways that adopt Codex descendants.
• merge-risk: 🚨 compatibility: The branch promotes native cleanup schedulers into a supported public plugin contract whose permanent shape remains undecided.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
• proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/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.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

History

Review history (3 earlier review cycles)

• reviewed 2026-10-08T10:42:48.908Z sha 2c78d0c :: needs real behavior proof before merge. :: none
• reviewed 2026-10-08T11:13:37.918Z sha f8f8533 :: blocked before merge. :: [P2] Clean up spawned fixture processes when the regression fails
• reviewed 2026-10-08T12:01:38.344Z sha 8429c35 :: blocked before merge. :: none

[BaaRai]

Added the requested real behavior proof and pushed it as the regression itself. The new real-process test closes a fixture app-server tree (holders and relays across independent and shared process groups) through the real containment-to-transport boundary under a Linux subreaper harness, and observes /proc from outside: base bd23988e fails with a retained {"state":"Z","ppid":<harness>} descendant after close, head f8f85338 passes 3/3 runs with cleanup: "closed", root exit 0, all descendants gone, and an unrelated process untouched. A red control plants an adopted zombie no close owns to prove the observer detects it, then reaps it via the exported scheduler. The earlier mocked scheduling test was removed per review. PR body Evidence section updated with both outputs. On the SDK direction question: keeping the two focused exports as documented, per the review recommendation; final signoff with the area owner.

[BaaRai]

Addressed the fixture-cleanup finding: teardown now runs unconditionally in both tests. It terminates every roster PID (the fixture log plus tracked handles), joins the tracked handles so libuv consumes their exits, then reaps any remaining harness-parented zombie statuses through the exported scheduler before the fixture files are removed. Verified on Debian: head run passes with zero leftover processes or zombies, and a forced failing run on the merge base kills every live fixture process (the base has no scheduler to consume the residual statuses, which die with the worker). Pushed as 1b538e7. The remaining items are the SDK-contract decisions for the area owner; per the review recommendation the two focused exports stay as documented.