Skip to content

Refactor skill handling in iii-directory#131

Merged
sergiofilhowz merged 2 commits into
mainfrom
feat/unifying-api-skills-and-prompts
May 13, 2026
Merged

Refactor skill handling in iii-directory#131
sergiofilhowz merged 2 commits into
mainfrom
feat/unifying-api-skills-and-prompts

Conversation

@sergiofilhowz
Copy link
Copy Markdown
Contributor

@sergiofilhowz sergiofilhowz commented May 13, 2026

  • Updated skill retrieval methods: replaced directory::skills::fetch-skill with directory::skills::get for fetching individual skill bodies.
  • Enhanced directory::skills::list to return enriched skill metadata including title and description.
  • Adjusted related documentation and comments to reflect the new method names and their usage.
  • Removed deprecated fetch-skill references throughout the codebase and documentation.
  • Updated tests to align with the new skill handling approach.

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced skills discovery with enriched listing including metadata (title, description).
    • Simplified single-skill retrieval API for improved accessibility.
  • Bug Fixes

    • Corrected skill ID validation rules for better flexibility.
  • Documentation

    • Revised API documentation to reflect streamlined skills access patterns.
    • Updated integration guides for skills loading and display.

- Updated skill retrieval methods: replaced `directory::skills::fetch-skill` with `directory::skills::get` for fetching individual skill bodies.
- Enhanced `directory::skills::list` to return enriched skill metadata including title and description.
- Adjusted related documentation and comments to reflect the new method names and their usage.
- Removed deprecated `fetch-skill` references throughout the codebase and documentation.
- Updated tests to align with the new skill handling approach.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Warning

Rate limit exceeded

@sergiofilhowz has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 3 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 577bcf53-ec17-4a4d-8038-df277f179303

📥 Commits

Reviewing files that changed from the base of the PR and between 3444cc5 and d6363c8.

📒 Files selected for processing (1)
  • turn-orchestrator/src/states/provisioning.rs
📝 Walkthrough

Walkthrough

This PR refactors the iii-directory skills API from a batched URI-based fetch-skill model to a dual-function directory::skills::list (enriched with markdown-derived title/description) plus directory::skills::get (single-skill body retrieval) model. The change cascades across documentation, consumer integrations (harness UI, turn-orchestrator, shell), and test suites, replacing all references to the deprecated fetch pathway with the new contract.

Changes

Skills API Refactoring

Layer / File(s) Summary
Core API contract redefinition
iii-directory/src/functions/skills.rs, iii-directory/skills/index.md, iii-directory/README.md
Introduces SkillGetInput/SkillGetOutput structs, normalize_get_id helper for optional iii:// prefix stripping, and rewires SkillEntry to include title/description parsed from markdown H1 and first paragraph. Removes FetchSkillInput, ParsedUri, parse_uri, validate_fetch_input, and fetch_skill. Updated validation rules drop the reserved-fn restriction. API docs redefined: list is now enriched (id/title/description/bytes/modified_at), get returns flat body-inclusive shape (id/title/description/body/modified_at).
Listing implementation with markdown parsing
iii-directory/src/functions/skills.rs
skill_entry_from_fs now reads each skill markdown body to extract title from first H1 and description from first paragraph, falling back to empty values on read errors. Maintains bytes and modified_at from filesystem metadata.
Skills ID validation and normalization
iii-directory/src/functions/skills.rs, AGENTS-NEW-WORKER.md
Updated validation rules specify 1+ /-separated segments with per-segment character/length constraints and removed the fn first-segment reservation. normalize_get_id validates incoming ids and conditionally strips iii:// prefix, rejecting other schemes. Input documentation clarified for both bare and legacy iii://{id} forms.
ID-based skill retrieval
iii-directory/src/functions/skills.rs, iii-directory/skills/directory/skills/get.md
New get_skill function loads skill body by caller-provided id, normalizes via normalize_get_id, reads markdown, extracts title/description, and returns complete metadata-inclusive payload. Handles legacy URI prefix transparently. Returns "not found" error for missing files. Full spec documented with worked example and related endpoints.
Skill contract and routing documentation
AGENTS-NEW-WORKER.md, iii-directory/skills/directory/skills/list.md, iii-directory/skills/directory/skills/get.md
Rewritten skill markdown "router vs leaf" contract explaining that router loads first via H1, leaf bodies load on-demand; leaf links use function id while resolution uses stripped skill id; leaf H1 becomes list row title. List and get API docs fully specified with input/output shapes, behavior notes (no caching, immediate file reflection), and related operations.
Config and related references
iii-directory/config.yaml, iii-directory/src/config.rs, iii-directory/src/lib.rs, iii-directory/src/main.rs
Added system_prompt_skills glob patterns. Updated field/module documentation to reference directory::skills::list/directory::skills::get instead of fetch-skill and URI schemes. No behavior changes to serialization or defaults.

