From 286f70c211aaee98954d8fca1023901886ddd8f4 Mon Sep 17 00:00:00 2001 From: sumit-m <33051892+sumit-m@users.noreply.github.com> Date: Sat, 1 Aug 2026 18:53:17 +0530 Subject: [PATCH] fix(buzz-acp): send legacy standing context once per session Protocol-v1 agents received base prompt, persona, team instructions, core memory and canvas on every turn. Deliver them in the session's first message only; both legacy paths now render through a shared StandingContext. Signed-off-by: sumit-m <33051892+sumit-m@users.noreply.github.com> --- crates/buzz-acp/src/lib.rs | 12 +- crates/buzz-acp/src/pool.rs | 261 +++++++++++++++++++---------------- crates/buzz-acp/src/queue.rs | 171 +++++++++++++++++------ 3 files changed, 280 insertions(+), 164 deletions(-) diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 65c9dd6203..287860730a 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -4574,17 +4574,23 @@ mod heartbeat_base_prompt_tests { // heartbeat user message, composed as `[Base]\n{bp}\n\n{prompt}`. This is // the second half of the round-2 regression (the first being initial_message). + fn heartbeat_standing() -> queue::StandingContext<'static> { + queue::StandingContext { + base_prompt: Some("you are a helpful agent"), + ..Default::default() + } + } + #[test] fn test_heartbeat_legacy_agent_gets_base_prepended() { // protocol_version 1 + Some(base_prompt): heartbeat prompt is prefixed // with the [Base] section exactly as the legacy session/new path would. let prompt = "[System: Heartbeat]\nrun feed get"; - let composed = pool::prepend_base_for_legacy(1, Some("you are a helpful agent"), prompt); + let composed = pool::prepend_standing_for_legacy(1, &heartbeat_standing(), prompt); assert_eq!( composed, "[Base]\nyou are a helpful agent\n\n[System: Heartbeat]\nrun feed get" ); - assert!(composed.starts_with("[Base]\nyou are a helpful agent\n\n")); } #[test] @@ -4592,7 +4598,7 @@ mod heartbeat_base_prompt_tests { // protocol_version 2 gets base_prompt via session/new; the heartbeat // prompt is sent verbatim. let prompt = "[System: Heartbeat]\nrun feed get"; - let composed = pool::prepend_base_for_legacy(2, Some("you are a helpful agent"), prompt); + let composed = pool::prepend_standing_for_legacy(2, &heartbeat_standing(), prompt); assert_eq!(composed, prompt); } } diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 8430307d9c..77fadab468 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -1205,42 +1205,30 @@ async fn apply_permission_mode( Ok(()) } -/// Prepend the `[Base]` section to a user-message body for legacy agents. +/// Prepend a legacy agent's standing context to a user-message body. /// -/// Legacy agents (`protocol_version < 2`) don't receive `base_prompt` via the -/// system role in `session/new`, so it must ride along in the user message. -/// Agents with `protocol_version >= 2`, or any agent without a `base_prompt`, -/// get `body` unchanged. The gate lives here so the heartbeat and -/// initial-message dispatch paths can't drift apart again. -pub(crate) fn prepend_base_for_legacy( - protocol_version: u32, - base_prompt: Option<&str>, - body: &str, -) -> String { - match base_prompt { - Some(bp) if protocol_version < 2 => { - format!("{}\n\n{body}", crate::queue::base_section(bp)) - } - _ => body.to_string(), - } -} - -/// Prepend the `[Channel Canvas]` section to the legacy initial-message body. +/// Legacy agents (`protocol_version < 2`) don't receive standing context via +/// the system role in `session/new`, so it must ride along in the user message +/// — in the session's *first* one, and never again. Agents with +/// `protocol_version >= 2`, or an empty [`StandingContext`], get `body` +/// unchanged. Both legacy dispatch paths (initial message, heartbeat) go +/// through this one gate so they can't drift apart again. /// -/// Protocol-v2 agents already receive the canvas in `systemPrompt`; only -/// legacy (protocol_version < 2) agents need it injected here so it arrives -/// before the first prompt — the same "every turn" semantics as per-turn core. -/// Heartbeats never have an initial_message, so the caller is responsible for -/// not passing a canvas when `source` is `Heartbeat`. -pub(crate) fn prepend_canvas_for_legacy( +/// A heartbeat passes base only: it has no channel, so there is no core or +/// canvas to carry, and it has never been given the persona. +pub(crate) fn prepend_standing_for_legacy( protocol_version: u32, - agent_canvas: Option<&str>, + standing: &crate::queue::StandingContext<'_>, body: &str, ) -> String { - match agent_canvas { - Some(canvas) if protocol_version < 2 => format!("{canvas}\n\n{body}"), - _ => body.to_string(), + if protocol_version >= 2 { + return body.to_string(); + } + let sections = standing.sections(); + if sections.is_empty() { + return body.to_string(); } + format!("{}\n\n{body}", sections.join("\n\n")) } /// Frame the `session/new` `systemPrompt` so each present prompt carries its own @@ -1713,6 +1701,22 @@ pub async fn run_prompt_task( }), ); + // Standing context is fixed for the life of a session. Agents with + // systemPrompt support already hold it from session/new; legacy agents + // receive it in the session's first user message and never again. + // + // `is_new_session` comes from the session registry, which is cleared + // whenever a session is invalidated — so the replacement session re-delivers + // rather than leaving the agent unbriefed. + let standing = crate::queue::StandingContext { + base_prompt: ctx.base_prompt, + system_prompt: ctx.system_prompt.as_deref(), + team_instructions: ctx.team_instructions.as_deref(), + agent_core: agent_core.as_deref(), + agent_canvas: agent_canvas.as_deref(), + }; + let mut standing_context_sent = !is_new_session; + if is_new_session { if let (PromptSource::Channel(cid), Some(ref initial_msg)) = (&source, &ctx.initial_message) { @@ -1720,30 +1724,15 @@ pub async fn run_prompt_task( target: "pool::session", "sending initial_message to session {session_id} for channel {cid}" ); - // For agents with systemPrompt support (protocol_version >= 2), - // base_prompt is delivered via the system role in session/new. - // Legacy agents receive it via [Base] in the user message instead. - // 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( + let init_msg = prepend_standing_for_legacy( if agent.has_system_prompt_support() { 2 } else { 1 }, - ctx.base_prompt, + &standing, initial_msg, ); - let init_msg = prepend_canvas_for_legacy( - if agent.has_system_prompt_support() { - 2 - } else { - 1 - }, - agent_canvas.as_deref(), - &init_msg, - ); let init_result = agent .acp .session_prompt_with_idle_timeout( @@ -1760,6 +1749,9 @@ pub async fn run_prompt_task( target: "pool::session", "initial_message complete for channel {cid}: {stop_reason:?}" ); + // The legacy agent has its standing context now; the turn + // prompt below must not repeat it. Every other arm returns. + standing_context_sent = true; } Err(AcpError::AgentExited) => { agent.state.invalidate_all(); @@ -1864,15 +1856,25 @@ pub async fn run_prompt_task( let prompt_sections: Vec = if let Some(text) = prompt_text { // 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, - ); + // + // Only the first heartbeat of a session carries `[Base]`; later ticks + // reuse the same session, so the agent already has it. + let text = if standing_context_sent { + text + } else { + prepend_standing_for_legacy( + if agent.has_system_prompt_support() { + 2 + } else { + 1 + }, + &crate::queue::StandingContext { + base_prompt: ctx.base_prompt, + ..Default::default() + }, + &text, + ) + }; vec![text] } else if let Some(ref b) = batch { // Build prompt from batch with context enrichment. @@ -1907,15 +1909,16 @@ pub async fn run_prompt_task( crate::queue::format_prompt( b, &crate::queue::FormatPromptArgs { - agent_core: agent_core.as_deref(), + agent_core: standing.agent_core, channel_info: channel_info.as_ref(), conversation_context: conversation_context.as_ref(), profile_lookup: profile_lookup.as_ref(), 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(), - agent_canvas: agent_canvas.as_deref(), + base_prompt: standing.base_prompt, + system_prompt: standing.system_prompt, + team_instructions: standing.team_instructions, + agent_canvas: standing.agent_canvas, + standing_context_sent, }, ) } else { @@ -4078,23 +4081,46 @@ mod tests { // a legacy agent WITH a base_prompt must get [Base] prepended to the user // message. This is the exact regression that shipped in the round-2 bug. + fn base_only(base_prompt: Option<&str>) -> crate::queue::StandingContext<'_> { + crate::queue::StandingContext { + base_prompt, + ..Default::default() + } + } + #[test] fn test_initial_message_legacy_agent_gets_base_prepended() { // protocol_version 1 + Some(base_prompt): [Base] rides along in the // user message, composed as `[Base]\n{bp}\n\n{initial_msg}`. - let composed = prepend_base_for_legacy(1, Some("you are a helpful agent"), "hello channel"); + let composed = prepend_standing_for_legacy( + 1, + &base_only(Some("you are a helpful agent")), + "hello channel", + ); assert_eq!(composed, "[Base]\nyou are a helpful agent\n\nhello channel"); - assert!(composed.starts_with("[Base]\nyou are a helpful agent\n\n")); } #[test] fn test_initial_message_modern_agent_omits_base() { // protocol_version 2 receives base_prompt via session/new, so the user // message is left untouched even when a base_prompt is present. - let composed = prepend_base_for_legacy(2, Some("you are a helpful agent"), "hello channel"); + let composed = prepend_standing_for_legacy( + 2, + &base_only(Some("you are a helpful agent")), + "hello channel", + ); assert_eq!(composed, "hello channel"); } + #[test] + fn test_heartbeat_standing_block_is_base_only() { + // A heartbeat has no channel, so core and canvas are absent by + // construction — and it has never carried the persona. Pin that the + // shared helper does not start handing heartbeats [System]. + let composed = prepend_standing_for_legacy(1, &base_only(Some("be helpful")), "tick"); + assert_eq!(composed, "[Base]\nbe helpful\n\ntick"); + } + #[test] fn goose_uses_system_prompt_only_after_custom_method_succeeds() { assert!(!has_system_prompt_support(2, "goose", None)); @@ -4149,82 +4175,75 @@ mod tests { #[test] fn test_initial_message_legacy_agent_without_base_is_unchanged() { // No base_prompt configured: nothing to prepend regardless of version. - let composed = prepend_base_for_legacy(1, None, "hello channel"); + let composed = prepend_standing_for_legacy(1, &base_only(None), "hello channel"); assert_eq!(composed, "hello channel"); } - // ── prepend_canvas_for_legacy ───────────────────────────────────────────── + // ── prepend_standing_for_legacy ─────────────────────────────────────────── + + fn full_standing() -> crate::queue::StandingContext<'static> { + crate::queue::StandingContext { + base_prompt: Some("be helpful"), + system_prompt: Some("you are Eva"), + team_instructions: Some("ship small"), + agent_core: Some("[Agent Memory — core]\nremember this"), + agent_canvas: Some("[Channel Canvas]\ncanvas content"), + } + } #[test] - fn test_initial_message_legacy_agent_gets_canvas_prepended() { - // Legacy agents (protocol_version < 2) receive the canvas section before - // the initial-message body so it arrives before the first prompt. - let canvas = "[Channel Canvas]\nCanvas revision (event ID): abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234\nLast modified: 2024-01-15T10:30:00Z\nFetch current content with: buzz canvas get --channel 00f1ccaf-1506-4dd7-9a0e-fa67e9e486ae"; - let composed = prepend_canvas_for_legacy(1, Some(canvas), "do the thing"); - assert!( - composed.starts_with("[Channel Canvas]"), - "canvas must precede the body" - ); - assert!( - composed.ends_with("do the thing"), - "body must follow the canvas" - ); + fn test_initial_message_legacy_agent_gets_whole_standing_block() { + // The initial message is the legacy agent's first contact, so it must + // carry every standing section — not just [Base] and the canvas, which + // left the agent acting on its first turn with no persona and no memory. + let composed = prepend_standing_for_legacy(1, &full_standing(), "do the thing"); + let positions: Vec = [ + "[Base]", + "[System]", + "[Team Instructions]", + "[Agent Memory — core]", + "[Channel Canvas]", + "do the thing", + ] + .iter() + .map(|needle| { + composed + .find(needle) + .unwrap_or_else(|| panic!("missing {needle} in: {composed}")) + }) + .collect(); assert!( - composed.contains("\n\ndo the thing"), - "canvas and body separated by blank line" + positions.windows(2).all(|w| w[0] < w[1]), + "sections must match the per-turn order, body last; got: {composed}" ); } #[test] - fn test_initial_message_modern_agent_omits_canvas_from_body() { - // Protocol-v2 agents receive canvas in systemPrompt; it must NOT be - // duplicated in the initial-message user turn. - let canvas = "[Channel Canvas]\nsome section"; - let composed = prepend_canvas_for_legacy(2, Some(canvas), "do the thing"); + fn test_initial_message_standing_order_matches_per_turn_order() { + // Both legacy paths render through StandingContext, so the initial + // message and a first-turn prompt agree section-for-section. + let standing = full_standing(); + let composed = prepend_standing_for_legacy(1, &standing, "do the thing"); assert_eq!( - composed, "do the thing", - "modern agent initial message must not contain canvas" - ); - assert!( - !composed.contains("[Channel Canvas]"), - "canvas must be absent from modern agent initial message" + composed, + format!("{}\n\ndo the thing", standing.sections().join("\n\n")) ); } #[test] - fn test_initial_message_legacy_agent_no_canvas_is_unchanged() { - // No canvas present: body passes through unmodified. - let composed = prepend_canvas_for_legacy(1, None, "do the thing"); + fn test_initial_message_modern_agent_omits_standing_block() { + // Protocol-v2 agents hold all of this from session/new; repeating it in + // the initial-message user turn would double-render every section. + let composed = prepend_standing_for_legacy(2, &full_standing(), "do the thing"); assert_eq!(composed, "do the thing"); } #[test] - fn test_initial_message_legacy_canvas_and_base_compose_correctly() { - // Verify the full composition order when both base and canvas are present: - // [Base] → canvas section → initial-message body. - let canvas = "[Channel Canvas]\ncanvas content"; - let base_composed = prepend_base_for_legacy(1, Some("be helpful"), "do the thing"); - let full = prepend_canvas_for_legacy(1, Some(canvas), &base_composed); - assert!( - full.starts_with("[Channel Canvas]"), - "canvas must be first in composed message" - ); - assert!( - full.contains("[Base]"), - "base must be present in composed message" - ); - assert!( - full.ends_with("do the thing"), - "body must be last in composed message" - ); - // Order: canvas → base → body - let canvas_pos = full.find("[Channel Canvas]").unwrap(); - let base_pos = full.find("[Base]").unwrap(); - let body_pos = full.find("do the thing").unwrap(); - assert!( - canvas_pos < base_pos && base_pos < body_pos, - "order must be: canvas → base → body" - ); + fn test_initial_message_legacy_agent_without_standing_is_unchanged() { + // Nothing configured: body passes through with no stray blank lines. + let composed = + prepend_standing_for_legacy(1, &crate::queue::StandingContext::default(), "do it"); + assert_eq!(composed, "do it"); } // Pin the session/new systemPrompt framing: each present prompt carries its diff --git a/crates/buzz-acp/src/queue.rs b/crates/buzz-acp/src/queue.rs index 5c960de202..755fc20bc7 100644 --- a/crates/buzz-acp/src/queue.rs +++ b/crates/buzz-acp/src/queue.rs @@ -1374,9 +1374,62 @@ pub struct FormatPromptArgs<'a> { /// /// For modern agents (protocol_version >= 2) the section is delivered via /// the system role in session/new; omit here to avoid duplication. - /// For legacy agents it rides in the user message on every turn of the - /// session, alongside `[Base]`/`[System]`/`[Agent Memory — core]`. pub agent_canvas: Option<&'a str>, + /// Set once this session's standing context has already been delivered — + /// see [`StandingContext`]. Only meaningful for legacy agents; modern + /// agents are gated by `has_system_prompt_support` regardless. + /// + /// Defaults to `false` so a caller that never sets it behaves as if this + /// were the session's first message. + pub standing_context_sent: bool, +} + +/// The prompt sections that do not change for the life of a session: base +/// prompt, persona, team instructions, core memory, and channel canvas. +/// +/// Protocol-v2 agents receive all of this through the system role at +/// `session/new`, once. Legacy agents (`protocol_version < 2`) have no system +/// role, so it has to ride in a user message — but only in the session's +/// *first* one. Re-sending it every turn makes the standing framing the newest +/// and most-repeated text in the window, outweighing the conversation it exists +/// to frame, and evicting real channel history that much sooner. +/// +/// Both legacy dispatch paths (initial message, batch flush) render through +/// this one type so their section set and ordering cannot drift apart. +#[derive(Default)] +pub(crate) struct StandingContext<'a> { + pub base_prompt: Option<&'a str>, + pub system_prompt: Option<&'a str>, + pub team_instructions: Option<&'a str>, + pub agent_core: Option<&'a str>, + pub agent_canvas: Option<&'a str>, +} + +impl StandingContext<'_> { + /// Render the sections in the order legacy agents have always seen them. + pub(crate) fn sections(&self) -> Vec { + let mut sections = Vec::with_capacity(5); + if let Some(bp) = self.base_prompt { + sections.push(base_section(bp)); + } + if let Some(sp) = self.system_prompt { + sections.push(format!("[System]\n{sp}")); + } + if let Some(team) = self + .team_instructions + .map(str::trim) + .filter(|value| !value.is_empty()) + { + sections.push(format!("[Team Instructions]\n{team}")); + } + if let Some(core) = self.agent_core { + sections.push(core.to_string()); + } + if let Some(canvas) = self.agent_canvas { + sections.push(canvas.to_string()); + } + sections + } } /// Format the `[Base]` section for the base prompt. @@ -1391,12 +1444,12 @@ pub(crate) fn base_section(base_prompt: &str) -> String { /// Format a [`FlushBatch`] into the per-section prompt blocks for the agent. /// /// Produces a stable prompt with these sections (in order): -/// 0. `[Base]` — base prompt (only for legacy agents without systemPrompt support) -/// 1. `[System]` — system prompt (only for legacy agents without systemPrompt support) -/// 2. `[Agent Memory — core]` — if agent core memory is set -/// 3. `[Context]` — scope, channel name, and contextual hints for the agent -/// 4. `[Thread Context]` or `[Conversation Context]` — if fetched -/// 5. `[Event]` / `[Buzz events]` — the triggering event(s) +/// 0. [`StandingContext`] — `[Base]`, `[System]`, `[Team Instructions]`, +/// `[Agent Memory — core]`, `[Channel Canvas]`. Legacy agents only, and only +/// on the session's first message (see `standing_context_sent`) +/// 1. `[Context]` — scope, channel name, and contextual hints for the agent +/// 2. `[Thread Context]` or `[Conversation Context]` — if fetched +/// 3. `[Event]` / `[Buzz events]` — the triggering event(s) /// /// Each section is returned as its own block rather than one joined string so /// the observer frame's size trimmer (`fit_observer_event_to_budget`) elides @@ -1428,38 +1481,22 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec = Vec::with_capacity(7); - // For legacy agents (protocol_version < 2), inject base_prompt and - // system_prompt as user-message sections. Modern agents receive these - // via the system role in session/new. - if !args.has_system_prompt_support { - if let Some(bp) = args.base_prompt { - sections.push(base_section(bp)); - } - if let Some(sp) = args.system_prompt { - sections.push(format!("[System]\n{sp}")); - } - if let Some(team) = args - .team_instructions - .map(str::trim) - .filter(|value| !value.is_empty()) - { - sections.push(format!("[Team Instructions]\n{team}")); - } - } - - // NIP-AE agent core memory (rendered by `engram_fetch::build_core_section`). - // For modern agents (protocol_version >= 2), core is delivered via the - // system role in session/new, so it is omitted here to avoid duplication. - // Legacy agents have no system role, so core rides in the user message - // alongside `[Base]`/`[System]`. - if !args.has_system_prompt_support { - if let Some(core) = args.agent_core { - sections.push(core.to_string()); - } - // Channel canvas metadata — same delivery semantics as core for legacy agents. - if let Some(canvas) = args.agent_canvas { - sections.push(canvas.to_string()); - } + // Standing context — base prompt, persona, team instructions, core memory + // and canvas. Modern agents received all of it via the system role in + // session/new. Legacy agents get it here, in the session's first message + // only; `standing_context_sent` means an earlier message in this session + // already carried it. + if !args.has_system_prompt_support && !args.standing_context_sent { + sections.extend( + StandingContext { + base_prompt: args.base_prompt, + system_prompt: args.system_prompt, + team_instructions: args.team_instructions, + agent_core: args.agent_core, + agent_canvas: args.agent_canvas, + } + .sections(), + ); } // 2. Context hints (with a human-aware reply anchor). @@ -2408,6 +2445,60 @@ mod tests { ); } + #[test] + fn test_format_prompt_legacy_agent_omits_standing_after_first_message() { + // The defect this pins: standing context was re-sent on every turn of a + // legacy session, so the largest and least informative part of the + // prompt was also the most recent — crowding out the conversation and + // evicting real channel history sooner. + let ch = Uuid::new_v4(); + let batch = FlushBatch { + channel_id: ch, + events: vec![BatchEvent { + event: make_event("hello"), + prompt_tag: "test".into(), + received_at: Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let canvas = "[Channel Canvas]\ncanvas content"; + let core = "[Agent Memory — core]\nremember this"; + let args = |sent| FormatPromptArgs { + has_system_prompt_support: false, + base_prompt: Some("test base prompt"), + system_prompt: Some("test system prompt"), + team_instructions: Some("ship small"), + agent_core: Some(core), + agent_canvas: Some(canvas), + standing_context_sent: sent, + ..Default::default() + }; + + let first = format_prompt(&batch, &args(false)).join("\n\n"); + let later = format_prompt(&batch, &args(true)).join("\n\n"); + + for section in [ + "[Base]", + "[System]", + "[Team Instructions]", + "[Agent Memory — core]", + "[Channel Canvas]", + ] { + assert!(first.contains(section), "first message missing {section}"); + assert!(!later.contains(section), "turn 2 repeated {section}"); + } + // What the turn is actually about survives, and now leads. + assert!(later.starts_with("[Context]"), "got: {later}"); + assert!(later.contains("hello")); + assert!( + later.len() < first.len(), + "later turns must be smaller: {} vs {}", + later.len(), + first.len() + ); + } + #[test] fn test_format_prompt_modern_agent_suppresses_base_and_system() { let ch = Uuid::new_v4();