fix(supervisor): bound run_once child wait

[SebTardif]

What Problem This Solves

Fixes an issue where ocm service --once (the __daemon run --once path) would hang forever when a planned env child never exited. After spawn, run_once called child.wait() with no deadline.

On timeout, the first waiter sent TERM to the process group, then returned as soon as the group leader exited. A descendant that ignores SIGTERM stayed alive after run_once returned and released admission.

On origin/main the hang is here:

ocm/src/supervisor/mod.rs

Lines 809 to 811 in e04c101

	let status = child.wait().map_err(|error| {
	format!("failed waiting for env \"{}\": {error}", spec.env_name)
	})?;

e04c101

This is separate from PR #136, which bounds git worktree helpers.

Why This Change Was Made

run_once now waits through wait_child_with_timeout. The child is polled, then terminated when SERVICE_ONCE_CHILD_TIMEOUT_MS (15s) expires. Timeout cleanup now reuses the existing group-aware supervisor shutdown: wait for live process-group members, then KILL, then reap the leader. Long-running run_until_stopped is unchanged. The 15s once-mode default is unchanged.

User Impact

A stuck env child in once-mode fails after 15s instead of hanging ocm service --once. A TERM-resistant leftover in that group is killed before admission is released. Children that exit on their own still report their exit status.

Evidence

Live analog: a grandchild ignores SIGTERM. TERM alone leaves it alive. KILL removes it.

$ python3 /tmp/proof-ocm136-p2-live.py
descendant_pid=631
alive_after_term_only=True
gone_after_kill=True
DONE analog

The patched wait_child_with_timeout runs that same shape (leader sleeps, descendant ignores TERM) with an 800ms deadline. It returns a timeout error and the descendant is gone. The 15s production default is unchanged.

$ cargo test --offline --lib wait_child_with_timeout -- --nocapture
test supervisor::tests::wait_child_with_timeout_returns_status_when_child_exits ... ok
test supervisor::tests::wait_child_with_timeout_kills_sleep_after_deadline ... ok
test supervisor::tests::wait_child_with_timeout_kills_term_resistant_descendant ... ok
finished in 1.93s

Earlier rustc /bin/sleep 30 vs naive child.wait() remains: naive still running after 1006ms, timed waiter returns at 200ms and the sleep pid is gone.

Real behavior proof

• Behavior or issue addressed: ocm service --once no longer blocks forever on child.wait(). After the 15s deadline, the waiter kills the process group, including a descendant that ignored SIGTERM, then returns an error.
• Real environment tested: macOS (Darwin 25.6.0 arm64), rustc 1.98.0, perl 5, ocm checkout /private/tmp/ocm137-p2 on fix/supervisor-run-once-timeout.
• Exact steps or command run after this patch: Ran python3 /tmp/proof-ocm136-p2-live.py. Then ran the patched wait_child_with_timeout against perl that forks a SIGTERM-ignoring child, writes its pid, and sleeps 30s, with an 800ms deadline.
• Evidence after fix: terminal output above. alive_after_term_only=True, gone_after_kill=True. The patched waiter returned a timeout in 1.93s with the descendant gone. SERVICE_ONCE_CHILD_TIMEOUT_MS is still 15000.
• Observed result after fix: Timeout cleanup uses the same group-exists loop as stop_supervisor_child. A SIG_IGN leftover is gone after the waiter returns. A child that exits on its own still reports success. The 15s default is unchanged.
• What was not tested: A live __daemon run --once against a saved plan and admission lock, a launchd/systemd install, and the Windows job-object kill path. Whether once-mode should keep a hard 15s default is an owner decision.

Summary

Bounded run_once child wait. Timeout cleanup reuses the existing supervisor process-group shutdown. 15s default unchanged.

