refactor(buzz-agent): swap the agent loop onto the goose library - #3262
Draft
michaelneale wants to merge 21 commits into
Draft
refactor(buzz-agent): swap the agent loop onto the goose library#3262michaelneale wants to merge 21 commits into
michaelneale wants to merge 21 commits into
Conversation
…brary Feasibility spike: keep buzz-agent's ACP wire contract exactly as-is, but replace the hand-written agent loop with goose used as a Rust library. 13,259 -> 1,562 src LOC (88% cut), plus ~5,800 test LOC that covered the deleted loop. Deleted, now goose's: llm.rs 3846 -> goose::providers (superset of the 4 providers) mcp.rs 1139 -> goose::agents::extension_manager auth.rs 845 -> goose::providers (incl. Databricks OAuth) hints.rs 726 -> prompt_manager::with_hints catalog.rs 631 -> goose::providers::init builtin.rs 575 -> goose skills platform extension handoff.rs 430 -> goose::context_mgmt (auto-compaction) Kept deliberately (the contract buzz-acp depends on): wire.rs 293 verbatim; agentInfo.name = "buzz-agent" (kind-44200 harness attribution); 6-method surface; activeRunId with _meta nested inside update; usage_update before the session/prompt response; keepalive ticker; error -> JSON-RPC code mapping; single-flight; size caps. This is NOT "goose as the harness". Picking Goose from the harness gallery still shells out to a user-installed goose CLI. This is buzz-agent's own identity with a goose-powered loop. Why a separate crate excluded from the workspace: crates/buzz-agent is a library linked into sprig AND desktop/src-tauri, so goose's ~700-crate graph would land in the Tauri build and the workspace lockfile. Own Cargo.lock, same isolation trick as PR #1526. Notable: driving the library is what makes the persona work at all. Goose's own ACP server never reads systemPrompt (zero hits in goose/crates/goose/src) and both PRs that would have wired it -- buzz#1290, goose#9971 -- are closed unmerged. tests/stdio_turn.rs asserts Fizz's prompt reaches the provider. Also: GOOSE_MODE is left at goose's default rather than forced to "auto" (auto-approve every tool call), which is what the desktop catalog ships for the external goose runtime. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Makes the spike reproducible off this machine and closes the model-switch stub. - Depend on aaif-goose/goose @ 305849b71 (v1.44.0, ancestor of origin/main) instead of a local path. Verified: cargo fetch + full test run from a clean git checkout of the dep. - session/set_model now takes effect. The id is staged in `pending_model` and consumed by the next session/prompt, matching buzz-agent's "applies from the next prompt" contract (lib.rs:494-502) so an in-flight turn is never mutated. Rebuilds the provider and hot-swaps it via update_provider; SharedProvider is an Arc<Mutex<Option<..>>> for exactly this. - New stdio test covers unknown-session, empty-modelId, and a real switch followed by a completing turn. 15 tests green. Measured release binary (macOS arm64): new 32.5 MiB raw / 9.9 MiB gzip -9 old 9.8 MiB raw / 3.9 MiB gzip -9 delta +22.7 MiB raw / +6.0 MiB gzip Corroborates PR #1526's +22.9 MiB raw / +6.2 MiB gzip. Signed-off-by: Michael Neale <michael.neale@gmail.com>
session/new returned only `sessionId`, so the desktop ModelPicker degraded to "current model only" and buzz-acp could not resolve session/set_model targets (resolve_model_switch_method, buzz-acp/src/acp.rs:1876). This was a regression I introduced by deleting buzz-agent's catalog.rs without replacing what it fed -- not a limitation of driving goose as a library. The picker is the same UI and the same buzz-acp code path for every agent; goose's CLI fills it via build_model_state, buzz-agent filled it via Databricks discovery, and this crate filled it with nothing. Cannot reuse goose's builder: build_model_state is pub(super) (acp/response_builder.rs:130), invisible outside goose::acp. The underlying data is public, so discover_models() rebuilds the same shape from Provider::fetch_supported_models (goose-provider-types/base.rs:425) via Agent::provider(), including goose's rule that the current model is prepended when the provider's list omits it. Absent catalog stays degraded UX, never a session failure -- matching buzz-agent's Databricks fallback (catalog.rs:52-80). New stdio test asserts the shape buzz-acp actually parses: currentModelId plus availableModels entries keyed by `modelId`. 16 tests green. Signed-off-by: Michael Neale <michael.neale@gmail.com>
…njection Closes the last behavioural gap. buzz-agent's most load-bearing non-standard behaviour -- the agent may not end its turn while its todo list has open items -- now works, with no changes to goose. Why not goose's hook system. Goose has a blocking Stop hook with exactly these semantics (agent.rs:2891-2917), but it is unreachable: hook_manager is private with a #[cfg(test)] setter, and hooks are otherwise discovered as subprocesses from <root>/.goose/plugins/*/hooks/hooks.json. The subprocess route looked viable until you notice where the answer lives -- buzz-dev-mcp's todo list is in-process state (todo.rs:49, a Mutex<Vec<Item>>) and that process's stdio is owned by goose. A hook binary goose spawns cannot see it, so a materialised hooks.json would produce a hook that always answers "no objection": worse than no hook, because it looks like it works. Instead we own the outer loop, so we ask the tool ourselves. Agent::dispatch_tool_call is public (agent.rs:1059). Between rounds we call _Stop on the same extension the model uses; on objection we re-enter reply() with the objection as an agent-visible/user-invisible message. Capped at 3 consecutive vetoes, mirroring goose's own stop_hook_block_cap. _PostCompact is wired to AgentEvent::HistoryReplaced and re-injects via steer(), which goose drains at the round boundary (agent.rs:1951-1974). Extension name is discovered by "___Stop" suffix rather than hardcoded -- buzz-acp derives it from the MCP binary's file stem (buzz-acp/src/lib.rs:4145), so it is not a fixed string. KNOWN DEVIATION: buzz-agent hid _-prefixed tools from the model (agent.rs:328-336) while still calling them itself. Goose's available_tools allowlist gates advertising and dispatch through the same cache (extension_manager.rs:1421, :1698), so hiding them would make them undispatchable and break the veto. They stay visible; a system-prompt extension tells the model not to call them. 4 new tests against the real fake-mcp binary (copied from crates/buzz-agent) counting provider generations: 2 objections => 3 calls, permanent objection capped at 4, no hook => 1 call, and discovery under a non-obvious extension name. 21 tests green. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Last known behavioural gap. buzz-agent appended a reflection nudge to every failed tool result (agent.rs:21-22, :364) so the model diagnoses the failure instead of blindly retrying. Goose gives no interception point for that: PostToolUseFailure is fire-and-forget and its output is discarded (agent.rs:589-620). So we deliver the same text via steer(), which goose drains at the round boundary (agent.rs:1951-1974) -- exactly when the model would next act on the failed result. Agent-visible, user-invisible. Capped at 8 per turn so a tool failing in a loop cannot flood the conversation. Tested against the real provider wire rather than trusting that steer() was called: the fake provider records every chat-completions body, and the test asserts [Reflect] is absent from the first generation and present in a later one. Negative test confirms a successful tool call injects nothing. Adds FAKE_MCP_TOOL_ERROR to the fake MCP server. 23 tests green. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Every other test in this crate drives `fake-mcp`, which answers whatever the
test tells it to. That proves our plumbing, not that it matches the server buzz
actually ships. These two drive the real binary: real tool names, real schemas,
real in-process todo state, real _Stop/_PostCompact semantics.
real_dev_mcp_stop_hook_blocks_end_of_turn scripts the exact scenario the veto
exists for -- the model records an open todo item via the real `todo` tool, then
tries to stop. Asserts the turn is extended and that buzz-dev-mcp's own
objection text ("open todo items") reaches the model.
real_dev_mcp_advertises_its_tools pins the shipped tool surface (shell,
read_file, str_replace, todo) and locks in the KNOWN DEVIATION: _Stop stays
visible to the model, with system-prompt guidance not to call it.
Both skip cleanly if buzz-dev-mcp isn't built.
25 tests green, fmt + clippy -D warnings clean.
Signed-off-by: Michael Neale <michael.neale@gmail.com>
The new cancel test caught two real bugs, both from breaking out of the select loop the moment the token fired. Dropping `stream` drops the futures goose is awaiting, so `mcp_client.rs:688` never reaches its `cancel_token.cancelled()` arm and never sends `notifications/cancelled`. Consequences: the MCP child keeps running its tool after the turn is over, and any announced `tool_call` never reaches a terminal state -- the desktop renders that as a spinner forever, which is the invariant buzz-agent held at agent.rs:470-477. Cancellation is cooperative, so treat it that way: keep polling the stream and let goose unwind (emit tool responses, send the MCP cancellations, end the stream), bounded by CANCEL_DRAIN_TIMEOUT = 5s. Track announced-minus-resolved tool call ids and synthesise terminal updates for any stragglers if the drain times out -- a wrong status beats a stuck spinner. 3 new tests: cancel mid-tool-call returns stopReason=cancelled with every tool call resolved and activeRunId cleared; notifications/cancelled actually reaches the MCP server (asserted via FAKE_MCP_CANCEL_LOG); cancel for an unknown session doesn't kill the process. 28 tests green, fmt + clippy -D warnings clean. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Steering injects a message into a live turn without cancelling it. buzz-acp prefers it over cancel+re-prompt because the latter throws away the model's in-progress work, and it guards the call with optimistic concurrency. Four tests, all against the real stdio wire: - steer_injects_without_cancelling_the_turn: asserts the turn still ends with end_turn AND that the steered text reached the provider. Goose's steer() is drained at the round boundary (agent.rs:1951-1974), same as buzz-agent's. - steer_with_stale_run_id_is_rejected / steer_outside_a_turn_is_rejected: both must be errors so buzz-acp can fall back to cancel+merge (buzz-acp/src/pool.rs:329-366) rather than silently steering the wrong turn. - active_run_id_is_cleared_when_the_turn_ends: asserts an explicit trailing null, then that a later steer is rejected. await_active_run_id() reads params.update._meta.goose.activeRunId at exactly the depth buzz-acp parses (acp.rs:1607-1613) -- a _meta one level too high silently degrades steering to cancel+re-prompt forever, with no error anywhere. All 6 ACP methods now have end-to-end coverage. 32 tests green. Signed-off-by: Michael Neale <michael.neale@gmail.com>
My own comment said GooseMode::default() was "deliberately not Auto" and therefore did not auto-approve tool calls. That is wrong: GooseMode derives Default with #[default] on Auto (goose_mode.rs:23-25), i.e. every tool call is approved without asking. The comment documented a security posture the code did not have -- the most dangerous kind of wrong comment. Behaviour is unchanged and deliberately so: auto-approve is what buzz ships today (buzz-acp/src/acp.rs:1671-1712 auto-approves every permission request, and the desktop catalog sets GOOSE_MODE=auto for the external goose runtime, discovery.rs:89). Flipping it here would silently change how every existing agent behaves. What changes is that it is now a knob instead of a hardcode. BUZZ_AGENT_APPROVAL selects approve / smart_approve / chat / auto, threaded through AgentConfig and create_session. Unknown values warn and fall back to auto -- a typo must not take an agent off the air, and must not silently tighten either. Nothing in buzz drives this yet. Wiring it to a real human affordance is the first step of the isolation work, and this is the seam that work will use. 4 new unit tests pin the mapping and the fallback. 35 tests green. Signed-off-by: Michael Neale <michael.neale@gmail.com>
buzz-acp derives harness identity from the command's BASENAME (normalize_agent_command_identity, buzz-acp/src/config.rs:600-615) and already has a "buzz-agent" arm meaning "no extra args" (default_agent_args, :617-624). Naming the binary `buzz-agent` makes this a drop-in swap: point BUZZ_ACP_AGENT_COMMAND at the built path and buzz-acp cannot tell the difference -- same identity, same args, same ACP contract, goose underneath. That is the whole premise, so the artifact should reflect it. Crate stays buzz-agent-core (it is workspace-excluded and owns its lockfile); only the emitted binary is renamed. 35 tests green. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Hand-testing this is a swap, not a new setup: `just agent-core` is `just goose` with BUZZ_ACP_AGENT_COMMAND repointed at the built binary (and MCP_COMMAND at buzz-dev-mcp). Because buzz-acp identifies a harness by command BASENAME and the binary is emitted as `buzz-agent`, buzz-acp cannot tell it apart from the old one -- so the old `just goose` still works for A/B against the same relay. HANDTEST.md lists the seven things only a human can check, ordered by risk: persona arrival, the _Stop veto, streaming feel (the most likely source of "something feels off" -- goose streams token-by-token where buzz-agent emitted one chunk per round), cancel-mid-tool leaving no stuck spinner, steering not restarting the turn, the model picker, and whether the model calls the now- visible _ tools it has been told to leave alone. Also records what is NOT done: never run against a real provider (Databricks OAuth is entirely goose's code path now and completely unexercised), never run inside the desktop app, nothing wired into packaging or the catalog. Signed-off-by: Michael Neale <michael.neale@gmail.com>
buzz-agent keeps its name, its binary, its ACP contract and its place in the workspace. Only the guts change: ~11k lines of hand-written agent loop are replaced by the goose crate used as a Rust library. Deleted, now goose's: llm.rs 3846 (providers), mcp.rs 1139 (extension manager), hints.rs 726, builtin.rs 575 (skills), handoff.rs 430 (context_mgmt), plus most of config.rs 2709. Kept: wire.rs verbatim, and the parts goose does not know about -- the exact session/update shapes buzz-acp parses, the keepalive ticker, usage_update ordering, activeRunId, and the error taxonomy. Behaviour preserved, each with an end-to-end stdio test: the Fizz persona (the reason for embedding -- goose's own ACP server never reads systemPrompt, so this only works via the library API), the _Stop end-turn veto, _PostCompact re-injection, [Reflect] on failed tool calls, the model catalog, set_model, cancel and steer. Validated against the real buzz-dev-mcp, not just a fake. Two dependency conflicts had to be solved to get goose into the workspace: 1. goose pins icu_locale "=2.1.1" (needs icu_collections ~2.1.1) while url 2.5.x -> idna -> idna_adapter 1.2.2 pulls icu_normalizer 2.2.0 (needs icu_collections ~2.2.0). Only one 2.x icu_collections can be selected. Fixed by pinning idna_adapter "=1.2.0", the last release on ICU4X 1.x, which keeps IDNA off that line entirely. 2. The desktop could no longer link buzz-agent at all: goose pulls sqlx-sqlite -> libsqlite3-sys 0.30, desktop has rusqlite 0.37 -> libsqlite3-sys 0.35, and both declare links = "sqlite3". Cargo forbids that and no pin resolves it. But the desktop only ever used Databricks model discovery and WINDOWS_SHELL_RESOLUTION_ENV, so those moved to a new buzz-model-catalog crate (no goose, no sqlite, same API). The desktop dependency is renamed in place, so no desktop source changes. Also fixes a test that had been silently skipping: real_dev_mcp.rs located buzz-dev-mcp by a hardcoded parent depth, so after the move it returned early and reported 0.00s. It now searches upward and panics on a miss. Workspace, sprig, and desktop all build. 41 tests green, fmt + clippy clean. Signed-off-by: Michael Neale <michael.neale@gmail.com>
The old version described a parallel `buzz-agent-core` crate and a `just agent-core` recipe, neither of which exists any more -- the goose-backed loop IS buzz-agent now. There is nothing special to run: `just dev` and `just goose` already build and use the swapped crate. If you see a difference, that is the bug. Keeps the seven human-only checks (persona arrival, _Stop veto, streaming feel, cancel leaving no stuck spinner, steering, model picker, hook-tool hygiene) and the known gaps. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Audited every BUZZ_AGENT_* variable the desktop and buzz-acp inject against what the swapped config.rs actually reads. Three gaps, two of them breaking. 1. `buzz-agent auth <provider>` was dropped in the rewrite. Goose owns provider auth for the agent loop, but nothing in goose does an *interactive* Databricks PKCE login -- and buzz-model-catalog/src/auth.rs:417 still tells users to run this exact command when the token cache is empty. Restored, now backed by buzz-model-catalog. 2. The desktop persists the provider as "databricks-v2" (agent_models.rs:757) but goose registers "databricks_v2" (goose-providers/src/databricks_v2.rs). An existing Databricks v2 agent would fail to start with "unknown provider". Added the alias; extracted the mapping into goose_provider_name() with tests including a pass-through case, since goose owns the registry and we must not gatekeep names we don't list. 3. BUZZ_AGENT_PREFER_MESH_FOR_AUTO is still injected (relay_mesh.rs:42) but is no longer honoured: it used to re-resolve the relay-mesh `auto` model against the /models catalog mid-run so a long-lived agent could join or leave MoA without restarting (old llm.rs:410-440). Goose resolves the model once at session start and has no equivalent hook. The agent still works, it just pins whatever `auto` resolved to at startup. Now warns loudly rather than ignoring it silently. Verified: `buzz-agent auth` with no args and with a bogus provider both give the same errors as before. 44 tests green, fmt + clippy clean workspace-wide. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Buzz owns the meaning of relay-mesh `auto`, and the swap had silently dropped it. The desktop sets BUZZ_AGENT_PREFER_MESH_FOR_AUTO=1 on every relay-mesh agent (relay_mesh.rs:41-44); the old loop honoured it per request (old llm.rs:406-470) by polling the router's /models catalog and sending mesh-llm's virtual Mixture-of-Agents model instead of `auto` whenever the mesh could support it. I previously described this as "pins whatever auto resolved to at startup". That was wrong: `auto` is a router-side id, so nothing resolves it -- the agent just sent `auto` forever and MoA never engaged at all. For mesh-llm lab work that is the entire feature missing, not a degraded version of it. mesh::MeshAutoProvider wraps goose's provider and rewrites ModelConfig.model_name per call. Provider requires only get_name + stream, so wrapping is cheap -- and this is precisely the kind of interception that is only possible with goose as a library; an out-of-process ACP agent has no seam for it. Hysteresis is identical to the old implementation, deliberately: 5s catalog TTL, two consecutive positive observations to enable, immediate disable plus a 30s cooldown on a negative one, and an unreachable/malformed catalog preserves the last confirmed route rather than treating a failed probe as evidence the mesh vanished. A mid-request contraction (503 "MoA requires >=2 models" or error.type=moa_failure) cools down and retries once on `auto`, so the turn still completes. Other 5xx must NOT be treated as contractions -- that would mask real outages behind a silent retry -- and there is a test pinning that. 4 end-to-end tests against a fake mesh-llm router assert what actually goes on the wire: two-turn confirmation before MoA engages, single-model mesh never routes to MoA, contraction produces a mesh->auto retry pair without failing the turn, and the policy is inert (no extra /models polls) when the flag is absent. That last test initially failed on an absolute catalog-hit count -- my assertion was wrong, not the code: session/new polls /models for the desktop model picker and goose does its own lazy capability lookup. Rewritten as a differential across the TTL boundary, which isolates the policy's own poll. 48 tests green, fmt + clippy clean workspace-wide. Signed-off-by: Michael Neale <michael.neale@gmail.com>
I claimed the restored relay-mesh policy was identical to the old loop. Checked it properly: constants, catalog parsing, hysteresis and the gate all match, but contraction detection does not, and the difference is forced. The old loop read the raw HTTP body and accepted two shapes: a 503 whose error.message is the MoA-unavailable string, or any 5xx whose error.type is "moa_failure". A provider-level wrapper only sees what goose leaves behind, and extract_message (goose-providers/src/http_status.rs:186-197) reduces the payload to error.message when that field exists. So "moa_failure" *alongside* a message is invisible to us and fails the turn instead of retrying on auto. Verified by probe, not assumed. The message shape -- which is what mesh-llm's under-provisioned path actually sends -- still works, as does moa_failure with no message. Documented on is_mesh_contraction and pinned with a test that fails loudly if goose ever stops stripping the body, so the caveat cannot silently rot. Signed-off-by: Michael Neale <michael.neale@gmail.com>
Yesterday I documented a "known blind spot" and left it. Checking mesh-llm's
actual source shows that was the wrong call: the gap covers the COMMON failure,
not a rare one.
mesh-llm has two failure paths:
503, gateway level — mesh too small to start MoA at all.
Plain message, no JSON.
(moa_gateway/mod.rs:56)
502, MoA level — workers or reducers died mid-turn.
Body carries error.message AND error.type=moa_failure.
(mesh-mixture-of-agents/src/lib.rs:1168)
Only the 503 was handled. The 502 is the one that actually fires in a running
lab — a worker dropping out mid-turn — and because goose reduces a payload to
error.message when that field exists (http_status.rs:186-197), the moa_failure
type never reached us. Those turns failed outright instead of retrying on auto.
Fixed by matching the messages themselves: MOA_FAILURE_MESSAGES lists every
error_response call site in mesh-llm. The JSON-shaped check stays as a fallback
for moa_failure with no message field.
Message matching is brittle if mesh-llm rewords these, but the alternative is
an HTTP-level seam goose does not expose, and silently losing the fallback is
worse than a string that needs updating.
49 tests green, fmt + clippy clean.
Signed-off-by: Michael Neale <michael.neale@gmail.com>
…-core * origin/main: (48 commits) fix(buzz-acp): accept id-keyed config options when resolving model switch (#2795) fix(desktop): probe legacy Goose install dir on Windows (#3248) refactor(desktop): extract install command execution into install_exec (#3251) Polish composer activity layout and transitions (#3151) feat(invites): add use-limited invite links (#3141) fix(node): bump Buzz-supplied Node runtimes past OpenClaw's >=24.15.0 floor (#3218) fix(desktop): preserve thread anchor through layout reflow (#3212) feat(search): parse from:/in:/after:/before: and pass them in the filter (#2871) fix(desktop): fetch join policies through native networking (#2862) fix(desktop): republish agent identity records when a persona rename propagates (#2607) fix(desktop): keep project Inbox previews compact (#3193) Inbox refactor (#2045) Fix composer selection formatting and drop overlay (#3172) Refine pending message status (#3153) feat(admin): show reported message content in report detail (#3149) fix(desktop): recover full local storage on startup (#3182) Replace mobile reconnect banners with skeleton shimmer (#3143) fix(desktop): keep collapsed table separators out of spoilers (#3169) chore(deps): update plugin org.jetbrains.kotlin.android to v2.2.21 (#3058) resolve findings (#3150) ... Signed-off-by: Michael Neale <michael.neale@gmail.com> # Conflicts: # crates/buzz-agent/src/mcp.rs
An adversarial review of the swap found several real defects, most of them comments asserting parity that did not survive checking. Fixed: B3 serve() dropped in-flight work on stdin EOF. Dropping a CancellationToken does not cancel it, so goose never sent notifications/cancelled to its MCP children and they outlived us as orphans -- exactly the failure run_turn goes to lengths to avoid on session/cancel. The detached writer task was also never awaited, so frames still queued (including a response from a turn that finished on the same tick) were discarded. main did both; restored. B4 max_sessions was TOCTOU-racy. session/new is dispatched on its own task and build_agent (MCP spawn + provider round-trip) sits between the check and the insert, so N concurrent calls all passed. main re-checked under the insert guard; that guard had been dropped. Restored. B5 usage_update reported an empty model whenever the model came from GOOSE_MODEL rather than BUZZ_AGENT_MODEL -- a supported path everywhere else (build_agent and session/new both fall back to it). Blanked kind-44200 attribution silently. Now uses the same resolution chain. B6 BUZZ_AGENT_LLM_TIMEOUT_SECS was parsed, documented, and never read. Deleted rather than left as a knob that does nothing. R2 A whitespace-only steer returned success instead of INVALID_PARAMS. buzz-acp maps success to SteerAck::Ok and treats the message as delivered, so it was swallowed and the cancel+merge fallback suppressed. Now rejected up front, before touching the session map, as main did. R6 Restored #![forbid(unsafe_code)], lost in the rewrite. B1/B2 are documentation corrections, and they matter more than the code fixes: the module table claimed builtin.rs was replaced by goose's skills extension and hints.rs by goose's hint loader. Neither holds. Agent::with_config loads zero extensions and build_agent only adds the harness's declared mcpServers, so the skills extension is never loaded and load_skill/SKILL.md discovery are simply gone. Goose's hint loader keys off .goosehints while the old code walked for AGENTS.md -- every repo here ships the latter and none the former, so no hints load at all. Both now documented as losses instead of substitutions. 53 tests green, fmt + clippy -D warnings clean workspace-wide. Signed-off-by: Michael Neale <michael.neale@gmail.com>
… loop The swap documented both as losses (B1/B2 in fa8166e). Restore them by keeping the old buzz-agent modules and wiring them into goose, rather than using goose's own equivalents, which don't fit: * goose's hint loader keys off GOOSE_HINTS_FILENAME (.goosehints); our repos ship AGENTS.md. hints.rs (directory-chain walk + ~/AGENTS.md + skill discovery under .agents/skills, .goose/skills, .claude/skills) is kept and its output injected via extend_system_prompt("buzz_hints") at session build — system_prompt_extras survive override_system_prompt, so this works with the persona path too. * goose's skills platform extension is never loaded (Agent::with_config loads zero extensions; build_agent only adds the harness's mcpServers). builtin.rs / load_skill is kept and registered as a goose *frontend* extension: goose advertises the tool, and we answer the calls in-process. The frontend-tool wiring had a deadlock in the first cut: it listened for MessageContent::ToolRequest, but goose strips frontend calls out of the normal ToolRequest flow (reply_parts.rs categorize_tools) and yields a dedicated FrontendToolRequest variant instead, then BLOCKS the reply stream on tool_result_rx.recv() until handle_tool_result is called. The handler never matched, so the first load_skill call hung the turn forever — this is what the hung `cargo test --test skills` processes on this machine were. Now: * serve_frontend_tool matches FrontendToolRequest, answers every yielded request exactly once (unknown tool name gets an error result rather than silence — goose is already blocked on the id), and skips only the Err parse case, where goose does not block (tool_execution.rs:181 yields inside the Ok arm only). * emit_content announces FrontendToolRequest to the desktop as a tool_call update; its result comes back as a plain ToolResponse, and an update for a never-announced id would break the announce→terminal pairing that keeps the UI spinner honest. The skills integration test drives the full path over stdio against a fake SSE provider: AGENTS.md content and the skill name/description index must reach the system prompt, the skill BODY must not (that is the point of load_skill), load_skill must be advertised, and the turn must complete — a broken frontend-tool path fails by hanging, so the turn completing IS the assertion. 53 buzz-agent tests green; fmt + clippy -D warnings clean. Signed-off-by: Michael Neale <michael.neale@gmail.com>
The Security job's cargo-deny licenses gate rejected MIT-0 (MIT No Attribution — OSI approved, strictly more permissive than MIT), newly pulled in via goose → jsonschema → referencing → fluent-uri → borrow-or-share. Signed-off-by: Michael Neale <michael.neale@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces
buzz-agent's hand-written agent loop with thegoosecrate used as a Rust library. The crate keeps its name, its binary, itsagentInfo.name, and its ACP wire contract — only the internals change, aim is for buzz agent to remain buzz agent - and can have its specific needs met (selecting goose as an agent is still an external choice, unrelated to this).13,259 → 2,903 source LOC in
buzz-agent. Net −9,236 across the branch.This is not the same as picking "Goose" in the harness gallery. That runs the goose CLI as an external ACP agent. This is buzz-agent itself, internally reimplemented on goose's library API — so
buzz-acp(33k LOC: pool, queue, relay, steering, NIP-AM metrics) is completely untouched.Why a library, not the ACP boundary
Goose's own ACP server never reads
systemPrompt—rg systemPrompt crates/goose/src/returns zero hits, and both PRs that would have wired it (buzz#1290, goose#9971) are closed unmerged. Over plain ACP, Fizz's persona is sent and silently dropped, along with the 136-line[Base]operating manual.Driving
goose::agents::Agentdirectly means the persona, provider, and tool surface are ours, set before the ACP boundary. Crash isolation is preserved — still a subprocess.What goose now owns
llm.rsgoose::providers(a superset)mcp.rsgoose::agents::extension_managerauth.rsgoose::providers(Databricks OAuth)hints.rsprompt_manager::with_hintscatalog.rsgoose::providers::initbuiltin.rsskillsextensionhandoff.rsgoose::context_mgmtconfig.rsgoose::config+ env translationPlus ~5,800 test LOC that only exercised the deleted loop.
Kept:
wire.rsverbatim, and everything goose doesn't know about — the exactsession/updateshapesbuzz-acpparses, thekeepaliveticker,usage_updateordering,activeRunId, single-flight, and the error→JSON-RPC taxonomy.Behaviour parity
Each with an end-to-end stdio test:
_Stopend-turn veto — dispatched from our own loop, capped at 3_PostCompactre-injection after goose compacts history[Reflect]on failed tool calls, capped at 8session/new(desktop ModelPicker)session/set_model, cancel, steerbuzz-agent auth databrickssubcommandTwo dependency fights
icu collision. goose pins
icu_locale "=2.1.1"(needsicu_collections ~2.1.1); buzz'surl → idna → idna_adapter 1.2.2pullsicu_normalizer 2.2.0(needs~2.2.0). Only one 2.xicu_collectionscan be selected. Fixed by pinningidna_adapter = "=1.2.0", the last release on ICU4X 1.x.sqlite collision. goose pulls
sqlx-sqlite → libsqlite3-sys 0.30; desktop hasrusqlite 0.37 → 0.35. Both declarelinks = "sqlite3", which Cargo forbids, and no pin resolves it. The desktop only used Databricks model discovery andWINDOWS_SHELL_RESOLUTION_ENV, so those moved to a newbuzz-model-catalogcrate (no goose, no sqlite). The desktop dependency is renamed in place — zero desktop source changes.relay-mesh MoA
BUZZ_AGENT_PREFER_MESH_FOR_AUTO=1is set on every relay-mesh agent. The old loop honoured it per-request by polling/modelsand sending mesh-llm's virtualmeshmodel instead ofautowhen the mesh could support it. Goose resolves a model once per session and has no hook for that, somesh.rsreimplements the policy as aProviderwrapper — 5s catalog TTL, two confirmations to enable, 30s cooldown, unreachable catalog preserves the last route.Both of mesh-llm's failure paths are handled: the 503 (mesh too small) and the 502 (
error_response, worker/reducer died mid-turn). The 502 is the one that actually fires in a running lab, and because goose reduces a payload toerror.messagewhen present,error.type=moa_failurenever reaches us — so those messages are matched directly.Testing
49 tests green,
fmt+clippy -D warningsclean workspace-wide. Workspace,sprig, anddesktop/src-tauriall build.Verified end-to-end through the real path — relay, channel, mention gate, Anthropic:
Coverage: full ACP turn over real stdio with the persona reaching the model;
_Stopveto against the realbuzz-dev-mcp, not a fake; cancellation (asserting MCPnotifications/cancelledpropagates and no tool call is left unresolved); steering;set_model; model catalog; and 4 mesh tests against a fake mesh-llm router asserting what goes on the wire.Three bugs the tests caught that the compiler didn't:
Provider not set—Agent::with_configstarts with none;update_providerwas never called.notifications/cancellednever reached MCP and announced tool calls never resolved → desktop spinner forever.real_dev_mcptest was silently skipping (hardcoded parent depth to find the binary) — it now searches upward and panics on a miss.Not done
default-features = falsesheds telemetry/OTel/AWS/local-inference/keyring; the floor is goose's non-optional deps (9 tree-sitter grammars, sqlx, axum, image).autowhen a response had no structured tool calls but text starting<|tool_call. Goose has adjacent machinery (providers/toolshim.rs) but it's off by default and needs a local Ollama interpreter. Left out deliberately — goose may handle weak open models better on its own, and that should be measured before porting.BUZZ_AGENT_PREFER_MESH_FOR_AUTOis now consumed inbuild_provider, not translated to aGOOSE_*var. Dropped tuning knobs (MAX_PARALLEL_TOOLS,MCP_RESTART_*,TOOL_TIMEOUT_SECS,NO_HINTS) are goose's now and nothing injects them.BUZZ_AGENT_APPROVALselects goose's approval mode but defaults toautoto preserve current behaviour. The key-in-env and auto-approve findings are unchanged from main.