Repository navigation
refactor(cache): generalize remote action protocol - #11763
Conversation
jdx
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThe PR generalizes the experimental task-cache contract into a typed action/CAS protocol and introduces nested remote-cache policy plus task-scoped Rust cache configuration.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains within the eligible follow-up review scope. No blocking failure remains. Important Files Changed
Reviews (15): Last reviewed commit: "refactor(cache): generalize remote actio..." | Re-trigger Greptile |
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.
|
|
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:
📝 WalkthroughWalkthroughThe remote cache protocol now supports typed build actions and version 2 endpoints. Remote cache settings move into nested ChangesRemote build cache
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
efaa429 to
e3a1c18
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/tasks/remote-cache-protocol.md (1)
78-96: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine the capability for supported action kinds.
The protocol allows compiler and build-system kinds only when advertised. The capabilities response defines no field for supported action kinds, action schema versions, or kind-specific metadata versions. Add a normative capability field before clients send non-
taskactions.Also applies to: 113-143
🤖 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 `@docs/tasks/remote-cache-protocol.md` around lines 78 - 96, Extend the capabilities response specification around GET /v2/capabilities to include a normative field advertising supported action kinds, including compiler and build-system kinds, along with their action schema and kind-specific metadata versions. Define how clients must use this field before sending any non-task action, and update the related capability examples and requirements consistently.
🤖 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/tasks/remote-cache-protocol.md`:
- Line 210: Update commit validation to compare the action descriptor’s kind
with the client metadata kind, and reject the commit before publication when
they differ. Preserve the existing schema and reference-graph validation for
matching kinds, covering the related validation paths identified in the diff.
- Around line 68-74: Align the protocol documentation with the task client’s
action-result URL construction: either update the task cache flow around the
action-result URL builder to negotiate and pass the selected digest algorithm,
or explicitly require BLAKE3 for action-result keys and scope the SHA-256
allowance to object types that support it. Ensure the documented negotiation
rules match the implemented behavior.
In `@docs/tasks/task-configuration.md`:
- Line 711: Update the documentation text near “bearer token” to hyphenate the
compound modifier as “Kubernetes-projected service account tokens,” without
changing the surrounding meaning.
---
Outside diff comments:
In `@docs/tasks/remote-cache-protocol.md`:
- Around line 78-96: Extend the capabilities response specification around GET
/v2/capabilities to include a normative field advertising supported action
kinds, including compiler and build-system kinds, along with their action schema
and kind-specific metadata versions. Define how clients must use this field
before sending any non-task action, and update the related capability examples
and requirements consistently.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d3e76f1-2639-4adf-984c-326c0cb826ef
📒 Files selected for processing (10)
docs/public/llms.txtdocs/tasks/remote-cache-protocol.mddocs/tasks/task-configuration.mde2e/tasks/test_task_artifact_cache_remote_modesschema/mise.jsonsettings.tomlsrc/cache.rssrc/task/mod.rssrc/task/task_cache.rssrc/task/task_cache_store.rs
💤 Files with no reviewable changes (1)
- src/task/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- schema/mise.json
- e2e/tasks/test_task_artifact_cache_remote_modes
- src/task/task_cache.rs
e3a1c18 to
7bf823c
Compare
7bf823c to
4e7d039
Compare
4e7d039 to
05ec75c
Compare
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)
build.rs (1)
617-724: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent generated struct-name collisions.
settings_struct_nameremoves component boundaries. Valid paths such asa.b_canda_b.cboth generateSettingsABC. The generated file then contains duplicate struct definitions and the build fails.Encode each path component with an unambiguous boundary. Add a regression test with colliding PascalCase paths.
🤖 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 `@build.rs` around lines 617 - 724, Update settings_struct_name to preserve unambiguous boundaries between path components so distinct paths such as a.b_c and a_b.c generate different struct names; keep the naming consistent for both props_to_code and emit_nested_settings. Add a regression test covering colliding PascalCase paths and verify the generated output contains unique struct definitions.
🤖 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/tasks/remote-cache-protocol.md`:
- Around line 399-403: Update scripts/task-cache-remote-compat.sh to test
version 2 conformance rather than version 1: use the /v2 status, capabilities,
blob, and action-result endpoints; generate and validate BLAKE3 action-key
hashes; test direct action results; and add kind validation coverage while
preserving the existing compatibility checks.
In `@src/task/task_cache.rs`:
- Around line 140-142: Update TaskArtifactCache::store() and its callers so
observed action digests are collected during the task run and passed into
CacheManifest.action_keys instead of always constructing an empty Vec. Preserve
the existing serde default while ensuring cache_actions-enabled runs write the
collected keys for remote prefetch.
---
Outside diff comments:
In `@build.rs`:
- Around line 617-724: Update settings_struct_name to preserve unambiguous
boundaries between path components so distinct paths such as a.b_c and a_b.c
generate different struct names; keep the naming consistent for both
props_to_code and emit_nested_settings. Add a regression test covering colliding
PascalCase paths and verify the generated output contains unique struct
definitions.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e4b92324-49e7-4376-a7a4-a1e23d1e78ba
📒 Files selected for processing (17)
build.rsdocs/tasks/remote-cache-protocol.mddocs/tasks/task-configuration.mde2e/tasks/test_task_artifact_cache_remote_modesschema/mise-settings.jsonschema/mise-task.jsonschema/mise.jsonsettings.tomlsrc/cache.rssrc/config/config_file/mod.rssrc/config/mod.rssrc/config/settings.rssrc/task/mod.rssrc/task/task_cache.rssrc/task/task_cache_store.rssrc/task/task_template.rsxtasks/render/schema.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/tasks/test_task_artifact_cache_remote_modes
- src/config/settings.rs
| The repository's compatibility suite is the executable definition of required version 2 behavior. | ||
| It must cover capability negotiation, canonical object validation, namespace isolation, independent | ||
| read/write authorization, missing-blob batches, streamed and resumable transfers, digest rejection, | ||
| atomic action-result commits, immutable conflicts, delegated-transfer credential isolation, | ||
| signatures, corruption handling, and retry semantics. | ||
| corruption handling, and retry semantics. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
while IFS= read -r file; do
echo "== $file =="
rg -n '/v1/|protocol\.major == 1|algorithm:"sha256"|signatures|\.result' "$file"
done < <(fd -t f 'task-cache-remote-compat\.sh$' .)Repository: jdx/mise
Length of output: 198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)docs/tasks/remote-cache-protocol\.md$|(^|/)scripts/task-cache-remote-compat\.sh$' || true
echo "== doc lines 360-420 =="
file="$(git ls-files | rg '(^|/)docs/tasks/remote-cache-protocol\.md$' | head -n1)"
sed -n '360,420p' "$file"
echo "== script outline/size =="
ws="$(git ls-files | rg '(^|/)scripts/task-cache-remote-compat\.sh$' | head -n1)"
[ -n "$ws" ] && { wc -l "$ws"; sed -n '60,175p' "$ws"; }
echo "== relevant v1/v2/version/key mentions across repo =="
rg -n 'protocol\.major|/v[0-9]|/v2/|/v1/|BLAKE|blake3|repository compatibility|remote-cache-protocol|task-cache-remote-compat|action-key|action_kind|result_envelope|Protocol Version 2' .Repository: jdx/mise
Length of output: 50365
Update the repository compatibility suite before declaring it as version 2 conformance.
docs/tasks/remote-cache-protocol.md:399 says the repository compatibility suite defines required version 2 behavior, but scripts/task-cache-remote-compat.sh only tests protocol major version 1, uses /v1 status/capabilities/blob/action-result endpoints, and expects SHA-256 objects. Add or update the suite to cover the version 2 path, including BLAKE3 action-key hashes, direct action results, /v2 endpoints, and kind validation.
🤖 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 `@docs/tasks/remote-cache-protocol.md` around lines 399 - 403, Update
scripts/task-cache-remote-compat.sh to test version 2 conformance rather than
version 1: use the /v2 status, capabilities, blob, and action-result endpoints;
generate and validate BLAKE3 action-key hashes; test direct action results; and
add kind validation coverage while preserving the existing compatibility checks.
| /// Action keys observed while this task ran, used to predict the next session's prefetch set. | ||
| #[serde(default)] | ||
| pub(crate) action_keys: Vec<CacheDigest>, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search the repository for any writer of CacheManifest.action_keys other than the
# hardcoded Vec::new() in task_cache.rs, and any TaskArtifactCache::store() caller that could supply
# observed action-cache digests.
echo "--- occurrences of action_keys ---"
rg -n 'action_keys' --type=rust
echo "--- store() definition and callers ---"
ast-grep run --pattern 'async fn store($$$) { $$$ }' --lang rust src/task/task_cache.rs
rg -n '\.store\(' --type=rust -C3
echo "--- any rustc/go compiler wrapper / interception code ---"
rg -n -i 'rustc[_-]?wrapper|RUSTC_WRAPPER|compiler.?action|action.?adapter' --type=rustRepository: jdx/mise
Length of output: 181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "git status:"
git status --short
echo "--- list task files ---"
git ls-files | grep -E '(^|/)(task|cache).*\.(rs|toml|md|json)$|src/task|docs/tasks' | head -200
echo "--- locate task_cache.rs and task cache files ---"
fd -a 'task_cache\.rs|task.*cache|cache.*task' . | sed 's#^\./##' | head -100
echo "--- search action_keys in entire repo ---"
rg -n -i 'action_keys|actions.*cache|cache.*actions|TaskCacheKey|CacheManifest|CacheDigest|TaskArtifactCache|TaskCache' . --glob '!target/**' --glob '!node_modules/**' | head -300 || true
echo "--- broad store call context ---"
rg -n '\.?store\(' --type rust -C2 src || true
echo "--- broad rustc / go / compiler-related context ---"
rg -n -i 'RUSTC|rustc|compiler.?wrapper|action.?adapter|cache_actions|prefs|prefetch|manifest' . --glob '!target/**' --glob '!node_modules/**' | head -400 || trueRepository: jdx/mise
Length of output: 50364
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- task executor cache store call ---"
sed -n '820,875p' src/task/task_executor.rs
echo "--- task_cache.store definition and manifest construction ---"
sed -n '635,692p' src/task/task_cache.rs
echo "--- CacheManifest definition ---"
sed -n '120,150p' src/task/task_cache.rs
echo "--- cache_actions handling definitions ---"
sed -n '926,956p' docs/tasks/task-configuration.md
sed -n '1310,1365p' src/task/task_cache.rs
sed -n '204,216p' src/task/task_cache.rs
sed -n '2735,2750p' src/config/mod.rs
echo "--- exact Rust action_keys and store call count/context ---"
python3 - <<'PY'
from pathlib import Path
for p in Path('src').rglob('*.rs'):
text=p.read_text(errors='replace').splitlines()
for i,l in enumerate(text,1):
if 'action_keys' in l or re.search(r'\bstore\s*\(', l):
if 'store' in l.lower() or 'action_keys' in l:
print(f"{p}:{i}:{l.rstrip()}")
PYRepository: jdx/mise
Length of output: 9945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- remaining task_cache action/cache keys references ---"
rg -n 'action|cache_actions|RUSTC|compiler' src task docs/tasks/task-configuration.md schema/mise-task.json --glob '!target/**' | head -300
echo "--- cache_actions enum and docs around remote session lifecycle ---"
sed -n '40,72p' src/task/task_cache.rs
sed -n '1248,1260p' docs/tasks/task-configuration.md
sed -n '220,245p' docs/tasks/remote-cache-protocol.md
echo "--- cache_actions references with context ---"
rg -n -C3 'cache_actions' src schema/mise-task.json docs/tasks/task-configuration.md docs/tasks/remote-cache-protocol.md | head -260Repository: jdx/mise
Length of output: 22527
Wire action_keys populated data before writing manifests.
CacheManifest.action_keys documents observed action digests for prefetch, but TaskArtifactCache::store() always builds action_keys: Vec::new() and no caller passes observed digests. As written, runs with cache_actions enabled produce manifests with no previous-action keys for remote prefetch to use; update the manifest construction to populate observed action keys, or change the docs/API until that wiring exists.
🤖 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 `@src/task/task_cache.rs` around lines 140 - 142, Update
TaskArtifactCache::store() and its callers so observed action digests are
collected during the task run and passed into CacheManifest.action_keys instead
of always constructing an empty Vec. Preserve the existing serde default while
ensuring cache_actions-enabled runs write the collected keys for remote
prefetch.
1eee9d1 to
c097811
Compare
c097811 to
c828af6
Compare
| } | ||
| let metadata = envelope | ||
| .result | ||
| let metadata = result |
There was a problem hiding this comment.
Legacy envelope breaks remote reads
Medium Severity
Remote cache GET handling now expects a flat action-result JSON body, but previously committed results used an envelope with a nested result (and optional signatures). Responses in the old shape no longer deserialize into the new type, so lookups can error instead of falling back to a cache miss when the server has not been upgraded in lockstep.
Reviewed by Cursor Bugbot for commit c828af6. Configure here.
There was a problem hiding this comment.
This compatibility case does not apply here. The envelope shape existed only in this still-unmerged experimental stack, and there are no supported deployments that can have persisted it. The flat action-result body deliberately replaces that intermediate shape before the protocol ships, so accepting both would add permanent compatibility code for data that cannot exist in a released cache. The protocol therefore remains v1.
AI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable.
4bf2396 to
59e57b3
Compare
59e57b3 to
db96bd5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit db96bd5. Configure here.
db96bd5 to
8a025ed
Compare
Summary
rust_cache = trueconfiguration, accepting both booleans and option tables so Rust-specific settings can grow without another migration[task_config]defaults, cascading, templates, and explicitrust_cache = falseoverrides[settings.task.cache]and theMISE_TASK_CACHE_REMOTE_*environment variablesValidation
cargo test --quiet task_rust_cachecargo test --quiet task_config_rust_cachecargo test --quiet test_parses_all_fieldscargo test --quiet task_cache_storemise run docs:buildmise run formatAI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable.
Note
Medium Risk
Touches experimental remote cache wire format, settings migration/aliases, and build-time settings codegen; misconfiguration or server mismatch could break CI caching, but behavior is largely gated behind experimental flags and compatibility shims.
Overview
This PR reframes the experimental remote cache as a shared build/action-cache protocol (still v1) and wires mise’s client, settings, schemas, and docs to match.
Remote cache & settings: Remote policy moves from flat
task.cache_remote_*keys to[settings.task.cache](sameMISE_TASK_CACHE_REMOTE_*env vars). Old names remain as hidden deprecated aliases that map into the nested struct when the new key is unset. Codegen inbuild.rsnow emits arbitrarily nested settings structs, metadata paths liketask.cache.remote_url, and docs/schema trees;mise settings lsJSON/TOML output nests by path. SharedCacheRemoteModeandCacheDigestlive insrc/cache.rs; the HTTP client drops the signed action-result envelope, uses lowercasemise-cache-*headers, typed action/metadatakind, and BLAKE3 action-result keys.Tasks: New experimental
rust_cacheon tasks (boolean or{ enabled }), plustask_config.rust_cachedefaults, cascade, and templates—with explicitfalseoverriding inherited defaults. Task cache key material gains akind: "task"action descriptor aligned with the generalized protocol.Docs & tests: Remote cache protocol doc becomes “Remote Build Cache Protocol”; task configuration documents
rust_cacheand nested remote settings. E2E and compat scripts updated for nested settings and protocol shape.Reviewed by Cursor Bugbot for commit 8a025ed. Bugbot is set up for automated code reviews on this repo. Configure here.