From 0a3346fe56dad3d09ee5002034b35fbbd946199a Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 12:50:30 +0200 Subject: [PATCH 01/13] feat(cli): add `storybook ai ` MCP passthrough behind STORYBOOK_FEATURE_AI_CLI Generic passthrough that forwards `storybook ai ` 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 --- code/lib/cli-storybook/package.json | 1 + .../cli-storybook/src/ai/mcp/client.test.ts | 174 +++++++++++++ code/lib/cli-storybook/src/ai/mcp/client.ts | 148 +++++++++++ .../src/ai/mcp/intercepts.test.ts | 59 +++++ .../cli-storybook/src/ai/mcp/intercepts.ts | 76 ++++++ .../cli-storybook/src/ai/mcp/register.test.ts | 153 +++++++++++ code/lib/cli-storybook/src/ai/mcp/register.ts | 67 +++++ .../cli-storybook/src/ai/mcp/registry.test.ts | 143 +++++++++++ code/lib/cli-storybook/src/ai/mcp/registry.ts | 110 ++++++++ .../src/ai/mcp/resolve-instance.test.ts | 125 +++++++++ .../src/ai/mcp/resolve-instance.ts | 83 ++++++ .../cli-storybook/src/ai/mcp/run-tool.test.ts | 238 ++++++++++++++++++ code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 222 ++++++++++++++++ .../src/ai/mcp/tool-args.test.ts | 136 ++++++++++ .../lib/cli-storybook/src/ai/mcp/tool-args.ts | 101 ++++++++ code/lib/cli-storybook/src/ai/mcp/types.ts | 61 +++++ .../src/ai/mcp/version-check.test.ts | 65 +++++ .../cli-storybook/src/ai/mcp/version-check.ts | 44 ++++ code/lib/cli-storybook/src/bin/run.ts | 7 + yarn.lock | 1 + 20 files changed, 2014 insertions(+) create mode 100644 code/lib/cli-storybook/src/ai/mcp/client.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/client.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/intercepts.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/register.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/register.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/registry.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/registry.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/run-tool.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/tool-args.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/types.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/version-check.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/version-check.ts diff --git a/code/lib/cli-storybook/package.json b/code/lib/cli-storybook/package.json index c666819df5d7..48bc59cd80dc 100644 --- a/code/lib/cli-storybook/package.json +++ b/code/lib/cli-storybook/package.json @@ -54,6 +54,7 @@ "envinfo": "^7.14.0", "globby": "^14.1.0", "leven": "^4.0.0", + "memfs": "^4.11.1", "p-limit": "^7.2.0", "picocolors": "^1.1.0", "semver": "^7.7.3", diff --git a/code/lib/cli-storybook/src/ai/mcp/client.test.ts b/code/lib/cli-storybook/src/ai/mcp/client.test.ts new file mode 100644 index 000000000000..fe0f2224f7f5 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/client.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; +import type { StorybookInstanceRecord } from './types.ts'; + +const record: StorybookInstanceRecord = { + schemaVersion: 1, + instanceId: 'i-1', + pid: 1, + cwd: '/projects/foo', + url: 'http://localhost:6006', + port: 6006, + mcp: { status: 'ready', endpoint: '/mcp' }, +}; + +const jsonResponse = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }); + +const sseResponse = (body: string, status = 200) => + new Response(body, { + status, + headers: { 'Content-Type': 'text/event-stream' }, + }); + +describe('callMcpTool', () => { + it('POSTs a JSON-RPC tools/call request to the endpoint (application/json)', async () => { + const fetchImpl = vi.fn(async () => + jsonResponse({ + jsonrpc: '2.0', + id: 'whatever', + result: { content: [{ type: 'text', text: 'hello' }] }, + }) + ) as unknown as typeof fetch; + + const result = await callMcpTool( + record, + { name: 'list-all-documentation', arguments: { withStoryIds: true } }, + fetchImpl + ); + + expect(result.content).toEqual([{ type: 'text', text: 'hello' }]); + + const call = vi.mocked(fetchImpl).mock.calls[0]; + expect(call[0]).toBe('http://localhost:6006/mcp'); + const init = call[1] as RequestInit; + const headers = init.headers as Record; + expect(headers.Accept).toBe('application/json, text/event-stream'); + expect(headers['X-Storybook-MCP-Proxy']).toBe('true'); + const body = JSON.parse(init.body as string); + expect(body).toMatchObject({ + jsonrpc: '2.0', + method: 'tools/call', + params: { + name: 'list-all-documentation', + arguments: { withStoryIds: true }, + }, + }); + expect(typeof body.id).toBe('string'); + }); + + it('resolves the endpoint path against the instance url without mangling the scheme', async () => { + const fetchImpl = vi.fn(async () => + jsonResponse({ jsonrpc: '2.0', id: 'whatever', result: { content: [] } }) + ) as unknown as typeof fetch; + + await callMcpTool( + { ...record, url: 'http://127.0.0.1:6007', mcp: { status: 'ready', endpoint: '/mcp' } }, + { name: 'list-all-documentation' }, + fetchImpl + ); + + expect(vi.mocked(fetchImpl).mock.calls[0][0]).toBe('http://127.0.0.1:6007/mcp'); + }); + + it('parses a single-event SSE response (text/event-stream)', async () => { + const sseBody = + 'event: message\n' + + 'data: {"jsonrpc":"2.0","id":1,"result":{"content":[{"type":"text","text":"hi"}]}}\n' + + '\n'; + const fetchImpl = (async () => sseResponse(sseBody)) as typeof fetch; + + const result = await callMcpTool(record, { name: 'list-all-documentation' }, fetchImpl); + expect(result.content).toEqual([{ type: 'text', text: 'hi' }]); + }); + + it('joins multi-line SSE data correctly', async () => { + const envelope = { + jsonrpc: '2.0', + id: 1, + result: { content: [{ type: 'text', text: 'line\nwith newline' }] }, + }; + const dataLines = JSON.stringify(envelope) + .split('\n') + .map((l) => `data: ${l}`) + .join('\n'); + const sseBody = `event: message\n${dataLines}\n\n`; + const fetchImpl = (async () => sseResponse(sseBody)) as typeof fetch; + + const result = await callMcpTool(record, { name: 'list-all-documentation' }, fetchImpl); + expect(result.content?.[0]).toEqual({ type: 'text', text: 'line\nwith newline' }); + }); + + it('throws on SSE responses that contain no data event', async () => { + const fetchImpl = (async () => sseResponse('event: ping\n\n')) as typeof fetch; + await expect( + callMcpTool(record, { name: 'list-all-documentation' }, fetchImpl) + ).rejects.toThrow(/SSE response with no data event/); + }); + + it('throws when the record has no mcp.endpoint', async () => { + const noEndpoint: StorybookInstanceRecord = { ...record, mcp: { status: 'ready' } }; + const fetchImpl = vi.fn() as unknown as typeof fetch; + await expect( + callMcpTool(noEndpoint, { name: 'list-all-documentation' }, fetchImpl) + ).rejects.toThrow(/missing mcp\.endpoint/); + }); + + it('throws when the response is not ok', async () => { + const fetchImpl = (async () => + new Response('boom', { status: 500, statusText: 'Server Error' })) as typeof fetch; + await expect( + callMcpTool(record, { name: 'list-all-documentation' }, fetchImpl) + ).rejects.toThrow(/responded with 500/); + }); + + it('throws when the response content-type is neither JSON nor SSE', async () => { + const fetchImpl = (async () => + new Response('', { + status: 200, + headers: { 'Content-Type': 'text/html' }, + })) as typeof fetch; + await expect( + callMcpTool(record, { name: 'list-all-documentation' }, fetchImpl) + ).rejects.toThrow(/unsupported content-type "text\/html"/); + }); + + it('throws an McpJsonRpcError when the JSON-RPC payload carries an error', async () => { + const fetchImpl = (async () => + jsonResponse({ + jsonrpc: '2.0', + id: 'whatever', + error: { code: -32601, message: 'unknown tool' }, + })) as typeof fetch; + const promise = callMcpTool(record, { name: 'nope' }, fetchImpl); + await expect(promise).rejects.toThrow(/Storybook MCP error -32601: unknown tool/); + await expect(promise).rejects.toBeInstanceOf(McpJsonRpcError); + }); +}); + +describe('listMcpTools', () => { + it('POSTs a JSON-RPC tools/list request and returns the tool descriptors', async () => { + const tools = [ + { name: 'get-documentation', description: 'Docs', inputSchema: { properties: {} } }, + { name: 'list-all-documentation' }, + ]; + const fetchImpl = vi.fn(async () => + jsonResponse({ jsonrpc: '2.0', id: 'x', result: { tools } }) + ) as unknown as typeof fetch; + + await expect(listMcpTools(record, fetchImpl)).resolves.toEqual(tools); + + const body = JSON.parse(vi.mocked(fetchImpl).mock.calls[0][1]?.body as string); + expect(body).toMatchObject({ method: 'tools/list', params: {} }); + }); + + it('returns [] when the result has no tools array', async () => { + const fetchImpl = (async () => + jsonResponse({ jsonrpc: '2.0', id: 'x', result: {} })) as typeof fetch; + await expect(listMcpTools(record, fetchImpl)).resolves.toEqual([]); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/client.ts b/code/lib/cli-storybook/src/ai/mcp/client.ts new file mode 100644 index 000000000000..22fa4459b227 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/client.ts @@ -0,0 +1,148 @@ +import type { McpToolDescriptor, StorybookInstanceRecord, ToolCallResult } from './types.ts'; + +/** + * Marks the request as coming from a trusted local Storybook client. `@storybook/addon-mcp` uses + * this header to skip auth flows meant for remote (composed) Storybooks. + */ +const STORYBOOK_MCP_PROXY_HEADER = 'X-Storybook-MCP-Proxy'; +const STORYBOOK_MCP_PROXY_HEADER_VALUE = 'true'; + +export type ToolCallParams = { + name: string; + arguments?: Record; +}; + +/** A JSON-RPC level error returned by the Storybook MCP server (e.g. unknown tool). */ +export class McpJsonRpcError extends Error { + constructor( + public readonly code: number, + message: string + ) { + super(`Storybook MCP error ${code}: ${message}`); + this.name = 'McpJsonRpcError'; + } +} + +/** Forward an MCP `tools/call` JSON-RPC request to a local Storybook MCP server. */ +export async function callMcpTool( + record: StorybookInstanceRecord, + params: ToolCallParams, + fetchImpl: typeof fetch = fetch +): Promise { + return (await sendJsonRpcRequest(record, 'tools/call', params, fetchImpl)) as ToolCallResult; +} + +/** List the tools exposed by a local Storybook MCP server via `tools/list`. */ +export async function listMcpTools( + record: StorybookInstanceRecord, + fetchImpl: typeof fetch = fetch +): Promise { + const result = (await sendJsonRpcRequest(record, 'tools/list', {}, fetchImpl)) as { + tools?: McpToolDescriptor[]; + }; + return result.tools ?? []; +} + +/** + * Send a single JSON-RPC request to the instance's MCP endpoint over HTTP. + * + * The downstream is `@storybook/addon-mcp` at `record.mcp.endpoint`. tmcp's HttpTransport + * hardcodes `text/event-stream` for any request with an id, so we accept both content-types and + * parse the SSE envelope when needed. Every call is independent; no session bookkeeping needed. + */ +async function sendJsonRpcRequest( + record: StorybookInstanceRecord, + method: 'tools/call' | 'tools/list', + params: unknown, + fetchImpl: typeof fetch +): Promise { + const endpoint = record.mcp.endpoint; + if (!endpoint) { + throw new Error(`Storybook MCP record for ${record.cwd} is missing mcp.endpoint`); + } + + const target = new URL(endpoint, record.url).href; + + const response = await fetchImpl(target, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream', + [STORYBOOK_MCP_PROXY_HEADER]: STORYBOOK_MCP_PROXY_HEADER_VALUE, + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: crypto.randomUUID(), + method, + params, + }), + }); + + if (!response.ok) { + throw new Error( + `Storybook MCP at ${target} responded with ${response.status} ${response.statusText}` + ); + } + + const payload = (await readJsonRpcResponse(response, target)) as { + result?: unknown; + error?: { code: number; message: string }; + }; + + if (payload.error) { + throw new McpJsonRpcError(payload.error.code, payload.error.message); + } + if (!payload.result) { + throw new Error('Storybook MCP returned no result'); + } + return payload.result; +} + +async function readJsonRpcResponse(response: Response, endpoint: string): Promise { + const contentType = (response.headers.get('content-type') ?? '').toLowerCase(); + + if (contentType.includes('application/json')) { + return await response.json(); + } + + if (contentType.includes('text/event-stream')) { + return parseSseEnvelope(await response.text(), endpoint); + } + + throw new Error( + `Storybook MCP at ${endpoint} returned unsupported content-type "${contentType}". Expected application/json or text/event-stream.` + ); +} + +/** + * Parse an MCP Streamable HTTP SSE response containing a single JSON-RPC envelope. Format per the + * SSE spec: lines starting with `data:` hold payload bytes; multiple `data:` lines in one event + * are joined with `\n`; the event terminates at the first blank line. We only care about the first + * event because a tools/call or tools/list response is always a single message. + */ +function parseSseEnvelope(body: string, endpoint: string): unknown { + const dataLines: string[] = []; + for (const rawLine of body.split('\n')) { + const line = rawLine.replace(/\r$/, ''); + if (line.startsWith('data:')) { + const value = line.slice(5); + dataLines.push(value.startsWith(' ') ? value.slice(1) : value); + continue; + } + if (line === '' && dataLines.length > 0) { + break; + } + } + if (dataLines.length === 0) { + throw new Error(`Storybook MCP at ${endpoint} returned an SSE response with no data event`); + } + try { + return JSON.parse(dataLines.join('\n')); + } catch (error) { + throw new Error( + `Storybook MCP at ${endpoint} returned an SSE event whose data could not be parsed as JSON: ${ + error instanceof Error ? error.message : String(error) + }` + ); + } +} diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts new file mode 100644 index 000000000000..4063737b6045 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest'; + +import { getInterceptMarkdown } from './intercepts.ts'; +import type { StorybookInstanceRecord } from './types.ts'; + +const record = (cwd: string, url: string): StorybookInstanceRecord => ({ + schemaVersion: 1, + instanceId: 'i-1', + pid: 1, + cwd, + url, + port: 6006, + mcp: { status: 'ready', endpoint: '/mcp' }, +}); + +describe('getInterceptMarkdown', () => { + it('no-instance without candidates tells the agent to start storybook dev', () => { + const markdown = getInterceptMarkdown('no-instance'); + expect(markdown).toContain('Storybook is not running at this cwd'); + expect(markdown).toContain('storybook dev'); + }); + + it('no-instance with candidates lists the running cwds', () => { + const markdown = getInterceptMarkdown('no-instance', { + records: [record('/projects/foo', 'http://localhost:6006')], + }); + expect(markdown).toContain('Running Storybooks:'); + expect(markdown).toContain('- `/projects/foo` (http://localhost:6006)'); + expect(markdown).toContain('--cwd'); + }); + + it('storybook-too-old names the detected and minimum versions and the upgrade skill', () => { + const markdown = getInterceptMarkdown('storybook-too-old', { version: '9.0.5' }); + expect(markdown).toContain('`9.0.5`'); + expect(markdown).toContain('`10.5.0`'); + expect(markdown).toContain('storybook-upgrade'); + expect(markdown).toContain('npx storybook add @storybook/addon-mcp'); + }); + + it('storybook-not-installed points at the init skill and the addon install', () => { + const markdown = getInterceptMarkdown('storybook-not-installed'); + expect(markdown).toContain('storybook-init'); + expect(markdown).toContain('npx storybook add @storybook/addon-mcp'); + }); + + it('addon-missing instructs installing the MCP addon', () => { + const markdown = getInterceptMarkdown('addon-missing'); + expect(markdown).toContain('`@storybook/addon-mcp` addon is missing'); + expect(markdown).toContain('npx storybook add @storybook/addon-mcp'); + }); + + it('mcp-starting asks to wait and retry', () => { + expect(getInterceptMarkdown('mcp-starting')).toContain('still starting up'); + }); + + it('mcp-error points at the Storybook terminal output', () => { + expect(getInterceptMarkdown('mcp-error')).toContain('Inspect the Storybook terminal output'); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts new file mode 100644 index 000000000000..21dd3152d215 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts @@ -0,0 +1,76 @@ +import type { InterceptReason, StorybookInstanceRecord } from './types.ts'; +import { STORYBOOK_MIN_VERSION } from './version-check.ts'; + +/** + * Repair-instruction markdown, copied from `@storybook/mcp-proxy` so agents get the same guidance + * from the CLI as from the MCP proxy. Wording is adapted from "retry the tool call" to "retry the + * command", and the proxy's version-cache instructions are dropped (the CLI is one-shot and + * re-detects the version on every run). + */ +const NO_INSTANCE_EMPTY = `Storybook is not running at this cwd. Start \`storybook dev\` from the project's cwd and retry the command.`; + +const buildNoInstanceWithCandidates = (records: StorybookInstanceRecord[]) => + `No Storybook is running at this cwd. Either start Storybook from the project's cwd, or retry with \`--cwd\` set to one of the running cwds below. + +Running Storybooks: +${records.map((r) => `- \`${r.cwd}\` (${r.url})`).join('\n')}`; + +const buildStorybookTooOld = (version: string) => + `The Storybook installed at this cwd is version \`${version}\`, but this command requires \`${STORYBOOK_MIN_VERSION}\` or newer. + +Ask the user whether they want to upgrade Storybook. If they agree, invoke the \`storybook-upgrade\` skill to perform the upgrade, then run: +\`\`\` +npx storybook add @storybook/addon-mcp +\`\`\` +to install the MCP addon. Restart Storybook, then retry the command.`; + +const STORYBOOK_NOT_INSTALLED = `No Storybook is running at this cwd, and Storybook does not appear to be installed here (\`storybook\` could not be resolved from this project). + +Ask the user whether they want to add Storybook. If they agree, invoke the \`storybook-init\` skill to set it up, then install the MCP addon: +\`\`\` +npx storybook add @storybook/addon-mcp +\`\`\` +Start Storybook, then retry the command. + +If you believe Storybook is in fact installed (e.g. a monorepo where \`storybook\` resolves from a different location), start \`storybook dev\` from this exact cwd and retry — a running instance is always used regardless of this check.`; + +const ADDON_MISSING = `Storybook is running but does not expose an MCP server. The \`@storybook/addon-mcp\` addon is missing. + +Install it: +\`\`\` +npx storybook add @storybook/addon-mcp +\`\`\` + +Restart Storybook, then retry the command.`; + +const MCP_STARTING = `Storybook is running but its MCP server is still starting up. Wait a moment and retry the command.`; + +const MCP_ERROR = `Storybook is running but its MCP server reported an error. Inspect the Storybook terminal output, fix the underlying issue, then retry the command.`; + +export type InterceptExtras = { + records?: StorybookInstanceRecord[]; + version?: string; +}; + +export function getInterceptMarkdown( + reason: InterceptReason, + extras: InterceptExtras = {} +): string { + const { records, version } = extras; + switch (reason) { + case 'no-instance': + return records && records.length > 0 + ? buildNoInstanceWithCandidates(records) + : NO_INSTANCE_EMPTY; + case 'storybook-not-installed': + return STORYBOOK_NOT_INSTALLED; + case 'addon-missing': + return ADDON_MISSING; + case 'mcp-starting': + return MCP_STARTING; + case 'mcp-error': + return MCP_ERROR; + case 'storybook-too-old': + return buildStorybookTooOld(version ?? 'unknown'); + } +} diff --git a/code/lib/cli-storybook/src/ai/mcp/register.test.ts b/code/lib/cli-storybook/src/ai/mcp/register.test.ts new file mode 100644 index 000000000000..916272eedd10 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/register.test.ts @@ -0,0 +1,153 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { Command } from 'commander'; + +import { isAiCliFeatureEnabled, registerAiMcpPassthrough } from './register.ts'; +import { runAiListTools, runAiTool } from './run-tool.ts'; + +vi.mock('./run-tool.ts', () => ({ + runAiTool: vi.fn(), + runAiListTools: vi.fn(), +})); + +describe('isAiCliFeatureEnabled', () => { + it.each([ + ['1', true], + ['true', true], + ['0', false], + ['false', false], + ['', false], + [undefined, false], + ])('STORYBOOK_FEATURE_AI_CLI=%j → %j', (value, expected) => { + expect(isAiCliFeatureEnabled({ STORYBOOK_FEATURE_AI_CLI: value })).toBe(expected); + }); +}); + +/** Replicate the `ai` command tree from `bin/run.ts`: a `setup` subcommand plus a help action. */ +function buildProgram({ withPassthrough }: { withPassthrough: boolean }) { + const program = new Command(); + program.exitOverride(); + const setupAction = vi.fn(); + const helpAction = vi.fn(); + + const aiCommand = program + .command('ai') + .description('AI agent helpers for Storybook') + .option('-o, --output ', 'Write the prompt output to a file') + .exitOverride(); + aiCommand.configureOutput({ writeOut: () => {}, writeErr: () => {} }); + aiCommand.command('setup').action(setupAction); + aiCommand.action(helpAction); + + if (withPassthrough) { + registerAiMcpPassthrough(program, aiCommand); + } + + return { program, aiCommand, setupAction, helpAction }; +} + +function parse(program: Command, argv: string[]) { + return program.parseAsync(['node', 'storybook', ...argv]); +} + +beforeEach(() => { + vi.mocked(runAiTool).mockResolvedValue({ exitCode: 0, output: 'ok' }); + vi.mocked(runAiListTools).mockResolvedValue({ exitCode: 0, output: 'tools' }); + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.mocked(runAiTool).mockReset(); + vi.mocked(runAiListTools).mockReset(); + process.exitCode = undefined; +}); + +describe('without the feature flag (no registration)', () => { + it('does not expose list-tools', async () => { + const { program, aiCommand } = buildProgram({ withPassthrough: false }); + expect(aiCommand.commands.map((c) => c.name())).toEqual(['setup']); + await expect(parse(program, ['ai', 'list-tools'])).rejects.toMatchObject({ + code: 'commander.excessArguments', + }); + expect(runAiListTools).not.toHaveBeenCalled(); + }); + + it('rejects tool names like today (excess arguments)', async () => { + const { program } = buildProgram({ withPassthrough: false }); + await expect(parse(program, ['ai', 'list-all-documentation'])).rejects.toMatchObject({ + code: 'commander.excessArguments', + }); + expect(runAiTool).not.toHaveBeenCalled(); + }); + + it('keeps the bare `ai` help action', async () => { + const { program, helpAction } = buildProgram({ withPassthrough: false }); + await parse(program, ['ai']); + expect(helpAction).toHaveBeenCalled(); + }); +}); + +describe('with the feature flag (passthrough registered)', () => { + it('forwards `ai ` with pass-through tokens to runAiTool', async () => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', 'get-documentation', '--id', 'button-docs']); + expect(runAiTool).toHaveBeenCalledWith('get-documentation', ['--id', 'button-docs'], { + cwd: undefined, + json: undefined, + }); + }); + + it('parses --cwd and --json before the tool name as CLI options', async () => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', '--cwd', '/x', '--json', '{"a":1}', 'get-documentation']); + expect(runAiTool).toHaveBeenCalledWith('get-documentation', [], { cwd: '/x', json: '{"a":1}' }); + }); + + it('passes tokens after the tool name through verbatim, even option-like ones', async () => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', 'tool-x', '--cwd', '/y', '--output', 'z']); + expect(runAiTool).toHaveBeenCalledWith('tool-x', ['--cwd', '/y', '--output', 'z'], { + cwd: undefined, + json: undefined, + }); + }); + + it('writes the result to stdout', async () => { + const { program } = buildProgram({ withPassthrough: true }); + vi.mocked(runAiTool).mockResolvedValue({ exitCode: 0, output: 'markdown result' }); + await parse(program, ['ai', 'tool-x']); + expect(process.stdout.write).toHaveBeenCalledWith('markdown result\n'); + expect(process.exitCode).toBeUndefined(); + }); + + it('sets a non-zero exit code on failure', async () => { + const { program } = buildProgram({ withPassthrough: true }); + vi.mocked(runAiTool).mockResolvedValue({ exitCode: 1, output: 'repair instructions' }); + await parse(program, ['ai', 'tool-x']); + expect(process.stdout.write).toHaveBeenCalledWith('repair instructions\n'); + expect(process.exitCode).toBe(1); + }); + + it('dispatches `ai list-tools` to runAiListTools with the cwd option', async () => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', 'list-tools', '--cwd', '/x']); + expect(runAiListTools).toHaveBeenCalledWith({ cwd: '/x' }); + expect(runAiTool).not.toHaveBeenCalled(); + }); + + it('still dispatches `ai setup` to the setup subcommand', async () => { + const { program, setupAction } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', 'setup']); + expect(setupAction).toHaveBeenCalled(); + expect(runAiTool).not.toHaveBeenCalled(); + }); + + it('shows help when `ai` is run without a tool', async () => { + const { program, aiCommand } = buildProgram({ withPassthrough: true }); + const outputHelp = vi.spyOn(aiCommand, 'outputHelp').mockImplementation(() => ''); + await parse(program, ['ai']); + expect(outputHelp).toHaveBeenCalled(); + expect(runAiTool).not.toHaveBeenCalled(); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/register.ts b/code/lib/cli-storybook/src/ai/mcp/register.ts new file mode 100644 index 000000000000..e8713c71bfbc --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/register.ts @@ -0,0 +1,67 @@ +import { optionalEnvToBoolean } from 'storybook/internal/common'; + +import type { Command } from 'commander'; + +import { type AiToolRunResult, runAiListTools, runAiTool } from './run-tool.ts'; + +/** + * The `storybook ai ` MCP passthrough is experimental (storybookjs/storybook#35124) and only + * registered when this feature flag is set; without it, `storybook ai` exposes `setup` only. + */ +export function isAiCliFeatureEnabled(env: NodeJS.ProcessEnv = process.env): boolean { + return optionalEnvToBoolean(env.STORYBOOK_FEATURE_AI_CLI) === true; +} + +const CWD_DESCRIPTION = + 'Project directory of the target Storybook (defaults to the current working directory)'; + +/** + * Register the MCP passthrough on the `ai` command: a `list-tools` subcommand plus a generic + * `[tool] [toolArgs...]` argument pair that forwards any tool call to the running Storybook's MCP + * server. `passThroughOptions` hands every token after the tool name to the tool untouched, which + * requires positional options on the program. + */ +export function registerAiMcpPassthrough(program: Command, aiCommand: Command): void { + program.enablePositionalOptions(); + + aiCommand + .command('list-tools') + .description('List the MCP tools exposed by the running Storybook') + .option('--cwd ', CWD_DESCRIPTION) + .action(async (options: { cwd?: string }) => { + printResult(await runAiListTools({ cwd: options.cwd })); + }); + + aiCommand + .argument('[tool]', 'Name of an MCP tool exposed by the running Storybook') + .argument( + '[toolArgs...]', + 'Tool arguments as `--key value` flags; values are JSON-parsed when possible' + ) + .option('--cwd ', CWD_DESCRIPTION) + .option( + '--json ', + 'Raw JSON object with the tool arguments (escape hatch for complex values)' + ) + .passThroughOptions() + .action( + async ( + tool: string | undefined, + toolArgs: string[], + options: { cwd?: string; json?: string } + ) => { + if (!tool) { + aiCommand.outputHelp(); + return; + } + printResult(await runAiTool(tool, toolArgs, { cwd: options.cwd, json: options.json })); + } + ); +} + +function printResult({ output, exitCode }: AiToolRunResult): void { + process.stdout.write(`${output}\n`); + if (exitCode !== 0) { + process.exitCode = exitCode; + } +} diff --git a/code/lib/cli-storybook/src/ai/mcp/registry.test.ts b/code/lib/cli-storybook/src/ai/mcp/registry.test.ts new file mode 100644 index 000000000000..0e0515ceba52 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/registry.test.ts @@ -0,0 +1,143 @@ +import { readFile, readdir, rm } from 'node:fs/promises'; + +import { vol } from 'memfs'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { readRegistry } from './registry.ts'; + +// Spy-only mock: keep the real `node:fs/promises` module shape, then redirect the calls used by +// the registry reader to `memfs` so disk state stays scoped to `vol`. +vi.mock('node:fs/promises', { spy: true }); + +const REGISTRY_DIR = '/registry'; + +beforeEach(async () => { + const memfs = await vi.importActual('memfs'); + + vi.mocked(readdir).mockImplementation( + memfs.fs.promises.readdir as unknown as typeof import('node:fs/promises').readdir + ); + vi.mocked(readFile).mockImplementation( + memfs.fs.promises.readFile as unknown as typeof import('node:fs/promises').readFile + ); + vi.mocked(rm).mockImplementation( + memfs.fs.promises.rm as unknown as typeof import('node:fs/promises').rm + ); + + // Deterministic PID liveness: in these tests only the current process counts as alive. + vi.spyOn(process, 'kill').mockImplementation((pid) => { + if (pid !== process.pid) { + const error = new Error('ESRCH') as NodeJS.ErrnoException; + error.code = 'ESRCH'; + throw error; + } + return true; + }); +}); + +afterEach(() => { + vol.reset(); + vi.restoreAllMocks(); +}); + +const aliveRecord = { + schemaVersion: 1, + instanceId: 'alive-uuid', + pid: process.pid, + cwd: '/projects/alive', + url: 'http://localhost:6006', + port: 6006, + storybookVersion: '10.5.0', + startedAt: '2026-05-18T12:00:00.000Z', + updatedAt: '2026-05-18T12:00:03.000Z', + mcp: { status: 'ready', endpoint: '/mcp' }, +}; + +describe('readRegistry', () => { + it('returns [] when the registry dir does not exist', async () => { + vol.fromNestedJSON({ '/elsewhere': {} }); + await expect(readRegistry('/registry-does-not-exist')).resolves.toEqual([]); + }); + + it('returns [] when the registry dir is empty', async () => { + vol.fromNestedJSON({ [REGISTRY_DIR]: {} }); + await expect(readRegistry(REGISTRY_DIR)).resolves.toEqual([]); + }); + + it('parses valid records and skips dead PIDs, bad schemas, malformed JSON and non-JSON files', async () => { + const dead = { ...aliveRecord, instanceId: 'dead-uuid', pid: 2147483646 }; + const unknownStatus = { + ...aliveRecord, + instanceId: 'bad-uuid', + mcp: { status: 'unrecognised', endpoint: '/mcp' }, + }; + + vol.fromNestedJSON({ + [REGISTRY_DIR]: { + 'alive.json': JSON.stringify(aliveRecord), + 'dead.json': JSON.stringify(dead), + 'bad-status.json': JSON.stringify(unknownStatus), + 'malformed.json': '{ not json', + 'wrong-shape.json': JSON.stringify({ foo: 'bar' }), + 'ignored.txt': 'should be ignored', + }, + }); + + await expect(readRegistry(REGISTRY_DIR)).resolves.toEqual([aliveRecord]); + }); + + it('removes the registry file of a dead PID', async () => { + const dead = { ...aliveRecord, instanceId: 'dead-uuid', pid: 2147483646 }; + vol.fromNestedJSON({ [REGISTRY_DIR]: { 'dead.json': JSON.stringify(dead) } }); + + await expect(readRegistry(REGISTRY_DIR)).resolves.toEqual([]); + expect(vol.toJSON()[`${REGISTRY_DIR}/dead.json`]).toBeUndefined(); + }); + + it('filters records with non-positive PIDs (process-group sentinels)', async () => { + vol.fromNestedJSON({ + [REGISTRY_DIR]: { + 'zero.json': JSON.stringify({ ...aliveRecord, instanceId: 'zero', pid: 0 }), + 'negative.json': JSON.stringify({ ...aliveRecord, instanceId: 'neg', pid: -1 }), + }, + }); + + await expect(readRegistry(REGISTRY_DIR)).resolves.toEqual([]); + }); + + it('treats EPERM on the liveness signal as alive (foreign-user process)', async () => { + vi.mocked(process.kill).mockImplementation(() => { + const error = new Error('EPERM') as NodeJS.ErrnoException; + error.code = 'EPERM'; + throw error; + }); + vol.fromNestedJSON({ [REGISTRY_DIR]: { 'alive.json': JSON.stringify(aliveRecord) } }); + + await expect(readRegistry(REGISTRY_DIR)).resolves.toEqual([aliveRecord]); + }); + + it('accepts records without the optional version and timestamp fields', async () => { + const minimal = { + schemaVersion: 1, + instanceId: 'minimal', + pid: process.pid, + cwd: '/projects/minimal', + url: 'http://localhost:6007', + port: 6007, + mcp: { status: 'starting' }, + }; + vol.fromNestedJSON({ [REGISTRY_DIR]: { 'minimal.json': JSON.stringify(minimal) } }); + + await expect(readRegistry(REGISTRY_DIR)).resolves.toEqual([minimal]); + }); + + it('rejects out-of-range ports', async () => { + vol.fromNestedJSON({ + [REGISTRY_DIR]: { + 'bad-port.json': JSON.stringify({ ...aliveRecord, port: 65536 }), + }, + }); + + await expect(readRegistry(REGISTRY_DIR)).resolves.toEqual([]); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/registry.ts b/code/lib/cli-storybook/src/ai/mcp/registry.ts new file mode 100644 index 000000000000..6310ef882bd7 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/registry.ts @@ -0,0 +1,110 @@ +import * as fs from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +import type { McpStatus, StorybookInstanceRecord } from './types.ts'; + +export const DEFAULT_REGISTRY_DIR = join(homedir(), '.storybook', 'instances'); + +/** + * Errno codes for which we degrade to "no instance" rather than throwing. The command is meant to + * fail-soft for environmental issues; a noisy stack trace would be worse UX than the + * missing-instance repair instructions. + */ +const SOFT_REGISTRY_ERRORS = new Set(['ENOENT', 'EACCES', 'EPERM', 'ENOTDIR']); + +const MCP_STATUSES: McpStatus[] = ['not-installed', 'starting', 'ready', 'error']; + +function isOptionalString(value: unknown): value is string | undefined { + return value === undefined || typeof value === 'string'; +} + +function isIntegerInRange(value: unknown, min: number, max: number): value is number { + return typeof value === 'number' && Number.isInteger(value) && value >= min && value <= max; +} + +/** Validate a parsed registry file against the record shape; null for anything malformed. */ +export function parseInstanceRecord(value: unknown): StorybookInstanceRecord | null { + if (typeof value !== 'object' || value === null) { + return null; + } + const record = value as Record; + const mcp = record.mcp as Record | null | undefined; + + const isValid = + record.schemaVersion === 1 && + typeof record.instanceId === 'string' && + isIntegerInRange(record.pid, 1, Number.MAX_SAFE_INTEGER) && + typeof record.cwd === 'string' && + typeof record.url === 'string' && + isIntegerInRange(record.port, 1, 65535) && + isOptionalString(record.storybookVersion) && + isOptionalString(record.startedAt) && + isOptionalString(record.updatedAt) && + typeof mcp === 'object' && + mcp !== null && + MCP_STATUSES.includes(mcp.status as McpStatus) && + isOptionalString(mcp.endpoint); + + return isValid ? (value as StorybookInstanceRecord) : null; +} + +/** + * Read all Storybook instance records from `registryDir`. + * + * Each file is expected to be a single JSON object matching {@link StorybookInstanceRecord}. + * Records whose PID is no longer alive are filtered out (and their files removed). Malformed files + * are skipped silently — the command should degrade to "no instance" rather than fail loudly. + */ +export async function readRegistry( + registryDir: string = DEFAULT_REGISTRY_DIR +): Promise { + let entries: string[]; + try { + entries = await fs.readdir(registryDir); + } catch (error) { + if (SOFT_REGISTRY_ERRORS.has((error as NodeJS.ErrnoException).code ?? '')) { + return []; + } + throw error; + } + + const records = await Promise.all( + entries + .filter((name) => name.endsWith('.json')) + .map(async (name) => { + try { + const raw = await fs.readFile(join(registryDir, name), 'utf-8'); + const record = parseInstanceRecord(JSON.parse(raw)); + if (!record) { + return null; + } + if (!isProcessAlive(record.pid)) { + await fs.rm(join(registryDir, name), { force: true }).catch(() => {}); + return null; + } + return record; + } catch { + return null; + } + }) + ); + + return records.filter((r): r is StorybookInstanceRecord => r !== null); +} + +/** + * Liveness check by sending signal 0. `EPERM` means the PID exists but we lack permission to + * signal it (foreign user), which still counts as alive. + */ +function isProcessAlive(pid: number): boolean { + if (!Number.isInteger(pid) || pid <= 0) { + return false; + } + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'EPERM'; + } +} diff --git a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts new file mode 100644 index 000000000000..d87f82c52af4 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it } from 'vitest'; + +import { resolveInstance } from './resolve-instance.ts'; +import type { McpStatus, StorybookInstanceRecord } from './types.ts'; + +let nextInstance = 0; + +function record( + cwd: string, + status: McpStatus = 'ready', + overrides: Partial = {} +): StorybookInstanceRecord { + nextInstance += 1; + return { + schemaVersion: 1, + instanceId: `inst-${nextInstance}`, + pid: 1000 + nextInstance, + cwd, + url: `http://localhost:${6000 + nextInstance}`, + port: 6000 + nextInstance, + mcp: { + status, + endpoint: + status === 'ready' || status === 'error' + ? `http://localhost:${6000 + nextInstance}/mcp` + : undefined, + }, + ...overrides, + }; +} + +describe('resolveInstance', () => { + it('returns no-instance with empty candidates when registry is empty', () => { + const result = resolveInstance([], '/Users/x/projects/foo'); + expect(result).toEqual({ kind: 'intercept', reason: 'no-instance', records: [], matches: [] }); + }); + + it('returns no-instance with candidates when no record cwd matches', () => { + const a = record('/Users/x/projects/foo'); + const b = record('/Users/x/projects/bar'); + const result = resolveInstance([a, b], '/Users/x/projects/baz'); + expect(result.kind).toBe('intercept'); + if (result.kind === 'intercept') { + expect(result.reason).toBe('no-instance'); + expect(result.records).toEqual([a, b]); + } + }); + + it('matches a record by exact normalized cwd', () => { + const r = record('/Users/x/projects/foo'); + const result = resolveInstance([r], '/Users/x/projects/foo'); + expect(result).toEqual({ kind: 'instance', record: r, matches: [r] }); + }); + + it('normalizes trailing slashes and dot segments before matching', () => { + const r = record('/Users/x/projects/foo'); + const result = resolveInstance([r], '/Users/x/projects/foo/./'); + expect(result).toEqual({ kind: 'instance', record: r, matches: [r] }); + }); + + it('does NOT match a child path of a record cwd (exact only)', () => { + const r = record('/Users/x/projects/foo'); + const result = resolveInstance([r], '/Users/x/projects/foo/src/Button.tsx'); + expect(result.kind).toBe('intercept'); + if (result.kind === 'intercept') { + expect(result.reason).toBe('no-instance'); + } + }); + + it('does NOT match a sibling string prefix', () => { + const r = record('/Users/x/projects/foo'); + const result = resolveInstance([r], '/Users/x/projects/foobar'); + expect(result.kind).toBe('intercept'); + if (result.kind === 'intercept') { + expect(result.reason).toBe('no-instance'); + } + }); + + it('returns the lowest-pid ready instance plus all matches when 2+ records share the same exact cwd', () => { + const a = record('/Users/x/projects/foo', 'ready', { pid: 200 }); + const b = record('/Users/x/projects/foo', 'ready', { pid: 100 }); + const result = resolveInstance([a, b], '/Users/x/projects/foo'); + expect(result.kind).toBe('instance'); + if (result.kind === 'instance') { + expect(result.record).toBe(b); + expect(result.matches).toEqual([b, a]); + } + }); + + it('prefers a ready record over non-ready ones when multiple records share the cwd', () => { + const starting = record('/Users/x/projects/foo', 'starting', { pid: 100 }); + const ready = record('/Users/x/projects/foo', 'ready', { pid: 200 }); + const result = resolveInstance([starting, ready], '/Users/x/projects/foo'); + expect(result.kind).toBe('instance'); + if (result.kind === 'instance') { + expect(result.record).toBe(ready); + expect(result.matches).toEqual([starting, ready]); + } + }); + + it('falls back to dispatching the lowest-pid status when no record at the cwd is ready', () => { + const a = record('/Users/x/projects/foo', 'starting', { pid: 200 }); + const b = record('/Users/x/projects/foo', 'error', { pid: 100 }); + const result = resolveInstance([a, b], '/Users/x/projects/foo'); + expect(result).toEqual({ kind: 'intercept', reason: 'mcp-error', matches: [b, a] }); + }); + + it('dispatches mcp.status=starting as mcp-starting intercept', () => { + const r = record('/p', 'starting'); + const result = resolveInstance([r], '/p'); + expect(result).toEqual({ kind: 'intercept', reason: 'mcp-starting', matches: [r] }); + }); + + it('dispatches mcp.status=not-installed as addon-missing intercept', () => { + const r = record('/p', 'not-installed'); + const result = resolveInstance([r], '/p'); + expect(result).toEqual({ kind: 'intercept', reason: 'addon-missing', matches: [r] }); + }); + + it('dispatches mcp.status=error as mcp-error intercept', () => { + const r = record('/p', 'error'); + const result = resolveInstance([r], '/p'); + expect(result).toEqual({ kind: 'intercept', reason: 'mcp-error', matches: [r] }); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts new file mode 100644 index 000000000000..a29471511cbd --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts @@ -0,0 +1,83 @@ +import { resolve } from 'node:path'; + +import type { InterceptReason, StorybookInstanceRecord } from './types.ts'; + +export type ResolveResult = + | { + kind: 'instance'; + record: StorybookInstanceRecord; + matches: StorybookInstanceRecord[]; + } + | { + kind: 'intercept'; + reason: InterceptReason; + records?: StorybookInstanceRecord[]; + matches: StorybookInstanceRecord[]; + }; + +/** + * Pick the Storybook instance whose cwd exactly matches `targetCwd` after normalisation. Per + * milestone 2 of storybookjs/storybook#34826: matching is exact-normalized, with no longest-prefix + * or fallback behaviour. + * + * If at least one record matches, dispatch based on the selected instance's `mcp.status`: + * + * - `ready` → forward the call + * - `starting` → mcp-starting intercept + * - `not-installed` → addon-missing intercept + * - `error` → mcp-error intercept + * + * Zero matches → no-instance intercept (callers may surface running cwds). 2+ matches at the same + * cwd → pick a deterministic instance (lowest pid among `ready` records, else lowest pid overall). + * All matches are returned (sorted by pid) as `matches` so callers can warn the agent without + * blocking the call. + */ +export function resolveInstance( + records: StorybookInstanceRecord[], + targetCwd: string +): ResolveResult { + const normalisedTarget = resolve(targetCwd); + const matches = records.filter((r) => resolve(r.cwd) === normalisedTarget); + + if (matches.length === 0) { + return { + kind: 'intercept', + reason: 'no-instance', + records, + matches: [], + }; + } + + const sortedMatches = [...matches].sort((a, b) => a.pid - b.pid); + const selected = sortedMatches.find((r) => r.mcp.status === 'ready') ?? sortedMatches[0]; + + switch (selected.mcp.status) { + case 'ready': + return { + kind: 'instance', + record: selected, + matches: sortedMatches, + }; + + case 'starting': + return { + kind: 'intercept', + reason: 'mcp-starting', + matches: sortedMatches, + }; + + case 'not-installed': + return { + kind: 'intercept', + reason: 'addon-missing', + matches: sortedMatches, + }; + + case 'error': + return { + kind: 'intercept', + reason: 'mcp-error', + matches: sortedMatches, + }; + } +} diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts new file mode 100644 index 000000000000..848b8971ec23 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -0,0 +1,238 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; +import { readRegistry } from './registry.ts'; +import { runAiListTools, runAiTool } from './run-tool.ts'; +import type { StorybookInstanceRecord } from './types.ts'; +import { checkStorybookVersion } from './version-check.ts'; + +vi.mock('./registry.ts', async (importOriginal) => ({ + ...(await importOriginal()), + readRegistry: vi.fn(), +})); + +// Preserve the McpJsonRpcError class identity so `instanceof` checks keep working. +vi.mock('./client.ts', async (importOriginal) => ({ + ...(await importOriginal()), + callMcpTool: vi.fn(), + listMcpTools: vi.fn(), +})); + +vi.mock('./version-check.ts', async (importOriginal) => ({ + ...(await importOriginal()), + checkStorybookVersion: vi.fn(), +})); + +const record: StorybookInstanceRecord = { + schemaVersion: 1, + instanceId: 'inst-1', + pid: 1, + cwd: '/projects/foo', + url: 'http://localhost:6006', + port: 6006, + mcp: { status: 'ready', endpoint: '/mcp' }, +}; + +beforeEach(() => { + vi.mocked(readRegistry).mockReset().mockResolvedValue([record]); + vi.mocked(checkStorybookVersion).mockReset().mockReturnValue({ status: 'ok' }); + vi.mocked(callMcpTool) + .mockReset() + .mockResolvedValue({ content: [{ type: 'text', text: 'upstream result' }] }); + vi.mocked(listMcpTools) + .mockReset() + .mockResolvedValue([{ name: 'list-all-documentation', description: 'List docs' }]); +}); + +describe('runAiTool', () => { + it('forwards the call to the matching instance and prints the markdown result', async () => { + const result = await runAiTool('list-all-documentation', ['--withStoryIds', 'true'], { + cwd: '/projects/foo', + }); + + expect(callMcpTool).toHaveBeenCalledWith( + record, + { name: 'list-all-documentation', arguments: { withStoryIds: true } }, + undefined + ); + expect(result).toEqual({ exitCode: 0, output: 'upstream result' }); + }); + + it('defaults the cwd to process.cwd()', async () => { + vi.mocked(readRegistry).mockResolvedValue([{ ...record, cwd: process.cwd() }]); + const result = await runAiTool('list-all-documentation', []); + expect(result.exitCode).toBe(0); + }); + + it('merges --json arguments with --key overrides', async () => { + await runAiTool('get-documentation', ['--id', 'override'], { + cwd: '/projects/foo', + json: '{"id":"base","verbose":true}', + }); + + expect(callMcpTool).toHaveBeenCalledWith( + record, + { name: 'get-documentation', arguments: { id: 'override', verbose: true } }, + undefined + ); + }); + + it('returns the arg-parsing error without contacting the registry', async () => { + const result = await runAiTool('get-documentation', ['positional'], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Unexpected argument'); + expect(readRegistry).not.toHaveBeenCalled(); + }); + + it('prints the no-instance repair markdown and exits non-zero when nothing runs at the cwd', async () => { + const result = await runAiTool('get-documentation', [], { cwd: '/projects/other' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('No Storybook is running at this cwd'); + expect(result.output).toContain('/projects/foo'); + }); + + it('prints the storybook-not-installed repair markdown when nothing runs and storybook is unresolvable', async () => { + vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'not-installed' }); + vi.mocked(readRegistry).mockResolvedValue([]); + const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('storybook-init'); + }); + + it('still forwards to a running instance when the version check reports not-installed (monorepo false negative)', async () => { + vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'not-installed' }); + const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); + expect(result).toEqual({ exitCode: 0, output: 'upstream result' }); + }); + + it('prints the storybook-too-old repair markdown before reading the registry', async () => { + vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'too-old', version: '9.0.5' }); + const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('`9.0.5`'); + expect(readRegistry).not.toHaveBeenCalled(); + }); + + it.each([ + ['starting', 'still starting up'], + ['not-installed', '`@storybook/addon-mcp` addon is missing'], + ['error', 'Inspect the Storybook terminal output'], + ] as const)('prints the repair markdown for mcp.status=%s', async (status, expected) => { + vi.mocked(readRegistry).mockResolvedValue([{ ...record, mcp: { status } }]); + const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain(expected); + }); + + it('exits non-zero when the tool result is an error', async () => { + vi.mocked(callMcpTool).mockResolvedValue({ + content: [{ type: 'text', text: 'tool failed' }], + isError: true, + }); + const result = await runAiTool('run-story-tests', [], { cwd: '/projects/foo' }); + expect(result).toEqual({ exitCode: 1, output: 'tool failed' }); + }); + + it('renders non-text content items as JSON blocks', async () => { + vi.mocked(callMcpTool).mockResolvedValue({ + content: [ + { type: 'text', text: 'intro' }, + { type: 'resource_link', uri: 'http://x' }, + ], + }); + const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); + expect(result.output).toContain('intro'); + expect(result.output).toContain('```json'); + expect(result.output).toContain('"resource_link"'); + }); + + it('lists the available tools when the call fails because the tool is unknown', async () => { + vi.mocked(callMcpTool).mockRejectedValue(new McpJsonRpcError(-32601, 'unknown tool')); + const result = await runAiTool('no-such-tool', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Unknown tool `no-such-tool`'); + expect(result.output).toContain('- `list-all-documentation`'); + }); + + it('prints the original JSON-RPC error when the tool exists', async () => { + vi.mocked(callMcpTool).mockRejectedValue(new McpJsonRpcError(-32602, 'invalid arguments')); + const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Storybook MCP error -32602: invalid arguments'); + }); + + it('prints the original JSON-RPC error when the tool list cannot be fetched', async () => { + vi.mocked(callMcpTool).mockRejectedValue(new McpJsonRpcError(-32601, 'unknown tool')); + vi.mocked(listMcpTools).mockRejectedValue(new Error('boom')); + const result = await runAiTool('no-such-tool', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Storybook MCP error -32601: unknown tool'); + }); + + it('surfaces a friendly error when the MCP server is unreachable', async () => { + vi.mocked(callMcpTool).mockRejectedValue(new Error('connection refused')); + const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Failed to reach Storybook MCP at /mcp'); + expect(result.output).toContain('connection refused'); + }); + + it('prepends a warning when multiple instances run at the same cwd', async () => { + const sibling = { ...record, instanceId: 'inst-2', pid: 2, url: 'http://localhost:6007' }; + vi.mocked(readRegistry).mockResolvedValue([record, sibling]); + const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(0); + expect(result.output).toContain('Multiple Storybook instances'); + expect(result.output).toContain('pid `1`'); + expect(result.output).toContain('pid `2`'); + expect(result.output).toContain('(used)'); + expect(result.output).toContain('upstream result'); + }); +}); + +describe('runAiListTools', () => { + it('prints the tool list with descriptions and arguments', async () => { + vi.mocked(listMcpTools).mockResolvedValue([ + { + name: 'get-documentation', + description: 'Get docs for a component.', + inputSchema: { + properties: { id: { type: 'string', description: 'Documentation id' } }, + required: ['id'], + }, + }, + { name: 'list-all-documentation' }, + ]); + + const result = await runAiListTools({ cwd: '/projects/foo' }); + expect(result.exitCode).toBe(0); + expect(result.output).toContain( + '# MCP tools exposed by the Storybook at http://localhost:6006' + ); + expect(result.output).toContain('## get-documentation'); + expect(result.output).toContain('Get docs for a component.'); + expect(result.output).toContain('- `--id` (string, required): Documentation id'); + expect(result.output).toContain('## list-all-documentation'); + }); + + it('reports an empty tool list', async () => { + vi.mocked(listMcpTools).mockResolvedValue([]); + const result = await runAiListTools({ cwd: '/projects/foo' }); + expect(result.exitCode).toBe(0); + expect(result.output).toContain('exposes no MCP tools'); + }); + + it('prints repair markdown and exits non-zero on intercepts', async () => { + vi.mocked(readRegistry).mockResolvedValue([]); + const result = await runAiListTools({ cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Storybook is not running at this cwd'); + }); + + it('surfaces a friendly error when the MCP server is unreachable', async () => { + vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); + const result = await runAiListTools({ cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Failed to reach Storybook MCP at /mcp'); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts new file mode 100644 index 000000000000..421d82f4a77c --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -0,0 +1,222 @@ +import { resolve } from 'node:path'; + +import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; +import { getInterceptMarkdown } from './intercepts.ts'; +import { readRegistry } from './registry.ts'; +import { resolveInstance } from './resolve-instance.ts'; +import { parseToolArgs } from './tool-args.ts'; +import type { McpToolDescriptor, StorybookInstanceRecord, ToolCallResult } from './types.ts'; +import { checkStorybookVersion } from './version-check.ts'; + +export type AiToolRunResult = { exitCode: 0 | 1; output: string }; + +/** Injectable dependencies for tests. */ +export type AiToolRunDeps = { + registryDir?: string; + fetchImpl?: typeof fetch; +}; + +export type AiToolOptions = { + /** Project directory of the target Storybook; defaults to `process.cwd()`. */ + cwd?: string; + /** Raw JSON object with tool arguments (escape hatch for complex values). */ + json?: string; +}; + +/** + * Run a single MCP tool against the Storybook running at the target cwd and return its result as + * markdown. Intercept conditions (no running instance, addon missing, version too old, ...) return + * the same repair-instruction markdown as `@storybook/mcp-proxy`, with exit code 1. + */ +export async function runAiTool( + toolName: string, + toolArgTokens: string[], + options: AiToolOptions = {}, + deps: AiToolRunDeps = {} +): Promise { + const parsed = parseToolArgs(toolArgTokens, { cwd: options.cwd, json: options.json }); + if (!parsed.ok) { + return { exitCode: 1, output: parsed.error }; + } + + const resolution = await resolveReadyInstance(parsed.cwd, deps); + if (resolution.kind === 'error') { + return { exitCode: 1, output: resolution.output }; + } + const { record, matches } = resolution; + + try { + const result = await callMcpTool( + record, + { name: toolName, arguments: parsed.args }, + deps.fetchImpl + ); + const siblings = matches.filter((r) => r !== record); + const sections = [ + ...(siblings.length > 0 ? [formatMultiInstanceWarning(record, siblings)] : []), + formatToolResult(result), + ]; + return { exitCode: result.isError ? 1 : 0, output: sections.join('\n\n') }; + } catch (error) { + if (error instanceof McpJsonRpcError) { + const unknownTool = await describeUnknownTool(record, toolName, deps.fetchImpl); + return { exitCode: 1, output: unknownTool ?? error.message }; + } + return { + exitCode: 1, + output: `Failed to reach Storybook MCP at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ + error instanceof Error ? error.message : String(error) + }`, + }; + } +} + +/** List the MCP tools exposed by the Storybook running at the target cwd. */ +export async function runAiListTools( + options: AiToolOptions = {}, + deps: AiToolRunDeps = {} +): Promise { + const resolution = await resolveReadyInstance(options.cwd, deps); + if (resolution.kind === 'error') { + return { exitCode: 1, output: resolution.output }; + } + const { record } = resolution; + + try { + const tools = await listMcpTools(record, deps.fetchImpl); + return { exitCode: 0, output: formatToolList(tools, record) }; + } catch (error) { + return { + exitCode: 1, + output: `Failed to reach Storybook MCP at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ + error instanceof Error ? error.message : String(error) + }`, + }; + } +} + +type InstanceResolution = + | { kind: 'ok'; record: StorybookInstanceRecord; matches: StorybookInstanceRecord[] } + | { kind: 'error'; output: string }; + +/** + * Resolve the running Storybook instance for `cwdInput`, mirroring the mcp-proxy dispatch order: + * version check first (fail fast on too-old), then registry lookup and cwd matching. + */ +async function resolveReadyInstance( + cwdInput: string | undefined, + deps: AiToolRunDeps +): Promise { + const cwd = resolve(cwdInput ?? process.cwd()); + + const versionStatus = checkStorybookVersion(cwd); + if (versionStatus.status === 'too-old') { + return { + kind: 'error', + output: getInterceptMarkdown('storybook-too-old', { version: versionStatus.version }), + }; + } + + const records = await readRegistry(deps.registryDir); + const resolution = resolveInstance(records, cwd); + + if (resolution.kind === 'intercept') { + if ( + resolution.reason === 'no-instance' && + versionStatus.status === 'not-installed' && + (resolution.records?.length ?? 0) === 0 + ) { + return { kind: 'error', output: getInterceptMarkdown('storybook-not-installed') }; + } + return { + kind: 'error', + output: getInterceptMarkdown(resolution.reason, { records: resolution.records }), + }; + } + + return { kind: 'ok', record: resolution.record, matches: resolution.matches }; +} + +/** + * Build the "unknown tool" error listing the available tools, or null when the tool does exist + * (the JSON-RPC error had another cause) or the tool list cannot be fetched. + */ +async function describeUnknownTool( + record: StorybookInstanceRecord, + toolName: string, + fetchImpl?: typeof fetch +): Promise { + let tools: McpToolDescriptor[]; + try { + tools = await listMcpTools(record, fetchImpl); + } catch { + return null; + } + if (tools.some((tool) => tool.name === toolName)) { + return null; + } + return `Unknown tool \`${toolName}\`. The Storybook at ${record.url} exposes: + +${tools.map((tool) => `- \`${tool.name}\``).join('\n')} + +Run \`storybook ai list-tools\` for descriptions and arguments.`; +} + +/** Render a tools/call result as markdown: text content verbatim, other content as JSON blocks. */ +function formatToolResult(result: ToolCallResult): string { + return (result.content ?? []) + .map((item) => + item.type === 'text' && typeof item.text === 'string' + ? item.text + : `\`\`\`json\n${JSON.stringify(item, null, 2)}\n\`\`\`` + ) + .join('\n\n'); +} + +function formatToolList(tools: McpToolDescriptor[], record: StorybookInstanceRecord): string { + if (tools.length === 0) { + return `The Storybook at ${record.url} exposes no MCP tools.`; + } + + const sections = tools.map((tool) => { + const lines = [`## ${tool.name}`]; + if (tool.description) { + lines.push('', tool.description.trim()); + } + const properties = Object.entries(tool.inputSchema?.properties ?? {}); + if (properties.length > 0) { + const required = new Set(tool.inputSchema?.required ?? []); + lines.push( + '', + 'Arguments:', + ...properties.map(([name, schema]) => { + const meta = [schema.type, required.has(name) ? 'required' : undefined] + .filter(Boolean) + .join(', '); + const description = schema.description ? `: ${schema.description}` : ''; + return `- \`--${name}\`${meta ? ` (${meta})` : ''}${description}`; + }) + ); + } + return lines.join('\n'); + }); + + return [`# MCP tools exposed by the Storybook at ${record.url}`, ...sections].join('\n\n'); +} + +function formatMultiInstanceWarning( + chosen: StorybookInstanceRecord, + siblings: StorybookInstanceRecord[] +): string { + const all = [chosen, ...siblings].sort((a, b) => a.pid - b.pid); + const lines = all.map((r) => { + const marker = r === chosen ? ' (used)' : ''; + return `> - pid \`${r.pid}\` at ${r.url} (mcp: \`${r.mcp.status}\`)${marker}`; + }); + return `> Warning: Multiple Storybook instances are running at this cwd. This call was sent to pid \`${chosen.pid}\`. +> +> Instances at \`${chosen.cwd}\`: +${lines.join('\n')} +> +> If results look unexpected, ask the user whether they want to stop the other instance(s).`; +} diff --git a/code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts b/code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts new file mode 100644 index 000000000000..02c85012c051 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest'; + +import { parseToolArgs } from './tool-args.ts'; + +function args(tokens: string[], defaults?: { cwd?: string; json?: string }) { + const result = parseToolArgs(tokens, defaults); + if (!result.ok) { + throw new Error(`expected ok, got error: ${result.error}`); + } + return result; +} + +function error(tokens: string[], defaults?: { cwd?: string; json?: string }) { + const result = parseToolArgs(tokens, defaults); + if (result.ok) { + throw new Error(`expected error, got ok: ${JSON.stringify(result.args)}`); + } + return result.error; +} + +describe('parseToolArgs', () => { + it('returns empty args for no tokens', () => { + expect(args([])).toEqual({ ok: true, cwd: undefined, args: {} }); + }); + + it('maps `--key value` pairs to tool arguments', () => { + expect(args(['--id', 'button-docs']).args).toEqual({ id: 'button-docs' }); + }); + + it('supports `--key=value`', () => { + expect(args(['--id=button-docs']).args).toEqual({ id: 'button-docs' }); + }); + + describe('JSON-parse coercion', () => { + it('coerces booleans, numbers and null', () => { + expect(args(['--a', 'true', '--b', '42', '--c', 'null']).args).toEqual({ + a: true, + b: 42, + c: null, + }); + }); + + it('coerces JSON arrays and objects', () => { + expect(args(['--ids', '["a","b"]', '--filter', '{"tag":"x"}']).args).toEqual({ + ids: ['a', 'b'], + filter: { tag: 'x' }, + }); + }); + + it('falls back to the raw string when the value is not valid JSON', () => { + expect(args(['--id', 'button-docs', '--path', 'src/Button.tsx']).args).toEqual({ + id: 'button-docs', + path: 'src/Button.tsx', + }); + }); + + it('unquotes explicitly quoted JSON strings', () => { + expect(args(['--id', '"true"']).args).toEqual({ id: 'true' }); + }); + + it('accepts negative numbers as values', () => { + expect(args(['--offset', '-1']).args).toEqual({ offset: -1 }); + }); + }); + + it('treats a bare `--flag` as true', () => { + expect(args(['--withStoryIds']).args).toEqual({ withStoryIds: true }); + expect(args(['--withStoryIds', '--id', 'x']).args).toEqual({ withStoryIds: true, id: 'x' }); + }); + + it('lets the last occurrence of a repeated key win', () => { + expect(args(['--id', 'a', '--id', 'b']).args).toEqual({ id: 'b' }); + }); + + describe('--cwd', () => { + it('consumes --cwd instead of forwarding it', () => { + expect(args(['--cwd', '/projects/foo', '--id', 'x'])).toEqual({ + ok: true, + cwd: '/projects/foo', + args: { id: 'x' }, + }); + }); + + it('uses the commander-parsed default when not repeated in the tokens', () => { + expect(args(['--id', 'x'], { cwd: '/projects/foo' }).cwd).toBe('/projects/foo'); + }); + + it('prefers a --cwd token over the commander-parsed default', () => { + expect(args(['--cwd', '/b'], { cwd: '/a' }).cwd).toBe('/b'); + }); + + it('errors when --cwd has no value', () => { + expect(error(['--cwd'])).toContain('`--cwd` requires a value'); + }); + }); + + describe('--json escape hatch', () => { + it('uses the JSON object as the tool arguments', () => { + expect(args(['--json', '{"id":"x","n":1}']).args).toEqual({ id: 'x', n: 1 }); + }); + + it('lets explicit --key flags override --json entries', () => { + expect(args(['--json', '{"id":"x","n":1}', '--id', 'y']).args).toEqual({ id: 'y', n: 1 }); + }); + + it('accepts --json parsed by commander before the tool name', () => { + expect(args(['--id', 'y'], { json: '{"id":"x","n":1}' }).args).toEqual({ id: 'y', n: 1 }); + }); + + it('errors on invalid JSON', () => { + expect(error(['--json', '{nope'])).toContain('`--json` must be valid JSON'); + }); + + it('errors when the JSON is not an object', () => { + expect(error(['--json', '[1,2]'])).toContain('must be a JSON object'); + expect(error(['--json', '"text"'])).toContain('must be a JSON object'); + expect(error(['--json', 'null'])).toContain('must be a JSON object'); + }); + + it('errors when --json has no value', () => { + expect(error(['--json'])).toContain('`--json` requires a value'); + }); + }); + + it('errors on positional tokens', () => { + expect(error(['positional'])).toContain('Unexpected argument `positional`'); + }); + + it('errors on a bare `--` separator', () => { + expect(error(['--'])).toContain('Unexpected argument `--`'); + }); + + it('errors on `--=value`', () => { + expect(error(['--=x'])).toContain('Invalid flag'); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/tool-args.ts b/code/lib/cli-storybook/src/ai/mcp/tool-args.ts new file mode 100644 index 000000000000..e095ccac8aa5 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/tool-args.ts @@ -0,0 +1,101 @@ +export type ParsedToolArgs = + | { ok: true; cwd: string | undefined; args: Record } + | { ok: false; error: string }; + +/** + * Parse the pass-through tokens after `storybook ai ` into MCP tool arguments. + * + * - `--key value` and `--key=value` become tool arguments; values are coerced by attempting + * `JSON.parse`, falling back to the raw string. + * - A bare `--key` (no value) becomes `true`. + * - `--json ''` is an escape hatch providing the raw argument object; explicit `--key` + * flags override its entries. + * - `--cwd ` is consumed by the CLI itself and never forwarded to the tool. + * + * `defaults` carries `--cwd`/`--json` values that commander already parsed before the tool name; + * the same flags appearing after the tool name take precedence. + */ +export function parseToolArgs( + tokens: string[], + defaults: { cwd?: string; json?: string } = {} +): ParsedToolArgs { + let cwd = defaults.cwd; + let rawJson = defaults.json; + const flagArgs: Record = {}; + + let i = 0; + while (i < tokens.length) { + const token = tokens[i]; + i += 1; + + if (!token.startsWith('--') || token === '--') { + return { + ok: false, + error: `Unexpected argument \`${token}\`. Tool arguments must be passed as \`--key value\` flags (or via \`--json ''\`).`, + }; + } + + let key = token.slice(2); + let value: string | undefined; + const equalsIndex = key.indexOf('='); + if (equalsIndex !== -1) { + value = key.slice(equalsIndex + 1); + key = key.slice(0, equalsIndex); + } else if (i < tokens.length && !tokens[i].startsWith('--')) { + value = tokens[i]; + i += 1; + } + + if (key === '') { + return { ok: false, error: `Invalid flag \`${token}\`.` }; + } + + if (key === 'cwd') { + if (value === undefined) { + return { ok: false, error: '`--cwd` requires a value.' }; + } + cwd = value; + continue; + } + + if (key === 'json') { + if (value === undefined) { + return { ok: false, error: '`--json` requires a value.' }; + } + rawJson = value; + continue; + } + + flagArgs[key] = value === undefined ? true : coerceValue(value); + } + + let jsonArgs: Record = {}; + if (rawJson !== undefined) { + let parsed: unknown; + try { + parsed = JSON.parse(rawJson); + } catch (error) { + return { + ok: false, + error: `\`--json\` must be valid JSON: ${error instanceof Error ? error.message : String(error)}`, + }; + } + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + return { + ok: false, + error: '`--json` must be a JSON object, e.g. \'{"id": "button-docs"}\'.', + }; + } + jsonArgs = parsed as Record; + } + + return { ok: true, cwd, args: { ...jsonArgs, ...flagArgs } }; +} + +function coerceValue(raw: string): unknown { + try { + return JSON.parse(raw); + } catch { + return raw; + } +} diff --git a/code/lib/cli-storybook/src/ai/mcp/types.ts b/code/lib/cli-storybook/src/ai/mcp/types.ts new file mode 100644 index 000000000000..75ef64c189e1 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/types.ts @@ -0,0 +1,61 @@ +/** + * Reader-side types for the `storybook ai ` MCP passthrough, copied from + * `@storybook/mcp-proxy` (storybookjs/mcp) per storybookjs/storybook#35124. The + * writer side lives in `code/core/src/core-server/utils/runtime-instance-registry.ts`; + * this reader is intentionally more lenient (extra statuses, optional fields) so it + * also accepts records written by other Storybook versions and wrappers. + */ + +export type McpStatus = 'not-installed' | 'starting' | 'ready' | 'error'; + +/** + * A single Storybook runtime record written under the registry dir (default + * `~/.storybook/instances`). One file per running `storybook dev` instance. + * Spec: storybookjs/storybook#34826. + */ +export interface StorybookInstanceRecord { + schemaVersion: 1; + instanceId: string; + pid: number; + cwd: string; + url: string; + port: number; + storybookVersion?: string; + startedAt?: string; + updatedAt?: string; + mcp: { + status: McpStatus; + endpoint?: string; + }; +} + +export type InterceptReason = + | 'no-instance' + | 'storybook-not-installed' + | 'addon-missing' + | 'mcp-starting' + | 'mcp-error' + | 'storybook-too-old'; + +export interface ToolResultContentItem { + type: string; + text?: string; + [key: string]: unknown; +} + +/** Result of an MCP `tools/call` request, as returned by `@storybook/addon-mcp`. */ +export interface ToolCallResult { + content?: ToolResultContentItem[]; + isError?: boolean; + _meta?: Record; +} + +/** A tool descriptor from an MCP `tools/list` response. */ +export interface McpToolDescriptor { + name: string; + description?: string; + inputSchema?: { + properties?: Record; + required?: string[]; + }; +} diff --git a/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts b/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts new file mode 100644 index 000000000000..2ca13173bd85 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts @@ -0,0 +1,65 @@ +import { createRequire } from 'node:module'; + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { STORYBOOK_MIN_VERSION, checkStorybookVersion } from './version-check.ts'; + +vi.mock('node:module', () => ({ + createRequire: vi.fn(), +})); + +function mockStorybookVersion(version: string | null) { + vi.mocked(createRequire).mockImplementation( + () => + ((id: string) => { + if (id === 'storybook/package.json') { + if (version === null) { + throw new Error('not found'); + } + return { version }; + } + throw new Error(`unexpected require: ${id}`); + }) as unknown as ReturnType + ); +} + +beforeEach(() => { + vi.mocked(createRequire).mockReset(); +}); + +describe('checkStorybookVersion', () => { + it('returns ok for the minimum version', () => { + mockStorybookVersion(STORYBOOK_MIN_VERSION); + expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); + }); + + it('returns too-old with the detected version for older Storybooks', () => { + mockStorybookVersion('9.1.16'); + expect(checkStorybookVersion('/a')).toEqual({ status: 'too-old', version: '9.1.16' }); + }); + + it('accepts a prerelease of the minimum (alpha/beta/rc)', () => { + mockStorybookVersion('10.5.0-alpha.1'); + expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); + }); + + it('treats a prerelease of an earlier version as too-old', () => { + mockStorybookVersion('10.4.0-rc.1'); + expect(checkStorybookVersion('/a')).toEqual({ status: 'too-old', version: '10.4.0-rc.1' }); + }); + + it('returns ok for a stable release above the minimum', () => { + mockStorybookVersion('11.0.0'); + expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); + }); + + it('returns not-installed when storybook is unresolvable', () => { + mockStorybookVersion(null); + expect(checkStorybookVersion('/a')).toEqual({ status: 'not-installed' }); + }); + + it('treats 0.0.0-... versions as ok (canary)', () => { + mockStorybookVersion('0.0.0-canary.1234'); + expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/version-check.ts b/code/lib/cli-storybook/src/ai/mcp/version-check.ts new file mode 100644 index 000000000000..b1c755e32a32 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/version-check.ts @@ -0,0 +1,44 @@ +import { createRequire } from 'node:module'; +import { join } from 'node:path'; + +import { lt } from 'semver'; + +/** Minimum Storybook version whose `storybook dev` registers itself for MCP discovery. */ +export const STORYBOOK_MIN_VERSION = '10.5.0'; + +/** + * Comparison floor for the minimum version. The `-0` prerelease suffix is the lowest possible + * 10.5.0 build, so every 10.5.0 prerelease (alpha/beta/rc) is accepted alongside the stable + * release, while anything below 10.5.0 is rejected. + */ +const STORYBOOK_MIN_VERSION_FLOOR = '10.5.0-0'; + +export type StorybookVersionStatus = + | { status: 'ok' } + | { status: 'too-old'; version: string } + | { status: 'not-installed' }; + +function readStorybookVersion(cwd: string): string | null { + try { + const requireFromCwd = createRequire(join(cwd, 'package.json')); + const { version } = requireFromCwd('storybook/package.json') as { version: string }; + return version; + } catch { + return null; + } +} + +export function checkStorybookVersion(cwd: string): StorybookVersionStatus { + const version = readStorybookVersion(cwd); + // Storybook canary releases are published as `0.0.0-*`; semver considers these < any stable + // version, but we still want to treat them as supported. + if (version?.startsWith('0.0.0-')) { + return { status: 'ok' }; + } + if (version === null) { + return { status: 'not-installed' }; + } + return lt(version, STORYBOOK_MIN_VERSION_FLOOR) + ? { status: 'too-old', version } + : { status: 'ok' }; +} diff --git a/code/lib/cli-storybook/src/bin/run.ts b/code/lib/cli-storybook/src/bin/run.ts index b784848b26aa..5f0ac08b0de6 100644 --- a/code/lib/cli-storybook/src/bin/run.ts +++ b/code/lib/cli-storybook/src/bin/run.ts @@ -24,6 +24,7 @@ import { link } from '../link.ts'; import { migrate } from '../migrate.ts'; import { sandbox } from '../sandbox.ts'; import { aiSetup } from '../ai/index.ts'; +import { isAiCliFeatureEnabled, registerAiMcpPassthrough } from '../ai/mcp/register.ts'; import { type UpgradeOptions, upgrade } from '../upgrade.ts'; addToGlobalContext('cliVersion', versions.storybook); @@ -329,6 +330,12 @@ aiCommand.action(() => { aiCommand.outputHelp(); }); +// Experimental `storybook ai ` passthrough to the local Storybook MCP server +// (storybookjs/storybook#35124). Overrides the help-only action above when enabled. +if (isAiCliFeatureEnabled()) { + registerAiMcpPassthrough(program, aiCommand); +} + program.on('command:*', ([invalidCmd]) => { let errorMessage = ` Invalid command: ${picocolors.bold(invalidCmd)}.\n See --help for a list of available commands.`; const availableCommands = program.commands.map((cmd) => cmd.name()); diff --git a/yarn.lock b/yarn.lock index 6635182c9967..cd693e924df8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8507,6 +8507,7 @@ __metadata: globby: "npm:^14.1.0" jscodeshift: "npm:^0.15.1" leven: "npm:^4.0.0" + memfs: "npm:^4.11.1" p-limit: "npm:^7.2.0" picocolors: "npm:^1.1.0" semver: "npm:^7.7.3" From cce2a3438ed5eb00cea85344b79e86080517e63f Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 12:54:34 +0200 Subject: [PATCH 02/13] fix(cli): list available tools when addon-mcp reports unknown tool as 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. --- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 30 +++++++++++++------ code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 7 +++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index 848b8971ec23..9419c72c2e70 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -124,15 +124,6 @@ describe('runAiTool', () => { expect(result.output).toContain(expected); }); - it('exits non-zero when the tool result is an error', async () => { - vi.mocked(callMcpTool).mockResolvedValue({ - content: [{ type: 'text', text: 'tool failed' }], - isError: true, - }); - const result = await runAiTool('run-story-tests', [], { cwd: '/projects/foo' }); - expect(result).toEqual({ exitCode: 1, output: 'tool failed' }); - }); - it('renders non-text content items as JSON blocks', async () => { vi.mocked(callMcpTool).mockResolvedValue({ content: [ @@ -154,6 +145,27 @@ describe('runAiTool', () => { expect(result.output).toContain('- `list-all-documentation`'); }); + it('lists the available tools when the server reports the unknown tool as an error result', async () => { + // addon-mcp (tmcp) reports unknown tools as an isError result, not a JSON-RPC error. + vi.mocked(callMcpTool).mockResolvedValue({ + content: [{ type: 'text', text: 'Tool no-such-tool not found' }], + isError: true, + }); + const result = await runAiTool('no-such-tool', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Unknown tool `no-such-tool`'); + expect(result.output).toContain('- `list-all-documentation`'); + }); + + it('keeps the original error result when the failing tool does exist', async () => { + vi.mocked(callMcpTool).mockResolvedValue({ + content: [{ type: 'text', text: 'tests failed' }], + isError: true, + }); + const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); + expect(result).toEqual({ exitCode: 1, output: 'tests failed' }); + }); + it('prints the original JSON-RPC error when the tool exists', async () => { vi.mocked(callMcpTool).mockRejectedValue(new McpJsonRpcError(-32602, 'invalid arguments')); const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index 421d82f4a77c..b624d03016bd 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -51,6 +51,13 @@ export async function runAiTool( { name: toolName, arguments: parsed.args }, deps.fetchImpl ); + if (result.isError) { + // addon-mcp reports unknown tools as an error *result* rather than a JSON-RPC error. + const unknownTool = await describeUnknownTool(record, toolName, deps.fetchImpl); + if (unknownTool) { + return { exitCode: 1, output: unknownTool }; + } + } const siblings = matches.filter((r) => r !== record); const sections = [ ...(siblings.length > 0 ? [formatMultiInstanceWarning(record, siblings)] : []), From a8b028c2a76a3ede3de96765d3ed83881c048d93 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 13:01:00 +0200 Subject: [PATCH 03/13] refactor(cli): address code-quality review findings for ai MCP passthrough - 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 --- code/lib/cli-storybook/src/ai/mcp/client.ts | 6 +++--- code/lib/cli-storybook/src/ai/mcp/intercepts.ts | 10 ++++++---- code/lib/cli-storybook/src/ai/mcp/registry.ts | 6 ++++++ .../cli-storybook/src/ai/mcp/resolve-instance.ts | 5 +++++ .../cli-storybook/src/ai/mcp/run-tool.test.ts | 16 ++++++++++++++++ code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 6 +++++- code/lib/cli-storybook/src/ai/mcp/types.ts | 4 ++++ 7 files changed, 45 insertions(+), 8 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/client.ts b/code/lib/cli-storybook/src/ai/mcp/client.ts index 22fa4459b227..5bf9720a2809 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.ts @@ -39,8 +39,8 @@ export async function listMcpTools( ): Promise { const result = (await sendJsonRpcRequest(record, 'tools/list', {}, fetchImpl)) as { tools?: McpToolDescriptor[]; - }; - return result.tools ?? []; + } | null; + return result?.tools ?? []; } /** @@ -92,7 +92,7 @@ async function sendJsonRpcRequest( if (payload.error) { throw new McpJsonRpcError(payload.error.code, payload.error.message); } - if (!payload.result) { + if (payload.result === undefined) { throw new Error('Storybook MCP returned no result'); } return payload.result; diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts index 21dd3152d215..bf34385d623c 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts @@ -2,10 +2,8 @@ import type { InterceptReason, StorybookInstanceRecord } from './types.ts'; import { STORYBOOK_MIN_VERSION } from './version-check.ts'; /** - * Repair-instruction markdown, copied from `@storybook/mcp-proxy` so agents get the same guidance - * from the CLI as from the MCP proxy. Wording is adapted from "retry the tool call" to "retry the - * command", and the proxy's version-cache instructions are dropped (the CLI is one-shot and - * re-detects the version on every run). + * Repair-instruction markdown for agents, mirroring `@storybook/mcp-proxy` (storybookjs/mcp) so + * the CLI and the proxy give the same guidance — keep the two in sync when updating either. */ const NO_INSTANCE_EMPTY = `Storybook is not running at this cwd. Start \`storybook dev\` from the project's cwd and retry the command.`; @@ -72,5 +70,9 @@ export function getInterceptMarkdown( return MCP_ERROR; case 'storybook-too-old': return buildStorybookTooOld(version ?? 'unknown'); + default: { + const unhandled: never = reason; + throw new Error(`Unhandled intercept reason: ${unhandled as string}`); + } } } diff --git a/code/lib/cli-storybook/src/ai/mcp/registry.ts b/code/lib/cli-storybook/src/ai/mcp/registry.ts index 6310ef882bd7..e09b22275bba 100644 --- a/code/lib/cli-storybook/src/ai/mcp/registry.ts +++ b/code/lib/cli-storybook/src/ai/mcp/registry.ts @@ -4,6 +4,12 @@ import { join } from 'node:path'; import type { McpStatus, StorybookInstanceRecord } from './types.ts'; +/** + * Must stay in sync with `getDefaultRuntimeInstanceRegistryDir` in + * `code/core/src/core-server/utils/runtime-instance-registry.ts` (the writer side). Duplicated + * here so this reader does not pull the core-server module graph into the CLI's unit tests; the + * path is specified in storybookjs/storybook#34826. + */ export const DEFAULT_REGISTRY_DIR = join(homedir(), '.storybook', 'instances'); /** diff --git a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts index a29471511cbd..8c71ac67a9b5 100644 --- a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts +++ b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts @@ -79,5 +79,10 @@ export function resolveInstance( reason: 'mcp-error', matches: sortedMatches, }; + + default: { + const unhandled: never = selected.mcp.status; + throw new Error(`Unhandled MCP status: ${unhandled as string}`); + } } } diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index 9419c72c2e70..cf7ca7aab0c1 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -124,6 +124,22 @@ describe('runAiTool', () => { expect(result.output).toContain(expected); }); + it('prints a placeholder when the tool returns no content', async () => { + vi.mocked(callMcpTool).mockResolvedValue({ content: [] }); + const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); + expect(result).toEqual({ exitCode: 0, output: '(the tool returned no content)' }); + }); + + it('surfaces a clean error when a ready record is missing its endpoint', async () => { + vi.mocked(callMcpTool).mockRejectedValue( + new Error('Storybook MCP record for /projects/foo is missing mcp.endpoint') + ); + vi.mocked(readRegistry).mockResolvedValue([{ ...record, mcp: { status: 'ready' } }]); + const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Failed to reach Storybook MCP at (no endpoint)'); + }); + it('renders non-text content items as JSON blocks', async () => { vi.mocked(callMcpTool).mockResolvedValue({ content: [ diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index b624d03016bd..58d89880e764 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -171,7 +171,11 @@ Run \`storybook ai list-tools\` for descriptions and arguments.`; /** Render a tools/call result as markdown: text content verbatim, other content as JSON blocks. */ function formatToolResult(result: ToolCallResult): string { - return (result.content ?? []) + const content = result.content ?? []; + if (content.length === 0) { + return '(the tool returned no content)'; + } + return content .map((item) => item.type === 'text' && typeof item.text === 'string' ? item.text diff --git a/code/lib/cli-storybook/src/ai/mcp/types.ts b/code/lib/cli-storybook/src/ai/mcp/types.ts index 75ef64c189e1..737d4ac630e0 100644 --- a/code/lib/cli-storybook/src/ai/mcp/types.ts +++ b/code/lib/cli-storybook/src/ai/mcp/types.ts @@ -6,6 +6,10 @@ * also accepts records written by other Storybook versions and wrappers. */ +/** + * The in-repo writer only emits `not-installed` and `ready`; `starting` and `error` are written by + * external wrappers (e.g. the storybookjs/mcp launch script) and must keep being dispatched here. + */ export type McpStatus = 'not-installed' | 'starting' | 'ready' | 'error'; /** From 3c2dbda94a82317b6b46bdbfc81b210a85ada813 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 13:42:54 +0200 Subject: [PATCH 04/13] feat(cli): align ai passthrough with current mcp-proxy and make --help 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 --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 --- .../cli-storybook/src/ai/mcp/client.test.ts | 1 + code/lib/cli-storybook/src/ai/mcp/client.ts | 18 +- .../src/ai/mcp/intercepts.test.ts | 10 ++ .../cli-storybook/src/ai/mcp/intercepts.ts | 11 +- .../cli-storybook/src/ai/mcp/register.test.ts | 111 +++++++++--- code/lib/cli-storybook/src/ai/mcp/register.ts | 64 +++++-- .../src/ai/mcp/resolve-instance.test.ts | 107 +++++++++++- .../src/ai/mcp/resolve-instance.ts | 56 +++++- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 132 ++++++++++++-- code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 163 +++++++++++++----- .../src/ai/mcp/tool-args.test.ts | 42 ++++- .../lib/cli-storybook/src/ai/mcp/tool-args.ts | 45 ++++- code/lib/cli-storybook/src/ai/mcp/types.ts | 1 + .../src/ai/mcp/version-check.test.ts | 126 +++++++++----- .../cli-storybook/src/ai/mcp/version-check.ts | 41 +++-- 15 files changed, 749 insertions(+), 179 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/client.test.ts b/code/lib/cli-storybook/src/ai/mcp/client.test.ts index fe0f2224f7f5..b277fd4adafb 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.test.ts @@ -49,6 +49,7 @@ describe('callMcpTool', () => { const headers = init.headers as Record; expect(headers.Accept).toBe('application/json, text/event-stream'); expect(headers['X-Storybook-MCP-Proxy']).toBe('true'); + expect(init.signal).toBeInstanceOf(AbortSignal); const body = JSON.parse(init.body as string); expect(body).toMatchObject({ jsonrpc: '2.0', diff --git a/code/lib/cli-storybook/src/ai/mcp/client.ts b/code/lib/cli-storybook/src/ai/mcp/client.ts index 5bf9720a2809..812a9c9164c8 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.ts @@ -7,6 +7,12 @@ import type { McpToolDescriptor, StorybookInstanceRecord, ToolCallResult } from const STORYBOOK_MCP_PROXY_HEADER = 'X-Storybook-MCP-Proxy'; const STORYBOOK_MCP_PROXY_HEADER_VALUE = 'true'; +/** + * Upper bound on a single request so a hung server cannot stall the CLI forever. Generous because + * `run-story-tests` on a full suite legitimately runs for minutes. + */ +const REQUEST_TIMEOUT_MS = 10 * 60 * 1000; + export type ToolCallParams = { name: string; arguments?: Record; @@ -46,9 +52,14 @@ export async function listMcpTools( /** * Send a single JSON-RPC request to the instance's MCP endpoint over HTTP. * - * The downstream is `@storybook/addon-mcp` at `record.mcp.endpoint`. tmcp's HttpTransport - * hardcodes `text/event-stream` for any request with an id, so we accept both content-types and - * parse the SSE envelope when needed. Every call is independent; no session bookkeeping needed. + * This is deliberately NOT a full MCP client: there is no `initialize` handshake, session, or + * protocol-version negotiation. The downstream is always `@storybook/addon-mcp`, whose tmcp + * HttpTransport is stateless and serves `tools/*` per-request — the same local shortcut + * `@storybook/mcp-proxy` takes in its proxy-client. If the CLI ever needs to talk to arbitrary + * MCP servers, replace this with a real client instead of extending it. + * + * tmcp hardcodes `text/event-stream` for any request with an id, so we accept both content-types + * and parse the SSE envelope when needed. */ async function sendJsonRpcRequest( record: StorybookInstanceRecord, @@ -76,6 +87,7 @@ async function sendJsonRpcRequest( method, params, }), + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), }); if (!response.ok) { diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts index 4063737b6045..0a308d30eb4b 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts @@ -29,6 +29,16 @@ describe('getInterceptMarkdown', () => { expect(markdown).toContain('--cwd'); }); + it('port-mismatch lists the ports running at the cwd', () => { + const markdown = getInterceptMarkdown('port-mismatch', { + port: 9999, + records: [record('/projects/foo', 'http://localhost:6006')], + }); + expect(markdown).toContain('not on port `9999`'); + expect(markdown).toContain('- port `6006`'); + expect(markdown).toContain('omit `--port`'); + }); + it('storybook-too-old names the detected and minimum versions and the upgrade skill', () => { const markdown = getInterceptMarkdown('storybook-too-old', { version: '9.0.5' }); expect(markdown).toContain('`9.0.5`'); diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts index bf34385d623c..d2342202f17b 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts @@ -13,6 +13,12 @@ const buildNoInstanceWithCandidates = (records: StorybookInstanceRecord[]) => Running Storybooks: ${records.map((r) => `- \`${r.cwd}\` (${r.url})`).join('\n')}`; +const buildPortMismatch = (port: number | undefined, records: StorybookInstanceRecord[]) => + `Storybook is running at this cwd, but not on port \`${port ?? 'unknown'}\`. Retry with one of the running ports below, or omit \`--port\` to route by cwd alone. + +Running Storybooks at this cwd: +${records.map((r) => `- port \`${r.port}\` (${r.url}, mcp: \`${r.mcp.status}\`)`).join('\n')}`; + const buildStorybookTooOld = (version: string) => `The Storybook installed at this cwd is version \`${version}\`, but this command requires \`${STORYBOOK_MIN_VERSION}\` or newer. @@ -48,18 +54,21 @@ const MCP_ERROR = `Storybook is running but its MCP server reported an error. In export type InterceptExtras = { records?: StorybookInstanceRecord[]; version?: string; + port?: number; }; export function getInterceptMarkdown( reason: InterceptReason, extras: InterceptExtras = {} ): string { - const { records, version } = extras; + const { records, version, port } = extras; switch (reason) { case 'no-instance': return records && records.length > 0 ? buildNoInstanceWithCandidates(records) : NO_INSTANCE_EMPTY; + case 'port-mismatch': + return buildPortMismatch(port, records ?? []); case 'storybook-not-installed': return STORYBOOK_NOT_INSTALLED; case 'addon-missing': diff --git a/code/lib/cli-storybook/src/ai/mcp/register.test.ts b/code/lib/cli-storybook/src/ai/mcp/register.test.ts index 916272eedd10..c3b7dff1fc45 100644 --- a/code/lib/cli-storybook/src/ai/mcp/register.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/register.test.ts @@ -1,15 +1,20 @@ +import { writeFile } from 'node:fs/promises'; + import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { Command } from 'commander'; import { isAiCliFeatureEnabled, registerAiMcpPassthrough } from './register.ts'; -import { runAiListTools, runAiTool } from './run-tool.ts'; +import { buildToolCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; vi.mock('./run-tool.ts', () => ({ runAiTool: vi.fn(), - runAiListTools: vi.fn(), + runAiToolHelp: vi.fn(), + buildToolCommandsHelp: vi.fn(), })); +vi.mock('node:fs/promises', { spy: true }); + describe('isAiCliFeatureEnabled', () => { it.each([ ['1', true], @@ -50,27 +55,33 @@ function parse(program: Command, argv: string[]) { return program.parseAsync(['node', 'storybook', ...argv]); } +function stdoutText(): string { + return vi + .mocked(process.stdout.write) + .mock.calls.map(([chunk]) => String(chunk)) + .join(''); +} + beforeEach(() => { vi.mocked(runAiTool).mockResolvedValue({ exitCode: 0, output: 'ok' }); - vi.mocked(runAiListTools).mockResolvedValue({ exitCode: 0, output: 'tools' }); + vi.mocked(runAiToolHelp).mockResolvedValue({ exitCode: 0, output: 'tool help' }); + vi.mocked(buildToolCommandsHelp).mockResolvedValue('Tool commands (from the Storybook):'); + vi.mocked(writeFile).mockResolvedValue(undefined); vi.spyOn(process.stdout, 'write').mockImplementation(() => true); }); afterEach(() => { vi.restoreAllMocks(); vi.mocked(runAiTool).mockReset(); - vi.mocked(runAiListTools).mockReset(); + vi.mocked(runAiToolHelp).mockReset(); + vi.mocked(buildToolCommandsHelp).mockReset(); process.exitCode = undefined; }); describe('without the feature flag (no registration)', () => { - it('does not expose list-tools', async () => { - const { program, aiCommand } = buildProgram({ withPassthrough: false }); + it('keeps `setup` as the only subcommand', () => { + const { aiCommand } = buildProgram({ withPassthrough: false }); expect(aiCommand.commands.map((c) => c.name())).toEqual(['setup']); - await expect(parse(program, ['ai', 'list-tools'])).rejects.toMatchObject({ - code: 'commander.excessArguments', - }); - expect(runAiListTools).not.toHaveBeenCalled(); }); it('rejects tool names like today (excess arguments)', async () => { @@ -85,6 +96,7 @@ describe('without the feature flag (no registration)', () => { const { program, helpAction } = buildProgram({ withPassthrough: false }); await parse(program, ['ai']); expect(helpAction).toHaveBeenCalled(); + expect(buildToolCommandsHelp).not.toHaveBeenCalled(); }); }); @@ -94,14 +106,28 @@ describe('with the feature flag (passthrough registered)', () => { await parse(program, ['ai', 'get-documentation', '--id', 'button-docs']); expect(runAiTool).toHaveBeenCalledWith('get-documentation', ['--id', 'button-docs'], { cwd: undefined, + port: undefined, json: undefined, }); }); - it('parses --cwd and --json before the tool name as CLI options', async () => { + it('parses --cwd, --port and --json before the tool name as CLI options', async () => { const { program } = buildProgram({ withPassthrough: true }); - await parse(program, ['ai', '--cwd', '/x', '--json', '{"a":1}', 'get-documentation']); - expect(runAiTool).toHaveBeenCalledWith('get-documentation', [], { cwd: '/x', json: '{"a":1}' }); + await parse(program, [ + 'ai', + '--cwd', + '/x', + '--port', + '6006', + '--json', + '{"a":1}', + 'get-documentation', + ]); + expect(runAiTool).toHaveBeenCalledWith('get-documentation', [], { + cwd: '/x', + port: '6006', + json: '{"a":1}', + }); }); it('passes tokens after the tool name through verbatim, even option-like ones', async () => { @@ -109,10 +135,19 @@ describe('with the feature flag (passthrough registered)', () => { await parse(program, ['ai', 'tool-x', '--cwd', '/y', '--output', 'z']); expect(runAiTool).toHaveBeenCalledWith('tool-x', ['--cwd', '/y', '--output', 'z'], { cwd: undefined, + port: undefined, json: undefined, }); }); + it('writes the result to the file given via --output instead of stdout', async () => { + const { program } = buildProgram({ withPassthrough: true }); + vi.mocked(runAiTool).mockResolvedValue({ exitCode: 0, output: 'markdown result' }); + await parse(program, ['ai', '-o', '/out/result.md', 'tool-x']); + expect(writeFile).toHaveBeenCalledWith('/out/result.md', 'markdown result\n', 'utf-8'); + expect(process.stdout.write).not.toHaveBeenCalledWith('markdown result\n'); + }); + it('writes the result to stdout', async () => { const { program } = buildProgram({ withPassthrough: true }); vi.mocked(runAiTool).mockResolvedValue({ exitCode: 0, output: 'markdown result' }); @@ -129,13 +164,6 @@ describe('with the feature flag (passthrough registered)', () => { expect(process.exitCode).toBe(1); }); - it('dispatches `ai list-tools` to runAiListTools with the cwd option', async () => { - const { program } = buildProgram({ withPassthrough: true }); - await parse(program, ['ai', 'list-tools', '--cwd', '/x']); - expect(runAiListTools).toHaveBeenCalledWith({ cwd: '/x' }); - expect(runAiTool).not.toHaveBeenCalled(); - }); - it('still dispatches `ai setup` to the setup subcommand', async () => { const { program, setupAction } = buildProgram({ withPassthrough: true }); await parse(program, ['ai', 'setup']); @@ -143,11 +171,44 @@ describe('with the feature flag (passthrough registered)', () => { expect(runAiTool).not.toHaveBeenCalled(); }); - it('shows help when `ai` is run without a tool', async () => { - const { program, aiCommand } = buildProgram({ withPassthrough: true }); - const outputHelp = vi.spyOn(aiCommand, 'outputHelp').mockImplementation(() => ''); - await parse(program, ['ai']); - expect(outputHelp).toHaveBeenCalled(); + it.each([[['ai']], [['ai', '--help']], [['ai', '-h']]])( + 'shows commander help plus the tool commands section for %j', + async (argv) => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, argv); + expect(buildToolCommandsHelp).toHaveBeenCalledWith({ cwd: undefined, port: undefined }); + const output = stdoutText(); + expect(output).toContain('Usage:'); + expect(output).toContain('setup'); + expect(output).toContain('Tool commands (from the Storybook):'); + expect(runAiTool).not.toHaveBeenCalled(); + } + ); + + it('passes --cwd and --port through to the tool commands section', async () => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', '--cwd', '/x', '--port', '6006', '--help']); + expect(buildToolCommandsHelp).toHaveBeenCalledWith({ cwd: '/x', port: '6006' }); + }); + + it('shows single-tool help for `ai --help `', async () => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', '--help', 'get-documentation']); + expect(runAiToolHelp).toHaveBeenCalledWith('get-documentation', { + cwd: undefined, + port: undefined, + }); + expect(process.stdout.write).toHaveBeenCalledWith('tool help\n'); expect(runAiTool).not.toHaveBeenCalled(); }); + + it('passes a --help token after the tool name through to runAiTool', async () => { + const { program } = buildProgram({ withPassthrough: true }); + await parse(program, ['ai', 'get-documentation', '--help']); + expect(runAiTool).toHaveBeenCalledWith('get-documentation', ['--help'], { + cwd: undefined, + port: undefined, + json: undefined, + }); + }); }); diff --git a/code/lib/cli-storybook/src/ai/mcp/register.ts b/code/lib/cli-storybook/src/ai/mcp/register.ts index e8713c71bfbc..201e3372aa2c 100644 --- a/code/lib/cli-storybook/src/ai/mcp/register.ts +++ b/code/lib/cli-storybook/src/ai/mcp/register.ts @@ -1,8 +1,17 @@ +import { writeFile } from 'node:fs/promises'; +import { resolve } from 'node:path'; + import { optionalEnvToBoolean } from 'storybook/internal/common'; +import { logger } from 'storybook/internal/node-logger'; import type { Command } from 'commander'; -import { type AiToolRunResult, runAiListTools, runAiTool } from './run-tool.ts'; +import { + type AiToolRunResult, + buildToolCommandsHelp, + runAiTool, + runAiToolHelp, +} from './run-tool.ts'; /** * The `storybook ai ` MCP passthrough is experimental (storybookjs/storybook#35124) and only @@ -14,53 +23,70 @@ export function isAiCliFeatureEnabled(env: NodeJS.ProcessEnv = process.env): boo const CWD_DESCRIPTION = 'Project directory of the target Storybook (defaults to the current working directory)'; +const PORT_DESCRIPTION = + 'Port of the target Storybook, to address one specific instance when several run at the same cwd'; /** - * Register the MCP passthrough on the `ai` command: a `list-tools` subcommand plus a generic - * `[tool] [toolArgs...]` argument pair that forwards any tool call to the running Storybook's MCP - * server. `passThroughOptions` hands every token after the tool name to the tool untouched, which - * requires positional options on the program. + * Register the MCP passthrough on the `ai` command: a generic `[tool] [toolArgs...]` argument pair + * that forwards any tool call to the running Storybook's MCP server. `passThroughOptions` hands + * every token after the tool name to the tool untouched, which requires positional options on the + * program. + * + * Commander's built-in (synchronous) help is replaced with our own `-h, --help` option so the help + * output can include the tool commands fetched from the running Storybook. */ export function registerAiMcpPassthrough(program: Command, aiCommand: Command): void { program.enablePositionalOptions(); aiCommand - .command('list-tools') - .description('List the MCP tools exposed by the running Storybook') - .option('--cwd ', CWD_DESCRIPTION) - .action(async (options: { cwd?: string }) => { - printResult(await runAiListTools({ cwd: options.cwd })); - }); - - aiCommand + .helpOption(false) .argument('[tool]', 'Name of an MCP tool exposed by the running Storybook') .argument( '[toolArgs...]', 'Tool arguments as `--key value` flags; values are JSON-parsed when possible' ) .option('--cwd ', CWD_DESCRIPTION) + .option('--port ', PORT_DESCRIPTION) .option( '--json ', 'Raw JSON object with the tool arguments (escape hatch for complex values)' ) + .option('-h, --help', 'Show help, including the tool commands of the running Storybook') .passThroughOptions() .action( async ( tool: string | undefined, toolArgs: string[], - options: { cwd?: string; json?: string } + options: { cwd?: string; port?: string; json?: string; output?: string; help?: boolean } ) => { - if (!tool) { - aiCommand.outputHelp(); + const target = { cwd: options.cwd, port: options.port }; + if (options.help && tool) { + await printResult(await runAiToolHelp(tool, target), options.output); return; } - printResult(await runAiTool(tool, toolArgs, { cwd: options.cwd, json: options.json })); + if (options.help || !tool) { + const toolSection = await buildToolCommandsHelp(target); + process.stdout.write(`${aiCommand.helpInformation()}\n${toolSection}\n`); + return; + } + const result = await runAiTool(tool, toolArgs, { ...target, json: options.json }); + await printResult(result, options.output); } ); } -function printResult({ output, exitCode }: AiToolRunResult): void { - process.stdout.write(`${output}\n`); +/** Print to stdout, or to the file given via the `ai` command's `-o, --output` option. */ +async function printResult( + { output, exitCode }: AiToolRunResult, + outputPath: string | undefined +): Promise { + if (outputPath) { + const resolvedPath = resolve(outputPath); + await writeFile(resolvedPath, `${output}\n`, 'utf-8'); + logger.log(`Output written to ${resolvedPath}`); + } else { + process.stdout.write(`${output}\n`); + } if (exitCode !== 0) { process.exitCode = exitCode; } diff --git a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts index d87f82c52af4..79d454a937ec 100644 --- a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.test.ts @@ -76,7 +76,7 @@ describe('resolveInstance', () => { } }); - it('returns the lowest-pid ready instance plus all matches when 2+ records share the same exact cwd', () => { + it('tie-breaks on lowest pid when 2+ records share the cwd and none carry a startedAt', () => { const a = record('/Users/x/projects/foo', 'ready', { pid: 200 }); const b = record('/Users/x/projects/foo', 'ready', { pid: 100 }); const result = resolveInstance([a, b], '/Users/x/projects/foo'); @@ -87,6 +87,68 @@ describe('resolveInstance', () => { } }); + it('picks the most recently started ready instance when 2+ records share the cwd', () => { + const older = record('/Users/x/projects/foo', 'ready', { + pid: 100, + startedAt: '2026-06-09T10:00:00.000Z', + }); + const newer = record('/Users/x/projects/foo', 'ready', { + pid: 200, + startedAt: '2026-06-09T11:00:00.000Z', + }); + const result = resolveInstance([older, newer], '/Users/x/projects/foo'); + expect(result.kind).toBe('instance'); + if (result.kind === 'instance') { + expect(result.record).toBe(newer); + expect(result.matches).toEqual([newer, older]); + } + }); + + it('treats a record without startedAt as older than one with a startedAt', () => { + const noStamp = record('/Users/x/projects/foo', 'ready', { pid: 100 }); + const stamped = record('/Users/x/projects/foo', 'ready', { + pid: 200, + startedAt: '2026-06-09T11:00:00.000Z', + }); + const result = resolveInstance([noStamp, stamped], '/Users/x/projects/foo'); + expect(result.kind).toBe('instance'); + if (result.kind === 'instance') { + expect(result.record).toBe(stamped); + } + }); + + it('prefers a ready record over a more recently started non-ready one', () => { + const ready = record('/Users/x/projects/foo', 'ready', { + pid: 100, + startedAt: '2026-06-09T10:00:00.000Z', + }); + const newerStarting = record('/Users/x/projects/foo', 'starting', { + pid: 200, + startedAt: '2026-06-09T11:00:00.000Z', + }); + const result = resolveInstance([ready, newerStarting], '/Users/x/projects/foo'); + expect(result.kind).toBe('instance'); + if (result.kind === 'instance') { + expect(result.record).toBe(ready); + } + }); + + it('dispatches the most recently started instance status when none are ready', () => { + const olderError = record('/Users/x/projects/foo', 'error', { + pid: 100, + startedAt: '2026-06-09T10:00:00.000Z', + }); + const newerStarting = record('/Users/x/projects/foo', 'starting', { + pid: 200, + startedAt: '2026-06-09T11:00:00.000Z', + }); + const result = resolveInstance([olderError, newerStarting], '/Users/x/projects/foo'); + expect(result.kind).toBe('intercept'); + if (result.kind === 'intercept') { + expect(result.reason).toBe('mcp-starting'); + } + }); + it('prefers a ready record over non-ready ones when multiple records share the cwd', () => { const starting = record('/Users/x/projects/foo', 'starting', { pid: 100 }); const ready = record('/Users/x/projects/foo', 'ready', { pid: 200 }); @@ -122,4 +184,47 @@ describe('resolveInstance', () => { const result = resolveInstance([r], '/p'); expect(result).toEqual({ kind: 'intercept', reason: 'mcp-error', matches: [r] }); }); + + it('selects the instance matching BOTH cwd and port when a port is supplied', () => { + const a = record('/Users/x/projects/foo', 'ready', { pid: 100, port: 6006 }); + const b = record('/Users/x/projects/foo', 'ready', { pid: 200, port: 6007 }); + const result = resolveInstance([a, b], '/Users/x/projects/foo', 6007); + expect(result.kind).toBe('instance'); + if (result.kind === 'instance') { + expect(result.record).toBe(b); + expect(result.matches).toEqual([b]); + } + }); + + it('ignores port when it is not supplied (routes by cwd alone)', () => { + const a = record('/Users/x/projects/foo', 'ready', { pid: 100, port: 6006 }); + const b = record('/Users/x/projects/foo', 'ready', { pid: 200, port: 6007 }); + const result = resolveInstance([a, b], '/Users/x/projects/foo'); + expect(result.kind).toBe('instance'); + if (result.kind === 'instance') { + expect(result.record).toBe(a); + expect(result.matches).toEqual([a, b]); + } + }); + + it('returns port-mismatch with the cwd instances as candidates when cwd matches but no instance is on the port', () => { + const a = record('/Users/x/projects/foo', 'ready', { pid: 100, port: 6006 }); + const b = record('/Users/x/projects/foo', 'ready', { pid: 200, port: 6007 }); + const result = resolveInstance([a, b], '/Users/x/projects/foo', 9999); + expect(result.kind).toBe('intercept'); + if (result.kind === 'intercept') { + expect(result.reason).toBe('port-mismatch'); + expect(result.records).toEqual([a, b]); + expect(result.matches).toEqual([]); + } + }); + + it('returns no-instance (not port-mismatch) when the cwd itself does not match', () => { + const a = record('/Users/x/projects/foo', 'ready', { port: 6006 }); + const result = resolveInstance([a], '/Users/x/projects/bar', 6006); + expect(result.kind).toBe('intercept'); + if (result.kind === 'intercept') { + expect(result.reason).toBe('no-instance'); + } + }); }); diff --git a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts index 8c71ac67a9b5..4af476eab3ae 100644 --- a/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts +++ b/code/lib/cli-storybook/src/ai/mcp/resolve-instance.ts @@ -20,6 +20,12 @@ export type ResolveResult = * milestone 2 of storybookjs/storybook#34826: matching is exact-normalized, with no longest-prefix * or fallback behaviour. * + * When `targetPort` is supplied (e.g. an agent that launched Storybook on a known port and wants + * to address that exact instance), it further constrains the cwd matches: an instance must match + * BOTH cwd and port. If the cwd matches but no instance there is on `targetPort`, a + * `port-mismatch` intercept is returned with the cwd's instances as candidates so callers can + * surface the running ports. + * * If at least one record matches, dispatch based on the selected instance's `mcp.status`: * * - `ready` → forward the call @@ -28,18 +34,31 @@ export type ResolveResult = * - `error` → mcp-error intercept * * Zero matches → no-instance intercept (callers may surface running cwds). 2+ matches at the same - * cwd → pick a deterministic instance (lowest pid among `ready` records, else lowest pid overall). - * All matches are returned (sorted by pid) as `matches` so callers can warn the agent without - * blocking the call. + * cwd → pick the most recently started instance (latest `startedAt` among `ready` records, else + * latest overall), on the assumption that the freshest instance is the one the agent just started. + * Records without a `startedAt` tie-break on lowest pid for determinism. All matches are returned + * (most-recent first) as `matches` so callers can warn the agent without blocking the call. */ export function resolveInstance( records: StorybookInstanceRecord[], - targetCwd: string + targetCwd: string, + targetPort?: number ): ResolveResult { const normalisedTarget = resolve(targetCwd); - const matches = records.filter((r) => resolve(r.cwd) === normalisedTarget); + const cwdMatches = records.filter((r) => resolve(r.cwd) === normalisedTarget); + const matches = targetPort == null ? cwdMatches : cwdMatches.filter((r) => r.port === targetPort); if (matches.length === 0) { + // cwd matched, but no instance there is on the requested port: a distinct, + // more actionable failure than "nothing is running here". + if (targetPort != null && cwdMatches.length > 0) { + return { + kind: 'intercept', + reason: 'port-mismatch', + records: cwdMatches, + matches: [], + }; + } return { kind: 'intercept', reason: 'no-instance', @@ -48,7 +67,7 @@ export function resolveInstance( }; } - const sortedMatches = [...matches].sort((a, b) => a.pid - b.pid); + const sortedMatches = [...matches].sort(byMostRecentlyStarted); const selected = sortedMatches.find((r) => r.mcp.status === 'ready') ?? sortedMatches[0]; switch (selected.mcp.status) { @@ -86,3 +105,28 @@ export function resolveInstance( } } } + +/** + * `startedAt` as epoch millis, or `-Infinity` when absent/unparseable so such records sort as the + * oldest (and fall through to the pid tie-break). + */ +function startedAtMs(r: StorybookInstanceRecord): number { + if (!r.startedAt) { + return Number.NEGATIVE_INFINITY; + } + const t = Date.parse(r.startedAt); + return Number.isNaN(t) ? Number.NEGATIVE_INFINITY : t; +} + +/** + * Sort comparator: most recently started first, tie-breaking on lowest pid so ordering stays + * deterministic when timestamps are equal or missing. + */ +function byMostRecentlyStarted(a: StorybookInstanceRecord, b: StorybookInstanceRecord): number { + const ta = startedAtMs(a); + const tb = startedAtMs(b); + if (ta !== tb) { + return tb > ta ? 1 : -1; + } + return a.pid - b.pid; +} diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index cf7ca7aab0c1..a56e3619db82 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; import { readRegistry } from './registry.ts'; -import { runAiListTools, runAiTool } from './run-tool.ts'; +import { buildToolCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; import type { StorybookInstanceRecord } from './types.ts'; import { checkStorybookVersion } from './version-check.ts'; @@ -105,11 +105,56 @@ describe('runAiTool', () => { expect(result).toEqual({ exitCode: 0, output: 'upstream result' }); }); - it('prints the storybook-too-old repair markdown before reading the registry', async () => { + it('prints the storybook-too-old repair markdown when the disk version is too old', async () => { vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'too-old', version: '9.0.5' }); const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); expect(result.output).toContain('`9.0.5`'); + }); + + it('prefers the version reported in the registry record over the disk lookup', async () => { + vi.mocked(readRegistry).mockResolvedValue([{ ...record, storybookVersion: '9.0.5' }]); + const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('`9.0.5`'); + expect(checkStorybookVersion).not.toHaveBeenCalled(); + }); + + it('forwards to a canary instance even when the disk lookup reports too-old', async () => { + vi.mocked(readRegistry).mockResolvedValue([ + { ...record, storybookVersion: '0.0.0-pr-1-sha-abc' }, + ]); + vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'too-old', version: '9.0.5' }); + const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); + expect(result).toEqual({ exitCode: 0, output: 'upstream result' }); + }); + + it('routes to the instance on the requested --port when several share the cwd', async () => { + const onOtherPort = { ...record, instanceId: 'inst-2', pid: 2, port: 6007 }; + vi.mocked(readRegistry).mockResolvedValue([record, onOtherPort]); + const result = await runAiTool('list-all-documentation', ['--port', '6007'], { + cwd: '/projects/foo', + }); + expect(callMcpTool).toHaveBeenCalledWith(onOtherPort, expect.anything(), undefined); + expect(result.exitCode).toBe(0); + }); + + it('prints the port-mismatch repair markdown when no instance at the cwd is on the port', async () => { + const result = await runAiTool('list-all-documentation', [], { + cwd: '/projects/foo', + port: '9999', + }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('not on port `9999`'); + expect(result.output).toContain('- port `6006`'); + }); + + it('rejects an invalid --port without contacting the registry', async () => { + const result = await runAiTool('list-all-documentation', ['--port', 'abc'], { + cwd: '/projects/foo', + }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('`--port` must be a port number'); expect(readRegistry).not.toHaveBeenCalled(); }); @@ -218,8 +263,47 @@ describe('runAiTool', () => { }); }); -describe('runAiListTools', () => { - it('prints the tool list with descriptions and arguments', async () => { +describe('buildToolCommandsHelp', () => { + it('lists each tool with the first line of its description', async () => { + vi.mocked(listMcpTools).mockResolvedValue([ + { + name: 'get-documentation', + description: 'Get docs for a component.\n\nLong details that should not appear.', + }, + { name: 'list-all-documentation' }, + ]); + + const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('Tool commands (from the Storybook at http://localhost:6006):'); + expect(section).toContain('get-documentation'); + expect(section).toContain('Get docs for a component.'); + expect(section).not.toContain('Long details'); + expect(section).toContain("Run 'storybook ai --help'"); + }); + + it('degrades to a note when no Storybook is running (help must not fail)', async () => { + vi.mocked(readRegistry).mockResolvedValue([]); + const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('Tool commands: (unavailable'); + expect(section).toContain('storybook dev'); + }); + + it('degrades to a note when the MCP server is unreachable', async () => { + vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); + const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('Tool commands: (unavailable'); + expect(section).toContain('could not be reached'); + }); + + it('degrades to a note when no tools are exposed', async () => { + vi.mocked(listMcpTools).mockResolvedValue([]); + const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('exposes no MCP tools'); + }); +}); + +describe('runAiToolHelp', () => { + it('prints the description and arguments of a single tool', async () => { vi.mocked(listMcpTools).mockResolvedValue([ { name: 'get-documentation', @@ -229,37 +313,51 @@ describe('runAiListTools', () => { required: ['id'], }, }, - { name: 'list-all-documentation' }, ]); - const result = await runAiListTools({ cwd: '/projects/foo' }); + const result = await runAiToolHelp('get-documentation', { cwd: '/projects/foo' }); expect(result.exitCode).toBe(0); - expect(result.output).toContain( - '# MCP tools exposed by the Storybook at http://localhost:6006' - ); - expect(result.output).toContain('## get-documentation'); + expect(result.output).toContain('Usage: storybook ai get-documentation'); expect(result.output).toContain('Get docs for a component.'); expect(result.output).toContain('- `--id` (string, required): Documentation id'); - expect(result.output).toContain('## list-all-documentation'); }); - it('reports an empty tool list', async () => { - vi.mocked(listMcpTools).mockResolvedValue([]); - const result = await runAiListTools({ cwd: '/projects/foo' }); + it('is reachable through runAiTool via a --help token after the tool name', async () => { + vi.mocked(listMcpTools).mockResolvedValue([ + { name: 'get-documentation', description: 'Get docs.' }, + ]); + const result = await runAiTool('get-documentation', ['--help'], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(0); - expect(result.output).toContain('exposes no MCP tools'); + expect(result.output).toContain('Usage: storybook ai get-documentation'); + expect(callMcpTool).not.toHaveBeenCalled(); + }); + + it('lists the available tools for an unknown tool name', async () => { + const result = await runAiToolHelp('no-such-tool', { cwd: '/projects/foo' }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('Unknown tool `no-such-tool`'); + expect(result.output).toContain('- `list-all-documentation`'); }); it('prints repair markdown and exits non-zero on intercepts', async () => { vi.mocked(readRegistry).mockResolvedValue([]); - const result = await runAiListTools({ cwd: '/projects/foo' }); + const result = await runAiToolHelp('get-documentation', { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); expect(result.output).toContain('Storybook is not running at this cwd'); }); + it('rejects an invalid --port', async () => { + const result = await runAiToolHelp('get-documentation', { + cwd: '/projects/foo', + port: 'abc', + }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('`--port` must be a port number'); + }); + it('surfaces a friendly error when the MCP server is unreachable', async () => { vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); - const result = await runAiListTools({ cwd: '/projects/foo' }); + const result = await runAiToolHelp('get-documentation', { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); expect(result.output).toContain('Failed to reach Storybook MCP at /mcp'); }); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index 58d89880e764..6b37b5cda73a 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -6,7 +6,7 @@ import { readRegistry } from './registry.ts'; import { resolveInstance } from './resolve-instance.ts'; import { parseToolArgs } from './tool-args.ts'; import type { McpToolDescriptor, StorybookInstanceRecord, ToolCallResult } from './types.ts'; -import { checkStorybookVersion } from './version-check.ts'; +import { checkStorybookVersion, classifyStorybookVersion } from './version-check.ts'; export type AiToolRunResult = { exitCode: 0 | 1; output: string }; @@ -19,6 +19,8 @@ export type AiToolRunDeps = { export type AiToolOptions = { /** Project directory of the target Storybook; defaults to `process.cwd()`. */ cwd?: string; + /** Port of the target Storybook, to address one specific instance when several share the cwd. */ + port?: string; /** Raw JSON object with tool arguments (escape hatch for complex values). */ json?: string; }; @@ -34,12 +36,19 @@ export async function runAiTool( options: AiToolOptions = {}, deps: AiToolRunDeps = {} ): Promise { - const parsed = parseToolArgs(toolArgTokens, { cwd: options.cwd, json: options.json }); + const parsed = parseToolArgs(toolArgTokens, { + cwd: options.cwd, + port: options.port, + json: options.json, + }); if (!parsed.ok) { return { exitCode: 1, output: parsed.error }; } + if (parsed.help) { + return runAiToolHelp(toolName, { cwd: parsed.cwd, port: options.port }, deps); + } - const resolution = await resolveReadyInstance(parsed.cwd, deps); + const resolution = await resolveReadyInstance(parsed.cwd, parsed.port, deps); if (resolution.kind === 'error') { return { exitCode: 1, output: resolution.output }; } @@ -78,20 +87,73 @@ export async function runAiTool( } } -/** List the MCP tools exposed by the Storybook running at the target cwd. */ -export async function runAiListTools( +/** + * Build the "Tool commands" help section listing the tools of the running Storybook, appended to + * `storybook ai --help`. Help must never fail, so any error degrades to a short note explaining + * why no tools are listed. + */ +export async function buildToolCommandsHelp( + options: AiToolOptions = {}, + deps: AiToolRunDeps = {} +): Promise { + const unavailable = (note: string) => `Tool commands: (unavailable — ${note})`; + + const parsed = parseToolArgs([], { cwd: options.cwd, port: options.port }); + if (!parsed.ok) { + return unavailable(parsed.error); + } + + const resolution = await resolveReadyInstance(parsed.cwd, parsed.port, deps); + if (resolution.kind === 'error') { + return unavailable( + `no running Storybook with MCP detected at this cwd; start \`storybook dev\` to list its tools` + ); + } + const { record } = resolution; + + let tools: McpToolDescriptor[]; + try { + tools = await listMcpTools(record, deps.fetchImpl); + } catch { + return unavailable(`the Storybook MCP at ${record.url} could not be reached`); + } + if (tools.length === 0) { + return unavailable(`the Storybook at ${record.url} exposes no MCP tools`); + } + + const width = Math.max(...tools.map((tool) => tool.name.length)) + 2; + const lines = tools.map((tool) => { + const summary = tool.description?.trim().split('\n')[0] ?? ''; + return ` ${tool.name.padEnd(width)}${summary}`; + }); + return [ + `Tool commands (from the Storybook at ${record.url}):`, + ...lines, + '', + `Run 'storybook ai --help' for a tool's description and arguments.`, + ].join('\n'); +} + +/** Show the description and arguments of a single MCP tool (`storybook ai --help`). */ +export async function runAiToolHelp( + toolName: string, options: AiToolOptions = {}, deps: AiToolRunDeps = {} ): Promise { - const resolution = await resolveReadyInstance(options.cwd, deps); + const parsed = parseToolArgs([], { cwd: options.cwd, port: options.port }); + if (!parsed.ok) { + return { exitCode: 1, output: parsed.error }; + } + + const resolution = await resolveReadyInstance(parsed.cwd, parsed.port, deps); if (resolution.kind === 'error') { return { exitCode: 1, output: resolution.output }; } const { record } = resolution; + let tools: McpToolDescriptor[]; try { - const tools = await listMcpTools(record, deps.fetchImpl); - return { exitCode: 0, output: formatToolList(tools, record) }; + tools = await listMcpTools(record, deps.fetchImpl); } catch (error) { return { exitCode: 1, @@ -100,6 +162,12 @@ export async function runAiListTools( }`, }; } + + const tool = tools.find((candidate) => candidate.name === toolName); + if (!tool) { + return { exitCode: 1, output: formatUnknownTool(toolName, tools, record) }; + } + return { exitCode: 0, output: formatToolHelp(tool) }; } type InstanceResolution = @@ -108,15 +176,25 @@ type InstanceResolution = /** * Resolve the running Storybook instance for `cwdInput`, mirroring the mcp-proxy dispatch order: - * version check first (fail fast on too-old), then registry lookup and cwd matching. + * registry lookup and cwd/port matching first, then the version check — preferring the version the + * running instance reported in its registry record over resolving the installed version from disk. */ async function resolveReadyInstance( cwdInput: string | undefined, + port: number | undefined, deps: AiToolRunDeps ): Promise { const cwd = resolve(cwdInput ?? process.cwd()); - const versionStatus = checkStorybookVersion(cwd); + const records = await readRegistry(deps.registryDir); + const resolution = resolveInstance(records, cwd, port); + + const matchedRecord = resolution.kind === 'instance' ? resolution.record : resolution.matches[0]; + const versionStatus = + matchedRecord?.storybookVersion !== undefined + ? classifyStorybookVersion(matchedRecord.storybookVersion) + : checkStorybookVersion(cwd); + if (versionStatus.status === 'too-old') { return { kind: 'error', @@ -124,9 +202,6 @@ async function resolveReadyInstance( }; } - const records = await readRegistry(deps.registryDir); - const resolution = resolveInstance(records, cwd); - if (resolution.kind === 'intercept') { if ( resolution.reason === 'no-instance' && @@ -137,7 +212,7 @@ async function resolveReadyInstance( } return { kind: 'error', - output: getInterceptMarkdown(resolution.reason, { records: resolution.records }), + output: getInterceptMarkdown(resolution.reason, { records: resolution.records, port }), }; } @@ -162,11 +237,19 @@ async function describeUnknownTool( if (tools.some((tool) => tool.name === toolName)) { return null; } + return formatUnknownTool(toolName, tools, record); +} + +function formatUnknownTool( + toolName: string, + tools: McpToolDescriptor[], + record: StorybookInstanceRecord +): string { return `Unknown tool \`${toolName}\`. The Storybook at ${record.url} exposes: ${tools.map((tool) => `- \`${tool.name}\``).join('\n')} -Run \`storybook ai list-tools\` for descriptions and arguments.`; +Run \`storybook ai --help\` for all commands, or \`storybook ai --help\` for a tool's arguments.`; } /** Render a tools/call result as markdown: text content verbatim, other content as JSON blocks. */ @@ -184,42 +267,34 @@ function formatToolResult(result: ToolCallResult): string { .join('\n\n'); } -function formatToolList(tools: McpToolDescriptor[], record: StorybookInstanceRecord): string { - if (tools.length === 0) { - return `The Storybook at ${record.url} exposes no MCP tools.`; +function formatToolHelp(tool: McpToolDescriptor): string { + const lines = [`Usage: storybook ai ${tool.name} [--key value ...]`]; + if (tool.description) { + lines.push('', tool.description.trim()); } - - const sections = tools.map((tool) => { - const lines = [`## ${tool.name}`]; - if (tool.description) { - lines.push('', tool.description.trim()); - } - const properties = Object.entries(tool.inputSchema?.properties ?? {}); - if (properties.length > 0) { - const required = new Set(tool.inputSchema?.required ?? []); - lines.push( - '', - 'Arguments:', - ...properties.map(([name, schema]) => { - const meta = [schema.type, required.has(name) ? 'required' : undefined] - .filter(Boolean) - .join(', '); - const description = schema.description ? `: ${schema.description}` : ''; - return `- \`--${name}\`${meta ? ` (${meta})` : ''}${description}`; - }) - ); - } - return lines.join('\n'); - }); - - return [`# MCP tools exposed by the Storybook at ${record.url}`, ...sections].join('\n\n'); + const properties = Object.entries(tool.inputSchema?.properties ?? {}); + if (properties.length > 0) { + const required = new Set(tool.inputSchema?.required ?? []); + lines.push( + '', + 'Arguments:', + ...properties.map(([name, schema]) => { + const meta = [schema.type, required.has(name) ? 'required' : undefined] + .filter(Boolean) + .join(', '); + const description = schema.description ? `: ${schema.description}` : ''; + return `- \`--${name}\`${meta ? ` (${meta})` : ''}${description}`; + }) + ); + } + return lines.join('\n'); } function formatMultiInstanceWarning( chosen: StorybookInstanceRecord, siblings: StorybookInstanceRecord[] ): string { - const all = [chosen, ...siblings].sort((a, b) => a.pid - b.pid); + const all = [chosen, ...siblings]; const lines = all.map((r) => { const marker = r === chosen ? ' (used)' : ''; return `> - pid \`${r.pid}\` at ${r.url} (mcp: \`${r.mcp.status}\`)${marker}`; diff --git a/code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts b/code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts index 02c85012c051..c41d3e138efb 100644 --- a/code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/tool-args.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest'; import { parseToolArgs } from './tool-args.ts'; -function args(tokens: string[], defaults?: { cwd?: string; json?: string }) { +function args(tokens: string[], defaults?: { cwd?: string; port?: string; json?: string }) { const result = parseToolArgs(tokens, defaults); if (!result.ok) { throw new Error(`expected ok, got error: ${result.error}`); @@ -10,7 +10,7 @@ function args(tokens: string[], defaults?: { cwd?: string; json?: string }) { return result; } -function error(tokens: string[], defaults?: { cwd?: string; json?: string }) { +function error(tokens: string[], defaults?: { cwd?: string; port?: string; json?: string }) { const result = parseToolArgs(tokens, defaults); if (result.ok) { throw new Error(`expected error, got ok: ${JSON.stringify(result.args)}`); @@ -20,7 +20,13 @@ function error(tokens: string[], defaults?: { cwd?: string; json?: string }) { describe('parseToolArgs', () => { it('returns empty args for no tokens', () => { - expect(args([])).toEqual({ ok: true, cwd: undefined, args: {} }); + expect(args([])).toEqual({ ok: true, cwd: undefined, port: undefined, help: false, args: {} }); + }); + + it('consumes --help and -h as a help request instead of forwarding them', () => { + expect(args(['--help'])).toMatchObject({ help: true, args: {} }); + expect(args(['-h'])).toMatchObject({ help: true, args: {} }); + expect(args(['--id', 'x', '--help'])).toMatchObject({ help: true, args: { id: 'x' } }); }); it('maps `--key value` pairs to tool arguments', () => { @@ -77,6 +83,8 @@ describe('parseToolArgs', () => { expect(args(['--cwd', '/projects/foo', '--id', 'x'])).toEqual({ ok: true, cwd: '/projects/foo', + port: undefined, + help: false, args: { id: 'x' }, }); }); @@ -94,6 +102,34 @@ describe('parseToolArgs', () => { }); }); + describe('--port', () => { + it('consumes --port as a number instead of forwarding it', () => { + expect(args(['--port', '6006', '--id', 'x'])).toEqual({ + ok: true, + cwd: undefined, + port: 6006, + help: false, + args: { id: 'x' }, + }); + }); + + it('uses the commander-parsed default and lets a token override it', () => { + expect(args(['--id', 'x'], { port: '6006' }).port).toBe(6006); + expect(args(['--port', '6007'], { port: '6006' }).port).toBe(6007); + }); + + it('errors on non-numeric or out-of-range ports', () => { + expect(error(['--port', 'abc'])).toContain('`--port` must be a port number'); + expect(error(['--port', '0'])).toContain('`--port` must be a port number'); + expect(error(['--port', '65536'])).toContain('`--port` must be a port number'); + expect(error(['--port', '6006.5'])).toContain('`--port` must be a port number'); + }); + + it('errors when --port has no value', () => { + expect(error(['--port'])).toContain('`--port` requires a value'); + }); + }); + describe('--json escape hatch', () => { it('uses the JSON object as the tool arguments', () => { expect(args(['--json', '{"id":"x","n":1}']).args).toEqual({ id: 'x', n: 1 }); diff --git a/code/lib/cli-storybook/src/ai/mcp/tool-args.ts b/code/lib/cli-storybook/src/ai/mcp/tool-args.ts index e095ccac8aa5..d97e99e55a5b 100644 --- a/code/lib/cli-storybook/src/ai/mcp/tool-args.ts +++ b/code/lib/cli-storybook/src/ai/mcp/tool-args.ts @@ -1,5 +1,11 @@ export type ParsedToolArgs = - | { ok: true; cwd: string | undefined; args: Record } + | { + ok: true; + cwd: string | undefined; + port: number | undefined; + help: boolean; + args: Record; + } | { ok: false; error: string }; /** @@ -10,17 +16,20 @@ export type ParsedToolArgs = * - A bare `--key` (no value) becomes `true`. * - `--json ''` is an escape hatch providing the raw argument object; explicit `--key` * flags override its entries. - * - `--cwd ` is consumed by the CLI itself and never forwarded to the tool. + * - `--cwd `, `--port ` and `--help`/`-h` are consumed by the CLI itself and never + * forwarded to the tool. * - * `defaults` carries `--cwd`/`--json` values that commander already parsed before the tool name; - * the same flags appearing after the tool name take precedence. + * `defaults` carries `--cwd`/`--port`/`--json` values that commander already parsed before the + * tool name; the same flags appearing after the tool name take precedence. */ export function parseToolArgs( tokens: string[], - defaults: { cwd?: string; json?: string } = {} + defaults: { cwd?: string; port?: string; json?: string } = {} ): ParsedToolArgs { let cwd = defaults.cwd; + let rawPort = defaults.port; let rawJson = defaults.json; + let help = false; const flagArgs: Record = {}; let i = 0; @@ -28,6 +37,11 @@ export function parseToolArgs( const token = tokens[i]; i += 1; + if (token === '--help' || token === '-h') { + help = true; + continue; + } + if (!token.startsWith('--') || token === '--') { return { ok: false, @@ -58,6 +72,14 @@ export function parseToolArgs( continue; } + if (key === 'port') { + if (value === undefined) { + return { ok: false, error: '`--port` requires a value.' }; + } + rawPort = value; + continue; + } + if (key === 'json') { if (value === undefined) { return { ok: false, error: '`--json` requires a value.' }; @@ -69,6 +91,17 @@ export function parseToolArgs( flagArgs[key] = value === undefined ? true : coerceValue(value); } + let port: number | undefined; + if (rawPort !== undefined) { + port = Number(rawPort); + if (!Number.isInteger(port) || port < 1 || port > 65535) { + return { + ok: false, + error: `\`--port\` must be a port number (1-65535), got \`${rawPort}\`.`, + }; + } + } + let jsonArgs: Record = {}; if (rawJson !== undefined) { let parsed: unknown; @@ -89,7 +122,7 @@ export function parseToolArgs( jsonArgs = parsed as Record; } - return { ok: true, cwd, args: { ...jsonArgs, ...flagArgs } }; + return { ok: true, cwd, port, help, args: { ...jsonArgs, ...flagArgs } }; } function coerceValue(raw: string): unknown { diff --git a/code/lib/cli-storybook/src/ai/mcp/types.ts b/code/lib/cli-storybook/src/ai/mcp/types.ts index 737d4ac630e0..2e007a15f115 100644 --- a/code/lib/cli-storybook/src/ai/mcp/types.ts +++ b/code/lib/cli-storybook/src/ai/mcp/types.ts @@ -35,6 +35,7 @@ export interface StorybookInstanceRecord { export type InterceptReason = | 'no-instance' + | 'port-mismatch' | 'storybook-not-installed' | 'addon-missing' | 'mcp-starting' diff --git a/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts b/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts index 2ca13173bd85..2f39c440e086 100644 --- a/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts @@ -1,65 +1,113 @@ +import { readFileSync } from 'node:fs'; import { createRequire } from 'node:module'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { vol } from 'memfs'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { STORYBOOK_MIN_VERSION, checkStorybookVersion } from './version-check.ts'; +import { + STORYBOOK_MIN_VERSION, + checkStorybookVersion, + classifyStorybookVersion, +} from './version-check.ts'; vi.mock('node:module', () => ({ createRequire: vi.fn(), })); -function mockStorybookVersion(version: string | null) { - vi.mocked(createRequire).mockImplementation( - () => - ((id: string) => { - if (id === 'storybook/package.json') { - if (version === null) { - throw new Error('not found'); - } - return { version }; - } - throw new Error(`unexpected require: ${id}`); - }) as unknown as ReturnType - ); -} - -beforeEach(() => { - vi.mocked(createRequire).mockReset(); -}); +// Spy-only mock: redirect the version reader's readFileSync to memfs. +vi.mock('node:fs', { spy: true }); -describe('checkStorybookVersion', () => { +describe('classifyStorybookVersion (pure)', () => { it('returns ok for the minimum version', () => { - mockStorybookVersion(STORYBOOK_MIN_VERSION); - expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); + expect(classifyStorybookVersion(STORYBOOK_MIN_VERSION)).toEqual({ status: 'ok' }); }); - it('returns too-old with the detected version for older Storybooks', () => { - mockStorybookVersion('9.1.16'); - expect(checkStorybookVersion('/a')).toEqual({ status: 'too-old', version: '9.1.16' }); + it('returns ok for a stable release above the minimum', () => { + expect(classifyStorybookVersion('10.6.2')).toEqual({ status: 'ok' }); }); - it('accepts a prerelease of the minimum (alpha/beta/rc)', () => { - mockStorybookVersion('10.5.0-alpha.1'); - expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); + it('accepts any prerelease of the minimum (alpha/beta/rc)', () => { + expect(classifyStorybookVersion('10.5.0-alpha.3')).toEqual({ status: 'ok' }); }); - it('treats a prerelease of an earlier version as too-old', () => { - mockStorybookVersion('10.4.0-rc.1'); - expect(checkStorybookVersion('/a')).toEqual({ status: 'too-old', version: '10.4.0-rc.1' }); + it('returns too-old for a version below the floor', () => { + expect(classifyStorybookVersion('9.1.16')).toEqual({ status: 'too-old', version: '9.1.16' }); }); - it('returns ok for a stable release above the minimum', () => { - mockStorybookVersion('11.0.0'); - expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); + it('treats a prerelease of an earlier version as too-old', () => { + expect(classifyStorybookVersion('10.4.0-rc.1')).toEqual({ + status: 'too-old', + version: '10.4.0-rc.1', + }); }); - it('returns not-installed when storybook is unresolvable', () => { - mockStorybookVersion(null); - expect(checkStorybookVersion('/a')).toEqual({ status: 'not-installed' }); + it('returns not-installed for undefined', () => { + expect(classifyStorybookVersion(undefined)).toEqual({ status: 'not-installed' }); }); it('treats 0.0.0-... versions as ok (canary)', () => { - mockStorybookVersion('0.0.0-canary.1234'); + expect(classifyStorybookVersion('0.0.0-canary.1234')).toEqual({ status: 'ok' }); + }); +}); + +describe('checkStorybookVersion (disk lookup)', () => { + function mockSearchPaths(paths: string[]) { + vi.mocked(createRequire).mockReturnValue({ + resolve: Object.assign(() => '', { paths: () => paths }), + } as unknown as ReturnType); + } + + beforeEach(async () => { + const memfs = await vi.importActual('memfs'); + vi.mocked(readFileSync).mockImplementation( + memfs.fs.readFileSync as unknown as typeof import('node:fs').readFileSync + ); + }); + + afterEach(() => { + vol.reset(); + vi.mocked(createRequire).mockReset(); + }); + + it('reads the version from the first search path holding a storybook package', () => { + mockSearchPaths(['/proj/node_modules', '/node_modules']); + vol.fromNestedJSON({ + '/proj/node_modules/storybook/package.json': JSON.stringify({ version: '10.5.0' }), + }); + expect(checkStorybookVersion('/proj')).toEqual({ status: 'ok' }); + }); + + it('walks up the search paths until storybook resolves (monorepo hoisting)', () => { + mockSearchPaths(['/repo/packages/app/node_modules', '/repo/node_modules']); + vol.fromNestedJSON({ + '/repo/node_modules/storybook/package.json': JSON.stringify({ version: '9.1.16' }), + }); + expect(checkStorybookVersion('/repo/packages/app')).toEqual({ + status: 'too-old', + version: '9.1.16', + }); + }); + + it('returns not-installed when no search path holds storybook', () => { + mockSearchPaths(['/proj/node_modules']); + vol.fromNestedJSON({ '/proj/node_modules': {} }); + expect(checkStorybookVersion('/proj')).toEqual({ status: 'not-installed' }); + }); + + it('skips malformed package.json files and keeps searching', () => { + mockSearchPaths(['/a/node_modules', '/b/node_modules']); + vol.fromNestedJSON({ + '/a/node_modules/storybook/package.json': '{ not json', + '/b/node_modules/storybook/package.json': JSON.stringify({ version: '10.5.0' }), + }); expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); }); + + it('treats a package.json without a string version as not-installed', () => { + mockSearchPaths(['/proj/node_modules']); + vol.fromNestedJSON({ + '/proj/node_modules/storybook/package.json': JSON.stringify({ name: 'storybook' }), + }); + expect(checkStorybookVersion('/proj')).toEqual({ status: 'not-installed' }); + }); }); diff --git a/code/lib/cli-storybook/src/ai/mcp/version-check.ts b/code/lib/cli-storybook/src/ai/mcp/version-check.ts index b1c755e32a32..44f3185b4288 100644 --- a/code/lib/cli-storybook/src/ai/mcp/version-check.ts +++ b/code/lib/cli-storybook/src/ai/mcp/version-check.ts @@ -1,3 +1,4 @@ +import { readFileSync } from 'node:fs'; import { createRequire } from 'node:module'; import { join } from 'node:path'; @@ -18,27 +19,37 @@ export type StorybookVersionStatus = | { status: 'too-old'; version: string } | { status: 'not-installed' }; -function readStorybookVersion(cwd: string): string | null { - try { - const requireFromCwd = createRequire(join(cwd, 'package.json')); - const { version } = requireFromCwd('storybook/package.json') as { version: string }; - return version; - } catch { - return null; +export function classifyStorybookVersion(version: string | undefined): StorybookVersionStatus { + if (version === undefined) { + return { status: 'not-installed' }; } -} - -export function checkStorybookVersion(cwd: string): StorybookVersionStatus { - const version = readStorybookVersion(cwd); // Storybook canary releases are published as `0.0.0-*`; semver considers these < any stable // version, but we still want to treat them as supported. - if (version?.startsWith('0.0.0-')) { + if (version.startsWith('0.0.0-')) { return { status: 'ok' }; } - if (version === null) { - return { status: 'not-installed' }; - } return lt(version, STORYBOOK_MIN_VERSION_FLOOR) ? { status: 'too-old', version } : { status: 'ok' }; } + +export function checkStorybookVersion(cwd: string): StorybookVersionStatus { + return classifyStorybookVersion(readStorybookVersion(cwd)); +} + +function readStorybookVersion(cwd: string): string | undefined { + const requireFromCwd = createRequire(join(cwd, 'package.json')); + // require.resolve.paths is the only way to get the actual search paths without going through + // Node's module-resolution cache. + const searchPaths = requireFromCwd.resolve.paths('storybook') ?? []; + for (const base of searchPaths) { + try { + const raw = readFileSync(join(base, 'storybook', 'package.json'), 'utf8'); + const { version } = JSON.parse(raw) as { version?: unknown }; + return typeof version === 'string' ? version : undefined; + } catch { + // parse error or file not found. + } + } + return undefined; +} From bcfa9dfd4c6c295d848326006ca6529b7d9c40bf Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 14:02:14 +0200 Subject: [PATCH 05/13] test(cli): adopt vi.mock spy mode and fix multi-line SSE test fixture 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. --- .../cli-storybook/src/ai/mcp/client.test.ts | 2 +- .../cli-storybook/src/ai/mcp/register.test.ts | 6 +----- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 19 +++---------------- .../src/ai/mcp/version-check.test.ts | 4 +--- 4 files changed, 6 insertions(+), 25 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/client.test.ts b/code/lib/cli-storybook/src/ai/mcp/client.test.ts index b277fd4adafb..63bd78e7bcd4 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.test.ts @@ -93,7 +93,7 @@ describe('callMcpTool', () => { id: 1, result: { content: [{ type: 'text', text: 'line\nwith newline' }] }, }; - const dataLines = JSON.stringify(envelope) + const dataLines = JSON.stringify(envelope, null, 2) .split('\n') .map((l) => `data: ${l}`) .join('\n'); diff --git a/code/lib/cli-storybook/src/ai/mcp/register.test.ts b/code/lib/cli-storybook/src/ai/mcp/register.test.ts index c3b7dff1fc45..95ca0ccc95cc 100644 --- a/code/lib/cli-storybook/src/ai/mcp/register.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/register.test.ts @@ -7,11 +7,7 @@ import { Command } from 'commander'; import { isAiCliFeatureEnabled, registerAiMcpPassthrough } from './register.ts'; import { buildToolCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; -vi.mock('./run-tool.ts', () => ({ - runAiTool: vi.fn(), - runAiToolHelp: vi.fn(), - buildToolCommandsHelp: vi.fn(), -})); +vi.mock('./run-tool.ts', { spy: true }); vi.mock('node:fs/promises', { spy: true }); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index a56e3619db82..6ffd712e3559 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -6,22 +6,9 @@ import { buildToolCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; import type { StorybookInstanceRecord } from './types.ts'; import { checkStorybookVersion } from './version-check.ts'; -vi.mock('./registry.ts', async (importOriginal) => ({ - ...(await importOriginal()), - readRegistry: vi.fn(), -})); - -// Preserve the McpJsonRpcError class identity so `instanceof` checks keep working. -vi.mock('./client.ts', async (importOriginal) => ({ - ...(await importOriginal()), - callMcpTool: vi.fn(), - listMcpTools: vi.fn(), -})); - -vi.mock('./version-check.ts', async (importOriginal) => ({ - ...(await importOriginal()), - checkStorybookVersion: vi.fn(), -})); +vi.mock('./registry.ts', { spy: true }); +vi.mock('./client.ts', { spy: true }); +vi.mock('./version-check.ts', { spy: true }); const record: StorybookInstanceRecord = { schemaVersion: 1, diff --git a/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts b/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts index 2f39c440e086..10786e0d032f 100644 --- a/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts @@ -10,9 +10,7 @@ import { classifyStorybookVersion, } from './version-check.ts'; -vi.mock('node:module', () => ({ - createRequire: vi.fn(), -})); +vi.mock('node:module', { spy: true }); // Spy-only mock: redirect the version reader's readFileSync to memfs. vi.mock('node:fs', { spy: true }); From 837b8a3cdb167d1c2d95137aab7d3bd3eb48ba40 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 14:20:04 +0200 Subject: [PATCH 06/13] feat(cli): say 'commands' in all user-facing copy, keep MCP an implementation 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. --- .../cli-storybook/src/ai/mcp/client.test.ts | 4 +- code/lib/cli-storybook/src/ai/mcp/client.ts | 16 ++++--- .../cli-storybook/src/ai/mcp/intercepts.ts | 12 ++--- .../cli-storybook/src/ai/mcp/register.test.ts | 16 ++++--- code/lib/cli-storybook/src/ai/mcp/register.ts | 38 ++++++++-------- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 44 ++++++++++--------- code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 30 ++++++------- .../lib/cli-storybook/src/ai/mcp/tool-args.ts | 2 +- 8 files changed, 85 insertions(+), 77 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/client.test.ts b/code/lib/cli-storybook/src/ai/mcp/client.test.ts index 63bd78e7bcd4..d61480fe6433 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.test.ts @@ -116,7 +116,7 @@ describe('callMcpTool', () => { const fetchImpl = vi.fn() as unknown as typeof fetch; await expect( callMcpTool(noEndpoint, { name: 'list-all-documentation' }, fetchImpl) - ).rejects.toThrow(/missing mcp\.endpoint/); + ).rejects.toThrow(/has no server endpoint registered/); }); it('throws when the response is not ok', async () => { @@ -146,7 +146,7 @@ describe('callMcpTool', () => { error: { code: -32601, message: 'unknown tool' }, })) as typeof fetch; const promise = callMcpTool(record, { name: 'nope' }, fetchImpl); - await expect(promise).rejects.toThrow(/Storybook MCP error -32601: unknown tool/); + await expect(promise).rejects.toThrow(/Storybook server error -32601: unknown tool/); await expect(promise).rejects.toBeInstanceOf(McpJsonRpcError); }); }); diff --git a/code/lib/cli-storybook/src/ai/mcp/client.ts b/code/lib/cli-storybook/src/ai/mcp/client.ts index 812a9c9164c8..6a8a9d5ec32a 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.ts @@ -24,7 +24,7 @@ export class McpJsonRpcError extends Error { public readonly code: number, message: string ) { - super(`Storybook MCP error ${code}: ${message}`); + super(`Storybook server error ${code}: ${message}`); this.name = 'McpJsonRpcError'; } } @@ -69,7 +69,7 @@ async function sendJsonRpcRequest( ): Promise { const endpoint = record.mcp.endpoint; if (!endpoint) { - throw new Error(`Storybook MCP record for ${record.cwd} is missing mcp.endpoint`); + throw new Error(`The Storybook instance at ${record.cwd} has no server endpoint registered`); } const target = new URL(endpoint, record.url).href; @@ -92,7 +92,7 @@ async function sendJsonRpcRequest( if (!response.ok) { throw new Error( - `Storybook MCP at ${target} responded with ${response.status} ${response.statusText}` + `The Storybook server at ${target} responded with ${response.status} ${response.statusText}` ); } @@ -105,7 +105,7 @@ async function sendJsonRpcRequest( throw new McpJsonRpcError(payload.error.code, payload.error.message); } if (payload.result === undefined) { - throw new Error('Storybook MCP returned no result'); + throw new Error('The Storybook server returned no result'); } return payload.result; } @@ -122,7 +122,7 @@ async function readJsonRpcResponse(response: Response, endpoint: string): Promis } throw new Error( - `Storybook MCP at ${endpoint} returned unsupported content-type "${contentType}". Expected application/json or text/event-stream.` + `The Storybook server at ${endpoint} returned unsupported content-type "${contentType}". Expected application/json or text/event-stream.` ); } @@ -146,13 +146,15 @@ function parseSseEnvelope(body: string, endpoint: string): unknown { } } if (dataLines.length === 0) { - throw new Error(`Storybook MCP at ${endpoint} returned an SSE response with no data event`); + throw new Error( + `The Storybook server at ${endpoint} returned an SSE response with no data event` + ); } try { return JSON.parse(dataLines.join('\n')); } catch (error) { throw new Error( - `Storybook MCP at ${endpoint} returned an SSE event whose data could not be parsed as JSON: ${ + `The Storybook server at ${endpoint} returned an SSE event whose data could not be parsed as JSON: ${ error instanceof Error ? error.message : String(error) }` ); diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts index d2342202f17b..5bb48f038270 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts @@ -17,7 +17,7 @@ const buildPortMismatch = (port: number | undefined, records: StorybookInstanceR `Storybook is running at this cwd, but not on port \`${port ?? 'unknown'}\`. Retry with one of the running ports below, or omit \`--port\` to route by cwd alone. Running Storybooks at this cwd: -${records.map((r) => `- port \`${r.port}\` (${r.url}, mcp: \`${r.mcp.status}\`)`).join('\n')}`; +${records.map((r) => `- port \`${r.port}\` (${r.url}, status: \`${r.mcp.status}\`)`).join('\n')}`; const buildStorybookTooOld = (version: string) => `The Storybook installed at this cwd is version \`${version}\`, but this command requires \`${STORYBOOK_MIN_VERSION}\` or newer. @@ -26,11 +26,11 @@ Ask the user whether they want to upgrade Storybook. If they agree, invoke the \ \`\`\` npx storybook add @storybook/addon-mcp \`\`\` -to install the MCP addon. Restart Storybook, then retry the command.`; +to install the addon that provides these commands. Restart Storybook, then retry the command.`; const STORYBOOK_NOT_INSTALLED = `No Storybook is running at this cwd, and Storybook does not appear to be installed here (\`storybook\` could not be resolved from this project). -Ask the user whether they want to add Storybook. If they agree, invoke the \`storybook-init\` skill to set it up, then install the MCP addon: +Ask the user whether they want to add Storybook. If they agree, invoke the \`storybook-init\` skill to set it up, then install the addon that provides these commands: \`\`\` npx storybook add @storybook/addon-mcp \`\`\` @@ -38,7 +38,7 @@ Start Storybook, then retry the command. If you believe Storybook is in fact installed (e.g. a monorepo where \`storybook\` resolves from a different location), start \`storybook dev\` from this exact cwd and retry — a running instance is always used regardless of this check.`; -const ADDON_MISSING = `Storybook is running but does not expose an MCP server. The \`@storybook/addon-mcp\` addon is missing. +const ADDON_MISSING = `Storybook is running but does not provide these commands. The \`@storybook/addon-mcp\` addon is missing. Install it: \`\`\` @@ -47,9 +47,9 @@ npx storybook add @storybook/addon-mcp Restart Storybook, then retry the command.`; -const MCP_STARTING = `Storybook is running but its MCP server is still starting up. Wait a moment and retry the command.`; +const MCP_STARTING = `Storybook is running but its command server is still starting up. Wait a moment and retry the command.`; -const MCP_ERROR = `Storybook is running but its MCP server reported an error. Inspect the Storybook terminal output, fix the underlying issue, then retry the command.`; +const MCP_ERROR = `Storybook is running but its command server reported an error. Inspect the Storybook terminal output, fix the underlying issue, then retry the command.`; export type InterceptExtras = { records?: StorybookInstanceRecord[]; diff --git a/code/lib/cli-storybook/src/ai/mcp/register.test.ts b/code/lib/cli-storybook/src/ai/mcp/register.test.ts index 95ca0ccc95cc..6d344a014c83 100644 --- a/code/lib/cli-storybook/src/ai/mcp/register.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/register.test.ts @@ -5,7 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { Command } from 'commander'; import { isAiCliFeatureEnabled, registerAiMcpPassthrough } from './register.ts'; -import { buildToolCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; +import { buildStorybookCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; vi.mock('./run-tool.ts', { spy: true }); @@ -61,7 +61,9 @@ function stdoutText(): string { beforeEach(() => { vi.mocked(runAiTool).mockResolvedValue({ exitCode: 0, output: 'ok' }); vi.mocked(runAiToolHelp).mockResolvedValue({ exitCode: 0, output: 'tool help' }); - vi.mocked(buildToolCommandsHelp).mockResolvedValue('Tool commands (from the Storybook):'); + vi.mocked(buildStorybookCommandsHelp).mockResolvedValue( + 'Storybook commands (from the running Storybook):' + ); vi.mocked(writeFile).mockResolvedValue(undefined); vi.spyOn(process.stdout, 'write').mockImplementation(() => true); }); @@ -70,7 +72,7 @@ afterEach(() => { vi.restoreAllMocks(); vi.mocked(runAiTool).mockReset(); vi.mocked(runAiToolHelp).mockReset(); - vi.mocked(buildToolCommandsHelp).mockReset(); + vi.mocked(buildStorybookCommandsHelp).mockReset(); process.exitCode = undefined; }); @@ -92,7 +94,7 @@ describe('without the feature flag (no registration)', () => { const { program, helpAction } = buildProgram({ withPassthrough: false }); await parse(program, ['ai']); expect(helpAction).toHaveBeenCalled(); - expect(buildToolCommandsHelp).not.toHaveBeenCalled(); + expect(buildStorybookCommandsHelp).not.toHaveBeenCalled(); }); }); @@ -172,11 +174,11 @@ describe('with the feature flag (passthrough registered)', () => { async (argv) => { const { program } = buildProgram({ withPassthrough: true }); await parse(program, argv); - expect(buildToolCommandsHelp).toHaveBeenCalledWith({ cwd: undefined, port: undefined }); + expect(buildStorybookCommandsHelp).toHaveBeenCalledWith({ cwd: undefined, port: undefined }); const output = stdoutText(); expect(output).toContain('Usage:'); expect(output).toContain('setup'); - expect(output).toContain('Tool commands (from the Storybook):'); + expect(output).toContain('Storybook commands (from the running Storybook):'); expect(runAiTool).not.toHaveBeenCalled(); } ); @@ -184,7 +186,7 @@ describe('with the feature flag (passthrough registered)', () => { it('passes --cwd and --port through to the tool commands section', async () => { const { program } = buildProgram({ withPassthrough: true }); await parse(program, ['ai', '--cwd', '/x', '--port', '6006', '--help']); - expect(buildToolCommandsHelp).toHaveBeenCalledWith({ cwd: '/x', port: '6006' }); + expect(buildStorybookCommandsHelp).toHaveBeenCalledWith({ cwd: '/x', port: '6006' }); }); it('shows single-tool help for `ai --help `', async () => { diff --git a/code/lib/cli-storybook/src/ai/mcp/register.ts b/code/lib/cli-storybook/src/ai/mcp/register.ts index 201e3372aa2c..411d04c73224 100644 --- a/code/lib/cli-storybook/src/ai/mcp/register.ts +++ b/code/lib/cli-storybook/src/ai/mcp/register.ts @@ -8,7 +8,7 @@ import type { Command } from 'commander'; import { type AiToolRunResult, - buildToolCommandsHelp, + buildStorybookCommandsHelp, runAiTool, runAiToolHelp, } from './run-tool.ts'; @@ -27,49 +27,51 @@ const PORT_DESCRIPTION = 'Port of the target Storybook, to address one specific instance when several run at the same cwd'; /** - * Register the MCP passthrough on the `ai` command: a generic `[tool] [toolArgs...]` argument pair - * that forwards any tool call to the running Storybook's MCP server. `passThroughOptions` hands - * every token after the tool name to the tool untouched, which requires positional options on the + * Register the passthrough on the `ai` command: a generic `[command] [args...]` argument pair that + * forwards any command to the running Storybook's server (MCP under the hood, but that is an + * implementation detail — user-facing copy says "commands"). `passThroughOptions` hands every + * token after the command name to the command untouched, which requires positional options on the * program. * * Commander's built-in (synchronous) help is replaced with our own `-h, --help` option so the help - * output can include the tool commands fetched from the running Storybook. + * output can include the commands fetched from the running Storybook. */ export function registerAiMcpPassthrough(program: Command, aiCommand: Command): void { program.enablePositionalOptions(); aiCommand .helpOption(false) - .argument('[tool]', 'Name of an MCP tool exposed by the running Storybook') + .usage('[options] [command] [args...]') + .argument('[command]', 'A command provided by the running Storybook') .argument( - '[toolArgs...]', - 'Tool arguments as `--key value` flags; values are JSON-parsed when possible' + '[args...]', + 'Command arguments as `--key value` flags; values are JSON-parsed when possible' ) .option('--cwd ', CWD_DESCRIPTION) .option('--port ', PORT_DESCRIPTION) .option( '--json ', - 'Raw JSON object with the tool arguments (escape hatch for complex values)' + 'Raw JSON object with the command arguments (escape hatch for complex values)' ) - .option('-h, --help', 'Show help, including the tool commands of the running Storybook') + .option('-h, --help', 'Show help, including the commands provided by the running Storybook') .passThroughOptions() .action( async ( - tool: string | undefined, - toolArgs: string[], + command: string | undefined, + commandArgs: string[], options: { cwd?: string; port?: string; json?: string; output?: string; help?: boolean } ) => { const target = { cwd: options.cwd, port: options.port }; - if (options.help && tool) { - await printResult(await runAiToolHelp(tool, target), options.output); + if (options.help && command) { + await printResult(await runAiToolHelp(command, target), options.output); return; } - if (options.help || !tool) { - const toolSection = await buildToolCommandsHelp(target); - process.stdout.write(`${aiCommand.helpInformation()}\n${toolSection}\n`); + if (options.help || !command) { + const commandsSection = await buildStorybookCommandsHelp(target); + process.stdout.write(`${aiCommand.helpInformation()}\n${commandsSection}\n`); return; } - const result = await runAiTool(tool, toolArgs, { ...target, json: options.json }); + const result = await runAiTool(command, commandArgs, { ...target, json: options.json }); await printResult(result, options.output); } ); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index 6ffd712e3559..b926948d19e5 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; import { readRegistry } from './registry.ts'; -import { buildToolCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; +import { buildStorybookCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; import type { StorybookInstanceRecord } from './types.ts'; import { checkStorybookVersion } from './version-check.ts'; @@ -159,17 +159,17 @@ describe('runAiTool', () => { it('prints a placeholder when the tool returns no content', async () => { vi.mocked(callMcpTool).mockResolvedValue({ content: [] }); const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); - expect(result).toEqual({ exitCode: 0, output: '(the tool returned no content)' }); + expect(result).toEqual({ exitCode: 0, output: '(the command returned no content)' }); }); it('surfaces a clean error when a ready record is missing its endpoint', async () => { vi.mocked(callMcpTool).mockRejectedValue( - new Error('Storybook MCP record for /projects/foo is missing mcp.endpoint') + new Error('The Storybook instance at /projects/foo has no server endpoint registered') ); vi.mocked(readRegistry).mockResolvedValue([{ ...record, mcp: { status: 'ready' } }]); const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Failed to reach Storybook MCP at (no endpoint)'); + expect(result.output).toContain('Failed to reach the Storybook server at (no endpoint)'); }); it('renders non-text content items as JSON blocks', async () => { @@ -189,7 +189,7 @@ describe('runAiTool', () => { vi.mocked(callMcpTool).mockRejectedValue(new McpJsonRpcError(-32601, 'unknown tool')); const result = await runAiTool('no-such-tool', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Unknown tool `no-such-tool`'); + expect(result.output).toContain('Unknown command `no-such-tool`'); expect(result.output).toContain('- `list-all-documentation`'); }); @@ -201,7 +201,7 @@ describe('runAiTool', () => { }); const result = await runAiTool('no-such-tool', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Unknown tool `no-such-tool`'); + expect(result.output).toContain('Unknown command `no-such-tool`'); expect(result.output).toContain('- `list-all-documentation`'); }); @@ -218,7 +218,7 @@ describe('runAiTool', () => { vi.mocked(callMcpTool).mockRejectedValue(new McpJsonRpcError(-32602, 'invalid arguments')); const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Storybook MCP error -32602: invalid arguments'); + expect(result.output).toContain('Storybook server error -32602: invalid arguments'); }); it('prints the original JSON-RPC error when the tool list cannot be fetched', async () => { @@ -226,14 +226,14 @@ describe('runAiTool', () => { vi.mocked(listMcpTools).mockRejectedValue(new Error('boom')); const result = await runAiTool('no-such-tool', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Storybook MCP error -32601: unknown tool'); + expect(result.output).toContain('Storybook server error -32601: unknown tool'); }); it('surfaces a friendly error when the MCP server is unreachable', async () => { vi.mocked(callMcpTool).mockRejectedValue(new Error('connection refused')); const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Failed to reach Storybook MCP at /mcp'); + expect(result.output).toContain('Failed to reach the Storybook server at /mcp'); expect(result.output).toContain('connection refused'); }); @@ -250,7 +250,7 @@ describe('runAiTool', () => { }); }); -describe('buildToolCommandsHelp', () => { +describe('buildStorybookCommandsHelp', () => { it('lists each tool with the first line of its description', async () => { vi.mocked(listMcpTools).mockResolvedValue([ { @@ -260,32 +260,34 @@ describe('buildToolCommandsHelp', () => { { name: 'list-all-documentation' }, ]); - const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); - expect(section).toContain('Tool commands (from the Storybook at http://localhost:6006):'); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain( + 'Storybook commands (from the Storybook running at http://localhost:6006):' + ); expect(section).toContain('get-documentation'); expect(section).toContain('Get docs for a component.'); expect(section).not.toContain('Long details'); - expect(section).toContain("Run 'storybook ai --help'"); + expect(section).toContain("Run 'storybook ai --help'"); }); it('degrades to a note when no Storybook is running (help must not fail)', async () => { vi.mocked(readRegistry).mockResolvedValue([]); - const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); - expect(section).toContain('Tool commands: (unavailable'); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('Storybook commands: (unavailable'); expect(section).toContain('storybook dev'); }); it('degrades to a note when the MCP server is unreachable', async () => { vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); - const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); - expect(section).toContain('Tool commands: (unavailable'); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('Storybook commands: (unavailable'); expect(section).toContain('could not be reached'); }); it('degrades to a note when no tools are exposed', async () => { vi.mocked(listMcpTools).mockResolvedValue([]); - const section = await buildToolCommandsHelp({ cwd: '/projects/foo' }); - expect(section).toContain('exposes no MCP tools'); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('provides no commands'); }); }); @@ -322,7 +324,7 @@ describe('runAiToolHelp', () => { it('lists the available tools for an unknown tool name', async () => { const result = await runAiToolHelp('no-such-tool', { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Unknown tool `no-such-tool`'); + expect(result.output).toContain('Unknown command `no-such-tool`'); expect(result.output).toContain('- `list-all-documentation`'); }); @@ -346,6 +348,6 @@ describe('runAiToolHelp', () => { vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); const result = await runAiToolHelp('get-documentation', { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); - expect(result.output).toContain('Failed to reach Storybook MCP at /mcp'); + expect(result.output).toContain('Failed to reach the Storybook server at /mcp'); }); }); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index 6b37b5cda73a..c6df94defd2f 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -80,7 +80,7 @@ export async function runAiTool( } return { exitCode: 1, - output: `Failed to reach Storybook MCP at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ + output: `Failed to reach the Storybook server at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ error instanceof Error ? error.message : String(error) }`, }; @@ -88,15 +88,15 @@ export async function runAiTool( } /** - * Build the "Tool commands" help section listing the tools of the running Storybook, appended to - * `storybook ai --help`. Help must never fail, so any error degrades to a short note explaining - * why no tools are listed. + * Build the "Storybook commands" help section listing the commands provided by the running + * Storybook, appended to `storybook ai --help`. Help must never fail, so any error degrades to a + * short note explaining why no commands are listed. */ -export async function buildToolCommandsHelp( +export async function buildStorybookCommandsHelp( options: AiToolOptions = {}, deps: AiToolRunDeps = {} ): Promise { - const unavailable = (note: string) => `Tool commands: (unavailable — ${note})`; + const unavailable = (note: string) => `Storybook commands: (unavailable — ${note})`; const parsed = parseToolArgs([], { cwd: options.cwd, port: options.port }); if (!parsed.ok) { @@ -106,7 +106,7 @@ export async function buildToolCommandsHelp( const resolution = await resolveReadyInstance(parsed.cwd, parsed.port, deps); if (resolution.kind === 'error') { return unavailable( - `no running Storybook with MCP detected at this cwd; start \`storybook dev\` to list its tools` + `no running Storybook detected at this cwd; start \`storybook dev\` to list its commands` ); } const { record } = resolution; @@ -115,10 +115,10 @@ export async function buildToolCommandsHelp( try { tools = await listMcpTools(record, deps.fetchImpl); } catch { - return unavailable(`the Storybook MCP at ${record.url} could not be reached`); + return unavailable(`the Storybook at ${record.url} could not be reached`); } if (tools.length === 0) { - return unavailable(`the Storybook at ${record.url} exposes no MCP tools`); + return unavailable(`the Storybook at ${record.url} provides no commands`); } const width = Math.max(...tools.map((tool) => tool.name.length)) + 2; @@ -127,10 +127,10 @@ export async function buildToolCommandsHelp( return ` ${tool.name.padEnd(width)}${summary}`; }); return [ - `Tool commands (from the Storybook at ${record.url}):`, + `Storybook commands (from the Storybook running at ${record.url}):`, ...lines, '', - `Run 'storybook ai --help' for a tool's description and arguments.`, + `Run 'storybook ai --help' for a command's description and arguments.`, ].join('\n'); } @@ -157,7 +157,7 @@ export async function runAiToolHelp( } catch (error) { return { exitCode: 1, - output: `Failed to reach Storybook MCP at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ + output: `Failed to reach the Storybook server at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ error instanceof Error ? error.message : String(error) }`, }; @@ -245,18 +245,18 @@ function formatUnknownTool( tools: McpToolDescriptor[], record: StorybookInstanceRecord ): string { - return `Unknown tool \`${toolName}\`. The Storybook at ${record.url} exposes: + return `Unknown command \`${toolName}\`. The Storybook running at ${record.url} provides: ${tools.map((tool) => `- \`${tool.name}\``).join('\n')} -Run \`storybook ai --help\` for all commands, or \`storybook ai --help\` for a tool's arguments.`; +Run \`storybook ai --help\` for all commands, or \`storybook ai --help\` for a command's arguments.`; } /** Render a tools/call result as markdown: text content verbatim, other content as JSON blocks. */ function formatToolResult(result: ToolCallResult): string { const content = result.content ?? []; if (content.length === 0) { - return '(the tool returned no content)'; + return '(the command returned no content)'; } return content .map((item) => diff --git a/code/lib/cli-storybook/src/ai/mcp/tool-args.ts b/code/lib/cli-storybook/src/ai/mcp/tool-args.ts index d97e99e55a5b..ee1357fba742 100644 --- a/code/lib/cli-storybook/src/ai/mcp/tool-args.ts +++ b/code/lib/cli-storybook/src/ai/mcp/tool-args.ts @@ -45,7 +45,7 @@ export function parseToolArgs( if (!token.startsWith('--') || token === '--') { return { ok: false, - error: `Unexpected argument \`${token}\`. Tool arguments must be passed as \`--key value\` flags (or via \`--json ''\`).`, + error: `Unexpected argument \`${token}\`. Command arguments must be passed as \`--key value\` flags (or via \`--json ''\`).`, }; } From f35dc82cb716309119a4d5221890f9195ee57124 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 15:02:09 +0200 Subject: [PATCH 07/13] refactor(cli): validate instance records with valibot Restores parity with @storybook/mcp-proxy's registry schema instead of a hand-rolled type guard; valibot was already a monorepo dependency. --- code/lib/cli-storybook/package.json | 3 +- code/lib/cli-storybook/src/ai/mcp/registry.ts | 48 +++---------------- code/lib/cli-storybook/src/ai/mcp/types.ts | 35 +++++++------- yarn.lock | 1 + 4 files changed, 29 insertions(+), 58 deletions(-) diff --git a/code/lib/cli-storybook/package.json b/code/lib/cli-storybook/package.json index 2cd185a664a8..85b00077aece 100644 --- a/code/lib/cli-storybook/package.json +++ b/code/lib/cli-storybook/package.json @@ -61,7 +61,8 @@ "slash": "^5.0.0", "tiny-invariant": "^1.3.3", "tinyclip": "^0.1.12", - "typescript": "^5.8.3" + "typescript": "^5.8.3", + "valibot": "^1.4.0" }, "publishConfig": { "access": "public" diff --git a/code/lib/cli-storybook/src/ai/mcp/registry.ts b/code/lib/cli-storybook/src/ai/mcp/registry.ts index e09b22275bba..9cc702d43ef1 100644 --- a/code/lib/cli-storybook/src/ai/mcp/registry.ts +++ b/code/lib/cli-storybook/src/ai/mcp/registry.ts @@ -2,7 +2,9 @@ import * as fs from 'node:fs/promises'; import { homedir } from 'node:os'; import { join } from 'node:path'; -import type { McpStatus, StorybookInstanceRecord } from './types.ts'; +import * as v from 'valibot'; + +import { type StorybookInstanceRecord, StorybookInstanceRecordSchema } from './types.ts'; /** * Must stay in sync with `getDefaultRuntimeInstanceRegistryDir` in @@ -19,42 +21,6 @@ export const DEFAULT_REGISTRY_DIR = join(homedir(), '.storybook', 'instances'); */ const SOFT_REGISTRY_ERRORS = new Set(['ENOENT', 'EACCES', 'EPERM', 'ENOTDIR']); -const MCP_STATUSES: McpStatus[] = ['not-installed', 'starting', 'ready', 'error']; - -function isOptionalString(value: unknown): value is string | undefined { - return value === undefined || typeof value === 'string'; -} - -function isIntegerInRange(value: unknown, min: number, max: number): value is number { - return typeof value === 'number' && Number.isInteger(value) && value >= min && value <= max; -} - -/** Validate a parsed registry file against the record shape; null for anything malformed. */ -export function parseInstanceRecord(value: unknown): StorybookInstanceRecord | null { - if (typeof value !== 'object' || value === null) { - return null; - } - const record = value as Record; - const mcp = record.mcp as Record | null | undefined; - - const isValid = - record.schemaVersion === 1 && - typeof record.instanceId === 'string' && - isIntegerInRange(record.pid, 1, Number.MAX_SAFE_INTEGER) && - typeof record.cwd === 'string' && - typeof record.url === 'string' && - isIntegerInRange(record.port, 1, 65535) && - isOptionalString(record.storybookVersion) && - isOptionalString(record.startedAt) && - isOptionalString(record.updatedAt) && - typeof mcp === 'object' && - mcp !== null && - MCP_STATUSES.includes(mcp.status as McpStatus) && - isOptionalString(mcp.endpoint); - - return isValid ? (value as StorybookInstanceRecord) : null; -} - /** * Read all Storybook instance records from `registryDir`. * @@ -81,15 +47,15 @@ export async function readRegistry( .map(async (name) => { try { const raw = await fs.readFile(join(registryDir, name), 'utf-8'); - const record = parseInstanceRecord(JSON.parse(raw)); - if (!record) { + const parsed = v.safeParse(StorybookInstanceRecordSchema, JSON.parse(raw)); + if (!parsed.success) { return null; } - if (!isProcessAlive(record.pid)) { + if (!isProcessAlive(parsed.output.pid)) { await fs.rm(join(registryDir, name), { force: true }).catch(() => {}); return null; } - return record; + return parsed.output; } catch { return null; } diff --git a/code/lib/cli-storybook/src/ai/mcp/types.ts b/code/lib/cli-storybook/src/ai/mcp/types.ts index 2e007a15f115..9580f5b359b8 100644 --- a/code/lib/cli-storybook/src/ai/mcp/types.ts +++ b/code/lib/cli-storybook/src/ai/mcp/types.ts @@ -5,33 +5,36 @@ * this reader is intentionally more lenient (extra statuses, optional fields) so it * also accepts records written by other Storybook versions and wrappers. */ +import * as v from 'valibot'; /** * The in-repo writer only emits `not-installed` and `ready`; `starting` and `error` are written by * external wrappers (e.g. the storybookjs/mcp launch script) and must keep being dispatched here. */ -export type McpStatus = 'not-installed' | 'starting' | 'ready' | 'error'; +export const McpStatusSchema = v.picklist(['not-installed', 'starting', 'ready', 'error']); +export type McpStatus = v.InferOutput; /** * A single Storybook runtime record written under the registry dir (default * `~/.storybook/instances`). One file per running `storybook dev` instance. * Spec: storybookjs/storybook#34826. */ -export interface StorybookInstanceRecord { - schemaVersion: 1; - instanceId: string; - pid: number; - cwd: string; - url: string; - port: number; - storybookVersion?: string; - startedAt?: string; - updatedAt?: string; - mcp: { - status: McpStatus; - endpoint?: string; - }; -} +export const StorybookInstanceRecordSchema = v.object({ + schemaVersion: v.literal(1), + instanceId: v.string(), + pid: v.pipe(v.number(), v.minValue(1), v.integer()), + cwd: v.string(), + url: v.string(), + port: v.pipe(v.number(), v.minValue(1), v.maxValue(65535), v.integer()), + storybookVersion: v.optional(v.string()), + startedAt: v.optional(v.string()), + updatedAt: v.optional(v.string()), + mcp: v.object({ + status: McpStatusSchema, + endpoint: v.optional(v.string()), + }), +}); +export type StorybookInstanceRecord = v.InferOutput; export type InterceptReason = | 'no-instance' diff --git a/yarn.lock b/yarn.lock index cd693e924df8..9c5289d30a31 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8517,6 +8517,7 @@ __metadata: tinyclip: "npm:^0.1.12" ts-dedent: "npm:^2.0.0" typescript: "npm:^5.8.3" + valibot: "npm:^1.4.0" bin: cli: ./dist/bin/index.js languageName: unknown From 39848688b95d540b9bb5778a5ab570b883b29af5 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 15:42:41 +0200 Subject: [PATCH 08/13] fix(cli): honor --port after the command name on the help path 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'. --- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 8 +++++ code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 36 ++++++++++--------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index b926948d19e5..70108eb98e34 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -321,6 +321,14 @@ describe('runAiToolHelp', () => { expect(callMcpTool).not.toHaveBeenCalled(); }); + it('honors a --port token given after the command name on the help path', async () => { + const result = await runAiTool('get-documentation', ['--port', '9999', '--help'], { + cwd: '/projects/foo', + }); + expect(result.exitCode).toBe(1); + expect(result.output).toContain('not on port `9999`'); + }); + it('lists the available tools for an unknown tool name', async () => { const result = await runAiToolHelp('no-such-tool', { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index c6df94defd2f..a5d82f90a396 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -45,7 +45,7 @@ export async function runAiTool( return { exitCode: 1, output: parsed.error }; } if (parsed.help) { - return runAiToolHelp(toolName, { cwd: parsed.cwd, port: options.port }, deps); + return toolHelp(toolName, parsed.cwd, parsed.port, deps); } const resolution = await resolveReadyInstance(parsed.cwd, parsed.port, deps); @@ -78,12 +78,7 @@ export async function runAiTool( const unknownTool = await describeUnknownTool(record, toolName, deps.fetchImpl); return { exitCode: 1, output: unknownTool ?? error.message }; } - return { - exitCode: 1, - output: `Failed to reach the Storybook server at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ - error instanceof Error ? error.message : String(error) - }`, - }; + return { exitCode: 1, output: formatServerUnreachable(record, error) }; } } @@ -134,7 +129,7 @@ export async function buildStorybookCommandsHelp( ].join('\n'); } -/** Show the description and arguments of a single MCP tool (`storybook ai --help`). */ +/** Show the description and arguments of a single command (`storybook ai --help`). */ export async function runAiToolHelp( toolName: string, options: AiToolOptions = {}, @@ -144,8 +139,16 @@ export async function runAiToolHelp( if (!parsed.ok) { return { exitCode: 1, output: parsed.error }; } + return toolHelp(toolName, parsed.cwd, parsed.port, deps); +} - const resolution = await resolveReadyInstance(parsed.cwd, parsed.port, deps); +async function toolHelp( + toolName: string, + cwd: string | undefined, + port: number | undefined, + deps: AiToolRunDeps +): Promise { + const resolution = await resolveReadyInstance(cwd, port, deps); if (resolution.kind === 'error') { return { exitCode: 1, output: resolution.output }; } @@ -155,12 +158,7 @@ export async function runAiToolHelp( try { tools = await listMcpTools(record, deps.fetchImpl); } catch (error) { - return { - exitCode: 1, - output: `Failed to reach the Storybook server at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ - error instanceof Error ? error.message : String(error) - }`, - }; + return { exitCode: 1, output: formatServerUnreachable(record, error) }; } const tool = tools.find((candidate) => candidate.name === toolName); @@ -170,6 +168,12 @@ export async function runAiToolHelp( return { exitCode: 0, output: formatToolHelp(tool) }; } +function formatServerUnreachable(record: StorybookInstanceRecord, error: unknown): string { + return `Failed to reach the Storybook server at ${record.mcp.endpoint ?? '(no endpoint)'}: ${ + error instanceof Error ? error.message : String(error) + }`; +} + type InstanceResolution = | { kind: 'ok'; record: StorybookInstanceRecord; matches: StorybookInstanceRecord[] } | { kind: 'error'; output: string }; @@ -297,7 +301,7 @@ function formatMultiInstanceWarning( const all = [chosen, ...siblings]; const lines = all.map((r) => { const marker = r === chosen ? ' (used)' : ''; - return `> - pid \`${r.pid}\` at ${r.url} (mcp: \`${r.mcp.status}\`)${marker}`; + return `> - pid \`${r.pid}\` at ${r.url} (status: \`${r.mcp.status}\`)${marker}`; }); return `> Warning: Multiple Storybook instances are running at this cwd. This call was sent to pid \`${chosen.pid}\`. > From d28b6f31f6b3604da0c954f3d3d55af11ddbbb04 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 16:00:34 +0200 Subject: [PATCH 09/13] feat(cli): accurate multi-instance and intercept notes in ai --help 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. --- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 31 ++++++++ code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 74 +++++++++++++++++-- 2 files changed, 97 insertions(+), 8 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index 70108eb98e34..d57c0fba3b2d 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -277,6 +277,37 @@ describe('buildStorybookCommandsHelp', () => { expect(section).toContain('storybook dev'); }); + it('lists the sibling ports when several instances run at the cwd', async () => { + const older = { ...record, instanceId: 'inst-2', pid: 2, port: 6007 }; + const newest = { ...record, startedAt: '2026-06-10T12:00:00.000Z' }; + vi.mocked(readRegistry).mockResolvedValue([newest, older]); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('2 instances are running at this cwd'); + expect(section).toContain('port 6006'); + expect(section).toContain('other ports: 6007'); + expect(section).toContain('`--port`'); + }); + + it('names the port mismatch instead of claiming nothing is running', async () => { + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo', port: '9999' }); + expect(section).toContain('Storybook commands: (unavailable'); + expect(section).toContain('no instance on port `9999`'); + expect(section).toContain('running ports: 6006'); + expect(section).not.toContain('no running Storybook detected'); + }); + + it('says the Storybook is starting up instead of claiming nothing is running', async () => { + vi.mocked(readRegistry).mockResolvedValue([{ ...record, mcp: { status: 'starting' } }]); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('still starting up'); + }); + + it('points at the missing addon instead of claiming nothing is running', async () => { + vi.mocked(readRegistry).mockResolvedValue([{ ...record, mcp: { status: 'not-installed' } }]); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('install `@storybook/addon-mcp`'); + }); + it('degrades to a note when the MCP server is unreachable', async () => { vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index a5d82f90a396..48bb9bebdcb4 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -5,8 +5,17 @@ import { getInterceptMarkdown } from './intercepts.ts'; import { readRegistry } from './registry.ts'; import { resolveInstance } from './resolve-instance.ts'; import { parseToolArgs } from './tool-args.ts'; -import type { McpToolDescriptor, StorybookInstanceRecord, ToolCallResult } from './types.ts'; -import { checkStorybookVersion, classifyStorybookVersion } from './version-check.ts'; +import type { + InterceptReason, + McpToolDescriptor, + StorybookInstanceRecord, + ToolCallResult, +} from './types.ts'; +import { + STORYBOOK_MIN_VERSION, + checkStorybookVersion, + classifyStorybookVersion, +} from './version-check.ts'; export type AiToolRunResult = { exitCode: 0 | 1; output: string }; @@ -100,11 +109,9 @@ export async function buildStorybookCommandsHelp( const resolution = await resolveReadyInstance(parsed.cwd, parsed.port, deps); if (resolution.kind === 'error') { - return unavailable( - `no running Storybook detected at this cwd; start \`storybook dev\` to list its commands` - ); + return unavailable(helpUnavailableNote(resolution, parsed.port)); } - const { record } = resolution; + const { record, matches } = resolution; let tools: McpToolDescriptor[]; try { @@ -116,6 +123,14 @@ export async function buildStorybookCommandsHelp( return unavailable(`the Storybook at ${record.url} provides no commands`); } + const siblingPorts = matches.filter((r) => r !== record).map((r) => r.port); + const siblingNote = + siblingPorts.length > 0 + ? [ + `(${matches.length} instances are running at this cwd — using the most recently started, port ${record.port}; other ports: ${siblingPorts.join(', ')}. Pass \`--port\` to target a specific one.)`, + ] + : []; + const width = Math.max(...tools.map((tool) => tool.name.length)) + 2; const lines = tools.map((tool) => { const summary = tool.description?.trim().split('\n')[0] ?? ''; @@ -123,12 +138,41 @@ export async function buildStorybookCommandsHelp( }); return [ `Storybook commands (from the Storybook running at ${record.url}):`, + ...siblingNote, ...lines, '', `Run 'storybook ai --help' for a command's description and arguments.`, ].join('\n'); } +/** One-line reason why the help section cannot list commands, accurate per intercept. */ +function helpUnavailableNote( + error: Extract, + port: number | undefined +): string { + switch (error.reason) { + case 'no-instance': + case 'storybook-not-installed': + return 'no running Storybook detected at this cwd; start `storybook dev` to list its commands'; + case 'port-mismatch': + return `no instance on port \`${port}\` at this cwd — running ports: ${error.records + .map((r) => r.port) + .join(', ')}`; + case 'mcp-starting': + return 'the Storybook at this cwd is still starting up; retry in a moment'; + case 'addon-missing': + return 'the running Storybook does not provide commands — install `@storybook/addon-mcp`'; + case 'mcp-error': + return "the running Storybook's command server reported an error"; + case 'storybook-too-old': + return `the installed Storybook is too old for these commands (requires ${STORYBOOK_MIN_VERSION} or newer)`; + default: { + const unhandled: never = error.reason; + throw new Error(`Unhandled intercept reason: ${unhandled as string}`); + } + } +} + /** Show the description and arguments of a single command (`storybook ai --help`). */ export async function runAiToolHelp( toolName: string, @@ -176,7 +220,12 @@ function formatServerUnreachable(record: StorybookInstanceRecord, error: unknown type InstanceResolution = | { kind: 'ok'; record: StorybookInstanceRecord; matches: StorybookInstanceRecord[] } - | { kind: 'error'; output: string }; + | { + kind: 'error'; + output: string; + reason: InterceptReason; + records: StorybookInstanceRecord[]; + }; /** * Resolve the running Storybook instance for `cwdInput`, mirroring the mcp-proxy dispatch order: @@ -203,6 +252,8 @@ async function resolveReadyInstance( return { kind: 'error', output: getInterceptMarkdown('storybook-too-old', { version: versionStatus.version }), + reason: 'storybook-too-old', + records: [], }; } @@ -212,11 +263,18 @@ async function resolveReadyInstance( versionStatus.status === 'not-installed' && (resolution.records?.length ?? 0) === 0 ) { - return { kind: 'error', output: getInterceptMarkdown('storybook-not-installed') }; + return { + kind: 'error', + output: getInterceptMarkdown('storybook-not-installed'), + reason: 'storybook-not-installed', + records: [], + }; } return { kind: 'error', output: getInterceptMarkdown(resolution.reason, { records: resolution.records, port }), + reason: resolution.reason, + records: resolution.records ?? [], }; } From 838d82b6a087bf427f4053634164500ce6c0b83e Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 16:03:38 +0200 Subject: [PATCH 10/13] feat(cli): show the Storybook version in the ai --help section The section header includes the version reported by the running instance, and the too-old note names the detected version next to the minimum. --- .../lib/cli-storybook/src/ai/mcp/run-tool.test.ts | 15 +++++++++++++++ code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 8 ++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index d57c0fba3b2d..f75724b3e657 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -308,6 +308,21 @@ describe('buildStorybookCommandsHelp', () => { expect(section).toContain('install `@storybook/addon-mcp`'); }); + it('shows the Storybook version reported by the running instance', async () => { + vi.mocked(readRegistry).mockResolvedValue([{ ...record, storybookVersion: '10.5.0' }]); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain( + 'Storybook commands (from the Storybook running at http://localhost:6006, Storybook 10.5.0):' + ); + }); + + it('names the detected version in the too-old note', async () => { + vi.mocked(readRegistry).mockResolvedValue([{ ...record, storybookVersion: '9.0.5' }]); + const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); + expect(section).toContain('version `9.0.5`'); + expect(section).toContain('require `10.5.0` or newer'); + }); + it('degrades to a note when the MCP server is unreachable', async () => { vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index 48bb9bebdcb4..24fc269dd6ad 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -136,8 +136,9 @@ export async function buildStorybookCommandsHelp( const summary = tool.description?.trim().split('\n')[0] ?? ''; return ` ${tool.name.padEnd(width)}${summary}`; }); + const version = record.storybookVersion ? `, Storybook ${record.storybookVersion}` : ''; return [ - `Storybook commands (from the Storybook running at ${record.url}):`, + `Storybook commands (from the Storybook running at ${record.url}${version}):`, ...siblingNote, ...lines, '', @@ -165,7 +166,7 @@ function helpUnavailableNote( case 'mcp-error': return "the running Storybook's command server reported an error"; case 'storybook-too-old': - return `the installed Storybook is too old for these commands (requires ${STORYBOOK_MIN_VERSION} or newer)`; + return `the installed Storybook is version \`${error.version ?? 'unknown'}\`, but these commands require \`${STORYBOOK_MIN_VERSION}\` or newer`; default: { const unhandled: never = error.reason; throw new Error(`Unhandled intercept reason: ${unhandled as string}`); @@ -225,6 +226,8 @@ type InstanceResolution = output: string; reason: InterceptReason; records: StorybookInstanceRecord[]; + /** Detected Storybook version, set for the `storybook-too-old` reason. */ + version?: string; }; /** @@ -254,6 +257,7 @@ async function resolveReadyInstance( output: getInterceptMarkdown('storybook-too-old', { version: versionStatus.version }), reason: 'storybook-too-old', records: [], + version: versionStatus.version, }; } From 0e314d5e62bb2d2217f58c5e4047076636b0953d Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 16:47:14 +0200 Subject: [PATCH 11/13] fix(cli): validate JSON-RPC payloads instead of trusting them 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. --- .../cli-storybook/src/ai/mcp/client.test.ts | 39 +++++++++++ code/lib/cli-storybook/src/ai/mcp/client.ts | 64 ++++++++++++++----- code/lib/cli-storybook/src/ai/mcp/types.ts | 53 +++++++++------ 3 files changed, 121 insertions(+), 35 deletions(-) diff --git a/code/lib/cli-storybook/src/ai/mcp/client.test.ts b/code/lib/cli-storybook/src/ai/mcp/client.test.ts index d61480fe6433..6dc23864a145 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.test.ts @@ -149,6 +149,35 @@ describe('callMcpTool', () => { await expect(promise).rejects.toThrow(/Storybook server error -32601: unknown tool/); await expect(promise).rejects.toBeInstanceOf(McpJsonRpcError); }); + + it.each([ + ['a primitive result', { jsonrpc: '2.0', id: 1, result: 'hello' }], + ['a null result', { jsonrpc: '2.0', id: 1, result: null }], + [ + 'a content item without a type', + { jsonrpc: '2.0', id: 1, result: { content: [{ text: 'x' }] } }, + ], + ['a malformed error object', { jsonrpc: '2.0', id: 1, error: { code: 'x' } }], + ])('rejects %s as an unexpected response shape', async (_label, body) => { + const fetchImpl = (async () => jsonResponse(body)) as typeof fetch; + await expect( + callMcpTool(record, { name: 'list-all-documentation' }, fetchImpl) + ).rejects.toThrow(/unexpected response shape/); + }); + + it('passes through extra content fields and result keys (loose validation)', async () => { + const fetchImpl = (async () => + jsonResponse({ + jsonrpc: '2.0', + id: 1, + result: { + content: [{ type: 'resource_link', uri: 'http://x' }], + _meta: { 'storybook.dev/foo': 1 }, + }, + })) as typeof fetch; + const result = await callMcpTool(record, { name: 'x' }, fetchImpl); + expect(result.content?.[0]).toMatchObject({ type: 'resource_link', uri: 'http://x' }); + }); }); describe('listMcpTools', () => { @@ -172,4 +201,14 @@ describe('listMcpTools', () => { jsonResponse({ jsonrpc: '2.0', id: 'x', result: {} })) as typeof fetch; await expect(listMcpTools(record, fetchImpl)).resolves.toEqual([]); }); + + it('rejects tool descriptors without a name as an unexpected response shape', async () => { + const fetchImpl = (async () => + jsonResponse({ + jsonrpc: '2.0', + id: 'x', + result: { tools: [{ description: 'nameless' }] }, + })) as typeof fetch; + await expect(listMcpTools(record, fetchImpl)).rejects.toThrow(/unexpected response shape/); + }); }); diff --git a/code/lib/cli-storybook/src/ai/mcp/client.ts b/code/lib/cli-storybook/src/ai/mcp/client.ts index 6a8a9d5ec32a..d7800642d6de 100644 --- a/code/lib/cli-storybook/src/ai/mcp/client.ts +++ b/code/lib/cli-storybook/src/ai/mcp/client.ts @@ -1,4 +1,12 @@ -import type { McpToolDescriptor, StorybookInstanceRecord, ToolCallResult } from './types.ts'; +import * as v from 'valibot'; + +import { + type McpToolDescriptor, + McpToolDescriptorSchema, + type StorybookInstanceRecord, + type ToolCallResult, + ToolCallResultSchema, +} from './types.ts'; /** * Marks the request as coming from a trusted local Storybook client. `@storybook/addon-mcp` uses @@ -29,13 +37,22 @@ export class McpJsonRpcError extends Error { } } +const JsonRpcEnvelopeSchema = v.looseObject({ + result: v.optional(v.unknown()), + error: v.optional(v.looseObject({ code: v.number(), message: v.string() })), +}); + +const ToolListResultSchema = v.looseObject({ + tools: v.optional(v.array(McpToolDescriptorSchema)), +}); + /** Forward an MCP `tools/call` JSON-RPC request to a local Storybook MCP server. */ export async function callMcpTool( record: StorybookInstanceRecord, params: ToolCallParams, fetchImpl: typeof fetch = fetch ): Promise { - return (await sendJsonRpcRequest(record, 'tools/call', params, fetchImpl)) as ToolCallResult; + return sendJsonRpcRequest(record, 'tools/call', params, ToolCallResultSchema, fetchImpl); } /** List the tools exposed by a local Storybook MCP server via `tools/list`. */ @@ -43,10 +60,14 @@ export async function listMcpTools( record: StorybookInstanceRecord, fetchImpl: typeof fetch = fetch ): Promise { - const result = (await sendJsonRpcRequest(record, 'tools/list', {}, fetchImpl)) as { - tools?: McpToolDescriptor[]; - } | null; - return result?.tools ?? []; + const result = await sendJsonRpcRequest( + record, + 'tools/list', + {}, + ToolListResultSchema, + fetchImpl + ); + return result.tools ?? []; } /** @@ -61,12 +82,13 @@ export async function listMcpTools( * tmcp hardcodes `text/event-stream` for any request with an id, so we accept both content-types * and parse the SSE envelope when needed. */ -async function sendJsonRpcRequest( +async function sendJsonRpcRequest( record: StorybookInstanceRecord, method: 'tools/call' | 'tools/list', params: unknown, + resultSchema: v.GenericSchema, fetchImpl: typeof fetch -): Promise { +): Promise { const endpoint = record.mcp.endpoint; if (!endpoint) { throw new Error(`The Storybook instance at ${record.cwd} has no server endpoint registered`); @@ -96,18 +118,28 @@ async function sendJsonRpcRequest( ); } - const payload = (await readJsonRpcResponse(response, target)) as { - result?: unknown; - error?: { code: number; message: string }; - }; + const payload = await readJsonRpcResponse(response, target); - if (payload.error) { - throw new McpJsonRpcError(payload.error.code, payload.error.message); + const envelope = v.safeParse(JsonRpcEnvelopeSchema, payload); + if (!envelope.success) { + throw unexpectedShapeError(target); + } + if (envelope.output.error) { + throw new McpJsonRpcError(envelope.output.error.code, envelope.output.error.message); } - if (payload.result === undefined) { + if (envelope.output.result === undefined) { throw new Error('The Storybook server returned no result'); } - return payload.result; + + const result = v.safeParse(resultSchema, envelope.output.result); + if (!result.success) { + throw unexpectedShapeError(target); + } + return result.output; +} + +function unexpectedShapeError(target: string): Error { + return new Error(`The Storybook server at ${target} returned an unexpected response shape`); } async function readJsonRpcResponse(response: Response, endpoint: string): Promise { diff --git a/code/lib/cli-storybook/src/ai/mcp/types.ts b/code/lib/cli-storybook/src/ai/mcp/types.ts index 9580f5b359b8..9c5a3d7a3fcf 100644 --- a/code/lib/cli-storybook/src/ai/mcp/types.ts +++ b/code/lib/cli-storybook/src/ai/mcp/types.ts @@ -45,25 +45,40 @@ export type InterceptReason = | 'mcp-error' | 'storybook-too-old'; -export interface ToolResultContentItem { - type: string; - text?: string; - [key: string]: unknown; -} +/** + * Result of an MCP `tools/call` request, as returned by `@storybook/addon-mcp`. Loose: servers may + * legally attach extra fields (`_meta`, `structuredContent`, image/audio content properties); we + * validate only what the CLI renders and pass the rest through. + */ +export const ToolResultContentItemSchema = v.looseObject({ + type: v.string(), + text: v.optional(v.string()), +}); +export type ToolResultContentItem = v.InferOutput; -/** Result of an MCP `tools/call` request, as returned by `@storybook/addon-mcp`. */ -export interface ToolCallResult { - content?: ToolResultContentItem[]; - isError?: boolean; - _meta?: Record; -} +export const ToolCallResultSchema = v.looseObject({ + content: v.optional(v.array(ToolResultContentItemSchema)), + isError: v.optional(v.boolean()), +}); +export type ToolCallResult = v.InferOutput; /** A tool descriptor from an MCP `tools/list` response. */ -export interface McpToolDescriptor { - name: string; - description?: string; - inputSchema?: { - properties?: Record; - required?: string[]; - }; -} +export const McpToolDescriptorSchema = v.looseObject({ + name: v.string(), + description: v.optional(v.string()), + inputSchema: v.optional( + v.looseObject({ + properties: v.optional( + v.record( + v.string(), + v.looseObject({ + type: v.optional(v.string()), + description: v.optional(v.string()), + }) + ) + ), + required: v.optional(v.array(v.string())), + }) + ), +}); +export type McpToolDescriptor = v.InferOutput; From d499509fbf6b80b24d639cbba33e1d412847dfe4 Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 17:31:15 +0200 Subject: [PATCH 12/13] =?UTF-8?q?refactor(cli):=20drop=20the=20version=20c?= =?UTF-8?q?heck=20=E2=80=94=20npx=20storybook=20always=20runs=20the=20proj?= =?UTF-8?q?ect's=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/ai/mcp/installed-check.test.ts | 50 ++++++++ .../src/ai/mcp/installed-check.ts | 14 +++ .../src/ai/mcp/intercepts.test.ts | 8 -- .../cli-storybook/src/ai/mcp/intercepts.ts | 15 +-- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 44 ++----- code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 41 ++----- code/lib/cli-storybook/src/ai/mcp/types.ts | 3 +- .../src/ai/mcp/version-check.test.ts | 111 ------------------ .../cli-storybook/src/ai/mcp/version-check.ts | 55 --------- 9 files changed, 82 insertions(+), 259 deletions(-) create mode 100644 code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts create mode 100644 code/lib/cli-storybook/src/ai/mcp/installed-check.ts delete mode 100644 code/lib/cli-storybook/src/ai/mcp/version-check.test.ts delete mode 100644 code/lib/cli-storybook/src/ai/mcp/version-check.ts diff --git a/code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts b/code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts new file mode 100644 index 000000000000..443de966c253 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts @@ -0,0 +1,50 @@ +import { existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; + +import { vol } from 'memfs'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { isStorybookInstalled } from './installed-check.ts'; + +vi.mock('node:module', { spy: true }); + +// Spy-only mock: redirect the probe's existsSync to memfs. +vi.mock('node:fs', { spy: true }); + +function mockSearchPaths(paths: string[]) { + vi.mocked(createRequire).mockReturnValue({ + resolve: Object.assign(() => '', { paths: () => paths }), + } as unknown as ReturnType); +} + +beforeEach(async () => { + const memfs = await vi.importActual('memfs'); + vi.mocked(existsSync).mockImplementation( + memfs.fs.existsSync as unknown as typeof import('node:fs').existsSync + ); +}); + +afterEach(() => { + vol.reset(); + vi.mocked(createRequire).mockReset(); +}); + +describe('isStorybookInstalled', () => { + it('finds storybook in the nearest node_modules', () => { + mockSearchPaths(['/proj/node_modules', '/node_modules']); + vol.fromNestedJSON({ '/proj/node_modules/storybook/package.json': '{}' }); + expect(isStorybookInstalled('/proj')).toBe(true); + }); + + it('walks up the search paths (monorepo hoisting)', () => { + mockSearchPaths(['/repo/packages/app/node_modules', '/repo/node_modules']); + vol.fromNestedJSON({ '/repo/node_modules/storybook/package.json': '{}' }); + expect(isStorybookInstalled('/repo/packages/app')).toBe(true); + }); + + it('returns false when storybook is nowhere on the search paths', () => { + mockSearchPaths(['/proj/node_modules']); + vol.fromNestedJSON({ '/proj/node_modules': {} }); + expect(isStorybookInstalled('/proj')).toBe(false); + }); +}); diff --git a/code/lib/cli-storybook/src/ai/mcp/installed-check.ts b/code/lib/cli-storybook/src/ai/mcp/installed-check.ts new file mode 100644 index 000000000000..53412c973d89 --- /dev/null +++ b/code/lib/cli-storybook/src/ai/mcp/installed-check.ts @@ -0,0 +1,14 @@ +import { existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { join } from 'node:path'; + +/** + * Whether `storybook` is resolvable from `cwd`, used to pick between the "start storybook dev" + * and "set up Storybook first" repairs when nothing is running. Deliberately not a version check: + * the CLI is invoked as `npx storybook`, so it always runs the project's own Storybook version. + */ +export function isStorybookInstalled(cwd: string): boolean { + const requireFromCwd = createRequire(join(cwd, 'package.json')); + const searchPaths = requireFromCwd.resolve.paths('storybook') ?? []; + return searchPaths.some((base) => existsSync(join(base, 'storybook', 'package.json'))); +} diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts index 0a308d30eb4b..5c4d356f5896 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts @@ -39,14 +39,6 @@ describe('getInterceptMarkdown', () => { expect(markdown).toContain('omit `--port`'); }); - it('storybook-too-old names the detected and minimum versions and the upgrade skill', () => { - const markdown = getInterceptMarkdown('storybook-too-old', { version: '9.0.5' }); - expect(markdown).toContain('`9.0.5`'); - expect(markdown).toContain('`10.5.0`'); - expect(markdown).toContain('storybook-upgrade'); - expect(markdown).toContain('npx storybook add @storybook/addon-mcp'); - }); - it('storybook-not-installed points at the init skill and the addon install', () => { const markdown = getInterceptMarkdown('storybook-not-installed'); expect(markdown).toContain('storybook-init'); diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts index 5bb48f038270..4a3c2359aa1c 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts @@ -1,5 +1,4 @@ import type { InterceptReason, StorybookInstanceRecord } from './types.ts'; -import { STORYBOOK_MIN_VERSION } from './version-check.ts'; /** * Repair-instruction markdown for agents, mirroring `@storybook/mcp-proxy` (storybookjs/mcp) so @@ -19,15 +18,6 @@ const buildPortMismatch = (port: number | undefined, records: StorybookInstanceR Running Storybooks at this cwd: ${records.map((r) => `- port \`${r.port}\` (${r.url}, status: \`${r.mcp.status}\`)`).join('\n')}`; -const buildStorybookTooOld = (version: string) => - `The Storybook installed at this cwd is version \`${version}\`, but this command requires \`${STORYBOOK_MIN_VERSION}\` or newer. - -Ask the user whether they want to upgrade Storybook. If they agree, invoke the \`storybook-upgrade\` skill to perform the upgrade, then run: -\`\`\` -npx storybook add @storybook/addon-mcp -\`\`\` -to install the addon that provides these commands. Restart Storybook, then retry the command.`; - const STORYBOOK_NOT_INSTALLED = `No Storybook is running at this cwd, and Storybook does not appear to be installed here (\`storybook\` could not be resolved from this project). Ask the user whether they want to add Storybook. If they agree, invoke the \`storybook-init\` skill to set it up, then install the addon that provides these commands: @@ -53,7 +43,6 @@ const MCP_ERROR = `Storybook is running but its command server reported an error export type InterceptExtras = { records?: StorybookInstanceRecord[]; - version?: string; port?: number; }; @@ -61,7 +50,7 @@ export function getInterceptMarkdown( reason: InterceptReason, extras: InterceptExtras = {} ): string { - const { records, version, port } = extras; + const { records, port } = extras; switch (reason) { case 'no-instance': return records && records.length > 0 @@ -77,8 +66,6 @@ export function getInterceptMarkdown( return MCP_STARTING; case 'mcp-error': return MCP_ERROR; - case 'storybook-too-old': - return buildStorybookTooOld(version ?? 'unknown'); default: { const unhandled: never = reason; throw new Error(`Unhandled intercept reason: ${unhandled as string}`); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index f75724b3e657..238cc8b9bd57 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -1,14 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; +import { isStorybookInstalled } from './installed-check.ts'; import { readRegistry } from './registry.ts'; import { buildStorybookCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; import type { StorybookInstanceRecord } from './types.ts'; -import { checkStorybookVersion } from './version-check.ts'; vi.mock('./registry.ts', { spy: true }); vi.mock('./client.ts', { spy: true }); -vi.mock('./version-check.ts', { spy: true }); +vi.mock('./installed-check.ts', { spy: true }); const record: StorybookInstanceRecord = { schemaVersion: 1, @@ -22,7 +22,7 @@ const record: StorybookInstanceRecord = { beforeEach(() => { vi.mocked(readRegistry).mockReset().mockResolvedValue([record]); - vi.mocked(checkStorybookVersion).mockReset().mockReturnValue({ status: 'ok' }); + vi.mocked(isStorybookInstalled).mockReset().mockReturnValue(true); vi.mocked(callMcpTool) .mockReset() .mockResolvedValue({ content: [{ type: 'text', text: 'upstream result' }] }); @@ -79,41 +79,18 @@ describe('runAiTool', () => { }); it('prints the storybook-not-installed repair markdown when nothing runs and storybook is unresolvable', async () => { - vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'not-installed' }); + vi.mocked(isStorybookInstalled).mockReturnValue(false); vi.mocked(readRegistry).mockResolvedValue([]); const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); expect(result.exitCode).toBe(1); expect(result.output).toContain('storybook-init'); }); - it('still forwards to a running instance when the version check reports not-installed (monorepo false negative)', async () => { - vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'not-installed' }); - const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); - expect(result).toEqual({ exitCode: 0, output: 'upstream result' }); - }); - - it('prints the storybook-too-old repair markdown when the disk version is too old', async () => { - vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'too-old', version: '9.0.5' }); - const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); - expect(result.exitCode).toBe(1); - expect(result.output).toContain('`9.0.5`'); - }); - - it('prefers the version reported in the registry record over the disk lookup', async () => { - vi.mocked(readRegistry).mockResolvedValue([{ ...record, storybookVersion: '9.0.5' }]); - const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); - expect(result.exitCode).toBe(1); - expect(result.output).toContain('`9.0.5`'); - expect(checkStorybookVersion).not.toHaveBeenCalled(); - }); - - it('forwards to a canary instance even when the disk lookup reports too-old', async () => { - vi.mocked(readRegistry).mockResolvedValue([ - { ...record, storybookVersion: '0.0.0-pr-1-sha-abc' }, - ]); - vi.mocked(checkStorybookVersion).mockReturnValue({ status: 'too-old', version: '9.0.5' }); + it('still forwards to a running instance even when storybook is not resolvable from the cwd (monorepo false negative)', async () => { + vi.mocked(isStorybookInstalled).mockReturnValue(false); const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); expect(result).toEqual({ exitCode: 0, output: 'upstream result' }); + expect(isStorybookInstalled).not.toHaveBeenCalled(); }); it('routes to the instance on the requested --port when several share the cwd', async () => { @@ -316,13 +293,6 @@ describe('buildStorybookCommandsHelp', () => { ); }); - it('names the detected version in the too-old note', async () => { - vi.mocked(readRegistry).mockResolvedValue([{ ...record, storybookVersion: '9.0.5' }]); - const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); - expect(section).toContain('version `9.0.5`'); - expect(section).toContain('require `10.5.0` or newer'); - }); - it('degrades to a note when the MCP server is unreachable', async () => { vi.mocked(listMcpTools).mockRejectedValue(new Error('connection refused')); const section = await buildStorybookCommandsHelp({ cwd: '/projects/foo' }); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index 24fc269dd6ad..1d804ba6fd0f 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -1,6 +1,7 @@ import { resolve } from 'node:path'; import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; +import { isStorybookInstalled } from './installed-check.ts'; import { getInterceptMarkdown } from './intercepts.ts'; import { readRegistry } from './registry.ts'; import { resolveInstance } from './resolve-instance.ts'; @@ -11,11 +12,6 @@ import type { StorybookInstanceRecord, ToolCallResult, } from './types.ts'; -import { - STORYBOOK_MIN_VERSION, - checkStorybookVersion, - classifyStorybookVersion, -} from './version-check.ts'; export type AiToolRunResult = { exitCode: 0 | 1; output: string }; @@ -36,8 +32,8 @@ export type AiToolOptions = { /** * Run a single MCP tool against the Storybook running at the target cwd and return its result as - * markdown. Intercept conditions (no running instance, addon missing, version too old, ...) return - * the same repair-instruction markdown as `@storybook/mcp-proxy`, with exit code 1. + * markdown. Intercept conditions (no running instance, addon missing, ...) return the same + * repair-instruction markdown as `@storybook/mcp-proxy`, with exit code 1. */ export async function runAiTool( toolName: string, @@ -165,8 +161,6 @@ function helpUnavailableNote( return 'the running Storybook does not provide commands — install `@storybook/addon-mcp`'; case 'mcp-error': return "the running Storybook's command server reported an error"; - case 'storybook-too-old': - return `the installed Storybook is version \`${error.version ?? 'unknown'}\`, but these commands require \`${STORYBOOK_MIN_VERSION}\` or newer`; default: { const unhandled: never = error.reason; throw new Error(`Unhandled intercept reason: ${unhandled as string}`); @@ -226,14 +220,13 @@ type InstanceResolution = output: string; reason: InterceptReason; records: StorybookInstanceRecord[]; - /** Detected Storybook version, set for the `storybook-too-old` reason. */ - version?: string; }; /** - * Resolve the running Storybook instance for `cwdInput`, mirroring the mcp-proxy dispatch order: - * registry lookup and cwd/port matching first, then the version check — preferring the version the - * running instance reported in its registry record over resolving the installed version from disk. + * Resolve the running Storybook instance for `cwdInput` via the registry. No version check: the + * CLI is invoked as `npx storybook`, so it always runs the project's own Storybook version. Only + * when nothing is running anywhere do we probe whether Storybook is installed at all, to pick + * between the "start storybook dev" and "set up Storybook first" repairs. */ async function resolveReadyInstance( cwdInput: string | undefined, @@ -245,27 +238,11 @@ async function resolveReadyInstance( const records = await readRegistry(deps.registryDir); const resolution = resolveInstance(records, cwd, port); - const matchedRecord = resolution.kind === 'instance' ? resolution.record : resolution.matches[0]; - const versionStatus = - matchedRecord?.storybookVersion !== undefined - ? classifyStorybookVersion(matchedRecord.storybookVersion) - : checkStorybookVersion(cwd); - - if (versionStatus.status === 'too-old') { - return { - kind: 'error', - output: getInterceptMarkdown('storybook-too-old', { version: versionStatus.version }), - reason: 'storybook-too-old', - records: [], - version: versionStatus.version, - }; - } - if (resolution.kind === 'intercept') { if ( resolution.reason === 'no-instance' && - versionStatus.status === 'not-installed' && - (resolution.records?.length ?? 0) === 0 + (resolution.records?.length ?? 0) === 0 && + !isStorybookInstalled(cwd) ) { return { kind: 'error', diff --git a/code/lib/cli-storybook/src/ai/mcp/types.ts b/code/lib/cli-storybook/src/ai/mcp/types.ts index 9c5a3d7a3fcf..8596d312e4c4 100644 --- a/code/lib/cli-storybook/src/ai/mcp/types.ts +++ b/code/lib/cli-storybook/src/ai/mcp/types.ts @@ -42,8 +42,7 @@ export type InterceptReason = | 'storybook-not-installed' | 'addon-missing' | 'mcp-starting' - | 'mcp-error' - | 'storybook-too-old'; + | 'mcp-error'; /** * Result of an MCP `tools/call` request, as returned by `@storybook/addon-mcp`. Loose: servers may diff --git a/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts b/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts deleted file mode 100644 index 10786e0d032f..000000000000 --- a/code/lib/cli-storybook/src/ai/mcp/version-check.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { readFileSync } from 'node:fs'; -import { createRequire } from 'node:module'; - -import { vol } from 'memfs'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -import { - STORYBOOK_MIN_VERSION, - checkStorybookVersion, - classifyStorybookVersion, -} from './version-check.ts'; - -vi.mock('node:module', { spy: true }); - -// Spy-only mock: redirect the version reader's readFileSync to memfs. -vi.mock('node:fs', { spy: true }); - -describe('classifyStorybookVersion (pure)', () => { - it('returns ok for the minimum version', () => { - expect(classifyStorybookVersion(STORYBOOK_MIN_VERSION)).toEqual({ status: 'ok' }); - }); - - it('returns ok for a stable release above the minimum', () => { - expect(classifyStorybookVersion('10.6.2')).toEqual({ status: 'ok' }); - }); - - it('accepts any prerelease of the minimum (alpha/beta/rc)', () => { - expect(classifyStorybookVersion('10.5.0-alpha.3')).toEqual({ status: 'ok' }); - }); - - it('returns too-old for a version below the floor', () => { - expect(classifyStorybookVersion('9.1.16')).toEqual({ status: 'too-old', version: '9.1.16' }); - }); - - it('treats a prerelease of an earlier version as too-old', () => { - expect(classifyStorybookVersion('10.4.0-rc.1')).toEqual({ - status: 'too-old', - version: '10.4.0-rc.1', - }); - }); - - it('returns not-installed for undefined', () => { - expect(classifyStorybookVersion(undefined)).toEqual({ status: 'not-installed' }); - }); - - it('treats 0.0.0-... versions as ok (canary)', () => { - expect(classifyStorybookVersion('0.0.0-canary.1234')).toEqual({ status: 'ok' }); - }); -}); - -describe('checkStorybookVersion (disk lookup)', () => { - function mockSearchPaths(paths: string[]) { - vi.mocked(createRequire).mockReturnValue({ - resolve: Object.assign(() => '', { paths: () => paths }), - } as unknown as ReturnType); - } - - beforeEach(async () => { - const memfs = await vi.importActual('memfs'); - vi.mocked(readFileSync).mockImplementation( - memfs.fs.readFileSync as unknown as typeof import('node:fs').readFileSync - ); - }); - - afterEach(() => { - vol.reset(); - vi.mocked(createRequire).mockReset(); - }); - - it('reads the version from the first search path holding a storybook package', () => { - mockSearchPaths(['/proj/node_modules', '/node_modules']); - vol.fromNestedJSON({ - '/proj/node_modules/storybook/package.json': JSON.stringify({ version: '10.5.0' }), - }); - expect(checkStorybookVersion('/proj')).toEqual({ status: 'ok' }); - }); - - it('walks up the search paths until storybook resolves (monorepo hoisting)', () => { - mockSearchPaths(['/repo/packages/app/node_modules', '/repo/node_modules']); - vol.fromNestedJSON({ - '/repo/node_modules/storybook/package.json': JSON.stringify({ version: '9.1.16' }), - }); - expect(checkStorybookVersion('/repo/packages/app')).toEqual({ - status: 'too-old', - version: '9.1.16', - }); - }); - - it('returns not-installed when no search path holds storybook', () => { - mockSearchPaths(['/proj/node_modules']); - vol.fromNestedJSON({ '/proj/node_modules': {} }); - expect(checkStorybookVersion('/proj')).toEqual({ status: 'not-installed' }); - }); - - it('skips malformed package.json files and keeps searching', () => { - mockSearchPaths(['/a/node_modules', '/b/node_modules']); - vol.fromNestedJSON({ - '/a/node_modules/storybook/package.json': '{ not json', - '/b/node_modules/storybook/package.json': JSON.stringify({ version: '10.5.0' }), - }); - expect(checkStorybookVersion('/a')).toEqual({ status: 'ok' }); - }); - - it('treats a package.json without a string version as not-installed', () => { - mockSearchPaths(['/proj/node_modules']); - vol.fromNestedJSON({ - '/proj/node_modules/storybook/package.json': JSON.stringify({ name: 'storybook' }), - }); - expect(checkStorybookVersion('/proj')).toEqual({ status: 'not-installed' }); - }); -}); diff --git a/code/lib/cli-storybook/src/ai/mcp/version-check.ts b/code/lib/cli-storybook/src/ai/mcp/version-check.ts deleted file mode 100644 index 44f3185b4288..000000000000 --- a/code/lib/cli-storybook/src/ai/mcp/version-check.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { readFileSync } from 'node:fs'; -import { createRequire } from 'node:module'; -import { join } from 'node:path'; - -import { lt } from 'semver'; - -/** Minimum Storybook version whose `storybook dev` registers itself for MCP discovery. */ -export const STORYBOOK_MIN_VERSION = '10.5.0'; - -/** - * Comparison floor for the minimum version. The `-0` prerelease suffix is the lowest possible - * 10.5.0 build, so every 10.5.0 prerelease (alpha/beta/rc) is accepted alongside the stable - * release, while anything below 10.5.0 is rejected. - */ -const STORYBOOK_MIN_VERSION_FLOOR = '10.5.0-0'; - -export type StorybookVersionStatus = - | { status: 'ok' } - | { status: 'too-old'; version: string } - | { status: 'not-installed' }; - -export function classifyStorybookVersion(version: string | undefined): StorybookVersionStatus { - if (version === undefined) { - return { status: 'not-installed' }; - } - // Storybook canary releases are published as `0.0.0-*`; semver considers these < any stable - // version, but we still want to treat them as supported. - if (version.startsWith('0.0.0-')) { - return { status: 'ok' }; - } - return lt(version, STORYBOOK_MIN_VERSION_FLOOR) - ? { status: 'too-old', version } - : { status: 'ok' }; -} - -export function checkStorybookVersion(cwd: string): StorybookVersionStatus { - return classifyStorybookVersion(readStorybookVersion(cwd)); -} - -function readStorybookVersion(cwd: string): string | undefined { - const requireFromCwd = createRequire(join(cwd, 'package.json')); - // require.resolve.paths is the only way to get the actual search paths without going through - // Node's module-resolution cache. - const searchPaths = requireFromCwd.resolve.paths('storybook') ?? []; - for (const base of searchPaths) { - try { - const raw = readFileSync(join(base, 'storybook', 'package.json'), 'utf8'); - const { version } = JSON.parse(raw) as { version?: unknown }; - return typeof version === 'string' ? version : undefined; - } catch { - // parse error or file not found. - } - } - return undefined; -} From 484b30b847142a09088599ef6d8e2e73b970d56f Mon Sep 17 00:00:00 2001 From: Kasper Peulen Date: Wed, 10 Jun 2026 17:40:24 +0200 Subject: [PATCH 13/13] refactor(cli): drop the installed-check probe too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/ai/mcp/installed-check.test.ts | 50 ------------------- .../src/ai/mcp/installed-check.ts | 14 ------ .../src/ai/mcp/intercepts.test.ts | 6 --- .../cli-storybook/src/ai/mcp/intercepts.ts | 12 ----- .../cli-storybook/src/ai/mcp/run-tool.test.ts | 18 ------- code/lib/cli-storybook/src/ai/mcp/run-tool.ts | 21 ++------ code/lib/cli-storybook/src/ai/mcp/types.ts | 1 - 7 files changed, 3 insertions(+), 119 deletions(-) delete mode 100644 code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts delete mode 100644 code/lib/cli-storybook/src/ai/mcp/installed-check.ts diff --git a/code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts b/code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts deleted file mode 100644 index 443de966c253..000000000000 --- a/code/lib/cli-storybook/src/ai/mcp/installed-check.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { existsSync } from 'node:fs'; -import { createRequire } from 'node:module'; - -import { vol } from 'memfs'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -import { isStorybookInstalled } from './installed-check.ts'; - -vi.mock('node:module', { spy: true }); - -// Spy-only mock: redirect the probe's existsSync to memfs. -vi.mock('node:fs', { spy: true }); - -function mockSearchPaths(paths: string[]) { - vi.mocked(createRequire).mockReturnValue({ - resolve: Object.assign(() => '', { paths: () => paths }), - } as unknown as ReturnType); -} - -beforeEach(async () => { - const memfs = await vi.importActual('memfs'); - vi.mocked(existsSync).mockImplementation( - memfs.fs.existsSync as unknown as typeof import('node:fs').existsSync - ); -}); - -afterEach(() => { - vol.reset(); - vi.mocked(createRequire).mockReset(); -}); - -describe('isStorybookInstalled', () => { - it('finds storybook in the nearest node_modules', () => { - mockSearchPaths(['/proj/node_modules', '/node_modules']); - vol.fromNestedJSON({ '/proj/node_modules/storybook/package.json': '{}' }); - expect(isStorybookInstalled('/proj')).toBe(true); - }); - - it('walks up the search paths (monorepo hoisting)', () => { - mockSearchPaths(['/repo/packages/app/node_modules', '/repo/node_modules']); - vol.fromNestedJSON({ '/repo/node_modules/storybook/package.json': '{}' }); - expect(isStorybookInstalled('/repo/packages/app')).toBe(true); - }); - - it('returns false when storybook is nowhere on the search paths', () => { - mockSearchPaths(['/proj/node_modules']); - vol.fromNestedJSON({ '/proj/node_modules': {} }); - expect(isStorybookInstalled('/proj')).toBe(false); - }); -}); diff --git a/code/lib/cli-storybook/src/ai/mcp/installed-check.ts b/code/lib/cli-storybook/src/ai/mcp/installed-check.ts deleted file mode 100644 index 53412c973d89..000000000000 --- a/code/lib/cli-storybook/src/ai/mcp/installed-check.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { existsSync } from 'node:fs'; -import { createRequire } from 'node:module'; -import { join } from 'node:path'; - -/** - * Whether `storybook` is resolvable from `cwd`, used to pick between the "start storybook dev" - * and "set up Storybook first" repairs when nothing is running. Deliberately not a version check: - * the CLI is invoked as `npx storybook`, so it always runs the project's own Storybook version. - */ -export function isStorybookInstalled(cwd: string): boolean { - const requireFromCwd = createRequire(join(cwd, 'package.json')); - const searchPaths = requireFromCwd.resolve.paths('storybook') ?? []; - return searchPaths.some((base) => existsSync(join(base, 'storybook', 'package.json'))); -} diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts index 5c4d356f5896..df444e6efcd6 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.test.ts @@ -39,12 +39,6 @@ describe('getInterceptMarkdown', () => { expect(markdown).toContain('omit `--port`'); }); - it('storybook-not-installed points at the init skill and the addon install', () => { - const markdown = getInterceptMarkdown('storybook-not-installed'); - expect(markdown).toContain('storybook-init'); - expect(markdown).toContain('npx storybook add @storybook/addon-mcp'); - }); - it('addon-missing instructs installing the MCP addon', () => { const markdown = getInterceptMarkdown('addon-missing'); expect(markdown).toContain('`@storybook/addon-mcp` addon is missing'); diff --git a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts index 4a3c2359aa1c..3390445461c0 100644 --- a/code/lib/cli-storybook/src/ai/mcp/intercepts.ts +++ b/code/lib/cli-storybook/src/ai/mcp/intercepts.ts @@ -18,16 +18,6 @@ const buildPortMismatch = (port: number | undefined, records: StorybookInstanceR Running Storybooks at this cwd: ${records.map((r) => `- port \`${r.port}\` (${r.url}, status: \`${r.mcp.status}\`)`).join('\n')}`; -const STORYBOOK_NOT_INSTALLED = `No Storybook is running at this cwd, and Storybook does not appear to be installed here (\`storybook\` could not be resolved from this project). - -Ask the user whether they want to add Storybook. If they agree, invoke the \`storybook-init\` skill to set it up, then install the addon that provides these commands: -\`\`\` -npx storybook add @storybook/addon-mcp -\`\`\` -Start Storybook, then retry the command. - -If you believe Storybook is in fact installed (e.g. a monorepo where \`storybook\` resolves from a different location), start \`storybook dev\` from this exact cwd and retry — a running instance is always used regardless of this check.`; - const ADDON_MISSING = `Storybook is running but does not provide these commands. The \`@storybook/addon-mcp\` addon is missing. Install it: @@ -58,8 +48,6 @@ export function getInterceptMarkdown( : NO_INSTANCE_EMPTY; case 'port-mismatch': return buildPortMismatch(port, records ?? []); - case 'storybook-not-installed': - return STORYBOOK_NOT_INSTALLED; case 'addon-missing': return ADDON_MISSING; case 'mcp-starting': diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts index 238cc8b9bd57..6fce644ccd97 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.test.ts @@ -1,14 +1,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; -import { isStorybookInstalled } from './installed-check.ts'; import { readRegistry } from './registry.ts'; import { buildStorybookCommandsHelp, runAiTool, runAiToolHelp } from './run-tool.ts'; import type { StorybookInstanceRecord } from './types.ts'; vi.mock('./registry.ts', { spy: true }); vi.mock('./client.ts', { spy: true }); -vi.mock('./installed-check.ts', { spy: true }); const record: StorybookInstanceRecord = { schemaVersion: 1, @@ -22,7 +20,6 @@ const record: StorybookInstanceRecord = { beforeEach(() => { vi.mocked(readRegistry).mockReset().mockResolvedValue([record]); - vi.mocked(isStorybookInstalled).mockReset().mockReturnValue(true); vi.mocked(callMcpTool) .mockReset() .mockResolvedValue({ content: [{ type: 'text', text: 'upstream result' }] }); @@ -78,21 +75,6 @@ describe('runAiTool', () => { expect(result.output).toContain('/projects/foo'); }); - it('prints the storybook-not-installed repair markdown when nothing runs and storybook is unresolvable', async () => { - vi.mocked(isStorybookInstalled).mockReturnValue(false); - vi.mocked(readRegistry).mockResolvedValue([]); - const result = await runAiTool('get-documentation', [], { cwd: '/projects/foo' }); - expect(result.exitCode).toBe(1); - expect(result.output).toContain('storybook-init'); - }); - - it('still forwards to a running instance even when storybook is not resolvable from the cwd (monorepo false negative)', async () => { - vi.mocked(isStorybookInstalled).mockReturnValue(false); - const result = await runAiTool('list-all-documentation', [], { cwd: '/projects/foo' }); - expect(result).toEqual({ exitCode: 0, output: 'upstream result' }); - expect(isStorybookInstalled).not.toHaveBeenCalled(); - }); - it('routes to the instance on the requested --port when several share the cwd', async () => { const onOtherPort = { ...record, instanceId: 'inst-2', pid: 2, port: 6007 }; vi.mocked(readRegistry).mockResolvedValue([record, onOtherPort]); diff --git a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts index 1d804ba6fd0f..596834ece95d 100644 --- a/code/lib/cli-storybook/src/ai/mcp/run-tool.ts +++ b/code/lib/cli-storybook/src/ai/mcp/run-tool.ts @@ -1,7 +1,6 @@ import { resolve } from 'node:path'; import { McpJsonRpcError, callMcpTool, listMcpTools } from './client.ts'; -import { isStorybookInstalled } from './installed-check.ts'; import { getInterceptMarkdown } from './intercepts.ts'; import { readRegistry } from './registry.ts'; import { resolveInstance } from './resolve-instance.ts'; @@ -149,7 +148,6 @@ function helpUnavailableNote( ): string { switch (error.reason) { case 'no-instance': - case 'storybook-not-installed': return 'no running Storybook detected at this cwd; start `storybook dev` to list its commands'; case 'port-mismatch': return `no instance on port \`${port}\` at this cwd — running ports: ${error.records @@ -223,10 +221,9 @@ type InstanceResolution = }; /** - * Resolve the running Storybook instance for `cwdInput` via the registry. No version check: the - * CLI is invoked as `npx storybook`, so it always runs the project's own Storybook version. Only - * when nothing is running anywhere do we probe whether Storybook is installed at all, to pick - * between the "start storybook dev" and "set up Storybook first" repairs. + * Resolve the running Storybook instance for `cwdInput` via the registry. No version or + * installed checks: the CLI is invoked as `npx storybook`, so the fact that it is executing + * already proves the project has a compatible Storybook. */ async function resolveReadyInstance( cwdInput: string | undefined, @@ -239,18 +236,6 @@ async function resolveReadyInstance( const resolution = resolveInstance(records, cwd, port); if (resolution.kind === 'intercept') { - if ( - resolution.reason === 'no-instance' && - (resolution.records?.length ?? 0) === 0 && - !isStorybookInstalled(cwd) - ) { - return { - kind: 'error', - output: getInterceptMarkdown('storybook-not-installed'), - reason: 'storybook-not-installed', - records: [], - }; - } return { kind: 'error', output: getInterceptMarkdown(resolution.reason, { records: resolution.records, port }), diff --git a/code/lib/cli-storybook/src/ai/mcp/types.ts b/code/lib/cli-storybook/src/ai/mcp/types.ts index 8596d312e4c4..fc6329479128 100644 --- a/code/lib/cli-storybook/src/ai/mcp/types.ts +++ b/code/lib/cli-storybook/src/ai/mcp/types.ts @@ -39,7 +39,6 @@ export type StorybookInstanceRecord = v.InferOutput