chore: three small cleanups flagged in the v2.2.0 merge review

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
+# `<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

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<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);

scripts/smoke-web-app.mjs

@@ -169,13 +169,22 @@ 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(
     process.execPath,
     [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();