Skip to content

feat(cargo-anvil): local impact scoping via the anvil-impact recipe - #47

Open
martin-kolinek wants to merge 21 commits into
mainfrom
anvil-local-impact
Open

feat(cargo-anvil): local impact scoping via the anvil-impact recipe#47
martin-kolinek wants to merge 21 commits into
mainfrom
anvil-local-impact

Conversation

@martin-kolinek

@martin-kolinek martin-kolinek commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Local impact scoping via the anvil-impact recipe

Pulls cargo-delta impact analysis into a single shared anvil-impact just recipe that runs identically locally and in CI, producing durable artifacts under target/anvil/impact/ that scoped checks consume. Re-applied on top of main's per-file justfiles layout (#48), reusing main's _anvil-base-ref and _anvil-impact-format helpers.

What it does

  • impact.just — anvil-impact + _anvil-impact-snapshot + _anvil-impact-include. Two independent cache keys: baseline.json keyed on the base commit sha (expensive worktree snapshot only retaken when the base moves), current.json keyed on HEAD+worktree-diff. Fail-fast on missing base/shallow clone; active-toolchain baseline; first-time-adoption fallback. ANVIL_IMPACT=off disables scoping.
  • Checks — the 25 scoped checks gain a : anvil-impact dep + a one-line self-populate of ANVIL_INCLUDE_<TIER> from the cache when unset (preserving each body and CI env threading). Scoping is on by default for just anvil-pr.
  • Tiers — anvil-scheduled/anvil-full are ANVIL_IMPACT=off wrappers so scheduled tiers stay full-workspace.
  • CI — the GitHub/ADO impact steps now run just anvil-impact and thread its include files downstream, replacing the inline shell.

Scoping model + the dirty-tree safety net

cargo-delta scopes on the committed diff vs the base ref, so local scoping reflects your branch's commits like a PR does (not uncommitted edits). Because that would silently scope out a crate you're actively editing but haven't committed, anvil-impact detects an uncommitted (dirty) working tree and widens every tier to --workspace locally (with a notice) so WIP is never skipped; committing restores scoping. CI checkouts are clean, so CI keeps the scoped result.

Tests

tests/impact.rs drives real git/cargo-delta/just to verify: baseline regenerates only when the base moves, current regenerates only when the tree changes, both reused when unchanged, ANVIL_IMPACT=off, the no-workspace fallback, and dirty-tree widening (clean→scoped, dirty→--workspace, commit→scoping restored). Full suite green (256 lib + integration + regenerated snapshots).

@codecov-commenter

codecov-commenter commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (beeaf82) to head (2bf2459).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main      #47   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files          50       50           
  Lines        2652     2666   +14     
=======================================
+ Hits         2652     2666   +14
Flag Coverage Δ
linux 100.0% <100.0%> (ø)
linux-arm 100.0% <100.0%> (ø)
scheduled ?
windows 100.0% <100.0%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@martin-kolinek
martin-kolinek force-pushed the anvil-local-impact branch 2 times, most recently from 5fb5aef to d7ee000 Compare June 26, 2026 15:34
@martin-kolinek martin-kolinek changed the title docs(cargo-anvil): design local impact scoping via shared anvil-impact recipe feat(cargo-anvil): local impact scoping via the anvil-impact recipe Jul 20, 2026
martin-kolinek
martin-kolinek commented Jul 21, 2026
View reviewed changes
Comment thread crates/cargo-anvil/docs/design/ado.md Outdated
@martin-kolinek
feat(cargo-anvil): local impact scoping via a shared anvil-impact recipe
bcd6baa 
Pull cargo-delta impact analysis into a single `anvil-impact` just recipe
that runs identically locally and in CI, producing a durable
target/anvil/impact/ cache consumed by every scoped check.

- impact.just: anvil-impact + _anvil-impact-snapshot + _anvil-impact-include
  (two-key cache, reuses _anvil-base-ref/_anvil-impact-format). Consumers with
  a present, matching cache short-circuit without cargo-delta or a base ref.
  A dirty working tree widens all tiers to --workspace (with a warning).
- Scoped checks depend on anvil-impact and self-populate ANVIL_INCLUDE_<TIER>
  from the cache; scheduled/full tiers are ANVIL_IMPACT=off wrappers.
- CI propagates the target/anvil/impact/ folder as a per-OS artifact
  (upload/download) instead of threading env vars / stage outputs. ADO adds a
  job.yml inputArtifacts param (DownloadPipelineArtifact, 1ESPT-overridable).