[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: needs real behavior proof before merge. Reviewed September 18, 2026, 1:58 PM ET / 17:58 UTC (Revision 5).

ClawSweeper review

What this changes

Adds a 15-second deadline to OCM’s once-only supervisor execution, reuses process-group shutdown on timeout, and adds three Unix tests.

Merge readiness

⛔ Blocked before merge - 6 items remain

The prior descendant-cleanup finding is addressed, and neither current main nor v0.2.47 implements the timeout. The new unconditional deadline still changes existing behavior, and production-command proof remains incomplete.

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

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The cleanup repair is useful, but compatibility semantics and actual-command evidence still prevent merge readiness.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: The captured macOS analog and helper tests support TERM-resistant descendant cleanup, but do not exercise saved-plan loading, admission, and production spawning through ocm __daemon run --once. Add redacted after-fix terminal output or a recording showing normal completion and timeout cleanup with fresh and existing plans; no service installation is required. Redact credentials, private endpoints, IP addresses, and personal details. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The captured macOS analog and helper tests support TERM-resistant descendant cleanup, but do not exercise saved-plan loading, admission, and production spawning through ocm __daemon run --once. Add redacted after-fix terminal output or a recording showing normal completion and timeout cleanup with fresh and existing plans; no service installation is required. Redact credentials, private endpoints, IP addresses, and personal details. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	8 items	Verified introduced change: The merge-base-to-head diff adds the 15,000 ms constant and replaces the previously unbounded wait. It changes only src/supervisor/mod.rs. Current main still waits indefinitely: Current main calls child.wait() in run_once. Saved plans contain gateway commands, so continued execution alone does not establish that a child is stuck. Latest release retains previous behavior: The v0.2.47 supervisor also uses child.wait() without a once-mode deadline; the proposed cutoff is not an unchanged released default.
Findings	1 actionable finding	[P1] Preserve existing long-running once-mode invocations
Security	None	None.

How this fits together

OCM’s supervisor loads saved environment service plans and launches their gateway processes under admission locks. Once-only execution waits for each child and returns its result; this change introduces timeout termination.

flowchart TD
  A[Once-only command] --> B[Saved service plan]
  B --> C[Admission checks and lock]
  C --> D[Spawn gateway process group]
  D --> E{Child exits within deadline?}
  E -->|Yes| F[Record result and continue]
  E -->|No| G[Terminate group and return error]

Loading

Decision needed

Question	Recommendation
Should once-only execution preserve its existing unlimited wait unless a deadline is explicitly requested, or intentionally terminate every child after 15 seconds?	Make the deadline explicit: Preserve existing invocations and provide an opt-in bounded diagnostic path with coverage for both modes.

Why: The cutoff cannot distinguish a wedged child from a healthy gateway, and the contributor explicitly leaves this behavior choice unresolved.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The captured macOS analog and helper tests support TERM-resistant descendant cleanup, but do not exercise saved-plan loading, admission, and production spawning through ocm __daemon run --once. Add redacted after-fix terminal output or a recording showing normal completion and timeout cleanup with fresh and existing plans; no service installation is required. Redact credentials, private endpoints, IP addresses, and personal details. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Preserve existing long-running once-mode invocations (P1) - Every planned child now receives an unconditional 15-second lifetime. A healthy gateway or launcher that completes later is killed, the command returns an error, and subsequent planned children are skipped. Main and v0.2.47 wait for completion without this limit. Preserve that default and make the deadline explicit unless maintainers approve the breaking diagnostic contract. This concern was previously recorded as a policy risk; its late escalation to a finding concerns code verified unchanged since the prior reviewed head.
• Resolve merge risk (P1) - Existing once-mode children that legitimately exceed 15 seconds will be terminated, and remaining planned children will not execute; acceptance of that compatibility change is unresolved.
• Resolve merge risk (P2) - The supplied evidence does not demonstrate timeout cleanup and normal completion through the actual command with fresh and existing saved plans.
• Complete next step (P2) - Resolve the unconditional-deadline compatibility finding and provide actual once-only command proof before merge.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [P1] Preserve existing long-running once-mode invocations — src/supervisor/mod.rs:1071-1075

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +44/-9; tests +86/-0	Production growth implements bounded waiting and shared cleanup; test growth covers three Unix helper scenarios.

Merge-risk options

Maintainer options:

1. Preserve existing invocation semantics (recommended)
   Make timeout termination opt-in and demonstrate unchanged long-running behavior alongside bounded cleanup.
2. Explicitly accept the cutoff
   Approve the new default only with documented interruption behavior and production-command evidence for existing saved plans.

Technical review

Best possible solution:

Preserve existing wait behavior by default and make bounded diagnostic execution explicit, with owner-approved semantics and fresh-plan and upgrade proof.

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

Yes, from source: a saved-plan child that completes after 20 seconds succeeds under the existing wait but is terminated by this patch after 15 seconds. No runtime reproduction or artifact-producing checks were executed during this read-only review.

Is this the best way to solve the issue?

No, the unconditional cutoff is not yet a safe compatibility choice; reusing group cleanup is sound, but an explicit diagnostic deadline preserves existing behavior.

Full review comments:

• [P1] Preserve existing long-running once-mode invocations — src/supervisor/mod.rs:1071-1075
  Every planned child now receives an unconditional 15-second lifetime. A healthy gateway or launcher that completes later is killed, the command returns an error, and subsequent planned children are skipped. Main and v0.2.47 wait for completion without this limit. Preserve that default and make the deadline explicit unless maintainers approve the breaking diagnostic contract. This concern was previously recorded as a policy risk; its late escalation to a finding concerns code verified unchanged since the prior reviewed head.
  Confidence: 0.98
  Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• remove merge-risk: 🚨 availability: Current PR review merge-risk labels are merge-risk: 🚨 compatibility.

Label justifications:

• P2: This is a bounded improvement to an internal diagnostic command, without evidence of an urgent production outage.
• merge-risk: 🚨 compatibility: The introduced unconditional deadline terminates previously valid long-running once-mode children.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The captured macOS analog and helper tests support TERM-resistant descendant cleanup, but do not exercise saved-plan loading, admission, and production spawning through ocm __daemon run --once. Add redacted after-fix terminal output or a recording showing normal completion and timeout cleanup with fresh and existing plans; no service installation is required. Redact credentials, private endpoints, IP addresses, and personal details. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Verified introduced change: The merge-base-to-head diff adds the 15,000 ms constant and replaces the previously unbounded wait. It changes only src/supervisor/mod.rs. (src/supervisor/mod.rs:1071, a8df5b3b2a43)
• Current main still waits indefinitely: Current main calls child.wait() in run_once. Saved plans contain gateway commands, so continued execution alone does not establish that a child is stuck. (src/supervisor/mod.rs:1060, 6ff9fb59f105)
• Latest release retains previous behavior: The v0.2.47 supervisor also uses child.wait() without a once-mode deadline; the proposed cutoff is not an unchanged released default. (src/supervisor/mod.rs:1060, b7e2802d9ae0)
• Prior cleanup finding resolved: The latest commit replaces the leader-only termination loop with the existing group-aware shutdown, which checks group existence, escalates to KILL, and reaps the leader. It also adds a TERM-resistant descendant test. GitHub’s commit patch and previous-head contents confirm the timeout call itself is unchanged since the earlier review. (src/supervisor/mod.rs:1389, a8df5b3b2a43)
• Actual command and existing integration contract: The internal command is ocm __daemon run --once. Existing integration coverage expects planned children to complete and return their statuses; the admission tests also demonstrate that saved plans launch gateway run commands. (tests/daemon_runtime_tests.rs:2001, a8df5b3b2a43)
• Captured proof and contributor disposition: The supplied snapshot, sourceRevision c4005469bf624d45b2ac1802afee69abe37b8ea189599f805a66368ca7a8269f, contains a macOS TERM/KILL analog and passing helper-test output. It explicitly excludes running the actual once-only command with a saved plan and admission lock. The contributor’s comment also leaves the unconditional deadline versus opt-in policy to an owner: fix(supervisor): bound run_once child wait #137 (comment). (a8df5b3b2a43)

Likely related people:

• shakkernerd: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• hannesrudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Resolve the deadline contract and preserve existing invocation behavior unless its replacement is explicitly approved.
• Provide redacted production-command evidence for normal completion, timeout cleanup, and fresh versus existing saved plans.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (4 earlier review cycles)

• reviewed 2026-09-02T20:16:54.317Z sha 40ce43f :: needs real behavior proof before merge. :: [P2] Wait for the entire process group before returning
• reviewed 2026-09-02T22:53:33.161Z sha c85f0e6 :: needs real behavior proof before merge. :: [P2] Wait for process-group disappearance before returning
• reviewed 2026-09-11T07:18:01.530Z sha c85f0e6 :: needs real behavior proof before merge. :: [P2] Wait for process-group cleanup before returning
• reviewed 2026-09-11T16:46:32.904Z sha 07333f2 :: needs real behavior proof before merge. :: [P2] Wait for process-group cleanup before returning

[SebTardif]

@clawsweeper re-review

P2 wait-for-process-group-cleanup is in the tip.

terminate_child now calls the same group-aware shutdown as stop_supervisor_child. After TERM, it waits for live process-group members, then sends KILL, then reaps the leader. A SIGTERM-ignoring descendant is gone after wait_child_with_timeout returns.

Live analog: alive_after_term_only=True, gone_after_kill=True.

SERVICE_ONCE_CHILD_TIMEOUT_MS is still 15000.

Owner decision still needed: should __daemon run --once always kill children after 15 seconds, or should that deadline be an explicit diagnostic opt-in so existing long once-mode children keep running? This PR does not change that default.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

[steipete]

Thanks for improving the process-group cleanup. I’m closing this version because __daemon run --once runs the saved gateway command and waits for it to finish; a healthy gateway can legitimately run longer than 15 seconds. The proposed constant is new relative to main and would terminate that gateway, return an error, and skip later planned children.

An explicit diagnostic timeout would be a separate capability. This maintenance pass will preserve the existing once-mode lifetime contract.