From edff54b0b14465f32cc4817d9040b3b84597857e Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sun, 8 Mar 2026 08:10:46 +0000 Subject: [PATCH] fix: persist /model selection across restarts (#707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: persist /model selection across restarts The /model command called set_model() on the LLM provider but never saved the choice to settings, so the model reverted on restart. Now persists to both the DB settings store and config.toml. Co-Authored-By: Claude Opus 4.6 * fix: address CI clippy lint and use spawn_blocking for TOML I/O - Use struct init syntax instead of field reassignment in test (clippy) - Wrap sync filesystem operations in spawn_blocking to avoid blocking the tokio executor Co-Authored-By: Claude Opus 4.6 * style: fix rustfmt formatting Co-Authored-By: Claude Opus 4.6 * fix: address review feedback — handle JoinError, remove exists() guard - Log warning if spawn_blocking task panics/is cancelled (JoinError) - Remove toml_path.exists() guard; load_toml already returns Ok(None) for missing files, so permission errors are no longer silently skipped Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- src/agent/commands.rs | 50 +++++++++++++++++++++++++++++++++++++++---- src/settings.rs | 25 ++++++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/agent/commands.rs b/src/agent/commands.rs index f0b79896..bf1c7e6c 100644 --- a/src/agent/commands.rs +++ b/src/agent/commands.rs @@ -663,10 +663,14 @@ impl Agent { } match self.llm().set_model(requested) { - Ok(()) => Ok(SubmissionResult::response(format!( - "Switched model to: {}", - requested - ))), + Ok(()) => { + // Persist the model choice so it survives restarts. + self.persist_selected_model(requested).await; + Ok(SubmissionResult::response(format!( + "Switched model to: {}", + requested + ))) + } Err(e) => Ok(SubmissionResult::error(format!( "Failed to switch model: {}", e @@ -822,4 +826,42 @@ impl Agent { _ => Ok(None), } } + + /// Persist the selected model to the settings store (DB and/or TOML config). + /// + /// Best-effort: logs warnings on failure but does not propagate errors, + /// since the in-memory model switch already succeeded. + async fn persist_selected_model(&self, model: &str) { + // 1. Persist to DB if available. + if let Some(store) = self.store() { + let value = serde_json::Value::String(model.to_string()); + if let Err(e) = store.set_setting("default", "selected_model", &value).await { + tracing::warn!("Failed to persist model to DB: {}", e); + } + } + + // 2. Update TOML config file if it exists (sync I/O in spawn_blocking). + let model_owned = model.to_string(); + if let Err(e) = tokio::task::spawn_blocking(move || { + let toml_path = crate::settings::Settings::default_toml_path(); + match crate::settings::Settings::load_toml(&toml_path) { + Ok(Some(mut settings)) => { + settings.selected_model = Some(model_owned); + if let Err(e) = settings.save_toml(&toml_path) { + tracing::warn!("Failed to persist model to config.toml: {}", e); + } + } + Ok(None) => { + // No config file on disk; nothing to update. + } + Err(e) => { + tracing::warn!("Failed to load config.toml for model persistence: {}", e); + } + } + }) + .await + { + tracing::warn!("Model TOML persistence task failed: {}", e); + } + } } diff --git a/src/settings.rs b/src/settings.rs index 92fe207d..45eae536 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1198,6 +1198,31 @@ mod tests { assert_eq!(loaded.heartbeat.interval_secs, 900); } + /// Regression test: /model command must persist selected_model to TOML config. + /// Prior to the fix, `set_model()` only changed the in-memory provider and the + /// choice was lost on restart. + #[test] + fn toml_selected_model_update_persists() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + + // Start with a config that has a different model. + let settings = Settings { + selected_model: Some("old-model".to_string()), + ..Default::default() + }; + settings.save_toml(&path).unwrap(); + + // Simulate what persist_selected_model does: load, update, save. + let mut loaded = Settings::load_toml(&path).unwrap().unwrap(); + loaded.selected_model = Some("new-model".to_string()); + loaded.save_toml(&path).unwrap(); + + // Verify the change survived a reload. + let reloaded = Settings::load_toml(&path).unwrap().unwrap(); + assert_eq!(reloaded.selected_model, Some("new-model".to_string())); + } + #[test] fn toml_missing_file_returns_none() { let result = Settings::load_toml(std::path::Path::new("/tmp/nonexistent_config.toml"));