fix: bound git worktree helper subprocesses

[SebTardif]

What Problem This Solves

Fixes an issue where ocm dev, setup, and upgrade would hang forever when a git worktree helper (worktree add / remove / status / ls-files / submodule foreach) blocked on a lock or a stuck child. Those helpers called Command::output() with no kill deadline.

On origin/main the hang is here:

ocm/src/openclaw_repo.rs

Lines 123 to 129 in e04c101

	let output = Command::new("git")
	.arg("-C")
	.arg(&repo_root)
	.args(["worktree", "add", "--detach"])
	.arg(&worktree_root)
	.output()
	.map_err(|error| format!("failed to run git worktree add: {error}"))?;

e04c101

Why This Change Was Made

Production worktree helpers now go through a shared timed runner. The child is polled, then terminated (process group TERM, then KILL) when the deadline expires. Fixture git used only to build temp repos stays unbounded.

User Impact

A wedged git during worktree setup or cleanup fails after 15s instead of blocking the CLI. Successful worktree add/remove/status behavior is unchanged.

Evidence

terminal output from a rustc one-off against /bin/sleep 30. Naive Command::output() is still running after 1s. The timed runner returns an error at the 200ms deadline.

$ rustc /tmp/naive_sleep_output.rs -o /tmp/naive_sleep_output && /tmp/naive_sleep_output
naive Command::output() still running after 1002ms: true

$ rustc /tmp/timed_sleep_output.rs -o /tmp/timed_sleep_output && /tmp/timed_sleep_output
sleep timed out after 200ms after 202.924541ms

The same deadline-and-kill path is what git_output now uses for worktree add, remove, status, ls-files, and submodule foreach.

$ /tmp/ocm-F004/target/debug/deps/ocm-c8830440f3406352 git_timeout --nocapture
running 1 test
test openclaw_repo::tests::git_timeout_kills_sleep_after_deadline ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 333 filtered out; finished in 0.26s

Real behavior proof

• Behavior or issue addressed: Git worktree helpers no longer block ocm forever. A child that does not exit is killed when the 15s deadline is reached.

• Real environment tested: macOS (Darwin 25.6.0 arm64), rustc 1.98.0, ocm checkout /tmp/ocm-F004 on fix/git-worktree-timeout above origin/main e04c10166d6b58932213bf8422ccfc201cf6ac37.

• Exact steps or command run after this patch:

  rustc /tmp/naive_sleep_output.rs and /tmp/timed_sleep_output.rs, then run both binaries against /bin/sleep 30. Then run the built lib binary ocm-c8830440f3406352 git_timeout --nocapture.

• Evidence after fix: terminal output above. Naive Command::output() was still running after 1002ms. The timed runner printed sleep timed out after 200ms after 202.924541ms.

• Observed result after fix: The helper returns a timeout error in about 200ms and the sleep child is gone. Worktree add/remove/status on a temp OpenClaw fixture still succeeds through the same git_output path.

• What was not tested: A live index.lock hang inside a full OpenClaw checkout during ocm setup, 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:31 PM ET / 16:31 UTC (Revision 6).

ClawSweeper review

What this changes

Adds a shared subprocess waiter, applies a 15-second deadline to Git worktree helpers, and reuses the waiter for service restart handoffs.

Regression provenance

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

Merge readiness

⛔ Blocked before merge - 11 items remain

The work remains necessary, but both prior blocking findings persist. Current main and v0.2.44 still use unbounded Git output calls; the submitted timeout implementation does not yet provide reliable completion or preserve valid slow operations.

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

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The focused reliability change remains useful, but two established blockers and incomplete production-path evidence prevent readiness.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the macOS one-off sleep transcript and helper test do not demonstrate the production worktree path handling descendant-held pipes, slow success, or interrupted mutations. The fixture-success statement is useful but does not cover those failures. 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	🦪 silver shellfish (2/6)	2 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the macOS one-off sleep transcript and helper test do not demonstrate the production worktree path handling descendant-held pipes, slow success, or interrupted mutations. The fixture-success statement is useful but does not cover those failures. 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	Applicable repository policy: Read the complete root policy and contribution guidance; no nested AGENTS.md files or maintainer-notes directory were found. Applied isolation, actual-proof, and read-only constraints; builds and tests were not run. Unbounded output completion: The new runner joins read-to-end threads after waiting for only the direct child. Termination also returns when that child exits, before necessarily killing surviving descendants. Compatibility-sensitive deadline: All converted Git operations share a fixed 15-second limit, including worktree creation, recursive status inspection, and removal of generated dependencies. There is no progress distinction or timeout override.
Findings	2 actionable findings	[P1] Keep the deadline active while draining inherited output pipes [P1] Preserve valid worktree operations that exceed 15 seconds
Security	None	None.

