From d4e1a773a76f3f335ade877049bdf7a16b03045d Mon Sep 17 00:00:00 2001 From: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Date: Fri, 17 Jul 2026 12:21:18 -0400 Subject: [PATCH] fix(acp): deliver instructions to Goose Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> --- crates/buzz-acp/src/acp.rs | 73 +++++++++++++++++ crates/buzz-acp/src/lib.rs | 49 +++++++++-- crates/buzz-acp/src/pool.rs | 159 ++++++++++++++++++++++++++++++------ 3 files changed, 245 insertions(+), 36 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 4b8d043ab..b7bca5477 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -597,6 +597,24 @@ impl AcpClient { .session_id) } + /// Send Goose's custom system-prompt request after `session/new`. + pub async fn session_set_goose_system_prompt( + &mut self, + session_id: &str, + text: &str, + ) -> Result { + self.send_request( + "_goose/unstable/session/system-prompt/set", + serde_json::json!({ + "sessionId": session_id, + "mode": "append", + "key": "buzz", + "text": text, + }), + ) + .await + } + /// Send `session/set_config_option` (stable ACP path). pub async fn session_set_config_option( &mut self, @@ -2889,6 +2907,61 @@ mod tests { ); } + #[tokio::test] + async fn goose_system_prompt_request_uses_append_contract() { + let script = r#" + read -t 2 REQ + echo '{"jsonrpc":"2.0","id":0,"result":{"_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + let result = client + .session_set_goose_system_prompt("ses_goose", "Be terse") + .await + .expect("custom request succeeds"); + let received = &result["_receivedRequest"]; + assert_eq!( + received["method"], + "_goose/unstable/session/system-prompt/set" + ); + assert_eq!(received["params"]["sessionId"], "ses_goose"); + assert_eq!(received["params"]["mode"], "append"); + assert_eq!(received["params"]["key"], "buzz"); + assert_eq!(received["params"]["text"], "Be terse"); + } + + #[tokio::test] + async fn goose_system_prompt_preserves_method_not_found_for_fallback() { + let script = r#" + read -t 2 _REQ + echo '{"jsonrpc":"2.0","id":0,"error":{"code":-32601,"message":"Method not found"}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + assert!(matches!( + client + .session_set_goose_system_prompt("ses_goose", "Be terse") + .await, + Err(AcpError::AgentError { code: -32601, .. }) + )); + } + + #[tokio::test] + async fn goose_system_prompt_preserves_invalid_params_as_error() { + let script = r#" + read -t 2 _REQ + echo '{"jsonrpc":"2.0","id":0,"error":{"code":-32602,"message":"Invalid params"}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + assert!(matches!( + client + .session_set_goose_system_prompt("ses_goose", "Be terse") + .await, + Err(AcpError::AgentError { code: -32602, .. }) + )); + } + #[tokio::test] async fn session_new_full_omits_system_prompt_when_none() { // When system_prompt is None, the field should not appear in params. diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 3d8612529..02cd8a2fd 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -984,7 +984,7 @@ struct RespawnResult { /// Tuple: (initialized client, protocol version, supports_goose_steer). /// The third element is always `true` — the supervisor uses /// try-and-tolerate for the steer extension. - result: Result<(AcpClient, u32, bool)>, + result: Result<(AcpClient, u32, String)>, } /// Outcome of a non-cancelling steer attempt, forwarded from a per-attempt @@ -1028,7 +1028,7 @@ impl RespawnGuard { /// Send the result and disarm the guard. Uses `try_send` (sync) so there /// is no await boundary between marking `sent` and actually enqueueing — /// cancellation cannot slip between the two. - fn send(mut self, result: Result<(AcpClient, u32, bool)>) { + fn send(mut self, result: Result<(AcpClient, u32, String)>) { // Invariant: try_send succeeds because the channel capacity equals the // slot count, and respawn_in_flight guarantees at most one outstanding // result per slot. If this ever fails, the channel sizing or the @@ -1198,6 +1198,7 @@ async fn tokio_main() -> Result<()> { "initializeResult": init_result, }), ); + let agent_name = normalized_agent_name(&init_result); agent_slots.push(Some(OwnedAgent { index: i, acp, @@ -1205,6 +1206,8 @@ async fn tokio_main() -> Result<()> { model_capabilities: None, desired_model: config.model.clone(), model_overridden: false, + agent_name, + goose_system_prompt_supported: None, protocol_version, })); } @@ -1640,7 +1643,7 @@ async fn tokio_main() -> Result<()> { while let Ok(rr) = respawn_rx.try_recv() { crash_history[rr.index].respawn_in_flight = false; match rr.result { - Ok((acp, protocol_version, _)) => { + Ok((acp, protocol_version, agent_name)) => { let agent = OwnedAgent { index: rr.index, acp, @@ -1648,6 +1651,8 @@ async fn tokio_main() -> Result<()> { model_capabilities: None, desired_model: config.model.clone(), model_overridden: false, + agent_name, + goose_system_prompt_supported: None, protocol_version, }; pool.return_agent(agent); @@ -3199,10 +3204,6 @@ fn dispatch_heartbeat( .heartbeat_prompt .clone() .unwrap_or_else(default_heartbeat_prompt); - // For legacy agents (protocol_version < 2), prepend base_prompt to the - // heartbeat user message since they don't receive it via session/new. - let prompt_text = - pool::prepend_base_for_legacy(agent.protocol_version, ctx.base_prompt, &prompt_text); let result_tx = pool.result_tx(); let ctx_clone = Arc::clone(ctx); let agent_index = agent.index; @@ -3315,6 +3316,17 @@ fn spawn_respawn_task( true } +fn normalized_agent_name(init_result: &serde_json::Value) -> String { + init_result + .get("agentInfo") + .or_else(|| init_result.get("serverInfo")) + .and_then(|info| info.get("name")) + .and_then(|value| value.as_str()) + .unwrap_or("unknown") + .trim() + .to_ascii_lowercase() +} + // ── spawn_and_init ──────────────────────────────────────────────────────────── /// Spawn an agent subprocess and run the MCP `initialize` handshake. /// @@ -3327,7 +3339,7 @@ async fn spawn_and_init( has_generated_codex_config: bool, agent_index: usize, observer: Option, -) -> Result<(AcpClient, u32, bool)> { +) -> Result<(AcpClient, u32, String)> { let mut acp = AcpClient::spawn(command, args, extra_env, has_generated_codex_config) .await .map_err(|e| anyhow::anyhow!("failed to spawn agent: {e}"))?; @@ -3344,7 +3356,8 @@ async fn spawn_and_init( "initializeResult": init_result, }), ); - Ok((acp, protocol_version, true)) + let agent_name = normalized_agent_name(&init_result); + Ok((acp, protocol_version, agent_name)) } Err(e) => { // Explicitly shut down the spawned child to prevent zombie/leak. @@ -4287,6 +4300,22 @@ mod error_outcome_emission_tests { } } + #[test] + fn normalizes_agent_name_from_initialize_result() { + assert_eq!( + normalized_agent_name(&serde_json::json!({ + "agentInfo": { "name": " Goose ", "version": "1.43.0" } + })), + "goose" + ); + assert_eq!( + normalized_agent_name(&serde_json::json!({ + "serverInfo": { "name": "buzz-agent" } + })), + "buzz-agent" + ); + } + /// Spawn a real but inert agent subprocess (`cat`) so the error paths have /// an `OwnedAgent` to move into respawn or return to the pool. The error /// branches never talk to the subprocess. @@ -4300,6 +4329,8 @@ mod error_outcome_emission_tests { model_capabilities: None, desired_model: None, model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, // Error branches under test never read this; 1 is the legacy // non-systemPrompt path, the simplest valid value. protocol_version: 1, diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index ca0806404..5578a563b 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -154,11 +154,50 @@ pub struct OwnedAgent { /// desktop reader to distinguish a genuine runtime override from a stale /// session whose persona model was edited. Reset on spawn/restart. pub model_overridden: bool, + /// Normalized agent name from initialize (`agentInfo.name`/`serverInfo.name`). + pub agent_name: String, + /// Whether Goose accepted its custom system-prompt method. `None` probes on + /// the first session; method-not-found is cached as `Some(false)` so legacy + /// user-message framing is used for this process thereafter. + pub goose_system_prompt_supported: Option, /// Protocol version reported by the agent in its initialize response. - /// Agents declaring >= 2 support `systemPrompt` in session/new. pub protocol_version: u32, } +fn has_system_prompt_support( + protocol_version: u32, + agent_name: &str, + goose_system_prompt_supported: Option, +) -> bool { + if agent_name == "goose" { + goose_system_prompt_supported == Some(true) + } else { + protocol_version >= 2 + } +} + +fn session_new_system_prompt( + is_goose: bool, + protocol_version: u32, + prompt: Option<&str>, +) -> Option<&str> { + if is_goose || protocol_version < 2 { + None + } else { + prompt + } +} + +impl OwnedAgent { + pub(crate) fn has_system_prompt_support(&self) -> bool { + has_system_prompt_support( + self.protocol_version, + &self.agent_name, + self.goose_system_prompt_supported, + ) + } +} + /// Pool of agents with take-and-return ownership semantics. /// /// Agents are either idle (sitting in `agents[i]`) or checked out @@ -702,37 +741,57 @@ async fn create_session_and_apply_model( agent_core: Option<&str>, agent_canvas: Option<&str>, ) -> Result { - // Combine base_prompt + system_prompt + agent core + canvas metadata into a - // single systemPrompt value for the session/new request. Only sent when the - // agent declares protocol version >= 2 (supports systemPrompt); legacy agents - // ignore it and receive the same content as user-message sections via - // `format_prompt`. Core carries its own `[Agent Memory — core]` header, and - // canvas carries its own `[Channel Canvas]` header; both are appended with a - // blank-line separator. - let combined_system_prompt: Option = if agent.protocol_version >= 2 { - with_canvas( - with_core( - with_team( - framed_system_prompt(&ctx.cwd, ctx.base_prompt, ctx.system_prompt.as_deref()), - ctx.team_instructions.as_deref(), - ), - agent_core, + // Build base_prompt + system_prompt + agent core + canvas metadata into a + // single prompt. Standard protocol-v2 agents receive it in `session/new`; + // Goose receives it through the custom request below. Legacy agents receive + // the same content as user-message sections via `format_prompt`. Core carries + // its own `[Agent Memory — core]` header, and canvas carries its own + // `[Channel Canvas]` header; both are appended with a blank-line separator. + let is_goose = agent.agent_name == "goose"; + let combined_system_prompt = with_canvas( + with_core( + with_team( + framed_system_prompt(&ctx.cwd, ctx.base_prompt, ctx.system_prompt.as_deref()), + ctx.team_instructions.as_deref(), ), - agent_canvas, - ) - } else { - None - }; + agent_core, + ), + agent_canvas, + ); let resp = agent .acp .session_new_full( &ctx.cwd, ctx.mcp_servers.clone(), - combined_system_prompt.as_deref(), + session_new_system_prompt( + is_goose, + agent.protocol_version, + combined_system_prompt.as_deref(), + ), ) .await?; + if is_goose && agent.goose_system_prompt_supported != Some(false) { + if let Some(prompt) = combined_system_prompt.as_deref() { + match agent + .acp + .session_set_goose_system_prompt(&resp.session_id, prompt) + .await + { + Ok(_) => agent.goose_system_prompt_supported = Some(true), + Err(AcpError::AgentError { code: -32601, .. }) => { + agent.goose_system_prompt_supported = Some(false); + tracing::warn!( + target: "pool::session", + "Goose does not support its system-prompt extension; using user-message framing" + ); + } + Err(error) => return Err(error), + } + } + } + // Populate model capabilities on first session creation. if agent.model_capabilities.is_none() { agent.model_capabilities = Some(AgentModelCapabilities { @@ -1421,10 +1480,21 @@ pub async fn run_prompt_task( // Canvas is also injected here for legacy agents: protocol-v2 agents // already have it in systemPrompt; legacy agents need it before the // first prompt, matching the "every turn" per-turn delivery semantics. - let init_msg = - prepend_base_for_legacy(agent.protocol_version, ctx.base_prompt, initial_msg); + let init_msg = prepend_base_for_legacy( + if agent.has_system_prompt_support() { + 2 + } else { + 1 + }, + ctx.base_prompt, + initial_msg, + ); let init_msg = prepend_canvas_for_legacy( - agent.protocol_version, + if agent.has_system_prompt_support() { + 2 + } else { + 1 + }, agent_canvas.as_deref(), &init_msg, ); @@ -1540,7 +1610,17 @@ pub async fn run_prompt_task( // follows as a second block. let mut slash_command: Option = None; let prompt_sections: Vec = if let Some(text) = prompt_text { - // Pre-built prompt (heartbeat or legacy path) — a single block. + // Heartbeats create their session before this point, so a Goose method-not-found + // probe has already selected the correct framing for this process. + let text = prepend_base_for_legacy( + if agent.has_system_prompt_support() { + 2 + } else { + 1 + }, + ctx.base_prompt, + &text, + ); vec![text] } else if let Some(ref b) = batch { // Build prompt from batch with context enrichment. @@ -1585,7 +1665,7 @@ pub async fn run_prompt_task( channel_info: channel_info.as_ref(), conversation_context: conversation_context.as_ref(), profile_lookup: profile_lookup.as_ref(), - has_system_prompt_support: agent.protocol_version >= 2, + has_system_prompt_support: agent.has_system_prompt_support(), base_prompt: ctx.base_prompt, system_prompt: ctx.system_prompt.as_deref(), team_instructions: ctx.team_instructions.as_deref(), @@ -3421,6 +3501,27 @@ mod tests { assert_eq!(composed, "hello channel"); } + #[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_eq!( + session_new_system_prompt(true, 2, Some("instructions")), + None + ); + assert_eq!( + session_new_system_prompt(false, 2, Some("instructions")), + Some("instructions") + ); + assert_eq!( + session_new_system_prompt(false, 1, Some("instructions")), + None + ); + } + #[test] fn test_initial_message_legacy_agent_without_base_is_unchanged() { // No base_prompt configured: nothing to prepend regardless of version. @@ -4507,6 +4608,8 @@ mod tests { model_capabilities: None, desired_model: None, model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, protocol_version: 2, }; @@ -4562,6 +4665,8 @@ mod tests { model_capabilities: None, desired_model: None, model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, protocol_version: 2, };