Harness UI Integration

Layer / File(s) Summary
Menu item generation from skill rows
harness/web/src/menuItems.ts, harness/web/src/menuItems.test.ts
New SkillRow interface (id/title?/description?) and skillsListToMenuItems function replaces markdown-based skillsIndexToMenuItems. Converts enriched rows into /id-keyed menu items with formatted description (title — description fallback), meta.uri as iii:// prefix added back. Handles null/empty/invalid rows gracefully. Tests cover null handling, row projection, title fallbacks, nested ids, and filtering empty ids.
Composer component props update
harness/web/src/components/Composer.tsx, harness/web/src/App.tsx
Composer now takes `skillRows: SkillRow[]

System Prompt & Provisioning Refactoring

Layer / File(s) Summary
Skills bootstrap pipeline rewrite
turn-orchestrator/src/states/provisioning.rs
Replaces pre-rendered iii://directory/skills index fetching with new fetch_skills_bootstrap that calls directory::skills::list (via list_skills), parses enriched rows (via parse_skill_row), locally renders markdown index with id-depth-based indentation and optional descriptions, and fetches root skill bodies individually via directory::skills::get (via fetch_root_bodies). Introduces SkillRow struct, is_root_skill_id depth predicate, and error-tolerant per-skill body loading. Expands test coverage for row parsing, index rendering (indentation/links/descriptions), and zero-skill edge case.
System prompt instructions update
turn-orchestrator/src/system_prompt.rs
Canonical prompt and generated "Available skills" section updated to instruct directory::skills::get (bare id, single call per skill, iii:// stripping) instead of batched fetch-skill. Updated tests assert rule against re-fetching inlined roots, continued guidance for deeper sub-skills, and fallback references to list/get vs old tree index. Removed batching language.

Test Coverage & Documentation Updates

Layer / File(s) Summary
Read surface feature & step tests
iii-directory/tests/features/read.feature, iii-directory/tests/steps/read.rs
New directory::skills::list scenarios assert title/description population from markdown. New directory::skills::get scenarios validate id/title/description/body/modified_at fields, acceptance of iii:// prefix, immediate file reflection, unknown-id failures, and non-iii:// scheme rejection. Removed legacy iii:// tree and fetch-skill concatenation scenarios. Added list_entry helper and get result stashing (LAST_GET/LAST_GET_ERR).
Inline documentation & comment updates
harness/docs/iii-skill.md, iii-directory/src/functions/directory.rs, iii-directory/src/how_to.rs, iii-directory/skills/directory/engine/functions/info.md, iii-directory/skills/directory/skills/download.md, shell/src/lib.rs, turn-orchestrator/src/agent_call.rs
All references to directory::skills::fetch-skill replaced with directory::skills::get. Comments clarified function-vs-skill-id distinction, iii:// prefix handling, and per-skill loading semantics. No code logic changes in these files.
Prompts contract documentation
iii-directory/skills/directory/prompts.md
New file fully specifying directory::prompts::list and directory::prompts::get with JSON shapes, YAML frontmatter behavior, naming/validation, sorting, caching-free re-reads, and on-change side effect after download. Aligns flat shape ({ name, description, body, modified_at }) with skills get output.
Deprecated API removal
iii-directory/skills/directory/skills/fetch-skill.md
File removed; documentation for deprecated directory::skills::fetch-skill (including URI batching, output wrapping, worked examples) eliminated entirely.

Sequence Diagram

sequenceDiagram
    participant UI as Harness UI
    participant Dir as directory::skills::*
    participant Prompt as System Prompt<br/>Builder
    participant Agent as Agent

    Note over UI,Prompt: Old Flow: Fetch-based (removed)
    UI->>Dir: fetch-skill iii://directory/skills
    Dir-->>UI: rendered tree index
    
    Note over UI,Prompt: New Flow: List + Get
    UI->>Dir: list()
    Dir-->>UI: enriched rows<br/>(id/title/description)
    UI->>UI: build menu items<br/>from rows
    
    Prompt->>Dir: list()
    Dir-->>Prompt: enriched rows
    Prompt->>Prompt: render markdown index<br/>locally (root only)
    
    loop For each root skill
        Prompt->>Dir: get(id)
        Dir-->>Prompt: body + metadata
    end
    
    Prompt->>Agent: system prompt<br/>(inlined roots)
    Agent->>Agent: parse instructions:<br/>call get() for sub-skills
    
    Agent->>Dir: get(sub-skill-id)
    Dir-->>Agent: body + metadata
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

The PR spans ~30 files with substantial logic changes in two critical areas: skills.rs (~400 line delta with validation/normalization/listing logic), and provisioning.rs (~120 line delta refactoring the bootstrap pipeline). While many files receive straightforward documentation or prop-wiring updates, the semantic API migration requires careful verification of contract consistency across consumers (harness UI, turn-orchestrator, shell). Test coverage is comprehensive but necessitates understanding the new enriched-row and per-skill-get patterns. The changes are heterogeneous enough to demand separate reasoning for the three major integration points (UI, prompting, system prompt), though they follow a consistent refactoring theme.

Possibly related PRs

  • iii-hq/workers#124: Establishes the original directory::skills::list and fetch-skill behavior that this PR refactors away in favor of the new directory::skills::get single-skill contract.
  • iii-hq/workers#102: Introduces the skillsIndex + skillsIndexToMenuItems pipeline in harness UI that this PR replaces entirely with enriched SkillRow + skillsListToMenuItems sourced from directory::skills::list.

Suggested reviewers

  • ytallo

🐰 A skills rewrite, oh what a sight!
From fetch-batches murky to list-and-get bright,
Rich titles now gleam in each markdown row,
While sub-skills load solo—no batching in tow! ✨
The prompt now directs this fresh skill-loading way,
A cleaner contract to rule the new day.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main refactoring effort: replacing fetch-skill with get and enriching list for skill handling across the iii-directory worker.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/unifying-api-skills-and-prompts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 13, 2026

skill-check — worker

2 verified, 23 skipped (no docs/).

Layer Result
structure
vale
ai

Three for three. Nicely done.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
iii-directory/skills/directory/prompts.md (1)

89-91: 💤 Low value

Documentation claim about mirroring is slightly imprecise.

The text states "The shape mirrors directory::skills::get exactly (with name standing in for that surface's id)" but there's a subtle difference:

  • directory::skills::get returns: { id, title, description, body, modified_at }
  • directory::prompts::get returns: { name, description, body, modified_at }

Prompts don't have a separate title field because name serves as both identifier and display label, whereas skills separate the path-like id from the human-readable title.

Consider revising to "The shape closely mirrors directory::skills::get (with name serving as both identifier and display label, equivalent to the combined role of id + title in skills)."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iii-directory/skills/directory/prompts.md` around lines 89 - 91, Update the
documentation in prompts.md to avoid saying the shapes mirror exactly: change
the sentence referencing directory::skills::get so it explains prompts lack a
separate title and that name functions as both identifier and display label;
e.g., replace "The shape mirrors `directory::skills::get` exactly (with `name`
standing in for that surface's `id`)" with wording like "The shape closely
mirrors `directory::skills::get` (with `name` serving as both identifier and
display label, equivalent to the combined role of `id` + `title` in skills)."
Ensure this edit is applied where `directory::prompts::get` is described so
callers understand the difference.
AGENTS-NEW-WORKER.md (1)

