ci(release): prevent duplicate auto-release runs racing to the same version - #325
Merged
Conversation
…ersion When several PRs merge in quick succession, each CI completion triggers auto-release.yml. They ran concurrently, each computed the same next version from the same last tag, and each opened a bump PR (e.g. #323 and #324 both "chore: release v0.51.0"). One release.yml run published; the other hit the already-tagged guard and exited 1 — a spurious red ❌ on an otherwise-clean release. - auto-release.yml: serialize with `concurrency: auto-release` and, before opening the bump PR, skip if `v$VERSION` is already tagged or an `auto-release/v$VERSION` branch already exists (the queued run now observes what the earlier run created). - release.yml: the already-tagged path is an expected no-op, not an error — warn and exit 0 instead of exit 1 so a duplicate-PR re-trigger isn't red. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
WalkthroughAuto-release and release workflows now prevent duplicate PR creation by serializing auto-release runs via concurrency control, checking for existing tags or in-flight branches before creating PRs, and handling already-tagged cases as benign no-ops instead of failures. ChangesPrevent Duplicate Release PR Creation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #325 | ci(release): prevent duplicate auto-release runs racing to the same version
VERDICT: PASS (pending CI)
CI Status
- Lint: in_progress
- CodeQL: in_progress
Diff Review
auto-release.yml: Addedconcurrencyblock to serialize concurrent triggers; new pre-check step guards against duplicate PRs by verifying the version tag orauto-release/v$VERSIONbranch doesn't already exist before creating the bump PRrelease.yml: Changedexit 1→ no-op warning onalready_taggedso a redundant re-trigger is a warning, not a spurious red ❌
Observations
- Logic correctly traced: with the concurrency lock the second run waits, then its guard finds the branch/tag and skips cleanly, preventing the duplicate PR and spurious failure
- Workflow YAML structure is valid; no syntax issues
— Quinn, QA Engineer (formal verdict pending check completion)
|
Checks still in progress. I'll report back — the review is in progress and the formal verdict will be submitted once CI completes. Current status: Submitted an interim COMMENT on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
165-174:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConsider sanitizing tag before shell expansion to prevent command injection.
Line 173 expands
steps.version.outputs.tag(which containsv${VERSION}from package.json) directly into a shell command without validation. This has the same security concern as in auto-release.yml: a malicious version string in package.json could inject arbitrary commands.🛡️ Proposed fix to validate tag format
- name: Note already-tagged (no-op release) if: steps.version.outputs.already_tagged == 'true' run: | + TAG="${{ steps.version.outputs.tag }}" + # Validate tag format (vX.Y.Z with optional pre-release) + if ! echo "$TAG" | grep -qE '^v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.-]+)?$'; then + echo "::error::Invalid tag format: $TAG" + exit 1 + fi - echo "::warning::${{ steps.version.outputs.tag }} is already tagged; nothing to publish. This run is a no-op (likely a duplicate release PR)." + echo "::warning::${TAG} is already tagged; nothing to publish. This run is a no-op (likely a duplicate release PR)."
The logic change from error to warning is correct.
Converting the already-tagged case to a successful no-op with a warning appropriately handles the benign race condition scenario where duplicate release PRs reach this workflow. This prevents spurious CI failures while still providing visibility through the warning annotation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 165 - 174, The workflow step "Note already-tagged (no-op release)" currently expands steps.version.outputs.tag directly into a shell run, which risks shell injection; fix it by reading steps.version.outputs.tag into a safe shell variable (e.g., TAG) and validate/sanitize it with a strict regex (e.g., allow only semver with optional v prefix like ^v?\d+\.\d+\.\d+(-[A-Za-z0-9.-]+)?$) before using it, and then emit the warning using a safe, non-interpreting write (e.g., printf '%s' or echo with the quoted variable) so the value is not subject to shell expansion or special characters.
🧹 Nitpick comments (1)
.github/workflows/auto-release.yml (1)
121-121: ⚡ Quick winConsider explicit error handling for git ls-remote failures.
The
git ls-remotecommands could fail due to network issues or authentication problems. The currentgrep -q .pattern with implicit continuation could mask such failures, causing the guard to incorrectly reportskip=falsewhen it should error out or retry.♻️ Proposed fix to add explicit error handling
VERSION="${{ steps.version.outputs.version }}" - if git ls-remote --tags origin "refs/tags/v${VERSION}" | grep -q .; then + if TAG_CHECK=$(git ls-remote --tags origin "refs/tags/v${VERSION}"); then + if echo "$TAG_CHECK" | grep -q .; then - echo "v${VERSION} is already tagged — skipping duplicate release." - echo "skip=true" >> "$GITHUB_OUTPUT" - elif git ls-remote --heads origin "refs/heads/auto-release/v${VERSION}" | grep -q .; then + echo "v${VERSION} is already tagged — skipping duplicate release." + echo "skip=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + else + echo "::error::Failed to check remote tags" + exit 1 + fi + + if BRANCH_CHECK=$(git ls-remote --heads origin "refs/heads/auto-release/v${VERSION}"); then + if echo "$BRANCH_CHECK" | grep -q .; then - echo "Branch auto-release/v${VERSION} already exists — a release PR is already in flight; skipping." - echo "skip=true" >> "$GITHUB_OUTPUT" + echo "Branch auto-release/v${VERSION} already exists — a release PR is already in flight; skipping." + echo "skip=true" >> "$GITHUB_OUTPUT" + exit 0 + fi else + echo "::error::Failed to check remote branches" + exit 1 + fi - echo "skip=false" >> "$GITHUB_OUTPUT" - fi + echo "skip=false" >> "$GITHUB_OUTPUT"Also applies to: 124-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/auto-release.yml at line 121, The git tag/branch existence checks using the pipeline "git ls-remote --tags origin \"refs/tags/v${VERSION}\" | grep -q ." (and the similar branch check) can mask git failures; update these checks to enable pipefail or explicitly test git's exit status before relying on grep: e.g., set -o pipefail at the top of the script or run the git ls-remote call and check its exit code (capture output into a variable or files, fail fast and log the git error if git ls-remote returns non-zero) and only then run grep -q on the output to set skip=false; do the same change for the corresponding branch check command.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 165-174: The workflow step "Note already-tagged (no-op release)"
currently expands steps.version.outputs.tag directly into a shell run, which
risks shell injection; fix it by reading steps.version.outputs.tag into a safe
shell variable (e.g., TAG) and validate/sanitize it with a strict regex (e.g.,
allow only semver with optional v prefix like
^v?\d+\.\d+\.\d+(-[A-Za-z0-9.-]+)?$) before using it, and then emit the warning
using a safe, non-interpreting write (e.g., printf '%s' or echo with the quoted
variable) so the value is not subject to shell expansion or special characters.
---
Nitpick comments:
In @.github/workflows/auto-release.yml:
- Line 121: The git tag/branch existence checks using the pipeline "git
ls-remote --tags origin \"refs/tags/v${VERSION}\" | grep -q ." (and the similar
branch check) can mask git failures; update these checks to enable pipefail or
explicitly test git's exit status before relying on grep: e.g., set -o pipefail
at the top of the script or run the git ls-remote call and check its exit code
(capture output into a variable or files, fail fast and log the git error if git
ls-remote returns non-zero) and only then run grep -q on the output to set
skip=false; do the same change for the corresponding branch check command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a7a7a1ed-f075-410d-8373-31823b7a58ae
📒 Files selected for processing (2)
.github/workflows/auto-release.yml.github/workflows/release.yml
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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.
Problem
When two PRs merge in quick succession (e.g. #321 and #322 earlier today), each CI completion on main triggers
auto-release.yml. The runs executed concurrently, each computed the same next version from the same last tag (v0.50.0→0.51.0), and each opened a bump PR — #323 and #324, bothchore: release v0.51.0. Both auto-merged. Onerelease.ymlrun published + tagged0.51.0; the other hit the already-tagged guard andexit 1— a spurious red ❌ on a release that was actually fine.(
0.51.0did publish correctly exactly once — the idempotency guard worked. This just removes the noise and the duplicate PR.)Fix
auto-release.ymlconcurrency: { group: auto-release, cancel-in-progress: false }so runs serialize instead of racing (and an in-flight bump is never aborted mid-way).v$VERSIONis already tagged or anauto-release/v$VERSIONbranch already exists. The queued second run now observes what the first created and bails cleanly.release.ymlexit 0instead ofexit 1, so a duplicate-PR re-trigger doesn't show red.Validation
Both workflows parse cleanly (YAML + structure checked). Logic verified by re-tracing today's #323/#324 race: with the concurrency lock the second run waits, then its new guard finds
auto-release/v0.51.0already present and skips before creating a duplicate PR.🤖 Generated with Claude Code
Summary by CodeRabbit