diff --git a/CHANGELOG.md b/CHANGELOG.md index f014f225..d02fc1c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `/config` now reports the canonical `~/.codewhale/settings.toml` path for TUI settings while still reading legacy DeepSeek-branded settings fallbacks and migrating them into the CodeWhale home on load. +- Provider switches now roll back transactionally when the first request to a + newly selected provider fails authentication: CodeWhale restores the previous + provider/model, model-ID passthrough, onboarding/API-key state, runtime + config, persisted provider selection, and engine handle so users can return + to DeepSeek after a failed Moonshot/Kimi switch (#2754, #2755). Thanks + @Dr3259 for the Windows repro and @cyq1017 for the draft fix. - `PATCH /v1/threads/{id}` can now update a thread's persisted workspace for GUI/runtime clients. Workspace changes reject active turns and evict idle cached engines so the next turn starts in the new workspace. diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index f014f225..d02fc1c4 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -55,6 +55,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `/config` now reports the canonical `~/.codewhale/settings.toml` path for TUI settings while still reading legacy DeepSeek-branded settings fallbacks and migrating them into the CodeWhale home on load. +- Provider switches now roll back transactionally when the first request to a + newly selected provider fails authentication: CodeWhale restores the previous + provider/model, model-ID passthrough, onboarding/API-key state, runtime + config, persisted provider selection, and engine handle so users can return + to DeepSeek after a failed Moonshot/Kimi switch (#2754, #2755). Thanks + @Dr3259 for the Windows repro and @cyq1017 for the draft fix. - `PATCH /v1/threads/{id}` can now update a thread's persisted workspace for GUI/runtime clients. Workspace changes reject active turns and evict idle cached engines so the next turn starts in the new workspace. diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index 024ce213..d2315448 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -1216,6 +1216,17 @@ pub struct ToolEvidence { pub summary: String, } +#[derive(Debug, Clone)] +pub(crate) struct PendingProviderSwitch { + pub previous_provider: ApiProvider, + pub previous_model: String, + pub previous_model_ids_passthrough: bool, + pub previous_config: Config, + pub previous_onboarding: OnboardingState, + pub previous_onboarding_needs_api_key: bool, + pub previous_api_key_env_only: bool, +} + /// Global UI state for the TUI. #[allow(clippy::struct_excessive_bools)] pub struct App { @@ -1272,6 +1283,9 @@ pub struct App { /// True when the active provider/base URL accepts arbitrary model IDs /// verbatim rather than DeepSeek-only aliases. pub model_ids_passthrough: bool, + /// Pending provider transition for transactional rollback when the next + /// auth failure indicates the new provider cannot be used. + pub pending_provider_switch: Option, /// Current reasoning-effort tier for DeepSeek thinking mode. /// Cycled via Shift+Tab; initialized from config at startup. pub reasoning_effort: ReasoningEffort, @@ -2067,6 +2081,7 @@ impl App { last_effective_model: None, api_provider: provider, model_ids_passthrough, + pending_provider_switch: None, reasoning_effort, last_effective_reasoning_effort: None, workspace, diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index ed07841c..1a814bd5 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -117,8 +117,8 @@ use crate::tui::workspace_context; use super::key_actions; use super::app::{ - App, AppAction, AppMode, OnboardingState, QueuedMessage, ReasoningEffort, SidebarFocus, - StatusToastLevel, SubmitDisposition, TaskPanelEntry, TuiOptions, + App, AppAction, AppMode, OnboardingState, PendingProviderSwitch, QueuedMessage, + ReasoningEffort, SidebarFocus, StatusToastLevel, SubmitDisposition, TaskPanelEntry, TuiOptions, looks_like_slash_command_input, shell_command_from_bang_input, }; use super::approval::{ @@ -1307,6 +1307,7 @@ async fn run_event_loop( let mut received_engine_event = false; let mut transcript_batch_updated = false; let mut queued_to_send: Option = None; + let mut respawn_after_provider_rollback: Option = None; { let mut rx = engine_handle.rx_event.write().await; loop { @@ -1720,6 +1721,7 @@ async fn run_event_loop( } app.is_loading = false; app.dispatch_started_at = None; + app.pending_provider_switch = None; app.offline_mode = false; app.streaming_state.reset(); if was_locally_cancelled { @@ -1986,7 +1988,18 @@ async fn run_event_loop( envelope, recoverable: _, } => { + let rollback_after_auth_failure = + matches!( + envelope.category, + crate::error_taxonomy::ErrorCategory::Authentication + ) && app.pending_provider_switch.is_some(); apply_engine_error_to_app(app, envelope); + if rollback_after_auth_failure + && let Some(rollback_warning) = + rollback_provider_after_auth_failure(app, config) + { + respawn_after_provider_rollback = Some(rollback_warning); + } } EngineEvent::Status { message } => { app.status_message = Some(message); @@ -2406,6 +2419,29 @@ async fn run_event_loop( } } } + if let Some(rollback_warning) = respawn_after_provider_rollback { + let _ = engine_handle.send(Op::Shutdown).await; + let engine_config = build_engine_config(app, config); + engine_handle = spawn_engine(engine_config, config); + if !app.api_messages.is_empty() { + let _ = engine_handle + .send(Op::SyncSession { + session_id: app.current_session_id.clone(), + messages: app.api_messages.clone(), + system_prompt: app.system_prompt.clone(), + system_prompt_override: false, + model: app.model.clone(), + workspace: app.workspace.clone(), + }) + .await; + } + let _ = engine_handle + .send(Op::SetCompaction { + config: app.compaction_config(), + }) + .await; + app.status_message = Some(rollback_warning); + } if let Some(index) = app.streaming_message_index { let committed = app.streaming_state.commit_text(0); if !committed.is_empty() { @@ -4569,6 +4605,68 @@ pub(crate) fn apply_engine_error_to_app( // toast in the footer — that duplicates the transcript entry. } +fn rollback_provider_after_auth_failure(app: &mut App, config: &mut Config) -> Option { + let pending = app.pending_provider_switch.take()?; + let PendingProviderSwitch { + previous_provider, + previous_model, + previous_model_ids_passthrough, + previous_config, + previous_onboarding, + previous_onboarding_needs_api_key, + previous_api_key_env_only, + } = pending; + + *config = previous_config; + app.api_provider = previous_provider; + app.set_model_selection(previous_model.clone()); + app.provider_models + .insert(previous_provider.as_str().to_string(), previous_model); + app.model_ids_passthrough = previous_model_ids_passthrough; + app.update_model_compaction_budget(); + app.clear_model_scoped_telemetry(); + app.offline_mode = false; + app.onboarding = previous_onboarding; + app.onboarding_needs_api_key = previous_onboarding_needs_api_key; + app.api_key_env_only = previous_api_key_env_only; + + let persistence_error = (|| -> anyhow::Result<()> { + commands::persist_root_string_key( + app.config_path.as_deref(), + "provider", + previous_provider.as_str(), + )?; + let mut settings = crate::settings::Settings::load()?; + settings.default_provider = Some(previous_provider.as_str().to_string()); + settings.set_model_for_provider( + previous_provider.as_str(), + &app.model_selection_for_persistence(), + ); + if matches!( + previous_provider, + ApiProvider::Deepseek | ApiProvider::DeepseekCN + ) { + settings.set("default_model", &app.model_selection_for_persistence())?; + } + settings.save()?; + Ok(()) + })() + .err() + .map(|err| format!("provider rollback not fully persisted: {err}")); + + Some(match persistence_error { + Some(warning) => format!( + "Provider switch failed and has been rolled back to {}. {}", + previous_provider.as_str(), + warning + ), + None => format!( + "Provider switch failed and has been rolled back to {}.", + previous_provider.as_str() + ), + }) +} + fn persist_offline_queue_state(app: &App) { if app.queued_messages.is_empty() && app.queued_draft.is_none() { persistence_actor::persist(PersistRequest::ClearOfflineQueue); @@ -5339,10 +5437,17 @@ async fn switch_provider( ) { let previous_provider = app.api_provider; let previous_model = app.model.clone(); - let previous_provider_str = config.provider.clone(); - let previous_base_url = config.base_url.clone(); - let previous_default_text_model = config.default_text_model.clone(); - let previous_providers = config.providers.clone(); + let previous_model_ids_passthrough = app.model_ids_passthrough; + let previous_config = config.clone(); + app.pending_provider_switch = Some(PendingProviderSwitch { + previous_provider, + previous_model: previous_model.clone(), + previous_model_ids_passthrough, + previous_config: previous_config.clone(), + previous_onboarding: app.onboarding, + previous_onboarding_needs_api_key: app.onboarding_needs_api_key, + previous_api_key_env_only: app.api_key_env_only, + }); config.provider = Some(target.as_str().to_string()); if matches!(target, ApiProvider::NvidiaNim) @@ -5368,10 +5473,8 @@ async fn switch_provider( } if let Err(err) = DeepSeekClient::new(config) { - config.provider = previous_provider_str; - config.base_url = previous_base_url; - config.default_text_model = previous_default_text_model; - config.providers = previous_providers; + app.pending_provider_switch = None; + *config = previous_config; app.add_message(HistoryCell::System { content: format!( "Failed to switch provider to {}: {err}\nProvider unchanged ({}).", diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index 5106859d..f45686c1 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -7300,6 +7300,89 @@ fn recoverable_engine_error_does_not_enter_offline_mode() { let _ = ErrorEnvelope::transient(""); } +#[tokio::test] +async fn provider_switch_auth_error_restores_previous_provider_and_model() { + use crate::error_taxonomy::ErrorEnvelope; + + let _home = SettingsHomeGuard::new(); + let mut app = create_test_app(); + app.api_provider = ApiProvider::Deepseek; + app.model = "deepseek-v4-pro".to_string(); + app.model_ids_passthrough = false; + app.onboarding = OnboardingState::None; + app.onboarding_needs_api_key = false; + app.api_key_env_only = true; + let mut engine = mock_engine_handle(); + let mut config = Config { + provider: Some("deepseek".to_string()), + api_key: Some("deepseek-key".to_string()), + default_text_model: Some("deepseek-v4-pro".to_string()), + providers: Some(ProvidersConfig { + deepseek: ProviderConfig { + api_key: Some("deepseek-key".to_string()), + ..Default::default() + }, + moonshot: ProviderConfig { + api_key: Some("kimi-key".to_string()), + ..Default::default() + }, + ..Default::default() + }), + ..Default::default() + }; + + switch_provider( + &mut app, + &mut engine.handle, + &mut config, + ApiProvider::Moonshot, + Some("kimi-k2.6".to_string()), + ) + .await; + assert_eq!(app.api_provider, ApiProvider::Moonshot); + assert_eq!(config.provider.as_deref(), Some("moonshot")); + assert!(app.pending_provider_switch.is_some()); + + apply_engine_error_to_app( + &mut app, + ErrorEnvelope::fatal_auth("Authentication failed: invalid API key"), + ); + let rollback_status = rollback_provider_after_auth_failure(&mut app, &mut config) + .expect("auth failure after provider switch should roll back"); + + assert_eq!(app.api_provider, ApiProvider::Deepseek); + assert_eq!(app.model, "deepseek-v4-pro"); + assert!(!app.model_ids_passthrough); + assert!(!app.offline_mode); + assert_eq!(app.onboarding, OnboardingState::None); + assert!(!app.onboarding_needs_api_key); + assert!(app.api_key_env_only); + assert_eq!(config.provider.as_deref(), Some("deepseek")); + assert_eq!( + config.default_text_model.as_deref(), + Some("deepseek-v4-pro") + ); + let settings = crate::settings::Settings::load().expect("load settings"); + assert_eq!(settings.default_provider.as_deref(), Some("deepseek")); + assert_eq!( + settings + .provider_models + .as_ref() + .and_then(|models| models.get("deepseek")) + .map(String::as_str), + Some("deepseek-v4-pro") + ); + assert_eq!(settings.default_model.as_deref(), Some("deepseek-v4-pro")); + assert!(app.pending_provider_switch.is_none()); + assert!(rollback_status.contains("Provider switch failed")); + assert!( + app.status_message + .as_deref() + .map_or(true, |status| !status.contains("Provider switch failed")), + "status message is set by the async event loop after engine respawn" + ); +} + #[test] fn stream_error_marks_active_turn_failed_without_waiting_for_turn_complete() { use crate::error_taxonomy::ErrorEnvelope; @@ -7363,6 +7446,7 @@ fn non_recoverable_engine_error_enters_offline_mode() { app.status_message.is_none(), "non-recoverable error should NOT set status_message — already in transcript as HistoryCell::Error" ); + assert!(app.pending_provider_switch.is_none()); } #[test]