From 7d59365f67769edb19275e8315755dd1f86c605c Mon Sep 17 00:00:00 2001 From: Brad Hallett <53977268+bradhallett@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:41:10 -0400 Subject: [PATCH 1/3] fix(acp): use preset launch args for managed agents with no runtime Managed agents pinned to a known ACP runtime via agent_command_override (e.g. `omp`, `grok`, `opencode`, `kimi`) but with no `runtime` id resolved the effective command (e.g. `omp`) yet launched it with empty args. With no controlling TTY under headless Buzz, the agent's interactive TUI started instead of its ACP/stdio mode, so the JSON-RPC `initialize` handshake never completed and the agent timed out at startup (omp/opencode/kimi: Request timeout; grok: ENXIO "Device not configured"). Root cause: resolve_effective_harness_descriptor only filled args when the record carried explicit instance args or a resolvable harness preset (record.runtime -> lookup_loaded_harness_by_id). A command-override agent with runtime=None hit neither, so descriptor.args stayed empty. Fix: when no preset resolves, fall back to the PRESET_HARNESSES entry whose command matches the effective command (new preset_args_for_command, mirroring the existing single-source preset_harness_ids helper and living alongside it in the managed_agents::discovery::presets submodule). The PresetHarness table stays the single source of truth for per-runtime launch args, so this covers omp, grok, opencode, kimi, cursor, openclaw, and devin at once with no hand-mirroring, and descriptor.args now flows correctly to the spawn env (BUZZ_ACP_AGENT_ARGS), the config hash, and the agent summary. No regressions: explicit instance args still win; goose still resolves via default_agent_args; amp/hermes presets (empty args) are unchanged; custom harness ids collide with reserved preset ids so cannot shadow them. Resolves #3399 Resolves #3457 Resolves #3729 Signed-off-by: Brad Hallett <53977268+bradhallett@users.noreply.github.com> --- .../src-tauri/src/managed_agents/discovery.rs | 4 +- .../src/managed_agents/discovery/presets.rs | 21 +++- .../src-tauri/src/managed_agents/readiness.rs | 109 ++++++++++++++++++ 3 files changed, 131 insertions(+), 3 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 1ee7e6e5562..15f2a6d915e 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -22,8 +22,8 @@ pub(crate) use login_shell::{ is_login_shell_path_uninit, is_safe_nvm_tag, login_shell_candidates, parse_semver_tag, }; pub(crate) use presets::{ - canonical_harness_command, command_for_runtime_id, preset_harness_definitions, - preset_harness_ids, + canonical_harness_command, command_for_runtime_id, preset_args_for_command, + preset_harness_definitions, preset_harness_ids, }; use presets::{preset_catalog_entry, PRESET_HARNESSES}; pub(crate) use runtime_metadata::KnownAcpRuntime; diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index fd853094515..670b1f09ebc 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -5,7 +5,7 @@ use crate::managed_agents::{ AcpAvailabilityStatus, AcpRuntimeCatalogEntry, AuthStatus, HarnessSource, }; -use super::normalize_agent_args; +use super::{normalize_agent_args, normalize_command_identity}; /// Static data for a well-known tier-2 ACP harness. pub(super) struct PresetHarness { @@ -206,6 +206,25 @@ pub(crate) fn preset_harness_ids() -> &'static [&'static str] { .as_slice() } +/// Launch args for the [`PRESET_HARNESSES`] entry whose command matches the +/// given command, when an agent has a command but no resolvable runtime preset +/// (e.g. an explicit `agent_command_override` like `omp` with no `runtime` id). +/// +/// Single source of truth: per-runtime launch args live only in +/// `PRESET_HARNESSES`, mirroring [`preset_harness_ids`], so every preset — omp, +/// grok, opencode, kimi, cursor, openclaw, … — is covered at the source instead +/// of being hand-mirrored. Without this, an agent pinned to e.g. `omp` with no +/// runtime id launches the bare command, which under headless Buzz (no +/// controlling TTY) drops into the interactive TUI instead of `omp acp` and the +/// ACP `initialize` handshake times out. +pub(crate) fn preset_args_for_command(command: &str) -> Option> { + let identity = normalize_command_identity(command); + PRESET_HARNESSES + .iter() + .find(|preset| normalize_command_identity(preset.command) == identity) + .map(|preset| preset.args.iter().map(|arg| arg.to_string()).collect()) +} + /// Return the primary command for a preset harness by id, or `None` if the id /// is not a known preset. /// diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 909b97d652d..32203e9239b 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -156,6 +156,15 @@ pub(crate) fn resolve_effective_harness_descriptor( normalize_agent_args(&effective_command, record_args) } else if let Some(ref def) = harness_def { normalize_agent_args(&effective_command, def.args.clone()) + } else if let Some(preset_args) = + crate::managed_agents::discovery::preset_args_for_command(&effective_command) + { + // No runtime id resolved a harness preset, but the command itself + // is a known ACP runtime (e.g. an explicit override like `omp`). + // Use that preset's launch args so we start its ACP/stdio mode + // rather than the bare command, which would launch the interactive + // TUI and time out at ACP `initialize` under headless Buzz. + normalize_agent_args(&effective_command, preset_args) } else { normalize_agent_args(&effective_command, record_args) } @@ -1548,6 +1557,106 @@ mod tests { ); } + #[test] + fn resolve_effective_harness_descriptor_uses_preset_args_for_pinned_command() { + // An agent pinned to a known ACP runtime via agent_command_override, + // with no runtime id and no instance args, must take the launch args + // from the matching PRESET_HARNESSES entry. Otherwise the bare command + // (e.g. `omp`) launches the interactive TUI, which times out at ACP + // `initialize` under headless Buzz (no controlling TTY). + let mk = |command: &str| crate::managed_agents::types::ManagedAgentRecord { + pubkey: "k".to_string(), + name: "agent".to_string(), + agent_command_override: Some(command.to_string()), + agent_args: vec![], + runtime: None, + persona_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: String::new(), + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars: BTreeMap::new(), + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_policy_pending: false, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + }; + let global = crate::managed_agents::GlobalAgentConfig::default(); + + let desc = |command: &str| { + resolve_effective_harness_descriptor(&mk(command), &[], &global).unwrap() + }; + + // ACP-mode runtimes resolve to `acp`. + assert_eq!(desc("devin").args, vec!["acp".to_string()]); + assert_eq!(desc("omp").args, vec!["acp".to_string()]); + assert_eq!(desc("opencode").args, vec!["acp".to_string()]); + assert_eq!(desc("kimi").args, vec!["acp".to_string()]); + assert_eq!(desc("cursor-agent").args, vec!["acp".to_string()]); + assert_eq!(desc("openclaw").args, vec!["acp".to_string()]); + // Grok launches its agent stdio mode (not the TUI). + assert_eq!( + desc("grok").args, + vec![ + "agent".to_string(), + "--always-approve".to_string(), + "stdio".to_string() + ] + ); + + // Explicit instance args always win over the preset fallback. + let mut explicit = mk("omp"); + explicit.agent_args = vec!["acp".to_string(), "--foo".to_string()]; + let explicit_desc = resolve_effective_harness_descriptor(&explicit, &[], &global).unwrap(); + assert_eq!( + explicit_desc.args, + vec!["acp".to_string(), "--foo".to_string()] + ); + + // An unknown command with no preset keeps the (empty) instance args. + assert!(desc("my-fancy-cli").args.is_empty()); + } + + // ── provider-specific model fallback tests ──────────────────────────── #[test] fn buzz_agent_databricks_v2_with_databricks_model_but_no_buzz_agent_model_is_ready() { // The baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL. From af6fca426af8a7e1f4f36a3aad5083adc87ad0d3 Mon Sep 17 00:00:00 2001 From: Brad Hallett <53977268+bradhallett@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:24:06 -0400 Subject: [PATCH 2/3] fix: address review item #1 - log resolved launch args Signed-off-by: Brad Hallett <53977268+bradhallett@users.noreply.github.com> --- desktop/src-tauri/src/managed_agents/runtime.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 0ce5ca7b219..1e567bd31fe 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -497,6 +497,13 @@ pub fn spawn_agent_child( let resolved_agent_command = resolve_command(effective_command) .map(|p| p.display().to_string()) .unwrap_or_else(|| effective_command.clone()); + append_log_marker( + &log_path, + &format!( + "resolved launch: agent_command={:?} args={:?}", + resolved_agent_command, agent_args + ), + )?; // The caller supplies the explicit canonical pair relay. This is the only // relay this child may connect to, regardless of the record/workspace default. From 74b60d89bde85dbebcfe4f40a1e84ffb4013eb10 Mon Sep 17 00:00:00 2001 From: Brad Hallett <53977268+bradhallett@users.noreply.github.com> Date: Wed, 19 Aug 2026 13:54:34 -0400 Subject: [PATCH 3/3] fix(acp): redact launch args from runtime log and honor file-size ratchet Review item #1 (security): spawn_agent_child logged the raw agent_args vector with `{:?}` into the runtime log retrievable end-to-end via get_managed_agent_log. Args may legally carry credentials (--token=...), so no argument value may ever be serialized. The resolved-launch marker now records only the resolved command and the argument count, mirroring spawn_snapshot::diff's MaskedBare policy. The marker writer lives in storage.rs as append_resolved_launch_marker(path, command, args): it resolves the command to a full path (DMG launches have a minimal PATH), appends the redacted marker, and returns the resolved command for the BUZZ_ACP_AGENT_COMMAND spawn env, replacing runtime.rs's inline resolution. open_log_file now creates runtime logs with mode 0o600 at creation time (Unix), so the permissions hold regardless of umask; agent stdout/stderr can echo installer/CLI credentials. Regression tests: append_resolved_launch_marker_never_writes_argument_ values asserts the marker carries the command and args_count only -- never token/argument values -- and that unresolvable commands fall through verbatim; open_log_file_creates_owner_only asserts 0o600. File-size ratchet: the two effective-launch tests moved from readiness.rs into readiness_effective_launch_tests.rs (external runtime.rs stays at/below the ratchet cap after folding command resolution into the storage helper. Unblocks Detect Changed Paths so the required CI jobs run. Signed-off-by: Brad Hallett <53977268+bradhallett@users.noreply.github.com> --- .../src/managed_agents/launch_log.rs | 76 +++++++ desktop/src-tauri/src/managed_agents/mod.rs | 2 + .../src-tauri/src/managed_agents/readiness.rs | 193 +---------------- .../readiness_effective_launch_tests.rs | 198 ++++++++++++++++++ .../src-tauri/src/managed_agents/runtime.rs | 20 +- .../src-tauri/src/managed_agents/storage.rs | 21 +- .../src/managed_agents/storage_tests.rs | 24 +++ 7 files changed, 325 insertions(+), 209 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/launch_log.rs create mode 100644 desktop/src-tauri/src/managed_agents/readiness_effective_launch_tests.rs diff --git a/desktop/src-tauri/src/managed_agents/launch_log.rs b/desktop/src-tauri/src/managed_agents/launch_log.rs new file mode 100644 index 00000000000..800bc8f10d6 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/launch_log.rs @@ -0,0 +1,76 @@ +//! Managed-agent runtime-log markers: generic run-lifecycle lines and the +//! resolved-launch marker recorded immediately before spawn. +//! +//! Split out of `storage.rs` to keep that module under the desktop file-size +//! ratchet. The resolved-launch marker deliberately serializes only the agent +//! command and the argument count — never argument values — mirroring the +//! `spawn_snapshot::diff` `MaskedBare` policy for `args`. + +use std::io::Write; +use std::path::Path; + +use super::storage::open_log_file; +use crate::managed_agents::resolve_command; + +pub(crate) fn append_log_marker(path: &Path, message: &str) -> Result<(), String> { + let mut file = open_log_file(path)?; + writeln!(file, "{message}").map_err(|error| format!("failed to write log marker: {error}")) +} + +/// Resolve `command` to a full path (DMG launches have a minimal PATH), append +/// the resolved-launch marker, and return the resolved command for the spawn +/// env (`BUZZ_ACP_AGENT_COMMAND`). The marker records the agent command and +/// the argument count, and nothing else: `agent_args` is user-controlled and +/// may legally carry credentials (`--token=...`), and the runtime log is +/// retrievable end-to-end via `get_managed_agent_log`, so no argument value +/// may ever be serialized — mirroring the `spawn_snapshot::diff` policy, +/// which masks `args` as `MaskedBare` for exactly this reason. When +/// resolution fails, both the marker and the return value carry the +/// unresolved command verbatim. +pub(crate) fn append_resolved_launch_marker( + path: &Path, + command: &str, + args: &[String], +) -> Result { + let resolved = resolve_command(command) + .map(|p| p.display().to_string()) + .unwrap_or_else(|| command.to_string()); + let marker = format!( + "resolved launch: agent_command={resolved:?} args_count={}", + args.len() + ); + append_log_marker(path, &marker)?; + Ok(resolved) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn append_resolved_launch_marker_never_writes_argument_values() { + // `agent_args` is user-controlled and may legally carry credentials + // (`--token=...`), and the runtime log is retrievable end-to-end via + // `get_managed_agent_log`, so the marker may carry only the command and + // the argument count. The command is a nonexistent absolute path so + // `acp` can only reach the log through an argument value. + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("agent.log"); + + let resolved = append_resolved_launch_marker( + &path, + "/nonexistent/zuzu-agent-cli", + &["acp".to_string(), "--token=supersecret-value".to_string()], + ) + .expect("append marker"); + + // Unresolvable commands fall through verbatim for the spawn env. + assert_eq!(resolved, "/nonexistent/zuzu-agent-cli"); + let logged = std::fs::read_to_string(&path).expect("read log"); + assert!(logged.contains("args_count=2")); + assert!(logged.contains("/nonexistent/zuzu-agent-cli")); + assert!(!logged.contains("supersecret-value")); + assert!(!logged.contains("--token")); + assert!(!logged.contains("acp")); + } +} diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index c005e8858b7..f8171ec051b 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -18,6 +18,7 @@ pub(crate) mod effective_config; mod env_vars; pub(crate) mod git_bash; pub(crate) mod global_config; +mod launch_log; mod managed_node_paths; mod nest; pub(crate) mod parallelism; @@ -67,6 +68,7 @@ pub(crate) use global_config::{ load_global_agent_config, resolve_effective_model_provider, save_global_agent_config, validate_global_config, GlobalAgentConfig, }; +pub(crate) use launch_log::*; pub(crate) use managed_node_paths::*; pub use nest::*; pub use parallelism::{acp_agents_value, effective_parallelism, harness_max_parallelism}; diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 32203e9239b..e3470564f51 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1467,193 +1467,6 @@ mod tests { let json = serde_json::to_value(&r).unwrap(); assert_eq!(json["surface"], "cli_login"); assert!(json["probe_args"].is_array()); - assert!(json["setup_copy"].as_str().unwrap().contains("codex login")); - } - - // ── resolve_effective_agent_env ───────────────────────────────────────── - - #[test] - fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { - // User env_vars must win over baked defaults; in OSS builds baked map is empty, - // so this validates the user-env layer is present in the output. - let mut env_vars = BTreeMap::new(); - env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); - env_vars.insert( - "BUZZ_AGENT_MODEL".to_string(), - "claude-opus-4-5".to_string(), - ); - - // Minimal record: only the fields resolve_effective_agent_env reads. - let record = crate::managed_agents::types::ManagedAgentRecord { - pubkey: "test-pubkey".to_string(), - name: "test-agent".to_string(), - persona_id: None, - private_key_nsec: String::new(), - auth_tag: None, - relay_url: String::new(), - avatar_url: None, - acp_command: "buzz-acp".to_string(), - agent_command: "buzz-agent".to_string(), - agent_command_override: None, - agent_args: vec![], - mcp_command: String::new(), - turn_timeout_seconds: 320, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - env_vars, - start_on_app_launch: false, - auto_restart_on_config_change: true, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_policy_pending: false, - provider_binary_path: None, - team_id: None, - persona_team_dir: None, - persona_name_in_team: None, - created_at: String::new(), - updated_at: String::new(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - last_error_code: None, - respond_to: Default::default(), - respond_to_allowlist: vec![], - display_name: None, - slug: None, - runtime: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - team_catalog_source: None, - definition_respond_to: None, - definition_respond_to_allowlist: Vec::new(), - definition_parallelism: None, - relay_mesh: None, - effort_level: None, - }; - - let runtime = known_acp_runtime_exact("buzz-agent"); - let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); - - // User env_vars must be present in the output (last-write-wins). - assert_eq!( - effective.env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), - Some("anthropic") - ); - assert_eq!( - effective.env.get("BUZZ_AGENT_MODEL").map(String::as_str), - Some("claude-opus-4-5") - ); - } - - #[test] - fn resolve_effective_harness_descriptor_uses_preset_args_for_pinned_command() { - // An agent pinned to a known ACP runtime via agent_command_override, - // with no runtime id and no instance args, must take the launch args - // from the matching PRESET_HARNESSES entry. Otherwise the bare command - // (e.g. `omp`) launches the interactive TUI, which times out at ACP - // `initialize` under headless Buzz (no controlling TTY). - let mk = |command: &str| crate::managed_agents::types::ManagedAgentRecord { - pubkey: "k".to_string(), - name: "agent".to_string(), - agent_command_override: Some(command.to_string()), - agent_args: vec![], - runtime: None, - persona_id: None, - private_key_nsec: String::new(), - auth_tag: None, - relay_url: String::new(), - avatar_url: None, - acp_command: "buzz-acp".to_string(), - agent_command: String::new(), - mcp_command: String::new(), - turn_timeout_seconds: 320, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - env_vars: BTreeMap::new(), - start_on_app_launch: false, - auto_restart_on_config_change: true, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_policy_pending: false, - provider_binary_path: None, - team_id: None, - persona_team_dir: None, - persona_name_in_team: None, - created_at: String::new(), - updated_at: String::new(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - last_error_code: None, - respond_to: Default::default(), - respond_to_allowlist: vec![], - display_name: None, - slug: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - definition_respond_to: None, - definition_respond_to_allowlist: Vec::new(), - definition_parallelism: None, - relay_mesh: None, - }; - let global = crate::managed_agents::GlobalAgentConfig::default(); - - let desc = |command: &str| { - resolve_effective_harness_descriptor(&mk(command), &[], &global).unwrap() - }; - - // ACP-mode runtimes resolve to `acp`. - assert_eq!(desc("devin").args, vec!["acp".to_string()]); - assert_eq!(desc("omp").args, vec!["acp".to_string()]); - assert_eq!(desc("opencode").args, vec!["acp".to_string()]); - assert_eq!(desc("kimi").args, vec!["acp".to_string()]); - assert_eq!(desc("cursor-agent").args, vec!["acp".to_string()]); - assert_eq!(desc("openclaw").args, vec!["acp".to_string()]); - // Grok launches its agent stdio mode (not the TUI). - assert_eq!( - desc("grok").args, - vec![ - "agent".to_string(), - "--always-approve".to_string(), - "stdio".to_string() - ] - ); - - // Explicit instance args always win over the preset fallback. - let mut explicit = mk("omp"); - explicit.agent_args = vec!["acp".to_string(), "--foo".to_string()]; - let explicit_desc = resolve_effective_harness_descriptor(&explicit, &[], &global).unwrap(); - assert_eq!( - explicit_desc.args, - vec!["acp".to_string(), "--foo".to_string()] - ); - - // An unknown command with no preset keeps the (empty) instance args. - assert!(desc("my-fancy-cli").args.is_empty()); } // ── provider-specific model fallback tests ──────────────────────────── @@ -1847,3 +1660,9 @@ mod tests { #[cfg(test)] #[path = "readiness_goose_file_config_tests.rs"] mod goose_file_config_tests; + +// Effective-launch resolution tests live in a sibling file so this module +// stays under the desktop file-size ratchet. +#[cfg(test)] +#[path = "readiness_effective_launch_tests.rs"] +mod effective_launch_tests; diff --git a/desktop/src-tauri/src/managed_agents/readiness_effective_launch_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_effective_launch_tests.rs new file mode 100644 index 00000000000..08305b3aaed --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness_effective_launch_tests.rs @@ -0,0 +1,198 @@ +//! Effective-launch resolution tests — `resolve_effective_agent_env` and +//! `resolve_effective_harness_descriptor`. +//! +//! Split out of `readiness.rs`'s inline `mod tests` to keep that module +//! under the desktop file-size ratchet. Included via `#[path]`; `super::*` +//! resolves against `readiness.rs`, matching the `storage_tests.rs` convention. + +use std::collections::BTreeMap; + +use super::*; +use crate::managed_agents::discovery::known_acp_runtime_exact; + +// ── resolve_effective_agent_env ───────────────────────────────────────── + +#[test] +fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { + // User env_vars must win over baked defaults; in OSS builds baked map is empty, + // so this validates the user-env layer is present in the output. + let mut env_vars = BTreeMap::new(); + env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); + env_vars.insert( + "BUZZ_AGENT_MODEL".to_string(), + "claude-opus-4-5".to_string(), + ); + + // Minimal record: only the fields resolve_effective_agent_env reads. + let record = crate::managed_agents::types::ManagedAgentRecord { + pubkey: "test-pubkey".to_string(), + name: "test-agent".to_string(), + persona_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: "buzz-agent".to_string(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars, + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_policy_pending: false, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + team_catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + effort_level: None, + }; + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + // User env_vars must be present in the output (last-write-wins). + assert_eq!( + effective.env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), + Some("anthropic") + ); + assert_eq!( + effective.env.get("BUZZ_AGENT_MODEL").map(String::as_str), + Some("claude-opus-4-5") + ); +} + +#[test] +fn resolve_effective_harness_descriptor_uses_preset_args_for_pinned_command() { + // An agent pinned to a known ACP runtime via agent_command_override, + // with no runtime id and no instance args, must take the launch args + // from the matching PRESET_HARNESSES entry. Otherwise the bare command + // (e.g. `omp`) launches the interactive TUI, which times out at ACP + // `initialize` under headless Buzz (no controlling TTY). + let mk = |command: &str| crate::managed_agents::types::ManagedAgentRecord { + pubkey: "k".to_string(), + name: "agent".to_string(), + agent_command_override: Some(command.to_string()), + agent_args: vec![], + runtime: None, + persona_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: String::new(), + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars: BTreeMap::new(), + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_policy_pending: false, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + team_catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + effort_level: None, + }; + let global = crate::managed_agents::GlobalAgentConfig::default(); + + let desc = + |command: &str| resolve_effective_harness_descriptor(&mk(command), &[], &global).unwrap(); + + // ACP-mode runtimes resolve to `acp`. + assert_eq!(desc("devin").args, vec!["acp".to_string()]); + assert_eq!(desc("omp").args, vec!["acp".to_string()]); + assert_eq!(desc("opencode").args, vec!["acp".to_string()]); + assert_eq!(desc("kimi").args, vec!["acp".to_string()]); + assert_eq!(desc("cursor-agent").args, vec!["acp".to_string()]); + assert_eq!(desc("openclaw").args, vec!["acp".to_string()]); + // Grok launches its agent stdio mode (not the TUI). + assert_eq!( + desc("grok").args, + vec![ + "agent".to_string(), + "--always-approve".to_string(), + "stdio".to_string() + ] + ); + + // Explicit instance args always win over the preset fallback. + let mut explicit = mk("omp"); + explicit.agent_args = vec!["acp".to_string(), "--foo".to_string()]; + let explicit_desc = resolve_effective_harness_descriptor(&explicit, &[], &global).unwrap(); + assert_eq!( + explicit_desc.args, + vec!["acp".to_string(), "--foo".to_string()] + ); + + // An unknown command with no preset keeps the (empty) instance args. + assert!(desc("my-fancy-cli").args.is_empty()); +} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 1e567bd31fe..894debec1b4 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -6,10 +6,10 @@ use super::agent_env::{build_buzz_agent_provider_defaults, idle_pool_sleep_env}; use crate::{ managed_agents::{ - append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path, - missing_command_message, normalize_agent_args, open_log_file, resolve_command, - spawn_key_refusal, KnownAcpRuntime, ManagedAgentPairRuntime, ManagedAgentRecord, - ManagedAgentRuntimeKey, ManagedAgentSummary, + append_log_marker, append_resolved_launch_marker, known_acp_runtime, login_shell_path, + managed_agent_log_path, missing_command_message, normalize_agent_args, open_log_file, + resolve_command, spawn_key_refusal, KnownAcpRuntime, ManagedAgentPairRuntime, + ManagedAgentRecord, ManagedAgentRuntimeKey, ManagedAgentSummary, }, util::now_iso, }; @@ -494,16 +494,8 @@ pub fn spawn_agent_child( } }; // Resolve agent command to a full path (DMG launches have minimal PATH). - let resolved_agent_command = resolve_command(effective_command) - .map(|p| p.display().to_string()) - .unwrap_or_else(|| effective_command.clone()); - append_log_marker( - &log_path, - &format!( - "resolved launch: agent_command={:?} args={:?}", - resolved_agent_command, agent_args - ), - )?; + let resolved_agent_command = + append_resolved_launch_marker(&log_path, effective_command, agent_args)?; // The caller supplies the explicit canonical pair relay. This is the only // relay this child may connect to, regardless of the record/workspace default. diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index f8a2c1039a8..17cd602aade 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -1,3 +1,5 @@ +#[cfg(unix)] +use std::os::unix::fs::OpenOptionsExt; use std::{ collections::HashMap, fs::{self, File, OpenOptions}, @@ -733,11 +735,19 @@ fn maybe_rotate_log(path: &Path) { let _ = fs::rename(path, &rotated); } +/// Open a managed-agent runtime log for appending, owner-only on Unix. +/// +/// Agent stdout/stderr can echo installers' and CLIs' credentials, so like +/// `open_install_log` the mode is set *in the create* — never chmod'd after, so +/// the file is not briefly readable to other local users. An existing file's +/// mode is left as-is, since `OpenOptions::mode` only applies on creation. pub(crate) fn open_log_file(path: &Path) -> Result { maybe_rotate_log(path); - OpenOptions::new() - .create(true) - .append(true) + let mut options = OpenOptions::new(); + options.create(true).append(true); + #[cfg(unix)] + options.mode(0o600); + options .open(path) .map_err(|error| format!("failed to open log file {}: {error}", path.display())) } @@ -798,11 +808,6 @@ fn open_install_log(path: &Path, truncate: bool) -> Result { .map_err(|error| format!("failed to open log file {}: {error}", path.display())) } -pub(crate) fn append_log_marker(path: &Path, message: &str) -> Result<(), String> { - let mut file = open_log_file(path)?; - writeln!(file, "{message}").map_err(|error| format!("failed to write log marker: {error}")) -} - fn agent_pids_dir(app: &AppHandle) -> Result { let dir = managed_agents_base_dir(app)?.join("agent-pids"); fs::create_dir_all(&dir) diff --git a/desktop/src-tauri/src/managed_agents/storage_tests.rs b/desktop/src-tauri/src/managed_agents/storage_tests.rs index 9943c6b3ac3..0e14e00e699 100644 --- a/desktop/src-tauri/src/managed_agents/storage_tests.rs +++ b/desktop/src-tauri/src/managed_agents/storage_tests.rs @@ -830,3 +830,27 @@ fn install_log_filename_accepts_ordinary_runtime_ids() { ); } } + +// ── runtime logs ───────────────────────────────────────────────────────────── + +/// Agent stdout/stderr (what runtime logs capture) can echo credentials from +/// installers and CLIs, so `0o600` must come from the create itself — a +/// post-write `chmod` would leave a window where the umask decides and the log +/// is briefly readable to other local users. +#[cfg(unix)] +#[test] +fn open_log_file_creates_owner_only() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("agent.log"); + + super::open_log_file(&path).expect("open runtime log"); + + let mode = std::fs::metadata(&path) + .expect("metadata") + .permissions() + .mode() + & 0o777; + assert_eq!(mode, 0o600, "runtime logs must be owner-only"); +}