CLI: Add storybook ai <tool> MCP passthrough behind STORYBOOK_FEATURE_AI_CLI#35125
Conversation
…_FEATURE_AI_CLI Generic passthrough that forwards `storybook ai <tool>` calls to the MCP server of the locally running Storybook via tools/call, plus an `ai list-tools` helper. Registry resolution (~/.storybook/instances), PID liveness, version check and repair instructions are copied from @storybook/mcp-proxy (storybookjs/mcp) so the CLI behaves like the proxy. Without STORYBOOK_FEATURE_AI_CLI, `storybook ai` is unchanged (setup only). Part of #35124
… error result addon-mcp (tmcp) returns unknown-tool failures as an isError tool result rather than a JSON-RPC error, so the unknown-tool listing must also check error results against tools/list.
…rough - exhaustive never guards on the status/reason switches - placeholder output when a tool returns no content - precise undefined check for JSON-RPC results - document why the registry dir constant and extra MCP statuses are duplicated - trim provenance prose from comments
|
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:
📝 WalkthroughWalkthroughAdds an experimental storybook ai CLI passthrough backed by a local MCP: shared types, registry reading with liveness and memfs tests, precise cwd+port instance resolution, Storybook version checks, JSON-RPC client (JSON + SSE), CLI token parsing, tool invocation/help formatting, feature-flagged Commander wiring, and tests. ChangesMCP AI CLI execution passthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
code/lib/cli-storybook/src/ai/mcp/version-check.test.ts (1)
7-9: ⚡ Quick winUse Vitest spy-mock mode for the module mock.
This file-level
node:modulemock should use the repository’s spy-mocking pattern.Suggested fix
-vi.mock('node:module', () => ({ - createRequire: vi.fn(), -})); +vi.mock('node:module', { spy: true });As per coding guidelines, "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests."🤖 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/cli-storybook/src/ai/mcp/version-check.test.ts` around lines 7 - 9, Replace the file-level Vitest mock for 'node:module' to use spy-mock mode; specifically update the vi.mock invocation that defines createRequire so it includes the { spy: true } option (i.e. call vi.mock('node:module', () => ({ createRequire: vi.fn() }), { spy: true })), leaving the createRequire vi.fn() factory intact.Source: Coding guidelines
🤖 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.
Inline comments:
In `@code/lib/cli-storybook/src/ai/mcp/client.test.ts`:
- Around line 89-100: The test 'joins multi-line SSE data correctly' currently
uses JSON.stringify(envelope) which emits escaped \n sequences, so the string
stays single-line; change the serialization to produce actual newlines (e.g.,
use JSON.stringify(envelope, null, 2) or replace escaped "\\n" with real "\n")
before splitting into data lines so the dataLines construction tests the
multi-line branch (update the variable dataLines / sseBody in the test that
creates the SSE body and leave fetchImpl/sseResponse usage unchanged).
In `@code/lib/cli-storybook/src/ai/mcp/register.test.ts`:
- Around line 8-11: Update the existing vi.mock call for './run-tool.ts' to
include the spy: true option so the mock follows Vitest spy mocking guidelines;
specifically modify the vi.mock invocation that currently registers runAiTool
and runAiListTools to pass { spy: true } as the second argument, leaving the
mock implementations for runAiTool and runAiListTools unchanged.
In `@code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts`:
- Around line 9-24: The vitest mocks in run-tool.test.ts are missing spy: true
and per-test mock setups live inside individual it blocks; update the three
vi.mock(...) calls that wrap './registry.ts', './client.ts', and
'./version-check.ts' to include { spy: true } (e.g. vi.mock('./registry.ts', {
spy: true }, async (importOriginal) => ...)), and move any per-test overrides
that call vi.mocked(readRegistry).mockResolvedValue(...),
vi.mocked(callMcpTool).mockResolvedValue/...mockRejectedValue(...), and
vi.mocked(listMcpTools).mockResolvedValue(...) out of individual it(...) cases
into a shared beforeEach (or top-level setup) so tests follow the repository
mocking convention and each test can adjust those mocked return values as
needed.
- Around line 62-266: Tests mix scenario-specific vi.mocked(...) setups inside
many it blocks; extract these into focused beforeEach hooks per scenario. Create
nested describe blocks for related scenarios (e.g., "version checks", "unknown
tool handling", "registry permutations") and move inline mocks for readRegistry,
checkStorybookVersion, callMcpTool, and listMcpTools into those
describe-specific beforeEach hooks so each it only contains assertions;
alternatively add small helper functions that configure the mocks (e.g.,
setupRegistryReady(), setupMcpError(), setupCallMcpToolReturns(...)) and call
those from the it blocks to keep tests declaration-focused while still using the
existing top-level beforeEach baseline. Ensure you update tests referencing
runAiTool and runAiListTools to rely on the scenario beforeEach mocks and remove
duplicated inline vi.mocked(...) calls.
---
Nitpick comments:
In `@code/lib/cli-storybook/src/ai/mcp/version-check.test.ts`:
- Around line 7-9: Replace the file-level Vitest mock for 'node:module' to use
spy-mock mode; specifically update the vi.mock invocation that defines
createRequire so it includes the { spy: true } option (i.e. call
vi.mock('node:module', () => ({ createRequire: vi.fn() }), { spy: true })),
leaving the createRequire vi.fn() factory intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f84a35ec-c848-4630-97b1-1d6211867c1f
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
code/lib/cli-storybook/package.jsoncode/lib/cli-storybook/src/ai/mcp/client.test.tscode/lib/cli-storybook/src/ai/mcp/client.tscode/lib/cli-storybook/src/ai/mcp/intercepts.test.tscode/lib/cli-storybook/src/ai/mcp/intercepts.tscode/lib/cli-storybook/src/ai/mcp/register.test.tscode/lib/cli-storybook/src/ai/mcp/register.tscode/lib/cli-storybook/src/ai/mcp/registry.test.tscode/lib/cli-storybook/src/ai/mcp/registry.tscode/lib/cli-storybook/src/ai/mcp/resolve-instance.test.tscode/lib/cli-storybook/src/ai/mcp/resolve-instance.tscode/lib/cli-storybook/src/ai/mcp/run-tool.test.tscode/lib/cli-storybook/src/ai/mcp/run-tool.tscode/lib/cli-storybook/src/ai/mcp/tool-args.test.tscode/lib/cli-storybook/src/ai/mcp/tool-args.tscode/lib/cli-storybook/src/ai/mcp/types.tscode/lib/cli-storybook/src/ai/mcp/version-check.test.tscode/lib/cli-storybook/src/ai/mcp/version-check.tscode/lib/cli-storybook/src/bin/run.ts
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 203 | 0 |
| Self size | 908 KB | 948 KB | 🚨 +39 KB 🚨 |
| Dependency size | 88.99 MB | 88.99 MB | 🚨 +30 B 🚨 |
| Bundle Size Analyzer | Link | Link |
…p the discovery surface Review follow-ups on #35125: - copy the current @storybook/mcp-proxy resolver: most-recently-started instance selection, optional --port targeting with a port-mismatch intercept, and version checks that prefer the registry record's storybookVersion over disk resolution (resolve.paths based lookup) - replace the list-tools subcommand with help: `storybook ai --help` appends the running Storybook's tool commands to the regular help, and `storybook ai <tool> --help` shows a tool's description and arguments - document the intentionally stateless JSON-RPC shortcut and add a request timeout so a hung server cannot stall the CLI - honor -o/--output for tool results instead of silently ignoring it
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
code/lib/cli-storybook/src/ai/mcp/register.test.ts (1)
10-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
spy: trueto./run-tool.tsmock.The
vi.mock('./run-tool.ts')call should include{ spy: true }to align with the repository's Vitest mocking convention.♻️ Proposed fix
-vi.mock('./run-tool.ts', () => ({ - runAiTool: vi.fn(), - runAiToolHelp: vi.fn(), - buildToolCommandsHelp: vi.fn(), -})); +vi.mock('./run-tool.ts', { spy: true });The mock implementations in
beforeEach(lines 66-68) will continue to work with this change.🤖 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/cli-storybook/src/ai/mcp/register.test.ts` around lines 10 - 14, The vi.mock call for './run-tool.ts' is missing the spy option; update the mock invocation vi.mock('./run-tool.ts', ...) to include the { spy: true } option so it follows the repository's Vitest convention and allows the existing beforeEach mock implementations for runAiTool, runAiToolHelp, and buildToolCommandsHelp to continue working as spies.Source: Coding guidelines
code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts (1)
9-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
spy: trueto all threevi.mock()calls.All three mocks (
./registry.ts,./client.ts,./version-check.ts) should include{ spy: true }to align with the repository's Vitest mocking convention.♻️ Proposed fix
-vi.mock('./registry.ts', async (importOriginal) => ({ - ...(await importOriginal<typeof import('./registry.ts')>()), - readRegistry: vi.fn(), -})); +vi.mock('./registry.ts', { spy: true }); -vi.mock('./client.ts', async (importOriginal) => ({ - ...(await importOriginal<typeof import('./client.ts')>()), - callMcpTool: vi.fn(), - listMcpTools: vi.fn(), -})); +// Preserve the McpJsonRpcError class identity so `instanceof` checks keep working. +vi.mock('./client.ts', { spy: true }); -vi.mock('./version-check.ts', async (importOriginal) => ({ - ...(await importOriginal<typeof import('./version-check.ts')>()), - checkStorybookVersion: vi.fn(), -})); +vi.mock('./version-check.ts', { spy: true });The existing
beforeEachmock setups will continue to work with this change.🤖 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/cli-storybook/src/ai/mcp/run-tool.test.ts` around lines 9 - 24, The three vi.mock calls that mock './registry.ts', './client.ts', and './version-check.ts' need the Vitest spy option added so they follow repository convention; update each vi.mock invocation to pass { spy: true } as the second argument (preserving the existing async importOriginal factory), ensuring the mocks for readRegistry (from './registry.ts'), callMcpTool and listMcpTools (from './client.ts'), and checkStorybookVersion (from './version-check.ts') are created with spy:true so existing beforeEach spy setups continue to work.Source: Coding guidelines
🤖 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.
Inline comments:
In `@code/lib/cli-storybook/src/ai/mcp/version-check.test.ts`:
- Around line 13-15: Update the vitest mock for 'node:module' to include the spy
option (vi.mock('node:module', () => ({ createRequire: vi.fn() }), { spy: true
})) and then change the mockSearchPaths helper to call vi.mocked(createRequire)
when accessing the mocked createRequire for type-safe usage; locate the
vi.mock('node:module') declaration and the mockSearchPaths helper and replace
direct createRequire casts/usages with vi.mocked(createRequire).
---
Duplicate comments:
In `@code/lib/cli-storybook/src/ai/mcp/register.test.ts`:
- Around line 10-14: The vi.mock call for './run-tool.ts' is missing the spy
option; update the mock invocation vi.mock('./run-tool.ts', ...) to include the
{ spy: true } option so it follows the repository's Vitest convention and allows
the existing beforeEach mock implementations for runAiTool, runAiToolHelp, and
buildToolCommandsHelp to continue working as spies.
In `@code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts`:
- Around line 9-24: The three vi.mock calls that mock './registry.ts',
'./client.ts', and './version-check.ts' need the Vitest spy option added so they
follow repository convention; update each vi.mock invocation to pass { spy: true
} as the second argument (preserving the existing async importOriginal factory),
ensuring the mocks for readRegistry (from './registry.ts'), callMcpTool and
listMcpTools (from './client.ts'), and checkStorybookVersion (from
'./version-check.ts') are created with spy:true so existing beforeEach spy
setups continue to work.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00128f61-fa3e-4a20-b42b-bd7fd75ef39b
📒 Files selected for processing (15)
code/lib/cli-storybook/src/ai/mcp/client.test.tscode/lib/cli-storybook/src/ai/mcp/client.tscode/lib/cli-storybook/src/ai/mcp/intercepts.test.tscode/lib/cli-storybook/src/ai/mcp/intercepts.tscode/lib/cli-storybook/src/ai/mcp/register.test.tscode/lib/cli-storybook/src/ai/mcp/register.tscode/lib/cli-storybook/src/ai/mcp/resolve-instance.test.tscode/lib/cli-storybook/src/ai/mcp/resolve-instance.tscode/lib/cli-storybook/src/ai/mcp/run-tool.test.tscode/lib/cli-storybook/src/ai/mcp/run-tool.tscode/lib/cli-storybook/src/ai/mcp/tool-args.test.tscode/lib/cli-storybook/src/ai/mcp/tool-args.tscode/lib/cli-storybook/src/ai/mcp/types.tscode/lib/cli-storybook/src/ai/mcp/version-check.test.tscode/lib/cli-storybook/src/ai/mcp/version-check.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- code/lib/cli-storybook/src/ai/mcp/client.test.ts
- code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts
- code/lib/cli-storybook/src/ai/mcp/intercepts.ts
- code/lib/cli-storybook/src/ai/mcp/version-check.ts
- code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts
- code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts
- code/lib/cli-storybook/src/ai/mcp/types.ts
- code/lib/cli-storybook/src/ai/mcp/client.ts
Review feedback from #35125: use the repository's vi.mock(..., { spy: true }) convention instead of mock factories, and serialize the SSE fixture with indentation so the multi-line data: join branch is actually exercised.
…entation detail User-facing output no longer mentions tools or MCP: the help section is 'Storybook commands', errors say 'Storybook server', the usage line is 'ai [options] [command] [args...]', and repair instructions reference the addon without protocol jargon. Internal names keep Tool/MCP since the protocol is the implementation.
…orybookjs/storybook into kasper/ai-cli-mcp-passthrough
Canary QA report (
|
| # | Scenario | Result |
|---|---|---|
| 1 | STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai --help |
✅ regular help + "Storybook commands" section listing all 7 commands fetched live |
| 2 | ai list-all-documentation --withStoryIds true |
✅ markdown docs index, boolean coerced |
| 3 | ai get-documentation --help |
✅ per-command usage, description, arguments |
| 4 | ai get-documentation --id example-button and --json '{"id":"example-button"}' |
✅ component docs, exit 0 |
| 5 | ai not-a-command |
✅ "Unknown command" error listing available commands, exit 1 |
| 6 | Without the env var | ✅ unchanged: "too many arguments" error, exit 1; ai --help shows setup only |
| 7 | Wrong cwd (Storybook running elsewhere) | ✅ repair instructions listing running cwds, exit 1 |
| 8 | Server stopped | ✅ "Storybook is not running at this cwd…" repair, exit 1 |
| 9 | ai --help with server stopped |
✅ help still renders with "(unavailable — … start `storybook dev` …)" note, exit 0 |
No "MCP" or "tool" wording appears in any CLI output — the remaining "Use this tool to…" phrases come from @storybook/addon-mcp's own command descriptions (follow-up in storybookjs/mcp).
Restores parity with @storybook/mcp-proxy's registry schema instead of a hand-rolled type guard; valibot was already a monorepo dependency.
Found by a second thermo-nuclear review pass: the help path re-parsed from the pre-command options and dropped a token-provided --port. Both help entry points now delegate to one post-parse helper, the duplicated server-unreachable error is a single formatter, and the multi-instance warning says 'status' instead of 'mcp'.
When several instances run at the cwd, the help section names the chosen port, lists sibling ports and points at --port. The unavailable note now reflects the actual intercept (port mismatch with running ports, starting up, addon missing, too old) instead of always claiming nothing is running.
The section header includes the version reported by the running instance, and the too-old note names the detected version next to the minimum.
The endpoint comes from a world-writable registry file, so the response deserves the same scrutiny as the transport: the envelope and the tools/call / tools/list results are now parsed with loose valibot schemas (deriving the types, deleting the boundary casts). Malformed payloads produce 'unexpected response shape' instead of confusing downstream errors; extra MCP fields still pass through.
… project's version Review feedback (huang-julien): the CLI is invoked as bare 'npx storybook', so it is always the project's own Storybook and a semver floor can never disagree with the running instance. Replaced with a minimal installed-check so projects without Storybook still get the setup repair instead of 'start storybook dev'. The help header still shows the version from the registry record.
If this code executes via 'npx storybook', npx already resolved Storybook from the project — so 'is storybook installed' is answered by the CLI running at all. Resolution is now purely registry-based; a cwd without a running instance always gets the 'start storybook dev' repair.
Closes #35124
This pull request has been released as version
0.0.0-pr-35125-sha-838d82b6. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-35125-sha-838d82b6 sandboxor in an existing project withnpx storybook@0.0.0-pr-35125-sha-838d82b6 upgrade.0.0.0-pr-35125-sha-838d82b6What I did
POC for a skill + CLI based agent integration: expose the commands of the locally running Storybook's server (
@storybook/addon-mcp) as real CLI commands, so agent skills can call them directly instead of going through the stdio MCP proxy.storybook ai <command>— generic passthrough: resolves the running Storybook for the target cwd and forwards the call via MCPtools/callover HTTP. No per-command subcommands; new addon commands work for free.--help:storybook ai --help(and barestorybook ai) appends a "Storybook commands" section fetched live from the running Storybook to the regular commander help — including the instance URL, its Storybook version, and a note listing sibling instances/ports when several run at the same cwd — andstorybook ai <command> --helpprints that command's full description and arguments. Unknown command names produce an error listing the available commands.@storybook/addon-mcpand are updated separately in storybookjs/mcp.)STORYBOOK_FEATURE_AI_CLI=1is set. Without it,storybook aibehaves exactly as today (setuponly), verified by unit tests and manual QA.~/.storybook/instances/*.json), cwd matching, PID liveness, most-recently-started instance selection, optional--porttargeting (with a port-mismatch intercept) and repair-instruction markdown are copied from the current@storybook/mcp-proxy(storybookjs/mcp) intocode/lib/cli-storybook/src/ai/mcp/— no cross-repo dependency, consolidate later if the approach wins. Intercept conditions print the proxy's repair markdown and exit non-zero. The proxy's version/installed checks were dropped per review (huang-julien): the CLI is invoked as barenpx storybook, so the fact that it executes already proves the project has a compatible Storybook.--cwd <path>optional (defaults toprocess.cwd());--key value/--key=valueflags become command arguments with JSON-parse coercion (string fallback);--json '<object>'escape hatch for raw argument objects;-o/--output <path>writes the result to a file. Everything after the command name is passed through verbatim (passThroughOptions), except--cwd/--port/--json/--help, which the CLI consumes.@storybook/mcp-proxytakes against@storybook/addon-mcp's stateless tmcp HTTP transport; documented as such inclient.ts. The JSON-RPC envelope and result payloads are validated with loose valibot schemas (the endpoint comes from a world-writable registry file), with the types derived from the schemas.Follow-ups happen in storybookjs/mcp: converting the Claude Code plugin skills to use these commands, the eval comparison against the mcp-proxy plugin, and skill-level handling of old-CLI errors (
Invalid command: ai/too many arguments) as the "upgrade Storybook" signal.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
138 unit tests in
code/lib/cli-storybook/src/ai/mcp/*.test.tscover flag gating, arg mapping/coercion, registry reading (memfs), PID liveness, instance resolution (incl. most-recent selection and port targeting), the HTTP/SSE client incl. payload-shape validation, repair-instruction intercepts, the help surfaces, and the command orchestration.Manual testing
@storybook/addon-mcpinstalled and Storybook >= 10.5 (e.g. the internal-storybook app in storybookjs/mcp), runstorybook dev.STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai --help→ regular help plus the commands of the running Storybook (URL, version, sibling instances when applicable).STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai get-documentation --help→ that command's description and arguments.STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai list-all-documentation --withStoryIds true→ markdown docs index.STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai get-documentation --id <id>→ component docs (also try--json '{"id":"<id>"}'and-o out.md).STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai not-a-real-command→ error listing the available commands, exit 1.STORYBOOK_FEATURE_AI_CLI=1 npx storybook ai list-all-documentation --port 9999→ port-mismatch repair listing the running ports, exit 1.storybook dev, rerun a command → repair instructions ("start storybook dev"), exit 1;ai --helpstill works with an "unavailable" note.npx storybook ai list-all-documentation→ "too many arguments" error as before;npx storybook ai --helpshows onlysetup.All of the above were performed against the internal-storybook app from storybookjs/mcp, and re-verified end-to-end on the published canary (see the canary QA report), including the failure paths (SIGKILLed dev server with stale registry record → dead-PID detection + stale file cleanup).
Documentation
Checklist for Maintainers
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.tsqa:neededorqa:skip