From ab5b289488908f2a76ab7db2beb5ccb86a3a87b9 Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Wed, 12 Aug 2026 15:18:27 +1000 Subject: [PATCH] fix(buzz-agent): restore the env surface main had The port dropped BUZZ_AGENT_* knobs whose implementation moved to goose. That conflated two different things: goose owning the mechanism, and the setting silently ceasing to work. Someone who set BUZZ_AGENT_TOOL_TIMEOUT_SECS=1200 got no error and a 300s timeout. Restored as mappings onto goose, defaults matching main: - TOOL_TIMEOUT_SECS -> GOOSE_DEFAULT_EXTENSION_TIMEOUT - MAX_TOOL_RESULT_TEXT_BYTES -> GOOSE_MAX_TOOL_RESPONSE_SIZE - NO_HINTS -> CONTEXT_FILE_NAMES=[] - STOP_MAX_REJECTIONS -> the veto cap, configurable again (0 disables) Both timeout and truncation project buzz's *default* as well as an explicit value, because goose's differ (300s vs 660s, 200KB vs 50KB) -- mapping only explicit values would still have changed behaviour for everyone who never set them. Two silent regressions fixed: - MAX_SESSIONS defaulted to 8; main was unlimited. A busy agent would have started refusing sessions it used to accept. - The reply guard was gone entirely. Desktop still enables it by default for shared-compute agents (relay_mesh.rs), so the flag was set and ignored. Restored as BuzzReplyGuardOperation with main's nag text, two-reminder budget, and publish-shaped-call latch. README: every removed knob is now listed as 'No longer read' with what replaced it, rather than documented as if still live. 74 unit + 21 integration green, handtest --all 18/18. Signed-off-by: Michael Neale Co-authored-by: Michael Neale --- crates/buzz-agent/README.md | 25 ++- crates/buzz-agent/src/config.rs | 168 ++++++++++++++++++- crates/buzz-agent/src/hooks.rs | 18 ++- crates/buzz-agent/src/lib.rs | 1 + crates/buzz-agent/src/loop_drive.rs | 5 + crates/buzz-agent/src/ops.rs | 242 +++++++++++++++++++++++++++- 6 files changed, 447 insertions(+), 12 deletions(-) diff --git a/crates/buzz-agent/README.md b/crates/buzz-agent/README.md index 5d942777d5..de7b64617f 100644 --- a/crates/buzz-agent/README.md +++ b/crates/buzz-agent/README.md @@ -155,17 +155,28 @@ Everything is environment variables. No flags, no config files. (We are a subpro | `BUZZ_AGENT_MAX_ROUNDS` | `0` | Tool-loop iteration cap. 0 = unlimited. | | `BUZZ_AGENT_MAX_OUTPUT_TOKENS` | `32768` | Per LLM call. Headroom for large tool-call inputs (e.g. file writes via heredoc); Sonnet 4 / Opus 4 cap at 64K. | | `BUZZ_AGENT_MAX_CONTEXT_TOKENS` | `200000` | Provider context window used by the handoff gate. | -| `BUZZ_AGENT_MAX_HANDOFFS` | `10` | Max context handoffs per session before falling back to truncation. | -| `BUZZ_AGENT_LLM_TIMEOUT_SECS` | `240` | Max seconds with no response bytes before abandoning an LLM call (per-read inactivity, not wall-clock). | -| `BUZZ_AGENT_TOOL_TIMEOUT_SECS` | `660` | Per-tool call timeout in seconds | -| `BUZZ_AGENT_MAX_PARALLEL_TOOLS` | `8` | Max concurrent tool calls per turn (1 = sequential) | +| `BUZZ_AGENT_MAX_HANDOFFS` | — | **No longer read.** goose's compaction replaced the handoff mechanism it bounded. | +| `BUZZ_AGENT_LLM_TIMEOUT_SECS` | — | **No longer read.** goose owns provider transport and its own per-provider timeouts (`OPENAI_TIMEOUT`, `ANTHROPIC_TIMEOUT`, default 600s). | +| `BUZZ_AGENT_TOOL_TIMEOUT_SECS` | `660` | Per-tool call timeout in seconds. Projected onto goose's `GOOSE_DEFAULT_EXTENSION_TIMEOUT`. | +| `BUZZ_AGENT_MAX_PARALLEL_TOOLS` | — | **No longer read.** All of a round's tool calls are dispatched concurrently; there is no cap to configure. | | `BUZZ_AGENT_MAX_SESSIONS` | unlimited | Max concurrent ACP sessions. Sessions are cheap; default has no cap. | -| `BUZZ_AGENT_MAX_LINE_BYTES` | `4194304` | 4 MiB. Hard cap on inbound JSON-RPC frames. | -| `BUZZ_AGENT_MAX_HISTORY_BYTES` | `1048576` | 1 MiB. Old turns are evicted past this. | -| `BUZZ_AGENT_MAX_TOOL_RESULT_TEXT_BYTES` | `51200` | 50 KiB. Per-result cap on tool-output text; oversize is middle-elided (head + tail kept) with an inline marker. Images are exempt. | +| `BUZZ_AGENT_MAX_LINE_BYTES` | — | **No longer read.** The frame cap is a fixed 16 MiB protocol limit (`config.rs::MAX_LINE_BYTES`). | +| `BUZZ_AGENT_MAX_HISTORY_BYTES` | — | **No longer read.** Byte-based eviction is replaced by goose's token-aware compaction; size the window with `BUZZ_AGENT_MAX_CONTEXT_TOKENS`. | +| `BUZZ_AGENT_MAX_TOOL_RESULT_TEXT_BYTES` | `51200` | 50 KiB. Per-result cap on tool-output text. Projected onto goose's `GOOSE_MAX_TOOL_RESPONSE_SIZE` (whose own default is 200 KiB). | | `BUZZ_AGENT_REQUIRE_REPLY` | `0` (`1` on mesh) | `1` enables the [reply guard](#reply-guard) — remind the model to publish when a turn is about to end with nothing posted to Buzz. Desktop defaults it to `1` for Buzz shared-compute agents. | +| `BUZZ_AGENT_STOP_MAX_REJECTIONS` | `3` | Max consecutive `_Stop` vetoes per turn. `0` disables the `_Stop` veto entirely. | +| `BUZZ_AGENT_NO_HINTS` | `0` | `1` suppresses `AGENTS.md` / `.goosehints` loading (sets goose's `CONTEXT_FILE_NAMES` empty). | +| `BUZZ_AGENT_APPROVAL` | `auto` | Tool-call approval policy passed to goose: `auto` \| `approve` \| `smart_approve` \| `chat`. `auto` matches previous behaviour. | +| `BUZZ_AGENT_HOOK_TIMEOUT_MS` | — | **No longer read.** Hook tools are dispatched through goose's normal tool path and share its extension timeout. | +| `BUZZ_AGENT_PROMPT_CACHING` | — | **No longer read.** goose's providers manage prompt caching. | +| `BUZZ_AGENT_THINKING_SUMMARY` | — | **No longer read.** goose's providers own reasoning-summary handling; `BUZZ_AGENT_THINKING_EFFORT` still applies. | +| `BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS`, `BUZZ_AGENT_MCP_RESTART_*` | — | **No longer read.** goose's `ExtensionManager` owns MCP startup and restart. | +> Variables marked **No longer read** were live in the pre-goose agent. They are +> listed rather than deleted so anyone who set one can see that it now has no +> effect, and what replaced it. Everything else behaves as it did before. + ## Reply Guard Off by default, except on Buzz shared-compute (mesh) agents, where Buzz Desktop diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index 693b493469..2a2b819f41 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -52,6 +52,10 @@ pub struct Config { /// this yet — wiring it to a real human affordance is the point of the /// isolation work, and this is the seam it will use. pub goose_mode: GooseMode, + /// Remind the model to publish when a turn is about to end with no + /// recognised attempt to post to Buzz. Default off; Desktop opts + /// shared-compute agents in via `BUZZ_AGENT_REQUIRE_REPLY=1`. + pub require_reply: bool, // ---- Databricks model-discovery fields ------------------------------- // Goose owns provider auth for the agent loop, but the desktop model @@ -141,9 +145,13 @@ impl Config { Self { model: env_str("BUZZ_AGENT_MODEL"), max_rounds: env_parse::("BUZZ_AGENT_MAX_ROUNDS").filter(|n| *n > 0), - max_sessions: env_parse("BUZZ_AGENT_MAX_SESSIONS").unwrap_or(8), + // `usize::MAX`, not a small number: main defaulted to unlimited + // and an agent that never set this must not start refusing + // sessions after some arbitrary count. + max_sessions: env_parse("BUZZ_AGENT_MAX_SESSIONS").unwrap_or(usize::MAX), system_prompt, goose_mode: parse_approval(env_str("BUZZ_AGENT_APPROVAL").as_deref()), + require_reply: env_str("BUZZ_AGENT_REQUIRE_REPLY").is_some_and(|v| v != "0"), // Discovery-only; the agent loop resolves providers through goose. provider: Provider::default(), api_key: String::new(), @@ -162,6 +170,7 @@ impl Config { max_sessions: 1, system_prompt: None, goose_mode: GooseMode::default(), + require_reply: false, provider, api_key, base_url, @@ -221,9 +230,56 @@ impl Config { if let Some(ctx) = env_str("BUZZ_AGENT_MAX_CONTEXT_TOKENS") { set_if_absent("GOOSE_CONTEXT_LIMIT", &ctx); } + + // ---- Knobs whose implementation moved to goose ------------------ + // + // goose owning the *mechanism* must not mean the `BUZZ_AGENT_*` name + // stops working. Anyone who set one of these against buzz-agent on + // main gets the same effect here, because we translate it onto goose's + // equivalent. Silently ignoring a knob someone deliberately set is the + // worst outcome: no error, just different behaviour. + // + // `set_if_absent`, unlike the provider/model pair above: these have no + // structured field behind them, so an explicit `GOOSE_*` in the + // environment is a deliberate override and should win. + + // Per-tool-call timeout. buzz's default was 660s; goose's is 300s + // (`DEFAULT_EXTENSION_TIMEOUT`), so leaving it unset would silently + // shorten every long tool call. We therefore project buzz's default + // too, not just an explicit value. + set_if_absent( + "GOOSE_DEFAULT_EXTENSION_TIMEOUT", + &env_str("BUZZ_AGENT_TOOL_TIMEOUT_SECS") + .unwrap_or_else(|| DEFAULT_TOOL_TIMEOUT_SECS.to_string()), + ); + + // Tool-result truncation threshold. Same reasoning: buzz truncated at + // 50 KB, goose at 200 KB, so the default has to be carried across or a + // tool returning 100 KB reaches the model on this branch when it did + // not on main. + set_if_absent( + "GOOSE_MAX_TOOL_RESPONSE_SIZE", + &env_str("BUZZ_AGENT_MAX_TOOL_RESULT_TEXT_BYTES") + .unwrap_or_else(|| DEFAULT_TOOL_RESULT_TEXT_BYTES.to_string()), + ); + + // `BUZZ_AGENT_NO_HINTS=1` suppressed AGENTS.md/.goosehints loading. + // goose has no boolean for this, but it takes the *filename list* — + // an empty list finds nothing, which is the same outcome. + if env_str("BUZZ_AGENT_NO_HINTS").is_some_and(|v| v != "0") { + set_if_absent("CONTEXT_FILE_NAMES", "[]"); + } } } +/// Per-tool-call timeout, in seconds. buzz-agent's long-standing default, +/// preserved because goose's is shorter (300s). +const DEFAULT_TOOL_TIMEOUT_SECS: u64 = 660; + +/// Tool-result text truncation threshold, in bytes. buzz-agent's default, +/// preserved because goose's is larger (200 KB). +const DEFAULT_TOOL_RESULT_TEXT_BYTES: usize = 50 * 1024; + fn set_if_absent(key: &str, value: &str) { if std::env::var_os(key).is_none() { std::env::set_var(key, value); @@ -234,9 +290,119 @@ fn set_if_absent(key: &str, value: &str) { mod tests { use super::*; + /// Process env is global; tests that mutate it must not interleave. + fn env_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + } + // Env is process-global; these tests set disjoint keys and assert only on // the pure mapping helpers where possible. + /// Every `BUZZ_AGENT_*` knob whose implementation moved to goose must + /// still reach goose under its goose name. A knob that silently stops + /// working is the regression this test exists to prevent. + /// + /// Serialised by the shared env lock: `project_goose_env` mutates process + /// globals. + #[test] + fn knobs_that_moved_to_goose_still_reach_it() { + let _guard = env_lock(); + let keys = [ + "BUZZ_AGENT_TOOL_TIMEOUT_SECS", + "BUZZ_AGENT_MAX_TOOL_RESULT_TEXT_BYTES", + "BUZZ_AGENT_NO_HINTS", + "GOOSE_DEFAULT_EXTENSION_TIMEOUT", + "GOOSE_MAX_TOOL_RESPONSE_SIZE", + "CONTEXT_FILE_NAMES", + ]; + for key in keys { + std::env::remove_var(key); + } + + std::env::set_var("BUZZ_AGENT_TOOL_TIMEOUT_SECS", "1200"); + std::env::set_var("BUZZ_AGENT_MAX_TOOL_RESULT_TEXT_BYTES", "12345"); + std::env::set_var("BUZZ_AGENT_NO_HINTS", "1"); + Config::project_goose_env(); + + assert_eq!( + std::env::var("GOOSE_DEFAULT_EXTENSION_TIMEOUT").unwrap(), + "1200" + ); + assert_eq!( + std::env::var("GOOSE_MAX_TOOL_RESPONSE_SIZE").unwrap(), + "12345" + ); + assert_eq!(std::env::var("CONTEXT_FILE_NAMES").unwrap(), "[]"); + + for key in keys { + std::env::remove_var(key); + } + } + + /// buzz's defaults differ from goose's, so leaving them unset would change + /// behaviour for everyone who never touched these knobs: tool calls would + /// time out at 300s instead of 660s, and tool output would truncate at + /// 200 KB instead of 50 KB. + #[test] + fn buzz_defaults_are_projected_not_left_to_goose() { + let _guard = env_lock(); + for key in [ + "BUZZ_AGENT_TOOL_TIMEOUT_SECS", + "BUZZ_AGENT_MAX_TOOL_RESULT_TEXT_BYTES", + "GOOSE_DEFAULT_EXTENSION_TIMEOUT", + "GOOSE_MAX_TOOL_RESPONSE_SIZE", + ] { + std::env::remove_var(key); + } + + Config::project_goose_env(); + + assert_eq!( + std::env::var("GOOSE_DEFAULT_EXTENSION_TIMEOUT").unwrap(), + "660", + "buzz's tool timeout, not goose's 300s" + ); + assert_eq!( + std::env::var("GOOSE_MAX_TOOL_RESPONSE_SIZE").unwrap(), + (50 * 1024).to_string(), + "buzz's truncation threshold, not goose's 200 KB" + ); + + for key in [ + "GOOSE_DEFAULT_EXTENSION_TIMEOUT", + "GOOSE_MAX_TOOL_RESPONSE_SIZE", + ] { + std::env::remove_var(key); + } + } + + /// main defaulted to unlimited concurrent sessions. A small default would + /// make a busy agent start refusing sessions it used to accept. + #[test] + fn max_sessions_defaults_to_unlimited() { + let _guard = env_lock(); + std::env::remove_var("BUZZ_AGENT_MAX_SESSIONS"); + assert_eq!(Config::from_env().max_sessions, usize::MAX); + } + + /// Desktop enables this for shared-compute agents; it must be read. + #[test] + fn require_reply_is_read_from_the_environment() { + let _guard = env_lock(); + std::env::remove_var("BUZZ_AGENT_REQUIRE_REPLY"); + assert!(!Config::from_env().require_reply); + + std::env::set_var("BUZZ_AGENT_REQUIRE_REPLY", "1"); + assert!(Config::from_env().require_reply); + + // An explicit 0 opts out, which relay_mesh.rs goes out of its way to + // preserve through the spawn path. + std::env::set_var("BUZZ_AGENT_REQUIRE_REPLY", "0"); + assert!(!Config::from_env().require_reply); + std::env::remove_var("BUZZ_AGENT_REQUIRE_REPLY"); + } + #[test] fn set_if_absent_does_not_clobber() { std::env::set_var("BUZZ_TEST_EXISTING", "native"); diff --git a/crates/buzz-agent/src/hooks.rs b/crates/buzz-agent/src/hooks.rs index 6fdb8db5be..3f2e51a499 100644 --- a/crates/buzz-agent/src/hooks.rs +++ b/crates/buzz-agent/src/hooks.rs @@ -38,10 +38,24 @@ use goose::agents::Agent; use goose::session::session_manager::Session; use rmcp::model::CallToolRequestParams; -/// Hard cap on consecutive vetoes, mirroring goose's own `stop_hook_block_cap` -/// (`agent.rs:108-119`). A tool that always objects must not trap the turn. +/// Default cap on consecutive vetoes, mirroring goose's own +/// `stop_hook_block_cap` (`agent.rs:108-119`). A tool that always objects must +/// not trap the turn. pub const MAX_STOP_BLOCKS: u32 = 3; +/// The veto cap in force, from `BUZZ_AGENT_STOP_MAX_REJECTIONS`. +/// +/// Configurable rather than hardcoded because it was configurable on main, and +/// **`0` disables the `_Stop` veto entirely** — documented behaviour there +/// ("set to 0 to disable `_Stop` hooks entirely"), which an operator with a +/// misbehaving hook relies on. A hardcoded cap silently takes that away. +pub fn stop_block_cap() -> u32 { + std::env::var("BUZZ_AGENT_STOP_MAX_REJECTIONS") + .ok() + .and_then(|raw| raw.trim().parse().ok()) + .unwrap_or(MAX_STOP_BLOCKS) +} + /// Tools are advertised to the model prefixed with their extension name /// (`extension_manager.rs:1425`), and dispatch resolves the same prefixed /// form. `_Stop` is hidden from the model but still callable by us. diff --git a/crates/buzz-agent/src/lib.rs b/crates/buzz-agent/src/lib.rs index b94c1d93b2..ceca9b6ca5 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -753,6 +753,7 @@ async fn session_prompt(app: &Arc, id: Value, params: Value, wire_tx: &Wire wire_tx, cancel: &cancel, hook_extension: hook_extension.as_deref(), + require_reply: app.cfg.require_reply, max_rounds: app.cfg.max_rounds, prompt: &prompt, steers: &steers, diff --git a/crates/buzz-agent/src/loop_drive.rs b/crates/buzz-agent/src/loop_drive.rs index 2c9697426d..30f37219d8 100644 --- a/crates/buzz-agent/src/loop_drive.rs +++ b/crates/buzz-agent/src/loop_drive.rs @@ -108,6 +108,10 @@ pub struct TurnContext<'a> { /// buzz-agent holds this across turns itself now that the turn's /// conversation is in memory rather than in goose's database. pub history: &'a [Message], + /// Whether the reply guard is armed for this session + /// (`BUZZ_AGENT_REQUIRE_REPLY`). Desktop turns it on by default for + /// shared-compute agents. + pub require_reply: bool, /// Provider and model config for this turn. Read from here rather than /// from goose's `Agent`, which resolves the config out of its session /// store; see [`crate::model`]. @@ -161,6 +165,7 @@ pub async fn run_turn( ctx.cancel.clone(), ctx.hook_extension .map(|extension| (Arc::clone(ctx.agent), extension.to_string())), + ctx.require_reply, ); let (event_tx, mut event_rx) = tokio::sync::mpsc::channel(16); // Operations may emit; nothing in this step does, but a dropped receiver diff --git a/crates/buzz-agent/src/ops.rs b/crates/buzz-agent/src/ops.rs index 666042275b..5c24455afd 100644 --- a/crates/buzz-agent/src/ops.rs +++ b/crates/buzz-agent/src/ops.rs @@ -28,7 +28,7 @@ use goose::agents::state_machine::{ use goose::agents::Agent; use goose::conversation::Conversation; use goose::session::Session; -use goose_provider_types::conversation::message::Message; +use goose_provider_types::conversation::message::{Message, ToolRequest}; use crate::types::StopReason; @@ -282,6 +282,125 @@ impl Operation for BuzzStopVetoOperation { } } +/// Reminds the model to publish when a turn is about to end with nothing +/// posted to Buzz. +/// +/// Ported from main's reply guard (`agent.rs`, pre-goose), which this branch +/// dropped. Desktop still enables it by default for shared-compute/mesh agents +/// (`relay_mesh.rs`), so without this the flag is set and silently ignored. +/// +/// Advisory: at most [`MAX_REPLY_NAGS`] reminders, then the turn ends whether +/// or not anything was published. The guard catches accidental omission; it +/// does not compel speech. +pub struct BuzzReplyGuardOperation { + max_nags: u32, +} + +/// Reminder text. Kept byte-identical to main's. +/// +/// Explicitly licenses silence: the base prompt tells agents publishing is +/// optional and "silence is usually correct", and a reminder that argued +/// otherwise would fight that instruction and make agents chattier. +const REPLY_GUARD_NAG: &str = "You are about to end this turn without calling `buzz messages send`. \ +Your assistant text and reasoning are never shown to anyone — if you did work, found an answer, \ +or hit a blocker that someone is waiting on, it exists only if you publish it. \ +If you already posted, or if silence is genuinely correct for this turn, ignore this and end your turn."; + +/// Maximum reminders per turn, as on main. +pub const MAX_REPLY_NAGS: u32 = 2; + +/// Metadata key marking a reminder, so the budget survives in the +/// conversation rather than in a loop-local counter. +const NAGGED: &str = "nagged"; + +impl BuzzReplyGuardOperation { + pub fn new(max_nags: u32) -> Self { + Self { max_nags } + } + + /// Whether a tool request is a recognised attempt to publish to Buzz. + /// + /// An *attempt*, not a success: a failed send already returns a non-zero + /// exit and error JSON to the model, which is louder than this reminder. + /// + /// Same matcher as main's `is_reply_shaped`, with one simplification the + /// port allows: main checked `mcp.has(name) && !mcp.is_hook(name)` first + /// because a hallucinated `fake__shell` would otherwise disarm the guard. + /// Here the conversation only contains requests goose actually dispatched, + /// so a hallucinated name never reaches this code. + fn is_reply_shaped(request: &ToolRequest) -> bool { + let Ok(call) = &request.tool_call else { + return false; + }; + if !call.name.ends_with("__shell") { + return false; + } + call.arguments + .as_ref() + .and_then(|args| args.get("command")) + .and_then(|command| command.as_str()) + // Coarse substring test against the structured `command` field, as + // on main: `messages send` also covers `send-diff`, and `reactions + // add` counts because the base prompt tells agents to react rather + // than post a bare acknowledgement. Missing a real post is the + // expensive direction, so the forgiving match is the right one. + .is_some_and(|cmd| cmd.contains("messages send") || cmd.contains("reactions add")) + } +} + +#[async_trait] +impl Operation for BuzzReplyGuardOperation { + fn name(&self) -> &'static str { + "buzz_reply_guard" + } + + async fn run( + &self, + _session: &Session, + conversation: &Conversation, + _emit: &Emitter, + ) -> Result { + let Some(messages) = messages_since_kickoff(conversation) else { + return not_applicable(); + }; + if !ends_turn(messages) { + return not_applicable(); + } + + let nags = messages + .iter() + .filter(|message| self.message_meta(message, NAGGED).is_some()) + .count() as u32; + if nags >= self.max_nags { + return not_applicable(); + } + + // Any publish-shaped call this turn disarms the guard for the rest of + // it, matching main's `buzz_reply_call_seen` latch. + let published = messages.iter().any(|message| { + message.content.iter().any(|content| { + matches!( + content, + goose_provider_types::conversation::message::MessageContent::ToolRequest( + request, + ) if Self::is_reply_shaped(request) + ) + }) + }); + if published { + return not_applicable(); + } + + tracing::info!(nags = nags + 1, "reply guard reminded the model to publish"); + + let mut message = Message::user() + .with_text(REPLY_GUARD_NAG) + .with_visibility(false, true); + self.set_message_meta(&mut message, NAGGED, serde_json::json!(true)); + applied([message.into()]) + } +} + /// The operations that gate the start of a round. /// /// One entry today. As `PLANS/BUZZ_OPERATIONS_MIGRATION.md` proceeds, the @@ -292,6 +411,7 @@ pub fn round_gate( outcome: Outcome, cancel: tokio_util::sync::CancellationToken, stop_veto: Option<(Arc, String)>, + require_reply: bool, ) -> goose::agents::state_machine::StateMachine<'static> { use goose::agents::state_machine::Step; @@ -305,7 +425,15 @@ pub fn round_gate( steps.push(Step::Operation(Arc::new(BuzzStopVetoOperation::new( agent, extension, - crate::hooks::MAX_STOP_BLOCKS, + crate::hooks::stop_block_cap(), + )))); + } + // After the `_Stop` veto: on main both objections rode the same gate, and + // a hook objection already keeps the turn alive, so reminding as well + // would spend two rounds where main spent one. + if require_reply { + steps.push(Step::Operation(Arc::new(BuzzReplyGuardOperation::new( + MAX_REPLY_NAGS, )))); } goose::agents::state_machine::StateMachine::new(steps, cancel) @@ -460,6 +588,116 @@ mod tests { assert!(matches!(result, OperationResult::NotApplicable)); } + fn shell_request(command: &str) -> Message { + let mut args = serde_json::Map::new(); + args.insert("command".into(), serde_json::json!(command)); + Message::assistant().with_tool_request( + "call_1", + Ok(rmcp::model::CallToolRequestParams::new("developer__shell").with_arguments(args)), + ) + } + + /// A turn that ran a tool and then answered. + fn turn_with(request: Message) -> Conversation { + Conversation::new_unvalidated(vec![ + Message::user().with_text("kickoff"), + request, + tool_response(), + Message::assistant().with_text("done"), + ]) + } + + #[tokio::test] + async fn the_reply_guard_reminds_when_nothing_was_published() { + let op = BuzzReplyGuardOperation::new(MAX_REPLY_NAGS); + let result = op + .run(&Session::default(), &conversation(1), &emitter()) + .await + .unwrap(); + match result { + OperationResult::Applied(step) => { + assert!(!step.yield_to_client, "a reminder must not end the turn"); + assert_eq!(step.effects.len(), 1); + } + OperationResult::NotApplicable => panic!("expected a reminder"), + } + } + + #[tokio::test] + async fn a_publish_attempt_disarms_the_guard() { + let op = BuzzReplyGuardOperation::new(MAX_REPLY_NAGS); + for command in [ + "buzz messages send --channel x --content hi", + "buzz messages send-diff --channel x", + "buzz reactions add --event x --emoji +1", + ] { + let result = op + .run( + &Session::default(), + &turn_with(shell_request(command)), + &emitter(), + ) + .await + .unwrap(); + assert!( + matches!(result, OperationResult::NotApplicable), + "`{command}` should disarm the guard" + ); + } + } + + #[tokio::test] + async fn an_unrelated_tool_call_does_not_disarm_the_guard() { + let op = BuzzReplyGuardOperation::new(MAX_REPLY_NAGS); + let result = op + .run( + &Session::default(), + &turn_with(shell_request("ls -la")), + &emitter(), + ) + .await + .unwrap(); + assert!(matches!(result, OperationResult::Applied(_))); + } + + #[tokio::test] + async fn the_guard_stops_after_its_budget() { + // Advisory, not compulsion: after MAX_REPLY_NAGS the turn ends whether + // or not anything was published. + let op = BuzzReplyGuardOperation::new(MAX_REPLY_NAGS); + let mut nag = Message::user() + .with_text(REPLY_GUARD_NAG) + .with_visibility(false, true); + op.set_message_meta(&mut nag, NAGGED, serde_json::json!(true)); + + let mut messages = vec![Message::user().with_text("kickoff")]; + for _ in 0..MAX_REPLY_NAGS { + messages.push(nag.clone()); + messages.push(Message::assistant().with_text("done")); + } + let result = op + .run( + &Session::default(), + &Conversation::new_unvalidated(messages), + &emitter(), + ) + .await + .unwrap(); + assert!(matches!(result, OperationResult::NotApplicable)); + } + + #[tokio::test] + async fn a_zero_cap_disables_the_veto_entirely() { + // Documented behaviour on main: BUZZ_AGENT_STOP_MAX_REJECTIONS=0 turns + // `_Stop` off. An operator with a misbehaving hook depends on it. + let op = BuzzStopVetoOperation::new(agent().await, "buzz-dev-mcp".to_string(), 0); + let result = op + .run(&Session::default(), &conversation(1), &emitter()) + .await + .unwrap(); + assert!(matches!(result, OperationResult::NotApplicable)); + } + #[tokio::test] async fn ends_turn_matches_the_shape_of_a_finished_answer() { assert!(ends_turn(&[Message::assistant().with_text("done")]));