fix(agent): persist /model selection to .env, TOML, and DB (#1581)

* fix(agent): persist /model selection to .env, TOML, and DB

The /model command only wrote selected_model to the DB and config.toml,
but env vars from ~/.ironclaw/.env (e.g. NEARAI_MODEL) have the highest
priority in LlmConfig::resolve_model(). The .env value was never
updated, so it always shadowed the new model on restart.

Now persist_selected_model updates all three persistence layers:
1. The backend-specific model env var in ~/.ironclaw/.env (only if the
   var already exists, to avoid injecting new vars)
2. The config.toml file (created if absent, since TOML > DB priority)
3. The DB settings table (for completeness)

Also adds diagnostic logging when the DB store is unavailable.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(agent): address PR review — backend from deps, exact .env match

Review feedback:
- Use resolved llm_backend from AgentDeps instead of re-reading from
  disk/env (fixes DB-only backend detection, eliminates redundant I/O)
- Match .env var with exact "KEY=" prefix and skip commented lines
  (prevents false matches on NEARAI_MODEL_VERSION etc.)
- TOML is now loaded once (no double-read for backend + model update)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>

src/agent/agent_loop.rs

@@ -169,6 +169,9 @@ pub struct AgentDeps {
     pub sandbox_readiness: crate::agent::routine_engine::SandboxReadiness,
     /// Software builder for self-repair tool rebuilding.
     pub builder: Option<Arc<dyn crate::tools::SoftwareBuilder>>,
+    /// Resolved LLM backend identifier (e.g., "nearai", "openai", "groq").
+    /// Used by `/model` persistence to determine which env var to update.
+    pub llm_backend: String,
 }
 
 /// The main agent that coordinates all components.

src/agent/commands.rs

@@ -841,12 +841,50 @@ impl Agent {
                 .await
             {
                 tracing::warn!("Failed to persist model to DB: {}", e);
+            } else {
+                tracing::debug!("Persisted selected_model to DB: {}", model);
             }
+        } else {
+            tracing::warn!("No database store available — model choice will not persist to DB");
         }
 
-        // 2. Update TOML config file if it exists (sync I/O in spawn_blocking).
+        // 2. Update .env and TOML config file (sync I/O in spawn_blocking).
         let model_owned = model.to_string();
+        let backend = self.deps.llm_backend.clone();
         if let Err(e) = tokio::task::spawn_blocking(move || {
+            // 2a. Update the backend-specific model env var in ~/.ironclaw/.env.
+            //
+            // Env vars have the HIGHEST priority in LlmConfig::resolve_model()
+            // (env var > TOML > DB > default). If the .env file has e.g.
+            // NEARAI_MODEL=old-model, it shadows everything else. We must
+            // update this var or the /model change is invisible on restart.
+            let registry = crate::llm::ProviderRegistry::load();
+            let model_env = registry.model_env_var(&backend);
+            let env_var_prefix = format!("{}=", model_env);
+
+            // Only update the .env file if the var is actually set there
+            // (avoid injecting new vars the user never configured).
+            let env_path = crate::bootstrap::ironclaw_env_path();
+            let env_has_var = std::fs::read_to_string(&env_path)
+                .ok()
+                .is_some_and(|content| {
+                    content.lines().any(|line| {
+                        let trimmed = line.trim_start();
+                        !trimmed.starts_with('#') && trimmed.starts_with(&env_var_prefix)
+                    })
+                });
+            if env_has_var {
+                if let Err(e) = crate::bootstrap::upsert_bootstrap_var(model_env, &model_owned) {
+                    tracing::warn!("Failed to update {} in .env: {}", model_env, e);
+                } else {
+                    tracing::debug!("Updated {} in .env to {}", model_env, model_owned);
+                }
+            }
+
+            // 2b. Update (or create) the TOML config file.
+            //
+            // The TOML overlay has higher priority than DB settings on
+            // startup, so it MUST stay in sync with the DB.
             let toml_path = crate::settings::Settings::default_toml_path();
             match crate::settings::Settings::load_toml(&toml_path) {
                 Ok(Some(mut settings)) => {
@@ -856,7 +894,15 @@ impl Agent {
                     }
                 }
                 Ok(None) => {
-                    // No config file on disk; nothing to update.
+                    // No config file yet — create one so the model choice
+                    // survives restarts even when the DB is unavailable.
+                    let settings = crate::settings::Settings {
+                        selected_model: Some(model_owned),
+                        ..Default::default()
+                    };
+                    if let Err(e) = settings.save_toml(&toml_path) {
+                        tracing::warn!("Failed to create config.toml for model persistence: {}", e);
+                    }
                 }
                 Err(e) => {
                     tracing::warn!("Failed to load config.toml for model persistence: {}", e);
@@ -865,7 +911,7 @@ impl Agent {
         })
         .await
         {
-            tracing::warn!("Model TOML persistence task failed: {}", e);
+            tracing::warn!("Model persistence task failed: {}", e);
         }
     }
 }

src/agent/dispatcher.rs

@@ -1233,6 +1233,7 @@ mod tests {
             document_extraction: None,
             sandbox_readiness: crate::agent::routine_engine::SandboxReadiness::DisabledByConfig,
             builder: None,
+            llm_backend: "nearai".to_string(),
         };
 
         Agent::new(
@@ -2100,6 +2101,7 @@ mod tests {
             document_extraction: None,
             sandbox_readiness: crate::agent::routine_engine::SandboxReadiness::DisabledByConfig,
             builder: None,
+            llm_backend: "nearai".to_string(),
         };
 
         Agent::new(
@@ -2220,6 +2222,7 @@ mod tests {
                 document_extraction: None,
                 sandbox_readiness: crate::agent::routine_engine::SandboxReadiness::DisabledByConfig,
                 builder: None,
+                llm_backend: "nearai".to_string(),
             };
 
             Agent::new(

src/main.rs

@@ -912,6 +912,7 @@ async fn async_main() -> anyhow::Result<()> {
             ironclaw::agent::routine_engine::SandboxReadiness::DockerUnavailable
         },
         builder: components.builder,
+        llm_backend: config.llm.backend.clone(),
     };
 
     let channels_for_warnings = Arc::clone(&channels);

src/settings.rs

@@ -1297,6 +1297,92 @@ mod tests {
         assert_eq!(loaded.heartbeat.interval_secs, 900);
     }
 
+    /// Regression: /model writes a single key ("selected_model") to the DB via
+    /// set_setting(). On restart, get_all_settings() returns ALL keys including
+    /// wizard-written defaults. The single-key update must survive the full
+    /// from_db_map() round trip.
+    #[test]
+    fn db_single_key_model_update_survives_roundtrip() {
+        // Step 1: Wizard writes full settings to DB (including selected_model
+        // from initial setup).
+        let wizard_settings = Settings {
+            llm_backend: Some("nearai".to_string()),
+            selected_model: Some("old-wizard-model".to_string()),
+            ..Default::default()
+        };
+        let mut db: std::collections::HashMap<String, serde_json::Value> =
+            wizard_settings.to_db_map();
+
+        // Step 2: User runs /model new-model — persist_selected_model writes
+        // a single key, overwriting the wizard value.
+        db.insert(
+            "selected_model".to_string(),
+            serde_json::Value::String("new-model".to_string()),
+        );
+
+        // Step 3: On restart, from_db_map() rebuilds Settings from the full
+        // DB map.
+        let restored = Settings::from_db_map(&db);
+        assert_eq!(
+            restored.selected_model,
+            Some("new-model".to_string()),
+            "/model change must survive DB round trip"
+        );
+    }
+
+    /// Regression: TOML overlay must not clobber a DB-persisted selected_model
+    /// when the TOML file matches the DB. This is the normal case after /model
+    /// successfully writes to both DB and TOML.
+    #[test]
+    fn toml_overlay_preserves_matching_model() {
+        // DB settings with new model from /model command.
+        let mut db_settings = Settings {
+            llm_backend: Some("nearai".to_string()),
+            selected_model: Some("new-model".to_string()),
+            ..Default::default()
+        };
+
+        // TOML also updated by /model command to the same value.
+        let toml_settings = Settings {
+            selected_model: Some("new-model".to_string()),
+            ..Default::default()
+        };
+
+        db_settings.merge_from(&toml_settings);
+        assert_eq!(
+            db_settings.selected_model,
+            Some("new-model".to_string()),
+            "TOML overlay must not clobber matching model"
+        );
+    }
+
+    /// Regression: when /model updates DB but TOML write fails, a stale TOML
+    /// file would overwrite the DB value. This test documents the priority:
+    /// TOML > DB (by design). persist_selected_model MUST update the TOML.
+    #[test]
+    fn stale_toml_overwrites_db_model() {
+        // DB has the new model from /model.
+        let mut db_settings = Settings {
+            selected_model: Some("new-model".to_string()),
+            ..Default::default()
+        };
+
+        // TOML still has the old model (write failed or was not attempted).
+        let stale_toml = Settings {
+            selected_model: Some("old-model".to_string()),
+            ..Default::default()
+        };
+
+        db_settings.merge_from(&stale_toml);
+        // This documents the current priority: TOML wins over DB.
+        // The fix in persist_selected_model ensures TOML is always updated.
+        assert_eq!(
+            db_settings.selected_model,
+            Some("old-model".to_string()),
+            "TOML overlay has higher priority than DB (by design)"
+        );
+    }
+
     /// 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.
@@ -1322,6 +1408,28 @@ mod tests {
         assert_eq!(reloaded.selected_model, Some("new-model".to_string()));
     }
 
+    /// Regression: /model must create config.toml when it doesn't exist, so the
+    /// model survives restarts. Previously the Ok(None) case was a no-op.
+    #[test]
+    fn toml_created_when_missing_for_model_persist() {
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("config.toml");
+
+        // No config.toml yet (fresh install, no wizard).
+        assert!(Settings::load_toml(&path).unwrap().is_none());
+
+        // Simulate what persist_selected_model now does for the Ok(None) case.
+        let settings = Settings {
+            selected_model: Some("new-model".to_string()),
+            ..Default::default()
+        };
+        settings.save_toml(&path).unwrap();
+
+        // Verify the model survived.
+        let loaded = Settings::load_toml(&path).unwrap().unwrap();
+        assert_eq!(loaded.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"));

src/testing/mod.rs

@@ -563,6 +563,7 @@ impl TestHarnessBuilder {
             document_extraction: None,
             sandbox_readiness: crate::agent::routine_engine::SandboxReadiness::DisabledByConfig,
             builder: None,
+            llm_backend: "nearai".to_string(),
         };
 
         TestHarness {

tests/e2e_telegram_message_routing.rs

@@ -200,6 +200,7 @@ mod tests {
             document_extraction: None,
             sandbox_readiness: ironclaw::agent::SandboxReadiness::DisabledByConfig,
             builder: None,
+            llm_backend: "nearai".to_string(),
         };
 
         let gateway = Arc::new(TestChannel::new());

tests/support/gateway_workflow_harness.rs

@@ -264,6 +264,7 @@ impl GatewayWorkflowHarness {
                 document_extraction: None,
                 sandbox_readiness: ironclaw::agent::SandboxReadiness::DisabledByConfig,
                 builder: None,
+                llm_backend: "nearai".to_string(),
             },
             channels,
             None,

tests/support/test_rig.rs

@@ -761,6 +761,7 @@ impl TestRigBuilder {
             document_extraction: None,
             sandbox_readiness: ironclaw::agent::SandboxReadiness::Available, // tests don't use real Docker
             builder: None,
+            llm_backend: "nearai".to_string(),
         };
 
         // 7. Create TestChannel and ChannelManager.