github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-21 09:54:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Clarify MCP OAuth reauthentication errors (#38418)

Browse Source 
## Why

When an MCP server rejects existing OAuth credentials, reporting that the server
is merely not logged in obscures that the user needs to authenticate again.

## What changed

Pass the MCP startup failure reason into error formatting and report that the
server requires OAuth reauthentication when appropriate. Preserve the existing
recovery hint for either `codex mcp login` or the client's OAuth sign-in flow.

## Testing

Cover the reauthentication message for both login flows and verify it in the
streamable HTTP OAuth round-trip test.

GitOrigin-RevId: 6f6d5fe7519b1a476f8b70d976223b2ea1ec938e
This commit is contained in:
Eric Traut
2026-08-13 18:45:13 +00:00
committed by copyberry
parent 4f7032173e
commit f8a3db0b99
4 changed files with 90 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/codex-mcp/src/connection_manager.rs
+8 -5
View File
@@ -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,
codex-rs/codex-mcp/src/connection_manager/startup.rs
+8 -1
View File
@@ -76,6 +76,7 @@ pub(super) fn mcp_init_error_display(
server_name: &str,
config: Option<&McpServerConfig>,
error: &StartupOutcomeError,
reason: Option<McpStartupFailureReason>,
) -> 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, .. }
codex-rs/codex-mcp/src/connection_manager_tests.rs
+54 -5
View File
@@ -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",
codex-rs/core/tests/suite/rmcp_client.rs
+20 -17
View File
@@ -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()