- Impact-scoped groups declare cargo-delta as a setup/validate-prereq.
- Catalog registration + unit tests + tests/impact.rs behavioral cache tests
  + regenerated snapshots; design docs (local S4, checks S5, github, ado).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
@martin-kolinek
refactor(cargo-anvil): address PR review on impact-scoping recipes
09288cb 
Three review comments:

1. impact.just: add a one-line summary comment before each recipe so
   `just --list` shows clean recipe documentation (matching the 0.3.0
   per-recipe doc convention) instead of the block comment's first line.

2. Drop the now-vestigial ANVIL_INCLUDE_* env vars. With artifact
   propagation, CI no longer threads those vars, so the self-populate
   guard always fell through. Each scoped check now captures its scope
   directly into a local `$include` variable via _anvil-impact-include.
   Updated all 25 scoped checks, the helpers.just contract comment, and
   the design docs (checks.md S5, local.md S4, github.md, ado.md).

3. group-action.yml / ado group.yml: reword the "No impact inputs"
   comment to describe the current state (impact arrives as the
   downloaded target/anvil/impact cache) rather than the change.

Regenerate .github + dogfood justfiles + snapshots; update the
scoped-check unit test to detect checks via _anvil-impact-include.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
martin-kolinek
martin-kolinek commented Jul 23, 2026
View reviewed changes
Comment thread justfiles/anvil/impact.just
Comment thread justfiles/anvil/checks/miri.just Outdated
Comment thread .github/actions/anvil-pr-test/action.yml Outdated
Comment thread justfiles/anvil/groups/pr-fast.just Outdated
martin-kolinek and others added 5 commits July 23, 2026 19:58
@martin-kolinek
fix(cargo-anvil): trim unfulfilled clippy expectations in impact test
3d11c3c 
tests/impact.rs uses only .unwrap() (not .expect()/panic!), so
expecting clippy::panic + clippy::expect_used was unfulfilled and
failed \cargo clippy -- -D warnings\ (anvil-clippy + static-analysis).
Expect only clippy::unwrap_used. Caught by CI running the dogfooded
anvil-pr workflow; cargo test alone doesn't invoke clippy.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
@martin-kolinek
fix(cargo-anvil): add ANVIL_IMPACT=consume so group jobs trust the cache
1306b9f 
CI surfaced a real failure: a shallow group-job checkout (pr-test,
pr-runtime-analysis) that downloaded the impact artifact fell through the
consumer fast path (its git-derived current.state differed slightly from
the impact job's) and hit the recompute path, which hard-failed resolving
origin/main -- a group job neither installs cargo-delta nor fetches the
base ref.

Make the consumer contract explicit instead of best-effort: a group job
that downloaded the authoritative target/anvil/impact artifact exports
ANVIL_IMPACT=consume, and anvil-impact / _anvil-impact-snapshot short-
circuit to a pure no-op in that mode -- trusting the downloaded cache
verbatim with no snapshot, no cargo-delta, and no base ref. Scheduled
group jobs (no artifact) still export ANVIL_IMPACT=off.

- impact.just: off-guards also honor `consume`; document the mode.
- github group-action.yml / ado group.yml: set consume when impact.state
  is present, off otherwise.
- tests/impact.rs: new test proving consume trusts the cache even with a
  perturbed tree + unresolvable base + no cargo-delta.
- Update unit tests, docs (github S6.1 / ado S4.3 + sketches), snapshots.

Also fixes the earlier clippy failure (unfulfilled lint expectations in
tests/impact.rs) that blocked every group job before this.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
@martin-kolinek
test(cargo-anvil): isolate impact tests from an inherited ANVIL_IMPACT
95400b6 
The impact integration tests drive a real `just anvil-impact` in a temp
repo to assert the recipe's default compute/fast-path behavior. In a
cloud PR group job the checks run under an exported ANVIL_IMPACT=consume
(or off), and one of those checks -- the coverage/mutants suite -- is what
runs these very tests, so the spawned recipe inherited the mode and
no-oped, failing the "snapshotting baseline" / "snapshots up to date"
assertions (green locally, red only in CI group jobs).

Clear ANVIL_IMPACT in the default `run_impact` helper and in the
unresolvable-base consumer test so they exercise the real path regardless
of the surrounding job env. Tests that want a specific mode (off, consume)
still set it explicitly on their own Command.

Verified: `ANVIL_IMPACT=consume cargo test -p cargo-anvil --test impact`
and `=off` both pass 6/6 (previously failed 4).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
@martin-kolinek
test(cargo-anvil): scrub CI base-ref env leaks in impact tests too
b715d98 
Follow-up to the ANVIL_IMPACT isolation fix. The impact integration tests
spawn `just anvil-impact` in a temp repo whose base ref is origin/master,
but on a PR build the outer job also exports GITHUB_BASE_REF=main (and ADO
sets SYSTEM_PULLREQUEST_TARGETBRANCH). _anvil-base-ref consults those, so
the temp-repo recipe resolved origin/main -- absent there -- and failed
base-ref resolution ("base ref 'origin/main' not found locally"), red only
in the coverage/mutants group jobs.

Introduce a single `just_cmd` helper that scrubs every CI-injected leak
(ANVIL_IMPACT, BASE_REF, GITHUB_BASE_REF, SYSTEM_PULLREQUEST_TARGETBRANCH)
and route all spawn sites through it; tests that need a specific mode/base
set it back explicitly on the returned Command.

Verified with the outer env fully simulated:
`ANVIL_IMPACT=consume GITHUB_BASE_REF=main SYSTEM_PULLREQUEST_TARGETBRANCH=refs/heads/main
cargo test -p cargo-anvil --test impact` -> 6/6 pass; clippy --tests -D warnings clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
@martin-kolinek
style(cargo-anvil): rustfmt the impact test spawn site
3e1750a 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
@martin-kolinek
martin-kolinek marked this pull request as ready for review July 23, 2026 19:10
Copilot AI review requested due to automatic review settings July 23, 2026 19:10
Copilot AI reviewed Jul 23, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Not ready to approve

There are correctness and security issues to address (dirty-tree scoping can still trigger snapshot recomputation, and new GitHub Actions uses should be pinned to immutable SHAs to match existing workflow conventions).

Pull request overview

This PR centralizes cargo-delta impact analysis into a single anvil-impact just recipe that produces a durable target/anvil/impact/ cache, and updates local tiers and CI wiring so scoped checks consume that cache (locally or via downloaded artifacts) instead of threading scope through environment variables.

Changes:

  • Add justfiles/anvil/impact.just implementing snapshotting + cached impact computation + _anvil-impact-include resolver, and wire scoped checks to depend on anvil-impact and read scope from the cache.
  • Update tiers and group recipes so anvil-scheduled / anvil-full run full-workspace via ANVIL_IMPACT=off, while PR tiers default to impact scoping.
  • Switch GitHub Actions and ADO pipelines from output/env-based include threading to uploading/downloading the target/anvil/impact/ artifact and consuming it via ANVIL_IMPACT=consume.
File summaries
File Description
justfiles/anvil/tiers.just Wrap scheduled/full tiers to force full-workspace via ANVIL_IMPACT=off.
justfiles/anvil/mod.just Import new impact.just.
justfiles/anvil/impact.just New shared impact recipe + cache + include resolver.
justfiles/anvil/helpers.just Update scoping contract documentation to reference cache-based resolver.
justfiles/anvil/groups/scheduled-test.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/groups/scheduled-runtime-analysis.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/groups/scheduled-exhaustive.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/groups/scheduled-advisories.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/groups/pr-test.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/groups/pr-runtime-analysis.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/groups/pr-mutants.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/groups/pr-fast.just Ensure group setup/validate includes cargo-delta prerequisite.
justfiles/anvil/checks/udeps.just Convert required-tier scoping to cache-based $include via _anvil-impact-include.
justfiles/anvil/checks/spellcheck.just Convert modified-tier skip logic to cache-based $include.
justfiles/anvil/checks/semver-check.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/readme-check.just Convert modified-tier scoping to cache-based $include.
justfiles/anvil/checks/mutants-diff.just Convert affected-tier skip logic to cache-based $include.
justfiles/anvil/checks/miri.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/miri-tree-borrows.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/miri-strict-provenance.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/miri-race-coverage.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/loom.just Convert affected-tier scoping to cache-based $include and token parsing.
justfiles/anvil/checks/llvm-cov.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/license-headers.just Convert modified-tier skip logic to cache-based $include.
justfiles/anvil/checks/fmt.just Convert modified-tier skip logic to cache-based $include.
justfiles/anvil/checks/external-types.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/examples.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/ensure-no-default-features.just Convert modified-tier skip logic to cache-based $include.
justfiles/anvil/checks/ensure-no-cyclic-deps.just Convert modified-tier skip logic to cache-based $include.
justfiles/anvil/checks/doc-test.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/doc-build.just Convert required-tier scoping to cache-based $include.
justfiles/anvil/checks/clippy.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/cargo-sort.just Convert modified-tier skip logic to cache-based $include.
justfiles/anvil/checks/cargo-hack.just Convert required-tier scoping to cache-based $include.
justfiles/anvil/checks/careful.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/bolero.just Convert affected-tier scoping to cache-based $include.
justfiles/anvil/checks/bench.just Convert affected-tier scoping to cache-based $include.
crates/cargo-anvil/templates/justfiles/anvil/tiers.just Template sync for tier wrappers (ANVIL_IMPACT=off).
crates/cargo-anvil/templates/justfiles/anvil/mod.just Template sync for impact.just import.
crates/cargo-anvil/templates/justfiles/anvil/helpers.just Template sync for cache-based scoping contract docs.
crates/cargo-anvil/templates/justfiles/anvil/groups/scheduled-test.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/groups/scheduled-runtime-analysis.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/groups/scheduled-exhaustive.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/groups/scheduled-advisories.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/groups/pr-test.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/groups/pr-runtime-analysis.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/groups/pr-mutants.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/groups/pr-fast.just Template sync for cargo-delta prereqs.
crates/cargo-anvil/templates/justfiles/anvil/checks/udeps.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/spellcheck.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/semver-check.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/readme-check.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/mutants-diff.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/miri.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/miri-tree-borrows.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/miri-strict-provenance.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/miri-race-coverage.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/loom.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/license-headers.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/fmt.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/external-types.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/examples.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/ensure-no-default-features.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/ensure-no-cyclic-deps.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/doc-test.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/doc-build.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/clippy.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/cargo-sort.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/cargo-hack.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/careful.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/bolero.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/justfiles/anvil/checks/bench.just Template sync: scoped check reads cache via _anvil-impact-include.
crates/cargo-anvil/templates/github/pr-impl-workflow.yml Template: consume impact via artifact download instead of job outputs.
crates/cargo-anvil/templates/github/impact-action.yml Template: run just anvil-impact and upload target/anvil/impact/.
crates/cargo-anvil/templates/github/group-action.yml Template: set ANVIL_IMPACT=consume/off based on downloaded cache presence.
crates/cargo-anvil/templates/ado/steps/job.yml Template: add inputArtifacts to download artifacts before steps.
crates/cargo-anvil/templates/ado/steps/impact.yml Template: run just anvil-impact (no output-variable threading).
crates/cargo-anvil/templates/ado/steps/group.yml Template: set ANVIL_IMPACT=consume/off based on downloaded cache presence.
crates/cargo-anvil/templates/ado/pr-stages.yml Template: publish per-OS impact artifacts; jobs download them via inputArtifacts.
crates/cargo-anvil/src/anvil/artifacts/mod.rs Register impact.just as an owned artifact.
crates/cargo-anvil/src/anvil/artifacts/justfile.rs Embed impact.just template and add tests for wiring expectations.
crates/cargo-anvil/src/anvil/artifacts/github.rs Update tests for artifact-based impact propagation.
crates/cargo-anvil/src/anvil/artifacts/ado.rs Update tests for artifact-based impact propagation.
crates/cargo-anvil/docs/design/local.md Document new local impact model and cache artifacts.
crates/cargo-anvil/docs/design/github.md Document GitHub Actions artifact upload/download wiring for impact.
crates/cargo-anvil/docs/design/checks.md Update scoping contract docs from env-vars to cache-based resolver.
crates/cargo-anvil/docs/design/ado.md Document ADO artifact-based impact propagation and inputArtifacts.
.github/workflows/anvil-pr-impl.yml Repo workflow: download impact artifact into target/anvil/impact/.
.github/actions/anvil-impact/action.yml Repo action: compute impact via just anvil-impact and upload cache.
.github/actions/anvil-pr-fast/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.github/actions/anvil-pr-test/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.github/actions/anvil-pr-runtime-analysis/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.github/actions/anvil-pr-mutants/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.github/actions/anvil-scheduled-test/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.github/actions/anvil-scheduled-advisories/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.github/actions/anvil-scheduled-runtime-analysis/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.github/actions/anvil-scheduled-exhaustive/action.yml Repo action: select ANVIL_IMPACT mode based on cache presence.
.anvil.lock Update managed-file checksums to reflect new artifacts and wiring.

Review details

Comments suppressed due to low confidence (1)

justfiles/anvil/impact.just:104

  • This comment claims scheduled-tier jobs never install cargo-delta, but the group setup/validate recipes now install/validate cargo-delta for scheduled groups too. The rationale for validating cargo-delta lazily here should be phrased in terms of ANVIL_IMPACT modes (off/consume) rather than scheduled tiers specifically.
  • Files reviewed: 103/104 changed files
  • Comments generated: 7
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread justfiles/anvil/impact.just
Comment thread crates/cargo-anvil/templates/ado/steps/impact.yml Outdated
Comment thread crates/cargo-anvil/templates/github/impact-action.yml Outdated
Comment thread .github/workflows/anvil-pr-impl.yml Outdated
Comment thread .github/actions/anvil-impact/action.yml Outdated
Comment thread crates/cargo-anvil/templates/github/pr-impl-workflow.yml Outdated
Comment thread crates/cargo-anvil/templates/github/impact-action.yml Outdated
@martin-kolinek
docs(cargo-anvil): correct stale cargo-delta rationale in impact.just
70e7794 
The comment justifying why cargo-delta is validated inside the recompute
path (rather than as a hard `just` dependency of anvil-impact) cited "the
scheduled tier, whose jobs never install cargo-delta" -- no longer true:
every group (PR and scheduled) now declares anvil-tool-cargo-delta-install
in its setup/validate-prereqs. Rewrite the rationale to the accurate one:
only the recompute path needs the tool, so the paths that skip it
(ANVIL_IMPACT=off/consume, or a cache hit) must no-op without it; groups
that can actually reach recompute declare cargo-delta as their own prereq
for local fail-fast, and keeping it off anvil-impact's dependency line is
what lets the no-op paths stay tool-agnostic.

Comment-only; regenerated dogfood justfile, .anvil.lock, and snapshots.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
Copilot AI review requested due to automatic review settings July 24, 2026 10:50
Copilot AI reviewed Jul 24, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Not ready to approve

New GitHub Actions references to actions/download-artifact@v4 / actions/upload-artifact@v4 are not pinned to immutable commit SHAs, which conflicts with the repo’s established action-pinning convention and is a supply-chain risk.

Review details

Comments suppressed due to low confidence (11)

crates/cargo-anvil/templates/ado/steps/impact.yml:4

  • The file header duplicates the copyright/license lines (they appear twice), which is redundant and can create merge noise in future template updates.
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

.github/workflows/anvil-pr-impl.yml:100

  • This workflow generally pins third-party GitHub Actions to immutable commit SHAs (e.g. actions/checkout and marocchino/sticky-pull-request-comment in this file). actions/download-artifact@v4 is unpinned; for supply-chain consistency it should also be pinned to a commit SHA (with the version noted in a comment).
        uses: actions/download-artifact@v4

.github/workflows/anvil-pr-impl.yml:148

  • actions/download-artifact@v4 is referenced by tag here, but this workflow otherwise pins external actions to commit SHAs. Please pin this usage to an immutable SHA as well (and keep the v4 version as a comment).
        uses: actions/download-artifact@v4

.github/workflows/anvil-pr-impl.yml:196

  • For consistency with the rest of this repo’s workflows (which pin actions to SHAs), actions/download-artifact@v4 should be pinned to a specific commit SHA here too.
        uses: actions/download-artifact@v4

.github/workflows/anvil-pr-impl.yml:222

  • actions/download-artifact@v4 should be pinned to an immutable commit SHA (matching the pinning approach used elsewhere in this workflow) to avoid floating-tag supply-chain risk.
        uses: actions/download-artifact@v4

.github/actions/anvil-impact/action.yml:47

  • This composite action uses actions/upload-artifact@v4 by tag. In this repo the convention is to pin external actions to immutable commit SHAs; please pin upload-artifact to a specific SHA (keeping the v4 version in a comment).
      uses: actions/upload-artifact@v4

crates/cargo-anvil/templates/github/pr-impl-workflow.yml:100

  • Template workflows pin other external actions to commit SHAs (e.g. actions/checkout and marocchino/sticky-pull-request-comment), but actions/download-artifact@v4 is unpinned here. Please pin it to an immutable commit SHA to keep the generated workflows consistent and reduce supply-chain risk.
        uses: actions/download-artifact@v4

crates/cargo-anvil/templates/github/pr-impl-workflow.yml:148

  • actions/download-artifact@v4 should be pinned to a commit SHA in this template (matching the rest of the generated workflows’ pinning style).
        uses: actions/download-artifact@v4

crates/cargo-anvil/templates/github/pr-impl-workflow.yml:196

  • This actions/download-artifact@v4 usage in the template should be pinned to a commit SHA, so downstream generated workflows don’t rely on a floating tag.
        uses: actions/download-artifact@v4

crates/cargo-anvil/templates/github/pr-impl-workflow.yml:222

  • Please pin actions/download-artifact@v4 to an immutable commit SHA in this template (consistent with the rest of the anvil-managed workflow templates).
        uses: actions/download-artifact@v4

crates/cargo-anvil/templates/github/impact-action.yml:47

  • This template uses actions/upload-artifact@v4 by tag. Since anvil-managed workflows/actions in this repo are generally pinned to commit SHAs, this should be pinned too (with the v4 version retained in a comment).
      uses: actions/upload-artifact@v4
  • Files reviewed: 103/104 changed files
  • Comments generated: 1
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Comment thread justfiles/anvil/tiers.just
@martin-kolinek
fix(cargo-anvil): address PR review comments (dirty-tree ordering, pi…
cdcc4e9 
…ns, doc nits)

Bot review feedback on PR #47:

* Dirty-tree ordering (impact.just): anvil-impact widens to --workspace on a
  dirty tree, but its dependency _anvil-impact-snapshot ran first and could
  fall into the recompute path (cargo-delta + resolvable, non-shallow base
  ref) BEFORE that safety net -- so a first-time/local WIP run with a dirty
  tree and an unresolvable base hard-failed instead of widening. Short-circuit
  _anvil-impact-snapshot on a dirty tree (the snapshots are unused when we
  widen), and have anvil-impact's dirty branch create the impact dir it writes
  into. New regression test proves a dirty tree widens even with an
  unresolvable base; the cache-granularity test now drives current/baseline
  regeneration via commits (clean tree) instead of an uncommitted edit, which
  the widening behavior now subsumes.

* Pin external actions to immutable SHAs for supply-chain consistency:
  actions/upload-artifact@v4 -> @ea165f8 # v4.6.2,
  actions/download-artifact@v4 -> @d3f86a1 # v4.3.0 (all four uses).

* impact-action.yml: reword "the only job that needs cargo-delta" -> "the only
  job that RUNS cargo-delta to compute the impact set" (group jobs install it
  as a prereq but consume the artifact).

