From c4b94ebc678269ba3c5f666874e7313c652a8dcc Mon Sep 17 00:00:00 2001 From: Mark Fenner Date: Sun, 26 Jul 2026 22:57:18 -0400 Subject: [PATCH 1/7] feat(desktop): add native Devin ACP runtime Project runtime metadata, auth readiness, install guidance, safe launch policy, model ownership, and avatar presentation from KnownAcpRuntime. Preserve existing runtime behavior while adding lifecycle cleanup and focused catalog, readiness, path, storage, and UI tests. Signed-off-by: Mark Fenner --- desktop/public/runtime-icons/devin.svg | 4 + desktop/src-tauri/src/commands/agent_auth.rs | 86 ++++- .../src-tauri/src/commands/agent_config.rs | 36 +-- .../commands/agent_config/tests/fixtures.rs | 50 +++ .../src-tauri/src/commands/agent_discovery.rs | 30 +- desktop/src-tauri/src/commands/agents.rs | 10 +- .../src-tauri/src/commands/agents_tests.rs | 21 ++ desktop/src-tauri/src/lib.rs | 15 + .../managed_agents/config_bridge/reader.rs | 2 + .../config_bridge/reader_tests.rs | 40 +++ .../src/managed_agents/config_bridge/types.rs | 2 + .../src-tauri/src/managed_agents/discovery.rs | 305 +++++++----------- .../discovery/catalog_projection.rs | 45 +++ .../discovery/runtime_catalog.rs | 277 ++++++++++++++++ .../discovery/runtime_metadata.rs | 115 ++++++- .../src/managed_agents/discovery/tests.rs | 40 +-- .../managed_agents/discovery/tests/avatar.rs | 33 ++ .../managed_agents/discovery/tests/devin.rs | 111 +++++++ .../tests/managed_path_resolution.rs | 34 ++ .../src-tauri/src/managed_agents/readiness.rs | 87 ++--- .../src/managed_agents/readiness/cli_login.rs | 9 +- .../src/managed_agents/readiness/cli_probe.rs | 33 +- .../readiness/tests/devin_tests.rs | 88 +++++ .../readiness/tests/fixtures.rs | 50 +++ .../src-tauri/src/managed_agents/runtime.rs | 87 +++-- .../src/managed_agents/runtime/cli_config.rs | 22 ++ .../src/managed_agents/runtime/env_policy.rs | 171 ++++++++++ .../src/managed_agents/runtime/path.rs | 98 ++++-- .../managed_agents/runtime/presentation.rs | 53 +++ .../src/managed_agents/runtime/process.rs | 19 +- .../runtime/process_tree_tests.rs | 83 +++++ .../src/managed_agents/runtime/sweep.rs | 126 +++++++- .../src/managed_agents/runtime/tests.rs | 45 +-- .../src/managed_agents/runtime/tests/devin.rs | 46 +++ .../src-tauri/src/managed_agents/storage.rs | 108 +++---- .../managed_agents/storage/normalization.rs | 12 + .../managed_agents/storage/tests/avatar.rs | 28 ++ .../storage/tests/log_errors.rs | 60 ++++ .../storage/tests/log_permissions.rs | 29 ++ desktop/src-tauri/src/managed_agents/types.rs | 21 +- .../agents/lib/agentCardModelLabel.test.mjs | 31 ++ .../agents/lib/agentCardModelLabel.ts | 4 + .../lib/friendlyAgentLastError.test.mjs | 9 + .../lib/resolveAgentCardAvatarUrl.test.mjs | 67 ++++ .../agents/lib/resolveAgentCardAvatarUrl.ts | 21 ++ .../features/agents/ui/AgentConfigPanel.tsx | 23 +- desktop/src/features/agents/ui/AgentsView.tsx | 1 + .../agents/ui/UnifiedAgentsSection.tsx | 54 +++- .../features/onboarding/ui/RuntimeIcon.tsx | 38 +-- .../onboarding/ui/agentReadiness.test.mjs | 53 ++- .../features/onboarding/ui/agentReadiness.ts | 5 +- .../ui/onboardingRuntimeSelection.test.mjs | 80 +++-- .../ui/onboardingRuntimeSelection.ts | 23 +- .../src/features/profile/ui/ProfileAvatar.tsx | 3 + .../settings/ui/DoctorSettingsPanel.tsx | 31 +- desktop/src/shared/api/runtimeCapabilities.ts | 46 +++ desktop/src/shared/api/tauri.ts | 29 ++ desktop/src/shared/api/types.ts | 42 +-- desktop/src/testing/e2eBridge.ts | 45 +++ .../e2e/onboarding-agent-defaults.spec.ts | 128 +++++++- desktop/tests/e2e/onboarding.spec.ts | 2 + 61 files changed, 2551 insertions(+), 715 deletions(-) create mode 100644 desktop/public/runtime-icons/devin.svg create mode 100644 desktop/src-tauri/src/commands/agent_config/tests/fixtures.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/catalog_projection.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/tests/avatar.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs create mode 100644 desktop/src-tauri/src/managed_agents/readiness/tests/devin_tests.rs create mode 100644 desktop/src-tauri/src/managed_agents/readiness/tests/fixtures.rs create mode 100644 desktop/src-tauri/src/managed_agents/runtime/cli_config.rs create mode 100644 desktop/src-tauri/src/managed_agents/runtime/env_policy.rs create mode 100644 desktop/src-tauri/src/managed_agents/runtime/presentation.rs create mode 100644 desktop/src-tauri/src/managed_agents/runtime/process_tree_tests.rs create mode 100644 desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs create mode 100644 desktop/src-tauri/src/managed_agents/storage/normalization.rs create mode 100644 desktop/src-tauri/src/managed_agents/storage/tests/avatar.rs create mode 100644 desktop/src-tauri/src/managed_agents/storage/tests/log_errors.rs create mode 100644 desktop/src-tauri/src/managed_agents/storage/tests/log_permissions.rs create mode 100644 desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.test.mjs create mode 100644 desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.ts create mode 100644 desktop/src/shared/api/runtimeCapabilities.ts diff --git a/desktop/public/runtime-icons/devin.svg b/desktop/public/runtime-icons/devin.svg new file mode 100644 index 0000000000..e760797d68 --- /dev/null +++ b/desktop/public/runtime-icons/devin.svg @@ -0,0 +1,4 @@ + + + + diff --git a/desktop/src-tauri/src/commands/agent_auth.rs b/desktop/src-tauri/src/commands/agent_auth.rs index bba0f33860..388a28c7d4 100644 --- a/desktop/src-tauri/src/commands/agent_auth.rs +++ b/desktop/src-tauri/src/commands/agent_auth.rs @@ -68,6 +68,12 @@ pub async fn connect_acp_runtime( } fn discover_acp_auth_methods_blocking(runtime_id: &str) -> Result { + if let Some(runtime) = known_acp_runtime_exact(runtime_id) { + if let Some(methods) = catalog_cli_auth_methods(runtime) { + return Ok(methods); + } + } + let output = run_buzz_acp_auth_command(runtime_id, ["auth-methods", "--json"])?; if !output.status.success() { return Err(command_error("buzz-acp auth-methods", &output)); @@ -77,6 +83,23 @@ fn discover_acp_auth_methods_blocking(runtime_id: &str) -> Result Option { + let command = runtime.auth_login_args?; + Some(AcpAuthMethodsResult { + methods: vec![AcpAuthMethod { + id: "cli-login".to_string(), + name: format!("Sign in to {}", runtime.label), + description: runtime.login_hint.map(str::to_string), + method_type: Some("terminal".to_string()), + args: Vec::new(), + command: command.iter().map(|arg| (*arg).to_string()).collect(), + meta: None, + }], + }) +} + fn connect_acp_runtime_blocking( request: &ConnectAcpRuntimeRequest, ) -> Result { @@ -256,7 +279,7 @@ fn launch_terminal_auth(runtime_id: &str, method: &AcpAuthMethod) -> Result<(), .ok_or_else(|| format!("{} ACP adapter is not installed", runtime.label))?; let fallback_command = adapter_command.1.display().to_string(); let argv = adapter_terminal_argv(runtime.label, method, &fallback_command)?; - launch_visible_terminal(&argv) + launch_visible_terminal(&argv, runtime.scrub_env_vars) } fn adapter_terminal_argv( @@ -361,15 +384,16 @@ fn spawn_without_stdio(mut command: Command) -> Result<(), String> { } #[cfg(target_os = "macos")] -fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { +fn launch_visible_terminal(argv: &[String], scrub_env_vars: &[&str]) -> Result<(), String> { let mut script = tempfile::Builder::new() .prefix("buzz-auth-") .suffix(".command") .tempfile() .map_err(|error| format!("failed to create terminal login script: {error}"))?; + let unset_commands = shell_unset_commands(scrub_env_vars); writeln!( script, - "#!/bin/sh\ntrap 'rm -f -- \"$0\"' EXIT\n{}", + "#!/bin/sh\ntrap 'rm -f -- \"$0\"' EXIT\n{unset_commands}{}", shell_join(argv) ) .map_err(|error| format!("failed to write terminal login script: {error}"))?; @@ -395,8 +419,12 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { } #[cfg(target_os = "linux")] -fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { - let command = shell_join(argv); +fn launch_visible_terminal(argv: &[String], scrub_env_vars: &[&str]) -> Result<(), String> { + let command = format!( + "{}{}", + shell_unset_commands(scrub_env_vars), + shell_join(argv) + ); let candidates: [(&str, &[&str]); 4] = [ ("x-terminal-emulator", &["-e", "sh", "-lc"]), ("gnome-terminal", &["--", "sh", "-lc"]), @@ -406,6 +434,9 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { for (terminal, prefix) in candidates { let mut terminal_command = Command::new(terminal); terminal_command.args(prefix).arg(&command); + for key in scrub_env_vars { + terminal_command.env_remove(key); + } if spawn_without_stdio(terminal_command).is_ok() { return Ok(()); } @@ -414,7 +445,7 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { } #[cfg(target_os = "windows")] -fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { +fn launch_visible_terminal(argv: &[String], scrub_env_vars: &[&str]) -> Result<(), String> { use std::os::windows::process::CommandExt; const CREATE_NEW_CONSOLE: u32 = 0x0000_0010; @@ -425,6 +456,9 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { command .args(windows_terminal_args(argv)) .creation_flags(CREATE_NEW_CONSOLE); + for key in scrub_env_vars { + command.env_remove(key); + } spawn_without_stdio(command) } @@ -436,10 +470,18 @@ fn windows_terminal_args(argv: &[String]) -> Vec { } #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))] -fn launch_visible_terminal(_argv: &[String]) -> Result<(), String> { +fn launch_visible_terminal(_argv: &[String], _scrub_env_vars: &[&str]) -> Result<(), String> { Err("opening a terminal is not supported on this platform".to_string()) } +#[cfg(any(target_os = "macos", target_os = "linux", test))] +fn shell_unset_commands(scrub_env_vars: &[&str]) -> String { + scrub_env_vars + .iter() + .map(|key| format!("unset {}\n", shell_escape(key))) + .collect() +} + fn shell_join(argv: &[String]) -> String { argv.iter() .map(|arg| shell_escape(arg)) @@ -461,11 +503,35 @@ fn shell_escape(arg: &str) -> String { #[cfg(test)] mod tests { use super::{ - adapter_terminal_argv, append_inherited_path, is_claude_subscription_login, - run_buzz_acp_auth_command_with_paths, shell_escape, shell_join, uses_terminal_auth, - windows_terminal_args, AcpAuthMethod, + adapter_terminal_argv, append_inherited_path, catalog_cli_auth_methods, + is_claude_subscription_login, run_buzz_acp_auth_command_with_paths, shell_escape, + shell_join, shell_unset_commands, uses_terminal_auth, windows_terminal_args, AcpAuthMethod, }; + #[test] + fn devin_uses_catalog_declared_visible_terminal_login() { + let runtime = + crate::managed_agents::known_acp_runtime_exact("devin").expect("Devin runtime"); + let result = catalog_cli_auth_methods(runtime).expect("catalog CLI login method"); + + assert_eq!(result.methods.len(), 1); + assert_eq!(result.methods[0].id, "cli-login"); + assert_eq!(result.methods[0].method_type.as_deref(), Some("terminal")); + assert_eq!( + result.methods[0].command, + ["devin", "auth", "login"].map(str::to_string) + ); + } + + #[test] + fn terminal_login_scrubs_only_catalog_declared_identity_overrides() { + assert_eq!( + shell_unset_commands(&["WINDSURF_API_KEY"]), + "unset WINDSURF_API_KEY\n" + ); + assert!(shell_unset_commands(&[]).is_empty()); + } + /// Windows regression: the augmented PATH there holds only Buzz-managed /// dirs and the exe parent (no login-shell PATH, no managed Node), so the /// user's inherited PATH must be appended for npm `.cmd` adapters to find diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 461fed5dbd..43f168c100 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -606,40 +606,8 @@ mod tests { use super::*; use crate::managed_agents::{BackendKind, RespondTo}; - fn goose_runtime() -> &'static KnownAcpRuntime { - &KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - } - } + mod fixtures; + use fixtures::goose_runtime; fn agent_record() -> ManagedAgentRecord { ManagedAgentRecord { diff --git a/desktop/src-tauri/src/commands/agent_config/tests/fixtures.rs b/desktop/src-tauri/src/commands/agent_config/tests/fixtures.rs new file mode 100644 index 0000000000..1a4ec99c3f --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_config/tests/fixtures.rs @@ -0,0 +1,50 @@ +use super::*; + +pub(super) fn goose_runtime() -> &'static KnownAcpRuntime { + &KnownAcpRuntime { + id: "goose", + label: "Goose", + display_label: "Goose", + sort_priority: 1, + onboarding_visible: false, + commands: &["goose"], + aliases: &[], + default_args: &["acp"], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, + avatar_url: "", + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "", + adapter_install_instructions_url: "", + cli_install_hint: "", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: Some("GOOSE_MODEL"), + provider_env_var: Some("GOOSE_PROVIDER"), + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.config/goose/config.yaml"), + config_file_format: Some("yaml"), + supports_acp_native_config: true, + thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), + context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + } +} diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 84a00433c0..5e597cdc32 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -108,9 +108,7 @@ pub async fn save_custom_harness( original_id: Option, app: tauri::AppHandle, ) -> Result { - use crate::managed_agents::{ - custom_harnesses, AcpAvailabilityStatus, AuthStatus, HarnessSource, - }; + use crate::managed_agents::{custom_harnesses, AcpAvailabilityStatus}; use tauri::Manager; // ── Phase 1: full validation before touching the filesystem ───────────── @@ -164,31 +162,13 @@ pub async fn save_custom_harness( let default_args = crate::managed_agents::normalize_agent_args(&definition.command, definition.args.clone()); - Ok(AcpRuntimeCatalogEntry { - id: definition.id, - label: definition.label, - // Security: no user-supplied avatar URL in catalog entries. - avatar_url: String::new(), + Ok(crate::managed_agents::custom_runtime_catalog_entry( + definition, availability, - command: command_opt, + command_opt, binary_path, default_args, - mcp_command: None, - model_env_var: None, - provider_env_var: None, - thinking_env_var: None, - install_hint: definition.install_hint, - install_instructions_url: definition.install_instructions_url, - can_auto_install: false, - requires_external_cli: false, - underlying_cli_path: None, - node_required: false, - auth_status: AuthStatus::NotApplicable, - login_hint: None, - source: HarnessSource::Custom, - // Carry definition env back so the edit form can read and preserve it. - definition_env: definition.env, - }) + )) } /// Remove a user-defined harness definition from `/custom_harnesses/`. diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 3b5ebeca4f..9cbea37949 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -7,11 +7,11 @@ use crate::{ build_managed_agent_summary, current_instance_id, discover_provider_candidates, ensure_persona_is_active, find_managed_agent_mut, load_managed_agents, load_personas, load_teams, managed_agent_avatar_url, managed_agents_base_dir, normalize_agent_args, - provider_deploy, resolve_provider_binary, save_managed_agents, start_managed_agent_process, - stop_managed_agent_process, stop_managed_agent_workspace_pair, + provider_deploy, resolve_agent_parallelism, resolve_provider_binary, save_managed_agents, + start_managed_agent_process, stop_managed_agent_process, stop_managed_agent_workspace_pair, sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind, CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentRecord, - ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, + ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, @@ -872,7 +872,7 @@ pub async fn create_managed_agent( input.parallelism, linked_persona.as_ref(), )?; - + let resolved_parallelism = resolve_agent_parallelism(minted.parallelism, &agent_command); let record = crate::managed_agents::ManagedAgentRecord { pubkey: pubkey.clone(), name: name.clone(), @@ -900,7 +900,7 @@ pub async fn create_managed_agent( // 0 or None → harness uses its own default (320s idle, 3600s max), and the CLI also clamps 0 → minimum. idle_timeout_seconds: input.idle_timeout_seconds.filter(|s| *s > 0), max_turn_duration_seconds: input.max_turn_duration_seconds.filter(|s| *s > 0), - parallelism: minted.parallelism.unwrap_or(DEFAULT_AGENT_PARALLELISM), + parallelism: resolved_parallelism, system_prompt: snapshot_prompt.or_else(|| { input .system_prompt diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index e32fc1cfe4..422b6c990a 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -86,6 +86,27 @@ fn persona_record(id: &str, model: Option<&str>, provider: Option<&str>) -> Agen } } +#[test] +fn created_devin_agents_default_to_one_worker() { + assert_eq!(resolve_agent_parallelism(None, "devin"), 1); +} + +#[test] +fn created_existing_runtime_agents_keep_the_global_default() { + for command in ["goose", "claude-agent-acp", "codex-acp", "buzz-agent"] { + assert_eq!( + resolve_agent_parallelism(None, command), + crate::managed_agents::DEFAULT_AGENT_PARALLELISM, + "{command}" + ); + } +} + +#[test] +fn explicit_or_persona_parallelism_overrides_runtime_default() { + assert_eq!(resolve_agent_parallelism(Some(3), "devin"), 3); +} + /// Auto-archive uses the same NIP-IA wire builder as the explicit GUI action, /// attaches owner consent, and marks a deliberate delete as `retired`. #[test] diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 64405d0440..96efd42992 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -395,6 +395,21 @@ pub fn run() { migration::run_boot_migrations(&app_handle); } + // Reclaim subprocesses left behind by an ungraceful prior exit + // before the webview can send messages. Agent restoration remains + // deferred until apply_workspace installs the correct relay, but + // cleanup needs neither relay nor identity and must not share that + // delay: a stale lazy harness can still subscribe as the same agent + // and consume the first post-relaunch prompt. + // + // The single-instance plugin has already admitted this process. + // Receipt, same-instance marker, and exact-bundle checks keep + // upstream Buzz plus other bundle identifiers out of scope. + let startup_instance_id = managed_agents::current_instance_id(&app_handle); + managed_agents::sweep_orphaned_agent_processes(&app_handle, &[]); + managed_agents::sweep_system_agent_processes(&startup_instance_id, &[]); + managed_agents::sweep_untracked_bundle_harnesses(&[]); + // Resolve persisted identity key (env var → file → generate+save). // This is fatal — the app should not start with an ephemeral identity // that will be lost on restart, as that silently breaks channel diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index 372d2cfde1..4539848bb7 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -205,6 +205,8 @@ pub(crate) fn read_config_surface( RuntimeConfigSurface { runtime_id: runtime_meta.map(|m| m.id.to_string()), runtime_label: runtime_meta.map(|m| m.label.to_string()), + supports_buzz_model_config: runtime_meta + .map(|m| m.model_env_var.is_some() || m.supports_acp_model_switching), is_pre_spawn, normalized, advanced, diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 4c11cd6c49..c52f6c804d 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -31,9 +31,19 @@ fn test_runtime() -> &'static KnownAcpRuntime { &KnownAcpRuntime { id: "goose", label: "Goose", + display_label: "Goose", + sort_priority: 1, + onboarding_visible: false, commands: &["goose"], aliases: &[], + default_args: &["acp"], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, avatar_url: "", + superseded_avatar_urls: &[], mcp_command: None, mcp_hooks: false, underlying_cli: None, @@ -46,10 +56,13 @@ fn test_runtime() -> &'static KnownAcpRuntime { adapter_install_hint: "", skill_dir: None, supports_acp_model_switching: false, + accepts_harness_model: true, model_env_var: Some("GOOSE_MODEL"), provider_env_var: Some("GOOSE_PROVIDER"), provider_locked: false, default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], config_file_path: Some("~/.config/goose/config.yaml"), config_file_format: Some("yaml"), supports_acp_native_config: true, @@ -59,6 +72,7 @@ fn test_runtime() -> &'static KnownAcpRuntime { required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, + auth_login_args: None, } } @@ -131,6 +145,18 @@ fn pre_spawn_surface_reports_pending_acp_tiers() { ConfigTierStatus::Pending ); assert_eq!(surface.sources.env_vars, ConfigTierStatus::Available); + assert_eq!(surface.supports_buzz_model_config, Some(true)); +} + +#[test] +fn devin_surface_projects_runtime_owned_model_capability() { + let record = test_record(); + let runtime = + crate::managed_agents::known_acp_runtime("devin").expect("Devin must remain cataloged"); + let surface = read_config_surface(&record, Some(runtime), None, None); + + assert_eq!(surface.runtime_id.as_deref(), Some("devin")); + assert_eq!(surface.supports_buzz_model_config, Some(false)); } #[test] @@ -607,9 +633,19 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { &KnownAcpRuntime { id: "buzz-agent", label: "Buzz Agent", + display_label: "Buzz", + sort_priority: 0, + onboarding_visible: false, commands: &["buzz-agent"], aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, avatar_url: "", + superseded_avatar_urls: &[], mcp_command: None, mcp_hooks: false, underlying_cli: None, @@ -622,10 +658,13 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { adapter_install_hint: "", skill_dir: None, supports_acp_model_switching: true, + accepts_harness_model: true, model_env_var: Some("BUZZ_AGENT_MODEL"), provider_env_var: Some("BUZZ_AGENT_PROVIDER"), provider_locked: false, default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], config_file_path: None, config_file_format: None, supports_acp_native_config: false, @@ -635,6 +674,7 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, + auth_login_args: None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs index 15ccb718e7..33559aff38 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs @@ -132,6 +132,8 @@ pub struct ConfigSourceReport { pub struct RuntimeConfigSurface { pub runtime_id: Option, pub runtime_label: Option, + /// Catalog-projected model capability. `None` for unknown runtimes. + pub supports_buzz_model_config: Option, pub is_pre_spawn: bool, pub normalized: NormalizedConfig, pub advanced: Vec, diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 2b7264b429..c903000194 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -7,19 +7,21 @@ use std::time::{Duration, Instant}; use crate::managed_agents::{ buzz_managed_command_path, buzz_managed_node_bin_dir, buzz_managed_npm_bin_dir, AcpAvailabilityStatus, AcpRuntimeCatalogEntry, AuthStatus, CommandAvailabilityInfo, - HarnessSource, + HarnessSource, DEFAULT_AGENT_PARALLELISM, }; +mod catalog_projection; +mod runtime_catalog; mod runtime_metadata; +pub(crate) use catalog_projection::custom_runtime_catalog_entry; +use runtime_catalog::KNOWN_ACP_RUNTIMES; +#[cfg(test)] +use runtime_catalog::{ + BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, +}; pub(crate) use runtime_metadata::KnownAcpRuntime; -const GOOSE_AVATAR_URL: &str = "https://goose-docs.ai/img/logo_dark.png"; -const CLAUDE_CODE_AVATAR_URL: &str = "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default"; -const CODEX_AVATAR_URL: &str = "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default"; -const BUZZ_AGENT_AVATAR_URL: &str = - "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png"; - fn common_binary_paths() -> &'static [PathBuf] { static PATHS: OnceLock> = OnceLock::new(); PATHS.get_or_init(|| { @@ -63,140 +65,6 @@ fn common_binary_paths() -> &'static [PathBuf] { }) } -const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ - KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: GOOSE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("goose"), - cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], - // Goose's stable release currently publishes only the Unix installer; - // its official Windows instructions intentionally point at this main-branch script. - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"$env:CONFIGURE='false'; irm https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1 | iex\""], - adapter_install_commands: &[], - cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", - adapter_install_instructions_url: "", - cli_install_hint: "Buzz requires the Goose CLI; the desktop app alone is not enough.", - adapter_install_hint: "", - skill_dir: Some(".goose/skills"), - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[("GOOSE_MODE", "auto")], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, - KnownAcpRuntime { - id: "claude", - label: "Claude Code", - commands: &["claude-agent-acp", "claude-code-acp"], - aliases: &["claude-code", "claudecode"], - avatar_url: CLAUDE_CODE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("claude"), - cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://claude.ai/install.ps1 | iex\""], - adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], - cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", - cli_install_hint: "Buzz requires the Claude Code CLI; the desktop app alone is not enough.", - adapter_install_hint: "Install the Claude Code ACP adapter via npm.", - skill_dir: Some(".claude/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: true, - default_env: &[], - config_file_path: Some("~/.claude/settings.json"), - config_file_format: Some("json"), - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run the Claude CLI to complete authentication."), - auth_probe_args: Some(&["claude", "auth", "status"]), - }, - KnownAcpRuntime { - id: "codex", - label: "Codex", - commands: &["codex-acp"], - aliases: &[], - avatar_url: CODEX_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: false, - underlying_cli: Some("codex"), - cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://chatgpt.com/codex/install.ps1 | iex\""], - adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], - cli_install_instructions_url: "https://developers.openai.com/codex/cli/", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", - cli_install_hint: "Buzz requires the Codex CLI; the desktop app alone is not enough.", - adapter_install_hint: "Install the Codex ACP adapter via npm.", - skill_dir: Some(".codex/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.codex/config.toml"), - config_file_format: Some("toml"), - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run `codex login` to authenticate."), - // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. - auth_probe_args: Some(&["codex", "login", "status"]), - }, - KnownAcpRuntime { - id: "buzz-agent", - label: "Buzz Agent", - commands: &["buzz-agent"], - aliases: &[], - avatar_url: BUZZ_AGENT_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: true, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "https://github.com/block/buzz", - adapter_install_instructions_url: "https://github.com/block/buzz", - cli_install_hint: "Ships with the Buzz desktop app.", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: true, - model_env_var: Some("BUZZ_AGENT_MODEL"), - provider_env_var: Some("BUZZ_AGENT_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: None, - config_file_format: None, - supports_acp_native_config: false, - thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), - max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), - context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, -]; - /// Skill discovery directories declared by known runtimes. pub(crate) fn known_skill_dirs() -> impl Iterator { KNOWN_ACP_RUNTIMES.iter().filter_map(|p| p.skill_dir) @@ -266,6 +134,12 @@ pub(crate) fn known_acp_runtime_exact(id: &str) -> Option<&'static KnownAcpRunti KNOWN_ACP_RUNTIMES.iter().find(|p| p.id == id) } +pub(crate) fn resolve_agent_parallelism(requested: Option, command: &str) -> u32 { + requested + .or_else(|| known_acp_runtime(command).and_then(|runtime| runtime.default_parallelism)) + .unwrap_or(DEFAULT_AGENT_PARALLELISM) +} + /// The agent command a freshly-created agent defaults to when the create /// request supplies none. Resolves the bundled `buzz-agent` from the catalog so /// the default cannot drift from the provider definition. Falls back to the id @@ -455,12 +329,13 @@ pub fn try_record_agent_command( } fn default_agent_args(command: &str) -> Option> { - match normalize_command_identity(command).as_str() { - "goose" => Some(vec!["acp".to_string()]), - "codex" | "codex-acp" | "claude-agent-acp" | "claude-code-acp" | "claude-code" - | "claudecode" | "buzz-agent" => Some(Vec::new()), - _ => None, - } + known_acp_runtime(command).map(|runtime| { + runtime + .default_args + .iter() + .map(|arg| (*arg).to_string()) + .collect() + }) } pub fn normalize_agent_args(command: &str, agent_args: Vec) -> Vec { @@ -486,8 +361,8 @@ pub fn normalize_agent_args(command: &str, agent_args: Vec) -> Vec [PathBuf; 2] { - if cfg!(debug_assertions) { +fn profile_target_dirs(root: &Path, debug_build: bool) -> [PathBuf; 2] { + if debug_build { // `just dev` builds fresh debug sidecars; never prefer stale release output. [root.join("target/debug"), root.join("target/release")] } else { @@ -495,17 +370,31 @@ fn profile_target_dirs(root: &Path) -> [PathBuf; 2] { } } -fn command_search_dirs() -> Vec { - let mut dirs = profile_target_dirs(&workspace_root_dir()).to_vec(); - if let Ok(current_dir) = std::env::current_dir() { - dirs.extend(profile_target_dirs(¤t_dir)); +fn command_search_dirs_for( + workspace_root: &Path, + current_dir: Option<&Path>, + executable_dir: Option<&Path>, + debug_build: bool, +) -> Vec { + let mut dirs = Vec::new(); + + // A packaged release must run the sidecar that was signed and shipped + // beside the desktop executable. Build-machine checkout paths can still + // exist on a developer Mac; searching them first silently mixes an + // installed release with stale target/debug binaries. + if !debug_build { + dirs.extend(executable_dir.map(Path::to_path_buf)); + } + + dirs.extend(profile_target_dirs(workspace_root, debug_build)); + if let Some(current_dir) = current_dir { + dirs.extend(profile_target_dirs(current_dir, debug_build)); + } + + if debug_build { + dirs.extend(executable_dir.map(Path::to_path_buf)); } - dirs.extend( - std::env::current_exe() - .ok() - .and_then(|path| path.parent().map(Path::to_path_buf)), - ); dirs.into_iter().fold(Vec::new(), |mut unique, dir| { if !unique.contains(&dir) { unique.push(dir); @@ -514,6 +403,19 @@ fn command_search_dirs() -> Vec { }) } +fn command_search_dirs() -> Vec { + let current_dir = std::env::current_dir().ok(); + let executable_dir = std::env::current_exe() + .ok() + .and_then(|path| path.parent().map(Path::to_path_buf)); + command_search_dirs_for( + &workspace_root_dir(), + current_dir.as_deref(), + executable_dir.as_deref(), + cfg!(debug_assertions), + ) +} + fn is_executable_file(path: &Path) -> bool { let Ok(metadata) = path.metadata() else { return false; @@ -534,7 +436,7 @@ fn is_executable_file(path: &Path) -> bool { } } -fn resolve_workspace_command(command: &str) -> Option { +pub(crate) fn resolve_workspace_command(command: &str) -> Option { if command_looks_like_path(command) { let path = PathBuf::from(command); return is_executable_file(&path).then_some(path); @@ -1019,16 +921,18 @@ pub(crate) fn is_npm_global_install(cmd: &str) -> bool { /// background threads to prevent pipe-buffer deadlock. On timeout the child is /// killed and `Unknown` is returned; no orphaned threads or processes are left /// behind. Returns `Unknown` on timeout. -fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { +fn probe_auth_status( + binary_path: &Path, + probe_args: &[&str], + scrub_env_vars: &[&str], +) -> AuthStatus { use crate::managed_agents::readiness::cli_probe; let augmented_path = cli_probe::augmented_path(); let mut command = std::process::Command::new(binary_path); command.args(&probe_args[1..]); - if let Some(ref path) = augmented_path { - command.env("PATH", path); - } + cli_probe::configure_probe_environment(&mut command, augmented_path.as_deref(), scrub_env_vars); command .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) @@ -1326,10 +1230,11 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr .and_then(find_command) .map(|p| p.display().to_string()); - let default_args = command - .as_deref() - .map(|cmd| normalize_agent_args(cmd, Vec::new())) - .unwrap_or_default(); + let default_args = runtime + .default_args + .iter() + .map(|arg| (*arg).to_string()) + .collect(); let can_auto_install = !runtime.cli_install_commands_for_os().is_empty() || !runtime.adapter_install_commands.is_empty(); @@ -1376,7 +1281,19 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr entry: AcpRuntimeCatalogEntry { id: runtime.id.to_string(), label: runtime.label.to_string(), + display_label: runtime.display_label.to_string(), + sort_priority: runtime.sort_priority, + onboarding_visible: runtime.onboarding_visible, + icon_url: runtime.icon_url.to_string(), + icon_scale: runtime.icon_scale, avatar_url: runtime.avatar_url.to_string(), + superseded_avatar_urls: runtime + .superseded_avatar_urls + .iter() + .map(|url| (*url).to_string()) + .collect(), + supports_buzz_model_config: runtime.model_env_var.is_some() + || runtime.supports_acp_model_switching, availability, command, binary_path, @@ -1483,8 +1400,15 @@ fn preset_catalog_entry( AcpRuntimeCatalogEntry { id: def.id.to_string(), label: def.label.to_string(), + display_label: def.label.to_string(), + sort_priority: 100, + onboarding_visible: false, + icon_url: String::new(), + icon_scale: 1.0, // No remote URL — all preset icons are bundled assets. avatar_url: String::new(), + superseded_avatar_urls: Vec::new(), + supports_buzz_model_config: true, availability, command, binary_path, @@ -1669,10 +1593,11 @@ pub fn discover_acp_runtimes_from( // Need the resolved binary path for the CLI (e.g. the actual `claude` binary). let binary_path = resolve_command(probe_args[0])?; let probe_args_owned: Vec = probe_args.iter().map(|s| s.to_string()).collect(); + let scrub_env_vars = partial.runtime.scrub_env_vars; let handle = std::thread::spawn(move || { let refs: Vec<&str> = probe_args_owned.iter().map(String::as_str).collect(); - probe_auth_status(&binary_path, &refs) + probe_auth_status(&binary_path, &refs, scrub_env_vars) }); Some((idx, handle)) }) @@ -1746,37 +1671,13 @@ pub fn discover_acp_runtimes_from( let default_args = normalize_agent_args(&def.command, def.args.clone()); - entries.push(AcpRuntimeCatalogEntry { - id: def.id.clone(), - label: def.label.clone(), - // F1 security fix: never copy user-supplied avatar URL into the catalog. - // All icons are bundled assets; customs fall back to TerminalSquare in the UI. - avatar_url: String::new(), + entries.push(custom_runtime_catalog_entry( + def, availability, command, binary_path, default_args, - // Custom harnesses are plain ACP — no MCP sidecar, no env-var - // model switching, no thinking knobs. - mcp_command: None, - model_env_var: None, - provider_env_var: None, - thinking_env_var: None, - install_hint: def.install_hint.clone(), - install_instructions_url: def.install_instructions_url.clone(), - // Security line: custom definitions carry no install scripts. - can_auto_install: false, - requires_external_cli: false, - underlying_cli_path: None, - node_required: false, - // No auth probe for custom harnesses. - auth_status: AuthStatus::NotApplicable, - login_hint: None, - source: HarnessSource::Custom, - // Carry definition env into the catalog so the edit form can - // read it back — prevents silently erasing env on save. - definition_env: def.env.clone(), - }); + )); } } @@ -1830,5 +1731,19 @@ pub fn managed_agent_avatar_url(command: &str) -> Option { Some(runtime.avatar_url.to_string()) } +/// Replace a superseded catalog-default avatar without touching user-selected +/// images. This is intentionally a read-time normalization: existing records +/// render correctly immediately, their relay profiles reconcile to the new +/// default, and the normalized value is persisted on the next ordinary save. +pub fn normalize_managed_agent_avatar(command: &str, avatar_url: Option) -> Option { + if let (Some(runtime), Some(avatar)) = (known_acp_runtime(command), avatar_url.as_deref()) { + if runtime.superseded_avatar_urls.contains(&avatar) { + return Some(runtime.avatar_url.to_string()); + } + } + + avatar_url +} + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/discovery/catalog_projection.rs b/desktop/src-tauri/src/managed_agents/discovery/catalog_projection.rs new file mode 100644 index 0000000000..71a4eee221 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/catalog_projection.rs @@ -0,0 +1,45 @@ +use crate::managed_agents::{ + custom_harnesses::HarnessDefinition, AcpAvailabilityStatus, AcpRuntimeCatalogEntry, AuthStatus, + HarnessSource, +}; + +pub(crate) fn custom_runtime_catalog_entry( + definition: HarnessDefinition, + availability: AcpAvailabilityStatus, + command: Option, + binary_path: Option, + default_args: Vec, +) -> AcpRuntimeCatalogEntry { + AcpRuntimeCatalogEntry { + id: definition.id, + display_label: definition.label.clone(), + label: definition.label, + sort_priority: 100, + onboarding_visible: false, + icon_url: String::new(), + icon_scale: 1.0, + // User-controlled custom avatar URLs never enter the catalog. + avatar_url: String::new(), + superseded_avatar_urls: Vec::new(), + // Preserve the established custom-harness model surface. + supports_buzz_model_config: true, + availability, + command, + binary_path, + default_args, + mcp_command: None, + model_env_var: None, + provider_env_var: None, + thinking_env_var: None, + install_hint: definition.install_hint, + install_instructions_url: definition.install_instructions_url, + can_auto_install: false, + requires_external_cli: false, + underlying_cli_path: None, + node_required: false, + auth_status: AuthStatus::NotApplicable, + login_hint: None, + source: HarnessSource::Custom, + definition_env: definition.env, + } +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs new file mode 100644 index 0000000000..c1804159f2 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs @@ -0,0 +1,277 @@ +use super::KnownAcpRuntime; + +pub(super) const GOOSE_AVATAR_URL: &str = "https://goose-docs.ai/img/logo_dark.png"; +pub(super) const CLAUDE_CODE_AVATAR_URL: &str = "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default"; +pub(super) const CODEX_AVATAR_URL: &str = "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default"; +pub(super) const LEGACY_DEVIN_AVATAR_URL: &str = "https://mintcdn.com/cognitionai/Hhrl_8XUBqA4VQ6v/logo/favicon.svg?fit=max&auto=format&n=Hhrl_8XUBqA4VQ6v&q=85&s=ab641f30c01bf5374b90b62209db569e"; +// The official Devin mark is transparent. Keep the profile avatar self-contained +// and add a white canvas so the black mark remains visible in dark themes. +pub(super) const DEVIN_AVATAR_URL: &str = concat!( + "data:image/svg+xml,%3Csvg%20width%3D%22425%22%20height%3D%22425%22%20viewBox%3D%220%200%20425%20425%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E", + "%3Crect%20width%3D%22425%22%20height%3D%22425%22%20fill%3D%22white%22%2F%3E", + "%3Cpath%20d%3D%22M70%20159.333V91.3471C70%2088.3592%2071.594%2085.5983%2074.1816%2084.1044L133.043%2050.1205C135.631%2048.6265%20138.819%2048.6265%20141.407%2050.1205L200.269%2084.1044C202.856%2085.5983%20204.45%2088.3592%20204.45%2091.3471V126.068C204.708%20137.606%20210.806%20148.734%20221.531%20154.926C232.256%20161.117%20244.942%20160.834%20255.063%20155.289L285.132%20137.929C287.719%20136.435%20290.907%20136.435%20293.495%20137.929L352.357%20171.913C354.944%20173.406%20356.538%20176.167%20356.538%20179.155V247.123C356.538%20250.111%20354.944%20252.872%20352.357%20254.366L293.495%20288.35C290.907%20289.844%20287.719%20289.844%20285.132%20288.35L255.306%20271.13C245.146%20265.456%20232.344%20265.117%20221.534%20271.358C210.809%20277.55%20204.711%20288.678%20204.453%20300.215V334.926C204.453%20337.914%20202.859%20340.675%20200.271%20342.169L141.41%20376.153C138.822%20377.647%20135.634%20377.647%20133.046%20376.153L74.1845%20342.169C71.5969%20340.675%2070.0028%20337.914%2070.0028%20334.926V266.959C70.0029%20263.971%2071.5969%20261.21%2074.1845%20259.716L133.046%20225.732C135.634%20224.238%20138.822%20224.238%20141.41%20225.732L171.547%20243.132C181.656%20248.638%20194.306%20248.906%20205.005%20242.729C215.815%20236.488%20221.922%20225.231%20222.088%20213.595C221.83%20202.057%20215.732%20189.737%20205.008%20183.545C194.283%20177.353%20181.597%20177.636%20171.476%20183.181L141.269%20200.72C138.67%20202.229%20135.461%20202.228%20132.864%20200.716L74.1576%20166.562C71.5835%20165.065%2070%20162.311%2070%20159.333Z%22%20fill%3D%22black%22%2F%3E", + "%3C%2Fsvg%3E" +); +pub(super) const BUZZ_AGENT_AVATAR_URL: &str = + "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png"; + +pub(super) const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ + KnownAcpRuntime { + id: "goose", + label: "Goose", + display_label: "Goose", + sort_priority: 50, + onboarding_visible: true, + commands: &["goose"], + aliases: &[], + default_args: &["acp"], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/runtime-icons/goose.svg", + icon_scale: 1.25, + avatar_url: GOOSE_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("goose"), + cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], + // Goose's stable release currently publishes only the Unix installer; + // its official Windows instructions intentionally point at this main-branch script. + cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"$env:CONFIGURE='false'; irm https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1 | iex\""], + adapter_install_commands: &[], + cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz requires the Goose CLI; the desktop app alone is not enough.", + adapter_install_hint: "", + skill_dir: Some(".goose/skills"), + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: Some("GOOSE_MODEL"), + provider_env_var: Some("GOOSE_PROVIDER"), + provider_locked: false, + default_env: &[("GOOSE_MODE", "auto")], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.config/goose/config.yaml"), + config_file_format: Some("yaml"), + supports_acp_native_config: true, + thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), + context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + }, + KnownAcpRuntime { + id: "claude", + label: "Claude Code", + display_label: "Claude Code", + sort_priority: 30, + onboarding_visible: true, + commands: &["claude-agent-acp", "claude-code-acp"], + aliases: &["claude-code", "claudecode"], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/runtime-icons/claude.png", + icon_scale: 1.1, + avatar_url: CLAUDE_CODE_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("claude"), + cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], + cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://claude.ai/install.ps1 | iex\""], + adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], + cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", + cli_install_hint: "Buzz requires the Claude Code CLI; the desktop app alone is not enough.", + adapter_install_hint: "Install the Claude Code ACP adapter via npm.", + skill_dir: Some(".claude/skills"), + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: None, + provider_env_var: None, + provider_locked: true, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.claude/settings.json"), + config_file_format: Some("json"), + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run the Claude CLI to complete authentication."), + auth_probe_args: Some(&["claude", "auth", "status"]), + auth_login_args: None, + }, + KnownAcpRuntime { + id: "codex", + label: "Codex", + display_label: "Codex", + sort_priority: 40, + onboarding_visible: true, + commands: &["codex-acp"], + aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/runtime-icons/codex.png", + icon_scale: 1.1, + avatar_url: CODEX_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: false, + underlying_cli: Some("codex"), + cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], + cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://chatgpt.com/codex/install.ps1 | iex\""], + adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], + cli_install_instructions_url: "https://developers.openai.com/codex/cli/", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", + cli_install_hint: "Buzz requires the Codex CLI; the desktop app alone is not enough.", + adapter_install_hint: "Install the Codex ACP adapter via npm.", + skill_dir: Some(".codex/skills"), + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.codex/config.toml"), + config_file_format: Some("toml"), + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run `codex login` to authenticate."), + // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. + auth_probe_args: Some(&["codex", "login", "status"]), + auth_login_args: None, + }, + KnownAcpRuntime { + id: "devin", + label: "Devin", + display_label: "Devin", + sort_priority: 20, + onboarding_visible: true, + commands: &["devin"], + aliases: &[], + default_args: &["acp"], + // One official CLI session is the safe local default. Explicit request + // or persona values still win. + default_parallelism: Some(1), + // The official CLI performs meaningful startup work before its first + // prompt. Pay that cost when the managed runtime starts so the first + // user message does not also become a process-health probe. + defer_agent_start_until_work: false, + // Devin normally emits ACP progress well inside this window. A fully + // silent native server is replaced and the queued batch retried rather + // than appearing to hang under the generic 15-minute tool allowance. + default_idle_timeout_secs: Some(120), + icon_url: "/runtime-icons/devin.svg", + icon_scale: 1.1, + avatar_url: DEVIN_AVATAR_URL, + superseded_avatar_urls: &[LEGACY_DEVIN_AVATAR_URL], + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("devin"), + cli_install_commands: &["curl -fsSL https://cli.devin.ai/install.sh | bash"], + cli_install_commands_windows: &[ + "powershell.exe -NoProfile -Command \"irm https://static.devin.ai/cli/setup.ps1 | iex\"", + ], + adapter_install_commands: &[], + cli_install_instructions_url: "https://docs.devin.ai/cli", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz requires the Devin CLI; the desktop app alone is not enough.", + adapter_install_hint: "", + skill_dir: Some(".devin/skills"), + supports_acp_model_switching: false, + // Devin's native ACP server owns model choice. Passing Buzz's global + // model would imply a capability we do not expose and causes the + // official CLI to reject unrelated Buzz model IDs before falling back. + accepts_harness_model: false, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + // Buzz's harness historically defaults to bypassing ACP permission + // requests. The native Devin runtime must always retain the official + // CLI's safe permission behavior. + enforced_env: &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ], + // The official CLI gives this legacy ambient key precedence over the + // account established by `devin auth login`. Remove it without reading + // its value so readiness and usage attribution share one identity. + scrub_env_vars: &["WINDSURF_API_KEY"], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run `devin auth login` to authenticate."), + // Verified locally: the command exits 0 for an authenticated CLI. + auth_probe_args: Some(&["devin", "auth", "status"]), + auth_login_args: Some(&["devin", "auth", "login"]), + }, + KnownAcpRuntime { + id: "buzz-agent", + label: "Buzz Agent", + display_label: "Buzz", + sort_priority: 60, + onboarding_visible: true, + commands: &["buzz-agent"], + aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/app-icon@2x.png", + icon_scale: 1.1, + avatar_url: BUZZ_AGENT_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: true, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "https://github.com/block/buzz", + adapter_install_instructions_url: "https://github.com/block/buzz", + cli_install_hint: "Ships with the Buzz desktop app.", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: true, + accepts_harness_model: true, + model_env_var: Some("BUZZ_AGENT_MODEL"), + provider_env_var: Some("BUZZ_AGENT_PROVIDER"), + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), + max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), + context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + }, +]; diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs index 2fb6a471d4..9a6be59707 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs @@ -2,9 +2,38 @@ pub(crate) struct KnownAcpRuntime { pub id: &'static str, pub label: &'static str, + /// Compact product label used by runtime pickers. + pub display_label: &'static str, + /// Stable catalog ordering before the label tie-breaker. + pub sort_priority: u16, + /// Whether first-run onboarding should offer this runtime. + pub onboarding_visible: bool, pub commands: &'static [&'static str], pub aliases: &'static [&'static str], + /// Arguments used when the runtime is launched without an explicit argv. + pub default_args: &'static [&'static str], + /// Runtime-specific worker default used when neither the request nor a + /// linked persona specifies parallelism. `None` preserves Buzz's global + /// default. + pub default_parallelism: Option, + /// Whether a desktop-requested lazy harness should defer spawning the ACP + /// subprocess until accepted work is queued. Runtimes with expensive or + /// failure-prone first handshakes can opt out while preserving the lazy + /// relay socket. + pub defer_agent_start_until_work: bool, + /// Runtime-specific idle timeout used only when the agent record, process + /// environment, and merged user environment do not supply an override. + /// `None` preserves the harness default. + pub default_idle_timeout_secs: Option, + /// App-local runtime mark used by catalog-driven frontend surfaces. + pub icon_url: &'static str, + /// Presentation scale for the runtime mark. Kept with the catalog entry so + /// React does not need a harness-ID lookup table. + pub icon_scale: f32, pub avatar_url: &'static str, + /// Catalog-default avatar URLs superseded by `avatar_url`. These are not + /// user-selected images and may be replaced during read-time migration. + pub superseded_avatar_urls: &'static [&'static str], /// Legacy MCP server binary field. Vestigial — all agents now use the bundled CLI /// directly. Will be removed when runtime discovery is simplified. pub mcp_command: Option<&'static str>, @@ -34,14 +63,28 @@ pub(crate) struct KnownAcpRuntime { /// runtime reads the canonical path directly or has no skill support. pub skill_dir: Option<&'static str>, /// Whether this runtime handles model switching via ACP protocol natively. - /// Currently unused — env var injection runs unconditionally regardless of - /// this value. Retained as scaffolding for when ACP model switching matures. - #[allow(dead_code)] + /// Env var injection still handles initial model selection separately. pub supports_acp_model_switching: bool, + /// Whether Buzz should pass its resolved model through the generic + /// `BUZZ_ACP_MODEL` harness setting at process launch. + /// + /// This is intentionally separate from `supports_acp_model_switching` and + /// `model_env_var`: existing adapters may consume the generic bootstrap + /// model without exposing Buzz-side model controls. Native runtimes whose + /// official ACP server owns model selection set this to `false`. + pub accepts_harness_model: bool, pub model_env_var: Option<&'static str>, pub provider_env_var: Option<&'static str>, pub provider_locked: bool, + /// Environment defaults applied only when neither the parent process nor + /// saved agent configuration supplies a value. pub default_env: &'static [(&'static str, &'static str)], + /// Environment values enforced at process launch after inherited and + /// user-configured values have been merged. + pub enforced_env: &'static [(&'static str, &'static str)], + /// Environment variables removed from runtime subprocesses. This prevents + /// ambient process state from overriding catalog-declared identity policy. + pub scrub_env_vars: &'static [&'static str], pub config_file_path: Option<&'static str>, #[allow(dead_code)] // reserved for format-based dispatch when readers are unified pub config_file_format: Option<&'static str>, @@ -62,6 +105,9 @@ pub(crate) struct KnownAcpRuntime { /// CLI args for probing authentication status. `args[0]` is the binary name; /// the remainder are the subcommand. `None` for runtimes with no login step. pub auth_probe_args: Option<&'static [&'static str]>, + /// CLI argv for an interactive login launched in a visible terminal. + /// `None` when authentication is adapter-owned or not applicable. + pub auth_login_args: Option<&'static [&'static str]>, } impl KnownAcpRuntime { @@ -85,6 +131,23 @@ impl KnownAcpRuntime { mod tests { use super::super::known_acp_runtime_exact; + #[test] + fn onboarding_visibility_and_order_are_catalog_owned() { + let mut visible = super::super::KNOWN_ACP_RUNTIMES + .iter() + .filter(|runtime| runtime.onboarding_visible) + .collect::>(); + visible.sort_by_key(|runtime| runtime.sort_priority); + + assert_eq!( + visible + .into_iter() + .map(|runtime| runtime.id) + .collect::>(), + ["devin", "claude", "codex", "goose", "buzz-agent"] + ); + } + #[test] fn vendor_metadata_distinguishes_cli_and_adapter_guidance() { let goose = known_acp_runtime_exact("goose").unwrap(); @@ -120,5 +183,51 @@ mod tests { ); assert!(codex.adapter_install_instructions_url.contains("codex-acp")); assert!(codex.cli_install_hint.contains("desktop app alone")); + + let devin = known_acp_runtime_exact("devin").unwrap(); + assert_eq!(devin.commands, &["devin"]); + assert_eq!(devin.default_args, &["acp"]); + assert_eq!(devin.default_parallelism, Some(1)); + assert!(!devin.defer_agent_start_until_work); + assert_eq!(devin.default_idle_timeout_secs, Some(120)); + assert_eq!(devin.display_label, "Devin"); + assert_eq!(devin.sort_priority, 20); + assert!(devin.onboarding_visible); + assert_eq!(devin.icon_url, "/runtime-icons/devin.svg"); + assert_eq!(devin.icon_scale, 1.1); + assert_eq!(devin.underlying_cli, Some("devin")); + assert_eq!(devin.skill_dir, Some(".devin/skills")); + assert_eq!( + devin.auth_probe_args, + Some(&["devin", "auth", "status"][..]) + ); + assert_eq!(devin.auth_login_args, Some(&["devin", "auth", "login"][..])); + assert!(devin.default_env.is_empty()); + assert_eq!( + devin.enforced_env, + &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ] + ); + assert_eq!(devin.scrub_env_vars, &["WINDSURF_API_KEY"]); + assert_eq!( + devin.cli_install_instructions_url, + "https://docs.devin.ai/cli" + ); + assert_eq!( + devin.cli_install_commands, + &["curl -fsSL https://cli.devin.ai/install.sh | bash"] + ); + assert_eq!( + devin.cli_install_commands_windows, + &[ + "powershell.exe -NoProfile -Command \"irm https://static.devin.ai/cli/setup.ps1 | iex\"" + ] + ); + assert!(devin.adapter_install_commands.is_empty()); + assert!(devin.cli_install_hint.contains("desktop app alone")); } } diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 48e8d5479c..726d5e9977 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -6,43 +6,15 @@ use super::{ codex_adapter_is_outdated, create_time_agent_command_override, default_agent_command, effective_agent_command, find_nvm_default_bin, find_via_login_shell, is_login_shell_path_uninit, is_safe_nvm_tag, managed_agent_avatar_url, normalize_agent_args, - parse_semver_tag, preset_catalog_entry, probe_codex_acp_major_version, record_agent_command, - refresh_login_shell_path, try_record_agent_command, PresetHarness, BUZZ_AGENT_AVATAR_URL, - CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, + normalize_managed_agent_avatar, parse_semver_tag, preset_catalog_entry, + probe_codex_acp_major_version, record_agent_command, refresh_login_shell_path, + try_record_agent_command, PresetHarness, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, + CODEX_AVATAR_URL, GOOSE_AVATAR_URL, }; use crate::managed_agents::AcpAvailabilityStatus; -#[test] -fn resolves_known_avatar_for_bare_command() { - let avatar_url = managed_agent_avatar_url("goose").expect("goose avatar should resolve"); - - assert_eq!(avatar_url, GOOSE_AVATAR_URL); -} - -#[test] -fn resolves_known_avatar_for_command_paths_and_aliases() { - assert_eq!( - managed_agent_avatar_url("/usr/local/bin/codex-acp"), - Some(CODEX_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url("Claude Code"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url(r"C:\Tools\claude-agent-acp.exe"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url("/usr/local/bin/claude-code-acp"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); -} - -#[test] -fn returns_none_for_unknown_commands() { - assert!(managed_agent_avatar_url("custom-agent").is_none()); -} +mod avatar; +mod devin; #[test] fn default_agent_command_resolves_bundled_buzz_agent() { diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/avatar.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/avatar.rs new file mode 100644 index 0000000000..af40349d01 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/avatar.rs @@ -0,0 +1,33 @@ +use super::*; + +#[test] +fn resolves_known_avatar_for_bare_command() { + let avatar_url = managed_agent_avatar_url("goose").expect("goose avatar should resolve"); + + assert_eq!(avatar_url, GOOSE_AVATAR_URL); +} + +#[test] +fn resolves_known_avatar_for_command_paths_and_aliases() { + assert_eq!( + managed_agent_avatar_url("/usr/local/bin/codex-acp"), + Some(CODEX_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url("Claude Code"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url(r"C:\Tools\claude-agent-acp.exe"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url("/usr/local/bin/claude-code-acp"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); +} + +#[test] +fn returns_none_for_unknown_commands() { + assert!(managed_agent_avatar_url("custom-agent").is_none()); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs new file mode 100644 index 0000000000..31b4664721 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs @@ -0,0 +1,111 @@ +use super::*; + +#[test] +fn resolves_devin_avatar() { + let avatar_url = + managed_agent_avatar_url("/usr/local/bin/devin").expect("Devin avatar should resolve"); + + assert_eq!(avatar_url, super::super::runtime_catalog::DEVIN_AVATAR_URL); + assert!(avatar_url.starts_with("data:image/svg+xml,")); + assert!(avatar_url.contains("fill%3D%22white%22")); +} + +#[test] +fn migrates_only_the_superseded_devin_default_avatar() { + let migrated = normalize_managed_agent_avatar( + "devin", + Some(super::super::runtime_catalog::LEGACY_DEVIN_AVATAR_URL.to_string()), + ); + assert_eq!( + migrated.as_deref(), + Some(super::super::runtime_catalog::DEVIN_AVATAR_URL) + ); + + let custom = Some("https://example.test/custom-devin.png".to_string()); + assert_eq!( + normalize_managed_agent_avatar("devin", custom.clone()), + custom + ); + + let other_runtime = Some(super::super::runtime_catalog::LEGACY_DEVIN_AVATAR_URL.to_string()); + assert_eq!( + normalize_managed_agent_avatar("goose", other_runtime.clone()), + other_runtime + ); +} + +#[test] +fn normalizes_devin_args_to_native_acp_subcommand() { + assert_eq!(normalize_agent_args("devin", Vec::new()), vec!["acp"]); + assert_eq!( + normalize_agent_args("/usr/local/bin/devin", vec!["".into()]), + vec!["acp"] + ); + assert_eq!( + normalize_agent_args( + "devin", + vec!["acp".into(), "--agent-type".into(), "review".into()] + ), + vec!["acp", "--agent-type", "review"] + ); +} + +#[test] +fn runtime_catalog_exposes_devin_once() { + let devin_entries = super::super::KNOWN_ACP_RUNTIMES + .iter() + .filter(|runtime| runtime.id == "devin") + .collect::>(); + + assert_eq!(devin_entries.len(), 1); + let devin = devin_entries[0]; + assert_eq!(devin.label, "Devin"); + assert_eq!(devin.display_label, "Devin"); + assert_eq!(devin.sort_priority, 20); + assert!(devin.onboarding_visible); + assert_eq!(devin.commands, &["devin"]); + assert_eq!(devin.default_args, &["acp"]); + assert_eq!(devin.icon_url, "/runtime-icons/devin.svg"); + assert_eq!(devin.icon_scale, 1.1); + assert_eq!(devin.underlying_cli, Some("devin")); + assert_eq!(devin.skill_dir, Some(".devin/skills")); + assert_eq!( + devin.auth_probe_args, + Some(&["devin", "auth", "status"][..]) + ); + assert!(devin.default_env.is_empty()); + assert_eq!( + devin.enforced_env, + &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ] + ); + assert_eq!(devin.scrub_env_vars, &["WINDSURF_API_KEY"]); + assert!(!devin.supports_acp_model_switching); + assert!(!devin.accepts_harness_model); +} + +#[test] +fn runtime_discovery_exposes_devin_entry() { + let runtimes = super::super::discover_acp_runtimes_from(None); + let devin = runtimes + .iter() + .find(|runtime| runtime.id == "devin") + .expect("runtime discovery must project the Devin catalog entry"); + + assert_eq!(devin.label, "Devin"); + assert_eq!(devin.display_label, "Devin"); + assert_eq!(devin.sort_priority, 20); + assert!(devin.onboarding_visible); + assert_eq!(devin.icon_url, "/runtime-icons/devin.svg"); + assert_eq!(devin.icon_scale, 1.1); + assert_eq!( + devin.superseded_avatar_urls, + [super::super::runtime_catalog::LEGACY_DEVIN_AVATAR_URL] + ); + assert!(!devin.supports_buzz_model_config); + assert_eq!(devin.default_args, ["acp"]); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs index 2f6b038deb..6995754ab5 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs @@ -1,4 +1,38 @@ use crate::managed_agents::discovery::{clear_resolve_cache, resolve_command}; +use std::path::{Path, PathBuf}; + +#[test] +fn packaged_release_prefers_its_bundled_sidecars_over_checkout_targets() { + let bundle = PathBuf::from("/Applications/Buzz.app/Contents/MacOS"); + let dirs = super::super::command_search_dirs_for( + Path::new("/build/buzz"), + Some(Path::new("/Users/developer/buzz")), + Some(&bundle), + false, + ); + + assert_eq!(dirs.first(), Some(&bundle)); + assert_eq!(dirs[1], PathBuf::from("/build/buzz/target/release")); + assert_eq!(dirs[2], PathBuf::from("/build/buzz/target/debug")); +} + +#[test] +fn debug_build_keeps_fresh_workspace_sidecars_ahead_of_executable_dir() { + let bundle = PathBuf::from("/build/buzz/target/debug"); + let dirs = super::super::command_search_dirs_for( + Path::new("/build/buzz"), + Some(Path::new("/build/buzz/desktop")), + Some(&bundle), + true, + ); + + assert_eq!(dirs.first(), Some(&bundle)); + assert_eq!(dirs[1], PathBuf::from("/build/buzz/target/release")); + assert_eq!( + dirs.last(), + Some(&PathBuf::from("/build/buzz/desktop/target/release")) + ); +} #[cfg(unix)] #[test] diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c5480b2479..c2bbc187ca 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -390,9 +390,9 @@ impl AgentReadiness { /// - `openai` → `OPENAI_COMPAT_API_KEY` /// - `databricks` / `databricks_v2` → `DATABRICKS_HOST` (token optional — /// OAuth PKCE is the fallback) -/// * **claude**: a successful `claude auth status` probe. -/// * **codex**: a successful `codex login status` probe (checks the codex -/// credential store — NOT `OPENAI_API_KEY`). +/// * **CLI-login runtimes**: a successful catalog-declared authentication +/// probe (for example `claude auth status`, `codex login status`, or +/// `devin auth status`). /// * **unknown / custom command**: always `Ready` (no requirements known). /// /// Databricks note: `DATABRICKS_TOKEN` is `.unwrap_or_default()` in @@ -427,7 +427,7 @@ fn collect_missing_requirements( return vec![]; }; - match rt.id { + let runtime_specific = match rt.id { "buzz-agent" => buzz_agent_requirements(effective), "goose" => { // Read the file config once at the call site so the inner fn is @@ -435,14 +435,21 @@ fn collect_missing_requirements( let file_cfg = read_goose_file_config(); goose_requirements(effective, file_cfg.as_ref()) } - "claude" => cli_login::requirements( - &["claude", "auth", "status"], - "complete Claude Code authentication by running the Claude CLI", - rt, - ), - "codex" => cli_login::requirements(&["codex", "login", "status"], "run `codex login`", rt), _ => vec![], + }; + if !runtime_specific.is_empty() || matches!(rt.id, "buzz-agent" | "goose") { + return runtime_specific; } + + let Some(probe_args) = rt.auth_probe_args else { + return vec![]; + }; + cli_login::requirements( + probe_args, + rt.login_hint + .unwrap_or("Complete authentication in the runtime CLI."), + rt, + ) } /// Requirements for buzz-agent (provider + model + provider-specific creds). @@ -645,6 +652,9 @@ mod tests { use super::*; use crate::managed_agents::discovery::known_acp_runtime_exact; + mod devin_tests; + mod fixtures; + /// Build a minimal `EffectiveAgentEnv` with the given env map and command. fn make_env(command: &str, env: BTreeMap) -> EffectiveAgentEnv { let runtime = known_acp_runtime_exact(command); @@ -1012,34 +1022,10 @@ mod tests { KnownAcpRuntime { id: "test-cli-runtime", label: "Test CLI", + display_label: "Test CLI", commands, - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, underlying_cli, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - config_file_path: None, - config_file_format: None, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, + ..fixtures::known_runtime_fixture() } } @@ -1207,34 +1193,11 @@ mod tests { KnownAcpRuntime { id: "codex", label: "Codex", + display_label: "Codex", commands: adapter_commands, - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, underlying_cli, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - config_file_path: None, - config_file_format: None, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, + onboarding_visible: true, + ..fixtures::known_runtime_fixture() } } diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs index 4036d9f239..2c0bbb4312 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs @@ -10,7 +10,7 @@ use crate::managed_agents::{ use super::{cli_probe, Requirement}; -/// Requirements for CLI-login runtimes (claude, codex). +/// Requirements for runtimes with a catalog-declared CLI authentication probe. pub(super) fn requirements( probe_args: &[&str], setup_copy: &str, @@ -47,7 +47,12 @@ pub(super) fn requirements( )]; }; let augmented_path = cli_probe::augmented_path(); - match cli_probe::login_probe(&binary_path, probe_args, augmented_path.as_deref()) { + match cli_probe::login_probe( + &binary_path, + probe_args, + augmented_path.as_deref(), + runtime.scrub_env_vars, + ) { cli_probe::ProbeOutcome::LoggedIn => vec![], cli_probe::ProbeOutcome::LoggedOut => vec![missing_requirement( probe_args, diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs index 513da4e2a8..247ddb09a1 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs @@ -57,12 +57,11 @@ pub(crate) fn login_probe( binary_path: &Path, probe_args: &[&str], augmented_path: Option<&str>, + scrub_env_vars: &[&str], ) -> ProbeOutcome { let mut command = std::process::Command::new(binary_path); command.args(&probe_args[1..]); - if let Some(path) = augmented_path { - command.env("PATH", path); - } + configure_probe_environment(&mut command, augmented_path, scrub_env_vars); crate::util::configure_no_window(&mut command); match command.output() { @@ -72,6 +71,19 @@ pub(crate) fn login_probe( } } +pub(crate) fn configure_probe_environment( + command: &mut std::process::Command, + augmented_path: Option<&str>, + scrub_env_vars: &[&str], +) { + if let Some(path) = augmented_path { + command.env("PATH", path); + } + for key in scrub_env_vars { + command.env_remove(key); + } +} + /// Classify collected probe output into a `ProbeOutcome`. /// /// Shared between `login_probe` (which has the full `Output`) and the @@ -100,6 +112,18 @@ pub(crate) fn classify_probe_output(stderr_bytes: &[u8], exit_success: bool) -> mod tests { use super::{ProbeOutcome, CONFIG_PARSE_SIGNALS}; + #[test] + fn probe_environment_removes_catalog_declared_identity_overrides() { + let mut command = std::process::Command::new("devin"); + command.env("WINDSURF_API_KEY", "sentinel"); + + super::configure_probe_environment(&mut command, None, &["WINDSURF_API_KEY"]); + + assert!(command + .get_envs() + .any(|(key, value)| { key == "WINDSURF_API_KEY" && value.is_none() })); + } + #[cfg(unix)] #[test] fn login_probe_uses_augmented_path_for_env_shebang_interpreter() { @@ -154,6 +178,7 @@ mod tests { &script_path, &["fake-codex", "login", "status"], Some(&augmented_path), + &[], ), ProbeOutcome::LoggedIn, "the injected augmented PATH should allow /usr/bin/env to find the interpreter" @@ -187,6 +212,7 @@ mod tests { &script_path, &["fake-codex-bad-config", "login", "status"], None, + &[], ); assert!( matches!(outcome, ProbeOutcome::ConfigInvalid { .. }), @@ -225,6 +251,7 @@ mod tests { &script_path, &["fake-codex-logged-out", "login", "status"], None, + &[], ); assert_eq!( outcome, diff --git a/desktop/src-tauri/src/managed_agents/readiness/tests/devin_tests.rs b/desktop/src-tauri/src/managed_agents/readiness/tests/devin_tests.rs new file mode 100644 index 0000000000..28f1343499 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness/tests/devin_tests.rs @@ -0,0 +1,88 @@ +use super::*; + +fn devin_runtime_for_test( + commands: &'static [&'static str], + underlying_cli: Option<&'static str>, + auth_probe_args: &'static [&'static str], +) -> KnownAcpRuntime { + KnownAcpRuntime { + id: "devin", + label: "Devin", + default_args: &["acp"], + login_hint: Some("Run `devin auth login` to authenticate."), + auth_probe_args: Some(auth_probe_args), + auth_login_args: Some(&["devin", "auth", "login"]), + ..make_cli_runtime(commands, underlying_cli) + } +} + +#[test] +fn devin_readiness_is_ready_when_auth_probe_succeeds() { + let exe = present_binary_str(); + let runtime = devin_runtime_for_test( + static_commands(vec![exe]), + Some(exe), + static_commands(vec![exe, "--list"]), + ); + let effective = EffectiveAgentEnv { + env: BTreeMap::new(), + config_file_path: None, + effective_command: "devin".to_string(), + }; + + assert!( + collect_missing_requirements(&effective, Some(&runtime)).is_empty(), + "a successful catalog-declared Devin auth probe must be ready" + ); +} + +#[test] +fn devin_readiness_requires_login_when_auth_probe_fails() { + let exe = present_binary_str(); + let runtime = devin_runtime_for_test( + static_commands(vec![exe]), + Some(exe), + static_commands(vec![exe, "--buzz-probe-fail-xyz"]), + ); + let effective = EffectiveAgentEnv { + env: BTreeMap::new(), + config_file_path: None, + effective_command: "devin".to_string(), + }; + + let requirements = collect_missing_requirements(&effective, Some(&runtime)); + assert_eq!(requirements.len(), 1); + assert!(matches!( + &requirements[0], + Requirement::CliLogin { + availability: AcpAvailabilityStatus::Available, + setup_copy, + .. + } if setup_copy.contains("devin auth login") + )); +} + +#[test] +fn devin_readiness_reports_missing_cli_before_authentication() { + let missing = "__buzz_nonexistent_devin_xyz789__"; + let runtime = devin_runtime_for_test( + static_commands(vec![missing]), + Some(missing), + static_commands(vec![missing, "auth", "status"]), + ); + let effective = EffectiveAgentEnv { + env: BTreeMap::new(), + config_file_path: None, + effective_command: "devin".to_string(), + }; + + let requirements = collect_missing_requirements(&effective, Some(&runtime)); + assert_eq!(requirements.len(), 1); + assert!(matches!( + requirements[0], + Requirement::CliLogin { + availability: AcpAvailabilityStatus::NotInstalled, + .. + } + )); +} diff --git a/desktop/src-tauri/src/managed_agents/readiness/tests/fixtures.rs b/desktop/src-tauri/src/managed_agents/readiness/tests/fixtures.rs new file mode 100644 index 0000000000..0c5994cb5f --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness/tests/fixtures.rs @@ -0,0 +1,50 @@ +use crate::managed_agents::KnownAcpRuntime; + +pub(super) fn known_runtime_fixture() -> KnownAcpRuntime { + KnownAcpRuntime { + id: "test-runtime", + label: "Test runtime", + display_label: "Test runtime", + sort_priority: 100, + onboarding_visible: false, + commands: &[], + aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, + avatar_url: "", + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "", + adapter_install_instructions_url: "", + cli_install_hint: "", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 32b3b328e9..13413b111e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -8,8 +8,8 @@ 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, + spawn_key_refusal, ManagedAgentPairRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey, + ManagedAgentSummary, }, util::now_iso, }; @@ -20,9 +20,21 @@ pub(crate) use path::compose_path_entries; pub(crate) use path::should_skip_claude_executable; pub(crate) use path::should_use_inherited; +mod cli_config; +pub(crate) use cli_config::configure_runtime_cli; + mod metadata; pub(crate) use metadata::{resolve_effective_prompt_model_provider, runtime_metadata_env_vars}; +mod env_policy; +use env_policy::{ + apply_runtime_env_policy, child_rust_log_filter, effective_idle_timeout, + harness_model_for_runtime, should_defer_agent_start, +}; + +mod presentation; +use presentation::runtime_presentation_for_summary; + mod stop; pub(crate) use stop::managed_agent_runtime_keys; pub use stop::{stop_managed_agent_process, stop_managed_agent_workspace_pair}; @@ -293,6 +305,7 @@ pub fn build_managed_agent_summary( .and_then(|r| r.mcp_command) .unwrap_or("") .to_string(); + let runtime_presentation = runtime_presentation_for_summary(&descriptor.command); Ok(ManagedAgentSummary { pubkey: record.pubkey.clone(), @@ -312,6 +325,10 @@ pub fn build_managed_agent_summary( parallelism: record.parallelism, system_prompt: effective_prompt, avatar_url: record.avatar_url.clone(), + runtime_icon_url: runtime_presentation.icon_url, + runtime_avatar_url: runtime_presentation.avatar_url, + runtime_superseded_avatar_urls: runtime_presentation.superseded_avatar_urls, + supports_buzz_model_config: runtime_presentation.supports_buzz_model_config, model: effective_model, model_source, provider: effective_provider, @@ -418,30 +435,6 @@ pub(crate) fn build_respond_to_env( Ok((set, remove)) } -pub(crate) fn configure_runtime_cli( - command: &mut std::process::Command, - runtime: Option<&KnownAcpRuntime>, -) { - let Some(runtime) = runtime else { - return; - }; - if runtime.id != "claude" { - return; - } - if let Some(cli_path) = runtime.underlying_cli.and_then(resolve_command) { - // On Windows, `.cmd` and `.bat` files are batch shims — they cannot be - // passed directly to `CreateProcess` and cause EINVAL when the Claude - // adapter tries to spawn them (issue #2397). Skip setting - // `CLAUDE_CODE_EXECUTABLE` for shim paths so the adapter falls back to - // its own PATH lookup and finds the real binary instead. - // Non-Windows: `.cmd`/`.bat` are valid executables and must be assigned. - if should_skip_claude_executable(&cli_path, cfg!(windows)) { - return; - } - command.env("CLAUDE_CODE_EXECUTABLE", cli_path); - } -} - /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -541,6 +534,7 @@ 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()); + let runtime_meta = known_acp_runtime(effective_command); // The caller supplies the explicit canonical pair relay. This is the only // relay this child may connect to, regardless of the record/workspace default. @@ -576,7 +570,11 @@ pub fn spawn_agent_child( command.env("RUST_LOG", child_rust_log_filter()); command.env("BUZZ_PRIVATE_KEY", &record.private_key_nsec); command.env("BUZZ_RELAY_URL", &effective_relay_url); - command.env("BUZZ_ACP_LAZY_POOL", if lazy { "true" } else { "false" }); + let defer_agent_start = should_defer_agent_start(lazy, runtime_meta); + command.env( + "BUZZ_ACP_LAZY_POOL", + if defer_agent_start { "true" } else { "false" }, + ); command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); command.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(",")); match &resolved_mcp_command { @@ -589,7 +587,6 @@ pub fn spawn_agent_child( } // Enable MCP hook tools (_Stop, _PostCompact) for agents that need them. // Uses "*" because build_mcp_servers() hard-codes the server name to "buzz-mcp". - let runtime_meta = known_acp_runtime(effective_command); if runtime_meta.is_some_and(|r| r.mcp_hooks) { command.env("MCP_HOOK_SERVERS", "*"); } @@ -710,13 +707,16 @@ pub fn spawn_agent_child( ); } } - // Only emit BUZZ_ACP_IDLE_TIMEOUT when the user has explicitly set an - // override. When unset, the buzz-acp harness applies its own default - // (see `DEFAULT_IDLE_TIMEOUT_SECS` in crates/buzz-acp/src/config.rs), - // which is the single source of truth. The previously-emitted - // `BUZZ_ACP_TURN_TIMEOUT` is deprecated upstream and was pinning every - // agent to the desktop's stale default (320s), bypassing harness bumps. - if let Some(idle) = record.idle_timeout_seconds { + // Emit BUZZ_ACP_IDLE_TIMEOUT for an explicit agent override or a + // KnownAcpRuntime catalog default. Otherwise the buzz-acp harness applies + // its generic default. Preserve an inherited process value; the merged + // global/persona/agent environment below can still override a catalog + // default. + if let Some(idle) = effective_idle_timeout( + record.idle_timeout_seconds, + std::env::var_os("BUZZ_ACP_IDLE_TIMEOUT").is_some(), + runtime_meta, + ) { command.env("BUZZ_ACP_IDLE_TIMEOUT", idle.to_string()); } @@ -755,13 +755,14 @@ pub fn spawn_agent_child( let effective_prompt = effective_cfg.system_prompt.value; let effective_model = effective_cfg.model.value; let effective_provider = effective_cfg.provider.value; + let harness_model = harness_model_for_runtime(runtime_meta, effective_model.as_deref()); if let Some(prompt) = &effective_prompt { command.env("BUZZ_ACP_SYSTEM_PROMPT", prompt); } else { command.env_remove("BUZZ_ACP_SYSTEM_PROMPT"); } - if let Some(model) = effective_model.as_deref() { + if let Some(model) = harness_model { command.env("BUZZ_ACP_MODEL", model); } else { command.env_remove("BUZZ_ACP_MODEL"); @@ -868,6 +869,10 @@ pub fn spawn_agent_child( } } + // Runtime identity and safety policy goes last so ambient, global, + // persona, and per-agent values cannot silently override it. + apply_runtime_env_policy(&mut command, runtime_meta); + // Stamp desktop ownership and an unpredictable harness-generation identity. let start_nonce = uuid::Uuid::new_v4().simple().to_string(); command @@ -950,14 +955,6 @@ pub fn spawn_agent_child( }) } -fn child_rust_log_filter() -> String { - match std::env::var("RUST_LOG") { - Ok(existing) if existing.contains("buzz_acp") => existing, - Ok(existing) if !existing.trim().is_empty() => format!("{existing},buzz_acp=info"), - _ => "buzz_acp=info".to_string(), - } -} - pub fn start_managed_agent_process( app: &AppHandle, record: &mut ManagedAgentRecord, @@ -1015,5 +1012,7 @@ pub fn start_managed_agent_process( Ok(()) } +#[cfg(all(test, unix))] +mod process_tree_tests; #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/cli_config.rs b/desktop/src-tauri/src/managed_agents/runtime/cli_config.rs new file mode 100644 index 0000000000..1d2c6d3954 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/cli_config.rs @@ -0,0 +1,22 @@ +use std::process::Command; + +use crate::managed_agents::{resolve_command, KnownAcpRuntime}; + +use super::should_skip_claude_executable; + +pub(crate) fn configure_runtime_cli(command: &mut Command, runtime: Option<&KnownAcpRuntime>) { + let Some(runtime) = runtime else { + return; + }; + if runtime.id != "claude" { + return; + } + if let Some(cli_path) = runtime.underlying_cli.and_then(resolve_command) { + // Windows batch shims cannot be passed directly to CreateProcess. + // Let the adapter use PATH when the resolved CLI is a shim. + if should_skip_claude_executable(&cli_path, cfg!(windows)) { + return; + } + command.env("CLAUDE_CODE_EXECUTABLE", cli_path); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs b/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs new file mode 100644 index 0000000000..b2768c5822 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs @@ -0,0 +1,171 @@ +use std::process::Command; + +use crate::managed_agents::KnownAcpRuntime; + +pub(super) fn child_rust_log_filter() -> String { + match std::env::var("RUST_LOG") { + Ok(existing) if existing.contains("buzz_acp") => existing, + Ok(existing) if !existing.trim().is_empty() => format!("{existing},buzz_acp=info"), + _ => "buzz_acp=info".to_string(), + } +} + +/// Honor the desktop caller's lazy request only when the runtime catalog says +/// deferring the ACP subprocess is appropriate. +pub(super) fn should_defer_agent_start( + requested_lazy: bool, + runtime: Option<&KnownAcpRuntime>, +) -> bool { + requested_lazy + && runtime + .map(|runtime| runtime.defer_agent_start_until_work) + .unwrap_or(true) +} + +/// Resolve the idle timeout that desktop should write into the harness +/// environment. Explicit record values win; an inherited process value remains +/// untouched; otherwise the runtime catalog may provide a safer default. +pub(super) fn effective_idle_timeout( + configured: Option, + inherited_is_set: bool, + runtime: Option<&KnownAcpRuntime>, +) -> Option { + configured.or_else(|| { + (!inherited_is_set) + .then(|| runtime.and_then(|runtime| runtime.default_idle_timeout_secs)) + .flatten() + }) +} + +/// Apply launch-only runtime environment policy after all user environment +/// layers have been merged. +pub(super) fn apply_runtime_env_policy(command: &mut Command, runtime: Option<&KnownAcpRuntime>) { + let Some(runtime) = runtime else { + return; + }; + for key in runtime.scrub_env_vars { + command.env_remove(key); + } + for (key, value) in runtime.enforced_env { + command.env(key, value); + } +} + +/// Resolve the generic harness bootstrap model for a known runtime. +/// +/// Unknown/custom runtimes preserve the historical behavior because Buzz +/// cannot infer their ACP capabilities. Known runtime policy comes only from +/// `KnownAcpRuntime`, so launch code never needs a runtime-ID branch. +pub(super) fn harness_model_for_runtime<'a>( + runtime: Option<&KnownAcpRuntime>, + effective_model: Option<&'a str>, +) -> Option<&'a str> { + if runtime.is_some_and(|runtime| !runtime.accepts_harness_model) { + None + } else { + effective_model + } +} + +#[cfg(test)] +mod tests { + use super::{ + apply_runtime_env_policy, effective_idle_timeout, harness_model_for_runtime, + should_defer_agent_start, + }; + use crate::managed_agents::known_acp_runtime; + + #[test] + fn devin_policy_enforces_safe_permissions_and_stored_login_identity() { + let mut command = std::process::Command::new("buzz-acp"); + command.env("BUZZ_ACP_PERMISSION_MODE", "bypassPermissions"); + command.env("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "true"); + command.env("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "false"); + command.env("WINDSURF_API_KEY", "sentinel"); + + apply_runtime_env_policy(&mut command, known_acp_runtime("devin")); + + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_PERMISSION_MODE" && value.is_some_and(|value| value == "default") + })); + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_AUTO_APPROVE_PERMISSIONS" + && value.is_some_and(|value| value == "false") + })); + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_INTERACTIVE_PERMISSIONS" && value.is_some_and(|value| value == "true") + })); + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE" + && value.is_some_and(|value| value == "30") + })); + assert!(command + .get_envs() + .any(|(key, value)| { key == "WINDSURF_API_KEY" && value.is_none() })); + } + + #[test] + fn existing_runtime_policy_remains_unchanged() { + let mut command = std::process::Command::new("buzz-acp"); + command.env("GOOSE_MODE", "custom"); + + apply_runtime_env_policy(&mut command, known_acp_runtime("goose")); + + assert!(command.get_envs().any(|(key, value)| { + key == "GOOSE_MODE" && value.is_some_and(|value| value == "custom") + })); + } + + #[test] + fn devin_owns_its_model_selection_without_changing_existing_runtime_bootstrap() { + let requested = Some("swe-1-7-lightning"); + let devin = known_acp_runtime("devin").expect("Devin must remain cataloged"); + assert_eq!(harness_model_for_runtime(Some(devin), requested), None); + + for runtime_id in ["goose", "claude", "codex", "buzz-agent"] { + let runtime = + known_acp_runtime(runtime_id).expect("existing runtime must remain cataloged"); + assert_eq!( + harness_model_for_runtime(Some(runtime), requested), + requested, + "{runtime_id} bootstrap behavior must remain unchanged" + ); + } + + assert_eq!( + harness_model_for_runtime(None, requested), + requested, + "custom runtimes preserve historical bootstrap behavior" + ); + } + + #[test] + fn devin_starts_eagerly_without_changing_existing_runtime_startup() { + assert!(!should_defer_agent_start(true, known_acp_runtime("devin"))); + for runtime_id in ["goose", "claude", "codex", "buzz-agent"] { + assert!( + should_defer_agent_start(true, known_acp_runtime(runtime_id)), + "{runtime_id} must retain lazy startup" + ); + } + assert!(should_defer_agent_start(true, None)); + assert!(!should_defer_agent_start(false, known_acp_runtime("goose"))); + } + + #[test] + fn devin_idle_default_preserves_all_override_layers() { + let devin = known_acp_runtime("devin"); + assert_eq!(effective_idle_timeout(None, false, devin), Some(120)); + assert_eq!(effective_idle_timeout(Some(45), false, devin), Some(45)); + assert_eq!(effective_idle_timeout(None, true, devin), None); + + for runtime_id in ["goose", "claude", "codex", "buzz-agent"] { + assert_eq!( + effective_idle_timeout(None, false, known_acp_runtime(runtime_id)), + None, + "{runtime_id} must retain the harness idle default" + ); + } + assert_eq!(effective_idle_timeout(None, false, None), None); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/path.rs b/desktop/src-tauri/src/managed_agents/runtime/path.rs index efec0c903e..5042ee0c42 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/path.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/path.rs @@ -80,13 +80,15 @@ pub(crate) fn compose_path_entries( /// Assemble the augmented `PATH` for a launched managed-agent child process. /// /// Concatenates, in priority order: -/// 1. `/.local/bin` — bundled CLI symlink -/// 2. Buzz-managed npm prefix bin dir — app-private ACP adapter shims -/// 3. Buzz-managed Node.js bin dir — app-private Node/npm runtime -/// 4. `nvm_bin` — nvm's default Node.js bin dir (if the user uses nvm) -/// 5. exe parent dir — DMG sidecars under `Contents/MacOS/` -/// 6. user's login-shell `PATH` — runtimes like node/python from other managers -/// 7. the current process `PATH` — appended on every platform when no +/// 1. exe parent dir when it contains the bundled `buzz` sidecar — this keeps +/// agents version-coupled to the app that launched them +/// 2. `/.local/bin` — user-local CLIs and the compatibility Buzz symlink +/// 3. Buzz-managed npm prefix bin dir — app-private ACP adapter shims +/// 4. Buzz-managed Node.js bin dir — app-private Node/npm runtime +/// 5. `nvm_bin` — nvm's default Node.js bin dir (if the user uses nvm) +/// 6. exe parent dir when it does not contain the bundled `buzz` sidecar +/// 7. user's login-shell `PATH` — runtimes like node/python from other managers +/// 8. the current process `PATH` — appended on every platform when no /// login-shell PATH exists, because callers use `Command::env("PATH", …)` /// which *replaces* the child's PATH. This is the steady state on Windows, /// where `login_shell_path()` always returns `None` and without it the @@ -110,9 +112,15 @@ pub(in crate::managed_agents) fn build_augmented_path( let home_added = home.is_some(); let exe_added = exe_parent.is_some(); let has_local_context = home_added || exe_added; + let prefer_exe_parent = exe_parent + .as_deref() + .is_some_and(|parent| parent.join(buzz_binary_name()).is_file()); // Build the managed/prefix entries (everything before login-shell PATH). let mut managed: Vec = Vec::new(); + if prefer_exe_parent { + managed.extend(exe_parent.iter().cloned()); + } if let Some(home) = home { managed.push(home.join(".local").join("bin")); } @@ -130,8 +138,8 @@ pub(in crate::managed_agents) fn build_augmented_path( if let Some(nvm_bin) = nvm_bin { managed.push(nvm_bin); } - if let Some(parent) = exe_parent { - managed.push(parent); + if !prefer_exe_parent { + managed.extend(exe_parent); } // Split the login-shell PATH into individual entries. @@ -156,6 +164,16 @@ pub(in crate::managed_agents) fn build_augmented_path( .map(|s| s.to_string_lossy().into_owned()) } +#[cfg(windows)] +fn buzz_binary_name() -> &'static str { + "buzz.exe" +} + +#[cfg(not(windows))] +fn buzz_binary_name() -> &'static str { + "buzz" +} + #[cfg(test)] mod tests { use super::build_augmented_path; @@ -164,22 +182,20 @@ mod tests { #[cfg(unix)] #[test] fn splits_colon_delimited_shell_path() { + let app_dir = tempfile::tempdir().expect("temp app directory"); // Regression: the shell PATH arrives as one colon-delimited string. It // must be split into segments before join_paths, or join_paths rejects // it and the whole augmented PATH collapses to None (managed agents then // lose `buzz`). let result = build_augmented_path( Some(PathBuf::from("/home/agent")), - Some(PathBuf::from("/Applications/Buzz.app/Contents/MacOS")), + Some(app_dir.path().to_path_buf()), Some("/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin".to_string()), None, ); let result = result.expect("path"); assert!(result.starts_with("/home/agent/.local/bin:"), "{result}"); - assert!( - result.contains(":/Applications/Buzz.app/Contents/MacOS:"), - "{result}" - ); + assert!(result.contains(app_dir.path().to_str().expect("utf-8 path"))); assert!( result.ends_with(":/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin"), "{result}" @@ -201,9 +217,10 @@ mod tests { #[cfg(unix)] #[test] fn nvm_bin_inserted_after_local_bin_before_exe_parent() { + let app_dir = tempfile::tempdir().expect("temp app directory"); let result = build_augmented_path( Some(PathBuf::from("/home/user")), - Some(PathBuf::from("/Applications/Buzz.app/Contents/MacOS")), + Some(app_dir.path().to_path_buf()), Some("/usr/bin:/bin".to_string()), Some(PathBuf::from("/home/user/.nvm/versions/node/v20.0.0/bin")), ); @@ -213,7 +230,7 @@ mod tests { .find("/home/user/.nvm/versions/node/v20.0.0/bin") .unwrap(); let exe = result - .find("/Applications/Buzz.app/Contents/MacOS") + .find(app_dir.path().to_str().expect("utf-8 path")) .unwrap(); assert!(local < nvm && nvm < exe, "{result}"); assert!(result.ends_with(":/usr/bin:/bin"), "{result}"); @@ -222,6 +239,7 @@ mod tests { #[cfg(unix)] #[test] fn nvm_bin_none_does_not_add_segment() { + let app_dir = tempfile::tempdir().expect("temp app directory"); let _guard = crate::managed_agents::lock_path_mutex(); let previous = std::env::var_os("PATH"); // With no shell_path the inherited process PATH is appended last, so @@ -230,7 +248,7 @@ mod tests { let result = build_augmented_path( Some(PathBuf::from("/home/user")), - Some(PathBuf::from("/usr/local/bin")), + Some(app_dir.path().to_path_buf()), None, None, ); @@ -244,7 +262,10 @@ mod tests { assert!(result.starts_with("/home/user/.local/bin:"), "{result}"); assert!(!result.contains(".nvm"), "no nvm segment: {result}"); assert!( - result.contains(":/usr/local/bin:"), + result.contains(&format!( + ":{}:", + app_dir.path().to_str().expect("utf-8 path") + )), "exe parent must precede the inherited PATH: {result}" ); assert!( @@ -253,6 +274,47 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn bundled_buzz_precedes_another_installations_local_symlink() { + let app_dir = tempfile::tempdir().expect("temp app directory"); + std::fs::File::create(app_dir.path().join("buzz")).expect("bundled buzz sidecar"); + + let result = build_augmented_path( + Some(PathBuf::from("/home/user")), + Some(app_dir.path().to_path_buf()), + Some("/usr/bin:/bin".to_string()), + None, + ) + .expect("path"); + + let bundled = result + .find(app_dir.path().to_str().expect("utf-8 path")) + .unwrap(); + let local = result.find("/home/user/.local/bin").unwrap(); + assert!(bundled < local, "{result}"); + } + + #[cfg(unix)] + #[test] + fn exe_parent_without_bundled_buzz_keeps_existing_path_order() { + let app_dir = tempfile::tempdir().expect("temp app directory"); + + let result = build_augmented_path( + Some(PathBuf::from("/home/user")), + Some(app_dir.path().to_path_buf()), + Some("/usr/bin:/bin".to_string()), + None, + ) + .expect("path"); + + let bundled = result + .find(app_dir.path().to_str().expect("utf-8 path")) + .unwrap(); + let local = result.find("/home/user/.local/bin").unwrap(); + assert!(local < bundled, "{result}"); + } + /// On Unix with no login-shell PATH, `build_augmented_path` must fall back to /// the inherited process PATH — otherwise the child gets only Buzz-managed /// dirs and loses every system binary (`curl`, `sh`, `tar`). diff --git a/desktop/src-tauri/src/managed_agents/runtime/presentation.rs b/desktop/src-tauri/src/managed_agents/runtime/presentation.rs new file mode 100644 index 0000000000..6f4d76dfeb --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/presentation.rs @@ -0,0 +1,53 @@ +use crate::managed_agents::known_acp_runtime; + +pub(super) struct RuntimePresentation { + pub(super) icon_url: Option, + pub(super) avatar_url: Option, + pub(super) superseded_avatar_urls: Vec, + pub(super) supports_buzz_model_config: Option, +} + +pub(super) fn runtime_presentation_for_summary(effective_command: &str) -> RuntimePresentation { + let runtime = known_acp_runtime(effective_command); + RuntimePresentation { + icon_url: runtime.map(|runtime| runtime.icon_url.to_string()), + avatar_url: runtime.map(|runtime| runtime.avatar_url.to_string()), + superseded_avatar_urls: runtime + .map(|runtime| { + runtime + .superseded_avatar_urls + .iter() + .map(|url| (*url).to_string()) + .collect() + }) + .unwrap_or_default(), + supports_buzz_model_config: runtime + .map(|runtime| runtime.model_env_var.is_some() || runtime.supports_acp_model_switching), + } +} + +#[cfg(test)] +mod tests { + use super::runtime_presentation_for_summary; + use crate::managed_agents::known_acp_runtime; + + #[test] + fn runtime_avatar_is_catalog_derived_without_process_state() { + let runtime = known_acp_runtime("devin").expect("Devin must remain a known runtime"); + let presentation = runtime_presentation_for_summary("devin"); + + assert_eq!(presentation.icon_url.as_deref(), Some(runtime.icon_url)); + assert_eq!(presentation.avatar_url.as_deref(), Some(runtime.avatar_url)); + assert_eq!( + presentation.superseded_avatar_urls, + runtime.superseded_avatar_urls + ); + assert_eq!(presentation.supports_buzz_model_config, Some(false)); + + let custom = runtime_presentation_for_summary("custom-agent"); + assert!(custom.icon_url.is_none()); + assert!(custom.avatar_url.is_none()); + assert!(custom.superseded_avatar_urls.is_empty()); + assert_eq!(custom.supports_buzz_model_config, None); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/process.rs b/desktop/src-tauri/src/managed_agents/runtime/process.rs index 37eb5659a4..66fe92ecbb 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/process.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/process.rs @@ -34,14 +34,15 @@ pub(crate) const KNOWN_SCRIPT_INTERPRETERS: &[&str] = &["node"]; /// Uses exact match or prefix-with-separator to avoid false positives /// (e.g. `"goose"` must not match `"mongoose"`). pub(super) fn name_matches_known_binary(name: &str) -> bool { - KNOWN_AGENT_BINARIES.iter().any(|&binary| { - name == binary || { - name.starts_with(binary) && { - let rest = &name[binary.len()..]; - rest.starts_with('-') || rest.starts_with('_') || rest.starts_with('.') + known_acp_runtime(name).is_some() + || KNOWN_AGENT_BINARIES.iter().any(|&binary| { + name == binary || { + name.starts_with(binary) && { + let rest = &name[binary.len()..]; + rest.starts_with('-') || rest.starts_with('_') || rest.starts_with('.') + } } - } - }) + }) } /// Check if a process name is a known script interpreter that may be hosting @@ -243,6 +244,10 @@ fn signal_process_group_or_leader(pid: u32, signal: i32, action: &str) -> Result #[cfg(unix)] pub(crate) fn terminate_process(pid: u32) -> Result<(), String> { + // Reap independently-grouped ACP descendants while their ownership can + // still be proven through the live harness ancestry. + sweep::terminate_owned_descendant_groups(pid); + // Try graceful shutdown first (SIGTERM to the group). signal_process_group_or_leader(pid, libc::SIGTERM, "terminate")?; diff --git a/desktop/src-tauri/src/managed_agents/runtime/process_tree_tests.rs b/desktop/src-tauri/src/managed_agents/runtime/process_tree_tests.rs new file mode 100644 index 0000000000..9f0f4b998b --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/process_tree_tests.rs @@ -0,0 +1,83 @@ +//! Process-tree regression tests that launch subprocesses. + +use std::os::unix::process::CommandExt; +use std::process::Command; + +/// Regression coverage for managed-agent restart teardown. The helper test +/// process acts as `buzz-acp` and starts a child in its own process group, +/// mirroring `AcpClient::spawn`. Terminating the helper must reap both groups. +#[test] +fn terminate_process_reaps_independent_descendant_group() { + let _path_guard = crate::managed_agents::lock_path_mutex(); + let marker_path = std::env::temp_dir().join(format!( + "buzz-process-tree-{}-{}.pid", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system clock after Unix epoch") + .as_nanos() + )); + let _ = std::fs::remove_file(&marker_path); + + let mut helper = { + let mut command = Command::new(std::env::current_exe().expect("resolve test executable")); + command + .args([ + "--exact", + "managed_agents::runtime::process_tree_tests::process_tree_descendant_helper", + "--nocapture", + ]) + .env("BUZZ_PROCESS_TREE_TEST_MARKER", &marker_path) + .process_group(0); + command.spawn().expect("spawn process-tree helper") + }; + let helper_pid = helper.id(); + + let child_pid = (0..100) + .find_map(|_| { + let value = std::fs::read_to_string(&marker_path).ok(); + if value.is_none() { + std::thread::sleep(std::time::Duration::from_millis(20)); + } + value.and_then(|pid| pid.trim().parse::().ok()) + }) + .expect("helper should report its independent child PID"); + + assert_eq!( + unsafe { libc::getpgid(child_pid as i32) }, + child_pid as i32, + "helper child should lead an independent process group" + ); + + super::terminate_process(helper_pid).expect("terminate complete helper process tree"); + let _ = helper.wait(); + + for _ in 0..50 { + if !super::process_is_running(child_pid) { + break; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + assert!( + !super::process_is_running(child_pid), + "independently-grouped ACP child must not survive harness teardown" + ); + let _ = std::fs::remove_file(marker_path); +} + +/// Subprocess-only half of [`terminate_process_reaps_independent_descendant_group`]. +#[test] +fn process_tree_descendant_helper() { + let Some(marker_path) = std::env::var_os("BUZZ_PROCESS_TREE_TEST_MARKER") else { + return; + }; + let mut child = { + let mut command = Command::new("sh"); + command + .args(["-c", "while :; do sleep 60; done"]) + .process_group(0); + command.spawn().expect("spawn independent helper child") + }; + std::fs::write(marker_path, child.id().to_string()).expect("write helper child PID"); + let _ = child.wait(); +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/sweep.rs b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs index 3060ff6593..87622fdf73 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/sweep.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs @@ -186,6 +186,82 @@ pub(super) fn ppid_of_linux(pid: u32) -> Option { proc_stat_ppid_pgid_linux(pid).map(|(ppid, _)| ppid) } +/// Snapshot every live, same-user descendant of `root_pid` while the root is +/// still running. +/// +/// Managed ACP runtimes may put their own subprocesses in independent process +/// groups. Signalling only the harness group therefore cannot guarantee that +/// the complete runtime tree exits. Callers use this snapshot immediately, +/// before terminating the root, so the ancestor relationship is still +/// available and unrelated same-user processes remain out of scope. +#[cfg(target_os = "macos")] +pub(super) fn collect_live_descendant_pids(root_pid: u32) -> Vec { + let my_uid = unsafe { libc::getuid() }; + collect_all_pids() + .into_iter() + .filter_map(|pid| { + if pid <= 0 || pid as u32 == root_pid { + return None; + } + let upid = pid as u32; + let mut info = std::mem::MaybeUninit::::zeroed(); + let ret = unsafe { + super::proc_pidinfo( + pid, + super::PROC_PIDTBSDINFO, + 0, + info.as_mut_ptr() as *mut libc::c_void, + std::mem::size_of::() as libc::c_int, + ) + }; + if ret <= 0 { + return None; + } + let info = unsafe { info.assume_init() }; + (info.pbi_uid == my_uid && walk_has_tracked_ancestor(upid, &[root_pid], ppid_of_macos)) + .then_some(upid) + }) + .collect() +} + +/// Terminate process groups led by a live, same-user descendant of `root_pid`. +/// +/// This must run while the root is still alive so the bounded ancestry walk +/// can prove ownership before any signal is sent. +#[cfg(unix)] +pub(super) fn terminate_owned_descendant_groups(root_pid: u32) { + let descendant_pids = collect_live_descendant_pids(root_pid) + .into_iter() + .map(|pid| pid as i32) + .collect::>(); + if !descendant_pids.is_empty() { + super::resolve_pgids_and_kill(&descendant_pids); + } +} + +/// Linux variant of [`collect_live_descendant_pids`]. +#[cfg(all(unix, not(target_os = "macos")))] +pub(super) fn collect_live_descendant_pids(root_pid: u32) -> Vec { + let my_uid = unsafe { libc::getuid() }; + let Ok(entries) = std::fs::read_dir("/proc") else { + return Vec::new(); + }; + entries + .flatten() + .filter_map(|entry| { + let pid = entry.file_name().to_str()?.parse::().ok()?; + if pid == 0 || pid == root_pid { + return None; + } + use std::os::unix::fs::MetadataExt; + if entry.metadata().ok()?.uid() != my_uid { + return None; + } + walk_has_tracked_ancestor(pid, &[root_pid], ppid_of_linux).then_some(pid) + }) + .collect() +} + /// True if `pid` is a live descendant of any tracked harness in `skip_pids`. /// /// Three complementary checks: @@ -433,7 +509,7 @@ fn collect_process_snapshots(harness_name: &str) -> Vec { snapshots } -// ── expected_harness_exe_path ───────────────────────────────────────────── +// ── expected_harness_exe_paths ──────────────────────────────────────────── /// Derive the expected path of the `buzz-acp` harness binary next to the /// current executable. Returns `None` if `current_exe()` fails or has no @@ -460,12 +536,29 @@ fn collect_process_snapshots(harness_name: &str) -> Vec { /// the same app (different bundle path, e.g. a prior DMG) will not match /// this path — that class is handled by `sweep_system_agent_processes`, which /// scopes by `BUZZ_MANAGED_AGENT` instance ID rather than exe path. -pub fn expected_harness_exe_path() -> Option { - let exe = std::env::current_exe().ok()?; - let dir = exe.parent()?; - let raw = dir.join("buzz-acp"); - // Canonicalize if possible; fall back to the raw path on failure. - Some(std::fs::canonicalize(&raw).unwrap_or(raw)) +pub fn expected_harness_exe_paths() -> Vec { + let mut paths = Vec::new(); + if let Some(raw) = std::env::current_exe() + .ok() + .and_then(|exe| exe.parent().map(|dir| dir.join("buzz-acp"))) + { + paths.push(std::fs::canonicalize(&raw).unwrap_or(raw)); + } + + // `tauri dev` builds the desktop crate under `desktop/src-tauri/target` + // while Buzz's sidecar build lives under the repository-level `target`. + // The launch resolver intentionally uses that workspace sidecar, so the + // boot sweeper must recognize the same exact path. Release builds remain + // scoped to the sibling binary inside the app bundle. + #[cfg(debug_assertions)] + if let Some(raw) = crate::managed_agents::discovery::resolve_workspace_command("buzz-acp") { + let path = std::fs::canonicalize(&raw).unwrap_or(raw); + if !paths.contains(&path) { + paths.push(path); + } + } + + paths } /// The basename of the harness binary — used for the cheap name pre-filter in @@ -496,19 +589,28 @@ const HARNESS_BINARY_NAME: &str = "buzz-acp"; /// when `resolve_pgids_and_kill` signals the PGID. #[cfg(unix)] pub(crate) fn sweep_untracked_bundle_harnesses(skip_pids: &[u32]) { - let Some(harness_exe) = expected_harness_exe_path() else { + let harness_exes = expected_harness_exe_paths(); + if harness_exes.is_empty() { return; - }; + } let snapshots = collect_process_snapshots(HARNESS_BINARY_NAME); - let to_kill = select_untracked_bundle_harnesses(&snapshots, &harness_exe, skip_pids); + let mut to_kill = std::collections::BTreeSet::new(); + for harness_exe in &harness_exes { + to_kill.extend(select_untracked_bundle_harnesses( + &snapshots, + harness_exe, + skip_pids, + )); + } if to_kill.is_empty() { return; } + let to_kill = to_kill.into_iter().collect::>(); eprintln!( - "buzz-desktop: sweep_untracked_bundle_harnesses: reaping {} stale harness process(es) {:?} (exe: {})", + "buzz-desktop: sweep_untracked_bundle_harnesses: reaping {} stale harness process(es) {:?} (expected exe paths: {:?})", to_kill.len(), to_kill, - harness_exe.display(), + harness_exes, ); // Small snapshot→kill PID-reuse window: a PID in `to_kill` could be // recycled between the snapshot and the kill call. This matches the diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 8deb0c4da9..2ee4cd27df 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -1,5 +1,7 @@ use crate::managed_agents::known_acp_runtime; +mod devin; + // ── desktop binary name tests ─────────────────────────────────────────── #[test] @@ -594,45 +596,6 @@ fn name_matches_interpreter_rejects_node_prefix() { assert!(!super::name_matches_interpreter("node-gyp")); } -#[test] -fn claude_spawn_uses_the_probed_cli_executable() { - let _guard = crate::managed_agents::lock_path_mutex(); - let temp = tempfile::tempdir().expect("temp dir"); - let cli = temp - .path() - .join(format!("claude{}", std::env::consts::EXE_SUFFIX)); - std::fs::write(&cli, "").expect("write fake cli"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&cli, std::fs::Permissions::from_mode(0o755)) - .expect("make fake cli executable"); - } - let original_path = std::env::var_os("PATH"); - std::env::set_var("PATH", temp.path()); - - let mut command = std::process::Command::new("buzz-acp"); - super::configure_runtime_cli(&mut command, super::known_acp_runtime("claude-agent-acp")); - - if let Some(path) = original_path { - std::env::set_var("PATH", path); - } else { - std::env::remove_var("PATH"); - } - assert!(command - .get_envs() - .any(|(key, value)| { key == "CLAUDE_CODE_EXECUTABLE" && value == Some(cli.as_os_str()) })); -} - -#[test] -fn codex_spawn_does_not_set_a_claude_executable() { - let mut command = std::process::Command::new("buzz-acp"); - super::configure_runtime_cli(&mut command, super::known_acp_runtime("codex-acp")); - assert!(!command - .get_envs() - .any(|(key, _)| key == "CLAUDE_CODE_EXECUTABLE")); -} - /// On Windows, `.cmd` and `.bat` batch shims must NOT be assigned to /// `CLAUDE_CODE_EXECUTABLE` — `CreateProcess` cannot exec them directly and /// returns EINVAL (issue #2397). The adapter must fall back to its own PATH @@ -703,6 +666,8 @@ fn grandchild_inherits_pgid_of_process_group_leader() { use std::os::unix::process::CommandExt; use std::process::Command; + let _path_guard = crate::managed_agents::lock_path_mutex(); + // Spawn a "harness" process in its own process group (mirrors // `command.process_group(0)` in the real spawn path). The harness // spawns an intermediate child which in turn spawns a grandchild. @@ -812,6 +777,8 @@ fn own_group_grandchild_detected_by_ancestor_walk() { use std::os::unix::process::CommandExt; use std::process::Command; + let _path_guard = crate::managed_agents::lock_path_mutex(); + // The test process is the "harness". Spawn an intermediate with its own // process group (mirrors the node shim). It backgrounds a grandchild // (sleep 30) and prints the grandchild PID so we can inspect it. diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs b/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs new file mode 100644 index 0000000000..18c920de88 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs @@ -0,0 +1,46 @@ +use crate::managed_agents::known_acp_runtime; + +#[test] +fn uses_native_acp_without_mcp_hooks() { + let runtime = known_acp_runtime("/usr/local/bin/devin").expect("should resolve"); + assert_eq!(runtime.id, "devin"); + assert_eq!(runtime.default_args, &["acp"]); + assert!(!runtime.defer_agent_start_until_work); + assert_eq!(runtime.default_idle_timeout_secs, Some(120)); + assert!(runtime.default_env.is_empty()); + assert_eq!( + runtime.enforced_env, + &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ] + ); + assert_eq!(runtime.scrub_env_vars, &["WINDSURF_API_KEY"]); + assert!(!runtime.mcp_hooks); + assert_eq!(runtime.mcp_command, None); +} + +#[test] +fn permission_default_does_not_change_existing_runtimes() { + assert_eq!( + known_acp_runtime("goose") + .expect("Goose runtime") + .default_env, + &[("GOOSE_MODE", "auto")] + ); + for command in ["claude-agent-acp", "codex-acp", "buzz-agent"] { + let runtime = known_acp_runtime(command).expect("existing runtime"); + assert!(runtime.default_env.is_empty()); + assert!(runtime.enforced_env.is_empty()); + assert!(runtime.scrub_env_vars.is_empty()); + assert!(runtime.defer_agent_start_until_work); + assert_eq!(runtime.default_idle_timeout_secs, None); + } +} + +#[test] +fn process_name_resolves_from_the_runtime_catalog() { + assert!(super::super::name_matches_known_binary("devin")); +} diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index b9c6c7e6cd..0118f6af84 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -11,7 +11,10 @@ use crate::app_state::keyring_service; use crate::managed_agents::{ ManagedAgentRecord, ManagedAgentRuntimeKey, ManagedAgentRuntimeReceipt, }; + +mod normalization; use crate::secret_store::{KeyringProbe, SecretStore}; +use normalization::normalize_runtime_avatars; /// Keyring key name for an agent's nsec, namespaced from the human identity /// key (`"identity"`) which shares the service. @@ -181,7 +184,7 @@ fn load_agent_store(app: &AppHandle) -> Result, String> let content = fs::read_to_string(&path) .map_err(|error| format!("failed to read agent store: {error}"))?; - serde_json::from_str(&content).map_err(|error| { + let mut records: Vec = serde_json::from_str(&content).map_err(|error| { // Fail loudly and preserve the evidence: a later in-app save rewrites // this file wholesale, which would silently destroy a malformed hand // edit. Best-effort file-authoring contract (see managed_agents:: @@ -190,7 +193,11 @@ fn load_agent_store(app: &AppHandle) -> Result, String> // swallowed into an empty store. backup_invalid_store(&path); format!("failed to parse agent store (preserved as .invalid): {error}") - }) + })?; + + normalize_runtime_avatars(&mut records); + + Ok(records) } /// Load the keyed agent *instances*. Key-less definitions (former personas, @@ -590,11 +597,34 @@ fn maybe_rotate_log(path: &Path) { 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)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.mode(0o600); + } + + let file = options .open(path) - .map_err(|error| format!("failed to open log file {}: {error}", path.display())) + .map_err(|error| format!("failed to open log file {}: {error}", path.display()))?; + + // `mode()` applies only when the file is created. Tighten logs written by + // older builds as soon as they are reopened so upgrades do not leave agent + // activity readable by other local accounts. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + file.set_permissions(fs::Permissions::from_mode(0o600)) + .map_err(|error| { + format!( + "failed to secure log file permissions for {}: {error}", + path.display() + ) + })?; + } + + Ok(file) } pub(crate) fn append_log_marker(path: &Path, message: &str) -> Result<(), String> { @@ -789,15 +819,17 @@ pub fn meaningful_agent_error_from_log(path: &Path) -> Option { mod tests { use std::cell::RefCell; use std::collections::HashMap; - use std::io::Write as _; - - use tempfile::NamedTempFile; use super::{ agent_keyring_name, hydrate_keys_with, migrate_inline_key, persist_agent_keys_with, KeyMigration, KeyStore, KeyringProbe, ManagedAgentRecord, }; + mod avatar; + mod log_errors; + #[cfg(unix)] + mod log_permissions; + /// In-memory [`KeyStore`] for testing the migrate decision without the OS /// keyring. `reachable=false` simulates a backend outage; `fail_verify` /// simulates a write whose read-back does not confirm. @@ -1095,12 +1127,6 @@ mod tests { assert!(records[1].private_key_nsec.is_empty()); } - fn write_log(content: &str) -> NamedTempFile { - let mut file = NamedTempFile::new().expect("temp log"); - file.write_all(content.as_bytes()).expect("write log"); - file - } - /// The keyringless fallback write must land `0o600` from the write itself — /// not a post-write `chmod` — so a crash in the umask window can never leave /// plaintext agent nsecs world-readable (Wes storage.rs:239, SECURITY.md:90). @@ -1127,58 +1153,6 @@ mod tests { ); } - #[test] - fn meaningful_agent_error_from_log_promotes_wrapped_llm_auth() { - let file = write_log( - "noise\nAgent reported error (code -32001): llm auth: 401 unauthorized: ...\n", - ); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert!(result.message.contains("llm auth")); - assert_eq!(result.code, Some(-32001)); - } - - #[test] - fn meaningful_agent_error_from_log_promotes_unwrapped_llm_auth() { - let file = write_log("noise\nllm auth: denied\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!(result.message, "Agent reported error: llm auth: denied"); - assert_eq!(result.code, Some(-32001)); - } - - #[test] - fn meaningful_agent_error_from_log_promotes_bare_model_not_found() { - let file = write_log("noise\nllm model not found: (some-model) 404\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!( - result.message, - "Agent reported error: llm model not found: (some-model) 404" - ); - assert_eq!(result.code, Some(-32002)); - } - - #[test] - fn meaningful_agent_error_from_log_promotes_legacy_format() { - let file = write_log("noise\nAgent reported error: llm: 500 internal\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!(result.message, "Agent reported error: llm: 500 internal"); - assert_eq!(result.code, None); - } - - #[test] - fn meaningful_agent_error_from_log_does_not_promote_midline_auth_text() { - let file = write_log("noise before llm auth: denied\n"); - assert!(super::meaningful_agent_error_from_log(file.path()).is_none()); - } - - #[test] - fn strips_ansi_from_typical_tracing_line() { - let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2mbuzz_acp\x1b[0m\x1b[2m:\x1b[0m starting"; - assert_eq!( - strip_ansi_escapes::strip_str(input), - "2026-05-27T15:16:32 INFO buzz_acp: starting" - ); - } - // ── keyring-dev-migration tests ──────────────────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/storage/normalization.rs b/desktop/src-tauri/src/managed_agents/storage/normalization.rs new file mode 100644 index 0000000000..741276c7ec --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/normalization.rs @@ -0,0 +1,12 @@ +use crate::managed_agents::{normalize_managed_agent_avatar, ManagedAgentRecord}; + +pub(super) fn normalize_runtime_avatars(records: &mut [ManagedAgentRecord]) { + for record in records { + let command = record + .runtime + .as_deref() + .filter(|value| !value.trim().is_empty()) + .unwrap_or(&record.agent_command); + record.avatar_url = normalize_managed_agent_avatar(command, record.avatar_url.take()); + } +} diff --git a/desktop/src-tauri/src/managed_agents/storage/tests/avatar.rs b/desktop/src-tauri/src/managed_agents/storage/tests/avatar.rs new file mode 100644 index 0000000000..a7179cd902 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/tests/avatar.rs @@ -0,0 +1,28 @@ +use super::super::normalize_runtime_avatars; +use super::record_with_key; + +#[test] +fn loaded_devin_records_replace_only_the_superseded_default_avatar() { + let mut devin = record_with_key(""); + devin.runtime = Some("devin".to_string()); + devin.avatar_url = Some( + "https://mintcdn.com/cognitionai/Hhrl_8XUBqA4VQ6v/logo/favicon.svg?fit=max&auto=format&n=Hhrl_8XUBqA4VQ6v&q=85&s=ab641f30c01bf5374b90b62209db569e" + .to_string(), + ); + + let mut custom = record_with_key(""); + custom.runtime = Some("devin".to_string()); + custom.avatar_url = Some("https://example.test/custom.png".to_string()); + + normalize_runtime_avatars(std::slice::from_mut(&mut devin)); + normalize_runtime_avatars(std::slice::from_mut(&mut custom)); + + assert_eq!( + devin.avatar_url, + crate::managed_agents::managed_agent_avatar_url("devin") + ); + assert_eq!( + custom.avatar_url.as_deref(), + Some("https://example.test/custom.png") + ); +} diff --git a/desktop/src-tauri/src/managed_agents/storage/tests/log_errors.rs b/desktop/src-tauri/src/managed_agents/storage/tests/log_errors.rs new file mode 100644 index 0000000000..6bb1b2e253 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/tests/log_errors.rs @@ -0,0 +1,60 @@ +use std::io::Write as _; + +use tempfile::NamedTempFile; + +fn write_log(content: &str) -> NamedTempFile { + let mut file = NamedTempFile::new().expect("temp log"); + file.write_all(content.as_bytes()).expect("write log"); + file +} + +#[test] +fn meaningful_agent_error_from_log_promotes_wrapped_llm_auth() { + let file = + write_log("noise\nAgent reported error (code -32001): llm auth: 401 unauthorized: ...\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert!(result.message.contains("llm auth")); + assert_eq!(result.code, Some(-32001)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_unwrapped_llm_auth() { + let file = write_log("noise\nllm auth: denied\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!(result.message, "Agent reported error: llm auth: denied"); + assert_eq!(result.code, Some(-32001)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_bare_model_not_found() { + let file = write_log("noise\nllm model not found: (some-model) 404\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!( + result.message, + "Agent reported error: llm model not found: (some-model) 404" + ); + assert_eq!(result.code, Some(-32002)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_legacy_format() { + let file = write_log("noise\nAgent reported error: llm: 500 internal\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!(result.message, "Agent reported error: llm: 500 internal"); + assert_eq!(result.code, None); +} + +#[test] +fn meaningful_agent_error_from_log_does_not_promote_midline_auth_text() { + let file = write_log("noise before llm auth: denied\n"); + assert!(super::super::meaningful_agent_error_from_log(file.path()).is_none()); +} + +#[test] +fn strips_ansi_from_typical_tracing_line() { + let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2mbuzz_acp\x1b[0m\x1b[2m:\x1b[0m starting"; + assert_eq!( + strip_ansi_escapes::strip_str(input), + "2026-05-27T15:16:32 INFO buzz_acp: starting" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/storage/tests/log_permissions.rs b/desktop/src-tauri/src/managed_agents/storage/tests/log_permissions.rs new file mode 100644 index 0000000000..5a2e0fee9c --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/tests/log_permissions.rs @@ -0,0 +1,29 @@ +use std::os::unix::fs::PermissionsExt as _; + +#[test] +fn agent_logs_are_created_owner_only_and_tighten_legacy_permissions() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("agent.log"); + + drop(super::super::open_log_file(&path).expect("create log")); + assert_eq!( + std::fs::metadata(&path) + .expect("created log metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)) + .expect("set legacy permissions"); + drop(super::super::open_log_file(&path).expect("reopen legacy log")); + assert_eq!( + std::fs::metadata(&path) + .expect("reopened log metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); +} diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index dcb8095a7c..c21d13ff3c 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -489,7 +489,19 @@ pub struct ManagedAgentSummary { pub max_turn_duration_seconds: Option, pub parallelism: u32, pub system_prompt: Option, + /// User/persona avatar snapshot persisted on the agent record. pub avatar_url: Option, + /// App-local presentation mark derived from the effective runtime catalog. + pub runtime_icon_url: Option, + /// Presentation-only fallback derived from the effective runtime catalog. + /// This is never persisted as a user-selected avatar. + pub runtime_avatar_url: Option, + /// Superseded catalog defaults that the frontend must not prefer over the + /// current runtime avatar while a stopped agent's relay profile is stale. + pub runtime_superseded_avatar_urls: Vec, + /// Whether Buzz can apply its configured model to this runtime. `None` + /// preserves the existing display for unknown/custom runtimes. + pub supports_buzz_model_config: Option, pub model: Option, #[serde(skip_serializing_if = "Option::is_none")] pub model_source: Option, @@ -589,7 +601,7 @@ pub enum AuthStatus { #[derive(Debug, Clone, Serialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum HarnessSource { - /// Compiled into the app — one of the four first-class runtimes. + /// Compiled into the app — one of the first-class runtimes. Builtin, /// Static preset entry with bundled logo, PATH-probed, not editable/deletable. Preset, @@ -601,7 +613,14 @@ pub enum HarnessSource { pub struct AcpRuntimeCatalogEntry { pub id: String, pub label: String, + pub display_label: String, + pub sort_priority: u16, + pub onboarding_visible: bool, + pub icon_url: String, + pub icon_scale: f32, pub avatar_url: String, + pub superseded_avatar_urls: Vec, + pub supports_buzz_model_config: bool, pub availability: AcpAvailabilityStatus, pub command: Option, pub binary_path: Option, diff --git a/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs b/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs index 696a055d20..7554fc5564 100644 --- a/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs +++ b/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs @@ -56,3 +56,34 @@ test("resolveAgentCardModelLabel — non-inherited agent with a blank resolved m }); assert.equal(label, "Default model (claude-sonnet)"); }); + +test("resolveAgentCardModelLabel — runtime-owned model choice ignores stale Buzz model state", () => { + const label = resolveAgentCardModelLabel({ + agent: { modelSource: "definition", model: "stale-model" }, + personaModel: "stale-persona-model", + defaultModel: "claude-sonnet", + supportsBuzzModelConfig: false, + }); + assert.equal(label, "Runtime default"); +}); + +test("resolveAgentCardModelLabel — supported and unknown capabilities preserve existing labels", () => { + assert.equal( + resolveAgentCardModelLabel({ + agent: { modelSource: "definition", model: "gpt-5" }, + personaModel: null, + defaultModel: "claude-sonnet", + supportsBuzzModelConfig: true, + }), + "gpt-5", + ); + assert.equal( + resolveAgentCardModelLabel({ + agent: undefined, + personaModel: null, + defaultModel: "claude-sonnet", + supportsBuzzModelConfig: null, + }), + "Default model (claude-sonnet)", + ); +}); diff --git a/desktop/src/features/agents/lib/agentCardModelLabel.ts b/desktop/src/features/agents/lib/agentCardModelLabel.ts index 4ec06f10c5..c15a606d6a 100644 --- a/desktop/src/features/agents/lib/agentCardModelLabel.ts +++ b/desktop/src/features/agents/lib/agentCardModelLabel.ts @@ -22,7 +22,11 @@ export function resolveAgentCardModelLabel(input: { agent: Pick | undefined; personaModel: string | null | undefined; defaultModel: string; + supportsBuzzModelConfig?: boolean | null; }): string { + if (input.supportsBuzzModelConfig === false) { + return "Runtime default"; + } if (input.agent) { const isInherited = !input.agent.modelSource || input.agent.modelSource === "global"; diff --git a/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs b/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs index 597b1b9323..46b03ec6bb 100644 --- a/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs +++ b/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs @@ -47,6 +47,15 @@ test("generic harness exit message → passthrough", () => { }); }); +test("Devin ACP startup failure remains distinct and actionable", () => { + const startupFailure = + "failed to start Devin ACP: process exited before initialization"; + assert.deepEqual(friendlyAgentLastError(startupFailure), { + severity: "generic", + copy: startupFailure, + }); +}); + test("trims whitespace before matching", () => { const result = friendlyAgentLastError( " Agent reported error: llm auth: nope\n", diff --git a/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.test.mjs b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.test.mjs new file mode 100644 index 0000000000..124f0b18a3 --- /dev/null +++ b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.test.mjs @@ -0,0 +1,67 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resolveAgentCardAvatarUrl } from "./resolveAgentCardAvatarUrl.ts"; + +test("stopped legacy agents use the catalog-projected runtime avatar", () => { + assert.equal( + resolveAgentCardAvatarUrl([null, null, "app-avatar://devin"]), + "app-avatar://devin", + ); +}); + +test("stored and custom profile avatars outrank the runtime fallback", () => { + assert.equal( + resolveAgentCardAvatarUrl([ + " https://relay.example/custom.png ", + "https://stored.example/custom.png", + "app-avatar://devin", + ]), + "https://relay.example/custom.png", + ); + assert.equal( + resolveAgentCardAvatarUrl([ + " ", + "https://stored.example/custom.png", + "app-avatar://devin", + ]), + "https://stored.example/custom.png", + ); +}); + +test("superseded relay defaults fall through to the current runtime avatar", () => { + const legacy = "https://runtime.example/old-default.svg"; + assert.equal( + resolveAgentCardAvatarUrl( + [null, legacy, "data:image/svg+xml,current"], + [legacy], + ), + "data:image/svg+xml,current", + ); +}); + +test("persona callers preserve custom instance avatars before the runtime fallback", () => { + assert.equal( + resolveAgentCardAvatarUrl([ + null, + "https://stored.example/custom.png", + "/runtime-icons/current.svg", + ]), + "https://stored.example/custom.png", + ); +}); + +test("persona-only cards use the catalog runtime icon before an instance exists", () => { + assert.equal( + resolveAgentCardAvatarUrl([ + null, + "/runtime-icons/devin.svg", + "data:image/svg+xml,runtime-avatar", + ]), + "/runtime-icons/devin.svg", + ); +}); + +test("missing custom and runtime avatars preserve the initials fallback", () => { + assert.equal(resolveAgentCardAvatarUrl([undefined, "", null]), null); +}); diff --git a/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.ts b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.ts new file mode 100644 index 0000000000..6b31195a66 --- /dev/null +++ b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.ts @@ -0,0 +1,21 @@ +/** + * Select the first non-empty avatar source for an agent-management card. + * + * Callers provide sources in presentation-priority order. The final source is + * normally the runtime-catalog fallback projected by the backend, so stopped + * legacy agents still render their runtime logo without overwriting stored or + * relay-published custom avatars. + */ +export function resolveAgentCardAvatarUrl( + candidates: Array, + supersededRuntimeAvatarUrls: readonly string[] = [], +): string | null { + const superseded = new Set( + supersededRuntimeAvatarUrls.map((candidate) => candidate.trim()), + ); + for (const candidate of candidates) { + const trimmed = candidate?.trim(); + if (trimmed && !superseded.has(trimmed)) return trimmed; + } + return null; +} diff --git a/desktop/src/features/agents/ui/AgentConfigPanel.tsx b/desktop/src/features/agents/ui/AgentConfigPanel.tsx index 3236b4d808..dcf5bf5569 100644 --- a/desktop/src/features/agents/ui/AgentConfigPanel.tsx +++ b/desktop/src/features/agents/ui/AgentConfigPanel.tsx @@ -168,6 +168,7 @@ function NormalizedRow({ field, isPreSpawn, configFilePath, + runtimeControlsModel = false, variant = "compact", }: { fieldKey: keyof NormalizedConfig; @@ -175,26 +176,31 @@ function NormalizedRow({ field: NormalizedField; isPreSpawn: boolean; configFilePath: string | null; + runtimeControlsModel?: boolean; variant?: RowVariant; }) { const Icon = NORMALIZED_ICONS[fieldKey]; // ACP-sourced origins only become meaningful post-spawn const isAcpOnly = field.origin === "acpNativeRead" || field.origin === "acpConfigOption"; - const rawDisplayValue = - isPreSpawn && isAcpOnly + const rawDisplayValue = runtimeControlsModel + ? "Runtime default" + : isPreSpawn && isAcpOnly ? "Available after agent starts" : (field.value ?? "—"); const displayValue = fieldKey === "provider" ? providerDisplayLabel(rawDisplayValue) : rawDisplayValue; - const provenance = field.value - ? provenanceSentence(field.origin, field.writeVia, configFilePath) - : null; - const locked = isReadOnlyField(field); + const provenance = runtimeControlsModel + ? "Controlled by runtime" + : field.value + ? provenanceSentence(field.origin, field.writeVia, configFilePath) + : null; + const locked = runtimeControlsModel || isReadOnlyField(field); const isCopyable = variant === "profile" && + !runtimeControlsModel && shouldOfferCopy({ fieldKey, origin: field.origin, @@ -216,7 +222,7 @@ function NormalizedRow({ )} {displayValue} {!(isPreSpawn && isAcpOnly) && field.overriddenValue ? ( @@ -417,6 +423,9 @@ export function AgentConfigPanel({ field={field} isPreSpawn={isPreSpawn} configFilePath={configFilePath} + runtimeControlsModel={ + key === "model" && data.supportsBuzzModelConfig === false + } variant={advancedMode === "flat" ? "profile" : "compact"} /> )) diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index 8e1c47c615..b3039b4f4d 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -142,6 +142,7 @@ export function AgentsView() {
{groups.map((group) => { const profileAgent = pickProfileAgent(group.agents); + const personaRuntime = runtimes.find( + (runtime) => runtime.id === group.persona.runtime, + ); return ( ; onOpenAgentProfile: ( @@ -270,12 +283,28 @@ function AgentPersonaCard({ agent, personaModel: persona.model, defaultModel, + supportsBuzzModelConfig: + runtime?.supportsBuzzModelConfig ?? + agent?.supportsBuzzModelConfig ?? + null, }); const isActive = agent ? isManagedAgentActive(agent) : false; const profileQuery = useUserProfileQuery(agent?.pubkey); const avatarUrl = agent - ? firstAvatarUrl(persona.avatarUrl, profileQuery.data?.avatarUrl) - : persona.avatarUrl; + ? resolveAgentCardAvatarUrl( + [ + persona.avatarUrl, + agent.avatarUrl, + profileQuery.data?.avatarUrl, + runtime?.iconUrl ?? agent.runtimeIconUrl, + runtime?.avatarUrl ?? agent.runtimeAvatarUrl, + ], + runtime?.supersededAvatarUrls ?? agent.runtimeSupersededAvatarUrls, + ) + : resolveAgentCardAvatarUrl( + [persona.avatarUrl, runtime?.iconUrl, runtime?.avatarUrl], + runtime?.supersededAvatarUrls ?? [], + ); const friendlyError = agent ? friendlyAgentLastError(agent.lastError, agent.lastErrorCode)?.copy : null; @@ -368,6 +397,10 @@ function StandaloneAgentCard({ )?.copy; const isActive = isManagedAgentActive(agent); const opensRuntimeTab = Boolean(friendlyError && !isActive); + const avatarUrl = resolveAgentCardAvatarUrl( + [agent.avatarUrl, profileQuery.data?.avatarUrl, agent.runtimeAvatarUrl], + agent.runtimeSupersededAvatarUrls, + ); return ( onStartAgent(agent.pubkey)} /> } - avatarUrl={profileQuery.data?.avatarUrl} + avatarUrl={avatarUrl} dataTestId={`managed-agent-${agent.pubkey}`} label={title} modelLabel={resolveAgentCardModelLabel({ agent, personaModel: null, defaultModel, + supportsBuzzModelConfig: agent.supportsBuzzModelConfig, })} onClick={() => { onOpenAgentProfile( @@ -419,16 +453,6 @@ function StandaloneAgentCard({ ); } -function firstAvatarUrl( - ...candidates: Array -): string | null { - for (const candidate of candidates) { - const trimmed = candidate?.trim(); - if (trimmed) return trimmed; - } - return null; -} - function NewAgentCard({ canChooseCatalog, isPersonasPending, diff --git a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx index 2debea409b..add4315b08 100644 --- a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx +++ b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx @@ -3,18 +3,6 @@ import { TerminalSquare } from "lucide-react"; import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; import { cn } from "@/shared/lib/cn"; -import { useTheme } from "@/shared/theme/ThemeProvider"; -import { BuzzMark } from "@/shared/ui/buzz-logo/BuzzMark"; -import chatgptLogoUrl from "../assets/harness-logos/chatgpt.png?inline"; -import claudeLogoUrl from "../assets/harness-logos/claude.png?inline"; -import gooseLogoUrl from "../assets/harness-logos/goose.png?inline"; - -// Bundled logos for compiled-in runtimes (inline base64, no network fetch). -const RUNTIME_LOGOS: Record = { - claude: claudeLogoUrl, - codex: chatgptLogoUrl, - goose: gooseLogoUrl, -}; // Public-path logos for bundled presets. Served from /harness-logos/ at runtime. // Keys match the preset `id` values emitted by the backend PRESET_HARNESSES. @@ -28,19 +16,22 @@ export const PRESET_LOGOS: Record = { openclaw: "/harness-logos/openclaw.svg", }; -function isBuzzRuntime(runtime: AcpRuntimeCatalogEntry): boolean { - return runtime.id.trim().toLowerCase() === "buzz-agent"; -} - export function getRuntimeDisplayLabel( runtime: AcpRuntimeCatalogEntry, ): string { - return isBuzzRuntime(runtime) ? "Buzz" : runtime.label; + return runtime.displayLabel; } function getRuntimeLogoUrl(runtime: AcpRuntimeCatalogEntry): string | null { const id = runtime.id.trim().toLowerCase(); - return RUNTIME_LOGOS[id] ?? PRESET_LOGOS[id] ?? null; + if (runtime.source === "builtin") { + return runtime.iconUrl || null; + } + if (runtime.source === "preset") { + return PRESET_LOGOS[id] ?? null; + } + // Never render user-controlled custom avatar URLs in onboarding. + return null; } export function RuntimeIcon({ @@ -51,16 +42,8 @@ export function RuntimeIcon({ runtime: AcpRuntimeCatalogEntry; }) { const [imageFailed, setImageFailed] = React.useState(false); - const { isDark } = useTheme(); - // Only use bundled logo maps — never render user-supplied avatar URLs for - // custom/preset entries (tracking pixel / spoofing vector, security line). const id = runtime.id.trim().toLowerCase(); const imageUrl = getRuntimeLogoUrl(runtime); - const shouldForceForegroundColor = !imageUrl && id === "goose"; - - if (isBuzzRuntime(runtime)) { - return ; - } if (imageUrl && !imageFailed) { return ( @@ -71,11 +54,10 @@ export function RuntimeIcon({ className, id === "omp" && "bg-[#0d0d0d] p-1", id === "grok" && "bg-white p-1", - shouldForceForegroundColor && - (isDark ? "brightness-0 invert" : "brightness-0"), )} onError={() => setImageFailed(true)} src={imageUrl} + style={{ transform: `scale(${runtime.iconScale})` }} /> ); } diff --git a/desktop/src/features/onboarding/ui/agentReadiness.test.mjs b/desktop/src/features/onboarding/ui/agentReadiness.test.mjs index 9d6deafb09..2ce228038d 100644 --- a/desktop/src/features/onboarding/ui/agentReadiness.test.mjs +++ b/desktop/src/features/onboarding/ui/agentReadiness.test.mjs @@ -15,6 +15,9 @@ function makeRuntime(overrides = {}) { binaryPath: "/usr/local/bin/goose", defaultArgs: [], mcpCommand: null, + modelEnvVar: "GOOSE_MODEL", + providerEnvVar: "GOOSE_PROVIDER", + thinkingEnvVar: "GOOSE_THINKING_EFFORT", installHint: "", installInstructionsUrl: "https://example.com", canAutoInstall: false, @@ -40,7 +43,15 @@ function makeConfig(overrides = {}) { // --------------------------------------------------------------------------- test("resolveAgentReadiness_cli_returns_ready_when_preferred_cli_runtime_is_logged_in", () => { - const runtimes = [makeRuntime({ id: "claude", label: "Claude" })]; + const runtimes = [ + makeRuntime({ + id: "claude", + label: "Claude", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), + ]; const result = resolveAgentReadiness( runtimes, makeConfig({ preferred_runtime: "claude" }), @@ -52,9 +63,37 @@ test("resolveAgentReadiness_cli_returns_ready_when_preferred_cli_runtime_is_logg }); }); +test("resolveAgentReadiness_devin_uses_catalog_capabilities_without_an_id_check", () => { + const runtimes = [ + makeRuntime({ + id: "devin", + label: "Devin", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), + ]; + const result = resolveAgentReadiness( + runtimes, + makeConfig({ preferred_runtime: "devin" }), + "preferred", + ); + assert.deepEqual(result, { + ready: true, + reason: "cli", + runtimeLabel: "Devin", + }); +}); + test("resolveAgentReadiness_uses_only_the_preferred_runtime", () => { const runtimes = [ - makeRuntime({ id: "claude", label: "Claude" }), + makeRuntime({ + id: "claude", + label: "Claude", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), makeRuntime({ id: "goose", label: "Goose" }), ]; const result = resolveAgentReadiness(runtimes, makeConfig(), "preferred"); @@ -197,7 +236,15 @@ test("resolveAgentReadiness_neither_returns_not_ready", () => { }); test("resolveAgentReadiness_welcome_readiness_uses_ready_cli_without_preference", () => { - const runtimes = [makeRuntime({ id: "claude", label: "Claude" })]; + const runtimes = [ + makeRuntime({ + id: "claude", + label: "Claude", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), + ]; const result = resolveAgentReadiness( runtimes, makeConfig({ preferred_runtime: null }), diff --git a/desktop/src/features/onboarding/ui/agentReadiness.ts b/desktop/src/features/onboarding/ui/agentReadiness.ts index 86b9721af3..d92d1467c8 100644 --- a/desktop/src/features/onboarding/ui/agentReadiness.ts +++ b/desktop/src/features/onboarding/ui/agentReadiness.ts @@ -12,7 +12,8 @@ export type AgentReadinessResult = /** * Determine whether the user has a working agent path configured. * - * CLI path: the preferred Claude or Codex runtime is available and logged in. + * CLI path: a catalog-declared runtime without provider configuration is + * available and logged in. * Provider path: the preferred Buzz Agent or Goose runtime has provider and * model set, plus all required credential env vars for that provider. * @@ -47,7 +48,7 @@ export function resolveAgentReadiness( } if ( - (preferredRuntime.id === "claude" || preferredRuntime.id === "codex") && + preferredRuntime.providerEnvVar == null && (preferredRuntime.authStatus.status === "logged_in" || preferredRuntime.authStatus.status === "not_applicable") ) { diff --git a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs index 221702ebb2..e7c7e459c5 100644 --- a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs +++ b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs @@ -8,29 +8,64 @@ import { runtimeIsVisibleInOnboarding, } from "./onboardingRuntimeSelection.ts"; -function runtime(id, availability, status) { - return { id, availability, authStatus: { status } }; +function runtime( + id, + availability, + status, + { onboardingVisible = true, sortPriority = 100 } = {}, +) { + return { + id, + availability, + authStatus: { status }, + onboardingVisible, + sortPriority, + }; } -test("all bundled harnesses are visible in onboarding", () => { - assert.equal(runtimeIsVisibleInOnboarding("claude"), true); - assert.equal(runtimeIsVisibleInOnboarding("codex"), true); - assert.equal(runtimeIsVisibleInOnboarding("goose"), true); - assert.equal(runtimeIsVisibleInOnboarding("buzz-agent"), true); - assert.equal(runtimeIsVisibleInOnboarding("custom"), false); +test("onboarding visibility comes from catalog metadata", () => { + assert.equal( + runtimeIsVisibleInOnboarding(runtime("devin", "available", "logged_in")), + true, + ); + assert.equal( + runtimeIsVisibleInOnboarding( + runtime("future-runtime", "available", "logged_in", { + onboardingVisible: false, + }), + ), + false, + ); }); -test("visible onboarding runtimes use the product order", () => { +test("visible onboarding runtimes use catalog ordering", () => { const runtimes = [ - runtime("buzz-agent", "available", "not_applicable"), - runtime("codex", "available", "logged_in"), - runtime("goose", "available", "not_applicable"), - runtime("claude", "available", "logged_in"), + runtime("buzz-agent", "available", "not_applicable", { + sortPriority: 60, + }), + runtime("codex", "available", "logged_in", { sortPriority: 40 }), + runtime("goose", "available", "not_applicable", { + sortPriority: 50, + }), + runtime("devin", "available", "logged_in", { sortPriority: 20 }), + runtime("claude", "available", "logged_in", { sortPriority: 30 }), ]; assert.deepEqual( getVisibleOnboardingRuntimes(runtimes).map(({ id }) => id), - ["claude", "codex", "goose", "buzz-agent"], + ["devin", "claude", "codex", "goose", "buzz-agent"], + ); +}); + +test("catalog ordering falls back to labels for rolling-upgrade payloads", () => { + const alpha = runtime("alpha", "available", "logged_in"); + alpha.label = "Alpha"; + const beta = runtime("beta", "available", "logged_in"); + beta.label = "Beta"; + + assert.deepEqual( + getVisibleOnboardingRuntimes([beta, alpha]).map(({ id }) => id), + ["alpha", "beta"], ); }); @@ -39,6 +74,10 @@ test("readiness requires an available and authenticated runtime", () => { runtimeIsReadyForOnboarding(runtime("claude", "available", "logged_in")), true, ); + assert.equal( + runtimeIsReadyForOnboarding(runtime("devin", "available", "logged_in")), + true, + ); assert.equal( runtimeIsReadyForOnboarding( runtime("codex", "available", "not_applicable"), @@ -57,11 +96,14 @@ test("readiness requires an available and authenticated runtime", () => { test("ready onboarding runtimes exclude unknown and non-ready harnesses", () => { const runtimes = [ - runtime("goose", "available", "not_applicable"), - runtime("codex", "available", "logged_out"), - runtime("buzz-agent", "available", "not_applicable"), - runtime("claude", "available", "logged_in"), - runtime("custom", "available", "not_applicable"), + runtime("goose", "available", "not_applicable", { sortPriority: 50 }), + runtime("codex", "available", "logged_out", { sortPriority: 40 }), + runtime("buzz-agent", "available", "not_applicable", { sortPriority: 60 }), + runtime("devin", "available", "logged_out", { sortPriority: 20 }), + runtime("claude", "available", "logged_in", { sortPriority: 30 }), + runtime("custom", "available", "not_applicable", { + onboardingVisible: false, + }), ]; assert.deepEqual( diff --git a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts index cd491dfcc5..baef13ac34 100644 --- a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts +++ b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts @@ -1,18 +1,7 @@ import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; -export const ONBOARDING_RUNTIME_ORDER = [ - "claude", - "codex", - "goose", - "buzz-agent", -]; - -const VISIBLE_ONBOARDING_RUNTIME_IDS = new Set( - ONBOARDING_RUNTIME_ORDER, -); - -export function runtimeIsVisibleInOnboarding(runtimeId: string) { - return VISIBLE_ONBOARDING_RUNTIME_IDS.has(runtimeId); +export function runtimeIsVisibleInOnboarding(runtime: AcpRuntimeCatalogEntry) { + return runtime.onboardingVisible; } export function runtimeIsReadyForOnboarding(runtime: AcpRuntimeCatalogEntry) { @@ -27,11 +16,13 @@ export function getVisibleOnboardingRuntimes( runtimes: readonly AcpRuntimeCatalogEntry[], ) { return runtimes - .filter((runtime) => runtimeIsVisibleInOnboarding(runtime.id)) + .filter(runtimeIsVisibleInOnboarding) .sort( (left, right) => - ONBOARDING_RUNTIME_ORDER.indexOf(left.id) - - ONBOARDING_RUNTIME_ORDER.indexOf(right.id), + left.sortPriority - right.sortPriority || + (left.displayLabel || left.label || left.id).localeCompare( + right.displayLabel || right.label || right.id, + ), ); } diff --git a/desktop/src/features/profile/ui/ProfileAvatar.tsx b/desktop/src/features/profile/ui/ProfileAvatar.tsx index 3153fb4be1..41f70ff819 100644 --- a/desktop/src/features/profile/ui/ProfileAvatar.tsx +++ b/desktop/src/features/profile/ui/ProfileAvatar.tsx @@ -16,6 +16,7 @@ type ProfileAvatarProps = { className?: string; iconClassName?: string; imageClassName?: string; + imageStyle?: React.CSSProperties; plain?: boolean; testId?: string; }; @@ -27,6 +28,7 @@ export function ProfileAvatar({ className, iconClassName, imageClassName, + imageStyle, plain = false, testId, }: ProfileAvatarProps) { @@ -87,6 +89,7 @@ export function ProfileAvatar({ }} referrerPolicy="no-referrer" src={src} + style={imageStyle} /> ) : null} {shouldShowFallback ? ( diff --git a/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx b/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx index 96127876a8..d1d669a532 100644 --- a/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx +++ b/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx @@ -35,25 +35,6 @@ import { SectionHeader } from "@/shared/ui/PageHeader"; import { Spinner } from "@/shared/ui/spinner"; import { Switch } from "@/shared/ui/switch"; -const RUNTIME_LOGO_URLS: Record = { - "buzz-agent": "/app-icon@2x.png", - claude: "/runtime-icons/claude.png", - codex: "/runtime-icons/codex.png", - goose: "/runtime-icons/goose.svg", -}; - -const RUNTIME_LOGO_SCALE: Record = { - "buzz-agent": "scale-110", - claude: "scale-110", - codex: "scale-110", - goose: "scale-125", -}; - -const RUNTIME_SORT_PRIORITY: Record = { - "buzz-agent": 0, - goose: 1, -}; - function runtimeInstallGuideLabel(runtime: AcpRuntimeCatalogEntry) { return runtime.availability === "adapter_missing" || runtime.availability === "adapter_outdated" @@ -81,13 +62,11 @@ function RuntimeLogo({ runtime }: { runtime: AcpRuntimeCatalogEntry }) { ); } - const avatarUrl = RUNTIME_LOGO_URLS[runtime.id] ?? runtime.avatarUrl; - return ( @@ -556,8 +535,10 @@ export function DoctorSettingsPanel() { () => [...(runtimesQuery.data ?? [])].sort( (left, right) => - (RUNTIME_SORT_PRIORITY[left.id] ?? Number.MAX_SAFE_INTEGER) - - (RUNTIME_SORT_PRIORITY[right.id] ?? Number.MAX_SAFE_INTEGER), + left.sortPriority - right.sortPriority || + (left.displayLabel || left.label || left.id).localeCompare( + right.displayLabel || right.label || right.id, + ), ), [runtimesQuery.data], ); diff --git a/desktop/src/shared/api/runtimeCapabilities.ts b/desktop/src/shared/api/runtimeCapabilities.ts new file mode 100644 index 0000000000..a7062bc4c3 --- /dev/null +++ b/desktop/src/shared/api/runtimeCapabilities.ts @@ -0,0 +1,46 @@ +export type ManagedAgentRuntimeCapabilities = { + runtimeIconUrl: string | null; + runtimeAvatarUrl: string | null; + runtimeSupersededAvatarUrls: string[]; + supportsBuzzModelConfig: boolean | null; +}; + +export type RuntimeCatalogCapabilities = { + displayLabel: string; + sortPriority: number; + onboardingVisible: boolean; + iconUrl: string; + iconScale: number; + supersededAvatarUrls: string[]; + supportsBuzzModelConfig: boolean; +}; + +export type RuntimeConfigCapabilities = { + supportsBuzzModelConfig: boolean | null; +}; + +export type ManagedAgentLog = { + content: string; + logPath: string; +}; + +export type CancelManagedAgentTurnResult = { + status: "sent" | "no_active_turn"; +}; + +/** + * Outcome of a live model-switch control frame, surfaced asynchronously via + * the agent's control-result observer frame. + */ +export type SwitchManagedAgentModelStatus = + | "sent" + | "turn_ending" + | "switched" + | "unsupported_model" + | "no_active_turn"; + +export type ControlResultFrame = { + type: "cancel_turn" | "switch_model" | "permission_decision"; + status: string; + modelId?: string; +}; diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index e8a8b885cf..5bfd1bb088 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -133,6 +133,10 @@ export type RawManagedAgent = { parallelism: number; system_prompt: string | null; avatar_url?: string | null; + runtime_icon_url?: string | null; + runtime_avatar_url?: string | null; + runtime_superseded_avatar_urls?: string[]; + supports_buzz_model_config?: boolean | null; model: string | null; model_source?: ManagedAgent["modelSource"]; provider: string | null; @@ -175,7 +179,21 @@ type RawManagedAgentLog = { export type RawAcpRuntimeCatalogEntry = { id: string; label: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + display_label?: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + sort_priority?: number; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + onboarding_visible?: boolean; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + icon_url?: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + icon_scale?: number; avatar_url: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + superseded_avatar_urls?: string[]; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + supports_buzz_model_config?: boolean; availability: AcpAvailabilityStatus; command: string | null; binary_path: string | null; @@ -710,6 +728,10 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { parallelism: agent.parallelism, systemPrompt: agent.system_prompt, avatarUrl: agent.avatar_url ?? null, + runtimeIconUrl: agent.runtime_icon_url ?? null, + runtimeAvatarUrl: agent.runtime_avatar_url ?? null, + runtimeSupersededAvatarUrls: agent.runtime_superseded_avatar_urls ?? [], + supportsBuzzModelConfig: agent.supports_buzz_model_config ?? null, model: agent.model, modelSource: agent.model_source ?? null, // Fallbacks for pre-feature mocks/fixtures. Real records always carry them. @@ -745,7 +767,14 @@ export function fromRawAcpRuntimeCatalogEntry( return { id: entry.id, label: entry.label, + displayLabel: entry.display_label ?? entry.label, + sortPriority: entry.sort_priority ?? 100, + onboardingVisible: entry.onboarding_visible ?? false, + iconUrl: entry.icon_url ?? entry.avatar_url, + iconScale: entry.icon_scale ?? 1, avatarUrl: entry.avatar_url, + supersededAvatarUrls: entry.superseded_avatar_urls ?? [], + supportsBuzzModelConfig: entry.supports_buzz_model_config ?? true, availability: entry.availability, command: entry.command, binaryPath: entry.binary_path, diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 50842d0d0a..d000177bf3 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -1,3 +1,11 @@ +import type * as RuntimeCapabilities from "./runtimeCapabilities"; +export type { + CancelManagedAgentTurnResult, + ControlResultFrame, + ManagedAgentLog, + SwitchManagedAgentModelStatus, +} from "./runtimeCapabilities"; + export type ChannelType = "stream" | "forum" | "dm"; export type ChannelVisibility = "open" | "private"; export type ChannelRole = "owner" | "admin" | "member" | "guest" | "bot"; @@ -415,7 +423,7 @@ export type ManagedAgent = { * `"allowlist"`. Preserved across mode toggles. */ respondToAllowlist: string[]; -}; +} & RuntimeCapabilities.ManagedAgentRuntimeCapabilities; /** * Inbound author gate mode. Mirrors `buzz-acp`'s `--respond-to` CLI flag. @@ -487,34 +495,6 @@ export type CreateManagedAgentResponse = { spawnError: string | null; }; -export type ManagedAgentLog = { - content: string; - logPath: string; -}; - -export type CancelManagedAgentTurnResult = { - status: "sent" | "no_active_turn"; -}; - -/** - * Outcome of a live `switch_model` control frame, surfaced asynchronously via - * the agent's `control_result` observer frame. Busy path: `sent` (cancel + - * requeue on the new model) or `turn_ending` (oneshot already consumed this - * turn). Idle path: `switched`, `unsupported_model`, or `no_active_turn`. - */ -export type SwitchManagedAgentModelStatus = - | "sent" - | "turn_ending" - | "switched" - | "unsupported_model" - | "no_active_turn"; - -export type ControlResultFrame = { - type: "cancel_turn" | "switch_model"; - status: string; - modelId?: string; -}; - export type GitBashPrerequisite = { available: boolean; path: string | null; @@ -579,7 +559,7 @@ export type AcpRuntimeCatalogEntry = { * for `builtin` and `preset` entries. */ definitionEnv?: Record; -}; +} & RuntimeCapabilities.RuntimeCatalogCapabilities; /** An AcpRuntimeCatalogEntry that is confirmed available — command and binaryPath are non-null. */ export type AcpRuntime = AcpRuntimeCatalogEntry & { @@ -724,7 +704,7 @@ export type RuntimeConfigSurface = { advanced: ConfigField[]; extensions: ExtensionEntry[]; sources: ConfigSourceReport; -}; +} & RuntimeCapabilities.RuntimeConfigCapabilities; export type UpdateManagedAgentInput = { pubkey: string; diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 493908c872..8cd9f97346 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -6984,6 +6984,11 @@ async function handleDiscoverAcpRuntimes( { id: "goose", label: "Goose", + display_label: "Goose", + sort_priority: 10, + onboarding_visible: false, + icon_url: "/runtime-icons/goose.svg", + icon_scale: 1.25, avatar_url: "", availability: "available", command: "goose", @@ -7003,6 +7008,11 @@ async function handleDiscoverAcpRuntimes( { id: "claude", label: "Claude Code", + display_label: "Claude Code", + sort_priority: 30, + onboarding_visible: true, + icon_url: "/runtime-icons/claude.png", + icon_scale: 1.1, avatar_url: "", availability: "adapter_missing", command: null, @@ -7020,9 +7030,39 @@ async function handleDiscoverAcpRuntimes( source: "builtin", login_hint: undefined, }, + { + id: "devin", + label: "Devin", + display_label: "Devin", + sort_priority: 20, + onboarding_visible: true, + icon_url: "/runtime-icons/devin.svg", + icon_scale: 1.1, + avatar_url: "", + availability: "not_installed", + command: null, + binary_path: null, + default_args: ["acp"], + mcp_command: null, + install_hint: + "Buzz requires the Devin CLI; the desktop app alone is not enough.", + install_instructions_url: "https://docs.devin.ai/cli", + can_auto_install: true, + requires_external_cli: true, + underlying_cli_path: null, + node_required: false, + auth_status: { status: "unknown" }, + login_hint: "Run `devin auth login` to authenticate.", + source: "builtin", + }, { id: "codex", label: "Codex", + display_label: "Codex", + sort_priority: 40, + onboarding_visible: true, + icon_url: "/runtime-icons/codex.png", + icon_scale: 1.1, avatar_url: "", availability: "not_installed", command: null, @@ -7043,6 +7083,11 @@ async function handleDiscoverAcpRuntimes( { id: "buzz-agent", label: "Buzz Agent", + display_label: "Buzz", + sort_priority: 0, + onboarding_visible: false, + icon_url: "/app-icon@2x.png", + icon_scale: 1.1, avatar_url: "", availability: "available", command: "buzz-agent", diff --git a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts index 1c3e86f133..7fee103d1a 100644 --- a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts +++ b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts @@ -3,7 +3,7 @@ import { installMockBridge } from "../helpers/bridge"; import { passThroughBackupStep } from "../helpers/onboarding"; function runtime( - id: "buzz-agent" | "claude" | "codex" | "goose", + id: "buzz-agent" | "claude" | "codex" | "devin" | "goose", availability: string, authStatus: Record, overrides: Record = {}, @@ -17,12 +17,20 @@ function runtime( ? "Claude Code" : id === "codex" ? "Codex" - : "Goose", + : id === "devin" + ? "Devin" + : "Goose", + display_label: id === "buzz-agent" ? "Buzz" : undefined, + source: "builtin", avatar_url: "", + sort_priority: 100, + onboarding_visible: true, + icon_url: "", + icon_scale: 1, availability, command: availability === "available" ? id : null, binary_path: availability === "available" ? `/usr/local/bin/${id}` : null, - default_args: [], + default_args: id === "devin" || id === "goose" ? ["acp"] : [], mcp_command: null, install_hint: `Install ${id}`, install_instructions_url: "https://example.com", @@ -57,15 +65,47 @@ async function readSavedRuntime(page: Parameters[0]) { }); } -test("setup shows all bundled harnesses as detected", async ({ page }) => { +test("setup projects catalog visibility and ordering, including Devin", async ({ + page, +}) => { await installMockBridge( page, { acpRuntimesCatalog: [ - runtime("buzz-agent", "available", { status: "not_applicable" }), - runtime("goose", "available", { status: "not_applicable" }), - runtime("codex", "available", { status: "logged_in" }), - runtime("claude", "available", { status: "logged_in" }), + runtime( + "buzz-agent", + "available", + { status: "not_applicable" }, + { onboarding_visible: true, sort_priority: 60 }, + ), + runtime( + "goose", + "available", + { status: "not_applicable" }, + { onboarding_visible: true, sort_priority: 50 }, + ), + runtime( + "codex", + "available", + { status: "logged_in" }, + { sort_priority: 40 }, + ), + runtime( + "devin", + "available", + { status: "logged_in" }, + { + sort_priority: 20, + icon_url: "/runtime-icons/devin.svg", + icon_scale: 1.1, + }, + ), + runtime( + "claude", + "available", + { status: "logged_in" }, + { sort_priority: 30 }, + ), ], }, { skipCommunitySeed: true, skipOnboardingSeed: true }, @@ -73,11 +113,83 @@ test("setup shows all bundled harnesses as detected", async ({ page }) => { await page.goto("/"); await navigateToSetupPage(page); + await expect(page.getByTestId("onboarding-runtime-devin")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-claude")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-codex")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-goose")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-buzz-agent")).toBeVisible(); await expect(page.getByRole("checkbox")).toHaveCount(0); + + const visibleRuntimeIds = await page + .locator("[data-testid^='onboarding-runtime-']") + .evaluateAll((elements) => + elements + .map((element) => element.getAttribute("data-testid")) + .filter( + (testId) => + testId != null && + !testId.includes("-ready-") && + !testId.includes("-checkmark-") && + !testId.includes("-instructions-") && + !testId.includes("-install-"), + ), + ); + expect(visibleRuntimeIds).toEqual([ + "onboarding-runtime-devin", + "onboarding-runtime-claude", + "onboarding-runtime-codex", + "onboarding-runtime-goose", + "onboarding-runtime-buzz-agent", + ]); + const devinIcon = page.getByTestId("onboarding-runtime-devin").locator("img"); + await expect(devinIcon).toHaveAttribute("src", "/runtime-icons/devin.svg"); + + const renderedIcon = await devinIcon.evaluate(async (element) => { + const image = element as HTMLImageElement; + await image.decode(); + + const canvas = document.createElement("canvas"); + canvas.width = image.naturalWidth; + canvas.height = image.naturalHeight; + const context = canvas.getContext("2d", { willReadFrequently: true }); + if (!context) throw new Error("2D canvas context unavailable"); + context.drawImage(image, 0, 0); + + const pixels = context.getImageData(0, 0, canvas.width, canvas.height).data; + let whitePixels = 0; + let darkPixels = 0; + let transparentPixels = 0; + for (let offset = 0; offset < pixels.length; offset += 4) { + const red = pixels[offset]; + const green = pixels[offset + 1]; + const blue = pixels[offset + 2]; + const alpha = pixels[offset + 3]; + if (alpha === 0) transparentPixels += 1; + if (alpha === 255 && red > 250 && green > 250 && blue > 250) { + whitePixels += 1; + } + if (alpha === 255 && red < 32 && green < 32 && blue < 32) { + darkPixels += 1; + } + } + + const pixelCount = canvas.width * canvas.height; + return { + naturalHeight: image.naturalHeight, + naturalWidth: image.naturalWidth, + transparentPixels, + whiteRatio: whitePixels / pixelCount, + darkRatio: darkPixels / pixelCount, + corner: Array.from(context.getImageData(0, 0, 1, 1).data), + }; + }); + + expect(renderedIcon.naturalWidth).toBe(425); + expect(renderedIcon.naturalHeight).toBe(425); + expect(renderedIcon.corner).toEqual([255, 255, 255, 255]); + expect(renderedIcon.transparentPixels).toBe(0); + expect(renderedIcon.whiteRatio).toBeGreaterThan(0.5); + expect(renderedIcon.darkRatio).toBeGreaterThan(0.05); }); test("setup distinguishes a missing CLI from an installed desktop app", async ({ diff --git a/desktop/tests/e2e/onboarding.spec.ts b/desktop/tests/e2e/onboarding.spec.ts index bfbfc6f063..dac0a68e60 100644 --- a/desktop/tests/e2e/onboarding.spec.ts +++ b/desktop/tests/e2e/onboarding.spec.ts @@ -1181,6 +1181,8 @@ test("first-community shows the scenario cards for localhost", async ({ { id: "claude", label: "Claude Code", + sort_priority: 30, + onboarding_visible: true, avatar_url: "", availability: "available", command: "claude", From 1387fbc41dad7f18c175590b58df726365c6512a Mon Sep 17 00:00:00 2001 From: Mark Fenner Date: Sun, 26 Jul 2026 22:57:30 -0400 Subject: [PATCH 2/7] feat(acp): mediate runtime permissions through owners Disable automatic approval for catalog-enforced managed runtimes, surface exact pending ACP requests in the activity transcript, and accept only encrypted owner-signed one-shot decisions that match the active channel, turn, request, and advertised option. Signed-off-by: Mark Fenner --- crates/buzz-acp/src/acp.rs | 432 ++++++++++++++++-- crates/buzz-acp/src/config.rs | 129 +++++- crates/buzz-acp/src/lib.rs | 354 +++++++++++++- crates/buzz-acp/src/pool.rs | 162 ++++++- crates/buzz-acp/src/queue.rs | 30 +- .../LifecycleActivity.tsx | 93 ++++ .../agents/ui/agentSessionPermission.ts | 114 +++++ .../agents/ui/agentSessionTranscript.test.mjs | 64 ++- .../agents/ui/agentSessionTranscript.ts | 104 +---- .../features/agents/ui/agentSessionTypes.ts | 10 + .../features/channels/ui/BotActivityBar.tsx | 46 +- desktop/src/shared/api/agentControl.ts | 21 + 12 files changed, 1422 insertions(+), 137 deletions(-) create mode 100644 desktop/src/features/agents/ui/agentSessionPermission.ts diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 78db7ff718..d2b5cb8378 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -132,6 +132,56 @@ fn build_initialize_params() -> serde_json::Value { }) } +#[cfg(test)] +fn permission_option_id( + options: &[serde_json::Value], + approve: bool, +) -> Result<(&str, bool), AcpError> { + if approve { + if let Some(option) = options + .iter() + .find(|option| option.get("kind").and_then(|kind| kind.as_str()) == Some("allow_once")) + { + let option_id = option["optionId"] + .as_str() + .ok_or_else(|| AcpError::Protocol("allow_once option missing optionId".into()))?; + return Ok((option_id, true)); + } + } + + let option = options + .iter() + .find(|option| option.get("kind").and_then(|kind| kind.as_str()) == Some("reject_once")) + .ok_or_else(|| { + AcpError::Protocol("no reject_once option available for permission response".into()) + })?; + let option_id = option["optionId"] + .as_str() + .ok_or_else(|| AcpError::Protocol("reject_once option missing optionId".into()))?; + Ok((option_id, false)) +} + +fn permission_option_for_selection<'a>( + options: &'a [serde_json::Value], + selection: &crate::pool::PermissionSelection, +) -> Option<(&'a str, &'a str)> { + options.iter().find_map(|option| { + let option_id = option.get("optionId")?.as_str()?; + let kind = option.get("kind")?.as_str()?; + // Interactive managed-runtime consent is deliberately one-shot. + // Never forward allow_always/reject_always, even if an owner-signed + // control names an exact persistent option offered by the runtime. + if !matches!(kind, "allow_once" | "reject_once") { + return None; + } + let matches = match selection { + crate::pool::PermissionSelection::OptionId(selected) => option_id == selected, + crate::pool::PermissionSelection::Kind(selected) => kind == selected, + }; + matches.then_some((option_id, kind)) + }) +} + /// ACP client that owns an agent subprocess and communicates over its stdio. /// /// One `AcpClient` per agent process. Multiple sessions can be created on the @@ -158,6 +208,16 @@ pub struct AcpClient { /// Guards against double-response if a timeout fires after the allow_once /// response was written but before `pending_permission_id` was cleared. permission_responded: bool, + /// Whether ACP permission requests may be approved with `allow_once`. + /// + /// Buzz historically auto-approves these requests. Managed runtimes may + /// disable that fallback so a headless community prompt cannot silently + /// stand in for user consent. + auto_approve_permissions: bool, + /// Whether owner-signed observer controls may resolve permission requests. + interactive_permissions: bool, + /// Per-turn channel for owner permission decisions. + permission_rx: Option>, /// The JSON-RPC id of the most recently sent `session/prompt` request. /// Used by [`cancel_with_cleanup`] to drain the correct response. /// Set in [`session_prompt_with_idle_timeout`]; consumed in [`cancel_with_cleanup`]. @@ -488,6 +548,9 @@ impl AcpClient { next_id: 0, pending_permission_id: None, permission_responded: false, + auto_approve_permissions: true, + interactive_permissions: false, + permission_rx: None, last_prompt_id: None, current_hard_deadline: None, observer: None, @@ -505,6 +568,38 @@ impl AcpClient { self.observer_agent_index = Some(agent_index); } + /// Control whether `session/request_permission` may select `allow_once`. + /// + /// When disabled, the client selects `reject_once` and never falls back to + /// an allow option. This is set from the managed runtime launch policy at + /// the start of every prompt. + pub fn set_auto_approve_permissions(&mut self, enabled: bool) { + self.auto_approve_permissions = enabled; + } + + /// Control whether owner-signed observer controls may select an exact option + /// offered by the current permission request. + pub fn set_interactive_permissions(&mut self, enabled: bool) { + self.interactive_permissions = enabled; + } + + /// Install the permission-decision channel for one prompt turn. + pub fn install_permission_rx( + &mut self, + rx: tokio::sync::mpsc::Receiver, + ) { + debug_assert!( + self.permission_rx.is_none(), + "install_permission_rx: previous turn receiver was not cleared" + ); + self.permission_rx = Some(rx); + } + + /// Clear any permission receiver before the agent returns to the pool. + pub fn clear_permission_rx(&mut self) { + self.permission_rx = None; + } + /// Update metadata that will be attached to subsequent raw wire events. pub fn set_observer_context(&mut self, context: ObserverContext) { self.observer_context = context; @@ -706,6 +801,19 @@ impl AcpClient { self.current_hard_deadline = None; return Err(e); } + let prompt_dispatched_at = tokio::time::Instant::now(); + self.observe( + "prompt_dispatched", + serde_json::json!({ + "idleTimeoutSeconds": idle_timeout.as_secs(), + "maxDurationSeconds": max_duration.as_secs(), + }), + ); + tracing::info!( + idle_timeout_secs = idle_timeout.as_secs(), + max_duration_secs = max_duration.as_secs(), + "ACP prompt dispatched" + ); let result = self .read_until_response_with_idle_timeout( @@ -716,6 +824,19 @@ impl AcpClient { max_duration, ) .await; + let elapsed_ms = prompt_dispatched_at.elapsed().as_millis() as u64; + self.observe( + "prompt_wait_finished", + serde_json::json!({ + "elapsedMs": elapsed_ms, + "success": result.is_ok(), + }), + ); + tracing::info!( + elapsed_ms, + success = result.is_ok(), + "ACP prompt wait finished" + ); // On timeout errors, leave current_hard_deadline set so cancel_with_cleanup // can inherit the remaining budget. Clear it on all other outcomes. @@ -1224,6 +1345,7 @@ impl AcpClient { let mut idle_deadline = now + idle_timeout; let mut hard_deadline = hard_deadline; let mut last_activity_at = now; + let mut first_activity_observed = false; loop { // Determine which deadline fires first BEFORE sleeping — this is @@ -1411,6 +1533,15 @@ impl AcpClient { continue; } }; + if !first_activity_observed { + first_activity_observed = true; + let elapsed_ms = now.elapsed().as_millis() as u64; + self.observe( + "prompt_first_activity", + serde_json::json!({ "elapsedMs": elapsed_ms }), + ); + tracing::info!(elapsed_ms, "ACP prompt produced first activity"); + } self.observe("acp_read", msg.clone()); let activity_now = Instant::now(); @@ -1668,10 +1799,12 @@ impl AcpClient { } } - /// Auto-approve a `session/request_permission` request from the agent. + /// Respond to a `session/request_permission` request from the agent. /// - /// Finds the option with `kind == "allow_once"` and responds with its `optionId`. - /// If no `allow_once` option exists, falls back to `reject_once`. + /// When auto-approval is enabled, finds the option with + /// `kind == "allow_once"` and responds with its `optionId`. Otherwise it + /// selects `reject_once`. A missing allow option also falls back to + /// `reject_once`; a missing reject option is a protocol error. /// /// **Critical:** Never hardcode `optionId` — always find it dynamically by `kind`. /// @@ -1699,39 +1832,79 @@ impl AcpClient { options.len() ); - // Find allow_once by kind — NEVER hardcode optionId. - let allow_once = options - .iter() - .find(|opt| opt.get("kind").and_then(|k| k.as_str()) == Some("allow_once")); + // Auto-approval preserves the historical behavior for existing + // runtimes. Interactive mode waits only for an owner-signed control + // routed to this exact turn and request id. Any missing, stale, closed, + // or timed-out decision fails closed. + let selection = if self.auto_approve_permissions { + Some(crate::pool::PermissionSelection::Kind( + "allow_once".to_string(), + )) + } else if self.interactive_permissions { + self.wait_for_permission_decision(&id).await + } else { + None + }; - let response = if let Some(opt) = allow_once { - let option_id = opt["optionId"] - .as_str() - .ok_or_else(|| AcpError::Protocol("allow_once option missing optionId".into()))?; - tracing::info!( - target: "acp::permission", - "auto-approving permission id={id} with allow_once optionId={option_id:?}" - ); + // Owner controls carry an exact option ID. Validate it against this + // live request before returning it to the runtime. Any missing or + // unknown selection fails closed to reject_once. + let selected = selection + .as_ref() + .and_then(|selection| permission_option_for_selection(options, selection)); + let (option_id, kind) = match selected { + Some(selected) => selected, + None => { + if selection.is_some() { + tracing::warn!( + target: "acp::permission", + "permission selection did not match the current request — rejecting" + ); + } + permission_option_for_selection( + options, + &crate::pool::PermissionSelection::Kind("reject_once".to_string()), + ) + .ok_or_else(|| { + AcpError::Protocol( + "no reject_once option available for permission response".into(), + ) + })? + } + }; + let approved = !kind.starts_with("reject"); + + let response = if approved { + if self.auto_approve_permissions { + tracing::info!( + target: "acp::permission", + "auto-approving permission id={id} with {kind} optionId={option_id:?}" + ); + } else { + tracing::info!( + target: "acp::permission", + "owner selected permission id={id} with {kind} optionId={option_id:?}" + ); + } permission_response_selected(&id, option_id) } else { - // No allow_once — fall back to reject_once. - tracing::warn!( - target: "acp::permission", - "no allow_once option found in permission request id={id}, falling back to reject_once" - ); - let reject = options - .iter() - .find(|opt| opt.get("kind").and_then(|k| k.as_str()) == Some("reject_once")); - - if let Some(opt) = reject { - let option_id = opt["optionId"].as_str().unwrap_or("reject"); - permission_response_selected(&id, option_id) + if self.auto_approve_permissions && selection.is_some() { + tracing::warn!( + target: "acp::permission", + "no allow_once option found in permission request id={id}, falling back to reject_once" + ); + } else if self.interactive_permissions { + tracing::info!( + target: "acp::permission", + "owner rejected or did not resolve permission request id={id}" + ); } else { - return Err(AcpError::Protocol( - "no suitable permission option found (neither allow_once nor reject_once)" - .into(), - )); + tracing::info!( + target: "acp::permission", + "rejecting permission request id={id} because auto-approval is disabled" + ); } + permission_response_selected(&id, option_id) }; // Write the response first, then mark as responded. @@ -1754,6 +1927,58 @@ impl AcpClient { Ok(()) } + /// Wait for a decision that matches this ACP JSON-RPC request id. + /// + /// The receiver is scoped to one prompt turn. The hard turn deadline also + /// bounds this wait so an unattended prompt cannot stay parked forever. + async fn wait_for_permission_decision( + &mut self, + request_id: &serde_json::Value, + ) -> Option { + let Some(deadline) = self.current_hard_deadline else { + tracing::warn!( + target: "acp::permission", + "interactive permission request has no hard deadline — rejecting" + ); + return None; + }; + let Some(rx) = self.permission_rx.as_mut() else { + tracing::warn!( + target: "acp::permission", + "interactive permission request has no owner decision channel — rejecting" + ); + return None; + }; + + loop { + match tokio::time::timeout_at(deadline, rx.recv()).await { + Ok(Some(decision)) if decision.request_id == *request_id => { + return Some(decision.selection); + } + Ok(Some(_)) => { + tracing::debug!( + target: "acp::permission", + "ignoring permission decision for a different request id" + ); + } + Ok(None) => { + tracing::warn!( + target: "acp::permission", + "permission decision channel closed — rejecting" + ); + return None; + } + Err(_) => { + tracing::info!( + target: "acp::permission", + "interactive permission request timed out — rejecting" + ); + return None; + } + } + } + } + /// Parse `stopReason` from a `session/prompt` result value. fn parse_stop_reason(&self, result: &serde_json::Value) -> Result { let raw = result["stopReason"].as_str().ok_or_else(|| { @@ -2111,6 +2336,109 @@ mod tests { assert_eq!(reject_once.unwrap()["optionId"].as_str(), Some("rej-x")); } + #[test] + fn permission_policy_rejects_when_auto_approval_is_disabled() { + let options = serde_json::json!([ + {"optionId": "reject-this", "kind": "reject_once"}, + {"optionId": "allow-this", "kind": "allow_once"} + ]); + let options = options.as_array().unwrap(); + + let (option_id, approved) = permission_option_id(options, false).unwrap(); + + assert_eq!(option_id, "reject-this"); + assert!(!approved); + } + + #[test] + fn permission_policy_preserves_existing_allow_once_behavior_when_enabled() { + let options = serde_json::json!([ + {"optionId": "reject-this", "kind": "reject_once"}, + {"optionId": "allow-this", "kind": "allow_once"} + ]); + let options = options.as_array().unwrap(); + + let (option_id, approved) = permission_option_id(options, true).unwrap(); + + assert_eq!(option_id, "allow-this"); + assert!(approved); + } + + #[test] + fn permission_selection_resolves_exact_one_shot_option_id() { + let options = serde_json::json!([ + {"optionId": "allow-once", "kind": "allow_once"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + let selection = crate::pool::PermissionSelection::OptionId("allow-once".to_string()); + + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + Some(("allow-once", "allow_once")) + ); + } + + #[test] + fn permission_selection_rejects_persistent_options() { + let options = serde_json::json!([ + {"optionId": "allow-once", "kind": "allow_once"}, + { + "optionId": "allow-buzz-messages-in-workspace", + "kind": "allow_always" + }, + {"optionId": "reject-always", "kind": "reject_always"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + + for option_id in ["allow-buzz-messages-in-workspace", "reject-always"] { + let selection = crate::pool::PermissionSelection::OptionId(option_id.to_string()); + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + None, + "{option_id} must not be actionable" + ); + } + } + + #[test] + fn permission_selection_by_kind_rejects_persistent_options() { + let options = serde_json::json!([ + {"optionId": "allow-always", "kind": "allow_always"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + let selection = crate::pool::PermissionSelection::Kind("allow_always".to_string()); + + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + None + ); + } + + #[test] + fn permission_selection_rejects_unknown_option_id() { + let options = serde_json::json!([ + {"optionId": "allow-once", "kind": "allow_once"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + let selection = + crate::pool::PermissionSelection::OptionId("not-offered-by-agent".to_string()); + + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + None + ); + } + + #[test] + fn permission_policy_never_allows_when_reject_is_unavailable() { + let options = serde_json::json!([ + {"optionId": "allow-this", "kind": "allow_once"} + ]); + let options = options.as_array().unwrap(); + + assert!(permission_option_id(options, false).is_err()); + } + #[test] fn request_has_id_field() { let id: u64 = 42; @@ -2627,6 +2955,48 @@ mod tests { ); } + #[tokio::test] + async fn prompt_timing_observer_events_do_not_capture_prompt_content() { + let script = r#"IFS= read -r _line +printf '%s\n' '{"jsonrpc":"2.0","id":0,"result":{"stopReason":"end_turn"}}'"#; + let mut client = spawn_script(script).await; + let observer = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(observer.clone()), 0); + + let result = client + .session_prompt_with_idle_timeout( + "test-session", + "SENSITIVE_PROMPT_SENTINEL", + std::time::Duration::from_secs(1), + std::time::Duration::from_secs(5), + ) + .await; + assert!(matches!(result, Ok(StopReason::EndTurn))); + + let events = observer.snapshot(); + for kind in [ + "prompt_dispatched", + "prompt_first_activity", + "prompt_wait_finished", + ] { + assert!( + events.iter().any(|event| event.kind == kind), + "missing {kind} timing event" + ); + } + let timing_payloads = events + .iter() + .filter(|event| event.kind.starts_with("prompt_")) + .map(|event| event.payload.to_string()) + .collect::>() + .join("\n"); + assert!( + !timing_payloads.contains("SENSITIVE_PROMPT_SENTINEL"), + "timing telemetry must never include prompt content" + ); + client.shutdown().await; + } + #[tokio::test] async fn hard_timeout_fires_when_deadline_is_immediate() { let mut client = spawn_script("while true; do echo 'noise'; sleep 0.01; done").await; diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index a38d6faa14..6395b9ef3a 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -361,6 +361,21 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_NO_IGNORE_SELF")] pub no_ignore_self: bool, + /// Seconds to wait after observing a self-authored channel message before + /// closing the exact still-running turn that published it. 0 disables the + /// compatibility recovery. + /// + /// Some ACP adapters can successfully publish their externally visible + /// result and then fail to return `session/prompt`. The grace period lets + /// the adapter finish naturally first; an exact turn-id match prevents a + /// delayed timer from cancelling later work in the same channel. + #[arg( + long = "self-publish-completion-grace", + env = "BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", + default_value_t = 0 + )] + pub self_publish_completion_grace_secs: u64, + /// Maximum number of context messages to include for thread replies and DMs. /// Set to 0 to disable automatic context fetching. Max 100. #[arg(long, env = "BUZZ_ACP_CONTEXT_MESSAGE_LIMIT", default_value_t = 12, @@ -437,6 +452,33 @@ pub struct CliArgs { )] pub permission_mode: PermissionMode, + /// Whether Buzz may answer ACP permission requests with `allow_once`. + /// + /// This preserves the historical harness behavior by default. A managed + /// runtime may enforce `false` when a headless community turn must never + /// substitute for interactive user consent. + #[arg( + long, + env = "BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", + default_value_t = true, + action = clap::ArgAction::Set + )] + pub auto_approve_permissions: bool, + + /// Whether owner-signed observer controls may resolve ACP permission + /// requests interactively with `allow_once` or `reject_once`. + /// + /// Disabled by default so existing runtimes preserve their historical + /// behavior. Managed runtimes that disable auto-approval may enable this + /// owner-consent path without selecting a bypass permission mode. + #[arg( + long, + env = "BUZZ_ACP_INTERACTIVE_PERMISSIONS", + default_value_t = false, + action = clap::ArgAction::Set + )] + pub interactive_permissions: bool, + /// Inbound author gate: which authors' events the harness forwards. /// Modes: owner-only (default), allowlist, anyone, nobody. #[arg( @@ -506,6 +548,9 @@ pub struct Config { pub dedup_mode: DedupMode, pub multiple_event_handling: MultipleEventHandling, pub ignore_self: bool, + /// Runtime compatibility recovery after a self-authored result publish. + /// 0 disables the recovery and preserves historical behavior. + pub self_publish_completion_grace_secs: u64, pub kinds_override: Option>, pub channels_override: Option>, pub no_mention_filter: bool, @@ -524,6 +569,10 @@ pub struct Config { pub model: Option, /// Permission mode to apply after session creation. `Default` = skip. pub permission_mode: PermissionMode, + /// Whether ACP permission requests may select `allow_once`. + pub auto_approve_permissions: bool, + /// Whether owner-signed observer controls may resolve permission requests. + pub interactive_permissions: bool, /// Inbound author gate mode. pub respond_to: RespondTo, /// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist). @@ -616,7 +665,7 @@ pub(crate) fn normalize_agent_command_identity(command: &str) -> String { fn default_agent_args(command: &str) -> Option> { match normalize_agent_command_identity(command).as_str() { - "goose" => Some(vec!["acp".to_string()]), + "goose" | "devin" => Some(vec!["acp".to_string()]), "codex" | "codex-acp" | "claude-agent-acp" | "claude-code-acp" | "claude-code" | "claudecode" | "buzz-agent" => Some(Vec::new()), _ => None, @@ -982,6 +1031,7 @@ impl Config { dedup_mode: args.dedup, multiple_event_handling: args.multiple_event_handling, ignore_self: !args.no_ignore_self, + self_publish_completion_grace_secs: args.self_publish_completion_grace_secs, kinds_override: args.kinds, channels_override: args.channels, no_mention_filter: args.no_mention_filter, @@ -993,6 +1043,8 @@ impl Config { memory_enabled: args.memory && !args.no_memory, model, permission_mode: args.permission_mode, + auto_approve_permissions: args.auto_approve_permissions, + interactive_permissions: args.interactive_permissions, respond_to: args.respond_to, respond_to_allowlist, allowed_respond_to, @@ -1024,7 +1076,7 @@ impl Config { format!(" allowed_respond_to=[{}]", modes.join(",")) }; format!( - "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}", + "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} self_publish_completion_grace={}s context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} auto_approve_permissions={} interactive_permissions={} {}{}", self.relay_url, self.keys.public_key().to_hex(), self.agent_command, @@ -1038,6 +1090,7 @@ impl Config { self.dedup_mode, self.multiple_event_handling, self.ignore_self, + self.self_publish_completion_grace_secs, self.context_message_limit, self.max_turns_per_session, self.presence_enabled, @@ -1045,6 +1098,8 @@ impl Config { self.memory_enabled, self.model.as_deref().unwrap_or("(agent default)"), self.permission_mode, + self.auto_approve_permissions, + self.interactive_permissions, respond_to_detail, allowed_respond_to_detail, ) @@ -1351,6 +1406,7 @@ mod tests { dedup_mode: DedupMode::Queue, multiple_event_handling: MultipleEventHandling::Queue, ignore_self: true, + self_publish_completion_grace_secs: 0, kinds_override: None, channels_override: None, no_mention_filter: false, @@ -1362,6 +1418,8 @@ mod tests { memory_enabled: true, model: None, permission_mode: PermissionMode::BypassPermissions, + auto_approve_permissions: true, + interactive_permissions: false, respond_to: RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: Vec::new(), @@ -1492,6 +1550,22 @@ mod tests { ); } + #[test] + fn normalizes_devin_args_to_native_acp_subcommand() { + assert_eq!(normalize_agent_args("devin", Vec::new()), vec!["acp"]); + assert_eq!( + normalize_agent_args("/usr/local/bin/devin", vec!["".into()]), + vec!["acp"] + ); + assert_eq!( + normalize_agent_args( + "devin", + vec!["acp".into(), "--agent-type".into(), "review".into()] + ), + vec!["acp", "--agent-type", "review"] + ); + } + #[test] fn normalize_agent_command_identity_variants() { assert_eq!(normalize_agent_command_identity("goose"), "goose"); @@ -2048,6 +2122,57 @@ channels = "ALL" assert!(CliArgs::parse_from(["buzz-acp", "--private-key", &key, "--lazy-pool"]).lazy_pool); } + #[test] + fn self_publish_completion_recovery_defaults_off_and_accepts_a_grace() { + let key = "0".repeat(64); + assert_eq!( + CliArgs::parse_from(["buzz-acp", "--private-key", &key]) + .self_publish_completion_grace_secs, + 0 + ); + assert_eq!( + CliArgs::parse_from([ + "buzz-acp", + "--private-key", + &key, + "--self-publish-completion-grace", + "30", + ]) + .self_publish_completion_grace_secs, + 30 + ); + } + + #[test] + fn permission_request_auto_approval_defaults_on_and_can_be_disabled() { + let key = "0".repeat(64); + assert!(CliArgs::parse_from(["buzz-acp", "--private-key", &key]).auto_approve_permissions); + assert!( + !CliArgs::parse_from([ + "buzz-acp", + "--private-key", + &key, + "--auto-approve-permissions=false", + ]) + .auto_approve_permissions + ); + } + + #[test] + fn interactive_permissions_default_off_and_can_be_enabled() { + let key = "0".repeat(64); + assert!(!CliArgs::parse_from(["buzz-acp", "--private-key", &key]).interactive_permissions); + assert!( + CliArgs::parse_from([ + "buzz-acp", + "--private-key", + &key, + "--interactive-permissions=true", + ]) + .interactive_permissions + ); + } + #[test] fn test_summary_includes_agents_and_heartbeat() { let config = test_config(SubscribeMode::Mentions); diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 0230ea0875..58369b98c1 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -885,12 +885,106 @@ fn handle_relay_observer_control_event( Some("switch_model") => { handle_switch_model_control(&payload, pool, observer); } + Some("permission_decision") => { + handle_permission_decision_control(&payload, pool, observer); + } _ => { tracing::debug!(payload = %payload, "ignoring unknown observer control frame"); } } } +/// Resolve one ACP permission request from an owner-signed encrypted control. +/// +/// Only per-request `allow_once` and `reject_once` decisions are accepted. +/// Channel, turn, and JSON-RPC request ids must all match the live task. +fn handle_permission_decision_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + observer: Option<&observer::ObserverHandle>, +) { + let Some(channel_id) = payload + .get("channelId") + .and_then(|value| value.as_str()) + .and_then(|value| value.parse::().ok()) + else { + tracing::warn!("permission decision control missing valid channelId"); + return; + }; + let Some(turn_id) = payload.get("turnId").and_then(|value| value.as_str()) else { + tracing::warn!("permission decision control missing turnId"); + return; + }; + let Some(request_id) = payload + .get("requestId") + .filter(|value| value.is_string() || value.is_number()) + .cloned() + else { + tracing::warn!("permission decision control missing valid requestId"); + return; + }; + let selection = match payload.get("optionId").and_then(|value| value.as_str()) { + Some(option_id) if !option_id.is_empty() && option_id.len() <= 512 => { + pool::PermissionSelection::OptionId(option_id.to_string()) + } + Some(_) => { + tracing::warn!("permission optionId must be a non-empty string of at most 512 bytes"); + return; + } + None => match payload.get("decision").and_then(|value| value.as_str()) { + Some("allow_once") => pool::PermissionSelection::Kind("allow_once".to_string()), + Some("reject_once") => pool::PermissionSelection::Kind("reject_once".to_string()), + _ => { + tracing::warn!( + "permission decision must include optionId or a legacy allow_once/reject_once decision" + ); + return; + } + }, + }; + let selected_option_id = match &selection { + pool::PermissionSelection::OptionId(option_id) => Some(option_id.clone()), + pool::PermissionSelection::Kind(_) => None, + }; + let selected_kind = match &selection { + pool::PermissionSelection::Kind(kind) => Some(kind.clone()), + pool::PermissionSelection::OptionId(_) => None, + }; + + let status = match pool.send_permission_decision( + channel_id, + turn_id, + pool::PermissionDecision { + request_id, + selection, + }, + ) { + Ok(()) => "sent", + Err(pool::PermissionDecisionError::NoActiveTurn) => "no_active_turn", + Err(pool::PermissionDecisionError::StaleTurn) => "stale_turn", + Err(pool::PermissionDecisionError::Unavailable) => "unavailable", + }; + + if let Some(observer) = observer { + observer.emit( + "control_result", + None, + &observer::ObserverContext { + channel_id: Some(channel_id.to_string()), + session_id: None, + turn_id: Some(turn_id.to_string()), + started_at: None, + }, + serde_json::json!({ + "type": "permission_decision", + "status": status, + "optionId": selected_option_id, + "decision": selected_kind, + }), + ); + } +} + /// Handle a `cancel_turn` control frame: signal the in-flight task to cancel. fn handle_cancel_turn_control( payload: &serde_json::Value, @@ -1553,6 +1647,8 @@ async fn tokio_main() -> Result<()> { context_message_limit: config.context_message_limit, max_turns_per_session: config.max_turns_per_session, permission_mode: config.permission_mode, + auto_approve_permissions: config.auto_approve_permissions, + interactive_permissions: config.interactive_permissions, agent_keys: config.keys.clone(), agent_owner_pubkey: startup_owner .as_deref() @@ -1627,6 +1723,13 @@ async fn tokio_main() -> Result<()> { // withheld event in `EventQueue::withheld_native_steer` until // `IN_FLIGHT_DEADLINE_SECS` expires. let (steer_ack_tx, mut steer_ack_rx) = mpsc::unbounded_channel::(); + // Runtime-scoped compatibility recovery for ACP adapters that publish a + // visible Buzz result but fail to finish `session/prompt`. One pending + // timer per turn bounds task/channel growth even if the agent publishes + // several messages during the grace window. + let (self_publish_completion_tx, mut self_publish_completion_rx) = + mpsc::unbounded_channel::<(Uuid, String, String)>(); + let mut pending_self_publish_completions = HashSet::::new(); // ── Step 7: Shutdown signal ─────────────────────────────────────────────── let (shutdown_tx, mut shutdown_rx) = watch::channel(()); @@ -1701,6 +1804,11 @@ async fn tokio_main() -> Result<()> { Result(Box), Panic(tokio::task::JoinError), SteerAck(SteerAckEvent), + SelfPublishCompletion { + channel_id: Uuid, + turn_id: String, + event_id: String, + }, Wake(u32, Result), } @@ -1844,6 +1952,15 @@ async fn tokio_main() -> Result<()> { Some(ack_event) = steer_ack_rx.recv() => { Some(PoolEvent::SteerAck(ack_event)) } + Some((channel_id, turn_id, event_id)) = self_publish_completion_rx.recv(), + if config.self_publish_completion_grace_secs > 0 => + { + Some(PoolEvent::SelfPublishCompletion { + channel_id, + turn_id, + event_id, + }) + } Some((attempt, result)) = wake_rx.recv(), if config.lazy_pool && !pool_ready => { Some(PoolEvent::Wake(attempt, result)) } @@ -2024,6 +2141,29 @@ async fn tokio_main() -> Result<()> { continue; } + if buzz_event.event.pubkey.to_hex() == pubkey_hex + && kind_u32 == KIND_STREAM_MESSAGE + && config.self_publish_completion_grace_secs > 0 + { + if let Some(turn_id) = + in_flight_turn_id(&pool, buzz_event.channel_id) + { + let turn_id = turn_id.to_owned(); + if pending_self_publish_completions.insert(turn_id.clone()) { + let tx = self_publish_completion_tx.clone(); + let channel_id = buzz_event.channel_id; + let event_id = buzz_event.event.id.to_hex(); + let grace = Duration::from_secs( + config.self_publish_completion_grace_secs, + ); + tokio::spawn(async move { + tokio::time::sleep(grace).await; + let _ = tx.send((channel_id, turn_id, event_id)); + }); + } + } + } + if config.ignore_self && buzz_event.event.pubkey.to_hex() == pubkey_hex { tracing::debug!(channel_id = %buzz_event.channel_id, "dropping self-authored event"); continue; @@ -2533,6 +2673,34 @@ async fn tokio_main() -> Result<()> { typing_channels.insert(channel_id, thread_tags); } } + Some(PoolEvent::SelfPublishCompletion { + channel_id, + turn_id, + event_id, + }) => { + pending_self_publish_completions.remove(&turn_id); + if signal_exact_in_flight_task( + &mut pool, + channel_id, + &turn_id, + ControlSignal::PublishedResult, + ) { + tracing::warn!( + channel = %channel_id, + turn_id, + event_id, + grace_secs = config.self_publish_completion_grace_secs, + "self-authored result was published but ACP turn remained open — completing exact turn" + ); + } else { + tracing::debug!( + channel = %channel_id, + turn_id, + event_id, + "self-publish completion grace elapsed after turn already finished" + ); + } + } Some(PoolEvent::Wake(attempt, result)) => { let completion = result.as_ref().map(|_| ()).map_err(|error| error.clone()); if let Err(error) = @@ -2728,6 +2896,14 @@ fn is_owner_control_command( // ── signal_in_flight_task ───────────────────────────────────────────────────── +/// Return the turn id currently checked out for `channel_id`. +fn in_flight_turn_id(pool: &AgentPool, channel_id: uuid::Uuid) -> Option<&str> { + pool.task_map() + .values() + .find(|meta| meta.channel_id == Some(channel_id)) + .map(|meta| meta.turn_id.as_str()) +} + /// Decide which [`ControlSignal`] (if any) to send to an in-flight turn when a /// new, already-author-gated event arrives for that channel. /// @@ -2776,6 +2952,36 @@ fn signal_in_flight_task( false } +/// Send a control signal only when both channel and turn id still match. +/// +/// Delayed compatibility timers use this stricter boundary so a stale timer +/// from a completed turn can never cancel later work in the same channel. +fn signal_exact_in_flight_task( + pool: &mut AgentPool, + channel_id: uuid::Uuid, + turn_id: &str, + mode: ControlSignal, +) -> bool { + let entry = pool + .task_map_mut() + .values_mut() + .find(|meta| meta.channel_id == Some(channel_id) && meta.turn_id == turn_id); + + if let Some(meta) = entry { + if let Some(tx) = meta.control_tx.take() { + tracing::info!( + channel = %channel_id, + turn_id, + ?mode, + "exact-turn control signal sent to in-flight task" + ); + let _ = tx.send(mode); + return true; + } + } + false +} + /// Attempt the non-cancelling (ACP) steer for a freshly-queued event. /// /// Caller invariants: @@ -2937,6 +3143,9 @@ fn dispatch_pending( let (tx, rx) = tokio::sync::mpsc::channel::(1); agent.acp.install_steer_rx(rx); let steer_tx = Some(tx); + let (permission_tx, permission_rx) = + tokio::sync::mpsc::channel::(4); + agent.acp.install_permission_rx(permission_rx); // Prompt text is now built inside run_prompt_task (needs async for // context fetching). Pass None for prompt_text; batch carries the data. @@ -2966,6 +3175,7 @@ fn dispatch_pending( recoverable_batch, control_tx: Some(control_tx), steer_tx, + permission_tx: Some(permission_tx), }, ); dispatched_channels.push((channel_id, typing_scope)); @@ -3579,6 +3789,7 @@ fn dispatch_heartbeat( recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); *heartbeat_in_flight = true; @@ -3742,6 +3953,11 @@ async fn initialize_agent_pool( startup: &PoolStartup, mut shutdown: Option>, ) -> Result { + let pool_started_at = std::time::Instant::now(); + tracing::info!( + agents = startup.agents, + "ACP agent pool initialization started" + ); // One agent failing to start must not kill the whole pool. // Attempt each spawn under a 60-second timeout; a partial pool is valid. let mut agent_slots: Vec> = Vec::with_capacity(startup.agents as usize); @@ -3836,7 +4052,11 @@ async fn initialize_agent_pool( startup.agents ); } - tracing::info!("agent_pool_ready agents={}", live_count); + tracing::info!( + agents = live_count, + elapsed_ms = pool_started_at.elapsed().as_millis() as u64, + "agent_pool_ready" + ); Ok(AgentPool::from_slots(agent_slots)) } @@ -4321,6 +4541,7 @@ mod owner_control_command_tests { recoverable_batch: None, control_tx: Some(control_tx), steer_tx: None, + permission_tx: None, }, ); @@ -4341,6 +4562,120 @@ mod owner_control_command_tests { ControlSignal::Rotate )); } + + #[tokio::test] + async fn self_publish_completion_signal_requires_the_exact_turn() { + let mut pool = AgentPool::from_slots(vec![]); + let channel_id = Uuid::new_v4(); + let (control_tx, control_rx) = tokio::sync::oneshot::channel(); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "current-turn".to_string(), + recoverable_batch: None, + control_tx: Some(control_tx), + steer_tx: None, + permission_tx: None, + }, + ); + + assert_eq!(in_flight_turn_id(&pool, channel_id), Some("current-turn")); + assert!(!signal_exact_in_flight_task( + &mut pool, + channel_id, + "stale-turn", + ControlSignal::PublishedResult, + )); + assert!(signal_exact_in_flight_task( + &mut pool, + channel_id, + "current-turn", + ControlSignal::PublishedResult, + )); + assert_eq!(control_rx.await.unwrap(), ControlSignal::PublishedResult); + } + + #[tokio::test] + async fn permission_control_forwards_exact_option_and_preserves_numeric_request_id() { + let mut pool = AgentPool::from_slots(vec![None]); + let channel_id = Uuid::new_v4(); + let (permission_tx, mut permission_rx) = + tokio::sync::mpsc::channel::(1); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "permission-turn".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + permission_tx: Some(permission_tx), + }, + ); + + handle_permission_decision_control( + &serde_json::json!({ + "type": "permission_decision", + "channelId": channel_id, + "turnId": "permission-turn", + "requestId": 42, + "optionId": "allow-buzz-messages-in-workspace", + }), + &mut pool, + None, + ); + let decision = permission_rx + .recv() + .await + .expect("exact permission selection should be delivered"); + assert_eq!(decision.request_id, serde_json::json!(42)); + assert_eq!( + decision.selection, + pool::PermissionSelection::OptionId("allow-buzz-messages-in-workspace".to_string()) + ); + } + + #[tokio::test] + async fn permission_control_rejects_invalid_option_ids() { + let mut pool = AgentPool::from_slots(vec![None]); + let channel_id = Uuid::new_v4(); + let (permission_tx, mut permission_rx) = + tokio::sync::mpsc::channel::(1); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "permission-turn".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + permission_tx: Some(permission_tx), + }, + ); + + handle_permission_decision_control( + &serde_json::json!({ + "type": "permission_decision", + "channelId": channel_id, + "turnId": "permission-turn", + "requestId": 42, + "optionId": "", + }), + &mut pool, + None, + ); + assert!(matches!( + permission_rx.try_recv(), + Err(tokio::sync::mpsc::error::TryRecvError::Empty) + )); + } } #[cfg(test)] @@ -4963,6 +5298,7 @@ mod build_mcp_servers_tests { dedup_mode: config::DedupMode::Queue, multiple_event_handling: config::MultipleEventHandling::Queue, ignore_self: true, + self_publish_completion_grace_secs: 0, kinds_override: None, channels_override: None, no_mention_filter: false, @@ -4974,6 +5310,8 @@ mod build_mcp_servers_tests { memory_enabled: false, model: None, permission_mode: config::PermissionMode::BypassPermissions, + auto_approve_permissions: true, + interactive_permissions: false, respond_to: config::RespondTo::Anyone, respond_to_allowlist: std::collections::HashSet::new(), allowed_respond_to: vec![], @@ -5129,6 +5467,7 @@ mod error_outcome_emission_tests { dedup_mode: config::DedupMode::Queue, multiple_event_handling: config::MultipleEventHandling::Queue, ignore_self: true, + self_publish_completion_grace_secs: 0, kinds_override: None, channels_override: None, no_mention_filter: false, @@ -5140,6 +5479,8 @@ mod error_outcome_emission_tests { memory_enabled: false, model: None, permission_mode: config::PermissionMode::BypassPermissions, + auto_approve_permissions: true, + interactive_permissions: false, respond_to: config::RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: vec![], @@ -5210,6 +5551,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); @@ -5286,6 +5628,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); started_rx.await.unwrap(); @@ -5378,6 +5721,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5469,6 +5813,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5574,6 +5919,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5650,6 +5996,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5744,6 +6091,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let config = test_config(); @@ -5860,6 +6208,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5999,6 +6348,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6187,6 +6537,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6272,6 +6623,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index cc537f8683..b60c127dff 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -65,6 +65,35 @@ pub struct TaskMeta { /// tasks only — all prompt tasks install a steer channel regardless /// of the agent's name. pub steer_tx: Option>, + /// Owner-approved permission decisions for the in-flight turn. + /// + /// Decisions are matched to both `turn_id` and the ACP JSON-RPC request id + /// before the read loop may select `allow_once`. + pub permission_tx: Option>, +} + +/// One owner selection for an ACP `session/request_permission` request. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PermissionDecision { + pub request_id: serde_json::Value, + pub selection: PermissionSelection, +} + +/// How an owner-selected ACP permission option is identified. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PermissionSelection { + /// Exact `optionId` copied from the current permission request. + OptionId(String), + /// Backward-compatible selection by ACP option kind. + Kind(String), +} + +/// Failure to deliver an interactive permission decision to a live turn. +#[derive(Debug, PartialEq, Eq)] +pub enum PermissionDecisionError { + NoActiveTurn, + StaleTurn, + Unavailable, } /// Agent-level model capabilities. Populated on first session creation. @@ -263,6 +292,11 @@ fn apply_completed_before_control_signal( pub enum ControlSignal { /// Stop the current turn and drop its triggering batch. Cancel, + /// The agent already published an externally visible channel result, but + /// its ACP adapter did not finish `session/prompt` within the configured + /// compatibility grace. Stop the exact publishing turn, drop its already + /// satisfied batch, and report successful completion. + PublishedResult, /// Stop the current turn and requeue its triggering batch for a merged /// re-prompt framed as a **supersede**: the new request replaces the old. Interrupt, @@ -510,6 +544,10 @@ pub struct PromptContext { pub max_turns_per_session: u32, /// Permission mode to apply after session creation. `Default` = skip. pub permission_mode: PermissionMode, + /// Whether ACP permission requests may select `allow_once`. + pub auto_approve_permissions: bool, + /// Whether owner-signed controls may resolve ACP permission requests. + pub interactive_permissions: bool, /// Agent identity — used to derive the NIP-AE conversation key at /// session creation for core injection. pub agent_keys: nostr::Keys, @@ -661,6 +699,32 @@ impl AgentPool { .map_err(|e| SteerError::Transport(e.to_string())) } + /// Deliver an owner-approved permission decision to one exact in-flight + /// turn. Matching the turn id prevents a delayed control frame from + /// authorizing a later turn in the same channel. + pub fn send_permission_decision( + &mut self, + channel_id: Uuid, + turn_id: &str, + decision: PermissionDecision, + ) -> Result<(), PermissionDecisionError> { + let Some(meta) = self + .task_map + .values_mut() + .find(|meta| meta.channel_id == Some(channel_id)) + else { + return Err(PermissionDecisionError::NoActiveTurn); + }; + if meta.turn_id != turn_id { + return Err(PermissionDecisionError::StaleTurn); + } + let Some(tx) = meta.permission_tx.as_ref() else { + return Err(PermissionDecisionError::Unavailable); + }; + tx.try_send(decision) + .map_err(|_| PermissionDecisionError::Unavailable) + } + pub fn result_tx(&self) -> mpsc::UnboundedSender { self.result_tx.clone() } @@ -824,7 +888,6 @@ async fn create_session_and_apply_model( ), agent_canvas, ); - let resp = agent .acp .session_new_full( @@ -1241,6 +1304,7 @@ fn send_prompt_result( batch: Option, ) { agent.acp.clear_steer_rx(); + agent.acp.clear_permission_rx(); let _ = result_tx.send(PromptResult { agent, source, @@ -1271,6 +1335,13 @@ pub async fn run_prompt_task( control_rx: Option>, turn_id: String, ) { + agent + .acp + .set_auto_approve_permissions(ctx.auto_approve_permissions); + agent + .acp + .set_interactive_permissions(ctx.interactive_permissions); + // Is this a channel prompt or a heartbeat? let source = match &batch { Some(b) => PromptSource::Channel(b.channel_id), @@ -1576,7 +1647,6 @@ pub async fn run_prompt_task( "isNewSession": is_new_session, }), ); - if is_new_session { if let (PromptSource::Channel(cid), Some(ref initial_msg)) = (&source, &ctx.initial_message) { @@ -1819,7 +1889,6 @@ pub async fn run_prompt_task( .collect(), None => prompt_sections.iter().map(String::as_str).collect(), }; - // When control_rx is Some (channel tasks), wrap the prompt in select! so // the main loop can cancel, interrupt, or rotate it. Heartbeats // (control_rx=None) take the simple await path — they are not controllable. @@ -1848,6 +1917,8 @@ pub async fn run_prompt_task( ) => result, mode = rx => { let control_signal = mode.unwrap_or(ControlSignal::Cancel); + let published_result = + matches!(control_signal, ControlSignal::PublishedResult); // Land the model switch before any cancel/requeue work: setting // `desired_model` here means the fresh session created by the // requeued turn (busy) or the next turn (already-completed) @@ -1878,7 +1949,11 @@ pub async fn run_prompt_task( observer_channel_id, &session_id, &turn_id, - Some(buzz_core::agent_turn_metric::StopReason::Cancelled), + Some(if published_result { + buzz_core::agent_turn_metric::StopReason::EndTurn + } else { + buzz_core::agent_turn_metric::StopReason::Cancelled + }), ) .await; send_prompt_result( @@ -1886,7 +1961,11 @@ pub async fn run_prompt_task( &turn_id, agent, source, - PromptOutcome::Cancelled, + if published_result { + PromptOutcome::Ok(StopReason::EndTurn) + } else { + PromptOutcome::Cancelled + }, retry_batch, ); return; @@ -2991,8 +3070,12 @@ fn requeue_cancelled_batch( let reason = match signal { ControlSignal::Steer => CancelReason::Steer, ControlSignal::Interrupt | ControlSignal::SwitchModel(_) => CancelReason::Interrupt, - // Cancel/Rotate discard the batch — no merged re-prompt. - ControlSignal::Cancel | ControlSignal::Rotate => return None, + // Cancel/Rotate discard the batch — no merged re-prompt. PublishedResult + // also discards it because the agent's self-authored channel event is + // evidence that the triggering work already produced its visible result. + ControlSignal::Cancel | ControlSignal::Rotate | ControlSignal::PublishedResult => { + return None; + } }; requeue_batch_if_queue(ctx, batch).map(|mut b| { b.cancel_reason = Some(reason); @@ -3653,6 +3736,59 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; use serde_json::json; + #[tokio::test] + async fn permission_decision_requires_exact_turn_and_delivers_once() { + let channel_id = Uuid::new_v4(); + let mut pool = AgentPool::from_slots(vec![None]); + let task_id = pool.join_set.spawn(async {}).id(); + let (permission_tx, mut permission_rx) = + tokio::sync::mpsc::channel::(1); + pool.task_map_mut().insert( + task_id, + TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "turn-current".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + permission_tx: Some(permission_tx), + }, + ); + + assert_eq!( + pool.send_permission_decision( + channel_id, + "turn-stale", + PermissionDecision { + request_id: json!("request-1"), + selection: PermissionSelection::Kind("allow_once".to_string()), + }, + ), + Err(PermissionDecisionError::StaleTurn) + ); + + pool.send_permission_decision( + channel_id, + "turn-current", + PermissionDecision { + request_id: json!("request-1"), + selection: PermissionSelection::OptionId("allow-workspace".to_string()), + }, + ) + .expect("matching owner decision should be delivered"); + + let delivered = permission_rx + .recv() + .await + .expect("permission decision should arrive"); + assert_eq!(delivered.request_id, json!("request-1")); + assert_eq!( + delivered.selection, + PermissionSelection::OptionId("allow-workspace".to_string()) + ); + } + // These pin the initial_message dispatch path (run_prompt_task, ~line 855): // a legacy agent WITH a base_prompt must get [Base] prepended to the user // message. This is the exact regression that shipped in the round-2 bug. @@ -4466,6 +4602,7 @@ mod tests { ), (ControlSignal::Cancel, None), (ControlSignal::Rotate, None), + (ControlSignal::PublishedResult, None), ]; let mut ctx = make_prompt_context_no_owner(); ctx.dedup_mode = DedupMode::Queue; @@ -4556,6 +4693,15 @@ mod tests { expected_reason: None, invalidate_all: false, }, + Case { + name: "CancelDrainTimeout + PublishedResult drops the satisfied batch", + error: || AcpError::CancelDrainTimeout(CONTROL_CANCEL_GRACE), + signal: ControlSignal::PublishedResult, + expected_outcome: "CancelDrainTimeout", + batch_preserved: false, + expected_reason: None, + invalidate_all: false, + }, Case { name: "CancelDrainTimeout + Interrupt preserves batch with Interrupt reason", error: || AcpError::CancelDrainTimeout(CONTROL_CANCEL_GRACE), @@ -5302,6 +5448,8 @@ mod tests { context_message_limit: 0, max_turns_per_session: 0, permission_mode: PermissionMode::Default, + auto_approve_permissions: true, + interactive_permissions: false, agent_keys: agent_keys.clone(), agent_owner_pubkey: owner_pubkey, memory_enabled: false, diff --git a/crates/buzz-acp/src/queue.rs b/crates/buzz-acp/src/queue.rs index 029bf86dbf..f2026002b3 100644 --- a/crates/buzz-acp/src/queue.rs +++ b/crates/buzz-acp/src/queue.rs @@ -1171,6 +1171,21 @@ fn append_new_thread_reply_instruction(s: &mut String, event_id: &str) { )); } +/// Append the destination instruction for an ordinary top-level DM response. +/// +/// Omitting a reply anchor is not enough for every ACP runtime: an agent may +/// still choose to thread a conversational response. State the main-timeline +/// destination explicitly while preserving an explicit human request to open a +/// thread. +fn append_top_level_dm_reply_instruction(s: &mut String) { + s.push_str( + "\nIMPORTANT: This is a top-level DM message. For ordinary replies in \ + this turn, use `buzz messages send` without `--reply-to` so the answer \ + appears directly in the DM's main timeline. Only use `--reply-to` if \ + the human explicitly asks for a threaded response.", + ); +} + /// Decide whether a turn is human-facing for reply-anchor purposes. /// /// A turn is human-facing when the triggering sender is a human, OR a human @@ -1275,6 +1290,8 @@ fn format_context_hints( if let Some(event_id) = reply_anchor { append_reply_instruction(&mut s, event_id); } + } else { + append_top_level_dm_reply_instruction(&mut s); } s } else if let Some(ref root) = thread_tags.root_event_id { @@ -1463,7 +1480,8 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec(null); + + const decide = React.useCallback( + async (optionId: string) => { + setSubmitting(optionId); + try { + await resolveManagedAgentPermission( + agentPubkey, + channelId, + turnId, + requestId, + optionId, + ); + } catch { + setSubmitting(null); + toast.error("Couldn’t send the permission decision. Try again."); + } + }, + [agentPubkey, channelId, requestId, turnId], + ); + + const oneShotOptions = oneShotPermissionOptions(options); + const allowOnce = oneShotOptions.find( + (option) => option.kind === "allow_once", + ); + const rejectOnce = oneShotOptions.find( + (option) => option.kind === "reject_once", + ); + + return ( + <> +
+
+ Permission decision + {allowOnce ? ( + + ) : null} + {rejectOnce ? ( + + ) : null} +
+ + ); +} + export function LifecycleActivity(props: ActivityRenderClassItemProps) { if (props.item.type === "tool") { return ; @@ -97,6 +179,17 @@ export function LifecycleActivity(props: ActivityRenderClassItemProps) { {outcome}
+ ) : props.item.permissionRequestId != null && + props.item.channelId && + props.item.turnId && + props.item.permissionOptions?.length ? ( + ) : null}
); diff --git a/desktop/src/features/agents/ui/agentSessionPermission.ts b/desktop/src/features/agents/ui/agentSessionPermission.ts new file mode 100644 index 0000000000..5fc98c163c --- /dev/null +++ b/desktop/src/features/agents/ui/agentSessionPermission.ts @@ -0,0 +1,114 @@ +import type { + AgentActivityDescriptor, + AgentPermissionOption, +} from "./agentSessionTypes"; +import { asRecord, asString } from "./agentSessionUtils"; + +export type PermissionRequestDescription = { + title: string; + text: string; + optionNames: Map; + permissionOptions: AgentPermissionOption[]; + descriptor: AgentActivityDescriptor; +}; + +/** + * Interactive managed-runtime consent is intentionally one-shot. Persistent + * runtime choices stay visible in the transcript for auditability, but they + * are never returned as actionable controls. + */ +export function oneShotPermissionOptions( + options: AgentPermissionOption[], +): AgentPermissionOption[] { + return options.filter( + (option) => option.kind === "allow_once" || option.kind === "reject_once", + ); +} + +export function describePermissionRequest( + payload: Record, +): PermissionRequestDescription { + const params = asRecord(payload.params); + const toolCall = asRecord(params.toolCall); + const title = + asString(toolCall.title) ?? + asString(params.title) ?? + asString(params.message) ?? + asString(params.reason) ?? + "Permission requested"; + const toolCallId = + asString(toolCall.toolCallId) ?? + asString(toolCall.tool_call_id) ?? + asString(params.toolCallId) ?? + asString(params.tool_call_id); + const permissionOptions: AgentPermissionOption[] = Array.isArray( + params.options, + ) + ? params.options + .map((option) => { + const record = asRecord(option); + const optionId = asString(record.optionId); + const kind = asString(record.kind); + if (!optionId || !kind) return null; + return { + optionId, + kind, + name: asString(record.name) ?? kind, + }; + }) + .filter((option): option is AgentPermissionOption => option !== null) + : []; + const detail: string[] = []; + if (title !== "Permission requested") detail.push(title); + if (toolCallId) detail.push(`Tool call: ${toolCallId}`); + if (permissionOptions.length > 0) { + detail.push( + `Options: ${permissionOptions.map((option) => option.name).join(", ")}`, + ); + } + + const optionNames = new Map(); + for (const option of permissionOptions) { + optionNames.set(option.optionId, option.kind); + } + + return { + title, + text: detail.join("\n"), + optionNames, + permissionOptions, + descriptor: { + renderClass: "permission", + label: "Permission requested", + preview: title, + action: { verb: "Requested", object: title }, + tone: "admin", + operation: "session/request_permission", + object: title, + source: "acp", + groupKey: "permission:request", + }, + }; +} + +/** + * Format a human-readable outcome label from a permission response. + * kind values from ACP: allow_once, allow_always, reject_once, reject_always. + * "reject_*" kinds are denials; anything else that is selected is an approval. + */ +export function describePermissionOutcome( + outcome: string, + optionId: string | null, + optionNames: Map, +): string { + if (outcome === "cancelled") { + return "Cancelled"; + } + if (outcome === "selected" && optionId) { + const kind = optionNames.get(optionId) ?? optionId; + const isDenial = kind.startsWith("reject"); + const verb = isDenial ? "Denied" : "Approved"; + return `${verb} (${kind})`; + } + return outcome; +} diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs index cc6f0467d6..0d17629bc8 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs @@ -6,6 +6,7 @@ import { buildTranscriptDisplayBlocks, flattenDisplayBlocks, } from "./agentSessionTranscriptGrouping.ts"; +import { oneShotPermissionOptions } from "./agentSessionPermission.ts"; import { formatToolTitle } from "./agentSessionToolCatalog.ts"; const baseEvent = { @@ -572,12 +573,20 @@ test("buildTranscript surfaces session/request_permission as a permission lifecy turnId: "turn-1", payload: { jsonrpc: "2.0", + id: 41, method: "session/request_permission", params: { - toolCallId: "tool-1", - title: "Confirm force-with-lease push to block/buzz.", + toolCall: { + toolCallId: "tool-1", + title: "Confirm force-with-lease push to block/buzz.", + }, options: [ { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + { + optionId: "allow_workspace", + kind: "allow_always", + name: "Always allow in this workspace", + }, { optionId: "reject_once", kind: "reject_once", name: "Reject" }, ], }, @@ -590,6 +599,41 @@ test("buildTranscript surfaces session/request_permission as a permission lifecy assert.equal(transcript[0].renderClass, "permission"); assert.equal(transcript[0].title, "Permission requested"); assert.match(transcript[0].text, /Confirm force-with-lease push/); + assert.equal(transcript[0].permissionRequestId, 41); + assert.deepEqual(transcript[0].permissionOptions, [ + { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + { + optionId: "allow_workspace", + kind: "allow_always", + name: "Always allow in this workspace", + }, + { optionId: "reject_once", kind: "reject_once", name: "Reject" }, + ]); + assert.equal(transcript[0].channelId, "channel-1"); + assert.equal(transcript[0].turnId, "turn-1"); +}); + +test("interactive permission actions exclude persistent runtime choices", () => { + assert.deepEqual( + oneShotPermissionOptions([ + { optionId: "allow_once", kind: "allow_once", name: "Allow once" }, + { + optionId: "allow_workspace", + kind: "allow_always", + name: "Always allow in this workspace", + }, + { + optionId: "reject_always", + kind: "reject_always", + name: "Always reject", + }, + { optionId: "reject_once", kind: "reject_once", name: "Deny once" }, + ]), + [ + { optionId: "allow_once", kind: "allow_once", name: "Allow once" }, + { optionId: "reject_once", kind: "reject_once", name: "Deny once" }, + ], + ); }); test("buildTranscript stamps completedAt when a terminal tool update is inserted first", () => { @@ -821,6 +865,22 @@ test("buildTranscript no-ops on a permission response with an unmatched id", () assert.doesNotMatch(item.text ?? "", /Denied/); }); +test("buildTranscript keeps multiple permission requests in one turn separate", () => { + const transcript = buildTranscript([ + makePermissionRequest(1, "req-first"), + makePermissionResponse(2, "req-first", "selected", "allow_once"), + makePermissionRequest(3, "req-second"), + makePermissionResponse(4, "req-second", "selected", "reject_once"), + ]); + + assert.equal(transcript.length, 2); + assert.notEqual(transcript[0].id, transcript[1].id); + assert.equal(transcript[0].type, "lifecycle"); + assert.equal(transcript[0].outcome, "Approved (allow_once)"); + assert.equal(transcript[1].type, "lifecycle"); + assert.equal(transcript[1].outcome, "Denied (reject_once)"); +}); + test("buildTranscript appends Approved outcome for a numeric JSON-RPC id (selected allow_once)", () => { // JSON-RPC 2.0 allows numeric ids; the ACP runtime preserves them as // serde_json::Value. asString() drops numbers, so this exercises the diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index 962290c6ca..c24fe61379 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -1,6 +1,7 @@ import type { AgentActivityDescriptor, AgentActivityRenderClass, + AgentPermissionOption, ObserverEvent, PromptSection, ToolStatus, @@ -12,6 +13,10 @@ import { normalizeToolStatus, } from "./agentSessionToolCatalog"; import { classifyTool } from "./agentSessionToolClassifier"; +import { + describePermissionOutcome, + describePermissionRequest, +} from "./agentSessionPermission"; import { asRecord, asString, titleCase } from "./agentSessionUtils"; import { describeTurnStarted, @@ -171,85 +176,6 @@ function stringifyPayload(value: unknown) { } } -function describePermissionRequest(payload: Record) { - const params = asRecord(payload.params); - const title = - asString(params.title) ?? - asString(params.message) ?? - asString(params.reason) ?? - "Permission requested"; - const toolCallId = - asString(params.toolCallId) ?? asString(params.tool_call_id); - const options = Array.isArray(params.options) - ? params.options - .map((option) => { - const record = asRecord(option); - return ( - asString(record.name) ?? - asString(record.kind) ?? - asString(record.optionId) - ); - }) - .filter((option): option is string => Boolean(option)) - : []; - const detail: string[] = []; - if (title !== "Permission requested") detail.push(title); - if (toolCallId) detail.push(`Tool call: ${toolCallId}`); - if (options.length > 0) detail.push(`Options: ${options.join(", ")}`); - - // Build optionId → kind map for outcome labeling on the response. - const optionNames = new Map(); - if (Array.isArray(params.options)) { - for (const option of params.options) { - const record = asRecord(option); - const optionId = asString(record.optionId); - const kind = asString(record.kind); - if (optionId && kind) { - optionNames.set(optionId, kind); - } - } - } - - return { - title, - text: detail.join("\n"), - optionNames, - descriptor: { - renderClass: "permission" as const, - label: "Permission requested", - preview: title, - action: { verb: "Requested", object: title }, - tone: "admin" as const, - operation: "session/request_permission", - object: title, - source: "acp" as const, - groupKey: "permission:request", - }, - }; -} - -/** - * Format a human-readable outcome label from a permission response. - * kind values from ACP: allow_once, allow_always, reject_once, reject_always. - * "reject_*" kinds are denials; anything else that is selected is an approval. - */ -function describePermissionOutcome( - outcome: string, - optionId: string | null, - optionNames: Map, -): string { - if (outcome === "cancelled") { - return "Cancelled"; - } - if (outcome === "selected" && optionId) { - const kind = optionNames.get(optionId) ?? optionId; - const isDenial = kind.startsWith("reject"); - const verb = isDenial ? "Denied" : "Approved"; - return `${verb} (${kind})`; - } - return outcome; -} - /** * Stable map key for a JSON-RPC id, which may be a string or a finite number * per the spec. Using JSON.stringify avoids collisions between the number 1 and @@ -263,6 +189,12 @@ function jsonRpcId(value: unknown): string | null { return null; } +function jsonRpcIdValue(value: unknown): string | number | null { + if (typeof value === "string") return value; + if (typeof value === "number" && Number.isFinite(value)) return value; + return null; +} + function describeFreeformStatus(payload: Record) { const statusType = asString(payload.type) ?? asString(payload.status); const title = @@ -408,6 +340,8 @@ function upsertLifecycleItem( ctx: TranscriptItemContext, acpSource?: string, descriptor?: AgentActivityDescriptor, + permissionRequestId?: string | number, + permissionOptions?: AgentPermissionOption[], ) { const existing = d.itemsById.get(id); if (existing?.type === "lifecycle") { @@ -417,6 +351,8 @@ function upsertLifecycleItem( title, text: joinLifecycleText(existing.text, text), descriptor: descriptor ?? existing.descriptor, + permissionRequestId: permissionRequestId ?? existing.permissionRequestId, + permissionOptions: permissionOptions ?? existing.permissionOptions, channelId: ctx.channelId, turnId: ctx.turnId ?? existing.turnId, sessionId: ctx.sessionId ?? existing.sessionId, @@ -434,6 +370,8 @@ function upsertLifecycleItem( text, timestamp, descriptor, + permissionRequestId, + permissionOptions, channelId: ctx.channelId, turnId: ctx.turnId, sessionId: ctx.sessionId, @@ -792,7 +730,10 @@ export function processTranscriptEvent( if (method === "session/request_permission") { const request = describePermissionRequest(payload); - const itemId = `permission:${ch}:${event.turnId ?? event.seq}`; + const requestId = jsonRpcId(payload.id); + const itemId = `permission:${ch}:${event.turnId ?? event.seq}:${ + requestId ?? event.seq + }`; upsertLifecycleItem( d, itemId, @@ -803,10 +744,11 @@ export function processTranscriptEvent( ctx, "permission_request", request.descriptor, + jsonRpcIdValue(payload.id) ?? undefined, + request.permissionOptions, ); // Index by JSON-RPC id so the response (acp_write with result.outcome, // no method) can correlate by id rather than by turn/seq. - const requestId = jsonRpcId(payload.id); if (requestId) { d.pendingPermissions = new Map(d.pendingPermissions); d.pendingPermissions.set(requestId, { diff --git a/desktop/src/features/agents/ui/agentSessionTypes.ts b/desktop/src/features/agents/ui/agentSessionTypes.ts index 578f98076c..47bcb689ed 100644 --- a/desktop/src/features/agents/ui/agentSessionTypes.ts +++ b/desktop/src/features/agents/ui/agentSessionTypes.ts @@ -68,6 +68,12 @@ export type TranscriptItemIdentity = { channelId?: string | null; }; +export type AgentPermissionOption = { + optionId: string; + name: string; + kind: string; +}; + export type TranscriptItem = | ({ id: string; @@ -109,6 +115,10 @@ export type TranscriptItem = text: string; /** Resolved outcome for permission items (e.g. "Approved (allow_once)", "Denied (reject_once)", "Cancelled"). */ outcome?: string; + /** Raw ACP JSON-RPC id used for an exact interactive decision match. */ + permissionRequestId?: string | number; + /** Exact options offered by the runtime for this live request. */ + permissionOptions?: AgentPermissionOption[]; timestamp: string; descriptor?: AgentActivityDescriptor; acpSource?: TranscriptAcpSource; diff --git a/desktop/src/features/channels/ui/BotActivityBar.tsx b/desktop/src/features/channels/ui/BotActivityBar.tsx index 21ab04f8d6..cb22f40673 100644 --- a/desktop/src/features/channels/ui/BotActivityBar.tsx +++ b/desktop/src/features/channels/ui/BotActivityBar.tsx @@ -57,6 +57,24 @@ export function BotActivityComposerAction({ Boolean(singleWorkingAgent), singleWorkingAgent?.pubkey, ); + const pendingPermission = React.useMemo(() => { + if (!singleWorkingAgent) return null; + const scoped = channelId + ? transcript.filter((item) => item.channelId === channelId) + : transcript; + for (let index = scoped.length - 1; index >= 0; index--) { + const item = scoped[index]; + if ( + item?.type === "lifecycle" && + item.renderClass === "permission" && + !item.outcome && + item.permissionRequestId != null + ) { + return item; + } + } + return null; + }, [channelId, singleWorkingAgent, transcript]); const activityHeadlines = React.useMemo(() => { if (!singleWorkingAgent) { return []; @@ -144,17 +162,21 @@ export function BotActivityComposerAction({ profiles?.[agent.pubkey.toLowerCase()]?.avatarUrl ?? null; const selectedPubkey = openAgentSessionPubkey?.toLowerCase() ?? null; const triggerLabel = - workingAgents.length === 1 - ? `${workingAgents[0]?.name ?? "Agent"} is working` - : `${workingAgents.length} agents working`; + pendingPermission && singleWorkingAgent + ? `${singleWorkingAgent.name} needs permission` + : workingAgents.length === 1 + ? `${workingAgents[0]?.name ?? "Agent"} is working` + : `${workingAgents.length} agents working`; const isInline = variant === "inline"; const visibleStatusLabel = - workingAgents.length === 1 - ? `${workingAgents[0]?.name ?? "Agent"}: ${ - activityHeadlines[headlineIndex % activityHeadlines.length] ?? - "Working" - }` - : `${workingAgents[0]?.name ?? "Agent"} +${workingAgents.length - 1}`; + pendingPermission && singleWorkingAgent + ? `${singleWorkingAgent.name}: Approval required` + : workingAgents.length === 1 + ? `${workingAgents[0]?.name ?? "Agent"}: ${ + activityHeadlines[headlineIndex % activityHeadlines.length] ?? + "Working" + }` + : `${workingAgents[0]?.name ?? "Agent"} +${workingAgents.length - 1}`; return ( @@ -166,11 +188,17 @@ export function BotActivityComposerAction({ isInline ? "h-7 min-w-0 gap-2 overflow-visible border-transparent bg-transparent px-0 text-xs font-semibold leading-none shadow-none hover:border-transparent hover:bg-transparent data-[state=open]:border-transparent data-[state=open]:bg-transparent" : "h-9 min-w-9 gap-1.5 px-2 text-xs", + pendingPermission ? "text-amber-700 dark:text-amber-400" : null, )} data-testid="bot-activity-composer-trigger" onBlur={closeWithDelay} onClick={() => { clearHoverTimer(); + if (pendingPermission && singleWorkingAgent) { + setOpen(false); + onOpenAgentSession(singleWorkingAgent.pubkey, channelId); + return; + } setOpen((current) => !current); }} onFocus={() => setOpen(true)} diff --git a/desktop/src/shared/api/agentControl.ts b/desktop/src/shared/api/agentControl.ts index 677f0ffad4..2e0cce4a8e 100644 --- a/desktop/src/shared/api/agentControl.ts +++ b/desktop/src/shared/api/agentControl.ts @@ -29,3 +29,24 @@ export async function switchManagedAgentModel( modelId, }); } + +/** + * Resolve one live ACP permission request. The harness accepts only + * owner-signed, encrypted controls that match the exact channel, turn, and + * JSON-RPC request id, then verifies that optionId belongs to that request. + */ +export async function resolveManagedAgentPermission( + pubkey: string, + channelId: string, + turnId: string, + requestId: string | number, + optionId: string, +): Promise { + await sendAgentObserverControl(pubkey, { + type: "permission_decision", + channelId, + turnId, + requestId, + optionId, + }); +} From 6e052a519084910351c6d506ecb56f75ce455ae5 Mon Sep 17 00:00:00 2001 From: Mark Fenner Date: Mon, 27 Jul 2026 15:13:12 -0400 Subject: [PATCH 3/7] fix(desktop): read the agent directory from managed-agent events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent discovery queried kind:10100, which is an agent's channel-add policy and carries none of the directory metadata the caller needs. The authoritative public projection of an agent's identity and inbound-author policy is kind:30177, so an allowlisted identity could never resolve the agent it had been granted access to. Project the directory from kind:30177 instead, taking the agent's public key from the parameterized replaceable event's `d` tag rather than from content — a content-supplied key is not authoritative. Build a narrow output object with only the fields the frontend consumes so later additions to the record cannot cross this boundary implicitly. Co-authored-by: Mark Fenner Signed-off-by: Mark Fenner (cherry picked from commit c3754cac7c20db599f824c7b8c2a4985b04931df) --- .../src-tauri/src/commands/agent_discovery.rs | 6 +- desktop/src-tauri/src/nostr_convert.rs | 110 ++++++++++++++++++ 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 5e597cdc32..75d0019027 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -1271,18 +1271,18 @@ pub async fn discover_managed_agent_prereqs( #[tauri::command] pub async fn list_relay_agents(state: State<'_, AppState>) -> Result, String> { - // Query kind:10100 agent profile events from the relay. + // Kind:30177 carries identity + inbound-author policy; 10100 is channel-add. let events = query_relay( &state, &[serde_json::json!({ - "kinds": [10100], + "kinds": [buzz_core_pkg::kind::KIND_MANAGED_AGENT], })], ) .await?; // The convert helper returns `{"agents": [...]}`. Extract and re-deserialize // into the strongly-typed `Vec` the frontend expects. - let value = nostr_convert::agents_from_events(&events); + let value = nostr_convert::managed_agents_from_events(&events); let agents = value .get("agents") .cloned() diff --git a/desktop/src-tauri/src/nostr_convert.rs b/desktop/src-tauri/src/nostr_convert.rs index ec4970e0c9..6f5dc78924 100644 --- a/desktop/src-tauri/src/nostr_convert.rs +++ b/desktop/src-tauri/src/nostr_convert.rs @@ -495,6 +495,69 @@ pub fn agents_from_events(events: &[Event]) -> Value { json!({ "agents": arr }) } +// ── kind:30177 (managed-agent directory) ─────────────────────────────────── + +/// Convert public managed-agent projections into the relay-agent directory +/// shape consumed by the desktop. +/// +/// The event author is the human owner. The managed agent's public key is the +/// parameterized replaceable event's `d` tag, so a content-supplied key is +/// never authoritative. Build a narrow output object instead of forwarding +/// the complete content projection so future fields cannot accidentally cross +/// this frontend boundary. +pub fn managed_agents_from_events(events: &[Event]) -> Value { + let arr: Vec = events + .iter() + .filter_map(|event| { + let agent_pubkey = nostr::PublicKey::from_hex(first_tag_value(event, "d")?).ok()?; + let pubkey = agent_pubkey.to_hex(); + let npub = agent_pubkey.to_bech32().unwrap_or_else(|_| pubkey.clone()); + let content: Value = serde_json::from_str(&event.content).ok()?; + let object = content.as_object()?; + let name = object + .get("name") + .and_then(Value::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .unwrap_or(&npub); + let respond_to = object + .get("respond_to") + .and_then(Value::as_str) + .filter(|mode| matches!(*mode, "owner-only" | "allowlist" | "anyone" | "nobody")); + + let mut respond_to_allowlist = Vec::new(); + if let Some(values) = object.get("respond_to_allowlist").and_then(Value::as_array) { + for value in values { + let Some(raw) = value.as_str() else { + continue; + }; + let Ok(key) = nostr::PublicKey::from_hex(raw.trim()) else { + continue; + }; + let normalized = key.to_hex(); + if !respond_to_allowlist.contains(&normalized) { + respond_to_allowlist.push(normalized); + } + } + } + + Some(json!({ + "pubkey": pubkey, + "name": name, + "agent_type": "agent", + "channels": [], + "channel_ids": [], + "capabilities": [], + "status": "offline", + "respond_to": respond_to, + "respond_to_allowlist": respond_to_allowlist, + })) + }) + .collect(); + + json!({ "agents": arr }) +} + // ── kind:13534 (relay membership list) ────────────────────────────────────── /// Convert a kind:13534 relay membership list to the relay members format. @@ -963,6 +1026,53 @@ mod tests { assert_eq!(parsed[0].respond_to_allowlist, vec!["a".repeat(64)]); } + #[test] + fn managed_agents_use_d_tag_identity_and_preserve_allowlist_metadata() { + let agent_pubkey = "02".repeat(32); + let allowed_pubkey = "03".repeat(32); + let e = ev( + 30177, + &format!( + r#"{{"pubkey":"forged","name":"Scout","parallelism":1,"respond_to":"allowlist","respond_to_allowlist":["{allowed_pubkey}"],"env_vars":{{"SECRET":"do-not-project"}}}}"# + ), + vec![vec!["d", &agent_pubkey]], + ); + let v = managed_agents_from_events(std::slice::from_ref(&e)); + let agents = v.get("agents").cloned().unwrap(); + let parsed: Vec = + serde_json::from_value(agents).unwrap(); + + assert_eq!(parsed.len(), 1); + assert_eq!(parsed[0].pubkey, agent_pubkey); + assert_eq!(parsed[0].name, "Scout"); + assert_eq!( + parsed[0].respond_to, + Some(crate::managed_agents::RespondTo::Allowlist) + ); + assert_eq!(parsed[0].respond_to_allowlist, vec![allowed_pubkey]); + assert!( + !v.to_string().contains("SECRET"), + "the directory projection must remain an explicit public-field allowlist" + ); + } + + #[test] + fn managed_agents_drop_events_without_a_valid_agent_d_tag() { + let missing = ev( + 30177, + r#"{"name":"Missing","parallelism":1,"respond_to":"owner-only"}"#, + vec![], + ); + let invalid = ev( + 30177, + r#"{"name":"Invalid","parallelism":1,"respond_to":"owner-only"}"#, + vec![vec!["d", "not-a-pubkey"]], + ); + let v = managed_agents_from_events(&[missing, invalid]); + + assert_eq!(v.get("agents").and_then(Value::as_array).unwrap().len(), 0); + } + #[test] fn relay_members_dedupes_and_defaults_role() { let pk1 = "a".repeat(64); From 68b05d27bbffbdd68d8e5491e48e8875e65d70a7 Mon Sep 17 00:00:00 2001 From: Mark Fenner Date: Mon, 27 Jul 2026 15:14:06 -0400 Subject: [PATCH 4/7] fix(desktop): edit the agent instance from its profile panel The profile panel is an instance view: it shows the agent's own public key, runtime state, and stop/restart controls. Its Edit action nonetheless opened the definition editor whenever the agent was definition-linked, so a change to "Who can talk to this agent" was written to the definition. A definition's behavior group is copied onto an instance only when a new instance is minted from it, and update_persona propagates only display_name and avatar_url to instances that already exist. The instance kept the policy it had. Because the inbound-author gate is built from the instance record at spawn, the dialog could show an allowlist that the running agent did not enforce. The failure is bidirectional and worse for revocation: an owner who removed an identity would believe access was revoked while the agent kept honouring the previous policy. Route an instance-backed profile to the instance editor via resolveProfileEditTarget, so the policy on display is the policy in force. Definition editing is unaffected: the agent library actions menu still opens it directly, and the instance editor keeps its linked-definition hop. Also make the instance editor submit its own respond-to state rather than diffing against the agent prop. The dialog stays mounted while managed-agent polling can replace that prop, so local form state is the only reliable snapshot of what the user is saving; Rust validates the merged policy and the retention layer suppresses unchanged relay publications. Co-authored-by: Mark Fenner Signed-off-by: Mark Fenner (cherry picked from commit da34d69f72d8f00876547a9970b4b3f7329b5d2f) --- .../agents/lib/agentRespondToUpdate.test.mjs | 32 ++++++++++++ .../agents/lib/agentRespondToUpdate.ts | 29 +++++++++++ .../agents/ui/AgentInstanceEditDialog.tsx | 18 +++---- .../features/profile/ui/UserProfilePanel.tsx | 11 +++- .../profile/ui/UserProfilePanelUtils.test.mjs | 51 +++++++++++++++++++ .../profile/ui/UserProfilePanelUtils.ts | 33 ++++++++++++ 6 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 desktop/src/features/agents/lib/agentRespondToUpdate.test.mjs create mode 100644 desktop/src/features/agents/lib/agentRespondToUpdate.ts diff --git a/desktop/src/features/agents/lib/agentRespondToUpdate.test.mjs b/desktop/src/features/agents/lib/agentRespondToUpdate.test.mjs new file mode 100644 index 0000000000..c320a2ef9e --- /dev/null +++ b/desktop/src/features/agents/lib/agentRespondToUpdate.test.mjs @@ -0,0 +1,32 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { buildAgentRespondToUpdate } from "./agentRespondToUpdate.ts"; + +const ALLOWED_PUBKEY = "c".repeat(64); + +test("instance edits always submit the form's respond-to mode", () => { + assert.deepEqual(buildAgentRespondToUpdate("owner-only", []), { + respondTo: "owner-only", + respondToAllowlist: undefined, + }); +}); + +test("allowlist edits always submit the complete form allowlist", () => { + const allowlist = [ALLOWED_PUBKEY]; + + const update = buildAgentRespondToUpdate("allowlist", allowlist); + + assert.deepEqual(update, { + respondTo: "allowlist", + respondToAllowlist: [ALLOWED_PUBKEY], + }); + assert.notEqual(update.respondToAllowlist, allowlist); +}); + +test("non-allowlist edits do not overwrite a preserved allowlist", () => { + assert.deepEqual(buildAgentRespondToUpdate("anyone", [ALLOWED_PUBKEY]), { + respondTo: "anyone", + respondToAllowlist: undefined, + }); +}); diff --git a/desktop/src/features/agents/lib/agentRespondToUpdate.ts b/desktop/src/features/agents/lib/agentRespondToUpdate.ts new file mode 100644 index 0000000000..fad21a7601 --- /dev/null +++ b/desktop/src/features/agents/lib/agentRespondToUpdate.ts @@ -0,0 +1,29 @@ +import type { + RespondToMode, + UpdateManagedAgentInput, +} from "@/shared/api/types"; + +type RespondToUpdate = Pick< + UpdateManagedAgentInput, + "respondTo" | "respondToAllowlist" +>; + +/** + * Build the authoritative inbound-author policy for an instance edit. + * + * The edit dialog stays mounted while managed-agent polling can replace its + * `agent` prop. Its local form state is therefore the only reliable snapshot + * of what the user is saving. Always send that state instead of diffing it + * against a possibly refreshed prop; Rust validates the merged policy and the + * retention layer suppresses unchanged relay publications. + */ +export function buildAgentRespondToUpdate( + respondTo: RespondToMode, + respondToAllowlist: string[], +): RespondToUpdate { + return { + respondTo, + respondToAllowlist: + respondTo === "allowlist" ? [...respondToAllowlist] : undefined, + }; +} diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 601d57f95d..694d1af10a 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -11,6 +11,7 @@ import { useStartManagedAgentMutation, useUpdateManagedAgentMutation, } from "@/features/agents/hooks"; +import { buildAgentRespondToUpdate } from "@/features/agents/lib/agentRespondToUpdate"; import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions"; import type { ManagedAgent, @@ -631,6 +632,10 @@ export function AgentInstanceEditDialog({ // all agree. See resolveInheritedRuntimeSubmission. const normalizedSubmitProvider = inheritedSubmission.provider; const submitEnvVars = inheritedSubmission.envVars; + const respondToUpdate = buildAgentRespondToUpdate( + respondTo, + respondToAllowlist, + ); const input: UpdateManagedAgentInput = { pubkey: agent.pubkey, name: name.trim() !== agent.name ? name.trim() : undefined, @@ -688,18 +693,7 @@ export function AgentInstanceEditDialog({ envVars: envVarsEqual(submitEnvVars, agent.envVars) ? undefined : submitEnvVars, - respondTo: respondTo !== agent.respondTo ? respondTo : undefined, - // The allowlist is preserved across mode toggles in local UI state - // (so a user can flip away from allowlist and back without losing - // their entries), but we only send it on the wire when (a) it - // actually changed, AND (b) the saved mode will need it. Sending - // an allowlist while switching to a non-allowlist mode would be - // harmless server-side, but it's noise in the persisted record. - respondToAllowlist: - respondTo === "allowlist" && - respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",") - ? respondToAllowlist - : undefined, + ...respondToUpdate, }; const result = await updateMutation.mutateAsync(input); diff --git a/desktop/src/features/profile/ui/UserProfilePanel.tsx b/desktop/src/features/profile/ui/UserProfilePanel.tsx index c1f713d230..4015ef3180 100644 --- a/desktop/src/features/profile/ui/UserProfilePanel.tsx +++ b/desktop/src/features/profile/ui/UserProfilePanel.tsx @@ -80,6 +80,7 @@ import { resolveAgentInstruction, resolvePanelProfile, resolveProfileDisplayName, + resolveProfileEditTarget, truncatePubkey, type UserProfilePanelProps, useRetainedPersona, @@ -398,12 +399,18 @@ export function UserProfilePanel({ }); const handleEditAgent = React.useCallback(() => { - if (resolvedPersona) { + // See resolveProfileEditTarget: an instance-backed profile must edit the + // instance, whose respond-to pair is the one enforced at spawn. + const target = resolveProfileEditTarget({ + hasManagedInstance: managedAgent !== undefined, + hasDefinition: resolvedPersona !== undefined, + }); + if (target === "definition" && resolvedPersona) { setPersonaDialogState(editPersonaDialogState(resolvedPersona)); return; } setEditAgentOpen(true); - }, [resolvedPersona]); + }, [managedAgent, resolvedPersona]); const { deleteManagedAgentRecord, deleteManagedAgentsForPersona } = useProfileAgentDeletion({ diff --git a/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs b/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs index 89837f6017..3cb9b8b0d7 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs +++ b/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs @@ -7,6 +7,7 @@ import { personaManagedAgentUpdate, profilePanelTabFromSearch, profilePanelViewFromSearch, + resolveProfileEditTarget, } from "./UserProfilePanelUtils.ts"; function agent(overrides = {}) { @@ -189,3 +190,53 @@ test("profilePanelTabFromSearch falls back to info for invalid values", () => { assert.equal(profilePanelTabFromSearch("missing"), "info"); assert.equal(profilePanelTabFromSearch(null), "info"); }); + +// ── Profile Edit routing: displayed policy must be the enforced policy ─────── +// +// Regression: a persona-linked agent routed Edit to the DEFINITION editor, so +// the dialog showed the definition's inbound-author policy while the running +// agent still enforced the instance's own policy. A definition's behavior +// group is copied onto an instance only at mint time, so an owner who granted +// (or revoked) access there changed nothing about the live agent. + +test("resolveProfileEditTarget: an instance-backed profile edits the instance", () => { + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: true, + hasDefinition: true, + }), + "instance", + "a persona-linked instance must still edit the instance it displays", + ); + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: true, + hasDefinition: false, + }), + "instance", + ); +}); + +test("resolveProfileEditTarget: a definition-only profile edits the definition", () => { + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: false, + hasDefinition: true, + }), + "definition", + "with no minted instance the definition is the only editable record", + ); +}); + +test("resolveProfileEditTarget: no instance and no definition falls back to instance", () => { + // Preserves the pre-existing fallback: the caller renders the instance + // dialog only when a managed agent exists, so this is inert rather than a + // route into a dialog that cannot edit anything. + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: false, + hasDefinition: false, + }), + "instance", + ); +}); diff --git a/desktop/src/features/profile/ui/UserProfilePanelUtils.ts b/desktop/src/features/profile/ui/UserProfilePanelUtils.ts index 07f57803b4..951a0905f7 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelUtils.ts +++ b/desktop/src/features/profile/ui/UserProfilePanelUtils.ts @@ -259,6 +259,39 @@ export function resolveAgentInstruction( ); } +/** + * Decide which editor the profile panel's Edit action must open. + * + * The profile panel is an *instance* view: it shows this agent's own public + * key, runtime state, and Stop/Restart controls. So when a concrete managed + * instance exists, Edit has to open the instance editor — the instance + * record's `respond_to`/allowlist is the pair `build_respond_to_env` turns + * into `BUZZ_ACP_RESPOND_TO` at spawn, and it is therefore the only policy the + * running agent actually enforces. + * + * Routing an instance-backed profile to the definition editor instead lets the + * dialog display an inbound-author policy that the live agent does not apply: + * a definition's behavior group is copied onto an instance only when a *new* + * instance is minted from it, never onto instances that already exist. An + * owner who added someone to an allowlist there would believe they had granted + * access — and, worse, an owner who removed someone would believe they had + * revoked it — while the running agent kept its original policy. + * + * Definition editing stays reachable: the agent library's actions menu opens + * the definition editor directly, and the instance editor offers a hop to the + * linked definition. + */ +export function resolveProfileEditTarget({ + hasManagedInstance, + hasDefinition, +}: { + hasManagedInstance: boolean; + hasDefinition: boolean; +}): "instance" | "definition" { + if (hasManagedInstance) return "instance"; + return hasDefinition ? "definition" : "instance"; +} + export function personaManagedAgentUpdate( agent: ManagedAgent, persona: AgentPersona, From af40e94206400b188676a4c42b9c277059cea451 Mon Sep 17 00:00:00 2001 From: Mark Fenner Date: Mon, 27 Jul 2026 15:14:21 -0400 Subject: [PATCH 5/7] fix(desktop): stop publishing a revoked allowlist The instance record retains its allowlist across mode toggles on purpose, so an owner can switch away from allowlist mode and back without retyping entries. That retention leaked onto the wire: agent_event_content published respond_to_allowlist unconditionally, so after revoking access the kind:30177 projection still advertised the removed pubkey next to "respond_to":"owner-only". Exclude the allowlist from the projection unless the mode is Allowlist. This matches the field's documented contract, the definition side already does the same in apply_persona_behavior for the same reason, and spawn agrees, since build_respond_to_env emits no allowlist variable for other modes. Access control was never affected. The stale entry granted nothing and did not restore the agent to the revoked identity's autocomplete, because relayAgentIsSharedWithUser gates on the mode before consulting membership. The defect was that a revoked association stayed publicly readable. Co-authored-by: Mark Fenner Signed-off-by: Mark Fenner (cherry picked from commit f49abf294c314454d0789621da15874a8f60da0c) --- .../src/managed_agents/agent_events.rs | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index ba4407d164..90b6ae66a0 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -102,7 +102,18 @@ pub fn agent_event_content(record: &ManagedAgentRecord) -> ManagedAgentEventCont }, parallelism: record.parallelism, respond_to: record.respond_to, - respond_to_allowlist: record.respond_to_allowlist.clone(), + // The allowlist only means something in allowlist mode. The instance + // record deliberately retains it across mode toggles so an owner can + // flip away and back without retyping entries, but publishing it under + // another mode would advertise pubkeys the owner has already revoked — + // the same reason `apply_persona_behavior` clears it on the definition + // side. Spawn agrees: `build_respond_to_env` emits no allowlist + // variable unless the mode is Allowlist. + respond_to_allowlist: if record.respond_to == RespondTo::Allowlist { + record.respond_to_allowlist.clone() + } else { + Vec::new() + }, } } @@ -336,6 +347,44 @@ mod tests { assert_eq!(a, b); } + /// Revoking an allowlist by switching modes must not keep advertising the + /// revoked pubkeys. The record retains them on purpose (so the owner can + /// toggle back without retyping), so the projection is what has to drop + /// them — otherwise a revoked association stays publicly readable. + #[test] + fn projection_omits_allowlist_for_non_allowlist_modes() { + let mut agent = sample_agent(); + agent.respond_to = RespondTo::Allowlist; + agent.respond_to_allowlist = vec!["a".repeat(64)]; + assert_eq!( + agent_event_content(&agent).respond_to_allowlist, + vec!["a".repeat(64)], + "allowlist mode must still publish its entries" + ); + + // `nobody` is intentionally absent from this enum (harness-only). + for mode in [RespondTo::OwnerOnly, RespondTo::Anyone] { + let mut revoked = agent.clone(); + revoked.respond_to = mode; + let content = agent_event_content(&revoked); + assert!( + content.respond_to_allowlist.is_empty(), + "{mode:?} must not publish a retained allowlist" + ); + assert!( + !serde_json::to_string(&content) + .unwrap() + .contains(&"a".repeat(64)), + "{mode:?} projection must not carry the revoked pubkey on the wire" + ); + assert_eq!( + revoked.respond_to_allowlist, + vec!["a".repeat(64)], + "the local record keeps its entries for mode round-tripping" + ); + } + } + /// Mutating only runtime fields must NOT change the projection — the /// guarantee that operational start/stop never republishes. #[test] From 667b5ec2c5bf33992f4ae3d1cf2fd493801acc84 Mon Sep 17 00:00:00 2001 From: Mark Fenner Date: Mon, 27 Jul 2026 15:14:39 -0400 Subject: [PATCH 6/7] fix(desktop): scrub inherited ACP_BACKEND from the Devin adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cognition's documentation frames Devin Desktop as an ACP host that launches agents and injects environment into them. ACP_BACKEND=windsurf is part of that host-side state: it tells a spawned "devin acp" that its host is Devin Desktop and therefore supplies credentials over ACP. When Buzz is the host that claim is false, and inheriting the variable — for instance when Buzz is launched from a terminal running inside Devin Desktop — makes the adapter announce that the ACP host is the sole source of credentials and refuse the stored CLI credentials. Every turn then fails with "-32000 ACP host has not authenticated" even though devin auth login succeeded. With the variable absent the same binary falls back to env vars and stored CLI credentials, and turns succeed. The leak also corrupted readiness: devin auth status reported "Not logged in" while contaminated and "Logged in (via Devin)" without it. Scrub it alongside WINDSURF_API_KEY. One entry covers all three consumers of that list — process launch, the login/readiness probe, and runtime discovery — so invocation and readiness are fixed together. Verified in a packaged build: the desktop process carried ACP_BACKEND while the Devin harness it spawned did not. Co-authored-by: Mark Fenner Signed-off-by: Mark Fenner (cherry picked from commit 277c6f698454f7fd38995dc5f74fac78f8bd8869) --- .../discovery/runtime_catalog.rs | 17 ++++++++++++---- .../discovery/runtime_metadata.rs | 2 +- .../managed_agents/discovery/tests/devin.rs | 9 ++++++++- .../src/managed_agents/runtime/env_policy.rs | 20 +++++++++++++++++++ .../src/managed_agents/runtime/tests/devin.rs | 2 +- 5 files changed, 43 insertions(+), 7 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs index c1804159f2..7416f125fb 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs @@ -212,10 +212,19 @@ pub(super) const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), ], - // The official CLI gives this legacy ambient key precedence over the - // account established by `devin auth login`. Remove it without reading - // its value so readiness and usage attribution share one identity. - scrub_env_vars: &["WINDSURF_API_KEY"], + // WINDSURF_API_KEY: the official CLI gives this legacy ambient key + // precedence over the account established by `devin auth login`. Remove + // it without reading its value so readiness and usage attribution share + // one identity. + // + // ACP_BACKEND: set by the Devin IDE for its own ACP integration. When it + // leaks in — Buzz launched from a terminal inside that IDE, for example + // — `devin acp` switches to "ACP host is the sole source of + // credentials", refuses the stored CLI credentials, and fails every turn + // with "ACP host has not authenticated" even though `devin auth login` + // succeeded. Ambient state must not redefine the adapter's credential + // policy. + scrub_env_vars: &["WINDSURF_API_KEY", "ACP_BACKEND"], config_file_path: None, config_file_format: None, supports_acp_native_config: false, diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs index 9a6be59707..6b215da6d3 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs @@ -212,7 +212,7 @@ mod tests { ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), ] ); - assert_eq!(devin.scrub_env_vars, &["WINDSURF_API_KEY"]); + assert_eq!(devin.scrub_env_vars, &["WINDSURF_API_KEY", "ACP_BACKEND"]); assert_eq!( devin.cli_install_instructions_url, "https://docs.devin.ai/cli" diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs index 31b4664721..6530414a95 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs @@ -83,7 +83,14 @@ fn runtime_catalog_exposes_devin_once() { ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), ] ); - assert_eq!(devin.scrub_env_vars, &["WINDSURF_API_KEY"]); + // ACP_BACKEND must stay scrubbed: inherited from the Devin IDE it flips the + // adapter to host-supplied credentials only, and every turn then fails with + // "ACP host has not authenticated" despite a valid `devin auth login`. + assert_eq!( + devin.scrub_env_vars, + &["WINDSURF_API_KEY", "ACP_BACKEND"], + "ambient IDE state must not redefine Devin's credential policy" + ); assert!(!devin.supports_acp_model_switching); assert!(!devin.accepts_harness_model); } diff --git a/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs b/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs index b2768c5822..38000e2f9a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs @@ -104,6 +104,26 @@ mod tests { .any(|(key, value)| { key == "WINDSURF_API_KEY" && value.is_none() })); } + /// Regression: Buzz launched from a terminal inside the Devin IDE inherits + /// `ACP_BACKEND=windsurf`. Passing it through makes `devin acp` treat the + /// ACP host as the sole credential source, so it refuses the stored CLI + /// credentials and every turn fails with "ACP host has not authenticated" + /// even though `devin auth login` succeeded. + #[test] + fn devin_policy_scrubs_inherited_acp_backend() { + let mut command = std::process::Command::new("buzz-acp"); + command.env("ACP_BACKEND", "windsurf"); + + apply_runtime_env_policy(&mut command, known_acp_runtime("devin")); + + assert!( + command + .get_envs() + .any(|(key, value)| { key == "ACP_BACKEND" && value.is_none() }), + "ACP_BACKEND must be removed before spawning the Devin adapter" + ); + } + #[test] fn existing_runtime_policy_remains_unchanged() { let mut command = std::process::Command::new("buzz-acp"); diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs b/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs index 18c920de88..399057efe6 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs @@ -17,7 +17,7 @@ fn uses_native_acp_without_mcp_hooks() { ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), ] ); - assert_eq!(runtime.scrub_env_vars, &["WINDSURF_API_KEY"]); + assert_eq!(runtime.scrub_env_vars, &["WINDSURF_API_KEY", "ACP_BACKEND"]); assert!(!runtime.mcp_hooks); assert_eq!(runtime.mcp_command, None); } From 004eb4020a25e8465e97486ef192b646141ae3c1 Mon Sep 17 00:00:00 2001 From: Mark Fenner Date: Mon, 27 Jul 2026 16:59:34 -0400 Subject: [PATCH 7/7] fix(desktop): stop one unsupported respond-to mode hiding every agent The directory projection accepted "nobody" and forwarded it verbatim. RelayAgentInfo deserializes respond_to into RespondTo, which deliberately has no nobody variant because the desktop exposes no surface for it, so a single validly signed event carrying that mode failed the whole Vec and every other agent vanished from discovery. Project only the modes RespondTo can represent. An unrepresentable mode becomes absent, and every eligibility check treats absent as not-invocable, so the projection degrades closed rather than either crashing or over-granting. Co-authored-by: Mark Fenner Signed-off-by: Mark Fenner --- desktop/src-tauri/src/nostr_convert.rs | 113 +---------- .../src/nostr_convert/managed_agents.rs | 180 ++++++++++++++++++ 2 files changed, 183 insertions(+), 110 deletions(-) create mode 100644 desktop/src-tauri/src/nostr_convert/managed_agents.rs diff --git a/desktop/src-tauri/src/nostr_convert.rs b/desktop/src-tauri/src/nostr_convert.rs index 6f5dc78924..fe941ca708 100644 --- a/desktop/src-tauri/src/nostr_convert.rs +++ b/desktop/src-tauri/src/nostr_convert.rs @@ -22,6 +22,9 @@ pub use user_search::{ // ── Tag helpers ───────────────────────────────────────────────────────────── +mod managed_agents; +pub use managed_agents::managed_agents_from_events; + /// Find the first tag whose name matches `name` and return its first value. /// /// e.g. for tag `["name", "general"]` with `name="name"` returns `Some("general")`. @@ -495,69 +498,6 @@ pub fn agents_from_events(events: &[Event]) -> Value { json!({ "agents": arr }) } -// ── kind:30177 (managed-agent directory) ─────────────────────────────────── - -/// Convert public managed-agent projections into the relay-agent directory -/// shape consumed by the desktop. -/// -/// The event author is the human owner. The managed agent's public key is the -/// parameterized replaceable event's `d` tag, so a content-supplied key is -/// never authoritative. Build a narrow output object instead of forwarding -/// the complete content projection so future fields cannot accidentally cross -/// this frontend boundary. -pub fn managed_agents_from_events(events: &[Event]) -> Value { - let arr: Vec = events - .iter() - .filter_map(|event| { - let agent_pubkey = nostr::PublicKey::from_hex(first_tag_value(event, "d")?).ok()?; - let pubkey = agent_pubkey.to_hex(); - let npub = agent_pubkey.to_bech32().unwrap_or_else(|_| pubkey.clone()); - let content: Value = serde_json::from_str(&event.content).ok()?; - let object = content.as_object()?; - let name = object - .get("name") - .and_then(Value::as_str) - .map(str::trim) - .filter(|value| !value.is_empty()) - .unwrap_or(&npub); - let respond_to = object - .get("respond_to") - .and_then(Value::as_str) - .filter(|mode| matches!(*mode, "owner-only" | "allowlist" | "anyone" | "nobody")); - - let mut respond_to_allowlist = Vec::new(); - if let Some(values) = object.get("respond_to_allowlist").and_then(Value::as_array) { - for value in values { - let Some(raw) = value.as_str() else { - continue; - }; - let Ok(key) = nostr::PublicKey::from_hex(raw.trim()) else { - continue; - }; - let normalized = key.to_hex(); - if !respond_to_allowlist.contains(&normalized) { - respond_to_allowlist.push(normalized); - } - } - } - - Some(json!({ - "pubkey": pubkey, - "name": name, - "agent_type": "agent", - "channels": [], - "channel_ids": [], - "capabilities": [], - "status": "offline", - "respond_to": respond_to, - "respond_to_allowlist": respond_to_allowlist, - })) - }) - .collect(); - - json!({ "agents": arr }) -} - // ── kind:13534 (relay membership list) ────────────────────────────────────── /// Convert a kind:13534 relay membership list to the relay members format. @@ -1026,53 +966,6 @@ mod tests { assert_eq!(parsed[0].respond_to_allowlist, vec!["a".repeat(64)]); } - #[test] - fn managed_agents_use_d_tag_identity_and_preserve_allowlist_metadata() { - let agent_pubkey = "02".repeat(32); - let allowed_pubkey = "03".repeat(32); - let e = ev( - 30177, - &format!( - r#"{{"pubkey":"forged","name":"Scout","parallelism":1,"respond_to":"allowlist","respond_to_allowlist":["{allowed_pubkey}"],"env_vars":{{"SECRET":"do-not-project"}}}}"# - ), - vec![vec!["d", &agent_pubkey]], - ); - let v = managed_agents_from_events(std::slice::from_ref(&e)); - let agents = v.get("agents").cloned().unwrap(); - let parsed: Vec = - serde_json::from_value(agents).unwrap(); - - assert_eq!(parsed.len(), 1); - assert_eq!(parsed[0].pubkey, agent_pubkey); - assert_eq!(parsed[0].name, "Scout"); - assert_eq!( - parsed[0].respond_to, - Some(crate::managed_agents::RespondTo::Allowlist) - ); - assert_eq!(parsed[0].respond_to_allowlist, vec![allowed_pubkey]); - assert!( - !v.to_string().contains("SECRET"), - "the directory projection must remain an explicit public-field allowlist" - ); - } - - #[test] - fn managed_agents_drop_events_without_a_valid_agent_d_tag() { - let missing = ev( - 30177, - r#"{"name":"Missing","parallelism":1,"respond_to":"owner-only"}"#, - vec![], - ); - let invalid = ev( - 30177, - r#"{"name":"Invalid","parallelism":1,"respond_to":"owner-only"}"#, - vec![vec!["d", "not-a-pubkey"]], - ); - let v = managed_agents_from_events(&[missing, invalid]); - - assert_eq!(v.get("agents").and_then(Value::as_array).unwrap().len(), 0); - } - #[test] fn relay_members_dedupes_and_defaults_role() { let pk1 = "a".repeat(64); diff --git a/desktop/src-tauri/src/nostr_convert/managed_agents.rs b/desktop/src-tauri/src/nostr_convert/managed_agents.rs new file mode 100644 index 0000000000..d9da5007e4 --- /dev/null +++ b/desktop/src-tauri/src/nostr_convert/managed_agents.rs @@ -0,0 +1,180 @@ +//! Managed-agent directory projection (kind:30177). +//! +//! Split out of `nostr_convert` so the generic event converters stay readable +//! and this authorization-facing projection has an obvious home. + +use nostr::{Event, ToBech32}; +use serde_json::{json, Value}; + +use super::first_tag_value; + +/// Convert public managed-agent projections into the relay-agent directory +/// shape consumed by the desktop. +/// +/// The event author is the human owner. The managed agent's public key is the +/// parameterized replaceable event's `d` tag, so a content-supplied key is +/// never authoritative. Build a narrow output object instead of forwarding +/// the complete content projection so future fields cannot accidentally cross +/// this frontend boundary. +pub fn managed_agents_from_events(events: &[Event]) -> Value { + let arr: Vec = events + .iter() + .filter_map(|event| { + let agent_pubkey = nostr::PublicKey::from_hex(first_tag_value(event, "d")?).ok()?; + let pubkey = agent_pubkey.to_hex(); + let npub = agent_pubkey.to_bech32().unwrap_or_else(|_| pubkey.clone()); + let content: Value = serde_json::from_str(&event.content).ok()?; + let object = content.as_object()?; + let name = object + .get("name") + .and_then(Value::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .unwrap_or(&npub); + // Only the modes `RespondTo` can represent may be projected. The + // harness also supports `nobody`, but the desktop enum deliberately + // omits it, so emitting it here would fail deserialization for the + // WHOLE directory response — one agent publishing an unsupported + // mode would hide every other agent. An unrepresentable mode + // becomes absent, which every eligibility check treats as + // not-invocable, so the projection degrades closed. + let respond_to = object + .get("respond_to") + .and_then(Value::as_str) + .filter(|mode| matches!(*mode, "owner-only" | "allowlist" | "anyone")); + + let mut respond_to_allowlist = Vec::new(); + if let Some(values) = object.get("respond_to_allowlist").and_then(Value::as_array) { + for value in values { + let Some(raw) = value.as_str() else { + continue; + }; + let Ok(key) = nostr::PublicKey::from_hex(raw.trim()) else { + continue; + }; + let normalized = key.to_hex(); + if !respond_to_allowlist.contains(&normalized) { + respond_to_allowlist.push(normalized); + } + } + } + + Some(json!({ + "pubkey": pubkey, + "name": name, + "agent_type": "agent", + "channels": [], + "channel_ids": [], + "capabilities": [], + "status": "offline", + "respond_to": respond_to, + "respond_to_allowlist": respond_to_allowlist, + })) + }) + .collect(); + + json!({ "agents": arr }) +} +#[cfg(test)] +mod tests { + use super::*; + use nostr::{EventBuilder, Keys, Kind, Tag}; + + fn ev(kind: u16, content: &str, tags: Vec>) -> Event { + let keys = Keys::generate(); + let tags: Vec = tags + .into_iter() + .map(|t| Tag::parse(t.iter().map(|s| s.to_string()).collect::>()).unwrap()) + .collect(); + EventBuilder::new(Kind::from(kind), content) + .tags(tags) + .sign_with_keys(&keys) + .unwrap() + } + + #[test] + fn managed_agents_use_d_tag_identity_and_preserve_allowlist_metadata() { + let agent_pubkey = "02".repeat(32); + let allowed_pubkey = "03".repeat(32); + let e = ev( + 30177, + &format!( + r#"{{"pubkey":"forged","name":"Scout","parallelism":1,"respond_to":"allowlist","respond_to_allowlist":["{allowed_pubkey}"],"env_vars":{{"SECRET":"do-not-project"}}}}"# + ), + vec![vec!["d", &agent_pubkey]], + ); + let v = managed_agents_from_events(std::slice::from_ref(&e)); + let agents = v.get("agents").cloned().unwrap(); + let parsed: Vec = + serde_json::from_value(agents).unwrap(); + + assert_eq!(parsed.len(), 1); + assert_eq!(parsed[0].pubkey, agent_pubkey); + assert_eq!(parsed[0].name, "Scout"); + assert_eq!( + parsed[0].respond_to, + Some(crate::managed_agents::RespondTo::Allowlist) + ); + assert_eq!(parsed[0].respond_to_allowlist, vec![allowed_pubkey]); + assert!( + !v.to_string().contains("SECRET"), + "the directory projection must remain an explicit public-field allowlist" + ); + } + + /// One agent publishing a mode the desktop enum cannot represent must not + /// take down the entire directory. `respond_to` deserializes into + /// `RespondTo`, which has no `nobody` variant, so projecting that string + /// would fail the whole `Vec` and hide every other agent. + #[test] + fn managed_agents_unsupported_mode_does_not_break_other_entries() { + let good_pubkey = "02".repeat(32); + let nobody_pubkey = "03".repeat(32); + let good = ev( + 30177, + r#"{"name":"Good","respond_to":"owner-only"}"#, + vec![vec!["d", &good_pubkey]], + ); + let nobody = ev( + 30177, + r#"{"name":"Nope","respond_to":"nobody"}"#, + vec![vec!["d", &nobody_pubkey]], + ); + let garbage = ev( + 30177, + r#"{"name":"Junk","respond_to":"not-a-mode"}"#, + vec![vec!["d", &"04".repeat(32)]], + ); + + let v = managed_agents_from_events(&[good, nobody, garbage]); + let parsed: Vec = + serde_json::from_value(v.get("agents").cloned().unwrap()) + .expect("an unsupported mode must not fail the whole directory"); + + assert_eq!(parsed.len(), 3, "every entry survives"); + assert_eq!( + parsed[0].respond_to, + Some(crate::managed_agents::RespondTo::OwnerOnly) + ); + // Unrepresentable modes degrade closed: absent, never invocable. + assert_eq!(parsed[1].respond_to, None); + assert_eq!(parsed[2].respond_to, None); + } + + #[test] + fn managed_agents_drop_events_without_a_valid_agent_d_tag() { + let missing = ev( + 30177, + r#"{"name":"Missing","parallelism":1,"respond_to":"owner-only"}"#, + vec![], + ); + let invalid = ev( + 30177, + r#"{"name":"Invalid","parallelism":1,"respond_to":"owner-only"}"#, + vec![vec!["d", "not-a-pubkey"]], + ); + let v = managed_agents_from_events(&[missing, invalid]); + + assert_eq!(v.get("agents").and_then(Value::as_array).unwrap().len(), 0); + } +}