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 80a72a88e..8eb1697fc 100644 --- a/codex-rs/core/src/tools/code_mode/execute_handler.rs +++ b/codex-rs/core/src/tools/code_mode/execute_handler.rs @@ -8,17 +8,20 @@ use codex_tools::ToolSpec; use super::ExecContext; use super::PUBLIC_TOOL_NAME; -use super::build_enabled_tools; use super::handle_runtime_response; use super::is_exec_tool_name; pub struct CodeModeExecuteHandler { spec: ToolSpec, + nested_tool_specs: Vec, } impl CodeModeExecuteHandler { - pub(crate) fn new(spec: ToolSpec) -> Self { - Self { spec } + pub(crate) fn new(spec: ToolSpec, nested_tool_specs: Vec) -> Self { + Self { + spec, + nested_tool_specs, + } } async fn execute( @@ -31,7 +34,8 @@ impl CodeModeExecuteHandler { let args = codex_code_mode::parse_exec_source(&code).map_err(FunctionCallError::RespondToModel)?; let exec = ExecContext { session, turn }; - let enabled_tools = build_enabled_tools(&exec).await; + let enabled_tools = + codex_tools::collect_code_mode_tool_definitions(&self.nested_tool_specs); let stored_values = exec .session .services diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index e82faf695..ff9f8c889 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -29,12 +29,9 @@ use crate::tools::context::ToolPayload; use crate::tools::parallel::ToolCallRuntime; use crate::tools::router::ToolCall; use crate::tools::router::ToolCallSource; -use crate::tools::router::ToolRouterParams; -use crate::tools::router::extension_tool_bundles; use crate::unified_exec::resolve_max_tokens; use codex_features::Feature; use codex_tools::ToolName; -use codex_tools::collect_code_mode_tool_definitions; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::formatted_truncate_text_content_items_with_policy; use codex_utils_output_truncation::truncate_function_output_items_with_policy; @@ -260,36 +257,6 @@ fn truncate_code_mode_result( truncate_function_output_items_with_policy(&items, policy) } -pub(super) async fn build_enabled_tools( - exec: &ExecContext, -) -> Vec { - let router = build_nested_router(exec).await; - let specs = router.specs(); - collect_code_mode_tool_definitions(&specs) -} - -#[expect( - clippy::await_holding_invalid_type, - reason = "nested tool router construction reads through the session-owned manager guard" -)] -async fn build_nested_router(exec: &ExecContext) -> ToolRouter { - let nested_tools_config = exec.turn.tools_config.for_code_mode_nested_tools(); - let mcp_connection_manager = exec.session.services.mcp_connection_manager.read().await; - let listed_mcp_tools = mcp_connection_manager.list_all_tools().await; - - ToolRouter::from_config( - &nested_tools_config, - ToolRouterParams { - deferred_mcp_tools: None, - mcp_tools: Some(listed_mcp_tools), - unavailable_called_tools: Vec::new(), - discoverable_tools: None, - extension_tool_bundles: extension_tool_bundles(exec.session.as_ref()), - dynamic_tools: exec.turn.dynamic_tools.as_slice(), - }, - ) -} - async fn call_nested_tool( _exec: ExecContext, tool_runtime: ToolCallRuntime, diff --git a/codex-rs/core/src/tools/handlers/dynamic.rs b/codex-rs/core/src/tools/handlers/dynamic.rs index 7cc9e61e3..9ecdf2340 100644 --- a/codex-rs/core/src/tools/handlers/dynamic.rs +++ b/codex-rs/core/src/tools/handlers/dynamic.rs @@ -9,10 +9,13 @@ use crate::tools::registry::ToolHandler; use crate::turn_timing::now_unix_timestamp_ms; use codex_protocol::dynamic_tools::DynamicToolCallRequest; use codex_protocol::dynamic_tools::DynamicToolResponse; +use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::protocol::DynamicToolCallResponseEvent; use codex_protocol::protocol::EventMsg; use codex_tools::ToolName; +use codex_tools::ToolSpec; +use codex_tools::dynamic_tool_to_loadable_tool_spec; use serde_json::Value; use std::time::Instant; use tokio::sync::oneshot; @@ -20,11 +23,17 @@ use tracing::warn; pub struct DynamicToolHandler { tool_name: ToolName, + spec: Option, } impl DynamicToolHandler { - pub fn new(tool_name: ToolName) -> Self { - Self { tool_name } + pub fn new(tool: &DynamicToolSpec) -> Option { + let tool_name = ToolName::new(tool.namespace.clone(), tool.name.clone()); + let spec = dynamic_tool_to_loadable_tool_spec(tool).ok()?.into(); + Some(Self { + tool_name, + spec: Some(spec), + }) } } @@ -35,6 +44,10 @@ impl ToolHandler for DynamicToolHandler { self.tool_name.clone() } + fn spec(&self) -> Option { + self.spec.clone() + } + async fn handle(&self, invocation: ToolInvocation) -> Result { let ToolInvocation { session, diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 8bb833733..89666c80a 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -14,7 +14,11 @@ use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolTelemetryTags; use codex_mcp::ToolInfo; +use codex_tools::ResponsesApiNamespace; +use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolName; +use codex_tools::ToolSpec; +use codex_tools::mcp_tool_to_responses_api_tool; use serde_json::Map; use serde_json::Value; @@ -35,6 +39,26 @@ impl ToolHandler for McpHandler { self.tool_info.canonical_tool_name() } + fn spec(&self) -> Option { + let tool_name = self.tool_name(); + let namespace_name = tool_name.namespace.as_ref()?; + let tool = mcp_tool_to_responses_api_tool(&tool_name, &self.tool_info.tool).ok()?; + let description = self + .tool_info + .namespace_description + .as_deref() + .map(str::trim) + .filter(|description| !description.is_empty()) + .map(str::to_string) + .unwrap_or_default(); + + Some(ToolSpec::Namespace(ResponsesApiNamespace { + name: namespace_name.clone(), + description, + tools: vec![ResponsesApiNamespaceTool::Function(tool)], + })) + } + fn supports_parallel_tool_calls(&self) -> bool { self.tool_info.supports_parallel_tool_calls } diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 01b3caa34..95e7d17ce 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -168,7 +168,11 @@ pub(crate) struct PostToolUsePayload { pub(crate) tool_response: Value, } -trait AnyToolHandler: Send + Sync { +pub(crate) trait AnyToolHandler: Send + Sync { + fn tool_name(&self) -> ToolName; + + fn spec(&self) -> Option; + fn supports_parallel_tool_calls(&self) -> bool; fn matches_kind(&self, payload: &ToolPayload) -> bool; @@ -197,6 +201,14 @@ impl AnyToolHandler for T where T: ToolHandler, { + fn tool_name(&self) -> ToolName { + ToolHandler::tool_name(self) + } + + fn spec(&self) -> Option { + ToolHandler::spec(self) + } + fn supports_parallel_tool_calls(&self) -> bool { ToolHandler::supports_parallel_tool_calls(self) } @@ -531,24 +543,17 @@ impl ToolRegistry { pub struct ToolRegistryBuilder { handlers: HashMap>, specs: Vec, - code_mode_enabled: bool, } impl ToolRegistryBuilder { - pub fn new(code_mode_enabled: bool) -> Self { + pub fn new() -> Self { Self { handlers: HashMap::new(), specs: Vec::new(), - code_mode_enabled, } } pub(crate) fn push_spec(&mut self, spec: ToolSpec) { - let spec = if self.code_mode_enabled { - codex_tools::augment_tool_spec_for_code_mode(spec) - } else { - spec - }; self.specs.push(spec); } @@ -556,17 +561,32 @@ impl ToolRegistryBuilder { where H: ToolHandler + 'static, { + self.register_any_handler(handler); + } + + pub(crate) fn register_any_handler(&mut self, handler: Arc) { + self.register_any_handler_internal(handler, /*include_spec*/ true); + } + + pub(crate) fn register_any_handler_without_spec(&mut self, handler: Arc) { + self.register_any_handler_internal(handler, /*include_spec*/ false); + } + + fn register_any_handler_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 let Some(spec) = handler.spec() { + if include_spec && let Some(spec) = handler.spec() { self.push_spec(spec); } - let handler: Arc = handler; self.handlers.insert(name, handler); } diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index d6acb80fb..dc744321d 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -63,16 +63,13 @@ fn handler_looks_up_namespaced_aliases_explicitly() { } #[test] -fn register_handler_adds_handler_and_augments_specs_for_code_mode() { - let mut builder = ToolRegistryBuilder::new(/*code_mode_enabled*/ true); +fn register_handler_adds_handler_and_spec() { + let mut builder = ToolRegistryBuilder::new(); builder.register_handler(Arc::new(GetGoalHandler)); let (specs, registry) = builder.build(); assert_eq!(specs.len(), 1); - assert_eq!( - specs[0], - codex_tools::augment_tool_spec_for_code_mode(create_get_goal_tool()) - ); + 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 aed72186e..4bc366bf9 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -37,7 +37,6 @@ pub struct ToolCall { pub struct ToolRouter { registry: ToolRegistry, - specs: Vec, model_visible_specs: Vec, } @@ -90,15 +89,10 @@ impl ToolRouter { Self { registry, - specs, model_visible_specs, } } - pub fn specs(&self) -> Vec { - self.specs.clone() - } - pub fn model_visible_specs(&self) -> Vec { self.model_visible_specs.clone() } diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 76776b1ad..4d6119cd5 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -232,7 +232,7 @@ async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()> } #[tokio::test] -async fn model_visible_specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> { +async fn specs_filter_deferred_dynamic_tools() -> anyhow::Result<()> { let (_, turn) = make_session_and_context().await; let hidden_tool = "hidden_dynamic_tool"; let visible_tool = "visible_dynamic_tool"; @@ -273,10 +273,6 @@ async fn model_visible_specs_filter_deferred_dynamic_tools() -> anyhow::Result<( }, ); - assert_eq!( - namespace_function_names(&router.specs(), "codex_app"), - vec![hidden_tool.to_string(), visible_tool.to_string()] - ); assert_eq!( namespace_function_names(&router.model_visible_specs(), "codex_app"), vec![visible_tool.to_string()] @@ -334,13 +330,6 @@ async fn extension_tool_bundles_are_model_visible_and_dispatchable() -> anyhow:: }, ); - assert!( - router - .specs() - .iter() - .any(|spec| spec.name() == "extension_echo"), - "expected extension-provided tool spec to be registered" - ); assert!( router .model_visible_specs() diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index fc21ff337..8c54c2147 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -70,7 +70,6 @@ pub(crate) fn build_specs_with_discoverable_tools( use crate::tools::tool_search_entry::build_tool_search_entries_for_config; let mcp_tool_plan_inputs = mcp_tools.as_deref().map(map_mcp_tools_for_plan); - let deferred_mcp_tool_sources = deferred_mcp_tools.as_deref(); let default_agent_type_description = crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); let min_wait_timeout_ms = if config.multi_agent_v2 { @@ -85,12 +84,12 @@ pub(crate) fn build_specs_with_discoverable_tools( DEFAULT_WAIT_TIMEOUT_MS.clamp(min_wait_timeout_ms, MAX_WAIT_TIMEOUT_MS); let deferred_dynamic_tools = dynamic_tools .iter() - .filter(|tool| tool.defer_loading && (config.namespace_tools || tool.namespace.is_none())) + .filter(|tool| tool.defer_loading) .cloned() .collect::>(); let tool_search_entries = build_tool_search_entries_for_config( config, - deferred_mcp_tool_sources, + deferred_mcp_tools.as_deref(), &deferred_dynamic_tools, ); let mut builder = build_tool_registry_builder( @@ -99,7 +98,7 @@ pub(crate) fn build_specs_with_discoverable_tools( mcp_tools: mcp_tool_plan_inputs .as_ref() .map(|inputs| inputs.mcp_tools.as_slice()), - deferred_mcp_tools: deferred_mcp_tool_sources, + deferred_mcp_tools: deferred_mcp_tools.as_deref(), tool_namespaces: mcp_tool_plan_inputs .as_ref() .map(|inputs| &inputs.tool_namespaces), diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 69456a36e..7232dddd8 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -27,6 +27,7 @@ use crate::tools::handlers::ViewImageHandler; use crate::tools::handlers::WriteStdinHandler; use crate::tools::handlers::agent_jobs::ReportAgentJobResultHandler; use crate::tools::handlers::agent_jobs::SpawnAgentsOnCsvHandler; +use crate::tools::handlers::extension_tools::extension_tool_spec; use crate::tools::handlers::multi_agents::CloseAgentHandler; use crate::tools::handlers::multi_agents::ResumeAgentHandler; use crate::tools::handlers::multi_agents::SendInputHandler; @@ -44,12 +45,11 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::hosted_spec::WebSearchToolOptions; use crate::tools::hosted_spec::create_image_generation_tool; use crate::tools::hosted_spec::create_web_search_tool; +use crate::tools::registry::AnyToolHandler; use crate::tools::registry::ToolRegistryBuilder; use crate::tools::spec_plan_types::ToolRegistryBuildParams; use crate::tools::spec_plan_types::agent_type_description; -use codex_mcp::ToolInfo; use codex_protocol::openai_models::ConfigShellToolType; -use codex_tools::ResponsesApiNamespace; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolEnvironmentMode; use codex_tools::ToolName; @@ -57,12 +57,9 @@ use codex_tools::ToolSearchSource; use codex_tools::ToolSearchSourceInfo; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; -use codex_tools::coalesce_loadable_tool_specs; use codex_tools::collect_code_mode_exec_prompt_tool_definitions; use codex_tools::collect_tool_search_source_infos; use codex_tools::default_namespace_description; -use codex_tools::dynamic_tool_to_loadable_tool_spec; -use codex_tools::mcp_tool_to_responses_api_tool; use std::collections::BTreeMap; use std::collections::HashSet; use std::sync::Arc; @@ -71,8 +68,21 @@ pub fn build_tool_registry_builder( config: &ToolsConfig, params: ToolRegistryBuildParams<'_>, ) -> ToolRegistryBuilder { - let mut builder = ToolRegistryBuilder::new(config.code_mode_enabled); - let exec_permission_approvals_enabled = config.exec_permission_approvals_enabled; + let mut builder = ToolRegistryBuilder::new(); + let all_deferred_tools = params + .deferred_mcp_tools + .into_iter() + .flatten() + .map(codex_mcp::ToolInfo::canonical_tool_name) + .chain( + params + .dynamic_tools + .iter() + .filter(|tool| tool.defer_loading) + .map(|tool| ToolName::new(tool.namespace.clone(), tool.name.clone())), + ) + .collect::>(); + let handlers = collect_handler_tools(config, params); if config.code_mode_enabled { let namespace_descriptions = params @@ -89,16 +99,18 @@ pub fn build_tool_registry_builder( ) }) .collect::>(); - let nested_config = config.for_code_mode_nested_tools(); - let nested_builder = build_tool_registry_builder( - &nested_config, - ToolRegistryBuildParams { - discoverable_tools: None, - ..params - }, + let mut code_mode_nested_tool_specs = handlers + .iter() + .filter_map(|handler| handler.spec()) + .collect::>(); + code_mode_nested_tool_specs.extend( + params + .extension_tool_bundles + .iter() + .filter_map(|bundle| extension_tool_spec(bundle.spec()).ok()), ); let mut enabled_tools = - collect_code_mode_exec_prompt_tool_definitions(nested_builder.specs().iter()); + collect_code_mode_exec_prompt_tool_definitions(code_mode_nested_tool_specs.iter()); enabled_tools .sort_by(|left, right| compare_code_mode_tools(left, right, &namespace_descriptions)); builder.register_handler(Arc::new(CodeModeExecuteHandler::new( @@ -106,40 +118,162 @@ pub fn build_tool_registry_builder( &enabled_tools, &namespace_descriptions, config.code_mode_only_enabled, - config.search_tool - && params - .deferred_mcp_tools - .is_some_and(|tools| !tools.is_empty()), + config.search_tool && !all_deferred_tools.is_empty(), ), + code_mode_nested_tool_specs, ))); builder.register_handler(Arc::new(CodeModeWaitHandler)); } + let mut non_deferred_specs = Vec::new(); + for handler in &handlers { + let tool_name = handler.tool_name(); + if !all_deferred_tools.contains(&tool_name) + && let Some(spec) = handler.spec() + { + non_deferred_specs.push(spec); + } + } + + if let Some(web_search_tool) = create_web_search_tool(WebSearchToolOptions { + web_search_mode: config.web_search_mode, + web_search_config: config.web_search_config.as_ref(), + web_search_tool_type: config.web_search_tool_type, + }) { + non_deferred_specs.push(web_search_tool); + } + if config.image_gen_tool { + non_deferred_specs.push(create_image_generation_tool("png")); + } + + for spec in merge_into_namespaces(non_deferred_specs) { + if !config.namespace_tools && matches!(spec, ToolSpec::Namespace(_)) { + continue; + } + let spec = if config.code_mode_enabled { + codex_tools::augment_tool_spec_for_code_mode(spec) + } else { + spec + }; + builder.push_spec(spec); + } + + for handler in handlers { + builder.register_any_handler_without_spec(handler); + } + + if config.search_tool && config.namespace_tools && !all_deferred_tools.is_empty() { + let mut search_source_infos = params + .deferred_mcp_tools + .map(|mcp_tools| { + collect_tool_search_source_infos(mcp_tools.iter().map(|tool| ToolSearchSource { + server_name: tool.server_name.as_str(), + connector_name: tool.connector_name.as_deref(), + description: tool.namespace_description.as_deref(), + })) + }) + .unwrap_or_default(); + + if params.dynamic_tools.iter().any(|tool| { + all_deferred_tools.contains(&ToolName::new(tool.namespace.clone(), tool.name.clone())) + }) { + search_source_infos.push(ToolSearchSourceInfo { + name: "Dynamic tools".to_string(), + description: Some("Tools provided by the current Codex thread.".to_string()), + }); + } + + builder.register_handler(Arc::new(ToolSearchHandler::new( + params.tool_search_entries.to_vec(), + search_source_infos, + ))); + } + + for bundle in params.extension_tool_bundles.iter().cloned() { + builder.register_tool_bundle(bundle); + } + + builder +} + +fn merge_into_namespaces(specs: Vec) -> Vec { + let mut merged_specs = Vec::with_capacity(specs.len()); + let mut namespace_indices = BTreeMap::::new(); + for spec in specs { + match spec { + ToolSpec::Namespace(mut namespace) => { + if let Some(index) = namespace_indices.get(&namespace.name).copied() { + let ToolSpec::Namespace(existing_namespace) = &mut merged_specs[index] else { + unreachable!("namespace index must point to a namespace spec"); + }; + if existing_namespace.description.trim().is_empty() + && !namespace.description.trim().is_empty() + { + existing_namespace.description = namespace.description; + } + existing_namespace.tools.append(&mut namespace.tools); + continue; + } + + namespace_indices.insert(namespace.name.clone(), merged_specs.len()); + merged_specs.push(ToolSpec::Namespace(namespace)); + } + spec => merged_specs.push(spec), + } + } + + for spec in &mut merged_specs { + let ToolSpec::Namespace(namespace) = spec else { + continue; + }; + + namespace.tools.sort_by(|left, right| match (left, right) { + ( + ResponsesApiNamespaceTool::Function(left), + ResponsesApiNamespaceTool::Function(right), + ) => left.name.cmp(&right.name), + }); + + if namespace.description.trim().is_empty() { + namespace.description = default_namespace_description(&namespace.name); + } + } + + merged_specs +} + +fn collect_handler_tools( + config: &ToolsConfig, + params: ToolRegistryBuildParams<'_>, +) -> Vec> { + let exec_permission_approvals_enabled = config.exec_permission_approvals_enabled; + let mut handlers = Vec::>::new(); + if config.environment_mode.has_environment() { let include_environment_id = matches!(config.environment_mode, ToolEnvironmentMode::Multiple); match &config.shell_type { ConfigShellToolType::Default => { - builder.register_handler(Arc::new(ShellHandler::new(ShellToolOptions { + handlers.push(Arc::new(ShellHandler::new(ShellToolOptions { exec_permission_approvals_enabled, }))); } ConfigShellToolType::Local => { - builder.register_handler(Arc::new(LocalShellHandler::new())); + handlers.push(Arc::new(LocalShellHandler::new())); } ConfigShellToolType::UnifiedExec => { - builder.register_handler(Arc::new(ExecCommandHandler::new( + handlers.push(Arc::new(ExecCommandHandler::new( ExecCommandHandlerOptions { allow_login_shell: config.allow_login_shell, exec_permission_approvals_enabled, include_environment_id, }, ))); - builder.register_handler(Arc::new(WriteStdinHandler)); + handlers.push(Arc::new(WriteStdinHandler)); } ConfigShellToolType::Disabled => {} ConfigShellToolType::ShellCommand => { - builder.register_handler(Arc::new(ShellCommandHandler::new( + handlers.push(Arc::new(ShellCommandHandler::new( ShellCommandHandlerOptions { backend_config: config.shell_command_backend, allow_login_shell: config.allow_login_shell, @@ -155,101 +289,62 @@ pub fn build_tool_registry_builder( { match &config.shell_type { ConfigShellToolType::Default => { - builder.register_handler(Arc::new(ContainerExecHandler)); - builder.register_handler(Arc::new(LocalShellHandler::default())); - builder.register_handler(Arc::new(ShellCommandHandler::from( + handlers.push(Arc::new(ContainerExecHandler)); + handlers.push(Arc::new(LocalShellHandler::default())); + handlers.push(Arc::new(ShellCommandHandler::from( config.shell_command_backend, ))); } ConfigShellToolType::Local => { - builder.register_handler(Arc::new(ShellHandler::default())); - builder.register_handler(Arc::new(ContainerExecHandler)); - builder.register_handler(Arc::new(ShellCommandHandler::from( + handlers.push(Arc::new(ShellHandler::default())); + handlers.push(Arc::new(ContainerExecHandler)); + handlers.push(Arc::new(ShellCommandHandler::from( config.shell_command_backend, ))); } ConfigShellToolType::UnifiedExec => { - builder.register_handler(Arc::new(ShellHandler::default())); - builder.register_handler(Arc::new(ContainerExecHandler)); - builder.register_handler(Arc::new(LocalShellHandler::default())); - builder.register_handler(Arc::new(ShellCommandHandler::from( + handlers.push(Arc::new(ShellHandler::default())); + handlers.push(Arc::new(ContainerExecHandler)); + handlers.push(Arc::new(LocalShellHandler::default())); + handlers.push(Arc::new(ShellCommandHandler::from( config.shell_command_backend, ))); } ConfigShellToolType::ShellCommand => { - builder.register_handler(Arc::new(ShellHandler::default())); - builder.register_handler(Arc::new(ContainerExecHandler)); - builder.register_handler(Arc::new(LocalShellHandler::default())); + handlers.push(Arc::new(ShellHandler::default())); + handlers.push(Arc::new(ContainerExecHandler)); + handlers.push(Arc::new(LocalShellHandler::default())); } ConfigShellToolType::Disabled => {} } } if params.mcp_tools.is_some() { - builder.register_handler(Arc::new(ListMcpResourcesHandler)); - builder.register_handler(Arc::new(ListMcpResourceTemplatesHandler)); - builder.register_handler(Arc::new(ReadMcpResourceHandler)); + handlers.push(Arc::new(ListMcpResourcesHandler)); + handlers.push(Arc::new(ListMcpResourceTemplatesHandler)); + handlers.push(Arc::new(ReadMcpResourceHandler)); } - builder.register_handler(Arc::new(PlanHandler)); + handlers.push(Arc::new(PlanHandler)); if config.goal_tools { - builder.register_handler(Arc::new(GetGoalHandler)); - builder.register_handler(Arc::new(CreateGoalHandler)); - builder.register_handler(Arc::new(UpdateGoalHandler)); + handlers.push(Arc::new(GetGoalHandler)); + handlers.push(Arc::new(CreateGoalHandler)); + handlers.push(Arc::new(UpdateGoalHandler)); } - builder.register_handler(Arc::new(RequestUserInputHandler { + handlers.push(Arc::new(RequestUserInputHandler { available_modes: config.request_user_input_available_modes.clone(), })); if config.request_permissions_tool_enabled { - builder.register_handler(Arc::new(RequestPermissionsHandler)); - } - - let deferred_dynamic_tools = params - .dynamic_tools - .iter() - .filter(|tool| tool.defer_loading && (config.namespace_tools || tool.namespace.is_none())) - .collect::>(); - let deferred_mcp_tools_for_search = if config.namespace_tools { - params.deferred_mcp_tools - } else { - None - }; - - if config.search_tool - && (deferred_mcp_tools_for_search.is_some() || !deferred_dynamic_tools.is_empty()) - { - let mut search_source_infos = deferred_mcp_tools_for_search - .map(|deferred_mcp_tools| { - collect_tool_search_source_infos(deferred_mcp_tools.iter().map(|tool| { - ToolSearchSource { - server_name: tool.server_name.as_str(), - connector_name: tool.connector_name.as_deref(), - description: tool.namespace_description.as_deref(), - } - })) - }) - .unwrap_or_default(); - - if !deferred_dynamic_tools.is_empty() { - search_source_infos.push(ToolSearchSourceInfo { - name: "Dynamic tools".to_string(), - description: Some("Tools provided by the current Codex thread.".to_string()), - }); - } - - builder.register_handler(Arc::new(ToolSearchHandler::new( - params.tool_search_entries.to_vec(), - search_source_infos, - ))); + handlers.push(Arc::new(RequestPermissionsHandler)); } if config.tool_suggest && let Some(discoverable_tools) = params.discoverable_tools.filter(|tools| !tools.is_empty()) { - builder.register_handler(Arc::new(RequestPluginInstallHandler::new( + handlers.push(Arc::new(RequestPluginInstallHandler::new( discoverable_tools, ))); } @@ -257,7 +352,7 @@ pub fn build_tool_registry_builder( if config.environment_mode.has_environment() && config.apply_patch_tool_type.is_some() { let include_environment_id = matches!(config.environment_mode, ToolEnvironmentMode::Multiple); - builder.register_handler(Arc::new(ApplyPatchHandler::new(include_environment_id))); + handlers.push(Arc::new(ApplyPatchHandler::new(include_environment_id))); } if config @@ -265,25 +360,13 @@ pub fn build_tool_registry_builder( .iter() .any(|tool| tool == "test_sync_tool") { - builder.register_handler(Arc::new(TestSyncHandler)); - } - - if let Some(web_search_tool) = create_web_search_tool(WebSearchToolOptions { - web_search_mode: config.web_search_mode, - web_search_config: config.web_search_config.as_ref(), - web_search_tool_type: config.web_search_tool_type, - }) { - builder.push_spec(web_search_tool); - } - - if config.image_gen_tool { - builder.push_spec(create_image_generation_tool("png")); + handlers.push(Arc::new(TestSyncHandler)); } if config.environment_mode.has_environment() { let include_environment_id = matches!(config.environment_mode, ToolEnvironmentMode::Multiple); - builder.register_handler(Arc::new(ViewImageHandler::new(ViewImageToolOptions { + handlers.push(Arc::new(ViewImageHandler::new(ViewImageToolOptions { can_request_original_image_detail: config.can_request_original_image_detail, include_environment_id, }))); @@ -293,7 +376,7 @@ pub fn build_tool_registry_builder( if config.multi_agent_v2 { let agent_type_description = agent_type_description(config, params.default_agent_type_description); - builder.register_handler(Arc::new(SpawnAgentHandlerV2::new(SpawnAgentToolOptions { + handlers.push(Arc::new(SpawnAgentHandlerV2::new(SpawnAgentToolOptions { available_models: config.available_models.clone(), agent_type_description, hide_agent_type_model_reasoning: config.hide_spawn_agent_metadata, @@ -301,17 +384,17 @@ pub fn build_tool_registry_builder( usage_hint_text: config.spawn_agent_usage_hint_text.clone(), max_concurrent_threads_per_session: config.max_concurrent_threads_per_session, }))); - builder.register_handler(Arc::new(SendMessageHandlerV2)); - builder.register_handler(Arc::new(FollowupTaskHandlerV2)); - builder.register_handler(Arc::new(WaitAgentHandlerV2::new( + handlers.push(Arc::new(SendMessageHandlerV2)); + handlers.push(Arc::new(FollowupTaskHandlerV2)); + handlers.push(Arc::new(WaitAgentHandlerV2::new( params.wait_agent_timeouts, ))); - builder.register_handler(Arc::new(CloseAgentHandlerV2)); - builder.register_handler(Arc::new(ListAgentsHandlerV2)); + handlers.push(Arc::new(CloseAgentHandlerV2)); + handlers.push(Arc::new(ListAgentsHandlerV2)); } else { let agent_type_description = agent_type_description(config, params.default_agent_type_description); - builder.register_handler(Arc::new(SpawnAgentHandler::new(SpawnAgentToolOptions { + handlers.push(Arc::new(SpawnAgentHandler::new(SpawnAgentToolOptions { available_models: config.available_models.clone(), agent_type_description, hide_agent_type_model_reasoning: config.hide_spawn_agent_metadata, @@ -319,122 +402,45 @@ pub fn build_tool_registry_builder( usage_hint_text: config.spawn_agent_usage_hint_text.clone(), max_concurrent_threads_per_session: config.max_concurrent_threads_per_session, }))); - builder.register_handler(Arc::new(SendInputHandler)); - builder.register_handler(Arc::new(ResumeAgentHandler)); - builder.register_handler(Arc::new(WaitAgentHandler::new(params.wait_agent_timeouts))); - builder.register_handler(Arc::new(CloseAgentHandler)); + handlers.push(Arc::new(SendInputHandler)); + handlers.push(Arc::new(ResumeAgentHandler)); + handlers.push(Arc::new(WaitAgentHandler::new(params.wait_agent_timeouts))); + handlers.push(Arc::new(CloseAgentHandler)); } } if config.agent_jobs_tools { - builder.register_handler(Arc::new(SpawnAgentsOnCsvHandler)); + handlers.push(Arc::new(SpawnAgentsOnCsvHandler)); if config.agent_jobs_worker_tools { - builder.register_handler(Arc::new(ReportAgentJobResultHandler)); + handlers.push(Arc::new(ReportAgentJobResultHandler)); } } if let Some(mcp_tools) = params.mcp_tools { - let mut entries = mcp_tools - .iter() - .map(|tool| (tool.canonical_tool_name(), tool)) - .collect::>(); - entries.sort_by(|(left, _), (right, _)| left.cmp(right)); - let mut namespace_entries = BTreeMap::new(); - - for (tool_name, tool) in entries { - let Some(namespace) = tool_name.namespace.as_ref() else { - tracing::error!("Skipping MCP tool `{tool_name}`: MCP tools must be namespaced"); - continue; - }; - namespace_entries - .entry(namespace.clone()) - .or_insert_with(Vec::new) - .push((tool_name, tool)); - } - - for (namespace, mut entries) in namespace_entries { - entries.sort_by_key(|(tool_name, _)| tool_name.name.clone()); - let tool_namespace = params - .tool_namespaces - .and_then(|namespaces| namespaces.get(&namespace)); - let description = tool_namespace - .and_then(|namespace| namespace.description.as_deref()) - .map(str::trim) - .filter(|description| !description.is_empty()) - .map(str::to_string) - .unwrap_or_else(|| { - let namespace_name = tool_namespace - .map(|namespace| namespace.name.as_str()) - .unwrap_or(namespace.as_str()); - default_namespace_description(namespace_name) - }); - let mut tools = Vec::new(); - for (tool_name, tool) in entries { - match mcp_tool_to_responses_api_tool(&tool_name, &tool.tool) { - Ok(converted_tool) => { - tools.push(ResponsesApiNamespaceTool::Function(converted_tool)); - builder.register_handler(Arc::new(McpHandler::new(tool.clone()))); - } - Err(error) => { - tracing::error!( - "Failed to convert `{tool_name}` MCP tool to OpenAI tool: {error:?}" - ); - } - } - } - - if config.namespace_tools && !tools.is_empty() { - builder.push_spec(ToolSpec::Namespace(ResponsesApiNamespace { - name: namespace, - description, - tools, - })); - } - } - } - - let mut dynamic_tool_specs = Vec::new(); - for tool in params.dynamic_tools { - match dynamic_tool_to_loadable_tool_spec(tool) { - Ok(loadable_tool) => { - let handler_name = ToolName::new(tool.namespace.clone(), tool.name.clone()); - dynamic_tool_specs.push(loadable_tool); - builder.register_handler(Arc::new(DynamicToolHandler::new(handler_name))); - } - Err(error) => { - tracing::error!( - "Failed to convert dynamic tool {:?} to OpenAI tool: {error:?}", - tool.name - ); - } - } - } - for spec in coalesce_loadable_tool_specs(dynamic_tool_specs) { - let spec = spec.into(); - if config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_)) { - builder.push_spec(spec); + for tool in mcp_tools { + handlers.push(Arc::new(McpHandler::new(tool.clone()))); } } if let Some(deferred_mcp_tools) = params.deferred_mcp_tools { - let directly_registered_mcp_tools = params - .mcp_tools - .into_iter() - .flatten() - .map(ToolInfo::canonical_tool_name) - .collect::>(); for tool in deferred_mcp_tools { - if !directly_registered_mcp_tools.contains(&tool.canonical_tool_name()) { - builder.register_handler(Arc::new(McpHandler::new(tool.clone()))); - } + handlers.push(Arc::new(McpHandler::new(tool.clone()))); } } - for bundle in params.extension_tool_bundles.iter().cloned() { - builder.register_tool_bundle(bundle); + for tool in params.dynamic_tools { + let Some(handler) = DynamicToolHandler::new(tool).map(Arc::new) else { + tracing::error!( + "Failed to convert dynamic tool {:?} to OpenAI tool", + tool.name + ); + continue; + }; + + handlers.push(handler); } - builder + handlers } fn compare_code_mode_tools( diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index a4132391a..7c876d373 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -1579,20 +1579,7 @@ fn search_tool_description_lists_each_mcp_source_once() { let (tools, registry) = build_specs( &tools_config, - Some(HashMap::from([ - ( - ToolName::namespaced("mcp__codex_apps__calendar", "_create_event"), - mcp_tool( - "calendar_create_event", - "Create calendar event", - serde_json::json!({"type": "object"}), - ), - ), - ( - ToolName::namespaced("mcp__rmcp__", "echo"), - mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})), - ), - ])), + /*mcp_tools*/ None, Some(vec![ deferred_mcp_tool( "_create_event", @@ -1810,32 +1797,15 @@ fn search_tool_registers_for_deferred_dynamic_tools() { panic!("expected tool_search tool"); }; assert!(description.contains("- Dynamic tools: Tools provided by the current Codex thread.")); - assert_contains_tool_names(&tools, &[TOOL_SEARCH_TOOL_NAME, "codex_app"]); - assert_eq!( - tools - .iter() - .filter(|tool| tool.name() == "codex_app") - .count(), - 1 - ); - assert_eq!( - namespace_function_names(&tools, "codex_app"), - vec![ - "automation_update".to_string(), - "automation_list".to_string() - ] - ); - for tool_name in ["automation_update", "automation_list"] { - let dynamic_tool = find_namespace_function_tool(&tools, "codex_app", tool_name); - assert_eq!(dynamic_tool.defer_loading, Some(true)); - } + 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"))); } #[test] -fn search_tool_keeps_plain_deferred_dynamic_tools_when_namespace_tools_are_disabled() { +fn search_tool_is_hidden_for_deferred_dynamic_tools_when_namespace_tools_are_disabled() { let model_info = search_capable_model_info(); let mut features = Features::with_defaults(); features.enable(Feature::ToolSearch); @@ -1875,9 +1845,12 @@ fn search_tool_keeps_plain_deferred_dynamic_tools_when_namespace_tools_are_disab &dynamic_tools, ); - assert_contains_tool_names(&tools, &[TOOL_SEARCH_TOOL_NAME, "plain_dynamic"]); + assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); assert_lacks_tool_name(&tools, "codex_app"); - assert!(registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); + 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"))); } #[test] diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 008654285..52f92b281 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -365,7 +365,7 @@ async fn assert_model_tools( .iter() .map(ToolSpec::name) .collect::>(); - assert_eq!(&tool_names, &expected_tools,); + assert_eq!(&tool_names, &expected_tools); } async fn assert_default_model_tools( @@ -396,14 +396,14 @@ async fn test_build_specs_gpt5_codex_default() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -421,14 +421,14 @@ async fn test_build_specs_gpt51_codex_default() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -448,14 +448,14 @@ async fn test_build_specs_gpt5_codex_unified_exec_web_search() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -475,14 +475,14 @@ async fn test_build_specs_gpt51_codex_unified_exec_web_search() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -500,14 +500,14 @@ async fn test_gpt_5_1_codex_max_defaults() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -525,14 +525,14 @@ async fn test_codex_5_1_mini_defaults() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -550,14 +550,14 @@ async fn test_gpt_5_defaults() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -575,14 +575,14 @@ async fn test_gpt_5_1_defaults() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -602,14 +602,14 @@ async fn test_gpt_5_1_codex_max_unified_exec_web_search() { "update_plan", "request_user_input", "apply_patch", - "web_search", - "image_generation", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", + "image_generation", ], ) .await; @@ -841,7 +841,7 @@ async fn request_plugin_install_requires_apps_and_plugins_features() { } #[tokio::test] -async fn search_tool_description_handles_no_enabled_mcp_tools() { +async fn search_tool_is_hidden_without_deferred_tools() { let model_info = search_capable_model_info().await; let mut features = Features::with_defaults(); features.enable(Feature::Apps); @@ -865,13 +865,11 @@ async fn search_tool_description_handles_no_enabled_mcp_tools() { &[], ) .build(); - let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME); - let ToolSpec::ToolSearch { description, .. } = search_tool else { - panic!("expected tool_search tool"); - }; - - assert!(description.contains("None currently enabled.")); - assert!(!description.contains("{{source_descriptions}}")); + assert!( + !tools + .iter() + .any(|tool| tool.name() == TOOL_SEARCH_TOOL_NAME) + ); } #[tokio::test] @@ -1003,7 +1001,7 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() { } #[tokio::test] -async fn tool_search_entries_skip_namespace_outputs_when_namespace_tools_are_disabled() { +async fn tool_search_entries_skip_mcp_outputs_when_namespace_tools_are_disabled() { let model_info = search_capable_model_info().await; let mut features = Features::with_defaults(); features.enable(Feature::ToolSearch); @@ -1048,11 +1046,15 @@ async fn tool_search_entries_skip_namespace_outputs_when_namespace_tools_are_dis .map(|entry| entry.output) .collect::>(); - assert_eq!(outputs.len(), 1); - match &outputs[0] { - LoadableToolSpec::Function(tool) => assert_eq!(tool.name, "plain_dynamic"), - LoadableToolSpec::Namespace(_) => panic!("namespace tool_search output should be hidden"), - } + assert_eq!(outputs.len(), 2); + assert!(outputs.iter().any(|output| match output { + LoadableToolSpec::Namespace(namespace) => namespace.name == "codex_app", + LoadableToolSpec::Function(_) => false, + })); + assert!(outputs.iter().any(|output| match output { + LoadableToolSpec::Function(tool) => tool.name == "plain_dynamic", + LoadableToolSpec::Namespace(_) => false, + })); } #[tokio::test] diff --git a/codex-rs/core/src/tools/tool_search_entry.rs b/codex-rs/core/src/tools/tool_search_entry.rs index 6a8ccd640..cbaa4d87c 100644 --- a/codex-rs/core/src/tools/tool_search_entry.rs +++ b/codex-rs/core/src/tools/tool_search_entry.rs @@ -63,11 +63,7 @@ pub(crate) fn build_tool_search_entries_for_config( } else { None }; - let dynamic_tools = dynamic_tools - .iter() - .filter(|tool| config.namespace_tools || tool.namespace.is_none()) - .cloned() - .collect::>(); + let dynamic_tools = dynamic_tools.to_vec(); build_tool_search_entries(mcp_tools, &dynamic_tools) } diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index b81bb06bb..5b861ab21 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -179,13 +179,13 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> { "update_plan", "request_user_input", "apply_patch", - "web_search", "view_image", "spawn_agent", "send_input", "resume_agent", "wait_agent", "close_agent", + "web_search", ]); let body0 = req1.single_request().body_json();