-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Release tooling: Use npm Trusted Publishing #32607
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
Walkthrough
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Node as Setup Node.js
participant Yarn as Yarn 4.10.3
participant NPM as Registry
participant Discord as Discord Webhook
Dev->>GH: Push/Tag triggers publish workflow
GH->>Node: Setup Node.js (reads ".nvmrc")
GH->>Yarn: Install dependencies
GH->>Yarn: Build
note over GH,Yarn: Yarn version referenced via updated yarnPath/packageManager (4.10.3)
GH->>NPM: Publish package
note over GH,NPM: NPM auth token not set (YARN_NPM_AUTH_TOKEN removed)
GH-->>Discord: On failure, send failure message
note over GH,Discord: Message formatting uses double quotes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
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: 8
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)
108-111: Publishing will fail without npm credentialsWe removed
YARN_NPM_AUTH_TOKEN, but the workflow still needs credentials foryarn release:publish. There’s no replacement (noid-token: writepermission or other auth config), so the publish step will now hit the registry unauthenticated and error out. Please either keep the token flow or add the required trusted-publishing setup (grantid-token: write, configure npm for OIDC, etc.) before merging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.yarn/releases/yarn-4.10.3.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-4.9.1.cjsis excluded by!**/.yarn/**
📒 Files selected for processing (16)
.github/workflows/publish.yml(2 hunks).yarnrc.yml(1 hunks)code/.yarnrc.yml(1 hunks)code/package.json(1 hunks)package.json(1 hunks)scripts/.yarnrc.yml(1 hunks)scripts/package.json(1 hunks)test-storybooks/ember-cli/.yarnrc.yml(1 hunks)test-storybooks/external-docs/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/nextjs/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/react/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/svelte/.yarnrc.yml(1 hunks)test-storybooks/portable-stories-kitchen-sink/vue3/.yarnrc.yml(1 hunks)test-storybooks/server-kitchen-sink/.yarnrc.yml(1 hunks)test-storybooks/standalone-preview/.yarnrc.yml(1 hunks)test-storybooks/yarn-pnp/.yarnrc.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.yarnrc.yml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Yarn 4.9.1 as the package manager (configured via .yarnrc.yml)
Files:
.yarnrc.yml
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to .yarnrc.yml : Use Yarn 4.9.1 as the package manager (configured via .yarnrc.yml)
📚 Learning: 2025-09-25T09:21:27.284Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to .yarnrc.yml : Use Yarn 4.9.1 as the package manager (configured via .yarnrc.yml)
Applied to files:
.yarnrc.ymlpackage.jsontest-storybooks/portable-stories-kitchen-sink/vue3/.yarnrc.ymlcode/.yarnrc.ymltest-storybooks/portable-stories-kitchen-sink/react/.yarnrc.ymltest-storybooks/portable-stories-kitchen-sink/nextjs/.yarnrc.ymlcode/package.jsontest-storybooks/server-kitchen-sink/.yarnrc.ymltest-storybooks/standalone-preview/.yarnrc.ymlscripts/package.jsontest-storybooks/portable-stories-kitchen-sink/svelte/.yarnrc.ymltest-storybooks/yarn-pnp/.yarnrc.ymltest-storybooks/ember-cli/.yarnrc.ymltest-storybooks/external-docs/.yarnrc.ymlscripts/.yarnrc.yml
📚 Learning: 2025-09-25T09:21:27.284Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to .nvmrc : Use Node.js version 22.16.0 as specified by .nvmrc
Applied to files:
.github/workflows/publish.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/.yarnrc.yml (1)
5-5: Reconcile Yarn version with maintainer guidanceThis bumps the repo tooling to Yarn 4.10.3, but per the maintainer note we previously received for
.yarnrc.ymlfiles we’re expected to stay on 4.9.1. Please confirm that the policy has changed (and update the guidance if so) before landing this upgrade, or revert to 4.9.1 to stay compliant. Based on learnings
|
|
||
| npmPublishAccess: public | ||
|
|
||
| yarnPath: .yarn/releases/yarn-4.10.3.cjs |
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.
Keep the root Yarn binary at 4.9.1.
The documented guidance for this repo explicitly requires .yarnrc.yml to point at yarn-4.9.1.cjs; updating to 4.10.3 steps outside the approved toolchain.
As per coding guidelines
-yarnPath: .yarn/releases/yarn-4.10.3.cjs
+yarnPath: .yarn/releases/yarn-4.9.1.cjs🤖 Prompt for AI Agents
In .yarnrc.yml around line 11, the yarnPath points to yarn-4.10.3.cjs which
violates the repo guideline that requires the root Yarn binary be pinned to
yarn-4.9.1.cjs; change the value of yarnPath to ".yarn/releases/yarn-4.9.1.cjs"
so the project uses the approved toolchain and commit the updated .yarnrc.yml.
| } | ||
| }, | ||
| "packageManager": "yarn@4.9.1", | ||
| "packageManager": "yarn@4.10.3", |
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.
Restore Yarn 4.9.1 for workspace tooling.
The workspace must stick to Yarn 4.9.1 per repo guidelines; updating this packageManager entry to 4.10.3 conflicts with the mandated version.
As per coding guidelines
- "packageManager": "[email protected]",
+ "packageManager": "[email protected]",📝 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.
| "packageManager": "yarn@4.10.3", | |
| "packageManager": "yarn@4.9.1", |
🤖 Prompt for AI Agents
In code/package.json around line 243, the packageManager field was changed to
"[email protected]" which violates the repo guideline requiring Yarn 4.9.1; revert
this entry to "[email protected]" so workspace tooling uses the mandated version and
update any related lockfile or CI configs if they reference the newer version to
remain consistent.
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.
@coderabbitai I don't know where you're getting these guidelines from? but you must update them so they reference v ^4 of yarn instead.
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.
@JReinhold, you're absolutely right—I apologize for the confusion. That guideline was outdated and incorrectly pinned to Yarn 4.9.1 specifically. I've removed the old guideline and updated it to reflect that the repository uses Yarn v^4 (any 4.x version).
The upgrade to Yarn 4.10.3 is perfectly fine and aligns with keeping the tooling up to date within the v4 major version.
✏️ Learnings added
Learnt from: JReinhold
PR: storybookjs/storybook#32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.185Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to .yarnrc.yml : Use Yarn 4.9.1 as the package manager (configured via .yarnrc.yml)
🧠 Learnings used
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.284Z
Learning: Applies to .yarnrc.yml : Use Yarn 4.9.1 as the package manager (configured via .yarnrc.yml)
| "vite-ecosystem-ci:test": "yarn task --task test-runner-dev --template react-vite/default-ts --start-from=dev && yarn task --task test-runner --template react-vite/default-ts --start-from=build && yarn task --task vitest-integration --template react-vite/default-ts --start-from vitest-integration" | ||
| }, | ||
| "packageManager": "yarn@4.9.1", | ||
| "packageManager": "yarn@4.10.3", |
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.
Revert packageManager to Yarn 4.9.1.
This repo is required to stay on Yarn 4.9.1; bumping the manifest to 4.10.3 breaks that contract and risks desync with the enforced toolchain. Please keep the packageManager field at [email protected].
As per coding guidelines
- "packageManager": "[email protected]",
+ "packageManager": "[email protected]",📝 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.
| "packageManager": "yarn@4.10.3", | |
| "packageManager": "yarn@4.9.1", |
🤖 Prompt for AI Agents
In package.json around line 25, the packageManager field was changed to
"[email protected]"; revert it back to "[email protected]" to comply with the repo toolchain.
Update the packageManager entry to exactly "[email protected]" and save the file so the
manifest matches the enforced toolchain.
| enableGlobalCache: true | ||
|
|
||
| yarnPath: ../.yarn/releases/yarn-4.9.1.cjs | ||
| yarnPath: ../.yarn/releases/yarn-4.10.3.cjs |
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.
Re-pin the Yarn binary to 4.9.1.
The repo mandates Yarn 4.9.1 in all .yarnrc.yml files; pointing to 4.10.3 breaks the agreed tooling baseline.
As per coding guidelines
-yarnPath: ../.yarn/releases/yarn-4.10.3.cjs
+yarnPath: ../.yarn/releases/yarn-4.9.1.cjs📝 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.
| yarnPath: ../.yarn/releases/yarn-4.10.3.cjs | |
| yarnPath: ../.yarn/releases/yarn-4.9.1.cjs |
🤖 Prompt for AI Agents
In scripts/.yarnrc.yml around line 5, the yarnPath is pointing to
yarn-4.10.3.cjs which violates the repo's mandated Yarn version; update the
yarnPath to reference the pinned binary yarn-4.9.1.cjs (e.g.,
../.yarn/releases/yarn-4.9.1.cjs), save the file, and run or document any
follow-up steps to ensure the corresponding yarn release file exists in
.yarn/releases and then commit the change.
| "verdaccio-auth-memory": "^10.2.2" | ||
| }, | ||
| "packageManager": "yarn@4.9.1" | ||
| "packageManager": "yarn@4.10.3" |
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.
Align packageManager with required Yarn version.
This workspace also needs to stay on Yarn 4.9.1; updating to 4.10.3 conflicts with the enforced toolchain version.
As per coding guidelines
- "packageManager": "[email protected]"
+ "packageManager": "[email protected]"📝 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.
| "packageManager": "yarn@4.10.3" | |
| "packageManager": "yarn@4.9.1" |
🤖 Prompt for AI Agents
In scripts/package.json around line 188, the packageManager field was set to
"[email protected]" which conflicts with the enforced toolchain; change the value back
to "[email protected]" to match the required Yarn version and ensure consistency with
the workspace toolchain (also regenerate or verify Yarn files if needed so the
lockfile and .yarn/releases align with 4.9.1).
| nodeLinker: node-modules | ||
| enableImmutableInstalls: false | ||
| yarnPath: ../../.yarn/releases/yarn-4.9.1.cjs | ||
| yarnPath: ../../.yarn/releases/yarn-4.10.3.cjs |
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.
Restore Yarn path to the mandated 4.9.1 build.
All .yarnrc.yml files are required to target yarn-4.9.1.cjs; this bump to 4.10.3 violates the repo standard.
As per coding guidelines
-yarnPath: ../../.yarn/releases/yarn-4.10.3.cjs
+yarnPath: ../../.yarn/releases/yarn-4.9.1.cjs📝 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.
| yarnPath: ../../.yarn/releases/yarn-4.10.3.cjs | |
| yarnPath: ../../.yarn/releases/yarn-4.9.1.cjs |
🤖 Prompt for AI Agents
In test-storybooks/external-docs/.yarnrc.yml around line 3, the yarnPath was
bumped to yarn-4.10.3.cjs which violates the repo mandate; update the yarnPath
value to ../../.yarn/releases/yarn-4.9.1.cjs so the file targets the approved
yarn-4.9.1 build, save the change, and include a brief commit message noting
restoration to the mandated yarn version.
| @@ -1 +1 @@ | |||
| yarnPath: ../../../.yarn/releases/yarn-4.9.1.cjs | |||
| yarnPath: ../../../.yarn/releases/yarn-4.10.3.cjs | |||
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.
Revert Yarn path to the approved 4.9.1 release.
Per repo policy, .yarnrc.yml must continue pointing at yarn-4.9.1.cjs; upgrading this file to 4.10.3 conflicts with that mandate.
As per coding guidelines
-yarnPath: ../../../.yarn/releases/yarn-4.10.3.cjs
+yarnPath: ../../../.yarn/releases/yarn-4.9.1.cjs📝 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.
| yarnPath: ../../../.yarn/releases/yarn-4.10.3.cjs | |
| yarnPath: ../../../.yarn/releases/yarn-4.9.1.cjs |
🤖 Prompt for AI Agents
In test-storybooks/portable-stories-kitchen-sink/react/.yarnrc.yml around lines
1 to 1, the yarnPath points to yarn-4.10.3.cjs which violates the repo policy
requiring yarn-4.9.1.cjs; revert the value to yarn-4.9.1.cjs so the file reads
yarnPath: ../../../.yarn/releases/yarn-4.9.1.cjs and ensure no other references
to 4.10.3 remain in this file.
| @@ -1,3 +1,3 @@ | |||
| yarnPath: ../../.yarn/releases/yarn-4.9.1.cjs | |||
| yarnPath: ../../.yarn/releases/yarn-4.10.3.cjs | |||
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.
Keep Yarn release pinned at 4.9.1.
Per repository guidance, .yarnrc.yml files must reference yarn-4.9.1.cjs. Upgrading this path to 4.10.3 violates that requirement and will desync tooling.
As per coding guidelines
-yarnPath: ../../.yarn/releases/yarn-4.10.3.cjs
+yarnPath: ../../.yarn/releases/yarn-4.9.1.cjs📝 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.
| yarnPath: ../../.yarn/releases/yarn-4.10.3.cjs | |
| yarnPath: ../../.yarn/releases/yarn-4.9.1.cjs |
🤖 Prompt for AI Agents
In test-storybooks/yarn-pnp/.yarnrc.yml around lines 1 to 1, the yarnPath is set
to yarn-4.10.3.cjs which violates the repository guideline requiring
yarn-4.9.1.cjs; update the yarnPath value to ../../.yarn/releases/yarn-4.9.1.cjs
(restore the pinned release) and save the file so tooling remains in sync.
|
View your CI Pipeline Execution ↗ for commit 19c67c7
☁️ Nx Cloud last updated this comment at |
|
Self-merging @ndelangen |
|
For future reference, additional work was needed:
|
Release tooling: Use npm Trusted Publishing (cherry picked from commit b0b5902)
Moving the monorepo to npm Trusted Publishing. See:
This does not make the Canary release process work, it's currently broken due to not using trusted publishing. I want to test if this works out first, before making the modifications to the canary release workflow.
What I did
publish.ymlscriptOn npmjs.com, I enabled Trusted Publishing for all the 43 packages in the monorepo.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>