Files
buzz/crates/buzz-agent/tests/stop_hook.rs
Michael Neale a41b78f03e Merge origin/main into micn/buzz-agent-goose-core
Adopts main's tool-call permission gate (#5712) onto the goose loop. The
conflicts were semantic, not textual: #5712 landed +2494/-299 inside
`crates/buzz-agent`, the crate this branch rewrote, so five of main's files
(`llm.rs`, `mcp.rs`, `fake_llm.rs`, `regressions.rs`, and main's `agent.rs`
loop) no longer exist here and their share of the feature had to be
re-expressed against goose.

Taken from main unchanged: `permission.rs` (the broker), `wire.rs`'s
`session/request_permission` shapes, `Inbound::Response`, `send_checked`, and
`write_frames`.

Re-expressed:

* The gate moved from main's `RunCtx::execute_parallel` to `tools::run_one`,
  which is where this branch dispatches model-issued calls. It runs after the
  `tool_call` announcement and before `Agent::dispatch_tool_call`, so a denied
  call still reaches a terminal wire state.
* The broker takes goose's `CancellationToken` instead of a
  `watch::Receiver<bool>`, and an `AskSubject` instead of buzz's deleted
  `ToolCall` type.
* Argument-shape validation is dropped: it guarded main's `mcp.rs`, and goose's
  `CallToolRequestParams::arguments` is already `Option<Map>`, so a non-object
  cannot be represented.
* `max_pending_permissions` / `permission_timeout` follow this crate's
  `env_parse`-with-default style rather than main's validated `parse_env`.

Two behaviour notes worth review:

* `load_skill` is now **gated**, where main exempted it as an in-process
  built-in. Under goose it is a real tool on the skills platform extension,
  dispatched through the same path as any MCP tool. Exempting it would need an
  allowlist keyed on tool *name* — which an untrusted MCP server controls, and
  could register its own `load_skill` to inherit. Test renamed accordingly.
