diff --git a/hive-claude/src/classify.rs b/hive-claude/src/classify.rs index 80dc836d..45a82e20 100644 --- a/hive-claude/src/classify.rs +++ b/hive-claude/src/classify.rs @@ -5,18 +5,9 @@ //! are empirically stable across CLI versions; if one drifts the run degrades //! gracefully (a clean turn, or a hard [`crate::Error::Exit`] on a non-zero //! exit) rather than misbehaving. -//! -//! Stdout events are parsed into a typed [`StreamEvent`] (a serde-tagged enum) -//! rather than walked as a `serde_json::Value` — the event-type dispatch and -//! the terminal result event's fields (`is_error`, `api_error_status`) are -//! typed, so a shape drift is a clean fallback instead of a silent -//! mis-detection. The marker *strings* still gate detection (they're the -//! API's error text); typing removes the fragile field-probing around them. use std::sync::atomic::{AtomicBool, Ordering}; -use serde::Deserialize; - /// Emitted when the prompt/context exceeds the model's window. const PROMPT_TOO_LONG_MARKER: &str = "Prompt is too long"; @@ -48,53 +39,6 @@ const SESSION_NOT_FOUND_MARKERS: [&str; 2] = [ "No conversation found with session ID", ]; -/// The claude-code stdout stream-json events we classify, internally tagged -/// on `type`. Typing the dispatch (and the `result` event's fields below) -/// replaces walking a `serde_json::Value` with `get("field").and_then(...)`, -/// so a field-shape drift is a compile error / a clean fallback rather than a -/// silent mis-detection. Unknown event types fall into [`StreamEvent::Other`] -/// (scanned raw like any other control event). -#[derive(Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] -enum StreamEvent { - /// Model-authored turn output — never scanned (it can quote any marker). - Assistant, - /// Model-authored tool results — never scanned. - User, - /// The terminal result event (see [`ResultEvent`]). - Result(ResultEvent), - /// A control `error` event — the only event a rate-limit marker is - /// trusted on (a model *discussing* a rate limit can't forge this type). - Error, - /// `system` and any other/unknown control event — claude-authored, so - /// scanned raw for the failure markers. - #[serde(other)] - Other, -} - -/// The terminal `result` event's fields we act on. `subtype` is deliberately -/// absent: it reads `"success"` even on a hard failure, so it's not a usable -/// error signal — `is_error` is the discriminator (the CLI sets it from the -/// real outcome; the model can't forge it). On `is_error` the `result` / -/// `error` fields carry claude-code's own failure text. -#[derive(Deserialize)] -struct ResultEvent { - #[serde(default)] - is_error: bool, - /// claude-code's failure text on `is_error` (a genuine "Prompt is too - /// long" / auth message); the model's final answer on success. - #[serde(default)] - result: Option, - /// Structured error payload (a string, or an object like - /// `{"type":"rate_limit_error"}`) present on some failures. - #[serde(default)] - error: Option, - /// API status on an auth failure (e.g. `401`) — a typed signal that - /// doesn't depend on the human-readable string landing in `result`. - #[serde(default)] - api_error_status: Option, -} - /// Shared, lock-free sentinel flags accumulated while both output streams are /// pumped concurrently. Read once after the child exits. #[derive(Default)] @@ -127,56 +71,43 @@ impl Sentinels { /// an assistant/user message. This holds whatever exact shape claude-code /// uses for the message, so the gate can't suppress a genuine signal. pub(crate) fn scan_stdout_json(&self, event: &serde_json::Value, raw: &str) { - // Type the event instead of walking the `Value`. A parse failure - // (missing / non-string `type`) is rare and falls through to a raw - // scan — the safe default, matching the old "unknown type" branch. - match StreamEvent::deserialize(event) { - // Model-authored output — its serialized content can quote any - // marker verbatim (an agent discussing this very code), so it is - // never scanned. A genuine signal is emitted *instead of* a model - // turn, so it never rides an assistant/user event. - Ok(StreamEvent::Assistant | StreamEvent::User) => {} - - // The terminal result event. On success its `result` field is the - // model's final answer (same trust level as an assistant message - // — must NOT scan). On a FAILED result (`is_error`) the `result` / - // `error` fields carry claude-code's OWN failure text — 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 discriminator (the model can't forge it). - Ok(StreamEvent::Result(r)) => { - if r.is_error { - if let Some(text) = &r.result { - self.scan_failure_markers(text); - } - if let Some(err) = &r.error { - // `to_string()` re-serializes the JSON value (a string - // value round-trips to `"...quoted..."`), but the marker - // substring still appears inside the quoted form, so the - // scan finds it either way. - self.scan_failure_markers(&err.to_string()); - } - // Typed 401: an auth failure that set the status code but - // may not have put the human-readable string in `result`. - if r.api_error_status == Some(401) { - self.auth_failed.store(true, Ordering::Relaxed); - } - } + let ty = event.get("type").and_then(|t| t.as_str()); + if matches!(ty, Some("assistant" | "user")) { + return; + } + // 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. + // + // 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); + if ty == Some("result") && !error_result { + let mut scrubbed = event.clone(); + if let Some(obj) = scrubbed.as_object_mut() { + obj.remove("result"); } - - // A control `error` event — claude-authored end to end, and the - // only place a rate-limit marker is trusted. - Ok(StreamEvent::Error) => { - self.scan_failure_markers(raw); - if RATE_LIMIT_MARKERS.iter().any(|m| raw.contains(m)) { - self.rate_limited.store(true, Ordering::Relaxed); - } - } - - // `system` / unknown control events (claude-authored) → scan raw. - Ok(StreamEvent::Other) | Err(_) => self.scan_failure_markers(raw), + self.scan_failure_markers(&scrubbed.to_string()); + } else { + self.scan_failure_markers(raw); + } + // Rate-limit stays scoped to `error` events (unchanged): the only + // control event that carries a rate-limit marker. + if ty == Some("error") && RATE_LIMIT_MARKERS.iter().any(|m| raw.contains(m)) { + self.rate_limited.store(true, Ordering::Relaxed); } } @@ -252,28 +183,6 @@ mod tests { assert!(matches!(s.soft_error(), Some(Error::AuthFailed))); } - #[test] - fn typed_api_error_status_401_trips_auth_without_string_marker() { - // The typed `api_error_status` field catches an auth failure even when - // the human-readable 401 string isn't in `result` — a signal the old - // substring scan would have missed. - let s = Sentinels::default(); - let raw = r#"{"type":"result","is_error":true,"subtype":"success","result":"request failed","api_error_status":401}"#; - s.scan_stdout_json(&json(raw), raw); - assert!(matches!(s.soft_error(), Some(Error::AuthFailed))); - } - - #[test] - fn error_payload_object_on_result_is_scanned() { - // A failure result can carry the marker in a structured `error` - // payload rather than `result`; the typed `error: Value` is stringified - // and scanned. - let s = Sentinels::default(); - let raw = r#"{"type":"result","is_error":true,"error":"Prompt is too long","result":"ok"}"#; - s.scan_stdout_json(&json(raw), raw); - assert!(matches!(s.soft_error(), Some(Error::PromptTooLong))); - } - #[test] fn result_field_model_text_quoting_marker_is_ignored() { // The terminal `result` event's `result` field is the model's final