fix: a version collision published nothing and said nothing - #595
Merged
Conversation
#591 and #593 both bumped to 0.36.0. Git merged that without a conflict -- picking the same number means both sides wrote the identical version line, and the identical `## [0.36.0]` heading, so there was nothing to resolve -- and the second merge published nothing at all. `publish.yml` found the tag and reported `nothing to publish`, which is also what it says for every ordinary push, so the fix landed on main released nowhere with no check red and no line to act on. Two guards, because one place can see it and the other cannot. `scripts/version_untaken.py` runs on the pull request, and it is the one that would have caught #591: compare the version at `pull_request.base.sha` with the branch's, and if the branch proposes a bump, refuse a version that is already tagged. `base.sha` is the base as of the pull request rather than the moving tip of main -- for #591 it was 908a24e, before the rival release, where the version was still 0.35.0 -- so the comparison sees 0.35.0 -> 0.36.0 and asks the only question that matters about it. `test_the_collision_that_happened_is_refused` replays #591's own two manifests rather than a hand-built imitation. `publish.yml` gets the narrower half, and the commit message should be honest about how narrow. It now separates a re-run over the commit it already published (skip, idempotent) from a push that moves the version *onto* somebody else's tag (error, with the reason). That second arm would **not** have caught #591: by then the merge carried the same version as its first parent, so there was no bump left to see. `test_the_case_that_needs_the_pull_request_guard` pins that fact rather than leaving it as a claim in a comment. The decision step is extracted from the workflow and run as the shell it is, so the four arms are tested against the real text instead of a reimplementation that would pass while the original rotted. Claude-Session: https://claude.ai/code/session_01XvY78XEnrkzNjxZE5EtnyW
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideThe PR prevents silent version-collision releases with two complementary guards: a fail-closed pull-request check compares the proposed version against existing tags before merge, while publish.yml provides narrower defense in depth and clearer failure behavior for collisions visible on main. Tests execute the real workflow shell and cover both the historical collision and the cases each guard can or cannot detect. Sequence diagram for version collision preventionsequenceDiagram
participant PR as PullRequest
participant CI as CIWorkflow
participant Guard as version_untaken.py
participant Git as GitTags
participant Main as MainPush
participant Publish as publish.yml
PR->>CI: Open or update pull request
CI->>Guard: Compare base Cargo.toml and branch Cargo.toml
Guard->>Git: Check refs/tags/v<proposed_version>
alt version is already tagged
Git-->>Guard: Tag exists
Guard-->>CI: Fail with collision reason
CI-->>PR: Block merge
else version is unused
Git-->>Guard: Tag absent
Guard-->>CI: Pass
CI-->>PR: Allow merge
end
Main->>Publish: Run after push
Publish->>Git: Check version tag
alt tag is on HEAD
Git-->>Publish: Existing tag on current commit
Publish-->>Main: Skip idempotently
else version changed onto another existing tag
Git-->>Publish: Existing tag elsewhere
Publish-->>Main: Fail with collision error
else no relevant tag
Git-->>Publish: Tag absent
Publish-->>Main: Continue publishing
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
`test_the_collision_that_happened_is_refused` and its neighbour replayed #591's real manifests by SHA, which fails everywhere but a full clone: `actions/checkout` fetches depth 1, so `git show 908a24e:rust/Cargo.toml` is `fatal: path exists on disk, but not in ...`. Reading the real commits was wrong for a second reason the failure did not show. The guard's oracle is `git rev-parse refs/tags/v<version>` in whatever directory it runs in, and those tests ran it in the checkout -- which has not fetched tags by the time pytest runs, since `pixi run ci` is the step before the one that fetches them. The collision test would have gone green by the guard finding no tag and permitting the bump, which is the opposite of what it asserts. Each test builds its own repository and tags it now. The numbers and the shape are #591's still; only the bytes are the test's own. The module docstring said the replay was the point, so it says the opposite now and why. Also stops `test_publish_decision.py` hardcoding `PATH` for the shell it runs: the script needs `git` and `sed`, and naming three directories is a test that passes here and fails on a runner that puts them elsewhere. Claude-Session: https://claude.ai/code/session_01XvY78XEnrkzNjxZE5EtnyW
Merged
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
1 participant
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.
Follow-up to #594, which cleaned up the consequence of a version collision without doing anything about the cause.
What happened
#591 and #593 both bumped to 0.36.0. Git merged that without a conflict, because picking the same number means both sides made the identical edit: one
version = "0.36.0"line inrust/Cargo.toml, and one byte-identical## [0.36.0]heading inCHANGELOG.mdwith the differing bodies merged in underneath as separate additions. Had #591 picked 0.37.0,Cargo.tomlwould have conflicted loudly and it would have been caught at the merge.The second merge then published nothing:
That is also what the workflow says for every ordinary push, so the
dl -- <cmd>quoting fix sat onmainreleased nowhere, with no check red.Two guards, because only one place can see it
scripts/version_untaken.py, on the pull request. This is the one that would have caught #591. It compares the version atpull_request.base.shawith the branch's; if the branch proposes a bump, the proposed version must not already be tagged.base.shais the base as of the pull request rather than the moving tip ofmain— for #591 it was908a24e, the commit before #593 landed, where the version was still 0.35.0. So the comparison sees0.35.0 -> 0.36.0and asks the only question worth asking about it.test_the_collision_that_happened_is_refusedreplays #591's own two manifests rather than a hand-built imitation:The four cases it has to get right: an ordinary branch that never touches the version passes, a release cut to an unused version passes, a bump onto a taken version fails, and an unreadable manifest fails closed.
publish.yml, and this half is narrower than it looks. It now separates a re-run over the commit it already published (skip, idempotent —gh release create --target $GITHUB_SHAis what puts the tag on HEAD) from a push that moves the version onto somebody else's tag (error, with the reason).That second arm would not have caught #591, and the tests say so rather than leaving it as a claim in a comment. By the time the merge reached
mainit carried the same version as its first parent — both 0.36.0 — so there was no bump left to see.test_the_case_that_needs_the_pull_request_guardpins exactly that. The arm is defence in depth for the collision shape that is visible there, not a second answer to this one.The tag-existence check is still asked of the ref alone and never of the commit behind it: this is a
fetch-depth: 2clone, so a tag pointing anywhere but HEAD may have no commit object to resolve, and a failed resolve must not read as "free to publish".Testing the workflow as the shell it is
test_publish_decision.pyextracts therun:body frompublish.ymland executes it against throwaway repositories, one per arm. A reimplementation here would be a second hand-maintained copy of the logic and would keep passing while the real step rotted.What is still not covered
The convention is the other half and is not enforced by anything: every release for the last eight versions was cut on its own
release/X.Y.Zbranch doing nothing but the bump, which is why this is the first time it has happened. These guards make the collision loud; they do not make the bump land in the right kind of PR.Verification
776 passed in the Python guards,
prekclean, pylint 10.00/10 on the new script.🤖 Generated with Claude Code
Summary by Sourcery
Prevent version collisions from merging unnoticed and leaving releases unpublished by validating proposed versions in pull requests and reporting visible collisions during publishing.
New Features:
Bug Fixes:
Enhancements:
CI:
Tests: