Skip to content

Commit 56b7218

Browse files
zmanianclaude
andauthored
fix(setup): preserve model name when re-running onboarding with same provider (#600) (#694)
Each provider setup function unconditionally cleared selected_model, so re-running the wizard with "Keep current provider? Yes" would lose the model name, forcing the user to re-select it every time. Now only clears selected_model when the backend actually changes (old model may be invalid for the new provider). When keeping the same provider, the model is preserved and Step 4 shows the "Keep current model" prompt. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 200aed1 commit 56b7218

1 file changed

Lines changed: 54 additions & 8 deletions

File tree

src/setup/wizard.rs

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,10 +1037,11 @@ impl SetupWizard {
10371037

10381038
/// Anthropic OAuth setup: extract token from `claude login` credentials.
10391039
async fn setup_anthropic_oauth(&mut self) -> Result<(), SetupError> {
1040-
self.settings.llm_backend = Some("anthropic".to_string());
1041-
if self.settings.selected_model.is_some() {
1040+
// Clear model only when switching providers (old model may be invalid)
1041+
if self.settings.llm_backend.as_deref() != Some("anthropic") {
10421042
self.settings.selected_model = None;
10431043
}
1044+
self.settings.llm_backend = Some("anthropic".to_string());
10441045

10451046
// Try to extract existing OAuth token from Claude Code credentials
10461047
if let Some(token) = crate::config::ClaudeCodeConfig::extract_oauth_token() {
@@ -1134,10 +1135,11 @@ impl SetupWizard {
11341135
other => other,
11351136
});
11361137

1137-
self.settings.llm_backend = Some(backend.to_string());
1138-
if self.settings.selected_model.is_some() {
1138+
// Clear model only when switching providers (old model may be invalid)
1139+
if self.settings.llm_backend.as_deref() != Some(backend) {
11391140
self.settings.selected_model = None;
11401141
}
1142+
self.settings.llm_backend = Some(backend.to_string());
11411143

11421144
// Check env var first
11431145
if let Ok(existing) = std::env::var(env_var) {
@@ -1196,10 +1198,11 @@ impl SetupWizard {
11961198
&mut self,
11971199
def: &crate::llm::ProviderDefinition,
11981200
) -> Result<(), SetupError> {
1199-
self.settings.llm_backend = Some(def.id.clone());
1200-
if self.settings.selected_model.is_some() {
1201+
// Clear model only when switching providers (old model may be invalid)
1202+
if self.settings.llm_backend.as_deref() != Some(&def.id) {
12011203
self.settings.selected_model = None;
12021204
}
1205+
self.settings.llm_backend = Some(def.id.clone());
12031206

12041207
let default_url = self
12051208
.settings
@@ -1234,10 +1237,11 @@ impl SetupWizard {
12341237
secret_name: &str,
12351238
display_name: &str,
12361239
) -> Result<(), SetupError> {
1237-
self.settings.llm_backend = Some(backend_id.to_string());
1238-
if self.settings.selected_model.is_some() {
1240+
// Clear model only when switching providers (old model may be invalid)
1241+
if self.settings.llm_backend.as_deref() != Some(backend_id) {
12391242
self.settings.selected_model = None;
12401243
}
1244+
self.settings.llm_backend = Some(backend_id.to_string());
12411245

12421246
let existing_url = self
12431247
.settings
@@ -3521,6 +3525,48 @@ mod tests {
35213525
}
35223526
}
35233527

3528+
/// Regression test for #600: re-running provider setup for the same backend
3529+
/// must NOT clear selected_model. Only switching to a different backend should.
3530+
#[test]
3531+
fn test_same_provider_preserves_selected_model() {
3532+
let mut wizard = SetupWizard::new();
3533+
wizard.settings.llm_backend = Some("ollama".to_string());
3534+
wizard.settings.selected_model = Some("llama3".to_string());
3535+
3536+
// Simulate re-entering the same provider -- model should survive
3537+
// (This is the check that each setup_* function now performs)
3538+
if wizard.settings.llm_backend.as_deref() != Some("ollama") {
3539+
wizard.settings.selected_model = None;
3540+
}
3541+
wizard.settings.llm_backend = Some("ollama".to_string());
3542+
3543+
assert_eq!(
3544+
wizard.settings.selected_model.as_deref(),
3545+
Some("llama3"),
3546+
"model should be preserved when re-selecting the same provider"
3547+
);
3548+
}
3549+
3550+
/// Regression test for #600: switching to a different provider must clear
3551+
/// selected_model since the old model may not be valid for the new backend.
3552+
#[test]
3553+
fn test_different_provider_clears_selected_model() {
3554+
let mut wizard = SetupWizard::new();
3555+
wizard.settings.llm_backend = Some("ollama".to_string());
3556+
wizard.settings.selected_model = Some("llama3".to_string());
3557+
3558+
// Simulate switching to a different provider -- model should be cleared
3559+
if wizard.settings.llm_backend.as_deref() != Some("openai") {
3560+
wizard.settings.selected_model = None;
3561+
}
3562+
wizard.settings.llm_backend = Some("openai".to_string());
3563+
3564+
assert!(
3565+
wizard.settings.selected_model.is_none(),
3566+
"model should be cleared when switching providers"
3567+
);
3568+
}
3569+
35243570
#[tokio::test]
35253571
async fn test_run_provider_setup_no_setup_hint() {
35263572
// A provider with setup: None should not error. It should set the

0 commit comments

Comments
 (0)