[codex] Prune unused codex-mcp API and duplicate helpers (#19524)

## Why

`codex-mcp` currently exposes more API than the rest of the workspace
uses. Some of that surface is simply visibility that can be tightened,
and some of it is public helper code that remains compiler-valid because
it is exported even though no workspace caller uses it.

That distinction matters: Rust does not warn on exported API just
because the current workspace does not call it. This PR intentionally
treats those exported-but-workspace-unreferenced paths as stale
`codex-mcp` surface. The main example is MCP skill dependency
collection, where the active implementation now lives in
`codex-rs/core/src/mcp_skill_dependencies.rs`; keeping the older
`codex-mcp` copy makes it unclear which implementation owns skill MCP
installation.

## What Changed

- Pruned unused `codex-mcp` re-exports from `codex-mcp/src/lib.rs`.
- Removed non-runtime helper methods from `McpConnectionManager` so it
stays focused on live MCP clients.
- Made `ToolPluginProvenance` lookup methods crate-private.
- Removed workspace-unreferenced snapshot wrapper APIs and
qualified-tool grouping helpers.
- Deleted the duplicate `codex-mcp` skill dependency module and tests
now that skill MCP dependency handling is owned by `core`.

## Verification

- `cargo check -p codex-mcp`

codex-rs/codex-mcp/Cargo.toml

@@ -38,7 +38,6 @@ tracing = { workspace = true }
 url = { workspace = true }
 
 [dev-dependencies]
-codex-utils-absolute-path = { workspace = true }
 pretty_assertions = { workspace = true }
 rmcp = { workspace = true, default-features = false, features = ["base64", "macros", "schemars", "server"] }
 tempfile = { workspace = true }

codex-rs/codex-mcp/src/lib.rs

@@ -5,7 +5,6 @@ pub(crate) mod mcp_tool_names;
 pub use mcp::CODEX_APPS_MCP_SERVER_NAME;
 pub use mcp::McpAuthStatusEntry;
 pub use mcp::McpConfig;
-pub use mcp::McpManager;
 pub use mcp::McpOAuthLoginConfig;
 pub use mcp::McpOAuthLoginSupport;
 pub use mcp::McpOAuthScopesSource;
@@ -13,30 +12,21 @@ 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;
-pub use mcp::collect_mcp_snapshot_with_detail;
-pub use mcp::collect_missing_mcp_dependencies;
 pub use mcp::compute_auth_statuses;
 pub use mcp::configured_mcp_servers;
 pub use mcp::discover_supported_scopes;
 pub use mcp::effective_mcp_servers;
-pub use mcp::group_tools_by_server;
 pub use mcp::mcp_permission_prompt_is_auto_approved;
 pub use mcp::oauth_login_support;
 pub use mcp::qualified_mcp_tool_name_prefix;
 pub use mcp::read_mcp_resource;
 pub use mcp::resolve_oauth_scopes;
 pub use mcp::should_retry_without_scopes;
-pub use mcp::split_qualified_tool_name;
 pub use mcp::tool_plugin_provenance;
 pub use mcp::with_codex_apps_mcp;
 pub use mcp_connection_manager::CodexAppsToolsCacheKey;
-pub use mcp_connection_manager::DEFAULT_STARTUP_TIMEOUT;
 pub use mcp_connection_manager::MCP_SANDBOX_STATE_META_CAPABILITY;
 pub use mcp_connection_manager::McpConnectionManager;
 pub use mcp_connection_manager::McpRuntimeEnvironment;

codex-rs/codex-mcp/src/mcp/mod.rs

@@ -1,5 +1,4 @@
 pub(crate) mod auth;
-mod skill_dependencies;
 pub use auth::McpAuthStatusEntry;
 pub use auth::McpOAuthLoginConfig;
 pub use auth::McpOAuthLoginSupport;
@@ -10,8 +9,6 @@ pub use auth::discover_supported_scopes;
 pub use auth::oauth_login_support;
 pub use auth::resolve_oauth_scopes;
 pub use auth::should_retry_without_scopes;
-pub use skill_dependencies::canonical_mcp_server_key;
-pub use skill_dependencies::collect_missing_mcp_dependencies;
 
 use std::collections::HashMap;
 use std::env;
@@ -39,7 +36,6 @@ use serde_json::Value;
 use crate::mcp_connection_manager::McpConnectionManager;
 use crate::mcp_connection_manager::McpRuntimeEnvironment;
 use crate::mcp_connection_manager::codex_apps_tools_cache_key;
-pub type McpManager = McpConnectionManager;
 
 const MCP_TOOL_NAME_PREFIX: &str = "mcp";
 const MCP_TOOL_NAME_DELIMITER: &str = "__";
@@ -105,7 +101,7 @@ pub fn mcp_permission_prompt_is_auto_approved(
 /// approval/sandbox policy, locate OAuth state, and merge plugin-provided MCP
 /// servers. Request-scoped or auth-scoped state should not be stored here;
 /// thread those values explicitly into runtime entry points such as
-/// [`with_codex_apps_mcp`] and [`collect_mcp_snapshot`] so config objects do
+/// [`with_codex_apps_mcp`] and snapshot collection helpers so config objects do
 /// not go stale when auth changes.
 #[derive(Debug, Clone)]
 pub struct McpConfig {
@@ -335,78 +331,6 @@ pub async fn read_mcp_resource(
     result
 }
 
-pub async fn collect_mcp_snapshot(
-    config: &McpConfig,
-    auth: Option<&CodexAuth>,
-    submit_id: String,
-    runtime_environment: McpRuntimeEnvironment,
-) -> McpListToolsResponseEvent {
-    collect_mcp_snapshot_with_detail(
-        config,
-        auth,
-        submit_id,
-        runtime_environment,
-        McpSnapshotDetail::Full,
-    )
-    .await
-}
-
-pub async fn collect_mcp_snapshot_with_detail(
-    config: &McpConfig,
-    auth: Option<&CodexAuth>,
-    submit_id: String,
-    runtime_environment: McpRuntimeEnvironment,
-    detail: McpSnapshotDetail,
-) -> McpListToolsResponseEvent {
-    let mcp_servers = effective_mcp_servers(config, auth);
-    let tool_plugin_provenance = tool_plugin_provenance(config);
-    if mcp_servers.is_empty() {
-        return McpListToolsResponseEvent {
-            tools: HashMap::new(),
-            resources: HashMap::new(),
-            resource_templates: HashMap::new(),
-            auth_statuses: HashMap::new(),
-        };
-    }
-
-    let auth_status_entries = compute_auth_statuses(
-        mcp_servers.iter(),
-        config.mcp_oauth_credentials_store_mode,
-        auth,
-    )
-    .await;
-
-    let (tx_event, rx_event) = unbounded();
-    drop(rx_event);
-
-    let (mcp_connection_manager, cancel_token) = McpConnectionManager::new(
-        &mcp_servers,
-        config.mcp_oauth_credentials_store_mode,
-        auth_status_entries.clone(),
-        &config.approval_policy,
-        submit_id,
-        tx_event,
-        SandboxPolicy::new_read_only_policy(),
-        runtime_environment,
-        config.codex_home.clone(),
-        codex_apps_tools_cache_key(auth),
-        tool_plugin_provenance,
-        auth,
-    )
-    .await;
-
-    let snapshot = collect_mcp_snapshot_from_manager_with_detail(
-        &mcp_connection_manager,
-        auth_status_entries,
-        detail,
-    )
-    .await;
-
-    cancel_token.cancel();
-
-    snapshot
-}
-
 #[derive(Debug, Clone)]
 pub struct McpServerStatusSnapshot {
     pub tools_by_server: HashMap<String, HashMap<String, Tool>>,
@@ -415,22 +339,6 @@ pub struct McpServerStatusSnapshot {
     pub auth_statuses: HashMap<String, McpAuthStatus>,
 }
 
-pub async fn collect_mcp_server_status_snapshot(
-    config: &McpConfig,
-    auth: Option<&CodexAuth>,
-    submit_id: String,
-    runtime_environment: McpRuntimeEnvironment,
-) -> McpServerStatusSnapshot {
-    collect_mcp_server_status_snapshot_with_detail(
-        config,
-        auth,
-        submit_id,
-        runtime_environment,
-        McpSnapshotDetail::Full,
-    )
-    .await
-}
-
 pub async fn collect_mcp_server_status_snapshot_with_detail(
     config: &McpConfig,
     auth: Option<&CodexAuth>,
@@ -487,35 +395,6 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
     snapshot
 }
 
-pub fn split_qualified_tool_name(qualified_name: &str) -> Option<(String, String)> {
-    let mut parts = qualified_name.split(MCP_TOOL_NAME_DELIMITER);
-    let prefix = parts.next()?;
-    if prefix != MCP_TOOL_NAME_PREFIX {
-        return None;
-    }
-    let server_name = parts.next()?;
-    let tool_name: String = parts.collect::<Vec<_>>().join(MCP_TOOL_NAME_DELIMITER);
-    if tool_name.is_empty() {
-        return None;
-    }
-    Some((server_name.to_string(), tool_name))
-}
-
-pub fn group_tools_by_server(
-    tools: &HashMap<String, Tool>,
-) -> HashMap<String, HashMap<String, Tool>> {
-    let mut grouped = HashMap::new();
-    for (qualified_name, tool) in tools {
-        if let Some((server_name, tool_name)) = split_qualified_tool_name(qualified_name) {
-            grouped
-                .entry(server_name)
-                .or_insert_with(HashMap::new)
-                .insert(tool_name, tool.clone());
-        }
-    }
-    grouped
-}
-
 fn protocol_tool_from_rmcp_tool(name: &str, tool: &rmcp::model::Tool) -> Option<Tool> {
     match serde_json::to_value(tool) {
         Ok(value) => match Tool::from_mcp_value(value) {
@@ -676,7 +555,7 @@ pub async fn collect_mcp_snapshot_from_manager(
     .await
 }
 
-pub async fn collect_mcp_snapshot_from_manager_with_detail(
+async fn collect_mcp_snapshot_from_manager_with_detail(
     mcp_connection_manager: &McpConnectionManager,
     auth_status_entries: HashMap<String, McpAuthStatusEntry>,
     detail: McpSnapshotDetail,

codex-rs/codex-mcp/src/mcp/mod_tests.rs

@@ -25,27 +25,6 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
     }
 }
 
-fn make_tool(name: &str) -> Tool {
-    Tool {
-        name: name.to_string(),
-        title: None,
-        description: None,
-        input_schema: serde_json::json!({"type": "object", "properties": {}}),
-        output_schema: None,
-        annotations: None,
-        icons: None,
-        meta: None,
-    }
-}
-
-#[test]
-fn split_qualified_tool_name_returns_server_and_tool() {
-    assert_eq!(
-        split_qualified_tool_name("mcp__alpha__do_thing"),
-        Some(("alpha".to_string(), "do_thing".to_string()))
-    );
-}
-
 #[test]
 fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() {
     assert_eq!(
@@ -54,36 +33,6 @@ fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() {
     );
 }
 
-#[test]
-fn split_qualified_tool_name_rejects_invalid_names() {
-    assert_eq!(split_qualified_tool_name("other__alpha__do_thing"), None);
-    assert_eq!(split_qualified_tool_name("mcp__alpha__"), None);
-}
-
-#[test]
-fn group_tools_by_server_strips_prefix_and_groups() {
-    let mut tools = HashMap::new();
-    tools.insert("mcp__alpha__do_thing".to_string(), make_tool("do_thing"));
-    tools.insert(
-        "mcp__alpha__nested__op".to_string(),
-        make_tool("nested__op"),
-    );
-    tools.insert("mcp__beta__do_other".to_string(), make_tool("do_other"));
-
-    let mut expected_alpha = HashMap::new();
-    expected_alpha.insert("do_thing".to_string(), make_tool("do_thing"));
-    expected_alpha.insert("nested__op".to_string(), make_tool("nested__op"));
-
-    let mut expected_beta = HashMap::new();
-    expected_beta.insert("do_other".to_string(), make_tool("do_other"));
-
-    let mut expected = HashMap::new();
-    expected.insert("alpha".to_string(), expected_alpha);
-    expected.insert("beta".to_string(), expected_beta);
-
-    assert_eq!(group_tools_by_server(&tools), expected);
-}
-
 #[test]
 fn tool_plugin_provenance_collects_app_and_mcp_sources() {
     let provenance = ToolPluginProvenance::from_capability_summaries(&[

codex-rs/codex-mcp/src/mcp/skill_dependencies.rs

@@ -1,172 +0,0 @@
-use std::collections::HashMap;
-use std::collections::HashSet;
-
-use codex_config::McpServerConfig;
-use codex_config::McpServerTransportConfig;
-use codex_protocol::protocol::SkillMetadata;
-use codex_protocol::protocol::SkillToolDependency;
-use tracing::warn;
-
-pub fn collect_missing_mcp_dependencies(
-    mentioned_skills: &[SkillMetadata],
-    installed: &HashMap<String, McpServerConfig>,
-) -> HashMap<String, McpServerConfig> {
-    let mut missing = HashMap::new();
-    let installed_keys: HashSet<String> = installed
-        .iter()
-        .map(|(name, config)| canonical_mcp_server_key(name, config))
-        .collect();
-    let mut seen_canonical_keys = HashSet::new();
-
-    for skill in mentioned_skills {
-        let Some(dependencies) = skill.dependencies.as_ref() else {
-            continue;
-        };
-
-        for tool in &dependencies.tools {
-            if !tool.r#type.eq_ignore_ascii_case("mcp") {
-                continue;
-            }
-            let dependency_key = match canonical_mcp_dependency_key(tool) {
-                Ok(key) => key,
-                Err(err) => {
-                    let dependency = tool.value.as_str();
-                    let skill_name = skill.name.as_str();
-                    warn!(
-                        "unable to auto-install MCP dependency {dependency} for skill {skill_name}: {err}",
-                    );
-                    continue;
-                }
-            };
-            if installed_keys.contains(&dependency_key)
-                || seen_canonical_keys.contains(&dependency_key)
-            {
-                continue;
-            }
-
-            let config = match mcp_dependency_to_server_config(tool) {
-                Ok(config) => config,
-                Err(err) => {
-                    let dependency = dependency_key.as_str();
-                    let skill_name = skill.name.as_str();
-                    warn!(
-                        "unable to auto-install MCP dependency {dependency} for skill {skill_name}: {err}",
-                    );
-                    continue;
-                }
-            };
-
-            missing.insert(tool.value.clone(), config);
-            seen_canonical_keys.insert(dependency_key);
-        }
-    }
-
-    missing
-}
-
-fn canonical_mcp_key(transport: &str, identifier: &str, fallback: &str) -> String {
-    let identifier = identifier.trim();
-    if identifier.is_empty() {
-        fallback.to_string()
-    } else {
-        format!("mcp__{transport}__{identifier}")
-    }
-}
-
-pub fn canonical_mcp_server_key(name: &str, config: &McpServerConfig) -> String {
-    match &config.transport {
-        McpServerTransportConfig::Stdio { command, .. } => {
-            canonical_mcp_key("stdio", command, name)
-        }
-        McpServerTransportConfig::StreamableHttp { url, .. } => {
-            canonical_mcp_key("streamable_http", url, name)
-        }
-    }
-}
-
-fn canonical_mcp_dependency_key(dependency: &SkillToolDependency) -> Result<String, String> {
-    let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
-    if transport.eq_ignore_ascii_case("streamable_http") {
-        let url = dependency
-            .url
-            .as_ref()
-            .ok_or_else(|| "missing url for streamable_http dependency".to_string())?;
-        return Ok(canonical_mcp_key("streamable_http", url, &dependency.value));
-    }
-    if transport.eq_ignore_ascii_case("stdio") {
-        let command = dependency
-            .command
-            .as_ref()
-            .ok_or_else(|| "missing command for stdio dependency".to_string())?;
-        return Ok(canonical_mcp_key("stdio", command, &dependency.value));
-    }
-    Err(format!("unsupported transport {transport}"))
-}
-
-fn mcp_dependency_to_server_config(
-    dependency: &SkillToolDependency,
-) -> Result<McpServerConfig, String> {
-    let transport = dependency.transport.as_deref().unwrap_or("streamable_http");
-    if transport.eq_ignore_ascii_case("streamable_http") {
-        let url = dependency
-            .url
-            .as_ref()
-            .ok_or_else(|| "missing url for streamable_http dependency".to_string())?;
-        return Ok(McpServerConfig {
-            transport: McpServerTransportConfig::StreamableHttp {
-                url: url.clone(),
-                bearer_token_env_var: None,
-                http_headers: None,
-                env_http_headers: None,
-            },
-            experimental_environment: None,
-            enabled: true,
-            required: false,
-            supports_parallel_tool_calls: false,
-            disabled_reason: None,
-            startup_timeout_sec: None,
-            tool_timeout_sec: None,
-            default_tools_approval_mode: None,
-            enabled_tools: None,
-            disabled_tools: None,
-            scopes: None,
-            oauth_resource: None,
-            tools: HashMap::new(),
-        });
-    }
-
-    if transport.eq_ignore_ascii_case("stdio") {
-        let command = dependency
-            .command
-            .as_ref()
-            .ok_or_else(|| "missing command for stdio dependency".to_string())?;
-        return Ok(McpServerConfig {
-            transport: McpServerTransportConfig::Stdio {
-                command: command.clone(),
-                args: Vec::new(),
-                env: None,
-                env_vars: Vec::new(),
-                cwd: None,
-            },
-            experimental_environment: None,
-            enabled: true,
-            required: false,
-            supports_parallel_tool_calls: false,
-            disabled_reason: None,
-            startup_timeout_sec: None,
-            tool_timeout_sec: None,
-            default_tools_approval_mode: None,
-            enabled_tools: None,
-            disabled_tools: None,
-            scopes: None,
-            oauth_resource: None,
-            tools: HashMap::new(),
-        });
-    }
-
-    Err(format!("unsupported transport {transport}"))
-}
-
-#[cfg(test)]
-#[path = "skill_dependencies_tests.rs"]
-mod tests;

codex-rs/codex-mcp/src/mcp/skill_dependencies_tests.rs

@@ -1,115 +0,0 @@
-use super::*;
-use codex_protocol::protocol::SkillDependencies;
-use codex_protocol::protocol::SkillMetadata;
-use codex_protocol::protocol::SkillScope;
-use codex_utils_absolute_path::test_support::PathBufExt as _;
-use codex_utils_absolute_path::test_support::test_path_buf;
-use pretty_assertions::assert_eq;
-
-fn skill_with_tools(tools: Vec<SkillToolDependency>) -> SkillMetadata {
-    SkillMetadata {
-        name: "skill".to_string(),
-        description: "skill".to_string(),
-        short_description: None,
-        interface: None,
-        dependencies: Some(SkillDependencies { tools }),
-        path: test_path_buf("/tmp/skill").abs(),
-        scope: SkillScope::User,
-        enabled: true,
-    }
-}
-
-#[test]
-fn collect_missing_respects_canonical_installed_key() {
-    let url = "https://example.com/mcp".to_string();
-    let skills = vec![skill_with_tools(vec![SkillToolDependency {
-        r#type: "mcp".to_string(),
-        value: "github".to_string(),
-        description: None,
-        transport: Some("streamable_http".to_string()),
-        command: None,
-        url: Some(url.clone()),
-    }])];
-    let installed = HashMap::from([(
-        "alias".to_string(),
-        McpServerConfig {
-            transport: McpServerTransportConfig::StreamableHttp {
-                url,
-                bearer_token_env_var: None,
-                http_headers: None,
-                env_http_headers: None,
-            },
-            experimental_environment: None,
-            enabled: true,
-            required: false,
-            supports_parallel_tool_calls: false,
-            disabled_reason: None,
-            startup_timeout_sec: None,
-            tool_timeout_sec: None,
-            default_tools_approval_mode: None,
-            enabled_tools: None,
-            disabled_tools: None,
-            scopes: None,
-            oauth_resource: None,
-            tools: HashMap::new(),
-        },
-    )]);
-
-    assert_eq!(
-        collect_missing_mcp_dependencies(&skills, &installed),
-        HashMap::new()
-    );
-}
-
-#[test]
-fn collect_missing_dedupes_by_canonical_key_but_preserves_original_name() {
-    let url = "https://example.com/one".to_string();
-    let skills = vec![skill_with_tools(vec![
-        SkillToolDependency {
-            r#type: "mcp".to_string(),
-            value: "alias-one".to_string(),
-            description: None,
-            transport: Some("streamable_http".to_string()),
-            command: None,
-            url: Some(url.clone()),
-        },
-        SkillToolDependency {
-            r#type: "mcp".to_string(),
-            value: "alias-two".to_string(),
-            description: None,
-            transport: Some("streamable_http".to_string()),
-            command: None,
-            url: Some(url.clone()),
-        },
-    ])];
-
-    let expected = HashMap::from([(
-        "alias-one".to_string(),
-        McpServerConfig {
-            transport: McpServerTransportConfig::StreamableHttp {
-                url,
-                bearer_token_env_var: None,
-                http_headers: None,
-                env_http_headers: None,
-            },
-            experimental_environment: None,
-            enabled: true,
-            required: false,
-            supports_parallel_tool_calls: false,
-            disabled_reason: None,
-            startup_timeout_sec: None,
-            tool_timeout_sec: None,
-            default_tools_approval_mode: None,
-            enabled_tools: None,
-            disabled_tools: None,
-            scopes: None,
-            oauth_resource: None,
-            tools: HashMap::new(),
-        },
-    )]);
-
-    assert_eq!(
-        collect_missing_mcp_dependencies(&skills, &HashMap::new()),
-        expected
-    );
-}

codex-rs/codex-mcp/src/mcp_connection_manager.rs

@@ -21,12 +21,8 @@ use std::time::Instant;
 
 use crate::McpAuthStatusEntry;
 use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
-use crate::mcp::McpConfig;
 use crate::mcp::ToolPluginProvenance;
-use crate::mcp::configured_mcp_servers;
-use crate::mcp::effective_mcp_servers;
 use crate::mcp::mcp_permission_prompt_is_auto_approved;
-use crate::mcp::tool_plugin_provenance;
 pub(crate) use crate::mcp_tool_names::qualify_tools;
 use anyhow::Context;
 use anyhow::Result;
@@ -105,7 +101,7 @@ use codex_utils_plugins::mcp_connector::sanitize_name;
 const MCP_TOOL_NAME_DELIMITER: &str = "__";
 
 /// Default timeout for initializing MCP server & initially listing tools.
-pub const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(30);
+const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(30);
 
 /// Default timeout for individual tool calls.
 const DEFAULT_TOOL_TIMEOUT: Duration = Duration::from_secs(120);
@@ -689,22 +685,6 @@ impl McpRuntimeEnvironment {
 }
 
 impl McpConnectionManager {
-    pub fn configured_servers(&self, config: &McpConfig) -> HashMap<String, McpServerConfig> {
-        configured_mcp_servers(config)
-    }
-
-    pub fn effective_servers(
-        &self,
-        config: &McpConfig,
-        auth: Option<&CodexAuth>,
-    ) -> HashMap<String, McpServerConfig> {
-        effective_mcp_servers(config, auth)
-    }
-
-    pub fn tool_plugin_provenance(&self, config: &McpConfig) -> ToolPluginProvenance {
-        tool_plugin_provenance(config)
-    }
-
     pub fn new_uninitialized(
         approval_policy: &Constrained<AskForApproval>,
         sandbox_policy: &Constrained<SandboxPolicy>,