Changing AI settings never saves over ones that couldn't be read

Fixes #261

crates/ai/src/lib.rs

@@ -288,6 +288,15 @@ impl AiHub {
         }
     }
 
+    /// The configuration to change and save back. An [ai] section that can't be read is an error
+    /// here, never the empty default, so a change can't save over it (#261).
+    fn config_to_change(&self) -> Result<AiConfig, ToolError> {
+        match self.engine.try_session() {
+            Some(s) => AiConfig::from_table(&s.settings.ai),
+            None => AiConfig::from_table(&Settings::load_from(&self.settings_path()).ai),
+        }
+    }
+
     /// Save the AI configuration into the settings file, then the session: a save that fails
     /// changes nothing, so the app never shows a backend or role that isn't saved.
     pub fn save_config(&self, cfg: &AiConfig) -> Result<(), ToolError> {
@@ -326,7 +335,7 @@ impl AiHub {
     /// Choose the backend for `role`; `None` goes back to its default (the builder: an agent app,
     /// else any AI backend; the extractor: none).
     pub fn set_role(&self, role: Role, name: Option<&str>) -> Result<(), ToolError> {
-        let mut cfg = self.config();
+        let mut cfg = self.config_to_change()?;
         let role = config::role_key(role).to_string();
         match name {
             Some(n) if cfg.get(n).is_none() => {
@@ -392,10 +401,10 @@ impl AiHub {
         if entry.config.name.trim().is_empty() {
             return Err(ToolError::invalid("A backend needs a name."));
         }
+        let mut cfg = self.config_to_change()?;
         if let Some(k) = key.filter(|k| !k.trim().is_empty()) {
             self.keys.set(&entry.config.name, k.trim())?;
         }
-        let mut cfg = self.config();
         cfg.upsert(entry);
         self.save_config(&cfg)
     }
@@ -403,7 +412,7 @@ impl AiHub {
     /// Turn a backend on or off; it keeps its settings and key. Returns false when there was no
     /// such backend.
     pub fn set_enabled(&self, name: &str, on: bool) -> Result<bool, ToolError> {
-        let mut cfg = self.config();
+        let mut cfg = self.config_to_change()?;
         let Some(b) = cfg.backends.iter_mut().find(|b| b.config.name == name) else { return Ok(false) };
         b.config.enabled = on;
         self.save_config(&cfg)?;
@@ -413,7 +422,7 @@ impl AiHub {
     /// The program an agent app runs (`None` or empty: found by its usual name, on PATH and in the
     /// usual install folders). Returns false when there was no such backend.
     pub fn set_command(&self, name: &str, command: Option<&str>) -> Result<bool, ToolError> {
-        let mut cfg = self.config();
+        let mut cfg = self.config_to_change()?;
         let Some(b) = cfg.backends.iter_mut().find(|b| b.config.name == name) else { return Ok(false) };
         b.options.command = command.map(str::trim).filter(|c| !c.is_empty()).map(str::to_string);
         self.save_config(&cfg)?;
@@ -432,7 +441,7 @@ impl AiHub {
         if model.is_empty() {
             return Err(ToolError::invalid("Name a model."));
         }
-        let mut cfg = self.config();
+        let mut cfg = self.config_to_change()?;
         let Some(b) = cfg.backends.iter_mut().find(|b| b.config.name == name) else { return Ok(false) };
         match b.config.kind {
             BackendKind::AgentCli => b.options.cli_model = Some(model.to_string()),
@@ -455,7 +464,7 @@ impl AiHub {
 
     /// Remove a backend and its stored key. Returns false when there was no such backend.
     pub fn remove_backend(&self, name: &str) -> Result<bool, ToolError> {
-        let mut cfg = self.config();
+        let mut cfg = self.config_to_change()?;
         let removed = cfg.remove(name);
         if removed {
             self.save_config(&cfg)?;

crates/ai/src/tests.rs

@@ -138,6 +138,36 @@ fn a_failed_save_changes_nothing() {
     assert!(h.backends().is_empty(), "the app shows only what was saved");
 }
 
+/// #261: one bad price made the whole [ai] section read as empty, and adding a backend saved
+/// that over the backends, roles and prices.
+#[test]
+fn ai_settings_that_cant_be_read_are_never_saved_over() {
+    let (h, d) = hub();
+    add(&h, "Claude Code", BackendKind::AgentCli, "http://127.0.0.1:9", None);
+    add(&h, "Kimi K3 (OpenRouter)", BackendKind::Openrouter, "http://127.0.0.1:9", None);
+    h.set_role(Role::Builder, Some("Kimi K3 (OpenRouter)")).unwrap();
+    let file = d.path().join("config.toml");
+    let mut bad = h.engine.session().settings.clone();
+    let price: toml::Table = toml::from_str(
+        "[m]
+input_per_mtok = 1.0
+",
+    )
+    .unwrap();
+    bad.ai.insert("prices".into(), toml::Value::Table(price));
+    bad.save_to(&file).unwrap();
+    h.engine.session().settings = bad;
+    let before = std::fs::read(&file).unwrap();
+    let cfg = BackendConfig { name: "Other".into(), kind: BackendKind::Openrouter, model: "m".into(), base_url: None, ssh_host: None, enabled: true };
+    let e = h.add_backend(cfg, Some("sk-test".into())).unwrap_err();
+    assert_eq!(e.code, ErrorCode::BadSettings, "{e:?}");
+    assert!(h.set_role(Role::Builder, None).is_err());
+    assert!(h.set_enabled("Claude Code", false).is_err());
+    assert!(h.remove_backend("Claude Code").is_err());
+    assert_eq!(std::fs::read(&file).unwrap(), before, "the settings file was changed");
+    assert!(h.keys.get("Other").is_none(), "a refused add stored its key");
+}
+
 #[test]
 fn the_person_chooses_who_builds() {
     let (h, _d) = hub();