From 7b19a8c0cdcc558512bb34dae7595db137213e10 Mon Sep 17 00:00:00 2001 From: Soheil Norouzi Date: Mon, 11 May 2026 11:36:11 -0400 Subject: [PATCH 1/2] Preserve MCP JSON schema structure --- codex-rs/tools/src/dynamic_tool.rs | 4 +- codex-rs/tools/src/dynamic_tool_tests.rs | 28 ++-- codex-rs/tools/src/json_schema.rs | 180 ++++++++++++++++++++-- codex-rs/tools/src/json_schema_tests.rs | 114 +++++++++++++- codex-rs/tools/src/mcp_tool.rs | 4 +- codex-rs/tools/src/mcp_tool_tests.rs | 66 ++++++-- codex-rs/tools/src/responses_api_tests.rs | 113 ++++++++++++-- 7 files changed, 434 insertions(+), 75 deletions(-) diff --git a/codex-rs/tools/src/dynamic_tool.rs b/codex-rs/tools/src/dynamic_tool.rs index af74a3c1e388..53fce566002e 100644 --- a/codex-rs/tools/src/dynamic_tool.rs +++ b/codex-rs/tools/src/dynamic_tool.rs @@ -1,12 +1,12 @@ +use crate::JsonSchema; use crate::ToolDefinition; -use crate::parse_tool_input_schema; use codex_protocol::dynamic_tools::DynamicToolSpec; pub fn parse_dynamic_tool(tool: &DynamicToolSpec) -> Result { Ok(ToolDefinition { name: tool.name.clone(), description: tool.description.clone(), - input_schema: parse_tool_input_schema(&tool.input_schema)?, + input_schema: JsonSchema::from_raw_tool_input_schema(tool.input_schema.clone()), output_schema: None, defer_loading: tool.defer_loading, }) diff --git a/codex-rs/tools/src/dynamic_tool_tests.rs b/codex-rs/tools/src/dynamic_tool_tests.rs index 077a0622cefb..cdeec478faa2 100644 --- a/codex-rs/tools/src/dynamic_tool_tests.rs +++ b/codex-rs/tools/src/dynamic_tool_tests.rs @@ -3,10 +3,9 @@ use crate::JsonSchema; use crate::ToolDefinition; use codex_protocol::dynamic_tools::DynamicToolSpec; use pretty_assertions::assert_eq; -use std::collections::BTreeMap; #[test] -fn parse_dynamic_tool_sanitizes_input_schema() { +fn parse_dynamic_tool_preserves_raw_input_schema() { let tool = DynamicToolSpec { namespace: None, name: "lookup_ticket".to_string(), @@ -26,14 +25,14 @@ fn parse_dynamic_tool_sanitizes_input_schema() { ToolDefinition { 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()),), - )]), - /*required*/ None, - /*additional_properties*/ None - ), + input_schema: JsonSchema::from_raw_tool_input_schema(serde_json::json!({ + "type": "object", + "properties": { + "id": { + "description": "Ticket identifier" + } + } + })), output_schema: None, defer_loading: false, } @@ -58,11 +57,10 @@ fn parse_dynamic_tool_preserves_defer_loading() { ToolDefinition { name: "lookup_ticket".to_string(), description: "Fetch a ticket".to_string(), - input_schema: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), + input_schema: JsonSchema::from_raw_tool_input_schema(serde_json::json!({ + "type": "object", + "properties": {} + })), output_schema: None, defer_loading: true, } diff --git a/codex-rs/tools/src/json_schema.rs b/codex-rs/tools/src/json_schema.rs index 22a641491e57..c29aac254128 100644 --- a/codex-rs/tools/src/json_schema.rs +++ b/codex-rs/tools/src/json_schema.rs @@ -1,5 +1,7 @@ use serde::Deserialize; +use serde::Deserializer; use serde::Serialize; +use serde::Serializer; use serde_json::Value as JsonValue; use serde_json::json; use std::collections::BTreeMap; @@ -8,7 +10,8 @@ use std::collections::BTreeMap; /// /// This mirrors the OpenAI Structured Outputs subset for JSON Schema `type`: /// string, number, boolean, integer, object, array, and null. -/// Keywords such as `enum`, `const`, and `anyOf` are modeled separately. +/// Keywords such as `enum`, `const`, `anyOf`, `allOf`, and `oneOf` are modeled +/// separately. /// See . #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "lowercase")] @@ -31,30 +34,112 @@ pub enum JsonSchemaType { } /// Generic JSON-Schema subset needed for our tool definitions. -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, PartialEq)] pub struct JsonSchema { - #[serde(rename = "type", skip_serializing_if = "Option::is_none")] + raw_schema: Option, pub schema_type: Option, - #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, - #[serde(rename = "enum", skip_serializing_if = "Option::is_none")] pub enum_values: Option>, - #[serde(skip_serializing_if = "Option::is_none")] pub items: Option>, - #[serde(skip_serializing_if = "Option::is_none")] pub properties: Option>, - #[serde(skip_serializing_if = "Option::is_none")] pub required: Option>, + pub additional_properties: Option, + pub any_of: Option>, + pub all_of: Option>, + pub one_of: Option>, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +struct JsonSchemaFields { + #[serde(rename = "type", skip_serializing_if = "Option::is_none")] + schema_type: Option, + #[serde(skip_serializing_if = "Option::is_none")] + description: Option, + #[serde(rename = "enum", skip_serializing_if = "Option::is_none")] + enum_values: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + items: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + properties: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + required: Option>, #[serde( rename = "additionalProperties", skip_serializing_if = "Option::is_none" )] - pub additional_properties: Option, + additional_properties: Option, #[serde(rename = "anyOf", skip_serializing_if = "Option::is_none")] - pub any_of: Option>, + any_of: Option>, + #[serde(rename = "allOf", skip_serializing_if = "Option::is_none")] + all_of: Option>, + #[serde(rename = "oneOf", skip_serializing_if = "Option::is_none")] + one_of: Option>, +} + +impl From for JsonSchema { + fn from(fields: JsonSchemaFields) -> Self { + Self { + raw_schema: None, + schema_type: fields.schema_type, + description: fields.description, + enum_values: fields.enum_values, + items: fields.items, + properties: fields.properties, + required: fields.required, + additional_properties: fields.additional_properties, + any_of: fields.any_of, + all_of: fields.all_of, + one_of: fields.one_of, + } + } +} + +impl From<&JsonSchema> for JsonSchemaFields { + fn from(schema: &JsonSchema) -> Self { + Self { + schema_type: schema.schema_type.clone(), + description: schema.description.clone(), + enum_values: schema.enum_values.clone(), + items: schema.items.clone(), + properties: schema.properties.clone(), + required: schema.required.clone(), + additional_properties: schema.additional_properties.clone(), + any_of: schema.any_of.clone(), + all_of: schema.all_of.clone(), + one_of: schema.one_of.clone(), + } + } +} + +impl Serialize for JsonSchema { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + match &self.raw_schema { + Some(raw_schema) => raw_schema.serialize(serializer), + None => JsonSchemaFields::from(self).serialize(serializer), + } + } +} + +impl<'de> Deserialize<'de> for JsonSchema { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + JsonSchemaFields::deserialize(deserializer).map(Into::into) + } } impl JsonSchema { + pub fn from_raw_tool_input_schema(input_schema: JsonValue) -> Self { + Self { + raw_schema: Some(normalize_raw_tool_input_schema(input_schema)), + ..Default::default() + } + } + /// Construct a scalar/object/array schema with a single JSON Schema type. fn typed(schema_type: JsonSchemaPrimitiveType, description: Option) -> Self { Self { @@ -72,6 +157,14 @@ impl JsonSchema { } } + pub fn all_of(variants: Vec, description: Option) -> Self { + Self { + description, + all_of: Some(variants), + ..Default::default() + } + } + pub fn boolean(description: Option) -> Self { Self::typed(JsonSchemaPrimitiveType::Boolean, description) } @@ -159,17 +252,64 @@ pub fn parse_tool_input_schema(input_schema: &JsonValue) -> Result JsonValue { + let JsonValue::Object(mut map) = input_schema else { + return empty_root_tool_schema(); + }; + + if !root_schema_allows_object(&map) { + return empty_root_tool_schema(); + } + + if map.get("type").is_none_or(JsonValue::is_null) { + map.insert("type".to_string(), JsonValue::String("object".to_string())); + } + + if map.get("properties").is_none_or(JsonValue::is_null) { + map.insert( + "properties".to_string(), + JsonValue::Object(serde_json::Map::new()), + ); + } + + JsonValue::Object(map) +} + +fn empty_root_tool_schema() -> JsonValue { + json!({ + "type": "object", + "properties": {} + }) +} + +fn root_schema_allows_object(map: &serde_json::Map) -> bool { + match map.get("type") { + None | Some(JsonValue::Null) => true, + Some(JsonValue::String(schema_type)) => schema_type == "object", + Some(JsonValue::Array(schema_types)) => schema_types + .iter() + .any(|schema_type| schema_type.as_str() == Some("object")), + Some(_) => false, + } +} + /// Sanitize a JSON Schema (as serde_json::Value) so it can fit our limited /// schema representation. This function: /// - Ensures every typed schema object has a `"type"` when required. -/// - Preserves explicit `anyOf`. +/// - Preserves explicit `anyOf`, `allOf`, and `oneOf`. /// - Collapses `const` into single-value `enum`. /// - Fills required child fields for object/array schema types, including /// nullable unions, with permissive defaults when absent. fn sanitize_json_schema(value: &mut JsonValue) { match value { - JsonValue::Bool(_) => { - // JSON Schema boolean form: true/false. Coerce to an accept-all string. + JsonValue::Bool(true) => { + // JSON Schema `true` is an accept-all schema. + *value = json!({}); + } + JsonValue::Bool(false) => { + // `false` is unsatisfiable, which our tool schema subset cannot + // express. Keep the existing permissive fallback for that rare + // shape rather than rejecting the whole tool definition. *value = json!({ "type": "string" }); } JsonValue::Array(values) => { @@ -196,8 +336,10 @@ fn sanitize_json_schema(value: &mut JsonValue) { if let Some(value) = map.get_mut("prefixItems") { sanitize_json_schema(value); } - if let Some(value) = map.get_mut("anyOf") { - sanitize_json_schema(value); + for keyword in ["anyOf", "allOf", "oneOf"] { + if let Some(value) = map.get_mut(keyword) { + sanitize_json_schema(value); + } } if let Some(const_value) = map.remove("const") { @@ -206,7 +348,7 @@ fn sanitize_json_schema(value: &mut JsonValue) { let mut schema_types = normalized_schema_types(map); - if schema_types.is_empty() && map.contains_key("anyOf") { + if schema_types.is_empty() && (map.is_empty() || contains_schema_combiner(map)) { return; } @@ -239,6 +381,12 @@ fn sanitize_json_schema(value: &mut JsonValue) { } } +fn contains_schema_combiner(map: &serde_json::Map) -> bool { + ["anyOf", "allOf", "oneOf"] + .iter() + .any(|keyword| map.contains_key(*keyword)) +} + fn ensure_default_children_for_schema_types( map: &mut serde_json::Map, schema_types: &[JsonSchemaPrimitiveType], diff --git a/codex-rs/tools/src/json_schema_tests.rs b/codex-rs/tools/src/json_schema_tests.rs index 3f13df76384f..60886b7be369 100644 --- a/codex-rs/tools/src/json_schema_tests.rs +++ b/codex-rs/tools/src/json_schema_tests.rs @@ -10,16 +10,27 @@ use std::collections::BTreeMap; // formed JSON for consumption by the Responses API. #[test] -fn parse_tool_input_schema_coerces_boolean_schemas() { +fn parse_tool_input_schema_preserves_true_schema_as_any() { // Example schema shape: // true // // Expected normalization behavior: - // - JSON Schema boolean forms are coerced to `{ "type": "string" }` - // because the baseline enum model cannot represent boolean-schema - // semantics directly. + // - JSON Schema `true` is an accept-all schema and serializes as `{}`. let schema = parse_tool_input_schema(&serde_json::json!(true)).expect("parse schema"); + assert_eq!(schema, JsonSchema::default()); +} + +#[test] +fn parse_tool_input_schema_coerces_false_schema_to_string() { + // Example schema shape: + // false + // + // Expected normalization behavior: + // - JSON Schema `false` is unsatisfiable, which the current schema model + // cannot represent directly, so it keeps the existing permissive fallback. + let schema = parse_tool_input_schema(&serde_json::json!(false)).expect("parse schema"); + assert_eq!(schema, JsonSchema::string(/*description*/ None)); } @@ -250,16 +261,63 @@ 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_as_any() { // Example schema shape: // {} // // Expected normalization behavior: - // - With no structural hints at all, the normalizer falls back to a - // permissive string schema. + // - With no structural hints at all, `{}` remains an accept-all 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_empty_property_schema_as_any() { + // Example schema shape: + // { + // "type": "object", + // "properties": { + // "body": { + // "type": "object", + // "properties": { + // "parent": {} + // } + // } + // } + // } + // + // Expected normalization behavior: + // - Nested `{}` keeps its JSON Schema accept-all meaning instead of being + // rewritten into a string. + let schema = parse_tool_input_schema(&serde_json::json!({ + "type": "object", + "properties": { + "body": { + "type": "object", + "properties": { + "parent": {} + } + } + } + })) + .expect("parse schema"); + + assert_eq!( + schema, + JsonSchema::object( + BTreeMap::from([( + "body".to_string(), + JsonSchema::object( + BTreeMap::from([("parent".to_string(), JsonSchema::default())]), + /*required*/ None, + /*additional_properties*/ None, + ), + )]), + /*required*/ None, + /*additional_properties*/ None, + ) + ); } #[test] @@ -452,6 +510,46 @@ fn parse_tool_input_schema_fills_default_items_for_nullable_array_union() { ); } +#[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 + ) + ); +} + // Schemas that should be preserved for Responses API compatibility rather than // being rewritten into a different shape. diff --git a/codex-rs/tools/src/mcp_tool.rs b/codex-rs/tools/src/mcp_tool.rs index 337a8e42adbb..22d17e46f62f 100644 --- a/codex-rs/tools/src/mcp_tool.rs +++ b/codex-rs/tools/src/mcp_tool.rs @@ -1,5 +1,5 @@ +use crate::JsonSchema; use crate::ToolDefinition; -use crate::parse_tool_input_schema; use serde_json::Value as JsonValue; use serde_json::json; @@ -18,7 +18,7 @@ pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result rmcp::model::Tool { rmcp::model::Tool { @@ -34,17 +33,54 @@ fn parse_mcp_tool_inserts_empty_properties() { ToolDefinition { name: "no_props".to_string(), description: "No properties".to_string(), - input_schema: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), + input_schema: JsonSchema::from_raw_tool_input_schema(serde_json::json!({ + "type": "object", + "properties": {} + })), output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), defer_loading: false, } ); } +#[test] +fn parse_mcp_tool_preserves_raw_input_schema_keywords() { + let input_schema = serde_json::json!({ + "type": "object", + "properties": { + "parent": {}, + "start": { + "$ref": "#/$defs/date_time_zone", + "description": "Event start" + }, + "end": { + "allOf": [ + { "$ref": "#/$defs/date_time_zone" } + ] + } + }, + "$defs": { + "date_time_zone": { + "type": "object", + "properties": { + "dateTime": { "type": "string" }, + "timeZone": { "type": "string" } + }, + "required": ["dateTime", "timeZone"] + } + }, + "additionalProperties": true + }); + let tool = mcp_tool("create_page", "Create page", input_schema.clone()); + + let parsed = parse_mcp_tool(&tool).expect("parse MCP tool"); + + assert_eq!( + serde_json::to_value(&parsed.input_schema).expect("serialize input schema"), + input_schema + ); +} + #[test] fn parse_mcp_tool_preserves_top_level_output_schema() { let mut tool = mcp_tool( @@ -72,11 +108,10 @@ fn parse_mcp_tool_preserves_top_level_output_schema() { ToolDefinition { name: "with_output".to_string(), description: "Has output schema".to_string(), - input_schema: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), + input_schema: JsonSchema::from_raw_tool_input_schema(serde_json::json!({ + "type": "object", + "properties": {} + })), output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({ "properties": { "result": { @@ -112,11 +147,10 @@ fn parse_mcp_tool_preserves_output_schema_without_inferred_type() { ToolDefinition { name: "with_enum_output".to_string(), description: "Has enum output schema".to_string(), - input_schema: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), + input_schema: JsonSchema::from_raw_tool_input_schema(serde_json::json!({ + "type": "object", + "properties": {} + })), output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({ "enum": ["ok", "error"] }))), diff --git a/codex-rs/tools/src/responses_api_tests.rs b/codex-rs/tools/src/responses_api_tests.rs index 3ce13ebfe8c3..b32abae8ca2f 100644 --- a/codex-rs/tools/src/responses_api_tests.rs +++ b/codex-rs/tools/src/responses_api_tests.rs @@ -72,14 +72,14 @@ fn dynamic_tool_to_responses_api_tool_preserves_defer_loading() { description: "Look up an order".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::object( - BTreeMap::from([( - "order_id".to_string(), - JsonSchema::string(/*description*/ None), - )]), - Some(vec!["order_id".to_string()]), - Some(false.into()) - ), + parameters: JsonSchema::from_raw_tool_input_schema(json!({ + "type": "object", + "properties": { + "order_id": {"type": "string"} + }, + "required": ["order_id"], + "additionalProperties": false, + })), output_schema: None, } ); @@ -117,19 +117,100 @@ fn mcp_tool_to_deferred_responses_api_tool_sets_defer_loading() { description: "Look up an order".to_string(), strict: false, defer_loading: Some(true), - parameters: JsonSchema::object( - BTreeMap::from([( - "order_id".to_string(), - JsonSchema::string(/*description*/ None), - )]), - Some(vec!["order_id".to_string()]), - Some(false.into()) - ), + parameters: JsonSchema::from_raw_tool_input_schema(json!({ + "type": "object", + "properties": { + "order_id": {"type": "string"} + }, + "required": ["order_id"], + "additionalProperties": false, + })), output_schema: None, } ); } +#[test] +fn dynamic_tool_to_responses_api_tool_serializes_raw_non_strict_schema() { + let tool = DynamicToolSpec { + namespace: None, + name: "create_page".to_string(), + description: "Create a page".to_string(), + input_schema: json!({ + "type": "object", + "properties": { + "body": { + "type": "object", + "properties": { + "parent": {} + } + }, + "start": { + "$ref": "#/$defs/date_time_zone", + "description": "Event start" + }, + "end": { + "allOf": [ + { "$ref": "#/$defs/date_time_zone" } + ] + } + }, + "$defs": { + "date_time_zone": { + "type": "object", + "properties": { + "dateTime": { "type": "string" }, + "timeZone": { "type": "string" } + }, + "required": ["dateTime", "timeZone"] + } + }, + "additionalProperties": true + }), + defer_loading: false, + }; + + let value = serde_json::to_value( + dynamic_tool_to_responses_api_tool(&tool).expect("convert dynamic tool"), + ) + .expect("serialize responses tool"); + + assert_eq!( + value["parameters"], + json!({ + "type": "object", + "properties": { + "body": { + "type": "object", + "properties": { + "parent": {} + } + }, + "start": { + "$ref": "#/$defs/date_time_zone", + "description": "Event start" + }, + "end": { + "allOf": [ + { "$ref": "#/$defs/date_time_zone" } + ] + } + }, + "$defs": { + "date_time_zone": { + "type": "object", + "properties": { + "dateTime": { "type": "string" }, + "timeZone": { "type": "string" } + }, + "required": ["dateTime", "timeZone"] + } + }, + "additionalProperties": true + }) + ); +} + #[test] fn loadable_tool_spec_namespace_serializes_with_deferred_child_tools() { let namespace = LoadableToolSpec::Namespace(ResponsesApiNamespace { From 33b668df0bdc60ff126d48e49093e94762e3d976 Mon Sep 17 00:00:00 2001 From: Soheil Norouzi Date: Mon, 11 May 2026 13:32:09 -0400 Subject: [PATCH 2/2] Update schema preservation test expectations --- .../core/src/tools/handlers/tool_search.rs | 36 +-- codex-rs/core/src/tools/spec_plan_tests.rs | 71 ++---- codex-rs/core/src/tools/spec_tests.rs | 214 +++++------------- 3 files changed, 102 insertions(+), 219 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/tool_search.rs b/codex-rs/core/src/tools/handlers/tool_search.rs index 70410db3ae35..2780808839fc 100644 --- a/codex-rs/core/src/tools/handlers/tool_search.rs +++ b/codex-rs/core/src/tools/handlers/tool_search.rs @@ -250,10 +250,12 @@ mod tests { description: "Create events desktop tool".to_string(), strict: false, defer_loading: Some(true), - parameters: codex_tools::JsonSchema::object( - Default::default(), - /*required*/ None, - Some(false.into()), + parameters: codex_tools::JsonSchema::from_raw_tool_input_schema( + serde_json::json!({ + "type": "object", + "properties": {}, + "additionalProperties": false, + }), ), output_schema: None, }), @@ -262,10 +264,12 @@ mod tests { description: "List events desktop tool".to_string(), strict: false, defer_loading: Some(true), - parameters: codex_tools::JsonSchema::object( - Default::default(), - /*required*/ None, - Some(false.into()), + parameters: codex_tools::JsonSchema::from_raw_tool_input_schema( + serde_json::json!({ + "type": "object", + "properties": {}, + "additionalProperties": false, + }), ), output_schema: None, }), @@ -280,13 +284,15 @@ mod tests { .to_string(), strict: false, defer_loading: Some(true), - parameters: codex_tools::JsonSchema::object( - std::collections::BTreeMap::from([( - "mode".to_string(), - codex_tools::JsonSchema::string(/*description*/ None), - )]), - Some(vec!["mode".to_string()]), - Some(false.into()), + parameters: codex_tools::JsonSchema::from_raw_tool_input_schema( + serde_json::json!({ + "type": "object", + "properties": { + "mode": { "type": "string" }, + }, + "required": ["mode"], + "additionalProperties": false, + }), ), output_schema: None, })], diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 7cd33a2aaf9b..471a995ecf1d 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -1275,7 +1275,7 @@ fn test_test_model_info_includes_sync_tool() { } #[test] -fn test_build_specs_mcp_tools_converted() { +fn test_build_specs_mcp_tools_preserve_raw_schema() { let model_info = model_info(); let mut features = Features::with_defaults(); features.enable(Feature::UnifiedExec); @@ -1290,6 +1290,23 @@ fn test_build_specs_mcp_tools_converted() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); + let input_schema = serde_json::json!({ + "type": "object", + "properties": { + "string_argument": { "type": "string" }, + "number_argument": { "type": "number" }, + "object_argument": { + "type": "object", + "properties": { + "string_property": { "type": "string" }, + "number_property": { "type": "number" }, + }, + "required": ["string_property", "number_property"], + "additionalProperties": false, + }, + }, + }); + let (tools, _) = build_specs( &tools_config, Some(HashMap::from([( @@ -1297,22 +1314,7 @@ fn test_build_specs_mcp_tools_converted() { mcp_tool( "do_something_cool", "Do something cool", - serde_json::json!({ - "type": "object", - "properties": { - "string_argument": { "type": "string" }, - "number_argument": { "type": "number" }, - "object_argument": { - "type": "object", - "properties": { - "string_property": { "type": "string" }, - "number_property": { "type": "number" }, - }, - "required": ["string_property", "number_property"], - "additionalProperties": false, - }, - }, - }), + input_schema.clone(), ), )])), /*deferred_mcp_tools*/ None, @@ -1324,40 +1326,7 @@ fn test_build_specs_mcp_tools_converted() { tool, &ResponsesApiTool { name: "do_something_cool".to_string(), - parameters: JsonSchema::object( - BTreeMap::from([ - ( - "string_argument".to_string(), - JsonSchema::string(/*description*/ None), - ), - ( - "number_argument".to_string(), - JsonSchema::number(/*description*/ None), - ), - ( - "object_argument".to_string(), - JsonSchema::object( - BTreeMap::from([ - ( - "string_property".to_string(), - JsonSchema::string(/*description*/ None), - ), - ( - "number_property".to_string(), - JsonSchema::number(/*description*/ None), - ), - ]), - Some(vec![ - "string_property".to_string(), - "number_property".to_string(), - ]), - Some(false.into()), - ), - ), - ]), - /*required*/ None, - /*additional_properties*/ None - ), + parameters: JsonSchema::from_raw_tool_input_schema(input_schema), description: "Do something cool".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 06343fce0002..e68856d891cc 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -37,7 +37,6 @@ use codex_tools::mcp_tool_to_deferred_responses_api_tool; use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::assert_regex_match; use pretty_assertions::assert_eq; -use std::collections::BTreeMap; use std::path::PathBuf; use super::*; @@ -1129,7 +1128,7 @@ async fn unavailable_mcp_tools_are_exposed_as_dummy_function_tools() { } #[tokio::test] -async fn test_mcp_tool_property_missing_type_defaults_to_string() { +async fn test_mcp_tool_property_missing_type_preserves_raw_schema() { let config = test_config().await; let model_info = construct_model_info_offline("gpt-5.4", &config); let mut features = Features::with_defaults(); @@ -1146,20 +1145,18 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); + let input_schema = serde_json::json!({ + "type": "object", + "properties": { + "query": {"description": "search query"} + } + }); + let (tools, _) = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/search", - mcp_tool( - "search", - "Search docs", - serde_json::json!({ - "type": "object", - "properties": { - "query": {"description": "search query"} - } - }), - ), + mcp_tool("search", "Search docs", input_schema.clone()), )]), /*deferred_mcp_tools*/ None, &[], @@ -1171,15 +1168,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() { *tool, ResponsesApiTool { name: "search".to_string(), - parameters: JsonSchema::object( - /*properties*/ - BTreeMap::from([( - "query".to_string(), - JsonSchema::string(Some("search query".to_string())), - )]), - /*required*/ None, - /*additional_properties*/ None - ), + parameters: JsonSchema::from_raw_tool_input_schema(input_schema), description: "Search docs".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -1206,18 +1195,16 @@ async fn test_mcp_tool_preserves_integer_schema() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); + let input_schema = serde_json::json!({ + "type": "object", + "properties": {"page": {"type": "integer"}} + }); + let (tools, _) = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/paginate", - mcp_tool( - "paginate", - "Pagination", - serde_json::json!({ - "type": "object", - "properties": {"page": {"type": "integer"}} - }), - ), + mcp_tool("paginate", "Pagination", input_schema.clone()), )]), /*deferred_mcp_tools*/ None, &[], @@ -1229,15 +1216,7 @@ async fn test_mcp_tool_preserves_integer_schema() { *tool, ResponsesApiTool { name: "paginate".to_string(), - parameters: JsonSchema::object( - /*properties*/ - BTreeMap::from([( - "page".to_string(), - JsonSchema::integer(/*description*/ None), - )]), - /*required*/ None, - /*additional_properties*/ None - ), + parameters: JsonSchema::from_raw_tool_input_schema(input_schema), description: "Pagination".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -1247,7 +1226,7 @@ async fn test_mcp_tool_preserves_integer_schema() { } #[tokio::test] -async fn test_mcp_tool_array_without_items_gets_default_string_items() { +async fn test_mcp_tool_array_without_items_preserves_raw_schema() { let config = test_config().await; let model_info = construct_model_info_offline("gpt-5.4", &config); let mut features = Features::with_defaults(); @@ -1265,18 +1244,16 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); + let input_schema = serde_json::json!({ + "type": "object", + "properties": {"tags": {"type": "array"}} + }); + let (tools, _) = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/tags", - mcp_tool( - "tags", - "Tags", - serde_json::json!({ - "type": "object", - "properties": {"tags": {"type": "array"}} - }), - ), + mcp_tool("tags", "Tags", input_schema.clone()), )]), /*deferred_mcp_tools*/ None, &[], @@ -1288,18 +1265,7 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() { *tool, ResponsesApiTool { name: "tags".to_string(), - parameters: JsonSchema::object( - /*properties*/ - BTreeMap::from([( - "tags".to_string(), - JsonSchema::array( - JsonSchema::string(/*description*/ None), - /*description*/ None, - ), - )]), - /*required*/ None, - /*additional_properties*/ None - ), + parameters: JsonSchema::from_raw_tool_input_schema(input_schema), description: "Tags".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -1309,7 +1275,7 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() { } #[tokio::test] -async fn test_mcp_tool_anyof_defaults_to_string() { +async fn test_mcp_tool_anyof_preserves_raw_schema() { let config = test_config().await; let model_info = construct_model_info_offline("gpt-5.4", &config); let mut features = Features::with_defaults(); @@ -1326,20 +1292,18 @@ async fn test_mcp_tool_anyof_defaults_to_string() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); + let input_schema = serde_json::json!({ + "type": "object", + "properties": { + "value": {"anyOf": [{"type": "string"}, {"type": "number"}]} + } + }); + let (tools, _) = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/value", - mcp_tool( - "value", - "AnyOf Value", - serde_json::json!({ - "type": "object", - "properties": { - "value": {"anyOf": [{"type": "string"}, {"type": "number"}]} - } - }), - ), + mcp_tool("value", "AnyOf Value", input_schema.clone()), )]), /*deferred_mcp_tools*/ None, &[], @@ -1351,21 +1315,7 @@ async fn test_mcp_tool_anyof_defaults_to_string() { *tool, ResponsesApiTool { name: "value".to_string(), - parameters: JsonSchema::object( - /*properties*/ - BTreeMap::from([( - "value".to_string(), - JsonSchema::any_of( - vec![ - JsonSchema::string(/*description*/ None), - JsonSchema::number(/*description*/ None), - ], - /*description*/ None, - ), - )]), - /*required*/ None, - /*additional_properties*/ None - ), + parameters: JsonSchema::from_raw_tool_input_schema(input_schema), description: "AnyOf Value".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), @@ -1391,6 +1341,30 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); + let input_schema = serde_json::json!({ + "type": "object", + "properties": { + "string_argument": {"type": "string"}, + "number_argument": {"type": "number"}, + "object_argument": { + "type": "object", + "properties": { + "string_property": {"type": "string"}, + "number_property": {"type": "number"} + }, + "required": ["string_property", "number_property"], + "additionalProperties": { + "type": "object", + "properties": { + "addtl_prop": {"type": "string"} + }, + "required": ["addtl_prop"], + "additionalProperties": false + } + } + } + }); + let (tools, _) = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( @@ -1398,29 +1372,7 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { mcp_tool( "do_something_cool", "Do something cool", - serde_json::json!({ - "type": "object", - "properties": { - "string_argument": {"type": "string"}, - "number_argument": {"type": "number"}, - "object_argument": { - "type": "object", - "properties": { - "string_property": {"type": "string"}, - "number_property": {"type": "number"} - }, - "required": ["string_property", "number_property"], - "additionalProperties": { - "type": "object", - "properties": { - "addtl_prop": {"type": "string"} - }, - "required": ["addtl_prop"], - "additionalProperties": false - } - } - } - }), + input_schema.clone(), ), )]), /*deferred_mcp_tools*/ None, @@ -1433,51 +1385,7 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { *tool, ResponsesApiTool { name: "do_something_cool".to_string(), - parameters: JsonSchema::object( - /*properties*/ - BTreeMap::from([ - ( - "string_argument".to_string(), - JsonSchema::string(/*description*/ None), - ), - ( - "number_argument".to_string(), - JsonSchema::number(/*description*/ None), - ), - ( - "object_argument".to_string(), - JsonSchema::object( - BTreeMap::from([ - ( - "string_property".to_string(), - JsonSchema::string(/*description*/ None), - ), - ( - "number_property".to_string(), - JsonSchema::number(/*description*/ None), - ), - ]), - Some(vec![ - "string_property".to_string(), - "number_property".to_string(), - ]), - Some( - JsonSchema::object( - BTreeMap::from([( - "addtl_prop".to_string(), - JsonSchema::string(/*description*/ None), - )]), - Some(vec!["addtl_prop".to_string()]), - Some(false.into()), - ) - .into(), - ), - ), - ), - ]), - /*required*/ None, - /*additional_properties*/ None - ), + parameters: JsonSchema::from_raw_tool_input_schema(input_schema), description: "Do something cool".to_string(), strict: false, output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))),