From 344fb6ed8acad644c44fccab0858d5340ea0c9b2 Mon Sep 17 00:00:00 2001 From: Kamal Nayan Kumar Date: Fri, 31 Jul 2026 16:39:32 +0530 Subject: [PATCH 1/3] fix(acp): resolve preset args by command match when no runtime id is set Managed agents pinned to a preset's command via agent_command_override (e.g. "opencode") but without an explicit runtime id silently lost the preset's default args. resolve_effective_harness_descriptor only looked up the harness definition via record.runtime / persona.runtime, so a command-override-only agent found no definition and fell back to its own (empty) agent_args. For OpenCode this meant Buzz spawned the bare interactive TUI instead of `opencode acp`, and every pooled worker timed out at the ACP initialize handshake. Add lookup_loaded_harness_by_command as a fallback in both harness_def resolution sites in readiness.rs, matching the effective command against the already-loaded preset/custom registry when the id-based lookup misses. Adds a regression test reproducing the exact scenario (agent_command_override = "opencode", no runtime id, empty agent_args) and asserting the resolved descriptor carries args = ["acp"]. Signed-off-by: Kamal Nayan Kumar --- .../src/managed_agents/custom_harnesses.rs | 23 ++++++++++++ .../src/managed_agents/discovery/tests.rs | 35 +++++++++++++++++++ .../src-tauri/src/managed_agents/readiness.rs | 26 +++++++++++--- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/custom_harnesses.rs b/desktop/src-tauri/src/managed_agents/custom_harnesses.rs index e6bc09496c..52d62af0cc 100644 --- a/desktop/src-tauri/src/managed_agents/custom_harnesses.rs +++ b/desktop/src-tauri/src/managed_agents/custom_harnesses.rs @@ -308,6 +308,29 @@ pub(crate) fn lookup_loaded_harness_by_id(id: &str) -> Option Option> { + let guard = match loaded_harness_registry().read() { + Ok(g) => g, + Err(poisoned) => { + tracing::warn!( + "custom_harnesses: loaded-harness registry read lock was poisoned; recovering" + ); + poisoned.into_inner() + } + }; + guard.iter().find(|d| d.command == command).cloned() +} + /// Warm the loaded-harness registry synchronously from `custom_dir`. /// /// Must be called **before** `restore_managed_agents_on_launch` so that cold diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..517a5aae32 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -1650,6 +1650,41 @@ fn deleted_harness_summary_display_and_spawn_sentence_agree() { assert!(!sentence.contains(super::DANGLING_HARNESS_PREFIX)); } +/// An agent whose command was set directly to a preset's command string (e.g. +/// `agent_command_override: "opencode"`) — the shape produced when a user +/// pins a runtime via a raw command override rather than picking it from the +/// detected-harness list — must still pick up that preset's default args. +/// +/// Without a command-match fallback, `resolve_effective_harness_descriptor` +/// only finds a harness definition via `record.runtime` / `persona.runtime`, +/// which is unset here. That silently drops OpenCode's required `acp` +/// subcommand: Buzz then spawns bare `opencode` (the interactive TUI) instead +/// of `opencode acp` (the ACP server), and the agent hangs until the 60s +/// initialize timeout on every pooled worker. +#[test] +fn command_override_without_runtime_id_still_gets_preset_args() { + use crate::managed_agents::custom_harnesses::{ + registry_test_lock, warm_harness_registry_from_dir, + }; + use crate::managed_agents::{resolve_effective_harness_descriptor, GlobalAgentConfig}; + + let _lock = registry_test_lock(); + warm_harness_registry_from_dir(None); + + let record = record_with(None, None, Some("opencode")); + let global = GlobalAgentConfig::default(); + + let descriptor = resolve_effective_harness_descriptor(&record, &[], &global) + .expect("opencode preset should resolve via command match"); + + assert_eq!(descriptor.command, "opencode"); + assert_eq!( + descriptor.args, + vec!["acp".to_string()], + "opencode's preset default args (acp) must apply even without an explicit runtime id" + ); +} + // ── I2: custom catalog entry carries definition_env for the edit round-trip ─── /// A custom harness definition that includes env vars must surface those vars diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index fa8eb36fa1..b3a689a45c 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -131,7 +131,11 @@ pub(crate) fn resolve_effective_harness_descriptor( let runtime_meta = known_acp_runtime(&effective_command); // Look up the harness definition once — used for both args and env. - // Resolution order: record.runtime → persona.runtime → "". + // Resolution order: record.runtime → persona.runtime → command match → "". + // The command-match fallback covers agents whose `agent_command_override` + // was set directly to a preset's command (e.g. `"opencode"`) without also + // recording a `runtime` id — otherwise such agents silently lose the + // preset's default args (e.g. OpenCode's required `acp` subcommand). let harness_def = { let runtime_id = record .runtime @@ -145,7 +149,13 @@ pub(crate) fn resolve_effective_harness_descriptor( }) }) .unwrap_or(""); - crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id) + crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id).or_else( + || { + crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_command( + &effective_command, + ) + }, + ) }; // Args: explicit non-empty instance args win; otherwise use definition args. @@ -191,7 +201,7 @@ pub(crate) fn resolve_effective_agent_env( ) -> EffectiveAgentEnv { // Look up the harness definition for definition-level env (preset/custom). // Same resolution logic as spawn_agent_child: record runtime id first, then - // persona runtime id, then nothing. + // persona runtime id, then a command match, then nothing. let harness_def = { let runtime_id = record .runtime @@ -205,7 +215,15 @@ pub(crate) fn resolve_effective_agent_env( }) }) .unwrap_or(""); - crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id) + crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id).or_else( + || { + let effective_command = + crate::managed_agents::record_agent_command(record, personas); + crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_command( + &effective_command, + ) + }, + ) }; resolve_effective_agent_env_with_def(record, personas, runtime, global, harness_def) From af379e661b3b711395d221a62e8cd2840be3de45 Mon Sep 17 00:00:00 2001 From: Kamal Nayan Kumar Date: Fri, 31 Jul 2026 17:43:04 +0530 Subject: [PATCH 2/3] test(acp): pin down runtime-id-wins-over-command-match precedence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review on #3905: when record.runtime and the effective command (from agent_command_override) name different presets, the explicit runtime id wins — lookup_loaded_harness_by_id short-circuits the new lookup_loaded_harness_by_command fallback via or_else, so the fallback only ever fires when the id-based lookup misses. This precedence predates this PR; this test just documents it now that a command-match path exists to disagree with the id at all. Signed-off-by: Kamal Nayan Kumar --- .../src/managed_agents/discovery/tests.rs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 517a5aae32..a0e88edb32 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -1685,6 +1685,48 @@ fn command_override_without_runtime_id_still_gets_preset_args() { ); } +/// When `record.runtime` and the effective command (from +/// `agent_command_override`) name *different* presets, the explicit +/// `runtime` id wins over the command-match fallback — the fallback only +/// fires when the id-based lookup misses entirely, it never overrides an id +/// that resolved successfully. +/// +/// Here `runtime: Some("amp")` resolves via id to the `amp` preset (args: +/// `[]`), while the override command `"opencode"` would independently match +/// the `opencode` preset (args: `["acp"]`) if command-match ran first. The +/// id wins, so the effective args are `amp`'s (empty) — even though the +/// command actually being launched is `opencode`. This is a pre-existing +/// precedence (id-before-command), not something this PR's fallback +/// changes; this test just pins it down now that a command-match path +/// exists to disagree with the id at all. +#[test] +fn runtime_id_wins_over_disagreeing_command_match() { + use crate::managed_agents::custom_harnesses::{ + registry_test_lock, warm_harness_registry_from_dir, + }; + use crate::managed_agents::{resolve_effective_harness_descriptor, GlobalAgentConfig}; + + let _lock = registry_test_lock(); + warm_harness_registry_from_dir(None); + + let record = record_with(Some("amp"), None, Some("opencode")); + let global = GlobalAgentConfig::default(); + + let descriptor = resolve_effective_harness_descriptor(&record, &[], &global) + .expect("amp runtime id should resolve"); + + assert_eq!( + descriptor.command, "opencode", + "agent_command_override still determines the actual command to launch" + ); + assert_eq!( + descriptor.args, + Vec::::new(), + "runtime id 'amp' must win over the disagreeing 'opencode' command match, \ + so amp's (empty) args apply — not opencode's ['acp']" + ); +} + // ── I2: custom catalog entry carries definition_env for the edit round-trip ─── /// A custom harness definition that includes env vars must surface those vars From c03a7cfacfaa70c65bddcc630a25bf3225c2259e Mon Sep 17 00:00:00 2001 From: Kamal Nayan Kumar Date: Fri, 31 Jul 2026 18:07:01 +0530 Subject: [PATCH 3/3] fix(acp): warn on runtime-id/command mismatch in harness resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review on #3905: when record.runtime resolves to a harness whose command differs from the agent's effective command (from agent_command_override), that mismatched definition's args/env still apply — the id always wins over a disagreeing command match, per runtime_id_wins_over_disagreeing_command_match. This can silently strip args the effective command actually needs (e.g. an ACP subcommand), and previously surfaced only as a bare ACP initialize timeout with no clue why. Log a tracing::warn! in resolve_effective_harness_descriptor when this mismatch occurs, naming the agent pubkey, the runtime id, the harness's own command, and the effective command, so the agent's log explains the mismatch instead of just timing out. Scoped to resolve_effective_harness_descriptor only (spawn/hash/model probe path) — not the sibling resolve_effective_agent_env, which is also called from several UI preview paths (e.g. live env-diff previews as a user edits settings) where firing on every keystroke would be noise rather than signal. Signed-off-by: Kamal Nayan Kumar --- .../src-tauri/src/managed_agents/readiness.rs | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index b3a689a45c..253c99928f 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -136,6 +136,14 @@ pub(crate) fn resolve_effective_harness_descriptor( // was set directly to a preset's command (e.g. `"opencode"`) without also // recording a `runtime` id — otherwise such agents silently lose the // preset's default args (e.g. OpenCode's required `acp` subcommand). + // + // An explicit `runtime` id always wins over a disagreeing command match — + // it is never overridden, only filled in when it resolves to nothing. If + // the id resolves to a *different* command than what's actually being + // launched, that mismatched definition's args/env still apply, which can + // silently strip a required subcommand (e.g. OpenCode's `acp`) without + // any error — the agent simply hangs at the ACP handshake. Warn so this + // is visible in the agent's log instead of a bare timeout. let harness_def = { let runtime_id = record .runtime @@ -149,13 +157,26 @@ pub(crate) fn resolve_effective_harness_descriptor( }) }) .unwrap_or(""); - crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id).or_else( - || { - crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_command( - &effective_command, - ) - }, - ) + match crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id) { + Some(def) => { + if def.command != effective_command { + tracing::warn!( + agent = %record.pubkey, + runtime_id, + harness_command = %def.command, + effective_command = %effective_command, + "runtime id names a harness whose command differs from the \ + agent's effective command; the id's args/env apply anyway \ + (id always wins over a command match) — this may silently \ + drop args the effective command needs (e.g. an ACP subcommand)" + ); + } + Some(def) + } + None => crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_command( + &effective_command, + ), + } }; // Args: explicit non-empty instance args win; otherwise use definition args.