* ado/steps/impact.yml: drop the accidentally duplicated copyright/license
  header lines.

* tiers.just: drop the duplicated "Full tier" line (the block comment repeated
  the one-line recipe doc comment).

Regenerated dogfood justfiles, .github, .anvil.lock, and snapshots. Full
cargo-anvil suite (258 lib + 7 impact + snapshots), clippy -D warnings, fmt,
and spellcheck all clean; --dry-run drift = 0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
Copilot AI review requested due to automatic review settings July 24, 2026 12:28
Copilot AI reviewed Jul 24, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Not ready to approve

The group action’s ANVIL_IMPACT=consume auto-detection can be misled by restored target/ caches (including stale target/anvil/impact/), which risks incorrectly scoping/skipping scheduled checks.

Review details
  • Files reviewed: 102/104 changed files
  • Comments generated: 2
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Comment thread crates/cargo-anvil/templates/github/group-action.yml Outdated
Comment thread .github/actions/anvil-scheduled-test/action.yml Outdated
@martin-kolinek
fix(cargo-anvil): scheduled group jobs must force ANVIL_IMPACT=off
585901b 
Bot review caught a real correctness bug: the group action/step decided its
impact mode by testing for `target/anvil/impact/impact.state`, but anvil-setup
(GitHub) and setup.yml (ADO) both restore `target/` from cache -- including any
`impact.state`. A scheduled group job, which downloads no impact artifact and
must validate the FULL workspace, could restore a stale `impact.state` from a
prior run, flip itself into `ANVIL_IMPACT=consume`, and silently skip the
full-workspace backstop (running impact-scoped checks instead).

