From 813e73a0bb720b62fde767e5b48d213edca7534d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 28 Jul 2026 07:50:31 -0400 Subject: [PATCH 1/2] fix(workflow): validate outermost custom output object --- .../src/handler/structured_output.rs | 67 ++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/lib/components/fabro-workflow/src/handler/structured_output.rs b/lib/components/fabro-workflow/src/handler/structured_output.rs index d36435bf3..888256b21 100644 --- a/lib/components/fabro-workflow/src/handler/structured_output.rs +++ b/lib/components/fabro-workflow/src/handler/structured_output.rs @@ -226,6 +226,16 @@ pub(crate) fn apply_validated_output( /// Find all balanced `{...}` JSON object substrings in the text. fn find_json_objects(text: &str) -> Vec<&str> { + find_json_objects_with_nested(text, true) +} + +/// Find balanced `{...}` JSON object substrings in the text, skipping objects +/// nested within a previous match. +fn find_outermost_json_objects(text: &str) -> Vec<&str> { + find_json_objects_with_nested(text, false) +} + +fn find_json_objects_with_nested(text: &str, include_nested: bool) -> Vec<&str> { let mut results = Vec::new(); let bytes = text.as_bytes(); let mut i = 0; @@ -251,6 +261,9 @@ fn find_json_objects(text: &str) -> Vec<&str> { depth -= 1; if depth == 0 { results.push(&text[start..=j]); + if !include_nested { + i = j; + } break; } } @@ -334,7 +347,7 @@ fn validate_custom_response_text( validator: &Validator, text: &str, ) -> Result { - let candidates = find_json_objects(text); + let candidates = find_outermost_json_objects(text); let Some(candidate) = candidates.last() else { return Err(StructuredOutputError::new( StructuredOutputErrorKind::NoJsonObject, @@ -558,6 +571,58 @@ mod tests { assert_eq!(validated.value, serde_json::json!({"passed": true})); } + #[test] + fn validates_custom_schema_against_outermost_object() { + let schema = schema(serde_json::json!({ + "type": "object", + "required": ["issue"], + "properties": { + "issue": { + "type": "object", + "required": ["number"], + "properties": { + "number": { "type": "integer" } + } + } + } + })); + + let validated = validate_response_text(&schema, r#"{"issue":{"number":19}}"#).unwrap(); + + assert_eq!( + validated.value, + serde_json::json!({"issue": {"number": 19}}) + ); + } + + #[test] + fn validates_last_outermost_object_when_response_has_trailing_prose() { + let schema = schema(serde_json::json!({ + "type": "object", + "required": ["issue"], + "properties": { + "issue": { + "type": "object", + "required": ["number"], + "properties": { + "number": { "type": "integer" } + } + } + } + })); + + let validated = validate_response_text( + &schema, + r#"ignore {"issue":{"number":1}} final {"issue":{"number":19}} trailing"#, + ) + .unwrap(); + + assert_eq!( + validated.value, + serde_json::json!({"issue": {"number": 19}}) + ); + } + #[test] fn custom_schema_validation_errors_are_reported() { let schema = schema(serde_json::json!({ From 7bacd4cf12f4a638e78419273987c9b606a5254f Mon Sep 17 00:00:00 2001 From: Release Repro Date: Tue, 28 Jul 2026 16:10:47 -0400 Subject: [PATCH 2/2] refactor(workflow): scan for outermost JSON objects everywhere Replace the include_nested flag and its two wrapper functions with a single outermost-only scanner. Routing extraction used the nested scan and reverse iteration, so a routing object nested inside a wrapper could win over its parent -- the same bug class this branch fixes for custom schemas. No caller needs nested candidates. Custom schema validation now walks candidates from the end and takes the last one that parses, instead of parsing only the final candidate. Prose after the object can contain braces, and outermost-only scanning made that trailing text a candidate that shadowed the real JSON. Schema errors are still reported from the last parsable object, so an earlier object that happens to validate cannot mask a later violation. Add direct scanner coverage for nesting, adjacent objects, unclosed braces, and braces inside strings. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/handler/structured_output.rs | 152 +++++++++++------- 1 file changed, 94 insertions(+), 58 deletions(-) diff --git a/lib/components/fabro-workflow/src/handler/structured_output.rs b/lib/components/fabro-workflow/src/handler/structured_output.rs index 888256b21..fc89f6581 100644 --- a/lib/components/fabro-workflow/src/handler/structured_output.rs +++ b/lib/components/fabro-workflow/src/handler/structured_output.rs @@ -224,18 +224,13 @@ pub(crate) fn apply_validated_output( } } -/// Find all balanced `{...}` JSON object substrings in the text. +/// Find the outermost balanced `{...}` JSON object substrings in the text, in +/// document order. Objects nested inside a match are skipped. +/// +/// An unbalanced `{` does not suppress complete objects around or inside it: +/// the scan only skips ahead past a *matched* object, so it still walks into a +/// region that failed to close. fn find_json_objects(text: &str) -> Vec<&str> { - find_json_objects_with_nested(text, true) -} - -/// Find balanced `{...}` JSON object substrings in the text, skipping objects -/// nested within a previous match. -fn find_outermost_json_objects(text: &str) -> Vec<&str> { - find_json_objects_with_nested(text, false) -} - -fn find_json_objects_with_nested(text: &str, include_nested: bool) -> Vec<&str> { let mut results = Vec::new(); let bytes = text.as_bytes(); let mut i = 0; @@ -261,9 +256,7 @@ fn find_json_objects_with_nested(text: &str, include_nested: bool) -> Vec<&str> depth -= 1; if depth == 0 { results.push(&text[start..=j]); - if !include_nested { - i = j; - } + i = j; break; } } @@ -282,7 +275,8 @@ pub(crate) fn terminal_json_object(text: &str) -> Option<&str> { let trimmed = text.trim_end(); find_json_objects(trimmed) .into_iter() - .find(|candidate| trimmed.ends_with(candidate)) + .next_back() + .filter(|candidate| trimmed.ends_with(candidate)) } pub(crate) fn extract_status_fields(text: &str, outcome: &mut Outcome) -> bool { @@ -347,21 +341,32 @@ fn validate_custom_response_text( validator: &Validator, text: &str, ) -> Result { - let candidates = find_outermost_json_objects(text); - let Some(candidate) = candidates.last() else { - return Err(StructuredOutputError::new( + // Prose after the object can contain braces, so the last candidate is not + // always JSON. Take the last one that parses; report its schema errors + // rather than falling back to an earlier object that happens to validate. + let candidates = find_json_objects(text); + let mut invalid_json = None; + for candidate in candidates.iter().rev() { + match serde_json::from_str::(candidate) { + Ok(parsed) => { + validate_value_against_validator(validator, &parsed)?; + return Ok(ValidatedStructuredOutput { value: parsed }); + } + Err(err) if invalid_json.is_none() => invalid_json = Some(err.to_string()), + Err(_) => {} + } + } + + Err(match invalid_json { + Some(message) => StructuredOutputError::new( + StructuredOutputErrorKind::InvalidJson, + format!("invalid JSON object: {message}"), + ), + None => StructuredOutputError::new( StructuredOutputErrorKind::NoJsonObject, "no JSON object found in response", - )); - }; - let parsed = serde_json::from_str::(candidate).map_err(|err| { - StructuredOutputError::new( - StructuredOutputErrorKind::InvalidJson, - format!("invalid JSON object: {err}"), - ) - })?; - validate_value_against_validator(validator, &parsed)?; - Ok(ValidatedStructuredOutput { value: parsed }) + ), + }) } fn validate_value_against_validator( @@ -477,6 +482,24 @@ mod tests { } } + /// A schema whose required field is itself an object, so validating the + /// innermost `{...}` in the response would fail. + fn issue_schema() -> OutputSchemaKind { + schema(serde_json::json!({ + "type": "object", + "required": ["issue"], + "properties": { + "issue": { + "type": "object", + "required": ["number"], + "properties": { + "number": { "type": "integer" } + } + } + } + })) + } + #[test] fn validates_routing_json_and_applies_fields() { let validated = validate_response_text( @@ -573,21 +596,8 @@ mod tests { #[test] fn validates_custom_schema_against_outermost_object() { - let schema = schema(serde_json::json!({ - "type": "object", - "required": ["issue"], - "properties": { - "issue": { - "type": "object", - "required": ["number"], - "properties": { - "number": { "type": "integer" } - } - } - } - })); - - let validated = validate_response_text(&schema, r#"{"issue":{"number":19}}"#).unwrap(); + let validated = + validate_response_text(&issue_schema(), r#"{"issue":{"number":19}}"#).unwrap(); assert_eq!( validated.value, @@ -597,22 +607,8 @@ mod tests { #[test] fn validates_last_outermost_object_when_response_has_trailing_prose() { - let schema = schema(serde_json::json!({ - "type": "object", - "required": ["issue"], - "properties": { - "issue": { - "type": "object", - "required": ["number"], - "properties": { - "number": { "type": "integer" } - } - } - } - })); - let validated = validate_response_text( - &schema, + &issue_schema(), r#"ignore {"issue":{"number":1}} final {"issue":{"number":19}} trailing"#, ) .unwrap(); @@ -623,6 +619,46 @@ mod tests { ); } + #[test] + fn validates_last_parsable_object_when_trailing_prose_contains_braces() { + let schema = schema(serde_json::json!({ + "type": "object", + "required": ["passed"], + "properties": { + "passed": { "type": "boolean" } + } + })); + + let validated = validate_response_text( + &schema, + "{\"passed\":true}\n\nLet me know if {this works} for you.", + ) + .unwrap(); + + assert_eq!(validated.value, serde_json::json!({"passed": true})); + } + + #[test] + fn find_json_objects_returns_outermost_objects_only() { + let cases = [ + (r#"{"a":{"b":1}}"#, vec![r#"{"a":{"b":1}}"#]), + (r#"{"a":1} {"b":2}"#, vec![r#"{"a":1}"#, r#"{"b":2}"#]), + (r#"{"a":1}{"b":2}"#, vec![r#"{"a":1}"#, r#"{"b":2}"#]), + // An unclosed outer brace must not hide the complete object inside it. + (r#"{ {"a":1}"#, vec![r#"{"a":1}"#]), + (r#"{"a":1} {"#, vec![r#"{"a":1}"#]), + // An unterminated string swallows the rest of its own candidate. + (r#"{"a": "x} {"b":2}"#, vec![r#"{"b":2}"#]), + // Braces inside strings are not delimiters. + (r#"{"a":"} {"}"#, vec![r#"{"a":"} {"}"#]), + ("no json here", vec![]), + ]; + + for (text, expected) in cases { + assert_eq!(find_json_objects(text), expected, "input: {text}"); + } + } + #[test] fn custom_schema_validation_errors_are_reported() { let schema = schema(serde_json::json!({