ci(release): skip meta-package publish when the version already exists#18
ci(release): skip meta-package publish when the version already exists#18chethanuk wants to merge 1 commit into
Conversation
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.
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Description
The platform-package loop already guards itself against republishing a version that npm
already has (
release.yml:209-215):Twenty lines later the meta package is published unguarded (
release.yml:228-229):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
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 togreen it short of cutting a new tag.
npm publishexits non-zero after the registry hasalready committed the version — a timeout or
ECONNRESETon 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 npmis the last step of the last job (npm-publish→needs: 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'sMakefilecarries the comment verbatim: "Publish platform-independent packages last to avoidrace 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.shtakes the package dir as an argument, sothe 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, soboth branches are exercised without touching the registry:
npm ERR! 403 cannot publish over previously published versionSkip: @alibaba-group/open-code-review@1.2.3 already published, and the publish is never invokedPublished: @alibaba-group/open-code-review@1.2.3The load-bearing assertion is the absence of the stub's
STUB_PUBLISH_CALLEDmarker in thealready-published row — the guard doesn't just swallow the error, it skips the call.
Also run:
actionlintv1.7.12 — exit 0 onrelease.ymlyaml.safe_loadon the changed file — parsesshellcheck -s bashon the extracted step body — exit 0, no findingsSkip:/Published:strings are byte-identical torelease.yml:211and:214(differing only by one indentation level), so both paths read identically in a release log
Deliberately not changed
--access publicis redundant —package.jsonalready setspublishConfig.access: "public", as do all six platform packages. It matches theneighbouring loop at
:213, so removing it here would be scope creep.npm view→npm publishsequence is TOCTOU-racy and I left it that way on purpose.The registry rejects duplicate versions atomically (
EPUBLISHCONFLICT), so the race's worstcase is exactly the failure you get today. Closing it properly means parsing publish stderr —
more code, no benefit.
oxcdoes exactly that withpublish-if-needed.sh, so it is a fair question — but at two call sites and six lines, a newfile plus
chmod +xplus its own test harness is a bigger diff than the duplication, and itturns a bugfix into a refactor. Happy to do it separately if you'd prefer the helper.
Limitations
npmrather than publishing testversions of a real package. The stub reproduces both the
E403failure and thenpm viewsuccess/404 paths, but I have not watched this run in an actual release.
testcheck on this PR does not exerciserelease.ymlat all — it only fires onpush: tags: ['v*']. The verification above is the evidence.Type of Change
How Has This Been Tested?
npmtable test of the extracted step body, both branches — table abovealready-published case
actionlintv1.7.12 — exit 0shellcheck -s bashon the step body — exit 0yaml.safe_loadonrelease.ymlrelease.yml:211,214Checklist
executes
.github/workflows/. Verified with the stub table test above.AI assistance: Claude Code helped research and verify this change. I reviewed the full diff
and take responsibility for it.