docs(smoke): drop the #2132 references now that the issue is closed
#2132 was closed without the investigation going anywhere, so every 'pending #2132' pointer in the tree was about to send a reader to a closed issue whose own diagnosis had already been retracted. The docs now say what is actually known, and nothing more: the two App smokes fail under Playwright's WebKit for unidentified reasons; it does not reproduce in real Safari; an isolated repro of the suspected mechanism did not reproduce it under Playwright's WebKit either; and chasing it further was judged not worth the effort. A reader hitting it locally is told to treat it as unexplained rather than as a defect. The matrix decision is unchanged and never depended on the cause — the engine fails, so it is not gated. No behavior change. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
d493b232b3
commit
fb9e146043
2 changed files with 31 additions and 28 deletions
2
.github/copilot-instructions.md
vendored
2
.github/copilot-instructions.md
vendored
|
|
@ -105,7 +105,7 @@ 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, an undelivered-SSE-tail hang under Playwright's WebKit — not reproduced in real Safari, so don't cite it as one), 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.
|
||||
- **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 — the App smokes fail under it for unidentified reasons that do not reproduce in real Safari, and chasing it was dropped), 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.
|
||||
|
|
|
|||
14
.github/workflows/main.yml
vendored
14
.github/workflows/main.yml
vendored
|
|
@ -127,9 +127,8 @@ jobs:
|
|||
# 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, an undelivered-SSE-tail hang no Chromium-only tier could
|
||||
# see. (Note #2132 is NOT known to affect Safari — see the matrix comment.)
|
||||
# engine — which is why Firefox is gated here rather than assumed to behave
|
||||
# like Chromium.
|
||||
#
|
||||
# The unit tests cannot substitute — `sandbox-csp.test.ts` asserts
|
||||
# which policy STRING is built, which passes identically on an engine that
|
||||
|
|
@ -149,10 +148,11 @@ 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, 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
|
||||
# two App smokes fail there for reasons nobody has identified. It does
|
||||
# NOT reproduce in real Safari, and an isolated SSE repro did not
|
||||
# reproduce it under Playwright's WebKit either, so it reads as a
|
||||
# property of that build rather than a browser bug — and chasing it
|
||||
# further was judged not worth the effort. 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]
|
||||
|
|
|
|||
Loading…
Reference in a new issue