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..a0e88edb32 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -1650,6 +1650,83 @@ 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" + ); +} + +/// 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 diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index fa8eb36fa1..253c99928f 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -131,7 +131,19 @@ 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). + // + // 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 @@ -145,7 +157,26 @@ pub(crate) fn resolve_effective_harness_descriptor( }) }) .unwrap_or(""); - crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(runtime_id) + 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. @@ -191,7 +222,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 +236,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)