feat(iii-directory): migrate runtime config to the configuration worker#267
feat(iii-directory): migrate runtime config to the configuration worker#267andersonleal wants to merge 1 commit into
Conversation
iii-directory was the last common Rust worker still loading its runtime config from a local config.yaml parsed once at boot. Move it to the engine-built-in `configuration` worker (id `iii-directory`), mirroring the `database`/`storage` pattern: register a JSON Schema + seed at boot, read the authoritative env-expanded value via `configuration::get`, and bind a `configuration:updated` trigger for hot reload. Full hot-reload parity with `database`: - Tunable fields apply live: registry_url, download_timeout_ms, registry_cache_ttl_ms, filter_unregistered. - Topology changes are refused with a "restart required" log: skills_folder, local_skills_folder, auto_download. Implementation: - New src/configuration.rs (port of database/src/configuration.rs) with a SharedState bundling the live config, shared cache TTL, both caches, and the boot topology. on_config_change re-fetches the authoritative value (ignores the trigger payload), refuses topology changes, else swaps config + updates TTL + clears caches. - SkillsConfig: derive JsonSchema; add json_schema/from_json/to_json/from_file/ topology/into_shared and the SharedConfig alias (Arc<ArcSwap<SkillsConfig>>). - Handlers snapshot the live config per call via load_full(); RegistryCache and RegisteredWorkersCache read a shared Arc<AtomicU64> TTL. - main.rs: --config is now an optional seed; connect -> register_config -> fetch_config -> build state/caches -> register fns -> register_config_trigger. - config.yaml -> config.yaml.example (seed); README Configure section + manifest default_config updated. Verified: cargo build, clippy --all-targets, fmt --check, 249 lib tests, and all test targets compile + link. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 19 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthrough
ChangesHot-reloadable configuration system for iii-directory
Sequence Diagram(s)sequenceDiagram
participant CLI as iii-directory CLI
participant Main as main.rs
participant ConfMod as configuration.rs
participant ConfWorker as configuration worker
participant SharedState as SharedState / ArcSwap
CLI->>Main: startup (--config optional seed path)
Main->>ConfWorker: connect to engine
Main->>ConfMod: register_config(iii, seed?)
ConfMod->>ConfWorker: trigger_with_retry (schema + seed value)
Main->>ConfMod: fetch_config(iii)
ConfMod->>ConfWorker: configuration::get (env-expanded)
ConfWorker-->>ConfMod: stored SkillsConfig JSON
ConfMod-->>Main: SkillsConfig
Main->>SharedState: SharedState::new(config, ttl_ms, caches, boot_topology)
Main->>ConfMod: register_config_trigger(iii, state)
note over ConfMod: registers directory::on-config-change → configuration:updated
rect rgba(100, 149, 237, 0.5)
note over ConfWorker, SharedState: Hot-reload path (at runtime)
ConfWorker-->>ConfMod: configuration:updated trigger fires
ConfMod->>ConfWorker: fetch_config (re-fetch authoritative)
ConfWorker-->>ConfMod: new SkillsConfig
ConfMod->>ConfMod: compare Topology vs boot_topology
alt topology changed
ConfMod-->>ConfMod: log restart required, retain prior config
else tunables only
ConfMod->>SharedState: apply_config (ArcSwap store, update AtomicU64 TTL, clear caches)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
iii-directory/src/manifest.rs (1)
41-62: ⚡ Quick winExtend test to assert on the newly added fields.
The test currently verifies only four of the seven fields in
default_config. Adding assertions forlocal_skills_folder,filter_unregistered, andauto_downloadwould provide parity with the README documentation and catch future regressions if any of these fields are accidentally removed or misconfigured.✅ Suggested test additions
assert_eq!(parsed["default_config"]["download_timeout_ms"], 60_000); assert_eq!(parsed["default_config"]["registry_cache_ttl_ms"], 60_000); + assert_eq!( + parsed["default_config"]["local_skills_folder"], + DEFAULT_LOCAL_SKILLS_FOLDER + ); + assert_eq!(parsed["default_config"]["filter_unregistered"], true); + assert_eq!(parsed["default_config"]["auto_download"], true); assert!(!parsed["supported_targets"].as_array().unwrap().is_empty());🤖 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 41 - 62, The test function json_roundtrip_has_required_fields is incomplete as it only asserts on four of the seven fields in default_config. Add three additional assert statements after the existing default_config assertions to verify the values of the newly added fields: local_skills_folder, filter_unregistered, and auto_download. These assertions should validate that these fields are present in the parsed JSON and contain the expected values as documented in the README, ensuring parity with documentation and catching potential regressions if these fields are accidentally removed or misconfigured.
🤖 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 `@iii-directory/src/manifest.rs`:
- Around line 41-62: The test function json_roundtrip_has_required_fields is
incomplete as it only asserts on four of the seven fields in default_config. Add
three additional assert statements after the existing default_config assertions
to verify the values of the newly added fields: local_skills_folder,
filter_unregistered, and auto_download. These assertions should validate that
these fields are present in the parsed JSON and contain the expected values as
documented in the README, ensuring parity with documentation and catching
potential regressions if these fields are accidentally removed or misconfigured.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f887e71-e40b-4a9a-ae73-4e32fa6408f4
⛔ Files ignored due to path filters (1)
iii-directory/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
iii-directory/Cargo.tomliii-directory/README.mdiii-directory/config.yamliii-directory/config.yaml.exampleiii-directory/src/config.rsiii-directory/src/configuration.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/lib.rsiii-directory/src/main.rsiii-directory/src/manifest.rsiii-directory/tests/common/workers.rs
💤 Files with no reviewable changes (1)
- iii-directory/config.yaml
Summary
iii-directorywas the last common Rust worker still loading its runtime config from a localconfig.yamlparsed once at boot. This migrates it to the engine-built-inconfigurationworker (entry idiii-directory), following thedatabase/storagepattern: register a JSON Schema + seed at boot, read the authoritative (env-expanded) value viaconfiguration::get, and bind aconfiguration:updatedtrigger for hot reload.The change uses full hot-reload parity with
database:registry_url,download_timeout_ms,registry_cache_ttl_ms,filter_unregisteredskills_folder,local_skills_folder,auto_download— these define on-disk roots and boot-time task wiring.What changed
src/configuration.rs(new) — port ofdatabase/src/configuration.rs.CONFIG_ID = "iii-directory",CONFIG_FN_ID = "directory::on-config-change". ASharedStatebundles the live config handle, shared cache-TTL cell, both caches, and the boot topology.on_config_changere-fetches the authoritative value (ignores the trigger payload — same security rationale asdatabase), refuses topology changes, otherwise swaps the live config + updates TTL + clears both caches.src/config.rs—SkillsConfigderivesJsonSchema; addsjson_schema/from_json/to_json/from_yaml/from_file/topology/into_sharedand theSharedConfigalias (Arc<ArcSwap<SkillsConfig>>).functions/{mod,skills,registry,prompts,download}.rssnapshots the live config per call viaload_full()(lock-free).RegistryCacheandRegisteredWorkersCacheread a sharedArc<AtomicU64>TTL soregistry_cache_ttl_msalso hot-reloads.src/main.rs—--configis now an optional seed; boot reorders to connect →register_config(seed)→fetch_config→ build state/caches → register functions →register_config_trigger.config.yaml→config.yaml.example(seed header), README "Configuration" section rewritten (tunable vs topology),manifestdefault_configcompleted.arc-swapdependency (lock-free per-call reads; one-line change at each handler site).Verification
cargo build,cargo clippy --all-targets(clean),cargo fmt --check(clean)cargo test --lib— 249 passedcargo test --no-run— all test targets compile + linkBDD/e2e suites require a live engine so they were not run here; the e2e harness is engine-config-driven and stays compatible (the engine delivers the inlined worker config as the
--configseed).Notes
iii.trigger(noiii.worker.yamldeps entry), matchingdatabase.Cargo.lockversion bumps in other workers were intentionally left out of this branch.Summary by CodeRabbit
New Features
--configflag is now optional; configuration can be provided via seed file or stored valuesDocumentation