refactor(hotpath): swap cache route back onto WorkersCachekitIO (LAB-1492) - #12
Conversation
…1492) cachekit-rs 0.7.0 fixes the wasm32 SystemTime::now() panic (LAB-1079), so the direct worker::Fetch workaround is deleted and the cache route calls WorkersCachekitIO::get. The SDK sends X-CacheKit-L1-Status: disabled itself (metrics_headers with no provider), so no header behaviour is lost. cachekit-core aligns to 0.4 to keep a single copy in the wasm binary; urlencoding drops as a direct dep (workaround-only). Backend errors echo the BackendError kind only — the SDK message can embed backend response bodies, which are not ours to relay.
This comment has been minimized.
This comment has been minimized.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe hotpath upgrades ChangesWorkersCachekitIO integration
Sequence Diagram(s)sequenceDiagram
participant cache_handler
participant WorkersCachekitIO
participant Cachekit backend
cache_handler->>WorkersCachekitIO: Build with API key and API URL
WorkersCachekitIO->>Cachekit backend: Send cache request
Cachekit backend-->>WorkersCachekitIO: Return cache response or SDK error
WorkersCachekitIO-->>cache_handler: Return cache result or error class
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/architecture.md`:
- Line 12: Update the current-version references for the edge hot path in the
architecture documentation: change item 3’s build guidance from cachekit-rs
0.5.0 to 0.7.0, and mark the live verification at lines 63-65 as historical or
update it to verify 0.7.0.
In `@hotpath/README.md`:
- Line 16: Update the cache-read documentation in the table to include all
failure statuses from the handler: 500 when WorkersCachekitIO::build() fails,
502 when backend.get() fails, and 503 when CACHEKIT_API_KEY is missing. Replace
the claim that 503 is the only failure status while preserving the existing
successful-flow description.
🪄 Autofix
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 Plus
Run ID: d5c75cc5-aaa3-4d27-8bd4-bd1fe68e34e3
⛔ Files ignored due to path filters (1)
hotpath/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
docs/architecture.mdhotpath/Cargo.tomlhotpath/README.mdhotpath/src/lib.rshotpath/wrangler.toml
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
…ons (panel findings) Expert-panel MAJ items: a 502 previously carried zero server-side diagnostic (kind only, message dropped entirely) — full BackendError now goes to Worker logs via console_error! while the client response keeps the kind-only discipline. docs/architecture.md open item 3 claimed hotpath builds against 0.5.0 and that crates.io carries 0.8.0 (a version that never existed; real latest is 0.7.0).
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hotpath/src/lib.rs (1)
163-170: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReturn a sanitised builder error.
CachekitErrorexposes configuration reasons but has no public error kind. Log{e}withconsole_error!, then return a fixed generic 500 message. The error does not include the configured endpoint.🤖 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 `@hotpath/src/lib.rs` around lines 163 - 170, Update the WorkersCachekitIO builder error branch in the backend initialization flow to log the detailed error with console_error!, then return a fixed generic 500 response message instead of exposing {e} through json_error. Keep the successful backend path unchanged and ensure the returned message does not include the configured endpoint.
🤖 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.
Outside diff comments:
In `@hotpath/src/lib.rs`:
- Around line 163-170: Update the WorkersCachekitIO builder error branch in the
backend initialization flow to log the detailed error with console_error!, then
return a fixed generic 500 response message instead of exposing {e} through
json_error. Keep the successful backend path unchanged and ensure the returned
message does not include the configured endpoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 73d107f9-4676-4729-982e-1093a400aad7
📒 Files selected for processing (2)
docs/architecture.mdhotpath/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/architecture.md
Log the full builder error to Worker logs; the 500 response is now a fixed string. Today's CachekitError::Config messages are static literals, but the SDK's wording (which could name the endpoint) is not part of our client contract — same log-full/return-sanitized pattern as the 502 arm.
This comment has been minimized.
This comment has been minimized.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
@coderabbitai review |
|
a5c8af0
|
Resolved merge conflict in |
|
@27Bslash6 — ready for your signoff / merge. Automated review is clean on head Crypto/protocol gate re-verified independently, since the diff bumps the crates implementing interop key derivation and the wire checksum:
Non-blocking: the three checksum tests in Not merging — that is yours. |
1 participant
Closes LAB-1492. Consumer half of LAB-1079: cachekit-rs 0.7.0 (crates.io) fixes the wasm32
SystemTime::now()panic insession_headers(), so the interim directworker::FetchGET is deleted andGET /v1/cache/:operation/:windowroutes throughWorkersCachekitIO.Changes
hotpath/Cargo.toml:cachekit-rs0.5.0 → 0.7.0 (both native and wasm32 target deps; the stagedworkers/cachekitiofeature block is now the live path).cachekit-core0.3 → 0.4 to match cachekit-rs 0.7.0's pin (one copy in the wasm binary).urlencodingdirect dep removed — it existed only for the workaround fetch.hotpath/src/lib.rs: workaround block deleted (including the stale "≤ 0.8.0" comment — real affected range was 0.2.0–0.6.0). Cache route buildsWorkersCachekitIOwith.api_url(env)+.allow_custom_host(true)(dev instance is outside the SDK SSRF allowlist; HTTPS + private-IP checks still apply). Backend errors echo theBackendErrorkind only — the SDK message can embed backend response body content, preserving the old code's "never echo the body" property. The SDK emitsX-CacheKit-L1-Status: disableditself (metrics_headerswith no provider), so nothing the workaround sent by hand is lost.hotpath/README.md,hotpath/wrangler.toml,docs/architecture.md(version table, credentials section, build-chain pins — also fixed the staleencryption,macrosfeatures line).workerstays pinned at 0.4 in 0.7.0, so build-chain pins are unchanged.Verification
cargo test --locked13 passed;cargo clippy --all-targets --locked -- -D warningsclean.cargo clippy --target wasm32-unknown-unknown --lockedclean;worker-build --releasesucceeds.sdkreportscachekit-rs 0.7.0):GET https://skyline-hotpath.raywalker.workers.dev/v1/cache/posts_per_minute/5m→ HTTP 200 with fresh backend data, no CF error 1101.Supply-chain audit (Kody Rule 81)
cargo audit(RustSec advisory-db, 1190 advisories, 2026-08-07) against the committedhotpath/Cargo.lock(200 locked deps, includingcachekit-rs 0.7.0andcachekit-core 0.4.0): 0 vulnerabilities, 0 warnings. Both bumped crates are first-party (cachekit-io org), published via the repo's release-please train; the lockfile is committed and fully pinned.