From 57400770bdcfaee616f06bc797efbdd350ad21ba Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 14 May 2026 12:09:28 +0100 Subject: [PATCH 1/3] chore: clean up things --- .../core/src/session/tests/guardian_tests.rs | 24 +- .../src/tools/code_mode/execute_handler.rs | 19 +- .../core/src/tools/code_mode/wait_handler.rs | 14 +- codex-rs/core/src/tools/context.rs | 25 ++ .../agent_jobs/report_agent_job_result.rs | 14 +- .../agent_jobs/spawn_agents_on_csv.rs | 16 +- .../core/src/tools/handlers/apply_patch.rs | 19 +- codex-rs/core/src/tools/handlers/dynamic.rs | 17 +- .../src/tools/handlers/extension_tools.rs | 59 ++-- .../src/tools/handlers/goal/create_goal.rs | 15 +- .../core/src/tools/handlers/goal/get_goal.rs | 15 +- .../src/tools/handlers/goal/update_goal.rs | 15 +- codex-rs/core/src/tools/handlers/mcp.rs | 38 ++- .../list_mcp_resource_templates.rs | 15 +- .../mcp_resource/list_mcp_resources.rs | 15 +- .../mcp_resource/read_mcp_resource.rs | 15 +- .../core/src/tools/handlers/multi_agents.rs | 3 +- .../handlers/multi_agents/close_agent.rs | 11 +- .../handlers/multi_agents/resume_agent.rs | 11 +- .../tools/handlers/multi_agents/send_input.rs | 11 +- .../src/tools/handlers/multi_agents/spawn.rs | 11 +- .../src/tools/handlers/multi_agents/wait.rs | 11 +- .../src/tools/handlers/multi_agents_tests.rs | 33 +- .../src/tools/handlers/multi_agents_v2.rs | 3 +- .../handlers/multi_agents_v2/close_agent.rs | 11 +- .../handlers/multi_agents_v2/followup_task.rs | 11 +- .../handlers/multi_agents_v2/list_agents.rs | 11 +- .../handlers/multi_agents_v2/send_message.rs | 11 +- .../tools/handlers/multi_agents_v2/spawn.rs | 11 +- .../tools/handlers/multi_agents_v2/wait.rs | 11 +- codex-rs/core/src/tools/handlers/plan.rs | 14 +- .../src/tools/handlers/request_permissions.rs | 17 +- .../tools/handlers/request_plugin_install.rs | 17 +- .../src/tools/handlers/request_user_input.rs | 17 +- .../src/tools/handlers/shell/shell_command.rs | 17 +- .../core/src/tools/handlers/shell_tests.rs | 2 +- codex-rs/core/src/tools/handlers/test_sync.rs | 17 +- .../core/src/tools/handlers/tool_search.rs | 13 +- .../core/src/tools/handlers/unified_exec.rs | 13 +- .../handlers/unified_exec/exec_command.rs | 24 +- .../handlers/unified_exec/write_stdin.rs | 17 +- .../src/tools/handlers/unified_exec_tests.rs | 2 +- .../core/src/tools/handlers/view_image.rs | 16 +- codex-rs/core/src/tools/mod.rs | 1 - codex-rs/core/src/tools/registry.rs | 283 ++++++------------ codex-rs/core/src/tools/registry_tests.rs | 39 +-- codex-rs/core/src/tools/router.rs | 59 +--- codex-rs/core/src/tools/router_tests.rs | 11 +- codex-rs/core/src/tools/spec.rs | 77 ----- codex-rs/core/src/tools/spec_plan.rs | 255 +++++++++++----- codex-rs/core/src/tools/spec_plan_tests.rs | 45 ++- codex-rs/core/src/tools/spec_plan_types.rs | 29 -- codex-rs/core/src/tools/spec_tests.rs | 220 +++----------- .../src/tools/tool_dispatch_trace_tests.rs | 16 +- .../ext/extension-api/src/contributors.rs | 7 +- .../extension-api/src/contributors/tools.rs | 15 - codex-rs/ext/extension-api/src/lib.rs | 3 +- codex-rs/ext/memories/src/extension.rs | 2 +- codex-rs/ext/memories/src/tests.rs | 4 +- codex-rs/ext/memories/src/tools/list.rs | 7 +- codex-rs/ext/memories/src/tools/mod.rs | 4 +- codex-rs/ext/memories/src/tools/read.rs | 7 +- codex-rs/ext/memories/src/tools/search.rs | 7 +- codex-rs/tools/src/tool_executor.rs | 7 +- codex-rs/tools/src/tool_output.rs | 18 ++ 65 files changed, 807 insertions(+), 990 deletions(-) delete mode 100644 codex-rs/core/src/tools/spec_plan_types.rs delete mode 100644 codex-rs/ext/extension-api/src/contributors/tools.rs diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 718b9bf05fa3..5db4aab9854e 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -5,8 +5,9 @@ use crate::exec_policy::ExecPolicyManager; use crate::guardian::GUARDIAN_REVIEWER_NAME; use crate::sandboxing::SandboxPermissions; use crate::test_support::models_manager_with_provider; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolCallSource; +use crate::tools::context::ToolOutput; +use crate::tools::context::ToolPayload; use crate::turn_diff_tracker::TurnDiffTracker; use codex_app_server_protocol::ConfigLayerSource; use codex_config::ConfigLayerEntry; @@ -22,8 +23,8 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::AdditionalPermissionProfile as PermissionProfile; use codex_protocol::models::ContentItem; use codex_protocol::models::NetworkPermissions; +use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; -use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::AskForApproval; use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionProfile; @@ -48,8 +49,23 @@ use tempfile::tempdir; use tokio::time::timeout; use tokio_util::sync::CancellationToken; -fn expect_text_output(output: &FunctionToolOutput) -> String { - function_call_output_content_items_to_text(&output.body).unwrap_or_default() +fn expect_text_output(output: &T) -> String +where + T: ToolOutput + ?Sized, +{ + let response = output.to_response_item( + "call-guardian", + &ToolPayload::Function { + arguments: "{}".to_string(), + }, + ); + match response { + ResponseInputItem::FunctionCallOutput { output, .. } + | ResponseInputItem::CustomToolCallOutput { output, .. } => { + output.body.to_text().unwrap_or_default() + } + other => panic!("expected function output, got {other:?}"), + } } #[tokio::test] diff --git a/codex-rs/core/src/tools/code_mode/execute_handler.rs b/codex-rs/core/src/tools/code_mode/execute_handler.rs index 26ef021ad2c4..e101e1a8006b 100644 --- a/codex-rs/core/src/tools/code_mode/execute_handler.rs +++ b/codex-rs/core/src/tools/code_mode/execute_handler.rs @@ -2,8 +2,9 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -89,8 +90,6 @@ impl CodeModeExecuteHandler { #[async_trait::async_trait] impl ToolExecutor for CodeModeExecuteHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(PUBLIC_TOOL_NAME) } @@ -99,7 +98,10 @@ impl ToolExecutor for CodeModeExecuteHandler { Some(self.spec.clone()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -110,9 +112,10 @@ impl ToolExecutor for CodeModeExecuteHandler { } = invocation; match payload { - ToolPayload::Custom { input } if is_exec_tool_name(&tool_name) => { - self.execute(session, turn, call_id, input).await - } + ToolPayload::Custom { input } if is_exec_tool_name(&tool_name) => self + .execute(session, turn, call_id, input) + .await + .map(boxed_tool_output), _ => Err(FunctionCallError::RespondToModel(format!( "{PUBLIC_TOOL_NAME} expects raw JavaScript source text" ))), @@ -120,7 +123,7 @@ impl ToolExecutor for CodeModeExecuteHandler { } } -impl ToolHandler for CodeModeExecuteHandler { +impl CoreToolRuntime for CodeModeExecuteHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Custom { .. }) } diff --git a/codex-rs/core/src/tools/code_mode/wait_handler.rs b/codex-rs/core/src/tools/code_mode/wait_handler.rs index 4f828c89108a..725535339fbf 100644 --- a/codex-rs/core/src/tools/code_mode/wait_handler.rs +++ b/codex-rs/core/src/tools/code_mode/wait_handler.rs @@ -1,11 +1,11 @@ use serde::Deserialize; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -43,8 +43,6 @@ where #[async_trait::async_trait] impl ToolExecutor for CodeModeWaitHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(WAIT_TOOL_NAME) } @@ -53,7 +51,10 @@ impl ToolExecutor for CodeModeWaitHandler { Some(create_wait_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -99,6 +100,7 @@ impl ToolExecutor for CodeModeWaitHandler { } handle_runtime_response(&exec, wait_response.into(), args.max_tokens, started_at) .await + .map(boxed_tool_output) .map_err(FunctionCallError::RespondToModel) } _ => Err(FunctionCallError::RespondToModel(format!( @@ -108,4 +110,4 @@ impl ToolExecutor for CodeModeWaitHandler { } } -impl ToolHandler for CodeModeWaitHandler {} +impl CoreToolRuntime for CodeModeWaitHandler {} diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 0f3ae631a042..5fc19a2a2e20 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -28,6 +28,13 @@ use tokio_util::sync::CancellationToken; pub use codex_tools::ToolOutput; pub use codex_tools::ToolPayload; +pub(crate) fn boxed_tool_output(output: T) -> Box +where + T: ToolOutput + 'static, +{ + Box::new(output) +} + pub type SharedTurnDiffTracker = Arc>; #[derive(Clone, Debug, Eq, PartialEq)] @@ -91,6 +98,10 @@ impl ToolOutput for McpToolOutput { }) } + fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { + Some(self.tool_input.clone()) + } + fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { serde_json::to_value(&self.result).ok() } @@ -327,6 +338,20 @@ impl ToolOutput for ExecCommandToolOutput { ) } + fn post_tool_use_id(&self, call_id: &str) -> String { + if self.event_call_id.is_empty() { + call_id.to_string() + } else { + self.event_call_id.clone() + } + } + + fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { + self.hook_command + .as_ref() + .map(|command| serde_json::json!({ "command": command })) + } + fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { if self.process_id.is_some() || self.hook_command.is_none() { return None; diff --git a/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs b/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs index e08eb4b5fbb4..4641e793c83a 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::agent_jobs_spec::create_report_agent_job_result_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -14,8 +15,6 @@ pub struct ReportAgentJobResultHandler; #[async_trait::async_trait] impl ToolExecutor for ReportAgentJobResultHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("report_agent_job_result") } @@ -24,7 +23,10 @@ impl ToolExecutor for ReportAgentJobResultHandler { Some(create_report_agent_job_result_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, payload, .. } = invocation; @@ -38,11 +40,11 @@ impl ToolExecutor for ReportAgentJobResultHandler { } }; - handle(session, arguments).await + handle(session, arguments).await.map(boxed_tool_output) } } -impl ToolHandler for ReportAgentJobResultHandler { +impl CoreToolRuntime for ReportAgentJobResultHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs index a384f2d8cc00..0e51390e7961 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::agent_jobs_spec::create_spawn_agents_on_csv_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_utils_absolute_path::AbsolutePathBuf; @@ -15,8 +16,6 @@ pub struct SpawnAgentsOnCsvHandler; #[async_trait::async_trait] impl ToolExecutor for SpawnAgentsOnCsvHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("spawn_agents_on_csv") } @@ -25,7 +24,10 @@ impl ToolExecutor for SpawnAgentsOnCsvHandler { Some(create_spawn_agents_on_csv_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -42,11 +44,13 @@ impl ToolExecutor for SpawnAgentsOnCsvHandler { } }; - handle(session, turn, arguments).await + handle(session, turn, arguments) + .await + .map(boxed_tool_output) } } -impl ToolHandler for SpawnAgentsOnCsvHandler { +impl CoreToolRuntime for SpawnAgentsOnCsvHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index c8e2461e03e0..500751137b2c 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -17,8 +17,8 @@ use crate::tools::context::ApplyPatchToolOutput; use crate::tools::context::FunctionToolOutput; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::handlers::apply_granted_turn_permissions; @@ -27,11 +27,11 @@ use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::updated_hook_command; use crate::tools::hook_names::HookToolName; use crate::tools::orchestrator::ToolOrchestrator; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolArgumentDiffConsumer; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::runtimes::apply_patch::ApplyPatchRequest; use crate::tools::runtimes::apply_patch::ApplyPatchRuntime; use crate::tools::sandboxing::ToolCtx; @@ -299,8 +299,6 @@ async fn effective_patch_permissions( #[async_trait::async_trait] impl ToolExecutor for ApplyPatchHandler { - type Output = ApplyPatchToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("apply_patch") } @@ -309,7 +307,10 @@ impl ToolExecutor for ApplyPatchHandler { Some(create_apply_patch_freeform_tool(self.multi_environment)) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -359,7 +360,7 @@ impl ToolExecutor for ApplyPatchHandler { { InternalApplyPatchInvocation::Output(item) => { let content = item?; - Ok(ApplyPatchToolOutput::from_text(content)) + Ok(boxed_tool_output(ApplyPatchToolOutput::from_text(content))) } InternalApplyPatchInvocation::DelegateToRuntime(apply) => { let changes = convert_apply_patch_to_protocol(&apply.action); @@ -414,7 +415,7 @@ impl ToolExecutor for ApplyPatchHandler { Some(&tracker), ); let content = emitter.finish(event_ctx, out, delta.as_ref()).await?; - Ok(ApplyPatchToolOutput::from_text(content)) + Ok(boxed_tool_output(ApplyPatchToolOutput::from_text(content))) } } } @@ -438,7 +439,7 @@ impl ToolExecutor for ApplyPatchHandler { } } -impl ToolHandler for ApplyPatchHandler { +impl CoreToolRuntime for ApplyPatchHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Custom { .. }) } @@ -472,7 +473,7 @@ impl ToolHandler for ApplyPatchHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { let tool_response = result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; diff --git a/codex-rs/core/src/tools/handlers/dynamic.rs b/codex-rs/core/src/tools/handlers/dynamic.rs index 29dc5199e39d..5d20128ff3e5 100644 --- a/codex-rs/core/src/tools/handlers/dynamic.rs +++ b/codex-rs/core/src/tools/handlers/dynamic.rs @@ -4,10 +4,11 @@ use crate::session::turn_context::TurnContext; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; use crate::tools::registry::ToolExposure; -use crate::tools::registry::ToolHandler; use crate::tools::tool_search_entry::ToolSearchInfo; use crate::turn_timing::now_unix_timestamp_ms; use codex_protocol::dynamic_tools::DynamicToolCallRequest; @@ -62,8 +63,6 @@ impl DynamicToolHandler { #[async_trait::async_trait] impl ToolExecutor for DynamicToolHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { self.tool_name.clone() } @@ -76,7 +75,10 @@ impl ToolExecutor for DynamicToolHandler { self.exposure } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -117,11 +119,14 @@ impl ToolExecutor for DynamicToolHandler { .into_iter() .map(FunctionCallOutputContentItem::from) .collect::>(); - Ok(FunctionToolOutput::from_content(body, Some(success))) + Ok(boxed_tool_output(FunctionToolOutput::from_content( + body, + Some(success), + ))) } } -impl ToolHandler for DynamicToolHandler { +impl CoreToolRuntime for DynamicToolHandler { fn search_info(&self) -> Option { ToolSearchInfo::from_spec( self.search_text.clone(), diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index c4018fe9367b..37ad2d65fdc9 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -1,7 +1,5 @@ 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; @@ -13,18 +11,16 @@ use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; -pub(crate) struct ExtensionToolHandler { - executor: Arc, -} +pub(crate) struct ExtensionToolAdapter(Arc>); -impl ExtensionToolHandler { - pub(crate) fn new(executor: Arc) -> Self { - Self { executor } +impl ExtensionToolAdapter { + pub(crate) fn new(executor: Arc>) -> Self { + Self(executor) } fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> { @@ -36,23 +32,32 @@ impl ExtensionToolHandler { } #[async_trait::async_trait] -impl ToolExecutor for ExtensionToolHandler { - type Output = ExtensionToolOutput; - +impl ToolExecutor for ExtensionToolAdapter { fn tool_name(&self) -> ToolName { - self.executor.tool_name() + self.0.tool_name() } fn spec(&self) -> Option { - self.executor.spec() + self.0.spec() + } + + fn exposure(&self) -> crate::tools::registry::ToolExposure { + self.0.exposure() } - async fn handle(&self, invocation: ToolInvocation) -> Result { - self.executor.handle(to_extension_call(&invocation)).await + fn supports_parallel_tool_calls(&self) -> bool { + self.0.supports_parallel_tool_calls() + } + + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + self.0.handle(to_extension_call(&invocation)).await } } -impl ToolHandler for ExtensionToolHandler { +impl CoreToolRuntime for ExtensionToolAdapter { fn matches_kind(&self, payload: &ToolPayload) -> bool { self.arguments_from_payload(payload).is_some() } @@ -68,7 +73,7 @@ impl ToolHandler for ExtensionToolHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn ToolOutput, ) -> Option { let arguments = self.arguments_from_payload(&invocation.payload)?; Some(PostToolUsePayload { @@ -104,22 +109,20 @@ mod tests { use pretty_assertions::assert_eq; use serde_json::json; - use super::ExtensionToolHandler; + use super::ExtensionToolAdapter; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::hook_names::HookToolName; + use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; - use crate::tools::registry::ToolHandler; use crate::turn_diff_tracker::TurnDiffTracker; struct StubExtensionExecutor; #[async_trait::async_trait] impl codex_extension_api::ToolExecutor for StubExtensionExecutor { - type Output = codex_tools::JsonToolOutput; - fn tool_name(&self) -> codex_tools::ToolName { codex_tools::ToolName::plain("extension_echo") } @@ -148,14 +151,16 @@ mod tests { async fn handle( &self, _call: codex_tools::ToolCall, - ) -> Result { - Ok(codex_tools::JsonToolOutput::new(json!({ "ok": true }))) + ) -> Result, codex_tools::FunctionCallError> { + Ok(Box::new(codex_tools::JsonToolOutput::new( + json!({ "ok": true }), + ))) } } #[tokio::test] async fn exposes_generic_hook_payloads() { - let handler = ExtensionToolHandler::new(Arc::new(StubExtensionExecutor)); + let handler = ExtensionToolAdapter::new(Arc::new(StubExtensionExecutor)); let (session, turn) = crate::session::tests::make_session_and_context().await; let invocation = ToolInvocation { session: session.into(), @@ -172,14 +177,14 @@ mod tests { let output = codex_tools::JsonToolOutput::new(json!({ "ok": true })); assert_eq!( - ToolHandler::pre_tool_use_payload(&handler, &invocation), + CoreToolRuntime::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), + CoreToolRuntime::post_tool_use_payload(&handler, &invocation, &output), Some(PostToolUsePayload { tool_name: HookToolName::new("extension_echo"), tool_use_id: "call-extension".to_string(), diff --git a/codex-rs/core/src/tools/handlers/goal/create_goal.rs b/codex-rs/core/src/tools/handlers/goal/create_goal.rs index f7575e1e5242..9ae0107f34b3 100644 --- a/codex-rs/core/src/tools/handlers/goal/create_goal.rs +++ b/codex-rs/core/src/tools/handlers/goal/create_goal.rs @@ -1,13 +1,13 @@ use crate::function_tool::FunctionCallError; use crate::goals::CreateGoalRequest; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::goal_spec::CREATE_GOAL_TOOL_NAME; use crate::tools::handlers::goal_spec::create_create_goal_tool; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -20,8 +20,6 @@ pub struct CreateGoalHandler; #[async_trait::async_trait] impl ToolExecutor for CreateGoalHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(CREATE_GOAL_TOOL_NAME) } @@ -30,7 +28,10 @@ impl ToolExecutor for CreateGoalHandler { Some(create_create_goal_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -70,8 +71,8 @@ impl ToolExecutor for CreateGoalHandler { FunctionCallError::RespondToModel(format_goal_error(err)) } })?; - goal_response(Some(goal), CompletionBudgetReport::Omit) + goal_response(Some(goal), CompletionBudgetReport::Omit).map(boxed_tool_output) } } -impl ToolHandler for CreateGoalHandler {} +impl CoreToolRuntime for CreateGoalHandler {} diff --git a/codex-rs/core/src/tools/handlers/goal/get_goal.rs b/codex-rs/core/src/tools/handlers/goal/get_goal.rs index 20691b8b5bd5..ff40227fabeb 100644 --- a/codex-rs/core/src/tools/handlers/goal/get_goal.rs +++ b/codex-rs/core/src/tools/handlers/goal/get_goal.rs @@ -1,11 +1,11 @@ use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::goal_spec::GET_GOAL_TOOL_NAME; use crate::tools::handlers::goal_spec::create_get_goal_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -17,8 +17,6 @@ pub struct GetGoalHandler; #[async_trait::async_trait] impl ToolExecutor for GetGoalHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(GET_GOAL_TOOL_NAME) } @@ -27,7 +25,10 @@ impl ToolExecutor for GetGoalHandler { Some(create_get_goal_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, payload, .. } = invocation; @@ -38,7 +39,7 @@ impl ToolExecutor for GetGoalHandler { .get_thread_goal() .await .map_err(|err| FunctionCallError::RespondToModel(format_goal_error(err)))?; - goal_response(goal, CompletionBudgetReport::Omit) + goal_response(goal, CompletionBudgetReport::Omit).map(boxed_tool_output) } _ => Err(FunctionCallError::RespondToModel( "get_goal handler received unsupported payload".to_string(), @@ -47,4 +48,4 @@ impl ToolExecutor for GetGoalHandler { } } -impl ToolHandler for GetGoalHandler {} +impl CoreToolRuntime for GetGoalHandler {} diff --git a/codex-rs/core/src/tools/handlers/goal/update_goal.rs b/codex-rs/core/src/tools/handlers/goal/update_goal.rs index bb8ed10efd1f..8cf8ce546a8b 100644 --- a/codex-rs/core/src/tools/handlers/goal/update_goal.rs +++ b/codex-rs/core/src/tools/handlers/goal/update_goal.rs @@ -1,14 +1,14 @@ use crate::function_tool::FunctionCallError; use crate::goals::GoalRuntimeEvent; use crate::goals::SetGoalRequest; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::goal_spec::UPDATE_GOAL_TOOL_NAME; use crate::tools::handlers::goal_spec::create_update_goal_tool; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::protocol::ThreadGoalStatus; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -22,8 +22,6 @@ pub struct UpdateGoalHandler; #[async_trait::async_trait] impl ToolExecutor for UpdateGoalHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(UPDATE_GOAL_TOOL_NAME) } @@ -32,7 +30,10 @@ impl ToolExecutor for UpdateGoalHandler { Some(create_update_goal_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -73,8 +74,8 @@ impl ToolExecutor for UpdateGoalHandler { ) .await .map_err(|err| FunctionCallError::RespondToModel(format_goal_error(err)))?; - goal_response(Some(goal), CompletionBudgetReport::Include) + goal_response(Some(goal), CompletionBudgetReport::Include).map(boxed_tool_output) } } -impl ToolHandler for UpdateGoalHandler {} +impl CoreToolRuntime for UpdateGoalHandler {} diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index f82f741346eb..6f4f79505b1b 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -6,15 +6,15 @@ use crate::mcp_tool_call::handle_mcp_tool_call; use crate::original_image_detail::can_request_original_image_detail; use crate::tools::context::McpToolOutput; use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; use crate::tools::registry::ToolExposure; -use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolTelemetryTags; use crate::tools::tool_search_entry::ToolSearchInfo; use codex_mcp::ToolInfo; @@ -47,8 +47,6 @@ impl McpHandler { #[async_trait::async_trait] impl ToolExecutor for McpHandler { - type Output = McpToolOutput; - fn tool_name(&self) -> ToolName { self.tool_info.canonical_tool_name() } @@ -89,7 +87,10 @@ impl ToolExecutor for McpHandler { self.tool_info.supports_parallel_tool_calls } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -119,17 +120,17 @@ impl ToolExecutor for McpHandler { ) .await; - Ok(McpToolOutput { + Ok(boxed_tool_output(McpToolOutput { result: result.result, tool_input: result.tool_input, wall_time: started.elapsed(), original_image_detail_supported: can_request_original_image_detail(&turn.model_info), truncation_policy: turn.truncation_policy, - }) + })) } } -impl ToolHandler for McpHandler { +impl CoreToolRuntime for McpHandler { fn search_info(&self) -> Option { let source_name = self .tool_info @@ -156,12 +157,17 @@ impl ToolHandler for McpHandler { ) } - async fn telemetry_tags(&self, _invocation: &ToolInvocation) -> ToolTelemetryTags { - let mut tags = vec![("mcp_server", self.tool_info.server_name.clone())]; - if let Some(origin) = &self.tool_info.server_origin { - tags.push(("mcp_server_origin", origin.clone())); - } - tags + fn telemetry_tags<'a>( + &'a self, + _invocation: &'a ToolInvocation, + ) -> futures::future::BoxFuture<'a, ToolTelemetryTags> { + Box::pin(async { + let mut tags = vec![("mcp_server", self.tool_info.server_name.clone())]; + if let Some(origin) = &self.tool_info.server_origin { + tags.push(("mcp_server_origin", origin.clone())); + } + tags + }) } fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { @@ -200,7 +206,7 @@ impl ToolHandler for McpHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { let ToolPayload::Function { .. } = &invocation.payload else { return None; @@ -211,7 +217,7 @@ impl ToolHandler for McpHandler { Some(PostToolUsePayload { tool_name: HookToolName::new(self.tool_name().to_string()), tool_use_id: invocation.call_id.clone(), - tool_input: result.tool_input.clone(), + tool_input: result.post_tool_use_input(&invocation.payload)?, tool_response, }) } diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs index 71749e9cd1f9..866c7a9e123f 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs @@ -1,12 +1,12 @@ use std::time::Instant; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::mcp_resource_spec::create_list_mcp_resource_templates_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::McpInvocation; use codex_tools::ToolName; @@ -28,8 +28,6 @@ pub struct ListMcpResourceTemplatesHandler; #[async_trait::async_trait] impl ToolExecutor for ListMcpResourceTemplatesHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("list_mcp_resource_templates") } @@ -46,7 +44,10 @@ impl ToolExecutor for ListMcpResourceTemplatesHandler { clippy::await_holding_invalid_type, reason = "MCP resource template listing reads through the session-owned manager guard" )] - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -131,7 +132,7 @@ impl ToolExecutor for ListMcpResourceTemplatesHandler { Ok(call_tool_result_from_content(&content, output.success)), ) .await; - Ok(output) + Ok(boxed_tool_output(output)) } Err(err) => { let duration = start.elapsed(); @@ -166,4 +167,4 @@ impl ToolExecutor for ListMcpResourceTemplatesHandler { } } -impl ToolHandler for ListMcpResourceTemplatesHandler {} +impl CoreToolRuntime for ListMcpResourceTemplatesHandler {} diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs index 08b387376b4e..c87747eea42f 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs @@ -1,12 +1,12 @@ use std::time::Instant; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::mcp_resource_spec::create_list_mcp_resources_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::McpInvocation; use codex_tools::ToolName; @@ -28,8 +28,6 @@ pub struct ListMcpResourcesHandler; #[async_trait::async_trait] impl ToolExecutor for ListMcpResourcesHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("list_mcp_resources") } @@ -46,7 +44,10 @@ impl ToolExecutor for ListMcpResourcesHandler { clippy::await_holding_invalid_type, reason = "MCP resource listing reads through the session-owned manager guard" )] - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -129,7 +130,7 @@ impl ToolExecutor for ListMcpResourcesHandler { Ok(call_tool_result_from_content(&content, output.success)), ) .await; - Ok(output) + Ok(boxed_tool_output(output)) } Err(err) => { let duration = start.elapsed(); @@ -164,4 +165,4 @@ impl ToolExecutor for ListMcpResourcesHandler { } } -impl ToolHandler for ListMcpResourcesHandler {} +impl CoreToolRuntime for ListMcpResourcesHandler {} diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs index bd8172ac74a1..3bb4d7f0ecce 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs @@ -1,12 +1,12 @@ use std::time::Instant; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::mcp_resource_spec::create_read_mcp_resource_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::McpInvocation; use codex_tools::ToolName; @@ -28,8 +28,6 @@ pub struct ReadMcpResourceHandler; #[async_trait::async_trait] impl ToolExecutor for ReadMcpResourceHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("read_mcp_resource") } @@ -42,7 +40,10 @@ impl ToolExecutor for ReadMcpResourceHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -112,7 +113,7 @@ impl ToolExecutor for ReadMcpResourceHandler { Ok(call_tool_result_from_content(&content, output.success)), ) .await; - Ok(output) + Ok(boxed_tool_output(output)) } Err(err) => { let duration = start.elapsed(); @@ -147,4 +148,4 @@ impl ToolExecutor for ReadMcpResourceHandler { } } -impl ToolHandler for ReadMcpResourceHandler {} +impl CoreToolRuntime for ReadMcpResourceHandler {} diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index d0c5edfb776f..1587c4e84040 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -13,10 +13,11 @@ use crate::session::turn_context::TurnContext; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; pub(crate) use crate::tools::handlers::multi_agents_common::*; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::ThreadId; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::ReasoningEffort; diff --git a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs index 6459a899fe95..805f569b8e79 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs @@ -7,8 +7,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = CloseAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("close_agent") } @@ -17,8 +15,11 @@ impl ToolExecutor for Handler { Some(create_close_agent_tool_v1()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_close_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_close_agent(invocation).await.map(boxed_tool_output) } } @@ -105,7 +106,7 @@ async fn handle_close_agent( }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs index 9dab2d999d0c..71b29fb423aa 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs @@ -9,8 +9,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = ResumeAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("resume_agent") } @@ -19,8 +17,11 @@ impl ToolExecutor for Handler { Some(create_resume_agent_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_resume_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_resume_agent(invocation).await.map(boxed_tool_output) } } @@ -133,7 +134,7 @@ async fn handle_resume_agent( Ok(ResumeAgentResult { status }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs b/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs index a6067e5b1295..74f7eb05b4b8 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs @@ -8,8 +8,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = SendInputResult; - fn tool_name(&self) -> ToolName { ToolName::plain("send_input") } @@ -18,7 +16,10 @@ impl ToolExecutor for Handler { Some(create_send_input_tool_v1()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -85,11 +86,11 @@ impl ToolExecutor for Handler { .await; let submission_id = result?; - Ok(SendInputResult { submission_id }) + Ok(boxed_tool_output(SendInputResult { submission_id })) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 91f59902107a..d19849d44325 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -24,8 +24,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = SpawnAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("spawn_agent") } @@ -34,8 +32,11 @@ impl ToolExecutor for Handler { Some(create_spawn_agent_tool_v1(self.options.clone())) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_spawn_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_spawn_agent(invocation).await.map(boxed_tool_output) } } @@ -198,7 +199,7 @@ async fn handle_spawn_agent( }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs index d325edeeb05a..a69784b901c2 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs @@ -29,8 +29,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = WaitAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("wait_agent") } @@ -39,7 +37,10 @@ impl ToolExecutor for Handler { Some(create_wait_agent_tool_v1(self.options)) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -197,11 +198,11 @@ impl ToolExecutor for Handler { ) .await; - Ok(result) + Ok(boxed_tool_output(result)) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 12c25aa8108b..153de048eee3 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -317,7 +317,8 @@ async fn spawn_agent_fork_context_rejects_agent_type_override() { })), )) .await - .expect_err("fork_context should reject agent_type overrides"); + .err() + .expect("fork_context should reject agent_type overrides"); assert_eq!( err, @@ -351,7 +352,8 @@ async fn spawn_agent_fork_context_rejects_child_model_overrides() { })), )) .await - .expect_err("forked spawn should reject child model overrides"); + .err() + .expect("forked spawn should reject child model overrides"); assert_eq!( err, @@ -395,7 +397,8 @@ async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() { })), )) .await - .expect_err("fork_turns=all should reject agent_type overrides"); + .err() + .expect("fork_turns=all should reject agent_type overrides"); assert_eq!( err, @@ -435,7 +438,8 @@ async fn multi_agent_v2_spawn_defaults_to_full_fork_and_rejects_child_model_over })), )) .await - .expect_err("default full fork should reject child model overrides"); + .err() + .expect("default full fork should reject child model overrides"); assert_eq!( err, @@ -505,7 +509,8 @@ async fn spawn_agent_service_tier_override_validates_the_effective_child_model() })), )) .await - .expect_err("unknown service tier should be rejected"); + .err() + .expect("unknown service tier should be rejected"); assert_eq!( err, @@ -530,7 +535,8 @@ async fn spawn_agent_service_tier_override_validates_the_effective_child_model() })), )) .await - .expect_err("tier unsupported by the final child model should be rejected"); + .err() + .expect("tier unsupported by the final child model should be rejected"); assert_eq!( err, @@ -1116,7 +1122,8 @@ async fn multi_agent_v2_spawn_rejects_legacy_fork_context() { })), )) .await - .expect_err("legacy fork_context should be rejected"); + .err() + .expect("legacy fork_context should be rejected"); assert_eq!( err, @@ -1155,7 +1162,8 @@ async fn multi_agent_v2_spawn_rejects_invalid_fork_turns_string() { })), )) .await - .expect_err("invalid fork_turns should be rejected"); + .err() + .expect("invalid fork_turns should be rejected"); assert_eq!( err, @@ -1194,7 +1202,8 @@ async fn multi_agent_v2_spawn_rejects_zero_fork_turns() { })), )) .await - .expect_err("zero turn count should be rejected"); + .err() + .expect("zero turn count should be rejected"); assert_eq!( err, @@ -3630,7 +3639,8 @@ async fn multi_agent_v2_close_agent_rejects_root_target_and_id() { function_payload(json!({"target": "/root"})), )) .await - .expect_err("close_agent should reject the root path"); + .err() + .expect("close_agent should reject the root path"); assert_eq!( root_path_error, FunctionCallError::RespondToModel("root is not a spawned agent".to_string()) @@ -3644,7 +3654,8 @@ async fn multi_agent_v2_close_agent_rejects_root_target_and_id() { function_payload(json!({"target": root.thread_id.to_string()})), )) .await - .expect_err("close_agent should reject the root thread id"); + .err() + .expect("close_agent should reject the root thread id"); assert_eq!( root_id_error, FunctionCallError::RespondToModel("root is not a spawned agent".to_string()) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2.rs index 0190fe04acaf..068f393379f4 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2.rs @@ -6,10 +6,11 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::multi_agents_common::*; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::AgentPath; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::ReasoningEffort; diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs index 7b575f82588c..180cf1a9d89b 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs @@ -7,8 +7,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = CloseAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("close_agent") } @@ -17,8 +15,11 @@ impl ToolExecutor for Handler { Some(create_close_agent_tool_v2()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_close_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_close_agent(invocation).await.map(boxed_tool_output) } } @@ -117,7 +118,7 @@ async fn handle_close_agent( }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs index 7d111f041856..4b3edcd4873b 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs @@ -2,7 +2,6 @@ use super::message_tool::FollowupTaskArgs; use super::message_tool::MessageDeliveryMode; use super::message_tool::handle_message_string_tool; use super::*; -use crate::tools::context::FunctionToolOutput; use crate::tools::handlers::multi_agents_spec::create_followup_task_tool; use codex_tools::ToolSpec; @@ -10,8 +9,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("followup_task") } @@ -20,7 +17,10 @@ impl ToolExecutor for Handler { Some(create_followup_task_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let arguments = function_arguments(invocation.payload.clone())?; let args: FollowupTaskArgs = parse_arguments(&arguments)?; handle_message_string_tool( @@ -30,10 +30,11 @@ impl ToolExecutor for Handler { args.message, ) .await + .map(boxed_tool_output) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs index 8b0ee551e3d2..b8b9072da9ac 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs @@ -7,8 +7,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = ListAgentsResult; - fn tool_name(&self) -> ToolName { ToolName::plain("list_agents") } @@ -17,7 +15,10 @@ impl ToolExecutor for Handler { Some(create_list_agents_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -37,11 +38,11 @@ impl ToolExecutor for Handler { .await .map_err(collab_spawn_error)?; - Ok(ListAgentsResult { agents }) + Ok(boxed_tool_output(ListAgentsResult { agents })) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs index 584feec61fdf..202e28e0c972 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs @@ -2,7 +2,6 @@ use super::message_tool::MessageDeliveryMode; use super::message_tool::SendMessageArgs; use super::message_tool::handle_message_string_tool; use super::*; -use crate::tools::context::FunctionToolOutput; use crate::tools::handlers::multi_agents_spec::create_send_message_tool; use codex_tools::ToolSpec; @@ -10,8 +9,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("send_message") } @@ -20,7 +17,10 @@ impl ToolExecutor for Handler { Some(create_send_message_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let arguments = function_arguments(invocation.payload.clone())?; let args: SendMessageArgs = parse_arguments(&arguments)?; handle_message_string_tool( @@ -30,10 +30,11 @@ impl ToolExecutor for Handler { args.message, ) .await + .map(boxed_tool_output) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 3ad7b8714546..2daa1f07dbf4 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -26,8 +26,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = SpawnAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("spawn_agent") } @@ -36,8 +34,11 @@ impl ToolExecutor for Handler { Some(create_spawn_agent_tool_v2(self.options.clone())) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_spawn_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_spawn_agent(invocation).await.map(boxed_tool_output) } } @@ -229,7 +230,7 @@ async fn handle_spawn_agent( } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs index 53c976c9d4e1..8913411b53dc 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs @@ -21,8 +21,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = WaitAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("wait_agent") } @@ -31,7 +29,10 @@ impl ToolExecutor for Handler { Some(create_wait_agent_tool_v2(self.options)) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -97,11 +98,11 @@ impl ToolExecutor for Handler { ) .await; - Ok(result) + Ok(boxed_tool_output(result)) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/plan.rs b/codex-rs/core/src/tools/handlers/plan.rs index 9868c77ca75d..f5c94826f73b 100644 --- a/codex-rs/core/src/tools/handlers/plan.rs +++ b/codex-rs/core/src/tools/handlers/plan.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::plan_spec::create_update_plan_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::config_types::ModeKind; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; @@ -46,8 +47,6 @@ impl ToolOutput for PlanToolOutput { #[async_trait::async_trait] impl ToolExecutor for PlanHandler { - type Output = PlanToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("update_plan") } @@ -56,7 +55,10 @@ impl ToolExecutor for PlanHandler { Some(create_update_plan_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -85,11 +87,11 @@ impl ToolExecutor for PlanHandler { .send_event(turn.as_ref(), EventMsg::PlanUpdate(args)) .await; - Ok(PlanToolOutput) + Ok(boxed_tool_output(PlanToolOutput)) } } -impl ToolHandler for PlanHandler {} +impl CoreToolRuntime for PlanHandler {} fn parse_update_plan_arguments(arguments: &str) -> Result { serde_json::from_str::(arguments).map_err(|e| { diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index 2f2e71677e9b..007c8d220898 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -5,11 +5,12 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::shell_spec::create_request_permissions_tool; use crate::tools::handlers::shell_spec::request_permissions_tool_description; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -17,8 +18,6 @@ pub struct RequestPermissionsHandler; #[async_trait::async_trait] impl ToolExecutor for RequestPermissionsHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("request_permissions") } @@ -29,7 +28,10 @@ impl ToolExecutor for RequestPermissionsHandler { )) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -75,8 +77,11 @@ impl ToolExecutor for RequestPermissionsHandler { )) })?; - Ok(FunctionToolOutput::from_text(content, Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + content, + Some(true), + ))) } } -impl ToolHandler for RequestPermissionsHandler {} +impl CoreToolRuntime for RequestPermissionsHandler {} diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index 6073831e026d..3b53fd007ab5 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -32,10 +32,11 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::request_plugin_install_spec::create_request_plugin_install_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; #[derive(Default)] pub struct RequestPluginInstallHandler { @@ -52,8 +53,6 @@ impl RequestPluginInstallHandler { #[async_trait::async_trait] impl ToolExecutor for RequestPluginInstallHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME) } @@ -70,7 +69,10 @@ impl ToolExecutor for RequestPluginInstallHandler { clippy::await_holding_invalid_type, reason = "plugin install discovery reads through the session-owned manager guard" )] - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { payload, session, @@ -190,11 +192,14 @@ impl ToolExecutor for RequestPluginInstallHandler { )) })?; - Ok(FunctionToolOutput::from_text(content, Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + content, + Some(true), + ))) } } -impl ToolHandler for RequestPluginInstallHandler {} +impl CoreToolRuntime for RequestPluginInstallHandler {} async fn maybe_persist_disabled_install_request( session: &crate::session::session::Session, diff --git a/codex-rs/core/src/tools/handlers/request_user_input.rs b/codex-rs/core/src/tools/handlers/request_user_input.rs index 7597cb6004b1..8384e02637ae 100644 --- a/codex-rs/core/src/tools/handlers/request_user_input.rs +++ b/codex-rs/core/src/tools/handlers/request_user_input.rs @@ -2,14 +2,15 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::request_user_input_spec::REQUEST_USER_INPUT_TOOL_NAME; use crate::tools::handlers::request_user_input_spec::create_request_user_input_tool; use crate::tools::handlers::request_user_input_spec::normalize_request_user_input_args; use crate::tools::handlers::request_user_input_spec::request_user_input_tool_description; use crate::tools::handlers::request_user_input_spec::request_user_input_unavailable_message; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::config_types::ModeKind; use codex_protocol::request_user_input::RequestUserInputArgs; use codex_tools::ToolName; @@ -21,8 +22,6 @@ pub struct RequestUserInputHandler { #[async_trait::async_trait] impl ToolExecutor for RequestUserInputHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME) } @@ -33,7 +32,10 @@ impl ToolExecutor for RequestUserInputHandler { )) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -80,11 +82,14 @@ impl ToolExecutor for RequestUserInputHandler { )) })?; - Ok(FunctionToolOutput::from_text(content, Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + content, + Some(true), + ))) } } -impl ToolHandler for RequestUserInputHandler {} +impl CoreToolRuntime for RequestUserInputHandler {} #[cfg(test)] #[path = "request_user_input_tests.rs"] diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index 54d74f13b7ad..53156c5cfa98 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -10,19 +10,18 @@ use crate::function_tool::FunctionCallError; use crate::maybe_emit_implicit_skill_invocation; use crate::session::turn_context::TurnContext; use crate::shell::Shell; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::resolve_workdir_base_path; use crate::tools::handlers::rewrite_function_string_argument; use crate::tools::handlers::updated_hook_command; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::runtimes::shell::ShellRuntimeBackend; use codex_tools::ToolSpec; @@ -129,8 +128,6 @@ impl From for ShellCommandHandler { #[async_trait::async_trait] impl ToolExecutor for ShellCommandHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("shell_command") } @@ -148,7 +145,10 @@ impl ToolExecutor for ShellCommandHandler { self.options.is_some() } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -201,10 +201,11 @@ impl ToolExecutor for ShellCommandHandler { shell_runtime_backend: self.shell_runtime_backend(), }) .await + .map(boxed_tool_output) } } -impl ToolHandler for ShellCommandHandler { +impl CoreToolRuntime for ShellCommandHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } @@ -240,7 +241,7 @@ impl ToolHandler for ShellCommandHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { let tool_response = result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index 660bac678951..eda23533a352 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -18,7 +18,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::handlers::ShellCommandHandler; use crate::tools::hook_names::HookToolName; -use crate::tools::registry::ToolHandler; +use crate::tools::registry::CoreToolRuntime; use crate::turn_diff_tracker::TurnDiffTracker; use codex_shell_command::is_safe_command::is_known_safe_command; use codex_shell_command::powershell::try_find_powershell_executable_blocking; diff --git a/codex-rs/core/src/tools/handlers/test_sync.rs b/codex-rs/core/src/tools/handlers/test_sync.rs index feee6470a3f3..d3c95dea7eab 100644 --- a/codex-rs/core/src/tools/handlers/test_sync.rs +++ b/codex-rs/core/src/tools/handlers/test_sync.rs @@ -12,10 +12,11 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::test_sync_spec::create_test_sync_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -58,8 +59,6 @@ fn barrier_map() -> &'static tokio::sync::Mutex> { #[async_trait::async_trait] impl ToolExecutor for TestSyncHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("test_sync_tool") } @@ -72,7 +71,10 @@ impl ToolExecutor for TestSyncHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { payload, .. } = invocation; let arguments = match payload { @@ -102,11 +104,14 @@ impl ToolExecutor for TestSyncHandler { sleep(Duration::from_millis(delay)).await; } - Ok(FunctionToolOutput::from_text("ok".to_string(), Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + "ok".to_string(), + Some(true), + ))) } } -impl ToolHandler for TestSyncHandler {} +impl CoreToolRuntime for TestSyncHandler {} async fn wait_on_barrier(args: BarrierArgs) -> Result<(), FunctionCallError> { if args.participants == 0 { diff --git a/codex-rs/core/src/tools/handlers/tool_search.rs b/codex-rs/core/src/tools/handlers/tool_search.rs index 579826ac04a2..ffb25cb88088 100644 --- a/codex-rs/core/src/tools/handlers/tool_search.rs +++ b/codex-rs/core/src/tools/handlers/tool_search.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::context::ToolSearchOutput; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::tool_search_spec::create_tool_search_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::tool_search_entry::ToolSearchEntry; use crate::tools::tool_search_entry::ToolSearchInfo; use bm25::Document; @@ -54,8 +55,6 @@ impl ToolSearchHandler { #[async_trait::async_trait] impl ToolExecutor for ToolSearchHandler { - type Output = ToolSearchOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(TOOL_SEARCH_TOOL_NAME) } @@ -74,7 +73,7 @@ impl ToolExecutor for ToolSearchHandler { async fn handle( &self, invocation: ToolInvocation, - ) -> Result { + ) -> Result, FunctionCallError> { let ToolInvocation { payload, .. } = invocation; let args = match payload { @@ -101,16 +100,16 @@ impl ToolExecutor for ToolSearchHandler { } if self.entries.is_empty() { - return Ok(ToolSearchOutput { tools: Vec::new() }); + return Ok(boxed_tool_output(ToolSearchOutput { tools: Vec::new() })); } let tools = self.search(query, limit)?; - Ok(ToolSearchOutput { tools }) + Ok(boxed_tool_output(ToolSearchOutput { tools })) } } -impl ToolHandler for ToolSearchHandler {} +impl CoreToolRuntime for ToolSearchHandler {} impl ToolSearchHandler { fn search( diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 22e0a16dd8d9..b4e7bd6acf1f 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -2,7 +2,6 @@ use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; use crate::shell::ShellType; use crate::shell::get_shell_by_model_provided_path; -use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -88,23 +87,19 @@ pub(crate) struct ResolvedCommand { fn post_unified_exec_tool_use_payload( invocation: &ToolInvocation, - result: &ExecCommandToolOutput, + result: &dyn ToolOutput, ) -> Option { let ToolPayload::Function { .. } = &invocation.payload else { return None; }; - let command = result.hook_command.clone()?; - let tool_use_id = if result.event_call_id.is_empty() { - invocation.call_id.clone() - } else { - result.event_call_id.clone() - }; + let tool_input = result.post_tool_use_input(&invocation.payload)?; + let tool_use_id = result.post_tool_use_id(&invocation.call_id); let tool_response = result.post_tool_use_response(&tool_use_id, &invocation.payload)?; Some(PostToolUsePayload { tool_name: HookToolName::bash(), tool_use_id, - tool_input: serde_json::json!({ "command": command }), + tool_input, tool_response, }) } diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index 2df924b5f4bc..b048227cf15a 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -5,6 +5,7 @@ use crate::maybe_emit_implicit_skill_invocation; use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::apply_granted_turn_permissions; use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::handlers::implicit_granted_permissions; @@ -15,10 +16,10 @@ use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::rewrite_function_string_argument; use crate::tools::handlers::updated_hook_command; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::unified_exec::ExecCommandRequest; use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecError; @@ -70,8 +71,6 @@ impl ExecCommandHandler { #[async_trait::async_trait] impl ToolExecutor for ExecCommandHandler { - type Output = ExecCommandToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("exec_command") } @@ -90,7 +89,10 @@ impl ToolExecutor for ExecCommandHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -234,7 +236,7 @@ impl ToolExecutor for ExecCommandHandler { .await? { manager.release_process_id(process_id).await; - return Ok(ExecCommandToolOutput { + return Ok(boxed_tool_output(ExecCommandToolOutput { event_call_id: String::new(), chunk_id: String::new(), wall_time: std::time::Duration::ZERO, @@ -244,7 +246,7 @@ impl ToolExecutor for ExecCommandHandler { exit_code: None, original_token_count: None, hook_command: None, - }); + })); } emit_unified_exec_tty_metric(&turn.session_telemetry, tty); @@ -273,11 +275,11 @@ impl ToolExecutor for ExecCommandHandler { ) .await { - Ok(response) => Ok(response), + Ok(response) => Ok(boxed_tool_output(response)), Err(UnifiedExecError::SandboxDenied { output, .. }) => { let output_text = output.aggregated_output.text; let original_token_count = approx_token_count(&output_text); - Ok(ExecCommandToolOutput { + Ok(boxed_tool_output(ExecCommandToolOutput { event_call_id: context.call_id.clone(), chunk_id: generate_chunk_id(), wall_time: output.duration, @@ -289,7 +291,7 @@ impl ToolExecutor for ExecCommandHandler { exit_code: Some(output.exit_code), original_token_count: Some(original_token_count), hook_command: Some(hook_command), - }) + })) } Err(err) => Err(FunctionCallError::RespondToModel(format!( "exec_command failed for `{command_for_display}`: {err:?}" @@ -298,7 +300,7 @@ impl ToolExecutor for ExecCommandHandler { } } -impl ToolHandler for ExecCommandHandler { +impl CoreToolRuntime for ExecCommandHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } @@ -340,7 +342,7 @@ impl ToolHandler for ExecCommandHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { post_unified_exec_tool_use_payload(invocation, result) } diff --git a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs index 29ee4b4ea38a..3dea7092f5f8 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs @@ -1,11 +1,11 @@ use crate::function_tool::FunctionCallError; -use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::unified_exec::WriteStdinRequest; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::TerminalInteractionEvent; @@ -33,8 +33,6 @@ pub struct WriteStdinHandler; #[async_trait::async_trait] impl ToolExecutor for WriteStdinHandler { - type Output = ExecCommandToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("write_stdin") } @@ -43,7 +41,10 @@ impl ToolExecutor for WriteStdinHandler { Some(create_write_stdin_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -86,11 +87,11 @@ impl ToolExecutor for WriteStdinHandler { .send_event(turn.as_ref(), EventMsg::TerminalInteraction(interaction)) .await; - Ok(response) + Ok(boxed_tool_output(response)) } } -impl ToolHandler for WriteStdinHandler { +impl CoreToolRuntime for WriteStdinHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } @@ -98,7 +99,7 @@ impl ToolHandler for WriteStdinHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { post_unified_exec_tool_use_payload(invocation, result) } diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 6ab2752b94e5..50a8e44214ff 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -13,7 +13,7 @@ use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::hook_names::HookToolName; -use crate::tools::registry::ToolHandler; +use crate::tools::registry::CoreToolRuntime; use crate::turn_diff_tracker::TurnDiffTracker; use tokio::sync::Mutex; diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 7727fb4370b2..7ad6a79829d8 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -16,12 +16,13 @@ use crate::original_image_detail::can_request_original_image_detail; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::handlers::view_image_spec::create_view_image_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -64,8 +65,6 @@ enum ViewImageDetail { #[async_trait::async_trait] impl ToolExecutor for ViewImageHandler { - type Output = ViewImageOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("view_image") } @@ -78,7 +77,10 @@ impl ToolExecutor for ViewImageHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { if !invocation .turn .model_info @@ -195,14 +197,14 @@ impl ToolExecutor for ViewImageHandler { session.emit_turn_item_started(turn.as_ref(), &item).await; session.emit_turn_item_completed(turn.as_ref(), item).await; - Ok(ViewImageOutput { + Ok(boxed_tool_output(ViewImageOutput { image_url, image_detail, - }) + })) } } -impl ToolHandler for ViewImageHandler {} +impl CoreToolRuntime for ViewImageHandler {} pub struct ViewImageOutput { image_url: String, diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 3073d9f9da60..1effab52c332 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -13,7 +13,6 @@ pub(crate) mod runtimes; pub(crate) mod sandboxing; pub(crate) mod spec; pub(crate) mod spec_plan; -pub(crate) mod spec_plan_types; pub(crate) mod tool_dispatch_trace; pub(crate) mod tool_search_entry; diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 730c3b7ef934..0575691900d2 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::future::Future; use std::sync::Arc; use std::time::Duration; @@ -35,7 +34,11 @@ pub(crate) type ToolTelemetryTags = Vec<(&'static str, String)>; pub use codex_tools::ToolExecutor; pub use codex_tools::ToolExposure; -pub trait ToolHandler: ToolExecutor { +/// Typed runtime contract for locally executed tools. +/// +/// Implementors provide the shared `ToolExecutor` behavior plus optional +/// core-owned metadata for hooks, telemetry, tool search, and argument diffs. +pub(crate) trait CoreToolRuntime: ToolExecutor { fn search_info(&self) -> Option { None } @@ -47,17 +50,17 @@ pub trait ToolHandler: ToolExecutor { ) } - fn telemetry_tags( - &self, - _invocation: &ToolInvocation, - ) -> impl Future + Send { - async { Vec::new() } + fn telemetry_tags<'a>( + &'a self, + _invocation: &'a ToolInvocation, + ) -> BoxFuture<'a, ToolTelemetryTags> { + Box::pin(async { Vec::new() }) } fn post_tool_use_payload( &self, _invocation: &ToolInvocation, - _result: &Self::Output, + _result: &dyn ToolOutput, ) -> Option { None } @@ -154,117 +157,10 @@ pub(crate) struct PostToolUsePayload { pub(crate) tool_response: Value, } -/// Object-safe registry entry for heterogeneous tool handlers. -/// -/// Concrete handlers keep their typed `ToolExecutor::Output`; the registry -/// boxes that output only after typed hooks have run. -pub(crate) trait RegisteredTool: Send + Sync { - fn tool_name(&self) -> ToolName; - - fn spec(&self) -> Option; - - fn exposure(&self) -> ToolExposure; - - fn search_info(&self) -> Option; - - fn supports_parallel_tool_calls(&self) -> bool; - - fn matches_kind(&self, payload: &ToolPayload) -> bool; - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option; - - fn with_updated_hook_input( - &self, - invocation: ToolInvocation, - updated_input: Value, - ) -> Result; - - fn telemetry_tags<'a>( - &'a self, - invocation: &'a ToolInvocation, - ) -> BoxFuture<'a, ToolTelemetryTags>; - - fn create_diff_consumer(&self) -> Option>; - fn handle_any<'a>( - &'a self, - invocation: ToolInvocation, - ) -> BoxFuture<'a, Result>; -} - -impl RegisteredTool for T -where - T: ToolHandler, -{ - fn tool_name(&self) -> ToolName { - ToolExecutor::tool_name(self) - } - - fn spec(&self) -> Option { - ToolExecutor::spec(self) - } - - fn exposure(&self) -> ToolExposure { - ToolExecutor::exposure(self) - } - - fn search_info(&self) -> Option { - ToolHandler::search_info(self) - } - - fn supports_parallel_tool_calls(&self) -> bool { - ToolExecutor::supports_parallel_tool_calls(self) - } - - fn matches_kind(&self, payload: &ToolPayload) -> bool { - ToolHandler::matches_kind(self, payload) - } - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - ToolHandler::pre_tool_use_payload(self, invocation) - } - - fn with_updated_hook_input( - &self, - invocation: ToolInvocation, - updated_input: Value, - ) -> Result { - ToolHandler::with_updated_hook_input(self, invocation, updated_input) - } - - fn telemetry_tags<'a>( - &'a self, - invocation: &'a ToolInvocation, - ) -> BoxFuture<'a, ToolTelemetryTags> { - Box::pin(ToolHandler::telemetry_tags(self, invocation)) - } - - fn create_diff_consumer(&self) -> Option> { - ToolHandler::create_diff_consumer(self) - } - fn handle_any<'a>( - &'a self, - invocation: ToolInvocation, - ) -> BoxFuture<'a, Result> { - Box::pin(async move { - let call_id = invocation.call_id.clone(); - let payload = invocation.payload.clone(); - let output = ToolExecutor::handle(self, invocation.clone()).await?; - let post_tool_use_payload = - ToolHandler::post_tool_use_payload(self, &invocation, &output); - Ok(AnyToolResult { - call_id, - payload, - result: Box::new(output), - post_tool_use_payload, - }) - }) - } -} - pub(crate) fn override_tool_exposure( - handler: Arc, + handler: Arc, exposure: ToolExposure, -) -> Arc { +) -> Arc { if handler.exposure() == exposure { return handler; } @@ -273,11 +169,12 @@ pub(crate) fn override_tool_exposure( } struct ExposureOverride { - handler: Arc, + handler: Arc, exposure: ToolExposure, } -impl RegisteredTool for ExposureOverride { +#[async_trait::async_trait] +impl ToolExecutor for ExposureOverride { fn tool_name(&self) -> ToolName { self.handler.tool_name() } @@ -290,14 +187,23 @@ impl RegisteredTool for ExposureOverride { self.exposure } - fn search_info(&self) -> Option { - self.handler.search_info() - } - fn supports_parallel_tool_calls(&self) -> bool { self.handler.supports_parallel_tool_calls() } + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + self.handler.handle(invocation).await + } +} + +impl CoreToolRuntime for ExposureOverride { + fn search_info(&self) -> Option { + self.handler.search_info() + } + fn matches_kind(&self, payload: &ToolPayload) -> bool { self.handler.matches_kind(payload) } @@ -306,6 +212,14 @@ impl RegisteredTool for ExposureOverride { self.handler.pre_tool_use_payload(invocation) } + fn post_tool_use_payload( + &self, + invocation: &ToolInvocation, + result: &dyn ToolOutput, + ) -> Option { + self.handler.post_tool_use_payload(invocation, result) + } + fn with_updated_hook_input( &self, invocation: ToolInvocation, @@ -325,22 +239,28 @@ impl RegisteredTool for ExposureOverride { fn create_diff_consumer(&self) -> Option> { self.handler.create_diff_consumer() } - - fn handle_any<'a>( - &'a self, - invocation: ToolInvocation, - ) -> BoxFuture<'a, Result> { - self.handler.handle_any(invocation) - } } pub struct ToolRegistry { - handlers: HashMap>, + tools: HashMap>, } impl ToolRegistry { - fn new(handlers: HashMap>) -> Self { - Self { handlers } + fn new(tools: HashMap>) -> Self { + Self { tools } + } + + pub(crate) fn from_tools(tools: impl IntoIterator>) -> Self { + let mut tools_by_name = HashMap::new(); + for tool in tools { + let name = tool.tool_name(); + if tools_by_name.contains_key(&name) { + error_or_panic(format!("tool {name} already registered")); + continue; + } + tools_by_name.insert(name, tool); + } + Self::new(tools_by_name) } #[cfg(test)] @@ -351,35 +271,35 @@ impl ToolRegistry { #[cfg(test)] pub(crate) fn with_handler_for_test(handler: Arc) -> Self where - T: ToolHandler + 'static, + T: CoreToolRuntime + 'static, { let name = handler.tool_name(); - Self::new(HashMap::from([(name, handler as Arc)])) + Self::new(HashMap::from([(name, handler as Arc)])) } - fn handler(&self, name: &ToolName) -> Option> { - self.handlers.get(name).map(Arc::clone) + fn tool(&self, name: &ToolName) -> Option> { + self.tools.get(name).map(Arc::clone) } pub(crate) fn tool_exposure(&self, name: &ToolName) -> Option { - self.handlers.get(name).map(|handler| handler.exposure()) + self.tools.get(name).map(|tool| tool.exposure()) } #[cfg(test)] - pub(crate) fn has_handler(&self, name: &ToolName) -> bool { - self.handler(name).is_some() + pub(crate) fn has_tool(&self, name: &ToolName) -> bool { + self.tool(name).is_some() } pub(crate) fn create_diff_consumer( &self, name: &ToolName, ) -> Option> { - self.handler(name)?.create_diff_consumer() + self.tool(name)?.create_diff_consumer() } pub(crate) fn supports_parallel_tool_calls(&self, name: &ToolName) -> Option { - let handler = self.handler(name)?; - Some(handler.supports_parallel_tool_calls()) + let tool = self.tool(name)?; + Some(tool.supports_parallel_tool_calls()) } #[expect( @@ -422,8 +342,8 @@ impl ToolRegistry { } let dispatch_trace = ToolDispatchTrace::start(&invocation); - let handler = match self.handler(&tool_name) { - Some(handler) => handler, + let tool = match self.tool(&tool_name) { + Some(tool) => tool, None => { let message = unsupported_tool_call_message(&invocation.payload, &tool_name); let log_payload = invocation.payload.log_payload(); @@ -443,7 +363,7 @@ impl ToolRegistry { } }; - let telemetry_tags = handler.telemetry_tags(&invocation).await; + let telemetry_tags = tool.telemetry_tags(&invocation).await; let mut tool_result_tags = Vec::with_capacity(base_tool_result_tags.len() + telemetry_tags.len()); let mut extra_trace_fields = Vec::new(); @@ -455,7 +375,7 @@ impl ToolRegistry { tool_result_tags.push((*key, value.as_str())); } } - if !handler.matches_kind(&invocation.payload) { + if !tool.matches_kind(&invocation.payload) { let message = format!("tool {tool_name} invoked with incompatible payload"); let log_payload = invocation.payload.log_payload(); otel.tool_result_with_tags( @@ -473,7 +393,7 @@ impl ToolRegistry { return Err(err); } - if let Some(pre_tool_use_payload) = handler.pre_tool_use_payload(&invocation) { + if let Some(pre_tool_use_payload) = tool.pre_tool_use_payload(&invocation) { match run_pre_tool_use_hooks( &invocation.session, &invocation.turn, @@ -491,7 +411,7 @@ impl ToolRegistry { PreToolUseHookResult::Continue { updated_input: Some(updated_input), } => { - invocation = handler.with_updated_hook_input(invocation, updated_input)?; + invocation = tool.with_updated_hook_input(invocation, updated_input)?; } PreToolUseHookResult::Continue { updated_input: None, @@ -511,10 +431,10 @@ impl ToolRegistry { &tool_result_tags, &extra_trace_fields, || { - let handler = handler.clone(); + let tool = tool.clone(); let response_cell = &response_cell; async move { - match handler.handle_any(invocation_for_tool).await { + match handle_any_tool(tool.as_ref(), invocation_for_tool).await { Ok(result) => { let preview = result.result.log_preview(); let success = result.result.success_for_logging(); @@ -620,52 +540,21 @@ impl ToolRegistry { } } -pub struct ToolRegistryBuilder { - handlers: HashMap>, - specs: Vec, -} - -impl ToolRegistryBuilder { - pub fn new() -> Self { - Self { - handlers: HashMap::new(), - specs: Vec::new(), - } - } - - pub(crate) fn push_spec(&mut self, spec: ToolSpec) { - self.specs.push(spec); - } - - pub(crate) fn register_tool(&mut self, handler: Arc) { - self.register_tool_internal(handler, /*include_spec*/ true); - } - - pub(crate) fn register_tool_without_spec(&mut self, handler: Arc) { - self.register_tool_internal(handler, /*include_spec*/ false); - } - - fn register_tool_internal(&mut self, handler: Arc, include_spec: bool) { - let name = handler.tool_name(); - if self.handlers.contains_key(&name) { - error_or_panic(format!("handler for tool {name} already registered")); - return; - } - - if include_spec - && handler.exposure().is_direct() - && let Some(spec) = handler.spec() - { - self.push_spec(spec); - } - - self.handlers.insert(name, handler); - } - - pub fn build(self) -> (Vec, ToolRegistry) { - let registry = ToolRegistry::new(self.handlers); - (self.specs, registry) - } +async fn handle_any_tool( + tool: &dyn CoreToolRuntime, + invocation: ToolInvocation, +) -> Result { + let call_id = invocation.call_id.clone(); + let payload = invocation.payload.clone(); + let output = tool.handle(invocation.clone()).await?; + let post_tool_use_payload = + CoreToolRuntime::post_tool_use_payload(tool, &invocation, output.as_ref()); + Ok(AnyToolResult { + call_id, + payload, + result: output, + post_tool_use_payload, + }) } fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &ToolName) -> String { diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index b6c8eadadf78..defacf33c01d 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -1,7 +1,4 @@ use super::*; -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; struct TestHandler { @@ -10,21 +7,21 @@ struct TestHandler { #[async_trait::async_trait] impl ToolExecutor for TestHandler { - type Output = crate::tools::context::FunctionToolOutput; - fn tool_name(&self) -> codex_tools::ToolName { self.tool_name.clone() } - async fn handle(&self, _invocation: ToolInvocation) -> Result { - Ok(crate::tools::context::FunctionToolOutput::from_text( - "ok".to_string(), - Some(true), + async fn handle( + &self, + _invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + Ok(Box::new( + crate::tools::context::FunctionToolOutput::from_text("ok".to_string(), Some(true)), )) } } -impl ToolHandler for TestHandler {} +impl CoreToolRuntime for TestHandler {} #[test] fn handler_looks_up_namespaced_aliases_explicitly() { @@ -34,18 +31,18 @@ fn handler_looks_up_namespaced_aliases_explicitly() { let namespaced_name = codex_tools::ToolName::namespaced(namespace, tool_name); let plain_handler = Arc::new(TestHandler { tool_name: plain_name.clone(), - }) as Arc; + }) as Arc; let namespaced_handler = Arc::new(TestHandler { tool_name: namespaced_name.clone(), - }) as Arc; + }) as Arc; let registry = ToolRegistry::new(HashMap::from([ (plain_name.clone(), Arc::clone(&plain_handler)), (namespaced_name.clone(), Arc::clone(&namespaced_handler)), ])); - let plain = registry.handler(&plain_name); - let namespaced = registry.handler(&namespaced_name); - let missing_namespaced = registry.handler(&codex_tools::ToolName::namespaced( + let plain = registry.tool(&plain_name); + let namespaced = registry.tool(&namespaced_name); + let missing_namespaced = registry.tool(&codex_tools::ToolName::namespaced( "mcp__codex_apps__calendar", tool_name, )); @@ -64,15 +61,3 @@ fn handler_looks_up_namespaced_aliases_explicitly() { .is_some_and(|handler| Arc::ptr_eq(handler, &namespaced_handler)) ); } - -#[test] -fn register_tool_adds_executor_and_spec() { - let mut builder = ToolRegistryBuilder::new(); - builder.register_tool(Arc::new(GetGoalHandler)); - - let (specs, registry) = builder.build(); - - assert_eq!(specs.len(), 1); - assert_eq!(specs[0], create_get_goal_tool()); - assert!(registry.has_handler(&codex_tools::ToolName::plain(GET_GOAL_TOOL_NAME))); -} diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 0f111922c6c0..2477ba347c01 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -5,18 +5,16 @@ use crate::tools::context::SharedTurnDiffTracker; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::registry::AnyToolResult; -use crate::tools::registry::RegisteredTool; use crate::tools::registry::ToolArgumentDiffConsumer; -use crate::tools::registry::ToolExposure; use crate::tools::registry::ToolRegistry; -use crate::tools::spec::collect_tool_router_parts; -use crate::tools::spec_plan::build_tool_registry_builder_from_executors; -use codex_extension_api::ExtensionToolExecutor; +use crate::tools::spec_plan::build_tool_router; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::ResponseItem; use codex_protocol::models::SearchToolCallParams; use codex_tools::DiscoverableTool; +use codex_tools::ToolCall as ExtensionToolCall; +use codex_tools::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; @@ -42,42 +40,16 @@ 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_executors: Vec>, + pub(crate) extension_tool_executors: Vec>>, pub(crate) dynamic_tools: &'a [DynamicToolSpec], } impl ToolRouter { pub fn from_config(config: &ToolsConfig, params: ToolRouterParams<'_>) -> Self { - let ToolRouterParams { - mcp_tools, - deferred_mcp_tools, - discoverable_tools, - extension_tool_executors, - dynamic_tools, - } = params; - let parts = collect_tool_router_parts( - config, - mcp_tools, - deferred_mcp_tools, - discoverable_tools, - &extension_tool_executors, - dynamic_tools, - ); - Self::from_executors(config, parts.executors, parts.hosted_specs) + build_tool_router(config, params) } - pub(crate) fn from_executors( - config: &ToolsConfig, - executors: Vec>, - hosted_specs: Vec, - ) -> Self { - let builder = build_tool_registry_builder_from_executors(config, executors, hosted_specs); - let (specs, registry) = builder.build(); - let model_visible_specs = specs - .into_iter() - .filter(|spec| !is_hidden_by_code_mode_only(config, ®istry, spec)) - .collect(); - + pub(crate) fn from_parts(registry: ToolRegistry, model_visible_specs: Vec) -> Self { Self { registry, model_visible_specs, @@ -182,22 +154,9 @@ impl ToolRouter { } } -fn is_hidden_by_code_mode_only( - config: &ToolsConfig, - registry: &ToolRegistry, - spec: &ToolSpec, -) -> bool { - if !config.code_mode_only_enabled || !codex_code_mode::is_code_mode_nested_tool(spec.name()) { - return false; - } - - let exposure = registry - .tool_exposure(&ToolName::plain(spec.name())) - .unwrap_or(ToolExposure::Direct); - exposure != ToolExposure::DirectModelOnly -} - -pub(crate) fn extension_tool_executors(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 dd791d91355b..c9bb4bcafc6d 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -7,7 +7,6 @@ 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::ExtensionToolExecutor; use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall as ExtensionToolCall; use codex_extension_api::ToolExecutor; @@ -37,7 +36,7 @@ impl codex_extension_api::ToolContributor for ExtensionEchoContributor { &self, _session_store: &ExtensionData, _thread_store: &ExtensionData, - ) -> Vec> { + ) -> Vec>> { vec![Arc::new(ExtensionEchoExecutor)] } } @@ -46,8 +45,6 @@ struct ExtensionEchoExecutor; #[async_trait::async_trait] impl ToolExecutor for ExtensionEchoExecutor { - type Output = codex_tools::JsonToolOutput; - fn tool_name(&self) -> ToolName { ToolName::namespaced("extension/", "echo") } @@ -78,14 +75,14 @@ impl ToolExecutor for ExtensionEchoExecutor { async fn handle( &self, call: ExtensionToolCall, - ) -> Result { + ) -> Result, codex_tools::FunctionCallError> { let arguments: serde_json::Value = serde_json::from_str(call.function_arguments()?).expect("test arguments should parse"); - Ok(codex_tools::JsonToolOutput::new(json!({ + Ok(Box::new(codex_tools::JsonToolOutput::new(json!({ "arguments": arguments, "callId": call.call_id, "ok": true, - }))) + })))) } } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 64dfdb8316d2..7ee05c540b18 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1,23 +1,6 @@ -use crate::config::DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS; -use crate::config::DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS; -use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS; use crate::shell::Shell; use crate::shell::ShellType; -use crate::tools::handlers::multi_agents_common::DEFAULT_WAIT_TIMEOUT_MS; -use crate::tools::handlers::multi_agents_common::MAX_WAIT_TIMEOUT_MS; -use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; -use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; -use crate::tools::registry::RegisteredTool; -use crate::tools::spec_plan::collect_tool_executors; -use crate::tools::spec_plan::hosted_model_tool_specs; -use crate::tools::spec_plan_types::ToolRegistryBuildParams; -use codex_extension_api::ExtensionToolExecutor; -use codex_mcp::ToolInfo; -use codex_protocol::dynamic_tools::DynamicToolSpec; -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,66 +12,6 @@ pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType { } } -pub(crate) struct ToolRouterParts { - pub(crate) executors: Vec>, - pub(crate) hosted_specs: Vec, -} - -pub(crate) fn collect_tool_router_parts( - config: &ToolsConfig, - mcp_tools: Option>, - deferred_mcp_tools: Option>, - discoverable_tools: Option>, - extension_tool_executors: &[Arc], - dynamic_tools: &[DynamicToolSpec], -) -> ToolRouterParts { - let default_agent_type_description = - crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); - let (min_wait_timeout_ms, max_wait_timeout_ms, default_wait_timeout_ms) = - if config.multi_agent_v2 { - let min_wait_timeout_ms = config - .wait_agent_min_timeout_ms - .unwrap_or(DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS); - let max_wait_timeout_ms = config - .wait_agent_max_timeout_ms - .unwrap_or(DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS); - let default_wait_timeout_ms = config - .wait_agent_default_timeout_ms - .unwrap_or(DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS); - ( - min_wait_timeout_ms, - max_wait_timeout_ms, - default_wait_timeout_ms, - ) - } else { - ( - MIN_WAIT_TIMEOUT_MS, - MAX_WAIT_TIMEOUT_MS, - DEFAULT_WAIT_TIMEOUT_MS, - ) - }; - let executors = collect_tool_executors( - config, - ToolRegistryBuildParams { - mcp_tools: mcp_tools.as_deref(), - deferred_mcp_tools: deferred_mcp_tools.as_deref(), - discoverable_tools: discoverable_tools.as_deref(), - extension_tool_executors, - dynamic_tools, - default_agent_type_description: &default_agent_type_description, - wait_agent_timeouts: WaitAgentTimeoutOptions { - default_timeout_ms: default_wait_timeout_ms, - min_timeout_ms: min_wait_timeout_ms, - max_timeout_ms: max_wait_timeout_ms, - }, - }, - ); - ToolRouterParts { - executors, - hosted_specs: hosted_model_tool_specs(config), - } -} - #[cfg(test)] #[path = "spec_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 24f59a0f04e8..679772f4c768 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -1,3 +1,6 @@ +use crate::config::DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS; +use crate::config::DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS; +use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS; use crate::tools::code_mode::execute_spec::create_code_mode_tool; use crate::tools::handlers::ApplyPatchHandler; use crate::tools::handlers::CodeModeExecuteHandler; @@ -24,13 +27,17 @@ 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::ExtensionToolHandler; +use crate::tools::handlers::extension_tools::ExtensionToolAdapter; use crate::tools::handlers::multi_agents::CloseAgentHandler; use crate::tools::handlers::multi_agents::ResumeAgentHandler; use crate::tools::handlers::multi_agents::SendInputHandler; use crate::tools::handlers::multi_agents::SpawnAgentHandler; use crate::tools::handlers::multi_agents::WaitAgentHandler; +use crate::tools::handlers::multi_agents_common::DEFAULT_WAIT_TIMEOUT_MS; +use crate::tools::handlers::multi_agents_common::MAX_WAIT_TIMEOUT_MS; +use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions; +use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; use crate::tools::handlers::multi_agents_v2::CloseAgentHandler as CloseAgentHandlerV2; use crate::tools::handlers::multi_agents_v2::FollowupTaskHandler as FollowupTaskHandlerV2; use crate::tools::handlers::multi_agents_v2::ListAgentsHandler as ListAgentsHandlerV2; @@ -41,17 +48,21 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::hosted_spec::WebSearchToolOptions; use crate::tools::hosted_spec::create_image_generation_tool; use crate::tools::hosted_spec::create_web_search_tool; -use crate::tools::registry::RegisteredTool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExposure; -use crate::tools::registry::ToolRegistryBuilder; +use crate::tools::registry::ToolRegistry; use crate::tools::registry::override_tool_exposure; -use crate::tools::spec_plan_types::ToolRegistryBuildParams; -use crate::tools::spec_plan_types::agent_type_description; -use codex_extension_api::ExtensionToolExecutor; +use crate::tools::router::ToolRouter; +use crate::tools::router::ToolRouterParams; +use codex_mcp::ToolInfo; +use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::openai_models::ConfigShellToolType; +use codex_tools::DiscoverableTool; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::TOOL_SEARCH_TOOL_NAME; +use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolEnvironmentMode; +use codex_tools::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; @@ -62,74 +73,94 @@ use std::collections::HashSet; use std::sync::Arc; use tracing::warn; -pub(crate) fn build_tool_registry_builder_from_executors( - config: &ToolsConfig, - executors: Vec>, - hosted_specs: Vec, -) -> ToolRegistryBuilder { - let mut builder = ToolRegistryBuilder::new(); - let deferred_tools_available = executors - .iter() - .any(|executor| executor.exposure() == ToolExposure::Deferred); +#[derive(Clone, Copy)] +struct ToolRegistryBuildParams<'a> { + mcp_tools: Option<&'a [ToolInfo]>, + deferred_mcp_tools: Option<&'a [ToolInfo]>, + discoverable_tools: Option<&'a [DiscoverableTool]>, + extension_tool_executors: &'a [Arc>], + dynamic_tools: &'a [DynamicToolSpec], + default_agent_type_description: &'a str, + wait_agent_timeouts: WaitAgentTimeoutOptions, +} - for executor in build_code_mode_executors( +pub(crate) fn build_tool_router(config: &ToolsConfig, params: ToolRouterParams<'_>) -> ToolRouter { + let (model_visible_specs, registry) = build_tool_specs_and_registry(config, params); + ToolRouter::from_parts(registry, model_visible_specs) +} + +fn build_tool_specs_and_registry( + config: &ToolsConfig, + params: ToolRouterParams<'_>, +) -> (Vec, ToolRegistry) { + let ToolRouterParams { + mcp_tools, + deferred_mcp_tools, + discoverable_tools, + extension_tool_executors, + dynamic_tools, + } = params; + let default_agent_type_description = + crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); + let mut executors = collect_tool_executors( config, - &executors, - config.search_tool && deferred_tools_available, - ) { - builder.register_tool(executor); - } + ToolRegistryBuildParams { + mcp_tools: mcp_tools.as_deref(), + deferred_mcp_tools: deferred_mcp_tools.as_deref(), + discoverable_tools: discoverable_tools.as_deref(), + extension_tool_executors: &extension_tool_executors, + dynamic_tools, + default_agent_type_description: &default_agent_type_description, + wait_agent_timeouts: wait_agent_timeout_options(config), + }, + ); + append_tool_search_executor(config, &mut executors); + prepend_code_mode_executors(config, &mut executors); + build_model_visible_specs_and_registry(config, executors, hosted_model_tool_specs(config)) +} - let mut non_deferred_specs = Vec::new(); - let mut deferred_search_infos = Vec::new(); +fn build_model_visible_specs_and_registry( + config: &ToolsConfig, + executors: Vec>, + hosted_specs: Vec, +) -> (Vec, ToolRegistry) { + let mut specs = Vec::new(); + let mut seen_tool_names = HashSet::new(); for executor in &executors { - match executor.exposure() { - ToolExposure::Direct | ToolExposure::DirectModelOnly => { - if let Some(spec) = executor.spec() { - non_deferred_specs.push((spec, executor.exposure())); - } - } - ToolExposure::Deferred => { - if let Some(search_info) = executor.search_info() { - deferred_search_infos.push(search_info); - } - } + if !seen_tool_names.insert(executor.tool_name()) { + continue; + } + if executor.exposure().is_direct() + && let Some(spec) = executor.spec() + { + specs.push(spec_for_model_request(config, executor.exposure(), spec)); } } + specs.extend(hosted_specs); - non_deferred_specs.extend( - hosted_specs - .into_iter() - .map(|spec| (spec, ToolExposure::Direct)), - ); - - let non_deferred_specs = non_deferred_specs + let registry = ToolRegistry::from_tools(executors); + let model_visible_specs = merge_into_namespaces(specs) .into_iter() - .map(|(spec, exposure)| { - if config.code_mode_enabled && exposure != ToolExposure::DirectModelOnly { - codex_tools::augment_tool_spec_for_code_mode(spec) - } else { - spec - } - }) + .filter(|spec| config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_))) + .filter(|spec| !is_hidden_by_code_mode_only(config, ®istry, spec)) .collect(); - for spec in merge_into_namespaces(non_deferred_specs) { - if !config.namespace_tools && matches!(spec, ToolSpec::Namespace(_)) { - continue; - } - builder.push_spec(spec); - } - - for executor in executors { - builder.register_tool_without_spec(executor); - } + (model_visible_specs, registry) +} - if config.search_tool && config.namespace_tools && !deferred_search_infos.is_empty() { - builder.register_tool(Arc::new(ToolSearchHandler::new(deferred_search_infos))); +fn spec_for_model_request( + config: &ToolsConfig, + exposure: ToolExposure, + spec: ToolSpec, +) -> ToolSpec { + if config.code_mode_enabled + && exposure != ToolExposure::DirectModelOnly + && codex_code_mode::is_code_mode_nested_tool(spec.name()) + { + codex_tools::augment_tool_spec_for_code_mode(spec) + } else { + spec } - - builder } pub(crate) fn hosted_model_tool_specs(config: &ToolsConfig) -> Vec { @@ -147,11 +178,56 @@ pub(crate) fn hosted_model_tool_specs(config: &ToolsConfig) -> Vec { specs } +fn wait_agent_timeout_options(config: &ToolsConfig) -> WaitAgentTimeoutOptions { + if config.multi_agent_v2 { + return WaitAgentTimeoutOptions { + default_timeout_ms: config + .wait_agent_default_timeout_ms + .unwrap_or(DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS), + min_timeout_ms: config + .wait_agent_min_timeout_ms + .unwrap_or(DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS), + max_timeout_ms: config + .wait_agent_max_timeout_ms + .unwrap_or(DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS), + }; + } + + WaitAgentTimeoutOptions { + default_timeout_ms: DEFAULT_WAIT_TIMEOUT_MS, + min_timeout_ms: MIN_WAIT_TIMEOUT_MS, + max_timeout_ms: MAX_WAIT_TIMEOUT_MS, + } +} + +fn agent_type_description(config: &ToolsConfig, default_agent_type_description: &str) -> String { + if config.agent_type_description.is_empty() { + default_agent_type_description.to_string() + } else { + config.agent_type_description.clone() + } +} + +fn is_hidden_by_code_mode_only( + config: &ToolsConfig, + registry: &ToolRegistry, + spec: &ToolSpec, +) -> bool { + if !config.code_mode_only_enabled || !codex_code_mode::is_code_mode_nested_tool(spec.name()) { + return false; + } + + let exposure = registry + .tool_exposure(&ToolName::plain(spec.name())) + .unwrap_or(ToolExposure::Direct); + exposure != ToolExposure::DirectModelOnly +} + fn build_code_mode_executors( config: &ToolsConfig, - executors: &[Arc], + executors: &[Arc], deferred_tools_available: bool, -) -> Vec> { +) -> Vec> { if !config.code_mode_enabled { return vec![]; } @@ -254,12 +330,12 @@ fn code_mode_namespace_descriptions( namespace_descriptions } -pub(crate) fn collect_tool_executors( +fn collect_tool_executors( config: &ToolsConfig, params: ToolRegistryBuildParams<'_>, -) -> Vec> { +) -> Vec> { let exec_permission_approvals_enabled = config.exec_permission_approvals_enabled; - let mut executors = Vec::>::new(); + let mut executors = Vec::>::new(); if config.environment_mode.has_environment() { let include_environment_id = @@ -444,10 +520,43 @@ pub(crate) fn collect_tool_executors( executors } +fn append_tool_search_executor( + config: &ToolsConfig, + executors: &mut Vec>, +) { + if !(config.search_tool && config.namespace_tools) { + return; + } + + let search_infos = executors + .iter() + .filter(|executor| executor.exposure() == ToolExposure::Deferred) + .filter_map(|executor| executor.search_info()) + .collect::>(); + if search_infos.is_empty() { + return; + } + + executors.push(Arc::new(ToolSearchHandler::new(search_infos))); +} + +fn prepend_code_mode_executors( + config: &ToolsConfig, + executors: &mut Vec>, +) { + let deferred_tools_available = config.search_tool + && executors + .iter() + .any(|executor| executor.exposure() == ToolExposure::Deferred); + let code_mode_executors = + build_code_mode_executors(config, executors, deferred_tools_available); + executors.splice(0..0, code_mode_executors); +} + fn append_extension_tool_executors( config: &ToolsConfig, - executors: &[Arc], - registered_executors: &mut Vec>, + executors: &[Arc>], + registered_executors: &mut Vec>, ) { if executors.is_empty() { return; @@ -473,17 +582,17 @@ fn append_extension_tool_executors( for executor in executors.iter().cloned() { let tool_name = executor.tool_name(); if !reserved_tool_names.insert(tool_name.clone()) { - warn!("Skipping extension tool `{tool_name}`: handler already registered"); + warn!("Skipping extension tool `{tool_name}`: tool already registered"); continue; } - registered_executors.push(Arc::new(ExtensionToolHandler::new(executor))); + registered_executors.push(Arc::new(ExtensionToolAdapter::new(executor))); } } fn multi_agent_v2_handler( - handler: impl RegisteredTool + 'static, + handler: impl CoreToolRuntime + 'static, exposure: ToolExposure, -) -> Arc { +) -> Arc { override_tool_exposure(Arc::new(handler), exposure) } diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index dd690b5631c0..84e39e6acb4f 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -26,7 +26,6 @@ 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_extension_api::ToolExecutor; use codex_features::Feature; @@ -74,7 +73,10 @@ 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; -fn extension_tool_executor(name: &str, description: &str) -> Arc { +fn extension_tool_executor( + name: &str, + description: &str, +) -> Arc> { struct SpecOnlyExtensionExecutor { name: String, description: String, @@ -82,8 +84,6 @@ fn extension_tool_executor(name: &str, description: &str) -> Arc for SpecOnlyExtensionExecutor { - type Output = codex_tools::JsonToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(self.name.as_str()) } @@ -109,7 +109,7 @@ fn extension_tool_executor(name: &str, description: &str) -> Arc Result { + ) -> Result, codex_tools::FunctionCallError> { panic!("spec planning should not execute extension tools") } } @@ -1440,7 +1440,7 @@ fn namespace_specs_are_hidden_when_namespace_tools_are_disabled() { ); assert_lacks_tool_name(&tools, "mcp__sample__"); - assert!(registry.has_handler(&ToolName::namespaced("mcp__sample__", "echo"))); + assert!(registry.has_tool(&ToolName::namespaced("mcp__sample__", "echo"))); } #[test] @@ -1646,11 +1646,11 @@ fn search_tool_description_lists_each_mcp_source_once() { assert!(description.contains("- rmcp: Remote memory tools.")); assert!(!description.contains("mcp__rmcp__echo")); - assert!(registry.has_handler(&ToolName::namespaced( + assert!(registry.has_tool(&ToolName::namespaced( "mcp__codex_apps__calendar", "_create_event", ))); - assert!(registry.has_handler(&ToolName::namespaced("mcp__rmcp__", "echo"))); + assert!(registry.has_tool(&ToolName::namespaced("mcp__rmcp__", "echo"))); } #[test] @@ -1758,7 +1758,7 @@ fn no_search_tool_when_namespaces_disabled() { ); assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); - assert!(!registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); + assert!(!registry.has_tool(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); } #[test] @@ -1816,9 +1816,9 @@ fn search_tool_registers_for_deferred_dynamic_tools() { assert!(description.contains("- Dynamic tools: Tools provided by the current Codex thread.")); assert_contains_tool_names(&tools, &[TOOL_SEARCH_TOOL_NAME]); assert_lacks_tool_name(&tools, "codex_app"); - assert!(registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); - assert!(registry.has_handler(&ToolName::namespaced("codex_app", "automation_update"))); - assert!(registry.has_handler(&ToolName::namespaced("codex_app", "automation_list"))); + assert!(registry.has_tool(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); + assert!(registry.has_tool(&ToolName::namespaced("codex_app", "automation_update"))); + assert!(registry.has_tool(&ToolName::namespaced("codex_app", "automation_list"))); } #[test] @@ -1865,9 +1865,9 @@ fn search_tool_is_hidden_for_deferred_dynamic_tools_when_namespace_tools_are_dis assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); assert_lacks_tool_name(&tools, "codex_app"); assert_lacks_tool_name(&tools, "plain_dynamic"); - assert!(!registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); - assert!(registry.has_handler(&ToolName::namespaced("codex_app", "automation_update"))); - assert!(registry.has_handler(&ToolName::plain("plain_dynamic"))); + assert!(!registry.has_tool(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); + assert!(registry.has_tool(&ToolName::namespaced("codex_app", "automation_update"))); + assert!(registry.has_tool(&ToolName::plain("plain_dynamic"))); } #[test] @@ -2007,7 +2007,7 @@ fn request_plugin_install_description_lists_discoverable_tools() { /*extension_tool_executors*/ &[], &[], ); - assert!(registry.has_handler(&ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME))); + assert!(registry.has_tool(&ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME))); let request_plugin_install = find_tool(&tools, REQUEST_PLUGIN_INSTALL_TOOL_NAME); let ToolSpec::Function(ResponsesApiTool { @@ -2413,7 +2413,7 @@ fn build_specs_with_inputs_for_test( mcp_tools: Option>, deferred_mcp_tools: Option>, discoverable_tools: Option>, - extension_tool_executors: &[Arc], + extension_tool_executors: &[Arc>], dynamic_tools: &[DynamicToolSpec], ) -> (Vec, ToolRegistry) { let mcp_tool_inputs = mcp_tools.as_ref().map(|mcp_tools| { @@ -2431,13 +2431,10 @@ fn build_specs_with_inputs_for_test( default_agent_type_description: DEFAULT_AGENT_TYPE_DESCRIPTION, wait_agent_timeouts: wait_agent_timeout_options(), }; - let executors = collect_tool_executors(config, params); - let builder = build_tool_registry_builder_from_executors( - config, - executors, - hosted_model_tool_specs(config), - ); - builder.build() + let mut executors = collect_tool_executors(config, params); + append_tool_search_executor(config, &mut executors); + prepend_code_mode_executors(config, &mut executors); + build_model_visible_specs_and_registry(config, executors, hosted_model_tool_specs(config)) } fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> rmcp::model::Tool { diff --git a/codex-rs/core/src/tools/spec_plan_types.rs b/codex-rs/core/src/tools/spec_plan_types.rs deleted file mode 100644 index 247fa0571571..000000000000 --- a/codex-rs/core/src/tools/spec_plan_types.rs +++ /dev/null @@ -1,29 +0,0 @@ -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_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_executors: &'a [Arc], - pub dynamic_tools: &'a [DynamicToolSpec], - pub default_agent_type_description: &'a str, - pub wait_agent_timeouts: WaitAgentTimeoutOptions, -} - -pub(crate) fn agent_type_description( - config: &ToolsConfig, - default_agent_type_description: &str, -) -> String { - if config.agent_type_description.is_empty() { - default_agent_type_description.to_string() - } else { - config.agent_type_description.clone() - } -} diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index ca8739ccff4c..fb45f85d9cf2 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -3,17 +3,17 @@ use crate::shell::Shell; use crate::shell::ShellType; use crate::test_support::construct_model_info_offline; use crate::tools::ToolRouter; -use crate::tools::registry::ToolRegistryBuilder; use crate::tools::router::ToolRouterParams; -use crate::tools::spec_plan::build_tool_registry_builder_from_executors; use codex_app_server_protocol::AppInfo; use codex_features::Feature; use codex_features::Features; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_mcp::ToolInfo; use codex_models_manager::bundled_models_response; use codex_models_manager::model_info::with_config_overrides; use codex_protocol::config_types::WebSearchMode; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; @@ -56,21 +56,6 @@ fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> r } } -fn mcp_tool_info(tool: rmcp::model::Tool) -> ToolInfo { - ToolInfo { - server_name: "test_server".to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: tool.name.to_string(), - callable_namespace: "mcp__test_server__".to_string(), - namespace_description: None, - tool, - connector_id: None, - connector_name: None, - plugin_display_names: Vec::new(), - } -} - fn mcp_tool_info_with_display_name(display_name: &str, tool: rmcp::model::Tool) -> ToolInfo { let (callable_namespace, callable_name) = display_name .rsplit_once('/') @@ -240,13 +225,12 @@ async fn multi_agent_v2_tools_config() -> ToolsConfig { } fn multi_agent_v2_spawn_agent_description(tools_config: &ToolsConfig) -> String { - let (tools, _) = build_specs( + let tools = build_specs( tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let spawn_agent = find_tool(&tools, "spawn_agent"); let ToolSpec::Function(ResponsesApiTool { description, .. }) = spawn_agent else { panic!("spawn_agent should be a function tool"); @@ -266,13 +250,13 @@ async fn model_info_from_models_json(slug: &str) -> ModelInfo { with_config_overrides(model, &config.to_models_manager_config()) } -/// Builds the tool registry builder while collecting tool specs for later serialization. +/// Builds tool specs and the matching registry from the same executor list. fn build_specs( config: &ToolsConfig, mcp_tools: Option>, deferred_mcp_tools: Option>, dynamic_tools: &[DynamicToolSpec], -) -> ToolRegistryBuilder { +) -> Vec { build_specs_with_inputs_for_test( config, mcp_tools, @@ -288,18 +272,22 @@ fn build_specs_with_inputs_for_test( mcp_tools: Option>, deferred_mcp_tools: Option>, discoverable_tools: Option>, - extension_tool_executors: &[Arc], + extension_tool_executors: &[Arc< + dyn codex_extension_api::ToolExecutor, + >], dynamic_tools: &[DynamicToolSpec], -) -> ToolRegistryBuilder { - let parts = collect_tool_router_parts( +) -> Vec { + ToolRouter::from_config( config, - mcp_tools, - deferred_mcp_tools, - discoverable_tools, - extension_tool_executors, - dynamic_tools, - ); - build_tool_registry_builder_from_executors(config, parts.executors, parts.hosted_specs) + ToolRouterParams { + mcp_tools, + deferred_mcp_tools, + discoverable_tools, + extension_tool_executors: extension_tool_executors.to_vec(), + dynamic_tools, + }, + ) + .model_visible_specs() } #[tokio::test] @@ -319,13 +307,12 @@ async fn get_memory_requires_feature_flag() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); assert!( !tools.iter().any(|t| t.name() == "get_memory"), "get_memory should be disabled when memory_tool feature is off" @@ -633,13 +620,12 @@ async fn test_build_specs_default_shell_present() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(Vec::new()), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); // Only check the shell variant and a couple of core tools. let mut subset = vec!["exec_command", "write_stdin", "update_plan"]; @@ -774,13 +760,12 @@ async fn multi_agent_v2_wait_agent_schema_uses_configured_timeouts() { .with_wait_agent_min_timeout_ms(wait_agent_min_timeout_ms) .with_wait_agent_max_timeout_ms(wait_agent_max_timeout_ms) .with_wait_agent_default_timeout_ms(wait_agent_default_timeout_ms); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let wait_agent = find_tool(&tools, "wait_agent"); let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = wait_agent else { panic!("wait_agent should be a function tool"); @@ -825,15 +810,14 @@ async fn request_plugin_install_requires_apps_and_plugins_features() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs_with_inputs_for_test( + let tools = build_specs_with_inputs_for_test( &tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, discoverable_tools.clone(), /*extension_tool_executors*/ &[], &[], - ) - .build(); + ); assert!( !tools @@ -862,13 +846,12 @@ async fn search_tool_is_hidden_without_deferred_tools() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, /*mcp_tools*/ None, Some(Vec::new()), &[], - ) - .build(); + ); assert!( !tools .iter() @@ -894,7 +877,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, /*mcp_tools*/ None, Some(vec![ToolInfo { @@ -914,8 +897,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio plugin_display_names: Vec::new(), }]), &[], - ) - .build(); + ); let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME); let ToolSpec::ToolSearch { description, .. } = search_tool else { panic!("expected tool_search tool"); @@ -925,119 +907,6 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio assert!(!description.contains("- Calendar:")); } -#[tokio::test] -async fn search_tool_registers_namespaced_mcp_tool_aliases() { - let model_info = search_capable_model_info().await; - let mut features = Features::with_defaults(); - features.enable(Feature::Apps); - features.enable(Feature::ToolSearch); - 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 (_, registry) = build_specs( - &tools_config, - /*mcp_tools*/ None, - Some(vec![ - ToolInfo { - server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: "_create_event".to_string(), - callable_namespace: "mcp__codex_apps__calendar".to_string(), - namespace_description: None, - tool: mcp_tool( - "calendar-create-event", - "Create calendar event", - serde_json::json!({"type": "object"}), - ), - connector_id: Some("calendar".to_string()), - connector_name: Some("Calendar".to_string()), - plugin_display_names: Vec::new(), - }, - ToolInfo { - server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: "_list_events".to_string(), - callable_namespace: "mcp__codex_apps__calendar".to_string(), - namespace_description: None, - tool: mcp_tool( - "calendar-list-events", - "List calendar events", - serde_json::json!({"type": "object"}), - ), - connector_id: Some("calendar".to_string()), - connector_name: Some("Calendar".to_string()), - plugin_display_names: Vec::new(), - }, - ToolInfo { - server_name: "rmcp".to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: "echo".to_string(), - callable_namespace: "mcp__rmcp__".to_string(), - namespace_description: None, - tool: mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})), - connector_id: None, - connector_name: None, - plugin_display_names: Vec::new(), - }, - ]), - &[], - ) - .build(); - - let app_alias = ToolName::namespaced("mcp__codex_apps__calendar", "_create_event"); - let mcp_alias = ToolName::namespaced("mcp__rmcp__", "echo"); - - assert!(registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); - assert!(registry.has_handler(&app_alias)); - assert!(registry.has_handler(&mcp_alias)); -} - -#[tokio::test] -async fn direct_mcp_tools_register_namespaced_handlers() { - let config = test_config().await; - let model_info = construct_model_info_offline("gpt-5.4", &config); - let mut features = Features::with_defaults(); - features.enable(Feature::UnifiedExec); - 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 (_, registry) = build_specs( - &tools_config, - Some(vec![mcp_tool_info(mcp_tool( - "echo", - "Echo", - serde_json::json!({"type": "object"}), - ))]), - /*deferred_mcp_tools*/ None, - &[], - ) - .build(); - - assert!(registry.has_handler(&ToolName::namespaced("mcp__test_server__", "echo"))); - assert!(!registry.has_handler(&ToolName::plain("mcp__test_server__echo"))); -} - #[tokio::test] async fn test_mcp_tool_property_missing_type_defaults_to_string() { let config = test_config().await; @@ -1056,7 +925,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/search", @@ -1073,8 +942,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "search"); assert_eq!( @@ -1116,7 +984,7 @@ async fn test_mcp_tool_preserves_integer_schema() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/paginate", @@ -1131,8 +999,7 @@ async fn test_mcp_tool_preserves_integer_schema() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "paginate"); assert_eq!( @@ -1175,7 +1042,7 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/tags", @@ -1190,8 +1057,7 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "tags"); assert_eq!( @@ -1236,7 +1102,7 @@ async fn test_mcp_tool_anyof_defaults_to_string() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/value", @@ -1253,8 +1119,7 @@ async fn test_mcp_tool_anyof_defaults_to_string() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "value"); assert_eq!( @@ -1301,7 +1166,7 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "test_server/do_something_cool", @@ -1335,8 +1200,7 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "test_server/", "do_something_cool"); assert_eq!( diff --git a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs index cb0eff9ca938..bccaf60c829d 100644 --- a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs +++ b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs @@ -21,8 +21,8 @@ use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolRegistry; use crate::turn_diff_tracker::TurnDiffTracker; @@ -32,18 +32,22 @@ struct TestHandler { #[async_trait::async_trait] impl ToolExecutor for TestHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> codex_tools::ToolName { self.tool_name.clone() } - async fn handle(&self, _invocation: ToolInvocation) -> Result { - Ok(FunctionToolOutput::from_text("ok".to_string(), Some(true))) + async fn handle( + &self, + _invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + Ok(Box::new(FunctionToolOutput::from_text( + "ok".to_string(), + Some(true), + ))) } } -impl ToolHandler for TestHandler {} +impl CoreToolRuntime for TestHandler {} #[tokio::test] async fn dispatch_lifecycle_trace_records_direct_and_code_mode_requesters() -> anyhow::Result<()> { diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index ed9665f228f9..a8c200120011 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -4,12 +4,13 @@ use std::sync::Arc; use codex_protocol::items::TurnItem; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::TokenUsageInfo; +use codex_tools::ToolCall; +use codex_tools::ToolExecutor; use crate::ExtensionData; mod prompt; mod thread_lifecycle; -mod tools; mod turn_lifecycle; pub use prompt::PromptFragment; @@ -17,8 +18,6 @@ pub use prompt::PromptSlot; pub use thread_lifecycle::ThreadResumeInput; pub use thread_lifecycle::ThreadStartInput; pub use thread_lifecycle::ThreadStopInput; -pub use tools::ExtensionToolExecutor; -pub use tools::ExtensionToolOutput; pub use turn_lifecycle::TurnAbortInput; pub use turn_lifecycle::TurnStartInput; pub use turn_lifecycle::TurnStopInput; @@ -106,7 +105,7 @@ pub trait ToolContributor: Send + Sync { &self, session_store: &ExtensionData, thread_store: &ExtensionData, - ) -> Vec>; + ) -> 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 deleted file mode 100644 index a9f0f6dc3218..000000000000 --- a/codex-rs/ext/extension-api/src/contributors/tools.rs +++ /dev/null @@ -1,15 +0,0 @@ -use codex_tools::JsonToolOutput; -use codex_tools::ToolCall; -use codex_tools::ToolExecutor; - -/// Model-facing output returned by extension-owned tools. -pub type ExtensionToolOutput = JsonToolOutput; - -/// Thin alias for extension-owned executable tools. -/// -/// Extensions implement the shared `ToolExecutor` contract directly; -/// the marker keeps contributor signatures readable while preserving one -/// executable-tool abstraction across host and extension tools. -pub trait ExtensionToolExecutor: ToolExecutor {} - -impl ExtensionToolExecutor for T where T: ToolExecutor {} diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index a6d850468b8c..e8be7e030928 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -11,6 +11,7 @@ pub use codex_tools::ResponsesApiTool; pub use codex_tools::ToolCall; pub use codex_tools::ToolExecutor; pub use codex_tools::ToolName; +pub use codex_tools::ToolOutput; pub use codex_tools::ToolPayload; pub use codex_tools::ToolSpec; pub use codex_tools::parse_tool_input_schema; @@ -18,8 +19,6 @@ pub use contributors::ApprovalReviewContributor; pub use contributors::ApprovalReviewFuture; pub use contributors::ConfigContributor; pub use contributors::ContextContributor; -pub use contributors::ExtensionToolExecutor; -pub use contributors::ExtensionToolOutput; pub use contributors::PromptFragment; pub use contributors::PromptSlot; pub use contributors::ThreadLifecycleContributor; diff --git a/codex-rs/ext/memories/src/extension.rs b/codex-rs/ext/memories/src/extension.rs index 2b4955918340..9226ddfdccc9 100644 --- a/codex-rs/ext/memories/src/extension.rs +++ b/codex-rs/ext/memories/src/extension.rs @@ -63,7 +63,7 @@ impl ToolContributor for MemoriesExtension { &self, _session_store: &ExtensionData, thread_store: &ExtensionData, - ) -> Vec> { + ) -> Vec>> { let Some(config) = thread_store.get::() else { return Vec::new(); }; diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index 47c58f468684..94ccfc1f0f46 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -3,10 +3,10 @@ use std::sync::Arc; use codex_extension_api::ContextContributor; use codex_extension_api::ExtensionData; -use codex_extension_api::ExtensionToolExecutor; use codex_extension_api::PromptSlot; use codex_extension_api::ToolCall; use codex_extension_api::ToolContributor; +use codex_extension_api::ToolExecutor; use codex_extension_api::ToolName; use codex_extension_api::ToolPayload; use codex_tools::ToolOutput; @@ -303,7 +303,7 @@ async fn search_tool_rejects_legacy_single_query() { assert!(err.to_string().contains("query")); } -fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc { +fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc> { let expected_tool_name = memory_tool_name(tool_name); crate::tools::memory_tools(LocalMemoriesBackend::from_memory_root(memory_root)) .into_iter() diff --git a/codex-rs/ext/memories/src/tools/list.rs b/codex-rs/ext/memories/src/tools/list.rs index 1509c2344c16..baed3c30a234 100644 --- a/codex-rs/ext/memories/src/tools/list.rs +++ b/codex-rs/ext/memories/src/tools/list.rs @@ -39,8 +39,6 @@ impl ToolExecutor for ListTool where B: MemoriesBackend, { - type Output = JsonToolOutput; - fn tool_name(&self) -> ToolName { memory_tool_name(LIST_TOOL_NAME) } @@ -55,7 +53,8 @@ where async fn handle( &self, call: ToolCall, - ) -> Result { + ) -> Result, codex_extension_api::FunctionCallError> + { let backend = self.backend.clone(); let args: ListArgs = parse_args(&call)?; let response = backend @@ -70,6 +69,6 @@ where }) .await .map_err(backend_error_to_function_call)?; - Ok(JsonToolOutput::new(json!(response))) + Ok(Box::new(JsonToolOutput::new(json!(response)))) } } diff --git a/codex-rs/ext/memories/src/tools/mod.rs b/codex-rs/ext/memories/src/tools/mod.rs index f95922b28f52..b78a12565269 100644 --- a/codex-rs/ext/memories/src/tools/mod.rs +++ b/codex-rs/ext/memories/src/tools/mod.rs @@ -1,9 +1,9 @@ use std::sync::Arc; -use codex_extension_api::ExtensionToolExecutor; use codex_extension_api::FunctionCallError; use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall; +use codex_extension_api::ToolExecutor; use codex_extension_api::ToolName; use codex_extension_api::ToolSpec; use codex_extension_api::parse_tool_input_schema; @@ -23,7 +23,7 @@ mod list; mod read; mod search; -pub(crate) fn memory_tools(backend: B) -> Vec> +pub(crate) fn memory_tools(backend: B) -> Vec>> where B: MemoriesBackend, { diff --git a/codex-rs/ext/memories/src/tools/read.rs b/codex-rs/ext/memories/src/tools/read.rs index 06d8a1752ca9..6e7f771d756b 100644 --- a/codex-rs/ext/memories/src/tools/read.rs +++ b/codex-rs/ext/memories/src/tools/read.rs @@ -38,8 +38,6 @@ impl ToolExecutor for ReadTool where B: MemoriesBackend, { - type Output = JsonToolOutput; - fn tool_name(&self) -> ToolName { memory_tool_name(READ_TOOL_NAME) } @@ -54,7 +52,8 @@ where async fn handle( &self, call: ToolCall, - ) -> Result { + ) -> Result, codex_extension_api::FunctionCallError> + { let backend = self.backend.clone(); let args: ReadArgs = parse_args(&call)?; let response = backend @@ -66,6 +65,6 @@ where }) .await .map_err(backend_error_to_function_call)?; - Ok(JsonToolOutput::new(json!(response))) + Ok(Box::new(JsonToolOutput::new(json!(response)))) } } diff --git a/codex-rs/ext/memories/src/tools/search.rs b/codex-rs/ext/memories/src/tools/search.rs index f7cab7de6cc1..422ce06b7e19 100644 --- a/codex-rs/ext/memories/src/tools/search.rs +++ b/codex-rs/ext/memories/src/tools/search.rs @@ -47,8 +47,6 @@ impl ToolExecutor for SearchTool where B: MemoriesBackend, { - type Output = JsonToolOutput; - fn tool_name(&self) -> ToolName { memory_tool_name(SEARCH_TOOL_NAME) } @@ -63,14 +61,15 @@ where async fn handle( &self, call: ToolCall, - ) -> Result { + ) -> Result, codex_extension_api::FunctionCallError> + { let backend = self.backend.clone(); let args: SearchArgs = parse_args(&call)?; let response = backend .search(args.into_request()) .await .map_err(backend_error_to_function_call)?; - Ok(JsonToolOutput::new(json!(response))) + Ok(Box::new(JsonToolOutput::new(json!(response)))) } } diff --git a/codex-rs/tools/src/tool_executor.rs b/codex-rs/tools/src/tool_executor.rs index 62237740babd..2557dc349006 100644 --- a/codex-rs/tools/src/tool_executor.rs +++ b/codex-rs/tools/src/tool_executor.rs @@ -36,8 +36,6 @@ impl ToolExposure { /// top without reopening the spec/runtime split. #[async_trait::async_trait] pub trait ToolExecutor: Send + Sync { - type Output: ToolOutput + 'static; - /// The concrete tool name handled by this runtime instance. fn tool_name(&self) -> ToolName; @@ -53,5 +51,8 @@ pub trait ToolExecutor: Send + Sync { false } - async fn handle(&self, invocation: Invocation) -> Result; + async fn handle( + &self, + invocation: Invocation, + ) -> Result, FunctionCallError>; } diff --git a/codex-rs/tools/src/tool_output.rs b/codex-rs/tools/src/tool_output.rs index 2044295174b7..64a8164da36c 100644 --- a/codex-rs/tools/src/tool_output.rs +++ b/codex-rs/tools/src/tool_output.rs @@ -20,6 +20,16 @@ pub trait ToolOutput: Send { fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem; + /// Returns the tool call id exposed to `PostToolUse` hooks for this output. + fn post_tool_use_id(&self, call_id: &str) -> String { + call_id.to_string() + } + + /// Returns the tool input exposed to `PostToolUse` hooks for this output. + fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { + None + } + /// Returns the stable value exposed to `PostToolUse` hooks for this tool output. /// /// Tool handlers decide whether a tool participates in `PostToolUse`, but @@ -52,6 +62,14 @@ where (**self).to_response_item(call_id, payload) } + fn post_tool_use_id(&self, call_id: &str) -> String { + (**self).post_tool_use_id(call_id) + } + + fn post_tool_use_input(&self, payload: &ToolPayload) -> Option { + (**self).post_tool_use_input(payload) + } + fn post_tool_use_response(&self, call_id: &str, payload: &ToolPayload) -> Option { (**self).post_tool_use_response(call_id, payload) } From 8147a26b8caf91659533eddc331d0e8548358b5d Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 14 May 2026 13:24:50 +0100 Subject: [PATCH 2/3] fix simplify tools CI --- codex-rs/core/src/tools/registry.rs | 2 +- codex-rs/core/src/tools/spec_plan_tests.rs | 26 ++++++++++++---------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 0575691900d2..1cf0e00623c8 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -36,7 +36,7 @@ pub use codex_tools::ToolExposure; /// Typed runtime contract for locally executed tools. /// -/// Implementors provide the shared `ToolExecutor` behavior plus optional +/// Implementers provide the shared `ToolExecutor` behavior plus optional /// core-owned metadata for hooks, telemetry, tool search, and argument diffs. pub(crate) trait CoreToolRuntime: ToolExecutor { fn search_info(&self) -> Option { diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 84e39e6acb4f..063d75e6e8ca 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -2109,18 +2109,19 @@ fn code_mode_augments_mcp_tool_descriptions_with_namespaced_sample() { &[], ); - let ResponsesApiTool { description, .. } = - find_namespace_function_tool(&tools, "mcp__sample__", "echo"); + let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { + panic!("expected freeform tool"); + }; - assert_eq!( - description, - r#"Echo text + assert!(description.contains( + r#"### `mcp__sample__echo` +Echo text exec tool declaration: ```ts declare const tools: { mcp__sample__echo(args: { message: string; }): Promise; }; ```"# - ); + )); } #[test] @@ -2536,18 +2537,19 @@ fn code_mode_augments_mcp_tool_descriptions_with_structured_output_sample() { &[], ); - let ResponsesApiTool { description, .. } = - find_namespace_function_tool(&tools, "mcp__sample__", "echo"); + let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { + panic!("expected freeform tool"); + }; - assert_eq!( - description, - r#"Echo text + assert!(description.contains( + r#"### `mcp__sample__echo` +Echo text exec tool declaration: ```ts declare const tools: { mcp__sample__echo(args: { message: string; }): Promise>; }; ```"# - ); + )); } fn discoverable_connector(id: &str, name: &str, description: &str) -> DiscoverableTool { From 3a98816bbff814c0ef2ac6772d50b68aa7024425 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 14 May 2026 13:42:57 +0100 Subject: [PATCH 3/3] fix memories bazel test --- codex-rs/ext/memories/src/tests.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index 94ccfc1f0f46..b732ca22c2d5 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -290,14 +290,17 @@ async fn search_tool_rejects_legacy_single_query() { .to_string(), }; - let err = tool + let result = tool .handle(ToolCall { call_id: "call-1".to_string(), tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), payload, }) - .await - .expect_err("legacy query field should be rejected"); + .await; + let err = match result { + Ok(_) => panic!("legacy query field should be rejected"), + Err(err) => err, + }; assert!(err.to_string().contains("unknown field")); assert!(err.to_string().contains("query"));