From c7c2b3cf8d1b09128b4aface2895aa69363818d5 Mon Sep 17 00:00:00 2001 From: blevy-oai Date: Tue, 27 Jan 2026 13:22:54 -0800 Subject: [PATCH] Show OAuth error descriptions in callback responses (#9654) ### Motivation - The local OAuth callback server returned a generic "Invalid OAuth callback" on failures even when the query contained an `error_description`, making it hard to debug OAuth failures. ### Description - Update `codex-rs/rmcp-client/src/perform_oauth_login.rs` to surface `error_description` values from the callback query in the HTTP response. - Introduce a `CallbackOutcome` enum and change `parse_oauth_callback` to return it, parsing `code`, `state`, and `error_description` from the query string. - Change `spawn_callback_server` to match on `CallbackOutcome` and return `OAuth error: ` with a 400 status when `error_description` is present, while preserving the existing success and invalid flows. - Use inline formatting for the error response string. ### Testing - Ran `just fmt` in the `codex-rs` workspace to format changes successfully. - Ran `cargo test -p codex-rmcp-client` and all tests passed. ------ [Codex Task](https://chatgpt.com/codex/tasks/task_i_6971aadc68d0832e93159efea8cd48a9) --- .../rmcp-client/src/perform_oauth_login.rs | 75 +++++++++++++------ 1 file changed, 53 insertions(+), 22 deletions(-) diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index 64cf979ec..09b746837 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -103,21 +103,32 @@ fn spawn_callback_server(server: Arc, tx: oneshot::Sender<(String, Strin tokio::task::spawn_blocking(move || { while let Ok(request) = server.recv() { let path = request.url().to_string(); - if let Some(OauthCallbackResult { code, state }) = parse_oauth_callback(&path) { - let response = - Response::from_string("Authentication complete. You may close this window."); - if let Err(err) = request.respond(response) { - eprintln!("Failed to respond to OAuth callback: {err}"); + match parse_oauth_callback(&path) { + CallbackOutcome::Success(OauthCallbackResult { code, state }) => { + let response = Response::from_string( + "Authentication complete. You may close this window.", + ); + if let Err(err) = request.respond(response) { + eprintln!("Failed to respond to OAuth callback: {err}"); + } + if let Err(err) = tx.send((code, state)) { + eprintln!("Failed to send OAuth callback: {err:?}"); + } + break; } - if let Err(err) = tx.send((code, state)) { - eprintln!("Failed to send OAuth callback: {err:?}"); + CallbackOutcome::Error(description) => { + let response = Response::from_string(format!("OAuth error: {description}")) + .with_status_code(400); + if let Err(err) = request.respond(response) { + eprintln!("Failed to respond to OAuth callback: {err}"); + } } - break; - } else { - let response = - Response::from_string("Invalid OAuth callback").with_status_code(400); - if let Err(err) = request.respond(response) { - eprintln!("Failed to respond to OAuth callback: {err}"); + CallbackOutcome::Invalid => { + let response = + Response::from_string("Invalid OAuth callback").with_status_code(400); + if let Err(err) = request.respond(response) { + eprintln!("Failed to respond to OAuth callback: {err}"); + } } } } @@ -129,29 +140,49 @@ struct OauthCallbackResult { state: String, } -fn parse_oauth_callback(path: &str) -> Option { - let (route, query) = path.split_once('?')?; +enum CallbackOutcome { + Success(OauthCallbackResult), + Error(String), + Invalid, +} + +fn parse_oauth_callback(path: &str) -> CallbackOutcome { + let Some((route, query)) = path.split_once('?') else { + return CallbackOutcome::Invalid; + }; if route != "/callback" { - return None; + return CallbackOutcome::Invalid; } let mut code = None; let mut state = None; + let mut error_description = None; for pair in query.split('&') { - let (key, value) = pair.split_once('=')?; - let decoded = decode(value).ok()?.into_owned(); + let Some((key, value)) = pair.split_once('=') else { + continue; + }; + let Ok(decoded) = decode(value) else { + continue; + }; + let decoded = decoded.into_owned(); match key { "code" => code = Some(decoded), "state" => state = Some(decoded), + "error_description" => error_description = Some(decoded), _ => {} } } - Some(OauthCallbackResult { - code: code?, - state: state?, - }) + if let (Some(code), Some(state)) = (code, state) { + return CallbackOutcome::Success(OauthCallbackResult { code, state }); + } + + if let Some(description) = error_description { + return CallbackOutcome::Error(description); + } + + CallbackOutcome::Invalid } pub struct OauthLoginHandle {