Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe compile task now places Cargo home and target data under ChangesCargo sandbox isolation
Estimated code review effort: 2 (Simple) | ~5 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 |
Greptile SummaryThe PR keeps Cargo’s home and target output beneath the cache-session fixture’s permitted
Confidence Score: 5/5The PR appears safe to merge with no actionable defects identified. The changed Cargo environment paths remain inside the permitted sandbox, preserve the asserted build artifact location, and do not add Cargo home contents to task or Rust action-cache artifacts. Important Files Changed
Reviews (1): Last reviewed commit: "test(task): isolate cargo writes in cach..." | 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.
|
|
The e2e run on this PR's head ( https://github.com/jdx/mise/actions/runs/31453472371/job/93663096515 The reason is that https://github.com/jdx/mise/actions/runs/31449920970/job/93652822330 — an e2e job carrying an extra diagnostic print I had on #11853. The failure mode described here is real, just conditional on Landlock working. #11861 takes the other route — drop The coverage that gives up is smaller than it looks. The isolated dir lives under Either way is fine by me. The two changes are not exclusive: yours makes the fixture pass wherever Landlock works, and if you would rather keep the sandbox here I can close #11861 — the runners would then need Landlock for this to go green. On why this sat unnoticed: the e2e workflow runs two tranches in a single |
Summary
target/sandbox pathContext
The E2E failure seen on #11857 was unrelated to the install-hook change.
cargo buildcreates.global-cache,.package-cache, and.package-cache-mutateinCARGO_HOMEeven for the dependency-free fixture. On fresh CI containers, those files do not exist yet and the task’sdeny_write = truesandbox rejects their creation outsideallow_write = ["target"].Testing
mise run lint-fixe2e/run_test tasks/test_task_action_cache_sessionCARGO_HOME; the fixture now succeeds because it scopes Cargo writes totarget/AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Low Risk
E2E fixture-only change; no production task-cache or sandbox behavior is modified.
Overview
Fixes the cache-session E2E
compiletask socargo buildno longer writes outside the sandbox.The task uses
deny_write = truewithallow_write = ["target"], but plaincargo buildstill creates lock/cache files under the defaultCARGO_HOME, which fails on fresh CI. The run script now setsCARGO_HOME=target/cargo-homeandCARGO_TARGET_DIR=targetso all Cargo writes stay under the permittedtarget/path.Reviewed by Cursor Bugbot for commit 3fc941c. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit