docs(smoke): retract the #2132 mechanism claim; state that it is unknown
An isolated repro of the mechanism I described — a server writing three SSE frames then going silent with the connection held open, read via fetch + getReader — delivers every frame in all three engines, WebKit included. So 'the SSE stream's last message is not delivered' is not a supported description of the failure. The keep-alive experiment that appeared to confirm it was one observation, and the failure is not even stable in shape: two consecutive webkit runs failed at different stages (ready wait vs connect wait), which a deterministic buffering bug would not do. What is unchanged: the App smokes really do fail under Playwright's WebKit, which is why the engine stays out of the matrix; that reason never depended on the mechanism. The parseSSE cross-chunk defect also stands, being read off the code rather than inferred from an experiment. No behavior change. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
32cb4c63a7
commit
d493b232b3
5 changed files with 16 additions and 14 deletions
8
.github/workflows/main.yml
vendored
8
.github/workflows/main.yml
vendored
|
|
@ -149,10 +149,10 @@ jobs:
|
|||
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 #2132 — the SSE stream's last message is not
|
||||
# delivered, so an App never leaves "loading". That is NOT reproduced in
|
||||
# real Safari, so read it as a property of Playwright's WebKit build
|
||||
# rather than a browser bug. Adding it here is a one-word diff once it is
|
||||
# two App smokes fail on #2132, by a mechanism not yet identified — the
|
||||
# first diagnosis was retracted when an isolated repro did not reproduce
|
||||
# it. Not reproduced in real Safari either, so read it as a property of
|
||||
# Playwright's WebKit build rather than a browser bug. Adding it here is a one-word diff once it is
|
||||
# resolved. An engine is either green or absent; a `continue-on-error`
|
||||
# job would report coverage nobody is holding to a standard.
|
||||
browser: [firefox]
|
||||
|
|
|
|||
|
|
@ -920,7 +920,7 @@ Everything about the selection lives in **`scripts/lib/headless-browser.mjs`**
|
|||
- **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 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 — [#2132](https://github.com/modelcontextprotocol/inspector/issues/2132): the SSE stream's last message is not delivered, so an App never leaves "loading". Adding `webkit` to the matrix is a one-word diff once that is resolved; 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.
|
||||
- **WebKit is deliberately out of the CI matrix for now, and `continue-on-error` is not the answer.** The two App smokes fail there — [#2132](https://github.com/modelcontextprotocol/inspector/issues/2132), by a mechanism that is **not yet identified**: the first diagnosis was retracted when an isolated repro failed to reproduce it, and the failure stage varies between runs. Adding `webkit` to the matrix is a one-word diff once that is resolved; 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.
|
||||
- ⚠️ **A red WebKit run is not a Safari indictment, for the same reason a green one is not a Safari guarantee.** #2132 was first written up as a Safari bug on the strength of a Playwright-WebKit failure alone; a manual check in Safari did not reproduce it. The caveat below cuts both ways, and only one direction of it was applied. **Reproduce in the real browser before claiming user impact in one** — the divergence between Playwright's WebKit build and Safari is largest in exactly the layer (networking) where that bug lives.
|
||||
- ⚠️ **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.
|
||||
|
||||
|
|
|
|||
|
|
@ -481,7 +481,7 @@ SMOKE_BROWSER=firefox npm run smoke:web:engine # all three smokes, one engine
|
|||
|
||||
Unset, the engine is `chromium` — so `npm run ci` and `npm run smoke` are unchanged. CI runs Chromium inside the `build` job's `npm run smoke`, and every other gated engine in a separate `Sandbox smokes (<engine>)` matrix job — today that matrix holds **Firefox alone**; see the paragraph below for where WebKit stands. Splitting it that way, rather than running the whole `build` job once per engine, is deliberate: validate, the coverage gate, the two verify gates and Storybook are all engine-independent. An unrecognized value is an error, not a fallback: a silent fallback would report a green Chromium run under a job labelled `webkit`. A missing browser binary fails naming the engine and its `npx playwright install --with-deps <engine>`.
|
||||
|
||||
**CI green today: Chromium and Firefox.** WebKit is supported by the tooling and worth running locally, but it is **not yet in the CI matrix** — the two App smokes fail there, tracked in [#2132](https://github.com/modelcontextprotocol/inspector/issues/2132): the SSE stream's last message is not delivered, so an App never leaves "loading". **This has not been reproduced in real Safari** — an MCP App opens there normally — so on current evidence it is a property of Playwright's WebKit build (whose network stack is not Safari's) rather than a browser bug users hit. It still blocks gating the engine, and it is worth understanding before WebKit joins the matrix. An engine is either green in CI or absent from it — a `continue-on-error` job would report coverage nobody is held to.
|
||||
**CI green today: Chromium and Firefox.** WebKit is supported by the tooling and worth running locally, but it is **not yet in the CI matrix** — the two App smokes fail there, tracked in [#2132](https://github.com/modelcontextprotocol/inspector/issues/2132). **This has not been reproduced in real Safari** — an MCP App opens there normally — so on current evidence it is a property of Playwright's WebKit build rather than a browser bug users hit. The mechanism is not yet identified; see the issue, whose first diagnosis was retracted after an isolated repro failed to reproduce it. It still blocks gating the engine, and it is worth understanding before WebKit joins the matrix. An engine is either green in CI or absent from it — a `continue-on-error` job would report coverage nobody is held to.
|
||||
|
||||
**Why these smokes specifically.** Most of the web client's behavior is React and Mantine, where a second engine buys little. The MCP Apps sandbox is the exception — it 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: `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 point the iframe at a `data:` placeholder and hand the renderer a mock bridge). Storybook itself remains Chromium-only — broadening it covers a much larger and differently-shaped surface, and is a separate decision.
|
||||
|
||||
|
|
|
|||
|
|
@ -46,10 +46,11 @@
|
|||
* `SMOKE_BROWSER` picks the engine (`chromium` — the default — `firefox`, or
|
||||
* `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, where the SSE stream's last message is not
|
||||
* delivered under Playwright's WebKit. That has NOT been reproduced in real
|
||||
* Safari (an MCP App opens there fine), so treat it as a property of that
|
||||
* build, not as a browser bug, until someone shows otherwise (#2086).
|
||||
* gated yet — it fails on #2132 under Playwright's WebKit, by a mechanism that
|
||||
* is NOT yet identified (the first diagnosis was retracted when an isolated
|
||||
* repro did not reproduce it, and the failure stage varies between runs). It has
|
||||
* NOT been reproduced in real Safari, so treat it as a property of that build
|
||||
* rather than a browser bug until someone shows otherwise (#2086).
|
||||
*
|
||||
* This smoke is one of the two places the
|
||||
* MCP Apps sandbox is genuinely exercised, and the sandbox is built out of the
|
||||
|
|
|
|||
|
|
@ -27,10 +27,11 @@
|
|||
* `SMOKE_BROWSER` picks the engine (`chromium` — the default — `firefox`, or
|
||||
* `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, where the SSE stream's last message is not
|
||||
* delivered under Playwright's WebKit. That has NOT been reproduced in real
|
||||
* Safari (an MCP App opens there fine), so treat it as a property of that
|
||||
* build, not as a browser bug, until someone shows otherwise (#2086).
|
||||
* gated yet — it fails on #2132 under Playwright's WebKit, by a mechanism that
|
||||
* is NOT yet identified (the first diagnosis was retracted when an isolated
|
||||
* repro did not reproduce it, and the failure stage varies between runs). It has
|
||||
* NOT been reproduced in real Safari, so treat it as a property of that build
|
||||
* rather than a browser bug until someone shows otherwise (#2086).
|
||||
*
|
||||
* Along with `smoke:web:app` this is one
|
||||
* of the two places the MCP Apps sandbox is actually loaded, and the two nested
|
||||
|
|
|
|||
Loading…
Reference in a new issue