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.
This commit is contained in:
Will Pfleger
2026-07-14 17:34:04 -04:00
parent e27200baa2
commit 203a18d586
6 changed files with 185 additions and 3 deletions
+3 -1
View File
@@ -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
@@ -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)?;
@@ -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);
@@ -191,6 +191,9 @@ pub fn persona_from_event(event: &nostr::Event) -> Result<AgentDefinition, Strin
source_team: None,
source_team_persona_slug: Some(d_tag),
env_vars: BTreeMap::new(),
// MCP servers are local-only — never projected into kind:30175 events,
// so inbound reconcile always materializes an empty layer. This cannot
// raise an agent's effective MCP count.
mcp_servers: Vec::new(),
respond_to: content.respond_to,
respond_to_allowlist: content.respond_to_allowlist,
@@ -269,3 +269,29 @@ pub(crate) fn validate_effective_mcp_cap(
effective_buzz_agent_mcp_servers(record, personas, global, effective_command)?;
Ok(())
}
/// Validate the effective MCP cap across all existing agent records against a
/// prospective inherited layer (global or persona). Called at global-config
/// save and persona update to prevent an inherited-layer mutation that would
/// silently push an existing agent over the cap.
///
/// Non–buzz-agent runtimes are skipped (they don't use `BUZZ_ACP_MCP_SERVERS`).
/// Returns the first offending agent's name in the error message.
pub(crate) fn validate_effective_mcp_cap_for_records(
records: &[super::ManagedAgentRecord],
personas: &[super::AgentDefinition],
global: &[McpServerConfig],
) -> Result<(), String> {
for record in records {
let effective_cmd = crate::managed_agents::record_agent_command(record, personas);
if let Err(merge_err) =
effective_buzz_agent_mcp_servers(record, personas, global, &effective_cmd)
{
return Err(format!(
"saving would push agent `{}` over the MCP server limit: {merge_err}",
record.name
));
}
}
Ok(())
}
@@ -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<McpServerConfig>,
) -> 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<McpServerConfig>) -> 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");
}