fix: show per-user connectors in the security-group picker
Nightly Build / build (push) Successful in 7m40s

The Security-groups tool grid listed only global connectors. Its endpoint
built the MCP half from `skald.catalog()`, whose `ToolCatalog` is constructed
once around the ownerless GLOBAL `McpManager` — the per-user runtimes live on
each `UserContext` and it never sees them. `known_tools` did not cover the gap
either: `ToolDiscovery` records what is offered to a model, and an MCP tool
reaches the wire only once activated, so an unused connector was invisible
exactly when the admin wanted to write its rule.

The listing now unions three sources: the global runtime, the caller's own
per-user runtime (so a connector activated moments ago appears at once), and
`known_tools`, which per-user MCP startup now writes at login so a connector
belonging to an offline user is still nameable — security groups are
instance-wide config, and a grid that describes only whoever is online is a
grid the admin cannot finish.

An `mcp__<server>__<tool>` row from `known_tools` is routed to the MCP bucket
under its own server instead of the flat "dynamic" category, and a non-global
server takes its friendly name from the catalog entry it was activated from.
This commit is contained in:
2026-08-07 12:04:27 +01:00
parent 94bffe6760
commit 31b4c76f51
2 changed files with 108 additions and 16 deletions
@@ -302,6 +302,7 @@ impl UserContextFactory {
specs.push(crate::mcp::user_row_spec_resolved(r, &container, &registry).await);
}
um.connect_all(specs, false).await;
record_known_tools(&registry, &um).await;
}
Err(e) => tracing::warn!(error = %e, "per-user MCP init: failed to read mcp_user_servers"),
}
@@ -409,6 +410,33 @@ impl UserContextFactory {
}
}
/// Records this user's connector tools in the registry's `known_tools`, so an
/// instance-wide surface can name them while the user is offline.
///
/// Security groups are instance config, but a per-user connector's tools live in
/// a runtime that exists only between that user's login and the next restart —
/// so the Security-groups grid could only ever describe whoever happened to be
/// online. `ToolDiscovery` does not close the gap on its own: it records what is
/// *offered to a model*, and an MCP tool reaches the wire only once activated
/// (`SkaldToolSet::defs`), so a connector nobody has used yet is invisible
/// exactly when the admin wants to write its rule.
///
/// Registry-side is the right home under §2: the names say which connectors run
/// on this box, which the admin already curates in `mcp_catalog` — never who
/// activated one, and never a call or an argument. Best-effort: a row that does
/// not get written costs a tool that is gated by the catch-all `* require` until
/// the next login, which is the safe direction.
async fn record_known_tools(registry: &SqlitePool, mcp: &McpManager) {
for t in mcp.tools() {
let schema = serde_json::to_string(&t.input_schema).ok();
if let Err(e) = crate::db::known_tools::upsert(
registry, &t.tool_id(), &t.description, schema.as_deref(),
).await {
tracing::warn!(tool = %t.tool_id(), error = %e, "failed to record per-user MCP tool in known_tools");
}
}
}
/// The live per-user contexts, keyed by user id, plus the factory that builds them.
/// A `tokio::Mutex` serialises the build so a context (and its cron loop) is created
/// at most once per user, even under concurrent first-use.
+80 -16
View File
@@ -110,9 +110,27 @@ pub async fn list_pending(
//
// Returns all available tools (built-in + MCP) so the frontend can show a
// picker with names and descriptions when creating approval rules.
//
// The MCP half comes from three places, because no single one sees every
// connector on the box (§7 — two runtimes, and only one of them is shared):
//
// * the instance `ToolCatalog`, which wraps the ownerless GLOBAL runtime;
// * the caller's own PER-USER runtime, live in their container — the only
// way a connector activated moments ago shows up before any model has been
// offered it;
// * `known_tools`, the registry-side record of every tool that has existed on
// this box, which is what covers a connector belonging to a user who is not
// logged in right now. Security groups are instance-wide config, so leaving
// those out would make the grid describe only whoever happens to be online.
//
// An `mcp__<server>__<tool>` row from `known_tools` is routed to the MCP bucket
// under its own server rather than the flat "dynamic" category: the grid groups
// by server, and a connector's tools listed loose among the interface tools are
// findable only by someone who already knows their names.
pub async fn list_tools(
State(skald): State<Arc<Skald>>,
Extension(auth): Extension<AuthUser>,
) -> Result<Json<AllTools>, ApiError> {
let mut tools = skald.catalog().list_all();
let server_rows = skald_core::db::mcp_global_servers::all(skald.db()).await?;
@@ -120,30 +138,76 @@ pub async fn list_tools(
.map(|r| (r.name, McpServerMeta { friendly_name: r.friendly_name, description: r.description }))
.collect();
// The caller's per-user runtime. Best-effort: a context that is gone means a
// stale session, which is the login path's problem, not this listing's.
if let Ok(ctx) = require_context(&skald, &auth.user_id).await {
let seen: HashSet<String> = tools.mcp.iter().map(|t| t.name.clone()).collect();
for t in ctx.user_mcp.tools() {
let id = t.tool_id();
if seen.contains(&id) { continue; }
tools.mcp.push(ToolInfo {
name: id,
description: t.description,
source: "mcp".into(),
server: Some(t.server_name),
category: None,
});
}
}
// Merge dynamically-discovered tools (recorded by `ToolDiscovery` when they
// were offered to the LLM) that the catalog does not already surface — the
// interface/plugin/provider tools injected outside the `ToolRegistry`. This
// is what makes them configurable in the Security-groups grid. Names already
// known as built-in or MCP tools are deduped out; the rest are grouped under
// the "dynamic" category.
// interface/plugin/provider tools injected outside the `ToolRegistry`, plus
// the per-user MCP tools recorded at login. This is what makes them
// configurable in the Security-groups grid. Names already known as built-in
// or MCP tools are deduped out; an `mcp__*` name joins the MCP bucket, the
// rest are grouped under the "dynamic" category.
let discovered = skald_core::db::known_tools::all(skald.db()).await?;
let existing: HashSet<&str> = tools.built_in.iter()
let existing: HashSet<String> = tools.built_in.iter()
.chain(tools.mcp.iter())
.map(|t| t.name.as_str())
.collect();
let mut extra: Vec<ToolInfo> = discovered.into_iter()
.filter(|k| !existing.contains(k.name.as_str()))
.map(|k| ToolInfo {
name: k.name,
description: k.description,
source: "built-in".into(),
server: None,
category: Some("dynamic".into()),
})
.map(|t| t.name.clone())
.collect();
let mut extra: Vec<ToolInfo> = Vec::new();
for k in discovered {
if existing.contains(&k.name) { continue; }
match skald_core::mcp::parse_mcp_tool_name(&k.name) {
Some((server, _)) => tools.mcp.push(ToolInfo {
name: k.name.clone(),
description: k.description,
source: "mcp".into(),
server: Some(server.to_string()),
category: None,
}),
None => extra.push(ToolInfo {
name: k.name,
description: k.description,
source: "built-in".into(),
server: None,
category: Some("dynamic".into()),
}),
}
}
drop(existing);
tools.built_in.append(&mut extra);
tools.built_in.sort_by(|a, b| a.name.cmp(&b.name));
tools.mcp.sort_by(|a, b| a.name.cmp(&b.name));
// Metadata for every server that is not a global one: the catalog entry it
// was activated from. A self-registered remote has none and falls back to
// its raw server id in the UI.
let unnamed: Vec<String> = tools.mcp.iter()
.filter_map(|t| t.server.clone())
.filter(|s| !tools.mcp_servers.contains_key(s))
.collect();
for server in unnamed {
if tools.mcp_servers.contains_key(&server) { continue; }
if let Some(row) = skald_core::db::mcp_catalog::get_by_name(skald.db(), &server).await? {
tools.mcp_servers.insert(
server,
McpServerMeta { friendly_name: row.friendly_name, description: row.description },
);
}
}
Ok(Json(tools))
}