ci: add headless-browser boot smoke for the web client (#1615) (#1768)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled

* ci: add headless-browser boot smoke for the web client (#1615)

smoke:web only asserts GET / serves the SPA HTML with the injected token;
it never runs the React app, so a Node built-in reaching the browser
bundle (which crashes the app to a blank page at runtime, e.g. #1612)
slips through. Add smoke:web:browser, which boots the same prod --web
server and runs the bundle in headless Chromium (Playwright, already a
clients/web devDependency), asserting the app renders its first frame
(the "Add Servers" control) with no uncaught page errors — in particular
no `Module "node:*" has been externalized`. This catches "Node code
reached the browser bundle" as a class, not one import at a time.

Wire it into the root smoke chain (and thus npm run ci / GitHub CI), and
move the Playwright install/cache steps ahead of the smoke step so the
new smoke reuses the cache. Update AGENTS.md and README.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* ci: address review on the headless-browser boot smoke

- Don't fail on benign console.error (CDN font loads, React warnings) — the
  #1612 class arrives as an uncaught pageerror. console errors are now
  diagnostics, hard-failing only when they carry the externalization
  signature.
- Broaden the externalization regex to non-prefixed built-ins ("fs" as well
  as "node:fs") and label the signature on both the pageerror and console
  channels.
- Rename the port override to SMOKE_WEB_BROWSER_PORT so it doesn't collide
  with smoke:web's SMOKE_WEB_PORT.
- Add a settle window (networkidle + 500ms) so a late-evaluating lazy chunk
  that throws is caught, and race the render against launcher death so a
  mid-load crash reports the real cause instead of a render timeout.
- Print an actionable hint when Playwright/chromium can't launch (missing
  --with-deps system libs).
- Extract the shared spawn/readiness boilerplate into
  scripts/lib/prod-web-server.mjs so smoke-web and smoke-web-browser can't
  drift.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* ci: fix playwright resolution in the browser smoke (CI failure)

The bare `import("playwright")` resolved relative to the script's own
directory (scripts/), not the cwd — so `cd clients/web` in the npm script
never made it resolvable. It only passed locally because an ancestor
node_modules happened to carry playwright; CI has none, so it failed with
"Cannot find package 'playwright'". Resolve it explicitly from
clients/web via createRequire, independent of cwd and ancestor dirs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* ci: address second review on the prod-web-server helper

- waitForReady now records the last non-ok status and includes it in the
  timeout message, so a server that boots but persistently answers 500
  (broken dist / injection failure) reports "server did not start … (last
  response: HTTP 500)" instead of a bare timeout that points at the wrong
  thing. Keeps polling on non-ok (a warming server may answer 503).
- Add a child 'error' handler so a spawn-level failure surfaces as the
  smoke's FAILED line instead of an uncaught raw stack; readiness and the
  child-exit race both report it.
- Drop the unused isExited/exitCode/child exports from the helper API.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* ci: reframe browser smoke around uncaught pageerror; fix stop() edge

Third-review findings.

1. The `Module "…" has been externalized` string can't fire against a Vite 8
   *prod* bundle — verified end-to-end against the installed vite@8.0.0: a
   node: import reaching the browser graph is a BUILD-time warning, and the
   shipped stub is a silent `module.exports = {}`. The real #1612 manifestation
   is the first *call* into that empty stub throwing a TypeError at module init
   (confirmed: injecting a used `node:fs` import fails the smoke with
   "readFileSync is not a function"; an unused one ships {} and is harmless).
   So the signature machinery was dead against the bundle it runs on — dropped
   it. The smoke now asserts exactly what it verifies: the prod bundle boots and
   paints with no uncaught page error. Reworded the script header, the
   AGENTS.md bullet, and the README row to match. Console errors stay
   diagnostic-only.

2. The helper's child 'error' handler set `exited = true`, but Node emits
   'error' for a failed kill/send too (child still alive) — so `stop()`
   (guarded on `exited`) would skip the SIGTERM and orphan the launcher on the
   port. Track the error separately (`childError`); `bootFailed()` = exited ||
   childError, while `stop()` still SIGTERMs a live child.

Docs: fixed the stale AGENTS.md claim that `cd clients/web` makes
`import("playwright")` resolve (createRequire does, cwd-independently; the cd is
only for `npx playwright install`), and noted the shared
scripts/lib/prod-web-server.mjs helper on both smoke bullets.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* ci: also hard-fail the async half of the crash class (fourth review)

page.on("pageerror") fires for uncaught SYNCHRONOUS exceptions only. The
async twin of the #1612 shape — the same empty-stub TypeError reached
through an await/.then(), or a failed dynamic import (this app lazy-loads
chunks) — lands on the console channel as `Uncaught (in promise) …` /
`Failed to fetch dynamically imported module`, which were diagnostic-only,
so the smoke would have passed. Hard-fail console errors matching
/^Uncaught\b|Failed to fetch dynamically imported module/; every other
console error stays a printed non-fatal note. The prefixes are unambiguous
(a font/CDN miss reads "Failed to load resource: net::ERR_…"; React
warnings never start with "Uncaught"), so this can't reintroduce the flake
the earlier round fixed. Verified: an unhandled rejection at module init
now fails the smoke where it previously wouldn't.

Also prod-scope the AGENTS.md note that the "externalized" string isn't a
runtime message (under `npm run dev` Vite's stub is a Proxy that
console.warns it), and document both the sync and async failure signals.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* docs: reword smoke success line + README to two-channel wording (fifth review)

After the async hard-failure signature, the assertion spans both the
pageerror (sync) and console (unhandled rejection / failed dynamic import)
channels. The OK console line and the README smoke row still said "page
error"; match the two-channel wording already in the AGENTS.md bullet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* ci: correct the playwright-package-resolution hint (sixth review)

The catch around requireFromWeb("playwright") advised `playwright install
--with-deps chromium`, but that installs browser binaries, not the npm
package — and the npm script already ran `playwright install` a step
earlier. A failure there means the package isn't resolvable from
clients/web, so the actionable fix is `npm install` at the repo root (the
postinstall cascade installs clients/web's devDependencies). The
chromium.launch() catch keeps its --with-deps advice (correct for the
missing-system-libraries case).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall 2026-07-24 19:11:46 -04:00 • committed by GitHub
commit 742e82515a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 375 additions and 66 deletions

View file

@ -0,0 +1,128 @@
/**
* Shared boot/readiness helper for the prod web smokes.
*
* Both `scripts/smoke-web.mjs` (serves-the-HTML check) and
* `scripts/smoke-web-browser.mjs` (runs-the-bundle check, #1615) boot the *same*
* prod `mcp-inspector --web` server, so the spawn + readiness-poll boilerplate
* lives here once instead of being copy-pasted (and drifting) in each script.
*
* Repo-root paths are derived from import.meta.url, so a caller's cwd (e.g.
* `smoke:web:browser` does `cd clients/web` first so its `npx playwright
* install` finds the local bin) doesn't affect which launcher/build tree runs.
*/
import { spawn } from "node:child_process";
import { setTimeout as delay } from "node:timers/promises";
import { fileURLToPath } from "node:url";
import { dirname, resolve } from "node:path";
const libDir = dirname(fileURLToPath(import.meta.url));
const repoRoot = resolve(libDir, "..", "..");
const launcherEntry = resolve(repoRoot, "clients/launcher/build/index.js");
/**
* Spawn `mcp-inspector --web` (prod, no `--dev`) against the built
* `clients/web/dist` and return handles for readiness + teardown.
*
* @param {object} opts
* @param {string} opts.host
* @param {string} opts.port
* @param {string} opts.token value injected as MCP_INSPECTOR_API_TOKEN
*/
export function startProdWebServer({ host, port, token }) {
const baseUrl = `http://${host}:${port}`;
const child = spawn(process.execPath, [launcherEntry, "--web"], {
env: {
...process.env,
CLIENT_PORT: port,
HOST: host,
MCP_INSPECTOR_API_TOKEN: token,
// Don't pop a browser in CI.
MCP_AUTO_OPEN_ENABLED: "false",
},
stdio: ["ignore", "inherit", "inherit"],
});
let exited = false;
let exitCode = null;
let childError = null;
child.on("exit", (code) => {
exited = true;
exitCode = code;
});
// A child 'error' event covers a spawn failure (e.g. a missing launcher
// entry) but ALSO a failed kill / failed send where the process is still
// alive. Without a listener Node throws it uncaught with a raw stack instead
// of the smoke's `… FAILED —` line. Record it, but deliberately do NOT set
// `exited` — otherwise `stop()` (which guards on `exited`) would skip the
// SIGTERM and orphan a still-running launcher on the port.
child.on("error", (err) => {
childError = err;
});
// Boot has failed if the process exited or emitted an error.
const bootFailed = () => exited || childError !== null;
function bootFailure(phase) {
return new Error(
childError
? `launcher process error: ${childError.message}`
: `launcher exited (code ${exitCode}) ${phase} — see output above`,
);
}
/**
* Poll `GET /` until the server answers with an ok status. Keeps polling on a
* non-ok status (a warming server may legitimately answer 503), but records
* the last one so a server that boots yet never returns ok (e.g. a broken
* `dist` answering 500) reports that status instead of a bare timeout. Rejects
* early if the launcher exits before serving. Resolves with the first ok
* Response.
*/
async function waitForReady({ attempts = 120, intervalMs = 500 } = {}) {
let lastStatus = null;
for (let attempt = 0; attempt < attempts; attempt++) {
if (bootFailed()) throw bootFailure("before serving");
try {
const res = await fetch(`${baseUrl}/`);
if (res.ok) return res;
lastStatus = res.status;
} catch {
// not listening yet
}
await delay(intervalMs);
}
throw new Error(
`server did not start within ${(attempts * intervalMs) / 1000}s${
lastStatus !== null ? ` (last response: HTTP ${lastStatus})` : ""
}`,
);
}
/**
* A promise that rejects when the launcher process dies — race it against
* page-load work so a mid-flight server death is reported as the real cause
* instead of a downstream render timeout. Never resolves; if the server stays
* up it simply stays pending until the process exits.
*/
function whenChildExits() {
return new Promise((_resolve, reject) => {
const onDeath = () => reject(bootFailure("mid-run"));
if (bootFailed()) onDeath();
else {
child.once("exit", onDeath);
child.once("error", onDeath);
}
});
}
return {
baseUrl,
waitForReady,
whenChildExits,
stop: () => {
if (!exited) child.kill("SIGTERM");
},
};
}

View file

@ -0,0 +1,211 @@
#!/usr/bin/env node
/**
* Headless-browser boot smoke for the prod web client (#1615).
*
* `smoke:web` (scripts/smoke-web.mjs) only asserts that `GET /` serves the SPA
* HTML with the injected auth token — it never executes the React app, so a
* regression that only manifests when the bundle *runs* slips through. The
* canonical example (#1612): the browser bundle transitively value-imported a
* Node-only module (`store-io` → `atomically` → `stubborn-fs` → `node:process`),
* pulling a Node built-in into the browser graph and crashing the app to a blank
* page at runtime. Unit/integration tests run in node / happy-dom, never a real
* browser bundle, so none of them caught it.
*
* The assertion: the prod bundle **boots and paints its first meaningful frame
* (the "Add Servers" control) with no uncaught page error.** That is how a
* Node-only module reaching the browser bundle actually manifests under Vite:
* the excluded module is replaced by an empty stub, and the first call into it
* (e.g. `fs.readFileSync(...)` during a transitive module's init) throws a
* `TypeError` that aborts app mount — an uncaught `pageerror` here.
*
* What this does NOT rely on: the literal `Module "…" has been externalized`
* string. Under Vite 8 that is a **build-time** warning (surfaced by
* `vite build`, i.e. `npm run build`), not a runtime message — the shipped stub
* is a silent `module.exports = {}`. So the browser never sees that string; the
* load-bearing signal is the uncaught `TypeError`. (Corollary: an externalized
* import that is never *called* ships a harmless empty object and is invisible
* to this smoke by design — an unused Node import doesn't crash the app.)
*
* Two channels carry the failure. A *synchronous* uncaught exception (the
* CASE-1 shape above — a stub call during module init) fires `pageerror`. Its
* *async* twin — the same `TypeError` reached through an `await`/`.then()`, or a
* failed dynamic import (this app lazy-loads chunks) — is NOT a `pageerror`;
* Chromium logs it on the **console** channel as `Uncaught (in promise) …` /
* `Failed to fetch dynamically imported module`. Both are hard failures.
*
* Every *other* `console.error` is NOT a hard failure: the console is where
* Chromium also reports benign things a boot smoke shouldn't fail on — a failed
* subresource load (e.g. the Google-Fonts `<link>` in index.html on a
* network-restricted box) or a React key/prop warning. Those are printed as
* diagnostics. The `Uncaught` / dynamic-import prefixes are unambiguous — a
* font/CDN miss reads `Failed to load resource: net::ERR_…` and a React warning
* never starts with `Uncaught` — so hard-failing on them can't reintroduce that
* flake.
*
* Playwright lives in clients/web's node_modules, so it's resolved with a
* `createRequire` based at clients/web/package.json rather than a bare
* `import("playwright")`. A bare ESM specifier resolves relative to *this
* script's* directory (scripts/), not the cwd — so `cd clients/web` in the npm
* script would NOT make it resolvable (it only appeared to work locally when an
* ancestor node_modules happened to carry playwright; it fails in CI, which has
* none). createRequire pins resolution to clients/web regardless of cwd.
*
* Expects `clients/web/dist` and `clients/launcher/build` to be built first —
* the validate / CI ordering guarantees this.
*/
import { createRequire } from "node:module";
import { setTimeout as delay } from "node:timers/promises";
import { fileURLToPath } from "node:url";
import { dirname, resolve } from "node:path";
import { startProdWebServer } from "./lib/prod-web-server.mjs";
const scriptDir = dirname(fileURLToPath(import.meta.url));
// Resolve playwright from clients/web (where it's installed) no matter the cwd.
const requireFromWeb = createRequire(
resolve(scriptDir, "..", "clients/web/package.json"),
);
const HOST = "127.0.0.1";
// Distinct from smoke:web's SMOKE_WEB_PORT so overriding one doesn't make both
// back-to-back smokes bind the same port (→ EADDRINUSE on the second).
const PORT = process.env.SMOKE_WEB_BROWSER_PORT ?? "6298";
const TOKEN = "smoke-web-browser-token";
// Console messages that are the async half of the uncaught-crash class (an
// unhandled promise rejection or a failed dynamic import). Hard failures, unlike
// benign console noise. See the header comment for why this can't reintroduce
// the font/CDN flake.
const FATAL_CONSOLE = /^Uncaught\b|Failed to fetch dynamically imported module/;
const server = startProdWebServer({ host: HOST, port: PORT, token: TOKEN });
let browser = null;
async function shutdown() {
if (browser) {
try {
await browser.close();
} catch {
// best-effort
}
browser = null;
}
server.stop();
}
async function fail(message) {
console.error(`smoke:web:browser FAILED — ${message}`);
await shutdown();
process.exit(1);
}
async function loadChromium() {
let chromium;
try {
({ chromium } = requireFromWeb("playwright"));
} catch (err) {
// A failure here means the playwright *npm package* isn't resolvable from
// clients/web — fixed by installing devDependencies, NOT by `playwright
// install` (which fetches browser binaries). `npm install` at the repo root
// cascades into clients/web via postinstall.
throw new Error(
`could not resolve the Playwright package from clients/web — run \`npm install\` at the repo root (${err instanceof Error ? err.message : String(err)})`,
);
}
try {
return await chromium.launch({ headless: true });
} catch (err) {
throw new Error(
`chromium failed to launch — on a bare Linux box run \`npx playwright install --with-deps chromium\` for the system libraries (${err instanceof Error ? err.message : String(err)})`,
);
}
}
try {
await server.waitForReady();
browser = await loadChromium();
const page = await browser.newPage();
// Uncaught (synchronous) page errors are a hard failure — a Node-only module
// reaching the browser bundle surfaces here as a TypeError when its empty stub
// is called during module init.
const pageErrors = [];
// Console errors are diagnostic only EXCEPT those matching FATAL_CONSOLE (the
// async half of the same crash class) — see the header comment.
const consoleErrors = [];
page.on("pageerror", (err) =>
pageErrors.push(err instanceof Error ? err.message : String(err)),
);
page.on("console", (msg) => {
if (msg.type() === "error") consoleErrors.push(msg.text());
});
const render = async () => {
// Token is injected into index.html by the prod server, so a bare `/` load
// authenticates without a query param.
const response = await page.goto(server.baseUrl, {
waitUntil: "domcontentloaded",
timeout: 30_000,
});
if (!response || !response.ok()) {
throw new Error(
`GET / returned HTTP ${response ? response.status() : "no response"}`,
);
}
// First meaningful frame: the always-present "Add Servers" control.
await page
.getByRole("button", { name: /Add Servers/ })
.waitFor({ state: "visible", timeout: 30_000 });
// Settle window: let lazily-evaluated chunks that throw a tick after first
// paint surface before we assert a clean boot. networkidle is best-effort
// (the Google-Fonts request may never idle on a restricted network).
await page
.waitForLoadState("networkidle", { timeout: 5_000 })
.catch(() => {});
await delay(500);
};
// Race the render against launcher death so a mid-load server crash is
// reported as the real cause instead of a 30s render timeout.
try {
await Promise.race([server.whenChildExits(), render()]);
} catch (err) {
const diagnostics = [
...pageErrors,
...consoleErrors.map((m) => `console: ${m}`),
];
await fail(
`${err instanceof Error ? err.message : String(err)}${
diagnostics.length
? ` — page diagnostics: ${diagnostics.join("; ")}`
: ""
}`,
);
}
// Hard failures: any uncaught (sync) page error, plus console errors that are
// the async half of the class (unhandled rejection / failed dynamic import).
const fatalConsole = consoleErrors.filter((m) => FATAL_CONSOLE.test(m));
if (pageErrors.length > 0 || fatalConsole.length > 0) {
await fail(
`app logged uncaught error(s): ${[...pageErrors, ...fatalConsole].join("; ")}`,
);
}
// Non-fatal console errors: surface them so a real problem isn't invisible,
// without failing the smoke on benign subresource/warning noise.
const benignConsole = consoleErrors.filter((m) => !FATAL_CONSOLE.test(m));
if (benignConsole.length > 0) {
console.log(
`smoke:web:browser note — ${benignConsole.length} non-fatal console error(s): ${benignConsole.join("; ")}`,
);
}
console.log(
`smoke:web:browser OK — app booted at ${server.baseUrl}, rendered "Add Servers" with no uncaught errors (sync page error or unhandled rejection)`,
);
await shutdown();
process.exit(0);
} catch (err) {
await fail(err instanceof Error ? err.message : String(err));
}

View file

@ -9,15 +9,19 @@
* returns HTTP 200 with the injected auth-token global. It exits non-zero
* (failing CI / `npm run validate`) on any failure, then shuts the server down.
*
* This checks the served HTML only — it does not execute the bundle. The
* companion `smoke:web:browser` (scripts/smoke-web-browser.mjs, #1615) boots the
* same server and actually runs the app in headless Chromium; both share the
* spawn/readiness helper in ./lib/prod-web-server.mjs.
*
* Expects `clients/web/dist` and `clients/launcher/build` to be built first —
* the validate / CI ordering guarantees this, so the readiness wait below is
* sized for an already-built `dist`. (If `dist` happened to be missing, the
* launcher's build-on-demand path would build it on startup, which a cold
* `vite build` could push past that wait — not a scenario this script targets.)
* the validate / CI ordering guarantees this, so the readiness wait is sized for
* an already-built `dist`. (If `dist` happened to be missing, the launcher's
* build-on-demand path would build it on startup, which a cold `vite build`
* could push past that wait — not a scenario this script targets.)
*/
import { spawn } from "node:child_process";
import { setTimeout as delay } from "node:timers/promises";
import { startProdWebServer } from "./lib/prod-web-server.mjs";
const HOST = "127.0.0.1";
const PORT = process.env.SMOKE_WEB_PORT ?? "6299";
@ -25,59 +29,17 @@ const TOKEN = "smoke-web-token";
// Mirrors INSPECTOR_API_TOKEN_GLOBAL in core/mcp/remote/constants.ts; kept as a
// literal because this plain .mjs script can't import the TS source.
const TOKEN_GLOBAL = "__INSPECTOR_API_TOKEN__";
const BASE_URL = `http://${HOST}:${PORT}`;
const child = spawn(
process.execPath,
["clients/launcher/build/index.js", "--web"],
{
env: {
...process.env,
CLIENT_PORT: PORT,
HOST,
MCP_INSPECTOR_API_TOKEN: TOKEN,
// Don't pop a browser in CI.
MCP_AUTO_OPEN_ENABLED: "false",
},
stdio: ["ignore", "inherit", "inherit"],
},
);
let exited = false;
let exitCode = null;
child.on("exit", (code) => {
exited = true;
exitCode = code;
});
function shutdown() {
if (!exited) child.kill("SIGTERM");
}
const server = startProdWebServer({ host: HOST, port: PORT, token: TOKEN });
function fail(message) {
console.error(`smoke:web FAILED — ${message}`);
shutdown();
server.stop();
process.exit(1);
}
async function fetchRoot() {
for (let attempt = 0; attempt < 120; attempt++) {
if (exited) {
throw new Error(
`launcher exited (code ${exitCode}) before serving — see output above`,
);
}
try {
return await fetch(`${BASE_URL}/`);
} catch {
await delay(500);
}
}
throw new Error("server did not start within 60s");
}
try {
const res = await fetchRoot();
const res = await server.waitForReady();
if (res.status !== 200) {
fail(`GET / returned HTTP ${res.status}, expected 200`);
}
@ -91,9 +53,9 @@ try {
fail("served HTML is missing the injected auth-token value");
}
console.log(
`smoke:web OK — GET / => 200 with injected ${TOKEN_GLOBAL} at ${BASE_URL}`,
`smoke:web OK — GET / => 200 with injected ${TOKEN_GLOBAL} at ${server.baseUrl}`,
);
shutdown();
server.stop();
process.exit(0);
} catch (err) {
fail(err instanceof Error ? err.message : String(err));