fix(#2420): automate v0 floating tag move in release workflow#2421
fix(#2420): automate v0 floating tag move in release workflow#2421rh-hemartin wants to merge 1 commit into
Conversation
Site previewPreview: https://ea8d4507-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 1:40 PM UTC · Completed 1:52 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
Labels: PR modifies the release CI workflow and release skill documentation. Previous runReviewFindingsMedium
Low
Labels: PR modifies CI release workflow and release skill documentation Previous runReviewFindingsMedium
Low
Labels: PR modifies the release CI workflow and release skill documentation. Previous run (2)ReviewFindingsMedium
Low
Info
|
ralphbean
left a comment
There was a problem hiding this comment.
A couple things to sort out before we merge. See inline comments.
|
Thanks for digging into this. |
0c869d8 to
2eb1a21
Compare
|
🤖 Finished Review · ✅ Success · Started 6:09 AM UTC · Completed 6:20 AM UTC |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} | ||
|
|
||
| - name: Move v0 floating tag |
There was a problem hiding this comment.
[medium] shell-error-handling
The new shell script in the Move v0 floating tag step does not use set -euo pipefail. All other multi-line run: blocks across the repository workflows consistently use it. Without set -e, a failed git tag -f would silently continue to git push with stale state.
Suggested fix: Add set -euo pipefail as the first line of the run: block.
|
|
||
| ## A. Wait for CI workflows | ||
|
|
||
| Wait for the Release workflow (triggered by the `v*` tag) and the |
There was a problem hiding this comment.
[medium] documentation-accuracy
The Sandbox Images trigger description says triggered by release workflow but sandbox-images.yml is triggered by a tag push matching v[0-9]+.[0-9]+* (the semver version tag), not by the release workflow.
Suggested fix: Change the parenthetical to (triggered by the version tag push).
| if: "!contains(github.ref_name, '-')" | ||
| run: | | ||
| if git rev-parse v0 >/dev/null 2>&1; then | ||
| CURRENT=$(git rev-parse v0) |
There was a problem hiding this comment.
[low] race-condition
The ancestry guard checks v0 position then force-pushes in a separate git operation. A concurrent release could move v0 between check and push. Practical risk is negligible since releases are manual and sequential.
Suggested fix: Replace git push origin v0 --force with git push --force-with-lease=v0: origin v0.
GoReleaser picked up the v0 floating tag as previous-tag reference, causing broken changelogs (previous=<unknown>). The manual tag move step was also easy to forget, leaving @v0 refs stale. Add a post-GoReleaser step to force-move v0 with an ancestry guard against race conditions. Add git.ignore_tags to .goreleaser.yml so v0 never confuses changelog generation. Remove the manual step from the cutting-releases skill. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Hector Martinez <hemartin@redhat.com>
2eb1a21 to
4ee125a
Compare
|
🤖 Finished Review · ✅ Success · Started 7:17 AM UTC · Completed 7:29 AM UTC |
|
|
||
| ## A. Wait for CI workflows | ||
|
|
||
| Wait for the Release workflow (triggered by the `v*` tag) and the |
There was a problem hiding this comment.
[medium] documentation-code-mismatch
The diff changes the Sandbox Images workflow trigger description to 'triggered by release workflow', but sandbox-images.yml triggers on semver tag pushes matching v[0-9]+.[0-9]+*, not by the release workflow. The old text was also inaccurate.
Suggested fix: Change the parenthetical to '(triggered by the semver tag push)'.
| exit 0 | ||
| fi | ||
| fi | ||
| git tag -f v0 "${GITHUB_SHA}" |
There was a problem hiding this comment.
[low] race-condition
The --force-with-lease push is an atomic server-side check, so concurrent releases fail safely. However, there is no retry mechanism, so v0 would remain not updated on failure. Practical risk is very low.
| if git rev-parse v0 >/dev/null 2>&1; then | ||
| CURRENT=$(git rev-parse v0) | ||
| if ! git merge-base --is-ancestor "${CURRENT}" "${GITHUB_SHA}"; then | ||
| echo "::warning::v0 already points at a newer commit, skipping" |
There was a problem hiding this comment.
[low] error-message-style
The ::warning:: message could include the current v0 SHA (${CURRENT}) and ${GITHUB_SHA} for easier debugging when the ancestry check skips the v0 move.
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} | ||
|
|
||
| - name: Move v0 floating tag |
There was a problem hiding this comment.
[low] missing-authorization
The automated v0 tag move removes a human confirmation checkpoint (AskUserQuestion) from SKILL.md step 8. The semver tag push itself serves as the authorization gate.
Summary
release.ymlto force-movev0tag, with ancestry check to prevent race conditions between concurrent releasesgit.ignore_tagsin.goreleaser.ymlsov0doesn't confuse previous-tag detection (root cause ofprevious=<unknown>/ 1,216-entry changelogs)Closes #2420
Test plan
release.ymlsyntax is valid (actionlint / CI).goreleaser.ymlparses correctly (goreleaser check)v0tag should move automatically without manual interventionv0.19.0-rc.1) should skip thev0move step🤖 Generated with Claude Code