From fc2e0ef1199c15800e81d1123afd751684c5cccd Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 19 May 2026 17:38:08 +0200 Subject: [PATCH 01/26] feat: add apply-review-state tool for agentic review The agent pushes a curated review (narrative, labelled clusters with optional semantic kind, changed files, diff hunks, optional per-story depth/chain) via this MCP tool. It validates with valibot, caches the state in memory, broadcasts it over the Storybook channel, and returns the review-page URL. Gated behind FEATURES.changeDetection. Pairs with the @storybook/addon-review-changes Storybook addon. --- .changeset/apply-review-state.md | 5 + packages/addon-mcp/src/constants.ts | 10 ++ packages/addon-mcp/src/mcp-handler.ts | 2 + packages/addon-mcp/src/preset.ts | 13 +++ packages/addon-mcp/src/review-state-store.ts | 92 +++++++++++++++++++ .../addon-mcp/src/tools/apply-review-state.ts | 68 ++++++++++++++ packages/addon-mcp/src/tools/tool-names.ts | 1 + 7 files changed, 191 insertions(+) create mode 100644 .changeset/apply-review-state.md create mode 100644 packages/addon-mcp/src/review-state-store.ts create mode 100644 packages/addon-mcp/src/tools/apply-review-state.ts diff --git a/.changeset/apply-review-state.md b/.changeset/apply-review-state.md new file mode 100644 index 00000000..c2cf4f10 --- /dev/null +++ b/.changeset/apply-review-state.md @@ -0,0 +1,5 @@ +--- +"@storybook/addon-mcp": minor +--- + +Added the `apply-review-state` tool. The agent pushes a curated review of current changes and returns the review-page URL. Pairs with the `@storybook/addon-review-changes` Storybook addon. diff --git a/packages/addon-mcp/src/constants.ts b/packages/addon-mcp/src/constants.ts index 05fc627b..d4b39208 100644 --- a/packages/addon-mcp/src/constants.ts +++ b/packages/addon-mcp/src/constants.ts @@ -1,2 +1,12 @@ export const MCP_APP_PARAM = 'mcp-app'; export const MCP_APP_SIZE_CHANGED_EVENT = 'storybook-mcp:size-changed'; + +/** + * Channel events shared with `@storybook/addon-review-changes` (cross-repo + * contract). Keep in sync with that addon's `src/constants.ts`. + */ +export const APPLY_REVIEW_STATE_EVENT = 'storybook/addon-review-changes/apply-review-state'; +export const REQUEST_REVIEW_STATE_EVENT = 'storybook/addon-review-changes/request-review-state'; + +/** Storybook manager route the review page is registered at. */ +export const REVIEW_PAGE_PATH = '/review-changes/'; diff --git a/packages/addon-mcp/src/mcp-handler.ts b/packages/addon-mcp/src/mcp-handler.ts index 0a831a83..b32866ae 100644 --- a/packages/addon-mcp/src/mcp-handler.ts +++ b/packages/addon-mcp/src/mcp-handler.ts @@ -4,6 +4,7 @@ import { HttpTransport } from '@tmcp/transport-http'; import pkgJson from '../package.json' with { type: 'json' }; import { addPreviewStoriesTool } from './tools/preview-stories.ts'; import { addGetChangedStoriesTool } from './tools/get-changed-stories.ts'; +import { addApplyReviewStateTool } from './tools/apply-review-state.ts'; import { addGetUIBuildingInstructionsTool } from './tools/get-storybook-story-instructions.ts'; import { addListAllDocumentationTool, @@ -81,6 +82,7 @@ const initializeMCPServer = async (options: Options, multiSource?: boolean) => { if (changeDetectionEnabled) { await addGetChangedStoriesTool(server); + await addApplyReviewStateTool(server); } // Register test addon tools diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index c30e13cb..86072ac0 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -10,6 +10,8 @@ import path from 'node:path'; import { CompositionAuth, extractBearerToken, type ComposedRef } from './auth/index.ts'; import { logger } from 'storybook/internal/node-logger'; import type { Source } from '@storybook/mcp'; +import { APPLY_REVIEW_STATE_EVENT, REQUEST_REVIEW_STATE_EVENT } from './constants.ts'; +import { getReviewState } from './review-state-store.ts'; export const previewAnnotations: PresetPropertyFn<'previewAnnotations'> = async ( existingAnnotations = [], @@ -29,6 +31,17 @@ export const experimental_devServer: PresetPropertyFn<'experimental_devServer'> const origin = `http://localhost:${options.port}`; + // Replay the cached review state to any Storybook tab that mounts or + // refreshes after the agent already pushed. The tab emits + // `request-review-state`; we re-emit the cached state so it converges. + // The cache is the module singleton shared with the apply-review-state tool. + options.channel.on(REQUEST_REVIEW_STATE_EVENT, () => { + const state = getReviewState(); + if (state) { + options.channel.emit(APPLY_REVIEW_STATE_EVENT, state); + } + }); + // Get composed Storybook refs from config const refs = await getRefsFromConfig(options); const compositionAuth = new CompositionAuth(); diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts new file mode 100644 index 00000000..3c619be0 --- /dev/null +++ b/packages/addon-mcp/src/review-state-store.ts @@ -0,0 +1,92 @@ +/** + * In-memory store for the agent-pushed review state. + * + * The agent pushes a review via the `apply-review-state` MCP tool; the last + * one is cached here in module scope and replayed over the Storybook channel + * to any tab that asks (the `request-review-state` event). A dev-server + * restart clears it — server memory only, by design. + * + * This module owns the canonical valibot schema for the review contract; + * `@storybook/addon-review-changes` duplicates the inferred TS shape on its + * side (it only renders, so it needs the type, not the validator). + */ +import * as v from 'valibot'; + +export const ReviewClusterSchema = v.object({ + label: v.pipe( + v.string(), + v.description('Short human-readable cluster name, e.g. "Direct Button importers".'), + ), + rationale: v.pipe( + v.string(), + v.description('One sentence explaining why these stories are grouped together.'), + ), + sampleStoryIds: v.pipe( + v.array(v.string()), + v.description( + 'Story IDs that represent this cluster (e.g. "button--primary"). The page renders exactly these.', + ), + ), + kind: v.pipe( + v.optional(v.picklist(['atomic', 'consumer', 'transitive', 'catch-all'])), + v.description( + 'Semantic role of this cluster in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.', + ), + ), +}); + +const StoryMetaSchema = v.object({ + depth: v.pipe( + v.optional(v.number()), + v.description( + 'Graph distance from the changed file(s) to this story (0 = the changed component itself).', + ), + ), + chain: v.pipe( + v.optional(v.array(v.string())), + v.description( + 'Ordered intermediate file paths between the story file and the changed file, excluding both endpoints. Empty/omitted means a direct import.', + ), + ), +}); + +const DiffHunkSchema = v.object({ + path: v.pipe(v.string(), v.description('Path of the changed file this hunk belongs to.')), + hunk: v.pipe( + v.string(), + v.description('Unified-diff text for this hunk (with +/- line prefixes).'), + ), +}); + +export const ReviewStateSchema = v.object({ + narrative: v.pipe( + v.string(), + v.description('One-paragraph overview of what changed and where to start.'), + ), + clusters: v.array(ReviewClusterSchema), + changedFiles: v.pipe( + v.optional(v.array(v.string())), + v.description('Paths of the files you changed, most central first.'), + ), + diffHunks: v.pipe( + v.optional(v.array(DiffHunkSchema)), + v.description('The actual diff hunks of your change, shown in the review page.'), + ), + storyMeta: v.pipe( + v.optional(v.record(v.string(), StoryMetaSchema)), + v.description('Optional per-story metadata keyed by story ID: { depth, chain }.'), + ), +}); + +export type ReviewCluster = v.InferOutput; +export type ReviewState = v.InferOutput; + +let cached: ReviewState | undefined; + +export function setReviewState(state: ReviewState): void { + cached = state; +} + +export function getReviewState(): ReviewState | undefined { + return cached; +} diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts new file mode 100644 index 00000000..4c03dbeb --- /dev/null +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -0,0 +1,68 @@ +import type { McpServer } from 'tmcp'; +import * as v from 'valibot'; +import type { AddonContext } from '../types.ts'; +import { errorToMCPContent } from '../utils/errors.ts'; +import { ReviewStateSchema, setReviewState, type ReviewState } from '../review-state-store.ts'; +import { APPLY_REVIEW_STATE_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; +import { APPLY_REVIEW_STATE_TOOL_NAME } from './tool-names.ts'; + +const ApplyReviewStateOutput = v.object({ + reviewUrl: v.pipe( + v.string(), + v.description( + 'URL of the Storybook review page. Always include this URL in your final user-facing response so the user can open it directly.', + ), + ), +}); + +export async function addApplyReviewStateTool(server: McpServer) { + server.tool( + { + name: APPLY_REVIEW_STATE_TOOL_NAME, + title: 'Apply Storybook review state', + description: `Push a curated review of the current change to Storybook's review page. + +After you finish a UI code change, call this to help the user spot-check it. Provide: +- narrative: one paragraph on what changed and where to start. +- clusters: labelled groups of representative story IDs, each with a rationale and, when you can tell, a kind ("atomic" for the directly changed component, "consumer" for direct dependents, "transitive" for pages/containers, "catch-all" otherwise). +- changedFiles: the files you edited (most central first). +- diffHunks: the actual diff of your change (you made it — include the hunks). +- storyMeta: optional per-story { depth, chain }. + +Always include the returned reviewUrl in your final user-facing response so the user can open it.`, + schema: ReviewStateSchema, + outputSchema: ApplyReviewStateOutput, + enabled: () => server.ctx.custom?.toolsets?.dev ?? true, + }, + async (input: ReviewState) => { + try { + const { origin } = server.ctx.custom ?? {}; + if (!origin) { + throw new Error('Origin is required in addon context'); + } + + setReviewState(input); + + // Broadcast to all connected Storybook tabs. A warm tab navigates + // to the review page; a cold start relies on the returned URL. + server.ctx.custom?.options?.channel?.emit(APPLY_REVIEW_STATE_EVENT, input); + + const reviewUrl = `${origin}/?path=${REVIEW_PAGE_PATH}`; + const clusterCount = input.clusters.length; + const storyCount = input.clusters.reduce((n, c) => n + c.sampleStoryIds.length, 0); + + return { + content: [ + { + type: 'text' as const, + text: `Review applied: ${clusterCount} cluster${clusterCount === 1 ? '' : 's'}, ${storyCount} stor${storyCount === 1 ? 'y' : 'ies'}. Open it at: ${reviewUrl}`, + }, + ], + structuredContent: { reviewUrl }, + }; + } catch (error) { + return errorToMCPContent(error); + } + }, + ); +} diff --git a/packages/addon-mcp/src/tools/tool-names.ts b/packages/addon-mcp/src/tools/tool-names.ts index 30c9f0d5..809dcbfa 100644 --- a/packages/addon-mcp/src/tools/tool-names.ts +++ b/packages/addon-mcp/src/tools/tool-names.ts @@ -6,3 +6,4 @@ export const PREVIEW_STORIES_TOOL_NAME = 'preview-stories'; export const GET_CHANGED_STORIES_TOOL_NAME = 'get-changed-stories'; export const GET_UI_BUILDING_INSTRUCTIONS_TOOL_NAME = 'get-storybook-story-instructions'; export const RUN_STORY_TESTS_TOOL_NAME = 'run-story-tests'; +export const APPLY_REVIEW_STATE_TOOL_NAME = 'apply-review-state'; From f9727f0eb5db63021f71e44c02129daa73294048 Mon Sep 17 00:00:00 2001 From: yannbf Date: Thu, 21 May 2026 16:00:47 +0200 Subject: [PATCH 02/26] update schema --- packages/addon-mcp/src/preset.ts | 22 ++++++++++++++----- packages/addon-mcp/src/review-state-store.ts | 22 +++++++++++-------- .../addon-mcp/src/tools/apply-review-state.ts | 11 +++++----- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index 86072ac0..5b170e7d 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -35,12 +35,22 @@ export const experimental_devServer: PresetPropertyFn<'experimental_devServer'> // refreshes after the agent already pushed. The tab emits // `request-review-state`; we re-emit the cached state so it converges. // The cache is the module singleton shared with the apply-review-state tool. - options.channel.on(REQUEST_REVIEW_STATE_EVENT, () => { - const state = getReviewState(); - if (state) { - options.channel.emit(APPLY_REVIEW_STATE_EVENT, state); - } - }); + // + // Older Storybooks (≤10.3) don't pass `options.channel` to dev-server + // presets — the rest of addon-mcp still works without channel replay, + // so guard rather than crash. + if (options.channel?.on) { + options.channel.on(REQUEST_REVIEW_STATE_EVENT, () => { + const state = getReviewState(); + if (state) { + options.channel?.emit?.(APPLY_REVIEW_STATE_EVENT, state); + } + }); + } else { + logger.info( + '[addon-mcp] options.channel is unavailable on this Storybook version; review-state replay disabled.', + ); + } // Get composed Storybook refs from config const refs = await getRefsFromConfig(options); diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts index 3c619be0..44b60d26 100644 --- a/packages/addon-mcp/src/review-state-store.ts +++ b/packages/addon-mcp/src/review-state-store.ts @@ -12,10 +12,10 @@ */ import * as v from 'valibot'; -export const ReviewClusterSchema = v.object({ - label: v.pipe( +export const ReviewCollectionSchema = v.object({ + title: v.pipe( v.string(), - v.description('Short human-readable cluster name, e.g. "Direct Button importers".'), + v.description('Short, PR-dense title for this collection, e.g. "Direct Button importers".'), ), rationale: v.pipe( v.string(), @@ -24,13 +24,13 @@ export const ReviewClusterSchema = v.object({ sampleStoryIds: v.pipe( v.array(v.string()), v.description( - 'Story IDs that represent this cluster (e.g. "button--primary"). The page renders exactly these.', + 'Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.', ), ), kind: v.pipe( v.optional(v.picklist(['atomic', 'consumer', 'transitive', 'catch-all'])), v.description( - 'Semantic role of this cluster in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.', + 'Semantic role of this collection in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.', ), ), }); @@ -59,11 +59,15 @@ const DiffHunkSchema = v.object({ }); export const ReviewStateSchema = v.object({ - narrative: v.pipe( + title: v.pipe( v.string(), - v.description('One-paragraph overview of what changed and where to start.'), + v.description('PR-style title for the change — short and specific, e.g. "Recolour the primary button".'), ), - clusters: v.array(ReviewClusterSchema), + description: v.pipe( + v.string(), + v.description('One-line summary of what changed and where to start reviewing.'), + ), + collections: v.array(ReviewCollectionSchema), changedFiles: v.pipe( v.optional(v.array(v.string())), v.description('Paths of the files you changed, most central first.'), @@ -78,7 +82,7 @@ export const ReviewStateSchema = v.object({ ), }); -export type ReviewCluster = v.InferOutput; +export type ReviewCollection = v.InferOutput; export type ReviewState = v.InferOutput; let cached: ReviewState | undefined; diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index 4c03dbeb..bc9b5366 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -23,8 +23,9 @@ export async function addApplyReviewStateTool(server: McpServer n + c.sampleStoryIds.length, 0); + const collectionCount = input.collections.length; + const storyCount = input.collections.reduce((n, c) => n + c.sampleStoryIds.length, 0); return { content: [ { type: 'text' as const, - text: `Review applied: ${clusterCount} cluster${clusterCount === 1 ? '' : 's'}, ${storyCount} stor${storyCount === 1 ? 'y' : 'ies'}. Open it at: ${reviewUrl}`, + text: `Review applied: ${collectionCount} collection${collectionCount === 1 ? '' : 's'}, ${storyCount} stor${storyCount === 1 ? 'y' : 'ies'}. Open it at: ${reviewUrl}`, }, ], structuredContent: { reviewUrl }, From 4e3acfe105caf5fdf4dd42c6a08893f567eb9ff1 Mon Sep 17 00:00:00 2001 From: yannbf Date: Thu, 21 May 2026 16:01:14 +0200 Subject: [PATCH 03/26] add retries to story index fetch --- .../src/utils/fetch-story-index.test.ts | 126 ++++++++++++++++-- .../addon-mcp/src/utils/fetch-story-index.ts | 87 ++++++++++-- 2 files changed, 193 insertions(+), 20 deletions(-) diff --git a/packages/addon-mcp/src/utils/fetch-story-index.test.ts b/packages/addon-mcp/src/utils/fetch-story-index.test.ts index d2abcf3e..671f65a6 100644 --- a/packages/addon-mcp/src/utils/fetch-story-index.test.ts +++ b/packages/addon-mcp/src/utils/fetch-story-index.test.ts @@ -6,7 +6,6 @@ describe('fetchStoryIndex', () => { const originalFetch = global.fetch; beforeEach(() => { - // Mock fetch globally global.fetch = vi.fn(); }); @@ -29,28 +28,113 @@ describe('fetchStoryIndex', () => { expect(Object.keys(result.entries)).toHaveLength(3); }); - it('should throw error on 404 response', async () => { + it('should not retry on 404 (non-transient) and include status + statusText in error', async () => { const mockFetch = global.fetch as any; mockFetch.mockResolvedValue({ ok: false, status: 404, statusText: 'Not Found', + text: async () => '', }); - const origin = 'http://localhost:6006'; + await expect( + fetchStoryIndex('http://localhost:6006', { sleep: vi.fn() }), + ).rejects.toThrow(/Failed to fetch story index: 404 Not Found/); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('should retry transient 5xx responses up to maxAttempts and succeed on a later attempt', async () => { + const mockFetch = global.fetch as any; + mockFetch + .mockResolvedValueOnce({ + ok: false, + status: 500, + statusText: 'Internal Server Error', + text: async () => 'Vite: failed to compile preview.tsx', + }) + .mockResolvedValueOnce({ + ok: false, + status: 503, + statusText: 'Service Unavailable', + text: async () => '', + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => smallStoryIndexFixture, + }); + const sleep = vi.fn().mockResolvedValue(undefined); + + const result = await fetchStoryIndex('http://localhost:6006', { + maxAttempts: 3, + baseBackoffMs: 100, + sleep, + }); + + expect(result).toEqual(smallStoryIndexFixture); + expect(mockFetch).toHaveBeenCalledTimes(3); + // Backoff is `attempt * baseBackoffMs` — first wait 100ms, second wait 200ms. + expect(sleep).toHaveBeenNthCalledWith(1, 100); + expect(sleep).toHaveBeenNthCalledWith(2, 200); + }); - await expect(fetchStoryIndex(origin)).rejects.toThrow( - 'Failed to fetch story index: 404 Not Found', + it('should give up after maxAttempts of transient errors and include body snippet + attempt count in message', async () => { + const mockFetch = global.fetch as any; + mockFetch.mockResolvedValue({ + ok: false, + status: 500, + statusText: 'Internal Server Error', + text: async () => 'Vite: failed to compile preview.tsx (export StoreProvider not found)', + }); + const sleep = vi.fn().mockResolvedValue(undefined); + + await expect( + fetchStoryIndex('http://localhost:6006', { maxAttempts: 3, baseBackoffMs: 50, sleep }), + ).rejects.toThrow( + /500 Internal Server Error — Vite: failed to compile preview\.tsx \(export StoreProvider not found\) \(attempt 3\/3, transient — Storybook may be mid-recompile\)/, ); + expect(mockFetch).toHaveBeenCalledTimes(3); }); - it('should throw error on network failure', async () => { + it('should not retry on 4xx non-transient codes even with high maxAttempts', async () => { const mockFetch = global.fetch as any; - mockFetch.mockRejectedValue(new Error('Network error')); + mockFetch.mockResolvedValue({ + ok: false, + status: 401, + statusText: 'Unauthorized', + text: async () => '', + }); + const sleep = vi.fn(); - const origin = 'http://localhost:6006'; + await expect( + fetchStoryIndex('http://localhost:6006', { maxAttempts: 5, sleep }), + ).rejects.toThrow(/401 Unauthorized.*non-transient — giving up/); + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(sleep).not.toHaveBeenCalled(); + }); + + it('should retry on 429 (too many requests) and 408 (timeout)', async () => { + const mockFetch = global.fetch as any; + mockFetch + .mockResolvedValueOnce({ ok: false, status: 429, statusText: 'Too Many Requests', text: async () => '' }) + .mockResolvedValueOnce({ ok: false, status: 408, statusText: 'Request Timeout', text: async () => '' }) + .mockResolvedValueOnce({ ok: true, json: async () => smallStoryIndexFixture }); + const sleep = vi.fn().mockResolvedValue(undefined); + + const result = await fetchStoryIndex('http://localhost:6006', { + maxAttempts: 3, + baseBackoffMs: 10, + sleep, + }); + expect(result).toEqual(smallStoryIndexFixture); + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + + it('should bubble up network errors (fetch rejection) without retrying', async () => { + const mockFetch = global.fetch as any; + mockFetch.mockRejectedValue(new Error('Network error')); - await expect(fetchStoryIndex(origin)).rejects.toThrow('Network error'); + await expect(fetchStoryIndex('http://localhost:6006')).rejects.toThrow('Network error'); + expect(mockFetch).toHaveBeenCalledTimes(1); }); it('should handle invalid JSON response', async () => { @@ -62,8 +146,28 @@ describe('fetchStoryIndex', () => { }, }); - const origin = 'http://localhost:6006'; + await expect(fetchStoryIndex('http://localhost:6006')).rejects.toThrow('Invalid JSON'); + }); + + it('truncates very long body snippets in the error message', async () => { + const longBody = 'x'.repeat(500); + const mockFetch = global.fetch as any; + mockFetch.mockResolvedValue({ + ok: false, + status: 500, + statusText: 'Internal Server Error', + text: async () => longBody, + }); - await expect(fetchStoryIndex(origin)).rejects.toThrow('Invalid JSON'); + try { + await fetchStoryIndex('http://localhost:6006', { maxAttempts: 1, sleep: vi.fn() }); + throw new Error('expected throw'); + } catch (e) { + const msg = (e as Error).message; + // Snippet should be truncated with an ellipsis. + expect(msg).toMatch(/x{100,200}…/); + // And the full 500 chars must not be in the message. + expect(msg).not.toContain(longBody); + } }); }); diff --git a/packages/addon-mcp/src/utils/fetch-story-index.ts b/packages/addon-mcp/src/utils/fetch-story-index.ts index 50a85bcc..6e59f11e 100644 --- a/packages/addon-mcp/src/utils/fetch-story-index.ts +++ b/packages/addon-mcp/src/utils/fetch-story-index.ts @@ -4,24 +4,93 @@ import { logger } from 'storybook/internal/node-logger'; /** * Fetches the Storybook story index from the running Storybook instance. * + * Retries transient 5xx responses with a short backoff — these are common + * right after a story or preview file is edited because Vite is mid- + * recompile and the dev server hasn't repopulated the index yet. We don't + * want to give up after one shot when waiting a second would have worked. + * + * On failure (after retries), throws an Error whose message includes + * status + statusText + a truncated snippet of the response body so the + * caller has something actionable instead of a bare `500 Internal Server + * Error`. + * * @param origin - The origin URL of the Storybook instance (e.g., http://localhost:6006) + * @param options - Optional retry tuning. Defaults: 3 attempts total, 500ms base backoff. * @returns A promise that resolves to the StoryIndex - * @throws If the fetch fails or returns invalid data + * @throws If the fetch fails or returns invalid data after all retries */ -export async function fetchStoryIndex(origin: string): Promise { +export interface FetchStoryIndexOptions { + /** Total attempts including the first. Default 3. */ + maxAttempts?: number; + /** Base backoff in ms; each attempt waits `attempt * baseBackoffMs`. Default 500. */ + baseBackoffMs?: number; + /** + * Injected for tests so we don't sleep for real. Defaults to setTimeout. + */ + sleep?: (ms: number) => Promise; +} + +const DEFAULT_MAX_ATTEMPTS = 3; +const DEFAULT_BASE_BACKOFF_MS = 500; + +const defaultSleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +function isTransientStatus(status: number): boolean { + // 5xx are usually transient (server mid-recompile, restarting, etc). + // 408 (timeout) and 429 (too many requests) are also worth retrying. + return status >= 500 || status === 408 || status === 429; +} + +async function readBodySnippet(response: Response, maxChars = 200): Promise { + try { + const text = await response.text(); + const collapsed = text.replace(/\s+/g, ' ').trim(); + if (collapsed.length === 0) return ''; + return collapsed.length <= maxChars ? collapsed : collapsed.slice(0, maxChars - 1) + '…'; + } catch { + return ''; + } +} + +export async function fetchStoryIndex( + origin: string, + options: FetchStoryIndexOptions = {}, +): Promise { + const maxAttempts = Math.max(1, options.maxAttempts ?? DEFAULT_MAX_ATTEMPTS); + const baseBackoffMs = options.baseBackoffMs ?? DEFAULT_BASE_BACKOFF_MS; + const sleep = options.sleep ?? defaultSleep; const indexUrl = `${origin}/index.json`; logger.debug(`Fetching story index from: ${indexUrl}`); - const response = await fetch(indexUrl); + let lastErrorMessage = ''; + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + const response = await fetch(indexUrl); - if (!response.ok) { - throw new Error(`Failed to fetch story index: ${response.status} ${response.statusText}`); - } + if (response.ok) { + const index = (await response.json()) as StoryIndex; + logger.debug(`Story index entries found: ${Object.keys(index.entries).length}`); + return index; + } + + const snippet = await readBodySnippet(response); + const transient = isTransientStatus(response.status); + const baseMsg = + `Failed to fetch story index: ${response.status} ${response.statusText}` + + (snippet ? ` — ${snippet}` : ''); + lastErrorMessage = + `${baseMsg} (attempt ${attempt}/${maxAttempts}` + + (transient ? ', transient — Storybook may be mid-recompile' : ', non-transient — giving up') + + ')'; + logger.debug(lastErrorMessage); - const index = (await response.json()) as StoryIndex; + if (!transient || attempt === maxAttempts) break; - logger.debug(`Story index entries found: ${Object.keys(index.entries).length}`); + await sleep(attempt * baseBackoffMs); + } - return index; + throw new Error( + lastErrorMessage || + `Failed to fetch story index after ${maxAttempts} attempts`, + ); } From a2c1c20f9041d5f566dd8cbdf25d0f26f2d6d1b3 Mon Sep 17 00:00:00 2001 From: yannbf Date: Thu, 21 May 2026 16:40:47 +0200 Subject: [PATCH 04/26] fix tests and review comments --- .../tests/mcp-endpoint.e2e.test.ts | 140 ++++++++++++++- .../build-server-instructions.test.ts | 4 +- .../instructions/build-server-instructions.ts | 9 +- .../src/instructions/dev-instructions.md | 2 +- packages/addon-mcp/src/review-state-store.ts | 4 +- .../src/tools/apply-review-state.test.ts | 161 ++++++++++++++++++ .../addon-mcp/src/tools/apply-review-state.ts | 39 ++++- .../src/utils/fetch-story-index.test.ts | 20 ++- .../addon-mcp/src/utils/fetch-story-index.ts | 5 +- 9 files changed, 365 insertions(+), 19 deletions(-) create mode 100644 packages/addon-mcp/src/tools/apply-review-state.test.ts diff --git a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts index f4a4b09a..43c1b757 100644 --- a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts +++ b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts @@ -100,7 +100,7 @@ describe('MCP Endpoint E2E Tests', () => { expect(response.result).toHaveProperty('tools'); // Dev, docs, and test tools should be present - expect(response.result.tools).toHaveLength(7); + expect(response.result.tools).toHaveLength(8); expect(response.result.tools).toMatchInlineSnapshot(` [ @@ -378,6 +378,143 @@ describe('MCP Endpoint E2E Tests', () => { "name": "get-changed-stories", "title": "Get changed stories metadata", }, + { + "description": "Push a curated review of the current change to Storybook's review page. + + After you finish a UI code change, call this to help the user spot-check it. Provide: + - title: a PR-style title for the change — short and specific. + - description: a one-line summary of what changed and where to start reviewing. + - collections: titled groups of representative story IDs. Give each a concise, PR-dense title, a one-sentence rationale, and — when you can tell — a kind ("atomic" for the directly changed component, "consumer" for direct dependents, "transitive" for pages/containers, "catch-all" otherwise). + - changedFiles: the files you edited (most central first). + - diffHunks: the actual diff of your change (you made it — include the hunks). + - storyMeta: optional per-story { depth, chain }. + + Always include the returned reviewUrl in your final user-facing response so the user can open it.", + "inputSchema": { + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "changedFiles": { + "description": "Paths of the files you changed, most central first.", + "items": { + "type": "string", + }, + "type": "array", + }, + "collections": { + "items": { + "properties": { + "kind": { + "description": "Semantic role of this collection in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.", + "enum": [ + "atomic", + "consumer", + "transitive", + "catch-all", + ], + "type": "string", + }, + "rationale": { + "description": "One sentence explaining why these stories are grouped together.", + "type": "string", + }, + "sampleStoryIds": { + "description": "Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.", + "items": { + "type": "string", + }, + "type": "array", + }, + "title": { + "description": "Short, PR-dense title for this collection, e.g. "Direct Button importers".", + "type": "string", + }, + }, + "required": [ + "title", + "rationale", + "sampleStoryIds", + ], + "type": "object", + }, + "type": "array", + }, + "description": { + "description": "One-line summary of what changed and where to start reviewing.", + "type": "string", + }, + "diffHunks": { + "description": "The actual diff hunks of your change, shown in the review page.", + "items": { + "properties": { + "hunk": { + "description": "Unified-diff text for this hunk (with +/- line prefixes).", + "type": "string", + }, + "path": { + "description": "Path of the changed file this hunk belongs to.", + "type": "string", + }, + }, + "required": [ + "path", + "hunk", + ], + "type": "object", + }, + "type": "array", + }, + "storyMeta": { + "additionalProperties": { + "properties": { + "chain": { + "description": "Ordered intermediate file paths between the story file and the changed file, excluding both endpoints. Empty/omitted means a direct import.", + "items": { + "type": "string", + }, + "type": "array", + }, + "depth": { + "description": "Graph distance from the changed file(s) to this story (0 = the changed component itself).", + "type": "number", + }, + }, + "required": [], + "type": "object", + }, + "description": "Optional per-story metadata keyed by story ID: { depth, chain }.", + "propertyNames": { + "type": "string", + }, + "type": "object", + }, + "title": { + "description": "PR-style title for the change — short and specific, e.g. "Recolour the primary button".", + "type": "string", + }, + }, + "required": [ + "title", + "description", + "collections", + ], + "type": "object", + }, + "name": "apply-review-state", + "outputSchema": { + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "reviewUrl": { + "description": "URL of the Storybook review page. Always include this URL in your final user-facing response so the user can open it directly.", + "type": "string", + }, + }, + "required": [ + "reviewUrl", + ], + "type": "object", + }, + "title": "Apply Storybook review state", + }, { "description": "Run story tests. Provide stories for focused runs (faster while iterating), @@ -901,6 +1038,7 @@ describe('MCP Endpoint E2E Tests', () => { "preview-stories", "get-storybook-story-instructions", "get-changed-stories", + "apply-review-state", ] `); }); diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts index 5778ad80..5a4ae4c0 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts @@ -19,6 +19,7 @@ describe('buildServerInstructions', () => { - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. + - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response. ## Validation Workflow @@ -63,7 +64,8 @@ describe('buildServerInstructions', () => { - Before creating or editing components or stories, call **get-storybook-story-instructions**. - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - - Always include every returned preview URL in your user-facing response so the user can verify the visual result." + - Always include every returned preview URL in your user-facing response so the user can verify the visual result. + - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response." `); }); diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.ts b/packages/addon-mcp/src/instructions/build-server-instructions.ts index e85dd654..d7ccf729 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.ts @@ -13,14 +13,21 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) const sections = ['Follow these workflows when working with UI and/or Storybook.']; if (options.devEnabled) { + const changeDetection = options.changeDetectionEnabled ?? false; sections.push( devInstructions .replace( '{{PREVIEW_STORIES_STEP}}', - (options.changeDetectionEnabled ?? false) + changeDetection ? 'After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs.' : 'After changing any component or story, call **preview-stories** to retrieve preview URLs.', ) + .replace( + '{{APPLY_REVIEW_STATE_STEP}}', + changeDetection + ? "\n- After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response." + : '', + ) .trim(), ); } diff --git a/packages/addon-mcp/src/instructions/dev-instructions.md b/packages/addon-mcp/src/instructions/dev-instructions.md index 847625f6..cfceaa35 100644 --- a/packages/addon-mcp/src/instructions/dev-instructions.md +++ b/packages/addon-mcp/src/instructions/dev-instructions.md @@ -3,4 +3,4 @@ - Before creating or editing components or stories, call **get-storybook-story-instructions**. - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - {{PREVIEW_STORIES_STEP}} -- Always include every returned preview URL in your user-facing response so the user can verify the visual result. +- Always include every returned preview URL in your user-facing response so the user can verify the visual result.{{APPLY_REVIEW_STATE_STEP}} diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts index 44b60d26..a4b6ccbf 100644 --- a/packages/addon-mcp/src/review-state-store.ts +++ b/packages/addon-mcp/src/review-state-store.ts @@ -61,7 +61,9 @@ const DiffHunkSchema = v.object({ export const ReviewStateSchema = v.object({ title: v.pipe( v.string(), - v.description('PR-style title for the change — short and specific, e.g. "Recolour the primary button".'), + v.description( + 'PR-style title for the change — short and specific, e.g. "Recolour the primary button".', + ), ), description: v.pipe( v.string(), diff --git a/packages/addon-mcp/src/tools/apply-review-state.test.ts b/packages/addon-mcp/src/tools/apply-review-state.test.ts new file mode 100644 index 00000000..ddabb0b1 --- /dev/null +++ b/packages/addon-mcp/src/tools/apply-review-state.test.ts @@ -0,0 +1,161 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { McpServer } from 'tmcp'; +import { ValibotJsonSchemaAdapter } from '@tmcp/adapter-valibot'; +import { addApplyReviewStateTool, buildReviewUrl } from './apply-review-state.ts'; +import { APPLY_REVIEW_STATE_TOOL_NAME } from './tool-names.ts'; +import { APPLY_REVIEW_STATE_EVENT } from '../constants.ts'; +import { getReviewState, type ReviewState } from '../review-state-store.ts'; +import type { AddonContext } from '../types.ts'; + +const sampleReview: ReviewState = { + title: 'Recolour the primary button', + description: 'Button background changed from blue to green.', + collections: [ + { + title: 'Button', + rationale: 'The directly changed component.', + sampleStoryIds: ['button--primary', 'button--secondary'], + kind: 'atomic', + }, + { + title: 'Pages', + rationale: 'Pages that render Button.', + sampleStoryIds: ['page--home'], + kind: 'transitive', + }, + ], + changedFiles: ['src/Button.tsx'], +}; + +describe('buildReviewUrl', () => { + it('falls back to origin when there is no request', () => { + expect(buildReviewUrl({ origin: 'http://localhost:6006' })).toBe( + 'http://localhost:6006/?path=/review-changes/', + ); + }); + + it('derives the host and path prefix from the request for a proxied Storybook', () => { + expect( + buildReviewUrl({ + origin: 'http://localhost:6006', + request: new Request('https://example.com/storybook/mcp'), + }), + ).toBe('https://example.com/storybook/?path=/review-changes/'); + }); + + it('handles a request served at the host root', () => { + expect(buildReviewUrl({ request: new Request('http://localhost:6006/mcp') })).toBe( + 'http://localhost:6006/?path=/review-changes/', + ); + }); + + it('throws when neither request nor origin is available', () => { + expect(() => buildReviewUrl({})).toThrow(/Cannot resolve the Storybook URL/); + }); +}); + +describe('applyReviewStateTool', () => { + let server: McpServer; + let emitted: Array<{ event: string; payload: unknown }>; + + function makeContext(overrides: Partial = {}): AddonContext { + return { + origin: 'http://localhost:6006', + options: { + channel: { + emit: (event: string, payload: unknown) => emitted.push({ event, payload }), + }, + } as unknown as AddonContext['options'], + disableTelemetry: true, + ...overrides, + }; + } + + beforeEach(async () => { + emitted = []; + const adapter = new ValibotJsonSchemaAdapter(); + server = new McpServer( + { + name: 'test-server', + version: '1.0.0', + description: 'Test server for apply-review-state tool', + }, + { + adapter, + capabilities: { + tools: { listChanged: true }, + }, + }, + ).withContext(); + + await server.receive( + { + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { + protocolVersion: '2025-06-18', + capabilities: {}, + clientInfo: { name: 'test', version: '1.0.0' }, + }, + }, + { sessionId: 'test-session' }, + ); + + await addApplyReviewStateTool(server); + }); + + async function callTool(args: ReviewState, custom: AddonContext) { + return server.receive( + { + jsonrpc: '2.0' as const, + id: 1, + method: 'tools/call', + params: { name: APPLY_REVIEW_STATE_TOOL_NAME, arguments: args }, + }, + { sessionId: 'test-session', custom }, + ); + } + + function getResult(response: unknown) { + return ( + response as { + result?: { + content?: Array<{ text?: string }>; + structuredContent?: { reviewUrl?: string }; + isError?: boolean; + }; + } + ).result; + } + + it('stores the review state and returns the review URL', async () => { + const response = await callTool(sampleReview, makeContext()); + const result = getResult(response); + + expect(result?.isError).toBeFalsy(); + expect(result?.structuredContent?.reviewUrl).toBe( + 'http://localhost:6006/?path=/review-changes/', + ); + expect(result?.content?.[0]?.text).toContain('2 collections, 3 stories'); + expect(result?.content?.[0]?.text).toContain('http://localhost:6006/?path=/review-changes/'); + expect(getReviewState()).toEqual(sampleReview); + }); + + it('broadcasts the review over the Storybook channel', async () => { + await callTool(sampleReview, makeContext()); + expect(emitted).toContainEqual({ event: APPLY_REVIEW_STATE_EVENT, payload: sampleReview }); + }); + + it('builds a subpath-aware review URL from the incoming request', async () => { + const response = await callTool( + sampleReview, + makeContext({ request: new Request('https://sb.example.com/design-system/mcp') }), + ); + const result = getResult(response); + + expect(result?.structuredContent?.reviewUrl).toBe( + 'https://sb.example.com/design-system/?path=/review-changes/', + ); + }); +}); diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index bc9b5366..5a3f83bf 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -15,6 +15,39 @@ const ApplyReviewStateOutput = v.object({ ), }); +/** + * Resolve the Storybook manager root from the incoming MCP request. + * + * The MCP endpoint is mounted at `/mcp`, so stripping the + * trailing `/mcp` segment from the request path yields the root the + * review page lives under. Unlike the context `origin` (always just + * `http://localhost:`), this preserves the real host and any path + * prefix when Storybook is served behind a reverse proxy. + */ +function storybookRootFromRequest(request?: Request): string | undefined { + if (!request?.url) return undefined; + try { + const url = new URL(request.url); + const root = url.pathname.replace(/\/mcp\/?$/, ''); + return `${url.origin}${root}`; + } catch { + return undefined; + } +} + +/** + * Build the URL of the Storybook review page. Prefers the incoming + * request (carries the real host + path prefix); falls back to the + * context `origin` when there is no request. + */ +export function buildReviewUrl(ctx: { origin?: string; request?: Request }): string { + const root = storybookRootFromRequest(ctx.request) ?? ctx.origin; + if (!root) { + throw new Error('Cannot resolve the Storybook URL: no request or origin in addon context.'); + } + return `${root.replace(/\/$/, '')}/?path=${REVIEW_PAGE_PATH}`; +} + export async function addApplyReviewStateTool(server: McpServer) { server.tool( { @@ -37,10 +70,7 @@ Always include the returned reviewUrl in your final user-facing response so the }, async (input: ReviewState) => { try { - const { origin } = server.ctx.custom ?? {}; - if (!origin) { - throw new Error('Origin is required in addon context'); - } + const reviewUrl = buildReviewUrl(server.ctx.custom ?? {}); setReviewState(input); @@ -48,7 +78,6 @@ Always include the returned reviewUrl in your final user-facing response so the // to the review page; a cold start relies on the returned URL. server.ctx.custom?.options?.channel?.emit(APPLY_REVIEW_STATE_EVENT, input); - const reviewUrl = `${origin}/?path=${REVIEW_PAGE_PATH}`; const collectionCount = input.collections.length; const storyCount = input.collections.reduce((n, c) => n + c.sampleStoryIds.length, 0); diff --git a/packages/addon-mcp/src/utils/fetch-story-index.test.ts b/packages/addon-mcp/src/utils/fetch-story-index.test.ts index 671f65a6..da679798 100644 --- a/packages/addon-mcp/src/utils/fetch-story-index.test.ts +++ b/packages/addon-mcp/src/utils/fetch-story-index.test.ts @@ -37,9 +37,9 @@ describe('fetchStoryIndex', () => { text: async () => '', }); - await expect( - fetchStoryIndex('http://localhost:6006', { sleep: vi.fn() }), - ).rejects.toThrow(/Failed to fetch story index: 404 Not Found/); + await expect(fetchStoryIndex('http://localhost:6006', { sleep: vi.fn() })).rejects.toThrow( + /Failed to fetch story index: 404 Not Found/, + ); expect(mockFetch).toHaveBeenCalledTimes(1); }); @@ -115,8 +115,18 @@ describe('fetchStoryIndex', () => { it('should retry on 429 (too many requests) and 408 (timeout)', async () => { const mockFetch = global.fetch as any; mockFetch - .mockResolvedValueOnce({ ok: false, status: 429, statusText: 'Too Many Requests', text: async () => '' }) - .mockResolvedValueOnce({ ok: false, status: 408, statusText: 'Request Timeout', text: async () => '' }) + .mockResolvedValueOnce({ + ok: false, + status: 429, + statusText: 'Too Many Requests', + text: async () => '', + }) + .mockResolvedValueOnce({ + ok: false, + status: 408, + statusText: 'Request Timeout', + text: async () => '', + }) .mockResolvedValueOnce({ ok: true, json: async () => smallStoryIndexFixture }); const sleep = vi.fn().mockResolvedValue(undefined); diff --git a/packages/addon-mcp/src/utils/fetch-story-index.ts b/packages/addon-mcp/src/utils/fetch-story-index.ts index 6e59f11e..c8f5123d 100644 --- a/packages/addon-mcp/src/utils/fetch-story-index.ts +++ b/packages/addon-mcp/src/utils/fetch-story-index.ts @@ -89,8 +89,5 @@ export async function fetchStoryIndex( await sleep(attempt * baseBackoffMs); } - throw new Error( - lastErrorMessage || - `Failed to fetch story index after ${maxAttempts} attempts`, - ); + throw new Error(lastErrorMessage || `Failed to fetch story index after ${maxAttempts} attempts`); } From 755eb124d886d6ce5fe30218cb58035ffb8e26fd Mon Sep 17 00:00:00 2001 From: yannbf Date: Fri, 22 May 2026 00:07:14 +0200 Subject: [PATCH 05/26] add branch name --- packages/addon-mcp/src/review-state-store.ts | 15 +++++++++--- .../src/tools/apply-review-state.test.ts | 21 ++++++++++++++++- .../addon-mcp/src/tools/apply-review-state.ts | 23 +++++++++++++++---- packages/addon-mcp/src/utils/git-branch.ts | 21 +++++++++++++++++ 4 files changed, 71 insertions(+), 9 deletions(-) create mode 100644 packages/addon-mcp/src/utils/git-branch.ts diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts index a4b6ccbf..62fb0880 100644 --- a/packages/addon-mcp/src/review-state-store.ts +++ b/packages/addon-mcp/src/review-state-store.ts @@ -87,12 +87,21 @@ export const ReviewStateSchema = v.object({ export type ReviewCollection = v.InferOutput; export type ReviewState = v.InferOutput; -let cached: ReviewState | undefined; +/** + * A pushed `ReviewState` plus server-added metadata. `branchName` is the + * target repo's current git branch, resolved server-side at push time — + * the agent's payload does not (and should not) carry it. + */ +export type StoredReviewState = ReviewState & { + branchName?: string; +}; + +let cached: StoredReviewState | undefined; -export function setReviewState(state: ReviewState): void { +export function setReviewState(state: StoredReviewState): void { cached = state; } -export function getReviewState(): ReviewState | undefined { +export function getReviewState(): StoredReviewState | undefined { return cached; } diff --git a/packages/addon-mcp/src/tools/apply-review-state.test.ts b/packages/addon-mcp/src/tools/apply-review-state.test.ts index ddabb0b1..3b5f2978 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.test.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { McpServer } from 'tmcp'; import { ValibotJsonSchemaAdapter } from '@tmcp/adapter-valibot'; import { addApplyReviewStateTool, buildReviewUrl } from './apply-review-state.ts'; @@ -7,6 +7,13 @@ import { APPLY_REVIEW_STATE_EVENT } from '../constants.ts'; import { getReviewState, type ReviewState } from '../review-state-store.ts'; import type { AddonContext } from '../types.ts'; +// The tool resolves the target repo's git branch server-side. Mock it so +// the unit tests don't depend on the branch this repo happens to be on. +const { mockCurrentGitBranch } = vi.hoisted(() => ({ mockCurrentGitBranch: vi.fn() })); +vi.mock('../utils/git-branch.ts', () => ({ + currentGitBranch: (...args: unknown[]) => mockCurrentGitBranch(...args), +})); + const sampleReview: ReviewState = { title: 'Recolour the primary button', description: 'Button background changed from blue to green.', @@ -73,6 +80,8 @@ describe('applyReviewStateTool', () => { beforeEach(async () => { emitted = []; + mockCurrentGitBranch.mockReset(); + mockCurrentGitBranch.mockResolvedValue(undefined); const adapter = new ValibotJsonSchemaAdapter(); server = new McpServer( { @@ -158,4 +167,14 @@ describe('applyReviewStateTool', () => { 'https://sb.example.com/design-system/?path=/review-changes/', ); }); + + it('attaches the target repo git branch resolved server-side', async () => { + mockCurrentGitBranch.mockResolvedValue('feature/badge-pink'); + + await callTool(sampleReview, makeContext()); + + const expected = { ...sampleReview, branchName: 'feature/badge-pink' }; + expect(getReviewState()).toEqual(expected); + expect(emitted).toContainEqual({ event: APPLY_REVIEW_STATE_EVENT, payload: expected }); + }); }); diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index 5a3f83bf..7382fcd6 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -2,9 +2,15 @@ import type { McpServer } from 'tmcp'; import * as v from 'valibot'; import type { AddonContext } from '../types.ts'; import { errorToMCPContent } from '../utils/errors.ts'; -import { ReviewStateSchema, setReviewState, type ReviewState } from '../review-state-store.ts'; +import { + ReviewStateSchema, + setReviewState, + type ReviewState, + type StoredReviewState, +} from '../review-state-store.ts'; import { APPLY_REVIEW_STATE_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; import { APPLY_REVIEW_STATE_TOOL_NAME } from './tool-names.ts'; +import { currentGitBranch } from '../utils/git-branch.ts'; const ApplyReviewStateOutput = v.object({ reviewUrl: v.pipe( @@ -72,14 +78,21 @@ Always include the returned reviewUrl in your final user-facing response so the try { const reviewUrl = buildReviewUrl(server.ctx.custom ?? {}); - setReviewState(input); + // Resolve the target repo's current git branch server-side and + // attach it — the agent's payload doesn't carry it, but the + // review page shows it. Best-effort: omitted on a detached HEAD + // or a non-git target. + const branchName = await currentGitBranch(process.cwd()); + const state: StoredReviewState = branchName ? { ...input, branchName } : input; + + setReviewState(state); // Broadcast to all connected Storybook tabs. A warm tab navigates // to the review page; a cold start relies on the returned URL. - server.ctx.custom?.options?.channel?.emit(APPLY_REVIEW_STATE_EVENT, input); + server.ctx.custom?.options?.channel?.emit(APPLY_REVIEW_STATE_EVENT, state); - const collectionCount = input.collections.length; - const storyCount = input.collections.reduce((n, c) => n + c.sampleStoryIds.length, 0); + const collectionCount = state.collections.length; + const storyCount = state.collections.reduce((n, c) => n + c.sampleStoryIds.length, 0); return { content: [ diff --git a/packages/addon-mcp/src/utils/git-branch.ts b/packages/addon-mcp/src/utils/git-branch.ts new file mode 100644 index 00000000..d9081678 --- /dev/null +++ b/packages/addon-mcp/src/utils/git-branch.ts @@ -0,0 +1,21 @@ +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; + +const execFileAsync = promisify(execFile); + +/** + * Resolve the current git branch of the repo at `cwd`. + * + * Best-effort: returns `undefined` on a detached HEAD, a non-git + * directory, or any git error — callers treat the branch as simply + * unknown rather than failing. + */ +export async function currentGitBranch(cwd: string): Promise { + try { + const { stdout } = await execFileAsync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd }); + const branch = stdout.trim(); + return branch && branch !== 'HEAD' ? branch : undefined; + } catch { + return undefined; + } +} From d047ca89286b4c3ce76af7d67590fbd19542a1b8 Mon Sep 17 00:00:00 2001 From: yannbf Date: Fri, 22 May 2026 15:52:38 +0200 Subject: [PATCH 06/26] sampleStoryIds -> storyIds renaming --- apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts | 4 ++-- packages/addon-mcp/src/review-state-store.ts | 2 +- packages/addon-mcp/src/tools/apply-review-state.test.ts | 4 ++-- packages/addon-mcp/src/tools/apply-review-state.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts index 43c1b757..3e1fe4ac 100644 --- a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts +++ b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts @@ -417,7 +417,7 @@ describe('MCP Endpoint E2E Tests', () => { "description": "One sentence explaining why these stories are grouped together.", "type": "string", }, - "sampleStoryIds": { + "storyIds": { "description": "Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.", "items": { "type": "string", @@ -432,7 +432,7 @@ describe('MCP Endpoint E2E Tests', () => { "required": [ "title", "rationale", - "sampleStoryIds", + "storyIds", ], "type": "object", }, diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts index 62fb0880..0d89a727 100644 --- a/packages/addon-mcp/src/review-state-store.ts +++ b/packages/addon-mcp/src/review-state-store.ts @@ -21,7 +21,7 @@ export const ReviewCollectionSchema = v.object({ v.string(), v.description('One sentence explaining why these stories are grouped together.'), ), - sampleStoryIds: v.pipe( + storyIds: v.pipe( v.array(v.string()), v.description( 'Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.', diff --git a/packages/addon-mcp/src/tools/apply-review-state.test.ts b/packages/addon-mcp/src/tools/apply-review-state.test.ts index 3b5f2978..6654e435 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.test.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.test.ts @@ -21,13 +21,13 @@ const sampleReview: ReviewState = { { title: 'Button', rationale: 'The directly changed component.', - sampleStoryIds: ['button--primary', 'button--secondary'], + storyIds: ['button--primary', 'button--secondary'], kind: 'atomic', }, { title: 'Pages', rationale: 'Pages that render Button.', - sampleStoryIds: ['page--home'], + storyIds: ['page--home'], kind: 'transitive', }, ], diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index 7382fcd6..6f048a78 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -92,7 +92,7 @@ Always include the returned reviewUrl in your final user-facing response so the server.ctx.custom?.options?.channel?.emit(APPLY_REVIEW_STATE_EVENT, state); const collectionCount = state.collections.length; - const storyCount = state.collections.reduce((n, c) => n + c.sampleStoryIds.length, 0); + const storyCount = state.collections.reduce((n, c) => n + c.storyIds.length, 0); return { content: [ From 1556adc9957d83b0a38cc5d7900994c48d49d26f Mon Sep 17 00:00:00 2001 From: yannbf Date: Fri, 22 May 2026 17:59:45 +0200 Subject: [PATCH 07/26] improve agent phrasing at the end of session --- .../src/instructions/build-server-instructions.test.ts | 3 +++ packages/addon-mcp/src/instructions/dev-instructions.md | 1 + .../addon-mcp/src/instructions/storybook-story-instructions.md | 1 + packages/addon-mcp/src/tools/apply-review-state.ts | 2 +- 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts index 5a4ae4c0..c90318f1 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts @@ -18,6 +18,7 @@ describe('buildServerInstructions', () => { - Before creating or editing components or stories, call **get-storybook-story-instructions**. - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. + - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response. @@ -64,6 +65,7 @@ describe('buildServerInstructions', () => { - Before creating or editing components or stories, call **get-storybook-story-instructions**. - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. + - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response." `); @@ -85,6 +87,7 @@ describe('buildServerInstructions', () => { - Before creating or editing components or stories, call **get-storybook-story-instructions**. - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - After changing any component or story, call **preview-stories** to retrieve preview URLs. + - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result." `); }); diff --git a/packages/addon-mcp/src/instructions/dev-instructions.md b/packages/addon-mcp/src/instructions/dev-instructions.md index cfceaa35..6d681d42 100644 --- a/packages/addon-mcp/src/instructions/dev-instructions.md +++ b/packages/addon-mcp/src/instructions/dev-instructions.md @@ -3,4 +3,5 @@ - Before creating or editing components or stories, call **get-storybook-story-instructions**. - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - {{PREVIEW_STORIES_STEP}} +- In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result.{{APPLY_REVIEW_STATE_STEP}} diff --git a/packages/addon-mcp/src/instructions/storybook-story-instructions.md b/packages/addon-mcp/src/instructions/storybook-story-instructions.md index ac7589af..87db1ccf 100644 --- a/packages/addon-mcp/src/instructions/storybook-story-instructions.md +++ b/packages/addon-mcp/src/instructions/storybook-story-instructions.md @@ -147,6 +147,7 @@ play: async ({ canvas }) => { - ALWAYS provide story links after any changes to stories files, including changes to existing stories. - {{STORY_LINKING_WORKFLOW}} +- In the final user-facing response, present links in this order: review page URL first (when available), changed-stories fallback URL second (when relevant), then specific story preview URLs. - When sharing links, choose the most relevant subset for the user and avoid long lists (generally no more than 5 links). - {{CHANGED_STORY_FALLBACK_LINK_GUIDANCE}} - After changing any UI components, ALWAYS search for related stories that might cover the changes you've made. If you find any, provide the story links to the user. THIS IS VERY IMPORTANT, as it allows the user to visually inspect the changes you've made. Even later in a session when changing UI components or stories that have already been linked to previously, YOU MUST PROVIDE THE LINKS AGAIN. diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index 6f048a78..5e9f7f5e 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -64,7 +64,7 @@ export async function addApplyReviewStateTool(server: McpServer Date: Tue, 26 May 2026 09:31:34 +0200 Subject: [PATCH 08/26] fix feature gating --- packages/addon-mcp/src/preset.test.ts | 41 +++++++++++++++++++++++++++ packages/addon-mcp/src/preset.ts | 6 ++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/addon-mcp/src/preset.test.ts b/packages/addon-mcp/src/preset.test.ts index 2dba0538..fc4fc19d 100644 --- a/packages/addon-mcp/src/preset.test.ts +++ b/packages/addon-mcp/src/preset.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { Options } from 'storybook/internal/types'; import { experimental_devServer } from './preset.ts'; import * as runStoryTests from './tools/run-story-tests.ts'; +import { REQUEST_REVIEW_STATE_EVENT } from './constants.ts'; describe('experimental_devServer', () => { let mockApp: any; @@ -35,6 +36,46 @@ describe('experimental_devServer', () => { expect(mcpHandler).toBeDefined(); }); + it('registers review-state replay listener only when changeDetection is enabled', async () => { + const channel = { on: vi.fn(), emit: vi.fn() }; + const optionsWithChangeDetection = { + ...mockOptions, + channel, + presets: { + apply: vi.fn((key: string) => { + if (key === 'features') { + return Promise.resolve({ changeDetection: true }); + } + return Promise.resolve(undefined); + }), + }, + } as unknown as Options; + + await (experimental_devServer as any)(mockApp, optionsWithChangeDetection); + + expect(channel.on).toHaveBeenCalledWith(REQUEST_REVIEW_STATE_EVENT, expect.any(Function)); + }); + + it('does not register review-state replay listener when changeDetection is disabled', async () => { + const channel = { on: vi.fn(), emit: vi.fn() }; + const optionsWithoutChangeDetection = { + ...mockOptions, + channel, + presets: { + apply: vi.fn((key: string) => { + if (key === 'features') { + return Promise.resolve({ changeDetection: false }); + } + return Promise.resolve(undefined); + }), + }, + } as unknown as Options; + + await (experimental_devServer as any)(mockApp, optionsWithoutChangeDetection); + + expect(channel.on).not.toHaveBeenCalledWith(REQUEST_REVIEW_STATE_EVENT, expect.any(Function)); + }); + it('should register /mcp GET endpoint', async () => { await (experimental_devServer as any)(mockApp, mockOptions); diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index 5b170e7d..9f053470 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -28,6 +28,8 @@ export const experimental_devServer: PresetPropertyFn<'experimental_devServer'> const addonOptions = v.parse(AddonOptions, { toolsets: 'toolsets' in options ? options.toolsets : {}, }); + const features = await options.presets.apply('features', {}); + const changeDetectionEnabled = features?.changeDetection ?? false; const origin = `http://localhost:${options.port}`; @@ -39,14 +41,14 @@ export const experimental_devServer: PresetPropertyFn<'experimental_devServer'> // Older Storybooks (≤10.3) don't pass `options.channel` to dev-server // presets — the rest of addon-mcp still works without channel replay, // so guard rather than crash. - if (options.channel?.on) { + if (changeDetectionEnabled && options.channel?.on) { options.channel.on(REQUEST_REVIEW_STATE_EVENT, () => { const state = getReviewState(); if (state) { options.channel?.emit?.(APPLY_REVIEW_STATE_EVENT, state); } }); - } else { + } else if (!options.channel?.on) { logger.info( '[addon-mcp] options.channel is unavailable on this Storybook version; review-state replay disabled.', ); From 2e5d2fceb93fcb86f0af9256731f83790d7ca299 Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 09:31:52 +0200 Subject: [PATCH 09/26] add tests --- .../addon-mcp/src/utils/git-branch.test.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 packages/addon-mcp/src/utils/git-branch.test.ts diff --git a/packages/addon-mcp/src/utils/git-branch.test.ts b/packages/addon-mcp/src/utils/git-branch.test.ts new file mode 100644 index 00000000..26eb24a0 --- /dev/null +++ b/packages/addon-mcp/src/utils/git-branch.test.ts @@ -0,0 +1,55 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockExecFile } = vi.hoisted(() => ({ + mockExecFile: vi.fn(), +})); + +vi.mock('node:child_process', () => ({ + execFile: mockExecFile, +})); + +import { currentGitBranch } from './git-branch.ts'; + +type ExecFileCallback = (error: Error | null, result?: { stdout: string }) => void; + +describe('currentGitBranch', () => { + beforeEach(() => { + mockExecFile.mockReset(); + }); + + it('returns the current branch name in a git repo', async () => { + mockExecFile.mockImplementation( + (_file: string, _args: string[], _options: { cwd: string }, callback: ExecFileCallback) => { + callback(null, { stdout: 'feature/review-state\n' }); + }, + ); + + await expect(currentGitBranch('/repo')).resolves.toBe('feature/review-state'); + expect(mockExecFile).toHaveBeenCalledWith( + 'git', + ['rev-parse', '--abbrev-ref', 'HEAD'], + { cwd: '/repo' }, + expect.any(Function), + ); + }); + + it('returns undefined on detached HEAD', async () => { + mockExecFile.mockImplementation( + (_file: string, _args: string[], _options: { cwd: string }, callback: ExecFileCallback) => { + callback(null, { stdout: 'HEAD\n' }); + }, + ); + + await expect(currentGitBranch('/repo')).resolves.toBeUndefined(); + }); + + it('returns undefined when git command fails (e.g. non-git directory)', async () => { + mockExecFile.mockImplementation( + (_file: string, _args: string[], _options: { cwd: string }, callback: ExecFileCallback) => { + callback(new Error('fatal: not a git repository')); + }, + ); + + await expect(currentGitBranch('/tmp')).resolves.toBeUndefined(); + }); +}); From a9adf32e3378bac8e0a729782382cc4474aedf03 Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 09:51:00 +0200 Subject: [PATCH 10/26] improve security and prompt --- .../tests/mcp-endpoint.e2e.test.ts | 2 + .../src/tools/apply-review-state.test.ts | 15 +++++-- .../addon-mcp/src/tools/apply-review-state.ts | 42 +++++++++++++------ 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts index 3e1fe4ac..06a5b20a 100644 --- a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts +++ b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts @@ -389,6 +389,8 @@ describe('MCP Endpoint E2E Tests', () => { - diffHunks: the actual diff of your change (you made it — include the hunks). - storyMeta: optional per-story { depth, chain }. + The \`kind\` labels are for structured review grouping and UI behavior; do not repeat these labels verbatim in user-facing prose unless the user explicitly asks for them. + Always include the returned reviewUrl in your final user-facing response so the user can open it.", "inputSchema": { "$schema": "http://json-schema.org/draft-07/schema#", diff --git a/packages/addon-mcp/src/tools/apply-review-state.test.ts b/packages/addon-mcp/src/tools/apply-review-state.test.ts index 6654e435..373b9b41 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.test.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.test.ts @@ -41,13 +41,22 @@ describe('buildReviewUrl', () => { ); }); - it('derives the host and path prefix from the request for a proxied Storybook', () => { + it('uses trusted origin host and request path prefix for a proxied Storybook', () => { expect( buildReviewUrl({ origin: 'http://localhost:6006', request: new Request('https://example.com/storybook/mcp'), }), - ).toBe('https://example.com/storybook/?path=/review-changes/'); + ).toBe('http://localhost:6006/storybook/?path=/review-changes/'); + }); + + it('does not trust request host when origin is available', () => { + expect( + buildReviewUrl({ + origin: 'http://localhost:6006', + request: new Request('https://evil.example.org/prefix/mcp'), + }), + ).toBe('http://localhost:6006/prefix/?path=/review-changes/'); }); it('handles a request served at the host root', () => { @@ -164,7 +173,7 @@ describe('applyReviewStateTool', () => { const result = getResult(response); expect(result?.structuredContent?.reviewUrl).toBe( - 'https://sb.example.com/design-system/?path=/review-changes/', + 'http://localhost:6006/design-system/?path=/review-changes/', ); }); diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index 5e9f7f5e..9c87db61 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -24,30 +24,44 @@ const ApplyReviewStateOutput = v.object({ /** * Resolve the Storybook manager root from the incoming MCP request. * - * The MCP endpoint is mounted at `/mcp`, so stripping the - * trailing `/mcp` segment from the request path yields the root the - * review page lives under. Unlike the context `origin` (always just - * `http://localhost:`), this preserves the real host and any path - * prefix when Storybook is served behind a reverse proxy. + * Security note: authority (scheme + host + port) comes from a trusted + * origin, not from request.url (which can be derived from the Host header). + * The request contributes only the path prefix (Storybook may be mounted at + * `/some/prefix/mcp`). */ -function storybookRootFromRequest(request?: Request): string | undefined { +function storybookRootFromRequest( + request: Request | undefined, + trustedOrigin: string, +): string | undefined { if (!request?.url) return undefined; try { const url = new URL(request.url); - const root = url.pathname.replace(/\/mcp\/?$/, ''); - return `${url.origin}${root}`; + const rootPath = url.pathname.replace(/\/mcp\/?$/, ''); + return `${trustedOrigin.replace(/\/$/, '')}${rootPath}`; } catch { return undefined; } } /** - * Build the URL of the Storybook review page. Prefers the incoming - * request (carries the real host + path prefix); falls back to the - * context `origin` when there is no request. + * Build the URL of the Storybook review page. Uses the trusted context + * origin for host authority and preserves any request path prefix so this + * works when Storybook is mounted behind a reverse proxy. */ export function buildReviewUrl(ctx: { origin?: string; request?: Request }): string { - const root = storybookRootFromRequest(ctx.request) ?? ctx.origin; + const requestOrigin = (() => { + if (!ctx.request?.url) return undefined; + try { + return new URL(ctx.request.url).origin; + } catch { + return undefined; + } + })(); + const trustedOrigin = ctx.origin ?? requestOrigin; + const root = + ctx.request && trustedOrigin + ? storybookRootFromRequest(ctx.request, trustedOrigin) + : trustedOrigin; if (!root) { throw new Error('Cannot resolve the Storybook URL: no request or origin in addon context.'); } @@ -64,11 +78,13 @@ export async function addApplyReviewStateTool(server: McpServer Date: Tue, 26 May 2026 10:12:57 +0200 Subject: [PATCH 11/26] rename review-changes to review --- packages/addon-mcp/src/constants.ts | 8 ++++---- packages/addon-mcp/src/review-state-store.ts | 2 +- .../src/tools/apply-review-state.test.ts | 16 +++++++--------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/addon-mcp/src/constants.ts b/packages/addon-mcp/src/constants.ts index d4b39208..049d6376 100644 --- a/packages/addon-mcp/src/constants.ts +++ b/packages/addon-mcp/src/constants.ts @@ -2,11 +2,11 @@ export const MCP_APP_PARAM = 'mcp-app'; export const MCP_APP_SIZE_CHANGED_EVENT = 'storybook-mcp:size-changed'; /** - * Channel events shared with `@storybook/addon-review-changes` (cross-repo + * Channel events shared with `@storybook/addon-review` (cross-repo * contract). Keep in sync with that addon's `src/constants.ts`. */ -export const APPLY_REVIEW_STATE_EVENT = 'storybook/addon-review-changes/apply-review-state'; -export const REQUEST_REVIEW_STATE_EVENT = 'storybook/addon-review-changes/request-review-state'; +export const APPLY_REVIEW_STATE_EVENT = 'storybook/addon-review/apply-review-state'; +export const REQUEST_REVIEW_STATE_EVENT = 'storybook/addon-review/request-review-state'; /** Storybook manager route the review page is registered at. */ -export const REVIEW_PAGE_PATH = '/review-changes/'; +export const REVIEW_PAGE_PATH = '/review/'; diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts index 0d89a727..a106fe4e 100644 --- a/packages/addon-mcp/src/review-state-store.ts +++ b/packages/addon-mcp/src/review-state-store.ts @@ -7,7 +7,7 @@ * restart clears it — server memory only, by design. * * This module owns the canonical valibot schema for the review contract; - * `@storybook/addon-review-changes` duplicates the inferred TS shape on its + * `@storybook/addon-review` duplicates the inferred TS shape on its * side (it only renders, so it needs the type, not the validator). */ import * as v from 'valibot'; diff --git a/packages/addon-mcp/src/tools/apply-review-state.test.ts b/packages/addon-mcp/src/tools/apply-review-state.test.ts index 373b9b41..755795b1 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.test.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.test.ts @@ -37,7 +37,7 @@ const sampleReview: ReviewState = { describe('buildReviewUrl', () => { it('falls back to origin when there is no request', () => { expect(buildReviewUrl({ origin: 'http://localhost:6006' })).toBe( - 'http://localhost:6006/?path=/review-changes/', + 'http://localhost:6006/?path=/review/', ); }); @@ -47,7 +47,7 @@ describe('buildReviewUrl', () => { origin: 'http://localhost:6006', request: new Request('https://example.com/storybook/mcp'), }), - ).toBe('http://localhost:6006/storybook/?path=/review-changes/'); + ).toBe('http://localhost:6006/storybook/?path=/review/'); }); it('does not trust request host when origin is available', () => { @@ -56,12 +56,12 @@ describe('buildReviewUrl', () => { origin: 'http://localhost:6006', request: new Request('https://evil.example.org/prefix/mcp'), }), - ).toBe('http://localhost:6006/prefix/?path=/review-changes/'); + ).toBe('http://localhost:6006/prefix/?path=/review/'); }); it('handles a request served at the host root', () => { expect(buildReviewUrl({ request: new Request('http://localhost:6006/mcp') })).toBe( - 'http://localhost:6006/?path=/review-changes/', + 'http://localhost:6006/?path=/review/', ); }); @@ -152,11 +152,9 @@ describe('applyReviewStateTool', () => { const result = getResult(response); expect(result?.isError).toBeFalsy(); - expect(result?.structuredContent?.reviewUrl).toBe( - 'http://localhost:6006/?path=/review-changes/', - ); + expect(result?.structuredContent?.reviewUrl).toBe('http://localhost:6006/?path=/review/'); expect(result?.content?.[0]?.text).toContain('2 collections, 3 stories'); - expect(result?.content?.[0]?.text).toContain('http://localhost:6006/?path=/review-changes/'); + expect(result?.content?.[0]?.text).toContain('http://localhost:6006/?path=/review/'); expect(getReviewState()).toEqual(sampleReview); }); @@ -173,7 +171,7 @@ describe('applyReviewStateTool', () => { const result = getResult(response); expect(result?.structuredContent?.reviewUrl).toBe( - 'http://localhost:6006/design-system/?path=/review-changes/', + 'http://localhost:6006/design-system/?path=/review/', ); }); From 3734bc5d2a84cfc4dbd156c314c66444250e9afb Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 10:13:02 +0200 Subject: [PATCH 12/26] prepare release --- .changeset/{apply-review-state.md => cold-otters-wait.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename .changeset/{apply-review-state.md => cold-otters-wait.md} (73%) diff --git a/.changeset/apply-review-state.md b/.changeset/cold-otters-wait.md similarity index 73% rename from .changeset/apply-review-state.md rename to .changeset/cold-otters-wait.md index c2cf4f10..43e520cd 100644 --- a/.changeset/apply-review-state.md +++ b/.changeset/cold-otters-wait.md @@ -2,4 +2,4 @@ "@storybook/addon-mcp": minor --- -Added the `apply-review-state` tool. The agent pushes a curated review of current changes and returns the review-page URL. Pairs with the `@storybook/addon-review-changes` Storybook addon. +Added the `apply-review-state` tool. The agent pushes a curated review of current changes and returns the review-page URL. Pairs with the `@storybook/addon-review` Storybook addon. From 61d7eb762baa53b04e01954cedd855a957d8b583 Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 11:15:02 +0200 Subject: [PATCH 13/26] apply review changes --- packages/addon-mcp/src/mcp-handler.ts | 3 ++ packages/addon-mcp/src/preset.ts | 26 ++++++---- .../src/tools/apply-review-state.test.ts | 13 ++--- .../addon-mcp/src/tools/apply-review-state.ts | 52 +++++++++---------- packages/addon-mcp/src/types.ts | 5 ++ 5 files changed, 52 insertions(+), 47 deletions(-) diff --git a/packages/addon-mcp/src/mcp-handler.ts b/packages/addon-mcp/src/mcp-handler.ts index b32866ae..785ba672 100644 --- a/packages/addon-mcp/src/mcp-handler.ts +++ b/packages/addon-mcp/src/mcp-handler.ts @@ -113,6 +113,7 @@ type McpServerHandlerParams = { res: ServerResponse; options: Options; addonOptions: AddonOptionsOutput; + endpoint: string; /** Sources for multi-source mode (when refs are configured) */ sources?: Source[]; /** Optional custom manifest provider, receives source as third param in multi-source mode */ @@ -130,6 +131,7 @@ export const mcpServerHandler = async ({ res, options, addonOptions, + endpoint, sources, manifestProvider, compositionAuth, @@ -148,6 +150,7 @@ export const mcpServerHandler = async ({ const addonContext: AddonContext = { options, + endpoint, toolsets: getToolsets(webRequest, addonOptions), origin: origin!, disableTelemetry: disableTelemetry!, diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index 17f172cf..960104fd 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -55,17 +55,19 @@ export const experimental_devServer: PresetPropertyFn< // Older Storybooks (≤10.3) don't pass `options.channel` to dev-server // presets — the rest of addon-mcp still works without channel replay, // so guard rather than crash. - if (changeDetectionEnabled && options.channel?.on) { - options.channel.on(REQUEST_REVIEW_STATE_EVENT, () => { - const state = getReviewState(); - if (state) { - options.channel?.emit?.(APPLY_REVIEW_STATE_EVENT, state); - } - }); - } else if (!options.channel?.on) { - logger.info( - '[addon-mcp] options.channel is unavailable on this Storybook version; review-state replay disabled.', - ); + if (changeDetectionEnabled) { + if (options.channel?.on) { + options.channel.on(REQUEST_REVIEW_STATE_EVENT, () => { + const state = getReviewState(); + if (state) { + options.channel?.emit?.(APPLY_REVIEW_STATE_EVENT, state); + } + }); + } else { + logger.info( + '[addon-mcp] options.channel is unavailable on this Storybook version; review-state replay disabled.', + ); + } } // Get composed Storybook refs from config @@ -128,6 +130,7 @@ export const experimental_devServer: PresetPropertyFn< res, options, addonOptions, + endpoint, sources, manifestProvider: createManifestProvider?.(req), compositionAuth, @@ -151,6 +154,7 @@ export const experimental_devServer: PresetPropertyFn< res, options, addonOptions, + endpoint, sources, manifestProvider: createManifestProvider?.(req), compositionAuth, diff --git a/packages/addon-mcp/src/tools/apply-review-state.test.ts b/packages/addon-mcp/src/tools/apply-review-state.test.ts index 755795b1..f3e1ebbe 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.test.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.test.ts @@ -41,11 +41,12 @@ describe('buildReviewUrl', () => { ); }); - it('uses trusted origin host and request path prefix for a proxied Storybook', () => { + it('uses the configured endpoint to recover a proxied Storybook root', () => { expect( buildReviewUrl({ origin: 'http://localhost:6006', - request: new Request('https://example.com/storybook/mcp'), + request: new Request('https://example.com/storybook/custom-mcp'), + endpoint: '/custom-mcp', }), ).toBe('http://localhost:6006/storybook/?path=/review/'); }); @@ -59,14 +60,8 @@ describe('buildReviewUrl', () => { ).toBe('http://localhost:6006/prefix/?path=/review/'); }); - it('handles a request served at the host root', () => { - expect(buildReviewUrl({ request: new Request('http://localhost:6006/mcp') })).toBe( - 'http://localhost:6006/?path=/review/', - ); - }); - it('throws when neither request nor origin is available', () => { - expect(() => buildReviewUrl({})).toThrow(/Cannot resolve the Storybook URL/); + expect(() => buildReviewUrl({} as any)).toThrow(/Cannot resolve the Storybook URL/); }); }); diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index 9c87db61..fe6ef5be 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -21,49 +21,36 @@ const ApplyReviewStateOutput = v.object({ ), }); -/** - * Resolve the Storybook manager root from the incoming MCP request. - * - * Security note: authority (scheme + host + port) comes from a trusted - * origin, not from request.url (which can be derived from the Host header). - * The request contributes only the path prefix (Storybook may be mounted at - * `/some/prefix/mcp`). - */ function storybookRootFromRequest( request: Request | undefined, trustedOrigin: string, + endpoint: string, ): string | undefined { if (!request?.url) return undefined; try { const url = new URL(request.url); - const rootPath = url.pathname.replace(/\/mcp\/?$/, ''); + const normalizedEndpoint = endpoint.replace(/\/$/, ''); + const rootPath = url.pathname.endsWith(normalizedEndpoint) + ? url.pathname.slice(0, -normalizedEndpoint.length) + : url.pathname.replace(/\/[^/]+\/?$/, '/'); return `${trustedOrigin.replace(/\/$/, '')}${rootPath}`; } catch { return undefined; } } -/** - * Build the URL of the Storybook review page. Uses the trusted context - * origin for host authority and preserves any request path prefix so this - * works when Storybook is mounted behind a reverse proxy. - */ -export function buildReviewUrl(ctx: { origin?: string; request?: Request }): string { - const requestOrigin = (() => { - if (!ctx.request?.url) return undefined; - try { - return new URL(ctx.request.url).origin; - } catch { - return undefined; - } - })(); - const trustedOrigin = ctx.origin ?? requestOrigin; +export function buildReviewUrl(ctx: { + origin: string; + request?: Request; + endpoint?: string; +}): string { + const trustedOrigin = ctx.origin; const root = ctx.request && trustedOrigin - ? storybookRootFromRequest(ctx.request, trustedOrigin) + ? storybookRootFromRequest(ctx.request, trustedOrigin, ctx.endpoint ?? '/mcp') : trustedOrigin; if (!root) { - throw new Error('Cannot resolve the Storybook URL: no request or origin in addon context.'); + throw new Error('Cannot resolve the Storybook URL: missing trusted origin in addon context.'); } return `${root.replace(/\/$/, '')}/?path=${REVIEW_PAGE_PATH}`; } @@ -92,7 +79,18 @@ Always include the returned reviewUrl in your final user-facing response so the }, async (input: ReviewState) => { try { - const reviewUrl = buildReviewUrl(server.ctx.custom ?? {}); + const customContext = server.ctx.custom; + if (!customContext?.origin) { + throw new Error( + 'Cannot resolve the Storybook URL: missing trusted origin in addon context.', + ); + } + + const reviewUrl = buildReviewUrl({ + origin: customContext.origin, + request: customContext.request, + endpoint: customContext.endpoint, + }); // Resolve the target repo's current git branch server-side and // attach it — the agent's payload doesn't carry it, but the diff --git a/packages/addon-mcp/src/types.ts b/packages/addon-mcp/src/types.ts index 8a020643..ad233297 100644 --- a/packages/addon-mcp/src/types.ts +++ b/packages/addon-mcp/src/types.ts @@ -48,6 +48,11 @@ export type AddonContext = StorybookContext & { */ options: Options; + /** + * The resolved MCP endpoint pathname for this Storybook instance. + */ + endpoint?: string; + /** * The origin URL of the running Storybook instance. * Typically http://localhost:{port} From edb6a5b0450d9a4724190c48b033c91eb1f7a2ad Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 13:07:26 +0200 Subject: [PATCH 14/26] introduce opening the reviewUrl in instructions --- .../src/instructions/build-server-instructions.test.ts | 4 ++-- .../addon-mcp/src/instructions/build-server-instructions.ts | 2 +- packages/addon-mcp/src/tools/apply-review-state.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts index c90318f1..f5b24842 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts @@ -20,7 +20,7 @@ describe('buildServerInstructions', () => { - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response. + - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback. ## Validation Workflow @@ -67,7 +67,7 @@ describe('buildServerInstructions', () => { - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response." + - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback." `); }); diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.ts b/packages/addon-mcp/src/instructions/build-server-instructions.ts index d7ccf729..903e6ac1 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.ts @@ -25,7 +25,7 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) .replace( '{{APPLY_REVIEW_STATE_STEP}}', changeDetection - ? "\n- After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page, and include the returned reviewUrl in your final response." + ? "\n- After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback." : '', ) .trim(), diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/apply-review-state.ts index fe6ef5be..ab8a9b7c 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/apply-review-state.ts @@ -112,7 +112,7 @@ Always include the returned reviewUrl in your final user-facing response so the content: [ { type: 'text' as const, - text: `Review applied: ${collectionCount} collection${collectionCount === 1 ? '' : 's'}, ${storyCount} stor${storyCount === 1 ? 'y' : 'ies'}. Open it at: ${reviewUrl}`, + text: `Review applied: ${collectionCount} collection${collectionCount === 1 ? '' : 's'}, ${storyCount} stor${storyCount === 1 ? 'y' : 'ies'}. Next step: if a browser-preview tool is available in this session (e.g. preview_eval or an equivalent navigate tool), open ${reviewUrl} in it so the user sees the review immediately. Otherwise include the URL in your final response.`, }, ], structuredContent: { reviewUrl }, From e73e0dea8364631d68bcc809d52a057ffbde550d Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 13:24:23 +0200 Subject: [PATCH 15/26] rename tools from *-review-state to *-review --- .changeset/cold-otters-wait.md | 2 +- .../tests/mcp-endpoint.e2e.test.ts | 6 +++--- packages/addon-mcp/src/constants.ts | 4 ++-- .../build-server-instructions.test.ts | 4 ++-- .../instructions/build-server-instructions.ts | 4 ++-- .../src/instructions/dev-instructions.md | 2 +- packages/addon-mcp/src/mcp-handler.ts | 4 ++-- packages/addon-mcp/src/preset.test.ts | 6 +++--- packages/addon-mcp/src/preset.ts | 10 +++++----- packages/addon-mcp/src/review-state-store.ts | 4 ++-- ...ew-state.test.ts => display-review.test.ts} | 18 +++++++++--------- ...apply-review-state.ts => display-review.ts} | 16 ++++++++-------- packages/addon-mcp/src/tools/tool-names.ts | 2 +- 13 files changed, 41 insertions(+), 41 deletions(-) rename packages/addon-mcp/src/tools/{apply-review-state.test.ts => display-review.test.ts} (89%) rename packages/addon-mcp/src/tools/{apply-review-state.ts => display-review.ts} (90%) diff --git a/.changeset/cold-otters-wait.md b/.changeset/cold-otters-wait.md index 43e520cd..211fb38d 100644 --- a/.changeset/cold-otters-wait.md +++ b/.changeset/cold-otters-wait.md @@ -2,4 +2,4 @@ "@storybook/addon-mcp": minor --- -Added the `apply-review-state` tool. The agent pushes a curated review of current changes and returns the review-page URL. Pairs with the `@storybook/addon-review` Storybook addon. +Added the `display-review` tool. The agent pushes a curated review of current changes and returns the review-page URL. Pairs with the `@storybook/addon-review` Storybook addon. diff --git a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts index 06a5b20a..a6425568 100644 --- a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts +++ b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts @@ -501,7 +501,7 @@ describe('MCP Endpoint E2E Tests', () => { ], "type": "object", }, - "name": "apply-review-state", + "name": "display-review", "outputSchema": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -515,7 +515,7 @@ describe('MCP Endpoint E2E Tests', () => { ], "type": "object", }, - "title": "Apply Storybook review state", + "title": "Display Storybook review", }, { "description": "Run story tests. @@ -1040,7 +1040,7 @@ describe('MCP Endpoint E2E Tests', () => { "preview-stories", "get-storybook-story-instructions", "get-changed-stories", - "apply-review-state", + "display-review", ] `); }); diff --git a/packages/addon-mcp/src/constants.ts b/packages/addon-mcp/src/constants.ts index 049d6376..81fb6b69 100644 --- a/packages/addon-mcp/src/constants.ts +++ b/packages/addon-mcp/src/constants.ts @@ -5,8 +5,8 @@ export const MCP_APP_SIZE_CHANGED_EVENT = 'storybook-mcp:size-changed'; * Channel events shared with `@storybook/addon-review` (cross-repo * contract). Keep in sync with that addon's `src/constants.ts`. */ -export const APPLY_REVIEW_STATE_EVENT = 'storybook/addon-review/apply-review-state'; -export const REQUEST_REVIEW_STATE_EVENT = 'storybook/addon-review/request-review-state'; +export const DISPLAY_REVIEW_EVENT = 'storybook/addon-review/display-review'; +export const REQUEST_REVIEW_EVENT = 'storybook/addon-review/request-review'; /** Storybook manager route the review page is registered at. */ export const REVIEW_PAGE_PATH = '/review/'; diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts index f5b24842..002d0054 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts @@ -20,7 +20,7 @@ describe('buildServerInstructions', () => { - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback. + - After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback. ## Validation Workflow @@ -67,7 +67,7 @@ describe('buildServerInstructions', () => { - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - - After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback." + - After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback." `); }); diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.ts b/packages/addon-mcp/src/instructions/build-server-instructions.ts index 903e6ac1..e1079bcc 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.ts @@ -23,9 +23,9 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) : 'After changing any component or story, call **preview-stories** to retrieve preview URLs.', ) .replace( - '{{APPLY_REVIEW_STATE_STEP}}', + '{{DISPLAY_REVIEW_STEP}}', changeDetection - ? "\n- After completing the change, call **apply-review-state** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback." + ? "\n- After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback." : '', ) .trim(), diff --git a/packages/addon-mcp/src/instructions/dev-instructions.md b/packages/addon-mcp/src/instructions/dev-instructions.md index 6d681d42..8072e145 100644 --- a/packages/addon-mcp/src/instructions/dev-instructions.md +++ b/packages/addon-mcp/src/instructions/dev-instructions.md @@ -4,4 +4,4 @@ - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - {{PREVIEW_STORIES_STEP}} - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. -- Always include every returned preview URL in your user-facing response so the user can verify the visual result.{{APPLY_REVIEW_STATE_STEP}} +- Always include every returned preview URL in your user-facing response so the user can verify the visual result.{{DISPLAY_REVIEW_STEP}} diff --git a/packages/addon-mcp/src/mcp-handler.ts b/packages/addon-mcp/src/mcp-handler.ts index 785ba672..cd41226a 100644 --- a/packages/addon-mcp/src/mcp-handler.ts +++ b/packages/addon-mcp/src/mcp-handler.ts @@ -4,7 +4,7 @@ import { HttpTransport } from '@tmcp/transport-http'; import pkgJson from '../package.json' with { type: 'json' }; import { addPreviewStoriesTool } from './tools/preview-stories.ts'; import { addGetChangedStoriesTool } from './tools/get-changed-stories.ts'; -import { addApplyReviewStateTool } from './tools/apply-review-state.ts'; +import { addDisplayReviewTool } from './tools/display-review.ts'; import { addGetUIBuildingInstructionsTool } from './tools/get-storybook-story-instructions.ts'; import { addListAllDocumentationTool, @@ -82,7 +82,7 @@ const initializeMCPServer = async (options: Options, multiSource?: boolean) => { if (changeDetectionEnabled) { await addGetChangedStoriesTool(server); - await addApplyReviewStateTool(server); + await addDisplayReviewTool(server); } // Register test addon tools diff --git a/packages/addon-mcp/src/preset.test.ts b/packages/addon-mcp/src/preset.test.ts index 63a851f7..4aa5b4d7 100644 --- a/packages/addon-mcp/src/preset.test.ts +++ b/packages/addon-mcp/src/preset.test.ts @@ -5,7 +5,7 @@ import { experimental_devServer } from './preset.ts'; import { STORYBOOK_MCP_PROXY_HEADER } from './auth/index.ts'; import * as mcpHandlerModule from './mcp-handler.ts'; import * as runStoryTests from './tools/run-story-tests.ts'; -import { REQUEST_REVIEW_STATE_EVENT } from './constants.ts'; +import { REQUEST_REVIEW_EVENT } from './constants.ts'; describe('experimental_devServer', () => { let mockApp: any; @@ -114,7 +114,7 @@ describe('experimental_devServer', () => { await (experimental_devServer as any)(mockApp, optionsWithChangeDetection); - expect(channel.on).toHaveBeenCalledWith(REQUEST_REVIEW_STATE_EVENT, expect.any(Function)); + expect(channel.on).toHaveBeenCalledWith(REQUEST_REVIEW_EVENT, expect.any(Function)); }); it('does not register review-state replay listener when changeDetection is disabled', async () => { @@ -134,7 +134,7 @@ describe('experimental_devServer', () => { await (experimental_devServer as any)(mockApp, optionsWithoutChangeDetection); - expect(channel.on).not.toHaveBeenCalledWith(REQUEST_REVIEW_STATE_EVENT, expect.any(Function)); + expect(channel.on).not.toHaveBeenCalledWith(REQUEST_REVIEW_EVENT, expect.any(Function)); }); it('should register /mcp GET endpoint', async () => { diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index 960104fd..3f1c7d6d 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -17,7 +17,7 @@ import { } from './auth/index.ts'; import { logger } from 'storybook/internal/node-logger'; import type { Source } from '@storybook/mcp'; -import { APPLY_REVIEW_STATE_EVENT, REQUEST_REVIEW_STATE_EVENT } from './constants.ts'; +import { DISPLAY_REVIEW_EVENT, REQUEST_REVIEW_EVENT } from './constants.ts'; import { getReviewState } from './review-state-store.ts'; import type { IncomingMessage, ServerResponse } from 'node:http'; @@ -49,18 +49,18 @@ export const experimental_devServer: PresetPropertyFn< // Replay the cached review state to any Storybook tab that mounts or // refreshes after the agent already pushed. The tab emits - // `request-review-state`; we re-emit the cached state so it converges. - // The cache is the module singleton shared with the apply-review-state tool. + // `request-review`; we re-emit the cached state so it converges. + // The cache is the module singleton shared with the display-review tool. // // Older Storybooks (≤10.3) don't pass `options.channel` to dev-server // presets — the rest of addon-mcp still works without channel replay, // so guard rather than crash. if (changeDetectionEnabled) { if (options.channel?.on) { - options.channel.on(REQUEST_REVIEW_STATE_EVENT, () => { + options.channel.on(REQUEST_REVIEW_EVENT, () => { const state = getReviewState(); if (state) { - options.channel?.emit?.(APPLY_REVIEW_STATE_EVENT, state); + options.channel?.emit?.(DISPLAY_REVIEW_EVENT, state); } }); } else { diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts index a106fe4e..d402caa5 100644 --- a/packages/addon-mcp/src/review-state-store.ts +++ b/packages/addon-mcp/src/review-state-store.ts @@ -1,9 +1,9 @@ /** * In-memory store for the agent-pushed review state. * - * The agent pushes a review via the `apply-review-state` MCP tool; the last + * The agent pushes a review via the `display-review` MCP tool; the last * one is cached here in module scope and replayed over the Storybook channel - * to any tab that asks (the `request-review-state` event). A dev-server + * to any tab that asks (the `request-review` event). A dev-server * restart clears it — server memory only, by design. * * This module owns the canonical valibot schema for the review contract; diff --git a/packages/addon-mcp/src/tools/apply-review-state.test.ts b/packages/addon-mcp/src/tools/display-review.test.ts similarity index 89% rename from packages/addon-mcp/src/tools/apply-review-state.test.ts rename to packages/addon-mcp/src/tools/display-review.test.ts index f3e1ebbe..290a0227 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.test.ts +++ b/packages/addon-mcp/src/tools/display-review.test.ts @@ -1,9 +1,9 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { McpServer } from 'tmcp'; import { ValibotJsonSchemaAdapter } from '@tmcp/adapter-valibot'; -import { addApplyReviewStateTool, buildReviewUrl } from './apply-review-state.ts'; -import { APPLY_REVIEW_STATE_TOOL_NAME } from './tool-names.ts'; -import { APPLY_REVIEW_STATE_EVENT } from '../constants.ts'; +import { addDisplayReviewTool, buildReviewUrl } from './display-review.ts'; +import { DISPLAY_REVIEW_TOOL_NAME } from './tool-names.ts'; +import { DISPLAY_REVIEW_EVENT } from '../constants.ts'; import { getReviewState, type ReviewState } from '../review-state-store.ts'; import type { AddonContext } from '../types.ts'; @@ -65,7 +65,7 @@ describe('buildReviewUrl', () => { }); }); -describe('applyReviewStateTool', () => { +describe('displayReviewTool', () => { let server: McpServer; let emitted: Array<{ event: string; payload: unknown }>; @@ -91,7 +91,7 @@ describe('applyReviewStateTool', () => { { name: 'test-server', version: '1.0.0', - description: 'Test server for apply-review-state tool', + description: 'Test server for display-review tool', }, { adapter, @@ -115,7 +115,7 @@ describe('applyReviewStateTool', () => { { sessionId: 'test-session' }, ); - await addApplyReviewStateTool(server); + await addDisplayReviewTool(server); }); async function callTool(args: ReviewState, custom: AddonContext) { @@ -124,7 +124,7 @@ describe('applyReviewStateTool', () => { jsonrpc: '2.0' as const, id: 1, method: 'tools/call', - params: { name: APPLY_REVIEW_STATE_TOOL_NAME, arguments: args }, + params: { name: DISPLAY_REVIEW_TOOL_NAME, arguments: args }, }, { sessionId: 'test-session', custom }, ); @@ -155,7 +155,7 @@ describe('applyReviewStateTool', () => { it('broadcasts the review over the Storybook channel', async () => { await callTool(sampleReview, makeContext()); - expect(emitted).toContainEqual({ event: APPLY_REVIEW_STATE_EVENT, payload: sampleReview }); + expect(emitted).toContainEqual({ event: DISPLAY_REVIEW_EVENT, payload: sampleReview }); }); it('builds a subpath-aware review URL from the incoming request', async () => { @@ -177,6 +177,6 @@ describe('applyReviewStateTool', () => { const expected = { ...sampleReview, branchName: 'feature/badge-pink' }; expect(getReviewState()).toEqual(expected); - expect(emitted).toContainEqual({ event: APPLY_REVIEW_STATE_EVENT, payload: expected }); + expect(emitted).toContainEqual({ event: DISPLAY_REVIEW_EVENT, payload: expected }); }); }); diff --git a/packages/addon-mcp/src/tools/apply-review-state.ts b/packages/addon-mcp/src/tools/display-review.ts similarity index 90% rename from packages/addon-mcp/src/tools/apply-review-state.ts rename to packages/addon-mcp/src/tools/display-review.ts index ab8a9b7c..d049956f 100644 --- a/packages/addon-mcp/src/tools/apply-review-state.ts +++ b/packages/addon-mcp/src/tools/display-review.ts @@ -8,11 +8,11 @@ import { type ReviewState, type StoredReviewState, } from '../review-state-store.ts'; -import { APPLY_REVIEW_STATE_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; -import { APPLY_REVIEW_STATE_TOOL_NAME } from './tool-names.ts'; +import { DISPLAY_REVIEW_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; +import { DISPLAY_REVIEW_TOOL_NAME } from './tool-names.ts'; import { currentGitBranch } from '../utils/git-branch.ts'; -const ApplyReviewStateOutput = v.object({ +const DisplayReviewOutput = v.object({ reviewUrl: v.pipe( v.string(), v.description( @@ -55,11 +55,11 @@ export function buildReviewUrl(ctx: { return `${root.replace(/\/$/, '')}/?path=${REVIEW_PAGE_PATH}`; } -export async function addApplyReviewStateTool(server: McpServer) { +export async function addDisplayReviewTool(server: McpServer) { server.tool( { - name: APPLY_REVIEW_STATE_TOOL_NAME, - title: 'Apply Storybook review state', + name: DISPLAY_REVIEW_TOOL_NAME, + title: 'Display Storybook review', description: `Push a curated review of the current change to Storybook's review page. After you finish a UI code change, call this to help the user spot-check it. Provide: @@ -74,7 +74,7 @@ The \`kind\` labels are for structured review grouping and UI behavior; do not r Always include the returned reviewUrl in your final user-facing response so the user can open it.`, schema: ReviewStateSchema, - outputSchema: ApplyReviewStateOutput, + outputSchema: DisplayReviewOutput, enabled: () => server.ctx.custom?.toolsets?.dev ?? true, }, async (input: ReviewState) => { @@ -103,7 +103,7 @@ Always include the returned reviewUrl in your final user-facing response so the // Broadcast to all connected Storybook tabs. A warm tab navigates // to the review page; a cold start relies on the returned URL. - server.ctx.custom?.options?.channel?.emit(APPLY_REVIEW_STATE_EVENT, state); + server.ctx.custom?.options?.channel?.emit(DISPLAY_REVIEW_EVENT, state); const collectionCount = state.collections.length; const storyCount = state.collections.reduce((n, c) => n + c.storyIds.length, 0); diff --git a/packages/addon-mcp/src/tools/tool-names.ts b/packages/addon-mcp/src/tools/tool-names.ts index 809dcbfa..6d87dd13 100644 --- a/packages/addon-mcp/src/tools/tool-names.ts +++ b/packages/addon-mcp/src/tools/tool-names.ts @@ -6,4 +6,4 @@ export const PREVIEW_STORIES_TOOL_NAME = 'preview-stories'; export const GET_CHANGED_STORIES_TOOL_NAME = 'get-changed-stories'; export const GET_UI_BUILDING_INSTRUCTIONS_TOOL_NAME = 'get-storybook-story-instructions'; export const RUN_STORY_TESTS_TOOL_NAME = 'run-story-tests'; -export const APPLY_REVIEW_STATE_TOOL_NAME = 'apply-review-state'; +export const DISPLAY_REVIEW_TOOL_NAME = 'display-review'; From 2d0b6bc1973a5bc49dd6b156723719883ad01cf9 Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 14:14:17 +0200 Subject: [PATCH 16/26] ci: re-trigger workflows From 7bc54778f211b38a06cedd9003ac83a48a7192ad Mon Sep 17 00:00:00 2001 From: yannbf Date: Tue, 26 May 2026 14:56:02 +0200 Subject: [PATCH 17/26] ci: re-trigger workflows From d225bf4b59e8b93b0a4a75306398f4c86e541ef2 Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 14:22:23 +0200 Subject: [PATCH 18/26] Apply review feedback, move logic from MCP to addon review, gate features, simplify code --- .../tests/mcp-endpoint.e2e.test.ts | 142 +----------------- packages/addon-mcp/src/constants.ts | 7 +- .../build-server-instructions.test.ts | 24 +++ .../instructions/build-server-instructions.ts | 4 +- packages/addon-mcp/src/mcp-handler.ts | 6 + packages/addon-mcp/src/preset.test.ts | 30 +--- packages/addon-mcp/src/preset.ts | 27 ---- packages/addon-mcp/src/review-state-store.ts | 107 ------------- .../src/tools/display-review.test.ts | 33 +--- .../addon-mcp/src/tools/display-review.ts | 109 +++++++++++--- .../src/utils/fetch-story-index.test.ts | 136 ++--------------- .../addon-mcp/src/utils/fetch-story-index.ts | 84 ++--------- .../addon-mcp/src/utils/git-branch.test.ts | 55 ------- packages/addon-mcp/src/utils/git-branch.ts | 21 --- .../src/utils/is-review-available.test.ts | 74 +++++++++ .../src/utils/is-review-available.ts | 35 +++++ 16 files changed, 265 insertions(+), 629 deletions(-) delete mode 100644 packages/addon-mcp/src/review-state-store.ts delete mode 100644 packages/addon-mcp/src/utils/git-branch.test.ts delete mode 100644 packages/addon-mcp/src/utils/git-branch.ts create mode 100644 packages/addon-mcp/src/utils/is-review-available.test.ts create mode 100644 packages/addon-mcp/src/utils/is-review-available.ts diff --git a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts index a6425568..f4a4b09a 100644 --- a/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts +++ b/apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts @@ -100,7 +100,7 @@ describe('MCP Endpoint E2E Tests', () => { expect(response.result).toHaveProperty('tools'); // Dev, docs, and test tools should be present - expect(response.result.tools).toHaveLength(8); + expect(response.result.tools).toHaveLength(7); expect(response.result.tools).toMatchInlineSnapshot(` [ @@ -378,145 +378,6 @@ describe('MCP Endpoint E2E Tests', () => { "name": "get-changed-stories", "title": "Get changed stories metadata", }, - { - "description": "Push a curated review of the current change to Storybook's review page. - - After you finish a UI code change, call this to help the user spot-check it. Provide: - - title: a PR-style title for the change — short and specific. - - description: a one-line summary of what changed and where to start reviewing. - - collections: titled groups of representative story IDs. Give each a concise, PR-dense title, a one-sentence rationale, and — when you can tell — a kind ("atomic" for the directly changed component, "consumer" for direct dependents, "transitive" for pages/containers, "catch-all" otherwise). - - changedFiles: the files you edited (most central first). - - diffHunks: the actual diff of your change (you made it — include the hunks). - - storyMeta: optional per-story { depth, chain }. - - The \`kind\` labels are for structured review grouping and UI behavior; do not repeat these labels verbatim in user-facing prose unless the user explicitly asks for them. - - Always include the returned reviewUrl in your final user-facing response so the user can open it.", - "inputSchema": { - "$schema": "http://json-schema.org/draft-07/schema#", - "properties": { - "changedFiles": { - "description": "Paths of the files you changed, most central first.", - "items": { - "type": "string", - }, - "type": "array", - }, - "collections": { - "items": { - "properties": { - "kind": { - "description": "Semantic role of this collection in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.", - "enum": [ - "atomic", - "consumer", - "transitive", - "catch-all", - ], - "type": "string", - }, - "rationale": { - "description": "One sentence explaining why these stories are grouped together.", - "type": "string", - }, - "storyIds": { - "description": "Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.", - "items": { - "type": "string", - }, - "type": "array", - }, - "title": { - "description": "Short, PR-dense title for this collection, e.g. "Direct Button importers".", - "type": "string", - }, - }, - "required": [ - "title", - "rationale", - "storyIds", - ], - "type": "object", - }, - "type": "array", - }, - "description": { - "description": "One-line summary of what changed and where to start reviewing.", - "type": "string", - }, - "diffHunks": { - "description": "The actual diff hunks of your change, shown in the review page.", - "items": { - "properties": { - "hunk": { - "description": "Unified-diff text for this hunk (with +/- line prefixes).", - "type": "string", - }, - "path": { - "description": "Path of the changed file this hunk belongs to.", - "type": "string", - }, - }, - "required": [ - "path", - "hunk", - ], - "type": "object", - }, - "type": "array", - }, - "storyMeta": { - "additionalProperties": { - "properties": { - "chain": { - "description": "Ordered intermediate file paths between the story file and the changed file, excluding both endpoints. Empty/omitted means a direct import.", - "items": { - "type": "string", - }, - "type": "array", - }, - "depth": { - "description": "Graph distance from the changed file(s) to this story (0 = the changed component itself).", - "type": "number", - }, - }, - "required": [], - "type": "object", - }, - "description": "Optional per-story metadata keyed by story ID: { depth, chain }.", - "propertyNames": { - "type": "string", - }, - "type": "object", - }, - "title": { - "description": "PR-style title for the change — short and specific, e.g. "Recolour the primary button".", - "type": "string", - }, - }, - "required": [ - "title", - "description", - "collections", - ], - "type": "object", - }, - "name": "display-review", - "outputSchema": { - "$schema": "http://json-schema.org/draft-07/schema#", - "properties": { - "reviewUrl": { - "description": "URL of the Storybook review page. Always include this URL in your final user-facing response so the user can open it directly.", - "type": "string", - }, - }, - "required": [ - "reviewUrl", - ], - "type": "object", - }, - "title": "Display Storybook review", - }, { "description": "Run story tests. Provide stories for focused runs (faster while iterating), @@ -1040,7 +901,6 @@ describe('MCP Endpoint E2E Tests', () => { "preview-stories", "get-storybook-story-instructions", "get-changed-stories", - "display-review", ] `); }); diff --git a/packages/addon-mcp/src/constants.ts b/packages/addon-mcp/src/constants.ts index 81fb6b69..d99df331 100644 --- a/packages/addon-mcp/src/constants.ts +++ b/packages/addon-mcp/src/constants.ts @@ -2,11 +2,10 @@ export const MCP_APP_PARAM = 'mcp-app'; export const MCP_APP_SIZE_CHANGED_EVENT = 'storybook-mcp:size-changed'; /** - * Channel events shared with `@storybook/addon-review` (cross-repo - * contract). Keep in sync with that addon's `src/constants.ts`. + * Channel event shared with `@storybook/addon-review` (cross-repo contract). + * Emitted by the `display-review` tool to hand the agent's payload off to the addon-review server preset, where the state is handled */ -export const DISPLAY_REVIEW_EVENT = 'storybook/addon-review/display-review'; -export const REQUEST_REVIEW_EVENT = 'storybook/addon-review/request-review'; +export const PUSH_REVIEW_EVENT = 'storybook/addon-review/push-review'; /** Storybook manager route the review page is registered at. */ export const REVIEW_PAGE_PATH = '/review/'; diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts index 002d0054..d01089b1 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts @@ -8,6 +8,7 @@ describe('buildServerInstructions', () => { testEnabled: true, docsEnabled: true, changeDetectionEnabled: true, + reviewEnabled: true, }); expect(instructions).toMatchInlineSnapshot(` @@ -55,6 +56,7 @@ describe('buildServerInstructions', () => { testEnabled: false, docsEnabled: false, changeDetectionEnabled: true, + reviewEnabled: true, }); expect(instructions).toMatchInlineSnapshot(` @@ -92,6 +94,28 @@ describe('buildServerInstructions', () => { `); }); + it('omits display-review step when reviewEnabled is false even with change detection on', () => { + const instructions = buildServerInstructions({ + devEnabled: true, + testEnabled: false, + docsEnabled: false, + changeDetectionEnabled: true, + reviewEnabled: false, + }); + + expect(instructions).toMatchInlineSnapshot(` + "Follow these workflows when working with UI and/or Storybook. + + ## UI Building and Story Writing Workflow + + - Before creating or editing components or stories, call **get-storybook-story-instructions**. + - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. + - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. + - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. + - Always include every returned preview URL in your user-facing response so the user can verify the visual result." + `); + }); + it('builds a coherent instruction set for docs only', () => { const instructions = buildServerInstructions({ devEnabled: false, diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.ts b/packages/addon-mcp/src/instructions/build-server-instructions.ts index e1079bcc..2b784760 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.ts @@ -7,6 +7,7 @@ export type BuildServerInstructionsOptions = { testEnabled: boolean; docsEnabled: boolean; changeDetectionEnabled?: boolean; + reviewEnabled?: boolean; }; export function buildServerInstructions(options: BuildServerInstructionsOptions): string { @@ -14,6 +15,7 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) if (options.devEnabled) { const changeDetection = options.changeDetectionEnabled ?? false; + const reviewEnabled = options.reviewEnabled ?? false; sections.push( devInstructions .replace( @@ -24,7 +26,7 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) ) .replace( '{{DISPLAY_REVIEW_STEP}}', - changeDetection + reviewEnabled ? "\n- After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback." : '', ) diff --git a/packages/addon-mcp/src/mcp-handler.ts b/packages/addon-mcp/src/mcp-handler.ts index cd41226a..03a38c16 100644 --- a/packages/addon-mcp/src/mcp-handler.ts +++ b/packages/addon-mcp/src/mcp-handler.ts @@ -19,6 +19,7 @@ import { collectTelemetry } from './telemetry.ts'; import type { AddonContext, AddonOptionsOutput } from './types.ts'; import { logger } from 'storybook/internal/node-logger'; import { getManifestStatus } from './tools/is-manifest-available.ts'; +import { getReviewStatus } from './utils/is-review-available.ts'; import { addRunStoryTestsTool, getAddonVitestConstants } from './tools/run-story-tests.ts'; import { estimateTokens } from './utils/estimate-tokens.ts'; import { isAddonA11yEnabled } from './utils/is-addon-a11y-enabled.ts'; @@ -41,6 +42,7 @@ const initializeMCPServer = async (options: Options, multiSource?: boolean) => { // Determine tool availability before creating server so instructions can be tailored const addonVitestConstants = await getAddonVitestConstants(); const manifestStatus = await getManifestStatus(options); + const reviewStatus = await getReviewStatus(options); a11yEnabled = await isAddonA11yEnabled(options); let server: McpServer; @@ -53,6 +55,7 @@ const initializeMCPServer = async (options: Options, multiSource?: boolean) => { testEnabled: (server?.ctx.custom?.toolsets?.test ?? true) && !!addonVitestConstants, docsEnabled: (server?.ctx.custom?.toolsets?.docs ?? true) && manifestStatus.available, changeDetectionEnabled, + reviewEnabled: reviewStatus.available, }); }, capabilities: { @@ -82,6 +85,9 @@ const initializeMCPServer = async (options: Options, multiSource?: boolean) => { if (changeDetectionEnabled) { await addGetChangedStoriesTool(server); + } + + if (reviewStatus.available) { await addDisplayReviewTool(server); } diff --git a/packages/addon-mcp/src/preset.test.ts b/packages/addon-mcp/src/preset.test.ts index 4aa5b4d7..528f549c 100644 --- a/packages/addon-mcp/src/preset.test.ts +++ b/packages/addon-mcp/src/preset.test.ts @@ -5,7 +5,6 @@ import { experimental_devServer } from './preset.ts'; import { STORYBOOK_MCP_PROXY_HEADER } from './auth/index.ts'; import * as mcpHandlerModule from './mcp-handler.ts'; import * as runStoryTests from './tools/run-story-tests.ts'; -import { REQUEST_REVIEW_EVENT } from './constants.ts'; describe('experimental_devServer', () => { let mockApp: any; @@ -97,9 +96,9 @@ describe('experimental_devServer', () => { expect(mcpHandler).toBeDefined(); }); - it('registers review-state replay listener only when changeDetection is enabled', async () => { + it('does not register any review-related channel listeners (handled by addon-review preset)', async () => { const channel = { on: vi.fn(), emit: vi.fn() }; - const optionsWithChangeDetection = { + const optionsWithReview = { ...mockOptions, channel, presets: { @@ -107,34 +106,17 @@ describe('experimental_devServer', () => { if (key === 'features') { return Promise.resolve({ changeDetection: true }); } - return Promise.resolve(undefined); - }), - }, - } as unknown as Options; - - await (experimental_devServer as any)(mockApp, optionsWithChangeDetection); - - expect(channel.on).toHaveBeenCalledWith(REQUEST_REVIEW_EVENT, expect.any(Function)); - }); - - it('does not register review-state replay listener when changeDetection is disabled', async () => { - const channel = { on: vi.fn(), emit: vi.fn() }; - const optionsWithoutChangeDetection = { - ...mockOptions, - channel, - presets: { - apply: vi.fn((key: string) => { - if (key === 'features') { - return Promise.resolve({ changeDetection: false }); + if (key === 'addons') { + return Promise.resolve(['@storybook/addon-review']); } return Promise.resolve(undefined); }), }, } as unknown as Options; - await (experimental_devServer as any)(mockApp, optionsWithoutChangeDetection); + await (experimental_devServer as any)(mockApp, optionsWithReview); - expect(channel.on).not.toHaveBeenCalledWith(REQUEST_REVIEW_EVENT, expect.any(Function)); + expect(channel.on).not.toHaveBeenCalled(); }); it('should register /mcp GET endpoint', async () => { diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index 3f1c7d6d..892fe1f3 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -17,8 +17,6 @@ import { } from './auth/index.ts'; import { logger } from 'storybook/internal/node-logger'; import type { Source } from '@storybook/mcp'; -import { DISPLAY_REVIEW_EVENT, REQUEST_REVIEW_EVENT } from './constants.ts'; -import { getReviewState } from './review-state-store.ts'; import type { IncomingMessage, ServerResponse } from 'node:http'; const DEFAULT_MCP_ENDPOINT = '/mcp'; @@ -41,35 +39,10 @@ export const experimental_devServer: PresetPropertyFn< endpoint: options.endpoint, toolsets: options.toolsets ?? {}, }); - const features = await options.presets.apply('features', {}); - const changeDetectionEnabled = features?.changeDetection ?? false; const origin = `http://localhost:${options.port}`; const endpoint = addonOptions.endpoint ?? DEFAULT_MCP_ENDPOINT; - // Replay the cached review state to any Storybook tab that mounts or - // refreshes after the agent already pushed. The tab emits - // `request-review`; we re-emit the cached state so it converges. - // The cache is the module singleton shared with the display-review tool. - // - // Older Storybooks (≤10.3) don't pass `options.channel` to dev-server - // presets — the rest of addon-mcp still works without channel replay, - // so guard rather than crash. - if (changeDetectionEnabled) { - if (options.channel?.on) { - options.channel.on(REQUEST_REVIEW_EVENT, () => { - const state = getReviewState(); - if (state) { - options.channel?.emit?.(DISPLAY_REVIEW_EVENT, state); - } - }); - } else { - logger.info( - '[addon-mcp] options.channel is unavailable on this Storybook version; review-state replay disabled.', - ); - } - } - // Get composed Storybook refs from config const refs = await getRefsFromConfig(options); const compositionAuth = new CompositionAuth(); diff --git a/packages/addon-mcp/src/review-state-store.ts b/packages/addon-mcp/src/review-state-store.ts deleted file mode 100644 index d402caa5..00000000 --- a/packages/addon-mcp/src/review-state-store.ts +++ /dev/null @@ -1,107 +0,0 @@ -/** - * In-memory store for the agent-pushed review state. - * - * The agent pushes a review via the `display-review` MCP tool; the last - * one is cached here in module scope and replayed over the Storybook channel - * to any tab that asks (the `request-review` event). A dev-server - * restart clears it — server memory only, by design. - * - * This module owns the canonical valibot schema for the review contract; - * `@storybook/addon-review` duplicates the inferred TS shape on its - * side (it only renders, so it needs the type, not the validator). - */ -import * as v from 'valibot'; - -export const ReviewCollectionSchema = v.object({ - title: v.pipe( - v.string(), - v.description('Short, PR-dense title for this collection, e.g. "Direct Button importers".'), - ), - rationale: v.pipe( - v.string(), - v.description('One sentence explaining why these stories are grouped together.'), - ), - storyIds: v.pipe( - v.array(v.string()), - v.description( - 'Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.', - ), - ), - kind: v.pipe( - v.optional(v.picklist(['atomic', 'consumer', 'transitive', 'catch-all'])), - v.description( - 'Semantic role of this collection in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.', - ), - ), -}); - -const StoryMetaSchema = v.object({ - depth: v.pipe( - v.optional(v.number()), - v.description( - 'Graph distance from the changed file(s) to this story (0 = the changed component itself).', - ), - ), - chain: v.pipe( - v.optional(v.array(v.string())), - v.description( - 'Ordered intermediate file paths between the story file and the changed file, excluding both endpoints. Empty/omitted means a direct import.', - ), - ), -}); - -const DiffHunkSchema = v.object({ - path: v.pipe(v.string(), v.description('Path of the changed file this hunk belongs to.')), - hunk: v.pipe( - v.string(), - v.description('Unified-diff text for this hunk (with +/- line prefixes).'), - ), -}); - -export const ReviewStateSchema = v.object({ - title: v.pipe( - v.string(), - v.description( - 'PR-style title for the change — short and specific, e.g. "Recolour the primary button".', - ), - ), - description: v.pipe( - v.string(), - v.description('One-line summary of what changed and where to start reviewing.'), - ), - collections: v.array(ReviewCollectionSchema), - changedFiles: v.pipe( - v.optional(v.array(v.string())), - v.description('Paths of the files you changed, most central first.'), - ), - diffHunks: v.pipe( - v.optional(v.array(DiffHunkSchema)), - v.description('The actual diff hunks of your change, shown in the review page.'), - ), - storyMeta: v.pipe( - v.optional(v.record(v.string(), StoryMetaSchema)), - v.description('Optional per-story metadata keyed by story ID: { depth, chain }.'), - ), -}); - -export type ReviewCollection = v.InferOutput; -export type ReviewState = v.InferOutput; - -/** - * A pushed `ReviewState` plus server-added metadata. `branchName` is the - * target repo's current git branch, resolved server-side at push time — - * the agent's payload does not (and should not) carry it. - */ -export type StoredReviewState = ReviewState & { - branchName?: string; -}; - -let cached: StoredReviewState | undefined; - -export function setReviewState(state: StoredReviewState): void { - cached = state; -} - -export function getReviewState(): StoredReviewState | undefined { - return cached; -} diff --git a/packages/addon-mcp/src/tools/display-review.test.ts b/packages/addon-mcp/src/tools/display-review.test.ts index 290a0227..909d1484 100644 --- a/packages/addon-mcp/src/tools/display-review.test.ts +++ b/packages/addon-mcp/src/tools/display-review.test.ts @@ -1,19 +1,11 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach } from 'vitest'; import { McpServer } from 'tmcp'; import { ValibotJsonSchemaAdapter } from '@tmcp/adapter-valibot'; -import { addDisplayReviewTool, buildReviewUrl } from './display-review.ts'; +import { addDisplayReviewTool, buildReviewUrl, type ReviewState } from './display-review.ts'; import { DISPLAY_REVIEW_TOOL_NAME } from './tool-names.ts'; -import { DISPLAY_REVIEW_EVENT } from '../constants.ts'; -import { getReviewState, type ReviewState } from '../review-state-store.ts'; +import { PUSH_REVIEW_EVENT } from '../constants.ts'; import type { AddonContext } from '../types.ts'; -// The tool resolves the target repo's git branch server-side. Mock it so -// the unit tests don't depend on the branch this repo happens to be on. -const { mockCurrentGitBranch } = vi.hoisted(() => ({ mockCurrentGitBranch: vi.fn() })); -vi.mock('../utils/git-branch.ts', () => ({ - currentGitBranch: (...args: unknown[]) => mockCurrentGitBranch(...args), -})); - const sampleReview: ReviewState = { title: 'Recolour the primary button', description: 'Button background changed from blue to green.', @@ -84,8 +76,6 @@ describe('displayReviewTool', () => { beforeEach(async () => { emitted = []; - mockCurrentGitBranch.mockReset(); - mockCurrentGitBranch.mockResolvedValue(undefined); const adapter = new ValibotJsonSchemaAdapter(); server = new McpServer( { @@ -142,7 +132,7 @@ describe('displayReviewTool', () => { ).result; } - it('stores the review state and returns the review URL', async () => { + it('returns the review URL and a human summary', async () => { const response = await callTool(sampleReview, makeContext()); const result = getResult(response); @@ -150,12 +140,11 @@ describe('displayReviewTool', () => { expect(result?.structuredContent?.reviewUrl).toBe('http://localhost:6006/?path=/review/'); expect(result?.content?.[0]?.text).toContain('2 collections, 3 stories'); expect(result?.content?.[0]?.text).toContain('http://localhost:6006/?path=/review/'); - expect(getReviewState()).toEqual(sampleReview); }); - it('broadcasts the review over the Storybook channel', async () => { + it('hands the payload off to addon-review via the PUSH_REVIEW channel event', async () => { await callTool(sampleReview, makeContext()); - expect(emitted).toContainEqual({ event: DISPLAY_REVIEW_EVENT, payload: sampleReview }); + expect(emitted).toEqual([{ event: PUSH_REVIEW_EVENT, payload: sampleReview }]); }); it('builds a subpath-aware review URL from the incoming request', async () => { @@ -169,14 +158,4 @@ describe('displayReviewTool', () => { 'http://localhost:6006/design-system/?path=/review/', ); }); - - it('attaches the target repo git branch resolved server-side', async () => { - mockCurrentGitBranch.mockResolvedValue('feature/badge-pink'); - - await callTool(sampleReview, makeContext()); - - const expected = { ...sampleReview, branchName: 'feature/badge-pink' }; - expect(getReviewState()).toEqual(expected); - expect(emitted).toContainEqual({ event: DISPLAY_REVIEW_EVENT, payload: expected }); - }); }); diff --git a/packages/addon-mcp/src/tools/display-review.ts b/packages/addon-mcp/src/tools/display-review.ts index d049956f..4b1a7480 100644 --- a/packages/addon-mcp/src/tools/display-review.ts +++ b/packages/addon-mcp/src/tools/display-review.ts @@ -2,15 +2,90 @@ import type { McpServer } from 'tmcp'; import * as v from 'valibot'; import type { AddonContext } from '../types.ts'; import { errorToMCPContent } from '../utils/errors.ts'; -import { - ReviewStateSchema, - setReviewState, - type ReviewState, - type StoredReviewState, -} from '../review-state-store.ts'; -import { DISPLAY_REVIEW_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; +import { PUSH_REVIEW_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; import { DISPLAY_REVIEW_TOOL_NAME } from './tool-names.ts'; -import { currentGitBranch } from '../utils/git-branch.ts'; + +/** + * Canonical schema for the agent-pushed review payload. + * + * Server-side state caching and git-branch enrichment live on the + * `@storybook/addon-review` server preset, not here — this tool just + * validates the agent's input and forwards it over the channel. + */ +export const ReviewCollectionSchema = v.object({ + title: v.pipe( + v.string(), + v.description('Short, PR-dense title for this collection, e.g. "Direct Button importers".'), + ), + rationale: v.pipe( + v.string(), + v.description('One sentence explaining why these stories are grouped together.'), + ), + storyIds: v.pipe( + v.array(v.string()), + v.description( + 'Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.', + ), + ), + kind: v.pipe( + v.optional(v.picklist(['atomic', 'consumer', 'transitive', 'catch-all'])), + v.description( + 'Semantic role of this collection in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.', + ), + ), +}); + +const StoryMetaSchema = v.object({ + depth: v.pipe( + v.optional(v.number()), + v.description( + 'Graph distance from the changed file(s) to this story (0 = the changed component itself).', + ), + ), + chain: v.pipe( + v.optional(v.array(v.string())), + v.description( + 'Ordered intermediate file paths between the story file and the changed file, excluding both endpoints. Empty/omitted means a direct import.', + ), + ), +}); + +const DiffHunkSchema = v.object({ + path: v.pipe(v.string(), v.description('Path of the changed file this hunk belongs to.')), + hunk: v.pipe( + v.string(), + v.description('Unified-diff text for this hunk (with +/- line prefixes).'), + ), +}); + +export const ReviewStateSchema = v.object({ + title: v.pipe( + v.string(), + v.description( + 'PR-style title for the change — short and specific, e.g. "Recolour the primary button".', + ), + ), + description: v.pipe( + v.string(), + v.description('One-line summary of what changed and where to start reviewing.'), + ), + collections: v.array(ReviewCollectionSchema), + changedFiles: v.pipe( + v.optional(v.array(v.string())), + v.description('Paths of the files you changed, most central first.'), + ), + diffHunks: v.pipe( + v.optional(v.array(DiffHunkSchema)), + v.description('The actual diff hunks of your change, shown in the review page.'), + ), + storyMeta: v.pipe( + v.optional(v.record(v.string(), StoryMetaSchema)), + v.description('Optional per-story metadata keyed by story ID: { depth, chain }.'), + ), +}); + +export type ReviewCollection = v.InferOutput; +export type ReviewState = v.InferOutput; const DisplayReviewOutput = v.object({ reviewUrl: v.pipe( @@ -92,21 +167,11 @@ Always include the returned reviewUrl in your final user-facing response so the endpoint: customContext.endpoint, }); - // Resolve the target repo's current git branch server-side and - // attach it — the agent's payload doesn't carry it, but the - // review page shows it. Best-effort: omitted on a detached HEAD - // or a non-git target. - const branchName = await currentGitBranch(process.cwd()); - const state: StoredReviewState = branchName ? { ...input, branchName } : input; - - setReviewState(state); - - // Broadcast to all connected Storybook tabs. A warm tab navigates - // to the review page; a cold start relies on the returned URL. - server.ctx.custom?.options?.channel?.emit(DISPLAY_REVIEW_EVENT, state); + // Hand the payload off to @storybook/addon-review + server.ctx.custom?.options?.channel?.emit(PUSH_REVIEW_EVENT, input); - const collectionCount = state.collections.length; - const storyCount = state.collections.reduce((n, c) => n + c.storyIds.length, 0); + const collectionCount = input.collections.length; + const storyCount = input.collections.reduce((n, c) => n + c.storyIds.length, 0); return { content: [ diff --git a/packages/addon-mcp/src/utils/fetch-story-index.test.ts b/packages/addon-mcp/src/utils/fetch-story-index.test.ts index da679798..d2abcf3e 100644 --- a/packages/addon-mcp/src/utils/fetch-story-index.test.ts +++ b/packages/addon-mcp/src/utils/fetch-story-index.test.ts @@ -6,6 +6,7 @@ describe('fetchStoryIndex', () => { const originalFetch = global.fetch; beforeEach(() => { + // Mock fetch globally global.fetch = vi.fn(); }); @@ -28,123 +29,28 @@ describe('fetchStoryIndex', () => { expect(Object.keys(result.entries)).toHaveLength(3); }); - it('should not retry on 404 (non-transient) and include status + statusText in error', async () => { + it('should throw error on 404 response', async () => { const mockFetch = global.fetch as any; mockFetch.mockResolvedValue({ ok: false, status: 404, statusText: 'Not Found', - text: async () => '', }); - await expect(fetchStoryIndex('http://localhost:6006', { sleep: vi.fn() })).rejects.toThrow( - /Failed to fetch story index: 404 Not Found/, - ); - expect(mockFetch).toHaveBeenCalledTimes(1); - }); - - it('should retry transient 5xx responses up to maxAttempts and succeed on a later attempt', async () => { - const mockFetch = global.fetch as any; - mockFetch - .mockResolvedValueOnce({ - ok: false, - status: 500, - statusText: 'Internal Server Error', - text: async () => 'Vite: failed to compile preview.tsx', - }) - .mockResolvedValueOnce({ - ok: false, - status: 503, - statusText: 'Service Unavailable', - text: async () => '', - }) - .mockResolvedValueOnce({ - ok: true, - json: async () => smallStoryIndexFixture, - }); - const sleep = vi.fn().mockResolvedValue(undefined); - - const result = await fetchStoryIndex('http://localhost:6006', { - maxAttempts: 3, - baseBackoffMs: 100, - sleep, - }); - - expect(result).toEqual(smallStoryIndexFixture); - expect(mockFetch).toHaveBeenCalledTimes(3); - // Backoff is `attempt * baseBackoffMs` — first wait 100ms, second wait 200ms. - expect(sleep).toHaveBeenNthCalledWith(1, 100); - expect(sleep).toHaveBeenNthCalledWith(2, 200); - }); - - it('should give up after maxAttempts of transient errors and include body snippet + attempt count in message', async () => { - const mockFetch = global.fetch as any; - mockFetch.mockResolvedValue({ - ok: false, - status: 500, - statusText: 'Internal Server Error', - text: async () => 'Vite: failed to compile preview.tsx (export StoreProvider not found)', - }); - const sleep = vi.fn().mockResolvedValue(undefined); + const origin = 'http://localhost:6006'; - await expect( - fetchStoryIndex('http://localhost:6006', { maxAttempts: 3, baseBackoffMs: 50, sleep }), - ).rejects.toThrow( - /500 Internal Server Error — Vite: failed to compile preview\.tsx \(export StoreProvider not found\) \(attempt 3\/3, transient — Storybook may be mid-recompile\)/, + await expect(fetchStoryIndex(origin)).rejects.toThrow( + 'Failed to fetch story index: 404 Not Found', ); - expect(mockFetch).toHaveBeenCalledTimes(3); }); - it('should not retry on 4xx non-transient codes even with high maxAttempts', async () => { - const mockFetch = global.fetch as any; - mockFetch.mockResolvedValue({ - ok: false, - status: 401, - statusText: 'Unauthorized', - text: async () => '', - }); - const sleep = vi.fn(); - - await expect( - fetchStoryIndex('http://localhost:6006', { maxAttempts: 5, sleep }), - ).rejects.toThrow(/401 Unauthorized.*non-transient — giving up/); - expect(mockFetch).toHaveBeenCalledTimes(1); - expect(sleep).not.toHaveBeenCalled(); - }); - - it('should retry on 429 (too many requests) and 408 (timeout)', async () => { - const mockFetch = global.fetch as any; - mockFetch - .mockResolvedValueOnce({ - ok: false, - status: 429, - statusText: 'Too Many Requests', - text: async () => '', - }) - .mockResolvedValueOnce({ - ok: false, - status: 408, - statusText: 'Request Timeout', - text: async () => '', - }) - .mockResolvedValueOnce({ ok: true, json: async () => smallStoryIndexFixture }); - const sleep = vi.fn().mockResolvedValue(undefined); - - const result = await fetchStoryIndex('http://localhost:6006', { - maxAttempts: 3, - baseBackoffMs: 10, - sleep, - }); - expect(result).toEqual(smallStoryIndexFixture); - expect(mockFetch).toHaveBeenCalledTimes(3); - }); - - it('should bubble up network errors (fetch rejection) without retrying', async () => { + it('should throw error on network failure', async () => { const mockFetch = global.fetch as any; mockFetch.mockRejectedValue(new Error('Network error')); - await expect(fetchStoryIndex('http://localhost:6006')).rejects.toThrow('Network error'); - expect(mockFetch).toHaveBeenCalledTimes(1); + const origin = 'http://localhost:6006'; + + await expect(fetchStoryIndex(origin)).rejects.toThrow('Network error'); }); it('should handle invalid JSON response', async () => { @@ -156,28 +62,8 @@ describe('fetchStoryIndex', () => { }, }); - await expect(fetchStoryIndex('http://localhost:6006')).rejects.toThrow('Invalid JSON'); - }); - - it('truncates very long body snippets in the error message', async () => { - const longBody = 'x'.repeat(500); - const mockFetch = global.fetch as any; - mockFetch.mockResolvedValue({ - ok: false, - status: 500, - statusText: 'Internal Server Error', - text: async () => longBody, - }); + const origin = 'http://localhost:6006'; - try { - await fetchStoryIndex('http://localhost:6006', { maxAttempts: 1, sleep: vi.fn() }); - throw new Error('expected throw'); - } catch (e) { - const msg = (e as Error).message; - // Snippet should be truncated with an ellipsis. - expect(msg).toMatch(/x{100,200}…/); - // And the full 500 chars must not be in the message. - expect(msg).not.toContain(longBody); - } + await expect(fetchStoryIndex(origin)).rejects.toThrow('Invalid JSON'); }); }); diff --git a/packages/addon-mcp/src/utils/fetch-story-index.ts b/packages/addon-mcp/src/utils/fetch-story-index.ts index c8f5123d..50a85bcc 100644 --- a/packages/addon-mcp/src/utils/fetch-story-index.ts +++ b/packages/addon-mcp/src/utils/fetch-story-index.ts @@ -4,90 +4,24 @@ import { logger } from 'storybook/internal/node-logger'; /** * Fetches the Storybook story index from the running Storybook instance. * - * Retries transient 5xx responses with a short backoff — these are common - * right after a story or preview file is edited because Vite is mid- - * recompile and the dev server hasn't repopulated the index yet. We don't - * want to give up after one shot when waiting a second would have worked. - * - * On failure (after retries), throws an Error whose message includes - * status + statusText + a truncated snippet of the response body so the - * caller has something actionable instead of a bare `500 Internal Server - * Error`. - * * @param origin - The origin URL of the Storybook instance (e.g., http://localhost:6006) - * @param options - Optional retry tuning. Defaults: 3 attempts total, 500ms base backoff. * @returns A promise that resolves to the StoryIndex - * @throws If the fetch fails or returns invalid data after all retries + * @throws If the fetch fails or returns invalid data */ -export interface FetchStoryIndexOptions { - /** Total attempts including the first. Default 3. */ - maxAttempts?: number; - /** Base backoff in ms; each attempt waits `attempt * baseBackoffMs`. Default 500. */ - baseBackoffMs?: number; - /** - * Injected for tests so we don't sleep for real. Defaults to setTimeout. - */ - sleep?: (ms: number) => Promise; -} - -const DEFAULT_MAX_ATTEMPTS = 3; -const DEFAULT_BASE_BACKOFF_MS = 500; - -const defaultSleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - -function isTransientStatus(status: number): boolean { - // 5xx are usually transient (server mid-recompile, restarting, etc). - // 408 (timeout) and 429 (too many requests) are also worth retrying. - return status >= 500 || status === 408 || status === 429; -} - -async function readBodySnippet(response: Response, maxChars = 200): Promise { - try { - const text = await response.text(); - const collapsed = text.replace(/\s+/g, ' ').trim(); - if (collapsed.length === 0) return ''; - return collapsed.length <= maxChars ? collapsed : collapsed.slice(0, maxChars - 1) + '…'; - } catch { - return ''; - } -} - -export async function fetchStoryIndex( - origin: string, - options: FetchStoryIndexOptions = {}, -): Promise { - const maxAttempts = Math.max(1, options.maxAttempts ?? DEFAULT_MAX_ATTEMPTS); - const baseBackoffMs = options.baseBackoffMs ?? DEFAULT_BASE_BACKOFF_MS; - const sleep = options.sleep ?? defaultSleep; +export async function fetchStoryIndex(origin: string): Promise { const indexUrl = `${origin}/index.json`; logger.debug(`Fetching story index from: ${indexUrl}`); - let lastErrorMessage = ''; - for (let attempt = 1; attempt <= maxAttempts; attempt++) { - const response = await fetch(indexUrl); + const response = await fetch(indexUrl); - if (response.ok) { - const index = (await response.json()) as StoryIndex; - logger.debug(`Story index entries found: ${Object.keys(index.entries).length}`); - return index; - } - - const snippet = await readBodySnippet(response); - const transient = isTransientStatus(response.status); - const baseMsg = - `Failed to fetch story index: ${response.status} ${response.statusText}` + - (snippet ? ` — ${snippet}` : ''); - lastErrorMessage = - `${baseMsg} (attempt ${attempt}/${maxAttempts}` + - (transient ? ', transient — Storybook may be mid-recompile' : ', non-transient — giving up') + - ')'; - logger.debug(lastErrorMessage); + if (!response.ok) { + throw new Error(`Failed to fetch story index: ${response.status} ${response.statusText}`); + } - if (!transient || attempt === maxAttempts) break; + const index = (await response.json()) as StoryIndex; - await sleep(attempt * baseBackoffMs); - } + logger.debug(`Story index entries found: ${Object.keys(index.entries).length}`); - throw new Error(lastErrorMessage || `Failed to fetch story index after ${maxAttempts} attempts`); + return index; } diff --git a/packages/addon-mcp/src/utils/git-branch.test.ts b/packages/addon-mcp/src/utils/git-branch.test.ts deleted file mode 100644 index 26eb24a0..00000000 --- a/packages/addon-mcp/src/utils/git-branch.test.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; - -const { mockExecFile } = vi.hoisted(() => ({ - mockExecFile: vi.fn(), -})); - -vi.mock('node:child_process', () => ({ - execFile: mockExecFile, -})); - -import { currentGitBranch } from './git-branch.ts'; - -type ExecFileCallback = (error: Error | null, result?: { stdout: string }) => void; - -describe('currentGitBranch', () => { - beforeEach(() => { - mockExecFile.mockReset(); - }); - - it('returns the current branch name in a git repo', async () => { - mockExecFile.mockImplementation( - (_file: string, _args: string[], _options: { cwd: string }, callback: ExecFileCallback) => { - callback(null, { stdout: 'feature/review-state\n' }); - }, - ); - - await expect(currentGitBranch('/repo')).resolves.toBe('feature/review-state'); - expect(mockExecFile).toHaveBeenCalledWith( - 'git', - ['rev-parse', '--abbrev-ref', 'HEAD'], - { cwd: '/repo' }, - expect.any(Function), - ); - }); - - it('returns undefined on detached HEAD', async () => { - mockExecFile.mockImplementation( - (_file: string, _args: string[], _options: { cwd: string }, callback: ExecFileCallback) => { - callback(null, { stdout: 'HEAD\n' }); - }, - ); - - await expect(currentGitBranch('/repo')).resolves.toBeUndefined(); - }); - - it('returns undefined when git command fails (e.g. non-git directory)', async () => { - mockExecFile.mockImplementation( - (_file: string, _args: string[], _options: { cwd: string }, callback: ExecFileCallback) => { - callback(new Error('fatal: not a git repository')); - }, - ); - - await expect(currentGitBranch('/tmp')).resolves.toBeUndefined(); - }); -}); diff --git a/packages/addon-mcp/src/utils/git-branch.ts b/packages/addon-mcp/src/utils/git-branch.ts deleted file mode 100644 index d9081678..00000000 --- a/packages/addon-mcp/src/utils/git-branch.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; - -const execFileAsync = promisify(execFile); - -/** - * Resolve the current git branch of the repo at `cwd`. - * - * Best-effort: returns `undefined` on a detached HEAD, a non-git - * directory, or any git error — callers treat the branch as simply - * unknown rather than failing. - */ -export async function currentGitBranch(cwd: string): Promise { - try { - const { stdout } = await execFileAsync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd }); - const branch = stdout.trim(); - return branch && branch !== 'HEAD' ? branch : undefined; - } catch { - return undefined; - } -} diff --git a/packages/addon-mcp/src/utils/is-review-available.test.ts b/packages/addon-mcp/src/utils/is-review-available.test.ts new file mode 100644 index 00000000..62639537 --- /dev/null +++ b/packages/addon-mcp/src/utils/is-review-available.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi } from 'vitest'; +import { getReviewStatus } from './is-review-available.ts'; +import type { Options } from 'storybook/internal/types'; + +function createMockOptions({ + changeDetection = false, + addons = [] as Array, + hasFeaturesObject = true, +}: { + changeDetection?: boolean; + addons?: Array; + hasFeaturesObject?: boolean; +} = {}): Options { + return { + presets: { + apply: vi.fn(async (key: string) => { + if (key === 'features') { + return hasFeaturesObject ? { changeDetection } : {}; + } + if (key === 'addons') { + return addons; + } + return undefined; + }), + }, + } as unknown as Options; +} + +describe('getReviewStatus', () => { + it.each([ + { + description: 'both feature flag and addon are present (string entry)', + options: { changeDetection: true, addons: ['@storybook/addon-review'] }, + expected: { available: true, hasFeatureFlag: true, hasAddon: true }, + }, + { + description: 'both feature flag and addon are present (object entry)', + options: { changeDetection: true, addons: [{ name: '@storybook/addon-review' }] }, + expected: { available: true, hasFeatureFlag: true, hasAddon: true }, + }, + { + description: 'missing addon (unsupported config)', + options: { changeDetection: true, addons: [] }, + expected: { available: false, hasFeatureFlag: true, hasAddon: false }, + }, + { + description: 'missing feature flag', + options: { changeDetection: false, addons: ['@storybook/addon-review'] }, + expected: { available: false, hasFeatureFlag: false, hasAddon: true }, + }, + { + description: 'both are missing', + options: { changeDetection: false, addons: [] }, + expected: { available: false, hasFeatureFlag: false, hasAddon: false }, + }, + { + description: 'features object is missing the flag', + options: { addons: ['@storybook/addon-review'], hasFeaturesObject: false }, + expected: { available: false, hasFeatureFlag: false, hasAddon: true }, + }, + { + description: 'unrelated addons are ignored', + options: { + changeDetection: true, + addons: ['@storybook/addon-docs', { name: '@storybook/addon-a11y' }], + }, + expected: { available: false, hasFeatureFlag: true, hasAddon: false }, + }, + ])('returns the correct status when $description', async ({ options, expected }) => { + const mockOptions = createMockOptions(options); + const result = await getReviewStatus(mockOptions); + expect(result).toEqual(expected); + }); +}); diff --git a/packages/addon-mcp/src/utils/is-review-available.ts b/packages/addon-mcp/src/utils/is-review-available.ts new file mode 100644 index 00000000..ef9a31fc --- /dev/null +++ b/packages/addon-mcp/src/utils/is-review-available.ts @@ -0,0 +1,35 @@ +import type { Options } from 'storybook/internal/types'; + +const REVIEW_ADDON_NAME = '@storybook/addon-review'; + +export type ReviewStatus = { + available: boolean; + hasFeatureFlag: boolean; + hasAddon: boolean; +}; + +export const getReviewStatus = async (options: Options): Promise => { + const [features, addons] = await Promise.all([ + options.presets.apply('features', {}) as any, + options.presets.apply('addons', []) as any, + ]); + + const hasFeatureFlag = !!features?.changeDetection; + const hasAddon = + Array.isArray(addons) && + addons.some((addon: unknown) => { + const name = + typeof addon === 'string' + ? addon + : addon && typeof addon === 'object' && 'name' in addon + ? (addon as { name: unknown }).name + : undefined; + return typeof name === 'string' && name.includes(REVIEW_ADDON_NAME); + }); + + return { + available: hasFeatureFlag && hasAddon, + hasFeatureFlag, + hasAddon, + }; +}; From fe11c73f8af29b81b077bd9af03b1815ea46c1ac Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 14:59:40 +0200 Subject: [PATCH 19/26] improve getReviewStatus --- packages/addon-mcp/src/preset.test.ts | 3 - .../src/tools/display-review.test.ts | 20 ++++ .../src/utils/is-review-available.test.ts | 107 ++++++++++-------- .../src/utils/is-review-available.ts | 32 +++--- 4 files changed, 93 insertions(+), 69 deletions(-) diff --git a/packages/addon-mcp/src/preset.test.ts b/packages/addon-mcp/src/preset.test.ts index 528f549c..c7e4d5d5 100644 --- a/packages/addon-mcp/src/preset.test.ts +++ b/packages/addon-mcp/src/preset.test.ts @@ -106,9 +106,6 @@ describe('experimental_devServer', () => { if (key === 'features') { return Promise.resolve({ changeDetection: true }); } - if (key === 'addons') { - return Promise.resolve(['@storybook/addon-review']); - } return Promise.resolve(undefined); }), }, diff --git a/packages/addon-mcp/src/tools/display-review.test.ts b/packages/addon-mcp/src/tools/display-review.test.ts index 909d1484..0d21ca46 100644 --- a/packages/addon-mcp/src/tools/display-review.test.ts +++ b/packages/addon-mcp/src/tools/display-review.test.ts @@ -55,6 +55,13 @@ describe('buildReviewUrl', () => { it('throws when neither request nor origin is available', () => { expect(() => buildReviewUrl({} as any)).toThrow(/Cannot resolve the Storybook URL/); }); + + it('throws when the request URL is unparseable', () => { + const badRequest = { url: '::::not a url' } as unknown as Request; + expect(() => buildReviewUrl({ origin: 'http://localhost:6006', request: badRequest })).toThrow( + /Cannot resolve the Storybook URL/, + ); + }); }); describe('displayReviewTool', () => { @@ -158,4 +165,17 @@ describe('displayReviewTool', () => { 'http://localhost:6006/design-system/?path=/review/', ); }); + + it('returns an MCP error when origin is missing from the addon context', async () => { + const response = await callTool(sampleReview, { + // Intentionally omit `origin` to exercise the error path. + options: {} as unknown as AddonContext['options'], + disableTelemetry: true, + } as AddonContext); + const result = getResult(response); + + expect(result?.isError).toBe(true); + expect(result?.content?.[0]?.text).toMatch(/Cannot resolve the Storybook URL/); + expect(emitted).toEqual([]); + }); }); diff --git a/packages/addon-mcp/src/utils/is-review-available.test.ts b/packages/addon-mcp/src/utils/is-review-available.test.ts index 62639537..279d9597 100644 --- a/packages/addon-mcp/src/utils/is-review-available.test.ts +++ b/packages/addon-mcp/src/utils/is-review-available.test.ts @@ -1,25 +1,34 @@ -import { describe, it, expect, vi } from 'vitest'; -import { getReviewStatus } from './is-review-available.ts'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { Options } from 'storybook/internal/types'; +const { mockLoadMainConfig, mockGetAddonNames } = vi.hoisted(() => ({ + mockLoadMainConfig: vi.fn(), + mockGetAddonNames: vi.fn(), +})); + +vi.mock('storybook/internal/common', () => ({ + loadMainConfig: (...args: unknown[]) => mockLoadMainConfig(...args), + getAddonNames: (...args: unknown[]) => mockGetAddonNames(...args), +})); + +import { getReviewStatus } from './is-review-available.ts'; + function createMockOptions({ changeDetection = false, - addons = [] as Array, hasFeaturesObject = true, + configDir = '/project/.storybook', }: { changeDetection?: boolean; - addons?: Array; hasFeaturesObject?: boolean; + configDir?: string; } = {}): Options { return { + configDir, presets: { apply: vi.fn(async (key: string) => { if (key === 'features') { return hasFeaturesObject ? { changeDetection } : {}; } - if (key === 'addons') { - return addons; - } return undefined; }), }, @@ -27,48 +36,46 @@ function createMockOptions({ } describe('getReviewStatus', () => { - it.each([ - { - description: 'both feature flag and addon are present (string entry)', - options: { changeDetection: true, addons: ['@storybook/addon-review'] }, - expected: { available: true, hasFeatureFlag: true, hasAddon: true }, - }, - { - description: 'both feature flag and addon are present (object entry)', - options: { changeDetection: true, addons: [{ name: '@storybook/addon-review' }] }, - expected: { available: true, hasFeatureFlag: true, hasAddon: true }, - }, - { - description: 'missing addon (unsupported config)', - options: { changeDetection: true, addons: [] }, - expected: { available: false, hasFeatureFlag: true, hasAddon: false }, - }, - { - description: 'missing feature flag', - options: { changeDetection: false, addons: ['@storybook/addon-review'] }, - expected: { available: false, hasFeatureFlag: false, hasAddon: true }, - }, - { - description: 'both are missing', - options: { changeDetection: false, addons: [] }, - expected: { available: false, hasFeatureFlag: false, hasAddon: false }, - }, - { - description: 'features object is missing the flag', - options: { addons: ['@storybook/addon-review'], hasFeaturesObject: false }, - expected: { available: false, hasFeatureFlag: false, hasAddon: true }, - }, - { - description: 'unrelated addons are ignored', - options: { - changeDetection: true, - addons: ['@storybook/addon-docs', { name: '@storybook/addon-a11y' }], - }, - expected: { available: false, hasFeatureFlag: true, hasAddon: false }, - }, - ])('returns the correct status when $description', async ({ options, expected }) => { - const mockOptions = createMockOptions(options); - const result = await getReviewStatus(mockOptions); - expect(result).toEqual(expected); + beforeEach(() => { + mockLoadMainConfig.mockReset(); + mockGetAddonNames.mockReset(); + mockLoadMainConfig.mockResolvedValue({ addons: [] }); + mockGetAddonNames.mockReturnValue([]); + }); + + it('returns available when change detection is on and addon-review is in main.ts', async () => { + mockGetAddonNames.mockReturnValue(['@storybook/addon-docs', '@storybook/addon-review']); + const result = await getReviewStatus(createMockOptions({ changeDetection: true })); + + expect(mockLoadMainConfig).toHaveBeenCalledWith({ configDir: '/project/.storybook' }); + expect(result).toEqual({ available: true, hasFeatureFlag: true, hasAddon: true }); + }); + + it('returns hasAddon=false when addon-review is not in main.ts', async () => { + mockGetAddonNames.mockReturnValue(['@storybook/addon-docs', '@storybook/addon-a11y']); + const result = await getReviewStatus(createMockOptions({ changeDetection: true })); + + expect(result).toEqual({ available: false, hasFeatureFlag: true, hasAddon: false }); + }); + + it('returns hasFeatureFlag=false when changeDetection is disabled', async () => { + mockGetAddonNames.mockReturnValue(['@storybook/addon-review']); + const result = await getReviewStatus(createMockOptions({ changeDetection: false })); + + expect(result).toEqual({ available: false, hasFeatureFlag: false, hasAddon: true }); + }); + + it('returns hasFeatureFlag=false when features object is missing the flag', async () => { + mockGetAddonNames.mockReturnValue(['@storybook/addon-review']); + const result = await getReviewStatus(createMockOptions({ hasFeaturesObject: false })); + + expect(result).toEqual({ available: false, hasFeatureFlag: false, hasAddon: true }); + }); + + it('treats a failure to load main.ts as "addon not present" rather than crashing', async () => { + mockLoadMainConfig.mockRejectedValue(new Error('main.ts blew up')); + const result = await getReviewStatus(createMockOptions({ changeDetection: true })); + + expect(result).toEqual({ available: false, hasFeatureFlag: true, hasAddon: false }); }); }); diff --git a/packages/addon-mcp/src/utils/is-review-available.ts b/packages/addon-mcp/src/utils/is-review-available.ts index ef9a31fc..db2b08c1 100644 --- a/packages/addon-mcp/src/utils/is-review-available.ts +++ b/packages/addon-mcp/src/utils/is-review-available.ts @@ -1,3 +1,4 @@ +import { getAddonNames, loadMainConfig } from 'storybook/internal/common'; import type { Options } from 'storybook/internal/types'; const REVIEW_ADDON_NAME = '@storybook/addon-review'; @@ -9,23 +10,22 @@ export type ReviewStatus = { }; export const getReviewStatus = async (options: Options): Promise => { - const [features, addons] = await Promise.all([ - options.presets.apply('features', {}) as any, - options.presets.apply('addons', []) as any, - ]); - + const features = (await options.presets.apply('features', {})) as + | { changeDetection?: boolean } + | undefined; const hasFeatureFlag = !!features?.changeDetection; - const hasAddon = - Array.isArray(addons) && - addons.some((addon: unknown) => { - const name = - typeof addon === 'string' - ? addon - : addon && typeof addon === 'object' && 'name' in addon - ? (addon as { name: unknown }).name - : undefined; - return typeof name === 'string' && name.includes(REVIEW_ADDON_NAME); - }); + + // Read the user's `main.ts` addons array directly via Storybook's public + // helpers. `getAddonNames` normalizes each entry (strips `/preset`, + // `/manager`, `node_modules/`, file extensions, etc.) so we get the + // package name the user typed. + let hasAddon = false; + try { + const mainConfig = await loadMainConfig({ configDir: options.configDir }); + hasAddon = getAddonNames(mainConfig).includes(REVIEW_ADDON_NAME); + } catch { + hasAddon = false; + } return { available: hasFeatureFlag && hasAddon, From f3bee6f0c1d69ed88a5c9af292f687260a46d838 Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 15:15:37 +0200 Subject: [PATCH 20/26] PR review fixes --- packages/addon-mcp/src/constants.ts | 3 ++- .../instructions/build-server-instructions.ts | 7 +++++- .../src/instructions/dev-instructions.md | 3 ++- packages/addon-mcp/src/mcp-handler.ts | 19 ++++++++++++---- .../src/utils/is-review-available.test.ts | 15 +++++++++++++ .../src/utils/is-review-available.ts | 22 +++++++++++-------- 6 files changed, 53 insertions(+), 16 deletions(-) diff --git a/packages/addon-mcp/src/constants.ts b/packages/addon-mcp/src/constants.ts index d99df331..20a40b48 100644 --- a/packages/addon-mcp/src/constants.ts +++ b/packages/addon-mcp/src/constants.ts @@ -3,7 +3,8 @@ export const MCP_APP_SIZE_CHANGED_EVENT = 'storybook-mcp:size-changed'; /** * Channel event shared with `@storybook/addon-review` (cross-repo contract). - * Emitted by the `display-review` tool to hand the agent's payload off to the addon-review server preset, where the state is handled + * Emitted by the `display-review` tool to hand the agent's payload off to the + * addon-review server preset */ export const PUSH_REVIEW_EVENT = 'storybook/addon-review/push-review'; diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.ts b/packages/addon-mcp/src/instructions/build-server-instructions.ts index 2b784760..94cba46e 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.ts @@ -16,6 +16,11 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) if (options.devEnabled) { const changeDetection = options.changeDetectionEnabled ?? false; const reviewEnabled = options.reviewEnabled ?? false; + // The DISPLAY_REVIEW_STEP placeholder lives on its own line in the + // template. Match the preceding newline too so that, when reviewEnabled + // is false, we don't leave a blank line behind — and when enabled, the + // replacement bullet renders cleanly without the template having to + // encode whitespace conventions in its replacement string. sections.push( devInstructions .replace( @@ -25,7 +30,7 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) : 'After changing any component or story, call **preview-stories** to retrieve preview URLs.', ) .replace( - '{{DISPLAY_REVIEW_STEP}}', + /\n\{\{DISPLAY_REVIEW_STEP\}\}/, reviewEnabled ? "\n- After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback." : '', diff --git a/packages/addon-mcp/src/instructions/dev-instructions.md b/packages/addon-mcp/src/instructions/dev-instructions.md index 8072e145..f8d0db51 100644 --- a/packages/addon-mcp/src/instructions/dev-instructions.md +++ b/packages/addon-mcp/src/instructions/dev-instructions.md @@ -4,4 +4,5 @@ - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - {{PREVIEW_STORIES_STEP}} - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. -- Always include every returned preview URL in your user-facing response so the user can verify the visual result.{{DISPLAY_REVIEW_STEP}} +- Always include every returned preview URL in your user-facing response so the user can verify the visual result. +{{DISPLAY_REVIEW_STEP}} diff --git a/packages/addon-mcp/src/mcp-handler.ts b/packages/addon-mcp/src/mcp-handler.ts index 03a38c16..3ce4d795 100644 --- a/packages/addon-mcp/src/mcp-handler.ts +++ b/packages/addon-mcp/src/mcp-handler.ts @@ -26,6 +26,9 @@ import { isAddonA11yEnabled } from './utils/is-addon-a11y-enabled.ts'; import type { CompositionAuth } from './auth/index.ts'; import { buildServerInstructions } from './instructions/build-server-instructions.ts'; +/** Default MCP endpoint path. Kept in sync with `DEFAULT_MCP_ENDPOINT` in preset.ts. */ +const DEFAULT_ENDPOINT = '/mcp'; + let transport: HttpTransport | undefined; let origin: string | undefined; // Promise that ensures single initialization, even with concurrent requests @@ -39,10 +42,12 @@ const initializeMCPServer = async (options: Options, multiSource?: boolean) => { const changeDetectionEnabled = features?.changeDetection ?? false; disableTelemetry = core?.disableTelemetry ?? false; - // Determine tool availability before creating server so instructions can be tailored + // Determine tool availability before creating server so instructions can be tailored. + // Reuse the already-resolved `features` so getReviewStatus doesn't re-call + // `presets.apply('features', …)` and risk a different snapshot. const addonVitestConstants = await getAddonVitestConstants(); const manifestStatus = await getManifestStatus(options); - const reviewStatus = await getReviewStatus(options); + const reviewStatus = await getReviewStatus(options, { features }); a11yEnabled = await isAddonA11yEnabled(options); let server: McpServer; @@ -119,7 +124,13 @@ type McpServerHandlerParams = { res: ServerResponse; options: Options; addonOptions: AddonOptionsOutput; - endpoint: string; + /** + * The MCP endpoint path (e.g. `/mcp` or a user-configured override). + * Used to derive the Storybook root from the incoming request URL inside + * tools like `display-review`. Optional for backwards compatibility with + * external callers; defaults to {@link DEFAULT_ENDPOINT}. + */ + endpoint?: string; /** Sources for multi-source mode (when refs are configured) */ sources?: Source[]; /** Optional custom manifest provider, receives source as third param in multi-source mode */ @@ -137,7 +148,7 @@ export const mcpServerHandler = async ({ res, options, addonOptions, - endpoint, + endpoint = DEFAULT_ENDPOINT, sources, manifestProvider, compositionAuth, diff --git a/packages/addon-mcp/src/utils/is-review-available.test.ts b/packages/addon-mcp/src/utils/is-review-available.test.ts index 279d9597..e4051420 100644 --- a/packages/addon-mcp/src/utils/is-review-available.test.ts +++ b/packages/addon-mcp/src/utils/is-review-available.test.ts @@ -78,4 +78,19 @@ describe('getReviewStatus', () => { expect(result).toEqual({ available: false, hasFeatureFlag: true, hasAddon: false }); }); + + it('uses pre-resolved features when provided and skips presets.apply', async () => { + mockGetAddonNames.mockReturnValue(['@storybook/addon-review']); + const mockOptions = createMockOptions({ changeDetection: false }); + // Pass features explicitly with changeDetection=true; the mock's + // `presets.apply` would return changeDetection=false if asked, so a + // `true` result here proves we used the provided value and didn't + // re-resolve. + const result = await getReviewStatus(mockOptions, { + features: { changeDetection: true }, + }); + + expect(result).toEqual({ available: true, hasFeatureFlag: true, hasAddon: true }); + expect(mockOptions.presets.apply).not.toHaveBeenCalledWith('features', expect.anything()); + }); }); diff --git a/packages/addon-mcp/src/utils/is-review-available.ts b/packages/addon-mcp/src/utils/is-review-available.ts index db2b08c1..2c12271c 100644 --- a/packages/addon-mcp/src/utils/is-review-available.ts +++ b/packages/addon-mcp/src/utils/is-review-available.ts @@ -9,16 +9,20 @@ export type ReviewStatus = { hasAddon: boolean; }; -export const getReviewStatus = async (options: Options): Promise => { - const features = (await options.presets.apply('features', {})) as - | { changeDetection?: boolean } - | undefined; - const hasFeatureFlag = !!features?.changeDetection; +export interface GetReviewStatusOptions { + features?: { changeDetection?: boolean } | undefined; +} - // Read the user's `main.ts` addons array directly via Storybook's public - // helpers. `getAddonNames` normalizes each entry (strips `/preset`, - // `/manager`, `node_modules/`, file extensions, etc.) so we get the - // package name the user typed. +export const getReviewStatus = async ( + options: Options, + { features }: GetReviewStatusOptions = {}, +): Promise => { + const resolvedFeatures = + features ?? + ((await options.presets.apply('features', {})) as { changeDetection?: boolean } | undefined); + const hasFeatureFlag = !!resolvedFeatures?.changeDetection; + + // Read the user's `main.ts` addons array and detect @storybook/addon-review presence let hasAddon = false; try { const mainConfig = await loadMainConfig({ configDir: options.configDir }); From 5fe1a6d404286fa36d17f94163538ba43ab95c17 Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 15:22:48 +0200 Subject: [PATCH 21/26] fixes --- .../src/instructions/build-server-instructions.ts | 7 +------ packages/addon-mcp/src/instructions/dev-instructions.md | 3 +-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.ts b/packages/addon-mcp/src/instructions/build-server-instructions.ts index 94cba46e..2b784760 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.ts @@ -16,11 +16,6 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) if (options.devEnabled) { const changeDetection = options.changeDetectionEnabled ?? false; const reviewEnabled = options.reviewEnabled ?? false; - // The DISPLAY_REVIEW_STEP placeholder lives on its own line in the - // template. Match the preceding newline too so that, when reviewEnabled - // is false, we don't leave a blank line behind — and when enabled, the - // replacement bullet renders cleanly without the template having to - // encode whitespace conventions in its replacement string. sections.push( devInstructions .replace( @@ -30,7 +25,7 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) : 'After changing any component or story, call **preview-stories** to retrieve preview URLs.', ) .replace( - /\n\{\{DISPLAY_REVIEW_STEP\}\}/, + '{{DISPLAY_REVIEW_STEP}}', reviewEnabled ? "\n- After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback." : '', diff --git a/packages/addon-mcp/src/instructions/dev-instructions.md b/packages/addon-mcp/src/instructions/dev-instructions.md index f8d0db51..8072e145 100644 --- a/packages/addon-mcp/src/instructions/dev-instructions.md +++ b/packages/addon-mcp/src/instructions/dev-instructions.md @@ -4,5 +4,4 @@ - Treat that tool's output as the source of truth for framework-specific imports, story patterns, and testing conventions. - {{PREVIEW_STORIES_STEP}} - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. -- Always include every returned preview URL in your user-facing response so the user can verify the visual result. -{{DISPLAY_REVIEW_STEP}} +- Always include every returned preview URL in your user-facing response so the user can verify the visual result.{{DISPLAY_REVIEW_STEP}} From f453508476dc64101756106a0a29e306d79f2a60 Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 15:49:13 +0200 Subject: [PATCH 22/26] improve prompt and data --- .../addon-mcp/src/tools/display-review.ts | 46 +++++-------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/packages/addon-mcp/src/tools/display-review.ts b/packages/addon-mcp/src/tools/display-review.ts index 4b1a7480..0bd33a14 100644 --- a/packages/addon-mcp/src/tools/display-review.ts +++ b/packages/addon-mcp/src/tools/display-review.ts @@ -35,29 +35,6 @@ export const ReviewCollectionSchema = v.object({ ), }); -const StoryMetaSchema = v.object({ - depth: v.pipe( - v.optional(v.number()), - v.description( - 'Graph distance from the changed file(s) to this story (0 = the changed component itself).', - ), - ), - chain: v.pipe( - v.optional(v.array(v.string())), - v.description( - 'Ordered intermediate file paths between the story file and the changed file, excluding both endpoints. Empty/omitted means a direct import.', - ), - ), -}); - -const DiffHunkSchema = v.object({ - path: v.pipe(v.string(), v.description('Path of the changed file this hunk belongs to.')), - hunk: v.pipe( - v.string(), - v.description('Unified-diff text for this hunk (with +/- line prefixes).'), - ), -}); - export const ReviewStateSchema = v.object({ title: v.pipe( v.string(), @@ -74,14 +51,6 @@ export const ReviewStateSchema = v.object({ v.optional(v.array(v.string())), v.description('Paths of the files you changed, most central first.'), ), - diffHunks: v.pipe( - v.optional(v.array(DiffHunkSchema)), - v.description('The actual diff hunks of your change, shown in the review page.'), - ), - storyMeta: v.pipe( - v.optional(v.record(v.string(), StoryMetaSchema)), - v.description('Optional per-story metadata keyed by story ID: { depth, chain }.'), - ), }); export type ReviewCollection = v.InferOutput; @@ -137,13 +106,20 @@ export async function addDisplayReviewTool(server: McpServer) title: 'Display Storybook review', description: `Push a curated review of the current change to Storybook's review page. -After you finish a UI code change, call this to help the user spot-check it. Provide: +After you finish a UI code change, call this to help the user spot-check it. + +Before composing collections, answer two questions: +- *Where is this change rendered?* Trace upward from the edited file through the import graph until you hit page-level or top-level story files. +- *What would a reviewer want to spot-check in real context?* Include at least one story per layer of that chain. + +Provide: - title: a PR-style title for the change — short and specific. - description: a one-line summary of what changed and where to start reviewing. -- collections: titled groups of representative story IDs. Give each a concise, PR-dense title, a one-sentence rationale, and — when you can tell — a kind ("atomic" for the directly changed component, "consumer" for direct dependents, "transitive" for pages/containers, "catch-all" otherwise). +- collections: titled groups of stories covering the **visual cascade** of the change — not just where the code is read, but everywhere a reviewer will see it. For any non-trivial UI change, include the atomic component, its direct consumers, and the pages/containers that render them transitively. A single-collection review is a smell: only do it if the component is genuinely standalone (e.g. has no parents in the story graph). Theme tokens, shared styles, and layout primitives almost always need transitive page coverage even when only one file imports them. Give each collection a concise, PR-dense title, a one-sentence rationale, and a \`kind\`. +- kind (per collection): classify each collection as "atomic" (the directly changed component), "consumer" (direct dependents), "transitive" (pages/containers further away), or "catch-all" (everything else). Most UI changes should have at least two kinds present (e.g. \`consumer\` + \`transitive\`). A review with only \`atomic\` or only \`consumer\` is rarely complete. - changedFiles: the files you edited (most central first). -- diffHunks: the actual diff of your change (you made it — include the hunks). -- storyMeta: optional per-story { depth, chain }. + +Anti-pattern: editing a theme token that only one component reads, then publishing a review with just that one component's story. The token change is visible on every page that renders the component — include those pages. The \`kind\` labels are for structured review grouping and UI behavior; do not repeat these labels verbatim in user-facing prose unless the user explicitly asks for them. From ca09074ab47fa9d23b1ee80d2106e5a58dbabb85 Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 16:00:15 +0200 Subject: [PATCH 23/26] improve url handling --- .../src/tools/display-review.test.ts | 25 ++++++++++++++++--- .../addon-mcp/src/tools/display-review.ts | 17 +++++++------ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/addon-mcp/src/tools/display-review.test.ts b/packages/addon-mcp/src/tools/display-review.test.ts index 0d21ca46..0ca535a8 100644 --- a/packages/addon-mcp/src/tools/display-review.test.ts +++ b/packages/addon-mcp/src/tools/display-review.test.ts @@ -56,12 +56,31 @@ describe('buildReviewUrl', () => { expect(() => buildReviewUrl({} as any)).toThrow(/Cannot resolve the Storybook URL/); }); - it('throws when the request URL is unparseable', () => { + it('falls back to origin when the request URL is unparseable', () => { const badRequest = { url: '::::not a url' } as unknown as Request; - expect(() => buildReviewUrl({ origin: 'http://localhost:6006', request: badRequest })).toThrow( - /Cannot resolve the Storybook URL/, + expect(buildReviewUrl({ origin: 'http://localhost:6006', request: badRequest })).toBe( + 'http://localhost:6006/?path=/review/', ); }); + + it('handles a trailing slash on the request pathname', () => { + expect( + buildReviewUrl({ + origin: 'http://localhost:6006', + request: new Request('https://example.com/storybook/mcp/'), + }), + ).toBe('http://localhost:6006/storybook/?path=/review/'); + }); + + it('strips a multi-segment endpoint with a trailing slash on the request', () => { + expect( + buildReviewUrl({ + origin: 'http://localhost:6006', + request: new Request('https://example.com/api/mcp/'), + endpoint: '/api/mcp', + }), + ).toBe('http://localhost:6006/?path=/review/'); + }); }); describe('displayReviewTool', () => { diff --git a/packages/addon-mcp/src/tools/display-review.ts b/packages/addon-mcp/src/tools/display-review.ts index 0bd33a14..94563776 100644 --- a/packages/addon-mcp/src/tools/display-review.ts +++ b/packages/addon-mcp/src/tools/display-review.ts @@ -74,9 +74,10 @@ function storybookRootFromRequest( try { const url = new URL(request.url); const normalizedEndpoint = endpoint.replace(/\/$/, ''); - const rootPath = url.pathname.endsWith(normalizedEndpoint) - ? url.pathname.slice(0, -normalizedEndpoint.length) - : url.pathname.replace(/\/[^/]+\/?$/, '/'); + const normalizedPathname = url.pathname.replace(/\/$/, ''); + const rootPath = normalizedPathname.endsWith(normalizedEndpoint) + ? normalizedPathname.slice(0, -normalizedEndpoint.length) + : normalizedPathname.replace(/\/[^/]+$/, ''); return `${trustedOrigin.replace(/\/$/, '')}${rootPath}`; } catch { return undefined; @@ -89,13 +90,13 @@ export function buildReviewUrl(ctx: { endpoint?: string; }): string { const trustedOrigin = ctx.origin; - const root = - ctx.request && trustedOrigin - ? storybookRootFromRequest(ctx.request, trustedOrigin, ctx.endpoint ?? '/mcp') - : trustedOrigin; - if (!root) { + if (!trustedOrigin) { throw new Error('Cannot resolve the Storybook URL: missing trusted origin in addon context.'); } + const root = ctx.request + ? (storybookRootFromRequest(ctx.request, trustedOrigin, ctx.endpoint ?? '/mcp') ?? + trustedOrigin) + : trustedOrigin; return `${root.replace(/\/$/, '')}/?path=${REVIEW_PAGE_PATH}`; } From e406bc8e716259c36855b9d59ffbb21e24f39d6e Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 16:11:24 +0200 Subject: [PATCH 24/26] update prompt --- packages/addon-mcp/src/tools/display-review.test.ts | 2 -- packages/addon-mcp/src/tools/display-review.ts | 11 +---------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/packages/addon-mcp/src/tools/display-review.test.ts b/packages/addon-mcp/src/tools/display-review.test.ts index 0ca535a8..9d1efa55 100644 --- a/packages/addon-mcp/src/tools/display-review.test.ts +++ b/packages/addon-mcp/src/tools/display-review.test.ts @@ -14,13 +14,11 @@ const sampleReview: ReviewState = { title: 'Button', rationale: 'The directly changed component.', storyIds: ['button--primary', 'button--secondary'], - kind: 'atomic', }, { title: 'Pages', rationale: 'Pages that render Button.', storyIds: ['page--home'], - kind: 'transitive', }, ], changedFiles: ['src/Button.tsx'], diff --git a/packages/addon-mcp/src/tools/display-review.ts b/packages/addon-mcp/src/tools/display-review.ts index 94563776..9f548f0e 100644 --- a/packages/addon-mcp/src/tools/display-review.ts +++ b/packages/addon-mcp/src/tools/display-review.ts @@ -27,12 +27,6 @@ export const ReviewCollectionSchema = v.object({ 'Story IDs that represent this collection (e.g. "button--primary"). The page renders exactly these.', ), ), - kind: v.pipe( - v.optional(v.picklist(['atomic', 'consumer', 'transitive', 'catch-all'])), - v.description( - 'Semantic role of this collection in the change cascade: "atomic" = the directly changed component, "consumer" = direct dependents, "transitive" = pages/containers further away, "catch-all" = everything else. Omit if unknown.', - ), - ), }); export const ReviewStateSchema = v.object({ @@ -116,14 +110,11 @@ Before composing collections, answer two questions: Provide: - title: a PR-style title for the change — short and specific. - description: a one-line summary of what changed and where to start reviewing. -- collections: titled groups of stories covering the **visual cascade** of the change — not just where the code is read, but everywhere a reviewer will see it. For any non-trivial UI change, include the atomic component, its direct consumers, and the pages/containers that render them transitively. A single-collection review is a smell: only do it if the component is genuinely standalone (e.g. has no parents in the story graph). Theme tokens, shared styles, and layout primitives almost always need transitive page coverage even when only one file imports them. Give each collection a concise, PR-dense title, a one-sentence rationale, and a \`kind\`. -- kind (per collection): classify each collection as "atomic" (the directly changed component), "consumer" (direct dependents), "transitive" (pages/containers further away), or "catch-all" (everything else). Most UI changes should have at least two kinds present (e.g. \`consumer\` + \`transitive\`). A review with only \`atomic\` or only \`consumer\` is rarely complete. +- collections: titled groups of stories covering the **visual cascade** of the change — not just where the code is read, but everywhere a reviewer will see it. For any non-trivial UI change, include the changed component itself, the components that directly import it, and the pages/containers that render them further up the tree. A single-collection review is a smell: only do it if the component is genuinely standalone (e.g. has no parents in the story graph). Theme tokens, shared styles, and layout primitives almost always need page-level coverage even when only one file imports them. Give each collection a concise, PR-dense title and a one-sentence rationale. Titles should describe *what stories the reviewer is looking at* (e.g. "Button — all variants", "Checkout pages"), not the collection's role in the cascade. - changedFiles: the files you edited (most central first). Anti-pattern: editing a theme token that only one component reads, then publishing a review with just that one component's story. The token change is visible on every page that renders the component — include those pages. -The \`kind\` labels are for structured review grouping and UI behavior; do not repeat these labels verbatim in user-facing prose unless the user explicitly asks for them. - Always include the returned reviewUrl in your final user-facing response so the user can open it.`, schema: ReviewStateSchema, outputSchema: DisplayReviewOutput, From 238729f328c1e9663b7385d86a3b2c73984d8477 Mon Sep 17 00:00:00 2001 From: yannbf Date: Wed, 27 May 2026 16:23:59 +0200 Subject: [PATCH 25/26] extract default MCP endpoint to a constant --- packages/addon-mcp/src/constants.ts | 8 ++++++++ packages/addon-mcp/src/mcp-handler.ts | 8 +++----- packages/addon-mcp/src/preset.ts | 2 +- packages/addon-mcp/src/tools/display-review.ts | 4 ++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/addon-mcp/src/constants.ts b/packages/addon-mcp/src/constants.ts index 20a40b48..66747782 100644 --- a/packages/addon-mcp/src/constants.ts +++ b/packages/addon-mcp/src/constants.ts @@ -10,3 +10,11 @@ export const PUSH_REVIEW_EVENT = 'storybook/addon-review/push-review'; /** Storybook manager route the review page is registered at. */ export const REVIEW_PAGE_PATH = '/review/'; + +/** + * Default path the MCP server is mounted at on the Storybook dev server. + * The user can override this via the addon's `endpoint` option; everywhere + * else in the codebase that needs to compare against or fall back to the + * default should import this constant rather than hard-coding `'/mcp'`. + */ +export const DEFAULT_MCP_ENDPOINT = '/mcp'; diff --git a/packages/addon-mcp/src/mcp-handler.ts b/packages/addon-mcp/src/mcp-handler.ts index 3ce4d795..9928a6f2 100644 --- a/packages/addon-mcp/src/mcp-handler.ts +++ b/packages/addon-mcp/src/mcp-handler.ts @@ -25,9 +25,7 @@ import { estimateTokens } from './utils/estimate-tokens.ts'; import { isAddonA11yEnabled } from './utils/is-addon-a11y-enabled.ts'; import type { CompositionAuth } from './auth/index.ts'; import { buildServerInstructions } from './instructions/build-server-instructions.ts'; - -/** Default MCP endpoint path. Kept in sync with `DEFAULT_MCP_ENDPOINT` in preset.ts. */ -const DEFAULT_ENDPOINT = '/mcp'; +import { DEFAULT_MCP_ENDPOINT } from './constants.ts'; let transport: HttpTransport | undefined; let origin: string | undefined; @@ -128,7 +126,7 @@ type McpServerHandlerParams = { * The MCP endpoint path (e.g. `/mcp` or a user-configured override). * Used to derive the Storybook root from the incoming request URL inside * tools like `display-review`. Optional for backwards compatibility with - * external callers; defaults to {@link DEFAULT_ENDPOINT}. + * external callers; defaults to {@link DEFAULT_MCP_ENDPOINT}. */ endpoint?: string; /** Sources for multi-source mode (when refs are configured) */ @@ -148,7 +146,7 @@ export const mcpServerHandler = async ({ res, options, addonOptions, - endpoint = DEFAULT_ENDPOINT, + endpoint = DEFAULT_MCP_ENDPOINT, sources, manifestProvider, compositionAuth, diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index 892fe1f3..b6cbc5dd 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -18,8 +18,8 @@ import { import { logger } from 'storybook/internal/node-logger'; import type { Source } from '@storybook/mcp'; import type { IncomingMessage, ServerResponse } from 'node:http'; +import { DEFAULT_MCP_ENDPOINT } from './constants.ts'; -const DEFAULT_MCP_ENDPOINT = '/mcp'; const STORYBOOK_MCP_PROXY_HEADER_KEY = STORYBOOK_MCP_PROXY_HEADER.toLowerCase(); export const previewAnnotations: PresetPropertyFn<'previewAnnotations'> = async ( diff --git a/packages/addon-mcp/src/tools/display-review.ts b/packages/addon-mcp/src/tools/display-review.ts index 9f548f0e..2df1a1f3 100644 --- a/packages/addon-mcp/src/tools/display-review.ts +++ b/packages/addon-mcp/src/tools/display-review.ts @@ -2,7 +2,7 @@ import type { McpServer } from 'tmcp'; import * as v from 'valibot'; import type { AddonContext } from '../types.ts'; import { errorToMCPContent } from '../utils/errors.ts'; -import { PUSH_REVIEW_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; +import { DEFAULT_MCP_ENDPOINT, PUSH_REVIEW_EVENT, REVIEW_PAGE_PATH } from '../constants.ts'; import { DISPLAY_REVIEW_TOOL_NAME } from './tool-names.ts'; /** @@ -88,7 +88,7 @@ export function buildReviewUrl(ctx: { throw new Error('Cannot resolve the Storybook URL: missing trusted origin in addon context.'); } const root = ctx.request - ? (storybookRootFromRequest(ctx.request, trustedOrigin, ctx.endpoint ?? '/mcp') ?? + ? (storybookRootFromRequest(ctx.request, trustedOrigin, ctx.endpoint ?? DEFAULT_MCP_ENDPOINT) ?? trustedOrigin) : trustedOrigin; return `${root.replace(/\/$/, '')}/?path=${REVIEW_PAGE_PATH}`; From 7d424c197524127f2cf8423146b4d853fa69ca9e Mon Sep 17 00:00:00 2001 From: yannbf Date: Thu, 28 May 2026 09:42:54 +0200 Subject: [PATCH 26/26] improve server instructions --- .../src/instructions/build-server-instructions.test.ts | 4 ++-- .../addon-mcp/src/instructions/build-server-instructions.ts | 2 +- packages/addon-mcp/src/tools/display-review.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts index d01089b1..be3800cc 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.test.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.test.ts @@ -21,7 +21,7 @@ describe('buildServerInstructions', () => { - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - - After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback. + - After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback. Call this tool again whenever the user iterates on the changes, so you keep the review up to date. ## Validation Workflow @@ -69,7 +69,7 @@ describe('buildServerInstructions', () => { - After changing any component or story, call **get-changed-stories** to discover new/modified/related stories, then call **preview-stories** to retrieve preview URLs. - In final user-facing responses, order links consistently: review page first (if available), changed-stories fallback next (if relevant), then specific preview URLs. - Always include every returned preview URL in your user-facing response so the user can verify the visual result. - - After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback." + - After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned \`reviewUrl\` so the user sees the review without leaving the chat. Always include the \`reviewUrl\` in your final response as a fallback. Call this tool again whenever the user iterates on the changes, so you keep the review up to date." `); }); diff --git a/packages/addon-mcp/src/instructions/build-server-instructions.ts b/packages/addon-mcp/src/instructions/build-server-instructions.ts index 2b784760..18930b7b 100644 --- a/packages/addon-mcp/src/instructions/build-server-instructions.ts +++ b/packages/addon-mcp/src/instructions/build-server-instructions.ts @@ -27,7 +27,7 @@ export function buildServerInstructions(options: BuildServerInstructionsOptions) .replace( '{{DISPLAY_REVIEW_STEP}}', reviewEnabled - ? "\n- After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback." + ? "\n- After completing the change, call **display-review** to publish a curated review to Storybook's review page. If the session has a browser-preview tool, navigate it to the returned `reviewUrl` so the user sees the review without leaving the chat. Always include the `reviewUrl` in your final response as a fallback. Call this tool again whenever the user iterates on the changes, so you keep the review up to date." : '', ) .trim(), diff --git a/packages/addon-mcp/src/tools/display-review.ts b/packages/addon-mcp/src/tools/display-review.ts index 2df1a1f3..2ba2d23c 100644 --- a/packages/addon-mcp/src/tools/display-review.ts +++ b/packages/addon-mcp/src/tools/display-review.ts @@ -115,7 +115,7 @@ Provide: Anti-pattern: editing a theme token that only one component reads, then publishing a review with just that one component's story. The token change is visible on every page that renders the component — include those pages. -Always include the returned reviewUrl in your final user-facing response so the user can open it.`, +Always include the returned reviewUrl in your final user-facing response so the user can open it. This tool maintains a single active review state; each call replaces the previously published review.`, schema: ReviewStateSchema, outputSchema: DisplayReviewOutput, enabled: () => server.ctx.custom?.toolsets?.dev ?? true,