Release tooling: Use npm Trusted Publishing for canaries #32621
Conversation
📝 WalkthroughWalkthroughAdds a composite action at .github/actions/setup-node-and-install and updates multiple workflows to use it, consolidating Node setup, caching and dependency installs; removes the standalone canary-release-pr workflow and integrates canary logic into publish.yml; updates docs and some workflow string quoting and notification pins. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub (Triggers)
participant WF as Workflow (publish.yml)
participant DET as Determine release type
participant JOB_NORMAL as Job: publish-normal
participant JOB_CANARY as Job: publish-canary
participant ACTION as Local action: setup-node-and-install
participant GH_CLI as gh CLI
participant PR as Pull Request
participant REG as Registry/Release Services
GH->>WF: push / pull_request / workflow_dispatch
WF->>DET: evaluate event/inputs
alt normal release
DET-->>WF: normal
WF->>JOB_NORMAL: start
JOB_NORMAL->>ACTION: setup node & install deps
ACTION-->>JOB_NORMAL: node + deps ready
JOB_NORMAL->>REG: build & publish (normal)
else canary release
DET-->>WF: canary
WF->>JOB_CANARY: start (PR/manual)
JOB_CANARY->>GH_CLI: fetch PR, permissions
JOB_CANARY->>PR: checkout PR head
JOB_CANARY->>ACTION: setup node & install deps
ACTION-->>JOB_CANARY: node + deps ready
JOB_CANARY->>REG: set canary version & publish
JOB_CANARY->>PR: update PR body with release details
end
sequenceDiagram
autonumber
participant CA as Composite Action: setup-node-and-install
participant SN as actions/setup-node@v4
participant NPM as npm
participant CACHE as actions/cache@v4
participant YARN as yarn (scripts/)
CA->>SN: set Node (node-version-file: .nvmrc)
CA->>NPM: npm install -g npm@latest
CA->>CACHE: restore/cache Yarn paths (multi-key)
CA->>YARN: yarn install in ./scripts
alt install-code-deps == "true"
CA->>YARN: yarn install in ./code
end
CA-->>caller: done
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
JReinhold
added
build
|
View your CI Pipeline Execution ↗ for commit 35ece30
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
CONTRIBUTING/RELEASING.md (1)
453-457: Fix markdown bare URL (MD034).Wrap the URL in angle brackets or use link syntax to satisfy markdownlint.
-1. Open the workflow UI at https://github.com/storybookjs/storybook/actions/workflows/publish.yml +1. Open the workflow UI at <https://github.com/storybookjs/storybook/actions/workflows/publish.yml>.github/actions/setup-node-and-install/action.yml (2)
12-15: Avoid floating npm@latest; pin or drop.Upgrading npm globally to latest can introduce unreproducible builds. Prefer a pinned major (e.g., npm@10) or rely on the runner’s npm.
- run: npm install -g npm@latest + run: npm install -g npm@10 +# Or remove entirely if not strictly needed.
16-26: Broaden Yarn cache paths for Yarn Berry.Caching only ~/.yarn/berry/cache is a no-op unless enableGlobalCache is set. Cache repo caches too.
with: - path: | - ~/.yarn/berry/cache + path: | + ~/.yarn/berry/cache + scripts/.yarn/cache + code/.yarn/cache key: yarn-v1-${{ hashFiles('scripts/yarn.lock') }}-${{ hashFiles('code/yarn.lock') }} restore-keys: | yarn-v1-${{ hashFiles('scripts/yarn.lock') }}-${{ hashFiles('code/yarn.lock') }} yarn-v1-${{ hashFiles('scripts/yarn.lock') }} yarn-v1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/actions/setup-node-and-install/action.yml(1 hunks).github/workflows/canary-release-pr.yml(0 hunks).github/workflows/prepare-non-patch-release.yml(1 hunks).github/workflows/prepare-patch-release.yml(1 hunks).github/workflows/publish.yml(5 hunks).github/workflows/tests-unit.yml(1 hunks)CONTRIBUTING/RELEASING.md(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/canary-release-pr.yml
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/publish.yml
46-46: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details
(expression)
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING/RELEASING.md
453-453: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (2)
- GitHub Check: Danger JS
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
.github/workflows/prepare-patch-release.yml (1)
33-38: LGTM: consolidated setup + explicit code install.Using the local action and installing code deps from scripts context is correct.
.github/workflows/tests-unit.yml (1)
22-27: LGTM: unified setup; correct code install path.Composite action + cd code install looks good.
.github/workflows/prepare-non-patch-release.yml (1)
54-59: LGTM: local setup + code install from scripts context.Matches other workflows and repo layout.
.github/workflows/publish.yml (1)
78-80: LGTM: local setup for normal publish.Consolidation reduces duplication.
|
Self-merging, as the only way to try this out is by merging it to the default branch and run it. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/publish.yml (3)
45-45: Environment casing mismatch (repeat).Use the correct environment name to avoid bypassing protections.
- environment: Release + environment: release
230-230: Environment casing mismatch (repeat).Same here; align casing.
- environment: Release + environment: release
255-261: Avoid passingnullto actions/checkout repository input.When not a fork, this can resolve to the literal string "null". Split into conditional steps.
- - name: Checkout - uses: actions/checkout@v4 - with: - repository: ${{ steps.info.outputs.isFork == 'true' && steps.info.outputs.repository || null }} - ref: ${{ steps.info.outputs.sha }} - token: ${{ secrets.GH_TOKEN }} + - name: Checkout (fork) + if: steps.info.outputs.isFork == 'true' + uses: actions/checkout@v4 + with: + repository: ${{ steps.info.outputs.repository }} + ref: ${{ steps.info.outputs.sha }} + token: ${{ secrets.GH_TOKEN }} + + - name: Checkout (same repo) + if: steps.info.outputs.isFork != 'true' + uses: actions/checkout@v4 + with: + ref: ${{ steps.info.outputs.sha }} + token: ${{ secrets.GH_TOKEN }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/actions/setup-node-and-install/action.yml(1 hunks).github/workflows/generate-sandboxes.yml(6 hunks).github/workflows/prepare-non-patch-release.yml(3 hunks).github/workflows/prepare-patch-release.yml(3 hunks).github/workflows/publish.yml(8 hunks).github/workflows/tests-unit.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/actions/setup-node-and-install/action.yml
- .github/workflows/prepare-non-patch-release.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 (7)
.github/workflows/prepare-patch-release.yml (3)
33-35: LGTM: consolidated Node setup via local action.Good move to standardize setup/install.
168-168: LGTM: Discord action pinned by commit.Supply-chain hardening looks good.
22-22: Ignore environment casing — 'Release' is defined and used consistently
All workflows reference the uppercaseReleaseenvironment and it matches the configured environment.Likely an incorrect or invalid review comment.
.github/workflows/tests-unit.yml (2)
16-16: Verify runner label.“windows-11-arm” is uncommon for GitHub-hosted. Confirm a matching runner exists; otherwise use windows-latest or a matrix.
22-25: LGTM: shared setup action with code deps.Consistent with other workflows; should reduce flakiness and drift.
.github/workflows/publish.yml (2)
56-60: LGTM: shared setup action with code deps.Good consolidation; addresses prior “missing code deps before publish” concern.
Also applies to: 262-266
281-281: Pin action to full commit SHA. Replace the short SHA with the full 40-character commit:- uses: ivangabriele/find-and-replace-pull-request-body@042438c + uses: ivangabriele/find-and-replace-pull-request-body@042438c6cbfbacf6a4701d6042f59b1f73db2fd8
| if: | | ||
| ( | ||
| github.event_name == 'workflow_dispatch' || | ||
| (github.event_name == 'pull_request' && endsWith(github.head_ref, 'with-canary-release')) | ||
| ) && | ||
| contains(github.event.head_commit.message, '[skip ci]') != true | ||
| environment: Release |
There was a problem hiding this comment.
Fix [skip ci] check for PR events.
On pull_request, github.event.head_commit is undefined; contains() can error. Coalesce to empty string.
) &&
- contains(github.event.head_commit.message, '[skip ci]') != true
+ contains((github.event.head_commit.message || ''), '[skip ci]') != true📝 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.
| if: | | |
| ( | |
| github.event_name == 'workflow_dispatch' || | |
| (github.event_name == 'pull_request' && endsWith(github.head_ref, 'with-canary-release')) | |
| ) && | |
| contains(github.event.head_commit.message, '[skip ci]') != true | |
| environment: Release | |
| if: | | |
| ( | |
| github.event_name == 'workflow_dispatch' || | |
| (github.event_name == 'pull_request' && endsWith(github.head_ref, 'with-canary-release')) | |
| ) && | |
| contains((github.event.head_commit.message || ''), '[skip ci]') != true | |
| environment: Release |
🤖 Prompt for AI Agents
.github/workflows/publish.yml around lines 223-229: the contains() check uses
github.event.head_commit.message which is undefined for pull_request events and
can cause an error; wrap or coalesce the head_commit.message to an empty string
before calling contains (so that on PR events it evaluates safely) and keep the
rest of the conditional logic the same.
Release tooling: Use npm Trusted Publishing for canaries (cherry picked from commit 190788e)
github-actions
bot
added
the
patch:done
ndelangen
removed
the
patch:yes
Continues work on #32607
What I did
Migrated the canary workflow into
publish.ymlwhere normal releases happen too, because that is now the only workflow in the repo allowed by npmjs to make releases.Also hardened security in the workflows, by pinning actions to SHAs and using env vars for github expressions to avoid script injection. Also did some light cleanup of the various workflows.
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>Summary by CodeRabbit
New Features
Chores
Tests
Documentation