248-254: ⚡ Quick win

Clarify iii:// prefix handling in directory::skills::get.

The documentation instructs users to "strip the iii:// prefix" before calling directory::skills::get, but the test scenarios (particularly iii-directory/tests/features/read.feature line 83-92: "directory::skills::get accepts the legacy iii:// prefix") demonstrate that the API accepts both forms for backwards compatibility.

Consider clarifying whether:

  • Users must strip the prefix (strict requirement), or
  • Users should strip the prefix (best practice, but the API handles both).

The current wording implies a requirement but the implementation provides optional compatibility.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@AGENTS-NEW-WORKER.md` around lines 248 - 254, The doc paragraph incorrectly
implies a strict requirement to strip the iii:// prefix before calling
directory::skills::get; update the wording to state that directory::skills::get
accepts both the legacy iii://<worker>/<sub> form and the stripped id, but
recommend (best practice) callers use the stripped form; mention
directory::skills::get, the legacy iii:// prefix, iii.trigger, and the
auth-credentials / auth::set_token example so readers see the compatibility and
the preferred form.
iii-directory/src/functions/skills.rs (1)

262-265: ⚡ Quick win

Avoid full-tree scan for single get lookups.

Line 263 rescans the entire skills tree for each directory::skills::get. Consider a direct validated id→path resolver to keep single-skill reads O(1)-ish on larger skill sets.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iii-directory/src/functions/skills.rs` around lines 262 - 265, The
find_fs_skill function currently rescans the entire skills tree via
fs_source::scan_skills every time (used by directory::skills::get), which is
O(n); replace this with a direct id→path resolver that validates the id and
constructs the skill file path from SkillsConfig::resolved_skills_folder() (or a
canonical index if available) and then loads/parses that single FsSkill;
implement a new helper (e.g., resolve_skill_path or lookup_skill_by_id) that
checks for path traversal/invalid ids, builds the expected file path for the
given id, verifies existence, and returns the parsed FsSkill instead of calling
fs_source::scan_skills.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@iii-directory/src/functions/skills.rs`:
- Around line 143-146: The current code uses
extract_title(&body).map(str::to_string).unwrap_or_else(|| fs.id.clone()) which
treats an empty H1 ("#") as a valid title (empty string); change the chain to
reject empty/whitespace titles by filtering the mapped string (e.g.,
.map(str::to_string).filter(|s| !s.trim().is_empty()).unwrap_or_else(||
fs.id.clone())) so blank H1 falls back to fs.id; apply the same change to the
other occurrence (the title assignment near lines 275-278) to keep fallback
behavior stable while leaving extract_description usage unchanged.

---

Nitpick comments:
In `@AGENTS-NEW-WORKER.md`:
- Around line 248-254: The doc paragraph incorrectly implies a strict
requirement to strip the iii:// prefix before calling directory::skills::get;
update the wording to state that directory::skills::get accepts both the legacy
iii://<worker>/<sub> form and the stripped id, but recommend (best practice)
callers use the stripped form; mention directory::skills::get, the legacy iii://
prefix, iii.trigger, and the auth-credentials / auth::set_token example so
readers see the compatibility and the preferred form.

In `@iii-directory/skills/directory/prompts.md`:
- Around line 89-91: Update the documentation in prompts.md to avoid saying the
shapes mirror exactly: change the sentence referencing directory::skills::get so
it explains prompts lack a separate title and that name functions as both
identifier and display label; e.g., replace "The shape mirrors
`directory::skills::get` exactly (with `name` standing in for that surface's
`id`)" with wording like "The shape closely mirrors `directory::skills::get`
(with `name` serving as both identifier and display label, equivalent to the
combined role of `id` + `title` in skills)." Ensure this edit is applied where
`directory::prompts::get` is described so callers understand the difference.

In `@iii-directory/src/functions/skills.rs`:
- Around line 262-265: The find_fs_skill function currently rescans the entire
skills tree via fs_source::scan_skills every time (used by
directory::skills::get), which is O(n); replace this with a direct id→path
resolver that validates the id and constructs the skill file path from
SkillsConfig::resolved_skills_folder() (or a canonical index if available) and
then loads/parses that single FsSkill; implement a new helper (e.g.,
resolve_skill_path or lookup_skill_by_id) that checks for path traversal/invalid
ids, builds the expected file path for the given id, verifies existence, and
returns the parsed FsSkill instead of calling fs_source::scan_skills.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a9a9fe66-089f-4eff-b254-ea30d092ad1d

📥 Commits

Reviewing files that changed from the base of the PR and between 13037ca and 3444cc5.

📒 Files selected for processing (28)
  • AGENTS-NEW-WORKER.md
  • harness/docs/iii-skill.md
  • harness/web/src/App.tsx
  • harness/web/src/components/Composer.tsx
  • harness/web/src/menuItems.test.ts
  • harness/web/src/menuItems.ts
  • iii-directory/README.md
  • iii-directory/config.yaml
  • iii-directory/skills/directory/engine/functions/info.md
  • iii-directory/skills/directory/prompts.md
  • iii-directory/skills/directory/skills/download.md
  • iii-directory/skills/directory/skills/fetch-skill.md
  • iii-directory/skills/directory/skills/get.md
  • iii-directory/skills/directory/skills/list.md
  • iii-directory/skills/index.md
  • iii-directory/src/config.rs
  • iii-directory/src/functions/directory.rs
  • iii-directory/src/functions/mod.rs
  • iii-directory/src/functions/skills.rs
  • iii-directory/src/how_to.rs
  • iii-directory/src/lib.rs
  • iii-directory/src/main.rs
  • iii-directory/tests/features/read.feature
  • iii-directory/tests/steps/read.rs
  • shell/src/lib.rs
  • turn-orchestrator/src/agent_call.rs
  • turn-orchestrator/src/states/provisioning.rs
  • turn-orchestrator/src/system_prompt.rs
💤 Files with no reviewable changes (1)
  • iii-directory/skills/directory/skills/fetch-skill.md

Comment on lines +143 to +146
let title = extract_title(&body)
.map(str::to_string)
.unwrap_or_else(|| fs.id.clone());
let description = extract_description(&body).unwrap_or_default();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Treat empty H1 as missing so title fallback remains stable.

Line 143 and Line 275 can emit an empty title when markdown has a blank H1 (#), even though fallback behavior expects id when no usable title exists.

Proposed fix
-    let title = extract_title(&body)
-        .map(str::to_string)
-        .unwrap_or_else(|| fs.id.clone());
+    let title = extract_title(&body)
+        .filter(|s| !s.is_empty())
+        .map(str::to_string)
+        .unwrap_or_else(|| fs.id.clone());
-            let title = extract_title(&body)
-                .map(str::to_string)
-                .unwrap_or_else(|| fs.id.clone());
+            let title = extract_title(&body)
+                .filter(|s| !s.is_empty())
+                .map(str::to_string)
+                .unwrap_or_else(|| fs.id.clone());

Also applies to: 275-278

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iii-directory/src/functions/skills.rs` around lines 143 - 146, The current
code uses extract_title(&body).map(str::to_string).unwrap_or_else(||
fs.id.clone()) which treats an empty H1 ("#") as a valid title (empty string);
change the chain to reject empty/whitespace titles by filtering the mapped
string (e.g., .map(str::to_string).filter(|s|
!s.trim().is_empty()).unwrap_or_else(|| fs.id.clone())) so blank H1 falls back
to fs.id; apply the same change to the other occurrence (the title assignment
near lines 275-278) to keep fallback behavior stable while leaving
extract_description usage unchanged.

