modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

chore: three small cleanups from the v2.2.0 merge review

Browse source 
1. clients/web/.npmignore's explanatory comment still said the root "files"
   allowlist restricts publishing to build/ and dist/ and that everything else
   under clients/web "stays out regardless". #1934 added clients/web/static to
   that allowlist, so the claim was false — in a file that exists precisely
   because a packaging contract was misread once. Name static/ and say why it
   ships (read from disk at runtime by sandbox-controller.ts, at a path
   relative to the runner dir).

2. Replace the four unjustified `as unknown as Response` doubles in
   useServers.test.tsx with real Responses over real ReadableStreams: a
   null-body Response for the `!res.body` guard, a stream whose pull throws for
   the reader-rejects case, and pull-driven streams for the multi-frame and
   priming-comment cases (pull runs once per read, so the counting and the
   blocking second read behave exactly as the hand-rolled reader doubles did).
   The doubles are now structurally type-checked against the Response API.

3. scripts/smoke-web-app.mjs orphaned the MCP test server on a readiness
   timeout: startMcpServer() returned the child only once the announcement
   line matched, so a child that was alive but never announced left `mcpServer`
   unassigned, shutdown() could not stop it, and process.exit(1) left it
   holding its port. Publish the child to `mcpServer` the moment it is spawned,
   so every throw path is covered by teardown.

Closes #2000

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-23 12:02:55 -04:00
commit 031991cc90
3 changed files with 63 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

14
clients/web/.npmignore
View file

@ -8,11 +8,15 @@
# load. (The other clients don't hit this because none of them ship a nested
# .gitignore.)
#
# Crucially this file does NOT list `build` or `dist`, so both the prod runner
# (build/) and the SPA (dist/) are packed. The root "files" allowlist already
# restricts publishing to those two directories, so everything else in
# clients/web (src, configs, node_modules, coverage, storybook-static) stays out
# regardless — the entries below are just belt-and-suspenders.
# Crucially this file does NOT list `build`, `dist`, or `static`, so all three
# are packed: the prod runner (build/), the SPA (dist/), and the MCP Apps
# sandbox proxy page (static/sandbox_proxy.html — a committed source file, read
# from disk at runtime by server/sandbox-controller.ts as
# `<runner dir>/../static/sandbox_proxy.html`, so it must ship at exactly that
# path; #1859). The root "files" allowlist names those three directories and
# nothing else under clients/web, so the rest (src, configs, node_modules,
# coverage, storybook-static) stays out regardless — the entries below are just
# belt-and-suspenders.
node_modules
coverage
storybook-static

80
clients/web/src/test/core/react/useServers.test.tsx
View file

