fix(headless): stop reporting a cut-short run as a success - #778
Merged
Conversation
When the agent loop ends a run ITSELF, the model never finished the task. Headless reported `subtype: "success", is_error: false` anyway, so a batch runner recorded a clean completion that merely happened to score zero. That is why the tool-failure-loop guard's 31-second kills went unnoticed on terminal-bench until someone read the raw trajectories: the harness said the runs were fine. The eval data was not just missing a signal, it carried a wrong one. Now every terminal reason where the loop cut the run short maps to the reference's result subtypes (QueryEngine.ts:891 `error_max_turns`, :1142 `error_during_execution`), on both the `json` and `stream-json` outputs, with `is_error: true`: tool_failure_loop -> error_during_execution blocking_limit -> error_during_execution prompt_too_long -> error_during_execution image_error -> error_during_execution max_turns -> error_max_turns `blocking_limit` and `prompt_too_long` were the worst of the set: their explanation goes to `last_api_error_text`, which the compat layer only surfaces as response_text for `tool_failure_loop`, so those runs reported success with an EMPTY result — a clean completion carrying no evidence at all. `hook_stopped` and `stop_hook_prevented` are DELIBERATELY left as success: a hook refusing to continue is the operator's own policy working as configured, not the harness cutting a run short. Neither can fire without an installed hook that emits those signals. They are listed in the map's docstring and pinned by a test so the exclusion reads as a decision rather than an oversight. Like the reference, the event keeps its FULL metrics block — usage, num_turns, and the stop explanation as `result`. Eval adapters read token counts off it, so suppressing the event, or replacing it with a bare error, would have traded a silent-success bug for a missing-data bug. THE EXIT CODE DELIBERATELY STAYS 0 — a documented divergence from the reference, which exits `is_error ? 1 : 0` (print.ts:1069-1070). Not an ordering constraint: the emission gate is movable. The reason is what a non-zero code means downstream. Harbor raises NonZeroAgentExitCodeError on it, the trial records an exception.txt, and eval/harbor/compare_trajectories.py then treats that trial as "killed by harness" and EXCLUDES it from the step means. Flipping the code would right-censor every guard trip and every max_turns run straight out of the comparison — deleting precisely the trials this change exists to reveal, and reintroducing the denominator corruption that once inverted a whole iteration's metrics. The trade: a shell caller doing `clawcodex -p ...; echo $?` cannot detect an early stop and must read the result event instead. The `text` format gets a stderr line so it is not silent either. The stop reason is STICKY across a multi-prompt stream-json run. One terminal result event covers the whole run and every other field on it is cumulative, so an early stop on prompt N must not be erased by prompt N+1 completing normally — otherwise the explanation sits in `result` while `subtype` claims success, which is the original bug wearing a disguise. `EARLY_STOP_SUBTYPES` lives in query/transitions.py beside the TerminalReason literal it is keyed on, not in the entrypoint: more than one surface has to agree on it (the agent-server's turn outcome hardcodes success today and is the next fix), and importing an entrypoint for one dict would invert the layering. Also wires the adapter (eval/harbor/clawcodex_agent.py) to READ the subtype — it previously read only usage/cost, so without this the stream log would have become honest while the trial output stayed silent, which is the exact workflow that failed. It logs a warning and records `metadata["clawcodex_stop_subtype"]`, which lands in the trial's result.json. Deliberately not raised as an exception, for the censoring reason above. Verified: unit tests over both output formats and every mapped reason; a multi-prompt case asserting both prompts ran before checking the subtype; the subtype/is_error derivation extracted as a pure function and pinned as a cross-product table (the cancelled + early-stop collision is otherwise reachable only through a /goal continuation); and a REAL end-to-end guard trip against a local SSE server producing error_during_execution, is_error true, usage intact, and the explanation in `result`. All mutation-tested. Follow-ups deliberately not bundled: the "empty assistant turn -> re-prompting (3/3)" path also ends in Terminal(completed) and reports success, which needs a NEW terminal reason and touches a taxonomy the TUI and agent-server both read; and agent_server.py hardcodes `subtype="success"` regardless of terminal reason, so on the TUI path a guard-killed turn is fed to the /goal judge as a successful one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ericleepi314
added a commit
that referenced
this pull request
Aug 1, 2026
…779) #778 made HEADLESS report a non-success result subtype when the agent loop cut a run short. It named two gaps. This closes both. A. THE AGENT-SERVER TURN OUTCOME WAS HARDCODED TO SUCCESS. `_AgentSession._run_turn` emitted `subtype="success"` regardless of why the loop stopped, so on the TUI / VS Code path a guard-killed turn was indistinguishable from a completed one. Three consumers gate on that field: `_maybe_continue_goal` fed such a turn to the /goal judge as evidence of progress, `_maybe_review_memories` learned from it, and the cron loop rearmed on it. It now derives the subtype from `result.terminal.reason` through the SAME `EARLY_STOP_SUBTYPES` map headless uses, so the two surfaces cannot drift. The two remaining hardcoded successes in that function are UserPromptSubmit hook blocks, which never run the loop — left deliberately, consistent with excluding `hook_stopped` / `stop_hook_prevented` from the map (operator policy working as configured, not the harness cutting a run short). /goal AUTO-CONTINUES past an early stop rather than dying. Before the subtype was derived, such a turn arrived as "success" with "[Max tool turns reached]" as its text, was judged not-done, and the loop simply retried; bailing out would have silently killed loops that used to recover. So the judge is skipped — there is no output to judge, and asking a model whether "[Stopped: …]" satisfies the goal only invites a wrong answer — and a synthetic `continue` is applied instead. That is also `judge_goal`'s own fail-open verdict, so it is a path the loop already handles. Deliberately still routed through `apply_verdict` rather than enqueuing a continuation directly: that call is what ticks `turns_used` and lets the goal's cap decide. Short-circuiting it would let a turn that keeps stopping early retry forever. B. A DEGENERATE TURN FELL THROUGH TO `completed`. `query.py` nudges an assistant turn with no tool calls, no text and nothing in the outbox. Once the nudge budget was spent the WHOLE block was skipped — including the DETECTION — so a still-degenerate turn reached `Terminal(reason="completed")` and was reported as a clean success with an empty result. The last-text / outbox computation is hoisted into an `is_degenerate` flag and a new `Terminal(reason="empty_response")` fires when the budget is spent and it is still degenerate. `agent_loop_compat` surfaces an explanation as response_text, worded to stay true on BOTH routes in (budget spent, and max_turns blocking the retries) rather than claiming prompting that may not have happened. Two correctness fixes fell out of building it: * `spoke_via_outbox` read the SESSION-LIFETIME outbox, which is created once per session and never cleared — so one AskUserQuestion / Brief / SendUserMessage anywhere in a session permanently disabled the check for every prompt after it, and the silent empty success came straight back. Now scoped to the current query (a watermark taken before the loop) AND to SendUserMessage, the only tool whose entry `agent_loop_compat` promotes to response_text — an AskUserQuestion entry used to suppress the check while contributing nothing to the answer. * The empty-turn and continuation-signal arms shared one budget, so a few continuation nudges could spend it before the first empty turn arrived. The empty turn then got no retry at all and the run hard-failed without the one round trip that recovers it (measured at 3/89 terminal-bench trials). They now have separate counters and the empty-turn arm runs ahead of the shared gate. THE TAXONOMY. `TerminalReason` gains `empty_response`, which TS has no counterpart for — the arm that detects it is itself port-only (TS's continuation nudge gates on non-empty text, so it never sees the case). The parity test moves from set-EQUALITY against the TS snapshot to "TS is a SUBSET and every extra is declared in `PYTHON_ONLY_TERMINAL_REASONS`". That keeps both teeth that matter — a dropped TS reason still fails, an undeclared extra still fails — and a companion test requires every declared extra to be in `EARLY_STOP_SUBTYPES`, so the escape hatch cannot reintroduce a silent success. DOWNSTREAM. The VS Code chat pane matched `subtype === 'error'` exactly, so the new subtypes fell through to "Completed (N turns)"; it now checks `is_error` and falls back to `msg.result` when nothing streamed. The TUI already keyed on `is_error` but printed the explanation twice — once as the turn text, once as a red error line — because an early stop carries its explanation in `result` and sets no separate `error` field; it now emits a short "run stopped early (<subtype>)" instead of echoing. Verified end to end against a local SSE server returning a genuinely empty assistant turn: `error_during_execution`, `is_error: true`, num_turns 4 (1 + 3 retries), explanation in `result`. Every fix mutation-tested, including against the exact mutation that proved an earlier version of the agent-server test was a placebo (compute the subtype, then discard it). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
When the agent loop ends a run itself, the model never finished the task. Headless reported
subtype: "success", is_error: falseanyway — so a batch runner recorded a clean completion that merely happened to score zero.That is why the tool-failure-loop guard's 31-second kills (fixed in #777) went unnoticed on terminal-bench until someone read the raw trajectories: the harness said the runs were fine. The eval data wasn't just missing a signal, it carried a wrong one.
What changes
Every terminal reason where the loop cut the run short now maps to the reference's result subtypes (
QueryEngine.ts:891error_max_turns,:1142error_during_execution), on bothjsonandstream-json, withis_error: true:tool_failure_looperror_during_executionblocking_limiterror_during_executionprompt_too_longerror_during_executionimage_errorerror_during_executionmax_turnserror_max_turnsblocking_limitandprompt_too_longwere the worst of the set: their explanation goes tolast_api_error_text, which the compat layer only surfaces fortool_failure_loop— so those runs reported success with an empty result. A clean completion carrying no evidence at all.hook_stopped/stop_hook_preventedare deliberately left as success: a hook refusing to continue is the operator's own policy working as configured, not the harness cutting a run short. Neither can fire without an installed hook emitting those signals. Both are named in the map's docstring and pinned by a test, so the exclusion reads as a decision rather than an oversight.Like the reference, the event keeps its full metrics block — usage, num_turns, and the stop explanation as
result. Eval adapters read token counts off it, so suppressing the event would have traded a silent-success bug for a missing-data bug.A documented divergence from the reference, which exits
is_error ? 1 : 0(print.ts:1069-1070).Not an ordering constraint — the emission gate is movable. The reason is what a non-zero code means downstream: Harbor raises
NonZeroAgentExitCodeError, the trial records anexception.txt, andeval/harbor/compare_trajectories.pythen treats that trial as "killed by harness" and excludes it from the step means.Flipping the code would right-censor every guard trip and every max_turns run straight out of the comparison — deleting precisely the trials this change exists to reveal, and reintroducing the denominator corruption that once inverted a whole iteration's metrics.
The trade:
clawcodex -p ...; echo $?cannot detect an early stop and must read the result event instead. Thetextformat gets a stderr line so it isn't silent either.Sticky across multi-prompt runs
One terminal result event covers a whole
stream-jsonrun and every other field on it is cumulative, so an early stop on prompt N must not be erased by prompt N+1 completing normally — otherwise the explanation sits inresultwhilesubtypeclaims success. The original bug wearing a disguise.Placement
EARLY_STOP_SUBTYPESlives inquery/transitions.pybeside theTerminalReasonliteral it's keyed on, not in the entrypoint — more than one surface has to agree on it, and importing an entrypoint for one dict would invert the layering.The adapter now reads it
eval/harbor/clawcodex_agent.pypreviously read only usage/cost, so without this the stream log would have become honest while the trial output stayed silent — the exact workflow that failed. It now logs a warning and recordsmetadata["clawcodex_stop_subtype"], which lands in the trial'sresult.json. Deliberately not raised as an exception, for the censoring reason above.Verification
is_errorderivation extracted as a pure function and pinned as a cross-product table — thecancelled+ early-stop collision is otherwise reachable only through a/goalcontinuation.error_during_execution,is_error: true, usage intact, explanation inresult; and the text format printing[clawcodex] run stopped early (tool_failure_loop).Reviewed by the
criticsubagent across two rounds. It caught the multi-prompt erasure and the five unmapped terminals with runtime probes, and corrected my stated rationale for the exit code — my reason was a self-imposed ordering artifact; the real one (censoring) is much stronger and is now recorded in the code so nobody "finishes the job" and silently drops the data.Follow-ups, deliberately not bundled
empty assistant turn -> re-prompting (3/3)path also ends inTerminal(completed)and reports success. Needs a new terminal reason, touching a taxonomy the TUI and agent-server both read.agent_server.pyhardcodessubtype="success"regardless of terminal reason, so on the TUI path a guard-killed turn is fed to the/goaljudge as a successful one.🤖 Generated with Claude Code