From 085e905626e78581556e498886753bf0f1827f04 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 29 Jul 2026 17:22:47 +0700 Subject: [PATCH 1/4] Report a failed turn as an error, not as the answer 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. --- CHANGELOG.md | 23 +++++------ README.md | 33 ++++++++++++++++ src/error.rs | 25 ++++++++++++ src/event.rs | 92 +++++++++++++++++++++++++++++++++++++++++++- src/run.rs | 104 +++++++++++++++++++++++++++++++++++++++++++++++--- tests/live.rs | 62 ++++++++++++++++++++++++++++++ 6 files changed, 319 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c858c43..a90118e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,15 @@ already rendering events, check the first item. - `run()` parses a JSONL stream rather than one document. The `Outcome` is unchanged. Pin `Format::Json` to keep the old behaviour. +- **A turn the agent says failed is now an error, not an answer.** All three exit `0` for + some failures and put the explanation where the answer belongs: ask Claude for a model + that does not exist and it exits cleanly, reports `subtype: "success"`, and returns + "There's an issue with the selected model" as its reply. A caller checking `Result::is_ok` + rendered that as the model's answer. These are now `Error::AgentError`, carrying the + agent's own wording and the provider's status where one was given (typically 404 for an + unknown model). `NotAuthenticated` and `RateLimited` keep their own variants, since the + remedy differs; this covers the rest of that family. Codex forwards the upstream body as a + JSON *string*, so its message is unwrapped to the sentence and the status pulled out of it. - **An unauthenticated Copilot run is now reported as one.** Its wording shares no phrase with Claude's or Codex's, so it previously fell through to a generic `Error::Failed` with no login hint. Callers matching on `Error` should expect `NotAuthenticated` from Copilot now. @@ -60,23 +69,11 @@ already rendering events, check the first item. ## 0.2.0 Nothing was removed or renamed, so this compiles against any 0.1 code. It is a minor bump -rather than a patch because two changes alter behaviour, and `"0.1"` would have delivered +rather than a patch because some changes alter behaviour, and `"0.1"` would have delivered them through an ordinary `cargo update`. ### Behaviour changes -- **Streaming is now the default**, and a named session no longer disables it. `Format` - defaulted to `Json`, under which nothing is observable until the turn ends, so a caller - watching a twenty-minute run saw nothing for twenty minutes. Worse, `Request::session` - *pinned* that format, meaning the multi-turn path a chat UI always uses could not stream - at all. `Stream` carries everything `Json` does, so the default costs only parsing, and - `session()` now validates the format instead of overriding it. Pin `Format::Json` - explicitly if you only want the answer. -- **Claude streams token by token.** `--include-partial-messages` is passed under `Stream`, - without which Claude emits only completed messages and text arrives a paragraph at a time. - Claude sends both the deltas and the finished message; only the deltas are emitted, so a - transcript does not show every answer twice. - - **An unauthenticated run is now an error.** Claude exits `0` and puts "Not logged in · Please run /login" in its result text, so a missing login previously came back as a successful `Outcome` whose answer was a login prompt. Auth is now checked diff --git a/README.md b/README.md index 8fc3ef9..85bfce6 100644 --- a/README.md +++ b/README.md @@ -295,6 +295,10 @@ own variables do not reach the child. as a positional and the deny is silently lost. This crate always emits the combined form. - **Copilot needs `--allow-all-tools` to run headlessly at all**, or it stalls at the first tool confirmation. It is always emitted; `Permission` then narrows via denies. +- **A failed turn still exits 0.** Ask Claude for a model that does not exist and it exits + cleanly, reports `subtype: "success"`, and puts "There's an issue with the selected model" + where the answer belongs. Codex does the same and wraps the upstream body in a JSON string. + Both come back as `Error::AgentError`; see below. - **Claude's `stream-json` requires `--verbose`**, or it refuses to start. Handled. - **Large prompts move to stdin automatically** above 128 KiB, so a long prompt never fails with `E2BIG`. @@ -334,6 +338,35 @@ Under a structured format there is no silent fallback to raw stdout: a run that recognizable records, or never reached its terminal record, returns `Error::Parse` rather than a plausible-looking answer assembled from whatever was printed. +## A clean exit is not a successful turn + +All three agents report some failures with exit code 0, with the explanation where the answer +should be. An unknown model, a rejected schema and an upstream outage all arrive this way, so +a caller checking only `Result::is_ok` renders the error as the model's reply. + +These are `Error::AgentError`, carrying the agent's own wording and the provider's status +where one was reported: + +```rust +match run(&request).await { + Err(Error::AgentError { status: Some(404), message, .. }) => { + // Typically a model the account cannot reach. `message` is the agent's wording. + eprintln!("{message}"); + } + Err(e) if e.is_auth_failure() => { /* prompt for login */ } + Err(e) if e.is_transient() => { /* back off */ } + other => { /* ... */ } +} +``` + +`NotAuthenticated` and `RateLimited` are the two members of this family that predate it and +keep their own variants, because the remedy differs. Everything else in it lands here. + +Codex needs one extra step: it forwards the provider's response body as a *string*, so its +`turn.failed` message is JSON containing the sentence and the status. This crate unwraps it, +so `message` is the sentence and `status` is the code. A message that is not that shape is +passed through untouched. + ## No shell, ever Arguments are built as a `Vec` and passed straight to `exec`. There is no shell in diff --git a/src/error.rs b/src/error.rs index 3225d6a..dd3bac9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -143,6 +143,31 @@ pub enum Error { hint: &'static str, }, + /// The agent ran, exited cleanly, and reported that the turn itself failed. + /// + /// Its own variant because the process succeeding says nothing about the + /// turn succeeding. An unknown model, a schema the provider rejects or an + /// upstream outage all arrive this way: exit code 0, with the failure + /// described in the output. Reporting that as `Ok` hands back an + /// [`crate::Outcome`] whose `text` is an error message, which a caller + /// checking only `Result::is_ok` will render as the answer. + /// + /// The same reasoning already applied to [`Error::NotAuthenticated`] and + /// [`Error::RateLimited`], which are also reported with a zero exit; this + /// covers everything else in that family. + #[error("{agent} reported a failed turn{}: {message}", status.map(|s| format!(" (status {s})")).unwrap_or_default())] + AgentError { + /// The agent that failed. + agent: Agent, + /// The binary that ran. + bin: String, + /// The provider's status code, where the agent reported one. A 404 is + /// typically an unknown model, a 400 a rejected request. + status: Option, + /// The agent's own description, unedited. + message: String, + }, + /// The CLI rejected an argument this crate passed it. /// /// Almost always a version mismatch: the flag was verified against the diff --git a/src/event.rs b/src/event.rs index 9f32379..c848b8b 100644 --- a/src/event.rs +++ b/src/event.rs @@ -274,6 +274,36 @@ pub struct Terminal { pub first_unparsed: Option, /// The schema-conforming answer, where the agent reports one separately. pub structured: Option, + /// The provider status code when the agent reported a failed turn, such as + /// a 404 for an unknown model. + pub error_status: Option, + /// The agent's own description of a failed turn, where it gives one apart + /// from the answer text. Claude puts its explanation in `result`, so this + /// stays `None` there; Codex reports it under `turn.failed`. + pub error_message: Option, +} + +/// Unwrap an error body an agent passed through as a JSON string. +/// +/// Codex 0.145.0 forwards the upstream response verbatim, so `turn.failed` +/// carries `{"type":"error","status":400,"error":{"message":"..."}}` encoded as +/// a *string*. Showing that to a user means showing them JSON, and the status +/// worth branching on is buried inside it. Anything that is not that shape is +/// returned as-is. +fn unwrap_error_body(message: &str) -> (Option, String) { + let Ok(body) = serde_json::from_str::(message) else { + return (None, message.to_string()); + }; + let status = body + .get("status") + .and_then(Value::as_u64) + .and_then(|s| u16::try_from(s).ok()); + let inner = body + .get("error") + .and_then(|e| e.get("message")) + .and_then(Value::as_str) + .map(str::to_string); + (status, inner.unwrap_or_else(|| message.to_string())) } /// Incrementally turns one agent's output into [`Event`]s and a [`Terminal`]. @@ -507,7 +537,13 @@ impl Parser { self.term.structured = Some(value.clone()); } self.term.usage = claude_usage(v); + // `subtype` says "success" even for a failed turn, so + // `is_error` is the field that actually decides. self.term.stop = if v.get("is_error").and_then(Value::as_bool) == Some(true) { + self.term.error_status = v + .get("api_error_status") + .and_then(Value::as_u64) + .and_then(|s| u16::try_from(s).ok()); Stop::Error } else { stop_from(v.get("stop_reason")) @@ -651,6 +687,15 @@ impl Parser { "turn.failed" => { self.seen.terminal = true; self.term.stop = Stop::Error; + if let Some(message) = v + .get("error") + .and_then(|e| e.get("message")) + .and_then(Value::as_str) + { + let (status, message) = unwrap_error_body(message); + self.term.error_status = status; + self.term.error_message = Some(bound_text(message)); + } Vec::new() } "item.started" | "item.updated" | "item.completed" => { @@ -796,8 +841,13 @@ impl Parser { self.term.usage.premium_requests = usage.get("premiumRequests").and_then(Value::as_u64); } - if v.get("exitCode").and_then(Value::as_i64).unwrap_or(0) != 0 { + if let Some(code) = v.get("exitCode").and_then(Value::as_i64) + && code != 0 + { self.term.stop = Stop::Error; + // Copilot reports no explanation with the code, so name the + // code rather than leave the failure blank. + self.term.error_message = Some(format!("copilot exited with code {code}")); } Vec::new() } @@ -1151,6 +1201,46 @@ mod tests { ); } + /// Verbatim from codex-cli 0.145.0 with an unknown model. It exits **0** + /// and forwards the upstream body as a JSON *string*, so the status worth + /// branching on is nested one level inside a field that is itself text. + #[test] + fn a_codex_failed_turn_yields_the_reason_and_the_status() { + let (_, term) = run( + Agent::Codex, + &[ + r#"{"type":"thread.started","thread_id":"019fad62"}"#, + r#"{"type":"turn.failed","error":{"message":"{\"type\":\"error\",\"status\":400,\"error\":{\"type\":\"invalid_request_error\",\"message\":\"The 'bogus-model-xyz' model is not supported when using Codex with a ChatGPT account.\"}}"}}"#, + ], + ); + assert_eq!(term.stop, Stop::Error); + assert_eq!(term.error_status, Some(400)); + assert_eq!( + term.error_message.as_deref(), + Some( + "The 'bogus-model-xyz' model is not supported when using Codex with a ChatGPT account." + ), + "the caller should get the sentence, not the envelope" + ); + } + + /// An error that is not the double-encoded shape must survive untouched + /// rather than be dropped for failing to match it. + #[test] + fn a_plain_codex_failure_message_passes_through() { + let (_, term) = run( + Agent::Codex, + &[ + r#"{"type":"turn.failed","error":{"message":"stream disconnected before completion"}}"#, + ], + ); + assert_eq!(term.error_status, None); + assert_eq!( + term.error_message.as_deref(), + Some("stream disconnected before completion") + ); + } + #[test] fn codex_reads_the_thread_id_and_the_completed_message() { let (events, term) = run( diff --git a/src/run.rs b/src/run.rs index 7de0363..8b71cd2 100644 --- a/src/run.rs +++ b/src/run.rs @@ -659,7 +659,11 @@ async fn drive( // result text, so checking only the exit code would hand back a successful // Outcome whose answer is "Please run /login". let unauthenticated = looks_unauthenticated(&terminal.text); - if exit_code != 0 || quota_blocked || unauthenticated { + // The agent saying its turn failed is as much a failure as a non-zero exit, + // and Claude reports an unknown model exactly this way: exit 0, `is_error` + // true, and the explanation where the answer would be. + let turn_failed = terminal.stop == Stop::Error; + if exit_code != 0 || quota_blocked || unauthenticated || turn_failed { return Err(classify_run( request.agent, &bin, @@ -740,7 +744,7 @@ fn classify_run( }; } } - classify(bin, code, stderr, stdout, terminal) + classify(agent, bin, code, stderr, stdout, terminal) } /// Whether text is an agent saying it has no usable credentials. @@ -771,7 +775,14 @@ fn looks_unauthenticated(text: &str) -> bool { } /// Turn a non-zero exit into the most specific error available. -fn classify(bin: &str, code: i32, stderr: &str, stdout: &str, terminal: &Terminal) -> Error { +fn classify( + agent: crate::Agent, + bin: &str, + code: i32, + stderr: &str, + stdout: &str, + terminal: &Terminal, +) -> Error { let quota_signalled = terminal .rate_limit .as_ref() @@ -793,6 +804,24 @@ fn classify(bin: &str, code: i32, stderr: &str, stdout: &str, terminal: &Termina detail, }; } + // Checked before the generic failure but after quota and a rejected flag, + // which are more specific readings of the same output. + if terminal.stop == Stop::Error { + return Error::AgentError { + agent, + bin: bin.to_string(), + status: terminal.error_status, + // Codex reports the reason apart from the answer; Claude puts it + // where the answer would be. + message: terminal + .error_message + .clone() + .or_else(|| first_meaningful_line(&terminal.text)) + .or_else(|| first_meaningful_line(stderr)) + .unwrap_or_else(|| "the agent reported a failure without explaining it".into()), + }; + } + Error::Failed { bin: bin.to_string(), code, @@ -959,7 +988,7 @@ mod tests { ..Terminal::default() }; assert!(matches!( - classify("claude", 1, "", "", &terminal), + classify(Agent::Claude, "claude", 1, "", "", &terminal), Error::RateLimited { .. } )); } @@ -975,11 +1004,72 @@ mod tests { ..Terminal::default() }; assert!(matches!( - classify("claude", 1, "boom", "", &terminal), + classify(Agent::Claude, "claude", 1, "boom", "", &terminal), Error::Failed { .. } )); } + /// Verbatim from a real run with an unknown model. Claude exits **0** with + /// `subtype: "success"` while `is_error` is true and the explanation sits + /// where the answer would be, so a caller checking only `Result::is_ok` + /// renders "There's an issue with the selected model" as the answer. + #[test] + fn a_failed_turn_is_an_error_even_though_the_process_exited_cleanly() { + let terminal = Terminal { + stop: Stop::Error, + error_status: Some(404), + text: "There's an issue with the selected model (bogus-model-xyz). \ + It may not exist or you may not have access to it." + .into(), + ..Terminal::default() + }; + let err = classify_run(Agent::Claude, "claude", 0, "", "", &terminal); + let Error::AgentError { + agent, + status, + message, + .. + } = &err + else { + panic!("expected AgentError, got {err:?}") + }; + assert_eq!(*agent, Agent::Claude); + assert_eq!(*status, Some(404), "the provider status must survive"); + assert!(message.contains("selected model"), "{message}"); + } + + /// A quota refusal and a missing login are more specific readings of the + /// same shape, so they must not be swallowed by the general case. + #[test] + fn a_failed_turn_does_not_mask_a_more_specific_cause() { + let auth = Terminal { + stop: Stop::Error, + text: "Not logged in · Please run /login".into(), + ..Terminal::default() + }; + assert!( + classify_run(Agent::Claude, "claude", 0, "", "", &auth).is_auth_failure(), + "an unauthenticated failed turn must stay an auth failure" + ); + + let quota = Terminal { + stop: Stop::Error, + rate_limit: Some(crate::outcome::RateLimit { + status: "rejected".into(), + window: None, + resets_at: None, + }), + ..Terminal::default() + }; + assert!( + matches!( + classify_run(Agent::Claude, "claude", 0, "", "", "a), + Error::RateLimited { .. } + ), + "a quota-blocked failed turn must stay a rate limit" + ); + } + /// Verified against the real CLI: with `USER` withheld, claude answers /// "Not logged in · Please run /login" and exits **0**. Checking only the /// exit code hands back a successful Outcome whose answer is a login @@ -1075,6 +1165,7 @@ mod tests { #[test] fn a_rejected_flag_is_named_as_a_version_mismatch() { let err = classify( + Agent::Codex, "codex", 2, "error: unexpected argument '--sandbox' found", @@ -1097,7 +1188,7 @@ mod tests { ] { assert!( matches!( - classify("codex", 1, stderr, "", &Terminal::default()), + classify(Agent::Codex, "codex", 1, stderr, "", &Terminal::default()), Error::Failed { .. } ), "{stderr:?} should stay a plain failure" @@ -1152,6 +1243,7 @@ mod tests { #[test] fn failures_report_the_first_useful_line() { let err = classify( + Agent::Claude, "claude", 2, "\n\n real problem \nstack", diff --git a/tests/live.rs b/tests/live.rs index c17be6b..a347fd6 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -61,6 +61,68 @@ async fn claude_answers_and_reports_usage() { assert!(outcome.usage.cost_usd.is_some(), "{:?}", outcome.usage); } +/// The failure this crate most has to get right: claude exits **0** for an +/// unknown model and puts the explanation where the answer goes. Before this +/// was classified, a caller checking `Result::is_ok` rendered "There's an issue +/// with the selected model" as the model's reply. +/// +/// Cheap: the turn is refused before any tokens are spent. +#[tokio::test] +#[ignore = "spawns a real agent"] +async fn an_unknown_model_is_an_error_not_an_answer() { + if !available(Agent::Claude) { + return; + } + let request = ping(Agent::Claude).model("bogus-model-xyz"); + let err = run(&request) + .await + .expect_err("an unknown model must not come back as an answer"); + + let agent_abstraction::Error::AgentError { + status, message, .. + } = &err + else { + panic!("expected AgentError, got {err:?}") + }; + // Not asserted as exactly 404: the status is passed through from the + // provider, and the point is that whatever it sends survives. + assert!( + status.is_some(), + "the provider status should survive: {err:?}" + ); + assert!( + message.to_lowercase().contains("model"), + "the agent's own wording should survive: {message}" + ); +} + +/// Codex fails the same way and adds a wrinkle: it forwards the upstream error +/// body as a JSON string, so without unwrapping, the caller is handed JSON +/// instead of a sentence. +#[tokio::test] +#[ignore = "spawns a real agent"] +async fn codex_reports_an_unknown_model_as_a_sentence_not_json() { + if !available(Agent::Codex) { + return; + } + let request = ping(Agent::Codex).model("bogus-model-xyz"); + let err = run(&request) + .await + .expect_err("an unknown model must not come back as an answer"); + + let agent_abstraction::Error::AgentError { message, .. } = &err else { + panic!("expected AgentError, got {err:?}") + }; + assert!( + !message.trim_start().starts_with('{'), + "the upstream envelope should have been unwrapped: {message}" + ); + assert!( + message.to_lowercase().contains("model"), + "the agent's own wording should survive: {message}" + ); +} + #[tokio::test] #[ignore = "spawns a real agent and consumes quota"] async fn codex_answers_and_reports_usage() { From 32f4e690a99e6eabce7f60216a0d243ef8c103a3 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 29 Jul 2026 17:22:56 +0700 Subject: [PATCH 2/4] Correct the 0.2.0 changelog entry 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. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a90118e..9d891cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,10 @@ Nothing was removed or renamed, so this compiles against any 0.1 code. It is a m rather than a patch because some changes alter behaviour, and `"0.1"` would have delivered them through an ordinary `cargo update`. +*Corrected after release: this entry originally also credited 0.2.0 with streaming by default +and token-by-token Claude text. Those landed three hours after 0.2.0 was published and ship in +0.2.1, where they are now listed. The published 0.2.0 does not have them.* + ### Behaviour changes - **An unauthenticated run is now an error.** Claude exits `0` and puts From 96be0696f266b7b5adddfcc16c941d4896cdb08c Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 29 Jul 2026 17:25:15 +0700 Subject: [PATCH 3/4] Stop reading a healthy quota heartbeat as a refusal 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. --- CHANGELOG.md | 6 ++++++ src/run.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d891cb..d723238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,12 @@ already rendering events, check the first item. ### Fixed +- **A healthy quota heartbeat is no longer read as a refusal.** Every Claude `stream-json` + run prints a `rate_limit_event` record, and on a healthy run its status is `allowed`. + Classification scanned the raw stream for the substring `rate_limit`, which that record + contains, so any Claude failure came back as `Error::RateLimited`: a caller was told to + back off when the real cause was one they could fix. The parsed signal and the agent's own + prose now decide, and the raw scan is only the fallback for output that produced neither. - **A failure reports its cause rather than a status line.** The first line of stderr was taken as the explanation, but CLIs open with progress chatter: a rejected Codex schema reported "Reading additional input from stdin...", which is not what went wrong. A line that looks diff --git a/src/run.rs b/src/run.rs index 8b71cd2..4bade29 100644 --- a/src/run.rs +++ b/src/run.rs @@ -787,11 +787,22 @@ fn classify( .rate_limit .as_ref() .is_some_and(crate::outcome::RateLimit::is_blocking); - if quota_signalled || looks_rate_limited(stderr) || looks_rate_limited(stdout) { + // Scanning the *raw* stream for quota wording is a false-positive machine: + // under `stream-json` Claude prints a `rate_limit_event` record on every + // run, including one whose status is `allowed`, so the substring + // `rate_limit` is present in perfectly healthy output. Where the stream + // parsed, the parsed signal and the agent's own prose decide; the raw scan + // is only the fallback for output that produced neither. + let prose = match (&terminal.error_message, terminal.text.as_str()) { + (Some(message), text) => format!("{message}\n{text}"), + (None, text) if !text.is_empty() => text.to_string(), + _ => stdout.to_string(), + }; + if quota_signalled || looks_rate_limited(stderr) || looks_rate_limited(&prose) { return Error::RateLimited { bin: bin.to_string(), message: first_meaningful_line(stderr) - .or_else(|| first_meaningful_line(stdout)) + .or_else(|| first_meaningful_line(&prose)) .unwrap_or_else(|| "usage limit reached".to_string()), }; } @@ -1009,6 +1020,50 @@ mod tests { )); } + /// Verbatim from a healthy claude 2.1.205 run. Every `stream-json` run + /// carries this record, and its status is `allowed`: nothing is refused. + /// Scanning the raw stream for `rate_limit` matched it anyway, so any + /// Claude failure was reported as a quota refusal, sending a caller to back + /// off when the real cause was something they could fix. + #[test] + fn a_healthy_rate_limit_heartbeat_is_not_a_refusal() { + let stdout = r#"{"type":"rate_limit_event","rate_limit_info":{"status":"allowed","resetsAt":1785331800,"rateLimitType":"five_hour","overageStatus":"rejected","isUsingOverage":false}}"#; + let terminal = Terminal { + stop: Stop::Error, + error_status: Some(404), + text: "There's an issue with the selected model (bogus-model-xyz).".into(), + rate_limit: Some(crate::outcome::RateLimit { + status: "allowed".into(), + window: Some("five_hour".into()), + resets_at: Some(1_785_331_800), + }), + ..Terminal::default() + }; + let err = classify_run(Agent::Claude, "claude", 0, "", stdout, &terminal); + assert!( + matches!(err, Error::AgentError { .. }), + "the heartbeat must not mask the real cause: {err:?}" + ); + } + + /// The counterpart: a refusal the parser did read must still be one, even + /// though it arrives with the same zero exit code. + #[test] + fn a_rejected_quota_signal_is_still_a_refusal() { + let terminal = Terminal { + rate_limit: Some(crate::outcome::RateLimit { + status: "rejected".into(), + window: Some("five_hour".into()), + resets_at: None, + }), + ..Terminal::default() + }; + assert!(matches!( + classify_run(Agent::Claude, "claude", 0, "", "", &terminal), + Error::RateLimited { .. } + )); + } + /// Verbatim from a real run with an unknown model. Claude exits **0** with /// `subtype: "success"` while `is_error` is true and the explanation sits /// where the answer would be, so a caller checking only `Result::is_ok` From 35eb6914feee4850ec1f64663c8296d3c166a726 Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 29 Jul 2026 17:25:53 +0700 Subject: [PATCH 4/4] Note where an unknown model now surfaces 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. --- docs/operating-limits.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/operating-limits.md b/docs/operating-limits.md index f9cd213..45f07aa 100644 --- a/docs/operating-limits.md +++ b/docs/operating-limits.md @@ -15,7 +15,9 @@ of that line, so a future change does not quietly cross it. unedited, and surfaces Claude's `rate_limit_event` as an ordinary `Event::RateLimit` during a run. - **Passes model names through verbatim.** An unknown model is the provider's error to - raise, not this crate's to guess around. + raise, not this crate's to guess around. It is reported as [`Error::AgentError`] with the + provider's status, since the agents answer an unknown model with a clean exit and an + explanation where the answer belongs. ## What this crate deliberately does not do