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]

Codex review: needs real behavior proof before merge. Reviewed September 2, 2026, 4:06 PM ET / 20:06 UTC.

ClawSweeper review

What this changes

The PR routes OCM’s Git worktree commands through a shared timed subprocess runner and reuses its wait logic for restart-handoff commands.

Merge readiness

⛔ Blocked until stronger real behavior proof is added - 7 items remain

Keep open: the shared timeout approach addresses a real worktree-flow hang, but the new runner can still block forever while joining inherited output pipes, and its universal 15-second limit can reject valid slow worktrees without an approved compatibility policy.

Priority: P1
Reviewed head: b080857e1a4aa76b20424b165859fc5322039bd0
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The patch is focused and has partial real terminal evidence, but two P1 workflow and compatibility blockers remain.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: The supplied macOS terminal trace proves the changed shared runner times out a foreground /bin/sleep child, but it neither runs a git_output worktree caller nor exercises the leader-exit/inherited-pipe case that still defeats the deadline. Add a redacted terminal trace of the repaired production path; updating the PR body should trigger a fresh review, or a maintainer can request one with @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)	2 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The supplied macOS terminal trace proves the changed shared runner times out a foreground /bin/sleep child, but it neither runs a git_output worktree caller nor exercises the leader-exit/inherited-pipe case that still defeats the deadline. Add a redacted terminal trace of the repaired production path; updating the PR body should trigger a fresh review, or a maintainer can request one with @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	5 items	Current main has unbounded Git helpers: The fetched default branch invokes Command::output directly for worktree add, clean, remove, status, ignored-file inspection, worktree listing, and rev-parse; a blocked Git process therefore blocks the caller. Introduced fixed deadline: All routed Git commands now use one fixed 15-second timeout through git_output. Output-drain gap: The timer stops once the direct child exits, then the runner unconditionally joins both pipe readers. A descendant retaining inherited stdout or stderr can keep either reader blocked indefinitely after the leader has exited.
Findings	2 actionable findings	[P1] Keep the deadline active while draining inherited output pipes [P1] Avoid rejecting valid slow worktrees at a fixed 15 seconds
Security	None	None.

How this fits together

OCM creates, inspects, and removes disposable OpenClaw Git worktrees for development and upgrade simulations. These CLI flows invoke local Git commands and return either a usable worktree or an actionable setup or cleanup error.

flowchart LR
  A[OCM dev or upgrade] --> B[Worktree manager]
  B --> C[Git helper command]
  C --> D{Completes before deadline?}
  D -->|Yes| E[Worktree result]
  D -->|No| F[Terminate process group]
  F --> G[Timeout error to CLI]

Loading

Decision needed

Question	Recommendation
Should OCM preserve completion for valid Git worktree operations that exceed 15 seconds, or intentionally adopt a universal 15-second fail-fast policy?	Preserve slow-worktree compatibility: Use an approved operation-specific allowance, retry, or override while retaining bounded cleanup for genuinely stuck subprocesses.

Why: The branch changes existing successful-workflow behavior as well as fixing hangs; the appropriate tradeoff between responsiveness and slow-repository compatibility requires maintainer intent.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The supplied macOS terminal trace proves the changed shared runner times out a foreground /bin/sleep child, but it neither runs a git_output worktree caller nor exercises the leader-exit/inherited-pipe case that still defeats the deadline. Add a redacted terminal trace of the repaired production path; updating the PR body should trigger a fresh review, or a maintainer can request one with @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.
• Keep the deadline active while draining inherited output pipes (P1) - After wait_for_child observes the leader exit, these joins have no deadline. A child such as sh -c 'sleep 30 &' can exit successfully while its descendant retains stdout or stderr; the reader then blocks indefinitely, so the worktree helper remains unbounded. Keep the process-group deadline active until both pipes close and terminate the group when they do not.
• Avoid rejecting valid slow worktrees at a fixed 15 seconds (P1) - Every worktree Git call now receives this fixed limit, including recursive submodule inspection and cleanup. On an existing large or slow worktree those commands can legitimately exceed 15 seconds, turning a previously successful dev or upgrade operation into an error. Preserve a bounded stuck-process policy without imposing an unapproved universal failure limit.
• Resolve merge risk (P2) - A child that exits while a descendant retains inherited stdout or stderr bypasses the timeout and can leave OCM blocked in the pipe-reader joins.
• Resolve merge risk (P1) - A fixed 15-second limit now makes valid but slow Git clean, recursive submodule, status, or worktree operations fail where they previously completed.
• Complete next step (P2) - Repair the output-drain timeout gap, add production-path proof, and obtain maintainer approval for the slow-worktree timeout policy before merge.

Findings

• [P1] Keep the deadline active while draining inherited output pipes — src/infra/process.rs:64-70
• [P1] Avoid rejecting valid slow worktrees at a fixed 15 seconds — src/openclaw_repo.rs:15-23

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	3 files; 198 added, 114 removed	The patch centralizes subprocess handling and applies the new behavior across OCM worktree and restart-handoff paths.
Git coverage	9 Git invocation sites routed through one 15-second helper	A single timeout-policy choice now affects setup, cleanup, inspection, listing, and identity checks.

