From 672cc1f6696d90c568637f3cd8cac3a97d2c639e Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 11 May 2026 16:42:29 +0200 Subject: [PATCH] feat: wire extension tool bundles into core (#22147) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why This is the next narrow step toward moving concrete tool families out of core. After #22138 introduced `codex-tool-api`, we still needed a real end-to-end seam that lets an extension own an executable tool definition once and have core install it without the temporary `extension-api` wrapper or a dependency on `codex-tools`. `codex-tool-api` is the small extension-facing execution contract, while `codex-tools` still has a different job: host-side shared tool metadata and planning logic that is not “run this contributed tool”, like spec shaping, namespaces, discovery, code-mode augmentation, and MCP/dynamic-to-Responses API conversion ## What changed - Moved the shared leaf tool-spec and JSON Schema types into `codex-tool-api`, so the executable contract now lives with [`ToolBundle`](https://github.com/openai/codex/blob/c538758095337d4fe0a52a172363ccede4066bda/codex-rs/tool-api/src/bundle.rs#L19-L70). - Replaced the temporary extension-side tool wrapper with direct `ToolBundle` use in `codex-extension-api`. - Taught core to collect contributed bundles, include them in spec planning, register them through [`ToolRegistryBuilder::register_tool_bundle`](https://github.com/openai/codex/blob/c538758095337d4fe0a52a172363ccede4066bda/codex-rs/core/src/tools/registry.rs#L653-L667), and dispatch them through the existing router/runtime path. - Added focused coverage for contributed tools becoming model-visible and dispatchable, plus spec-planning coverage for contributed function and freeform tools. ## Verification - Added `extension_tool_bundles_are_model_visible_and_dispatchable` in `core/src/tools/router_tests.rs`. - Added spec-plan coverage in `core/src/tools/spec_plan_tests.rs` for contributed extension bundles. ## Related - Follow-up to #22138 --- codex-rs/Cargo.lock | 8 +- codex-rs/Cargo.toml | 1 - codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/session/tests.rs | 2 + codex-rs/core/src/session/turn.rs | 2 + codex-rs/core/src/tools/code_mode/mod.rs | 2 + codex-rs/core/src/tools/registry.rs | 174 ++++++++++++++++++ codex-rs/core/src/tools/registry_tests.rs | 67 +++++++ codex-rs/core/src/tools/router.rs | 19 ++ codex-rs/core/src/tools/router_tests.rs | 147 +++++++++++++++ codex-rs/core/src/tools/spec.rs | 3 + codex-rs/core/src/tools/spec_plan.rs | 4 + codex-rs/core/src/tools/spec_plan_tests.rs | 119 ++++++++++++ codex-rs/core/src/tools/spec_plan_types.rs | 2 + codex-rs/core/src/tools/spec_tests.rs | 3 + codex-rs/ext/extension-api/Cargo.toml | 4 +- .../ext/extension-api/src/contributors.rs | 10 +- .../extension-api/src/contributors/tool.rs | 68 ------- codex-rs/ext/extension-api/src/lib.rs | 9 +- codex-rs/tool-api/Cargo.toml | 3 - codex-rs/tool-api/README.md | 29 +++ codex-rs/tool-api/src/bundle.rs | 134 +++++--------- codex-rs/tool-api/src/call.rs | 15 +- codex-rs/tool-api/src/lib.rs | 11 +- codex-rs/tool-api/src/output.rs | 113 ------------ codex-rs/tool-api/src/spec.rs | 10 + codex-rs/tools/README.md | 75 +++----- 27 files changed, 677 insertions(+), 358 deletions(-) delete mode 100644 codex-rs/ext/extension-api/src/contributors/tool.rs create mode 100644 codex-rs/tool-api/README.md delete mode 100644 codex-rs/tool-api/src/output.rs create mode 100644 codex-rs/tool-api/src/spec.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index cee659284..7f371f9c9 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2525,6 +2525,7 @@ dependencies = [ "codex-terminal-detection", "codex-test-binary-support", "codex-thread-store", + "codex-tool-api", "codex-tools", "codex-utils-absolute-path", "codex-utils-cache", @@ -2825,9 +2826,7 @@ name = "codex-extension-api" version = "0.0.0" dependencies = [ "codex-protocol", - "codex-tools", - "serde_json", - "thiserror 2.0.18", + "codex-tool-api", ] [[package]] @@ -3710,10 +3709,7 @@ dependencies = [ name = "codex-tool-api" version = "0.0.0" dependencies = [ - "codex-protocol", - "codex-tools", "pretty_assertions", - "serde", "serde_json", "thiserror 2.0.18", ] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 5715fe726..9cda2271d 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -474,7 +474,6 @@ unwrap_used = "deny" [workspace.metadata.cargo-shear] ignored = [ "codex-agent-graph-store", - "codex-tool-api", "icu_provider", "openssl-sys", "codex-v8-poc", diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index bf58a2235..86e7c07ad 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -60,6 +60,7 @@ 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 e331ea19e..cdec39b21 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -545,6 +545,7 @@ fn test_tool_runtime(session: Arc, turn_context: Arc) -> T unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, + extension_tool_bundles: Vec::new(), dynamic_tools: turn_context.dynamic_tools.as_slice(), }, )); @@ -8555,6 +8556,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, + extension_tool_bundles: 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 bf552a79f..4a75ed063 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -55,6 +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::turn_diff_tracker::TurnDiffTracker; use crate::turn_timing::record_turn_ttft_metric; use crate::unavailable_tool::collect_unavailable_called_tools; @@ -1268,6 +1269,7 @@ pub(crate) async fn built_tools( unavailable_called_tools, parallel_mcp_server_names, discoverable_tools, + extension_tool_bundles: extension_tool_bundles(sess), dynamic_tools: turn_context.dynamic_tools.as_slice(), }, ))) diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index 4bafd2650..0cf7ed898 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -29,6 +29,7 @@ use crate::tools::parallel::ToolCallRuntime; use crate::tools::router::ToolCall; use crate::tools::router::ToolCallSource; use crate::tools::router::ToolRouterParams; +use crate::tools::router::extension_tool_bundles; use crate::unified_exec::resolve_max_tokens; use codex_features::Feature; use codex_tools::ToolName; @@ -285,6 +286,7 @@ async fn build_nested_router(exec: &ExecContext) -> ToolRouter { unavailable_called_tools: Vec::new(), parallel_mcp_server_names, discoverable_tools: None, + extension_tool_bundles: extension_tool_bundles(exec.session.as_ref()), dynamic_tools: exec.turn.dynamic_tools.as_slice(), }, ) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 01a3b811c..838b1bda2 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -19,9 +19,14 @@ use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; use crate::tools::tool_dispatch_trace::ToolDispatchTrace; use crate::util::error_or_panic; +use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::EventMsg; +use codex_tool_api::ToolBundle as ExtensionToolBundle; +use codex_tool_api::ToolError as ExtensionToolError; use codex_tools::ConfiguredToolSpec; +use codex_tools::ResponsesApiTool; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_utils_readiness::Readiness; @@ -35,6 +40,125 @@ pub enum ToolKind { Mcp, } +struct BundledToolOutput { + value: Value, +} + +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() + } +} + +struct BundledToolHandler { + bundle: ExtensionToolBundle, + spec: ToolSpec, +} + +impl BundledToolHandler { + fn new(bundle: ExtensionToolBundle, spec: ToolSpec) -> Self { + Self { bundle, spec } + } + + fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> { + let ToolPayload::Function { arguments } = payload else { + return None; + }; + Some(arguments) + } +} + +impl ToolHandler for BundledToolHandler { + type Output = BundledToolOutput; + + fn tool_name(&self) -> ToolName { + ToolName::plain(self.bundle.tool_name()) + } + + fn spec(&self) -> Option { + Some(self.spec.clone()) + } + + fn kind(&self) -> ToolKind { + ToolKind::Function + } + + fn matches_kind(&self, payload: &ToolPayload) -> bool { + self.arguments_from_payload(payload).is_some() + } + + async fn is_mutating(&self, _invocation: &ToolInvocation) -> bool { + true + } + + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + let arguments = self.arguments_from_payload(&invocation.payload)?; + Some(PreToolUsePayload { + tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()), + tool_input: extension_tool_hook_input(arguments), + }) + } + + fn post_tool_use_payload( + &self, + invocation: &ToolInvocation, + result: &Self::Output, + ) -> Option { + let arguments = self.arguments_from_payload(&invocation.payload)?; + Some(PostToolUsePayload { + tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()), + tool_use_id: invocation.call_id.clone(), + tool_input: extension_tool_hook_input(arguments), + tool_response: result + .post_tool_use_response(&invocation.call_id, &invocation.payload)?, + }) + } + + 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 }) + } +} + pub trait ToolHandler: Send + Sync { type Output: ToolOutput + 'static; @@ -538,6 +662,28 @@ impl ToolRegistryBuilder { self.handlers.insert(name, handler); } + pub fn register_tool_bundle(&mut self, bundle: ExtensionToolBundle) { + let tool_name = ToolName::plain(bundle.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(), /*supports_parallel_tool_calls*/ false); + + let handler: Arc = Arc::new(BundledToolHandler::new(bundle, spec)); + self.handlers.insert(tool_name, handler); + } + pub(crate) fn specs(&self) -> &[ConfiguredToolSpec] { &self.specs } @@ -555,6 +701,34 @@ fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &ToolName) -> } } +fn map_extension_tool_error(error: ExtensionToolError) -> FunctionCallError { + match error { + ExtensionToolError::RespondToModel(message) => FunctionCallError::RespondToModel(message), + ExtensionToolError::Fatal(message) => FunctionCallError::Fatal(message), + } +} + +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 extension_tool_hook_input(arguments: &str) -> Value { + if arguments.trim().is_empty() { + return Value::Object(serde_json::Map::new()); + } + + serde_json::from_str(arguments).unwrap_or_else(|_| Value::String(arguments.to_string())) +} + #[cfg(test)] #[path = "registry_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index d445b196a..6ae4f57d1 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -3,6 +3,7 @@ use crate::tools::handlers::GetGoalHandler; use crate::tools::handlers::goal_spec::GET_GOAL_TOOL_NAME; use crate::tools::handlers::goal_spec::create_get_goal_tool; use pretty_assertions::assert_eq; +use serde_json::json; struct TestHandler { tool_name: codex_tools::ToolName, @@ -80,3 +81,69 @@ fn register_handler_adds_handler_and_augments_specs_for_code_mode() { ); assert!(registry.has_handler(&codex_tools::ToolName::plain(GET_GOAL_TOOL_NAME))); } + +struct StubExtensionExecutor; + +impl codex_tool_api::ToolExecutor for StubExtensionExecutor { + fn execute<'a>(&'a self, _call: codex_tool_api::ToolCall) -> codex_tool_api::ToolFuture<'a> { + Box::pin(async { Ok(json!({ "ok": true })) }) + } +} + +#[tokio::test] +async fn bundled_tool_handler_exposes_generic_hook_payloads_and_is_conservatively_mutating() { + 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 (session, turn) = crate::session::tests::make_session_and_context().await; + let invocation = ToolInvocation { + session: session.into(), + turn: turn.into(), + cancellation_token: tokio_util::sync::CancellationToken::new(), + tracker: Arc::new(tokio::sync::Mutex::new( + crate::turn_diff_tracker::TurnDiffTracker::new(), + )), + call_id: "call-extension".to_string(), + tool_name: codex_tools::ToolName::plain("extension_echo"), + source: crate::tools::context::ToolCallSource::Direct, + payload: ToolPayload::Function { + arguments: json!({ "message": "hello" }).to_string(), + }, + }; + let output = BundledToolOutput { + value: json!({ "ok": true }), + }; + + assert!(ToolHandler::is_mutating(&handler, &invocation).await); + assert_eq!( + ToolHandler::pre_tool_use_payload(&handler, &invocation), + Some(PreToolUsePayload { + tool_name: HookToolName::new("extension_echo"), + tool_input: json!({ "message": "hello" }), + }) + ); + assert_eq!( + ToolHandler::post_tool_use_payload(&handler, &invocation, &output), + Some(PostToolUsePayload { + tool_name: HookToolName::new("extension_echo"), + tool_use_id: "call-extension".to_string(), + tool_input: json!({ "message": "hello" }), + tool_response: json!({ "ok": true }), + }) + ); +} diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 609fedf96..4649caa04 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -15,6 +15,7 @@ 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::ConfiguredToolSpec; use codex_tools::DiscoverableTool; use codex_tools::ResponsesApiNamespaceTool; @@ -48,6 +49,7 @@ pub(crate) struct ToolRouterParams<'a> { pub(crate) unavailable_called_tools: Vec, pub(crate) parallel_mcp_server_names: HashSet, pub(crate) discoverable_tools: Option>, + pub(crate) extension_tool_bundles: Vec, pub(crate) dynamic_tools: &'a [DynamicToolSpec], } @@ -59,6 +61,7 @@ impl ToolRouter { unavailable_called_tools, parallel_mcp_server_names, discoverable_tools, + extension_tool_bundles, dynamic_tools, } = params; let builder = build_specs_with_discoverable_tools( @@ -67,6 +70,7 @@ impl ToolRouter { deferred_mcp_tools, unavailable_called_tools, discoverable_tools, + &extension_tool_bundles, dynamic_tools, ); let (specs, registry) = builder.build(); @@ -296,6 +300,21 @@ impl ToolRouter { } } +pub(crate) fn extension_tool_bundles(session: &Session) -> Vec { + session + .services + .extensions + .tool_contributors() + .iter() + .flat_map(|contributor| { + contributor.tools( + &session.services.session_extension_data, + &session.services.thread_extension_data, + ) + }) + .collect() +} + fn filter_deferred_dynamic_tool_spec( spec: ToolSpec, deferred_dynamic_tools: &HashSet, diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index ac859d9aa..1eacbcf6c 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -1,19 +1,83 @@ use std::collections::HashSet; use std::sync::Arc; +use crate::config::Config; use crate::session::tests::make_session_and_context; use crate::tools::context::ToolPayload; +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_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; use pretty_assertions::assert_eq; use serde_json::json; +use tokio_util::sync::CancellationToken; use super::ToolCall; +use super::ToolCallSource; use super::ToolRouter; use super::ToolRouterParams; +use super::extension_tool_bundles; + +struct ExtensionEchoContributor; + +impl codex_extension_api::ToolContributor for ExtensionEchoContributor { + fn tools( + &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), + )] + } +} + +struct ExtensionEchoExecutor; + +impl ToolExecutor for ExtensionEchoExecutor { + fn execute<'a>(&'a self, call: ExtensionToolCall) -> ToolFuture<'a> { + Box::pin(async move { + let arguments: serde_json::Value = + serde_json::from_str(&call.arguments).expect("test arguments should parse"); + Ok(json!({ + "arguments": arguments, + "callId": call.call_id.clone(), + "ok": true, + })) + }) + } +} + +fn extension_tool_test_registry() -> Arc> { + let mut builder = ExtensionRegistryBuilder::new(); + builder.tool_contributor(Arc::new(ExtensionEchoContributor)); + Arc::new(builder.build()) +} #[tokio::test] #[expect( @@ -37,6 +101,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, + extension_tool_bundles: Vec::new(), dynamic_tools: turn.dynamic_tools.as_slice(), }, ); @@ -110,6 +175,7 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()> unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::from(["echo".to_string()]), discoverable_tools: None, + extension_tool_bundles: Vec::new(), dynamic_tools: turn.dynamic_tools.as_slice(), }, ); @@ -177,6 +243,7 @@ async fn model_visible_specs_filter_deferred_dynamic_tools() -> anyhow::Result<( unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, + extension_tool_bundles: Vec::new(), dynamic_tools: &dynamic_tools, }, ); @@ -198,6 +265,86 @@ async fn model_visible_specs_filter_deferred_dynamic_tools() -> anyhow::Result<( Ok(()) } +#[tokio::test] +async fn extension_tool_bundles_are_model_visible_and_dispatchable() -> anyhow::Result<()> { + let (mut session, turn) = make_session_and_context().await; + session.services.extensions = extension_tool_test_registry(); + + let router = ToolRouter::from_config( + &turn.tools_config, + ToolRouterParams { + deferred_mcp_tools: None, + mcp_tools: None, + unavailable_called_tools: Vec::new(), + parallel_mcp_server_names: HashSet::new(), + discoverable_tools: None, + extension_tool_bundles: extension_tool_bundles(&session), + dynamic_tools: turn.dynamic_tools.as_slice(), + }, + ); + + assert!( + router + .find_spec(&ToolName::plain("extension_echo")) + .is_some(), + "expected extension-provided tool spec to be registered" + ); + assert!( + router + .model_visible_specs() + .iter() + .any(|spec| spec.name() == "extension_echo"), + "expected extension-provided tool to be visible to the model" + ); + + let call = ToolRouter::build_tool_call( + &session, + ResponseItem::FunctionCall { + id: None, + name: "extension_echo".to_string(), + namespace: None, + arguments: json!({ "message": "hello" }).to_string(), + call_id: "call-extension".to_string(), + }, + ) + .await? + .expect("function_call should produce a tool call"); + + let result = router + .dispatch_tool_call_with_code_mode_result( + Arc::new(session), + Arc::new(turn), + CancellationToken::new(), + Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())), + call, + ToolCallSource::Direct, + ) + .await?; + + let response = result.into_response(); + match response { + ResponseInputItem::FunctionCallOutput { call_id, output } => { + assert_eq!(call_id, "call-extension"); + let FunctionCallOutputBody::Text(text) = output.body else { + panic!("expected text function call output") + }; + let value: serde_json::Value = + serde_json::from_str(&text).expect("extension tool output should be json"); + assert_eq!( + value, + json!({ + "arguments": { "message": "hello" }, + "callId": "call-extension", + "ok": true, + }) + ); + } + other => panic!("expected function call output, got {other:?}"), + } + + Ok(()) +} + fn namespace_function_names(specs: &[ToolSpec], namespace_name: &str) -> Vec { specs .iter() diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index a2fe8da97..2faac645a 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -13,6 +13,7 @@ use crate::tools::spec_plan_types::ToolRegistryBuildMcpTool; use crate::tools::spec_plan_types::ToolRegistryBuildParams; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tools::AdditionalProperties; use codex_tools::DiscoverableTool; use codex_tools::JsonSchema; @@ -69,6 +70,7 @@ pub(crate) fn build_specs_with_discoverable_tools( deferred_mcp_tools: Option>, unavailable_called_tools: Vec, discoverable_tools: Option>, + extension_tool_bundles: &[ExtensionToolBundle], dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { use crate::tools::handlers::UnavailableToolHandler; @@ -120,6 +122,7 @@ pub(crate) fn build_specs_with_discoverable_tools( .as_ref() .map(|inputs| &inputs.tool_namespaces), discoverable_tools: discoverable_tools.as_deref(), + extension_tool_bundles, 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 4361cfe4f..13a79e594 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -436,6 +436,10 @@ pub fn build_tool_registry_builder( } } + for bundle in params.extension_tool_bundles.iter().cloned() { + builder.register_tool_bundle(bundle); + } + builder } diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 3fa8797c5..7cd33a2aa 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -43,6 +43,10 @@ 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::ConfiguredToolSpec; use codex_tools::DiscoverablePluginInfo; @@ -73,6 +77,74 @@ 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_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), + ) +} + +#[test] +fn extension_tools_do_not_replace_builtin_tools() { + let model_info = model_info(); + let available_models = Vec::new(); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + available_models: &available_models, + features: &Features::with_defaults(), + image_generation_tool_auth_allowed: true, + web_search_mode: Some(WebSearchMode::Cached), + session_source: SessionSource::Cli, + permission_profile: &PermissionProfile::Disabled, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }); + let extension_tool_bundles = vec![extension_tool_bundle( + "update_plan", + "Extension attempt to replace a built-in tool.", + )]; + let (tools, _) = build_specs_with_discoverable_tools( + &tools_config, + /*mcp_tools*/ None, + /*deferred_mcp_tools*/ None, + /*discoverable_tools*/ None, + &extension_tool_bundles, + &[], + ); + + assert_eq!( + find_tool(&tools, "update_plan").spec, + create_update_plan_tool() + ); + assert_eq!( + tools + .iter() + .filter(|tool| tool.name() == "update_plan") + .count(), + 1 + ); +} + #[test] fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { let model_info = model_info(); @@ -1807,6 +1879,7 @@ fn request_plugin_install_is_not_registered_without_feature_flag() { "Google Calendar", "Plan events and schedules.", )]), + /*extension_tool_bundles*/ &[], &[], ); @@ -1847,6 +1920,7 @@ fn request_plugin_install_can_be_registered_without_search_tool() { "Google Calendar", "Plan events and schedules.", )]), + /*extension_tool_bundles*/ &[], &[], ); @@ -1913,6 +1987,7 @@ fn request_plugin_install_description_lists_discoverable_tools() { /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, Some(discoverable_tools), + /*extension_tool_bundles*/ &[], &[], ); assert!(registry.has_handler(&ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME))); @@ -2188,6 +2263,45 @@ fn code_mode_only_exec_description_includes_full_nested_tool_details() { assert!(description.contains("### `view_image`")); } +#[test] +fn code_mode_only_exec_description_includes_extension_tool_details() { + let model_info = model_info(); + let mut features = Features::with_defaults(); + features.enable(Feature::CodeMode); + features.enable(Feature::CodeModeOnly); + let available_models = Vec::new(); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + available_models: &available_models, + features: &features, + image_generation_tool_auth_allowed: true, + web_search_mode: Some(WebSearchMode::Cached), + session_source: SessionSource::Cli, + permission_profile: &PermissionProfile::Disabled, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }); + + let extension_tool_bundles = vec![extension_tool_bundle( + "extension_echo", + "Echoes arguments through an extension tool.", + )]; + let (tools, _) = build_specs_with_discoverable_tools( + &tools_config, + /*mcp_tools*/ None, + /*deferred_mcp_tools*/ None, + /*discoverable_tools*/ None, + &extension_tool_bundles, + &[], + ); + let ToolSpec::Freeform(FreeformTool { description, .. }) = &find_tool(&tools, "exec").spec + else { + panic!("expected freeform tool"); + }; + + assert!(description.contains("### `extension_echo`")); + assert!(description.contains("Echoes arguments through an extension tool.")); +} + #[test] fn code_mode_exec_description_omits_nested_tool_details_when_not_code_mode_only() { let model_info = model_info(); @@ -2276,6 +2390,7 @@ fn build_specs<'a>( mcp_tools, deferred_mcp_tools, /*discoverable_tools*/ None, + /*extension_tool_bundles*/ &[], dynamic_tools, ) } @@ -2285,6 +2400,7 @@ fn build_specs_with_discoverable_tools<'a>( mcp_tools: Option>, deferred_mcp_tools: Option>>, discoverable_tools: Option>, + extension_tool_bundles: &[codex_tool_api::ToolBundle], dynamic_tools: &[DynamicToolSpec], ) -> (Vec, ToolRegistry) { build_specs_with_optional_tool_namespaces( @@ -2293,6 +2409,7 @@ fn build_specs_with_discoverable_tools<'a>( deferred_mcp_tools, /*tool_namespaces*/ None, discoverable_tools, + extension_tool_bundles, dynamic_tools, ) } @@ -2303,6 +2420,7 @@ fn build_specs_with_optional_tool_namespaces<'a>( deferred_mcp_tools: Option>>, tool_namespaces: Option>, discoverable_tools: Option>, + extension_tool_bundles: &[codex_tool_api::ToolBundle], dynamic_tools: &[DynamicToolSpec], ) -> (Vec, ToolRegistry) { let mcp_tool_inputs = mcp_tools.as_ref().map(|mcp_tools| { @@ -2321,6 +2439,7 @@ fn build_specs_with_optional_tool_namespaces<'a>( deferred_mcp_tools: deferred_mcp_tools.as_deref(), tool_namespaces: tool_namespaces.as_ref(), discoverable_tools: discoverable_tools.as_deref(), + extension_tool_bundles, 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 a1cb654dd..7ac5c0f52 100644 --- a/codex-rs/core/src/tools/spec_plan_types.rs +++ b/codex-rs/core/src/tools/spec_plan_types.rs @@ -1,5 +1,6 @@ use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tools::DiscoverableTool; use codex_tools::ToolName; use codex_tools::ToolsConfig; @@ -11,6 +12,7 @@ pub struct ToolRegistryBuildParams<'a> { pub deferred_mcp_tools: Option<&'a [ToolRegistryBuildDeferredTool<'a>]>, pub tool_namespaces: Option<&'a HashMap>, pub discoverable_tools: Option<&'a [DiscoverableTool]>, + pub extension_tool_bundles: &'a [ExtensionToolBundle], 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 24e03fbf8..06343fce0 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -292,6 +292,7 @@ fn build_specs_with_unavailable_tools( deferred_mcp_tools, unavailable_called_tools, /*discoverable_tools*/ None, + /*extension_tool_bundles*/ &[], dynamic_tools, ) } @@ -353,6 +354,7 @@ async fn assert_model_tools( unavailable_called_tools: Vec::new(), parallel_mcp_server_names: std::collections::HashSet::new(), discoverable_tools: None, + extension_tool_bundles: Vec::new(), dynamic_tools: &[], }, ); @@ -822,6 +824,7 @@ async fn request_plugin_install_requires_apps_and_plugins_features() { /*deferred_mcp_tools*/ None, Vec::new(), discoverable_tools.clone(), + /*extension_tool_bundles*/ &[], &[], ) .build(); diff --git a/codex-rs/ext/extension-api/Cargo.toml b/codex-rs/ext/extension-api/Cargo.toml index 9ff3595f0..b281d438e 100644 --- a/codex-rs/ext/extension-api/Cargo.toml +++ b/codex-rs/ext/extension-api/Cargo.toml @@ -14,7 +14,5 @@ doctest = false workspace = true [dependencies] +codex-tool-api = { workspace = true } codex-protocol = { workspace = true } -codex-tools = { workspace = true } -serde_json = { workspace = true } -thiserror = { workspace = true } diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index 34096c9ba..4475e9271 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -1,17 +1,14 @@ use std::future::Future; use codex_protocol::items::TurnItem; +use codex_tool_api::ToolBundle; use crate::ExtensionData; mod prompt; -mod tool; pub use prompt::PromptFragment; pub use prompt::PromptSlot; -pub use tool::ToolCallError; -pub use tool::ToolContribution; -pub use tool::ToolHandler; /// Contributor that receives host-owned thread-start input before later /// contributors read from extension stores. @@ -30,8 +27,9 @@ 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 runtime context. - fn tools(&self, thread_store: &ExtensionData) -> Vec; + /// Returns the native tools visible for the supplied extension stores. + fn tools(&self, session_store: &ExtensionData, thread_store: &ExtensionData) + -> Vec; } /// Future returned by one ordered turn-item contribution. diff --git a/codex-rs/ext/extension-api/src/contributors/tool.rs b/codex-rs/ext/extension-api/src/contributors/tool.rs deleted file mode 100644 index b40e29a8e..000000000 --- a/codex-rs/ext/extension-api/src/contributors/tool.rs +++ /dev/null @@ -1,68 +0,0 @@ -use std::future::Future; -use std::pin::Pin; -use std::sync::Arc; - -use codex_tools::ResponsesApiTool; -use serde_json::Value; -use thiserror::Error; - -// TMP while we don't have the fully extracted tools -#[derive(Clone)] -pub struct ToolContribution { - spec: ResponsesApiTool, - handler: Arc, - supports_parallel_tool_calls: bool, -} - -impl ToolContribution { - pub fn new(spec: ResponsesApiTool, handler: Arc) -> Self { - Self { - spec, - handler, - supports_parallel_tool_calls: false, - } - } - - #[must_use] - pub fn allow_parallel_calls(mut self) -> Self { - self.supports_parallel_tool_calls = true; - self - } - - pub fn spec(&self) -> &ResponsesApiTool { - &self.spec - } - - pub fn supports_parallel_tool_calls(&self) -> bool { - self.supports_parallel_tool_calls - } - - pub fn handler(&self) -> Arc { - Arc::clone(&self.handler) - } -} - -//////// Just to make it compile //////////////////////////////// -pub trait ToolHandler: Send + Sync { - /// Handles one JSON-encoded invocation for this tool. - fn handle<'a>( - &'a self, - arguments: Value, - ) -> Pin> + Send + 'a>>; -} - -/// Error returned by a contributed native tool handler. -#[derive(Clone, Debug, Error, PartialEq, Eq)] -#[error("{message}")] -pub struct ToolCallError { - message: String, -} - -impl ToolCallError { - /// Creates a contributed-tool error with the supplied model-visible text. - pub fn new(message: impl Into) -> Self { - Self { - message: message.into(), - } - } -} diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index 951ffab22..7c9e412fd 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -2,15 +2,18 @@ mod contributors; mod registry; mod state; +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 contributors::ApprovalInterceptorContributor; pub use contributors::ContextContributor; pub use contributors::PromptFragment; pub use contributors::PromptSlot; pub use contributors::ThreadStartContributor; -pub use contributors::ToolCallError; -pub use contributors::ToolContribution; pub use contributors::ToolContributor; -pub use contributors::ToolHandler; pub use contributors::TurnItemContributionFuture; pub use contributors::TurnItemContributor; pub use registry::ExtensionRegistry; diff --git a/codex-rs/tool-api/Cargo.toml b/codex-rs/tool-api/Cargo.toml index 49c095dbf..13d1a2207 100644 --- a/codex-rs/tool-api/Cargo.toml +++ b/codex-rs/tool-api/Cargo.toml @@ -13,9 +13,6 @@ doctest = false workspace = true [dependencies] -codex-protocol = { workspace = true } -codex-tools = { workspace = true } -serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } thiserror = { workspace = true } diff --git a/codex-rs/tool-api/README.md b/codex-rs/tool-api/README.md new file mode 100644 index 000000000..418c1d177 --- /dev/null +++ b/codex-rs/tool-api/README.md @@ -0,0 +1,29 @@ +# 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 index 9c03184c6..18c84c4c0 100644 --- a/codex-rs/tool-api/src/bundle.rs +++ b/codex-rs/tool-api/src/bundle.rs @@ -2,126 +2,86 @@ use std::future::Future; use std::pin::Pin; use std::sync::Arc; -use codex_tools::ToolName; -use codex_tools::ToolSpec; +use serde_json::Value; +use crate::FunctionToolSpec; use crate::ToolCall; use crate::ToolError; -use crate::ToolOutput; -/// Future returned by one executable-tool invocation. -pub type ToolFuture<'a> = - Pin, ToolError>> + Send + 'a>>; +/// Future returned by one contributed function-tool invocation. +pub type ToolFuture<'a> = Pin> + Send + 'a>>; -/// Future returned by one mutability probe. -pub type BoolFuture<'a> = Pin + Send + 'a>>; - -/// Model-visible definition plus executable implementation for one tool. +/// Model-visible definition plus executable implementation for one contributed +/// function tool. #[derive(Clone)] -pub struct ToolBundle { - definition: ToolDefinition, - executor: Arc>, +pub struct ToolBundle { + spec: FunctionToolSpec, + executor: Arc, } -impl ToolBundle { - /// Creates one executable tool bundle. - pub fn new(name: ToolName, spec: ToolSpec, executor: Arc>) -> Self { - Self { - definition: ToolDefinition { - name, - spec, - supports_parallel_tool_calls: false, - }, - executor, - } +impl ToolBundle { + /// Creates one contributed function-tool bundle. + pub fn new(spec: FunctionToolSpec, executor: Arc) -> Self { + Self { spec, executor } } - /// Marks this tool as safe for the host to run in parallel with peers. - #[must_use] - pub fn allow_parallel_calls(mut self) -> Self { - self.definition.supports_parallel_tool_calls = true; - self + /// Returns the contributed function-tool spec. + pub fn spec(&self) -> &FunctionToolSpec { + &self.spec } - /// Returns the model-visible tool definition. - pub fn definition(&self) -> &ToolDefinition { - &self.definition + /// 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> { + pub fn executor(&self) -> Arc { Arc::clone(&self.executor) } } -/// Model-visible metadata owned by an executable tool bundle. -#[derive(Clone)] -pub struct ToolDefinition { - pub name: ToolName, - pub spec: ToolSpec, - pub supports_parallel_tool_calls: bool, -} - -/// Executable behavior for one contributed tool. +/// Executable behavior for one contributed function tool. /// -/// Implementations should keep host-specific needs inside `C`; tool owners that -/// do not require host state can implement the trait for any `C`. -pub trait ToolExecutor: Send + Sync { - fn execute<'a>(&'a self, call: ToolCall) -> ToolFuture<'a>; - - /// Returns whether the call may mutate user state. - /// - /// Hosts can use this conservative signal for serialization or approval - /// policy. Read-only tools should override this default. - fn is_mutating<'a>(&'a self, _call: &'a ToolCall) -> BoolFuture<'a> { - Box::pin(async { true }) - } +/// 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 std::task::Context; - use std::task::Poll; - use std::task::Wake; - use std::task::Waker; - use super::*; - use crate::JsonToolOutput; - use crate::ToolInput; + use pretty_assertions::assert_eq; + use serde_json::json; - struct DefaultMutatingExecutor; + use super::ToolBundle; + use super::ToolExecutor; + use super::ToolFuture; + use crate::FunctionToolSpec; + use crate::ToolCall; - impl ToolExecutor<()> for DefaultMutatingExecutor { - fn execute<'a>(&'a self, _call: ToolCall<()>) -> ToolFuture<'a> { - Box::pin(async { - Ok(Box::new(JsonToolOutput::new(serde_json::json!(null))) as Box) - }) + struct StubExecutor; + + impl ToolExecutor for StubExecutor { + fn execute<'a>(&'a self, _call: ToolCall) -> ToolFuture<'a> { + Box::pin(async { Ok(json!({ "ok": true })) }) } } - struct NoopWaker; - - impl Wake for NoopWaker { - fn wake(self: Arc) {} - } - #[test] - fn contributed_tools_default_to_mutating() { - let call = ToolCall { - context: (), - call_id: "call-default-mutating".to_string(), - input: ToolInput::Function { - arguments: "{}".to_string(), + 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" }), }, - }; - let mut future = DefaultMutatingExecutor.is_mutating(&call); - let waker = Waker::from(Arc::new(NoopWaker)); - let mut context = Context::from_waker(&waker); + Arc::new(StubExecutor), + ); - assert!(matches!( - future.as_mut().poll(&mut context), - Poll::Ready(true) - )); + assert_eq!(bundle.tool_name(), "echo"); } } diff --git a/codex-rs/tool-api/src/call.rs b/codex-rs/tool-api/src/call.rs index f6b813501..bcfab2092 100644 --- a/codex-rs/tool-api/src/call.rs +++ b/codex-rs/tool-api/src/call.rs @@ -1,14 +1,5 @@ -/// One executable tool call delivered to a contributed tool. -pub struct ToolCall { - pub context: C, +/// One contributed function-tool call. +pub struct ToolCall { pub call_id: String, - pub input: ToolInput, -} - -/// Model-supplied input for the executable tool families currently exposed by -/// the shared tool seam. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum ToolInput { - Function { arguments: String }, - Freeform { input: String }, + pub arguments: String, } diff --git a/codex-rs/tool-api/src/lib.rs b/codex-rs/tool-api/src/lib.rs index f65c9a6f9..11788f564 100644 --- a/codex-rs/tool-api/src/lib.rs +++ b/codex-rs/tool-api/src/lib.rs @@ -1,17 +1,14 @@ -//! Reusable executable-tool contracts shared between hosts and tool owners. +//! Minimal function-tool contracts shared between hosts and extension-owned +//! tool crates. mod bundle; mod call; mod error; -mod output; +mod spec; -pub use bundle::BoolFuture; pub use bundle::ToolBundle; -pub use bundle::ToolDefinition; pub use bundle::ToolExecutor; pub use bundle::ToolFuture; pub use call::ToolCall; -pub use call::ToolInput; pub use error::ToolError; -pub use output::JsonToolOutput; -pub use output::ToolOutput; +pub use spec::FunctionToolSpec; diff --git a/codex-rs/tool-api/src/output.rs b/codex-rs/tool-api/src/output.rs deleted file mode 100644 index 23da24d00..000000000 --- a/codex-rs/tool-api/src/output.rs +++ /dev/null @@ -1,113 +0,0 @@ -use codex_protocol::models::FunctionCallOutputBody; -use codex_protocol::models::FunctionCallOutputPayload; -use codex_protocol::models::ResponseInputItem; -use serde::Serialize; -use serde_json::Value; - -use crate::ToolError; -use crate::ToolInput; - -/// Tool-owned output rendering for each host-facing boundary. -pub trait ToolOutput: Send { - fn log_preview(&self) -> String; - - fn success_for_logging(&self) -> bool; - - fn to_response_item(&self, call_id: &str, input: &ToolInput) -> ResponseInputItem; - - /// Returns the stable value exposed to post-tool-use hook integration when a - /// host chooses to wire that surface for this tool. - fn post_tool_use_response(&self, _call_id: &str, _input: &ToolInput) -> Option { - None - } - - fn code_mode_result(&self, input: &ToolInput) -> Value; -} - -/// Convenience output for ordinary JSON-returning function tools. -#[derive(Clone, Debug)] -pub struct JsonToolOutput { - value: Value, -} - -impl JsonToolOutput { - /// Creates a JSON output from a serializable value. - pub fn from_serializable(value: impl Serialize) -> Result { - serde_json::to_value(value).map(Self::new).map_err(|err| { - ToolError::respond_to_model(format!("failed to serialize output: {err}")) - }) - } - - /// Creates a JSON output from an already materialized value. - pub fn new(value: Value) -> Self { - Self { value } - } -} - -impl ToolOutput for JsonToolOutput { - fn log_preview(&self) -> String { - self.value.to_string() - } - - fn success_for_logging(&self) -> bool { - true - } - - fn to_response_item(&self, call_id: &str, _input: &ToolInput) -> 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, _input: &ToolInput) -> Option { - Some(self.value.clone()) - } - - fn code_mode_result(&self, _input: &ToolInput) -> Value { - self.value.clone() - } -} - -#[cfg(test)] -mod tests { - use codex_protocol::models::FunctionCallOutputBody; - use codex_protocol::models::FunctionCallOutputPayload; - use codex_protocol::models::ResponseInputItem; - use pretty_assertions::assert_eq; - use serde_json::json; - - use super::JsonToolOutput; - use super::ToolOutput; - use crate::ToolInput; - - #[test] - fn json_tool_output_renders_function_output() { - let input = ToolInput::Function { - arguments: "{}".to_string(), - }; - let output = JsonToolOutput::from_serializable(json!({ "ok": true })) - .expect("serializable value should produce json output"); - - assert_eq!(output.log_preview(), "{\"ok\":true}"); - assert!(output.success_for_logging()); - assert_eq!( - output.to_response_item("call-1", &input), - ResponseInputItem::FunctionCallOutput { - call_id: "call-1".to_string(), - output: FunctionCallOutputPayload { - body: FunctionCallOutputBody::Text("{\"ok\":true}".to_string()), - success: Some(true), - }, - } - ); - assert_eq!( - output.post_tool_use_response("call-1", &input), - Some(json!({ "ok": true })) - ); - assert_eq!(output.code_mode_result(&input), json!({ "ok": true })); - } -} diff --git a/codex-rs/tool-api/src/spec.rs b/codex-rs/tool-api/src/spec.rs new file mode 100644 index 000000000..1b25837bb --- /dev/null +++ b/codex-rs/tool-api/src/spec.rs @@ -0,0 +1,10 @@ +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 9fa9b4543..79035dde3 100644 --- a/codex-rs/tools/README.md +++ b/codex-rs/tools/README.md @@ -1,45 +1,22 @@ # codex-tools -`codex-tools` is intended to become the home for tool-related code that is -shared across multiple crates and does not need to stay coupled to -`codex-core`. +`codex-tools` is the host-side support crate for building, adapting, and +planning tool sets outside `codex-core`. -Today this crate is intentionally small. It currently owns the shared tool -schema and Responses API tool primitives that no longer need to live in -`core/src/tools/spec.rs` or `core/src/client_common.rs`: +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. -- `JsonSchema` -- `AdditionalProperties` -- `ToolDefinition` -- `ToolSpec` -- `ConfiguredToolSpec` -- `ResponsesApiTool` -- `FreeformTool` -- `FreeformToolFormat` -- `LoadableToolSpec` -- `ResponsesApiWebSearchFilters` -- `ResponsesApiWebSearchUserLocation` -- `ResponsesApiNamespace` -- `ResponsesApiNamespaceTool` -- code-mode `ToolSpec` adapters and `exec` / `wait` spec builders -- MCP resource and `test_sync_tool` spec builders -- local host tool spec builders for shell/exec/request-permissions/view-image -- collaboration and agent-job `ToolSpec` builders for spawn/send/wait/close, - `request_user_input`, and CSV fanout/reporting -- discoverable-tool models, client filtering, and `ToolSpec` builders for - `tool_search` and `request_plugin_install` -- `parse_tool_input_schema()` -- `parse_dynamic_tool()` -- `parse_mcp_tool()` -- `create_tools_json_for_responses_api()` -- `mcp_call_tool_result_output_schema()` -- `tool_definition_to_responses_api_tool()` -- `dynamic_tool_to_loadable_tool_spec()` -- `dynamic_tool_to_responses_api_tool()` -- `mcp_tool_to_responses_api_tool()` -- `mcp_tool_to_deferred_responses_api_tool()` -- `augment_tool_spec_for_code_mode()` -- `tool_spec_to_code_mode_tool_definition()` +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`: + +- aggregate host models such as `ToolSpec`, `ConfiguredToolSpec`, + `LoadableToolSpec`, `ResponsesApiNamespace`, and + `ResponsesApiNamespaceTool` +- host config and discovery models used while assembling tool sets, including + `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 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 @@ -49,16 +26,18 @@ boundaries are ready. ## Vision -Over time, this crate should hold tool-facing primitives that are shared by +Over time, this crate should hold host-side tool machinery that is shared by multiple consumers, for example: -- schema and spec data models -- tool input/output parsing helpers -- tool metadata and compatibility shims that do not depend on `codex-core` -- other narrowly scoped utility code that multiple crates need +- host-visible aggregate tool models +- tool-set planning and discovery helpers +- MCP and dynamic-tool adaptation into Responses API shapes +- code-mode compatibility shims that do not depend on `codex-core` +- other narrowly scoped host utilities that multiple crates need 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 @@ -69,16 +48,14 @@ The corresponding non-goals are just as important: The expected migration shape is: -1. Move low-coupling tool primitives here. -2. Switch non-core consumers to depend on `codex-tools` directly. +1. Keep ordinary contributed function-tool authoring in `codex-tool-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 call sites are updated. -4. Only extract higher-level tool infrastructure after the crate boundaries are +4. Only extract higher-level host infrastructure after the crate boundaries are clear and independently testable. -That means it is normal for `codex-core` to temporarily re-export types or -helpers from `codex-tools` during the transition. - ## Crate conventions This crate should start with stricter structure than `core/src/tools` so it