test(smoke): run the headless web smokes in Firefox and WebKit, not just Chromium (#2086)

Every browser-driven check in this repo launched Chromium and nothing else.
That is fine for most of the web client, whose behavior is React and Mantine.
It is not fine for the MCP Apps sandbox, which is built out of the primitives
that genuinely diverge between engines: a CSP <meta> injected as the first
<head> child of a srcdoc document, a nested sandboxed iframe, a
Permissions-Policy allow attribute, and postMessage origin discipline across
those two frames. Nothing else covers that surface — sandbox-csp.test.ts
asserts which policy STRING is built (environment-independent by
construction), and no Storybook story reaches the sandbox at all, since all
three App stories use a data: placeholder iframe and a mock bridge.

The engine is now a parameter:

  SMOKE_BROWSER=webkit npm run smoke:web:app
  SMOKE_BROWSER=firefox npm run smoke:web:engine   # all three smokes

Unset means chromium, so `npm run ci` and `npm run smoke` are unchanged. An
unrecognized value is an error rather than a fallback — falling back would
report a green Chromium run under a job labelled "webkit", claiming coverage
that never ran — and a launch failure names the engine that failed and its own
`playwright install` invocation.

Selection, launching, and the fatal-vs-benign page-diagnostics split now live
in one shared scripts/lib/headless-browser.mjs. The three smokes had each
hand-rolled the diagnostics split and two had their own copy of loadChromium;
pack:verify goes through the same helper, pinned to Chromium explicitly since
it is a packaging check.

The npm scripts drop their `cd clients/web && npx playwright install chromium`
prefix for scripts/install-smoke-browser.mjs. The engine is a variable now, and
a ${VAR:-default} expansion does not expand under Windows' cmd.exe; npx is also
a .cmd shim there that a shell-free spawn cannot start, so this resolves the
Playwright CLI through resolveNodeBin — the #1939 problem solved the #1939 way.

CI gains a `Sandbox smokes (<engine>)` matrix job rather than matrixing `build`,
which also runs validate, the coverage gate, two verify gates and Storybook —
all engine-independent. Both publish jobs now need it.

WebKit is supported by the tooling but deliberately NOT in the CI matrix yet:
pointing the smokes at it immediately found #2132, a pre-existing Safari
incompatibility in the web client's SSE transport that strands the last message
on /api/mcp/events, so an MCP App never leaves "loading". That is a product bug
affecting real Safari users, with its own diagnosis and a verified fix, and it
is not a matrix change. Adding `webkit` here is a one-word diff once it lands.
An engine is either green in CI or absent from it; a continue-on-error job would
report coverage nobody is held to.

Signed-off-by: cliffhall <cliff@futurescale.com>

.github/copilot-instructions.md

@@ -105,6 +105,8 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
 - `npm run format` before committing; **`npm run ci` before pushing** (`validate` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → Storybook). `npm run validate` is the fast inner-loop check and is **not** a substitute — it runs `test`, not `test:coverage`, so it does zero coverage gating.
 - **A dependency bump must land in every install that declares it.** v2 is not a workspace — the root and each `clients/*` have their own `node_modules`, and a client's test project compiles `core/` and `test-servers/src` (which resolve from the **root**) alongside the client's own sources. Bumping a shared dependency in one manifest only puts two versions of it in one `tsc` program; for a recursive-generic surface like zod that exhausts the tsc heap (#1896). `verify:dep-lockstep` fails the build on this — it derives its candidates from what each `tsc` program actually resolves (`tsc --listFilesOnly`, keeping packages that reach one program from two installs), so a package reached only through another package's `.d.ts` counts too (#1965) — so a PR bumping a package the shared sources pull in should update the root **and every client that already lists it** — not every client unconditionally, since a package absent from an install can't skew and adding it there would be a spurious dependency.
 - **A test or smoke must not touch real user state.** The web smokes run against a throwaway catalog via the shared `scripts/lib/prod-web-server.mjs` helper, never the developer's `~/.mcp-inspector/mcp.json` (#1977); the cli/tui smokes drive a temp `--catalog`; `pack:verify`'s `--web` child sets its own `MCP_CATALOG_PATH` for the same reason (#2003 — its App deep link persists a server row). Anything that boots the web backend and then *navigates* it needs that isolation, not just the scripts named `smoke:*`. A new smoke spawning its own server, or teardown that removes a work dir without first awaiting `stopChild` (the #1801 race — `child-cleanup.mjs` exports both halves and both are required), should be flagged.
+- **The headless web smokes are engine-parameterized, not Chromium-only.** `smoke:web:browser` / `smoke:web:app` / `smoke:web:elicit` take their engine from `SMOKE_BROWSER` (chromium — the default — firefox, webkit) and CI covers Chromium plus Firefox (#2086; WebKit runs locally but is out of the matrix pending #2132, a real Safari SSE hang it found), because the MCP Apps sandbox is built out of the primitives that diverge between engines: `srcdoc` CSP inheritance, nested sandboxed iframes, `Permissions-Policy`, cross-frame `postMessage`. Nothing else covers that — `sandbox-csp.test.ts` asserts a policy *string*, and no Storybook story reaches the sandbox at all. Flag a new browser-driven script that launches Playwright itself instead of going through `scripts/lib/headless-browser.mjs`, an unrecognized-engine path that falls back to Chromium rather than failing (it would claim coverage that never ran), or a launch-failure message naming the wrong engine. `pack:verify` is pinned to Chromium on purpose — it is a packaging check.
+
 - **Build output is never a gate target.** Lint, format, and typecheck read first-party source only; everything a build writes (`clients/*/build`, `clients/web/dist`, `storybook-static`, `coverage`, `test-servers/build`, `core/**/{build,dist}`, `*.tsbuildinfo`) stays out via each scope's `globalIgnores`, `format` globs, and tsconfig `include`. Gating generated code reports defects in vendored third-party source that nobody can fix, and a rule promotion turns that warning into a `validate` failure (#2043). Flag a PR that adds a build location without ignoring it in the same change, that widens an ignore to silence a finding in first-party code, or that adds a build directory to a tsconfig `include` to make a generated `.d.ts` resolve. Note the coverage guards don't catch this — they assert source is _covered_, not that output is _excluded_.
 - **Lint has no warning tier.** Every `lint` script runs `--max-warnings 0`, so a warning fails `validate` exactly as an error does (#2085) — a `warn`-level `react-hooks/exhaustive-deps` finding otherwise let a stale-closure bug pass the pre-push gate and reach review. Flag a PR that silences a finding to make the gate pass (widening a `globalIgnores`, dropping a rule, or an inline disable with no justification comment); the fix is the defect, not the message. A rule meant to be enforced should be set to `error` rather than left at `warn` and carried by the flag.
 - **Every PR references an issue**, first body line `Closes #<ISSUE_NUMBER>`.

.github/workflows/main.yml

@@ -28,8 +28,8 @@ jobs:
       - name: Setup Node.js
         uses: actions/setup-node@v7
         with:
-          node-version: '22.x'
-          cache: 'npm'
+          node-version: "22.x"
+          cache: "npm"
 
       - name: Install dependencies (root + all clients)
         # The root postinstall (scripts/install-clients.mjs) cascades
@@ -115,6 +115,84 @@ jobs:
         working-directory: ./clients/web
         run: npm run test:storybook
 
+  # The same headless web smokes the `build` job already runs in Chromium, run
+  # again in the other engines this repo supports (#2086).
+  #
+  # Why this is a job of its own rather than a matrix over `build`: `build` also
+  # runs validate, the coverage gate, two verify gates and Storybook, none of
+  # which are engine-dependent — matrixing it would triple all of that to gain
+  # three browser smokes. Chromium is deliberately NOT in the matrix here for
+  # the same reason: `npm run smoke` inside `build` is exactly the local
+  # `npm run ci` path, and it already covers it. Supported set = the Chromium run
+  # there plus whatever the matrix below names.
+  #
+  # This is the only place the MCP Apps sandbox is exercised on a non-Chromium
+  # engine, and it earned its keep immediately: pointing the smokes at WebKit is
+  # what found #2132, a Safari-only hang in the web client's SSE transport that
+  # every Chromium-only tier had been green through.
+  #
+  # The unit tests cannot substitute — `sandbox-csp.test.ts` asserts
+  # which policy STRING is built, which passes identically on an engine that
+  # ignores `<meta>` CSP entirely — and no Storybook story reaches the sandbox at
+  # all (all three App stories use a `data:` placeholder iframe and a mock
+  # bridge). Note Playwright's WebKit is a WebKit build, not Safari: close enough
+  # to catch engine-level CSP and iframe divergence, not close enough to certify
+  # Safari.
+  browser-engine-smokes:
+    runs-on: ubuntu-latest
+    strategy:
+      # Report every engine's verdict. Failing fast would hide a WebKit-only
+      # regression behind a Firefox-only one, which is the exact distinction
+      # this job exists to draw.
+      fail-fast: false
+      matrix:
+        # `webkit` belongs here and is deliberately absent: the smokes RUN in it
+        # (`SMOKE_BROWSER=webkit` works, and smoke:web:browser passes), but the
+        # two App smokes fail on a real, pre-existing Safari incompatibility in
+        # the web client's SSE transport — #2132, which carries the diagnosis and
+        # the verified fix. Adding it here is a one-word diff once that lands.
+        # An engine is either green or absent; a `continue-on-error` job would
+        # report coverage nobody is holding to a standard.
+        browser: [firefox]
+    name: Sandbox smokes (${{ matrix.browser }})
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v7
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v7
+        with:
+          node-version: "22.x"
+          cache: "npm"
+
+      - name: Install dependencies (root + all clients)
+        run: npm install
+
+      - name: Build all clients
+        # The smokes need clients/web/dist and the cli/tui/launcher bundles.
+        # `build` rather than `validate` — the format/lint/typecheck half is
+        # engine-independent and already ran in the `build` job.
+        run: npm run build
+
+      - name: Cache Playwright browsers
+        uses: actions/cache@v6
+        with:
+          path: ~/.cache/ms-playwright
+          key: playwright-${{ matrix.browser }}-${{ runner.os }}-${{ hashFiles('clients/web/package-lock.json') }}
+
+      - name: Install Playwright ${{ matrix.browser }}
+        working-directory: ./clients/web
+        # `--with-deps` for the system libraries a bare runner lacks; WebKit is
+        # the large download here and dominates this step.
+        run: npx playwright install --with-deps ${{ matrix.browser }}
+
+      - name: Run the headless web smokes in ${{ matrix.browser }}
+        # The set of smokes lives in the `smoke:web:engine` npm script, not
+        # enumerated here, so adding one covers every engine automatically.
+        env:
+          SMOKE_BROWSER: ${{ matrix.browser }}
+        run: npm run smoke:web:engine
+
   # Publish the single `@modelcontextprotocol/inspector` package to npm on a
   # published GitHub release. v2 is not an npm workspace, so there is no
   # `publish-all` / `--workspaces` (v1) — just one `npm publish`, whose `prepack`
@@ -127,7 +205,7 @@ jobs:
     runs-on: ubuntu-latest
     if: github.event_name == 'release'
     environment: release
-    needs: build
+    needs: [build, browser-engine-smokes]
     # Serialize publishes so two releases cut in quick succession can't run
     # overlapping `npm publish`es. Never cancel an in-flight publish.
     concurrency:
@@ -145,9 +223,9 @@ jobs:
       - name: Setup Node.js
         uses: actions/setup-node@v7
         with:
-          node-version: '22.x'
-          cache: 'npm'
-          registry-url: 'https://registry.npmjs.org'
+          node-version: "22.x"
+          cache: "npm"
+          registry-url: "https://registry.npmjs.org"
 
       - name: Assert release tag matches package version
         # `npm publish` ships whatever `version` is in the root package.json,
@@ -220,7 +298,7 @@ jobs:
     runs-on: ubuntu-latest
     if: github.event_name == 'release'
     environment: release
-    needs: build
+    needs: [build, browser-engine-smokes]
     permissions:
       contents: read
       packages: write