-
Notifications
You must be signed in to change notification settings - Fork 55
fix(#2420): automate v0 floating tag move in release workflow #2421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,3 +32,22 @@ jobs: | |
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} | ||
|
|
||
| - name: Move v0 floating tag | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
| if: "!contains(github.ref_name, '-')" | ||
| run: | | ||
| set -euo pipefail | ||
| CURRENT="" | ||
| if git rev-parse v0 >/dev/null 2>&1; then | ||
| CURRENT=$(git rev-parse v0) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
| exit 0 | ||
| fi | ||
| fi | ||
| git tag -f v0 "${GITHUB_SHA}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 [[ -n "${CURRENT}" ]]; then | ||
| git push --force-with-lease="v0:${CURRENT}" origin v0 | ||
| else | ||
| git push origin v0 | ||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,15 @@ | |
|
|
||
| Part of the [cutting-releases](SKILL.md) skill. | ||
|
|
||
| Run after the version tag is pushed, the `v0` tag is moved, and the | ||
| CI workflows complete. Focus on the areas identified during pre-flight | ||
| Run after the version tag is pushed and the CI workflows complete. | ||
|
rh-hemartin marked this conversation as resolved.
|
||
| The release workflow automatically moves the `v0` floating tag after | ||
| GoReleaser succeeds (skipped for pre-release tags). Focus on the areas identified during pre-flight | ||
| step F. | ||
|
|
||
| ## A. Wait for CI workflows | ||
|
|
||
| Wait for the Release workflow (triggered by the `v*` tag) and the | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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)'. |
||
| Sandbox Images workflow (triggered by the `v0` tag move) to complete: | ||
| Sandbox Images workflow (triggered by release workflow) to complete: | ||
|
|
||
| ``` | ||
| gh run list --workflow=release.yml --limit=1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.