fix(supervisor): reap only leased process descendants

[skevetter]

Linux runtime shutdown can hang after a backend starts a detached VM. The supervisor becomes the VM's subreaper, then waits for every adopted child even though that VM has its own session and resource lifetime. In Devsy's real-VM baseline, built-in MicroSandbox passes while the external runtime stalls after RunImage returns.

Reap only descendants in the leased plugin process group. Keep group termination, reserved leader identity, and cleanup of command children unchanged. Separately owned backend sessions survive the operation and transfer to the OS when the supervisor exits.

The Linux regression starts a real detached backend with separate stdio, verifies that supervisor shutdown completes, and checks that the backend still responds over a Unix socket. Readiness and shutdown share a deadline. It fails with the previous implementation and passes with this fix.

Validation:

• Regression reproduced on the original implementation: supervisor wait exceeded 15 seconds.
• Full Linux and macOS SDK race suites and macOS go vet ./... passed, including existing child/grandchild ownership probes.
• Native Linux and macOS strict lint, formatting, and all prek hooks passed. The Linux regression uses a separate response assertion helper to satisfy the repository complexity limit.
• Complete fresh committed local CodeRabbit review covered all five changed files with zero findings.
• All ten final-head GitHub Actions validation jobs passed on Linux, macOS and Windows.
• Greptile reviewed the final head at 5/5; its PID-read timeout finding is fixed and resolved. Complete remote CodeRabbit review covered all five files with no actionable findings.
• All three commits have valid GitHub signatures.

CodeRabbit's docstring-coverage warning concerns internal helpers and test fixtures. Their names and the existing ownership comments express the contract; adding comments that repeat those functions would not improve clarity. Its proposed production backend retirement/recovery validation belongs to the Devsy lifecycle baseline, whose stop/start/recreate/delete checks exercise the separate VM owner; this SDK change preserves that ownership boundary.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: ba1a5b82-a78d-4ada-914f-12f87c40f836

📥 Commits

Reviewing files that changed from the base of the PR and between d9e8aba and ad28ff0.

📒 Files selected for processing (5)

• README.md
• supervisor/detached_linux_test.go
• supervisor/exit_darwin.go
• supervisor/exit_linux.go
• supervisor/tree_unix.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Linux descendant reaping now targets the leased process group. New Linux tests verify that a backend in a separate session remains reachable after the supervisor exits. The README describes both process ownership behaviors.

Changes

Process Reaping

Layer / File(s)	Summary
Group-scoped descendant reaping supervisor/exit_linux.go, supervisor/tree_unix.go, supervisor/exit_darwin.go, README.md	Linux reaping now targets the process group identified by the process leader PID. The README describes reaping orphaned descendants in that group. The Darwin no-op declarations remain behaviorally unchanged.
Detached backend verification supervisor/detached_linux_test.go, README.md	Linux tests start a backend in a separate session, check that the supervisor returns without waiting for it, and verify that it still responds with "alive". The README states that the OS adopts separately owned backend sessions when the supervisor exits.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk

Merge Risk: ⚪ Minimal · up to ad28f

The supervisor’s reaping scope matches the leased process group, and no issue remains that needs resolution before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ad28f

The change aligns shutdown with the existing process-group ownership boundary without expanding termination privileges. No introduced security weakness was established. Detached backends still require their own lifecycle owner, whose production cleanup behavior is not demonstrated here.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed cleanup selector is bounded to one leased process group and adopted children of its supervisor. It does not broaden signal targets or grant backend privileges. Tenant and environment exposure of independently owned production backends is not established by this scope.

Security Findings and Attack Paths

• inferred — No introduced sandbox-escape path is established by narrowing reaping: a runtime-created detached session was already outside group termination at the merge base. Waiting for that process did not terminate it or constrain its authority.

Trust Boundaries and Controls

• observed — The runtime executable, arguments, and environment continue to arrive through host-provided configuration over an inherited pipe. The runtime starts in a dedicated process group; the PR changes subsequent wait selection rather than the configuration channel or launch authority.

Resilience and Maintainability Implications

• observed — Cleanup still retries interrupted Linux waits, treats no matching children as completion, and joins cleanup errors. Internal repeated-Wait handling and the reaped flag ordering predate this PR; production uses one internal Wait, while external runner Wait reads its completion channel.

Hardening Proposals

• proposed — For production external backends, validate that the explicit backend owner handles expiry, cancellation, and host restart independently of command cleanup. The added liveness regression does not demonstrate those backend retirement and recovery states.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: limiting supervisor reaping to leased process descendants.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[skevetter]

@greptileai review

[greptile-apps]

Confidence Score: 5/5

[High impact] The PR appears safe to merge, with no outstanding findings.

Summary

The Linux supervisor now waits only for descendants in the leased process group. Separately owned backend sessions can survive supervisor shutdown.

• Adds a Linux test that checks shutdown completes and the detached backend still responds.
• Updates the macOS helper signature and ownership documentation.
• Fixes the previous PID-read timeout finding by closing the stdout reader when the test deadline expires.
• No new actionable issues were found in the changes since the previous review.

_{Reviews (2) · Last reviewed commit: "test(supervisor): bound detached backend..." · Reviewed by Greptile}

[skevetter]

@greptileai review