Circuit-break an engine after its first spawn failure#36
Conversation
When an engine binary is missing, the wrapper exits 127 (or 126) without ever invoking a model — but every such attempt wrote a FAIL row to the per-model log and then spent the task's retry on a failure a retry cannot fix. In the field this poisoned a scoreboard with 36 rows in one run, dragging an innocent model's pass rate from 1.00 to 0.40, and the operator had to hand-edit runs.jsonl to correct the record. The fix draws one line: no model evidence, no model row. - worker_never_ran() names the two signatures: exit 126/127 with no token usage (wrapper/shell command-not-found), or an error before exec with no exit code. Token usage always wins — a late 127 from a wrapper's cleanup after a real model run still counts as model evidence. - verdict_for() returns INFRA for these attempts, checked before verify.ok so a stale artifact passing the check cannot credit a model that never ran. - _log_attempt() skips the model-log row (and the steering observation) for such attempts, writing a breadcrumb to the worker log instead. This single choke point also covers _record_prepare_error, whose rows carried a model name for a model that never ran. - INFRA is not in the retry set, so the retry is skipped, with a log line saying why and what to fix; the run summary counts INFRA tasks and points at the engine bin/PATH config. Tests: unit coverage for every branch of the classification (including the rc=1-no-tokens case that must KEEP reaching the log, and timeout staying model evidence), plus an end-to-end run against a wrapper that exits 127 — asserting verdict INFRA, attempts == 1, an empty model log, and the summary hint. The pre-existing test_design_reference/test_scoreboard_page failures on main are untouched by this change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The startup preflight (preflight_engine_bins) cannot catch every spawn failure: wrapper-script engines resolve and execute fine, then exit 127 because the binary INSIDE the wrapper is missing. In the field a 16-task run burned 32 doomed spawns that way — every task, both attempts — before the summary printed. A missing binary fails identically for every task on that engine, so the first INFRA verdict now trips a per-engine circuit breaker: remaining tasks on that engine fail fast with verdict INFRA and zero attempts — no worker spawned, no taskdir prepared, no model-log row — and a worker-log breadcrumb names the engine and the fix. One broken engine costs one attempt, not tasks x attempts. Builds on the INFRA classification from the previous commit; the summary hint there counts breaker-skipped tasks automatically. Test: end-to-end run with three tasks on a wrapper engine that exits 127 — asserts the first task shows INFRA/1 attempt, the queued two show INFRA/0 attempts with breaker breadcrumbs and no 'attempt 1 exited' line, and the model log stays empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review verdict for #35 + #36 together (since #36 contains #35): the evidence rule — spawn failures are infrastructure evidence, not model evidence — is exactly right and we want it in main. But @jeffhamons' #33 builds a second run-scoped circuit breaker (API-terminal errors: 402/balance exhausted/auth expired) in the same two functions, and merging both as written leaves two parallel mechanisms with contradictory logging conventions. Full maintainer constraints are posted on #33 — short version: one mechanism, N detectors; log everything credit nothing (visible @mlava @jeffhamons — would you two put your heads together and produce the canonical version? Co-authored PR or a clearly layered stack, whatever suits you; authorship is preserved either way. You attacked the same real problem from opposite ends and the union of your two detectors on one breaker is the right architecture. Happy to review a design sketch in either thread before you build. (#37 stays separate — review coming on its own thread.) |
|
In — happy to build the canonical version with @jeffhamons. Sketch below against the five constraints; Jeff, correct anything I've got wrong about #33's internals and let's split the work. One breaker, N detectors. A single run-scoped engine-health breaker on the runner, per-engine, taken under the runner lock (#36's skeleton, generalized). Detectors classify a finished attempt and return a typed reason; the breaker only ever learns "engine X is down because Y".
Log everything, credit nothing. This reverses #35's "no model evidence, no model row": every attempt — including breaker-skipped tasks — writes a visible eval row with its verdict; aggregation excludes Display contract. Admission. Single-probe: while an engine's run-health is unknown, the first spawn is the probe; queued same-engine tasks hold until it either shows life (successful spawn / token evidence) or trips the breaker — killing the race where a whole wave spawns before the first verdict lands. In-flight tasks. v1 claims only "no new spawns, no retries": workers already running finish and get classified by the detectors on exit. Engine-aware cancellation can layer on later without changing the row/verdict contract (the #33 point-5 choice). Packaging. Mild preference for one co-authored PR superseding #33/#35/#36 — the mechanism is genuinely one thing, and #35's row-suppression rule dies in favor of visible-but-unranked rows, so the stack would need rebuilding from the bottom anyway. But a layered stack works if you'd rather, Jeff. If the maintainers are happy with this shape we'll build against it. |
2 participants
Stacked on #35 — review the last commit only (
f18e906); the first commit is #35's INFRA classification, which this builds on.Problem
preflight_engine_binscannot catch every spawn failure by construction: wrapper-script engines (e.g.engines/opencode-sandboxed.sh) resolve and execute fine, then exit 127 because the binary inside the wrapper is missing. In the field (2026-07-14), a 16-task run burned 32 doomed spawns — every task, both attempts — before the summary printed.Fix
A missing binary fails identically for every task on that engine, so the first INFRA verdict trips a per-engine circuit breaker:
INFRA, zero attempts, no worker spawned, no taskdir prepared, no model-log rowbin/PATH in config)One broken engine costs one attempt, not tasks × attempts. Engines unaffected by the failure keep running normally — the breaker is per-engine, taken under the runner lock.
Test
End-to-end (
tests/test_infra_circuit_breaker.py): three tasks on a wrapper engine that exits 127,max_parallel: 1for determinism. Assertsspawn-1 fail INFRA 1,spawn-2/3 fail INFRA 0with breaker breadcrumbs and noattempt 1 exitedline in their logs, and an empty model log. Full suite: no new failures (the pre-existingtest_design_reference/test_scoreboard_pagefailures on main reproduce with this change stashed).🤖 Generated with Claude Code