Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions desktop/src-tauri/src/managed_agents/custom_harnesses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,29 @@ pub(crate) fn lookup_loaded_harness_by_id(id: &str) -> Option<Arc<HarnessDefinit
guard.iter().find(|d| d.id == id).cloned()
}

/// Look up a loaded (non-builtin) harness by **command**. Returns `None` when
/// no loaded definition's `command` matches exactly.
///
/// Used as a fallback when a record has no explicit `runtime` id — e.g. an
/// agent whose `agent_command_override` was set directly to a preset's
/// command (such as `"opencode"`) without also recording the matching
/// `runtime` id. Without this fallback such agents silently lose the
/// preset's default `args` (e.g. OpenCode's required `acp` subcommand),
/// because `lookup_loaded_harness_by_id` only ever sees an empty id and the
/// spawn falls back to the record's own (empty) `agent_args`.
pub(crate) fn lookup_loaded_harness_by_command(command: &str) -> Option<Arc<HarnessDefinition>> {
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
Expand Down
77 changes: 77 additions & 0 deletions desktop/src-tauri/src/managed_agents/discovery/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<String>::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
Expand Down
47 changes: 43 additions & 4 deletions desktop/src-tauri/src/managed_agents/readiness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down