fix(#2351): key error-result scan on is_error to match the real event shape
This commit is contained in:
parent
9aaf8b93fb
commit
c48d6319f7
1 changed files with 29 additions and 41 deletions
|
|
@ -75,28 +75,26 @@ impl Sentinels {
|
||||||
if matches!(ty, Some("assistant" | "user")) {
|
if matches!(ty, Some("assistant" | "user")) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// A `result` event's `result` field is the model's final answer on a
|
// A `result` event's `result` field is model-authored answer text on a
|
||||||
// SUCCESSFUL turn — model-authored text, same trust level as an
|
// SUCCESSFUL turn — same trust level as an assistant message. A marker
|
||||||
// assistant message. A marker quoted there (the model explaining *this*
|
// quoted there (the model explaining *this* code, or echoing an error
|
||||||
// code, or echoing an error string back) would falsely trip a sentinel
|
// string back) would falsely trip a sentinel and needlessly kill /
|
||||||
// and needlessly kill/downgrade the turn, so for a *success* result we
|
// downgrade the turn, so for a successful result we scrub `result`
|
||||||
// scrub `result` before scanning and rely on the control fields.
|
// before scanning and rely on the other (claude-authored) fields.
|
||||||
//
|
//
|
||||||
// But on an ERROR result the same `result` field carries claude-code's
|
// On a FAILED result the same `result` field instead carries
|
||||||
// OWN failure text — a genuine "Prompt is too long" (an API 400) lands
|
// claude-code's OWN failure text, emitted *instead of* a model answer.
|
||||||
// there, emitted *instead of* a model answer. The model can't forge
|
// The real event (verified against captured stream-json) is:
|
||||||
// `is_error` / a non-`success` subtype (the CLI sets them from the real
|
// {"type":"result","is_error":true,"subtype":"success",
|
||||||
// outcome), so an error result is claude-authored end to end and is
|
// "result":"Prompt is too long","terminal_reason":"blocking_limit"}
|
||||||
// safe to scan raw. Scrubbing it unconditionally (the original
|
// Note `subtype` is "success" even on a hard failure — so `is_error` is
|
||||||
// result-scrub fix) blinded prompt-too-long / auth detection — hence
|
// the only reliable discriminator, and the model can't forge it (the CLI
|
||||||
// keying the scrub on success only. Other control events are
|
// sets it from the real outcome). Scan those raw. Scrubbing `result`
|
||||||
// claude-authored throughout, so they also scan raw.
|
// 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")
|
let error_result = ty == Some("result")
|
||||||
&& (event.get("is_error").and_then(serde_json::Value::as_bool) == Some(true)
|
&& 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"));
|
|
||||||
if ty == Some("result") && !error_result {
|
if ty == Some("result") && !error_result {
|
||||||
let mut scrubbed = event.clone();
|
let mut scrubbed = event.clone();
|
||||||
if let Some(obj) = scrubbed.as_object_mut() {
|
if let Some(obj) = scrubbed.as_object_mut() {
|
||||||
|
|
@ -168,31 +166,21 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[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 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);
|
s.scan_stdout_json(&json(raw), raw);
|
||||||
assert!(matches!(s.soft_error(), Some(Error::PromptTooLong)));
|
assert!(matches!(s.soft_error(), Some(Error::PromptTooLong)));
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
let s = Sentinels::default();
|
||||||
fn prompt_too_long_in_error_result_field_is_detected() {
|
let raw = r#"{"type":"result","is_error":true,"subtype":"success","result":"Failed to authenticate. API Error: 401 Invalid authentication credentials","api_error_status":401}"#;
|
||||||
// Regression guard: claude-code reports a genuine "Prompt is too long"
|
s.scan_stdout_json(&json(raw), raw);
|
||||||
// (API 400) in the `result` field of an ERROR result event. The earlier
|
assert!(matches!(s.soft_error(), Some(Error::AuthFailed)));
|
||||||
// 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}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue