From d8149f2d3d7e42c354f8bd5a4e4d531ea6d60783 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Fri, 9 Oct 2026 01:59:29 -0400 Subject: [PATCH 1/2] Preserve Claude failure diagnostics and supplied limit facts (#587) --- src/harness/interpretation/stream.rs | 141 ++++++++++++-- src/harness/interpretation/stream/tests.rs | 214 +++++++++++++++++++++ tests/run_notification.rs | 49 +++++ 3 files changed, 393 insertions(+), 11 deletions(-) diff --git a/src/harness/interpretation/stream.rs b/src/harness/interpretation/stream.rs index a8182b35..5be09b86 100644 --- a/src/harness/interpretation/stream.rs +++ b/src/harness/interpretation/stream.rs @@ -4,7 +4,7 @@ //! can resume it. Only the first init sets shared session fields, and only //! completion supplies the last result and any work killed since that result. -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use serde_json::Value; @@ -33,11 +33,21 @@ enum Dialect { #[derive(Default)] struct Claude { failed: bool, + diagnostic: Option, + /// The latest root API error, only until this turn's result. + api_error: Option, tasks: HashMap, /// Task id and description, in order of the first kill since the result. killed: Vec<(String, String)>, } +#[derive(Default)] +struct ClaudeApiError { + message: Option, + /// Only diagnostic fields, never unrelated raw event content. + facts: BTreeMap<&'static str, String>, +} + #[derive(Default)] struct Grok { failed: bool, @@ -136,13 +146,20 @@ impl Decoder for Stream { }); vec!["session started".to_string()] } - Some("assistant") => event["message"]["content"] - .as_array() - .into_iter() - .flatten() - .filter(|block| block["type"] == "tool_use") - .filter_map(|block| Some(self.tool_use(block["name"].as_str()?, &block["input"]))) - .collect(), + Some("assistant") => { + if let Dialect::Claude(claude) = &mut self.dialect { + claude.assistant(&event); + } + event["message"]["content"] + .as_array() + .into_iter() + .flatten() + .filter(|block| block["type"] == "tool_use") + .filter_map(|block| { + Some(self.tool_use(block["name"].as_str()?, &block["input"])) + }) + .collect() + } Some("system") => { if let Dialect::Claude(claude) = &mut self.dialect { claude.track_task(&event); @@ -150,6 +167,11 @@ impl Decoder for Stream { Vec::new() } Some("result") => { + if matches!(self.dialect, Dialect::Claude(_)) + && !event["parent_tool_use_id"].is_null() + { + return Vec::new(); + } self.final_message = event["result"].as_str().map(String::from); if let (Some(turns), Some(cost)) = ( event["num_turns"].as_u64(), @@ -160,8 +182,7 @@ impl Decoder for Stream { let failed = event["subtype"] != "success" || event["is_error"] == true; match &mut self.dialect { Dialect::Claude(claude) => { - claude.failed = failed; - claude.killed.clear(); + claude.result(&event, failed); } Dialect::Grok(grok) => grok.result(&event, failed), } @@ -198,7 +219,7 @@ impl Decoder for Stream { let (failed, diagnostic) = match self.dialect { Dialect::Claude(claude) => { ended.killed = claude.killed.into_iter().map(|(_, text)| text).collect(); - (claude.failed, None) + (claude.failed, claude.diagnostic) } Dialect::Grok(grok) => { ended.session_id = ended.session_id.or(grok.fallback_id); @@ -218,6 +239,38 @@ impl Decoder for Stream { } impl Claude { + fn assistant(&mut self, event: &Value) { + if !event["parent_tool_use_id"].is_null() { + return; + } + self.api_error = (event["is_api_error_message"] == true + || nonempty_text(&event["error"]).is_some() + || nonempty_text(&event["api_error"]).is_some()) + .then(|| ClaudeApiError::read(event)); + } + + fn result(&mut self, event: &Value, failed: bool) { + self.failed = failed; + self.killed.clear(); + let api_error = self.api_error.take().unwrap_or_default(); + self.diagnostic = if failed { + let errors: Vec<_> = event["errors"] + .as_array() + .into_iter() + .flatten() + .filter_map(nonempty_text) + .collect(); + let message = if errors.is_empty() { + nonempty_text(&event["result"]).map(String::from) + } else { + Some(errors.join("\n")) + }; + api_error.diagnostic(event, message) + } else { + None + }; + } + fn track_task(&mut self, event: &Value) { let Some(id) = event["task_id"].as_str() else { return; @@ -246,6 +299,72 @@ impl Claude { } } +impl ClaudeApiError { + fn read(event: &Value) -> Self { + let text: Vec<_> = event["message"]["content"] + .as_array() + .into_iter() + .flatten() + .filter(|block| block["type"] == "text") + .filter_map(|block| nonempty_text(&block["text"])) + .collect(); + Self { + message: (!text.is_empty()).then(|| text.join("\n")), + facts: Self::facts(event), + } + } + + fn facts(event: &Value) -> BTreeMap<&'static str, String> { + let mut facts = BTreeMap::new(); + if let Some(kind) = nonempty_text(&event["api_error"]) { + facts.insert("api_error", kind.to_string()); + } + if let Some(status) = event["api_error_status"].as_u64() { + facts.insert("api_error_status", status.to_string()); + } + let limit = &event["api_error_params"]["rate_limit_info"]; + for key in [ + "status", + "rateLimitType", + "overageStatus", + "overageDisabledReason", + ] { + if let Some(text) = nonempty_text(&limit[key]) { + facts.insert(key, text.to_string()); + } + } + for key in ["resetsAt", "overageResetsAt"] { + if let Some(timestamp) = limit[key].as_u64() { + facts.insert(key, timestamp.to_string()); + } + } + facts + } + + fn diagnostic(mut self, terminal: &Value, message: Option) -> Option { + // Terminal facts replace only the fields they actually supply. + self.facts.extend(Self::facts(terminal)); + let message = message.or(self.message); + if self.facts.is_empty() { + return message; + } + let facts = self + .facts + .into_iter() + .map(|(key, value)| format!("{key}: {value}")) + .collect::>() + .join("; "); + Some(format!( + "{}\n{facts}", + message.as_deref().unwrap_or("Claude's turn failed") + )) + } +} + +fn nonempty_text(value: &Value) -> Option<&str> { + value.as_str().filter(|text| !text.trim().is_empty()) +} + impl Grok { fn result(&mut self, event: &Value, failed: bool) { self.failed = failed; diff --git a/src/harness/interpretation/stream/tests.rs b/src/harness/interpretation/stream/tests.rs index d6a6e81d..5431f37d 100644 --- a/src/harness/interpretation/stream/tests.rs +++ b/src/harness/interpretation/stream/tests.rs @@ -5,6 +5,220 @@ use crate::harness::{ use serde_json::json; use std::os::unix::process::ExitStatusExt; +#[test] +fn claude_terminal_failure_keeps_the_captured_spend_limit_message_for_either_exit() { + const MESSAGE: &str = "You've hit your monthly spend limit · raise it at https://claude.ai/settings/usage?from=cc_cli_limit_message · your weekly limit resets Oct 10, 2am (UTC)"; + for exit in [0, 1] { + let mut interpretation = stream(Harness::Claude, ""); + interpretation.condense(r#"{"type":"result","subtype":"success","result":"Earlier work completed.","num_turns":17,"total_cost_usd":1.25}"#); + interpretation.condense(&json!({"type":"result","subtype":"success","is_error":true,"api_error_status":429,"result":MESSAGE}).to_string()); + let completion = interpretation.finish(Ok(std::process::ExitStatus::from_raw(exit << 8))); + let prefix = if exit == 0 { + "claude's turn failed" + } else { + "claude exited 1" + }; + let cause = completion.outcome.unwrap_err().to_string(); + assert!( + cause.starts_with(&format!("{prefix}: {MESSAGE}")), + "{cause}" + ); + assert!(cause.contains("api_error_status: 429"), "{cause}"); + assert_eq!( + completion.report.unwrap().summary.as_deref(), + Some("17 turns, $1.25") + ); + } +} + +#[test] +fn claude_terminal_diagnostics_prefer_meaningful_errors_and_tolerate_malformed_fields() { + for (errors, result, expected) in [ + ( + json!([null, "", " \n", 7, "first", "second"]), + json!("result text"), + "claude's turn failed: first\nsecond", + ), + ( + json!([null, 7, " "]), + json!("result text"), + "claude's turn failed: result text", + ), + ( + json!("malformed"), + json!("result text"), + "claude's turn failed: result text", + ), + (json!([""]), json!(" \n"), "claude's turn failed"), + ( + json!({"message":"malformed"}), + json!(42), + "claude's turn failed", + ), + ] { + let (completion, _) = lines( + Harness::Claude, + &[json!({"type":"result","subtype":"error","errors":errors,"result":result})], + ); + assert_eq!(completion.outcome.unwrap_err().to_string(), expected); + } +} + +#[test] +fn claude_failed_results_keep_root_api_fallback_and_supplied_limit_facts() { + let api_error = json!({ + "type":"assistant", "parent_tool_use_id":null, "is_api_error_message":true, + "api_error":"usage_limit_reached", "api_error_status":429, + "api_error_params":{"rate_limit_info":{ + "status":"rejected", "rateLimitType":"seven_day", "resetsAt":1791597600, + "overageStatus":"rejected", "overageDisabledReason":"org_level_disabled_until", + "unrelated":"private raw event content" + }}, + "message":{"content":[{"type":"text","text":"Provider's limit message."}]} + }); + for (terminal, message) in [ + ( + json!({"type":"result","subtype":"success","is_error":true,"api_error_status":429,"result":"Monthly spend limit; weekly reset Oct 10."}), + "Monthly spend limit; weekly reset Oct 10.", + ), + ( + json!({"type":"result","subtype":"error","errors":["terminal error"],"result":"ignored"}), + "terminal error", + ), + ( + json!({"type":"result","subtype":"error","errors":[null, " "],"result":""}), + "Provider's limit message.", + ), + ] { + let (completion, _) = lines(Harness::Claude, &[api_error.clone(), terminal]); + let cause = completion.outcome.unwrap_err().to_string(); + assert!( + cause.starts_with(&format!("claude's turn failed: {message}")), + "{cause}" + ); + for fact in [ + "api_error: usage_limit_reached", + "api_error_status: 429", + "rateLimitType: seven_day", + "resetsAt: 1791597600", + "overageStatus: rejected", + "overageDisabledReason: org_level_disabled_until", + ] { + assert!(cause.contains(fact), "missing {fact:?}: {cause}"); + } + assert!(!cause.contains("private raw event content"), "{cause}"); + assert!(!cause.contains("overageResetsAt"), "{cause}"); + } +} + +#[test] +fn claude_child_errors_cannot_replace_the_successful_parent_result() { + for exit in [0, 1] { + let mut interpretation = stream(Harness::Claude, ""); + for event in [ + json!({"type":"result","subtype":"success","result":"Parent done.","num_turns":2,"total_cost_usd":0.3}), + json!({"type":"assistant","parent_tool_use_id":"child","is_api_error_message":true,"api_error":"usage_limit_reached","message":{"content":[{"type":"text","text":"child failure"}]}}), + json!({"type":"result","parent_tool_use_id":"child","subtype":"error","errors":["child failure"],"result":"child result","num_turns":99,"total_cost_usd":99}), + ] { + interpretation.condense(&event.to_string()); + } + let completion = interpretation.finish(Ok(std::process::ExitStatus::from_raw(exit << 8))); + if exit == 0 { + assert_eq!( + completion.outcome.unwrap().final_message.as_deref(), + Some("Parent done.") + ); + } else { + assert_eq!( + completion.outcome.unwrap_err().to_string(), + "claude exited 1" + ); + } + assert_eq!( + completion.report.unwrap().summary.as_deref(), + Some("2 turns, $0.30") + ); + } +} + +#[test] +fn claude_parent_success_clears_recovered_errors_and_never_supplies_an_exit_diagnostic() { + for exit in [0, 1] { + let mut interpretation = stream(Harness::Claude, ""); + for event in [ + json!({"type":"assistant","parent_tool_use_id":null,"is_api_error_message":true,"api_error":"usage_limit_reached","api_error_params":{"rate_limit_info":{"rateLimitType":"seven_day","resetsAt":1791597600}},"message":{"content":[{"type":"text","text":"retried root error"}]}}), + json!({"type":"result","subtype":"error","errors":["earlier failed turn"]}), + json!({"type":"result","subtype":"success","errors":["not a failure"],"result":"Parent done."}), + ] { + interpretation.condense(&event.to_string()); + } + let completion = interpretation.finish(Ok(std::process::ExitStatus::from_raw(exit << 8))); + if exit == 0 { + assert_eq!( + completion.outcome.unwrap().final_message.as_deref(), + Some("Parent done.") + ); + } else { + assert_eq!( + completion.outcome.unwrap_err().to_string(), + "claude exited 1" + ); + } + } +} + +#[test] +fn claude_recovered_root_api_errors_do_not_contaminate_a_later_failure() { + for recovery in [ + json!({"type":"assistant","message":{"content":[{"type":"text","text":"Recovered."}]}}), + json!({"type":"result","subtype":"success","result":"Recovered."}), + ] { + let (completion, _) = lines( + Harness::Claude, + &[ + json!({"type":"assistant","is_api_error_message":true,"api_error":"usage_limit_reached","api_error_params":{"rate_limit_info":{"rateLimitType":"seven_day","resetsAt":1791597600}},"message":{"content":[{"type":"text","text":"earlier limit"}]}}), + recovery, + json!({"type":"assistant","parent_tool_use_id":"child","error":"rate_limit","message":{"content":[{"type":"text","text":"child limit"}]}}), + json!({"type":"result","subtype":"error"}), + ], + ); + assert_eq!( + completion.outcome.unwrap_err().to_string(), + "claude's turn failed" + ); + } +} + +#[test] +fn claude_reports_only_supplied_limit_facts_and_never_classifies_a_bare_429() { + for params in [ + json!(null), + json!({"rate_limit_info":"malformed"}), + json!({"rate_limit_info":{"rateLimitType":4,"resetsAt":"tomorrow","overageResetsAt":-1,"overageStatus":false}}), + ] { + let (completion, _) = lines( + Harness::Claude, + &[ + json!({"type":"result","subtype":"success","is_error":true,"api_error_status":429,"api_error_params":params}), + ], + ); + assert_eq!( + completion.outcome.unwrap_err().to_string(), + "claude's turn failed: Claude's turn failed\napi_error_status: 429" + ); + } + let (completion, _) = lines( + Harness::Claude, + &[ + json!({"type":"result","subtype":"error","result":"Extra usage rejected.","api_error_params":{"rate_limit_info":{"overageStatus":"rejected","overageResetsAt":1792000000}}}), + ], + ); + assert_eq!( + completion.outcome.unwrap_err().to_string(), + "claude's turn failed: Extra usage rejected.\noverageResetsAt: 1792000000; overageStatus: rejected" + ); +} + #[test] fn first_init_alone_sets_the_directory_subscription_and_init_id() { for harness in [Harness::Claude, Harness::Grok] { diff --git a/tests/run_notification.rs b/tests/run_notification.rs index 87421ffc..8b6ee9e4 100644 --- a/tests/run_notification.rs +++ b/tests/run_notification.rs @@ -168,6 +168,55 @@ fn a_failed_run_sends_one_notification_with_the_cause() { assert_contains(text, &the_one_log(&scenario)); } +#[test] +fn a_claude_limit_failure_has_the_same_meaningful_cause_in_terminal_and_notification() { + const MESSAGE: &str = "You've hit your monthly spend limit · raise it at https://claude.ai/settings/usage?from=cc_cli_limit_message · your weekly limit resets Oct 10, 2am (UTC)"; + const SCRIPT: &str = r#" +echo '{"type":"result","subtype":"success","result":"Earlier work completed."}' +cat > "$FAKE_CLAUDE_AFTER_RESULT" <<'EVENTS' +{"type":"assistant","parent_tool_use_id":null,"is_api_error_message":true,"api_error":"usage_limit_reached","api_error_status":429,"api_error_params":{"rate_limit_info":{"status":"rejected","rateLimitType":"seven_day","resetsAt":1791597600,"overageStatus":"rejected","overageDisabledReason":"org_level_disabled_until","unrelated":"private raw event content"}},"message":{"content":[{"type":"text","text":"Provider's limit message."}]}} +{"type":"result","subtype":"success","is_error":true,"api_error_status":429,"result":"You've hit your monthly spend limit · raise it at https://claude.ai/settings/usage?from=cc_cli_limit_message · your weekly limit resets Oct 10, 2am (UTC)"} +EVENTS +"#; + for exit in [0, 1] { + let scenario = Scenario::new(); + scenario.issue_titled(7, "Add export button"); + scenario.agent_does(&format!("{SCRIPT}\nexit {exit}\n")); + let resend = ResendStandIn::replying(200, ACCEPTED); + let result = run( + &scenario, + &resend, + &["--email", "me@example.com", &scenario.issue_url(7)], + Some(KEY), + ); + assert_eq!(result.code, Some(1), "{}", result.stderr); + let request = the_one_request(&resend); + assert!(subject(&request).ends_with(": failed"), "{request:?}"); + let prefix = if exit == 0 { + "claude's turn failed" + } else { + "claude exited 1" + }; + let cause = format!("{prefix}: {MESSAGE}"); + assert_contains(&result.stderr, &cause); + assert_contains(text(&request), &format!("Cause: {cause}")); + for output in [result.stderr.as_str(), text(&request)] { + for fact in [ + "api_error: usage_limit_reached", + "api_error_status: 429", + "rateLimitType: seven_day", + "resetsAt: 1791597600", + "overageStatus: rejected", + "overageDisabledReason: org_level_disabled_until", + ] { + assert_contains(output, fact); + } + assert!(!output.contains("private raw event content"), "{output}"); + } + assert_eq!(scenario.claude_calls().len(), 1, "unexpected Resume"); + } +} + #[test] fn a_run_that_fails_after_killed_background_work_names_that_work_in_its_notification() { let scenario = Scenario::new(); From d60bf020aaeae4bd6eaf1ba1a6f6d1d5ae8c703b Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Fri, 9 Oct 2026 02:08:56 -0400 Subject: [PATCH 2/2] Retain root API fallback when Claude exits before its result --- src/harness/interpretation/stream.rs | 16 +++--- src/harness/interpretation/stream/tests.rs | 60 ++++++++++++++++++++++ tests/architect_run.rs | 10 ++-- 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/src/harness/interpretation/stream.rs b/src/harness/interpretation/stream.rs index 5be09b86..70269541 100644 --- a/src/harness/interpretation/stream.rs +++ b/src/harness/interpretation/stream.rs @@ -219,7 +219,11 @@ impl Decoder for Stream { let (failed, diagnostic) = match self.dialect { Dialect::Claude(claude) => { ended.killed = claude.killed.into_iter().map(|(_, text)| text).collect(); - (claude.failed, claude.diagnostic) + let diagnostic = claude + .api_error + .and_then(|error| error.diagnostic(None)) + .or(claude.diagnostic); + (claude.failed, diagnostic) } Dialect::Grok(grok) => { ended.session_id = ended.session_id.or(grok.fallback_id); @@ -252,8 +256,10 @@ impl Claude { fn result(&mut self, event: &Value, failed: bool) { self.failed = failed; self.killed.clear(); - let api_error = self.api_error.take().unwrap_or_default(); + let mut api_error = self.api_error.take().unwrap_or_default(); self.diagnostic = if failed { + // Terminal facts replace only the fields they actually supply. + api_error.facts.extend(ClaudeApiError::facts(event)); let errors: Vec<_> = event["errors"] .as_array() .into_iter() @@ -265,7 +271,7 @@ impl Claude { } else { Some(errors.join("\n")) }; - api_error.diagnostic(event, message) + api_error.diagnostic(message) } else { None }; @@ -341,9 +347,7 @@ impl ClaudeApiError { facts } - fn diagnostic(mut self, terminal: &Value, message: Option) -> Option { - // Terminal facts replace only the fields they actually supply. - self.facts.extend(Self::facts(terminal)); + fn diagnostic(self, message: Option) -> Option { let message = message.or(self.message); if self.facts.is_empty() { return message; diff --git a/src/harness/interpretation/stream/tests.rs b/src/harness/interpretation/stream/tests.rs index 5431f37d..c6d5f8a1 100644 --- a/src/harness/interpretation/stream/tests.rs +++ b/src/harness/interpretation/stream/tests.rs @@ -966,3 +966,63 @@ mod grok { } } } + +#[test] +fn claude_root_api_error_is_a_fallback_when_process_exits_without_a_result() { + for earlier_success in [true, false] { + for exit in [0, 1] { + let mut interpretation = stream(Harness::Claude, ""); + if earlier_success { + interpretation.condense( + &json!({ + "type":"result", "subtype":"success", + "result":"Earlier work completed." + }) + .to_string(), + ); + } + interpretation.condense( + &json!({ + "type":"assistant", "parent_tool_use_id":null, + "is_api_error_message":true, + "api_error":"usage_limit_reached", "api_error_status":429, + "api_error_params":{"rate_limit_info":{ + "rateLimitType":"seven_day", "resetsAt":1791597600, + "overageStatus":"rejected", + "overageDisabledReason":"org_level_disabled_until", + "unrelated":"private raw event content" + }}, + "message":{"content":[{ + "type":"text", "text":"Provider's limit message." + }]} + }) + .to_string(), + ); + let completion = + interpretation.finish(Ok(std::process::ExitStatus::from_raw(exit << 8))); + if exit == 0 { + assert_eq!( + completion.outcome.unwrap().final_message.as_deref(), + earlier_success.then_some("Earlier work completed."), + ); + } else { + let cause = completion.outcome.unwrap_err().to_string(); + assert!( + cause.starts_with("claude exited 1: Provider's limit message."), + "{cause}", + ); + for fact in [ + "api_error: usage_limit_reached", + "api_error_status: 429", + "rateLimitType: seven_day", + "resetsAt: 1791597600", + "overageStatus: rejected", + "overageDisabledReason: org_level_disabled_until", + ] { + assert!(cause.contains(fact), "missing {fact:?}: {cause}"); + } + assert!(!cause.contains("private raw event content"), "{cause}"); + } + } + } +} diff --git a/tests/architect_run.rs b/tests/architect_run.rs index ef75a8e4..36cf1e62 100644 --- a/tests/architect_run.rs +++ b/tests/architect_run.rs @@ -30,6 +30,10 @@ use support::{REPO, RunResult, Scenario, TimelineEvent, before_command_log, leav /// The first issue the fake agent creates: the scenario starts with issue #7. const PLAN_URL: &str = "https://github.com/acme/widgets/issues/8"; +/// The fake marks its terminal result failed when the script exits 3, so +/// its supplied result text belongs in the failure cause. +const FAILED_REVIEW_CAUSE: &str = "claude exited 3: Published the plan.\n\nArchitecture review plan: https://github.com/acme/widgets/issues/8\n"; + /// The first pull request opened on the fake GitHub. const PR_URL: &str = "https://github.com/acme/widgets/pull/1"; @@ -454,7 +458,7 @@ fn a_failing_session_fails_the_architect_run_and_leaves_the_plan_needing_triage( let result = scenario.run(&["architect", "--plan-only"]); - assert_failed(&scenario, &result, "claude exited 3"); + assert_failed(&scenario, &result, FAILED_REVIEW_CAUSE); assert_eq!(scenario.issue_labels(8), ["needs-triage"]); } @@ -1455,7 +1459,7 @@ fn a_failed_review_sends_one_notification_with_the_cause_and_the_session_log() { &["architect", "--email", "me@example.com"], ); - assert_failed(&scenario, &result, "claude exited 3"); + assert_failed(&scenario, &result, FAILED_REVIEW_CAUSE); let (subject, text) = the_one_notification(&resend); assert_eq!( subject, @@ -1468,7 +1472,7 @@ fn a_failed_review_sends_one_notification_with_the_cause_and_the_session_log() { text.starts_with(&format!( "Result: review failed\n\ Review: failed\n\ - Cause: claude exited 3\n\ + Cause: {FAILED_REVIEW_CAUSE}\n\ Session log: {log}\n\ Command log: {command_log}\n" )),