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 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 (process group TERM, then KILL) when SERVICE_ONCE_CHILD_TIMEOUT_MS (15s) expires, matching the restart-handoff wait helper. On timeout the supervisor returns an error instead of blocking. Long-running run_until_stopped is unchanged.

User Impact

A stuck env child in once-mode fails after 15s instead of hanging ocm service --once. Children that exit on their own still report their exit status.

Evidence

terminal output from a rustc one-off against /bin/sleep 30. Naive child.wait() is still running after 1s. The timed waiter returns an error at the 200ms deadline and the sleep process is gone.

$ rustc /tmp/naive_sleep_wait.rs -o /tmp/naive_sleep_wait && /tmp/naive_sleep_wait
naive child.wait() still running after 1006ms: true
sleep pid 47907 still alive: true

$ rustc /tmp/timed_sleep_wait.rs -o /tmp/timed_sleep_wait && /tmp/timed_sleep_wait
sleep timed out after 200ms after 223.674792ms
sleep pid 47991 still alive: false

The same deadline-and-kill path is what run_once now uses after spawn_supervisor_child.

$ /tmp/ocm-F005/target/debug/deps/ocm-c8830440f3406352 supervisor::tests::wait_child_with_timeout_kills_sleep_after_deadline --exact --nocapture
running 1 test
test supervisor::tests::wait_child_with_timeout_kills_sleep_after_deadline ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 334 filtered out; finished in 0.78s

Real behavior proof

• Behavior or issue addressed: ocm service --once no longer blocks forever on child.wait(). A child that does not exit is killed when the 15s deadline is reached, and the supervisor returns an error.

• Real environment tested: macOS (Darwin 25.6.0 arm64), rustc 1.98.0, ocm checkout /tmp/ocm-F005 on fix/supervisor-run-once-timeout above origin/main e04c10166d6b58932213bf8422ccfc201cf6ac37.

• Exact steps or command run after this patch:

  rustc /tmp/naive_sleep_wait.rs and /tmp/timed_sleep_wait.rs, then run both binaries against /bin/sleep 30. Then run the built lib binary ocm-c8830440f3406352 supervisor::tests::wait_child_with_timeout_kills_sleep_after_deadline --exact --nocapture.

• Evidence after fix: terminal output above. Naive child.wait() was still running after 1006ms with sleep still alive. The timed waiter printed sleep timed out after 200ms after 223.674792ms and sleep pid 47991 still alive: false.

• Observed result after fix: The helper returns a timeout error in about 200ms and the sleep child is gone. run_once uses that same waiter with a 15s named deadline.

• What was not tested: A live launchd/systemd ocm service --once with a real OpenClaw gateway that refuses to exit, and the Windows job-object kill path.

