refactor(buzz-agent): swap the agent loop onto the goose library

[michaelneale]

What changed

Previously, Buzz built the agent stack itself:

loop {
    let response = buzz_llm.complete(&history, &tools).await?;
    history.push(response);

    if response.tool_calls.is_empty() {
        return stop_or_continue_via_buzz_hooks();
    }

    history.push(buzz_mcp.execute(response.tool_calls).await);
}

Buzz owned provider adapters, MCP, messages, tool execution, compaction, and loop policy.

Now, Buzz owns only orchestration and Buzz-specific policy, composing Goose GDK primitives:

loop {
    apply(gdk_operations.step(&conversation).await?);

    let response = gdk_provider.stream(&conversation, &gdk_tools).await?;
    conversation.push(response);

    if let Some(calls) = response.tool_calls() {
        conversation.push(gdk_agent.dispatch(calls).await);
    } else if no_operation_objects_to_stopping() {
        return EndTurn;
    }
}

Buzz-specific policy becomes small GDK operations:

operations = [
    MaxRounds,
    StopVeto,
    ReplyGuard,
    Steer,
    CompactThenRestoreTodos,
];

Compaction is mostly delegation:

let compacted = gdk::compact_messages(&conversation).await?;
replace_conversation(compacted);
restore_buzz_todos();

In one sentence: previously Buzz built an agent stack; now it composes GDK primitives and keeps the small amount of behavior that makes an agent a good Buzz participant.

This is intended to preserve Buzz agent behavior while removing duplicated infrastructure and making future improvements easier.

Presentation

BUZZ_GDK_BRIEF.pdf

[michaelneale]

🤖 Review note from MicBlock's AI agent (Galadriel) — re-review at head 2ea414f0b.

Verdict: the substantive regressions from the first review are fixed (diff-confirmed): compaction now uses each response's own occupancy total; usage/pricing emission is routed back through wire::usage_update_payload with separate cache read/write accounting, sticky-unknown totals, and origin-allowlisted pricingIdentity; cancel mid-inference returns cancelled and discards the partial; reply guard rejects hallucinated tool names via list_tools(); MAX_TOOL_CALLS_PER_TURN=64 restored; provider errors classified from typed ProviderError; Databricks 401 forced refresh wired via refresh_hook.

Before this leaves draft:

1. Desktop lockfile downgrades — desktop/src-tauri/Cargo.lock still carries anyhow 1.0.103 and nostr 0.44.6 vs main's 1.0.104 / 0.44.7. Nothing in this PR requires them; restore with cargo update -p anyhow -p nostr against the desktop manifest. (Root-lockfile icu 1.5.x moves are justified — forced by goose's idna_adapter = 1.2.0 pin, verified with cargo tree -i.)
2. Multi-round compaction test — the fix is correct by inspection, but the only compaction test exercises a single effect; the original bug was invisible to exactly that shape. Add a two-round test asserting occupancy doesn't accumulate.
3. CI — tooltip-semantics.spec.ts fails identically on current main (pre-existing); thread-reply-anchor-roleplay failed only here — rerun to confirm flake.

Smaller residuals to record as decisions (not merge-gates): steer queue not cleared on cancel/error return paths (stale steer can replay into the next turn); desktop env-strip ordering vs baked build env / default_env; mid-turn usage_update emission now end-of-turn only.

Full review discussion in the #feature-buzz-with-gdk channel.

[micspiral]

Test-parity review: three specific gaps in loop bounding

Read at origin/main e8172b5b vs this branch 0a4c78e1e. Everything below is a git grep result on those two trees, not an inference.

Framing first, because the headline numbers are misleading: 612 tests → 198 looks alarming but most of it is correct. llm.rs (186 tests) tested a request transport that no longer exists; ~38 of config.rs's 100 were anthropic_thinking_config_* per-model permutations for a request buzz no longer builds; the 13 handoff tests covered a summariser goose now owns. permission.rs (21) and wire.rs (18) carried over intact, and the Databricks OAuth tests moved to buzz-model-catalog (50). Those are not gaps and I am not asking for them back.

Three things I do think are gaps, all in loop bounding — the part that decides when a turn stops.

1. BUZZ_AGENT_MAX_ROUNDS unset changed meaning, and the README still documents the old one

• main: max_rounds: parse_env("BUZZ_AGENT_MAX_ROUNDS", 0), and agent.rs:362 guards if self.cfg.max_rounds > 0 && round >= … — unset means unbounded.
• branch: config.rs:223 env_parse::<u32>(…).filter(|n| *n > 0) → None, and loop_drive.rs:172 ctx.max_rounds.unwrap_or(DEFAULT_MAX_ROUNDS) where DEFAULT_MAX_ROUNDS = 1000 (loop_drive.rs:88) — unset means 1000.

The change is deliberate and the reasoning in the doc comment is good ("owning the loop means owning the bound"). Two problems:

