Repository navigation
feat: iii directory - #124
Merged
Merged
Conversation
Contributor
skill-check — worker6 verified, 20 skipped (no docs/). 82 errors across the verified workers.
|
📝 WalkthroughWalkthroughReplaces the state-backed Changesiii-directory worker (core features & wiring)
Test harness, BDD, and test helpers
Removals — legacy state-backed skills crate & tests
CI, release, scripts, and ignore updates
Sequence Diagram(s)sequenceDiagram
actor Client
Client->>DownloadFn: skills::download(repo|worker)
DownloadFn->>Classifier: classify_input
Classifier-->>DownloadFn: Repo or Registry
alt Repo path
DownloadFn->>GitSrc: git::download(repo, skill)
GitSrc->>Git: git clone --depth 1
Git-->>GitSrc: repo tree
GitSrc->>Disk: write files (skills/, prompts/)
GitSrc-->>DownloadFn: DownloadResult
else Registry path
DownloadFn->>RegSrc: registry::download(worker, spec)
RegSrc->>Registry: HTTP GET /w/{worker}?{spec}
Registry-->>RegSrc: JSON payload
RegSrc->>Disk: write skills and prompts
RegSrc-->>DownloadFn: DownloadResult
end
DownloadFn->>TriggerTypes: dispatch skills::on-change / prompts::on-change (conditionally)
DownloadFn-->>Client: DownloadOutput (namespace, skills_written, prompts_written, source)
sequenceDiagram
actor Client
Client->>RegistryProxy: registry::worker-info(name, tag|version)
RegistryProxy->>Cache: lookup(key)
alt cache hit
Cache-->>RegistryProxy: cached value
else cache miss
RegistryProxy->>Registry: HTTP GET /w/{name}?{spec}
Registry-->>RegistryProxy: JSON
RegistryProxy->>Cache: store(key, value, TTL)
end
RegistryProxy-->>Client: WorkerInfoOutput
sequenceDiagram
actor Client
Client->>DirectoryFn: directory::function-info(function_id)
DirectoryFn->>III_SDK: list functions/triggers/workers
III_SDK-->>DirectoryFn: SdkFunctionInfo / TriggerInfo / WorkerInfo
DirectoryFn->>HowTo: how_to::find_for_function(function_id)
HowTo->>Disk: scan_how_tos(skills_folder)
Disk-->>HowTo: FsHowTo list
HowTo-->>DirectoryFn: Option<HowTo>
DirectoryFn-->>Client: FunctionInfoOutput (with optional how_guide)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
iii-directory/src/trigger_types.rs (2)
121-125:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrigger description is inconsistent with module documentation.
The description "Fires after any mutation of the prompts registry (register / unregister)" doesn't match the updated module-level docs (lines 7-8), which state the trigger fires "after every successful
skills::downloadthat wrote at least one prompt markdown file."📝 Proposed fix to align with module docs
let _ = iii.register_trigger_type(RegisterTriggerType::new( PROMPTS_ON_CHANGE.to_string(), - "Fires after any mutation of the prompts registry (register / unregister).".to_string(), + "Fires after successful skills::download that wrote at least one prompt markdown file.".to_string(), SkillsTriggerHandler::new(PROMPTS_ON_CHANGE, prompts.clone()), ));🤖 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/trigger_types.rs` around lines 121 - 125, Update the trigger description passed to RegisterTriggerType::new for PROMPTS_ON_CHANGE to match the module docs: replace the current "Fires after any mutation of the prompts registry (register / unregister)." text with a description that states it fires "after every successful skills::download that wrote at least one prompt markdown file" (or equivalent wording). Modify the string in the register_trigger_type call (where register_trigger_type is invoked with RegisterTriggerType::new, PROMPTS_ON_CHANGE, and SkillsTriggerHandler::new) so the description accurately reflects the trigger semantics.
114-118:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrigger description is inconsistent with module documentation.
The description "Fires after any mutation of the skills registry (register / unregister)" doesn't match the updated module-level docs (lines 5-8), which state the trigger fires "after every successful
skills::downloadthat wrote at least one skill markdown file." The old description references a registry mutation pattern that no longer applies.📝 Proposed fix to align with module docs
let _ = iii.register_trigger_type(RegisterTriggerType::new( SKILLS_ON_CHANGE.to_string(), - "Fires after any mutation of the skills registry (register / unregister).".to_string(), + "Fires after successful skills::download that wrote at least one skill markdown file.".to_string(), SkillsTriggerHandler::new(SKILLS_ON_CHANGE, skills.clone()), ));🤖 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/trigger_types.rs` around lines 114 - 118, Update the trigger description passed to RegisterTriggerType::new for SKILLS_ON_CHANGE so it matches the module docs: replace the current string "Fires after any mutation of the skills registry (register / unregister)." with "Fires after every successful `skills::download` that wrote at least one skill markdown file."; keep the same use of SKILLS_ON_CHANGE and SkillsTriggerHandler::new(SKILLS_ON_CHANGE, skills.clone()) so only the human-readable description changes.
🧹 Nitpick comments (4)
iii-directory/src/how_to.rs (1)
135-145: ⚖️ Poor tradeoffConsider caching scan_how_tos results if find_for_function is called frequently.
find_for_functioncallsscan_how_toson every invocation, which walks and parses all**/*.mdfiles inskills_folder. If this function is called for multiple function IDs (e.g., during bulk directory introspection), the same filesystem scan repeats unnecessarily. For large skills folders, this could become a performance bottleneck.If
find_for_functionis called rarely or performance testing shows acceptable latency, the current approach is fine. Otherwise, consider memoizingscan_how_tosresults with cache invalidation on filesystem changes or passing a pre-scanned list to the caller.🤖 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/how_to.rs` around lines 135 - 145, find_for_function currently calls scan_how_tos every time which re-scans and reparses the skills_folder; to fix, avoid repeated scanning by either (A) adding an optional parameter to find_for_function like pre_scanned: Option<&[FsHowTo]> and use that when provided, or (B) implement a simple memo cache keyed by the skills_folder Path (e.g., static Mutex<HashMap<PathBuf, Vec<FsHowTo>>>) so scan_how_tos is only executed once and subsequent calls reuse cached Vec<FsHowTo>; ensure cache invalidation or a clear API if the filesystem may change, and keep references to function_id_to_uri and the existing find logic unchanged so callers can opt-in to pass pre-scanned data or rely on the cached results.iii-directory/src/sources/git.rs (1)
158-162: 💤 Low valueRemove unnecessary validation or handle its result.
Line 162 validates
rel_strbut explicitly ignores failures withlet _ = validate_relative_path(&rel_str);. Since the path was constructed from sanitized components in this function (with separators and leading dots already filtered at line 143), the validation is redundant. Either remove the validation call entirely or handle validation failures by skipping the file write.♻️ Proposed fix to remove unnecessary validation
write_file_atomic(&dest, &bytes)?; - // Re-validate the relative path for sanity, even though - // we built it ourselves. let rel_str = next_rel.to_string_lossy().replace('\\', "/"); - // ignore validation errors (path is already constructed) - let _ = validate_relative_path(&rel_str); if is_prompt_relpath(&next_rel) && next_rel.extension().is_some_and(|e| e == "md") {🤖 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/sources/git.rs` around lines 158 - 162, The call to validate_relative_path(&rel_str) is redundant because rel_str is built from sanitized components (next_rel) and its errors are ignored; remove the validation call and the ignore assignment so rel_str is used directly (delete the line containing let _ = validate_relative_path(&rel_str);), or alternatively replace it by handling the Result from validate_relative_path(&rel_str) and skipping the write when Err (e.g., if validate_relative_path(&rel_str).is_err() { continue; }); refer to rel_str, next_rel, and validate_relative_path when making the change.iii-directory/src/manifest.rs (1)
24-29: ⚡ Quick winConsider extracting timeout constants for consistency.
Lines 25-26 import
DEFAULT_SKILLS_FOLDERandDEFAULT_REGISTRY_URLfrom the config module, but lines 27-28 hardcode60_000for timeout values. For maintainability and consistency, consider also importingDEFAULT_DOWNLOAD_TIMEOUT_MSandDEFAULT_REGISTRY_CACHE_TTL_MSconstants if they exist in the config module.♻️ Suggested refactor
In
iii-directory/src/config.rs, define:pub const DEFAULT_DOWNLOAD_TIMEOUT_MS: u64 = 60_000; pub const DEFAULT_REGISTRY_CACHE_TTL_MS: u64 = 60_000;Then update this file:
-use crate::config::{DEFAULT_REGISTRY_URL, DEFAULT_SKILLS_FOLDER}; +use crate::config::{ + DEFAULT_DOWNLOAD_TIMEOUT_MS, DEFAULT_REGISTRY_CACHE_TTL_MS, + DEFAULT_REGISTRY_URL, DEFAULT_SKILLS_FOLDER, +}; pub fn build_manifest() -> ModuleManifest { ModuleManifest { // ... default_config: serde_json::json!({ "skills_folder": DEFAULT_SKILLS_FOLDER, "registry_url": DEFAULT_REGISTRY_URL, - "download_timeout_ms": 60_000, - "registry_cache_ttl_ms": 60_000, + "download_timeout_ms": DEFAULT_DOWNLOAD_TIMEOUT_MS, + "registry_cache_ttl_ms": DEFAULT_REGISTRY_CACHE_TTL_MS, }), // ... } }🤖 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/manifest.rs` around lines 24 - 29, The hardcoded timeout values in the default_config JSON should be replaced with named constants for consistency; add pub const DEFAULT_DOWNLOAD_TIMEOUT_MS: u64 = 60_000 and pub const DEFAULT_REGISTRY_CACHE_TTL_MS: u64 = 60_000 to the config module if they don't exist, then import and use DEFAULT_DOWNLOAD_TIMEOUT_MS and DEFAULT_REGISTRY_CACHE_TTL_MS alongside the existing DEFAULT_SKILLS_FOLDER and DEFAULT_REGISTRY_URL in manifest.rs (inside the default_config initializer) so the JSON uses those constants instead of literal 60_000 values.iii-directory/src/functions/mod.rs (1)
56-60: 💤 Low valueConsider making the registration count log message dynamic.
The hardcoded function count string at lines 57-59 could become outdated if functions are added or removed from the submodules. While this is informational logging and not critical, consider computing the counts dynamically or using a unit test to verify the message stays accurate.
🤖 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/mod.rs` around lines 56 - 60, The hardcoded tracing::info! message should be replaced with a dynamic log that computes the counts at runtime: determine how many entries were registered for each group (e.g., count the elements in whatever vectors/maps you use when registering skills, prompts, directory and registry functions—refer to the registration collections or functions that build them) and then call tracing::info! with formatted counts instead of a static string (update the call site that currently emits tracing::info! with the computed counts for "skills", "prompts", "skills::download", "directory", and "registry" so the message always matches actual registrations).
🤖 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/config.yaml`:
- Line 1: Update the top-line comment to reference the correct worker name
instead of "skills runtime config": change the comment in config.yaml to mention
the `iii-directory` worker (for example "iii-directory runtime config" or
similar) so the header accurately describes the file's purpose and aligns with
the worker name `iii-directory`.
In `@iii-directory/README.md`:
- Around line 166-210: Update the README header that currently reads "Thirteen
functions across four groups" to "Sixteen functions across four groups" to match
the actual list and the counts in functions/mod.rs (which logs "2 skills::*
(list + fetch_skill), 1 skill::fetch alias, 2 prompts::* (list + get), 1
skills::download, 8 directory::* and 2 registry::*"). Also correct the startup
log string in main.rs that currently prints "21 functions + 2 custom trigger
types" so it reads "16 functions + 2 custom trigger types" (ensure the literal
message emitted at startup is changed to the correct total).
In `@iii-directory/skills/directory/worker-info.md`:
- Around line 17-20: The doc currently asserts the LOCAL and registry `worker`
envelopes share the same shape but shows different fields; update the `worker`
envelope section (the LOCAL view paragraph and the registry::worker-info
reference) to explicitly list the guaranteed shared fields and mark any
additional fields as optional or surface-specific (e.g., readme, api_reference,
version_history) so clients know which keys they can rely on; ensure the wording
includes the canonical field set (e.g., id, name, description, manifestVersion)
and a clear note that registry responses may include extra optional metadata
under the same envelope, and mirror this clarification at the other occurrence
around lines 59–61.
In `@iii-directory/skills/directory/worker-list.md`:
- Around line 14-15: The doc currently contradicts itself by saying "same row
shape" but later that only core fields are shared; update both occurrences (the
phrase "same row shape" and the later lines 52-55) to a single precise contract:
state that rows share a fixed set of core fields (e.g., id, name, type,
connected) which consumers can rely on, while additional directory-specific
fields may be present and should be treated as optional/extension fields;
replace "same row shape" with this wording and ensure the later paragraph echoes
the same contract and lists the core fields by name.
In `@iii-directory/src/fs_source.rs`:
- Around line 354-360: read_body currently enforces SKILL_BODY_MAX_BYTES after
frontmatter stripping while scan_how_tos applies it to the raw file bytes,
causing inconsistency; make the cap consistent by enforcing SKILL_BODY_MAX_BYTES
on the raw file byte length in both places. Update read_body to check the
original raw bytes length (before frontmatter removal) against
SKILL_BODY_MAX_BYTES (matching scan_how_tos behavior), keeping the same error
message and identifiers (abs_path, SKILL_BODY_MAX_BYTES, read_body,
scan_how_tos) so both functions validate the same thing.
In `@iii-directory/tests/bdd.rs`:
- Around line 33-41: The shared test fixtures (world.cfg, world.skills_folder,
world.registry_url, reset_fs, reset_mocks) must be initialized regardless of
whether common::engine::get_or_init() returns Some; change the flow so you first
check common::workers::shared() and, if present, set world.cfg,
world.skills_folder, world.registry_url and call
common::workers::reset_fs(&shared.skills_folder) and await
common::workers::reset_mocks(&shared.mock_server) unconditionally, then
separately call common::engine::get_or_init().await and set world.iii =
Some(iii.clone()) only if the engine exists; keep references to
common::engine::get_or_init, common::workers::shared, common::workers::reset_fs,
common::workers::reset_mocks and the world.* fields to locate the changes.
In `@iii-directory/tests/common/workers.rs`:
- Around line 74-76: The fixed short sleep
(tokio::time::sleep(std::time::Duration::from_millis(150)).await) risks
flakiness on slow CI; replace this static 150ms wait with a polling-based
readiness check that retries until function registrations are visible (with a
sensible overall timeout), or if opting for a simpler change, increase the
Duration and add a comment explaining why; update the code around the sleep call
in workers.rs (the tokio::time::sleep invocation) to perform the readiness probe
(loop + small sleeps checking registration status) or extend the timeout and
document the rationale to avoid intermittent failures.
In `@iii-directory/tests/steps/download_registry.rs`:
- Around line 30-32: The code currently silently returns when workers::shared()
yields None (e.g., `let Some(shared) = workers::shared() else { return; };`),
which masks test wiring failures; replace these early-return branches with a
fail-fast assertion such as using expect or panic with a clear message (e.g.,
`workers::shared().expect("workers::shared() not initialized: ensure world.iii
is set up")`) and do the same for the other identical patterns that check for
Some(...) and return (the occurrences around the setup/invocation that reference
world.iii), so tests fail loudly when wiring is incorrect.
---
Outside diff comments:
In `@iii-directory/src/trigger_types.rs`:
- Around line 121-125: Update the trigger description passed to
RegisterTriggerType::new for PROMPTS_ON_CHANGE to match the module docs: replace
the current "Fires after any mutation of the prompts registry (register /
unregister)." text with a description that states it fires "after every
successful skills::download that wrote at least one prompt markdown file" (or
equivalent wording). Modify the string in the register_trigger_type call (where
register_trigger_type is invoked with RegisterTriggerType::new,
PROMPTS_ON_CHANGE, and SkillsTriggerHandler::new) so the description accurately
reflects the trigger semantics.
- Around line 114-118: Update the trigger description passed to
RegisterTriggerType::new for SKILLS_ON_CHANGE so it matches the module docs:
replace the current string "Fires after any mutation of the skills registry
(register / unregister)." with "Fires after every successful `skills::download`
that wrote at least one skill markdown file."; keep the same use of
SKILLS_ON_CHANGE and SkillsTriggerHandler::new(SKILLS_ON_CHANGE, skills.clone())
so only the human-readable description changes.
---
Nitpick comments:
In `@iii-directory/src/functions/mod.rs`:
- Around line 56-60: The hardcoded tracing::info! message should be replaced
with a dynamic log that computes the counts at runtime: determine how many
entries were registered for each group (e.g., count the elements in whatever
vectors/maps you use when registering skills, prompts, directory and registry
functions—refer to the registration collections or functions that build them)
and then call tracing::info! with formatted counts instead of a static string
(update the call site that currently emits tracing::info! with the computed
counts for "skills", "prompts", "skills::download", "directory", and "registry"
so the message always matches actual registrations).
In `@iii-directory/src/how_to.rs`:
- Around line 135-145: find_for_function currently calls scan_how_tos every time
which re-scans and reparses the skills_folder; to fix, avoid repeated scanning
by either (A) adding an optional parameter to find_for_function like
pre_scanned: Option<&[FsHowTo]> and use that when provided, or (B) implement a
simple memo cache keyed by the skills_folder Path (e.g., static
Mutex<HashMap<PathBuf, Vec<FsHowTo>>>) so scan_how_tos is only executed once and
subsequent calls reuse cached Vec<FsHowTo>; ensure cache invalidation or a clear
API if the filesystem may change, and keep references to function_id_to_uri and
the existing find logic unchanged so callers can opt-in to pass pre-scanned data
or rely on the cached results.
In `@iii-directory/src/manifest.rs`:
- Around line 24-29: The hardcoded timeout values in the default_config JSON
should be replaced with named constants for consistency; add pub const
DEFAULT_DOWNLOAD_TIMEOUT_MS: u64 = 60_000 and pub const
DEFAULT_REGISTRY_CACHE_TTL_MS: u64 = 60_000 to the config module if they don't
exist, then import and use DEFAULT_DOWNLOAD_TIMEOUT_MS and
DEFAULT_REGISTRY_CACHE_TTL_MS alongside the existing DEFAULT_SKILLS_FOLDER and
DEFAULT_REGISTRY_URL in manifest.rs (inside the default_config initializer) so
the JSON uses those constants instead of literal 60_000 values.
In `@iii-directory/src/sources/git.rs`:
- Around line 158-162: The call to validate_relative_path(&rel_str) is redundant
because rel_str is built from sanitized components (next_rel) and its errors are
ignored; remove the validation call and the ignore assignment so rel_str is used
directly (delete the line containing let _ = validate_relative_path(&rel_str);),
or alternatively replace it by handling the Result from
validate_relative_path(&rel_str) and skipping the write when Err (e.g., if
validate_relative_path(&rel_str).is_err() { continue; }); refer to rel_str,
next_rel, and validate_relative_path when making the change.
🪄 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: 37a4463c-f044-4936-bfb1-84065b01e029
⛔ Files ignored due to path filters (1)
iii-directory/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (85)
.gitignoreiii-directory/Cargo.tomliii-directory/README.mdiii-directory/build.rsiii-directory/config.yamliii-directory/iii.worker.yamliii-directory/skills/directory/function-info.mdiii-directory/skills/directory/function-list.mdiii-directory/skills/directory/registered-trigger-info.mdiii-directory/skills/directory/registered-trigger-list.mdiii-directory/skills/directory/trigger-info.mdiii-directory/skills/directory/trigger-list.mdiii-directory/skills/directory/worker-info.mdiii-directory/skills/directory/worker-list.mdiii-directory/skills/registry/worker-info.mdiii-directory/skills/registry/worker-list.mdiii-directory/src/config.rsiii-directory/src/fs_source.rsiii-directory/src/functions/directory.rsiii-directory/src/functions/download.rsiii-directory/src/functions/mod.rsiii-directory/src/functions/prompts.rsiii-directory/src/functions/registry.rsiii-directory/src/functions/skills.rsiii-directory/src/how_to.rsiii-directory/src/lib.rsiii-directory/src/main.rsiii-directory/src/manifest.rsiii-directory/src/sources/git.rsiii-directory/src/sources/mod.rsiii-directory/src/sources/registry.rsiii-directory/src/trigger_types.rsiii-directory/tests/bdd.rsiii-directory/tests/common/engine.rsiii-directory/tests/common/mod.rsiii-directory/tests/common/workers.rsiii-directory/tests/common/world.rsiii-directory/tests/features/directory_functions.featureiii-directory/tests/features/directory_triggers.featureiii-directory/tests/features/directory_workers.featureiii-directory/tests/features/download_registry.featureiii-directory/tests/features/download_repo.featureiii-directory/tests/features/prompts.featureiii-directory/tests/features/read.featureiii-directory/tests/features/registry_worker_info.featureiii-directory/tests/features/registry_worker_list.featureiii-directory/tests/steps/directory.rsiii-directory/tests/steps/download_registry.rsiii-directory/tests/steps/download_repo.rsiii-directory/tests/steps/mod.rsiii-directory/tests/steps/prompts.rsiii-directory/tests/steps/read.rsiii-directory/tests/steps/registry.rsskills/README.mdskills/config.yamlskills/iii.worker.yamlskills/src/config.rsskills/src/fs_source.rsskills/src/functions/mod.rsskills/src/functions/prompts.rsskills/src/lib.rsskills/src/state.rsskills/tests/common/workers.rsskills/tests/common/world.rsskills/tests/features/fs_sources.featureskills/tests/features/markdown.featureskills/tests/features/mcp_bridge.featureskills/tests/features/notifications.featureskills/tests/features/prompts_get.featureskills/tests/features/prompts_register.featureskills/tests/features/skills_fetch.featureskills/tests/features/skills_nested.featureskills/tests/features/skills_register.featureskills/tests/features/skills_resources.featureskills/tests/steps/fs_sources.rsskills/tests/steps/markdown.rsskills/tests/steps/mcp_bridge.rsskills/tests/steps/mod.rsskills/tests/steps/notifications.rsskills/tests/steps/prompts_get.rsskills/tests/steps/prompts_register.rsskills/tests/steps/skills_fetch.rsskills/tests/steps/skills_nested.rsskills/tests/steps/skills_register.rsskills/tests/steps/skills_resources.rs
💤 Files with no reviewable changes (32)
- skills/src/lib.rs
- skills/iii.worker.yaml
- skills/tests/features/prompts_get.feature
- skills/tests/steps/prompts_get.rs
- skills/tests/steps/prompts_register.rs
- skills/src/config.rs
- skills/tests/features/markdown.feature
- skills/tests/features/skills_fetch.feature
- skills/tests/features/notifications.feature
- skills/config.yaml
- skills/tests/common/workers.rs
- skills/src/state.rs
- skills/tests/common/world.rs
- skills/tests/features/fs_sources.feature
- skills/tests/steps/markdown.rs
- skills/tests/features/mcp_bridge.feature
- skills/README.md
- skills/tests/steps/mcp_bridge.rs
- skills/tests/features/skills_register.feature
- skills/tests/features/skills_resources.feature
- skills/tests/features/skills_nested.feature
- skills/tests/steps/mod.rs
- skills/tests/steps/skills_nested.rs
- skills/tests/steps/fs_sources.rs
- skills/src/fs_source.rs
- skills/src/functions/mod.rs
- skills/tests/features/prompts_register.feature
- skills/tests/steps/skills_fetch.rs
- skills/tests/steps/notifications.rs
- skills/tests/steps/skills_resources.rs
- skills/src/functions/prompts.rs
- skills/tests/steps/skills_register.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/README.md`:
- Line 174: The table cell for `skills::download` contains `{worker,
version|tag}` which breaks the markdown table because the `|` is parsed as a
column separator; update that cell to escape the pipe (e.g., change `{worker,
version|tag}` to `{worker, version\|tag}` or use the HTML entity `|`) so
the `skills::download` table row renders correctly.
🪄 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: 3ebc14bf-fdf1-4a8d-b0d1-a6d72fa5c533
📒 Files selected for processing (15)
iii-directory/README.mdiii-directory/config.yamliii-directory/skills/directory/worker-info.mdiii-directory/skills/directory/worker-list.mdiii-directory/skills/registry/worker-info.mdiii-directory/skills/registry/worker-list.mdiii-directory/src/fs_source.rsiii-directory/src/functions/directory.rsiii-directory/src/functions/registry.rsiii-directory/src/how_to.rsiii-directory/src/main.rsiii-directory/src/manifest.rsiii-directory/src/trigger_types.rsiii-directory/tests/steps/directory.rsiii-directory/tests/steps/registry.rs
✅ Files skipped from review due to trivial changes (5)
- iii-directory/skills/directory/worker-list.md
- iii-directory/skills/directory/worker-info.md
- iii-directory/skills/registry/worker-info.md
- iii-directory/skills/registry/worker-list.md
- iii-directory/src/trigger_types.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- iii-directory/config.yaml
- iii-directory/tests/steps/registry.rs
- iii-directory/src/how_to.rs
- iii-directory/src/manifest.rs
- iii-directory/src/main.rs
- iii-directory/src/functions/registry.rs
- iii-directory/src/fs_source.rs
- iii-directory/tests/steps/directory.rs
- iii-directory/src/functions/directory.rs
|
|
||
| | Function ID | Description | | ||
| |---|---| | ||
| | `skills::download` | Pull markdown into `skills_folder`. Either `{repo, skill}` or `{worker, version|tag}`. | |
There was a problem hiding this comment.
Escape the pipe in the table cell to fix markdown parsing.
Line 174 uses `{worker, version|tag}` inside a table cell; the | is parsed as an extra column delimiter, which breaks the table shape.
Proposed fix
-| `skills::download` | Pull markdown into `skills_folder`. Either `{repo, skill}` or `{worker, version|tag}`. |
+| `skills::download` | Pull markdown into `skills_folder`. Either `{repo, skill}` or `{worker, version\|tag}`. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `skills::download` | Pull markdown into `skills_folder`. Either `{repo, skill}` or `{worker, version|tag}`. | | |
| | `skills::download` | Pull markdown into `skills_folder`. Either `{repo, skill}` or `{worker, version\|tag}`. | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 174-174: Table column count
Expected: 2; Actual: 3; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 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/README.md` at line 174, The table cell for `skills::download`
contains `{worker, version|tag}` which breaks the markdown table because the `|`
is parsed as a column separator; update that cell to escape the pipe (e.g.,
change `{worker, version|tag}` to `{worker, version\|tag}` or use the HTML
entity `|`) so the `skills::download` table row renders correctly.
ytallo
approved these changes
May 12, 2026
Introduced a new Python script to build the skills payload for the workers registry from markdown files in the worker directory. Updated the _publish-registry workflow to include a step for generating this payload and conditionally posting it to the API. Also added an index.md file for the iii-directory skills, enhancing documentation and structure.
sergiofilhowz
force-pushed
the
feat/iii-directory
branch
from
May 12, 2026 19:51
a74d55c to
43747b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/scripts/build_skills_payload.py (1)
75-80: 💤 Low valueConsider adding explicit check for GITHUB_OUTPUT.
If
GITHUB_OUTPUTis not set (e.g., when testing locally), the skip signal won't be written to file, but the function returns successfully. The workflow would then POST the empty{}payload, which the docstring (lines 13-15) warns against.While
GITHUB_OUTPUTis always set in GitHub Actions, a defensive check would make the failure mode explicit.🛡️ Optional defensive fix
def _signal_skip(worker: str) -> None: gha_out = os.environ.get("GITHUB_OUTPUT") - if gha_out: - with open(gha_out, "a", encoding="utf-8") as f: - f.write("skip=true\n") + if not gha_out: + raise RuntimeError("GITHUB_OUTPUT environment variable is not set; cannot signal skip") + with open(gha_out, "a", encoding="utf-8") as f: + f.write("skip=true\n") print(f"::notice::no skills found for {worker}; skipping POST /w/.../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 @.github/scripts/build_skills_payload.py around lines 75 - 80, The _signal_skip function currently silently skips writing to GITHUB_OUTPUT when the env var is missing, which can allow an empty payload to be posted; modify _signal_skip to explicitly handle a missing gha_out by logging an error/notice and failing fast (e.g., call sys.exit(1) or raise a RuntimeError) so the workflow won't proceed to POST an empty payload; update the function (_signal_skip) to check os.environ.get("GITHUB_OUTPUT") and when it's falsy emit a clear error message and exit non-zero.
🤖 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/skills/index.md`:
- Around line 38-50: The Markdown links in iii-directory/skills/index.md use
redundant "skills/" prefixes (e.g., "skills/directory/function-list.md" and
"skills/registry/worker-list.md") which will resolve incorrectly; update each
link target to be relative to the current folder (remove the leading "skills/"
so they become "directory/function-list.md", "directory/function-info.md",
"registry/worker-list.md", etc.) so the `directory::*` and `registry::*` How-to
links resolve correctly.
In `@iii-directory/tests/common/workers.rs`:
- Around line 46-49: The register_all function currently uses SHARED.get()
followed by set(), allowing a race where multiple callers run the expensive init
(tempdir creation, mock server startup, handler registration); replace that
pattern by calling SHARED.get_or_try_init(...) so the initialization closure
runs atomically once and returns a Result<Arc<Shared>, _>, move all setup logic
(tempdir, mock server, handler registration, creation of Arc<Shared>) into that
closure and return the Arc, and make register_all simply await get_or_try_init
and clone/return the Arc on success; ensure the closure uses the same error type
as the function so failures propagate correctly.
---
Nitpick comments:
In @.github/scripts/build_skills_payload.py:
- Around line 75-80: The _signal_skip function currently silently skips writing
to GITHUB_OUTPUT when the env var is missing, which can allow an empty payload
to be posted; modify _signal_skip to explicitly handle a missing gha_out by
logging an error/notice and failing fast (e.g., call sys.exit(1) or raise a
RuntimeError) so the workflow won't proceed to POST an empty payload; update the
function (_signal_skip) to check os.environ.get("GITHUB_OUTPUT") and when it's
falsy emit a clear error message and exit non-zero.
🪄 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: 60538825-5cb7-4672-992a-903625d146fa
⛔ Files ignored due to path filters (1)
iii-directory/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (90)
.github/scripts/build_skills_payload.py.github/workflows/_publish-registry.yml.github/workflows/create-tag.yml.github/workflows/release.yml.gitignoreiii-directory/Cargo.tomliii-directory/README.mdiii-directory/build.rsiii-directory/config.yamliii-directory/iii.worker.yamliii-directory/skills/directory/function-info.mdiii-directory/skills/directory/function-list.mdiii-directory/skills/directory/registered-trigger-info.mdiii-directory/skills/directory/registered-trigger-list.mdiii-directory/skills/directory/trigger-info.mdiii-directory/skills/directory/trigger-list.mdiii-directory/skills/directory/worker-info.mdiii-directory/skills/directory/worker-list.mdiii-directory/skills/index.mdiii-directory/skills/registry/worker-info.mdiii-directory/skills/registry/worker-list.mdiii-directory/src/config.rsiii-directory/src/fs_source.rsiii-directory/src/functions/directory.rsiii-directory/src/functions/download.rsiii-directory/src/functions/mod.rsiii-directory/src/functions/prompts.rsiii-directory/src/functions/registry.rsiii-directory/src/functions/skills.rsiii-directory/src/how_to.rsiii-directory/src/lib.rsiii-directory/src/main.rsiii-directory/src/manifest.rsiii-directory/src/sources/git.rsiii-directory/src/sources/mod.rsiii-directory/src/sources/registry.rsiii-directory/src/trigger_types.rsiii-directory/tests/bdd.rsiii-directory/tests/common/engine.rsiii-directory/tests/common/mod.rsiii-directory/tests/common/workers.rsiii-directory/tests/common/world.rsiii-directory/tests/features/directory_functions.featureiii-directory/tests/features/directory_triggers.featureiii-directory/tests/features/directory_workers.featureiii-directory/tests/features/download_registry.featureiii-directory/tests/features/download_repo.featureiii-directory/tests/features/prompts.featureiii-directory/tests/features/read.featureiii-directory/tests/features/registry_worker_info.featureiii-directory/tests/features/registry_worker_list.featureiii-directory/tests/steps/directory.rsiii-directory/tests/steps/download_registry.rsiii-directory/tests/steps/download_repo.rsiii-directory/tests/steps/mod.rsiii-directory/tests/steps/prompts.rsiii-directory/tests/steps/read.rsiii-directory/tests/steps/registry.rsskills/README.mdskills/config.yamlskills/iii.worker.yamlskills/src/config.rsskills/src/fs_source.rsskills/src/functions/mod.rsskills/src/functions/prompts.rsskills/src/lib.rsskills/src/state.rsskills/tests/common/workers.rsskills/tests/common/world.rsskills/tests/features/fs_sources.featureskills/tests/features/markdown.featureskills/tests/features/mcp_bridge.featureskills/tests/features/notifications.featureskills/tests/features/prompts_get.featureskills/tests/features/prompts_register.featureskills/tests/features/skills_fetch.featureskills/tests/features/skills_nested.featureskills/tests/features/skills_register.featureskills/tests/features/skills_resources.featureskills/tests/steps/fs_sources.rsskills/tests/steps/markdown.rsskills/tests/steps/mcp_bridge.rsskills/tests/steps/mod.rsskills/tests/steps/notifications.rsskills/tests/steps/prompts_get.rsskills/tests/steps/prompts_register.rsskills/tests/steps/skills_fetch.rsskills/tests/steps/skills_nested.rsskills/tests/steps/skills_register.rsskills/tests/steps/skills_resources.rs
💤 Files with no reviewable changes (32)
- skills/tests/features/skills_resources.feature
- skills/tests/steps/notifications.rs
- skills/tests/features/notifications.feature
- skills/README.md
- skills/tests/features/skills_register.feature
- skills/tests/steps/mcp_bridge.rs
- skills/tests/features/fs_sources.feature
- skills/tests/common/workers.rs
- skills/tests/features/mcp_bridge.feature
- skills/tests/features/skills_fetch.feature
- skills/tests/steps/mod.rs
- skills/tests/features/prompts_get.feature
- skills/src/functions/prompts.rs
- skills/src/functions/mod.rs
- skills/tests/features/skills_nested.feature
- skills/tests/common/world.rs
- skills/tests/steps/skills_resources.rs
- skills/src/lib.rs
- skills/config.yaml
- skills/tests/steps/skills_nested.rs
- skills/tests/steps/skills_register.rs
- skills/src/fs_source.rs
- skills/tests/steps/prompts_get.rs
- skills/tests/steps/markdown.rs
- skills/src/config.rs
- skills/tests/steps/prompts_register.rs
- skills/tests/features/prompts_register.feature
- skills/iii.worker.yaml
- skills/tests/steps/fs_sources.rs
- skills/src/state.rs
- skills/tests/features/markdown.feature
- skills/tests/steps/skills_fetch.rs
✅ Files skipped from review due to trivial changes (12)
- iii-directory/config.yaml
- .github/workflows/release.yml
- iii-directory/skills/directory/registered-trigger-info.md
- iii-directory/skills/directory/trigger-list.md
- iii-directory/skills/directory/worker-list.md
- iii-directory/skills/directory/registered-trigger-list.md
- iii-directory/skills/registry/worker-list.md
- iii-directory/src/lib.rs
- .gitignore
- iii-directory/skills/directory/function-list.md
- iii-directory/skills/registry/worker-info.md
- iii-directory/skills/directory/function-info.md
🚧 Files skipped from review as they are similar to previous changes (30)
- iii-directory/tests/features/directory_workers.feature
- iii-directory/skills/directory/worker-info.md
- iii-directory/tests/features/download_registry.feature
- iii-directory/tests/features/directory_triggers.feature
- iii-directory/iii.worker.yaml
- iii-directory/tests/features/directory_functions.feature
- iii-directory/src/manifest.rs
- iii-directory/tests/steps/mod.rs
- iii-directory/src/functions/prompts.rs
- iii-directory/tests/features/registry_worker_list.feature
- iii-directory/tests/features/read.feature
- iii-directory/src/functions/mod.rs
- iii-directory/tests/steps/read.rs
- iii-directory/src/main.rs
- iii-directory/tests/features/download_repo.feature
- iii-directory/tests/steps/prompts.rs
- iii-directory/src/sources/git.rs
- iii-directory/tests/steps/download_registry.rs
- iii-directory/tests/features/registry_worker_info.feature
- iii-directory/src/fs_source.rs
- iii-directory/tests/common/world.rs
- iii-directory/src/config.rs
- iii-directory/tests/steps/download_repo.rs
- iii-directory/tests/steps/registry.rs
- iii-directory/src/functions/download.rs
- iii-directory/src/functions/directory.rs
- iii-directory/src/how_to.rs
- iii-directory/tests/steps/directory.rs
- iii-directory/src/functions/registry.rs
- iii-directory/src/functions/skills.rs
Comment on lines
+38
to
+50
| - [`directory::function-list`](skills/directory/function-list.md) — list functions registered with the engine; filter by search/prefix/worker. | ||
| - [`directory::function-info`](skills/directory/function-info.md) — inspect one function's schemas, owner, and how-to skill. | ||
| - [`directory::trigger-list`](skills/directory/trigger-list.md) — list trigger types registered with the engine. | ||
| - [`directory::trigger-info`](skills/directory/trigger-info.md) — inspect one trigger type's schemas + live instance count. | ||
| - [`directory::registered-trigger-list`](skills/directory/registered-trigger-list.md) — list registered trigger instances (subscriber rows). | ||
| - [`directory::registered-trigger-info`](skills/directory/registered-trigger-info.md) — inspect one registered trigger (instance + type + function). | ||
| - [`directory::worker-list`](skills/directory/worker-list.md) — list workers connected to the engine; same row shape as `registry::worker-list`. | ||
| - [`directory::worker-info`](skills/directory/worker-info.md) — inspect one connected worker's full surface. | ||
|
|
||
| ### `registry::*` — what's published in the public registry | ||
|
|
||
| - [`registry::worker-list`](skills/registry/worker-list.md) — search published workers in `api.workers.iii.dev`; same row shape as `directory::worker-list`. | ||
| - [`registry::worker-info`](skills/registry/worker-info.md) — full registry detail for one worker (envelope + readme + api_reference + skills_tree). |
There was a problem hiding this comment.
Fix relative paths in How-tos links (currently likely broken).
From iii-directory/skills/index.md, links like skills/directory/... and skills/registry/... likely resolve to iii-directory/skills/skills/.... These should be relative to the current folder (e.g., directory/..., registry/...) so docs navigation works.
Suggested diff
-- [`directory::function-list`](skills/directory/function-list.md) — list functions registered with the engine; filter by search/prefix/worker.
-- [`directory::function-info`](skills/directory/function-info.md) — inspect one function's schemas, owner, and how-to skill.
-- [`directory::trigger-list`](skills/directory/trigger-list.md) — list trigger types registered with the engine.
-- [`directory::trigger-info`](skills/directory/trigger-info.md) — inspect one trigger type's schemas + live instance count.
-- [`directory::registered-trigger-list`](skills/directory/registered-trigger-list.md) — list registered trigger instances (subscriber rows).
-- [`directory::registered-trigger-info`](skills/directory/registered-trigger-info.md) — inspect one registered trigger (instance + type + function).
-- [`directory::worker-list`](skills/directory/worker-list.md) — list workers connected to the engine; same row shape as `registry::worker-list`.
-- [`directory::worker-info`](skills/directory/worker-info.md) — inspect one connected worker's full surface.
+- [`directory::function-list`](directory/function-list.md) — list functions registered with the engine; filter by search/prefix/worker.
+- [`directory::function-info`](directory/function-info.md) — inspect one function's schemas, owner, and how-to skill.
+- [`directory::trigger-list`](directory/trigger-list.md) — list trigger types registered with the engine.
+- [`directory::trigger-info`](directory/trigger-info.md) — inspect one trigger type's schemas + live instance count.
+- [`directory::registered-trigger-list`](directory/registered-trigger-list.md) — list registered trigger instances (subscriber rows).
+- [`directory::registered-trigger-info`](directory/registered-trigger-info.md) — inspect one registered trigger (instance + type + function).
+- [`directory::worker-list`](directory/worker-list.md) — list workers connected to the engine; same row shape as `registry::worker-list`.
+- [`directory::worker-info`](directory/worker-info.md) — inspect one connected worker's full surface.
-- [`registry::worker-list`](skills/registry/worker-list.md) — search published workers in `api.workers.iii.dev`; same row shape as `directory::worker-list`.
-- [`registry::worker-info`](skills/registry/worker-info.md) — full registry detail for one worker (envelope + readme + api_reference + skills_tree).
+- [`registry::worker-list`](registry/worker-list.md) — search published workers in `api.workers.iii.dev`; same row shape as `directory::worker-list`.
+- [`registry::worker-info`](registry/worker-info.md) — full registry detail for one worker (envelope + readme + api_reference + skills_tree).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - [`directory::function-list`](skills/directory/function-list.md) — list functions registered with the engine; filter by search/prefix/worker. | |
| - [`directory::function-info`](skills/directory/function-info.md) — inspect one function's schemas, owner, and how-to skill. | |
| - [`directory::trigger-list`](skills/directory/trigger-list.md) — list trigger types registered with the engine. | |
| - [`directory::trigger-info`](skills/directory/trigger-info.md) — inspect one trigger type's schemas + live instance count. | |
| - [`directory::registered-trigger-list`](skills/directory/registered-trigger-list.md) — list registered trigger instances (subscriber rows). | |
| - [`directory::registered-trigger-info`](skills/directory/registered-trigger-info.md) — inspect one registered trigger (instance + type + function). | |
| - [`directory::worker-list`](skills/directory/worker-list.md) — list workers connected to the engine; same row shape as `registry::worker-list`. | |
| - [`directory::worker-info`](skills/directory/worker-info.md) — inspect one connected worker's full surface. | |
| ### `registry::*` — what's published in the public registry | |
| - [`registry::worker-list`](skills/registry/worker-list.md) — search published workers in `api.workers.iii.dev`; same row shape as `directory::worker-list`. | |
| - [`registry::worker-info`](skills/registry/worker-info.md) — full registry detail for one worker (envelope + readme + api_reference + skills_tree). | |
| - [`directory::function-list`](directory/function-list.md) — list functions registered with the engine; filter by search/prefix/worker. | |
| - [`directory::function-info`](directory/function-info.md) — inspect one function's schemas, owner, and how-to skill. | |
| - [`directory::trigger-list`](directory/trigger-list.md) — list trigger types registered with the engine. | |
| - [`directory::trigger-info`](directory/trigger-info.md) — inspect one trigger type's schemas + live instance count. | |
| - [`directory::registered-trigger-list`](directory/registered-trigger-list.md) — list registered trigger instances (subscriber rows). | |
| - [`directory::registered-trigger-info`](directory/registered-trigger-info.md) — inspect one registered trigger (instance + type + function). | |
| - [`directory::worker-list`](directory/worker-list.md) — list workers connected to the engine; same row shape as `registry::worker-list`. | |
| - [`directory::worker-info`](directory/worker-info.md) — inspect one connected worker's full surface. | |
| ### `registry::*` — what's published in the public registry | |
| - [`registry::worker-list`](registry/worker-list.md) — search published workers in `api.workers.iii.dev`; same row shape as `directory::worker-list`. | |
| - [`registry::worker-info`](registry/worker-info.md) — full registry detail for one worker (envelope + readme + api_reference + skills_tree). |
🤖 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/index.md` around lines 38 - 50, The Markdown links in
iii-directory/skills/index.md use redundant "skills/" prefixes (e.g.,
"skills/directory/function-list.md" and "skills/registry/worker-list.md") which
will resolve incorrectly; update each link target to be relative to the current
folder (remove the leading "skills/" so they become
"directory/function-list.md", "directory/function-info.md",
"registry/worker-list.md", etc.) so the `directory::*` and `registry::*` How-to
links resolve correctly.
Comment on lines
+46
to
+49
| pub async fn register_all(iii: &Arc<III>) -> Result<Arc<Shared>> { | ||
| if let Some(s) = SHARED.get() { | ||
| return Ok(s.clone()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify non-atomic OnceCell usage and whether get_or_try_init is already used.
rg -n -C3 'SHARED\.get\(|SHARED\.set\(|get_or_try_init|OnceCell' iii-directory/tests/common/workers.rs
rg -n 'tokio\s*=' iii-directory/Cargo.tomlRepository: iii-hq/workers
Length of output: 1405
🌐 Web query:
tokio sync OnceCell get_or_try_init API availability version
💡 Result:
In tokio::sync::OnceCell, the API you’re asking about exists as: 1) pub async fn get_or_try_init<F, Fut, E>(&self, f: F) -> Result<&T, E> - Signature is documented on docs.rs for tokio’s OnceCell.[1] - The semantics are: if the cell is initialized, it returns a reference; if another task is initializing, it waits; if f returns an error, the error is returned and the initialization attempt is cancelled/does not store a value.[1] 2) Availability / when it exists - Tokio’s async OnceCell support (including get_or_init / get_or_try_init) was added via PR “Add OnceCell” which was merged in 2021 (merged Apr 5, 2021).[2] - Therefore, get_or_try_init is available in Tokio versions that include that OnceCell addition; for example, it is present in tokio 1.6.0 documentation.[3] Notes - get_or_try_init exists on Tokio’s OnceCell (tokio::sync::OnceCell), not on std::sync::OnceCell.[4][1] If you tell me your target Tokio version (or MSRV), I can recommend the exact import/path and confirm it against that specific release docs.
Citations:
- 1: https://docs.rs/tokio/latest/tokio/sync/struct.OnceCell.html
- 2: Add OnceCell tokio-rs/tokio#3591
- 3: https://docs.rs/tokio/1.6.0/tokio/sync/struct.OnceCell.html
- 4: https://docs.rs/once_cell/1.7.2/once_cell/sync/struct.OnceCell.html
🏁 Script executed:
# Check the full register_all function and any other OnceCell usage patterns
cat -n iii-directory/tests/common/workers.rs | sed -n '45,86p'
# Search for any other OnceCell or SHARED usage in the entire codebase
rg -n 'SHARED\.' --type rsRepository: iii-hq/workers
Length of output: 1889
🏁 Script executed:
# Find all SHARED usages in the codebase (fixed: no -t flag)
rg -n 'SHARED' iii-directory/tests/common/workers.rs
# Check if there are any other OnceCell-like patterns in test infra
rg -n 'OnceCell|static.*:.*=' iii-directory/tests/common/Repository: iii-hq/workers
Length of output: 795
Make register_all truly idempotent by using OnceCell::get_or_try_init (Lines 47–84).
The current separate get/set pattern allows a race condition where concurrent callers can both pass the SHARED.get() check and proceed to initialize. Both will execute the expensive setup logic (tempdir creation, mock server startup, handler registration) before the first set() succeeds. Use get_or_try_init to atomically handle get-or-initialize in a single operation.
Proposed fix
pub async fn register_all(iii: &Arc<III>) -> Result<Arc<Shared>> {
- if let Some(s) = SHARED.get() {
- return Ok(s.clone());
- }
-
- // Build a leaked tempdir that lives for the test binary lifetime.
- let tmp = tempfile::tempdir()?;
- let skills_folder = tmp.keep();
- std::fs::create_dir_all(&skills_folder)?;
-
- // Boot the mock registry server before registering handlers so the
- // captured cfg already points at the right URL.
- let mock_server = Arc::new(MockServer::start().await);
-
- let cfg = Arc::new(SkillsConfig {
- skills_folder: skills_folder.to_string_lossy().into_owned(),
- registry_url: mock_server.uri(),
- config_dir: Some(
- skills_folder
- .parent()
- .map(Path::to_path_buf)
- .unwrap_or_else(|| std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."))),
- ),
- ..SkillsConfig::default()
- });
- let registered = trigger_types::register_all(iii);
- functions::register_all(iii, &cfg, ®istered);
-
- // Give the SDK a beat to publish the function registrations before
- // scenarios start triggering them.
- tokio::time::sleep(std::time::Duration::from_millis(150)).await;
-
- let shared = Arc::new(Shared {
- cfg,
- triggers: Arc::new(registered),
- skills_folder,
- mock_server,
- });
- let _ = SHARED.set(shared.clone());
- Ok(shared)
+ let shared = SHARED
+ .get_or_try_init(|| async {
+ let tmp = tempfile::tempdir()?;
+ let skills_folder = tmp.keep();
+ std::fs::create_dir_all(&skills_folder)?;
+
+ let mock_server = Arc::new(MockServer::start().await);
+ let cfg = Arc::new(SkillsConfig {
+ skills_folder: skills_folder.to_string_lossy().into_owned(),
+ registry_url: mock_server.uri(),
+ config_dir: Some(
+ skills_folder
+ .parent()
+ .map(Path::to_path_buf)
+ .unwrap_or_else(|| {
+ std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."))
+ }),
+ ),
+ ..SkillsConfig::default()
+ });
+ let registered = trigger_types::register_all(iii);
+ functions::register_all(iii, &cfg, ®istered);
+ tokio::time::sleep(std::time::Duration::from_millis(150)).await;
+
+ Ok(Arc::new(Shared {
+ cfg,
+ triggers: Arc::new(registered),
+ skills_folder,
+ mock_server,
+ }))
+ })
+ .await?;
+ Ok(shared.clone())
}🤖 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/tests/common/workers.rs` around lines 46 - 49, The register_all
function currently uses SHARED.get() followed by set(), allowing a race where
multiple callers run the expensive init (tempdir creation, mock server startup,
handler registration); replace that pattern by calling
SHARED.get_or_try_init(...) so the initialization closure runs atomically once
and returns a Result<Arc<Shared>, _>, move all setup logic (tempdir, mock
server, handler registration, creation of Arc<Shared>) into that closure and
return the Arc, and make register_all simply await get_or_try_init and
clone/return the Arc on success; ensure the closure uses the same error type as
the function so failures propagate correctly.
This was referenced May 13, 2026
5 tasks
This was referenced Jul 3, 2026
This was referenced Aug 30, 2026
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
2 participants
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.
iii-directory: one worker for “what’s on disk,” “what’s live,” and “what’s published”
Before, the repo centered on a skills worker: mostly “read markdown from a folder and expose it.” Useful for onboarding docs and static prompts, but it didn’t tell you what the engine actually had loaded—or what existed in the public workers registry.
Now, iii-directory is the same crate idea, expanded into a single, MCP-agnostic hub with four clear surfaces:
iii://…skills on diskThe pitch in one line: Stop juggling a filesystem reader and separate introspection hacks—iii-directory gives you a consistent story from local files → live engine → public catalog.
Why teams care
directory::*andregistry::*share list/info envelopes, so tools and agents can swap “local vs public” without new JSON shapes.skills::downloadmaterializes markdown under a configuredskills_folder; after that, files are normal repo assets—edit, review, ship.latestfrom the registry, or shallow-clone a skill subfolder from a public GitHub repo—then rely on on-change triggers so subscribers (e.g. MCP list refresh) stay in sync.Who it’s for
Migration: Anyone on the old skills worker should move to iii-directory and follow the README’s migration notes for paths, URIs, and config.
Summary by CodeRabbit