From 3ae55097293385f1960ef825d2219e4c5d434100 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Tue, 8 Sep 2026 21:56:44 -0400 Subject: [PATCH 1/5] fix(acp): deliver Pi session prompts through the fork Signed-off-by: Salman Mohammed --- crates/buzz-acp/TESTING.md | 56 +++ crates/buzz-acp/src/acp.rs | 150 ++----- .../buzz-acp/src/acp/system_prompt_tests.rs | 117 ++++++ crates/buzz-acp/src/lib.rs | 65 +-- crates/buzz-acp/src/pi_launcher.rs | 380 ------------------ crates/buzz-acp/src/pool.rs | 70 +--- crates/buzz-acp/src/pool/pi_prompt_tests.rs | 321 +++++++++++++++ .../buzz-acp/src/pool/system_prompt_tests.rs | 50 +++ .../src-tauri/src/managed_agents/discovery.rs | 1 + .../src/managed_agents/discovery/presets.rs | 154 ++----- .../src/managed_agents/env_vars/tests.rs | 8 - .../src/managed_agents/reserved_env_keys.rs | 3 - .../agents/ui/agentSessionTranscript.ts | 7 +- .../ui/agentSessionTranscriptPi.test.mjs | 43 ++ .../settings/ui/harnessCatalogCopy.ts | 2 +- 15 files changed, 697 insertions(+), 730 deletions(-) create mode 100644 crates/buzz-acp/TESTING.md create mode 100644 crates/buzz-acp/src/acp/system_prompt_tests.rs delete mode 100644 crates/buzz-acp/src/pi_launcher.rs create mode 100644 crates/buzz-acp/src/pool/pi_prompt_tests.rs create mode 100644 crates/buzz-acp/src/pool/system_prompt_tests.rs create mode 100644 desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs diff --git a/crates/buzz-acp/TESTING.md b/crates/buzz-acp/TESTING.md new file mode 100644 index 00000000000..d44035e7785 --- /dev/null +++ b/crates/buzz-acp/TESTING.md @@ -0,0 +1,56 @@ +# Pi adapter integration + +Buzz's Pi preset uses [salman1993/pi-acp](https://github.com/salman1993/pi-acp). +Install Pi separately, then build and install the adapter from source: + +```sh +npm install -g --ignore-scripts @earendil-works/pi-coding-agent +git clone https://github.com/salman1993/pi-acp.git +cd pi-acp +git checkout b893ff9241c35fd04f27b0e9dbfa2f7bc463fc42 +npm ci +npm run build +npm install -g . +``` + +Keep that checkout if npm links the global executable to it. Direct GitHub npm +installation at this revision does not build `dist/index.js`. The unscoped +`npm install -g pi-acp` command installs the upstream package, without these +extensions. Restart managed Pi agents after installing the fork; use fresh +sessions to replace old user-framed standing instructions. + +Buzz adds `-- --skill /.agents/skills` when launching `pi-acp`. +An existing separator and explicit Pi options are preserved. Managed agents +run from the Buzz workspace, so the default directory is its `.agents/skills`. +The path is fixed at adapter launch and applies to every Pi subprocess. + +The full composed session prompt is sent as a replacement string through +`_meta.systemPrompt` only when Pi advertises both `replace` and `persisted` +under `agentCapabilities._meta.piAcp.systemPrompt`. Older adapters retain +first-turn user framing. Session titles share `_meta.sessionTitle`. + +## Validation + +Activate Hermit from the Buzz repository root, then run the package tests: + +```sh +. ./bin/activate-hermit +cargo test -p buzz-acp +``` + +To exercise the real adapter through Buzz's production session composer: + +```sh +BUZZ_TEST_PI_ACP=/absolute/pi-acp/dist/index.js \ + cargo test -p buzz-acp real_pi_preserves -- --ignored +``` + +This test requires Node and Pi on PATH. It isolates HOME and Pi settings, +disables extensions and context files, and uses a synthetic transcript without +model calls. It inspects Pi's effective prompt through RPC HTML export after +switching sessions and restarting the adapter. Base, persona, team, core memory, +huddle, canvas, and the extra skill must each appear once, without another +session's instructions or Pi's default coding preamble. + +HTML export reports the exporting process's current system prompt. It cannot +recover a historical prompt from an old transcript alone. diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 0d87bca028c..c25d6b9c41d 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -200,6 +200,7 @@ pub struct AcpClient { /// a JSON-RPC *success*, not `-32601` — which the main loop would read as /// a delivered steer and drop the user's message from the queue. steering_supported: bool, + pi_system_prompt_supported: bool, /// Per-turn channel for receiving goose-native non-cancelling steer /// requests from the main loop. Installed by /// [`install_steer_rx`](Self::install_steer_rx) at dispatch and @@ -460,8 +461,15 @@ impl AcpClient { use std::process::Stdio; let mut cmd = tokio::process::Command::new(command); - cmd.args(args) - .stdin(Stdio::piped()) + cmd.args(args); + if crate::config::normalize_agent_command_identity(command) == "pi-acp" { + if !args.iter().any(|arg| arg == "--") { + cmd.arg("--"); + } + cmd.arg("--skill") + .arg(std::env::current_dir()?.join(".agents/skills")); + } + cmd.stdin(Stdio::piped()) .stdout(Stdio::piped()) // Inherit stderr so agent logs are visible in the harness terminal. .stderr(Stdio::inherit()) @@ -559,6 +567,7 @@ impl AcpClient { observer_context: ObserverContext::default(), active_run_id: None, steering_supported: false, + pi_system_prompt_supported: false, steer_rx: None, goose_usage: UsageTracker::default(), standard_usage: StandardUsageTracker::default(), @@ -617,10 +626,19 @@ impl AcpClient { .pointer("/_meta/steering/supported") .and_then(|v| v.as_bool()) .unwrap_or(false); + self.pi_system_prompt_supported = ["replace", "persisted"].iter().all(|key| { + result["agentCapabilities"]["_meta"]["piAcp"]["systemPrompt"][key].as_bool() + == Some(true) + }); tracing::debug!(target: "acp::init", "initialize response: {result}"); Ok(result) } + /// Whether Pi advertised persistent system-prompt replacement. + pub fn supports_pi_system_prompt(&self) -> bool { + self.pi_system_prompt_supported + } + /// Send the ACP `authenticate` request for an adapter-advertised method. pub async fn authenticate(&mut self, method_id: &str) -> Result { let params = serde_json::json!({ @@ -641,6 +659,9 @@ impl AcpClient { /// - `Some(SystemPromptTransport::ClaudeMeta(text))` — `_meta.systemPrompt` /// as `{"append": text}`, keeping claude-agent-acp's native preset intact. /// + /// - `Some(SystemPromptTransport::MetaReplace(text))` — `_meta.systemPrompt` + /// as a string, replacing Pi's native base prompt. + /// /// `session_title` rides in `_meta.sessionTitle` when `Some`; `_meta` is /// omitted entirely otherwise, since adapters may distinguish an absent /// member from a null one. When both `ClaudeMeta` and `session_title` are @@ -663,6 +684,9 @@ impl AcpClient { Some(SystemPromptTransport::Field(sp)) => { params["systemPrompt"] = serde_json::Value::String(sp.to_owned()); } + Some(SystemPromptTransport::MetaReplace(sp)) => { + params["_meta"]["systemPrompt"] = serde_json::Value::String(sp.to_owned()); + } Some(SystemPromptTransport::ClaudeMeta(sp)) => { // Merge into _meta so sessionTitle (set below) is not clobbered. params["_meta"]["systemPrompt"] = serde_json::json!({ "append": sp }); @@ -2131,8 +2155,6 @@ pub struct SessionNewResponse { /// How to deliver a system prompt on `session/new`. /// -/// The two variants match the two mechanisms supported by current adapters: -/// /// - **`Field`** — bare `systemPrompt` field (ACP protocol v2, buzz-agent). /// - **`ClaudeMeta`** — `_meta.systemPrompt: {"append": text}`, used by /// `claude-agent-acp` to append to the adapter's own native system prompt @@ -2143,6 +2165,8 @@ pub enum SystemPromptTransport<'a> { Field(&'a str), /// Deliver as `_meta.systemPrompt: {"append": text}`. ClaudeMeta(&'a str), + /// Deliver as `_meta.systemPrompt: text`, replacing the native base prompt. + MetaReplace(&'a str), } /// How to switch to a particular model on a session. @@ -3631,123 +3655,7 @@ mod tests { // ── claude-agent-acp _meta.systemPrompt transport ───────────────────── - #[tokio::test] - async fn session_new_full_sends_claude_meta_system_prompt_when_claude_meta_transport() { - // When ClaudeMeta transport is requested, the prompt must appear as - // _meta.systemPrompt: {"append": text} — never as a bare systemPrompt field. - let script = r#" - read -t 2 _init - echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' - read -t 2 REQ - echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_claude","_receivedRequest":'"$REQ"'}}' - sleep 1 - "#; - let mut client = spawn_script(script).await; - client - .initialize() - .await - .expect("initialize should succeed"); - - let resp = client - .session_new_full( - "/tmp", - vec![], - Some(SystemPromptTransport::ClaudeMeta("Be concise")), - None, - ) - .await - .expect("session_new_full should succeed"); - - let received = &resp.raw["_receivedRequest"]; - assert!( - received["params"].get("systemPrompt").is_none(), - "bare systemPrompt must not be present for ClaudeMeta transport" - ); - assert_eq!( - received["params"]["_meta"]["systemPrompt"]["append"].as_str(), - Some("Be concise"), - "_meta.systemPrompt.append must carry the prompt text" - ); - } - - #[tokio::test] - async fn session_new_full_merges_claude_meta_and_session_title_into_single_meta_object() { - // Both ClaudeMeta prompt and session_title must coexist under _meta — - // the prompt must not clobber sessionTitle or vice versa. - let script = r#" - read -t 2 _init - echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' - read -t 2 REQ - echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_merged","_receivedRequest":'"$REQ"'}}' - sleep 1 - "#; - let mut client = spawn_script(script).await; - client - .initialize() - .await - .expect("initialize should succeed"); - - let resp = client - .session_new_full( - "/tmp", - vec![], - Some(SystemPromptTransport::ClaudeMeta("Be concise")), - Some("Fizz · #buzz-dev"), - ) - .await - .expect("session_new_full should succeed"); - - let received = &resp.raw["_receivedRequest"]; - assert_eq!( - received["params"]["_meta"]["systemPrompt"]["append"].as_str(), - Some("Be concise"), - "_meta.systemPrompt.append must be present" - ); - assert_eq!( - received["params"]["_meta"]["sessionTitle"].as_str(), - Some("Fizz · #buzz-dev"), - "_meta.sessionTitle must be present alongside systemPrompt" - ); - } - - // ── Goose-native steer scaffold (PR follow-up to #1160) ────────────── - - /// Helper: spawn an inert `cat` subprocess so we have a real AcpClient - /// to drive `handle_session_update` against. `cat` never writes back, - /// which is fine — these tests don't read from the agent, they just - /// feed JSON into the parser. - async fn spawn_inert_client() -> AcpClient { - AcpClient::spawn("cat", &[], &[], false) - .await - .expect("spawn cat as inert client") - } - - /// Build a `session/update` JSON-RPC notification carrying a - /// `session_info_update` with the given `_meta.goose.activeRunId` value. - /// Pass `None` to omit the `activeRunId` field entirely. - /// - /// `_meta` is nested inside the `update` object (per the ACP - /// `SessionInfoUpdate` schema), matching what goose and buzz-agent - /// emit on the wire. - fn session_info_update_msg(active_run_id: Option) -> serde_json::Value { - let mut goose = serde_json::Map::new(); - if let Some(v) = active_run_id { - goose.insert("activeRunId".to_string(), v); - } - let mut meta = serde_json::Map::new(); - meta.insert("goose".to_string(), serde_json::Value::Object(goose)); - serde_json::json!({ - "jsonrpc": "2.0", - "method": "session/update", - "params": { - "sessionId": "test-session", - "update": { - "sessionUpdate": "session_info_update", - "_meta": serde_json::Value::Object(meta), - }, - } - }) - } + include!("acp/system_prompt_tests.rs"); #[tokio::test] async fn active_run_id_sets_on_string() { diff --git a/crates/buzz-acp/src/acp/system_prompt_tests.rs b/crates/buzz-acp/src/acp/system_prompt_tests.rs new file mode 100644 index 00000000000..6ed7b5ee783 --- /dev/null +++ b/crates/buzz-acp/src/acp/system_prompt_tests.rs @@ -0,0 +1,117 @@ +#[tokio::test] +async fn session_new_full_sends_claude_meta_system_prompt_when_claude_meta_transport() { + // When ClaudeMeta transport is requested, the prompt must appear as + // _meta.systemPrompt: {"append": text} — never as a bare systemPrompt field. + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ + echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_claude","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full( + "/tmp", + vec![], + Some(SystemPromptTransport::ClaudeMeta("Be concise")), + None, + ) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert!( + received["params"].get("systemPrompt").is_none(), + "bare systemPrompt must not be present for ClaudeMeta transport" + ); + assert_eq!( + received["params"]["_meta"]["systemPrompt"]["append"].as_str(), + Some("Be concise"), + "_meta.systemPrompt.append must carry the prompt text" + ); +} + +#[tokio::test] +async fn session_new_full_merges_claude_meta_and_session_title_into_single_meta_object() { + // Both ClaudeMeta prompt and session_title must coexist under _meta — + // the prompt must not clobber sessionTitle or vice versa. + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ + echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_merged","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full( + "/tmp", + vec![], + Some(SystemPromptTransport::ClaudeMeta("Be concise")), + Some("Fizz · #buzz-dev"), + ) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert_eq!( + received["params"]["_meta"]["systemPrompt"]["append"].as_str(), + Some("Be concise"), + "_meta.systemPrompt.append must be present" + ); + assert_eq!( + received["params"]["_meta"]["sessionTitle"].as_str(), + Some("Fizz · #buzz-dev"), + "_meta.sessionTitle must be present alongside systemPrompt" + ); +} + +// ── Goose-native steer scaffold (PR follow-up to #1160) ────────────── + +/// Helper: spawn an inert `cat` subprocess so we have a real AcpClient +/// to drive `handle_session_update` against. `cat` never writes back, +/// which is fine — these tests don't read from the agent, they just +/// feed JSON into the parser. +async fn spawn_inert_client() -> AcpClient { + AcpClient::spawn("cat", &[], &[], false) + .await + .expect("spawn cat as inert client") +} + +/// Build a `session/update` JSON-RPC notification carrying a +/// `session_info_update` with the given `_meta.goose.activeRunId` value. +/// Pass `None` to omit the `activeRunId` field entirely. +/// +/// `_meta` is nested inside the `update` object (per the ACP +/// `SessionInfoUpdate` schema), matching what goose and buzz-agent +/// emit on the wire. +fn session_info_update_msg(active_run_id: Option) -> serde_json::Value { + let mut goose = serde_json::Map::new(); + if let Some(v) = active_run_id { + goose.insert("activeRunId".to_string(), v); + } + let mut meta = serde_json::Map::new(); + meta.insert("goose".to_string(), serde_json::Value::Object(goose)); + serde_json::json!({ + "jsonrpc": "2.0", + "method": "session/update", + "params": { + "sessionId": "test-session", + "update": { + "sessionUpdate": "session_info_update", + "_meta": serde_json::Value::Object(meta), + }, + } + }) +} diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index cc2952a2f8d..12f241184a1 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -5,7 +5,6 @@ mod config; mod engram_fetch; mod filter; mod observer; -mod pi_launcher; mod pool; mod pool_lifecycle; mod prompt_framing; @@ -2511,49 +2510,6 @@ 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")), - ), - ) - }; - // 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 - .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() { - config.persona_env_vars.push(( - pi_launcher::PI_ACP_PI_COMMAND_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); @@ -2802,6 +2758,8 @@ 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(), @@ -2812,7 +2770,20 @@ 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, + 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")), + ), + ) + }, heartbeat_prompt: config.heartbeat_prompt.clone(), cwd, rest_client: relay.rest_client(), @@ -4166,10 +4137,6 @@ 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(()) } diff --git a/crates/buzz-acp/src/pi_launcher.rs b/crates/buzz-acp/src/pi_launcher.rs deleted file mode 100644 index 892500bb200..00000000000 --- a/crates/buzz-acp/src/pi_launcher.rs +++ /dev/null @@ -1,380 +0,0 @@ -//! 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 -//! `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"; - -/// 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, - 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. - let prepared = Self::create("pi", 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(), 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"), false) - .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()), - Path::new("/buzz/.agents/skills"), - false, - ) - .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)] - assert!(fs::read_to_string(prepared.launcher_path()) - .expect("read launcher") - .contains("exec 'pi'")); - } - - #[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/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index dbeafedda70..f26d9f74d18 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -299,6 +299,8 @@ fn has_system_prompt_support( ) -> bool { if agent_name == "goose" { goose_system_prompt_supported == Some(true) + } else if agent_name == "pi-acp" { + false // Pi uses an explicit capability, not its protocol version. } else if agent_name == CLAUDE_AGENT_ACP_NAME { true } else { @@ -311,8 +313,15 @@ fn session_new_system_prompt<'a>( protocol_version: u32, agent_name: &str, prompt: Option<&'a str>, + pi_system_prompt_supported: bool, ) -> Option> { - if is_goose || (protocol_version < 2 && agent_name != CLAUDE_AGENT_ACP_NAME) { + if is_goose { + None + } else if agent_name == "pi-acp" { + prompt + .filter(|_| pi_system_prompt_supported) + .map(SystemPromptTransport::MetaReplace) + } else if protocol_version < 2 && agent_name != CLAUDE_AGENT_ACP_NAME { None } else if agent_name == CLAUDE_AGENT_ACP_NAME { prompt.map(SystemPromptTransport::ClaudeMeta) @@ -323,6 +332,9 @@ fn session_new_system_prompt<'a>( impl OwnedAgent { pub(crate) fn has_system_prompt_support(&self) -> bool { + if self.agent_name == "pi-acp" { + return self.acp.supports_pi_system_prompt(); + } has_system_prompt_support( self.protocol_version, &self.agent_name, @@ -1517,6 +1529,7 @@ async fn create_session_and_apply_model( agent.protocol_version, &agent.agent_name, combined_system_prompt.as_deref(), + agent.acp.supports_pi_system_prompt(), ), session_title.as_deref(), ) @@ -5485,56 +5498,7 @@ mod tests { assert_eq!(composed, "\nbe helpful\n\n\ntick"); } - #[test] - fn goose_uses_system_prompt_only_after_custom_method_succeeds() { - assert!(!has_system_prompt_support(2, "goose", None)); - assert!(!has_system_prompt_support(2, "goose", Some(false))); - assert!(has_system_prompt_support(2, "goose", Some(true))); - assert!(has_system_prompt_support(1, "goose", Some(true))); - assert!(has_system_prompt_support(2, "buzz-agent", None)); - // Goose never receives system prompt via session/new (uses post-hoc method). - assert_eq!( - session_new_system_prompt(true, 2, "goose", Some("instructions")), - None - ); - // Protocol-v2 non-goose gets Field transport. - assert_eq!( - session_new_system_prompt(false, 2, "buzz-agent", Some("instructions")), - Some(SystemPromptTransport::Field("instructions")) - ); - // Protocol-v1 non-goose, non-claude gets None (legacy user-message framing). - assert_eq!( - session_new_system_prompt(false, 1, "codex", Some("instructions")), - None - ); - // claude-agent-acp gets ClaudeMeta transport regardless of protocol version. - assert_eq!( - session_new_system_prompt(false, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions")), - Some(SystemPromptTransport::ClaudeMeta("instructions")) - ); - assert_eq!( - session_new_system_prompt(true, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions")), - None, - "goose path must never produce a transport even when agent_name matches" - ); - } - - #[test] - fn claude_agent_acp_has_system_prompt_support_regardless_of_protocol_version() { - // claude-agent-acp declares protocolVersion:1 but supports _meta.systemPrompt; - // has_system_prompt_support must return true so user-message framing is suppressed. - assert!(has_system_prompt_support(1, CLAUDE_AGENT_ACP_NAME, None)); - assert!(has_system_prompt_support(2, CLAUDE_AGENT_ACP_NAME, None)); - } - - #[test] - fn old_zed_adapter_name_falls_through_to_protocol_version_gate() { - // The renamed @zed-industries package predates the _meta.systemPrompt support, - // so it must not be treated as capable and stays on legacy user-message framing. - let old_name = "@zed-industries/claude-code-acp"; - assert!(!has_system_prompt_support(1, old_name, None)); - assert!(has_system_prompt_support(2, old_name, None)); - } + include!("pool/system_prompt_tests.rs"); #[test] fn test_initial_message_legacy_agent_without_base_is_unchanged() { @@ -11214,3 +11178,7 @@ done"# ); } } + +#[cfg(all(test, unix))] +#[path = "pool/pi_prompt_tests.rs"] +mod pi_prompt_tests; diff --git a/crates/buzz-acp/src/pool/pi_prompt_tests.rs b/crates/buzz-acp/src/pool/pi_prompt_tests.rs new file mode 100644 index 00000000000..04955ce6fba --- /dev/null +++ b/crates/buzz-acp/src/pool/pi_prompt_tests.rs @@ -0,0 +1,321 @@ +use super::*; +use std::os::unix::fs::PermissionsExt; + +fn owned_pi(acp: AcpClient, protocol_version: u32) -> OwnedAgent { + OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_model_request_id: None, + desired_model_pending_ack: false, + startup_effort: None, + agent_name: "pi-acp".into(), + goose_system_prompt_supported: None, + protocol_version, + } +} + +fn fixture_dir() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("buzz pi transport {}", Uuid::new_v4())); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +fn script_at(dir: &std::path::Path, script: &str) -> std::path::PathBuf { + let path = dir.join("pi-acp"); + std::fs::write(&path, format!("#!/bin/bash\n{script}\n")).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o700)).unwrap(); + path +} + +#[tokio::test] +async fn pi_capability_controls_composed_prompt_and_legacy_framing() { + for (capability, supported) in [ + ( + serde_json::json!({"replace": true, "persisted": true}), + true, + ), + (serde_json::json!({"replace": true}), false), + ( + serde_json::json!({"append": true, "persisted": true}), + false, + ), + ( + serde_json::json!({"replace": "true", "persisted": true}), + false, + ), + (serde_json::Value::Null, false), + ] { + let dir = fixture_dir(); + let init = serde_json::json!({"jsonrpc":"2.0", "id":0, "result": { + "protocolVersion":2, "agentCapabilities":{"_meta":{"piAcp":{"systemPrompt":capability}}} + }}); + let path = script_at( + &dir, + &format!( + r#" + printf '%s\n' "$@" > '{dir}/args' + read -r request + echo '{init}' + read -r request + printf '%s\n' "$request" > '{dir}/request' + echo '{{"jsonrpc":"2.0","id":1,"result":{{"sessionId":"fixture"}}}}' + read -r request + "#, + dir = dir.display() + ), + ); + let mut acp = AcpClient::spawn(path.to_str().unwrap(), &[], &[], false) + .await + .unwrap(); + acp.initialize().await.unwrap(); + let mut agent = owned_pi(acp, 2); + assert_eq!(agent.has_system_prompt_support(), supported); + let mut ctx = tests::make_prompt_context_no_owner(); + ctx.base_prompt = Some("BUZZ_BASE".into()); + ctx.system_prompt = Some("BUZZ_PERSONA".into()); + ctx.team_instructions = Some("BUZZ_TEAM".into()); + ctx.session_title = Some("Pi fixture".into()); + let core = "BUZZ_CORE"; + let canvas = "BUZZ_CANVAS"; + create_session_and_apply_model( + &mut agent, + &ctx, + Some(core), + NewSessionChannelContext { + huddle_instructions: Some("BUZZ_HUDDLE"), + canvas: Some(canvas), + name: Some("channel"), + scope: None, + channel_type: None, + }, + ) + .await + .unwrap(); + agent.acp.shutdown().await; + let request: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(dir.join("request")).unwrap()).unwrap(); + let params = &request["params"]; + assert!(params.get("systemPrompt").is_none()); + assert!(params["_meta"]["sessionTitle"] + .as_str() + .unwrap() + .contains("Pi fixture")); + let standing = crate::queue::StandingContext { + base_prompt: ctx.base_prompt.as_deref(), + system_prompt: ctx.system_prompt.as_deref(), + team_instructions: ctx.team_instructions.as_deref(), + agent_core: Some(core), + huddle_instructions: Some("BUZZ_HUDDLE"), + agent_canvas: Some(canvas), + }; + let user = prepend_standing_for_legacy(if supported { 2 } else { 1 }, &standing, "EVENT"); + for marker in [ + "BUZZ_BASE", + "BUZZ_PERSONA", + "BUZZ_TEAM", + "BUZZ_CORE", + "BUZZ_HUDDLE", + "BUZZ_CANVAS", + ] { + if supported { + assert_eq!( + params["_meta"]["systemPrompt"] + .as_str() + .unwrap() + .matches(marker) + .count(), + 1 + ); + assert!(!user.contains(marker)); + } else { + assert!(params["_meta"].get("systemPrompt").is_none()); + assert_eq!(user.matches(marker).count(), 1); + } + } + let args = std::fs::read_to_string(dir.join("args")).unwrap(); + assert_eq!( + args, + format!( + "--\n--skill\n{}\n", + std::env::current_dir() + .unwrap() + .join(".agents/skills") + .display() + ) + ); + std::fs::remove_dir_all(dir).unwrap(); + } +} + +#[tokio::test] +async fn pi_launch_preserves_existing_forwarded_skills_and_one_separator() { + let dir = fixture_dir(); + let path = script_at( + &dir, + &format!("printf '%s\\n' \"$@\" > '{}/args'", dir.display()), + ); + let mut client = AcpClient::spawn( + path.to_str().unwrap(), + &["--".into(), "--skill".into(), "/extra skills".into()], + &[], + false, + ) + .await + .unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(5), client.initialize()) + .await + .unwrap() + .unwrap_err(); + client.shutdown().await; + assert_eq!( + std::fs::read_to_string(dir.join("args")).unwrap(), + format!( + "--\n--skill\n/extra skills\n--skill\n{}\n", + std::env::current_dir() + .unwrap() + .join(".agents/skills") + .display() + ) + ); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[tokio::test] +#[ignore = "requires BUZZ_TEST_PI_ACP pointing to a built fork and Pi on PATH"] +async fn real_pi_preserves_buzz_prompt_and_launch_skills_on_restore() { + use base64::Engine; + let adapter = std::env::var("BUZZ_TEST_PI_ACP").expect("set BUZZ_TEST_PI_ACP"); + let dir = fixture_dir(); + let home = dir.join("home"); + let workspace = dir.join("workspace"); + let skill = dir.join("extra skills"); + for path in [&home, &workspace, &skill] { + std::fs::create_dir_all(path).unwrap(); + } + std::fs::write( + skill.join("SKILL.md"), + "---\nname: buzz-fixture\ndescription: BUZZ_SKILL_MARKER\n---\nSynthetic instructions.\n", + ) + .unwrap(); + let path = script_at(&dir, &format!( + "export HOME='{}' PI_CODING_AGENT_DIR='{}/agent' ANTHROPIC_API_KEY=synthetic-test-key PI_ACP_PI_COMMAND=pi\nexec node '{}' \"$@\"", + home.display(), dir.display(), adapter.replace('\'', "'\\''") + )); + let args = vec![ + "--".into(), + "--offline".into(), + "--no-extensions".into(), + "--no-context-files".into(), + "--skill".into(), + skill.to_string_lossy().into_owned(), + ]; + let mut acp = AcpClient::spawn(path.to_str().unwrap(), &args, &[], false) + .await + .unwrap(); + acp.initialize().await.unwrap(); + let mut agent = owned_pi(acp, 1); + assert!(agent.has_system_prompt_support()); + let mut ctx = tests::make_prompt_context_no_owner(); + ctx.cwd = workspace.to_string_lossy().into_owned(); + ctx.base_prompt = Some("BUZZ_BASE".into()); + ctx.system_prompt = Some("BUZZ_PERSONA".into()); + ctx.team_instructions = Some("BUZZ_TEAM".into()); + ctx.session_title = Some("Pi fixture".into()); + let id = create_session_and_apply_model( + &mut agent, + &ctx, + Some("BUZZ_CORE"), + NewSessionChannelContext { + huddle_instructions: Some("BUZZ_HUDDLE"), + canvas: Some("BUZZ_CANVAS"), + name: None, + scope: None, + channel_type: None, + }, + ) + .await + .unwrap(); + let map: serde_json::Value = serde_json::from_str( + &std::fs::read_to_string(home.join(".pi/pi-acp/session-map.json")).unwrap(), + ) + .unwrap(); + let transcript = map["sessions"][&id]["sessionFile"].as_str().unwrap(); + let timestamp = "2026-01-01T00:00:00.000Z"; + std::fs::create_dir_all(std::path::Path::new(transcript).parent().unwrap()).unwrap(); + std::fs::write(transcript, format!("{}\n{}\n", + serde_json::json!({"type":"session","version":3,"id":id,"timestamp":timestamp,"cwd":ctx.cwd}), + serde_json::json!({"type":"message","id":"00000001","parentId":null,"timestamp":timestamp,"message":{"role":"user","content":[{"type":"text","text":"fixture"}],"timestamp":1767225600000u64}}) + )).unwrap(); + agent + .acp + .session_new_full( + &ctx.cwd, + vec![], + Some(SystemPromptTransport::MetaReplace("OTHER_SESSION")), + None, + ) + .await + .unwrap(); + for restart in [false, true] { + if restart { + agent.acp.shutdown().await; + agent.acp = AcpClient::spawn(path.to_str().unwrap(), &args, &[], false) + .await + .unwrap(); + agent.acp.initialize().await.unwrap(); + } + agent + .acp + .session_prompt_with_idle_timeout( + &id, + "/export", + Duration::from_secs(15), + Duration::from_secs(30), + ) + .await + .unwrap(); + let html = + std::fs::read_to_string(workspace.join(format!("pi-session-{id}.html"))).unwrap(); + let encoded = html + .split("id=\"session-data\"") + .nth(1) + .unwrap() + .split_once('>') + .unwrap() + .1 + .split("") + .next() + .unwrap() + .trim(); + let data: serde_json::Value = serde_json::from_slice( + &base64::engine::general_purpose::STANDARD + .decode(encoded) + .unwrap(), + ) + .unwrap(); + let prompt = data["systemPrompt"].as_str().unwrap(); + for marker in [ + "BUZZ_BASE", + "BUZZ_PERSONA", + "BUZZ_TEAM", + "BUZZ_CORE", + "BUZZ_HUDDLE", + "BUZZ_CANVAS", + "BUZZ_SKILL_MARKER", + ] { + assert_eq!( + prompt.matches(marker).count(), + 1, + "{marker}, restart={restart}" + ); + } + assert!(!prompt.contains("OTHER_SESSION")); + assert!(!prompt.contains("You are an expert coding assistant")); + } + agent.acp.shutdown().await; + std::fs::remove_dir_all(dir).unwrap(); +} diff --git a/crates/buzz-acp/src/pool/system_prompt_tests.rs b/crates/buzz-acp/src/pool/system_prompt_tests.rs new file mode 100644 index 00000000000..85fee114693 --- /dev/null +++ b/crates/buzz-acp/src/pool/system_prompt_tests.rs @@ -0,0 +1,50 @@ +#[test] +fn goose_uses_system_prompt_only_after_custom_method_succeeds() { + assert!(!has_system_prompt_support(2, "goose", None)); + assert!(!has_system_prompt_support(2, "goose", Some(false))); + assert!(has_system_prompt_support(2, "goose", Some(true))); + assert!(has_system_prompt_support(1, "goose", Some(true))); + assert!(has_system_prompt_support(2, "buzz-agent", None)); + // Goose never receives system prompt via session/new (uses post-hoc method). + assert_eq!( + session_new_system_prompt(true, 2, "goose", Some("instructions"), false), + None + ); + // Protocol-v2 non-goose gets Field transport. + assert_eq!( + session_new_system_prompt(false, 2, "buzz-agent", Some("instructions"), false), + Some(SystemPromptTransport::Field("instructions")) + ); + // Protocol-v1 non-goose, non-claude gets None (legacy user-message framing). + assert_eq!( + session_new_system_prompt(false, 1, "codex", Some("instructions"), false), + None + ); + // claude-agent-acp gets ClaudeMeta transport regardless of protocol version. + assert_eq!( + session_new_system_prompt(false, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions"), false), + Some(SystemPromptTransport::ClaudeMeta("instructions")) + ); + assert_eq!( + session_new_system_prompt(true, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions"), false), + None, + "goose path must never produce a transport even when agent_name matches" + ); +} + +#[test] +fn claude_agent_acp_has_system_prompt_support_regardless_of_protocol_version() { + // claude-agent-acp declares protocolVersion:1 but supports _meta.systemPrompt; + // has_system_prompt_support must return true so user-message framing is suppressed. + assert!(has_system_prompt_support(1, CLAUDE_AGENT_ACP_NAME, None)); + assert!(has_system_prompt_support(2, CLAUDE_AGENT_ACP_NAME, None)); +} + +#[test] +fn old_zed_adapter_name_falls_through_to_protocol_version_gate() { + // The renamed @zed-industries package predates the _meta.systemPrompt support, + // so it must not be treated as capable and stays on legacy user-message framing. + let old_name = "@zed-industries/claude-code-acp"; + assert!(!has_system_prompt_support(1, old_name, None)); + assert!(has_system_prompt_support(2, old_name, None)); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 531ae335ce5..84f88e406b5 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(); + // Phase 2.5: insert static preset entries (PATH-probed, not editable/deletable). for def in PRESET_HARNESSES { if seen_ids.contains(def.id) { diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index b184559c157..2f3c21b7187 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -16,10 +16,11 @@ 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. @@ -27,44 +28,28 @@ 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(), @@ -86,10 +71,12 @@ pub(super) fn preset_catalog_entry( max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, - install_hint, - install_instructions_url: install_instructions_url.to_string(), + install_hint: def.install_hint.to_string(), + install_instructions_url: def.install_instructions_url.to_string(), can_auto_install: false, - requires_external_cli: def.underlying_cli.is_some(), + // Presets carry one flat install hint, so builtin external-CLI copy + // would name the wrong missing component for adapter presets. + requires_external_cli: false, underlying_cli_path, node_required: false, auth_status: AuthStatus::NotApplicable, @@ -109,15 +96,9 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ label: "Pi", command: "pi-acp", args: &[], - install_instructions_url: "https://github.com/svkozak/pi-acp", - install_hint: "Install the Pi ACP adapter with npm install -g pi-acp.", + install_instructions_url: "https://github.com/salman1993/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 clone https://github.com/salman1993/pi-acp and run `npm ci && npm run build && npm install -g .` in the checkout.", 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", @@ -127,8 +108,6 @@ 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", @@ -138,8 +117,6 @@ 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", @@ -149,8 +126,6 @@ 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", @@ -160,8 +135,6 @@ 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", @@ -171,8 +144,6 @@ 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", @@ -182,8 +153,6 @@ 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", @@ -193,8 +162,6 @@ 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", @@ -204,8 +171,6 @@ 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", @@ -222,8 +187,6 @@ 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, }, ]; @@ -340,8 +303,6 @@ 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] @@ -405,6 +366,13 @@ mod tests { assert_eq!(preset.label, "Pi"); assert_eq!(preset.command, "pi-acp"); + assert_eq!( + preset.install_instructions_url, + "https://github.com/salman1993/pi-acp" + ); + assert!(preset + .install_hint + .contains("npm ci && npm run build && npm install -g .")); assert!(preset.args.is_empty()); assert_eq!(preset.underlying_cli, Some("pi")); @@ -416,8 +384,6 @@ 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") @@ -432,38 +398,12 @@ 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] @@ -478,12 +418,8 @@ 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] @@ -491,12 +427,7 @@ 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_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"); + assert!(!entry.requires_external_cli); } #[test] @@ -513,20 +444,17 @@ mod tests { entry.underlying_cli_path.as_deref(), Some("/usr/local/bin/amp") ); - assert!(entry.install_hint.is_empty()); } #[test] - fn adapter_without_underlying_cli_reports_cli_missing() { + fn adapter_presence_is_enough_for_availability() { 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.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")); 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] 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 7aa886c672a..dc38c3d126f 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -145,14 +145,6 @@ 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 107171b3d47..c01d29f3c2a 100644 --- a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs +++ b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs @@ -41,9 +41,6 @@ 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 diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index e3f0448bd19..6d9a46e20cb 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -872,13 +872,12 @@ export function processTranscriptEvent( } else if (event.kind === "acp_write" && method === "session/new") { // The base + persona prompts ride session/new's systemPrompt, framed by // the harness as ///. - // claude-agent-acp uses _meta.systemPrompt.append instead; both paths + // Pi uses _meta.systemPrompt; Claude uses _meta.systemPrompt.append. // produce the same standalone card (turnId: null, acpSource "session/new"); // the bare field takes precedence when both are present. const params = asRecord(payload.params); - const metaPrompt = asString( - asRecord(asRecord(params._meta).systemPrompt).append, - ); + const meta = asRecord(params._meta).systemPrompt; + const metaPrompt = asString(meta) ?? asString(asRecord(meta).append); const systemPrompt = asString(params.systemPrompt) ?? metaPrompt; if (systemPrompt) { const sections = parseSystemPromptSections(systemPrompt); diff --git a/desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs new file mode 100644 index 00000000000..f1990a7a08e --- /dev/null +++ b/desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs @@ -0,0 +1,43 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { buildTranscript } from "./agentSessionTranscript.ts"; + +test("Pi replacement metadata produces a standalone system prompt card", () => { + const events = [ + { + seq: 1, + timestamp: "2026-09-09T00:00:00Z", + kind: "acp_write", + agentIndex: 0, + channelId: "11111111-1111-1111-1111-111111111111", + sessionId: "pi-session", + turnId: "turn-1", + payload: { + method: "session/new", + params: { + _meta: { + sessionTitle: "Pi fixture", + systemPrompt: + "\nBuzz base\n\n\n\nPersona\n", + }, + }, + }, + }, + ]; + const cards = buildTranscript(events).filter( + (item) => item.acpSource === "session/new", + ); + assert.equal(cards.length, 1); + assert.equal(cards[0].turnId, null); + assert.deepEqual( + cards[0].sections.map((section) => section.body), + ["Buzz base", "Persona"], + ); + events[0].payload.params.systemPrompt = "\nBare field wins\n"; + assert.deepEqual( + buildTranscript(events) + .find((item) => item.acpSource === "session/new") + .sections.map((section) => section.body), + ["Bare field wins"], + ); +}); diff --git a/desktop/src/features/settings/ui/harnessCatalogCopy.ts b/desktop/src/features/settings/ui/harnessCatalogCopy.ts index f91dada83fc..4ad2419fa80 100644 --- a/desktop/src/features/settings/ui/harnessCatalogCopy.ts +++ b/desktop/src/features/settings/ui/harnessCatalogCopy.ts @@ -27,7 +27,7 @@ const HARNESS_DESCRIPTIONS: Record = { cursor: "Cursor's coding agent, connected to Buzz through its ACP server.", // Source: https://github.com/can1357/oh-my-pi omp: "A terminal coding agent with integrated development tools.", - // Sources: https://pi.dev/docs/latest, https://github.com/svkozak/pi-acp + // Sources: https://pi.dev/docs/latest, https://github.com/salman1993/pi-acp pi: "A minimal terminal coding harness, connected through the pi-acp adapter.", // Source: https://build.x.ai (docs unavailable during research; kept // deliberately conservative). From ef21680b70750258d16d671f0e8da47708064c4a Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Wed, 9 Sep 2026 09:52:55 -0400 Subject: [PATCH 2/5] fix(agents): preserve vendor CLI availability checks Signed-off-by: Salman Mohammed --- .../src/managed_agents/discovery/presets.rs | 152 +++++++++++++----- 1 file changed, 112 insertions(+), 40 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index 2f3c21b7187..fd358206023 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/salman1993/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 clone https://github.com/salman1993/pi-acp and run `npm ci && npm run build && npm install -g .` in the checkout.", + install_hint: "Install the Pi ACP adapter by cloning https://github.com/salman1993/pi-acp and running `npm ci && npm run build && npm install -g .` in the checkout.", 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] @@ -366,13 +405,6 @@ mod tests { assert_eq!(preset.label, "Pi"); assert_eq!(preset.command, "pi-acp"); - assert_eq!( - preset.install_instructions_url, - "https://github.com/salman1993/pi-acp" - ); - assert!(preset - .install_hint - .contains("npm ci && npm run build && npm install -g .")); assert!(preset.args.is_empty()); assert_eq!(preset.underlying_cli, Some("pi")); @@ -384,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") @@ -398,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 by cloning https://github.com/salman1993/pi-acp and running `npm ci && npm run build && npm install -g .` in the checkout." + ); + assert_eq!( + adapter_missing.install_instructions_url, + "https://github.com/salman1993/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 by cloning https://github.com/salman1993/pi-acp and running `npm ci && npm run build && npm install -g .` in the checkout." + ); } #[test] @@ -418,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] @@ -427,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] @@ -444,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 0a86962eb20eb0868bf11cad08c827cb61da54ef Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Wed, 9 Sep 2026 10:02:05 -0400 Subject: [PATCH 3/5] docs(pi): simplify fork installation instructions Signed-off-by: Salman Mohammed --- crates/buzz-acp/TESTING.md | 23 ++++++++----------- .../src/managed_agents/discovery/presets.rs | 10 ++++---- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/crates/buzz-acp/TESTING.md b/crates/buzz-acp/TESTING.md index d44035e7785..6935d7da4cf 100644 --- a/crates/buzz-acp/TESTING.md +++ b/crates/buzz-acp/TESTING.md @@ -1,23 +1,20 @@ # Pi adapter integration Buzz's Pi preset uses [salman1993/pi-acp](https://github.com/salman1993/pi-acp). -Install Pi separately, then build and install the adapter from source: +Requires Node.js 22 or newer. Install Pi and configure its model provider, +then install the adapter directly from the fork: ```sh -npm install -g --ignore-scripts @earendil-works/pi-coding-agent -git clone https://github.com/salman1993/pi-acp.git -cd pi-acp -git checkout b893ff9241c35fd04f27b0e9dbfa2f7bc463fc42 -npm ci -npm run build -npm install -g . +npm install -g @earendil-works/pi-coding-agent +pi +npm install -g --install-links=true git+https://github.com/salman1993/pi-acp.git#main ``` -Keep that checkout if npm links the global executable to it. Direct GitHub npm -installation at this revision does not build `dist/index.js`. The unscoped -`npm install -g pi-acp` command installs the upstream package, without these -extensions. Restart managed Pi agents after installing the fork; use fresh -sessions to replace old user-framed standing instructions. +Restart Buzz, then select **Pi** as the agent harness. Buzz starts `pi-acp` +automatically. Run the same adapter install command again to update it. +The unscoped `npm install -g pi-acp` command installs the upstream package, +without these extensions. Use fresh sessions to replace old user-framed +standing instructions. Buzz adds `-- --skill /.agents/skills` when launching `pi-acp`. An existing separator and explicit Pi options are preserved. Managed agents diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index fd358206023..8f86215289d 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -110,10 +110,10 @@ pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ command: "pi-acp", args: &[], install_instructions_url: "https://github.com/salman1993/pi-acp", - install_hint: "Install the Pi ACP adapter by cloning https://github.com/salman1993/pi-acp and running `npm ci && npm run build && npm install -g .` in the checkout.", + install_hint: "Requires Node.js 22 or newer. Install the Pi ACP adapter with `npm install -g --install-links=true git+https://github.com/salman1993/pi-acp.git#main`. Restart Buzz, then select Pi as the agent harness. Run the same install command again to update the adapter.", underlying_cli: Some("pi"), underlying_cli_install_hint: Some( - "Install Pi with npm install -g --ignore-scripts @earendil-works/pi-coding-agent.", + "Install Pi with `npm install -g @earendil-works/pi-coding-agent`, then run `pi` to configure its model provider.", ), underlying_cli_install_instructions_url: Some( "https://github.com/badlogic/pi-mono/tree/main/packages/coding-agent", @@ -434,7 +434,7 @@ mod tests { assert!(adapter_missing.default_args.is_empty()); assert_eq!( adapter_missing.install_hint, - "Install the Pi ACP adapter by cloning https://github.com/salman1993/pi-acp and running `npm ci && npm run build && npm install -g .` in the checkout." + "Requires Node.js 22 or newer. Install the Pi ACP adapter with `npm install -g --install-links=true git+https://github.com/salman1993/pi-acp.git#main`. Restart Buzz, then select Pi as the agent harness. Run the same install command again to update the adapter." ); assert_eq!( adapter_missing.install_instructions_url, @@ -448,7 +448,7 @@ mod tests { 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." + "Install Pi with `npm install -g @earendil-works/pi-coding-agent`, then run `pi` to configure its model provider." ); assert_eq!( cli_missing.install_instructions_url, @@ -462,7 +462,7 @@ mod tests { ); assert_eq!( not_installed.install_hint, - "Install Pi with npm install -g --ignore-scripts @earendil-works/pi-coding-agent. Install the Pi ACP adapter by cloning https://github.com/salman1993/pi-acp and running `npm ci && npm run build && npm install -g .` in the checkout." + "Install Pi with `npm install -g @earendil-works/pi-coding-agent`, then run `pi` to configure its model provider. Requires Node.js 22 or newer. Install the Pi ACP adapter with `npm install -g --install-links=true git+https://github.com/salman1993/pi-acp.git#main`. Restart Buzz, then select Pi as the agent harness. Run the same install command again to update the adapter." ); } From 3e7903dee430e02407366208f7761e7f8e20c1bd Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Wed, 9 Sep 2026 11:22:04 -0400 Subject: [PATCH 4/5] refactor(pi): centralize prompt support and verify workspace Signed-off-by: Salman Mohammed --- crates/buzz-acp/TESTING.md | 5 +- crates/buzz-acp/src/acp.rs | 2 + crates/buzz-acp/src/pool.rs | 7 +- crates/buzz-acp/src/pool/pi_prompt_tests.rs | 70 ++++++++++++++----- .../buzz-acp/src/pool/system_prompt_tests.rs | 36 +++++++--- 5 files changed, 89 insertions(+), 31 deletions(-) diff --git a/crates/buzz-acp/TESTING.md b/crates/buzz-acp/TESTING.md index 6935d7da4cf..1fec453f236 100644 --- a/crates/buzz-acp/TESTING.md +++ b/crates/buzz-acp/TESTING.md @@ -18,7 +18,10 @@ standing instructions. Buzz adds `-- --skill /.agents/skills` when launching `pi-acp`. An existing separator and explicit Pi options are preserved. Managed agents -run from the Buzz workspace, so the default directory is its `.agents/skills`. +run from the Buzz nest (normally `~/.buzz`): Desktop sets the `buzz-acp` child +CWD through `default_agent_workdir()`, and adapters inherit it. The default +skill directory is that launch workspace's `.agents/skills`. Standalone CLI +launches use the caller's working directory. The path is fixed at adapter launch and applies to every Pi subprocess. The full composed session prompt is sent as a replacement string through diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index c25d6b9c41d..59fdbf2080a 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -466,6 +466,8 @@ impl AcpClient { if !args.iter().any(|arg| arg == "--") { cmd.arg("--"); } + // Desktop launches buzz-acp in the Buzz nest; adapters inherit that + // workspace. Keep managed skills tied to launch CWD across sessions. cmd.arg("--skill") .arg(std::env::current_dir()?.join(".agents/skills")); } diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index f26d9f74d18..426d70b81a8 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -296,11 +296,12 @@ fn has_system_prompt_support( protocol_version: u32, agent_name: &str, goose_system_prompt_supported: Option, + pi_system_prompt_supported: bool, ) -> bool { if agent_name == "goose" { goose_system_prompt_supported == Some(true) } else if agent_name == "pi-acp" { - false // Pi uses an explicit capability, not its protocol version. + pi_system_prompt_supported } else if agent_name == CLAUDE_AGENT_ACP_NAME { true } else { @@ -332,13 +333,11 @@ fn session_new_system_prompt<'a>( impl OwnedAgent { pub(crate) fn has_system_prompt_support(&self) -> bool { - if self.agent_name == "pi-acp" { - return self.acp.supports_pi_system_prompt(); - } has_system_prompt_support( self.protocol_version, &self.agent_name, self.goose_system_prompt_supported, + self.acp.supports_pi_system_prompt(), ) } } diff --git a/crates/buzz-acp/src/pool/pi_prompt_tests.rs b/crates/buzz-acp/src/pool/pi_prompt_tests.rs index 04955ce6fba..cccb95cf809 100644 --- a/crates/buzz-acp/src/pool/pi_prompt_tests.rs +++ b/crates/buzz-acp/src/pool/pi_prompt_tests.rs @@ -152,33 +152,69 @@ async fn pi_capability_controls_composed_prompt_and_legacy_framing() { } #[tokio::test] -async fn pi_launch_preserves_existing_forwarded_skills_and_one_separator() { +async fn pi_launch_preserves_existing_skills_in_explicit_workspace() { + const FIXTURE_ENV: &str = "BUZZ_TEST_PI_LAUNCH_WORKSPACE"; + if let Some(dir) = std::env::var_os(FIXTURE_ENV) { + let path = std::path::PathBuf::from(dir).join("pi-acp"); + let mut client = AcpClient::spawn( + path.to_str().unwrap(), + &["--".into(), "--skill".into(), "/extra skills".into()], + &[], + false, + ) + .await + .unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(5), client.initialize()) + .await + .unwrap() + .unwrap_err(); + client.shutdown().await; + return; + } + let dir = fixture_dir(); - let path = script_at( + let workspace = dir.join("chosen workspace"); + std::fs::create_dir_all(workspace.join(".agents/skills")).unwrap(); + // Canonicalize macOS's /var -> /private/var before comparing with getcwd. + let workspace = workspace.canonicalize().unwrap(); + script_at( &dir, - &format!("printf '%s\\n' \"$@\" > '{}/args'", dir.display()), + r#"printf '%s\n' "$@" > "$(dirname "$0")/args" +pwd -P > "$(dirname "$0")/cwd""#, ); - let mut client = AcpClient::spawn( - path.to_str().unwrap(), - &["--".into(), "--skill".into(), "/extra skills".into()], - &[], - false, + // Re-enter only this test in a separate process so parallel tests never + // share a mutated CWD. This models Desktop setting its harness child's CWD. + let output = tokio::time::timeout( + std::time::Duration::from_secs(20), + tokio::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "pool::pi_prompt_tests::pi_launch_preserves_existing_skills_in_explicit_workspace", + "--nocapture", + ]) + .kill_on_drop(true) + .env(FIXTURE_ENV, &dir) + .current_dir(&workspace) + .output(), ) .await + .unwrap() .unwrap(); - tokio::time::timeout(std::time::Duration::from_secs(5), client.initialize()) - .await - .unwrap() - .unwrap_err(); - client.shutdown().await; + assert!( + output.status.success(), + "child failed: {}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + std::fs::read_to_string(dir.join("cwd")).unwrap().trim(), + workspace.to_str().unwrap() + ); assert_eq!( std::fs::read_to_string(dir.join("args")).unwrap(), format!( "--\n--skill\n/extra skills\n--skill\n{}\n", - std::env::current_dir() - .unwrap() - .join(".agents/skills") - .display() + workspace.join(".agents/skills").display() ) ); std::fs::remove_dir_all(dir).unwrap(); diff --git a/crates/buzz-acp/src/pool/system_prompt_tests.rs b/crates/buzz-acp/src/pool/system_prompt_tests.rs index 85fee114693..69a09b418ea 100644 --- a/crates/buzz-acp/src/pool/system_prompt_tests.rs +++ b/crates/buzz-acp/src/pool/system_prompt_tests.rs @@ -1,10 +1,10 @@ #[test] fn goose_uses_system_prompt_only_after_custom_method_succeeds() { - assert!(!has_system_prompt_support(2, "goose", None)); - assert!(!has_system_prompt_support(2, "goose", Some(false))); - assert!(has_system_prompt_support(2, "goose", Some(true))); - assert!(has_system_prompt_support(1, "goose", Some(true))); - assert!(has_system_prompt_support(2, "buzz-agent", None)); + assert!(!has_system_prompt_support(2, "goose", None, false)); + assert!(!has_system_prompt_support(2, "goose", Some(false), false)); + assert!(has_system_prompt_support(2, "goose", Some(true), false)); + assert!(has_system_prompt_support(1, "goose", Some(true), false)); + assert!(has_system_prompt_support(2, "buzz-agent", None, false)); // Goose never receives system prompt via session/new (uses post-hoc method). assert_eq!( session_new_system_prompt(true, 2, "goose", Some("instructions"), false), @@ -36,8 +36,18 @@ fn goose_uses_system_prompt_only_after_custom_method_succeeds() { fn claude_agent_acp_has_system_prompt_support_regardless_of_protocol_version() { // claude-agent-acp declares protocolVersion:1 but supports _meta.systemPrompt; // has_system_prompt_support must return true so user-message framing is suppressed. - assert!(has_system_prompt_support(1, CLAUDE_AGENT_ACP_NAME, None)); - assert!(has_system_prompt_support(2, CLAUDE_AGENT_ACP_NAME, None)); + assert!(has_system_prompt_support( + 1, + CLAUDE_AGENT_ACP_NAME, + None, + false + )); + assert!(has_system_prompt_support( + 2, + CLAUDE_AGENT_ACP_NAME, + None, + false + )); } #[test] @@ -45,6 +55,14 @@ fn old_zed_adapter_name_falls_through_to_protocol_version_gate() { // The renamed @zed-industries package predates the _meta.systemPrompt support, // so it must not be treated as capable and stays on legacy user-message framing. let old_name = "@zed-industries/claude-code-acp"; - assert!(!has_system_prompt_support(1, old_name, None)); - assert!(has_system_prompt_support(2, old_name, None)); + assert!(!has_system_prompt_support(1, old_name, None, false)); + assert!(has_system_prompt_support(2, old_name, None, false)); +} + +#[test] +fn pi_prompt_support_requires_capability_regardless_of_protocol_version() { + for version in [1, 2] { + assert!(!has_system_prompt_support(version, "pi-acp", None, false)); + assert!(has_system_prompt_support(version, "pi-acp", None, true)); + } } From 73a8567cd5388e17be2feb4e4777def927d30b87 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Wed, 9 Sep 2026 17:12:02 -0400 Subject: [PATCH 5/5] refactor(pi): use top-level session system prompt Signed-off-by: Salman Mohammed --- crates/buzz-acp/TESTING.md | 9 +-- crates/buzz-acp/src/acp.rs | 23 +------- crates/buzz-acp/src/pool.rs | 12 +--- crates/buzz-acp/src/pool/pi_prompt_tests.rs | 55 ++++++------------- .../buzz-acp/src/pool/system_prompt_tests.rs | 51 ++++++++--------- .../agents/ui/agentSessionTranscript.ts | 7 ++- .../ui/agentSessionTranscriptPi.test.mjs | 17 ++---- 7 files changed, 59 insertions(+), 115 deletions(-) diff --git a/crates/buzz-acp/TESTING.md b/crates/buzz-acp/TESTING.md index 1fec453f236..efc1b7c3909 100644 --- a/crates/buzz-acp/TESTING.md +++ b/crates/buzz-acp/TESTING.md @@ -24,10 +24,11 @@ skill directory is that launch workspace's `.agents/skills`. Standalone CLI launches use the caller's working directory. The path is fixed at adapter launch and applies to every Pi subprocess. -The full composed session prompt is sent as a replacement string through -`_meta.systemPrompt` only when Pi advertises both `replace` and `persisted` -under `agentCapabilities._meta.piAcp.systemPrompt`. Older adapters retain -first-turn user framing. Session titles share `_meta.sessionTitle`. +The full composed session prompt is sent as a replacement string through the +provisional top-level `session/new.params.systemPrompt` field. Buzz recognizes +`pi-acp` by the agent name returned during initialization, regardless of protocol +version. No custom capability negotiation or legacy Pi prompt fallback is used. +Session titles continue to use `_meta.sessionTitle`. ## Validation diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 59fdbf2080a..fefc241ecb5 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -200,7 +200,6 @@ pub struct AcpClient { /// a JSON-RPC *success*, not `-32601` — which the main loop would read as /// a delivered steer and drop the user's message from the queue. steering_supported: bool, - pi_system_prompt_supported: bool, /// Per-turn channel for receiving goose-native non-cancelling steer /// requests from the main loop. Installed by /// [`install_steer_rx`](Self::install_steer_rx) at dispatch and @@ -569,7 +568,6 @@ impl AcpClient { observer_context: ObserverContext::default(), active_run_id: None, steering_supported: false, - pi_system_prompt_supported: false, steer_rx: None, goose_usage: UsageTracker::default(), standard_usage: StandardUsageTracker::default(), @@ -628,19 +626,10 @@ impl AcpClient { .pointer("/_meta/steering/supported") .and_then(|v| v.as_bool()) .unwrap_or(false); - self.pi_system_prompt_supported = ["replace", "persisted"].iter().all(|key| { - result["agentCapabilities"]["_meta"]["piAcp"]["systemPrompt"][key].as_bool() - == Some(true) - }); tracing::debug!(target: "acp::init", "initialize response: {result}"); Ok(result) } - /// Whether Pi advertised persistent system-prompt replacement. - pub fn supports_pi_system_prompt(&self) -> bool { - self.pi_system_prompt_supported - } - /// Send the ACP `authenticate` request for an adapter-advertised method. pub async fn authenticate(&mut self, method_id: &str) -> Result { let params = serde_json::json!({ @@ -657,13 +646,10 @@ impl AcpClient { /// /// - `None` — no system-prompt field in the request (legacy framing). /// - `Some(SystemPromptTransport::Field(text))` — bare `systemPrompt` field - /// (ACP protocol v2, buzz-agent, goose unused). + /// (ACP protocol v2, buzz-agent, pi-acp; goose unused). /// - `Some(SystemPromptTransport::ClaudeMeta(text))` — `_meta.systemPrompt` /// as `{"append": text}`, keeping claude-agent-acp's native preset intact. /// - /// - `Some(SystemPromptTransport::MetaReplace(text))` — `_meta.systemPrompt` - /// as a string, replacing Pi's native base prompt. - /// /// `session_title` rides in `_meta.sessionTitle` when `Some`; `_meta` is /// omitted entirely otherwise, since adapters may distinguish an absent /// member from a null one. When both `ClaudeMeta` and `session_title` are @@ -686,9 +672,6 @@ impl AcpClient { Some(SystemPromptTransport::Field(sp)) => { params["systemPrompt"] = serde_json::Value::String(sp.to_owned()); } - Some(SystemPromptTransport::MetaReplace(sp)) => { - params["_meta"]["systemPrompt"] = serde_json::Value::String(sp.to_owned()); - } Some(SystemPromptTransport::ClaudeMeta(sp)) => { // Merge into _meta so sessionTitle (set below) is not clobbered. params["_meta"]["systemPrompt"] = serde_json::json!({ "append": sp }); @@ -2157,7 +2140,7 @@ pub struct SessionNewResponse { /// How to deliver a system prompt on `session/new`. /// -/// - **`Field`** — bare `systemPrompt` field (ACP protocol v2, buzz-agent). +/// - **`Field`** — bare `systemPrompt` field (ACP protocol v2, buzz-agent, pi-acp). /// - **`ClaudeMeta`** — `_meta.systemPrompt: {"append": text}`, used by /// `claude-agent-acp` to append to the adapter's own native system prompt /// while keeping its tool-use preset intact. @@ -2167,8 +2150,6 @@ pub enum SystemPromptTransport<'a> { Field(&'a str), /// Deliver as `_meta.systemPrompt: {"append": text}`. ClaudeMeta(&'a str), - /// Deliver as `_meta.systemPrompt: text`, replacing the native base prompt. - MetaReplace(&'a str), } /// How to switch to a particular model on a session. diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 426d70b81a8..33f4f7b8468 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -296,13 +296,10 @@ fn has_system_prompt_support( protocol_version: u32, agent_name: &str, goose_system_prompt_supported: Option, - pi_system_prompt_supported: bool, ) -> bool { if agent_name == "goose" { goose_system_prompt_supported == Some(true) - } else if agent_name == "pi-acp" { - pi_system_prompt_supported - } else if agent_name == CLAUDE_AGENT_ACP_NAME { + } else if agent_name == "pi-acp" || agent_name == CLAUDE_AGENT_ACP_NAME { true } else { protocol_version >= 2 @@ -314,14 +311,11 @@ fn session_new_system_prompt<'a>( protocol_version: u32, agent_name: &str, prompt: Option<&'a str>, - pi_system_prompt_supported: bool, ) -> Option> { if is_goose { None } else if agent_name == "pi-acp" { - prompt - .filter(|_| pi_system_prompt_supported) - .map(SystemPromptTransport::MetaReplace) + prompt.map(SystemPromptTransport::Field) } else if protocol_version < 2 && agent_name != CLAUDE_AGENT_ACP_NAME { None } else if agent_name == CLAUDE_AGENT_ACP_NAME { @@ -337,7 +331,6 @@ impl OwnedAgent { self.protocol_version, &self.agent_name, self.goose_system_prompt_supported, - self.acp.supports_pi_system_prompt(), ) } } @@ -1528,7 +1521,6 @@ async fn create_session_and_apply_model( agent.protocol_version, &agent.agent_name, combined_system_prompt.as_deref(), - agent.acp.supports_pi_system_prompt(), ), session_title.as_deref(), ) diff --git a/crates/buzz-acp/src/pool/pi_prompt_tests.rs b/crates/buzz-acp/src/pool/pi_prompt_tests.rs index cccb95cf809..f0122b8b075 100644 --- a/crates/buzz-acp/src/pool/pi_prompt_tests.rs +++ b/crates/buzz-acp/src/pool/pi_prompt_tests.rs @@ -32,26 +32,11 @@ fn script_at(dir: &std::path::Path, script: &str) -> std::path::PathBuf { } #[tokio::test] -async fn pi_capability_controls_composed_prompt_and_legacy_framing() { - for (capability, supported) in [ - ( - serde_json::json!({"replace": true, "persisted": true}), - true, - ), - (serde_json::json!({"replace": true}), false), - ( - serde_json::json!({"append": true, "persisted": true}), - false, - ), - ( - serde_json::json!({"replace": "true", "persisted": true}), - false, - ), - (serde_json::Value::Null, false), - ] { +async fn pi_composed_prompt_uses_field_without_capability_negotiation() { + for version in [1, 2] { let dir = fixture_dir(); let init = serde_json::json!({"jsonrpc":"2.0", "id":0, "result": { - "protocolVersion":2, "agentCapabilities":{"_meta":{"piAcp":{"systemPrompt":capability}}} + "protocolVersion":version, "agentInfo":{"name":"pi-acp", "version":"fixture"}, "agentCapabilities":{} }}); let path = script_at( &dir, @@ -72,8 +57,8 @@ async fn pi_capability_controls_composed_prompt_and_legacy_framing() { .await .unwrap(); acp.initialize().await.unwrap(); - let mut agent = owned_pi(acp, 2); - assert_eq!(agent.has_system_prompt_support(), supported); + let mut agent = owned_pi(acp, version); + assert!(agent.has_system_prompt_support()); let mut ctx = tests::make_prompt_context_no_owner(); ctx.base_prompt = Some("BUZZ_BASE".into()); ctx.system_prompt = Some("BUZZ_PERSONA".into()); @@ -99,7 +84,7 @@ async fn pi_capability_controls_composed_prompt_and_legacy_framing() { let request: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(dir.join("request")).unwrap()).unwrap(); let params = &request["params"]; - assert!(params.get("systemPrompt").is_none()); + assert!(params["_meta"].get("systemPrompt").is_none()); assert!(params["_meta"]["sessionTitle"] .as_str() .unwrap() @@ -112,7 +97,7 @@ async fn pi_capability_controls_composed_prompt_and_legacy_framing() { huddle_instructions: Some("BUZZ_HUDDLE"), agent_canvas: Some(canvas), }; - let user = prepend_standing_for_legacy(if supported { 2 } else { 1 }, &standing, "EVENT"); + let user = prepend_standing_for_legacy(2, &standing, "EVENT"); for marker in [ "BUZZ_BASE", "BUZZ_PERSONA", @@ -121,21 +106,17 @@ async fn pi_capability_controls_composed_prompt_and_legacy_framing() { "BUZZ_HUDDLE", "BUZZ_CANVAS", ] { - if supported { - assert_eq!( - params["_meta"]["systemPrompt"] - .as_str() - .unwrap() - .matches(marker) - .count(), - 1 - ); - assert!(!user.contains(marker)); - } else { - assert!(params["_meta"].get("systemPrompt").is_none()); - assert_eq!(user.matches(marker).count(), 1); - } + assert_eq!( + params["systemPrompt"] + .as_str() + .unwrap() + .matches(marker) + .count(), + 1 + ); + assert!(!user.contains(marker)); } + let args = std::fs::read_to_string(dir.join("args")).unwrap(); assert_eq!( args, @@ -291,7 +272,7 @@ async fn real_pi_preserves_buzz_prompt_and_launch_skills_on_restore() { .session_new_full( &ctx.cwd, vec![], - Some(SystemPromptTransport::MetaReplace("OTHER_SESSION")), + Some(SystemPromptTransport::Field("OTHER_SESSION")), None, ) .await diff --git a/crates/buzz-acp/src/pool/system_prompt_tests.rs b/crates/buzz-acp/src/pool/system_prompt_tests.rs index 69a09b418ea..f3518a8f20f 100644 --- a/crates/buzz-acp/src/pool/system_prompt_tests.rs +++ b/crates/buzz-acp/src/pool/system_prompt_tests.rs @@ -1,32 +1,32 @@ #[test] fn goose_uses_system_prompt_only_after_custom_method_succeeds() { - assert!(!has_system_prompt_support(2, "goose", None, false)); - assert!(!has_system_prompt_support(2, "goose", Some(false), false)); - assert!(has_system_prompt_support(2, "goose", Some(true), false)); - assert!(has_system_prompt_support(1, "goose", Some(true), false)); - assert!(has_system_prompt_support(2, "buzz-agent", None, false)); + assert!(!has_system_prompt_support(2, "goose", None)); + assert!(!has_system_prompt_support(2, "goose", Some(false))); + assert!(has_system_prompt_support(2, "goose", Some(true))); + assert!(has_system_prompt_support(1, "goose", Some(true))); + assert!(has_system_prompt_support(2, "buzz-agent", None)); // Goose never receives system prompt via session/new (uses post-hoc method). assert_eq!( - session_new_system_prompt(true, 2, "goose", Some("instructions"), false), + session_new_system_prompt(true, 2, "goose", Some("instructions")), None ); // Protocol-v2 non-goose gets Field transport. assert_eq!( - session_new_system_prompt(false, 2, "buzz-agent", Some("instructions"), false), + session_new_system_prompt(false, 2, "buzz-agent", Some("instructions")), Some(SystemPromptTransport::Field("instructions")) ); // Protocol-v1 non-goose, non-claude gets None (legacy user-message framing). assert_eq!( - session_new_system_prompt(false, 1, "codex", Some("instructions"), false), + session_new_system_prompt(false, 1, "codex", Some("instructions")), None ); // claude-agent-acp gets ClaudeMeta transport regardless of protocol version. assert_eq!( - session_new_system_prompt(false, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions"), false), + session_new_system_prompt(false, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions")), Some(SystemPromptTransport::ClaudeMeta("instructions")) ); assert_eq!( - session_new_system_prompt(true, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions"), false), + session_new_system_prompt(true, 1, CLAUDE_AGENT_ACP_NAME, Some("instructions")), None, "goose path must never produce a transport even when agent_name matches" ); @@ -36,18 +36,8 @@ fn goose_uses_system_prompt_only_after_custom_method_succeeds() { fn claude_agent_acp_has_system_prompt_support_regardless_of_protocol_version() { // claude-agent-acp declares protocolVersion:1 but supports _meta.systemPrompt; // has_system_prompt_support must return true so user-message framing is suppressed. - assert!(has_system_prompt_support( - 1, - CLAUDE_AGENT_ACP_NAME, - None, - false - )); - assert!(has_system_prompt_support( - 2, - CLAUDE_AGENT_ACP_NAME, - None, - false - )); + assert!(has_system_prompt_support(1, CLAUDE_AGENT_ACP_NAME, None)); + assert!(has_system_prompt_support(2, CLAUDE_AGENT_ACP_NAME, None)); } #[test] @@ -55,14 +45,21 @@ fn old_zed_adapter_name_falls_through_to_protocol_version_gate() { // The renamed @zed-industries package predates the _meta.systemPrompt support, // so it must not be treated as capable and stays on legacy user-message framing. let old_name = "@zed-industries/claude-code-acp"; - assert!(!has_system_prompt_support(1, old_name, None, false)); - assert!(has_system_prompt_support(2, old_name, None, false)); + assert!(!has_system_prompt_support(1, old_name, None)); + assert!(has_system_prompt_support(2, old_name, None)); } #[test] -fn pi_prompt_support_requires_capability_regardless_of_protocol_version() { +fn pi_prompt_support_uses_field_regardless_of_protocol_version() { for version in [1, 2] { - assert!(!has_system_prompt_support(version, "pi-acp", None, false)); - assert!(has_system_prompt_support(version, "pi-acp", None, true)); + assert!(has_system_prompt_support(version, "pi-acp", None)); + assert_eq!( + session_new_system_prompt(false, version, "pi-acp", Some("instructions")), + Some(SystemPromptTransport::Field("instructions")) + ); + assert_eq!( + session_new_system_prompt(false, version, "pi-acp", None), + None + ); } } diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index 6d9a46e20cb..e3f0448bd19 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -872,12 +872,13 @@ export function processTranscriptEvent( } else if (event.kind === "acp_write" && method === "session/new") { // The base + persona prompts ride session/new's systemPrompt, framed by // the harness as ///. - // Pi uses _meta.systemPrompt; Claude uses _meta.systemPrompt.append. + // claude-agent-acp uses _meta.systemPrompt.append instead; both paths // produce the same standalone card (turnId: null, acpSource "session/new"); // the bare field takes precedence when both are present. const params = asRecord(payload.params); - const meta = asRecord(params._meta).systemPrompt; - const metaPrompt = asString(meta) ?? asString(asRecord(meta).append); + const metaPrompt = asString( + asRecord(asRecord(params._meta).systemPrompt).append, + ); const systemPrompt = asString(params.systemPrompt) ?? metaPrompt; if (systemPrompt) { const sections = parseSystemPromptSections(systemPrompt); diff --git a/desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs index f1990a7a08e..2f1e5fe7586 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscriptPi.test.mjs @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { buildTranscript } from "./agentSessionTranscript.ts"; -test("Pi replacement metadata produces a standalone system prompt card", () => { +test("Pi top-level replacement prompt produces a standalone system prompt card", () => { const events = [ { seq: 1, @@ -15,11 +15,9 @@ test("Pi replacement metadata produces a standalone system prompt card", () => { payload: { method: "session/new", params: { - _meta: { - sessionTitle: "Pi fixture", - systemPrompt: - "\nBuzz base\n\n\n\nPersona\n", - }, + _meta: { sessionTitle: "Pi fixture" }, + systemPrompt: + "\nBuzz base\n\n\n\nPersona\n", }, }, }, @@ -33,11 +31,4 @@ test("Pi replacement metadata produces a standalone system prompt card", () => { cards[0].sections.map((section) => section.body), ["Buzz base", "Persona"], ); - events[0].payload.params.systemPrompt = "\nBare field wins\n"; - assert.deepEqual( - buildTranscript(events) - .find((item) => item.acpSource === "session/new") - .sections.map((section) => section.body), - ["Bare field wins"], - ); });