diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index efca321642..f145e566b7 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -283,6 +283,9 @@ fn resolve_json_schema_reference( let reference = map.get("$ref")?.as_str()?; let pointer = reference.strip_prefix('#')?; let mut replacement = root_schema.pointer(pointer)?.clone(); + if matches!(replacement, JsonValue::Bool(true)) { + replacement = JsonValue::Object(serde_json::Map::new()); + } if let JsonValue::Object(replacement_map) = &mut replacement { merge_json_schema_objects(replacement_map, map); } @@ -425,10 +428,13 @@ fn merge_schema_types(base_value: &mut JsonValue, overlay_value: &JsonValue) { return; } - let merged_types: Vec = base_types - .into_iter() - .filter(|base_type| overlay_types.contains(base_type)) - .collect(); + let merged_types: Vec = [ + "string", "number", "boolean", "integer", "object", "array", "null", + ] + .into_iter() + .filter(|candidate| type_sets_overlap(&base_types, &overlay_types, candidate)) + .map(str::to_string) + .collect(); if merged_types.is_empty() { return; } @@ -444,6 +450,26 @@ fn merge_schema_types(base_value: &mut JsonValue, overlay_value: &JsonValue) { }; } +fn type_sets_overlap(base_types: &[String], overlay_types: &[String], candidate: &str) -> bool { + if base_types.iter().any(|base_type| base_type == candidate) + && overlay_types + .iter() + .any(|overlay_type| overlay_type == candidate) + { + return true; + } + + candidate == "integer" + && ((base_types.iter().any(|base_type| base_type == "integer") + && overlay_types + .iter() + .any(|overlay_type| overlay_type == "number")) + || (base_types.iter().any(|base_type| base_type == "number") + && overlay_types + .iter() + .any(|overlay_type| overlay_type == "integer"))) +} + fn json_schema_type_names(value: &JsonValue) -> Vec { match value { JsonValue::String(schema_type) => vec![schema_type.clone()], @@ -499,13 +525,8 @@ fn unwrap_single_variant_combiner(map: &serde_json::Map) -> O let mut replacement = variants[0].clone(); if let JsonValue::Object(replacement_map) = &mut replacement { - for (key, value) in map { - if key != combiner { - replacement_map - .entry(key.clone()) - .or_insert_with(|| value.clone()); - } - } + merge_json_schema_objects(replacement_map, map); + replacement_map.remove(combiner); } return Some(replacement); } diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index 798321afc5..12bd157bcc 100644 --- a/codex-rs/tools/src/json_schema_tests.rs +++ b/codex-rs/tools/src/json_schema_tests.rs @@ -609,6 +609,119 @@ fn parse_tool_input_schema_handles_cyclic_local_refs_without_recursing_forever() ); } +#[test] +fn parse_tool_input_schema_merges_outer_constraints_when_unwrapping_single_variant_combiners() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "wrapped": { + "allOf": [{ + "type": "object", + "properties": { + "a": { "type": "string" } + }, + "required": ["a"] + }], + "properties": { + "b": { "type": "string" } + }, + "required": ["b"] + } + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "wrapped".to_string(), + JsonSchema::object( + BTreeMap::from([ + ("a".to_string(), JsonSchema::string(/*description*/ None)), + ("b".to_string(), JsonSchema::string(/*description*/ None)), + ]), + Some(vec!["a".to_string(), "b".to_string()]), + /*additional_properties*/ None, + ), + )]), + /*required*/ None, + /*additional_properties*/ None + ) + ); +} + +#[test] +fn parse_tool_input_schema_preserves_sibling_constraints_for_true_ref_targets() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "config": { + "$ref": "#/$defs/anything", + "type": "object", + "properties": { + "dateTime": { "type": "string" } + }, + "required": ["dateTime"] + } + }, + "$defs": { + "anything": true + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "config".to_string(), + JsonSchema::object( + BTreeMap::from([( + "dateTime".to_string(), + JsonSchema::string(/*description*/ None), + )]), + Some(vec!["dateTime".to_string()]), + /*additional_properties*/ None, + ), + )]), + /*required*/ None, + /*additional_properties*/ None + ) + ); +} + +#[test] +fn parse_tool_input_schema_narrows_number_and_integer_type_intersections() { + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "count": { + "$ref": "#/$defs/number_like", + "type": "integer" + } + }, + "$defs": { + "number_like": { + "type": "number" + } + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "count".to_string(), + JsonSchema::integer(/*description*/ None), + )]), + /*required*/ None, + /*additional_properties*/ None + ) + ); +} + #[test] fn parse_tool_input_schema_unwraps_single_variant_all_of_objects() { let schema = parse_tool_input_schema(&serde_json::json!({