diff --git a/Cargo.lock b/Cargo.lock index 96e1bf1ed..67e956711 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -751,6 +751,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.11.0", + "shlex", "thiserror 2.0.18", "tokio", "tokio-tungstenite 0.29.0", diff --git a/crates/buzz-acp/Cargo.toml b/crates/buzz-acp/Cargo.toml index d3afb5062..d26b39bf1 100644 --- a/crates/buzz-acp/Cargo.toml +++ b/crates/buzz-acp/Cargo.toml @@ -71,6 +71,9 @@ toml = "1.0" # Filter expressions evalexpr = { workspace = true } +# Shell-word splitting (MCP server command field) +shlex = "1" + # Process-group kill (safe wrapper around killpg) — Unix-only; kill_process_group # has a #[cfg(not(unix))] fallback in acp.rs. [target.'cfg(unix)'.dependencies] diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index e8e8fdd51..8708c9dbb 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -4,6 +4,8 @@ //! Config file (TOML) for complex subscription rules. use std::collections::{HashMap, HashSet}; + +use serde::{Deserialize, Serialize}; use std::path::PathBuf; use clap::Parser; @@ -37,6 +39,9 @@ pub(crate) const MAX_TURN_DURATION_CEILING_SECS: u64 = 604_800; #[derive(Debug, Error)] pub enum ConfigError { + #[error("invalid BUZZ_ACP_MCP_SERVERS: {0}")] + McpServers(String), + #[error("failed to parse nostr keys: {0}")] KeyParse(#[from] nostr::key::Error), @@ -193,6 +198,34 @@ pub struct ModelsArgs { pub json: bool, } +/// User-configured stdio MCP server received from desktop via +/// `BUZZ_ACP_MCP_SERVERS`. The desktop resolves `enabled` as local layering +/// metadata before serializing this transport shape. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct ConfiguredMcpServer { + pub name: String, + pub command: String, + #[serde(default)] + pub args: Vec, + #[serde(default)] + pub env: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct ConfiguredMcpEnvVar { + pub name: String, + pub value: String, +} + +pub fn parse_configured_mcp_servers( + raw: Option<&str>, +) -> Result, ConfigError> { + let Some(raw) = raw.filter(|value| !value.trim().is_empty()) else { + return Ok(Vec::new()); + }; + serde_json::from_str(raw).map_err(|error| ConfigError::McpServers(error.to_string())) +} + #[derive(Debug, Parser)] #[command( name = "buzz-acp", @@ -447,6 +480,8 @@ pub struct Config { pub agent_command: String, pub agent_args: Vec, pub mcp_command: String, + /// Desktop-resolved MCP servers, appended after the built-in Buzz server. + pub configured_mcp_servers: Vec, pub idle_timeout_secs: u64, pub max_turn_duration_secs: u64, pub agents: u32, @@ -917,12 +952,16 @@ impl Config { validate_multiple_event_handling(args.multiple_event_handling, args.dedup)?; + let configured_mcp_servers = + parse_configured_mcp_servers(std::env::var("BUZZ_ACP_MCP_SERVERS").ok().as_deref())?; + let config = Config { keys, relay_url: args.relay_url, agent_command, agent_args, mcp_command: args.mcp_command, + configured_mcp_servers, idle_timeout_secs, max_turn_duration_secs, agents: args.agents, @@ -1300,6 +1339,7 @@ mod tests { agent_command: "goose".into(), agent_args: vec!["acp".into()], mcp_command: "".into(), + configured_mcp_servers: vec![], idle_timeout_secs: DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, @@ -1706,6 +1746,28 @@ mod tests { assert!(f.kinds.is_none(), "wildcard should propagate"); } + #[test] + fn parse_configured_mcp_servers_accepts_json_transport() { + let servers = parse_configured_mcp_servers(Some( + r#"[{"name":"github","command":"npx","args":["github-mcp"],"env":[{"name":"TOKEN","value":"secret"}]}]"#, + )) + .unwrap(); + + assert_eq!(servers.len(), 1); + assert_eq!(servers[0].name, "github"); + assert_eq!(servers[0].args, vec!["github-mcp"]); + assert_eq!(servers[0].env[0].name, "TOKEN"); + assert_eq!( + serde_json::to_value(&servers).unwrap()[0]["enabled"], + serde_json::Value::Null + ); + } + + #[test] + fn parse_configured_mcp_servers_rejects_invalid_json() { + assert!(parse_configured_mcp_servers(Some("not-json")).is_err()); + } + #[test] fn test_config_mode_no_matching_rules_empty_result() { let config = test_config(SubscribeMode::Config); diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index e459c9f57..db15a141a 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -3402,48 +3402,86 @@ async fn run_models(args: ModelsArgs) -> Result<()> { } fn build_mcp_servers(config: &Config) -> Vec { - if config.mcp_command.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, - }); + let mut servers = Vec::new(); + if !config.mcp_command.is_empty() { + servers.push(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, + }); + } } - } - env - }, - }] + env + }, + }); + } + servers.extend( + config + .configured_mcp_servers + .iter() + .map(configured_mcp_server), + ); + servers +} + +fn configured_mcp_server(server: &config::ConfiguredMcpServer) -> McpServer { + // Shell-word split: users may type `uv run "/My Bot/jambot"` as the command. + // shlex handles quoting so paths with spaces survive. On parse failure + // (unmatched quote), pass the whole string as the command verbatim — the + // spawn will fail, and D1 surfaces MCP spawn errors as warnings. + let (command, prefix_args) = match shlex::split(&server.command) { + Some(mut tokens) if !tokens.is_empty() => { + let cmd = tokens.remove(0); + (cmd, tokens) + } + _ => (server.command.clone(), vec![]), + }; + + let mut args = prefix_args; + args.extend(server.args.iter().cloned()); + + McpServer { + name: server.name.clone(), + command, + args, + env: server + .env + .iter() + .map(|variable| EnvVar { + name: variable.name.clone(), + value: variable.value.clone(), + }) + .collect(), + } } #[cfg(test)] @@ -3875,6 +3913,7 @@ mod build_mcp_servers_tests { agent_command: "goose".into(), agent_args: vec!["acp".into()], mcp_command: "test-mcp-server".into(), + configured_mcp_servers: vec![], idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, @@ -3911,6 +3950,28 @@ mod build_mcp_servers_tests { } } + #[test] + fn build_mcp_servers_appends_configured_servers_after_builtin() { + let mut config = test_config(); + config.configured_mcp_servers = vec![config::ConfiguredMcpServer { + name: "github".into(), + command: "npx".into(), + args: vec!["github-mcp".into()], + env: vec![config::ConfiguredMcpEnvVar { + name: "TOKEN".into(), + value: "secret".into(), + }], + }]; + + let servers = build_mcp_servers(&config); + assert_eq!(servers.len(), 2); + assert_eq!(servers[0].name, "test-mcp-server"); + assert_eq!(servers[1].name, "github"); + assert_eq!(servers[1].command, "npx"); + assert_eq!(servers[1].args, vec!["github-mcp"]); + assert_eq!(servers[1].env[0].name, "TOKEN"); + } + #[test] fn session_new_mcp_server_has_required_fields() { let config = test_config(); @@ -3961,13 +4022,32 @@ mod build_mcp_servers_tests { } #[test] - fn empty_mcp_command_returns_no_servers() { + fn empty_mcp_command_omits_builtin_but_includes_user_servers() { let mut config = test_config(); config.mcp_command = "".into(); + config.configured_mcp_servers = vec![config::ConfiguredMcpServer { + name: "github".into(), + command: "npx".into(), + args: vec![], + env: vec![], + }]; + let servers = build_mcp_servers(&config); + // The built-in slot is omitted (mcp_command is empty), but user-configured + // servers are always included — they do not depend on the built-in server. + assert_eq!(servers.len(), 1); + assert_eq!(servers[0].name, "github"); + } + + #[test] + fn empty_mcp_command_with_no_user_servers_returns_empty() { + let mut config = test_config(); + config.mcp_command = "".into(); + config.configured_mcp_servers = vec![]; + assert!( - servers.is_empty(), - "empty mcp_command should produce no MCP servers" + build_mcp_servers(&config).is_empty(), + "no built-in and no user servers should yield an empty list" ); } @@ -4003,6 +4083,71 @@ mod build_mcp_servers_tests { "Path::new(\".\").file_stem() is None — should fall back to \"mcp\"" ); } + + #[test] + fn configured_mcp_server_shell_splits_command() { + let server = config::ConfiguredMcpServer { + name: "jambot".into(), + command: "uv run /path/to/jambot".into(), + args: vec!["--port".into(), "8080".into()], + env: vec![], + }; + + let result = super::configured_mcp_server(&server); + assert_eq!(result.command, "uv"); + assert_eq!( + result.args, + vec!["run", "/path/to/jambot", "--port", "8080"], + ); + assert_eq!(result.name, "jambot"); + } + + #[test] + fn configured_mcp_server_bare_command_unchanged() { + let server = config::ConfiguredMcpServer { + name: "github".into(), + command: "npx".into(), + args: vec!["github-mcp".into()], + env: vec![], + }; + + let result = super::configured_mcp_server(&server); + assert_eq!(result.command, "npx"); + assert_eq!(result.args, vec!["github-mcp"]); + } + + #[test] + fn configured_mcp_server_quoted_path_with_spaces() { + let server = config::ConfiguredMcpServer { + name: "jambot".into(), + command: r#"uv run "/Users/me/My Bot/.venv/bin/jambot""#.into(), + args: vec!["--port".into(), "8080".into()], + env: vec![], + }; + + let result = super::configured_mcp_server(&server); + assert_eq!(result.command, "uv"); + assert_eq!( + result.args, + vec!["run", "/Users/me/My Bot/.venv/bin/jambot", "--port", "8080"], + "quoted path must preserve internal spaces and strip quotes" + ); + } + + #[test] + fn configured_mcp_server_unmatched_quote_falls_back_to_verbatim() { + let server = config::ConfiguredMcpServer { + name: "broken".into(), + command: r#"uv run "/unclosed/path"#.into(), + args: vec!["--flag".into()], + env: vec![], + }; + + let result = super::configured_mcp_server(&server); + // Unmatched quote → shlex returns None → verbatim fallback + assert_eq!(result.command, r#"uv run "/unclosed/path"#); + assert_eq!(result.args, vec!["--flag"]); + } } #[cfg(test)] @@ -4040,6 +4185,7 @@ mod error_outcome_emission_tests { agent_command: "true".into(), agent_args: vec![], mcp_command: "test-mcp-server".into(), + configured_mcp_servers: vec![], idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index e57f44882..8a050fe6b 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -707,14 +707,39 @@ async fn create_session_and_apply_model( None }; - let resp = agent + let resp = match agent .acp .session_new_full( &ctx.cwd, ctx.mcp_servers.clone(), combined_system_prompt.as_deref(), ) - .await?; + .await + { + Ok(r) => r, + Err(AcpError::AgentError { ref message, .. }) + if !ctx.mcp_servers.is_empty() && message.to_lowercase().contains("mcp") => + { + // MCP spawn failure — retry without MCP servers so the agent can + // still respond. Surface the original error as a warning. + tracing::warn!( + target: "pool::mcp", + "MCP server spawn failed ({message}); retrying session without MCP servers" + ); + agent.acp.observe( + "mcp_spawn_warning", + serde_json::json!({ + "error": message, + "action": "retried_without_mcp_servers", + }), + ); + agent + .acp + .session_new_full(&ctx.cwd, vec![], combined_system_prompt.as_deref()) + .await? + } + Err(e) => return Err(e), + }; // Populate model capabilities on first session creation. if agent.model_capabilities.is_none() { @@ -4875,4 +4900,191 @@ mod tests { "timestamp must not use +00:00 offset" ); } + + // ── MCP fallback (D1) regression tests ─────────────────────────────────── + + use crate::acp::{AcpClient, McpServer}; + use crate::relay::RestClient; + + /// Spawn a bash script as an AcpClient, matching the acp.rs test helper. + async fn spawn_script(script: &str) -> AcpClient { + AcpClient::spawn("bash", &["-c".into(), script.into()], &[], false) + .await + .expect("failed to spawn test script") + } + + /// Build a minimal `PromptContext` with the given MCP servers. + /// Only `mcp_servers` and `cwd` are exercised by `create_session_and_apply_model`; + /// other fields are inert defaults. + fn test_prompt_context(mcp_servers: Vec) -> PromptContext { + let keys = Keys::generate(); + PromptContext { + mcp_servers, + initial_message: None, + idle_timeout: Duration::from_secs(60), + max_turn_duration: Duration::from_secs(60), + turn_liveness_interval: Duration::ZERO, + dedup_mode: crate::config::DedupMode::Drop, + system_prompt: None, + team_instructions: None, + heartbeat_prompt: None, + base_prompt: None, + cwd: "/tmp".to_string(), + rest_client: RestClient { + http: reqwest::Client::new(), + base_url: String::new(), + keys: keys.clone(), + auth_tag_json: None, + }, + channel_info: std::collections::HashMap::new(), + context_message_limit: 0, + max_turns_per_session: 0, + permission_mode: crate::config::PermissionMode::Default, + agent_keys: keys, + agent_owner_pubkey: None, + memory_enabled: false, + harness_name: "test".to_string(), + } + } + + /// Build a minimal `OwnedAgent` from a pre-initialized `AcpClient`. + fn test_owned_agent(acp: AcpClient) -> OwnedAgent { + OwnedAgent { + index: 0, + acp, + state: SessionState { + sessions: HashMap::new(), + heartbeat_session: None, + turn_counts: HashMap::new(), + heartbeat_turn_count: 0, + core_sections: HashMap::new(), + canvas_sections: HashMap::new(), + }, + model_capabilities: None, + desired_model: None, + model_overridden: false, + protocol_version: 1, + } + } + + #[tokio::test] + async fn test_mcp_spawn_error_retries_without_mcp_servers() { + // Script: respond to initialize, then return an MCP error on the first + // session/new, and a valid session on the second (which should carry + // empty mcpServers). + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ1 + echo '{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"mcp: spawn jambot: No such file or directory"}}' + read -t 2 REQ2 + echo '{"jsonrpc":"2.0","id":2,"result":{"sessionId":"ses_fallback","_retryRequest":'"$REQ2"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let mut agent = test_owned_agent(client); + let ctx = test_prompt_context(vec![McpServer { + name: "jambot".into(), + command: "jambot".into(), + args: vec![], + env: vec![], + }]); + + let result = create_session_and_apply_model(&mut agent, &ctx, None, None).await; + let session_id = result.expect("fallback session should succeed"); + assert_eq!(session_id, "ses_fallback"); + } + + #[tokio::test] + async fn test_mcp_retry_sends_empty_mcp_servers() { + // Capture the retry request to a temp file and assert mcpServers: []. + let capture_file = + std::env::temp_dir().join(format!("buzz_mcp_retry_test_{}.json", std::process::id())); + let capture_path = capture_file.display().to_string(); + let script = format!( + r#" + read -t 2 _init + echo '{{"jsonrpc":"2.0","id":0,"result":{{"protocolVersion":1,"agentCapabilities":{{}}}}}}' + read -t 2 _req1 + echo '{{"jsonrpc":"2.0","id":1,"error":{{"code":-32000,"message":"mcp: spawn failed"}}}}' + read -t 2 REQ2 + echo "$REQ2" > {capture_path} + echo '{{"jsonrpc":"2.0","id":2,"result":{{"sessionId":"ses_ok"}}}}' + sleep 1 + "# + ); + let mut client = spawn_script(&script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let mut agent = test_owned_agent(client); + let ctx = test_prompt_context(vec![McpServer { + name: "broken".into(), + command: "broken-server".into(), + args: vec![], + env: vec![], + }]); + + let result = create_session_and_apply_model(&mut agent, &ctx, None, None).await; + let session_id = result.expect("retry should succeed"); + assert_eq!(session_id, "ses_ok"); + + // Read the captured retry request and verify mcpServers is empty. + let captured = std::fs::read_to_string(&capture_file) + .expect("retry request should have been written to capture file"); + std::fs::remove_file(&capture_file).ok(); + let parsed: serde_json::Value = + serde_json::from_str(captured.trim()).expect("captured request should be valid JSON"); + let mcp_servers = parsed + .pointer("/params/mcpServers") + .expect("retry request must contain params.mcpServers"); + assert_eq!( + mcp_servers, + &serde_json::json!([]), + "retry must send empty mcpServers" + ); + } + + #[tokio::test] + async fn test_non_mcp_agent_error_propagates() { + // Non-MCP AgentError must NOT trigger the retry — it should propagate. + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 _req + echo '{"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"authentication failed"}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let mut agent = test_owned_agent(client); + let ctx = test_prompt_context(vec![McpServer { + name: "server".into(), + command: "some-server".into(), + args: vec![], + env: vec![], + }]); + + let result = create_session_and_apply_model(&mut agent, &ctx, None, None).await; + assert!( + result.is_err(), + "non-MCP error must propagate, not trigger retry" + ); + let err = result.unwrap_err(); + assert!( + err.to_string().contains("authentication failed"), + "error message must be preserved: {err}" + ); + } } diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 2b75d8b72..b3b41e6a6 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -79,7 +79,10 @@ const overrides = new Map([ // 3-phase (stage/stop/commit) + commit_cascade_agents injectable helper for // retry-safety. Load-bearing reviewer-required change; queued to split. // Consolidation removed the legacy persona-card import/export codecs. - ["src-tauri/src/commands/personas/mod.rs", 984], + // +6: local-only MCP layer validation/update wiring. Load-bearing; queued to split. + // MCP validation and inherited-layer effective-cap save gate; the gate counts + // trailing newlines, so 994 physical lines are 995 counted lines. + ["src-tauri/src/commands/personas/mod.rs", 995], // #1418 read-path fix: get_thread_replies' blocker fix (shared TIMELINE_KINDS // const + build_thread_replies_filter helper, mirroring the channel sibling so // the two p-gate filters can't drift) plus two guard unit tests. The file was @@ -165,9 +168,10 @@ const overrides = new Map([ // setup-mode requirements. The Windows-only requirement and serialization // test add eight lines; split remains queued with the existing file debt. // Windows Doctor install fix: cli_install_commands_windows field added to test stubs. - // team-instructions-first-class: ManagedAgentRecord fixture gains the new - // team_id field (+1 line). - ["src-tauri/src/managed_agents/readiness.rs", 1765], + // `team-instructions-first-class` added `team_id`; MCP configuration added + // the required `mcp_servers` minimal-fixture field. The gate counts the + // trailing newline, so 1,765 physical lines are 1,766 counted lines. + ["src-tauri/src/managed_agents/readiness.rs", 1766], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing @@ -200,9 +204,10 @@ const overrides = new Map([ // RawInstallRuntimeResult + fromRawInstallRuntimeResult mapper (+2). // Git Bash Doctor discovery adds the raw Tauri response and its camelCase // mapper. This is the existing API boundary; split remains queued. - // team-instructions-first-class: createManagedAgent Tauri bridge threads the - // new teamId input through to the backend (+1 line). - ["src/shared/api/tauri.ts", 1305], + // `team-instructions-first-class` added the create `teamId` wire field; MCP + // configuration adds the raw/read/create `mcpServers` plumbing. The gate + // counts trailing newlines, so 1,308 physical lines are 1,309 counted lines. + ["src/shared/api/tauri.ts", 1309], // doctor-npm-eacces-preflight: hint field added to InstallStepResult (+1 line). // codex-acp-package-swap: "adapter_outdated" variant added to AcpAvailabilityStatus (+1 line). // doctor-install-reliability: AuthStatus tagged union + nodeRequired/authStatus/ @@ -212,12 +217,16 @@ const overrides = new Map([ // mcp-readonly-view rebase: PR2 MCP config surface FE-type fields force +1 over the grandfathered ceiling. // Git Bash prerequisite payload adds four fields to the shared Tauri API // contract. This is the canonical type location; split remains queued. - // signout-wipe: resetFailed field added to Identity type (+6 lines). - // team-instructions-first-class: CreateManagedAgentInput.teamId (+2, incl. - // doc comment) and AgentTeam/CreateTeamInput/UpdateTeamInput.instructions - // (+3) — the new team-id spawn link and the runtime-layered instructions - // field. - ["src/shared/api/types.ts", 1047], + // `signout-wipe` added `Identity.resetFailed`; `team-instructions-first-class` + // added team/instructions fields; MCP config adds the shared server types, + // layer fields, IPC input fields, and effective-surface field. The gate + // counts trailing newlines. Upstream added eight non-MCP type lines after the + // prior ceiling, and the MCP snapshot exclusion added no production types. + ["src/shared/api/types.ts", 1085], + // MCP snapshot-exclusion coverage adds a credential-bearing fixture and a + // negative serialization assertion. `agent_snapshot.rs` was already exactly + // at the 1,000-line limit; the gate counts trailing newlines. + ["src-tauri/src/managed_agents/agent_snapshot.rs", 1010], // readiness-gate: PersonaDialog.tsx threads computeLocalModeGate + // requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog // shows required markers and credential amber rows (parity with @@ -289,9 +298,10 @@ const overrides = new Map([ // candidates, cli_install_commands_for_os PowerShell selection, login_shell_path // None regression, .cmd shim resolution, no-git-bash error hint. // +32: deterministic .cmd resolver + no-registry + install_shell_from tests. - // team-instructions-first-class: record_with test fixture gained the new - // ManagedAgentRecord.team_id field (+1 line) alongside persona_team_dir. - ["src-tauri/src/managed_agents/discovery/tests.rs", 1271], + // `team-instructions-first-class` added `team_id`; MCP configuration added + // two required `mcp_servers` test-fixture fields. The gate counts the + // trailing newline, so 1,272 physical lines are 1,273 counted lines. + ["src-tauri/src/managed_agents/discovery/tests.rs", 1273], // identity-import-keyring: the identity resolution state machine's behavioral // matrix (46 tests over FakeIdentityStore — probe × marker × file cells, // adoption / read-back-corruption / marker-failure arms, recovery-mode @@ -419,7 +429,9 @@ const overrides = new Map([ // (if let Some(provider_update) = input.provider { record.provider = provider_update; }). // +8: harness_override thread-through in update_managed_agent so a deliberate // Custom pin routes to update_time_agent_command_override (comment + call). - ["src-tauri/src/commands/agent_models.rs", 1079], + // +3: effective-merge MCP cap backstop in update_managed_agent — validates + // the three-layer merge stays within MAX_USER_MCP_SERVERS at save time. + ["src-tauri/src/commands/agent_models.rs", 1082], // global-agent-config: get_agent_config_surface / write_agent_config_field / // put_agent_session_config commands + GlobalAgentConfig serde types. New file // in this PR; queued to split with the command module refactor. @@ -427,7 +439,11 @@ const overrides = new Map([ // is_safe_to_reveal allowlist + baked_env_thinking_effort_is_unmasked test. // +1: doctor-install-reliability: login_hint: None added to goose_runtime test stub. // +1: doctor-install-reliability review fixes: auth_probe_args: None added to stub. - ["src-tauri/src/commands/agent_config.rs", 1021], + // MCP config surface: `resolve_config_surface` receives the effective command + // and exposes the effective `buzz_agent_mcp_servers` merge; coverage pins the + // merge, disabled-mask, and non-buzz-agent cases. The gate counts trailing + // newlines, so 1,139 physical lines are 1,140 counted lines. + ["src-tauri/src/commands/agent_config.rs", 1140], // codex-install-auto-restart review-fixes: should_restart_after_install // takes pid_alive:bool (pure predicate, no OS-dependent call); 3 racy // cache tests replaced with 6 pure availability_drift predicate tests; @@ -479,7 +495,9 @@ const overrides = new Map([ // +2 provider-aware effort: model/provider props threaded to BuzzAgentModelTuningFields. // +15 provider/model dropdown fixes: useBakedBuildEnvKeysQuery + hideProviderIds // for Databricks v1 gate; prospectiveRuntimeId default fallback for builtins. - ["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1180], + // +18 PR3 MCP servers UI: mcpServers state, inheritedMcpServers memo, + // submit wire, and EditAgentAdvancedFields prop threading. + ["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1198], // AgentDefinitionDialog grew past 1000 with the following load-bearing fixes: // isRuntimeAutoSeededRef tracking for edit-mode seeding (Fizz shows models); // runtimeSupportsLlmProviderSelection guard on discovery provider (codex fix); diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index f6141a53e..5a250491c 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -55,6 +55,7 @@ fn resolve_config_surface( mut record: ManagedAgentRecord, personas: &[AgentDefinition], runtime_meta: Option<&KnownAcpRuntime>, + effective_command: &str, session_cache: Option<&SessionConfigCache>, global: &GlobalAgentConfig, ) -> RuntimeConfigSurface { @@ -152,6 +153,19 @@ fn resolve_config_surface( baseline.as_ref().map(|(m, o)| (m.as_str(), o.clone())), ); + if runtime_meta.is_some_and(|m| m.id == "buzz-agent") { + surface.buzz_agent_mcp_servers = crate::managed_agents::effective_buzz_agent_mcp_servers( + &record, + personas, + &global.mcp_servers, + effective_command, + ) + .unwrap_or_else(|error| { + eprintln!("buzz-desktop: invalid persisted MCP server configuration: {error}"); + Vec::new() + }); + } + // Re-tag persona-sourced fields from BuzzExplicit to PersonaDefault. if !had_prompt { retag_persona_default(&mut surface.normalized.system_prompt); @@ -375,6 +389,7 @@ pub async fn get_agent_config_surface( record, &personas, runtime_meta, + &effective_cmd, session_cache.as_ref(), &global, )) @@ -607,6 +622,7 @@ mod tests { fn agent_record() -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: "agent".to_string(), name: "Agent".to_string(), persona_id: Some("persona-1".to_string()), @@ -663,6 +679,7 @@ mod tests { fn persona_with_model(model: &str) -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: "persona-1".to_string(), display_name: "Persona".to_string(), avatar_url: None, @@ -711,6 +728,7 @@ mod tests { record, &personas, Some(goose_runtime()), + "goose", None, &Default::default(), ); @@ -738,6 +756,7 @@ mod tests { record, &personas, Some(goose_runtime()), + "goose", Some(&cache), &Default::default(), ); @@ -766,6 +785,7 @@ mod tests { record, &personas, Some(goose_runtime()), + "goose", Some(&cache), &Default::default(), ); @@ -793,6 +813,7 @@ mod tests { record, &personas, Some(goose_runtime()), + "goose", Some(&cache), &Default::default(), ); @@ -817,6 +838,7 @@ mod tests { record, &personas, Some(goose_runtime()), + "goose", Some(&cache), &Default::default(), ); @@ -844,6 +866,7 @@ mod tests { let personas: Vec = vec![]; let cache = session_cache("model-y", true); let global = crate::managed_agents::GlobalAgentConfig { + mcp_servers: vec![], model: Some("global-model".to_string()), ..Default::default() }; @@ -852,6 +875,7 @@ mod tests { record, &personas, Some(goose_runtime()), + "goose", Some(&cache), &global, ); @@ -873,6 +897,102 @@ mod tests { ); } + // ── buzz_agent_mcp_servers surface tests ──────────────────────────────── + + fn mcp_server( + name: &str, + command: &str, + enabled: bool, + ) -> crate::managed_agents::McpServerConfig { + crate::managed_agents::McpServerConfig { + name: name.to_string(), + command: command.to_string(), + args: vec![], + env: vec![], + enabled, + } + } + + /// The buzz-agent runtime surfaces its effective-merged MCP servers onto + /// `RuntimeConfigSurface.buzz_agent_mcp_servers`, matching what + /// `effective_buzz_agent_mcp_servers` (the spawn-time source of truth) + /// would compute for the same record/persona/global layers. + #[test] + fn buzz_agent_runtime_surfaces_effective_merged_mcp_servers() { + let mut record = agent_record(); + record.persona_id = None; + record.mcp_servers = vec![mcp_server("agent-server", "agent-cmd", true)]; + let personas: Vec = vec![]; + let global = crate::managed_agents::GlobalAgentConfig { + mcp_servers: vec![mcp_server("global-server", "global-cmd", true)], + ..Default::default() + }; + + let surface = resolve_config_surface( + record, + &personas, + known_acp_runtime("buzz-agent"), + "buzz-agent", + None, + &global, + ); + + let mut names: Vec<&str> = surface + .buzz_agent_mcp_servers + .iter() + .map(|s| s.name.as_str()) + .collect(); + names.sort_unstable(); + assert_eq!(names, vec!["agent-server", "global-server"]); + } + + /// A disabled agent-layer entry masks a same-named global entry — the + /// merged effective list must NOT include it. Fails against a variant + /// that surfaces the raw record layer instead of the merged result. + #[test] + fn buzz_agent_disabled_override_masks_inherited_server_in_surface() { + let mut record = agent_record(); + record.persona_id = None; + record.mcp_servers = vec![mcp_server("shared", "", false)]; + let personas: Vec = vec![]; + let global = crate::managed_agents::GlobalAgentConfig { + mcp_servers: vec![mcp_server("shared", "global-cmd", true)], + ..Default::default() + }; + + let surface = resolve_config_surface( + record, + &personas, + known_acp_runtime("buzz-agent"), + "buzz-agent", + None, + &global, + ); + + assert!(surface.buzz_agent_mcp_servers.is_empty()); + } + + /// Non-buzz-agent runtimes (e.g. goose) must never populate + /// `buzz_agent_mcp_servers` — they surface their servers via `extensions` + /// instead. Fails against a variant that populates the field unconditionally. + #[test] + fn non_buzz_agent_runtime_leaves_mcp_surface_empty() { + let mut record = agent_record(); + record.mcp_servers = vec![mcp_server("agent-server", "agent-cmd", true)]; + let personas = vec![persona_with_model("persona-model")]; + + let surface = resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + "goose", + None, + &Default::default(), + ); + + assert!(surface.buzz_agent_mcp_servers.is_empty()); + } + // ── get_baked_build_env / is_secret_key tests ────────────────────────── /// Build a `BakedEnvEntry` vec from a synthetic map, mirroring what diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 9d180539d..a4bab7735 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -872,6 +872,10 @@ pub async fn update_managed_agent( crate::managed_agents::validate_user_env_keys(&env_vars)?; record.env_vars = env_vars; } + if let Some(mcp_servers) = input.mcp_servers { + crate::managed_agents::validate_mcp_servers(&mcp_servers)?; + record.mcp_servers = mcp_servers; + } // Inbound author gate: merge patch onto current values, then validate // the merged state. This lets a single update switch to Allowlist AND @@ -896,6 +900,22 @@ pub async fn update_managed_agent( record.respond_to_allowlist = prospective_allowlist; } + // Effective-merge cap: the record now carries the updated fields — + // validate that the three-layer merge stays within the cap. A rename + // can un-mask an inherited server, pushing the effective count over. + { + let personas = load_personas(&app).unwrap_or_default(); + let effective_command = crate::managed_agents::record_agent_command(record, &personas); + let global_config = + crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); + crate::managed_agents::validate_effective_mcp_cap( + record, + &personas, + &global_config.mcp_servers, + &effective_command, + )?; + } + record.updated_at = now_iso(); save_managed_agents(&app, &records)?; diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index daa448cc6..f626058d3 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -412,6 +412,7 @@ pub async fn create_managed_agent( } } crate::managed_agents::validate_user_env_keys(&input.env_vars)?; + crate::managed_agents::validate_mcp_servers(&input.mcp_servers)?; // Validate & normalize the respond-to allowlist BEFORE any side effects. // The harness has its own validator (buzz-acp/src/config.rs) but we want @@ -716,6 +717,7 @@ pub async fn create_managed_agent( persona_team_dir: None, persona_name_in_team: None, env_vars: input.env_vars.clone(), + mcp_servers: input.mcp_servers.clone(), created_at: now_iso(), updated_at: now_iso(), last_started_at: None, @@ -739,6 +741,19 @@ pub async fn create_managed_agent( relay_mesh: relay_mesh.clone(), }; + // Effective-merge cap: validate the prospective three-layer merge + // (global < definition < agent) stays within MAX_USER_MCP_SERVERS. + // Per-layer validation already ran, but a per-layer-valid agent can + // still exceed the cap when layered on top of inherited servers. + let global_config = + crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); + crate::managed_agents::validate_effective_mcp_cap( + &record, + &personas, + &global_config.mcp_servers, + &record.agent_command, + )?; + records.push(record); save_managed_agents(&app, &records)?; diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index be132225e..e1911bef6 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -93,6 +93,13 @@ pub(super) fn build_deploy_payload( effective_model.map(str::to_string), effective_provider.map(str::to_string), ); + let effective_command = crate::managed_agents::record_agent_command(record, &personas); + let effective_mcp_servers = crate::managed_agents::effective_buzz_agent_mcp_servers( + record, + &personas, + &global_config.mcp_servers, + &effective_command, + )?; Ok(deploy_payload_json( record, @@ -103,6 +110,7 @@ pub(super) fn build_deploy_payload( effective_model, effective_provider, merged_env, + crate::managed_agents::mcp_server_transport(effective_mcp_servers), )) } @@ -115,6 +123,7 @@ pub(super) fn deploy_payload_json( effective_model: Option, effective_provider: Option, merged_env: std::collections::BTreeMap, + effective_mcp_servers: Vec, ) -> serde_json::Value { serde_json::json!({ "name": &record.name, @@ -145,5 +154,7 @@ pub(super) fn deploy_payload_json( // Merged persona + agent env vars. Providers that don't read this // field will simply ignore it — no protocol break. "env_vars": merged_env, + // Local MCP configuration resolves exactly like the local spawn path. + "mcp_servers": effective_mcp_servers, }) } diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index eb673353f..517001800 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -1,5 +1,5 @@ use super::*; -use crate::managed_agents::AgentDefinition; +use crate::managed_agents::{mcp_server_transport, AgentDefinition, McpServerConfig}; fn bare_agent_record( persona_id: Option<&str>, @@ -9,6 +9,7 @@ fn bare_agent_record( use crate::managed_agents::{BackendKind, RespondTo}; use std::collections::BTreeMap; ManagedAgentRecord { + mcp_servers: vec![], pubkey: "agent".to_string(), name: "Agent".to_string(), persona_id: persona_id.map(str::to_string), @@ -65,6 +66,7 @@ fn bare_agent_record( fn persona_record(id: &str, model: Option<&str>, provider: Option<&str>) -> AgentDefinition { use std::collections::BTreeMap; AgentDefinition { + mcp_servers: vec![], id: id.to_string(), display_name: "Test Persona".to_string(), avatar_url: None, @@ -140,6 +142,7 @@ fn deploy_resolver_falls_back_to_global_when_persona_and_record_have_none() { let record = bare_agent_record(Some("p1"), None, None); let personas = vec![persona_record("p1", None, None)]; let global = crate::managed_agents::GlobalAgentConfig { + mcp_servers: vec![], model: Some("global-model".to_string()), provider: Some("global-prov".to_string()), ..Default::default() @@ -365,6 +368,13 @@ fn deploy_payload_carries_the_full_behavioral_quad() { Some("gpt-x".to_string()), Some("openai".to_string()), std::collections::BTreeMap::new(), + mcp_server_transport(vec![McpServerConfig { + name: "example".to_string(), + command: "example-mcp".to_string(), + args: vec![], + env: vec![], + enabled: true, + }]), ); assert_eq!(payload["parallelism"], 4); @@ -373,4 +383,6 @@ fn deploy_payload_carries_the_full_behavioral_quad() { assert_eq!(payload["model"], "gpt-x"); assert_eq!(payload["provider"], "openai"); assert_eq!(payload["relay_url"], "wss://relay.example"); + assert_eq!(payload["mcp_servers"][0]["name"], "example"); + assert!(payload["mcp_servers"][0].get("enabled").is_none()); } diff --git a/desktop/src-tauri/src/commands/global_agent_config.rs b/desktop/src-tauri/src/commands/global_agent_config.rs index 37a1be522..1bc447fb8 100644 --- a/desktop/src-tauri/src/commands/global_agent_config.rs +++ b/desktop/src-tauri/src/commands/global_agent_config.rs @@ -76,6 +76,16 @@ pub async fn set_global_agent_config( let phase1 = tokio::task::spawn_blocking(move || { validate_global_config(&config)?; + // Reject global save if the prospective MCP config would push any + // existing buzz-agent record over the effective-server cap. + let records = load_managed_agents(&app_for_write)?; + let personas = load_personas(&app_for_write)?; + crate::managed_agents::validate_effective_mcp_cap_for_records( + &records, + &personas, + &config.mcp_servers, + )?; + let old_global = load_global_agent_config(&app_for_write).unwrap_or_default(); save_global_agent_config(&app_for_write, &config)?; diff --git a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs index 316af5f72..a4a0a3f8c 100644 --- a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs +++ b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs @@ -17,6 +17,7 @@ fn make_agent( runtime_pid: Option, ) -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: pubkey.to_string(), name: "Test Agent".to_string(), persona_id: persona_id.map(str::to_string), diff --git a/desktop/src-tauri/src/commands/personas/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound_tests.rs index 6c7012e7e..b15d67f44 100644 --- a/desktop/src-tauri/src/commands/personas/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound_tests.rs @@ -10,6 +10,7 @@ const UUID: &str = "11111111-2222-3333-4444-555555555555"; /// IS its UUID id. Carries env_vars + source_team that must survive a patch. fn local_in_app() -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: UUID.to_string(), display_name: "Local".to_string(), avatar_url: None, @@ -35,6 +36,7 @@ fn local_in_app() -> AgentDefinition { /// slug = Some(d-tag), empty env_vars, source_team None. fn inbound_for(d_tag: &str, display_name: &str) -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: d_tag.to_string(), display_name: display_name.to_string(), avatar_url: Some("https://example.com/a.png".to_string()), @@ -155,6 +157,7 @@ const AGENT_PUBKEY: &str = "agentpubkeyhex00000000000000000000000000000000000000 /// event must NEVER be able to overwrite. fn local_agent() -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: AGENT_PUBKEY.to_string(), name: "Local Agent".to_string(), persona_id: Some("persona-local".to_string()), diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index 5ae6fbf48..e5925418b 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -77,6 +77,7 @@ pub async fn create_persona( .filter(|s| !s.is_empty()) .collect(); crate::managed_agents::validate_user_env_keys(&input.env_vars)?; + crate::managed_agents::validate_mcp_servers(&input.mcp_servers)?; let mut persona = AgentDefinition { id: Uuid::new_v4().to_string(), display_name, @@ -91,6 +92,7 @@ pub async fn create_persona( source_team: None, source_team_persona_slug: None, env_vars: input.env_vars, + mcp_servers: input.mcp_servers, respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, @@ -200,10 +202,38 @@ pub async fn update_persona( crate::managed_agents::validate_user_env_keys(&env_vars)?; persona.env_vars = env_vars; } + crate::managed_agents::replace_mcp_servers( + &mut persona.mcp_servers, + &input.mcp_servers, + )?; apply_persona_behavior(persona, input.behavior)?; persona.updated_at = now_iso(); + // Clone before the cap check so the mutable borrow from + // `personas.iter_mut().find()` ends here. let result = persona.clone(); + + // Reject persona save if the prospective MCP config would push + // any existing buzz-agent referencing this persona over the + // effective-server cap. + { + let agents = load_managed_agents(&app)?; + let global_config = + crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); + // Filter to agents referencing this persona — the rest are + // unaffected and skipped by the resolver (wrong persona_id). + let referencing: Vec<_> = agents + .iter() + .filter(|a| a.persona_id.as_deref() == Some(input.id.as_str())) + .cloned() + .collect(); + crate::managed_agents::validate_effective_mcp_cap_for_records( + &referencing, + &personas, + &global_config.mcp_servers, + )?; + } + save_personas(&app, &personas)?; retain_persona_pending(&app, &state, &result); diff --git a/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs index ba855ccbd..2ee21fbe3 100644 --- a/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs +++ b/desktop/src-tauri/src/commands/personas/name_propagation_tests.rs @@ -5,6 +5,7 @@ use super::*; fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: format!("pubkey-{name}"), name: name.to_string(), persona_id: Some(persona_id.to_string()), diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index 9e2b8b392..bf2392c4d 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -411,6 +411,7 @@ pub async fn confirm_agent_snapshot_import( source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + mcp_servers: Vec::new(), respond_to: respond_to_wire.clone(), respond_to_allowlist: minted.respond_to_allowlist.clone(), parallelism: minted_parallelism, @@ -453,6 +454,7 @@ pub async fn confirm_agent_snapshot_import( provider: snapshot.definition.provider.clone(), persona_source_version: None, env_vars: std::collections::BTreeMap::new(), + mcp_servers: Vec::new(), start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index b1d19f06b..58de423bc 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -42,6 +42,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { provider: None, persona_source_version: None, env_vars: BTreeMap::new(), + mcp_servers: Vec::new(), start_on_app_launch: false, auto_restart_on_config_change: false, runtime_pid: None, diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index dc376ca06..f7fb383e1 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -132,6 +132,7 @@ fn definition_from_snapshot( source_team: None, source_team_persona_slug: None, env_vars: Default::default(), + mcp_servers: Default::default(), respond_to, respond_to_allowlist: behavior.respond_to_allowlist, parallelism: behavior.parallelism, @@ -572,6 +573,9 @@ pub async fn confirm_team_snapshot_import( provider: member.definition.provider.clone(), persona_source_version: None, env_vars: std::collections::BTreeMap::new(), + // MCP configuration is local-only, never part of a shared team snapshot. + // Imported instances therefore inherit no local override. + mcp_servers: Vec::new(), start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index ca7dc6183..039119a12 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -67,6 +67,7 @@ fn team_export_round_trip_preserves_team_and_excludes_member_memory() { source_team: None, source_team_persona_slug: None, env_vars: Default::default(), + mcp_servers: vec![], respond_to: None, respond_to_allowlist: vec![], parallelism: None, @@ -87,6 +88,7 @@ fn team_export_round_trip_preserves_team_and_excludes_member_memory() { source_team: None, source_team_persona_slug: None, env_vars: Default::default(), + mcp_servers: vec![], respond_to: None, respond_to_allowlist: vec![], parallelism: None, @@ -148,6 +150,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { source_team: None, source_team_persona_slug: None, env_vars: Default::default(), + mcp_servers: vec![], respond_to: None, respond_to_allowlist: vec![], parallelism: None, @@ -194,6 +197,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { provider: None, persona_source_version: None, env_vars: Default::default(), + mcp_servers: vec![], start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, @@ -315,6 +319,7 @@ fn team_import_definitions_are_built_for_all_members() { definition.id.len() == 36 && definition.source_team.is_none() && definition.env_vars.is_empty() + && definition.mcp_servers.is_empty() && definition.respond_to_allowlist.is_empty() })); assert_eq!(definitions[0].system_prompt, "Alice prompt"); diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index ba4407d16..0737a2830 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -18,6 +18,7 @@ //! - `private_key_nsec` — the agent's secret key. //! - `auth_tag` — the NIP-OA owner attestation. //! - `env_vars` — may hold API keys / credentials. +//! - `mcp_servers` — env entries may hold API keys / credentials. //! - `backend` — `Provider { config }` is an opaque blob that may hold secrets. //! - any runtime field (`runtime_pid`, `last_*`, `backend_agent_id`, …) — these //! mutate on every start/stop and describe transient process state. @@ -179,6 +180,16 @@ mod tests { provider: Some("anthropic".to_string()), persona_source_version: Some("abc123".to_string()), env_vars: BTreeMap::from([("OPENAI_API_KEY".to_string(), "sk-secret".to_string())]), + mcp_servers: vec![super::super::McpServerConfig { + name: "secret-mcp".to_string(), + command: "mcp-command".to_string(), + args: vec!["--secret".to_string()], + env: vec![super::super::McpServerEnvVar { + name: "MCP_TOKEN".to_string(), + value: "mcp-secret".to_string(), + }], + enabled: true, + }], start_on_app_launch: true, auto_restart_on_config_change: true, runtime_pid: Some(4242), @@ -260,6 +271,9 @@ mod tests { assert!(!json.contains("OPENAI_API_KEY"), "leaked env var key"); assert!(!json.contains("sk-secret"), "leaked env var value"); assert!(!json.contains("env_vars"), "leaked env var field"); + assert!(!json.contains("mcp_servers"), "leaked MCP server field"); + assert!(!json.contains("secret-mcp"), "leaked MCP server name"); + assert!(!json.contains("mcp-secret"), "leaked MCP server secret"); assert!( !json.contains("sk-provider-secret"), "leaked provider config secret" diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs index b0bf8f599..f42a78367 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs @@ -21,7 +21,7 @@ //! The following fields are NEVER serialized: //! - `private_key_nsec` / any private key material //! - `auth_tag` (NIP-OA) -//! - `env_vars` (API keys / credentials) +//! - `env_vars` and `mcp_servers` (API keys / credentials) //! - `relay_url` (machine-local endpoint) //! - `acp_command` / `agent_command` / `agent_command_override` / `agent_args` //! (machine-local harness paths) @@ -500,6 +500,16 @@ mod tests { m.insert("API_KEY".to_string(), "secret123".to_string()); // MUST NOT appear m }, + mcp_servers: vec![crate::managed_agents::McpServerConfig { + name: "SENTINEL_MCP_NAME".to_string(), + command: "SENTINEL_MCP_COMMAND".to_string(), + args: Vec::new(), + env: vec![crate::managed_agents::McpServerEnvVar { + name: "SENTINEL_MCP_TOKEN".to_string(), + value: "SENTINEL_MCP_SECRET".to_string(), + }], + enabled: true, + }], // MUST NOT appear in snapshot start_on_app_launch: true, auto_restart_on_config_change: true, runtime_pid: Some(12345), // MUST NOT appear @@ -744,6 +754,18 @@ mod tests { ); } + #[test] + fn secret_exclusion_mcp_servers_absent() { + let json = snapshot_json_string(&minimal_record()); + assert!( + !json.contains("mcpServers") + && !json.contains("mcp_servers") + && !json.contains("SENTINEL_MCP_NAME") + && !json.contains("SENTINEL_MCP_SECRET"), + "local MCP configuration must not appear in snapshots" + ); + } + #[test] fn secret_exclusion_relay_url_absent() { let record = minimal_record(); diff --git a/desktop/src-tauri/src/managed_agents/backend.rs b/desktop/src-tauri/src/managed_agents/backend.rs index 0eada757d..2ab467769 100644 --- a/desktop/src-tauri/src/managed_agents/backend.rs +++ b/desktop/src-tauri/src/managed_agents/backend.rs @@ -318,14 +318,22 @@ fn redact_secrets_with(s: &str, extras: &[&str]) -> String { result } -/// Collect string values from `request["agent"]["env_vars"]` (if present) -/// to feed into [`redact_secrets_with`]. Returns an empty Vec if the -/// request shape doesn't match, which is fine — falls back to the default -/// prefix-based scrubbing. +/// Collect secret values from a provider deploy request to feed into +/// [`redact_secrets_with`]. Harvests: +/// - `request["agent"]["env_vars"]` — flat string map of env values. +/// - `request["agent"]["mcp_servers"][*]["env"][*]["value"]` — per-server env +/// values, which may hold API keys for third-party MCP servers. +/// +/// Returns an empty Vec if the request shape doesn't match, which is fine — +/// falls back to the default prefix-based scrubbing. fn env_secrets_from_request(request: &serde_json::Value) -> Vec { - request - .get("agent") - .and_then(|a| a.get("env_vars")) + let agent = match request.get("agent") { + Some(a) => a, + None => return Vec::new(), + }; + + let mut secrets: Vec = agent + .get("env_vars") .and_then(|e| e.as_object()) .map(|obj| { obj.values() @@ -334,7 +342,25 @@ fn env_secrets_from_request(request: &serde_json::Value) -> Vec { .map(String::from) .collect() }) - .unwrap_or_default() + .unwrap_or_default(); + + if let Some(servers) = agent.get("mcp_servers").and_then(|s| s.as_array()) { + for server in servers { + if let Some(env) = server.get("env").and_then(|e| e.as_array()) { + for entry in env { + if let Some(value) = entry + .get("value") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + { + secrets.push(value.to_string()); + } + } + } + } + } + + secrets } /// Public-in-crate helper: redact every non-empty value from `env` (plus @@ -584,6 +610,76 @@ mod tests { ); } + #[test] + fn env_secrets_from_request_extracts_mcp_server_env_values() { + let req = serde_json::json!({ + "op": "deploy", + "agent": { + "env_vars": { + "ANTHROPIC_API_KEY": "sk-ant-agent", + }, + "mcp_servers": [ + { + "name": "github", + "command": "npx", + "args": [], + "env": [ + {"name": "GITHUB_TOKEN", "value": "ghp-secret"}, + {"name": "EMPTY_VAR", "value": ""}, + ], + }, + { + "name": "no-env", + "command": "npx", + "args": [], + }, + ], + }, + }); + let secrets = env_secrets_from_request(&req); + assert!( + secrets.iter().any(|v| v == "sk-ant-agent"), + "env_vars value missing" + ); + assert!( + secrets.iter().any(|v| v == "ghp-secret"), + "mcp server env value missing" + ); + // Empty values must be filtered out. + assert!( + !secrets.iter().any(|v| v.is_empty()), + "empty value must not be collected" + ); + assert_eq!(secrets.len(), 2); + } + + #[test] + fn mcp_server_env_secret_is_redacted_from_provider_error() { + // Regression test for F1: a provider that echoes an MCP env value in + // its stderr must not leak the secret into the desktop-visible error. + let secret = "ghp-very-secret-token"; + let request = serde_json::json!({ + "agent": { + "env_vars": {}, + "mcp_servers": [{ + "name": "github", + "command": "npx", + "args": [], + "env": [{"name": "GITHUB_TOKEN", "value": secret}], + }], + }, + }); + let secrets = env_secrets_from_request(&request); + let secret_refs: Vec<&str> = secrets.iter().map(String::as_str).collect(); + let stderr = format!("provider failed: invalid token {secret}"); + let redacted = redact_secrets_with(&stderr, &secret_refs); + assert!( + !redacted.contains(secret), + "secret must be redacted from provider error" + ); + assert!(redacted.contains("[REDACTED]")); + } + #[test] fn redact_env_values_in_scrubs_map_values() { let mut env = std::collections::BTreeMap::new(); diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index 372d2cfde..be6393e40 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -209,6 +209,10 @@ pub(crate) fn read_config_surface( normalized, advanced, extensions, + // Populated by `resolve_config_surface` for the buzz-agent runtime only; + // this reader has no access to the global/persona MCP layers needed to + // compute the effective merge. + buzz_agent_mcp_servers: Vec::new(), sources, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index f5fc7c389..85a8b7ae6 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -81,6 +81,7 @@ fn test_record() -> ManagedAgentRecord { system_prompt: None, model: None, env_vars: BTreeMap::new(), + mcp_servers: Vec::new(), start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs index 15ccb718e..a2a6ee7d9 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs @@ -2,6 +2,8 @@ use std::collections::BTreeMap; use serde::{Deserialize, Serialize}; +use crate::managed_agents::types::McpServerConfig; + /// Where a config value came from — determines precedence and UI annotations. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -136,6 +138,12 @@ pub struct RuntimeConfigSurface { pub normalized: NormalizedConfig, pub advanced: Vec, pub extensions: Vec, + /// Effective (global < definition < agent merged, enabled-only) buzz-agent + /// MCP servers — "what runs." Populated only for the `buzz-agent` runtime; + /// empty for every other runtime, which surface their servers via + /// `extensions` instead. + #[serde(default)] + pub buzz_agent_mcp_servers: Vec, pub sources: ConfigSourceReport, } diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 608b511fa..94c12d766 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -191,6 +191,7 @@ fn classifies_cli_missing_when_adapter_found_but_cli_absent() { fn persona_with_runtime(id: &str, runtime: Option<&str>) -> crate::managed_agents::AgentDefinition { crate::managed_agents::AgentDefinition { + mcp_servers: vec![], id: id.to_string(), display_name: id.to_string(), avatar_url: None, @@ -230,6 +231,7 @@ fn record_with( override_cmd: Option<&str>, ) -> crate::managed_agents::types::ManagedAgentRecord { crate::managed_agents::types::ManagedAgentRecord { + mcp_servers: vec![], pubkey: String::new(), name: "r".to_string(), persona_id: persona_id.map(str::to_string), diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 6318a5c83..08f382389 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -71,6 +71,9 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_ACP_AGENT_COMMAND", "BUZZ_ACP_AGENT_ARGS", "BUZZ_ACP_MCP_COMMAND", + // Structured MCP configuration transport. Must only be set by desktop + // after resolving local-only global/definition/agent layers. + "BUZZ_ACP_MCP_SERVERS", // Security gates: respond-to mode + allowlist + legacy owner-only // fallback. Overriding would make the running agent's gate diverge // from the saved/UI-visible settings. diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index cf8629f82..70ddb6777 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -30,7 +30,9 @@ use crate::managed_agents::env_vars::{ validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS, MAX_ENV_VALUE_BYTES, }; use crate::managed_agents::storage::{atomic_write_json_restricted, managed_agents_base_dir}; -use crate::managed_agents::types::{AgentDefinition, ManagedAgentRecord}; +use crate::managed_agents::types::{ + validate_mcp_servers, AgentDefinition, ManagedAgentRecord, McpServerConfig, +}; /// The global agent configuration record. /// @@ -50,6 +52,10 @@ pub struct GlobalAgentConfig { #[serde(default)] pub env_vars: BTreeMap, + /// Local-only MCP servers inherited by every buzz-agent instance. + #[serde(default)] + pub mcp_servers: Vec, + /// Global fallback provider (e.g. `"databricks_v2"`, `"anthropic"`). /// /// Used only when neither the agent record nor the linked persona specifies @@ -93,6 +99,8 @@ pub fn validate_global_config(config: &GlobalAgentConfig) -> Result<(), String> // Standard env-var key validation (POSIX shape, reserved-key check, NUL/size caps). validate_user_env_keys(&non_empty)?; + validate_mcp_servers(&config.mcp_servers)?; + // Reject derived provider/model keys in global env_vars. let derived: Vec<&str> = non_empty .keys() diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 24027b75f..f4e4c068f 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -8,6 +8,7 @@ use crate::managed_agents::{AgentDefinition, BackendKind, ManagedAgentRecord, Re fn config_with_env(pairs: &[(&str, &str)]) -> GlobalAgentConfig { GlobalAgentConfig { + mcp_servers: vec![], env_vars: pairs .iter() .map(|(k, v)| (k.to_string(), v.to_string())) @@ -92,6 +93,7 @@ fn validate_ignores_empty_values_for_reserved_key_check() { #[test] fn validate_rejects_provider_with_nul_byte() { let config = GlobalAgentConfig { + mcp_servers: vec![], provider: Some("anthropic\0evil".to_string()), ..Default::default() }; @@ -105,6 +107,7 @@ fn validate_rejects_provider_with_nul_byte() { #[test] fn validate_rejects_model_with_nul_byte() { let config = GlobalAgentConfig { + mcp_servers: vec![], model: Some("claude-opus-4\0evil".to_string()), ..Default::default() }; @@ -119,6 +122,7 @@ fn validate_rejects_model_with_nul_byte() { fn validate_rejects_provider_exceeding_size_cap() { use crate::managed_agents::env_vars::MAX_ENV_VALUE_BYTES; let config = GlobalAgentConfig { + mcp_servers: vec![], provider: Some("x".repeat(MAX_ENV_VALUE_BYTES + 1)), ..Default::default() }; @@ -133,6 +137,7 @@ fn validate_rejects_provider_exceeding_size_cap() { fn validate_rejects_model_exceeding_size_cap() { use crate::managed_agents::env_vars::MAX_ENV_VALUE_BYTES; let config = GlobalAgentConfig { + mcp_servers: vec![], model: Some("x".repeat(MAX_ENV_VALUE_BYTES + 1)), ..Default::default() }; @@ -146,6 +151,7 @@ fn validate_rejects_model_exceeding_size_cap() { #[test] fn validate_accepts_valid_provider_and_model() { let config = GlobalAgentConfig { + mcp_servers: vec![], provider: Some("anthropic".to_string()), model: Some("claude-opus-4-5".to_string()), ..Default::default() @@ -158,6 +164,7 @@ fn validate_accepts_valid_provider_and_model() { #[test] fn normalize_some_empty_provider_becomes_none() { let mut config = GlobalAgentConfig { + mcp_servers: vec![], provider: Some("".to_string()), ..Default::default() }; @@ -171,6 +178,7 @@ fn normalize_some_empty_provider_becomes_none() { #[test] fn normalize_whitespace_only_provider_becomes_none() { let mut config = GlobalAgentConfig { + mcp_servers: vec![], provider: Some(" ".to_string()), ..Default::default() }; @@ -184,6 +192,7 @@ fn normalize_whitespace_only_provider_becomes_none() { #[test] fn normalize_some_empty_model_becomes_none() { let mut config = GlobalAgentConfig { + mcp_servers: vec![], model: Some("".to_string()), ..Default::default() }; @@ -197,6 +206,7 @@ fn normalize_some_empty_model_becomes_none() { #[test] fn normalize_whitespace_only_model_becomes_none() { let mut config = GlobalAgentConfig { + mcp_servers: vec![], model: Some(" \t ".to_string()), ..Default::default() }; @@ -210,6 +220,7 @@ fn normalize_whitespace_only_model_becomes_none() { #[test] fn normalize_valid_provider_and_model_unchanged() { let mut config = GlobalAgentConfig { + mcp_servers: vec![], provider: Some("anthropic".to_string()), model: Some("claude-opus-4-5".to_string()), ..Default::default() @@ -256,6 +267,7 @@ fn strip_is_idempotent_on_all_non_empty() { fn default_config_is_all_none_empty() { let config = GlobalAgentConfig::default(); assert!(config.env_vars.is_empty()); + assert!(config.mcp_servers.is_empty()); assert!(config.provider.is_none()); assert!(config.model.is_none()); } @@ -263,6 +275,7 @@ fn default_config_is_all_none_empty() { #[test] fn roundtrip_serialization() { let config = GlobalAgentConfig { + mcp_servers: vec![], env_vars: BTreeMap::from([("ANTHROPIC_API_KEY".to_string(), "sk-test".to_string())]), provider: Some("anthropic".to_string()), model: Some("claude-opus-4".to_string()), @@ -284,6 +297,10 @@ fn default_global_config_serializes_all_fields() { json.contains("\"env_vars\""), "serialized JSON must always include env_vars; got: {json}" ); + assert!( + json.contains("\"mcp_servers\""), + "serialized JSON must always include mcp_servers; got: {json}" + ); assert!( json.contains("\"provider\""), "serialized JSON must always include provider; got: {json}" @@ -298,6 +315,7 @@ fn default_global_config_serializes_all_fields() { fn bare_record() -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: "agent".to_string(), name: "Agent".to_string(), persona_id: None, @@ -354,6 +372,7 @@ fn bare_record() -> ManagedAgentRecord { fn persona(id: &str, model: Option<&str>, provider: Option<&str>) -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: id.to_string(), display_name: "Test Persona".to_string(), avatar_url: None, @@ -390,6 +409,7 @@ fn resolve_agent_record_wins_over_persona_and_global() { Some("persona-provider"), )]; let global = GlobalAgentConfig { + mcp_servers: vec![], model: Some("global-model".to_string()), provider: Some("global-provider".to_string()), ..Default::default() @@ -419,6 +439,7 @@ fn resolve_persona_fallback_when_record_has_none() { Some("persona-provider"), )]; let global = GlobalAgentConfig { + mcp_servers: vec![], model: Some("global-model".to_string()), provider: Some("global-provider".to_string()), ..Default::default() @@ -449,6 +470,7 @@ fn resolve_global_fallback_when_record_and_persona_have_none() { // record.model / provider = None; persona.model / provider = None let personas = vec![persona("p1", None, None)]; let global = GlobalAgentConfig { + mcp_servers: vec![], model: Some("global-model".to_string()), provider: Some("global-provider".to_string()), ..Default::default() @@ -475,6 +497,7 @@ fn resolve_global_fallback_when_no_persona_linked() { let record = bare_record(); // persona_id = None, model/provider = None let personas: Vec = vec![]; let global = GlobalAgentConfig { + mcp_servers: vec![], model: Some("global-model".to_string()), provider: Some("global-provider".to_string()), ..Default::default() @@ -519,6 +542,7 @@ fn resolve_each_field_resolves_independently_through_tiers() { // persona.model = None → global fills model if record also had none, but // record has model here so global is not needed for model. let global = GlobalAgentConfig { + mcp_servers: vec![], model: Some("global-model".to_string()), provider: Some("global-provider".to_string()), ..Default::default() @@ -541,6 +565,7 @@ fn resolve_each_field_resolves_independently_through_tiers() { #[test] fn populated_global_config_round_trips() { let original = GlobalAgentConfig { + mcp_servers: vec![], env_vars: [("ANTHROPIC_API_KEY".to_string(), "sk-test".to_string())] .into_iter() .collect(), @@ -572,6 +597,7 @@ fn record_runtime_wins_over_persona_runtime_for_command_resolution() { record.persona_id = Some("p1".to_string()); let persona = AgentDefinition { + mcp_servers: vec![], id: "p1".to_string(), display_name: "Goose persona".to_string(), avatar_url: None, diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index a95938160..46a0bbad3 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -412,6 +412,7 @@ fn ensure_cli_symlink_does_not_clobber_regular_file_dev() { fn make_persona(id: &str, display_name: &str) -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: id.to_string(), display_name: display_name.to_string(), avatar_url: None, @@ -435,6 +436,7 @@ fn make_persona(id: &str, display_name: &str) -> AgentDefinition { fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: String::new(), name: name.to_string(), persona_id: persona_id.map(|s| s.to_string()), diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index 02b281523..31c94b906 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -191,6 +191,10 @@ pub fn persona_from_event(event: &nostr::Event) -> Result AgentDefinition { source_team: None, source_team_persona_slug: Some("test-slug".to_string()), env_vars: BTreeMap::from([("KEY".to_string(), "value".to_string())]), + mcp_servers: vec![crate::managed_agents::McpServerConfig { + name: "local-mcp".to_string(), + command: "mcp-command".to_string(), + args: vec![], + env: vec![crate::managed_agents::McpServerEnvVar { + name: "MCP_TOKEN".to_string(), + value: "mcp-secret".to_string(), + }], + enabled: true, + }], respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, @@ -132,9 +142,13 @@ fn round_trip_serialization() { assert_eq!(restored.model, Some("claude-opus-4".to_string())); assert_eq!(restored.provider, Some("anthropic".to_string())); assert_eq!(restored.name_pool, vec!["Alpha", "Beta"]); - // env_vars are not included in public persona events (secrets travel - // via NIP-44-encrypted engrams only). + // Env vars and MCP servers are local-only and absent from public events. assert!(restored.env_vars.is_empty()); + assert!(restored.mcp_servers.is_empty()); + let wire = serde_json::to_string(&persona_event_content(&record)).unwrap(); + assert!(!wire.contains("mcp_servers")); + assert!(!wire.contains("local-mcp")); + assert!(!wire.contains("mcp-secret")); assert_eq!( restored.source_team_persona_slug, Some("test-slug".to_string()) @@ -208,6 +222,7 @@ fn content_matches_nip_ap_vector() { // signed content, so a second implementer following the spec computes // the same NIP-01 id. let record = AgentDefinition { + mcp_servers: vec![], id: "test-agent".to_string(), display_name: "Test Agent".to_string(), avatar_url: Some("https://example.com/avatar.png".to_string()), @@ -237,6 +252,7 @@ fn content_matches_nip_ap_vector() { #[test] fn round_trip_minimal_persona() { let record = AgentDefinition { + mcp_servers: vec![], id: "minimal".to_string(), display_name: "Minimal".to_string(), avatar_url: None, @@ -332,6 +348,7 @@ fn behavioral_defaults_survive_record_round_trip() { #[test] fn quad_absent_definition_hash_stable_across_activation() { let record = AgentDefinition { + mcp_servers: vec![], id: "quad-absent".to_string(), display_name: "Test".to_string(), avatar_url: None, @@ -374,6 +391,7 @@ fn quad_absent_definition_hash_stable_across_activation() { /// way `persona_from_event` maps fields, without needing a signed event. fn persona_from_event_content_for_test(content: PersonaEventContent) -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: "staged".to_string(), display_name: content.display_name, avatar_url: content.avatar_url, @@ -506,6 +524,7 @@ fn snapshot_runtime_verbatim_from_persona() { /// Helper: a persona with no model/provider configured. fn blank_model_persona() -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], model: None, provider: None, ..sample_persona() diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index 605148b19..5359f3a9c 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -237,6 +237,7 @@ fn built_in_persona_records(now: &str) -> Vec { source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + mcp_servers: Vec::new(), respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, @@ -295,6 +296,7 @@ fn merge_personas(mut stored: Vec, now: &str) -> (Vec AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: id.to_string(), display_name: display_name.to_string(), avatar_url: Some("https://example.com/avatar.png".to_string()), @@ -330,6 +331,7 @@ fn migrate_retires_unmodified_personas() { let mut stored: Vec = RETIRED_PERSONAS .iter() .map(|(id, prompt)| AgentDefinition { + mcp_servers: vec![], id: id.to_string(), system_prompt: prompt.to_string(), is_builtin: false, // already demoted by merge_personas @@ -365,6 +367,7 @@ fn migrate_retires_unmodified_personas() { fn migrate_preserves_customized_personas() { let now = "2026-04-01T00:00:00Z"; let mut stored = vec![AgentDefinition { + mcp_servers: vec![], id: "builtin:researcher".to_string(), display_name: "My Researcher".to_string(), system_prompt: "My custom research workflow with special instructions".to_string(), @@ -398,6 +401,7 @@ fn migrate_is_idempotent() { // 2. Already-retired persona (display_name ends with " (retired)") — no-op. let mut stored_with_retired = vec![AgentDefinition { + mcp_servers: vec![], id: "builtin:researcher".to_string(), display_name: "Researcher (retired)".to_string(), system_prompt: "My custom prompt".to_string(), @@ -413,6 +417,7 @@ fn migrate_is_idempotent() { // 3. Retired persona still marked is_builtin: true (pre-demotion). // migrate_retired_personas should still soft-deprecate it. let mut stored_pre_demotion = vec![AgentDefinition { + mcp_servers: vec![], id: "builtin:reviewer".to_string(), display_name: "Reviewer".to_string(), system_prompt: "Custom review prompt".to_string(), diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 6a5af8925..e0dbfc04e 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1369,6 +1369,7 @@ mod tests { // Minimal record: only the fields resolve_effective_agent_env reads. let record = crate::managed_agents::types::ManagedAgentRecord { + mcp_servers: vec![], pubkey: "test-pubkey".to_string(), name: "test-agent".to_string(), persona_id: None, diff --git a/desktop/src-tauri/src/managed_agents/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/relay_mesh.rs index 4c58344d6..94e9ecda5 100644 --- a/desktop/src-tauri/src/managed_agents/relay_mesh.rs +++ b/desktop/src-tauri/src/managed_agents/relay_mesh.rs @@ -62,6 +62,7 @@ mod tests { fn fixture() -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: "p".into(), name: "n".into(), persona_id: None, diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 3745fe571..7f90f7421 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1465,6 +1465,20 @@ pub(crate) fn build_respond_to_env( Ok((set, remove)) } +/// Set the desktop-resolved MCP transport only for the bundled `buzz-agent`. +/// Explicitly remove any inherited parent value for other runtimes. +fn set_buzz_agent_mcp_servers_env( + command: &mut std::process::Command, + effective_command: &str, + servers_json: String, +) { + if known_acp_runtime(effective_command).is_some_and(|runtime| runtime.id == "buzz-agent") { + command.env("BUZZ_ACP_MCP_SERVERS", servers_json); + } else { + command.env_remove("BUZZ_ACP_MCP_SERVERS"); + } +} + /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -1505,6 +1519,15 @@ pub fn spawn_agent_child( // and for the env-var merge at spawn time. let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); let effective_command = super::record_agent_command(record, &personas); + let effective_mcp_servers = super::effective_buzz_agent_mcp_servers( + record, + &personas, + &global.mcp_servers, + &effective_command, + )?; + let effective_mcp_servers_json = + serde_json::to_string(&super::mcp_server_transport(effective_mcp_servers)) + .map_err(|error| format!("failed to serialize effective MCP servers: {error}"))?; let agent_args = normalize_agent_args(&effective_command, record.agent_args.clone()); let resolved_acp_command = resolve_command(&record.acp_command) .ok_or_else(|| missing_command_message(&record.acp_command, "ACP harness command"))?; @@ -1573,6 +1596,7 @@ pub fn spawn_agent_child( command.env("BUZZ_RELAY_URL", &effective_relay_url); command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); command.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(",")); + set_buzz_agent_mcp_servers_env(&mut command, &effective_command, effective_mcp_servers_json); match &resolved_mcp_command { Some(mcp_cmd) => { command.env("BUZZ_ACP_MCP_COMMAND", mcp_cmd); diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 06d3ac6db..89c8043fc 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -123,6 +123,7 @@ fn fixture( auth_tag: Option, ) -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: "p".into(), name: "n".into(), persona_id: None, @@ -277,6 +278,7 @@ fn persona_with_provider( provider: Option<&str>, ) -> crate::managed_agents::AgentDefinition { crate::managed_agents::AgentDefinition { + mcp_servers: vec![], id: id.to_string(), display_name: id.to_string(), avatar_url: None, @@ -549,6 +551,30 @@ fn runtime_metadata_env_vars_injects_model_even_with_acp_model_switching() { // ── name_matches_known_binary / name_matches_interpreter tests ─────────── +#[test] +fn mcp_transport_is_absent_for_non_buzz_agent_children() { + let mut command = std::process::Command::new("true"); + command.env("BUZZ_ACP_MCP_SERVERS", "inherited"); + + super::set_buzz_agent_mcp_servers_env(&mut command, "goose", "[]".into()); + assert!( + !command + .get_envs() + .any(|(key, value)| key == "BUZZ_ACP_MCP_SERVERS" && value.is_some()), + "non-buzz-agent child must not receive MCP transport" + ); +} + +#[test] +fn mcp_transport_is_set_for_buzz_agent_children() { + let mut command = std::process::Command::new("true"); + super::set_buzz_agent_mcp_servers_env(&mut command, "buzz-agent", "[]".into()); + + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_MCP_SERVERS" && value == Some(std::ffi::OsStr::new("[]")) + })); +} + #[test] fn name_matches_known_binary_rejects_node() { // `node` must NOT be in KNOWN_AGENT_BINARIES — adding it there would diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 63a57ce5d..7f543de65 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -88,6 +88,21 @@ pub(crate) fn spawn_config_hash( let effective_command = crate::managed_agents::record_agent_command(record, personas); let runtime_meta = known_acp_runtime(&effective_command); let effective = resolve_effective_agent_env(record, personas, runtime_meta, global); + let effective_mcp_servers = (runtime_meta.is_some_and(|runtime| runtime.id == "buzz-agent")) + .then(|| { + super::effective_buzz_agent_mcp_servers( + record, + personas, + &global.mcp_servers, + &effective_command, + ) + // Invalid legacy disk state must still affect restart detection rather than + // panicking the summary path. A real spawn rejects the same configuration. + .unwrap_or_else(|error| { + eprintln!("buzz-desktop: invalid persisted MCP server configuration: {error}"); + Vec::new() + }) + }); let mut hasher = DefaultHasher::new(); @@ -103,6 +118,7 @@ pub(crate) fn spawn_config_hash( // Effective env layering (baked floor → runtime metadata → user env). // BTreeMap iteration is ordered, so this is deterministic. effective.env.hash(&mut hasher); + effective_mcp_servers.hash(&mut hasher); // Record fields the spawn env writes read directly. The relay is hashed // resolved: a blank record relay spawns on the workspace relay, so a diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index 9b7988162..ebe52f822 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -1,9 +1,25 @@ use super::*; -use crate::managed_agents::types::RespondTo; +use crate::managed_agents::{ + types::RespondTo, AgentDefinition, GlobalAgentConfig, McpServerConfig, McpServerEnvVar, +}; use std::collections::BTreeMap; +fn mcp_server(name: &str) -> McpServerConfig { + McpServerConfig { + name: name.into(), + command: "mcp-command".into(), + args: vec![], + env: vec![McpServerEnvVar { + name: "MCP_TOKEN".into(), + value: "secret".into(), + }], + enabled: true, + } +} + fn record() -> ManagedAgentRecord { ManagedAgentRecord { + mcp_servers: vec![], pubkey: "p".repeat(64), name: "agent".into(), persona_id: None, @@ -60,6 +76,7 @@ fn record() -> ManagedAgentRecord { fn persona(id: &str, runtime: Option<&str>, prompt: &str) -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: id.into(), display_name: id.into(), avatar_url: None, @@ -124,6 +141,80 @@ fn materializing_runtime_keeps_hash_stable() { ); } +#[test] +fn buzz_agent_mcp_edit_changes_hash_but_other_runtime_ignores_it() { + let mut buzz_agent = record(); + buzz_agent.runtime = Some("buzz-agent".into()); + let mut edited = buzz_agent.clone(); + edited.mcp_servers = vec![mcp_server("local-mcp")]; + + assert_ne!( + spawn_config_hash( + &buzz_agent, + &[], + &[], + "wss://ws.example", + &GlobalAgentConfig::default() + ), + spawn_config_hash( + &edited, + &[], + &[], + "wss://ws.example", + &GlobalAgentConfig::default() + ) + ); + + let mut goose = buzz_agent; + goose.runtime = Some("goose".into()); + let mut goose_edited = goose.clone(); + goose_edited.mcp_servers = vec![mcp_server("local-mcp")]; + assert_eq!( + spawn_config_hash( + &goose, + &[], + &[], + "wss://ws.example", + &GlobalAgentConfig::default() + ), + spawn_config_hash( + &goose_edited, + &[], + &[], + "wss://ws.example", + &GlobalAgentConfig::default() + ) + ); +} + +#[test] +fn definition_mcp_edit_changes_buzz_agent_hash_without_becoming_agent_override() { + let mut record = record(); + record.runtime = Some("buzz-agent".into()); + record.persona_id = Some("persona".into()); + let persona = persona("persona", Some("buzz-agent"), "prompt"); + let mut edited_persona = persona.clone(); + edited_persona.mcp_servers = vec![mcp_server("definition-mcp")]; + + assert_ne!( + spawn_config_hash( + &record, + &[persona], + &[], + "wss://ws.example", + &GlobalAgentConfig::default() + ), + spawn_config_hash( + &record, + &[edited_persona], + &[], + "wss://ws.example", + &GlobalAgentConfig::default() + ) + ); + assert!(record.mcp_servers.is_empty()); +} + #[test] fn record_env_var_edit_changes_hash() { let rec = record(); diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index d88a36272..173480c7a 100644 --- a/desktop/src-tauri/src/managed_agents/team_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/team_snapshot.rs @@ -278,6 +278,7 @@ mod tests { m.insert("API_KEY".to_string(), "secret123".to_string()); // MUST NOT appear m }, + mcp_servers: vec![], start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 7e9c7656b..d2a2c5b2e 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -176,6 +176,7 @@ fn managed_agent(name: &str) -> ManagedAgentRecord { agent_command_override: None, agent_args: vec![], mcp_command: String::new(), + mcp_servers: vec![], turn_timeout_seconds: 300, idle_timeout_seconds: None, max_turn_duration_seconds: None, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 9cb235476..24ce7b702 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -1,6 +1,9 @@ use serde::{Deserialize, Serialize}; use std::{collections::BTreeMap, path::PathBuf, process::Child}; +mod mcp_servers; +pub(crate) use mcp_servers::*; + #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(tag = "type", rename_all = "snake_case")] pub enum BackendKind { @@ -63,6 +66,9 @@ pub struct AgentDefinition { /// Stored as a BTreeMap for deterministic on-disk ordering. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub env_vars: BTreeMap, + /// Local-only MCP server layer. Never included in kind:30175 content. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub mcp_servers: Vec, /// NIP-AP behavioral defaults, stored in WIRE shape (kebab-case string, /// not the `RespondTo` enum) so `persona_event_content` is a verbatim /// copy and quad-absent records serialize byte-identically to the @@ -106,6 +112,10 @@ impl AgentDefinition { provider: self.provider, persona_source_version: None, env_vars: self.env_vars, + // Definition MCP servers stay on the definition layer and are + // resolved live at spawn/deploy. Copying them here would turn + // inheritance into a per-agent override and block later edits. + mcp_servers: Vec::new(), start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, @@ -164,6 +174,7 @@ impl ManagedAgentRecord { source_team: self.source_team.clone(), source_team_persona_slug: self.source_team_persona_slug.clone(), env_vars: self.env_vars.clone(), + mcp_servers: self.mcp_servers.clone(), respond_to: self.definition_respond_to.clone(), respond_to_allowlist: self.definition_respond_to_allowlist.clone(), parallelism: self.definition_parallelism, @@ -281,6 +292,10 @@ pub struct ManagedAgentRecord { /// To "override" a persona env var: set the same key here. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub env_vars: BTreeMap, + /// Local-only per-agent MCP overrides. Higher entries replace definition + /// and global entries by name; disabled entries mask inherited servers. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub mcp_servers: Vec, #[serde(default = "default_start_on_app_launch")] pub start_on_app_launch: bool, /// Auto-restart this agent when its effective spawn config drifts from diff --git a/desktop/src-tauri/src/managed_agents/types/mcp_servers.rs b/desktop/src-tauri/src/managed_agents/types/mcp_servers.rs new file mode 100644 index 000000000..e68ea17f5 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/types/mcp_servers.rs @@ -0,0 +1,297 @@ +use std::collections::{BTreeMap, HashSet}; + +use serde::{Deserialize, Serialize}; + +/// Maximum number of enabled user-configured MCP servers. The bundled +/// `buzz-dev-mcp` occupies the sixteenth `buzz-agent` MCP slot. +pub(crate) const MAX_USER_MCP_SERVERS: usize = 15; + +/// Maximum byte length of an MCP server name. Mirrors `MAX_NAME_LEN` in +/// `buzz-agent/src/mcp.rs`. +const MAX_SERVER_NAME_LEN: usize = 128; + +/// The built-in MCP server name reserved by the buzz-dev-mcp sidecar. +/// User-configured servers must not use this name — buzz-acp always appends +/// the built-in after user servers, making it a duplicate at spawn time. +const RESERVED_SERVER_NAME: &str = "buzz-dev-mcp"; + +/// An environment variable passed to an MCP subprocess. +/// +/// This deliberately mirrors the ACP `EnvVar` wire shape so the effective +/// configuration can be serialized without translating local secrets through +/// relay event content. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] +pub struct McpServerEnvVar { + pub name: String, + pub value: String, +} + +/// A locally persisted stdio MCP server configuration. +/// +/// Entries are layered `global < definition < agent` by name. A higher layer +/// replaces an entire entry; setting `enabled` to `false` masks a lower entry. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] +pub struct McpServerConfig { + pub name: String, + /// Required for enabled servers. Disabled entries may omit it because + /// their only purpose is to mask a lower-precedence server by name. + #[serde(default)] + pub command: String, + #[serde(default)] + pub args: Vec, + #[serde(default)] + pub env: Vec, + #[serde(default = "default_mcp_server_enabled")] + pub enabled: bool, +} + +/// The resolved MCP shape sent to `buzz-acp` and provider deployments. +/// `enabled` is local layering metadata, so it deliberately does not cross +/// the process or provider boundary. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub(crate) struct McpServerTransport { + pub name: String, + pub command: String, + pub args: Vec, + pub env: Vec, +} + +impl From for McpServerTransport { + fn from(server: McpServerConfig) -> Self { + Self { + name: server.name, + command: server.command, + args: server.args, + env: server.env, + } + } +} + +pub(crate) fn mcp_server_transport(servers: Vec) -> Vec { + servers.into_iter().map(Into::into).collect() +} + +fn default_mcp_server_enabled() -> bool { + true +} + +/// Validate one persisted MCP layer at every IPC save boundary. +/// +/// Checks: +/// - Names match the buzz-agent grammar: 1–128 bytes, ASCII alphanumeric, +/// `_`, or `-` only, no `__` substring. This mirrors `valid_name` in +/// `buzz-agent/src/mcp.rs` so save-time rejection matches spawn-time +/// rejection. +/// - Names must not be `buzz-dev-mcp` — that slot is reserved for the +/// bundled sidecar and would become a duplicate when buzz-acp appends it. +/// - No duplicate names within the layer. +/// - Enabled servers must have a non-empty command. +/// - No NUL bytes; individual values under the length ceiling. +/// - Total payload under the overall byte budget. +/// - At most `MAX_USER_MCP_SERVERS` enabled entries (layer-level cap; +/// the merged effective-server cap is enforced separately in +/// `merge_mcp_servers`). +pub(crate) fn validate_mcp_servers(servers: &[McpServerConfig]) -> Result<(), String> { + let mut names = HashSet::new(); + let mut enabled_count = 0usize; + let mut total_bytes = 0usize; + for server in servers { + // Grammar validation — must match buzz-agent's `valid_name` exactly. + if server.name.is_empty() { + return Err("MCP server name is required".to_string()); + } + if server.name.len() > MAX_SERVER_NAME_LEN { + return Err(format!( + "MCP server name `{}` exceeds the maximum length ({MAX_SERVER_NAME_LEN} bytes)", + server.name + )); + } + if !server + .name + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-') + { + return Err(format!( + "MCP server name `{}` contains invalid characters (only ASCII alphanumeric, `_`, and `-` are allowed)", + server.name + )); + } + if server.name.contains("__") { + return Err(format!( + "MCP server name `{}` cannot contain `__`", + server.name + )); + } + if server.name == RESERVED_SERVER_NAME { + return Err(format!( + "MCP server name `{RESERVED_SERVER_NAME}` is reserved" + )); + } + + if !names.insert(server.name.as_str()) { + return Err(format!("MCP server names must be unique: {}", server.name)); + } + if server.enabled && server.command.trim().is_empty() { + return Err(format!("MCP server `{}` command is required", server.name)); + } + if server.enabled { + enabled_count += 1; + } + + for (field, value) in std::iter::once(("name", &server.name)) + .chain((!server.command.is_empty()).then_some(("command", &server.command))) + .chain(server.args.iter().map(|arg| ("argument", arg))) + { + if value.contains('\0') { + return Err(format!( + "MCP server `{}` {field} cannot contain NUL bytes", + server.name + )); + } + if value.len() > crate::managed_agents::env_vars::MAX_ENV_VALUE_BYTES { + return Err(format!( + "MCP server `{}` {field} exceeds the maximum allowed length ({} bytes)", + server.name, + crate::managed_agents::env_vars::MAX_ENV_VALUE_BYTES + )); + } + total_bytes = total_bytes.saturating_add(value.len()); + } + + let mut env = BTreeMap::new(); + for variable in &server.env { + if env + .insert(variable.name.clone(), variable.value.clone()) + .is_some() + { + return Err(format!( + "MCP server `{}` has duplicate env var `{}`", + server.name, variable.name + )); + } + } + crate::managed_agents::validate_user_env_keys(&env)?; + total_bytes = total_bytes.saturating_add( + env.iter() + .map(|(key, value)| key.len() + value.len()) + .sum::(), + ); + } + + if enabled_count > MAX_USER_MCP_SERVERS { + return Err(format!( + "MCP server layer has {enabled_count} enabled servers; limit is {MAX_USER_MCP_SERVERS}" + )); + } + if total_bytes > crate::managed_agents::env_vars::MAX_ENV_TOTAL_BYTES { + return Err(format!( + "total MCP server payload is {total_bytes} bytes; limit is {}", + crate::managed_agents::env_vars::MAX_ENV_TOTAL_BYTES + )); + } + Ok(()) +} + +/// Validate and replace an optional MCP layer from an update request. +pub(crate) fn replace_mcp_servers( + current: &mut Vec, + replacement: &Option>, +) -> Result<(), String> { + if let Some(replacement) = replacement { + validate_mcp_servers(replacement)?; + *current = replacement.clone(); + } + Ok(()) +} + +/// Resolve global, definition, and per-agent MCP layers. Higher layers replace +/// whole entries by name, including disabled entries, so a disabled override +/// masks an inherited server instead of allowing it to leak through. +pub(crate) fn merge_mcp_servers( + global: &[McpServerConfig], + definition: &[McpServerConfig], + agent: &[McpServerConfig], +) -> Result, String> { + let mut merged = BTreeMap::new(); + for layer in [global, definition, agent] { + for server in layer { + merged.insert(server.name.clone(), server.clone()); + } + } + let effective: Vec = merged + .into_values() + .filter(|server| server.enabled) + .collect(); + if effective.len() > MAX_USER_MCP_SERVERS { + return Err(format!( + "effective MCP server count is {} but the maximum is {MAX_USER_MCP_SERVERS}", + effective.len() + )); + } + Ok(effective) +} + +/// Resolve MCP configuration only for the bundled `buzz-agent` runtime. +/// Other ACP runtimes keep their MCP configuration in their own config files. +pub(crate) fn effective_buzz_agent_mcp_servers( + record: &super::ManagedAgentRecord, + personas: &[super::AgentDefinition], + global: &[McpServerConfig], + effective_command: &str, +) -> Result, String> { + if super::super::known_acp_runtime(effective_command).map(|runtime| runtime.id) + != Some("buzz-agent") + { + return Ok(Vec::new()); + } + let definition = record + .persona_id + .as_deref() + .and_then(|id| personas.iter().find(|persona| persona.id == id)) + .map(|persona| persona.mcp_servers.as_slice()) + .unwrap_or_default(); + merge_mcp_servers(global, definition, &record.mcp_servers) +} + +/// Validate that the prospective effective-merge count stays within the +/// [`MAX_USER_MCP_SERVERS`] cap. Called at agent create/update to prevent a +/// record that passes per-layer validation but exceeds the cap after the +/// global < definition < agent merge. Non–buzz-agent runtimes skip the check +/// (they don't use `BUZZ_ACP_MCP_SERVERS`). +pub(crate) fn validate_effective_mcp_cap( + record: &super::ManagedAgentRecord, + personas: &[super::AgentDefinition], + global: &[McpServerConfig], + effective_command: &str, +) -> Result<(), String> { + // effective_buzz_agent_mcp_servers returns Ok(empty) for non-buzz-agent + // runtimes, so the cap check only fires for buzz-agent. + effective_buzz_agent_mcp_servers(record, personas, global, effective_command)?; + Ok(()) +} + +/// Validate the effective MCP cap across all existing agent records against a +/// prospective inherited layer (global or persona). Called at global-config +/// save and persona update to prevent an inherited-layer mutation that would +/// silently push an existing agent over the cap. +/// +/// Non–buzz-agent runtimes are skipped (they don't use `BUZZ_ACP_MCP_SERVERS`). +/// Returns the first offending agent's name in the error message. +pub(crate) fn validate_effective_mcp_cap_for_records( + records: &[super::ManagedAgentRecord], + personas: &[super::AgentDefinition], + global: &[McpServerConfig], +) -> Result<(), String> { + for record in records { + let effective_cmd = crate::managed_agents::record_agent_command(record, personas); + if let Err(merge_err) = + effective_buzz_agent_mcp_servers(record, personas, global, &effective_cmd) + { + return Err(format!( + "saving would push agent `{}` over the MCP server limit: {merge_err}", + record.name + )); + } + } + Ok(()) +} diff --git a/desktop/src-tauri/src/managed_agents/types/requests.rs b/desktop/src-tauri/src/managed_agents/types/requests.rs index 58d60218a..8f53a2085 100644 --- a/desktop/src-tauri/src/managed_agents/types/requests.rs +++ b/desktop/src-tauri/src/managed_agents/types/requests.rs @@ -7,7 +7,7 @@ use serde::Deserialize; use super::{ default_start_on_app_launch, validate_respond_to_allowlist, AgentDefinition, BackendKind, - RelayMeshConfig, RespondTo, + McpServerConfig, RelayMeshConfig, RespondTo, }; /// The NIP-AP behavioral group as one grouped request field. @@ -88,6 +88,9 @@ pub struct CreatePersonaRequest { /// Environment variables for agents created from this persona. #[serde(default)] pub env_vars: BTreeMap, + /// Local-only MCP server layer inherited by agents created from this definition. + #[serde(default)] + pub mcp_servers: Vec, /// NIP-AP behavioral group. Absent = behavior group stays unset. #[serde(default)] pub behavior: Option, @@ -116,6 +119,9 @@ pub struct UpdatePersonaRequest { /// stored credentials when an unrelated field is edited. #[serde(default)] pub env_vars: Option>, + /// Absent = don't touch. Present = replace this local-only MCP layer. + #[serde(default)] + pub mcp_servers: Option>, /// NIP-AP behavioral group. Same absent-vs-present contract as `env_vars`: /// absent = don't touch the stored behavior group (legacy callers don't send it), /// present = validate and replace the fields as a unit. @@ -166,6 +172,9 @@ pub struct CreateManagedAgentRequest { /// Environment variables for this agent. Layered on top of persona env. #[serde(default)] pub env_vars: BTreeMap, + /// Local-only MCP overrides for this agent. + #[serde(default)] + pub mcp_servers: Vec, #[serde(default)] pub spawn_after_create: bool, #[serde(default = "default_start_on_app_launch")] @@ -207,6 +216,9 @@ pub struct UpdateManagedAgentRequest { /// Absent = don't touch. Present = replace the env_vars map entirely. #[serde(default)] pub env_vars: Option>, + /// Absent = don't touch. Present = replace this local-only MCP layer. + #[serde(default)] + pub mcp_servers: Option>, #[serde(default)] pub parallelism: Option, /// Accepted for wire compatibility; not applied to the stored record. @@ -265,6 +277,7 @@ mod tests { fn record_without_quad() -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: "p-1".to_string(), display_name: "Test".to_string(), avatar_url: None, diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 667a41a53..838824426 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -1,4 +1,8 @@ -use super::{AgentDefinition, ManagedAgentRecord}; +use super::{ + merge_mcp_servers, replace_mcp_servers, validate_effective_mcp_cap, + validate_effective_mcp_cap_for_records, validate_mcp_servers, AgentDefinition, + ManagedAgentRecord, McpServerConfig, McpServerEnvVar, MAX_USER_MCP_SERVERS, +}; use std::path::PathBuf; #[test] @@ -472,6 +476,7 @@ fn sample_agent_record() -> ManagedAgentRecord { fn sample_persona() -> AgentDefinition { AgentDefinition { + mcp_servers: vec![], id: "custom:helper".to_string(), display_name: "Helper".to_string(), avatar_url: Some("https://example.com/a.png".to_string()), @@ -504,6 +509,10 @@ fn persona_into_agent_record_is_keyless_and_slugged() { assert_eq!(record.runtime.as_deref(), Some("goose")); assert_eq!(record.source_team.as_deref(), Some("team-1")); assert_eq!(record.env_vars.get("K").map(String::as_str), Some("v")); + assert!( + record.mcp_servers.is_empty(), + "definition MCP servers remain inherited" + ); } #[test] @@ -649,3 +658,329 @@ fn mint_rejects_out_of_range_input_parallelism() { "input-branch error must not blame the definition: {err}" ); } + +fn mcp_server(name: &str, command: &str, enabled: bool) -> McpServerConfig { + McpServerConfig { + name: name.into(), + command: command.into(), + args: vec![], + env: vec![], + enabled, + } +} + +#[test] +fn merge_mcp_servers_higher_layer_replaces_same_name() { + let merged = merge_mcp_servers( + &[mcp_server("shared", "global", true)], + &[mcp_server("shared", "persona", true)], + &[mcp_server("shared", "agent", true)], + ) + .unwrap(); + + assert_eq!(merged, vec![mcp_server("shared", "agent", true)]); +} + +#[test] +fn merge_mcp_servers_disabled_override_masks_lower_server() { + let merged = merge_mcp_servers( + &[mcp_server("shared", "global", true)], + &[], + &[mcp_server("shared", "agent", false)], + ) + .unwrap(); + + assert!(merged.is_empty()); +} + +#[test] +fn disabled_mask_requires_only_a_name() { + let mask = McpServerConfig { + name: "shared".into(), + command: String::new(), + args: vec![], + env: vec![], + enabled: false, + }; + validate_mcp_servers(std::slice::from_ref(&mask)).unwrap(); + assert!( + merge_mcp_servers(&[mcp_server("shared", "global", true)], &[], &[mask]) + .unwrap() + .is_empty() + ); +} + +#[test] +fn replace_mcp_servers_preserves_absent_and_replaces_present_empty_layer() { + let mut current = vec![mcp_server("existing", "mcp", true)]; + replace_mcp_servers(&mut current, &None).unwrap(); + assert_eq!(current, vec![mcp_server("existing", "mcp", true)]); + + replace_mcp_servers(&mut current, &Some(vec![])).unwrap(); + assert!(current.is_empty()); +} + +#[test] +fn merge_mcp_servers_rejects_more_than_fifteen_enabled_servers_after_merge() { + let global: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|index| mcp_server(&format!("global-{index}"), "mcp", true)) + .collect(); + let error = merge_mcp_servers(&global, &[mcp_server("persona", "mcp", true)], &[]) + .expect_err("post-merge cap must include servers from all layers"); + + assert!(error.contains("effective MCP server count is 16")); +} + +#[test] +fn validate_mcp_servers_rejects_empty_duplicate_and_reserved_env_inputs() { + assert!(validate_mcp_servers(&[mcp_server("", "mcp", true)]).is_err()); + assert!(validate_mcp_servers(&[ + mcp_server("same", "mcp", true), + mcp_server("same", "other", true), + ]) + .is_err()); + let mut server = mcp_server("server", "mcp", true); + server.env.push(McpServerEnvVar { + name: "BUZZ_ACP_MCP_SERVERS".into(), + value: "spoof".into(), + }); + assert!(validate_mcp_servers(&[server]).is_err()); +} + +#[test] +fn validate_mcp_servers_rejects_invalid_name_characters() { + // Space, slash, dot — not in the buzz-agent grammar. + for bad_name in &["bad name", "bad/name", "bad.name", "bad@name"] { + assert!( + validate_mcp_servers(&[mcp_server(bad_name, "cmd", true)]).is_err(), + "expected error for name {bad_name:?}" + ); + } +} + +#[test] +fn validate_mcp_servers_rejects_double_underscore_in_name() { + assert!(validate_mcp_servers(&[mcp_server("bad__name", "cmd", true)]).is_err()); +} + +#[test] +fn validate_mcp_servers_rejects_reserved_name() { + assert!(validate_mcp_servers(&[mcp_server("buzz-dev-mcp", "cmd", true)]).is_err()); +} + +#[test] +fn validate_mcp_servers_accepts_valid_grammar() { + // Alphanumeric, hyphens, underscores (but not double) are all OK. + assert!(validate_mcp_servers(&[mcp_server("github", "cmd", true)]).is_ok()); + assert!(validate_mcp_servers(&[mcp_server("my-server", "cmd", true)]).is_ok()); + assert!(validate_mcp_servers(&[mcp_server("my_server", "cmd", true)]).is_ok()); + assert!(validate_mcp_servers(&[mcp_server("Server123", "cmd", true)]).is_ok()); +} + +#[test] +fn validate_mcp_servers_rejects_more_than_fifteen_enabled_in_one_layer() { + let servers: Vec<_> = (0..=MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("server-{i}"), "cmd", true)) + .collect(); + let err = validate_mcp_servers(&servers).expect_err("layer cap must fire at 16 enabled"); + assert!(err.contains("enabled servers"), "error: {err}"); +} + +#[test] +fn validate_mcp_servers_allows_fifteen_enabled_in_one_layer() { + let servers: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("server-{i}"), "cmd", true)) + .collect(); + assert!(validate_mcp_servers(&servers).is_ok()); +} + +// ── validate_effective_mcp_cap — save-time effective merge cap ────────── + +/// Build a minimal buzz-agent ManagedAgentRecord with the given mcp_servers +/// layer. `agent_command = "buzz-agent"` so the effective check fires. +fn buzz_agent_record(mcp_servers: Vec) -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": "aabbccdd", + "name": "test", + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "buzz-agent", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "mcp_servers": mcp_servers, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + })) + .expect("minimal record should deserialize") +} + +#[test] +fn effective_cap_rejects_15_global_plus_1_local() { + let global: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + let record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + let err = validate_effective_mcp_cap(&record, &[], &global, "buzz-agent") + .expect_err("effective cap must reject 16 enabled"); + assert!(err.contains("effective MCP server count"), "error: {err}"); +} + +#[test] +fn effective_cap_allows_at_cap() { + let global: Vec<_> = (0..MAX_USER_MCP_SERVERS - 1) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + let record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + validate_effective_mcp_cap(&record, &[], &global, "buzz-agent") + .expect("15 effective should pass"); +} + +#[test] +fn effective_cap_rejects_rename_unmask() { + // Global has 15 enabled. The agent has a same-name override that masks + // one of them. If the override is "renamed" (simulated by changing the + // name to something unique), the masked global server is un-masked and + // the effective count goes to 16. + let global: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + // Agent overrides global-0 (masks it) — effective = 15. + let record_masked = buzz_agent_record(vec![mcp_server("global-0", "", false)]); + validate_effective_mcp_cap(&record_masked, &[], &global, "buzz-agent") + .expect("masked state should be at-cap and valid"); + + // Agent renamed its override → unique name, un-masks global-0 → effective = 16. + let record_unmasked = buzz_agent_record(vec![mcp_server("unique-name", "cmd", true)]); + let err = validate_effective_mcp_cap(&record_unmasked, &[], &global, "buzz-agent") + .expect_err("rename-unmask must reject"); + assert!(err.contains("effective MCP server count"), "error: {err}"); +} + +#[test] +fn effective_cap_skips_non_buzz_agent_runtime() { + let global: Vec<_> = (0..=MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + let record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + // Non-buzz-agent runtime → effective check returns Ok (skip). + validate_effective_mcp_cap(&record, &[], &global, "goose") + .expect("non-buzz-agent runtime should skip the cap"); +} + +// ── validate_effective_mcp_cap_for_records — inherited-layer gates ─────── + +/// Build a buzz-agent record with a persona reference and custom mcp_servers. +fn buzz_agent_record_with_persona( + name: &str, + persona_id: &str, + mcp_servers: Vec, +) -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": format!("pk-{name}"), + "name": name, + "persona_id": persona_id, + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "buzz-agent", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "mcp_servers": mcp_servers, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + })) + .expect("minimal record should deserialize") +} + +/// Build a minimal AgentDefinition (persona) with the given mcp_servers. +fn persona_with_mcp(id: &str, mcp_servers: Vec) -> AgentDefinition { + serde_json::from_value(serde_json::json!({ + "id": id, + "display_name": format!("Persona {id}"), + "system_prompt": "", + "mcp_servers": mcp_servers, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + })) + .expect("minimal persona should deserialize") +} + +#[test] +fn inherited_gate_rejects_global_14_to_15_with_1_local_agent() { + // Agent has 1 local enabled server. Global goes from 14 → 15. Effective + // would be 16 — the gate must reject with the agent's name. + let record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + let prospective_global: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + let err = validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect_err("global 14→15 with 1 local must reject"); + assert!( + err.contains("test"), + "error must name the offending agent: {err}" + ); + assert!( + err.contains("saving would push agent"), + "error must use the required phrasing: {err}" + ); +} + +#[test] +fn inherited_gate_allows_global_14_with_1_local_agent() { + // Agent has 1 local. Global stays at 14. Effective = 15 (at cap) — OK. + let record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + let prospective_global: Vec<_> = (0..MAX_USER_MCP_SERVERS - 1) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect("15 effective should pass"); +} + +#[test] +fn inherited_gate_rejects_persona_unmask_pushing_agent_over_cap() { + // Agent has 1 local enabled. Global has 14. Persona goes from 0 → 1 + // unique enabled server. Effective = 16 — reject. + let persona_id = "p1"; + let record = buzz_agent_record_with_persona( + "agent-a", + persona_id, + vec![mcp_server("local-0", "cmd", true)], + ); + let global: Vec<_> = (0..MAX_USER_MCP_SERVERS - 1) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + let persona = persona_with_mcp(persona_id, vec![mcp_server("persona-0", "cmd", true)]); + let err = validate_effective_mcp_cap_for_records(&[record], &[persona], &global) + .expect_err("persona add pushing over cap must reject"); + assert!( + err.contains("agent-a"), + "error must name the offending agent: {err}" + ); +} + +#[test] +fn inherited_gate_skips_non_buzz_agent_records() { + // A goose-runtime agent should be unaffected by the cap. + let mut record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + // record_agent_command resolves via agent_command_override first. + record.agent_command_override = Some("goose".to_string()); + let prospective_global: Vec<_> = (0..=MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect("non-buzz-agent runtime should skip the cap"); +} + +#[test] +fn inherited_gate_allows_unaffected_agent() { + // Agent with no local servers. Global at 15 → effective = 15. OK. + let record = buzz_agent_record(vec![]); + let prospective_global: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect("agent with no local servers at cap should pass"); +} diff --git a/desktop/src/features/agents/ui/AgentConfigPanel.tsx b/desktop/src/features/agents/ui/AgentConfigPanel.tsx index 26ecab3c0..c68563425 100644 --- a/desktop/src/features/agents/ui/AgentConfigPanel.tsx +++ b/desktop/src/features/agents/ui/AgentConfigPanel.tsx @@ -17,7 +17,7 @@ import { useAgentConfigSurface } from "../hooks"; import { cn } from "@/shared/lib/cn"; import { copyTextToClipboard } from "@/shared/lib/clipboard"; import { Spinner } from "@/shared/ui/spinner"; -import { McpServersSection } from "./McpServersSection"; +import { McpServersSection, BuzzAgentMcpServerRow } from "./McpServersSection"; import type { ConfigField, ConfigOrigin, @@ -366,8 +366,15 @@ export function AgentConfigPanel({ ); } - const { normalized, advanced, extensions, runtimeId, sources, isPreSpawn } = - data; + const { + normalized, + advanced, + extensions, + runtimeId, + sources, + isPreSpawn, + buzzAgentMcpServers, + } = data; const configFilePath = sources.configFilePath; const normalizedEntries = ( @@ -419,6 +426,19 @@ export function AgentConfigPanel({ 0 ? ( +
+ {buzzAgentMcpServers.map((server) => ( + + ))} +
+ ) : undefined + } extensions={extensions} runtimeId={runtimeId} variant={advancedMode === "flat" ? "profile" : "compact"} diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index ef7b80354..e2983b3d7 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -16,6 +16,7 @@ import { Textarea } from "@/shared/ui/textarea"; import { AgentCreationPreview } from "./AgentCreationPreview"; import { PersonaDropdownField } from "./PersonaDropdownField"; import type { EnvVarsValue } from "./EnvVarsEditor"; +import type { McpServersValue } from "./McpServersEditor"; import { PersonaAdvancedFields } from "./PersonaAdvancedFields"; import { PersonaModelField } from "./PersonaModelField"; import { @@ -128,6 +129,7 @@ export function AgentDefinitionDialog({ React.useState(false); const [namePoolText, setNamePoolText] = React.useState(""); const [envVars, setEnvVars] = React.useState({}); + const [mcpServers, setMcpServers] = React.useState([]); const [behaviorDraft, setBehaviorDraft] = React.useState( emptyPersonaBehaviorDraft, ); @@ -179,14 +181,18 @@ export function AgentDefinitionDialog({ : ""; const nextEnvVars = "envVars" in initialValues ? (initialValues.envVars ?? {}) : {}; + const nextMcpServers = + "mcpServers" in initialValues ? (initialValues.mcpServers ?? []) : []; const nextBehaviorDraft = draftFromBehavior(initialValues.behavior); behaviorSeedRef.current = draftFromBehavior(initialValues.behavior); setBehaviorDraft(nextBehaviorDraft); setNamePoolText(nextNamePoolText); setEnvVars(nextEnvVars); + setMcpServers(nextMcpServers); setShowAdvancedFields( nextNamePoolText.trim().length > 0 || Object.keys(nextEnvVars).length > 0 || + nextMcpServers.length > 0 || nextBehaviorDraft.respondTo !== null || nextBehaviorDraft.parallelism.trim().length > 0, ); @@ -282,6 +288,7 @@ export function AgentDefinitionDialog({ provider: providerForSubmit, namePool: namePoolInput, envVars, + mcpServers, behavior: behaviorForSubmit( behaviorDraft, behaviorSeedRef.current, @@ -867,11 +874,14 @@ export function AgentDefinitionDialog({ envVars={envVars} fileSatisfiedEnvKeys={localModeGate.fileSatisfiedEnvKeys} inheritedEnvVars={globalConfig.env_vars} + inheritedMcpServers={globalConfig.mcp_servers} + mcpServers={mcpServers} model={model} modelTuningRuntimeId={runtime} namePoolText={namePoolText} onBehaviorDraftChange={setBehaviorDraft} onEnvVarsChange={setEnvVars} + onMcpServersChange={setMcpServers} onNamePoolTextChange={setNamePoolText} provider={provider} requiredEnvKeys={requiredEnvKeys} diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 52f839431..fe792876b 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -63,6 +63,11 @@ import { } from "./runtimeModelProviderSelection"; import { AgentCreationPreview } from "./AgentCreationPreview"; import type { EnvVarsValue } from "./EnvVarsEditor"; +import { + mergeMcpServersByName, + serversEqual, + type McpServersValue, +} from "./McpServersEditor"; import { useRequiredCredentialState } from "./useRequiredCredentialState"; import { CreateAgentRespondToField } from "./RespondToField"; import { PersonaDropdownField } from "./PersonaDropdownField"; @@ -125,6 +130,9 @@ export function AgentInstanceEditDialog({ const [isCustomProviderEditing, setIsCustomProviderEditing] = React.useState(false); const [envVars, setEnvVars] = React.useState(agent.envVars); + const [mcpServers, setMcpServers] = React.useState( + agent.mcpServers, + ); const [autoRestartOnConfigChange, setAutoRestartOnConfigChange] = React.useState(agent.autoRestartOnConfigChange); const personasQuery = usePersonasQuery(); @@ -175,6 +183,7 @@ export function AgentInstanceEditDialog({ setProvider(agent.provider ?? ""); setIsCustomProviderEditing(false); setEnvVars(agent.envVars); + setMcpServers(agent.mcpServers); setAutoRestartOnConfigChange(agent.autoRestartOnConfigChange); setRespondTo(agent.respondTo); setRespondToAllowlist(agent.respondToAllowlist); @@ -424,6 +433,17 @@ export function AgentInstanceEditDialog({ return { ...globalConfig.env_vars, ...inheritedEnvVars }; }, [globalConfig.env_vars, inheritedEnvVars]); + // Merge global + persona MCP servers for the inherited display hint in + // McpServersEditor. Persona-layer servers override same-named global ones. + const inheritedMcpServers = React.useMemo( + () => + mergeMcpServersByName( + globalConfig.mcp_servers, + linkedPersona?.mcpServers ?? [], + ), + [globalConfig.mcp_servers, linkedPersona?.mcpServers], + ); + // Auto-expand Advanced whenever the prospective runtime is buzz-agent so the // model-tuning knobs are reachable even when no required key is missing. // Fires once per dialog-open cycle; does not re-open if the user manually @@ -684,6 +704,9 @@ export function AgentInstanceEditDialog({ envVars: envVarsChanged(submitEnvVars, agent.envVars) ? submitEnvVars : undefined, + mcpServers: serversEqual(mcpServers, agent.mcpServers) + ? undefined + : mcpServers, respondTo: respondTo !== agent.respondTo ? respondTo : undefined, // The allowlist is preserved across mode toggles in local UI state // (so a user can flip away from allowlist and back without losing @@ -1081,8 +1104,10 @@ export function AgentInstanceEditDialog({ : undefined } inheritedEnvVars={inheritedWithGlobal} + inheritedMcpServers={inheritedMcpServers} inheritHarness={inheritHarness} linkedPersona={linkedPersona} + mcpServers={mcpServers} model={model} modelTuningRuntimeId={prospectiveRuntimeId} parallelism={parallelism} @@ -1097,6 +1122,7 @@ export function AgentInstanceEditDialog({ onAutoRestartChange={setAutoRestartOnConfigChange} onEnvVarsChange={setEnvVars} onInheritHarnessChange={setInheritHarness} + onMcpServersChange={setMcpServers} onParallelismChange={setParallelism} onRelayUrlChange={setRelayUrl} onSystemPromptChange={setSystemPrompt} diff --git a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx index 614c44434..2d75aff10 100644 --- a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx @@ -2,6 +2,7 @@ import { cn } from "@/shared/lib/cn"; import { Input } from "@/shared/ui/input"; import { Textarea } from "@/shared/ui/textarea"; import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor"; +import { McpServersEditor, type McpServersValue } from "./McpServersEditor"; import { PERSONA_FIELD_CONTROL_CLASS, PERSONA_FIELD_SHELL_CLASS, @@ -21,8 +22,10 @@ export function EditAgentAdvancedFields({ fileSatisfiedEnvKeys, focusKey, inheritedEnvVars, + inheritedMcpServers = [], inheritHarness, linkedPersona, + mcpServers, model, modelTuningRuntimeId, parallelism, @@ -36,6 +39,7 @@ export function EditAgentAdvancedFields({ onAgentCommandChange, onEnvVarsChange, onInheritHarnessChange, + onMcpServersChange, onParallelismChange, onRelayUrlChange, onAutoRestartChange, @@ -51,7 +55,11 @@ export function EditAgentAdvancedFields({ /** When set, EnvVarsEditor scrolls and focuses this key's input on mount. */ focusKey?: string; inheritedEnvVars: Record; + /** Read-only MCP servers inherited from the global + persona layers, merged. */ + inheritedMcpServers?: McpServersValue; inheritHarness: boolean; + /** This agent instance's own MCP server layer. */ + mcpServers: McpServersValue; linkedPersona: AgentPersona | null; /** Active LLM model — forwarded to BuzzAgentModelTuningFields for effort filtering. */ model?: string; @@ -73,6 +81,7 @@ export function EditAgentAdvancedFields({ onAgentCommandChange: (value: string) => void; onEnvVarsChange: (value: EnvVarsValue) => void; onInheritHarnessChange: (value: boolean) => void; + onMcpServersChange: (value: McpServersValue) => void; onParallelismChange: (value: string) => void; onRelayUrlChange: (value: string) => void; onAutoRestartChange: (value: boolean) => void; @@ -312,6 +321,17 @@ export function EditAgentAdvancedFields({ value={envVars} /> + {/* MCP servers */} + + {/* Tier-1 buzz-agent model-tuning knobs — only shown for buzz-agent. */} {isBuzzAgentRuntime(modelTuningRuntimeId) ? ( ) { const effort = config.env_vars[BUZZ_AGENT_THINKING_EFFORT]; const merged = @@ -283,6 +292,16 @@ export function GlobalAgentConfigFields({ )} /> + + {/* MCP servers */} +
+ +
); } diff --git a/desktop/src/features/agents/ui/McpServersEditor.test.mjs b/desktop/src/features/agents/ui/McpServersEditor.test.mjs new file mode 100644 index 000000000..3cdcfc440 --- /dev/null +++ b/desktop/src/features/agents/ui/McpServersEditor.test.mjs @@ -0,0 +1,566 @@ +/** + * Unit tests for the McpServersEditor state helpers. + * + * Tests four invariants: + * + * 1. validateMcpServerName / validateMcpServerRow mirror the Rust + * `validate_mcp_servers` grammar (name charset, length, `__`, + * reserved name, uniqueness, command-required-when-enabled). + * 2. toRows / toServers round-trip a server list, and toServers skips + * empty-name rows and blank arg/env rows (no phantom empty args or + * env entries reach the emitted config). + * 3. serversEqual detects the resync-guard transitions the effect relies + * on (reordering, field-level diffs, arg/env diffs). + * 4. mergeMcpServersByName replaces whole entries by name across layers, + * in the same order the Rust `merge_mcp_servers` BTreeMap produces. + * + * These are pure-logic tests — no React renderer needed. + */ + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + validateMcpServerName, + validateMcpServerRow, + validateMcpServerArg, + validateMcpServerEnvEntry, + validateMcpServerListPayload, + toRows, + toServers, + serversEqual, + mergeMcpServersByName, + effectiveEnabledCount, + MAX_USER_MCP_SERVERS, + MAX_ENV_VALUE_BYTES, + MAX_ENV_TOTAL_BYTES, + MCP_SERVER_NAME_MAX_LEN, + MCP_RESERVED_SERVER_NAME, + RESERVED_ENV_KEYS, +} from "./McpServersEditor.tsx"; + +// ── Invariant 1: name grammar mirrors validate_mcp_servers ───────────────── + +test("validateMcpServerName_empty_name_is_required", () => { + assert.equal(validateMcpServerName(""), "Name is required."); +}); + +test("validateMcpServerName_valid_name_returns_null", () => { + assert.equal(validateMcpServerName("my-server_1"), null); +}); + +test("validateMcpServerName_over_max_length_rejected", () => { + const tooLong = "a".repeat(MCP_SERVER_NAME_MAX_LEN + 1); + const error = validateMcpServerName(tooLong); + assert.match(error, /exceeds the maximum length/); +}); + +test("validateMcpServerName_at_max_length_accepted", () => { + const atMax = "a".repeat(MCP_SERVER_NAME_MAX_LEN); + assert.equal(validateMcpServerName(atMax), null); +}); + +test("validateMcpServerName_rejects_non_ascii_alnum_characters", () => { + // Rust grammar: ASCII alphanumeric, `_`, `-` only. Space, dot, and + // non-ASCII letters must all be rejected. + assert.match(validateMcpServerName("my server"), /Only letters/); + assert.match(validateMcpServerName("my.server"), /Only letters/); + assert.match(validateMcpServerName("café"), /Only letters/); +}); + +test("validateMcpServerName_rejects_double_underscore", () => { + assert.match(validateMcpServerName("my__server"), /cannot contain/); +}); + +test("validateMcpServerName_rejects_reserved_name", () => { + const error = validateMcpServerName(MCP_RESERVED_SERVER_NAME); + assert.match(error, /is reserved/); +}); + +test("validateMcpServerRow_flags_duplicate_name_in_layer", () => { + const row = { name: "shared", command: "npx", enabled: true }; + const error = validateMcpServerRow(row, new Set(["shared"])); + assert.equal(error, "Server names must be unique."); +}); + +test("validateMcpServerRow_enabled_without_command_is_rejected", () => { + const row = { name: "srv", command: "", enabled: true }; + const error = validateMcpServerRow(row, new Set()); + assert.equal(error, "Command is required for an enabled server."); +}); + +test("validateMcpServerRow_disabled_without_command_is_valid", () => { + // A disabled row's only purpose may be to mask a lower-precedence + // server by name, so it may omit command (mirrors Rust: the + // command-required check is gated on `server.enabled`). + const row = { name: "srv", command: "", enabled: false }; + assert.equal(validateMcpServerRow(row, new Set()), null); +}); + +test("validateMcpServerRow_valid_enabled_row_returns_null", () => { + const row = { name: "srv", command: "npx", enabled: true }; + assert.equal(validateMcpServerRow(row, new Set(["other"])), null); +}); + +// ── Invariant 2: toRows / toServers round-trip and blank-row skipping ────── + +test("toRows_produces_one_row_per_server_with_stable_shape", () => { + const servers = [ + { + name: "fs", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem"], + env: [{ name: "ROOT", value: "/tmp" }], + enabled: true, + }, + ]; + const rows = toRows(servers); + assert.equal(rows.length, 1); + assert.equal(rows[0].name, "fs"); + assert.equal(rows[0].command, "npx"); + assert.deepEqual( + rows[0].args.map((a) => a.value), + ["-y", "@modelcontextprotocol/server-filesystem"], + ); + assert.deepEqual( + rows[0].env.map((e) => [e.name, e.value]), + [["ROOT", "/tmp"]], + ); + assert.equal(rows[0].enabled, true); + assert.equal(typeof rows[0].id, "string"); +}); + +test("toServers_round_trips_toRows_output", () => { + const servers = [ + { + name: "fs", + command: "npx", + args: ["-y", "server"], + env: [{ name: "ROOT", value: "/tmp" }], + enabled: true, + }, + { name: "masked", command: "", args: [], env: [], enabled: false }, + ]; + const rows = toRows(servers); + assert.ok(serversEqual(toServers(rows), servers)); +}); + +test("toServers_skips_empty_name_rows", () => { + // A freshly-added row with no name typed yet must not become a + // phantom entry in the emitted config. + const rows = toRows([]); + rows.push({ + id: "new", + name: "", + command: "npx", + args: [], + env: [], + enabled: true, + }); + assert.deepEqual(toServers(rows), []); +}); + +test("toServers_skips_blank_arg_rows", () => { + // An unfilled "Add argument" row must not inject an empty-string arg + // into the spawned command. + const rows = toRows([ + { name: "srv", command: "npx", args: ["-y"], env: [], enabled: true }, + ]); + rows[0].args.push({ id: "blank", value: "" }); + const [server] = toServers(rows); + assert.deepEqual(server.args, ["-y"]); +}); + +test("toServers_skips_blank_env_rows_but_keeps_named_ones", () => { + const rows = toRows([ + { name: "srv", command: "npx", args: [], env: [], enabled: true }, + ]); + rows[0].env.push({ id: "blank", name: "", value: "leftover" }); + rows[0].env.push({ id: "named", name: "TOKEN", value: "abc" }); + const [server] = toServers(rows); + assert.deepEqual(server.env, [{ name: "TOKEN", value: "abc" }]); +}); + +test("toServers_duplicate_env_names_collapse_last_write_wins", () => { + // The backend's per-server env is a Vec, not a map, so it rejects + // duplicate names outright (`mcp_servers.rs` "has duplicate env var"). + // The editor must dedupe client-side before that round-trip, the same + // way EnvVarsEditor's object-keyed `toRecord` does implicitly. + const rows = toRows([ + { name: "srv", command: "npx", args: [], env: [], enabled: true }, + ]); + rows[0].env.push({ id: "a", name: "TOKEN", value: "first" }); + rows[0].env.push({ id: "b", name: "TOKEN", value: "second" }); + const [server] = toServers(rows); + assert.deepEqual(server.env, [{ name: "TOKEN", value: "second" }]); +}); + +// ── Invariant 3: serversEqual — resync-guard transitions ─────────────────── + +test("serversEqual_identical_lists_are_equal", () => { + const servers = [ + { name: "a", command: "npx", args: ["-y"], env: [], enabled: true }, + ]; + assert.ok(serversEqual(servers, structuredClone(servers))); +}); + +test("serversEqual_different_lengths_are_not_equal", () => { + const a = [{ name: "a", command: "npx", args: [], env: [], enabled: true }]; + assert.equal(serversEqual(a, []), false); +}); + +test("serversEqual_detects_arg_diff", () => { + const a = [ + { name: "a", command: "npx", args: ["-y"], env: [], enabled: true }, + ]; + const b = [ + { name: "a", command: "npx", args: ["-x"], env: [], enabled: true }, + ]; + assert.equal(serversEqual(a, b), false); +}); + +test("serversEqual_detects_env_diff", () => { + const a = [ + { + name: "a", + command: "npx", + args: [], + env: [{ name: "K", value: "1" }], + enabled: true, + }, + ]; + const b = [ + { + name: "a", + command: "npx", + args: [], + env: [{ name: "K", value: "2" }], + enabled: true, + }, + ]; + assert.equal(serversEqual(a, b), false); +}); + +test("serversEqual_detects_enabled_diff", () => { + const a = [{ name: "a", command: "npx", args: [], env: [], enabled: true }]; + const b = [{ name: "a", command: "npx", args: [], env: [], enabled: false }]; + assert.equal(serversEqual(a, b), false); +}); + +// ── Invariant 4: mergeMcpServersByName — layered replace-by-name ────────── + +test("mergeMcpServersByName_higher_layer_replaces_whole_entry", () => { + const global = [ + { + name: "fs", + command: "npx", + args: ["-y", "global-pkg"], + env: [], + enabled: true, + }, + ]; + const agent = [ + { + name: "fs", + command: "npx", + args: ["-y", "agent-pkg"], + env: [], + enabled: true, + }, + ]; + const merged = mergeMcpServersByName(global, agent); + assert.equal(merged.length, 1); + assert.deepEqual(merged[0].args, ["-y", "agent-pkg"]); +}); + +test("mergeMcpServersByName_disabled_higher_layer_masks_lower_layer", () => { + const global = [ + { name: "fs", command: "npx", args: ["-y"], env: [], enabled: true }, + ]; + const definition = [ + { name: "fs", command: "", args: [], env: [], enabled: false }, + ]; + const merged = mergeMcpServersByName(global, definition); + assert.equal(merged.length, 1); + assert.equal( + merged[0].enabled, + false, + "disabled override must mask, not disappear", + ); +}); + +test("mergeMcpServersByName_distinct_names_from_all_layers_are_kept", () => { + const global = [ + { name: "g", command: "npx", args: [], env: [], enabled: true }, + ]; + const definition = [ + { name: "d", command: "npx", args: [], env: [], enabled: true }, + ]; + const agent = [ + { name: "a", command: "npx", args: [], env: [], enabled: true }, + ]; + const merged = mergeMcpServersByName(global, definition, agent); + assert.deepEqual( + merged.map((s) => s.name), + ["a", "d", "g"], + ); +}); + +test("mergeMcpServersByName_empty_layers_produce_empty_result", () => { + assert.deepEqual(mergeMcpServersByName([], [], []), []); +}); + +test("mergeMcpServersByName_sorts_by_ascii_byte_order_not_locale", () => { + // Rust BTreeMap orders by byte value: '_' (0x5F) > 'Z' (0x5A) + // > 'A' (0x41), and uppercase < lowercase. localeCompare would put + // these in a different order (e.g. case-insensitive or locale-dependent). + const servers = [ + { name: "beta", command: "", args: [], env: [], enabled: true }, + { name: "Alpha", command: "", args: [], env: [], enabled: true }, + { name: "_underscore", command: "", args: [], env: [], enabled: true }, + { name: "alpha", command: "", args: [], env: [], enabled: true }, + ]; + const merged = mergeMcpServersByName(servers); + // ASCII: 'A'(65) < '_'(95) < 'a'(97) < 'b'(98) + assert.deepEqual( + merged.map((s) => s.name), + ["Alpha", "_underscore", "alpha", "beta"], + ); +}); + +// ── Invariant 5: effectiveEnabledCount — cap across layers ────────────── + +function makeServer(name, enabled = true) { + return { name, command: "npx", args: [], env: [], enabled }; +} + +test("effectiveEnabledCount_local_only", () => { + const local = [makeServer("a"), makeServer("b", false)]; + assert.equal(effectiveEnabledCount(local, []), 1); +}); + +test("effectiveEnabledCount_inherited_not_overridden", () => { + const local = [makeServer("a")]; + const inherited = [makeServer("b"), makeServer("c")]; + assert.equal(effectiveEnabledCount(local, inherited), 3); +}); + +test("effectiveEnabledCount_inherited_overridden_by_local_not_counted", () => { + // Local row overrides inherited — only the local row's enabled state + // counts, not the inherited one. + const local = [makeServer("a", false)]; + const inherited = [makeServer("a"), makeServer("b")]; + // "a" overridden (local disabled → 0), "b" inherited enabled → 1 + assert.equal(effectiveEnabledCount(local, inherited), 1); +}); + +test("effectiveEnabledCount_disabled_inherited_not_counted", () => { + const local = []; + const inherited = [makeServer("a", false), makeServer("b")]; + assert.equal(effectiveEnabledCount(local, inherited), 1); +}); + +test("effectiveEnabledCount_15_inherited_plus_1_local_hits_cap", () => { + const inherited = Array.from({ length: MAX_USER_MCP_SERVERS }, (_, i) => + makeServer(`inh-${i}`), + ); + const local = [makeServer("local-1")]; + assert.equal( + effectiveEnabledCount(local, inherited), + MAX_USER_MCP_SERVERS + 1, + ); +}); + +test("effectiveEnabledCount_toggling_disabled_to_16th_hits_cap", () => { + // Simulates toggling a disabled local row to enabled when 14 inherited + + // 1 other local are already enabled = 15. The 16th would exceed the cap. + const inherited = Array.from({ length: 14 }, (_, i) => + makeServer(`inh-${i}`), + ); + const local = [makeServer("local-1"), makeServer("local-2", false)]; + // Before toggle: 1 local enabled + 14 inherited = 15 (at cap) + assert.equal(effectiveEnabledCount(local, inherited), MAX_USER_MCP_SERVERS); + // After toggle: 2 local enabled + 14 inherited = 16 (exceeds cap) + const toggled = local.map((s) => + s.name === "local-2" ? { ...s, enabled: true } : s, + ); + assert.equal( + effectiveEnabledCount(toggled, inherited), + MAX_USER_MCP_SERVERS + 1, + ); +}); + +// ── Invariant 6: validateMcpServerEnvEntry — per-server env boundary ──── + +test("validateMcpServerEnvEntry_valid_entry_returns_null", () => { + assert.equal( + validateMcpServerEnvEntry({ name: "MY_TOKEN", value: "abc" }), + null, + ); +}); + +test("validateMcpServerEnvEntry_empty_name_returns_null_skipped", () => { + // Blank rows are skipped by toServers, so no error shown. + assert.equal( + validateMcpServerEnvEntry({ name: "", value: "anything" }), + null, + ); +}); + +test("validateMcpServerEnvEntry_malformed_key_rejected", () => { + const err = validateMcpServerEnvEntry({ name: "1BAD", value: "" }); + assert.match(err, /must match/); +}); + +test("validateMcpServerEnvEntry_key_with_equals_rejected", () => { + const err = validateMcpServerEnvEntry({ name: "KEY=val", value: "" }); + assert.match(err, /must match/); +}); + +test("validateMcpServerEnvEntry_key_with_spaces_rejected", () => { + const err = validateMcpServerEnvEntry({ name: "MY KEY", value: "" }); + assert.match(err, /must match/); +}); + +test("validateMcpServerEnvEntry_reserved_key_rejected", () => { + const err = validateMcpServerEnvEntry({ + name: "BUZZ_ACP_MCP_SERVERS", + value: "", + }); + assert.match(err, /reserved/); +}); + +test("validateMcpServerEnvEntry_reserved_key_case_insensitive", () => { + const err = validateMcpServerEnvEntry({ + name: "buzz_private_key", + value: "", + }); + assert.match(err, /reserved/); +}); + +test("validateMcpServerEnvEntry_nul_in_value_rejected", () => { + const err = validateMcpServerEnvEntry({ name: "TOKEN", value: "abc\0def" }); + assert.match(err, /NUL/); +}); + +test("validateMcpServerEnvEntry_oversized_value_rejected", () => { + const err = validateMcpServerEnvEntry({ + name: "TOKEN", + value: "x".repeat(MAX_ENV_VALUE_BYTES + 1), + }); + assert.match(err, /exceeds/); +}); + +test("validateMcpServerEnvEntry_at_limit_value_accepted", () => { + assert.equal( + validateMcpServerEnvEntry({ + name: "TOKEN", + value: "x".repeat(MAX_ENV_VALUE_BYTES), + }), + null, + ); +}); + +test("validateMcpServerEnvEntry_all_reserved_keys_are_rejected", () => { + for (const key of RESERVED_ENV_KEYS) { + const err = validateMcpServerEnvEntry({ name: key, value: "" }); + assert.ok(err, `Expected "${key}" to be rejected as reserved`); + } +}); + +// ── Invariant 7: validateMcpServerRow — command NUL + size ────────────── + +test("validateMcpServerRow_command_nul_rejected", () => { + const err = validateMcpServerRow( + { name: "srv", command: "npx\0bad", enabled: true }, + new Set(), + ); + assert.match(err, /NUL/); +}); + +test("validateMcpServerRow_command_oversize_rejected", () => { + const err = validateMcpServerRow( + { + name: "srv", + command: "x".repeat(MAX_ENV_VALUE_BYTES + 1), + enabled: true, + }, + new Set(), + ); + assert.match(err, /exceeds/); +}); + +test("validateMcpServerRow_command_at_limit_accepted", () => { + assert.equal( + validateMcpServerRow( + { name: "srv", command: "x".repeat(MAX_ENV_VALUE_BYTES), enabled: true }, + new Set(), + ), + null, + ); +}); + +// ── Invariant 8: validateMcpServerArg — arg NUL + size ────────────────── + +test("validateMcpServerArg_nul_rejected", () => { + assert.match(validateMcpServerArg("arg\0val"), /NUL/); +}); + +test("validateMcpServerArg_oversize_rejected", () => { + assert.match( + validateMcpServerArg("x".repeat(MAX_ENV_VALUE_BYTES + 1)), + /exceeds/, + ); +}); + +test("validateMcpServerArg_valid_accepted", () => { + assert.equal(validateMcpServerArg("--flag"), null); +}); + +// ── Invariant 9: validateMcpServerListPayload — aggregate cap ─────────── + +test("validateMcpServerListPayload_under_limit_accepted", () => { + assert.equal( + validateMcpServerListPayload([ + { name: "srv", command: "npx", args: ["-y"], env: [], enabled: true }, + ]), + null, + ); +}); + +test("validateMcpServerListPayload_over_limit_rejected", () => { + // Nine servers, each with an env value just under 32KB → total > 256KB. + const servers = Array.from({ length: 9 }, (_, i) => ({ + name: `srv-${i}`, + command: "npx", + args: [], + env: [{ name: "DATA", value: "x".repeat(MAX_ENV_VALUE_BYTES - 1) }], + enabled: true, + })); + const err = validateMcpServerListPayload(servers); + assert.ok(err, "aggregate payload should be rejected"); + assert.match(err, /limit/); +}); + +test("validateMcpServerListPayload_at_limit_accepted", () => { + // A single server whose total payload is exactly at the limit. + // Total = name + command + env-key + env-value bytes. + const budget = MAX_ENV_TOTAL_BYTES; + const name = "s"; + const command = "c"; + const envKey = "K"; + const overhead = name.length + command.length + envKey.length; + const value = "x".repeat(budget - overhead); + assert.equal( + validateMcpServerListPayload([ + { + name, + command, + args: [], + env: [{ name: envKey, value }], + enabled: true, + }, + ]), + null, + ); +}); diff --git a/desktop/src/features/agents/ui/McpServersEditor.tsx b/desktop/src/features/agents/ui/McpServersEditor.tsx new file mode 100644 index 000000000..2e6b7acc8 --- /dev/null +++ b/desktop/src/features/agents/ui/McpServersEditor.tsx @@ -0,0 +1,896 @@ +/** + * Editor for the local-only buzz-agent MCP server layer (Q1/Q3, PLANS/PR3). + * + * Modeled on `EnvVarsEditor`: an ordered row list keyed by stable ids (so + * duplicate/empty names don't collapse mid-edit), inherited-layer read-only + * rows that a same-named local row overrides or masks, and pure emit/resync + * helpers exported for unit testing. + * + * STDIO only — no SSE/HTTP, no timeout, no headers (the backend doesn't + * support them). Command and Args are separate fields (Q1): no combined + * command string, no shell-quote dependency. + */ +import { + AlertCircle, + CheckCircle2, + CircleSlash, + Lock, + Plus, + X, +} from "lucide-react"; +import * as React from "react"; + +import type { McpServerConfig } from "@/shared/api/types"; +import { Button } from "@/shared/ui/button"; +import { Input } from "@/shared/ui/input"; +import { Switch } from "@/shared/ui/switch"; +import { cn } from "@/shared/lib/cn"; +import { + PERSONA_FIELD_CONTROL_CLASS, + PERSONA_FIELD_SHELL_CLASS, +} from "./personaDialogPickers"; + +export type McpServersValue = McpServerConfig[]; + +/** Max byte length of a server name. Mirrors `MAX_SERVER_NAME_LEN` in `mcp_servers.rs`. */ +export const MCP_SERVER_NAME_MAX_LEN = 128; +/** The bundled sidecar's reserved name. Mirrors `RESERVED_SERVER_NAME`. */ +export const MCP_RESERVED_SERVER_NAME = "buzz-dev-mcp"; +/** Effective/per-layer enabled-server cap. Mirrors `MAX_USER_MCP_SERVERS`. */ +export const MAX_USER_MCP_SERVERS = 15; + +/** Per-value byte cap. Mirrors `MAX_ENV_VALUE_BYTES` in `env_vars.rs`. */ +export const MAX_ENV_VALUE_BYTES = 32 * 1024; + +/** Aggregate payload cap across all servers. Mirrors `MAX_ENV_TOTAL_BYTES`. */ +export const MAX_ENV_TOTAL_BYTES = 256 * 1024; + +/** + * Reserved env keys (case-insensitive). Mirrors `RESERVED_ENV_KEYS` in + * `env_vars.rs`. Keys that override agent identity, code-execution surface, + * security gates, or structured transport must never be user-settable. + */ +export const RESERVED_ENV_KEYS: readonly string[] = [ + "BUZZ_PRIVATE_KEY", + "NOSTR_PRIVATE_KEY", + "BUZZ_AUTH_TAG", + "BUZZ_API_TOKEN", + "BUZZ_ACP_PRIVATE_KEY", + "BUZZ_ACP_API_TOKEN", + "BUZZ_RELAY_URL", + "BUZZ_ACP_AGENT_COMMAND", + "BUZZ_ACP_AGENT_ARGS", + "BUZZ_ACP_MCP_COMMAND", + "BUZZ_ACP_MCP_SERVERS", + "BUZZ_ACP_RESPOND_TO", + "BUZZ_ACP_RESPOND_TO_ALLOWLIST", + "BUZZ_ACP_AGENT_OWNER", + "BUZZ_ACP_SETUP_PAYLOAD", +]; + +const RESERVED_ENV_KEYS_UPPER = new Set( + RESERVED_ENV_KEYS.map((k) => k.toUpperCase()), +); + +const NAME_CHARS_RE = /^[A-Za-z0-9_-]*$/; +/** POSIX env key: `[A-Za-z_][A-Za-z0-9_]*`. Mirrors `is_well_formed_env_key`. */ +const ENV_KEY_RE = /^[A-Za-z_][A-Za-z0-9_]*$/; + +/** + * Client-side mirror of the Rust `validate_mcp_servers` name grammar, for + * inline feedback before a save round-trip. Returns `null` when valid. + * Exported for unit tests. + */ +export function validateMcpServerName(name: string): string | null { + if (name.length === 0) return "Name is required."; + if (name.length > MCP_SERVER_NAME_MAX_LEN) { + return `Name exceeds the maximum length (${MCP_SERVER_NAME_MAX_LEN} bytes).`; + } + if (!NAME_CHARS_RE.test(name)) { + return "Only letters, numbers, `_`, and `-` are allowed."; + } + if (name.includes("__")) return "Name cannot contain `__`."; + if (name === MCP_RESERVED_SERVER_NAME) { + return `"${MCP_RESERVED_SERVER_NAME}" is reserved.`; + } + return null; +} + +/** + * Full per-row validation: name grammar + uniqueness within the layer + + * command-required-when-enabled + command NUL/size. `otherNames` is every + * OTHER row's name in the same layer (excluding this row), so a collision + * flags both rows. Exported for unit tests. + */ +export function validateMcpServerRow( + row: { name: string; command: string; enabled: boolean }, + otherNames: ReadonlySet, +): string | null { + const nameError = validateMcpServerName(row.name); + if (nameError) return nameError; + if (otherNames.has(row.name)) return "Server names must be unique."; + if (row.enabled && row.command.trim().length === 0) { + return "Command is required for an enabled server."; + } + if (row.command.includes("\0")) { + return "Command cannot contain NUL bytes."; + } + if ( + row.command.length > 0 && + new TextEncoder().encode(row.command).length > MAX_ENV_VALUE_BYTES + ) { + return `Command exceeds the ${MAX_ENV_VALUE_BYTES}-byte limit.`; + } + return null; +} + +/** + * Validate a single arg value. Mirrors `validate_mcp_servers`'s per-arg + * NUL + size check (`mcp_servers.rs:141-157`). Returns `null` when valid. + * Exported for unit tests. + */ +export function validateMcpServerArg(value: string): string | null { + if (value.includes("\0")) return "Argument cannot contain NUL bytes."; + if (new TextEncoder().encode(value).length > MAX_ENV_VALUE_BYTES) { + return `Argument exceeds the ${MAX_ENV_VALUE_BYTES}-byte limit.`; + } + return null; +} + +/** + * Validate aggregate payload across all servers. Mirrors the Rust + * `validate_mcp_servers` total-payload cap (`mcp_servers.rs:186-190`): + * sum of name+command+args+env-key+env-value bytes must stay under + * `MAX_ENV_TOTAL_BYTES`. Returns `null` when valid. Exported for unit tests. + */ +export function validateMcpServerListPayload( + servers: McpServersValue, +): string | null { + const encoder = new TextEncoder(); + let total = 0; + for (const server of servers) { + total += encoder.encode(server.name).length; + if (server.command.length > 0) { + total += encoder.encode(server.command).length; + } + for (const arg of server.args) { + total += encoder.encode(arg).length; + } + for (const entry of server.env) { + total += encoder.encode(entry.name).length; + total += encoder.encode(entry.value).length; + } + } + if (total > MAX_ENV_TOTAL_BYTES) { + return `Total MCP server payload is ${total} bytes; limit is ${MAX_ENV_TOTAL_BYTES}.`; + } + return null; +} + +/** + * Validate a single per-server env var entry. Mirrors the Rust + * `validate_user_env_keys` boundary (`env_vars.rs`): POSIX key format, + * reserved-key check (case-insensitive), NUL-free values, and per-value + * byte cap. Returns `null` when valid. Exported for unit tests. + */ +export function validateMcpServerEnvEntry(entry: { + name: string; + value: string; +}): string | null { + if (entry.name.length === 0) return null; // blank rows are skipped by toServers + if (!ENV_KEY_RE.test(entry.name)) { + return "Key must match [A-Za-z_][A-Za-z0-9_]*."; + } + if (RESERVED_ENV_KEYS_UPPER.has(entry.name.toUpperCase())) { + return `"${entry.name}" is reserved by Buzz.`; + } + if (entry.value.includes("\0")) { + return "Value cannot contain NUL bytes."; + } + if (new TextEncoder().encode(entry.value).length > MAX_ENV_VALUE_BYTES) { + return `Value exceeds the ${MAX_ENV_VALUE_BYTES}-byte limit.`; + } + return null; +} + +type ArgRow = { id: string; value: string }; +type EnvRow = { id: string; name: string; value: string }; +type ServerRow = { + id: string; + name: string; + command: string; + args: ArgRow[]; + env: EnvRow[]; + enabled: boolean; +}; + +/** Build a rows array from a server-config list. Exported for unit tests. */ +export function toRows(servers: McpServersValue): ServerRow[] { + return servers.map((server) => ({ + id: crypto.randomUUID(), + name: server.name, + command: server.command, + args: server.args.map((value) => ({ id: crypto.randomUUID(), value })), + env: server.env.map((entry) => ({ + id: crypto.randomUUID(), + name: entry.name, + value: entry.value, + })), + enabled: server.enabled, + })); +} + +/** + * Collapse rows back to a server-config list. Rows with an empty name are + * mid-edit (the user hasn't identified the server yet) and are skipped, + * mirroring `EnvVarsEditor`'s empty-key skip. Blank arg/env rows are skipped + * too — an unfilled "Add argument" row must never inject an empty-string + * arg into the spawned command. Duplicate env var names within a server + * collapse last-write-wins, matching `EnvVarsEditor`'s `toRecord` (its + * object-keyed shape does this automatically; the per-server `env` here is + * an array per the Rust `Vec` shape, so it needs the same + * dedupe applied explicitly to avoid a save-time rejection from the Rust + * validator's duplicate-env-var check). Exported for unit tests. + */ +export function toServers(rows: readonly ServerRow[]): McpServersValue { + const out: McpServerConfig[] = []; + for (const row of rows) { + if (row.name.length === 0) continue; + const env = new Map(); + for (const entry of row.env) { + if (entry.name.length > 0) env.set(entry.name, entry.value); + } + out.push({ + name: row.name, + command: row.command, + args: row.args.map((a) => a.value).filter((v) => v.length > 0), + env: Array.from(env, ([name, value]) => ({ name, value })), + enabled: row.enabled, + }); + } + return out; +} + +/** True iff two server lists are element-wise identical. Exported for unit tests. */ +export function serversEqual(a: McpServersValue, b: McpServersValue): boolean { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + const x = a[i]; + const y = b[i]; + if ( + x.name !== y.name || + x.command !== y.command || + x.enabled !== y.enabled + ) { + return false; + } + if ( + x.args.length !== y.args.length || + x.args.some((v, j) => v !== y.args[j]) + ) { + return false; + } + if (x.env.length !== y.env.length) return false; + for (let j = 0; j < x.env.length; j++) { + if ( + x.env[j].name !== y.env[j].name || + x.env[j].value !== y.env[j].value + ) { + return false; + } + } + } + return true; +} + +/** + * Merge server layers by name — later layers replace earlier entries whole, + * matching the Rust `merge_mcp_servers` replace-by-name rule. Unlike the + * Rust helper this keeps disabled entries and does not enforce the + * effective-server cap: it is a DISPLAY merge for the editor's read-only + * "inherited" rows, not the runtime WYSIWYG surface (that's + * `RuntimeConfigSurface.buzzAgentMcpServers`, computed server-side). Sorted + * by ASCII byte order (`<` / `>`) to match Rust's `BTreeMap` + * ordering. Exported for unit tests and dialog callers. + */ +export function mergeMcpServersByName( + ...layers: readonly McpServersValue[] +): McpServersValue { + const merged = new Map(); + for (const layer of layers) { + for (const server of layer) { + merged.set(server.name, server); + } + } + return Array.from(merged.values()).sort((a, b) => + a.name < b.name ? -1 : a.name > b.name ? 1 : 0, + ); +} + +/** + * Compute the effective enabled count across local and inherited layers. + * Local enabled rows plus inherited enabled rows NOT overridden/masked by + * a local row with the same name. Mirrors the Rust effective-merge cap + * check (`mcp_servers.rs:225-229`). Exported for unit tests. + */ +export function effectiveEnabledCount( + local: McpServersValue, + inherited: McpServersValue, +): number { + const localNames = new Set(local.map((s) => s.name)); + const localEnabled = local.filter((s) => s.enabled).length; + const inheritedEnabled = inherited.filter( + (s) => s.enabled && !localNames.has(s.name), + ).length; + return localEnabled + inheritedEnabled; +} + +/** True when a row has any content beyond a bare name — used to decide + * whether an empty-name row's "Name is required" hint should show (a + * pristine just-added row stays quiet; a row with typed content but no name + * yet warns, so filling in command/args/env doesn't silently go unsaved). */ +function rowHasContent(row: ServerRow): boolean { + return ( + row.command.trim().length > 0 || + row.args.some((a) => a.value.trim().length > 0) || + row.env.some( + (e) => e.name.trim().length > 0 || e.value.trim().length > 0, + ) || + !row.enabled + ); +} + +function commandLine(command: string, args: readonly string[]): string { + return [command, ...args].filter(Boolean).join(" "); +} + +export type McpServersEditorProps = { + /** This layer's own MCP server list. */ + value: McpServersValue; + /** Called with the next list whenever the user edits a row. */ + onChange: (next: McpServersValue) => void; + /** Read-only servers inherited from lower layer(s), already merged + * (see `mergeMcpServersByName`). A local row with a matching name + * overrides (or, if disabled, masks) the inherited entry. */ + inheritedServers?: McpServersValue; + /** Label for the inherited source (e.g. "global" or "global + persona"). */ + inheritedLabel?: string; + /** Section header. Defaults to "MCP servers". */ + label?: string; + /** Short description below the header. */ + helperText?: string; + /** Disables all editing. */ + disabled?: boolean; +}; + +/** + * Per-server row editor: Name / Command / Args (repeatable) / Env + * (repeatable) / Enabled toggle, plus read-only inherited-layer rows. + */ +export function McpServersEditor({ + value, + onChange, + inheritedServers = [], + inheritedLabel = "inherited", + label = "MCP servers", + helperText, + disabled = false, +}: McpServersEditorProps) { + // Defense-in-depth: coerce at the exported boundary so a caller passing + // `undefined` (e.g. a partial test bridge seed) degrades to empty rather + // than crashing inside `toRows`. Production never produces `undefined` + // (Rust `#[serde(default)]` + `EMPTY_CONFIG`), but a single coercion + // here is cheaper than scattering `?? []` across every mount site. + const safeValue = value ?? []; + const [rows, setRows] = React.useState(() => toRows(safeValue)); + const lastEmitted = React.useRef(safeValue); + + // Resync from `value` when the parent supplies a value we did not just + // emit (e.g. dialog reopened against a different persona/agent). Mirrors + // EnvVarsEditor's `recordsEqual` resync guard. + React.useEffect(() => { + if (!serversEqual(lastEmitted.current, safeValue)) { + lastEmitted.current = safeValue; + setRows(toRows(safeValue)); + } + }, [safeValue]); + + function emit(nextRows: ServerRow[]) { + setRows(nextRows); + const servers = toServers(nextRows); + lastEmitted.current = servers; + onChange(servers); + } + + function updateRow(id: string, patch: Partial) { + emit(rows.map((row) => (row.id === id ? { ...row, ...patch } : row))); + } + + function removeRow(id: string) { + emit(rows.filter((row) => row.id !== id)); + } + + function addRow(seed?: Partial>) { + emit([ + ...rows, + { + id: crypto.randomUUID(), + name: seed?.name ?? "", + command: "", + args: [], + env: [], + enabled: seed?.enabled ?? true, + }, + ]); + } + + function addArg(rowId: string) { + const row = rows.find((r) => r.id === rowId); + if (!row) return; + updateRow(rowId, { + args: [...row.args, { id: crypto.randomUUID(), value: "" }], + }); + } + + function updateArg(rowId: string, argId: string, argValue: string) { + const row = rows.find((r) => r.id === rowId); + if (!row) return; + updateRow(rowId, { + args: row.args.map((a) => + a.id === argId ? { ...a, value: argValue } : a, + ), + }); + } + + function removeArg(rowId: string, argId: string) { + const row = rows.find((r) => r.id === rowId); + if (!row) return; + updateRow(rowId, { args: row.args.filter((a) => a.id !== argId) }); + } + + function addEnvRow(rowId: string) { + const row = rows.find((r) => r.id === rowId); + if (!row) return; + updateRow(rowId, { + env: [...row.env, { id: crypto.randomUUID(), name: "", value: "" }], + }); + } + + function updateEnvRow( + rowId: string, + envId: string, + patch: Partial>, + ) { + const row = rows.find((r) => r.id === rowId); + if (!row) return; + updateRow(rowId, { + env: row.env.map((e) => (e.id === envId ? { ...e, ...patch } : e)), + }); + } + + function removeEnvRow(rowId: string, envId: string) { + const row = rows.find((r) => r.id === rowId); + if (!row) return; + updateRow(rowId, { env: row.env.filter((e) => e.id !== envId) }); + } + + const localNames = new Set(rows.map((r) => r.name)); + const visibleInherited = inheritedServers.filter( + (server) => !localNames.has(server.name), + ); + const localEnabledCount = rows.filter((r) => r.enabled).length; + const inheritedEnabledCount = visibleInherited.filter( + (s) => s.enabled, + ).length; + const currentEffective = localEnabledCount + inheritedEnabledCount; + const atCap = currentEffective >= MAX_USER_MCP_SERVERS; + const overCap = currentEffective > MAX_USER_MCP_SERVERS; + const payloadError = validateMcpServerListPayload(toServers(rows)); + + return ( +
+
+
{label}
+ {helperText ? ( +

{helperText}

+ ) : null} +
+
+ {visibleInherited.map((server) => ( + addRow({ name: server.name, enabled: false })} + disabled={disabled} + server={server} + /> + ))} + + {rows.length === 0 && visibleInherited.length === 0 ? ( +

+ No MCP servers configured. +

+ ) : null} + + {rows.map((row) => { + const otherNames = new Set( + rows.filter((r) => r.id !== row.id).map((r) => r.name), + ); + const rowError = validateMcpServerRow(row, otherNames); + const showError = Boolean( + rowError && (row.name.length > 0 || rowHasContent(row)), + ); + const overridesInherited = inheritedServers.find( + (s) => s.name === row.name, + ); + + return ( +
+
+
+ + updateRow(row.id, { name: event.target.value }) + } + placeholder="server-name" + value={row.name} + /> +
+
+ + updateRow(row.id, { command: event.target.value }) + } + placeholder="npx" + value={row.command} + /> +
+ + updateRow(row.id, { enabled: checked }) + } + /> + +
+ + {showError ? ( +

+ + {rowError} +

+ ) : null} + {!showError && overridesInherited ? ( +

+ {row.enabled ? "Overrides" : "Masks"} {inheritedLabel} server{" "} + + {commandLine( + overridesInherited.command, + overridesInherited.args, + ) || "stdio"} + +

+ ) : null} + + {/* Args */} +
+

+ Args +

+ {row.args.map((arg) => { + const argError = validateMcpServerArg(arg.value); + return ( +
+
+
+ + updateArg(row.id, arg.id, event.target.value) + } + value={arg.value} + /> +
+ +
+ {argError ? ( +

+ + {argError} +

+ ) : null} +
+ ); + })} + +
+ + {/* Env */} +
+

+ Environment +

+ {row.env.map((entry) => { + const envError = validateMcpServerEnvEntry(entry); + return ( +
+
+
+ + updateEnvRow(row.id, entry.id, { + name: event.target.value, + }) + } + placeholder="VARIABLE_NAME" + value={entry.name} + /> +
+
+ + updateEnvRow(row.id, entry.id, { + value: event.target.value, + }) + } + placeholder="value" + value={entry.value} + /> +
+ +
+ {envError ? ( +

+ + {envError} +

+ ) : null} +
+ ); + })} + +
+
+ ); + })} + +
+ + {overCap ? ( +

+ + {currentEffective} enabled servers exceed the{" "} + {MAX_USER_MCP_SERVERS}-server limit (including inherited). Save + will be rejected. +

+ ) : atCap ? ( +

+ {MAX_USER_MCP_SERVERS}-server limit reached (including inherited). +

+ ) : null} + {payloadError ? ( +

+ + {payloadError} +

+ ) : null} +
+
+
+ ); +} + +function InheritedServerRow({ + disabled, + inheritedLabel, + onMask, + server, +}: { + disabled: boolean; + inheritedLabel: string; + onMask: () => void; + server: McpServerConfig; +}) { + const StatusIcon = server.enabled ? CheckCircle2 : CircleSlash; + const line = commandLine(server.command, server.args); + + return ( +
+
+ + + + + {server.name} + + + Inherited from {inheritedLabel} + + {line ? ( + + {line} + + ) : null} + +
+ +
+ ); +} diff --git a/desktop/src/features/agents/ui/McpServersSection.tsx b/desktop/src/features/agents/ui/McpServersSection.tsx index 3e6c93ca5..f20beda4f 100644 --- a/desktop/src/features/agents/ui/McpServersSection.tsx +++ b/desktop/src/features/agents/ui/McpServersSection.tsx @@ -1,6 +1,6 @@ import type * as React from "react"; import { CheckCircle2, CircleSlash } from "lucide-react"; -import type { ExtensionEntry } from "@/shared/api/types"; +import type { ExtensionEntry, McpServerConfig } from "@/shared/api/types"; import { cn } from "@/shared/lib/cn"; type McpServersSectionProps = { @@ -38,9 +38,18 @@ export function McpServersSection({ MCP Servers

- {isBuzzAgent && buzzAgentSlot ? buzzAgentSlot : null} - - {extensions.length > 0 ? ( + {isBuzzAgent ? ( + (buzzAgentSlot ?? ( +

+ No custom servers configured +

+ )) + ) : extensions.length > 0 ? (
{extensions.map((extension) => ( ); } + +/** + * Read-only row for an *effective merged* buzz-agent MCP server — "what + * runs." Shows the resolved command line rather than `kind`/enabled state + * (unlike `McpServerRow`), since every entry here is already enabled by + * contract (`RuntimeConfigSurface.buzzAgentMcpServers` is enabled-only). + */ +export function BuzzAgentMcpServerRow({ + server, + variant, +}: { + server: McpServerConfig; + variant: "compact" | "profile"; +}) { + const StatusIcon = server.enabled ? CheckCircle2 : CircleSlash; + const commandLine = [server.command, ...server.args] + .filter(Boolean) + .join(" "); + + return ( +
+ + + + + + {server.name} + + + {commandLine || "stdio"} + + +
+ ); +} diff --git a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx index 113e5e5a7..47a235208 100644 --- a/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/PersonaAdvancedFields.tsx @@ -1,6 +1,7 @@ import { Input } from "@/shared/ui/input"; import { cn } from "@/shared/lib/cn"; import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor"; +import { McpServersEditor, type McpServersValue } from "./McpServersEditor"; import { CreateAgentRespondToField } from "./RespondToField"; import type { PersonaBehaviorDraft } from "./personaBehaviorDraft"; import { isBuzzAgentRuntime } from "./buzzAgentConfig"; @@ -16,11 +17,14 @@ export function PersonaAdvancedFields({ disabled, envVars, inheritedEnvVars = {}, + mcpServers, + inheritedMcpServers = [], model, modelTuningRuntimeId = "", namePoolText, onBehaviorDraftChange, onEnvVarsChange, + onMcpServersChange, onNamePoolTextChange, provider, requiredEnvKeys = [], @@ -32,6 +36,10 @@ export function PersonaAdvancedFields({ /** Env vars to display as inherited defaults in tuning-field placeholders. * For templates, pass `globalConfig.env_vars` (the fallback layer). */ inheritedEnvVars?: EnvVarsValue; + /** This definition's local-only MCP server layer. */ + mcpServers: McpServersValue; + /** Read-only servers inherited from the global layer, already merged. */ + inheritedMcpServers?: McpServersValue; /** Active LLM model — forwarded to BuzzAgentModelTuningFields for effort filtering. */ model?: string; /** Runtime id for the buzz-agent tuning knobs visibility gate. */ @@ -39,6 +47,7 @@ export function PersonaAdvancedFields({ namePoolText: string; onBehaviorDraftChange: (value: PersonaBehaviorDraft) => void; onEnvVarsChange: (value: EnvVarsValue) => void; + onMcpServersChange: (value: McpServersValue) => void; onNamePoolTextChange: (value: string) => void; /** Active LLM provider id — forwarded to BuzzAgentModelTuningFields for effort filtering. */ provider?: string; @@ -144,6 +153,16 @@ export function PersonaAdvancedFields({ value={envVars} /> + + {/* Tier-1 buzz-agent model-tuning knobs — only shown for buzz-agent. */} {isBuzzAgentRuntime(modelTuningRuntimeId) ? ( ; + mcp_servers?: McpServerConfig[]; status: ManagedAgent["status"]; pid: number | null; created_at: string; @@ -872,6 +874,7 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { personaOrphaned: agent.persona_orphaned ?? false, needsRestart: agent.needs_restart ?? false, envVars: agent.env_vars ?? {}, + mcpServers: agent.mcp_servers ?? [], status: agent.status, pid: agent.pid, createdAt: agent.created_at, @@ -1031,6 +1034,7 @@ export async function createManagedAgent(input: CreateManagedAgentInput) { avatarUrl: input.avatarUrl, model: input.model, envVars: input.envVars ?? {}, + mcpServers: input.mcpServers ?? [], spawnAfterCreate: input.spawnAfterCreate, startOnAppLaunch: input.startOnAppLaunch, backend: input.backend, diff --git a/desktop/src/shared/api/tauriPersonas.ts b/desktop/src/shared/api/tauriPersonas.ts index bd3be9240..c1cf53336 100644 --- a/desktop/src/shared/api/tauriPersonas.ts +++ b/desktop/src/shared/api/tauriPersonas.ts @@ -2,6 +2,7 @@ import { invokeTauri } from "@/shared/api/tauri"; import type { AgentPersona, CreatePersonaInput, + McpServerConfig, RespondToMode, UpdatePersonaInput, } from "@/shared/api/types"; @@ -19,6 +20,7 @@ export type RawPersona = { is_active?: boolean; source_team?: string | null; env_vars?: Record; + mcp_servers?: McpServerConfig[]; respond_to?: string | null; respond_to_allowlist?: string[]; parallelism?: number | null; @@ -42,6 +44,7 @@ export function fromRawPersona(persona: RawPersona): AgentPersona { isActive: persona.is_active ?? true, sourceTeam: persona.source_team ?? null, envVars: persona.env_vars ?? {}, + mcpServers: persona.mcp_servers ?? [], respondTo: (persona.respond_to as RespondToMode | undefined) ?? null, respondToAllowlist: persona.respond_to_allowlist ?? [], parallelism: persona.parallelism ?? null, @@ -68,6 +71,7 @@ export async function createPersona( provider: input.provider, namePool: input.namePool ?? [], envVars: input.envVars ?? {}, + mcpServers: input.mcpServers ?? [], behavior: input.behavior, }, }), @@ -91,6 +95,8 @@ export async function updatePersona( // tells the backend "don't touch the stored env vars" so editing // unrelated fields can't silently wipe saved credentials. envVars: input.envVars, + // Same absent-vs-present contract as envVars for the local MCP layer. + mcpServers: input.mcpServers, // Same absent-vs-present contract as envVars for the behavioral quad. behavior: input.behavior, }, diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 8d125d56e..3b298907e 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -426,6 +426,9 @@ export type ManagedAgent = { needsRestart: boolean; /** Per-agent env vars. Layered on top of persona envVars. */ envVars: Record; + /** Local-only MCP server layer. Layered on top of persona mcpServers + * (global < definition < agent, by name). */ + mcpServers: McpServerConfig[]; status: "running" | "stopped" | "deployed" | "not_deployed"; pid: number | null; createdAt: string; @@ -499,6 +502,8 @@ export type CreateManagedAgentInput = { model?: string; provider?: string; envVars?: Record; + /** Local-only MCP server layer. Absent = empty (matches backend default). */ + mcpServers?: McpServerConfig[]; spawnAfterCreate?: boolean; startOnAppLaunch?: boolean; backend?: ManagedAgentBackend; @@ -697,6 +702,24 @@ export type ConfigSourceReport = { export type ExtensionEntry = { name: string; kind: string; enabled: boolean }; +/** An env var passed to an MCP server subprocess. Mirrors the Rust `McpServerEnvVar`. */ +export type McpServerEnvVar = { name: string; value: string }; + +/** + * A locally persisted stdio MCP server configuration, layered + * `global < definition < agent` by name. Mirrors the Rust `McpServerConfig`. + * STDIO only — no SSE/HTTP, no timeout, no headers. + */ +export type McpServerConfig = { + name: string; + /** Required for enabled servers. Disabled entries may omit it — their + * only purpose is to mask a lower-precedence server by name. */ + command: string; + args: string[]; + env: McpServerEnvVar[]; + enabled: boolean; +}; + export type NormalizedConfig = { model: NormalizedField | null; provider: NormalizedField | null; @@ -714,6 +737,10 @@ export type RuntimeConfigSurface = { normalized: NormalizedConfig; advanced: ConfigField[]; extensions: ExtensionEntry[]; + /** Effective (global < definition < agent merged, enabled-only) buzz-agent + * MCP servers — "what runs." Empty for every other runtime, which + * surface their servers via `extensions` instead. */ + buzzAgentMcpServers: McpServerConfig[]; sources: ConfigSourceReport; }; @@ -725,6 +752,8 @@ export type UpdateManagedAgentInput = { systemPrompt?: string | null; /** Absent = don't touch. Present = replace the env_vars map entirely. */ envVars?: Record; + /** Absent = don't touch. Present = replace this local-only MCP layer. */ + mcpServers?: McpServerConfig[]; parallelism?: number; turnTimeoutSeconds?: number; relayUrl?: string; @@ -766,6 +795,9 @@ export type AgentPersona = { /** Environment variables injected for agents created from this persona. * Layered as: desktop parent env < persona envVars < agent envVars. */ envVars: Record; + /** Local-only MCP server layer inherited by agents created from this + * definition. Layered as: global < persona mcpServers < agent mcpServers. */ + mcpServers: McpServerConfig[]; /** NIP-AP behavioral defaults (wire shape). Null/empty = unset. */ respondTo: RespondToMode | null; respondToAllowlist: string[]; @@ -794,6 +826,8 @@ export type CreatePersonaInput = { provider?: string; namePool?: string[]; envVars?: Record; + /** Local-only MCP server layer. Absent = empty (matches backend default). */ + mcpServers?: McpServerConfig[]; behavior?: PersonaBehaviorInput; }; @@ -807,6 +841,8 @@ export type UpdatePersonaInput = { provider?: string; namePool?: string[]; envVars?: Record; + /** Absent = don't touch. Present = replace this local-only MCP layer. */ + mcpServers?: McpServerConfig[]; behavior?: PersonaBehaviorInput; }; @@ -1025,6 +1061,8 @@ export type ChannelMessagesPageResponse = { export type GlobalAgentConfig = { /** Global env vars injected into all agents unconditionally. */ env_vars: Record; + /** Local-only MCP server layer inherited by every buzz-agent instance. */ + mcp_servers: McpServerConfig[]; /** Global fallback provider (e.g. "anthropic", "databricks_v2"). Null = no global default. */ provider: string | null; /** Global fallback model identifier. Null = no global default. */ diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index eba0c053a..f544597cf 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -6,7 +6,7 @@ import { parse as yamlParse } from "yaml"; import { relayClient } from "@/shared/api/relayClient"; import type { ConnectionState } from "@/shared/api/relayClientShared"; -import type { RelayEvent } from "@/shared/api/types"; +import type { McpServerConfig, RelayEvent } from "@/shared/api/types"; import { getMarkdownParseCount } from "@/shared/ui/markdown/nodeCache"; import { syncAgentTurnsFromEvents } from "@/features/agents/activeAgentTurnsStore"; import { recordTimeoutFromRejection } from "@/features/moderation/lib/timeoutStore"; @@ -73,6 +73,10 @@ type MockManagedAgentSeed = { autoRestartOnConfigChange?: boolean; respondTo?: RawManagedAgent["respond_to"]; respondToAllowlist?: string[]; + /** Override the default "goose" agent command (e.g. "buzz-agent"). */ + agentCommand?: string; + /** Pre-seeded MCP server layer for the agent record. */ + mcpServers?: McpServerConfig[]; }; type MockRelayAgentSeed = { @@ -229,11 +233,13 @@ type E2eConfig = { identityLocked?: boolean; /** * Global agent config returned by `get_global_agent_config`. Defaults to - * an empty config (no provider, model, or env vars) if not specified. - * Pass a config with a provider to test Inherit-from-global behavior. + * an empty config (no provider, model, env vars, or MCP servers) if not + * specified. Pass a config with a provider to test Inherit-from-global + * behavior. */ globalAgentConfig?: { env_vars: Record; + mcp_servers?: McpServerConfig[]; provider: string | null; model: string | null; }; @@ -549,6 +555,7 @@ type RawManagedAgent = { avatar_url: string | null; model: string | null; env_vars?: Record; + mcp_servers?: McpServerConfig[]; status: "running" | "stopped" | "deployed" | "not_deployed"; pid: number | null; created_at: string; @@ -621,6 +628,7 @@ type RawPersona = { is_active: boolean; source_team?: string | null; env_vars?: Record; + mcp_servers?: McpServerConfig[]; respond_to?: string | null; respond_to_allowlist?: string[]; parallelism?: number | null; @@ -1198,6 +1206,14 @@ function cloneRelayAgent(agent: RawRelayAgent): RawRelayAgent { }; } +function cloneMcpServer(server: McpServerConfig): McpServerConfig { + return { + ...server, + args: [...server.args], + env: server.env.map((envVar) => ({ ...envVar })), + }; +} + function cloneManagedAgent(agent: MockManagedAgent): RawManagedAgent { return { pubkey: agent.pubkey, @@ -1216,6 +1232,7 @@ function cloneManagedAgent(agent: MockManagedAgent): RawManagedAgent { avatar_url: agent.avatar_url ?? null, model: agent.model, env_vars: { ...(agent.env_vars ?? {}) }, + mcp_servers: (agent.mcp_servers ?? []).map(cloneMcpServer), status: agent.status, pid: agent.pid, created_at: agent.created_at, @@ -1295,6 +1312,7 @@ function buildMockConfigSurface(pubkey: string): { normalized: Record; advanced: unknown[]; extensions: unknown[]; + buzzAgentMcpServers: McpServerConfig[]; sources: Record; } { // Goose running — mixed origins, override on model @@ -1360,6 +1378,7 @@ function buildMockConfigSurface(pubkey: string): { { name: "web_search", kind: "stdio", enabled: true }, { name: "memory", kind: "stdio", enabled: false }, ], + buzzAgentMcpServers: [], sources: { acpNative: "available", acpConfigOptions: "available", @@ -1424,6 +1443,7 @@ function buildMockConfigSurface(pubkey: string): { { name: "filesystem", kind: "mcp", enabled: true }, { name: "github", kind: "mcp", enabled: true }, ], + buzzAgentMcpServers: [], sources: { acpNative: "available", acpConfigOptions: "available", @@ -1481,6 +1501,7 @@ function buildMockConfigSurface(pubkey: string): { }, advanced: [], extensions: [{ name: "developer", kind: "stdio", enabled: true }], + buzzAgentMcpServers: [], sources: { acpNative: "pending", acpConfigOptions: "pending", @@ -1560,6 +1581,7 @@ function buildMockConfigSurface(pubkey: string): { { name: "filesystem", kind: "mcp", enabled: true }, { name: "github", kind: "mcp", enabled: true }, ], + buzzAgentMcpServers: [], sources: { acpNative: "notApplicable", acpConfigOptions: "notApplicable", @@ -1619,6 +1641,7 @@ function buildMockConfigSurface(pubkey: string): { }, advanced: [], extensions: [{ name: "web_search", kind: "stdio", enabled: true }], + buzzAgentMcpServers: [], sources: { acpNative: "available", acpConfigOptions: "available", @@ -1679,6 +1702,7 @@ function buildMockConfigSurface(pubkey: string): { }, advanced: [], extensions: [], + buzzAgentMcpServers: [], sources: { acpNative: "available", acpConfigOptions: "available", @@ -1689,7 +1713,7 @@ function buildMockConfigSurface(pubkey: string): { }, }; - const buzzAgentSurface = { + const buzzAgentBase = { ...gooseSurface, runtimeId: "buzz-agent", runtimeLabel: "Buzz Agent", @@ -1702,12 +1726,35 @@ function buildMockConfigSurface(pubkey: string): { }, }; + // Empty WYSIWYG surface — "No custom servers configured" path. + const buzzAgentEmptySurface = { + ...buzzAgentBase, + buzzAgentMcpServers: [] satisfies McpServerConfig[], + }; + + // Populated WYSIWYG surface — exercises the read-only BuzzAgentMcpServerRow + // rendering with an effective-merged server list. + const buzzAgentPopulatedSurface = { + ...buzzAgentBase, + buzzAgentMcpServers: [ + { + name: "filesystem", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + env: [], + enabled: true, + }, + ] satisfies McpServerConfig[], + }; + // Map well-known test pubkeys to specific fixtures. // Synthetic agents are intentionally not TEST_IDENTITIES. const PUBKEY_MULTI_ORIGIN = "abc1230000000000000000000000000000000000000000000000000000000def"; const PUBKEY_BUZZ_AGENT = "b0220000000000000000000000000000000000000000000000000000000000a9"; + const PUBKEY_BUZZ_AGENT_POPULATED = + "b0220000000000000000000000000000000000000000000000000000000000b1"; switch (pubkey) { case ALICE_PUBKEY: @@ -1721,7 +1768,9 @@ function buildMockConfigSurface(pubkey: string): { case PUBKEY_MULTI_ORIGIN: return multiOriginSurface; case PUBKEY_BUZZ_AGENT: - return buzzAgentSurface; + return buzzAgentEmptySurface; + case PUBKEY_BUZZ_AGENT_POPULATED: + return buzzAgentPopulatedSurface; default: return gooseSurface; } @@ -1737,7 +1786,7 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { persona_id: seed.personaId ?? null, relay_url: DEFAULT_RELAY_WS_URL, acp_command: "buzz-acp", - agent_command: "goose", + agent_command: seed.agentCommand ?? "goose", agent_args: ["acp"], mcp_command: "", turn_timeout_seconds: 320, @@ -1748,6 +1797,7 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { avatar_url: seed.avatarUrl ?? null, model: null, env_vars: {}, + mcp_servers: (seed.mcpServers ?? []).map(cloneMcpServer), status, pid: status === "running" ? 42000 + mockManagedAgents.length : null, created_at: now, @@ -6633,6 +6683,7 @@ async function handleCreatePersona(args: { avatarUrl?: string; systemPrompt: string; envVars?: Record; + mcpServers?: McpServerConfig[]; behavior?: PersonaBehaviorInput; }; }): Promise { @@ -6646,6 +6697,7 @@ async function handleCreatePersona(args: { is_active: true, source_team: null, env_vars: { ...(args.input.envVars ?? {}) }, + mcp_servers: (args.input.mcpServers ?? []).map(cloneMcpServer), created_at: now, updated_at: now, }; @@ -6661,6 +6713,7 @@ async function handleUpdatePersona(args: { avatarUrl?: string; systemPrompt: string; envVars?: Record; + mcpServers?: McpServerConfig[]; behavior?: PersonaBehaviorInput; }; }): Promise { @@ -6681,6 +6734,10 @@ async function handleUpdatePersona(args: { // Absent = preserve; present = replace entirely (matches Rust handler). persona.env_vars = { ...args.input.envVars }; } + if (args.input.mcpServers !== undefined) { + // Same absent-vs-present contract as envVars. + persona.mcp_servers = args.input.mcpServers.map(cloneMcpServer); + } applyMockPersonaBehavior(persona, args.input.behavior); persona.updated_at = new Date().toISOString(); @@ -6924,6 +6981,7 @@ async function handleCreateManagedAgent( avatarUrl?: string; model?: string; envVars?: Record; + mcpServers?: McpServerConfig[]; spawnAfterCreate?: boolean; startOnAppLaunch?: boolean; backend?: @@ -6997,6 +7055,7 @@ async function handleCreateManagedAgent( avatar_url: avatarUrl, model: args.input.model?.trim() || null, env_vars: { ...(args.input.envVars ?? {}) }, + mcp_servers: (args.input.mcpServers ?? []).map(cloneMcpServer), status: args.input.spawnAfterCreate ? "running" : "stopped", pid: args.input.spawnAfterCreate ? 42000 + mockManagedAgents.length : null, created_at: now, @@ -7195,6 +7254,7 @@ async function handleUpdateManagedAgent(args: { model?: string | null; systemPrompt?: string | null; envVars?: Record; + mcpServers?: McpServerConfig[]; respondTo?: "owner-only" | "allowlist" | "anyone"; respondToAllowlist?: string[]; }; @@ -7212,6 +7272,10 @@ async function handleUpdateManagedAgent(args: { if (args.input.envVars !== undefined) { agent.env_vars = { ...args.input.envVars }; } + if (args.input.mcpServers !== undefined) { + // Same absent-vs-present contract as envVars. + agent.mcp_servers = args.input.mcpServers.map(cloneMcpServer); + } if (args.input.respondTo !== undefined) { agent.respond_to = args.input.respondTo; } @@ -9257,15 +9321,16 @@ export function maybeInstallE2eTauriMocks() { return null; } case "get_global_agent_config": { - // Return the mock global agent config if provided; otherwise return - // an empty config (no global provider, model, or env vars). - return ( - config?.mock?.globalAgentConfig ?? { - env_vars: {}, - provider: null, - model: null, - } - ); + // Return the mock global agent config, defaulting each field + // individually so a partial seed (e.g. {env_vars, provider, model} + // without mcp_servers) still produces a valid shape. + return { + env_vars: {}, + mcp_servers: [], + provider: null, + model: null, + ...config?.mock?.globalAgentConfig, + }; } case "set_global_agent_config": { // Echo back the submitted config as the saved value (mirrors the @@ -9275,6 +9340,7 @@ export function maybeInstallE2eTauriMocks() { payload as { config: { env_vars: Record; + mcp_servers?: McpServerConfig[]; provider: string | null; model: string | null; }; diff --git a/desktop/tests/e2e/config-bridge-screenshots.spec.ts b/desktop/tests/e2e/config-bridge-screenshots.spec.ts index 11d4a73fa..94477e229 100644 --- a/desktop/tests/e2e/config-bridge-screenshots.spec.ts +++ b/desktop/tests/e2e/config-bridge-screenshots.spec.ts @@ -14,6 +14,8 @@ const MULTI_ORIGIN_PUBKEY = "abc1230000000000000000000000000000000000000000000000000000000def"; const BUZZ_AGENT_PUBKEY = "b0220000000000000000000000000000000000000000000000000000000000a9"; +const BUZZ_AGENT_POPULATED_PUBKEY = + "b0220000000000000000000000000000000000000000000000000000000000b1"; const MANAGED_AGENTS = [ { @@ -281,6 +283,30 @@ test.describe("config bridge screenshots", () => { ); }); + test("06b — buzz-agent populated MCP servers", async ({ page }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: BUZZ_AGENT_POPULATED_PUBKEY, + name: "Buzz Populated", + status: "running" as const, + channelNames: ["agents"], + }, + ], + }); + + const panel = await openAgentProfileFromChannel(page, "Buzz Populated"); + + // The populated WYSIWYG surface includes a "filesystem" server row. + await expect(panel.getByText("filesystem", { exact: true })).toBeVisible(); + await expect( + panel.getByText("npx -y @modelcontextprotocol/server-filesystem /tmp"), + ).toBeVisible(); + await expect(panel.getByText("MCP Servers", { exact: true })).toHaveCount( + 1, + ); + }); + test("07 — profile side panel — Configuration section", async ({ page }) => { // charlie (554cef…) is the well-known test pubkey that the mock bridge // seeds as a bot owned by the test viewer, so isBot + isOwner + managedAgent diff --git a/desktop/tests/e2e/mcp-servers.spec.ts b/desktop/tests/e2e/mcp-servers.spec.ts new file mode 100644 index 000000000..fa4d90ea8 --- /dev/null +++ b/desktop/tests/e2e/mcp-servers.spec.ts @@ -0,0 +1,414 @@ +import { expect, test } from "@playwright/test"; + +import { waitForAnimations } from "../helpers/animations"; +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +test.beforeEach(async ({ page }) => { + await installMockBridge(page); +}); + +// ── helpers ───────────────────────────────────────────────────────────────── + +async function gotoApp(page: import("@playwright/test").Page) { + await page.goto("/", { waitUntil: "domcontentloaded" }); + await waitForInvokeBridge(page); + await expect(page.getByTestId("open-agents-view")).toBeVisible({ + timeout: 10_000, + }); +} + +async function waitForInvokeBridge(page: import("@playwright/test").Page) { + await page.waitForFunction( + () => { + const w = window as Window & { + __BUZZ_E2E_INVOKE_MOCK_COMMAND__?: unknown; + __TAURI_INTERNALS__?: { invoke?: unknown }; + }; + return ( + typeof w.__BUZZ_E2E_INVOKE_MOCK_COMMAND__ === "function" || + typeof w.__TAURI_INTERNALS__?.invoke === "function" + ); + }, + null, + { timeout: 5_000 }, + ); +} + +async function invokeTauri( + page: import("@playwright/test").Page, + command: string, + payload?: Record, +): Promise { + await waitForInvokeBridge(page); + return page.evaluate( + async ({ command: c, payload: p }) => { + const w = window as Window & { + __BUZZ_E2E_INVOKE_MOCK_COMMAND__?: ( + c: string, + p?: Record, + ) => Promise; + __TAURI_INTERNALS__?: { + invoke?: (c: string, p?: Record) => Promise; + }; + }; + const invoke = + w.__BUZZ_E2E_INVOKE_MOCK_COMMAND__ ?? w.__TAURI_INTERNALS__?.invoke; + if (!invoke) throw new Error("Mock invoke bridge is unavailable."); + return (await invoke(c, p)) as T; + }, + { command, payload }, + ); +} + +const AGENT_PUBKEY = TEST_IDENTITIES.tyler.pubkey; +const AGENT_NAME = "MCP Test Agent"; + +async function openEditDialog(page: import("@playwright/test").Page) { + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + const agentButton = page.getByRole("button", { + name: `${AGENT_NAME} agent profile`, + }); + await expect(agentButton).toBeVisible({ timeout: 10_000 }); + await agentButton.click(); + await expect(page.getByTestId("user-profile-panel")).toBeVisible({ + timeout: 10_000, + }); + await page.getByTestId("user-profile-edit-agent").click(); + await expect(page.getByTestId("edit-agent-dialog")).toBeVisible({ + timeout: 10_000, + }); + // Wait for the runtime catalog to load and form to settle (buzz-agent auto- + // expands Advanced, which makes the MCP servers editor immediately visible). + await expect(page.getByTestId("mcp-servers-editor").first()).toBeVisible({ + timeout: 10_000, + }); +} + +// ── round-trip tests ───────────────────────────────────────────────────────── + +test("persona mcp_servers round-trip through create_persona + update_persona", async ({ + page, +}) => { + await gotoApp(page); + + const fsServer = { + name: "filesystem", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + env: [], + enabled: true, + }; + const fetchServer = { + name: "fetch-mcp", + command: "uvx", + args: ["mcp-server-fetch"], + env: [], + enabled: true, + }; + + const created = await invokeTauri<{ + id: string; + mcp_servers?: unknown[]; + }>(page, "create_persona", { + input: { + displayName: "MCP Persona", + systemPrompt: "You use MCP.", + mcpServers: [fsServer, fetchServer], + }, + }); + + expect(created.mcp_servers).toHaveLength(2); + expect(created.mcp_servers?.[0]).toMatchObject({ name: "filesystem" }); + expect(created.mcp_servers?.[1]).toMatchObject({ name: "fetch-mcp" }); + + // Update: drop one, change one, add one. + const dbServer = { + name: "sqlite-db", + command: "uvx", + args: ["mcp-server-sqlite", "--db", "/tmp/test.db"], + env: [], + enabled: true, + }; + const updated = await invokeTauri<{ mcp_servers?: unknown[] }>( + page, + "update_persona", + { + input: { + id: created.id, + displayName: "MCP Persona", + systemPrompt: "You use MCP.", + mcpServers: [ + { ...fsServer, command: "bunx" }, // changed command + dbServer, // added + // fetch-mcp dropped + ], + }, + }, + ); + + expect(updated.mcp_servers).toHaveLength(2); + expect(updated.mcp_servers?.[0]).toMatchObject({ + name: "filesystem", + command: "bunx", + }); + expect(updated.mcp_servers?.[1]).toMatchObject({ name: "sqlite-db" }); +}); + +test("update_persona preserves mcp_servers when caller omits the field", async ({ + page, +}) => { + await gotoApp(page); + + const created = await invokeTauri<{ + id: string; + mcp_servers?: unknown[]; + }>(page, "create_persona", { + input: { + displayName: "MCP Keeper", + systemPrompt: "You keep MCP.", + mcpServers: [ + { + name: "filesystem", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + env: [], + enabled: true, + }, + ], + }, + }); + expect(created.mcp_servers).toHaveLength(1); + + // Update WITHOUT including mcpServers. Stored list must survive. + const preserved = await invokeTauri<{ mcp_servers?: unknown[] }>( + page, + "update_persona", + { + input: { + id: created.id, + displayName: "MCP Keeper (renamed)", + systemPrompt: "You keep MCP.", + // mcpServers intentionally omitted + }, + }, + ); + expect(preserved.mcp_servers).toHaveLength(1); + + // Explicit empty array still clears (intentional). + const cleared = await invokeTauri<{ mcp_servers?: unknown[] }>( + page, + "update_persona", + { + input: { + id: created.id, + displayName: "MCP Keeper (renamed)", + systemPrompt: "You keep MCP.", + mcpServers: [], + }, + }, + ); + expect(cleared.mcp_servers ?? []).toHaveLength(0); +}); + +test("agent mcp_servers round-trip through create_managed_agent + update_managed_agent", async ({ + page, +}) => { + await gotoApp(page); + + const created = await invokeTauri<{ + agent: { pubkey: string; mcp_servers?: unknown[] }; + }>(page, "create_managed_agent", { + input: { + name: "mcp-e2e-agent", + backend: { type: "local" }, + mcpServers: [ + { + name: "filesystem", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + env: [], + enabled: true, + }, + ], + }, + }); + expect(created.agent.mcp_servers).toHaveLength(1); + expect(created.agent.mcp_servers?.[0]).toMatchObject({ + name: "filesystem", + command: "npx", + }); + + // Update mcpServers — replaces entire list. + const updated = await invokeTauri<{ + agent: { mcp_servers?: unknown[] }; + }>(page, "update_managed_agent", { + input: { + pubkey: created.agent.pubkey, + mcpServers: [ + { + name: "fetch-mcp", + command: "uvx", + args: ["mcp-server-fetch"], + env: [], + enabled: true, + }, + ], + }, + }); + expect(updated.agent.mcp_servers).toHaveLength(1); + expect(updated.agent.mcp_servers?.[0]).toMatchObject({ + name: "fetch-mcp", + command: "uvx", + }); + + // Omitting mcpServers on a subsequent update preserves the stored list. + const preserved = await invokeTauri<{ + agent: { mcp_servers?: unknown[] }; + }>(page, "update_managed_agent", { + input: { + pubkey: created.agent.pubkey, + name: "mcp-e2e-agent-renamed", + // mcpServers intentionally omitted + }, + }); + expect(preserved.agent.mcp_servers).toHaveLength(1); + expect(preserved.agent.mcp_servers?.[0]).toMatchObject({ name: "fetch-mcp" }); +}); + +// ── UI tests ───────────────────────────────────────────────────────────────── + +test("mcp servers editor renders in PersonaDialog new-persona form", async ({ + page, +}) => { + await gotoApp(page); + + // Open the Agents view, click New > New agent. + await page.getByTestId("open-agents-view").click(); + await page.getByTestId("new-agent-card").click(); + await page.getByRole("menuitem", { name: /^New agent$/ }).click(); + + const dialog = page.getByRole("dialog"); + + // The default runtime is buzz-agent — Advanced auto-expands so the MCP + // servers editor is immediately visible without a click. + await expect(dialog.getByTestId("mcp-servers-editor")).toBeVisible({ + timeout: 10_000, + }); + // Initially empty — no rows. + await expect(dialog.getByTestId("mcp-servers-row")).toHaveCount(0); + + // Add a row and fill the name + command fields. + await dialog.getByTestId("mcp-servers-add").click(); + await expect(dialog.getByTestId("mcp-servers-row")).toHaveCount(1); + + await dialog.getByTestId("mcp-servers-name").fill("filesystem"); + await dialog.getByTestId("mcp-servers-command").fill("npx"); + + // Add a second row to verify multi-row rendering. + await dialog.getByTestId("mcp-servers-add").click(); + await dialog.getByTestId("mcp-servers-name").last().fill("fetch-mcp"); + await dialog.getByTestId("mcp-servers-command").last().fill("uvx"); + + await waitForAnimations(page); + await dialog.screenshot({ + path: "test-results/mcp-servers-persona-dialog.png", + }); + + // Remove the first row — verify per-row removal works. + await dialog.getByTestId("mcp-servers-remove").first().click(); + await expect(dialog.getByTestId("mcp-servers-row")).toHaveCount(1); +}); + +test("mcp servers editor renders in global agent config card", async ({ + page, +}) => { + await installMockBridge(page, { + globalAgentConfig: { + env_vars: {}, + mcp_servers: [ + { + name: "filesystem", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + env: [], + enabled: true, + }, + ], + provider: null, + model: null, + }, + }); + + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + await expect(page.getByTestId("settings-global-agent-config")).toBeVisible({ + timeout: 10_000, + }); + + const card = page.getByTestId("settings-global-agent-config"); + + // The McpServersEditor is inside the global config card. + await expect(card.getByTestId("mcp-servers-editor")).toBeVisible({ + timeout: 5_000, + }); + + // The existing server from the mock shows as a pre-populated row. + await expect(card.getByTestId("mcp-servers-row")).toHaveCount(1); + await expect(card.getByTestId("mcp-servers-name")).toHaveValue("filesystem"); + + await card.scrollIntoViewIfNeeded(); + await waitForAnimations(page); + await card.screenshot({ + path: "test-results/mcp-servers-global-config-card.png", + }); +}); + +test("mcp servers editor renders in agent-instance edit dialog", async ({ + page, +}) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: AGENT_PUBKEY, + name: AGENT_NAME, + status: "stopped", + channelNames: ["agents"], + agentCommand: "buzz-agent", + mcpServers: [ + { + name: "filesystem", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], + env: [], + enabled: true, + }, + ], + }, + ], + }); + + await openEditDialog(page); + + const dialog = page.getByRole("dialog"); + + // buzz-agent auto-expands Advanced — the MCP servers editor is already visible. + const editor = dialog.getByTestId("mcp-servers-editor"); + await expect(editor).toBeVisible(); + + // The agent's pre-existing MCP server renders as an editable row. + await expect(editor.getByTestId("mcp-servers-row")).toHaveCount(1); + await expect(editor.getByTestId("mcp-servers-name")).toHaveValue( + "filesystem", + ); + + // Add a second row. + await editor.getByTestId("mcp-servers-add").click(); + await editor.getByTestId("mcp-servers-name").last().fill("fetch-mcp"); + await editor.getByTestId("mcp-servers-command").last().fill("uvx"); + + await waitForAnimations(page); + await dialog.screenshot({ + path: "test-results/mcp-servers-edit-agent-dialog.png", + }); +}); diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index cbbef3c00..5c041aa9f 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -58,6 +58,16 @@ type MockManagedAgentSeed = { autoRestartOnConfigChange?: boolean; respondTo?: "owner-only" | "allowlist" | "anyone"; respondToAllowlist?: string[]; + /** Override the default "goose" agent command (e.g. "buzz-agent"). */ + agentCommand?: string; + /** Pre-seeded MCP server layer for the agent record. */ + mcpServers?: Array<{ + name: string; + command: string; + args: string[]; + env: Array<{ name: string; value: string }>; + enabled: boolean; + }>; }; type MockSearchProfileSeed = { @@ -283,11 +293,19 @@ type MockBridgeOptions = { identityLocked?: boolean; /** * Global agent config returned by `get_global_agent_config`. Defaults to - * an empty config (no provider, model, or env vars) if not specified. - * Pass a config with a provider to test Inherit-from-global behavior. + * an empty config (no provider, model, env vars, or MCP servers) if not + * specified. Pass a config with a provider to test Inherit-from-global + * behavior. */ globalAgentConfig?: { env_vars: Record; + mcp_servers?: Array<{ + name: string; + command: string; + args: string[]; + env: Array<{ name: string; value: string }>; + enabled: boolean; + }>; provider: string | null; model: string | null; }; diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index 260de02c1..a58476302 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -708,10 +708,10 @@ packages: dependency: transitive description: name: meta - sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" + sha256: "1741988757a65eb6b36abe716829688cf01910bbf91c34354ff7ec1c3de2b349" url: "https://pub.dev" source: hosted - version: "1.17.0" + version: "1.18.0" mime: dependency: transitive description: @@ -1137,26 +1137,26 @@ packages: dependency: transitive description: name: test - sha256: "280d6d890011ca966ad08df7e8a4ddfab0fb3aa49f96ed6de56e3521347a9ae7" + sha256: "8d9ceddbab833f180fbefed08afa76d7c03513dfdba87ffcec2718b02bbcbf20" url: "https://pub.dev" source: hosted - version: "1.30.0" + version: "1.31.0" test_api: dependency: transitive description: name: test_api - sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" + sha256: "949a932224383300f01be9221c39180316445ecb8e7547f70a41a35bf421fb9e" url: "https://pub.dev" source: hosted - version: "0.7.10" + version: "0.7.11" test_core: dependency: transitive description: name: test_core - sha256: "0381bd1585d1a924763c308100f2138205252fb90c9d4eeaf28489ee65ccde51" + sha256: "1991d4cfe85d5043241acac92962c3977c8d2f2add1ee73130c7b286417d1d34" url: "https://pub.dev" source: hosted - version: "0.6.16" + version: "0.6.17" tuple: dependency: transitive description: