From 67c8486462a17ca76d29656302d07004d29c87e4 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 12 May 2026 20:47:50 -0700 Subject: [PATCH] tools: infer code-mode namespace descriptions from specs (#22406) ## Why Code mode already builds the merged nested `ToolSpec`s that feed the `exec` prompt. Keeping a separate `tool_namespaces` map in the planning path duplicated that metadata and left extra wrapper plumbing in `spec.rs`. ## What changed - derive code-mode namespace descriptions from the merged `ToolSpec::Namespace` entries before building the code-mode handlers - extract `build_code_mode_handlers(...)` so the code-mode-specific planning stays in one place - remove `tool_namespaces` from `ToolRegistryBuildParams` - delete the now-unused `McpToolPlanInputs` wrapper and related test helper plumbing ## Testing - `cargo test -p codex-core spec_plan` --- codex-rs/core/src/tools/spec.rs | 33 +------ codex-rs/core/src/tools/spec_plan.rs | 107 +++++++++++++-------- codex-rs/core/src/tools/spec_plan_tests.rs | 22 ----- codex-rs/core/src/tools/spec_plan_types.rs | 8 -- 4 files changed, 69 insertions(+), 101 deletions(-) diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 8c54c21479..812997ae84 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -7,7 +7,6 @@ use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; use crate::tools::registry::ToolRegistryBuilder; use crate::tools::spec_plan::build_tool_registry_builder; -use crate::tools::spec_plan_types::ToolNamespace; use crate::tools::spec_plan_types::ToolRegistryBuildParams; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; @@ -19,7 +18,6 @@ use codex_tools::ResponsesApiTool; use codex_tools::ToolName; use codex_tools::ToolUserShellType; use codex_tools::ToolsConfig; -use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; @@ -33,29 +31,6 @@ pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType { } } -struct McpToolPlanInputs { - mcp_tools: Vec, - tool_namespaces: HashMap, -} - -fn map_mcp_tools_for_plan(mcp_tools: &[ToolInfo]) -> McpToolPlanInputs { - McpToolPlanInputs { - mcp_tools: mcp_tools.to_vec(), - tool_namespaces: mcp_tools - .iter() - .map(|tool| { - ( - tool.callable_namespace.clone(), - ToolNamespace { - name: tool.callable_namespace.clone(), - description: tool.namespace_description.clone(), - }, - ) - }) - .collect(), - } -} - pub(crate) fn build_specs_with_discoverable_tools( config: &ToolsConfig, mcp_tools: Option>, @@ -69,7 +44,6 @@ pub(crate) fn build_specs_with_discoverable_tools( use crate::tools::handlers::unavailable_tool_message; 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 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 { @@ -95,13 +69,8 @@ pub(crate) fn build_specs_with_discoverable_tools( let mut builder = build_tool_registry_builder( config, ToolRegistryBuildParams { - mcp_tools: mcp_tool_plan_inputs - .as_ref() - .map(|inputs| inputs.mcp_tools.as_slice()), + mcp_tools: mcp_tools.as_deref(), deferred_mcp_tools: deferred_mcp_tools.as_deref(), - tool_namespaces: mcp_tool_plan_inputs - .as_ref() - .map(|inputs| &inputs.tool_namespaces), discoverable_tools: discoverable_tools.as_deref(), extension_tool_bundles, dynamic_tools, diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 7232dddd85..4b2eb3ed24 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -84,45 +84,13 @@ pub fn build_tool_registry_builder( .collect::>(); let handlers = collect_handler_tools(config, params); - if config.code_mode_enabled { - let namespace_descriptions = params - .tool_namespaces - .into_iter() - .flatten() - .map(|(namespace, detail)| { - ( - namespace.clone(), - codex_code_mode::ToolNamespaceDescription { - name: detail.name.clone(), - description: detail.description.clone().unwrap_or_default(), - }, - ) - }) - .collect::>(); - 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(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( - create_code_mode_tool( - &enabled_tools, - &namespace_descriptions, - config.code_mode_only_enabled, - config.search_tool && !all_deferred_tools.is_empty(), - ), - code_mode_nested_tool_specs, - ))); - builder.register_handler(Arc::new(CodeModeWaitHandler)); + for handler in build_code_mode_handlers( + config, + &handlers, + params.extension_tool_bundles, + config.search_tool && !all_deferred_tools.is_empty(), + ) { + builder.register_any_handler(handler); } let mut non_deferred_specs = Vec::new(); @@ -196,6 +164,45 @@ pub fn build_tool_registry_builder( builder } +fn build_code_mode_handlers( + config: &ToolsConfig, + handlers: &[Arc], + extension_tool_bundles: &[codex_tool_api::ToolBundle], + deferred_tools_available: bool, +) -> Vec> { + if !config.code_mode_enabled { + return vec![]; + } + + let mut code_mode_nested_tool_specs = handlers + .iter() + .filter_map(|handler| handler.spec()) + .collect::>(); + code_mode_nested_tool_specs.extend( + extension_tool_bundles + .iter() + .filter_map(|bundle| extension_tool_spec(bundle.spec()).ok()), + ); + let namespace_descriptions = code_mode_namespace_descriptions(&code_mode_nested_tool_specs); + let mut enabled_tools = + 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)); + + vec![ + Arc::new(CodeModeExecuteHandler::new( + create_code_mode_tool( + &enabled_tools, + &namespace_descriptions, + config.code_mode_only_enabled, + deferred_tools_available, + ), + code_mode_nested_tool_specs, + )), + Arc::new(CodeModeWaitHandler), + ] +} + fn merge_into_namespaces(specs: Vec) -> Vec { let mut merged_specs = Vec::with_capacity(specs.len()); let mut namespace_indices = BTreeMap::::new(); @@ -242,6 +249,28 @@ fn merge_into_namespaces(specs: Vec) -> Vec { merged_specs } +fn code_mode_namespace_descriptions( + specs: &[ToolSpec], +) -> BTreeMap { + let mut namespace_descriptions = BTreeMap::new(); + for spec in specs { + let ToolSpec::Namespace(namespace) = spec else { + continue; + }; + + let entry = namespace_descriptions + .entry(namespace.name.clone()) + .or_insert_with(|| codex_code_mode::ToolNamespaceDescription { + name: namespace.name.clone(), + description: namespace.description.clone(), + }); + if entry.description.trim().is_empty() && !namespace.description.trim().is_empty() { + entry.description = namespace.description.clone(); + } + } + namespace_descriptions +} + fn collect_handler_tools( config: &ToolsConfig, params: ToolRegistryBuildParams<'_>, diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 7c876d3738..b028c2a2a9 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -25,7 +25,6 @@ use crate::tools::handlers::shell_spec::request_permissions_tool_description; 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 crate::tools::spec_plan_types::ToolNamespace; use codex_app_server_protocol::AppInfo; use codex_features::Feature; use codex_features::Features; @@ -2399,26 +2398,6 @@ fn build_specs_with_discoverable_tools( discoverable_tools: Option>, extension_tool_bundles: &[codex_tool_api::ToolBundle], dynamic_tools: &[DynamicToolSpec], -) -> (Vec, ToolRegistry) { - build_specs_with_optional_tool_namespaces( - config, - mcp_tools, - deferred_mcp_tools, - /*tool_namespaces*/ None, - discoverable_tools, - extension_tool_bundles, - dynamic_tools, - ) -} - -fn build_specs_with_optional_tool_namespaces( - config: &ToolsConfig, - mcp_tools: Option>, - deferred_mcp_tools: Option>, - tool_namespaces: Option>, - discoverable_tools: Option>, - extension_tool_bundles: &[codex_tool_api::ToolBundle], - dynamic_tools: &[DynamicToolSpec], ) -> (Vec, ToolRegistry) { let mcp_tool_inputs = mcp_tools.as_ref().map(|mcp_tools| { mcp_tools @@ -2431,7 +2410,6 @@ fn build_specs_with_optional_tool_namespaces( ToolRegistryBuildParams { mcp_tools: mcp_tool_inputs.as_deref(), deferred_mcp_tools: deferred_mcp_tools.as_deref(), - tool_namespaces: tool_namespaces.as_ref(), discoverable_tools: discoverable_tools.as_deref(), extension_tool_bundles, dynamic_tools, diff --git a/codex-rs/core/src/tools/spec_plan_types.rs b/codex-rs/core/src/tools/spec_plan_types.rs index 9e9ff555ce..53a53dca46 100644 --- a/codex-rs/core/src/tools/spec_plan_types.rs +++ b/codex-rs/core/src/tools/spec_plan_types.rs @@ -4,13 +4,11 @@ use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_tool_api::ToolBundle as ExtensionToolBundle; use codex_tools::DiscoverableTool; use codex_tools::ToolsConfig; -use std::collections::HashMap; #[derive(Clone, Copy)] pub struct ToolRegistryBuildParams<'a> { pub mcp_tools: Option<&'a [ToolInfo]>, pub deferred_mcp_tools: Option<&'a [ToolInfo]>, - pub tool_namespaces: Option<&'a HashMap>, pub discoverable_tools: Option<&'a [DiscoverableTool]>, pub extension_tool_bundles: &'a [ExtensionToolBundle], pub dynamic_tools: &'a [DynamicToolSpec], @@ -19,12 +17,6 @@ pub struct ToolRegistryBuildParams<'a> { pub tool_search_entries: &'a [crate::tools::tool_search_entry::ToolSearchEntry], } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct ToolNamespace { - pub name: String, - pub description: Option, -} - pub(crate) fn agent_type_description( config: &ToolsConfig, default_agent_type_description: &str,