jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-29 21:41:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(buzz-agent): keep goose's bundled skills out of Buzz agents

Browse Source 
goose's skills platform factory calls plain `SkillsClient::new`
(`platform_extensions/mod.rs:219-221`), which leaves goose's two compiled-in
skills switched on: `goose-doc-guide` and `web-search`
(`goose/src/skills/builtins/`). Neither is a Buzz skill. `web-search` tells the
model to shell out to `uvx ddgs` / Tavily / SearXNG — a capability Buzz does not
provide and never advertised on main, where `buzz-agent` scanned four
filesystem directories and had no builtins at all (`hints.rs:8`, `:204-217`).
So the goose swap widened every Buzz agent's advertised surface as a side
effect, which is not what "same agent, goose underneath" means.

`with_builtin_skills(false)` is only reachable off the constructor, so buzz
builds the client itself and registers it with `ExtensionManager::add_client`
instead of by name. The prompt index is filtered to match: a skill listed in
the index but absent from the client is a dead `load_skill` reference.

The `add_client` route keeps the bare tool name. `is_unprefixed_extension`
(`extension_manager.rs:392-400`) keys off the `ExtensionConfig`, not the
registration route, and this passes the same
`ExtensionConfig::Platform { name: "skills" }` the factory does, so the table's
`unprefixed_tools: true` still applies. The context is given the session
explicitly because `SkillsClient::new` reads `session.working_dir` for
discovery and falls back to the process cwd without it.

Measured against the real `~/.buzz` nest with the built binary and a local
fake SSE provider (no model call): index 13 skills -> 11, tool list still
`["load_skill"]`, `load_skill("buzz-cli")` still returns its body,
`load_skill("web-search")` now returns "not found". System prompt 12,632 ->
12,079 bytes.

`tests/skills.rs` gains two assertions it was missing. The existing test only
checked `stopReason == end_turn` and that a second round happened — a
`load_skill` answering "Skill not found." passes both, so the suite could not
distinguish a working skills path from a broken one. It now asserts the skill
body reached the model as a tool result. The new test pins the builtins from
both sides (absent from the index AND unresolvable through the tool) and was
verified to fail on the unpatched `lib.rs`.

buzz-agent: 111 unit + 36 integration green, `clippy --all-targets -D warnings`
clean, `fmt --check` clean.

Co-authored-by: Michael Neale <michael.neale@gmail.com>
Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
Michael Neale
2026-08-26 17:12:53 +10:00
parent a41b78f03e
commit 0a4c78e1ef
2 changed files with 153 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

73
crates/buzz-agent/src/lib.rs
View File

