From 9c5dfa7b1a8ee8672e1313859ca0d11b3817b932 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 13 May 2026 12:12:06 +0200 Subject: [PATCH] Refactor extension tools onto shared ToolExecutor (#22369) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Extension tools were split across two public runtime contracts: `codex-tool-api` exposed `ToolBundle` plus its own call/spec/error types, while core native tools used `codex_tools::ToolExecutor`. That made contributed tool specs and execution behavior easy to drift apart and added another crate boundary for what should be one executable-tool seam. This PR makes `ToolExecutor` the single runtime contract and keeps extension-specific pinning in `codex-extension-api`. ## Remaining todo https://github.com/openai/codex/pull/22369/changes#diff-b935ea8245c3ce568a30cff660175fa6390b66b872ae409e1e2e965738250741R5 Either generic `Invocation` or sub-extract the `ToolCall` and clean `ToolInvocation` ## What changed - Removed the `codex-tool-api` workspace crate and its dependencies from core and `codex-extension-api`. - Made `codex_tools::ToolExecutor` object-safe with `async_trait` so extension contributors can return a dyn executor. - Added the extension-facing aliases under `ext/extension-api/src/contributors/tools.rs`, including `ExtensionToolExecutor = dyn ToolExecutor`. - Changed `ToolContributor::tools` to return extension executors directly instead of `ToolBundle`s. - Updated core’s extension tool handler/registry/router path to adapt those extension executors into the existing native `ToolInvocation` runtime path. - Added focused coverage for extension tools being registered, model-visible, dispatchable, and not replacing built-in tools. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-extension-api` --- codex-rs/Cargo.lock | 12 +- codex-rs/Cargo.toml | 2 - codex-rs/core/Cargo.toml | 1 - codex-rs/core/src/session/tests.rs | 4 +- codex-rs/core/src/session/turn.rs | 4 +- .../src/tools/handlers/extension_tools.rs | 167 ++++++------------ codex-rs/core/src/tools/registry.rs | 24 +-- codex-rs/core/src/tools/router.rs | 10 +- codex-rs/core/src/tools/router_tests.rs | 77 ++++---- codex-rs/core/src/tools/spec.rs | 7 +- codex-rs/core/src/tools/spec_plan.rs | 14 +- codex-rs/core/src/tools/spec_plan_tests.rs | 87 +++++---- codex-rs/core/src/tools/spec_plan_types.rs | 5 +- codex-rs/core/src/tools/spec_tests.rs | 6 +- codex-rs/ext/extension-api/Cargo.toml | 2 +- .../ext/extension-api/src/contributors.rs | 13 +- .../extension-api/src/contributors/tools.rs | 32 ++++ codex-rs/ext/extension-api/src/lib.rs | 17 +- codex-rs/tool-api/BUILD.bazel | 6 - codex-rs/tool-api/Cargo.toml | 20 --- codex-rs/tool-api/README.md | 29 --- codex-rs/tool-api/src/bundle.rs | 87 --------- codex-rs/tool-api/src/call.rs | 5 - codex-rs/tool-api/src/error.rs | 22 --- codex-rs/tool-api/src/lib.rs | 14 -- codex-rs/tool-api/src/spec.rs | 10 -- codex-rs/tools/README.md | 13 +- codex-rs/tools/src/lib.rs | 3 + codex-rs/tools/src/tool_call.rs | 23 +++ codex-rs/tools/src/tool_output.rs | 83 +++++++++ 30 files changed, 349 insertions(+), 450 deletions(-) create mode 100644 codex-rs/ext/extension-api/src/contributors/tools.rs delete mode 100644 codex-rs/tool-api/BUILD.bazel delete mode 100644 codex-rs/tool-api/Cargo.toml delete mode 100644 codex-rs/tool-api/README.md delete mode 100644 codex-rs/tool-api/src/bundle.rs delete mode 100644 codex-rs/tool-api/src/call.rs delete mode 100644 codex-rs/tool-api/src/error.rs delete mode 100644 codex-rs/tool-api/src/lib.rs delete mode 100644 codex-rs/tool-api/src/spec.rs create mode 100644 codex-rs/tools/src/tool_call.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 47cba4ad5..c2b95f5a9 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2518,7 +2518,6 @@ dependencies = [ "codex-terminal-detection", "codex-test-binary-support", "codex-thread-store", - "codex-tool-api", "codex-tools", "codex-utils-absolute-path", "codex-utils-cache", @@ -2820,7 +2819,7 @@ name = "codex-extension-api" version = "0.0.0" dependencies = [ "codex-protocol", - "codex-tool-api", + "codex-tools", ] [[package]] @@ -3710,15 +3709,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "codex-tool-api" -version = "0.0.0" -dependencies = [ - "pretty_assertions", - "serde_json", - "thiserror 2.0.18", -] - [[package]] name = "codex-tools" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 612ca8a7e..233edcdee 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -108,7 +108,6 @@ members = [ "test-binary-support", "thread-manager-sample", "thread-store", - "tool-api", "uds", "codex-experimental-api-macros", "plugin", @@ -209,7 +208,6 @@ codex-stdio-to-uds = { path = "stdio-to-uds" } codex-terminal-detection = { path = "terminal-detection" } codex-test-binary-support = { path = "test-binary-support" } codex-thread-store = { path = "thread-store" } -codex-tool-api = { path = "tool-api" } codex-tools = { path = "tools" } codex-tui = { path = "tui" } codex-uds = { path = "uds" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 28e8eaed9..8472918dc 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -60,7 +60,6 @@ codex-sandboxing = { workspace = true } codex-state = { workspace = true } codex-terminal-detection = { workspace = true } codex-thread-store = { workspace = true } -codex-tool-api = { workspace = true } codex-tools = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cache = { workspace = true } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 616633ee9..c5602ddaa 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -543,7 +543,7 @@ fn test_tool_runtime(session: Arc, turn_context: Arc) -> T mcp_tools: None, deferred_mcp_tools: None, discoverable_tools: None, - extension_tool_bundles: Vec::new(), + extension_tool_executors: Vec::new(), dynamic_tools: turn_context.dynamic_tools.as_slice(), }, )); @@ -8607,7 +8607,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { deferred_mcp_tools, mcp_tools: Some(tools), discoverable_tools: None, - extension_tool_bundles: Vec::new(), + extension_tool_executors: Vec::new(), dynamic_tools: turn_context.dynamic_tools.as_slice(), }, ); diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 400966420..2dd84213e 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -55,7 +55,7 @@ use crate::tools::context::SharedTurnDiffTracker; use crate::tools::parallel::ToolCallRuntime; use crate::tools::registry::ToolArgumentDiffConsumer; use crate::tools::router::ToolRouterParams; -use crate::tools::router::extension_tool_bundles; +use crate::tools::router::extension_tool_executors; use crate::turn_diff_tracker::TurnDiffTracker; use crate::turn_timing::record_turn_ttft_metric; use crate::util::backoff; @@ -1250,7 +1250,7 @@ pub(crate) async fn built_tools( mcp_tools, deferred_mcp_tools, discoverable_tools, - extension_tool_bundles: extension_tool_bundles(sess), + extension_tool_executors: extension_tool_executors(sess), dynamic_tools: turn_context.dynamic_tools.as_slice(), }, ))) diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index 7f1ee24e1..cd94d8869 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -1,9 +1,8 @@ -use codex_protocol::models::FunctionCallOutputBody; -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 std::sync::Arc; + +use codex_extension_api::ExtensionToolExecutor; +use codex_extension_api::ExtensionToolOutput; +use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolName; use codex_tools::ToolSpec; use serde_json::Value; @@ -19,46 +18,13 @@ use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; use crate::tools::registry::ToolHandler; -pub(crate) struct BundledToolOutput { - value: Value, +pub(crate) struct ExtensionToolHandler { + executor: Arc, } -impl ToolOutput for BundledToolOutput { - fn log_preview(&self) -> String { - self.value.to_string() - } - - fn success_for_logging(&self) -> bool { - true - } - - fn to_response_item(&self, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem { - ResponseInputItem::FunctionCallOutput { - call_id: call_id.to_string(), - output: FunctionCallOutputPayload { - body: FunctionCallOutputBody::Text(self.value.to_string()), - success: Some(true), - }, - } - } - - fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { - Some(self.value.clone()) - } - - fn code_mode_result(&self, _payload: &ToolPayload) -> Value { - self.value.clone() - } -} - -pub(crate) struct BundledToolHandler { - bundle: ExtensionToolBundle, - spec: ToolSpec, -} - -impl BundledToolHandler { - pub(crate) fn new(bundle: ExtensionToolBundle, spec: ToolSpec) -> Self { - Self { bundle, spec } +impl ExtensionToolHandler { + pub(crate) fn new(executor: Arc) -> Self { + Self { executor } } fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> { @@ -69,41 +35,23 @@ impl BundledToolHandler { } } -impl ToolExecutor for BundledToolHandler { - type Output = BundledToolOutput; +impl ToolExecutor for ExtensionToolHandler { + type Output = ExtensionToolOutput; fn tool_name(&self) -> ToolName { - ToolName::plain(self.bundle.tool_name()) + self.executor.tool_name() } fn spec(&self) -> Option { - Some(self.spec.clone()) + self.executor.spec() } async fn handle(&self, invocation: ToolInvocation) -> Result { - let arguments = self - .arguments_from_payload(&invocation.payload) - .ok_or_else(|| { - FunctionCallError::Fatal(format!( - "tool {} invoked with incompatible payload", - self.bundle.tool_name() - )) - })? - .to_string(); - let value = self - .bundle - .executor() - .execute(codex_tool_api::ToolCall { - call_id: invocation.call_id, - arguments, - }) - .await - .map_err(map_extension_tool_error)?; - Ok(BundledToolOutput { value }) + self.executor.handle(to_extension_call(&invocation)).await } } -impl ToolHandler for BundledToolHandler { +impl ToolHandler for ExtensionToolHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { self.arguments_from_payload(payload).is_some() } @@ -132,23 +80,11 @@ 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), - ExtensionToolError::Fatal(message) => FunctionCallError::Fatal(message), +fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { + ExtensionToolCall { + call_id: invocation.call_id.clone(), + tool_name: invocation.tool_name.clone(), + payload: invocation.payload.clone(), } } @@ -167,9 +103,7 @@ mod tests { use pretty_assertions::assert_eq; use serde_json::json; - use super::BundledToolHandler; - use super::BundledToolOutput; - use super::extension_tool_spec; + use super::ExtensionToolHandler; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -181,32 +115,43 @@ mod tests { struct StubExtensionExecutor; - impl codex_tool_api::ToolExecutor for StubExtensionExecutor { - fn execute(&self, _call: codex_tool_api::ToolCall) -> codex_tool_api::ToolFuture<'_> { - Box::pin(async { Ok(json!({ "ok": true })) }) + impl codex_extension_api::ExtensionToolExecutor for StubExtensionExecutor { + fn tool_name(&self) -> codex_tools::ToolName { + codex_tools::ToolName::plain("extension_echo") + } + + fn spec(&self) -> Option { + Some(codex_tools::ToolSpec::Function( + codex_tools::ResponsesApiTool { + name: "extension_echo".to_string(), + description: "Echoes arguments.".to_string(), + strict: true, + parameters: codex_tools::parse_tool_input_schema(&json!({ + "type": "object", + "properties": { + "message": { "type": "string" }, + }, + "required": ["message"], + "additionalProperties": false, + })) + .expect("extension schema should parse"), + output_schema: None, + defer_loading: None, + }, + )) + } + + fn handle( + &self, + _call: codex_tools::ToolCall, + ) -> codex_extension_api::ExtensionToolFuture<'_> { + Box::pin(async { Ok(codex_tools::JsonToolOutput::new(json!({ "ok": true }))) }) } } #[tokio::test] async fn exposes_generic_hook_payloads() { - let bundle = codex_tool_api::ToolBundle::new( - codex_tool_api::FunctionToolSpec { - name: "extension_echo".to_string(), - description: "Echoes arguments.".to_string(), - strict: true, - parameters: json!({ - "type": "object", - "properties": { - "message": { "type": "string" }, - }, - "required": ["message"], - "additionalProperties": false, - }), - }, - Arc::new(StubExtensionExecutor), - ); - let spec = extension_tool_spec(bundle.spec()).expect("extension spec should convert"); - let handler = BundledToolHandler::new(bundle, spec); + let handler = ExtensionToolHandler::new(Arc::new(StubExtensionExecutor)); let (session, turn) = crate::session::tests::make_session_and_context().await; let invocation = ToolInvocation { session: session.into(), @@ -220,9 +165,7 @@ mod tests { arguments: json!({ "message": "hello" }).to_string(), }, }; - let output = BundledToolOutput { - value: json!({ "ok": true }), - }; + let output = codex_tools::JsonToolOutput::new(json!({ "ok": true })); assert_eq!( ToolHandler::pre_tool_use_payload(&handler, &invocation), diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index baae26407..b8480600f 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -18,15 +18,14 @@ 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::handlers::extension_tools::ExtensionToolHandler; use crate::tools::hook_names::HookToolName; use crate::tools::tool_dispatch_trace::ToolDispatchTrace; use crate::tools::tool_search_entry::ToolSearchInfo; use crate::util::error_or_panic; +use codex_extension_api::ExtensionToolExecutor; 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 futures::future::BoxFuture; @@ -584,25 +583,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 fn register_extension_tool_executor(&mut self, executor: Arc) { + let tool_name = executor.tool_name(); if self.handlers.contains_key(&tool_name) { warn!("Skipping extension tool `{tool_name}`: handler already registered"); return; } - 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()); + if let Some(spec) = executor.spec() { + self.push_spec(spec); + } - let handler: Arc = Arc::new(BundledToolHandler::new(bundle, spec)); + let handler: Arc = Arc::new(ExtensionToolHandler::new(executor)); self.handlers.insert(tool_name, handler); } diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index c8a4679d2..abb0e1897 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -9,13 +9,13 @@ use crate::tools::registry::AnyToolResult; use crate::tools::registry::ToolArgumentDiffConsumer; use crate::tools::registry::ToolRegistry; use crate::tools::spec::build_specs_with_discoverable_tools; +use codex_extension_api::ExtensionToolExecutor; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::LocalShellAction; use codex_protocol::models::ResponseItem; use codex_protocol::models::SearchToolCallParams; use codex_protocol::models::ShellToolCallParams; -use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tools::DiscoverableTool; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; @@ -44,7 +44,7 @@ pub(crate) struct ToolRouterParams<'a> { pub(crate) mcp_tools: Option>, pub(crate) deferred_mcp_tools: Option>, pub(crate) discoverable_tools: Option>, - pub(crate) extension_tool_bundles: Vec, + pub(crate) extension_tool_executors: Vec>, pub(crate) dynamic_tools: &'a [DynamicToolSpec], } @@ -54,7 +54,7 @@ impl ToolRouter { mcp_tools, deferred_mcp_tools, discoverable_tools, - extension_tool_bundles, + extension_tool_executors, dynamic_tools, } = params; let builder = build_specs_with_discoverable_tools( @@ -62,7 +62,7 @@ impl ToolRouter { mcp_tools, deferred_mcp_tools, discoverable_tools, - &extension_tool_bundles, + &extension_tool_executors, dynamic_tools, ); let (specs, registry) = builder.build(); @@ -217,7 +217,7 @@ impl ToolRouter { } } -pub(crate) fn extension_tool_bundles(session: &Session) -> Vec { +pub(crate) fn extension_tool_executors(session: &Session) -> Vec> { session .services .extensions diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index f662c1909..b4e935efb 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -7,15 +7,14 @@ use crate::turn_diff_tracker::TurnDiffTracker; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionRegistry; use codex_extension_api::ExtensionRegistryBuilder; -use codex_extension_api::FunctionToolSpec; -use codex_extension_api::ToolBundle; -use codex_extension_api::ToolExecutor; -use codex_extension_api::ToolFuture; +use codex_extension_api::ExtensionToolExecutor; +use codex_extension_api::ExtensionToolOutput; +use codex_extension_api::ResponsesApiTool; +use codex_extension_api::ToolCall as ExtensionToolCall; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; -use codex_tool_api::ToolCall as ExtensionToolCall; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -27,7 +26,7 @@ use super::ToolCall; use super::ToolCallSource; use super::ToolRouter; use super::ToolRouterParams; -use super::extension_tool_bundles; +use super::extension_tool_executors; struct ExtensionEchoContributor; @@ -36,38 +35,46 @@ impl codex_extension_api::ToolContributor for ExtensionEchoContributor { &self, _session_store: &ExtensionData, _thread_store: &ExtensionData, - ) -> Vec { - vec![ToolBundle::new( - FunctionToolSpec { - name: "extension_echo".to_string(), - description: "Echoes arguments through an extension tool.".to_string(), - strict: true, - parameters: json!({ - "type": "object", - "properties": { - "message": { "type": "string" }, - }, - "required": ["message"], - "additionalProperties": false, - }), - }, - Arc::new(ExtensionEchoExecutor), - )] + ) -> Vec> { + vec![Arc::new(ExtensionEchoExecutor)] } } struct ExtensionEchoExecutor; -impl ToolExecutor for ExtensionEchoExecutor { - fn execute<'a>(&'a self, call: ExtensionToolCall) -> ToolFuture<'a> { +impl ExtensionToolExecutor for ExtensionEchoExecutor { + fn tool_name(&self) -> ToolName { + ToolName::plain("extension_echo") + } + + fn spec(&self) -> Option { + Some(ToolSpec::Function(ResponsesApiTool { + name: "extension_echo".to_string(), + description: "Echoes arguments through an extension tool.".to_string(), + strict: true, + parameters: codex_extension_api::parse_tool_input_schema(&json!({ + "type": "object", + "properties": { + "message": { "type": "string" }, + }, + "required": ["message"], + "additionalProperties": false, + })) + .expect("extension schema should parse"), + output_schema: None, + defer_loading: None, + })) + } + + fn handle(&self, call: ExtensionToolCall) -> codex_extension_api::ExtensionToolFuture<'_> { Box::pin(async move { - let arguments: serde_json::Value = - serde_json::from_str(&call.arguments).expect("test arguments should parse"); - Ok(json!({ + let arguments: serde_json::Value = serde_json::from_str(call.function_arguments()?) + .expect("test arguments should parse"); + Ok(ExtensionToolOutput::new(json!({ "arguments": arguments, "callId": call.call_id.clone(), "ok": true, - })) + }))) }) } } @@ -98,7 +105,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow deferred_mcp_tools: None, mcp_tools: Some(mcp_tools), discoverable_tools: None, - extension_tool_bundles: Vec::new(), + extension_tool_executors: Vec::new(), dynamic_tools: turn.dynamic_tools.as_slice(), }, ); @@ -177,7 +184,7 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { ), ]), discoverable_tools: None, - extension_tool_bundles: Vec::new(), + extension_tool_executors: Vec::new(), dynamic_tools: turn.dynamic_tools.as_slice(), }, ); @@ -212,7 +219,7 @@ async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()> deferred_mcp_tools: None, mcp_tools: None, discoverable_tools: None, - extension_tool_bundles: Vec::new(), + extension_tool_executors: Vec::new(), dynamic_tools: turn.dynamic_tools.as_slice(), }, ); @@ -264,7 +271,7 @@ async fn specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> { deferred_mcp_tools: None, mcp_tools: None, discoverable_tools: None, - extension_tool_bundles: Vec::new(), + extension_tool_executors: Vec::new(), dynamic_tools: &dynamic_tools, }, ); @@ -310,7 +317,7 @@ fn mcp_tool_info( } #[tokio::test] -async fn extension_tool_bundles_are_model_visible_and_dispatchable() -> anyhow::Result<()> { +async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow::Result<()> { let (mut session, turn) = make_session_and_context().await; session.services.extensions = extension_tool_test_registry(); @@ -320,7 +327,7 @@ async fn extension_tool_bundles_are_model_visible_and_dispatchable() -> anyhow:: deferred_mcp_tools: None, mcp_tools: None, discoverable_tools: None, - extension_tool_bundles: extension_tool_bundles(&session), + extension_tool_executors: extension_tool_executors(&session), dynamic_tools: turn.dynamic_tools.as_slice(), }, ); diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 46061adfc..1eca4d1dd 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -7,12 +7,13 @@ use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; use crate::tools::registry::ToolRegistryBuilder; use crate::tools::spec_plan::build_tool_registry_builder; use crate::tools::spec_plan_types::ToolRegistryBuildParams; +use codex_extension_api::ExtensionToolExecutor; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; -use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tools::DiscoverableTool; use codex_tools::ToolUserShellType; use codex_tools::ToolsConfig; +use std::sync::Arc; pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType { match user_shell.shell_type { @@ -29,7 +30,7 @@ pub(crate) fn build_specs_with_discoverable_tools( mcp_tools: Option>, deferred_mcp_tools: Option>, discoverable_tools: Option>, - extension_tool_bundles: &[ExtensionToolBundle], + extension_tool_executors: &[Arc], dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { let default_agent_type_description = @@ -50,7 +51,7 @@ pub(crate) fn build_specs_with_discoverable_tools( mcp_tools: mcp_tools.as_deref(), deferred_mcp_tools: deferred_mcp_tools.as_deref(), discoverable_tools: discoverable_tools.as_deref(), - extension_tool_bundles, + extension_tool_executors, dynamic_tools, default_agent_type_description: &default_agent_type_description, wait_agent_timeouts: WaitAgentTimeoutOptions { diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 6087cd653..86ff84f8e 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -27,7 +27,6 @@ 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::extension_tool_spec; use crate::tools::handlers::multi_agents::CloseAgentHandler; use crate::tools::handlers::multi_agents::ResumeAgentHandler; use crate::tools::handlers::multi_agents::SendInputHandler; @@ -49,6 +48,7 @@ use crate::tools::registry::AnyToolHandler; use crate::tools::registry::ToolRegistryBuilder; use crate::tools::spec_plan_types::ToolRegistryBuildParams; use crate::tools::spec_plan_types::agent_type_description; +use codex_extension_api::ExtensionToolExecutor; use codex_protocol::openai_models::ConfigShellToolType; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolEnvironmentMode; @@ -84,7 +84,7 @@ pub fn build_tool_registry_builder( for handler in build_code_mode_handlers( config, &handlers, - params.extension_tool_bundles, + params.extension_tool_executors, config.search_tool && !all_deferred_tools.is_empty(), ) { builder.register_any_handler(handler); @@ -134,8 +134,8 @@ pub fn build_tool_registry_builder( builder.register_handler(Arc::new(ToolSearchHandler::new(deferred_search_infos))); } - for bundle in params.extension_tool_bundles.iter().cloned() { - builder.register_tool_bundle(bundle); + for executor in params.extension_tool_executors.iter().cloned() { + builder.register_extension_tool_executor(executor); } builder @@ -144,7 +144,7 @@ pub fn build_tool_registry_builder( fn build_code_mode_handlers( config: &ToolsConfig, handlers: &[Arc], - extension_tool_bundles: &[codex_tool_api::ToolBundle], + extension_tool_executors: &[Arc], deferred_tools_available: bool, ) -> Vec> { if !config.code_mode_enabled { @@ -156,9 +156,9 @@ fn build_code_mode_handlers( .filter_map(|handler| handler.spec()) .collect::>(); code_mode_nested_tool_specs.extend( - extension_tool_bundles + extension_tool_executors .iter() - .filter_map(|bundle| extension_tool_spec(bundle.spec()).ok()), + .filter_map(|executor| executor.spec()), ); let namespace_descriptions = code_mode_namespace_descriptions(&code_mode_nested_tool_specs); let mut enabled_tools = diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 41159c630..207aa7923 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -26,6 +26,8 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::handlers::view_image_spec::create_view_image_tool; use crate::tools::registry::ToolRegistry; use codex_app_server_protocol::AppInfo; +use codex_extension_api::ExtensionToolExecutor; +use codex_extension_api::ToolCall as ExtensionToolCall; use codex_features::Feature; use codex_features::Features; use codex_mcp::ToolInfo; @@ -41,10 +43,6 @@ use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::WebSearchToolType; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; -use codex_tool_api::FunctionToolSpec; -use codex_tool_api::ToolBundle as ExtensionToolBundle; -use codex_tool_api::ToolExecutor; -use codex_tool_api::ToolFuture; use codex_tools::AdditionalProperties; use codex_tools::DiscoverablePluginInfo; use codex_tools::DiscoverableTool; @@ -67,6 +65,7 @@ use pretty_assertions::assert_eq; use serde_json::json; use std::collections::BTreeMap; use std::collections::HashMap; +use std::sync::Arc; const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps"; const DEFAULT_AGENT_TYPE_DESCRIPTION: &str = "Test agent type description."; @@ -74,32 +73,44 @@ const DEFAULT_WAIT_TIMEOUT_MS: i64 = 30_000; const MIN_WAIT_TIMEOUT_MS: i64 = 10_000; const MAX_WAIT_TIMEOUT_MS: i64 = 3_600_000; -struct UnusedExtensionExecutor; - -impl ToolExecutor for UnusedExtensionExecutor { - fn execute<'a>(&'a self, _call: codex_tool_api::ToolCall) -> ToolFuture<'a> { - Box::pin(async { panic!("spec planning should not execute extension tools") }) +fn extension_tool_executor(name: &str, description: &str) -> Arc { + struct SpecOnlyExtensionExecutor { + name: String, + description: String, } -} -fn extension_tool_bundle(name: &str, description: &str) -> ExtensionToolBundle { - ExtensionToolBundle::new( - FunctionToolSpec { - name: name.to_string(), - description: description.to_string(), - strict: true, - parameters: serde_json::to_value(JsonSchema::object( - BTreeMap::from([( - "message".to_string(), - JsonSchema::string(/*description*/ None), - )]), - Some(vec!["message".to_string()]), - Some(false.into()), - )) - .expect("extension schema should serialize"), - }, - std::sync::Arc::new(UnusedExtensionExecutor), - ) + impl ExtensionToolExecutor for SpecOnlyExtensionExecutor { + fn tool_name(&self) -> ToolName { + ToolName::plain(self.name.as_str()) + } + + fn spec(&self) -> Option { + Some(ToolSpec::Function(ResponsesApiTool { + name: self.name.clone(), + description: self.description.clone(), + strict: true, + parameters: JsonSchema::object( + BTreeMap::from([( + "message".to_string(), + JsonSchema::string(/*description*/ None), + )]), + Some(vec!["message".to_string()]), + Some(false.into()), + ), + output_schema: None, + defer_loading: None, + })) + } + + fn handle(&self, _call: ExtensionToolCall) -> codex_extension_api::ExtensionToolFuture<'_> { + Box::pin(async { panic!("spec planning should not execute extension tools") }) + } + } + + Arc::new(SpecOnlyExtensionExecutor { + name: name.to_string(), + description: description.to_string(), + }) } #[test] @@ -116,7 +127,7 @@ fn extension_tools_do_not_replace_builtin_tools() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let extension_tool_bundles = vec![extension_tool_bundle( + let extension_tool_executors = vec![extension_tool_executor( "update_plan", "Extension attempt to replace a built-in tool.", )]; @@ -125,7 +136,7 @@ fn extension_tools_do_not_replace_builtin_tools() { /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, /*discoverable_tools*/ None, - &extension_tool_bundles, + &extension_tool_executors, &[], ); @@ -1880,7 +1891,7 @@ fn request_plugin_install_is_not_registered_without_feature_flag() { "Google Calendar", "Plan events and schedules.", )]), - /*extension_tool_bundles*/ &[], + /*extension_tool_executors*/ &[], &[], ); @@ -1921,7 +1932,7 @@ fn request_plugin_install_can_be_registered_without_search_tool() { "Google Calendar", "Plan events and schedules.", )]), - /*extension_tool_bundles*/ &[], + /*extension_tool_executors*/ &[], &[], ); @@ -1986,7 +1997,7 @@ fn request_plugin_install_description_lists_discoverable_tools() { /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, Some(discoverable_tools), - /*extension_tool_bundles*/ &[], + /*extension_tool_executors*/ &[], &[], ); assert!(registry.has_handler(&ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME))); @@ -2279,7 +2290,7 @@ fn code_mode_only_exec_description_includes_extension_tool_details() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let extension_tool_bundles = vec![extension_tool_bundle( + let extension_tool_executors = vec![extension_tool_executor( "extension_echo", "Echoes arguments through an extension tool.", )]; @@ -2288,7 +2299,7 @@ fn code_mode_only_exec_description_includes_extension_tool_details() { /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, /*discoverable_tools*/ None, - &extension_tool_bundles, + &extension_tool_executors, &[], ); let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { @@ -2386,7 +2397,7 @@ fn build_specs( mcp_tools, deferred_mcp_tools, /*discoverable_tools*/ None, - /*extension_tool_bundles*/ &[], + /*extension_tool_executors*/ &[], dynamic_tools, ) } @@ -2396,7 +2407,7 @@ fn build_specs_with_discoverable_tools( mcp_tools: Option>, deferred_mcp_tools: Option>, discoverable_tools: Option>, - extension_tool_bundles: &[codex_tool_api::ToolBundle], + extension_tool_executors: &[Arc], dynamic_tools: &[DynamicToolSpec], ) -> (Vec, ToolRegistry) { let mcp_tool_inputs = mcp_tools.as_ref().map(|mcp_tools| { @@ -2411,7 +2422,7 @@ fn build_specs_with_discoverable_tools( mcp_tools: mcp_tool_inputs.as_deref(), deferred_mcp_tools: deferred_mcp_tools.as_deref(), discoverable_tools: discoverable_tools.as_deref(), - extension_tool_bundles, + extension_tool_executors, dynamic_tools, default_agent_type_description: DEFAULT_AGENT_TYPE_DESCRIPTION, wait_agent_timeouts: wait_agent_timeout_options(), diff --git a/codex-rs/core/src/tools/spec_plan_types.rs b/codex-rs/core/src/tools/spec_plan_types.rs index 30cae6096..247fa0571 100644 --- a/codex-rs/core/src/tools/spec_plan_types.rs +++ b/codex-rs/core/src/tools/spec_plan_types.rs @@ -1,16 +1,17 @@ use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; +use codex_extension_api::ExtensionToolExecutor; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; -use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tools::DiscoverableTool; use codex_tools::ToolsConfig; +use std::sync::Arc; #[derive(Clone, Copy)] pub struct ToolRegistryBuildParams<'a> { pub mcp_tools: Option<&'a [ToolInfo]>, pub deferred_mcp_tools: Option<&'a [ToolInfo]>, pub discoverable_tools: Option<&'a [DiscoverableTool]>, - pub extension_tool_bundles: &'a [ExtensionToolBundle], + pub extension_tool_executors: &'a [Arc], pub dynamic_tools: &'a [DynamicToolSpec], pub default_agent_type_description: &'a str, pub wait_agent_timeouts: WaitAgentTimeoutOptions, diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 36eb140ae..560f87b2f 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -275,7 +275,7 @@ fn build_specs( mcp_tools, deferred_mcp_tools, /*discoverable_tools*/ None, - /*extension_tool_bundles*/ &[], + /*extension_tool_executors*/ &[], dynamic_tools, ) } @@ -335,7 +335,7 @@ async fn assert_model_tools( mcp_tools: None, deferred_mcp_tools: None, discoverable_tools: None, - extension_tool_bundles: Vec::new(), + extension_tool_executors: Vec::new(), dynamic_tools: &[], }, ); @@ -804,7 +804,7 @@ async fn request_plugin_install_requires_apps_and_plugins_features() { /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, discoverable_tools.clone(), - /*extension_tool_bundles*/ &[], + /*extension_tool_executors*/ &[], &[], ) .build(); diff --git a/codex-rs/ext/extension-api/Cargo.toml b/codex-rs/ext/extension-api/Cargo.toml index b281d438e..ceecc4802 100644 --- a/codex-rs/ext/extension-api/Cargo.toml +++ b/codex-rs/ext/extension-api/Cargo.toml @@ -14,5 +14,5 @@ doctest = false workspace = true [dependencies] -codex-tool-api = { workspace = true } codex-protocol = { workspace = true } +codex-tools = { workspace = true } diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index 36057944d..99505e2af 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -1,16 +1,20 @@ use std::future::Future; +use std::sync::Arc; use codex_protocol::ThreadId; use codex_protocol::items::TurnItem; use codex_protocol::protocol::ReviewDecision; -use codex_tool_api::ToolBundle; use crate::ExtensionData; mod prompt; +mod tools; pub use prompt::PromptFragment; pub use prompt::PromptSlot; +pub use tools::ExtensionToolExecutor; +pub use tools::ExtensionToolFuture; +pub use tools::ExtensionToolOutput; /// Contributor that receives the live thread id and host-owned thread-start /// input before later contributors read from extension stores. @@ -36,8 +40,11 @@ pub trait ContextContributor: Send + Sync { /// Extension contribution that exposes native tools owned by a feature. pub trait ToolContributor: Send + Sync { /// Returns the native tools visible for the supplied extension stores. - fn tools(&self, session_store: &ExtensionData, thread_store: &ExtensionData) - -> Vec; + fn tools( + &self, + session_store: &ExtensionData, + thread_store: &ExtensionData, + ) -> Vec>; } /// Future returned by one claimed approval-review contribution. diff --git a/codex-rs/ext/extension-api/src/contributors/tools.rs b/codex-rs/ext/extension-api/src/contributors/tools.rs new file mode 100644 index 000000000..41d08986e --- /dev/null +++ b/codex-rs/ext/extension-api/src/contributors/tools.rs @@ -0,0 +1,32 @@ +use std::future::Future; +use std::pin::Pin; + +use codex_tools::FunctionCallError; +use codex_tools::JsonToolOutput; +use codex_tools::ToolCall; +use codex_tools::ToolName; +use codex_tools::ToolSpec; + +/// Model-facing output returned by extension-owned tools. +pub type ExtensionToolOutput = JsonToolOutput; + +/// Future returned by extension-owned tool execution. +pub type ExtensionToolFuture<'a> = + Pin> + Send + 'a>>; + +/// Object-safe runtime contract for extension-owned model-visible tools. +/// +/// Implementations keep an extension tool's model-visible spec attached to the +/// executable runtime that handles calls for that tool. +pub trait ExtensionToolExecutor: Send + Sync { + /// The concrete tool name handled by this extension runtime. + fn tool_name(&self) -> ToolName; + + /// The model-visible spec for this extension tool. + fn spec(&self) -> Option { + None + } + + /// Execute one extension tool invocation. + fn handle(&self, call: ToolCall) -> ExtensionToolFuture<'_>; +} diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index d5e439212..f1667b777 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -5,15 +5,20 @@ mod state; pub use capabilities::AgentSpawnFuture; pub use capabilities::AgentSpawner; -pub use codex_tool_api::FunctionToolSpec; -pub use codex_tool_api::ToolBundle; -pub use codex_tool_api::ToolCall; -pub use codex_tool_api::ToolError; -pub use codex_tool_api::ToolExecutor; -pub use codex_tool_api::ToolFuture; +pub use codex_tools::FunctionCallError; +pub use codex_tools::JsonToolOutput; +pub use codex_tools::ResponsesApiTool; +pub use codex_tools::ToolCall; +pub use codex_tools::ToolName; +pub use codex_tools::ToolPayload; +pub use codex_tools::ToolSpec; +pub use codex_tools::parse_tool_input_schema; pub use contributors::ApprovalReviewContributor; pub use contributors::ApprovalReviewFuture; pub use contributors::ContextContributor; +pub use contributors::ExtensionToolExecutor; +pub use contributors::ExtensionToolFuture; +pub use contributors::ExtensionToolOutput; pub use contributors::PromptFragment; pub use contributors::PromptSlot; pub use contributors::ThreadStartContributor; diff --git a/codex-rs/tool-api/BUILD.bazel b/codex-rs/tool-api/BUILD.bazel deleted file mode 100644 index 1dc9eb3bf..000000000 --- a/codex-rs/tool-api/BUILD.bazel +++ /dev/null @@ -1,6 +0,0 @@ -load("//:defs.bzl", "codex_rust_crate") - -codex_rust_crate( - name = "tool-api", - crate_name = "codex_tool_api", -) diff --git a/codex-rs/tool-api/Cargo.toml b/codex-rs/tool-api/Cargo.toml deleted file mode 100644 index 13d1a2207..000000000 --- a/codex-rs/tool-api/Cargo.toml +++ /dev/null @@ -1,20 +0,0 @@ -[package] -edition.workspace = true -license.workspace = true -name = "codex-tool-api" -version.workspace = true - -[lib] -name = "codex_tool_api" -path = "src/lib.rs" -doctest = false - -[lints] -workspace = true - -[dependencies] -serde_json = { workspace = true } -thiserror = { workspace = true } - -[dev-dependencies] -pretty_assertions = { workspace = true } diff --git a/codex-rs/tool-api/README.md b/codex-rs/tool-api/README.md deleted file mode 100644 index 418c1d177..000000000 --- a/codex-rs/tool-api/README.md +++ /dev/null @@ -1,29 +0,0 @@ -# codex-tool-api - -`codex-tool-api` is the minimal extension-facing contract for contributed -function tools that can be injected into Codex without making `codex-core` -depend on the tool owner's crate. - -Crates that define contributed tools should depend on this crate. It owns: - -- the executable bundle contract: `ToolBundle`, `ToolExecutor`, `ToolCall`, - and `ToolError` -- 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 intended dependency direction is: - -```text -tool-owning extension crate --> codex-tool-api <-- codex-core -``` - -`codex-tools` has a different job. It remains the host-side owner of Responses -API tool models, schema parsing, namespaces, discovery, MCP/dynamic conversion, -code-mode shaping, and other aggregate host concerns. A crate that only wants -to contribute one ordinary function tool through an extension should not need -to depend on `codex-tools`. diff --git a/codex-rs/tool-api/src/bundle.rs b/codex-rs/tool-api/src/bundle.rs deleted file mode 100644 index 18c84c4c0..000000000 --- a/codex-rs/tool-api/src/bundle.rs +++ /dev/null @@ -1,87 +0,0 @@ -use std::future::Future; -use std::pin::Pin; -use std::sync::Arc; - -use serde_json::Value; - -use crate::FunctionToolSpec; -use crate::ToolCall; -use crate::ToolError; - -/// Future returned by one contributed function-tool invocation. -pub type ToolFuture<'a> = Pin> + Send + 'a>>; - -/// Model-visible definition plus executable implementation for one contributed -/// function tool. -#[derive(Clone)] -pub struct ToolBundle { - spec: FunctionToolSpec, - executor: Arc, -} - -impl ToolBundle { - /// Creates one contributed function-tool bundle. - pub fn new(spec: FunctionToolSpec, executor: Arc) -> Self { - Self { spec, executor } - } - - /// Returns the contributed function-tool spec. - pub fn spec(&self) -> &FunctionToolSpec { - &self.spec - } - - /// Returns the contributed function-tool name. - pub fn tool_name(&self) -> &str { - self.spec.name.as_str() - } - - /// Returns the executable implementation. - pub fn executor(&self) -> Arc { - Arc::clone(&self.executor) - } -} - -/// Executable behavior for one contributed function tool. -/// -/// Implementations receive the model-supplied call id and JSON arguments and -/// return the JSON value that should be exposed to the model. -pub trait ToolExecutor: Send + Sync { - fn execute<'a>(&'a self, call: ToolCall) -> ToolFuture<'a>; -} - -#[cfg(test)] -mod tests { - use std::sync::Arc; - - use pretty_assertions::assert_eq; - use serde_json::json; - - use super::ToolBundle; - use super::ToolExecutor; - use super::ToolFuture; - use crate::FunctionToolSpec; - use crate::ToolCall; - - struct StubExecutor; - - impl ToolExecutor for StubExecutor { - fn execute<'a>(&'a self, _call: ToolCall) -> ToolFuture<'a> { - Box::pin(async { Ok(json!({ "ok": true })) }) - } - } - - #[test] - fn bundle_derives_name_from_function_spec() { - let bundle = ToolBundle::new( - FunctionToolSpec { - name: "echo".to_string(), - description: "Echo arguments.".to_string(), - strict: false, - parameters: json!({ "type": "object" }), - }, - Arc::new(StubExecutor), - ); - - assert_eq!(bundle.tool_name(), "echo"); - } -} diff --git a/codex-rs/tool-api/src/call.rs b/codex-rs/tool-api/src/call.rs deleted file mode 100644 index bcfab2092..000000000 --- a/codex-rs/tool-api/src/call.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// One contributed function-tool call. -pub struct ToolCall { - pub call_id: String, - pub arguments: String, -} diff --git a/codex-rs/tool-api/src/error.rs b/codex-rs/tool-api/src/error.rs deleted file mode 100644 index c4d30e63d..000000000 --- a/codex-rs/tool-api/src/error.rs +++ /dev/null @@ -1,22 +0,0 @@ -use thiserror::Error; - -/// Error returned by a contributed executable tool. -#[derive(Clone, Debug, Error, PartialEq, Eq)] -pub enum ToolError { - #[error("{0}")] - RespondToModel(String), - #[error("fatal tool error: {0}")] - Fatal(String), -} - -impl ToolError { - /// Creates a model-visible tool error. - pub fn respond_to_model(message: impl Into) -> Self { - Self::RespondToModel(message.into()) - } - - /// Creates a host-fatal tool error. - pub fn fatal(message: impl Into) -> Self { - Self::Fatal(message.into()) - } -} diff --git a/codex-rs/tool-api/src/lib.rs b/codex-rs/tool-api/src/lib.rs deleted file mode 100644 index 11788f564..000000000 --- a/codex-rs/tool-api/src/lib.rs +++ /dev/null @@ -1,14 +0,0 @@ -//! Minimal function-tool contracts shared between hosts and extension-owned -//! tool crates. - -mod bundle; -mod call; -mod error; -mod spec; - -pub use bundle::ToolBundle; -pub use bundle::ToolExecutor; -pub use bundle::ToolFuture; -pub use call::ToolCall; -pub use error::ToolError; -pub use spec::FunctionToolSpec; diff --git a/codex-rs/tool-api/src/spec.rs b/codex-rs/tool-api/src/spec.rs deleted file mode 100644 index 1b25837bb..000000000 --- a/codex-rs/tool-api/src/spec.rs +++ /dev/null @@ -1,10 +0,0 @@ -use serde_json::Value; - -/// Model-visible definition for one contributed function tool. -#[derive(Debug, Clone, PartialEq)] -pub struct FunctionToolSpec { - pub name: String, - pub description: String, - pub strict: bool, - pub parameters: Value, -} diff --git a/codex-rs/tools/README.md b/codex-rs/tools/README.md index 79035dde3..68f408095 100644 --- a/codex-rs/tools/README.md +++ b/codex-rs/tools/README.md @@ -1,11 +1,7 @@ # codex-tools -`codex-tools` is the host-side support crate for building, adapting, and -planning tool sets outside `codex-core`. - -It is deliberately not the API that external tool owners should depend on. -Crates that contribute ordinary function tools through extensions should use -`codex-tool-api`, which owns only that small executable-tool seam. +`codex-tools` is the shared support crate for building, adapting, planning, and +executing model-visible tool sets outside `codex-core`. Today this crate owns the host-facing tool models and helpers that no longer need to live in `core/src/tools/spec.rs` or `core/src/client_common.rs`: @@ -17,6 +13,8 @@ need to live in `core/src/tools/spec.rs` or `core/src/client_common.rs`: `ToolsConfig`, discoverable-tool models, and request-plugin-install helpers - host adapters such as schema sanitization, MCP/dynamic conversion, code-mode augmentation, and image-detail normalization +- shared executable-tool contracts such as `ToolExecutor`, `ToolCall`, and + `ToolOutput` 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 @@ -37,7 +35,6 @@ multiple consumers, for example: The corresponding non-goals are just as important: -- do not become the extension-facing authoring API for executable tools - do not move `codex-core` orchestration here prematurely - do not pull `Session` / `TurnContext` / approval flow / runtime execution logic into this crate unless those dependencies have first been split into @@ -48,7 +45,7 @@ The corresponding non-goals are just as important: The expected migration shape is: -1. Keep ordinary contributed function-tool authoring in `codex-tool-api`. +1. Keep extension-owned executable-tool authoring in `codex-extension-api`. 2. Move host-side planning/adaptation helpers here when they no longer need to stay coupled to `codex-core`. 3. Leave compatibility-sensitive adapters in `codex-core` while downstream diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 554ee61af..86e4750b3 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -9,6 +9,7 @@ mod json_schema; mod mcp_tool; mod request_plugin_install; mod responses_api; +mod tool_call; mod tool_config; mod tool_definition; mod tool_discovery; @@ -56,6 +57,7 @@ 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_responses_api_tool; +pub use tool_call::ToolCall; pub use tool_config::ShellCommandBackendConfig; pub use tool_config::ToolEnvironmentMode; pub use tool_config::ToolUserShellType; @@ -77,6 +79,7 @@ pub use tool_discovery::ToolSearchSourceInfo; pub use tool_discovery::collect_request_plugin_install_entries; pub use tool_discovery::filter_request_plugin_install_discoverable_tools_for_client; pub use tool_executor::ToolExecutor; +pub use tool_output::JsonToolOutput; pub use tool_output::ToolOutput; pub use tool_payload::ToolPayload; pub use tool_spec::ResponsesApiWebSearchFilters; diff --git a/codex-rs/tools/src/tool_call.rs b/codex-rs/tools/src/tool_call.rs new file mode 100644 index 000000000..f12a338cf --- /dev/null +++ b/codex-rs/tools/src/tool_call.rs @@ -0,0 +1,23 @@ +use crate::FunctionCallError; +use crate::ToolName; +use crate::ToolPayload; + +// TODO: this is temporary and will disappear in the next PR (as we make codex-extension-api generic on Invocation. +#[derive(Clone, Debug)] +pub struct ToolCall { + pub call_id: String, + pub tool_name: ToolName, + pub payload: ToolPayload, +} + +impl ToolCall { + pub fn function_arguments(&self) -> Result<&str, FunctionCallError> { + match &self.payload { + ToolPayload::Function { arguments } => Ok(arguments), + _ => Err(FunctionCallError::Fatal(format!( + "tool {} invoked with incompatible payload", + self.tool_name + ))), + } + } +} diff --git a/codex-rs/tools/src/tool_output.rs b/codex-rs/tools/src/tool_output.rs index 8fc99d786..204429517 100644 --- a/codex-rs/tools/src/tool_output.rs +++ b/codex-rs/tools/src/tool_output.rs @@ -1,6 +1,7 @@ use codex_protocol::models::DEFAULT_IMAGE_DETAIL; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputContentItem; +use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_utils_string::take_bytes_at_char_boundary; use serde_json::Value as JsonValue; @@ -35,6 +36,88 @@ pub trait ToolOutput: Send { } } +impl ToolOutput for Box +where + T: ToolOutput + ?Sized, +{ + fn log_preview(&self) -> String { + (**self).log_preview() + } + + fn success_for_logging(&self) -> bool { + (**self).success_for_logging() + } + + fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem { + (**self).to_response_item(call_id, payload) + } + + fn post_tool_use_response(&self, call_id: &str, payload: &ToolPayload) -> Option { + (**self).post_tool_use_response(call_id, payload) + } + + fn code_mode_result(&self, payload: &ToolPayload) -> JsonValue { + (**self).code_mode_result(payload) + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct JsonToolOutput { + value: JsonValue, + success: Option, +} + +impl JsonToolOutput { + pub fn new(value: JsonValue) -> Self { + Self { + value, + success: Some(true), + } + } + + pub fn with_success(value: JsonValue, success: Option) -> Self { + Self { value, success } + } +} + +impl ToolOutput for JsonToolOutput { + fn log_preview(&self) -> String { + telemetry_preview(&self.value.to_string()) + } + + fn success_for_logging(&self) -> bool { + self.success.unwrap_or(true) + } + + fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem { + let output = FunctionCallOutputPayload { + body: FunctionCallOutputBody::Text(self.value.to_string()), + success: self.success, + }; + + if matches!(payload, ToolPayload::Custom { .. }) { + return ResponseInputItem::CustomToolCallOutput { + call_id: call_id.to_string(), + name: None, + output, + }; + } + + ResponseInputItem::FunctionCallOutput { + call_id: call_id.to_string(), + output, + } + } + + fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { + Some(self.value.clone()) + } + + fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue { + self.value.clone() + } +} + impl ToolOutput for codex_protocol::mcp::CallToolResult { fn log_preview(&self) -> String { let output = self.as_function_call_output_payload();