Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions frontend/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
24 changes: 24 additions & 0 deletions frontend/src/views/connections/McpServersSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,30 @@ export function McpServersSection({ client, company, canManage, chrome = "inline
</span>
</div>
<p className="truncate text-xs text-muted-foreground">{server.endpoint}</p>
{/* 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 ? (
<p
data-testid="mcp-reachability-none"
className="flex items-start gap-1.5 rounded-md border border-destructive/30 bg-destructive/10 px-2 py-1 text-xs font-medium text-destructive"
>
<AlertTriangle className="mt-0.5 size-3.5 shrink-0" />
<span>
No agent can reach this server — no teammate&apos;s tool grants cover{" "}
<code className="font-mono">mcp:{server.name}</code>. Widen a company or
per-agent tool grant, or this server is unused.
</span>
</p>
) : (
<p data-testid="mcp-reachability" className="text-xs text-muted-foreground">
Reachable by:{" "}
<span className="font-medium text-foreground">
{server.reachableBy.join(", ")}
</span>
</p>
))}
{health && health.status !== "ok" && health.message && (
<p className="text-xs text-muted-foreground">{health.message}</p>
)}
Expand Down
10 changes: 1 addition & 9 deletions src/harness/mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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).
Expand Down
13 changes: 13 additions & 0 deletions src/runtime/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<ToolSpec>> {
Expand Down
106 changes: 99 additions & 7 deletions src/server/ops/mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<String>,
/// The last recorded probe outcome (scrubbed), or `None` when never probed.
#[serde(skip_serializing_if = "Option::is_none")]
health: Option<McpHealth>,
Expand Down Expand Up @@ -215,9 +228,13 @@ async fn manifest_servers(runtime: &CompanyRuntime) -> Result<Vec<McpServer>, 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<McpHealth>) -> 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<String>,
health: Option<McpHealth>,
) -> McpServerDto {
McpServerDto {
name: decl.name.clone(),
endpoint: decl.endpoint.clone(),
Expand All @@ -228,24 +245,88 @@ fn dto_from_decl(decl: &mcp::McpServerDecl, health: Option<McpHealth>) -> 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<String>)> {
let allow = &record.manifest.tools.allow;
let mut grants: Vec<(String, Vec<String>)> = 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<String>)], name: &str) -> Vec<String> {
roster_grants
.iter()
.filter(|(_, grants)| grants_cover_server(grants, name))
.map(|(id, _)| id.clone())
.collect()
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

/// `GET …/mcp/servers` — the company's effective MCP servers, each with its last
/// recorded (scrubbed) probe health.
async fn list_servers(company: ScopedCompany) -> Result<Json<Vec<McpServerDto>>, 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))
}
Expand Down Expand Up @@ -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)?;
Expand All @@ -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,
Expand Down
Loading