diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index ee7dde1fe..37819203f 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -57,6 +57,7 @@ use codex_rmcp_client::LocalStdioServerLauncher; use codex_rmcp_client::RmcpClient; use codex_rmcp_client::StdioServerLauncher; use codex_rmcp_client::ToolWithConnectorId; +use codex_rmcp_client::is_authentication_required_error; use futures::future::BoxFuture; use futures::future::FutureExt; use futures::future::Shared; @@ -67,7 +68,6 @@ 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::time::Instant as TokioInstant; use tokio_util::sync::CancellationToken; use tracing::Instrument; @@ -552,16 +552,7 @@ impl StartupOutcomeError { 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 - ) - }) - }); + let is_authentication_required = is_authentication_required_error(&error); Self::Failed { error: error.to_string(), is_authentication_required, diff --git a/codex-rs/rmcp-client/src/lib.rs b/codex-rs/rmcp-client/src/lib.rs index 5be97a56a..e57eb3686 100644 --- a/codex-rs/rmcp-client/src/lib.rs +++ b/codex-rs/rmcp-client/src/lib.rs @@ -9,6 +9,7 @@ mod oauth_http_client; mod perform_oauth_login; mod program_resolver; mod rmcp_client; +mod startup_error; mod stdio_server_launcher; mod utils; @@ -40,6 +41,7 @@ pub use rmcp_client::ListToolsWithConnectorIdResult; pub use rmcp_client::RmcpClient; pub use rmcp_client::SendElicitation; pub use rmcp_client::ToolWithConnectorId; +pub use startup_error::is_authentication_required_error; pub use stdio_server_launcher::ExecutorStdioServerLauncher; pub use stdio_server_launcher::LocalStdioServerLauncher; pub use stdio_server_launcher::StdioServerLauncher; diff --git a/codex-rs/rmcp-client/src/startup_error.rs b/codex-rs/rmcp-client/src/startup_error.rs new file mode 100644 index 000000000..c74f637e3 --- /dev/null +++ b/codex-rs/rmcp-client/src/startup_error.rs @@ -0,0 +1,46 @@ +use anyhow::Error; +use rmcp::service::ClientInitializeError; +use rmcp::transport::auth::AuthError; +use rmcp::transport::streamable_http_client::StreamableHttpError; + +use crate::http_client_adapter::StreamableHttpClientAdapterError; + +/// Returns whether an RMCP client error indicates that authentication is required. +/// +/// This does not distinguish first-time login from reauthentication. +/// Streamable HTTP initialization errors are stored inside RMCP's dynamic +/// transport error, which is not part of the standard error source chain. +pub fn is_authentication_required_error(error: &Error) -> bool { + error.chain().any(|source| { + source + .downcast_ref::() + .is_some_and(auth_error_requires_authentication) + || source + .downcast_ref::() + .is_some_and(client_initialize_error_requires_authentication) + }) +} + +fn client_initialize_error_requires_authentication(error: &ClientInitializeError) -> bool { + let ClientInitializeError::TransportError { error, .. } = error else { + return false; + }; + + error + .error + .downcast_ref::>() + .is_some_and(|error| { + matches!( + error, + StreamableHttpError::Auth(auth_error) + if auth_error_requires_authentication(auth_error) + ) + }) +} + +fn auth_error_requires_authentication(error: &AuthError) -> bool { + matches!( + error, + AuthError::AuthorizationRequired | AuthError::TokenExpired + ) +} 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 bc5a701ed..32f90bc41 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs @@ -13,6 +13,7 @@ use codex_rmcp_client::RmcpClient; use codex_rmcp_client::StoredOAuthTokens; use codex_rmcp_client::WrappedOAuthTokenResponse; use codex_rmcp_client::determine_streamable_http_auth_status; +use codex_rmcp_client::is_authentication_required_error; use codex_rmcp_client::save_oauth_tokens; use oauth2::AccessToken; use oauth2::RefreshToken; @@ -144,6 +145,40 @@ async fn reports_auth_status_for_persisted_credentials() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn identifies_expired_unrefreshable_token_startup_error() -> anyhow::Result<()> { + let 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", server.uri()), + "token_endpoint": format!("{}/oauth/token", server.uri()), + }))) + .expect(1) + .mount(&server) + .await; + + let codex_home = TempDir::new()?; + let status = Command::new(std::env::current_exe()?) + .args([ + "expired_unrefreshable_startup_child", + "--exact", + "--ignored", + "--nocapture", + ]) + .env("CODEX_HOME", codex_home.path()) + .env(CHILD_SERVER_URL_ENV, format!("{}/mcp", server.uri())) + .status() + .await?; + + assert!( + status.success(), + "expired OAuth startup child failed: {status}" + ); + server.verify().await; + Ok(()) +} + #[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<()> { @@ -298,3 +333,46 @@ async fn oauth_startup_child() -> anyhow::Result<()> { initialize_client(&client).await?; Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +#[ignore = "spawned by identifies_expired_unrefreshable_token_startup_error"] +async fn expired_unrefreshable_startup_child() -> anyhow::Result<()> { + let server_url = std::env::var(CHILD_SERVER_URL_ENV)?; + let response = OAuthTokenResponse::new( + AccessToken::new(EXPIRED_ACCESS_TOKEN.to_string()), + BasicTokenType::Bearer, + VendorExtraTokenFields::default(), + ); + let tokens = StoredOAuthTokens { + server_name: SERVER_NAME.to_string(), + url: server_url.clone(), + client_id: "test-client-id".to_string(), + token_response: WrappedOAuthTokenResponse(response), + expires_at: Some(0), + }; + save_oauth_tokens( + SERVER_NAME, + &tokens, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + )?; + + let client = RmcpClient::new_streamable_http_client( + SERVER_NAME, + &server_url, + /*bearer_token*/ None, + /*http_headers*/ None, + /*env_http_headers*/ None, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + Environment::default_for_tests().get_http_client(), + /*auth_provider*/ None, + ) + .await?; + + let error = initialize_client(&client) + .await + .expect_err("expired token without a refresh token should fail startup"); + assert!(is_authentication_required_error(&error)); + Ok(()) +}