Repository navigation
refactor(task): extract versioned cache store - #11620
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Greptile SummaryThe PR extracts local task-cache persistence behind a versioned
Confidence Score: 5/5The PR appears safe to merge. The previously reported release-build gap is fixed by an unconditional runtime version comparison in the sole Important Files Changed
Reviews (9): Last reviewed commit: "refactor(task): extract versioned cache ..." | Re-trigger Greptile |
a59d790 to
15f114f
Compare
15f114f to
c58af1d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c58af1d. Configure here.
c58af1d to
8dc4b26
Compare
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
8dc4b26 to
05988fc
Compare
05988fc to
8dc4b26
Compare
8dc4b26 to
f39fb79
Compare
f39fb79 to
70e41fd
Compare
70e41fd to
c1f8883
Compare
## Summary
- define a versioned `TaskCacheStore` contract independent of the
artifact manifest format
- move atomic filesystem lookup, staged publish, removal, and
access-time updates into the local store implementation
- keep artifacts file-backed so future stores can transfer large
payloads without buffering them in memory
- mark the cache-store interface item complete in the parity tracker
## Validation
- `cargo test --all-features task_cache_store::tests -- --nocapture`
- task artifact cache E2E suite, including concurrency, corruption
recovery, result-only entries, portability, limits, modes, and
inspection
- `mise run lint-fix`
- `cargo clippy --workspace --all-features --all-targets -- -D warnings`
*AI-assisted — Tool: Codex; model: unavailable/unavailable; version:
unavailable.*
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches core cache read/write/restore paths; behavior should match the
prior inline FS logic, but regressions in concurrency, corruption
handling, or result-only entries would affect task execution
correctness.
>
> **Overview**
> Introduces a **versioned `TaskCacheStore` trait** (`get`, staged
`begin_write`/`commit`, `remove`, `touch`) separate from the artifact
manifest format, with **`LocalTaskCacheStore`** as the filesystem
implementation (partial writes, atomic renames, access-time updates).
>
> **`TaskArtifactCache`** now holds `Arc<dyn TaskCacheStore>` and routes
restore, store, hit checks, and corrupt-entry cleanup through the store
instead of hard-coded `{key}.tar.zst` / `{key}.json` paths. Artifacts
stay **file-backed** on disk for streaming-friendly future backends.
>
> Checksum verification accepts an optional archive path; a unit test
covers verifying when the manifest already embeds the expected checksum.
**`TASK_CACHE_PARITY.md`** marks the cache-store interface item done.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c1f8883. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Summary
TaskCacheStorecontract independent of the artifact manifest formatValidation
cargo test --all-features task_cache_store::tests -- --nocapturemise run lint-fixcargo clippy --workspace --all-features --all-targets -- -D warningsAI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Medium Risk
Touches core cache read/write/restore paths; behavior should match the prior inline FS logic, but regressions in concurrency, corruption handling, or result-only entries would affect task execution correctness.
Overview
Introduces a versioned
TaskCacheStoretrait (get, stagedbegin_write/commit,remove,touch) separate from the artifact manifest format, withLocalTaskCacheStoreas the filesystem implementation (partial writes, atomic renames, access-time updates).TaskArtifactCachenow holdsArc<dyn TaskCacheStore>and routes restore, store, hit checks, and corrupt-entry cleanup through the store instead of hard-coded{key}.tar.zst/{key}.jsonpaths. Artifacts stay file-backed on disk for streaming-friendly future backends.Checksum verification accepts an optional archive path; a unit test covers verifying when the manifest already embeds the expected checksum.
TASK_CACHE_PARITY.mdmarks the cache-store interface item done.Reviewed by Cursor Bugbot for commit c1f8883. Bugbot is set up for automated code reviews on this repo. Configure here.