From 791222d276c69de43db91a1fe778e36041714676 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 11:34:51 -0400 Subject: [PATCH 1/3] fix(desktop): preserve agent model/provider when persona snapshot fields are blank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a persona has no configured model/provider, the existing re-snapshot logic unconditionally wrote the blank values onto the agent record at every spawn and app-launch restore. This clobbered any values the user had set on the agent, and the readiness gate would immediately flag the agent as needing configuration — a loop users could not escape. Fix: add `persona_snapshot_with_agent_config_fallback` which applies the same persona-wins precedence for non-blank values but preserves the agent record's existing model/provider when the persona leaves them blank. Swap all three call sites (agent start, restore backfill, restore re-snapshot). Both-blank stays None so a genuinely unconfigured agent still reads as needing setup. --- desktop/scripts/check-file-sizes.mjs | 5 +- desktop/src-tauri/src/commands/agents.rs | 11 +- .../src/managed_agents/persona_events.rs | 204 ++++++++++++++++++ .../src-tauri/src/managed_agents/restore.rs | 18 +- 4 files changed, 232 insertions(+), 6 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 54e51ebbdb..5331cb96ef 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -67,7 +67,10 @@ 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], + // persona-blank-fallback: persona_snapshot_with_agent_config_fallback call + // sites in start_local_agent_with_preflight add ~3 lines vs the old + // persona_snapshot calls (extra fallback params). Load-bearing bug fix. + ["src-tauri/src/commands/agents.rs", 1441], // #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 diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 475df6e7f7..1fe34685bb 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/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(), + record.provider.as_deref(), + ); if let Some(prompt) = snapshot.system_prompt { record.system_prompt = Some(prompt); } diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index 7ce7ec0fdb..e3008cec71 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -329,6 +329,60 @@ 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. +pub fn persona_snapshot_with_agent_config_fallback( + persona: &PersonaRecord, + agent_env_overrides: &BTreeMap, + current_agent_model: Option<&str>, + current_agent_provider: Option<&str>, +) -> PersonaSnapshot { + let mut env_vars = persona.env_vars.clone(); + for (key, value) in agent_env_overrides { + env_vars.insert(key.clone(), value.clone()); + } + + // Persona wins when it has a non-blank value; agent record is the fallback + // for blank persona fields so a configured agent stays configured. + let is_present = |v: &Option| v.as_deref().is_some_and(|s| !s.trim().is_empty()); + let model = if is_present(&persona.model) { + persona.model.clone() + } else { + current_agent_model + .filter(|s| !s.trim().is_empty()) + .map(str::to_owned) + }; + let provider = if is_present(&persona.provider) { + persona.provider.clone() + } else { + current_agent_provider + .filter(|s| !s.trim().is_empty()) + .map(str::to_owned) + }; + + PersonaSnapshot { + system_prompt: Some(persona.system_prompt.clone()), + model, + provider, + env_vars, + source_version: persona_content_hash(&persona_event_content(persona)), + } +} + #[cfg(test)] mod tests { use super::*; @@ -625,4 +679,154 @@ mod tests { persona_content_hash(&content2) ); } + + // ── 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" + ); + } + + /// 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" + ); + } } diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 85f5391128..b8c2b831b8 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -55,8 +55,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(), + record.provider.as_deref(), + ); if let Some(prompt) = snapshot.system_prompt { record.system_prompt = Some(prompt); } @@ -170,7 +177,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(), + record.provider.as_deref(), + ); if let Some(prompt) = snapshot.system_prompt { record.system_prompt = Some(prompt); } From 33f0e0d5d4d4a9ef5e1462767c3203644d3ab021 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 12:25:46 -0400 Subject: [PATCH 2/3] refactor(desktop): extract persona_field_with_record_fallback and fix deploy/display paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce `persona_field_with_record_fallback` in `persona_events.rs` as the single source of truth for the precedence rule: persona field wins when non-blank (trimmed), otherwise falls back to the record's own field (also blank-normalized). Three paths previously duplicated or lacked this rule: - `persona_snapshot_with_agent_config_fallback`: refactored to call `persona_snapshot` for env/system_prompt/source_version, then patch model/provider via the shared helper — future PersonaSnapshot field additions stay consistent automatically. - `build_deploy_payload`: was using `persona.model.or(record.model)` with no blank normalization and dropped `record.provider` entirely when the persona had a blank provider. Provider-backend agents with a blank persona provider now correctly deploy with the record's configured provider. - `resolve_effective_prompt_model_provider`: was returning live persona fields with no record fallback, so the config/display surface could show an agent as unconfigured while the spawn path ran it with the record's values. Now matches spawn behavior by applying the same precedence rule. Also: extend `backfill_persona_snapshots` doc comment to note already-snapshotted records (persona_source_version set) self-heal on next manual start; add inline fallback-arg comments at all three call sites; update file-size overrides. --- desktop/scripts/check-file-sizes.mjs | 12 ++- .../src-tauri/src/commands/agent_config.rs | 1 + desktop/src-tauri/src/commands/agents.rs | 18 ++-- .../src/managed_agents/persona_events.rs | 91 ++++++++++++++----- .../src-tauri/src/managed_agents/restore.rs | 12 ++- .../src-tauri/src/managed_agents/runtime.rs | 15 ++- 6 files changed, 103 insertions(+), 46 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 5331cb96ef..32e7554e6b 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/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,10 +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], - // persona-blank-fallback: persona_snapshot_with_agent_config_fallback call - // sites in start_local_agent_with_preflight add ~3 lines vs the old - // persona_snapshot calls (extra fallback params). Load-bearing bug fix. - ["src-tauri/src/commands/agents.rs", 1441], + ["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 +94,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], diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 59547764db..09c18b68bc 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/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 diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 1fe34685bb..c8b5c7bcba 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -268,8 +268,8 @@ async fn start_local_agent_with_preflight( crate::managed_agents::persona_events::persona_snapshot_with_agent_config_fallback( persona, &record.env_vars, - record.model.as_deref(), - record.provider.as_deref(), + 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); @@ -337,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!( @@ -350,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!({ diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index e3008cec71..ed05fadb27 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/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 { + 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); @@ -345,41 +363,29 @@ pub fn persona_snapshot( /// 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, current_agent_model: Option<&str>, current_agent_provider: Option<&str>, ) -> PersonaSnapshot { - let mut env_vars = persona.env_vars.clone(); - for (key, value) in agent_env_overrides { - env_vars.insert(key.clone(), value.clone()); - } + // 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); - // Persona wins when it has a non-blank value; agent record is the fallback - // for blank persona fields so a configured agent stays configured. - let is_present = |v: &Option| v.as_deref().is_some_and(|s| !s.trim().is_empty()); - let model = if is_present(&persona.model) { - persona.model.clone() - } else { - current_agent_model - .filter(|s| !s.trim().is_empty()) - .map(str::to_owned) - }; - let provider = if is_present(&persona.provider) { - persona.provider.clone() - } else { - current_agent_provider - .filter(|s| !s.trim().is_empty()) - .map(str::to_owned) - }; + // 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 { - system_prompt: Some(persona.system_prompt.clone()), model, provider, - env_vars, - source_version: persona_content_hash(&persona_event_content(persona)), + ..base } } @@ -680,6 +686,43 @@ mod tests { ); } + // ── 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. diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index b8c2b831b8..d87e61e555 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/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 @@ -61,8 +65,8 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String> let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback( persona, &record.env_vars, - record.model.as_deref(), - record.provider.as_deref(), + 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); @@ -180,8 +184,8 @@ pub async fn restore_managed_agents_on_launch( let snapshot = super::persona_events::persona_snapshot_with_agent_config_fallback( persona, &record.env_vars, - record.model.as_deref(), - record.provider.as_deref(), + 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); diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index dbf3bc2301..17773f1475 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -2181,8 +2181,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 @@ -2192,14 +2195,16 @@ pub(crate) fn resolve_effective_prompt_model_provider( personas: &[crate::managed_agents::types::PersonaRecord], record_prompt: Option, record_model: Option, + record_provider: Option, ) -> (Option, Option, Option) { + 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), } } From c1a80cebe11fb02ec116998fe5254384f81892b6 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 7 Jul 2026 12:26:22 -0400 Subject: [PATCH 3/3] test(desktop): add cross-field independence tests for persona snapshot fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two fields (model, provider) are independent in the fallback rule — a persona that sets only model wins on model while the agent's provider is preserved, and vice versa. This is the practically common case (model-only personas). Add two tests pinning both directions of the asymmetric cross-field case so regression in either field's independence is immediately visible. --- .../src/managed_agents/persona_events.rs | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index ed05fadb27..e58f7499a4 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -838,6 +838,59 @@ mod tests { ); } + /// 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.