From 4add004df0c34ed317af9f89ede46b6ac67928e3 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 22 Jun 2026 18:06:32 -0700 Subject: [PATCH] Move Codex Apps identity to MCP clients --- codex-rs/codex-mcp/src/codex_apps.rs | 29 ++----- codex-rs/codex-mcp/src/connection_manager.rs | 45 ++++------ .../codex-mcp/src/connection_manager_tests.rs | 54 ++++-------- codex-rs/codex-mcp/src/lib.rs | 1 - codex-rs/codex-mcp/src/mcp/auth.rs | 4 +- codex-rs/codex-mcp/src/mcp/mod.rs | 5 -- codex-rs/codex-mcp/src/rmcp_client.rs | 84 +++++++++++++------ codex-rs/core/src/connectors.rs | 7 +- codex-rs/core/src/mcp_tool_call.rs | 59 ++++++++----- codex-rs/core/src/mcp_tool_call_tests.rs | 61 +++++++++++++- codex-rs/core/src/mcp_tool_exposure.rs | 8 +- codex-rs/core/src/plugins/injection.rs | 4 +- .../core/src/tools/handlers/mcp_resource.rs | 4 +- codex-rs/tui/src/bottom_pane/app_link_view.rs | 8 +- 14 files changed, 214 insertions(+), 159 deletions(-) diff --git a/codex-rs/codex-mcp/src/codex_apps.rs b/codex-rs/codex-mcp/src/codex_apps.rs index 5e6fee8db0..602e4e598b 100644 --- a/codex-rs/codex-mcp/src/codex_apps.rs +++ b/codex-rs/codex-mcp/src/codex_apps.rs @@ -8,7 +8,6 @@ use std::path::PathBuf; use std::time::Instant; -use crate::mcp::is_codex_apps_mcp; use crate::runtime::emit_duration; use crate::tools::MCP_TOOLS_CACHE_WRITE_DURATION_METRIC; use crate::tools::ToolInfo; @@ -67,11 +66,11 @@ pub(crate) enum CachedCodexAppsToolsLoad { } pub(crate) fn normalize_codex_apps_tool_title( - server_name: &str, + is_codex_apps_mcp: bool, connector_name: Option<&str>, value: &str, ) -> String { - if !is_codex_apps_mcp(server_name) { + if !is_codex_apps_mcp { return value.to_string(); } @@ -93,12 +92,12 @@ pub(crate) fn normalize_codex_apps_tool_title( } pub(crate) fn normalize_codex_apps_callable_name( - server_name: &str, + is_codex_apps_mcp: bool, tool_name: &str, connector_id: Option<&str>, connector_name: Option<&str>, ) -> String { - if !is_codex_apps_mcp(server_name) { + if !is_codex_apps_mcp { return tool_name.to_string(); } @@ -128,12 +127,11 @@ pub(crate) fn normalize_codex_apps_callable_name( } pub(crate) fn normalize_codex_apps_callable_namespace( + is_codex_apps_mcp: bool, server_name: &str, connector_name: Option<&str>, ) -> String { - if is_codex_apps_mcp(server_name) - && let Some(connector_name) = connector_name - { + if is_codex_apps_mcp && let Some(connector_name) = connector_name { format!("{}__{}", server_name, sanitize_name(connector_name)) } else { server_name.to_string() @@ -141,15 +139,10 @@ pub(crate) fn normalize_codex_apps_callable_namespace( } pub(crate) fn write_cached_codex_apps_tools_if_needed( - server_name: &str, cache_context: Option<&CodexAppsToolsCacheContext>, server_info: &McpServerInfo, tools: &[ToolInfo], ) { - if !is_codex_apps_mcp(server_name) { - return; - } - if let Some(cache_context) = cache_context { let cache_write_start = Instant::now(); write_cached_codex_apps_tools(cache_context, tools); @@ -165,13 +158,8 @@ pub(crate) fn write_cached_codex_apps_tools_if_needed( } pub(crate) fn load_startup_cached_codex_apps_tools_snapshot( - server_name: &str, cache_context: Option<&CodexAppsToolsCacheContext>, ) -> Option> { - if !is_codex_apps_mcp(server_name) { - return None; - } - let cache_context = cache_context?; match load_cached_codex_apps_tools(cache_context) { @@ -181,13 +169,8 @@ pub(crate) fn load_startup_cached_codex_apps_tools_snapshot( } pub(crate) fn load_startup_cached_codex_apps_server_info( - server_name: &str, cache_context: Option<&CodexAppsToolsCacheContext>, ) -> Option { - if !is_codex_apps_mcp(server_name) { - return None; - } - load_cached_codex_apps_server_info(cache_context?) } diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index c9169c3124..d9ad4ee4e1 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -14,14 +14,12 @@ use std::time::Duration; use std::time::Instant; use crate::McpAuthStatusEntry; -use crate::codex_apps::CodexAppsToolsCacheContext; use crate::codex_apps::CodexAppsToolsCacheKey; use crate::codex_apps::write_cached_codex_apps_tools_if_needed; use crate::elicitation::ElicitationRequestManager; use crate::elicitation::ElicitationReviewerHandle; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::ToolPluginProvenance; -use crate::mcp::is_codex_apps_mcp; use crate::rmcp_client::AsyncManagedClient; use crate::rmcp_client::DEFAULT_STARTUP_TIMEOUT; use crate::rmcp_client::MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC; @@ -172,30 +170,6 @@ impl McpConnectionManager { }, ) .await; - let codex_apps_tools_cache_context = if is_codex_apps_mcp(&server_name) { - Some(CodexAppsToolsCacheContext { - codex_home: codex_home.clone(), - user_key: codex_apps_tools_cache_key.clone(), - }) - } else { - None - }; - let uses_env_bearer_token = - server - .configured_config() - .is_some_and(|config| match &config.transport { - McpServerTransportConfig::StreamableHttp { - bearer_token_env_var, - .. - } => bearer_token_env_var.is_some(), - McpServerTransportConfig::Stdio { .. } => false, - }); - let runtime_auth_provider = if is_codex_apps_mcp(&server_name) && !uses_env_bearer_token - { - codex_apps_auth_provider.clone() - } else { - None - }; let async_managed_client = AsyncManagedClient::new( server_name.clone(), server, @@ -204,10 +178,11 @@ impl McpConnectionManager { cancel_token.clone(), tx_event.clone(), elicitation_requests.clone(), - codex_apps_tools_cache_context, + codex_home.clone(), + codex_apps_tools_cache_key.clone(), Arc::clone(&tool_plugin_provenance), runtime_context.clone(), - runtime_auth_provider, + codex_apps_auth_provider.clone(), client_elicitation_capability.clone(), supports_openai_form_elicitation, ); @@ -354,6 +329,13 @@ impl McpConnectionManager { self.clients.contains_key(server_name) } + /// Returns whether the named registered client is the Codex Apps MCP server. + pub fn is_codex_apps_mcp(&self, server_name: &str) -> bool { + self.clients + .get(server_name) + .is_some_and(AsyncManagedClient::is_codex_apps_mcp) + } + /// Stop all MCP clients owned by this manager and terminate stdio server processes. pub async fn shutdown(&self) { self.startup_cancellation_token.cancel(); @@ -491,10 +473,11 @@ impl McpConnectionManager { /// latest filtered tools are returned directly to the caller. On /// failure, the existing cache remains unchanged. pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result> { - let managed_client = self + let async_managed_client = self .clients .get(CODEX_APPS_MCP_SERVER_NAME) - .ok_or_else(|| anyhow!("unknown MCP server '{CODEX_APPS_MCP_SERVER_NAME}'"))? + .ok_or_else(|| anyhow!("unknown MCP server '{CODEX_APPS_MCP_SERVER_NAME}'"))?; + let managed_client = async_managed_client .client() .await .context("failed to get client")?; @@ -503,6 +486,7 @@ impl McpConnectionManager { let fetch_start = Instant::now(); let tools = list_tools_for_client_uncached( CODEX_APPS_MCP_SERVER_NAME, + async_managed_client.is_codex_apps_mcp(), &managed_client.client, managed_client.tool_timeout, managed_client.server_instructions.as_deref(), @@ -518,7 +502,6 @@ impl McpConnectionManager { ); write_cached_codex_apps_tools_if_needed( - CODEX_APPS_MCP_SERVER_NAME, managed_client.codex_apps_tools_cache_context.as_ref(), &managed_client.server_info, &tools, diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index b2b3b98be0..0038ea41a3 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -704,22 +704,11 @@ fn startup_cached_codex_apps_tools_loads_from_disk_cache() { "calendar_search", )]; let server_info = create_test_server_info("Codex Apps"); - write_cached_codex_apps_tools_if_needed( - CODEX_APPS_MCP_SERVER_NAME, - Some(&cache_context), - &server_info, - &cached_tools, - ); + write_cached_codex_apps_tools_if_needed(Some(&cache_context), &server_info, &cached_tools); - let startup_tools = load_startup_cached_codex_apps_tools_snapshot( - CODEX_APPS_MCP_SERVER_NAME, - Some(&cache_context), - ) - .expect("expected startup snapshot to load from cache"); - let cached_server_info = load_startup_cached_codex_apps_server_info( - CODEX_APPS_MCP_SERVER_NAME, - Some(&cache_context), - ); + let startup_tools = load_startup_cached_codex_apps_tools_snapshot(Some(&cache_context)) + .expect("expected startup snapshot to load from cache"); + let cached_server_info = load_startup_cached_codex_apps_server_info(Some(&cache_context)); assert_eq!(startup_tools.len(), 1); assert_eq!(startup_tools[0].server_name, CODEX_APPS_MCP_SERVER_NAME); @@ -746,15 +735,9 @@ fn startup_cached_codex_apps_tools_loads_without_server_info_cache() { .expect("serialize"); std::fs::write(cache_path, bytes).expect("write"); - let startup_tools = load_startup_cached_codex_apps_tools_snapshot( - CODEX_APPS_MCP_SERVER_NAME, - Some(&cache_context), - ) - .expect("legacy startup snapshot should remain available"); - let cached_server_info = load_startup_cached_codex_apps_server_info( - CODEX_APPS_MCP_SERVER_NAME, - Some(&cache_context), - ); + let startup_tools = load_startup_cached_codex_apps_tools_snapshot(Some(&cache_context)) + .expect("legacy startup snapshot should remain available"); + let cached_server_info = load_startup_cached_codex_apps_server_info(Some(&cache_context)); assert_eq!(startup_tools.len(), 1); assert_eq!(startup_tools[0].callable_name, "calendar_search"); @@ -771,7 +754,6 @@ fn codex_apps_server_info_cache_survives_legacy_tools_cache_write() { ); let server_info = create_test_server_info("Codex Apps"); write_cached_codex_apps_tools_if_needed( - CODEX_APPS_MCP_SERVER_NAME, Some(&cache_context), &server_info, &[create_test_tool( @@ -792,19 +774,10 @@ fn codex_apps_server_info_cache_survives_legacy_tools_cache_write() { std::fs::write(cache_path, bytes).expect("write legacy tools cache"); assert_eq!( - load_startup_cached_codex_apps_server_info( - CODEX_APPS_MCP_SERVER_NAME, - Some(&cache_context), - ), + load_startup_cached_codex_apps_server_info(Some(&cache_context)), Some(server_info) ); - assert!( - load_startup_cached_codex_apps_tools_snapshot( - CODEX_APPS_MCP_SERVER_NAME, - Some(&cache_context), - ) - .is_none() - ); + assert!(load_startup_cached_codex_apps_tools_snapshot(Some(&cache_context)).is_none()); } #[tokio::test] @@ -832,6 +805,7 @@ async fn list_all_tools_uses_cached_tool_info_snapshot_while_client_is_pending() startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), }, ); @@ -869,6 +843,7 @@ async fn list_available_server_infos_uses_cache_while_client_is_pending() { startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), }, ); @@ -906,6 +881,7 @@ async fn list_all_tools_accepts_canonical_namespaced_tool_names() { startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: "rmcp".to_string(), }, ); @@ -949,6 +925,7 @@ async fn list_all_tools_applies_legacy_mcp_prefix_by_default() { startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: "rmcp".to_string(), }, ); @@ -991,6 +968,7 @@ async fn list_all_tools_blocks_while_client_is_pending_without_cached_tool_info_ startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), }, ); @@ -1027,6 +1005,7 @@ async fn shutdown_cancels_pending_tool_listing() { startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token, + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), }, ); let manager = Arc::new(manager); @@ -1062,6 +1041,7 @@ async fn list_all_tools_does_not_block_when_cached_tool_info_snapshot_is_empty() startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), }, ); @@ -1102,6 +1082,7 @@ async fn list_all_tools_uses_cached_tool_info_snapshot_when_client_startup_fails startup_complete, tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), }, ); @@ -1160,6 +1141,7 @@ async fn list_all_tools_adds_server_metadata_to_cached_tools() { startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), cancel_token: CancellationToken::new(), + server_name: server_name.to_string(), }, ); diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 61230c03f8..04698977ba 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -42,7 +42,6 @@ pub use mcp::configured_mcp_servers; pub use mcp::effective_mcp_servers; pub use mcp::effective_mcp_servers_from_configured; pub use mcp::hosted_plugin_runtime_mcp_server_config; -pub use mcp::is_codex_apps_mcp; pub use mcp::tool_plugin_provenance; pub use plugin_config::PluginMcpConfigParseOutcome; pub use plugin_config::PluginMcpServerParseError; diff --git a/codex-rs/codex-mcp/src/mcp/auth.rs b/codex-rs/codex-mcp/src/mcp/auth.rs index 48c27ef732..ec67f07a82 100644 --- a/codex-rs/codex-mcp/src/mcp/auth.rs +++ b/codex-rs/codex-mcp/src/mcp/auth.rs @@ -15,7 +15,7 @@ use tracing::warn; use crate::server::EffectiveMcpServer; -use super::is_codex_apps_mcp; +use super::CODEX_APPS_MCP_SERVER_NAME; #[derive(Debug, Clone)] pub struct McpOAuthLoginConfig { @@ -140,7 +140,7 @@ where let futures = servers.into_iter().map(|(name, server)| { let name = name.clone(); let config = server.configured_config().cloned(); - let has_runtime_auth = is_codex_apps_mcp(&name) + let has_runtime_auth = name == CODEX_APPS_MCP_SERVER_NAME && auth.is_some_and(CodexAuth::uses_codex_backend) && config.as_ref().is_some_and(|config| { matches!( diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index f93a9e150a..616d100896 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -50,11 +50,6 @@ const MCP_TOOL_NAME_PREFIX: &str = "mcp"; const MCP_TOOL_NAME_DELIMITER: &str = "__"; const CODEX_CONNECTORS_TOKEN_ENV_VAR: &str = "CODEX_CONNECTORS_TOKEN"; -/// Returns whether `server_name` identifies the reserved Codex Apps MCP server. -pub fn is_codex_apps_mcp(server_name: &str) -> bool { - server_name == CODEX_APPS_MCP_SERVER_NAME -} - #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum McpSnapshotDetail { #[default] diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index d966f53664..70345a17c9 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -11,6 +11,7 @@ use std::collections::BTreeMap; use std::collections::HashMap; use std::env; use std::ffi::OsString; +use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; @@ -19,6 +20,7 @@ use std::time::Instant; use crate::codex_apps::CachedCodexAppsToolsLoad; use crate::codex_apps::CodexAppsToolsCacheContext; +use crate::codex_apps::CodexAppsToolsCacheKey; use crate::codex_apps::load_cached_codex_apps_tools; use crate::codex_apps::load_startup_cached_codex_apps_server_info; use crate::codex_apps::load_startup_cached_codex_apps_tools_snapshot; @@ -27,8 +29,8 @@ use crate::codex_apps::normalize_codex_apps_callable_namespace; use crate::codex_apps::normalize_codex_apps_tool_title; use crate::codex_apps::write_cached_codex_apps_tools_if_needed; use crate::elicitation::ElicitationRequestManager; +use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::ToolPluginProvenance; -use crate::mcp::is_codex_apps_mcp; use crate::runtime::McpRuntimeContext; use crate::runtime::emit_duration; use crate::server::EffectiveMcpServer; @@ -134,6 +136,7 @@ pub(crate) struct AsyncManagedClient { pub(crate) startup_complete: Arc, pub(crate) tool_plugin_provenance: Arc, pub(crate) cancel_token: CancellationToken, + pub(crate) server_name: String, } impl AsyncManagedClient { @@ -148,31 +151,49 @@ impl AsyncManagedClient { cancel_token: CancellationToken, tx_event: Sender, elicitation_requests: ElicitationRequestManager, - codex_apps_tools_cache_context: Option, + codex_home: PathBuf, + codex_apps_tools_cache_key: CodexAppsToolsCacheKey, tool_plugin_provenance: Arc, runtime_context: McpRuntimeContext, - runtime_auth_provider: Option, + codex_apps_auth_provider: Option, client_elicitation_capability: ElicitationCapability, supports_openai_form_elicitation: bool, ) -> Self { + let is_codex_apps_mcp = server_name == CODEX_APPS_MCP_SERVER_NAME; + let codex_apps_tools_cache_context = + is_codex_apps_mcp.then_some(CodexAppsToolsCacheContext { + codex_home, + user_key: codex_apps_tools_cache_key, + }); + let uses_env_bearer_token = server + .configured_config() + .is_some_and(|config| match &config.transport { + McpServerTransportConfig::StreamableHttp { + bearer_token_env_var, + .. + } => bearer_token_env_var.is_some(), + McpServerTransportConfig::Stdio { .. } => false, + }); + let runtime_auth_provider = if is_codex_apps_mcp && !uses_env_bearer_token { + codex_apps_auth_provider + } else { + None + }; let tool_filter = server .configured_config() .map(ToolFilter::from_config) .unwrap_or_default(); - let cached_tool_info_snapshot = load_startup_cached_codex_apps_tools_snapshot( - &server_name, - codex_apps_tools_cache_context.as_ref(), - ); + let cached_tool_info_snapshot = + load_startup_cached_codex_apps_tools_snapshot(codex_apps_tools_cache_context.as_ref()); let cached_tool_info_snapshot = cached_tool_info_snapshot.map(|tools| filter_tools(tools, &tool_filter)); - let cached_server_info = load_startup_cached_codex_apps_server_info( - &server_name, - codex_apps_tools_cache_context.as_ref(), - ); + let cached_server_info = + load_startup_cached_codex_apps_server_info(codex_apps_tools_cache_context.as_ref()); let startup_tool_filter = tool_filter; let startup_complete = Arc::new(AtomicBool::new(false)); let startup_complete_for_fut = Arc::clone(&startup_complete); let cancel_token_for_fut = cancel_token.clone(); + let client_server_name = server_name.clone(); let fut = async move { let outcome = match async { if let Err(error) = validate_mcp_server_name(&server_name) { @@ -206,6 +227,7 @@ impl AsyncManagedClient { tx_event, elicitation_requests, codex_apps_tools_cache_context, + is_codex_apps_mcp, client_elicitation_capability, supports_openai_form_elicitation, }, @@ -237,9 +259,14 @@ impl AsyncManagedClient { startup_complete, tool_plugin_provenance, cancel_token, + server_name: client_server_name, } } + pub(crate) fn is_codex_apps_mcp(&self) -> bool { + self.server_name == CODEX_APPS_MCP_SERVER_NAME + } + pub(crate) async fn client(&self) -> Result { self.client.clone().await } @@ -266,7 +293,7 @@ impl AsyncManagedClient { let annotate_tools = |tools: Vec| { let mut tools = tools; for tool in &mut tools { - if is_codex_apps_mcp(&tool.server_name) { + if self.is_codex_apps_mcp() { tool.tool = tool_with_model_visible_input_schema(&tool.tool); } @@ -348,6 +375,7 @@ impl From for StartupOutcomeError { pub(crate) async fn list_tools_for_client_uncached( server_name: &str, + is_codex_apps_mcp: bool, client: &Arc, timeout: Option, server_instructions: Option<&str>, @@ -362,23 +390,29 @@ pub(crate) async fn list_tools_for_client_uncached( let mut tool_def = tool.tool; let (connector_id, connector_name, connector_description) = sanitize_tool_connector_metadata( - server_name, + is_codex_apps_mcp, &mut tool_def, tool.connector_id, tool.connector_name, tool.connector_description, ); let callable_name = normalize_codex_apps_callable_name( - server_name, + is_codex_apps_mcp, &tool_def.name, connector_id.as_deref(), connector_name.as_deref(), ); - let callable_namespace = - normalize_codex_apps_callable_namespace(server_name, connector_name.as_deref()); + let callable_namespace = normalize_codex_apps_callable_namespace( + is_codex_apps_mcp, + server_name, + connector_name.as_deref(), + ); if let Some(title) = tool_def.title.as_deref() { - let normalized_title = - normalize_codex_apps_tool_title(server_name, connector_name.as_deref(), title); + let normalized_title = normalize_codex_apps_tool_title( + is_codex_apps_mcp, + connector_name.as_deref(), + title, + ); if tool_def.title.as_deref() != Some(normalized_title.as_str()) { tool_def.title = Some(normalized_title); } @@ -409,13 +443,13 @@ pub(crate) async fn list_tools_for_client_uncached( } fn sanitize_tool_connector_metadata( - server_name: &str, + is_codex_apps_mcp: bool, tool: &mut RmcpTool, connector_id: Option, connector_name: Option, connector_description: Option, ) -> (Option, Option, Option) { - if is_codex_apps_mcp(server_name) { + if is_codex_apps_mcp { return (connector_id, connector_name, connector_description); } @@ -483,6 +517,7 @@ async fn start_server_task( tx_event, elicitation_requests, codex_apps_tools_cache_context, + is_codex_apps_mcp, client_elicitation_capability, supports_openai_form_elicitation, } = params; @@ -508,6 +543,7 @@ async fn start_server_task( let fetch_start = Instant::now(); let tools = list_tools_for_client_uncached( &server_name, + is_codex_apps_mcp, &client, startup_timeout, initialize_result.instructions.as_deref(), @@ -521,12 +557,11 @@ async fn start_server_task( ); let server_info = mcp_server_info_from_implementation(initialize_result.server_info); write_cached_codex_apps_tools_if_needed( - &server_name, codex_apps_tools_cache_context.as_ref(), &server_info, &tools, ); - if is_codex_apps_mcp(&server_name) { + if is_codex_apps_mcp { emit_duration( MCP_TOOLS_LIST_DURATION_METRIC, list_start.elapsed(), @@ -591,6 +626,7 @@ struct StartServerTaskParams { tx_event: Sender, elicitation_requests: ElicitationRequestManager, codex_apps_tools_cache_context: Option, + is_codex_apps_mcp: bool, client_elicitation_capability: ElicitationCapability, supports_openai_form_elicitation: bool, } @@ -738,7 +774,7 @@ mod tests { let (connector_id, connector_name, connector_description) = sanitize_tool_connector_metadata( - "minimaltest", + /*is_codex_apps_mcp*/ false, &mut tool, Some("connector_gmail".to_string()), Some("Gmail".to_string()), @@ -774,7 +810,7 @@ mod tests { let (connector_id, connector_name, connector_description) = sanitize_tool_connector_metadata( - CODEX_APPS_MCP_SERVER_NAME, + /*is_codex_apps_mcp*/ true, &mut tool, Some("connector_gmail".to_string()), Some("Gmail".to_string()), diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 715ed3b865..87c3097162 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -40,7 +40,6 @@ use codex_mcp::ToolPluginProvenance; use codex_mcp::codex_apps_tools_cache_key; use codex_mcp::compute_auth_statuses; use codex_mcp::effective_mcp_servers; -use codex_mcp::is_codex_apps_mcp; use codex_mcp::tool_plugin_provenance; const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30); @@ -239,7 +238,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( } let mut mcp_servers = effective_mcp_servers(&mcp_config, auth.as_ref()); - mcp_servers.retain(|name, _| is_codex_apps_mcp(name)); + mcp_servers.retain(|name, _| name == CODEX_APPS_MCP_SERVER_NAME); if mcp_servers.is_empty() { return Ok(AccessibleConnectorsStatus { connectors: Vec::new(), @@ -477,7 +476,7 @@ fn collect_accessible_connectors_from_mcp_tools<'a>( // ToolInfo already carries plugin provenance, so app-level plugin sources // can be derived here instead of requiring a separate enrichment pass. let tools = mcp_tools.filter_map(|tool| { - if !is_codex_apps_mcp(&tool.server_name) { + if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { return None; } let connector_id = tool.connector_id.as_deref()?; @@ -548,7 +547,7 @@ pub(crate) fn mcp_approvals_reviewer( server_name: &str, connector_id: Option<&str>, ) -> ApprovalsReviewer { - let app_reviewer = if is_codex_apps_mcp(server_name) { + let app_reviewer = if server_name == CODEX_APPS_MCP_SERVER_NAME { apps_config_from_layer_stack(&config.config_layer_stack).and_then(|apps_config| { connector_id .and_then(|connector_id| apps_config.apps.get(connector_id)) diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 93167c097e..98b9c9b2dc 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -45,7 +45,6 @@ use codex_mcp::SandboxState; use codex_mcp::auth_elicitation_completed_result; use codex_mcp::build_auth_elicitation_plan; use codex_mcp::declared_openai_file_input_param_names; -use codex_mcp::is_codex_apps_mcp; use codex_mcp::mcp_permission_prompt_is_auto_approved; use codex_otel::sanitize_metric_tag_value; use codex_protocol::items::McpToolCallError; @@ -144,8 +143,14 @@ pub(crate) async fn handle_mcp_tool_call( let metadata = lookup_mcp_tool_metadata(sess.as_ref(), turn_context.as_ref(), &server, &tool_name).await; - let item_metadata = McpToolCallItemMetadata::from_tool_metadata(&server, metadata.as_ref()); - let app_tool_policy = if is_codex_apps_mcp(&server) { + let is_codex_apps_mcp = sess + .services + .mcp_connection_manager + .load() + .is_codex_apps_mcp(&server); + let item_metadata = + McpToolCallItemMetadata::from_tool_metadata(is_codex_apps_mcp, metadata.as_ref()); + let app_tool_policy = if is_codex_apps_mcp { let annotations = metadata .as_ref() .and_then(|metadata| metadata.annotations.as_ref()); @@ -165,7 +170,7 @@ pub(crate) async fn handle_mcp_tool_call( } else { AppToolPolicy::default() }; - let approval_mode = if is_codex_apps_mcp(&server) { + let approval_mode = if is_codex_apps_mcp { app_tool_policy.approval } else if let Some(approval_mode) = { // Selected-plugin registrations are absent from config.toml and the legacy plugin manager, @@ -181,7 +186,7 @@ pub(crate) async fn handle_mcp_tool_call( .await }; - if is_codex_apps_mcp(&server) && !app_tool_policy.enabled { + if is_codex_apps_mcp && !app_tool_policy.enabled { let result = notify_mcp_tool_call_skip( sess.as_ref(), turn_context.as_ref(), @@ -315,12 +320,11 @@ struct McpToolCallItemMetadata { } impl McpToolCallItemMetadata { - fn from_tool_metadata(server: &str, metadata: Option<&McpToolApprovalMetadata>) -> Self { - let trusted_mcp_app_metadata = if is_codex_apps_mcp(server) { - metadata - } else { - None - }; + fn from_tool_metadata( + is_codex_apps_mcp: bool, + metadata: Option<&McpToolApprovalMetadata>, + ) -> Self { + let trusted_mcp_app_metadata = if is_codex_apps_mcp { metadata } else { None }; Self { connector_id: trusted_mcp_app_metadata .and_then(|metadata| metadata.connector_id.clone()), @@ -632,7 +636,12 @@ async fn maybe_request_codex_apps_auth_elicitation( metadata: Option<&McpToolApprovalMetadata>, result: CallToolResult, ) -> CallToolResult { - if !is_codex_apps_mcp(server) { + if !sess + .services + .mcp_connection_manager + .load() + .is_codex_apps_mcp(server) + { return result; } @@ -966,7 +975,12 @@ async fn maybe_track_codex_app_used( server: &str, tool_name: &str, ) { - if !is_codex_apps_mcp(server) { + if !sess + .services + .mcp_connection_manager + .load() + .is_codex_apps_mcp(server) + { return; } let metadata = lookup_mcp_app_usage_metadata(sess, server, tool_name).await; @@ -1098,7 +1112,7 @@ fn build_mcp_tool_call_request_meta( ); } - if is_codex_apps_mcp(server) { + if server == CODEX_APPS_MCP_SERVER_NAME { let mut codex_apps_meta = metadata .and_then(|metadata| metadata.codex_apps_meta.clone()) .unwrap_or_default(); @@ -1403,7 +1417,7 @@ fn session_mcp_tool_approval_key( } let connector_id = metadata.and_then(|metadata| metadata.connector_id.clone()); - if is_codex_apps_mcp(&invocation.server) && connector_id.is_none() { + if invocation.server == CODEX_APPS_MCP_SERVER_NAME && connector_id.is_none() { return None; } @@ -1481,7 +1495,7 @@ pub(crate) async fn lookup_mcp_tool_metadata( let tool_info = tools .into_iter() .find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?; - let connector_description = if is_codex_apps_mcp(server) { + let connector_description = if manager.is_codex_apps_mcp(server) { let connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools( turn_context.config.as_ref(), ) @@ -1540,7 +1554,7 @@ fn openai_file_input_params_for_server( server: &str, meta: Option<&serde_json::Map>, ) -> Option> { - is_codex_apps_mcp(server) + (server == CODEX_APPS_MCP_SERVER_NAME) .then_some(declared_openai_file_input_param_names(meta)) .filter(|params| !params.is_empty()) } @@ -1645,7 +1659,7 @@ fn build_mcp_tool_approval_fallback_message( .filter(|name| !name.is_empty()) .map(ToString::to_string) .unwrap_or_else(|| { - if is_codex_apps_mcp(server) { + if server == CODEX_APPS_MCP_SERVER_NAME { "this app".to_string() } else { format!("the {server} MCP server") @@ -1739,7 +1753,7 @@ fn build_mcp_tool_approval_elicitation_meta( serde_json::Value::String(tool_description.clone()), ); } - if is_codex_apps_mcp(server) + if server == CODEX_APPS_MCP_SERVER_NAME && (metadata.connector_id.is_some() || metadata.connector_name.is_some() || metadata.connector_description.is_some()) @@ -1966,7 +1980,12 @@ async fn maybe_persist_mcp_tool_approval( ) { let tool_name = key.tool_name.clone(); - let persist_result = if is_codex_apps_mcp(&key.server) { + let is_codex_apps_mcp = sess + .services + .mcp_connection_manager + .load() + .is_codex_apps_mcp(&key.server); + let persist_result = if is_codex_apps_mcp { let Some(connector_id) = key.connector_id.clone() else { remember_mcp_tool_approval(sess, key).await; return; diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 91202c5c09..edb23da7ef 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -48,6 +48,7 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use tempfile::tempdir; +use tokio_util::sync::CancellationToken; use tracing::Instrument; use tracing::Level; use tracing_subscriber::fmt::format::FmtSpan; @@ -1161,7 +1162,10 @@ fn mcp_tool_call_item_metadata_only_trusts_codex_apps_identity() { metadata.link_id = Some("link_fedcba9876543210fedcba9876543210".to_string()); assert_eq!( - McpToolCallItemMetadata::from_tool_metadata(CODEX_APPS_MCP_SERVER_NAME, Some(&metadata),), + McpToolCallItemMetadata::from_tool_metadata( + /*is_codex_apps_mcp*/ true, + Some(&metadata), + ), McpToolCallItemMetadata { connector_id: Some("asdk_app_0123456789abcdef0123456789abcdef".to_string()), link_id: Some("link_fedcba9876543210fedcba9876543210".to_string()), @@ -1170,7 +1174,10 @@ fn mcp_tool_call_item_metadata_only_trusts_codex_apps_identity() { } ); assert_eq!( - McpToolCallItemMetadata::from_tool_metadata("custom_server", Some(&metadata)), + McpToolCallItemMetadata::from_tool_metadata( + /*is_codex_apps_mcp*/ false, + Some(&metadata), + ), McpToolCallItemMetadata { connector_id: None, link_id: None, @@ -1332,9 +1339,56 @@ fn codex_apps_auth_failure_metadata() -> McpToolApprovalMetadata { ) } +async fn install_codex_apps_manager(session: &Session, turn_context: &TurnContext) { + let auth = session.services.auth_manager.auth().await; + let mcp_servers = HashMap::from([( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + codex_mcp::EffectiveMcpServer::configured(codex_mcp::codex_apps_mcp_server_config( + "http://127.0.0.1:9", + /*apps_mcp_product_sku*/ None, + )), + )]); + let (tx_event, rx_event) = async_channel::unbounded(); + drop(rx_event); + let startup_cancellation_token = CancellationToken::new(); + startup_cancellation_token.cancel(); + let manager = codex_mcp::McpConnectionManager::new( + &mcp_servers, + turn_context.config.mcp_oauth_credentials_store_mode, + turn_context.config.auth_keyring_backend_kind(), + HashMap::new(), + &turn_context.approval_policy, + turn_context.sub_id.clone(), + tx_event, + startup_cancellation_token, + turn_context.permission_profile(), + codex_mcp::McpRuntimeContext::new( + session.services.turn_environments.environment_manager(), + { + #[allow(deprecated)] + turn_context.cwd.to_path_buf() + }, + ), + turn_context.config.codex_home.to_path_buf(), + codex_mcp::codex_apps_tools_cache_key(auth.as_ref()), + turn_context.config.prefix_mcp_tool_names(), + rmcp::model::ElicitationCapability::default(), + /*supports_openai_form_elicitation*/ false, + codex_mcp::ToolPluginProvenance::default(), + auth.as_ref(), + /*elicitation_reviewer*/ None, + ) + .await; + session + .services + .mcp_connection_manager + .store(Arc::new(manager)); +} + #[tokio::test] async fn codex_apps_auth_elicitation_feature_disabled_returns_original_result() { let (session, turn_context, rx_event) = make_session_and_context_with_rx().await; + install_codex_apps_manager(&session, &turn_context).await; let result = codex_apps_auth_failure_result(); let metadata = codex_apps_auth_failure_metadata(); @@ -1379,6 +1433,7 @@ async fn codex_apps_auth_elicitation_other_server_returns_original_result() { #[tokio::test] async fn codex_apps_auth_elicitation_disallowed_by_policy_returns_original_result() { let (session, mut turn_context, rx_event) = make_session_and_context_with_rx().await; + install_codex_apps_manager(&session, &turn_context).await; let mut features = Features::with_defaults(); features.enable(Feature::AuthElicitation); let turn_context = Arc::get_mut(&mut turn_context).expect("single turn context ref"); @@ -1407,6 +1462,7 @@ async fn codex_apps_auth_elicitation_disallowed_by_policy_returns_original_resul #[tokio::test] async fn codex_apps_auth_elicitation_granular_mcp_disabled_returns_original_result() { let (session, mut turn_context, rx_event) = make_session_and_context_with_rx().await; + install_codex_apps_manager(&session, &turn_context).await; let mut features = Features::with_defaults(); features.enable(Feature::AuthElicitation); let turn_context = Arc::get_mut(&mut turn_context).expect("single turn context ref"); @@ -1441,6 +1497,7 @@ async fn codex_apps_auth_elicitation_granular_mcp_disabled_returns_original_resu #[tokio::test] async fn codex_apps_auth_elicitation_feature_enabled_requests_elicitation() { let (session, mut turn_context, rx_event) = make_session_and_context_with_rx().await; + install_codex_apps_manager(&session, &turn_context).await; *session.active_turn.lock().await = Some(ActiveTurn::default()); let mut features = Features::with_defaults(); features.enable(Feature::AuthElicitation); diff --git a/codex-rs/core/src/mcp_tool_exposure.rs b/codex-rs/core/src/mcp_tool_exposure.rs index 1e1ad34bd8..fda2b04c9b 100644 --- a/codex-rs/core/src/mcp_tool_exposure.rs +++ b/codex-rs/core/src/mcp_tool_exposure.rs @@ -2,8 +2,8 @@ use std::collections::HashSet; use codex_connectors::AppToolPolicyEvaluator; use codex_connectors::AppToolPolicyInput; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ToolInfo as McpToolInfo; -use codex_mcp::is_codex_apps_mcp; use codex_mcp::tool_is_model_visible; use tracing::instrument; @@ -47,7 +47,9 @@ pub(crate) fn build_mcp_tool_exposure( fn filter_non_codex_apps_mcp_tools_only(mcp_tools: &[McpToolInfo]) -> Vec { mcp_tools .iter() - .filter(|tool| !is_codex_apps_mcp(&tool.server_name) && tool_is_model_visible(tool)) + .filter(|tool| { + tool.server_name != CODEX_APPS_MCP_SERVER_NAME && tool_is_model_visible(tool) + }) .cloned() .collect() } @@ -66,7 +68,7 @@ fn filter_codex_apps_mcp_tools( mcp_tools .iter() .filter(|tool| { - if !is_codex_apps_mcp(&tool.server_name) { + if tool.server_name != CODEX_APPS_MCP_SERVER_NAME { return false; } if !tool_is_model_visible(tool) { diff --git a/codex-rs/core/src/plugins/injection.rs b/codex-rs/core/src/plugins/injection.rs index 9610e5026e..48e15247b5 100644 --- a/codex-rs/core/src/plugins/injection.rs +++ b/codex-rs/core/src/plugins/injection.rs @@ -8,8 +8,8 @@ use crate::context::ContextualUserFragment; use crate::context::PluginInstructions; use crate::plugins::PluginCapabilitySummary; use crate::plugins::render_explicit_plugin_instructions; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ToolInfo; -use codex_mcp::is_codex_apps_mcp; pub(crate) fn build_plugin_injections( mentioned_plugins: &[PluginCapabilitySummary], @@ -28,7 +28,7 @@ pub(crate) fn build_plugin_injections( let available_mcp_servers = mcp_tools .iter() .filter(|tool| { - !is_codex_apps_mcp(&tool.server_name) + tool.server_name != CODEX_APPS_MCP_SERVER_NAME && tool .plugin_display_names .iter() diff --git a/codex-rs/core/src/tools/handlers/mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource.rs index c23bc28382..231bdf92a9 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; -use codex_mcp::is_codex_apps_mcp; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_protocol::items::McpToolCallError; use codex_protocol::items::McpToolCallItem; use codex_protocol::items::McpToolCallStatus; @@ -35,7 +35,7 @@ pub use list_mcp_resources::ListMcpResourcesHandler; pub use read_mcp_resource::ReadMcpResourceHandler; fn model_can_access_mcp_server(turn: &TurnContext, server: &str) -> bool { - turn.config.orchestrator_mcp_enabled || !is_codex_apps_mcp(server) + turn.config.orchestrator_mcp_enabled || server != CODEX_APPS_MCP_SERVER_NAME } fn ensure_model_can_access_mcp_server( diff --git a/codex-rs/tui/src/bottom_pane/app_link_view.rs b/codex-rs/tui/src/bottom_pane/app_link_view.rs index 4c96cd971b..bbd7f355c3 100644 --- a/codex-rs/tui/src/bottom_pane/app_link_view.rs +++ b/codex-rs/tui/src/bottom_pane/app_link_view.rs @@ -2,7 +2,6 @@ use crate::app_command::AppCommand as Op; use codex_app_server_protocol::McpServerElicitationAction; use codex_app_server_protocol::RequestId as AppServerRequestId; -use codex_mcp::is_codex_apps_mcp; use codex_protocol::ThreadId; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -38,6 +37,7 @@ use crate::style::user_message_style; use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_lines; +const MCP_CODEX_APPS_SERVER_NAME: &str = "codex_apps"; const MCP_TOOL_CODEX_APPS_META_KEY: &str = "_codex_apps"; const CONNECTOR_AUTH_FAILURE_META_KEY: &str = "connector_auth_failure"; const CONNECTOR_AUTH_FAILURE_IS_AUTH_FAILURE_KEY: &str = "is_auth_failure"; @@ -95,7 +95,7 @@ impl AppLinkViewParams { else { return None; }; - if is_codex_apps_mcp(server_name) { + if server_name == MCP_CODEX_APPS_SERVER_NAME { let url = validate_external_url(url, /*require_chatgpt_host*/ true)?; return Self::from_codex_apps_auth_url_parts( thread_id, @@ -381,7 +381,7 @@ impl AppLinkView { let should_refresh_connectors = self .elicitation_target .as_ref() - .is_none_or(|target| is_codex_apps_mcp(&target.server_name)); + .is_none_or(|target| target.server_name == MCP_CODEX_APPS_SERVER_NAME); if should_refresh_connectors { self.app_event_tx.send(AppEvent::RefreshConnectors { force_refetch: true, @@ -559,7 +559,7 @@ impl AppLinkView { && self .elicitation_target .as_ref() - .is_some_and(|target| is_codex_apps_mcp(&target.server_name)); + .is_some_and(|target| target.server_name == MCP_CODEX_APPS_SERVER_NAME); lines.push(Line::from( if is_auth_suggestion { if is_codex_apps_auth {