Conversation
- Add environment: npm for GitHub OIDC auth - Remove sed/unset workarounds for auth token - Add files field to package.json to limit published contents - Add PR trigger on staging for testing
📝 WalkthroughWalkthroughReorders and consolidates Node.js and npm setup in the publish CI workflow: removes earlier OIDC/auth/token manipulation steps, adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
5-10:⚠️ Potential issue | 🟡 Minor
mainpoints tosrc/index.ts, which is excluded by the newfilesfield.
srcis not listed infiles, sosrc/index.tswon't be in the published package. Anyone importing this package programmatically (rather than using the CLI binary) would get a resolution error. If this is strictly a CLI tool and never imported as a library, consider updatingmainto point todist/index.jsfor consistency. If it's intentionally left for local dev tooling, this is fine but worth a note.Suggested fix
- "main": "src/index.ts", + "main": "dist/index.js",
🤖 Fix all issues with AI agents
In @.github/workflows/publish.yml:
- Around line 5-7: The workflow currently triggers the release job on
pull_request against main and will run the "release" job's step that executes
"npm run release" (release-it) on PRs; update the workflow so the release job or
the specific step is skipped for pull_request events (e.g., add an if-condition
such as checking github.event_name != 'pull_request' on the release job or on
the step that runs "npm run release") so tags/version bumps/publishes only run
on push/merge events; target the "release" job and the step that runs "npm run
release" when adding the condition.
- Remove pull_request trigger after successful OIDC publish test - Bump version past 0.34.1 which was published during testing
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/publish.yml (1)
47-55:⚠️ Potential issue | 🔴 Critical
NODE_AUTH_TOKENmust be passed to the Release step —npm publishwill fail with authentication error.
actions/setup-nodewithregistry-urlcreates an.npmrcthat references${NODE_AUTH_TOKEN}, but this variable is never set in the Release step's environment. Whenrelease-itcallsnpm publish(configured in.release-it.json), authentication will fail because the token resolves to an empty string, resulting in a 401/403 error.Fix — add NODE_AUTH_TOKEN to env
- name: Release run: | if [ "${{ github.ref_name }}" = "staging" ]; then npm run release-beta else npm run release fi env: GITHUB_TOKEN: ${{ steps.ci_bot_token.outputs.token }} + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}Replace
NPM_TOKENwith the actual secret name configured in thenpmenvironment.
🧹 Nitpick comments (2)
.github/workflows/publish.yml (2)
38-45: Unnamed steps reduce workflow readability.Lines 38, 43, and 45 use the shorthand
- uses:/- run:form without aname:key, unlike every other step in the workflow. Adding names improves log readability in the GitHub Actions UI.Proposed fix
- - uses: actions/setup-node@v4 + - name: Setup Node.js + uses: actions/setup-node@v4 with: node-version: "22" registry-url: "https://registry.npmjs.org" - - run: npm install -g npm@latest + - name: Upgrade npm + run: npm install -g npm@latest - - run: npm ci + - name: Install dependencies + run: npm ci
43-43:npm install -g npm@latestpins to a moving target.Upgrading npm to
lateston every CI run can silently introduce breaking changes. Node 22 already ships with a compatible npm version. Consider either dropping this step or pinning to a specific version (e.g.,npm@10) if a newer npm is actually needed.
Summary by CodeRabbit