diff --git a/clients/web/.npmignore b/clients/web/.npmignore index 133cadc2..ca99459a 100644 --- a/clients/web/.npmignore +++ b/clients/web/.npmignore @@ -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 +# `/../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 diff --git a/clients/web/src/test/core/react/useServers.test.tsx b/clients/web/src/test/core/react/useServers.test.tsx index c0d5260c..06eaa215 100644 --- a/clients/web/src/test/core/react/useServers.test.tsx +++ b/clients/web/src/test/core/react/useServers.test.tsx @@ -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({ + 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({ + 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({ + 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); diff --git a/scripts/smoke-web-app.mjs b/scripts/smoke-web-app.mjs index 4dafa18f..b7f3a3b4 100644 --- a/scripts/smoke-web-app.mjs +++ b/scripts/smoke-web-app.mjs @@ -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();