diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index aa93a93192..053e1b9ea5 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -46,7 +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::ParsedToolDefinition; +use codex_tools::ToolDefinition; use codex_tools::parse_dynamic_tool; use codex_tools::parse_mcp_tool; use codex_utils_absolute_path::AbsolutePathBuf; @@ -2386,9 +2386,8 @@ pub(crate) fn mcp_tool_to_openai_tool( fully_qualified_name: String, tool: rmcp::model::Tool, ) -> Result { - Ok(parsed_tool_to_openai_tool( - fully_qualified_name, - parse_mcp_tool(&tool)?, + Ok(tool_definition_to_openai_tool( + parse_mcp_tool(&tool)?.renamed(fully_qualified_name), )) } @@ -2396,35 +2395,25 @@ pub(crate) fn mcp_tool_to_deferred_openai_tool( name: String, tool: rmcp::model::Tool, ) -> Result { - let parsed_tool = parse_mcp_tool(&tool)?; - - Ok(parsed_tool_to_openai_tool( - name, - ParsedToolDefinition { - output_schema: None, - defer_loading: true, - ..parsed_tool - }, + Ok(tool_definition_to_openai_tool( + parse_mcp_tool(&tool)?.renamed(name).into_deferred(), )) } fn dynamic_tool_to_openai_tool( tool: &DynamicToolSpec, ) -> Result { - Ok(parsed_tool_to_openai_tool( - tool.name.clone(), - parse_dynamic_tool(tool)?, - )) + Ok(tool_definition_to_openai_tool(parse_dynamic_tool(tool)?)) } -fn parsed_tool_to_openai_tool(name: String, parsed_tool: ParsedToolDefinition) -> ResponsesApiTool { +fn tool_definition_to_openai_tool(tool_definition: ToolDefinition) -> ResponsesApiTool { ResponsesApiTool { - name, - description: parsed_tool.description, + name: tool_definition.name, + description: tool_definition.description, strict: false, - defer_loading: parsed_tool.defer_loading.then_some(true), - parameters: parsed_tool.input_schema, - output_schema: parsed_tool.output_schema, + defer_loading: tool_definition.defer_loading.then_some(true), + parameters: tool_definition.input_schema, + output_schema: tool_definition.output_schema, } } diff --git a/codex-rs/tools/README.md b/codex-rs/tools/README.md index ad2a3e0596..aeedbbdde3 100644 --- a/codex-rs/tools/README.md +++ b/codex-rs/tools/README.md @@ -9,7 +9,7 @@ schema primitives that no longer need to live in `core/src/tools/spec.rs`: - `JsonSchema` - `AdditionalProperties` -- `ParsedToolDefinition` +- `ToolDefinition` - `parse_tool_input_schema()` - `parse_dynamic_tool()` - `parse_mcp_tool()` diff --git a/codex-rs/tools/src/dynamic_tool.rs b/codex-rs/tools/src/dynamic_tool.rs index 851fecf6fb..372dbce29d 100644 --- a/codex-rs/tools/src/dynamic_tool.rs +++ b/codex-rs/tools/src/dynamic_tool.rs @@ -1,17 +1,16 @@ -use crate::ParsedToolDefinition; +use crate::ToolDefinition; use crate::parse_tool_input_schema; use codex_protocol::dynamic_tools::DynamicToolSpec; -pub fn parse_dynamic_tool( - tool: &DynamicToolSpec, -) -> Result { +pub fn parse_dynamic_tool(tool: &DynamicToolSpec) -> Result { let DynamicToolSpec { - name: _, + name, description, input_schema, defer_loading, } = tool; - Ok(ParsedToolDefinition { + Ok(ToolDefinition { + name: name.clone(), description: description.clone(), input_schema: parse_tool_input_schema(input_schema)?, output_schema: None, diff --git a/codex-rs/tools/src/dynamic_tool_tests.rs b/codex-rs/tools/src/dynamic_tool_tests.rs index 568400bb06..2f4de34c23 100644 --- a/codex-rs/tools/src/dynamic_tool_tests.rs +++ b/codex-rs/tools/src/dynamic_tool_tests.rs @@ -1,6 +1,6 @@ use super::parse_dynamic_tool; use crate::JsonSchema; -use crate::ParsedToolDefinition; +use crate::ToolDefinition; use codex_protocol::dynamic_tools::DynamicToolSpec; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -22,7 +22,8 @@ fn parse_dynamic_tool_sanitizes_input_schema() { assert_eq!( parse_dynamic_tool(&tool).expect("parse dynamic tool"), - ParsedToolDefinition { + ToolDefinition { + name: "lookup_ticket".to_string(), description: "Fetch a ticket".to_string(), input_schema: JsonSchema::Object { properties: BTreeMap::from([( @@ -54,7 +55,8 @@ fn parse_dynamic_tool_preserves_defer_loading() { assert_eq!( parse_dynamic_tool(&tool).expect("parse dynamic tool"), - ParsedToolDefinition { + ToolDefinition { + name: "lookup_ticket".to_string(), description: "Fetch a ticket".to_string(), input_schema: JsonSchema::Object { properties: BTreeMap::new(), diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 517d705768..0579e8e725 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -3,7 +3,7 @@ mod dynamic_tool; mod json_schema; mod mcp_tool; -mod parsed_tool_definition; +mod tool_definition; pub use dynamic_tool::parse_dynamic_tool; pub use json_schema::AdditionalProperties; @@ -11,4 +11,4 @@ pub use json_schema::JsonSchema; pub use json_schema::parse_tool_input_schema; pub use mcp_tool::mcp_call_tool_result_output_schema; pub use mcp_tool::parse_mcp_tool; -pub use parsed_tool_definition::ParsedToolDefinition; +pub use tool_definition::ToolDefinition; diff --git a/codex-rs/tools/src/mcp_tool.rs b/codex-rs/tools/src/mcp_tool.rs index 0fda8677a7..3fd3c5e751 100644 --- a/codex-rs/tools/src/mcp_tool.rs +++ b/codex-rs/tools/src/mcp_tool.rs @@ -1,9 +1,9 @@ -use crate::ParsedToolDefinition; +use crate::ToolDefinition; use crate::parse_tool_input_schema; use serde_json::Value as JsonValue; use serde_json::json; -pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result { +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 @@ -25,7 +25,8 @@ pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result, - pub defer_loading: bool, -} diff --git a/codex-rs/tools/src/tool_definition.rs b/codex-rs/tools/src/tool_definition.rs new file mode 100644 index 0000000000..d3c5b0209b --- /dev/null +++ b/codex-rs/tools/src/tool_definition.rs @@ -0,0 +1,30 @@ +use crate::JsonSchema; +use serde_json::Value as JsonValue; + +/// Tool metadata and schemas that downstream crates can adapt into higher-level +/// tool specs. +#[derive(Debug, PartialEq)] +pub struct ToolDefinition { + pub name: String, + pub description: String, + pub input_schema: JsonSchema, + pub output_schema: Option, + pub defer_loading: bool, +} + +impl ToolDefinition { + pub fn renamed(mut self, name: String) -> Self { + self.name = name; + self + } + + pub fn into_deferred(mut self) -> Self { + self.output_schema = None; + self.defer_loading = true; + self + } +} + +#[cfg(test)] +#[path = "tool_definition_tests.rs"] +mod tests; diff --git a/codex-rs/tools/src/tool_definition_tests.rs b/codex-rs/tools/src/tool_definition_tests.rs new file mode 100644 index 0000000000..4a3b3708b2 --- /dev/null +++ b/codex-rs/tools/src/tool_definition_tests.rs @@ -0,0 +1,43 @@ +use super::ToolDefinition; +use crate::JsonSchema; +use pretty_assertions::assert_eq; +use std::collections::BTreeMap; + +fn tool_definition() -> ToolDefinition { + ToolDefinition { + name: "lookup_order".to_string(), + description: "Look up an order".to_string(), + input_schema: JsonSchema::Object { + properties: BTreeMap::new(), + required: None, + additional_properties: None, + }, + output_schema: Some(serde_json::json!({ + "type": "object", + })), + defer_loading: false, + } +} + +#[test] +fn renamed_overrides_name_only() { + assert_eq!( + tool_definition().renamed("mcp__orders__lookup_order".to_string()), + ToolDefinition { + name: "mcp__orders__lookup_order".to_string(), + ..tool_definition() + } + ); +} + +#[test] +fn into_deferred_drops_output_schema_and_sets_defer_loading() { + assert_eq!( + tool_definition().into_deferred(), + ToolDefinition { + output_schema: None, + defer_loading: true, + ..tool_definition() + } + ); +}