From e88c2cf4d79c9bfe4fd75b8335b866ce35ab15a3 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 6 Apr 2026 15:47:26 -0700 Subject: [PATCH] tui: route device-code auth through app server (#16827) Addresses #7646 Also enables device code auth for remote TUI sessions Problem: TUI onboarding handled device-code login directly rather than using the recently-added app server support for device auth. Also, auth screens kept animating while users needed to copy login details. Solution: Route device-code onboarding through app-server login APIs and make the auth screens static while those copy-oriented flows are visible. --- codex-rs/tui/src/local_chatgpt_auth.rs | 2 + codex-rs/tui/src/onboarding/auth.rs | 170 ++++-- .../onboarding/auth/headless_chatgpt_login.rs | 548 +++++------------- .../tui/src/onboarding/onboarding_screen.rs | 26 +- codex-rs/tui/src/onboarding/welcome.rs | 10 +- 5 files changed, 322 insertions(+), 434 deletions(-) diff --git a/codex-rs/tui/src/local_chatgpt_auth.rs b/codex-rs/tui/src/local_chatgpt_auth.rs index cb8a3a4aa..e1728add6 100644 --- a/codex-rs/tui/src/local_chatgpt_auth.rs +++ b/codex-rs/tui/src/local_chatgpt_auth.rs @@ -1,3 +1,5 @@ +#![cfg(test)] + use std::path::Path; use codex_app_server_protocol::AuthMode; diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index a5915618c..592fbd975 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -8,8 +8,6 @@ use codex_app_server_protocol::CancelLoginAccountParams; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::LoginAccountParams; use codex_app_server_protocol::LoginAccountResponse; -use codex_login::AuthCredentialsStoreMode; -use codex_login::DeviceCode; use codex_login::read_openai_api_key_from_env; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -33,6 +31,7 @@ use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; use codex_protocol::config_types::ForcedLoginMethod; +use std::cell::Cell; use std::sync::Arc; use std::sync::RwLock; use uuid::Uuid; @@ -77,8 +76,6 @@ pub(crate) fn mark_url_hyperlink(buf: &mut Buffer, area: Rect, url: &str) { } } } -use std::path::PathBuf; -use tokio::sync::Notify; use super::onboarding_screen::StepState; @@ -108,6 +105,20 @@ fn onboarding_request_id() -> codex_app_server_protocol::RequestId { codex_app_server_protocol::RequestId::String(Uuid::new_v4().to_string()) } +pub(super) async fn cancel_login_attempt( + request_handle: &AppServerRequestHandle, + login_id: String, +) { + let _ = request_handle + .request_typed::( + ClientRequest::CancelLoginAccount { + request_id: onboarding_request_id(), + params: CancelLoginAccountParams { login_id }, + }, + ) + .await; +} + #[derive(Clone, Default)] pub(crate) struct ApiKeyInputState { value: String, @@ -123,8 +134,49 @@ pub(crate) struct ContinueInBrowserState { #[derive(Clone)] pub(crate) struct ContinueWithDeviceCodeState { - device_code: Option, - cancel: Option>, + request_id: String, + login_id: Option, + verification_url: Option, + user_code: Option, +} + +impl ContinueWithDeviceCodeState { + pub(crate) fn pending(request_id: String) -> Self { + Self { + request_id, + login_id: None, + verification_url: None, + user_code: None, + } + } + + pub(crate) fn ready( + request_id: String, + login_id: String, + verification_url: String, + user_code: String, + ) -> Self { + Self { + request_id, + login_id: Some(login_id), + verification_url: Some(verification_url), + user_code: Some(user_code), + } + } + + pub(crate) fn login_id(&self) -> Option<&str> { + self.login_id.as_deref() + } + + pub(crate) fn is_showing_copyable_auth(&self) -> bool { + self.verification_url + .as_deref() + .is_some_and(|url| !url.is_empty()) + && self + .user_code + .as_deref() + .is_some_and(|user_code| !user_code.is_empty()) + } } impl KeyboardHandler for AuthModeWidget { @@ -181,16 +233,25 @@ pub(crate) struct AuthModeWidget { pub highlighted_mode: SignInOption, pub error: Arc>>, pub sign_in_state: Arc>, - pub codex_home: PathBuf, - pub cli_auth_credentials_store_mode: AuthCredentialsStoreMode, pub login_status: LoginStatus, pub app_server_request_handle: AppServerRequestHandle, - pub forced_chatgpt_workspace_id: Option, pub forced_login_method: Option, pub animations_enabled: bool, + pub animations_suppressed: Cell, } impl AuthModeWidget { + pub(crate) fn set_animations_suppressed(&self, suppressed: bool) { + self.animations_suppressed.set(suppressed); + } + + pub(crate) fn should_suppress_animations(&self) -> bool { + matches!( + &*self.sign_in_state.read().unwrap(), + SignInState::ChatGptContinueInBrowser(_) | SignInState::ChatGptDeviceCode(_) + ) + } + pub(crate) fn cancel_active_attempt(&self) { let mut sign_in_state = self.sign_in_state.write().unwrap(); match &*sign_in_state { @@ -198,19 +259,15 @@ impl AuthModeWidget { let request_handle = self.app_server_request_handle.clone(); let login_id = state.login_id.clone(); tokio::spawn(async move { - let _ = request_handle - .request_typed::( - ClientRequest::CancelLoginAccount { - request_id: onboarding_request_id(), - params: CancelLoginAccountParams { login_id }, - }, - ) - .await; + cancel_login_attempt(&request_handle, login_id).await; }); } SignInState::ChatGptDeviceCode(state) => { - if let Some(cancel) = &state.cancel { - cancel.notify_one(); + if let Some(login_id) = state.login_id().map(str::to_owned) { + let request_handle = self.app_server_request_handle.clone(); + tokio::spawn(async move { + cancel_login_attempt(&request_handle, login_id).await; + }); } } _ => return, @@ -415,7 +472,7 @@ impl AuthModeWidget { fn render_continue_in_browser(&self, area: Rect, buf: &mut Buffer) { let mut spans = vec![" ".into()]; - if self.animations_enabled { + if self.animations_enabled && !self.animations_suppressed.get() { // Schedule a follow-up frame to keep the shimmer animation going. self.request_frame .schedule_frame_in(std::time::Duration::from_millis(100)); @@ -814,6 +871,9 @@ impl AuthModeWidget { let is_matching_login = matches!( &*guard, SignInState::ChatGptContinueInBrowser(state) if state.login_id == login_id + ) || matches!( + &*guard, + SignInState::ChatGptDeviceCode(state) if state.login_id() == Some(login_id.as_str()) ); drop(guard); if !is_matching_login { @@ -901,6 +961,7 @@ mod tests { use codex_arg0::Arg0DispatchPaths; use codex_cloud_requirements::cloud_requirements_loader_for_storage; use codex_core::config::ConfigBuilder; + use codex_login::AuthCredentialsStoreMode; use codex_protocol::protocol::SessionSource; use pretty_assertions::assert_eq; @@ -943,13 +1004,11 @@ mod tests { highlighted_mode: SignInOption::ChatGpt, error: Arc::new(RwLock::new(None)), sign_in_state: Arc::new(RwLock::new(SignInState::PickMode)), - codex_home: codex_home_path.clone(), - cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, login_status: LoginStatus::NotAuthenticated, app_server_request_handle: AppServerRequestHandle::InProcess(client.request_handle()), - forced_chatgpt_workspace_id: None, forced_login_method: Some(ForcedLoginMethod::Chatgpt), animations_enabled: true, + animations_suppressed: std::cell::Cell::new(false), }; (widget, codex_home) } @@ -1023,13 +1082,14 @@ mod tests { #[tokio::test] async fn cancel_active_attempt_notifies_device_code_login() { let (widget, _tmp) = widget_forced_chatgpt().await; - let cancel = Arc::new(Notify::new()); *widget.error.write().unwrap() = Some("still logging in".to_string()); *widget.sign_in_state.write().unwrap() = - SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState { - device_code: None, - cancel: Some(cancel.clone()), - }); + SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState::ready( + "request-1".to_string(), + "login-1".to_string(), + "https://chatgpt.com/device".to_string(), + "ABCD-EFGH".to_string(), + )); widget.cancel_active_attempt(); @@ -1038,11 +1098,6 @@ mod tests { &*widget.sign_in_state.read().unwrap(), SignInState::PickMode )); - assert!( - tokio::time::timeout(std::time::Duration::from_millis(50), cancel.notified()) - .await - .is_ok() - ); } /// Collects all buffer cell symbols that contain the OSC 8 open sequence @@ -1085,6 +1140,55 @@ mod tests { assert_eq!(found, url, "OSC 8 hyperlink should cover the full URL"); } + #[test] + fn auth_widget_suppresses_animations_when_device_code_is_visible() { + let runtime = tokio::runtime::Runtime::new().unwrap(); + let (widget, _tmp) = runtime.block_on(widget_forced_chatgpt()); + *widget.sign_in_state.write().unwrap() = + SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState::ready( + "request-1".to_string(), + "login-1".to_string(), + "https://chatgpt.com/device".to_string(), + "ABCD-EFGH".to_string(), + )); + + assert_eq!(widget.should_suppress_animations(), true); + } + + #[test] + fn auth_widget_suppresses_animations_while_requesting_device_code() { + let runtime = tokio::runtime::Runtime::new().unwrap(); + let (widget, _tmp) = runtime.block_on(widget_forced_chatgpt()); + *widget.sign_in_state.write().unwrap() = SignInState::ChatGptDeviceCode( + ContinueWithDeviceCodeState::pending("request-1".to_string()), + ); + + assert_eq!(widget.should_suppress_animations(), true); + } + + #[tokio::test] + async fn device_code_login_completion_advances_to_success_message() { + let (mut widget, _tmp) = widget_forced_chatgpt().await; + *widget.sign_in_state.write().unwrap() = + SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState::ready( + "request-1".to_string(), + "login-1".to_string(), + "https://chatgpt.com/device".to_string(), + "ABCD-EFGH".to_string(), + )); + + widget.on_account_login_completed(AccountLoginCompletedNotification { + login_id: Some("login-1".to_string()), + success: true, + error: None, + }); + + assert!(matches!( + &*widget.sign_in_state.read().unwrap(), + SignInState::ChatGptSuccessMessage + )); + } + #[test] fn mark_url_hyperlink_wraps_cyan_underlined_cells() { let url = "https://example.com"; diff --git a/codex-rs/tui/src/onboarding/auth/headless_chatgpt_login.rs b/codex-rs/tui/src/onboarding/auth/headless_chatgpt_login.rs index 97709e949..a3e82e6cd 100644 --- a/codex-rs/tui/src/onboarding/auth/headless_chatgpt_login.rs +++ b/codex-rs/tui/src/onboarding/auth/headless_chatgpt_login.rs @@ -1,12 +1,6 @@ -#![allow(dead_code)] - use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::LoginAccountParams; use codex_app_server_protocol::LoginAccountResponse; -use codex_login::CLIENT_ID; -use codex_login::ServerOptions; -use codex_login::complete_device_code_login; -use codex_login::request_device_code; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::prelude::Widget; @@ -14,109 +8,74 @@ use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::widgets::Paragraph; use ratatui::widgets::Wrap; +use uuid::Uuid; -use std::sync::Arc; -use std::sync::RwLock; -use tokio::sync::Notify; - -use crate::local_chatgpt_auth::LocalChatgptAuth; -use crate::local_chatgpt_auth::load_local_chatgpt_auth; use crate::shimmer::shimmer_spans; -use crate::tui::FrameRequester; use super::AuthModeWidget; -use super::ContinueInBrowserState; use super::ContinueWithDeviceCodeState; use super::SignInState; +use super::cancel_login_attempt; use super::mark_url_hyperlink; -use super::maybe_open_auth_url_in_browser; use super::onboarding_request_id; pub(super) fn start_headless_chatgpt_login(widget: &mut AuthModeWidget) { - let mut opts = ServerOptions::new( - widget.codex_home.clone(), - CLIENT_ID.to_string(), - widget.forced_chatgpt_workspace_id.clone(), - widget.cli_auth_credentials_store_mode, - ); - opts.open_browser = false; + let request_id = Uuid::new_v4().to_string(); + *widget.sign_in_state.write().unwrap() = + SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState::pending(request_id.clone())); + widget.request_frame.schedule_frame(); + let request_handle = widget.app_server_request_handle.clone(); let sign_in_state = widget.sign_in_state.clone(); let request_frame = widget.request_frame.clone(); let error = widget.error.clone(); - let request_handle = widget.app_server_request_handle.clone(); - let codex_home = widget.codex_home.clone(); - let cli_auth_credentials_store_mode = widget.cli_auth_credentials_store_mode; - let forced_chatgpt_workspace_id = widget.forced_chatgpt_workspace_id.clone(); - let cancel = begin_device_code_attempt(&sign_in_state, &request_frame); - tokio::spawn(async move { - let device_code = match request_device_code(&opts).await { - Ok(device_code) => device_code, - Err(err) => { - if err.kind() == std::io::ErrorKind::NotFound { - fallback_to_browser_login( - request_handle, - sign_in_state, - request_frame, - error, - cancel, - ) - .await; + match request_handle + .request_typed::(ClientRequest::LoginAccount { + request_id: onboarding_request_id(), + params: LoginAccountParams::ChatgptDeviceCode, + }) + .await + { + Ok(LoginAccountResponse::ChatgptDeviceCode { + login_id, + verification_url, + user_code, + }) => { + let updated = set_device_code_state_for_active_attempt( + &sign_in_state, + &request_frame, + &request_id, + ContinueWithDeviceCodeState::ready( + request_id.clone(), + login_id.clone(), + verification_url, + user_code, + ), + ); + if updated { + *error.write().unwrap() = None; } else { - set_device_code_error_for_active_attempt( - &sign_in_state, - &request_frame, - &error, - &cancel, - err.to_string(), - ); + cancel_login_attempt(&request_handle, login_id).await; } - return; } - }; - - if !set_device_code_state_for_active_attempt( - &sign_in_state, - &request_frame, - &cancel, - SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState { - device_code: Some(device_code.clone()), - cancel: Some(cancel.clone()), - }), - ) { - return; - } - - tokio::select! { - _ = cancel.notified() => {} - result = complete_device_code_login(opts, device_code) => { - match result { - Ok(()) => { - let local_auth = load_local_chatgpt_auth( - &codex_home, - cli_auth_credentials_store_mode, - forced_chatgpt_workspace_id.as_deref(), - ); - handle_chatgpt_auth_tokens_login_result_for_active_attempt( - request_handle, - sign_in_state, - request_frame, - error, - cancel, - local_auth, - ).await; - } - Err(err) => { - set_device_code_error_for_active_attempt( - &sign_in_state, - &request_frame, - &error, - &cancel, - err.to_string(), - ); - } - } + Ok(other) => { + let _updated = set_device_code_error_for_active_attempt( + &sign_in_state, + &request_frame, + &error, + &request_id, + format!("Unexpected account/login/start response: {other:?}"), + ); + } + Err(err) => { + let _updated = set_device_code_error_for_active_attempt( + &sign_in_state, + &request_frame, + &error, + &request_id, + err.to_string(), + ); } } }); @@ -128,15 +87,14 @@ pub(super) fn render_device_code_login( buf: &mut Buffer, state: &ContinueWithDeviceCodeState, ) { - let banner = if state.device_code.is_some() { + let banner = if state.is_showing_copyable_auth() { "Finish signing in via your browser" } else { "Preparing device code login" }; let mut spans = vec![" ".into()]; - if widget.animations_enabled { - // Schedule a follow-up frame to keep the shimmer animation going. + if widget.animations_enabled && !widget.animations_suppressed.get() { widget .request_frame .schedule_frame_in(std::time::Duration::from_millis(100)); @@ -147,13 +105,14 @@ pub(super) fn render_device_code_login( let mut lines = vec![spans.into(), "".into()]; - // Capture the verification URL for OSC 8 hyperlink marking after render. - let verification_url = if let Some(device_code) = &state.device_code { + let verification_url = if let (Some(verification_url), Some(user_code)) = + (&state.verification_url, &state.user_code) + { lines.push(" 1. Open this link in your browser and sign in".into()); lines.push("".into()); lines.push(Line::from(vec![ " ".into(), - device_code.verification_url.as_str().cyan().underlined(), + verification_url.as_str().cyan().underlined(), ])); lines.push("".into()); lines.push( @@ -162,7 +121,7 @@ pub(super) fn render_device_code_login( lines.push("".into()); lines.push(Line::from(vec![ " ".into(), - device_code.user_code.as_str().cyan().bold(), + user_code.as_str().cyan().bold(), ])); lines.push("".into()); lines.push( @@ -171,7 +130,7 @@ pub(super) fn render_device_code_login( .into(), ); lines.push("".into()); - Some(device_code.verification_url.clone()) + Some(verification_url.clone()) } else { lines.push(" Requesting a one-time code...".dim().into()); lines.push("".into()); @@ -183,286 +142,139 @@ pub(super) fn render_device_code_login( .wrap(Wrap { trim: false }) .render(area, buf); - // Wrap cyan+underlined URL cells with OSC 8 so the terminal treats - // the entire region as a single clickable hyperlink. if let Some(url) = &verification_url { mark_url_hyperlink(buf, area, url); } } -fn device_code_attempt_matches(state: &SignInState, cancel: &Arc) -> bool { +fn device_code_attempt_matches(state: &SignInState, request_id: &str) -> bool { matches!( state, - SignInState::ChatGptDeviceCode(state) - if state - .cancel - .as_ref() - .is_some_and(|existing| Arc::ptr_eq(existing, cancel)) + SignInState::ChatGptDeviceCode(state) if state.request_id == request_id ) } -fn begin_device_code_attempt( - sign_in_state: &Arc>, - request_frame: &FrameRequester, -) -> Arc { - let cancel = Arc::new(Notify::new()); - *sign_in_state.write().unwrap() = SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState { - device_code: None, - cancel: Some(cancel.clone()), - }); - request_frame.schedule_frame(); - cancel -} - fn set_device_code_state_for_active_attempt( - sign_in_state: &Arc>, - request_frame: &FrameRequester, - cancel: &Arc, - next_state: SignInState, + sign_in_state: &std::sync::Arc>, + request_frame: &crate::tui::FrameRequester, + request_id: &str, + next_state: ContinueWithDeviceCodeState, ) -> bool { let mut guard = sign_in_state.write().unwrap(); - if !device_code_attempt_matches(&guard, cancel) { + if !device_code_attempt_matches(&guard, request_id) { return false; } - *guard = next_state; - drop(guard); - request_frame.schedule_frame(); - true -} - -fn set_device_code_success_message_for_active_attempt( - sign_in_state: &Arc>, - request_frame: &FrameRequester, - cancel: &Arc, -) -> bool { - let mut guard = sign_in_state.write().unwrap(); - if !device_code_attempt_matches(&guard, cancel) { - return false; - } - - *guard = SignInState::ChatGptSuccessMessage; + *guard = SignInState::ChatGptDeviceCode(next_state); drop(guard); request_frame.schedule_frame(); true } fn set_device_code_error_for_active_attempt( - sign_in_state: &Arc>, - request_frame: &FrameRequester, - error: &Arc>>, - cancel: &Arc, + sign_in_state: &std::sync::Arc>, + request_frame: &crate::tui::FrameRequester, + error: &std::sync::Arc>>, + request_id: &str, message: String, ) -> bool { - if !set_device_code_state_for_active_attempt( - sign_in_state, - request_frame, - cancel, - SignInState::PickMode, - ) { + let mut guard = sign_in_state.write().unwrap(); + if !device_code_attempt_matches(&guard, request_id) { return false; } + + *guard = SignInState::PickMode; + drop(guard); *error.write().unwrap() = Some(message); request_frame.schedule_frame(); true } -async fn fallback_to_browser_login( - request_handle: codex_app_server_client::AppServerRequestHandle, - sign_in_state: Arc>, - request_frame: FrameRequester, - error: Arc>>, - cancel: Arc, -) { - let should_fallback = { - let guard = sign_in_state.read().unwrap(); - device_code_attempt_matches(&guard, &cancel) - }; - if !should_fallback { - return; - } - - match request_handle - .request_typed::(ClientRequest::LoginAccount { - request_id: onboarding_request_id(), - params: LoginAccountParams::Chatgpt, - }) - .await - { - Ok(LoginAccountResponse::Chatgpt { login_id, auth_url }) => { - maybe_open_auth_url_in_browser(&request_handle, &auth_url); - *error.write().unwrap() = None; - let _updated = set_device_code_state_for_active_attempt( - &sign_in_state, - &request_frame, - &cancel, - SignInState::ChatGptContinueInBrowser(ContinueInBrowserState { - login_id, - auth_url, - }), - ); - } - Ok(other) => { - set_device_code_error_for_active_attempt( - &sign_in_state, - &request_frame, - &error, - &cancel, - format!("Unexpected account/login/start response: {other:?}"), - ); - } - Err(err) => { - set_device_code_error_for_active_attempt( - &sign_in_state, - &request_frame, - &error, - &cancel, - err.to_string(), - ); - } - } -} - -async fn handle_chatgpt_auth_tokens_login_result_for_active_attempt( - request_handle: codex_app_server_client::AppServerRequestHandle, - sign_in_state: Arc>, - request_frame: FrameRequester, - error: Arc>>, - cancel: Arc, - local_auth: Result, -) { - let local_auth = match local_auth { - Ok(local_auth) => local_auth, - Err(err) => { - set_device_code_error_for_active_attempt( - &sign_in_state, - &request_frame, - &error, - &cancel, - err, - ); - return; - } - }; - - let result = request_handle - .request_typed::(ClientRequest::LoginAccount { - request_id: onboarding_request_id(), - params: LoginAccountParams::ChatgptAuthTokens { - access_token: local_auth.access_token, - chatgpt_account_id: local_auth.chatgpt_account_id, - chatgpt_plan_type: local_auth.chatgpt_plan_type, - }, - }) - .await; - apply_chatgpt_auth_tokens_login_response_for_active_attempt( - &sign_in_state, - &request_frame, - &error, - &cancel, - result.map_err(|err| err.to_string()), - ); -} - -fn apply_chatgpt_auth_tokens_login_response_for_active_attempt( - sign_in_state: &Arc>, - request_frame: &FrameRequester, - error: &Arc>>, - cancel: &Arc, - result: Result, -) { - match result { - Ok(LoginAccountResponse::ChatgptAuthTokens {}) => { - *error.write().unwrap() = None; - let _updated = set_device_code_success_message_for_active_attempt( - sign_in_state, - request_frame, - cancel, - ); - } - Ok(other) => { - set_device_code_error_for_active_attempt( - sign_in_state, - request_frame, - error, - cancel, - format!("Unexpected account/login/start response: {other:?}"), - ); - } - Err(err) => { - set_device_code_error_for_active_attempt( - sign_in_state, - request_frame, - error, - cancel, - err, - ); - } - } -} - #[cfg(test)] mod tests { use super::*; - use pretty_assertions::assert_eq; + use std::sync::Arc; + use std::sync::RwLock; - fn device_code_sign_in_state(cancel: Arc) -> Arc> { + fn pending_device_code_state(request_id: &str) -> Arc> { Arc::new(RwLock::new(SignInState::ChatGptDeviceCode( - ContinueWithDeviceCodeState { - device_code: None, - cancel: Some(cancel), - }, + ContinueWithDeviceCodeState::pending(request_id.to_string()), ))) } #[test] - fn device_code_attempt_matches_only_for_matching_cancel() { - let cancel = Arc::new(Notify::new()); - let state = SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState { - device_code: None, - cancel: Some(cancel.clone()), - }); - - assert_eq!(device_code_attempt_matches(&state, &cancel), true); - assert_eq!( - device_code_attempt_matches(&state, &Arc::new(Notify::new())), - false - ); - assert_eq!( - device_code_attempt_matches(&SignInState::PickMode, &cancel), - false - ); - } - - #[test] - fn begin_device_code_attempt_sets_state() { - let sign_in_state = Arc::new(RwLock::new(SignInState::PickMode)); - let request_frame = FrameRequester::test_dummy(); - - let cancel = begin_device_code_attempt(&sign_in_state, &request_frame); - let guard = sign_in_state.read().unwrap(); - - let state: &SignInState = &guard; - assert_eq!(device_code_attempt_matches(state, &cancel), true); - assert!(matches!( - state, - SignInState::ChatGptDeviceCode(state) if state.device_code.is_none() + fn device_code_attempt_matches_only_for_matching_request_id() { + let state = SignInState::ChatGptDeviceCode(ContinueWithDeviceCodeState::pending( + "request-1".to_string(), )); + + assert_eq!(device_code_attempt_matches(&state, "request-1"), true); + assert_eq!(device_code_attempt_matches(&state, "request-2"), false); + assert_eq!( + device_code_attempt_matches(&SignInState::PickMode, "request-1"), + false + ); } #[test] fn set_device_code_state_for_active_attempt_updates_only_when_active() { - let request_frame = FrameRequester::test_dummy(); - let cancel = Arc::new(Notify::new()); - let sign_in_state = device_code_sign_in_state(cancel.clone()); + let request_frame = crate::tui::FrameRequester::test_dummy(); + let sign_in_state = pending_device_code_state("request-1"); assert_eq!( set_device_code_state_for_active_attempt( &sign_in_state, &request_frame, - &cancel, - SignInState::PickMode, + "request-1", + ContinueWithDeviceCodeState::ready( + "request-1".to_string(), + "login-1".to_string(), + "https://example.com/device".to_string(), + "ABCD-EFGH".to_string(), + ), + ), + true + ); + assert!(matches!( + &*sign_in_state.read().unwrap(), + SignInState::ChatGptDeviceCode(state) if state.login_id() == Some("login-1") + )); + + let sign_in_state = pending_device_code_state("request-2"); + assert_eq!( + set_device_code_state_for_active_attempt( + &sign_in_state, + &request_frame, + "request-1", + ContinueWithDeviceCodeState::ready( + "request-1".to_string(), + "login-1".to_string(), + "https://example.com/device".to_string(), + "ABCD-EFGH".to_string(), + ), + ), + false + ); + assert!(matches!( + &*sign_in_state.read().unwrap(), + SignInState::ChatGptDeviceCode(state) if state.login_id.is_none() + )); + } + + #[test] + fn set_device_code_error_for_active_attempt_updates_only_when_active() { + let request_frame = crate::tui::FrameRequester::test_dummy(); + let error = Arc::new(RwLock::new(None)); + let sign_in_state = pending_device_code_state("request-1"); + + assert_eq!( + set_device_code_error_for_active_attempt( + &sign_in_state, + &request_frame, + &error, + "request-1", + "device code unavailable".to_string(), ), true ); @@ -470,79 +282,27 @@ mod tests { &*sign_in_state.read().unwrap(), SignInState::PickMode )); - - let sign_in_state = device_code_sign_in_state(Arc::new(Notify::new())); assert_eq!( - set_device_code_state_for_active_attempt( - &sign_in_state, - &request_frame, - &cancel, - SignInState::PickMode, - ), - false + error.read().unwrap().as_deref(), + Some("device code unavailable") ); - assert!(matches!( - &*sign_in_state.read().unwrap(), - SignInState::ChatGptDeviceCode(_) - )); - } - #[test] - fn set_device_code_success_message_for_active_attempt_updates_only_when_active() { - let request_frame = FrameRequester::test_dummy(); - let cancel = Arc::new(Notify::new()); - let sign_in_state = device_code_sign_in_state(cancel.clone()); - assert_eq!( - set_device_code_success_message_for_active_attempt( - &sign_in_state, - &request_frame, - &cancel, - ), - true - ); - assert!(matches!( - &*sign_in_state.read().unwrap(), - SignInState::ChatGptSuccessMessage - )); - - let sign_in_state = device_code_sign_in_state(Arc::new(Notify::new())); - assert_eq!( - set_device_code_success_message_for_active_attempt( - &sign_in_state, - &request_frame, - &cancel, - ), - false - ); - assert!(matches!( - &*sign_in_state.read().unwrap(), - SignInState::ChatGptDeviceCode(_) - )); - } - - #[test] - fn chatgpt_auth_tokens_success_sets_success_message_without_login_id() { - let sign_in_state = device_code_sign_in_state(Arc::new(Notify::new())); - let request_frame = FrameRequester::test_dummy(); let error = Arc::new(RwLock::new(None)); - let cancel = match &*sign_in_state.read().unwrap() { - SignInState::ChatGptDeviceCode(state) => { - state.cancel.as_ref().expect("cancel handle").clone() - } - _ => panic!("expected device-code state"), - }; - - apply_chatgpt_auth_tokens_login_response_for_active_attempt( - &sign_in_state, - &request_frame, - &error, - &cancel, - Ok(LoginAccountResponse::ChatgptAuthTokens {}), + let sign_in_state = pending_device_code_state("request-2"); + assert_eq!( + set_device_code_error_for_active_attempt( + &sign_in_state, + &request_frame, + &error, + "request-1", + "device code unavailable".to_string(), + ), + false ); - assert!(matches!( &*sign_in_state.read().unwrap(), - SignInState::ChatGptSuccessMessage + SignInState::ChatGptDeviceCode(_) )); + assert_eq!(*error.read().unwrap(), None); } } diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index ec8e42894..be2e41bc8 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -86,10 +86,8 @@ impl OnboardingScreen { config, } = args; let cwd = config.cwd.to_path_buf(); - let forced_chatgpt_workspace_id = config.forced_chatgpt_workspace_id.clone(); - let forced_login_method = config.forced_login_method; let codex_home = config.codex_home.clone(); - let cli_auth_credentials_store_mode = config.cli_auth_credentials_store_mode; + let forced_login_method = config.forced_login_method; let mut steps: Vec = Vec::new(); steps.push(Step::Welcome(WelcomeWidget::new( !matches!(login_status, LoginStatus::NotAuthenticated), @@ -107,13 +105,11 @@ impl OnboardingScreen { highlighted_mode, error: Arc::new(RwLock::new(None)), sign_in_state: Arc::new(RwLock::new(SignInState::PickMode)), - codex_home: codex_home.clone(), - cli_auth_credentials_store_mode, login_status, app_server_request_handle, - forced_chatgpt_workspace_id, forced_login_method, animations_enabled: config.animations, + animations_suppressed: std::cell::Cell::new(false), })); } else { tracing::warn!("skipping onboarding login step without app-server request handle"); @@ -175,6 +171,15 @@ impl OnboardingScreen { out } + fn should_suppress_animations(&self) -> bool { + // Freeze the whole onboarding screen when auth is showing copyable login + // material so terminal selection is not interrupted by redraws. + self.current_steps().into_iter().any(|step| match step { + Step::Auth(widget) => widget.should_suppress_animations(), + Step::Welcome(_) | Step::TrustDirectory(_) => false, + }) + } + fn is_auth_in_progress(&self) -> bool { self.steps.iter().any(|step| { matches!(step, Step::Auth(_)) && matches!(step.get_step_state(), StepState::InProgress) @@ -323,6 +328,15 @@ impl KeyboardHandler for OnboardingScreen { impl WidgetRef for &OnboardingScreen { fn render_ref(&self, area: Rect, buf: &mut Buffer) { + let suppress_animations = self.should_suppress_animations(); + for step in self.current_steps() { + match step { + Step::Welcome(widget) => widget.set_animations_suppressed(suppress_animations), + Step::Auth(widget) => widget.set_animations_suppressed(suppress_animations), + Step::TrustDirectory(_) => {} + } + } + Clear.render(area, buf); // Render steps top-to-bottom, measuring each step's height dynamically. let mut y = area.y; diff --git a/codex-rs/tui/src/onboarding/welcome.rs b/codex-rs/tui/src/onboarding/welcome.rs index fa668caab..acb12abaa 100644 --- a/codex-rs/tui/src/onboarding/welcome.rs +++ b/codex-rs/tui/src/onboarding/welcome.rs @@ -27,6 +27,7 @@ pub(crate) struct WelcomeWidget { pub is_logged_in: bool, animation: AsciiAnimation, animations_enabled: bool, + animations_suppressed: Cell, layout_area: Cell>, } @@ -55,6 +56,7 @@ impl WelcomeWidget { is_logged_in, animation: AsciiAnimation::new(request_frame), animations_enabled, + animations_suppressed: Cell::new(false), layout_area: Cell::new(None), } } @@ -62,18 +64,23 @@ impl WelcomeWidget { pub(crate) fn update_layout_area(&self, area: Rect) { self.layout_area.set(Some(area)); } + + pub(crate) fn set_animations_suppressed(&self, suppressed: bool) { + self.animations_suppressed.set(suppressed); + } } impl WidgetRef for &WelcomeWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { Clear.render(area, buf); - if self.animations_enabled { + if self.animations_enabled && !self.animations_suppressed.get() { self.animation.schedule_next_frame(); } let layout_area = self.layout_area.get().unwrap_or(area); // Skip the animation entirely when the viewport is too small so we don't clip frames. let show_animation = self.animations_enabled + && !self.animations_suppressed.get() && layout_area.height >= MIN_ANIMATION_HEIGHT && layout_area.width >= MIN_ANIMATION_WIDTH; @@ -167,6 +174,7 @@ mod tests { /*variant_idx*/ 0, ), animations_enabled: true, + animations_suppressed: Cell::new(false), layout_area: Cell::new(None), };