From 8a5beca0ed4d068819df850398b5708d49b74598 Mon Sep 17 00:00:00 2001 From: joshualwrnc <75241961+joshualwrnc@users.noreply.github.com> Date: Tue, 4 Aug 2026 13:38:34 +0800 Subject: [PATCH] fix(desktop): clear stale instance agent args on harness switch Instance agent_args are harness-specific and win over the harness definition's default args at spawn. apply_agent_command_update updated the runtime pin without touching record.agent_args, so switching an agent's harness (e.g. Grok Build to Oh My Pi) left the old harness's flags in place and every spawn passed them verbatim to the new binary: omp launched as `omp agent --always-approve stdio` and crash-looped on `unknown flag: --always-approve`. Clear record.agent_args when the edit changes the effective command. Same-command re-saves keep customized args, and replacement args sent in the same update are applied after the clear, so the edit dialog's harness-switch flow (which sends the new harness's defaults) is unaffected. Co-authored-by: joshualwrnc <75241961+joshualwrnc@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: joshualwrnc <75241961+joshualwrnc@users.noreply.github.com> --- .../src/managed_agents/discovery/overrides.rs | 14 +++++- .../src/managed_agents/discovery/tests.rs | 49 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/desktop/src-tauri/src/managed_agents/discovery/overrides.rs b/desktop/src-tauri/src/managed_agents/discovery/overrides.rs index 5140bb2cdd..2d892cdfef 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/overrides.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/overrides.rs @@ -3,7 +3,7 @@ //! of `discovery` (split under the desktop file-size cap) so the override //! decisions stay next to the resolution ladder they feed. -use super::{effective_agent_command, known_acp_runtime}; +use super::{effective_agent_command, known_acp_runtime, record_agent_command}; /// Decide whether a user-picked harness command is an explicit per-instance /// pin or merely the persona's own runtime restated. Returns the override to @@ -90,12 +90,21 @@ pub fn update_time_agent_command_override( /// record the materialized runtime is the only harness source left after the /// override clear, so a stray empty `agent_command` from a non-dialog caller /// must not change what the agent runs. +/// +/// When the edit changes the effective command, `record.agent_args` is +/// cleared: instance args are harness-specific (e.g. Grok Build's +/// `agent --always-approve stdio`), win over the harness definition's +/// defaults at spawn, and are passed verbatim to the new binary — carrying +/// them across a harness switch crash-loops the agent on flags the new +/// binary does not understand. Callers that supply replacement args in the +/// same update write them AFTER this apply, so explicit new args survive. pub fn apply_agent_command_update( record: &mut crate::managed_agents::types::ManagedAgentRecord, personas: &[crate::managed_agents::types::AgentDefinition], agent_command: &str, harness_override: bool, ) { + let previous_command = record_agent_command(record, personas); record.agent_command_override = update_time_agent_command_override( record.persona_id.as_deref(), personas, @@ -105,6 +114,9 @@ pub fn apply_agent_command_update( if agent_command.trim().is_empty() && record.persona_id.is_some() { record.runtime = None; } + if record_agent_command(record, personas) != previous_command { + record.agent_args.clear(); + } } /// Decide the `agent_command_override` to persist at AGENT CREATE time. diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..10c9b8f9f7 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -667,6 +667,55 @@ fn apply_agent_command_update_concrete_pin_keeps_materialized_runtime() { assert_eq!(record_agent_command(&record, &personas), "codex-acp"); } +#[test] +fn apply_agent_command_update_clears_args_when_command_changes() { + // The Lens repro: a record carrying Grok Build's args switches to Oh My + // Pi. Instance args win over harness defaults at spawn, so keeping them + // would pass `agent --always-approve stdio` to `omp` and crash-loop. + let personas = vec![persona_with_runtime("p1", Some("grok"))]; + let mut record = record_with(Some("grok"), Some("p1"), None); + record.agent_args = vec![ + "agent".to_string(), + "--always-approve".to_string(), + "stdio".to_string(), + ]; + + apply_agent_command_update(&mut record, &personas, "omp", true); + + assert_eq!(record_agent_command(&record, &personas), "omp"); + assert!( + record.agent_args.is_empty(), + "harness-specific instance args must not survive a command change" + ); +} + +#[test] +fn apply_agent_command_update_preserves_args_when_command_unchanged() { + // Re-saving the same harness (e.g. a name edit that echoes the command) + // must not wipe deliberately customized args. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + let mut record = record_with(Some("claude"), Some("p1"), None); + record.agent_args = vec!["--verbose".to_string()]; + + apply_agent_command_update(&mut record, &personas, "claude-agent-acp", false); + + assert_eq!(record.agent_args, vec!["--verbose".to_string()]); +} + +#[test] +fn apply_agent_command_update_inherit_sentinel_clears_args_on_fallback() { + // Choosing Inherit drops the pin; when the persona's command differs from + // the pinned one, the pin's args must not leak onto the persona harness. + let personas = vec![persona_with_runtime("p1", Some("goose"))]; + let mut record = record_with(Some("claude"), Some("p1"), Some("codex-acp")); + record.agent_args = vec!["--codex-flag".to_string()]; + + apply_agent_command_update(&mut record, &personas, "", false); + + assert_eq!(record_agent_command(&record, &personas), "goose"); + assert!(record.agent_args.is_empty()); +} + // ── probe_codex_acp_version ─────────────────────────────────────────────────── mod managed_path_resolution;