@ -875,7 +875,9 @@ describe("useServers", () => {
fetchFn: async (input, init) => {
const url = input instanceof Request ? input.url : String(input);
if (url.endsWith("/api/servers/events")) {
return { ok: true, body: null } as unknown as Response;
// A real Response constructed from `null` has a null `.body`,
// so the guard is exercised through the actual Response API.
return new Response(null, { status: 200 });
}
return h.fetchFn(url, init);
},
@ -894,12 +896,14 @@ describe("useServers", () => {
fetchFn: async (input, init) => {
const url = input instanceof Request ? input.url : String(input);
if (url.endsWith("/api/servers/events")) {
const body = {
getReader: () => ({
read: () => Promise.reject(new Error("stream broke")),
}),
};
return { ok: true, body } as unknown as Response;
// A real stream whose first pull throws — `reader.read()` then
// rejects exactly as a broken network body would, with no cast.
const body = new ReadableStream<Uint8Array>({
pull() {
throw new Error("stream broke");
},
});
return new Response(body, { status: 200 });
}
return h.fetchFn(url, init);
},
@ -928,22 +932,18 @@ describe("useServers", () => {
fetchFn: async (input, init) => {
const url = input instanceof Request ? input.url : String(input);
if (url.endsWith("/api/servers/events")) {
const body = {
getReader: () => ({
read: async () => {
reads += 1;
if (reads === 1) {
// Two frames in one chunk → one background refresh.
return {
done: false,
value: encoder.encode("event: change\n\n\n\n"),
};
}
return { done: true, value: undefined };
},
}),
};
return { ok: true, body } as unknown as Response;
const body = new ReadableStream<Uint8Array>({
pull(controller) {
reads += 1;
if (reads === 1) {
// Two frames in one chunk → one background refresh.
controller.enqueue(encoder.encode("event: change\n\n\n\n"));
return;
}
controller.close();
},
});
return new Response(body, { status: 200 });
}
return h.fetchFn(url, init);
},
@ -1138,22 +1138,24 @@ describe("useServers", () => {
const fetchFn: typeof fetch = async (input, init) => {
const url = input instanceof Request ? input.url : String(input);
if (url.endsWith("/api/servers/events")) {
const body = {
getReader: () => ({
read: async () => {
reads += 1;
// Priming comment only — no `event:` / `data:` line.
if (reads === 1) {
return { done: false, value: encoder.encode(":\n\n") };
}
// Hold the stream open so the loop can't end and let a
// teardown-time settle hide a queued refresh.
await secondRead;
return { done: true, value: undefined };
},
}),
};
return { ok: true, body } as unknown as Response;
// `pull` runs once per read, so the counting and the blocking
// second read work the same way they would on a hand-rolled reader
// double — while staying type-checked against the Response API.
const body = new ReadableStream<Uint8Array>({
async pull(controller) {
reads += 1;
// Priming comment only — no `event:` / `data:` line.
if (reads === 1) {
controller.enqueue(encoder.encode(":\n\n"));
return;
}
// Hold the stream open so the loop can't end and let a
// teardown-time settle hide a queued refresh.
await secondRead;
controller.close();
},
});
return new Response(body, { status: 200 });
}
if (url.endsWith("/api/servers")) listGets += 1;
return h.fetchFn(url, init);

15
scripts/smoke-web-app.mjs
View file

@ -169,6 +169,14 @@ function ensureTestServer() {
* this times out with an empty diagnostic. Piping both also keeps the child's
* noise out of the smoke's own output while still making it available in the
* failure message.
*
* The child is published to `mcpServer` the moment it is spawned rather than
* handed back on success, so `shutdown()` can stop it on every throw path.
* Returning it only after the announcement matched left the readiness timeout
* — a child that is alive but never announces — with no reachable handle, so
* `process.exit(1)` orphaned a live server still holding its port. The
* `spawnError` and `exited` paths never had that problem (the child is already
* gone there), but they cost nothing to cover the same way.
*/
async function startMcpServer() {
const child = spawn(
@ -176,6 +184,7 @@ async function startMcpServer() {
[composableServer, "--config", appConfig],
{ cwd: repoRoot, stdio: ["ignore", "pipe", "pipe"] },
);
mcpServer = child;
let out = "";
child.stdout.on("data", (d) => (out += d));
child.stderr.on("data", (d) => (out += d));
@ -193,7 +202,7 @@ async function startMcpServer() {
for (let attempt = 0; attempt < 120; attempt++) {
// Take the port the server actually bound, not the one we asked for.
const announced = out.match(/listening at (http:\/\/\S+)/i);
if (announced) return { child, url: announced[1] };
if (announced) return { url: announced[1] };
if (spawnError) {
throw new Error(
`could not spawn the MCP test server (${composableServer}): ${spawnError.message}`,
@ -247,7 +256,9 @@ try {
}
ensureTestServer();
({ child: mcpServer, url: mcpUrl } = await startMcpServer());
// `startMcpServer` publishes the child to `mcpServer` itself, so teardown
// reaches it even when this throws before returning.
({ url: mcpUrl } = await startMcpServer());
await server.waitForReady();
browser = await loadChromium();
const page = await browser.newPage();