diff --git a/hive-claude/src/classify.rs b/hive-claude/src/classify.rs index 95a11292..45a82e20 100644 --- a/hive-claude/src/classify.rs +++ b/hive-claude/src/classify.rs @@ -75,28 +75,26 @@ impl Sentinels { if matches!(ty, Some("assistant" | "user")) { return; } - // A `result` event's `result` field is the model's final answer on a - // SUCCESSFUL turn — model-authored text, same trust level as an - // assistant message. A marker quoted there (the model explaining *this* - // code, or echoing an error string back) would falsely trip a sentinel - // and needlessly kill/downgrade the turn, so for a *success* result we - // scrub `result` before scanning and rely on the control fields. + // A `result` event's `result` field is model-authored answer text on a + // SUCCESSFUL turn — same trust level as an assistant message. A marker + // quoted there (the model explaining *this* code, or echoing an error + // string back) would falsely trip a sentinel and needlessly kill / + // downgrade the turn, so for a successful result we scrub `result` + // before scanning and rely on the other (claude-authored) fields. // - // But on an ERROR result the same `result` field carries claude-code's - // OWN failure text — a genuine "Prompt is too long" (an API 400) lands - // there, emitted *instead of* a model answer. The model can't forge - // `is_error` / a non-`success` subtype (the CLI sets them from the real - // outcome), so an error result is claude-authored end to end and is - // safe to scan raw. Scrubbing it unconditionally (the original - // result-scrub fix) blinded prompt-too-long / auth detection — hence - // keying the scrub on success only. Other control events are - // claude-authored throughout, so they also scan raw. + // On a FAILED result the same `result` field instead carries + // claude-code's OWN failure text, emitted *instead of* a model answer. + // The real event (verified against captured stream-json) is: + // {"type":"result","is_error":true,"subtype":"success", + // "result":"Prompt is too long","terminal_reason":"blocking_limit"} + // Note `subtype` is "success" even on a hard failure — so `is_error` is + // the only reliable discriminator, and the model can't forge it (the CLI + // sets it from the real outcome). Scan those raw. Scrubbing `result` + // unconditionally (the original fix) blinded prompt-too-long / auth + // detection. Other control events are claude-authored throughout, so + // they also scan raw. let error_result = ty == Some("result") - && (event.get("is_error").and_then(serde_json::Value::as_bool) == Some(true) - || event - .get("subtype") - .and_then(serde_json::Value::as_str) - .is_some_and(|s| s != "success")); + && event.get("is_error").and_then(serde_json::Value::as_bool) == Some(true); if ty == Some("result") && !error_result { let mut scrubbed = event.clone(); if let Some(obj) = scrubbed.as_object_mut() { @@ -168,31 +166,21 @@ mod tests { } #[test] - fn control_event_marker_is_detected() { + fn genuine_failure_result_is_detected() { + // Both fixtures are the real claude-code failure shape, verified against + // captured stream-json: the marker lives in the `result` field with + // `is_error: true` and — counterintuitively — `subtype: "success"`. The + // earlier unconditional `result` scrub blinded this; `is_error` is the + // discriminator that restores it (and the model can't forge it). let s = Sentinels::default(); - let raw = r#"{"type":"result","subtype":"error","error":"Prompt is too long"}"#; + let raw = r#"{"type":"result","is_error":true,"subtype":"success","result":"Prompt is too long","terminal_reason":"blocking_limit","stop_reason":"stop_sequence"}"#; s.scan_stdout_json(&json(raw), raw); assert!(matches!(s.soft_error(), Some(Error::PromptTooLong))); - } - #[test] - fn prompt_too_long_in_error_result_field_is_detected() { - // Regression guard: claude-code reports a genuine "Prompt is too long" - // (API 400) in the `result` field of an ERROR result event. The earlier - // result-scrub blinded this by stripping `result` unconditionally. An - // error result is claude-authored (the model can't forge `is_error`), - // so it must be scanned raw. - for raw in [ - r#"{"type":"result","subtype":"error_during_execution","is_error":true,"result":"API Error: 400 Prompt is too long"}"#, - r#"{"type":"result","subtype":"success","is_error":true,"result":"Prompt is too long"}"#, - ] { - let s = Sentinels::default(); - s.scan_stdout_json(&json(raw), raw); - assert!( - matches!(s.soft_error(), Some(Error::PromptTooLong)), - "prompt-too-long in an error result's `result` field must be detected: {raw}" - ); - } + let s = Sentinels::default(); + let raw = r#"{"type":"result","is_error":true,"subtype":"success","result":"Failed to authenticate. API Error: 401 Invalid authentication credentials","api_error_status":401}"#; + s.scan_stdout_json(&json(raw), raw); + assert!(matches!(s.soft_error(), Some(Error::AuthFailed))); } #[test]