diff --git a/src/work_item_pipeline.rs b/src/work_item_pipeline.rs index 594c0d82..d2414b19 100644 --- a/src/work_item_pipeline.rs +++ b/src/work_item_pipeline.rs @@ -106,24 +106,72 @@ impl std::fmt::Display for JudgeParseError { } } +/// The first fenced code block in `reply` (` ```json ... ``` ` or a bare +/// ` ``` ... ``` `), if any -- an explicit fence is an unambiguous boundary +/// the judge only produces on purpose, so it's tried before brace-scanning. +fn extract_fenced_block(reply: &str) -> Option<&str> { + let after_open = reply.find("```")? + 3; + let rest = &reply[after_open..]; + // Skip an optional language tag (e.g. `json`) up to the fence's newline. + let body_start = rest.find('\n').map(|i| i + 1).unwrap_or(0); + let body = &rest[body_start..]; + let end = body.find("```")?; + Some(body[..end].trim()) +} + +/// Scans forward from the first `{` for its own matching `}`, tracking +/// nesting depth and skipping brace-like bytes inside JSON string literals +/// (so a `{`/`}` embedded in a string value, or in unrelated commentary +/// after the object, can't extend or corrupt the span). Returns the first +/// complete top-level object instead of naively spanning from the first `{` +/// to the *last* `}` anywhere in the reply, which a second unrelated +/// brace-shaped span later in the text could throw off. +fn extract_first_balanced_object(reply: &str) -> Option<&str> { + let start = reply.find('{')?; + let bytes = reply.as_bytes(); + let mut depth = 0i32; + let mut in_string = false; + let mut escaped = false; + for (i, &b) in bytes.iter().enumerate().skip(start) { + if in_string { + match b { + _ if escaped => escaped = false, + b'\\' => escaped = true, + b'"' => in_string = false, + _ => {} + } + continue; + } + match b { + b'"' => in_string = true, + b'{' => depth += 1, + b'}' => { + depth -= 1; + if depth == 0 { + return Some(&reply[start..=i]); + } + } + _ => {} + } + } + None +} + /// The judge is prompted to reply with exactly one JSON object; this -/// tolerates a reply that wraps the object in a sentence by extracting the -/// first `{...}` span before parsing, but does not otherwise repair -/// malformed JSON — a genuine parse failure is a step Failure, retried by -/// the step's own RetryPolicy. +/// tolerates a reply that wraps the object in prose, a fenced code block, or +/// trailing commentary containing its own unrelated braces, but does not +/// otherwise repair malformed JSON — a genuine parse failure (including +/// syntactically valid JSON missing a required field) is a step Failure, +/// retried by the step's own RetryPolicy. pub(crate) fn parse_judge_decision(reply: &str) -> Result { - let start = reply - .find('{') - .ok_or_else(|| JudgeParseError::InvalidJson("no '{' found".to_string()))?; - let end = reply - .rfind('}') - .ok_or_else(|| JudgeParseError::InvalidJson("no '}' found".to_string()))?; - if end < start { - return Err(JudgeParseError::InvalidJson( - "unbalanced braces".to_string(), - )); + if let Some(fenced) = extract_fenced_block(reply) + && let Ok(decision) = serde_json::from_str(fenced) + { + return Ok(decision); } - let candidate = &reply[start..=end]; + let candidate = extract_first_balanced_object(reply).ok_or_else(|| { + JudgeParseError::InvalidJson("no balanced '{...}' object found".to_string()) + })?; serde_json::from_str(candidate).map_err(|e| JudgeParseError::InvalidJson(e.to_string())) } @@ -1344,6 +1392,55 @@ mod judge_decision_tests { let err = parse_judge_decision(reply).unwrap_err(); assert!(matches!(err, JudgeParseError::InvalidJson(_))); } + + #[test] + fn parses_decision_from_a_json_fenced_code_block() { + let reply = "Here's my decision:\n```json\n{\"action\":\"advance_task\",\"rationale\":\"spec met\",\"ledger_line\":\"Task 0: complete\",\"task_model_tier\":null}\n```\nThanks."; + let decision = parse_judge_decision(reply).expect("parses fenced block"); + assert_eq!(decision.action, JudgeAction::AdvanceTask); + } + + #[test] + fn parses_decision_from_a_bare_fenced_code_block_with_no_language_tag() { + let reply = "```\n{\"action\":\"skip_task\",\"rationale\":\"x\",\"ledger_line\":\"x\",\"task_model_tier\":null}\n```"; + let decision = parse_judge_decision(reply).expect("parses bare fenced block"); + assert_eq!(decision.action, JudgeAction::SkipTask); + } + + #[test] + fn extracts_only_the_first_object_when_trailing_prose_has_its_own_unrelated_braces() { + // A naive first-`{`-to-last-`}` span would run from the real + // object's opening brace all the way through the unrelated `{cfg}` + // in the trailing sentence, producing invalid combined JSON. + let reply = "{\"action\":\"advance_task\",\"rationale\":\"spec met\",\"ledger_line\":\"Task 0: complete\",\"task_model_tier\":null}\nNote: this respects the {cfg} override."; + let decision = + parse_judge_decision(reply).expect("parses despite trailing unrelated braces"); + assert_eq!(decision.action, JudgeAction::AdvanceTask); + } + + #[test] + fn a_brace_inside_a_json_string_value_does_not_confuse_balance_tracking() { + let reply = r#"{"action":"advance_task","rationale":"uses a {placeholder} pattern","ledger_line":"x","task_model_tier":null}"#; + let decision = parse_judge_decision(reply).expect("brace inside string value is inert"); + assert_eq!(decision.action, JudgeAction::AdvanceTask); + } + + #[test] + fn rejects_syntactically_valid_json_missing_a_required_field() { + // Reproduction of the live 2026-08-15 production failure (item + // #478): a well-formed JSON object that's simply missing `action` + // must still be a genuine, retried parse failure -- not silently + // defaulted. + let reply = r#"{"rationale":"x","ledger_line":"x","task_model_tier":null}"#; + let err = parse_judge_decision(reply).unwrap_err(); + assert!(matches!(err, JudgeParseError::InvalidJson(msg) if msg.contains("action"))); + } + + #[test] + fn rejects_an_empty_reply() { + let err = parse_judge_decision("").unwrap_err(); + assert!(matches!(err, JudgeParseError::InvalidJson(_))); + } } /// Shared fixtures for `sdd_loop_tests` (this task) and later plan tasks'