diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 18435f6f2..8a8064c0b 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -2384,6 +2384,12 @@ ], "type": "object" }, + "McpServerStartupFailureReason": { + "enum": [ + "reauthenticationRequired" + ], + "type": "string" + }, "McpServerStartupState": { "enum": [ "starting", @@ -2401,6 +2407,16 @@ "null" ] }, + "failureReason": { + "anyOf": [ + { + "$ref": "#/definitions/McpServerStartupFailureReason" + }, + { + "type": "null" + } + ] + }, "name": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 8625c08da..56ea39276 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -12094,6 +12094,12 @@ "title": "McpServerRefreshResponse", "type": "object" }, + "McpServerStartupFailureReason": { + "enum": [ + "reauthenticationRequired" + ], + "type": "string" + }, "McpServerStartupState": { "enum": [ "starting", @@ -12165,6 +12171,16 @@ "null" ] }, + "failureReason": { + "anyOf": [ + { + "$ref": "#/definitions/v2/McpServerStartupFailureReason" + }, + { + "type": "null" + } + ] + }, "name": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 94fb8a989..0f2910bef 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -8498,6 +8498,12 @@ "title": "McpServerRefreshResponse", "type": "object" }, + "McpServerStartupFailureReason": { + "enum": [ + "reauthenticationRequired" + ], + "type": "string" + }, "McpServerStartupState": { "enum": [ "starting", @@ -8569,6 +8575,16 @@ "null" ] }, + "failureReason": { + "anyOf": [ + { + "$ref": "#/definitions/McpServerStartupFailureReason" + }, + { + "type": "null" + } + ] + }, "name": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/McpServerStatusUpdatedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/McpServerStatusUpdatedNotification.json index 87dab90cf..8efb36bda 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/McpServerStatusUpdatedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/McpServerStatusUpdatedNotification.json @@ -1,6 +1,12 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "McpServerStartupFailureReason": { + "enum": [ + "reauthenticationRequired" + ], + "type": "string" + }, "McpServerStartupState": { "enum": [ "starting", @@ -18,6 +24,16 @@ "null" ] }, + "failureReason": { + "anyOf": [ + { + "$ref": "#/definitions/McpServerStartupFailureReason" + }, + { + "type": "null" + } + ] + }, "name": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStartupFailureReason.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStartupFailureReason.ts new file mode 100644 index 000000000..0373e5444 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStartupFailureReason.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type McpServerStartupFailureReason = "reauthenticationRequired"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatusUpdatedNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatusUpdatedNotification.ts index b5c0a9058..fd192f22c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatusUpdatedNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerStatusUpdatedNotification.ts @@ -1,6 +1,7 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { McpServerStartupFailureReason } from "./McpServerStartupFailureReason"; import type { McpServerStartupState } from "./McpServerStartupState"; -export type McpServerStatusUpdatedNotification = { threadId: string | null, name: string, status: McpServerStartupState, error: string | null, }; +export type McpServerStatusUpdatedNotification = { threadId: string | null, name: string, status: McpServerStartupState, error: string | null, failureReason: McpServerStartupFailureReason | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index ff7df9e9e..82b74711c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -238,6 +238,7 @@ export type { McpServerOauthLoginCompletedNotification } from "./McpServerOauthL export type { McpServerOauthLoginParams } from "./McpServerOauthLoginParams"; export type { McpServerOauthLoginResponse } from "./McpServerOauthLoginResponse"; export type { McpServerRefreshResponse } from "./McpServerRefreshResponse"; +export type { McpServerStartupFailureReason } from "./McpServerStartupFailureReason"; export type { McpServerStartupState } from "./McpServerStartupState"; export type { McpServerStatus } from "./McpServerStatus"; export type { McpServerStatusDetail } from "./McpServerStatusDetail"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs index ca0e124d5..d24da0c87 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs @@ -23,6 +23,12 @@ v2_enum_from_core!( } ); +v2_enum_from_core!( + pub enum McpServerStartupFailureReason from codex_protocol::protocol::McpStartupFailureReason { + ReauthenticationRequired + } +); + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -242,6 +248,7 @@ pub struct McpServerStatusUpdatedNotification { pub name: String, pub status: McpServerStartupState, pub error: Option, + pub failure_reason: Option, } #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 7c78fee4c..03c2bcaa5 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::ServerNotification; use codex_protocol::approvals::ElicitationRequest as CoreElicitationRequest; use codex_protocol::config_types::MultiAgentMode; use codex_protocol::items::AgentMessageContent; @@ -2155,6 +2156,7 @@ fn mcp_server_status_updated_accepts_missing_thread_id() { name: "optional_broken".to_string(), status: McpServerStartupState::Failed, error: Some("handshake failed".to_string()), + failure_reason: None, }; assert_eq!(notification, expected); assert_eq!( @@ -2164,6 +2166,33 @@ fn mcp_server_status_updated_accepts_missing_thread_id() { "name": "optional_broken", "status": "failed", "error": "handshake failed", + "failureReason": null, + }) + ); +} + +#[test] +fn mcp_server_status_updated_serializes_failure_reason() { + let notification = + ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification { + thread_id: Some("thread-1".to_string()), + name: "expired-oauth".to_string(), + status: McpServerStartupState::Failed, + error: Some("OAuth credentials expired".to_string()), + failure_reason: Some(McpServerStartupFailureReason::ReauthenticationRequired), + }); + + assert_eq!( + serde_json::to_value(notification).expect("notification should serialize"), + json!({ + "method": "mcpServer/startupStatus/updated", + "params": { + "threadId": "thread-1", + "name": "expired-oauth", + "status": "failed", + "error": "OAuth credentials expired", + "failureReason": "reauthenticationRequired", + }, }) ); } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 1a3684a02..a4b488acd 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1345,7 +1345,7 @@ Because audio is intentionally separate from `ThreadItem`, clients can opt out o ### MCP server startup events -- `mcpServer/startupStatus/updated` — `{ threadId, name, status, error }` when app-server observes an MCP server startup transition. `threadId` identifies the owning thread when startup is thread-scoped and is `null` when startup is app-scoped. `status` is one of `starting`, `ready`, `failed`, or `cancelled`. `error` is `null` except for `failed`. +- `mcpServer/startupStatus/updated` — `{ threadId, name, status, error, failureReason }` when app-server observes an MCP server startup transition. `threadId` identifies the owning thread when startup is thread-scoped and is `null` when startup is app-scoped. `status` is one of `starting`, `ready`, `failed`, or `cancelled`. `error` and `failureReason` are `null` except for `failed`; `failureReason` is `reauthenticationRequired` when stored OAuth credentials have expired and cannot be refreshed, so clients can prompt the user to reconnect the named server. ### Turn events @@ -1935,7 +1935,7 @@ Codex supports these authentication modes. The current mode is surfaced in `acco - `account/rateLimits/updated` (notify) — emitted whenever a user's ChatGPT rate limits change. This is a sparse rolling update; merge available values into the most recent `account/rateLimits/read` response or refetch that snapshot. - `account/sendAddCreditsNudgeEmail` — ask ChatGPT to email the workspace owner about depleted credits or a reached usage limit. - `mcpServer/oauthLogin/completed` (notify) — emitted after a `mcpServer/oauth/login` flow finishes for a server; payload includes `{ name, threadId, success, error? }`. -- `mcpServer/startupStatus/updated` (notify) — emitted when a configured MCP server's startup status changes; payload includes `{ threadId, name, status, error }`, where `threadId` is the owning thread when startup is thread-scoped and `null` when it is app-scoped, and `status` is `starting`, `ready`, `failed`, or `cancelled`. +- `mcpServer/startupStatus/updated` (notify) — emitted when a configured MCP server's startup status changes; payload includes `{ threadId, name, status, error, failureReason }`, where `threadId` is the owning thread when startup is thread-scoped and `null` when it is app-scoped, and `status` is `starting`, `ready`, `failed`, or `cancelled`. `failureReason` is `reauthenticationRequired` when stored OAuth credentials have expired and cannot be refreshed, so clients can prompt the user to reconnect the named server. ### 1) Check auth state diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 36dbdd9d3..bcba603bc 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -200,18 +200,20 @@ pub(crate) async fn apply_bespoke_event_handling( .await; } EventMsg::McpStartupUpdate(update) => { - let (status, error) = match update.status { + let (status, error, failure_reason) = match update.status { codex_protocol::protocol::McpStartupStatus::Starting => { - (McpServerStartupState::Starting, None) + (McpServerStartupState::Starting, None, None) } codex_protocol::protocol::McpStartupStatus::Ready => { - (McpServerStartupState::Ready, None) - } - codex_protocol::protocol::McpStartupStatus::Failed { error } => { - (McpServerStartupState::Failed, Some(error)) + (McpServerStartupState::Ready, None, None) } + codex_protocol::protocol::McpStartupStatus::Failed { error, reason } => ( + McpServerStartupState::Failed, + Some(error), + reason.map(Into::into), + ), codex_protocol::protocol::McpStartupStatus::Cancelled => { - (McpServerStartupState::Cancelled, None) + (McpServerStartupState::Cancelled, None, None) } }; let notification = McpServerStatusUpdatedNotification { @@ -219,6 +221,7 @@ pub(crate) async fn apply_bespoke_event_handling( name: update.server, status, error, + failure_reason, }; outgoing .send_server_notification(ServerNotification::McpServerStatusUpdated(notification)) diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 93dcb47ed..683a2bed1 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -945,6 +945,7 @@ async fn thread_start_emits_mcp_server_status_updated_notifications() -> Result< name: "optional_broken".to_string(), status: McpServerStartupState::Starting, error: None, + failure_reason: None, } ); @@ -977,6 +978,7 @@ async fn thread_start_emits_mcp_server_status_updated_notifications() -> Result< assert_eq!(failed.thread_id, Some(start_response.thread.id)); assert_eq!(failed.name, "optional_broken"); assert_eq!(failed.status, McpServerStartupState::Failed); + assert_eq!(failed.failure_reason, None); assert!( failed .error diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 586f9c184..fd0b1a0e4 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -573,7 +573,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> .map(|(name, cfg)| { let auth_status = auth_statuses .get(name.as_str()) - .map(|entry| entry.auth_status) + .map(|entry| McpAuthStatus::from(entry.auth_state)) .unwrap_or(McpAuthStatus::Unsupported); let transport = match &cfg.transport { McpServerTransportConfig::Stdio { @@ -657,7 +657,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> let status = format_mcp_status(cfg); let auth_status = auth_statuses .get(name.as_str()) - .map(|entry| entry.auth_status) + .map(|entry| McpAuthStatus::from(entry.auth_state)) .unwrap_or(McpAuthStatus::Unsupported) .to_string(); stdio_rows.push([ @@ -678,7 +678,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> let status = format_mcp_status(cfg); let auth_status = auth_statuses .get(name.as_str()) - .map(|entry| entry.auth_status) + .map(|entry| McpAuthStatus::from(entry.auth_state)) .unwrap_or(McpAuthStatus::Unsupported) .to_string(); let bearer_token_display = diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 0e8154955..5902bc960 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -55,9 +55,12 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::McpStartupCompleteEvent; use codex_protocol::protocol::McpStartupFailure; +use codex_protocol::protocol::McpStartupFailureReason; use codex_protocol::protocol::McpStartupStatus; use codex_protocol::protocol::McpStartupUpdateEvent; use codex_rmcp_client::ElicitationResponse; +use codex_rmcp_client::McpAuthState; +use codex_rmcp_client::McpLoginRequirement; use rmcp::model::ElicitationCapability; use rmcp::model::ListResourceTemplatesResult; use rmcp::model::ListResourcesResult; @@ -228,12 +231,16 @@ impl McpConnectionManager { Ok(_) => McpStartupStatus::Ready, Err(StartupOutcomeError::Cancelled) => McpStartupStatus::Cancelled, Err(error) => { + let reason = mcp_startup_failure_reason(auth_entry.as_ref(), error); let error_str = mcp_init_error_display( server_name.as_str(), auth_entry.as_ref(), error, ); - McpStartupStatus::Failed { error: error_str } + McpStartupStatus::Failed { + error: error_str, + reason, + } } }; @@ -266,7 +273,7 @@ impl McpConnectionManager { match outcome { Ok(_) => summary.ready.push(server_name), Err(StartupOutcomeError::Cancelled) => summary.cancelled.push(server_name), - Err(StartupOutcomeError::Failed { error }) => { + Err(StartupOutcomeError::Failed { error, .. }) => { summary.failed.push(McpStartupFailure { server: server_name, error, @@ -909,6 +916,28 @@ async fn emit_update( .await } +fn mcp_startup_failure_reason( + entry: Option<&McpAuthStatusEntry>, + error: &StartupOutcomeError, +) -> Option { + if !error.is_authentication_required() { + return None; + } + + match entry.map(|entry| entry.auth_state) { + Some(McpAuthState::LoggedOut(McpLoginRequirement::Reauthentication)) => { + Some(McpStartupFailureReason::ReauthenticationRequired) + } + Some( + McpAuthState::Unsupported + | McpAuthState::LoggedOut(McpLoginRequirement::Login) + | McpAuthState::BearerToken + | McpAuthState::OAuth, + ) + | None => None, + } +} + fn mcp_init_error_display( server_name: &str, entry: Option<&McpAuthStatusEntry>, @@ -955,20 +984,20 @@ fn mcp_init_error_display( fn startup_outcome_error_message(error: StartupOutcomeError) -> String { match error { StartupOutcomeError::Cancelled => "MCP startup cancelled".to_string(), - StartupOutcomeError::Failed { error } => error, + StartupOutcomeError::Failed { error, .. } => error, } } fn is_mcp_client_auth_required_error(error: &StartupOutcomeError) -> bool { match error { - StartupOutcomeError::Failed { error } => error.contains("Auth required"), + StartupOutcomeError::Failed { error, .. } => error.contains("Auth required"), _ => false, } } fn is_mcp_client_startup_timeout_error(error: &StartupOutcomeError) -> bool { match error { - StartupOutcomeError::Failed { error } => { + StartupOutcomeError::Failed { error, .. } => { error.contains("request timed out") || error.contains("timed out handshaking with MCP server") } diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 05caf0489..e11fcb591 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -25,7 +25,6 @@ use codex_protocol::ToolName; use codex_protocol::mcp::McpServerInfo; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::GranularApprovalConfig; -use codex_protocol::protocol::McpAuthStatus; use codex_rmcp_client::InProcessTransportFactory; use codex_rmcp_client::RmcpClient; use futures::FutureExt; @@ -927,6 +926,7 @@ async fn list_all_tools_uses_shared_codex_apps_cache_when_client_startup_fails() let failed_client = futures::future::ready::>(Err( StartupOutcomeError::Failed { error: "startup failed".to_string(), + is_authentication_required: false, }, )) .boxed() @@ -1236,7 +1236,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { oauth_resource: None, tools: HashMap::new(), }), - auth_status: McpAuthStatus::Unsupported, + auth_state: McpAuthState::Unsupported, }; let err: StartupOutcomeError = anyhow::anyhow!("OAuth is unsupported").into(); @@ -1263,6 +1263,50 @@ fn mcp_init_error_display_prompts_for_login_when_auth_required() { assert_eq!(expected, display); } +#[test] +fn mcp_startup_failure_reason_requires_existing_oauth_and_auth_failure() { + for (auth_state, is_authentication_required, expected) in [ + ( + Some(McpAuthState::LoggedOut( + McpLoginRequirement::Reauthentication, + )), + true, + Some(McpStartupFailureReason::ReauthenticationRequired), + ), + ( + Some(McpAuthState::LoggedOut( + McpLoginRequirement::Reauthentication, + )), + false, + None, + ), + ( + Some(McpAuthState::LoggedOut(McpLoginRequirement::Login)), + true, + None, + ), + (Some(McpAuthState::Unsupported), true, None), + (Some(McpAuthState::BearerToken), true, None), + (Some(McpAuthState::OAuth), true, None), + (None, true, None), + ] { + let entry = auth_state.map(|auth_state| McpAuthStatusEntry { + config: None, + auth_state, + }); + let error = StartupOutcomeError::Failed { + error: "startup failed".to_string(), + is_authentication_required, + }; + + assert_eq!( + mcp_startup_failure_reason(entry.as_ref(), &error), + expected, + "auth_state={auth_state:?}, is_authentication_required={is_authentication_required}" + ); + } +} + #[test] fn mcp_init_error_display_reports_generic_errors() { let server_name = "custom"; @@ -1290,7 +1334,7 @@ fn mcp_init_error_display_reports_generic_errors() { oauth_resource: None, tools: HashMap::new(), }), - auth_status: McpAuthStatus::Unsupported, + auth_state: McpAuthState::Unsupported, }; let err: StartupOutcomeError = anyhow::anyhow!("boom").into(); diff --git a/codex-rs/codex-mcp/src/mcp/auth.rs b/codex-rs/codex-mcp/src/mcp/auth.rs index 111f04163..fcdaefe3a 100644 --- a/codex-rs/codex-mcp/src/mcp/auth.rs +++ b/codex-rs/codex-mcp/src/mcp/auth.rs @@ -9,7 +9,7 @@ use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::HttpClient; use codex_login::CodexAuth; -use codex_protocol::protocol::McpAuthStatus; +use codex_rmcp_client::McpAuthState; use codex_rmcp_client::OAuthProviderError; use codex_rmcp_client::determine_streamable_http_auth_status; use codex_rmcp_client::determine_streamable_http_auth_status_with_http_client; @@ -54,7 +54,7 @@ pub struct ResolvedMcpOAuthScopes { #[derive(Debug, Clone)] pub struct McpAuthStatusEntry { pub config: Option, - pub auth_status: McpAuthStatus, + pub auth_state: McpAuthState, } pub async fn oauth_login_support(transport: &McpServerTransportConfig) -> McpOAuthLoginSupport { @@ -208,7 +208,7 @@ where ) }); async move { - let auth_status = match config.as_ref() { + let auth_state = match config.as_ref() { Some(config) => { match compute_auth_status( &name, @@ -225,16 +225,13 @@ where warn!( "failed to determine auth status for MCP server `{name}`: {error:?}" ); - McpAuthStatus::Unsupported + McpAuthState::Unsupported } } } - None => McpAuthStatus::Unsupported, - }; - let entry = McpAuthStatusEntry { - config, - auth_status, + None => McpAuthState::Unsupported, }; + let entry = McpAuthStatusEntry { config, auth_state }; (name, entry) } }); @@ -249,17 +246,17 @@ async fn compute_auth_status( keyring_backend_kind: AuthKeyringBackendKind, has_runtime_auth: bool, runtime_context: &McpRuntimeContext, -) -> Result { +) -> Result { if !config.enabled { - return Ok(McpAuthStatus::Unsupported); + return Ok(McpAuthState::Unsupported); } if has_runtime_auth { - return Ok(McpAuthStatus::BearerToken); + return Ok(McpAuthState::BearerToken); } match &config.transport { - McpServerTransportConfig::Stdio { .. } => Ok(McpAuthStatus::Unsupported), + McpServerTransportConfig::Stdio { .. } => Ok(McpAuthState::Unsupported), McpServerTransportConfig::StreamableHttp { url, bearer_token_env_var, diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 7532c6466..3f24a276e 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -567,7 +567,7 @@ fn auth_statuses_from_entries( ) -> HashMap { auth_status_entries .iter() - .map(|(name, entry)| (name.clone(), entry.auth_status)) + .map(|(name, entry)| (name.clone(), McpAuthStatus::from(entry.auth_state))) .collect::>() } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 73ad33df7..ddfb3ffdb 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -63,6 +63,7 @@ use rmcp::model::InitializeRequestParams; use rmcp::model::JsonObject; use rmcp::model::ProtocolVersion; use rmcp::model::Tool as RmcpTool; +use rmcp::transport::auth::AuthError; use tokio_util::sync::CancellationToken; use tracing::Instrument; use tracing::instrument; @@ -304,13 +305,39 @@ pub(crate) enum StartupOutcomeError { // We can't store the original error here because anyhow::Error doesn't implement // `Clone`. #[error("MCP startup failed: {error}")] - Failed { error: String }, + Failed { + error: String, + is_authentication_required: bool, + }, +} + +impl StartupOutcomeError { + pub(crate) fn is_authentication_required(&self) -> bool { + match self { + Self::Cancelled => false, + Self::Failed { + is_authentication_required, + .. + } => *is_authentication_required, + } + } } impl From for StartupOutcomeError { fn from(error: anyhow::Error) -> Self { + let is_authentication_required = error.chain().any(|source| { + source + .downcast_ref::() + .is_some_and(|auth_error| { + matches!( + auth_error, + AuthError::AuthorizationRequired | AuthError::TokenExpired + ) + }) + }); Self::Failed { error: error.to_string(), + is_authentication_required, } } } @@ -765,6 +792,17 @@ mod tests { use pretty_assertions::assert_eq; use rmcp::model::JsonObject; use rmcp::model::Meta; + use rmcp::transport::auth::AuthError; + + #[test] + fn startup_outcome_error_identifies_authentication_required() { + let error = anyhow::Error::new(AuthError::AuthorizationRequired) + .context("failed to initialize MCP server"); + + let error = StartupOutcomeError::from(error); + + assert!(error.is_authentication_required()); + } #[test] fn mcp_initialize_advertises_openai_form_only_when_supported() { diff --git a/codex-rs/config/src/mcp_requirements_tests.rs b/codex-rs/config/src/mcp_requirements_tests.rs index b85487356..68b02316c 100644 --- a/codex-rs/config/src/mcp_requirements_tests.rs +++ b/codex-rs/config/src/mcp_requirements_tests.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig { McpServerConfig { + auth: Default::default(), transport: McpServerTransportConfig::Stdio { command: command.to_string(), args: args.iter().map(ToString::to_string).collect(), diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 42b01b16f..5f3e75dd8 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -3621,10 +3621,22 @@ pub struct McpStartupUpdateEvent { pub enum McpStartupStatus { Starting, Ready, - Failed { error: String }, + Failed { + error: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional = nullable)] + reason: Option, + }, Cancelled, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(rename_all = "snake_case")] +pub enum McpStartupFailureReason { + ReauthenticationRequired, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, Default)] pub struct McpStartupCompleteEvent { pub ready: Vec, @@ -5749,6 +5761,7 @@ mod tests { server: "srv".to_string(), status: McpStartupStatus::Failed { error: "boom".to_string(), + reason: Some(McpStartupFailureReason::ReauthenticationRequired), }, }), }; @@ -5758,6 +5771,10 @@ mod tests { assert_eq!(value["msg"]["server"], "srv"); assert_eq!(value["msg"]["status"]["state"], "failed"); assert_eq!(value["msg"]["status"]["error"], "boom"); + assert_eq!( + value["msg"]["status"]["reason"], + "reauthentication_required" + ); Ok(()) } diff --git a/codex-rs/rmcp-client/src/auth_status.rs b/codex-rs/rmcp-client/src/auth_status.rs index 5b8378e67..32c244af8 100644 --- a/codex-rs/rmcp-client/src/auth_status.rs +++ b/codex-rs/rmcp-client/src/auth_status.rs @@ -28,8 +28,33 @@ pub struct StreamableHttpOAuthDiscovery { pub scopes_supported: Option>, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum McpLoginRequirement { + Login, + Reauthentication, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum McpAuthState { + Unsupported, + LoggedOut(McpLoginRequirement), + BearerToken, + OAuth, +} + +impl From for McpAuthStatus { + fn from(value: McpAuthState) -> Self { + match value { + McpAuthState::Unsupported => Self::Unsupported, + McpAuthState::LoggedOut(_) => Self::NotLoggedIn, + McpAuthState::BearerToken => Self::BearerToken, + McpAuthState::OAuth => Self::OAuth, + } + } +} + enum AuthStatusCheck { - Complete(McpAuthStatus), + Complete(McpAuthState), Discover(HeaderMap), } @@ -42,7 +67,7 @@ pub async fn determine_streamable_http_auth_status( env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, keyring_backend_kind: AuthKeyringBackendKind, -) -> Result { +) -> Result { let default_headers = match auth_status_before_discovery( server_name, url, @@ -75,7 +100,7 @@ pub async fn determine_streamable_http_auth_status_with_http_client( store_mode: OAuthCredentialsStoreMode, keyring_backend_kind: AuthKeyringBackendKind, http_client: Arc, -) -> Result { +) -> Result { let default_headers = match auth_status_before_discovery( server_name, url, @@ -110,20 +135,22 @@ fn auth_status_before_discovery( keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { if bearer_token_env_var.is_some() { - return Ok(AuthStatusCheck::Complete(McpAuthStatus::BearerToken)); + return Ok(AuthStatusCheck::Complete(McpAuthState::BearerToken)); } let default_headers = build_default_headers(http_headers, env_http_headers)?; if default_headers.contains_key(AUTHORIZATION) { - return Ok(AuthStatusCheck::Complete(McpAuthStatus::BearerToken)); + return Ok(AuthStatusCheck::Complete(McpAuthState::BearerToken)); } match oauth_token_status(server_name, url, store_mode, keyring_backend_kind)? { StoredOAuthTokenStatus::Usable => { - return Ok(AuthStatusCheck::Complete(McpAuthStatus::OAuth)); + return Ok(AuthStatusCheck::Complete(McpAuthState::OAuth)); } StoredOAuthTokenStatus::AuthorizationRequired => { - return Ok(AuthStatusCheck::Complete(McpAuthStatus::NotLoggedIn)); + return Ok(AuthStatusCheck::Complete(McpAuthState::LoggedOut( + McpLoginRequirement::Reauthentication, + ))); } StoredOAuthTokenStatus::Missing => {} } @@ -135,15 +162,15 @@ fn determine_auth_status_from_discovery( server_name: &str, url: &str, discovery: Result>, -) -> Result { +) -> Result { match discovery { - Ok(Some(_)) => Ok(McpAuthStatus::NotLoggedIn), - Ok(None) => Ok(McpAuthStatus::Unsupported), + Ok(Some(_)) => Ok(McpAuthState::LoggedOut(McpLoginRequirement::Login)), + Ok(None) => Ok(McpAuthState::Unsupported), Err(error) => { debug!( "failed to detect OAuth support for MCP server `{server_name}` at {url}: {error:?}" ); - Ok(McpAuthStatus::Unsupported) + Ok(McpAuthState::Unsupported) } } } @@ -336,7 +363,7 @@ mod tests { .await .expect("status should compute"); - assert_eq!(status, McpAuthStatus::BearerToken); + assert_eq!(status, McpAuthState::BearerToken); } #[tokio::test] @@ -358,7 +385,7 @@ mod tests { .await .expect("status should compute"); - assert_eq!(status, McpAuthStatus::BearerToken); + assert_eq!(status, McpAuthState::BearerToken); } #[tokio::test] diff --git a/codex-rs/rmcp-client/src/lib.rs b/codex-rs/rmcp-client/src/lib.rs index e1041fdaf..5be97a56a 100644 --- a/codex-rs/rmcp-client/src/lib.rs +++ b/codex-rs/rmcp-client/src/lib.rs @@ -12,6 +12,8 @@ mod rmcp_client; mod stdio_server_launcher; mod utils; +pub use auth_status::McpAuthState; +pub use auth_status::McpLoginRequirement; pub use auth_status::StreamableHttpOAuthDiscovery; pub use auth_status::determine_streamable_http_auth_status; pub use auth_status::determine_streamable_http_auth_status_with_http_client; diff --git a/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs b/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs index 866daa67a..bc5a701ed 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs @@ -7,7 +7,8 @@ use std::time::UNIX_EPOCH; use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::Environment; -use codex_rmcp_client::McpAuthStatus; +use codex_rmcp_client::McpAuthState; +use codex_rmcp_client::McpLoginRequirement; use codex_rmcp_client::RmcpClient; use codex_rmcp_client::StoredOAuthTokens; use codex_rmcp_client::WrappedOAuthTokenResponse; @@ -146,6 +147,21 @@ async fn reports_auth_status_for_persisted_credentials() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[ignore = "spawned by reports_auth_status_for_persisted_credentials"] async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> { + let first_login_server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/.well-known/oauth-authorization-server/mcp")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "authorization_endpoint": format!("{}/oauth/authorize", first_login_server.uri()), + "token_endpoint": format!("{}/oauth/token", first_login_server.uri()), + }))) + .expect(1) + .mount(&first_login_server) + .await; + + let status = auth_status(&format!("{}/mcp", first_login_server.uri())).await?; + assert_eq!(status, McpAuthState::LoggedOut(McpLoginRequirement::Login)); + first_login_server.verify().await; + let response = OAuthTokenResponse::new( AccessToken::new(EXPIRED_ACCESS_TOKEN.to_string()), BasicTokenType::Bearer, @@ -166,7 +182,10 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> { )?; let status = auth_status(UNREFRESHABLE_SERVER_URL).await?; - assert_eq!(status, McpAuthStatus::NotLoggedIn); + assert_eq!( + status, + McpAuthState::LoggedOut(McpLoginRequirement::Reauthentication) + ); let response = OAuthTokenResponse::new( AccessToken::new("unexpired-access-token".to_string()), @@ -192,7 +211,7 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> { )?; let status = auth_status(UNEXPIRED_SERVER_URL).await?; - assert_eq!(status, McpAuthStatus::OAuth); + assert_eq!(status, McpAuthState::OAuth); let mut response = OAuthTokenResponse::new( AccessToken::new(EXPIRED_ACCESS_TOKEN.to_string()), @@ -215,11 +234,11 @@ async fn persisted_credentials_auth_status_child() -> anyhow::Result<()> { )?; let status = auth_status(REFRESHABLE_SERVER_URL).await?; - assert_eq!(status, McpAuthStatus::OAuth); + assert_eq!(status, McpAuthState::OAuth); Ok(()) } -async fn auth_status(server_url: &str) -> anyhow::Result { +async fn auth_status(server_url: &str) -> anyhow::Result { determine_streamable_http_auth_status( SERVER_NAME, server_url, diff --git a/codex-rs/tui/src/app/app_server_event_targets.rs b/codex-rs/tui/src/app/app_server_event_targets.rs index 212c45b4b..6c55850e4 100644 --- a/codex-rs/tui/src/app/app_server_event_targets.rs +++ b/codex-rs/tui/src/app/app_server_event_targets.rs @@ -285,6 +285,7 @@ mod tests { name: "sentry".to_string(), status: McpServerStartupState::Failed, error: Some("sentry is not logged in".to_string()), + failure_reason: None, }); let target = server_notification_thread_target(¬ification); @@ -300,6 +301,7 @@ mod tests { name: "sentry".to_string(), status: McpServerStartupState::Failed, error: Some("sentry is not logged in".to_string()), + failure_reason: None, }); let target = server_notification_thread_target(¬ification); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 21aca66bc..29e29541a 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3496,6 +3496,7 @@ async fn primary_thread_ignores_child_mcp_startup_notifications() { name: "sentry".to_string(), status: McpServerStartupState::Failed, error: Some("sentry is not logged in".to_string()), + failure_reason: None, }), ), ) @@ -3567,6 +3568,7 @@ async fn app_scoped_mcp_startup_notifications_do_not_render_in_active_thread() { name: "sentry".to_string(), status: McpServerStartupState::Failed, error: Some("sentry is not logged in".to_string()), + failure_reason: None, }), ), ) @@ -3631,6 +3633,7 @@ async fn active_side_thread_renders_live_mcp_startup_notifications() { status, error: matches!(status, McpServerStartupState::Failed) .then(|| "sentry is not logged in".to_string()), + failure_reason: None, }), ), ) diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index 00b7fcded..8d41f1c05 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -619,6 +619,7 @@ mod tests { name: "sentry".to_string(), status: codex_app_server_protocol::McpServerStartupState::Failed, error: Some("sentry is not logged in".to_string()), + failure_reason: None, }, ); let mut store = ThreadEventStore::new(/*capacity*/ 8); diff --git a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs index 60efbe135..e2992585a 100644 --- a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs +++ b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs @@ -8,6 +8,7 @@ fn notify_mcp_status(chat: &mut ChatWidget, name: &str, status: McpServerStartup name: name.to_string(), status, error: None, + failure_reason: None, }), /*replay_kind*/ None, ); @@ -20,6 +21,7 @@ fn notify_mcp_status_error(chat: &mut ChatWidget, name: &str, error: &str) { name: name.to_string(), status: McpServerStartupState::Failed, error: Some(error.to_string()), + failure_reason: None, }), /*replay_kind*/ None, ); @@ -51,6 +53,7 @@ async fn mcp_startup_ignores_status_for_other_thread() { status, error: matches!(status, McpServerStartupState::Failed) .then(|| "sentry is not logged in".to_string()), + failure_reason: None, }), /*replay_kind*/ None, );