Split the impact-mode selection by tier in the renderers (github.rs /
ado.rs) via a new `__IMPACT_MODE__` template placeholder:

* PR groups (which genuinely download the artifact) keep the
  impact.state -> consume / else off detection.
* Scheduled groups export `ANVIL_IMPACT=off` UNCONDITIONALLY -- never gating
  on the cacheable impact.state marker.

Fixes both GitHub composite actions and ADO group steps (both back ends cache
target/). Updated unit tests (new scheduled-forces-off tests on both back
ends; PR tests unchanged), regenerated .github actions + .anvil.lock +
snapshots, and updated the propagation docs (github.md S6.1 / ado.md S4.3).

Full cargo-anvil suite (260 lib + 7 impact + snapshots), clippy -D warnings,
fmt, and spellcheck all clean; --dry-run drift = 0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
Copilot AI review requested due to automatic review settings July 24, 2026 13:37
Copilot AI reviewed Jul 27, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Human review recommended

It makes broad, interdependent changes across the just recipe graph and both CI backends’ artifact/caching behavior, which warrants final human verification despite only minor review findings.

Review details
  • Files reviewed: 106/109 changed files
  • Comments generated: 2
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Comment thread justfiles/anvil/helpers.just Outdated
Comment thread crates/cargo-anvil/templates/justfiles/anvil/helpers.just Outdated
@martin-kolinek
Add cargo heather to the modified-tier bucket examples
0ed250d 
The 'modified' bucket description in helpers.just listed cargo fmt --all and
cargo spellcheck as example workspace/directory-scoped tools but omitted
cargo heather, which the modified-tier anvil-license-headers check runs.
Keeps the scoping docs consistent with the shipped checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
Copilot AI review requested due to automatic review settings July 27, 2026 16:12
Copilot AI dismissed their stale review, a newer Copilot review was requested July 27, 2026 16:17
Copilot AI previously approved these changes Jul 27, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Ready to approve