How this fits together

OCM uses Git helpers to identify, create, validate, and remove OpenClaw source worktrees. Their results feed environment registration and simulation cleanup, so subprocess completion affects whether those workflows finish safely.

flowchart TD
  A[Environment or simulation request] --> B[Worktree checks]
  B --> C[Git subprocess]
  C --> D[Deadline and termination]
  C --> E[Captured output]
  D --> F[Completion result]
  E --> F
  F --> G[Continue or report failure]

Loading

Decision needed

Question	Recommendation
Should worktree mutations preserve existing slow-operation behavior or enforce an unconditional elapsed-time deadline?	Preserve compatibility: Keep legitimate slow operations working by default and make strict cancellation explicit, with bounded cleanup.

Why: The proposed 15-second cutoff changes successful existing workflows, and the discussion contains no approved compatibility policy.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the macOS one-off sleep transcript and helper test do not demonstrate the production worktree path handling descendant-held pipes, slow success, or interrupted mutations. The fixture-success statement is useful but does not cover those failures. 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.
• Keep the deadline active while draining inherited output pipes (P1) - If a Git hook or helper leaves a descendant holding stdout/stderr, the direct child can exit while these joins wait indefinitely for EOF. The timeout path also stops escalation as soon as the leader exits after TERM, leaving a TERM-resistant descendant alive. Keep output completion and descendant cleanup inside the bounded operation; the existing single-sleep test cannot exercise either case.
• Preserve valid worktree operations that exceed 15 seconds (P1) - This limit also applies to worktree checkout, recursive inspection, and deletion of dependency trees. A healthy operation taking more than 15 seconds on a slow filesystem is now killed, failing existing setup or cleanup workflows and potentially interrupting a mutation. Preserve slow-operation compatibility and establish an explicit timeout policy with successful slow-operation and interrupted-retry coverage.
• Resolve merge risk (P1) - Existing slow checkouts and cleanup operations can begin failing after upgrade because the new deadline measures elapsed time, not lack of progress.
• Resolve merge risk (P1) - Interrupted worktree mutations may leave partial directories or Git registration requiring recovery; the submitted evidence does not establish safe retry behavior.
• Resolve merge risk (P2) - A surviving descendant can retain output pipes and keep the caller blocked despite the advertised timeout.
• Complete next step (P2) - Resolve both existing findings, agree on slow-operation compatibility, and add redacted production-worktree evidence covering timeout cleanup and safe retry.
• Improve patch quality - Bound output completion and surviving-descendant cleanup, including an exited leader and a TERM-resistant descendant.
• Improve patch quality - Resolve the timeout policy and demonstrate slow success plus safe interrupted-worktree recovery.
• Improve patch quality - Add production-path terminal output or logs; screenshots or recordings are welcome when useful. Redact private paths, IP addresses, endpoints, and credentials. 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

• [P1] Keep the deadline active while draining inherited output pipes — src/infra/process.rs:64-70
• [P1] Preserve valid worktree operations that exceed 15 seconds — src/openclaw_repo.rs:70-78

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	production +61 net lines; tests +23 lines	Growth is justified as shared timeout handling, but the single added sleep test covers only direct-child termination.

Merge-risk options

Maintainer options:

1. Repair completion and preserve slow operations (recommended)
   Keep the deadline active through output completion and implement the approved compatibility policy before landing.
2. Pause pending timeout policy
   Hold the branch until the acceptable slow-operation and interrupted-cleanup behavior is agreed.

Technical review

Best possible solution:

Bound subprocess and output completion together, preserve slow-operation compatibility by default, and make any stricter deadline an explicit policy backed by safe interruption and retry evidence.

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

Yes, source establishes the failure paths: a descendant retaining stdout blocks the unbounded join, and any valid Git operation exceeding 15 seconds is terminated. No runtime reproduction was executed during this read-only review.

Is this the best way to solve the issue?

No. Sharing the waiter is reasonable, but direct-child exit is insufficient to bound captured output, and one fixed deadline does not preserve slow worktree operations.

Full review comments:

• [P1] Keep the deadline active while draining inherited output pipes — src/infra/process.rs:64-70
  If a Git hook or helper leaves a descendant holding stdout/stderr, the direct child can exit while these joins wait indefinitely for EOF. The timeout path also stops escalation as soon as the leader exits after TERM, leaving a TERM-resistant descendant alive. Keep output completion and descendant cleanup inside the bounded operation; the existing single-sleep test cannot exercise either case.
  Confidence: 0.99
• [P1] Preserve valid worktree operations that exceed 15 seconds — src/openclaw_repo.rs:70-78
  This limit also applies to worktree checkout, recursive inspection, and deletion of dependency trees. A healthy operation taking more than 15 seconds on a slow filesystem is now killed, failing existing setup or cleanup workflows and potentially interrupting a mutation. Preserve slow-operation compatibility and establish an explicit timeout policy with successful slow-operation and interrupted-retry coverage.
  Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P2: This is a bounded reliability improvement for source-worktree workflows without evidence of a widespread urgent outage.
• merge-risk: 🚨 availability: The new timeout path can still wait indefinitely on descendant-held output pipes.
• merge-risk: 🚨 compatibility: A new unconditional 15-second cutoff can terminate previously successful slow Git operations.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Needs real behavior proof before merge: the macOS one-off sleep transcript and helper test do not demonstrate the production worktree path handling descendant-held pipes, slow success, or interrupted mutations. The fixture-success statement is useful but does not cover those failures. 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:

• Applicable repository policy: Read the complete root policy and contribution guidance; no nested AGENTS.md files or maintainer-notes directory were found. Applied isolation, actual-proof, and read-only constraints; builds and tests were not run. (AGENTS.md:1, 6a6dfd11a91e)
• Unbounded output completion: The new runner joins read-to-end threads after waiting for only the direct child. Termination also returns when that child exits, before necessarily killing surviving descendants. (src/infra/process.rs:64, 6a6dfd11a91e)
• Compatibility-sensitive deadline: All converted Git operations share a fixed 15-second limit, including worktree creation, recursive status inspection, and removal of generated dependencies. There is no progress distinction or timeout override. (src/openclaw_repo.rs:70, 6a6dfd11a91e)
• Current-main behavior: Inspected the pinned main implementation: worktree creation still invokes Command::output without a deadline. The PR therefore retains a distinct useful purpose. (src/openclaw_repo.rs:486, c5ac5392e142)
• Latest-release check: GitHub identifies v0.2.44 as the latest release, published September 11. Its supplied release commit also retains the unbounded worktree-add call. (src/openclaw_repo.rs, 9ed6256922b3)
• Prior-review continuity: The previous review retained these same two findings. Reading the earlier process helper through GitHub confirms the unbounded joins and early termination return remain unchanged. Local comparison could not read the earlier object; GitHub supplied the source fallback. (src/infra/process.rs:64, db422a742c94)

Likely related people:

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

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 (5 earlier review cycles)

• reviewed 2026-09-02T20:06:36.861Z sha b080857 :: needs real behavior proof before merge. :: [P1] Keep the deadline active while draining inherited output pipes | [P1] Avoid rejecting valid slow worktrees at a fixed 15 seconds
• reviewed 2026-09-09T20:48:12.274Z sha 0284e88 :: needs real behavior proof before merge. :: [P1] Keep the deadline active while draining inherited output pipes | [P1] Preserve valid worktree operations that exceed 15 seconds
• reviewed 2026-09-10T14:19:46.628Z sha 11e9595 :: needs real behavior proof before merge. :: [P1] Keep the deadline active while draining inherited output pipes | [P1] Preserve valid worktree operations that exceed 15 seconds
• reviewed 2026-09-10T14:32:54.500Z sha 638df70 :: needs real behavior proof before merge. :: [P1] Keep the deadline active while draining inherited output pipes | [P1] Preserve valid worktree operations that exceed 15 seconds
• reviewed 2026-09-10T23:24:35.376Z sha db422a7 :: needs real behavior proof before merge. :: [P1] Keep the deadline active while draining inherited output pipes | [P1] Preserve valid worktree operations that exceed 15 seconds