diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index d0da7ad805..ca4a0d3654 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -413,17 +413,19 @@ impl McpConnectionSet { }, ); if let Some(error) = unchanged_auth_failure { + let reason = connection_identity + .oauth_credentials() + .ok() + .flatten() + .map(|_| McpStartupFailureReason::ReauthenticationRequired); let status = McpStartupStatus::Failed { error: mcp_init_error_display( &server_name, Some(&configured_config), &error, + reason, ), - reason: connection_identity - .oauth_credentials() - .ok() - .flatten() - .map(|_| McpStartupFailureReason::ReauthenticationRequired), + reason, }; let tx_event = tx_event.clone(); let submit_id = startup_submit_id.clone(); @@ -603,6 +605,7 @@ impl McpConnectionSet { server_name.as_str(), Some(&configured_config), error, + reason, ); McpStartupStatus::Failed { error: error_str, diff --git a/codex-rs/codex-mcp/src/connection_manager/startup.rs b/codex-rs/codex-mcp/src/connection_manager/startup.rs index fc933e1574..138d6877be 100644 --- a/codex-rs/codex-mcp/src/connection_manager/startup.rs +++ b/codex-rs/codex-mcp/src/connection_manager/startup.rs @@ -76,6 +76,7 @@ pub(super) fn mcp_init_error_display( server_name: &str, config: Option<&McpServerConfig>, error: &StartupOutcomeError, + reason: Option, ) -> String { if let Some(McpServerTransportConfig::StreamableHttp { url, @@ -101,7 +102,13 @@ pub(super) fn mcp_init_error_display( } else { format!("Run `codex mcp login {server_name}`.") }; - format!("The {server_name} MCP server is not logged in. {recovery_hint}") + let auth_status = match reason { + Some(McpStartupFailureReason::ReauthenticationRequired) => { + "requires OAuth reauthentication" + } + None => "is not logged in", + }; + format!("The {server_name} MCP server {auth_status}. {recovery_hint}") } else if matches!( error, StartupOutcomeError::Failed { error, .. } diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 65356f3ed3..7f2bac697b 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -3700,7 +3700,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { }; let err: StartupOutcomeError = anyhow::anyhow!("OAuth is unsupported").into(); - let display = mcp_init_error_display(server_name, Some(&config), &err); + let display = mcp_init_error_display(server_name, Some(&config), &err, /*reason*/ None); let expected = format!( "GitHub MCP does not support OAuth. Log in by adding a personal access token (https://github.com/settings/personal-access-tokens) to your environment and config.toml:\n[mcp_servers.{server_name}]\nbearer_token_env_var = CODEX_GITHUB_PERSONAL_ACCESS_TOKEN" @@ -3728,10 +3728,20 @@ fn mcp_init_error_display_prompts_for_login_when_auth_required() { is_authentication_required: true, }, ] { - let display = mcp_init_error_display(server_name, /*config*/ None, &error); + let display = mcp_init_error_display( + server_name, + /*config*/ None, + &error, + /*reason*/ None, + ); assert_eq!(expected, display); - let executor_display = mcp_init_error_display(server_name, Some(&executor_config), &error); + let executor_display = mcp_init_error_display( + server_name, + Some(&executor_config), + &error, + /*reason*/ None, + ); assert_eq!( format!( "The {server_name} MCP server is not logged in. Use your client's MCP OAuth sign-in flow." @@ -3741,6 +3751,40 @@ fn mcp_init_error_display_prompts_for_login_when_auth_required() { } } +#[test] +fn mcp_init_error_display_identifies_oauth_reauthentication() { + let server_name = "example"; + let error = StartupOutcomeError::Failed { + error: "authorization required: Bearer error=\"invalid_token\"".to_string(), + is_authentication_required: true, + }; + let executor_config: McpServerConfig = serde_json::from_value(serde_json::json!({ + "url": "https://example.com/mcp", + "environment_id": "executor-1", + })) + .expect("executor MCP configuration should deserialize"); + + for (config, recovery_hint) in [ + (None, "Run `codex mcp login example`."), + ( + Some(&executor_config), + "Use your client's MCP OAuth sign-in flow.", + ), + ] { + assert_eq!( + mcp_init_error_display( + server_name, + config, + &error, + Some(McpStartupFailureReason::ReauthenticationRequired), + ), + format!( + "The {server_name} MCP server requires OAuth reauthentication. {recovery_hint}" + ), + ); + } +} + #[test] fn mcp_startup_failure_reason_requires_existing_oauth_and_auth_failure() { for (auth_state, is_authentication_required, expected) in [ @@ -3816,7 +3860,7 @@ fn mcp_init_error_display_reports_generic_errors() { }; let err: StartupOutcomeError = anyhow::anyhow!("boom").into(); - let display = mcp_init_error_display(server_name, Some(&config), &err); + let display = mcp_init_error_display(server_name, Some(&config), &err, /*reason*/ None); let expected = format!("MCP client for `{server_name}` failed to start: {err:#}"); @@ -3832,7 +3876,12 @@ fn mcp_init_error_display_includes_startup_timeout_hint() { ] { let err: StartupOutcomeError = anyhow::anyhow!(error).into(); - let display = mcp_init_error_display(server_name, /*config*/ None, &err); + let display = mcp_init_error_display( + server_name, + /*config*/ None, + &err, + /*reason*/ None, + ); assert_eq!( "MCP client for `slow` timed out after 30 seconds. Add or adjust `startup_timeout_sec` in your config.toml:\n[mcp_servers.slow]\nstartup_timeout_sec = XX", diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 33614db3e5..098c5d74c8 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -3289,13 +3289,22 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { .build_with_auto_env(&server) .await?; // Phase 5: replace rejected credentials as an external OAuth login would. - let mut failure_reason = None; + let recovery_hint = if credential_config.is_local_environment() { + format!("Run `codex mcp login {server_name}`.") + } else { + "Use your client's MCP OAuth sign-in flow.".to_string() + }; + let expected_failure = ( + format!("The {server_name} MCP server requires OAuth reauthentication. {recovery_hint}"), + Some(McpStartupFailureReason::ReauthenticationRequired), + ); + let mut failure = None; let startup = wait_for_event(&fixture.codex, |event| { if let EventMsg::McpStartupUpdate(update) = event && update.server == server_name - && let McpStartupStatus::Failed { reason, .. } = &update.status + && let McpStartupStatus::Failed { error, reason } = &update.status { - failure_reason = *reason; + failure = Some((error.clone(), *reason)); } matches!(event, EventMsg::McpStartupComplete(_)) }) @@ -3305,10 +3314,7 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { }; assert_eq!(startup.failed.len(), 1); assert_eq!(startup.failed[0].server, server_name); - assert_eq!( - failure_reason, - Some(McpStartupFailureReason::ReauthenticationRequired), - ); + assert_eq!(failure, Some(expected_failure.clone())); let store_lock = fs::OpenOptions::new() .read(true) @@ -3322,23 +3328,23 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { "continue while OAuth credentials are locked", )) .await?; - let (contended_turn, refreshed_failure_reason, refreshed_failed_servers) = + let (contended_turn, refreshed_failure, refreshed_failed_servers) = tokio::time::timeout(Duration::from_secs(5), async { let mut contended_turn = None; - let mut refreshed_failure_reason = None; + let mut refreshed_failure = None; let mut refreshed_failed_servers = None; let mut refreshed_starting = false; while contended_turn.is_none() - || refreshed_failure_reason.is_none() + || refreshed_failure.is_none() || refreshed_failed_servers.is_none() { match fixture.codex.next_event().await?.msg { EventMsg::McpStartupUpdate(update) if update.server == server_name => { match update.status { McpStartupStatus::Starting => refreshed_starting = true, - McpStartupStatus::Failed { reason, .. } => { + McpStartupStatus::Failed { error, reason } => { assert!(refreshed_starting); - refreshed_failure_reason = Some(reason); + refreshed_failure = Some((error, reason)); } McpStartupStatus::Ready | McpStartupStatus::Cancelled => {} } @@ -3352,16 +3358,13 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { } Ok::<_, anyhow::Error>(( contended_turn.expect("turn completion was observed"), - refreshed_failure_reason.expect("failure status was observed"), + refreshed_failure.expect("failure status was observed"), refreshed_failed_servers.expect("startup summary was observed"), )) }) .await .context("OAuth credential-store contention blocked the user turn or startup status")??; - assert_eq!( - refreshed_failure_reason, - Some(McpStartupFailureReason::ReauthenticationRequired), - ); + assert_eq!(refreshed_failure, expected_failure); assert_eq!( refreshed_failed_servers .into_iter()