Repository navigation
CLI: Use detected package manager instead of npx for remote commands - #35409
Conversation
pnpm-only repos with devEngines fail when Storybook upgrade or setup instructions invoke npm via npx. Route remote CLI packages through the detected package manager and emit matching commands in AI setup prompts.
The setup-prompt files imported getVitestStorybookRunCommand/getMswInitCommand relatively from common/js-package-manager/util.ts, so the helper params resolved to the src JsPackageManager while the passed packageManager was typed from the built dist (storybook/internal/common). The two nominally-incompatible classes caused TS2345 and broke typecheck. Export util.ts through the common barrel and import the helpers from storybook/internal/common so both sides share one type. Also drop the unused getStorybookAiSetupCommand helper (its logic is inlined in create-storybook's initiate.ts). Co-authored-by: Cursor <cursoragent@cursor.com>
…e-manager-aware-cli
There was a problem hiding this comment.
Pull request overview
Updates Storybook’s CLI dispatch and AI setup messaging to prefer the project’s detected package manager (pnpm/yarn/bun) instead of always falling back to npx, improving compatibility with repos that enforce non-npm package managers.
Changes:
- Route dispatcher “remote” execution through the detected package manager (e.g.
pnpm dlx,yarn dlx,bunx) rather than hardcodingnpx. - Make
create-storybookfinalization and AI-agent instructions display a package-manager-awarestorybook ai setupcommand. - Centralize package-manager-aware command string building for Vitest/MSW usage inside AI setup prompts.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| code/lib/create-storybook/src/initiate.ts | Passes a package-manager-aware storybook ai setup command into finalization. |
| code/lib/create-storybook/src/commands/FinalizationCommand.ts | Adds configurable aiSetupCommand and uses it in follow-up messaging/prompts. |
| code/lib/create-storybook/src/commands/FinalizationCommand.test.ts | Updates test to assert the package-manager-aware setup command is shown. |
| code/core/src/shared/utils/ai-prompts.ts | Refactors default executor constant for AI setup prompt generation. |
| code/core/src/common/js-package-manager/util.ts | Adds helpers for remote runner args + package-manager-aware Vitest/MSW command strings. |
| code/core/src/common/js-package-manager/util.test.ts | Adds unit coverage for the new js-package-manager util helpers. |
| code/core/src/common/js-package-manager/index.ts | Re-exports util helpers for broader internal consumption. |
| code/core/src/cli/ai/setup-prompts/setup.ts | Replaces hardcoded npx commands with package-manager-aware commands in instructions. |
| code/core/src/cli/ai/setup-prompts/pattern-copy-play.ts | Same as above, plus MSW init command becomes package-manager-aware. |
| code/core/src/cli/ai/setup-prompts/partials/steps.ts | Updates verification/MSW steps to use package-manager-aware commands. |
| code/core/src/cli/ai/setup-prompts/partials/rules.ts | Strengthens “don’t use npx” rule and provides the correct Storybook command form. |
| code/core/src/cli/ai/setup-prompts/partials/dod.ts | Updates DoD strings to match package-manager-aware Vitest commands. |
| code/core/src/bin/dispatcher.ts | Switches remote fallback execution from npx to detected package manager execution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 32 | 33 | 🚨 +1 🚨 |
| Self size | 22.38 MB | 22.48 MB | 🚨 +94 KB 🚨 |
| Dependency size | 20.07 MB | 20.07 MB | 🚨 +491 B 🚨 |
| Bundle Size Analyzer | Link | Link |
📝 WalkthroughWalkthroughThis PR replaces hardcoded ChangesPackage manager command generation
Estimated code review effort: 3 (Moderate) | ~30 minutes ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
code/core/src/cli/ai/setup-prompts/setup.ts (1)
158-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated command-derivation block into a shared helper.
The same
vitestRunFile/vitestRunAll/storybookBuild/storybookDoctorderivation frompackageManagerappears here and inpattern-copy-play.ts(and a similarvitestRunAll/vitestRunFilepair insteps.ts'sverifyStep). Consolidating into one helper (e.g. inutil.tsor a shared setup-prompts util) would avoid drift when a command changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/cli/ai/setup-prompts/setup.ts` around lines 158 - 162, Extract the repeated packageManager command-derivation logic into a shared helper and reuse it here, in pattern-copy-play.ts, and in steps.ts’s verifyStep so the vitestRunFile/vitestRunAll/storybookBuild/storybookDoctor values come from one place. Move the command-building into a common utility (for example in util.ts or a shared setup-prompts helper) and update the affected call sites to import and call that helper instead of duplicating the derivation.code/lib/create-storybook/src/commands/FinalizationCommand.test.ts (1)
259-301: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for
aiSetupCommandforwarding viaexecuteFinalization.Every other
FinalizationCommandOptionsfield (showAgentFollowUp,showAiInstructions,storybookCommand) has a dedicated test in this block, but the newaiSetupCommandforwarding path isn't covered here.✅ Suggested test
+ it('should forward aiSetupCommand to the command', async () => { + vi.mocked(find.up).mockReturnValue(undefined); + + await executeFinalization({ + showAgentFollowUp: true, + showAiInstructions: true, + logfile: undefined, + aiSetupCommand: 'pnpm exec storybook ai setup', + }); + + expect(logger.step).toHaveBeenCalledWith( + expect.stringContaining('pnpm exec storybook ai setup') + ); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/lib/create-storybook/src/commands/FinalizationCommand.test.ts` around lines 259 - 301, The executeFinalization helper test block is missing coverage for the new aiSetupCommand forwarding path. Add a dedicated test alongside the existing showAgentFollowUp, showAiInstructions, and storybookCommand cases that calls executeFinalization with aiSetupCommand set, then assert the command receives or logs that value through the same forwarding behavior used by FinalizationCommandOptions.code/core/src/common/js-package-manager/util.test.ts (1)
107-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
getVitestStorybookRunCommand's file argument andgetMswInitCommandwith npm.The new tests only exercise
getVitestStorybookRunCommand(pkgManager)without the optionalfileparameter, even thoughsteps.tsandpattern-copy-play.tscall it with a file path (e.g.getVitestStorybookRunCommand(packageManager, 'path/to/Foo.stories.tsx')). Similarly,getMswInitCommandis only tested with pnpm, not npm.✅ Suggested additional assertions
it('builds package-manager-aware setup commands', () => { const npm = new NPMProxy({ cwd: process.cwd(), configDir: '.storybook' }); const pnpm = new PNPMProxy({ cwd: process.cwd(), configDir: '.storybook' }); expect(getVitestStorybookRunCommand(npm)).toBe('npx vitest --project storybook run'); expect(getVitestStorybookRunCommand(pnpm)).toBe('pnpm exec vitest --project storybook run'); + expect(getVitestStorybookRunCommand(pnpm, 'path/to/Foo.stories.tsx')).toBe( + 'pnpm exec vitest --project storybook run path/to/Foo.stories.tsx' + ); expect(getMswInitCommand(pnpm)).toBe('pnpm exec msw init ./public --save'); + expect(getMswInitCommand(npm)).toBe('npx msw init ./public --save'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/common/js-package-manager/util.test.ts` around lines 107 - 114, Add coverage for the optional file-path behavior in getVitestStorybookRunCommand and for npm in getMswInitCommand. Update util.test.ts to assert getVitestStorybookRunCommand(NPMProxy, 'path/to/Foo.stories.tsx') and getVitestStorybookRunCommand(PNPMProxy, 'path/to/Foo.stories.tsx') produce the expected command including the file argument, and add an assertion for getMswInitCommand(NPMProxy) alongside the existing PNPMProxy case. Use the existing helpers getVitestStorybookRunCommand and getMswInitCommand so the tests cover the call sites used by steps.ts and pattern-copy-play.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@code/core/src/cli/ai/setup-prompts/setup.ts`:
- Around line 158-162: Extract the repeated packageManager command-derivation
logic into a shared helper and reuse it here, in pattern-copy-play.ts, and in
steps.ts’s verifyStep so the
vitestRunFile/vitestRunAll/storybookBuild/storybookDoctor values come from one
place. Move the command-building into a common utility (for example in util.ts
or a shared setup-prompts helper) and update the affected call sites to import
and call that helper instead of duplicating the derivation.
In `@code/core/src/common/js-package-manager/util.test.ts`:
- Around line 107-114: Add coverage for the optional file-path behavior in
getVitestStorybookRunCommand and for npm in getMswInitCommand. Update
util.test.ts to assert getVitestStorybookRunCommand(NPMProxy,
'path/to/Foo.stories.tsx') and getVitestStorybookRunCommand(PNPMProxy,
'path/to/Foo.stories.tsx') produce the expected command including the file
argument, and add an assertion for getMswInitCommand(NPMProxy) alongside the
existing PNPMProxy case. Use the existing helpers getVitestStorybookRunCommand
and getMswInitCommand so the tests cover the call sites used by steps.ts and
pattern-copy-play.ts.
In `@code/lib/create-storybook/src/commands/FinalizationCommand.test.ts`:
- Around line 259-301: The executeFinalization helper test block is missing
coverage for the new aiSetupCommand forwarding path. Add a dedicated test
alongside the existing showAgentFollowUp, showAiInstructions, and
storybookCommand cases that calls executeFinalization with aiSetupCommand set,
then assert the command receives or logs that value through the same forwarding
behavior used by FinalizationCommandOptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00b6171d-c5bd-4abb-86c8-82f303e8203c
📒 Files selected for processing (13)
code/core/src/bin/dispatcher.tscode/core/src/cli/ai/setup-prompts/partials/dod.tscode/core/src/cli/ai/setup-prompts/partials/rules.tscode/core/src/cli/ai/setup-prompts/partials/steps.tscode/core/src/cli/ai/setup-prompts/pattern-copy-play.tscode/core/src/cli/ai/setup-prompts/setup.tscode/core/src/common/js-package-manager/index.tscode/core/src/common/js-package-manager/util.test.tscode/core/src/common/js-package-manager/util.tscode/core/src/shared/utils/ai-prompts.tscode/lib/create-storybook/src/commands/FinalizationCommand.test.tscode/lib/create-storybook/src/commands/FinalizationCommand.tscode/lib/create-storybook/src/initiate.ts
…iSetupCommand - getRemotePackageRunnerArgs: also pass npx's `--yes` for Yarn Classic. Yarn1 runs remote packages through `npx` (Yarn1Proxy), so without `--yes` it would re-introduce the interactive install prompt/hang in CI that the pre-PR `npx --yes` fallback avoided. Documented why the flag is npx-only. - dispatcher: import JsPackageManagerFactory and getRemotePackageRunnerArgs from storybook/internal/common instead of deep relative paths. - FinalizationCommand: make aiSetupCommand required and drop the hardcoded `npx storybook ai setup` fallback, so a caller can't silently regress to npx — the sole caller (initiate.ts) already passes a package-manager-aware command. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/lib/create-storybook/src/commands/FinalizationCommand.ts (1)
88-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
getAiSetupPromptText()in the agent follow-up message.The inline text in
printSuccessMessage(Lines 96-97) duplicates the intent ofgetAiSetupPromptText()with slightly different wording. Reusing the helper avoids the two messages drifting apart if the phrasing changes later.♻️ Proposed refactor
private printSuccessMessage(storybookCommand?: string | null): void { if (this.options.showAgentFollowUp) { logger.step( CLI_COLORS.storybook( dedent`Storybook is installed but is not entirely set up yet. - To finish setting up, now run \`${this.options.aiSetupCommand}\` and follow its instructions precisely.` + To finish setting up, now ${this.getAiSetupPromptText()}` ) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/lib/create-storybook/src/commands/FinalizationCommand.ts` around lines 88 - 104, The agent follow-up message in printSuccessMessage duplicates the same setup instruction already encapsulated by getAiSetupPromptText(). Update printSuccessMessage to reuse getAiSetupPromptText() for the “run ... and follow its instructions precisely” part, keeping the Storybook-specific lead-in but removing the duplicated inline wording so FinalizationCommand stays consistent if the prompt text changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@code/lib/create-storybook/src/commands/FinalizationCommand.ts`:
- Around line 88-104: The agent follow-up message in printSuccessMessage
duplicates the same setup instruction already encapsulated by
getAiSetupPromptText(). Update printSuccessMessage to reuse
getAiSetupPromptText() for the “run ... and follow its instructions precisely”
part, keeping the Storybook-specific lead-in but removing the duplicated inline
wording so FinalizationCommand stays consistent if the prompt text changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1975800-62a3-4090-82e1-63d1e9daaec8
📒 Files selected for processing (5)
code/core/src/bin/dispatcher.tscode/core/src/common/js-package-manager/util.test.tscode/core/src/common/js-package-manager/util.tscode/lib/create-storybook/src/commands/FinalizationCommand.test.tscode/lib/create-storybook/src/commands/FinalizationCommand.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- code/core/src/common/js-package-manager/util.test.ts
- code/core/src/bin/dispatcher.ts
- code/core/src/common/js-package-manager/util.ts
- code/lib/create-storybook/src/commands/FinalizationCommand.test.ts
Closes #
What I did
Storybook's CLI dispatcher always fell back to
npxwhen@storybook/cli/create-storybookwas not available locally. That invokes npm, which fails in pnpm-only repos that enforcedevEngines.packageManager(reported in Chromatic Slack while runningpnpm dlx storybook@next upgradeandnpx storybook ai setup).pnpm dlx,bunx, etc.) instead of hardcodednpxstorybook ai setuppromptsstorybook ai setupcommand fromstorybook initfinalizationChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
No manual testing required beyond automated coverage: the regression is in CLI dispatch and prompt string generation, both covered by unit tests. A maintainer can optionally verify in a pnpm monorepo with
devEngines.packageManagerset topnpmby runningpnpm dlx storybook@next upgrade --package-manager pnpm --yesand confirming it no longer invokes npm.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.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Bug Fixes
npxapproach; external tool routing now respects the detected package manager, reducing incorrect copy/paste instructions.Tests