mirror of
https://github.com/block/buzz.git
synced 2026-10-29 13:31:44 +00:00
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 <michael.neale@gmail.com> Co-authored-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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::<u32>("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");
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -753,6 +753,7 @@ async fn session_prompt(app: &Arc<App>, 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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<OperationResult> {
|
||||
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<Agent>, 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")]));
|
||||
|
||||
Reference in New Issue
Block a user