From d8b0406f7c05cb164e5a4f7ec1393715a58f1a1b Mon Sep 17 00:00:00 2001 From: YellowSnnowmann Date: Tue, 11 Aug 2026 15:33:58 +0530 Subject: [PATCH 1/3] feat(mcp): surface which agents can reach each MCP server (#568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP console showed a server's health and tool count but never who could actually call it. An agent reaches `mcp:` only when its effective grants cover it, so the moment a company narrows grants per agent a server can be live, healthy, and reachable by nobody — silently, with no error anywhere in the chain (such an agent doesn't even get `mcp_list_servers`). GET .../mcp/servers (and every mutating response) now carries `reachableBy`: the ids of the agents whose effective grants cover the server. The console renders it per row and flags the empty case loudly, since a healthy server no teammate can reach is almost always a misconfiguration rather than intent. Reachability is computed over the same roster the harness builds — manifest agents plus promoted overlay teammates — using the exact machinery the harness registry uses, so the console can't disagree with what an agent is actually granted: `agent_effective_grants` for each agent's grants, `grants_cover_server` for the `mcp:` match. `grants_cover_server` moves from `harness::mcp` (openhuman-gated) to `runtime::tools` beside `grant_matches` so the always-compiled console route reads the one primitive instead of reimplementing it; the registry path is unchanged. Overlay teammates are included deliberately — omitting them would let the zero-state falsely fire when an overlay agent reaches the server. Closes #568 Co-Authored-By: Claude Opus 4.8 --- frontend/src/api/types.ts | 8 + .../views/connections/McpServersSection.tsx | 24 +++ src/harness/mcp.rs | 10 +- src/runtime/tools.rs | 13 ++ src/server/ops/mcp.rs | 106 +++++++++++++- src/server/ops/write_test.rs | 137 ++++++++++++++++++ 6 files changed, 282 insertions(+), 16 deletions(-) diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 8b012f84..a093de13 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -751,6 +751,14 @@ export interface McpServer { timeoutSecs: number; /** Whether an outbound credential is stored — never the credential itself. */ authConfigured: boolean; + /** + * Ids of the company's agents whose effective tool grants cover this server — + * who can actually call it (issue #568). An empty array means no teammate can + * reach it, a probable misconfiguration the console flags loudly. Optional + * only for forward-compat with an older backend that does not send the field; + * `undefined` (unknown) is treated differently from `[]` (known-empty). + */ + reachableBy?: string[]; /** The last recorded (scrubbed) probe outcome, when the server has been probed. */ health?: McpHealth; } diff --git a/frontend/src/views/connections/McpServersSection.tsx b/frontend/src/views/connections/McpServersSection.tsx index 3493da6c..ed947598 100644 --- a/frontend/src/views/connections/McpServersSection.tsx +++ b/frontend/src/views/connections/McpServersSection.tsx @@ -444,6 +444,30 @@ export function McpServersSection({ client, company, canManage, chrome = "inline

{server.endpoint}

+ {/* Reachability (issue #568): who can actually call this server. A + healthy server no agent reaches is almost always a misconfiguration, + so the empty case is flagged loudly rather than shown as a blank list. */} + {server.reachableBy !== undefined && + (server.reachableBy.length === 0 ? ( +

+ + + No agent can reach this server — no teammate's tool grants cover{" "} + mcp:{server.name}. Widen a company or + per-agent tool grant, or this server is unused. + +

+ ) : ( +

+ Reachable by:{" "} + + {server.reachableBy.join(", ")} + +

+ ))} {health && health.status !== "ok" && health.message && (

{health.message}

)} diff --git a/src/harness/mcp.rs b/src/harness/mcp.rs index 3721cb8a..f1c5ce35 100644 --- a/src/harness/mcp.rs +++ b/src/harness/mcp.rs @@ -36,7 +36,7 @@ use crate::error::OpenCompanyError; use crate::harness::mcp_probe::{ McpFailure, McpFailureQueue, classify_mcp_error, operator_message, scrub, strip_endpoint, }; -use crate::runtime::tools::grant_matches; +use crate::runtime::tools::{grant_matches, grants_cover_server}; /// Builds a registry from a set of decls, keeping only the enabled ones. /// @@ -88,14 +88,6 @@ pub fn registry_for_agent( } } -/// Whether an agent's effective `grants` cover the MCP server named `name`, -/// using the same glob semantics as every other tool grant (`mcp:*` = all, -/// `mcp:notion` = exact). -fn grants_cover_server(grants: &[String], name: &str) -> bool { - let want = format!("mcp:{name}"); - grants.iter().any(|grant| grant_matches(grant, &want)) -} - /// Whether `agent`'s tool grants reach the MCP server named `name`, using the /// same glob semantics as every other tool grant (`mcp:*` = all, `mcp:notion` = /// exact). diff --git a/src/runtime/tools.rs b/src/runtime/tools.rs index 734cb0d2..72185bca 100644 --- a/src/runtime/tools.rs +++ b/src/runtime/tools.rs @@ -89,6 +89,19 @@ pub(crate) fn grant_matches(grant: &str, tool: &str) -> bool { grant == tool } +/// Whether an agent's effective tool `grants` cover the MCP server named `name`, +/// using the same glob semantics as every other grant (`mcp:*` grants all, +/// `mcp:notion` is exact). The single primitive read by both the harness's +/// per-agent registry assembly (`registry_for_agent`) and the console's +/// reachability view (issue #568), so the two can never disagree about which +/// agents reach a server. `grants` are the *effective* grants — resolve them +/// with [`agent_effective_grants`](crate::runtime::builder::agent_effective_grants) +/// first, never the raw per-agent `tools`. +pub(crate) fn grants_cover_server(grants: &[String], name: &str) -> bool { + let want = format!("mcp:{name}"); + grants.iter().any(|grant| grant_matches(grant, &want)) +} + #[async_trait] impl ToolProvider for StubToolProvider { async fn catalog(&self, _company: &CompanyId) -> Result> { diff --git a/src/server/ops/mcp.rs b/src/server/ops/mcp.rs index fe06d67e..f560f75d 100644 --- a/src/server/ops/mcp.rs +++ b/src/server/ops/mcp.rs @@ -29,6 +29,9 @@ use crate::company::mcp::{ }; use crate::company::runtime::CompanyRuntime; use crate::error::OpenCompanyError; +use crate::ports::types::CompanyRecord; +use crate::runtime::builder::agent_effective_grants; +use crate::runtime::tools::grants_cover_server; use crate::server::error::ApiError; use crate::server::ops::{AdminScopedCompany, ScopedCompany, scoped}; @@ -66,6 +69,16 @@ struct McpServerDto { timeout_secs: u64, /// Whether an outbound credential is stored — never the credential itself. auth_configured: bool, + /// The ids of the company's agents whose effective tool grants cover this + /// server — who can actually call it (issue #568). Computed over the same + /// roster the harness builds (manifest agents + promoted overlay teammates), + /// through the shared + /// [`grants_cover_server`](crate::runtime::tools::grants_cover_server), so the + /// console cannot disagree with the harness about reachability. **An empty + /// list is meaningful**: a live, healthy server no teammate can reach is + /// almost always a misconfiguration, and the console flags it rather than + /// showing an empty list silently. Always serialized (even when empty). + reachable_by: Vec, /// The last recorded probe outcome (scrubbed), or `None` when never probed. #[serde(skip_serializing_if = "Option::is_none")] health: Option, @@ -215,9 +228,13 @@ async fn manifest_servers(runtime: &CompanyRuntime) -> Result, Ap } /// Projects an effective decl (already merged + auth-resolved) to the console -/// DTO, reducing the resolved credential to a boolean and attaching the last -/// (scrubbed) probe health. -fn dto_from_decl(decl: &mcp::McpServerDecl, health: Option) -> McpServerDto { +/// DTO, reducing the resolved credential to a boolean, listing the agents that +/// can reach it (issue #568), and attaching the last (scrubbed) probe health. +fn dto_from_decl( + decl: &mcp::McpServerDecl, + reachable_by: Vec, + health: Option, +) -> McpServerDto { McpServerDto { name: decl.name.clone(), endpoint: decl.endpoint.clone(), @@ -228,24 +245,88 @@ fn dto_from_decl(decl: &mcp::McpServerDecl, health: Option) -> McpSer disallowed_tools: decl.disallowed_tools.clone(), timeout_secs: decl.timeout_secs, auth_configured: decl.auth.is_configured(), + reachable_by, health, } } +/// Every roster agent's *effective* tool grants (issue #568), as +/// `(agent_id, grants)`. The roster is exactly what the harness builds in +/// `build_roster`: the manifest agents (each with its own `tools` narrowed by +/// the company `allow`), plus the promoted overlay teammates. An overlay +/// teammate has no manifest `tools` row, so it inherits the full company `allow` +/// — the standard grant `overlay_agent_to_manifest` gives it — and an overlay id +/// already claimed by a manifest agent is skipped, both mirroring the harness so +/// console reachability equals what an agent is actually granted. +fn roster_grants(record: &CompanyRecord) -> Vec<(String, Vec)> { + let allow = &record.manifest.tools.allow; + let mut grants: Vec<(String, Vec)> = record + .manifest + .agents + .iter() + .map(|agent| { + ( + agent.id.clone(), + agent_effective_grants(allow, &agent.tools), + ) + }) + .collect(); + let manifest_ids: std::collections::HashSet<&str> = record + .manifest + .agents + .iter() + .map(|agent| agent.id.as_str()) + .collect(); + for overlay in &record.overlay_agents { + if manifest_ids.contains(overlay.id.as_str()) { + continue; + } + // No manifest tools row → the company's standard grant (empty `tools` + // ⇒ inherit `allow`), matching `overlay_agent_to_manifest`. + grants.push((overlay.id.clone(), agent_effective_grants(allow, &[]))); + } + grants +} + +/// The ids of the agents whose effective `grants` reach the server `name` +/// (issue #568), read through the shared [`grants_cover_server`] so this agrees +/// with the harness registry. Empty ⇒ no teammate can reach the server. +fn reachers_of(roster_grants: &[(String, Vec)], name: &str) -> Vec { + roster_grants + .iter() + .filter(|(_, grants)| grants_cover_server(grants, name)) + .map(|(id, _)| id.clone()) + .collect() +} + /// `GET …/mcp/servers` — the company's effective MCP servers, each with its last /// recorded (scrubbed) probe health. async fn list_servers(company: ScopedCompany) -> Result>, ApiError> { let runtime = company.runtime.as_ref(); - let manifest = manifest_servers(runtime).await?; + // One record load feeds both the manifest servers (merged into the effective + // set) and the roster used for reachability (issue #568), rather than loading + // it twice. + let record = runtime.store().load(runtime.id()).await.map_err(ApiError)?; + let manifest = record + .as_ref() + .map(|r| r.manifest.mcp_servers.clone()) + .unwrap_or_default(); let decls = resolve_effective(runtime.id(), &manifest, runtime.secrets().as_ref()) .await .map_err(ApiError)?; + // Resolve every agent's effective grants once, then ask per server who is + // covered — the wildcard-heavy work happens N(agents) times, not N×M. + let grants = record.as_ref().map(roster_grants).unwrap_or_default(); let mut out = Vec::with_capacity(decls.len()); for decl in &decls { let health = load_health(runtime.id(), &decl.name, runtime.secrets().as_ref()) .await .map_err(ApiError)?; - out.push(dto_from_decl(decl, health)); + out.push(dto_from_decl( + decl, + reachers_of(&grants, &decl.name), + health, + )); } Ok(Json(out)) } @@ -450,7 +531,13 @@ async fn mutation_response( // DTO so the response and a later `GET` agree. let test = probe_and_persist(runtime, name).await; - let manifest = manifest_servers(runtime).await?; + // One record load: the manifest servers merged into the effective set, and + // the roster the mutated server's reachability is computed against (#568). + let record = runtime.store().load(runtime.id()).await.map_err(ApiError)?; + let manifest = record + .as_ref() + .map(|r| r.manifest.mcp_servers.clone()) + .unwrap_or_default(); let decls = resolve_effective(runtime.id(), &manifest, runtime.secrets().as_ref()) .await .map_err(ApiError)?; @@ -459,11 +546,16 @@ async fn mutation_response( "`{name}` not found" ))) })?; + let reachable_by = record + .as_ref() + .map(roster_grants) + .map(|grants| reachers_of(&grants, name)) + .unwrap_or_default(); let health = load_health(runtime.id(), name, runtime.secrets().as_ref()) .await .map_err(ApiError)?; Ok(Json(MutationResponse { - server: dto_from_decl(decl, health), + server: dto_from_decl(decl, reachable_by, health), note: REBUILD_NOTE.to_string(), test, warning, diff --git a/src/server/ops/write_test.rs b/src/server/ops/write_test.rs index cddc88c1..71f6e37d 100644 --- a/src/server/ops/write_test.rs +++ b/src/server/ops/write_test.rs @@ -2388,6 +2388,45 @@ async fn state_with_manifest(home: &std::path::Path, manifest: CompanyManifest) state } +/// Like [`state_with_manifest`], but seeds operator-added overlay teammates too, +/// so a test can assert MCP reachability over the full runtime roster — manifest +/// agents plus overlay agents — the way `build_roster` composes it (issue #568). +async fn state_with_manifest_and_overlays( + home: &std::path::Path, + manifest: CompanyManifest, + overlay_agents: Vec, +) -> AppState { + use crate::ports::CompanyStore; + let store = FsCompanyStore::new(home.to_path_buf()); + let id = CompanyId::new("acme"); + store + .save(&CompanyRecord { + id: id.clone(), + manifest: manifest.clone(), + ledger: Vec::new(), + lifecycle: "running".to_string(), + overlay_agents, + overlay_desk_members: Vec::new(), + overlay_desk_order: Vec::new(), + overlay_desks: Vec::new(), + overlay_workflows: Vec::new(), + overlay_budgets: Vec::new(), + disabled_workflows: Vec::new(), + template_provenance: None, + }) + .await + .unwrap(); + let runtime = RuntimeBuilder::new(home.to_path_buf(), manifest) + .with_id(id.clone()) + .build() + .await + .unwrap(); + let state = AppState::new(AppConfig::default()); + state.registry().insert(id, std::sync::Arc::new(runtime)); + crate::server::test_support::seed_fixed_admin(&state, "acme").await; + state +} + #[tokio::test] async fn mcp_servers_crud_round_trips_and_token_is_write_only() { let home_dir = home(); @@ -2505,6 +2544,104 @@ async fn mcp_manifest_server_cannot_be_deleted_but_can_be_overridden() { assert_eq!(updated["server"]["enabled"], false); } +/// Issue #568: each listed server carries the ids of the agents whose *effective* +/// grants reach it — over the full runtime roster, manifest agents plus overlay +/// teammates. With a company `allow = ["*"]`, an agent that declares no `tools` +/// (and every overlay teammate, which has no tools row) inherits the wildcard and +/// reaches everything; an agent that narrows itself to `mcp:notion` reaches only +/// that server. +#[tokio::test] +async fn mcp_reachability_lists_reaching_agents_including_overlay() { + let manifest: CompanyManifest = toml::from_str( + "[company]\nname = \"Acme\"\n[tools]\nallow = [\"*\"]\n\ + [[agent]]\nid = \"ceo\"\nrole = \"Chief\"\ntools = [\"mcp:notion\"]\n\ + [[agent]]\nid = \"eng\"\nrole = \"Engineer\"\n[policy]\nmode = \"full\"\n\ + [[mcp_server]]\nname = \"notion\"\nendpoint = \"https://notion.example/mcp\"\n\ + [[mcp_server]]\nname = \"linear\"\nendpoint = \"https://linear.example/mcp\"\n", + ) + .unwrap(); + let home_dir = home(); + let home = home_dir.path().to_path_buf(); + let overlay = crate::ports::types::OverlayAgent { + id: "helper".to_string(), + name: "Helper".to_string(), + role: "Assistant".to_string(), + description: None, + }; + let state = state_with_manifest_and_overlays(&home, manifest, vec![overlay]).await; + + let (status, list) = send(&state, "GET", "/api/v1/company/mcp/servers", None).await; + assert_eq!(status, StatusCode::OK); + let reach = |name: &str| -> Vec { + let row = list + .as_array() + .unwrap() + .iter() + .find(|s| s["name"] == name) + .unwrap_or_else(|| panic!("server `{name}` is listed")); + let mut ids: Vec = row["reachableBy"] + .as_array() + .expect("reachableBy serializes as an array") + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect(); + ids.sort(); + ids + }; + + // notion: the narrowed ceo, the wildcard-inheriting eng, and the overlay. + assert_eq!(reach("notion"), vec!["ceo", "eng", "helper"]); + // linear: only the wildcard holders — ceo scoped itself out of it. + assert_eq!( + reach("linear"), + vec!["eng", "helper"], + "ceo narrowed to mcp:notion, so it cannot reach linear" + ); +} + +/// Issue #568: a server no agent's grants cover comes back with an **empty** +/// `reachableBy` — the signal the console flags loudly rather than showing a +/// healthy server that is silently unreachable. Here a narrow company +/// `allow = ["mcp:docs"]` reaches `docs` but never `notion`. +#[tokio::test] +async fn mcp_reachability_flags_a_server_no_agent_can_reach() { + let manifest: CompanyManifest = toml::from_str( + "[company]\nname = \"Acme\"\n[tools]\nallow = [\"mcp:docs\"]\n\ + [[agent]]\nid = \"ceo\"\nrole = \"Chief\"\ntools = [\"mcp:docs\"]\n[policy]\nmode = \"full\"\n\ + [[mcp_server]]\nname = \"docs\"\nendpoint = \"https://docs.example/mcp\"\n\ + [[mcp_server]]\nname = \"notion\"\nendpoint = \"https://notion.example/mcp\"\n", + ) + .unwrap(); + let home_dir = home(); + let home = home_dir.path().to_path_buf(); + let state = state_with_manifest(&home, manifest).await; + + let (status, list) = send(&state, "GET", "/api/v1/company/mcp/servers", None).await; + assert_eq!(status, StatusCode::OK); + let row = |name: &str| { + list.as_array() + .unwrap() + .iter() + .find(|s| s["name"] == name) + .unwrap_or_else(|| panic!("server `{name}` is listed")) + .clone() + }; + assert_eq!( + row("docs")["reachableBy"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap()) + .collect::>(), + vec!["ceo"], + "the company allow covers mcp:docs for the one agent" + ); + assert!( + row("notion")["reachableBy"].as_array().unwrap().is_empty(), + "no agent's grants cover mcp:notion — the flagged zero case" + ); +} + /// Without the `openhuman` feature there is no MCP transport, so live discovery /// is "not wired". (Under the feature it would attempt a real network call.) #[cfg(not(feature = "openhuman"))] From 9e221aba8e1f1ef38f030182e1975dc1354e16cf Mon Sep 17 00:00:00 2001 From: YellowSnnowmann Date: Tue, 11 Aug 2026 15:43:07 +0530 Subject: [PATCH 2/3] test(mcp): assert the mutating response carries reachableBy (#568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit list_servers had reachability coverage but mutation_response did not — add a shape assertion on the PUT response in the delete-guard test so the field stays wired on the mutating path, not just on GET. Co-Authored-By: Claude Opus 4.8 --- src/server/ops/write_test.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/server/ops/write_test.rs b/src/server/ops/write_test.rs index 71f6e37d..2a123d50 100644 --- a/src/server/ops/write_test.rs +++ b/src/server/ops/write_test.rs @@ -2542,6 +2542,12 @@ async fn mcp_manifest_server_cannot_be_deleted_but_can_be_overridden() { assert_eq!(status, StatusCode::OK); assert_eq!(updated["server"]["source"], "manifest"); assert_eq!(updated["server"]["enabled"], false); + // The mutating response carries reachability too (issue #568), so the console + // reflects who can reach the server right after an edit, not only on reload. + assert!( + updated["server"]["reachableBy"].is_array(), + "a mutating response also carries reachableBy" + ); } /// Issue #568: each listed server carries the ids of the agents whose *effective* From e606ce1f7745718a81edfea92c690c31915963b9 Mon Sep 17 00:00:00 2001 From: Shanu Date: Tue, 11 Aug 2026 16:02:53 +0530 Subject: [PATCH 3/3] fix(mcp): a disabled server is reachable by nobody (#568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `registry_for_agent` filters on `decl.enabled && grants_cover_server(..)`, so an agent holding `mcp:` is handed no such tool while the server is off. `reachers_of` mirrored only the grant half, so a disabled server came back listing agents that cannot call it — the console/harness disagreement this feature exists to remove. It now takes the declaration and returns an empty list when the server is disabled. The console scopes its loud zero-state to enabled servers: a disabled server is empty by construction, so flagging it would cry wolf on intent. Test: a disabled server with an otherwise matching grant reports `reachableBy: []` in both readers — the mutating response that turns it off, and the later list. --- frontend/src/api/types.ts | 9 ++- .../views/connections/McpServersSection.tsx | 10 +++- src/server/ops/mcp.rs | 36 +++++++----- src/server/ops/write_test.rs | 56 +++++++++++++++++++ 4 files changed, 91 insertions(+), 20 deletions(-) diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index a093de13..dea28133 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -753,9 +753,12 @@ export interface McpServer { authConfigured: boolean; /** * Ids of the company's agents whose effective tool grants cover this server — - * who can actually call it (issue #568). An empty array means no teammate can - * reach it, a probable misconfiguration the console flags loudly. Optional - * only for forward-compat with an older backend that does not send the field; + * who can actually call it (issue #568). On an **enabled** server an empty + * array means no teammate can reach it, a probable misconfiguration the + * console flags loudly. A **disabled** server is always empty (the harness + * hands out no tool for it whatever the grants say), so the console reads the + * empty case against `enabled` and stays quiet there. Optional only for + * forward-compat with an older backend that does not send the field; * `undefined` (unknown) is treated differently from `[]` (known-empty). */ reachableBy?: string[]; diff --git a/frontend/src/views/connections/McpServersSection.tsx b/frontend/src/views/connections/McpServersSection.tsx index ed947598..a310de1d 100644 --- a/frontend/src/views/connections/McpServersSection.tsx +++ b/frontend/src/views/connections/McpServersSection.tsx @@ -444,10 +444,14 @@ export function McpServersSection({ client, company, canManage, chrome = "inline

{server.endpoint}

- {/* Reachability (issue #568): who can actually call this server. A - healthy server no agent reaches is almost always a misconfiguration, - so the empty case is flagged loudly rather than shown as a blank list. */} + {/* Reachability (issue #568): who can actually call this server. An + enabled server no agent reaches is almost always a misconfiguration, + so that empty case is flagged loudly rather than shown as a blank list. + A disabled server is empty by construction — the harness hands out no + tool for it whatever the grants say — so the loud state is scoped to + enabled servers; flagging an off server would cry wolf on intent. */} {server.reachableBy !== undefined && + server.enabled && (server.reachableBy.length === 0 ? (

, /// The last recorded probe outcome (scrubbed), or `None` when never probed. #[serde(skip_serializing_if = "Option::is_none")] @@ -289,13 +292,22 @@ fn roster_grants(record: &CompanyRecord) -> Vec<(String, Vec)> { grants } -/// The ids of the agents whose effective `grants` reach the server `name` -/// (issue #568), read through the shared [`grants_cover_server`] so this agrees -/// with the harness registry. Empty ⇒ no teammate can reach the server. -fn reachers_of(roster_grants: &[(String, Vec)], name: &str) -> Vec { +/// The ids of the agents whose effective `grants` reach `decl` (issue #568), +/// read through the shared [`grants_cover_server`] so this agrees with the +/// harness registry. Empty ⇒ no teammate can reach the server. +/// +/// A **disabled** server reaches nobody regardless of grants: `registry_for_agent` +/// filters on `decl.enabled && grants_cover_server(..)`, so an agent granted +/// `mcp:` still gets no such tool while the server is off. Mirroring both +/// halves of that filter here is what keeps the console from claiming a +/// reachability the harness does not hand out. +fn reachers_of(roster_grants: &[(String, Vec)], decl: &mcp::McpServerDecl) -> Vec { + if !decl.enabled { + return Vec::new(); + } roster_grants .iter() - .filter(|(_, grants)| grants_cover_server(grants, name)) + .filter(|(_, grants)| grants_cover_server(grants, &decl.name)) .map(|(id, _)| id.clone()) .collect() } @@ -323,11 +335,7 @@ async fn list_servers(company: ScopedCompany) -> Result>, let health = load_health(runtime.id(), &decl.name, runtime.secrets().as_ref()) .await .map_err(ApiError)?; - out.push(dto_from_decl( - decl, - reachers_of(&grants, &decl.name), - health, - )); + out.push(dto_from_decl(decl, reachers_of(&grants, decl), health)); } Ok(Json(out)) } @@ -550,7 +558,7 @@ async fn mutation_response( let reachable_by = record .as_ref() .map(roster_grants) - .map(|grants| reachers_of(&grants, name)) + .map(|grants| reachers_of(&grants, decl)) .unwrap_or_default(); let health = load_health(runtime.id(), name, runtime.secrets().as_ref()) .await diff --git a/src/server/ops/write_test.rs b/src/server/ops/write_test.rs index 0c6aaa43..4257531b 100644 --- a/src/server/ops/write_test.rs +++ b/src/server/ops/write_test.rs @@ -2661,6 +2661,62 @@ async fn mcp_reachability_flags_a_server_no_agent_can_reach() { ); } +/// Issue #568: a **disabled** server reaches nobody, however wide the grants. +/// `registry_for_agent` filters on `decl.enabled && grants_cover_server(..)`, so +/// an agent holding `mcp:docs` is handed no such tool while the server is off — +/// reporting it as reachable would be the console/harness disagreement this +/// feature exists to remove. Asserted on both readers: the mutating response +/// that turns the server off, and the later list. +#[tokio::test] +async fn mcp_reachability_is_empty_for_a_disabled_server() { + let manifest: CompanyManifest = toml::from_str( + "[company]\nname = \"Acme\"\n[tools]\nallow = [\"*\"]\n\ + [[agent]]\nid = \"ceo\"\nrole = \"Chief\"\ntools = [\"mcp:docs\"]\n[policy]\nmode = \"full\"\n\ + [[mcp_server]]\nname = \"docs\"\nendpoint = \"https://docs.example/mcp\"\n", + ) + .unwrap(); + let home_dir = home(); + let home = home_dir.path().to_path_buf(); + let state = state_with_manifest(&home, manifest).await; + + let reach = |body: &serde_json::Value| -> Vec { + body["reachableBy"] + .as_array() + .expect("reachableBy serializes as an array") + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect() + }; + + // Enabled: the one agent's grant covers it. + let (status, list) = send(&state, "GET", "/api/v1/company/mcp/servers", None).await; + assert_eq!(status, StatusCode::OK); + assert_eq!(reach(&list[0]), vec!["ceo".to_string()]); + + // Disabling it empties reachability in the mutating response itself. + let (status, updated) = send( + &state, + "PUT", + "/api/v1/company/mcp/servers/docs", + Some(json!({ "enabled": false })), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(updated["server"]["enabled"], false); + assert!( + reach(&updated["server"]).is_empty(), + "a disabled server is handed to no agent, so it is reachable by none" + ); + + // And the list agrees on the next read — the grant is unchanged, the server is off. + let (_, list) = send(&state, "GET", "/api/v1/company/mcp/servers", None).await; + assert_eq!(list[0]["enabled"], false); + assert!( + reach(&list[0]).is_empty(), + "the list reader applies the same enabled filter as the harness" + ); +} + /// Without the `openhuman` feature there is no MCP transport, so live discovery /// is "not wired". (Under the feature it would attempt a real network call.) #[cfg(not(feature = "openhuman"))]