jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-11-13 12:51:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(buzz-agent): dispatch load_skill as a platform extension, not a frontend tool

Browse Source 
Adopts the shape Maple uses for its in-process tools (MapleDeveloperClient,
SkillsClient): an `McpClientTrait` impl registered via
`extension_manager.add_client` with an `ExtensionConfig::Platform`, rather than
`ExtensionConfig::Frontend`.

Goose advertises frontend tools but refuses to dispatch them. It yields a
`FrontendToolRequest` and blocks the reply stream until the embedder calls
`handle_tool_result` -- strictly sequential, no timeout, and a single result
channel with no request-id correlation, so one missed or duplicated result
wedges the session for good and the cancel token will not free it. That is a
failure mode with no upside here. A platform client goes through goose's
ordinary tool path and gets concurrency, per-request timeouts and
`notifications/cancelled` for free.

Net effect is less code: `serve_frontend_tool` is gone, and with it the
`skills` parameter threaded through run_turn -> drive_stream -> handle_event
and the `Session.skills` field. The BuiltinClient owns the skill list.

Also fixes a real mismatch the swap exposed. Goose namespaces platform tools as
`{extension}__{tool}` (`extension_manager.rs:1415`), so the model sees
`buzz__load_skill`, but the skills section of the system prompt told it to call
`load_skill`. The prompt now derives the name from the same constants the
registration uses, so the two cannot drift.

89 tests green, fmt + clippy -D warnings clean.

Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
Michael Neale
2026-07-31 16:20:23 +10:00
parent 66d1bffc85
commit 3a7ffc07af
5 changed files with 161 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
crates/buzz-agent/src/agent.rs
View File

