CLI: Bundle the ai command in core so it never downloads @storybook/cli#35147
Conversation
…rybook/cli The dispatcher only routed dev/build/index to the bundled core binary; `storybook ai` was proxied to @storybook/cli, falling back to an npx download on version mismatch. Since agent skills invoke `npx storybook ai <tool>` repeatedly, that download sat on a hot path. Move the whole ai command (MCP passthrough + setup) from @storybook/cli into code/core/src/cli/ai, register it in bin/core.ts next to dev/build, and add 'ai' to the dispatcher's bundled list. getStorybookData is reimplemented on top of core's getStorybookInfo; ProjectTypeService keeps the cross-package source import pattern. Closes #35146
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR relocates the ChangesRelocate storybook ai CLI command to core
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Comment |
…rt, shared story-paths helper - Extract detectLanguage/detectIncompatiblePackageVersions into core (storybook/internal/cli) and delegate from ProjectTypeService, so core no longer imports source from create-storybook (which depends on core). - Move getStoriesPathsFromConfig into core-server as the single shared copy; cli-storybook re-exports it and the ai module imports it, instead of two verbatim duplicates.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/cli/ai/storybook-data.ts (1)
34-36:⚠️ Potential issue | 🟠 MajorFix
workingDirderivation forconfigDir="."
With the current relative-path logic,dirname(join(process.cwd(), configDir))turnsconfigDir="."intodirname(process.cwd())(one level up), which will mis-point any setup that relies onworkingDir.Proposed fix
-import { dirname, isAbsolute, join } from 'node:path'; +import { dirname, isAbsolute, resolve } from 'node:path'; @@ - const workingDir = isAbsolute(configDir) - ? dirname(configDir) - : dirname(join(process.cwd(), configDir)); + const workingDir = isAbsolute(configDir) + ? dirname(configDir) + : resolve(process.cwd(), dirname(configDir));🤖 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/storybook-data.ts` around lines 34 - 36, The workingDir calculation incorrectly uses dirname(join(process.cwd(), configDir)) for relative paths, which turns configDir="." into dirname(process.cwd()) (one level up); update the logic in the workingDir derivation (the expression using isAbsolute, dirname, join, process.cwd(), and configDir) to special-case configDir === "." (or equivalently when join(process.cwd(), configDir) equals process.cwd()) and return process.cwd() directly for that case, otherwise keep the existing dirname(join(process.cwd(), configDir)) branch; ensure absolute paths still use dirname(configDir).
🤖 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.
Outside diff comments:
In `@code/core/src/cli/ai/storybook-data.ts`:
- Around line 34-36: The workingDir calculation incorrectly uses
dirname(join(process.cwd(), configDir)) for relative paths, which turns
configDir="." into dirname(process.cwd()) (one level up); update the logic in
the workingDir derivation (the expression using isAbsolute, dirname, join,
process.cwd(), and configDir) to special-case configDir === "." (or equivalently
when join(process.cwd(), configDir) equals process.cwd()) and return
process.cwd() directly for that case, otherwise keep the existing
dirname(join(process.cwd(), configDir)) branch; ensure absolute paths still use
dirname(configDir).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f966a75-0e0b-46e7-a534-5da571791031
📒 Files selected for processing (2)
code/core/src/cli/ai/storybook-data.tsscripts/eval/README.md
… in detectLanguage Addresses the codex thermo-nuclear review: - getStorybookData moves to storybook/internal/cli as the one canonical project-metadata collector; automigrate/doctor/add delegate to it via a re-export instead of keeping a near-duplicate. - detectLanguage takes an explicit workingDir for its js/tsconfig lookup (defaulting to process.cwd() for init), and ai setup passes the target Storybook's working dir instead of relying on the process cwd.
Independent review findings and resolutionsTwo independent reviews were run per #35146: the
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/cli/getStorybookData.ts (1)
40-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
workingDiranchored to the project root for root-level config directories.Lines 40-42 always strip one path segment from
configDirbefore exposingworkingDiron Line 68. That works forpackages/foo/.storybook, but--config-dir .or an absolute project-root config directory resolvesworkingDirto the parent of the project instead.aiSetupnow passes this value intodetectLanguage(), so JS/TS detection can read the wrongjsconfig.json/tsconfig.jsonand emit the wrong prompt. Please deriveworkingDirfrom the actual project directory instead of unconditionally callingdirname(configDir).Also applies to: 68-68
🤖 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/getStorybookData.ts` around lines 40 - 42, The workingDir computation currently always does dirname(configDir), which wrongly returns the project parent for root-level config dirs; change workingDir so it resolves configDir first (resolve(join/process.cwd() or use the absolute path) and then: if the resolved configDir is the project root (resolvedConfigDir === process.cwd()) or does not point into a subfolder named ".storybook", set workingDir = resolvedConfigDir (or process.cwd()), otherwise set workingDir = dirname(resolvedConfigDir); update the code that defines workingDir (the const workingDir using isAbsolute/configDir) and ensure aiSetup/detectLanguage receive this corrected workingDir so JS/TS detection reads the correct jsconfig.json/tsconfig.json.
🤖 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.
Outside diff comments:
In `@code/core/src/cli/getStorybookData.ts`:
- Around line 40-42: The workingDir computation currently always does
dirname(configDir), which wrongly returns the project parent for root-level
config dirs; change workingDir so it resolves configDir first
(resolve(join/process.cwd() or use the absolute path) and then: if the resolved
configDir is the project root (resolvedConfigDir === process.cwd()) or does not
point into a subfolder named ".storybook", set workingDir = resolvedConfigDir
(or process.cwd()), otherwise set workingDir = dirname(resolvedConfigDir);
update the code that defines workingDir (the const workingDir using
isAbsolute/configDir) and ensure aiSetup/detectLanguage receive this corrected
workingDir so JS/TS detection reads the correct jsconfig.json/tsconfig.json.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a6c4d88-d845-4d92-84ed-6ce3f8a224ba
📒 Files selected for processing (5)
code/core/src/cli/ai/index.tscode/core/src/cli/detectLanguage.tscode/core/src/cli/getStorybookData.tscode/core/src/cli/index.tscode/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- code/core/src/cli/index.ts
- code/core/src/cli/detectLanguage.ts
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 72 | 72 | 0 |
| Self size | 20.86 MB | 21.01 MB | 🚨 +157 KB 🚨 |
| Dependency size | 36.12 MB | 36.12 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 203 | 0 |
| Self size | 952 KB | 802 KB | 🎉 -150 KB 🎉 |
| Dependency size | 89.01 MB | 89.17 MB | 🚨 +155 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 196 | 0 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 87.50 MB | 87.66 MB | 🚨 +157 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 1.08 MB | 1.08 MB | 🎉 -2 KB 🎉 |
| Dependency size | 56.97 MB | 57.13 MB | 🚨 +157 KB 🚨 |
| Bundle Size Analyzer | node | node |
…-export Second thermo-nuclear review pass: detectLanguage's workingDir scoping is an intended fix (config files are found next to the target Storybook, not the invoking cwd) — covered with memfs tests; the re-export in util.ts moves below the import block.
…sper/ai-cli-bundled
`dirname(join(process.cwd(), configDir))` collapsed `--config-dir .` to the project's parent directory, mispointing story-glob resolution and the tsconfig/jsconfig lookup in detectLanguage. Taking dirname before resolving keeps root-level config dirs anchored to the project root. Pre-existing on next, surfaced by the move (CodeRabbit review on #35147).
|
CodeRabbit's outside-diff finding on the |
Closes #35146
What I did
Yann found that
npx storybook aiwas not using the local binary: the dispatcher only routeddev/build/indexto the bundledstorybook/dist/bin/core.js, soaiwas proxied to@storybook/cli— falling back to annpx --yes @storybook/cli@<version>download whenever the package wasn't locally installed at the pinned version. Since agent skills invokenpx storybook ai <tool>repeatedly, that download sat on a hot path (Slack).aicommand — theSTORYBOOK_FEATURE_AI_CLI-gated MCP passthrough and the unflaggedai setup(setup-prompts, utils, types, tests) — fromcode/lib/cli-storybook/src/ai/tocode/core/src/cli/ai/, registered incode/core/src/bin/core.tsnext todev/build.aito the dispatcher's bundled-commands list.aicommand from@storybook/cli(bin/run.ts). No shim: the dispatcher pins CLI versions, so olderstorybookversions keep their old paired CLI.getStorybookDatais now the one canonical project-metadata collector, living in core and exported viastorybook/internal/cli; cli-storybook'sautomigrate/doctor/addconsume it through a re-export instead of keeping their near-duplicate.getStoriesPathsFromConfigsimilarly moved to a single copy instorybook/internal/core-server.detectLanguage/detectIncompatiblePackageVersionsextracted fromcreate-storybook'sProjectTypeServiceinto core (storybook/internal/cli), withProjectTypeServicedelegating — so core never imports source from a package that depends on core.storybook ai <command>passthrough #35138 is untouched.Known trade-off (decided in #35146):
npx storybook --help(which routes to@storybook/cli) no longer listsai;npx storybook ai --helpis the entry point.Two independent reviews (the
thermo-nuclear-code-quality-reviewagent, and Codex xhigh running the same skill from the cursor-team-kit plugin) were run; all findings fixed or explicitly resolved — see the review-resolution comment.Note: this PR is stacked on #35138 (
kasper/ai-cli-telemetry); retarget tonextonce that merges.How to test
Manual QA performed in a linked
react-vite/default-tssandbox with a cleared npx cache (~/.npm/_npxremoved) and Storybook dev running:STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai --helpshows static options plus the live commands fetched from the running Storybook.STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai list-all-documentationandpreview-storiessucceed;~/.npm/_npxstays empty — even withnode_modules/@storybook/cliremoved entirely, which previously forced the npx download.npx storybook ai setupgenerates the setup prompt (language detection now scoped to the target working dir).npx storybook doctorstill routes to@storybook/clivia the dispatcher, and works through the delegatedgetStorybookData.STORYBOOK_TELEMETRY_DEBUG=1shows theai-commandevent firing withcommand/success/durationand target-project metadata, identical to CLI: Add telemetry for thestorybook ai <command>passthrough #35138.yarn nx run-many -t checkpasses (thenextjs-vite:checkfailure is pre-existing on the base branch).Summary by CodeRabbit
Release Notes
New Features
Improvements