diff --git a/crates/skald-core/src/loop_adapters/activation.rs b/crates/skald-core/src/loop_adapters/activation.rs index 119122e..bce5f82 100644 --- a/crates/skald-core/src/loop_adapters/activation.rs +++ b/crates/skald-core/src/loop_adapters/activation.rs @@ -8,10 +8,13 @@ use std::sync::{Arc, RwLock}; use agent_loop::activation::{Activation, ActivationSource, ToolActivator}; use agent_loop::ids::{FrameId, MessageId}; use agent_loop::tool::{ToolCtx, ToolFailure}; -use serde_json::Value; +use serde_json::{Value, json}; use sqlx::SqlitePool; -use crate::db::{activated_tools, chat_llm_tools}; +use crate::db::{ + activated_tools, chat_llm_tools, mcp_catalog, mcp_catalog_access, mcp_global_access, + mcp_global_servers, mcp_user_servers, +}; use crate::mcp::McpProvider; use crate::tools::tool_names::CONFIG_GROUP; @@ -77,29 +80,323 @@ impl ActivationSource for SkaldActivationSource { // ── ToolActivator ──────────────────────────────────────────────────────────── -/// Backend of the crate's shipped `activate_tools` tool: validates the groups -/// against the catalog, updates the in-memory grant set **immediately** (next -/// round sees the tools), and persists the activation anchored at the -/// triggering assistant message (derived from the call's `chat_llm_tools` -/// row). Unifies what today lives split between `tools/activate_tools.rs` -/// (grants) and `llm_loop.rs` (persistence). +/// What a requested group resolved to. Only [`Status::Activated`] grants +/// anything: every other state means the group's tools cannot appear in this +/// session, and saying otherwise would have the model call `mcp__x__…` a round +/// later and fail there instead of here. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Status { + /// Running (or the built-in `config` group) — granted and persisted. + Activated, + /// The user activated the connector but never finished signing in + /// (`auth_state != 'ready'`), or they disabled it. + NeedsLogin, + /// Installed and disabled, or in the catalog and never activated: the USER + /// can fix it from Connectors. + NotActivated, + /// Exists but the ADMIN must act (a global connector not enabled/granted, a + /// catalog entry this user is not authorized for). + NotAuthorized, + /// Meant to be running and isn't — a start/connect failure, not a + /// configuration one. Transient. + Unavailable, + /// No such connector anywhere. + Unknown, +} + +impl Status { + fn as_str(self) -> &'static str { + match self { + Status::Activated => "activated", + Status::NeedsLogin => "needs_login", + Status::NotActivated => "not_activated", + Status::NotAuthorized => "not_authorized", + Status::Unavailable => "unavailable", + Status::Unknown => "unknown", + } + } +} + +/// One group's outcome, rendered as one JSON object. The shape is identical in +/// success and failure: `status` is what the model branches on, `message` is +/// what it relays to the user (the audience is non-technical — see `docs/`). +struct GroupReport { + status: Status, + tool_prefix: Option, + tool_count: usize, + description: Option, + message: String, +} + +impl GroupReport { + fn to_json(&self) -> Value { + json!({ + "status": self.status.as_str(), + "tool_prefix": self.tool_prefix, + "tool_count": self.tool_count, + "description": self.description, + "message": self.message, + }) + } +} + +/// Backend of the crate's shipped `activate_tools` tool: resolves each group +/// against the runtime and the connector tables, updates the in-memory grant +/// set **immediately** for the ones that resolved (next round sees the tools), +/// and persists those activations anchored at the triggering assistant message +/// (derived from the call's `chat_llm_tools` row). +/// +/// A group that cannot be activated is diagnosed rather than accepted: it +/// touches neither the grant set nor `activated_tools`, and the report says +/// which of the connector states (§7/§15) it is in and who can fix it. pub struct SkaldToolActivator { + /// Owner pool — `mcp_user_servers` (this user's activations). pool: Arc, + /// Registry pool — `mcp_catalog`, `mcp_global_servers` and the access grants. + shared_pool: Arc, + user_id: String, mcp: Arc, + /// The reserved `config` group's defs, for its tool count. + config_defs: Arc>, grants: Arc>>, session_id: i64, stack: Option, } impl SkaldToolActivator { + #[allow(clippy::too_many_arguments)] pub fn new( - pool: Arc, - mcp: Arc, - grants: Arc>>, - session_id: i64, - stack: Option, + pool: Arc, + shared_pool: Arc, + user_id: String, + mcp: Arc, + config_defs: Arc>, + grants: Arc>>, + session_id: i64, + stack: Option, ) -> Self { - Self { pool, mcp, grants, session_id, stack } + Self { pool, shared_pool, user_id, mcp, config_defs, grants, session_id, stack } + } + + /// Where the activated tools land, for the confirmation message. + fn scope_label(&self) -> &'static str { + match self.stack { + None => "this session", + Some(_) => "this sub-agent frame", + } + } + + /// Resolves one group name to its state. A diagnosis query that fails is + /// logged and treated as "no row" — a broken lookup must not turn into a + /// false claim about the connector. + async fn resolve(&self, name: &str) -> GroupReport { + if name == CONFIG_GROUP { + return GroupReport { + status: Status::Activated, + tool_prefix: None, + tool_count: self.config_defs.len(), + description: Some( + "Built-in system-configuration tools: connectors, plugins, scheduled jobs, secrets." + .into(), + ), + message: format!("Tools are in context for {} from the next round.", self.scope_label()), + }; + } + + // Running in this user's view (global ∪ per-user, already access-filtered). + let key = [name.to_string()]; + let running = self.mcp.tools_for(&key); + if !running.is_empty() { + return GroupReport { + status: Status::Activated, + tool_prefix: Some(format!("mcp__{name}__")), + tool_count: running.len(), + description: self.mcp.server_descriptions().get(name).cloned().flatten(), + message: format!("Tools are in context for {} from the next round.", self.scope_label()), + }; + } + + // Not running — diagnose, from the user's own activations outward. + if let Some(row) = self + .lookup(mcp_user_servers::get_by_name(&self.pool, name).await, "mcp_user_servers", name) + .flatten() + { + let description = self.catalog_description(row.catalog_name.as_deref()).await; + if !row.enabled { + return GroupReport { + status: Status::NotActivated, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "The `{name}` connector is set up for this user but switched off. \ + Tell the user to re-enable it in Connectors." + ), + }; + } + if row.auth_state != "ready" { + let how = self.login_hint(row.catalog_name.as_deref()).await; + return GroupReport { + status: Status::NeedsLogin, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "The `{name}` connector is installed but the sign-in was never completed. \ + Tell the user to open Connectors → {name} and {how}." + ), + }; + } + return GroupReport { + status: Status::Unavailable, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "The `{name}` connector is set up and enabled but its server is not running \ + right now — it failed to start. This is temporary and not something the user \ + can fix from the interface." + ), + }; + } + + if let Some(row) = self + .lookup(mcp_global_servers::get_by_name(&self.shared_pool, name).await, "mcp_global_servers", name) + .flatten() + { + let description = row.description.clone(); + if !row.enabled { + return GroupReport { + status: Status::NotAuthorized, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "`{name}` is a shared connector that the administrator has disabled. \ + Only an administrator can turn it back on." + ), + }; + } + let granted = self + .lookup(mcp_global_access::has_access(&self.shared_pool, row.id, &self.user_id).await, "mcp_global_access", name) + .unwrap_or(false); + if !granted { + return GroupReport { + status: Status::NotAuthorized, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "`{name}` is a shared connector this user has not been given access to. \ + Only an administrator can grant it." + ), + }; + } + return GroupReport { + status: Status::Unavailable, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "`{name}` is enabled and granted but its server is not running right now — \ + it failed to start. This is temporary and not something the user can fix." + ), + }; + } + + if let Some(row) = self + .lookup(mcp_catalog::get_by_name(&self.shared_pool, name).await, "mcp_catalog", name) + .flatten() + { + let description = row.description.clone(); + if row.scope == "global" { + return GroupReport { + status: Status::NotAuthorized, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "`{name}` is installed but not switched on as a shared connector. \ + Only an administrator can enable it." + ), + }; + } + let authorized = self + .lookup(mcp_catalog_access::has_access(&self.shared_pool, name, &self.user_id).await, "mcp_catalog_access", name) + .unwrap_or(false); + return if authorized { + GroupReport { + status: Status::NotActivated, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "`{name}` is available to this user but not activated yet. \ + Tell the user they can activate it in Connectors, then ask again." + ), + } + } else { + GroupReport { + status: Status::NotAuthorized, + tool_prefix: None, + tool_count: 0, + description, + message: format!( + "`{name}` is installed on this instance but this user is not authorized \ + to activate it. Only an administrator can authorize them." + ), + } + }; + } + + GroupReport { + status: Status::Unknown, + tool_prefix: None, + tool_count: 0, + description: None, + message: format!( + "There is no connector named `{name}`. Valid group names are the ones listed in \ + the MCP servers table of your context, plus the reserved `config`. Do not guess \ + a name; if the user needs this capability, they can look for it in the Connectors \ + marketplace." + ), + } + } + + /// A diagnosis lookup: `Err` is a broken query, not an answer — log it and + /// fall through to the next candidate rather than mislabelling the group. + fn lookup(&self, r: anyhow::Result, table: &str, name: &str) -> Option { + match r { + Ok(v) => Some(v), + Err(e) => { + tracing::warn!(table, group = name, error = %e, "activate_tools: diagnosis lookup failed"); + None + } + } + } + + /// The catalog blurb of the entry a user activation came from — the only + /// description a non-running connector has. + async fn catalog_description(&self, catalog_name: Option<&str>) -> Option { + let name = catalog_name?; + mcp_catalog::get_by_name(&self.shared_pool, name).await.ok().flatten()?.description + } + + /// How this connector's sign-in is completed, per its catalog `auth_kind`. + async fn login_hint(&self, catalog_name: Option<&str>) -> &'static str { + let kind = match catalog_name { + Some(n) => mcp_catalog::get_by_name(&self.shared_pool, n) + .await + .ok() + .flatten() + .map(|c| c.auth_kind), + None => None, + }; + match kind.as_deref() { + Some("oauth") => "complete the sign-in (approve access, then paste the code back)", + Some("qr") => "scan the QR code with the device they want to link", + _ => "finish setting it up", + } } } @@ -110,50 +407,67 @@ impl ToolActivator for SkaldToolActivator { return Err(ToolFailure::Failed("activate_tools: `groups` is empty".into())); } - let available: HashSet = self.mcp.tools().iter().map(|t| t.server_name.clone()).collect(); + // Resolve first, act only on what resolved: an unknown or unconfigured + // group must leave no trace, in RAM or in the DB. + let mut reports: Vec<(String, GroupReport)> = Vec::new(); + for g in &groups { + if reports.iter().any(|(n, _)| n == g) { + continue; // the same group twice in one call + } + let report = self.resolve(g).await; + reports.push((g.clone(), report)); + } - // Immediate in-memory effect (the defs re-read at the next round picks - // the new grants up for free). - { - let mut set = self.grants.write().map_err(|_| ToolFailure::Failed("activate_tools: lock poisoned".into()))?; - for g in &groups { - set.insert(g.clone()); + let activated: Vec = reports + .iter() + .filter(|(_, r)| r.status == Status::Activated) + .map(|(n, _)| n.clone()) + .collect(); + + if !activated.is_empty() { + // Immediate in-memory effect (the defs re-read at the next round + // picks the new grants up for free). + { + let mut set = self + .grants + .write() + .map_err(|_| ToolFailure::Failed("activate_tools: lock poisoned".into()))?; + for g in &activated { + set.insert(g.clone()); + } + } + + // Durable effect, anchored at the triggering assistant message. The + // anchor is derived from the call row — the crate's ToolCtx carries + // the call id, the message id is one lookup away. + let call = chat_llm_tools::get(&self.pool, ctx.call_id.get()) + .await + .map_err(|e| ToolFailure::Failed(format!("activate_tools: anchor lookup failed: {e}")))? + .ok_or_else(|| ToolFailure::Failed("activate_tools: call row not found".into()))?; + for g in &activated { + let kind = if g == CONFIG_GROUP { "builtin" } else { "mcp" }; + activated_tools::grant(&self.pool, self.session_id, self.stack, call.message_id, kind, g) + .await + .map_err(|e| ToolFailure::Failed(format!("activate_tools: grant failed: {e}")))?; } } - // Durable effect, anchored at the triggering assistant message. The - // anchor is derived from the call row — the crate's ToolCtx carries - // the call id, the message id is one lookup away. - let call = chat_llm_tools::get(&self.pool, ctx.call_id.get()) - .await - .map_err(|e| ToolFailure::Failed(format!("activate_tools: anchor lookup failed: {e}")))? - .ok_or_else(|| ToolFailure::Failed("activate_tools: call row not found".into()))?; - for g in &groups { - let kind = if g == CONFIG_GROUP { "builtin" } else { "mcp" }; - activated_tools::grant(&self.pool, self.session_id, self.stack, call.message_id, kind, g) - .await - .map_err(|e| ToolFailure::Failed(format!("activate_tools: grant failed: {e}")))?; - } + // One JSON object keyed by group name, same shape whether the call + // succeeded or not — the model parses one thing, never prose. + let body = Value::Object( + reports + .iter() + .map(|(name, r)| (name.clone(), r.to_json())) + .collect(), + ); + let text = serde_json::to_string(&body).unwrap_or_else(|_| "{}".to_string()); - let activated: Vec = groups - .iter() - .map(|n| { - if n == CONFIG_GROUP || available.contains(n) { - format!("{n} ✓") - } else { - format!("{n} (registered but not yet running — tools will appear after reconnect)") - } - }) - .collect(); - let scope = match self.stack { - None => "session".to_string(), - Some(s) => format!("stack {s}"), - }; - Ok(format!( - "Tool groups activated for this {scope}: {}. \ - Their tools are available from the next tool-call round.", - activated.join(", ") - )) + if activated.is_empty() { + // Nothing was activated: fail, so the model treats it as an error + // and relays the diagnosis instead of proceeding as if it worked. + return Err(ToolFailure::Failed(text)); + } + Ok(text) } } @@ -249,23 +563,54 @@ mod tests { } } - #[tokio::test] - async fn activate_grants_in_memory_and_persists_anchored() { - let f = fixture("act-grant").await; - let mcp: Arc = Arc::new(FakeMcp::with_server("gmail", &["send", "read"])); - let grants = Arc::new(RwLock::new(HashSet::new())); - let activator = SkaldToolActivator::new(f.pool.clone(), mcp, grants.clone(), 1, None); + /// The fixture's pool is both the owner and the registry pool: `init_system_pool` + /// creates both buckets in one file, which is exactly what a diagnosis needs. + fn activator(f: &Fixture, mcp: Arc, grants: Arc>>) -> SkaldToolActivator { + SkaldToolActivator::new( + f.pool.clone(), + f.pool.clone(), + "u1".into(), + mcp, + Arc::new(vec![serde_json::json!({"type":"function","function":{"name":"cron_list"}})]), + grants, + 1, + None, + ) + } - let ctx = ToolCtx { + fn ctx_of(f: &Fixture) -> ToolCtx { + ToolCtx { conversation: agent_loop::ids::ConversationId::new("session:1"), frame: f.frame, agent: "assistant".into(), call_id: f.call, cancel: tokio_util::sync::CancellationToken::new(), extensions: Default::default(), - }; - let text = activator.activate(vec!["gmail".into(), CONFIG_GROUP.into()], &ctx).await.unwrap(); - assert!(text.contains("gmail ✓")); + } + } + + fn report(text: &str, group: &str) -> Value { + serde_json::from_str::(text).expect("tool result is JSON")[group].clone() + } + + #[tokio::test] + async fn activate_grants_in_memory_and_persists_anchored() { + let f = fixture("act-grant").await; + let mcp: Arc = Arc::new(FakeMcp::with_server("gmail", &["send", "read"])); + let grants = Arc::new(RwLock::new(HashSet::new())); + let activator = activator(&f, mcp, grants.clone()); + + let text = activator + .activate(vec!["gmail".into(), CONFIG_GROUP.into()], &ctx_of(&f)) + .await + .unwrap(); + + let gmail = report(&text, "gmail"); + assert_eq!(gmail["status"], "activated"); + assert_eq!(gmail["tool_prefix"], "mcp__gmail__"); + assert_eq!(gmail["tool_count"], 2); + assert_eq!(report(&text, CONFIG_GROUP)["status"], "activated"); + assert_eq!(report(&text, CONFIG_GROUP)["tool_count"], 1); // In-memory effect. assert!(grants.read().unwrap().contains("gmail")); @@ -281,6 +626,114 @@ mod tests { cleanup(&f.path); } + /// The bug this taxonomy exists for: a name nobody knows used to be granted, + /// persisted and reported as a success. + #[tokio::test] + async fn unknown_group_fails_and_leaves_no_trace() { + let f = fixture("act-unknown").await; + let mcp: Arc = Arc::new(FakeMcp::with_server("tavily", &["search"])); + let grants = Arc::new(RwLock::new(HashSet::new())); + let activator = activator(&f, mcp, grants.clone()); + + let err = activator.activate(vec!["gmail".into()], &ctx_of(&f)).await.unwrap_err(); + let ToolFailure::Failed(text) = err else { panic!("expected Failed") }; + assert_eq!(report(&text, "gmail")["status"], "unknown"); + + assert!(grants.read().unwrap().is_empty(), "no in-memory grant"); + assert!( + activated_tools::list_refs_session(&f.pool, 1).await.unwrap().is_empty(), + "no durable row" + ); + + f.pool.close().await; + cleanup(&f.path); + } + + /// Activated by the user, sign-in never completed (§15): diagnosed, not granted. + #[tokio::test] + async fn pending_activation_reports_needs_login() { + let f = fixture("act-pending").await; + crate::db::mcp_catalog::upsert(&f.pool, catalog_entry("gmail", "per_user", "oauth")).await.unwrap(); + crate::db::mcp_user_servers::insert(&f.pool, mcp_user_servers::InsertUserServer { + name: "gmail", catalog_name: Some("gmail"), source: "local_script", transport: "stdio", + command: Some("node"), args_json: None, env_json: None, url: None, api_key: None, + oauth_provider: Some("google"), deliver_json: None, script_rel_path: None, + verify_command: None, verify_script_rel_path: None, auth_state: "pending", + }).await.unwrap(); + + let mcp: Arc = Arc::new(FakeMcp::with_server("tavily", &["search"])); + let grants = Arc::new(RwLock::new(HashSet::new())); + let activator = activator(&f, mcp, grants.clone()); + + let err = activator.activate(vec!["gmail".into()], &ctx_of(&f)).await.unwrap_err(); + let ToolFailure::Failed(text) = err else { panic!("expected Failed") }; + let r = report(&text, "gmail"); + assert_eq!(r["status"], "needs_login"); + assert_eq!(r["description"], "Mail for the user"); + assert!(r["message"].as_str().unwrap().contains("paste the code"), "{r}"); + + assert!(grants.read().unwrap().is_empty()); + assert!(activated_tools::list_refs_session(&f.pool, 1).await.unwrap().is_empty()); + + f.pool.close().await; + cleanup(&f.path); + } + + /// In the catalog, never granted to this user: the admin is the one who can fix it. + #[tokio::test] + async fn catalog_entry_without_grant_reports_not_authorized() { + let f = fixture("act-cat").await; + crate::db::mcp_catalog::upsert(&f.pool, catalog_entry("gmail", "per_user", "oauth")).await.unwrap(); + + let mcp: Arc = Arc::new(FakeMcp::with_server("tavily", &["search"])); + let grants = Arc::new(RwLock::new(HashSet::new())); + let activator = activator(&f, mcp, grants.clone()); + + let err = activator.activate(vec!["gmail".into()], &ctx_of(&f)).await.unwrap_err(); + let ToolFailure::Failed(text) = err else { panic!("expected Failed") }; + assert_eq!(report(&text, "gmail")["status"], "not_authorized"); + + f.pool.close().await; + cleanup(&f.path); + } + + /// A mixed batch activates what it can and diagnoses the rest — the whole + /// call is not lost because one name was wrong. + #[tokio::test] + async fn mixed_batch_activates_only_the_resolvable_ones() { + let f = fixture("act-mixed").await; + let mcp: Arc = Arc::new(FakeMcp::with_server("tavily", &["search"])); + let grants = Arc::new(RwLock::new(HashSet::new())); + let activator = activator(&f, mcp, grants.clone()); + + let text = activator + .activate(vec!["tavily".into(), "gmail".into()], &ctx_of(&f)) + .await + .unwrap(); + assert_eq!(report(&text, "tavily")["status"], "activated"); + assert_eq!(report(&text, "gmail")["status"], "unknown"); + + let set = grants.read().unwrap().clone(); + assert_eq!(set, HashSet::from(["tavily".to_string()])); + assert_eq!(activated_tools::list_refs_session(&f.pool, 1).await.unwrap(), vec!["tavily"]); + + f.pool.close().await; + cleanup(&f.path); + } + + fn catalog_entry<'a>(name: &'a str, scope: &'a str, auth_kind: &'a str) -> mcp_catalog::UpsertCatalog<'a> { + mcp_catalog::UpsertCatalog { + name, scope, source: "local_script", transport: "stdio", + command: Some("node"), args_json: None, env_json: None, url: None, + script_path: Some("gmail/server.js"), config_schema_json: None, auth_kind, + oauth_provider: Some("google"), oauth_scopes_json: None, deliver_json: None, + role_filter: None, verify_command: None, verify_script_path: None, + icon_small_path: None, icon_large_path: None, friendly_name: Some("Gmail"), + description: Some("Mail for the user"), tool_meta_json: None, + version: None, version_string: None, version_release_date: None, + } + } + #[tokio::test] async fn activation_source_resolves_defs_per_anchor() { let f = fixture("act-src").await; diff --git a/crates/skald-core/src/loop_adapters/catalog.rs b/crates/skald-core/src/loop_adapters/catalog.rs index fdcb710..5b5df18 100644 --- a/crates/skald-core/src/loop_adapters/catalog.rs +++ b/crates/skald-core/src/loop_adapters/catalog.rs @@ -202,7 +202,10 @@ impl AgentCatalog for SkaldAgentCatalog { } native.push(Arc::new(ActivateToolsTool::new(Arc::new(SkaldToolActivator::new( self.pool.clone(), + self.shared_pool.clone(), + self.user_id.clone(), self.mcp.clone(), + scope.config_defs.clone(), child_grants.clone(), scope.session_id, Some(child_frame.get()), diff --git a/crates/skald-core/src/loop_adapters/runtime.rs b/crates/skald-core/src/loop_adapters/runtime.rs index 1d6c627..699509d 100644 --- a/crates/skald-core/src/loop_adapters/runtime.rs +++ b/crates/skald-core/src/loop_adapters/runtime.rs @@ -315,7 +315,10 @@ impl UserLoopRuntime { native.push(Arc::new( ActivateToolsTool::new(Arc::new(SkaldToolActivator::new( self.pool.clone(), + self.shared_pool.clone(), + self.user_id.clone(), self.mcp.clone(), + scope.config_defs.clone(), scope.grants.clone(), scope.session_id, None, diff --git a/crates/skald-core/src/session/handler/config.rs b/crates/skald-core/src/session/handler/config.rs index 4f51c61..a1035f6 100644 --- a/crates/skald-core/src/session/handler/config.rs +++ b/crates/skald-core/src/session/handler/config.rs @@ -5,7 +5,7 @@ use serde_json::Value; use crate::tools::tool_names as tn; use super::{ChatSessionHandler, update_scratchpad_tool_def, write_todos_tool_def}; -use super::interface_tools::{AgentRunConfig, InterfaceTool, ToolFuture}; +use super::interface_tools::{AgentRunConfig, InterfaceTool}; /// Returns an `activate_tools` OpenAI tool definition. pub(crate) fn activate_tools_tool_def() -> Value { @@ -18,7 +18,14 @@ pub(crate) fn activate_tools_tool_def() -> Value { keyword `config`, which loads all system-configuration tools (managing \ MCP servers, plugins, scheduled cron jobs, and secrets). \ Pass an array of group names (e.g. [\"gmail\", \"config\"]). \ - Once activated, the tools are available from the next tool-call round onward.", + Only names listed in your context can be activated — never guess one. \ + Returns a JSON object keyed by group name, each with a `status` \ + (`activated`, `needs_login`, `not_activated`, `not_authorized`, \ + `unavailable`, `unknown`), the `tool_prefix` its tools are called \ + under, a `tool_count`, a `description` and a `message` to relay to \ + the user. Only `activated` groups become callable, from the next \ + tool-call round onward; for any other status, tell the user what the \ + `message` says instead of retrying.", "parameters": { "type": "object", "properties": { @@ -44,7 +51,7 @@ impl ChatSessionHandler { client_name: Option, extra_system: Option, extra_system_dynamic: Option, - mut interface_tools: Vec, + interface_tools: Vec, system_substitutions: HashMap, ) -> anyhow::Result { let meta = crate::agents::load_meta(&self.agent_id).ok(); @@ -137,39 +144,15 @@ impl ChatSessionHandler { // ── Tool-group grant initialisation ───────────────────────────────────── // // Load persisted session grants from DB (MCP server names and/or the reserved - // `config` keyword), then inject `activate_tools` so the LLM can activate - // additional groups on demand. + // `config` keyword). The tool itself is native to the loop + // (`SkaldToolActivator`, which shares this very set through the turn scope): + // an interface tool of the same name would be dropped by `NATIVE_NAMES`. let persisted = crate::db::activated_tools::list_refs_session( &self.db, self.session_id, ).await.unwrap_or_default(); let active_mcp_grants: Arc>> = Arc::new(RwLock::new(persisted.into_iter().collect())); - - { - let mcp_clone = Arc::clone(&self.mcp); - let grants_clone = Arc::clone(&active_mcp_grants); - - let activate_tool = crate::tools::activate_tools::ActivateTools { - stack_id: None, - mcp: mcp_clone, - active_mcp_grants: grants_clone, - }; - - let activate_tool = Arc::new(activate_tool); - interface_tools.push(InterfaceTool { - definition: activate_tools_tool_def(), - handler: Arc::new(move |args| -> ToolFuture { - use crate::tools::Tool as _; - let tool = Arc::clone(&activate_tool); - Box::pin(async move { - tokio::task::spawn_blocking(move || tool.execute(args)) - .await - .map_err(|e| anyhow::anyhow!("activate_tools task panicked: {e}"))? - }) - }), - }); - } // ── End tool-group grant initialisation ───────────────────────────────── // Append RunContext system prompt fragments to the dynamic tail (not cached). diff --git a/crates/skald-core/src/tools/activate_tools.rs b/crates/skald-core/src/tools/activate_tools.rs deleted file mode 100644 index c0f1543..0000000 --- a/crates/skald-core/src/tools/activate_tools.rs +++ /dev/null @@ -1,131 +0,0 @@ -use std::collections::HashSet; -use std::sync::{Arc, RwLock}; - -use anyhow::Result; -use serde_json::{Value, json}; - -use crate::mcp::McpProvider; -use crate::tools::tool_names::CONFIG_GROUP; -use crate::tools::{Tool, ToolDescriptionLength, truncate_label, MAX_LABEL_SHORT}; - -/// Per-session (or per-stack) tool that activates **tool groups** on demand. -/// -/// A group is either: -/// - an **MCP server name** — loads that server's tools, or -/// - the reserved keyword `"config"` — loads all built-in `Config`-category -/// tools (system configuration: MCP/plugin/cron management, secrets). -/// -/// When the LLM calls `activate_tools(["gmail", "config"])`, this tool updates -/// the in-memory grant set **immediately**, so the group's tools appear in the -/// *next LLM round* of the current turn (via `all_tool_defs()`). -/// -/// The **durable** record of the activation is written by the round loop -/// (`handle_tool_call`), not here: it anchors the activation to the assistant -/// `message_id` that triggered it, in the owner table `activated_tools` -/// (`stack_id NULL` = session-scoped root grant; `Some(id)` = sub-agent frame -/// grant, deleted on frame exit). Splitting the write this way lets the loop -/// supply the `message_id` — the anchor the DTL serializer positions injected -/// tool blocks against — which this tool does not have. -/// -/// Not in the global `ToolRegistry` — injected as an `InterfaceTool` in -/// `build_agent_config` (root) and `build_sub_agent_config` (sub-agents). -pub struct ActivateTools { - /// `None` for root agents, `Some(stack_id)` for sub-agents. Used only to - /// label the confirmation message; the durable scope is decided by the loop. - pub stack_id: Option, - pub mcp: Arc, - /// Shared in-memory grant set. Updated in-place on every call so subsequent - /// rounds within the same turn see the new tools via `all_tool_defs()`. - pub active_mcp_grants: Arc>>, -} - -impl Tool for ActivateTools { - fn name(&self) -> &str { crate::tools::tool_names::ACTIVATE_TOOLS } - - fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Config } - - fn description(&self) -> &str { - "Activate one or more tool groups so their tools become available. \ - A group is either an MCP server name (see the MCP list) or the reserved \ - keyword `config`, which loads all system-configuration tools (managing \ - MCP servers, plugins, scheduled cron jobs, and secrets). \ - Pass an array of group names. \ - Once activated, the tools are available from the next tool-call round onward." - } - - fn parameters_schema(&self) -> Value { - json!({ - "type": "object", - "properties": { - "groups": { - "type": "array", - "items": { "type": "string" }, - "description": "Tool groups to activate: MCP server names and/or the reserved \ - keyword \"config\" (e.g. [\"gmail\", \"config\"])." - } - }, - "required": ["groups"] - }) - } - - fn describe(&self, args: &Value, _length: ToolDescriptionLength) -> String { - let names = args["groups"] - .as_array() - .map(|a| a.iter().filter_map(|v| v.as_str()).collect::>().join(", ")) - .unwrap_or_else(|| "?".to_string()); - truncate_label(&format!("activate tools [{names}]"), MAX_LABEL_SHORT) - } - - fn execute(&self, args: Value) -> Result { - let names: Vec = args["groups"] - .as_array() - .ok_or_else(|| anyhow::anyhow!("activate_tools: `groups` must be an array"))? - .iter() - .filter_map(|v| v.as_str().map(|s| s.to_string())) - .collect(); - - if names.is_empty() { - anyhow::bail!("activate_tools: `groups` is empty"); - } - - let available: HashSet = self.mcp.tools() - .iter() - .map(|t| t.server_name.clone()) - .collect(); - - // Update the in-memory set so the next LLM round sees the new grants. - // The durable record (with the triggering `message_id`) is written by the - // round loop after this tool returns. - { - let mut set = self.active_mcp_grants.write() - .map_err(|_| anyhow::anyhow!("activate_tools: lock poisoned"))?; - for name in &names { - set.insert(name.clone()); - } - } - - let activated: Vec = names.iter() - .map(|n| { - if n == CONFIG_GROUP { - // Built-in group — always available, no MCP server to reconnect. - format!("{n} ✓") - } else if available.contains(n) { - format!("{n} ✓") - } else { - format!("{n} (registered but not yet running — tools will appear after reconnect)") - } - }) - .collect(); - - let scope = match self.stack_id { - None => "session".to_string(), - Some(s) => format!("stack {s}"), - }; - - Ok(format!( - "Tool groups activated for this {scope}: {}. \ - Their tools are available from the next tool-call round.", - activated.join(", ") - )) - } -} diff --git a/crates/skald-core/src/tools/mod.rs b/crates/skald-core/src/tools/mod.rs index 40c2ff4..07faa8e 100644 --- a/crates/skald-core/src/tools/mod.rs +++ b/crates/skald-core/src/tools/mod.rs @@ -32,7 +32,6 @@ pub fn is_file_read_tool(name: &str) -> bool { } pub mod tool_names; -pub mod activate_tools; pub mod ast_outline; pub mod configure_plugin; pub mod cron_jobs;