fix(cargo-each): prefer PATH when launching Windows child commands

[Evgenii (Vaiz)]

🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.

What this changes

cargo-each explicitly forwards the inherited PATH to child commands on Windows. This prevents adjacent executables from shadowing commands selected through PATH.

Effects

• Windows child commands search PATH before the directory containing cargo-each.
• Explicit executable paths, fallback lookup, and Unix behavior remain unchanged.
• Cargo commands retain support for +toolchain arguments. No dependencies are added.

Also in this PR: the CI repairs needed to land it

The cargo-each fix could not be validated on its own, because the branch hit three separate failures in the shared Anvil CI path. They are fixed here rather than split out, since splitting leaves this PR permanently red.

Scoped doctests skipped no packages. anvil-doc-test passed its affected --package selection straight to cargo test --doc, so a binary-only package in that set failed the run — which is exactly what happened to cargo-each itself, across four platforms. The recipe now filters the selection to packages with a library target, keeping every library kind including proc-macro, keeping version-qualified specs, and keeping both the all-feature and default-feature runs. An all-binary selection now skips visibly; unknown packages and metadata failures stay hard errors.

Tool installation was unauthenticated. The Linux ARM fast-checks job hit anonymous GitHub release-API 403s and fell back to a slow source build that then failed on a missing system dependency. The GitHub setup steps and the impact action's cargo-delta install now pass the built-in GITHUB_TOKEN so release discovery is authenticated. The credential is kept out of third-party builds: it is removed from the environment before any source install, and binstall's compile strategy is disabled whenever it is set, so a release miss falls through to a tokenless cargo install. Token scopes remain the caller's decision, which is where this design puts permissions.

Three cargo-gamma-lib doctests only ever compiled with --all-features. They import cargo_gamma_lib::internals, which is gated behind the feature that exposes it, so the recipe's default-features pass could not compile them. This was pre-existing and simply unreachable while the recipe was dying earlier on binary-only packages; fixing the recipe surfaced it. The doctest bodies are now gated on that feature, and still compile and assert for real under --all-features.

Generated files, .anvil.lock and the emitted-tree snapshots are regenerated to match.

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.5%. Comparing base (7986dd4) to head (a83bb9e).
⚠️ Report is 1 commits behind head on main.

