Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughUpdates the npm publishing workflow in GitHub Actions to use OIDC-based authentication instead of implicit token handling. Replaces registry-url configuration with direct npmrc file generation containing the auth token from NODE_AUTH_TOKEN secret, and adds provenance flag to npm publish command. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @.github/workflows/release-please.yml:
- Around line 62-67: The workflow step that currently echoes an npm token into
~/.npmrc and runs `npm publish --access public --provenance` is inconsistent
with the PR objective/inline comment; update the inline comment to state this
step is a token-based fallback, ensure the env var and secret usage is explicit
(use NODE_AUTH_TOKEN from secrets.NPM_TOKEN) and confirm the echo line correctly
interpolates that token, and either verify that `--provenance` is supported with
token-based publishing (and add a short comment if verified) or remove the
`--provenance` flag if it is not supported.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/release-please.yml (2)
18-19: Update comment to reflect token-based authentication.The comment claims "Uses Trusted Publishing (OIDC) - no secrets needed!" but the implementation at lines 63-67 uses
secrets.NPM_TOKENfor token-based authentication. This contradicts the PR objective to "fall back to token-based auth."🔎 Proposed fix
-# npm Publishing: -# Uses Trusted Publishing (OIDC) - no secrets needed! -# Configure at: https://www.npmjs.com/package/@verygoodplugins/mcp-{name}/access +# npm Publishing: +# Uses token-based authentication via NPM_TOKEN secret +# Configure the NPM_TOKEN secret in repository settings
48-50: Remove unnecessaryid-token: writepermission and correct the misleading comment.The
id-token: writepermission is only required for OIDC trusted publishing. However, this workflow uses token-based authentication (NPM_TOKEN), making this permission unnecessary. Additionally, the comment on line 62 stating "Trusted Publishing via OIDC - no NPM_TOKEN needed" contradicts the actual implementation which explicitly usesNPM_TOKEN. Either migrate to proper OIDC trusted publishing (which requires removingNPM_TOKENand configuring a trusted publisher on npmjs.com) or simplify to token-based auth by removingid-token: writeand updating the comment.
| # Trusted Publishing via OIDC - no NPM_TOKEN needed | ||
| - run: | | ||
| echo "//registry.npmjs.org/:_authToken=\${NODE_AUTH_TOKEN}" > ~/.npmrc | ||
| npm publish --access public --provenance | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} |
There was a problem hiding this comment.
Critical inconsistency between PR objectives, comment, and implementation.
There are several contradictions here:
- PR objective states: "OIDC trusted publishing isn't working. Fall back to token-based auth"
- Line 62 comment claims: "Trusted Publishing via OIDC - no NPM_TOKEN needed"
- Line 67 implementation: Uses
secrets.NPM_TOKENfor authentication
Additionally, the --provenance flag at line 65 is typically associated with OIDC trusted publishing. If you're truly falling back to token-based authentication, you should:
- Update the comment to reflect token-based authentication
- Verify that
--provenanceworks with token-based auth (or remove it if it doesn't) - Ensure the npmrc configuration approach is correct for your use case
🔎 Proposed fix for consistent token-based authentication
- # Trusted Publishing via OIDC - no NPM_TOKEN needed
+ # Token-based authentication using NPM_TOKEN
- run: |
echo "//registry.npmjs.org/:_authToken=\${NODE_AUTH_TOKEN}" > ~/.npmrc
- npm publish --access public --provenance
+ npm publish --access public
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}Note: If you want to keep --provenance for attestation generation, verify that it works with token-based auth and document this in the comment.
🤖 Prompt for AI Agents
In @.github/workflows/release-please.yml around lines 62-67, The workflow step
that currently echoes an npm token into ~/.npmrc and runs `npm publish --access
public --provenance` is inconsistent with the PR objective/inline comment;
update the inline comment to state this step is a token-based fallback, ensure
the env var and secret usage is explicit (use NODE_AUTH_TOKEN from
secrets.NPM_TOKEN) and confirm the echo line correctly interpolates that token,
and either verify that `--provenance` is supported with token-based publishing
(and add a short comment if verified) or remove the `--provenance` flag if it is
not supported.
OIDC trusted publishing isn't working. Fall back to token-based auth.