mirror of
https://github.com/openai/codex.git
synced 2026-05-18 02:02:30 +00:00
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`
This commit is contained in:
@@ -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<ToolInfo>,
|
||||
tool_namespaces: HashMap<String, ToolNamespace>,
|
||||
}
|
||||
|
||||
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<Vec<ToolInfo>>,
|
||||
@@ -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,
|
||||
|
||||
@@ -84,45 +84,13 @@ pub fn build_tool_registry_builder(
|
||||
.collect::<HashSet<_>>();
|
||||
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::<BTreeMap<_, _>>();
|
||||
let mut code_mode_nested_tool_specs = handlers
|
||||
.iter()
|
||||
.filter_map(|handler| handler.spec())
|
||||
.collect::<Vec<_>>();
|
||||
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<dyn AnyToolHandler>],
|
||||
extension_tool_bundles: &[codex_tool_api::ToolBundle],
|
||||
deferred_tools_available: bool,
|
||||
) -> Vec<Arc<dyn AnyToolHandler>> {
|
||||
if !config.code_mode_enabled {
|
||||
return vec![];
|
||||
}
|
||||
|
||||
let mut code_mode_nested_tool_specs = handlers
|
||||
.iter()
|
||||
.filter_map(|handler| handler.spec())
|
||||
.collect::<Vec<_>>();
|
||||
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<ToolSpec>) -> Vec<ToolSpec> {
|
||||
let mut merged_specs = Vec::with_capacity(specs.len());
|
||||
let mut namespace_indices = BTreeMap::<String, usize>::new();
|
||||
@@ -242,6 +249,28 @@ fn merge_into_namespaces(specs: Vec<ToolSpec>) -> Vec<ToolSpec> {
|
||||
merged_specs
|
||||
}
|
||||
|
||||
fn code_mode_namespace_descriptions(
|
||||
specs: &[ToolSpec],
|
||||
) -> BTreeMap<String, codex_code_mode::ToolNamespaceDescription> {
|
||||
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<'_>,
|
||||
|
||||
@@ -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<Vec<DiscoverableTool>>,
|
||||
extension_tool_bundles: &[codex_tool_api::ToolBundle],
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> (Vec<ToolSpec>, 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<HashMap<ToolName, rmcp::model::Tool>>,
|
||||
deferred_mcp_tools: Option<Vec<ToolInfo>>,
|
||||
tool_namespaces: Option<HashMap<String, ToolNamespace>>,
|
||||
discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
extension_tool_bundles: &[codex_tool_api::ToolBundle],
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> (Vec<ToolSpec>, 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,
|
||||
|
||||
@@ -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<String, ToolNamespace>>,
|
||||
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<String>,
|
||||
}
|
||||
|
||||
pub(crate) fn agent_type_description(
|
||||
config: &ToolsConfig,
|
||||
default_agent_type_description: &str,
|
||||
|
||||
Reference in New Issue
Block a user