@sergiofilhowz sergiofilhowz merged commit 8d3b77b into main May 13, 2026
36 checks passed
ytallo added a commit that referenced this pull request May 13, 2026
Reflects PR #131 (merged to main) which removed directory::skills::fetch-skill
in favor of directory::skills::get { id }. Adds two snapshot pins so a
future edit that re-introduces the old function name fails CI.
ytallo added a commit that referenced this pull request May 13, 2026
Reflects PR #131 (merged to main):
- DefaultSkillBody gains id field, populated by new from_config_uri
  constructor that strips the iii:// prefix.
- IDENTITY_PREAMBLE references directory::skills::get with the id field.
- Failed-skill recovery stub teaches `directory::skills::get { id: "..." }`.
- fetch_uri calls directory::skills::get with { id }; the existing
  response_to_string already handles the new full envelope because the
  body field is at the same JSON path.
- Adds 4 required test updates and 4 hygiene assertions per the plan.
ytallo added a commit that referenced this pull request May 13, 2026
Reflects PR #131 (merged to main):
- Function rename: directory::skills::fetch-skill -> directory::skills::get
- Field rename: { uri } -> { id }
- DefaultSkillBody gains id field + from_config_uri constructor
- Chat-init pseudocode updated to show singular per-URI calls (the new
  API has no batched form)