@@ -189,7 +189,7 @@ async fn build_agent(
)
.await
.map_err(|e| AgentError::Llm(format!("session create: {e}")))?;
let session_id = session.id;
let session_id = session.id.clone();
// Provider/model names come from the `GOOSE_*` variables `Config::from_env`
// projected out of the `BUZZ_AGENT_*` ones; goose's registry owns base-url
@@ -275,11 +275,32 @@ async fn build_agent(
// global ones, plugin-installed skills and its own builtins, and supports
// `args` templating we never had.
//
// Registered by *name* rather than by handing over a client: `skills` is
// in goose's `PLATFORM_EXTENSIONS` table, so `add_extension` runs its
// factory. It is declared `unprefixed_tools: true` there, which is why the
// tool the model sees is plain `load_skill` and not `skills__load_skill`.
let skills = goose::skills::discover_skills(Some(std::path::Path::new(cwd)));
// Registered by handing over a *client* rather than by name, and that is
// the whole point: goose's platform factory
// (`platform_extensions/mod.rs:219-221`) calls plain `SkillsClient::new`,
// which leaves goose's own bundled skills switched on. Those are
// `goose-doc-guide` and `web-search` (`skills/builtins/`) — neither is a
// Buzz skill, and `web-search` advertises a capability Buzz does not
// provide, so every Buzz agent would offer the model a tool-shaped promise
// to shell out to `uvx ddgs`. `with_builtin_skills(false)` is only
// reachable off the constructor, so buzz builds the client itself and
// registers it with `add_client`.
//
// What that route costs: `add_client` does not consult
// `PLATFORM_EXTENSIONS`, so nothing here inherits the table's
// `unprefixed_tools: true`. It does not have to — `is_unprefixed_extension`
// (`extension_manager.rs:392-400`) keys off the *config*, and this passes
// the same `ExtensionConfig::Platform { name: "skills" }` the factory route
// does, so the lookup still hits the table entry and the model still sees a
// bare `load_skill`. Verified by driving both routes: same tool name, same
// filesystem skills, builtins gone.
//
// The prompt index is filtered to match. A skill listed in the index but
// absent from the client is a dead `load_skill` reference.
let skills: Vec<_> = goose::skills::discover_skills(Some(std::path::Path::new(cwd)))
.into_iter()
.filter(|s| s.source_type != goose::custom_requests::SourceType::BuiltinSkill)
.collect();
if !skills.is_empty() {
let ext = ExtensionConfig::Platform {
name: goose::skills::EXTENSION_NAME.to_string(),
@@ -288,15 +309,37 @@ async fn build_agent(
bundled: Some(true),
available_tools: Vec::new(),
};
if let Err(e) = agent.add_extension(ext, &session_id).await {
// Unlike an MCP server, a missing skills extension is not fatal:
// the agent still has every other tool. Losing it silently would
// be worse than losing it loudly, hence the warning.
tracing::warn!(error = %e, "skills extension unavailable");
} else {
prompt
.add_extra("skills", crate::skills::skill_index(&skills))
.await;
// The factory receives a context carrying the session, because
// `SkillsClient::new` reads `session.working_dir` for discovery and
// falls back to the *process* cwd without it — which for a
// desktop-spawned agent is not the nest.
let mut ctx = agent.extension_manager.get_context().clone();
ctx.extension_manager = Some(Arc::downgrade(&agent.extension_manager));
ctx.session = Some(Arc::new(session.clone()));
match goose::skills::SkillsClient::new(ctx) {
Ok(client) => {
let client = client.with_builtin_skills(false);
let info = goose::agents::mcp_client::McpClientTrait::get_info(&client).cloned();
agent
.extension_manager
.add_client(
goose::skills::EXTENSION_NAME.to_string(),
ext,
Arc::new(client),
info,
None,
)
.await;
prompt
.add_extra("skills", crate::skills::skill_index(&skills))
.await;
}
Err(e) => {
// Unlike an MCP server, a missing skills extension is not
// fatal: the agent still has every other tool. Losing it
// silently would be worse than losing it loudly.
tracing::warn!(error = %e, "skills extension unavailable");
}
}
}

99
crates/buzz-agent/tests/skills.rs
View File

@@ -19,7 +19,17 @@ mod approve;
/// Fake OpenAI provider that asks for `load_skill` on the first turn, then
/// answers normally. Sends every observed `system` prompt and tool list back.
fn spawn_provider() -> (String, mpsc::Receiver<(String, Vec<String>)>) {
type Observed = (String, Vec<String>, String);
/// `(system prompt, tool names, whole request body)` for each provider call.
fn spawn_provider() -> (String, mpsc::Receiver<Observed>) {
spawn_provider_requesting("widget-maker")
}
/// As [`spawn_provider`], but the first turn asks for `skill_name`. Used to
/// drive `load_skill` at a name the client is expected *not* to know.
fn spawn_provider_requesting(skill_name: &str) -> (String, mpsc::Receiver<Observed>) {
let skill_name = skill_name.to_string();
let listener = TcpListener::bind("127.0.0.1:0").expect("bind");
let addr = listener.local_addr().expect("addr");
let (tx, rx) = mpsc::channel();
@@ -92,7 +102,7 @@ fn spawn_provider() -> (String, mpsc::Receiver<(String, Vec<String>)>) {
.collect()
})
.unwrap_or_default();
let _ = tx.send((system, tools));
let _ = tx.send((system, tools, req.to_string()));
}
call_count += 1;
@@ -103,7 +113,7 @@ fn spawn_provider() -> (String, mpsc::Receiver<(String, Vec<String>)>) {
"id":"c","object":"chat.completion.chunk","created":1,"model":"fake-model",
"choices":[{"index":0,"delta":{"role":"assistant","tool_calls":[{
"index":0,"id":"call_1","type":"function",
"function":{"name":"load_skill","arguments":"{\"name\":\"widget-maker\"}"}
"function":{"name":"load_skill","arguments": format!("{{\"name\":\"{skill_name}\"}}")}
}]},"finish_reason":null}]
});
let d = json!({
@@ -257,9 +267,11 @@ fn agents_md_and_skill_index_reach_the_model_and_load_skill_resolves() {
let mut systems = Vec::new();
let mut tool_lists = Vec::new();
while let Ok((sys, tools)) = seen.recv_timeout(Duration::from_millis(500)) {
let mut bodies = Vec::new();
while let Ok((sys, tools, body)) = seen.recv_timeout(Duration::from_millis(500)) {
systems.push(sys);
tool_lists.push(tools);
bodies.push(body);
}
assert!(!systems.is_empty(), "provider was never called");
@@ -285,4 +297,83 @@ fn agents_md_and_skill_index_reach_the_model_and_load_skill_resolves() {
systems.len() >= 2,
"expected a second round after the tool result"
);
// The turn completing is not evidence the skill loaded. A `load_skill`
// answering "Skill not found." also reaches `end_turn` with a second
// round, so `stopReason` alone passes on a broken skills path. Assert on
// the tool result the model was actually handed.
let second = &bodies[1];
assert!(
second.contains("Step 2: cut the flange."),
"skill body never reached the model as a tool result:\n{second}"
);
assert!(
!second.contains("not found"),
"load_skill failed to resolve the skill:\n{second}"
);
}
/// goose's `SkillsClient` ships two skills compiled into the crate —
/// `goose-doc-guide` and `web-search` (`goose/src/skills/builtins/`). Neither
/// is a Buzz skill, and `web-search` tells the model to shell out to `uvx
/// ddgs`, a capability Buzz does not provide. buzz-agent on `main` had no such
/// thing, so registering the client with builtins on would have widened every
/// Buzz agent's advertised surface as a side effect of the goose swap.
///
/// This pins both halves: absent from the prompt index, and unresolvable
/// through the tool. Index-only would pass while the tool still served them.
#[test]
fn goose_builtin_skills_are_not_offered_to_buzz_agents() {
let (base_url, seen) = spawn_provider_requesting("web-search");
let home = tempfile::tempdir().expect("home");
let ws = workspace();
let mut h = Harness::start(&base_url, home.path());
h.call("initialize", json!({"protocolVersion": 2}));
let r = h.call(
"session/new",
json!({"cwd": ws.path().to_str().unwrap(), "mcpServers": []}),
);
let sid = r["result"]["sessionId"]
.as_str()
.unwrap_or_else(|| panic!("session/new failed: {r}"))
.to_string();
let r = h.call(
"session/prompt",
json!({"sessionId": sid, "prompt": [{"type":"text","text":"search the web"}]}),
);
assert_eq!(r["result"]["stopReason"], "end_turn", "turn stalled: {r}");
let mut systems = Vec::new();
let mut bodies = Vec::new();
while let Ok((sys, _tools, body)) = seen.recv_timeout(Duration::from_millis(500)) {
systems.push(sys);
bodies.push(body);
}
assert!(!systems.is_empty(), "provider was never called");
let first = &systems[0];
for builtin in ["web-search", "goose-doc-guide"] {
assert!(
!first.contains(builtin),
"goose builtin {builtin:?} is advertised in the system prompt:\n{first}"
);
}
// The filesystem skill still has to be there — an empty index would pass
// the assertions above for the wrong reason.
assert!(
first.contains("widget-maker"),
"filesystem skills were lost along with the builtins:\n{first}"
);
assert!(
bodies.len() >= 2,
"expected a second round carrying the tool result"
);
let second = &bodies[1];
assert!(
second.contains("not found"),
"load_skill served a goose builtin:\n{second}"
);
}