diff --git a/Cargo.lock b/Cargo.lock index edfe1fe..77e6269 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -458,7 +458,7 @@ dependencies = [ [[package]] name = "agentkit-tool-skills" -version = "0.10.7" +version = "0.10.8" dependencies = [ "agentkit-capabilities", "agentkit-core", diff --git a/crates/agentkit-tool-compose/Cargo.toml b/crates/agentkit-tool-compose/Cargo.toml index f87921c..c8046cd 100644 --- a/crates/agentkit-tool-compose/Cargo.toml +++ b/crates/agentkit-tool-compose/Cargo.toml @@ -18,7 +18,7 @@ runlet = { version = "0.4.0", optional = true } serde = { workspace = true, features = ["derive"] } serde_json.workspace = true serde_toon2 = { version = "0.2.0", optional = true } -tokio = { workspace = true, features = ["sync"] } +tokio = { workspace = true, features = ["sync", "time"] } [features] default = ["lua"] diff --git a/crates/agentkit-tool-compose/src/tests/runlet.rs b/crates/agentkit-tool-compose/src/tests/runlet.rs index 86f69b7..43ea2a4 100644 --- a/crates/agentkit-tool-compose/src/tests/runlet.rs +++ b/crates/agentkit-tool-compose/src/tests/runlet.rs @@ -1,5 +1,8 @@ -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex as StdMutex}; +use std::time::Duration; + +use tokio::sync::Barrier; use agentkit_core::{MetadataMap, SessionId, ToolCallId, ToolOutput, ToolResultPart, TurnId}; use agentkit_tools_core::{ @@ -40,8 +43,7 @@ async fn execute_compose( #[derive(Clone)] struct OrderingProbeTool { spec: ToolSpec, - active_parallel: Arc, - saw_parallel_overlap: Arc, + parallel_barrier: Arc, events: Arc>>, } @@ -55,8 +57,7 @@ impl OrderingProbeTool { "record compose scheduling", json!({"type": "object"}), ), - active_parallel: Arc::new(AtomicUsize::new(0)), - saw_parallel_overlap: Arc::new(AtomicBool::new(false)), + parallel_barrier: Arc::new(Barrier::new(2)), events: Arc::new(StdMutex::new(Vec::new())), } } @@ -76,15 +77,13 @@ impl Tool for OrderingProbeTool { let kind = request.input["kind"].as_str().unwrap_or_default(); match kind { "parallel_a" | "parallel_b" => { - self.active_parallel.fetch_add(1, Ordering::SeqCst); - for _ in 0..1_000 { - if self.active_parallel.load(Ordering::SeqCst) >= 2 { - self.saw_parallel_overlap.store(true, Ordering::SeqCst); - break; - } - tokio::task::yield_now().await; - } - self.active_parallel.fetch_sub(1, Ordering::SeqCst); + tokio::time::timeout(Duration::from_secs(5), self.parallel_barrier.wait()) + .await + .map_err(|_| { + ToolError::ExecutionFailed( + "independent effectful calls did not overlap".into(), + ) + })?; } "prerequisite" => { self.events @@ -117,7 +116,6 @@ impl Tool for OrderingProbeTool { #[tokio::test] async fn effectful_calls_run_concurrently_and_after_orders_without_data_flow() { let child = OrderingProbeTool::new(); - let saw_parallel_overlap = child.saw_parallel_overlap.clone(); let events = child.events.clone(); let outcome = execute_compose( ComposeConfig::default(), @@ -142,10 +140,6 @@ return [a.kind, b.kind, later.kind]"#, ), other => panic!("unexpected outcome: {other:?}"), } - assert!( - saw_parallel_overlap.load(Ordering::SeqCst), - "independent effectful calls should overlap" - ); assert_eq!( *events.lock().expect("events lock"), vec!["prerequisite:start", "prerequisite:finish", "after:start"] diff --git a/crates/agentkit-tool-skills/Cargo.toml b/crates/agentkit-tool-skills/Cargo.toml index 6f2cdc5..8c05cf8 100644 --- a/crates/agentkit-tool-skills/Cargo.toml +++ b/crates/agentkit-tool-skills/Cargo.toml @@ -4,7 +4,7 @@ homepage.workspace = true name = "agentkit-tool-skills" readme = "README.md" repository.workspace = true -version = "0.10.7" +version = "0.10.8" edition.workspace = true license.workspace = true rust-version.workspace = true diff --git a/crates/agentkit-tool-skills/README.md b/crates/agentkit-tool-skills/README.md index d7232e0..fb75112 100644 --- a/crates/agentkit-tool-skills/README.md +++ b/crates/agentkit-tool-skills/README.md @@ -10,7 +10,7 @@ Progressive Agent Skills discovery and activation for agentkit. This crate discovers `SKILL.md` files, builds a lightweight catalog for the -model, and exposes an `activate_skill` tool that loads full skill instructions +model, and exposes a `skill` tool that loads full skill instructions on demand. ## What it provides @@ -18,7 +18,6 @@ on demand. - recursive skill discovery from one or more roots - exact-directory loading for package formats with fixed discovery rules - frontmatter parsing for skill metadata -- per-session activation tracking to avoid duplicate loads - progressive disclosure so the model sees descriptions first and bodies later ## Example @@ -31,7 +30,7 @@ use agentkit_tool_skills::SkillRegistry; // (`./.agents/skills` and `~/.agents/skills`). let registry = SkillRegistry::discover(".").build().await; -// `tool_registry()` returns a `ToolRegistry` exposing only `activate_skill`, +// `tool_registry()` returns a `ToolRegistry` exposing only `skill`, // ready to merge with the rest of your agent's tools. let tools = agentkit_tools_core::ToolRegistry::new() .merge(registry.tool_registry()); diff --git a/crates/agentkit-tool-skills/src/lib.rs b/crates/agentkit-tool-skills/src/lib.rs index 0cf3e06..2028cea 100644 --- a/crates/agentkit-tool-skills/src/lib.rs +++ b/crates/agentkit-tool-skills/src/lib.rs @@ -2,7 +2,7 @@ //! //! This crate implements the [Agent Skills specification](https://agentskills.io/specification), //! providing a [`SkillRegistry`] that discovers `SKILL.md` files, parses their -//! frontmatter, and exposes an `activate_skill` tool for on-demand loading. +//! frontmatter, and exposes a `skill` tool for on-demand loading. //! //! # Progressive disclosure //! @@ -12,7 +12,7 @@ //! 1. **Catalog** -- skill names and descriptions are listed in the tool //! description at session start (~50-100 tokens per skill). //! 2. **Instructions** -- the full `SKILL.md` body (frontmatter stripped) is -//! loaded only when the model calls `activate_skill`. +//! loaded only when the model calls `skill`. //! 3. **Resources** -- supporting files (scripts, references, assets) are //! enumerated in the activation response; the model reads them on demand. //! @@ -31,12 +31,12 @@ //! # } //! ``` -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::BTreeMap; use std::fs; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; -use agentkit_core::{MetadataMap, SessionId, ToolOutput, ToolResultPart}; +use agentkit_core::{MetadataMap, ToolOutput, ToolResultPart}; use agentkit_tools_core::{ Tool, ToolAnnotations, ToolContext, ToolError, ToolName, ToolRegistry, ToolRequest, ToolResult, ToolSpec, @@ -47,7 +47,7 @@ use serde_json::{Value, json}; use thiserror::Error; const DEFAULT_SKILL_FILE: &str = "SKILL.md"; -const TOOL_NAME: &str = "activate_skill"; +const TOOL_NAME: &str = "skill"; // --------------------------------------------------------------------------- // Public types @@ -132,12 +132,12 @@ where } } -/// Registry of discovered skills that provides the `activate_skill` tool. +/// Registry of discovered skills that provides the `skill` tool. /// /// The registry discovers `SKILL.md` files from one or more directory roots, /// parses their frontmatter, and builds an in-memory catalog. When registered /// as a tool, the model sees a YAML catalog of available skills in the tool -/// description and can call `activate_skill` with a skill name to load its +/// description and can call `skill` with a skill name to load its /// full instructions. /// /// # Discovery order and name collisions @@ -162,7 +162,7 @@ where /// .discover_skills() /// .await; /// -/// // Compose with the agent's other tools; activate_skill rediscovers each turn. +/// // Compose with the agent's other tools; skill rediscovers each turn. /// let tools = agentkit_tools_core::ToolRegistry::new().merge(registry.tool_registry()); /// # Ok(()) /// # } @@ -171,7 +171,6 @@ pub struct SkillRegistry { source: SkillDiscoverySource, filters: Vec>, skills: BTreeMap, - activations: Arc>>>, } impl SkillRegistry { @@ -187,7 +186,6 @@ impl SkillRegistry { source: SkillDiscoverySource::Recursive(roots), filters: Vec::new(), skills: BTreeMap::new(), - activations: Arc::new(Mutex::new(HashMap::new())), } } @@ -207,7 +205,6 @@ impl SkillRegistry { source: SkillDiscoverySource::ExactDirectories(dirs), filters: Vec::new(), skills: BTreeMap::new(), - activations: Arc::new(Mutex::new(HashMap::new())), } } @@ -250,8 +247,6 @@ impl SkillRegistry { /// Re-scan all roots and re-apply filters. /// /// Call this between turns to pick up newly installed or removed skills. - /// Activation tracking is preserved across reloads — skills that were - /// already activated remain marked. pub async fn reload(&mut self) { self.skills = discover_filtered_skills(&self.source, &self.filters); } @@ -267,7 +262,7 @@ impl SkillRegistry { self.skills.values().collect() } - /// Build a [`ToolRegistry`] containing only the `activate_skill` tool. + /// Build a [`ToolRegistry`] containing only the `skill` tool. /// /// The tool remains registered even when discovery is currently empty so /// future turns can surface newly added skills. If no roots are @@ -280,13 +275,11 @@ impl SkillRegistry { registry } - /// Reset activation tracking. After this call, all skills will return - /// their full content on next activation instead of "Skill already read." - pub fn reset_activations(&self) { - self.activations.lock().unwrap().clear(); - } + /// Retained for compatibility; skill reads are no longer deduplicated. + #[deprecated(since = "0.10.8", note = "skill reads are no longer deduplicated")] + pub fn reset_activations(&self) {} - fn build_tool(&self) -> ActivateSkillTool { + fn build_tool(&self) -> SkillTool { let spec = ToolSpec { name: ToolName::new(TOOL_NAME), description: "Load a skill's full instructions into the conversation.".into(), @@ -295,13 +288,13 @@ impl SkillRegistry { "properties": { "name": { "type": "string", - "description": "Name of the skill to activate." + "description": "Name of the skill to load." } }, "required": ["name"], "additionalProperties": false }), - output_schema: None, + output_schema: Some(skill_output_schema()), annotations: ToolAnnotations { read_only_hint: true, ..Default::default() @@ -309,11 +302,10 @@ impl SkillRegistry { metadata: MetadataMap::new(), }; - ActivateSkillTool { + SkillTool { static_spec: spec, source: self.source.clone(), filters: self.filters.clone(), - activations: Arc::clone(&self.activations), } } } @@ -355,30 +347,29 @@ impl SkillRegistry { } // --------------------------------------------------------------------------- -// ActivateSkillTool +// SkillTool // --------------------------------------------------------------------------- #[derive(Deserialize)] -struct ActivateSkillInput { +struct SkillInput { name: String, } -struct ActivateSkillTool { +struct SkillTool { static_spec: ToolSpec, source: SkillDiscoverySource, filters: Vec>, - activations: Arc>>>, } #[async_trait] -impl Tool for ActivateSkillTool { +impl Tool for SkillTool { fn spec(&self) -> &ToolSpec { &self.static_spec } fn current_spec(&self) -> Option { let skills = discover_filtered_skills(&self.source, &self.filters); - (!skills.is_empty()).then(|| build_activate_skill_spec(&skills)) + (!skills.is_empty()).then(|| build_skill_spec(&skills)) } async fn invoke( @@ -386,7 +377,7 @@ impl Tool for ActivateSkillTool { request: ToolRequest, _ctx: &mut ToolContext<'_>, ) -> Result { - let input: ActivateSkillInput = serde_json::from_value(request.input) + let input: SkillInput = serde_json::from_value(request.input) .map_err(|e| ToolError::InvalidInput(format!("invalid input: {e}")))?; let skills = discover_filtered_skills(&self.source, &self.filters); @@ -394,25 +385,6 @@ impl Tool for ActivateSkillTool { .get(&input.name) .ok_or_else(|| ToolError::InvalidInput(format!("unknown skill: {}", input.name)))?; - // Deduplicate within a session, not globally across the registry. - { - let mut activated = self.activations.lock().unwrap(); - let session_activations = activated.entry(request.session_id.clone()).or_default(); - if session_activations.contains(&input.name) { - return Ok(ToolResult { - result: ToolResultPart { - call_id: request.call_id, - output: ToolOutput::Text("Skill already read.".into()), - is_error: false, - metadata: MetadataMap::new(), - }, - duration: None, - metadata: MetadataMap::new(), - }); - } - session_activations.insert(input.name.clone()); - } - // Build the response with body + directory + resources. let mut response = format!( "skill: {name}\ndir: {dir}\n\n{body}", @@ -445,11 +417,11 @@ impl Tool for ActivateSkillTool { // Catalog formatting // --------------------------------------------------------------------------- -fn build_activate_skill_spec(skills: &BTreeMap) -> ToolSpec { +fn build_skill_spec(skills: &BTreeMap) -> ToolSpec { let catalog = build_catalog_yaml(skills); let mut name_schema = json!({ "type": "string", - "description": "Name of the skill to activate." + "description": "Name of the skill to load." }); if !skills.is_empty() { let enum_values: Vec = skills.keys().map(|n| Value::String(n.clone())).collect(); @@ -477,7 +449,7 @@ fn build_activate_skill_spec(skills: &BTreeMap) -> ToolSpec { "required": ["name"], "additionalProperties": false }), - output_schema: None, + output_schema: Some(skill_output_schema()), annotations: ToolAnnotations { read_only_hint: true, ..Default::default() @@ -486,6 +458,13 @@ fn build_activate_skill_spec(skills: &BTreeMap) -> ToolSpec { } } +fn skill_output_schema() -> Value { + json!({ + "type": "string", + "description": "Full skill instructions, skill directory, and resource paths." + }) +} + fn build_catalog_yaml(skills: &BTreeMap) -> String { let mut lines = Vec::new(); for skill in skills.values() { @@ -952,8 +931,8 @@ mod tests { } #[tokio::test] - async fn activation_deduplication() { - let root = temp_dir("dedup"); + async fn repeated_reads_return_instructions() { + let root = temp_dir("repeated-reads"); write_skill(&root, "test-skill", "Test.", "Body content here.").await; let reg = SkillRegistry::from_paths(vec![root.clone()]) @@ -963,7 +942,7 @@ mod tests { let tool = reg.build_tool(); let call_id = ToolCallId::new("call-1"); - // First activation returns body. + // First read returns the body. let request = ToolRequest { call_id: call_id.clone(), tool_name: ToolName::new(TOOL_NAME), @@ -994,7 +973,7 @@ mod tests { }; assert!(text.contains("Body content here.")); - // Second activation returns dedup message. + // Re-reading returns the body again so callers cannot lose the instructions. let request2 = ToolRequest { call_id: ToolCallId::new("call-2"), tool_name: ToolName::new(TOOL_NAME), @@ -1009,7 +988,7 @@ mod tests { ToolOutput::Text(t) => t.clone(), _ => panic!("expected text output"), }; - assert_eq!(text2, "Skill already read."); + assert!(text2.contains("Body content here.")); async_fs::remove_dir_all(&root).await.unwrap(); } @@ -1177,6 +1156,8 @@ mod tests { let schema = &spec.input_schema; let enum_values = schema["properties"]["name"]["enum"].as_array().unwrap(); + assert_eq!(spec.name.0, "skill"); + assert_eq!(spec.output_schema, Some(skill_output_schema())); assert_eq!(enum_values.len(), 2); let names: Vec<&str> = enum_values.iter().map(|v| v.as_str().unwrap()).collect(); assert!(names.contains(&"alpha")); @@ -1186,8 +1167,8 @@ mod tests { } #[tokio::test] - async fn activation_deduplicates_per_session() { - let root = temp_dir("session-dedup"); + async fn reads_are_independent_across_sessions() { + let root = temp_dir("session-reads"); write_skill(&root, "test-skill", "Test.", "Body content here.").await; let reg = SkillRegistry::from_paths(vec![root.clone()]) @@ -1359,70 +1340,6 @@ mod tests { assert!(!reg.has_skills()); } - #[tokio::test] - async fn reset_activations_allows_reread() { - let root = temp_dir("reset"); - write_skill(&root, "reread", "Reread.", "Content.").await; - - let reg = SkillRegistry::from_paths(vec![root.clone()]) - .discover_skills() - .await; - - let tool = reg.build_tool(); - let noop_perms = NoopPermissions; - let mut ctx = ToolContext { - capability: agentkit_capabilities::CapabilityContext { - session_id: None, - turn_id: None, - metadata: &MetadataMap::new(), - }, - permissions: &noop_perms, - resources: &(), - cancellation: None, - execution_scope: None, - approved_request: None, - }; - - // First read. - let req1 = ToolRequest { - call_id: ToolCallId::new("c1"), - tool_name: ToolName::new(TOOL_NAME), - input: json!({ "name": "reread" }), - session_id: agentkit_core::SessionId::new("s"), - turn_id: agentkit_core::TurnId::new("t"), - metadata: MetadataMap::new(), - }; - let r1 = tool.invoke(req1, &mut ctx).await.unwrap(); - assert!(matches!(r1.result.output, ToolOutput::Text(ref t) if t.contains("Content."))); - - // Second read is deduplicated. - let req2 = ToolRequest { - call_id: ToolCallId::new("c2"), - tool_name: ToolName::new(TOOL_NAME), - input: json!({ "name": "reread" }), - session_id: agentkit_core::SessionId::new("s"), - turn_id: agentkit_core::TurnId::new("t"), - metadata: MetadataMap::new(), - }; - let r2 = tool.invoke(req2, &mut ctx).await.unwrap(); - assert!(matches!(r2.result.output, ToolOutput::Text(ref t) if t == "Skill already read.")); - - // Reset and re-read. - reg.reset_activations(); - let req3 = ToolRequest { - call_id: ToolCallId::new("c3"), - tool_name: ToolName::new(TOOL_NAME), - input: json!({ "name": "reread" }), - session_id: agentkit_core::SessionId::new("s"), - turn_id: agentkit_core::TurnId::new("t"), - metadata: MetadataMap::new(), - }; - let r3 = tool.invoke(req3, &mut ctx).await.unwrap(); - assert!(matches!(r3.result.output, ToolOutput::Text(ref t) if t.contains("Content."))); - - async_fs::remove_dir_all(&root).await.unwrap(); - } - // -- test helpers ------------------------------------------------------- struct NoopPermissions; diff --git a/crates/agentkit/Cargo.toml b/crates/agentkit/Cargo.toml index 43738da..234fda3 100644 --- a/crates/agentkit/Cargo.toml +++ b/crates/agentkit/Cargo.toml @@ -33,7 +33,7 @@ agentkit-task-manager = { version = "0.10.5", path = "../agentkit-task-manager", agentkit-tool-compose = { version = "0.10.5", path = "../agentkit-tool-compose", optional = true } agentkit-tool-fs = { version = "0.10.5", path = "../agentkit-tool-fs", optional = true } agentkit-tool-shell = { version = "0.10.5", path = "../agentkit-tool-shell", optional = true } -agentkit-tool-skills = { version = "0.10.6", path = "../agentkit-tool-skills", optional = true } +agentkit-tool-skills = { version = "0.10.8", path = "../agentkit-tool-skills", optional = true } agentkit-tools-core = { version = "0.10.5", path = "../agentkit-tools-core", optional = true } [features] diff --git a/crates/agentkit/src/lib.rs b/crates/agentkit/src/lib.rs index 30f664c..f533ad9 100644 --- a/crates/agentkit/src/lib.rs +++ b/crates/agentkit/src/lib.rs @@ -351,7 +351,7 @@ pub use agentkit_tool_shell as tool_shell; /// Agent Skills tool for progressive skill discovery and activation. /// /// Provides [`tool_skills::SkillRegistry`] which discovers `SKILL.md` files -/// and exposes an `activate_skill` tool for on-demand loading. Skills are +/// and exposes a `skill` tool for on-demand loading. Skills are /// listed in the tool description (catalog tier) and their full content is /// returned only when the model activates them. /// diff --git a/examples/openrouter-context-agent/README.md b/examples/openrouter-context-agent/README.md index 7665e67..f967e0d 100644 --- a/examples/openrouter-context-agent/README.md +++ b/examples/openrouter-context-agent/README.md @@ -5,7 +5,7 @@ One-shot OpenRouter example that wires together: - `agentkit-loop` - `agentkit-provider-openrouter` - `agentkit-context` — loads `AGENTS.md` eagerly -- `agentkit-tool-skills` — discovers skills progressively via the `activate_skill` tool +- `agentkit-tool-skills` — discovers skills progressively via the `skill` tool - `agentkit-reporting` It accepts a single prompt argument, loads `AGENTS.md` into context, registers discovered skills as a tool, runs one turn to completion, and exits. Skills are **not** loaded eagerly — the model sees a catalog of skill names and descriptions in the tool description and activates them on demand. @@ -42,4 +42,4 @@ cargo run -p openrouter-context-agent -- \ - If `--context-root` is omitted, the current working directory is used. - `AGENTS.md` is discovered by searching the context root and its ancestors. - Skills are discovered from `/skills` and `/.agents/skills`. -- If no skills are found, the `activate_skill` tool is not registered. +- If no skills are found, the `skill` tool is not registered.