fix(eval): time out hung docker kill and compose down

[SebTardif]

What Problem This Solves

Native eval already bounds docker exec with asyncio.timeout. After that
timeout fires, cleanup still called run_process(["docker", "kill", ...])
with no deadline. stop() did the same for docker compose down and
docker rm -f.

An enclosing timeout only cancelled the await. run_process had already
started the Docker CLI child and never terminated it, so a hung docker
process leaked for every affected trial.

Evidence

Live python on this branch imported run_process and spawned a real
30-second child (python -c sleep). A 0.4s deadline cancelled the await
and the helper terminated the child.

$ python3 - <<'PY'
# run_process(["python", "-c", "print(pid); sleep(30)"])
# under asyncio.timeout(0.4)
timeout after 0.40s
child pid 13267
reaped
PY

The same helper is used for docker kill, docker compose down, and
docker rm -f.

Real behavior proof

• Behavior or issue addressed: Hung Docker CLI cleanup after an agent timeout leaked the child. run_process now terminates and reaps the subprocess when the enclosing deadline expires.

• Real environment tested: macOS, Python 3.14, branch fix/docker-cleanup-timeout at /tmp/shellbench-66.

• Exact steps or command run after this patch:

  python3 - <<'PY'
  import asyncio, os, sys, time
  from pathlib import Path
  from scripts.native_eval.runtime import run_process
  out = Path("/tmp/sb66-child.out")
  child = [sys.executable, "-c", "import os,time; print(os.getpid(), flush=True); time.sleep(30)"]
  async def main():
      t0 = time.monotonic()
      try:
          async with asyncio.timeout(0.4):
              await run_process(child, stdout_path=out, stderr_path=Path("/tmp/sb66-child.err"))
      except TimeoutError:
          print(f"timeout after {time.monotonic()-t0:.2f}s")
      pid = int(out.read_text().strip())
      print(f"child pid {pid}")
      try:
          os.kill(pid, 0)
          print("ALIVE")
      except OSError:
          print("reaped")
  asyncio.run(main())
  PY

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

  timeout after 0.40s
  child pid 13267
  reaped

• Observed result after fix: Control returns in 0.40s. The child PID is gone. After SIGKILL, wait() is also bounded (2s). A child stuck in uninterruptible I/O cannot pin the 30s cleanup deadline.

• What was not tested: A real dockerd hang on this machine. The live command uses a real long-lived child in place of a stuck Docker CLI.

What does this PR do?

Own the Docker CLI child inside run_process. On cancel or timeout,
terminate, then kill, and bound both wait() calls so a stuck child
cannot pin cleanup. Keep the 30s deadline around docker kill,
compose down, and docker rm -f.

Why?

Introduced in #42
(69f75c6629c4,
2026-07-29). Related wait hardening: #19.
Related 30s bound: #8.

Claw review on c1a5352 asked to terminate and reap the timed-out
client. Review on 636c2d4 asked to bound the wait after SIGKILL.

Changes

• _reap_process on TimeoutError / CancelledError in run_process
• 2s deadline on both terminate-wait and kill-wait
• 30s deadline still wraps the three cleanup call sites
• Real-child hang coverage (sleeping Python process, PID gone after timeout)
• No changelog edit (release-owned)

Tests

• python3 -m pytest -q tests/test_native_eval_runtime.py passes locally
• python3 -m ruff check / ruff format --check on the changed files

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 20, 2026, 3:34 AM ET / 07:34 UTC.

ClawSweeper review

What this changes

The PR adds deadlines to Docker cleanup commands and makes the native evaluator terminate, kill, and reap cancelled Docker CLI subprocesses.

Regression provenance

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

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep this PR open for normal merge review: the revised helper bounds both graceful and forced-reap waits, and the focused tests plus real-child transcript support the intended recovery behavior. No blocking correctness defect was found in the current head.

Priority: P2
Reviewed head: e7e75af5c7867765554b2c647d418ffac342cb15

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, well-covered reliability fix with credible after-fix terminal proof.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The PR body records an after-fix live run against a real sleeping child, showing timeout return and that the child PID was reaped.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body records an after-fix live run against a real sleeping child, showing timeout return and that the child PID was reaped.
Evidence reviewed	5 items	Bounded Docker cleanup: The three Docker cleanup call sites are wrapped in a 30-second timeout, covering kill, compose teardown, and container removal. Cancellation reaping: The subprocess helper catches timeout or cancellation, terminates the child, escalates to kill after two seconds, and bounds the post-kill wait. Focused regression coverage: Coverage exercises each hanging cleanup path, a real long-lived child that is observed reaped, and a process whose wait remains stuck after kill.
Findings	None	None.
Security	None	None.

