diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index f494c8528..c5846fbe1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2633,6 +2633,7 @@ name = "codex-tools" version = "0.0.0" dependencies = [ "pretty_assertions", + "rmcp", "serde", "serde_json", ] diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index a81bf5a3c..ea85eaacd 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -46,6 +46,7 @@ use codex_protocol::openai_models::WebSearchToolType; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; +use codex_tools::parse_mcp_tool; pub use codex_tools::parse_tool_input_schema; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; @@ -58,6 +59,9 @@ use std::path::PathBuf; pub type JsonSchema = codex_tools::JsonSchema; +#[cfg(test)] +pub(crate) use codex_tools::mcp_call_tool_result_output_schema; + const TOOL_SEARCH_DESCRIPTION_TEMPLATE: &str = include_str!("../../templates/search_tool/tool_description.md"); const TOOL_SUGGEST_DESCRIPTION_TEMPLATE: &str = @@ -2362,15 +2366,15 @@ pub(crate) fn mcp_tool_to_openai_tool( fully_qualified_name: String, tool: rmcp::model::Tool, ) -> Result { - let (description, input_schema, output_schema) = mcp_tool_to_openai_tool_parts(tool)?; + let parsed_tool = parse_mcp_tool(&tool)?; Ok(ResponsesApiTool { name: fully_qualified_name, - description, + description: parsed_tool.description, strict: false, defer_loading: None, - parameters: input_schema, - output_schema, + parameters: parsed_tool.input_schema, + output_schema: Some(parsed_tool.output_schema), }) } @@ -2378,14 +2382,14 @@ pub(crate) fn mcp_tool_to_deferred_openai_tool( name: String, tool: rmcp::model::Tool, ) -> Result { - let (description, input_schema, _) = mcp_tool_to_openai_tool_parts(tool)?; + let parsed_tool = parse_mcp_tool(&tool)?; Ok(ResponsesApiTool { name, - description, + description: parsed_tool.description, strict: false, defer_loading: Some(true), - parameters: input_schema, + parameters: parsed_tool.input_schema, output_schema: None, }) } @@ -2405,61 +2409,6 @@ fn dynamic_tool_to_openai_tool( }) } -fn mcp_tool_to_openai_tool_parts( - tool: rmcp::model::Tool, -) -> Result<(String, JsonSchema, Option), serde_json::Error> { - let rmcp::model::Tool { - description, - input_schema, - output_schema, - .. - } = tool; - - let mut serialized_input_schema = serde_json::Value::Object(input_schema.as_ref().clone()); - - // OpenAI models mandate the "properties" field in the schema. Some MCP - // servers omit it (or set it to null), so we insert an empty object to - // match the behavior of the Agents SDK. - if let serde_json::Value::Object(obj) = &mut serialized_input_schema - && obj.get("properties").is_none_or(serde_json::Value::is_null) - { - obj.insert( - "properties".to_string(), - serde_json::Value::Object(serde_json::Map::new()), - ); - } - - let input_schema = parse_tool_input_schema(&serialized_input_schema)?; - let structured_content_schema = output_schema - .map(|output_schema| serde_json::Value::Object(output_schema.as_ref().clone())) - .unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())); - let output_schema = Some(mcp_call_tool_result_output_schema( - structured_content_schema, - )); - let description = description.map(Into::into).unwrap_or_default(); - - Ok((description, input_schema, output_schema)) -} - -fn mcp_call_tool_result_output_schema(structured_content_schema: JsonValue) -> JsonValue { - json!({ - "type": "object", - "properties": { - "content": { - "type": "array", - "items": {} - }, - "structuredContent": structured_content_schema, - "isError": { - "type": "boolean" - }, - "_meta": {} - }, - "required": ["content"], - "additionalProperties": false - }) -} - /// Builds the tool registry builder while collecting tool specs for later serialization. #[cfg(test)] pub(crate) fn build_specs( diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 82310e872..5b9016ec6 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -63,139 +63,6 @@ fn search_capable_model_info() -> ModelInfo { model_info } -#[test] -fn mcp_tool_to_openai_tool_inserts_empty_properties() { - let mut schema = rmcp::model::JsonObject::new(); - schema.insert("type".to_string(), serde_json::json!("object")); - - let tool = rmcp::model::Tool { - name: "no_props".to_string().into(), - title: None, - description: Some("No properties".to_string().into()), - input_schema: std::sync::Arc::new(schema), - output_schema: None, - annotations: None, - execution: None, - icons: None, - meta: None, - }; - - let openai_tool = - mcp_tool_to_openai_tool("server/no_props".to_string(), tool).expect("convert tool"); - let parameters = serde_json::to_value(openai_tool.parameters).expect("serialize schema"); - - assert_eq!(parameters.get("properties"), Some(&serde_json::json!({}))); -} - -#[test] -fn mcp_tool_to_openai_tool_preserves_top_level_output_schema() { - let mut input_schema = rmcp::model::JsonObject::new(); - input_schema.insert("type".to_string(), serde_json::json!("object")); - - let mut output_schema = rmcp::model::JsonObject::new(); - output_schema.insert( - "properties".to_string(), - serde_json::json!({ - "result": { - "properties": { - "nested": {} - } - } - }), - ); - output_schema.insert("required".to_string(), serde_json::json!(["result"])); - - let tool = rmcp::model::Tool { - name: "with_output".to_string().into(), - title: None, - description: Some("Has output schema".to_string().into()), - input_schema: std::sync::Arc::new(input_schema), - output_schema: Some(std::sync::Arc::new(output_schema)), - annotations: None, - execution: None, - icons: None, - meta: None, - }; - - let openai_tool = mcp_tool_to_openai_tool("mcp__server__with_output".to_string(), tool) - .expect("convert tool"); - - assert_eq!( - openai_tool.output_schema, - Some(serde_json::json!({ - "type": "object", - "properties": { - "content": { - "type": "array", - "items": {} - }, - "structuredContent": { - "properties": { - "result": { - "properties": { - "nested": {} - } - } - }, - "required": ["result"] - }, - "isError": { - "type": "boolean" - }, - "_meta": {} - }, - "required": ["content"], - "additionalProperties": false - })) - ); -} - -#[test] -fn mcp_tool_to_openai_tool_preserves_output_schema_without_inferred_type() { - let mut input_schema = rmcp::model::JsonObject::new(); - input_schema.insert("type".to_string(), serde_json::json!("object")); - - let mut output_schema = rmcp::model::JsonObject::new(); - output_schema.insert("enum".to_string(), serde_json::json!(["ok", "error"])); - - let tool = rmcp::model::Tool { - name: "with_enum_output".to_string().into(), - title: None, - description: Some("Has enum output schema".to_string().into()), - input_schema: std::sync::Arc::new(input_schema), - output_schema: Some(std::sync::Arc::new(output_schema)), - annotations: None, - execution: None, - icons: None, - meta: None, - }; - - let openai_tool = mcp_tool_to_openai_tool("mcp__server__with_enum_output".to_string(), tool) - .expect("convert tool"); - - assert_eq!( - openai_tool.output_schema, - Some(serde_json::json!({ - "type": "object", - "properties": { - "content": { - "type": "array", - "items": {} - }, - "structuredContent": { - "enum": ["ok", "error"] - }, - "isError": { - "type": "boolean" - }, - "_meta": {} - }, - "required": ["content"], - "additionalProperties": false - })) - ); -} - #[test] fn search_tool_deferred_tools_always_set_defer_loading_true() { let tool = mcp_tool( diff --git a/codex-rs/tools/Cargo.toml b/codex-rs/tools/Cargo.toml index c596520c5..d695336fa 100644 --- a/codex-rs/tools/Cargo.toml +++ b/codex-rs/tools/Cargo.toml @@ -8,6 +8,12 @@ version.workspace = true workspace = true [dependencies] +rmcp = { workspace = true, default-features = false, features = [ + "base64", + "macros", + "schemars", + "server", +] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/tools/README.md b/codex-rs/tools/README.md index d4c2f28e9..b8780fe7b 100644 --- a/codex-rs/tools/README.md +++ b/codex-rs/tools/README.md @@ -4,12 +4,15 @@ shared across multiple crates and does not need to stay coupled to `codex-core`. -Today this crate is intentionally small. It only owns the shared tool input -schema model and parser that were previously defined in `core/src/tools/spec.rs`: +Today this crate is intentionally small. It currently owns the shared tool +schema primitives that no longer need to live in `core/src/tools/spec.rs`: - `JsonSchema` - `AdditionalProperties` - `parse_tool_input_schema()` +- `ParsedMcpTool` +- `parse_mcp_tool()` +- `mcp_call_tool_result_output_schema()` That extraction is the first step in a longer migration. The goal is not to move all of `core/src/tools` into this crate in one shot. Instead, the plan is diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 90d3af10c..e719d3e23 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -1,7 +1,11 @@ //! Shared tool-schema parsing primitives that can live outside `codex-core`. mod json_schema; +mod mcp_tool; pub use json_schema::AdditionalProperties; pub use json_schema::JsonSchema; pub use json_schema::parse_tool_input_schema; +pub use mcp_tool::ParsedMcpTool; +pub use mcp_tool::mcp_call_tool_result_output_schema; +pub use mcp_tool::parse_mcp_tool; diff --git a/codex-rs/tools/src/mcp_tool.rs b/codex-rs/tools/src/mcp_tool.rs new file mode 100644 index 000000000..99294a0e6 --- /dev/null +++ b/codex-rs/tools/src/mcp_tool.rs @@ -0,0 +1,65 @@ +use crate::JsonSchema; +use crate::parse_tool_input_schema; +use serde_json::Value as JsonValue; +use serde_json::json; + +/// Parsed MCP tool metadata and schemas that can be adapted into a higher-level +/// tool spec by downstream crates. +#[derive(Debug, PartialEq)] +pub struct ParsedMcpTool { + pub description: String, + pub input_schema: JsonSchema, + pub output_schema: JsonValue, +} + +pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result { + let mut serialized_input_schema = serde_json::Value::Object(tool.input_schema.as_ref().clone()); + + // OpenAI models mandate the "properties" field in the schema. Some MCP + // servers omit it (or set it to null), so we insert an empty object to + // match the behavior of the Agents SDK. + if let serde_json::Value::Object(obj) = &mut serialized_input_schema + && obj.get("properties").is_none_or(serde_json::Value::is_null) + { + obj.insert( + "properties".to_string(), + serde_json::Value::Object(serde_json::Map::new()), + ); + } + + let input_schema = parse_tool_input_schema(&serialized_input_schema)?; + let structured_content_schema = tool + .output_schema + .as_ref() + .map(|output_schema| serde_json::Value::Object(output_schema.as_ref().clone())) + .unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())); + + Ok(ParsedMcpTool { + description: tool.description.clone().map(Into::into).unwrap_or_default(), + input_schema, + output_schema: mcp_call_tool_result_output_schema(structured_content_schema), + }) +} + +pub fn mcp_call_tool_result_output_schema(structured_content_schema: JsonValue) -> JsonValue { + json!({ + "type": "object", + "properties": { + "content": { + "type": "array", + "items": {} + }, + "structuredContent": structured_content_schema, + "isError": { + "type": "boolean" + }, + "_meta": {} + }, + "required": ["content"], + "additionalProperties": false + }) +} + +#[cfg(test)] +#[path = "mcp_tool_tests.rs"] +mod tests; diff --git a/codex-rs/tools/src/mcp_tool_tests.rs b/codex-rs/tools/src/mcp_tool_tests.rs new file mode 100644 index 000000000..2d4df6f36 --- /dev/null +++ b/codex-rs/tools/src/mcp_tool_tests.rs @@ -0,0 +1,120 @@ +use super::ParsedMcpTool; +use super::mcp_call_tool_result_output_schema; +use super::parse_mcp_tool; +use crate::JsonSchema; +use pretty_assertions::assert_eq; +use std::collections::BTreeMap; + +fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> rmcp::model::Tool { + rmcp::model::Tool { + name: name.to_string().into(), + title: None, + description: Some(description.to_string().into()), + input_schema: std::sync::Arc::new(rmcp::model::object(input_schema)), + output_schema: None, + annotations: None, + execution: None, + icons: None, + meta: None, + } +} + +#[test] +fn parse_mcp_tool_inserts_empty_properties() { + let tool = mcp_tool( + "no_props", + "No properties", + serde_json::json!({ + "type": "object" + }), + ); + + assert_eq!( + parse_mcp_tool(&tool).expect("parse MCP tool"), + ParsedMcpTool { + description: "No properties".to_string(), + input_schema: JsonSchema::Object { + properties: BTreeMap::new(), + required: None, + additional_properties: None, + }, + output_schema: mcp_call_tool_result_output_schema(serde_json::json!({})), + } + ); +} + +#[test] +fn parse_mcp_tool_preserves_top_level_output_schema() { + let mut tool = mcp_tool( + "with_output", + "Has output schema", + serde_json::json!({ + "type": "object" + }), + ); + tool.output_schema = Some(std::sync::Arc::new(rmcp::model::object( + serde_json::json!({ + "properties": { + "result": { + "properties": { + "nested": {} + } + } + }, + "required": ["result"] + }), + ))); + + assert_eq!( + parse_mcp_tool(&tool).expect("parse MCP tool"), + ParsedMcpTool { + description: "Has output schema".to_string(), + input_schema: JsonSchema::Object { + properties: BTreeMap::new(), + required: None, + additional_properties: None, + }, + output_schema: mcp_call_tool_result_output_schema(serde_json::json!({ + "properties": { + "result": { + "properties": { + "nested": {} + } + } + }, + "required": ["result"] + })), + } + ); +} + +#[test] +fn parse_mcp_tool_preserves_output_schema_without_inferred_type() { + let mut tool = mcp_tool( + "with_enum_output", + "Has enum output schema", + serde_json::json!({ + "type": "object" + }), + ); + tool.output_schema = Some(std::sync::Arc::new(rmcp::model::object( + serde_json::json!({ + "enum": ["ok", "error"] + }), + ))); + + assert_eq!( + parse_mcp_tool(&tool).expect("parse MCP tool"), + ParsedMcpTool { + description: "Has enum output schema".to_string(), + input_schema: JsonSchema::Object { + properties: BTreeMap::new(), + required: None, + additional_properties: None, + }, + output_schema: mcp_call_tool_result_output_schema(serde_json::json!({ + "enum": ["ok", "error"] + })), + } + ); +}