docs(smoke): state the actual CI browser coverage, not the intended one

The three smoke headers and one AGENTS.md bullet still said CI runs all
three engines. That was written before pointing the smokes at WebKit
found #2132; the matrix was then narrowed to Firefox and these were not
brought along, so they claimed a gate that does not exist. Each now says
what CI actually enforces — Chromium in the build job's `npm run smoke`,
Firefox in the Sandbox smokes matrix — and where WebKit stands.

smoke-web-browser.mjs additionally notes that IT passes in WebKit; the
two App smokes are the ones that do not, and a reader of this file should
not infer otherwise from a bare '#2132' reference.

The fail-fast: false rationale no longer cites a WebKit-vs-Firefox
example it cannot currently produce, and says why the flag is still right
for a one-entry matrix.

Addresses Copilot review on #2133.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-25 20:52:43 -04:00
commit 822919f3ef
4 changed files with 20 additions and 6 deletions

View file

@ -908,7 +908,7 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab
- `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog <temp>` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. **`smoke:tui` is local-only: it self-skips when `process.env.CI` is set**, because the Ink TUI needs a real TTY (raw mode) that headless CI lacks — so run it (via `npm run smoke`) on your own machine before pushing. Both build `test-servers/build` on demand if it's missing.
- Storybook play-function tests (`clients/web` `test:storybook`) run in headless Chromium via `@vitest/browser-playwright` (~10s). They are part of `npm run ci` (which installs Playwright chromium first); kept out of `validate` because they need the browser binary and are slower than the unit suite.
### The web smokes run in three browser engines (#2086)
### The web smokes are engine-parameterized; CI gates Chromium and Firefox (#2086)
**`smoke:web:browser`, `smoke:web:app` and `smoke:web:elicit` support Chromium, Firefox and WebKit; `SMOKE_BROWSER` picks one, unset means `chromium`.** CI is green on Chromium and Firefox; **WebKit runs but is not in the CI matrix yet** — see the last bullet. `npm run smoke:web:engine` runs all three smokes in whichever engine is selected — that script, not the workflow YAML, is the list of engine-covered smokes, so adding a fourth covers every engine without touching CI.
@ -918,7 +918,7 @@ Everything about the selection lives in **`scripts/lib/headless-browser.mjs`**
- **No other tier can substitute, so don't propose one.** `sandbox-csp.test.ts` asserts which policy _string_ is built — environment-independent by construction, and it would pass identically on an engine that ignores `<meta>` CSP entirely. And **no Storybook story reaches the sandbox at all**: all three App stories (`AppRenderer`, `AppsScreen`, `AppElicitationHost`) point the iframe at a `data:` placeholder and hand the renderer a mock bridge, so `sandbox-csp.ts` is imported by exactly two things in the tree — its own test and `createAppBridgeFactory.ts`. Storybook stays Chromium-only; broadening it covers a much larger, differently-shaped surface and is a separate decision to be judged on its own cost.
- **An unrecognized `SMOKE_BROWSER` is an error, never a fallback.** Falling back to Chromium would report a green Chromium run under a job labelled `webkit` — coverage claimed but not run, which is worse than none.
- **A launch failure names the engine that failed** and its `npx playwright install --with-deps <engine>`. Naming `chromium` while WebKit was the missing one sends the reader to install a browser they already have.
- **CI runs Chromium in the `build` job's `npm run smoke` and the rest in a `Sandbox smokes (<engine>)` matrix job** with `fail-fast: false`, so a WebKit-only regression is not hidden behind a Firefox-only one. Chromium is deliberately _not_ in that matrix — `build`'s `npm run smoke` is exactly the local `npm run ci` path and already covers it, and matrixing `build` itself would triple validate, the coverage gate, two verify gates and Storybook to gain three browser smokes. Both `publish` jobs `needs` the matrix, so a release cannot ship past a non-Chromium failure.
- **CI runs Chromium in the `build` job's `npm run smoke` and every other gated engine in a `Sandbox smokes (<engine>)` matrix job** — today that matrix is `[firefox]` alone (see the WebKit bullet below). It sets `fail-fast: false` so that once it holds more than one engine, a failure in one cannot hide a failure in another. Chromium is deliberately _not_ in that matrix — `build`'s `npm run smoke` is exactly the local `npm run ci` path and already covers it, and matrixing `build` itself would triple validate, the coverage gate, two verify gates and Storybook to gain three browser smokes. Both `publish` jobs `needs` the matrix, so a release cannot ship past a non-Chromium failure.
- **`pack:verify` stays pinned to Chromium**, passing it explicitly rather than reading `SMOKE_BROWSER`. It is a _packaging_ check; the engine question belongs where the sandbox is under test, and pinning it also means it can't be pointed at an engine its npm script never installed.
- **WebKit is deliberately out of the CI matrix for now, and `continue-on-error` is not the answer.** The two App smokes fail there on a real, pre-existing Safari incompatibility in the web client's SSE transport — [#2132](https://github.com/modelcontextprotocol/inspector/issues/2132) carries the diagnosis and a verified fix. Adding `webkit` to the matrix is a one-word diff once that lands; until then an engine is either green in CI or absent from it, because a job allowed to fail reports coverage nobody is held to. Worth noting the matrix found this on its first outing, against a bug every Chromium-only tier had been green through — and it is a _product_ bug, not a test one.
- ⚠️ **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. Don't write, in a doc or a PR description, that a green run means Safari works.

