diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index dafaffca3c49..8c1e4c399a9e 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -249,11 +249,11 @@ use codex_login::default_client::set_default_client_residency_requirement; use codex_login::login_with_api_key; use codex_login::request_device_code; use codex_login::run_login_server; +use codex_mcp::McpServerStatusSnapshot; use codex_mcp::McpSnapshotDetail; -use codex_mcp::collect_mcp_snapshot_with_detail; +use codex_mcp::collect_mcp_server_status_snapshot_with_detail; use codex_mcp::discover_supported_scopes; use codex_mcp::effective_mcp_servers; -use codex_mcp::qualified_mcp_tool_name_prefix; use codex_mcp::resolve_oauth_scopes; use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig; use codex_protocol::ThreadId; @@ -5178,7 +5178,7 @@ impl CodexMessageProcessor { McpServerStatusDetail::ToolsAndAuthOnly => McpSnapshotDetail::ToolsAndAuthOnly, }; - let snapshot = collect_mcp_snapshot_with_detail( + let snapshot = collect_mcp_server_status_snapshot_with_detail( &mcp_config, auth.as_ref(), request_id.request_id.to_string(), @@ -5186,47 +5186,13 @@ impl CodexMessageProcessor { ) .await; - // Rebuild the tool list per original server name instead of using - // `group_tools_by_server()`: qualified tool names are sanitized for the - // Responses API, so a config key like `some-server` is encoded as the - // `mcp__some_server__` prefix. Matching with the original server name's - // sanitized prefix preserves `/mcp` output for hyphenated names. let effective_servers = effective_mcp_servers(&mcp_config, auth.as_ref()); - let mut sanitized_prefix_counts = HashMap::::new(); - for name in effective_servers.keys() { - let prefix = qualified_mcp_tool_name_prefix(name); - *sanitized_prefix_counts.entry(prefix).or_default() += 1; - } - let tools_by_server = effective_servers - .keys() - .map(|name| { - let prefix = qualified_mcp_tool_name_prefix(name); - // If multiple server names normalize to the same prefix, the - // qualified tool namespace is ambiguous (for example - // `some-server` and `some_server` both become - // `mcp__some_server__`). In that case, avoid attributing the - // same tools to multiple servers. - let tools = if sanitized_prefix_counts - .get(&prefix) - .copied() - .unwrap_or_default() - == 1 - { - snapshot - .tools - .iter() - .filter_map(|(qualified_name, tool)| { - qualified_name - .strip_prefix(&prefix) - .map(|tool_name| (tool_name.to_string(), tool.clone())) - }) - .collect::>() - } else { - HashMap::new() - }; - (name.clone(), tools) - }) - .collect::>(); + let McpServerStatusSnapshot { + tools_by_server, + resources, + resource_templates, + auth_statuses, + } = snapshot; let mut server_names: Vec = config .mcp_servers @@ -5236,9 +5202,9 @@ impl CodexMessageProcessor { // effective runtime config even when they are not user-declared in // `config.mcp_servers`. .chain(effective_servers.keys().cloned()) - .chain(snapshot.auth_statuses.keys().cloned()) - .chain(snapshot.resources.keys().cloned()) - .chain(snapshot.resource_templates.keys().cloned()) + .chain(auth_statuses.keys().cloned()) + .chain(resources.keys().cloned()) + .chain(resource_templates.keys().cloned()) .collect(); server_names.sort(); server_names.dedup(); @@ -5279,14 +5245,9 @@ impl CodexMessageProcessor { .map(|name| McpServerStatus { name: name.clone(), tools: tools_by_server.get(name).cloned().unwrap_or_default(), - resources: snapshot.resources.get(name).cloned().unwrap_or_default(), - resource_templates: snapshot - .resource_templates - .get(name) - .cloned() - .unwrap_or_default(), - auth_status: snapshot - .auth_statuses + resources: resources.get(name).cloned().unwrap_or_default(), + resource_templates: resource_templates.get(name).cloned().unwrap_or_default(), + auth_status: auth_statuses .get(name) .cloned() .unwrap_or(CoreMcpAuthStatus::Unsupported) diff --git a/codex-rs/app-server/tests/suite/v2/mcp_server_status.rs b/codex-rs/app-server/tests/suite/v2/mcp_server_status.rs index e637be237d5c..44efec4ed3f8 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_server_status.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_server_status.rs @@ -38,9 +38,9 @@ use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); #[tokio::test] -async fn mcp_server_status_list_returns_tools_for_hyphenated_server_names() -> Result<()> { +async fn mcp_server_status_list_returns_raw_server_and_tool_names() -> Result<()> { let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server("lookup").await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server("look-up.raw").await?; let codex_home = TempDir::new()?; write_mock_responses_config_toml( codex_home.path(), @@ -85,7 +85,14 @@ url = "{mcp_server_url}/mcp" assert_eq!(status.name, "some-server"); assert_eq!( status.tools.keys().cloned().collect::>(), - BTreeSet::from(["lookup".to_string()]) + BTreeSet::from(["look-up.raw".to_string()]) + ); + assert_eq!( + status + .tools + .get("look-up.raw") + .map(|tool| tool.name.as_str()), + Some("look-up.raw") ); mcp_server_handle.abort(); @@ -261,8 +268,7 @@ url = "{mcp_server_url}/mcp" } #[tokio::test] -async fn mcp_server_status_list_does_not_duplicate_tools_for_sanitized_name_collisions() --> Result<()> { +async fn mcp_server_status_list_keeps_tools_for_sanitized_name_collisions() -> Result<()> { let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await; let (dash_server_url, dash_server_handle) = start_mcp_server("dash_lookup").await?; let (underscore_server_url, underscore_server_handle) = @@ -313,11 +319,22 @@ url = "{underscore_server_url}/mcp" let status_tools = response .data .iter() - .map(|status| (status.name.as_str(), status.tools.keys().count())) + .map(|status| { + ( + status.name.as_str(), + status.tools.keys().cloned().collect::>(), + ) + }) .collect::>(); assert_eq!( status_tools, - BTreeMap::from([("some-server", 0), ("some_server", 0)]) + BTreeMap::from([ + ("some-server", BTreeSet::from(["dash_lookup".to_string()])), + ( + "some_server", + BTreeSet::from(["underscore_lookup".to_string()]) + ) + ]) ); dash_server_handle.abort(); diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 8912c71df69e..7b9aea77ff1a 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -1,5 +1,6 @@ pub(crate) mod mcp; pub(crate) mod mcp_connection_manager; +pub(crate) mod mcp_tool_names; pub use mcp::CODEX_APPS_MCP_SERVER_NAME; pub use mcp::McpAuthStatusEntry; @@ -8,10 +9,13 @@ pub use mcp::McpManager; pub use mcp::McpOAuthLoginConfig; pub use mcp::McpOAuthLoginSupport; pub use mcp::McpOAuthScopesSource; +pub use mcp::McpServerStatusSnapshot; pub use mcp::McpSnapshotDetail; pub use mcp::ResolvedMcpOAuthScopes; pub use mcp::ToolPluginProvenance; pub use mcp::canonical_mcp_server_key; +pub use mcp::collect_mcp_server_status_snapshot; +pub use mcp::collect_mcp_server_status_snapshot_with_detail; pub use mcp::collect_mcp_snapshot; pub use mcp::collect_mcp_snapshot_from_manager; pub use mcp::collect_mcp_snapshot_from_manager_with_detail; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 34d6837296ee..ba0442957878 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -29,6 +29,7 @@ use codex_protocol::mcp::Resource; use codex_protocol::mcp::ResourceTemplate; use codex_protocol::mcp::Tool; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::McpAuthStatus; use codex_protocol::protocol::McpListToolsResponseEvent; use codex_protocol::protocol::SandboxPolicy; use serde_json::Value; @@ -379,6 +380,79 @@ pub async fn collect_mcp_snapshot_with_detail( snapshot } +#[derive(Debug, Clone)] +pub struct McpServerStatusSnapshot { + pub tools_by_server: HashMap>, + pub resources: HashMap>, + pub resource_templates: HashMap>, + pub auth_statuses: HashMap, +} + +pub async fn collect_mcp_server_status_snapshot( + config: &McpConfig, + auth: Option<&CodexAuth>, + submit_id: String, +) -> McpServerStatusSnapshot { + collect_mcp_server_status_snapshot_with_detail(config, auth, submit_id, McpSnapshotDetail::Full) + .await +} + +pub async fn collect_mcp_server_status_snapshot_with_detail( + config: &McpConfig, + auth: Option<&CodexAuth>, + submit_id: String, + detail: McpSnapshotDetail, +) -> McpServerStatusSnapshot { + let mcp_servers = effective_mcp_servers(config, auth); + let tool_plugin_provenance = tool_plugin_provenance(config); + if mcp_servers.is_empty() { + return McpServerStatusSnapshot { + tools_by_server: HashMap::new(), + resources: HashMap::new(), + resource_templates: HashMap::new(), + auth_statuses: HashMap::new(), + }; + } + + let auth_status_entries = + compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode).await; + + let (tx_event, rx_event) = unbounded(); + drop(rx_event); + + let sandbox_state = SandboxState { + sandbox_policy: SandboxPolicy::new_read_only_policy(), + codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), + sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), + use_legacy_landlock: config.use_legacy_landlock, + }; + + let (mcp_connection_manager, cancel_token) = McpConnectionManager::new( + &mcp_servers, + config.mcp_oauth_credentials_store_mode, + auth_status_entries.clone(), + &config.approval_policy, + submit_id, + tx_event, + sandbox_state, + config.codex_home.clone(), + codex_apps_tools_cache_key(auth), + tool_plugin_provenance, + ) + .await; + + let snapshot = collect_mcp_server_status_snapshot_from_manager( + &mcp_connection_manager, + auth_status_entries, + detail, + ) + .await; + + cancel_token.cancel(); + + snapshot +} + pub fn split_qualified_tool_name(qualified_name: &str) -> Option<(String, String)> { let mut parts = qualified_name.split(MCP_TOOL_NAME_DELIMITER); let prefix = parts.next()?; @@ -408,64 +482,35 @@ pub fn group_tools_by_server( grouped } -pub async fn collect_mcp_snapshot_from_manager( - mcp_connection_manager: &McpConnectionManager, - auth_status_entries: HashMap, -) -> McpListToolsResponseEvent { - collect_mcp_snapshot_from_manager_with_detail( - mcp_connection_manager, - auth_status_entries, - McpSnapshotDetail::Full, - ) - .await -} - -pub async fn collect_mcp_snapshot_from_manager_with_detail( - mcp_connection_manager: &McpConnectionManager, - auth_status_entries: HashMap, - detail: McpSnapshotDetail, -) -> McpListToolsResponseEvent { - let (tools, resources, resource_templates) = tokio::join!( - mcp_connection_manager.list_all_tools(), - async { - if detail.include_resources() { - mcp_connection_manager.list_all_resources().await - } else { - HashMap::new() - } - }, - async { - if detail.include_resources() { - mcp_connection_manager.list_all_resource_templates().await - } else { - HashMap::new() +fn protocol_tool_from_rmcp_tool(name: &str, tool: &rmcp::model::Tool) -> Option { + match serde_json::to_value(tool) { + Ok(value) => match Tool::from_mcp_value(value) { + Ok(tool) => Some(tool), + Err(err) => { + tracing::warn!("Failed to convert MCP tool '{name}': {err}"); + None } }, - ); + Err(err) => { + tracing::warn!("Failed to serialize MCP tool '{name}': {err}"); + None + } + } +} - let auth_statuses = auth_status_entries +fn auth_statuses_from_entries( + auth_status_entries: &HashMap, +) -> HashMap { + auth_status_entries .iter() .map(|(name, entry)| (name.clone(), entry.auth_status)) - .collect::>(); - - let tools = tools - .into_iter() - .filter_map(|(name, tool)| match serde_json::to_value(tool.tool) { - Ok(value) => match Tool::from_mcp_value(value) { - Ok(tool) => Some((name, tool)), - Err(err) => { - tracing::warn!("Failed to convert MCP tool '{name}': {err}"); - None - } - }, - Err(err) => { - tracing::warn!("Failed to serialize MCP tool '{name}': {err}"); - None - } - }) - .collect::>(); + .collect::>() +} - let resources = resources +fn convert_mcp_resources( + resources: HashMap>, +) -> HashMap> { + resources .into_iter() .map(|(name, resources)| { let resources = resources @@ -498,9 +543,13 @@ pub async fn collect_mcp_snapshot_from_manager_with_detail( .collect::>(); (name, resources) }) - .collect::>(); + .collect::>() +} - let resource_templates = resource_templates +fn convert_mcp_resource_templates( + resource_templates: HashMap>, +) -> HashMap> { + resource_templates .into_iter() .map(|(name, templates)| { let templates = templates @@ -534,13 +583,100 @@ pub async fn collect_mcp_snapshot_from_manager_with_detail( .collect::>(); (name, templates) }) + .collect::>() +} + +async fn collect_mcp_server_status_snapshot_from_manager( + mcp_connection_manager: &McpConnectionManager, + auth_status_entries: HashMap, + detail: McpSnapshotDetail, +) -> McpServerStatusSnapshot { + let (tools, resources, resource_templates) = tokio::join!( + mcp_connection_manager.list_all_tools(), + async { + if detail.include_resources() { + mcp_connection_manager.list_all_resources().await + } else { + HashMap::new() + } + }, + async { + if detail.include_resources() { + mcp_connection_manager.list_all_resource_templates().await + } else { + HashMap::new() + } + }, + ); + + let mut tools_by_server = HashMap::>::new(); + for (_qualified_name, tool_info) in tools { + let raw_tool_name = tool_info.tool.name.to_string(); + let Some(tool) = protocol_tool_from_rmcp_tool(&raw_tool_name, &tool_info.tool) else { + continue; + }; + let tool_name = tool.name.clone(); + tools_by_server + .entry(tool_info.server_name) + .or_default() + .insert(tool_name, tool); + } + + McpServerStatusSnapshot { + tools_by_server, + resources: convert_mcp_resources(resources), + resource_templates: convert_mcp_resource_templates(resource_templates), + auth_statuses: auth_statuses_from_entries(&auth_status_entries), + } +} + +pub async fn collect_mcp_snapshot_from_manager( + mcp_connection_manager: &McpConnectionManager, + auth_status_entries: HashMap, +) -> McpListToolsResponseEvent { + collect_mcp_snapshot_from_manager_with_detail( + mcp_connection_manager, + auth_status_entries, + McpSnapshotDetail::Full, + ) + .await +} + +pub async fn collect_mcp_snapshot_from_manager_with_detail( + mcp_connection_manager: &McpConnectionManager, + auth_status_entries: HashMap, + detail: McpSnapshotDetail, +) -> McpListToolsResponseEvent { + let (tools, resources, resource_templates) = tokio::join!( + mcp_connection_manager.list_all_tools(), + async { + if detail.include_resources() { + mcp_connection_manager.list_all_resources().await + } else { + HashMap::new() + } + }, + async { + if detail.include_resources() { + mcp_connection_manager.list_all_resource_templates().await + } else { + HashMap::new() + } + }, + ); + + let tools = tools + .into_iter() + .filter_map(|(name, tool)| { + protocol_tool_from_rmcp_tool(&name, &tool.tool).map(|tool| (name, tool)) + }) .collect::>(); McpListToolsResponseEvent { tools, - resources, - resource_templates, - auth_statuses, + resources: convert_mcp_resources(resources), + resource_templates: convert_mcp_resource_templates(resource_templates), + auth_statuses: auth_statuses_from_entries(&auth_status_entries), } } diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager.rs b/codex-rs/codex-mcp/src/mcp_connection_manager.rs index f0f292fcb8ea..d7a5a34d3d46 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager.rs @@ -3,8 +3,8 @@ //! The [`McpConnectionManager`] owns one [`codex_rmcp_client::RmcpClient`] per //! configured server (keyed by the *server name*). It offers convenience //! helpers to query the available tools across *all* servers and returns them -//! in a single aggregated map using the fully-qualified tool name -//! `""` as the key. +//! in a single aggregated map using the model-visible fully-qualified tool name +//! as the key. use std::borrow::Cow; use std::collections::HashMap; @@ -26,8 +26,8 @@ use crate::mcp::ToolPluginProvenance; use crate::mcp::configured_mcp_servers; use crate::mcp::effective_mcp_servers; use crate::mcp::mcp_permission_prompt_is_auto_approved; -use crate::mcp::sanitize_responses_api_tool_name; use crate::mcp::tool_plugin_provenance; +pub(crate) use crate::mcp_tool_names::qualify_tools; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; @@ -90,13 +90,8 @@ use codex_login::CodexAuth; use codex_utils_plugins::mcp_connector::is_connector_id_allowed; use codex_utils_plugins::mcp_connector::sanitize_name; -/// Delimiter used to separate the server name from the tool name in a fully -/// qualified tool name. -/// -/// OpenAI requires tool names to conform to `^[a-zA-Z0-9_-]+$`, so we must -/// choose a delimiter from this character set. +/// Delimiter used to separate MCP tool-name parts. const MCP_TOOL_NAME_DELIMITER: &str = "__"; -const MAX_TOOL_NAME_LENGTH: usize = 64; /// Default timeout for initializing MCP server & initially listing tools. pub const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(30); @@ -104,7 +99,7 @@ pub const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(30); /// Default timeout for individual tool calls. const DEFAULT_TOOL_TIMEOUT: Duration = Duration::from_secs(120); -const CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION: u8 = 1; +const CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION: u8 = 2; const CODEX_APPS_TOOLS_CACHE_DIR: &str = "cache/codex_apps_tools"; const MCP_TOOLS_LIST_DURATION_METRIC: &str = "codex.mcp.tools.list.duration_ms"; const MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC: &str = "codex.mcp.tools.fetch_uncached.duration_ms"; @@ -136,59 +131,20 @@ pub fn codex_apps_tools_cache_key(auth: Option<&CodexAuth>) -> CodexAppsToolsCac } } -fn qualify_tools(tools: I) -> HashMap -where - I: IntoIterator, -{ - let mut used_names = HashSet::new(); - let mut seen_raw_names = HashSet::new(); - let mut qualified_tools = HashMap::new(); - for tool in tools { - let qualified_name_raw = if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { - format!( - "mcp{}{}{}{}", - MCP_TOOL_NAME_DELIMITER, tool.server_name, MCP_TOOL_NAME_DELIMITER, tool.tool_name - ) - } else { - format!("{}{}", tool.tool_namespace, tool.tool_name) - }; - if !seen_raw_names.insert(qualified_name_raw.clone()) { - warn!("skipping duplicated tool {}", qualified_name_raw); - continue; - } - - // Start from a "pretty" name (sanitized), then deterministically disambiguate on - // collisions by appending a hash of the *raw* (unsanitized) qualified name. This - // ensures tools like `foo.bar` and `foo_bar` don't collapse to the same key. - let mut qualified_name = sanitize_responses_api_tool_name(&qualified_name_raw); - - // Enforce length constraints early; use the raw name for the hash input so the - // output remains stable even when sanitization changes. - if qualified_name.len() > MAX_TOOL_NAME_LENGTH { - let sha1_str = sha1_hex(&qualified_name_raw); - let prefix_len = MAX_TOOL_NAME_LENGTH - sha1_str.len(); - qualified_name = format!("{}{}", &qualified_name[..prefix_len], sha1_str); - } - - if used_names.contains(&qualified_name) { - warn!("skipping duplicated tool {}", qualified_name); - continue; - } - - used_names.insert(qualified_name.clone()); - qualified_tools.insert(qualified_name, tool); - } - - qualified_tools -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ToolInfo { + /// Raw MCP server name used for routing the tool call. pub server_name: String, - pub tool_name: String, - pub tool_namespace: String, + /// Model-visible tool name used in Responses API tool declarations. + #[serde(rename = "tool_name", alias = "callable_name")] + pub callable_name: String, + /// Model-visible namespace used for deferred tool loading. + #[serde(rename = "tool_namespace", alias = "callable_namespace")] + pub callable_namespace: String, + /// Instructions from the MCP server initialize result. #[serde(default)] pub server_instructions: Option, + /// Raw MCP tool definition; `tool.name` is sent back to the MCP server. pub tool: Tool, pub connector_id: Option, pub connector_name: Option, @@ -862,14 +818,14 @@ impl McpConnectionManager { /// fully-qualified name for the tool. #[instrument(level = "trace", skip_all)] pub async fn list_all_tools(&self) -> HashMap { - let mut tools = HashMap::new(); + let mut tools = Vec::new(); for managed_client in self.clients.values() { let Some(server_tools) = managed_client.listed_tools().await else { continue; }; - tools.extend(qualify_tools(server_tools)); + tools.extend(server_tools); } - tools + qualify_tools(tools) } /// Force-refresh codex apps tools by bypassing the in-process cache. @@ -1270,7 +1226,7 @@ fn normalize_codex_apps_tool_title( value.to_string() } -fn normalize_codex_apps_tool_name( +fn normalize_codex_apps_callable_name( server_name: &str, tool_name: &str, connector_id: Option<&str>, @@ -1305,10 +1261,13 @@ fn normalize_codex_apps_tool_name( tool_name } -fn normalize_codex_apps_namespace(server_name: &str, connector_name: Option<&str>) -> String { - if server_name != CODEX_APPS_MCP_SERVER_NAME { - server_name.to_string() - } else if let Some(connector_name) = connector_name { +fn normalize_codex_apps_callable_namespace( + server_name: &str, + connector_name: Option<&str>, +) -> String { + if server_name == CODEX_APPS_MCP_SERVER_NAME + && let Some(connector_name) = connector_name + { format!( "mcp{}{}{}{}", MCP_TOOL_NAME_DELIMITER, @@ -1317,7 +1276,7 @@ fn normalize_codex_apps_namespace(server_name: &str, connector_name: Option<&str sanitize_name(connector_name) ) } else { - server_name.to_string() + format!("mcp{MCP_TOOL_NAME_DELIMITER}{server_name}{MCP_TOOL_NAME_DELIMITER}") } } @@ -1649,14 +1608,16 @@ async fn list_tools_for_client_uncached( .tools .into_iter() .map(|tool| { - let tool_name = normalize_codex_apps_tool_name( + let callable_name = normalize_codex_apps_callable_name( server_name, &tool.tool.name, tool.connector_id.as_deref(), tool.connector_name.as_deref(), ); - let tool_namespace = - normalize_codex_apps_namespace(server_name, tool.connector_name.as_deref()); + let callable_namespace = normalize_codex_apps_callable_namespace( + server_name, + tool.connector_name.as_deref(), + ); let connector_name = tool.connector_name; let connector_description = tool.connector_description; let mut tool_def = tool.tool; @@ -1669,8 +1630,8 @@ async fn list_tools_for_client_uncached( } ToolInfo { server_name: server_name.to_owned(), - tool_name, - tool_namespace, + callable_name, + callable_namespace, server_instructions: server_instructions.map(str::to_string), tool: tool_def, connector_id: tool.connector_id, diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs index 76b16c959d1a..192058ddde26 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs @@ -9,14 +9,11 @@ use std::sync::Arc; use tempfile::tempdir; fn create_test_tool(server_name: &str, tool_name: &str) -> ToolInfo { + let tool_namespace = format!("mcp__{server_name}__"); ToolInfo { server_name: server_name.to_string(), - tool_name: tool_name.to_string(), - tool_namespace: if server_name == CODEX_APPS_MCP_SERVER_NAME { - format!("mcp__{server_name}__") - } else { - server_name.to_string() - }, + callable_name: tool_name.to_string(), + callable_namespace: tool_namespace, server_instructions: None, tool: Tool { name: tool_name.to_string().into(), @@ -213,16 +210,12 @@ fn test_qualify_tools_long_names_same_server() { let mut keys: Vec<_> = qualified_tools.keys().cloned().collect(); keys.sort(); - assert_eq!(keys[0].len(), 64); - assert_eq!( - keys[0], - "mcp__my_server__extremel119a2b97664e41363932dc84de21e2ff1b93b3e9" - ); - - assert_eq!(keys[1].len(), 64); - assert_eq!( - keys[1], - "mcp__my_server__yet_anot419a82a89325c1b477274a41f8c65ea5f3a7f341" + assert!(keys.iter().all(|key| key.len() == 64)); + assert!(keys.iter().all(|key| key.starts_with("mcp__my_server__"))); + assert!( + keys.iter() + .all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')), + "qualified names must be code-mode compatible: {keys:?}" ); } @@ -235,17 +228,94 @@ fn test_qualify_tools_sanitizes_invalid_characters() { assert_eq!(qualified_tools.len(), 1); let (qualified_name, tool) = qualified_tools.into_iter().next().expect("one tool"); assert_eq!(qualified_name, "mcp__server_one__tool_two_three"); + assert_eq!( + format!("{}{}", tool.callable_namespace, tool.callable_name), + qualified_name + ); - // The key is sanitized for OpenAI, but we keep original parts for the actual MCP call. + // The key and callable parts are sanitized for model-visible tool calls, but + // the raw MCP name is preserved for the actual MCP call. assert_eq!(tool.server_name, "server.one"); - assert_eq!(tool.tool_name, "tool.two-three"); + assert_eq!(tool.callable_namespace, "mcp__server_one__"); + assert_eq!(tool.callable_name, "tool_two_three"); + assert_eq!(tool.tool.name, "tool.two-three"); assert!( qualified_name .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-'), - "qualified name must be Responses API compatible: {qualified_name:?}" + .all(|c| c.is_ascii_alphanumeric() || c == '_'), + "qualified name must be code-mode compatible: {qualified_name:?}" + ); +} + +#[test] +fn test_qualify_tools_keeps_hyphenated_mcp_tools_callable() { + let tools = vec![create_test_tool("music-studio", "get-strudel-guide")]; + + let qualified_tools = qualify_tools(tools); + + assert_eq!(qualified_tools.len(), 1); + let (qualified_name, tool) = qualified_tools.into_iter().next().expect("one tool"); + assert_eq!(qualified_name, "mcp__music_studio__get_strudel_guide"); + assert_eq!(tool.callable_namespace, "mcp__music_studio__"); + assert_eq!(tool.callable_name, "get_strudel_guide"); + assert_eq!(tool.tool.name, "get-strudel-guide"); +} + +#[test] +fn test_qualify_tools_disambiguates_sanitized_namespace_collisions() { + let tools = vec![ + create_test_tool("basic-server", "lookup"), + create_test_tool("basic_server", "query"), + ]; + + let qualified_tools = qualify_tools(tools); + + assert_eq!(qualified_tools.len(), 2); + let mut namespaces = qualified_tools + .values() + .map(|tool| tool.callable_namespace.as_str()) + .collect::>(); + namespaces.sort(); + namespaces.dedup(); + assert_eq!(namespaces.len(), 2); + + let raw_servers = qualified_tools + .values() + .map(|tool| tool.server_name.as_str()) + .collect::>(); + assert_eq!(raw_servers, HashSet::from(["basic-server", "basic_server"])); + assert!( + qualified_tools + .keys() + .all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')), + "qualified names must be code-mode compatible: {qualified_tools:?}" + ); +} + +#[test] +fn test_qualify_tools_disambiguates_sanitized_tool_name_collisions() { + let tools = vec![ + create_test_tool("server", "tool-name"), + create_test_tool("server", "tool_name"), + ]; + + let qualified_tools = qualify_tools(tools); + + assert_eq!(qualified_tools.len(), 2); + let raw_tool_names = qualified_tools + .values() + .map(|tool| tool.tool.name.to_string()) + .collect::>(); + assert_eq!( + raw_tool_names, + HashSet::from(["tool-name".to_string(), "tool_name".to_string()]) ); + let callable_tool_names = qualified_tools + .values() + .map(|tool| tool.callable_name.as_str()) + .collect::>(); + assert_eq!(callable_tool_names.len(), 2); } #[test] @@ -312,7 +382,7 @@ fn filter_tools_applies_per_server_filters() { assert_eq!(filtered.len(), 1); assert_eq!(filtered[0].server_name, "server1"); - assert_eq!(filtered[0].tool_name, "tool_a"); + assert_eq!(filtered[0].callable_name, "tool_a"); } #[test] @@ -329,12 +399,12 @@ fn codex_apps_tools_cache_is_overwritten_by_last_write() { write_cached_codex_apps_tools(&cache_context, &tools_gateway_1); let cached_gateway_1 = read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for first write"); - assert_eq!(cached_gateway_1[0].tool_name, "one"); + assert_eq!(cached_gateway_1[0].callable_name, "one"); write_cached_codex_apps_tools(&cache_context, &tools_gateway_2); let cached_gateway_2 = read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for second write"); - assert_eq!(cached_gateway_2[0].tool_name, "two"); + assert_eq!(cached_gateway_2[0].callable_name, "two"); } #[test] @@ -361,8 +431,8 @@ fn codex_apps_tools_cache_is_scoped_per_user() { let read_user_2 = read_cached_codex_apps_tools(&cache_context_user_2).expect("cache entry for user two"); - assert_eq!(read_user_1[0].tool_name, "one"); - assert_eq!(read_user_2[0].tool_name, "two"); + assert_eq!(read_user_1[0].callable_name, "one"); + assert_eq!(read_user_2[0].callable_name, "two"); assert_ne!( cache_context_user_1.cache_path(), cache_context_user_2.cache_path(), @@ -397,7 +467,7 @@ fn codex_apps_tools_cache_filters_disallowed_connectors() { let cached = read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for user"); assert_eq!(cached.len(), 1); - assert_eq!(cached[0].tool_name, "allowed_tool"); + assert_eq!(cached[0].callable_name, "allowed_tool"); assert_eq!(cached[0].connector_id.as_deref(), Some("calendar")); } @@ -462,7 +532,7 @@ fn startup_cached_codex_apps_tools_loads_from_disk_cache() { assert_eq!(startup_tools.len(), 1); assert_eq!(startup_tools[0].server_name, CODEX_APPS_MCP_SERVER_NAME); - assert_eq!(startup_tools[0].tool_name, "calendar_search"); + assert_eq!(startup_tools[0].callable_name, "calendar_search"); } #[tokio::test] @@ -492,7 +562,7 @@ async fn list_all_tools_uses_startup_snapshot_while_client_is_pending() { .get("mcp__codex_apps__calendar_create_event") .expect("tool from startup cache"); assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME); - assert_eq!(tool.tool_name, "calendar_create_event"); + assert_eq!(tool.callable_name, "calendar_create_event"); } #[tokio::test] @@ -574,7 +644,7 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() { .get("mcp__codex_apps__calendar_create_event") .expect("tool from startup cache"); assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME); - assert_eq!(tool.tool_name, "calendar_create_event"); + assert_eq!(tool.callable_name, "calendar_create_event"); } #[test] diff --git a/codex-rs/codex-mcp/src/mcp_tool_names.rs b/codex-rs/codex-mcp/src/mcp_tool_names.rs new file mode 100644 index 000000000000..5a323dfe7163 --- /dev/null +++ b/codex-rs/codex-mcp/src/mcp_tool_names.rs @@ -0,0 +1,202 @@ +//! Allocates model-visible MCP tool names while preserving raw MCP identities. + +use std::collections::HashMap; +use std::collections::HashSet; + +use sha1::Digest; +use sha1::Sha1; +use tracing::warn; + +use crate::mcp::sanitize_responses_api_tool_name; +use crate::mcp_connection_manager::ToolInfo; + +const MCP_TOOL_NAME_DELIMITER: &str = "__"; +const MAX_TOOL_NAME_LENGTH: usize = 64; +const CALLABLE_NAME_HASH_LEN: usize = 12; + +fn sha1_hex(s: &str) -> String { + let mut hasher = Sha1::new(); + hasher.update(s.as_bytes()); + let sha1 = hasher.finalize(); + format!("{sha1:x}") +} + +fn callable_name_hash_suffix(raw_identity: &str) -> String { + let hash = sha1_hex(raw_identity); + format!("_{}", &hash[..CALLABLE_NAME_HASH_LEN]) +} + +fn append_hash_suffix(value: &str, raw_identity: &str) -> String { + format!("{value}{}", callable_name_hash_suffix(raw_identity)) +} + +fn append_namespace_hash_suffix(namespace: &str, raw_identity: &str) -> String { + if let Some(namespace) = namespace.strip_suffix(MCP_TOOL_NAME_DELIMITER) { + format!( + "{}{}{}", + namespace, + callable_name_hash_suffix(raw_identity), + MCP_TOOL_NAME_DELIMITER + ) + } else { + append_hash_suffix(namespace, raw_identity) + } +} + +fn truncate_name(value: &str, max_len: usize) -> String { + value.chars().take(max_len).collect() +} + +fn fit_callable_parts_with_hash( + namespace: &str, + tool_name: &str, + raw_identity: &str, +) -> (String, String) { + let suffix = callable_name_hash_suffix(raw_identity); + let max_tool_len = MAX_TOOL_NAME_LENGTH.saturating_sub(namespace.len()); + if max_tool_len >= suffix.len() { + let prefix_len = max_tool_len - suffix.len(); + return ( + namespace.to_string(), + format!("{}{}", truncate_name(tool_name, prefix_len), suffix), + ); + } + + let max_namespace_len = MAX_TOOL_NAME_LENGTH - suffix.len(); + (truncate_name(namespace, max_namespace_len), suffix) +} + +fn unique_callable_parts( + namespace: &str, + tool_name: &str, + raw_identity: &str, + used_names: &mut HashSet, +) -> (String, String, String) { + let qualified_name = format!("{namespace}{tool_name}"); + if qualified_name.len() <= MAX_TOOL_NAME_LENGTH && used_names.insert(qualified_name.clone()) { + return (namespace.to_string(), tool_name.to_string(), qualified_name); + } + + let mut attempt = 0_u32; + loop { + let hash_input = if attempt == 0 { + raw_identity.to_string() + } else { + format!("{raw_identity}\0{attempt}") + }; + let (namespace, tool_name) = + fit_callable_parts_with_hash(namespace, tool_name, &hash_input); + let qualified_name = format!("{namespace}{tool_name}"); + if used_names.insert(qualified_name.clone()) { + return (namespace, tool_name, qualified_name); + } + attempt = attempt.saturating_add(1); + } +} + +#[derive(Debug)] +struct CallableToolCandidate { + tool: ToolInfo, + raw_namespace_identity: String, + raw_tool_identity: String, + callable_namespace: String, + callable_name: String, +} + +/// Returns a qualified-name lookup for MCP tools. +/// +/// Raw MCP server/tool names are kept on each [`ToolInfo`] for protocol calls, while +/// `callable_namespace` / `callable_name` are sanitized and, when necessary, hashed so +/// every model-visible `mcp__namespace__tool` name is unique and <= 64 bytes. +pub(crate) fn qualify_tools(tools: I) -> HashMap +where + I: IntoIterator, +{ + let mut seen_raw_names = HashSet::new(); + let mut candidates = Vec::new(); + for tool in tools { + let raw_namespace_identity = format!( + "{}\0{}\0{}", + tool.server_name, + tool.callable_namespace, + tool.connector_id.as_deref().unwrap_or_default() + ); + let raw_tool_identity = format!( + "{}\0{}\0{}", + raw_namespace_identity, tool.callable_name, tool.tool.name + ); + if !seen_raw_names.insert(raw_tool_identity.clone()) { + warn!("skipping duplicated tool {}", tool.tool.name); + continue; + } + + candidates.push(CallableToolCandidate { + callable_namespace: sanitize_responses_api_tool_name(&tool.callable_namespace), + callable_name: sanitize_responses_api_tool_name(&tool.callable_name), + raw_namespace_identity, + raw_tool_identity, + tool, + }); + } + + let mut namespace_identities_by_base = HashMap::>::new(); + for candidate in &candidates { + namespace_identities_by_base + .entry(candidate.callable_namespace.clone()) + .or_default() + .insert(candidate.raw_namespace_identity.clone()); + } + let colliding_namespaces = namespace_identities_by_base + .into_iter() + .filter_map(|(namespace, identities)| (identities.len() > 1).then_some(namespace)) + .collect::>(); + for candidate in &mut candidates { + if colliding_namespaces.contains(&candidate.callable_namespace) { + candidate.callable_namespace = append_namespace_hash_suffix( + &candidate.callable_namespace, + &candidate.raw_namespace_identity, + ); + } + } + + let mut tool_identities_by_base = HashMap::<(String, String), HashSet>::new(); + for candidate in &candidates { + tool_identities_by_base + .entry(( + candidate.callable_namespace.clone(), + candidate.callable_name.clone(), + )) + .or_default() + .insert(candidate.raw_tool_identity.clone()); + } + let colliding_tools = tool_identities_by_base + .into_iter() + .filter_map(|(key, identities)| (identities.len() > 1).then_some(key)) + .collect::>(); + for candidate in &mut candidates { + if colliding_tools.contains(&( + candidate.callable_namespace.clone(), + candidate.callable_name.clone(), + )) { + candidate.callable_name = + append_hash_suffix(&candidate.callable_name, &candidate.raw_tool_identity); + } + } + + candidates.sort_by(|left, right| left.raw_tool_identity.cmp(&right.raw_tool_identity)); + + let mut used_names = HashSet::new(); + let mut qualified_tools = HashMap::new(); + for mut candidate in candidates { + let (callable_namespace, callable_name, qualified_name) = unique_callable_parts( + &candidate.callable_namespace, + &candidate.callable_name, + &candidate.raw_tool_identity, + &mut used_names, + ); + candidate.tool.callable_namespace = callable_namespace; + candidate.tool.callable_name = callable_name; + qualified_tools.insert(qualified_name, candidate.tool); + } + qualified_tools +} diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 77d41ba51cc4..b4db00493dd3 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -25,6 +25,7 @@ use crate::config::ManagedFeatures; use crate::connectors; use crate::exec_policy::ExecPolicyManager; use crate::installation_id::resolve_installation_id; +use crate::mcp_tool_exposure::build_mcp_tool_exposure; use crate::parse_turn_item; use crate::path_utils::normalize_for_native_workdir; use crate::realtime_conversation::RealtimeConversationManager; @@ -74,9 +75,7 @@ use codex_login::auth_env_telemetry::collect_auth_env_telemetry; use codex_login::default_client::originator; use codex_mcp::McpConnectionManager; use codex_mcp::SandboxState; -use codex_mcp::ToolInfo as McpToolInfo; use codex_mcp::codex_apps_tools_cache_key; -use codex_mcp::filter_non_codex_apps_mcp_tools_only; #[cfg(test)] use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig; use codex_models_manager::manager::ModelsManager; @@ -323,7 +322,6 @@ use crate::util::backoff; use crate::windows_sandbox::WindowsSandboxLevelExt; use codex_async_utils::OrCancelExt; use codex_git_utils::get_git_repo_root; -use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::compute_auth_statuses; use codex_mcp::with_codex_apps_mcp; use codex_otel::SessionTelemetry; @@ -442,8 +440,6 @@ pub(crate) const INITIAL_SUBMIT_ID: &str = ""; pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 512; const CYBER_VERIFY_URL: &str = "https://chatgpt.com/cyber"; const CYBER_SAFETY_URL: &str = "https://developers.openai.com/codex/concepts/cyber-safety"; -const DIRECT_APP_TOOL_EXPOSURE_THRESHOLD: usize = 100; - impl Codex { /// Spawn a new [`Codex`] and initialize the session. pub(crate) async fn spawn(args: CodexSpawnArgs) -> CodexResult { @@ -6600,35 +6596,6 @@ fn connector_inserted_in_messages( connector_count == 1 && skill_count == 0 && mention_names_lower.contains(&mention_slug) } -fn filter_codex_apps_mcp_tools( - mcp_tools: &HashMap, - connectors: &[connectors::AppInfo], - config: &Config, -) -> HashMap { - let allowed: HashSet<&str> = connectors - .iter() - .map(|connector| connector.id.as_str()) - .collect(); - - mcp_tools - .iter() - .filter(|(_, tool)| { - if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { - return false; - } - let Some(connector_id) = codex_apps_connector_id(tool) else { - return false; - }; - allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool) - }) - .map(|(name, tool)| (name.clone(), tool.clone())) - .collect() -} - -fn codex_apps_connector_id(tool: &McpToolInfo) -> Option<&str> { - tool.connector_id.as_deref() -} - pub(crate) fn build_prompt( input: Vec, router: &ToolRouter, @@ -6815,7 +6782,7 @@ pub(crate) async fn built_tools( ) -> CodexResult> { let mcp_connection_manager = sess.services.mcp_connection_manager.read().await; let has_mcp_servers = mcp_connection_manager.has_servers(); - let mut mcp_tools = mcp_connection_manager + let all_mcp_tools = mcp_connection_manager .list_all_tools() .or_cancel(cancellation_token) .await?; @@ -6830,7 +6797,7 @@ pub(crate) async fn built_tools( let apps_enabled = turn_context.apps_enabled(); let accessible_connectors = - apps_enabled.then(|| connectors::accessible_connectors_from_mcp_tools(&mcp_tools)); + apps_enabled.then(|| connectors::accessible_connectors_from_mcp_tools(&all_mcp_tools)); let accessible_connectors_with_enabled_state = accessible_connectors.as_ref().map(|connectors| { connectors::with_app_enabled_state(connectors.clone(), &turn_context.config) @@ -6876,59 +6843,34 @@ pub(crate) async fn built_tools( None }; - let app_tools = connectors.as_ref().map(|connectors| { - filter_codex_apps_mcp_tools(&mcp_tools, connectors, &turn_context.config) - }); - - if let Some(connectors) = connectors.as_ref() { + let explicitly_enabled = if let Some(connectors) = connectors.as_ref() { let skill_name_counts_lower = skills_outcome.map_or_else(HashMap::new, |outcome| { build_skill_name_counts(&outcome.skills, &outcome.disabled_paths).1 }); - let explicitly_enabled = filter_connectors_for_input( + filter_connectors_for_input( connectors, input, &effective_explicitly_enabled_connectors, &skill_name_counts_lower, - ); - - let mut selected_mcp_tools = filter_non_codex_apps_mcp_tools_only(&mcp_tools); - selected_mcp_tools.extend(filter_codex_apps_mcp_tools( - &mcp_tools, - explicitly_enabled.as_ref(), - &turn_context.config, - )); - - mcp_tools = selected_mcp_tools; - } - - // Expose app tools directly when tool_search is disabled, or when tool_search - // is enabled but the accessible app tool set stays below the direct-exposure threshold. - let expose_app_tools_directly = !turn_context.tools_config.search_tool - || app_tools - .as_ref() - .is_some_and(|tools| tools.len() < DIRECT_APP_TOOL_EXPOSURE_THRESHOLD); - if expose_app_tools_directly && let Some(app_tools) = app_tools.as_ref() { - mcp_tools.extend(app_tools.clone()); - } - let app_tools = if expose_app_tools_directly { - None + ) } else { - app_tools + Vec::new() }; - let mcp_tool_router_inputs = - has_mcp_servers.then(|| crate::tools::router::map_mcp_tool_infos(&mcp_tools)); + let mcp_tool_exposure = build_mcp_tool_exposure( + &all_mcp_tools, + connectors.as_deref(), + explicitly_enabled.as_slice(), + &turn_context.config, + &turn_context.tools_config, + ); + let direct_mcp_tools = has_mcp_servers.then_some(mcp_tool_exposure.direct_tools); Ok(Arc::new(ToolRouter::from_config( &turn_context.tools_config, ToolRouterParams { - mcp_tools: mcp_tool_router_inputs - .as_ref() - .map(|inputs| inputs.mcp_tools.clone()), - tool_namespaces: mcp_tool_router_inputs - .as_ref() - .map(|inputs| inputs.tool_namespaces.clone()), - app_tools, + deferred_mcp_tools: mcp_tool_exposure.deferred_tools, + mcp_tools: direct_mcp_tools, discoverable_tools, dynamic_tools: turn_context.dynamic_tools.as_slice(), }, diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 80b98004fb92..5b2a5a5487ce 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -10,11 +10,14 @@ use crate::config_loader::RequirementSource; use crate::config_loader::Sourced; use crate::exec::ExecCapturePolicy; use crate::function_tool::FunctionCallError; +use crate::mcp_tool_exposure::DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD; +use crate::mcp_tool_exposure::build_mcp_tool_exposure; use crate::shell::default_user_shell; use crate::tools::format_exec_output_str; use codex_features::Features; use codex_login::CodexAuth; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ToolInfo; use codex_model_provider_info::ModelProviderInfo; use codex_models_manager::bundled_models_response; @@ -305,8 +308,7 @@ fn test_tool_runtime(session: Arc, turn_context: Arc) -> T &turn_context.tools_config, crate::tools::router::ToolRouterParams { mcp_tools: None, - tool_namespaces: None, - app_tools: None, + deferred_mcp_tools: None, discoverable_tools: None, dynamic_tools: turn_context.dynamic_tools.as_slice(), }, @@ -407,13 +409,13 @@ fn make_mcp_tool( .map(|connector_name| format!("mcp__{server_name}__{connector_name}")) .unwrap_or_else(|| server_name.to_string()) } else { - server_name.to_string() + format!("mcp__{server_name}__") }; ToolInfo { server_name: server_name.to_string(), - tool_name: tool_name.to_string(), - tool_namespace, + callable_name: tool_name.to_string(), + callable_namespace: tool_namespace, server_instructions: None, tool: Tool { name: tool_name.to_string().into(), @@ -433,6 +435,42 @@ fn make_mcp_tool( } } +fn numbered_mcp_tools(count: usize) -> HashMap { + (0..count) + .map(|index| { + let tool_name = format!("tool_{index}"); + ( + format!("mcp__rmcp__{tool_name}"), + make_mcp_tool( + "rmcp", &tool_name, /*connector_id*/ None, /*connector_name*/ None, + ), + ) + }) + .collect() +} + +fn tools_config_for_mcp_tool_exposure(search_tool: bool) -> ToolsConfig { + let config = test_config(); + let model_info = ModelsManager::construct_model_info_offline_for_tests( + "gpt-5-codex", + &config.to_models_manager_config(), + ); + let features = Features::with_defaults(); + let available_models = Vec::new(); + let mut 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, + sandbox_policy: &SandboxPolicy::DangerFullAccess, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }); + tools_config.search_tool = search_tool; + tools_config +} + #[test] fn validated_network_policy_amendment_host_allows_normalized_match() { let amendment = NetworkPolicyAmendment { @@ -880,156 +918,93 @@ fn collect_explicit_app_ids_from_skill_items_skips_plain_mentions_with_skill_con } #[test] -fn non_app_mcp_tools_remain_visible_without_search_selection() { - let mcp_tools = HashMap::from([ - ( - "mcp__codex_apps__calendar_create_event".to_string(), - make_mcp_tool( - CODEX_APPS_MCP_SERVER_NAME, - "calendar_create_event", - Some("calendar"), - Some("Calendar"), - ), - ), - ( - "mcp__rmcp__echo".to_string(), - make_mcp_tool( - "rmcp", "echo", /*connector_id*/ None, /*connector_name*/ None, - ), - ), - ]); - - let mut selected_mcp_tools = mcp_tools - .iter() - .filter(|(_, tool)| tool.server_name != CODEX_APPS_MCP_SERVER_NAME) - .map(|(name, tool)| (name.clone(), tool.clone())) - .collect::>(); - - let connectors = connectors::accessible_connectors_from_mcp_tools(&mcp_tools); - let explicitly_enabled_connectors = HashSet::new(); - let connectors = filter_connectors_for_input( - &connectors, - &[user_message("run echo")], - &explicitly_enabled_connectors, - &HashMap::new(), - ); +fn mcp_tool_exposure_directly_exposes_small_effective_tool_sets() { let config = test_config(); - selected_mcp_tools.extend(filter_codex_apps_mcp_tools( + let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true); + let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1); + + let exposure = build_mcp_tool_exposure( &mcp_tools, - &connectors, + /*connectors*/ None, + &[], &config, - )); + &tools_config, + ); - let mut tool_names: Vec = selected_mcp_tools.into_keys().collect(); - tool_names.sort(); - assert_eq!(tool_names, vec!["mcp__rmcp__echo".to_string()]); + let mut direct_tool_names: Vec<_> = exposure.direct_tools.keys().cloned().collect(); + direct_tool_names.sort(); + let mut expected_tool_names: Vec<_> = mcp_tools.keys().cloned().collect(); + expected_tool_names.sort(); + assert_eq!(direct_tool_names, expected_tool_names); + assert!(exposure.deferred_tools.is_none()); } #[test] -fn search_tool_selection_keeps_codex_apps_tools_without_mentions() { - let selected_tool_names = [ - "mcp__codex_apps__calendar_create_event".to_string(), - "mcp__rmcp__echo".to_string(), - ]; - let mcp_tools = HashMap::from([ - ( - "mcp__codex_apps__calendar_create_event".to_string(), - make_mcp_tool( - CODEX_APPS_MCP_SERVER_NAME, - "calendar_create_event", - Some("calendar"), - Some("Calendar"), - ), - ), - ( - "mcp__rmcp__echo".to_string(), - make_mcp_tool( - "rmcp", "echo", /*connector_id*/ None, /*connector_name*/ None, - ), - ), - ]); - - let mut selected_mcp_tools = mcp_tools - .iter() - .filter(|(name, _)| selected_tool_names.contains(name)) - .map(|(name, tool)| (name.clone(), tool.clone())) - .collect::>(); - let connectors = connectors::accessible_connectors_from_mcp_tools(&mcp_tools); - let explicitly_enabled_connectors = HashSet::new(); - let connectors = filter_connectors_for_input( - &connectors, - &[user_message("run the selected tools")], - &explicitly_enabled_connectors, - &HashMap::new(), - ); +fn mcp_tool_exposure_searches_large_effective_tool_sets() { let config = test_config(); - selected_mcp_tools.extend(filter_codex_apps_mcp_tools( + let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true); + let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD); + + let exposure = build_mcp_tool_exposure( &mcp_tools, - &connectors, + /*connectors*/ None, + &[], &config, - )); - - let mut tool_names: Vec = selected_mcp_tools.into_keys().collect(); - tool_names.sort(); - assert_eq!( - tool_names, - vec![ - "mcp__codex_apps__calendar_create_event".to_string(), - "mcp__rmcp__echo".to_string(), - ] + &tools_config, ); + + assert!(exposure.direct_tools.is_empty()); + let deferred_tools = exposure + .deferred_tools + .as_ref() + .expect("large tool sets should be discoverable through tool_search"); + let mut deferred_tool_names: Vec<_> = deferred_tools.keys().cloned().collect(); + deferred_tool_names.sort(); + let mut expected_tool_names: Vec<_> = mcp_tools.keys().cloned().collect(); + expected_tool_names.sort(); + assert_eq!(deferred_tool_names, expected_tool_names); } #[test] -fn apps_mentions_add_codex_apps_tools_to_search_selected_set() { - let selected_tool_names = ["mcp__rmcp__echo".to_string()]; - let mcp_tools = HashMap::from([ - ( - "mcp__codex_apps__calendar_create_event".to_string(), - make_mcp_tool( - CODEX_APPS_MCP_SERVER_NAME, - "calendar_create_event", - Some("calendar"), - Some("Calendar"), - ), - ), - ( - "mcp__rmcp__echo".to_string(), - make_mcp_tool( - "rmcp", "echo", /*connector_id*/ None, /*connector_name*/ None, - ), +fn mcp_tool_exposure_directly_exposes_explicit_apps_in_large_search_sets() { + let config = test_config(); + let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true); + let mut mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1); + mcp_tools.extend([( + "mcp__codex_apps__calendar_create_event".to_string(), + make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "calendar_create_event", + Some("calendar"), + Some("Calendar"), ), - ]); + )]); + let connectors = vec![make_connector("calendar", "Calendar")]; - let mut selected_mcp_tools = mcp_tools - .iter() - .filter(|(name, _)| selected_tool_names.contains(name)) - .map(|(name, tool)| (name.clone(), tool.clone())) - .collect::>(); - let connectors = connectors::accessible_connectors_from_mcp_tools(&mcp_tools); - let explicitly_enabled_connectors = HashSet::new(); - let connectors = filter_connectors_for_input( - &connectors, - &[user_message("use $calendar and then echo the response")], - &explicitly_enabled_connectors, - &HashMap::new(), - ); - let config = test_config(); - selected_mcp_tools.extend(filter_codex_apps_mcp_tools( + let exposure = build_mcp_tool_exposure( &mcp_tools, - &connectors, + Some(connectors.as_slice()), + connectors.as_slice(), &config, - )); + &tools_config, + ); - let mut tool_names: Vec = selected_mcp_tools.into_keys().collect(); + let mut tool_names: Vec = exposure.direct_tools.into_keys().collect(); tool_names.sort(); assert_eq!( tool_names, - vec![ - "mcp__codex_apps__calendar_create_event".to_string(), - "mcp__rmcp__echo".to_string(), - ] + vec!["mcp__codex_apps__calendar_create_event".to_string()] + ); + assert_eq!( + exposure.deferred_tools.as_ref().map(HashMap::len), + Some(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD) ); + let deferred_tools = exposure + .deferred_tools + .as_ref() + .expect("large tool sets should be discoverable through tool_search"); + assert!(deferred_tools.contains_key("mcp__codex_apps__calendar_create_event")); + assert!(deferred_tools.contains_key("mcp__rmcp__tool_0")); } #[tokio::test] @@ -5297,14 +5272,12 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { .list_all_tools() .await }; - let app_tools = Some(tools.clone()); - let mcp_tool_router_inputs = crate::tools::router::map_mcp_tool_infos(&tools); + let deferred_mcp_tools = Some(tools.clone()); let router = ToolRouter::from_config( &turn_context.tools_config, crate::tools::router::ToolRouterParams { - mcp_tools: Some(mcp_tool_router_inputs.mcp_tools), - tool_namespaces: Some(mcp_tool_router_inputs.tool_namespaces), - app_tools, + deferred_mcp_tools, + mcp_tools: Some(tools), discoverable_tools: None, dynamic_tools: turn_context.dynamic_tools.as_slice(), }, diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 57be9f3a4484..3c6504111a23 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -110,8 +110,8 @@ fn codex_app_tool( ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: tool_name.to_string(), - tool_namespace, + callable_name: tool_name.to_string(), + callable_namespace: tool_namespace, server_instructions: None, tool: test_tool_definition(tool_name), connector_id: Some(connector_id.to_string()), @@ -189,8 +189,8 @@ fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() { "mcp__sample__echo".to_string(), ToolInfo { server_name: "sample".to_string(), - tool_name: "echo".to_string(), - tool_namespace: "sample".to_string(), + callable_name: "echo".to_string(), + callable_namespace: "sample".to_string(), server_instructions: None, tool: test_tool_definition("echo"), connector_id: None, @@ -314,8 +314,8 @@ fn accessible_connectors_from_mcp_tools_preserves_description() { "mcp__codex_apps__calendar_create_event".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "calendar_create_event".to_string(), - tool_namespace: "mcp__codex_apps__calendar".to_string(), + callable_name: "calendar_create_event".to_string(), + callable_namespace: "mcp__codex_apps__calendar".to_string(), server_instructions: None, tool: Tool { name: "calendar_create_event".to_string().into(), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 83232de734f1..7233e7431839 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -45,6 +45,7 @@ pub use landlock::spawn_command_under_linux_sandbox; pub(crate) mod mcp; mod mcp_skill_dependencies; mod mcp_tool_approval_templates; +mod mcp_tool_exposure; mod network_policy_decision; pub(crate) mod network_proxy_loader; pub use mcp::McpManager; diff --git a/codex-rs/core/src/mcp_tool_exposure.rs b/codex-rs/core/src/mcp_tool_exposure.rs new file mode 100644 index 000000000000..d7858c17f0d1 --- /dev/null +++ b/codex-rs/core/src/mcp_tool_exposure.rs @@ -0,0 +1,73 @@ +use std::collections::HashMap; +use std::collections::HashSet; + +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_mcp::ToolInfo as McpToolInfo; +use codex_mcp::filter_non_codex_apps_mcp_tools_only; +use codex_tools::ToolsConfig; + +use crate::config::Config; +use crate::connectors; + +pub(crate) const DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD: usize = 100; + +pub(crate) struct McpToolExposure { + pub(crate) direct_tools: HashMap, + pub(crate) deferred_tools: Option>, +} + +pub(crate) fn build_mcp_tool_exposure( + all_mcp_tools: &HashMap, + connectors: Option<&[connectors::AppInfo]>, + explicitly_enabled_connectors: &[connectors::AppInfo], + config: &Config, + tools_config: &ToolsConfig, +) -> McpToolExposure { + let mut deferred_tools = filter_non_codex_apps_mcp_tools_only(all_mcp_tools); + if let Some(connectors) = connectors { + deferred_tools.extend(filter_codex_apps_mcp_tools( + all_mcp_tools, + connectors, + config, + )); + } + + if !tools_config.search_tool || deferred_tools.len() < DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD { + return McpToolExposure { + direct_tools: deferred_tools, + deferred_tools: None, + }; + } + + let direct_tools = + filter_codex_apps_mcp_tools(all_mcp_tools, explicitly_enabled_connectors, config); + McpToolExposure { + direct_tools, + deferred_tools: Some(deferred_tools), + } +} + +fn filter_codex_apps_mcp_tools( + mcp_tools: &HashMap, + connectors: &[connectors::AppInfo], + config: &Config, +) -> HashMap { + let allowed: HashSet<&str> = connectors + .iter() + .map(|connector| connector.id.as_str()) + .collect(); + + mcp_tools + .iter() + .filter(|(_, tool)| { + if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { + return false; + } + let Some(connector_id) = tool.connector_id.as_deref() else { + return false; + }; + allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool) + }) + .map(|(name, tool)| (name.clone(), tool.clone())) + .collect() +} diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index e6a10b967de7..c0320387f4c4 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -258,14 +258,12 @@ async fn build_nested_router(exec: &ExecContext) -> ToolRouter { .await .list_all_tools() .await; - let mcp_tool_router_inputs = crate::tools::router::map_mcp_tool_infos(&mcp_tools); ToolRouter::from_config( &nested_tools_config, ToolRouterParams { - mcp_tools: Some(mcp_tool_router_inputs.mcp_tools), - tool_namespaces: Some(mcp_tool_router_inputs.tool_namespaces), - app_tools: None, + deferred_mcp_tools: None, + mcp_tools: Some(mcp_tools), discoverable_tools: None, dynamic_tools: exec.turn.dynamic_tools.as_slice(), }, diff --git a/codex-rs/core/src/tools/handlers/tool_search.rs b/codex-rs/core/src/tools/handlers/tool_search.rs index d1033c613591..1c715e43fca9 100644 --- a/codex-rs/core/src/tools/handlers/tool_search.rs +++ b/codex-rs/core/src/tools/handlers/tool_search.rs @@ -6,21 +6,36 @@ use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use bm25::Document; use bm25::Language; +use bm25::SearchEngine; use bm25::SearchEngineBuilder; use codex_mcp::ToolInfo; use codex_tools::TOOL_SEARCH_DEFAULT_LIMIT; use codex_tools::TOOL_SEARCH_TOOL_NAME; use codex_tools::ToolSearchResultSource; use codex_tools::collect_tool_search_output_tools; -use std::collections::HashMap; pub struct ToolSearchHandler { - tools: HashMap, + entries: Vec<(String, ToolInfo)>, + search_engine: SearchEngine, } impl ToolSearchHandler { - pub fn new(tools: HashMap) -> Self { - Self { tools } + pub fn new(tools: std::collections::HashMap) -> Self { + let mut entries: Vec<(String, ToolInfo)> = tools.into_iter().collect(); + entries.sort_by(|a, b| a.0.cmp(&b.0)); + + let documents: Vec> = entries + .iter() + .enumerate() + .map(|(idx, (name, info))| Document::new(idx, build_search_text(name, info))) + .collect(); + let search_engine = + SearchEngineBuilder::::with_documents(Language::English, documents).build(); + + Self { + entries, + search_engine, + } } } @@ -60,29 +75,20 @@ impl ToolHandler for ToolSearchHandler { )); } - let mut entries: Vec<(String, ToolInfo)> = self.tools.clone().into_iter().collect(); - entries.sort_by(|a, b| a.0.cmp(&b.0)); - - if entries.is_empty() { + if self.entries.is_empty() { return Ok(ToolSearchOutput { tools: Vec::new() }); } - let documents: Vec> = entries - .iter() - .enumerate() - .map(|(idx, (name, info))| Document::new(idx, build_search_text(name, info))) - .collect(); - let search_engine = - SearchEngineBuilder::::with_documents(Language::English, documents).build(); - let results = search_engine.search(query, limit); + let results = self.search_engine.search(query, limit); let tools = collect_tool_search_output_tools( results .into_iter() - .filter_map(|result| entries.get(result.document.id)) - .map(|(_name, tool)| ToolSearchResultSource { - tool_namespace: tool.tool_namespace.as_str(), - tool_name: tool.tool_name.as_str(), + .filter_map(|result| self.entries.get(result.document.id)) + .map(|(_, tool)| ToolSearchResultSource { + server_name: tool.server_name.as_str(), + tool_namespace: tool.callable_namespace.as_str(), + tool_name: tool.callable_name.as_str(), tool: &tool.tool, connector_name: tool.connector_name.as_deref(), connector_description: tool.connector_description.as_deref(), @@ -101,7 +107,8 @@ impl ToolHandler for ToolSearchHandler { fn build_search_text(name: &str, info: &ToolInfo) -> String { let mut parts = vec![ name.to_string(), - info.tool_name.clone(), + info.callable_name.clone(), + info.tool.name.to_string(), info.server_name.clone(), ]; @@ -129,6 +136,15 @@ fn build_search_text(name: &str, info: &ToolInfo) -> String { parts.push(connector_description.to_string()); } + parts.extend( + info.plugin_display_names + .iter() + .map(String::as_str) + .map(str::trim) + .filter(|name| !name.is_empty()) + .map(str::to_string), + ); + parts.extend( info.tool .input_schema diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 65ab56ec8db2..f3acf13d5335 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -1561,14 +1561,12 @@ impl JsReplManager { .await .list_all_tools() .await; - let mcp_tool_router_inputs = crate::tools::router::map_mcp_tool_infos(&mcp_tools); let router = ToolRouter::from_config( &exec.turn.tools_config, crate::tools::router::ToolRouterParams { - mcp_tools: Some(mcp_tool_router_inputs.mcp_tools), - tool_namespaces: Some(mcp_tool_router_inputs.tool_namespaces), - app_tools: None, + deferred_mcp_tools: None, + mcp_tools: Some(mcp_tools), discoverable_tools: None, dynamic_tools: exec.turn.dynamic_tools.as_slice(), }, diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 030152e0fd5b..ce59eebe0193 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -16,10 +16,8 @@ use codex_protocol::models::SearchToolCallParams; use codex_protocol::models::ShellToolCallParams; use codex_tools::ConfiguredToolSpec; use codex_tools::DiscoverableTool; -use codex_tools::ToolNamespace; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; -use rmcp::model::Tool; use std::collections::HashMap; use std::sync::Arc; use tracing::instrument; @@ -41,53 +39,24 @@ pub struct ToolRouter { } pub(crate) struct ToolRouterParams<'a> { - pub(crate) mcp_tools: Option>, - pub(crate) tool_namespaces: Option>, - pub(crate) app_tools: Option>, + pub(crate) mcp_tools: Option>, + pub(crate) deferred_mcp_tools: Option>, pub(crate) discoverable_tools: Option>, pub(crate) dynamic_tools: &'a [DynamicToolSpec], } -pub(crate) struct McpToolRouterInputs { - pub(crate) mcp_tools: HashMap, - pub(crate) tool_namespaces: HashMap, -} - -pub(crate) fn map_mcp_tool_infos(mcp_tools: &HashMap) -> McpToolRouterInputs { - McpToolRouterInputs { - mcp_tools: mcp_tools - .iter() - .map(|(name, tool)| (name.clone(), tool.tool.clone())) - .collect(), - tool_namespaces: mcp_tools - .iter() - .map(|(name, tool)| { - ( - name.clone(), - ToolNamespace { - name: tool.tool_namespace.clone(), - description: tool.server_instructions.clone(), - }, - ) - }) - .collect(), - } -} - impl ToolRouter { pub fn from_config(config: &ToolsConfig, params: ToolRouterParams<'_>) -> Self { let ToolRouterParams { mcp_tools, - tool_namespaces, - app_tools, + deferred_mcp_tools, discoverable_tools, dynamic_tools, } = params; let builder = build_specs_with_discoverable_tools( config, mcp_tools, - app_tools, - tool_namespaces, + deferred_mcp_tools, discoverable_tools, dynamic_tools, ); diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 0ce7601349fc..e54981b3b1c3 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -25,18 +25,12 @@ async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> { .await .list_all_tools() .await; - let app_tools = Some(mcp_tools.clone()); + let deferred_mcp_tools = Some(mcp_tools.clone()); let router = ToolRouter::from_config( &turn.tools_config, ToolRouterParams { - mcp_tools: Some( - mcp_tools - .into_iter() - .map(|(name, tool)| (name, tool.tool)) - .collect(), - ), - tool_namespaces: None, - app_tools, + deferred_mcp_tools, + mcp_tools: Some(mcp_tools), discoverable_tools: None, dynamic_tools: turn.dynamic_tools.as_slice(), }, @@ -84,18 +78,12 @@ async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()> .await .list_all_tools() .await; - let app_tools = Some(mcp_tools.clone()); + let deferred_mcp_tools = Some(mcp_tools.clone()); let router = ToolRouter::from_config( &turn.tools_config, ToolRouterParams { - mcp_tools: Some( - mcp_tools - .into_iter() - .map(|(name, tool)| (name, tool.tool)) - .collect(), - ), - tool_namespaces: None, - app_tools, + deferred_mcp_tools, + mcp_tools: Some(mcp_tools), discoverable_tools: None, dynamic_tools: turn.dynamic_tools.as_slice(), }, diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index ac1ab97f5a5e..bfad3cdb3531 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -5,13 +5,12 @@ 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::registry::ToolRegistryBuilder; -use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_tools::DiscoverableTool; use codex_tools::ToolHandlerKind; use codex_tools::ToolNamespace; -use codex_tools::ToolRegistryPlanAppTool; +use codex_tools::ToolRegistryPlanDeferredTool; use codex_tools::ToolRegistryPlanParams; use codex_tools::ToolUserShellType; use codex_tools::ToolsConfig; @@ -30,11 +29,36 @@ pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType { } } +struct McpToolPlanInputs { + mcp_tools: HashMap, + tool_namespaces: HashMap, +} + +fn map_mcp_tools_for_plan(mcp_tools: &HashMap) -> McpToolPlanInputs { + McpToolPlanInputs { + mcp_tools: mcp_tools + .iter() + .map(|(name, tool)| (name.clone(), tool.tool.clone())) + .collect(), + tool_namespaces: mcp_tools + .iter() + .map(|(name, tool)| { + ( + name.clone(), + ToolNamespace { + name: tool.callable_namespace.clone(), + description: tool.server_instructions.clone(), + }, + ) + }) + .collect(), + } +} + pub(crate) fn build_specs_with_discoverable_tools( config: &ToolsConfig, - mcp_tools: Option>, - app_tools: Option>, - tool_namespaces: Option>, + mcp_tools: Option>, + deferred_mcp_tools: Option>, discoverable_tools: Option>, dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { @@ -70,12 +94,13 @@ pub(crate) fn build_specs_with_discoverable_tools( use crate::tools::handlers::multi_agents_v2::WaitAgentHandler as WaitAgentHandlerV2; let mut builder = ToolRegistryBuilder::new(); - let app_tool_sources = app_tools.as_ref().map(|app_tools| { - app_tools + let mcp_tool_plan_inputs = mcp_tools.as_ref().map(map_mcp_tools_for_plan); + let deferred_mcp_tool_sources = deferred_mcp_tools.as_ref().map(|tools| { + tools .values() - .map(|tool| ToolRegistryPlanAppTool { - tool_name: tool.tool_name.as_str(), - tool_namespace: tool.tool_namespace.as_str(), + .map(|tool| ToolRegistryPlanDeferredTool { + tool_name: tool.callable_name.as_str(), + tool_namespace: tool.callable_namespace.as_str(), server_name: tool.server_name.as_str(), connector_name: tool.connector_name.as_deref(), connector_description: tool.connector_description.as_deref(), @@ -87,9 +112,13 @@ pub(crate) fn build_specs_with_discoverable_tools( let plan = build_tool_registry_plan( config, ToolRegistryPlanParams { - mcp_tools: mcp_tools.as_ref(), - tool_namespaces: tool_namespaces.as_ref(), - app_tools: app_tool_sources.as_deref(), + mcp_tools: mcp_tool_plan_inputs + .as_ref() + .map(|inputs| &inputs.mcp_tools), + deferred_mcp_tools: deferred_mcp_tool_sources.as_deref(), + tool_namespaces: mcp_tool_plan_inputs + .as_ref() + .map(|inputs| &inputs.tool_namespaces), discoverable_tools: discoverable_tools.as_deref(), dynamic_tools, default_agent_type_description: &default_agent_type_description, @@ -98,7 +127,6 @@ pub(crate) fn build_specs_with_discoverable_tools( min_timeout_ms: MIN_WAIT_TIMEOUT_MS, max_timeout_ms: MAX_WAIT_TIMEOUT_MS, }, - codex_apps_mcp_server_name: CODEX_APPS_MCP_SERVER_NAME, }, ); let shell_handler = Arc::new(ShellHandler); @@ -210,9 +238,9 @@ pub(crate) fn build_specs_with_discoverable_tools( } ToolHandlerKind::ToolSearch => { if tool_search_handler.is_none() { - tool_search_handler = app_tools + tool_search_handler = deferred_mcp_tools .as_ref() - .map(|app_tools| Arc::new(ToolSearchHandler::new(app_tools.clone()))); + .map(|tools| Arc::new(ToolSearchHandler::new(tools.clone()))); } if let Some(tool_search_handler) = tool_search_handler.as_ref() { builder.register_handler(handler.name, tool_search_handler.clone()); diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 2a2c11a917df..9e519607aefb 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -53,6 +53,20 @@ 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(), + callable_name: tool.name.to_string(), + callable_namespace: "mcp__test_server__".to_string(), + server_instructions: None, + tool, + connector_id: None, + connector_name: None, + plugin_display_names: Vec::new(), + connector_description: None, + } +} + fn discoverable_connector(id: &str, name: &str, description: &str) -> DiscoverableTool { let slug = name.replace(' ', "-").to_lowercase(); DiscoverableTool::Connector(Box::new(AppInfo { @@ -182,7 +196,7 @@ fn multi_agent_v2_spawn_agent_description(tools_config: &ToolsConfig) -> String let (tools, _) = build_specs( tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); @@ -208,15 +222,14 @@ fn model_info_from_models_json(slug: &str) -> ModelInfo { /// Builds the tool registry builder while collecting tool specs for later serialization. fn build_specs( config: &ToolsConfig, - mcp_tools: Option>, - app_tools: Option>, + mcp_tools: Option>, + deferred_mcp_tools: Option>, dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { build_specs_with_discoverable_tools( config, mcp_tools, - app_tools, - /*tool_namespaces*/ None, + deferred_mcp_tools, /*discoverable_tools*/ None, dynamic_tools, ) @@ -267,7 +280,7 @@ fn get_memory_requires_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); @@ -300,8 +313,7 @@ fn assert_model_tools( &tools_config, ToolRouterParams { mcp_tools: None, - tool_namespaces: None, - app_tools: None, + deferred_mcp_tools: None, discoverable_tools: None, dynamic_tools: &[], }, @@ -562,7 +574,7 @@ fn test_build_specs_default_shell_present() { let (tools, _) = build_specs( &tools_config, Some(HashMap::new()), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); @@ -708,8 +720,7 @@ fn tool_suggest_requires_apps_and_plugins_features() { let (tools, _) = build_specs_with_discoverable_tools( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, - /*tool_namespaces*/ None, + /*deferred_mcp_tools*/ None, discoverable_tools.clone(), &[], ) @@ -725,7 +736,7 @@ fn tool_suggest_requires_apps_and_plugins_features() { } #[test] -fn search_tool_description_handles_no_enabled_apps() { +fn search_tool_description_handles_no_enabled_mcp_tools() { let model_info = search_capable_model_info(); let mut features = Features::with_defaults(); features.enable(Feature::Apps); @@ -755,7 +766,7 @@ fn search_tool_description_handles_no_enabled_apps() { }; assert!(description.contains("None currently enabled.")); - assert!(!description.contains("{{app_descriptions}}")); + assert!(!description.contains("{{source_descriptions}}")); } #[test] @@ -783,8 +794,8 @@ fn search_tool_description_falls_back_to_connector_name_without_description() { "mcp__codex_apps__calendar_create_event".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "_create_event".to_string(), - tool_namespace: "mcp__codex_apps__calendar".to_string(), + callable_name: "_create_event".to_string(), + callable_namespace: "mcp__codex_apps__calendar".to_string(), server_instructions: None, tool: mcp_tool( "calendar_create_event", @@ -810,7 +821,7 @@ fn search_tool_description_falls_back_to_connector_name_without_description() { } #[test] -fn search_tool_registers_namespaced_app_tool_aliases() { +fn search_tool_registers_namespaced_mcp_tool_aliases() { let model_info = search_capable_model_info(); let mut features = Features::with_defaults(); features.enable(Feature::Apps); @@ -835,8 +846,8 @@ fn search_tool_registers_namespaced_app_tool_aliases() { "mcp__codex_apps__calendar_create_event".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "_create_event".to_string(), - tool_namespace: "mcp__codex_apps__calendar".to_string(), + callable_name: "_create_event".to_string(), + callable_namespace: "mcp__codex_apps__calendar".to_string(), server_instructions: None, tool: mcp_tool( "calendar-create-event", @@ -853,8 +864,8 @@ fn search_tool_registers_namespaced_app_tool_aliases() { "mcp__codex_apps__calendar_list_events".to_string(), ToolInfo { server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - tool_name: "_list_events".to_string(), - tool_namespace: "mcp__codex_apps__calendar".to_string(), + callable_name: "_list_events".to_string(), + callable_namespace: "mcp__codex_apps__calendar".to_string(), server_instructions: None, tool: mcp_tool( "calendar-list-events", @@ -867,15 +878,31 @@ fn search_tool_registers_namespaced_app_tool_aliases() { plugin_display_names: Vec::new(), }, ), + ( + "mcp__rmcp__echo".to_string(), + ToolInfo { + server_name: "rmcp".to_string(), + callable_name: "echo".to_string(), + callable_namespace: "mcp__rmcp__".to_string(), + server_instructions: None, + tool: mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})), + connector_id: None, + connector_name: None, + connector_description: None, + plugin_display_names: Vec::new(), + }, + ), ])), &[], ) .build(); - let alias = tool_handler_key("_create_event", Some("mcp__codex_apps__calendar")); + let app_alias = tool_handler_key("_create_event", Some("mcp__codex_apps__calendar")); + let mcp_alias = tool_handler_key("echo", Some("mcp__rmcp__")); assert!(registry.has_handler(TOOL_SEARCH_TOOL_NAME, /*namespace*/ None)); - assert!(registry.has_handler(alias.as_str(), /*namespace*/ None)); + assert!(registry.has_handler(app_alias.as_str(), /*namespace*/ None)); + assert!(registry.has_handler(mcp_alias.as_str(), /*namespace*/ None)); } #[test] @@ -900,7 +927,7 @@ fn test_mcp_tool_property_missing_type_defaults_to_string() { &tools_config, Some(HashMap::from([( "dash/search".to_string(), - mcp_tool( + mcp_tool_info(mcp_tool( "search", "Search docs", serde_json::json!({ @@ -909,9 +936,9 @@ fn test_mcp_tool_property_missing_type_defaults_to_string() { "query": {"description": "search query"} } }), - ), + )), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); @@ -960,16 +987,16 @@ fn test_mcp_tool_preserves_integer_schema() { &tools_config, Some(HashMap::from([( "dash/paginate".to_string(), - mcp_tool( + mcp_tool_info(mcp_tool( "paginate", "Pagination", serde_json::json!({ "type": "object", "properties": {"page": {"type": "integer"}} }), - ), + )), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); @@ -1019,16 +1046,16 @@ fn test_mcp_tool_array_without_items_gets_default_string_items() { &tools_config, Some(HashMap::from([( "dash/tags".to_string(), - mcp_tool( + mcp_tool_info(mcp_tool( "tags", "Tags", serde_json::json!({ "type": "object", "properties": {"tags": {"type": "array"}} }), - ), + )), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); @@ -1080,7 +1107,7 @@ fn test_mcp_tool_anyof_defaults_to_string() { &tools_config, Some(HashMap::from([( "dash/value".to_string(), - mcp_tool( + mcp_tool_info(mcp_tool( "value", "AnyOf Value", serde_json::json!({ @@ -1089,9 +1116,9 @@ fn test_mcp_tool_anyof_defaults_to_string() { "value": {"anyOf": [{"type": "string"}, {"type": "number"}]} } }), - ), + )), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); @@ -1145,7 +1172,7 @@ fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { &tools_config, Some(HashMap::from([( "test_server/do_something_cool".to_string(), - mcp_tool( + mcp_tool_info(mcp_tool( "do_something_cool", "Do something cool", serde_json::json!({ @@ -1171,9 +1198,9 @@ fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { } } }), - ), + )), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ) .build(); diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index d41785bd4eae..39402500a6fb 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -2,6 +2,8 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use anyhow::Result; +use codex_config::types::McpServerConfig; +use codex_config::types::McpServerTransportConfig; use codex_core::config::Config; use codex_features::Feature; use codex_login::CodexAuth; @@ -24,15 +26,18 @@ use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; +use core_test_support::stdio_server_bin; use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use std::collections::HashMap; +use std::time::Duration; const SEARCH_TOOL_DESCRIPTION_SNIPPETS: [&str; 2] = [ - "You have access to all the tools of the following apps/connectors", + "You have access to tools from the following MCP servers/connectors", "- Calendar: Plan events and manage your calendar.", ]; const TOOL_SEARCH_TOOL_NAME: &str = "tool_search"; @@ -165,7 +170,7 @@ async fn search_tool_flag_adds_tool_search() -> Result<()> { "parameters": { "type": "object", "properties": { - "query": {"type": "string", "description": "Search query for apps tools."}, + "query": {"type": "string", "description": "Search query for MCP tools."}, "limit": {"type": "number", "description": "Maximum number of tools to return (defaults to 8)."}, }, "required": ["query"], @@ -581,3 +586,123 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() - Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount_searchable(&server).await?; + let echo_call_id = "tool-search-echo"; + let image_call_id = "tool-search-image"; + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_tool_search_call( + echo_call_id, + &json!({ + "query": "Echo back the provided message and include environment data.", + "limit": 8, + }), + ), + ev_tool_search_call( + image_call_id, + &json!({ + "query": "Return a single image content block.", + "limit": 8, + }), + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let rmcp_test_server_bin = stdio_server_bin()?; + let mut builder = + configured_builder(apps_server.chatgpt_base_url.clone()).with_config(move |config| { + let mut servers = config.mcp_servers.get().clone(); + servers.insert( + "rmcp".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: rmcp_test_server_bin, + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: None, + }, + enabled: true, + required: false, + disabled_reason: None, + startup_timeout_sec: Some(Duration::from_secs(10)), + tool_timeout_sec: None, + enabled_tools: Some(vec!["echo".to_string(), "image".to_string()]), + disabled_tools: Some(vec!["image".to_string()]), + scopes: None, + oauth_resource: None, + tools: HashMap::new(), + }, + ); + config + .mcp_servers + .set(servers) + .expect("test mcp servers should accept any configuration"); + }); + let test = builder.build(&server).await?; + + test.submit_turn_with_policies( + "Find the rmcp echo and image tools.", + AskForApproval::Never, + SandboxPolicy::DangerFullAccess, + ) + .await?; + + let requests = mock.requests(); + assert_eq!(requests.len(), 2); + + let first_request_tools = tool_names(&requests[0].body_json()); + assert!( + first_request_tools + .iter() + .any(|name| name == TOOL_SEARCH_TOOL_NAME), + "first request should advertise tool_search: {first_request_tools:?}" + ); + assert!( + !first_request_tools + .iter() + .any(|name| name == "mcp__rmcp__echo"), + "non-app MCP tools should be hidden before search in large-search mode: {first_request_tools:?}" + ); + + let echo_tools = tool_search_output_tools(&requests[1], echo_call_id); + let rmcp_echo_tools = echo_tools + .iter() + .filter(|tool| tool.get("name").and_then(Value::as_str) == Some("mcp__rmcp__")) + .flat_map(|namespace| namespace.get("tools").and_then(Value::as_array)) + .flatten() + .filter_map(|tool| tool.get("name").and_then(Value::as_str).map(str::to_string)) + .collect::>(); + assert_eq!(rmcp_echo_tools, vec!["echo".to_string()]); + + let image_tools = tool_search_output_tools(&requests[1], image_call_id); + let found_rmcp_image_tool = image_tools + .iter() + .filter(|tool| tool.get("name").and_then(Value::as_str) == Some("mcp__rmcp__")) + .flat_map(|namespace| namespace.get("tools").and_then(Value::as_array)) + .flatten() + .any(|tool| tool.get("name").and_then(Value::as_str).is_some()); + assert!( + !found_rmcp_image_tool, + "disabled non-app MCP tools should not be searchable: {image_tools:?}" + ); + + Ok(()) +} diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 8bfd3c31214d..07d044521398 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -101,12 +101,12 @@ pub use tool_discovery::DiscoverableToolType; pub use tool_discovery::TOOL_SEARCH_DEFAULT_LIMIT; pub use tool_discovery::TOOL_SEARCH_TOOL_NAME; pub use tool_discovery::TOOL_SUGGEST_TOOL_NAME; -pub use tool_discovery::ToolSearchAppInfo; -pub use tool_discovery::ToolSearchAppSource; pub use tool_discovery::ToolSearchResultSource; +pub use tool_discovery::ToolSearchSource; +pub use tool_discovery::ToolSearchSourceInfo; pub use tool_discovery::ToolSuggestEntry; -pub use tool_discovery::collect_tool_search_app_infos; pub use tool_discovery::collect_tool_search_output_tools; +pub use tool_discovery::collect_tool_search_source_infos; pub use tool_discovery::collect_tool_suggest_entries; pub use tool_discovery::create_tool_search_tool; pub use tool_discovery::create_tool_suggest_tool; @@ -116,7 +116,7 @@ pub use tool_registry_plan_types::ToolHandlerKind; pub use tool_registry_plan_types::ToolHandlerSpec; pub use tool_registry_plan_types::ToolNamespace; pub use tool_registry_plan_types::ToolRegistryPlan; -pub use tool_registry_plan_types::ToolRegistryPlanAppTool; +pub use tool_registry_plan_types::ToolRegistryPlanDeferredTool; pub use tool_registry_plan_types::ToolRegistryPlanParams; pub use tool_spec::ConfiguredToolSpec; pub use tool_spec::ResponsesApiWebSearchFilters; diff --git a/codex-rs/tools/src/tool_discovery.rs b/codex-rs/tools/src/tool_discovery.rs index 7033160205f9..594d4011b862 100644 --- a/codex-rs/tools/src/tool_discovery.rs +++ b/codex-rs/tools/src/tool_discovery.rs @@ -16,13 +16,13 @@ pub const TOOL_SEARCH_DEFAULT_LIMIT: usize = 8; pub const TOOL_SUGGEST_TOOL_NAME: &str = "tool_suggest"; #[derive(Clone, Debug, PartialEq, Eq)] -pub struct ToolSearchAppInfo { +pub struct ToolSearchSourceInfo { pub name: String, pub description: Option, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct ToolSearchAppSource<'a> { +pub struct ToolSearchSource<'a> { pub server_name: &'a str, pub connector_name: Option<&'a str>, pub connector_description: Option<&'a str>, @@ -30,6 +30,7 @@ pub struct ToolSearchAppSource<'a> { #[derive(Clone, Copy, Debug, PartialEq)] pub struct ToolSearchResultSource<'a> { + pub server_name: &'a str, pub tool_namespace: &'a str, pub tool_name: &'a str, pub tool: &'a rmcp::model::Tool, @@ -143,11 +144,14 @@ pub struct ToolSuggestEntry { pub app_connector_ids: Vec, } -pub fn create_tool_search_tool(app_tools: &[ToolSearchAppInfo], default_limit: usize) -> ToolSpec { +pub fn create_tool_search_tool( + searchable_sources: &[ToolSearchSourceInfo], + default_limit: usize, +) -> ToolSpec { let properties = BTreeMap::from([ ( "query".to_string(), - JsonSchema::string(Some("Search query for apps tools.".to_string())), + JsonSchema::string(Some("Search query for MCP tools.".to_string())), ), ( "limit".to_string(), @@ -157,22 +161,22 @@ pub fn create_tool_search_tool(app_tools: &[ToolSearchAppInfo], default_limit: u ), ]); - let mut app_descriptions = BTreeMap::new(); - for app_tool in app_tools { - app_descriptions - .entry(app_tool.name.clone()) + let mut source_descriptions = BTreeMap::new(); + for source in searchable_sources { + source_descriptions + .entry(source.name.clone()) .and_modify(|existing: &mut Option| { if existing.is_none() { - *existing = app_tool.description.clone(); + *existing = source.description.clone(); } }) - .or_insert(app_tool.description.clone()); + .or_insert(source.description.clone()); } - let app_descriptions = if app_descriptions.is_empty() { + let source_descriptions = if source_descriptions.is_empty() { "None currently enabled.".to_string() } else { - app_descriptions + source_descriptions .into_iter() .map(|(name, description)| match description { Some(description) => format!("- {name}: {description}"), @@ -183,7 +187,7 @@ pub fn create_tool_search_tool(app_tools: &[ToolSearchAppInfo], default_limit: u }; let description = format!( - "# Apps (Connectors) tool discovery\n\nSearches over apps/connectors tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to all the tools of the following apps/connectors:\n{app_descriptions}\nSome of the tools may not have been provided to you upfront, and you should use this tool (`{TOOL_SEARCH_TOOL_NAME}`) to search for the required tools and load them for the apps mentioned above. For the apps mentioned above, always use `{TOOL_SEARCH_TOOL_NAME}` instead of `list_mcp_resources` or `list_mcp_resource_templates` for tool discovery." + "# MCP tool discovery\n\nSearches over MCP tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to tools from the following MCP servers/connectors:\n{source_descriptions}\nSome of the tools may not have been provided to you upfront, and you should use this tool (`{TOOL_SEARCH_TOOL_NAME}`) to search for the required MCP tools. For MCP tool discovery, always use `{TOOL_SEARCH_TOOL_NAME}` instead of `list_mcp_resources` or `list_mcp_resource_templates`." ); ToolSpec::ToolSearch { @@ -201,9 +205,12 @@ pub fn collect_tool_search_output_tools<'a>( tool_sources: impl IntoIterator>, ) -> Result, serde_json::Error> { let grouped = tool_sources.into_iter().fold( - BTreeMap::<&'a str, Vec>>::new(), + BTreeMap::)>>::new(), |mut grouped, tool| { - grouped.entry(tool.tool_namespace).or_default().push(tool); + grouped + .entry(tool.tool_namespace.to_string()) + .or_default() + .push((tool.tool_name.to_string(), tool)); grouped }, ); @@ -215,20 +222,28 @@ pub fn collect_tool_search_output_tools<'a>( }; let description = first_tool + .1 .connector_description .map(str::to_string) .or_else(|| { first_tool + .1 .connector_name .map(str::trim) .filter(|connector_name| !connector_name.is_empty()) .map(|connector_name| format!("Tools for working with {connector_name}.")) + }) + .or_else(|| { + Some(format!( + "Tools from the {} MCP server.", + first_tool.1.server_name + )) }); let tools = tools .iter() .map(|tool| { - mcp_tool_to_deferred_responses_api_tool(tool.tool_name.to_string(), tool.tool) + mcp_tool_to_deferred_responses_api_tool(tool.0.clone(), tool.1.tool) .map(ResponsesApiNamespaceTool::Function) }) .collect::, _>>()?; @@ -243,25 +258,36 @@ pub fn collect_tool_search_output_tools<'a>( Ok(results) } -pub fn collect_tool_search_app_infos<'a>( - app_tools: impl IntoIterator>, - codex_apps_server_name: &str, -) -> Vec { - app_tools +pub fn collect_tool_search_source_infos<'a>( + searchable_tools: impl IntoIterator>, +) -> Vec { + searchable_tools .into_iter() - .filter(|tool| tool.server_name == codex_apps_server_name) .filter_map(|tool| { - let name = tool + if let Some(name) = tool .connector_name .map(str::trim) - .filter(|connector_name| !connector_name.is_empty())? - .to_string(); - let description = tool - .connector_description - .map(str::trim) - .filter(|connector_description| !connector_description.is_empty()) - .map(str::to_string); - Some(ToolSearchAppInfo { name, description }) + .filter(|connector_name| !connector_name.is_empty()) + { + return Some(ToolSearchSourceInfo { + name: name.to_string(), + description: tool + .connector_description + .map(str::trim) + .filter(|description| !description.is_empty()) + .map(str::to_string), + }); + } + + let name = tool.server_name.trim(); + if name.is_empty() { + return None; + } + + Some(ToolSearchSourceInfo { + name: name.to_string(), + description: None, + }) }) .collect() } diff --git a/codex-rs/tools/src/tool_discovery_tests.rs b/codex-rs/tools/src/tool_discovery_tests.rs index ea5acf9aa6c9..dbbf56c3cdb8 100644 --- a/codex-rs/tools/src/tool_discovery_tests.rs +++ b/codex-rs/tools/src/tool_discovery_tests.rs @@ -26,23 +26,23 @@ fn mcp_tool(name: &str, description: &str) -> Tool { } #[test] -fn create_tool_search_tool_deduplicates_and_renders_enabled_apps() { +fn create_tool_search_tool_deduplicates_and_renders_enabled_sources() { assert_eq!( create_tool_search_tool( &[ - ToolSearchAppInfo { + ToolSearchSourceInfo { name: "Google Drive".to_string(), description: Some( "Use Google Drive as the single entrypoint for Drive, Docs, Sheets, and Slides work." .to_string(), ), }, - ToolSearchAppInfo { + ToolSearchSourceInfo { name: "Google Drive".to_string(), description: None, }, - ToolSearchAppInfo { - name: "Slack".to_string(), + ToolSearchSourceInfo { + name: "docs".to_string(), description: None, }, ], @@ -50,7 +50,7 @@ fn create_tool_search_tool_deduplicates_and_renders_enabled_apps() { ), ToolSpec::ToolSearch { execution: "client".to_string(), - description: "# Apps (Connectors) tool discovery\n\nSearches over apps/connectors tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to all the tools of the following apps/connectors:\n- Google Drive: Use Google Drive as the single entrypoint for Drive, Docs, Sheets, and Slides work.\n- Slack\nSome of the tools may not have been provided to you upfront, and you should use this tool (`tool_search`) to search for the required tools and load them for the apps mentioned above. For the apps mentioned above, always use `tool_search` instead of `list_mcp_resources` or `list_mcp_resource_templates` for tool discovery.".to_string(), + description: "# MCP tool discovery\n\nSearches over MCP tool metadata with BM25 and exposes matching tools for the next model call.\n\nYou have access to tools from the following MCP servers/connectors:\n- Google Drive: Use Google Drive as the single entrypoint for Drive, Docs, Sheets, and Slides work.\n- docs\nSome of the tools may not have been provided to you upfront, and you should use this tool (`tool_search`) to search for the required MCP tools. For MCP tool discovery, always use `tool_search` instead of `list_mcp_resources` or `list_mcp_resource_templates`.".to_string(), parameters: JsonSchema::object(BTreeMap::from([ ( "limit".to_string(), @@ -61,7 +61,7 @@ fn create_tool_search_tool_deduplicates_and_renders_enabled_apps() { ), ( "query".to_string(), - JsonSchema::string(Some("Search query for apps tools.".to_string()),), + JsonSchema::string(Some("Search query for MCP tools.".to_string()),), ), ]), Some(vec!["query".to_string()]), Some(false.into())), } @@ -141,9 +141,11 @@ fn collect_tool_search_output_tools_groups_results_by_namespace() { let calendar_create_event = mcp_tool("calendar-create-event", "Create a calendar event."); let gmail_read_email = mcp_tool("gmail-read-email", "Read an email."); let calendar_list_events = mcp_tool("calendar-list-events", "List calendar events."); + let docs_search = mcp_tool("search", "Search docs."); let tools = collect_tool_search_output_tools([ ToolSearchResultSource { + server_name: "codex_apps", tool_namespace: "mcp__codex_apps__calendar", tool_name: "_create_event", tool: &calendar_create_event, @@ -151,6 +153,7 @@ fn collect_tool_search_output_tools_groups_results_by_namespace() { connector_description: Some("Plan events"), }, ToolSearchResultSource { + server_name: "codex_apps", tool_namespace: "mcp__codex_apps__gmail", tool_name: "_read_email", tool: &gmail_read_email, @@ -158,12 +161,21 @@ fn collect_tool_search_output_tools_groups_results_by_namespace() { connector_description: Some("Read mail"), }, ToolSearchResultSource { + server_name: "codex_apps", tool_namespace: "mcp__codex_apps__calendar", tool_name: "_list_events", tool: &calendar_list_events, connector_name: Some("Calendar"), connector_description: Some("Plan events"), }, + ToolSearchResultSource { + server_name: "docs", + tool_namespace: "mcp__docs__", + tool_name: "search", + tool: &docs_search, + connector_name: None, + connector_description: None, + }, ]) .expect("collect tool search output tools"); @@ -216,6 +228,22 @@ fn collect_tool_search_output_tools_groups_results_by_namespace() { output_schema: None, })], }), + ToolSearchOutputTool::Namespace(ResponsesApiNamespace { + name: "mcp__docs__".to_string(), + description: "Tools from the docs MCP server.".to_string(), + tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool { + name: "search".to_string(), + description: "Search docs.".to_string(), + strict: false, + defer_loading: Some(true), + parameters: JsonSchema::object( + Default::default(), + /*required*/ None, + /*additional_properties*/ None + ), + output_schema: None, + })], + }), ], ); } @@ -225,6 +253,7 @@ fn collect_tool_search_output_tools_falls_back_to_connector_name_description() { let gmail_batch_read_email = mcp_tool("gmail-batch-read-email", "Read multiple emails."); let tools = collect_tool_search_output_tools([ToolSearchResultSource { + server_name: "codex_apps", tool_namespace: "mcp__codex_apps__gmail", tool_name: "_batch_read_email", tool: &gmail_batch_read_email, diff --git a/codex-rs/tools/src/tool_registry_plan.rs b/codex-rs/tools/src/tool_registry_plan.rs index ddec91c05485..1377324e637d 100644 --- a/codex-rs/tools/src/tool_registry_plan.rs +++ b/codex-rs/tools/src/tool_registry_plan.rs @@ -8,13 +8,13 @@ use crate::TOOL_SUGGEST_TOOL_NAME; use crate::ToolHandlerKind; use crate::ToolRegistryPlan; use crate::ToolRegistryPlanParams; -use crate::ToolSearchAppSource; +use crate::ToolSearchSource; use crate::ToolSpec; use crate::ToolsConfig; use crate::ViewImageToolOptions; use crate::WebSearchToolOptions; use crate::collect_code_mode_tool_definitions; -use crate::collect_tool_search_app_infos; +use crate::collect_tool_search_source_infos; use crate::collect_tool_suggest_entries; use crate::create_apply_patch_freeform_tool; use crate::create_apply_patch_json_tool; @@ -250,24 +250,24 @@ pub fn build_tool_registry_plan( } if config.search_tool - && let Some(app_tools) = params.app_tools + && let Some(deferred_mcp_tools) = params.deferred_mcp_tools { - let search_app_infos = collect_tool_search_app_infos( - app_tools.iter().map(|tool| ToolSearchAppSource { - server_name: tool.server_name, - connector_name: tool.connector_name, - connector_description: tool.connector_description, - }), - params.codex_apps_mcp_server_name, - ); + let search_source_infos = + collect_tool_search_source_infos(deferred_mcp_tools.iter().map(|tool| { + ToolSearchSource { + server_name: tool.server_name, + connector_name: tool.connector_name, + connector_description: tool.connector_description, + } + })); plan.push_spec( - create_tool_search_tool(&search_app_infos, TOOL_SEARCH_DEFAULT_LIMIT), + create_tool_search_tool(&search_source_infos, TOOL_SEARCH_DEFAULT_LIMIT), /*supports_parallel_tool_calls*/ true, config.code_mode_enabled, ); plan.register_handler(TOOL_SEARCH_TOOL_NAME, ToolHandlerKind::ToolSearch); - for tool in app_tools { + for tool in deferred_mcp_tools { plan.register_handler( format!("{}:{}", tool.tool_namespace, tool.tool_name), ToolHandlerKind::Mcp, diff --git a/codex-rs/tools/src/tool_registry_plan_tests.rs b/codex-rs/tools/src/tool_registry_plan_tests.rs index 7a66eaf0e43d..13ef18495e5f 100644 --- a/codex-rs/tools/src/tool_registry_plan_tests.rs +++ b/codex-rs/tools/src/tool_registry_plan_tests.rs @@ -12,7 +12,7 @@ use crate::ResponsesApiWebSearchFilters; use crate::ResponsesApiWebSearchUserLocation; use crate::ToolHandlerSpec; use crate::ToolNamespace; -use crate::ToolRegistryPlanAppTool; +use crate::ToolRegistryPlanDeferredTool; use crate::ToolsConfigParams; use crate::WaitAgentTimeoutOptions; use crate::mcp_call_tool_result_output_schema; @@ -60,7 +60,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { let (tools, _) = build_specs( &config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -162,7 +162,7 @@ fn test_build_specs_collab_tools_enabled() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -202,7 +202,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -345,7 +345,7 @@ fn test_build_specs_enable_fanout_enables_agent_jobs_and_collab_tools() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -380,7 +380,7 @@ fn view_image_tool_omits_detail_without_original_detail_feature() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let view_image = find_tool(&tools, VIEW_IMAGE_TOOL_NAME); @@ -411,7 +411,7 @@ fn view_image_tool_includes_detail_with_original_detail_feature() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let view_image = find_tool(&tools, VIEW_IMAGE_TOOL_NAME); @@ -453,7 +453,7 @@ fn disabled_environment_omits_environment_backed_tools() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -489,7 +489,7 @@ fn test_build_specs_agent_job_worker_tools_enabled() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -526,7 +526,7 @@ fn request_user_input_description_reflects_default_mode_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let request_user_input_tool = find_tool(&tools, REQUEST_USER_INPUT_TOOL_NAME); @@ -549,7 +549,7 @@ fn request_user_input_description_reflects_default_mode_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let request_user_input_tool = find_tool(&tools, REQUEST_USER_INPUT_TOOL_NAME); @@ -577,7 +577,7 @@ fn request_permissions_requires_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); assert_lacks_tool_name(&tools, "request_permissions"); @@ -597,7 +597,7 @@ fn request_permissions_requires_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let request_permissions_tool = find_tool(&tools, "request_permissions"); @@ -626,7 +626,7 @@ fn request_permissions_tool_is_independent_from_additional_permissions() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -652,7 +652,7 @@ fn js_repl_requires_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -686,7 +686,7 @@ fn js_repl_enabled_adds_tools() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -717,7 +717,7 @@ fn image_generation_tools_require_feature_and_supported_model() { let (default_tools, _) = build_specs( &default_tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); assert!( @@ -740,7 +740,7 @@ fn image_generation_tools_require_feature_and_supported_model() { let (supported_tools, _) = build_specs( &supported_tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); assert_contains_tool_names(&supported_tools, &["image_generation"]); @@ -766,7 +766,7 @@ fn image_generation_tools_require_feature_and_supported_model() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); assert!( @@ -796,7 +796,7 @@ fn web_search_mode_cached_sets_external_web_access_false() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -832,7 +832,7 @@ fn web_search_mode_live_sets_external_web_access_true() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -882,7 +882,7 @@ fn web_search_config_is_forwarded_to_tool_spec() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -923,7 +923,7 @@ fn web_search_tool_type_text_and_image_sets_search_content_types() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -958,7 +958,7 @@ fn mcp_resource_tools_are_hidden_without_mcp_servers() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -989,7 +989,7 @@ fn mcp_resource_tools_are_included_when_mcp_servers_are_present() { let (tools, _) = build_specs( &tools_config, Some(HashMap::new()), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -1023,7 +1023,7 @@ fn test_parallel_support_flags() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -1050,7 +1050,7 @@ fn test_test_model_info_includes_sync_tool() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -1098,7 +1098,7 @@ fn test_build_specs_mcp_tools_converted() { }), ), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -1181,7 +1181,12 @@ fn test_build_specs_mcp_tools_sorted_by_name() { ), ]); - let (tools, _) = build_specs(&tools_config, Some(tools_map), /*app_tools*/ None, &[]); + let (tools, _) = build_specs( + &tools_config, + Some(tools_map), + /*deferred_mcp_tools*/ None, + &[], + ); let mcp_names: Vec<_> = tools .iter() @@ -1197,7 +1202,7 @@ fn test_build_specs_mcp_tools_sorted_by_name() { } #[test] -fn search_tool_description_lists_each_codex_apps_connector_once() { +fn search_tool_description_lists_each_mcp_source_once() { let model_info = search_capable_model_info(); let mut features = Features::with_defaults(); features.enable(Feature::Apps); @@ -1214,7 +1219,7 @@ fn search_tool_description_lists_each_codex_apps_connector_once() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let (tools, handlers) = build_specs( &tools_config, Some(HashMap::from([ ( @@ -1231,29 +1236,32 @@ fn search_tool_description_lists_each_codex_apps_connector_once() { ), ])), Some(vec![ - app_tool( + deferred_mcp_tool( "_create_event", "mcp__codex_apps__calendar", CODEX_APPS_MCP_SERVER_NAME, Some("Calendar"), Some("Plan events and manage your calendar."), ), - app_tool( + deferred_mcp_tool( "_list_events", "mcp__codex_apps__calendar", CODEX_APPS_MCP_SERVER_NAME, Some("Calendar"), Some("Plan events and manage your calendar."), ), - app_tool( + deferred_mcp_tool( "_search_threads", "mcp__codex_apps__gmail", CODEX_APPS_MCP_SERVER_NAME, Some("Gmail"), Some("Find and summarize email threads."), ), - app_tool( - "echo", "rmcp", "rmcp", /*connector_name*/ None, + deferred_mcp_tool( + "echo", + "mcp__rmcp__", + "rmcp", + /*connector_name*/ None, /*connector_description*/ None, ), ]), @@ -1273,14 +1281,24 @@ fn search_tool_description_lists_each_codex_apps_connector_once() { .count(), 1 ); + assert!(description.contains("- rmcp")); assert!(!description.contains("mcp__rmcp__echo")); + + assert!(handlers.contains(&ToolHandlerSpec { + name: "mcp__codex_apps__calendar:_create_event".to_string(), + kind: ToolHandlerKind::Mcp, + })); + assert!(handlers.contains(&ToolHandlerSpec { + name: "mcp__rmcp__:echo".to_string(), + kind: ToolHandlerKind::Mcp, + })); } #[test] fn search_tool_requires_model_capability_and_feature_flag() { let model_info = search_capable_model_info(); - let app_tools = Some(vec![app_tool( - "calendar_create_event", + let deferred_mcp_tools = Some(vec![deferred_mcp_tool( + "_create_event", "mcp__codex_apps__calendar", CODEX_APPS_MCP_SERVER_NAME, Some("Calendar"), @@ -1305,7 +1323,7 @@ fn search_tool_requires_model_capability_and_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - app_tools.clone(), + deferred_mcp_tools.clone(), &[], ); assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); @@ -1323,7 +1341,7 @@ fn search_tool_requires_model_capability_and_feature_flag() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - app_tools.clone(), + deferred_mcp_tools.clone(), &[], ); assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); @@ -1340,7 +1358,12 @@ fn search_tool_requires_model_capability_and_feature_flag() { sandbox_policy: &SandboxPolicy::DangerFullAccess, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs(&tools_config, /*mcp_tools*/ None, app_tools, &[]); + let (tools, _) = build_specs( + &tools_config, + /*mcp_tools*/ None, + deferred_mcp_tools, + &[], + ); assert_contains_tool_names(&tools, &[TOOL_SEARCH_TOOL_NAME]); } @@ -1366,8 +1389,7 @@ fn tool_suggest_is_not_registered_without_feature_flag() { let (tools, _) = build_specs_with_discoverable_tools( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, - /*tool_namespaces*/ None, + /*deferred_mcp_tools*/ None, Some(vec![discoverable_connector( "connector_2128aebfecb84f64a069897515042a44", "Google Calendar", @@ -1407,8 +1429,7 @@ fn tool_suggest_can_be_registered_without_search_tool() { let (tools, _) = build_specs_with_discoverable_tools( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, - /*tool_namespaces*/ None, + /*deferred_mcp_tools*/ None, Some(vec![discoverable_connector( "connector_2128aebfecb84f64a069897515042a44", "Google Calendar", @@ -1476,8 +1497,7 @@ fn tool_suggest_description_lists_discoverable_tools() { let (tools, _) = build_specs_with_discoverable_tools( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, - /*tool_namespaces*/ None, + /*deferred_mcp_tools*/ None, Some(discoverable_tools), &[], ); @@ -1572,7 +1592,7 @@ fn code_mode_augments_mcp_tool_descriptions_with_namespaced_sample() { }), ), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -1661,7 +1681,7 @@ fn code_mode_preserves_nullable_and_literal_mcp_input_shapes() { }), ), )])), - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); @@ -1700,7 +1720,7 @@ fn code_mode_augments_builtin_tool_descriptions_with_typed_sample() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let ToolSpec::Function(ResponsesApiTool { description, .. }) = @@ -1736,7 +1756,7 @@ fn code_mode_only_exec_description_includes_full_nested_tool_details() { let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let ToolSpec::Freeform(FreeformTool { description, .. }) = &find_tool(&tools, "exec").spec @@ -1773,7 +1793,7 @@ fn code_mode_exec_description_omits_nested_tool_details_when_not_code_mode_only( let (tools, _) = build_specs( &tools_config, /*mcp_tools*/ None, - /*app_tools*/ None, + /*deferred_mcp_tools*/ None, &[], ); let ToolSpec::Freeform(FreeformTool { description, .. }) = &find_tool(&tools, "exec").spec @@ -1833,14 +1853,13 @@ fn search_capable_model_info() -> ModelInfo { fn build_specs<'a>( config: &ToolsConfig, mcp_tools: Option>, - app_tools: Option>>, + deferred_mcp_tools: Option>>, dynamic_tools: &[DynamicToolSpec], ) -> (Vec, Vec) { build_specs_with_discoverable_tools( config, mcp_tools, - app_tools, - /*tool_namespaces*/ None, + deferred_mcp_tools, /*discoverable_tools*/ None, dynamic_tools, ) @@ -1849,16 +1868,15 @@ fn build_specs<'a>( fn build_specs_with_discoverable_tools<'a>( config: &ToolsConfig, mcp_tools: Option>, - app_tools: Option>>, - tool_namespaces: Option>, + deferred_mcp_tools: Option>>, discoverable_tools: Option>, dynamic_tools: &[DynamicToolSpec], ) -> (Vec, Vec) { build_specs_with_optional_tool_namespaces( config, mcp_tools, - tool_namespaces, - app_tools, + deferred_mcp_tools, + /*tool_namespaces*/ None, discoverable_tools, dynamic_tools, ) @@ -1867,8 +1885,8 @@ fn build_specs_with_discoverable_tools<'a>( fn build_specs_with_optional_tool_namespaces<'a>( config: &ToolsConfig, mcp_tools: Option>, + deferred_mcp_tools: Option>>, tool_namespaces: Option>, - app_tools: Option>>, discoverable_tools: Option>, dynamic_tools: &[DynamicToolSpec], ) -> (Vec, Vec) { @@ -1876,13 +1894,12 @@ fn build_specs_with_optional_tool_namespaces<'a>( config, ToolRegistryPlanParams { mcp_tools: mcp_tools.as_ref(), + deferred_mcp_tools: deferred_mcp_tools.as_deref(), tool_namespaces: tool_namespaces.as_ref(), - app_tools: app_tools.as_deref(), discoverable_tools: discoverable_tools.as_deref(), dynamic_tools, default_agent_type_description: DEFAULT_AGENT_TYPE_DESCRIPTION, wait_agent_timeouts: wait_agent_timeout_options(), - codex_apps_mcp_server_name: CODEX_APPS_MCP_SERVER_NAME, }, ); (plan.specs, plan.handlers) @@ -1921,14 +1938,14 @@ fn discoverable_connector(id: &str, name: &str, description: &str) -> Discoverab })) } -fn app_tool<'a>( +fn deferred_mcp_tool<'a>( tool_name: &'a str, tool_namespace: &'a str, server_name: &'a str, connector_name: Option<&'a str>, connector_description: Option<&'a str>, -) -> ToolRegistryPlanAppTool<'a> { - ToolRegistryPlanAppTool { +) -> ToolRegistryPlanDeferredTool<'a> { + ToolRegistryPlanDeferredTool { tool_name, tool_namespace, server_name, diff --git a/codex-rs/tools/src/tool_registry_plan_types.rs b/codex-rs/tools/src/tool_registry_plan_types.rs index bf77090aa5eb..fb6286717111 100644 --- a/codex-rs/tools/src/tool_registry_plan_types.rs +++ b/codex-rs/tools/src/tool_registry_plan_types.rs @@ -58,13 +58,12 @@ pub struct ToolRegistryPlan { #[derive(Debug, Clone, Copy)] pub struct ToolRegistryPlanParams<'a> { pub mcp_tools: Option<&'a HashMap>, + pub deferred_mcp_tools: Option<&'a [ToolRegistryPlanDeferredTool<'a>]>, pub tool_namespaces: Option<&'a HashMap>, - pub app_tools: Option<&'a [ToolRegistryPlanAppTool<'a>]>, pub discoverable_tools: Option<&'a [DiscoverableTool]>, pub dynamic_tools: &'a [DynamicToolSpec], pub default_agent_type_description: &'a str, pub wait_agent_timeouts: WaitAgentTimeoutOptions, - pub codex_apps_mcp_server_name: &'a str, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -74,7 +73,7 @@ pub struct ToolNamespace { } #[derive(Debug, Clone, Copy)] -pub struct ToolRegistryPlanAppTool<'a> { +pub struct ToolRegistryPlanDeferredTool<'a> { pub tool_name: &'a str, pub tool_namespace: &'a str, pub server_name: &'a str,