Handle private composed Storybooks in MCP proxy#235
Conversation
🦋 Changeset detectedLatest commit: c3059b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
commit: |
ff4a46e to
17acd0e
Compare
Bundle ReportChanges will increase total bundle size by 4.7kB (6.1%) ⬆️
Affected Assets, Files, and Routes:view changes for bundle: @storybook/mcp-esmAssets Changed:
Files in
view changes for bundle: @storybook/mcp-proxy-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #235 +/- ##
==========================================
+ Coverage 76.06% 76.79% +0.73%
==========================================
Files 52 53 +1
Lines 1412 1504 +92
Branches 391 420 +29
==========================================
+ Hits 1074 1155 +81
- Misses 205 214 +9
- Partials 133 135 +2 ☔ View full report in Codecov by Sentry. |
17acd0e to
d762a64
Compare
d762a64 to
1ec40a4
Compare
1ec40a4 to
464cfbc
Compare
✅ Deploy Preview for storybook-mcp-self-host-example canceled.
|
464cfbc to
d6c19d0
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Storybook MCP proxy + addon-mcp + @storybook/mcp docs tooling to handle private composed Storybooks without hiding sources or surfacing tool errors. Instead, proxy-originated requests can reach the tools, and private sources render as normal MCP content notices instructing the agent to use the private Storybook’s own /mcp endpoint.
Changes:
- Add a proxy marker header (
X-Storybook-MCP-Proxy: true) and use it in addon-mcp to bypass only the top-level OAuth 401 challenge for proxy-originated requests. - Model “private source” as a
ManifestSourceNoticevalue returned bymanifestProvider, and render it as normal tool output (single- and multi-source). - Update unit/e2e tests and increase internal-storybook startup/registry timeouts for composed Storybooks.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mcp/src/utils/manifest-formatter/markdown.ts | Render per-source notice as normal markdown output in multi-source lists. |
| packages/mcp/src/utils/get-manifest.ts | Extend manifest fetching to support ManifestSourceNotice results and convert notices/errors to MCP content. |
| packages/mcp/src/utils/get-manifest.test.ts | Add coverage for preserving notices and multi-source notice handling. |
| packages/mcp/src/types.ts | Introduce ManifestSourceNotice, ManifestProviderResult, notice?: string on SourceManifests, and a type guard. |
| packages/mcp/src/tools/list-all-documentation.ts | Return notice content (non-error) in single-source mode; support notice-aware utilities. |
| packages/mcp/src/tools/list-all-documentation.test.ts | Verify notices appear in multi-source listing without error: entries. |
| packages/mcp/src/tools/get-documentation.ts | Return notice content (non-error) when manifests resolve to a notice. |
| packages/mcp/src/tools/get-documentation.test.ts | Add test asserting notices are returned as normal content. |
| packages/mcp/src/tools/get-documentation-for-story.ts | Return notice content (non-error) when manifests resolve to a notice. |
| packages/mcp/src/tools/get-documentation-for-story.test.ts | Add test asserting notices are returned as normal content. |
| packages/mcp/src/index.ts | Export new types/helpers for reuse (ManifestSourceNotice, ManifestProviderResult, sourceNoticeToMCPContent, isManifestSourceNotice). |
| packages/mcp-proxy/src/utils/proxy-client.ts | Attach X-Storybook-MCP-Proxy: true to downstream tool-call requests. |
| packages/mcp-proxy/src/utils/proxy-client.test.ts | Assert the proxy marker header is sent. |
| packages/mcp-proxy/src/instructions.md | Document how agents should respond to private-source notices. |
| packages/addon-mcp/src/preset.ts | Bypass top-level OAuth challenge when proxy marker header is present; update manifestProvider typing. |
| packages/addon-mcp/src/preset.test.ts | Add tests for 401 challenge behavior vs proxy-marked requests. |
| packages/addon-mcp/src/mcp-handler.ts | Update manifestProvider typing to ManifestProviderResult. |
| packages/addon-mcp/src/auth/index.ts | Re-export proxy marker constants and proxy-request helper. |
| packages/addon-mcp/src/auth/composition-auth.ts | Keep inconclusive refs in source list; return private-source notices for proxy requests; avoid serving cached manifests to proxy requests. |
| packages/addon-mcp/src/auth/composition-auth.test.ts | Add tests for proxy marker detection and notice/caching behavior. |
| apps/internal-storybook/tests/mcp-endpoint.e2e.test.ts | Increase Storybook startup timeout to 60s. |
| apps/internal-storybook/tests/mcp-composition.e2e.test.ts | Increase composed Storybook startup timeout to 60s. |
| apps/internal-storybook/tests/mcp-composition-auth.e2e.test.ts | Increase composed+auth Storybook startup timeout to 60s. |
| apps/internal-storybook/tests/check-deps.e2e.test.ts | Increase dependency registry check test timeout to 60s. |
| .changeset/private-composition-proxy.md | Changeset documenting the user-facing behavior change in addon-mcp and @storybook/mcp. |
d6c19d0 to
924c7fd
Compare
924c7fd to
2632c4d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 29 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
2632c4d to
e9bb785
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 27 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
| @@ -42,7 +76,11 @@ type MCPErrorResult = { | |||
| * @param error - The error to convert (can be any type) | |||
| * @returns A tool result with error content and isError flag | |||
| */ | |||
| export const errorToMCPContent = (error: unknown): MCPErrorResult => { | |||
| export const errorToMCPContent = (error: unknown): MCPTextResult => { | |||
| if (error instanceof SourceManifestError) { | |||
| return sourceManifestFailureToMCPContent(error.failure); | |||
| } | |||
|
|
|||
| function formatSourceError(error: Extract<SourceManifests, { kind: 'error' }>['error']): string { | ||
| switch (error.kind) { | ||
| case 'requires-own-mcp': | ||
| return error.message; | ||
| case 'fetch-failed': | ||
| return `error: ${error.message}`; | ||
| default: | ||
| return assertNever(error); | ||
| } | ||
| } | ||
|
|
||
| function assertNever(value: never): never { | ||
| throw new Error(`Unhandled source manifest result: ${JSON.stringify(value)}`); | ||
| } |
| @@ -230,8 +271,9 @@ export class CompositionAuth { | |||
| } | |||
|
|
|||
| // Invalid manifest — check /mcp to see if it's an auth issue | |||
| if (await this.#isMcpUnauthorized(new URL(url).origin)) { | |||
| throw new AuthenticationError(url); | |||
| const mcpAuth = await this.#tryCheckMcpAuth(new URL(url).origin); | |||
| if (mcpAuth.unauthorized) { | |||
| throw new AuthenticationError(url, mcpAuth.authRequirement); | |||
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 28 changed files in this pull request and generated no new comments.
Files not reviewed (3)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 33 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- apps/internal-storybook/pnpm-lock.yaml: Language not supported
- eval/pnpm-lock.yaml: Language not supported
- packages/addon-mcp/pnpm-lock.yaml: Language not supported
| const hasDisplayableResult = results.some( | ||
| (result) => result.kind === 'manifest' || result.error.kind === 'requires-own-mcp', | ||
| ); |
TL;DR
The local Storybook MCP proxy cannot authenticate into private composed Storybooks. This PR keeps those sources visible and returns normal MCP guidance telling the agent to use the composed Storybook's own
/mcpendpoint, while direct non-proxy requests keep the existing OAuth challenge behavior.Targets
maindirectly.Behavior Contract
/mcpstill get the OAuth401challenge when composed refs require auth.@storybook/mcp-proxyare marked withX-Storybook-MCP-Proxy: true, soaddon-mcplets verified local proxy traffic reach the docs tools instead of challenging at the route boundary.public,requires-auth, orunknown) rather than parallel auth/source collections.ManifestProviderResult/SourceManifestFailure, not a normal throw/catch path.SourceManifestFailurestays pure data (kind,endpoint,authProvider, or fetch-failure message); user-facing Markdown is formatted in@storybook/mcp.Review Order
packages/mcp-proxy/src/utils/proxy-client.tsadds the request header;packages/addon-mcp/src/preset.tsonly bypasses the top-level OAuth challenge for verified local proxy requests.packages/addon-mcp/src/auth/composition-auth.tsowns per-source auth state and creates a per-request manifest provider from explicit request access mode.packages/mcp/src/types.tsandpackages/mcp/src/utils/get-manifest.tsmodel source failures as result data viaManifestProviderResult/getManifestResult.packages/mcp/src/tools/*documentation*.tsandpackages/mcp/src/utils/manifest-formatter/*render source-failure data as normal tool content/list entries.packages/addon-mcp/src/mcp-handler.tsandpackages/addon-mcp/src/preset.tsenable docs tools/status for composed remote sources even without local manifests.packages/addon-mcp/src/auth/composition-auth.fetch.test.tssplits fetch/cache behavior out of the auth-state tests.Test Plan
pnpm exec oxfmt --check packages/addon-mcp/src/auth/composition-auth.ts packages/addon-mcp/src/auth/composition-auth.test.ts packages/addon-mcp/src/auth/composition-auth.fetch.test.ts packages/addon-mcp/src/auth/index.ts packages/addon-mcp/src/mcp-handler.ts packages/addon-mcp/src/mcp-handler.test.ts packages/addon-mcp/src/preset.ts packages/addon-mcp/src/preset.test.ts packages/mcp/src/types.ts packages/mcp/src/index.ts packages/mcp/src/utils/get-manifest.ts packages/mcp/src/tools/get-documentation.ts packages/mcp/src/tools/get-documentation-for-story.ts packages/mcp/src/tools/list-all-documentation.ts packages/mcp/src/tools/get-documentation.test.ts packages/mcp/src/tools/get-documentation-for-story.test.ts packages/mcp/src/tools/list-all-documentation.test.ts packages/mcp/src/utils/get-manifest.test.ts packages/mcp/README.mdgit diff --checkpnpm turbo run typecheck --filter=@storybook/addon-mcp --filter=@storybook/mcppnpm vitest --project=@storybook/mcp --runpnpm vitest --project=@storybook/addon-mcp --run