View file

@ -44,7 +44,12 @@
* ── Which engine ────────────────────────────────────────────────────────────
*
* `SMOKE_BROWSER` picks the engine (`chromium` — the default — `firefox`, or
* `webkit`); CI runs all three (#2086). This smoke is one of the two places the
* `webkit`). CI gates **Chromium** (in the `build` job's `npm run smoke`) and
* **Firefox** (in the `Sandbox smokes` matrix job); WebKit runs here but is not
* gated yet — it fails on #2132, a Safari incompatibility in the web client's
* SSE transport that this smoke is what found (#2086).
*
* This smoke is one of the two places the
* MCP Apps sandbox is genuinely exercised, and the sandbox is built out of the
* primitives that actually diverge between engines — `srcdoc` CSP inheritance,
* nested sandboxed iframes, `Permissions-Policy`, cross-frame `postMessage`. See

View file

@ -46,8 +46,12 @@
* Launching the browser (and resolving Playwright from clients/web, which has
* its own gotcha — see `lib/headless-browser.mjs`) is delegated to that module,
* which is also where `SMOKE_BROWSER` picks the engine: `chromium` (the
* default), `firefox`, or `webkit`, all three of which CI runs (#2086). The
* engine question here is narrower than in the App smokes — this asserts a clean
* default), `firefox`, or `webkit` (#2086). CI gates Chromium (in the `build`
* job's `npm run smoke`) and Firefox (in the `Sandbox smokes` matrix job);
* WebKit runs but is not gated yet, pending #2132 — note this smoke PASSES in
* WebKit, it is the two App smokes that do not.
*
* The engine question here is narrower than in the App smokes — this asserts a clean
* first paint, i.e. that the shipped bundle's syntax and API level are
* *reachable* on the engine at all, rather than anything about the sandbox — but
* it is real, and it is nearly free to include.

View file

@ -25,7 +25,12 @@
* proof to a PR); unset, it asserts only.
*
* `SMOKE_BROWSER` picks the engine (`chromium` — the default — `firefox`, or
* `webkit`); CI runs all three (#2086). Along with `smoke:web:app` this is one
* `webkit`). CI gates **Chromium** (in the `build` job's `npm run smoke`) and
* **Firefox** (in the `Sandbox smokes` matrix job); WebKit runs here but is not
* gated yet — it fails on #2132, a Safari incompatibility in the web client's
* SSE transport that this smoke is what found (#2086).
*
* Along with `smoke:web:app` this is one
* of the two places the MCP Apps sandbox is actually loaded, and the two nested
* frames below are precisely the surface that diverges between engines. See
* `lib/headless-browser.mjs`, including why a green WebKit run is not a Safari