fix(fleet): timeout crabbox inspect and warmup

[SebTardif]

What Problem This Solves

The native fleet controller talks to Crabbox over inspect and warmup
to lease AWS machines. Those two CLI calls used SubprocessExecutor.run(),
which wraps subprocess.run with no timeout. Only crabbox stop already
used run_with_timeout.

If the coordinator never returns, the controller stays blocked in that
child. A native eval wave cannot recover, re-lease, or export while inspect
or warmup is stuck. The public path is python -m scripts.native_eval.fleet.

Evidence

Live python3 on this branch pointed crabbox_bin at a real 30-second
sleep child. FleetController._inspect_lease and _warmup_lease both
returned a FleetError in 0.40s instead of waiting for the child to exit.
SubprocessExecutor.run() without a timeout was still running after 1s
and finished only when the 30s sleep ended.

$ python3 /tmp/shellbench-F004-proof.py
production_inspect_timeout_s=45
production_warmup_timeout_s=120
proof_deadline_s=0.4
inspect_status=FleetError
inspect_elapsed_s=0.40
inspect_error=Crabbox inspect failed for cbx_hung: timed out after 0.4s
warmup_status=FleetError
warmup_elapsed_s=0.40
warmup_error=lease Crabbox hung failed with exit 124: timed out after 0.4s
direct_returncode=124
direct_stderr=timed out after 0.4s
OK: inspect and warmup returned instead of waiting out the 30s child

$ python3 - <<'PY'
# unbounded run() vs run_with_timeout on the same sleep 30 child
after_1s_unbounded_still_alive=True
bounded_elapsed_s=0.40
bounded_returncode=124
bounded_stderr=timed out after 0.4s
unbounded_finished=True
unbounded_elapsed_s=30.12928366700362
unbounded_returncode=0
PY

Real behavior proof

• Behavior or issue addressed: Hung Crabbox inspect and warmup could pin the native fleet controller because those lease CLI calls used unbounded subprocess.run. They now go through run_with_timeout and return exit 124 with timed out after ...s.

• Real environment tested: macOS Darwin 25.6.0 arm64, Python 3.14.7, checkout /tmp/oc-pr-shellbench-F004 on fix/f004-fleet-subprocess-timeout.

• Exact steps or command run after this patch:

  python3 /tmp/shellbench-F004-proof.py

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

  inspect_status=FleetError
  inspect_elapsed_s=0.40
  inspect_error=Crabbox inspect failed for cbx_hung: timed out after 0.4s
  warmup_status=FleetError
  warmup_elapsed_s=0.40
  warmup_error=lease Crabbox hung failed with exit 124: timed out after 0.4s
  direct_returncode=124
  direct_stderr=timed out after 0.4s
  OK: inspect and warmup returned instead of waiting out the 30s child

• Observed result after fix: Inspect and warmup returned in 0.40s against a 30s child. Production bounds stay 45s for inspect and 120s for warmup. The same child under unbounded run() stayed alive past 1s and exited 0 after 30.13s.

• What was not tested: A live Crabbox coordinator hang against real AWS leases.

What does this PR do?

Route Crabbox inspect and warmup through run_with_timeout, the same
helper already used for crabbox stop.

Why?

Unbounded subprocess.run landed with the native fleet runner in
#42
(69f75c6629c4,
2026-07-29). run_with_timeout was added later the same day for stop
only (b9acd9f7). Related timeout class:
#66. No open pull request
already changes inspect or warmup.

Changes

• 45s timeout on Crabbox inspect
• 120s timeout on Crabbox warmup (create POST; readiness still uses inspect polling)
• CommandExecutor now includes run_with_timeout
• No changelog edit (release-owned)

Tests

• python -m pytest -q tests/test_native_eval_fleet.py passes locally
• python -m ruff check scripts/native_eval/fleet.py tests/test_native_eval_fleet.py passes locally

