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 26, 2026, 1:07 PM ET / 17:07 UTC.

ClawSweeper review

What this changes

The native evaluator now bounds Docker cleanup commands and terminates, kills, and reaps their child processes when cancellation or a timeout occurs.

Regression provenance

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

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This PR remains necessary: current main still runs Docker kill, compose down, and container removal without a cleanup deadline or subprocess reaping. The patch is focused, has no blocking correctness finding, and includes credible real-process proof; it is ready for normal maintainer review.

Priority: P2
Reviewed head: e7e75af5c7867765554b2c647d418ffac342cb15

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, well-covered reliability repair with credible real-process proof is ready for ordinary maintainer review.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The PR body records an after-fix live Python run that starts a real sleeping child, times out after 0.40 seconds, and confirms 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 Python run that starts a real sleeping child, times out after 0.40 seconds, and confirms the child PID was reaped.
Evidence reviewed	4 items	Current main lacks the requested cleanup bound: Current main calls Docker kill, compose down, and docker rm -f directly from timeout and stop paths; none is enclosed in a cleanup timeout, and its process helper does not handle cancellation. PR implements bounded cleanup and reaping: The PR wraps all three cleanup call sites in a 30-second timeout and adds terminate, kill, and bounded wait handling when the subprocess helper is cancelled. Regression coverage includes a real child process: The added test starts an actual sleeping Python subprocess under a 0.4-second deadline, verifies the timeout returns promptly, and confirms its PID is gone; separate tests cover each hanging Docker cleanup path.
Findings	None	None.
Security	None	None.

Live Verification

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

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.24.0.tgz

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

Assertions:

• FAIL expect_output: 1 passed

How this fits together

ShellBench’s native evaluator creates Docker task environments, runs an agent inside them, then tears down containers and Compose projects. Docker CLI subprocesses write logs and return control to the trial runner, which records benchmark results.

flowchart LR
  A[Native trial runner] --> B[Docker task environment]
  B --> C[Agent command timeout]
  C --> D[Docker cleanup command]
  D --> E[Subprocess reaping]
  E --> F[Trial completion and results]
  B --> G[Environment stop]
  G --> D

Loading

Before merge

• Resolve merge risk (P1) - A Docker CLI stuck in uninterruptible I/O can outlive SIGKILL; the patch bounds evaluator waiting rather than guaranteeing that the operating system process has exited, and its two reaping waits can extend a nominal 30-second cleanup by up to four seconds.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Runtime and coverage delta	production +55/-22, tests +148	The added subprocess lifecycle handling is accompanied by coverage for all three cleanup call sites and a real child-process timeout.

Merge-risk options

Maintainer options:

1. Accept bounded best-effort cleanup (recommended)
   Accept that a Docker client stuck in uninterruptible I/O may survive while the evaluator regains control after the bounded terminate and kill waits.
2. Make 30 seconds a total cleanup cap
   If the 30-second value is a hard end-to-end limit, pass the remaining deadline into reaping so terminate and kill waits cannot extend it.

Technical review

Best possible solution:

Merge the focused cancellation-safe cleanup once a reviewer accepts the bounded best-effort behavior for an uninterruptible Docker client.

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

Yes. Current main’s unbounded cleanup calls are directly visible in source, and the PR supplies a concrete real-child timeout command that demonstrates the proposed reaping behavior.

Is this the best way to solve the issue?

Yes. Centralizing cancellation cleanup in the existing subprocess helper while preserving narrow time bounds at the three Docker cleanup call sites is the smallest maintainable repair.

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

• P2: Hung Docker cleanup can delay or stall native benchmark trials, but the impact is limited to the evaluator path.
• merge-risk: 🚨 availability: This patch changes process cancellation and cleanup timing for native evaluator Docker commands.
• 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 Python run that starts a real sleeping child, times out after 0.40 seconds, and confirms the child PID was reaped.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body records an after-fix live Python run that starts a real sleeping child, times out after 0.40 seconds, and confirms the child PID was reaped.

Evidence

What I checked:

• Current main lacks the requested cleanup bound: Current main calls Docker kill, compose down, and docker rm -f directly from timeout and stop paths; none is enclosed in a cleanup timeout, and its process helper does not handle cancellation. (scripts/native_eval/runtime.py:420, 884dd1bb5511)
• PR implements bounded cleanup and reaping: The PR wraps all three cleanup call sites in a 30-second timeout and adds terminate, kill, and bounded wait handling when the subprocess helper is cancelled. (scripts/native_eval/runtime.py:422, e7e75af5c786)
• Regression coverage includes a real child process: The added test starts an actual sleeping Python subprocess under a 0.4-second deadline, verifies the timeout returns promptly, and confirms its PID is gone; separate tests cover each hanging Docker cleanup path. (tests/test_native_eval_runtime.py:98, e7e75af5c786)
• Native evaluator provenance: The native evaluator was introduced by the merged native-matrix work and was most recently updated on current main by native-eval trace work; the PR’s proposed helper is not present in either current-main source path. (scripts/native_eval/runtime.py:420, 884dd1bb5511)

Likely related people:

• vincentkoc: Vincent Koc authored merged native-matrix commit 69f75c6 and the current-main native-eval follow-up 884dd1b, which establishes the surrounding runtime ownership history. (role: native evaluator introducer and recent area contributor; confidence: high; commits: 69f75c6629c4, 884dd1bb5511; files: scripts/native_eval/runtime.py)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Confirm whether the documented 30-second cleanup duration is intended to exclude the bounded reaping period.

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-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
• reviewed 2026-08-20T07:34:39.197Z sha e7e75af :: needs maintainer review before merge. :: none

[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