mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
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>
This commit is contained in:
parent
2be524e9a3
commit
d4e1a773a7
@@ -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<serde_json::Value, AcpError> {
|
||||
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.
|
||||
|
||||
@@ -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<observer::ObserverHandle>,
|
||||
) -> 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,
|
||||
|
||||
+132
-27
@@ -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<bool>,
|
||||
/// 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>,
|
||||
) -> 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<String, AcpError> {
|
||||
// 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<String> = 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<String> = None;
|
||||
let prompt_sections: Vec<String> = 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,
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user