feat(iii-directory): skills index and fixes to registry operations - #133
Merged
Conversation
- Bump iii-sdk dependency version from 0.11.3 to 0.11.6 in Cargo.toml and update the corresponding checksum in Cargo.lock. - Enhance README and skill documentation to reflect changes in skill metadata, including the addition of the `type` field in skill responses. - Introduce a new `directory::skills::index` function to provide a concise overview of installed workers, improving agent bootstrapping efficiency. - Update various skill-related documentation to clarify the structure and usage of skills and prompts, ensuring consistency across the codebase.
📝 WalkthroughWalkthroughThis PR evolves the ChangesDirectory API Evolution
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 docstrings
🧪 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 |
Contributor
skill-check — worker0 verified, 25 skipped (no docs/).
Three for three. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
iii-directory/tests/steps/registry.rs (1)
304-312: ⚡ Quick winMake the string-specific
next_cursorassertion strict.This step currently converts non-string/null to
"", which can mask schema regressions.Proposed tightening
fn worker_list_pagination_next_cursor(world: &mut IiiSkillsWorld, expected: String) { if world.iii.is_none() { return; } let v = last_ok(world); - let actual = v["pagination"]["next_cursor"].as_str().unwrap_or(""); + let actual = v["pagination"]["next_cursor"] + .as_str() + .expect("missing pagination.next_cursor string"); assert_eq!(actual, expected, "pagination: {:?}", v["pagination"]); }🤖 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/steps/registry.rs` around lines 304 - 312, The test worker_list_pagination_next_cursor currently masks non-string values by using as_str().unwrap_or(""), so change it to require a real string: assert that v["pagination"]["next_cursor"] is a string (e.g. with is_string() or by using as_str().expect with a clear failure message) and then compare the unwrapped str to expected; reference the worker_list_pagination_next_cursor function and the v/last_ok usage so the failure shows the actual JSON type/value instead of silently turning it into "".iii-directory/src/functions/directory.rs (2)
147-149: 💤 Low valueStale SDK version reference in doc comments.
Both doc blocks reference "SDK 0.11.3", but the PR bumps
iii-sdkto 0.11.6. Update (or generalize) the comments to avoid future churn:
- Line 147–149: "SDK 0.11.3 surfaces a single
trigger_request_format…"- Line 824–826: "no
WorkerInfo.trigger_types[]field exists in SDK 0.11.3"Either re-anchor to 0.11.6 or drop the version pin if the behavior is expected to hold across the 0.11.x line.
Also applies to: 824-826
🤖 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/directory.rs` around lines 147 - 149, Update the stale "SDK 0.11.3" references in the doc comments around the trigger_request_format explanation and the note about WorkerInfo.trigger_types[]: either change the text to "SDK 0.11.6" or remove the exact patch-level pin so it reads generically (e.g., "SDK 0.11.x" or no version) to avoid future churn; edit the doc blocks that mention `trigger_request_format` and the one referencing `WorkerInfo.trigger_types[]` accordingly in directory.rs.
720-737: ⚡ Quick winSequential engine round-trips where
tokio::try_join!would halve latency.
worker_infoissues four serialengine_list_*calls (workers, then functions, then trigger_types, then triggers). Each is an independent trigger call to the engine; given typical agent-bootstrap usage whereworker_infois called repeatedly, this adds avoidable RTT. After the initialengine_list_workers(whose result drives the not-found check), the remaining three are independent and can be parallelized.The same applies to
registered_trigger_info(lines 642–648) andfetch_functions_and_workers(lines 995–1005).♻️ Example refactor for `worker_info`
- let functions = engine_list_functions(iii) - .await - .map_err(|e| format!("engine::functions::list: {e}"))?; - let trigger_types = engine_list_trigger_types(iii, true) - .await - .map_err(|e| format!("engine::trigger-types::list: {e}"))?; - let triggers = engine_list_triggers(iii, true) - .await - .map_err(|e| format!("engine::triggers::list: {e}"))?; + let (functions, trigger_types, triggers) = tokio::try_join!( + async { + engine_list_functions(iii) + .await + .map_err(|e| format!("engine::functions::list: {e}")) + }, + async { + engine_list_trigger_types(iii, true) + .await + .map_err(|e| format!("engine::trigger-types::list: {e}")) + }, + async { + engine_list_triggers(iii, true) + .await + .map_err(|e| format!("engine::triggers::list: {e}")) + }, + )?;Apply the same idea to
fetch_functions_and_workersand to the three independent fetches at the top ofregistered_trigger_info.🤖 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/directory.rs` around lines 720 - 737, worker_info currently performs engine_list_functions, engine_list_trigger_types, and engine_list_triggers sequentially after engine_list_workers; change this to keep engine_list_workers and the subsequent worker not-found check sequential, then parallelize the three independent calls using tokio::try_join! (or futures::try_join!) to await engine_list_functions(iii), engine_list_trigger_types(iii, true), and engine_list_triggers(iii, true) concurrently and propagate errors the same way; apply the same pattern to registered_trigger_info (the three independent trigger fetches) and fetch_functions_and_workers (the independent function/worker fetches) so each set of independent engine_list_* calls use try_join! and preserve existing error formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@iii-directory/src/fs_source.rs`:
- Around line 392-395: The validation for an empty body only trims '\n' so
bodies containing spaces or '\r' slip through; update the logic around
split_frontmatter / fm_text / body / trimmed (and the subsequent error path
using abs_path) to trim all Unicode whitespace (e.g. use body.trim() or
body.trim_matches(|c: char| c.is_whitespace())) before checking is_empty() so
whitespace-only bodies are treated as empty and return the same error.
In `@iii-directory/src/functions/directory.rs`:
- Around line 929-993: The helper functions engine_list_functions,
engine_list_workers, engine_list_triggers, and engine_list_trigger_types are
silently swallowing deserialization errors by using .and_then(...
.ok()).unwrap_or_default(), so change each to explicitly propagate
deserialization errors via Result mapping: fetch the response with
iii.trigger(...).await?, then get the expected field ("functions", "workers",
"triggers", "trigger_types") and convert it using serde_json::from_value(...)
returning Err(IIIError::from or map the serde error) instead of
.ok()/unwrap_or_default(); ensure the function signatures still return
Result<Vec<...>, IIIError> and map serde_json errors into that IIIError so
callers receive a failure rather than an empty Vec for malformed or incompatible
responses (apply the same pattern to engine_list_functions, engine_list_workers,
engine_list_triggers, engine_list_trigger_types).
In `@iii-directory/src/functions/registry.rs`:
- Around line 411-415: The cache key construction using string concatenation
(the cache_key variable in registry.rs and the similar construction around the
second occurrence) can collide when inputs contain ":" or "="; replace the
ad-hoc concatenation with a structured, unambiguous serializer—for example
JSON-serialize or URL-encode the key components or base64-encode a serialized
tuple (e.g., serde_json::to_string(&(search, cursor, other_fields)) or
percent-encode each component) and use that serialized string as the cache key;
update both the cache_key creation sites (the format! call that builds
"worker-list:...") and the analogous key at the other location so keys are safe
from delimiter collisions.
---
Nitpick comments:
In `@iii-directory/src/functions/directory.rs`:
- Around line 147-149: Update the stale "SDK 0.11.3" references in the doc
comments around the trigger_request_format explanation and the note about
WorkerInfo.trigger_types[]: either change the text to "SDK 0.11.6" or remove the
exact patch-level pin so it reads generically (e.g., "SDK 0.11.x" or no version)
to avoid future churn; edit the doc blocks that mention `trigger_request_format`
and the one referencing `WorkerInfo.trigger_types[]` accordingly in
directory.rs.
- Around line 720-737: worker_info currently performs engine_list_functions,
engine_list_trigger_types, and engine_list_triggers sequentially after
engine_list_workers; change this to keep engine_list_workers and the subsequent
worker not-found check sequential, then parallelize the three independent calls
using tokio::try_join! (or futures::try_join!) to await
engine_list_functions(iii), engine_list_trigger_types(iii, true), and
engine_list_triggers(iii, true) concurrently and propagate errors the same way;
apply the same pattern to registered_trigger_info (the three independent trigger
fetches) and fetch_functions_and_workers (the independent function/worker
fetches) so each set of independent engine_list_* calls use try_join! and
preserve existing error formatting.
In `@iii-directory/tests/steps/registry.rs`:
- Around line 304-312: The test worker_list_pagination_next_cursor currently
masks non-string values by using as_str().unwrap_or(""), so change it to require
a real string: assert that v["pagination"]["next_cursor"] is a string (e.g. with
is_string() or by using as_str().expect with a clear failure message) and then
compare the unwrapped str to expected; reference the
worker_list_pagination_next_cursor function and the v/last_ok usage so the
failure shows the actual JSON type/value instead of silently turning it into "".
🪄 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: 01c7e5cd-e3a1-4655-893c-c15c71222a92
⛔ Files ignored due to path filters (1)
iii-directory/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
iii-directory/Cargo.tomliii-directory/README.mdiii-directory/skills/directory/registry/workers/info.mdiii-directory/skills/directory/registry/workers/list.mdiii-directory/skills/directory/skills/get.mdiii-directory/skills/directory/skills/index.mdiii-directory/skills/directory/skills/list.mdiii-directory/skills/index.mdiii-directory/src/fs_source.rsiii-directory/src/functions/directory.rsiii-directory/src/functions/mod.rsiii-directory/src/functions/registry.rsiii-directory/src/functions/skills.rsiii-directory/src/lib.rsiii-directory/src/sources/registry.rsiii-directory/tests/features/download_registry.featureiii-directory/tests/features/read.featureiii-directory/tests/features/registry_worker_info.featureiii-directory/tests/features/registry_worker_list.featureiii-directory/tests/steps/download_registry.rsiii-directory/tests/steps/read.rsiii-directory/tests/steps/registry.rs
Comment on lines
+392
to
395
| let (fm_text, body) = split_frontmatter(&raw); | ||
| let trimmed = body.trim_matches('\n'); | ||
| if trimmed.is_empty() { | ||
| return Err(format!("file {} has empty body", abs_path.display())); |
There was a problem hiding this comment.
Use whitespace-aware empty-body validation.
The current check only trims \n, so a body with just spaces and/or \r can pass as non-empty after frontmatter stripping.
Suggested fix
- let trimmed = body.trim_matches('\n');
+ let trimmed = body.trim();
if trimmed.is_empty() {
return Err(format!("file {} has empty body", abs_path.display()));
}📝 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
| let (fm_text, body) = split_frontmatter(&raw); | |
| let trimmed = body.trim_matches('\n'); | |
| if trimmed.is_empty() { | |
| return Err(format!("file {} has empty body", abs_path.display())); | |
| let (fm_text, body) = split_frontmatter(&raw); | |
| let trimmed = body.trim(); | |
| if trimmed.is_empty() { | |
| return Err(format!("file {} has empty body", abs_path.display())); |
🤖 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/fs_source.rs` around lines 392 - 395, The validation for an
empty body only trims '\n' so bodies containing spaces or '\r' slip through;
update the logic around split_frontmatter / fm_text / body / trimmed (and the
subsequent error path using abs_path) to trim all Unicode whitespace (e.g. use
body.trim() or body.trim_matches(|c: char| c.is_whitespace())) before checking
is_empty() so whitespace-only bodies are treated as empty and return the same
error.
Comment on lines
+929
to
+993
| async fn engine_list_functions(iii: &III) -> Result<Vec<SdkFunctionInfo>, IIIError> { | ||
| let result = iii | ||
| .trigger(TriggerRequest { | ||
| function_id: "engine::functions::list".into(), | ||
| payload: serde_json::json!({}), | ||
| action: None, | ||
| timeout_ms: None, | ||
| }) | ||
| .await?; | ||
| Ok(result | ||
| .get("functions") | ||
| .and_then(|v| serde_json::from_value(v.clone()).ok()) | ||
| .unwrap_or_default()) | ||
| } | ||
|
|
||
| async fn engine_list_workers(iii: &III) -> Result<Vec<WorkerInfo>, IIIError> { | ||
| let result = iii | ||
| .trigger(TriggerRequest { | ||
| function_id: "engine::workers::list".into(), | ||
| payload: serde_json::json!({}), | ||
| action: None, | ||
| timeout_ms: None, | ||
| }) | ||
| .await?; | ||
| Ok(result | ||
| .get("workers") | ||
| .and_then(|v| serde_json::from_value(v.clone()).ok()) | ||
| .unwrap_or_default()) | ||
| } | ||
|
|
||
| async fn engine_list_triggers( | ||
| iii: &III, | ||
| include_internal: bool, | ||
| ) -> Result<Vec<SdkTriggerInfo>, IIIError> { | ||
| let result = iii | ||
| .trigger(TriggerRequest { | ||
| function_id: "engine::triggers::list".into(), | ||
| payload: serde_json::json!({ "include_internal": include_internal }), | ||
| action: None, | ||
| timeout_ms: None, | ||
| }) | ||
| .await?; | ||
| Ok(result | ||
| .get("triggers") | ||
| .and_then(|v| serde_json::from_value(v.clone()).ok()) | ||
| .unwrap_or_default()) | ||
| } | ||
|
|
||
| async fn engine_list_trigger_types( | ||
| iii: &III, | ||
| include_internal: bool, | ||
| ) -> Result<Vec<TriggerTypeInfo>, IIIError> { | ||
| let result = iii | ||
| .trigger(TriggerRequest { | ||
| function_id: "engine::trigger-types::list".into(), | ||
| payload: serde_json::json!({ "include_internal": include_internal }), | ||
| action: None, | ||
| timeout_ms: None, | ||
| }) | ||
| .await?; | ||
| Ok(result | ||
| .get("trigger_types") | ||
| .and_then(|v| serde_json::from_value(v.clone()).ok()) | ||
| .unwrap_or_default()) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect TriggerRequest fields, III::trigger signature, and IIIError variants used in this repo.
fd -t f 'Cargo.toml' | xargs rg -n 'iii-sdk'
rg -nP '\bpub\s+(struct|enum)\s+IIIError\b' -C 20
rg -nP '\bfn\s+trigger\s*\(' -C 5
rg -nP '\bIIIError::Handler\b' -C 2
rg -nP '\bTriggerRequest\s*\{' -C 8Repository: iii-hq/workers
Length of output: 50371
🏁 Script executed:
# Check for stale SDK version comments in iii-directory/src/functions/directory.rs
sed -n '147,149p; 824,826p' iii-directory/src/functions/directory.rs
# Check how many handlers call engine_list_* functions
rg -n 'engine_list_' iii-directory/src/functions/directory.rs | head -20Repository: iii-hq/workers
Length of output: 1484
Silent error swallowing in engine_list_ helpers masks deserialization failures.*
All four engine_list_* helpers use .and_then(|v| serde_json::from_value(v.clone()).ok()).unwrap_or_default(), which collapses three distinct failure modes into a single empty-Vec result:
- Key missing from response → empty Vec.
- Malformed/incompatible response shape (e.g., schema drift after SDK update) → empty Vec, silently.
- Deserialization error on any element → empty Vec, silently.
These helpers feed critical introspection handlers (trigger_info, registered_trigger_info, worker_info, function_list, etc.). Silent failures surface as "no functions / workers / triggers" with no error signal—defeating introspection's diagnostic purpose. Propagating deserialization errors preserves the IIIError channel and surfaces regressions immediately.
Minor: Lines 147–149 and 152–153 reference "SDK 0.11.3", but Cargo.toml specifies 0.11.6.
Proposed fix: surface deserialization errors instead of swallowing them
async fn engine_list_functions(iii: &III) -> Result<Vec<SdkFunctionInfo>, IIIError> {
let result = iii
.trigger(TriggerRequest {
function_id: "engine::functions::list".into(),
payload: serde_json::json!({}),
action: None,
timeout_ms: None,
})
.await?;
- Ok(result
- .get("functions")
- .and_then(|v| serde_json::from_value(v.clone()).ok())
- .unwrap_or_default())
+ match result.get("functions") {
+ Some(v) => serde_json::from_value(v.clone()).map_err(|e| {
+ IIIError::Handler(format!("engine::functions::list: invalid `functions` payload: {e}"))
+ }),
+ None => Ok(Vec::new()),
+ }
}Apply the same pattern to engine_list_workers, engine_list_triggers, and engine_list_trigger_types, swapping the field name and function id accordingly.
🤖 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/directory.rs` around lines 929 - 993, The helper
functions engine_list_functions, engine_list_workers, engine_list_triggers, and
engine_list_trigger_types are silently swallowing deserialization errors by
using .and_then(... .ok()).unwrap_or_default(), so change each to explicitly
propagate deserialization errors via Result mapping: fetch the response with
iii.trigger(...).await?, then get the expected field ("functions", "workers",
"triggers", "trigger_types") and convert it using serde_json::from_value(...)
returning Err(IIIError::from or map the serde error) instead of
.ok()/unwrap_or_default(); ensure the function signatures still return
Result<Vec<...>, IIIError> and map serde_json errors into that IIIError so
callers receive a failure rather than an empty Vec for malformed or incompatible
responses (apply the same pattern to engine_list_functions, engine_list_workers,
engine_list_triggers, engine_list_trigger_types).
Comment on lines
+411
to
+415
| let cache_key = format!( | ||
| "worker-list:{}:{}", | ||
| search.as_deref().unwrap_or(""), | ||
| cursor.as_deref().unwrap_or("") | ||
| ); |
There was a problem hiding this comment.
Use structured cache keys to avoid cross-request collisions.
These keys are delimiter-concatenated strings, so different input tuples can collide when values contain : / =. A collision can return the wrong cached payload for up to registry_cache_ttl_ms.
💡 Suggested fix
- let cache_key = format!(
- "worker-list:{}:{}",
- search.as_deref().unwrap_or(""),
- cursor.as_deref().unwrap_or("")
- );
+ let cache_key = format!(
+ "worker-list:{}",
+ serde_json::json!({
+ "search": search.as_deref().unwrap_or(""),
+ "cursor": cursor.as_deref().unwrap_or("")
+ })
+ );- let cache_key = format!(
- "worker-info:{name}:{}={}",
- spec.label(),
- spec.query_value()
- );
+ let cache_key = format!(
+ "worker-info:{}",
+ serde_json::json!({
+ "name": &name,
+ "label": spec.label(),
+ "query": spec.query_value()
+ })
+ );Also applies to: 465-469
🤖 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/registry.rs` around lines 411 - 415, The cache
key construction using string concatenation (the cache_key variable in
registry.rs and the similar construction around the second occurrence) can
collide when inputs contain ":" or "="; replace the ad-hoc concatenation with a
structured, unambiguous serializer—for example JSON-serialize or URL-encode the
key components or base64-encode a serialized tuple (e.g.,
serde_json::to_string(&(search, cursor, other_fields)) or percent-encode each
component) and use that serialized string as the cache key; update both the
cache_key creation sites (the format! call that builds "worker-list:...") and
the analogous key at the other location so keys are safe from delimiter
collisions.
4 tasks
This was referenced May 27, 2026
This was referenced Jul 3, 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
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.
typefield in skill responses.directory::skills::indexfunction to provide a concise overview of installed workers, improving agent bootstrapping efficiency.Summary by CodeRabbit
Release Notes
New Features
directory::skills::indexfunction for rendering token-light skill indicesImprovements