The scoping, artifact propagation, and tier-fixed impact mode wiring appear consistent across local justfiles, templates, and both CI backends with no remaining correctness gaps identified in the changed regions.

Note: this review does not count toward required approvals for merging.

Review details
  • Files reviewed: 106/109 changed files
  • Comments generated: 0 new
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

martin-kolinek
martin-kolinek commented Jul 28, 2026
View reviewed changes
Comment thread crates/cargo-anvil/templates/ado/pr-stages.yml
martin-kolinek
martin-kolinek commented Jul 28, 2026
View reviewed changes
Comment thread crates/cargo-anvil/docs/design/github.md
martin-kolinek
martin-kolinek commented Jul 28, 2026
View reviewed changes
Comment thread crates/cargo-anvil/docs/design/checks.md Outdated
martin-kolinek
martin-kolinek commented Jul 28, 2026
View reviewed changes
Comment thread crates/cargo-anvil/templates/justfiles/anvil/impact.just
martin-kolinek
martin-kolinek commented Jul 28, 2026
View reviewed changes
Comment thread crates/cargo-anvil/tests/impact.rs
@martin-kolinek

martin-kolinek commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

🤖: crates/cargo-anvil/docs/design/ado.md around line 635 is stale. This paragraph still describes threading the three include_* impact env vars/parameters through each template ("The recipes themselves consume only the env vars they need… Threading all three to every template…", plus the following note that direct-template: users "default to empty (recipes fall back to --workspace)"). This PR removed that transport in favor of publishing/downloading target/anvil/impact/ and an emit-time ANVIL_IMPACT=consume|off mode. Please rewrite this paragraph and the direct-template fallback text below it so the ADO design matches the implemented artifact-based contract.