- Failed-skill stub example text updated to teach the new call shape
ytallo added a commit that referenced this pull request May 13, 2026
Reflects PR #131 (merged to main) which removed directory::skills::fetch-skill
in favor of directory::skills::get { id }. Adds two snapshot pins so a
future edit that re-introduces the old function name fails CI.
ytallo added a commit that referenced this pull request May 13, 2026
Reflects PR #131 (merged to main):
- DefaultSkillBody gains id field, populated by new from_config_uri
  constructor that strips the iii:// prefix.
- IDENTITY_PREAMBLE references directory::skills::get with the id field.
- Failed-skill recovery stub teaches `directory::skills::get { id: "..." }`.
- fetch_uri calls directory::skills::get with { id }; the existing
  response_to_string already handles the new full envelope because the
  body field is at the same JSON path.
- Adds 4 required test updates and 4 hygiene assertions per the plan.
ytallo added a commit that referenced this pull request May 13, 2026
Reflects PR #131 (merged to main):
- Function rename: directory::skills::fetch-skill -> directory::skills::get
- Field rename: { uri } -> { id }
- DefaultSkillBody gains id field + from_config_uri constructor
- Chat-init pseudocode updated to show singular per-URI calls (the new
  API has no batched form)
- Failed-skill stub example text updated to teach the new call shape
ytallo added a commit that referenced this pull request May 14, 2026
…127)

