Repository navigation
feat(task): explain task cache keys - #11595
Conversation
📝 WalkthroughWalkthroughThe change adds ChangesTask cache explanations and affected-task handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Run
participant TaskExecutor
participant TaskArtifactCache
Run->>TaskExecutor: enable task_cache_explain
TaskExecutor->>TaskArtifactCache: build cache-key explanation
TaskArtifactCache-->>TaskExecutor: return redacted explanation
TaskExecutor-->>Run: print explanation
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Greptile SummaryThe PR adds a structural explanation for task-cache key inputs while preventing cache restore/write during diagnostic dry-run and raw execution. It also ensures unmatched pnpm lockfile changes fall back to ordinary affected-path handling.
Confidence Score: 5/5The PR appears safe to merge. The two previously reported secret-oracle paths are closed because the diagnostic output now contains only structural metadata and does not emit either per-value digests or the aggregate cache key; no blocking failure remains. Important Files Changed
Reviews (3): Last reviewed commit: "feat(task): explain task cache keys" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
|
4e39e95 to
f23ca56
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f23ca56. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/config/mod.rs (1)
526-535: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplace check-then-set caching with an atomic get-or-init.
workspace_project_graph_for_task_loadinglocks, checks, unlocks, then computes and locks again to store the result. Two callers can both observeNoneat the same time, both rundiscover_workspace_project_graph()(a full filesystem scan), and both write. The result is redundant work, not corrupted data, since discovery is a pure read and any run's output is equally valid to cache. Useonce_cell::sync::OnceCell::get_or_try_init(already used elsewhere in this file, for exampleenv: OnceCell<EnvResults>) so concurrent callers block on the first computation instead of duplicating it.♻️ Proposed refactor
- workspace_project_graph_cache: - Mutex<Option<Arc<crate::task::workspace::WorkspaceProjectGraph>>>, + workspace_project_graph_cache: OnceCell<Arc<crate::task::workspace::WorkspaceProjectGraph>>,- fn workspace_project_graph_for_task_loading( - &self, - ) -> Result<Arc<crate::task::workspace::WorkspaceProjectGraph>> { - if let Some(graph) = self.workspace_project_graph_cache.lock().unwrap().clone() { - return Ok(graph); - } - let graph = Arc::new(self.discover_workspace_project_graph()?); - *self.workspace_project_graph_cache.lock().unwrap() = Some(graph.clone()); - Ok(graph) - } + fn workspace_project_graph_for_task_loading( + &self, + ) -> Result<Arc<crate::task::workspace::WorkspaceProjectGraph>> { + self.workspace_project_graph_cache + .get_or_try_init(|| self.discover_workspace_project_graph().map(Arc::new)) + .cloned() + }Update every
workspace_project_graph_cache: Mutex::new(None)test-fixture initializer toOnceCell::new().🤖 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/config/mod.rs` around lines 526 - 535, Replace the Mutex-based check-then-set logic in workspace_project_graph_for_task_loading with OnceCell::get_or_try_init, preserving error propagation and returning the cached Arc. Change workspace_project_graph_cache’s type and every test-fixture initializer from Mutex::new(None) to OnceCell::new().src/cli/mod.rs (1)
774-818: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce duplicated
Runfield enumeration with#[derive(Default)]. Both sites build a fullrun::Run { ... }literal by hand becauseRunhas noDefaultimplementation. This PR had to add the same 6 new fields to both places, and any future CLI option onRunwill require the same duplicated edit at every construction site.
src/cli/mod.rs#L774-L818: build the naked-taskRunasrun::Run { task, args: ..., affected: false, ..Default::default() }, keeping only the fields that differ from default explicit (e.g.cd: self.cd,task_cache: crate::task::TaskCacheMode::from_env()?).src/cli/bootstrap.rs#L897-L948: build the bootstrap-taskRunthe same way, keeping onlytask,dry_run,skip_tools, andtask_cacheexplicit.This requires adding
#[derive(Default)]to theRunstruct definition insrc/cli/run.rs; most fields already resolve toDefault::default()individually at both sites today, so this should compile with minimal further changes.🤖 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/cli/mod.rs` around lines 774 - 818, Add #[derive(Default)] to the Run struct in src/cli/run.rs, then replace the full Run literals in src/cli/mod.rs:774-818 and src/cli/bootstrap.rs:897-948 with Default-based initialization. In src/cli/mod.rs, retain explicit task, args, affected, and other non-default values such as cd and task_cache; in src/cli/bootstrap.rs, retain only task, dry_run, skip_tools, and task_cache as specified.e2e/tasks/test_task_artifact_cache (1)
27-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCapture the explanation once instead of running the task six times.
Each
assert_contains/assert_not_containscall re-invokesPROFILE=debug mise run --task-cache-explain build, so the block runs the task six times just to match substrings of the same output. Store the explanation once and run the assertions against the stored string so failures show the full output.🤖 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 `@e2e/tasks/test_task_artifact_cache` around lines 27 - 34, Capture the output of the PROFILE=debug mise run --task-cache-explain build command once in a variable, then update all assert_contains and assert_not_contains calls in this test to inspect that stored explanation instead of rerunning the task. Preserve all existing substring checks and ensure assertion failures can display the complete captured output.src/task/workspace/node.rs (1)
405-444: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid scanning all lockfile entries for each dependency candidate.
resolve_package_keysiterates the wholeavailableset for every candidate path, and this runs duringreachable_packages, which is called once per importer in the pnpm diff path. Sinceavailableis aBTreeSetcontaining bothpackagesandsnapshots, a prefixrangescan can stop after the exact key and its(...)peer variants, avoiding a linear scan over unrelated lockfile entries.🤖 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/workspace/node.rs` around lines 405 - 444, Optimize resolve_package_keys by replacing the full available.iter() scan for each candidate with a BTreeSet range lookup that starts at the candidate and stops after its exact key and parenthesized peer variants. Preserve matching of the exact candidate and keys with suffixes beginning with '(' while avoiding unrelated packages and snapshots.
🤖 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 `@e2e/tasks/test_task_artifact_cache`:
- Around line 27-34: Capture the output of the PROFILE=debug mise run
--task-cache-explain build command once in a variable, then update all
assert_contains and assert_not_contains calls in this test to inspect that
stored explanation instead of rerunning the task. Preserve all existing
substring checks and ensure assertion failures can display the complete captured
output.
In `@src/cli/mod.rs`:
- Around line 774-818: Add #[derive(Default)] to the Run struct in
src/cli/run.rs, then replace the full Run literals in src/cli/mod.rs:774-818 and
src/cli/bootstrap.rs:897-948 with Default-based initialization. In
src/cli/mod.rs, retain explicit task, args, affected, and other non-default
values such as cd and task_cache; in src/cli/bootstrap.rs, retain only task,
dry_run, skip_tools, and task_cache as specified.
In `@src/config/mod.rs`:
- Around line 526-535: Replace the Mutex-based check-then-set logic in
workspace_project_graph_for_task_loading with OnceCell::get_or_try_init,
preserving error propagation and returning the cached Arc. Change
workspace_project_graph_cache’s type and every test-fixture initializer from
Mutex::new(None) to OnceCell::new().
In `@src/task/workspace/node.rs`:
- Around line 405-444: Optimize resolve_package_keys by replacing the full
available.iter() scan for each candidate with a BTreeSet range lookup that
starts at the candidate and stops after its exact key and parenthesized peer
variants. Preserve matching of the exact candidate and keys with suffixes
beginning with '(' while avoiding unrelated packages and snapshots.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 66fe3053-0193-49da-b4c4-4830808a9268
📒 Files selected for processing (18)
TASK_CACHE_PARITY.mddocs/cli/run.mddocs/cli/tasks/run.mddocs/tasks/monorepo.mddocs/tasks/task-configuration.mde2e/tasks/test_task_affectede2e/tasks/test_task_artifact_cacheman/man1/mise.1mise.usage.kdlsrc/cli/bootstrap.rssrc/cli/mod.rssrc/cli/run.rssrc/config/mod.rssrc/git.rssrc/task/task_cache.rssrc/task/task_executor.rssrc/task/workspace.rssrc/task/workspace/node.rs
f23ca56 to
9087e2c
Compare
9087e2c to
864c270
Compare
Stack created with GitHub Stacks CLI • Give Feedback 💬
Summary
mise run --task-cache-explain <task>Why
Task output cache keys were opaque, which made it difficult to understand what contributes to them. The explanation is deliberately structural: potentially secret-bearing values and hashes are unavailable to the renderer. Review also identified that dry-run skipped explanations and that an empty pnpm provider result could swallow a changed path; both are corrected here because the affected-task prerequisite PRs have merged.
User impact
Cache-enabled tasks can inspect key inputs without executing, restoring, or storing task output. Ordinary runs do not pay diagnostic construction costs. A pnpm lockfile in a non-Node workspace now affects projects through the normal workspace-global path rules.
Validation
cargo test cache_explanation_omits_environment_and_variable_values -- --nocapturecargo test cli::tests::test_subcommands_are_sorted -- --nocapturemise run test:e2e e2e/tasks/test_task_affected e2e/tasks/test_task_artifact_cachemise run rendermise run lintcargo clippy --workspace --all-features --all-targets -- -D warningsAI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Medium Risk
Touches task cache key construction and affected-project lockfile routing; behavior is guarded by an opt-in flag and e2e tests, but cache and monorepo selection paths are user-visible when enabled.
Overview
Adds
mise run --task-cache-explainso cache-enabled tasks can print a structural breakdown of what feeds the output cache key—categories, counts, env/var names and set/unset state, and platform—without emitting the aggregate key or secret-bearing hashes.The executor builds that explanation only when the flag is set, prints it before cache logic, and still skips restore/write under
--dry-runor raw/interactive runs while allowing explain (including running cache command inputs when needed for the key). CLI wiring, generated docs, task-configuration guide, e2e coverage, and the parity tracker are updated accordingly.--affectedlockfile handling now treats an empty provider attribution as “not handled” and falls back to ordinary changed-path / workspace-global mapping, so an unrelatedpnpm-lock.yamlin a non-Node workspace is no longer swallowed.Reviewed by Cursor Bugbot for commit 864c270. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
--task-cache-explaintomise runandmise tasks run.Bug Fixes