Live Verification

Command: python3 -m pytest -q tests/test_native_eval_runtime.py

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

• FAIL expect_output: 5 passed

How this fits together

ShellBench’s native evaluator launches Docker commands for benchmark trials and writes their output to trial logs. When a trial or teardown times out, the evaluator must clean up the Docker CLI child before returning control to the trial lifecycle.

flowchart LR
A[Native evaluation trial] --> B[Docker CLI subprocess]
B --> C[Trial and environment logs]
A --> D[Timeout or cancellation]
D --> E[Terminate and reap child]
E --> F[Docker cleanup command]
F --> G[Trial lifecycle returns]

Loading

Before merge

• Resolve merge risk (P1) - A Docker CLI process stuck in uninterruptible I/O can still outlive the evaluator; the patch intentionally bounds evaluator cleanup rather than waiting indefinitely, and its maximum path includes the two bounded reap waits.
• Complete next step (P2) - No discrete repair finding remains; this PR should proceed through normal merge and required-check gating.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Implementation and coverage	production +55, tests +148	The added coverage directly exercises all three cleanup call sites and the subprocess reaping path.

Merge-risk options

Maintainer options:

1. Keep bounded cleanup (recommended)
   Accept the bounded-reaping tradeoff so an uninterruptible Docker CLI child cannot hold the evaluator indefinitely.

Technical review

Best possible solution:

Land the centralized subprocess-reaping behavior with the existing focused tests, retaining bounded cleanup so a wedged Docker client cannot indefinitely stall a benchmark trial.

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

Yes. The PR supplies a concrete real-child timeout transcript, and current source plus focused tests provide a high-confidence path that exercises the same cancellation and reaping helper.

Is this the best way to solve the issue?

Yes. Centralizing reaping in the subprocess helper covers the Docker cleanup calls without creating a parallel cleanup mechanism.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body records an after-fix live run against a real sleeping child, showing timeout return and that the child PID was reaped.
• remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
• remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

• P2: The change repairs bounded cleanup for individual Docker-based benchmark trials without an established broad user outage.
• merge-risk: 🚨 availability: The PR changes evaluator timeout and subprocess-lifecycle behavior during Docker teardown.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body records an after-fix live run against a real sleeping child, showing timeout return and that the child PID was reaped.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body records an after-fix live run against a real sleeping child, showing timeout return and that the child PID was reaped.

Evidence

What I checked:

• Bounded Docker cleanup: The three Docker cleanup call sites are wrapped in a 30-second timeout, covering kill, compose teardown, and container removal. (scripts/native_eval/runtime.py:421, e7e75af5c786)
• Cancellation reaping: The subprocess helper catches timeout or cancellation, terminates the child, escalates to kill after two seconds, and bounds the post-kill wait. (scripts/native_eval/runtime.py:1304, e7e75af5c786)
• Focused regression coverage: Coverage exercises each hanging cleanup path, a real long-lived child that is observed reaped, and a process whose wait remains stuck after kill. (tests/test_native_eval_runtime.py:60, e7e75af5c786)
• Feature history: Current-main history shows Vincent Koc’s native-evaluator work in the runtime path after the merged native matrix runner, making that area the best routing surface for follow-up review. (scripts/native_eval/runtime.py:389, 884dd1bb5511)
• Runtime compatibility: The repository supports Python 3.11 and later, which provides asyncio.timeout used by this patch. (pyproject.toml:7, e7e75af5c786)

Likely related people:

• vincentkoc: The merged native matrix runner introduced this runtime surface, and later current-main commits by Vincent Koc continue to modify the native evaluator runtime. (role: native-evaluator feature author and recent area contributor; confidence: medium; commits: 69f75c6629c4, b9acd9f7a010, 884dd1bb5511; files: scripts/native_eval/runtime.py)

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

• reviewed 2026-08-15T22:44:08.729Z sha c1a5352 :: needs real behavior proof before merge. :: [P2] Terminate and reap timed-out Docker clients | [P3] Leave the release-owned changelog unchanged
• reviewed 2026-08-20T04:19:59.997Z sha 636c2d4 :: needs changes before merge. :: [P2] Bound the wait after killing a child

[SebTardif]

@clawsweeper re-review

Terminate and reap timed-out Docker clients

Done on 636c2d4. run_process now terminates/kills and waits on cancel. Live child 13267 was reaped after a 0.40s deadline.

[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.

[SebTardif]

@clawsweeper re-review

Bound the wait after killing a child

Done on e7e75af. Both terminate-wait and kill-wait use a 2s deadline. A stuck wait() after SIGKILL returns instead of pinning cleanup.

[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.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/32345552619
• Updated: 2026-08-20T07:46:36.242Z