* Connection teardown already cancels every session (`serve`'s shutdown), so
  main's `cancel_all_sessions` plus writer-death select arm had no separate work
  to do here; the broker's wire-closed path covers the rest.

Test harness changes:

* `tests/common/mod.rs`'s fake LLM now answers SSE. goose's openai-compatible
  provider requests `stream: true` and parses `chat.completion.chunk`; against
  main's plain `chat.completion` body every turn ended `end_turn` with no tool
  call and no ask, which reads as a broken gate rather than a broken fixture.
  Its `/models` lookup is also answered without popping the canned queue.
* `tests/approve/mod.rs` auto-approves for the nine suites whose subject is not
  the boundary, selecting by option `kind` rather than a hardcoded `optionId`.
* `cancel.rs` now answers the ask and waits for `FAKE_MCP_CALL_RECEIVED` before
  cancelling. Without that the cancel resolved the *permission wait*, the tool
  was never dispatched, and `cancel_propagates_notifications_cancelled_to_mcp`
  failed — a test named "cancel mid tool call" was exercising "cancel before
  tool call".

Verified at this tree: `buzz-agent` full suite 111 unit + 36 integration
(including all 9 boundary tests) pass, `clippy --all-targets -D warnings`
clean, `cargo fmt --check` clean, `cargo build --workspace` clean, and both
`Cargo.lock` files resolve `--locked` (8 goose deps in root, 0 in desktop).

Co-authored-by: Michael Neale <michael.neale@gmail.com>
Signed-off-by: Michael Neale <michael.neale@gmail.com>
2026-08-26 13:39:15 +10:00

349 lines
12 KiB
Rust

//! Proves the `_Stop` veto: the turn must not end while the hook objects.
//!
//! This is buzz-agent's most load-bearing non-standard behaviour — it is why
//! the agent finishes its todo list instead of stopping halfway. Goose has the
//! same mechanism internally, but its `hook_manager` is private and its hooks
//! are subprocesses that cannot see buzz-dev-mcp's in-process todo state, so
//! we drive it from our own loop (see `src/hooks.rs`).
//!
//! Setup: a fake MCP server exposing `_Stop`, plus a fake provider that counts
//! how many times it is asked to generate. One user prompt with a hook that
//! objects twice must produce three provider calls, not one.
use std::io::{BufRead, BufReader, Write};
use std::net::TcpListener;
use std::process::{Child, Command, Stdio};
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;
use std::time::Duration;
use serde_json::{json, Value};
mod approve;
/// Fake OpenAI-compatible provider that always answers "done" and counts calls.
fn spawn_counting_provider() -> (String, Arc<AtomicUsize>) {
let listener = TcpListener::bind("127.0.0.1:0").expect("bind");
let addr = listener.local_addr().expect("addr");
let calls = Arc::new(AtomicUsize::new(0));
let counter = calls.clone();
std::thread::spawn(move || {
for stream in listener.incoming() {
let Ok(mut stream) = stream else { continue };
let mut reader = BufReader::new(stream.try_clone().expect("clone"));
let mut request_line = String::new();
if reader.read_line(&mut request_line).is_err() {
continue;
}
let mut content_length = 0usize;
loop {
let mut line = String::new();
if reader.read_line(&mut line).is_err() {
break;
}
let t = line.trim_end();
if t.is_empty() {
break;
}
if let Some(v) = t
.strip_prefix("content-length: ")
.or_else(|| t.strip_prefix("Content-Length: "))
{
content_length = v.parse().unwrap_or(0);
}
}
let mut body = vec![0u8; content_length];
use std::io::Read;
let _ = reader.read_exact(&mut body);
if request_line.contains("/models") {
let payload = json!({ "object": "list", "data": [] }).to_string();
let _ = stream.write_all(
format!(
"HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}",
payload.len(), payload
).as_bytes(),
);
let _ = stream.flush();
continue;
}
counter.fetch_add(1, Ordering::SeqCst);
let chunk = |delta: Value, finish: Value, usage: Value| {
let mut c = json!({
"id": "c", "object": "chat.completion.chunk", "created": 1,
"model": "fake-model",
"choices": [{ "index": 0, "delta": delta, "finish_reason": finish }],
});
if !usage.is_null() {
c["usage"] = usage;
}
format!("data: {c}\n\n")
};
let sse = format!(
"{}{}data: [DONE]\n\n",
chunk(
json!({ "role": "assistant", "content": "done" }),
Value::Null,
Value::Null
),
chunk(
json!({}),
json!("stop"),
json!({ "prompt_tokens": 5, "completion_tokens": 2, "total_tokens": 7 })
),
);
let _ = stream.write_all(
format!(
"HTTP/1.1 200 OK\r\ncontent-type: text/event-stream\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}",
sse.len(), sse
).as_bytes(),
);
let _ = stream.flush();
}
});
(format!("http://{addr}"), calls)
}
struct Harness {
child: Child,
stdin: std::process::ChildStdin,
stdout: BufReader<std::process::ChildStdout>,
next_id: i64,
}
impl Harness {
fn start(base_url: &str, home: &std::path::Path) -> Self {
let mut child = Command::new(env!("CARGO_BIN_EXE_buzz-agent"))
.env("BUZZ_AGENT_PROVIDER", "openai-compat")
.env("BUZZ_AGENT_MODEL", "fake-model")
.env("OPENAI_COMPAT_API_KEY", "k")
.env("OPENAI_COMPAT_BASE_URL", base_url)
.env("HOME", home)
.env("XDG_CONFIG_HOME", home.join("config"))
.env("XDG_DATA_HOME", home.join("data"))
.env("GOOSE_DISABLE_KEYRING", "1")
.env("RUST_LOG", "warn")
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::inherit())
.spawn()
.expect("spawn");
let stdin = child.stdin.take().expect("stdin");
let stdout = BufReader::new(child.stdout.take().expect("stdout"));
Self {
child,
stdin,
stdout,
next_id: 0,
}
}
fn request(&mut self, method: &str, params: Value) -> i64 {
self.next_id += 1;
let id = self.next_id;
writeln!(
self.stdin,
"{}",
json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params })
)
.expect("write");
self.stdin.flush().expect("flush");
id
}
fn await_response(&mut self, id: i64) -> Value {
loop {
let mut line = String::new();
let n = self.stdout.read_line(&mut line).expect("read");
assert_ne!(n, 0, "agent closed stdout awaiting id={id}");
let Ok(msg) = serde_json::from_str::<Value>(&line) else {
continue;
};
// Answer the authorization gate so the tool under test actually
// runs. This suite's subject is not the permission boundary (see
// `permission_boundary.rs`), so approval is automatic.
if approve::is_permission_request(&msg) {
let response = approve::approve(&msg);
writeln!(self.stdin, "{response}").expect("write approval");
self.stdin.flush().expect("flush approval");
continue;
}
if msg.get("id").and_then(Value::as_i64) == Some(id) {
return msg;
}
}
}
}
impl Drop for Harness {
fn drop(&mut self) {
let _ = self.child.kill();
let _ = self.child.wait();
}
}
/// Declare the fake MCP server, with `_Stop` objecting `objections` times.
fn mcp_servers(objections: usize) -> Value {
json!([{
"name": "devtools",
"command": env!("CARGO_BIN_EXE_fake-mcp"),
"args": [],
"env": [
{ "name": "FAKE_MCP_STOP_HOOK", "value": "1" },
{ "name": "FAKE_MCP_STOP_TEXT", "value": "1 item still open" },
{ "name": "FAKE_MCP_STOP_COUNT", "value": objections.to_string() },
],
}])
}
fn new_session(h: &mut Harness, cwd: &std::path::Path, objections: usize) -> String {
let id = h.request("initialize", json!({ "protocolVersion": 2 }));
let _ = h.await_response(id);
let id = h.request(
"session/new",
json!({
"cwd": cwd.to_str().unwrap(),
"mcpServers": mcp_servers(objections),
}),
);
let resp = h.await_response(id);
resp["result"]["sessionId"]
.as_str()
.unwrap_or_else(|| panic!("session/new failed: {resp}"))
.to_string()
}
#[test]
fn stop_hook_veto_extends_the_turn() {
let (base_url, calls) = spawn_counting_provider();
let home = tempfile::tempdir().expect("home");
let cwd = tempfile::tempdir().expect("cwd");
let mut h = Harness::start(&base_url, home.path());
// Object twice, then allow.
let session_id = new_session(&mut h, cwd.path(), 2);
let id = h.request(
"session/prompt",
json!({
"sessionId": session_id,
"prompt": [{ "type": "text", "text": "do the work" }],
}),
);
let resp = h.await_response(id);
assert_eq!(
resp["result"]["stopReason"], "end_turn",
"turn should still end cleanly once the hook stops objecting: {resp}"
);
// The model says "done" immediately, so without the veto this is 1.
// Two objections must force two more generations.
let n = calls.load(Ordering::SeqCst);
assert_eq!(
n, 3,
"expected 3 provider calls (initial + 2 vetoes), got {n} — the _Stop veto did not fire"
);
}
#[test]
fn stop_hook_veto_is_capped() {
let (base_url, calls) = spawn_counting_provider();
let home = tempfile::tempdir().expect("home");
let cwd = tempfile::tempdir().expect("cwd");
let mut h = Harness::start(&base_url, home.path());
// Always objects. Must not trap the turn forever.
let session_id = new_session(&mut h, cwd.path(), usize::MAX);
let id = h.request(
"session/prompt",
json!({
"sessionId": session_id,
"prompt": [{ "type": "text", "text": "do the work" }],
}),
);
let resp = h.await_response(id);
assert_eq!(
resp["result"]["stopReason"], "end_turn",
"a permanently objecting hook must not hang the turn: {resp}"
);
// MAX_STOP_BLOCKS = 3, so: initial + 3 vetoes = 4, then give up.
let n = calls.load(Ordering::SeqCst);
assert_eq!(
n, 4,
"expected the veto to stop at MAX_STOP_BLOCKS (1 + 3 = 4 calls), got {n}"
);
}
#[test]
fn no_stop_hook_means_no_extra_rounds() {
let (base_url, calls) = spawn_counting_provider();
let home = tempfile::tempdir().expect("home");
let cwd = tempfile::tempdir().expect("cwd");
let mut h = Harness::start(&base_url, home.path());
let id = h.request("initialize", json!({ "protocolVersion": 2 }));
let _ = h.await_response(id);
// No MCP servers at all: nothing to veto with.
let id = h.request(
"session/new",
json!({ "cwd": cwd.path().to_str().unwrap(), "mcpServers": [] }),
);
let resp = h.await_response(id);
let session_id = resp["result"]["sessionId"].as_str().unwrap().to_string();
let id = h.request(
"session/prompt",
json!({
"sessionId": session_id,
"prompt": [{ "type": "text", "text": "hi" }],
}),
);
let resp = h.await_response(id);
assert_eq!(resp["result"]["stopReason"], "end_turn");
let n = calls.load(Ordering::SeqCst);
assert_eq!(n, 1, "no hook should mean exactly one generation, got {n}");
}
#[test]
fn stop_hook_tools_are_discovered_by_suffix() {
// The extension name comes from buzz-acp's MCP binary file stem
// (`buzz-acp/src/lib.rs:4145`), so discovery must not hardcode a name.
// Here the server is called "devtools", not "developer" or "buzz-dev-mcp".
let (base_url, calls) = spawn_counting_provider();
let home = tempfile::tempdir().expect("home");
let cwd = tempfile::tempdir().expect("cwd");
let mut h = Harness::start(&base_url, home.path());
let session_id = new_session(&mut h, cwd.path(), 1);
let id = h.request(
"session/prompt",
json!({
"sessionId": session_id,
"prompt": [{ "type": "text", "text": "go" }],
}),
);
let resp = h.await_response(id);
assert_eq!(resp["result"]["stopReason"], "end_turn");
let n = calls.load(Ordering::SeqCst);
assert_eq!(
n, 2,
"one objection under a non-obvious extension name should yield 2 calls, got {n}"
);
let _ = Duration::from_secs(0);
}