Remove string-keyed MCP tool maps

This commit is contained in:
pakrym-oai
2026-05-06 18:34:46 -07:00
parent 527d52df03
commit a101766ff4
19 changed files with 434 additions and 461 deletions

View File

@@ -5,7 +5,6 @@
//! connector allow-list filtering, and the normalization that turns app
//! connector/tool metadata into model-visible MCP callable names.
use std::collections::HashMap;
use std::path::PathBuf;
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)]
pub(crate) struct CodexAppsToolsCacheContext {
pub(crate) codex_home: PathBuf,

View File

@@ -31,7 +31,7 @@ use crate::runtime::McpRuntimeEnvironment;
use crate::runtime::emit_duration;
use crate::tools::ToolInfo;
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 anyhow::Context;
use anyhow::Result;
@@ -337,10 +337,9 @@ impl McpConnectionManager {
failures
}
/// Returns a single map that contains all tools. Each key is the
/// fully-qualified name for the tool.
/// Returns all tools with model-visible names normalized.
#[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();
for managed_client in self.clients.values() {
let Some(server_tools) = managed_client.listed_tools().await else {
@@ -348,15 +347,15 @@ impl McpConnectionManager {
};
tools.extend(server_tools);
}
qualify_tools(tools)
normalize_tools_for_model(tools)
}
/// Force-refresh codex apps tools by bypassing the in-process cache.
///
/// 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.
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
.clients
.get(CODEX_APPS_MCP_SERVER_NAME)
@@ -399,7 +398,7 @@ impl McpConnectionManager {
tool.tool = tool_with_model_visible_input_schema(&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
@@ -638,7 +637,7 @@ impl McpConnectionManager {
pub async fn resolve_tool_info(&self, tool_name: &ToolName) -> Option<ToolInfo> {
let all_tools = self.list_all_tools().await;
all_tools
.into_values()
.into_iter()
.find(|tool| tool.canonical_tool_name() == *tool_name)
}

View File

@@ -14,7 +14,7 @@ use crate::rmcp_client::elicitation_capability_for_server;
use crate::tools::ToolFilter;
use crate::tools::ToolInfo;
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 codex_config::Constrained;
use codex_protocol::ToolName;
@@ -85,6 +85,15 @@ fn create_codex_apps_tools_cache_context(
}
}
fn sorted_model_tool_names(tools: &[ToolInfo]) -> Vec<String> {
let mut names = tools
.iter()
.map(|tool| tool.canonical_tool_name().display())
.collect::<Vec<_>>();
names.sort();
names
}
#[test]
fn declared_openai_file_fields_treat_names_literally() {
let meta = serde_json::json!({
@@ -272,35 +281,41 @@ async fn disabled_permissions_do_not_auto_accept_elicitation_with_requested_fiel
}
#[test]
fn test_qualify_tools_short_non_duplicated_names() {
fn test_normalize_tools_short_non_duplicated_names() {
let tools = vec![
create_test_tool("server1", "tool1"),
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!(qualified_tools.contains_key("mcp__server1__tool1"));
assert!(qualified_tools.contains_key("mcp__server1__tool2"));
assert_eq!(
sorted_model_tool_names(&model_tools),
vec![
"mcp__server1__tool1".to_string(),
"mcp__server1__tool2".to_string()
]
);
}
#[test]
fn test_qualify_tools_duplicated_names_skipped() {
fn test_normalize_tools_duplicated_names_skipped() {
let tools = vec![
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
assert_eq!(qualified_tools.len(), 1);
assert!(qualified_tools.contains_key("mcp__server1__duplicate_tool"));
assert_eq!(
sorted_model_tool_names(&model_tools),
vec!["mcp__server1__duplicate_tool".to_string()]
);
}
#[test]
fn test_qualify_tools_long_names_same_server() {
fn test_normalize_tools_long_names_same_server() {
let server_name = "my_server";
let tools = vec![
@@ -314,116 +329,125 @@ 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();
keys.sort();
let names = sorted_model_tool_names(&model_tools);
assert!(keys.iter().all(|key| key.len() == 64));
assert!(keys.iter().all(|key| key.starts_with("mcp__my_server__")));
assert!(names.iter().all(|name| name.len() == 64));
assert!(
keys.iter()
.all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')),
"qualified names must be code-mode compatible: {keys:?}"
names
.iter()
.all(|name| name.starts_with("mcp__my_server__"))
);
assert!(
names
.iter()
.all(|name| name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')),
"model-visible names must be code-mode compatible: {names:?}"
);
}
#[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 qualified_tools = qualify_tools(tools);
let model_tools = normalize_tools_for_model(tools);
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!(model_tools.len(), 1);
let tool = model_tools.into_iter().next().expect("one tool");
let model_name = tool.canonical_tool_name().display();
assert_eq!(model_name, "mcp__server_one__tool_two_three");
assert_eq!(
format!("{}{}", tool.callable_namespace, tool.callable_name),
qualified_name
model_name
);
// The key and callable parts are sanitized for model-visible tool calls, but
// the raw MCP name is preserved for the actual MCP call.
// The 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.callable_namespace, "mcp__server_one__");
assert_eq!(tool.callable_name, "tool_two_three");
assert_eq!(tool.tool.name, "tool.two-three");
assert!(
qualified_name
model_name
.chars()
.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]
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 qualified_tools = qualify_tools(tools);
let model_tools = normalize_tools_for_model(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!(model_tools.len(), 1);
let tool = model_tools.into_iter().next().expect("one tool");
assert_eq!(
tool.canonical_tool_name().display(),
"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() {
fn test_normalize_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);
let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 2);
let mut namespaces = qualified_tools
.values()
assert_eq!(model_tools.len(), 2);
let mut namespaces = model_tools
.iter()
.map(|tool| tool.callable_namespace.as_str())
.collect::<Vec<_>>();
namespaces.sort();
namespaces.dedup();
assert_eq!(namespaces.len(), 2);
let raw_servers = qualified_tools
.values()
let raw_servers = model_tools
.iter()
.map(|tool| tool.server_name.as_str())
.collect::<HashSet<_>>();
assert_eq!(raw_servers, HashSet::from(["basic-server", "basic_server"]));
let model_names = sorted_model_tool_names(&model_tools);
assert!(
qualified_tools
.keys()
.all(|key| key.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')),
"qualified names must be code-mode compatible: {qualified_tools:?}"
model_names
.iter()
.all(|name| name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')),
"model-visible names must be code-mode compatible: {model_names:?}"
);
}
#[test]
fn test_qualify_tools_disambiguates_sanitized_tool_name_collisions() {
fn test_normalize_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);
let model_tools = normalize_tools_for_model(tools);
assert_eq!(qualified_tools.len(), 2);
let raw_tool_names = qualified_tools
.values()
assert_eq!(model_tools.len(), 2);
let raw_tool_names = model_tools
.iter()
.map(|tool| tool.tool.name.to_string())
.collect::<HashSet<_>>();
assert_eq!(
raw_tool_names,
HashSet::from(["tool-name".to_string(), "tool_name".to_string()])
);
let callable_tool_names = qualified_tools
.values()
let callable_tool_names = model_tools
.iter()
.map(|tool| tool.callable_name.as_str())
.collect::<HashSet<_>>();
assert_eq!(callable_tool_names.len(), 2);
@@ -672,7 +696,10 @@ async fn list_all_tools_uses_startup_snapshot_while_client_is_pending() {
let tools = manager.list_all_tools().await;
let tool = tools
.get("mcp__codex_apps__calendar_create_event")
.iter()
.find(|tool| {
tool.canonical_tool_name().display() == "mcp__codex_apps__calendar_create_event"
})
.expect("tool from startup cache");
assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME);
assert_eq!(tool.callable_name, "calendar_create_event");
@@ -798,7 +825,10 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() {
let tools = manager.list_all_tools().await;
let tool = tools
.get("mcp__codex_apps__calendar_create_event")
.iter()
.find(|tool| {
tool.canonical_tool_name().display() == "mcp__codex_apps__calendar_create_event"
})
.expect("tool from startup cache");
assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME);
assert_eq!(tool.callable_name, "calendar_create_event");

View File

@@ -48,7 +48,6 @@ pub use mcp::oauth_login_support;
pub use mcp::resolve_oauth_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::mcp_permission_prompt_is_auto_approved;
pub use mcp::qualified_mcp_tool_name_prefix;

View File

@@ -564,7 +564,7 @@ async fn collect_mcp_server_status_snapshot_from_manager(
);
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 Some(tool) = protocol_tool_from_rmcp_tool(&raw_tool_name, &tool_info.tool) else {
continue;

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
//! 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()
}
/// 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
/// `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<I>(tools: I) -> HashMap<String, ToolInfo>
/// every model-visible name is unique and <= 64 bytes.
pub(crate) fn normalize_tools_for_model<I>(tools: I) -> Vec<ToolInfo>
where
I: IntoIterator<Item = ToolInfo>,
{
@@ -213,9 +213,9 @@ where
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();
let mut model_tools = Vec::new();
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_name,
&candidate.raw_tool_identity,
@@ -223,9 +223,9 @@ where
);
candidate.tool.callable_namespace = callable_namespace;
candidate.tool.callable_name = callable_name;
qualified_tools.insert(qualified_name, candidate.tool);
model_tools.push(candidate.tool);
}
qualified_tools
model_tools
}
#[derive(Debug)]
@@ -345,10 +345,10 @@ fn unique_callable_parts(
tool_name: &str,
raw_identity: &str,
used_names: &mut HashSet<String>,
) -> (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);
) -> (String, String) {
let model_name = format!("{namespace}{tool_name}");
if model_name.len() <= MAX_TOOL_NAME_LENGTH && used_names.insert(model_name) {
return (namespace.to_string(), tool_name.to_string());
}
let mut attempt = 0_u32;
@@ -360,9 +360,9 @@ fn unique_callable_parts(
};
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);
let model_name = format!("{namespace}{tool_name}");
if used_names.insert(model_name) {
return (namespace, tool_name);
}
attempt = attempt.saturating_add(1);
}

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(
config: &Config,
auth: Option<&CodexAuth>,
mcp_tools: &HashMap<String, ToolInfo>,
mcp_tools: &[ToolInfo],
) {
if !config.features.enabled(Feature::Apps) {
return;
@@ -516,12 +516,10 @@ async fn chatgpt_get_request_with_auth_provider<T: DeserializeOwned>(
}
}
pub(crate) fn accessible_connectors_from_mcp_tools(
mcp_tools: &HashMap<String, ToolInfo>,
) -> Vec<AppInfo> {
pub(crate) fn accessible_connectors_from_mcp_tools(mcp_tools: &[ToolInfo]) -> Vec<AppInfo> {
// ToolInfo already carries plugin provenance, so app-level plugin sources
// 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 {
return None;
}

View File

@@ -172,39 +172,30 @@ fn merge_connectors_replaces_plugin_placeholder_name_with_accessible_name() {
#[test]
fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() {
let tools = HashMap::from([
(
"mcp__codex_apps__calendar_list_events".to_string(),
codex_app_tool(
"calendar_list_events",
"calendar",
/*connector_name*/ None,
&["sample", "sample"],
),
let tools = vec![
codex_app_tool(
"calendar_list_events",
"calendar",
/*connector_name*/ None,
&["sample", "sample"],
),
(
"mcp__codex_apps__calendar_create_event".to_string(),
codex_app_tool(
"calendar_create_event",
"calendar",
Some("Google Calendar"),
&["beta", "sample"],
),
codex_app_tool(
"calendar_create_event",
"calendar",
Some("Google Calendar"),
&["beta", "sample"],
),
(
"mcp__sample__echo".to_string(),
ToolInfo {
server_name: "sample".to_string(),
callable_name: "echo".to_string(),
callable_namespace: "sample".to_string(),
namespace_description: None,
tool: test_tool_definition("echo"),
connector_id: None,
connector_name: None,
plugin_display_names: plugin_names(&["ignored"]),
},
),
]);
ToolInfo {
server_name: "sample".to_string(),
callable_name: "echo".to_string(),
callable_namespace: "sample".to_string(),
namespace_description: None,
tool: test_tool_definition("echo"),
connector_id: None,
connector_name: None,
plugin_display_names: plugin_names(&["ignored"]),
},
];
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");
let _ = config.features.set_enabled(Feature::Apps, /*enabled*/ true);
let cache_key = accessible_connectors_cache_key(&config, /*auth*/ None);
let tools = HashMap::from([
(
"mcp__codex_apps__calendar_list_events".to_string(),
codex_app_tool(
"calendar_list_events",
"calendar",
Some("Google Calendar"),
&["calendar-plugin"],
),
let tools = vec![
codex_app_tool(
"calendar_list_events",
"calendar",
Some("Google Calendar"),
&["calendar-plugin"],
),
(
"mcp__codex_apps__openai_hidden".to_string(),
codex_app_tool(
"openai_hidden",
"connector_openai_hidden",
Some("Hidden"),
&[],
),
codex_app_tool(
"openai_hidden",
"connector_openai_hidden",
Some("Hidden"),
&[],
),
]);
];
let cached = with_accessible_connectors_cache_cleared(|| {
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]
fn accessible_connectors_from_mcp_tools_preserves_description() {
let mcp_tools = HashMap::from([(
"mcp__codex_apps__calendar_create_event".to_string(),
ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "calendar_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: Some("Plan events".to_string()),
tool: Tool {
name: "calendar_create_event".to_string().into(),
title: None,
description: Some("Create a calendar event".into()),
input_schema: Arc::new(JsonObject::default()),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
let mcp_tools = vec![ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "calendar_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: Some("Plan events".to_string()),
tool: Tool {
name: "calendar_create_event".to_string().into(),
title: None,
description: Some("Create a calendar event".into()),
input_schema: Arc::new(JsonObject::default()),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
)]);
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
}];
assert_eq!(
accessible_connectors_from_mcp_tools(&mcp_tools),

View File

@@ -1477,7 +1477,7 @@ pub(crate) async fn lookup_mcp_tool_metadata(
.list_all_tools()
.await;
let tool_info = tools
.into_values()
.into_iter()
.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 connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools(
@@ -1572,7 +1572,7 @@ async fn lookup_mcp_app_usage_metadata(
.list_all_tools()
.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 {
Some(McpAppUsageMetadata {
connector_id: tool_info.connector_id,

View File

@@ -1,10 +1,8 @@
use std::collections::HashMap;
use std::collections::HashSet;
use codex_features::Feature;
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;
@@ -13,12 +11,12 @@ use crate::connectors;
pub(crate) const DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD: usize = 100;
pub(crate) struct McpToolExposure {
pub(crate) direct_tools: HashMap<String, McpToolInfo>,
pub(crate) deferred_tools: Option<HashMap<String, McpToolInfo>>,
pub(crate) direct_tools: Vec<McpToolInfo>,
pub(crate) deferred_tools: Option<Vec<McpToolInfo>>,
}
pub(crate) fn build_mcp_tool_exposure(
all_mcp_tools: &HashMap<String, McpToolInfo>,
all_mcp_tools: &[McpToolInfo],
connectors: Option<&[connectors::AppInfo]>,
explicitly_enabled_connectors: &[connectors::AppInfo],
config: &Config,
@@ -48,9 +46,11 @@ pub(crate) fn build_mcp_tool_exposure(
let direct_tools =
filter_codex_apps_mcp_tools(all_mcp_tools, explicitly_enabled_connectors, config);
for direct_tool_name in direct_tools.keys() {
deferred_tools.remove(direct_tool_name);
}
let direct_tool_names = direct_tools
.iter()
.map(McpToolInfo::canonical_tool_name)
.collect::<HashSet<_>>();
deferred_tools.retain(|tool| !direct_tool_names.contains(&tool.canonical_tool_name()));
McpToolExposure {
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(
mcp_tools: &HashMap<String, McpToolInfo>,
mcp_tools: &[McpToolInfo],
connectors: &[connectors::AppInfo],
config: &Config,
) -> HashMap<String, McpToolInfo> {
) -> Vec<McpToolInfo> {
let allowed: HashSet<&str> = connectors
.iter()
.map(|connector| connector.id.as_str())
@@ -70,7 +78,7 @@ fn filter_codex_apps_mcp_tools(
mcp_tools
.iter()
.filter(|(_, tool)| {
.filter(|tool| {
if tool.server_name != CODEX_APPS_MCP_SERVER_NAME {
return false;
}
@@ -79,7 +87,7 @@ fn filter_codex_apps_mcp_tools(
};
allowed.contains(connector_id) && connectors::codex_app_tool_is_enabled(config, tool)
})
.map(|(name, tool)| (name.clone(), tool.clone()))
.cloned()
.collect()
}

View File

@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
use codex_connectors::metadata::sanitize_name;
@@ -11,6 +11,7 @@ use codex_protocol::config_types::WebSearchMode;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::SessionSource;
use codex_tools::ToolName;
use codex_tools::ToolsConfig;
use codex_tools::ToolsConfigParams;
use pretty_assertions::assert_eq;
@@ -53,10 +54,25 @@ fn make_mcp_tool(
} else {
format!("mcp__{server_name}__")
};
let callable_name = if server_name == CODEX_APPS_MCP_SERVER_NAME {
let tool_name = sanitize_name(tool_name);
if let Some(connector_name) = connector_name
.map(sanitize_name)
.filter(|name| !name.is_empty())
&& let Some(stripped) = tool_name.strip_prefix(&connector_name)
&& !stripped.is_empty()
{
stripped.to_string()
} else {
tool_name
}
} else {
tool_name.to_string()
};
ToolInfo {
server_name: server_name.to_string(),
callable_name: tool_name.to_string(),
callable_name,
callable_namespace: tool_namespace,
namespace_description: None,
tool: Tool {
@@ -76,20 +92,24 @@ fn make_mcp_tool(
}
}
fn numbered_mcp_tools(count: usize) -> HashMap<String, ToolInfo> {
fn numbered_mcp_tools(count: usize) -> Vec<ToolInfo> {
(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,
),
make_mcp_tool(
"rmcp", &tool_name, /*connector_id*/ None, /*connector_name*/ None,
)
})
.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 {
let config = test_config().await;
let model_info =
@@ -124,11 +144,7 @@ async fn directly_exposes_small_effective_tool_sets() {
&tools_config,
);
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_eq!(tool_names(&exposure.direct_tools), tool_names(&mcp_tools));
assert!(exposure.deferred_tools.is_none());
}
@@ -151,11 +167,7 @@ async fn searches_large_effective_tool_sets() {
.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);
assert_eq!(tool_names(deferred_tools), tool_names(&mcp_tools));
}
#[tokio::test]
@@ -163,15 +175,12 @@ async fn directly_exposes_explicit_apps_without_deferred_overlap() {
let config = test_config().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);
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"),
),
)]);
mcp_tools.push(make_mcp_tool(
CODEX_APPS_MCP_SERVER_NAME,
"calendar_create_event",
Some("calendar"),
Some("Calendar"),
));
let connectors = vec![make_connector("calendar", "Calendar")];
let exposure = build_mcp_tool_exposure(
@@ -182,28 +191,32 @@ async fn directly_exposes_explicit_apps_without_deferred_overlap() {
&tools_config,
);
let mut tool_names: Vec<String> = exposure.direct_tools.into_keys().collect();
tool_names.sort();
let direct_tool_names = tool_names(&exposure.direct_tools);
assert_eq!(
tool_names,
vec!["mcp__codex_apps__calendar_create_event".to_string()]
direct_tool_names,
HashSet::from([ToolName::namespaced(
"mcp__codex_apps__calendar",
"_create_event"
)])
);
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)
);
let deferred_tools = exposure
.deferred_tools
.as_ref()
.expect("large tool sets should be discoverable through tool_search");
let deferred_tool_names = tool_names(deferred_tools);
assert!(
tool_names
.iter()
.all(|direct_tool_name| !deferred_tools.contains_key(direct_tool_name)),
"direct tools should not also be deferred: {tool_names:?}"
direct_tool_names.is_disjoint(&deferred_tool_names),
"direct tools should not also be deferred: {direct_tool_names:?}"
);
assert!(!deferred_tools.contains_key("mcp__codex_apps__calendar_create_event"));
assert!(deferred_tools.contains_key("mcp__rmcp__tool_0"));
assert!(!deferred_tool_names.contains(&ToolName::namespaced(
"mcp__codex_apps__calendar",
"_create_event"
)));
assert!(deferred_tool_names.contains(&ToolName::namespaced("mcp__rmcp__", "tool_0")));
}
#[tokio::test]
@@ -214,23 +227,17 @@ async fn always_defer_feature_preserves_explicit_apps() {
.enable(Feature::ToolSearchAlwaysDeferMcpTools)
.expect("test config should allow feature update");
let tools_config = tools_config_for_mcp_tool_exposure(/*search_tool*/ true).await;
let mcp_tools = HashMap::from([
(
"mcp__rmcp__tool".to_string(),
make_mcp_tool(
"rmcp", "tool", /*connector_id*/ None, /*connector_name*/ None,
),
let mcp_tools = vec![
make_mcp_tool(
"rmcp", "tool", /*connector_id*/ None, /*connector_name*/ None,
),
(
"mcp__codex_apps__calendar_create_event".to_string(),
make_mcp_tool(
CODEX_APPS_MCP_SERVER_NAME,
"calendar_create_event",
Some("calendar"),
Some("Calendar"),
),
make_mcp_tool(
CODEX_APPS_MCP_SERVER_NAME,
"calendar_create_event",
Some("calendar"),
Some("Calendar"),
),
]);
];
let connectors = vec![make_connector("calendar", "Calendar")];
let exposure = build_mcp_tool_exposure(
@@ -241,16 +248,22 @@ async fn always_defer_feature_preserves_explicit_apps() {
&tools_config,
);
let mut direct_tool_names: Vec<String> = exposure.direct_tools.into_keys().collect();
direct_tool_names.sort();
let direct_tool_names = tool_names(&exposure.direct_tools);
assert_eq!(
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
.deferred_tools
.as_ref()
.expect("MCP tools should be discoverable through tool_search");
assert!(deferred_tools.contains_key("mcp__rmcp__tool"));
assert!(!deferred_tools.contains_key("mcp__codex_apps__calendar_create_event"));
let deferred_tool_names = tool_names(deferred_tools);
assert!(deferred_tool_names.contains(&ToolName::namespaced("mcp__rmcp__", "tool")));
assert!(!deferred_tool_names.contains(&ToolName::namespaced(
"mcp__codex_apps__calendar",
"_create_event"
)));
}

View File

@@ -1,5 +1,4 @@
use std::collections::BTreeSet;
use std::collections::HashMap;
use codex_connectors::metadata::connector_display_label;
use codex_protocol::models::ResponseItem;
@@ -14,7 +13,7 @@ use codex_mcp::ToolInfo;
pub(crate) fn build_plugin_injections(
mentioned_plugins: &[PluginCapabilitySummary],
mcp_tools: &HashMap<String, ToolInfo>,
mcp_tools: &[ToolInfo],
available_connectors: &[connectors::AppInfo],
) -> Vec<ResponseItem> {
if mentioned_plugins.is_empty() {
@@ -27,7 +26,7 @@ pub(crate) fn build_plugin_injections(
.iter()
.filter_map(|plugin| {
let available_mcp_servers = mcp_tools
.values()
.iter()
.filter(|tool| {
tool.server_name != CODEX_APPS_MCP_SERVER_NAME
&& tool

View File

@@ -195,10 +195,10 @@ pub(crate) async fn run_turn(
{
Ok(mcp_tools) => mcp_tools,
Err(_) if turn_context.apps_enabled() => return None,
Err(_) => HashMap::new(),
Err(_) => Vec::new(),
}
} else {
HashMap::new()
Vec::new()
};
let available_connectors = if turn_context.apps_enabled() {
let connectors = codex_connectors::merge::merge_plugin_connectors_with_accessible(
@@ -1247,7 +1247,7 @@ pub(crate) async fn built_tools(
let exposed_tool_names = 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_unavailable_called_tools(input, &exposed_tool_names)
} else {

View File

@@ -204,19 +204,11 @@ mod tests {
}),
defer_loading: true,
}];
let handler = handler_from_tools(
Some(&std::collections::HashMap::from([
(
"mcp__calendar__create_event".to_string(),
tool_info("calendar", "create_event", "Create events"),
),
(
"mcp__calendar__list_events".to_string(),
tool_info("calendar", "list_events", "List events"),
),
])),
&dynamic_tools,
);
let mcp_tools = vec![
tool_info("calendar", "create_event", "Create events"),
tool_info("calendar", "list_events", "List events"),
];
let handler = handler_from_tools(Some(&mcp_tools), &dynamic_tools);
let results = [
&handler.entries[0],
&handler.entries[2],
@@ -376,18 +368,11 @@ mod tests {
assert!(count_results_for_server(&results, "other-server") <= TOOL_SEARCH_DEFAULT_LIMIT);
}
fn numbered_tools(
server_name: &str,
description_prefix: &str,
count: usize,
) -> std::collections::HashMap<String, ToolInfo> {
fn numbered_tools(server_name: &str, description_prefix: &str, count: usize) -> Vec<ToolInfo> {
(0..count)
.map(|index| {
let tool_name = format!("tool_{index:03}");
(
format!("mcp__{server_name}__{tool_name}"),
tool_info(server_name, &tool_name, description_prefix),
)
tool_info(server_name, &tool_name, description_prefix)
})
.collect()
}
@@ -427,7 +412,7 @@ mod tests {
}
fn handler_from_tools(
mcp_tools: Option<&std::collections::HashMap<String, ToolInfo>>,
mcp_tools: Option<&[ToolInfo]>,
dynamic_tools: &[DynamicToolSpec],
) -> ToolSearchHandler {
ToolSearchHandler::new(build_tool_search_entries(mcp_tools, dynamic_tools))

View File

@@ -21,7 +21,6 @@ use codex_tools::ResponsesApiNamespaceTool;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use codex_tools::ToolsConfig;
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
use tokio_util::sync::CancellationToken;
@@ -44,8 +43,8 @@ pub struct ToolRouter {
}
pub(crate) struct ToolRouterParams<'a> {
pub(crate) mcp_tools: Option<HashMap<String, ToolInfo>>,
pub(crate) deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
pub(crate) mcp_tools: Option<Vec<ToolInfo>>,
pub(crate) deferred_mcp_tools: Option<Vec<ToolInfo>>,
pub(crate) unavailable_called_tools: Vec<ToolName>,
pub(crate) parallel_mcp_server_names: HashSet<String>,
pub(crate) discoverable_tools: Option<Vec<DiscoverableTool>>,

View File

@@ -42,17 +42,17 @@ struct McpToolPlanInputs<'a> {
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 {
mcp_tools: mcp_tools
.values()
.iter()
.map(|tool| ToolRegistryPlanMcpTool {
name: tool.canonical_tool_name(),
tool: &tool.tool,
})
.collect(),
tool_namespaces: mcp_tools
.values()
.iter()
.map(|tool| {
(
tool.callable_namespace.clone(),
@@ -68,8 +68,8 @@ fn map_mcp_tools_for_plan(mcp_tools: &HashMap<String, ToolInfo>) -> McpToolPlanI
pub(crate) fn build_specs_with_discoverable_tools(
config: &ToolsConfig,
mcp_tools: Option<HashMap<String, ToolInfo>>,
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
mcp_tools: Option<Vec<ToolInfo>>,
deferred_mcp_tools: Option<Vec<ToolInfo>>,
unavailable_called_tools: Vec<ToolName>,
discoverable_tools: Option<Vec<DiscoverableTool>>,
dynamic_tools: &[DynamicToolSpec],
@@ -114,10 +114,10 @@ pub(crate) fn build_specs_with_discoverable_tools(
use crate::tools::tool_search_entry::build_tool_search_entries_for_config;
let mut builder = ToolRegistryBuilder::new();
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| {
tools
.values()
.iter()
.map(|tool| ToolRegistryPlanDeferredTool {
name: tool.canonical_tool_name(),
server_name: tool.server_name.as_str(),
@@ -279,7 +279,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
ToolHandlerKind::ToolSearch => {
let entries = build_tool_search_entries_for_config(
config,
deferred_mcp_tools.as_ref(),
deferred_mcp_tools.as_deref(),
&deferred_dynamic_tools,
);
builder.register_handler(Arc::new(ToolSearchHandler::new(entries)));
@@ -305,10 +305,13 @@ pub(crate) fn build_specs_with_discoverable_tools(
}
}
if let Some(deferred_mcp_tools) = deferred_mcp_tools.as_ref() {
for (_, tool) in deferred_mcp_tools.iter().filter(|(name, _)| {
!mcp_tools
.as_ref()
.is_some_and(|tools| tools.contains_key(*name))
for tool in deferred_mcp_tools.iter().filter(|tool| {
let tool_name = tool.canonical_tool_name();
!mcp_tools.as_ref().is_some_and(|direct_tools| {
direct_tools
.iter()
.any(|direct_tool| direct_tool.canonical_tool_name() == tool_name)
})
}) {
builder.register_handler(Arc::new(McpHandler::new(tool.canonical_tool_name())));
}

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.
fn build_specs(
config: &ToolsConfig,
mcp_tools: Option<HashMap<String, ToolInfo>>,
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
mcp_tools: Option<Vec<ToolInfo>>,
deferred_mcp_tools: Option<Vec<ToolInfo>>,
dynamic_tools: &[DynamicToolSpec],
) -> ToolRegistryBuilder {
build_specs_with_unavailable_tools(
@@ -281,8 +281,8 @@ fn build_specs(
fn build_specs_with_unavailable_tools(
config: &ToolsConfig,
mcp_tools: Option<HashMap<String, ToolInfo>>,
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
mcp_tools: Option<Vec<ToolInfo>>,
deferred_mcp_tools: Option<Vec<ToolInfo>>,
unavailable_called_tools: Vec<ToolName>,
dynamic_tools: &[DynamicToolSpec],
) -> ToolRegistryBuilder {
@@ -630,7 +630,7 @@ async fn test_build_specs_default_shell_present() {
});
let (tools, _) = build_specs(
&tools_config,
Some(HashMap::new()),
Some(Vec::new()),
/*deferred_mcp_tools*/ None,
&[],
)
@@ -856,7 +856,7 @@ async fn search_tool_description_handles_no_enabled_mcp_tools() {
let (tools, _) = build_specs(
&tools_config,
/*mcp_tools*/ None,
Some(HashMap::new()),
Some(Vec::new()),
&[],
)
.build();
@@ -890,23 +890,20 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
let (tools, _) = build_specs(
&tools_config,
/*mcp_tools*/ None,
Some(HashMap::from([(
"mcp__codex_apps__calendar_create_event".to_string(),
ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: None,
tool: mcp_tool(
"calendar_create_event",
"Create calendar event",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
},
)])),
Some(vec![ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: None,
tool: mcp_tool(
"calendar_create_event",
"Create calendar event",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
}]),
&[],
)
.build();
@@ -940,55 +937,46 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() {
let (_, registry) = build_specs(
&tools_config,
/*mcp_tools*/ None,
Some(HashMap::from([
(
"mcp__codex_apps__calendar_create_event".to_string(),
ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: None,
tool: mcp_tool(
"calendar-create-event",
"Create calendar event",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
},
),
(
"mcp__codex_apps__calendar_list_events".to_string(),
ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_list_events".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: None,
tool: mcp_tool(
"calendar-list-events",
"List calendar events",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
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(),
namespace_description: None,
tool: mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})),
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
},
),
])),
Some(vec![
ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
namespace_description: None,
tool: mcp_tool(
"calendar-create-event",
"Create calendar event",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
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(),
namespace_description: None,
tool: mcp_tool(
"calendar-list-events",
"List calendar events",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
},
ToolInfo {
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();
@@ -1018,14 +1006,11 @@ async fn tool_search_entries_skip_namespace_outputs_when_namespace_tools_are_dis
windows_sandbox_level: WindowsSandboxLevel::Disabled,
});
tools_config.namespace_tools = false;
let mcp_tools = HashMap::from([(
"mcp__test_server__echo".to_string(),
mcp_tool_info(mcp_tool(
"echo",
"Echo",
serde_json::json!({"type": "object"}),
)),
)]);
let mcp_tools = vec![mcp_tool_info(mcp_tool(
"echo",
"Echo",
serde_json::json!({"type": "object"}),
))];
let dynamic_tools = vec![
DynamicToolSpec {
namespace: Some("codex_app".to_string()),
@@ -1077,14 +1062,11 @@ async fn direct_mcp_tools_register_namespaced_handlers() {
let (_, registry) = build_specs(
&tools_config,
Some(HashMap::from([(
"mcp__test_server__echo".to_string(),
mcp_tool_info(mcp_tool(
"echo",
"Echo",
serde_json::json!({"type": "object"}),
)),
)])),
Some(vec![mcp_tool_info(mcp_tool(
"echo",
"Echo",
serde_json::json!({"type": "object"}),
))]),
/*deferred_mcp_tools*/ None,
&[],
)
@@ -1163,22 +1145,19 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() {
let (tools, _) = build_specs(
&tools_config,
Some(HashMap::from([(
"dash/search".to_string(),
mcp_tool_info_with_display_name(
"dash/search",
mcp_tool(
"search",
"Search docs",
serde_json::json!({
"type": "object",
"properties": {
"query": {"description": "search query"}
}
}),
),
Some(vec![mcp_tool_info_with_display_name(
"dash/search",
mcp_tool(
"search",
"Search docs",
serde_json::json!({
"type": "object",
"properties": {
"query": {"description": "search query"}
}
}),
),
)])),
)]),
/*deferred_mcp_tools*/ None,
&[],
)
@@ -1226,20 +1205,17 @@ async fn test_mcp_tool_preserves_integer_schema() {
let (tools, _) = build_specs(
&tools_config,
Some(HashMap::from([(
"dash/paginate".to_string(),
mcp_tool_info_with_display_name(
"dash/paginate",
mcp_tool(
"paginate",
"Pagination",
serde_json::json!({
"type": "object",
"properties": {"page": {"type": "integer"}}
}),
),
Some(vec![mcp_tool_info_with_display_name(
"dash/paginate",
mcp_tool(
"paginate",
"Pagination",
serde_json::json!({
"type": "object",
"properties": {"page": {"type": "integer"}}
}),
),
)])),
)]),
/*deferred_mcp_tools*/ None,
&[],
)
@@ -1288,20 +1264,17 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() {
let (tools, _) = build_specs(
&tools_config,
Some(HashMap::from([(
"dash/tags".to_string(),
mcp_tool_info_with_display_name(
"dash/tags",
mcp_tool(
"tags",
"Tags",
serde_json::json!({
"type": "object",
"properties": {"tags": {"type": "array"}}
}),
),
Some(vec![mcp_tool_info_with_display_name(
"dash/tags",
mcp_tool(
"tags",
"Tags",
serde_json::json!({
"type": "object",
"properties": {"tags": {"type": "array"}}
}),
),
)])),
)]),
/*deferred_mcp_tools*/ None,
&[],
)
@@ -1352,22 +1325,19 @@ async fn test_mcp_tool_anyof_defaults_to_string() {
let (tools, _) = build_specs(
&tools_config,
Some(HashMap::from([(
"dash/value".to_string(),
mcp_tool_info_with_display_name(
"dash/value",
mcp_tool(
"value",
"AnyOf Value",
serde_json::json!({
"type": "object",
"properties": {
"value": {"anyOf": [{"type": "string"}, {"type": "number"}]}
}
}),
),
Some(vec![mcp_tool_info_with_display_name(
"dash/value",
mcp_tool(
"value",
"AnyOf Value",
serde_json::json!({
"type": "object",
"properties": {
"value": {"anyOf": [{"type": "string"}, {"type": "number"}]}
}
}),
),
)])),
)]),
/*deferred_mcp_tools*/ None,
&[],
)
@@ -1420,39 +1390,36 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() {
});
let (tools, _) = build_specs(
&tools_config,
Some(HashMap::from([(
"test_server/do_something_cool".to_string(),
mcp_tool_info_with_display_name(
"test_server/do_something_cool",
mcp_tool(
"do_something_cool",
"Do something cool",
serde_json::json!({
Some(vec![mcp_tool_info_with_display_name(
"test_server/do_something_cool",
mcp_tool(
"do_something_cool",
"Do something cool",
serde_json::json!({
"type": "object",
"properties": {
"string_argument": {"type": "string"},
"number_argument": {"type": "number"},
"object_argument": {
"type": "object",
"properties": {
"string_argument": {"type": "string"},
"number_argument": {"type": "number"},
"object_argument": {
"string_property": {"type": "string"},
"number_property": {"type": "number"}
},
"required": ["string_property", "number_property"],
"additionalProperties": {
"type": "object",
"properties": {
"string_property": {"type": "string"},
"number_property": {"type": "number"}
"addtl_prop": {"type": "string"}
},
"required": ["string_property", "number_property"],
"additionalProperties": {
"type": "object",
"properties": {
"addtl_prop": {"type": "string"}
},
"required": ["addtl_prop"],
"additionalProperties": false
}
"required": ["addtl_prop"],
"additionalProperties": false
}
}
}),
),
}
}),
),
)])),
)]),
/*deferred_mcp_tools*/ None,
&[],
)

View File

@@ -5,7 +5,6 @@ use codex_tools::ToolSearchResultSource;
use codex_tools::ToolsConfig;
use codex_tools::dynamic_tool_to_loadable_tool_spec;
use codex_tools::tool_search_result_source_to_loadable_tool_spec;
use std::collections::HashMap;
#[derive(Clone)]
pub(crate) struct ToolSearchEntry {
@@ -15,13 +14,13 @@ pub(crate) struct ToolSearchEntry {
}
pub(crate) fn build_tool_search_entries(
mcp_tools: Option<&HashMap<String, ToolInfo>>,
mcp_tools: Option<&[ToolInfo]>,
dynamic_tools: &[DynamicToolSpec],
) -> Vec<ToolSearchEntry> {
let mut entries = Vec::new();
let mut mcp_tools = mcp_tools
.map(|tools| tools.values().collect::<Vec<_>>())
.map(|tools| tools.iter().collect::<Vec<_>>())
.unwrap_or_default();
mcp_tools.sort_by_key(|info| info.canonical_tool_name().display());
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(
config: &ToolsConfig,
mcp_tools: Option<&HashMap<String, ToolInfo>>,
mcp_tools: Option<&[ToolInfo]>,
dynamic_tools: &[DynamicToolSpec],
) -> Vec<ToolSearchEntry> {
let mcp_tools = if config.namespace_tools {

View File

@@ -6,7 +6,7 @@ use codex_tools::ToolName;
pub(crate) fn collect_unavailable_called_tools(
input: &[ResponseItem],
exposed_tool_names: &HashSet<&str>,
exposed_tool_names: &HashSet<ToolName>,
) -> Vec<ToolName> {
let mut unavailable_tools = BTreeMap::new();
@@ -25,11 +25,11 @@ pub(crate) fn collect_unavailable_called_tools(
Some(namespace) => ToolName::namespaced(namespace.clone(), name.clone()),
None => ToolName::plain(name.clone()),
};
let display_name = tool_name.display();
if exposed_tool_names.contains(display_name.as_str()) {
if exposed_tool_names.contains(&tool_name) {
continue;
}
let display_name = tool_name.display();
unavailable_tools
.entry(display_name)
.or_insert_with(|| tool_name);
@@ -78,7 +78,10 @@ mod tests {
#[test]
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![
function_call("mcp__server__lookup", /*namespace*/ None),
function_call("mcp__server__search", /*namespace*/ None),