* feat(iii-directory): ship iii and sandbox orientation skills

Move the two harness-embedded orientation bodies (previously pushed via
the deprecated `skills::register` shim) into iii-directory's own skills
tree so they are served from disk like every other worker's docs.

After download, they resolve at iii://iii-directory/iii and
iii://iii-directory/sandbox.

* refactor(workers): drop in-process skill registration from bundled workers

Each of auth-credentials, approval-gate, shell, turn-orchestrator,
models-catalog, llm-budget, and session-tree boots its own copy of the
canonical 5s→60s→180s skills::register / skills::unregister retry loop
(~80 LOC per worker). With iii-directory now serving skills from disk,
those calls fire against a function id the engine no longer routes —
silent on success, noisy on shutdown.

Strip the retry loops, the include_str! of skill.md at boot, and the
constants that only fed unregister. The skill.md files stay at each
worker root; the existing registry-publish workflow already maps them
to index.md, so the URI is now iii://<worker>/index.

Two surgical fixes in turn-orchestrator unblock the new URI shape:

* system_prompt.rs: the hardcoded iii://sandbox reference becomes
  iii://iii-directory/sandbox to match where the body now lives.
* states/provisioning.rs: is_root_skill_id rejected any id containing
  a slash, which would have dropped every <worker>/index body from
  the bootstrap. Accept <single-segment> OR <single-segment>/index;
  still reject deeper paths like shell/bash.

Drop now-unused skills handshake/timeout keys from llm-budget and
models-catalog configs. approval-gate/skill.md's top link is bumped
to /index so it survives the URI shape change.

* feat(harness): migrate to iii-directory with first-boot bootstrap

Replace the deprecated skills worker dep with iii-directory and add a
synchronous, bounded-concurrency bootstrap step so a fresh install ends
up with iii://<worker>/index resolvable for every bundled worker before
register_with_iii_with_engine_url returns.

Wiring:

* iii.worker.yaml swaps skills ^0.2.0 for iii-directory ^0.4.1.
* harness/config.yaml gains the iii-directory block (skills_folder
  ./data/skills, registry/cache defaults).

New module harness::skills:

* BOOTSTRAP_NAMES lists the nine workers that publish skills (the
  harness itself, iii-directory, plus the seven cleaned up in the
  previous commit). Infra deps that never ship skill.md are
  intentionally excluded.
* bootstrap_run calls skills::list once, diffs against BOOTSTRAP_NAMES,
  and triggers skills::download {worker, version} for the missing
  namespaces. Versions are read from iii.lock so binaries and docs
  stay aligned across bundle releases.
* Downloads run on a JoinSet bounded by a Semaphore(4); failures log
  warnings but never panic. Eight unit tests cover the
  success / list-fails / iii-directory-not-ready / download-error /
  download-timeout paths plus the BOOTSTRAP_NAMES drift guard.

harness::fanout grows two new pumps: skills::on-change and
prompts::on-change broadcast to ui::skills::changed::<browser_id> and
ui::prompts::changed::<browser_id>, mirroring the existing agent::events
shape so mcp and the web UI refresh without polling.

The three skills::register trigger blocks and their builder helpers are
deleted; EXPECTED_WORKERS stays as the build-time manifest assert and
keeps feeding harness::status. The old TEMP-revert comments referencing
the orientation shims are gone with the shims.

A new harness/skill.md replaces the runtime-pushed orientation body so
iii://harness/index resolves from disk like every other worker; a lib
test asserts every EXPECTED_WORKERS entry is referenced in it.

Web surface:

* menuItems.ts parses iii://{id} directly (was iii://skills/<id>),
  matching iii-directory's renderer. Tests follow.

CI:

* Per-worker pr-checks enforces <worker>/skill.md presence (non-empty,
  ≤256 KiB) for every BOOTSTRAP_NAMES entry, with failure messages
  pointing at AGENTS-NEW-WORKER.md §10.
* A new harness-only step forbids the function_id "skills::register"
  string literal in Rust sources to prevent regressions. The pattern is
  scoped tightly enough that iii-directory's own namespace registration
  and mcp's is_hidden test fixture stay false-positive-free.

Docs:

* AGENTS-NEW-WORKER.md §10 is rewritten around the file-based publish
  flow; no boot-time RPC required.
* TODOS.md drops the iii-sdk register/unregister helpers entry (closed
  by deletion of the duplicated boot code) and the CI presence-check
  entry (closed by the new pr-checks step).