@@ -20,9 +20,9 @@
//! * **`builtin.rs` / `load_skill`.** `Agent::with_config` loads zero
//! extensions and `build_agent` only adds the `mcpServers` the harness
//! declares, so goose's `skills` platform extension is never loaded. Instead
//! `build_agent` registers `load_skill` as a goose *frontend* extension and
//! [`serve_frontend_tool`] answers the calls in-process (goose yields a
//! `FrontendToolRequest` and blocks the stream until we respond).
//! `build_agent` registers `load_skill` as a goose *platform* extension
//! backed by an `McpClientTrait` (see [`crate::builtin_client`]), so goose
//! dispatches it on its ordinary tool path.
//! * **`AGENTS.md` hints / skill index.** Goose's loader keys off
//! `GOOSE_HINTS_FILENAME` (`.goosehints`); the old code walked the directory
//! chain for `AGENTS.md` plus `~/AGENTS.md`. Every repo here ships the
@@ -140,7 +140,6 @@ pub async fn run_turn(
wire_tx: &WireSender,
cancel: CancellationToken,
hook_extension: Option<&str>,
skills: &[crate::hints::SkillEntry],
) -> (Result<StopReason, AgentError>, TurnTokens) {
let mut tokens = TurnTokens::default();
let mut next_message = Message::user().with_text(prompt_to_text(&prompt));
@@ -161,7 +160,6 @@ pub async fn run_turn(
&cancel,
&mut tokens,
hook_extension,
skills,
)
.await
{
@@ -226,7 +224,6 @@ async fn drive_stream(
cancel: &CancellationToken,
tokens: &mut TurnTokens,
hook_extension: Option<&str>,
skills: &[crate::hints::SkillEntry],
) -> Result<StopReason, AgentError> {
let session_config = SessionConfig {
id: session_id.to_string(),
@@ -308,7 +305,6 @@ async fn drive_stream(
&mut compacted,
&mut reflections,
&mut open_tool_calls,
skills,
)
.await
{
@@ -397,7 +393,6 @@ async fn handle_event(
compacted: &mut bool,
reflections: &mut usize,
open_tool_calls: &mut Vec<String>,
skills: &[crate::hints::SkillEntry],
) -> Option<StopReason> {
match event {
AgentEvent::Message(msg) => {
@@ -408,17 +403,6 @@ async fn handle_event(
open_tool_calls.push(req.id.clone());
}
// Frontend tool calls (`load_skill`) never arrive as
// `ToolRequest`: goose strips them from the normal flow
// (`reply_parts.rs` `categorize_tools`) and yields this
// dedicated variant instead, then blocks the reply stream on
// `tool_result_rx` until we answer. Answer it or the turn
// hangs forever.
if let MessageContent::FrontendToolRequest(req) = content {
open_tool_calls.push(req.id.clone());
serve_frontend_tool(req, agent, skills).await;
}
if let MessageContent::ToolResponse(resp) = content {
open_tool_calls.retain(|id| id != &resp.id);
@@ -585,68 +569,6 @@ async fn emit_content(content: &MessageContent, session_id: &str, wire_tx: &Wire
}
}
/// Serve a goose *frontend* tool call.
///
/// Goose never dispatches frontend tools itself. `handle_frontend_tool_request`
/// (`goose/src/agents/tool_execution.rs:175`) yields the
/// `FrontendToolRequest` message we are handling right now and then **blocks
/// the reply stream** on `tool_result_rx.recv()` until the embedder calls
/// `agent.handle_tool_result(id, ..)`. That has two consequences:
///
/// * every `FrontendToolRequest` event MUST be answered — returning without
/// sending a result hangs the turn forever, so even an unrecognised tool
/// name gets an error result rather than silence;
/// * exactly one answer per event — a stray extra send would be picked up by
/// the recv of the *next* frontend call and mis-pair results.
async fn serve_frontend_tool(
req: &goose_provider_types::conversation::message::FrontendToolRequest,
agent: &Arc<Agent>,
skills: &[crate::hints::SkillEntry],
) {
let result = match &req.tool_call {
Ok(call) if call.name == crate::builtin::LOAD_SKILL_TOOL => {
let args = call
.arguments
.clone()
.map(serde_json::Value::Object)
.unwrap_or(serde_json::Value::Null);
crate::builtin::call_load_skill(&args, skills).await
}
// Only `load_skill` is registered, so this arm is unreachable today —
// but goose is already blocked waiting for this id, so answer anyway.
Ok(call) => crate::builtin::error_result(&format!(
"unknown frontend tool {:?}; only {} is served in-process",
call.name,
crate::builtin::LOAD_SKILL_TOOL
)),
// An Err here means goose did NOT block on the result channel — its
// yield only happens inside the `Ok` branch (`tool_execution.rs:181`).
// Sending would leave a stray result in the queue that mis-pairs the
// NEXT frontend call (the recv trusts the id coming off the channel).
Err(e) => {
tracing::warn!("unparseable frontend tool call {}: {e}", req.id);
return;
}
};
let content: Vec<rmcp::model::Content> = result
.content
.iter()
.map(|c| rmcp::model::Content::text(c.as_text_lossy()))
.collect();
agent
.handle_tool_result(
req.id.clone(),
Ok(if result.is_error {
rmcp::model::CallToolResult::error(content)
} else {
rmcp::model::CallToolResult::success(content)
}),
)
.await;
}
#[cfg(test)]
mod tests {
use super::*;

119
crates/buzz-agent/src/builtin_client.rs Normal file
View File

@@ -0,0 +1,119 @@
//! Buzz's in-process tools, exposed to Goose as a platform extension.
//!
//! Goose has two ways to take a tool the embedder implements in Rust:
//!
//! * [`ExtensionConfig::Frontend`] — Goose *advertises* the tool but refuses to
//! run it. It returns "Frontend tool execution required" and blocks until the
//! embedder calls `handle_tool_result`. Dispatch is strictly sequential, has
//! no timeout, and the result channel carries no request id, so a single
//! missed result wedges the agent for the rest of the session and the cancel
//! token will not free it.
//! * [`ExtensionConfig::Platform`] + `extension_manager.add_client` with an
//! [`McpClientTrait`] — Goose dispatches it through the ordinary tool path,
//! so concurrency, per-request timeouts and `notifications/cancelled` all
//! work, and there is nothing for us to correlate.
//!
//! This uses the second. It is the shape Maple settled on for its own
//! in-process developer and skills tools.
//!
//! The only tool here is `load_skill`. The system prompt carries the skill
//! *index* (names and descriptions, from [`crate::hints`]); bodies are pulled
//! on demand so a large skill library costs nothing per turn.
use std::sync::Arc;
use async_trait::async_trait;
use goose::agents::mcp_client::McpClientTrait;
use goose::agents::{ExtensionConfig, ToolCallContext};
use rmcp::model::{CallToolResult, Content, InitializeResult, JsonObject, ListToolsResult, Tool};
use rmcp::service::ServiceError as Error;
use tokio_util::sync::CancellationToken;
use crate::hints::SkillEntry;
/// Extension key and display name. Goose prefixes tool names with the
/// extension name (`{extension}__{tool}`) unless the extension is registered
/// as unprefixed, so this shows up in the model's tool list.
pub const BUILTIN_EXTENSION_NAME: &str = "buzz";
/// Goose-side declaration for the extension backed by [`BuiltinClient`].
pub fn builtin_extension_config() -> ExtensionConfig {
ExtensionConfig::Platform {
name: BUILTIN_EXTENSION_NAME.to_string(),
description: "Buzz built-in tools".to_string(),
display_name: Some("Buzz".to_string()),
bundled: Some(true),
available_tools: vec![crate::builtin::LOAD_SKILL_TOOL.to_string()],
}
}
/// In-process MCP client serving [`crate::builtin`]'s tools.
pub struct BuiltinClient {
skills: Vec<SkillEntry>,
}
impl BuiltinClient {
pub fn new(skills: Vec<SkillEntry>) -> Self {
Self { skills }
}
fn load_skill_tool() -> Tool {
let def = crate::builtin::load_skill_def();
let schema = match def.input_schema {
serde_json::Value::Object(map) => Arc::new(map),
_ => Arc::new(serde_json::Map::new()),
};
Tool::new(def.name, def.description, schema)
}
}
#[async_trait]
impl McpClientTrait for BuiltinClient {
async fn list_tools(
&self,
_session_id: &str,
_next_cursor: Option<String>,
_cancel_token: CancellationToken,
) -> Result<ListToolsResult, Error> {
Ok(ListToolsResult {
tools: vec![Self::load_skill_tool()],
next_cursor: None,
meta: None,
})
}
async fn call_tool(
&self,
_ctx: &ToolCallContext,
name: &str,
arguments: Option<JsonObject>,
_cancel_token: CancellationToken,
) -> Result<CallToolResult, Error> {
if name != crate::builtin::LOAD_SKILL_TOOL {
return Ok(CallToolResult::error(vec![Content::text(format!(
"unknown tool: {name}"
))]));
}
let args = arguments
.map(serde_json::Value::Object)
.unwrap_or(serde_json::Value::Null);
let result = crate::builtin::call_load_skill(&args, &self.skills).await;
let content: Vec<Content> = result
.content
.iter()
.map(|c| Content::text(c.as_text_lossy()))
.collect();
Ok(if result.is_error {
CallToolResult::error(content)
} else {
CallToolResult::success(content)
})
}
fn get_info(&self) -> Option<&InitializeResult> {
None
}
}

12
crates/buzz-agent/src/hints.rs
View File

@@ -241,9 +241,15 @@ fn build_hints_section_impl(cwd: &Path, home: Option<&Path>) -> (String, Vec<Ski
for skill in &skills {
out.push_str(&format!("- {}: {}\n", skill.name, skill.description));
}
out.push_str(
"\nUse the `load_skill` tool to read the full content of a skill before using it.\n",
);
// Goose namespaces platform-extension tools as `{extension}__{tool}`
// (`extension_manager.rs:1415`), so name the tool the way the model
// actually sees it -- otherwise the prompt advertises a tool that does
// not exist in its tool list.
out.push_str(&format!(
"\nUse the `{}__{}` tool to read the full content of a skill before using it.\n",
crate::builtin_client::BUILTIN_EXTENSION_NAME,
crate::builtin::LOAD_SKILL_TOOL,
));
}
(out, skills)

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

@@ -22,6 +22,7 @@
pub mod agent;
pub mod builtin;
pub mod builtin_client;
pub mod config;
pub mod hints;
pub mod hooks;
@@ -71,8 +72,6 @@ struct Session {
/// the override takes effect "from the next prompt" and never mutates a
/// turn already in flight (`buzz-agent/src/lib.rs:494-502`).
pending_model: Option<String>,
/// Skills discovered at session start; the bodies `load_skill` serves.
skills: Vec<crate::hints::SkillEntry>,
/// Name of the MCP extension carrying `_Stop`/`_PostCompact`, if any.
/// See [`crate::hooks`] for why we dispatch these ourselves.
hook_extension: Option<String>,
@@ -97,15 +96,7 @@ async fn build_agent(
cwd: &str,
system_prompt: Option<&str>,
mcp_servers: &[McpServerStdio],
) -> Result<
(
Arc<Agent>,
String,
Option<String>,
Vec<crate::hints::SkillEntry>,
),
AgentError,
> {
) -> Result<(Arc<Agent>, String, Option<String>), AgentError> {
let session_manager = Arc::new(SessionManager::instance());
let permission_manager = PermissionManager::instance();
@@ -201,32 +192,24 @@ async fn build_agent(
.await;
}
// `load_skill` runs in-process. The prompt carries only skill names and
// descriptions; the body is pulled on demand, which is the whole point of
// the index above. Registered as a goose *frontend* tool: goose advertises
// it, then hands the call back to us rather than dispatching it itself
// (`agent.rs:1167`).
// `load_skill` runs in-process, registered the way Maple does it: a
// platform extension backed by an `McpClientTrait`, NOT
// `ExtensionConfig::Frontend`. Goose advertises frontend tools but refuses
// to dispatch them -- it blocks on an uncorrelated result channel until the
// embedder calls `handle_tool_result`, with no timeout and no cancellation,
// so one missed result wedges the session. A platform client goes through
// goose's ordinary tool path instead. See `crate::builtin_client`.
if !skills.is_empty() {
let def = crate::builtin::load_skill_def();
let tool = rmcp::model::Tool::new(
def.name.clone(),
def.description.clone(),
match def.input_schema {
serde_json::Value::Object(map) => std::sync::Arc::new(map),
_ => std::sync::Arc::new(serde_json::Map::new()),
},
);
let ext = ExtensionConfig::Frontend {
name: "buzz_builtin".to_string(),
description: String::new(),
tools: vec![tool],
instructions: None,
bundled: None,
available_tools: Vec::new(),
};
if let Err(e) = agent.add_extension(ext, &session.id).await {
tracing::warn!("load_skill unavailable: {e}");
}
agent
.extension_manager
.add_client(
crate::builtin_client::BUILTIN_EXTENSION_NAME.to_string(),
crate::builtin_client::builtin_extension_config(),
std::sync::Arc::new(crate::builtin_client::BuiltinClient::new(skills)),
None,
None,
)
.await;
}
let agent = Arc::new(agent);
@@ -259,7 +242,7 @@ async fn build_agent(
tracing::info!(extension = %ext, "lifecycle hooks available");
}
Ok((agent, session.id, hook_extension, skills))
Ok((agent, session.id, hook_extension))
}
/// Keep the model's hands off the lifecycle hooks.
@@ -472,7 +455,7 @@ async fn session_new(app: &Arc<App>, id: Value, params: Value, wire_tx: &WireSen
.await;
}
let (agent, goose_session_id, hook_extension, skills) =
let (agent, goose_session_id, hook_extension) =
match build_agent(&app.cfg, &p.cwd, p.system_prompt.as_deref(), &p.mcp_servers).await {
Ok(v) => v,
Err(e) => {
@@ -514,7 +497,6 @@ async fn session_new(app: &Arc<App>, id: Value, params: Value, wire_tx: &WireSen
model_override: None,
pending_model: None,
hook_extension,
skills,
accumulated_input_tokens: 0,
accumulated_output_tokens: 0,
},
@@ -609,7 +591,7 @@ async fn session_prompt(app: &Arc<App>, id: Value, params: Value, wire_tx: &Wire
let cancel = CancellationToken::new();
// Single-flight per session, and capture the agent handle.
let (agent, goose_session_id, pending_model, hook_extension, skills) = {
let (agent, goose_session_id, pending_model, hook_extension) = {
let mut sessions = app.sessions.lock().await;
let Some(s) = sessions.get_mut(&p.session_id) else {
return wire::send(
@@ -639,7 +621,6 @@ async fn session_prompt(app: &Arc<App>, id: Value, params: Value, wire_tx: &Wire
s.goose_session_id.clone(),
s.pending_model.take(),
s.hook_extension.clone(),
s.skills.clone(),
)
};
@@ -678,7 +659,6 @@ async fn session_prompt(app: &Arc<App>, id: Value, params: Value, wire_tx: &Wire
wire_tx,
cancel,
hook_extension.as_deref(),
&skills,
)
.await;

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

@@ -1,12 +1,10 @@
//! Proves `load_skill` and `AGENTS.md` hints survive the goose swap.
//!
//! Both were briefly lost in the port. `load_skill` is registered as a goose
//! *frontend* tool, which means goose advertises it to the model but refuses to
//! dispatch it — it returns "Frontend tool execution required" and waits for
//! the embedder to call `handle_tool_result`
//! (`goose/src/agents/agent.rs:1167`). If that wiring is wrong the turn does
//! not fail, it **hangs forever**, so the timeout in `await_response` is the
//! real assertion here.
//! *platform* extension backed by an `McpClientTrait` (the shape Maple uses),
//! so goose dispatches it on its ordinary tool path. Goose namespaces such
//! tools as `{extension}__{tool}`, which the system prompt has to match — a
//! prompt naming a tool that is not in the tool list is a silent failure.
use std::io::{BufRead, BufReader, Write};
use std::net::TcpListener;
@@ -96,13 +94,13 @@ fn spawn_provider() -> (String, mpsc::Receiver<(String, Vec<String>)>) {
call_count += 1;
let sse = if call_count == 1 {
// Ask for the skill. If our frontend-tool plumbing is broken,
// Ask for the skill by its namespaced name.
// goose never resolves this and the turn hangs.
let c = json!({
"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":"buzz__load_skill","arguments":"{\"name\":\"widget-maker\"}"}
}]},"finish_reason":null}]
});
let d = json!({
@@ -233,7 +231,7 @@ fn agents_md_and_skill_index_reach_the_model_and_load_skill_resolves() {
.unwrap_or_else(|| panic!("session/new failed: {r}"))
.to_string();
// The turn only returns if load_skill was served. A broken frontend-tool
// The turn only returns if load_skill was dispatched. A broken tool
// path hangs here rather than erroring.
let r = h.call(
"session/prompt",
@@ -266,7 +264,7 @@ fn agents_md_and_skill_index_reach_the_model_and_load_skill_resolves() {
"skill BODY was inlined — the point of load_skill is that it is not:\n{first}"
);
assert!(
tool_lists[0].iter().any(|t| t == "load_skill"),
tool_lists[0].iter().any(|t| t == "buzz__load_skill"),
"load_skill not advertised: {:?}",
tool_lists[0]
);