Conversation
tirumerla
left a comment
tirumerla
left a comment
There was a problem hiding this comment.
Not sure if we still need the token, otherwise lgtm
.github/workflows/publish.yml
Outdated
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} | ||
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} |
There was a problem hiding this comment.
Do we still need the TOKEN?
WalkthroughGitHub Actions configuration updates enable OIDC-based trusted publishing for npm packages. The setup action now configures the npm registry URL. The publish workflow installs npm@11.12.1, detects new packages in Yarn workspaces, and conditionally exports authentication tokens for publishing operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
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)
50-66:⚠️ Potential issue | 🟠 MajorUpgrade
changesets/actionto v1.6.0+ to enable OIDC-aware authentication.The workflow uses
changesets/action@v1.5.3, which lacks OIDC-aware logic added in v1.6.0+. Whenhas_new_packagesis true,NODE_AUTH_TOKENis set in the job environment, and v1.5.3 will use this token for all packages in the subsequentpublish: npm run publishinvocation—including existing packages that could use OIDC trusted publishing. Upgrading to v1.6.0 or later enables the action to skip token configuration when OIDC conditions are met, allowing existing packages to publish via OIDC while only new packages (or those requiring token fallback) use NPM_TOKEN.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 50 - 66, The workflow step using changesets/action pinned as changesets/action@e0145edc7d9d8679003495b11f87bd8ef63c0cba (v1.5.3) should be updated to v1.6.0 or later so the action can perform OIDC-aware authentication; change the step with id "changesets" (the "Create Prepare Release PR or Publish" step) to use a v1.6.0+ reference, and keep the existing NODE_AUTH_TOKEN/NPM_TOKEN logic and has_new_packages conditional intact so the action will skip injecting the token when OIDC is available and only fall back to NPM_TOKEN when necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 35-49: The script currently treats any failure of the npm view
command as a new-package signal; change the loop that runs npm view "$pkg" so it
captures stderr/exit status and only treats definite 404 responses (e.g., stderr
contains "404" / "Not Found" / "E404") as a new package to append to
"$RUNNER_TEMP/new_packages.txt"; for any other non-404 failure (timeouts, 5xx,
rate limits, network errors) have the step exit non-zero (fail) with an error
message so the workflow doesn’t silently switch to token publishing. Update the
logic around the npm view call and the determination of has_new_packages to
reflect this behavior while still writing to "$RUNNER_TEMP/new_packages.txt"
only for true 404s.
---
Outside diff comments:
In @.github/workflows/publish.yml:
- Around line 50-66: The workflow step using changesets/action pinned as
changesets/action@e0145edc7d9d8679003495b11f87bd8ef63c0cba (v1.5.3) should be
updated to v1.6.0 or later so the action can perform OIDC-aware authentication;
change the step with id "changesets" (the "Create Prepare Release PR or Publish"
step) to use a v1.6.0+ reference, and keep the existing
NODE_AUTH_TOKEN/NPM_TOKEN logic and has_new_packages conditional intact so the
action will skip injecting the token when OIDC is available and only fall back
to NPM_TOKEN when necessary.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 464b403d-d857-4a0a-b7c9-2f87dec0ef89
📒 Files selected for processing (2)
.github/actions/setup/action.yml.github/workflows/publish.yml
| " | while IFS=: read -r pkg dir; do | ||
| if ! npm view "$pkg" version > /dev/null 2>&1; then | ||
| echo "New package detected: $pkg ($dir)" | ||
| echo "$dir" >> "$RUNNER_TEMP/new_packages.txt" | ||
| else | ||
| echo "Existing package: $pkg" | ||
| fi | ||
| done | ||
| if [ -s "$RUNNER_TEMP/new_packages.txt" ]; then | ||
| echo "::notice::New packages detected — will use NPM token for publish" | ||
| echo "has_new_packages=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "All packages exist on npm — using OIDC trusted publishing" | ||
| echo "has_new_packages=false" >> "$GITHUB_OUTPUT" | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the current branch condition treats every non-zero `npm view` exit as "new package".
sed -n '35,49p' .github/workflows/publish.yml
rg -n -C2 'npm view "\$pkg" version > /dev/null 2>&1|New package detected|has_new_packages=true' .github/workflows/publish.ymlRepository: OpenZeppelin/contracts-wizard
Length of output: 1347
Differentiate 404s from transient npm view failures to avoid silent auth-path changes.
Line 36 treats every npm view error identically—timing out, returning 5xx, or hitting rate limits will silently flip the job into token mode instead of surfacing the actual registry failure. Only treat definite 404 responses as "new package"; fail the step on other errors.
Suggested fix
- " | while IFS=: read -r pkg dir; do
- if ! npm view "$pkg" version > /dev/null 2>&1; then
+ " | while IFS=: read -r pkg dir; do
+ if output="$(npm view "$pkg" version 2>&1)"; then
+ echo "Existing package: $pkg"
+ elif printf '%s' "$output" | grep -qE 'E404|404 Not Found'; then
echo "New package detected: $pkg ($dir)"
echo "$dir" >> "$RUNNER_TEMP/new_packages.txt"
else
- echo "Existing package: $pkg"
+ printf '%s\n' "$output" >&2
+ exit 1
fi
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| " | while IFS=: read -r pkg dir; do | |
| if ! npm view "$pkg" version > /dev/null 2>&1; then | |
| echo "New package detected: $pkg ($dir)" | |
| echo "$dir" >> "$RUNNER_TEMP/new_packages.txt" | |
| else | |
| echo "Existing package: $pkg" | |
| fi | |
| done | |
| if [ -s "$RUNNER_TEMP/new_packages.txt" ]; then | |
| echo "::notice::New packages detected — will use NPM token for publish" | |
| echo "has_new_packages=true" >> "$GITHUB_OUTPUT" | |
| else | |
| echo "All packages exist on npm — using OIDC trusted publishing" | |
| echo "has_new_packages=false" >> "$GITHUB_OUTPUT" | |
| fi | |
| " | while IFS=: read -r pkg dir; do | |
| if output="$(npm view "$pkg" version 2>&1)"; then | |
| echo "Existing package: $pkg" | |
| elif printf '%s' "$output" | grep -qE 'E404|404 Not Found'; then | |
| echo "New package detected: $pkg ($dir)" | |
| echo "$dir" >> "$RUNNER_TEMP/new_packages.txt" | |
| else | |
| printf '%s\n' "$output" >&2 | |
| exit 1 | |
| fi | |
| done | |
| if [ -s "$RUNNER_TEMP/new_packages.txt" ]; then | |
| echo "::notice::New packages detected — will use NPM token for publish" | |
| echo "has_new_packages=true" >> "$GITHUB_OUTPUT" | |
| else | |
| echo "All packages exist on npm — using OIDC trusted publishing" | |
| echo "has_new_packages=false" >> "$GITHUB_OUTPUT" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/publish.yml around lines 35 - 49, The script currently
treats any failure of the npm view command as a new-package signal; change the
loop that runs npm view "$pkg" so it captures stderr/exit status and only treats
definite 404 responses (e.g., stderr contains "404" / "Not Found" / "E404") as a
new package to append to "$RUNNER_TEMP/new_packages.txt"; for any other non-404
failure (timeouts, 5xx, rate limits, network errors) have the step exit non-zero
(fail) with an error message so the workflow doesn’t silently switch to token
publishing. Update the logic around the npm view call and the determination of
has_new_packages to reflect this behavior while still writing to
"$RUNNER_TEMP/new_packages.txt" only for true 404s.
2 participants
Summary
Add npm trusted publishing support to the publish workflow while keeping
NPM_TOKENfallback for first-time publishes of new packages.Changes
registry-url: 'https://registry.npmjs.org'to the shared setup action sosetup-nodewrites npm config for the npm registry and publish auth is wired correctly.id-token: writeto.github/workflows/publish.ymlso GitHub Actions can mint the OIDC token required by npm trusted publishing.npm@11.12.1in the publish workflow because trusted publishing requires npm11.5.1+, while the default npm on Node 22 is older.NPM_TOKENtoNODE_AUTH_TOKENbecausesetup-nodewrites.npmrcusing${NODE_AUTH_TOKEN}, which is the env var npm will actually read.NPM_CONFIG_PROVENANCE: trueso provenance is generated consistently, including when the workflow falls back to token-based publish.Notes
NPM_TOKENfallback for new packages that do not yet have a trusted publisher configured.