@martin-kolinek
Address PR #47 review: inputArtifacts docs, stale scoping wording, im…
344cb9e 
…pact tests

Docs:
- ado.md: document the job.yml wrapper's inputArtifacts parameter in the
  contract table and demonstrate it in both the default and 1ESPT wrapper
  examples (the parameter already ships in steps/job.yml; the design doc was
  out of sync). Backward-compat for adopters who customized job.yml is handled
  by anvil's existing .proposed migration model, same as every owned file.
- github.md: scheduled groups no longer receive include_* inputs; reword both
  scheduled blocks to the ANVIL_IMPACT=off mode-based behavior.
- checks.md: rename the "env-var mapping" section to "include mapping" and fix
  the remaining stale env-var phrasing (\ -eq '--skip', include files);
  update the section anchor in ado.md, github.md, and local.md.

Tests (crates/cargo-anvil/tests/impact.rs):
- Split workspace() into workspace_at_base() + the HEAD-advance step.
- impact_empty_output_when_head_equals_base: a clean HEAD==base checkout
  produces no cargo-delta output; assert impact.json is {} and every tier
  projects to --skip, and an unchanged rerun is a cache hit.
- scoped_check_consumes_cached_package_list_and_skips_on_sentinel: drive a
  representative scoped check (anvil-examples) against the downloaded cache
  with a fake cargo on PATH; prove the cached --package name@version list
  reaches the tool and that --skip short-circuits invocation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
