fix(desktop): make agent definition authoritative for model/provider/prompt

desktop/scripts/check-file-sizes.mjs

@@ -123,8 +123,7 @@ 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).
-  // persona-blank-fallback: resolve_effective_prompt_model_provider gains a
-  // record_provider param + applies persona_field_with_record_fallback. +5 lines.
+  // effective-config resolver: single resolver replaces the old fallback chain.
   // global-agent-config: spawn_agent_child loads global config and merges as
   // lowest env layer (+8 lines). Queued to split.
   ["src-tauri/src/managed_agents/runtime.rs", 2216],
@@ -208,7 +207,8 @@ const overrides = new Map([
   // mapper. This is the existing API boundary; split remains queued.
   // team-instructions-first-class: createManagedAgent Tauri bridge threads the
   // new teamId input through to the backend (+1 line).
-  ["src/shared/api/tauri.ts", 1305],
+  // +2 for model_source field in RawManagedAgent + fromRawManagedAgent mapping.
+  ["src/shared/api/tauri.ts", 1307],
   // doctor-npm-eacces-preflight: hint field added to InstallStepResult (+1 line).
   // codex-acp-package-swap: "adapter_outdated" variant added to AcpAvailabilityStatus (+1 line).
   // doctor-install-reliability: AuthStatus tagged union + nodeRequired/authStatus/
@@ -442,15 +442,28 @@ const overrides = new Map([
   // (if let Some(provider_update) = input.provider { record.provider = provider_update; }).
   // +8: harness_override thread-through in update_managed_agent so a deliberate
   // Custom pin routes to update_time_agent_command_override (comment + call).
-  ["src-tauri/src/commands/agent_models.rs", 1079],
+  // +22 (1079 -> 1101): Finding 2 — model discovery now resolves through
+  // resolve_effective_model_provider instead of raw record bytes
+  // (saved_agent_model_discovery_config takes personas/global and the
+  // get_agent_models call site loads global config), plus
+  // apply_model_provider_prompt_update's linked-instance write-guard
+  // extraction and its regression tests.
+  ["src-tauri/src/commands/agent_models.rs", 1101],
   // global-agent-config: get_agent_config_surface / write_agent_config_field /
   // put_agent_session_config commands + GlobalAgentConfig serde types. New file
   // in this PR; queued to split with the command module refactor.
   // +17: baked-env-global-unify: BUZZ_AGENT_THINKING_EFFORT added to
   // is_safe_to_reveal allowlist + baked_env_thinking_effort_is_unmasked test.
   // +1: doctor-install-reliability: login_hint: None added to goose_runtime test stub.
   // +1: doctor-install-reliability review fixes: auth_probe_args: None added to stub.
-  ["src-tauri/src/commands/agent_config.rs", 1021],
+  // +59 (1021 -> 1080): Finding 2 (definition-authoritative config surface) —
+  // resolve_config_surface now clears a linked instance's own
+  // system_prompt/model/provider before computing had_* so stale
+  // materialized snapshot bytes can never be tagged BuzzExplicit and
+  // shadow the definition/global fallthrough; the dead persona-model
+  // re-tag branch this replaced is removed, plus two new regression tests
+  // pinning the stale-record-never-outranks-definition contract.
+  ["src-tauri/src/commands/agent_config.rs", 1080],
   // codex-install-auto-restart review-fixes: should_restart_after_install
   // takes pid_alive:bool (pure predicate, no OS-dependent call); 3 racy
   // cache tests replaced with 6 pure availability_drift predicate tests;

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

@@ -38,6 +38,13 @@ pub struct RuntimeFileConfigSubset {
 
 /// Resolve the config surface with persona and global default values applied.
 ///
+/// Linked instances are definition-authoritative: the record's own
+/// system_prompt/model/provider are cleared before applying, so a stale
+/// materialized snapshot can never shadow the persona's current values or a
+/// blank-definition fallthrough to global defaults (mirrors
+/// `effective_config::resolve_linked`). Definition-less instances keep their
+/// own explicit values.
+///
 /// The pipeline: resolve the linked persona's prompt/model/provider, inject
 /// each into the record only where the record lacks its own value, let
 /// `read_config_surface` tag those injected fields `BuzzExplicit`, then re-tag
@@ -58,6 +65,22 @@ fn resolve_config_surface(
     session_cache: Option<&SessionConfigCache>,
     global: &GlobalAgentConfig,
 ) -> RuntimeConfigSurface {
+    // Linked instances are definition-authoritative (mirrors
+    // `effective_config::resolve_linked`): the record's own
+    // system_prompt/model/provider fields are, at best, a stale materialized
+    // snapshot from the last `apply_persona_snapshot` — never a legitimate
+    // live override, since `update_managed_agent` blocks writing these three
+    // fields for linked instances. Clear them before computing `had_*` below
+    // so a stale byte can never masquerade as BuzzExplicit and suppress
+    // definition/global injection. Env var overrides (set via the advanced
+    // env-vars editor) are untouched — those remain a legitimate
+    // per-instance override regardless of link status.
+    if record.persona_id.is_some() {
+        record.system_prompt = None;
+        record.model = None;
+        record.provider = None;
+    }
+
     let had_prompt =
         record.system_prompt.is_some() || record.env_vars.contains_key("BUZZ_ACP_SYSTEM_PROMPT");
     let had_model = record.model.is_some();
@@ -171,22 +194,6 @@ fn resolve_config_surface(
         retag_global_default(&mut surface.normalized.provider);
     }
 
-    // Re-tag persona-snapshotted model from BuzzExplicit to PersonaDefault.
-    // Persona-created agents have record.model set at create time from the
-    // persona snapshot — had_model is true, but the model came from the persona,
-    // not an explicit user choice. Re-tag when the record model matches the
-    // persona model and no live override is active. Only applies when a persona
-    // is actually linked — non-persona agents with an explicit model keep BuzzExplicit.
-    if had_model && !model_overridden && record.persona_id.is_some() {
-        if let (Some(ref record_model), Some(ref persona_model_val)) =
-            (&record.model, &persona_model)
-        {
-            if record_model == persona_model_val {
-                retag_persona_default(&mut surface.normalized.model);
-            }
-        }
-    }
-
     surface
 }
 
@@ -699,11 +706,64 @@ mod tests {
         }
     }
 
-    /// A model the user set explicitly in Buzz must never be re-tagged to
-    /// `PersonaDefault`, even when the linked persona also has a model.
+    /// Definition-authoritative: a stale materialized `record.model` on a
+    /// linked instance must never outrank (or even be consulted against) the
+    /// linked persona's model. `update_managed_agent` already blocks writing
+    /// model/provider/prompt for linked instances, so a non-`None` value here
+    /// can only be leftover snapshot bytes from before a persona edit — the
+    /// panel must report the persona's current model, tagged `PersonaDefault`,
+    /// not the stale byte as `BuzzExplicit`.
+    #[test]
+    fn linked_stale_record_model_never_outranks_persona_model() {
+        let mut record = agent_record();
+        record.model = Some("stale-explicit-model".to_string());
+        let personas = vec![persona_with_model("persona-model")];
+
+        let surface = resolve_config_surface(
+            record,
+            &personas,
+            Some(goose_runtime()),
+            None,
+            &Default::default(),
+        );
+
+        let model = surface.normalized.model.as_ref().expect("model resolved");
+        assert_eq!(model.value.as_deref(), Some("persona-model"));
+        assert_eq!(model.origin, ConfigOrigin::PersonaDefault);
+    }
+
+    /// Definition-authoritative, blank-definition case: a linked instance
+    /// whose persona has no model of its own must fall through to the global
+    /// default, tagged `GlobalDefault` — mirroring
+    /// `effective_config::resolve_linked`'s `None => global` arm. A stale
+    /// materialized record model must not shadow this fallthrough either.
     #[test]
-    fn explicit_record_model_outranks_persona_and_keeps_buzz_explicit_origin() {
+    fn linked_blank_definition_model_falls_through_to_global_default() {
         let mut record = agent_record();
+        record.model = Some("stale-explicit-model".to_string());
+        let mut persona = persona_with_model("unused");
+        persona.model = None;
+        let personas = vec![persona];
+        let global = crate::managed_agents::GlobalAgentConfig {
+            model: Some("global-model".to_string()),
+            ..Default::default()
+        };
+
+        let surface =
+            resolve_config_surface(record, &personas, Some(goose_runtime()), None, &global);
+
+        let model = surface.normalized.model.as_ref().expect("model resolved");
+        assert_eq!(model.value.as_deref(), Some("global-model"));
+        assert_eq!(model.origin, ConfigOrigin::GlobalDefault);
+    }
+
+    /// A definition-less (no `persona_id`) instance's own explicit model IS
+    /// authoritative — the stale-record clearing above is scoped to linked
+    /// instances only.
+    #[test]
+    fn definition_less_explicit_record_model_keeps_buzz_explicit_origin() {
+        let mut record = agent_record();
+        record.persona_id = None;
         record.model = Some("explicit-model".to_string());
         let personas = vec![persona_with_model("persona-model")];
 

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

@@ -71,9 +71,13 @@ pub async fn get_agent_models(
 
         // ModelPicker can persist a selected model but not rewrite the saved
         // provider/env snapshot, and runtime spawn reads that same snapshot.
-        // Discover models against the record snapshot so an out-of-date persona
-        // cannot offer models for a provider this agent will not launch with.
-        let discovery = saved_agent_model_discovery_config(record, &effective_command);
+        // Discover models against the resolver's effective model/provider —
+        // definition-authoritative for linked instances — so discovery can
+        // never query a stale provider this agent will not actually launch
+        // with.
+        let global = crate::managed_agents::load_global_agent_config(&app).unwrap_or_default();
+        let discovery =
+            saved_agent_model_discovery_config(record, &effective_command, &personas, &global);
 
         (
             resolved,
@@ -136,26 +140,37 @@ struct SavedAgentModelDiscoveryConfig {
     env: BTreeMap<String, String>,
 }
 
+/// Resolve the model/provider discovery config from the same authoritative
+/// source spawn uses (`resolve_effective_model_provider`) — linked instances
+/// read their definition, never a stale materialized `record.model`/
+/// `record.provider`, so model discovery cannot query a provider this agent
+/// will not actually launch with. Definition-less instances keep their own
+/// record values, matching spawn's `resolve_definition_less` arm.
 fn saved_agent_model_discovery_config(
     record: &crate::managed_agents::ManagedAgentRecord,
     agent_command: &str,
+    personas: &[crate::managed_agents::AgentDefinition],
+    global: &crate::managed_agents::GlobalAgentConfig,
 ) -> SavedAgentModelDiscoveryConfig {
+    let (effective_model, effective_provider) =
+        crate::managed_agents::resolve_effective_model_provider(record, personas, global);
+
     let mut derived_env = BTreeMap::new();
     if let Some(meta) = known_acp_runtime(agent_command) {
         for (key, value) in crate::managed_agents::runtime_metadata_env_vars(
             meta.model_env_var,
             meta.provider_env_var,
             meta.provider_locked,
-            record.model.as_deref(),
-            record.provider.as_deref(),
+            effective_model.as_deref(),
+            effective_provider.as_deref(),
         ) {
             derived_env.insert(key.to_string(), value.to_string());
         }
     }
 
     SavedAgentModelDiscoveryConfig {
-        model: record.model.clone(),
-        provider: record.provider.clone(),
+        model: effective_model,
+        provider: effective_provider,
         env: crate::managed_agents::merged_user_env(&derived_env, &record.env_vars),
     }
 }
@@ -766,6 +781,35 @@ async fn discover_databricks_models(
     }))
 }
 
+/// Apply an `UpdateManagedAgentRequest`'s model/provider/system_prompt patch
+/// to `record`, enforcing the linked-instance write guard: a definition-linked
+/// record's model/provider/prompt are definition-authoritative (see
+/// `effective_config::resolve_linked`), so writes to these three fields are
+/// silently dropped for a linked instance rather than persisting a byte the
+/// resolver will never read. Definition-less instances accept the patch
+/// as-is. Extracted so the guard is exercised by both `update_managed_agent`
+/// and its regression tests — a test that reimplements this check instead of
+/// calling it can go green after the real guard is deleted.
+fn apply_model_provider_prompt_update(
+    record: &mut crate::managed_agents::ManagedAgentRecord,
+    model: Option<Option<String>>,
+    provider: Option<Option<String>>,
+    system_prompt: Option<Option<String>>,
+) {
+    if record.persona_id.is_some() {
+        return;
+    }
+    if let Some(model_update) = model {
+        record.model = model_update;
+    }
+    if let Some(provider_update) = provider {
+        record.provider = provider_update;
+    }
+    if let Some(prompt_update) = system_prompt {
+        record.system_prompt = prompt_update;
+    }
+}
+
 /// Update mutable fields on an existing managed agent record.
 ///
 /// Does NOT auto-restart the agent. Runtime config changes (system prompt,
@@ -804,15 +848,12 @@ pub async fn update_managed_agent(
                 name_changed = true;
             }
         }
-        if let Some(model_update) = input.model {
-            record.model = model_update;
-        }
-        if let Some(provider_update) = input.provider {
-            record.provider = provider_update;
-        }
-        if let Some(prompt_update) = input.system_prompt {
-            record.system_prompt = prompt_update;
-        }
+        apply_model_provider_prompt_update(
+            record,
+            input.model,
+            input.provider,
+            input.system_prompt,
+        );
         if let Some(parallelism) = input.parallelism {
             record.parallelism = parallelism;
         }