fix(anvil): harden workspace checks - #107
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (97.7%) 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 #107 +/- ##
=======================================
- Coverage 97.7% 97.7% -0.1%
=======================================
Files 286 286
Lines 62460 62438 -22
=======================================
- Hits 61032 61010 -22
Misses 1428 1428
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e42622a to
3fdad6f
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
There was a problem hiding this comment.
Pull request overview
This pull request hardens cargo-anvil’s workspace-oriented checks by tightening the “what should be checked” contract (publishable libraries only where appropriate) and by delegating workspace iteration for formatting to a pinned cargo-each, improving robustness on large workspaces and across platforms.
Changes:
- Pin and install/validate
cargo-each, and use it to runrustfmtonce per workspace member manifest with--keep-going. - Update
external-typesandsemver-checkto skippublish = falselibraries (no registry consumer contract), and improve skip/exclusion messaging. - Update design docs, recipe-contract tests, snapshots, and the
.anvil.lockcatalog checksum to reflect the hardened behavior.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| justfiles/anvil/versions.just | Adds pinned cargo_each_version. |
| justfiles/anvil/tools.just | Adds install/validate recipes for pinned cargo-each. |
| justfiles/anvil/helpers.just | Clarifies modified-tier semantics as a skip gate (no package splicing). |
| justfiles/anvil/checks/semver-check.just | Skips non-publishable library crates and updates messaging. |
| justfiles/anvil/checks/fmt.just | Switches to cargo each --workspace --keep-going per-manifest cargo fmt. |
| justfiles/anvil/checks/external-types.just | Skips publish = false libraries; reports exclusions; checks publishable libs only. |
| justfiles/anvil/checks/cargo-sort.just | Tightens/clarifies rationale for --check-format and --grouped. |
| crates/cargo-anvil/tests/snapshots/snapshots__local_only.snap | Refreshes generated snapshot output for updated recipes/docs. |
| crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap | Refreshes generated snapshot output for updated recipes/docs. |
| crates/cargo-anvil/tests/snapshots/snapshots__ado_backend.snap | Refreshes generated snapshot output for updated recipes/docs. |
| crates/cargo-anvil/tests/recipe_contracts.rs | Adds recipe contract tests for fmt via cargo-each and publish filtering behavior. |
| crates/cargo-anvil/templates/justfiles/anvil/versions.just | Mirrors cargo_each_version pin in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/tools.just | Mirrors cargo-each tool recipes in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/helpers.just | Mirrors helper-doc updates in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/semver-check.just | Mirrors semver publish filtering in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/fmt.just | Mirrors fmt delegation to cargo-each in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/external-types.just | Mirrors external-types publish filtering/reporting in templates. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/cargo-sort.just | Mirrors cargo-sort comment updates in templates. |
| crates/cargo-anvil/src/anvil/artifacts/justfile.rs | Ensures versions.just contract includes cargo_each_version. |
| crates/cargo-anvil/docs/design/local.md | Updates local design docs to match modified-tier and fmt execution model. |
| crates/cargo-anvil/docs/design/checks.md | Updates check catalog docs for fmt, cargo-sort, semver-check, and external-types. |
| .anvil.lock | Updates catalog checksum and file checksums for the changed catalog files. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Suppressed comments (7)
Previously missed (5) — in code that hasn't changed since the last review.
crates/cargo-anvil/docs/design/local.md:469
- The
anvil-fmtexample usescargo fmtwithout the pinned nightly toolchain, but the actual recipe invokes rustfmt via the pinned nightly (cargo '+{{ rust_nightly }}' fmt ...). This example should include the+<pinned-nightly>prefix to avoid suggesting a different/incorrect invocation.
cargo each --workspace --keep-going -- \
cargo fmt --manifest-path '{manifest}' --check
justfiles/anvil/checks/external-types.just:32
- This comment says "private libraries", but the code is specifically partitioning on Cargo metadata
publishbeing an empty array (i.e.publish = false/ non-publishable). Calling these "private" is ambiguous (it could be read as "published to a private registry").
# Build pkg-name -> manifest-path maps for publishable and private libraries.
crates/cargo-anvil/templates/justfiles/anvil/checks/external-types.just:32
- This comment says "private libraries", but the code is specifically partitioning on Cargo metadata
publishbeing an empty array (i.e.publish = false/ non-publishable). Calling these "private" is ambiguous (it could be read as "published to a private registry").
# Build pkg-name -> manifest-path maps for publishable and private libraries.
crates/cargo-anvil/docs/design/checks.md:173
- The
semver-checkrow says "Private and bin-only crates..." but the implemented filter is specificallypublish = false(Cargo metadatapublish: []). Consider spelling that out to match the actual semantics and avoid ambiguity with "private registry" crates.
| `semver-check` | `cargo semver-checks --baseline-rev <baseline>` per affected publishable library crate. Private and bin-only crates have no registry consumer contract and are skipped. The PR target is the baseline. Exit 100 is a completed check with deny-level findings; exit 101 or another nonzero status means the comparison was inconclusive. Both outcomes write `target/anvil/comments/semver.md` and remain advisory, matching the repository's native `semver` job (`continue-on-error: true`). Proven rename and bin→lib transitions with no comparable baseline are skipped without a comment. Anvil preflight failures such as invalid current-workspace metadata or an unavailable baseline ref still fail because the recipe cannot establish what to compare. | oxidizer-github |
crates/cargo-anvil/tests/recipe_contracts.rs:687
- This test name uses "private", but the scenario being exercised is
publish = false/ non-publishable crates. Renaming to "non_publishable" would better match the behavior and avoid confusion with crates published to a private registry.
This issue also appears in the following locations of the same file:
- line 715
- line 770
fn external_types_checks_every_publishable_library_and_reports_private_ones() {
crates/cargo-anvil/tests/recipe_contracts.rs:715
- This assertion message says "private" but the filtered-out case is
publish = false/ non-publishable libraries. Using the same terminology as the recipe output ("non-publishable") makes failures easier to understand.
"private library filtering failed\nstdout:\n{}\nstderr:\n{}",
crates/cargo-anvil/tests/recipe_contracts.rs:770
- This assertion message says "private" but the filtered-out case is
publish = false/ non-publishable libraries. Using consistent terminology ("non-publishable") makes failures clearer and avoids confusion with private registries.
"private semver filtering failed\nstdout:\n{}\nstderr:\n{}",
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
Make dry-run project and compare the complete lockfile state, and run the dogfood regeneration gate on every pull request. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/src/plan.rs:391
- This rustdoc comment references a design doc section number ("§5"), but repo guidelines prohibit citing design sections from code/rustdoc because it couples the code to the doc’s structure. Please remove the section-number reference and link to the doc generally.
/// subsequent runs see the divergence as resolved
/// (`LeaveAlone`) until the template moves again — see
/// [`updates.md §5`](../../docs/design/updates.md).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/src/plan.rs:391
- Repo guidelines (AGENTS.md) discourage referencing design-doc section numbers from rustdoc/code comments. This link text includes a
§5section reference; please remove the section number to avoid coupling the code to the doc’s structure.
/// [`updates.md §5`](../../docs/design/updates.md).
Preserve the container execution refactor and path-containment checks while retaining dry-run manifest projection. Treat CRLF and LF lockfiles equivalently without relaxing canonical content checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
Sander Saares (sandersaares)
left a comment
There was a problem hiding this comment.
[Copilot speaking]
Published 11 findings. 5 findings follow up on existing discussion threads.
See diagnostics
| Diagnostic | Value |
|---|---|
| Cache | Miss |
Clarify modified-tier ownership, SemVer publishability, and Loom model invariants. Cover named-registry SemVer candidates and keep generated recipes and snapshots synchronized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4112f005-775f-4c4b-88ed-db8f0c5d1cf1
🤖 This hardens generic cargo-anvil checks surfaced while adopting Anvil in ox-sdk and by runtime-analysis failures in ox-tools.
Summary
--dry-rundetect exact.anvil.lockdrift—including stale checksums, missing inventory, catalog metadata, and formatting—and run the dogfood regeneration gate on every pull requestpublish = falsecrates with in-workspace consumers, while semver skipspublish = falsecrates because they have no registry baselineCargo Sort continues to enforce both dependency ordering and its manifest formatting convention with
--check-format.Validation
cargo test -p cargo-anvilcargo clippy -p cargo-anvil --all-targets --all-features -- -D warningscargo-gamma-libLoom models with no global preemption bound