fix: time out unbounded lane prepare hook

[SebTardif]

What Problem This Solves

When CLAWBENCH_LANE_PREPARE_CMD is set, each parallel lane runs that
executable before the gateway starts. _run_lane_prepare_hook called
subprocess.run([hook], env=hook_env, check=True) with no timeout.

If the hook never exits (stuck install, hung network, a sleep that never
returns), lane startup blocks in that child. The EvalWorker cannot start
the gateway, probe /health, or finish the job. container_lane_eval.sh
sets this hook for gbrain (setup_gbrain_runtime.sh). Operators can also
point it at any command on a Space or worker.

Sibling timeouts already exist in this repo: gateway health defaults to
180s in the same file, and fleet Crabbox stop uses run_with_timeout.
This hook was the remaining unbounded subprocess.run on the lane
startup path.

Evidence

Live python3 on this branch pointed CLAWBENCH_LANE_PREPARE_CMD at a
real sleep 1000 hook and set CLAWBENCH_LANE_PREPARE_TIMEOUT_SECONDS=1.
EvalWorker._run_lane_prepare_hook raised after 1.00s and the hook pid
was gone. The same child under subprocess.run with timeout=1 raised
TimeoutExpired instead of waiting out the 1000s sleep.

$ python3 /tmp/proof-f005.py
=== after-fix: EvalWorker._run_lane_prepare_hook ===
hook=/var/folders/7_/3g02szlx2h3_4pdqp0knlw740000gn/T/f005-proof-6ngy_6o5/hung-hook
timeout_seconds=1
raised=RuntimeError: Lane 1 prepare hook timed out after 1s
elapsed_s=1.00
hook_pid=33276
hook_alive=False

=== sibling-unbounded pattern (no timeout=) still hangs ===
with timeout=1: TimeoutExpired after 1.00s

=== inspect production call site ===
timeout_seconds = int(os.environ.get("CLAWBENCH_LANE_PREPARE_TIMEOUT_SECONDS", "180"))
subprocess.run([hook], env=hook_env, check=True, timeout=timeout_seconds)

Real behavior proof

• Behavior or issue addressed: A hung CLAWBENCH_LANE_PREPARE_CMD could pin EvalWorker lane startup because _run_lane_prepare_hook used unbounded subprocess.run. The hook now takes timeout= from CLAWBENCH_LANE_PREPARE_TIMEOUT_SECONDS (default 180, same as gateway health) and fails the lane with RuntimeError after the child is killed.

• Real environment tested: macOS 26.6.2 Darwin 25.6.0 arm64, Python 3.14.7, checkout /tmp/shellbench-F005 on fix/prepare-hook-timeout.

• Exact steps or command run after this patch:

  python3 /tmp/proof-f005.py

• Evidence after fix: terminal output from the live command:

  raised=RuntimeError: Lane 1 prepare hook timed out after 1s
  elapsed_s=1.00
  hook_pid=33276
  hook_alive=False
  with timeout=1: TimeoutExpired after 1.00s
  timeout_seconds = int(os.environ.get("CLAWBENCH_LANE_PREPARE_TIMEOUT_SECONDS", "180"))
  subprocess.run([hook], env=hook_env, check=True, timeout=timeout_seconds)

• Observed result after fix: The prepare hook returned in 1.00s against a 1000s child. The hook pid was not alive afterward. Production default remains 180s and is overridable through CLAWBENCH_LANE_PREPARE_TIMEOUT_SECONDS.

• What was not tested: A live gbrain setup_gbrain_runtime.sh hang inside a container Space.

What does this PR do?

Bound _run_lane_prepare_hook with subprocess.run(..., timeout=...) and
fail the lane when the hook expires.

Why?

An unbounded prepare hook stalls every later step on that lane. The hang
was introduced in #78
(c1a79f7,
2026-08-31). Related timeout work: #66
(docker cleanup) and #77
(fleet inspect/warmup). CPython documents subprocess.TimeoutExpired as
the way to bound subprocess.run
(docs).

Changes

• Read CLAWBENCH_LANE_PREPARE_TIMEOUT_SECONDS (default 180) in
  _run_lane_prepare_hook
• Pass timeout= to subprocess.run and wrap TimeoutExpired as
  RuntimeError so the lane fails instead of hanging

Tests

• tests/test_worker.py::test_run_lane_prepare_hook_kills_hung_hook
  starts a real sleep 1000 hook, requires the worker to return, and
  checks the child pid is gone
• python -m pytest -q passes locally
• python -m ruff check clawbench app.py scripts tests passes locally, or the change is docs-only

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 4, 2026, 3:57 AM ET / 07:57 UTC.

ClawSweeper review

What this changes

Adds a configurable 180-second deadline to evaluation-lane preparation hooks, reports timeout failures, and tests termination of a directly executed hanging hook.

Merge readiness

⛔ Blocked before merge - 5 items remain

The fix remains necessary on current main. The process-group cleanup finding still applies; the earlier container-preflight finding is withdrawn because that unchanged path is not regressed by this PR. The supplied direct-child timeout trace is valid real behavior proof.

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

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Useful, focused work with real subprocess proof, but incomplete descendant cleanup and unresolved default compatibility prevent landing.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The captured macOS terminal trace exercises the actual EvalWorker preparation helper with a real hanging executable and observes failure after 1.00 seconds with its PID gone. This proves the direct-child deadline; descendant cleanup remains a separate correctness finding, and the unchanged container preflight does not require proof for this patch.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured macOS terminal trace exercises the actual EvalWorker preparation helper with a real hanging executable and observes failure after 1.00 seconds with its PID gone. This proves the direct-child deadline; descendant cleanup remains a separate correctness finding, and the unchanged container preflight does not require proof for this patch.
Evidence reviewed	10 items	Pinned patch and clean checkout: The introduced delta contains only worker timeout handling and its test. The host-verified test merge has the pinned main and head parents, and its comparison against main contains the same changes. Final repository status was clean. Current main still waits without a deadline: The default-branch preparation helper still calls subprocess.run without timeout. Gateway health checks begin only after preparation, so their separate deadline does not bound this call. Timeout cleanup does not own hook descendants: The new subprocess.run deadline has no isolated process group. The caller invokes preparation before registering a gateway process, while job cleanup stops registered gateways and removes the lane directory. Existing gateway launches already use isolated process groups.
Findings	1 actionable finding	[P2] Terminate the complete prepare-hook process group
Security	None	None.

How this fits together

ShellBench’s evaluation worker prepares isolated lanes before starting their OpenClaw gateways and running benchmark tasks. Operator-supplied preparation executables receive the lane environment; their completion or failure determines whether startup proceeds.

flowchart TD
  A[Benchmark job] --> B[Prepare isolated lane]
  B --> C[Run configured preparation hook]
  C --> D{Completes before deadline?}
  D -->|Yes| E[Start gateway and evaluate tasks]
  D -->|No| F[Report failure and clean up lane]

Loading

Decision needed

Question	Recommendation
Should existing preparation hooks receive a mandatory 180-second deadline, or should enforcing a deadline initially require explicit configuration?	Make the deadline opt-in initially: Preserve existing hook behavior while allowing operators to enable bounded preparation explicitly.

Why: Existing hooks are arbitrary operator executables, and the supplied expiry trace does not establish that legitimate installations finish within the new default.

Before merge

• Terminate the complete prepare-hook process group (P2) - For a hook that starts a child and waits, the new timeout kills only the hook PID. The child survives while the lane reports failure and job cleanup can remove its state directory; gateway cleanup cannot reach it because preparation runs before the gateway is registered. Launch the hook in an isolated process group, retain its group identity, stop the group on expiry, and reap the direct child. The current exec sleep test replaces the shell and misses this case.
• Resolve merge risk (P2) - Shell-spawned installers or services can survive timeout and continue consuming resources or accessing lane files during cleanup.
• Resolve merge risk (P1) - Existing valid hooks taking longer than 180 seconds will now fail unless operators configure the new override; normal preparation and upgrade compatibility have not been demonstrated.
• Complete next step (P2) - Repair process-group termination, then resolve the default cutoff with maintainers and demonstrate compatible fresh and upgraded preparation.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [P2] Terminate the complete prepare-hook process group — clawbench/worker.py:633-638

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +7/-1; tests +57/-0	The production growth has a stated timeout purpose, and the new regression test exercises a real direct child.

Merge-risk options

Maintainer options:

1. Stop the complete hook process group (recommended)
   Launch preparation in an isolated group, terminate its descendants on expiry, and verify cleanup with a non-exec shell child.
2. Resolve the default cutoff before landing
   Pause landing until maintainers choose opt-in timing or approve the 180-second default with successful fresh and upgrade coverage.

Technical review

Best possible solution:

Own and terminate the hook’s process group, and preserve existing timing behavior through an explicit opt-in deadline unless maintainers approve a default cutoff backed by upgrade evidence.

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