• README.md:334 still says | Loop rounds | 0 (unlimited) | BUZZ_AGENT_MAX_ROUNDS |, and README.md:155 still says 0 = unlimited. Both now describe main's behaviour.
• DEFAULT_MAX_ROUNDS has two references in the whole branch — its definition and its one use. No test pins it. Main had finite_round_cap_still_binds_without_a_context_overflow and max_tokens_recovery_respects_finite_round_cap.

To close: fix the two README rows to say 1000, and add one test asserting an unset max_rounds stops at 1000 with StopReason::MaxTurnRequests. ops.rs's at_budget_ends_the_turn_with_max_turn_requests is the pattern; it just needs the default-path case.

2. The documented ordering between the tool-call cap and the reply guard is untested

Correcting my own earlier count: reply-guard coverage is 10 → 5, not 10 → 1. ops.rs has the_reply_guard_reminds_when_nothing_was_published, a_publish_attempt_disarms_the_guard, an_unrelated_tool_call_does_not_disarm_the_guard, a_hallucinated_shell_does_not_disarm_the_guard, the_guard_stops_after_its_budget. That is the core of the guard and it is well covered.

Four of main's cases have no counterpart:

main test	branch
reply_guard_ignores_calls_lost_to_the_turn_cap	none
reply_guard_bounded_by_stop_rejection_budget	none
reply_guard_off_when_stop_budget_is_zero	none
reply_guard_combines_with_stop_hook_objection	none

The first is the one worth adding, because README.md:217-219 states the property as a guarantee: "Detection is checked after the per-turn tool-call cap (MAX_TOOL_CALLS_PER_TURN) is applied: a publish-shaped call that was discarded never ran." loop_drive.rs:408-415 truncates, BuzzReplyGuardOperation inspects the conversation afterwards — so the behaviour looks right, but nothing tests it, and it is now an emergent property of two separate components rather than one function's control flow. The other three are guard × stop-hook budget interactions; both mechanisms exist on this branch and both have tests, but nothing tests them together.

To close: one test with >64 publish-shaped calls in a round asserting the guard still nags, plus one guard-under-exhausted-stop-budget test.

3. Six documented caps have no implementation on this branch

README.md:325-332 lists these. Grepping the entire branch for each name returns exactly one hit — the README row itself:

cap	main implementation	branch
MAX_MCP_SERVERS (16)	mcp.rs:26,207-209 rejects	README only
MAX_TOOLS_PER_SESSION (128)	mcp.rs:22,262-264 rejects	README only
MAX_DESCRIPTION_BYTES (1 KiB)	mcp.rs:23,286 clamps	README only
MAX_SCHEMA_BYTES (4 KiB)	mcp.rs:24,880-884 replaces with {}	README only
MAX_TOOL_RESULT_BYTES (8 MiB)	config.rs:384, agent.rs:891	README only
MAX_LLM_RESPONSE_BYTES (16 MiB)	llm.rs:23,1966	README only

MAX_LLM_RESPONSE_BYTES is fine to drop — that is transport, goose's now. The other five are MCP-facing limits, and MCP servers are the untrusted input here: MAX_SCHEMA_BYTES and MAX_DESCRIPTION_BYTES existed so one server's oversized tool definitions could not crowd out the prompt.

MAX_TOOL_RESULT_BYTES is the one I would check first. BUZZ_AGENT_MAX_TOOL_RESULT_TEXT_BYTES is projected onto goose's GOOSE_MAX_TOOL_RESPONSE_SIZE (config.rs:347-348), so tool result text is bounded — but main's 8 MiB was the total including images, and I found nothing on this branch enforcing a total. Mitigating: lib.rs:549 declares promptCapabilities.image: false, so images cannot arrive via the prompt; the unbounded path would be an MCP tool returning image content.

To close: for each of the five, either cite the goose mechanism that enforces it (and change the README row to name that mechanism) or delete the row. A documented limit with no enforcement is worse than no limit, because a reviewer reads the table and concludes it is handled.

Also worth listing explicitly in the PR body

permission_boundary.rs inverts a main assertion: test_load_skill_emits_no_permission_request (main, asserts zero requests) → test_load_skill_is_gated_like_any_other_tool (branch, asserts one). The doc comment argues it well — exempting load_skill would need a name-based allowlist and tool names are attacker-controlled — and I agree with the call. But it is a user-visible behaviour change and the PR body does not mention it. Any test whose assertion flipped should be listed there, because both suites are green while asserting opposite things, so review cannot see it from CI.

One process note

scripts/run-tests.sh:121-122 runs cargo test -p buzz-agent --lib — unit tests only, on both main and this branch. The 10 integration files in tests/ (stdio_turn.rs, provider_faults.rs, steer.rs, cancel.rs, stop_hook.rs, skills.rs, real_dev_mcp.rs, permission_boundary.rs, memory.rs, reflect.rs) are the best evidence in this PR — they drive the real binary over stdio against a live socket provider — and CI is not running them. Green CI is not evidence they pass. Adding them to that script would make this PR's strongest evidence load-bearing.

---

I have not run either suite; disk and toolchain constraints on this machine meant I read both trees rather than executing them. Every claim above is a grep on e8172b5b / 0a4c78e1e with the file and line cited, so each is cheap to falsify — please do.