github/codex
4
0 Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-13 14:46:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

[codex] Remove string-keyed MCP tool maps (#21454)

Browse Source 
## Summary

This PR removes the synthetic `HashMap<String, ToolInfo>` keys from MCP
tool discovery. `McpConnectionManager::list_all_tools()` now returns
normalized `Vec<ToolInfo>`, and downstream code derives identity from
`ToolInfo::canonical_tool_name()`.

The motivation is to keep model-visible tool identity on
`ToolName`/`ToolInfo` instead of parallel string map keys, so future
namespace changes do not have to preserve otherwise-unused lookup keys.

## Changes

- Rename the MCP normalization path from `qualify_tools` to
`normalize_tools_for_model` and return tool values directly.
- Flow MCP tool lists through connectors, plugin injection, router/spec
building, code mode, and tool search as vectors/slices.
- Keep direct/deferred subtraction local to `mcp_tool_exposure`, using
`ToolName` values.
- Update tests to compare `ToolName` instances where MCP identity
matters.

## Validation

- `cargo test -p codex-mcp test_normalize_tools`
- `cargo test -p codex-core mcp_tool_exposure`
- `cargo test -p codex-core
direct_mcp_tools_register_namespaced_handlers`
- `cargo test -p codex-core
search_tool_registers_namespaced_mcp_tool_aliases`
- `just fix -p codex-mcp`
- `just fix -p codex-core`
This commit is contained in:
pakrym-oai authored and GitHub committed 2026-05-07 10:16:10 -07:00
1 parent 114bac1409
commit 857e731478
20 files changed
+459 -471
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

codex-rs/codex-mcp/src/codex_apps.rs
-11
View File
@@ -5,7 +5,6 @@
//! connector allow-list filtering, and the normalization that turns app //! connector allow-list filtering, and the normalization that turns app
//! connector/tool metadata into model-visible MCP callable names. //! connector/tool metadata into model-visible MCP callable names.
use std::collections::HashMap;
use std::path::PathBuf; use std::path::PathBuf;
use std::time::Instant; use std::time::Instant;
@@ -38,16 +37,6 @@ pub fn codex_apps_tools_cache_key(auth: Option<&CodexAuth>) -> CodexAppsToolsCac
} }
} }
pub fn filter_non_codex_apps_mcp_tools_only(
mcp_tools: &HashMap<String, ToolInfo>,
) -> HashMap<String, ToolInfo> {
mcp_tools
.iter()
.filter(|(_, tool)| tool.server_name != CODEX_APPS_MCP_SERVER_NAME)
.map(|(name, tool)| (name.clone(), tool.clone()))
.collect()
}
#[derive(Clone)] #[derive(Clone)]
pub(crate) struct CodexAppsToolsCacheContext { pub(crate) struct CodexAppsToolsCacheContext {
pub(crate) codex_home: PathBuf, pub(crate) codex_home: PathBuf,
codex-rs/codex-mcp/src/connection_manager.rs
+8 -9
View File
@@ -34,7 +34,7 @@ use crate::server::EffectiveMcpServer;
use crate::server::McpServerMetadata; use crate::server::McpServerMetadata;
use crate::tools::ToolInfo; use crate::tools::ToolInfo;
use crate::tools::filter_tools; use crate::tools::filter_tools;
use crate::tools::qualify_tools; use crate::tools::normalize_tools_for_model;
use crate::tools::tool_with_model_visible_input_schema; use crate::tools::tool_with_model_visible_input_schema;
use anyhow::Context; use anyhow::Context;
use anyhow::Result; use anyhow::Result;
@@ -363,10 +363,9 @@ impl McpConnectionManager {
failures failures
} }
/// Returns a single map that contains all tools. Each key is the /// Returns all tools with model-visible names normalized.
/// fully-qualified name for the tool.
#[instrument(level = "trace", skip_all)] #[instrument(level = "trace", skip_all)]
pub async fn list_all_tools(&self) -> HashMap<String, ToolInfo> { pub async fn list_all_tools(&self) -> Vec<ToolInfo> {
let mut tools = Vec::new(); let mut tools = Vec::new();
for managed_client in self.clients.values() { for managed_client in self.clients.values() {
let Some(server_tools) = managed_client.listed_tools().await else { let Some(server_tools) = managed_client.listed_tools().await else {
@@ -374,15 +373,15 @@ impl McpConnectionManager {
}; };
tools.extend(server_tools); tools.extend(server_tools);
} }
qualify_tools(tools) normalize_tools_for_model(tools)
} }
/// Force-refresh codex apps tools by bypassing the in-process cache. /// Force-refresh codex apps tools by bypassing the in-process cache.
/// ///
/// On success, the refreshed tools replace the cache contents and the /// On success, the refreshed tools replace the cache contents and the
/// latest filtered tool map is returned directly to the caller. On /// latest filtered tools are returned directly to the caller. On
/// failure, the existing cache remains unchanged. /// failure, the existing cache remains unchanged.
pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result<HashMap<String, ToolInfo>> { pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result<Vec<ToolInfo>> {
let managed_client = self let managed_client = self
.clients .clients
.get(CODEX_APPS_MCP_SERVER_NAME) .get(CODEX_APPS_MCP_SERVER_NAME)
@@ -425,7 +424,7 @@ impl McpConnectionManager {
tool.tool = tool_with_model_visible_input_schema(&tool.tool); tool.tool = tool_with_model_visible_input_schema(&tool.tool);
tool tool
}); });
Ok(qualify_tools(tools)) Ok(normalize_tools_for_model(tools))
} }
/// Returns a single map that contains all resources. Each key is the /// Returns a single map that contains all resources. Each key is the
@@ -664,7 +663,7 @@ impl McpConnectionManager {
pub async fn resolve_tool_info(&self, tool_name: &ToolName) -> Option<ToolInfo> { pub async fn resolve_tool_info(&self, tool_name: &ToolName) -> Option<ToolInfo> {
let all_tools = self.list_all_tools().await; let all_tools = self.list_all_tools().await;
all_tools all_tools
.into_values() .into_iter()
.find(|tool| tool.canonical_tool_name() == *tool_name) .find(|tool| tool.canonical_tool_name() == *tool_name)
} }
codex-rs/codex-mcp/src/connection_manager_tests.rs
+91 -55
View File
@@ -14,7 +14,7 @@ use crate::rmcp_client::elicitation_capability_for_server;
use crate::tools::ToolFilter; use crate::tools::ToolFilter;
use crate::tools::ToolInfo; use crate::tools::ToolInfo;
use crate::tools::filter_tools; use crate::tools::filter_tools;
use crate::tools::qualify_tools; use crate::tools::normalize_tools_for_model;
use crate::tools::tool_with_model_visible_input_schema; use crate::tools::tool_with_model_visible_input_schema;
use codex_config::Constrained; use codex_config::Constrained;
use codex_config::McpServerConfig; use codex_config::McpServerConfig;
@@ -86,6 +86,13 @@ fn create_codex_apps_tools_cache_context(
} }
} }
fn model_tool_names(tools: &[ToolInfo]) -> HashSet<ToolName> {
tools
.iter()
.map(ToolInfo::canonical_tool_name)
.collect::<HashSet<_>>()
}
#[test] #[test]
fn declared_openai_file_fields_treat_names_literally() { fn declared_openai_file_fields_treat_names_literally() {
let meta = serde_json::json!({ let meta = serde_json::json!({
@@ -273,35 +280,41 @@ async fn disabled_permissions_do_not_auto_accept_elicitation_with_requested_fiel
} }
#[test] #[test]
fn test_qualify_tools_short_non_duplicated_names() { fn test_normalize_tools_short_non_duplicated_names() {
let tools = vec![ let tools = vec![
create_test_tool("server1", "tool1"), create_test_tool("server1", "tool1"),
create_test_tool("server1", "tool2"), create_test_tool("server1", "tool2"),
]; ];
let qualified_tools = qualify_tools(tools); let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 2); assert_eq!(
assert!(qualified_tools.contains_key("mcp__server1__tool1")); model_tool_names(&model_tools),
assert!(qualified_tools.contains_key("mcp__server1__tool2")); HashSet::from([
ToolName::namespaced("mcp__server1__", "tool1"),
ToolName::namespaced("mcp__server1__", "tool2")
])
);
} }
#[test] #[test]
fn test_qualify_tools_duplicated_names_skipped() { fn test_normalize_tools_duplicated_names_skipped() {
let tools = vec![ let tools = vec![
create_test_tool("server1", "duplicate_tool"), create_test_tool("server1", "duplicate_tool"),
create_test_tool("server1", "duplicate_tool"), create_test_tool("server1", "duplicate_tool"),
]; ];
let qualified_tools = qualify_tools(tools); let model_tools = normalize_tools_for_model(tools);
// Only the first tool should remain, the second is skipped // Only the first tool should remain, the second is skipped
assert_eq!(qualified_tools.len(), 1); assert_eq!(
assert!(qualified_tools.contains_key("mcp__server1__duplicate_tool")); model_tool_names(&model_tools),
HashSet::from([ToolName::namespaced("mcp__server1__", "duplicate_tool")])
);
} }
#[test] #[test]
fn test_qualify_tools_long_names_same_server() { fn test_normalize_tools_long_names_same_server() {
let server_name = "my_server"; let server_name = "my_server";
let tools = vec![ let tools = vec![
@@ -315,116 +328,131 @@ fn test_qualify_tools_long_names_same_server() {
), ),
]; ];
let qualified_tools = qualify_tools(tools); let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 2); assert_eq!(model_tools.len(), 2);
let mut keys: Vec<_> = qualified_tools.keys().cloned().collect(); let names = model_tool_names(&model_tools);
keys.sort();
assert!(keys.iter().all(|key| key.len() == 64)); assert!(names.iter().all(|name| name.display().len() == 64));
assert!(keys.iter().all(|key| key.starts_with("mcp__my_server__")));
assert!( assert!(
keys.iter() names
.all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')), .iter()
"qualified names must be code-mode compatible: {keys:?}" .all(|name| name.namespace.as_deref() == Some("mcp__my_server__"))
);
assert!(
names.iter().all(|name| name
.display()
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '_')),
"model-visible names must be code-mode compatible: {names:?}"
); );
} }
#[test] #[test]
fn test_qualify_tools_sanitizes_invalid_characters() { fn test_normalize_tools_sanitizes_invalid_characters() {
let tools = vec![create_test_tool("server.one", "tool.two-three")]; let tools = vec![create_test_tool("server.one", "tool.two-three")];
let qualified_tools = qualify_tools(tools); let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 1); assert_eq!(model_tools.len(), 1);
let (qualified_name, tool) = qualified_tools.into_iter().next().expect("one tool"); let tool = model_tools.into_iter().next().expect("one tool");
assert_eq!(qualified_name, "mcp__server_one__tool_two_three"); let model_name = tool.canonical_tool_name();
assert_eq!(
model_name,
ToolName::namespaced("mcp__server_one__", "tool_two_three")
);
assert_eq!( assert_eq!(
format!("{}{}", tool.callable_namespace, tool.callable_name), format!("{}{}", tool.callable_namespace, tool.callable_name),
qualified_name model_name.display()
); );
// The key and callable parts are sanitized for model-visible tool calls, but // The callable parts are sanitized for model-visible tool calls, but the raw
// the raw MCP name is preserved for the actual MCP call. // MCP name is preserved for the actual MCP call.
assert_eq!(tool.server_name, "server.one"); assert_eq!(tool.server_name, "server.one");
assert_eq!(tool.callable_namespace, "mcp__server_one__"); assert_eq!(tool.callable_namespace, "mcp__server_one__");
assert_eq!(tool.callable_name, "tool_two_three"); assert_eq!(tool.callable_name, "tool_two_three");
assert_eq!(tool.tool.name, "tool.two-three"); assert_eq!(tool.tool.name, "tool.two-three");
assert!( assert!(
qualified_name model_name
.display()
.chars() .chars()
.all(|c| c.is_ascii_alphanumeric() || c == '_'), .all(|c| c.is_ascii_alphanumeric() || c == '_'),
"qualified name must be code-mode compatible: {qualified_name:?}" "model-visible name must be code-mode compatible: {model_name:?}"
); );
} }
#[test] #[test]
fn test_qualify_tools_keeps_hyphenated_mcp_tools_callable() { fn test_normalize_tools_keeps_hyphenated_mcp_tools_callable() {
let tools = vec![create_test_tool("music-studio", "get-strudel-guide")]; let tools = vec![create_test_tool("music-studio", "get-strudel-guide")];
let qualified_tools = qualify_tools(tools); let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 1); assert_eq!(model_tools.len(), 1);
let (qualified_name, tool) = qualified_tools.into_iter().next().expect("one tool"); let tool = model_tools.into_iter().next().expect("one tool");
assert_eq!(qualified_name, "mcp__music_studio__get_strudel_guide"); assert_eq!(
tool.canonical_tool_name(),
ToolName::namespaced("mcp__music_studio__", "get_strudel_guide")
);
assert_eq!(tool.callable_namespace, "mcp__music_studio__"); assert_eq!(tool.callable_namespace, "mcp__music_studio__");
assert_eq!(tool.callable_name, "get_strudel_guide"); assert_eq!(tool.callable_name, "get_strudel_guide");
assert_eq!(tool.tool.name, "get-strudel-guide"); assert_eq!(tool.tool.name, "get-strudel-guide");
} }
#[test] #[test]
fn test_qualify_tools_disambiguates_sanitized_namespace_collisions() { fn test_normalize_tools_disambiguates_sanitized_namespace_collisions() {
let tools = vec![ let tools = vec![
create_test_tool("basic-server", "lookup"), create_test_tool("basic-server", "lookup"),
create_test_tool("basic_server", "query"), create_test_tool("basic_server", "query"),
]; ];
let qualified_tools = qualify_tools(tools); let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 2); assert_eq!(model_tools.len(), 2);
let mut namespaces = qualified_tools let mut namespaces = model_tools
.values() .iter()
.map(|tool| tool.callable_namespace.as_str()) .map(|tool| tool.callable_namespace.as_str())
.collect::<Vec<_>>(); .collect::<Vec<_>>();
namespaces.sort(); namespaces.sort();
namespaces.dedup(); namespaces.dedup();
assert_eq!(namespaces.len(), 2); assert_eq!(namespaces.len(), 2);
let raw_servers = qualified_tools let raw_servers = model_tools
.values() .iter()
.map(|tool| tool.server_name.as_str()) .map(|tool| tool.server_name.as_str())
.collect::<HashSet<_>>(); .collect::<HashSet<_>>();
assert_eq!(raw_servers, HashSet::from(["basic-server", "basic_server"])); assert_eq!(raw_servers, HashSet::from(["basic-server", "basic_server"]));
let model_names = model_tool_names(&model_tools);
assert!( assert!(
qualified_tools model_names.iter().all(|name| name
.keys() .display()
.all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')), .chars()
"qualified names must be code-mode compatible: {qualified_tools:?}" .all(|c| c.is_ascii_alphanumeric() || c == '_')),
"model-visible names must be code-mode compatible: {model_names:?}"
); );
} }
#[test] #[test]
fn test_qualify_tools_disambiguates_sanitized_tool_name_collisions() { fn test_normalize_tools_disambiguates_sanitized_tool_name_collisions() {
let tools = vec![ let tools = vec![
create_test_tool("server", "tool-name"), create_test_tool("server", "tool-name"),
create_test_tool("server", "tool_name"), create_test_tool("server", "tool_name"),
]; ];
let qualified_tools = qualify_tools(tools); let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 2); assert_eq!(model_tools.len(), 2);
let raw_tool_names = qualified_tools let raw_tool_names = model_tools
.values() .iter()
.map(|tool| tool.tool.name.to_string()) .map(|tool| tool.tool.name.to_string())
.collect::<HashSet<_>>(); .collect::<HashSet<_>>();
assert_eq!( assert_eq!(
raw_tool_names, raw_tool_names,
HashSet::from(["tool-name".to_string(), "tool_name".to_string()]) HashSet::from(["tool-name".to_string(), "tool_name".to_string()])
); );
let callable_tool_names = qualified_tools let callable_tool_names = model_tools
.values() .iter()
.map(|tool| tool.callable_name.as_str()) .map(|tool| tool.callable_name.as_str())
.collect::<HashSet<_>>(); .collect::<HashSet<_>>();
assert_eq!(callable_tool_names.len(), 2); assert_eq!(callable_tool_names.len(), 2);
@@ -673,7 +701,11 @@ async fn list_all_tools_uses_startup_snapshot_while_client_is_pending() {
let tools = manager.list_all_tools().await; let tools = manager.list_all_tools().await;
let tool = tools let tool = tools
.get("mcp__codex_apps__calendar_create_event") .iter()
.find(|tool| {
tool.canonical_tool_name()
== ToolName::namespaced("mcp__codex_apps__", "calendar_create_event")
})
.expect("tool from startup cache"); .expect("tool from startup cache");
assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME); assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME);
assert_eq!(tool.callable_name, "calendar_create_event"); assert_eq!(tool.callable_name, "calendar_create_event");
@@ -799,7 +831,11 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() {
let tools = manager.list_all_tools().await; let tools = manager.list_all_tools().await;
let tool = tools let tool = tools
.get("mcp__codex_apps__calendar_create_event") .iter()
.find(|tool| {
tool.canonical_tool_name()
== ToolName::namespaced("mcp__codex_apps__", "calendar_create_event")
})
.expect("tool from startup cache"); .expect("tool from startup cache");
assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME); assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME);
assert_eq!(tool.callable_name, "calendar_create_event"); assert_eq!(tool.callable_name, "calendar_create_event");
codex-rs/codex-mcp/src/lib.rs
-1
View File
@@ -51,7 +51,6 @@ pub use mcp::oauth_login_support;
pub use mcp::resolve_oauth_scopes; pub use mcp::resolve_oauth_scopes;
pub use mcp::should_retry_without_scopes; pub use mcp::should_retry_without_scopes;
pub use codex_apps::filter_non_codex_apps_mcp_tools_only;
pub use mcp::McpPermissionPromptAutoApproveContext; pub use mcp::McpPermissionPromptAutoApproveContext;
pub use mcp::mcp_permission_prompt_is_auto_approved; pub use mcp::mcp_permission_prompt_is_auto_approved;
pub use mcp::qualified_mcp_tool_name_prefix; pub use mcp::qualified_mcp_tool_name_prefix;
codex-rs/codex-mcp/src/mcp/mod.rs
+1 -1
View File
@@ -583,7 +583,7 @@ async fn collect_mcp_server_status_snapshot_from_manager(
); );
let mut tools_by_server = HashMap::<String, HashMap<String, Tool>>::new(); let mut tools_by_server = HashMap::<String, HashMap<String, Tool>>::new();
for (_qualified_name, tool_info) in tools { for tool_info in tools {
let raw_tool_name = tool_info.tool.name.to_string(); 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 { let Some(tool) = protocol_tool_from_rmcp_tool(&raw_tool_name, &tool_info.tool) else {
continue; continue;
codex-rs/codex-mcp/src/tools.rs
+15 -15
View File
@@ -1,4 +1,4 @@
//! MCP tool metadata, filtering, schema shaping, and name qualification. //! MCP tool metadata, filtering, schema shaping, and name normalization.
//! //!
//! Raw MCP tool identities must be preserved for protocol calls, while //! Raw MCP tool identities must be preserved for protocol calls, while
//! model-visible tool names must be sanitized, deduplicated, and kept within API //! model-visible tool names must be sanitized, deduplicated, and kept within API
@@ -130,12 +130,12 @@ pub(crate) fn filter_tools(tools: Vec<ToolInfo>, filter: &ToolFilter) -> Vec<Too
.collect() .collect()
} }
/// Returns a qualified-name lookup for MCP tools. /// Returns MCP tools with model-visible names normalized.
/// ///
/// Raw MCP server/tool names are kept on each [`ToolInfo`] for protocol calls, while /// 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 /// `callable_namespace` / `callable_name` are sanitized and, when necessary, hashed so
/// every model-visible `mcp__namespace__tool` name is unique and <= 64 bytes. /// every model-visible name is unique and <= 64 bytes.
pub(crate) fn qualify_tools<I>(tools: I) -> HashMap<String, ToolInfo> pub(crate) fn normalize_tools_for_model<I>(tools: I) -> Vec<ToolInfo>
where where
I: IntoIterator<Item = ToolInfo>, I: IntoIterator<Item = ToolInfo>,
{ {
@@ -213,9 +213,9 @@ where
candidates.sort_by(|left, right| left.raw_tool_identity.cmp(&right.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 used_names = HashSet::new();
let mut qualified_tools = HashMap::new(); let mut model_tools = Vec::new();
for mut candidate in candidates { for mut candidate in candidates {
let (callable_namespace, callable_name, qualified_name) = unique_callable_parts( let (callable_namespace, callable_name) = unique_callable_parts(
&candidate.callable_namespace, &candidate.callable_namespace,
&candidate.callable_name, &candidate.callable_name,
&candidate.raw_tool_identity, &candidate.raw_tool_identity,
@@ -223,9 +223,9 @@ where
); );
candidate.tool.callable_namespace = callable_namespace; candidate.tool.callable_namespace = callable_namespace;
candidate.tool.callable_name = callable_name; candidate.tool.callable_name = callable_name;
qualified_tools.insert(qualified_name, candidate.tool); model_tools.push(candidate.tool);
} }
qualified_tools model_tools
} }
#[derive(Debug)] #[derive(Debug)]
@@ -345,10 +345,10 @@ fn unique_callable_parts(
tool_name: &str, tool_name: &str,
raw_identity: &str, raw_identity: &str,
used_names: &mut HashSet<String>, used_names: &mut HashSet<String>,
) -> (String, String, String) { ) -> (String, String) {
let qualified_name = format!("{namespace}{tool_name}"); let model_name = format!("{namespace}{tool_name}");
if qualified_name.len() <= MAX_TOOL_NAME_LENGTH && used_names.insert(qualified_name.clone()) { if model_name.len() <= MAX_TOOL_NAME_LENGTH && used_names.insert(model_name) {
return (namespace.to_string(), tool_name.to_string(), qualified_name); return (namespace.to_string(), tool_name.to_string());
} }
let mut attempt = 0_u32; let mut attempt = 0_u32;
@@ -360,9 +360,9 @@ fn unique_callable_parts(
}; };
let (namespace, tool_name) = let (namespace, tool_name) =
fit_callable_parts_with_hash(namespace, tool_name, &hash_input); fit_callable_parts_with_hash(namespace, tool_name, &hash_input);
let qualified_name = format!("{namespace}{tool_name}"); let model_name = format!("{namespace}{tool_name}");
if used_names.insert(qualified_name.clone()) { if used_names.insert(model_name) {
return (namespace, tool_name, qualified_name); return (namespace, tool_name);
} }
attempt = attempt.saturating_add(1); attempt = attempt.saturating_add(1);
} }
codex-rs/core/src/connectors.rs
+3 -5
View File
@@ -165,7 +165,7 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools(
pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools( pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools(
config: &Config, config: &Config,
auth: Option<&CodexAuth>, auth: Option<&CodexAuth>,
mcp_tools: &HashMap<String, ToolInfo>, mcp_tools: &[ToolInfo],
) { ) {
if !config.features.enabled(Feature::Apps) { if !config.features.enabled(Feature::Apps) {
return; return;
@@ -517,12 +517,10 @@ async fn chatgpt_get_request_with_auth_provider<T: DeserializeOwned>(
} }
} }
pub(crate) fn accessible_connectors_from_mcp_tools( pub(crate) fn accessible_connectors_from_mcp_tools(mcp_tools: &[ToolInfo]) -> Vec<AppInfo> {
mcp_tools: &HashMap<String, ToolInfo>,
) -> Vec<AppInfo> {
// ToolInfo already carries plugin provenance, so app-level plugin sources // ToolInfo already carries plugin provenance, so app-level plugin sources
// can be derived here instead of requiring a separate enrichment pass. // can be derived here instead of requiring a separate enrichment pass.
let tools = mcp_tools.values().filter_map(|tool| { let tools = mcp_tools.iter().filter_map(|tool| {
if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { if tool.server_name != CODEX_APPS_MCP_SERVER_NAME {
return None; return None;
} }
codex-rs/core/src/connectors_tests.rs
+53 -71
View File
@@ -172,39 +172,30 @@ fn merge_connectors_replaces_plugin_placeholder_name_with_accessible_name() {
#[test] #[test]
fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() { fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() {
let tools = HashMap::from([ let tools = vec![
( codex_app_tool(
"mcp__codex_apps__calendar_list_events".to_string(), "calendar_list_events",
codex_app_tool( "calendar",
"calendar_list_events", /*connector_name*/ None,
"calendar", &["sample", "sample"],
/*connector_name*/ None,
&["sample", "sample"],
),
), ),
( codex_app_tool(
"mcp__codex_apps__calendar_create_event".to_string(), "calendar_create_event",
codex_app_tool( "calendar",
"calendar_create_event", Some("Google Calendar"),
"calendar", &["beta", "sample"],
Some("Google Calendar"),
&["beta", "sample"],
),
), ),
( ToolInfo {
"mcp__sample__echo".to_string(), server_name: "sample".to_string(),
ToolInfo { callable_name: "echo".to_string(),
server_name: "sample".to_string(), callable_namespace: "sample".to_string(),
callable_name: "echo".to_string(), namespace_description: None,
callable_namespace: "sample".to_string(), tool: test_tool_definition("echo"),
namespace_description: None, connector_id: None,
tool: test_tool_definition("echo"), connector_name: None,
connector_id: None, plugin_display_names: plugin_names(&["ignored"]),
connector_name: None, },
plugin_display_names: plugin_names(&["ignored"]), ];
},
),
]);
let connectors = accessible_connectors_from_mcp_tools(&tools); let connectors = accessible_connectors_from_mcp_tools(&tools);
@@ -238,26 +229,20 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal
.expect("config should load"); .expect("config should load");
let _ = config.features.set_enabled(Feature::Apps, /*enabled*/ true); let _ = config.features.set_enabled(Feature::Apps, /*enabled*/ true);
let cache_key = accessible_connectors_cache_key(&config, /*auth*/ None); let cache_key = accessible_connectors_cache_key(&config, /*auth*/ None);
let tools = HashMap::from([ let tools = vec![
( codex_app_tool(
"mcp__codex_apps__calendar_list_events".to_string(), "calendar_list_events",
codex_app_tool( "calendar",
"calendar_list_events", Some("Google Calendar"),
"calendar", &["calendar-plugin"],
Some("Google Calendar"),
&["calendar-plugin"],
),
), ),
( codex_app_tool(
"mcp__codex_apps__openai_hidden".to_string(), "openai_hidden",
codex_app_tool( "connector_openai_hidden",
"openai_hidden", Some("Hidden"),
"connector_openai_hidden", &[],
Some("Hidden"),
&[],
),
), ),
]); ];
let cached = with_accessible_connectors_cache_cleared(|| { let cached = with_accessible_connectors_cache_cleared(|| {
refresh_accessible_connectors_cache_from_mcp_tools(&config, /*auth*/ None, &tools); refresh_accessible_connectors_cache_from_mcp_tools(&config, /*auth*/ None, &tools);
@@ -315,29 +300,26 @@ fn merge_connectors_unions_and_dedupes_plugin_display_names() {
#[test] #[test]
fn accessible_connectors_from_mcp_tools_preserves_description() { fn accessible_connectors_from_mcp_tools_preserves_description() {
let mcp_tools = HashMap::from([( let mcp_tools = vec![ToolInfo {
"mcp__codex_apps__calendar_create_event".to_string(), server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
ToolInfo { callable_name: "calendar_create_event".to_string(),
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), callable_namespace: "mcp__codex_apps__calendar".to_string(),
callable_name: "calendar_create_event".to_string(), namespace_description: Some("Plan events".to_string()),
callable_namespace: "mcp__codex_apps__calendar".to_string(), tool: Tool {
namespace_description: Some("Plan events".to_string()), name: "calendar_create_event".to_string().into(),
tool: Tool { title: None,
name: "calendar_create_event".to_string().into(), description: Some("Create a calendar event".into()),
title: None, input_schema: Arc::new(JsonObject::default()),
description: Some("Create a calendar event".into()), output_schema: None,
input_schema: Arc::new(JsonObject::default()), annotations: None,
output_schema: None, execution: None,
annotations: None, icons: None,
execution: None, meta: None,
icons: None,
meta: None,
},
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
}, },
)]); connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
}];
assert_eq!( assert_eq!(
accessible_connectors_from_mcp_tools(&mcp_tools), accessible_connectors_from_mcp_tools(&mcp_tools),
codex-rs/core/src/mcp_tool_call.rs
+2 -2
View File
@@ -1490,7 +1490,7 @@ pub(crate) async fn lookup_mcp_tool_metadata(
.list_all_tools() .list_all_tools()
.await; .await;
let tool_info = tools let tool_info = tools
.into_values() .into_iter()
.find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?; .find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?;
let connector_description = if server == CODEX_APPS_MCP_SERVER_NAME { let connector_description = if server == CODEX_APPS_MCP_SERVER_NAME {
let connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools( let connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools(
@@ -1585,7 +1585,7 @@ async fn lookup_mcp_app_usage_metadata(
.list_all_tools() .list_all_tools()
.await; .await;
tools.into_values().find_map(|tool_info| { tools.into_iter().find_map(|tool_info| {
if tool_info.server_name == server && tool_info.tool.name == tool_name { if tool_info.server_name == server && tool_info.tool.name == tool_name {
Some(McpAppUsageMetadata { Some(McpAppUsageMetadata {
connector_id: tool_info.connector_id, connector_id: tool_info.connector_id,
codex-rs/core/src/mcp_tool_exposure.rs
+20 -12
View File
@@ -1,10 +1,8 @@
use std::collections::HashMap;
use std::collections::HashSet; use std::collections::HashSet;
use codex_features::Feature; use codex_features::Feature;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::ToolInfo as McpToolInfo; use codex_mcp::ToolInfo as McpToolInfo;
use codex_mcp::filter_non_codex_apps_mcp_tools_only;
use codex_tools::ToolsConfig; use codex_tools::ToolsConfig;
use crate::config::Config; use crate::config::Config;
@@ -13,12 +11,12 @@ use crate::connectors;
pub(crate) const DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD: usize = 100; pub(crate) const DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD: usize = 100;
pub(crate) struct McpToolExposure { pub(crate) struct McpToolExposure {
pub(crate) direct_tools: HashMap<String, McpToolInfo>, pub(crate) direct_tools: Vec<McpToolInfo>,
pub(crate) deferred_tools: Option<HashMap<String, McpToolInfo>>, pub(crate) deferred_tools: Option<Vec<McpToolInfo>>,
} }
pub(crate) fn build_mcp_tool_exposure( pub(crate) fn build_mcp_tool_exposure(
all_mcp_tools: &HashMap<String, McpToolInfo>, all_mcp_tools: &[McpToolInfo],
connectors: Option<&[connectors::AppInfo]>, connectors: Option<&[connectors::AppInfo]>,
explicitly_enabled_connectors: &[connectors::AppInfo], explicitly_enabled_connectors: &[connectors::AppInfo],
config: &Config, config: &Config,
@@ -48,9 +46,11 @@ pub(crate) fn build_mcp_tool_exposure(
let direct_tools = let direct_tools =
filter_codex_apps_mcp_tools(all_mcp_tools, explicitly_enabled_connectors, config); filter_codex_apps_mcp_tools(all_mcp_tools, explicitly_enabled_connectors, config);
for direct_tool_name in direct_tools.keys() { let direct_tool_names = direct_tools
deferred_tools.remove(direct_tool_name); .iter()
} .map(McpToolInfo::canonical_tool_name)
.collect::<HashSet<_>>();
deferred_tools.retain(|tool| !direct_tool_names.contains(&tool.canonical_tool_name()));
McpToolExposure { McpToolExposure {
direct_tools, direct_tools,
@@ -58,11 +58,19 @@ pub(crate) fn build_mcp_tool_exposure(
} }
} }
fn filter_non_codex_apps_mcp_tools_only(mcp_tools: &[McpToolInfo]) -> Vec<McpToolInfo> {
mcp_tools
.iter()
.filter(|tool| tool.server_name != CODEX_APPS_MCP_SERVER_NAME)
.cloned()
.collect()
}
fn filter_codex_apps_mcp_tools( fn filter_codex_apps_mcp_tools(
mcp_tools: &HashMap<String, McpToolInfo>, mcp_tools: &[McpToolInfo],
connectors: &[connectors::AppInfo], connectors: &[connectors::AppInfo],
config: &Config, config: &Config,
) -> HashMap<String, McpToolInfo> { ) -> Vec<McpToolInfo> {
let allowed: HashSet<&str> = connectors let allowed: HashSet<&str> = connectors
.iter() .iter()
.map(|connector| connector.id.as_str()) .map(|connector| connector.id.as_str())
@@ -70,7 +78,7 @@ fn filter_codex_apps_mcp_tools(
mcp_tools mcp_tools
.iter() .iter()
.filter(|(_, tool)| { .filter(|tool| {
if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { if tool.server_name != CODEX_APPS_MCP_SERVER_NAME {
return false; return false;
} }
@@ -79,7 +87,7 @@ fn filter_codex_apps_mcp_tools(
}; };
allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool) allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool)
}) })
.map(|(name, tool)| (name.clone(), tool.clone())) .cloned()
.collect() .collect()
} }
codex-rs/core/src/mcp_tool_exposure_test.rs
+73 -69
View File
@@ -1,7 +1,6 @@
use std::collections::HashMap; use std::collections::HashSet;
use std::sync::Arc; use std::sync::Arc;
use codex_connectors::metadata::sanitize_name;
use codex_features::Feature; use codex_features::Feature;
use codex_features::Features; use codex_features::Features;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
@@ -11,6 +10,7 @@ use codex_protocol::config_types::WebSearchMode;
use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile; use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SessionSource;
use codex_tools::ToolName;
use codex_tools::ToolsConfig; use codex_tools::ToolsConfig;
use codex_tools::ToolsConfigParams; use codex_tools::ToolsConfigParams;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
@@ -42,22 +42,15 @@ fn make_connector(id: &str, name: &str) -> AppInfo {
fn make_mcp_tool( fn make_mcp_tool(
server_name: &str, server_name: &str,
tool_name: &str, tool_name: &str,
callable_namespace: &str,
callable_name: &str,
connector_id: Option<&str>, connector_id: Option<&str>,
connector_name: Option<&str>, connector_name: Option<&str>,
) -> ToolInfo { ) -> ToolInfo {
let tool_namespace = if server_name == CODEX_APPS_MCP_SERVER_NAME {
connector_name
.map(sanitize_name)
.map(|connector_name| format!("mcp__{server_name}__{connector_name}"))
.unwrap_or_else(|| server_name.to_string())
} else {
format!("mcp__{server_name}__")
};
ToolInfo { ToolInfo {
server_name: server_name.to_string(), server_name: server_name.to_string(),
callable_name: tool_name.to_string(), callable_name: callable_name.to_string(),
callable_namespace: tool_namespace, callable_namespace: callable_namespace.to_string(),
namespace_description: None, namespace_description: None,
tool: Tool { tool: Tool {
name: tool_name.to_string().into(), name: tool_name.to_string().into(),
@@ -76,20 +69,29 @@ fn make_mcp_tool(
} }
} }
fn numbered_mcp_tools(count: usize) -> HashMap<String, ToolInfo> { fn numbered_mcp_tools(count: usize) -> Vec<ToolInfo> {
(0..count) (0..count)
.map(|index| { .map(|index| {
let tool_name = format!("tool_{index}"); let tool_name = format!("tool_{index}");
( make_mcp_tool(
format!("mcp__rmcp__{tool_name}"), "rmcp",
make_mcp_tool( &tool_name,
"rmcp", &tool_name, /*connector_id*/ None, /*connector_name*/ None, "mcp__rmcp__",
), &tool_name,
/*connector_id*/ None,
/*connector_name*/ None,
) )
}) })
.collect() .collect()
} }
fn tool_names(tools: &[ToolInfo]) -> HashSet<ToolName> {
tools
.iter()
.map(codex_mcp::ToolInfo::canonical_tool_name)
.collect()
}
async fn tools_config_for_mcp_tool_exposure(search_tool: bool) -> ToolsConfig { async fn tools_config_for_mcp_tool_exposure(search_tool: bool) -> ToolsConfig {
let config = test_config().await; let config = test_config().await;
let model_info = let model_info =
@@ -124,11 +126,7 @@ async fn directly_exposes_small_effective_tool_sets() {
&tools_config, &tools_config,
); );
let mut direct_tool_names: Vec<_> = exposure.direct_tools.keys().cloned().collect(); assert_eq!(tool_names(&exposure.direct_tools), tool_names(&mcp_tools));
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()); assert!(exposure.deferred_tools.is_none());
} }
@@ -151,11 +149,7 @@ async fn searches_large_effective_tool_sets() {
.deferred_tools .deferred_tools
.as_ref() .as_ref()
.expect("large tool sets should be discoverable through tool_search"); .expect("large tool sets should be discoverable through tool_search");
let mut deferred_tool_names: Vec<_> = deferred_tools.keys().cloned().collect(); assert_eq!(tool_names(deferred_tools), tool_names(&mcp_tools));
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);
} }
#[tokio::test] #[tokio::test]
@@ -163,15 +157,14 @@ async fn directly_exposes_explicit_apps_without_deferred_overlap() {
let config = test_config().await; let config = test_config().await;
let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true).await; let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true).await;
let mut mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1); let mut mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1);
mcp_tools.extend([( mcp_tools.push(make_mcp_tool(
"mcp__codex_apps__calendar_create_event".to_string(), CODEX_APPS_MCP_SERVER_NAME,
make_mcp_tool( "calendar_create_event",
CODEX_APPS_MCP_SERVER_NAME, "mcp__codex_apps__calendar",
"calendar_create_event", "_create_event",
Some("calendar"), Some("calendar"),
Some("Calendar"), Some("Calendar"),
), ));
)]);
let connectors = vec![make_connector("calendar", "Calendar")]; let connectors = vec![make_connector("calendar", "Calendar")];
let exposure = build_mcp_tool_exposure( let exposure = build_mcp_tool_exposure(
@@ -182,28 +175,32 @@ async fn directly_exposes_explicit_apps_without_deferred_overlap() {
&tools_config, &tools_config,
); );
let mut tool_names: Vec<String> = exposure.direct_tools.into_keys().collect(); let direct_tool_names = tool_names(&exposure.direct_tools);
tool_names.sort();
assert_eq!( assert_eq!(
tool_names, direct_tool_names,
vec!["mcp__codex_apps__calendar_create_event".to_string()] HashSet::from([ToolName::namespaced(
"mcp__codex_apps__calendar",
"_create_event"
)])
); );
assert_eq!( assert_eq!(
exposure.deferred_tools.as_ref().map(HashMap::len), exposure.deferred_tools.as_ref().map(Vec::len),
Some(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1) Some(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1)
); );
let deferred_tools = exposure let deferred_tools = exposure
.deferred_tools .deferred_tools
.as_ref() .as_ref()
.expect("large tool sets should be discoverable through tool_search"); .expect("large tool sets should be discoverable through tool_search");
let deferred_tool_names = tool_names(deferred_tools);
assert!( assert!(
tool_names direct_tool_names.is_disjoint(&deferred_tool_names),
.iter() "direct tools should not also be deferred: {direct_tool_names:?}"
.all(|direct_tool_name| !deferred_tools.contains_key(direct_tool_name)),
"direct tools should not also be deferred: {tool_names:?}"
); );
assert!(!deferred_tools.contains_key("mcp__codex_apps__calendar_create_event")); assert!(!deferred_tool_names.contains(&ToolName::namespaced(
assert!(deferred_tools.contains_key("mcp__rmcp__tool_0")); "mcp__codex_apps__calendar",
"_create_event"
)));
assert!(deferred_tool_names.contains(&ToolName::namespaced("mcp__rmcp__", "tool_0")));
} }
#[tokio::test] #[tokio::test]
@@ -214,23 +211,24 @@ async fn always_defer_feature_preserves_explicit_apps() {
.enable(Feature::ToolSearchAlwaysDeferMcpTools) .enable(Feature::ToolSearchAlwaysDeferMcpTools)
.expect("test config should allow feature update"); .expect("test config should allow feature update");
let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true).await; let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true).await;
let mcp_tools = HashMap::from([ let mcp_tools = vec![
( make_mcp_tool(
"mcp__rmcp__tool".to_string(), "rmcp",
make_mcp_tool( "tool",
"rmcp", "tool", /*connector_id*/ None, /*connector_name*/ None, "mcp__rmcp__",
), "tool",
/*connector_id*/ None,
/*connector_name*/ None,
), ),
( make_mcp_tool(
"mcp__codex_apps__calendar_create_event".to_string(), CODEX_APPS_MCP_SERVER_NAME,
make_mcp_tool( "calendar_create_event",
CODEX_APPS_MCP_SERVER_NAME, "mcp__codex_apps__calendar",
"calendar_create_event", "_create_event",
Some("calendar"), Some("calendar"),
Some("Calendar"), Some("Calendar"),
),
), ),
]); ];
let connectors = vec![make_connector("calendar", "Calendar")]; let connectors = vec![make_connector("calendar", "Calendar")];
let exposure = build_mcp_tool_exposure( let exposure = build_mcp_tool_exposure(
@@ -241,16 +239,22 @@ async fn always_defer_feature_preserves_explicit_apps() {
&tools_config, &tools_config,
); );
let mut direct_tool_names: Vec<String> = exposure.direct_tools.into_keys().collect(); let direct_tool_names = tool_names(&exposure.direct_tools);
direct_tool_names.sort();
assert_eq!( assert_eq!(
direct_tool_names, direct_tool_names,
vec!["mcp__codex_apps__calendar_create_event".to_string()] HashSet::from([ToolName::namespaced(
"mcp__codex_apps__calendar",
"_create_event"
)])
); );
let deferred_tools = exposure let deferred_tools = exposure
.deferred_tools .deferred_tools
.as_ref() .as_ref()
.expect("MCP tools should be discoverable through tool_search"); .expect("MCP tools should be discoverable through tool_search");
assert!(deferred_tools.contains_key("mcp__rmcp__tool")); let deferred_tool_names = tool_names(deferred_tools);
assert!(!deferred_tools.contains_key("mcp__codex_apps__calendar_create_event")); assert!(deferred_tool_names.contains(&ToolName::namespaced("mcp__rmcp__", "tool")));
assert!(!deferred_tool_names.contains(&ToolName::namespaced(
"mcp__codex_apps__calendar",
"_create_event"
)));
} }
codex-rs/core/src/plugins/injection.rs
+2 -3
View File
@@ -1,5 +1,4 @@
use std::collections::BTreeSet; use std::collections::BTreeSet;
use std::collections::HashMap;
use codex_connectors::metadata::connector_display_label; use codex_connectors::metadata::connector_display_label;
use codex_protocol::models::ResponseItem; use codex_protocol::models::ResponseItem;
@@ -14,7 +13,7 @@ use codex_mcp::ToolInfo;
pub(crate) fn build_plugin_injections( pub(crate) fn build_plugin_injections(
mentioned_plugins: &[PluginCapabilitySummary], mentioned_plugins: &[PluginCapabilitySummary],
mcp_tools: &HashMap<String, ToolInfo>, mcp_tools: &[ToolInfo],
available_connectors: &[connectors::AppInfo], available_connectors: &[connectors::AppInfo],
) -> Vec<ResponseItem> { ) -> Vec<ResponseItem> {
if mentioned_plugins.is_empty() { if mentioned_plugins.is_empty() {
@@ -27,7 +26,7 @@ pub(crate) fn build_plugin_injections(
.iter() .iter()
.filter_map(|plugin| { .filter_map(|plugin| {
let available_mcp_servers = mcp_tools let available_mcp_servers = mcp_tools
.values() .iter()
.filter(|tool| { .filter(|tool| {
tool.server_name != CODEX_APPS_MCP_SERVER_NAME tool.server_name != CODEX_APPS_MCP_SERVER_NAME
&& tool && tool
codex-rs/core/src/session/turn.rs
+3 -3
View File
@@ -196,10 +196,10 @@ pub(crate) async fn run_turn(
{ {
Ok(mcp_tools) => mcp_tools, Ok(mcp_tools) => mcp_tools,
Err(_) if turn_context.apps_enabled() => return None, Err(_) if turn_context.apps_enabled() => return None,
Err(_) => HashMap::new(), Err(_) => Vec::new(),
} }
} else { } else {
HashMap::new() Vec::new()
}; };
let available_connectors = if turn_context.apps_enabled() { let available_connectors = if turn_context.apps_enabled() {
let connectors = codex_connectors::merge::merge_plugin_connectors_with_accessible( let connectors = codex_connectors::merge::merge_plugin_connectors_with_accessible(
@@ -1253,7 +1253,7 @@ pub(crate) async fn built_tools(
let exposed_tool_names = mcp_tools let exposed_tool_names = mcp_tools
.iter() .iter()
.chain(deferred_mcp_tools.iter()) .chain(deferred_mcp_tools.iter())
.flat_map(|tools| tools.keys().map(String::as_str)) .flat_map(|tools| tools.iter().map(codex_mcp::ToolInfo::canonical_tool_name))
.collect::<HashSet<_>>(); .collect::<HashSet<_>>();
collect_unavailable_called_tools(input, &exposed_tool_names) collect_unavailable_called_tools(input, &exposed_tool_names)
} else { } else {
codex-rs/core/src/tools/handlers/tool_search.rs
+8 -23
View File
@@ -204,19 +204,11 @@ mod tests {
}), }),
defer_loading: true, defer_loading: true,
}]; }];
let handler = handler_from_tools( let mcp_tools = vec![
Some(&std::collections::HashMap::from([ tool_info("calendar", "create_event", "Create events"),
( tool_info("calendar", "list_events", "List events"),
"mcp__calendar__create_event".to_string(), ];
tool_info("calendar", "create_event", "Create events"), let handler = handler_from_tools(Some(&mcp_tools), &dynamic_tools);
),
(
"mcp__calendar__list_events".to_string(),
tool_info("calendar", "list_events", "List events"),
),
])),
&dynamic_tools,
);
let results = [ let results = [
&handler.entries[0], &handler.entries[0],
&handler.entries[2], &handler.entries[2],
@@ -376,18 +368,11 @@ mod tests {
assert!(count_results_for_server(&results, "other-server") <= TOOL_SEARCH_DEFAULT_LIMIT); assert!(count_results_for_server(&results, "other-server") <= TOOL_SEARCH_DEFAULT_LIMIT);
} }
fn numbered_tools( fn numbered_tools(server_name: &str, description_prefix: &str, count: usize) -> Vec<ToolInfo> {
server_name: &str,
description_prefix: &str,
count: usize,
) -> std::collections::HashMap<String, ToolInfo> {
(0..count) (0..count)
.map(|index| { .map(|index| {
let tool_name = format!("tool_{index:03}"); let tool_name = format!("tool_{index:03}");
( tool_info(server_name, &tool_name, description_prefix)
format!("mcp__{server_name}__{tool_name}"),
tool_info(server_name, &tool_name, description_prefix),
)
}) })
.collect() .collect()
} }
@@ -427,7 +412,7 @@ mod tests {
} }
fn handler_from_tools( fn handler_from_tools(
mcp_tools: Option<&std::collections::HashMap<String, ToolInfo>>, mcp_tools: Option<&[ToolInfo]>,
dynamic_tools: &[DynamicToolSpec], dynamic_tools: &[DynamicToolSpec],
) -> ToolSearchHandler { ) -> ToolSearchHandler {
ToolSearchHandler::new(build_tool_search_entries(mcp_tools, dynamic_tools)) ToolSearchHandler::new(build_tool_search_entries(mcp_tools, dynamic_tools))
codex-rs/core/src/tools/router.rs
+2 -3
View File
@@ -21,7 +21,6 @@ use codex_tools::ResponsesApiNamespaceTool;
use codex_tools::ToolName; use codex_tools::ToolName;
use codex_tools::ToolSpec; use codex_tools::ToolSpec;
use codex_tools::ToolsConfig; use codex_tools::ToolsConfig;
use std::collections::HashMap;
use std::collections::HashSet; use std::collections::HashSet;
use std::sync::Arc; use std::sync::Arc;
use tokio_util::sync::CancellationToken; use tokio_util::sync::CancellationToken;
@@ -44,8 +43,8 @@ pub struct ToolRouter {
} }
pub(crate) struct ToolRouterParams<'a> { pub(crate) struct ToolRouterParams<'a> {
pub(crate) mcp_tools: Option<HashMap<String, ToolInfo>>, pub(crate) mcp_tools: Option<Vec<ToolInfo>>,
pub(crate) deferred_mcp_tools: Option<HashMap<String, ToolInfo>>, pub(crate) deferred_mcp_tools: Option<Vec<ToolInfo>>,
pub(crate) unavailable_called_tools: Vec<ToolName>, pub(crate) unavailable_called_tools: Vec<ToolName>,
pub(crate) parallel_mcp_server_names: HashSet<String>, pub(crate) parallel_mcp_server_names: HashSet<String>,
pub(crate) discoverable_tools: Option<Vec<DiscoverableTool>>, pub(crate) discoverable_tools: Option<Vec<DiscoverableTool>>,
codex-rs/core/src/tools/spec.rs
+8 -8
View File
@@ -38,17 +38,17 @@ struct McpToolPlanInputs<'a> {
tool_namespaces: HashMap<String, ToolNamespace>, tool_namespaces: HashMap<String, ToolNamespace>,
} }
fn map_mcp_tools_for_plan(mcp_tools: &HashMap<String, ToolInfo>) -> McpToolPlanInputs<'_> { fn map_mcp_tools_for_plan(mcp_tools: &[ToolInfo]) -> McpToolPlanInputs<'_> {
McpToolPlanInputs { McpToolPlanInputs {
mcp_tools: mcp_tools mcp_tools: mcp_tools
.values() .iter()
.map(|tool| ToolRegistryBuildMcpTool { .map(|tool| ToolRegistryBuildMcpTool {
name: tool.canonical_tool_name(), name: tool.canonical_tool_name(),
tool: &tool.tool, tool: &tool.tool,
}) })
.collect(), .collect(),
tool_namespaces: mcp_tools tool_namespaces: mcp_tools
.values() .iter()
.map(|tool| { .map(|tool| {
( (
tool.callable_namespace.clone(), tool.callable_namespace.clone(),
@@ -64,8 +64,8 @@ fn map_mcp_tools_for_plan(mcp_tools: &HashMap<String, ToolInfo>) -> McpToolPlanI
pub(crate) fn build_specs_with_discoverable_tools( pub(crate) fn build_specs_with_discoverable_tools(
config: &ToolsConfig, config: &ToolsConfig,
mcp_tools: Option<HashMap<String, ToolInfo>>, mcp_tools: Option<Vec<ToolInfo>>,
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>, deferred_mcp_tools: Option<Vec<ToolInfo>>,
unavailable_called_tools: Vec<ToolName>, unavailable_called_tools: Vec<ToolName>,
discoverable_tools: Option<Vec<DiscoverableTool>>, discoverable_tools: Option<Vec<DiscoverableTool>>,
dynamic_tools: &[DynamicToolSpec], dynamic_tools: &[DynamicToolSpec],
@@ -74,10 +74,10 @@ pub(crate) fn build_specs_with_discoverable_tools(
use crate::tools::handlers::unavailable_tool_message; use crate::tools::handlers::unavailable_tool_message;
use crate::tools::tool_search_entry::build_tool_search_entries_for_config; use crate::tools::tool_search_entry::build_tool_search_entries_for_config;
let mcp_tool_plan_inputs = mcp_tools.as_ref().map(map_mcp_tools_for_plan); let mcp_tool_plan_inputs = mcp_tools.as_deref().map(map_mcp_tools_for_plan);
let deferred_mcp_tool_sources = deferred_mcp_tools.as_ref().map(|tools| { let deferred_mcp_tool_sources = deferred_mcp_tools.as_ref().map(|tools| {
tools tools
.values() .iter()
.map(|tool| ToolRegistryBuildDeferredTool { .map(|tool| ToolRegistryBuildDeferredTool {
name: tool.canonical_tool_name(), name: tool.canonical_tool_name(),
server_name: tool.server_name.as_str(), server_name: tool.server_name.as_str(),
@@ -105,7 +105,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let tool_search_entries = build_tool_search_entries_for_config( let tool_search_entries = build_tool_search_entries_for_config(
config, config,
deferred_mcp_tools.as_ref(), deferred_mcp_tools.as_deref(),
&deferred_dynamic_tools, &deferred_dynamic_tools,
); );
let mut builder = build_tool_registry_builder( let mut builder = build_tool_registry_builder(
codex-rs/core/src/tools/spec_plan.rs
+8 -4
View File
@@ -101,6 +101,7 @@ use codex_tools::default_namespace_description;
use codex_tools::dynamic_tool_to_loadable_tool_spec; use codex_tools::dynamic_tool_to_loadable_tool_spec;
use codex_tools::mcp_tool_to_responses_api_tool; use codex_tools::mcp_tool_to_responses_api_tool;
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::collections::HashSet;
use std::sync::Arc; use std::sync::Arc;
pub fn build_tool_registry_builder( pub fn build_tool_registry_builder(
@@ -617,11 +618,14 @@ pub fn build_tool_registry_builder(
} }
if let Some(deferred_mcp_tools) = params.deferred_mcp_tools { if let Some(deferred_mcp_tools) = params.deferred_mcp_tools {
let directly_registered_mcp_tools = params
.mcp_tools
.into_iter()
.flatten()
.map(|direct| direct.name.clone())
.collect::<HashSet<_>>();
for tool in deferred_mcp_tools { for tool in deferred_mcp_tools {
let registered_directly = params if !directly_registered_mcp_tools.contains(&tool.name) {
.mcp_tools
.is_some_and(|mcp_tools| mcp_tools.iter().any(|direct| direct.name == tool.name));
if !registered_directly {
builder.register_handler(Arc::new(McpHandler::new(tool.name.clone()))); builder.register_handler(Arc::new(McpHandler::new(tool.name.clone())));
} }
} }
codex-rs/core/src/tools/spec_tests.rs
+136 -169
View File
@@ -266,8 +266,8 @@ async fn model_info_from_models_json(slug: &str) -> ModelInfo {
/// Builds the tool registry builder while collecting tool specs for later serialization. /// Builds the tool registry builder while collecting tool specs for later serialization.
fn build_specs( fn build_specs(
config: &ToolsConfig, config: &ToolsConfig,
mcp_tools: Option<HashMap<String, ToolInfo>>, mcp_tools: Option<Vec<ToolInfo>>,
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>, deferred_mcp_tools: Option<Vec<ToolInfo>>,
dynamic_tools: &[DynamicToolSpec], dynamic_tools: &[DynamicToolSpec],
) -> ToolRegistryBuilder { ) -> ToolRegistryBuilder {
build_specs_with_unavailable_tools( build_specs_with_unavailable_tools(
@@ -281,8 +281,8 @@ fn build_specs(
fn build_specs_with_unavailable_tools( fn build_specs_with_unavailable_tools(
config: &ToolsConfig, config: &ToolsConfig,
mcp_tools: Option<HashMap<String, ToolInfo>>, mcp_tools: Option<Vec<ToolInfo>>,
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>, deferred_mcp_tools: Option<Vec<ToolInfo>>,
unavailable_called_tools: Vec<ToolName>, unavailable_called_tools: Vec<ToolName>,
dynamic_tools: &[DynamicToolSpec], dynamic_tools: &[DynamicToolSpec],
) -> ToolRegistryBuilder { ) -> ToolRegistryBuilder {
@@ -630,7 +630,7 @@ async fn test_build_specs_default_shell_present() {
}); });
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
Some(HashMap::new()), Some(Vec::new()),
/*deferred_mcp_tools*/ None, /*deferred_mcp_tools*/ None,
&[], &[],
) )
@@ -856,7 +856,7 @@ async fn search_tool_description_handles_no_enabled_mcp_tools() {
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
/*mcp_tools*/ None, /*mcp_tools*/ None,
Some(HashMap::new()), Some(Vec::new()),
&[], &[],
) )
.build(); .build();
@@ -890,23 +890,20 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
/*mcp_tools*/ None, /*mcp_tools*/ None,
Some(HashMap::from([( Some(vec![ToolInfo {
"mcp__codex_apps__calendar_create_event".to_string(), server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
ToolInfo { callable_name: "_create_event".to_string(),
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), callable_namespace: "mcp__codex_apps__calendar".to_string(),
callable_name: "_create_event".to_string(), namespace_description: None,
callable_namespace: "mcp__codex_apps__calendar".to_string(), tool: mcp_tool(
namespace_description: None, "calendar_create_event",
tool: mcp_tool( "Create calendar event",
"calendar_create_event", serde_json::json!({"type": "object"}),
"Create calendar event", ),
serde_json::json!({"type": "object"}), connector_id: Some("calendar".to_string()),
), connector_name: Some("Calendar".to_string()),
connector_id: Some("calendar".to_string()), plugin_display_names: Vec::new(),
connector_name: Some("Calendar".to_string()), }]),
plugin_display_names: Vec::new(),
},
)])),
&[], &[],
) )
.build(); .build();
@@ -940,55 +937,46 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() {
let (_, registry) = build_specs( let (_, registry) = build_specs(
&tools_config, &tools_config,
/*mcp_tools*/ None, /*mcp_tools*/ None,
Some(HashMap::from([ Some(vec![
( ToolInfo {
"mcp__codex_apps__calendar_create_event".to_string(), server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
ToolInfo { callable_name: "_create_event".to_string(),
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), callable_namespace: "mcp__codex_apps__calendar".to_string(),
callable_name: "_create_event".to_string(), namespace_description: None,
callable_namespace: "mcp__codex_apps__calendar".to_string(), tool: mcp_tool(
namespace_description: None, "calendar-create-event",
tool: mcp_tool( "Create calendar event",
"calendar-create-event", serde_json::json!({"type": "object"}),
"Create calendar event", ),
serde_json::json!({"type": "object"}), connector_id: Some("calendar".to_string()),
), connector_name: Some("Calendar".to_string()),
connector_id: Some("calendar".to_string()), plugin_display_names: Vec::new(),
connector_name: Some("Calendar".to_string()), },
plugin_display_names: Vec::new(), ToolInfo {
}, server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
), callable_name: "_list_events".to_string(),
( callable_namespace: "mcp__codex_apps__calendar".to_string(),
"mcp__codex_apps__calendar_list_events".to_string(), namespace_description: None,
ToolInfo { tool: mcp_tool(
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), "calendar-list-events",
callable_name: "_list_events".to_string(), "List calendar events",
callable_namespace: "mcp__codex_apps__calendar".to_string(), serde_json::json!({"type": "object"}),
namespace_description: None, ),
tool: mcp_tool( connector_id: Some("calendar".to_string()),
"calendar-list-events", connector_name: Some("Calendar".to_string()),
"List calendar events", plugin_display_names: Vec::new(),
serde_json::json!({"type": "object"}), },
), ToolInfo {
connector_id: Some("calendar".to_string()), server_name: "rmcp".to_string(),
connector_name: Some("Calendar".to_string()), callable_name: "echo".to_string(),
plugin_display_names: Vec::new(), callable_namespace: "mcp__rmcp__".to_string(),
}, namespace_description: None,
), tool: mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})),
( connector_id: None,
"mcp__rmcp__echo".to_string(), connector_name: None,
ToolInfo { plugin_display_names: Vec::new(),
server_name: "rmcp".to_string(), },
callable_name: "echo".to_string(), ]),
callable_namespace: "mcp__rmcp__".to_string(),
namespace_description: None,
tool: mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})),
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
},
),
])),
&[], &[],
) )
.build(); .build();
@@ -1018,14 +1006,11 @@ async fn tool_search_entries_skip_namespace_outputs_when_namespace_tools_are_dis
windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled,
}); });
tools_config.namespace_tools = false; tools_config.namespace_tools = false;
let mcp_tools = HashMap::from([( let mcp_tools = vec![mcp_tool_info(mcp_tool(
"mcp__test_server__echo".to_string(), "echo",
mcp_tool_info(mcp_tool( "Echo",
"echo", serde_json::json!({"type": "object"}),
"Echo", ))];
serde_json::json!({"type": "object"}),
)),
)]);
let dynamic_tools = vec![ let dynamic_tools = vec![
DynamicToolSpec { DynamicToolSpec {
namespace: Some("codex_app".to_string()), namespace: Some("codex_app".to_string()),
@@ -1077,14 +1062,11 @@ async fn direct_mcp_tools_register_namespaced_handlers() {
let (_, registry) = build_specs( let (_, registry) = build_specs(
&tools_config, &tools_config,
Some(HashMap::from([( Some(vec![mcp_tool_info(mcp_tool(
"mcp__test_server__echo".to_string(), "echo",
mcp_tool_info(mcp_tool( "Echo",
"echo", serde_json::json!({"type": "object"}),
"Echo", ))]),
serde_json::json!({"type": "object"}),
)),
)])),
/*deferred_mcp_tools*/ None, /*deferred_mcp_tools*/ None,
&[], &[],
) )
@@ -1163,22 +1145,19 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() {
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
Some(HashMap::from([( Some(vec![mcp_tool_info_with_display_name(
"dash/search".to_string(), "dash/search",
mcp_tool_info_with_display_name( mcp_tool(
"dash/search", "search",
mcp_tool( "Search docs",
"search", serde_json::json!({
"Search docs", "type": "object",
serde_json::json!({ "properties": {
"type": "object", "query": {"description": "search query"}
"properties": { }
"query": {"description": "search query"} }),
}
}),
),
), ),
)])), )]),
/*deferred_mcp_tools*/ None, /*deferred_mcp_tools*/ None,
&[], &[],
) )
@@ -1226,20 +1205,17 @@ async fn test_mcp_tool_preserves_integer_schema() {
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
Some(HashMap::from([( Some(vec![mcp_tool_info_with_display_name(
"dash/paginate".to_string(), "dash/paginate",
mcp_tool_info_with_display_name( mcp_tool(
"dash/paginate", "paginate",
mcp_tool( "Pagination",
"paginate", serde_json::json!({
"Pagination", "type": "object",
serde_json::json!({ "properties": {"page": {"type": "integer"}}
"type": "object", }),
"properties": {"page": {"type": "integer"}}
}),
),
), ),
)])), )]),
/*deferred_mcp_tools*/ None, /*deferred_mcp_tools*/ None,
&[], &[],
) )
@@ -1288,20 +1264,17 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() {
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
Some(HashMap::from([( Some(vec![mcp_tool_info_with_display_name(
"dash/tags".to_string(), "dash/tags",
mcp_tool_info_with_display_name( mcp_tool(
"dash/tags", "tags",
mcp_tool( "Tags",
"tags", serde_json::json!({
"Tags", "type": "object",
serde_json::json!({ "properties": {"tags": {"type": "array"}}
"type": "object", }),
"properties": {"tags": {"type": "array"}}
}),
),
), ),
)])), )]),
/*deferred_mcp_tools*/ None, /*deferred_mcp_tools*/ None,
&[], &[],
) )
@@ -1352,22 +1325,19 @@ async fn test_mcp_tool_anyof_defaults_to_string() {
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
Some(HashMap::from([( Some(vec![mcp_tool_info_with_display_name(
"dash/value".to_string(), "dash/value",
mcp_tool_info_with_display_name( mcp_tool(
"dash/value", "value",
mcp_tool( "AnyOf Value",
"value", serde_json::json!({
"AnyOf Value", "type": "object",
serde_json::json!({ "properties": {
"type": "object", "value": {"anyOf": [{"type": "string"}, {"type": "number"}]}
"properties": { }
"value": {"anyOf": [{"type": "string"}, {"type": "number"}]} }),
}
}),
),
), ),
)])), )]),
/*deferred_mcp_tools*/ None, /*deferred_mcp_tools*/ None,
&[], &[],
) )
@@ -1420,39 +1390,36 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() {
}); });
let (tools, _) = build_specs( let (tools, _) = build_specs(
&tools_config, &tools_config,
Some(HashMap::from([( Some(vec![mcp_tool_info_with_display_name(
"test_server/do_something_cool".to_string(), "test_server/do_something_cool",
mcp_tool_info_with_display_name( mcp_tool(
"test_server/do_something_cool", "do_something_cool",
mcp_tool( "Do something cool",
"do_something_cool", serde_json::json!({
"Do something cool", "type": "object",
serde_json::json!({ "properties": {
"string_argument": {"type": "string"},
"number_argument": {"type": "number"},
"object_argument": {
"type": "object", "type": "object",
"properties": { "properties": {
"string_argument": {"type": "string"}, "string_property": {"type": "string"},
"number_argument": {"type": "number"}, "number_property": {"type": "number"}
"object_argument": { },
"required": ["string_property", "number_property"],
"additionalProperties": {
"type": "object", "type": "object",
"properties": { "properties": {
"string_property": {"type": "string"}, "addtl_prop": {"type": "string"}
"number_property": {"type": "number"}
}, },
"required": ["string_property", "number_property"], "required": ["addtl_prop"],
"additionalProperties": { "additionalProperties": false
"type": "object",
"properties": {
"addtl_prop": {"type": "string"}
},
"required": ["addtl_prop"],
"additionalProperties": false
}
} }
} }
}), }
), }),
), ),
)])), )]),
/*deferred_mcp_tools*/ None, /*deferred_mcp_tools*/ None,
&[], &[],
) )
codex-rs/core/src/tools/tool_search_entry.rs
+3 -4
View File
@@ -5,7 +5,6 @@ use codex_tools::ToolSearchResultSource;
use codex_tools::ToolsConfig; use codex_tools::ToolsConfig;
use codex_tools::dynamic_tool_to_loadable_tool_spec; use codex_tools::dynamic_tool_to_loadable_tool_spec;
use codex_tools::tool_search_result_source_to_loadable_tool_spec; use codex_tools::tool_search_result_source_to_loadable_tool_spec;
use std::collections::HashMap;
#[derive(Clone)] #[derive(Clone)]
pub(crate) struct ToolSearchEntry { pub(crate) struct ToolSearchEntry {
@@ -15,13 +14,13 @@ pub(crate) struct ToolSearchEntry {
} }
pub(crate) fn build_tool_search_entries( pub(crate) fn build_tool_search_entries(
mcp_tools: Option<&HashMap<String, ToolInfo>>, mcp_tools: Option<&[ToolInfo]>,
dynamic_tools: &[DynamicToolSpec], dynamic_tools: &[DynamicToolSpec],
) -> Vec<ToolSearchEntry> { ) -> Vec<ToolSearchEntry> {
let mut entries = Vec::new(); let mut entries = Vec::new();
let mut mcp_tools = mcp_tools let mut mcp_tools = mcp_tools
.map(|tools| tools.values().collect::<Vec<_>>()) .map(|tools| tools.iter().collect::<Vec<_>>())
.unwrap_or_default(); .unwrap_or_default();
mcp_tools.sort_by_key(|info| info.canonical_tool_name().display()); mcp_tools.sort_by_key(|info| info.canonical_tool_name().display());
for info in mcp_tools { for info in mcp_tools {
@@ -55,7 +54,7 @@ pub(crate) fn build_tool_search_entries(
pub(crate) fn build_tool_search_entries_for_config( pub(crate) fn build_tool_search_entries_for_config(
config: &ToolsConfig, config: &ToolsConfig,
mcp_tools: Option<&HashMap<String, ToolInfo>>, mcp_tools: Option<&[ToolInfo]>,
dynamic_tools: &[DynamicToolSpec], dynamic_tools: &[DynamicToolSpec],
) -> Vec<ToolSearchEntry> { ) -> Vec<ToolSearchEntry> {
let mcp_tools = if config.namespace_tools { let mcp_tools = if config.namespace_tools {
codex-rs/core/src/unavailable_tool.rs
+23 -3
View File
@@ -6,9 +6,13 @@ use codex_tools::ToolName;
pub(crate) fn collect_unavailable_called_tools( pub(crate) fn collect_unavailable_called_tools(
input: &[ResponseItem], input: &[ResponseItem],
exposed_tool_names: &HashSet<&str>, exposed_tool_names: &HashSet<ToolName>,
) -> Vec<ToolName> { ) -> Vec<ToolName> {
let mut unavailable_tools = BTreeMap::new(); let mut unavailable_tools = BTreeMap::new();
let exposed_display_names = exposed_tool_names
.iter()
.map(ToolName::display)
.collect::<HashSet<_>>();
for item in input { for item in input {
let ResponseItem::FunctionCall { let ResponseItem::FunctionCall {
@@ -26,7 +30,7 @@ pub(crate) fn collect_unavailable_called_tools(
None => ToolName::plain(name.clone()), None => ToolName::plain(name.clone()),
}; };
let display_name = tool_name.display(); let display_name = tool_name.display();
if exposed_tool_names.contains(display_name.as_str()) { if exposed_display_names.contains(&display_name) {
continue; continue;
} }
@@ -78,7 +82,10 @@ mod tests {
#[test] #[test]
fn collect_unavailable_called_tools_skips_currently_available_tools() { fn collect_unavailable_called_tools_skips_currently_available_tools() {
let exposed_tool_names = HashSet::from(["mcp__server__lookup", "mcp__server__search"]); let exposed_tool_names = HashSet::from([
ToolName::plain("mcp__server__lookup"),
ToolName::plain("mcp__server__search"),
]);
let input = vec![ let input = vec![
function_call("mcp__server__lookup", /*namespace*/ None), function_call("mcp__server__lookup", /*namespace*/ None),
function_call("mcp__server__search", /*namespace*/ None), function_call("mcp__server__search", /*namespace*/ None),
@@ -89,4 +96,17 @@ mod tests {
assert_eq!(tools, vec![ToolName::plain("mcp__server__missing")]); assert_eq!(tools, vec![ToolName::plain("mcp__server__missing")]);
} }
#[test]
fn collect_unavailable_called_tools_matches_exposed_display_names() {
let exposed_tool_names = HashSet::from([ToolName::namespaced("mcp__server__", "lookup")]);
let input = vec![function_call(
"mcp__server__lookup",
/*namespace*/ None,
)];
let tools = collect_unavailable_called_tools(&input, &exposed_tool_names);
assert_eq!(tools, Vec::new());
}
} }