mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
felixxia-oai
526f495f3a
## Summary - classify authentication-required RMCP startup failures, including errors nested inside `ClientInitializeError::TransportError` - let `codex-mcp` consume that classification so the existing `reauthenticationRequired` startup failure reason is emitted - add a regression test that performs real startup with an expired persisted OAuth token and no refresh token ## Why Follow-up to #29877. RMCP stores streamable HTTP initialization failures inside a dynamic transport error whose payload is not exposed through the standard Rust error source chain. The original `anyhow::Error::chain()` check therefore missed the nested `AuthError::AuthorizationRequired` seen during real MCP startup and emitted `failureReason: null`. The transport-specific inspection now lives in `codex-rmcp-client`, while `codex-mcp` consumes only the domain-level authentication-required result. This classifier does not distinguish first-time login from reauthentication; the existing auth-state logic remains responsible for that distinction. ## User impact When stored MCP OAuth credentials are expired and cannot be refreshed, app clients now receive `failureReason: "reauthenticationRequired"` on the failed startup update and can show the reconnect action. First-time login and unrelated startup failures remain unchanged. ## Validation - `just test -p codex-rmcp-client --test streamable_http_oauth_startup identifies_expired_unrefreshable_token_startup_error` - `just test -p codex-mcp startup_outcome_error_identifies_authentication_required` - `just test -p codex-mcp mcp_startup_failure_reason_requires_existing_oauth_and_auth_failure` - `cargo build -p codex-cli --bin codex` - local app-server probe emitted `failureReason: "reauthenticationRequired"` - manual end-to-end reconnect flow confirmed - `just fmt`
47 lines
1.6 KiB
Rust
47 lines
1.6 KiB
Rust
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::<AuthError>()
|
|
.is_some_and(auth_error_requires_authentication)
|
|
|| source
|
|
.downcast_ref::<ClientInitializeError>()
|
|
.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::<StreamableHttpError<StreamableHttpClientAdapterError>>()
|
|
.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
|
|
)
|
|
}
|