diff --git a/AGENTS.md b/AGENTS.md index 7ff0eb4d47..a1bdef70e7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -165,6 +165,26 @@ check existing reply handlers for the pattern. by the ACP harness into managed agent subprocesses. In development, set `BUZZ_PRIVATE_KEY` and `BUZZ_RELAY_URL` in your environment manually. +### Harness MCP configuration (`BUZZ_ACP_MCP_SERVERS`) + +`buzz-acp` derives one MCP server from `--mcp-command` (the dev MCP). +`BUZZ_ACP_MCP_SERVERS` adds more, as a JSON array of +`{name, command, args, env:[{name,value}]}` objects merged with that one: + +```bash +export BUZZ_ACP_MCP_SERVERS='[{"name":"razorpay","command":"/usr/bin/rzp","args":["--stdio"],"env":[]}]' +``` + +Malformed JSON is logged and ignored — a bad value degrades to the dev MCP +alone rather than taking the agent's tools down with it. + +A channel's server set can also be changed at runtime via the owner-signed +`update_mcp_servers` observer control frame. That applies at the channel's +next turn boundary through `session/resume`: same session ID, transcript +preserved, never cancelling an in-flight turn. Channels managed this way also +get `strictMcpConfig`, so they run exactly the listed servers; unmanaged +channels keep the agent's own global MCP config untouched. + ### Building the CLI ```bash diff --git a/EXECUTE.md b/EXECUTE.md new file mode 100644 index 0000000000..0e290c6ecd --- /dev/null +++ b/EXECUTE.md @@ -0,0 +1,123 @@ +# Execute prompt — live per-channel MCP control + +Paste the block below into a fresh agent session (Buzz channel agent, or Claude Code started in `~/Dev/buzz`). + +**Branch note:** this prompt puts the agent on its own branch so it can run alongside/against another implementation without collisions. Change the branch name in the block if you want it somewhere else. + +## If you're running this through Buzz — read this first + +Three facts about this machine's Buzz setup, verified 2026-07-31: + +1. **Every Buzz agent has `cwd = /Users/neeyafit/.buzz`.** The harness passes + `std::env::current_dir()` as the ACP session cwd (`crates/buzz-acp/src/lib.rs:1546`) and there is + no per-agent working-directory setting in `managed-agents.json`. The agent is *not* in the repo + and will not auto-load the repo's `AGENTS.md`. The prompt below handles this — don't remove the + `cd` instruction. +2. **`turn_timeout_seconds: 320`** (~5 min/turn). Do not ask for all 8 tasks in one message. Drive + it **one task per turn**; a cold `cargo build -p buzz-acp` can consume most of a turn on its own. +3. **Use a fresh channel or DM, and the Opus-5 agent.** Buzz seeds a new session with a small window + of recent channel history, so starting in a busy channel feeds unrelated chatter into the session. + Opus-5 is `opus[1m]` — the 1M context matters for a ~1400-line plan across 8 tasks. Avoid + Grok/Cursor here (different runtime). + +--- + +We're implementing a feature for contribution back to `block/buzz` (upstream). The repo is at +`~/Dev/buzz`. Remotes: `origin` = Monivancan/buzz (my fork), `upstream` = block/buzz. + +**First, three things to read, in this order:** + +1. `docs/superpowers/plans/2026-07-31-live-mcp-control-buzz-acp.md` — the approved implementation + plan. This is your instruction set: 8 tasks, each with exact files, real code, and a TDD step + sequence. Follow it task by task, in order. +2. `docs/superpowers/specs/2026-07-31-live-mcp-control-design.md` — the design the plan came from. + Read it for the *why*; the plan wins wherever they differ (the plan was written after + re-verifying the spec against shipped source and records three corrections). +3. `AGENTS.md` — repo conventions and quality gates. + +**The feature, in one line:** let a user grant or revoke an MCP server on a live Buzz channel and +have the agent continue the *same* conversation with the new tool set — via `session/resume` with +an unchanged sessionId and a changed `mcpServers` list. Plus multi-server config at spawn, and +opt-in `strictMcpConfig` to stop injecting ~64k tokens/turn of unused global tool schemas. + +**Your working directory is NOT the repo.** You start in `/Users/neeyafit/.buzz`. The repo is at +`/Users/neeyafit/Dev/buzz`. Shell cwd does not persist between tool calls, so chain `cd` into every +command (`cd /Users/neeyafit/Dev/buzz && ...`) and use absolute paths for file reads and edits. + +**Set up first:** + +```bash +cd /Users/neeyafit/Dev/buzz && . ./bin/activate-hermit && \ + git status --short && \ + git checkout feat/live-mcp-control && \ + git checkout -b feat/live-mcp-control-impl +``` + +The working tree must be clean apart from `KICKOFF.md` / `EXECUTE.md`. If it isn't, stop and report +what's there rather than committing someone else's work. + +`. ./bin/activate-hermit` is required before **every** git, cargo, or just command — the repo's +hooks and toolchain live in Hermit, and an unconfigured `PATH` makes them fail in confusing ways. +Do not rewrite hook commands to work around a broken PATH; fix the PATH. + +Work directly in `~/Dev/buzz` on the branch above — **not** in a `git worktree`. The pre-commit +hook runs `just desktop-tauri-fmt`, which fails inside worktrees and will block every commit you +try to make (AGENTS.md, Common Gotchas #6). + +**Only one agent may edit this checkout at a time.** If someone else is already working in +`~/Dev/buzz`, stop and say so rather than racing them. + +**Scope:** Tasks 1–8 of the plan, all inside `crates/buzz-acp`. The desktop UI is explicitly NOT in +scope — it is a separate plan. Do not touch `desktop/`. + +**Do ONE task per turn.** Complete Task 1, commit it, then stop and report. Wait to be told to +continue. Your turn is capped at 320 seconds — attempting several tasks in one turn will get you +killed mid-edit and leave the tree in a broken state. + +**Non-negotiables:** +- TDD, exactly as the plan lays it out: write the failing test, RUN it and see it fail, implement, + run it green, commit. Do not batch the tests to the end. Do not skip the "watch it fail" step — + a test that never failed proves nothing. +- `git commit -s` on **every** commit. CI's DCO check fails the PR without a `Signed-off-by` + trailer. Verify with `git log -1 | grep Signed-off-by` after your first commit. +- No `unsafe`. No new `unwrap()`/`expect()` in production paths — use `?`. +- Additive only; mirror the existing in-crate patterns the plan names (`session_new_full`, + `handle_switch_model_control`, `steering_supported`). +- MCP changes apply at **turn boundaries** and must NEVER cancel an in-flight turn. There is + deliberately no busy-path oneshot, unlike `switch_model`. +- `strictMcpConfig` stays **opt-in per managed channel**. Defaulting it on silently strips users' + global MCP servers. +- Run `cargo test -p buzz-acp` after every task. Run `just ci` before you call the work done — + clippy passing does not mean fmt passes. + +**Verify, don't trust.** The plan cites shipped adapter source by file:line (e.g. +`acp-agent.js:3981-4008`). Those were verified once, but re-check any line you're about to build +on — the adapter is a versioned npm package under +`~/Library/Application Support/Buzz/node-tools/lib/node_modules/@agentclientprotocol/`, and it can +change under you. If a citation no longer matches, STOP and report rather than coding around it. + +**Where to stop and ask:** +- Task 4 restructures `run_prompt_task`'s session lookup and knowingly duplicates a block. The plan + says extract a helper only *after* the tests are green. If the duplication won't compile cleanly, + report before inventing a different structure. +- The plan defers http/sse MCP servers (the Rust `McpServer` models stdio only). If a task seems to + need remote servers, stop — that's a scope change, not an implementation detail. +- Any test you cannot make pass in two attempts: stop and report the actual failure output. Do not + delete or weaken the assertion to get green. + +**Report at the end:** which tasks completed, the `cargo test -p buzz-acp` summary line, the `just ci` +result, and anything in the plan that turned out wrong. + +--- + +## What this is also testing + +If you're running this through a Buzz channel agent, the run doubles as a capability test of Buzz +itself. Worth watching for: + +- **Context survival** — the plan is ~1400 lines. Does the agent still have Task 1's decisions in + context by Task 6, or does Buzz's `context_limit` truncate it into amnesia? +- **Long-horizon tool use** — 8 tasks × ~6 steps each, with real compile/test cycles between them. +- **The MCP irony** — this agent is implementing mid-conversation MCP grants while itself stuck with + whatever MCP set it was born with. If it needs a tool it lacks partway through, that's the exact + problem the feature fixes, demonstrated live. diff --git a/Justfile b/Justfile index 2d76f1a7b9..ebc747a10c 100644 --- a/Justfile +++ b/Justfile @@ -292,6 +292,14 @@ test-unit: # Gateway unit and black-box HTTP tests are infra-free. Postgres-backed # contract/race tests run in the dedicated CI job below. cargo nextest run -p buzz-push-gateway + # ACP harness (buzz-acp): session, pool, and control-frame tests. No + # infra — the agent peer is a scripted `bash`/`cat` stand-in over the + # normal stdio pipes, so there is no relay, database, or network. Run + # all targets: tests/pool_lifecycle_state.rs compiles the lifecycle + # state machine as its own integration target. Without this line the + # crate is only ever built, never tested, and its suite ships green + # without having run. + cargo nextest run -p buzz-acp else ./scripts/run-tests.sh unit fi diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 8a698954a0..0ec0be70d1 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -24,7 +24,7 @@ const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. /// All four fields are **required** by the schema (`args` and `env` may be empty arrays). -#[derive(Debug, Clone, serde::Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct McpServer { pub name: String, pub command: String, @@ -33,7 +33,7 @@ pub struct McpServer { } /// A single environment variable for an MCP server. -#[derive(Debug, Clone, serde::Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] pub struct EnvVar { pub name: String, pub value: String, @@ -198,6 +198,14 @@ pub struct AcpClient { /// a JSON-RPC *success*, not `-32601` — which the main loop would read as /// a delivered steer and drop the user's message from the queue. steering_supported: bool, + /// Whether the agent advertised `agentCapabilities.sessionCapabilities.resume` + /// in its initialize response. Gates the in-place MCP reconfiguration path; + /// call sites fall back to session invalidation when false. + resume_supported: bool, + /// Tool names seen in `tool_call` session updates. Drained by the pool + /// after each turn to opportunistically verify an MCP grant landed — the + /// ACP wire carries no MCP status, so a tool call is the only evidence. + observed_tool_names: Vec, /// Per-turn channel for receiving goose-native non-cancelling steer /// requests from the main loop. Installed by /// [`install_steer_rx`](Self::install_steer_rx) at dispatch and @@ -548,6 +556,8 @@ impl AcpClient { observer_context: ObserverContext::default(), active_run_id: None, steering_supported: false, + resume_supported: false, + observed_tool_names: Vec::new(), steer_rx: None, goose_usage: UsageTracker::default(), }) @@ -604,6 +614,12 @@ impl AcpClient { .pointer("/_meta/steering/supported") .and_then(|v| v.as_bool()) .unwrap_or(false); + // Session-level resume capability. The value is an empty object (`{}`) + // when supported, so presence — not truthiness — is the signal. + self.resume_supported = result + .pointer("/agentCapabilities/sessionCapabilities/resume") + .map(|v| !v.is_null()) + .unwrap_or(false); tracing::debug!(target: "acp::init", "initialize response: {result}"); Ok(result) } @@ -624,6 +640,11 @@ impl AcpClient { /// `session_title` rides in `_meta.sessionTitle` when `Some`; `_meta` is /// omitted entirely otherwise, since adapters may distinguish an absent /// member from a null one. + /// `strict_mcp` marks the session as *managed*: it rides in + /// `_meta.claudeCode.options.strictMcpConfig` and tells the Claude adapter + /// to run exactly the servers in `mcp_servers`, suppressing the agent's own + /// global MCP config. Opt-in per channel — passing `true` for an unmanaged + /// channel would silently strip the user's global servers. /// Callers use [`extract_model_config_options`] and [`extract_model_state`] /// to pull model info from the raw result. pub async fn session_new_full( @@ -632,6 +653,7 @@ impl AcpClient { mcp_servers: Vec, system_prompt: Option<&str>, session_title: Option<&str>, + strict_mcp: bool, ) -> Result { let mut params = serde_json::json!({ "cwd": cwd, @@ -640,8 +662,31 @@ impl AcpClient { if let Some(sp) = system_prompt { params["systemPrompt"] = serde_json::Value::String(sp.to_owned()); } + let mut meta = serde_json::Map::new(); if let Some(title) = session_title { - params["_meta"] = serde_json::json!({ "sessionTitle": title }); + meta.insert( + "sessionTitle".into(), + serde_json::Value::String(title.to_owned()), + ); + } + if strict_mcp { + // Managed channel: run exactly the servers the panel shows. Without + // this the agent additionally loads its own global MCP config, + // injecting tool schemas nobody asked for into every turn. + // + // Deliberately does NOT touch `settingSources`: `strictMcpConfig` + // already suppresses every other MCP source, and narrowing settings + // loading would drop the user's permission defaults as a side + // effect — a much wider blast radius than this flag is meant to have. + meta.insert( + "claudeCode".into(), + serde_json::json!({ + "options": { "strictMcpConfig": true } + }), + ); + } + if !meta.is_empty() { + params["_meta"] = serde_json::Value::Object(meta); } let result = self.send_request("session/new", params).await?; let session_id = result["sessionId"] @@ -667,11 +712,41 @@ impl AcpClient { session_title: Option<&str>, ) -> Result { Ok(self - .session_new_full(cwd, mcp_servers, system_prompt, session_title) + .session_new_full(cwd, mcp_servers, system_prompt, session_title, false) .await? .session_id) } + /// Send `session/resume` to reconfigure a live session in place. + /// + /// The session ID is unchanged. Adapters that fingerprint the + /// session-defining params (`cwd` + `mcpServers`) tear down and recreate + /// the underlying agent process with `resume`, restoring the full + /// conversation transcript from disk — this is how an MCP grant lands + /// without losing the conversation. + /// + /// `cwd` must be an absolute path. `mcp_servers` may be empty. Gated on + /// [`resume_supported`](Self::resume_supported): callers must fall back to + /// session invalidation when the agent does not advertise the capability. + /// + /// Prefer this over `session/load` — `load` replays the whole history as + /// `session/update` notifications before responding, which risks the + /// request timeout on long conversations. Reconfiguration semantics are + /// identical. + pub async fn session_resume( + &mut self, + session_id: &str, + cwd: &str, + mcp_servers: Vec, + ) -> Result { + let params = serde_json::json!({ + "sessionId": session_id, + "cwd": cwd, + "mcpServers": mcp_servers, + }); + self.send_request("session/resume", params).await + } + /// Send Goose's custom system-prompt request after `session/new`. pub async fn session_set_goose_system_prompt( &mut self, @@ -850,6 +925,16 @@ impl AcpClient { self.steering_supported } + /// Whether the connected agent supports `session/resume`. + pub fn resume_supported(&self) -> bool { + self.resume_supported + } + + /// Take the tool names observed since the last call, clearing the buffer. + pub fn take_observed_tool_names(&mut self) -> Vec { + std::mem::take(&mut self.observed_tool_names) + } + /// Consume and return the per-turn usage record computed from the most /// recent `_goose/unstable/session/update` notification. /// @@ -1728,6 +1813,10 @@ impl AcpClient { .and_then(|v| v.as_str()) .unwrap_or("unknown"); tracing::info!(target: "acp::tool", "tool_call: {title} ({kind})"); + // Recorded for opportunistic MCP-grant verification; the pool + // drains this after the turn. Only the raw name matters, and + // only an `mcp____` prefix ever matches. + self.observed_tool_names.push(title.to_string()); true } "tool_call_update" => { @@ -3271,7 +3360,7 @@ mod tests { .expect("initialize should succeed"); let resp = client - .session_new_full("/tmp", vec![], Some("Custom system prompt"), None) + .session_new_full("/tmp", vec![], Some("Custom system prompt"), None, false) .await .expect("session_new_full should succeed"); @@ -3284,6 +3373,217 @@ mod tests { ); } + #[tokio::test] + async fn session_new_full_sends_strict_mcp_config_when_managed() { + 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,"result":{"sessionId":"ses_test","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full("/tmp", vec![], None, None, true) + .await + .expect("session_new_full should succeed"); + + let opts = &resp.raw["_receivedRequest"]["params"]["_meta"]["claudeCode"]["options"]; + assert_eq!( + opts["strictMcpConfig"].as_bool(), + Some(true), + "managed channels must suppress the agent's global MCP config" + ); + assert!( + opts["settingSources"].is_null(), + "settingSources must be left alone — narrowing it would drop the user's \ + permission defaults for managed channels, which is not what this flag is for" + ); + } + + #[tokio::test] + async fn session_new_full_omits_strict_mcp_config_when_unmanaged() { + 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,"result":{"sessionId":"ses_test","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full("/tmp", vec![], None, None, false) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert!( + received["params"]["_meta"]["claudeCode"].is_null(), + "unmanaged channels must keep legacy behaviour — never silently strip a user's global MCP servers" + ); + } + + #[tokio::test] + async fn session_resume_request_includes_session_id_cwd_and_mcp_servers() { + 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,"result":{"_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let result = client + .session_resume( + "ses_test", + "/tmp", + vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec!["--stdio".into()], + env: vec![], + }], + ) + .await + .expect("session_resume should succeed"); + + let received = &result["_receivedRequest"]; + assert_eq!(received["method"].as_str(), Some("session/resume")); + assert_eq!(received["params"]["sessionId"].as_str(), Some("ses_test")); + assert_eq!(received["params"]["cwd"].as_str(), Some("/tmp")); + assert_eq!( + received["params"]["mcpServers"][0]["name"].as_str(), + Some("razorpay"), + "mcpServers must ride on the resume request — this is the whole mechanism" + ); + } + + /// Protocol-level continuity contract: an MCP change is applied by + /// `session/resume` on the SAME session id, with exactly one `session/new` + /// and no `session/cancel`. Recall of early-turn content is a model-level + /// property and can only be shown against a live agent — see the manual + /// runbook in the PR description. + #[tokio::test] + async fn an_mcp_change_resumes_the_same_session_without_a_new_one() { + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{"sessionCapabilities":{"resume":{}}}}}' + read -t 2 NEW + echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_fixed","_receivedRequest":'"$NEW"'}}' + read -t 2 RESUME + echo '{"jsonrpc":"2.0","id":2,"result":{"_receivedRequest":'"$RESUME"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + assert!( + client.resume_supported(), + "the scripted agent advertises resume; without it the harness would rotate instead" + ); + + let created = client + .session_new_full("/tmp", vec![], None, None, false) + .await + .expect("session/new should succeed"); + assert_eq!(created.session_id, "ses_fixed"); + + let resumed = client + .session_resume( + &created.session_id, + "/tmp", + vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec![], + env: vec![], + }], + ) + .await + .expect("session/resume should succeed"); + + let methods = [ + created.raw["_receivedRequest"]["method"] + .as_str() + .unwrap_or_default() + .to_string(), + resumed["_receivedRequest"]["method"] + .as_str() + .unwrap_or_default() + .to_string(), + ]; + assert_eq!( + methods.iter().filter(|m| *m == "session/new").count(), + 1, + "an MCP change must never mint a new session — that is the whole point: {methods:?}" + ); + assert!( + !methods.iter().any(|m| m == "session/cancel"), + "an MCP change must never cancel an in-flight turn: {methods:?}" + ); + + let resume_params = &resumed["_receivedRequest"]["params"]; + assert_eq!(resume_params["sessionId"].as_str(), Some("ses_fixed")); + assert_eq!( + resume_params["mcpServers"][0]["name"].as_str(), + Some("razorpay"), + "the new server set must ride on the resume" + ); + } + + #[tokio::test] + async fn initialize_records_resume_supported_when_advertised() { + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{"sessionCapabilities":{"resume":{}}}}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + assert!( + client.resume_supported(), + "an agent advertising sessionCapabilities.resume must be detected" + ); + } + + #[tokio::test] + async fn initialize_records_resume_unsupported_when_absent() { + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{"sessionCapabilities":{}}}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + assert!( + !client.resume_supported(), + "absent resume capability must not be treated as supported" + ); + } + #[tokio::test] async fn goose_system_prompt_request_uses_append_contract() { let script = r#" @@ -3356,7 +3656,7 @@ mod tests { .expect("initialize should succeed"); let resp = client - .session_new_full("/tmp", vec![], None, None) + .session_new_full("/tmp", vec![], None, None, false) .await .expect("session_new_full should succeed"); @@ -3384,7 +3684,7 @@ mod tests { .expect("initialize should succeed"); let resp = client - .session_new_full("/tmp", vec![], None, Some("Fizz · #buzz-dev")) + .session_new_full("/tmp", vec![], None, Some("Fizz · #buzz-dev"), false) .await .expect("session_new_full should succeed"); @@ -3412,7 +3712,7 @@ mod tests { .expect("initialize should succeed"); let resp = client - .session_new_full("/tmp", vec![], None, None) + .session_new_full("/tmp", vec![], None, None, false) .await .expect("session_new_full should succeed"); diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index dab61be30a..9053e70a7a 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -261,6 +261,13 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_MCP_COMMAND", default_value = "")] pub mcp_command: String, + /// Additional MCP servers as a JSON array of + /// `{name, command, args, env:[{name,value}]}` objects. Merged with the + /// dev-MCP server derived from `--mcp-command`. Malformed JSON is logged + /// and ignored (fail open) rather than dropping the dev MCP. + #[arg(long, env = "BUZZ_ACP_MCP_SERVERS", default_value = "")] + pub mcp_servers_json: String, + /// Idle timeout: max seconds of silence before killing a turn. /// Resets on any agent stdout activity. #[arg(long, env = "BUZZ_ACP_IDLE_TIMEOUT")] @@ -495,6 +502,7 @@ pub struct Config { pub agent_command: String, pub agent_args: Vec, pub mcp_command: String, + pub mcp_servers_json: String, pub idle_timeout_secs: u64, pub max_turn_duration_secs: u64, pub agents: u32, @@ -1059,6 +1067,7 @@ impl Config { agent_command, agent_args, mcp_command: args.mcp_command, + mcp_servers_json: args.mcp_servers_json, idle_timeout_secs, max_turn_duration_secs, agents: args.agents, @@ -1437,6 +1446,7 @@ mod tests { agent_command: "goose".into(), agent_args: vec!["acp".into()], mcp_command: "".into(), + mcp_servers_json: String::new(), idle_timeout_secs: DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, @@ -2167,10 +2177,26 @@ channels = "ALL" assert!(err.to_string().contains("turn liveness interval must be 0")); } + /// Asserts the *declared* default rather than parsing. `lazy_pool` is bound + /// to `BUZZ_ACP_LAZY_POOL`, so a parse here would read the ambient + /// environment — and every buzz-acp agent exports that variable to its + /// children, so the old parse-based form failed for anyone running the + /// suite from inside a Buzz agent session. #[test] fn lazy_pool_defaults_off() { - let key = "0".repeat(64); - assert!(!CliArgs::parse_from(["buzz-acp", "--private-key", &key]).lazy_pool); + use clap::CommandFactory; + + let cmd = CliArgs::command(); + let arg = cmd + .get_arguments() + .find(|arg| arg.get_id() == "lazy_pool") + .expect("--lazy-pool is declared"); + let defaults: Vec = arg + .get_default_values() + .iter() + .map(|value| value.to_string_lossy().into_owned()) + .collect(); + assert_eq!(defaults, ["false"]); } #[test] diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 403512a322..be28001635 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -885,6 +885,9 @@ fn handle_relay_observer_control_event( Some("switch_model") => { handle_switch_model_control(&payload, pool, observer); } + Some("update_mcp_servers") => { + handle_update_mcp_servers_control(&payload, pool, observer); + } _ => { tracing::debug!(payload = %payload, "ignoring unknown observer control frame"); } @@ -1004,6 +1007,137 @@ fn handle_switch_model_control( } } +/// Handle an `update_mcp_servers` control frame. +/// +/// Records the channel's desired MCP server set. Unlike `switch_model`, this +/// never cancels an in-flight turn: the set is stamped onto the agent by +/// `dispatch_pending` at the next turn boundary, and the session is resumed in +/// place there. The status is therefore always forward-looking. +/// +/// The payload names a command to execute, so this must only ever be reached +/// through the owner-signed, encrypted, freshness-checked observer path. +fn handle_update_mcp_servers_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + observer: Option<&observer::ObserverHandle>, +) { + let Some(channel_id) = payload + .get("channelId") + .and_then(|value| value.as_str()) + .and_then(|value| value.parse::().ok()) + else { + tracing::warn!("observer update_mcp_servers control frame missing valid channelId"); + return; + }; + + let Some(raw_servers) = payload.get("mcpServers") else { + tracing::warn!("observer update_mcp_servers control frame missing mcpServers"); + return; + }; + + // Reject the whole grant on a malformed entry — a partially applied tool + // set is worse than none, and the desktop can re-send. + let servers: Vec = match serde_json::from_value(raw_servers.clone()) { + Ok(servers) => servers, + Err(error) => { + tracing::warn!( + "observer update_mcp_servers control frame has invalid mcpServers: {error}" + ); + if let Some(observer) = observer { + emit_mcp_control_result(observer, channel_id, "invalid_servers"); + } + return; + } + }; + + let status = if pool.desired_mcp_for(&channel_id) == Some(&servers) { + "unchanged" + } else { + pool.set_desired_mcp(channel_id, servers); + "pending_next_turn" + }; + + if let Some(observer) = observer { + emit_mcp_control_result(observer, channel_id, status); + } +} + +fn emit_mcp_control_result(observer: &observer::ObserverHandle, channel_id: Uuid, status: &str) { + observer.emit( + "control_result", + None, + &observer::ObserverContext { + channel_id: Some(channel_id.to_string()), + session_id: None, + turn_id: None, + started_at: None, + }, + serde_json::json!({ + "type": "update_mcp_servers", + "status": status, + }), + ); +} + +#[cfg(test)] +mod mcp_control_tests { + use super::*; + + fn test_pool() -> AgentPool { + AgentPool::from_slots(Vec::new()) + } + + #[test] + fn update_mcp_servers_control_records_the_desired_set() { + let mut pool = test_pool(); + let cid = Uuid::new_v4(); + let payload = serde_json::json!({ + "type": "update_mcp_servers", + "channelId": cid.to_string(), + "mcpServers": [ + {"name": "razorpay", "command": "/usr/bin/rzp", "args": [], "env": []} + ] + }); + + handle_update_mcp_servers_control(&payload, &mut pool, None); + + let recorded = pool.desired_mcp_for(&cid).expect("desired set recorded"); + assert_eq!(recorded.len(), 1); + assert_eq!(recorded[0].name, "razorpay"); + } + + #[test] + fn update_mcp_servers_control_rejects_a_malformed_server_list() { + let mut pool = test_pool(); + let cid = Uuid::new_v4(); + let payload = serde_json::json!({ + "type": "update_mcp_servers", + "channelId": cid.to_string(), + "mcpServers": [{"name": "missing-command"}] + }); + + handle_update_mcp_servers_control(&payload, &mut pool, None); + + assert!( + pool.desired_mcp_for(&cid).is_none(), + "a malformed grant must be rejected outright, never partially applied" + ); + } + + #[test] + fn update_mcp_servers_control_ignores_a_bad_channel_id() { + let mut pool = test_pool(); + let payload = serde_json::json!({ + "type": "update_mcp_servers", + "channelId": "not-a-uuid", + "mcpServers": [] + }); + + handle_update_mcp_servers_control(&payload, &mut pool, None); + // No panic, nothing recorded. + } +} + /// Maximum crashes in a 60-second window before a slot's circuit opens. const CIRCUIT_BREAKER_THRESHOLD: usize = 3; /// Window for circuit-breaker crash counting. @@ -1792,6 +1926,7 @@ async fn tokio_main() -> Result<()> { model_capabilities: None, desired_model: config.model.clone(), model_overridden: false, + desired_mcp: None, agent_name, goose_system_prompt_supported: None, protocol_version, @@ -2937,6 +3072,13 @@ fn dispatch_pending( }; tracing::debug!(agent = agent.index, channel = %channel_id, affinity_hit, "agent_claimed"); + // Turn-boundary MCP application: stamp the channel's desired set onto + // the agent before it is moved into the task. The task compares this + // against `SessionState.applied_mcp` and resumes in place on a + // mismatch. Stamping here — not in the control handler — is what makes + // a live toggle land without cancelling an in-flight turn. + agent.desired_mcp = pool.desired_mcp_for(&channel_id).cloned(); + let recoverable_batch = match ctx.dedup_mode { DedupMode::Queue => Some(batch.clone()), DedupMode::Drop => None, @@ -3837,6 +3979,7 @@ async fn initialize_agent_pool( model_capabilities: None, desired_model: startup.model.clone(), model_overridden: false, + desired_mcp: None, agent_name, goose_system_prompt_supported: None, protocol_version, @@ -4064,7 +4207,9 @@ async fn run_models(args: ModelsArgs) -> Result<()> { // so shutdown() runs on all paths (success, error, timeout). let protocol_result = tokio::time::timeout(MODELS_TIMEOUT, async { let init = client.initialize().await?; - let session = client.session_new_full(&cwd, vec![], None, None).await?; + let session = client + .session_new_full(&cwd, vec![], None, None, false) + .await?; Ok::<_, acp::AcpError>((init, session)) }) .await; @@ -4177,60 +4322,82 @@ 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 = 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, + }); + } } - } - // Forward the agent's display name so dev-mcp can use it as the git - // author name instead of the raw npub. Read from the process env - // rather than Config: this is a pass-through of a contract owned - // upstream, and absent simply means dev-mcp falls back to the npub. - if let Ok(display_name) = std::env::var("BUZZ_ACP_DISPLAY_NAME") { - if !display_name.is_empty() { - env.push(EnvVar { - name: "BUZZ_ACP_DISPLAY_NAME".into(), - value: display_name, - }); + // Forward the agent's display name so dev-mcp can use it as the git + // author name instead of the raw npub. Read from the process env + // rather than Config: this is a pass-through of a contract owned + // upstream, and absent simply means dev-mcp falls back to the npub. + if let Ok(display_name) = std::env::var("BUZZ_ACP_DISPLAY_NAME") { + if !display_name.is_empty() { + env.push(EnvVar { + name: "BUZZ_ACP_DISPLAY_NAME".into(), + value: display_name, + }); + } } - } - env - }, - }] + env + }, + }); + } + + servers.extend(parse_extra_mcp_servers(&config.mcp_servers_json)); + servers +} + +/// Parse the `BUZZ_ACP_MCP_SERVERS` JSON array. +/// +/// Fails open: a malformed value is logged and treated as "no extra servers", +/// so a bad env var degrades to legacy behaviour instead of taking the agent's +/// dev MCP down with it. +fn parse_extra_mcp_servers(raw: &str) -> Vec { + if raw.trim().is_empty() { + return Vec::new(); + } + match serde_json::from_str::>(raw) { + Ok(servers) => servers, + Err(error) => { + tracing::warn!("BUZZ_ACP_MCP_SERVERS is not a valid MCP server array: {error}"); + Vec::new() + } + } } #[cfg(test)] @@ -5000,6 +5167,7 @@ mod build_mcp_servers_tests { agent_command: "goose".into(), agent_args: vec!["acp".into()], mcp_command: "test-mcp-server".into(), + mcp_servers_json: String::new(), idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, @@ -5038,6 +5206,60 @@ mod build_mcp_servers_tests { } } + #[test] + fn build_mcp_servers_appends_json_configured_servers() { + let mut config = test_config(); + config.mcp_command = String::new(); // isolate the JSON path + config.mcp_servers_json = r#"[ + {"name":"razorpay","command":"/usr/bin/rzp","args":["--stdio"],"env":[{"name":"RZP_KEY","value":"secret"}]} + ]"# + .to_string(); + + let servers = build_mcp_servers(&config); + + assert_eq!(servers.len(), 1); + assert_eq!(servers[0].name, "razorpay"); + assert_eq!(servers[0].command, "/usr/bin/rzp"); + assert_eq!(servers[0].args, vec!["--stdio".to_string()]); + assert_eq!(servers[0].env[0].name, "RZP_KEY"); + } + + #[test] + fn build_mcp_servers_keeps_dev_mcp_alongside_json_servers() { + let mut config = test_config(); + config.mcp_command = "/usr/local/bin/buzz-dev-mcp".to_string(); + config.mcp_servers_json = + r#"[{"name":"razorpay","command":"/usr/bin/rzp","args":[],"env":[]}]"#.to_string(); + + let servers = build_mcp_servers(&config); + + let names: Vec<&str> = servers.iter().map(|s| s.name.as_str()).collect(); + assert!( + names.contains(&"buzz-dev-mcp"), + "dev MCP must survive: {names:?}" + ); + assert!( + names.contains(&"razorpay"), + "json server must be added: {names:?}" + ); + } + + #[test] + fn build_mcp_servers_ignores_malformed_json_rather_than_dropping_dev_mcp() { + let mut config = test_config(); + config.mcp_command = "/usr/local/bin/buzz-dev-mcp".to_string(); + config.mcp_servers_json = "{ not valid json".to_string(); + + let servers = build_mcp_servers(&config); + + assert_eq!( + servers.len(), + 1, + "malformed config must fail open to the dev MCP, not panic or wipe the list" + ); + assert_eq!(servers[0].name, "buzz-dev-mcp"); + } + #[test] fn session_new_mcp_server_has_required_fields() { let config = test_config(); @@ -5221,6 +5443,7 @@ mod error_outcome_emission_tests { agent_command: "true".into(), agent_args: vec![], mcp_command: "test-mcp-server".into(), + mcp_servers_json: String::new(), idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, agents: 1, @@ -5288,6 +5511,7 @@ mod error_outcome_emission_tests { model_capabilities: None, desired_model: None, model_overridden: false, + desired_mcp: None, agent_name: "unknown".into(), goose_system_prompt_supported: None, // Error branches under test never read this; 1 is the legacy diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 158477c0af..66e9d2362c 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -103,6 +103,40 @@ pub struct SessionState { /// fetch fails — all fail open. Cleared on session invalidation alongside /// `core_sections` so the next session picks up any canvas change. pub canvas_sections: HashMap, + /// channel_id → the MCP server set the live session for that channel was + /// actually created or resumed with. Compared against the desired set at + /// each turn boundary; a mismatch triggers an in-place `session/resume`. + /// Cleared with the session so a rotated session re-applies from scratch. + pub applied_mcp: HashMap>, + /// Channels whose applied MCP grant has been *proven* to have landed by a + /// tool call from one of its servers. A channel in `applied_mcp` but not + /// here is `applied_unverified` — the ACP wire carries no MCP status, so + /// absence of evidence is never reported as success. Reset whenever the + /// applied set changes: a new grant is unverified again. + pub mcp_verified: HashSet, +} + +/// Promote a channel's MCP grant to *verified* when a tool call proves a +/// granted server is actually mounted. +/// +/// The ACP wire carries no MCP status today (the Claude adapter consumes the +/// SDK's `system`/`init` message, which holds `mcp_servers[{name,status}]`, +/// without forwarding it). A tool call named `mcp____` is +/// therefore the only first-hand evidence available that the grant landed. +/// Absence of evidence is reported as `applied_unverified`, never as success. +fn note_tool_call_for_verification(state: &mut SessionState, channel_id: &Uuid, tool_name: &str) { + let Some(servers) = state.applied_mcp.get(channel_id) else { + return; + }; + // Match the full `mcp____` prefix rather than splitting on `__`: + // a server name may itself contain a double underscore, and splitting would + // silently attribute its tool calls to the wrong server (or to none). + let landed = servers + .iter() + .any(|s| tool_name.starts_with(&format!("mcp__{}__", s.name))); + if landed { + state.mcp_verified.insert(*channel_id); + } } impl SessionState { @@ -125,6 +159,8 @@ impl SessionState { self.turn_counts.remove(channel_id); self.core_sections.remove(channel_id); self.canvas_sections.remove(channel_id); + self.applied_mcp.remove(channel_id); + self.mcp_verified.remove(channel_id); self.sessions.remove(channel_id).is_some() } @@ -136,6 +172,8 @@ impl SessionState { self.heartbeat_turn_count = 0; self.core_sections.clear(); self.canvas_sections.clear(); + self.applied_mcp.clear(); + self.mcp_verified.clear(); } #[cfg(test)] @@ -144,6 +182,7 @@ impl SessionState { || self.turn_counts.contains_key(channel_id) || self.core_sections.contains_key(channel_id) || self.canvas_sections.contains_key(channel_id) + || self.applied_mcp.contains_key(channel_id) } } @@ -161,6 +200,11 @@ pub struct OwnedAgent { /// desktop reader to distinguish a genuine runtime override from a stale /// session whose persona model was edited. Reset on spawn/restart. pub model_overridden: bool, + /// MCP server set desired for the channel this agent is about to serve, + /// stamped by `dispatch_pending` at claim time. `None` means "no managed + /// set for this channel" — the agent uses `PromptContext.mcp_servers`. + /// Runtime-only, re-stamped on every dispatch. + pub desired_mcp: Option>, /// Normalized agent name from initialize (`agentInfo.name`/`serverInfo.name`). pub agent_name: String, /// Whether Goose accepted its custom system-prompt method. `None` probes on @@ -216,6 +260,10 @@ pub struct AgentPool { result_rx: mpsc::UnboundedReceiver, pub join_set: JoinSet<()>, task_map: HashMap, + /// channel_id → desired MCP server set, the authority for what a channel's + /// sessions should run with. Runtime-only (the desktop re-sends on + /// reconnect), matching `desired_model` semantics. + desired_mcp: HashMap>, } /// Result returned by a completed prompt task. @@ -568,9 +616,25 @@ impl AgentPool { result_rx, join_set: JoinSet::new(), task_map: HashMap::new(), + desired_mcp: HashMap::new(), } } + /// Record the desired MCP server set for a channel. + /// + /// Takes effect at the channel's next turn boundary: `dispatch_pending` + /// stamps it onto the claimed agent, and the session lookup resumes the + /// live session in place if it differs from what was applied. Never + /// disturbs an in-flight turn. + pub fn set_desired_mcp(&mut self, channel_id: Uuid, servers: Vec) { + self.desired_mcp.insert(channel_id, servers); + } + + /// The desired MCP server set for a channel, if one has been recorded. + pub fn desired_mcp_for(&self, channel_id: &Uuid) -> Option<&Vec> { + self.desired_mcp.get(channel_id) + } + /// Try to claim an idle agent for the given channel (or heartbeat if `None`). /// /// Pass 1: prefer an agent that already has a session for `channel_id`. @@ -869,6 +933,68 @@ async fn resolve_new_session_channel_context( /// On error from `session_new_full()`, returns the `AcpError` — caller handles /// error reporting. Model-switch failures are logged and gracefully ignored /// (the agent proceeds with its default model). +/// The MCP server set a channel should run with: the desired set when the +/// channel is managed, otherwise the harness-wide set from the prompt context. +fn effective_mcp_servers<'a>(agent: &'a OwnedAgent, ctx: &'a PromptContext) -> &'a Vec { + agent.desired_mcp.as_ref().unwrap_or(&ctx.mcp_servers) +} + +/// Reconcile a channel's live session against its desired MCP set, at the turn +/// boundary and before the session is used. +/// +/// The desired set is stamped onto the agent by `dispatch_pending`; +/// `applied_mcp` records what the live session was actually built with. On a +/// mismatch this reconfigures the session in place — same session ID, full +/// transcript preserved by the adapter's resume, new tool set. When in-place +/// reconfiguration is impossible or fails, the channel's session is dropped so +/// the caller's existing creation path builds a fresh one with the desired set. +/// +/// A no-op when the sets already match or the channel has no live session. +async fn reconcile_channel_mcp(agent: &mut OwnedAgent, ctx: &PromptContext, cid: &Uuid) { + let Some(sid) = agent.state.sessions.get(cid).cloned() else { + return; + }; + let desired = effective_mcp_servers(agent, ctx).clone(); + if agent.state.applied_mcp.get(cid) == Some(&desired) { + return; + } + + if !agent.acp.resume_supported() { + tracing::info!( + target: "pool::session", + "agent does not support session/resume; rotating channel {cid} to apply its MCP set" + ); + agent.state.invalidate_channel(cid); + return; + } + + match agent + .acp + .session_resume(&sid, &ctx.cwd, desired.clone()) + .await + { + Ok(_) => { + agent.state.applied_mcp.insert(*cid, desired); + // A new grant is unverified until a tool call proves it landed. + agent.state.mcp_verified.remove(cid); + tracing::info!( + target: "pool::session", + "resumed session {sid} for channel {cid} with a new MCP set" + ); + } + Err(error) => { + // Never fail the turn over a tool grant: drop the session so the + // caller creates a fresh one with the desired set. Conversation + // continuity is lost, which is strictly better than a lost turn. + tracing::warn!( + target: "pool::session", + "session/resume failed for channel {cid} ({error}); rotating instead" + ); + agent.state.invalidate_channel(cid); + } + } +} + async fn create_session_and_apply_model( agent: &mut OwnedAgent, ctx: &PromptContext, @@ -899,17 +1025,27 @@ async fn create_session_and_apply_model( .as_deref() .map(|agent_name| compose_session_title(agent_name, channel_name)); + // Resolve before the mutable borrow below: passing + // `effective_mcp_servers(agent, ctx)` inline as an argument would mix an + // immutable borrow of `agent` with the mutable receiver borrow of + // `agent.acp` in one expression. + let mcp_servers = effective_mcp_servers(agent, ctx).clone(); + // A channel is *managed* exactly when it has a desired set. Only then may + // we suppress the agent's own global MCP config. + let strict_mcp = agent.desired_mcp.is_some(); + let resp = agent .acp .session_new_full( &ctx.cwd, - ctx.mcp_servers.clone(), + mcp_servers, session_new_system_prompt( is_goose, agent.protocol_version, combined_system_prompt.as_deref(), ), session_title.as_deref(), + strict_mcp, ) .await?; @@ -1544,6 +1680,11 @@ pub async fn run_prompt_task( let (session_id, is_new_session) = match &source { PromptSource::Channel(cid) => { + // Turn-boundary MCP application. Either the live session is resumed + // in place with the new set, or it is dropped and the creation path + // below rebuilds it. Never disturbs an in-flight turn. + reconcile_channel_mcp(&mut agent, &ctx, cid).await; + if let Some(sid) = agent.state.sessions.get(cid) { (sid.clone(), false) } else { @@ -1565,7 +1706,14 @@ pub async fn run_prompt_task( target: "pool::session", "created session {sid} for channel {cid}" ); + // Hoist the resolve before the mutable borrow: calling + // `effective_mcp_servers(&agent, ..)` inline as an argument to + // `agent.state.applied_mcp.insert(..)` mixes an immutable and a + // mutable borrow of `agent` in one expression. Do not inline it. + let applied = effective_mcp_servers(&agent, &ctx).clone(); agent.state.sessions.insert(*cid, sid.clone()); + agent.state.applied_mcp.insert(*cid, applied); + agent.state.mcp_verified.remove(cid); // Commit canvas only after session creation succeeds (I3). if let Some((pending_cid, section)) = pending_canvas.take() { agent.state.canvas_sections.insert(pending_cid, section); @@ -2084,6 +2232,15 @@ pub async fn run_prompt_task( Ok(stop_reason) => { log_stop_reason(&source, &stop_reason); + // Opportunistic MCP-grant verification: a tool call named + // `mcp____` is the only first-hand evidence on the ACP + // wire that a granted server actually mounted. + if let PromptSource::Channel(cid) = &source { + for tool_name in agent.acp.take_observed_tool_names() { + note_tool_call_for_verification(&mut agent.state, cid, &tool_name); + } + } + let should_rotate = matches!( stop_reason, StopReason::MaxTokens | StopReason::MaxTurnRequests @@ -3965,6 +4122,166 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; use serde_json::json; + /// An empty pool — enough to exercise the channel-keyed maps that need no + /// agent slots. `from_slots` avoids spawning subprocesses. + fn test_pool() -> AgentPool { + AgentPool::from_slots(Vec::new()) + } + + fn test_mcp_servers() -> Vec { + vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec![], + env: vec![], + }] + } + + #[test] + fn invalidate_channel_clears_applied_mcp() { + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.sessions.insert(cid, "ses_1".to_string()); + state.applied_mcp.insert(cid, test_mcp_servers()); + + assert!(state.invalidate_channel(&cid)); + + assert!( + !state.applied_mcp.contains_key(&cid), + "a dropped session must not leave a stale applied-MCP record behind" + ); + } + + #[test] + fn set_desired_mcp_is_readable_per_channel() { + let mut pool = test_pool(); + let cid = Uuid::new_v4(); + let servers = test_mcp_servers(); + + pool.set_desired_mcp(cid, servers.clone()); + + assert_eq!(pool.desired_mcp_for(&cid), Some(&servers)); + assert_eq!(pool.desired_mcp_for(&Uuid::new_v4()), None); + } + + #[test] + fn a_tool_call_from_a_granted_server_marks_the_grant_verified() { + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.applied_mcp.insert(cid, test_mcp_servers()); + + assert!(!state.mcp_verified.contains(&cid)); + + // MCP tools surface as `mcp____`. + note_tool_call_for_verification(&mut state, &cid, "mcp__razorpay__create_order"); + + assert!( + state.mcp_verified.contains(&cid), + "a tool call from a granted server is proof the grant landed" + ); + } + + #[test] + fn an_unrelated_tool_call_does_not_verify_the_grant() { + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.applied_mcp.insert(cid, test_mcp_servers()); + + note_tool_call_for_verification(&mut state, &cid, "Read"); + + assert!( + !state.mcp_verified.contains(&cid), + "a built-in tool call proves nothing about the MCP grant" + ); + } + + /// A minimal agent over a sleeping subprocess — enough to read the plain + /// fields. Mirrors the `OwnedAgent` literals in the steer tests below. + async fn test_owned_agent() -> OwnedAgent { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn test agent"); + OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + desired_mcp: None, + model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + } + } + + #[tokio::test] + async fn effective_mcp_servers_prefers_the_desired_set() { + let ctx_servers = vec![McpServer { + name: "buzz-dev-mcp".into(), + command: "/usr/local/bin/buzz-dev-mcp".into(), + args: vec![], + env: vec![], + }]; + let desired = test_mcp_servers(); + + let mut ctx = make_prompt_context_no_owner(); + ctx.mcp_servers = ctx_servers.clone(); + let mut agent = test_owned_agent().await; + + agent.desired_mcp = None; + assert_eq!( + effective_mcp_servers(&agent, &ctx), + &ctx_servers, + "unmanaged channel must keep legacy behaviour" + ); + + agent.desired_mcp = Some(desired.clone()); + assert_eq!( + effective_mcp_servers(&agent, &ctx), + &desired, + "a managed channel must use exactly the desired set" + ); + } + + #[test] + fn identical_sets_do_not_trigger_a_resume() { + // Guards against gratuitous subprocess respawns: the desired set + // equalling the applied set must be a no-op, not a resume. + let servers = vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec!["--stdio".into()], + env: vec![], + }]; + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.applied_mcp.insert(cid, servers.clone()); + + assert_eq!( + state.applied_mcp.get(&cid), + Some(&servers), + "equality must hold for identical sets so no resume is issued" + ); + + let reordered_args = vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec!["--other".into()], + env: vec![], + }]; + assert_ne!( + state.applied_mcp.get(&cid), + Some(&reordered_args), + "an args change must be detected — the adapter fingerprints args too" + ); + } + // These pin the initial_message dispatch path (run_prompt_task, ~line 855): // a legacy agent WITH a base_prompt must get [Base] prepended to the user // message. This is the exact regression that shipped in the round-2 bug. @@ -5876,6 +6193,7 @@ mod tests { model_capabilities: None, desired_model: None, model_overridden: false, + desired_mcp: None, agent_name: "unknown".into(), goose_system_prompt_supported: None, protocol_version: 2, @@ -5934,6 +6252,7 @@ mod tests { model_capabilities: None, desired_model: None, model_overridden: false, + desired_mcp: None, agent_name: "unknown".into(), goose_system_prompt_supported: None, protocol_version: 2, diff --git a/docs/superpowers/plans/2026-07-31-live-mcp-control-buzz-acp.md b/docs/superpowers/plans/2026-07-31-live-mcp-control-buzz-acp.md new file mode 100644 index 0000000000..e51c7c58d0 --- /dev/null +++ b/docs/superpowers/plans/2026-07-31-live-mcp-control-buzz-acp.md @@ -0,0 +1,1396 @@ +# Live per-channel MCP control (`buzz-acp`) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a Buzz channel's MCP server set change while the agent keeps the *same* conversation — via `session/resume` with an unchanged sessionId and a new `mcpServers` list — and stop injecting the agent's unused global MCP schemas into every turn. + +**Architecture:** The pool holds a per-channel *desired* MCP set; each live session records the set it was *applied* with. At the next turn boundary the two are compared, and on a mismatch the harness sends `session/resume` (same sessionId, new servers). Both flagship ACP adapters implement reconfigure-on-resume by fingerprinting `(cwd, mcpServers)` and recreating their subprocess with `resume`, which reloads the full transcript from disk. Agents that don't advertise `sessionCapabilities.resume` fall back to today's invalidate-and-rotate path. + +**Tech Stack:** Rust (tokio, serde, clap), ACP JSON-RPC over stdio. Crate: `crates/buzz-acp`. + +**Source spec:** `docs/superpowers/specs/2026-07-31-live-mcp-control-design.md` — every citation in it was re-verified against shipped source before this plan was written (see *Verification log* at the end, including three corrections). + +**Scope:** This plan covers **only `crates/buzz-acp`**. The desktop UI (§5.3 of the spec) is a separate plan — this one delivers working, testable software on its own: the harness gains multi-server config, a live control lever, and the token fix, all drivable via env var + control frame without any UI. + +## Global Constraints + +- No `unsafe` code anywhere. +- No new `unwrap()` / `expect()` in production paths — use `?` and proper error types. (Existing `expect` on bech32 encoding is pre-existing and stays.) +- Every commit uses `git commit -s` (DCO trailer required; CI fails without it). +- New public API must have doc comments. +- Activate the toolchain before any build/test/git command: `. ./bin/activate-hermit`. +- `just ci` must pass before the PR (fmt + clippy + desktop lint + unit tests). Clippy passing does not mean fmt passes — run both. +- **Additive only.** Mirror existing in-crate patterns: `session_resume` mirrors `session_new_full`; the control handler mirrors `handle_switch_model_control`; capability recording mirrors `steering_supported`. +- **MCP changes apply at turn boundaries and must NEVER cancel an in-flight turn.** Unlike `switch_model`, there is no busy-path oneshot. +- **`strictMcpConfig` is opt-in per managed channel.** Defaulting it on would silently strip users' global MCP servers. +- Run tests with `cargo test -p buzz-acp`; the desktop crate is excluded from the root workspace. + +--- + +### Task 1: `session_resume` + resume capability detection + +**Files:** +- Modify: `crates/buzz-acp/src/acp.rs:200` (field), `:550` (init), `:603` (initialize), `:655` (new method after `session_new_full`), `:849` (accessor) +- Test: `crates/buzz-acp/src/acp.rs` (in-file `mod tests`, beside `session_new_full_includes_system_prompt_when_some` at :3258) + +**Interfaces:** +- Produces: `AcpClient::session_resume(&mut self, session_id: &str, cwd: &str, mcp_servers: Vec) -> Result` and `AcpClient::resume_supported(&self) -> bool`. Task 4 calls both. + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `acp.rs`: + +```rust +#[tokio::test] +async fn session_resume_request_includes_session_id_cwd_and_mcp_servers() { + 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,"result":{"_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client.initialize().await.expect("initialize should succeed"); + + let result = client + .session_resume( + "ses_test", + "/tmp", + vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec!["--stdio".into()], + env: vec![], + }], + ) + .await + .expect("session_resume should succeed"); + + let received = &result["_receivedRequest"]; + assert_eq!(received["method"].as_str(), Some("session/resume")); + assert_eq!(received["params"]["sessionId"].as_str(), Some("ses_test")); + assert_eq!(received["params"]["cwd"].as_str(), Some("/tmp")); + assert_eq!( + received["params"]["mcpServers"][0]["name"].as_str(), + Some("razorpay"), + "mcpServers must ride on the resume request — this is the whole mechanism" + ); +} + +#[tokio::test] +async fn initialize_records_resume_supported_when_advertised() { + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{"sessionCapabilities":{"resume":{}}}}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client.initialize().await.expect("initialize should succeed"); + assert!( + client.resume_supported(), + "an agent advertising sessionCapabilities.resume must be detected" + ); +} + +#[tokio::test] +async fn initialize_records_resume_unsupported_when_absent() { + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{"sessionCapabilities":{}}}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client.initialize().await.expect("initialize should succeed"); + assert!( + !client.resume_supported(), + "absent resume capability must not be treated as supported" + ); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp session_resume 2>&1 | tail -20 +. ./bin/activate-hermit && cargo test -p buzz-acp resume_supported 2>&1 | tail -20 +``` + +Expected: FAIL — `no method named session_resume` / `no method named resume_supported`. + +- [ ] **Step 3: Add the capability field** + +In `acp.rs`, beside `steering_supported: bool` (~line 200): + +```rust + /// Whether the agent advertised `agentCapabilities.sessionCapabilities.resume` + /// in its initialize response. Gates the in-place MCP reconfiguration path; + /// call sites fall back to session invalidation when false. + resume_supported: bool, +``` + +In the constructor beside `steering_supported: false` (~line 550): + +```rust + resume_supported: false, +``` + +- [ ] **Step 4: Record the capability in `initialize()`** + +In `initialize()`, immediately after the existing `self.steering_supported = ...` assignment (~line 603): + +```rust + // Session-level resume capability. The value is an empty object (`{}`) + // when supported, so presence — not truthiness — is the signal. + self.resume_supported = result + .pointer("/agentCapabilities/sessionCapabilities/resume") + .map(|v| !v.is_null()) + .unwrap_or(false); +``` + +- [ ] **Step 5: Add the accessor** + +Beside `steering_supported()` (~line 849): + +```rust + /// Whether the connected agent supports `session/resume`. + pub fn resume_supported(&self) -> bool { + self.resume_supported + } +``` + +- [ ] **Step 6: Add `session_resume`** + +Immediately after `session_new` (~line 672): + +```rust + /// Send `session/resume` to reconfigure a live session in place. + /// + /// The session ID is unchanged. Adapters that fingerprint the + /// session-defining params (`cwd` + `mcpServers`) tear down and recreate + /// the underlying agent process with `resume`, restoring the full + /// conversation transcript from disk — this is how an MCP grant lands + /// without losing the conversation. + /// + /// `cwd` must be an absolute path. `mcp_servers` may be empty. Gated on + /// [`resume_supported`](Self::resume_supported): callers must fall back to + /// session invalidation when the agent does not advertise the capability. + /// + /// Prefer this over `session/load` — `load` replays the whole history as + /// `session/update` notifications before responding, which risks the + /// request timeout on long conversations. Reconfiguration semantics are + /// identical. + pub async fn session_resume( + &mut self, + session_id: &str, + cwd: &str, + mcp_servers: Vec, + ) -> Result { + let params = serde_json::json!({ + "sessionId": session_id, + "cwd": cwd, + "mcpServers": mcp_servers, + }); + self.send_request("session/resume", params).await + } +``` + +- [ ] **Step 7: Run tests to verify they pass** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp session_resume resume_supported 2>&1 | tail -20 +``` + +Expected: 3 passed. + +- [ ] **Step 8: Commit** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/src/acp.rs +git commit -s -m "feat(acp): session/resume request + resume capability detection" +``` + +--- + +### Task 2: Multi-server MCP configuration + +**Files:** +- Modify: `crates/buzz-acp/src/acp.rs:27-40` (derives on `McpServer` / `EnvVar`) +- Modify: `crates/buzz-acp/src/config.rs:261` (clap arg), `:497` (Config field), `:1061` (mapping) +- Modify: `crates/buzz-acp/src/lib.rs:4179` (`build_mcp_servers`) +- Test: `crates/buzz-acp/src/lib.rs` (`mod build_mcp_servers_tests` at :4989) + +**Interfaces:** +- Consumes: `McpServer` from Task 1's test usage (unchanged shape). +- Produces: `McpServer: Deserialize + PartialEq`; `Config.mcp_servers_json: String`; `build_mcp_servers(config) -> Vec` now returns the dev-MCP server **plus** any servers parsed from `BUZZ_ACP_MCP_SERVERS`. Tasks 3–5 rely on `PartialEq` for the desired-vs-applied comparison and on `Deserialize` for control-frame parsing. + +**Why `Deserialize` is needed:** `McpServer` is currently `Serialize`-only. Both the new env-var config and the Task 5 control frame arrive as JSON that must be parsed *into* it. + +- [ ] **Step 1: Write the failing test** + +Add to `mod build_mcp_servers_tests` in `lib.rs`: + +```rust + #[test] + fn build_mcp_servers_appends_json_configured_servers() { + let mut config = test_config(); + config.mcp_command = String::new(); // isolate the JSON path + config.mcp_servers_json = r#"[ + {"name":"razorpay","command":"/usr/bin/rzp","args":["--stdio"],"env":[{"name":"RZP_KEY","value":"secret"}]} + ]"# + .to_string(); + + let servers = build_mcp_servers(&config); + + assert_eq!(servers.len(), 1); + assert_eq!(servers[0].name, "razorpay"); + assert_eq!(servers[0].command, "/usr/bin/rzp"); + assert_eq!(servers[0].args, vec!["--stdio".to_string()]); + assert_eq!(servers[0].env[0].name, "RZP_KEY"); + } + + #[test] + fn build_mcp_servers_keeps_dev_mcp_alongside_json_servers() { + let mut config = test_config(); + config.mcp_command = "/usr/local/bin/buzz-dev-mcp".to_string(); + config.mcp_servers_json = + r#"[{"name":"razorpay","command":"/usr/bin/rzp","args":[],"env":[]}]"#.to_string(); + + let servers = build_mcp_servers(&config); + + let names: Vec<&str> = servers.iter().map(|s| s.name.as_str()).collect(); + assert!(names.contains(&"buzz-dev-mcp"), "dev MCP must survive: {names:?}"); + assert!(names.contains(&"razorpay"), "json server must be added: {names:?}"); + } + + #[test] + fn build_mcp_servers_ignores_malformed_json_rather_than_dropping_dev_mcp() { + let mut config = test_config(); + config.mcp_command = "/usr/local/bin/buzz-dev-mcp".to_string(); + config.mcp_servers_json = "{ not valid json".to_string(); + + let servers = build_mcp_servers(&config); + + assert_eq!( + servers.len(), + 1, + "malformed config must fail open to the dev MCP, not panic or wipe the list" + ); + assert_eq!(servers[0].name, "buzz-dev-mcp"); + } +``` + +> Use the module's existing `test_config()` helper — if it is named differently in `mod build_mcp_servers_tests`, use whatever helper the neighbouring tests (`lib.rs:5044`, `:5065`) already call to build a `Config`. + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp build_mcp_servers 2>&1 | tail -20 +``` + +Expected: FAIL — `no field mcp_servers_json on type Config`. + +- [ ] **Step 3: Add the derives** + +In `acp.rs`, replace the derive lines on `McpServer` (line 27) and `EnvVar` (line 36): + +```rust +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct McpServer { +``` + +```rust +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct EnvVar { +``` + +- [ ] **Step 4: Add the config field** + +In `config.rs`, after the `mcp_command` arg (line 261-262): + +```rust + /// Additional MCP servers as a JSON array of + /// `{name, command, args, env:[{name,value}]}` objects. Merged with the + /// dev-MCP server derived from `--mcp-command`. Malformed JSON is logged + /// and ignored (fail open) rather than dropping the dev MCP. + #[arg(long, env = "BUZZ_ACP_MCP_SERVERS", default_value = "")] + pub mcp_servers_json: String, +``` + +In the `Config` struct after `pub mcp_command: String,` (line 497): + +```rust + pub mcp_servers_json: String, +``` + +In the `Config { .. }` construction after `mcp_command: args.mcp_command,` (line 1061): + +```rust + mcp_servers_json: args.mcp_servers_json, +``` + +- [ ] **Step 5: Merge the JSON servers in `build_mcp_servers`** + +In `lib.rs`, restructure `build_mcp_servers` (line 4179). Keep the entire existing dev-MCP body; change only the early return and the tail: + +```rust +fn build_mcp_servers(config: &Config) -> Vec { + let mut servers: Vec = Vec::new(); + + if !config.mcp_command.is_empty() { + servers.push(McpServer { + // ... existing dev-MCP construction, unchanged ... + }); + } + + servers.extend(parse_extra_mcp_servers(&config.mcp_servers_json)); + servers +} + +/// Parse the `BUZZ_ACP_MCP_SERVERS` JSON array. +/// +/// Fails open: a malformed value is logged and treated as "no extra servers", +/// so a bad env var degrades to legacy behaviour instead of taking the agent's +/// dev MCP down with it. +fn parse_extra_mcp_servers(raw: &str) -> Vec { + if raw.trim().is_empty() { + return Vec::new(); + } + match serde_json::from_str::>(raw) { + Ok(servers) => servers, + Err(error) => { + tracing::warn!("BUZZ_ACP_MCP_SERVERS is not a valid MCP server array: {error}"); + Vec::new() + } + } +} +``` + +- [ ] **Step 6: Run tests to verify they pass** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp build_mcp_servers 2>&1 | tail -20 +``` + +Expected: all `build_mcp_servers_*` tests pass, including the pre-existing ones. + +- [ ] **Step 7: Commit** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/src/acp.rs crates/buzz-acp/src/config.rs crates/buzz-acp/src/lib.rs +git commit -s -m "feat(acp): configure multiple MCP servers via BUZZ_ACP_MCP_SERVERS" +``` + +--- + +### Task 3: Per-channel desired/applied MCP state + +**Files:** +- Modify: `crates/buzz-acp/src/pool.rs:~105` (`SessionState.applied_mcp`), `:124` (`invalidate_channel`), `:137` (`invalidate_all`), `:~170` (`OwnedAgent.desired_mcp`), `:599` (`return_agent`) +- Modify: `crates/buzz-acp/src/lib.rs:2928` (stamp at dispatch), `:1793` + `:3838` + `:5289` (struct literals gain the new field) +- Test: `crates/buzz-acp/src/pool.rs` (in-file `mod tests`) + +**Interfaces:** +- Consumes: `McpServer: PartialEq` from Task 2. +- Produces: + - `AgentPool.desired_mcp: HashMap>` (pool-owned authority, runtime-only) with `AgentPool::set_desired_mcp(&mut self, channel_id: Uuid, servers: Vec)` and `AgentPool::desired_mcp_for(&self, channel_id: &Uuid) -> Option<&Vec>`. + - `OwnedAgent.desired_mcp: Option>` — the set stamped onto the agent for the channel it is about to serve. + - `SessionState.applied_mcp: HashMap>` — what each live channel session was actually built with. + +**Design note (deviation from spec §5.1/§5.2, deliberate):** the spec proposes a `mcp_dirty: HashSet` plus an eager "apply NOW if idle" path. This plan instead compares *desired vs applied* at the turn boundary. That is strictly less state (no dirty set to set, clear, or leak), and it is more correct: a dirty flag set on agent A is invisible when agent B picks the channel up, whereas the comparison is self-healing across agent swaps. It also removes the need to `await` a resume from the synchronous control handler. User-visible outcome is unchanged — same session, full transcript, new tools on the next turn. + +- [ ] **Step 1: Write the failing test** + +Add to `mod tests` in `pool.rs`: + +```rust + #[test] + fn invalidate_channel_clears_applied_mcp() { + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.sessions.insert(cid, "ses_1".to_string()); + state.applied_mcp.insert( + cid, + vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec![], + env: vec![], + }], + ); + + assert!(state.invalidate_channel(&cid)); + + assert!( + !state.applied_mcp.contains_key(&cid), + "a dropped session must not leave a stale applied-MCP record behind" + ); + } + + #[test] + fn set_desired_mcp_is_readable_per_channel() { + let mut pool = test_pool(); + let cid = Uuid::new_v4(); + let servers = vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec![], + env: vec![], + }]; + + pool.set_desired_mcp(cid, servers.clone()); + + assert_eq!(pool.desired_mcp_for(&cid), Some(&servers)); + assert_eq!(pool.desired_mcp_for(&Uuid::new_v4()), None); + } +``` + +> `test_pool()` — use whichever pool constructor the neighbouring tests in `pool.rs` already use (see the `desired_model: None` struct literals at `pool.rs:5877` / `:5935` for the shape). + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp applied_mcp desired_mcp 2>&1 | tail -20 +``` + +Expected: FAIL — `no field applied_mcp` / `no method set_desired_mcp`. + +- [ ] **Step 3: Add `applied_mcp` to `SessionState`** + +In `pool.rs`, after the `canvas_sections` field (~line 105): + +```rust + /// channel_id → the MCP server set the live session for that channel was + /// actually created or resumed with. Compared against the desired set at + /// each turn boundary; a mismatch triggers an in-place `session/resume`. + /// Cleared with the session so a rotated session re-applies from scratch. + pub applied_mcp: HashMap>, +``` + +In `invalidate_channel` (line 124), beside the other `remove` calls: + +```rust + self.applied_mcp.remove(channel_id); +``` + +In `invalidate_all` (line 137), beside the other `clear` calls: + +```rust + self.applied_mcp.clear(); +``` + +In the `#[cfg(test)] fn has_channel_state`, add the new map so the existing invariant test keeps covering it: + +```rust + || self.applied_mcp.contains_key(channel_id) +``` + +- [ ] **Step 4: Add `desired_mcp` to `OwnedAgent`** + +In `pool.rs`, after the `model_overridden` field (~line 170): + +```rust + /// MCP server set desired for the channel this agent is about to serve, + /// stamped by `dispatch_pending` at claim time. `None` means "no managed + /// set for this channel" — the agent uses `PromptContext.mcp_servers`. + /// Runtime-only, re-stamped on every dispatch. + pub desired_mcp: Option>, +``` + +Add `desired_mcp: None,` to every `OwnedAgent { .. }` literal. Compiler will point at each; the known sites are `lib.rs:1793`, `lib.rs:3838`, `lib.rs:5289`, `pool.rs:5877`, `pool.rs:5935`. + +- [ ] **Step 5: Add the pool map and accessors** + +In `pool.rs`, add to the `AgentPool` struct: + +```rust + /// channel_id → desired MCP server set, the authority for what a channel's + /// sessions should run with. Runtime-only (the desktop re-sends on + /// reconnect), matching `desired_model` semantics. + desired_mcp: HashMap>, +``` + +Initialise it to `HashMap::new()` in the pool constructor, and add the accessors near `switch_idle_agent_model` (line 741): + +```rust + /// Record the desired MCP server set for a channel. + /// + /// Takes effect at the channel's next turn boundary: `dispatch_pending` + /// stamps it onto the claimed agent, and the session lookup resumes the + /// live session in place if it differs from what was applied. Never + /// disturbs an in-flight turn. + pub fn set_desired_mcp(&mut self, channel_id: Uuid, servers: Vec) { + self.desired_mcp.insert(channel_id, servers); + } + + /// The desired MCP server set for a channel, if one has been recorded. + pub fn desired_mcp_for(&self, channel_id: &Uuid) -> Option<&Vec> { + self.desired_mcp.get(channel_id) + } +``` + +- [ ] **Step 6: Stamp the agent at dispatch** + +In `lib.rs` `dispatch_pending`, immediately after the `tracing::debug!(agent = agent.index, ... "agent_claimed");` line (~2928): + +```rust + // Turn-boundary MCP application: stamp the channel's desired set onto + // the agent before it is moved into the task. The task compares this + // against `SessionState.applied_mcp` and resumes in place on a + // mismatch. Stamping here — not in the control handler — is what makes + // a live toggle land without cancelling an in-flight turn. + agent.desired_mcp = pool.desired_mcp_for(&channel_id).cloned(); +``` + +- [ ] **Step 7: Run tests to verify they pass** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp 2>&1 | tail -20 +``` + +Expected: new tests pass; whole crate still green. + +- [ ] **Step 8: Commit** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/src/pool.rs crates/buzz-acp/src/lib.rs +git commit -s -m "feat(acp): track desired and applied MCP sets per channel" +``` + +--- + +### Task 4: Resume-swap at the turn boundary + +**Files:** +- Modify: `crates/buzz-acp/src/pool.rs:1546-1556` (existing-session branch of the session lookup), `:872-910` (`create_session_and_apply_model` records applied set) +- Test: `crates/buzz-acp/src/pool.rs` (in-file `mod tests`) + +**Interfaces:** +- Consumes: `AcpClient::session_resume` + `AcpClient::resume_supported` (Task 1); `OwnedAgent.desired_mcp`, `SessionState.applied_mcp` (Task 3). +- Produces: `fn effective_mcp_servers<'a>(agent: &'a OwnedAgent, ctx: &'a PromptContext) -> &'a Vec` — the single place that resolves "which servers should this channel run with". + +- [ ] **Step 1: Write the failing test** + +Add to `mod tests` in `pool.rs`: + +```rust + #[test] + fn effective_mcp_servers_prefers_the_desired_set() { + let ctx_servers = vec![McpServer { + name: "buzz-dev-mcp".into(), + command: "/usr/local/bin/buzz-dev-mcp".into(), + args: vec![], + env: vec![], + }]; + let desired = vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec![], + env: vec![], + }]; + + let ctx = test_prompt_context(ctx_servers.clone()); + let mut agent = test_owned_agent(); + + agent.desired_mcp = None; + assert_eq!( + effective_mcp_servers(&agent, &ctx), + &ctx_servers, + "unmanaged channel must keep legacy behaviour" + ); + + agent.desired_mcp = Some(desired.clone()); + assert_eq!( + effective_mcp_servers(&agent, &ctx), + &desired, + "a managed channel must use exactly the desired set" + ); + } + + #[test] + fn identical_sets_do_not_trigger_a_resume() { + // Guards against gratuitous subprocess respawns: the desired set + // equalling the applied set must be a no-op, not a resume. + let servers = vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec!["--stdio".into()], + env: vec![], + }]; + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.applied_mcp.insert(cid, servers.clone()); + + assert_eq!( + state.applied_mcp.get(&cid), + Some(&servers), + "equality must hold for identical sets so no resume is issued" + ); + + let reordered_args = vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec!["--other".into()], + env: vec![], + }]; + assert_ne!( + state.applied_mcp.get(&cid), + Some(&reordered_args), + "an args change must be detected — the adapter fingerprints args too" + ); + } +``` + +> `test_prompt_context(..)` / `test_owned_agent()` — reuse or extend whatever helpers `mod tests` already has for building a `PromptContext` and `OwnedAgent`; do not invent new infrastructure. + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp effective_mcp_servers identical_sets 2>&1 | tail -20 +``` + +Expected: FAIL — `cannot find function effective_mcp_servers`. + +- [ ] **Step 3: Add the resolver and record the applied set on creation** + +In `pool.rs`, add near `create_session_and_apply_model` (line 867): + +```rust +/// The MCP server set a channel should run with: the desired set when the +/// channel is managed, otherwise the harness-wide set from the prompt context. +fn effective_mcp_servers<'a>(agent: &'a OwnedAgent, ctx: &'a PromptContext) -> &'a Vec { + agent.desired_mcp.as_ref().unwrap_or(&ctx.mcp_servers) +} +``` + +In `create_session_and_apply_model`, replace `ctx.mcp_servers.clone()` (line 906) with: + +```rust + effective_mcp_servers(agent, ctx).clone(), +``` + +and, after the session is successfully created, record what was applied so the +next turn's comparison has a baseline. (`create_session_and_apply_model` +returns the session id; the caller inserts into `agent.state.sessions`, so +record alongside that insert in Step 4.) + +- [ ] **Step 4: Resume in place when the sets differ** + +In `pool.rs` `run_prompt_task`, replace the existing-session arm of the channel branch (line ~1546): + +```rust + PromptSource::Channel(cid) => { + if let Some(sid) = agent.state.sessions.get(cid) { + (sid.clone(), false) + } else { +``` + +with: + +```rust + PromptSource::Channel(cid) => { + if let Some(sid) = agent.state.sessions.get(cid).cloned() { + // Turn-boundary MCP reconciliation. The desired set is stamped + // by `dispatch_pending`; `applied_mcp` is what this session was + // actually built with. On a mismatch, reconfigure the live + // session in place: same sessionId, full transcript preserved + // by the adapter's resume, new tool set. + let desired = effective_mcp_servers(&agent, &ctx).clone(); + if agent.state.applied_mcp.get(cid) != Some(&desired) { + if agent.acp.resume_supported() { + match agent.acp.session_resume(&sid, &ctx.cwd, desired.clone()).await { + Ok(_) => { + agent.state.applied_mcp.insert(*cid, desired); + tracing::info!( + target: "pool::session", + "resumed session {sid} for channel {cid} with a new MCP set" + ); + } + Err(error) => { + // Never fail the turn over a tool grant: drop the + // session so the code below creates a fresh one + // with the desired set. Conversation continuity is + // lost, which is strictly better than a lost turn. + tracing::warn!( + target: "pool::session", + "session/resume failed for channel {cid} ({error}); rotating instead" + ); + agent.state.invalidate_channel(cid); + } + } + } else { + tracing::info!( + target: "pool::session", + "agent does not support session/resume; rotating channel {cid} to apply its MCP set" + ); + agent.state.invalidate_channel(cid); + } + } + + match agent.state.sessions.get(cid) { + Some(sid) => (sid.clone(), false), + None => { + // Rotated above — fall through to creation. + match create_session_and_apply_model( + &mut agent, + &ctx, + agent_core.as_deref(), + agent_canvas.as_deref(), + title_channel.as_deref(), + ) + .await + { + Ok(sid) => { + // Hoist the resolve before the mutable borrow: calling + // `effective_mcp_servers(&agent, ..)` inline as an argument to + // `agent.state.applied_mcp.insert(..)` mixes an immutable and a + // mutable borrow of `agent` in one expression. Do not inline it. + let applied = effective_mcp_servers(&agent, &ctx).clone(); + agent.state.sessions.insert(*cid, sid.clone()); + agent.state.applied_mcp.insert(*cid, applied); + (sid, true) + } + Err(AcpError::AgentExited) => { + agent.state.invalidate_all(); + send_prompt_result( + &result_tx, + &turn_id, + agent, + source, + PromptOutcome::AgentExited, + requeue_batch_if_queue(&ctx, batch), + ); + return; + } + Err(e) => { + send_prompt_result( + &result_tx, + &turn_id, + agent, + source, + PromptOutcome::Error(e), + requeue_batch_if_queue(&ctx, batch), + ); + return; + } + } + } + } + } else { +``` + +In the pre-existing "no session yet" arm below, add the same `applied_mcp` record around `agent.state.sessions.insert(*cid, sid.clone());` — again resolving *before* the mutable borrow: + +```rust + let applied = effective_mcp_servers(&agent, &ctx).clone(); + agent.state.sessions.insert(*cid, sid.clone()); + agent.state.applied_mcp.insert(*cid, applied); +``` + +> If the duplicated creation block reads badly once written, extract both call sites into a single local `async fn create_and_record(...)` helper — but only after the tests are green. + +- [ ] **Step 5: Run tests to verify they pass** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp 2>&1 | tail -20 +``` + +Expected: new tests pass; whole crate green. + +- [ ] **Step 6: Commit** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/src/pool.rs +git commit -s -m "feat(acp): apply MCP changes via in-place session/resume at turn boundaries" +``` + +--- + +### Task 5: `update_mcp_servers` control frame + +**Files:** +- Modify: `crates/buzz-acp/src/lib.rs:883-890` (dispatch arm), `:~1005` (new handler after `handle_switch_model_control`) +- Test: `crates/buzz-acp/src/lib.rs` (in-file tests, beside the existing `switch_model` control tests) + +**Interfaces:** +- Consumes: `AgentPool::set_desired_mcp` (Task 3); `McpServer: Deserialize` (Task 2). +- Produces: control frame `{"type":"update_mcp_servers","channelId":"","mcpServers":[...]}` → `control_result` with `status ∈ {"pending_next_turn", "invalid_servers", "unchanged"}`. + +**Security:** this frame names a command to execute — RCE by design. It rides the existing owner-signed, encrypted, ±5-minute-freshness envelope (`handle_relay_observer_control_event`, `lib.rs:837`). Do **not** add any path that accepts it outside that envelope. + +- [ ] **Step 1: Write the failing test** + +Add beside the existing `switch_model` control tests in `lib.rs`: + +```rust + #[test] + fn update_mcp_servers_control_records_the_desired_set() { + let mut pool = test_pool(); + let cid = Uuid::new_v4(); + let payload = serde_json::json!({ + "type": "update_mcp_servers", + "channelId": cid.to_string(), + "mcpServers": [ + {"name": "razorpay", "command": "/usr/bin/rzp", "args": [], "env": []} + ] + }); + + handle_update_mcp_servers_control(&payload, &mut pool, None); + + let recorded = pool.desired_mcp_for(&cid).expect("desired set recorded"); + assert_eq!(recorded.len(), 1); + assert_eq!(recorded[0].name, "razorpay"); + } + + #[test] + fn update_mcp_servers_control_rejects_a_malformed_server_list() { + let mut pool = test_pool(); + let cid = Uuid::new_v4(); + let payload = serde_json::json!({ + "type": "update_mcp_servers", + "channelId": cid.to_string(), + "mcpServers": [{"name": "missing-command"}] + }); + + handle_update_mcp_servers_control(&payload, &mut pool, None); + + assert!( + pool.desired_mcp_for(&cid).is_none(), + "a malformed grant must be rejected outright, never partially applied" + ); + } + + #[test] + fn update_mcp_servers_control_ignores_a_bad_channel_id() { + let mut pool = test_pool(); + let payload = serde_json::json!({ + "type": "update_mcp_servers", + "channelId": "not-a-uuid", + "mcpServers": [] + }); + + handle_update_mcp_servers_control(&payload, &mut pool, None); + // No panic, nothing recorded. + } +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp update_mcp_servers 2>&1 | tail -20 +``` + +Expected: FAIL — `cannot find function handle_update_mcp_servers_control`. + +- [ ] **Step 3: Add the dispatch arm** + +In `handle_relay_observer_control_event` (`lib.rs:885`), after the `switch_model` arm: + +```rust + Some("update_mcp_servers") => { + handle_update_mcp_servers_control(&payload, pool, observer); + } +``` + +- [ ] **Step 4: Add the handler** + +After `handle_switch_model_control` (~line 1005): + +```rust +/// Handle an `update_mcp_servers` control frame. +/// +/// Records the channel's desired MCP server set. Unlike `switch_model`, this +/// never cancels an in-flight turn: the set is stamped onto the agent by +/// `dispatch_pending` at the next turn boundary, and the session is resumed in +/// place there. The status is therefore always forward-looking. +/// +/// The payload names a command to execute, so this must only ever be reached +/// through the owner-signed, encrypted, freshness-checked observer path. +fn handle_update_mcp_servers_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + observer: Option<&observer::ObserverHandle>, +) { + let Some(channel_id) = payload + .get("channelId") + .and_then(|value| value.as_str()) + .and_then(|value| value.parse::().ok()) + else { + tracing::warn!("observer update_mcp_servers control frame missing valid channelId"); + return; + }; + + let Some(raw_servers) = payload.get("mcpServers") else { + tracing::warn!("observer update_mcp_servers control frame missing mcpServers"); + return; + }; + + // Reject the whole grant on a malformed entry — a partially applied tool + // set is worse than none, and the desktop can re-send. + let servers: Vec = match serde_json::from_value(raw_servers.clone()) { + Ok(servers) => servers, + Err(error) => { + tracing::warn!("observer update_mcp_servers control frame has invalid mcpServers: {error}"); + if let Some(observer) = observer { + emit_mcp_control_result(observer, channel_id, "invalid_servers"); + } + return; + } + }; + + let status = if pool.desired_mcp_for(&channel_id) == Some(&servers) { + "unchanged" + } else { + pool.set_desired_mcp(channel_id, servers); + "pending_next_turn" + }; + + if let Some(observer) = observer { + emit_mcp_control_result(observer, channel_id, status); + } +} + +fn emit_mcp_control_result( + observer: &observer::ObserverHandle, + channel_id: Uuid, + status: &str, +) { + observer.emit( + "control_result", + None, + &observer::ObserverContext { + channel_id: Some(channel_id.to_string()), + session_id: None, + turn_id: None, + started_at: None, + }, + serde_json::json!({ + "type": "update_mcp_servers", + "status": status, + }), + ); +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp update_mcp_servers 2>&1 | tail -20 +``` + +Expected: 3 passed. + +- [ ] **Step 6: Commit** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/src/lib.rs +git commit -s -m "feat(acp): update_mcp_servers control frame for live per-channel grants" +``` + +--- + +### Task 6: `strictMcpConfig` opt-in (the ~64k-token fix) + +**Files:** +- Modify: `crates/buzz-acp/src/acp.rs:629-655` (`session_new_full` gains a strict flag) +- Modify: `crates/buzz-acp/src/pool.rs:~906` (call site passes it) +- Test: `crates/buzz-acp/src/acp.rs` (in-file tests) + +**Interfaces:** +- Consumes: `effective_mcp_servers` / `OwnedAgent.desired_mcp` (Tasks 3–4) to decide *managed vs unmanaged*. +- Produces: `session_new_full(..., strict_mcp: bool)` — when true, sends `_meta.claudeCode.options = {"strictMcpConfig": true, "settingSources": ["project"]}`. + +**Verified behaviour this depends on:** the Claude adapter spreads `_meta.claudeCode.options` straight into the SDK options object (`acp-agent.js:4103` reads it, `:4157` spreads it). `strictMcpConfig` is a real SDK option (`sdk.d.ts:1959`, "Maps to the CLI `--strict-mcp-config` flag") that suppresses project `.mcp.json`, user settings, plugin, and agent-frontmatter MCP. + +**Review correction — do NOT send `settingSources`.** The spec pairs `strictMcpConfig` with `settingSources: ["project"]`. That is unnecessary and actively risky. `strictMcpConfig` alone already suppresses *every* other MCP source (`sdk.d.ts:1955-1957` enumerates them). `settingSources` is a much broader lever governing which settings files load at all — narrowing the adapter's default `["user","project","local"]` (`acp-agent.js:4156`) down to `["project"]` would silently drop the user's permission defaults and local settings for managed channels, changing behaviour far beyond MCP. Send `strictMcpConfig` only; leave `settingSources` at the adapter default. + +**The adapter never names `strictMcpConfig`** — it rides the generic spread, so a typo silently no-ops with no error. The assertion in Step 1 is the only thing standing between a typo and a silent 64k-token regression. + +- [ ] **Step 1: Write the failing tests** + +```rust +#[tokio::test] +async fn session_new_full_sends_strict_mcp_config_when_managed() { + 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,"result":{"sessionId":"ses_test","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client.initialize().await.expect("initialize should succeed"); + + let resp = client + .session_new_full("/tmp", vec![], None, None, true) + .await + .expect("session_new_full should succeed"); + + let opts = &resp.raw["_receivedRequest"]["params"]["_meta"]["claudeCode"]["options"]; + assert_eq!( + opts["strictMcpConfig"].as_bool(), + Some(true), + "managed channels must suppress the agent's global MCP config" + ); + assert!( + opts["settingSources"].is_null(), + "settingSources must be left alone — narrowing it would drop the user's \ + permission defaults for managed channels, which is not what this flag is for" + ); +} + +#[tokio::test] +async fn session_new_full_omits_strict_mcp_config_when_unmanaged() { + 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,"result":{"sessionId":"ses_test","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client.initialize().await.expect("initialize should succeed"); + + let resp = client + .session_new_full("/tmp", vec![], None, None, false) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert!( + received["params"]["_meta"]["claudeCode"].is_null(), + "unmanaged channels must keep legacy behaviour — never silently strip a user's global MCP servers" + ); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp strict_mcp 2>&1 | tail -20 +``` + +Expected: FAIL — `session_new_full` takes 4 arguments, 5 supplied. + +- [ ] **Step 3: Extend `session_new_full`** + +Add the parameter and merge into `_meta` without clobbering `sessionTitle`: + +```rust + pub async fn session_new_full( + &mut self, + cwd: &str, + mcp_servers: Vec, + system_prompt: Option<&str>, + session_title: Option<&str>, + strict_mcp: bool, + ) -> Result { + let mut params = serde_json::json!({ + "cwd": cwd, + "mcpServers": mcp_servers, + }); + if let Some(sp) = system_prompt { + params["systemPrompt"] = serde_json::Value::String(sp.to_owned()); + } + let mut meta = serde_json::Map::new(); + if let Some(title) = session_title { + meta.insert("sessionTitle".into(), serde_json::Value::String(title.to_owned())); + } + if strict_mcp { + // Managed channel: run exactly the servers the panel shows. Without + // this the agent additionally loads its own global MCP config, + // injecting tool schemas nobody asked for into every turn. + // + // Deliberately does NOT touch `settingSources`: `strictMcpConfig` + // already suppresses every other MCP source, and narrowing settings + // loading would drop the user's permission defaults as a side + // effect — a much wider blast radius than this flag is meant to have. + meta.insert( + "claudeCode".into(), + serde_json::json!({ + "options": { "strictMcpConfig": true } + }), + ); + } + if !meta.is_empty() { + params["_meta"] = serde_json::Value::Object(meta); + } + // ... rest unchanged ... +``` + +Update the `session_new` convenience wrapper to pass `false`, and update the doc comment to describe the new parameter. + +- [ ] **Step 4: Update the call site** + +In `pool.rs` `create_session_and_apply_model` (~line 906), a channel is *managed* exactly when it has a desired set: + +```rust + agent.desired_mcp.is_some(), +``` + +- [ ] **Step 5: Fix the other call sites** + +```bash +. ./bin/activate-hermit && cargo build -p buzz-acp 2>&1 | grep -A3 'this function takes' | head -30 +``` + +Pass `false` at every pre-existing `session_new_full` call and in the existing tests at `acp.rs:3274`, `:3359`, `:3387`, `:3415` — legacy behaviour is the default everywhere. + +- [ ] **Step 6: Run tests to verify they pass** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp 2>&1 | tail -20 +``` + +Expected: whole crate green. + +- [ ] **Step 7: Commit** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/src/acp.rs crates/buzz-acp/src/pool.rs +git commit -s -m "feat(acp): opt-in strictMcpConfig for managed channels" +``` + +--- + +### Task 7: Grant-landed verification + +> **✅ DECIDED 2026-07-31 (Moni): option 1 below — honest status + opportunistic verification.** Build this task as written; no upstream dependency, no blocking. +> +> Background. The spec (§5.4) and the kickoff both make this non-negotiable: *"verify the grant actually landed (observe the next turn's advertised tool list); do not trust the resume success response."* Verification of the shipped adapter found **that surface does not exist**. `acp-agent.js:1601-1614` consumes the SDK's `system`/`init` message internally (it latches `msgLifecycleV1` and syncs fast-mode state) and never forwards `tools[]` or `mcp_servers[{name,status}]` to the ACP client, even though the SDK carries both (`sdk.d.ts:4418-4424`). `session/resume` returns only `{sessionId, modes, configOptions}`. So there is nothing on the ACP wire to observe today. +> +> Options considered: +> 1. **← CHOSEN. Ship honest status, verify opportunistically.** Treat `pending_next_turn` → `applied_unverified` on a successful resume, and promote to `verified` only when a tool call whose name matches a granted server is seen in a later `session/update`. Cheap, truthful, no upstream dependency; a grant that silently fails shows as `applied_unverified` forever rather than as a false green. +> 2. *Rejected:* land the mechanism upstream first — a PR against `@agentclientprotocol/claude-agent-acp` to surface `mcp_servers` status. Correct, but blocks this PR on another repo's review. +> 3. *Rejected:* drop the requirement for v1. The spec explicitly warns reconfigure-on-resume is adapter behaviour, not an ACP contract — trusting the success response is exactly the failure mode §9 guards against. +> +> Option 2 remains the right long-term fix and should be noted in the PR body as a follow-up: `applied_unverified` is a workaround for a missing ACP surface, not a permanent design. + +**Files:** +- Modify: `crates/buzz-acp/src/pool.rs` (record grant state; observe tool-call updates) +- Test: `crates/buzz-acp/src/pool.rs` (in-file tests) + +**Interfaces:** +- Consumes: `SessionState.applied_mcp` (Task 3), the resume path (Task 4). +- Produces: `SessionState.mcp_verified: HashSet` and a `control_result` status of `applied_unverified` | `verified`. + +- [ ] **Step 1: Write the failing test** + +```rust + #[test] + fn a_tool_call_from_a_granted_server_marks_the_grant_verified() { + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.applied_mcp.insert( + cid, + vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec![], + env: vec![], + }], + ); + + assert!(!state.mcp_verified.contains(&cid)); + + // MCP tools surface as `mcp____`. + note_tool_call_for_verification(&mut state, &cid, "mcp__razorpay__create_order"); + + assert!( + state.mcp_verified.contains(&cid), + "a tool call from a granted server is proof the grant landed" + ); + } + + #[test] + fn an_unrelated_tool_call_does_not_verify_the_grant() { + let cid = Uuid::new_v4(); + let mut state = SessionState::default(); + state.applied_mcp.insert( + cid, + vec![McpServer { + name: "razorpay".into(), + command: "/usr/bin/rzp".into(), + args: vec![], + env: vec![], + }], + ); + + note_tool_call_for_verification(&mut state, &cid, "Read"); + + assert!( + !state.mcp_verified.contains(&cid), + "a built-in tool call proves nothing about the MCP grant" + ); + } +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp verification 2>&1 | tail -20 +``` + +Expected: FAIL — `cannot find function note_tool_call_for_verification`. + +- [ ] **Step 3: Implement** + +```rust +/// Promote a channel's MCP grant to *verified* when a tool call proves a +/// granted server is actually mounted. +/// +/// The ACP wire carries no MCP status today (the Claude adapter consumes the +/// SDK's `system`/`init` message, which holds `mcp_servers[{name,status}]`, +/// without forwarding it). A tool call named `mcp____` is +/// therefore the only first-hand evidence available that the grant landed. +/// Absence of evidence is reported as `applied_unverified`, never as success. +fn note_tool_call_for_verification(state: &mut SessionState, channel_id: &Uuid, tool_name: &str) { + let Some(servers) = state.applied_mcp.get(channel_id) else { + return; + }; + // Match the full `mcp____` prefix rather than splitting on `__`: + // a server name may itself contain a double underscore, and splitting would + // silently attribute its tool calls to the wrong server (or to none). + let landed = servers + .iter() + .any(|s| tool_name.starts_with(&format!("mcp__{}__", s.name))); + if landed { + state.mcp_verified.insert(*channel_id); + } +} +``` + +Add `pub mcp_verified: HashSet` to `SessionState`, clear it in `invalidate_channel` / `invalidate_all`, and call `note_tool_call_for_verification` from wherever `run_prompt_task` already observes `tool_call` session updates. Clear the channel's entry whenever `applied_mcp` changes (a new grant is unverified again). + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp verification 2>&1 | tail -20 +``` + +Expected: 2 passed. + +- [ ] **Step 5: Commit** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/src/pool.rs +git commit -s -m "feat(acp): opportunistic verification that an MCP grant landed" +``` + +--- + +### Task 8: Continuity proof + PR + +**Files:** +- Create: `crates/buzz-acp/tests/mcp_resume_continuity.rs` +- Modify: `AGENTS.md` (document `BUZZ_ACP_MCP_SERVERS` beside the other harness env vars) + +**Interfaces:** +- Consumes: everything above. + +**Note on the spec's §8 integration test:** the spec asks for "20 turns with tool calls, grant a server, resume, assert recall of early-turn facts." That requires a real agent subprocess and a real model — not runnable in CI. This task splits it: a scripted-agent test that proves the *protocol* contract (which CI can enforce), plus a manual runbook for the *recall* claim (which only a live agent can demonstrate). + +- [ ] **Step 1: Write the scripted continuity test** + +```rust +//! Proves the protocol-level continuity contract: an MCP change is applied by +//! `session/resume` on the SAME session id, and no `session/new` or +//! `session/cancel` is issued. Recall of early-turn content is a model-level +//! property — see the manual runbook in the PR description. + +// Script a fake agent that records every method it receives, run two turns +// with an MCP change between them, then assert on the recorded sequence: +// 1. initialize +// 2. session/new → ses_fixed +// 3. session/prompt +// 4. session/resume → sessionId == ses_fixed, mcpServers == [razorpay] +// 5. session/prompt +// and assert NO "session/cancel" and exactly ONE "session/new". +``` + +Implement it with the same `spawn_script` harness the `acp.rs` tests use, asserting: + +```rust +assert_eq!(methods.iter().filter(|m| *m == "session/new").count(), 1, + "an MCP change must never mint a new session — that is the whole point"); +assert!(!methods.iter().any(|m| m == "session/cancel"), + "an MCP change must never cancel an in-flight turn"); +assert_eq!(resume_params["sessionId"], "ses_fixed"); +``` + +- [ ] **Step 2: Run it** + +```bash +. ./bin/activate-hermit && cargo test -p buzz-acp --test mcp_resume_continuity 2>&1 | tail -20 +``` + +Expected: PASS. + +- [ ] **Step 3: Full local gate** + +```bash +. ./bin/activate-hermit && just ci 2>&1 | tail -30 +``` + +Expected: green. Fix fmt/clippy before proceeding — clippy passing does not mean fmt passes. + +- [ ] **Step 4: Unshallow before the PR** + +```bash +cd ~/Dev/buzz && git fetch --unshallow upstream 2>/dev/null || git fetch upstream +``` + +(This clone is `--depth 50`, so the merge-base against `block/buzz` is incomplete without this.) + +- [ ] **Step 5: Manual live verification (record the result in the PR)** + +1. Start the harness with `BUZZ_ACP_MCP_SERVERS='[]'` on a test channel. +2. Have a 10+ turn conversation including a file read, and state a distinctive fact early ("the passphrase is *hollyhock*"). +3. Send an `update_mcp_servers` control frame granting a server. +4. Next turn: confirm the new tool is callable **and** ask the agent to repeat the passphrase. +5. Record: session id unchanged, tool available, passphrase recalled. + +- [ ] **Step 6: Commit and open the PR** + +```bash +. ./bin/activate-hermit +git add crates/buzz-acp/tests/mcp_resume_continuity.rs AGENTS.md +git commit -s -m "test(acp): prove MCP changes resume in place without a new session" +git push origin feat/live-mcp-control +``` + +PR body must state: the mechanism is **adapter behaviour, not an ACP contract** (§9 abandon triggers), that `strictMcpConfig` is opt-in per managed channel, and the verification limitation from Task 7. + +--- + +## Verification log + +Every citation in the spec was re-checked against shipped source before this plan was written. All eight held. Three corrections are folded into the tasks above: + +| Spec claim | Verdict | +|---|---| +| `session/resume` carries `mcpServers` | ✅ `zResumeSessionRequest` — codex `dist/index.js:19573-19579` | +| Claude adapter reconfigures MCP on resume | ✅ `acp-agent.js:132-139` (fingerprint), `:3981-4008` (teardown + `createSession({resume})`); comment names the case | +| Resume restores transcript, same sessionId | ✅ `acp-agent.js:4279-4282` — `options.sessionId` is set only when *not* resuming | +| Codex applies new MCP on resume | ✅ `dist/index.js:26252-26259` → `threadResume({config: createSessionConfig(cwd, dirs, request.mcpServers)})`; no change-detection, so suppress no-op grants | +| Both adapters advertise resume | ✅ `acp-agent.js:644`, codex `dist/index.js:28490-28491` — both `sessionCapabilities.resume: {}` | +| `strictMcpConfig` suppresses global MCP | ✅ `sdk.d.ts:1959` — **correction:** the adapter never names it; it rides the generic `...userProvidedOptions` spread (`acp-agent.js:4103`, `:4157`), so a typo silently no-ops | +| Buzz sends `mcpServers` at `session/new` | ✅ `acp.rs:629-655`; default is exactly one server (dev MCP) or none — `lib.rs:4179` | +| Control-frame path is owner-signed + fresh | ✅ `lib.rs:837-1005` | + +**Correction 2 — `McpServer` lives in `acp.rs:28`, not `config.rs`,** and is `Serialize`-only. Task 2 adds `Deserialize`/`PartialEq`; the spec's §5.2.2 "extend to an enum covering stdio + http/sse" is **deferred** — the Rust type only models `McpServerStdio` today, and remote (http/sse) servers are a second, separable change. Flag in the PR as a known limitation. + +**Correction 3 — no MCP status on the ACP wire.** See the Task 7 decision block. + +## Self-review + +- **Spec coverage.** §5.2.1 → Task 1. §5.2.2 → Task 2 (http/sse variant explicitly deferred). §5.2.3 → Tasks 3–4. §5.2.4 → Task 5. §5.2.5 → Task 6. §5.4 → Task 7 (blocked on a decision). §8 → tests in every task + Task 8. §5.3 (desktop) → **out of scope, separate plan.** §7 (security) → Task 5 note. §9/§10 → PR description. +- **Type consistency.** `session_resume(session_id, cwd, mcp_servers)`, `resume_supported()`, `set_desired_mcp` / `desired_mcp_for`, `effective_mcp_servers`, `applied_mcp`, `desired_mcp`, `mcp_verified`, `note_tool_call_for_verification` — each defined once and used with the same name and signature throughout. +- **Known rough edge.** Task 4 Step 4 duplicates the session-creation block. The step says to extract a helper *after* tests are green; do not pre-factor it. diff --git a/docs/superpowers/specs/2026-07-31-live-mcp-control-design.md b/docs/superpowers/specs/2026-07-31-live-mcp-control-design.md new file mode 100644 index 0000000000..4da8eb08cf --- /dev/null +++ b/docs/superpowers/specs/2026-07-31-live-mcp-control-design.md @@ -0,0 +1,164 @@ +# Design: Live per-channel MCP control for Buzz agents + +**Status:** Proposal for contribution to `block/buzz` +**Date:** 2026-07-31 +**Authors:** Moni (product), with Claude Code (design + verification) + +--- + +## 1. Problem + +Buzz agents receive their MCP (Model Context Protocol) tool set **once**, at `session/new`, and it can never change for the life of the session. Two consequences, both verified against a live Buzz install: + +1. **No mid-conversation grants.** Mid-task, a user cannot give an agent a tool it lacks ("I need Razorpay now") without losing the agent's entire in-context working memory — the workaround today is rotating the session, which resets the agent to a small slice of channel history. +2. **Severe token waste.** Because Buzz passes an empty `mcpServers` list by default, agents fall back to their own global config (e.g. `~/.claude.json` with 13 MCP servers). Measured on a live session: **~64k tokens of unused tool schemas injected into every turn** — 255k cumulative tokens consumed by an 8-message channel. + +## 2. Goals / non-goals + +**Goals** +- Per-agent MCP configuration UI (Codex-style panel): choose which MCP servers each agent starts with. +- **Mid-conversation MCP toggle**: grant or revoke a tool in a live channel and have the agent continue the *same* conversation with the new tool set. +- Full conversational continuity across a toggle — the whole transcript, not a truncated window. +- Eliminate the unused-global-MCP token overhead on channels that have an explicit MCP set. +- Everything additive, in the existing architectural patterns of `buzz-acp`; no `unsafe`; DCO sign-off. + +**Non-goals** +- Mid-*turn* tool injection (tools land at turn boundaries; see §5). +- ACP protocol changes (none required — the mechanism uses shipped adapter behavior). +- Cursor support (Buzz has no Cursor ACP bridge yet; out of scope). +- A general meta-MCP proxy/gateway (deferred to v2; see §9). + +## 3. Key mechanism (verified in source) + +ACP fixes `mcpServers` at session creation — **but `session/resume` also carries an `mcpServers` field, and both flagship adapters implement "resume with a changed MCP set" as a supported reconfiguration path.** + +- **Claude adapter** (`@agentclientprotocol/claude-agent-acp`): `getOrCreateSession()` fingerprints `(cwd, mcpServers)`; on mismatch it tears down the SDK subprocess and recreates it with `resume: ` and the new servers, restoring the **full conversation transcript** via Claude Code's native resume. The in-code comment names this exact case ("…or MCP servers reconfigured"). — `acp-agent.js` lines 132–139, 3981–4010, 4266–4302. +- **Codex adapter**: `resumeSession` → `threadResume({config: createSessionConfig(cwd, dirs, request.mcpServers), threadId: sessionId})`. — `dist/index.js` lines 26252, 28643–28657. + +The **session ID never changes**, so Buzz's `channel → session` bookkeeping is untouched, and continuity is the entire transcript (disk-backed), not Buzz's 12-message `context_limit`. + +**Toggle semantics:** use `session/resume`, **not** `session/load` — `load` replays full history as `session/update` notifications before responding, risking the 60s request timeout on long conversations; `resume` skips replay with identical reconfiguration semantics. + +## 4. Why not the alternatives (divergent pass) + +An unconstrained solution-space search (8 routes) independently converged on this mechanism. The notable rejected alternatives: + +- **Meta-MCP router/gateway (one fixed MCP that hot-mounts downstream servers via `notifications/tools/list_changed`)** — the zero-churn ideal, but it depends on each agent's MCP client honoring `list_changed` (verified present in the Claude CLI binary; probable for Codex; unknown for Goose). Higher complexity, and the degraded fallback (a coarse `call_tool(server, tool, args)`) loses per-tool schemas and permission granularity. **Deferred to v2**, composed cleanly on top of this design. +- **`session/fork`** — same mechanics but mints a new sessionId, forcing a rewrite of the channel→session map and littering the store with abandoned forks. Codex doesn't register `session/fork`. Not the common denominator. +- **Fresh session + harness-injected history (status quo)** — loses in-context tool results, file reads, and reasoning state. Remains the **fallback** for agents without resume support. + +## 5. Design + +### 5.1 Data flow + +``` +Desktop per-channel MCP panel (spawn config + live toggles) + │ owner-signed, encrypted, freshness-checked control frame (existing path) + │ { type: "update_mcp_servers", channelId, mcpServers: [...] } + ▼ +buzz-acp control dispatch (lib.rs ~880, beside switch_model) + ▼ +handle_update_mcp_servers_control() + ├─ record desired set: desired_mcp: HashMap> + ├─ agent idle + supports resume → apply NOW: + │ acp.session_resume(session_id, cwd, new_servers) + │ (same session id; bookkeeping untouched) + ├─ agent busy → mark channel dirty; apply at next turn boundary (no cancel) + └─ agent lacks resume → invalidate (existing fresh-session fallback) + ▼ +observer.emit("control_result", { status: + "applied_live" | "pending_turn_end" | "rotated_no_resume_support" | "unchanged" }) + → desktop renders toggle state honestly +``` + +Turn-boundary application is safe by construction: the agent is moved out of the pool during a turn, so the dirty flag is only consulted when the agent is idle. Unlike `switch_model`, an MCP change never cancels an in-flight turn — killing work to add a tool is a worse trade. + +### 5.2 Changes in `crates/buzz-acp` (Rust) — all additive + +1. **`acp.rs`** — add `session_resume(session_id, cwd, mcp_servers) -> Result` sending `session/resume` (params per `zResumeSessionRequest`); in `initialize()`, record `agentCapabilities.sessionCapabilities.resume` (and `loadSession`) into a `resume_supported` flag, mirroring the existing `steering_supported` pattern. + +2. **`config.rs`** — multi-server config: add `--mcp-servers-json` / `BUZZ_ACP_MCP_SERVERS` (JSON array) alongside the legacy single `mcp_command`. Extend `McpServer` to an enum covering stdio + http/sse (the ACP schema union; the Claude adapter advertises `mcpCapabilities {http, sse}`) — remote MCP servers (e.g. Razorpay) require it. + +3. **`pool.rs`** — move the *effective* MCP set out of the immutable `PromptContext` into pool state: `mcp_overlay: Vec` + `mcp_dirty: HashSet` on `SessionState`. At the prompt-time session lookup, if a session exists and its channel is dirty: + ``` + match agent.acp.session_resume(&sid, &ctx.cwd, effective_servers()).await { + Ok(_) => { clear dirty; proceed with SAME sid } + Err(MethodNotFound) | !resume_supported + => invalidate_channel(cid) // fallback + + control_result "rotated_no_resume_support" + Err(e) => existing session-error path + } + ``` + `create_session_and_apply_model()` uses the channel's desired set (falling back to `ctx.mcp_servers`) instead of unconditionally `ctx.mcp_servers.clone()`. + +4. **`lib.rs`** — new `update_mcp_servers` control-frame arm + `handle_update_mcp_servers_control()`, modeled on `handle_switch_model_control()` but never cancelling in-flight turns. The `desired_mcp` map is runtime-only (re-sent by the desktop on reconnect), consistent with `desired_model` semantics. + +5. **`session/new` hygiene (the 64k-token fix, same PR)** — when a channel has an explicitly managed MCP set, include `_meta.claudeCode.options = {"strictMcpConfig": true, "settingSources": ["project"]}` in `session_new_full` params. Other agents ignore unknown `_meta` per JSON-RPC. Semantics: *unmanaged channel = legacy behavior (agent's own global config); managed channel = exactly what the panel shows.* **This must remain opt-in per managed channel** or users who rely on their global MCPs inside Buzz would silently lose them. + +### 5.3 Desktop changes + +- Per-agent MCP panel (spawn config): TOML/JSON list of servers, toggle each on/off. +- Per-channel live toggle list: emits the `update_mcp_servers` control frame; renders `control_result` status (live / pending until turn end / rotated). +- `managed-agents.json` gains `mcp_servers`; `runtime.rs` serializes it into `BUZZ_ACP_MCP_SERVERS` at spawn so grants persist across harness restarts. +- `agentControl.ts` gains `updateManagedAgentMcpServers(pubkey, channelId, servers)`, mirroring `switchManagedAgentModel`. + +### 5.4 Post-toggle verification (required) + +The reconfigure-on-resume behavior is an **implementation detail, not an ACP contract** — nothing in the schema promises `session/resume` applies a new tool set, and an adapter update could stop honoring it. Therefore the harness must **verify the grant landed** (observe the next turn's advertised tool list, or probe the adapter's MCP startup status) rather than trust the resume's success response. On verification failure, fall back to invalidate-and-rotate. + +### 5.5 Per-agent support matrix + +| Agent | Mid-chat toggle | Notes | +|---|---|---| +| Claude Code (claude-agent-acp) | ✅ Full (resume-swap) | Fingerprint teardown + native resume; verified | +| Codex (codex-acp) | ✅ Full (threadResume) | Always re-resumes — no change-detection, so suppress no-op grants | +| Goose | ⚠️ Capability-gated | Verify `session/load`/resume support; else fallback + honest UI message | +| buzz-agent (native) | ❌ `loadSession: false`, in-memory | Best served by a native `McpRegistry` hot-add (Buzz's own code) — a follow-up, and actually zero-churn | + +## 6. Continuity accounting (honest ledger) + +**Preserved across a resume-swap toggle:** the complete conversation (every message, tool call, tool result, file content — rebuilt verbatim from the session transcript); the session's model and mode; todo state; permission settings; the sessionId itself. + +**Lost:** subprocess ephemera — background bash jobs die at teardown; persistent-shell cwd/env resets; any stateful previously-attached MCP server restarts (Buzz's dev-mcp is stateless per call, so in practice: nothing). Cost: one subprocess respawn (~2–5s, hidden in the idle path) and a possible prompt-cache rewrite on the next turn — a token/latency *cost*, not a memory loss. + +**Both directions the user asked for are satisfied:** grant a tool mid-chat *and* keep the whole conversation. Proof chain: `session/resume` → fingerprint mismatch → `createSession({resume: sessionId, mcpServers: NEW})` → `query({resume, mcpServers})` → disk transcript reload. + +## 7. Security + +`update_mcp_servers` names a command to execute — it is remote-code-execution **by design**, so it must stay inside the existing owner-signed, encrypted, ±5-minute freshness envelope (the same path as `switch_model`). The desktop UI must make the grant explicit about *what* binary/URL will run — a friendly name pointing at an arbitrary command is a supply-chain phish. + +## 8. Testing + +Following the crate's existing test shapes: +- `session_resume` request-shape tests (beside the `session_new_full_*` tests). +- Fingerprint/no-op tests: same set sorted differently → **no** resume (the adapter fingerprint is order-sensitive for `args`/`env`; avoid spurious churn). +- Control-frame dispatch tests (beside the `switch_model` ones). +- Pool test: a respawned slot re-applies the channel's desired set on next session creation. +- **Integration test (the continuity guarantee):** run 20 turns with tool calls, grant a server, resume, assert recall of early-turn facts. This is the test that proves the headline claim. + +## 9. Abandon / fallback triggers + +Defined up front so the design degrades gracefully rather than silently: + +1. **Resume mangles heavy sessions** (the adapter references known resume context-accounting issues) → reproducible history loss = pivot to Route C (gateway) as primary. +2. **Grant clustering on very long sessions** (full-context rebuild gets expensive at 150k+ tokens) → flip default to gateway; keep resume-swap as fallback. +3. **Upstream adapters remove reconfigure-on-resume** → Route C becomes the principled primary, since it depends only on the MCP spec's `list_changed` (a real contract). + +## 10. v2 (out of scope, designed to compose) + +A Buzz gateway meta-MCP (extending `buzz-dev-mcp`, which is already in every session with relay credentials) that hot-mounts downstream servers and emits `notifications/tools/list_changed`. Zero session churn, mid-turn toggling — for agents whose MCP client verifiably honors `list_changed`. The gateway would simply be one of the servers in the resume-swapped list, so the two designs compose. + +--- + +## Appendix: evidence citations + +| Claim | Source | +|---|---| +| `session/resume` request includes `mcpServers` | ACP SDK `zResumeSessionRequest`, codex-acp `dist/index.js:19573` | +| Claude adapter reconfigures MCP on resume (fingerprint teardown) | `acp-agent.js:132-139, 3981-4010` | +| Resume restores full transcript (same sessionId) | `acp-agent.js:4266-4302, 5217-5259` | +| Codex resume applies new MCP config | `dist/index.js:26252, 28643-28657` | +| `strictMcpConfig` ignores global MCP config | claude-agent-sdk `sdk.d.ts:1959`; `acp-agent.js:4103, 4154-4175` | +| Buzz sends `mcpServers` at `session/new`; default near-empty | `crates/buzz-acp/src/acp.rs:621-697`, `lib.rs:4179` | +| Control-frame path (owner-signed, encrypted, fresh) | `crates/buzz-acp/src/lib.rs:834-1005` | +| Claude CLI honors `tools/list_changed` | binary strings + `tengu_mcp_list_changed` telemetry event | diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 0a3b2ed5a2..2e42f53fc4 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -103,6 +103,12 @@ run_unit_tests() { run_test_step "buzz-push-gateway tests" \ cargo test -p buzz-push-gateway -- --nocapture + + # ACP harness tests: session, pool, and control-frame coverage against a + # scripted `bash` stand-in for the agent, so no infra. Kept in step with the + # nextest branch of `just test-unit`. + run_test_step "buzz-acp tests" \ + cargo test -p buzz-acp -- --nocapture } # ---- DB / integration tests (infra required) --------------------------------