Use effective server eligibility as source of truth

This commit is contained in:
Ahmed Ibrahim
2026-06-22 17:43:30 -07:00
parent f9b1a921dc
commit f15adb86b4
6 changed files with 10 additions and 111 deletions
@@ -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<AskForApproval>) {
if let Ok(mut policy) = self.elicitation_requests.approval_policy.lock() {
*policy = approval_policy.value();
@@ -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(),
},
+7 -15
View File
@@ -253,22 +253,14 @@ pub fn effective_mcp_servers_from_configured(
) -> HashMap<String, EffectiveMcpServer> {
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::<HashMap<_, _>>();
if !codex_apps_available {
servers.remove(CODEX_APPS_MCP_SERVER_NAME);
}
servers
}
pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance {
-17
View File
@@ -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<McpServerOrigin>,
pub supports_parallel_tool_calls: bool,
pub supports_connector_auth_repair: bool,
pub default_tools_approval_mode: Option<AppToolApproval>,
pub tool_approval_modes: HashMap<String, AppToolApproval>,
}
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
+1 -6
View File
@@ -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;
}
+2 -66
View File
@@ -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);