diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index 29740a02b8..559a5a0044 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -673,6 +673,13 @@ fn singleton_null_schema_error() -> serde_json::Error { )) } +#[cfg(test)] +pub(crate) use crate::json_schema_structured_outputs::validate_structured_outputs_schema; + #[cfg(test)] #[path = "json_schema_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "json_schema_structured_outputs_tests.rs"] +mod structured_outputs_tests; diff --git a/codex-rs/tools/src/json_schema_structured_outputs.rs b/codex-rs/tools/src/json_schema_structured_outputs.rs new file mode 100644 index 0000000000..62a22f6c38 --- /dev/null +++ b/codex-rs/tools/src/json_schema_structured_outputs.rs @@ -0,0 +1,103 @@ +use crate::AdditionalProperties; +use crate::JsonSchema; +use crate::JsonSchemaPrimitiveType; +use crate::JsonSchemaType; +use std::collections::BTreeSet; + +/// Validates the subset of JSON Schema invariants required by OpenAI +/// Structured Outputs for the object-heavy MCP regression tests in this crate. +/// +/// Source of truth: +/// https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas +/// +/// This validator currently focuses on the object constraints that matter for +/// the `start` / `end` nested-object regression: +/// - the root schema must be an object and must not use root-level `anyOf` +/// - every object must set `additionalProperties: false` +/// - every object property must appear in `required` +/// - nested `anyOf` branches and array items must themselves satisfy the same +/// subset whenever they contain objects +/// +/// It intentionally does not yet enforce the documented global size limits +/// (property count, nesting depth, enum count, total string budget). Those are +/// broader policy checks and can be layered on later without changing the +/// regression tests that pin the object-shape bug fixed in PR #18159. +pub(crate) fn validate_structured_outputs_schema(schema: &JsonSchema) -> Result<(), String> { + validate_structured_outputs_schema_at_path(schema, "root", /*is_root*/ true) +} + +fn validate_structured_outputs_schema_at_path( + schema: &JsonSchema, + path: &str, + is_root: bool, +) -> Result<(), String> { + if is_root { + if schema.any_of.is_some() { + return Err(format!( + "{path}: root schema must not use `anyOf`; see https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas" + )); + } + + if schema.schema_type != Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) { + return Err(format!( + "{path}: root schema must be an object; see https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas" + )); + } + } + + if let Some(any_of) = &schema.any_of { + for (index, variant) in any_of.iter().enumerate() { + validate_structured_outputs_schema_at_path( + variant, + &format!("{path}.anyOf[{index}]"), + /*is_root*/ false, + )?; + } + } + + if let Some(items) = &schema.items { + validate_structured_outputs_schema_at_path( + items, + &format!("{path}.items"), + /*is_root*/ false, + )?; + } + + if let Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)) = &schema.schema_type { + let Some(properties) = schema.properties.as_ref() else { + return Err(format!( + "{path}: object schemas must carry a properties map" + )); + }; + + if schema.additional_properties != Some(AdditionalProperties::Boolean(false)) { + return Err(format!( + "{path}: object schemas must set `additionalProperties: false`; see https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas" + )); + } + + let property_names = properties.keys().cloned().collect::>(); + let Some(required) = schema.required.as_ref() else { + return Err(format!( + "{path}: object schemas must list every field in `required`; see https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas" + )); + }; + let required_names = required.iter().cloned().collect::>(); + + if required_names != property_names { + return Err(format!( + "{path}: object schema `required` entries must exactly match declared properties; see https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas" + )); + } + + for (property_name, property_schema) in properties { + validate_structured_outputs_schema_at_path( + property_schema, + &format!("{path}.{property_name}"), + /*is_root*/ false, + )?; + } + } + + Ok(()) +} diff --git a/codex-rs/tools/src/json_schema_structured_outputs_tests.rs b/codex-rs/tools/src/json_schema_structured_outputs_tests.rs new file mode 100644 index 0000000000..5b8a3e427c --- /dev/null +++ b/codex-rs/tools/src/json_schema_structured_outputs_tests.rs @@ -0,0 +1,144 @@ +use super::JsonSchema; +use super::parse_tool_input_schema; +use super::validate_structured_outputs_schema; +use pretty_assertions::assert_eq; +use std::collections::BTreeMap; + +// This file codifies the Structured Outputs invariants that matter for the MCP +// schema regression in PR #18159. +// +// Source of truth: +// https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas +// +// The doc page calls out several constraints that are relevant to the object +// schemas we expose through the Responses API: +// - the root schema must be an object and must not be `anyOf` +// - all object fields / function parameters must appear in `required` +// - every object must set `additionalProperties: false` +// +// The flattening bug in this PR was specifically about `$ref` and +// single-variant combiner wrappers causing nested object parameters like +// `start` / `end` to collapse to `string`. The tests below intentionally use +// Structured Outputs-compliant inputs so we can assert not only that the object +// shape survives, but also that the surviving shape still lives inside the +// Responses API subset documented above. + +#[test] +fn parse_tool_input_schema_keeps_local_ref_objects_inside_structured_outputs_subset() { + // This mirrors the Outlook Calendar `create_event.start` / `end` shape we + // care about, but does so with the exact Structured Outputs object + // invariants from: + // https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas + // + // We want to prove that resolving a local `$ref` no longer collapses the + // nested object to `string`, while also preserving: + // - root object shape + // - `additionalProperties: false` + // - `required` coverage for every field + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "start": { "$ref": "#/$defs/date_time_zone" } + }, + "required": ["start"], + "additionalProperties": false, + "$defs": { + "date_time_zone": { + "type": "object", + "properties": { + "dateTime": { "type": "string" }, + "timeZone": { "type": "string" } + }, + "required": ["dateTime", "timeZone"], + "additionalProperties": false + } + } + })) + .expect("parse schema"); + + validate_structured_outputs_schema(&schema).expect("schema should stay in supported subset"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "start".to_string(), + JsonSchema::object( + BTreeMap::from([ + ( + "dateTime".to_string(), + JsonSchema::string(/*description*/ None), + ), + ( + "timeZone".to_string(), + JsonSchema::string(/*description*/ None), + ), + ]), + Some(vec!["dateTime".to_string(), "timeZone".to_string()]), + Some(false.into()), + ), + )]), + Some(vec!["start".to_string()]), + Some(false.into()), + ) + ); +} + +#[test] +fn parse_tool_input_schema_keeps_single_variant_combiner_objects_inside_structured_outputs_subset() +{ + // The docs allow nested objects, but those nested objects still need the + // same strict object invariants: + // https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas + // + // This test covers the second half of the regression: a single-variant + // `allOf` wrapper around the `DateTimeTimeZone` object must unwrap back to + // an object without losing the Structured Outputs constraints that make the + // schema acceptable to the Responses API. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "end": { + "allOf": [{ + "type": "object", + "properties": { + "dateTime": { "type": "string" }, + "timeZone": { "type": "string" } + }, + "required": ["dateTime", "timeZone"], + "additionalProperties": false + }] + } + }, + "required": ["end"], + "additionalProperties": false + })) + .expect("parse schema"); + + validate_structured_outputs_schema(&schema).expect("schema should stay in supported subset"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "end".to_string(), + JsonSchema::object( + BTreeMap::from([ + ( + "dateTime".to_string(), + JsonSchema::string(/*description*/ None), + ), + ( + "timeZone".to_string(), + JsonSchema::string(/*description*/ None), + ), + ]), + Some(vec!["dateTime".to_string(), "timeZone".to_string()]), + Some(false.into()), + ), + )]), + Some(vec!["end".to_string()]), + Some(false.into()), + ) + ); +} diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index b7c6a57d2b..bf57902930 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -25,6 +25,9 @@ mod tool_suggest; mod utility_tool; mod view_image; +#[cfg(test)] +mod json_schema_structured_outputs; + pub use agent_job_tool::create_report_agent_job_result_tool; pub use agent_job_tool::create_spawn_agents_on_csv_tool; pub use agent_tool::SpawnAgentToolOptions;