❌ Your project status has failed because the head coverage (97.5%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #172     +/-   ##
=======================================
- Coverage   97.5%   97.5%   -0.1%     
=======================================
  Files        300     300             
  Lines      69048   69051      +3     
=======================================
+ Hits       67387   67388      +1     
- Misses      1661    1663      +2     

Flag	Coverage Δ
linux	97.5% <ø> (-0.1%)	⬇️
linux-arm	97.5% <ø> (-0.1%)	⬇️
scheduled	?
windows	97.7% <100.0%> (-0.1%)	⬇️

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.

[Copilot]

🟢 Approval recommended

The remaining documentation update is a minor, non-blocking nit.

Pull request overview

Updates cargo-each so Windows child commands honor inherited PATH precedence while preserving explicit paths, fallback lookup, Unix behavior, and +toolchain arguments.

Changes:

• Forward PATH explicitly on Windows.
• Add regression coverage for lookup precedence and explicit paths.
• Document executable lookup semantics.

File summaries

File	Summary
crates/cargo-each/tests/cli.rs	Adds Windows lookup and explicit-path coverage.
crates/cargo-each/src/run.rs	Forwards inherited PATH to Windows child commands.
crates/cargo-each/docs/design/README.md	Documents the updated lookup contract; crate-level docs still need synchronization.

Review details

Suppressed comments (1)

crates/cargo-each/docs/design/README.md:250

• This externally visible lookup rule is only added to the design doc. The crate-level docs in crates/cargo-each/src/main.rs are the source for the published crates/cargo-each/README.md (the repository documents that generation in README.md:97-100), and they still omit PATH precedence and the unchanged explicit-path/fallback behavior. Add a concise user-facing note to the crate docs and regenerate the README so the published documentation matches this change.

- **Executable lookup.** On Windows, the inherited `PATH` is explicitly passed
  to each child so its entries take precedence over the directory containing
  `cargo-each`. An adjacent executable must not shadow a tool selected on `PATH`
  (for example, a different Cargo toolchain proxy). Explicit executable paths
  and the platform's fallback search when no `PATH` entry matches are unchanged.

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved critical findings remain in the setup template and change scope.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

crates/cargo-anvil/docs/design/github.md:800

• This new section documents cargo-binstall as part of the GitHub setup, but the crate-level security contract still says that all cargo-tool installs use locked Cargo installs and explicitly says “No cargo-binstall” (crates/cargo-anvil/docs/design/README.md:388-392). Please reconcile those design statements in this change so adopters are not given contradictory installation guarantees.

The Just bootstrap and catalog tool installation steps receive the built-in
`GITHUB_TOKEN`. Cargo-binstall uses it for authenticated release discovery,
avoiding anonymous API rate limits that can trigger unnecessary source builds.
This adds no token permissions and does not install system dependencies.

crates/cargo-anvil/templates/justfiles/anvil/checks/doc-test.just:29

• This map uses PowerShell's default case-insensitive comparer, so a workspace containing case-distinct package names (for example Foo and foo) can overwrite one package's target classification with the other's. The impact formatter already uses an ordinal comparer for this exact case (crates/cargo-anvil/templates/justfiles/anvil/impact.just:530-540); use the same comparer here so a binary-only package cannot be selected or skipped based on a different package.

        $packages = @{}

crates/cargo-anvil/templates/justfiles/anvil/checks/doc-test.just:24

• Please refresh the full-tree snapshots for this generated change. crates/cargo-anvil/tests/snapshots.rs:11-23 says these tests pin every emitted file byte-for-byte, but tests/snapshots__local_only.snap:1135-1142, tests/snapshots__github_backend.snap:2451-2458, and tests/snapshots__ado_backend.snap:2309-2316 still contain the old two-command recipe; the GitHub snapshot also lacks the new setup-action env blocks (tests/snapshots__github_backend.snap:647-682). The expected generated trees therefore do not match the changed templates.

    # Explicitly selecting a binary-only package makes cargo test --doc fail.
    if ($pkg -notcontains '--workspace') {

• Files reviewed: 11/12 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical token-exposure and installation-scoping issues remain in the cargo-anvil setup.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

crates/cargo-anvil/templates/github/setup-action.yml:155

• The impact composite does not install cargo-delta inside this setup step: templates/github/impact-action.yml follows anvil-setup group=none with a separate run: just anvil-tool-cargo-delta-install binstall step that has no GITHUB_TOKEN environment. Consequently the impact job's binstall lookup remains anonymous and can still hit the rate limit or fall back to a slow source build; propagate the token to that step as well and regenerate the action.

        # Authenticate binstall's release discovery before it falls back to
        # source builds. The tool installer drops it before any source install.
        GITHUB_TOKEN: ${{ github.token }}

crates/cargo-anvil/templates/github/setup-action.yml:130

• GITHUB_TOKEN is still present when this cargo binstall call starts, and removing it only after a nonzero result is too late: a missing prebuilt release can take binstall's compile strategy, exposing the token to the just crate's build scripts. Add --disable-strategies compile to this call so a release miss falls through to the tokenless cargo install below.

          & cargo binstall --no-confirm --locked --version ">=$minimum" just
          if ($LASTEXITCODE -ne 0) {
            # Drop the release-discovery token before compiling third-party code.
            Remove-Item Env:GITHUB_TOKEN -ErrorAction SilentlyContinue

crates/cargo-anvil/templates/justfiles/anvil/checks/doc-test.just:25

• The title and description scope this PR to cargo-each's Windows PATH lookup, but this hunk also changes the cargo-anvil doc-test contract; the PR additionally changes cargo-anvil workflow token handling and generated snapshots. Please either split the unrelated cargo-anvil work into a separate PR or update the description and review scope to cover it.

    # Explicitly selecting a binary-only package makes cargo test --doc fail.
    if ($pkg -notcontains '--workspace') {
        $metadataJson = & cargo {{_anvil_stable_toolchain_args}} metadata --no-deps --format-version 1

crates/cargo-anvil/templates/justfiles/anvil/tools.just:458

• The title and description scope this change to cargo-each's Windows child lookup, but this hunk (along with the doc-test, template, action, and snapshot changes) also changes cargo-anvil installation, GitHub token exposure, and generated CI behavior. Please split these unrelated changes into a separate PR or update the PR description and acceptance scope before approval so the security/CI changes can be reviewed under their actual purpose.

    # The step-scoped GITHUB_TOKEN exists only so cargo-binstall can query
    # GitHub Releases as an authenticated caller instead of being rate-limited
    # anonymously. Keep it out of every source install: those compile
    # third-party crates and run their build scripts.
    $releaseToken = $env:GITHUB_TOKEN

• Files reviewed: 19/20 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Write-capable tokens are still passed to third-party installers and must be isolated or read-only.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

crates/cargo-anvil/templates/github/setup-action.yml:157

• The token passed here inherits the calling job's permissions. In pr-fast that includes pull-requests: write, and in the other group jobs it includes statuses: write; _install-tool-core then restores the token for cargo-binstall. A third-party installer therefore receives a write-capable token, which contradicts the read-only claim for non-pr-fast jobs and lets a compromised installer use release discovery as a PR/status write capability. Keep installation/check jobs read-only and isolate advisory/status publication, or provide a separately scoped token.

        # Authenticate binstall's release discovery before it falls back to
        # source builds. The tool installer drops it before any source install.
        GITHUB_TOKEN: ${{ github.token }}

crates/cargo-anvil/templates/justfiles/anvil/tools.just:479

• The new $releaseToken condition disables cargo-binstall's compile strategy for every GitHub binstall, including tools without a sourcePrereq. However, crates/cargo-anvil/docs/design/local.md:301-307 still states that tools without a source prerequisite retain that strategy, so the installer contract is now stale; qualify that documentation for tokenless/local runs or update it.

        if ($sourcePrereq -or $releaseToken) {
            $binstallArgs += @('--disable-strategies', 'compile')

• Files reviewed: 25/26 changed files
• Comments generated: 3
• Review effort level: Lite

[Evgenii (Vaiz)]

Picking up the suppressed findings from the automated reviews, since they do not appear as threads.

The case-insensitive map in doc-test.just was a real defect. $packages = @{} uses PowerShell's default comparer, so Foo and foo collapse into one key and a package could be classified by another package's targets. It is now a Dictionary[string, bool] with StringComparer::Ordinal, matching the precedent already set in impact.just.

The docs/design/README.md contradiction was real and in fact predates this PR: the GitHub backend has been installing through cargo-binstall for some time while §8.1 still claimed "No cargo-binstall". Rewritten to state the actual split — ADO installs from source only, GitHub uses binstall with a locked source fallback — plus the rules about when the token is dropped.

The cargo-each lookup rule is now in the crate-level docs in src/main.rs, not only the design doc, and crates/cargo-each/README.md has been regenerated through just anvil-readme --fix so the published documentation matches.

The impact-action.yml gap was correct too: that step installs cargo-delta outside the setup action and had no token, so its lookup stayed anonymous. It now receives the same token.

One suppressed suggestion I did not take: adding --disable-strategies compile only to the just bootstrap call. That is in, but the same reasoning applies to the catalog installer, so tools.just now disables the compile strategy whenever the release token is set rather than only when a source prerequisite exists.

[Copilot]

🟡 Changes recommended

Token-isolation regression coverage is missing, and a documentation clarification remains.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-anvil/docs/design/README.md:396

• This new sentence says the built-in token is exposed to the GitHub setup steps only, but the existing generated anvil-run-group action also exports GITHUB_TOKEN to group commands at .github/actions/anvil-run-group/action.yml:50-59. Narrow this wording to the new installation-step use so the security design does not imply that setup is the token's only exposure.

crates/cargo-anvil/templates/justfiles/anvil/tools.just:478

• The existing install_tool_controls_source_fallback_and_prerequisite_ordering contract test expects an ordinary binstall to retain the compile strategy, but run_just inherits GITHUB_TOKEN and .github/actions/anvil-run-group/action.yml:50-59 exports it for the whole Anvil group. With this condition, that test's ordinary_tool case takes the new token branch, making its assertion at recipe_contracts.rs:1744-1746 false. Make the fixture runner remove or explicitly clear GITHUB_TOKEN, then add a separate token-enabled case for this behavior.

        if ($sourcePrereq -or $releaseToken) {

• Files reviewed: 23/24 changed files
• Comments generated: 1
• Review effort level: Lite

[Evgenii (Vaiz)]

Answering the second suppressed finding from the latest review, on docs/design/README.md:396.

You are right and the sentence I wrote was wrong. It said the built-in token is exposed to the GitHub setup steps only, but anvil-run-group has exported GITHUB_TOKEN to group commands for some time (.github/actions/anvil-run-group/action.yml:50-59), so checks such as cargo-aprz get the authenticated quota. Presenting setup as the sole exposure understated the design.

Rewritten in 2f501ee to name both surfaces, and to keep the guarantee attached to the path it actually covers: on the installation path the token is removed before any source install and binstall's compile strategy is disabled while it is set, so no third-party build runs with the credential in scope.

[Copilot]

🔵 Needs a closer look

Broad CI, installer, generated-artifact, and Windows behavior changes include an unresolved moderate contract-coverage gap and require human review.

Review details

Suppressed comments (2)

crates/cargo-anvil/templates/github/setup-action.yml:132

• The separate Install just bootstrap now has its own token-present binstall-to-cargo install fallback, but the new GitHub artifact test only checks that the token is present; the token contract test exercises _install-tool-core, not this shell step. A regression removing this cleanup would leave tests green while the source build receives the credential, so add a focused contract or harness that records the environment for both calls.

            Remove-Item Env:GITHUB_TOKEN -ErrorAction SilentlyContinue

crates/cargo-anvil/templates/justfiles/anvil/tools.just:478

• This new condition changes the documented installer contract: when GITHUB_TOKEN is present, even tools without source prerequisites no longer retain binstall's compile strategy. However, crates/cargo-anvil/docs/design/local.md:301-306 still says those tools retain it unconditionally, while that document also says local and cloud invocations have the same behavior; please qualify the documented no-token case.

        if ($sourcePrereq -or $releaseToken) {

• Files reviewed: 23/24 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Evgenii (Vaiz)]

Both suppressed findings from the latest review are addressed in a6f3967, along with a Clippy failure I caused myself.

The Install just bootstrap had no coverage. Correct — that step is a shell step in the generated action, not a Just recipe, so install_tool_controls_source_fallback_and_prerequisite_ordering exercises _install-tool-core and never reaches it. A regression dropping its cleanup would have left every behavioral test green, which is exactly the failure mode worth pinning. setup_action_bootstraps_just_before_the_catalog now asserts that binstall cannot compile in that step and that Remove-Item Env:GITHUB_TOKEN sits after release discovery and before the cargo install fallback. These are static assertions on the emitted action rather than a behavioral harness — driving a composite-action shell step end to end would need machinery this PR does not have — but they pin the ordering the guarantee depends on.

docs/design/local.md:301-306 was falsified by this change. Also correct. It said tools without a source prerequisite retain binstall's compile strategy unconditionally, which stopped being true once the token disables it for every tool. Rewritten to state the condition, and to name the consequence the doc's own "same behavior locally and in the cloud" claim implies: a local run normally has no token and sees the retain-compile path, a GitHub runner does not.

Separately, my own regression. Adding the token case pushed the installer contract test to 151 lines, past this repository's clippy::too-many-lines threshold, and failed all four Fast Checks legs. I split the token isolation into its own test behind a shared fixture rather than adding an allow — the two contracts read better apart anyway. Clippy with --all-targets --all-features --locked -D warnings is clean now; skipping it before the previous push is what let this through.

[Copilot]

🔵 Needs a closer look

Add recipe-specific error handling for malformed Cargo metadata in the doctest recipe.

Review details

Suppressed comments (1)

crates/cargo-anvil/templates/justfiles/anvil/checks/doc-test.just:27

• This parse is not wrapped with the recipe-specific diagnostic used by the sibling metadata consumers (for example, checks/examples.just:39-43). If Cargo returns malformed JSON while exiting successfully, the global Stop preference makes PowerShell surface its raw ConvertFrom-Json error, so the new hard-error path does not identify that anvil-doc-test failed to parse Cargo metadata. Wrap this assignment in try/catch, emit anvil-doc-test: could not parse cargo metadata output: ..., and exit nonzero.

        $metadata = $metadataJson | ConvertFrom-Json

• Files reviewed: 24/25 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Evgenii (Vaiz)]

Taking the suppressed finding on doc-test.just:27 — it is correct, and the inconsistency was mine.

Every sibling metadata consumer wraps the parse and names itself: examples.just, semver-check.just, external-types.just and bolero.just all emit <recipe>: could not parse cargo metadata output: $_. The hunk I added did not, so with $ErrorActionPreference = 'Stop' a successful Cargo run returning malformed JSON would surface PowerShell's raw ConvertFrom-Json error and never identify that anvil-doc-test was the thing that could not read it. Fixed in a83bb9e, matching the sibling pattern exactly. I also gave the metadata command's own nonzero exit the same treatment, which those siblings have and this recipe was missing too.

Rather than just fixing the one recipe, I strengthened the shared contract: public_api_checks_fail_when_metadata_discovery_fails previously only asserted that malformed metadata made the recipe fail, which is precisely the assertion that stayed green while the diagnostic was absent. It now also asserts each recipe names itself. All three recipes it covers satisfy that today, so a future one added without a diagnostic is caught rather than silently accepted.

For the record on the previous head: a6f39677 cleared the Clippy failure — the rollup went to 18 success, 0 failing, 9 pending before this push.

[Copilot]

🟡 Changes recommended

Unresolved critical, moderate, and nit findings remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

crates/cargo-anvil/docs/design/README.md:399

• The global security description says the token reaches only two generated surfaces, but this change also exports it in .github/actions/anvil-impact/action.yml:36-40 so the cargo-delta binstall step can use authenticated release discovery. Please include that impact-installation surface here; otherwise the documented token boundary omits one of the newly changed paths.

- The built-in `GITHUB_TOKEN` reaches two generated surfaces. The run-group action
  has long exported it so checks that call GitHub's API get the authenticated
  quota, and the setup steps now receive it so `cargo-binstall` can discover
  releases as an authenticated caller instead of hitting anonymous rate limits.

crates/cargo-anvil/templates/justfiles/anvil/checks/doc-test.just:25

• This metadata probe is not locked, although both doctest invocations below are explicitly --locked. On a workspace with a missing or stale lockfile, cargo metadata can resolve and update Cargo.lock before the test, so the recipe can repair the lockfile and bypass the lockfile validation that the check is meant to enforce. Pass --locked to this metadata command as well so package classification uses the same read-only lock contract.

        $metadataJson = & cargo {{_anvil_stable_toolchain_args}} metadata --no-deps --format-version 1

justfiles/anvil/checks/doc-test.just:24

• This new metadata-filtering path makes the cargo-each design contract stale: crates/cargo-each/docs/design/README.md:306-308 explicitly lists doc-test among recipes that become a single cargo each … --once line, while the new recipe now calls cargo metadata and classifies targets. Please update that design documentation (or revise the claim) in the same change so the documented Anvil integration matches the implementation.

    # Explicitly selecting a binary-only package makes cargo test --doc fail.
    if ($pkg -notcontains '--workspace') {

• Files reviewed: 24/25 changed files
• Comments generated: 1
• Review effort level: Lite

[Evgenii (Vaiz)]

🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.

Correct, and worth stating plainly: the old assertion was passing for the wrong reason. tools.just contains a second, unrelated if ($sourcePrereq) block further down — the one that runs the prerequisite recipe before the source install — so the contains check kept matching even though the guard it claimed to describe had become if ($sourcePrereq -or $releaseToken).

Fixed in #174, where this work has been re-landed on current main. The assertion now pins the guard together with the body it controls:

TOOLS_JUST.contains(concat!(
    "if ($sourcePrereq -or $releaseToken) {\n",
    "            $binstallArgs += @('--disable-strategies', 'compile')\n",
    "        }"
))

Verified the way it should have been the first time: I weakened the guard back to if ($sourcePrereq), confirmed the new assertion fails, and restored it.

[martin-kolinek]

This is a duplicate of changes made in this PR: #153

[Evgenii (Vaiz)]

🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.

You are right, and thank you — I checked rather than taking it on faith, and #153 merged 16 minutes before you commented. origin/main now carries exactly this in execute():

let mut command = Command::new(program);
if let Some(path) = std::env::var_os("PATH") {
    command.env("PATH", path);
}

That is the whole of what this PR's title promises, so the headline change here is redundant.

It is worse than redundant, in fact. This branch is based on the pre-#153 main, so as it stands its diff against current main removes 1207 lines under crates/cargo-each — your target-aware selection, filter expressions and per-target execution included. Merging it as-is would revert #153, which is presumably why the PR has just gone to DIRTY. So this needs restructuring, not a routine rebase.

What is still unique here is the Anvil CI work that got pulled in along the way: the scoped doctest recipe no longer passes binary-only packages to cargo test --doc, tool installation authenticates its release discovery while keeping the token out of source builds, and three cargo-gamma-lib doctests that only ever compiled under --all-features are gated. None of that is in #153.

I have deliberately not rebased or force-pushed, since choosing between "drop the cargo-each changes and keep the Anvil repairs" and "close this and re-land the Anvil work separately" is my author's call, not mine. I have flagged it to them. Sorry for the duplicated effort on your side.

[Evgenii (Vaiz)]

🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.

Resolved by dropping this change entirely rather than rebasing it.

I checked what was actually left that main does not already have. The PATH forwarding is yours, in main, and so is a regression test for it — relative_child_program_uses_path_before_cargo_each_directory, which compiles two real probes and asserts the adjacent one is not the one that runs. Nothing here improved on that, so there was nothing to keep.

What was unique was the Anvil CI work that got pulled in along the way, and that has been re-landed as #174 on top of current main, with everything under crates/cargo-each dropped. This PR is superseded; no rebase of it will happen. Thanks again for catching it early — it would have been a quiet revert of #153 otherwise.

[Evgenii (Vaiz)]

Superseded by #174.

martin-kolinek was right: #153 landed the PATH forwarding independently, and this branch predates it, so its diff against current main had become a revert of #153's target-aware selection rather than an addition. A rebase would not have fixed that cleanly, and the history here no longer describes what the change is.

#174 is the remaining unique half — the Anvil CI repairs — rebuilt from scratch on top of current main as a single commit. Everything under crates/cargo-each is dropped; main owns that behaviour and already carries a regression test for it (relative_child_program_uses_path_before_cargo_each_directory).

Both open threads here are carried over and answered inline. This PR can be closed in favour of #174.