From f15adb86b43087adeab47e7bf0e2e53bc19d3e53 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 22 Jun 2026 17:43:30 -0700 Subject: [PATCH] Use effective server eligibility as source of truth --- codex-rs/codex-mcp/src/connection_manager.rs | 6 -- .../codex-mcp/src/connection_manager_tests.rs | 1 - codex-rs/codex-mcp/src/mcp/mod.rs | 22 ++---- codex-rs/codex-mcp/src/server.rs | 17 ----- codex-rs/core/src/mcp_tool_call.rs | 7 +- codex-rs/core/src/mcp_tool_call_tests.rs | 68 +------------------ 6 files changed, 10 insertions(+), 111 deletions(-) diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index a06020b2cc..67c837d7b1 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -401,12 +401,6 @@ impl McpConnectionManager { .unwrap_or_default() } - pub fn server_supports_connector_auth_repair(&self, server_name: &str) -> bool { - self.server_metadata - .get(server_name) - .is_some_and(McpServerMetadata::supports_connector_auth_repair) - } - pub fn set_approval_policy(&self, approval_policy: &Constrained) { if let Ok(mut policy) = self.elicitation_requests.approval_policy.lock() { *policy = approval_policy.value(); diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index c0f7755e61..b2b3b98be0 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -1147,7 +1147,6 @@ async fn list_all_tools_adds_server_metadata_to_cached_tools() { "https://docs.example".to_string(), )), supports_parallel_tool_calls: true, - supports_connector_auth_repair: false, default_tools_approval_mode: None, tool_approval_modes: HashMap::new(), }, diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index d79570bbf3..da1af5d11e 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -253,22 +253,14 @@ pub fn effective_mcp_servers_from_configured( ) -> HashMap { let codex_apps_available = config.apps_enabled && auth.is_some_and(CodexAuth::uses_codex_backend); - configured_servers + let mut servers = configured_servers .into_iter() - .filter_map(|(name, server)| { - if name == CODEX_APPS_MCP_SERVER_NAME && !codex_apps_available { - return None; - } - - let server = EffectiveMcpServer::configured(server); - let server = if name == CODEX_APPS_MCP_SERVER_NAME { - server.with_connector_auth_repair() - } else { - server - }; - Some((name, server)) - }) - .collect() + .map(|(name, server)| (name, EffectiveMcpServer::configured(server))) + .collect::>(); + if !codex_apps_available { + servers.remove(CODEX_APPS_MCP_SERVER_NAME); + } + servers } pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance { diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index 6b8a7fba25..39287b2136 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -14,22 +14,15 @@ pub(crate) enum McpServerLaunch { #[derive(Debug, Clone)] pub struct EffectiveMcpServer { launch: McpServerLaunch, - connector_auth_repair: bool, } impl EffectiveMcpServer { pub fn configured(config: McpServerConfig) -> Self { Self { launch: McpServerLaunch::Configured(Box::new(config)), - connector_auth_repair: false, } } - pub(crate) fn with_connector_auth_repair(mut self) -> Self { - self.connector_auth_repair = true; - self - } - pub(crate) fn launch(&self) -> &McpServerLaunch { &self.launch } @@ -51,10 +44,6 @@ impl EffectiveMcpServer { McpServerLaunch::Configured(config) => config.required, } } - - fn supports_connector_auth_repair(&self) -> bool { - self.connector_auth_repair - } } /// Transport origin retained for metrics and diagnostics after server launch. @@ -90,16 +79,11 @@ pub(crate) struct McpServerMetadata { pub pollutes_memory: bool, pub origin: Option, pub supports_parallel_tool_calls: bool, - pub supports_connector_auth_repair: bool, pub default_tools_approval_mode: Option, pub tool_approval_modes: HashMap, } impl McpServerMetadata { - pub fn supports_connector_auth_repair(&self) -> bool { - self.supports_connector_auth_repair - } - pub fn tool_approval_mode(&self, tool_name: &str) -> AppToolApproval { self.tool_approval_modes .get(tool_name) @@ -117,7 +101,6 @@ impl From<&EffectiveMcpServer> for McpServerMetadata { pollutes_memory: true, origin: McpServerOrigin::from_transport(&config.transport), supports_parallel_tool_calls: config.supports_parallel_tool_calls, - supports_connector_auth_repair: server.supports_connector_auth_repair(), default_tools_approval_mode: config.default_tools_approval_mode, tool_approval_modes: config .tools diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index db795d5328..e9a2275db1 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -631,12 +631,7 @@ async fn maybe_request_codex_apps_auth_elicitation( metadata: Option<&McpToolApprovalMetadata>, result: CallToolResult, ) -> CallToolResult { - if !sess - .services - .mcp_connection_manager - .load_full() - .server_supports_connector_auth_repair(server) - { + if server != CODEX_APPS_MCP_SERVER_NAME { return result; } diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 5b0ccacd7d..d68accbe70 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -8,7 +8,6 @@ use crate::state::ActiveTurn; use crate::test_support::models_manager_with_provider; use crate::tools::hook_names::HookToolName; use crate::turn_metadata::McpTurnMetadataContext; -use async_channel::unbounded; use codex_config::CONFIG_TOML_FILE; use codex_config::config_toml::ConfigToml; use codex_config::types::AppConfig; @@ -21,9 +20,6 @@ use codex_config::types::McpServerToolConfig; use codex_features::Features; use codex_hooks::Hooks; use codex_hooks::HooksConfig; -use codex_login::CodexAuth; -use codex_mcp::codex_apps_mcp_server_config; -use codex_mcp::effective_mcp_servers_from_configured; use codex_model_provider::create_model_provider; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -52,7 +48,6 @@ 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; @@ -1337,65 +1332,9 @@ fn codex_apps_auth_failure_metadata() -> McpToolApprovalMetadata { ) } -async fn install_connector_auth_repair_manager(session: &Session, turn_context: &TurnContext) { - let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - let mut mcp_config = turn_context - .config - .to_mcp_config(session.services.plugins_manager.as_ref()) - .await; - mcp_config.apps_enabled = true; - let mcp_servers = effective_mcp_servers_from_configured( - HashMap::from([( - CODEX_APPS_MCP_SERVER_NAME.to_string(), - codex_apps_mcp_server_config( - &turn_context.config.chatgpt_base_url, - /*apps_mcp_product_sku*/ None, - ), - )]), - &mcp_config, - Some(&auth), - ); - let (tx_event, rx_event) = 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(Some(&auth)), - turn_context.config.prefix_mcp_tool_names(), - rmcp::model::ElicitationCapability::default(), - /*supports_openai_form_elicitation*/ false, - codex_mcp::ToolPluginProvenance::default(), - Some(&auth), - /*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_connector_auth_repair_manager(&session, &turn_context).await; let result = codex_apps_auth_failure_result(); let metadata = codex_apps_auth_failure_metadata(); @@ -1414,7 +1353,7 @@ async fn codex_apps_auth_elicitation_feature_disabled_returns_original_result() } #[tokio::test] -async fn codex_apps_auth_elicitation_server_without_repair_capability_returns_original_result() { +async fn codex_apps_auth_elicitation_non_codex_apps_server_returns_original_result() { let (session, mut turn_context, rx_event) = make_session_and_context_with_rx().await; let mut features = Features::with_defaults(); features.enable(Feature::AuthElicitation); @@ -1427,7 +1366,7 @@ async fn codex_apps_auth_elicitation_server_without_repair_capability_returns_or &session, turn_context, "call_123", - CODEX_APPS_MCP_SERVER_NAME, + "custom_server", Some(&metadata), result.clone(), ) @@ -1440,7 +1379,6 @@ async fn codex_apps_auth_elicitation_server_without_repair_capability_returns_or #[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_connector_auth_repair_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"); @@ -1469,7 +1407,6 @@ 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_connector_auth_repair_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"); @@ -1504,7 +1441,6 @@ 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_connector_auth_repair_manager(&session, &turn_context).await; *session.active_turn.lock().await = Some(ActiveTurn::default()); let mut features = Features::with_defaults(); features.enable(Feature::AuthElicitation);