From 5d5c3ff2ff488a46aea0fc3e1172b05bd36e8a98 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 00:22:35 +0100 Subject: [PATCH] mcp: live-refresh global connector access without restart; add MCP list to kid agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user's session sees global MCP connectors through UserMcpView, filtered by accessible_global — a snapshot of mcp_global_access taken when the user's UserContext is built at login. That context is cached until restart, so an admin enabling/deleting a global connector or changing its access set was invisible in MCP_LIST (and in the tool surface) until the whole process restarted. Make accessible_global a swappable cell (SharedGlobalAccess, the MCP twin of SharedFs for §6 fs remount): UserContext::refresh_global_access re-reads the registry and stores it in place, and Skald::refresh_global_mcp_access broadcasts that to every live context. Wire it into global_enable, global_delete, global_set_access and user_connectors_set so a grant/enable is reflected in running sessions immediately. Also add the shared common/mcp.md include (the sentinel) to the kid agent, aligning it with the other agents. Co-Authored-By: Claude Opus 4.8 --- agents/kid/AGENT.md | 4 ++ crates/skald-core/src/mcp/mod.rs | 2 +- crates/skald-core/src/mcp/provider.rs | 46 ++++++++++++++++----- crates/skald-core/src/skald/accessors.rs | 15 +++++++ crates/skald-core/src/skald/user_context.rs | 38 ++++++++++++++++- src/frontend/api/mcp.rs | 18 ++++++-- 6 files changed, 107 insertions(+), 16 deletions(-) diff --git a/agents/kid/AGENT.md b/agents/kid/AGENT.md index aee42ad..1a1c17c 100644 --- a/agents/kid/AGENT.md +++ b/agents/kid/AGENT.md @@ -73,6 +73,10 @@ There may be other helpers in the household's team — each good at different th --- + + +--- + ## Shared folders Shared folders are special places where some members of the household can read and write the same files together — photo albums, a family story, a playlist. You reach them at `shared/{name}/…`. Your folders, who else can see each one, and what each is for: diff --git a/crates/skald-core/src/mcp/mod.rs b/crates/skald-core/src/mcp/mod.rs index 98d540d..15acc72 100644 --- a/crates/skald-core/src/mcp/mod.rs +++ b/crates/skald-core/src/mcp/mod.rs @@ -32,7 +32,7 @@ pub mod verify; pub use install::{CONNECTORS_DIR, MANIFEST_FILE, connector_dir, ensure_installed_host, install_into_home, split_script_path}; pub use oauth::DeliverSpec; -pub use provider::{McpProvider, UserMcpView}; +pub use provider::{McpProvider, SharedGlobalAccess, UserMcpView}; pub use verify::{VerifyReport, VerifyTarget, apply_placeholders, run_verify}; const SERVER_START_TIMEOUT_SECS: u64 = 120; diff --git a/crates/skald-core/src/mcp/provider.rs b/crates/skald-core/src/mcp/provider.rs index d7ca767..b330b16 100644 --- a/crates/skald-core/src/mcp/provider.rs +++ b/crates/skald-core/src/mcp/provider.rs @@ -9,7 +9,7 @@ //! the union. use std::collections::{HashMap, HashSet}; -use std::sync::Arc; +use std::sync::{Arc, RwLock}; use anyhow::Result; use async_trait::async_trait; @@ -46,20 +46,43 @@ impl McpProvider for McpManager { } } +/// The set of global-connector names one user may see, behind a swappable cell so +/// an admin enabling/granting a global connector refreshes every live session in +/// place (the MCP twin of `SharedFs` for fs membership) — no restart needed. The +/// inner `Arc` lets a reader hold a cheap snapshot while a writer swaps. +#[derive(Clone)] +pub struct SharedGlobalAccess(Arc>>>); + +impl SharedGlobalAccess { + pub fn new(names: HashSet) -> Self { + Self(Arc::new(RwLock::new(Arc::new(names)))) + } + /// Cheap snapshot of the current set (clones an `Arc`, not the set). + pub fn load(&self) -> Arc> { + Arc::clone(&self.0.read().expect("SharedGlobalAccess lock poisoned")) + } + /// Replace the set in place — every `UserMcpView` sharing this cell sees it. + pub fn store(&self, names: HashSet) { + *self.0.write().expect("SharedGlobalAccess lock poisoned") = Arc::new(names); + } +} + /// One logged-in user's MCP view: the access-filtered global runtime unioned with /// their per-user container runtime. A per-user server wins on a name collision /// (which activation prevents anyway — see the uniqueness check at activation). pub struct UserMcpView { pub global: Arc, pub user: Arc, - /// Names of the global servers this user may use — a snapshot of - /// `mcp_global_access`, captured when the user's context is built. - pub accessible_global: HashSet, + /// Names of the global servers this user may use — read from `mcp_global_access` + /// when the user's context is built, then held in a swappable cell so an admin + /// enabling/granting a global connector refreshes it in place (§7 — the MCP twin + /// of the §6 fs remount) rather than settling only at the next restart. + pub accessible_global: SharedGlobalAccess, } impl UserMcpView { fn accessible_names(&self) -> Vec { - self.accessible_global.iter().cloned().collect() + self.accessible_global.load().iter().cloned().collect() } } @@ -75,8 +98,9 @@ impl McpProvider for UserMcpView { // A granted name belongs to exactly one runtime (unique per user); route // the accessible-global ones to the global runtime and the rest to the // per-user one, which filters to its own server map. + let accessible = self.accessible_global.load(); let global_names: Vec = names.iter() - .filter(|n| self.accessible_global.contains(*n)) + .filter(|n| accessible.contains(*n)) .cloned() .collect(); let mut out = self.global.tools_for(&global_names); @@ -85,19 +109,21 @@ impl McpProvider for UserMcpView { } fn server_descriptions(&self) -> HashMap> { + let accessible = self.accessible_global.load(); let mut m: HashMap> = self.global.server_descriptions() .into_iter() - .filter(|(name, _)| self.accessible_global.contains(name)) + .filter(|(name, _)| accessible.contains(name)) .collect(); m.extend(self.user.server_descriptions()); m } fn server_infos(&self) -> Vec { + let accessible = self.accessible_global.load(); let mut v: Vec = self.global.server_infos() .into_iter() .filter(|info| info["name"].as_str() - .map(|n| self.accessible_global.contains(n)) + .map(|n| accessible.contains(n)) .unwrap_or(false)) .collect(); v.extend(self.user.server_infos()); @@ -105,7 +131,7 @@ impl McpProvider for UserMcpView { } fn tool_display_name(&self, server: &str, tool: &str) -> Option { - if self.accessible_global.contains(server) { + if self.accessible_global.load().contains(server) { self.global.tool_display_name(server, tool) } else { self.user.tool_display_name(server, tool) @@ -113,7 +139,7 @@ impl McpProvider for UserMcpView { } async fn call(&self, server: &str, tool: &str, args: Value) -> Result { - if self.accessible_global.contains(server) { + if self.accessible_global.load().contains(server) { self.global.call(server, tool, args).await } else { // A per-user server, or an unknown/forbidden one — the per-user diff --git a/crates/skald-core/src/skald/accessors.rs b/crates/skald-core/src/skald/accessors.rs index 2b2897a..3656cf7 100644 --- a/crates/skald-core/src/skald/accessors.rs +++ b/crates/skald-core/src/skald/accessors.rs @@ -118,6 +118,21 @@ impl Skald { } Ok(()) } + + /// Refresh every live user's global-connector access set in place — call after an + /// admin enables/deletes a global connector or changes who may use it, so running + /// sessions see it without a restart (the §7 MCP twin of the §6 fs remount). The + /// global runtime itself is already updated by the caller (`start_server` / + /// `stop_server`); this only re-snapshots each user's access filter. Best-effort: + /// a locked (not-live) user has no snapshot to refresh — their next login rebuilds + /// it from the now-current tables. + pub async fn refresh_global_mcp_access(&self) { + for ctx in self.rt_user_contexts().all_live().await { + if let Err(e) = ctx.refresh_global_access().await { + tracing::warn!(user = %ctx.user_id, error = %e, "failed to refresh global MCP access"); + } + } + } pub fn sessions(&self) -> &Arc { &self.rt.sessions } pub fn config(&self) -> &Arc { &self.rt.config } pub fn config_properties(&self) -> &[core_api::ConfigSet] { &self.rt.config_properties } diff --git a/crates/skald-core/src/skald/user_context.rs b/crates/skald-core/src/skald/user_context.rs index 2083e73..546b34d 100644 --- a/crates/skald-core/src/skald/user_context.rs +++ b/crates/skald-core/src/skald/user_context.rs @@ -49,7 +49,7 @@ use crate::elicitation::ElicitationManager; use crate::image_generate::ImageGeneratorManager; use crate::inbox::Inbox; use crate::llm::LlmManager; -use crate::mcp::{McpManager, McpProvider, UserMcpView}; +use crate::mcp::{McpManager, McpProvider, SharedGlobalAccess, UserMcpView}; use crate::memory::MemoryManager; use crate::run_context::RunContextManager; use crate::session::handler::{DEFAULT_MAX_PARALLEL_SUBAGENTS, DEFAULT_MAX_TOOL_ROUNDS}; @@ -82,11 +82,33 @@ pub struct UserContext { /// here so its lifetime equals the pool's; its `docker exec -i` children die /// via `kill_on_drop` when the context is dropped at shutdown. pub user_mcp: Arc, + /// The registry (`system.db`) pool — used to re-read this user's global-connector + /// access when it changes (see [`UserContext::refresh_global_access`]). + pub registry_pool: Arc, + /// This user's global-connector access set, shared (swappable) with their live + /// `UserMcpView` so an admin's enable/grant is visible without a restart (§7). + pub global_access: SharedGlobalAccess, /// Per-user server→client push channel. WS handlers subscribe here (via the /// hub) so a user's `ServerEvent`s never reach another user's socket. pub global_tx: broadcast::Sender, } +impl UserContext { + /// Re-reads this user's global-connector access from the registry and swaps it + /// into the live `UserMcpView` in place — so an admin enabling/deleting a global + /// connector, or changing who may use it, is reflected in running sessions + /// without a restart (the §7 MCP twin of the §6 fs remount). + pub async fn refresh_global_access(&self) -> anyhow::Result<()> { + let names: std::collections::HashSet = + crate::db::mcp_global_access::server_names_for_user(&self.registry_pool, &self.user_id) + .await? + .into_iter() + .collect(); + self.global_access.store(names); + Ok(()) + } +} + /// Captures the global capability managers + resolved config once, and stamps out /// a [`UserContext`] per unlocked pool. pub(super) struct UserContextFactory { @@ -263,10 +285,14 @@ impl UserContextFactory { .unwrap_or_default() .into_iter() .collect(); + // A swappable cell shared with the view below, so an admin enabling or + // granting a global connector refreshes it in place (§7 — the MCP twin of + // the §6 fs remount) instead of settling only at the next restart. + let global_access = SharedGlobalAccess::new(accessible_global); let mcp_view: Arc = Arc::new(UserMcpView { global: Arc::clone(&self.mcp), user: Arc::clone(&user_mcp), - accessible_global, + accessible_global: global_access.clone(), }); let manager = Arc::new(ChatSessionManager::new( @@ -336,6 +362,8 @@ impl UserContextFactory { elicitation, inbox, user_mcp, + registry_pool: Arc::clone(&self.registry_pool), + global_access, global_tx, })) } @@ -372,6 +400,12 @@ impl UserContextRegistry { pub(super) async fn peek(&self, user_id: &str) -> Option> { self.contexts.lock().await.get(user_id).cloned() } + + /// A snapshot of every live context — for a broadcast refresh (e.g. global-MCP + /// access changing). Cheap: clones `Arc`s under a short lock. + pub(super) async fn all_live(&self) -> Vec> { + self.contexts.lock().await.values().cloned().collect() + } } // ── UserChannelHandle impl ──────────────────────────────────────────────────── diff --git a/src/frontend/api/mcp.rs b/src/frontend/api/mcp.rs index 754a2fe..9bf33ac 100644 --- a/src/frontend/api/mcp.rs +++ b/src/frontend/api/mcp.rs @@ -499,7 +499,11 @@ pub async fn global_enable( let row = mcp_global_servers::get(skald.db(), id).await? .ok_or_else(|| ApiError::bad_request("global server vanished after upsert"))?; let spec = skald_core::mcp::global_row_spec(&row); - match skald.mcp().start_server(spec).await { + let started = skald.mcp().start_server(spec).await; + // Now that the server runs in the global runtime, refresh every live user's + // access snapshot so the connector shows up in `MCP_LIST` without a restart. + skald.refresh_global_mcp_access().await; + match started { Ok(tools) => Ok(Json(json!({ "id": id, "tools": tools, "verify": verify }))), Err(e) => Ok(Json(json!({ "id": id, "error": e.to_string(), "verify": verify }))), } @@ -515,6 +519,8 @@ pub async fn global_delete( skald.mcp().stop_server(&row.name); } mcp_global_servers::delete(skald.db(), id).await?; + // Drop the now-deleted server from every live user's access snapshot in place. + skald.refresh_global_mcp_access().await; Ok(Json(json!({ "ok": true }))) } @@ -633,6 +639,8 @@ pub async fn global_set_access( ) -> Result, ApiError> { require_cap(&skald, &auth.user_id, role_capabilities::MANAGE_CATALOG).await?; mcp_global_access::set_access(skald.db(), id, &body.user_ids).await?; + // Reflect the new grant set in every live user's access snapshot at once. + skald.refresh_global_mcp_access().await; Ok(Json(json!({ "ok": true }))) } @@ -716,8 +724,8 @@ pub async fn user_connectors_set( skald_core::db::users::get(skald.db(), &target).await? .ok_or_else(|| ApiError::not_found("no such user"))?; - // Globals settle at the target user's next login (their `accessible_global` - // snapshot is captured then) — same as the existing per-server access flow. + // Apply the target user's global grants; the live-access snapshot is refreshed + // in place below (the §7 MCP remount), so a grant/revoke shows without a restart. mcp_global_access::set_for_user(skald.db(), &target, &body.global_ids).await?; // Catalog: apply the grant set; `set_for_user` returns the names this revoked. @@ -738,6 +746,10 @@ pub async fn user_connectors_set( } } } + + // Refresh live access snapshots so the target user's global grants take effect + // without a restart (the catalog side is handled by the live-revoke block above). + skald.refresh_global_mcp_access().await; Ok(Json(json!({ "ok": true }))) }