From 4dbca61e200d0e1f048e875e87dd3f12a69f95c3 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Mon, 18 May 2026 12:41:10 -0700 Subject: [PATCH] fix: default unknown tool schemas to empty schemas (#22380) ## Why Some tool providers, especially MCP servers and dynamic tool sources, can supply schema nodes that omit `type` and have no recognized JSON Schema shape hints. Previously, `sanitize_json_schema` filled those unknown nodes in as `string`, which made the schema parseable but invented a scalar constraint that the provider did not specify. For description-only fields, that could incorrectly steer tool arguments away from the provider's actual accepted shape. The Responses API accepts permissive empty schemas such as `{}` at nested property positions, so Codex should preserve that permissive meaning instead of coercing unknown schema nodes into a misleading scalar type. ## What Changed - Changed the no-hints fallback in `codex-rs/tools/src/json_schema.rs` to clear unrecognized object schema nodes to `{}`. - Empty schemas now remain `{}` rather than becoming `type: "string"`. - Description-only or otherwise metadata-only nested property schemas now become `{}` while surrounding object/array/string/number inference still applies when recognized hints are present. - Updated `codex-tools` and `codex-core` tests to cover top-level empty schemas, nested empty schemas, metadata-only malformed schemas, dynamic tools, and MCP tool specs. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-core test_mcp_tool_property_missing_type_defaults_to_empty_schema` - Manually verified the real Responses API behavior for both empty-schema positions: - Top-level function `parameters: {}` is accepted and echoed back as `{"type":"object","properties":{}}`; when forced to call the tool, Responses emitted empty object arguments: `"arguments": "{}"`. - Nested property schema `{}` is accepted and preserved as `{}`; when forced to call a tool with `metadata.extra`, Responses emitted `"arguments": "{\"metadata\":{\"extra\":\"codex schema sanitizer behavior\"}}"`. --- .../core/src/tools/spec_plan_model_tests.rs | 7 +- codex-rs/tools/src/dynamic_tool_tests.rs | 5 +- codex-rs/tools/src/json_schema.rs | 4 +- codex-rs/tools/src/json_schema_tests.rs | 82 +++++++++++++++++-- 4 files changed, 79 insertions(+), 19 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 86692e48df..3390d26e0d 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 077a0622ce..284ce4853b 100644 --- a/codex-rs/tools/src/dynamic_tool_tests.rs +++ b/codex-rs/tools/src/dynamic_tool_tests.rs @@ -27,10 +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::string(Some("Ticket identifier".to_string()),), - )]), + 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 22a641491e..ecd880cc7a 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,7 +229,8 @@ fn sanitize_json_schema(value: &mut JsonValue) { { schema_types.push(JsonSchemaPrimitiveType::Number); } else { - schema_types.push(JsonSchemaPrimitiveType::String); + map.clear(); + return; } } diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index 3f13df7638..5daaf048d0 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 a string type. + // - 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,16 +46,33 @@ fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() { assert_eq!( schema, JsonSchema::object( - BTreeMap::from([( - "query".to_string(), - JsonSchema::string(Some("search query".to_string())), - )]), + 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: @@ -250,16 +268,62 @@ 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_preserves_empty_schema() { // Example schema shape: // {} // // Expected normalization behavior: - // - With no structural hints at all, the normalizer falls back to a - // permissive string 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::string(/*description*/ None)); + assert_eq!(schema, JsonSchema::default()); +} + +#[test] +fn parse_tool_input_schema_preserves_nested_empty_schema() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "metadata": { + // "properties": { + // "extra": {} + // } + // } + // } + // } + // + // Expected normalization behavior: + // - The sanitizer recurses through nested object properties. + // - The innermost `extra` field is an empty JSON Schema and stays empty. + 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::default())]), + /*required*/ None, + /*additional_properties*/ None, + ) + )]), + /*required*/ None, + /*additional_properties*/ None, + ) + ); } #[test]