clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 06:35:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Normalize /mcp tool grouping for hyphenated server names

Browse Source
This commit is contained in:
pakrym-oai
2026-03-26 16:52:29 -10:00
parent 41fe98b185
commit 7a98b41b19
5 changed files with 154 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
codex-rs/core/src/mcp/mod.rs
View File

@@ -33,6 +33,32 @@ const MCP_TOOL_NAME_DELIMITER: &str = "__";
pub(crate) const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps";
const CODEX_CONNECTORS_TOKEN_ENV_VAR: &str = "CODEX_CONNECTORS_TOKEN";
/// The Responses API requires tool names to match `^[a-zA-Z0-9_-]+$`.
/// MCP server/tool names are user-controlled, so sanitize the fully-qualified
/// name we expose to the model by replacing any disallowed character with `_`.
pub(crate) fn sanitize_responses_api_tool_name(name: &str) -> String {
let mut sanitized = String::with_capacity(name.len());
for c in name.chars() {
if c.is_ascii_alphanumeric() || c == '_' {
sanitized.push(c);
} else {
sanitized.push('_');
}
}
if sanitized.is_empty() {
"_".to_string()
} else {
sanitized
}
}
pub fn qualified_mcp_tool_name_prefix(server_name: &str) -> String {
sanitize_responses_api_tool_name(&format!(
"{MCP_TOOL_NAME_PREFIX}{MCP_TOOL_NAME_DELIMITER}{server_name}{MCP_TOOL_NAME_DELIMITER}"
))
}
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct ToolPluginProvenance {
plugin_display_names_by_connector_id: HashMap<String, Vec<String>>,

8
codex-rs/core/src/mcp/mod_tests.rs
View File

@@ -52,6 +52,14 @@ fn split_qualified_tool_name_returns_server_and_tool() {
);
}
#[test]
fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() {
assert_eq!(
qualified_mcp_tool_name_prefix("Some-Server"),
"mcp__Some_Server__".to_string()
);
}
#[test]
fn split_qualified_tool_name_rejects_invalid_names() {
assert_eq!(split_qualified_tool_name("other__alpha__do_thing"), None);

21
codex-rs/core/src/mcp_connection_manager.rs
View File

@@ -22,6 +22,7 @@ use std::time::Instant;
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
use crate::mcp::ToolPluginProvenance;
use crate::mcp::auth::McpAuthStatusEntry;
use crate::mcp::sanitize_responses_api_tool_name;
use anyhow::Context;
use anyhow::Result;
use anyhow::anyhow;
@@ -104,26 +105,6 @@ const MCP_TOOLS_LIST_DURATION_METRIC: &str = "codex.mcp.tools.list.duration_ms";
const MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC: &str = "codex.mcp.tools.fetch_uncached.duration_ms";
const MCP_TOOLS_CACHE_WRITE_DURATION_METRIC: &str = "codex.mcp.tools.cache_write.duration_ms";
/// The Responses API requires tool names to match `^[a-zA-Z0-9_-]+$`.
/// MCP server/tool names are user-controlled, so sanitize the fully-qualified
/// name we expose to the model by replacing any disallowed character with `_`.
fn sanitize_responses_api_tool_name(name: &str) -> String {
let mut sanitized = String::with_capacity(name.len());
for c in name.chars() {
if c.is_ascii_alphanumeric() || c == '_' {
sanitized.push(c);
} else {
sanitized.push('_');
}
}
if sanitized.is_empty() {
"_".to_string()
} else {
sanitized
}
}
fn sha1_hex(s: &str) -> String {
let mut hasher = Sha1::new();
hasher.update(s.as_bytes());

60
codex-rs/tui/src/history_cell.rs
View File

@@ -42,6 +42,7 @@ use base64::Engine;
use codex_core::config::Config;
use codex_core::config::types::McpServerTransportConfig;
use codex_core::mcp::McpManager;
use codex_core::mcp::qualified_mcp_tool_name_prefix;
use codex_core::plugins::PluginsManager;
use codex_core::web_search::web_search_detail;
use codex_otel::RuntimeMetricsSummary;
@@ -1824,7 +1825,7 @@ pub(crate) fn new_mcp_tools_output(
servers.sort_by(|(a, _), (b, _)| a.cmp(b));
for (server, cfg) in servers {
let prefix = format!("mcp__{server}__");
let prefix = qualified_mcp_tool_name_prefix(server);
let mut names: Vec<String> = tools
.keys()
.filter(|k| k.starts_with(&prefix))
@@ -2988,6 +2989,63 @@ mod tests {
insta::assert_snapshot!(rendered);
}
#[tokio::test]
async fn mcp_tools_output_lists_tools_for_hyphenated_server_names() {
let mut config = test_config().await;
let mut servers = config.mcp_servers.get().clone();
servers.insert(
"some-server".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: "docs-server".to_string(),
args: vec!["--stdio".to_string()],
env: None,
env_vars: vec![],
cwd: None,
},
enabled: true,
required: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
},
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
let tools = HashMap::from([(
"mcp__some_server__lookup".to_string(),
Tool {
description: None,
name: "lookup".to_string(),
title: None,
input_schema: serde_json::json!({"type": "object", "properties": {}}),
output_schema: None,
annotations: None,
icons: None,
meta: None,
},
)]);
let auth_statuses: HashMap<String, McpAuthStatus> = HashMap::new();
let cell = new_mcp_tools_output(
&config,
tools,
HashMap::new(),
HashMap::new(),
&auth_statuses,
);
let rendered = render_lines(&cell.display_lines(120)).join("\n");
insta::assert_snapshot!(rendered);
}
#[test]
fn empty_agent_message_cell_transcript() {
let cell = AgentMessageCell::new(vec![Line::default()], false);

61
codex-rs/tui_app_server/src/history_cell.rs
View File

@@ -45,6 +45,8 @@ use codex_core::config::types::McpServerTransportConfig;
#[cfg(test)]
use codex_core::mcp::McpManager;
#[cfg(test)]
use codex_core::mcp::qualified_mcp_tool_name_prefix;
#[cfg(test)]
use codex_core::plugins::PluginsManager;
use codex_core::web_search::web_search_detail;
use codex_otel::RuntimeMetricsSummary;
@@ -1831,7 +1833,7 @@ pub(crate) fn new_mcp_tools_output(
servers.sort_by(|(a, _), (b, _)| a.cmp(b));
for (server, cfg) in servers {
let prefix = format!("mcp__{server}__");
let prefix = qualified_mcp_tool_name_prefix(server);
let mut names: Vec<String> = tools
.keys()
.filter(|k| k.starts_with(&prefix))
@@ -3217,6 +3219,63 @@ mod tests {
insta::assert_snapshot!(rendered);
}
#[tokio::test]
async fn mcp_tools_output_lists_tools_for_hyphenated_server_names() {
let mut config = test_config().await;
let mut servers = config.mcp_servers.get().clone();
servers.insert(
"some-server".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: "docs-server".to_string(),
args: vec!["--stdio".to_string()],
env: None,
env_vars: vec![],
cwd: None,
},
enabled: true,
required: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
},
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
let tools = HashMap::from([(
"mcp__some_server__lookup".to_string(),
Tool {
description: None,
name: "lookup".to_string(),
title: None,
input_schema: serde_json::json!({"type": "object", "properties": {}}),
output_schema: None,
annotations: None,
icons: None,
meta: None,
},
)]);
let auth_statuses: HashMap<String, McpAuthStatus> = HashMap::new();
let cell = new_mcp_tools_output(
&config,
tools,
HashMap::new(),
HashMap::new(),
&auth_statuses,
);
let rendered = render_lines(&cell.display_lines(120)).join("\n");
insta::assert_snapshot!(rendered);
}
#[tokio::test]
async fn mcp_tools_output_from_statuses_renders_status_only_servers() {
let mut config = test_config().await;