feat: migrate harness + bundled workers from skills to iii-directory#127
Merged
Conversation
Contributor
skill-check — worker6 verified, 19 skipped (no docs/).
Three for three. Nicely done. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes boot-time ChangesMigration from boot-time skill registration to file-based iii-directory
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent/Client
participant Harness as Harness
participant Directory as iii-directory
participant Engine as iii Engine
participant TurnOrch as turn-orchestrator
participant UI as UI Browser
Agent->>Harness: Chat request
Harness->>Engine: Trigger turn::step
TurnOrch->>TurnOrch: Provisioning state
TurnOrch->>Engine: Poll engine::functions::list
Engine-->>TurnOrch: directory::skills::download ready
TurnOrch->>Directory: Trigger directory::skills::download<br/>(system_default_skills URIs)
Directory-->>TurnOrch: Skill bundles written to skills_folder
TurnOrch->>TurnOrch: Fetch via directory::skills::get<br/>(per-URI with timeout)
TurnOrch->>TurnOrch: Build DefaultSkillBody[] with bodies
TurnOrch->>TurnOrch: Assemble system prompt<br/>(identity preamble + skills)
TurnOrch->>Engine: Transition to AwaitingAssistant
Directory->>Engine: Trigger directory::skills::on-change
Engine->>Harness: Fire on-change pump
Harness->>UI: Trigger ui::skills::changed::{browser_id}
UI-->>Agent: Skills list updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
llm-budget/src/main.rs (1)
40-46: 💤 Low valueConsider removing unused config loading or adding a comment.
The config is loaded but never used (assigned to
_cfg). If this is intentional (e.g., to validate the config file still parses), consider adding a comment explaining why. Otherwise, this code could be removed entirely.♻️ Optional: remove unused config load
- let _cfg = match llm_budget::config::load_config(&cli.config) { - Ok(c) => c, - Err(e) => { - tracing::warn!(error = %e, path = %cli.config, "failed to load config, using defaults"); - llm_budget::config::WorkerConfig::default() - } - }; - let iii = register_worker(Or add a comment if intentional:
+ // Config is still loaded to validate the file parses, even though + // the migration removed the code that consumed it. let _cfg = match llm_budget::config::load_config(&cli.config) {🤖 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 `@llm-budget/src/main.rs` around lines 40 - 46, The variable _cfg is assigned from llm_budget::config::load_config(&cli.config) but never used; either remove the whole load (call) if it’s unnecessary, or keep it and add a clear comment explaining intent (e.g., validating/parsing cli.config) so it’s not mistaken for dead code; update the block around llm_budget::config::load_config, the Err branch that returns llm_budget::config::WorkerConfig::default(), and the _cfg binding (rename or remove) accordingly to reflect the chosen approach.
🤖 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 @.github/workflows/ci.yml:
- Around line 345-348: The CI workflow contains a YAML-breaking multiline echo
of the error marker starting with ::error::"skills::register" TriggerRequest
which uses backslash-wrapped line breaks; replace that multiline wrapped echo
with a YAML-safe single-line message (or use a YAML folded scalar) so the entire
::error::... string is emitted as one contiguous string and does not break YAML
parsing—update the echo command in the workflow step that emits the
::error::"skills::register" TriggerRequest message accordingly.
---
Nitpick comments:
In `@llm-budget/src/main.rs`:
- Around line 40-46: The variable _cfg is assigned from
llm_budget::config::load_config(&cli.config) but never used; either remove the
whole load (call) if it’s unnecessary, or keep it and add a clear comment
explaining intent (e.g., validating/parsing cli.config) so it’s not mistaken for
dead code; update the block around llm_budget::config::load_config, the Err
branch that returns llm_budget::config::WorkerConfig::default(), and the _cfg
binding (rename or remove) accordingly to reflect the chosen approach.
🪄 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: 73184cf6-5cd9-4b53-b001-35a0986e3c9e
⛔ Files ignored due to path filters (1)
iii-directory/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
.github/workflows/ci.ymlAGENTS-NEW-WORKER.mdTODOS.mdapproval-gate/skill.mdapproval-gate/src/lib.rsapproval-gate/src/main.rsapproval-gate/tests/skill.rsauth-credentials/src/lib.rsauth-credentials/src/main.rsauth-credentials/tests/skill.rsharness/build.rsharness/config.yamlharness/iii.worker.yamlharness/skill.mdharness/src/fanout.rsharness/src/lib.rsharness/src/skills.rsharness/tests/bootstrap_e2e.rsharness/tests/skills_register.rsharness/web/src/menuItems.test.tsharness/web/src/menuItems.tsiii-directory/skills/iii.mdiii-directory/skills/sandbox.mdllm-budget/config.yamlllm-budget/src/main.rsmodels-catalog/config.yamlmodels-catalog/src/main.rssession-tree/src/main.rsshell/src/lib.rsshell/src/main.rsshell/tests/skill.rsturn-orchestrator/src/main.rsturn-orchestrator/src/states/provisioning.rsturn-orchestrator/src/system_prompt.rs
💤 Files with no reviewable changes (8)
- harness/tests/skills_register.rs
- shell/tests/skill.rs
- auth-credentials/tests/skill.rs
- approval-gate/tests/skill.rs
- approval-gate/src/lib.rs
- auth-credentials/src/lib.rs
- shell/src/lib.rs
- models-catalog/config.yaml
ytallo
force-pushed
the
feat/iii-directory-migration
branch
2 times, most recently
from
af1b63c to
6709bc5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
124-127:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix YAML-breaking multiline echo (already flagged).
This multiline echo with backslashes causes YAML parsing to fail, blocking CI execution.
🤖 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 @.github/workflows/ci.yml around lines 124 - 127, The YAML step contains a multiline shell echo using backslashes that breaks workflow parsing; locate the echo command containing the marker ::error::"skills::register" and replace the broken multiline form with a YAML-safe single-line or properly quoted multiline string (e.g., collapse into one echo invocation without backslashes or use a YAML block scalar) so the message is emitted as intended without introducing illegal line continuations.
🤖 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 `@harness/skill.md`:
- Around line 8-9: Add brief endpoint descriptions for the two missing
registered functions: document bridge::info as an endpoint that returns metadata
about the bridge (e.g., status, version, connected clients) and document
harness::fs::read_inline as an endpoint that reads and returns the contents of
an inline/test file from the harness filesystem; place these descriptions
alongside the other endpoints in the existing endpoints section so they appear
with the other documented functions (the section that lists bridge::trigger,
bridge::events, ui::subscribe, ui::unsubscribe).
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 124-127: The YAML step contains a multiline shell echo using
backslashes that breaks workflow parsing; locate the echo command containing the
marker ::error::"skills::register" and replace the broken multiline form with a
YAML-safe single-line or properly quoted multiline string (e.g., collapse into
one echo invocation without backslashes or use a YAML block scalar) so the
message is emitted as intended without introducing illegal line continuations.
🪄 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: e8e25a9f-6562-44d2-831a-23d7610021d7
📒 Files selected for processing (36)
.github/scripts/validate_worker.py.github/workflows/ci.ymlAGENTS-NEW-WORKER.mdTODOS.mdapproval-gate/skill.mdapproval-gate/src/lib.rsapproval-gate/src/main.rsapproval-gate/tests/skill.rsauth-credentials/src/lib.rsauth-credentials/src/main.rsauth-credentials/tests/skill.rsharness/build.rsharness/config.yamlharness/iii.worker.yamlharness/skill.mdharness/src/fanout.rsharness/src/lib.rsharness/src/skills.rsharness/tests/bootstrap_e2e.rsharness/tests/skills_register.rsharness/web/src/menuItems.test.tsiii-directory/skills/iii.mdiii-directory/skills/sandbox.mdllm-budget/README.mdllm-budget/config.yamlllm-budget/src/main.rsmodels-catalog/README.mdmodels-catalog/config.yamlmodels-catalog/src/main.rssession/src/main.rsshell/src/lib.rsshell/src/main.rsshell/tests/skill.rsturn-orchestrator/src/main.rsturn-orchestrator/src/states/provisioning.rsturn-orchestrator/src/system_prompt.rs
💤 Files with no reviewable changes (9)
- models-catalog/config.yaml
- harness/tests/skills_register.rs
- auth-credentials/tests/skill.rs
- approval-gate/tests/skill.rs
- approval-gate/src/lib.rs
- auth-credentials/src/lib.rs
- models-catalog/README.md
- shell/tests/skill.rs
- shell/src/lib.rs
✅ Files skipped from review due to trivial changes (5)
- approval-gate/skill.md
- harness/build.rs
- TODOS.md
- harness/config.yaml
- harness/iii.worker.yaml
🚧 Files skipped from review as they are similar to previous changes (13)
- llm-budget/config.yaml
- approval-gate/src/main.rs
- harness/tests/bootstrap_e2e.rs
- turn-orchestrator/src/system_prompt.rs
- turn-orchestrator/src/states/provisioning.rs
- llm-budget/src/main.rs
- turn-orchestrator/src/main.rs
- harness/web/src/menuItems.test.ts
- harness/src/lib.rs
- harness/src/skills.rs
- auth-credentials/src/main.rs
- models-catalog/src/main.rs
- harness/src/fanout.rs
Comment on lines
+8
to
+9
| `bridge::trigger`, `bridge::events`, `bridge::info`, `ui::subscribe`, | ||
| `ui::unsubscribe`, and `harness::fs::read_inline`; wires the upstream |
There was a problem hiding this comment.
Document the missing registered functions.
Lines 8-9 mention that the harness registers bridge::info and harness::fs::read_inline, but these functions are not documented in the endpoints section (lines 15-24). For completeness, add brief descriptions of these two functions alongside the other documented endpoints.
Also applies to: 15-24
🤖 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 `@harness/skill.md` around lines 8 - 9, Add brief endpoint descriptions for the
two missing registered functions: document bridge::info as an endpoint that
returns metadata about the bridge (e.g., status, version, connected clients) and
document harness::fs::read_inline as an endpoint that reads and returns the
contents of an inline/test file from the harness filesystem; place these
descriptions alongside the other endpoints in the existing endpoints section so
they appear with the other documented functions (the section that lists
bridge::trigger, bridge::events, ui::subscribe, ui::unsubscribe).
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.
…rkers 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.
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.
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.
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.
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.
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.
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.
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.
Subscriber, transitions, and provisioning state now receive TurnOrchestratorConfig. Plumbing only — provisioning ignores the new parameter pending the prompt-assembly rewrite in the next commit.
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.
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.
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.
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.
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.
- 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.
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.
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.
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
There was a problem hiding this comment.
🧹 Nitpick comments (2)
turn-orchestrator/src/states/provisioning.rs (1)
19-21: ⚡ Quick winConsider documenting cumulative timeout impact for operators.
The 10-second per-URI timeout is reasonable for the default single-entry config, but if operators add multiple
system_default_skillsURIs and several timeout, chat-init could take N×10 seconds. For example, 5 URIs timing out = 50 seconds before the chat starts.This is acceptable given the comment at line 62 ("N sequential calls is acceptable"), but it may surprise operators who don't realize the cumulative effect.
Consider adding a comment in
turn-orchestrator/config.yamlnearsystem_default_skillsto warn operators about cumulative timeout, or adding a harness-level max total timeout for the entire fetch phase.🤖 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 `@turn-orchestrator/src/states/provisioning.rs` around lines 19 - 21, Add a note warning about cumulative per-URI timeouts: document in turn-orchestrator/config.yaml next to system_default_skills that each URI uses FETCH_TIMEOUT_MS (10_000 ms) and that N failing URIs can delay startup by N×FETCH_TIMEOUT_MS; alternatively (or additionally) implement a harness-level cap by introducing a total timeout/threshold and enforcing it when the provisioning code iterates the system_default_skills URIs (referencing FETCH_TIMEOUT_MS in provisioning.rs) so the fetch phase aborts after the max total timeout rather than allowing unbounded N×FETCH_TIMEOUT_MS delays.turn-orchestrator/src/config.rs (1)
41-45: ⚡ Quick winConsider adding URI format validation at config load.
The design doc (line 63) states that the harness validates URI format at config load, but the current
load_configimplementation only deserializes YAML without checking URI syntax. Invalid URIs will only fail later during chat-init fetch, which may surprise operators.🛡️ Example validation to add after deserialization
pub fn load_config(path: &str) -> Result<TurnOrchestratorConfig> { let contents = std::fs::read_to_string(path)?; let cfg: TurnOrchestratorConfig = serde_yaml::from_str(&contents)?; + // Validate system_default_skills URIs + for uri in &cfg.system_default_skills { + if uri.is_empty() { + anyhow::bail!("system_default_skills contains an empty URI"); + } + // Basic format check; iii-directory resolves the URI + } Ok(cfg) }🤖 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 `@turn-orchestrator/src/config.rs` around lines 41 - 45, Update load_config to validate URI syntax immediately after deserializing into TurnOrchestratorConfig: for each configuration field that is expected to be a URI (fields on TurnOrchestratorConfig used later by chat-init/fetch), attempt to parse them with a URI parser (e.g., url::Url::parse) and return an Err with a clear message if any parse fails; ensure the function still returns Result<TurnOrchestratorConfig> and include context about which field failed so operators get immediate, actionable feedback at config load time.
🤖 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.
Nitpick comments:
In `@turn-orchestrator/src/config.rs`:
- Around line 41-45: Update load_config to validate URI syntax immediately after
deserializing into TurnOrchestratorConfig: for each configuration field that is
expected to be a URI (fields on TurnOrchestratorConfig used later by
chat-init/fetch), attempt to parse them with a URI parser (e.g.,
url::Url::parse) and return an Err with a clear message if any parse fails;
ensure the function still returns Result<TurnOrchestratorConfig> and include
context about which field failed so operators get immediate, actionable feedback
at config load time.
In `@turn-orchestrator/src/states/provisioning.rs`:
- Around line 19-21: Add a note warning about cumulative per-URI timeouts:
document in turn-orchestrator/config.yaml next to system_default_skills that
each URI uses FETCH_TIMEOUT_MS (10_000 ms) and that N failing URIs can delay
startup by N×FETCH_TIMEOUT_MS; alternatively (or additionally) implement a
harness-level cap by introducing a total timeout/threshold and enforcing it when
the provisioning code iterates the system_default_skills URIs (referencing
FETCH_TIMEOUT_MS in provisioning.rs) so the fetch phase aborts after the max
total timeout rather than allowing unbounded N×FETCH_TIMEOUT_MS delays.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45cf344c-10c8-45f8-b89f-fc82f99a7522
📒 Files selected for processing (12)
docs/superpowers/plans/2026-05-13-iii-base-prompt-default-skills.mddocs/superpowers/specs/2026-05-13-iii-base-prompt-default-skills-design.mdiii-directory/skills/iii.mdiii-directory/tests/iii_skill_content.rsturn-orchestrator/config.yamlturn-orchestrator/src/agent_call.rsturn-orchestrator/src/config.rsturn-orchestrator/src/register.rsturn-orchestrator/src/states/provisioning.rsturn-orchestrator/src/subscriber.rsturn-orchestrator/src/system_prompt.rsturn-orchestrator/src/transitions.rs
✅ Files skipped from review due to trivial changes (1)
- docs/superpowers/plans/2026-05-13-iii-base-prompt-default-skills.md
ytallo
force-pushed
the
feat/iii-directory-migration
branch
from
f03891a to
b8c7cad
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
harness/skill.md (1)
8-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocument the missing registered functions.
Lines 8-9 mention
bridge::infoandharness::fs::read_inlinebut these functions are not documented in the endpoints section (lines 15-24). For completeness, add brief descriptions of these two functions.📝 Suggested addition
Add these entries to the endpoints list:
- [`bridge::info`](iii://harness/index) — returns metadata about the bridge (status, version, connected clients). - [`harness::fs::read_inline`](iii://harness/index) — reads and returns the contents of an inline/test file from the harness filesystem.🤖 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 `@harness/skill.md` around lines 8 - 24, Add documentation entries for the two missing registered functions referenced earlier: insert a bullet for `bridge::info` describing it "returns metadata about the bridge (status, version, connected clients)" and a bullet for `harness::fs::read_inline` describing it "reads and returns the contents of an inline/test file from the harness filesystem", placing both alongside the other endpoints (`harness::status`, `bridge::trigger`, `bridge::events`, `ui::subscribe`/`ui::unsubscribe`) so the endpoints list includes all referenced functions..github/workflows/ci.yml (1)
124-127:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix YAML syntax error in multiline echo command.
The backslash-wrapped line continuation inside the echo command is causing YAML parsing to fail. This breaks the CI workflow.
🐛 Proposed fix
if [ -n "$matches" ]; then - echo "::error::\"skills::register\" TriggerRequest found in Rust sources — \ -the iii-directory migration replaced it with on-disk skill.md (see \ -docs/superpowers/specs/2026-05-12-iii-directory-migration-design.md). \ -Matches:" + echo '::error::"skills::register" TriggerRequest found in Rust sources — the iii-directory migration replaced it with on-disk skill.md (see docs/superpowers/specs/2026-05-12-iii-directory-migration-design.md). Matches:' echo "$matches" exit 1 fi🤖 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 @.github/workflows/ci.yml around lines 124 - 127, The CI YAML has a malformed multiline shell echo using backslash line-continuation inside the YAML scalar (the echo starting with "::error::\"skills::register\" TriggerRequest..."), which breaks parsing; fix by replacing the backslash-wrapped continuation with a properly quoted single-line string or a YAML multiline scalar (using |) so the entire echo argument is one valid YAML string and ensure internal quotes are escaped correctly (reference the echo command text "::error::\"skills::register\" TriggerRequest..." in the ci.yml step).
🧹 Nitpick comments (2)
docs/superpowers/specs/2026-05-13-iii-base-prompt-default-skills-design.md (2)
88-88: ⚡ Quick winPreamble wording assumes iii skill presence.
Line 88 instructs agents to "discover via the iii skill below," but Scenario A (lines 187-196) documents that all skills may be unavailable at chat start, leaving only stubs. When
iii://iiifetch fails, there is no iii skill below to discover from.The preamble does provide fallback instructions (
engine::functions::liston line 92-93), but the primary instruction assumes the happy path. Consider revising to:
- "discover via the skills below, or via
engine::functions::listif unavailable"- Or explicitly acknowledge that skills may be stubs
This reduces confusion when agents boot in degraded mode and immediately need the fallback path.
🤖 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 `@docs/superpowers/specs/2026-05-13-iii-base-prompt-default-skills-design.md` at line 88, The preamble currently assumes the iii skill is present ("discover via the iii skill below") which conflicts with Scenario A where skills may be unavailable; update the wording to account for degraded mode by changing that sentence to indicate discovery via the skills below or via engine::functions::list (or by explicitly noting skills may be stubs) so agents know to fall back when iii://iii fetch fails; ensure references to "iii://iii" and "engine::functions::list" are mentioned so the intended fallback path is clear.
63-63: ⚡ Quick winClarify URI validation scope for security.
The spec states the harness "validates URI format at config load" but doesn't specify what format validation entails. Consider adding a brief note about:
- Whether only
iii://scheme URIs are accepted- Whether there are any path traversal or injection concerns with URI values
- Whether malformed or overly long URIs are rejected
This helps implementers understand security boundaries and prevents operators from accidentally introducing malicious or problematic URIs.
🤖 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 `@docs/superpowers/specs/2026-05-13-iii-base-prompt-default-skills-design.md` at line 63, The spec line "harness validates URI format at config load" is underspecified for security; update the document to explicitly state the allowed URI schemes (e.g., only accept iii://), that URI resolution remains the responsibility of iii-directory (no resolution in the harness), and list concrete validation rules the harness enforces: reject malformed URIs, enforce a maximum length, normalize and validate percent‑encoding, disallow path traversal and suspicious characters (e.g., ../ sequences or control chars), and return a clear validation error on rejection so operators know to correct the config; reference this change alongside the original phrase "harness validates URI format at config load" so implementers know where the constraints apply.
🤖 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 `@harness/iii.worker.yaml`:
- Line 33: Update the iii-directory dependency version constraint to match the
local package version: change the caret range for iii-directory from ^0.4.1 to
^0.5.1 so the constraint allows >=0.5.1 and <0.6.0; locate the dependency entry
for iii-directory in the harness/iii.worker.yaml dependency list and replace the
version string accordingly (look for the key "iii-directory").
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 124-127: The CI YAML has a malformed multiline shell echo using
backslash line-continuation inside the YAML scalar (the echo starting with
"::error::\"skills::register\" TriggerRequest..."), which breaks parsing; fix by
replacing the backslash-wrapped continuation with a properly quoted single-line
string or a YAML multiline scalar (using |) so the entire echo argument is one
valid YAML string and ensure internal quotes are escaped correctly (reference
the echo command text "::error::\"skills::register\" TriggerRequest..." in the
ci.yml step).
In `@harness/skill.md`:
- Around line 8-24: Add documentation entries for the two missing registered
functions referenced earlier: insert a bullet for `bridge::info` describing it
"returns metadata about the bridge (status, version, connected clients)" and a
bullet for `harness::fs::read_inline` describing it "reads and returns the
contents of an inline/test file from the harness filesystem", placing both
alongside the other endpoints (`harness::status`, `bridge::trigger`,
`bridge::events`, `ui::subscribe`/`ui::unsubscribe`) so the endpoints list
includes all referenced functions.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-05-13-iii-base-prompt-default-skills-design.md`:
- Line 88: The preamble currently assumes the iii skill is present ("discover
via the iii skill below") which conflicts with Scenario A where skills may be
unavailable; update the wording to account for degraded mode by changing that
sentence to indicate discovery via the skills below or via
engine::functions::list (or by explicitly noting skills may be stubs) so agents
know to fall back when iii://iii fetch fails; ensure references to "iii://iii"
and "engine::functions::list" are mentioned so the intended fallback path is
clear.
- Line 63: The spec line "harness validates URI format at config load" is
underspecified for security; update the document to explicitly state the allowed
URI schemes (e.g., only accept iii://), that URI resolution remains the
responsibility of iii-directory (no resolution in the harness), and list
concrete validation rules the harness enforces: reject malformed URIs, enforce a
maximum length, normalize and validate percent‑encoding, disallow path traversal
and suspicious characters (e.g., ../ sequences or control chars), and return a
clear validation error on rejection so operators know to correct the config;
reference this change alongside the original phrase "harness validates URI
format at config load" so implementers know where the constraints apply.
🪄 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: 47242cbe-009b-4b62-9e74-df16e26c16f5
📒 Files selected for processing (44)
.github/scripts/validate_worker.py.github/workflows/ci.ymlAGENTS-NEW-WORKER.mdTODOS.mdapproval-gate/skill.mdapproval-gate/src/lib.rsapproval-gate/src/main.rsapproval-gate/tests/skill.rsauth-credentials/src/lib.rsauth-credentials/src/main.rsauth-credentials/tests/skill.rsdocs/superpowers/plans/2026-05-13-iii-base-prompt-default-skills.mddocs/superpowers/specs/2026-05-13-iii-base-prompt-default-skills-design.mdharness/build.rsharness/config.yamlharness/iii.worker.yamlharness/skill.mdharness/src/fanout.rsharness/src/lib.rsharness/src/skills.rsharness/tests/bootstrap_e2e.rsharness/tests/skills_register.rsiii-directory/skills/iii.mdiii-directory/skills/sandbox.mdiii-directory/tests/iii_skill_content.rsllm-budget/README.mdllm-budget/config.yamlllm-budget/src/main.rsmodels-catalog/README.mdmodels-catalog/config.yamlmodels-catalog/src/main.rssession/src/main.rsshell/src/lib.rsshell/src/main.rsshell/tests/skill.rsturn-orchestrator/README.mdturn-orchestrator/config.yamlturn-orchestrator/src/config.rsturn-orchestrator/src/main.rsturn-orchestrator/src/register.rsturn-orchestrator/src/states/provisioning.rsturn-orchestrator/src/subscriber.rsturn-orchestrator/src/system_prompt.rsturn-orchestrator/src/transitions.rs
💤 Files with no reviewable changes (9)
- models-catalog/README.md
- models-catalog/config.yaml
- auth-credentials/tests/skill.rs
- approval-gate/tests/skill.rs
- approval-gate/src/lib.rs
- shell/src/lib.rs
- auth-credentials/src/lib.rs
- shell/tests/skill.rs
- harness/tests/skills_register.rs
✅ Files skipped from review due to trivial changes (7)
- approval-gate/skill.md
- TODOS.md
- turn-orchestrator/README.md
- llm-budget/README.md
- harness/config.yaml
- harness/build.rs
- docs/superpowers/plans/2026-05-13-iii-base-prompt-default-skills.md
🚧 Files skipped from review as they are similar to previous changes (19)
- llm-budget/config.yaml
- turn-orchestrator/config.yaml
- harness/tests/bootstrap_e2e.rs
- harness/src/lib.rs
- turn-orchestrator/src/register.rs
- iii-directory/tests/iii_skill_content.rs
- turn-orchestrator/src/transitions.rs
- turn-orchestrator/src/subscriber.rs
- iii-directory/skills/iii.md
- models-catalog/src/main.rs
- turn-orchestrator/src/config.rs
- llm-budget/src/main.rs
- .github/scripts/validate_worker.py
- approval-gate/src/main.rs
- harness/src/skills.rs
- turn-orchestrator/src/main.rs
- harness/src/fanout.rs
- turn-orchestrator/src/states/provisioning.rs
- turn-orchestrator/src/system_prompt.rs
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.
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.
Partially reverts d4208a2 (kept iii.md, removed sandbox.md).
- 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.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
harness/skill.md (1)
8-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocument the missing registered functions.
Lines 8-9 mention that the harness registers
bridge::infoandharness::fs::read_inline, but these functions are not documented in the endpoints section (lines 15-24). For completeness, add brief descriptions of these two functions alongside the other documented endpoints.Also applies to: 15-24
🤖 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 `@harness/skill.md` around lines 8 - 9, Add brief documentation entries for the two missing registered functions mentioned earlier: add a one-line description for bridge::info (what data it returns or its purpose) and for harness::fs::read_inline (what it reads and return format) into the endpoints list/section where the other endpoints (e.g., bridge::trigger, bridge::events, ui::subscribe) are documented; ensure the descriptions are concise and consistent in style with the existing entries so readers can find usage and purpose alongside the other endpoints.
🤖 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 `@turn-orchestrator/src/bootstrap.rs`:
- Around line 90-113: The tracing::info format strings are missing arguments: in
the tracing::info call that logs "bootstrap: downloaded ({} of {total})" include
both i + 1 and total as positional args (e.g., use "{} of {}" then pass i + 1,
total) and in the final tracing::info that logs "bootstrap: {ok}/{total}
namespaces..." replace the placeholders with positional formatting "{} / {}" and
pass ok, total as arguments; update the two tracing::info macro invocations so
their format strings and argument lists match (keep existing fields like target,
namespace, skills, prompts unchanged).
---
Duplicate comments:
In `@harness/skill.md`:
- Around line 8-9: Add brief documentation entries for the two missing
registered functions mentioned earlier: add a one-line description for
bridge::info (what data it returns or its purpose) and for
harness::fs::read_inline (what it reads and return format) into the endpoints
list/section where the other endpoints (e.g., bridge::trigger, bridge::events,
ui::subscribe) are documented; ensure the descriptions are concise and
consistent in style with the existing entries so readers can find usage and
purpose alongside the other endpoints.
🪄 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: 557c3dfe-03cd-4e9a-bada-aa69a05637c5
⛔ Files ignored due to path filters (1)
iii-directory/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
harness/Makefileharness/build.rsharness/iii.worker.yamlharness/scripts/demo.shharness/skill.mdharness/skills/iii/iii.mdharness/src/lib.rsturn-orchestrator/README.mdturn-orchestrator/config.yamlturn-orchestrator/src/bootstrap.rsturn-orchestrator/src/config.rsturn-orchestrator/src/lib.rsturn-orchestrator/src/main.rsturn-orchestrator/src/manifest.rs
✅ Files skipped from review due to trivial changes (3)
- harness/build.rs
- turn-orchestrator/src/lib.rs
- turn-orchestrator/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- turn-orchestrator/config.yaml
- harness/iii.worker.yaml
Comment on lines
+90
to
+113
| tracing::info!( | ||
| target: "turn_orchestrator::bootstrap", | ||
| namespace = %ns, | ||
| skills, | ||
| prompts, | ||
| "bootstrap: downloaded ({} of {total})", | ||
| i + 1, | ||
| ); | ||
| } | ||
| Err(err) => { | ||
| tracing::warn!( | ||
| target: "turn_orchestrator::bootstrap", | ||
| namespace = %ns, | ||
| %err, | ||
| "bootstrap: download failed; chats will see recovery stubs \ | ||
| for this namespace until manually re-downloaded", | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| tracing::info!( | ||
| target: "turn_orchestrator::bootstrap", | ||
| "bootstrap: {ok}/{total} namespaces populated from system_default_skills", | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if turn-orchestrator builds successfully
cd turn-orchestrator && cargo build --lib 2>&1 | rg -i "error|warning.*format"Repository: iii-hq/workers
Length of output: 187
🏁 Script executed:
cat -n turn-orchestrator/src/bootstrap.rs | sed -n '90,113p'Repository: iii-hq/workers
Length of output: 1073
🏁 Script executed:
# Search for other tracing macro patterns in the codebase to understand the conventions
rg "tracing::(info|warn|debug|error)!" --context 2 turn-orchestrator/src/ | head -100Repository: iii-hq/workers
Length of output: 6334
🌐 Web query:
tracing crate Rust format string variable capture syntax
💡 Result:
In the Rust tracing crate, there isn’t a special “format-string variable capture syntax” like {var}. Instead, the formatting syntax is provided by normal Rust formatting arguments, and the tracing macro captures whatever you pass as arguments/fields into structured fields. 1) Capturing variables as structured fields Use key-value syntax inside the macro: key = value (or shorthand key to use the local variable value). For example, fields on an event/span are written as field_name = field_value and values can use local-variable shorthand (i.e., span!(..., user) is shorthand for user = user). [1] Example: let user = "alice"; tracing::info!("login", user = user); // explicit tracing::info!("login", user); // shorthand [1] 2) “Format string” message + captured variables After the key-value fields (if any), you may add a human-readable format string and its formatting arguments. tracing then creates an implicit field named message whose value is the provided set of format arguments. [1] Example: let question = "the ultimate question..."; let answer = 42; tracing::event!( Level::DEBUG, question.answer = answer, "the answer to {} is {}.", question, answer); In this call, the format string/arguments become the implicit message field. [1] 3) What does “%{} / { }” capture?” The {} / {var}-style you’re thinking of is not tracing-specific capture syntax; it’s just the normal Rust formatting placeholders used in the “format string and arguments” position described above. The variables you pass (e.g., question, answer) are captured as part of the message field construction, while any structured fields you specify earlier are captured separately as key-value fields. [1]
Citations:
🏁 Script executed:
cat -n turn-orchestrator/src/bootstrap.rs | sed -n '60,120p'Repository: iii-hq/workers
Length of output: 2665
🏁 Script executed:
# Also search for where ok and total are assigned/declared in bootstrap.rs
rg "let.*\b(ok|total)\b" turn-orchestrator/src/bootstrap.rs -B 2 -A 2Repository: iii-hq/workers
Length of output: 193
Fix format string arguments in tracing macros.
The format strings at lines 95 and 112 have placeholders without corresponding arguments. Rust format strings require arguments for each placeholder:
- Line 95:
"bootstrap: downloaded ({} of {total})"with only one arg (i + 1) —{total}will print literally - Line 112:
"bootstrap: {ok}/{total} namespaces populated from system_default_skills"with no args — will print as literal{ok}/{total}
These should be:
- Line 95:
"bootstrap: downloaded ({} of {})", i + 1, total, - Line 112:
"bootstrap: {}/{} namespaces populated from system_default_skills", ok, total,
🤖 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 `@turn-orchestrator/src/bootstrap.rs` around lines 90 - 113, The tracing::info
format strings are missing arguments: in the tracing::info call that logs
"bootstrap: downloaded ({} of {total})" include both i + 1 and total as
positional args (e.g., use "{} of {}" then pass i + 1, total) and in the final
tracing::info that logs "bootstrap: {ok}/{total} namespaces..." replace the
placeholders with positional formatting "{} / {}" and pass ok, total as
arguments; update the two tracing::info macro invocations so their format
strings and argument lists match (keep existing fields like target, namespace,
skills, prompts unchanged).
… iii-directory - Added iii-directory to the BUNDLE_EXCLUDE list to ensure it is excluded from the bundle during the release process.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skillsworker dep; the harness now talks toiii-directory(v0.4.1) and bootstrapsiii://<worker>/indexfor each bundled worker on first boot, with versions pinned fromiii.lock.skills::register/skills::unregisterretry loops from all seven bundled workers (auth-credentials, approval-gate, shell, turn-orchestrator, models-catalog, llm-budget, session-tree); each worker's existingskill.mdis now served from disk via iii-directory.turn-orchestrator/src/states/provisioning.rsis_root_skill_idaccepts<seg>/index;harness/web/src/menuItems.tsparsesiii://{id}directly) and refresh hardcoded single-segment URI references.harness::skills::bootstrap_run(bounded JoinSet + Semaphore(4), 8 unit tests) and two fanout pumps forskills::on-change/prompts::on-change.harness/skill.md(a lib test asserts everyEXPECTED_WORKERSentry is referenced).<worker>/skill.mdpresence + size cap, plus a regression grep that forbids thefunction_id: "skills::register"literal.AGENTS-NEW-WORKER.md§10 to the file-based publish model and close the two related entries inTODOS.md.Net: 35 files changed, +834 / −1197.
Test plan
cargo buildandcargo test --libgreen in each ofharness,iii-directory,auth-credentials,turn-orchestrator,approval-gate,shell,models-catalog,llm-budget,session-tree(541 lib tests locally).cargo testinharness(58 passed, 1#[ignore]'d as the engine-requiredbootstrap_e2escenario).harness/webmenuItems unit tests pass (10/10).grep -rn 'function_id: "skills::register"' --include='*.rs'returns zero matches; eachBOOTSTRAP_NAMESentry has a non-emptyskill.md≤256 KiB../data/skills, boot the harness, confirmskill::fetch iii://shell/index(and every otherBOOTSTRAP_NAMESentry) returns content.skills::downloadfor one worker, observeui::skills::changed::<browser_id>.iii://iii-directory/iiiandiii://iii-directory/sandboxresolve after bootstrap (the orientation bodies that previously lived inharness/docs/).Summary by CodeRabbit
New Features
Infrastructure