Skip to content

Commit 9f75d0f

Browse files
authored
Fix internal stdio-based MCP server (#85)
* allow undefined requests when using custom manifestProvider * changeset * add tests for internal stdio transport * cleanup
1 parent 5d18405 commit 9f75d0f

File tree

8 files changed

+1444
-22
lines changed

8 files changed

+1444
-22
lines changed

.changeset/heavy-towns-beam.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@storybook/mcp': patch
3+
---
4+
5+
Allow undefined request in server context when using custom manifestProvider

packages/mcp/bin.test.ts

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
/**
2+
* Integration tests for the stdio MCP server in bin.ts
3+
*
4+
* These tests spawn the bin.ts process as a child process and communicate
5+
* with it via stdin/stdout, simulating how an MCP client would interact
6+
* with the server in production.
7+
*/
8+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
9+
import { x } from 'tinyexec';
10+
import { resolve, dirname } from 'node:path';
11+
import { fileURLToPath } from 'node:url';
12+
import type { ChildProcess } from 'node:child_process';
13+
14+
/**
15+
* Helper to send a JSON-RPC request and wait for the response
16+
*/
17+
async function sendRequest(
18+
child: ChildProcess,
19+
stdoutData: string[],
20+
request: unknown,
21+
requestId: number,
22+
timeoutMs = 10_000,
23+
): Promise<unknown> {
24+
// Send request
25+
child.stdin?.write(JSON.stringify(request) + '\n');
26+
27+
// Wait for response with timeout
28+
const { promise, resolve, reject } = Promise.withResolvers<void>();
29+
const timeout = setTimeout(() => {
30+
reject(new Error(`Timeout waiting for response to request ${requestId}`));
31+
}, timeoutMs);
32+
33+
const checkResponse = () => {
34+
const allData = stdoutData.join('');
35+
if (allData.includes(`"id":${requestId}`)) {
36+
clearTimeout(timeout);
37+
resolve();
38+
} else {
39+
setTimeout(checkResponse, 50);
40+
}
41+
};
42+
checkResponse();
43+
44+
await promise;
45+
46+
// Parse and return the response
47+
const allData = stdoutData.join('');
48+
const lines = allData.split('\n').filter((line) => line.trim());
49+
const responseLine = lines.find((line) => {
50+
try {
51+
const parsed = JSON.parse(line);
52+
return parsed.id === requestId;
53+
} catch {
54+
return false;
55+
}
56+
});
57+
58+
if (!responseLine) {
59+
throw new Error(`No response found for request ${requestId}`);
60+
}
61+
62+
return JSON.parse(responseLine);
63+
}
64+
65+
describe('bin.ts stdio MCP server', () => {
66+
let child: ChildProcess;
67+
let stdoutData: string[] = [];
68+
let stderrData: string[] = [];
69+
70+
beforeAll(() => {
71+
const currentDir = dirname(fileURLToPath(import.meta.url));
72+
const binPath = resolve(currentDir, './bin.ts');
73+
const fixturePath = resolve(
74+
currentDir,
75+
'./fixtures/full-manifest.fixture.json',
76+
);
77+
78+
const proc = x('node', [binPath, '--manifestPath', fixturePath]);
79+
80+
child = proc.process as ChildProcess;
81+
82+
// Collect stdout for later assertions
83+
child.stdout?.on('data', (chunk) => {
84+
stdoutData.push(chunk.toString());
85+
});
86+
87+
// Collect stderr for debugging
88+
child.stderr?.on('data', (chunk) => {
89+
stderrData.push(chunk.toString());
90+
});
91+
92+
child.on('error', (err) => {
93+
console.error('Process error:', err);
94+
});
95+
});
96+
97+
afterAll(() => {
98+
child.kill();
99+
});
100+
101+
it('should respond to initialize request', async () => {
102+
const request = {
103+
jsonrpc: '2.0',
104+
id: 1,
105+
method: 'initialize',
106+
params: {
107+
protocolVersion: '2024-11-05',
108+
capabilities: {},
109+
clientInfo: {
110+
name: 'test-client',
111+
version: '1.0.0',
112+
},
113+
},
114+
};
115+
116+
const response = await sendRequest(child, stdoutData, request, 1);
117+
118+
expect(response).toMatchObject({
119+
jsonrpc: '2.0',
120+
id: 1,
121+
result: {
122+
protocolVersion: '2024-11-05',
123+
capabilities: {
124+
tools: {
125+
listChanged: true,
126+
},
127+
},
128+
serverInfo: {
129+
name: '@storybook/mcp',
130+
},
131+
},
132+
});
133+
}, 15000);
134+
135+
it('should list available tools', async () => {
136+
const request = {
137+
jsonrpc: '2.0',
138+
id: 2,
139+
method: 'tools/list',
140+
params: {},
141+
};
142+
143+
const response = await sendRequest(child, stdoutData, request, 2);
144+
145+
expect(response).toMatchObject({
146+
jsonrpc: '2.0',
147+
id: 2,
148+
result: {
149+
tools: expect.arrayContaining([
150+
expect.objectContaining({
151+
name: 'list-all-components',
152+
}),
153+
expect.objectContaining({
154+
name: 'get-component-documentation',
155+
}),
156+
]),
157+
},
158+
});
159+
}, 15000);
160+
161+
it('should execute list-all-components tool', async () => {
162+
const request = {
163+
jsonrpc: '2.0',
164+
id: 3,
165+
method: 'tools/call',
166+
params: {
167+
name: 'list-all-components',
168+
arguments: {},
169+
},
170+
};
171+
172+
const response = await sendRequest(child, stdoutData, request, 3);
173+
174+
expect(response).toMatchObject({
175+
jsonrpc: '2.0',
176+
id: 3,
177+
result: {
178+
content: [
179+
{
180+
type: 'text',
181+
text: expect.stringContaining('<components>'),
182+
},
183+
],
184+
},
185+
});
186+
}, 15000);
187+
});

packages/mcp/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
"devDependencies": {
4242
"@tmcp/transport-stdio": "catalog:",
4343
"react-docgen": "^8.0.2",
44-
"srvx": "^0.8.16"
44+
"srvx": "^0.8.16",
45+
"tinyexec": "^1.0.2"
4546
}
4647
}