[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 11, 2026, 12:46 PM ET / 16:46 UTC (Revision 4).

ClawSweeper review

What this changes

Adds a 15-second limit to each child launched by OCM’s once-only supervisor command, timeout termination, and two Unix helper tests.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 7 items remain

The proposed deadline remains absent from main and v0.2.44, so this PR is not obsolete. The previously reported descendant-cleanup defect remains unresolved, and the supplied demonstrations do not establish production-path behavior.

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

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is focused, but the repeated cleanup defect and helper-only demonstration leave it short of merge readiness.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: The captured macOS terminal traces prove standalone timeout examples and a helper test using sleep, but do not exercise saved-plan loading, admission, and production process-group spawning through ocm __daemon run --once. After-fix timeout cleanup and normal completion through that entrypoint remain unproven; no full launchd/systemd installation is necessary. 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 terminal traces prove standalone timeout examples and a helper test using sleep, but do not exercise saved-plan loading, admission, and production process-group spawning through ocm __daemon run --once. After-fix timeout cleanup and normal completion through that entrypoint remain unproven; no full launchd/systemd installation is necessary. 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 full local diff matches the host-pinned introduction evidence: one supervisor file changes, adding the deadline, waiter, termination helper, and two tests. No dependency, workflow, or persisted-format changes are introduced. Current-main necessity: Current main still calls child.wait() without a deadline in run_once; its admission lock remains held during that wait. Latest-release check: The same unbounded wait is present in v0.2.44. The tag was separately resolved to the recorded release commit.
Findings	1 actionable finding	[P2] Wait for process-group cleanup before returning
Security	None	None.

How this fits together

OCM’s supervisor reads saved environment service plans and launches their gateway processes. Its internal once-only command waits for each planned child and returns execution results instead of continuously supervising restarts.

flowchart TD
  A[Once-only command] --> B[Saved service plans]
  B --> C[Environment admission check]
  C --> D[Launch child process group]
  D --> E[Wait for exit or deadline]
  E --> F[Return child result]
  E --> G[Terminate group and report timeout]

Loading

Decision needed

Question	Recommendation
Should the internal once-only command always kill children after 15 seconds, or should a deadline require explicit selection?	Make the deadline explicit: Preserve existing invocation behavior and add an explicitly selected diagnostic timeout.

Why: The existing command waits for child completion, and elapsed time alone does not distinguish a stuck gateway from a healthy long-running one.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The captured macOS terminal traces prove standalone timeout examples and a helper test using sleep, but do not exercise saved-plan loading, admission, and production process-group spawning through ocm __daemon run --once. After-fix timeout cleanup and normal completion through that entrypoint remain unproven; no full launchd/systemd installation is necessary. 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.
• Wait for process-group cleanup before returning (P2) - When the timeout sends TERM to a launcher group, its leader can exit while a descendant ignores TERM. Returning on the leader’s status skips group KILL, leaving that descendant alive after run_once returns and releases admission. This previously reported blocker remains present. Reuse or extract the existing group-aware supervisor shutdown logic, retain leader reaping, and cover a TERM-exiting leader with a TERM-resistant descendant.
• Resolve merge risk (P1) - The unconditional deadline kills healthy once-mode children that legitimately run longer than 15 seconds; existing invocations may begin failing after upgrade.
• Resolve merge risk (P2) - The timeout path can leave a TERM-resistant descendant running after returning an error and releasing environment admission.
• Resolve merge risk (P1) - Fresh and existing saved-plan behavior through the actual once-only command remains unproven.
• Complete next step (P2) - Fix descendant cleanup, obtain a decision on deadline semantics, and add after-fix evidence from the actual once-only command using isolated state. Terminal output or logs are acceptable; screenshots or recordings help when they show the result. Redact private paths, endpoints, IP addresses, keys, and other personal details. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [P2] Wait for process-group cleanup before returning — src/supervisor/mod.rs:1398-1401

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Net code growth	production +53 lines; tests +40 lines	The deadline is the stated justification, but production growth includes a second shutdown implementation despite an existing group-aware path.

Merge-risk options

Maintainer options:

1. Repair cleanup and settle deadline semantics (recommended)
   Use group-aware termination, resolve the default lifetime policy, and demonstrate the production command before landing.
2. Pause the deadline change
   Keep the PR open until its diagnostic lifetime contract is agreed if preserving existing behavior is outside the intended scope.

Technical review

Best possible solution:

Reuse group-aware shutdown and preserve existing once-mode lifetime by default, with an explicit diagnostic deadline unless maintainers approve a documented compatibility change.

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

Yes, from source: a non-exiting child keeps main’s wait blocked, and a leader that exits on TERM with a TERM-resistant descendant exposes the proposed cleanup defect. No runtime reproduction or artifact-producing tests were executed in this read-only review.

Is this the best way to solve the issue?

No, not as submitted: the deadline introduces a new lifetime policy, and its separate termination helper omits the group-aware cleanup already used by the supervisor.

Full review comments:

• [P2] Wait for process-group cleanup before returning — src/supervisor/mod.rs:1398-1401
  When the timeout sends TERM to a launcher group, its leader can exit while a descendant ignores TERM. Returning on the leader’s status skips group KILL, leaving that descendant alive after run_once returns and releases admission. This previously reported blocker remains present. Reuse or extract the existing group-aware supervisor shutdown logic, retain leader reaping, and cover a TERM-exiting leader with a TERM-resistant descendant.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P2: This is bounded supervisor diagnostic work, with no evidence of an urgent regression in the normal continuous daemon.
• merge-risk: 🚨 availability: Timeout cleanup can leave descendants running and holding gateway resources.
• merge-risk: 🚨 compatibility: Existing once-mode invocations exceeding 15 seconds would now be terminated and fail.
• 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 terminal traces prove standalone timeout examples and a helper test using sleep, but do not exercise saved-plan loading, admission, and production process-group spawning through ocm __daemon run --once. After-fix timeout cleanup and normal completion through that entrypoint remain unproven; no full launchd/systemd installation is necessary. 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 full local diff matches the host-pinned introduction evidence: one supervisor file changes, adding the deadline, waiter, termination helper, and two tests. No dependency, workflow, or persisted-format changes are introduced. (src/supervisor/mod.rs:1071, 07333f28756d)
• Current-main necessity: Current main still calls child.wait() without a deadline in run_once; its admission lock remains held during that wait. (src/supervisor/mod.rs:1070, c5ac5392e142)
• Latest-release check: The same unbounded wait is present in v0.2.44. The tag was separately resolved to the recorded release commit. (src/supervisor/mod.rs:1070, 9ed6256922b3)
• Unresolved cleanup finding: After sending group TERM, terminate_child returns immediately when the leader exits. A descendant ignoring TERM therefore bypasses group KILL. Existing stop_supervisor_child instead checks group existence before escalation, and existing tests cover descendants surviving their leader. (src/supervisor/mod.rs:1400, 07333f28756d)
• Production entrypoint and existing coverage: src/cli/internal.rs accepts __daemon run --once and delegates to SupervisorService::run. The existing daemon_run_once_executes_planned_children integration test exercises saved plans and checks normal completion; the added tests exercise only standalone helper children without production process-group setup. (tests/daemon_runtime_tests.rs:2000, 07333f28756d)
• Captured proof and review continuity: The complete supplied body, captured under sourceRevision 41cf04a37a8858bf9f89b7992639e1bc8e716afaf0e75326ffb4e6d5b1731caf, contains macOS standalone Rust traces and a helper-test result, but explicitly excludes the actual once-only gateway run. The previous review already requested group cleanup, production-command evidence, and resolution of the deadline policy. The current finding repeats that unresolved concern; it is not a newly discovered late finding.

Likely related people:

• hannesrudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• shakkernerd: 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.

• Repair group cleanup and cover a leader exiting on TERM while a descendant ignores it.
• Add redacted after-fix terminal evidence from the actual once-only command showing timeout cleanup and normal completion with isolated fresh and existing saved plans.
• Resolve whether the deadline is explicit or an intentional new default.

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 (3 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