Conversation
🦋 Changeset detectedLatest commit: 55f93ac The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR 💥 An error occurred when fetching the changed packages and changesets in this PR |
commit: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #134 +/- ##
=======================================
Coverage ? 76.16%
=======================================
Files ? 26
Lines ? 646
Branches ? 175
=======================================
Hits ? 492
Misses ? 109
Partials ? 45 ☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will decrease total bundle size by 27.23kB (-100.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for the MCP Apps specification to the Storybook MCP addon, enabling AI agents to render interactive story previews directly in supporting MCP clients. The core change renames the get-story-urls tool to preview-stories and extends it to return both raw URLs and structured data for MCP App rendering.
Changes:
- Renamed
get-story-urlstool topreview-storieswith MCP Apps support via UI resources - Added MCP App rendering infrastructure including HTML templates and browser-side scripts for size reporting
- Upgraded Storybook dependencies from 10.2.0-alpha.14 to 10.3.0-alpha.0
- Updated all documentation, tests, and configuration files to reflect the renamed tool
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Bumped Storybook catalog versions to 10.3.0-alpha.0 |
| pnpm-lock.yaml | Updated lockfile with new Storybook versions and transitive dependencies (including jsdom) |
| packages/addon-mcp/tsdown.config.ts | Added browser build targets for preview.ts and preview-stories-app-script.ts |
| packages/addon-mcp/src/types.ts | Removed unused StoryUrlArray type (replaced by structured PreviewStoriesOutput) |
| packages/addon-mcp/src/tools/preview-stories/preview-stories-app-template.html | New HTML template for MCP App UI with theme support and responsive iframes |
| packages/addon-mcp/src/tools/preview-stories/preview-stories-app-script.ts | New browser-side MCP App protocol implementation for story rendering |
| packages/addon-mcp/src/tools/preview-stories.ts | Renamed and enhanced from get-story-urls.ts with MCP resource support and structured output |
| packages/addon-mcp/src/tools/preview-stories.test.ts | Renamed test file with updated assertions for structuredContent |
| packages/addon-mcp/src/tools/get-storybook-story-instructions.ts | Updated to reference new PREVIEW_STORIES_TOOL_NAME |
| packages/addon-mcp/src/tools/get-storybook-story-instructions.test.ts | Updated test assertions for renamed tool |
| packages/addon-mcp/src/tools/get-story-urls.ts | Deleted (replaced by preview-stories.ts) |
| packages/addon-mcp/src/template.html | Updated tool name display from get-story-urls to preview-stories |
| packages/addon-mcp/src/storybook-story-instructions.md | Updated tool name reference in instructions template |
| packages/addon-mcp/src/preview.ts | New browser-side script for iframe size reporting to MCP App |
| packages/addon-mcp/src/preset.ts | Added previewAnnotations to inject preview.ts into Storybook |
| packages/addon-mcp/src/mcp-handler.ts | Updated tool registration and added resources capability |
| packages/addon-mcp/src/constants.ts | New file with MCP App parameter and event constants |
| packages/addon-mcp/package.json | Added new export paths for preview-annotation and app-script |
| packages/addon-mcp/README.md | Updated documentation with new tool name |
| apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts | Updated E2E tests with new tool schema and structuredContent assertions |
| apps/internal-storybook/.storybook/main.ts | Commented out debug logLevel |
| apps/internal-storybook/.claude/settings.local.json | Updated tool permission from get-story-urls to preview-stories |
| .github/instructions/eval.instructions.md | Fixed play function to use canvas directly |
| .github/instructions/addon-mcp.instructions.md | Updated tool references (contains unresolved merge conflicts) |
| .github/copilot-instructions.md | Updated tool name references in examples |
| .claude/settings.local.json | Updated tool permission reference |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 10 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/addon-mcp/README.md:171
- The README documentation for the Preview Stories tool (lines 156-171) doesn't mention the new MCP Apps capability - that stories can now be rendered directly in the agent chat interface for supporting MCP clients. Consider adding a note about this feature and how it works (e.g., "In MCP clients that support MCP Apps, stories will be rendered directly in the chat interface. Otherwise, the tool returns URLs that users can visit.")
#### 2. Preview Stories (`preview-stories`)
Allows agents to retrieve direct URLs to specific stories in your Storybook. The agent can request URLs for multiple stories by providing:
- `absoluteStoryPath`: Absolute path to the story file
- `exportName`: The export name of the story
- `explicitStoryName`: Optional explicit story name
Example agent usage:
Prompt: I need to see the primary variant of the Button component
Agent calls tool, gets response:
http://localhost:6006/?path=/story/example-button--primary
| let errorMessage = `No story found for export name "${exportName}" with absolute file path "${absoluteStoryPath}"`; | ||
| if (!explicitStoryName) { | ||
| errorMessage += ` (did you forget to pass the explicit story name?)`; | ||
| } |
There was a problem hiding this comment.
Would this error message be something presented to the user? or sent back to the AI/LLM to try and fix?
Is this error message good enough to achieve either or both?
There was a problem hiding this comment.
it's sent back to the LLM, which always triggers the LLM to re-call the tool with the correct arguments. It's not shown to the user.
Ideally it would always call the tool with correct parameters, but I haven't figured that out yet.
| interface McpUiHostStyles { | ||
| variables?: Record<string, string | undefined>; | ||
| css?: { | ||
| fonts?: string; | ||
| }; | ||
| } | ||
|
|
||
| interface McpUiHostContext { | ||
| [key: string]: unknown; | ||
| theme?: 'light' | 'dark'; | ||
| styles?: McpUiHostStyles; | ||
| displayMode?: 'inline' | 'fullscreen' | 'pip'; | ||
| availableDisplayModes?: string[]; | ||
| locale?: string; | ||
| timeZone?: string; | ||
| userAgent?: string; | ||
| platform?: 'web' | 'desktop' | 'mobile'; | ||
| deviceCapabilities?: { | ||
| touch?: boolean; | ||
| hover?: boolean; | ||
| }; | ||
| safeAreaInsets?: { | ||
| top: number; | ||
| right: number; | ||
| bottom: number; | ||
| left: number; | ||
| }; | ||
| } | ||
|
|
||
| interface McpUiInitializeResult { | ||
| protocolVersion: string; | ||
| hostInfo: { | ||
| name: string; | ||
| version: string; | ||
| }; | ||
| hostCapabilities: Record<string, unknown>; | ||
| hostContext: McpUiHostContext; | ||
| [key: string]: unknown; | ||
| } |
There was a problem hiding this comment.
Are there no packages to import these types from?
There was a problem hiding this comment.
Yes there is @modelcontextprotocol/ext-apps, but that's the package that depends on 400 MBs of Bun binaries, I don't want to introduce that to my workflow.
I am in contact with one of the authors that is working on a minimal package that would help us out here, not only with the types, but with utility functions for the MCP host messaging work we do.
|
|
||
| for (const storyResult of stories) { | ||
| if ('error' in storyResult) { | ||
| console.warn('Skipping story with error:', storyResult.error); |
There was a problem hiding this comment.
Can you explain where this logs to?
Is it useful to the AI/LLM? To the user? only for debugging the MCP itself?
There was a problem hiding this comment.
This log goes into VSCode's console, which is only shown when opening VSCode's internal DevTools. The tool result already tells the LLM that the story URL is wrong, so I don't think we need to expose it from here as well.
In fact, if there are any story URLs that returns errors instead, the tool response goes into an isError: true-mode, which makes VSCode not even render the MCP app at all, so I don't even think this code is reachable in practice.
ndelangen
left a comment
ndelangen
left a comment
There was a problem hiding this comment.
Since we cannot fully e2e test this automatically, we should write a detailed manual steps on how development & testing of this repo should happen.
Please automate as many steps as possible, such as building, running the storybooks, and whatever else might be needed to do manual QA.
Then have explicit steps that anyone could follow which has a clear "you should expect to see X" so future us know if everything still functions correctly 1 year from now. 🙏
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 39 changed files in this pull request and generated 10 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/addon-mcp/src/tools/preview-stories.test.ts:502
- The test suite for preview-stories.ts doesn't include tests for the MCP resource registration. Consider adding tests to verify that the resource is correctly registered with the proper URI, MIME type, and metadata (CSP domains, prefersBorder, etc.). This would ensure the MCP App integration is properly tested.
| '@storybook/addon-mcp': patch | ||
| --- | ||
|
|
||
| Add support for MCP App, rendering stories directly in the agent chat in MCP clients that support it |
There was a problem hiding this comment.
The tool has been renamed from get-story-urls to preview-stories, which is a breaking change for users who have configured MCP client permissions to allow/deny the old tool name. While the changeset marks this as a patch, consider documenting this breaking change in the changeset description and providing migration guidance (e.g., updating MCP client configuration files like .claude/settings.local.json).
| Add support for MCP App, rendering stories directly in the agent chat in MCP clients that support it | |
| Add support for MCP App, rendering stories directly in the agent chat in MCP clients that support it. | |
| Note: the story preview tool has been renamed from `get-story-urls` to `preview-stories`. This can be a | |
| breaking change for MCP clients that have explicit allow/deny lists based on tool names. | |
| Migration guidance: | |
| - Update your MCP client configuration (for example, `.claude/settings.local.json` or equivalent) to | |
| allow the new `preview-stories` tool. | |
| - If you previously allowed or denied `get-story-urls`, replace it with `preview-stories` in your | |
| configuration. Some clients may allow you to list both names during a transition period. |
| "description": "If the story has an explicit name set via the "name" propoerty, that is different from the export name, provide it here. | ||
| Otherwise don't set this.", |
There was a problem hiding this comment.
There's a typo in the word "property" - it's spelled as "propoerty" in the test snapshot. This snapshot will need to be updated after fixing the typo in types.ts.
| window.parent.postMessage({ jsonrpc: '2.0', id, method, params }, '*'); | ||
|
|
||
| window.addEventListener('message', function listener(event: MessageEvent) { | ||
| if (event.data?.id !== id) { | ||
| return; | ||
| } | ||
| window.removeEventListener('message', listener); | ||
| if (event.data?.result) { | ||
| resolve(event.data.result); | ||
| } else if (event.data?.error) { | ||
| reject(new Error(String(event.data.error))); | ||
| } | ||
| }); | ||
| return promise; | ||
| } | ||
|
|
||
| function sendHostNotification(method: McpMethod, params: unknown): void { | ||
| window.parent.postMessage({ jsonrpc: '2.0', method, params }, '*'); | ||
| } | ||
|
|
||
| function onHostNotification<T = unknown>( | ||
| method: McpMethod, | ||
| handler: (params: T) => void, | ||
| ): void { | ||
| window.addEventListener('message', function listener(event: MessageEvent) { | ||
| if (event.data?.method === method) { | ||
| handler(event.data.params as T); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
The postMessage calls use a wildcard origin ('*') which could expose the application to security risks. While this follows the MCP Apps specification pattern, the message listeners should validate the event.origin against expected origins (like the Storybook origin from CSP domains). Consider adding origin validation in the message event listeners to ensure messages are only accepted from trusted sources.
| ], | ||
| framework: '@storybook/react-vite', | ||
| logLevel: 'debug', | ||
| // logLevel: 'debug', |
There was a problem hiding this comment.
The logLevel has been commented out, changing from 'debug' to the default level. While this is likely intentional to reduce noise during development, this change should either be committed separately or documented in the PR description since it's not directly related to MCP Apps support.
| explicitStoryName: v.pipe( | ||
| v.optional(v.string()), | ||
| v.description( | ||
| `If the story has an explicit name set via the "name" propoerty, that is different from the export name, provide it here. |
There was a problem hiding this comment.
There's a typo in the word "property" - it's spelled as "propoerty" in the description.
| "type": "string", | ||
| }, | ||
| "explicitStoryName": { | ||
| "description": "If the story has an explicit name set via the "name" propoerty, that is different from the export name, provide it here. |
There was a problem hiding this comment.
There's a typo in the word "property" - it's spelled as "propoerty" in the test snapshot. This snapshot will need to be updated after fixing the typo in types.ts.
| "description": "If the story has an explicit name set via the "name" propoerty, that is different from the export name, provide it here. | |
| "description": "If the story has an explicit name set via the "name" property, that is different from the export name, provide it here. |
| }, | ||
| "storybook": { | ||
| "dependsOn": ["^build"], | ||
| "cache": false, |
There was a problem hiding this comment.
Disabling cache for the storybook task may slow down development workflows. The cache: false setting means Turbo won't cache the storybook task outputs, which could impact build performance. Consider documenting why this change was necessary (e.g., if there are issues with stale caches when developing the MCP App), or investigate if there's a more targeted solution.
| "publint": "turbo run publint", | ||
| "release": "turbo run build && pnpm changeset publish", | ||
| "storybook": "turbo watch storybook", | ||
| "storybook": "turbo watch storybook --filter=@storybook/mcp-internal-storybook", |
There was a problem hiding this comment.
The storybook script now filters to only run @storybook/mcp-internal-storybook, which changes the behavior from running all storybook tasks to just the internal one. This could be a breaking change for developers expecting to run multiple Storybook instances. Consider documenting this change in the commit message or adding a separate script (e.g., storybook:internal) if both behaviors are needed.
| "storybook": "turbo watch storybook --filter=@storybook/mcp-internal-storybook", | |
| "storybook": "turbo watch storybook", | |
| "storybook:internal": "turbo watch storybook --filter=@storybook/mcp-internal-storybook", |
| } | ||
|
|
||
| // is object | ||
| if (typeof value === 'object' && value !== null) { |
There was a problem hiding this comment.
Variable 'value' is of type date, object or regular expression, but it is compared to an expression of type null.
| 'utf-8', | ||
| ); | ||
|
|
||
| const appHtml = appTemplate.replace( |
There was a problem hiding this comment.
The base expression of this property access is always undefined.
| const DEBOUNCE_MS = 100; | ||
|
|
||
| function sendSizeToParent() { | ||
| const height = document.body.scrollHeight; |
There was a problem hiding this comment.
perhaps it's more accurate to check document.documentElement.scrollHeight (and observer document.documentElement in the rest of the file) , since this is more accurately tied to the external iframe's height (in cases where there are paddings/margins, etc)
There was a problem hiding this comment.
I had that at one point, but it would actually be the wrong numbers. Here we specifically want to send the required height of the content, and we assume that the content stretches the body when needed. The problem with using documentElement (or html) is that it is always 100%, so that number is always the height of the iframe regardless of the content, which the parent already knows and controls.
This PR adds support for MCP Apps, as specced in https://github.com/modelcontextprotocol/ext-apps/blob/main/specification/draft/apps.mdx
It changes the current get-story-urls tool. It still just returns URLs as usual, but it contains meta information that instructs any supporting MCP clients to render the UI resource whenever the tool is called. The tool has been renamed to preview-stories.
This is currently supported in VSCode Insiders, or can be debugged further with the MCPJam Inspector.
mcp-app-1.mp4