From 75965a6e151fa95d22c7f478634ad57f4cf70a62 Mon Sep 17 00:00:00 2001 From: celia-oai Date: Tue, 12 May 2026 13:49:31 -0700 Subject: [PATCH 1/2] changes --- codex-rs/tools/src/dynamic_tool_tests.rs | 10 +++- codex-rs/tools/src/json_schema.rs | 5 +- codex-rs/tools/src/json_schema_tests.rs | 73 ++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/codex-rs/tools/src/dynamic_tool_tests.rs b/codex-rs/tools/src/dynamic_tool_tests.rs index 077a0622cefb..7d212342abe3 100644 --- a/codex-rs/tools/src/dynamic_tool_tests.rs +++ b/codex-rs/tools/src/dynamic_tool_tests.rs @@ -29,7 +29,15 @@ fn parse_dynamic_tool_sanitizes_input_schema() { input_schema: JsonSchema::object( BTreeMap::from([( "id".to_string(), - JsonSchema::string(Some("Ticket identifier".to_string()),), + JsonSchema { + schema_type: Some(crate::JsonSchemaType::Single( + crate::JsonSchemaPrimitiveType::Object, + )), + description: Some("Ticket identifier".to_string()), + properties: Some(BTreeMap::new()), + additional_properties: Some(true.into()), + ..Default::default() + }, )]), /*required*/ None, /*additional_properties*/ None diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index 22a641491e57..7d17c5335cc2 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -228,7 +228,10 @@ fn sanitize_json_schema(value: &mut JsonValue) { { schema_types.push(JsonSchemaPrimitiveType::Number); } else { - schema_types.push(JsonSchemaPrimitiveType::String); + // With no schema hints, fall back to an open object so unknown + // tool argument shapes can still carry arbitrary named fields. + schema_types.push(JsonSchemaPrimitiveType::Object); + map.insert("additionalProperties".to_string(), JsonValue::Bool(true)); } } diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index 3f13df76384f..c28e0c19ed46 100644 --- a/codex-rs/tools/src/json_schema_tests.rs +++ b/codex-rs/tools/src/json_schema_tests.rs @@ -34,7 +34,7 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() { // // Expected normalization behavior: // - `properties` implies an object schema when `type` is omitted. - // - The child property keeps its description and defaults to a string type. + // - The child property keeps its description and defaults to an open object. let schema = parse_tool_input_schema(&serde_json::json!({ "properties": { "query": {"description": "search query"} @@ -47,7 +47,13 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() { JsonSchema::object( BTreeMap::from([( "query".to_string(), - JsonSchema::string(Some("search query".to_string())), + JsonSchema { + schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), + description: Some("search query".to_string()), + properties: Some(BTreeMap::new()), + additional_properties: Some(true.into()), + ..Default::default() + }, )]), /*required*/ None, /*additional_properties*/ None @@ -250,16 +256,73 @@ fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() { } #[test] -fn parse_tool_input_schema_defaults_empty_schema_to_string() { +fn parse_tool_input_schema_defaults_empty_schema_to_open_object() { // Example schema shape: // {} // // Expected normalization behavior: // - With no structural hints at all, the normalizer falls back to a - // permissive string schema. + // permissive object schema. let schema = parse_tool_input_schema(&serde_json::json!({})).expect("parse schema"); - assert_eq!(schema, JsonSchema::string(/*description*/ None)); + assert_eq!( + schema, + JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(true.into())) + ); +} + +#[test] +fn parse_tool_input_schema_defaults_nested_empty_schema_to_open_object() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "metadata": { + // "properties": { + // "extra": {} + // } + // } + // } + // } + // + // Expected normalization behavior: + // - The sanitizer recurses through nested object properties. + // - The innermost `extra` field has no hints, so it falls back to an open + // object. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "metadata": { + "properties": { + "extra": {} + } + } + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "metadata".to_string(), + JsonSchema::object( + BTreeMap::from([( + "extra".to_string(), + JsonSchema::object( + BTreeMap::new(), + /*required*/ None, + Some(true.into()), + ), + )]), + /*required*/ None, + /*additional_properties*/ None, + ) + )]), + /*required*/ None, + /*additional_properties*/ None, + ) + ); } #[test] From 91009236b0ed4d5b1904831aec078e572d698636 Mon Sep 17 00:00:00 2001 From: celia-oai Date: Tue, 12 May 2026 15:50:19 -0700 Subject: [PATCH 2/2] test --- .../core/src/tools/spec_plan_model_tests.rs | 7 +-- codex-rs/tools/src/dynamic_tool_tests.rs | 13 +--- codex-rs/tools/src/json_schema.rs | 7 +-- codex-rs/tools/src/json_schema_tests.rs | 59 ++++++++++--------- 4 files changed, 36 insertions(+), 50 deletions(-) diff --git a/codex-rs/core/src/tools/spec_plan_model_tests.rs b/codex-rs/core/src/tools/spec_plan_model_tests.rs index 86692e48df0d..3390d26e0de8 100644 --- a/codex-rs/core/src/tools/spec_plan_model_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_model_tests.rs @@ -907,7 +907,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio } #[tokio::test] -async fn test_mcp_tool_property_missing_type_defaults_to_string() { +async fn test_mcp_tool_property_missing_type_defaults_to_empty_schema() { let config = test_config().await; let model_info = construct_model_info_offline("gpt-5.4", &config); let mut features = Features::with_defaults(); @@ -950,10 +950,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() { name: "search".to_string(), parameters: JsonSchema::object( /*properties*/ - BTreeMap::from([( - "query".to_string(), - JsonSchema::string(Some("search query".to_string())), - )]), + BTreeMap::from([("query".to_string(), JsonSchema::default())]), /*required*/ None, /*additional_properties*/ None ), diff --git a/codex-rs/tools/src/dynamic_tool_tests.rs b/codex-rs/tools/src/dynamic_tool_tests.rs index 7d212342abe3..284ce4853b4f 100644 --- a/codex-rs/tools/src/dynamic_tool_tests.rs +++ b/codex-rs/tools/src/dynamic_tool_tests.rs @@ -27,18 +27,7 @@ fn parse_dynamic_tool_sanitizes_input_schema() { name: "lookup_ticket".to_string(), description: "Fetch a ticket".to_string(), input_schema: JsonSchema::object( - BTreeMap::from([( - "id".to_string(), - JsonSchema { - schema_type: Some(crate::JsonSchemaType::Single( - crate::JsonSchemaPrimitiveType::Object, - )), - description: Some("Ticket identifier".to_string()), - properties: Some(BTreeMap::new()), - additional_properties: Some(true.into()), - ..Default::default() - }, - )]), + BTreeMap::from([("id".to_string(), JsonSchema::default(),)]), /*required*/ None, /*additional_properties*/ None ), diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index 7d17c5335cc2..ecd880cc7aa9 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -166,6 +166,7 @@ pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result { @@ -228,10 +229,8 @@ fn sanitize_json_schema(value: &mut JsonValue) { { schema_types.push(JsonSchemaPrimitiveType::Number); } else { - // With no schema hints, fall back to an open object so unknown - // tool argument shapes can still carry arbitrary named fields. - schema_types.push(JsonSchemaPrimitiveType::Object); - map.insert("additionalProperties".to_string(), JsonValue::Bool(true)); + map.clear(); + return; } } diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index c28e0c19ed46..5daaf048d09c 100644 --- a/codex-rs/tools/src/json_schema_tests.rs +++ b/codex-rs/tools/src/json_schema_tests.rs @@ -34,7 +34,8 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() { // // Expected normalization behavior: // - `properties` implies an object schema when `type` is omitted. - // - The child property keeps its description and defaults to an open object. + // - The child property has no recognized schema hints, so it is coerced to + // an empty permissive schema. let schema = parse_tool_input_schema(&serde_json::json!({ "properties": { "query": {"description": "search query"} @@ -45,22 +46,33 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() { assert_eq!( schema, JsonSchema::object( - BTreeMap::from([( - "query".to_string(), - JsonSchema { - schema_type: Some(JsonSchemaType::Single(JsonSchemaPrimitiveType::Object)), - description: Some("search query".to_string()), - properties: Some(BTreeMap::new()), - additional_properties: Some(true.into()), - ..Default::default() - }, - )]), + BTreeMap::from([("query".to_string(), JsonSchema::default())]), /*required*/ None, /*additional_properties*/ None ) ); } +#[test] +fn parse_tool_input_schema_coerces_unrecognized_object_schema_to_empty_schema() { + // Example schema shape: + // { + // "description": "Ticket identifier", + // "title": "Ticket ID" + // } + // + // Expected normalization behavior: + // - Object schemas with no recognized schema hints are treated as + // malformed and coerced to the empty permissive schema. + let schema = parse_tool_input_schema(&serde_json::json!({ + "description": "Ticket identifier", + "title": "Ticket ID" + })) + .expect("parse schema"); + + assert_eq!(schema, JsonSchema::default()); +} + #[test] fn parse_tool_input_schema_preserves_integer_and_defaults_array_items() { // Example schema shape: @@ -256,23 +268,20 @@ fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() { } #[test] -fn parse_tool_input_schema_defaults_empty_schema_to_open_object() { +fn parse_tool_input_schema_preserves_empty_schema() { // Example schema shape: // {} // // Expected normalization behavior: - // - With no structural hints at all, the normalizer falls back to a - // permissive object schema. + // - An empty JSON Schema is already a valid permissive schema, so it stays + // empty rather than being rewritten as an object schema. let schema = parse_tool_input_schema(&serde_json::json!({})).expect("parse schema"); - assert_eq!( - schema, - JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(true.into())) - ); + assert_eq!(schema, JsonSchema::default()); } #[test] -fn parse_tool_input_schema_defaults_nested_empty_schema_to_open_object() { +fn parse_tool_input_schema_preserves_nested_empty_schema() { // Example schema shape: // { // "type": "object", @@ -287,8 +296,7 @@ fn parse_tool_input_schema_defaults_nested_empty_schema_to_open_object() { // // Expected normalization behavior: // - The sanitizer recurses through nested object properties. - // - The innermost `extra` field has no hints, so it falls back to an open - // object. + // - The innermost `extra` field is an empty JSON Schema and stays empty. let schema = parse_tool_input_schema(&serde_json::json!({ "type": "object", "properties": { @@ -307,14 +315,7 @@ fn parse_tool_input_schema_defaults_nested_empty_schema_to_open_object() { BTreeMap::from([( "metadata".to_string(), JsonSchema::object( - BTreeMap::from([( - "extra".to_string(), - JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - Some(true.into()), - ), - )]), + BTreeMap::from([("extra".to_string(), JsonSchema::default())]), /*required*/ None, /*additional_properties*/ None, )