fix(cosim): the excluded crate's lockfile named neither of the last two releases - #521
Merged
Merged
Conversation
…wo releases
`crates/rustynes-cosim/Cargo.lock` pinned the first-party crates at 2.6.17
while the workspace was 2.6.19. The drift started one release earlier:
v2.6.15 workspace 2.6.15 cosim lock 2.6.15 ok
v2.6.16 workspace 2.6.16 cosim lock 2.6.16 ok
v2.6.17 workspace 2.6.17 cosim lock 2.6.17 ok
v2.6.18 workspace 2.6.18 cosim lock 2.6.17 DRIFT
v2.6.19 workspace 2.6.19 cosim lock 2.6.17 DRIFT
The mechanism is the crate's exclusion, which is the whole reason it needs a
committed lockfile in the first place. No workspace command reaches an excluded
package, so its `Cargo.toml` version is bumped BY HAND at each release -- and
the hand step does not touch the lockfile beside it, which moves only when
somebody runs cargo in that directory.
`the_excluded_crates_lockfile_is_tracked` asserts the file is committed, because
without one CI re-resolves. That is necessary and not sufficient: a tracked file
can be stale, and cargo rewrites a stale one on the next build, so the committed
resolve is not the resolve anything actually uses. The tracked-ness test was
guarding the door while the contents drifted.
HOW IT SURFACED, which is the part worth keeping. It was not found by reading
the lockfile. The pinned oracle worktree at `~/.cache/rustynes-cosim/` -- checked
out detached at the v2.6.18 commit, and the source the co-simulation goldens are
exported from -- came up DIRTY during a branch cleanup, carrying an unstaged
`Cargo.lock` change. That looked like build residue to discard. It was cargo
correcting a stale file, at a commit where the committed one was already wrong.
Refreshed with `cargo metadata --manifest-path crates/rustynes-cosim/Cargo.toml`,
which is a minimal resolve: the diff is exactly six version lines, 2.6.17 ->
2.6.19, with no third-party churn.
The gate is `the_excluded_crates_lockfile_names_the_workspace_version`. It walks
`[[package]]` blocks rather than grepping `version = `, so a third-party crate
that happens to sit at the same number cannot stand in for a first-party one
that does not, and it FAILS CLOSED on fewer than five first-party entries --
a parse finding nothing must not read as agreement, which is this project's
most-repeated defect in its smallest form.
Demonstrated by three mutations, all CAUGHT:
* the actual file this release shipped (all six at 2.6.17)
* one entry wrong and the other five correct
* the lockfile emptied of `[[package]]` blocks -- caught by the fail-closed
clause rather than by the comparison, reporting "only 0 first-party
packages found"
Baseline green, `cargo fmt --all --check` clean, clippy clean on the harness.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe test harness now checks that first-party packages in ChangesLockfile Version Audit
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The new lockfile audit guards against version drift without an identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Antigravity review (Gemini via Ultra)Updates the excluded Blocking issues
Suggestions
Nitpicks
Automated first-pass review by |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
crates/rustynes-cosim/Cargo.lockpinned the first-party crates at 2.6.17while the workspace is 2.6.19.
Why it drifted
The crate is excluded from the workspace — which is the whole reason it needs
a committed lockfile. No workspace command reaches it, so its
Cargo.tomlversion is bumped by hand each release, and the hand step does not touch the
lockfile beside it. That file moves only when somebody runs cargo in that
directory.
the_excluded_crates_lockfile_is_trackedasserts the file is committed, becausewithout one CI re-resolves. Necessary, not sufficient: a tracked file can be
stale, and cargo rewrites a stale one on the next build — so the committed
resolve was not the resolve anything used.
How it surfaced
Not by reading the lockfile. The pinned oracle worktree under
~/.cache/rustynes-cosim/— detached at the v2.6.18 commit, and the source theco-simulation goldens are exported from — came up dirty during a branch
cleanup. That looked like build residue to discard. It was cargo correcting a
stale file, at a commit where the committed one was already wrong.
The fix
Refreshed with
cargo metadata --manifest-path crates/rustynes-cosim/Cargo.toml— a minimal resolve, so the diff is exactly six version lines with no
third-party churn.
The gate is
the_excluded_crates_lockfile_names_the_workspace_version. It walks[[package]]blocks rather than greppingversion =, so a third-party cratesitting at the same number cannot stand in for a first-party one that is not,
and it fails closed below five first-party entries — a parse that finds
nothing must not read as agreement.
Three mutations, all CAUGHT:
[[package]]blocks → caught by the fail-closedclause, reporting
only 0 first-party packages foundBaseline green afterwards, and the committed lockfile verified to read 2.6.19 at
all six entries.
Gates
cargo fmt --all --checkclean · clippy clean on the harness · all 5cosim_manifest_audittests pass. No emulation code changes, so AccuracyCoin144/144 and nestest 0-diff hold by construction.
🤖 Generated with Claude Code
https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
Summary by CodeRabbit