packages/mcp/src/types.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,14 @@ export interface StorybookContext extends Record<string, unknown> {
1515
* If provided, this function will be called instead of the default fetch-based provider.
1616
* The function receives the request object and a path to the manifest file,
1717
* and should return the manifest as a string.
18-
* The default provider constructs the manifest URL from the request origin,
19-
* replacing /mcp with /manifests/components.json
18+
* The default provider requires a request object and constructs the manifest URL from the request origin,
19+
* replacing /mcp with /manifests/components.json.
20+
* Custom providers can use the request parameter to determine the manifest source, or ignore it entirely.
2021
*/
21-
manifestProvider?: (request: Request, path: string) => Promise<string>;
22+
manifestProvider?: (
23+
request: Request | undefined,
24+
path: string,
25+
) => Promise<string>;
2226
/**
2327
* Optional handler called when list-all-components tool is invoked.
2428
* Receives the context and the component manifest.

packages/mcp/src/utils/get-manifest.test.ts

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,18 @@ describe('getManifest', () => {
2020
});
2121

2222
describe('error cases', () => {
23-
it('should throw ManifestGetError when request is not provided', async () => {
23+
it('should throw ManifestGetError when request is not provided and using default provider', async () => {
2424
await expect(getManifest()).rejects.toThrow(ManifestGetError);
2525
await expect(getManifest()).rejects.toThrow(
26-
'The request is required but was not provided in the context',
26+
"You must either pass the original request forward to the server context, or set a custom manifestProvider that doesn't need the request",
2727
);
2828
});
29-
30-
it('should throw ManifestGetError when request is undefined', async () => {
29+
it('should throw ManifestGetError when request is undefined and using default provider', async () => {
3130
await expect(getManifest(undefined)).rejects.toThrow(ManifestGetError);
31+
await expect(getManifest(undefined)).rejects.toThrow(
32+
"You must either pass the original request forward to the server context, or set a custom manifestProvider that doesn't need the request",
33+
);
3234
});
33-
3435
it('should throw ManifestGetError when fetch fails with 404', async () => {
3536
global.fetch = vi.fn().mockResolvedValue({
3637
ok: false,
@@ -225,6 +226,35 @@ describe('getManifest', () => {
225226
expect(global.fetch).not.toHaveBeenCalled();
226227
});
227228

229+
it('should allow manifestProvider to work without request', async () => {
230+
const validManifest: ComponentManifestMap = {
231+
v: 1,
232+
components: {
233+
button: {
234+
id: 'button',
235+
path: 'src/components/Button.tsx',
236+
name: 'Button',
237+
description: 'A button component',
238+
},
239+
},
240+
};
241+
242+
// Custom provider that doesn't need the request
243+
const manifestProvider = vi
244+
.fn()
245+
.mockResolvedValue(JSON.stringify(validManifest));
246+
247+
const result = await getManifest(undefined, manifestProvider);
248+
249+
expect(result).toEqual(validManifest);
250+
expect(manifestProvider).toHaveBeenCalledExactlyOnceWith(
251+
undefined,
252+
'./manifests/components.json',
253+
);
254+
// fetch should not be called when manifestProvider is used
255+
expect(global.fetch).not.toHaveBeenCalled();
256+
});
257+
228258
it('should fallback to fetch when manifestProvider is not provided', async () => {
229259
const validManifest: ComponentManifestMap = {
230260
v: 1,

packages/mcp/src/utils/get-manifest.ts

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,20 +64,18 @@ export const errorToMCPContent = (error: unknown): MCPErrorResult => {
6464
/**
6565
* Gets a component manifest from a request or using a custom provider
6666
*
67-
* @param request - The HTTP request to get the manifest for
67+
* @param request - The HTTP request to get the manifest for (optional when using custom manifestProvider)
6868
* @param manifestProvider - Optional custom function to get the manifest
6969
* @returns A promise that resolves to the parsed ComponentManifestMap
7070
* @throws {ManifestGetError} If getting the manifest fails or the response is invalid
7171
*/
7272
export async function getManifest(
7373
request?: Request,
74-
manifestProvider?: (request: Request, path: string) => Promise<string>,
74+
manifestProvider?: (
75+
request: Request | undefined,
76+
path: string,
77+
) => Promise<string>,
7578
): Promise<ComponentManifestMap> {
76-
if (!request) {
77-
throw new ManifestGetError(
78-
'The request is required but was not provided in the context',
79-
);
80-
}
8179
try {
8280
// Use custom manifestProvider if provided, otherwise fallback to default
8381
const manifestString = await (manifestProvider ?? defaultManifestProvider)(
@@ -89,7 +87,9 @@ export async function getManifest(
8987
const manifest = v.parse(ComponentManifestMap, manifestData);
9088

9189
if (Object.keys(manifest.components).length === 0) {
92-
const url = getManifestUrlFromRequest(request, MANIFEST_PATH);
90+
const url = request
91+
? getManifestUrlFromRequest(request, MANIFEST_PATH)
92+
: 'Unknown manifest source';
9393
throw new ManifestGetError(`No components found in the manifest`, url);
9494
}
9595

@@ -100,9 +100,12 @@ export async function getManifest(
100100
}
101101

102102
// Wrap network errors and other unexpected errors
103+
const url = request
104+
? getManifestUrlFromRequest(request, MANIFEST_PATH)
105+
: 'Unknown manifest source';
103106
throw new ManifestGetError(
104107
`Failed to get manifest: ${error instanceof Error ? error.message : String(error)}`,
105-
getManifestUrlFromRequest(request, MANIFEST_PATH),
108+
url,
106109
error instanceof Error ? error : undefined,
107110
);
108111
}
@@ -125,9 +128,14 @@ function getManifestUrlFromRequest(request: Request, path: string): string {
125128
* replacing /mcp with the provided path
126129
*/
127130
async function defaultManifestProvider(
128-
request: Request,
131+
request: Request | undefined,
129132
path: string,
130133
): Promise<string> {
134+
if (!request) {
135+
throw new ManifestGetError(
136+
"Request is required when using the default manifest provider. You must either pass the original request forward to the server context, or set a custom manifestProvider that doesn't need the request.",
137+
);
138+
}
131139
const manifestUrl = getManifestUrlFromRequest(request, path);
132140
const response = await fetch(manifestUrl);
133141

0 commit comments

Comments
 (0)