From 203a18d5868f94aa09048be83910cfde5ea55fbc Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 14 Jul 2026 09:17:38 -0400 Subject: [PATCH] fix(desktop): validate effective MCP cap at global and persona save An inherited-layer mutation (global config or persona definition) could silently push an existing buzz-agent over the 15-server effective cap, breaking it at next spawn. Add pre-save cross-checks at both save paths so the save is rejected atomically with the offending agent named. --- desktop/scripts/check-file-sizes.mjs | 4 +- .../src/commands/global_agent_config.rs | 10 ++ .../src-tauri/src/commands/personas/mod.rs | 24 ++++ .../src/managed_agents/persona_events.rs | 3 + .../src/managed_agents/types/mcp_servers.rs | 26 ++++ .../src/managed_agents/types/tests.rs | 121 +++++++++++++++++- 6 files changed, 185 insertions(+), 3 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index f3b7d9d3d..0761e97f3 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -80,7 +80,9 @@ const overrides = new Map([ // retry-safety. Load-bearing reviewer-required change; queued to split. // Consolidation removed the legacy persona-card import/export codecs. // +6: local-only MCP layer validation/update wiring. Load-bearing; queued to split. - ["src-tauri/src/commands/personas/mod.rs", 990], + // MCP validation and inherited-layer effective-cap save gate; the gate counts + // trailing newlines, so 994 physical lines are 995 counted lines. + ["src-tauri/src/commands/personas/mod.rs", 995], // #1418 read-path fix: get_thread_replies' blocker fix (shared TIMELINE_KINDS // const + build_thread_replies_filter helper, mirroring the channel sibling so // the two p-gate filters can't drift) plus two guard unit tests. The file was diff --git a/desktop/src-tauri/src/commands/global_agent_config.rs b/desktop/src-tauri/src/commands/global_agent_config.rs index 37a1be522..1bc447fb8 100644 --- a/desktop/src-tauri/src/commands/global_agent_config.rs +++ b/desktop/src-tauri/src/commands/global_agent_config.rs @@ -76,6 +76,16 @@ pub async fn set_global_agent_config( let phase1 = tokio::task::spawn_blocking(move || { validate_global_config(&config)?; + // Reject global save if the prospective MCP config would push any + // existing buzz-agent record over the effective-server cap. + let records = load_managed_agents(&app_for_write)?; + let personas = load_personas(&app_for_write)?; + crate::managed_agents::validate_effective_mcp_cap_for_records( + &records, + &personas, + &config.mcp_servers, + )?; + let old_global = load_global_agent_config(&app_for_write).unwrap_or_default(); save_global_agent_config(&app_for_write, &config)?; diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index ad3f0b0f7..e5925418b 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -209,7 +209,31 @@ pub async fn update_persona( apply_persona_behavior(persona, input.behavior)?; persona.updated_at = now_iso(); + // Clone before the cap check so the mutable borrow from + // `personas.iter_mut().find()` ends here. let result = persona.clone(); + + // Reject persona save if the prospective MCP config would push + // any existing buzz-agent referencing this persona over the + // effective-server cap. + { + let agents = load_managed_agents(&app)?; + let global_config = + crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); + // Filter to agents referencing this persona — the rest are + // unaffected and skipped by the resolver (wrong persona_id). + let referencing: Vec<_> = agents + .iter() + .filter(|a| a.persona_id.as_deref() == Some(input.id.as_str())) + .cloned() + .collect(); + crate::managed_agents::validate_effective_mcp_cap_for_records( + &referencing, + &personas, + &global_config.mcp_servers, + )?; + } + save_personas(&app, &personas)?; retain_persona_pending(&app, &state, &result); diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index fe760431b..31c94b906 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -191,6 +191,9 @@ pub fn persona_from_event(event: &nostr::Event) -> Result Result<(), String> { + for record in records { + let effective_cmd = crate::managed_agents::record_agent_command(record, personas); + if let Err(merge_err) = + effective_buzz_agent_mcp_servers(record, personas, global, &effective_cmd) + { + return Err(format!( + "saving would push agent `{}` over the MCP server limit: {merge_err}", + record.name + )); + } + } + Ok(()) +} diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 573a0cb98..838824426 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -1,6 +1,7 @@ use super::{ - merge_mcp_servers, replace_mcp_servers, validate_effective_mcp_cap, validate_mcp_servers, - AgentDefinition, ManagedAgentRecord, McpServerConfig, McpServerEnvVar, MAX_USER_MCP_SERVERS, + merge_mcp_servers, replace_mcp_servers, validate_effective_mcp_cap, + validate_effective_mcp_cap_for_records, validate_mcp_servers, AgentDefinition, + ManagedAgentRecord, McpServerConfig, McpServerEnvVar, MAX_USER_MCP_SERVERS, }; use std::path::PathBuf; @@ -867,3 +868,119 @@ fn effective_cap_skips_non_buzz_agent_runtime() { validate_effective_mcp_cap(&record, &[], &global, "goose") .expect("non-buzz-agent runtime should skip the cap"); } + +// ── validate_effective_mcp_cap_for_records — inherited-layer gates ─────── + +/// Build a buzz-agent record with a persona reference and custom mcp_servers. +fn buzz_agent_record_with_persona( + name: &str, + persona_id: &str, + mcp_servers: Vec, +) -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": format!("pk-{name}"), + "name": name, + "persona_id": persona_id, + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "buzz-agent", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "mcp_servers": mcp_servers, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + })) + .expect("minimal record should deserialize") +} + +/// Build a minimal AgentDefinition (persona) with the given mcp_servers. +fn persona_with_mcp(id: &str, mcp_servers: Vec) -> AgentDefinition { + serde_json::from_value(serde_json::json!({ + "id": id, + "display_name": format!("Persona {id}"), + "system_prompt": "", + "mcp_servers": mcp_servers, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + })) + .expect("minimal persona should deserialize") +} + +#[test] +fn inherited_gate_rejects_global_14_to_15_with_1_local_agent() { + // Agent has 1 local enabled server. Global goes from 14 → 15. Effective + // would be 16 — the gate must reject with the agent's name. + let record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + let prospective_global: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + let err = validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect_err("global 14→15 with 1 local must reject"); + assert!( + err.contains("test"), + "error must name the offending agent: {err}" + ); + assert!( + err.contains("saving would push agent"), + "error must use the required phrasing: {err}" + ); +} + +#[test] +fn inherited_gate_allows_global_14_with_1_local_agent() { + // Agent has 1 local. Global stays at 14. Effective = 15 (at cap) — OK. + let record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + let prospective_global: Vec<_> = (0..MAX_USER_MCP_SERVERS - 1) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect("15 effective should pass"); +} + +#[test] +fn inherited_gate_rejects_persona_unmask_pushing_agent_over_cap() { + // Agent has 1 local enabled. Global has 14. Persona goes from 0 → 1 + // unique enabled server. Effective = 16 — reject. + let persona_id = "p1"; + let record = buzz_agent_record_with_persona( + "agent-a", + persona_id, + vec![mcp_server("local-0", "cmd", true)], + ); + let global: Vec<_> = (0..MAX_USER_MCP_SERVERS - 1) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + let persona = persona_with_mcp(persona_id, vec![mcp_server("persona-0", "cmd", true)]); + let err = validate_effective_mcp_cap_for_records(&[record], &[persona], &global) + .expect_err("persona add pushing over cap must reject"); + assert!( + err.contains("agent-a"), + "error must name the offending agent: {err}" + ); +} + +#[test] +fn inherited_gate_skips_non_buzz_agent_records() { + // A goose-runtime agent should be unaffected by the cap. + let mut record = buzz_agent_record(vec![mcp_server("local-0", "cmd", true)]); + // record_agent_command resolves via agent_command_override first. + record.agent_command_override = Some("goose".to_string()); + let prospective_global: Vec<_> = (0..=MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect("non-buzz-agent runtime should skip the cap"); +} + +#[test] +fn inherited_gate_allows_unaffected_agent() { + // Agent with no local servers. Global at 15 → effective = 15. OK. + let record = buzz_agent_record(vec![]); + let prospective_global: Vec<_> = (0..MAX_USER_MCP_SERVERS) + .map(|i| mcp_server(&format!("global-{i}"), "cmd", true)) + .collect(); + validate_effective_mcp_cap_for_records(&[record], &[], &prospective_global) + .expect("agent with no local servers at cap should pass"); +}