Merge-risk options

Maintainer options:

1. Repair full deadline enforcement (recommended)
   Keep the deadline active until inherited output pipes drain, terminating the original process group if a descendant keeps them open.
2. Choose the slow-repository policy
   Before merging, approve a timeout behavior that does not silently turn valid long-running worktree commands into failures.

Technical review

Best possible solution:

Keep the shared cancellation helper, but make the deadline cover process-group lifetime and output draining, then adopt an explicitly approved timeout policy that preserves valid slow worktree operations.

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

Yes, source inspection gives a high-confidence reproduction shape: a direct child can exit while a descendant retains inherited output, causing the introduced reader joins to block indefinitely. The supplied terminal trace only covers a foreground child that exceeds its deadline.

Is this the best way to solve the issue?

No, not as submitted: centralizing cancellation is maintainable, but the deadline must include output-drain completion and the 15-second compatibility policy needs maintainer approval.

Full review comments:

• [P1] Keep the deadline active while draining inherited output pipes — src/infra/process.rs:64-70
  After wait_for_child observes the leader exit, these joins have no deadline. A child such as sh -c 'sleep 30 &' can exit successfully while its descendant retains stdout or stderr; the reader then blocks indefinitely, so the worktree helper remains unbounded. Keep the process-group deadline active until both pipes close and terminate the group when they do not.
  Confidence: 0.96
• [P1] Avoid rejecting valid slow worktrees at a fixed 15 seconds — src/openclaw_repo.rs:15-23
  Every worktree Git call now receives this fixed limit, including recursive submodule inspection and cleanup. On an existing large or slow worktree those commands can legitimately exceed 15 seconds, turning a previously successful dev or upgrade operation into an error. Preserve a bounded stuck-process policy without imposing an unapproved universal failure limit.
  Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against e04c10166d6b.

Labels

Label changes:

• add P1: The PR targets hangs in active development and upgrade worktree workflows, while its remaining defects can still block or fail those flows.
• add merge-risk: 🚨 compatibility: The introduced fixed deadline changes previously unbounded successful Git operations into errors on slow existing repositories.
• add merge-risk: 🚨 availability: An inherited output pipe held by a descendant can keep the CLI stalled after the direct child exits.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The supplied macOS terminal trace proves the changed shared runner times out a foreground /bin/sleep child, but it neither runs a git_output worktree caller nor exercises the leader-exit/inherited-pipe case that still defeats the deadline. Add a redacted terminal trace of the repaired production path; updating the PR body should trigger a fresh review, or a maintainer can request one with @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.

Label justifications:

• P1: The PR targets hangs in active development and upgrade worktree workflows, while its remaining defects can still block or fail those flows.
• merge-risk: 🚨 compatibility: The introduced fixed deadline changes previously unbounded successful Git operations into errors on slow existing repositories.
• merge-risk: 🚨 availability: An inherited output pipe held by a descendant can keep the CLI stalled after the direct child exits.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp 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 supplied macOS terminal trace proves the changed shared runner times out a foreground /bin/sleep child, but it neither runs a git_output worktree caller nor exercises the leader-exit/inherited-pipe case that still defeats the deadline. Add a redacted terminal trace of the repaired production path; updating the PR body should trigger a fresh review, or a maintainer can request one with @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:

• Current main has unbounded Git helpers: The fetched default branch invokes Command::output directly for worktree add, clean, remove, status, ignored-file inspection, worktree listing, and rev-parse; a blocked Git process therefore blocks the caller. (src/openclaw_repo.rs:123, e04c10166d6b)
• Introduced fixed deadline: All routed Git commands now use one fixed 15-second timeout through git_output. (src/openclaw_repo.rs:15, b080857e1a4a)
• Output-drain gap: The timer stops once the direct child exits, then the runner unconditionally joins both pipe readers. A descendant retaining inherited stdout or stderr can keep either reader blocked indefinitely after the leader has exited. (src/infra/process.rs:64, b080857e1a4a)
• Submitted terminal proof: The PR body records a macOS run where the new runner times out a foreground /bin/sleep child at roughly 200 ms and a compiled regression test passes. It does not exercise a Git caller or a leader that exits while a descendant retains an output pipe.
• Worktree history: The worktree-backed development feature was introduced by Shakker in the recorded-parent commit below; Vitor Cepeda Lopes later made the most recent merged cleanup change in this path. (src/openclaw_repo.rs:102, 03e4a8e297d2)

Likely related people:

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

• Make output-drain completion subject to the same process-group deadline and add regression coverage for an exited leader with a descendant holding an output pipe.
• Get an explicit timeout-compatibility decision and prove both a valid slow worktree operation and timeout recovery with redacted terminal output.

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.