diff --git a/crates/skald-core/src/skald/user_context.rs b/crates/skald-core/src/skald/user_context.rs index c51ed9b..09b6cbf 100644 --- a/crates/skald-core/src/skald/user_context.rs +++ b/crates/skald-core/src/skald/user_context.rs @@ -302,6 +302,7 @@ impl UserContextFactory { specs.push(crate::mcp::user_row_spec_resolved(r, &container, ®istry).await); } um.connect_all(specs, false).await; + record_known_tools(®istry, &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. diff --git a/src/frontend/api/approval.rs b/src/frontend/api/approval.rs index fb97765..af0e735 100644 --- a/src/frontend/api/approval.rs +++ b/src/frontend/api/approval.rs @@ -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____` 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>, + Extension(auth): Extension, ) -> Result, 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 = 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 = tools.built_in.iter() .chain(tools.mcp.iter()) - .map(|t| t.name.as_str()) - .collect(); - let mut extra: Vec = 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 = 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 = 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)) }