feat(iii-worker): replace static registry with per-worker API#1460
Merged
andersonleal merged 2 commits intomainfrom Apr 10, 2026
Merged
feat(iii-worker): replace static registry with per-worker API#1460andersonleal merged 2 commits intomainfrom
andersonleal merged 2 commits intomainfrom
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughReplaces registry/index resolution with a worker-info API, consolidates binary metadata into Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant API as Worker-Info API
participant Downloader
participant Verifier
participant FS as Filesystem
CLI->>API: fetch_worker_info(name, version?)
API-->>CLI: WorkerInfoResponse { Binary | Oci }
alt Binary response
CLI->>CLI: select platform key from response.binaries
CLI->>Downloader: GET binary_info.url
Downloader-->>CLI: binary_data
CLI->>Verifier: verify_sha256(binary_data, binary_info.sha256)
Verifier-->>CLI: ok / mismatch
alt ok
CLI->>FS: install/unpack binary -> PathBuf
FS-->>CLI: installed path
else mismatch
CLI-->>CLI: error (checksum mismatch)
end
else Oci response
CLI->>CLI: handle_oci_pull_and_add(image_url)
CLI->>Downloader: pull OCI image
Downloader-->>FS: store image / layer
FS-->>CLI: installed image
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Replace the GitHub-hosted static registry index (registry/index.json) with a per-worker API at https://api.workers.iii.dev/download/:worker-name. - Add fetch_worker_info(name, version) replacing fetch_registry() - Binary downloads use API-provided URLs + inline SHA256 checksums - Simplify download_and_install_binary() from 6 params to 2 - Checksums are now always verified (no more has_checksum flag) - OCI passthrough preserved for full refs with '/' or ':' - Env var: III_REGISTRY_URL → III_API_URL - Comprehensive test coverage: deserialization edge cases, error paths, platform selection, routing logic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/iii-worker/src/cli/registry.rs (1)
117-122: Consider URL encoding for the worker name.If a worker name contains characters that are valid per
validate_worker_name(alphanumeric, dash, underscore, dot) but have special meaning in URLs (like.), the current string interpolation should be safe. However, for defense-in-depth, consider using proper URL encoding for thenamesegment.♻️ Optional: URL-encode worker name
- let url = format!("{}/download/{}", base_or_file, name); + let url = format!( + "{}/download/{}", + base_or_file, + urlencoding::encode(name) + );This would require adding
urlencodingas a dependency, so it may not be worth it givenvalidate_worker_namealready restricts the character set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iii-worker/src/cli/registry.rs` around lines 117 - 122, The code builds the download URL by interpolating base_or_file and name into url without encoding; update the construction so the worker name is percent-encoded before interpolation (encode the variable name used to build url = format!("{}/download/{}", base_or_file, name)) to defend against characters with special URL meaning, e.g., use a percent-encoding utility (percent-encoding or urlencoding crate) to encode name, keep the query handling with HTTP_CLIENT.get(&url) and request.query(&[("version", v)]) unchanged, and add the chosen crate to Cargo.toml if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/iii-worker/src/cli/registry.rs`:
- Around line 117-122: The code builds the download URL by interpolating
base_or_file and name into url without encoding; update the construction so the
worker name is percent-encoded before interpolation (encode the variable name
used to build url = format!("{}/download/{}", base_or_file, name)) to defend
against characters with special URL meaning, e.g., use a percent-encoding
utility (percent-encoding or urlencoding crate) to encode name, keep the query
handling with HTTP_CLIENT.get(&url) and request.query(&[("version", v)])
unchanged, and add the chosen crate to Cargo.toml if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: df2b61bd-c93c-4bb0-9f0e-d37f86861105
📒 Files selected for processing (7)
crates/iii-worker/src/cli/binary_download.rscrates/iii-worker/src/cli/managed.rscrates/iii-worker/src/cli/registry.rscrates/iii-worker/src/main.rscrates/iii-worker/tests/binary_worker_integration.rscrates/iii-worker/tests/config_force_integration.rscrates/iii-worker/tests/config_managed_integration.rs
💤 Files with no reviewable changes (1)
- crates/iii-worker/src/main.rs
andersonleal
force-pushed
the
feat/worker-registry-api
branch
from
7c9eb1d to
c919737
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/iii-worker/src/cli/managed.rs (1)
178-243:⚠️ Potential issue | 🟠 MajorDerive the effective OCI worker name once and reuse it for
--force.The force branch keys running checks, artifact deletion, and
remove_worker()off the full image ref, but Lines 233-239 later normalize that same ref down to the last segment before adding it.iii worker add ghcr.io/org/image:tag --force --reset-configtherefore will not clear or stop the existing worker namedimage, and digest refs can end up with names likeimage@sha256.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/iii-worker/tests/config_managed_integration.rs`:
- Around line 173-193: The test handle_managed_add_oci_ref_passthrough currently
only asserts result != 0 which can false-pass; change the test to verify the
failure source by examining the error output instead of only the exit code:
invoke iii_worker::cli::managed::handle_managed_add (and the other similar test)
in a way that captures its stderr/returned error message (or adjust
handle_managed_add to return a Result with an error string if needed), then
assert the message contains pull/OCI-related text (e.g. "pull" or "OCI" or
"failed to pull") and does NOT mention API resolution (e.g. "API", "resolve", or
"registry resolution"); apply the same stricter assertion to the other test
mentioned (the one covering the non-passthrough case) so both tests check
failure source, not just non-zero exit.
- Around line 221-229: The test mutates the process-global III_API_URL without
synchronization, causing races; add a file-level static ENV_LOCK: Mutex<()>
(same pattern as in registry.rs) and acquire the lock before each env var
mutation block around calls to iii_worker::cli::managed::handle_managed_add (and
other tests in this file) so the unsafe std::env::set_var / remove_var pairs
execute while holding let _guard = ENV_LOCK.lock().unwrap(); to serialize
modifications and prevent cross-test contamination.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 89b5aca9-2c3e-43b6-bc45-92917f2b8e38
📒 Files selected for processing (7)
crates/iii-worker/src/cli/binary_download.rscrates/iii-worker/src/cli/managed.rscrates/iii-worker/src/cli/registry.rscrates/iii-worker/src/main.rscrates/iii-worker/tests/binary_worker_integration.rscrates/iii-worker/tests/config_force_integration.rscrates/iii-worker/tests/config_managed_integration.rs
💤 Files with no reviewable changes (1)
- crates/iii-worker/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/iii-worker/tests/binary_worker_integration.rs
sergiofilhowz
previously approved these changes
Apr 10, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/iii-worker/src/cli/managed.rs (2)
225-241: OCI reference name extraction may produce unexpected results for certain image formats.The name extraction logic uses
rsplit('/').next()thensplit(':').next(), which works for standard formats likeghcr.io/org/image:tag. However, for edge cases like:
image:tag(no slashes) → correctly extractsimageorg/image(no tag) → correctly extractsimage:tag(starts with colon, malformed) → extracts empty stringThe fallback to
image_or_namemitigates the empty-string case, but consider whether such malformed inputs should be validated earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iii-worker/src/cli/managed.rs` around lines 225 - 241, The name extraction for OCI references can yield an empty or invalid name (e.g., ":tag"); update the logic around image_or_name -> name in managed.rs to validate the extracted value before calling handle_oci_pull_and_add: after computing name from rsplit('/') and split(':'), trim it and if it is empty or contains invalid characters, emit a clear error/early return (or fallback to a safer validation path) instead of passing an empty name into handle_oci_pull_and_add; ensure any logging (e.g., the resolving messages) reflects the validation failure so malformed inputs are rejected or handled deterministically.
90-96: Consider handling potential serialization failure more gracefully.The
unwrap_or_default()silently swallows YAML serialization errors. While rare, ifresponse.config.configcontains values that can't be serialized to YAML, this would produce an empty config without any indication of the problem.💡 Proposed improvement
let config_yaml = response .config .config .as_object() - .map(|_| serde_yaml::to_string(&response.config.config).unwrap_or_default()); + .and_then(|_| { + serde_yaml::to_string(&response.config.config) + .map_err(|e| tracing::warn!("Failed to serialize worker config: {}", e)) + .ok() + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/iii-worker/src/cli/managed.rs` around lines 90 - 96, The current code silently swallows YAML serialization errors by using serde_yaml::to_string(...).unwrap_or_default() when building config_yaml from response.config.config; change this to explicitly handle serialization failures: call serde_yaml::to_string on the actual object (not using unwrap_or_default), match on the Result and on Err either return or propagate an error from this function (or log and skip append) so you don't pass an empty config to super::config_file::append_worker; ensure config_yaml (used in the call to super::config_file::append_worker(worker_name, config_yaml.as_deref())) is an Option<String> only when serialization succeeded and use early return or Result propagation to surface the serialization error instead of hiding it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/iii-worker/src/cli/managed.rs`:
- Around line 225-241: The name extraction for OCI references can yield an empty
or invalid name (e.g., ":tag"); update the logic around image_or_name -> name in
managed.rs to validate the extracted value before calling
handle_oci_pull_and_add: after computing name from rsplit('/') and split(':'),
trim it and if it is empty or contains invalid characters, emit a clear
error/early return (or fallback to a safer validation path) instead of passing
an empty name into handle_oci_pull_and_add; ensure any logging (e.g., the
resolving messages) reflects the validation failure so malformed inputs are
rejected or handled deterministically.
- Around line 90-96: The current code silently swallows YAML serialization
errors by using serde_yaml::to_string(...).unwrap_or_default() when building
config_yaml from response.config.config; change this to explicitly handle
serialization failures: call serde_yaml::to_string on the actual object (not
using unwrap_or_default), match on the Result and on Err either return or
propagate an error from this function (or log and skip append) so you don't pass
an empty config to super::config_file::append_worker; ensure config_yaml (used
in the call to super::config_file::append_worker(worker_name,
config_yaml.as_deref())) is an Option<String> only when serialization succeeded
and use early return or Result propagation to surface the serialization error
instead of hiding it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f4b00ce3-01f3-4b96-87c5-f7c0286a8a09
📒 Files selected for processing (5)
crates/iii-worker/src/cli/managed.rscrates/iii-worker/src/cli/registry.rscrates/iii-worker/tests/binary_worker_integration.rscrates/iii-worker/tests/config_force_integration.rscrates/iii-worker/tests/config_managed_integration.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/iii-worker/tests/config_managed_integration.rs
guibeira
approved these changes
Apr 10, 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
3 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.
Summary
registry/index.json) with a per-worker API athttps://api.workers.iii.dev/download/:worker-name?version=<version>fetch_worker_info(name, version)replacesfetch_registry()+resolve_image().sha256file fetchesdownload_and_install_binary()from 6 parameters to 2 (worker_name,&BinaryInfo)has_checksumflag that defaulted tofalse)III_REGISTRY_URL→III_API_URLAPI contract
Binary worker:
GET /download/image-resize?version=0.1.2returns per-platform URLs + sha256 checksumsOCI worker:
GET /download/todo-workerreturnsimage_urlfor direct pullPassthrough: Full OCI refs (
ghcr.io/org/image:tag) bypass the API entirelyTest plan
cargo test -p iii-workerpasses (426 tests)cargo clippy -p iii-workerhas no new warningsiii worker add image-resizeresolves via new API and downloads binaryiii worker add image-resize@0.1.2resolves specific versioniii worker add ghcr.io/org/image:tagstill works as OCI passthroughiii worker add ./local-pathstill works for local workersiii worker add w1 w2 w3multi-add works (sequential API calls)III_API_URL=file:///path/to/fixture.json iii worker add test-workerworks in debug buildsSummary by CodeRabbit
Refactor
Bug Fixes
Tests