mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Normalize /mcp tool grouping for hyphenated server names (#15946)
Fix display for servers with special characters.
This commit is contained in:
@@ -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))
|
||||
@@ -2544,7 +2545,6 @@ mod tests {
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core::config::types::McpServerConfig;
|
||||
use codex_core::config::types::McpServerTransportConfig;
|
||||
use codex_otel::RuntimeMetricTotals;
|
||||
use codex_otel::RuntimeMetricsSummary;
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -2582,6 +2582,88 @@ mod tests {
|
||||
std::env::temp_dir()
|
||||
}
|
||||
|
||||
fn stdio_server_config(
|
||||
command: &str,
|
||||
args: Vec<&str>,
|
||||
env: Option<HashMap<String, String>>,
|
||||
env_vars: Vec<&str>,
|
||||
) -> McpServerConfig {
|
||||
let mut table = toml::Table::new();
|
||||
table.insert(
|
||||
"command".to_string(),
|
||||
toml::Value::String(command.to_string()),
|
||||
);
|
||||
if !args.is_empty() {
|
||||
table.insert(
|
||||
"args".to_string(),
|
||||
toml::Value::Array(
|
||||
args.into_iter()
|
||||
.map(|arg| toml::Value::String(arg.to_string()))
|
||||
.collect(),
|
||||
),
|
||||
);
|
||||
}
|
||||
if let Some(env) = env {
|
||||
table.insert("env".to_string(), string_map_to_toml_value(env));
|
||||
}
|
||||
if !env_vars.is_empty() {
|
||||
table.insert(
|
||||
"env_vars".to_string(),
|
||||
toml::Value::Array(
|
||||
env_vars
|
||||
.into_iter()
|
||||
.map(|name| toml::Value::String(name.to_string()))
|
||||
.collect(),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
toml::Value::Table(table)
|
||||
.try_into()
|
||||
.expect("test stdio MCP config should deserialize")
|
||||
}
|
||||
|
||||
fn streamable_http_server_config(
|
||||
url: &str,
|
||||
bearer_token_env_var: Option<&str>,
|
||||
http_headers: Option<HashMap<String, String>>,
|
||||
env_http_headers: Option<HashMap<String, String>>,
|
||||
) -> McpServerConfig {
|
||||
let mut table = toml::Table::new();
|
||||
table.insert("url".to_string(), toml::Value::String(url.to_string()));
|
||||
if let Some(bearer_token_env_var) = bearer_token_env_var {
|
||||
table.insert(
|
||||
"bearer_token_env_var".to_string(),
|
||||
toml::Value::String(bearer_token_env_var.to_string()),
|
||||
);
|
||||
}
|
||||
if let Some(http_headers) = http_headers {
|
||||
table.insert(
|
||||
"http_headers".to_string(),
|
||||
string_map_to_toml_value(http_headers),
|
||||
);
|
||||
}
|
||||
if let Some(env_http_headers) = env_http_headers {
|
||||
table.insert(
|
||||
"env_http_headers".to_string(),
|
||||
string_map_to_toml_value(env_http_headers),
|
||||
);
|
||||
}
|
||||
|
||||
toml::Value::Table(table)
|
||||
.try_into()
|
||||
.expect("test streamable_http MCP config should deserialize")
|
||||
}
|
||||
|
||||
fn string_map_to_toml_value(entries: HashMap<String, String>) -> toml::Value {
|
||||
toml::Value::Table(
|
||||
entries
|
||||
.into_iter()
|
||||
.map(|(key, value)| (key, toml::Value::String(value)))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
fn render_lines(lines: &[Line<'static>]) -> Vec<String> {
|
||||
lines
|
||||
.iter()
|
||||
@@ -2897,25 +2979,7 @@ mod tests {
|
||||
let mut config = test_config().await;
|
||||
let mut env = HashMap::new();
|
||||
env.insert("TOKEN".to_string(), "secret".to_string());
|
||||
let stdio_config = McpServerConfig {
|
||||
transport: McpServerTransportConfig::Stdio {
|
||||
command: "docs-server".to_string(),
|
||||
args: vec![],
|
||||
env: Some(env),
|
||||
env_vars: vec!["APP_TOKEN".to_string()],
|
||||
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,
|
||||
tools: HashMap::new(),
|
||||
};
|
||||
let stdio_config = stdio_server_config("docs-server", vec![], Some(env), vec!["APP_TOKEN"]);
|
||||
let mut servers = config.mcp_servers.get().clone();
|
||||
servers.insert("docs".to_string(), stdio_config);
|
||||
|
||||
@@ -2923,24 +2987,12 @@ mod tests {
|
||||
headers.insert("Authorization".to_string(), "Bearer secret".to_string());
|
||||
let mut env_headers = HashMap::new();
|
||||
env_headers.insert("X-API-Key".to_string(), "API_KEY_ENV".to_string());
|
||||
let http_config = McpServerConfig {
|
||||
transport: McpServerTransportConfig::StreamableHttp {
|
||||
url: "https://example.com/mcp".to_string(),
|
||||
bearer_token_env_var: Some("MCP_TOKEN".to_string()),
|
||||
http_headers: Some(headers),
|
||||
env_http_headers: Some(env_headers),
|
||||
},
|
||||
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,
|
||||
tools: HashMap::new(),
|
||||
};
|
||||
let http_config = streamable_http_server_config(
|
||||
"https://example.com/mcp",
|
||||
Some("MCP_TOKEN"),
|
||||
Some(headers),
|
||||
Some(env_headers),
|
||||
);
|
||||
servers.insert("http".to_string(), http_config);
|
||||
config
|
||||
.mcp_servers
|
||||
@@ -2988,6 +3040,46 @@ 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(),
|
||||
stdio_server_config("docs-server", vec!["--stdio"], None, vec![]),
|
||||
);
|
||||
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);
|
||||
|
||||
+16
@@ -0,0 +1,16 @@
|
||||
---
|
||||
source: tui/src/history_cell.rs
|
||||
assertion_line: 3080
|
||||
expression: rendered
|
||||
---
|
||||
/mcp
|
||||
|
||||
🔌 MCP Tools
|
||||
|
||||
• some-server
|
||||
• Status: enabled
|
||||
• Auth: Unsupported
|
||||
• Command: docs-server --stdio
|
||||
• Tools: lookup
|
||||
• Resources: (none)
|
||||
• Resource templates: (none)
|
||||
Reference in New Issue
Block a user