From 668e638a11dc09536ad342331be71a5b723d3bbd Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Fri, 4 Sep 2026 12:24:26 -0400 Subject: [PATCH 1/7] Add Buzz context and skills to Pi agents Signed-off-by: Salman Mohammed --- crates/buzz-acp/src/acp.rs | 57 ++- crates/buzz-acp/src/base_prompt.md | 2 +- crates/buzz-acp/src/lib.rs | 74 +++- crates/buzz-acp/src/pi_system_prompt.rs | 363 ++++++++++++++++++ .../src/managed_agents/discovery/presets.rs | 145 +++++-- 5 files changed, 589 insertions(+), 52 deletions(-) create mode 100644 crates/buzz-acp/src/pi_system_prompt.rs diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 0d87bca028c..697abe3027d 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -508,8 +508,10 @@ impl AcpClient { // Handled by build_codex_config_env; skip here to avoid double-setting. continue; } - if std::env::var_os(key).is_none() { - cmd.env(key, value); + let target_key = extra_env_target_key(key); + let parent_has_value = std::env::var_os(target_key).is_some(); + if should_apply_extra_env(key, parent_has_value) { + cmd.env(target_key, value); } } if let Some(merged) = codex_config_value { @@ -2040,6 +2042,21 @@ impl AcpClient { } } +/// Translate Buzz's internal forced Pi launcher input to the environment key +/// consumed by `pi-acp`. All real adapter environment keys continue through +/// the ordinary operator-wins path. +fn extra_env_target_key(key: &str) -> &str { + if key == crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV { + crate::pi_system_prompt::PI_ACP_PI_COMMAND_ENV + } else { + key + } +} + +fn should_apply_extra_env(key: &str, parent_has_value: bool) -> bool { + key == crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV || !parent_has_value +} + /// Build `session/prompt` params from one or more text content blocks. fn build_prompt_params(session_id: &str, prompt_blocks: &[&str]) -> serde_json::Value { let blocks: Vec = prompt_blocks @@ -2396,6 +2413,21 @@ mod tests { assert_eq!(StopReason::from_str("Refusal"), Some(StopReason::Refusal)); } + #[test] + fn buzz_pi_launcher_overrides_inherited_adapter_command() { + assert_eq!( + extra_env_target_key(crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV), + crate::pi_system_prompt::PI_ACP_PI_COMMAND_ENV + ); + assert_eq!(extra_env_target_key("ORDINARY_ENV"), "ORDINARY_ENV"); + assert!(should_apply_extra_env( + crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV, + true + )); + assert!(!should_apply_extra_env("ORDINARY_ENV", true)); + assert!(should_apply_extra_env("ORDINARY_ENV", false)); + } + #[test] fn find_allow_once_by_kind_not_by_option_id() { // optionId values are intentionally non-obvious to prove we don't hardcode them. @@ -3125,6 +3157,27 @@ mod tests { ); } + #[cfg(unix)] + #[tokio::test] + async fn spawn_forces_generated_pi_launcher_into_adapter_environment() { + let launcher = "/tmp/buzz-generated-pi-launcher"; + let extra_env = [( + crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), + launcher.to_string(), + )]; + + assert_eq!( + spawn_named_and_read_child_env( + "pi-acp", + crate::pi_system_prompt::PI_ACP_PI_COMMAND_ENV, + &extra_env, + ) + .await, + launcher, + "pi-acp must observe Buzz's generated launcher even when the parent has PI_ACP_PI_COMMAND" + ); + } + #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; diff --git a/crates/buzz-acp/src/base_prompt.md b/crates/buzz-acp/src/base_prompt.md index 7e90b07b6a9..3df5aa0f6f7 100644 --- a/crates/buzz-acp/src/base_prompt.md +++ b/crates/buzz-acp/src/base_prompt.md @@ -1,4 +1,4 @@ -You are operating inside the Buzz platform — a Nostr-based messaging platform for human-agent collaboration. The buzz-acp harness routes channel events to your session. +You are a managed agent in Buzz, a shared workspace where humans and agents collaborate as colleagues. Buzz is a desktop and mobile collaboration app organized around channels, conversations, and shared work. ## Buzz CLI diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 17d3030f635..e9d7746e7f4 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -5,6 +5,7 @@ mod config; mod engram_fetch; mod filter; mod observer; +mod pi_system_prompt; mod pool; mod pool_lifecycle; mod prompt_framing; @@ -2510,6 +2511,55 @@ async fn tokio_main() -> Result<()> { tracing::info!("buzz-acp starting: {}", config.summary()); + let cwd = current_working_directory()?; + let base_prompt_content = config.base_prompt_content.take(); + let base_prompt = if config.no_base_prompt { + None + } else { + // Build standing context once under the configured policy, before any + // agent process starts. Pi consumes this through its native + // `--system-prompt`; other ACP agents consume the same bytes through + // session/new or legacy first-turn framing. + Some( + config.session_policy.append_session_model( + base_prompt_content + .as_deref() + .unwrap_or(include_str!("base_prompt.md")), + ), + ) + }; + let configured_pi_command = config + .persona_env_vars + .iter() + .rev() + .find(|(key, _)| key == pi_system_prompt::PI_ACP_PI_COMMAND_ENV) + .map(|(_, value)| value.clone()); + let managed_skills_dir = std::path::Path::new(&cwd).join(".agents/skills"); + let (pi_launch_override, base_prompt) = pi_system_prompt::PiLaunchOverride::prepare( + &config.agent_command, + base_prompt, + configured_pi_command.as_deref(), + &managed_skills_dir, + ) + .context("failed to prepare Pi launch overrides")?; + if let Some(prepared) = pi_launch_override.as_ref() { + // The wrapper invokes the effective inherited/persona Pi command, so + // remove any persona copy before installing the one forced adapter + // override that pi-acp must observe. + config.persona_env_vars.retain(|(key, _)| { + key != pi_system_prompt::PI_ACP_PI_COMMAND_ENV + && key != pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV + }); + config.persona_env_vars.push(( + pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), + prepared.launcher_path().to_string_lossy().into_owned(), + )); + tracing::info!( + skills_dir = %managed_skills_dir.display(), + "configured Pi to consume Buzz standing context and managed skills through native CLI flags" + ); + } + let observer = config .relay_observer .then(observer::ObserverHandle::in_process); @@ -2758,8 +2808,6 @@ async fn tokio_main() -> Result<()> { ); } - let base_prompt_content = config.base_prompt_content.take(); - let cwd = current_working_directory()?; let ctx = Arc::new(PromptContext { mcp_servers: build_mcp_servers(&config), initial_message: config.initial_message.clone(), @@ -2770,20 +2818,7 @@ async fn tokio_main() -> Result<()> { system_prompt: config.system_prompt.clone(), session_title: config.session_title.clone(), team_instructions: config.team_instructions.clone(), - base_prompt: if config.no_base_prompt { - None - } else { - // Build standing context once under the configured policy, before - // any session/new. Both modern ACP and legacy first-turn framing - // consume this same assembled base (including custom base files). - Some( - config.session_policy.append_session_model( - base_prompt_content - .as_deref() - .unwrap_or(include_str!("base_prompt.md")), - ), - ) - }, + base_prompt, heartbeat_prompt: config.heartbeat_prompt.clone(), cwd, rest_client: relay.rest_client(), @@ -4116,6 +4151,10 @@ async fn tokio_main() -> Result<()> { // for the background task to finish, rather than aborting immediately (#40). relay.shutdown().await; + // Pi may restore subprocesses throughout the pool lifetime. Remove its + // private prompt and launcher only after every adapter has shut down. + drop(pi_launch_override); + tracing::info!("buzz-acp stopped"); Ok(()) } @@ -5208,6 +5247,9 @@ mod agent_draft_prompt_tests { #[test] fn shared_base_prompt_teaches_portable_agent_drafts() { let prompt = include_str!("base_prompt.md"); + assert!(prompt.starts_with( + "You are a managed agent in Buzz, a shared workspace where humans and agents collaborate as colleagues. Buzz is a desktop and mobile collaboration app organized around channels, conversations, and shared work." + )); assert!(prompt.contains("buzz agents draft-create")); assert!(prompt.contains("ask for at most two things")); assert!(prompt.contains("what it should do day-to-day")); diff --git a/crates/buzz-acp/src/pi_system_prompt.rs b/crates/buzz-acp/src/pi_system_prompt.rs new file mode 100644 index 00000000000..51a1f43aacb --- /dev/null +++ b/crates/buzz-acp/src/pi_system_prompt.rs @@ -0,0 +1,363 @@ +//! Pi-specific native launch setup. +//! +//! `pi-acp` does not currently consume ACP `session/new.systemPrompt`, but it +//! does let callers replace the `pi` executable through +//! `PI_ACP_PI_COMMAND`. For Pi sessions, Buzz points that variable at a +//! private launcher which adds `--system-prompt ` and the canonical Buzz +//! `--skill ` before forwarding the adapter's RPC/session arguments +//! unchanged. + +use std::fs::{self, OpenOptions}; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; + +#[cfg(unix)] +use std::ffi::OsStr; + +use uuid::Uuid; + +pub(crate) const PI_ACP_PI_COMMAND_ENV: &str = "PI_ACP_PI_COMMAND"; +/// Internal `AcpClient` input translated to `PI_ACP_PI_COMMAND` with forced +/// precedence. Keeping this distinct preserves normal operator-wins behavior +/// for every actual adapter environment variable. +pub(crate) const BUZZ_PI_LAUNCHER_OVERRIDE_ENV: &str = "BUZZ_ACP_INTERNAL_PI_LAUNCHER_OVERRIDE"; + +/// Files backing the Pi launcher for one `buzz-acp` process. +/// +/// The guard must live as long as the ACP pool because `pi-acp` may start or +/// restore Pi subprocesses after its own initialization. +pub(crate) struct PiLaunchOverride { + directory: PathBuf, + launcher: PathBuf, +} + +impl PiLaunchOverride { + /// Prepare a Pi launcher when the configured ACP adapter is `pi-acp`. + /// + /// Returns the prompt that still needs ordinary ACP delivery. For Pi, the + /// base prompt moves into Pi's native system role and is therefore removed + /// from first-turn user framing. Other adapters receive it unchanged. + pub(crate) fn prepare( + agent_command: &str, + base_prompt: Option, + configured_pi_command: Option<&str>, + managed_skills_dir: &Path, + ) -> io::Result<(Option, Option)> { + if crate::config::normalize_agent_command_identity(agent_command) != "pi-acp" { + return Ok((None, base_prompt)); + } + + // Match AcpClient's normal environment precedence: an inherited + // operator value wins over persona/default environment. The generated + // launcher then becomes pi-acp's command and invokes that effective + // value as the real Pi executable. + let inherited_pi_command = std::env::var(PI_ACP_PI_COMMAND_ENV).ok(); + let pi_command = inherited_pi_command + .as_deref() + .or(configured_pi_command) + .unwrap_or("pi"); + let prepared = Self::create(pi_command, base_prompt.as_deref(), managed_skills_dir)?; + Ok((Some(prepared), None)) + } + + pub(crate) fn launcher_path(&self) -> &Path { + &self.launcher + } + + fn create( + pi_command: &str, + prompt: Option<&str>, + managed_skills_dir: &Path, + ) -> io::Result { + let directory = std::env::temp_dir().join(format!( + "buzz-acp-pi-launcher-{}-{}", + std::process::id(), + Uuid::new_v4() + )); + create_private_directory(&directory)?; + + let prompt_path = directory.join("SYSTEM.md"); + let launcher = directory.join(launcher_file_name()); + // Construct the cleanup guard before either file write. Any later `?` + // drops it, so a partial setup cannot strand the private prompt file. + let prepared = Self { + directory, + launcher, + }; + + if let Some(prompt) = prompt { + write_private_file(&prompt_path, prompt.as_bytes(), false)?; + } + + let script = launcher_script( + pi_command, + prompt.map(|_| prompt_path.as_path()), + managed_skills_dir, + )?; + write_private_file(&prepared.launcher, script.as_bytes(), true)?; + + Ok(prepared) + } +} + +impl Drop for PiLaunchOverride { + fn drop(&mut self) { + if let Err(error) = fs::remove_dir_all(&self.directory) { + if error.kind() != io::ErrorKind::NotFound { + tracing::warn!( + path = %self.directory.display(), + %error, + "failed to remove temporary Pi launcher" + ); + } + } + } +} + +#[cfg(unix)] +fn create_private_directory(path: &Path) -> io::Result<()> { + use std::os::unix::fs::DirBuilderExt; + + let mut builder = fs::DirBuilder::new(); + builder.mode(0o700).create(path) +} + +#[cfg(not(unix))] +fn create_private_directory(path: &Path) -> io::Result<()> { + fs::create_dir(path) +} + +fn write_private_file(path: &Path, content: &[u8], executable: bool) -> io::Result<()> { + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(if executable { 0o700 } else { 0o600 }); + } + + #[cfg(not(unix))] + let _ = executable; + + let mut file = options.open(path)?; + file.write_all(content)?; + file.sync_all() +} + +#[cfg(unix)] +fn launcher_file_name() -> &'static str { + "pi-with-buzz-context" +} + +#[cfg(windows)] +fn launcher_file_name() -> &'static str { + "pi-with-buzz-context.cmd" +} + +#[cfg(not(any(unix, windows)))] +fn launcher_file_name() -> &'static str { + "pi-with-buzz-context" +} + +#[cfg(unix)] +fn launcher_script( + pi_command: &str, + prompt_path: Option<&Path>, + managed_skills_dir: &Path, +) -> io::Result { + let system_prompt_arg = match prompt_path { + Some(prompt_path) => format!(" --system-prompt {}", shell_quote(prompt_path.as_os_str())?), + None => String::new(), + }; + Ok(format!( + "#!/bin/sh\nexec {}{} --skill {} \"$@\"\n", + shell_quote(OsStr::new(pi_command))?, + system_prompt_arg, + shell_quote(managed_skills_dir.as_os_str())?, + )) +} + +#[cfg(unix)] +fn shell_quote(value: &OsStr) -> io::Result { + let value = value.to_str().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + "Pi launcher paths must be valid UTF-8", + ) + })?; + Ok(format!("'{}'", value.replace('\'', "'\"'\"'"))) +} + +#[cfg(windows)] +fn launcher_script( + pi_command: &str, + prompt_path: Option<&Path>, + managed_skills_dir: &Path, +) -> io::Result { + let system_prompt_arg = match prompt_path { + Some(prompt_path) => { + let prompt_path = prompt_path.to_str().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + "Pi launcher paths must be valid UTF-8", + ) + })?; + format!(" --system-prompt \"{}\"", batch_escape(prompt_path)) + } + None => String::new(), + }; + let managed_skills_dir = managed_skills_dir.to_str().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + "Pi skill paths must be valid UTF-8", + ) + })?; + Ok(format!( + "@echo off\r\n\"{}\"{} --skill \"{}\" %*\r\nexit /b %ERRORLEVEL%\r\n", + batch_escape(pi_command), + system_prompt_arg, + batch_escape(managed_skills_dir), + )) +} + +#[cfg(windows)] +fn batch_escape(value: &str) -> String { + value.replace('%', "%%").replace('"', "\"\"") +} + +#[cfg(not(any(unix, windows)))] +fn launcher_script( + _pi_command: &str, + _prompt_path: Option<&Path>, + _managed_skills_dir: &Path, +) -> io::Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "Pi launch overrides are unsupported on this platform", + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn non_pi_adapter_keeps_base_prompt_for_acp_delivery() { + let base = Some("Buzz base".to_string()); + let (prepared, remaining) = + PiLaunchOverride::prepare("goose", base.clone(), None, Path::new("/unused/skills")) + .expect("prepare"); + assert!(prepared.is_none()); + assert_eq!(remaining, base); + } + + #[test] + fn disabled_base_prompt_still_creates_pi_skills_launcher() { + let (prepared, remaining) = + PiLaunchOverride::prepare("pi-acp", None, None, Path::new("/unused/skills")) + .expect("prepare"); + let prepared = prepared.expect("Pi skills launcher"); + assert!(remaining.is_none()); + assert!(!prepared.directory.join("SYSTEM.md").exists()); + + #[cfg(unix)] + assert!(fs::read_to_string(prepared.launcher_path()) + .expect("read launcher") + .contains("--skill '/unused/skills'")); + } + + #[test] + fn pi_adapter_moves_buzz_base_out_of_ordinary_acp_delivery() { + let base = crate::scope::SessionPolicy::Thread + .append_session_model(include_str!("base_prompt.md")); + let (prepared, remaining) = PiLaunchOverride::prepare( + "/opt/bin/pi-acp", + Some(base.clone()), + Some("alternate-pi"), + Path::new("/buzz/.agents/skills"), + ) + .expect("prepare"); + let prepared = prepared.expect("Pi launcher"); + + assert!(remaining.is_none()); + assert_eq!( + fs::read_to_string(prepared.directory.join("SYSTEM.md")).expect("read prompt"), + base + ); + assert!(base.contains("each thread gets its own")); + } + + #[cfg(unix)] + #[test] + fn pi_launcher_replaces_system_prompt_and_forwards_adapter_args() { + use std::os::unix::fs::PermissionsExt; + use std::process::Command; + + let fixture_dir = + std::env::temp_dir().join(format!("buzz-acp-pi-system-prompt-test-{}", Uuid::new_v4())); + create_private_directory(&fixture_dir).expect("create fixture dir"); + let capture_path = fixture_dir.join("args.txt"); + let fake_pi = fixture_dir.join("fake-pi"); + let managed_skills_dir = fixture_dir.join("managed skills"); + let fake_script = format!( + "#!/bin/sh\nprintf '%s\\n' \"$@\" > {}\n", + shell_quote(capture_path.as_os_str()).expect("quote capture path") + ); + write_private_file(&fake_pi, fake_script.as_bytes(), true).expect("write fake pi"); + + let prepared = PiLaunchOverride::create( + fake_pi.to_str().expect("UTF-8 fake Pi path"), + Some("Buzz base\n\n## Session Model\nThread scoped"), + &managed_skills_dir, + ) + .expect("prepare Pi launcher"); + let prompt_path = prepared.directory.join("SYSTEM.md"); + + let status = Command::new(prepared.launcher_path()) + .args(["--mode", "rpc", "--session", "/tmp/session.jsonl"]) + .status() + .expect("run launcher"); + assert!(status.success()); + assert_eq!( + fs::read_to_string(&capture_path).expect("read captured args"), + format!( + "--system-prompt\n{}\n--skill\n{}\n--mode\nrpc\n--session\n/tmp/session.jsonl\n", + prompt_path.display(), + managed_skills_dir.display(), + ) + ); + assert_eq!( + fs::read_to_string(&prompt_path).expect("read system prompt"), + "Buzz base\n\n## Session Model\nThread scoped" + ); + assert_eq!( + fs::metadata(&prompt_path) + .expect("prompt metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + assert_eq!( + fs::metadata(prepared.launcher_path()) + .expect("launcher metadata") + .permissions() + .mode() + & 0o777, + 0o700 + ); + assert_eq!( + fs::metadata(&prepared.directory) + .expect("directory metadata") + .permissions() + .mode() + & 0o777, + 0o700 + ); + + drop(prepared); + assert!(!prompt_path.exists()); + fs::remove_dir_all(fixture_dir).expect("remove fixture dir"); + } +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index 438cb5eb863..b184559c157 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -16,11 +16,10 @@ pub(super) struct PresetHarness { install_instructions_url: &'static str, install_hint: &'static str, /// Vendor CLI the ACP command wraps, when the preset is an adapter. - /// - /// Consulted only when the adapter is absent, so `AdapterMissing` replaces - /// `NotInstalled` when the CLI is present but the adapter is not. `None` - /// when the command is itself the vendor CLI. underlying_cli: Option<&'static str>, + /// State-specific setup guidance for the wrapped vendor CLI. + underlying_cli_install_hint: Option<&'static str>, + underlying_cli_install_instructions_url: Option<&'static str>, } /// Build one preset catalog entry through an injectable command resolver. @@ -28,28 +27,44 @@ pub(super) fn preset_catalog_entry( def: &PresetHarness, resolve: impl Fn(&str) -> Option, ) -> AcpRuntimeCatalogEntry { - let (availability, command, binary_path) = match resolve(def.command) { - Some(path) => ( - AcpAvailabilityStatus::Available, - Some(def.command.to_string()), - Some(path.display().to_string()), - ), - None => { - let underlying_cli_found = def - .underlying_cli - .map(|cli| resolve(cli).is_some()) - .unwrap_or(false); - if underlying_cli_found { - (AcpAvailabilityStatus::AdapterMissing, None, None) - } else { - (AcpAvailabilityStatus::NotInstalled, None, None) - } - } - }; let underlying_cli_path = def .underlying_cli - .and_then(resolve) + .and_then(&resolve) .map(|path| path.display().to_string()); + let (availability, command, binary_path) = super::classify_runtime( + resolve(def.command).map(|path| (def.command, path)), + def.underlying_cli, + underlying_cli_path.is_some(), + ); + + let cli_install_hint = def.underlying_cli.map(|cli| { + def.underlying_cli_install_hint + .map(str::to_string) + .unwrap_or_else(|| { + format!( + "Install the {} CLI and make sure {} is on your PATH.", + def.label, cli + ) + }) + }); + let install_hint = match availability { + AcpAvailabilityStatus::Available if def.underlying_cli.is_some() => String::new(), + AcpAvailabilityStatus::CliMissing => cli_install_hint.unwrap_or_default(), + AcpAvailabilityStatus::NotInstalled if def.underlying_cli.is_some() => { + format!( + "{} {}", + cli_install_hint.unwrap_or_default(), + def.install_hint + ) + } + _ => def.install_hint.to_string(), + }; + let install_instructions_url = match availability { + AcpAvailabilityStatus::CliMissing | AcpAvailabilityStatus::NotInstalled => def + .underlying_cli_install_instructions_url + .unwrap_or(def.install_instructions_url), + _ => def.install_instructions_url, + }; AcpRuntimeCatalogEntry { id: def.id.to_string(), @@ -71,12 +86,10 @@ pub(super) fn preset_catalog_entry( max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, - install_hint: def.install_hint.to_string(), - install_instructions_url: def.install_instructions_url.to_string(), + install_hint, + install_instructions_url: install_instructions_url.to_string(), can_auto_install: false, - // Presets carry one flat install hint, so builtin external-CLI copy - // would name the wrong missing component for adapter presets. - requires_external_cli: false, + requires_external_cli: def.underlying_cli.is_some(), underlying_cli_path, node_required: false, auth_status: AuthStatus::NotApplicable, @@ -97,8 +110,14 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ command: "pi-acp", args: &[], install_instructions_url: "https://github.com/svkozak/pi-acp", - install_hint: "Buzz talks to Pi through the pi-acp adapter. Install Pi with `npm install -g --ignore-scripts @earendil-works/pi-coding-agent`, then install the adapter with `npm install -g pi-acp`.", + install_hint: "Install the Pi ACP adapter with npm install -g pi-acp.", underlying_cli: Some("pi"), + underlying_cli_install_hint: Some( + "Install Pi with npm install -g --ignore-scripts @earendil-works/pi-coding-agent.", + ), + underlying_cli_install_instructions_url: Some( + "https://github.com/badlogic/pi-mono/tree/main/packages/coding-agent", + ), }, PresetHarness { id: "devin", @@ -108,6 +127,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://docs.devin.ai/cli", install_hint: "Buzz talks to Devin through the official Devin CLI's ACP mode (devin acp).", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "cursor", @@ -117,6 +138,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://cursor.com/downloads", install_hint: "Buzz talks to Cursor through the cursor-agent CLI's ACP mode.", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "omp", @@ -126,6 +149,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://omp.sh/", install_hint: "Buzz talks to Oh My Pi through its CLI's ACP mode (omp acp).", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "grok", @@ -135,6 +160,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://build.x.ai/docs", install_hint: "Buzz talks to Grok Build through its CLI's agent stdio mode.", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "opencode", @@ -144,6 +171,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://opencode.ai/docs", install_hint: "Buzz talks to OpenCode through its CLI's ACP mode (opencode acp).", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "kimi", @@ -153,6 +182,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://kimi.ai/download", install_hint: "Buzz talks to Kimi Code through its CLI's ACP mode (kimi acp).", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "amp", @@ -162,6 +193,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://github.com/tao12345666333/amp-acp", install_hint: "Buzz talks to the Amp CLI through the amp-acp adapter. Follow the setup guide to install the adapter so the amp-acp command is on your PATH.", underlying_cli: Some("amp"), + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "hermes", @@ -171,6 +204,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ install_instructions_url: "https://hermes-agent.nousresearch.com", install_hint: "Buzz talks to Hermes Agent through its hermes-acp command.", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, PresetHarness { id: "openclaw", @@ -187,6 +222,8 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ needs BUZZ_* credentials at execution time, set them on the \ Gateway's own environment separately.", underlying_cli: None, + underlying_cli_install_hint: None, + underlying_cli_install_instructions_url: None, }, ]; @@ -303,6 +340,8 @@ mod tests { install_instructions_url: "https://example.com/install", install_hint: "Install the amp-acp npm adapter.", underlying_cli: Some("amp"), + underlying_cli_install_hint: Some("Install the Amp Test CLI."), + underlying_cli_install_instructions_url: Some("https://example.com/amp"), }; #[test] @@ -377,6 +416,8 @@ mod tests { assert_eq!(available.availability, AcpAvailabilityStatus::Available); assert_eq!(available.command.as_deref(), Some("pi-acp")); assert!(available.default_args.is_empty()); + assert!(available.install_hint.is_empty()); + assert!(available.requires_external_cli); assert_eq!( available.underlying_cli_path.as_deref(), Some("/usr/local/bin/pi") @@ -391,12 +432,38 @@ mod tests { ); assert!(adapter_missing.command.is_none()); assert!(adapter_missing.default_args.is_empty()); + assert_eq!( + adapter_missing.install_hint, + "Install the Pi ACP adapter with npm install -g pi-acp." + ); + assert_eq!( + adapter_missing.install_instructions_url, + "https://github.com/svkozak/pi-acp" + ); + + let cli_missing = preset_catalog_entry(preset, |command| { + (command == "pi-acp").then(|| PathBuf::from("/usr/local/bin/pi-acp")) + }); + assert_eq!(cli_missing.availability, AcpAvailabilityStatus::CliMissing); + assert_eq!(cli_missing.command.as_deref(), Some("pi-acp")); + assert_eq!( + cli_missing.install_hint, + "Install Pi with npm install -g --ignore-scripts @earendil-works/pi-coding-agent." + ); + assert_eq!( + cli_missing.install_instructions_url, + "https://github.com/badlogic/pi-mono/tree/main/packages/coding-agent" + ); let not_installed = preset_catalog_entry(preset, |_| None); assert_eq!( not_installed.availability, AcpAvailabilityStatus::NotInstalled ); + assert_eq!( + not_installed.install_hint, + "Install Pi with npm install -g --ignore-scripts @earendil-works/pi-coding-agent. Install the Pi ACP adapter with npm install -g pi-acp." + ); } #[test] @@ -411,8 +478,12 @@ mod tests { entry.underlying_cli_path.as_deref(), Some("/usr/local/bin/amp") ); - assert!(!entry.requires_external_cli); + assert!(entry.requires_external_cli); assert_eq!(entry.install_hint, "Install the amp-acp npm adapter."); + assert_eq!( + entry.install_instructions_url, + "https://example.com/install" + ); } #[test] @@ -420,7 +491,12 @@ mod tests { let entry = preset_catalog_entry(&ADAPTER_PRESET, |_| None); assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); assert!(entry.underlying_cli_path.is_none()); - assert!(!entry.requires_external_cli); + assert!(entry.requires_external_cli); + assert_eq!( + entry.install_hint, + "Install the Amp Test CLI. Install the amp-acp npm adapter." + ); + assert_eq!(entry.install_instructions_url, "https://example.com/amp"); } #[test] @@ -437,17 +513,20 @@ mod tests { entry.underlying_cli_path.as_deref(), Some("/usr/local/bin/amp") ); + assert!(entry.install_hint.is_empty()); } #[test] - fn adapter_presence_is_enough_for_availability() { + fn adapter_without_underlying_cli_reports_cli_missing() { let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| { (command == "amp-acp").then(|| PathBuf::from("/usr/local/bin/amp-acp")) }); - assert_eq!(entry.availability, AcpAvailabilityStatus::Available); + assert_eq!(entry.availability, AcpAvailabilityStatus::CliMissing); assert_eq!(entry.command.as_deref(), Some("amp-acp")); assert_eq!(entry.binary_path.as_deref(), Some("/usr/local/bin/amp-acp")); assert!(entry.underlying_cli_path.is_none()); + assert_eq!(entry.install_hint, "Install the Amp Test CLI."); + assert_eq!(entry.install_instructions_url, "https://example.com/amp"); } #[test] From 34e22c97623894d98ef56ae6e7d94d70433e2a2a Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Fri, 4 Sep 2026 12:31:33 -0400 Subject: [PATCH 2/7] Rename Pi launcher module Signed-off-by: Salman Mohammed --- crates/buzz-acp/src/acp.rs | 16 ++++++++-------- crates/buzz-acp/src/lib.rs | 12 ++++++------ .../src/{pi_system_prompt.rs => pi_launcher.rs} | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) rename crates/buzz-acp/src/{pi_system_prompt.rs => pi_launcher.rs} (99%) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 697abe3027d..b1e92d5cf7f 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -2046,15 +2046,15 @@ impl AcpClient { /// consumed by `pi-acp`. All real adapter environment keys continue through /// the ordinary operator-wins path. fn extra_env_target_key(key: &str) -> &str { - if key == crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV { - crate::pi_system_prompt::PI_ACP_PI_COMMAND_ENV + if key == crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV { + crate::pi_launcher::PI_ACP_PI_COMMAND_ENV } else { key } } fn should_apply_extra_env(key: &str, parent_has_value: bool) -> bool { - key == crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV || !parent_has_value + key == crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV || !parent_has_value } /// Build `session/prompt` params from one or more text content blocks. @@ -2416,12 +2416,12 @@ mod tests { #[test] fn buzz_pi_launcher_overrides_inherited_adapter_command() { assert_eq!( - extra_env_target_key(crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV), - crate::pi_system_prompt::PI_ACP_PI_COMMAND_ENV + extra_env_target_key(crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV), + crate::pi_launcher::PI_ACP_PI_COMMAND_ENV ); assert_eq!(extra_env_target_key("ORDINARY_ENV"), "ORDINARY_ENV"); assert!(should_apply_extra_env( - crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV, + crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV, true )); assert!(!should_apply_extra_env("ORDINARY_ENV", true)); @@ -3162,14 +3162,14 @@ mod tests { async fn spawn_forces_generated_pi_launcher_into_adapter_environment() { let launcher = "/tmp/buzz-generated-pi-launcher"; let extra_env = [( - crate::pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), + crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), launcher.to_string(), )]; assert_eq!( spawn_named_and_read_child_env( "pi-acp", - crate::pi_system_prompt::PI_ACP_PI_COMMAND_ENV, + crate::pi_launcher::PI_ACP_PI_COMMAND_ENV, &extra_env, ) .await, diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index e9d7746e7f4..a8efe88ca24 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -5,7 +5,7 @@ mod config; mod engram_fetch; mod filter; mod observer; -mod pi_system_prompt; +mod pi_launcher; mod pool; mod pool_lifecycle; mod prompt_framing; @@ -2532,10 +2532,10 @@ async fn tokio_main() -> Result<()> { .persona_env_vars .iter() .rev() - .find(|(key, _)| key == pi_system_prompt::PI_ACP_PI_COMMAND_ENV) + .find(|(key, _)| key == pi_launcher::PI_ACP_PI_COMMAND_ENV) .map(|(_, value)| value.clone()); let managed_skills_dir = std::path::Path::new(&cwd).join(".agents/skills"); - let (pi_launch_override, base_prompt) = pi_system_prompt::PiLaunchOverride::prepare( + let (pi_launch_override, base_prompt) = pi_launcher::PiLaunchOverride::prepare( &config.agent_command, base_prompt, configured_pi_command.as_deref(), @@ -2547,11 +2547,11 @@ async fn tokio_main() -> Result<()> { // remove any persona copy before installing the one forced adapter // override that pi-acp must observe. config.persona_env_vars.retain(|(key, _)| { - key != pi_system_prompt::PI_ACP_PI_COMMAND_ENV - && key != pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV + key != pi_launcher::PI_ACP_PI_COMMAND_ENV + && key != pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV }); config.persona_env_vars.push(( - pi_system_prompt::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), + pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), prepared.launcher_path().to_string_lossy().into_owned(), )); tracing::info!( diff --git a/crates/buzz-acp/src/pi_system_prompt.rs b/crates/buzz-acp/src/pi_launcher.rs similarity index 99% rename from crates/buzz-acp/src/pi_system_prompt.rs rename to crates/buzz-acp/src/pi_launcher.rs index 51a1f43aacb..7efee1f04aa 100644 --- a/crates/buzz-acp/src/pi_system_prompt.rs +++ b/crates/buzz-acp/src/pi_launcher.rs @@ -1,4 +1,4 @@ -//! Pi-specific native launch setup. +//! Pi-specific native launcher setup. //! //! `pi-acp` does not currently consume ACP `session/new.systemPrompt`, but it //! does let callers replace the `pi` executable through From 1cc0d507e80b014b48a7ad8b33667fbec113504f Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Fri, 4 Sep 2026 13:10:34 -0400 Subject: [PATCH 3/7] Honor configured Pi command in discovery Signed-off-by: Salman Mohammed --- .../src-tauri/src/managed_agents/discovery.rs | 7 +- .../src/managed_agents/discovery/presets.rs | 168 ++++++++++++++---- 2 files changed, 137 insertions(+), 38 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 84f88e406b5..d613f21e612 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -1130,6 +1130,7 @@ pub fn discover_acp_runtimes_from( // Track all ids seen so far (builtins) to prevent preset/custom collisions. let mut seen_ids: std::collections::HashSet = entries.iter().map(|e| e.id.clone()).collect(); + let pi_command_override = std::env::var(presets::PI_ACP_PI_COMMAND_ENV).ok(); // Phase 2.5: insert static preset entries (PATH-probed, not editable/deletable). for def in PRESET_HARNESSES { @@ -1139,7 +1140,11 @@ pub fn discover_acp_runtimes_from( } seen_ids.insert(def.id.to_string()); - entries.push(preset_catalog_entry(def, resolve)); + entries.push(preset_catalog_entry( + def, + resolve, + pi_command_override.as_deref(), + )); } // Phase 3: load and append custom harness definitions. diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index b184559c157..aeac261b869 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -7,6 +7,8 @@ use crate::managed_agents::{ use super::normalize_agent_args; +pub(super) const PI_ACP_PI_COMMAND_ENV: &str = "PI_ACP_PI_COMMAND"; + /// Static data for a well-known tier-2 ACP harness. pub(super) struct PresetHarness { pub(super) id: &'static str, @@ -26,9 +28,17 @@ pub(super) struct PresetHarness { pub(super) fn preset_catalog_entry( def: &PresetHarness, resolve: impl Fn(&str) -> Option, + pi_command_override: Option<&str>, ) -> AcpRuntimeCatalogEntry { - let underlying_cli_path = def - .underlying_cli + // pi-acp treats PI_ACP_PI_COMMAND as the authoritative Pi executable. + // Resolve that exact value when configured instead of requiring the bare + // `pi` command to be on PATH; launch uses the same precedence. + let underlying_cli_command = if def.id == "pi" { + pi_command_override.or(def.underlying_cli) + } else { + def.underlying_cli + }; + let underlying_cli_path = underlying_cli_command .and_then(&resolve) .map(|path| path.display().to_string()); let (availability, command, binary_path) = super::classify_runtime( @@ -357,9 +367,11 @@ mod tests { assert_eq!(preset.underlying_cli, None); assert_eq!(preset.install_instructions_url, "https://docs.devin.ai/cli"); - let entry = preset_catalog_entry(preset, |command| { - (command == "devin").then(|| PathBuf::from("/usr/local/bin/devin")) - }); + let entry = preset_catalog_entry( + preset, + |command| (command == "devin").then(|| PathBuf::from("/usr/local/bin/devin")), + None, + ); assert_eq!(entry.availability, AcpAvailabilityStatus::Available); assert_eq!(entry.command.as_deref(), Some("devin")); assert_eq!(entry.default_args, vec!["acp"]); @@ -367,7 +379,7 @@ mod tests { assert_eq!(entry.auth_status, AuthStatus::NotApplicable); assert_eq!(entry.source, HarnessSource::Preset); - let missing_entry = preset_catalog_entry(preset, |_| None); + let missing_entry = preset_catalog_entry(preset, |_| None, None); assert_eq!( missing_entry.availability, AcpAvailabilityStatus::NotInstalled @@ -396,6 +408,68 @@ mod tests { assert_eq!(entry.source, HarnessSource::Preset); } + #[cfg(unix)] + #[test] + fn pi_runtime_catalog_honors_command_override_outside_path() { + use std::os::unix::fs::PermissionsExt; + + use crate::managed_agents::custom_harnesses::registry_test_lock; + use crate::managed_agents::discovery::clear_resolve_cache; + + // Discovery reads process environment and mutates shared resolution + // caches, so serialize this production-path check with its peers. + let _path_guard = crate::managed_agents::lock_path_mutex(); + let _registry_guard = registry_test_lock(); + + let temp = tempfile::tempdir().expect("tempdir"); + let path_dir = temp.path().join("path"); + let override_dir = temp.path().join("override"); + std::fs::create_dir_all(&path_dir).expect("create PATH dir"); + std::fs::create_dir_all(&override_dir).expect("create override dir"); + + let adapter = path_dir.join("pi-acp"); + let configured_pi = override_dir.join("custom-pi"); + for executable in [&adapter, &configured_pi] { + std::fs::write(executable, "#!/bin/sh\nexit 0\n").expect("write fake executable"); + std::fs::set_permissions(executable, std::fs::Permissions::from_mode(0o755)) + .expect("make fake executable runnable"); + } + + let old_path = std::env::var_os("PATH").unwrap_or_default(); + let old_override = std::env::var_os(super::PI_ACP_PI_COMMAND_ENV); + let path = + std::env::join_paths(std::iter::once(path_dir).chain(std::env::split_paths(&old_path))) + .expect("join PATH"); + std::env::set_var("PATH", path); + std::env::set_var(super::PI_ACP_PI_COMMAND_ENV, &configured_pi); + clear_resolve_cache(); + + let result = std::panic::catch_unwind(|| { + let pi = super::super::discover_acp_runtimes_from(None, true) + .into_iter() + .find(|entry| entry.id == "pi") + .expect("Pi preset should appear in runtime catalog"); + + assert_eq!(pi.availability, AcpAvailabilityStatus::Available); + assert_eq!(pi.command.as_deref(), Some("pi-acp")); + assert_eq!( + pi.underlying_cli_path.as_deref(), + Some(configured_pi.to_string_lossy().as_ref()), + "catalog must resolve PI_ACP_PI_COMMAND rather than the bare pi command" + ); + }); + + std::env::set_var("PATH", old_path); + match old_override { + Some(value) => std::env::set_var(super::PI_ACP_PI_COMMAND_ENV, value), + None => std::env::remove_var(super::PI_ACP_PI_COMMAND_ENV), + } + clear_resolve_cache(); + if let Err(error) = result { + std::panic::resume_unwind(error); + } + } + #[test] fn pi_preset_uses_zero_arg_adapter_and_reports_missing_component() { let preset = PRESET_HARNESSES @@ -408,11 +482,15 @@ mod tests { assert!(preset.args.is_empty()); assert_eq!(preset.underlying_cli, Some("pi")); - let available = preset_catalog_entry(preset, |command| match command { - "pi-acp" => Some(PathBuf::from("/usr/local/bin/pi-acp")), - "pi" => Some(PathBuf::from("/usr/local/bin/pi")), - _ => None, - }); + let available = preset_catalog_entry( + preset, + |command| match command { + "pi-acp" => Some(PathBuf::from("/usr/local/bin/pi-acp")), + "pi" => Some(PathBuf::from("/usr/local/bin/pi")), + _ => None, + }, + None, + ); assert_eq!(available.availability, AcpAvailabilityStatus::Available); assert_eq!(available.command.as_deref(), Some("pi-acp")); assert!(available.default_args.is_empty()); @@ -423,9 +501,11 @@ mod tests { Some("/usr/local/bin/pi") ); - let adapter_missing = preset_catalog_entry(preset, |command| { - (command == "pi").then(|| PathBuf::from("/usr/local/bin/pi")) - }); + let adapter_missing = preset_catalog_entry( + preset, + |command| (command == "pi").then(|| PathBuf::from("/usr/local/bin/pi")), + None, + ); assert_eq!( adapter_missing.availability, AcpAvailabilityStatus::AdapterMissing @@ -441,9 +521,11 @@ mod tests { "https://github.com/svkozak/pi-acp" ); - let cli_missing = preset_catalog_entry(preset, |command| { - (command == "pi-acp").then(|| PathBuf::from("/usr/local/bin/pi-acp")) - }); + let cli_missing = preset_catalog_entry( + preset, + |command| (command == "pi-acp").then(|| PathBuf::from("/usr/local/bin/pi-acp")), + None, + ); assert_eq!(cli_missing.availability, AcpAvailabilityStatus::CliMissing); assert_eq!(cli_missing.command.as_deref(), Some("pi-acp")); assert_eq!( @@ -455,7 +537,7 @@ mod tests { "https://github.com/badlogic/pi-mono/tree/main/packages/coding-agent" ); - let not_installed = preset_catalog_entry(preset, |_| None); + let not_installed = preset_catalog_entry(preset, |_| None, None); assert_eq!( not_installed.availability, AcpAvailabilityStatus::NotInstalled @@ -468,9 +550,11 @@ mod tests { #[test] fn adapter_missing_when_underlying_cli_present() { - let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| { - (command == "amp").then(|| PathBuf::from("/usr/local/bin/amp")) - }); + let entry = preset_catalog_entry( + &ADAPTER_PRESET, + |command| (command == "amp").then(|| PathBuf::from("/usr/local/bin/amp")), + None, + ); assert_eq!(entry.availability, AcpAvailabilityStatus::AdapterMissing); assert!(entry.command.is_none()); assert!(entry.binary_path.is_none()); @@ -488,7 +572,7 @@ mod tests { #[test] fn not_installed_when_adapter_and_cli_are_missing() { - let entry = preset_catalog_entry(&ADAPTER_PRESET, |_| None); + let entry = preset_catalog_entry(&ADAPTER_PRESET, |_| None, None); assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); assert!(entry.underlying_cli_path.is_none()); assert!(entry.requires_external_cli); @@ -501,11 +585,15 @@ mod tests { #[test] fn available_when_adapter_and_cli_are_present() { - let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| match command { - "amp-acp" => Some(PathBuf::from("/usr/local/bin/amp-acp")), - "amp" => Some(PathBuf::from("/usr/local/bin/amp")), - _ => None, - }); + let entry = preset_catalog_entry( + &ADAPTER_PRESET, + |command| match command { + "amp-acp" => Some(PathBuf::from("/usr/local/bin/amp-acp")), + "amp" => Some(PathBuf::from("/usr/local/bin/amp")), + _ => None, + }, + None, + ); assert_eq!(entry.availability, AcpAvailabilityStatus::Available); assert_eq!(entry.command.as_deref(), Some("amp-acp")); assert_eq!(entry.binary_path.as_deref(), Some("/usr/local/bin/amp-acp")); @@ -518,9 +606,11 @@ mod tests { #[test] fn adapter_without_underlying_cli_reports_cli_missing() { - let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| { - (command == "amp-acp").then(|| PathBuf::from("/usr/local/bin/amp-acp")) - }); + let entry = preset_catalog_entry( + &ADAPTER_PRESET, + |command| (command == "amp-acp").then(|| PathBuf::from("/usr/local/bin/amp-acp")), + None, + ); assert_eq!(entry.availability, AcpAvailabilityStatus::CliMissing); assert_eq!(entry.command.as_deref(), Some("amp-acp")); assert_eq!(entry.binary_path.as_deref(), Some("/usr/local/bin/amp-acp")); @@ -535,7 +625,7 @@ mod tests { underlying_cli: None, ..ADAPTER_PRESET }; - let entry = preset_catalog_entry(&preset, |_| None); + let entry = preset_catalog_entry(&preset, |_| None, None); assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); assert!(!entry.requires_external_cli); assert!(entry.underlying_cli_path.is_none()); @@ -554,7 +644,7 @@ mod tests { .expect("openclaw preset must be present"); // Simulate "not installed" — resolver always returns None. - let entry = preset_catalog_entry(openclaw, |_| None); + let entry = preset_catalog_entry(openclaw, |_| None, None); assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); assert!( entry.command.is_none(), @@ -576,10 +666,14 @@ mod tests { .find(|p| p.id == "openclaw") .expect("openclaw preset must be present"); - let entry = preset_catalog_entry(openclaw, |cmd| { - (cmd == openclaw.id || cmd == "openclaw") - .then(|| std::path::PathBuf::from("/usr/local/bin/openclaw")) - }); + let entry = preset_catalog_entry( + openclaw, + |cmd| { + (cmd == openclaw.id || cmd == "openclaw") + .then(|| std::path::PathBuf::from("/usr/local/bin/openclaw")) + }, + None, + ); assert_eq!( entry.max_parallelism, Some(crate::managed_agents::parallelism::OPENCLAW_MAX_PARALLELISM), @@ -595,7 +689,7 @@ mod tests { .iter() .find(|p| p.id == "devin") .expect("devin preset must be present"); - let entry = preset_catalog_entry(devin, |_| None); + let entry = preset_catalog_entry(devin, |_| None, None); assert_eq!( entry.max_parallelism, None, "uncapped preset (devin) must have max_parallelism: None" From a64e81acc03595dd068d34c76c99ced93fd26867 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Fri, 4 Sep 2026 13:32:18 -0400 Subject: [PATCH 4/7] Make Pi command override Buzz-owned Signed-off-by: Salman Mohammed --- crates/buzz-acp/src/acp.rs | 29 +-- crates/buzz-acp/src/lib.rs | 19 +- crates/buzz-acp/src/pi_launcher.rs | 29 ++- .../src-tauri/src/managed_agents/discovery.rs | 8 +- .../src/managed_agents/discovery/presets.rs | 168 ++++-------------- .../src/managed_agents/env_vars/tests.rs | 8 + .../src/managed_agents/reserved_env_keys.rs | 3 + 7 files changed, 72 insertions(+), 192 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index b1e92d5cf7f..e45b6a2f03b 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -508,10 +508,9 @@ impl AcpClient { // Handled by build_codex_config_env; skip here to avoid double-setting. continue; } - let target_key = extra_env_target_key(key); - let parent_has_value = std::env::var_os(target_key).is_some(); + let parent_has_value = std::env::var_os(key).is_some(); if should_apply_extra_env(key, parent_has_value) { - cmd.env(target_key, value); + cmd.env(key, value); } } if let Some(merged) = codex_config_value { @@ -2042,19 +2041,10 @@ impl AcpClient { } } -/// Translate Buzz's internal forced Pi launcher input to the environment key -/// consumed by `pi-acp`. All real adapter environment keys continue through -/// the ordinary operator-wins path. -fn extra_env_target_key(key: &str) -> &str { - if key == crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV { - crate::pi_launcher::PI_ACP_PI_COMMAND_ENV - } else { - key - } -} - fn should_apply_extra_env(key: &str, parent_has_value: bool) -> bool { - key == crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV || !parent_has_value + // buzz-acp strips user-provided copies and injects only its generated Pi + // launcher under this key, so it must override any inherited process value. + key == crate::pi_launcher::PI_ACP_PI_COMMAND_ENV || !parent_has_value } /// Build `session/prompt` params from one or more text content blocks. @@ -2415,13 +2405,8 @@ mod tests { #[test] fn buzz_pi_launcher_overrides_inherited_adapter_command() { - assert_eq!( - extra_env_target_key(crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV), - crate::pi_launcher::PI_ACP_PI_COMMAND_ENV - ); - assert_eq!(extra_env_target_key("ORDINARY_ENV"), "ORDINARY_ENV"); assert!(should_apply_extra_env( - crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV, + crate::pi_launcher::PI_ACP_PI_COMMAND_ENV, true )); assert!(!should_apply_extra_env("ORDINARY_ENV", true)); @@ -3162,7 +3147,7 @@ mod tests { async fn spawn_forces_generated_pi_launcher_into_adapter_environment() { let launcher = "/tmp/buzz-generated-pi-launcher"; let extra_env = [( - crate::pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), + crate::pi_launcher::PI_ACP_PI_COMMAND_ENV.to_string(), launcher.to_string(), )]; diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index a8efe88ca24..a649cb50a85 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2528,30 +2528,21 @@ async fn tokio_main() -> Result<()> { ), ) }; - let configured_pi_command = config + // PI_ACP_PI_COMMAND is Buzz-owned. Strip stale/user-provided copies from + // every adapter before optionally installing Buzz's generated Pi launcher. + config .persona_env_vars - .iter() - .rev() - .find(|(key, _)| key == pi_launcher::PI_ACP_PI_COMMAND_ENV) - .map(|(_, value)| value.clone()); + .retain(|(key, _)| !key.eq_ignore_ascii_case(pi_launcher::PI_ACP_PI_COMMAND_ENV)); let managed_skills_dir = std::path::Path::new(&cwd).join(".agents/skills"); let (pi_launch_override, base_prompt) = pi_launcher::PiLaunchOverride::prepare( &config.agent_command, base_prompt, - configured_pi_command.as_deref(), &managed_skills_dir, ) .context("failed to prepare Pi launch overrides")?; if let Some(prepared) = pi_launch_override.as_ref() { - // The wrapper invokes the effective inherited/persona Pi command, so - // remove any persona copy before installing the one forced adapter - // override that pi-acp must observe. - config.persona_env_vars.retain(|(key, _)| { - key != pi_launcher::PI_ACP_PI_COMMAND_ENV - && key != pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV - }); config.persona_env_vars.push(( - pi_launcher::BUZZ_PI_LAUNCHER_OVERRIDE_ENV.to_string(), + pi_launcher::PI_ACP_PI_COMMAND_ENV.to_string(), prepared.launcher_path().to_string_lossy().into_owned(), )); tracing::info!( diff --git a/crates/buzz-acp/src/pi_launcher.rs b/crates/buzz-acp/src/pi_launcher.rs index 7efee1f04aa..7ca35b64636 100644 --- a/crates/buzz-acp/src/pi_launcher.rs +++ b/crates/buzz-acp/src/pi_launcher.rs @@ -17,10 +17,6 @@ use std::ffi::OsStr; use uuid::Uuid; pub(crate) const PI_ACP_PI_COMMAND_ENV: &str = "PI_ACP_PI_COMMAND"; -/// Internal `AcpClient` input translated to `PI_ACP_PI_COMMAND` with forced -/// precedence. Keeping this distinct preserves normal operator-wins behavior -/// for every actual adapter environment variable. -pub(crate) const BUZZ_PI_LAUNCHER_OVERRIDE_ENV: &str = "BUZZ_ACP_INTERNAL_PI_LAUNCHER_OVERRIDE"; /// Files backing the Pi launcher for one `buzz-acp` process. /// @@ -40,23 +36,16 @@ impl PiLaunchOverride { pub(crate) fn prepare( agent_command: &str, base_prompt: Option, - configured_pi_command: Option<&str>, managed_skills_dir: &Path, ) -> io::Result<(Option, Option)> { if crate::config::normalize_agent_command_identity(agent_command) != "pi-acp" { return Ok((None, base_prompt)); } - // Match AcpClient's normal environment precedence: an inherited - // operator value wins over persona/default environment. The generated - // launcher then becomes pi-acp's command and invokes that effective - // value as the real Pi executable. - let inherited_pi_command = std::env::var(PI_ACP_PI_COMMAND_ENV).ok(); - let pi_command = inherited_pi_command - .as_deref() - .or(configured_pi_command) - .unwrap_or("pi"); - let prepared = Self::create(pi_command, base_prompt.as_deref(), managed_skills_dir)?; + // Buzz owns PI_ACP_PI_COMMAND and always uses it to point pi-acp at + // this generated launcher. The launcher resolves the ordinary `pi` + // command from Buzz's effective PATH. + let prepared = Self::create("pi", base_prompt.as_deref(), managed_skills_dir)?; Ok((Some(prepared), None)) } @@ -246,7 +235,7 @@ mod tests { fn non_pi_adapter_keeps_base_prompt_for_acp_delivery() { let base = Some("Buzz base".to_string()); let (prepared, remaining) = - PiLaunchOverride::prepare("goose", base.clone(), None, Path::new("/unused/skills")) + PiLaunchOverride::prepare("goose", base.clone(), Path::new("/unused/skills")) .expect("prepare"); assert!(prepared.is_none()); assert_eq!(remaining, base); @@ -255,7 +244,7 @@ mod tests { #[test] fn disabled_base_prompt_still_creates_pi_skills_launcher() { let (prepared, remaining) = - PiLaunchOverride::prepare("pi-acp", None, None, Path::new("/unused/skills")) + PiLaunchOverride::prepare("pi-acp", None, Path::new("/unused/skills")) .expect("prepare"); let prepared = prepared.expect("Pi skills launcher"); assert!(remaining.is_none()); @@ -274,7 +263,6 @@ mod tests { let (prepared, remaining) = PiLaunchOverride::prepare( "/opt/bin/pi-acp", Some(base.clone()), - Some("alternate-pi"), Path::new("/buzz/.agents/skills"), ) .expect("prepare"); @@ -286,6 +274,11 @@ mod tests { base ); assert!(base.contains("each thread gets its own")); + + #[cfg(unix)] + assert!(fs::read_to_string(prepared.launcher_path()) + .expect("read launcher") + .contains("exec 'pi'")); } #[cfg(unix)] diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index d613f21e612..531ae335ce5 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -1130,8 +1130,6 @@ pub fn discover_acp_runtimes_from( // Track all ids seen so far (builtins) to prevent preset/custom collisions. let mut seen_ids: std::collections::HashSet = entries.iter().map(|e| e.id.clone()).collect(); - let pi_command_override = std::env::var(presets::PI_ACP_PI_COMMAND_ENV).ok(); - // Phase 2.5: insert static preset entries (PATH-probed, not editable/deletable). for def in PRESET_HARNESSES { if seen_ids.contains(def.id) { @@ -1140,11 +1138,7 @@ pub fn discover_acp_runtimes_from( } seen_ids.insert(def.id.to_string()); - entries.push(preset_catalog_entry( - def, - resolve, - pi_command_override.as_deref(), - )); + entries.push(preset_catalog_entry(def, resolve)); } // Phase 3: load and append custom harness definitions. diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index aeac261b869..b184559c157 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -7,8 +7,6 @@ use crate::managed_agents::{ use super::normalize_agent_args; -pub(super) const PI_ACP_PI_COMMAND_ENV: &str = "PI_ACP_PI_COMMAND"; - /// Static data for a well-known tier-2 ACP harness. pub(super) struct PresetHarness { pub(super) id: &'static str, @@ -28,17 +26,9 @@ pub(super) struct PresetHarness { pub(super) fn preset_catalog_entry( def: &PresetHarness, resolve: impl Fn(&str) -> Option, - pi_command_override: Option<&str>, ) -> AcpRuntimeCatalogEntry { - // pi-acp treats PI_ACP_PI_COMMAND as the authoritative Pi executable. - // Resolve that exact value when configured instead of requiring the bare - // `pi` command to be on PATH; launch uses the same precedence. - let underlying_cli_command = if def.id == "pi" { - pi_command_override.or(def.underlying_cli) - } else { - def.underlying_cli - }; - let underlying_cli_path = underlying_cli_command + let underlying_cli_path = def + .underlying_cli .and_then(&resolve) .map(|path| path.display().to_string()); let (availability, command, binary_path) = super::classify_runtime( @@ -367,11 +357,9 @@ mod tests { assert_eq!(preset.underlying_cli, None); assert_eq!(preset.install_instructions_url, "https://docs.devin.ai/cli"); - let entry = preset_catalog_entry( - preset, - |command| (command == "devin").then(|| PathBuf::from("/usr/local/bin/devin")), - None, - ); + let entry = preset_catalog_entry(preset, |command| { + (command == "devin").then(|| PathBuf::from("/usr/local/bin/devin")) + }); assert_eq!(entry.availability, AcpAvailabilityStatus::Available); assert_eq!(entry.command.as_deref(), Some("devin")); assert_eq!(entry.default_args, vec!["acp"]); @@ -379,7 +367,7 @@ mod tests { assert_eq!(entry.auth_status, AuthStatus::NotApplicable); assert_eq!(entry.source, HarnessSource::Preset); - let missing_entry = preset_catalog_entry(preset, |_| None, None); + let missing_entry = preset_catalog_entry(preset, |_| None); assert_eq!( missing_entry.availability, AcpAvailabilityStatus::NotInstalled @@ -408,68 +396,6 @@ mod tests { assert_eq!(entry.source, HarnessSource::Preset); } - #[cfg(unix)] - #[test] - fn pi_runtime_catalog_honors_command_override_outside_path() { - use std::os::unix::fs::PermissionsExt; - - use crate::managed_agents::custom_harnesses::registry_test_lock; - use crate::managed_agents::discovery::clear_resolve_cache; - - // Discovery reads process environment and mutates shared resolution - // caches, so serialize this production-path check with its peers. - let _path_guard = crate::managed_agents::lock_path_mutex(); - let _registry_guard = registry_test_lock(); - - let temp = tempfile::tempdir().expect("tempdir"); - let path_dir = temp.path().join("path"); - let override_dir = temp.path().join("override"); - std::fs::create_dir_all(&path_dir).expect("create PATH dir"); - std::fs::create_dir_all(&override_dir).expect("create override dir"); - - let adapter = path_dir.join("pi-acp"); - let configured_pi = override_dir.join("custom-pi"); - for executable in [&adapter, &configured_pi] { - std::fs::write(executable, "#!/bin/sh\nexit 0\n").expect("write fake executable"); - std::fs::set_permissions(executable, std::fs::Permissions::from_mode(0o755)) - .expect("make fake executable runnable"); - } - - let old_path = std::env::var_os("PATH").unwrap_or_default(); - let old_override = std::env::var_os(super::PI_ACP_PI_COMMAND_ENV); - let path = - std::env::join_paths(std::iter::once(path_dir).chain(std::env::split_paths(&old_path))) - .expect("join PATH"); - std::env::set_var("PATH", path); - std::env::set_var(super::PI_ACP_PI_COMMAND_ENV, &configured_pi); - clear_resolve_cache(); - - let result = std::panic::catch_unwind(|| { - let pi = super::super::discover_acp_runtimes_from(None, true) - .into_iter() - .find(|entry| entry.id == "pi") - .expect("Pi preset should appear in runtime catalog"); - - assert_eq!(pi.availability, AcpAvailabilityStatus::Available); - assert_eq!(pi.command.as_deref(), Some("pi-acp")); - assert_eq!( - pi.underlying_cli_path.as_deref(), - Some(configured_pi.to_string_lossy().as_ref()), - "catalog must resolve PI_ACP_PI_COMMAND rather than the bare pi command" - ); - }); - - std::env::set_var("PATH", old_path); - match old_override { - Some(value) => std::env::set_var(super::PI_ACP_PI_COMMAND_ENV, value), - None => std::env::remove_var(super::PI_ACP_PI_COMMAND_ENV), - } - clear_resolve_cache(); - if let Err(error) = result { - std::panic::resume_unwind(error); - } - } - #[test] fn pi_preset_uses_zero_arg_adapter_and_reports_missing_component() { let preset = PRESET_HARNESSES @@ -482,15 +408,11 @@ mod tests { assert!(preset.args.is_empty()); assert_eq!(preset.underlying_cli, Some("pi")); - let available = preset_catalog_entry( - preset, - |command| match command { - "pi-acp" => Some(PathBuf::from("/usr/local/bin/pi-acp")), - "pi" => Some(PathBuf::from("/usr/local/bin/pi")), - _ => None, - }, - None, - ); + let available = preset_catalog_entry(preset, |command| match command { + "pi-acp" => Some(PathBuf::from("/usr/local/bin/pi-acp")), + "pi" => Some(PathBuf::from("/usr/local/bin/pi")), + _ => None, + }); assert_eq!(available.availability, AcpAvailabilityStatus::Available); assert_eq!(available.command.as_deref(), Some("pi-acp")); assert!(available.default_args.is_empty()); @@ -501,11 +423,9 @@ mod tests { Some("/usr/local/bin/pi") ); - let adapter_missing = preset_catalog_entry( - preset, - |command| (command == "pi").then(|| PathBuf::from("/usr/local/bin/pi")), - None, - ); + let adapter_missing = preset_catalog_entry(preset, |command| { + (command == "pi").then(|| PathBuf::from("/usr/local/bin/pi")) + }); assert_eq!( adapter_missing.availability, AcpAvailabilityStatus::AdapterMissing @@ -521,11 +441,9 @@ mod tests { "https://github.com/svkozak/pi-acp" ); - let cli_missing = preset_catalog_entry( - preset, - |command| (command == "pi-acp").then(|| PathBuf::from("/usr/local/bin/pi-acp")), - None, - ); + let cli_missing = preset_catalog_entry(preset, |command| { + (command == "pi-acp").then(|| PathBuf::from("/usr/local/bin/pi-acp")) + }); assert_eq!(cli_missing.availability, AcpAvailabilityStatus::CliMissing); assert_eq!(cli_missing.command.as_deref(), Some("pi-acp")); assert_eq!( @@ -537,7 +455,7 @@ mod tests { "https://github.com/badlogic/pi-mono/tree/main/packages/coding-agent" ); - let not_installed = preset_catalog_entry(preset, |_| None, None); + let not_installed = preset_catalog_entry(preset, |_| None); assert_eq!( not_installed.availability, AcpAvailabilityStatus::NotInstalled @@ -550,11 +468,9 @@ mod tests { #[test] fn adapter_missing_when_underlying_cli_present() { - let entry = preset_catalog_entry( - &ADAPTER_PRESET, - |command| (command == "amp").then(|| PathBuf::from("/usr/local/bin/amp")), - None, - ); + let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| { + (command == "amp").then(|| PathBuf::from("/usr/local/bin/amp")) + }); assert_eq!(entry.availability, AcpAvailabilityStatus::AdapterMissing); assert!(entry.command.is_none()); assert!(entry.binary_path.is_none()); @@ -572,7 +488,7 @@ mod tests { #[test] fn not_installed_when_adapter_and_cli_are_missing() { - let entry = preset_catalog_entry(&ADAPTER_PRESET, |_| None, None); + let entry = preset_catalog_entry(&ADAPTER_PRESET, |_| None); assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); assert!(entry.underlying_cli_path.is_none()); assert!(entry.requires_external_cli); @@ -585,15 +501,11 @@ mod tests { #[test] fn available_when_adapter_and_cli_are_present() { - let entry = preset_catalog_entry( - &ADAPTER_PRESET, - |command| match command { - "amp-acp" => Some(PathBuf::from("/usr/local/bin/amp-acp")), - "amp" => Some(PathBuf::from("/usr/local/bin/amp")), - _ => None, - }, - None, - ); + let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| match command { + "amp-acp" => Some(PathBuf::from("/usr/local/bin/amp-acp")), + "amp" => Some(PathBuf::from("/usr/local/bin/amp")), + _ => None, + }); assert_eq!(entry.availability, AcpAvailabilityStatus::Available); assert_eq!(entry.command.as_deref(), Some("amp-acp")); assert_eq!(entry.binary_path.as_deref(), Some("/usr/local/bin/amp-acp")); @@ -606,11 +518,9 @@ mod tests { #[test] fn adapter_without_underlying_cli_reports_cli_missing() { - let entry = preset_catalog_entry( - &ADAPTER_PRESET, - |command| (command == "amp-acp").then(|| PathBuf::from("/usr/local/bin/amp-acp")), - None, - ); + let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| { + (command == "amp-acp").then(|| PathBuf::from("/usr/local/bin/amp-acp")) + }); assert_eq!(entry.availability, AcpAvailabilityStatus::CliMissing); assert_eq!(entry.command.as_deref(), Some("amp-acp")); assert_eq!(entry.binary_path.as_deref(), Some("/usr/local/bin/amp-acp")); @@ -625,7 +535,7 @@ mod tests { underlying_cli: None, ..ADAPTER_PRESET }; - let entry = preset_catalog_entry(&preset, |_| None, None); + let entry = preset_catalog_entry(&preset, |_| None); assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); assert!(!entry.requires_external_cli); assert!(entry.underlying_cli_path.is_none()); @@ -644,7 +554,7 @@ mod tests { .expect("openclaw preset must be present"); // Simulate "not installed" — resolver always returns None. - let entry = preset_catalog_entry(openclaw, |_| None, None); + let entry = preset_catalog_entry(openclaw, |_| None); assert_eq!(entry.availability, AcpAvailabilityStatus::NotInstalled); assert!( entry.command.is_none(), @@ -666,14 +576,10 @@ mod tests { .find(|p| p.id == "openclaw") .expect("openclaw preset must be present"); - let entry = preset_catalog_entry( - openclaw, - |cmd| { - (cmd == openclaw.id || cmd == "openclaw") - .then(|| std::path::PathBuf::from("/usr/local/bin/openclaw")) - }, - None, - ); + let entry = preset_catalog_entry(openclaw, |cmd| { + (cmd == openclaw.id || cmd == "openclaw") + .then(|| std::path::PathBuf::from("/usr/local/bin/openclaw")) + }); assert_eq!( entry.max_parallelism, Some(crate::managed_agents::parallelism::OPENCLAW_MAX_PARALLELISM), @@ -689,7 +595,7 @@ mod tests { .iter() .find(|p| p.id == "devin") .expect("devin preset must be present"); - let entry = preset_catalog_entry(devin, |_| None, None); + let entry = preset_catalog_entry(devin, |_| None); assert_eq!( entry.max_parallelism, None, "uncapped preset (devin) must have max_parallelism: None" diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index dc38c3d126f..7aa886c672a 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -145,6 +145,14 @@ fn reserved_keys_include_agent_owner_for_legacy_records() { assert!(merged.is_empty()); } +#[test] +fn reserved_keys_include_pi_acp_command() { + assert!(is_reserved_env_key("PI_ACP_PI_COMMAND")); + let agent = map(&[("PI_ACP_PI_COMMAND", "/tmp/custom-pi")]); + let merged = merged_user_env(&BTreeMap::new(), &agent); + assert!(merged.is_empty()); +} + #[test] fn reserved_keys_include_respond_to_gate() { // Respond-to mode + allowlist control who the agent answers. diff --git a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs index c01d29f3c2a..107171b3d47 100644 --- a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs +++ b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs @@ -41,6 +41,9 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_ACP_AGENT_COMMAND", "BUZZ_ACP_AGENT_ARGS", "BUZZ_ACP_MCP_COMMAND", + // pi-acp's executable override is reserved for Buzz's generated launcher, + // which injects the managed system prompt and skills. + "PI_ACP_PI_COMMAND", // Control-plane parallelism: the Desktop resolves the effective // worker-pool size (applying any per-harness cap) and writes it into // launch.policy_env. A user-supplied BUZZ_ACP_AGENTS would bypass the From fac789e82a0e7e5429a79e77e84508a86fd450b8 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Fri, 4 Sep 2026 14:00:43 -0400 Subject: [PATCH 5/7] Reject inherited Pi command overrides Signed-off-by: Salman Mohammed --- crates/buzz-acp/src/acp.rs | 39 +----------------------------- crates/buzz-acp/src/lib.rs | 3 +++ crates/buzz-acp/src/pi_launcher.rs | 28 +++++++++++++++++++-- 3 files changed, 30 insertions(+), 40 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index e45b6a2f03b..d52ca875679 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -509,7 +509,7 @@ impl AcpClient { continue; } let parent_has_value = std::env::var_os(key).is_some(); - if should_apply_extra_env(key, parent_has_value) { + if !parent_has_value { cmd.env(key, value); } } @@ -2041,12 +2041,6 @@ impl AcpClient { } } -fn should_apply_extra_env(key: &str, parent_has_value: bool) -> bool { - // buzz-acp strips user-provided copies and injects only its generated Pi - // launcher under this key, so it must override any inherited process value. - key == crate::pi_launcher::PI_ACP_PI_COMMAND_ENV || !parent_has_value -} - /// Build `session/prompt` params from one or more text content blocks. fn build_prompt_params(session_id: &str, prompt_blocks: &[&str]) -> serde_json::Value { let blocks: Vec = prompt_blocks @@ -2403,16 +2397,6 @@ mod tests { assert_eq!(StopReason::from_str("Refusal"), Some(StopReason::Refusal)); } - #[test] - fn buzz_pi_launcher_overrides_inherited_adapter_command() { - assert!(should_apply_extra_env( - crate::pi_launcher::PI_ACP_PI_COMMAND_ENV, - true - )); - assert!(!should_apply_extra_env("ORDINARY_ENV", true)); - assert!(should_apply_extra_env("ORDINARY_ENV", false)); - } - #[test] fn find_allow_once_by_kind_not_by_option_id() { // optionId values are intentionally non-obvious to prove we don't hardcode them. @@ -3142,27 +3126,6 @@ mod tests { ); } - #[cfg(unix)] - #[tokio::test] - async fn spawn_forces_generated_pi_launcher_into_adapter_environment() { - let launcher = "/tmp/buzz-generated-pi-launcher"; - let extra_env = [( - crate::pi_launcher::PI_ACP_PI_COMMAND_ENV.to_string(), - launcher.to_string(), - )]; - - assert_eq!( - spawn_named_and_read_child_env( - "pi-acp", - crate::pi_launcher::PI_ACP_PI_COMMAND_ENV, - &extra_env, - ) - .await, - launcher, - "pi-acp must observe Buzz's generated launcher even when the parent has PI_ACP_PI_COMMAND" - ); - } - #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index a649cb50a85..36b2e43518b 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2534,10 +2534,13 @@ async fn tokio_main() -> Result<()> { .persona_env_vars .retain(|(key, _)| !key.eq_ignore_ascii_case(pi_launcher::PI_ACP_PI_COMMAND_ENV)); let managed_skills_dir = std::path::Path::new(&cwd).join(".agents/skills"); + let inherited_pi_command_is_set = + std::env::var_os(pi_launcher::PI_ACP_PI_COMMAND_ENV).is_some(); let (pi_launch_override, base_prompt) = pi_launcher::PiLaunchOverride::prepare( &config.agent_command, base_prompt, &managed_skills_dir, + inherited_pi_command_is_set, ) .context("failed to prepare Pi launch overrides")?; if let Some(prepared) = pi_launch_override.as_ref() { diff --git a/crates/buzz-acp/src/pi_launcher.rs b/crates/buzz-acp/src/pi_launcher.rs index 7ca35b64636..892500bb200 100644 --- a/crates/buzz-acp/src/pi_launcher.rs +++ b/crates/buzz-acp/src/pi_launcher.rs @@ -37,11 +37,19 @@ impl PiLaunchOverride { agent_command: &str, base_prompt: Option, managed_skills_dir: &Path, + inherited_pi_command_is_set: bool, ) -> io::Result<(Option, Option)> { if crate::config::normalize_agent_command_identity(agent_command) != "pi-acp" { return Ok((None, base_prompt)); } + if inherited_pi_command_is_set { + return Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "PI_ACP_PI_COMMAND is managed by Buzz; unset it before starting a managed Pi agent", + )); + } + // Buzz owns PI_ACP_PI_COMMAND and always uses it to point pi-acp at // this generated launcher. The launcher resolves the ordinary `pi` // command from Buzz's effective PATH. @@ -235,16 +243,31 @@ mod tests { fn non_pi_adapter_keeps_base_prompt_for_acp_delivery() { let base = Some("Buzz base".to_string()); let (prepared, remaining) = - PiLaunchOverride::prepare("goose", base.clone(), Path::new("/unused/skills")) + PiLaunchOverride::prepare("goose", base.clone(), Path::new("/unused/skills"), true) .expect("prepare"); assert!(prepared.is_none()); assert_eq!(remaining, base); } + #[test] + fn pi_adapter_rejects_inherited_pi_command() { + let error = PiLaunchOverride::prepare( + "pi-acp", + Some("Buzz base".to_string()), + Path::new("/unused/skills"), + true, + ) + .err() + .expect("inherited PI_ACP_PI_COMMAND must be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::AlreadyExists); + assert!(error.to_string().contains("managed by Buzz")); + } + #[test] fn disabled_base_prompt_still_creates_pi_skills_launcher() { let (prepared, remaining) = - PiLaunchOverride::prepare("pi-acp", None, Path::new("/unused/skills")) + PiLaunchOverride::prepare("pi-acp", None, Path::new("/unused/skills"), false) .expect("prepare"); let prepared = prepared.expect("Pi skills launcher"); assert!(remaining.is_none()); @@ -264,6 +287,7 @@ mod tests { "/opt/bin/pi-acp", Some(base.clone()), Path::new("/buzz/.agents/skills"), + false, ) .expect("prepare"); let prepared = prepared.expect("Pi launcher"); From d4315354bf0411e85ae51112131aae24dc05ca02 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Fri, 4 Sep 2026 14:11:16 -0400 Subject: [PATCH 6/7] Restore concise ACP environment check Signed-off-by: Salman Mohammed --- crates/buzz-acp/src/acp.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index d52ca875679..0d87bca028c 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -508,8 +508,7 @@ impl AcpClient { // Handled by build_codex_config_env; skip here to avoid double-setting. continue; } - let parent_has_value = std::env::var_os(key).is_some(); - if !parent_has_value { + if std::env::var_os(key).is_none() { cmd.env(key, value); } } From a3d567823cf0bc2fb87f5fb58800a07660a0526b Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Fri, 4 Sep 2026 16:08:53 -0400 Subject: [PATCH 7/7] Clarify Buzz agent identity Signed-off-by: Salman Mohammed --- crates/buzz-acp/src/base_prompt.md | 3 ++- crates/buzz-acp/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/buzz-acp/src/base_prompt.md b/crates/buzz-acp/src/base_prompt.md index 3df5aa0f6f7..305d5b8880d 100644 --- a/crates/buzz-acp/src/base_prompt.md +++ b/crates/buzz-acp/src/base_prompt.md @@ -1,4 +1,5 @@ -You are a managed agent in Buzz, a shared workspace where humans and agents collaborate as colleagues. Buzz is a desktop and mobile collaboration app organized around channels, conversations, and shared work. +You are an agent operating inside Buzz — a Nostr-based messaging platform for human-agent collaboration. +Buzz is a desktop and mobile collaboration app organized around channels, conversations, and shared work. ## Buzz CLI diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 36b2e43518b..ddd594b142d 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -5242,7 +5242,7 @@ mod agent_draft_prompt_tests { fn shared_base_prompt_teaches_portable_agent_drafts() { let prompt = include_str!("base_prompt.md"); assert!(prompt.starts_with( - "You are a managed agent in Buzz, a shared workspace where humans and agents collaborate as colleagues. Buzz is a desktop and mobile collaboration app organized around channels, conversations, and shared work." + "You are an agent operating inside Buzz — a Nostr-based messaging platform for human-agent collaboration.\nBuzz is a desktop and mobile collaboration app organized around channels, conversations, and shared work." )); assert!(prompt.contains("buzz agents draft-create")); assert!(prompt.contains("ask for at most two things"));