Copilot AI review requested due to automatic review settings July 28, 2026 10:37
@martin-kolinek
Rewrite stale ado.md impact-threading paragraph to artifact-based con…
554a394 
…tract

ado.md around the per-group parameters table still described threading the
three include_* impact vars/parameters through every step template, and said
direct-template users get empty includes that fall back to --workspace. This PR
replaced that transport with publishing/downloading target/anvil/impact/ plus
an emit-time ANVIL_IMPACT=consume|off mode. Rewrite both paragraphs so the ADO
design matches the implemented artifact-based contract: scope is read from the
downloaded cache via _anvil-impact-include, never threaded through env vars or
template parameters.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
Copilot AI dismissed their stale review, a newer Copilot review was requested July 28, 2026 10:42
Copilot AI previously approved these changes Jul 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Ready to approve

The impact-cache model is consistently applied across local recipes, templates, and CI/ADO wiring, and the updated tests/docs align with the new behavior.

Note: this review does not count toward required approvals for merging.

Review details
  • Files reviewed: 106/109 changed files
  • Comments generated: 0 new
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Copilot AI review requested due to automatic review settings July 28, 2026 10:42
@martin-kolinek

martin-kolinek commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

🤖: Fixed in 554a394. Rewrote the stale ado.md paragraph (and the direct-template fallback text below it): impact scope is no longer threaded through include_* env vars/template parameters — each scoped check reads its tier from the downloaded target/anvil/impact/ cache via _anvil-impact-include <tier>. PR groups download the anvil-impact-<os> artifact (mode consume); scheduled groups run ANVIL_IMPACT=off; a direct-template consumer with no cache falls back to --workspace. PR-context strings like PR_TITLE are still passed as before.