* The design spec is updated to reflect the final file-naming decision
  and the per-worker cargo invocation.

End-to-end smoke is captured as harness/tests/bootstrap_e2e.rs and
gated behind #[ignore] since it needs a running engine.

* chore: auto-render skill artifacts

* docs: design spec for iii agent base prompt with default skills

Captures the two-part system prompt (identity preamble + config-driven
default skill bodies fetched per chat from iii-directory), the new
system_default_skills key in harness/config.yaml, failure model, test
strategy, and 5-step migration plan.

* docs: implementation plan for iii base prompt with default skills

Seven tasks: absorb iii teaching into iii.md, add system_default_skills
config, thread cfg through subscriber/transitions/provisioning, rewrite
system_prompt::build with the new DefaultSkillBody shape, rewrite
provisioning to fetch per-URI from iii-directory at chat start, add iii.md
snapshot tests in iii-directory, sweep for dead helpers. TDD throughout
with one combined commit for the build() + caller rewrite to preserve
build-ability.

* docs(iii-directory): absorb iii teaching content into iii.md

Adds primitives definitions, agent_call argument contract, injection
boundary, recovery rules, and path conventions previously baked into
turn-orchestrator's BASE_BODY constant. Removes engine::workers::register
(worker-boot machinery, not agent-facing). Prepares iii.md to be served
as a fetched default skill instead of inlined at build time.

* docs(iii-directory): use agent-facing `function` in iii.md examples

Code review on the prior commit flagged that TL;DR/Step 1/checklist
examples used the SDK-side `function_id` field, contradicting the
`function` rule taught at the top of the file. An agent copying those
examples into agent_call would get {error: "missing_function"} on the
first try. Examples now use the agent path; SDK examples in Step 3 are
left intact.

* feat(turn-orchestrator): add system_default_skills config

