fix(desktop): preserve agent model/provider when persona snapshot fields are blank (#1583)

desktop/scripts/check-file-sizes.mjs

@@ -60,6 +60,9 @@ const overrides = new Map([
   // config-bridge: get_agent_config_surface/write_agent_config_field/put_agent_session_config
   // commands add ~40 lines. Queued to split.
   // branch cut; override bumped to cover the merged total. Queued to split.
+  // persona-blank-fallback: persona_snapshot_with_agent_config_fallback call
+  // sites add ~4 lines (extra fallback params + inline comments). build_deploy_payload
+  // fix (blank-persona provider/model fallback) adds ~6 lines. Bug fix.
   // archive/mod_tests.rs carries the full test module for archive/mod.rs:
   // unit tests + 4 real-relay integration tests (ignored, live-relay only).
   // Production logic in mod.rs is now ~527 lines (under 1000). mod_tests.rs
@@ -67,7 +70,7 @@ const overrides = new Map([
   // across the local-archive + agent-metric-archive PR series. store_tests.rs
   // (~731 lines) is under 1000 so needs no override.
   ["src-tauri/src/archive/mod_tests.rs", 1208],
-  ["src-tauri/src/commands/agents.rs", 1437],
+  ["src-tauri/src/commands/agents.rs", 1443],
   // #1418 read-path fix: get_thread_replies' blocker fix (shared TIMELINE_KINDS
   // const + build_thread_replies_filter helper, mirroring the channel sibling so
   // the two p-gate filters can't drift) plus two guard unit tests. The file was
@@ -94,7 +97,9 @@ const overrides = new Map([
   // activity-feed threads avatar_url into build_managed_agent_summary for the
   // assistant-bubble pinned snapshot.
   // +1 for agent_pubkey field in setup payload (config-nudge card wire).
-  ["src-tauri/src/managed_agents/runtime.rs", 2208],
+  // persona-blank-fallback: resolve_effective_prompt_model_provider gains a
+  // record_provider param + applies persona_field_with_record_fallback. +5 lines.
+  ["src-tauri/src/managed_agents/runtime.rs", 2213],
   // config-bridge setup-payload env-boundary fix adds readiness wiring in
   // spawn_agent_child; load-bearing security fix, queued to split.
   ["src-tauri/src/managed_agents/config_bridge/reader.rs", 1016],

desktop/src-tauri/src/commands/agent_config.rs

@@ -65,6 +65,7 @@ fn resolve_config_surface(
         personas,
         record.system_prompt.clone(),
         record.model.clone(),
+        record.provider.clone(),
     );
 
     // Build the baseline the reader overrides a live model against, paired with

desktop/src-tauri/src/commands/agents.rs

@@ -258,12 +258,19 @@ async fn start_local_agent_with_preflight(
     // starts with the current persona config (system_prompt, model, provider,
     // env_vars). This clears the "out of date" drift badge without requiring a
     // delete+recreate. Agent-level env_vars overrides still win (persona_snapshot
-    // layers persona env under agent overrides).
+    // layers persona env under agent overrides). When the persona leaves model or
+    // provider blank, the agent record's own configured values are preserved so a
+    // user-set model/provider is never clobbered by an unconfigured persona.
     if let Some(persona_id) = record.persona_id.clone() {
         let personas = load_personas(app).unwrap_or_default();
         if let Some(persona) = personas.iter().find(|p| p.id == persona_id) {
             let snapshot =
-                crate::managed_agents::persona_events::persona_snapshot(persona, &record.env_vars);
+                crate::managed_agents::persona_events::persona_snapshot_with_agent_config_fallback(
+                    persona,
+                    &record.env_vars,
+                    record.model.as_deref(),    // fallback: record.model
+                    record.provider.as_deref(), // fallback: record.provider
+                );
             if let Some(prompt) = snapshot.system_prompt {
                 record.system_prompt = Some(prompt);
             }
@@ -330,9 +337,10 @@ fn build_deploy_payload(
     // stale derived env copies in `env_vars` (or have no provider at all for
     // imported personas whose derived keys were filtered at import time).
     //
-    // Precedence mirrors local spawn: persona structured model is authoritative
-    // when present; the agent record's `model` is a fallback for personas that
-    // don't specify one (or when no persona is linked).
+    // Precedence: persona field wins when non-blank; otherwise falls back to the
+    // record's own field (same blank-normalization as the snapshot path). This
+    // matches `persona_snapshot_with_agent_config_fallback` exactly — a blank
+    // persona field must not wipe a record value that the user configured.
     let (effective_model, effective_provider) = if let Some(pid) = record.persona_id.as_deref() {
         let personas = load_personas(app).map_err(|e| {
             format!(
@@ -343,11 +351,12 @@ fn build_deploy_payload(
             .into_iter()
             .find(|p| p.id == pid)
             .ok_or_else(|| format!("persona `{pid}` not found while building deploy payload"))?;
-        let model = persona.model.clone().or(record.model.clone());
-        let provider = persona.provider;
+        let fallback = crate::managed_agents::persona_events::persona_field_with_record_fallback;
+        let model = fallback(persona.model.as_deref(), record.model.as_deref()); // fallback: record.model
+        let provider = fallback(persona.provider.as_deref(), record.provider.as_deref()); // fallback: record.provider
         (model, provider)
     } else {
-        (record.model.clone(), None)
+        (record.model.clone(), record.provider.clone())
     };
 
     Ok(serde_json::json!({

desktop/src-tauri/src/managed_agents/persona_events.rs

@@ -306,6 +306,24 @@ pub struct PersonaSnapshot {
     pub source_version: String,
 }
 
+/// Apply persona-wins-when-set precedence for a single optional string field.
+///
+/// Returns the persona's value when it is non-`None` and non-whitespace-only;
+/// otherwise falls back to the record's value with the same blank filter applied.
+/// Returns `None` only when both are blank — a genuinely unconfigured field stays
+/// unconfigured.
+///
+/// This is the single source of truth for the precedence rule used by
+/// `persona_snapshot_with_agent_config_fallback`, `build_deploy_payload`, and
+/// `resolve_effective_prompt_model_provider` so the three paths cannot drift.
+pub fn persona_field_with_record_fallback(
+    persona_value: Option<&str>,
+    record_value: Option<&str>,
+) -> Option<String> {
+    let non_blank = |v: Option<&str>| v.filter(|s| !s.trim().is_empty()).map(str::to_owned);
+    non_blank(persona_value).or_else(|| non_blank(record_value))
+}
+
 /// Build the pinned snapshot for an agent created from `persona`.
 ///
 /// `agent_env_overrides` are the agent's own env vars (persona-independent);
@@ -329,6 +347,48 @@ pub fn persona_snapshot(
     }
 }
 
+/// Build the pinned snapshot for an **existing** agent record being re-snapshotted
+/// from its linked persona (on spawn or app-launch restore).
+///
+/// Precedence rule: when the persona sets `model` or `provider` (non-`None`, non-empty),
+/// the persona wins — this is the expected inheritance. When the persona leaves
+/// these fields blank (`None` or empty string), the agent record's own values are
+/// preserved instead. This prevents a persona with no configured model/provider from
+/// clobbering a value the user already set on the agent, which would trap the agent
+/// in a permanent "needs configuration" loop that users cannot escape.
+///
+/// `source_version` is always updated to the current persona content hash so the
+/// drift badge clears correctly even when model/provider are not touched.
+///
+/// Env-var layering is unchanged: persona env < agent env (agent wins on collision).
+/// A persona with an empty env map does not wipe the agent's env vars — the agent's
+/// own overrides are merged on top and always win.
+///
+/// The two fields (`model`, `provider`) are independent: a persona that sets only
+/// `model` wins on `model` while the agent's `provider` is preserved, and vice versa.
+pub fn persona_snapshot_with_agent_config_fallback(
+    persona: &PersonaRecord,
+    agent_env_overrides: &BTreeMap<String, String>,
+    current_agent_model: Option<&str>,
+    current_agent_provider: Option<&str>,
+) -> PersonaSnapshot {
+    // Delegate env-merge, system_prompt, and source_version to persona_snapshot
+    // so future PersonaSnapshot field additions stay automatically consistent.
+    let base = persona_snapshot(persona, agent_env_overrides);
+
+    // Apply the shared precedence rule: persona wins when non-blank, else
+    // the agent record's value is preserved so a configured agent stays configured.
+    let model = persona_field_with_record_fallback(base.model.as_deref(), current_agent_model);
+    let provider =
+        persona_field_with_record_fallback(base.provider.as_deref(), current_agent_provider);
+
+    PersonaSnapshot {
+        model,
+        provider,
+        ..base
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -625,4 +685,244 @@ mod tests {
             persona_content_hash(&content2)
         );
     }
+
+    // ── persona_field_with_record_fallback ────────────────────────────────────
+
+    #[test]
+    fn field_fallback_persona_present_wins() {
+        assert_eq!(
+            persona_field_with_record_fallback(Some("persona-model"), Some("record-model")),
+            Some("persona-model".to_owned()),
+        );
+    }
+
+    #[test]
+    fn field_fallback_persona_blank_uses_record() {
+        assert_eq!(
+            persona_field_with_record_fallback(None, Some("record-model")),
+            Some("record-model".to_owned()),
+        );
+        assert_eq!(
+            persona_field_with_record_fallback(Some("  "), Some("record-model")),
+            Some("record-model".to_owned()),
+        );
+    }
+
+    #[test]
+    fn field_fallback_both_blank_is_none() {
+        assert_eq!(persona_field_with_record_fallback(None, None), None);
+        assert_eq!(persona_field_with_record_fallback(Some(""), Some("")), None);
+    }
+
+    #[test]
+    fn field_fallback_record_blank_is_none() {
+        assert_eq!(
+            persona_field_with_record_fallback(None, Some("  ")),
+            None,
+            "whitespace-only record value must also be treated as blank"
+        );
+    }
+
+    // ── persona_snapshot_with_agent_config_fallback ────────────────────────────
+
+    /// Helper: a persona with no model/provider configured.
+    fn blank_model_persona() -> PersonaRecord {
+        PersonaRecord {
+            model: None,
+            provider: None,
+            ..sample_persona()
+        }
+    }
+
+    /// (a) Persona leaves model/provider blank, agent record has values →
+    /// record values preserved AND source_version still updated to current hash.
+    #[test]
+    fn fallback_preserves_record_values_when_persona_blank() {
+        let persona = blank_model_persona();
+        let expected_version = persona_content_hash(&persona_event_content(&persona));
+
+        let snapshot = persona_snapshot_with_agent_config_fallback(
+            &persona,
+            &BTreeMap::new(),
+            Some("gpt-4o"),
+            Some("openai"),
+        );
+
+        assert_eq!(
+            snapshot.model.as_deref(),
+            Some("gpt-4o"),
+            "blank persona model must fall back to agent record value"
+        );
+        assert_eq!(
+            snapshot.provider.as_deref(),
+            Some("openai"),
+            "blank persona provider must fall back to agent record value"
+        );
+        assert_eq!(
+            snapshot.source_version, expected_version,
+            "source_version must still reflect current persona hash"
+        );
+    }
+
+    /// (b) Persona has model/provider set → persona wins over agent record.
+    #[test]
+    fn fallback_persona_wins_when_set() {
+        let persona = sample_persona(); // has model=Some("claude-opus-4"), provider=Some("anthropic")
+
+        let snapshot = persona_snapshot_with_agent_config_fallback(
+            &persona,
+            &BTreeMap::new(),
+            Some("gpt-4o"), // agent had a different model
+            Some("openai"), // agent had a different provider
+        );
+
+        assert_eq!(
+            snapshot.model.as_deref(),
+            Some("claude-opus-4"),
+            "persona model must win when persona has a value"
+        );
+        assert_eq!(
+            snapshot.provider.as_deref(),
+            Some("anthropic"),
+            "persona provider must win when persona has a value"
+        );
+    }
+
+    /// (c) Both blank → snapshot keeps None; a genuinely unconfigured agent
+    /// stays unconfigured (no fabricated values).
+    #[test]
+    fn fallback_both_blank_stays_none() {
+        let persona = blank_model_persona();
+
+        let snapshot = persona_snapshot_with_agent_config_fallback(
+            &persona,
+            &BTreeMap::new(),
+            None, // agent also has no model
+            None, // agent also has no provider
+        );
+
+        assert!(
+            snapshot.model.is_none(),
+            "neither persona nor agent has model — snapshot must be None"
+        );
+        assert!(
+            snapshot.provider.is_none(),
+            "neither persona nor agent has provider — snapshot must be None"
+        );
+    }
+
+    /// Whitespace-only values on the persona are treated as blank; agent
+    /// fallback applies.
+    #[test]
+    fn fallback_treats_whitespace_only_persona_value_as_blank() {
+        let mut persona = sample_persona();
+        persona.model = Some("  ".to_string());
+        persona.provider = Some("\t".to_string());
+
+        let snapshot = persona_snapshot_with_agent_config_fallback(
+            &persona,
+            &BTreeMap::new(),
+            Some("claude-opus-4"),
+            Some("anthropic"),
+        );
+
+        assert_eq!(
+            snapshot.model.as_deref(),
+            Some("claude-opus-4"),
+            "whitespace-only persona model must be treated as blank"
+        );
+        assert_eq!(
+            snapshot.provider.as_deref(),
+            Some("anthropic"),
+            "whitespace-only persona provider must be treated as blank"
+        );
+    }
+
+    /// Cross-field independence: persona sets model but not provider → model
+    /// comes from persona, provider falls back to the record.  This is the
+    /// practically common case (model-only personas).
+    #[test]
+    fn fallback_persona_model_set_provider_blank_uses_record_provider() {
+        let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic")
+        persona.provider = None; // blank provider on persona
+
+        let snapshot = persona_snapshot_with_agent_config_fallback(
+            &persona,
+            &BTreeMap::new(),
+            Some("gpt-4o"), // record model (should be overridden by persona)
+            Some("openai"), // record provider (should be preserved)
+        );
+
+        assert_eq!(
+            snapshot.model.as_deref(),
+            Some("claude-opus-4"),
+            "persona model must win when persona has a value"
+        );
+        assert_eq!(
+            snapshot.provider.as_deref(),
+            Some("openai"),
+            "record provider must be used when persona provider is blank"
+        );
+    }
+
+    /// Inverse: persona sets provider but not model → provider comes from
+    /// persona, model falls back to the record.
+    #[test]
+    fn fallback_persona_provider_set_model_blank_uses_record_model() {
+        let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic")
+        persona.model = None; // blank model on persona
+
+        let snapshot = persona_snapshot_with_agent_config_fallback(
+            &persona,
+            &BTreeMap::new(),
+            Some("gpt-4o"), // record model (should be preserved)
+            Some("openai"), // record provider (should be overridden by persona)
+        );
+
+        assert_eq!(
+            snapshot.model.as_deref(),
+            Some("gpt-4o"),
+            "record model must be used when persona model is blank"
+        );
+        assert_eq!(
+            snapshot.provider.as_deref(),
+            Some("anthropic"),
+            "persona provider must win when persona has a value"
+        );
+    }
+
+    /// Env-var layering: persona env < agent env — agent overrides always win
+    /// on key collision and a persona with an empty env map does not wipe the
+    /// agent's env vars.
+    #[test]
+    fn fallback_agent_env_wins_over_persona_env() {
+        let mut persona = blank_model_persona();
+        persona.env_vars = BTreeMap::from([
+            ("SHARED_KEY".to_string(), "persona-value".to_string()),
+            ("PERSONA_ONLY".to_string(), "from-persona".to_string()),
+        ]);
+        let agent_env = BTreeMap::from([
+            ("SHARED_KEY".to_string(), "agent-override".to_string()),
+            ("AGENT_ONLY".to_string(), "from-agent".to_string()),
+        ]);
+
+        let snapshot =
+            persona_snapshot_with_agent_config_fallback(&persona, &agent_env, None, None);
+
+        assert_eq!(
+            snapshot.env_vars.get("SHARED_KEY").map(String::as_str),
+            Some("agent-override"),
+            "agent env must win on key collision"
+        );
+        assert_eq!(
+            snapshot.env_vars.get("PERSONA_ONLY").map(String::as_str),
+            Some("from-persona"),
+            "persona-only key must be present"
+        );
+        assert_eq!(
+            snapshot.env_vars.get("AGENT_ONLY").map(String::as_str),
+            Some("from-agent"),
+            "agent-only key must be present"
+        );
+    }
 }

desktop/src-tauri/src/managed_agents/restore.rs

@@ -19,6 +19,10 @@ type AgentSpawnResult = (String, SpawnResult);
 /// empty snapshot.
 ///
 /// Only records with a `persona_id` but no `persona_source_version` are touched.
+/// Records that already have a `persona_source_version` — including those whose
+/// `model`/`provider` were clobbered by the old unconditional snapshot code before
+/// this fix — are skipped here; they self-heal on the next manual start via the
+/// start-path re-snapshot in `start_local_agent_with_preflight`.
 /// If the linked persona is gone, we log loudly and leave the snapshot empty —
 /// the record's own `system_prompt`/`model` (possibly empty for persona-created
 /// agents) is then all the config that remains, which is the same fallback an
@@ -55,8 +59,15 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String>
             continue;
         };
         // Layer the agent's own env overrides over persona env, matching
-        // create-time precedence (persona env < agent env).
-        let snapshot = super::persona_events::persona_snapshot(persona, &record.env_vars);
+        // create-time precedence (persona env < agent env). When the persona
+        // leaves model/provider blank, the record's own configured values are
+        // preserved — a blank persona must not clobber a user-configured agent.
+        let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback(
+            persona,
+            &record.env_vars,
+            record.model.as_deref(),    // fallback: record.model
+            record.provider.as_deref(), // fallback: record.provider
+        );
         if let Some(prompt) = snapshot.system_prompt {
             record.system_prompt = Some(prompt);
         }
@@ -181,7 +192,12 @@ pub async fn restore_managed_agents_on_launch(
             let Some(persona) = personas_for_snapshot.iter().find(|p| p.id == persona_id) else {
                 continue;
             };
-            let snapshot = super::persona_events::persona_snapshot(persona, &record.env_vars);
+            let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback(
+                persona,
+                &record.env_vars,
+                record.model.as_deref(),    // fallback: record.model
+                record.provider.as_deref(), // fallback: record.provider
+            );
             if let Some(prompt) = snapshot.system_prompt {
                 record.system_prompt = Some(prompt);
             }

desktop/src-tauri/src/managed_agents/runtime.rs

@@ -2025,8 +2025,11 @@ pub(crate) fn runtime_metadata_env_vars<'a>(
 /// Resolve the effective (prompt, model, provider) triple for a persona-linked agent.
 ///
 /// Given a persona_id, finds the persona in the list and returns its system_prompt,
-/// model, and provider as the authoritative values. Falls back to the record's own
-/// prompt/model and None for provider when no persona is linked or found.
+/// model, and provider as the authoritative values. When the persona leaves `model`
+/// or `provider` blank (None or whitespace-only), falls back to the record's own
+/// field using the same precedence rule as `persona_snapshot_with_agent_config_fallback`
+/// so the display surface matches spawn behavior. Falls back to the record's own
+/// prompt/model/provider when no persona is linked or found.
 ///
 /// Used by `agent_config.rs` to inject persona defaults into the config surface
 /// before running the reader, so BuzzExplicit-tagged fields can be re-tagged to
@@ -2036,14 +2039,16 @@ pub(crate) fn resolve_effective_prompt_model_provider(
     personas: &[crate::managed_agents::types::PersonaRecord],
     record_prompt: Option<String>,
     record_model: Option<String>,
+    record_provider: Option<String>,
 ) -> (Option<String>, Option<String>, Option<String>) {
+    let fallback = crate::managed_agents::persona_events::persona_field_with_record_fallback;
     match persona_id.and_then(|pid| personas.iter().find(|p| p.id == pid)) {
         Some(p) => (
             Some(p.system_prompt.clone()),
-            p.model.clone(),
-            p.provider.clone(),
+            fallback(p.model.as_deref(), record_model.as_deref()), // fallback: record.model
+            fallback(p.provider.as_deref(), record_provider.as_deref()), // fallback: record.provider
         ),
-        None => (record_prompt, record_model, None),
+        None => (record_prompt, record_model, record_provider),
     }
 }