diff --git a/.changeset/loud-maps-promise.md b/.changeset/loud-maps-promise.md new file mode 100644 index 00000000..6780ca52 --- /dev/null +++ b/.changeset/loud-maps-promise.md @@ -0,0 +1,6 @@ +--- +"@storybook/addon-mcp": patch +"@storybook/mcp": patch +--- + +Show private composed Storybooks as own-MCP guidance when accessed through the local MCP proxy. diff --git a/packages/addon-mcp/src/auth/composition-auth.test.ts b/packages/addon-mcp/src/auth/composition-auth.test.ts index 2de08b7e..01d81185 100644 --- a/packages/addon-mcp/src/auth/composition-auth.test.ts +++ b/packages/addon-mcp/src/auth/composition-auth.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { RequiresOwnMcpError } from '@storybook/mcp'; import { CompositionAuth, extractBearerToken } from './composition-auth.ts'; describe('CompositionAuth', () => { @@ -326,6 +327,54 @@ describe('CompositionAuth', () => { ); }); + it('throws requires-own-mcp for trusted proxy requests without a token', async () => { + const auth = new CompositionAuth(); + + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValueOnce({ + ok: false, + status: 401, + headers: new Headers({ + 'WWW-Authenticate': + 'Bearer resource_metadata="http://remote.example.com/.well-known/oauth-protected-resource"', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + resource: 'http://remote.example.com/mcp', + authorization_servers: ['http://auth.example.com'], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + issuer: 'http://auth.example.com', + authorization_endpoint: 'http://auth.example.com/authorize', + token_endpoint: 'http://auth.example.com/token', + }), + }), + ); + + const source = { id: 'remote', title: 'Remote', url: 'http://remote.example.com' }; + await auth.initialize([source]); + + const provider = auth.createManifestProvider('http://localhost:6006', { + requiresOwnMcpForUnauthenticatedRequests: true, + }); + const request = new Request('http://localhost:6006/mcp'); + + await expect(provider(request, './manifests/components.json', source)).rejects.toBeInstanceOf( + RequiresOwnMcpError, + ); + expect(auth.hadAuthError(request)).toBe(false); + }); + it('throws auth error when response is invalid manifest and /mcp returns 401', async () => { const auth = new CompositionAuth(); diff --git a/packages/addon-mcp/src/auth/composition-auth.ts b/packages/addon-mcp/src/auth/composition-auth.ts index 1a256466..b5404267 100644 --- a/packages/addon-mcp/src/auth/composition-auth.ts +++ b/packages/addon-mcp/src/auth/composition-auth.ts @@ -6,7 +6,12 @@ * MCP clients like VS Code to handle the OAuth flow with Chromatic. */ -import { ComponentManifestMap, DocsManifestMap, type Source } from '@storybook/mcp'; +import { + ComponentManifestMap, + DocsManifestMap, + RequiresOwnMcpError, + type Source, +} from '@storybook/mcp'; import * as v from 'valibot'; export interface ComposedRef { @@ -41,6 +46,10 @@ export type ManifestProvider = ( path: string, source?: Source, ) => Promise; +type RemoteSource = Source & { url: string }; + +export const STORYBOOK_MCP_PROXY_HEADER = 'X-Storybook-MCP-Proxy'; +export const STORYBOOK_MCP_PROXY_HEADER_VALUE = 'true'; const MANIFEST_CACHE_TTL = 60 * 60 * 1000; // 60 minutes const REVALIDATION_TTL = 60 * 1000; // 60 seconds @@ -144,14 +153,26 @@ export class CompositionAuth { } /** Create a manifest provider for multi-source mode. */ - createManifestProvider(localOrigin: string): ManifestProvider { + createManifestProvider( + localOrigin: string, + options: { requiresOwnMcpForUnauthenticatedRequests?: boolean } = {}, + ): ManifestProvider { return async (request, path, source) => { const token = extractBearerToken(request?.headers.get('Authorization')); - const baseUrl = source?.url ?? localOrigin; + const remoteSource: RemoteSource | undefined = source?.url + ? { ...source, url: source.url } + : undefined; + const baseUrl = remoteSource?.url ?? localOrigin; const manifestUrl = `${baseUrl}${path.replace('./', '/')}`; - const isRemote = !!source?.url; + const isRemote = !!remoteSource; const needsAuth = isRemote && this.#isAuthRequiredUrl(baseUrl); const tokenForRequest = needsAuth ? token : null; + const shouldUseOwnMcp = + isRemote && !token && options.requiresOwnMcpForUnauthenticatedRequests; + + if (needsAuth && shouldUseOwnMcp && remoteSource) { + throw new RequiresOwnMcpError(remoteSource); + } // New token = user re-authenticated, invalidate all cached manifests if (token && token !== this.#lastToken) { @@ -197,6 +218,9 @@ export class CompositionAuth { return text; } catch (error) { if (error instanceof AuthenticationError && request) { + if (shouldUseOwnMcp && remoteSource) { + throw new RequiresOwnMcpError(remoteSource); + } this.#authErrors.set(request, error); } throw error; @@ -355,3 +379,13 @@ export function extractBearerToken( const bearer = values.find((value) => typeof value === 'string' && value.startsWith('Bearer ')); return bearer ? bearer.slice(7) : null; } + +export function isStorybookMcpProxyRequest( + headerValue: string | string[] | null | undefined, +): boolean { + const values = Array.isArray(headerValue) ? headerValue : [headerValue]; + return values.some( + (value) => + typeof value === 'string' && value.trim().toLowerCase() === STORYBOOK_MCP_PROXY_HEADER_VALUE, + ); +} diff --git a/packages/addon-mcp/src/auth/index.ts b/packages/addon-mcp/src/auth/index.ts index e507688c..469d3b1a 100644 --- a/packages/addon-mcp/src/auth/index.ts +++ b/packages/addon-mcp/src/auth/index.ts @@ -5,7 +5,9 @@ export { CompositionAuth, AuthenticationError, + STORYBOOK_MCP_PROXY_HEADER, extractBearerToken, + isStorybookMcpProxyRequest, type ComposedRef, type ManifestProvider, } from './composition-auth.ts'; diff --git a/packages/addon-mcp/src/preset.test.ts b/packages/addon-mcp/src/preset.test.ts index 5df48aff..1e266262 100644 --- a/packages/addon-mcp/src/preset.test.ts +++ b/packages/addon-mcp/src/preset.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { Options } from 'storybook/internal/types'; +import { RequiresOwnMcpError } from '@storybook/mcp'; 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'; @@ -35,6 +37,58 @@ describe('experimental_devServer', () => { vi.unstubAllGlobals(); }); + const stubPrivateRefDiscovery = () => { + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValueOnce({ + ok: false, + status: 401, + headers: new Headers({ + 'WWW-Authenticate': + 'Bearer resource_metadata="https://private.example.com/.well-known/oauth-protected-resource"', + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + resource: 'https://private.example.com/mcp', + authorization_servers: ['https://auth.example.com'], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + }), + }), + ); + }; + + const createOptionsWithPrivateRef = () => + ({ + ...mockOptions, + port: 6006, + presets: { + apply: vi.fn((key: string) => { + if (key === 'refs') { + return Promise.resolve({ + private: { title: 'Private', url: 'https://private.example.com' }, + }); + } + if (key === 'features') { + return Promise.resolve({ componentsManifest: false }); + } + return Promise.resolve(undefined); + }), + }, + }) as unknown as Options; + it('should register /mcp POST endpoint', async () => { await (experimental_devServer as any)(mockApp, mockOptions); @@ -196,6 +250,63 @@ describe('experimental_devServer', () => { ); }); + it('should keep direct unauthenticated requests on the OAuth challenge path', async () => { + const mcpServerHandler = vi + .spyOn(mcpHandlerModule, 'mcpServerHandler') + .mockResolvedValue(undefined); + stubPrivateRefDiscovery(); + + await (experimental_devServer as any)(mockApp, createOptionsWithPrivateRef()); + + const mockRes = { writeHead: vi.fn(), end: vi.fn() } as any; + await mcpHandler( + { + headers: {}, + }, + mockRes, + ); + + expect(mockRes.writeHead).toHaveBeenCalledWith( + 401, + expect.objectContaining({ + 'WWW-Authenticate': expect.stringContaining('/.well-known/oauth-protected-resource'), + }), + ); + expect(mockRes.end).toHaveBeenCalledWith('401 - Unauthorized'); + expect(mcpServerHandler).not.toHaveBeenCalled(); + }); + + it('should let proxy requests reach MCP with a requires-own-mcp manifest provider', async () => { + const mcpServerHandler = vi + .spyOn(mcpHandlerModule, 'mcpServerHandler') + .mockResolvedValue(undefined); + stubPrivateRefDiscovery(); + + await (experimental_devServer as any)(mockApp, createOptionsWithPrivateRef()); + + const mockRes = { writeHead: vi.fn(), end: vi.fn() } as any; + await mcpHandler( + { + headers: { [STORYBOOK_MCP_PROXY_HEADER.toLowerCase()]: 'true' }, + }, + mockRes, + ); + + expect(mockRes.writeHead).not.toHaveBeenCalledWith(401, expect.anything()); + expect(mcpServerHandler).toHaveBeenCalledTimes(1); + + const { manifestProvider, sources } = mcpServerHandler.mock.calls[0]![0]; + expect(manifestProvider).toBeDefined(); + expect(sources).toBeDefined(); + await expect( + manifestProvider!( + new Request('http://localhost:6006/mcp'), + './manifests/components.json', + sources![1], + ), + ).rejects.toBeInstanceOf(RequiresOwnMcpError); + }); + it('should serve HTML for browser GET requests', async () => { let getHandler: any; mockApp.get = vi.fn((path, handler) => { diff --git a/packages/addon-mcp/src/preset.ts b/packages/addon-mcp/src/preset.ts index 45cad3a9..9b4e3680 100644 --- a/packages/addon-mcp/src/preset.ts +++ b/packages/addon-mcp/src/preset.ts @@ -9,7 +9,9 @@ import htmlTemplate from './template.html'; import path from 'node:path'; import { CompositionAuth, + STORYBOOK_MCP_PROXY_HEADER, extractBearerToken, + isStorybookMcpProxyRequest as hasStorybookMcpProxyHeader, type ComposedRef, type ManifestProvider, } from './auth/index.ts'; @@ -18,6 +20,7 @@ import type { Source } from '@storybook/mcp'; import type { IncomingMessage, ServerResponse } from 'node:http'; const DEFAULT_MCP_ENDPOINT = '/mcp'; +const STORYBOOK_MCP_PROXY_HEADER_KEY = STORYBOOK_MCP_PROXY_HEADER.toLowerCase(); export const previewAnnotations: PresetPropertyFn<'previewAnnotations'> = async ( existingAnnotations = [], @@ -46,7 +49,7 @@ export const experimental_devServer: PresetPropertyFn< // Build sources and manifest provider let sources: Source[] | undefined; - let manifestProvider: ManifestProvider | undefined; + let createManifestProvider: ((req: IncomingMessage) => ManifestProvider) | undefined; if (refs.length > 0) { logger.info(`Initializing composition with ${refs.length} remote Storybook(s)`); @@ -60,7 +63,10 @@ export const experimental_devServer: PresetPropertyFn< logger.info(`Sources: ${sources.map((s) => s.id).join(', ')}`); // Create manifest provider that handles multi-source - manifestProvider = compositionAuth.createManifestProvider(origin); + createManifestProvider = (req) => + compositionAuth.createManifestProvider(origin, { + requiresOwnMcpForUnauthenticatedRequests: isStorybookMcpProxyHttpRequest(req), + }); } // Serve .well-known/oauth-protected-resource for MCP auth @@ -78,7 +84,7 @@ export const experimental_devServer: PresetPropertyFn< const requireAuth = (req: IncomingMessage, res: ServerResponse): boolean => { const token = extractBearerToken(req.headers['authorization']); - if (compositionAuth.requiresAuth && !token) { + if (compositionAuth.requiresAuth && !token && !isStorybookMcpProxyHttpRequest(req)) { res.writeHead(401, { 'Content-Type': 'text/plain', 'WWW-Authenticate': compositionAuth.buildWwwAuthenticate(origin), @@ -98,7 +104,7 @@ export const experimental_devServer: PresetPropertyFn< options, addonOptions, sources, - manifestProvider, + manifestProvider: createManifestProvider?.(req), compositionAuth, }); }); @@ -121,7 +127,7 @@ export const experimental_devServer: PresetPropertyFn< options, addonOptions, sources, - manifestProvider, + manifestProvider: createManifestProvider?.(req), compositionAuth, }); } @@ -180,6 +186,12 @@ export const features: PresetPropertyFn<'features'> = async (existingFeatures) = }; }; +function isStorybookMcpProxyHttpRequest(req: IncomingMessage): boolean { + const headerValue = + req.headers[STORYBOOK_MCP_PROXY_HEADER_KEY] ?? req.headers[STORYBOOK_MCP_PROXY_HEADER]; + return hasStorybookMcpProxyHeader(headerValue); +} + /** * Get composed Storybook refs from Storybook config. * See: https://storybook.js.org/docs/sharing/storybook-composition diff --git a/packages/mcp-proxy/src/utils/proxy-client.test.ts b/packages/mcp-proxy/src/utils/proxy-client.test.ts index 43f67eba..774dbabe 100644 --- a/packages/mcp-proxy/src/utils/proxy-client.test.ts +++ b/packages/mcp-proxy/src/utils/proxy-client.test.ts @@ -47,6 +47,7 @@ describe('proxyToolCall', () => { const init = call[1] as RequestInit; const headers = init.headers as Record; expect(headers.Accept).toBe('application/json, text/event-stream'); + expect(headers['X-Storybook-MCP-Proxy']).toBe('true'); const body = JSON.parse(init.body as string); expect(body).toMatchObject({ jsonrpc: '2.0', diff --git a/packages/mcp-proxy/src/utils/proxy-client.ts b/packages/mcp-proxy/src/utils/proxy-client.ts index 213598c9..68ec6e0d 100644 --- a/packages/mcp-proxy/src/utils/proxy-client.ts +++ b/packages/mcp-proxy/src/utils/proxy-client.ts @@ -4,6 +4,9 @@ import type { StorybookInstanceRecordV1, } from '../types/index.ts'; +const STORYBOOK_MCP_PROXY_HEADER = 'X-Storybook-MCP-Proxy'; +const STORYBOOK_MCP_PROXY_HEADER_VALUE = 'true'; + /** * Forward an MCP `tools/call` JSON-RPC request to a local Storybook MCP server. * @@ -28,6 +31,7 @@ export async function proxyToolCall( headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/event-stream', + [STORYBOOK_MCP_PROXY_HEADER]: STORYBOOK_MCP_PROXY_HEADER_VALUE, }, body: JSON.stringify({ jsonrpc: '2.0', diff --git a/packages/mcp/README.md b/packages/mcp/README.md index f32c5921..63d4d4f7 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -260,10 +260,24 @@ type SourceManifests = { componentManifest: ComponentManifestMap; docsManifest?: DocsManifestMap; error?: string; + notice?: RequiresOwnMcpNotice; }; ``` -Represents fetched manifests (or an error) for a single source. +Represents fetched manifests, an error, or non-error routing guidance for a single source. + +#### `RequiresOwnMcpNotice` + +Type: + +```ts +type RequiresOwnMcpNotice = { + kind: 'requires-own-mcp'; + endpoint: string; +}; +``` + +Indicates that a composed Storybook source cannot be read through the local MCP proxy and should be accessed through its own MCP endpoint instead. #### Tool registration exports diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index ffe9f378..0ebec533 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -22,11 +22,12 @@ export { export { COMPONENT_MANIFEST_PATH, DOCS_MANIFEST_PATH, + RequiresOwnMcpError, getMultiSourceManifests, } from './utils/get-manifest.ts'; // Export types for reuse -export type { StorybookContext, Source, SourceManifests } from './types.ts'; +export type { RequiresOwnMcpNotice, StorybookContext, Source, SourceManifests } from './types.ts'; // copied from tmcp internals as it's not exposed type InitializeRequestParams = { diff --git a/packages/mcp/src/tools/get-documentation-for-story.test.ts b/packages/mcp/src/tools/get-documentation-for-story.test.ts index f08399cb..dfa0b571 100644 --- a/packages/mcp/src/tools/get-documentation-for-story.test.ts +++ b/packages/mcp/src/tools/get-documentation-for-story.test.ts @@ -5,7 +5,7 @@ import { addGetStoryDocumentationTool, GET_STORY_TOOL_NAME, } from './get-documentation-for-story.ts'; -import type { StorybookContext } from '../types.ts'; +import type { Source, StorybookContext } from '../types.ts'; import smallManifestFixture from '../../fixtures/small-manifest.fixture.json' with { type: 'json' }; import * as getManifest from '../utils/get-manifest.ts'; @@ -376,5 +376,38 @@ describe('getComponentStoryDocumentationTool', () => { expect((response.result as any).content[0].text).toContain('# Badge - Default'); expect(getManifestSpy).toHaveBeenCalledWith(mockHttpRequest, undefined, sources[1]); }); + + it('should return a routing notice when the selected source requires its own MCP', async () => { + const remoteSource = sources[1] as Source & { url: string }; + getManifestSpy.mockRejectedValue(new getManifest.RequiresOwnMcpError(remoteSource)); + + const request = { + jsonrpc: '2.0' as const, + id: 1, + method: 'tools/call', + params: { + name: GET_STORY_TOOL_NAME, + arguments: { + componentId: 'badge', + storyName: 'Default', + storybookId: 'remote', + }, + }, + }; + + const mockHttpRequest = new Request('https://example.com/mcp'); + const response = await server.receive(request, { + custom: { request: mockHttpRequest, sources }, + }); + + expect((response.result as any).isError).toBeUndefined(); + expect((response.result as any).content[0].text).toBe(`# Remote +id: remote + +This composed Storybook is private and cannot be read through the local Storybook MCP proxy. + +Use this source's own MCP endpoint instead: +http://remote.example.com/mcp`); + }); }); }); diff --git a/packages/mcp/src/tools/get-documentation.test.ts b/packages/mcp/src/tools/get-documentation.test.ts index be5e12e6..c2b2b955 100644 --- a/packages/mcp/src/tools/get-documentation.test.ts +++ b/packages/mcp/src/tools/get-documentation.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { McpServer } from 'tmcp'; import { ValibotJsonSchemaAdapter } from '@tmcp/adapter-valibot'; import { addGetDocumentationTool, GET_TOOL_NAME } from './get-documentation.ts'; -import type { StorybookContext } from '../types.ts'; +import type { Source, StorybookContext } from '../types.ts'; import smallManifestFixture from '../../fixtures/small-manifest.fixture.json' with { type: 'json' }; import smallDocsManifestFixture from '../../fixtures/small-docs-manifest.fixture.json' with { type: 'json' }; import * as getManifest from '../utils/get-manifest.ts'; @@ -645,6 +645,35 @@ describe('getDocumentationTool', () => { expect((response.result as any).content[0].text).toContain('in source "local"'); }); + it('should return a routing notice when the selected source requires its own MCP', async () => { + const remoteSource = sources[1] as Source & { url: string }; + getManifestsSpy.mockRejectedValue(new getManifest.RequiresOwnMcpError(remoteSource)); + + const request = { + jsonrpc: '2.0' as const, + id: 1, + method: 'tools/call', + params: { + name: GET_TOOL_NAME, + arguments: { id: 'badge', storybookId: 'remote' }, + }, + }; + + const mockHttpRequest = new Request('https://example.com/mcp'); + const response = await server.receive(request, { + custom: { request: mockHttpRequest, sources }, + }); + + expect((response.result as any).isError).toBeUndefined(); + expect((response.result as any).content[0].text).toBe(`# Remote +id: remote + +This composed Storybook is private and cannot be read through the local Storybook MCP proxy. + +Use this source's own MCP endpoint instead: +http://remote.example.com/mcp`); + }); + it('should call onGetDocumentation with storybookId', async () => { getManifestsSpy.mockResolvedValue({ componentManifest: smallManifestFixture, diff --git a/packages/mcp/src/tools/list-all-documentation.test.ts b/packages/mcp/src/tools/list-all-documentation.test.ts index 16187e79..4436e4fd 100644 --- a/packages/mcp/src/tools/list-all-documentation.test.ts +++ b/packages/mcp/src/tools/list-all-documentation.test.ts @@ -245,6 +245,56 @@ describe('listAllDocumentationTool', () => { getMultiSourceManifestsSpy.mockRestore(); }); + + it('should show private composed sources as routing notices', async () => { + const getMultiSourceManifestsSpy = vi.spyOn(getManifest, 'getMultiSourceManifests'); + getMultiSourceManifestsSpy.mockResolvedValue([ + { + source: sources[0]!, + componentManifest: smallManifestFixture, + }, + { + source: { + id: 'tetra', + title: 'Tetra Design System', + url: 'https://tetra.chromatic.com', + }, + componentManifest: { v: 1, components: {} }, + notice: { + kind: 'requires-own-mcp', + endpoint: 'https://tetra.chromatic.com/mcp', + }, + }, + ]); + + const request = { + jsonrpc: '2.0' as const, + id: 1, + method: 'tools/call', + params: { + name: LIST_TOOL_NAME, + arguments: {}, + }, + }; + + const mockHttpRequest = new Request('https://example.com/mcp'); + const response = await server.receive(request, { + custom: { request: mockHttpRequest, sources }, + }); + + const text = (response.result as any).content[0].text; + expect(text).toContain('# Local'); + expect(text).toContain('Button (button)'); + expect(text).toContain('# Tetra Design System'); + expect(text).toContain('id: tetra'); + expect(text).toContain( + 'This composed Storybook is private and cannot be read through the local Storybook MCP proxy.', + ); + expect(text).toContain('https://tetra.chromatic.com/mcp'); + expect(text).not.toContain('error:'); + + getMultiSourceManifestsSpy.mockRestore(); + }); }); it('should handle fetch errors gracefully', async () => { diff --git a/packages/mcp/src/tools/list-all-documentation.ts b/packages/mcp/src/tools/list-all-documentation.ts index baefcabc..55ddfb38 100644 --- a/packages/mcp/src/tools/list-all-documentation.ts +++ b/packages/mcp/src/tools/list-all-documentation.ts @@ -50,7 +50,7 @@ export async function addListAllDocumentationTool( withStoryIds, }); - const firstSuccess = multiSourceManifests.find((m) => !m.error); + const firstSuccess = multiSourceManifests.find((m) => !m.error && !m.notice); if (firstSuccess) { await ctx.onListAllDocumentation?.({ context: ctx, diff --git a/packages/mcp/src/types.ts b/packages/mcp/src/types.ts index f0c64c89..1f2daff5 100644 --- a/packages/mcp/src/types.ts +++ b/packages/mcp/src/types.ts @@ -13,6 +13,11 @@ export type Source = { url?: string; }; +export type RequiresOwnMcpNotice = { + kind: 'requires-own-mcp'; + endpoint: string; +}; + /** * All manifests for a single source. */ @@ -22,6 +27,8 @@ export type SourceManifests = { docsManifest?: DocsManifestMap; /** Error message if fetching this source failed */ error?: string; + /** Non-error guidance for sources that must be accessed through their own MCP endpoint */ + notice?: RequiresOwnMcpNotice; }; /** diff --git a/packages/mcp/src/utils/get-manifest.test.ts b/packages/mcp/src/utils/get-manifest.test.ts index 59fa58bc..33fa107e 100644 --- a/packages/mcp/src/utils/get-manifest.test.ts +++ b/packages/mcp/src/utils/get-manifest.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { getManifests, getMultiSourceManifests, ManifestGetError } from './get-manifest.ts'; +import { + getManifests, + getMultiSourceManifests, + ManifestGetError, + RequiresOwnMcpError, +} from './get-manifest.ts'; import type { ComponentManifestMap, DocsManifestMap, Source } from '../types.ts'; global.fetch = vi.fn(); @@ -451,7 +456,7 @@ Invalid key: Expected "v" but received undefined]`); describe('getMultiSourceManifests', () => { const localSource: Source = { id: 'local', title: 'Local' }; - const remoteSource: Source = { + const remoteSource: Source & { url: string } = { id: 'remote', title: 'Remote', url: 'http://remote.example.com', @@ -531,6 +536,34 @@ Invalid key: Expected "v" but received undefined]`); expect(results[1]!.componentManifest).toEqual({ v: 1, components: {} }); }); + it('should capture requires-own-mcp notices without treating them as failed output', async () => { + const manifestProvider = vi + .fn() + .mockImplementation((_req: Request | undefined, path: string, source?: Source) => { + if (source?.id === 'remote') { + return Promise.reject(new RequiresOwnMcpError(remoteSource)); + } + if (path.includes('docs.json')) { + return Promise.reject(new Error('Not found')); + } + return Promise.resolve(JSON.stringify(localManifest)); + }); + + const results = await getMultiSourceManifests( + [localSource, remoteSource], + undefined, + manifestProvider, + ); + + expect(results).toHaveLength(2); + expect(results[0]!.error).toBeUndefined(); + expect(results[1]!.error).toBeUndefined(); + expect(results[1]!.notice).toEqual({ + kind: 'requires-own-mcp', + endpoint: 'http://remote.example.com/mcp', + }); + }); + it('should throw when all sources fail', async () => { const manifestProvider = vi.fn().mockRejectedValue(new Error('Failed')); diff --git a/packages/mcp/src/utils/get-manifest.ts b/packages/mcp/src/utils/get-manifest.ts index 1f94faed..28a02066 100644 --- a/packages/mcp/src/utils/get-manifest.ts +++ b/packages/mcp/src/utils/get-manifest.ts @@ -6,6 +6,9 @@ import { type SourceManifests, } from '../types.ts'; import * as v from 'valibot'; +import { formatRequiresOwnMcpNotice, getSourceMcpEndpoint } from './requires-own-mcp.ts'; + +type SourceWithUrl = Source & { url: string }; /** * The paths to the manifest files relative to the Storybook build @@ -28,12 +31,25 @@ export class ManifestGetError extends Error { } } +export class RequiresOwnMcpError extends Error { + public readonly source: SourceWithUrl; + public readonly endpoint: string; + + constructor(source: SourceWithUrl) { + const endpoint = getSourceMcpEndpoint(source); + super(`Composed Storybook "${source.title}" requires its own MCP endpoint: ${endpoint}`); + this.name = 'RequiresOwnMcpError'; + this.source = source; + this.endpoint = endpoint; + } +} + /** - * MCP tool result type for error responses + * MCP tool result type for text responses */ -type MCPErrorResult = { +type MCPTextResult = { content: Array<{ type: 'text'; text: string }>; - isError: true; + isError?: true; }; /** @@ -42,7 +58,18 @@ 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 RequiresOwnMcpError) { + return { + content: [ + { + type: 'text', + text: formatRequiresOwnMcpNotice(error.source, error.endpoint), + }, + ], + }; + } + const errorPrefix = error instanceof ManifestGetError ? 'Error getting manifest' : 'Unexpected error'; const errorMessage = error instanceof Error ? error.message : String(error); @@ -121,6 +148,9 @@ export async function getManifests( if (componentResult.status === 'rejected') { const reason = componentResult.reason; + if (reason instanceof RequiresOwnMcpError) { + throw reason; + } const is404 = reason instanceof ManifestGetError && reason.message.includes('404'); const hint = is404 ? `\nHint: The Storybook at this URL may not have the component manifest enabled. Add \`features: { componentsManifest: true }\` (or \`features: { experimentalComponentsManifest: true }\` for older Storybook versions) to its main.ts config.` @@ -234,6 +264,16 @@ export async function getMultiSourceManifests( }; } catch (error) { // Capture error but don't fail the entire request + if (error instanceof RequiresOwnMcpError) { + return { + source, + componentManifest: { v: 1, components: {} }, + notice: { + kind: 'requires-own-mcp' as const, + endpoint: error.endpoint, + }, + }; + } const errorMessage = error instanceof Error ? error.message : String(error); return { source, @@ -244,9 +284,10 @@ export async function getMultiSourceManifests( }), ); - // Check if at least one source succeeded - const successCount = results.filter((r) => !r.error).length; - if (successCount === 0) { + // Check if at least one source produced useful tool output. + const successCount = results.filter((r) => !r.error && !r.notice).length; + const noticeCount = results.filter((r) => r.notice).length; + if (successCount === 0 && noticeCount === 0) { throw new ManifestGetError( `Failed to fetch manifests from any source. Errors:\n${results.map((r) => `- ${r.source.title}: ${r.error}`).join('\n')}`, ); diff --git a/packages/mcp/src/utils/manifest-formatter/markdown.test.ts b/packages/mcp/src/utils/manifest-formatter/markdown.test.ts index fe07039d..990a4a86 100644 --- a/packages/mcp/src/utils/manifest-formatter/markdown.test.ts +++ b/packages/mcp/src/utils/manifest-formatter/markdown.test.ts @@ -1,7 +1,11 @@ import { describe, it, expect } from 'vitest'; import type { AllManifests, ComponentManifest, ComponentManifestMap } from '../../types.ts'; import fullManifestFixture from '../../../fixtures/full-manifest.fixture.json' with { type: 'json' }; -import { formatComponentManifest, formatManifestsToLists } from './markdown.ts'; +import { + formatComponentManifest, + formatManifestsToLists, + formatMultiSourceManifestsToLists, +} from './markdown.ts'; describe('MarkdownFormatter - formatComponentManifest', () => { it('formats all full fixtures', () => { @@ -1061,6 +1065,32 @@ describe('MarkdownFormatter - formatComponentManifest', () => { }); }); +describe('MarkdownFormatter - formatMultiSourceManifestsToLists', () => { + it('formats requires-own-mcp source notices without an error prefix', () => { + const result = formatMultiSourceManifestsToLists([ + { + source: { id: 'tetra', title: 'Tetra Design System', url: 'https://tetra.chromatic.com' }, + componentManifest: { v: 1, components: {} }, + notice: { + kind: 'requires-own-mcp', + endpoint: 'https://tetra.chromatic.com/mcp', + }, + }, + ]); + + expect(result).toMatchInlineSnapshot(` + "# Tetra Design System + id: tetra + + This composed Storybook is private and cannot be read through the local Storybook MCP proxy. + + Use this source's own MCP endpoint instead: + https://tetra.chromatic.com/mcp" + `); + expect(result).not.toContain('error:'); + }); +}); + describe('MarkdownFormatter - formatManifestsToLists', () => { it('formats the full manifest fixture', () => { const result = formatManifestsToLists({ diff --git a/packages/mcp/src/utils/manifest-formatter/markdown.ts b/packages/mcp/src/utils/manifest-formatter/markdown.ts index 25f2799e..85b44518 100644 --- a/packages/mcp/src/utils/manifest-formatter/markdown.ts +++ b/packages/mcp/src/utils/manifest-formatter/markdown.ts @@ -14,6 +14,7 @@ import { } from '../parse-react-docgen.ts'; import { dedent } from '../dedent.ts'; import { extractDocsSummary, MAX_SUMMARY_LENGTH } from './extract-docs-summary.ts'; +import { formatRequiresOwnMcpNotice } from '../requires-own-mcp.ts'; /** * Maximum number of stories to show in full detail in component manifests. @@ -357,7 +358,7 @@ export function formatMultiSourceManifestsToLists( ): string { const parts: string[] = []; - for (const { source, componentManifest, docsManifest, error } of manifests) { + for (const { source, componentManifest, docsManifest, error, notice } of manifests) { parts.push(`# ${source.title}`); parts.push(`id: ${source.id}`); parts.push(''); @@ -368,6 +369,12 @@ export function formatMultiSourceManifestsToLists( continue; } + if (notice) { + parts.push(formatRequiresOwnMcpNotice(source, notice.endpoint, { includeHeader: false })); + parts.push(''); + continue; + } + const components = Object.values(componentManifest.components); if (components.length > 0) { parts.push('## Components'); diff --git a/packages/mcp/src/utils/requires-own-mcp.test.ts b/packages/mcp/src/utils/requires-own-mcp.test.ts new file mode 100644 index 00000000..085c06e1 --- /dev/null +++ b/packages/mcp/src/utils/requires-own-mcp.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, it } from 'vitest'; +import { getSourceMcpEndpoint } from './requires-own-mcp.ts'; + +describe('getSourceMcpEndpoint', () => { + it.each([ + ['https://example.com', 'https://example.com/mcp'], + ['https://example.com/storybook/', 'https://example.com/storybook/mcp'], + ['https://example.com/storybook/?foo=bar#section', 'https://example.com/storybook/mcp'], + ])('returns the source-specific MCP endpoint for %s', (url, expected) => { + expect(getSourceMcpEndpoint({ id: 'remote', title: 'Remote', url })).toBe(expected); + }); +}); diff --git a/packages/mcp/src/utils/requires-own-mcp.ts b/packages/mcp/src/utils/requires-own-mcp.ts new file mode 100644 index 00000000..0f553835 --- /dev/null +++ b/packages/mcp/src/utils/requires-own-mcp.ts @@ -0,0 +1,32 @@ +import type { Source } from '../types.ts'; + +type SourceWithUrl = Source & { url: string }; + +export function getSourceMcpEndpoint(source: SourceWithUrl): string { + const base = new URL(source.url); + base.pathname = `${base.pathname.replace(/\/$/, '')}/`; + return new URL('mcp', base).toString(); +} + +export function formatRequiresOwnMcpNotice( + source: Source, + endpoint: string, + options: { includeHeader?: boolean } = {}, +): string { + const parts: string[] = []; + + if (options.includeHeader ?? true) { + parts.push(`# ${source.title}`); + parts.push(`id: ${source.id}`); + parts.push(''); + } + + parts.push( + 'This composed Storybook is private and cannot be read through the local Storybook MCP proxy.', + ); + parts.push(''); + parts.push("Use this source's own MCP endpoint instead:"); + parts.push(endpoint); + + return parts.join('\n'); +}