Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,23 @@ describe('sourceUrlManifestProvider', () => {
{ signal: expect.any(AbortSignal) }
);
});

it('lists a source that answers 401 as a notice pointing at its own MCP endpoint', async () => {
vi.mocked(fetch).mockResolvedValue(new Response('{}', { status: 401 }));
Comment thread
kasperpeulen marked this conversation as resolved.

const [listing] = await listSources(
createCompositionDocsSources({
sources: [source],
manifestProvider: sourceUrlManifestProvider,
}),
{ withStoryIds: false }
);

expect(listing).toEqual({
source,
notice: { kind: 'requires-own-mcp', endpoint: 'https://ds.example.com/sub/mcp' },
});
});
});

describe('resolveComponentEntry', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ async function defaultManifestProvider(
}

// For a composition assembled at boot rather than per request (the docs toolset core registers
// for the tools CLI): no request, no credentials, so a private source lands in its own error section.
// for the tools CLI): no request, no credentials, so a private source points at its own MCP.
export const sourceUrlManifestProvider: ManifestProvider = async (_request, path, source) => {
if (!source?.url) {
throw new ManifestGetError('The local source has no URL to fetch manifests from.');
Expand All @@ -92,6 +92,9 @@ export const sourceUrlManifestProvider: ManifestProvider = async (_request, path
const response = await fetch(manifestUrl, {
signal: AbortSignal.timeout(REF_MANIFEST_FETCH_TIMEOUT_MS),
});
if (response.status === 401) {
throw new RequiresOwnMcpError({ ...source, url: source.url });
}
return readManifestText(response, manifestUrl);
};

Expand Down
44 changes: 44 additions & 0 deletions code/core/src/shared/open-service/toolsets/docs/definition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest';
import { invokeToolsetMethod, type ToolsetCtx } from '../../toolset-definition.ts';
import type { DocsAccess } from './access.ts';
import { createDocsToolset } from './definition.ts';
import { RequiresOwnMcpError } from './sources.ts';

const button = {
id: 'button',
Expand Down Expand Up @@ -271,6 +272,49 @@ describe('docs.showStory in a composition', () => {
});
});

describe('a composed source that requires its own MCP', () => {
const privateSource = { id: 'private', title: 'Private', url: 'https://private.example.com' };
const composed = createDocsToolset({
sources: [
{ source: { id: 'local', title: 'Local' }, access: docsAccess },
{
source: privateSource,
access: {
list: () => Promise.reject(new RequiresOwnMcpError(privateSource)),
resolve: () => Promise.reject(new RequiresOwnMcpError(privateSource)),
},
},
],
});
const notice = `# Private
id: private

This composed Storybook is private and cannot be read through the local Storybook MCP proxy.

Use this source's own MCP endpoint instead:
https://private.example.com/mcp`;

it('answers show with the own-MCP notice', async () => {
const outcome = await composed.methods.show.handler(
{ id: 'button', storybookId: 'private' },
cliCtx
);

expect(outcome.ok).toBe(true);
expect(outcome.markdown).toBe(notice);
});

it('answers showStory with the own-MCP notice', async () => {
const outcome = await composed.methods.showStory.handler(
{ storyId: 'button--primary', storybookId: 'private' },
cliCtx
);

expect(outcome.ok).toBe(true);
expect(outcome.markdown).toBe(notice);
});
});

describe('usage reporting', () => {
async function run(methodName: 'list' | 'show' | 'showStory', input: unknown) {
return (await invokeToolsetMethod(toolset, methodName, input, mcpCtx)).telemetry;
Expand Down
50 changes: 40 additions & 10 deletions code/core/src/shared/open-service/toolsets/docs/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from './manifest-formatter/markdown.ts';
import type { AllManifests } from './manifest-formatter/manifest-types.ts';
import { listSources, type DocsSource } from './multi-source.ts';
import type { SourceListing } from './sources.ts';
import { RequiresOwnMcpError, type SourceListing } from './sources.ts';
import { estimateTokens } from '../estimate-tokens.ts';

const DOCS_TOOLSET_ID = 'docs';
Expand Down Expand Up @@ -61,6 +61,8 @@ export type DocsShowOutput = {
storybookId?: string;
/** Set when the request named no source, or one that does not exist. */
sourceError?: string;
/** Set when the named source can only be read through its own MCP endpoint. */
notice?: string;
};

export type DocsShowStoryOutput = {
Expand All @@ -70,6 +72,7 @@ export type DocsShowStoryOutput = {
entry?: ResolvedDocsEntry;
storybookId?: string;
sourceError?: string;
notice?: string;
};

/**
Expand All @@ -92,13 +95,17 @@ export function selectReportedManifests({
*/
type ShowResolution =
| { kind: 'source-error'; message: string }
| { kind: 'notice'; message: string }
| { kind: 'entry-missing' }
| { kind: 'found'; entry: ResolvedDocsEntry };

function resolveShow({ entry, sourceError }: DocsShowOutput): ShowResolution {
function resolveShow({ entry, sourceError, notice }: DocsShowOutput): ShowResolution {
if (sourceError !== undefined) {
return { kind: 'source-error', message: sourceError };
}
if (notice !== undefined) {
return { kind: 'notice', message: notice };
}
if (entry === undefined) {
return { kind: 'entry-missing' };
}
Expand All @@ -112,6 +119,7 @@ type ComponentStory = NonNullable<ComponentEntry['component']['stories']>[number
type ShowStoryResolution =
| { kind: 'input-invalid' }
| { kind: 'source-error'; message: string }
| { kind: 'notice'; message: string }
| { kind: 'component-missing' }
| { kind: 'story-missing'; component: ComponentEntry['component'] }
| { kind: 'found'; component: ComponentEntry['component']; story: ComponentStory };
Expand All @@ -132,13 +140,16 @@ function componentIdOfStoryId(storyId: string): string {
}

function resolveShowStory(data: DocsShowStoryOutput): ShowStoryResolution {
const { entry, storyId, storyName, sourceError } = data;
const { entry, storyId, storyName, sourceError, notice } = data;
if (!isShowStorySelector(data)) {
return { kind: 'input-invalid' };
}
if (sourceError !== undefined) {
return { kind: 'source-error', message: sourceError };
}
if (notice !== undefined) {
return { kind: 'notice', message: notice };
}
if (entry === undefined || entry.kind !== 'component') {
return { kind: 'component-missing' };
}
Expand All @@ -157,12 +168,14 @@ function resolveShowStory(data: DocsShowStoryOutput): ShowStoryResolution {
* the frozen `@storybook/mcp` API.
*/
export function isDocsShowError(output: DocsShowOutput): boolean {
return resolveShow(output).kind !== 'found';
const { kind } = resolveShow(output);
return kind !== 'found' && kind !== 'notice';
}

/** Whether `docs.showStory` failed: an unusable source, a missing component, or a missing story. */
export function isDocsShowStoryError(output: DocsShowStoryOutput): boolean {
return resolveShowStory(output).kind !== 'found';
const { kind } = resolveShowStory(output);
return kind !== 'found' && kind !== 'notice';
}

function describeList(ctx: ToolsetCtx): string {
Expand All @@ -188,6 +201,7 @@ function renderShow(data: DocsShowOutput, ctx: ToolsetCtx): string {
const resolution = resolveShow(data);
switch (resolution.kind) {
case 'source-error':
case 'notice':
return resolution.message;
case 'entry-missing':
return formatEntryNotFound(data.id, data.storybookId, ctx);
Expand Down Expand Up @@ -220,6 +234,7 @@ function renderShowStory(
case 'input-invalid':
return `Provide either \`storyId\`, or both \`componentId\` and \`storyName\`. Story ids are listed by the ${getToolName(ctx)(DOCS_METHOD_REFS.list)} tool with \`withStoryIds: true\` and in ${getToolName(ctx)(DOCS_METHOD_REFS.show)} output.`;
case 'source-error':
case 'notice':
return resolution.message;
case 'component-missing':
return data.storyId !== undefined
Expand Down Expand Up @@ -252,6 +267,21 @@ const storybookIdField = {
),
};

// A source that needs its own MCP is an answer to route the agent, not a failed lookup.
async function resolveFromSource(
access: DocsAccess,
id: string
): Promise<Pick<DocsShowOutput, 'entry' | 'notice'>> {
try {
return { entry: await access.resolve(id) };
} catch (error) {
if (error instanceof RequiresOwnMcpError) {
return { notice: error.message };
}
throw error;
}
}

/**
* Picks the access for a lookup, or explains which source the caller should have named.
*
Expand Down Expand Up @@ -398,7 +428,7 @@ export function createDocsToolset(options: CreateDocsToolsetOptions) {
const selected = access(storybookId, ctx);
const data: DocsShowOutput = selected.sourceError
? { id, storybookId, sourceError: selected.sourceError }
: { id, storybookId, entry: await selected.access!.resolve(id) };
: { id, storybookId, ...(await resolveFromSource(selected.access!, id)) };

const markdown = renderShow(data, ctx);

Expand Down Expand Up @@ -439,7 +469,7 @@ export function createDocsToolset(options: CreateDocsToolsetOptions) {
const selected = resolveId !== undefined ? access(storybookId, ctx) : {};
const data: DocsShowStoryOutput =
selected.access && resolveId !== undefined
? { ...request, entry: await selected.access.resolve(resolveId) }
? { ...request, ...(await resolveFromSource(selected.access, resolveId)) }
: { ...request, sourceError: selected.sourceError };

const resolution = resolveShowStory(data);
Expand All @@ -453,9 +483,9 @@ export function createDocsToolset(options: CreateDocsToolsetOptions) {
resultTokenCount: estimateTokens(markdown),
},
};
return resolution.kind === 'found'
? { ok: true, data, markdown, telemetry }
: { ok: false, data, markdown, telemetry };
return isDocsShowStoryError(data)
? { ok: false, data, markdown, telemetry }
: { ok: true, data, markdown, telemetry };
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion code/core/src/shared/open-service/toolsets/docs/sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class RequiresOwnMcpError extends Error {

constructor(source: SourceWithUrl) {
const endpoint = getSourceMcpEndpoint(source);
super(`Composed Storybook "${source.title}" requires its own MCP endpoint: ${endpoint}`);
super(formatRequiresOwnMcpNotice(source, endpoint));
this.name = 'RequiresOwnMcpError';
this.source = source;
this.endpoint = endpoint;
Expand Down
Loading