AI response generated by rocket

Copilot AI dismissed their stale review, a newer Copilot review was requested July 28, 2026 10:46
Copilot AI previously approved these changes Jul 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Ready to approve

The changes consistently implement artifact-backed impact scoping across local/CI with only minor documentation wording nits remaining.

Note: this review does not count toward required approvals for merging.

Review details

Comments suppressed due to low confidence (2)

justfiles/anvil/impact.just:33

  • This comment says group jobs “lack” cargo-delta, but the group setup/validate-prereqs recipes now install/verify cargo-delta (even though it isn’t run in ANVIL_IMPACT=consume mode). Rewording avoids implying the tool is absent and keeps the docs aligned with the current design.
    crates/cargo-anvil/templates/justfiles/anvil/impact.just:33
  • Same as the repo’s managed impact.just: this comment says group jobs “lack” cargo-delta, but group setup/validate-prereqs now install/verify cargo-delta (even though it isn’t run in consume mode). Reword to avoid implying the tool is absent.
  • Files reviewed: 106/109 changed files
  • Comments generated: 0 new
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

martin-kolinek
martin-kolinek commented Jul 28, 2026
View reviewed changes
Comment thread crates/cargo-anvil/tests/impact.rs Outdated
@martin-kolinek

martin-kolinek commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

🤖: crates/cargo-anvil/docs/design/checks.md — two miri catalog rows still describe the removed impact env-var transport, contradicting this PR's §5 rewrite (now "Impact-scoping check → include mapping", cache-based):

  • line ~200 (pr-runtime-analysis miri): "The recipe defaults to --workspace locally when no impact env vars are set…". Scope is now read from the downloaded target/anvil/impact/ cache via _anvil-impact-include; the recipe falls back to --workspace only when no cache is present (or ANVIL_IMPACT=off).
  • line ~251 (scheduled-runtime-analysis miri): "Same recipe as the pr-runtime-analysis member, but invoked without impact env vars so the run is full-workspace". The scheduled tier now runs full-workspace via ANVIL_IMPACT=off (emit-time __IMPACT_MODE__), not by omitting env vars — there are no impact env vars anymore.

Please reword both rows to match the cache / ANVIL_IMPACT mechanism the rest of this PR (incl. §5) now documents.

@martin-kolinek
Fix stale workspace() doc comment (origin/master, not origin/main)
2bf2459 
The workspace() fixture delegates to workspace_at_base(), which creates
refs/remotes/origin/master (the ref the recipe and cargo-delta resolve to);
no origin/main ref is ever created. Correct the doc comment to match.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37923be4-ff18-4207-bf8c-8a49849a657b
Copilot AI review requested due to automatic review settings July 28, 2026 16:20
Copilot AI dismissed their stale review, a newer Copilot review was requested July 28, 2026 16:24
Copilot AI approved these changes Jul 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Ready to approve

The changes consistently implement cache-based impact scoping across justfiles and CI templates, and I did not find any correctness issues in the updated wiring (artifact naming, tier mode selection, or workflow matrices).

Note: this review does not count toward required approvals for merging.

Review details
  • Files reviewed: 106/109 changed files
  • Comments generated: 0 new
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@martin-kolinek @codecov-commenter