Yes, from source: a never-exiting configured hook blocks current main before gateway startup; on the PR, a shell waiting for a child can be killed at timeout while that child survives. No target code or tests were executed during this read-only review.

Is this the best way to solve the issue?

Partly: placing a deadline at the worker-owned subprocess boundary is appropriate, but direct-PID termination is incomplete and the default cutoff needs an explicit compatibility decision.

Full review comments:

• [P2] Terminate the complete prepare-hook process group — clawbench/worker.py:633-638
  For a hook that starts a child and waits, the new timeout kills only the hook PID. The child survives while the lane reports failure and job cleanup can remove its state directory; gateway cleanup cannot reach it because preparation runs before the gateway is registered. Launch the hook in an isolated process group, retain its group identity, stop the group on expiry, and reap the direct child. The current exec sleep test replaces the shell and misses this case.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

• P2: This addresses a bounded worker-startup reliability problem affecting configured preparation hooks.
• merge-risk: 🚨 availability: The introduced timeout can orphan shell descendants while the worker proceeds with failure cleanup.
• merge-risk: 🚨 compatibility: Existing hooks acquire a new 180-second cutoff without demonstrated upgrade compatibility.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The captured macOS terminal trace exercises the actual EvalWorker preparation helper with a real hanging executable and observes failure after 1.00 seconds with its PID gone. This proves the direct-child deadline; descendant cleanup remains a separate correctness finding, and the unchanged container preflight does not require proof for this patch.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured macOS terminal trace exercises the actual EvalWorker preparation helper with a real hanging executable and observes failure after 1.00 seconds with its PID gone. This proves the direct-child deadline; descendant cleanup remains a separate correctness finding, and the unchanged container preflight does not require proof for this patch.

Evidence

What I checked:

• Pinned patch and clean checkout: The introduced delta contains only worker timeout handling and its test. The host-verified test merge has the pinned main and head parents, and its comparison against main contains the same changes. Final repository status was clean. (clawbench/worker.py:632, 335003e7cbbe)
• Current main still waits without a deadline: The default-branch preparation helper still calls subprocess.run without timeout. Gateway health checks begin only after preparation, so their separate deadline does not bound this call. (clawbench/worker.py:632, c1a79f731541)
• Timeout cleanup does not own hook descendants: The new subprocess.run deadline has no isolated process group. The caller invokes preparation before registering a gateway process, while job cleanup stops registered gateways and removes the lane directory. Existing gateway launches already use isolated process groups. (clawbench/worker.py:634, 335003e7cbbe)
• Inspected subprocess termination contract: The installed Python 3.12 standard library’s subprocess.run timeout handler calls process.kill(), then wait(); POSIX kill ultimately calls os.kill(self.pid, sig), not killpg. This dependency contract applies directly to the newly added timeout argument. Installed-source repository ownership and commit were not established.
• Captured real behavior proof: The complete supplied PR body at sourceRevision a4ba900eb4da8ca27ff91cf65f79f7dafc42e8b409f97b4c287735740b78af7b records the actual EvalWorker helper on macOS with a real hanging executable: RuntimeError after 1.00 seconds and hook_alive=False. This demonstrates the changed direct-child timeout. The added test uses exec sleep, so neither establishes descendant cleanup. (tests/test_worker.py:464, 335003e7cbbe)
• Prior finding withdrawn after ownership check: The container gbrain preflight directly invokes the hook before constructing EvalWorker, but that invocation is identical on main and the reviewed head. The previous review used the same head. Its preflight finding and associated proof request concern pre-existing work outside this patch. (scripts/container_lane_eval.sh:582, 335003e7cbbe)

Likely related people:

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

• Add process-group cleanup and verify that a non-exec shell child cannot survive timeout or perform a delayed write.
• Resolve the timeout default and document successful fresh preparation plus existing-hook behavior with any required override.

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-02T17:05:28.258Z sha 335003e :: needs changes before merge. :: [P2] Cover the shell-level prepare invocation
• reviewed 2026-09-03T06:04:16.121Z sha 335003e :: blocked before merge. :: [P2] Terminate the complete prepare-hook process group | [P2] Bound the gbrain shell preflight too
• reviewed 2026-09-03T17:01:23.455Z sha 335003e :: blocked before merge. :: [P2] Terminate the complete prepare-hook process group | [P2] Bound the gbrain preflight invocation
• reviewed 2026-09-03T19:38:00.787Z sha 335003e :: needs real behavior proof before merge. :: [P2] Terminate the complete prepare-hook process group | [P2] Bound the container gbrain preflight invocation