Report a failed turn as an error, not as the answer - #14
Merged
Conversation
added 4 commits
July 29, 2026 17:22
All three agents report some failures with exit code 0, putting the
explanation where the answer belongs. Asking Claude for a model that does
not exist returns, verbatim:
type "result", subtype "success", is_error true, api_error_status 404,
result "There's an issue with the selected model (bogus-model-xyz)."
Note `subtype` says success while `is_error` is true, so `is_error` is the
field that decides. A caller checking `Result::is_ok` rendered that
sentence as the model's reply.
This is the same shape auth and quota failures already had, and both were
already lifted out of `Ok` for the same reason. `Error::AgentError` covers
the rest of the family, carrying the agent's own wording and the provider's
status where one was reported. It is classified after auth, quota and a
rejected flag so those more specific readings still win.
Codex needs one more step: it exits 0 too, but forwards the upstream
response body as a JSON *string*, so `turn.failed` carries
`{"status":400,"error":{"message":"..."}}` encoded as text. Left alone the
caller is handed JSON instead of a sentence, with the status buried inside
it. It is unwrapped to the sentence plus the code; anything not in that
shape passes through untouched. Copilot reports no explanation with a
non-zero `exitCode`, so the code itself is named rather than leaving the
failure blank.
Both paths are covered by live tests against the installed CLIs, which is
how the Codex double-encoding turned up: the unit test built from Claude's
shape passed while the real Codex message was a JSON blob.
It credits 0.2.0 with streaming by default and token-by-token Claude text. Those shipped in 0.2.1: the commit is timestamped three hours after the 0.2.0 release commit, and `git show v0.2.0:src/agent.rs` has no `--include-partial-messages`. Anyone reading the entry against the published 0.2.0 would find the feature missing. Corrected in place with a note rather than deleted, since 0.2.0 is on crates.io and a silent edit gives no clue to someone who already read it. The bullets stay in the 0.2.1 entry, where they belong.
Every claude `stream-json` run prints a `rate_limit_event`, and on a healthy run it says `"status":"allowed"` and refuses nothing. Quota classification scanned the raw stream for the substring `rate_limit`, which that record contains by construction, so any Claude failure was reported as `Error::RateLimited`. A caller was told to wait out a limit that was not hit, while the actual cause never reached them. Prose heuristics applied to structured JSON match field names. Where the stream parsed, the parsed signal and the agent's own wording decide; the raw scan stays as the fallback for output that produced neither, which is what it was written for. The error message follows the same rule, so a genuine refusal quotes the provider rather than a JSON line. Found by the new unknown-model live test, which failed only in the full suite: the heartbeat is not emitted on every single run, so running the test alone passed. Both directions are now covered by unit tests built from the verbatim record.
The operating-limits note says an unknown model is the provider's error to raise. Still true, but it now has a variant rather than arriving as a plausible-looking answer, so name it.
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
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.
The problem
Ask Claude for a model that does not exist and it exits 0. Verbatim from the run:
{"type":"result","subtype":"success","is_error":true,"api_error_status":404, "result":"There's an issue with the selected model (bogus-model-xyz). It may not exist or you may not have access to it."}subtypesayssuccess.is_erroris the field that decides. Before this, that came back asOk(Outcome)with the sentence intext, so a GUI checkingResult::is_ok()rendered "There's an issue with the selected model" as the model's reply.This is the same shape as auth and quota failures, both of which were already lifted out of
Okfor exactly this reason.Error::AgentErrorcovers the rest of that family.What is here
Error::AgentError { agent, bin, status, message }— the agent's own wording plus the provider's status where one was reported. Classified after auth, quota and a rejected flag, so those more specific readings still win.Codex needed a second step. It exits 0 too, but forwards the upstream response body as a JSON string:
{"type":"turn.failed","error":{"message":"{\"type\":\"error\",\"status\":400,\"error\":{\"message\":\"The 'bogus-model-xyz' model is not supported when using Codex with a ChatGPT account.\"}}"}}Left alone the caller gets JSON instead of a sentence, with the status buried inside it. It is unwrapped to the sentence plus the code; anything not in that shape passes through untouched. Copilot reports no explanation with a non-zero
exitCode, so the code is named rather than leaving the failure blank.A bug this turned up
The new live test passed alone and failed in the full suite. The reason was worth having:
Every Claude
stream-jsonrun prints arate_limit_event, and on a healthy run it says"status":"allowed"and refuses nothing. Quota classification scanned the raw stream for the substringrate_limit, which that record contains by construction. So any Claude failure was reported asError::RateLimited: the caller was told to wait out a limit that was never hit, and the actual cause never reached them. The record is not emitted on every single run, which is why running the test alone passed.Prose heuristics applied to structured JSON match field names. Where the stream parsed, the parsed signal and the agent's own wording now decide; the raw scan stays as the fallback for output that produced neither, which is what it was written for.
Also
The
0.2.0changelog entry credited that release with streaming by default and token-by-token Claude text. The commit is timestamped three hours after the 0.2.0 release commit, and the tree of52db91c("Release 0.2.0") has no--include-partial-messages.The behaviour change folds into the unpublished
0.2.1entry rather than taking a new version.Verification
fmt,clippy --all-targets,cargo doc,cargo package: cleanEvery payload in the new tests is copied from a real run, not constructed.