[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: blocked before merge. Reviewed September 4, 2026, 12:53 PM ET / 16:53 UTC.

ClawSweeper review

What this changes

The PR adds fixed subprocess deadlines to Crabbox lease inspection and warmup, with regression coverage for timeout handling.

Regression provenance

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

Merge readiness

⛔ Blocked before merge - 3 items remain

Keep open: the timeout fix has credible real-process proof, but it breaks the existing injected-executor boundary. A caller that supplies an executor implementing the prior run-only contract now fails in lease inspection or warmup with AttributeError; this is the unresolved prior P2 finding on the unchanged PR head.

Priority: P2
Reviewed head: 9463ebb96ce64048081ce2c0bcfa6d6c7505f784

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The timeout behavior has solid real-process evidence, but the unresolved injected-executor regression prevents merge readiness.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The changed production owner is FleetController's Crabbox inspect and warmup dispatch; the PR body supplies a terminal trace from those methods against a real long-lived child, showing both recover with FleetError in 0.40 seconds instead of waiting for 30 seconds. The trace does not exercise a live AWS coordinator, but it directly proves the subprocess-timeout behavior changed here.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owner is FleetController's Crabbox inspect and warmup dispatch; the PR body supplies a terminal trace from those methods against a real long-lived child, showing both recover with FleetError in 0.40 seconds instead of waiting for 30 seconds. The trace does not exercise a live AWS coordinator, but it directly proves the subprocess-timeout behavior changed here.
Evidence reviewed	7 items	Current main still needs this fix: The fetched main snapshot calls the injected executor's run method without a timeout in both warmup and inspection, so current main has not already implemented this PR's central behavior. Introduced compatibility regression: The PR unconditionally calls run_with_timeout for warmup and inspection, while FleetController continues to accept a caller-supplied executor. Run-only injected executors therefore fail before either lease operation can run. Existing compatibility pattern: The existing stop path preserves injected executor compatibility by using run_with_timeout only for SubprocessExecutor and otherwise calling run; inspect and warmup do not retain that fallback.
Findings	1 actionable finding	[P2] Preserve run-only injected executors
Security	None	None.

How this fits together

The native fleet controller provisions and inspects Crabbox AWS leases before dispatching benchmark runs. It invokes the Crabbox CLI through an injectable command executor and turns lease results into scheduling state.

flowchart LR
  A[Native evaluation command] --> B[Fleet controller]
  B --> C[Lease inspection]
  B --> D[Lease warmup]
  C --> E[Command executor]
  D --> E
  E --> F[Crabbox coordinator]
  F --> G[Ready lease]
  G --> H[Remote benchmark run]

Loading

Before merge

• Preserve run-only injected executors (P2) - FleetController still accepts a caller-supplied executor, but these unconditional calls require its new run_with_timeout method and raise AttributeError for the prior run-only contract. The existing stop path retains a SubprocessExecutor guard and fallback; the new test uses SubprocessExecutor and expands FakeExecutor, so it misses this regression. This is the unresolved finding from the earlier review of the same head.
• Resolve merge risk (P1) - Merging as written makes any existing run-only injected executor fail during inspect or warmup, preventing lease work from progressing.
• Complete next step (P2) - Restore the run-only executor fallback for inspect and warmup and add regression coverage for both operations.

Findings

• [P2] Preserve run-only injected executors — scripts/native_eval/fleet.py:711-715

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Lease operations bounded	2 operations changed	Both lease lookup and lease creation now receive explicit subprocess deadlines.
Production versus test delta	production +21 net, tests +49	Most added lines are focused timeout regression coverage, while the production change remains small.

Merge-risk options

Maintainer options:

1. Restore injected-executor compatibility (recommended)
   Use the bounded subprocess path for the production executor while retaining the existing run-only fallback for supplied executors, then cover inspect and warmup.
2. Accept the contract break
   Deliberately require all injected executors to implement run_with_timeout and document the breaking interface change before merge.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Preserve run-only injected executor compatibility in inspect and warmup, and add focused regression coverage for both operations.

Technical review

Best possible solution:

Keep the production subprocess deadlines, but retain the run-only fallback for injected executors and add focused coverage for both lease operations.

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

Yes. The supplied real-child trace shows the bounded production paths returning promptly, and the fetched main source confirms the corresponding unbounded calls that make the hang reproducible.

Is this the best way to solve the issue?

No. Fixed deadlines are the right production remedy, but unconditionally requiring a new executor method is not the narrowest compatible implementation; preserve the established injected-executor fallback.

Full review comments:

• [P2] Preserve run-only injected executors — scripts/native_eval/fleet.py:711-715
FleetController still accepts a caller-supplied executor, but these unconditional calls require its new run_with_timeout method and raise AttributeError for the prior run-only contract. The existing stop path retains a SubprocessExecutor guard and fallback; the new test uses SubprocessExecutor and expands FakeExecutor, so it misses this regression. This is the unresolved finding from the earlier review of the same head.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

• P2: This is a bounded native-fleet reliability fix whose current compatibility regression blocks callers using the supported executor injection path.
• merge-risk: 🚨 compatibility: The PR changes the effective CommandExecutor contract and can break existing run-only injected executors at runtime.
• 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 changed production owner is FleetController's Crabbox inspect and warmup dispatch; the PR body supplies a terminal trace from those methods against a real long-lived child, showing both recover with FleetError in 0.40 seconds instead of waiting for 30 seconds. The trace does not exercise a live AWS coordinator, but it directly proves the subprocess-timeout behavior changed here.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is FleetController's Crabbox inspect and warmup dispatch; the PR body supplies a terminal trace from those methods against a real long-lived child, showing both recover with FleetError in 0.40 seconds instead of waiting for 30 seconds. The trace does not exercise a live AWS coordinator, but it directly proves the subprocess-timeout behavior changed here.

Evidence

Acceptance criteria:

• [P1] python -m pytest -q tests/test_native_eval_fleet.py.
• [P1] python -m ruff check scripts/native_eval/fleet.py tests/test_native_eval_fleet.py.

What I checked:

• Current main still needs this fix: The fetched main snapshot calls the injected executor's run method without a timeout in both warmup and inspection, so current main has not already implemented this PR's central behavior. (scripts/native_eval/fleet.py:698, c1a79f731541)
• Introduced compatibility regression: The PR unconditionally calls run_with_timeout for warmup and inspection, while FleetController continues to accept a caller-supplied executor. Run-only injected executors therefore fail before either lease operation can run. (scripts/native_eval/fleet.py:711, 9463ebb96ce6)
• Existing compatibility pattern: The existing stop path preserves injected executor compatibility by using run_with_timeout only for SubprocessExecutor and otherwise calling run; inspect and warmup do not retain that fallback. (scripts/native_eval/fleet.py:1321, 9463ebb96ce6)
• Regression test masks the contract break: The added timeout test constructs SubprocessExecutor, and the test fake was expanded with run_with_timeout, so neither exercises the previously supported run-only injected-executor path. (tests/test_native_eval_fleet.py:292, 9463ebb96ce6)
• Real behavior proof supplied: The PR body records a macOS run through the production fleet methods against a real long-lived child: both operations returned FleetError in 0.40 seconds rather than waiting for the child, while the unbounded comparison completed after about 30 seconds. (scripts/native_eval/fleet.py:97, 9463ebb96ce6)
• Feature-history routing: The merged native matrix runner that introduced the fleet surface is commit 69f75c6, associated with feat(eval): add auditable native matrix runner #42. (scripts/native_eval/fleet.py:315, 69f75c6629c4)

Likely related people:

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

• Preserve the run-only injected-executor path and add regression coverage for inspect and warmup.

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 (9 earlier review cycles; latest 8 shown)

• reviewed 2026-09-01T13:02:01.505Z sha 9463ebb :: needs changes before merge. :: [P2] Preserve run-only injected executors
• reviewed 2026-09-01T16:30:22.784Z sha 9463ebb :: needs changes before merge. :: [P2] Preserve run-only injected executors
• reviewed 2026-09-02T17:19:26.430Z sha 9463ebb :: blocked before merge. :: [P2] Preserve run-only injected executors
• reviewed 2026-09-03T08:34:37.885Z sha 9463ebb :: blocked before merge. :: [P2] Preserve run-only injected executors
• reviewed 2026-09-03T20:03:35.074Z sha 9463ebb :: blocked before merge. :: [P2] Preserve run-only injected executors
• reviewed 2026-09-03T21:25:18.609Z sha 9463ebb :: blocked before merge. :: [P2] Preserve run-only injected executors
• reviewed 2026-09-03T23:16:57.599Z sha 9463ebb :: blocked before merge. :: [P2] Preserve run-only injected executors
• reviewed 2026-09-04T07:56:24.720Z sha 9463ebb :: blocked before merge. :: [P2] Preserve run-only injected executors