Lists iii:// URIs to fetch from iii-directory at the start of every
new chat. Default ships with [iii://iii]; operators add more URIs to
pre-load worker root skills. Field is parsed but not yet consumed —
plumbing and prompt assembly land in subsequent commits.

* test(turn-orchestrator): close impl-default cross-check gap

Extends impl_default_matches_yaml_defaults to cover the new
system_default_skills field. Also clarifies the YAML comment ('per-URI
headers' → 'a heading per URI') so operators don't mistake the term
for HTTP headers.

* feat(turn-orchestrator): thread config through state machine

Subscriber, transitions, and provisioning state now receive
TurnOrchestratorConfig. Plumbing only — provisioning ignores the new
parameter pending the prompt-assembly rewrite in the next commit.

* style(turn-orchestrator): clean Arc import + restore Stopped comment

Address code-review nits from c1ac8ef: replace inline std::sync::Arc
path in provisioning::handle signature with a top-level use, and
restore the 'no-op: idempotent terminal state' comment on the Stopped
match arm that was inadvertently dropped.

* feat(turn-orchestrator): two-part system prompt with fetched defaults

Replaces the ~280-line BASE_BODY constant with a 9-line IDENTITY_PREAMBLE
hard-coded in the binary, plus per-URI skill bodies fetched at chat
start from iii-directory via directory::skills::fetch-skill.

- system_prompt::build now takes &[DefaultSkillBody] (uri + Option<body>)
  instead of an opaque skills_index string.
- Each fetched body is inlined under a '# <uri>' header.
- Failed fetches degrade per-URI to a recovery stub naming the URI;
  the preamble always survives.
- provisioning::handle now zips cfg.system_default_skills with the
  fetched body map in config order.

Old BASE_BODY content lives in iii://iii (see prior commit). The legacy
SkillsIndex pathway and root-skill auto-inlining (is_root_skill_id,
list_root_skill_uris) are removed.

* fix(turn-orchestrator): prompt newline + fetch error context

Address review of c78ed09:
- Drop the extra trailing newline after the cwd line so every section
  boundary in the assembled prompt uses the same \n\n separator.
- Remove a dead let-binding in the provisioning test that claimed to
  suppress an unused-warning that never existed.
- Preserve the underlying trigger error when directory::skills::fetch-skill
  fails so chat-init failures are diagnosable from logs.

* test(iii-directory): snapshot tests for agent-facing iii.md content

Pins primitives definitions, agent_call argument contract, error
envelope shapes, recovery rules, injection boundary, descriptor field
names, schema-probe block, and absolute-paths rule. These assertions
previously lived in turn-orchestrator/system_prompt.rs but moved here
when BASE_BODY was deleted — iii-directory now owns the wording.

* test(iii-directory): tighten iii_skill_content snapshot needles

Address code-review nits on 44d2b65:
- Drop bare-word "Function"/"Trigger"/"Worker" asserts that could
  false-match in tables; assert the bolded primitive names from the
  framing block instead.
- Add failure messages to the four error-envelope asserts so a
  regression surfaces with context, matching the rest of the file.
- Tighten the missing_function recovery needle from 10 chars to the
  full intended sentence.

* chore: address final review notes

- Document the per-URI fetch decision in fetch_default_skills and align
  the spec with what the implementation actually does (sequential
  singular calls, not batched, for per-URI failure granularity).
- Replace the whitespace-sensitive 'Resend with\n  exactly' snapshot
  needle with two line-wrap-independent assertions covering the same
  intent.

* docs(iii-directory): rename fetch-skill → get in iii.md + hygiene pins

Reflects PR #131 (merged to main) which removed directory::skills::fetch-skill
in favor of directory::skills::get { id }. Adds two snapshot pins so a
future edit that re-introduces the old function name fails CI.

* refactor(turn-orchestrator): adopt directory::skills::get API

Reflects PR #131 (merged to main):
- DefaultSkillBody gains id field, populated by new from_config_uri
  constructor that strips the iii:// prefix.
- IDENTITY_PREAMBLE references directory::skills::get with the id field.
- Failed-skill recovery stub teaches `directory::skills::get { id: "..." }`.
- fetch_uri calls directory::skills::get with { id }; the existing
  response_to_string already handles the new full envelope because the
  body field is at the same JSON path.
- Adds 4 required test updates and 4 hygiene assertions per the plan.

* docs: align spec + plan with directory::skills::get API rename

Reflects PR #131 (merged to main):
- Function rename: directory::skills::fetch-skill -> directory::skills::get
- Field rename: { uri } -> { id }
- DefaultSkillBody gains id field + from_config_uri constructor
- Chat-init pseudocode updated to show singular per-URI calls (the new
  API has no batched form)
- Failed-skill stub example text updated to teach the new call shape

* chore: auto-render skill artifacts

* refactor(harness): remove first-boot skills bootstrap

system_default_skills (turn-orchestrator config) now drives the agent's
starting context and any other skill is fetched on demand via
directory::skills::get. The harness no longer needs to pre-populate
skills_folder from the registry at boot.

Removes:
- harness/src/skills.rs (bootstrap_run, BOOTSTRAP_NAMES, helpers)
- harness/tests/bootstrap_e2e.rs (the matching integration test)
- the tokio::spawn bootstrap call in register_with_iii_with_engine_url
- the bootstrap doc lines in harness/build.rs and harness/skill.md

directory::skills::download stays registered for the
'iii worker add <third-party>' workflow.

* chore: sync iii-directory Cargo.lock to v0.5.1

Lockfile was stale at 0.4.1 (pre-rebase); Cargo.toml is at 0.5.1 after
cee4aef. Local cargo build regenerated the correct entry; committing
so CI doesn't see the drift.

* docs: remove iii base prompt default skills plan and spec

* revert(iii-directory): drop sandbox orientation skill

Partially reverts d4208a2 (kept iii.md, removed sandbox.md).

* chore: update iii-directory dependency and improve worker bootstrapping

- Bump iii-directory dependency version from ^0.4.1 to ^0.5.1 in iii.worker.yaml.
- Modify Makefile and demo.sh to ensure iii-directory is included in worker spawning.
- Enhance engine startup checks by replacing list_topics with health check.
- Introduce a bootstrap process to pre-populate iii-directory's skills folder based on system_default_skills.
- Update turn-orchestrator configuration to load the iii-directory root skill at startup.

* chore: auto-render skill artifacts

* chore: update BUNDLE_EXCLUDE in release-harness-bundle.yml to include iii-directory

- Added iii-directory to the BUNDLE_EXCLUDE list to ensure it is excluded from the bundle during the release process.

* chore: bump bundled iii.worker.yaml deps to ^0.2.0

The release-harness-bundle workflow rewrites each dep's
[package].version to the harness tag but does not touch
iii.worker.yaml. Without these bumps, [email protected] ships with
constraints like `turn-orchestrator: ^0.1.0` and won't resolve
against the @0.2.0 artifacts it publishes alongside itself.

shell stays ^0.3.0 (separate release), iii-directory stays ^0.5.1
(bundle-excluded), engine workers (iii-state/queue/stream/bridge/http,
iii-sandbox) stay ^0.11.0.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants