Fix npm release recovery - #2830
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe npm release workflow now serializes runs, validates the current Changesnpm Release Workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant MainBranch
participant NpmRegistry
participant GitHub
GitHubActions->>MainBranch: validate current main head
GitHubActions->>NpmRegistry: resolve package versions and tarball hashes
NpmRegistry-->>GitHubActions: return registry state
GitHubActions->>MainBranch: commit required version reservations
GitHubActions->>NpmRegistry: publish selected package tarballs
GitHubActions->>GitHub: create release tag after publishing
GitHubActions->>GitHub: create GitHub release when required
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/npm-package-release.yml (2)
352-357: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Create GitHub Releaseruns even when the release already exists.The step condition uses only
RELEASE_REQUIRED.GITHUB_RELEASE_EXISTSis computed at line 265 but never used in a step condition.softprops/action-gh-release@v2updates an existing release by default, so this is not a failure, but it re-uploads the tarball assets on every recovery run. Gate the step on the existing-release state if you want the recovery path to be a no-op.Note also that the tarball assets at lines 360-361 are only produced when
RELEASE_REQUIREDis true, which matches this condition.🤖 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/npm-package-release.yml around lines 352 - 357, Update the Create GitHub Release step’s if condition to require both RELEASE_REQUIRED to be true and GITHUB_RELEASE_EXISTS to indicate that no release already exists. Preserve the existing tarball production condition and release configuration.
296-299: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueMove
${{ ... }}expansions intoenv:blocks to satisfy zizmor.zizmor flags template expansion inside
run:bodies at lines 298, 306, 321, 331, 336, and 345-349. The values come from this workflow's own step, so the practical risk is low. The values still originate from repository files (package.jsonversions) and fromnpm packoutput. Referencing them as shell variables removes the injection surface and clears the warnings.♻️ Example for the tag step
- name: Push release tag if: ${{ env.RELEASE_REQUIRED == 'true' }} + env: + RELEASE_TAG: ${{ env.RELEASE_TAG }} + RELEASE_TAG_EXISTS: ${{ env.RELEASE_TAG_EXISTS }} run: | - if [ "${{ env.RELEASE_TAG_EXISTS }}" = false ]; then - git tag "${{ env.RELEASE_TAG }}" - git push origin "${{ env.RELEASE_TAG }}" + if [ "$RELEASE_TAG_EXISTS" = false ]; then + git tag "$RELEASE_TAG" + git push origin "$RELEASE_TAG" else - echo "Tag ${{ env.RELEASE_TAG }} already exists" + echo "Tag $RELEASE_TAG already exists" fiAlso applies to: 304-306, 320-324, 329-331, 335-339, 344-350
🤖 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/npm-package-release.yml around lines 296 - 299, Move all GitHub Actions expression expansions currently embedded in the run scripts around the release commit, tag, publish, and packaging steps into step-level env entries. Update the affected shell commands to reference those environment variables instead, including values derived from RAINDEX_NEW_VERSION, UC_NEW_VERSION, and npm pack output, while preserving the existing release behavior.Source: Linters/SAST tools
🤖 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.
Inline comments:
In @.github/workflows/npm-package-release.yml:
- Around line 181-196: Update resolve_baseline_version to determine the highest
published semver rather than relying on npm view "$package_name@latest" version.
Compare the maximum registry version against repository_version and return the
higher baseline, preserving the repository fallback when no published versions
are available.
- Around line 198-224: Update the RAINDEX publish decision flow around
package_hash and the prerelease npm version step to verify each generated alpha
candidate with npm before publishing. After npm version prerelease selects a
candidate, query npm for that exact package version and continue incrementing
until the candidate is unused, while preserving the existing hash comparison and
publish behavior.
---
Nitpick comments:
In @.github/workflows/npm-package-release.yml:
- Around line 352-357: Update the Create GitHub Release step’s if condition to
require both RELEASE_REQUIRED to be true and GITHUB_RELEASE_EXISTS to indicate
that no release already exists. Preserve the existing tarball production
condition and release configuration.
- Around line 296-299: Move all GitHub Actions expression expansions currently
embedded in the run scripts around the release commit, tag, publish, and
packaging steps into step-level env entries. Update the affected shell commands
to reference those environment variables instead, including values derived from
RAINDEX_NEW_VERSION, UC_NEW_VERSION, and npm pack output, while preserving the
existing release behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd2e4f2a-6844-4a5f-b4fe-ac246867df8b
📒 Files selected for processing (1)
.github/workflows/npm-package-release.yml
1 participant
Motivation
The npm release workflow can leave Git and npm permanently out of sync. Run 31486644553 published raindex
0.0.1-alpha.244and UI components0.0.1-alpha.243, then failed to push its version commit becausemainadvanced. Run 31487015421 consequently derived raindex0.0.1-alpha.244from the stale manifest and npm rejected the duplicate version.This is not isolated to one run: #2815 previously had to synchronize package manifests after another partial two-package release.
Solution
mainhead supersedes it.Checks
nix run nixpkgs#actionlint -- .github/workflows/npm-package-release.ymlnpx --no-install prettier --check .github/workflows/npm-package-release.ymlgit diff --check.243/.242, npm.244/.243; selected.245/.244..245/.244with npm still behind; preserved both reserved versions without another bump.no-consumer-prettierandyamlfmt.Summary by CodeRabbit