diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index bbb61f8337..2299b52e4a 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3689,6 +3689,7 @@ dependencies = [ name = "codex-tool-api" version = "0.0.0" dependencies = [ + "codex-protocol", "pretty_assertions", "serde_json", "thiserror 2.0.18", @@ -3702,6 +3703,7 @@ dependencies = [ "codex-code-mode", "codex-features", "codex-protocol", + "codex-tool-api", "codex-utils-absolute-path", "codex-utils-pty", "pretty_assertions", diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index f76708478d..74cc004ead 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -3,9 +3,7 @@ use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tool_api::ToolError as ExtensionToolError; -use codex_tools::ResponsesApiTool; use codex_tools::ToolName; -use codex_tools::ToolSpec; use serde_json::Value; use crate::function_tool::FunctionCallError; @@ -52,12 +50,11 @@ impl ToolOutput for BundledToolOutput { pub(crate) struct BundledToolHandler { bundle: ExtensionToolBundle, - spec: ToolSpec, } impl BundledToolHandler { - pub(crate) fn new(bundle: ExtensionToolBundle, spec: ToolSpec) -> Self { - Self { bundle, spec } + pub(crate) fn new(bundle: ExtensionToolBundle) -> Self { + Self { bundle } } fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> { @@ -75,10 +72,6 @@ impl ToolHandler for BundledToolHandler { ToolName::plain(self.bundle.tool_name()) } - fn spec(&self) -> Option { - Some(self.spec.clone()) - } - fn matches_kind(&self, payload: &ToolPayload) -> bool { self.arguments_from_payload(payload).is_some() } @@ -133,19 +126,6 @@ impl ToolHandler for BundledToolHandler { } } -pub(crate) fn extension_tool_spec( - spec: &codex_tool_api::FunctionToolSpec, -) -> Result { - Ok(ToolSpec::Function(ResponsesApiTool { - name: spec.name.clone(), - description: spec.description.clone(), - strict: spec.strict, - defer_loading: None, - parameters: codex_tools::parse_tool_input_schema(&spec.parameters)?, - output_schema: None, - })) -} - fn map_extension_tool_error(error: ExtensionToolError) -> FunctionCallError { match error { ExtensionToolError::RespondToModel(message) => FunctionCallError::RespondToModel(message), @@ -170,7 +150,6 @@ mod tests { use super::BundledToolHandler; use super::BundledToolOutput; - use super::extension_tool_spec; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -206,8 +185,7 @@ mod tests { }, Arc::new(StubExtensionExecutor), ); - let spec = extension_tool_spec(bundle.spec()).expect("extension spec should convert"); - let handler = BundledToolHandler::new(bundle, spec); + let handler = BundledToolHandler::new(bundle); let (session, turn) = crate::session::tests::make_session_and_context().await; let invocation = ToolInvocation { session: session.into(), diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 235cb6fef3..e486366ba9 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -17,14 +17,11 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::flat_tool_name; -use crate::tools::handlers::extension_tools::BundledToolHandler; -use crate::tools::handlers::extension_tools::extension_tool_spec; use crate::tools::hook_names::HookToolName; use crate::tools::tool_dispatch_trace::ToolDispatchTrace; use crate::util::error_or_panic; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::EventMsg; -use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_utils_readiness::Readiness; @@ -180,7 +177,7 @@ pub(crate) struct PostToolUsePayload { pub(crate) tool_response: Value, } -trait AnyToolHandler: Send + Sync { +pub(crate) trait AnyToolHandler: Send + Sync { fn supports_parallel_tool_calls(&self) -> bool; fn matches_kind(&self, payload: &ToolPayload) -> bool; @@ -593,26 +590,18 @@ impl ToolRegistryBuilder { self.handlers.insert(name, handler); } - pub fn register_tool_bundle(&mut self, bundle: ExtensionToolBundle) { - let tool_name = ToolName::plain(bundle.tool_name()); + pub(crate) fn register_erased_handler( + &mut self, + tool_name: ToolName, + handler: Arc, + ) -> bool { if self.handlers.contains_key(&tool_name) { - warn!("Skipping extension tool `{tool_name}`: handler already registered"); - return; + warn!("Skipping tool handler `{tool_name}`: handler already registered"); + return false; } - let spec = match extension_tool_spec(bundle.spec()) { - Ok(spec) => spec, - Err(error) => { - error_or_panic(format!( - "failed to convert extension tool `{tool_name}` to a host spec: {error}" - )); - return; - } - }; - self.push_spec(spec.clone()); - - let handler: Arc = Arc::new(BundledToolHandler::new(bundle, spec)); self.handlers.insert(tool_name, handler); + true } pub(crate) fn specs(&self) -> &[ToolSpec] { diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 69456a36ef..27cc98a987 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -27,6 +27,7 @@ use crate::tools::handlers::ViewImageHandler; use crate::tools::handlers::WriteStdinHandler; use crate::tools::handlers::agent_jobs::ReportAgentJobResultHandler; use crate::tools::handlers::agent_jobs::SpawnAgentsOnCsvHandler; +use crate::tools::handlers::extension_tools::BundledToolHandler; use crate::tools::handlers::multi_agents::CloseAgentHandler; use crate::tools::handlers::multi_agents::ResumeAgentHandler; use crate::tools::handlers::multi_agents::SendInputHandler; @@ -44,13 +45,15 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::hosted_spec::WebSearchToolOptions; use crate::tools::hosted_spec::create_image_generation_tool; use crate::tools::hosted_spec::create_web_search_tool; +use crate::tools::registry::AnyToolHandler; use crate::tools::registry::ToolRegistryBuilder; +use crate::tools::spec_plan_types::ToolNamespace; use crate::tools::spec_plan_types::ToolRegistryBuildParams; use crate::tools::spec_plan_types::agent_type_description; use codex_mcp::ToolInfo; use codex_protocol::openai_models::ConfigShellToolType; -use codex_tools::ResponsesApiNamespace; -use codex_tools::ResponsesApiNamespaceTool; +use codex_tool_api::ToolBundle as ExtensionToolBundle; +use codex_tool_api::ToolDefinition; use codex_tools::ToolEnvironmentMode; use codex_tools::ToolName; use codex_tools::ToolSearchSource; @@ -61,8 +64,9 @@ use codex_tools::coalesce_loadable_tool_specs; use codex_tools::collect_code_mode_exec_prompt_tool_definitions; use codex_tools::collect_tool_search_source_infos; use codex_tools::default_namespace_description; -use codex_tools::dynamic_tool_to_loadable_tool_spec; -use codex_tools::mcp_tool_to_responses_api_tool; +use codex_tools::mcp_tool_definition; +use codex_tools::parse_dynamic_tool; +use codex_tools::tool_definition_to_loadable_tool_spec; use std::collections::BTreeMap; use std::collections::HashSet; use std::sync::Arc; @@ -333,88 +337,15 @@ pub fn build_tool_registry_builder( } } - if let Some(mcp_tools) = params.mcp_tools { - let mut entries = mcp_tools - .iter() - .map(|tool| (tool.canonical_tool_name(), tool)) - .collect::>(); - entries.sort_by(|(left, _), (right, _)| left.cmp(right)); - let mut namespace_entries = BTreeMap::new(); - - for (tool_name, tool) in entries { - let Some(namespace) = tool_name.namespace.as_ref() else { - tracing::error!("Skipping MCP tool `{tool_name}`: MCP tools must be namespaced"); - continue; - }; - namespace_entries - .entry(namespace.clone()) - .or_insert_with(Vec::new) - .push((tool_name, tool)); - } - - for (namespace, mut entries) in namespace_entries { - entries.sort_by_key(|(tool_name, _)| tool_name.name.clone()); - let tool_namespace = params - .tool_namespaces - .and_then(|namespaces| namespaces.get(&namespace)); - let description = tool_namespace - .and_then(|namespace| namespace.description.as_deref()) - .map(str::trim) - .filter(|description| !description.is_empty()) - .map(str::to_string) - .unwrap_or_else(|| { - let namespace_name = tool_namespace - .map(|namespace| namespace.name.as_str()) - .unwrap_or(namespace.as_str()); - default_namespace_description(namespace_name) - }); - let mut tools = Vec::new(); - for (tool_name, tool) in entries { - match mcp_tool_to_responses_api_tool(&tool_name, &tool.tool) { - Ok(converted_tool) => { - tools.push(ResponsesApiNamespaceTool::Function(converted_tool)); - builder.register_handler(Arc::new(McpHandler::new(tool.clone()))); - } - Err(error) => { - tracing::error!( - "Failed to convert `{tool_name}` MCP tool to OpenAI tool: {error:?}" - ); - } - } - } - - if config.namespace_tools && !tools.is_empty() { - builder.push_spec(ToolSpec::Namespace(ResponsesApiNamespace { - name: namespace, - description, - tools, - })); - } - } - } - - let mut dynamic_tool_specs = Vec::new(); - for tool in params.dynamic_tools { - match dynamic_tool_to_loadable_tool_spec(tool) { - Ok(loadable_tool) => { - let handler_name = ToolName::new(tool.namespace.clone(), tool.name.clone()); - dynamic_tool_specs.push(loadable_tool); - builder.register_handler(Arc::new(DynamicToolHandler::new(handler_name))); - } - Err(error) => { - tracing::error!( - "Failed to convert dynamic tool {:?} to OpenAI tool: {error:?}", - tool.name - ); - } - } - } - for spec in coalesce_loadable_tool_specs(dynamic_tool_specs) { - let spec = spec.into(); - if config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_)) { - builder.push_spec(spec); - } - } + register_and_publish_tool_definitions( + &mut builder, + config, + params.tool_namespaces, + mcp_tool_definitions(params.mcp_tools) + .into_iter() + .chain(dynamic_tool_definitions(params.dynamic_tools)) + .chain(extension_tool_definitions(params.extension_tool_bundles)), + ); if let Some(deferred_mcp_tools) = params.deferred_mcp_tools { let directly_registered_mcp_tools = params @@ -425,16 +356,129 @@ pub fn build_tool_registry_builder( .collect::>(); for tool in deferred_mcp_tools { if !directly_registered_mcp_tools.contains(&tool.canonical_tool_name()) { - builder.register_handler(Arc::new(McpHandler::new(tool.clone()))); + let definition = mcp_tool_definition(tool.canonical_tool_name(), &tool.tool) + .deferred() + .with_runtime( + Arc::new(McpHandler::new(tool.clone())) as Arc + ); + register_tool_definition_handler(&mut builder, &definition); } } } - for bundle in params.extension_tool_bundles.iter().cloned() { - builder.register_tool_bundle(bundle); + builder +} + +type RuntimeToolDefinition = ToolDefinition>; + +fn register_and_publish_tool_definitions( + builder: &mut ToolRegistryBuilder, + config: &ToolsConfig, + tool_namespaces: Option<&std::collections::HashMap>, + definitions: impl IntoIterator, +) { + let mut loadable_specs = Vec::new(); + + for definition in definitions { + let tool_name = definition.tool_name().clone(); + let namespace_description = namespace_description_for_tool(&tool_name, tool_namespaces); + let loadable_spec = + match tool_definition_to_loadable_tool_spec(&definition, namespace_description) { + Ok(spec) => spec, + Err(error) => { + tracing::error!( + "Failed to convert tool `{tool_name}` to OpenAI tool: {error:?}" + ); + continue; + } + }; + if register_tool_definition_handler(builder, &definition) { + loadable_specs.push(loadable_spec); + } } - builder + for spec in coalesce_loadable_tool_specs(loadable_specs) { + let spec = spec.into(); + if config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_)) { + builder.push_spec(spec); + } + } +} + +fn register_tool_definition_handler( + builder: &mut ToolRegistryBuilder, + definition: &RuntimeToolDefinition, +) -> bool { + builder.register_erased_handler( + definition.tool_name().clone(), + Arc::clone(definition.runtime()), + ) +} + +fn mcp_tool_definitions(mcp_tools: Option<&[ToolInfo]>) -> Vec { + let mut tools = mcp_tools.into_iter().flatten().collect::>(); + tools.sort_by_key(|tool| tool.canonical_tool_name()); + + tools + .into_iter() + .filter_map(|tool| { + let tool_name = tool.canonical_tool_name(); + if tool_name.namespace.is_none() { + tracing::error!("Skipping MCP tool `{tool_name}`: MCP tools must be namespaced"); + return None; + } + Some( + mcp_tool_definition(tool_name, &tool.tool) + .with_runtime( + Arc::new(McpHandler::new(tool.clone())) as Arc + ), + ) + }) + .collect() +} + +fn dynamic_tool_definitions( + dynamic_tools: &[codex_protocol::dynamic_tools::DynamicToolSpec], +) -> Vec { + dynamic_tools + .iter() + .map(|tool| { + let definition = parse_dynamic_tool(tool); + let handler = Arc::new(DynamicToolHandler::new(definition.tool_name().clone())) + as Arc; + definition.with_runtime(handler) + }) + .collect() +} + +fn extension_tool_definitions( + extension_tool_bundles: &[ExtensionToolBundle], +) -> Vec { + extension_tool_bundles + .iter() + .map(|bundle| { + let handler = + Arc::new(BundledToolHandler::new(bundle.clone())) as Arc; + bundle.definition().clone().with_runtime(handler) + }) + .collect() +} + +fn namespace_description_for_tool( + tool_name: &ToolName, + tool_namespaces: Option<&std::collections::HashMap>, +) -> Option { + let namespace = tool_name.namespace.as_ref()?; + let tool_namespace = tool_namespaces.and_then(|namespaces| namespaces.get(namespace)); + tool_namespace.map(|tool_namespace| { + tool_namespace + .description + .as_deref() + .map(str::trim) + .filter(|description| !description.is_empty()) + .map(str::to_string) + .unwrap_or_else(|| default_namespace_description(tool_namespace.name.as_str())) + }) } fn compare_code_mode_tools( diff --git a/codex-rs/tool-api/Cargo.toml b/codex-rs/tool-api/Cargo.toml index 13d1a22071..324a581ab8 100644 --- a/codex-rs/tool-api/Cargo.toml +++ b/codex-rs/tool-api/Cargo.toml @@ -13,6 +13,7 @@ doctest = false workspace = true [dependencies] +codex-protocol = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } diff --git a/codex-rs/tool-api/README.md b/codex-rs/tool-api/README.md index 418c1d177c..f79a1ee81c 100644 --- a/codex-rs/tool-api/README.md +++ b/codex-rs/tool-api/README.md @@ -8,13 +8,17 @@ Crates that define contributed tools should depend on this crate. It owns: - the executable bundle contract: `ToolBundle`, `ToolExecutor`, `ToolCall`, and `ToolError` +- the shared definition envelope: `ToolDefinition`, `ToolExposure`, and + `ToolName` - the one model-visible spec an extension may contribute directly: `FunctionToolSpec` -The contract is intentionally narrow: contributed tools receive a call id plus -raw JSON arguments and return a JSON value. If a feature needs richer host -integration, its extension is expected to do that wiring before exposing the -tool rather than widening this crate around the hardest native tools. +The contract is intentionally narrow: a definition keeps one tool's canonical +name, function metadata, exposure mode, and opaque runtime together. Contributed +tools receive a call id plus raw JSON arguments and return a JSON value. If a +feature needs richer host integration, its extension is expected to do that +wiring before exposing the tool rather than widening this crate around the +hardest native tools. The intended dependency direction is: diff --git a/codex-rs/tool-api/src/bundle.rs b/codex-rs/tool-api/src/bundle.rs index 18c84c4c07..57bc897572 100644 --- a/codex-rs/tool-api/src/bundle.rs +++ b/codex-rs/tool-api/src/bundle.rs @@ -6,7 +6,9 @@ use serde_json::Value; use crate::FunctionToolSpec; use crate::ToolCall; +use crate::ToolDefinition; use crate::ToolError; +use crate::ToolName; /// Future returned by one contributed function-tool invocation. pub type ToolFuture<'a> = Pin> + Send + 'a>>; @@ -15,29 +17,36 @@ pub type ToolFuture<'a> = Pin> /// function tool. #[derive(Clone)] pub struct ToolBundle { - spec: FunctionToolSpec, - executor: Arc, + definition: ToolDefinition>, } impl ToolBundle { /// Creates one contributed function-tool bundle. pub fn new(spec: FunctionToolSpec, executor: Arc) -> Self { - Self { spec, executor } + let tool_name = ToolName::plain(spec.name.clone()); + Self { + definition: ToolDefinition::new(tool_name, spec, executor), + } } /// Returns the contributed function-tool spec. pub fn spec(&self) -> &FunctionToolSpec { - &self.spec + self.definition.spec() } /// Returns the contributed function-tool name. pub fn tool_name(&self) -> &str { - self.spec.name.as_str() + self.definition.tool_name().name.as_str() } /// Returns the executable implementation. pub fn executor(&self) -> Arc { - Arc::clone(&self.executor) + Arc::clone(self.definition.runtime()) + } + + /// Returns the shared definition behind this extension-owned bundle. + pub fn definition(&self) -> &ToolDefinition> { + &self.definition } } diff --git a/codex-rs/tool-api/src/definition.rs b/codex-rs/tool-api/src/definition.rs new file mode 100644 index 0000000000..a603646bd6 --- /dev/null +++ b/codex-rs/tool-api/src/definition.rs @@ -0,0 +1,134 @@ +use codex_protocol::ToolName; +use serde_json::Value; + +use crate::FunctionToolSpec; + +/// One callable function tool, its exposure mode, and the runtime object that +/// executes it. +#[derive(Clone, Debug, PartialEq)] +pub struct ToolDefinition { + tool_name: ToolName, + spec: FunctionToolSpec, + output_schema: Option, + exposure: ToolExposure, + runtime: R, +} + +/// Whether a tool is advertised immediately or marked for deferred loading. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub enum ToolExposure { + #[default] + Published, + Deferred, +} + +impl ToolDefinition { + /// Creates one immediately-published function tool definition. + pub fn new(tool_name: ToolName, spec: FunctionToolSpec, runtime: R) -> Self { + debug_assert_eq!( + tool_name.name, spec.name, + "tool definition name must match its function spec name" + ); + Self { + tool_name, + spec, + output_schema: None, + exposure: ToolExposure::Published, + runtime, + } + } + + /// Returns the callable tool name, including any namespace. + pub fn tool_name(&self) -> &ToolName { + &self.tool_name + } + + /// Returns the function-tool metadata exposed to the model. + pub fn spec(&self) -> &FunctionToolSpec { + &self.spec + } + + /// Returns the optional tool-output schema kept alongside the model spec. + pub fn output_schema(&self) -> Option<&Value> { + self.output_schema.as_ref() + } + + /// Returns how this tool should be exposed to the model. + pub fn exposure(&self) -> ToolExposure { + self.exposure + } + + /// Returns the runtime object bound to this tool definition. + pub fn runtime(&self) -> &R { + &self.runtime + } + + /// Rebinds the same tool definition to a different runtime object. + pub fn with_runtime(self, runtime: S) -> ToolDefinition { + ToolDefinition { + tool_name: self.tool_name, + spec: self.spec, + output_schema: self.output_schema, + exposure: self.exposure, + runtime, + } + } + + /// Attaches a tool-output schema. + pub fn with_output_schema(mut self, output_schema: Value) -> Self { + self.output_schema = Some(output_schema); + self + } + + /// Marks this tool as deferred. Deferred tools intentionally omit output + /// schema metadata until they are loaded. + pub fn deferred(mut self) -> Self { + self.exposure = ToolExposure::Deferred; + self.output_schema = None; + self + } +} + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_eq; + use serde_json::json; + + use super::ToolDefinition; + use super::ToolExposure; + use crate::FunctionToolSpec; + use crate::ToolName; + + fn definition() -> ToolDefinition<()> { + ToolDefinition::new( + ToolName::namespaced("mcp__calendar", "create_event"), + FunctionToolSpec { + name: "create_event".to_string(), + description: "Create an event.".to_string(), + strict: false, + parameters: json!({ "type": "object" }), + }, + (), + ) + .with_output_schema(json!({ "type": "object" })) + } + + #[test] + fn deferred_tools_drop_output_schema() { + let definition = definition().deferred(); + + assert_eq!(definition.exposure(), ToolExposure::Deferred); + assert_eq!(definition.output_schema(), None); + } + + #[test] + fn runtime_can_be_rebound_without_rebuilding_metadata() { + let definition = definition().with_runtime("handler"); + + assert_eq!( + definition.tool_name(), + &ToolName::namespaced("mcp__calendar", "create_event") + ); + assert_eq!(definition.runtime(), &"handler"); + } +} diff --git a/codex-rs/tool-api/src/lib.rs b/codex-rs/tool-api/src/lib.rs index 11788f5649..dc61816bc2 100644 --- a/codex-rs/tool-api/src/lib.rs +++ b/codex-rs/tool-api/src/lib.rs @@ -3,6 +3,7 @@ mod bundle; mod call; +mod definition; mod error; mod spec; @@ -10,5 +11,8 @@ pub use bundle::ToolBundle; pub use bundle::ToolExecutor; pub use bundle::ToolFuture; pub use call::ToolCall; +pub use codex_protocol::ToolName; +pub use definition::ToolDefinition; +pub use definition::ToolExposure; pub use error::ToolError; pub use spec::FunctionToolSpec; diff --git a/codex-rs/tools/Cargo.toml b/codex-rs/tools/Cargo.toml index 0029352d4b..6d9cea4f6f 100644 --- a/codex-rs/tools/Cargo.toml +++ b/codex-rs/tools/Cargo.toml @@ -12,6 +12,7 @@ codex-app-server-protocol = { workspace = true } codex-code-mode = { workspace = true } codex-features = { workspace = true } codex-protocol = { workspace = true } +codex-tool-api = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-pty = { workspace = true } rmcp = { workspace = true, default-features = false, features = [ diff --git a/codex-rs/tools/src/dynamic_tool.rs b/codex-rs/tools/src/dynamic_tool.rs index af74a3c1e3..c07ea3ae00 100644 --- a/codex-rs/tools/src/dynamic_tool.rs +++ b/codex-rs/tools/src/dynamic_tool.rs @@ -1,15 +1,25 @@ -use crate::ToolDefinition; -use crate::parse_tool_input_schema; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_tool_api::FunctionToolSpec; +use codex_tool_api::ToolDefinition; +use codex_tool_api::ToolName; -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)?, - output_schema: None, - defer_loading: tool.defer_loading, - }) +pub fn parse_dynamic_tool(tool: &DynamicToolSpec) -> ToolDefinition<()> { + let definition = ToolDefinition::new( + ToolName::new(tool.namespace.clone(), tool.name.clone()), + FunctionToolSpec { + name: tool.name.clone(), + description: tool.description.clone(), + strict: false, + parameters: tool.input_schema.clone(), + }, + (), + ); + + if tool.defer_loading { + definition.deferred() + } else { + definition + } } #[cfg(test)] diff --git a/codex-rs/tools/src/dynamic_tool_tests.rs b/codex-rs/tools/src/dynamic_tool_tests.rs index 077a0622ce..e330e0028a 100644 --- a/codex-rs/tools/src/dynamic_tool_tests.rs +++ b/codex-rs/tools/src/dynamic_tool_tests.rs @@ -1,12 +1,13 @@ use super::parse_dynamic_tool; -use crate::JsonSchema; use crate::ToolDefinition; +use crate::ToolExposure; +use crate::ToolName; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_tool_api::FunctionToolSpec; use pretty_assertions::assert_eq; -use std::collections::BTreeMap; #[test] -fn parse_dynamic_tool_sanitizes_input_schema() { +fn parse_dynamic_tool_preserves_definition_metadata() { let tool = DynamicToolSpec { namespace: None, name: "lookup_ticket".to_string(), @@ -22,21 +23,23 @@ fn parse_dynamic_tool_sanitizes_input_schema() { }; assert_eq!( - parse_dynamic_tool(&tool).expect("parse dynamic tool"), - 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 - ), - output_schema: None, - defer_loading: false, - } + parse_dynamic_tool(&tool), + ToolDefinition::new( + ToolName::plain("lookup_ticket"), + FunctionToolSpec { + name: "lookup_ticket".to_string(), + description: "Fetch a ticket".to_string(), + strict: false, + parameters: serde_json::json!({ + "properties": { + "id": { + "description": "Ticket identifier" + } + } + }), + }, + (), + ) ); } @@ -53,18 +56,8 @@ fn parse_dynamic_tool_preserves_defer_loading() { defer_loading: true, }; - assert_eq!( - parse_dynamic_tool(&tool).expect("parse dynamic tool"), - ToolDefinition { - name: "lookup_ticket".to_string(), - description: "Fetch a ticket".to_string(), - input_schema: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), - output_schema: None, - defer_loading: true, - } - ); + let definition = parse_dynamic_tool(&tool); + + assert_eq!(definition.exposure(), ToolExposure::Deferred); + assert_eq!(definition.tool_name(), &ToolName::plain("lookup_ticket")); } diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 8c63e6e631..16c0d4dfaa 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -9,7 +9,6 @@ mod mcp_tool; mod request_plugin_install; mod responses_api; mod tool_config; -mod tool_definition; mod tool_discovery; mod tool_spec; @@ -19,6 +18,8 @@ pub use code_mode::collect_code_mode_exec_prompt_tool_definitions; pub use code_mode::collect_code_mode_tool_definitions; pub use code_mode::tool_spec_to_code_mode_tool_definition; pub use codex_protocol::ToolName; +pub use codex_tool_api::ToolDefinition; +pub use codex_tool_api::ToolExposure; pub use dynamic_tool::parse_dynamic_tool; pub use image_detail::can_request_original_image_detail; pub use image_detail::normalize_output_image_detail; @@ -29,7 +30,7 @@ pub use json_schema::JsonSchemaPrimitiveType; pub use json_schema::JsonSchemaType; 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 mcp_tool::mcp_tool_definition; pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_APPROVAL_KIND_VALUE; pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE; pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_PERSIST_KEY; @@ -50,7 +51,7 @@ pub use responses_api::default_namespace_description; pub use responses_api::dynamic_tool_to_loadable_tool_spec; pub use responses_api::dynamic_tool_to_responses_api_tool; pub use responses_api::mcp_tool_to_deferred_responses_api_tool; -pub use responses_api::mcp_tool_to_responses_api_tool; +pub use responses_api::tool_definition_to_loadable_tool_spec; pub use responses_api::tool_definition_to_responses_api_tool; pub use tool_config::ShellCommandBackendConfig; pub use tool_config::ToolEnvironmentMode; @@ -60,7 +61,6 @@ pub use tool_config::ToolsConfigParams; pub use tool_config::UnifiedExecShellMode; pub use tool_config::ZshForkConfig; pub use tool_config::request_user_input_available_modes; -pub use tool_definition::ToolDefinition; pub use tool_discovery::DiscoverablePluginInfo; pub use tool_discovery::DiscoverableTool; pub use tool_discovery::DiscoverableToolAction; diff --git a/codex-rs/tools/src/mcp_tool.rs b/codex-rs/tools/src/mcp_tool.rs index 337a8e42ad..1f0e6b1caf 100644 --- a/codex-rs/tools/src/mcp_tool.rs +++ b/codex-rs/tools/src/mcp_tool.rs @@ -1,9 +1,10 @@ -use crate::ToolDefinition; -use crate::parse_tool_input_schema; +use codex_tool_api::FunctionToolSpec; +use codex_tool_api::ToolDefinition; +use codex_tool_api::ToolName; use serde_json::Value as JsonValue; use serde_json::json; -pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result { +pub fn mcp_tool_definition(tool_name: ToolName, tool: &rmcp::model::Tool) -> ToolDefinition<()> { 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 @@ -18,22 +19,25 @@ pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result JsonValue { diff --git a/codex-rs/tools/src/mcp_tool_tests.rs b/codex-rs/tools/src/mcp_tool_tests.rs index 5a94526329..c1b9da444d 100644 --- a/codex-rs/tools/src/mcp_tool_tests.rs +++ b/codex-rs/tools/src/mcp_tool_tests.rs @@ -1,9 +1,9 @@ use super::mcp_call_tool_result_output_schema; -use super::parse_mcp_tool; -use crate::JsonSchema; +use super::mcp_tool_definition; use crate::ToolDefinition; +use crate::ToolName; +use codex_tool_api::FunctionToolSpec; 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 { @@ -30,21 +30,42 @@ fn parse_mcp_tool_inserts_empty_properties() { ); assert_eq!( - parse_mcp_tool(&tool).expect("parse MCP tool"), - ToolDefinition { - name: "no_props".to_string(), - description: "No properties".to_string(), - input_schema: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), - output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))), - defer_loading: false, - } + mcp_tool_definition(ToolName::plain("no_props"), &tool), + ToolDefinition::new( + ToolName::plain("no_props"), + FunctionToolSpec { + name: "no_props".to_string(), + description: "No properties".to_string(), + strict: false, + parameters: serde_json::json!({ + "type": "object", + "properties": {} + }), + }, + (), + ) + .with_output_schema(mcp_call_tool_result_output_schema(serde_json::json!({}))) ); } +#[test] +fn mcp_tool_definition_uses_callable_name() { + let tool = mcp_tool( + "calendar_create_event", + "Create an event", + serde_json::json!({ + "type": "object" + }), + ); + + let definition = mcp_tool_definition( + ToolName::namespaced("mcp__codex_apps__calendar", "_create_event"), + &tool, + ); + + assert_eq!(definition.spec().name, "_create_event"); +} + #[test] fn parse_mcp_tool_preserves_top_level_output_schema() { let mut tool = mcp_tool( @@ -68,27 +89,30 @@ fn parse_mcp_tool_preserves_top_level_output_schema() { ))); assert_eq!( - parse_mcp_tool(&tool).expect("parse MCP tool"), - ToolDefinition { - name: "with_output".to_string(), - description: "Has output schema".to_string(), - input_schema: JsonSchema::object( - BTreeMap::new(), - /*required*/ None, - /*additional_properties*/ None - ), - output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({ - "properties": { - "result": { - "properties": { - "nested": {} - } + mcp_tool_definition(ToolName::plain("with_output"), &tool), + ToolDefinition::new( + ToolName::plain("with_output"), + FunctionToolSpec { + name: "with_output".to_string(), + description: "Has output schema".to_string(), + strict: false, + parameters: serde_json::json!({ + "type": "object", + "properties": {} + }), + }, + (), + ) + .with_output_schema(mcp_call_tool_result_output_schema(serde_json::json!({ + "properties": { + "result": { + "properties": { + "nested": {} } - }, - "required": ["result"] - }))), - defer_loading: false, - } + } + }, + "required": ["result"] + }))) ); } @@ -108,19 +132,22 @@ fn parse_mcp_tool_preserves_output_schema_without_inferred_type() { ))); assert_eq!( - parse_mcp_tool(&tool).expect("parse MCP tool"), - 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 - ), - output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({ - "enum": ["ok", "error"] - }))), - defer_loading: false, - } + mcp_tool_definition(ToolName::plain("with_enum_output"), &tool), + ToolDefinition::new( + ToolName::plain("with_enum_output"), + FunctionToolSpec { + name: "with_enum_output".to_string(), + description: "Has enum output schema".to_string(), + strict: false, + parameters: serde_json::json!({ + "type": "object", + "properties": {} + }), + }, + (), + ) + .with_output_schema(mcp_call_tool_result_output_schema(serde_json::json!({ + "enum": ["ok", "error"] + }))) ); } diff --git a/codex-rs/tools/src/responses_api.rs b/codex-rs/tools/src/responses_api.rs index a5b26abae4..29f865e957 100644 --- a/codex-rs/tools/src/responses_api.rs +++ b/codex-rs/tools/src/responses_api.rs @@ -1,9 +1,11 @@ use crate::JsonSchema; -use crate::ToolDefinition; use crate::ToolName; +use crate::mcp_tool_definition; use crate::parse_dynamic_tool; -use crate::parse_mcp_tool; +use crate::parse_tool_input_schema; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_tool_api::ToolDefinition; +use codex_tool_api::ToolExposure; use serde::Deserialize; use serde::Serialize; use serde_json::Value; @@ -69,24 +71,16 @@ pub enum ResponsesApiNamespaceTool { pub fn dynamic_tool_to_responses_api_tool( tool: &DynamicToolSpec, ) -> Result { - Ok(tool_definition_to_responses_api_tool(parse_dynamic_tool( - tool, - )?)) + tool_definition_to_responses_api_tool(&parse_dynamic_tool(tool)) } pub fn dynamic_tool_to_loadable_tool_spec( tool: &DynamicToolSpec, ) -> Result { - let output_tool = dynamic_tool_to_responses_api_tool(tool)?; - Ok(match tool.namespace.as_ref() { - Some(namespace) => LoadableToolSpec::Namespace(ResponsesApiNamespace { - name: namespace.clone(), - // the user doesn't provide a description for dynamic tools, so we use the default - description: default_namespace_description(namespace), - tools: vec![ResponsesApiNamespaceTool::Function(output_tool)], - }), - None => LoadableToolSpec::Function(output_tool), - }) + tool_definition_to_loadable_tool_spec( + &parse_dynamic_tool(tool), + /*namespace_description*/ None, + ) } pub fn coalesce_loadable_tool_specs( @@ -119,35 +113,41 @@ pub fn coalesce_loadable_tool_specs( coalesced_specs } -pub fn mcp_tool_to_responses_api_tool( - tool_name: &ToolName, - tool: &rmcp::model::Tool, -) -> Result { - Ok(tool_definition_to_responses_api_tool( - parse_mcp_tool(tool)?.renamed(tool_name.name.clone()), - )) -} - pub fn mcp_tool_to_deferred_responses_api_tool( tool_name: &ToolName, tool: &rmcp::model::Tool, ) -> Result { - Ok(tool_definition_to_responses_api_tool( - parse_mcp_tool(tool)? - .renamed(tool_name.name.clone()) - .into_deferred(), - )) + tool_definition_to_responses_api_tool(&mcp_tool_definition(tool_name.clone(), tool).deferred()) } -pub fn tool_definition_to_responses_api_tool(tool_definition: ToolDefinition) -> ResponsesApiTool { - ResponsesApiTool { - name: tool_definition.name, - description: tool_definition.description, - strict: false, - defer_loading: tool_definition.defer_loading.then_some(true), - parameters: tool_definition.input_schema, - output_schema: tool_definition.output_schema, - } +pub fn tool_definition_to_responses_api_tool( + tool_definition: &ToolDefinition, +) -> Result { + let spec = tool_definition.spec(); + Ok(ResponsesApiTool { + name: tool_definition.tool_name().name.clone(), + description: spec.description.clone(), + strict: spec.strict, + defer_loading: matches!(tool_definition.exposure(), ToolExposure::Deferred).then_some(true), + parameters: parse_tool_input_schema(&spec.parameters)?, + output_schema: tool_definition.output_schema().cloned(), + }) +} + +pub fn tool_definition_to_loadable_tool_spec( + tool_definition: &ToolDefinition, + namespace_description: Option, +) -> Result { + let output_tool = tool_definition_to_responses_api_tool(tool_definition)?; + Ok(match tool_definition.tool_name().namespace.as_ref() { + Some(namespace) => LoadableToolSpec::Namespace(ResponsesApiNamespace { + name: namespace.clone(), + description: namespace_description + .unwrap_or_else(|| default_namespace_description(namespace)), + tools: vec![ResponsesApiNamespaceTool::Function(output_tool)], + }), + None => LoadableToolSpec::Function(output_tool), + }) } #[cfg(test)] diff --git a/codex-rs/tools/src/responses_api_tests.rs b/codex-rs/tools/src/responses_api_tests.rs index 3ce13ebfe8..20089a1e5a 100644 --- a/codex-rs/tools/src/responses_api_tests.rs +++ b/codex-rs/tools/src/responses_api_tests.rs @@ -9,6 +9,7 @@ use crate::JsonSchema; use crate::ToolDefinition; use crate::ToolName; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_tool_api::FunctionToolSpec; use pretty_assertions::assert_eq; use serde_json::json; use std::collections::BTreeMap; @@ -16,20 +17,27 @@ use std::collections::BTreeMap; #[test] fn tool_definition_to_responses_api_tool_omits_false_defer_loading() { assert_eq!( - tool_definition_to_responses_api_tool(ToolDefinition { - name: "lookup_order".to_string(), - description: "Look up an order".to_string(), - input_schema: JsonSchema::object( - BTreeMap::from([( - "order_id".to_string(), - JsonSchema::string(/*description*/ None), - )]), - Some(vec!["order_id".to_string()]), - Some(false.into()) - ), - output_schema: Some(json!({"type": "object"})), - defer_loading: false, - }), + tool_definition_to_responses_api_tool( + &ToolDefinition::new( + ToolName::plain("lookup_order"), + FunctionToolSpec { + name: "lookup_order".to_string(), + description: "Look up an order".to_string(), + strict: false, + parameters: json!({ + "type": "object", + "properties": { + "order_id": {"type": "string"} + }, + "required": ["order_id"], + "additionalProperties": false, + }), + }, + (), + ) + .with_output_schema(json!({"type": "object"})), + ) + .expect("convert definition"), ResponsesApiTool { name: "lookup_order".to_string(), description: "Look up an order".to_string(), diff --git a/codex-rs/tools/src/tool_definition.rs b/codex-rs/tools/src/tool_definition.rs deleted file mode 100644 index d3c5b0209b..0000000000 --- a/codex-rs/tools/src/tool_definition.rs +++ /dev/null @@ -1,30 +0,0 @@ -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 deleted file mode 100644 index 0d4d97f5ce..0000000000 --- a/codex-rs/tools/src/tool_definition_tests.rs +++ /dev/null @@ -1,43 +0,0 @@ -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( - 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() - } - ); -}