-
-
Notifications
You must be signed in to change notification settings - Fork 265
ci: enable trusted publishing #345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe release workflow adds a step to upgrade npm after setting up Node.js LTS and removes NPM_CONFIG_PROVENANCE and NPM_TOKEN environment variables from the publish step. No source code or exported entities are modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions Runner
participant Node as Setup Node.js LTS
participant NPM as npm CLI
participant Pub as Release/Publish Step
Dev->>GH: Push/Tag triggers release workflow
GH->>Node: actions/setup-node@... (LTS)
Note over Node,GH: Node.js runtime prepared
GH->>NPM: npm install -g npm@latest
Note over NPM,GH: New step: Upgrade npm
GH->>Pub: Create Release PR / Publish
Note over Pub,GH: Env updated (no NPM_CONFIG_PROVENANCE, no NPM_TOKEN)
Pub-->>Dev: Release PR created or publish executed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to d09acdf in 1 minute and 8 seconds. Click for details.
- Reviewed
20
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/release.yml:48
- Draft comment:
Removal of NPM_TOKEN and NPM_CONFIG_PROVENANCE is intentional for trusted publishing – verify that npm publishing works without these. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that npm publishing works without certain tokens, which is against the rules. It is not making a specific code suggestion or asking for a test to be written. Therefore, this comment should be removed.
Workflow ID: wflow_PIpkm6ijeVuqNgit
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/release.yml (3)
34-36
: Pin npm to a safe minimum compatible with Trusted Publishing (avoid npm@latest).To reduce surprise from npm breaking changes while still meeting OIDC requirements, pin to a compatible floor (>= 11.5.1). Example:
- - name: Upgrade npm - run: npm install -g npm@latest + - name: Ensure npm supports Trusted Publishing + run: npm install -g npm@^11.5.1Re: the earlier suggestion to pin npm@9.8.1 — that version will not work with npm’s Trusted Publishing, which requires npm CLI 11.5.1 or newer. (docs.npmjs.com)
34-36
: Optional: Skip the upgrade when the runner already has a new-enough npm.Saves CI time and avoids mutating the global toolchain unnecessarily:
- - name: Upgrade npm - run: npm install -g npm@latest + - name: Ensure npm supports Trusted Publishing + run: | + REQUIRED_MINOR=5 + MAJOR=$(npm -v | cut -d. -f1) + MINOR=$(npm -v | cut -d. -f2) + if [ "$MAJOR" -lt 11 ] || { [ "$MAJOR" -eq 11 ] && [ "$MINOR" -lt "$REQUIRED_MINOR" ]; }; then + npm install -g npm@^11.5.1 + fi
34-36
: Optional: Make the target registry explicit in setup-node.Not required, but being explicit can prevent accidental publishes to a custom/defaulted registry in other contexts. You can add this to the existing “Setup Node.js LTS” step:
with: node-version: lts/* cache: yarn registry-url: 'https://registry.npmjs.org'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/release.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/release.yml (1)
34-36
: Trusted Publishing alignment: this step and removal of NPM_TOKEN/PROVENANCE look correct (OIDC-only).With
permissions.id-token: write
already set and npm upgraded, npm CLI will authenticate via OIDC for publishes; noNPM_TOKEN
orNPM_CONFIG_PROVENANCE
is needed, and provenance is generated automatically. Ensure your package is configured on npmjs.com with this repo and workflow filename (release.yml
) as the Trusted Publisher. (docs.npmjs.com)
Important
Upgrade npm and remove unused environment variables in
release.yml
for improved release workflow.release.yml
.NPM_CONFIG_PROVENANCE
andNPM_TOKEN
environment variables fromrelease.yml
.This description was created by
for d09acdf. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit