Skip to content

Fail fast on terminal engine-down errors (402/balance exhausted/auth-expired)#33

Open
jeffhamons wants to merge 1 commit into
NateBJones-Projects:mainfrom
jeffhamons:claude/beautiful-noether-85a577
Open

Fail fast on terminal engine-down errors (402/balance exhausted/auth-expired)#33
jeffhamons wants to merge 1 commit into
NateBJones-Projects:mainfrom
jeffhamons:claude/beautiful-noether-85a577

Conversation

@jeffhamons

Copy link
Copy Markdown

Summary

Fixes an incident (2026-07-12) where a Grok Build engine returning 402 Payment Required: usage balance exhausted caused every task in a 5-lane run to burn both retry attempts (~5s each) before failing — 10 wasted worker invocations logged to the scoreboard as ordinary model failures.

  • detect_engine_down_reason() classifies worker stdout/stderr (tail-scoped to the last ~2000 chars, where a terminal harness error actually lands) for billing/auth patterns: HTTP 402 (plain text and JSON status/code fields), "payment required", "balance exhausted", insufficient credits/quota, expired auth/token/session/API-key, invalid API key.
  • RingerRunner tracks which engines are down for the run. The first task to hit a terminal error marks its engine down; every other task on that engine — already running or still queued — fails fast with a distinct engine-down status / ENGINE_DOWN verdict instead of burning a second attempt or even preparing a taskdir/worktree.
  • aggregate_model_log_rows / aggregate_model_scoreboard_rows now exclude ENGINE_DOWN-verdict tasks entirely (same pattern as the existing reserved-fixture guard), so a billing outage no longer drags down a model's pass_rate/first_try_pass_rate.
  • engines/mock_worker.py gains a MOCK_ENGINE_DOWN directive for deterministic testing.

Design notes for reviewers

  • Invariant deviation (intentional): the engine-down path skips running the check command entirely — there's no artifact to verify from a billing/auth failure, and PASS is never claimed, so "verification executes the artifact" still holds in spirit. Stdin-closed, explicit sandbox mode, and "logs carry raw worker output" are unaffected; the new [ringer.py]-prefixed log lines follow the same convention as the existing attempt-lifecycle lines.
  • False-positive tradeoff: content-pattern detection on worker stdout is inherently fuzzy. A coding task that legitimately implements 402/auth handling could in principle trip a pattern in its own narration. Tail-scoping the scan to the last ~2000 chars (vs. the full ~1MB capture) cuts most of that risk, since terminal engine errors are the last thing printed before the process dies — but it isn't zero-risk and is worth watching on real runs.
  • Left docs/MODEL-NOTES.md and engines/opencode-sandboxed.sh untouched (other owners' files per repo convention).

Test plan

  • New test suite tests/test_engine_down.py: pattern-matching unit tests (positive/negative cases including the exact incident string), an aggregation-exclusion unit test, and a full subprocess end-to-end test (3 tasks, max_parallel=1) proving task-one burns exactly 1 attempt and task-two/three never even get a taskdir.
  • Full existing suite run: 185 passed, 3 pre-existing failures unrelated to this change (hardcoded-date HTML assertions expecting "July 6, 2026" against the current date) — confirmed via git stash that they fail identically on the unmodified baseline.
  • ruff check on changed files: no new findings.

A Grok Build 402 "usage balance exhausted" incident caused every task on
that engine to burn both retry attempts (~5s each) before failing, and all
of it landed in the scoreboard as ordinary model failures.

- detect_engine_down_reason() classifies worker output (tail-scoped, ~2000
  chars) for billing/auth terminal patterns: HTTP 402, "payment required",
  "balance exhausted", insufficient credits/quota, expired auth/token/
  session/api-key, invalid api key.
- RingerRunner tracks which engines are down for the run; once one task
  reveals it, every other task on that engine (running or queued) fails
  fast with status "engine-down" / verdict ENGINE_DOWN instead of burning
  a second attempt or a fresh taskdir.
- aggregate_model_log_rows / aggregate_model_scoreboard_rows now exclude
  ENGINE_DOWN-verdict tasks entirely, so a billing outage no longer drags
  down a model's pass_rate/first_try_pass_rate.
- engines/mock_worker.py gains a MOCK_ENGINE_DOWN directive for testing.

Deviation from the "verification executes the artifact" invariant: the
engine-down path skips running the check entirely (nothing to verify from
a billing/auth failure, and PASS is never claimed). Stdin-closed, explicit
sandbox mode, and "logs carry raw worker output" are unaffected — the new
[ringer.py]-prefixed log lines follow the same convention as existing
attempt-lifecycle lines.

🤖 Generated by JeffOS
@justfinethanku

Copy link
Copy Markdown
Contributor

Review verdict: we want this capability, and we want it once. @mlava's #35/#36 solve the sibling problem — spawn-level failures (missing binary, exit 126/127) — with a second run-scoped circuit breaker living in the same two functions as yours. Merging both as written gives the codebase two parallel engine-health mechanisms with contradictory logging conventions, so instead of us picking a winner:

@jeffhamons @mlava — would you two put your heads together and design the canonical mechanism? One PR (co-authored, or stacked with clear layering — your call; authorship is preserved either way). Constraints from the maintainers, which are firm:

  1. One run-scoped engine-health mechanism, N detectors. API-terminal errors (402/balance/auth — this PR) and spawn-level failures (Spawn failures are infrastructure evidence, not model evidence #35/Circuit-break an engine after its first spawn failure #36) are two detectors feeding one breaker, not two breakers.
  2. Log everything, credit nothing. Skipped/infra attempts write visible eval rows with distinct verdicts (ENGINE_DOWN, INFRA) that scoreboard aggregation excludes from model stats — same doctrine as the unattributed-rows quarantine. No silent suppression of rows.
  3. The display contract must learn the new states. task_state_bucket(), the run summary, final artifact pages, and Ringside must render engine-down/infra runs truthfully — a dead-engine run that displays as "passed" or "waiting" is a wrong display, which is the one bug class this project treats as unforgivable.
  4. False positives are worse than false negatives. A worker whose output text legitimately discusses billing errors must not kill its engine. Narrow the matchers (and cover the plain-text HTTP 402 case the current regex misses); prefer structured signals where they exist.
  5. Per-PR feedback still applies: single-probe admission so concurrent queued tasks can't all race past the breaker check (Circuit-break an engine after its first spawn failure #36), and either implement engine-aware cancellation of in-flight tasks or claim only "no retries" (Fail fast on terminal engine-down errors (402/balance exhausted/auth-expired) #33).

Fail-fast-on-dead-engine is a real waste-killer and we'd like it in main soon — happy to review a joint design sketch in either thread before you build. Thanks to you both for attacking the same real problem from opposite ends.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants