Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 15 additions & 54 deletions codex-rs/app-server/src/codex_message_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,11 @@ use codex_login::default_client::set_default_client_residency_requirement;
use codex_login::login_with_api_key;
use codex_login::request_device_code;
use codex_login::run_login_server;
use codex_mcp::McpServerStatusSnapshot;
use codex_mcp::McpSnapshotDetail;
use codex_mcp::collect_mcp_snapshot_with_detail;
use codex_mcp::collect_mcp_server_status_snapshot_with_detail;
use codex_mcp::discover_supported_scopes;
use codex_mcp::effective_mcp_servers;
use codex_mcp::qualified_mcp_tool_name_prefix;
use codex_mcp::resolve_oauth_scopes;
use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig;
use codex_protocol::ThreadId;
Expand Down Expand Up @@ -5178,55 +5178,21 @@ impl CodexMessageProcessor {
McpServerStatusDetail::ToolsAndAuthOnly => McpSnapshotDetail::ToolsAndAuthOnly,
};

let snapshot = collect_mcp_snapshot_with_detail(
let snapshot = collect_mcp_server_status_snapshot_with_detail(
&mcp_config,
auth.as_ref(),
request_id.request_id.to_string(),
detail,
)
.await;

// 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 McpServerStatusSnapshot {
tools_by_server,
resources,
resource_templates,
auth_statuses,
} = snapshot;

let mut server_names: Vec<String> = config
.mcp_servers
Expand All @@ -5236,9 +5202,9 @@ impl CodexMessageProcessor {
// 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())
.chain(auth_statuses.keys().cloned())
.chain(resources.keys().cloned())
.chain(resource_templates.keys().cloned())
.collect();
server_names.sort();
server_names.dedup();
Expand Down Expand Up @@ -5279,14 +5245,9 @@ impl CodexMessageProcessor {
.map(|name| McpServerStatus {
name: name.clone(),
tools: tools_by_server.get(name).cloned().unwrap_or_default(),
resources: snapshot.resources.get(name).cloned().unwrap_or_default(),
resource_templates: snapshot
.resource_templates
.get(name)
.cloned()
.unwrap_or_default(),
auth_status: snapshot
.auth_statuses
resources: resources.get(name).cloned().unwrap_or_default(),
resource_templates: resource_templates.get(name).cloned().unwrap_or_default(),
auth_status: auth_statuses
.get(name)
.cloned()
.unwrap_or(CoreMcpAuthStatus::Unsupported)
Expand Down
31 changes: 24 additions & 7 deletions codex-rs/app-server/tests/suite/v2/mcp_server_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ 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<()> {
async fn mcp_server_status_list_returns_raw_server_and_tool_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 (mcp_server_url, mcp_server_handle) = start_mcp_server("look-up.raw").await?;
let codex_home = TempDir::new()?;
write_mock_responses_config_toml(
codex_home.path(),
Expand Down Expand Up @@ -85,7 +85,14 @@ url = "{mcp_server_url}/mcp"
assert_eq!(status.name, "some-server");
assert_eq!(
status.tools.keys().cloned().collect::<BTreeSet<_>>(),
BTreeSet::from(["lookup".to_string()])
BTreeSet::from(["look-up.raw".to_string()])
);
assert_eq!(
status
.tools
.get("look-up.raw")
.map(|tool| tool.name.as_str()),
Some("look-up.raw")
);

mcp_server_handle.abort();
Expand Down Expand Up @@ -261,8 +268,7 @@ url = "{mcp_server_url}/mcp"
}

#[tokio::test]
async fn mcp_server_status_list_does_not_duplicate_tools_for_sanitized_name_collisions()
-> Result<()> {
async fn mcp_server_status_list_keeps_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) =
Expand Down Expand Up @@ -313,11 +319,22 @@ url = "{underscore_server_url}/mcp"
let status_tools = response
.data
.iter()
.map(|status| (status.name.as_str(), status.tools.keys().count()))
.map(|status| {
(
status.name.as_str(),
status.tools.keys().cloned().collect::<BTreeSet<_>>(),
)
})
.collect::<BTreeMap<_, _>>();
assert_eq!(
status_tools,
BTreeMap::from([("some-server", 0), ("some_server", 0)])
BTreeMap::from([
("some-server", BTreeSet::from(["dash_lookup".to_string()])),
(
"some_server",
BTreeSet::from(["underscore_lookup".to_string()])
)
])
);

dash_server_handle.abort();
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/codex-mcp/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub(crate) mod mcp;
pub(crate) mod mcp_connection_manager;
pub(crate) mod mcp_tool_names;

pub use mcp::CODEX_APPS_MCP_SERVER_NAME;
pub use mcp::McpAuthStatusEntry;
Expand All @@ -8,10 +9,13 @@ pub use mcp::McpManager;
pub use mcp::McpOAuthLoginConfig;
pub use mcp::McpOAuthLoginSupport;
pub use mcp::McpOAuthScopesSource;
pub use mcp::McpServerStatusSnapshot;
pub use mcp::McpSnapshotDetail;
pub use mcp::ResolvedMcpOAuthScopes;
pub use mcp::ToolPluginProvenance;
pub use mcp::canonical_mcp_server_key;
pub use mcp::collect_mcp_server_status_snapshot;
pub use mcp::collect_mcp_server_status_snapshot_with_detail;
pub use mcp::collect_mcp_snapshot;
pub use mcp::collect_mcp_snapshot_from_manager;
pub use mcp::collect_mcp_snapshot_from_manager_with_detail;
Expand Down
Loading
Loading