diff --git a/skills/Cargo.lock b/skills/Cargo.lock index 6c77166e..b2b7c96e 100644 --- a/skills/Cargo.lock +++ b/skills/Cargo.lock @@ -1021,29 +1021,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "skills" -version = "0.1.0" -dependencies = [ - "anyhow", - "async-trait", - "chrono", - "clap", - "cucumber", - "futures", - "iii-sdk", - "schemars", - "serde", - "serde_json", - "serde_yaml", - "tempfile", - "thiserror", - "tokio", - "tracing", - "tracing-subscriber", - "uuid", -] - [[package]] name = "indexmap" version = "2.14.0" @@ -1974,6 +1951,29 @@ dependencies = [ "libc", ] +[[package]] +name = "skills" +version = "0.2.0" +dependencies = [ + "anyhow", + "async-trait", + "chrono", + "clap", + "cucumber", + "futures", + "iii-sdk", + "schemars", + "serde", + "serde_json", + "serde_yaml", + "tempfile", + "thiserror", + "tokio", + "tracing", + "tracing-subscriber", + "uuid", +] + [[package]] name = "slab" version = "0.4.12" diff --git a/skills/Cargo.toml b/skills/Cargo.toml index 945e8c7e..984528bf 100644 --- a/skills/Cargo.toml +++ b/skills/Cargo.toml @@ -2,7 +2,7 @@ [package] name = "skills" -version = "0.1.5" +version = "0.2.0" edition = "2021" publish = false diff --git a/skills/README.md b/skills/README.md index 03fc6f3b..85884eab 100644 --- a/skills/README.md +++ b/skills/README.md @@ -86,8 +86,14 @@ iii.trigger(TriggerRequest { Validation rules (rejected at registration time): -- `id`: lowercase ASCII letters, digits, `-` and `_` only; max 64 chars -- `skill`: non-empty; max 256 KiB +- `id`: 1+ path segments separated by `/`. Each segment uses lowercase + ASCII letters, digits, `-` and `_`; max 64 chars per segment. Total + id length capped at 1024 chars. +- The literal `fn` is **reserved as the first segment** of any id + (used as the section-URI marker — see [Section URIs](#section-uris-function-backed-content)). + `fn` may appear at deeper segments freely (`docs/fn-reference` is + fine). +- `skill`: non-empty; max 256 KiB. Re-registering with the same id overwrites the body and refreshes the `registered_at` timestamp. **Workers MUST re-register on every boot** @@ -99,69 +105,180 @@ during local development. | URI | Returns | |---|---| -| `iii://skills` | Auto-rendered markdown index of every registered skill (entry point) | -| `iii://{your_id}` | The body you registered | -| `iii://{your_id}/{your_function_id}` | Triggers `your_function_id` with `{}` and returns its output as content | +| `iii://skills` | Auto-rendered markdown index of every registered skill (entry point). | +| `iii://{id}` | The body registered at that id (single segment). | +| `iii://{a}/{b}/.../{leaf}` | The body registered at the slashed path. Any depth. First segment must NOT equal `fn`. | +| `iii://fn/{a}/{b}/.../{leaf}` | Trigger function `a::b::...::leaf` with `{}` and serve its output. Each `/` after `fn/` becomes `::`. | + +The first-segment `fn` literal flips the URI from "skill body lookup" +to "function trigger". Anything else as the first segment is treated +as a slashed path of skill ids and resolved against the `skills` +state scope as a single key. This keeps the scheme unambiguous at +arbitrary depth: `iii://resend/email/send` is always a stored +markdown body, while `iii://fn/resend/email/send` is always a call to +`resend::email::send`. -The third shape lets you split a skill across multiple files. Register -your top-level skill at `iii://{your_id}`, then reference sub-content -inside it as markdown links to functions you also registered. Any -function the engine knows about is reachable this way; there's no -opt-in flag. A short reserved-prefix list (engine internals, state -plumbing, this worker's own admin functions) is rejected at read time -to keep the resolver from tunneling back into infra. +The auto-rendered `iii://skills` index uses the first H1 of each skill +as the link title and the first non-heading paragraph as the +description (truncated at 140 chars). Nested entries are indented by +`2 * depth` spaces so the index reads as a tree. Lead each registered +body with a `# {title}` and a short summary paragraph and the index +reads cleanly without further work. -A function backing `iii://{id}/{fn}` should return one of: +#### Modeling a deep skill tree -- a string → served as `text/markdown` -- `{ "content": "..." }` → the `content` field is served as `text/markdown` -- anything else → pretty-printed JSON, served as `application/json` +Top-level skill bodies should stay **small** — a router that links to +deeper, more granular content. The agent only loads `iii://yourworker` +initially; everything else is fetched lazily through `skill::fetch` +(see below). Token cost grows with **how deep the agent actually drills**, +not with how big your worker's docs are. -The auto-rendered `iii://skills` index uses the first H1 of each skill -as the link title and the first non-heading paragraph as the -description (truncated at 140 chars). Lead with a `# {title}` and a -short summary paragraph and the index reads cleanly without further -work. +Each level is an independent state row. Parents don't have to exist +first (orphans are allowed but read awkwardly in the index). A +re-register at the same id overwrites that row only — siblings and +children are untouched. -#### Worked example: skill with sub-skills +Worked example: a small `resend` worker with two depths of nested +skills plus one section URI that points at a function. ```rust use iii_sdk::{RegisterFunction, TriggerRequest}; -use schemars::JsonSchema; -use serde::Serialize; use serde_json::{json, Value}; -#[derive(Serialize, JsonSchema)] -struct SkillContent { - content: String, -} +// 1. Top-level router (small body — links into children). +iii.trigger(TriggerRequest { + function_id: "skills::register".into(), + payload: json!({ + "id": "resend", + "skill": "# resend\n\nEmail provider integration.\n\n\ + - [`email`](iii://resend/email) — sending and tracking\n\ + - [`contacts`](iii://resend/contacts) — audience management\n\ + \n\ + Live status check: \ + [`health`](iii://fn/resend/health)\n", + }), + action: None, + timeout_ms: Some(5_000), +}).await?; +// 2. Mid-level group (also a router into its own children). iii.trigger(TriggerRequest { function_id: "skills::register".into(), payload: json!({ - "id": "brain", - "skill": "# brain\n\nHelps build UIs with Tailwind.\n\n\ - See [`summarize`](iii://brain/brain::summarize) \ - for the catalogue.\n", + "id": "resend/email", + "skill": "# resend/email\n\nEmail flows.\n\n\ + - [`send`](iii://resend/email/send) — outbound\n\ + - [`track`](iii://resend/email/track) — webhooks + status\n", }), action: None, timeout_ms: Some(5_000), }).await?; +// 3. Leaf body — the actual content the agent will read once it +// decides to drill in. Loaded on demand via skill::fetch. +iii.trigger(TriggerRequest { + function_id: "skills::register".into(), + payload: json!({ + "id": "resend/email/send", + "skill": include_str!("../docs/resend-email-send.md"), + }), + action: None, + timeout_ms: Some(5_000), +}).await?; + +// 4. A section URI: live function output instead of a static body. +// `iii://fn/resend/health` triggers `resend::health` with {} and +// returns its output. Useful for status, dynamic catalogues, etc. iii.register_function( - RegisterFunction::new("brain::summarize", |_input: Value| { - Ok::<_, String>(SkillContent { - content: include_str!("../docs/summarize.md").to_string(), - }) + RegisterFunction::new("resend::health", |_input: Value| { + Ok::<_, String>(json!({ + "content": "# Resend health\n\nAPI: ok. Quota: 73% remaining." + })) }) - .description("Index of UI design guidelines."), + .description("Live operational health for the resend integration."), ); ``` -Now any consumer can read `iii://brain` for the orientation and click -through to `iii://brain/brain::summarize` for the catalogue. The -resource resolver invokes the sub-skill function directly through -`iii.trigger`, so it doesn't need any extra opt-in flag. +Once registered, an agent reading `iii://resend` sees the small +router. If it needs send semantics, it asks `skill::fetch` for +`iii://resend/email/send` (one round trip, ~200 lines of markdown +instead of the worker's full 2000-line documentation). + +#### Section URIs (function-backed content) + +Section URIs trigger an iii function with `{}` and serve its output +as resource content. The URI shape is `iii://fn/{a}/{b}/...` and the +mapping to a function id is mechanical: every `/` after `fn/` becomes +`::`. This means agents and humans can construct a URI from any +function id by string-replace alone. + +| URI | Function id triggered | +|---|---| +| `iii://fn/foo` | `foo` | +| `iii://fn/scope/foo` | `scope::foo` | +| `iii://fn/resend/email/send` | `resend::email::send` | +| `iii://fn/a/b/c/d` | `a::b::c::d` | + +A function backing a section URI should return one of: + +- a `String` → served as `text/markdown`. +- `{ "content": "..." }` → the `content` field is served as `text/markdown`. +- anything else → pretty-printed JSON, served as `application/json`. + +The recursion guard blocks the internal namespaces (`engine::*`, +`state::*`, `mcp::*`, `skills::*`, `prompts::*`, `iii::*`, `iii.*`, +`a2a::*`) at read time so a crafted `iii://fn/state/set` can't tunnel +into infra. + +Function ids that contain `.` (e.g. the engine's `iii.on_foo` +lifecycle handlers) are NOT reachable through this shape. That's +intentional — those handlers are triggered on engine events, not by +agents. + +#### Fetching skills on demand + +Top-level skill bodies are designed to stay small (a router that +points at sub-skills) so they don't burn the LLM's context budget on +content the agent might never need. Consumers resolve those `iii://` +links lazily via the **`skill::fetch`** tool — a thin batched wrapper +over the same resolver that backs `skills::resources-read`. + +| Field | Type | Description | +|---|---|---| +| `uri` | string | A single `iii://` URI to read. | +| `uris` | string[] | Multiple `iii://` URIs to read and concatenate. Wins when both are provided. | + +Each URI is wrapped as `# {uri}\n\n{body}` and sections are joined +with `\n\n---\n\n` into one markdown document, so the agent can pull +several sub-skills in one round trip: + +```json +{ "uris": ["iii://resend/email/send", "iii://resend/email/track"] } +``` + +This composes naturally with the multi-level body shape: the agent +loads the top-level router once, then batches fetches for the deeper +bodies it actually needs (and the `iii://fn/...` section URIs of the +functions it expects to call). + +Two registrations, one handler: + +- **`skill::fetch`** — public alias on a non-hidden namespace. MCP + clients see it as the tool `skill__fetch`, which is what the + description tells agents to call when they encounter `iii://` + links in skill instructions. This is the id agents should use. +- **`skills::fetch_skill`** — canonical id, colocated with the rest + of the registry. Hidden from MCP `tools/list` because the bridge + hard-floors every `skills::` prefix; sibling workers can still call + it directly via `iii.trigger`. + +Validation rules (rejected before any resource is touched): + +- At least one of `uri` / `uris` must be present and non-blank after + trim. +- Every URI must start with `iii://`. Other schemes are rejected with + a clear error so an agent can correct the call rather than silently + fetching nothing. ### Prompts (slash commands) @@ -519,11 +636,13 @@ config path. ## Functions -Eleven functions across the two registries. The six public CRUD -entries are callable by any worker over `iii.trigger`. The five -internal-RPC entries are reserved for protocol-bridge workers that -serve the registries to external clients; they never surface as agent -tools. +Thirteen functions across the two registries plus one fetch alias. +The seven public CRUD entries (six registry CRUD + the public fetch +alias) are callable by any worker over `iii.trigger`; the alias is +also surfaced as an MCP tool. The remaining six entries are +internal-RPC reserved for protocol-bridge workers; they never appear +in `tools/list` because their ids start with the `skills::` / +`prompts::` hard-floor prefixes. | Function ID | Description | |---|---| @@ -533,6 +652,8 @@ tools. | `skills::resources-list` | Internal: enumerate registered skills as resource entries. | | `skills::resources-read` | Internal: resolve an `iii://` URI to its content. | | `skills::resources-templates` | Internal: declare the `iii://{id}` URI templates. | +| `skills::fetch_skill` | Internal: batched read across one or more `iii://` URIs. Hidden from MCP because the id starts with `skills::`. | +| `skill::fetch` | Public alias of `skills::fetch_skill` on a non-hidden namespace. Surfaces as the MCP tool `skill__fetch` so agents can resolve `iii://` links on demand. | | `prompts::register` | Store a slash-command prompt definition. | | `prompts::unregister` | Delete a prompt by name. Idempotent. | | `prompts::list` | Metadata-only listing, sorted by name. | @@ -572,12 +693,45 @@ cargo test # One feature group at a time. Available tags: # @pure @markdown -# @engine @skills_register @skills_resources -# @prompts_register @prompts_get @notifications -cargo test --test bdd -- --tags @skills_resources +# @engine @skills_register @skills_resources @skills_fetch @skills_nested +# @prompts_register @prompts_get @notifications @mcp_bridge +cargo test --test bdd -- --tags @skills_nested ``` The BDD harness lives under [tests/](tests/). Feature files mirror the modules in [src/functions/](src/functions/). Step definitions under [tests/steps/](tests/steps/) drive each feature through the same `iii.trigger` path the production binary uses. + +--- + +## Migration (v0.1.x → v0.2.0) + +The URI scheme changed in two breaking ways. Workers that registered +skills against `0.1.x` need to update before upgrading. + +**1. Function-backed URIs use `fn/` as a fixed prefix.** The legacy +two-segment form `iii://{anything}/{worker::function_id}` is gone. +Rewrite each link by prefixing `iii://fn/` and replacing every `::` +with `/`: + +| Old | New | +|---|---| +| `iii://anything/myworker::echo` | `iii://fn/myworker/echo` | +| `iii://brain/brain::summarize` | `iii://fn/brain/summarize` | +| `iii://x/state::get` | `iii://fn/state/get` (still blocked by the recursion guard) | + +The legacy two-segment form now resolves to a state-backed sub-skill +lookup, so it returns "Skill not found" instead of triggering the +function. There is no silent fallback. + +**2. Multi-segment ids are now stored skills.** `iii://resend/email` +and similar are state-backed bodies you can register. There is no +depth cap (per-segment cap stays at 64 chars; total id capped at +1024 chars). The `fn` literal is reserved as the **first** segment of +any registered id; deeper occurrences (`docs/fn-reference`) are fine. + +Workers MUST update their bundled markdown bodies (replace any +`iii://x/worker::fn` in skill text with `iii://fn/worker/fn`) and +re-register at boot. The `skills::on-change` trigger fires on every +re-registration, so subscribers stay in sync automatically. diff --git a/skills/src/functions/mod.rs b/skills/src/functions/mod.rs index 96d43d03..426c8206 100644 --- a/skills/src/functions/mod.rs +++ b/skills/src/functions/mod.rs @@ -22,5 +22,5 @@ pub fn register_all( ) { skills::register(iii, cfg, &trigger_types.skills); prompts::register(iii, cfg, &trigger_types.prompts); - tracing::info!("skills registered 5 skills::* and 5 prompts::* functions"); + tracing::info!("skills registered 7 skills::*, 1 skill::* and 5 prompts::* functions"); } diff --git a/skills/src/functions/skills.rs b/skills/src/functions/skills.rs index e1526166..0c65f7b3 100644 --- a/skills/src/functions/skills.rs +++ b/skills/src/functions/skills.rs @@ -5,6 +5,16 @@ //! * `skills::register` — store a markdown skill body keyed by id. //! * `skills::unregister` — delete one by id (idempotent). //! * `skills::list` — metadata-only listing, sorted by id. +//! * `skills::fetch_skill` — batched read over one or more `iii://` URIs. +//! Internal id only; MCP clients never see it because the bridge +//! hard-floors `skills::*`. +//! +//! Public alias on a non-hidden namespace (visible as the MCP tool +//! `skill__fetch`): +//! +//! * `skill::fetch` — same handler as `skills::fetch_skill`. Lets agents +//! resolve `iii://` links in skill bodies on demand without changes +//! to the mcp worker. //! //! Internal RPC called only by the `mcp` worker (hard-floored under //! `skills::*` so never an MCP tool): @@ -29,10 +39,33 @@ use crate::state; use crate::trigger_types::{self, SubscriberSet}; const SKILL_BODY_MAX_BYTES: usize = 256 * 1024; -const ID_MAX_LEN: usize = 64; + +/// Per-segment cap for both skill ids and section URI segments. +/// The total id is allowed to chain many segments via `/`, but each +/// individual segment stays short so directory listings stay readable. +const ID_SEGMENT_MAX_LEN: usize = 64; + +/// Soft ceiling on the slashed id length. With the per-segment cap above +/// this allows depth ~16 in practice — far deeper than any reasonable +/// tree, while preventing pathological inputs. +const ID_TOTAL_MAX_LEN: usize = 1024; + +/// Reserved as the first path segment of any URI. `iii://fn/...` is the +/// section-URI marker; `iii://anything-else/...` is a state-backed +/// skill body lookup. The reservation only applies to the first segment +/// — `iii://docs/fn-reference` (the literal `fn` deeper in the path) is +/// a perfectly valid skill id. +const FN_PREFIX: &str = "fn"; const INDEX_URI: &str = "iii://skills"; const URI_PREFIX: &str = "iii://"; +/// Description shared by both `skills::fetch_skill` and its public alias +/// `skill::fetch`. Phrased to nudge an MCP client that sees an +/// `iii://...` link in a skill body to call the tool with that URI. +const FETCH_DESCRIPTION: &str = "Fetches the content of one or more skill resources identified by iii:// URIs. \ + When you encounter iii:// links in skill instructions, use this tool to retrieve their contents \ + (batch with `uris` when helpful)."; + /// Prefixes that are NEVER allowed as the `function_id` half of an /// `iii://{skill}/{fn}` resource URI. Mirrors the hard-floor list in /// [mcp/src/handler.rs](../../mcp/src/handler.rs); duplicated here so @@ -105,6 +138,16 @@ struct ReadResourceInput { uri: String, } +#[derive(Debug, Default, Deserialize, JsonSchema)] +pub struct FetchSkillInput { + /// A single iii:// URI to read. Must start with "iii://". + #[serde(default)] + pub uri: Option, + /// Multiple iii:// URIs to read and concatenate into one response. + #[serde(default)] + pub uris: Option>, +} + #[derive(Debug, Serialize, Deserialize, Clone)] struct StoredSkill { id: String, @@ -119,6 +162,8 @@ pub fn register(iii: &Arc, cfg: &Arc, subscribers: &Subscribe register_resources_list(iii, cfg); register_resources_read(iii, cfg); register_resources_templates(iii); + register_fetch_skill(iii, cfg); + register_fetch_skill_public_alias(iii, cfg); } fn register_register_skill(iii: &Arc, cfg: &Arc, subscribers: &SubscriberSet) { @@ -281,6 +326,46 @@ fn register_resources_templates(iii: &Arc) { ); } +/// Internal id, hidden from MCP `tools/list` because the bridge +/// hard-floors every `skills::` prefix. Sibling workers can still call +/// it directly via `iii.trigger`. +fn register_fetch_skill(iii: &Arc, cfg: &Arc) { + let iii_inner = iii.clone(); + let cfg_inner = cfg.clone(); + iii.register_function( + RegisterFunction::new_async("skills::fetch_skill", move |req: FetchSkillInput| { + let iii = iii_inner.clone(); + let cfg = cfg_inner.clone(); + async move { + fetch_skill(&iii, &cfg, req) + .await + .map_err(IIIError::Handler) + } + }) + .description(FETCH_DESCRIPTION), + ); +} + +/// Public alias on a non-hidden namespace so MCP clients see the tool +/// (as `skill__fetch`) without changing the mcp worker. Delegates to +/// the same shared core fn as `skills::fetch_skill`. +fn register_fetch_skill_public_alias(iii: &Arc, cfg: &Arc) { + let iii_inner = iii.clone(); + let cfg_inner = cfg.clone(); + iii.register_function( + RegisterFunction::new_async("skill::fetch", move |req: FetchSkillInput| { + let iii = iii_inner.clone(); + let cfg = cfg_inner.clone(); + async move { + fetch_skill(&iii, &cfg, req) + .await + .map_err(IIIError::Handler) + } + }) + .description(FETCH_DESCRIPTION), + ); +} + // ---------- core resource helpers (also usable by in-process tests) ---------- pub async fn list_resources(iii: &III, cfg: &SkillsConfig) -> Value { @@ -309,13 +394,13 @@ pub fn list_templates() -> Value { { "uriTemplate": "iii://{skill_id}", "name": "Skill", - "description": "Markdown body of a registered skill", + "description": "Markdown body of a registered skill (1+ segments separated by '/')", "mimeType": "text/markdown" }, { - "uriTemplate": "iii://{skill_id}/{function_id}", + "uriTemplate": "iii://fn/{function_path}", "name": "Skill section", - "description": "Markdown returned by triggering function_id with empty input", + "description": "Trigger the function at `function_path` (each '/' becomes '::') with empty input and serve its output. e.g. `iii://fn/scope/echo` triggers `scope::echo`.", "mimeType": "text/markdown" } ] @@ -330,16 +415,20 @@ pub async fn read(iii: &III, cfg: &SkillsConfig, uri: &str) -> Result { + // The slashed path is the state key. Re-validate so a + // crafted `iii://Foo` URI fails fast even if it slipped + // past the section-prefix check. validate_id(&id)?; let stored = read_skill(iii, cfg, &id) .await? .ok_or_else(|| format!("Skill not found: {id}"))?; Ok(wrap_contents(uri, "text/markdown", &stored.skill)) } - ParsedUri::Section { function_id, .. } => { - // Recursion guard — a client that crafts iii://x/state::set would - // otherwise tunnel into infra. We also block skills::* / prompts::* - // so the resource resolver can't drive the admin API. + ParsedUri::Section { function_id } => { + // Recursion guard — a client that crafts iii://fn/state/set + // would otherwise tunnel into infra. We also block + // skills::* / prompts::* so the resource resolver can't + // drive the admin API. if is_always_hidden(&function_id) { return Err(format!( "Function '{function_id}' is in an internal namespace and cannot back a skill resource" @@ -360,18 +449,95 @@ pub async fn read(iii: &III, cfg: &SkillsConfig, uri: &str) -> Result Result, String> { + // `uris` wins when both are provided — matches the TS reference + // impl and the handoff doc. + let raw: Vec = match (input.uris, input.uri) { + (Some(v), _) if !v.is_empty() => v, + (_, Some(s)) if !s.trim().is_empty() => vec![s], + _ => return Err("Provide uri or a non-empty uris array".into()), + }; + let list: Vec = raw + .into_iter() + .map(|u| u.trim().to_string()) + .filter(|u| !u.is_empty()) + .collect(); + if list.is_empty() { + return Err("Provide uri or a non-empty uris array".into()); + } + for u in &list { + if !u.starts_with(URI_PREFIX) { + return Err(format!("Invalid URI (must start with iii://): {u}")); + } + } + Ok(list) +} + +/// Resolve every `iii://` URI in `input` through [`read`], wrap each +/// result as `# {uri}\n\n{text}`, and join sections with +/// `\n\n---\n\n`. Returns plain markdown — the MCP bridge's +/// `tool_text` passes through `Value::String` as `text/plain` content +/// without re-encoding it as JSON. +pub async fn fetch_skill( + iii: &III, + cfg: &SkillsConfig, + input: FetchSkillInput, +) -> Result { + let list = validate_fetch_input(input)?; + let mut sections = Vec::with_capacity(list.len()); + for uri in &list { + let v = read(iii, cfg, uri).await?; + let text = v["contents"] + .as_array() + .map(|arr| { + arr.iter() + .filter_map(|c| c["text"].as_str()) + .collect::>() + .join("\n") + }) + .unwrap_or_default(); + sections.push(format!("# {uri}\n\n{}", text.trim_end())); + } + Ok(sections.join("\n\n---\n\n")) +} + // ---------- URI parsing ---------- #[derive(Debug, PartialEq, Eq)] pub enum ParsedUri { + /// `iii://skills` — the auto-rendered tree-of-skills index. Index, + /// State-backed skill body. The payload is the full slashed path + /// the body was registered under (1+ segments). The first segment + /// is never `fn` — that prefix is reserved for [`Section`]. Skill(String), - Section { - skill_id: String, - function_id: String, - }, + /// Function trigger. The payload is the resolved iii function id + /// built by joining the URI segments after `fn/` with `::`. + /// e.g. `iii://fn/scope/echo` → `function_id == "scope::echo"`. + Section { function_id: String }, } +/// Parse an `iii://...` resource URI into a [`ParsedUri`]. +/// +/// Branching is on the **first path segment**: +/// +/// - Empty body → error. +/// - `skills` → [`ParsedUri::Index`]. +/// - `fn` → [`ParsedUri::Section`]; remaining segments must satisfy +/// [`validate_id_segment`] and are joined with `::` to form the +/// function id. `iii://fn` alone (no segments after) is an error. +/// - Anything else → [`ParsedUri::Skill`] with the full slashed path +/// as the state key. +/// +/// Empty path segments anywhere (`iii://a//b`, leading or trailing +/// `/`) are rejected so the parser stays a strict bijection with the +/// state key. pub fn parse_uri(uri: &str) -> Result { let rest = uri .strip_prefix(URI_PREFIX) @@ -382,51 +548,88 @@ pub fn parse_uri(uri: &str) -> Result { if rest == "skills" { return Ok(ParsedUri::Index); } - match rest.split_once('/') { - None => Ok(ParsedUri::Skill(rest.to_string())), - Some((skill_id, function_id)) => { - if skill_id.is_empty() { - return Err(format!("Empty skill id in URI: {uri}")); - } - if function_id.is_empty() { - return Err(format!("Empty function id in URI: {uri}")); - } - // Function ids contain `::` but not `/`; reject extra path - // segments rather than silently joining them. - if function_id.contains('/') { - return Err(format!( - "Resource URI may not have more than one path segment after the skill id: {uri}" - )); - } - Ok(ParsedUri::Section { - skill_id: skill_id.to_string(), - function_id: function_id.to_string(), - }) + + let segments: Vec<&str> = rest.split('/').collect(); + if segments.iter().any(|s| s.is_empty()) { + return Err(format!( + "Resource URI may not contain empty segments (no leading, trailing, or doubled '/'): {uri}" + )); + } + + if segments[0] == FN_PREFIX { + let fn_segments = &segments[1..]; + if fn_segments.is_empty() { + return Err(format!( + "Section URI 'iii://fn' is missing a function path: expected iii://fn/{{a}}/{{b}}/...: {uri}" + )); + } + for seg in fn_segments { + validate_id_segment(seg) + .map_err(|e| format!("invalid section URI segment {seg:?}: {e}"))?; } + Ok(ParsedUri::Section { + function_id: fn_segments.join("::"), + }) + } else { + Ok(ParsedUri::Skill(rest.to_string())) } } -pub fn validate_id(id: &str) -> Result<(), String> { - if id.is_empty() { - return Err("id must be non-empty".into()); +/// Validate a single id segment. Used for both the per-segment check +/// in [`validate_id`] and the per-segment check inside section URIs +/// in [`parse_uri`]. +pub fn validate_id_segment(s: &str) -> Result<(), String> { + if s.is_empty() { + return Err("segment must be non-empty".into()); } - if id.len() > ID_MAX_LEN { + if s.len() > ID_SEGMENT_MAX_LEN { return Err(format!( - "id too long ({} chars; max {ID_MAX_LEN})", - id.len() + "segment too long ({} chars; max {ID_SEGMENT_MAX_LEN}): {s:?}", + s.len() )); } - for c in id.chars() { + for c in s.chars() { let ok = c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-' || c == '_'; if !ok { return Err(format!( - "id may only contain lowercase ASCII letters, digits, '-' and '_': {id:?}" + "segment may only contain lowercase ASCII letters, digits, '-' and '_': {s:?}" )); } } Ok(()) } +/// Validate a full skill id. Accepts 1+ segments separated by `/`. +/// The first segment must NOT equal [`FN_PREFIX`] (`"fn"`) — that +/// literal is reserved as the section-URI prefix at the top level so +/// `iii://fn/...` is unambiguously a function trigger. Other segments +/// can use `fn` freely (e.g. `docs/fn-reference` is a valid id). +pub fn validate_id(id: &str) -> Result<(), String> { + if id.is_empty() { + return Err("id must be non-empty".into()); + } + if id.starts_with('/') || id.ends_with('/') { + return Err(format!("id may not have a leading or trailing '/': {id:?}")); + } + if id.len() > ID_TOTAL_MAX_LEN { + return Err(format!( + "id too long ({} chars; max {ID_TOTAL_MAX_LEN}): {id:?}", + id.len() + )); + } + let segments: Vec<&str> = id.split('/').collect(); + for (i, seg) in segments.iter().enumerate() { + validate_id_segment(seg) + .map_err(|e| format!("invalid id (segment {} of {:?}): {e}", i + 1, id))?; + } + if segments[0] == FN_PREFIX { + return Err(format!( + "id may not have {FN_PREFIX:?} as its first segment (reserved as the iii://fn/ section-URI marker): {id:?}" + )); + } + Ok(()) +} + // ---------- markdown helpers ---------- async fn render_index(iii: &III, cfg: &SkillsConfig) -> String { @@ -437,13 +640,22 @@ async fn render_index(iii: &III, cfg: &SkillsConfig) -> String { } }; let mut out = String::from( - "# Skills\n\nRead each skill's resource for orientation on when and why to call its functions.\n\n", + "# Skills\n\nRead each skill's resource for orientation on when and why to call its functions. \ + Sub-skills are indented under their parent path so a top-level skill stays small \ + and the LLM can drill in only when it needs more detail.\n\n", ); if skills.is_empty() { out.push_str("_No skills are currently registered._\n"); return out; } + // `list_stored` returns entries sorted lexicographically by id, so a + // single linear pass yields a correct tree: every nested entry + // appears immediately after its parent (or its parent's last + // descendant). Indent each entry by `2 * depth` spaces, where depth + // is the number of '/' separators in the id. for s in skills { + let depth = s.id.matches('/').count(); + let indent = " ".repeat(depth * 2); let title = extract_title(&s.skill).unwrap_or(&s.id); let desc = extract_description(&s.skill).unwrap_or_default(); let suffix = if desc.is_empty() { @@ -452,7 +664,7 @@ async fn render_index(iii: &III, cfg: &SkillsConfig) -> String { format!(" — {desc}") }; out.push_str(&format!( - "- [`{}`](iii://{}) — {}{}\n", + "{indent}- [`{}`](iii://{}) — {}{}\n", s.id, s.id, title, suffix )); } @@ -554,11 +766,15 @@ async fn list_stored(iii: &III, cfg: &SkillsConfig) -> Result, mod tests { use super::*; + // ── parse_uri: index ──────────────────────────────────────────────── + #[test] fn parse_index_uri() { assert_eq!(parse_uri("iii://skills").unwrap(), ParsedUri::Index); } + // ── parse_uri: skill bodies (state-backed) ────────────────────────── + #[test] fn parse_single_skill_uri() { assert_eq!( @@ -568,16 +784,88 @@ mod tests { } #[test] - fn parse_section_uri() { + fn parse_two_segment_skill_uri() { + assert_eq!( + parse_uri("iii://parent/sub").unwrap(), + ParsedUri::Skill("parent/sub".into()) + ); + } + + #[test] + fn parse_three_segment_skill_uri() { + assert_eq!( + parse_uri("iii://a/b/c").unwrap(), + ParsedUri::Skill("a/b/c".into()) + ); + } + + #[test] + fn parse_deeply_nested_skill_uri() { + assert_eq!( + parse_uri("iii://a/b/c/d/e").unwrap(), + ParsedUri::Skill("a/b/c/d/e".into()) + ); + } + + #[test] + fn parse_skill_uri_allows_fn_at_non_first_segment() { + // `fn` is reserved only at depth 0. Deeper occurrences are + // ordinary path segments. assert_eq!( - parse_uri("iii://brain/brain::summarize").unwrap(), + parse_uri("iii://docs/fn-reference").unwrap(), + ParsedUri::Skill("docs/fn-reference".into()) + ); + assert_eq!( + parse_uri("iii://a/fn/c").unwrap(), + ParsedUri::Skill("a/fn/c".into()) + ); + } + + // ── parse_uri: section URIs (function triggers) ───────────────────── + + #[test] + fn parse_section_uri_single_segment() { + // `iii://fn/foo` triggers function `foo` (no scope). + assert_eq!( + parse_uri("iii://fn/foo").unwrap(), + ParsedUri::Section { + function_id: "foo".into(), + } + ); + } + + #[test] + fn parse_section_uri_two_segments_join_with_double_colon() { + assert_eq!( + parse_uri("iii://fn/scope/echo").unwrap(), + ParsedUri::Section { + function_id: "scope::echo".into(), + } + ); + } + + #[test] + fn parse_section_uri_three_segments() { + assert_eq!( + parse_uri("iii://fn/resend/email/send").unwrap(), + ParsedUri::Section { + function_id: "resend::email::send".into(), + } + ); + } + + #[test] + fn parse_section_uri_arbitrary_depth() { + assert_eq!( + parse_uri("iii://fn/a/b/c/d").unwrap(), ParsedUri::Section { - skill_id: "brain".into(), - function_id: "brain::summarize".into(), + function_id: "a::b::c::d".into(), } ); } + // ── parse_uri: error cases ────────────────────────────────────────── + #[test] fn rejects_missing_prefix() { assert!(parse_uri("brain").is_err()); @@ -585,33 +873,105 @@ mod tests { } #[test] - fn rejects_empty_id() { + fn rejects_empty_body() { assert!(parse_uri("iii://").is_err()); + } + + #[test] + fn rejects_empty_segments() { + // Leading slash, trailing slash, doubled slash all yield empty + // segments and must error rather than collapse silently. assert!(parse_uri("iii:///fn").is_err()); assert!(parse_uri("iii://skill/").is_err()); + assert!(parse_uri("iii://a//b").is_err()); + assert!(parse_uri("iii://fn/").is_err()); + } + + #[test] + fn rejects_section_uri_with_no_function_path() { + // `iii://fn` alone has nothing to call. + let err = parse_uri("iii://fn").unwrap_err(); + assert!(err.contains("missing a function path"), "got: {err}"); } #[test] - fn rejects_extra_path_segments() { - assert!(parse_uri("iii://x/y/z").is_err()); + fn rejects_section_uri_with_invalid_segment() { + // Per-segment rules apply to the function path too — uppercase + // ASCII would otherwise smuggle through as a function id. + assert!(parse_uri("iii://fn/Bad-Case").is_err()); + assert!(parse_uri("iii://fn/a/b::c").is_err()); + assert!(parse_uri("iii://fn/a b").is_err()); } + // ── validate_id: happy paths ──────────────────────────────────────── + #[test] - fn id_validation_accepts_kebab_and_underscore() { + fn id_validation_accepts_single_segment() { assert!(validate_id("brain").is_ok()); assert!(validate_id("agent_memory").is_ok()); assert!(validate_id("my-skill-1").is_ok()); assert!(validate_id("a").is_ok()); } + #[test] + fn id_validation_accepts_multi_segment() { + assert!(validate_id("a/b").is_ok()); + assert!(validate_id("a/b/c").is_ok()); + assert!(validate_id("a/b/c/d/e").is_ok()); + assert!(validate_id("resend/email/send").is_ok()); + } + + #[test] + fn id_validation_allows_fn_at_non_first_segment() { + // Reserved-word rule only applies at depth 0. + assert!(validate_id("docs/fn-reference").is_ok()); + assert!(validate_id("a/fn").is_ok()); + assert!(validate_id("a/fn/c").is_ok()); + } + + // ── validate_id: error cases ──────────────────────────────────────── + #[test] fn id_validation_rejects_bad_chars() { assert!(validate_id("").is_err()); assert!(validate_id("UpperCase").is_err()); assert!(validate_id("with space").is_err()); - assert!(validate_id("with/slash").is_err()); assert!(validate_id("with::colon").is_err()); - assert!(validate_id(&"x".repeat(ID_MAX_LEN + 1)).is_err()); + } + + #[test] + fn id_validation_rejects_leading_or_trailing_slash() { + assert!(validate_id("/a").is_err()); + assert!(validate_id("a/").is_err()); + assert!(validate_id("a//b").is_err()); + } + + #[test] + fn id_validation_rejects_fn_as_first_segment() { + let err = validate_id("fn").unwrap_err(); + assert!(err.contains("first segment"), "got: {err}"); + assert!(validate_id("fn/anything").is_err()); + assert!(validate_id("fn/a/b").is_err()); + } + + #[test] + fn id_validation_enforces_per_segment_length() { + let too_long = "x".repeat(ID_SEGMENT_MAX_LEN + 1); + assert!(validate_id(&too_long).is_err()); + let nested_with_long_segment = format!("ok/{too_long}"); + assert!(validate_id(&nested_with_long_segment).is_err()); + let max_segment = "x".repeat(ID_SEGMENT_MAX_LEN); + assert!(validate_id(&max_segment).is_ok()); + } + + #[test] + fn id_validation_enforces_total_length() { + // Build an id just over the total cap: many short segments. + // Each "ab/" is 3 chars; ~342 of them get over 1024. + let too_long: String = "ab/".repeat((ID_TOTAL_MAX_LEN / 3) + 5); + let trimmed = too_long.trim_end_matches('/').to_string(); + assert!(trimmed.len() > ID_TOTAL_MAX_LEN); + assert!(validate_id(&trimmed).is_err()); } #[test] @@ -712,4 +1072,103 @@ mod tests { let templates = v["resourceTemplates"].as_array().unwrap(); assert_eq!(templates.len(), 2); } + + // ── fetch input validation ───────────────────────────────────────── + + #[test] + fn fetch_skill_rejects_no_uri() { + let err = validate_fetch_input(FetchSkillInput::default()).unwrap_err(); + assert!(err.contains("Provide uri"), "got: {err}"); + } + + #[test] + fn fetch_skill_rejects_blank_uri() { + let err = validate_fetch_input(FetchSkillInput { + uri: Some(" ".into()), + uris: None, + }) + .unwrap_err(); + assert!(err.contains("Provide uri"), "got: {err}"); + } + + #[test] + fn fetch_skill_rejects_empty_uris_array() { + let err = validate_fetch_input(FetchSkillInput { + uri: None, + uris: Some(vec![]), + }) + .unwrap_err(); + assert!(err.contains("Provide uri"), "got: {err}"); + } + + #[test] + fn fetch_skill_rejects_non_iii_uri() { + let err = validate_fetch_input(FetchSkillInput { + uri: Some("https://example.com".into()), + uris: None, + }) + .unwrap_err(); + assert!(err.contains("iii://"), "got: {err}"); + } + + #[test] + fn fetch_skill_rejects_non_iii_uri_in_array() { + let err = validate_fetch_input(FetchSkillInput { + uri: None, + uris: Some(vec!["iii://ok".into(), "ftp://nope".into()]), + }) + .unwrap_err(); + assert!(err.contains("iii://"), "got: {err}"); + } + + #[test] + fn fetch_skill_uris_takes_precedence_when_both_provided() { + // `uris` wins; the single `uri` is ignored. + let list = validate_fetch_input(FetchSkillInput { + uri: Some("iii://from-uri".into()), + uris: Some(vec!["iii://from-uris".into()]), + }) + .unwrap(); + assert_eq!(list, vec!["iii://from-uris".to_string()]); + } + + #[test] + fn fetch_skill_trims_whitespace_around_uris() { + let list = validate_fetch_input(FetchSkillInput { + uri: None, + uris: Some(vec![" iii://a ".into(), "iii://b\n".into()]), + }) + .unwrap(); + assert_eq!(list, vec!["iii://a".to_string(), "iii://b".to_string()]); + } + + #[test] + fn fetch_skill_drops_blank_entries_in_uris_array() { + let list = validate_fetch_input(FetchSkillInput { + uri: None, + uris: Some(vec!["iii://a".into(), " ".into(), "iii://b".into()]), + }) + .unwrap(); + assert_eq!(list, vec!["iii://a".to_string(), "iii://b".to_string()]); + } + + #[test] + fn fetch_skill_single_uri_preserved_after_trim() { + let list = validate_fetch_input(FetchSkillInput { + uri: Some(" iii://only ".into()), + uris: None, + }) + .unwrap(); + assert_eq!(list, vec!["iii://only".to_string()]); + } + + #[test] + fn skill_fetch_alias_namespace_not_in_hard_floor() { + // Regression guard: if someone broadens ALWAYS_HIDDEN_PREFIXES + // to also catch `skill::` (singular), the public alias would + // disappear from `tools/list` and fail under the section + // resolver's recursion guard. This pins the namespace as + // user-callable. + assert!(!is_always_hidden("skill::fetch")); + } } diff --git a/skills/src/main.rs b/skills/src/main.rs index cf6ab03c..344fffee 100644 --- a/skills/src/main.rs +++ b/skills/src/main.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use anyhow::Result; use clap::Parser; -use iii_sdk::{register_worker, InitOptions, OtelConfig}; +use iii_sdk::{register_worker, InitOptions, OtelConfig, WorkerMetadata}; use iii_skills::{config, functions, manifest, trigger_types}; @@ -73,7 +73,16 @@ async fn main() -> Result<()> { &cli.url, InitOptions { otel: Some(OtelConfig::default()), - ..Default::default() + metadata: Some(WorkerMetadata { + runtime: "rust".to_string(), + version: env!("CARGO_PKG_VERSION").to_string(), + name: "skills".to_string(), + os: std::env::consts::OS.to_string(), + pid: Some(std::process::id()), + telemetry: None, + ..WorkerMetadata::default() + }), + ..InitOptions::default() }, ); let iii = Arc::new(iii); @@ -83,7 +92,7 @@ async fn main() -> Result<()> { let registered = trigger_types::register_all(&iii); functions::register_all(&iii, &cfg, ®istered); - tracing::info!("skills ready: 10 functions + 2 custom trigger types"); + tracing::info!("skills ready: 13 functions + 2 custom trigger types"); tokio::signal::ctrl_c().await?; tracing::info!("skills shutting down"); diff --git a/skills/tests/features/markdown.feature b/skills/tests/features/markdown.feature index b37a2367..402315fd 100644 --- a/skills/tests/features/markdown.feature +++ b/skills/tests/features/markdown.feature @@ -75,16 +75,36 @@ Feature: pure markdown, URI, and validation helpers When I parse the URI "iii://brain" Then parse_uri returns a skill with id "brain" - Scenario: parse_uri returns Section for iii://skill/fn - When I parse the URI "iii://brain/brain::summarize" - Then parse_uri returns a section with skill "brain" and function "brain::summarize" + Scenario: parse_uri returns Skill for nested iii://parent/sub + When I parse the URI "iii://parent/sub" + Then parse_uri returns a skill with id "parent/sub" + + Scenario: parse_uri returns Skill for arbitrarily deep paths + When I parse the URI "iii://a/b/c/d/e" + Then parse_uri returns a skill with id "a/b/c/d/e" + + Scenario: parse_uri returns Section for iii://fn/{single} + When I parse the URI "iii://fn/echo" + Then parse_uri returns a section with function "echo" + + Scenario: parse_uri returns Section for iii://fn/{a}/{b} joining with :: + When I parse the URI "iii://fn/scope/echo" + Then parse_uri returns a section with function "scope::echo" + + Scenario: parse_uri returns Section for deep iii://fn/{...} + When I parse the URI "iii://fn/resend/email/send" + Then parse_uri returns a section with function "resend::email::send" Scenario: parse_uri rejects a missing scheme When I parse the URI "brain" Then parse_uri fails - Scenario: parse_uri rejects extra path segments - When I parse the URI "iii://x/y/z" + Scenario: parse_uri rejects iii://fn alone (no function path) + When I parse the URI "iii://fn" + Then parse_uri fails + + Scenario: parse_uri rejects empty segments anywhere + When I parse the URI "iii://a//b" Then parse_uri fails # ── validate_id ────────────────────────────────────────────────────── diff --git a/skills/tests/features/mcp_bridge.feature b/skills/tests/features/mcp_bridge.feature index dd8c842d..ad3a7b46 100644 --- a/skills/tests/features/mcp_bridge.feature +++ b/skills/tests/features/mcp_bridge.feature @@ -26,3 +26,15 @@ Feature: skills::resources-* and prompts::mcp-* bridge to the mcp worker Scenario: prompts::mcp-get dispatches the handler and returns the MCP messages shape When I call prompts::mcp-get through the bridge with arguments to=alice@example.com Then the bridged prompts::mcp-get returns a single user message + + # The fetch tool is registered twice: `skills::fetch_skill` (canonical, + # hidden by the MCP hard floor) and `skill::fetch` (public alias). + # This scenario locks in that the alias namespace stays non-hidden so + # the mcp worker continues to surface it in `tools/list`. + Scenario: skill::fetch is registered on a non-hidden namespace so the MCP bridge surfaces it + When I list registered functions through engine::functions::list + Then the listing includes a function "skill::fetch" + And the function "skill::fetch" has a non-empty description + And the function "skill::fetch" is in a non-hidden namespace + And the listing includes a function "skills::fetch_skill" + And the function "skills::fetch_skill" is in the always-hidden namespace floor diff --git a/skills/tests/features/skills_fetch.feature b/skills/tests/features/skills_fetch.feature new file mode 100644 index 00000000..41d6ef9f --- /dev/null +++ b/skills/tests/features/skills_fetch.feature @@ -0,0 +1,75 @@ +@engine @skills_fetch +Feature: skills::fetch_skill and skill::fetch — batched iii:// resource fetch + The fetch tool resolves one or more iii:// URIs through the same + in-process resolver as `skills::resources-read`, wraps each result + as `# {uri}\n\n{text}`, and joins sections with `\n\n---\n\n`. + Both ids share one handler body: `skills::fetch_skill` is the + internal id (hidden from MCP), `skill::fetch` is the public alias + that MCP clients see. + + Background: + Given the iii engine is reachable + And a fetch-skill seeded as "alpha" with body: + """ + # alpha-skill + Alpha body line. + """ + And a fetch-skill seeded as "beta" with body: + """ + # beta-skill + Beta body line. + """ + + # ── happy paths ─────────────────────────────────────────────────────── + + Scenario: skills::fetch_skill with a single uri returns the body wrapped under the URI heading + When I call skills::fetch_skill on seeds "alpha" + Then the fetch call succeeds + And the fetch text starts with "# iii://" followed by the seeded "alpha" id + And the fetch text contains "Alpha body line." + And the fetch text has 1 section + + Scenario: skills::fetch_skill with a uris array concatenates sections separated by --- + When I call skills::fetch_skill on seeds "alpha,beta" + Then the fetch call succeeds + And the fetch text contains "Alpha body line." + And the fetch text contains "Beta body line." + And the fetch text has 2 sections + + Scenario: skill::fetch alias returns identical output to skills::fetch_skill + When I call skill::fetch on seeds "alpha,beta" + Then the fetch call succeeds + And the fetch text contains "Alpha body line." + And the fetch text contains "Beta body line." + And the fetch text has 2 sections + + Scenario: uris takes precedence over uri when both are provided + When I call skills::fetch_skill with both seeds "beta" in uris and seed "alpha" in uri + Then the fetch call succeeds + And the fetch text contains "Beta body line." + And the fetch text does not contain "Alpha body line." + And the fetch text has 1 section + + # ── validation ─────────────────────────────────────────────────────── + + Scenario: missing uri and uris fields returns a validation error + When I call skills::fetch_skill with no uri or uris + Then the fetch call fails with a message mentioning "Provide uri" + + Scenario: a non-iii:// URI is rejected + When I call skills::fetch_skill with the raw URI "https://example.com" + Then the fetch call fails with a message mentioning "iii://" + + Scenario: a blank uri string is rejected + When I call skills::fetch_skill with the raw URI " " + Then the fetch call fails with a message mentioning "Provide uri" + + Scenario: an unknown iii:// id surfaces a not-found error + When I call skills::fetch_skill with the raw URI "iii://does-not-exist-xyz" + Then the fetch call fails with a message mentioning "not found" + + # ── alias parity on the validation branch ──────────────────────────── + + Scenario: skill::fetch surfaces the same validation error as skills::fetch_skill + When I call skill::fetch with no uri or uris + Then the fetch call fails with a message mentioning "Provide uri" diff --git a/skills/tests/features/skills_nested.feature b/skills/tests/features/skills_nested.feature new file mode 100644 index 00000000..516f3dce --- /dev/null +++ b/skills/tests/features/skills_nested.feature @@ -0,0 +1,93 @@ +@engine @skills_nested +Feature: nested skills (multi-level state-backed bodies + path-mapped sections) + Skills can be registered at any depth via slashed ids (`a/b/c/...`). + Each level is an independent state row; parents don't have to exist + first. The auto-rendered `iii://skills` index indents every entry by + `2 * depth` spaces. The function-trigger URI shape uses `fn` as a + fixed first-segment marker — `iii://fn/a/b/c` triggers `a::b::c`. + The `fn` literal is reserved as the first segment of any registered + id so `iii://fn/...` is never ambiguous with a stored body. + + Background: + Given the iii engine is reachable + + # ── deep state-backed bodies ───────────────────────────────────────── + + Scenario: a single-level sub-skill resolves to its own body + Given a nested skill at "parent" with body "# parent\nrouter\n" + And a nested skill at "parent/sub" with body "# child\nleaf body\n" + When I read the nested skill at "parent/sub" + Then the nested read mime is "text/markdown" + And the nested read text contains "leaf body" + + Scenario: deep traversal — three independent bodies at depths 1/2/3 + Given a nested skill at "deep" with body "# deep\ntop\n" + And a nested skill at "deep/middle" with body "# deep/middle\nmid\n" + And a nested skill at "deep/middle/leaf" with body "# deep/middle/leaf\nleaf\n" + When I read the nested skill at "deep" + Then the nested read text contains "top" + When I read the nested skill at "deep/middle" + Then the nested read text contains "mid" + When I read the nested skill at "deep/middle/leaf" + Then the nested read text contains "leaf" + + Scenario: an orphan grandchild is still readable when the parents are absent + Given a nested skill at "orphan/middle/grand" with body "# grand\nleaf only\n" + When I read the nested skill at "orphan/middle/grand" + Then the nested read text contains "leaf only" + + Scenario: skills::list returns nested entries sorted lexicographically + Given a nested skill at "z-tree" with body "# z\n" + And a nested skill at "z-tree/a" with body "# a\n" + And a nested skill at "z-tree/b" with body "# b\n" + When I list the seeded nested ids + Then the listing is sorted with parent before children for "z-tree" + + Scenario: the iii://skills index indents nested entries by depth + Given a nested skill at "tree-root" with body "# Tree root\nTop level body.\n" + And a nested skill at "tree-root/leaf" with body "# Tree leaf\nA child entry.\n" + When I read iii://skills via fetch + Then the index has the seeded entry "tree-root" indented by 0 spaces + And the index has the seeded entry "tree-root/leaf" indented by 2 spaces + + # ── reserved `fn` first segment ────────────────────────────────────── + + Scenario: registering id "fn" is rejected with a "reserved" hint + When I register a skill with id "fn" and body "# nope\n" + Then the skills::register call fails + And the skills::register error mentions "reserved" + + Scenario: registering id starting with "fn/" is rejected + When I register a skill with id "fn/anything" and body "# nope\n" + Then the skills::register call fails + And the skills::register error mentions "reserved" + + Scenario: registering an id with "fn" at a non-first segment succeeds + When I register a skill with id "docs/fn-reference" and body "# fn reference\nallowed\n" + Then the skills::register call succeeds + + # ── path-mapped section URIs ───────────────────────────────────────── + + Scenario: iii://fn/{a}/{b} triggers a::b + Given a deep section function whose id has 2 scope segments + When I read the section URI for the seeded deep function + Then the nested read mime is "text/markdown" + And the nested read text contains "deep-section" + + Scenario: iii://fn/{a}/{b}/{c} triggers a::b::c + Given a deep section function whose id has 3 scope segments + When I read the section URI for the seeded deep function + Then the nested read mime is "text/markdown" + And the nested read text contains "triple-section" + + # ── skill::fetch composition over deep URIs ────────────────────────── + + Scenario: skill::fetch concatenates bodies across depths + Given a nested skill at "fetched" with body "# fetched\nALPHA-BODY\n" + And a nested skill at "fetched/sub" with body "# fetched/sub\nBETA-BODY\n" + And a nested skill at "fetched/sub/leaf" with body "# fetched/sub/leaf\nGAMMA-BODY\n" + When I fetch the seeded nested URIs at depths 1, 2, 3 + Then the fetched text contains "ALPHA-BODY" + And the fetched text contains "BETA-BODY" + And the fetched text contains "GAMMA-BODY" + And the fetched text has 3 sections joined by "---" diff --git a/skills/tests/features/skills_register.feature b/skills/tests/features/skills_register.feature index 69498297..4d792ade 100644 --- a/skills/tests/features/skills_register.feature +++ b/skills/tests/features/skills_register.feature @@ -23,8 +23,16 @@ Feature: skills::register / unregister / list When I register a skill with id "with space" and body "# hi" Then the skills::register call fails - Scenario: register rejects an id with a slash - When I register a skill with id "with/slash" and body "# hi" + Scenario: register rejects an id with a leading slash + When I register a skill with id "/leading-slash" and body "# hi" + Then the skills::register call fails + + Scenario: register rejects an id with a trailing slash + When I register a skill with id "trailing-slash/" and body "# hi" + Then the skills::register call fails + + Scenario: register rejects an id with a doubled slash + When I register a skill with id "doubled//slash" and body "# hi" Then the skills::register call fails Scenario: register rejects an id with a colon diff --git a/skills/tests/features/skills_resources.feature b/skills/tests/features/skills_resources.feature index ef092045..6c0da15b 100644 --- a/skills/tests/features/skills_resources.feature +++ b/skills/tests/features/skills_resources.feature @@ -1,8 +1,9 @@ @engine @skills_resources Feature: iii:// resource resolver (skills::resources-list / read / templates) The `skills::resources-*` internal RPC serves the `iii://skills` - index, registered `iii://{id}` bodies, and `iii://{id}/{fn}` sections - that delegate to a sub-skill function. Normalization turns strings, + index, registered `iii://{id}` bodies (any depth), and + `iii://fn/{...}` sections that delegate to an iii function with the + segments after `fn/` joined by `::`. Normalization turns strings, `{content}` objects, and JSON into the contents envelope shape the `mcp` worker forwards to MCP `resources/read`. @@ -54,54 +55,42 @@ Feature: iii:// resource resolver (skills::resources-list / read / templates) Then the contents mime type is "text/markdown" And the contents text contains "The full body goes here." - # ── section shapes ──────────────────────────────────────────────────── + # ── section shapes (iii://fn/{...}) ─────────────────────────────────── - Scenario: iii://{skill}/{fn} returning a markdown string is served as text/markdown - Given a skill with id "sec-str" and body: - """ - # sec-str - """ - And a sub-skill function that returns a markdown string - When I read the section URI with skill id "sec-str" + Scenario: iii://fn/{...} returning a markdown string is served as text/markdown + Given a sub-skill function that returns a markdown string + When I read the section URI for the seeded function Then the contents mime type is "text/markdown" And the contents text contains "str-section" - Scenario: iii://{skill}/{fn} returning {content} is served as text/markdown - Given a skill with id "sec-obj" and body: - """ - # sec-obj - """ - And a sub-skill function that returns a {content} object - When I read the section URI with skill id "sec-obj" + Scenario: iii://fn/{...} returning {content} is served as text/markdown + Given a sub-skill function that returns a {content} object + When I read the section URI for the seeded function Then the contents mime type is "text/markdown" And the contents text contains "wrapped" - Scenario: iii://{skill}/{fn} returning arbitrary JSON falls back to application/json - Given a skill with id "sec-json" and body: - """ - # sec-json - """ - And a sub-skill function that returns an arbitrary JSON object - When I read the section URI with skill id "sec-json" + Scenario: iii://fn/{...} returning arbitrary JSON falls back to application/json + Given a sub-skill function that returns an arbitrary JSON object + When I read the section URI for the seeded function Then the contents mime type is "application/json" And the contents text contains "count" # ── recursion guard ────────────────────────────────────────────────── Scenario: the section resolver refuses state:: as a function id - When I read the URI "iii://anything/state::set" + When I read the URI "iii://fn/state/set" Then the read fails with a message mentioning "internal namespace" Scenario: the section resolver refuses mcp:: as a function id - When I read the URI "iii://x/mcp::handler" + When I read the URI "iii://fn/mcp/handler" Then the read fails with a message mentioning "internal namespace" Scenario: the section resolver refuses skills:: to block tunnelling into the admin API - When I read the URI "iii://x/skills::register" + When I read the URI "iii://fn/skills/register" Then the read fails with a message mentioning "internal namespace" Scenario: the section resolver refuses prompts:: as a function id - When I read the URI "iii://x/prompts::register" + When I read the URI "iii://fn/prompts/register" Then the read fails with a message mentioning "internal namespace" # ── URI validation ─────────────────────────────────────────────────── @@ -110,10 +99,18 @@ Feature: iii:// resource resolver (skills::resources-list / read / templates) When I read the URI "https://example.com" Then the read fails with a message mentioning "iii://" - Scenario: a URI with more than two path segments is rejected - When I read the URI "iii://a/b/c" - Then the read fails with a message mentioning "more than one path segment" + Scenario: iii://fn alone (no function path after the prefix) is rejected + When I read the URI "iii://fn" + Then the read fails with a message mentioning "missing a function path" + + Scenario: iii://fn/ (trailing slash) is rejected + When I read the URI "iii://fn/" + Then the read fails with a message mentioning "empty segment" Scenario: reading an unknown skill returns a not-found error When I read the URI "iii://no-such-skill-does-not-exist" Then the read fails with a message mentioning "not found" + + Scenario: reading an unknown deep skill returns a not-found error + When I read the URI "iii://no-parent/no-child" + Then the read fails with a message mentioning "not found" diff --git a/skills/tests/steps/markdown.rs b/skills/tests/steps/markdown.rs index 0a328b31..99d7f694 100644 --- a/skills/tests/steps/markdown.rs +++ b/skills/tests/steps/markdown.rs @@ -116,13 +116,10 @@ fn when_parse_uri(world: &mut IiiSkillsWorld, uri: String) { .stash .insert(BUF_OUTPUT.into(), json!({ "kind": "skill", "id": id })); } - Ok(ParsedUri::Section { - skill_id, - function_id, - }) => { + Ok(ParsedUri::Section { function_id }) => { world.stash.insert( BUF_OUTPUT.into(), - json!({ "kind": "section", "skill_id": skill_id, "function_id": function_id }), + json!({ "kind": "section", "function_id": function_id }), ); } Err(e) => { @@ -144,11 +141,10 @@ fn then_parse_skill(world: &mut IiiSkillsWorld, id: String) { assert_eq!(v["id"].as_str(), Some(id.as_str())); } -#[then(regex = r#"^parse_uri returns a section with skill "([^"]+)" and function "([^"]+)"$"#)] -fn then_parse_section(world: &mut IiiSkillsWorld, skill: String, function: String) { +#[then(regex = r#"^parse_uri returns a section with function "([^"]+)"$"#)] +fn then_parse_section(world: &mut IiiSkillsWorld, function: String) { let v = world.stash.get(BUF_OUTPUT).unwrap(); assert_eq!(v["kind"].as_str(), Some("section")); - assert_eq!(v["skill_id"].as_str(), Some(skill.as_str())); assert_eq!(v["function_id"].as_str(), Some(function.as_str())); } diff --git a/skills/tests/steps/mcp_bridge.rs b/skills/tests/steps/mcp_bridge.rs index 743cbc54..5d99da81 100644 --- a/skills/tests/steps/mcp_bridge.rs +++ b/skills/tests/steps/mcp_bridge.rs @@ -230,3 +230,58 @@ fn bridge_mcp_get_shape(world: &mut IiiSkillsWorld) { assert_eq!(messages.len(), 1, "{v}"); assert_eq!(messages[0]["role"].as_str(), Some("user")); } + +// ── engine::functions::list visibility checks (fetch tool) ───────────── + +#[when("I list registered functions through engine::functions::list")] +async fn bridge_engine_list(world: &mut IiiSkillsWorld) { + call(world, "engine::functions::list", json!({})).await; +} + +fn function_entry<'a>(v: &'a Value, function_id: &str) -> Option<&'a Value> { + v["functions"] + .as_array()? + .iter() + .find(|f| f["function_id"].as_str() == Some(function_id)) +} + +#[then(regex = r#"^the listing includes a function "([^"]+)"$"#)] +fn bridge_listing_has(world: &mut IiiSkillsWorld, id: String) { + if world.iii.is_none() { + return; + } + let v = world.stash.get(LAST).expect("no response recorded"); + assert!( + function_entry(v, &id).is_some(), + "missing function {id:?}: {:?}", + v["functions"] + ); +} + +#[then(regex = r#"^the function "([^"]+)" has a non-empty description$"#)] +fn bridge_function_description(world: &mut IiiSkillsWorld, id: String) { + if world.iii.is_none() { + return; + } + let v = world.stash.get(LAST).expect("no response recorded"); + let entry = function_entry(v, &id) + .unwrap_or_else(|| panic!("function {id:?} not in listing: {:?}", v["functions"])); + let desc = entry["description"].as_str().unwrap_or(""); + assert!(!desc.is_empty(), "{id:?} has empty description: {entry}"); +} + +#[then(regex = r#"^the function "([^"]+)" is in a non-hidden namespace$"#)] +fn bridge_function_visible(_world: &mut IiiSkillsWorld, id: String) { + assert!( + !iii_skills::functions::skills::is_always_hidden(&id), + "{id:?} is in the hard-floor namespace list (would be hidden from MCP tools/list)" + ); +} + +#[then(regex = r#"^the function "([^"]+)" is in the always-hidden namespace floor$"#)] +fn bridge_function_hidden(_world: &mut IiiSkillsWorld, id: String) { + assert!( + iii_skills::functions::skills::is_always_hidden(&id), + "{id:?} should be in the hard-floor namespace list (internal-only ids must stay hidden)" + ); +} diff --git a/skills/tests/steps/mod.rs b/skills/tests/steps/mod.rs index f14e1698..16e3e999 100644 --- a/skills/tests/steps/mod.rs +++ b/skills/tests/steps/mod.rs @@ -5,5 +5,7 @@ pub mod mcp_bridge; pub mod notifications; pub mod prompts_get; pub mod prompts_register; +pub mod skills_fetch; +pub mod skills_nested; pub mod skills_register; pub mod skills_resources; diff --git a/skills/tests/steps/skills_fetch.rs b/skills/tests/steps/skills_fetch.rs new file mode 100644 index 00000000..ac300a2a --- /dev/null +++ b/skills/tests/steps/skills_fetch.rs @@ -0,0 +1,233 @@ +//! Step defs for tests/features/skills_fetch.feature. +//! +//! Drives the batched `skills::fetch_skill` tool and its public alias +//! `skill::fetch` through `iii.trigger(...)`. Both ids share one core +//! fn so most scenarios exercise either id; the alias-parity scenarios +//! re-use the same assertions to confirm they return identical output. + +use cucumber::{given, then, when}; +use iii_sdk::TriggerRequest; +use serde_json::{json, Value}; + +use crate::common::world::IiiSkillsWorld; + +const LAST_OK: &str = "skills_fetch_last_ok"; +const LAST_ERR: &str = "skills_fetch_last_err"; + +fn seed_slot(name: &str) -> String { + format!("skills_fetch_seed_id_{name}") +} + +async fn trigger_fetch(world: &mut IiiSkillsWorld, function_id: &str, payload: Value) { + world.stash.remove(LAST_OK); + world.stash.remove(LAST_ERR); + let Some(iii) = world.iii.clone() else { + return; + }; + match iii + .trigger(TriggerRequest { + function_id: function_id.to_string(), + payload, + action: None, + timeout_ms: Some(5_000), + }) + .await + { + Ok(v) => { + world.stash.insert(LAST_OK.into(), v); + } + Err(e) => { + world + .stash + .insert(LAST_ERR.into(), Value::String(e.to_string())); + } + } +} + +async fn register_skill(world: &mut IiiSkillsWorld, id: &str, body: &str) { + let Some(iii) = world.iii.clone() else { + return; + }; + let _ = iii + .trigger(TriggerRequest { + function_id: "skills::register".to_string(), + payload: json!({ "id": id, "skill": body }), + action: None, + timeout_ms: Some(5_000), + }) + .await; +} + +fn seeded_id(world: &IiiSkillsWorld, name: &str) -> String { + world + .stash + .get(&seed_slot(name)) + .and_then(|v| v.as_str()) + .map(String::from) + .unwrap_or_default() +} + +fn last_text(world: &IiiSkillsWorld) -> String { + world + .stash + .get(LAST_OK) + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string() +} + +// ── seeding ───────────────────────────────────────────────────────────── + +#[given(regex = r#"^a fetch-skill seeded as "([^"]+)" with body:$"#)] +async fn seed_named(world: &mut IiiSkillsWorld, name: String, step: &cucumber::gherkin::Step) { + if world.iii.is_none() { + return; + } + let body = step + .docstring + .clone() + .expect("seed body requires a docstring"); + let id = world.scoped_id(&name); + world + .stash + .insert(seed_slot(&name), Value::String(id.clone())); + register_skill(world, &id, &body).await; +} + +// ── fetch invocations ────────────────────────────────────────────────── + +fn build_uris_payload(world: &IiiSkillsWorld, names_csv: &str) -> Value { + let uris: Vec = names_csv + .split(',') + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(|name| format!("iii://{}", seeded_id(world, name))) + .collect(); + json!({ "uris": uris }) +} + +#[when(regex = r#"^I call (skills::fetch_skill|skill::fetch) on seeds "([^"]+)"$"#)] +async fn call_on_seeds(world: &mut IiiSkillsWorld, function_id: String, names_csv: String) { + let payload = build_uris_payload(world, &names_csv); + trigger_fetch(world, &function_id, payload).await; +} + +#[when( + regex = r#"^I call skills::fetch_skill with both seeds "([^"]+)" in uris and seed "([^"]+)" in uri$"# +)] +async fn call_with_both_uri_and_uris( + world: &mut IiiSkillsWorld, + uris_name: String, + uri_name: String, +) { + let uris_payload = build_uris_payload(world, &uris_name); + let single_uri = format!("iii://{}", seeded_id(world, &uri_name)); + let payload = json!({ + "uris": uris_payload["uris"].clone(), + "uri": single_uri, + }); + trigger_fetch(world, "skills::fetch_skill", payload).await; +} + +#[when(regex = r#"^I call (skills::fetch_skill|skill::fetch) with no uri or uris$"#)] +async fn call_without_input(world: &mut IiiSkillsWorld, function_id: String) { + trigger_fetch(world, &function_id, json!({})).await; +} + +#[when(regex = r#"^I call skills::fetch_skill with the raw URI "([^"]*)"$"#)] +async fn call_with_raw_uri(world: &mut IiiSkillsWorld, uri: String) { + trigger_fetch(world, "skills::fetch_skill", json!({ "uri": uri })).await; +} + +// ── assertions ───────────────────────────────────────────────────────── + +#[then("the fetch call succeeds")] +fn fetch_succeeds(world: &mut IiiSkillsWorld) { + if world.iii.is_none() { + return; + } + assert!( + world.stash.contains_key(LAST_OK), + "expected fetch to succeed; got error: {:?}", + world.stash.get(LAST_ERR) + ); +} + +#[then(regex = r#"^the fetch call fails with a message mentioning "([^"]+)"$"#)] +fn fetch_fails(world: &mut IiiSkillsWorld, needle: String) { + if world.iii.is_none() { + return; + } + let err = world + .stash + .get(LAST_ERR) + .and_then(|v| v.as_str()) + .map(String::from) + .unwrap_or_else(|| { + panic!( + "expected fetch to fail; got success: {:?}", + world.stash.get(LAST_OK) + ) + }); + assert!( + err.contains(&needle), + "expected fetch error to mention {needle:?}; got {err:?}" + ); +} + +#[then(regex = r#"^the fetch text contains "([^"]+)"$"#)] +fn fetch_text_contains(world: &mut IiiSkillsWorld, needle: String) { + if world.iii.is_none() { + return; + } + let text = last_text(world); + assert!( + text.contains(&needle), + "expected fetch text to contain {needle:?}; got {text:?}" + ); +} + +#[then(regex = r#"^the fetch text does not contain "([^"]+)"$"#)] +fn fetch_text_excludes(world: &mut IiiSkillsWorld, needle: String) { + if world.iii.is_none() { + return; + } + let text = last_text(world); + assert!( + !text.contains(&needle), + "expected fetch text NOT to contain {needle:?}; got {text:?}" + ); +} + +#[then(regex = r#"^the fetch text starts with "([^"]+)" followed by the seeded "([^"]+)" id$"#)] +fn fetch_text_starts_with_uri_for_seed( + world: &mut IiiSkillsWorld, + prefix: String, + seed_name: String, +) { + if world.iii.is_none() { + return; + } + let text = last_text(world); + let id = seeded_id(world, &seed_name); + let want = format!("{prefix}{id}"); + assert!( + text.starts_with(&want), + "expected fetch text to start with {want:?}; got {text:?}" + ); +} + +#[then(regex = r#"^the fetch text has (\d+) sections?$"#)] +fn fetch_text_section_count(world: &mut IiiSkillsWorld, want: usize) { + if world.iii.is_none() { + return; + } + let text = last_text(world); + // Sections are joined with `\n\n---\n\n`. n sections => n-1 separators. + let separators = text.matches("\n\n---\n\n").count(); + let got = if text.is_empty() { 0 } else { separators + 1 }; + assert_eq!( + got, want, + "expected {want} sections (separators+1); got {got}. text={text:?}" + ); +} diff --git a/skills/tests/steps/skills_nested.rs b/skills/tests/steps/skills_nested.rs new file mode 100644 index 00000000..87d04bf5 --- /dev/null +++ b/skills/tests/steps/skills_nested.rs @@ -0,0 +1,332 @@ +//! Step defs for tests/features/skills_nested.feature. +//! +//! Exercises the v2 URI scheme: arbitrary-depth state-backed skills +//! (`iii://a/b/c/...`), the `fn`-reserved-first-segment rule on +//! `skills::register`, the path-mapped section URI shape +//! (`iii://fn/a/b/c` → `a::b::c`), and `skill::fetch` composition +//! across depths. + +use cucumber::{given, then, when}; +use iii_sdk::{IIIError, RegisterFunction, TriggerRequest}; +use serde_json::{json, Value}; + +use crate::common::world::IiiSkillsWorld; + +const LAST_OK: &str = "skills_nested_last_ok"; +const LAST_ERR: &str = "skills_nested_last_err"; +const LAST_READ: &str = "skills_nested_last_read"; +const LAST_LIST: &str = "skills_nested_last_list"; +const LAST_INDEX: &str = "skills_nested_last_index"; +const LAST_FETCH: &str = "skills_nested_last_fetch"; +const DEEP_FN: &str = "skills_nested_deep_fn"; + +fn nested_id_slot(id: &str) -> String { + format!("skills_nested_id_{id}") +} + +fn seeded_full(world: &IiiSkillsWorld, base_id: &str) -> String { + world + .stash + .get(&nested_id_slot(base_id)) + .and_then(|v| v.as_str()) + .map(String::from) + .unwrap_or_default() +} + +/// Map a base id like `parent/sub` to its scenario-scoped form by +/// scoping ONLY the first segment so the slashed structure of the +/// remaining segments is preserved. e.g. `parent/sub` becomes +/// `parent-/sub`. +fn scoped_nested(world: &IiiSkillsWorld, base_id: &str) -> String { + match base_id.split_once('/') { + Some((root, rest)) => format!("{}/{}", world.scoped_id(root), rest), + None => world.scoped_id(base_id), + } +} + +async fn trigger( + world: &mut IiiSkillsWorld, + function_id: &str, + payload: Value, + ok_slot: &str, + err_slot: &str, +) { + world.stash.remove(ok_slot); + world.stash.remove(err_slot); + let Some(iii) = world.iii.clone() else { + return; + }; + match iii + .trigger(TriggerRequest { + function_id: function_id.to_string(), + payload, + action: None, + timeout_ms: Some(5_000), + }) + .await + { + Ok(v) => { + world.stash.insert(ok_slot.into(), v); + } + Err(e) => { + world + .stash + .insert(err_slot.into(), Value::String(e.to_string())); + } + } +} + +// ── seeding nested skills ────────────────────────────────────────────── + +#[given(regex = r#"^a nested skill at "([^"]+)" with body "([^"]*)"$"#)] +async fn seed_nested(world: &mut IiiSkillsWorld, base_id: String, body: String) { + let scoped = scoped_nested(world, &base_id); + world + .stash + .insert(nested_id_slot(&base_id), Value::String(scoped.clone())); + trigger( + world, + "skills::register", + json!({ "id": scoped, "skill": body }), + LAST_OK, + LAST_ERR, + ) + .await; + if world.stash.contains_key(LAST_ERR) { + let err = world + .stash + .get(LAST_ERR) + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + panic!("seeding nested skill {scoped:?} failed: {err}"); + } +} + +// ── reading nested skills ────────────────────────────────────────────── + +#[when(regex = r#"^I read the nested skill at "([^"]+)"$"#)] +async fn read_nested(world: &mut IiiSkillsWorld, base_id: String) { + let scoped = seeded_full(world, &base_id); + let uri = format!("iii://{scoped}"); + trigger( + world, + "skills::resources-read", + json!({ "uri": uri }), + LAST_READ, + LAST_ERR, + ) + .await; +} + +#[then(regex = r#"^the nested read mime is "([^"]+)"$"#)] +fn nested_read_mime(world: &mut IiiSkillsWorld, want: String) { + if world.iii.is_none() { + return; + } + let v = world.stash.get(LAST_READ).expect("no read recorded"); + let mime = v["contents"][0]["mimeType"].as_str().unwrap_or(""); + assert_eq!(mime, want, "mime mismatch in: {v}"); +} + +#[then(regex = r#"^the nested read text contains "([^"]+)"$"#)] +fn nested_read_text_contains(world: &mut IiiSkillsWorld, needle: String) { + if world.iii.is_none() { + return; + } + let v = world.stash.get(LAST_READ).expect("no read recorded"); + let text = v["contents"][0]["text"].as_str().unwrap_or(""); + assert!(text.contains(&needle), "missing {needle:?} in: {text}"); +} + +// ── skills::list (lex sort confirms parent precedes children) ────────── + +#[when("I list the seeded nested ids")] +async fn list_nested(world: &mut IiiSkillsWorld) { + trigger(world, "skills::list", json!({}), LAST_LIST, LAST_ERR).await; +} + +#[then(regex = r#"^the listing is sorted with parent before children for "([^"]+)"$"#)] +fn listing_sorted_for_root(world: &mut IiiSkillsWorld, root_base: String) { + if world.iii.is_none() { + return; + } + let parent = seeded_full(world, &root_base); + let v = world.stash.get(LAST_LIST).expect("no list recorded"); + let arr = v["skills"].as_array().expect("missing .skills array"); + let ids: Vec<&str> = arr.iter().filter_map(|e| e["id"].as_str()).collect(); + let parent_pos = ids + .iter() + .position(|id| *id == parent) + .unwrap_or_else(|| panic!("parent {parent:?} not in listing: {ids:?}")); + // Every entry whose id starts with "/" must appear AFTER parent. + let prefix = format!("{parent}/"); + for (i, id) in ids.iter().enumerate() { + if id.starts_with(&prefix) { + assert!( + i > parent_pos, + "child {id:?} at pos {i} appears before parent {parent:?} at pos {parent_pos}; ids={ids:?}" + ); + } + } + // Confirm the listing is globally sorted, which is the property + // the index renderer relies on. + let mut sorted = ids.clone(); + sorted.sort(); + assert_eq!(ids, sorted, "listing not sorted by id: {ids:?}"); +} + +// ── iii://skills index render ────────────────────────────────────────── + +#[when("I read iii://skills via fetch")] +async fn fetch_index(world: &mut IiiSkillsWorld) { + trigger( + world, + "skills::resources-read", + json!({ "uri": "iii://skills" }), + LAST_INDEX, + LAST_ERR, + ) + .await; +} + +#[then(regex = r#"^the index has the seeded entry "([^"]+)" indented by (\d+) spaces$"#)] +fn index_indent(world: &mut IiiSkillsWorld, base_id: String, indent_spaces: usize) { + if world.iii.is_none() { + return; + } + let scoped = seeded_full(world, &base_id); + let v = world.stash.get(LAST_INDEX).expect("no index recorded"); + let text = v["contents"][0]["text"].as_str().unwrap_or(""); + let needle_indent = " ".repeat(indent_spaces); + let bullet_prefix = format!("{needle_indent}- [`{scoped}`](iii://{scoped})"); + assert!( + text.lines().any(|l| l.starts_with(&bullet_prefix)), + "missing line starting with {bullet_prefix:?} in:\n{text}" + ); +} + +// ── reuse of skills_register's failure-path assertions ───────────────── +// +// The `fn`-reserved scenarios share the existing skills_register step +// helpers (`I register a skill with id "" and body ""`, +// `the skills::register call fails`, `the skills::register error +// mentions ""`) — so no new step defs are needed for them. + +// ── path-mapped section URIs ─────────────────────────────────────────── + +#[given("a deep section function whose id has 2 scope segments")] +async fn seed_deep_2(world: &mut IiiSkillsWorld) { + let Some(iii) = world.iii.clone() else { + return; + }; + let fn_id = format!("bdd::deep-{}", world.unique_id); + world + .stash + .insert(DEEP_FN.into(), Value::String(fn_id.clone())); + iii.register_function( + RegisterFunction::new_async(fn_id.clone(), |_input: Value| async move { + Ok::<_, IIIError>(Value::String("# deep-section\nlevel 2".into())) + }) + .description("bdd: deep section, 2 segments"), + ); + tokio::time::sleep(std::time::Duration::from_millis(60)).await; +} + +#[given("a deep section function whose id has 3 scope segments")] +async fn seed_deep_3(world: &mut IiiSkillsWorld) { + let Some(iii) = world.iii.clone() else { + return; + }; + // Three `::`-separated parts. We use kebab-case so each segment + // satisfies validate_id_segment and the URI builder produces a + // clean `iii://fn/a/b/c` path. + let fn_id = format!("bdd::deep::triple-{}", world.unique_id); + world + .stash + .insert(DEEP_FN.into(), Value::String(fn_id.clone())); + iii.register_function( + RegisterFunction::new_async(fn_id.clone(), |_input: Value| async move { + Ok::<_, IIIError>(Value::String("# triple-section\nthree levels".into())) + }) + .description("bdd: deep section, 3 segments"), + ); + tokio::time::sleep(std::time::Duration::from_millis(60)).await; +} + +#[when("I read the section URI for the seeded deep function")] +async fn read_deep_section(world: &mut IiiSkillsWorld) { + let fn_id = world + .stash + .get(DEEP_FN) + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + let path = fn_id.replace("::", "/"); + let uri = format!("iii://fn/{path}"); + trigger( + world, + "skills::resources-read", + json!({ "uri": uri }), + LAST_READ, + LAST_ERR, + ) + .await; +} + +// ── skill::fetch composition ─────────────────────────────────────────── + +#[when("I fetch the seeded nested URIs at depths 1, 2, 3")] +async fn fetch_three_depths(world: &mut IiiSkillsWorld) { + let one = seeded_full(world, "fetched"); + let two = seeded_full(world, "fetched/sub"); + let three = seeded_full(world, "fetched/sub/leaf"); + let uris = vec![ + format!("iii://{one}"), + format!("iii://{two}"), + format!("iii://{three}"), + ]; + trigger( + world, + "skill::fetch", + json!({ "uris": uris }), + LAST_FETCH, + LAST_ERR, + ) + .await; +} + +fn fetch_text(world: &IiiSkillsWorld) -> String { + world + .stash + .get(LAST_FETCH) + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string() +} + +#[then(regex = r#"^the fetched text contains "([^"]+)"$"#)] +fn fetched_text_contains(world: &mut IiiSkillsWorld, needle: String) { + if world.iii.is_none() { + return; + } + let text = fetch_text(world); + assert!(text.contains(&needle), "missing {needle:?} in: {text}"); +} + +#[then(regex = r#"^the fetched text has (\d+) sections joined by "([^"]+)"$"#)] +fn fetched_section_count(world: &mut IiiSkillsWorld, want: usize, sep_inner: String) { + if world.iii.is_none() { + return; + } + let text = fetch_text(world); + // Sections are joined with `\n\n---\n\n`; the visible delimiter + // inside the join is just `---`. + let delim = format!("\n\n{sep_inner}\n\n"); + let sep_count = text.matches(delim.as_str()).count(); + let got = if text.is_empty() { 0 } else { sep_count + 1 }; + assert_eq!( + got, want, + "expected {want} sections joined by {sep_inner:?}; got {got}. text={text:?}" + ); +} diff --git a/skills/tests/steps/skills_resources.rs b/skills/tests/steps/skills_resources.rs index 51407a5e..c48182eb 100644 --- a/skills/tests/steps/skills_resources.rs +++ b/skills/tests/steps/skills_resources.rs @@ -66,12 +66,23 @@ async fn seed_skill_block( .await; } +/// Each scenario uses a per-scenario unique function id whose +/// `::`-separated parts each pass `validate_id_segment`. The unique +/// id is constructed from the World's `unique_id` (a UUID-derived +/// kebab string) so it survives the new section-URI segment validator. +fn section_fn_id(world: &IiiSkillsWorld, label: &str) -> String { + // Two segments: `bdd::