From dbd545737e3e3527789d87b1c81f0da8cb0b7b82 Mon Sep 17 00:00:00 2001 From: Hunter B Date: Fri, 24 Jul 2026 12:14:26 -0700 Subject: [PATCH 1/2] fix(tui): make provider onboarding navigable and escapable (#4763) Provider onboarding could trap a returning user whose persisted route was xAI OAuth: the Step 3/4 backdrop rendered no list, the picker opened straight into the OAuth modal for the persisted route, Escape popped the whole modal back to that empty backdrop, and Ctrl+C was swallowed. - Open the onboarding picker on the navigable catalog. `new_for_onboarding` focuses the persisted route without jumping to key/OAuth entry, so the provider list is always seen before a secret is asked for. `/setup` and `/provider` keep the existing `new_for_setup` behavior. - Stop intercepting Escape on the picker's behalf. The picker's own per-stage Escape walks key/OAuth entry back to the list, and dismissal from the list still runs the non-mutating `back_from_provider_onboarding` transition via `ProviderPickerDismissed`, preserving missing-key-recovery semantics. - Route Ctrl+C to shutdown from any onboarding state, modal or not. - Complete provider onboarding after an external CLI grant is reused, the same as a submitted key or an applied route. Previously the picker closed on success and returned the user to the provider step they had just satisfied. - Retell the backdrop footer as what actually happens (Enter opens the provider list, Esc goes back, Ctrl+C quits) across all locales that carry the string. Closes #4763 --- crates/tui/locales/en.json | 2 +- crates/tui/locales/es-419.json | 2 +- crates/tui/locales/ja.json | 2 +- crates/tui/locales/ko.json | 2 +- crates/tui/locales/pt-BR.json | 2 +- crates/tui/locales/vi.json | 2 +- crates/tui/locales/zh-Hans.json | 2 +- crates/tui/src/tui/provider_picker.rs | 89 ++++++++++++++++++++- crates/tui/src/tui/ui.rs | 108 +++++++++++++++++++------- crates/tui/src/tui/ui/tests.rs | 96 +++++++++++++++++++++++ crates/tui/src/xai_oauth.rs | 79 +++++++++++++++++++ 11 files changed, 349 insertions(+), 37 deletions(-) diff --git a/crates/tui/locales/en.json b/crates/tui/locales/en.json index a4c706bc99..12fd81cb31 100644 --- a/crates/tui/locales/en.json +++ b/crates/tui/locales/en.json @@ -414,7 +414,7 @@ "OnboardLanguageFooter": "Press 1-9 to choose, or Enter to keep the current setting", "OnboardProviderTitle": "Choose your model provider", "OnboardProviderBlurb": "Pick where your model runs. Hosted providers need a key; local runtimes can continue without one.", - "OnboardProviderFooter": "Press 0-9 to choose, ↑/↓ to move, Enter to continue, Esc to go back.", + "OnboardProviderFooter": "Enter opens the provider list · Esc goes back · Ctrl+C quits.", "OnboardApiKeyTitle": "Connect your API key", "OnboardApiKeyStep1": "Step 1. Open your provider credential page and create a key for", "OnboardApiKeyLocalHint": "Local runtimes usually need no pasted key — start the server, then press Enter.", diff --git a/crates/tui/locales/es-419.json b/crates/tui/locales/es-419.json index 409874eefa..ff2be437f8 100644 --- a/crates/tui/locales/es-419.json +++ b/crates/tui/locales/es-419.json @@ -414,7 +414,7 @@ "OnboardLanguageFooter": "Presiona 1-9 para elegir, o Enter para mantener la configuración actual", "OnboardProviderTitle": "Elige tu proveedor de modelos", "OnboardProviderBlurb": "Elige dónde se ejecutará tu modelo. Los proveedores alojados necesitan una clave; los runtimes locales pueden continuar sin ella.", - "OnboardProviderFooter": "Presiona 0-9 para elegir, ↑/↓ para mover, Enter para continuar, Esc para volver.", + "OnboardProviderFooter": "Enter abre la lista de proveedores · Esc vuelve · Ctrl+C sale.", "OnboardApiKeyTitle": "Conecta tu clave API", "OnboardApiKeyStep1": "Paso 1. Abre la página de credenciales de tu proveedor y crea una clave para", "OnboardApiKeyLocalHint": "Los runtimes locales normalmente no requieren pegar una clave — inicia el servidor y presiona Enter.", diff --git a/crates/tui/locales/ja.json b/crates/tui/locales/ja.json index 42bcf031e4..2956800a00 100644 --- a/crates/tui/locales/ja.json +++ b/crates/tui/locales/ja.json @@ -414,7 +414,7 @@ "OnboardLanguageFooter": "1〜9 で選択、または Enter で現在の設定を維持", "OnboardProviderTitle": "モデルプロバイダーを選択", "OnboardProviderBlurb": "モデルの実行先を選びます。ホステッドプロバイダーにはキーが必要ですが、ローカルランタイムはキーなしで続行できます。", - "OnboardProviderFooter": "0-9 で選択、↑/↓ で移動、Enter で続行、Esc で戻る。", + "OnboardProviderFooter": "Enter でプロバイダー一覧を開く · Esc で戻る · Ctrl+C で終了。", "OnboardApiKeyTitle": "API キーを接続", "OnboardApiKeyStep1": "ステップ 1. プロバイダーの認証情報ページを開き、次のキーを作成:", "OnboardApiKeyLocalHint": "ローカルランタイムは通常キーの貼り付け不要 — サーバーを起動して Enter。", diff --git a/crates/tui/locales/ko.json b/crates/tui/locales/ko.json index 7662988f70..02ef9788e7 100644 --- a/crates/tui/locales/ko.json +++ b/crates/tui/locales/ko.json @@ -414,7 +414,7 @@ "OnboardLanguageFooter": "1-9를 눌러 선택하거나 Enter로 현재 설정을 유지하세요", "OnboardProviderTitle": "모델 프로바이더를 선택하세요", "OnboardProviderBlurb": "모델이 실행될 위치를 선택하세요. 호스팅 프로바이더는 키가 필요하지만 로컬 런타임은 키 없이 계속할 수 있습니다.", - "OnboardProviderFooter": "0-9를 눌러 선택하거나 ↑/↓로 이동, Enter로 계속, Esc로 뒤로 가세요.", + "OnboardProviderFooter": "Enter로 프로바이더 목록을 열고 · Esc로 뒤로 가며 · Ctrl+C로 종료합니다.", "OnboardApiKeyTitle": "API 키를 연결하세요", "OnboardApiKeyStep1": "1단계. 프로바이더의 자격 증명 페이지를 열고 다음의 키를 생성하세요:", "OnboardApiKeyLocalHint": "로컬 런타임은 보통 키를 붙여넣을 필요가 없습니다. 서버를 시작한 뒤 Enter를 누르세요.", diff --git a/crates/tui/locales/pt-BR.json b/crates/tui/locales/pt-BR.json index 2c940f3db0..f1aa32c764 100644 --- a/crates/tui/locales/pt-BR.json +++ b/crates/tui/locales/pt-BR.json @@ -414,7 +414,7 @@ "OnboardLanguageFooter": "Pressione 1-9 para escolher, ou Enter para manter a configuração atual", "OnboardProviderTitle": "Escolha seu provedor de modelos", "OnboardProviderBlurb": "Escolha onde o modelo será executado. Provedores hospedados precisam de chave; runtimes locais podem continuar sem uma.", - "OnboardProviderFooter": "Pressione 0-9 para escolher, ↑/↓ para mover, Enter para continuar, Esc para voltar.", + "OnboardProviderFooter": "Enter abre a lista de provedores · Esc volta · Ctrl+C sai.", "OnboardApiKeyTitle": "Conecte sua chave API", "OnboardApiKeyStep1": "Passo 1. Abra a página de credenciais do provedor e crie uma chave para", "OnboardApiKeyLocalHint": "Runtimes locais geralmente não precisam de chave colada — inicie o servidor e pressione Enter.", diff --git a/crates/tui/locales/vi.json b/crates/tui/locales/vi.json index aff67d7434..b734c4193c 100644 --- a/crates/tui/locales/vi.json +++ b/crates/tui/locales/vi.json @@ -414,7 +414,7 @@ "OnboardLanguageFooter": "Nhấn phím từ 1-9 để chọn, hoặc Enter để giữ cài đặt hiện tại", "OnboardProviderTitle": "Chọn nhà cung cấp mô hình", "OnboardProviderBlurb": "Chọn nơi mô hình chạy. Nhà cung cấp được lưu trữ cần khóa; runtime cục bộ có thể tiếp tục mà không cần khóa.", - "OnboardProviderFooter": "Nhấn 0-9 để chọn, ↑/↓ để di chuyển, Enter để tiếp tục, Esc để quay lại.", + "OnboardProviderFooter": "Enter mở danh sách nhà cung cấp · Esc quay lại · Ctrl+C thoát.", "OnboardApiKeyTitle": "Kết nối khóa API của bạn", "OnboardApiKeyStep1": "Bước 1. Mở trang thông tin xác thực của nhà cung cấp và tạo khóa cho", "OnboardApiKeyLocalHint": "Runtime cục bộ thường không cần dán khóa — khởi động máy chủ rồi nhấn Enter.", diff --git a/crates/tui/locales/zh-Hans.json b/crates/tui/locales/zh-Hans.json index c1ff2c31c3..08ae953703 100644 --- a/crates/tui/locales/zh-Hans.json +++ b/crates/tui/locales/zh-Hans.json @@ -414,7 +414,7 @@ "OnboardLanguageFooter": "按 1-9 选择，或按 Enter 保留当前设置", "OnboardProviderTitle": "选择模型提供商", "OnboardProviderBlurb": "选择模型运行的位置。托管提供商需要密钥；本地运行时无需密钥即可继续。", - "OnboardProviderFooter": "按 0-9 选择，↑/↓ 移动，Enter 继续，Esc 返回。", + "OnboardProviderFooter": "Enter 打开提供商列表 · Esc 返回 · Ctrl+C 退出。", "OnboardApiKeyTitle": "连接你的 API 密钥", "OnboardApiKeyStep1": "步骤 1. 打开你的提供商凭据页面并为", "OnboardApiKeyLocalHint": "本地运行时通常无需粘贴密钥——启动服务器后按 Enter。", diff --git a/crates/tui/src/tui/provider_picker.rs b/crates/tui/src/tui/provider_picker.rs index d22d52de53..b43d802a5d 100644 --- a/crates/tui/src/tui/provider_picker.rs +++ b/crates/tui/src/tui/provider_picker.rs @@ -1457,6 +1457,31 @@ impl ProviderPickerView { target: Option, config: &Config, runtime_status: Option, + ) -> Self { + Self::new_for_setup_inner(active, target, config, runtime_status, true) + } + + /// Open the setup catalog for first-run/recovery onboarding (#4763). + /// Identical to [`Self::new_for_setup`] except that a missing-auth + /// `target` is only *focused*: onboarding must show the navigable + /// provider list before it asks for a secret, so key/OAuth entry is + /// reached by picking a row, never by opening straight into it. + #[must_use] + pub fn new_for_onboarding( + active: ApiProvider, + target: Option, + config: &Config, + runtime_status: Option, + ) -> Self { + Self::new_for_setup_inner(active, target, config, runtime_status, false) + } + + fn new_for_setup_inner( + active: ApiProvider, + target: Option, + config: &Config, + runtime_status: Option, + key_entry_for_missing_auth: bool, ) -> Self { let mut picker = Self::new_with_runtime_status(active, config, runtime_status); picker.view = ProviderListView::Catalog; @@ -1465,7 +1490,7 @@ impl ProviderPickerView { && let Some(idx) = picker.rows.iter().position(|row| row.provider == target) { picker.selected_idx = idx; - if !picker.selected_has_key() { + if key_entry_for_missing_auth && !picker.selected_has_key() { picker.enter_key_entry(); } } @@ -5121,6 +5146,68 @@ mod tests { assert!(picker.api_key_input.is_empty()); } + /// #4763: onboarding focuses the persisted route but must still open on + /// the navigable list. Jumping straight into key/OAuth entry hid the + /// provider catalog from returning users with a missing key. + #[test] + fn onboarding_catalog_focuses_missing_provider_without_leaving_the_list() { + let _lock = crate::test_support::lock_test_env(); + let _anthropic_key = crate::test_support::EnvVarGuard::remove("ANTHROPIC_API_KEY"); + let config = Config::default(); + let picker = ProviderPickerView::new_for_onboarding( + ApiProvider::Deepseek, + Some(ApiProvider::Anthropic), + &config, + None, + ); + + assert_eq!(picker.stage, Stage::List); + assert_eq!(picker.view, ProviderListView::Catalog); + assert_eq!(picker.selected_provider(), ApiProvider::Anthropic); + assert_eq!( + picker.visible_row_count(), + picker.rows.len(), + "onboarding must show the whole provider catalog" + ); + } + + /// #4763: Escape backs out one stage at a time — key entry returns to the + /// list, and only the list dismisses the picker. + #[test] + fn onboarding_escape_walks_key_entry_back_to_the_list_then_dismisses() { + let _lock = crate::test_support::lock_test_env(); + let _anthropic_key = crate::test_support::EnvVarGuard::remove("ANTHROPIC_API_KEY"); + let config = Config::default(); + let mut picker = ProviderPickerView::new_for_onboarding( + ApiProvider::Deepseek, + Some(ApiProvider::Anthropic), + &config, + None, + ); + assert_eq!(picker.stage, Stage::List); + + picker.enter_key_entry(); + assert_eq!(picker.stage, Stage::KeyEntry); + + assert!(matches!( + picker.handle_key(key(KeyCode::Esc)), + ViewAction::None + )); + assert_eq!( + picker.stage, + Stage::List, + "Escape from key entry returns to the provider list" + ); + + assert!( + matches!( + picker.handle_key(key(KeyCode::Esc)), + ViewAction::EmitAndClose(ViewEvent::ProviderPickerDismissed { .. }) + ), + "Escape from the list dismisses the picker" + ); + } + #[test] fn setup_catalog_uses_setup_title() { let config = Config::default(); diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 2fa3ad26e5..2f4c6cf8b9 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -804,6 +804,41 @@ fn back_from_api_key_onboarding(app: &mut App) { app.status_message = None; } +/// Where a key goes while onboarding owns the screen (#4763). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum OnboardingKeyRoute { + /// Terminate the session. Ctrl+C is unconditional during onboarding. + Quit, + /// Hand the key to the provider picker on the view stack. + ProviderPicker, + /// Fall through to the legacy onboarding key switch. + Legacy, +} + +/// Decide the onboarding route for one key press. +/// +/// Two invariants this encodes, both regressions reported in #4763: +/// Ctrl+C quits from *any* onboarding state — a modal on the stack must not +/// swallow it — and Escape is never intercepted on the picker's behalf, so +/// the picker can back out one stage at a time instead of the shell popping +/// the whole modal from a key/OAuth sub-stage. +fn onboarding_key_route( + onboarding: OnboardingState, + top_kind: Option, + key: &KeyEvent, +) -> OnboardingKeyRoute { + if onboarding == OnboardingState::None { + return OnboardingKeyRoute::Legacy; + } + if key.code == KeyCode::Char('c') && key.modifiers.contains(KeyModifiers::CONTROL) { + return OnboardingKeyRoute::Quit; + } + if onboarding == OnboardingState::Provider && top_kind == Some(ModalKind::ProviderPicker) { + return OnboardingKeyRoute::ProviderPicker; + } + OnboardingKeyRoute::Legacy +} + fn back_from_provider_onboarding(app: &mut App) { if app.onboarding_missing_key_recovery { // A returning user declined missing-key recovery: leave onboarding @@ -4960,35 +4995,39 @@ async fn run_event_loop( // parallel ten-provider key handler. Route its keys before the // legacy onboarding switch so List/Key/Model/Confirm retain the // same behavior as `/provider` and `/setup`. - if app.onboarding == OnboardingState::Provider - && app.view_stack.top_kind() == Some(ModalKind::ProviderPicker) - { - if key.code == KeyCode::Esc { - // Onboarding has no committed provider choice yet. A - // single Escape always abandons the whole setup modal and - // returns to Language; do not let an inner key/model - // stage mutate config or mark onboarding complete. - app.view_stack.pop(); - back_from_provider_onboarding(app); + match onboarding_key_route(app.onboarding, app.view_stack.top_kind(), &key) { + // #4763: onboarding must never be a trap. Ctrl+C terminates + // from every onboarding state, including while the picker + // owns the keys — the legacy handler below is unreachable + // once a modal is on the stack. + OnboardingKeyRoute::Quit => { + let _ = engine_handle.send(Op::Shutdown).await; + return Ok(()); + } + // Every other key, Escape included, belongs to the picker. + // The picker's own per-stage Escape walks key/OAuth entry + // back to the list and only dismisses from the list, where + // `ProviderPickerDismissed` runs the same non-mutating + // onboarding back-transition the shell used to force. + OnboardingKeyRoute::ProviderPicker => { + let events = app.view_stack.handle_key(key); app.needs_redraw = true; + if handle_view_events_boxed( + terminal, + app, + config, + &task_manager, + &mut engine_handle, + &mut web_config_session, + events, + ) + .await? + { + return Ok(()); + } continue; } - let events = app.view_stack.handle_key(key); - app.needs_redraw = true; - if handle_view_events_boxed( - terminal, - app, - config, - &task_manager, - &mut engine_handle, - &mut web_config_session, - events, - ) - .await? - { - return Ok(()); - } - continue; + OnboardingKeyRoute::Legacy => {} } // Handle onboarding flow @@ -10155,7 +10194,9 @@ async fn query_provider_runtime_status( /// Open the one canonical provider setup surface for onboarding. Fresh /// onboarding starts at the full catalog; missing-key recovery focuses the /// current route so an exact Kimi Code K3 configuration can expose its plan -/// route before a secret is entered. +/// route before a secret is entered. Either way the picker opens on the +/// navigable list (#4763): onboarding never drops a user straight into a +/// key/OAuth prompt for a route they were not shown. async fn open_onboarding_provider_picker( app: &mut App, config: &Config, @@ -10169,7 +10210,7 @@ async fn open_onboarding_provider_picker( } let runtime_status = query_provider_runtime_status(engine_handle).await; app.view_stack.push( - crate::tui::provider_picker::ProviderPickerView::new_for_setup( + crate::tui::provider_picker::ProviderPickerView::new_for_onboarding( app.api_provider, focus_current_route.then_some(app.onboarding_provider), config, @@ -13809,7 +13850,16 @@ async fn handle_view_events( .replace("{provider}", provider.as_str()); app.push_status_toast(toast, StatusToastLevel::Success, Some(8_000)); let model_override = provider_picker_model_override(app, config, provider); - switch_provider(app, engine_handle, config, provider, model_override).await; + let switched = + switch_provider(app, engine_handle, config, provider, model_override).await; + // #4763: reusing an external CLI grant completes provider + // onboarding exactly like a submitted key or an applied + // route. Without this the picker closes on success and + // the user is returned to the provider step they just + // satisfied — the second half of the reported loop. + if switched && app.onboarding == OnboardingState::Provider { + complete_provider_picker_onboarding(app, provider); + } refresh_config_view_if_open(app, "provider"); } Err(error) => app.push_status_toast( diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index cbdda9a985..9dd9a9266f 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -10978,6 +10978,102 @@ fn mental_models_backtracks_to_the_last_first_run_decision() { assert_eq!(app.onboarding, OnboardingState::Language); } +// ---- Issue #4763: provider onboarding must never be a trap ---- + +/// Ctrl+C quits from every onboarding state, including while the provider +/// picker owns the keys. Before #4763 the picker swallowed it and the only +/// exit was Escape-then-Ctrl+C. +#[test] +fn onboarding_ctrl_c_quits_even_with_the_provider_picker_on_the_view_stack() { + let ctrl_c = KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL); + + assert_eq!( + onboarding_key_route( + OnboardingState::Provider, + Some(ModalKind::ProviderPicker), + &ctrl_c, + ), + OnboardingKeyRoute::Quit, + ); + assert_eq!( + onboarding_key_route(OnboardingState::Provider, None, &ctrl_c), + OnboardingKeyRoute::Quit, + ); + assert_eq!( + onboarding_key_route(OnboardingState::Language, None, &ctrl_c), + OnboardingKeyRoute::Quit, + ); + assert_eq!( + onboarding_key_route( + OnboardingState::MentalModels, + Some(ModalKind::Help), + &ctrl_c, + ), + OnboardingKeyRoute::Quit, + ); +} + +/// Escape is no longer intercepted on the picker's behalf, so the picker can +/// back out one stage (key/OAuth entry → list) and only dismiss from the +/// list. Non-onboarding keys still reach the legacy switch. +#[test] +fn onboarding_escape_is_routed_to_the_provider_picker_not_intercepted() { + let esc = KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE); + + assert_eq!( + onboarding_key_route( + OnboardingState::Provider, + Some(ModalKind::ProviderPicker), + &esc + ), + OnboardingKeyRoute::ProviderPicker, + "the picker owns Escape so it can back out one stage at a time" + ); + assert_eq!( + onboarding_key_route(OnboardingState::Provider, None, &esc), + OnboardingKeyRoute::Legacy, + "without the picker the legacy onboarding switch still handles Escape" + ); + assert_eq!( + onboarding_key_route( + OnboardingState::ApiKey, + Some(ModalKind::ProviderPicker), + &esc + ), + OnboardingKeyRoute::Legacy, + ); + assert_eq!( + onboarding_key_route(OnboardingState::None, Some(ModalKind::ProviderPicker), &esc), + OnboardingKeyRoute::Legacy, + "a picker outside onboarding is not an onboarding route" + ); +} + +/// #4763: reusing an external Grok CLI grant must finish provider onboarding +/// the same way a submitted key does. The consent handler used to switch the +/// route and stop there, returning the user to the provider step they had +/// just satisfied. +#[test] +fn external_grant_reuse_completes_provider_onboarding() { + let mut app = create_test_app(); + app.onboarding = OnboardingState::Provider; + app.onboarding_needs_api_key = true; + app.onboarding_missing_key_recovery = true; + app.offline_mode = true; + app.trust_mode = true; + + complete_provider_picker_onboarding(&mut app, crate::config::ApiProvider::Xai); + + assert_ne!( + app.onboarding, + OnboardingState::Provider, + "a satisfied provider must not return to the provider step" + ); + assert_eq!(app.onboarding_provider, crate::config::ApiProvider::Xai); + assert!(!app.onboarding_needs_api_key); + assert!(!app.offline_mode); +} + #[test] fn api_key_escape_returns_to_provider_step() { let mut app = create_test_app(); diff --git a/crates/tui/src/xai_oauth.rs b/crates/tui/src/xai_oauth.rs index 575e89346c..9a245f633a 100644 --- a/crates/tui/src/xai_oauth.rs +++ b/crates/tui/src/xai_oauth.rs @@ -1440,6 +1440,85 @@ mod tests { ); } + /// #4763 root trigger. A returning xAI-OAuth user whose only material is + /// an external Grok CLI grant loses readiness the moment that CLI's + /// short-lived access token expires, even though a refresh token sits + /// right beside it — read-only consent deliberately never refreshes or + /// rewrites another CLI's file, so there is nothing to renew it with. + /// `needs_api_key` therefore flips to true and onboarding reopens. That + /// is the intended invariant, not a leak; this test pins it so the + /// onboarding entry point stays explainable. + #[test] + fn expired_external_grok_grant_reads_as_missing_key_despite_refresh_token() { + let _guard = crate::test_support::lock_test_env(); + let dir = TempDir::new().unwrap(); + let root = dir.path().canonicalize().expect("canonical temp root"); + let path = root.join("external-grok-auth.json"); + let scope = format!("https://auth.x.ai::{GROK_OIDC_CLIENT_ID}"); + fs::write( + &path, + serde_json::json!({ + scope.clone(): { + "key": "expired-external-access", + "refresh_token": "present-but-unusable-under-read-only-consent", + "expires_at": rfc3339_from_unix(now_unix_secs().unwrap_or(0) - 3600), + "oidc_client_id": GROK_OIDC_CLIENT_ID, + } + }) + .to_string(), + ) + .unwrap(); + let _home_guard = + crate::test_support::EnvVarGuard::set("CODEWHALE_HOME", root.join("codewhale-owned")); + let _path_guard = crate::test_support::EnvVarGuard::set("GROK_AUTH_PATH", &path); + let _key_guard = crate::test_support::EnvVarGuard::remove("XAI_API_KEY"); + let config = Config { + provider: Some(ApiProvider::Xai.as_str().to_string()), + providers: Some(crate::config::ProvidersConfig { + xai: crate::config::ProviderConfig { + auth_mode: Some("oauth".to_string()), + external_credentials: Some( + codewhale_config::ExternalCredentialConsentToml::read_only( + codewhale_config::ProviderKind::Xai, + codewhale_config::ExternalCredentialSource::GrokCli, + path.clone(), + ), + ), + ..Default::default() + }, + ..Default::default() + }), + ..Default::default() + }; + + assert!( + !credentials_present(&config), + "an expired external access token is not usable material" + ); + assert!( + !crate::config::has_api_key_for(&config, ApiProvider::Xai), + "expired external xAI OAuth must fall through to the missing-key path" + ); + + // The same file with a live access token is ready, so the check is + // expiry-driven rather than a blanket rejection of external grants. + fs::write( + &path, + serde_json::json!({ + scope: { + "key": "fresh-external-access", + "refresh_token": "unused", + "expires_at": rfc3339_from_now(3600), + "oidc_client_id": GROK_OIDC_CLIENT_ID, + } + }) + .to_string(), + ) + .unwrap(); + assert!(credentials_present(&config)); + assert!(crate::config::has_api_key_for(&config, ApiProvider::Xai)); + } + #[test] fn native_login_storage_is_codewhale_owned() { let _guard = crate::test_support::lock_test_env(); From 1d0085cd74d660041549206c1d141794ad4d85d8 Mon Sep 17 00:00:00 2001 From: Hunter B Date: Sun, 26 Jul 2026 06:22:42 -0700 Subject: [PATCH 2/2] test(pty): follow current Kimi recovery detail Keep the returning-user provider recovery test aligned with the current truthful missing-key diagnostic while preserving the picker and config immutability assertions. --- crates/tui/tests/qa_pty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tui/tests/qa_pty.rs b/crates/tui/tests/qa_pty.rs index 3a46ed45e6..bd8f38e143 100644 --- a/crates/tui/tests/qa_pty.rs +++ b/crates/tui/tests/qa_pty.rs @@ -741,7 +741,7 @@ model = "k3" h.wait_for_text("Moonshot/Kimi", BOOT_TIMEOUT)?; h.wait_for_text("api.kimi.com", BOOT_TIMEOUT)?; - h.wait_for_text("does not import Kimi CLI credentials", BOOT_TIMEOUT)?; + h.wait_for_text("missing MOONSHOT_API_KEY / KIMI_API_KEY", BOOT_TIMEOUT)?; // The picker rendered without mutating the configured route. assert_eq!(