Skip to content

ci(release): skip meta-package publish when the version already exists#18

Open
chethanuk wants to merge 1 commit into
mainfrom
ci/release-meta-idempotent
Open

ci(release): skip meta-package publish when the version already exists#18
chethanuk wants to merge 1 commit into
mainfrom
ci/release-meta-idempotent

Conversation

@chethanuk

Copy link
Copy Markdown
Owner

Description

The platform-package loop already guards itself against republishing a version that npm
already has (release.yml:209-215):

already=$(npm view "${pkg_name}@${VERSION}" version 2>/dev/null || true)
if [ "$already" = "$VERSION" ]; then
  echo "Skip: ${pkg_name}@${VERSION} already published"
else
  npm publish "$pkg_dir" --access public
fi

Twenty lines later the meta package is published unguarded (release.yml:228-229):

- name: Publish to npm
  run: npm publish --access public

This applies the same guard to that last step, so it is as re-runnable as the six before it.

Two states where this actually bites

  1. Re-running an already-successful release. A maintainer clicks Re-run all jobs on a
    green release — common when chasing an unrelated flake, or to refresh artifacts. The six
    platform packages skip correctly; the meta package hits E403 cannot publish over previously published version. A red job on a complete, correct release, with no way to
    green it short of cutting a new tag.
  2. Registry accepted, client errored. npm publish exits non-zero after the registry has
    already committed the version — a timeout or ECONNRESET on the response. The job is red,
    the version is live, and every subsequent re-run fails permanently at that step.

Both are exactly the states the platform loop is already protected against.

I want to be precise about one thing, because it is the obvious-sounding scenario and it is
not real: re-running a partially-failed release is not a case this fixes. Publish to npm is the last step of the last job (npm-publishneeds: release, nothing downstream),
so if the platform loop failed, the meta publish never ran, and a re-run publishes it cleanly.
The two cases above are the ones that matter.

Ordering is unchanged and still correct

Publishing the platform-independent package last is deliberate — evanw/esbuild's
Makefile carries the comment verbatim: "Publish platform-independent packages last to avoid
race conditions"
. This guard preserves that ordering rather than changing it.

Peer evidence

Every peer with this architecture (binary + platform npm packages + meta wrapper) guards the
wrapper the same way it guards the platform packages:

  • oxc.github/scripts/publish-if-needed.sh takes the package dir as an argument, so
    the wrapper runs through the same helper as the platform packages.
  • pnpm — publishes the wrapper last and uses it as the explicit "release completed"
    resume gate: "rerunning a tag resumes a partial release".

Verification

Extracted the real step body from the file and table-tested it against a stubbed npm, so
both branches are exercised without touching the registry:

Case Before this PR After this PR
Version already published exit 1, npm ERR! 403 cannot publish over previously published version exit 0, Skip: @alibaba-group/open-code-review@1.2.3 already published, and the publish is never invoked
New version exit 0, publish invoked exit 0, publish invoked, Published: @alibaba-group/open-code-review@1.2.3

The load-bearing assertion is the absence of the stub's STUB_PUBLISH_CALLED marker in the
already-published row — the guard doesn't just swallow the error, it skips the call.

Also run:

  • actionlint v1.7.12 — exit 0 on release.yml
  • yaml.safe_load on the changed file — parses
  • shellcheck -s bash on the extracted step body — exit 0, no findings
  • The Skip: / Published: strings are byte-identical to release.yml:211 and :214
    (differing only by one indentation level), so both paths read identically in a release log

Deliberately not changed

  • --access public is redundant — package.json already sets
    publishConfig.access: "public", as do all six platform packages. It matches the
    neighbouring loop at :213, so removing it here would be scope creep.
  • The npm viewnpm publish sequence is TOCTOU-racy and I left it that way on purpose.
    The registry rejects duplicate versions atomically (EPUBLISHCONFLICT), so the race's worst
    case is exactly the failure you get today. Closing it properly means parsing publish stderr —
    more code, no benefit.
  • I did not factor the two call sites into a shared script. oxc does exactly that with
    publish-if-needed.sh, so it is a fair question — but at two call sites and six lines, a new
    file plus chmod +x plus its own test harness is a bigger diff than the duplication, and it
    turns a bugfix into a refactor. Happy to do it separately if you'd prefer the helper.

Limitations

  • Not exercised against the real npm registry. I stubbed npm rather than publishing test
    versions of a real package. The stub reproduces both the E403 failure and the npm view
    success/404 paths, but I have not watched this run in an actual release.
  • The test check on this PR does not exercise release.yml at all — it only fires on
    push: tags: ['v*']. The verification above is the evidence.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • Stubbed-npm table test of the extracted step body, both branches — table above
  • Confirmed the publish call is genuinely skipped (not just error-swallowed) in the
    already-published case
  • actionlint v1.7.12 — exit 0
  • shellcheck -s bash on the step body — exit 0
  • yaml.safe_load on release.yml
  • Byte-identity check of the log strings against release.yml:211,214

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective — n/a, workflow config; no harness
    executes .github/workflows/. Verified with the stub table test above.
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly (not applicable)
  • I have signed the CLA

AI assistance: Claude Code helped research and verify this change. I reviewed the full diff
and take responsibility for it.

The platform-package loop guards itself against an already-published
version (release.yml:209-215), but the meta package's publish was a bare
'npm publish'. Re-running a successful release - or a publish whose
client errored after the registry had already committed the version -
fails permanently at that last step with E403.

Wrap it in the same guard, reusing the identical log strings so both
paths read the same in a release log. Publishing the meta package last
is unchanged and still correct.
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@chethanuk, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0d1dada1-12a0-49f9-829f-82021836bd08

📥 Commits

Reviewing files that changed from the base of the PR and between c60e886 and 395a7b9.

📒 Files selected for processing (1)
  • .github/workflows/release.yml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/release-meta-idempotent

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chethanuk

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant