Fix MCP tool listing for hyphenated server names (#16674)

Addresses #16671 and #14927

Problem: `mcpServerStatus/list` rebuilt MCP tool groups from sanitized
tool prefixes but looked them up by unsanitized server names, so
hyphenated servers rendered as having no tools in `/mcp`. This was
reported as a regression when the TUI switched to use the app server.

Solution: Build each server's tool map using the original server name's
sanitized prefix, include effective runtime MCP servers in the status
response, and add a regression test for hyphenated server names.
This commit is contained in:
Eric Traut
2026-04-03 09:05:50 -07:00
committed by GitHub
parent cc8fd0ff65
commit a3b3e7a6cc
4 changed files with 275 additions and 2 deletions

View File

@@ -248,7 +248,8 @@ use codex_login::run_login_server;
use codex_mcp::mcp::auth::discover_supported_scopes;
use codex_mcp::mcp::auth::resolve_oauth_scopes;
use codex_mcp::mcp::collect_mcp_snapshot;
use codex_mcp::mcp::group_tools_by_server;
use codex_mcp::mcp::effective_mcp_servers;
use codex_mcp::mcp::qualified_mcp_tool_name_prefix;
use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig;
use codex_protocol::ThreadId;
use codex_protocol::config_types::CollaborationMode;
@@ -5134,12 +5135,56 @@ impl CodexMessageProcessor {
)
.await;
let tools_by_server = group_tools_by_server(&snapshot.tools);
// Rebuild the tool list per original server name instead of using
// `group_tools_by_server()`: qualified tool names are sanitized for the
// Responses API, so a config key like `some-server` is encoded as the
// `mcp__some_server__` prefix. Matching with the original server name's
// sanitized prefix preserves `/mcp` output for hyphenated names.
let effective_servers = effective_mcp_servers(&mcp_config, auth.as_ref());
let mut sanitized_prefix_counts = HashMap::<String, usize>::new();
for name in effective_servers.keys() {
let prefix = qualified_mcp_tool_name_prefix(name);
*sanitized_prefix_counts.entry(prefix).or_default() += 1;
}
let tools_by_server = effective_servers
.keys()
.map(|name| {
let prefix = qualified_mcp_tool_name_prefix(name);
// If multiple server names normalize to the same prefix, the
// qualified tool namespace is ambiguous (for example
// `some-server` and `some_server` both become
// `mcp__some_server__`). In that case, avoid attributing the
// same tools to multiple servers.
let tools = if sanitized_prefix_counts
.get(&prefix)
.copied()
.unwrap_or_default()
== 1
{
snapshot
.tools
.iter()
.filter_map(|(qualified_name, tool)| {
qualified_name
.strip_prefix(&prefix)
.map(|tool_name| (tool_name.to_string(), tool.clone()))
})
.collect::<HashMap<_, _>>()
} else {
HashMap::new()
};
(name.clone(), tools)
})
.collect::<HashMap<_, _>>();
let mut server_names: Vec<String> = config
.mcp_servers
.keys()
.cloned()
// Include built-in/plugin MCP servers that are present in the
// effective runtime config even when they are not user-declared in
// `config.mcp_servers`.
.chain(effective_servers.keys().cloned())
.chain(snapshot.auth_statuses.keys().cloned())
.chain(snapshot.resources.keys().cloned())
.chain(snapshot.resource_templates.keys().cloned())

View File

@@ -45,6 +45,7 @@ use codex_app_server_protocol::JSONRPCMessage;
use codex_app_server_protocol::JSONRPCNotification;
use codex_app_server_protocol::JSONRPCRequest;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::ListMcpServerStatusParams;
use codex_app_server_protocol::LoginAccountParams;
use codex_app_server_protocol::MockExperimentalMethodParams;
use codex_app_server_protocol::ModelListParams;
@@ -526,6 +527,15 @@ impl McpProcess {
self.send_request("plugin/read", params).await
}
/// Send an `mcpServerStatus/list` JSON-RPC request.
pub async fn send_list_mcp_server_status_request(
&mut self,
params: ListMcpServerStatusParams,
) -> anyhow::Result<i64> {
let params = Some(serde_json::to_value(params)?);
self.send_request("mcpServerStatus/list", params).await
}
/// Send a JSON-RPC request with raw params for protocol-level validation tests.
pub async fn send_raw_request(
&mut self,

View File

@@ -0,0 +1,217 @@
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::sync::Arc;
use std::time::Duration;
use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::to_response;
use app_test_support::write_mock_responses_config_toml;
use axum::Router;
use codex_app_server_protocol::ListMcpServerStatusParams;
use codex_app_server_protocol::ListMcpServerStatusResponse;
use codex_app_server_protocol::RequestId;
use pretty_assertions::assert_eq;
use rmcp::handler::server::ServerHandler;
use rmcp::model::JsonObject;
use rmcp::model::ListToolsResult;
use rmcp::model::ServerCapabilities;
use rmcp::model::ServerInfo;
use rmcp::model::Tool;
use rmcp::model::ToolAnnotations;
use rmcp::transport::StreamableHttpServerConfig;
use rmcp::transport::StreamableHttpService;
use rmcp::transport::streamable_http_server::session::local::LocalSessionManager;
use serde_json::json;
use tempfile::TempDir;
use tokio::net::TcpListener;
use tokio::task::JoinHandle;
use tokio::time::timeout;
const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10);
#[tokio::test]
async fn mcp_server_status_list_returns_tools_for_hyphenated_server_names() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let (mcp_server_url, mcp_server_handle) = start_mcp_server("lookup").await?;
let codex_home = TempDir::new()?;
write_mock_responses_config_toml(
codex_home.path(),
&server.uri(),
&BTreeMap::new(),
/*auto_compact_limit*/ 1024,
/*requires_openai_auth*/ None,
"mock_provider",
"compact",
)?;
let config_path = codex_home.path().join("config.toml");
let mut config_toml = std::fs::read_to_string(&config_path)?;
config_toml.push_str(&format!(
r#"
[mcp_servers.some-server]
url = "{mcp_server_url}/mcp"
"#
));
std::fs::write(config_path, config_toml)?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_list_mcp_server_status_request(ListMcpServerStatusParams {
cursor: None,
limit: None,
})
.await?;
let response = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let response: ListMcpServerStatusResponse = to_response(response)?;
assert_eq!(response.next_cursor, None);
assert_eq!(response.data.len(), 1);
let status = &response.data[0];
assert_eq!(status.name, "some-server");
assert_eq!(
status.tools.keys().cloned().collect::<BTreeSet<_>>(),
BTreeSet::from(["lookup".to_string()])
);
mcp_server_handle.abort();
let _ = mcp_server_handle.await;
Ok(())
}
#[derive(Clone)]
struct McpStatusServer {
tool_name: Arc<String>,
}
impl ServerHandler for McpStatusServer {
fn get_info(&self) -> ServerInfo {
ServerInfo {
capabilities: ServerCapabilities::builder().enable_tools().build(),
..ServerInfo::default()
}
}
async fn list_tools(
&self,
_request: Option<rmcp::model::PaginatedRequestParams>,
_context: rmcp::service::RequestContext<rmcp::service::RoleServer>,
) -> Result<ListToolsResult, rmcp::ErrorData> {
let input_schema: JsonObject = serde_json::from_value(json!({
"type": "object",
"additionalProperties": false
}))
.map_err(|err| rmcp::ErrorData::internal_error(err.to_string(), None))?;
let mut tool = Tool::new(
Cow::Owned(self.tool_name.as_ref().clone()),
Cow::Borrowed("Look up test data."),
Arc::new(input_schema),
);
tool.annotations = Some(ToolAnnotations::new().read_only(true));
Ok(ListToolsResult {
tools: vec![tool],
next_cursor: None,
meta: None,
})
}
}
#[tokio::test]
async fn mcp_server_status_list_does_not_duplicate_tools_for_sanitized_name_collisions()
-> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let (dash_server_url, dash_server_handle) = start_mcp_server("dash_lookup").await?;
let (underscore_server_url, underscore_server_handle) =
start_mcp_server("underscore_lookup").await?;
let codex_home = TempDir::new()?;
write_mock_responses_config_toml(
codex_home.path(),
&server.uri(),
&BTreeMap::new(),
/*auto_compact_limit*/ 1024,
/*requires_openai_auth*/ None,
"mock_provider",
"compact",
)?;
let config_path = codex_home.path().join("config.toml");
let mut config_toml = std::fs::read_to_string(&config_path)?;
config_toml.push_str(&format!(
r#"
[mcp_servers.some-server]
url = "{dash_server_url}/mcp"
[mcp_servers.some_server]
url = "{underscore_server_url}/mcp"
"#
));
std::fs::write(config_path, config_toml)?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_list_mcp_server_status_request(ListMcpServerStatusParams {
cursor: None,
limit: None,
})
.await?;
let response = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let response: ListMcpServerStatusResponse = to_response(response)?;
assert_eq!(response.next_cursor, None);
assert_eq!(response.data.len(), 2);
let status_tools = response
.data
.iter()
.map(|status| (status.name.as_str(), status.tools.keys().count()))
.collect::<BTreeMap<_, _>>();
assert_eq!(
status_tools,
BTreeMap::from([("some-server", 0), ("some_server", 0)])
);
dash_server_handle.abort();
let _ = dash_server_handle.await;
underscore_server_handle.abort();
let _ = underscore_server_handle.await;
Ok(())
}
async fn start_mcp_server(tool_name: &str) -> Result<(String, JoinHandle<()>)> {
let listener = TcpListener::bind("127.0.0.1:0").await?;
let addr = listener.local_addr()?;
let tool_name = Arc::new(tool_name.to_string());
let mcp_service = StreamableHttpService::new(
move || {
Ok(McpStatusServer {
tool_name: Arc::clone(&tool_name),
})
},
Arc::new(LocalSessionManager::default()),
StreamableHttpServerConfig::default(),
);
let router = Router::new().nest_service("/mcp", mcp_service);
let handle = tokio::spawn(async move {
let _ = axum::serve(listener, router).await;
});
Ok((format!("http://{addr}"), handle))
}

View File

@@ -15,6 +15,7 @@ mod experimental_feature_list;
mod fs;
mod initialize;
mod mcp_server_elicitation;
mod mcp_server_status;
mod model_list;
mod output_schema;
mod plan_item;