From a6b46f6a5857ae3b7380590bd174918f97df8de4 Mon Sep 17 00:00:00 2001 From: Ken Lathrop <103387858+ArgussLabs@users.noreply.github.com> Date: Sat, 25 Jul 2026 15:12:27 -0700 Subject: [PATCH] feat(acp): allow multiple MCP servers per agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--mcp-command` / `BUZZ_ACP_MCP_COMMAND` accepted a single binary, so an agent got exactly one MCP server. That is an either/or in practice: a Buzz agent acts in Buzz by shelling out to the `buzz` CLI, and that shell tool comes from buzz-dev-mcp — so pointing the slot at any domain-specific MCP server leaves the agent able to answer but unable to post. Observed while running an external MCP server behind buzz-acp: the agent was offered 61 tools (60 from the domain server plus load_skill) and zero shell or buzz tools. It could query and had no way to say anything. Makes the flag repeatable and the env var comma-separated, matching the `agent_args` field directly above it. Defaults are unchanged: no value still means no MCP servers, and a single value behaves exactly as before. - Args.mcp_command: String -> Args.mcp_commands: Vec, value_delimiter=',' keeping the `--mcp-command` flag name, so existing invocations and env values keep working. - Empty entries are filtered, since BUZZ_ACP_MCP_COMMAND="" yields one empty entry through the delimiter. - build_mcp_servers maps over the list, injecting BUZZ_RELAY_URL / BUZZ_PRIVATE_KEY / BUZZ_AUTH_TAG into each server as before. - Server names are made unique. Two distinct binaries can share a file stem (/a/bin/mcp and /b/bin/mcp); a duplicate name would let one shadow the other in the agent's tool namespace, so subsequent collisions get a -2, -3 suffix. Tests: the four existing build_mcp_servers tests are updated to the Vec form, plus four new ones covering two servers reaching the agent, env injection per server, and file-stem collisions. Signed-off-by: Ken Lathrop <103387858+ArgussLabs@users.noreply.github.com> --- .env.example | 7 +- crates/buzz-acp/src/config.rs | 27 ++++-- crates/buzz-acp/src/lib.rs | 168 ++++++++++++++++++++++++---------- 3 files changed, 147 insertions(+), 55 deletions(-) diff --git a/.env.example b/.env.example index db5a7ea25c..63028da9d4 100644 --- a/.env.example +++ b/.env.example @@ -137,8 +137,11 @@ RUST_LOG=buzz_relay=debug,buzz_db=debug,buzz_auth=debug,buzz_pubsub=debug,tower_ # Goose default: "acp". Codex/Claude default: "" (empty). # BUZZ_ACP_AGENT_ARGS=acp -# Binary for an optional MCP server sidecar (e.g. buzz-dev-mcp for buzz-agent). -# BUZZ_ACP_MCP_COMMAND= +# MCP server binaries to expose to the agent (e.g. buzz-dev-mcp for buzz-agent). +# Comma-separated for more than one; --mcp-command may also be repeated. +# Each server is spawned with BUZZ_RELAY_URL / BUZZ_PRIVATE_KEY / BUZZ_AUTH_TAG +# injected. Servers are named after each binary's file stem. +# BUZZ_ACP_MCP_COMMAND=buzz-dev-mcp,/opt/bin/my-domain-mcp # Number of parallel agent subprocesses (1–32). # BUZZ_ACP_AGENTS=1 diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index a38d6faa14..e34cfecae4 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -258,8 +258,14 @@ pub struct CliArgs { )] pub agent_args: Vec, - #[arg(long, env = "BUZZ_ACP_MCP_COMMAND", default_value = "")] - pub mcp_command: String, + /// MCP server binaries to expose to the agent. Repeat the flag, or pass a + /// comma-separated list via `BUZZ_ACP_MCP_COMMAND`. Empty entries are ignored. + #[arg( + long = "mcp-command", + env = "BUZZ_ACP_MCP_COMMAND", + value_delimiter = ',' + )] + pub mcp_commands: Vec, /// Idle timeout: max seconds of silence before killing a turn. /// Resets on any agent stdout activity. @@ -488,7 +494,9 @@ pub struct Config { pub relay_url: String, pub agent_command: String, pub agent_args: Vec, - pub mcp_command: String, + /// MCP server binaries exposed to the agent, in order. Empty after filtering + /// means the agent gets no MCP servers. + pub mcp_commands: Vec, pub idle_timeout_secs: u64, pub max_turn_duration_secs: u64, pub agents: u32, @@ -963,7 +971,14 @@ impl Config { relay_url: args.relay_url, agent_command, agent_args, - mcp_command: args.mcp_command, + // Drop empties: an unset-but-present env var (BUZZ_ACP_MCP_COMMAND="") + // yields one empty entry through the comma delimiter. + mcp_commands: args + .mcp_commands + .into_iter() + .map(|c| c.trim().to_string()) + .filter(|c| !c.is_empty()) + .collect(), idle_timeout_secs, max_turn_duration_secs, agents: args.agents, @@ -1029,7 +1044,7 @@ impl Config { self.keys.public_key().to_hex(), self.agent_command, self.agent_args.join(" "), - self.mcp_command, + self.mcp_commands.join(","), self.idle_timeout_secs, self.max_turn_duration_secs, self.agents, @@ -1337,7 +1352,7 @@ mod tests { relay_url: "ws://localhost:3000".into(), agent_command: "goose".into(), agent_args: vec!["acp".into()], - mcp_command: "".into(), + mcp_commands: Vec::new(), idle_timeout_secs: DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 0230ea0875..9e61d09446 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -4139,48 +4139,70 @@ async fn run_models(args: ModelsArgs) -> Result<()> { } fn build_mcp_servers(config: &Config) -> Vec { - if config.mcp_command.is_empty() { + if config.mcp_commands.is_empty() { return vec![]; } - vec![McpServer { - name: std::path::Path::new(&config.mcp_command) - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("mcp") - .to_string(), - command: config.mcp_command.clone(), - args: vec![], - env: { - let mut env = vec![ - EnvVar { - name: "BUZZ_RELAY_URL".into(), - value: config.relay_url.clone(), - }, - EnvVar { - name: "BUZZ_PRIVATE_KEY".into(), - // bech32 encoding of a valid secret key is infallible. - // Panic here is correct: injecting a bogus secret would cause - // delayed, hard-to-diagnose agent failures downstream. - value: config - .keys - .secret_key() - .to_bech32() - .expect("secret key bech32 encoding should never fail"), - }, - ]; - // Forward BUZZ_AUTH_TAG (NIP-OA owner attestation credential) - // so the MCP server can attach it to every signed event. - if let Ok(auth_tag) = std::env::var("BUZZ_AUTH_TAG") { - if !auth_tag.is_empty() { - env.push(EnvVar { - name: "BUZZ_AUTH_TAG".into(), - value: auth_tag, - }); - } + + // Every server gets the same injected env: the relay URL, this agent's secret, + // and the NIP-OA credential if one is set. + let base_env = || { + let mut env = vec![ + EnvVar { + name: "BUZZ_RELAY_URL".into(), + value: config.relay_url.clone(), + }, + EnvVar { + name: "BUZZ_PRIVATE_KEY".into(), + // bech32 encoding of a valid secret key is infallible. + // Panic here is correct: injecting a bogus secret would cause + // delayed, hard-to-diagnose agent failures downstream. + value: config + .keys + .secret_key() + .to_bech32() + .expect("secret key bech32 encoding should never fail"), + }, + ]; + // Forward BUZZ_AUTH_TAG (NIP-OA owner attestation credential) + // so the MCP server can attach it to every signed event. + if let Ok(auth_tag) = std::env::var("BUZZ_AUTH_TAG") { + if !auth_tag.is_empty() { + env.push(EnvVar { + name: "BUZZ_AUTH_TAG".into(), + value: auth_tag, + }); } - env - }, - }] + } + env + }; + + let mut used: HashSet = HashSet::new(); + config + .mcp_commands + .iter() + .map(|command| { + let stem = std::path::Path::new(command) + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("mcp") + .to_string(); + // Server names must be unique — two binaries can share a file stem + // (/a/bin/mcp and /b/bin/mcp), and a duplicate name would make one + // shadow the other in the agent's tool namespace. + let mut name = stem.clone(); + let mut n = 2; + while !used.insert(name.clone()) { + name = format!("{stem}-{n}"); + n += 1; + } + McpServer { + name, + command: command.clone(), + args: vec![], + env: base_env(), + } + }) + .collect() } #[cfg(test)] @@ -4949,7 +4971,7 @@ mod build_mcp_servers_tests { relay_url: "ws://localhost:3000".into(), agent_command: "goose".into(), agent_args: vec!["acp".into()], - mcp_command: "test-mcp-server".into(), + mcp_commands: vec!["test-mcp-server".into()], idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, @@ -5039,29 +5061,81 @@ mod build_mcp_servers_tests { #[test] fn empty_mcp_command_returns_no_servers() { let mut config = test_config(); - config.mcp_command = "".into(); + config.mcp_commands = Vec::new(); let servers = build_mcp_servers(&config); assert!( servers.is_empty(), - "empty mcp_command should produce no MCP servers" + "no mcp commands should produce no MCP servers" ); } #[test] fn absolute_path_mcp_command_uses_file_stem_as_name() { let mut config = test_config(); - config.mcp_command = "/opt/bin/my-mcp-server".into(); + config.mcp_commands = vec!["/opt/bin/my-mcp-server".into()]; let servers = build_mcp_servers(&config); assert_eq!(servers.len(), 1); assert_eq!(servers[0].name, "my-mcp-server"); } + #[test] + fn multiple_mcp_commands_all_reach_the_agent() { + // The reason this flag is repeatable: an agent needs buzz-dev-mcp to act in + // Buzz (it shells out to the `buzz` CLI) *and* whatever domain server the + // operator adds. One slot forced an either/or. + let mut config = test_config(); + config.mcp_commands = vec!["/opt/bin/buzz-dev-mcp".into(), "/opt/bin/domain-mcp".into()]; + let servers = build_mcp_servers(&config); + assert_eq!(servers.len(), 2); + assert_eq!(servers[0].name, "buzz-dev-mcp"); + assert_eq!(servers[1].name, "domain-mcp"); + assert_eq!(servers[0].command, "/opt/bin/buzz-dev-mcp"); + assert_eq!(servers[1].command, "/opt/bin/domain-mcp"); + } + + #[test] + fn every_mcp_server_gets_the_injected_env() { + let mut config = test_config(); + config.mcp_commands = vec!["/a/first-mcp".into(), "/b/second-mcp".into()]; + let servers = build_mcp_servers(&config); + assert_eq!(servers.len(), 2); + for server in &servers { + for key in ["BUZZ_RELAY_URL", "BUZZ_PRIVATE_KEY"] { + assert!( + server.env.iter().any(|e| e.name == key), + "{} should be injected into {}", + key, + server.name + ); + } + } + } + + #[test] + fn colliding_file_stems_get_unique_names() { + // Two distinct binaries can share a file stem. Duplicate server names would + // make one shadow the other in the agent's tool namespace. + let mut config = test_config(); + config.mcp_commands = vec![ + "/a/bin/mcp".into(), + "/b/bin/mcp".into(), + "/c/bin/mcp".into(), + ]; + let servers = build_mcp_servers(&config); + assert_eq!(servers.len(), 3); + assert_eq!(servers[0].name, "mcp"); + assert_eq!(servers[1].name, "mcp-2"); + assert_eq!(servers[2].name, "mcp-3"); + let unique: HashSet<&str> = servers.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(unique.len(), 3, "server names must be unique"); + } + #[test] fn mcp_command_with_no_stem_falls_back_to_mcp() { // Path::new("").file_stem() returns None — exercises the unwrap_or("mcp") path. let mut config = test_config(); - config.mcp_command = "".into(); - // Empty command returns no servers; test the stem logic directly. + config.mcp_commands = Vec::new(); + // No commands returns no servers; test the stem logic directly. assert_eq!( std::path::Path::new("") .file_stem() @@ -5071,7 +5145,7 @@ mod build_mcp_servers_tests { ); // Confirm a non-empty command with no stem (e.g. just a dot) also falls back. - config.mcp_command = ".".into(); + config.mcp_commands = vec![".".into()]; let servers = build_mcp_servers(&config); assert_eq!(servers.len(), 1); assert_eq!( @@ -5115,7 +5189,7 @@ mod error_outcome_emission_tests { // feed emission under test. agent_command: "true".into(), agent_args: vec![], - mcp_command: "test-mcp-server".into(), + mcp_commands: vec!["test-mcp-server".into()], idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, agents: 1,