fix: four harness bugs found by the tb2.1 fusion-vs-opus differential - #789
Merged
Conversation
Two independent defects in the pre-spawn safety guard, both found by a terminal-bench 2.1 differential (2026-08-02). `\breboot\b` matches inside `-no-reboot` — a standard QEMU flag — because `-` is a non-word character. That refused `qemu-system-x86_64 … -no-reboot` as "potentially dangerous": 7 blocked Bash calls across qemu-startup and qemu-alpine-ssh, both of which then scored 0 (the opus baseline solved qemu-alpine-ssh). Replaced the `\b` boundaries on the command-NAME patterns with `(?<![-\w])X(?![-\w])`, which still matches a bare `reboot`, `/sbin/reboot` and `reboot -f` while never matching a hyphenated neighbour. Strictly more precise; nothing new gets through. Separately the blocklist ran unconditionally, so it refused `sudo` and a real `reboot` even under --dangerously-skip-permissions. Real Claude Code has no un-bypassable command blocklist; this list is a port addition. Same parity direction as #673. The guard now skips the list in a deliberate bypass posture and stays fully armed in default/acceptEdits/plan. The bypass predicate requires `is_bypass_permissions_mode_available` as well as the mode, and that half is load-bearing: `ToolContext` DEFAULTS to `ToolPermissionContext(mode="bypassPermissions")` (the inversion mcp_serve.py already documents), so keying on mode alone would disarm the blocklist for every default-constructed context. Only a session that actually resolved the posture sets both. Fails closed otherwise. The C8 sandbox hard-gate is deliberately NOT bypassed — that one comes from managed settings, and a CLI flag must not override an admin control. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Anthropic wire has had an idle watchdog since WI-5.2. The OpenAI-compatible consumers had no elapsed-time bound of any kind: their poll loops checked `guard.aborted` and nothing else, so a connection that was accepted and then produced nothing hung until something above killed the process. On terminal-bench 2.1 (2026-08-02) model-extraction-relu-logits sat through 880 SECONDS of total silence after a single tool call, then died at harbor's 900 s ceiling having written 2 KB of log. The httpx `read` timeout does not cover this. `read` bounds the gap between BYTES, and a stalled provider is usually still sending — keepalive comments, empty delta frames — so it re-arms indefinitely. That 880 s stall exceeding 120 s x (1 + 1 retry) is the proof it never fired. Byte liveness cannot answer "is this stream still producing anything?". Adds `ContentProgressDeadline`, the semantic sibling of `StreamWatchdog`: progress is recorded for content, reasoning and tool-call deltas and the terminal finish_reason — deliberately NOT for bare frame arrival, since a provider emitting empty frames forever is the failure being caught. Wired into both consumers that had the gap (Chat Completions and the ChatGPT subscription Responses loop), driven from their existing poll ticks, so no timer thread is introduced. Checked on every loop iteration rather than only when the chunk queue is empty. Gating it on Empty looks natural and is wrong for exactly the case this exists for: a stalled-but-chatty stream keeps the queue non-empty, so the check never runs. Both new tests hang against that variant. Threshold is the FIRST-EVENT grace (300 s), not the 90 s inter-event idle, and stays flat instead of tightening after the first delta. Prompt processing on a large context legitimately runs minutes before the first token, and a model doing hidden internal reasoning can legitimately go quiet mid-response; no healthy provider goes five minutes between deltas. Erring long costs one stalled request, erring short would truncate healthy generations on slow providers. A test pins that a slow but productive stream is never interrupted. Recovery is one re-issue through the existing retry wrapper, reusing its `emitted` guard so a stream that already reached the caller is never replayed. StreamIdleTimeout stays a plain Exception and stays out of `is_transport_error`, so the two retry budgets still refuse to compose — see the note in services/api/errors.py. Reuses CLAUDE_STREAM_FIRST_EVENT_TIMEOUT_MS / CLAUDE_STREAM_IDLE_TIMEOUT_MS; no new env surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`_describe` raised on an empty 200 and `_substitute` cached that as a permanent `_Failure`, so one transient empty completion lost the image for the rest of the session. Observed on terminal-bench gcode-to-text (2026-08-02): `openai:gpt-5.6-luna` returned no text for image 2 of 5 and the task scored 0 against a baseline that solved it. The empty case now gets exactly one retry. Nothing else does, and that asymmetry is the point. A transport failure means the provider is unreachable, and caching it permanently is deliberate — the docstring's arithmetic (8 images x 60 s x 2 attempts on every turn, forever, uninterruptible) is why it exists. An empty completion is the opposite situation: the provider is up, answered fast, and just produced nothing. Scoping the retry to it keeps the outage cost at one call per image. Typed as `_EmptyVisionResponse` rather than matched on the message, and the retry is charged to the request's call budget so a provider stuck returning empty 200s cannot double the fan-out that cap bounds. A retry is also skipped outright once the budget's call/time limit is reached. Mutants covered by the new tests: no retry, retrying every failure, and a retry that does not charge the budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ResultEvent` carries the authoritative usage but only exists if the run REACHES the end. A run that is killed — an eval harness hitting its per-task ceiling, a SIGKILL, a dropped connection — emitted nothing, so everything it had spent became unmeasurable. On terminal-bench 2.1 (2026-08-02) every trial missing token metrics was a killed one: 21 of 46 in the fusion job, 17 of 89 in the opus job. Those are the LONGEST and most expensive trials, since length is what makes them time out. Harbor sums job totals from per-trial values, so the headline cost was a floor biased low by exactly the trials that cost the most, and the two jobs' timeout rates differ 2.4x — which makes any cost comparison between them meaningless. Adds a `usage` stream-json event carrying cumulative totals, emitted per assistant message (one model round trip). That granularity is the point: a turn killed before it completes never reaches the per-turn accounting, but it has already produced assistant messages. Verified live — the first usage event lands before the first tool_use, which is exactly where model-extraction-relu-logits died. Accumulated in its own `live_usage` dict rather than the existing `usage_total`: that one folds `result.usage` once per completed turn, and feeding both from either source would double-count. The ResultEvent is untouched. Additive by design — a new event `type`, so consumers that switch on the types they know are unaffected, and `ResultEvent.usage` keeps its exact meaning. The harbor adapter gains it as a third and last-resort lane behind session totals and the result event, never as a replacement. The adapter's arithmetic is now shared through `_usage_columns` so the two lanes cannot drift on counting cached tokens — the bug #786 fixed. Adapter tests skip in the repo's 3.11 venv (the adapter needs 3.12+ `typing.override` and harbor); verified under `uv run --python 3.13 --with harbor`, 7 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Test Results 1 files 1 suites 7m 39s ⏱️ Results for commit 61b1eae. |
ericleepi314
added a commit
that referenced
this pull request
Aug 3, 2026
A critic pass on #789 returned REVISE with four defects and three coverage holes; the re-review of the fixes returned APPROVE with two residual gaps, closed here as well. Every finding verified by mutation or direct probe. Two defects where #789 asserted something untrue: * UsageEvent.num_turns was structurally always 0 — the headless turn counter only advances after the agent loop returns, and the harbor adapter promoted that zero into killed trials' metrics, so the trial that did the most work reported num_turns: 0. The same floor-biased-low-on-the-longest-trials defect #789 set out to remove, relocated from tokens to turns. Replaced with assistant_messages, deliberately NOT mapped back onto num_turns. * "Cached permanently for the rest of the session" was false — _FAILURE_TTL_SECONDS = 90.0 and entries expire. The retry is still right; the reason given for it was not. ContentProgressDeadline's docstring documented the REJECTED design (`except Empty: check()`) and asserted a false safety property — mutating to it hangs. Rewritten as a warning rather than a description. The new bound now has a real escape hatch: CLAWCODEX_CONTENT_PROGRESS_TIMEOUT_MS (0 disables, malformed fails closed), independent of the Anthropic knob it previously shared, named in the error message and documented in the README. The 300s default is measured, not asserted: 727 completed trials across 44 harbor jobs, median 14.2 s/turn, p99 100.3, max 183.8, zero above 300 — on a metric that overstates model latency. What the corpus cannot rule out is stated too. Five previously-surviving mutants now fail, including deleting the usage emission entirely, the live_usage/usage_total double-count, and the tool-call progress signal (a model streaming a large Write emits argument deltas and nothing else, so the biggest tool calls would be the ones killed and replayed). Nothing called run_headless before. Tests that regressed by HANGING now fail in 16s instead. New Harbor adapter (3.13) CI job runs the guards the 3.11 job cannot import — the num_turns regression could previously return with CI green. Suite: 9641 passed, 9 skipped, 0 failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Diagnosing
tb21-fusion-flash-lunaagainsttb21-clawcodex-3surfaced fourdefects in clawcodex itself. On the 41 shared scored tasks the fusion run
scores 0.585 against opus-5's 0.829; roughly 40% of that gap is
harness, not model. Selection bias is ruled out — opus scores 0.829 on the
shared 41 and 0.826 on the 46 the fusion run never reached.
Each fix is general. None of them trades another model's behaviour for this
run's score, and two carry an explicit regression test for that.
The four
1. No elapsed-time bound on the OpenAI-compatible wires (
4dbc78b6)model-extraction-relu-logitssat through 880 seconds of total silenceafter one tool call, then died at harbor's 900 s ceiling with a 2 KB log. The
Anthropic wire has had an idle watchdog since WI-5.2; these consumers checked
guard.abortedand nothing else.The httpx
readtimeout does not cover it — that bounds the gap betweenBYTES, and a stalled provider is usually still sending keepalives, so it
re-arms indefinitely. Adds
ContentProgressDeadline, which measures semanticdeltas instead. Threshold is the 300 s first-event grace, flat, so prompt
processing and hidden reasoning are never cut short; a test pins that a slow
but productive stream is untouched.
The subtle part: the check runs on every loop iteration, not just when the
chunk queue is empty. Gating it on Empty looks natural and is wrong for exactly
the case it exists for — a stalled-but-chatty stream keeps the queue non-empty.
Both new tests hang against that variant.
2.
-no-rebootrefused as a dangerous command (f8442768)\breboot\bmatches inside the standard QEMU flag, because-is a non-wordcharacter. 7 blocked Bash calls across
qemu-startupandqemu-alpine-ssh,both of which then scored 0 — opus solved
qemu-alpine-ssh. Now(?<![-\w])X(?![-\w]), which still matches barereboot,/sbin/rebootandreboot -f.Separately the blocklist ignored
--dangerously-skip-permissionsentirely.Real Claude Code has no un-bypassable command blocklist; same parity direction
as #673. The bypass predicate requires
is_bypass_permissions_mode_availableas well as the mode — load-bearing, because
ToolContextdefaults tomode="bypassPermissions", so keying on mode alone would disarm the blocklistfor every default-constructed context. The C8 sandbox hard-gate is deliberately
still not bypassable: it comes from managed settings.
3. Killed trials reported no tokens and no cost (
a421fb2e)Every trial missing metrics was a killed one — 21 of 46 in the fusion job, 17
of 89 in the opus job — i.e. the longest and most expensive. Job totals are
summed per-trial, so both headline costs are floors biased low by exactly the
trials that cost the most, and the jobs' timeout rates differ 2.4x.
Adds a cumulative
usagestream-json event per assistant message. Verifiedlive: the first one lands before the first tool_use, which is precisely
where
model-extraction-relu-logitsdied. Additive (new event type,ResultEventuntouched), and the adapter takes it as a last resort behindsession totals and the result event.
4. Fusion vision empty response cached as permanent failure (
f5322272)gcode-to-text: the vision leg returned no text for image 2 of 5, thatcached as a
_Failure, and the task scored 0 against a baseline that solvedit. The empty-200 case now gets one retry; transport failures keep the old
single-attempt + negative-cache behaviour, which is what keeps a real outage
from costing 2x the calls every turn forever. The retry is charged to the
request's call budget.
Verification
(deadline removed; deadline gated on Empty) make the tests hang, which is
the production bug reproduced.
uv run --python 3.13 --with harbor(7 passed); they skip in the repo's venv, which lacks harbor.
openssl-selfsigned-certas an untouched control.🤖 Generated with Claude Code