Repository navigation
Pin the release tag to the commit that declares its version - #27
Merged
Merged
Conversation
Both come out of the 0.9.20 recovery and neither is visible from the code. The `status` job asks whether the release exists BEFORE it bumps `Latest` and nothing looks again afterwards, so deleting a release after the job has run leaves `status.txt` naming a version with no release -- the early-bump failure the automation was written to prevent, arriving by a route it does not cover. The remedy is a revert of `status.txt` or a re-created release, and it belongs beside the automation's guarantee rather than in a commit message. The recovery dispatch also re-applies the crafted notes with `gh release edit --title --notes-file` whenever the tag has a file on `release-notes`, so a body edited by hand on GitHub is thrown away. Fixing the file before dispatching, not the release after, is part of the procedure; this session got the order right by accident. The same step creates the release when there is none, which is what made a deleted release recoverable at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
The sibling repos' copy of this script resolves the tag to a commit whose tree declares the prepared version, rather than to the branch head, and refuses to fall back silently. This copy never had it -- it was taken from them in e894a3a, before the pin existed -- and that is the whole mechanism behind issue #21. PK-0.9.19 was published 50 minutes before its own version bump merged, so the tag landed on 3916815, whose csproj still said 0.9.18.0. The release workflow checks out the tag, so it built 0.9.18: the downloads are WinCompose-Setup-0.9.18.exe and WinCompose-NoInstall-0.9.18.zip (real 0.9.19 builds, wrong number), the About tab reads 0.9.18, and the update file could not be bumped at all without offering every install an endless update to itself. PK-0.9.20 repeated it on 7 October. With the pin, a publish whose bump HAS merged lands the tag on the bump commit and the build produces correctly-named assets -- verified here: a dry-run of the prepared PK-0.9.20 now resolves target 113093c (declares 0.9.20) instead of main's tip. PK-0.9.20 itself still needs a workflow_dispatch, because its tag already exists at the pre-bump commit and a tag is not re-pointed by publishing. Brings over three more things this copy had fallen behind on: - make_latest is no longer hardcoded true. Only the newest prepared tag takes the Latest badge, so publishing an older one with --tag cannot re-point releases/latest -- which Updater.cs resolves for its download links -- at an older build. - The empty-notes guard covers the auto-select path, not just --tag. - run() reports a missing executable as an ordinary non-zero result, which subsumes the try/except this copy had grown around get_token(). Keeps this copy's own create-time guard, which the siblings lacked and which covers the case the pin cannot: when NO commit declares the version, there is nothing to pin to, and creating the release is refused rather than shipping assets labelled with the old version. All three copies are byte-identical again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
Nothing in CI reads Markdown, so a stale procedure ships silently and reads as authoritative. RELEASE.md said the tag is created at `main`'s tip and that the csproj version is "read only to print a note when the two disagree". Both were true of this copy of the script and are no longer: the tag is pinned to the commit that declares the version, drift past it is harmless, and a version no commit declares is refused. Step ① now says publishing is refused rather than that it ships the previous version, and step ③ separates which version gets published (the notes branch) from where the tag lands (the csproj). CLAUDE.md's "Not fixed structurally, deliberately" claimed the only structural fix was deriving the version from the tag via GitVersion. That is still the deeper answer and still blocked on having no .NET toolchain, but it is no longer the ONLY one -- the pin makes the tag land on a commit whose tree declares the version, which is structural for the half that caused #21. Rewritten to say which half each fixes. Adds two things a future session would otherwise have to rediscover: that the gate must NOT fire on the tree merely differing from the tag (the firmware tree is normally ahead, and the pin already makes that safe, so refusing there blocks almost every legitimate publish), and that the script is byte-identical across the three repos with nothing checking it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
![revixappdev[bot]](https://avatars.githubusercontent.com/in/3743861?s=60&v=4){kind=small}
There was a problem hiding this comment.
- [FIX] Release tag targets: Tags now point to the oldest default-branch commit whose version file declares the prepared release version, instead of the branch head. If no matching commit is found, the script reports the fallback and refuses to create a mismatched release unless
--allow-version-mismatchis used. - [FIX] Latest release badge: Only the newest prepared tag is marked Latest, preventing publication of an older tag from moving
releases/latestbackwards. - [FIX] Release notes and CLI errors: The non-empty notes check now covers both automatic selection and
--tag; missing executables return a non-zero result instead of raising anOSErrortraceback. - [DOCS] Release guidance:
CLAUDE.mdandRELEASE.mdexplain version-based tag pinning, refusal behavior, shallow-clone recovery, and release recovery details. - This PR isn't getting a full Revix review right now — reviews are free for open-source projects but limited to a few a day, and this organization has already used today's. Reviews resume on Oct 08 at 00:00 UTC.
Free open-source reviews are in cooldown — Revix reviews open-source pull requests for free every day, and the summary above is still on us. Want every PR reviewed? https://www.revix.ai
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Merged
2 tasks
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
urllib's default is the socket default, which is None -- block forever. So a connection GitHub accepts and then stops answering hangs the script rather than failing. That defeats release_exists()'s whole design. It is written to return None on any trouble so the caller takes the stricter path and treats the release as not existing; a hang reaches neither. It also hangs --dry-run, the one command that has to stay quick and safe, since the lookup happens before anything is printed. api() had the same defect, from before this change, and is fixed in the same pass: same one-line remedy, same file, and a partial fix would mean a second three-repo sweep later to keep the copies byte-identical. Its exposure is a real publish rather than a dry-run, where a person is at least watching. One module constant rather than a literal per call site, so the next urlopen cannot quietly omit it. Verified against a socket that accepts and never answers: release_exists() returns None after the configured timeout instead of blocking, and the TimeoutError lands in the existing handler. The same server with no timeout never returns. The three dry-runs still resolve their pinned commits, and the live lookup still answers True for an existing release and False for a tag without one. Found by Greptile on PolyKybdHost#318. Applied to all three copies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
All three found by Greptile, all three verified against the live repo. An EXISTING tag beats the pin, and the pin's own report hid it. `target_commitish` is documented as "Unused if the Git tag already exists" -- creating a release never moves a tag -- so when the tag is already there the build comes from wherever it points, and the commit the script prints is only a claim. PK-0.9.20 is the live case: its tag sits on the pre-bump 6cb987c, its release was deleted, and the pin resolves 113093c, so the script would have printed the right commit and published the wrong one. Creating is now refused when an existing tag points elsewhere.⚠️ This fires even when the version check passes, which is the point: there the pin found the correct commit and the mismatch guard sees nothing wrong. The Latest badge was decided from PREPARED NOTES rather than published releases. Staging notes for a version that has not shipped then withheld the badge from the release being published: `releases/latest`, which the download links resolve, stayed on an older release while CI raised `status.txt` to the new version -- every install offered the new one and handed the old one, which is the 0.9.18 failure again. It now compares against what GitHub calls latest. The original concern is preserved and tested: publishing an older tag with --tag still does not steal the badge. `--allow-version-mismatch` promised one consequence and the repos differ. The text said the workflow refuses the assets, which is true only of wincompose, whose release.yml asserts the built exe's version against the tag. The firmware workflow NAMES its assets from the tag (`VER="${TAG#PolyKybd-fw-v}"`, release.yml:202) and checks nothing, so the override there ships a .bin whose filename and reported FW_VERSION disagree -- the mis-flash class that sent a firmware bisect to a wrong conclusion for several rounds (samhocevar#258). The flag's --help and the warning now state both outcomes instead of one. Verified: the guard refuses the real PK-0.9.20 publish and stays silent on PK-0.9.19, whose release exists so nothing would be built; the badge decision was exercised both ways against the live `releases/latest`, including across the 0.9.x/0.10.x boundary; the three dry-runs still resolve their pinned commits; and the no-`gh` dry-run still works. The wincompose default dry-run now exits 1, which is the new guard firing on the real state of that tag rather than a regression. Firmware and host release workflows still have no assertion that the built version matches the tag. That is a workflow change, not a script one, so it is not in this scripts-only PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
CodeQL flagged `latest_why` as assigned and never used. It was the visible
symptom of two defects I introduced in the previous commit.
The reason was built and dropped, so the fallback path -- where the
published-releases lookup fails and the decision comes from the prepared
notes instead -- was silent. I had claimed in review that it explains
itself either way; it did not.
Worse, and not what CodeQL named: the line that reports a withheld badge
still referenced `newest_tag`, which now exists only inside that fallback
branch. So on the normal path a not-latest publish died with
UnboundLocalError -- on exactly the path `--tag` exists for:
publish_release.py --dry-run --tag PK-0.9.17
UnboundLocalError: cannot access local variable 'newest_tag'
The badge line now prints in both directions and carries the reason, which
is the thing worth seeing: it says whether the decision came from the
published releases or from the fallback.
⚠️ My test for the previous commit drove the predicate directly and never
ran main(), which is why a crash in the printing right beside it survived
a table of five passing cases. PolyKybdHost's own CLAUDE.md has the rule --
run the real entry point once before believing a mocked suite. Every
prepared tag now goes through main() in all three repos: 5 in wincompose
(4 pass, PK-0.9.20 refused by the tag guard, as it should be) and the
newest 4 in each sibling, all exiting 0 with the badge line reading NO for
older tags and YES for the newest.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
thpoll83
pushed a commit
that referenced
this pull request
Oct 7, 2026
Both came out of PK-0.9.20 after #27's docs were written, so the files lagged the code again -- the failure mode this whole change exists to fix. `target_commitish` is documented as "Unused if the Git tag already exists", so publishing never moves a tag: the build comes from wherever the tag points while the script reports the commit it wanted. Reached by an ordinary sequence -- publish early, delete the release, merge the bump, try again -- and PK-0.9.20 sat exactly there, tag on 6cb987c declaring 0.9.19 with the pin resolving 113093c.⚠️ The note says the check must fire even when the version gate passes, which is the part that is easy to get wrong: there the pin found a correct commit and the mismatch gate sees nothing wrong. RELEASE.md gains its own section for it, because the recovery it already described was the WORSE of the two. A dispatch attaches correct assets to the tag where it is, leaving `git checkout <tag>` on a tree declaring the previous version -- the kind of thing that sends a bisect wrong. Moving the tag and publishing normally fixes the cause, and is what actually resolved 0.9.20; the commands are there, with the caveat to check no release exists on the tag before force-pushing. Also corrects the window in ④ from "the window in between" to ~4 hours, now that it has an end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
thpoll83
added a commit
that referenced
this pull request
Oct 9, 2026
* A failed tag lookup must refuse, not skip the guard `tag_commit()` returned None both for a confirmed 404 and for any failure, and the caller SKIPS its check when there is no tag -- so a 5xx, a 403 or a timeout read as "no tag" and let a publish through at whatever commit the tag really points at, which is the thing the guard exists to prevent. That is the same shape as the failed-lookup bug already fixed once in this script, in release_exists(), where only a 404 counts as an answer. I reproduced it in the helper written to fix the related bug, one function away from the comment warning about it. Three outcomes now, matching published_latest_version(): a sha, "" for a confirmed 404, None for "cannot tell". The annotated-tag dereference and any ref shape the code does not understand also return None rather than guessing. The caller refuses when it cannot check, naming that an existing tag is never moved by publishing, so there is nothing safe to assume. The sha comparison is exact now (`at != target`) instead of a `startswith` on the first ten characters; both sides are full 40-char shas, one from `git log --format=%H` and one from the API. Tested through main() with the lookup stubbed to each outcome: 5xx refuses, 403 refuses, a 404 correctly skips the guard and proceeds, and a tag genuinely at the wrong commit refuses naming both shas. Plus the regression run -- the newest four prepared tags through main() in each of the three repos, all exiting 0 with the right Latest verdict. Found by Greptile on PolyKybdHost#318 and qmk_firmware#356. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i * Document the existing-tag rule and the tag-move recovery Both came out of PK-0.9.20 after #27's docs were written, so the files lagged the code again -- the failure mode this whole change exists to fix. `target_commitish` is documented as "Unused if the Git tag already exists", so publishing never moves a tag: the build comes from wherever the tag points while the script reports the commit it wanted. Reached by an ordinary sequence -- publish early, delete the release, merge the bump, try again -- and PK-0.9.20 sat exactly there, tag on 6cb987c declaring 0.9.19 with the pin resolving 113093c.⚠️ The note says the check must fire even when the version gate passes, which is the part that is easy to get wrong: there the pin found a correct commit and the mismatch gate sees nothing wrong. RELEASE.md gains its own section for it, because the recovery it already described was the WORSE of the two. A dispatch attaches correct assets to the tag where it is, leaving `git checkout <tag>` on a tree declaring the previous version -- the kind of thing that sends a bisect wrong. Moving the tag and publishing normally fixes the cause, and is what actually resolved 0.9.20; the commands are there, with the caveat to check no release exists on the tag before force-pushing. Also corrects the window in ④ from "the window in between" to ~4 hours, now that it has an end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i * Point the tag-move recovery at the refusal, not a line that never prints The instruction said `--dry-run` prints the commit to use on its `target:` line. It does not, in the one case the section is about: the tag check dies before the summary is printed, so a dry-run on a misplaced tag shows the refusal and nothing else. Reproduced with the lookups stubbed -- no `target:` line in the output, and the refusal naming both commits -- which is also exactly what the maintainer's own paste of the PK-0.9.20 refusal showed earlier. I wrote the sentence with that evidence already on screen. Describes the ordering rather than citing line numbers, which is the stale-prose hazard this change exists to reduce. Found by Greptile on #28. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Closes the mechanism behind #21, which has now produced two mislabelled releases.
The mechanism
scripts/publish_release.pycreated the tag atmain's tip.release.ymlchecks out the tag. So a version bump still sitting on a branch meant the release built the previous version:cf7e91emerged at 10:35. The tag landed on3916815, whose csproj said0.9.18.0— downloads namedWinCompose-Setup-0.9.18.exe, About tab0.9.18, andstatus.txtunbumpable without offering every install an endless update to itself.The sibling repos' copy of this script already resolved the tag to a commit whose tree declares the version. This copy was seeded from them in
e894a3a, before that existed — which is why the bug landed here and not there.What changed
commit_for_version()comes over, so the tag is pinned to the commit that declares the version and a csproj that has drifted forward since the notes were prepared is harmless. Verified: a dry-run of the preparedPK-0.9.20now resolvestarget: 113093cfa2 (declares 0.9.20)instead ofmain's tip.Three more things this copy had fallen behind on:
make_latestis no longer hardcodedtrue. Only the newest prepared tag takes the Latest badge, so--tagon an older one cannot re-pointreleases/latest— whichUpdater.csresolves for its download links — at an older build.--tag.run()reports a missing executable as a non-zero result, which subsumes thetry/exceptthis copy had grown aroundget_token().This copy's own create-time guard stays, because it covers what the pin cannot: when no commit declares the version there is nothing to pin to, and creating the release is refused rather than shipping assets labelled with the old version.
scripts/publish_release.pyis byte-identical across the three repos again (md5sum */scripts/publish_release.py). It had diverged in both directions at once — the siblings held the pin and themake_latestfix, this copy held the create-time gate, and each was the newer one for a different thing.Docs
The pin made three prose claims stale, and nothing in CI reads Markdown:
RELEASE.md① said publishing before the bump merges ships the previous version; it is now refused.RELEASE.md③ said the csproj version is "read only to print a note"; it decides where the tag lands.CLAUDE.md's "Not fixed structurally, deliberately" claimed GitVersion was the only structural fix. It remains the deeper answer and remains blocked on having no .NET toolchain, but it is no longer the only one.Also records two things from the 0.9.20 recovery that the code cannot show: deleting a release leaves
status.txtnaming a version with no release (thestatusjob checks existence before it bumps and never looks again), and a recoveryworkflow_dispatchre-applies the notes fromrelease-notes, so a wrong body is fixed on the branch first rather than on the release afterwards.Testing
No .NET in the container, so the Python is what was exercised:
--allow-version-mismatch, note on an already-published release.release_exists()against the live API:PK-0.9.19→ True,PK-0.9.20→ False, unreachable → None.--dry-runin all three repos, and once withPATHstripped ofghto re-check the regression Greptile found on 🟢 Bump to 0.9.20, and stop a release labelling itself with the old version #25.git fetch origin --unshallowfirst — told to merge a bump that merged months ago, the obvious next move is--allow-version-mismatch, which publishes the release the gate exists to prevent.Deliberately not gated on the tree merely differing from the tag: every merge auto-bumps in the sibling repos, so their tree is normally ahead of a prepared tag and the pin already makes that safe. The condition is "no commit declares it".
Review
Per
CLAUDE.md, all three bots are unavailable on this repo and each goes quiet in a way that looks like a pass — so a green board here means it compiles, and there is nothing to compile in this diff. CodeRabbit can be asked with@coderabbitai review.🤖 Generated with Claude Code
https://claude.ai/code/session_015jui4uFYS3SXuhXSRgew9i
Generated by Claude Code
Summary by Sourcery
Pin releases to commits that declare their version and harden publishing safeguards so release labels, assets, and update links remain consistent.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
The PR appears safe to merge; the older-release publishing crash is fixed.
What we checked:
latest_why. Every comparison branch assigns it before the message runs.Summary
The PR pins new release tags to commits that declare their version, checks existing tags before creating releases, and decides the Latest badge from published releases. It also improves publishing errors and updates release guidance.
Reviews (4) · Last reviewed commit: "Print the Latest-badge reason, and stop ..." · Reviewed by Greptile