From 47a60a7eac0128c88d65fd26fabf03e97f84ea39 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Fri, 10 Jul 2026 11:17:24 -0400 Subject: [PATCH 01/10] fix(desktop): propagate definition runtime edits to linked agent instances The unified-agent materialization left definition-runtime edits inert once an instance carried a materialized runtime: the spawn re-pin and spawn_config_hash prospective re-snapshot copied model/provider but not runtime, so the stale instance value shadowed the definition via ladder step 2 with no drift badge and no auto-restart. The Inherit sentinel in the update command also left a stale materialized runtime that contradicted the just-expressed inherit intent until the next spawn. Adds runtime to PersonaSnapshot, applies it verbatim from the persona at all four re-snapshot sites (spawn, backfill, launch-restore, spawn_config_hash), and clears record.runtime when the Inherit sentinel clears agent_command_override. --- .../src-tauri/src/commands/agent_models.rs | 7 +++ desktop/src-tauri/src/commands/agents.rs | 1 + .../src/managed_agents/persona_events.rs | 7 +++ .../managed_agents/persona_events/tests.rs | 27 +++++++++ .../src-tauri/src/managed_agents/restore.rs | 2 + .../src/managed_agents/spawn_hash.rs | 1 + .../src/managed_agents/spawn_hash/tests.rs | 56 +++++++++++++++++++ 7 files changed, 101 insertions(+) diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 17938e0b90..e994c7efe8 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -871,6 +871,13 @@ pub async fn update_managed_agent( Some(&agent_command), input.harness_override, ); + // The empty/whitespace sentinel means "Inherit runtime from + // persona": clear the materialized record runtime so the resolution + // ladder falls through to the live definition rather than silently + // keeping the stale instance copy. + if agent_command.trim().is_empty() { + record.runtime = None; + } } if let Some(agent_args) = input.agent_args { record.agent_args = agent_args; diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 1300b97d61..e6409bd712 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -276,6 +276,7 @@ async fn start_local_agent_with_preflight( } record.model = snapshot.model; record.provider = snapshot.provider; + record.runtime = snapshot.runtime; // Self-heal records written before env refresh: persona env used to // be baked into `record.env_vars` at create/spawn, turning inherited // values into pseudo-overrides that shadow later persona edits. An diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index faa9507989..bbc89f7a13 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -334,6 +334,12 @@ pub struct PersonaSnapshot { pub system_prompt: Option, pub model: Option, pub provider: Option, + /// Preferred ACP runtime ID, copied verbatim from the persona (including + /// `None`). Unlike `model`/`provider`, there is no record-fallback: the + /// materialized instance `runtime` must mirror the definition so that + /// definition edits propagate on the next spawn rather than being silently + /// shadowed by the stale materialized value. + pub runtime: Option, /// `persona_content_hash` of the persona at snapshot time; the drift basis. pub source_version: String, } @@ -367,6 +373,7 @@ pub fn persona_snapshot(persona: &PersonaRecord) -> PersonaSnapshot { system_prompt: Some(persona.system_prompt.clone()), model: persona.model.clone(), provider: persona.provider.clone(), + runtime: persona.runtime.clone(), source_version: persona_content_hash(&persona_event_content(persona)), } } diff --git a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs index 87b37c3fb1..d6e46edcc3 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -484,6 +484,33 @@ fn field_fallback_record_blank_is_none() { ); } +// ── PersonaSnapshot.runtime ─────────────────────────────────────────────── + +/// (b) The snapshot carries the persona's runtime VERBATIM — including None, +/// which clears a stale materialized value on the instance record. Unlike +/// model/provider, runtime does not fall back to the record's own value: +/// instances have no user-owned runtime, so the definition must stay +/// authoritative. +#[test] +fn snapshot_runtime_verbatim_from_persona() { + let persona = sample_persona(); // runtime = Some("goose") + let snap = persona_snapshot_with_agent_config_fallback(&persona, Some("gpt-4"), Some("openai")); + assert_eq!( + snap.runtime.as_deref(), + Some("goose"), + "persona runtime Some must be copied verbatim into snapshot" + ); + + let mut no_runtime = sample_persona(); + no_runtime.runtime = None; + let snap = + persona_snapshot_with_agent_config_fallback(&no_runtime, Some("gpt-4"), Some("openai")); + assert_eq!( + snap.runtime, None, + "persona runtime None must produce None snapshot (clears stale materialized value)" + ); +} + // ── 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 1abefe8dac..9039ef4db2 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -72,6 +72,7 @@ pub fn backfill_persona_snapshots(app: &tauri::AppHandle) -> Result<(), String> } record.model = snapshot.model; record.provider = snapshot.provider; + record.runtime = snapshot.runtime; // env_vars stay overrides-only; see the create-path comment. Self-heal // pre-refresh records that baked persona env in as pseudo-overrides. record @@ -205,6 +206,7 @@ pub async fn restore_managed_agents_on_launch( } record.model = snapshot.model; record.provider = snapshot.provider; + record.runtime = snapshot.runtime; // env_vars stay overrides-only; see the create-path comment. // Self-heal pre-refresh records that baked persona env in as // pseudo-overrides. diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index e369fd443e..d5365a49c2 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -77,6 +77,7 @@ pub(crate) fn spawn_config_hash( } record.model = snapshot.model; record.provider = snapshot.provider; + record.runtime = snapshot.runtime; // Mirror the start/restore self-heal: overrides equal to the live // persona value are treated as inherited. The persona env itself // reaches the hash through `resolve_effective_agent_env` below. diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index a524457f9e..5e09aaff1a 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -376,6 +376,62 @@ fn team_pack_records_keep_empty_vs_absent_prompt_distinction() { ); } +/// (a) A definition-runtime edit must change spawn_config_hash for a +/// materialized, override-free record — the prospective re-snapshot now +/// copies the persona's runtime onto the record before hashing. +#[test] +fn definition_runtime_edit_changes_hash_for_materialized_record() { + let mut rec = record(); + rec.persona_id = Some("pers".into()); + rec.runtime = Some("goose".into()); // materialized runtime on instance + + let before = [persona("pers", Some("goose"), "prompt")]; + let after = [persona("pers", Some("claude"), "prompt")]; + assert_ne!( + spawn_config_hash(&rec, &before, "wss://ws.example"), + spawn_config_hash(&rec, &after, "wss://ws.example"), + "definition runtime edit must badge a materialized, override-free instance" + ); +} + +/// (c) An explicit agent_command_override (ladder step 1) must beat a +/// changed definition runtime — the badge must NOT fire for a pinned instance. +#[test] +fn agent_command_override_beats_definition_runtime_change() { + let mut rec = record(); + rec.persona_id = Some("pers".into()); + rec.runtime = Some("goose".into()); // materialized runtime + rec.agent_command_override = Some("goose".into()); // explicit per-instance pin + + let before = [persona("pers", Some("goose"), "prompt")]; + let after = [persona("pers", Some("claude"), "prompt")]; + assert_eq!( + spawn_config_hash(&rec, &before, "wss://ws.example"), + spawn_config_hash(&rec, &after, "wss://ws.example"), + "explicit override must win regardless of definition runtime change" + ); +} + +/// (d) When the linked definition is absent the prospective re-snapshot is +/// skipped entirely: the materialized runtime must still affect the hash. +#[test] +fn missing_definition_leaves_materialized_runtime_in_hash() { + let mut rec = record(); + rec.persona_id = Some("missing".into()); + rec.runtime = Some("goose".into()); // materialized runtime + + let no_personas: &[PersonaRecord] = &[]; + + let mut no_runtime = rec.clone(); + no_runtime.runtime = None; + + assert_ne!( + spawn_config_hash(&rec, no_personas, "wss://ws.example"), + spawn_config_hash(&no_runtime, no_personas, "wss://ws.example"), + "materialized runtime must still affect hash when definition is absent" + ); +} + #[test] fn effective_spawn_prompt_matches_hash_semantics() { // The env write and the hash share effective_spawn_prompt — this row From 6e82ba7d65969633307bde8ad3b37fbaef23cf25 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Fri, 10 Jul 2026 11:21:13 -0400 Subject: [PATCH 02/10] fix(desktop): stop persisting inert per-record mcp_command edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-instance mcp_command field is write-only: spawn derives the MCP command exclusively from the runtime catalog (known_acp_runtime), and the stored record value is never read at spawn time. Edits made in the instance dialog were silently discarded. Stop persisting it on update (remove the patching arm) and on create (always derive from the catalog instead of honoring input.mcp_command). Keep the wire fields on both request structs for backward compatibility — existing frontends that still send the field will parse cleanly with no side effects. Remove the editable input from the instance dialog. --- .../src-tauri/src/commands/agent_models.rs | 6 +-- .../src/commands/agent_models_tests.rs | 45 +++++++++++++++++++ desktop/src-tauri/src/commands/agents.rs | 20 ++++----- desktop/src-tauri/src/managed_agents/types.rs | 21 +++++++++ .../agents/ui/AgentInstanceEditDialog.tsx | 8 ---- .../agents/ui/EditAgentAdvancedFields.tsx | 34 -------------- 6 files changed, 77 insertions(+), 57 deletions(-) diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index e994c7efe8..9bafc845de 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -882,9 +882,9 @@ pub async fn update_managed_agent( if let Some(agent_args) = input.agent_args { record.agent_args = agent_args; } - if let Some(mcp_command) = input.mcp_command { - record.mcp_command = mcp_command; - } + // mcp_command is intentionally not applied here — the effective MCP + // command is always catalog-derived (known_acp_runtime at spawn time) + // and the per-record field is never read by the runtime. if let Some(env_vars) = input.env_vars { crate::managed_agents::validate_user_env_keys(&env_vars)?; record.env_vars = env_vars; diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index 2fe72e6e1a..96f4c1e15c 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -219,6 +219,51 @@ fn saved_agent_model_discovery_uses_record_snapshot() { // Parse/filter/pagination tests live in crates/buzz-agent/src/catalog.rs // (they moved there with the Option C refactor). +// --------------------------------------------------------------------------- +// Dead-knob guards: mcp_command and turn_timeout_seconds +// --------------------------------------------------------------------------- + +#[test] +fn update_request_mcp_command_roundtrips_for_wire_compat() { + // UpdateManagedAgentRequest accepts mcpCommand for backward-compatibility + // with frontends that still send it. The field must parse cleanly but the + // patching loop in update_managed_agent has no arm for it, so the stored + // record value is never overwritten. This test guards that invariant. + let req: crate::managed_agents::UpdateManagedAgentRequest = + serde_json::from_str(r#"{"pubkey": "abc", "mcpCommand": "user-override"}"#) + .expect("request with deprecated mcpCommand parses"); + assert_eq!(req.mcp_command.as_deref(), Some("user-override")); + + // Construct a record whose mcp_command differs from the request value. + let record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "abc", + "name": "test-agent", + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "catalog-derived-goose-mcp", + "turn_timeout_seconds": 320, + "parallelism": 1, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("record parses"); + + // After update_managed_agent (which has no mcp_command arm), the stored + // value is the catalog-derived value from create time, not the user override. + // If someone re-adds the arm, the record.mcp_command would become + // "user-override" — this assertion would then fail as intended. + assert_eq!(record.mcp_command, "catalog-derived-goose-mcp"); + assert_ne!(record.mcp_command, req.mcp_command.as_deref().unwrap_or("")); +} + #[test] fn is_databricks_provider_matches_both_variants() { assert!(is_databricks_provider(Some("databricks"))); diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index e6409bd712..133137bc66 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -595,18 +595,14 @@ pub async fn create_managed_agent( .collect::>(), ); - let mcp_command = input - .mcp_command - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - .map(str::to_string) - .unwrap_or_else( - || match crate::managed_agents::known_acp_runtime(&agent_command) { - Some(p) => p.mcp_command.unwrap_or("").to_string(), - None => String::new(), - }, - ); + // Derive MCP command exclusively from the runtime catalog — the + // per-record field is never read at spawn time so user-supplied input + // is silently discarded. Always sourcing from the catalog ensures + // new agents pick up the correct value without any stored override. + let mcp_command = match crate::managed_agents::known_acp_runtime(&agent_command) { + Some(p) => p.mcp_command.unwrap_or("").to_string(), + None => String::new(), + }; // For pack-backed personas, resolve the installed pack path and the // persona's internal name (slug). ACP's resolve_persona_by_name() diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 28a734e6e3..c1dbb68c1d 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -505,7 +505,18 @@ pub struct CreateManagedAgentRequest { pub harness_override: bool, #[serde(default)] pub agent_args: Vec, + /// Accepted for wire compatibility; not applied to the record. The + /// effective MCP command is always derived from the runtime catalog at + /// spawn time — a per-record override is never read. + /// + /// @deprecated — sending this field has no effect. + #[allow(dead_code)] pub mcp_command: Option, + /// Accepted for wire compatibility; not applied to the record. + /// `BUZZ_ACP_TURN_TIMEOUT` is deprecated and ignored by the harness. + /// + /// @deprecated — sending this field has no effect. + #[allow(dead_code)] pub turn_timeout_seconds: Option, pub idle_timeout_seconds: Option, pub max_turn_duration_seconds: Option, @@ -641,6 +652,11 @@ pub struct UpdateManagedAgentRequest { pub env_vars: Option>, #[serde(default)] pub parallelism: Option, + /// Accepted for wire compatibility; not applied to the stored record. + /// `BUZZ_ACP_TURN_TIMEOUT` is deprecated and ignored by the harness. + /// + /// @deprecated — sending this field has no effect. + #[allow(dead_code)] #[serde(default)] pub turn_timeout_seconds: Option, #[serde(default)] @@ -659,6 +675,11 @@ pub struct UpdateManagedAgentRequest { pub harness_override: bool, #[serde(default)] pub agent_args: Option>, + /// Accepted for wire compatibility; not applied to the stored record. + /// The effective MCP command is always catalog-derived at spawn time. + /// + /// @deprecated — sending this field has no effect. + #[allow(dead_code)] #[serde(default)] pub mcp_command: Option, /// Absent = don't touch. null = clear to runtime default. "id" = set. diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 7ac337badc..cd5e46e81d 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -96,7 +96,6 @@ export function AgentInstanceEditDialog({ agent.personaId != null && agent.agentCommandOverride == null, ); const [agentArgs, setAgentArgs] = React.useState(agent.agentArgs.join(",")); - const [mcpCommand, setMcpCommand] = React.useState(agent.mcpCommand); const [mcpToolsets, setMcpToolsets] = React.useState(agent.mcpToolsets ?? ""); const [turnTimeoutSeconds, setTurnTimeoutSeconds] = React.useState( String(agent.turnTimeoutSeconds), @@ -155,7 +154,6 @@ export function AgentInstanceEditDialog({ agent.personaId != null && agent.agentCommandOverride == null, ); setAgentArgs(agent.agentArgs.join(",")); - setMcpCommand(agent.mcpCommand); setMcpToolsets(agent.mcpToolsets ?? ""); setTurnTimeoutSeconds(String(agent.turnTimeoutSeconds)); setParallelism(String(agent.parallelism)); @@ -532,10 +530,6 @@ export function AgentInstanceEditDialog({ parsedArgs.join(",") !== agent.agentArgs.join(",") ? parsedArgs : undefined, - mcpCommand: - mcpCommand.trim() !== agent.mcpCommand - ? mcpCommand.trim() - : undefined, mcpToolsets: (mcpToolsets.trim() || null) !== agent.mcpToolsets ? mcpToolsets.trim() || null @@ -943,7 +937,6 @@ export function AgentInstanceEditDialog({ inheritedEnvVars={inheritedEnvVars} inheritHarness={inheritHarness} linkedPersona={linkedPersona} - mcpCommand={mcpCommand} mcpToolsets={mcpToolsets} parallelism={parallelism} relayUrl={relayUrl} @@ -957,7 +950,6 @@ export function AgentInstanceEditDialog({ onAutoRestartChange={setAutoRestartOnConfigChange} onEnvVarsChange={setEnvVars} onInheritHarnessChange={setInheritHarness} - onMcpCommandChange={setMcpCommand} onMcpToolsetsChange={setMcpToolsets} onParallelismChange={setParallelism} onRelayUrlChange={setRelayUrl} diff --git a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx index d66d10fafb..45c866b54c 100644 --- a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx @@ -20,7 +20,6 @@ export function EditAgentAdvancedFields({ inheritedEnvVars, inheritHarness, linkedPersona, - mcpCommand, mcpToolsets, parallelism, relayUrl, @@ -33,7 +32,6 @@ export function EditAgentAdvancedFields({ onAgentCommandChange, onEnvVarsChange, onInheritHarnessChange, - onMcpCommandChange, onMcpToolsetsChange, onParallelismChange, onRelayUrlChange, @@ -51,7 +49,6 @@ export function EditAgentAdvancedFields({ inheritedEnvVars: Record; inheritHarness: boolean; linkedPersona: AgentPersona | null; - mcpCommand: string; mcpToolsets: string; parallelism: string; relayUrl: string; @@ -64,7 +61,6 @@ export function EditAgentAdvancedFields({ onAgentCommandChange: (value: string) => void; onEnvVarsChange: (value: EnvVarsValue) => void; onInheritHarnessChange: (value: boolean) => void; - onMcpCommandChange: (value: string) => void; onMcpToolsetsChange: (value: string) => void; onParallelismChange: (value: string) => void; onRelayUrlChange: (value: string) => void; @@ -181,36 +177,6 @@ export function EditAgentAdvancedFields({ - {/* MCP command */} -
- -
- onMcpCommandChange(event.target.value)} - placeholder="Optional MCP server command" - value={mcpCommand} - /> -
-
- {/* MCP toolsets */}