modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

Merge pull request #2167 from modelcontextprotocol/v2/chore/2146-rename-local-gate

Browse source 
chore(scripts): rename the local pre-push gate off `ci`, and guard it out of CI (#2146)
This commit is contained in:
Cliff Hall 2026-08-27 10:16:05 -04:00 • committed by GitHub
commit df53d718c1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 1279 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

6
.github/copilot-instructions.md vendored
View file

@ -11,7 +11,7 @@ The Inspector ships as one package with three clients (**Web**, **CLI**, **TUI**
- **Avoid double casts (`as unknown as T`).** They erase all type safety and usually mean the real type is being worked around. Prefer a type guard, a narrower single cast, or fixing the underlying type. If genuinely unavoidable (a documented gap in a third-party type, or bridging structurally-identical shapes TS can't relate), it **must** carry an inline comment justifying why it's safe and why nothing better exists. An unjustified `as unknown as` is not acceptable in review.
- Prefer inference, type guards, and precise annotations over assertions.
- An `_` prefix is the intentionally-unused marker (`argsIgnorePattern` / `varsIgnorePattern` / `caughtErrorsIgnorePattern`).
- **Never float a promise.** `@typescript-eslint/no-floating-promises` is `error` in all five ESLint scopes, so every promise must be awaited, `.catch(…)`-terminated, returned, or explicitly discarded with `void`. Prefer **holding and settling** it — a floated rejection surfaces in an unrelated file as an SDK-internal stack, which is how two un-held `callTool` calls failed the whole `npm run ci` chain (#1947). Reach for `void` only when the callee already owns its failures (it ends in a `catch`) or the caller genuinely cannot await (a sync effect body, an Ink key handler), and say which in a comment. In Storybook play functions, `expect(...)` from `storybook/test` is instrumented and returns a promise — it must be awaited.
- **Never float a promise.** `@typescript-eslint/no-floating-promises` is `error` in all five ESLint scopes, so every promise must be awaited, `.catch(…)`-terminated, returned, or explicitly discarded with `void`. Prefer **holding and settling** it — a floated rejection surfaces in an unrelated file as an SDK-internal stack, which is how two un-held `callTool` calls failed the whole `npm run local:gate` chain (#1947). Reach for `void` only when the callee already owns its failures (it ends in a `catch`) or the caller genuinely cannot await (a sync effect body, an Ink key handler), and say which in a comment. In Storybook play functions, `expect(...)` from `storybook/test` is instrumented and returns a promise — it must be awaited.
## React and UI (web client)
@ -102,11 +102,11 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
## Gates and PR hygiene
- `npm run format` before committing; **`npm run ci` before pushing** (`validate` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → Storybook; the Firefox step is local-only and not mirrored in GitHub CI). `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.
- `npm run format` before committing; **`npm run local:gate` before pushing** (`validate` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`; the Firefox step is local-only and not mirrored in GitHub CI). The gate is named `local:` rather than `ci` on purpose and has **no alias** (#2146) — `npm run ci` is gone, and a workflow that invokes a `local:*` script, a non-Chromium engine pass, `smoke:web:engine`, or sets `SMOKE_BROWSER` to a non-Chromium value fails `test:scripts` via `scripts/lib/workflow-gate.mjs` — as does naming either family through a workflow expression (`smoke:web:${{ matrix.browser }}`). It parses the workflow and scans only the executable paths — `run:` scalars, a custom `shell:` command template (step, and `defaults.run.shell` at workflow/job level, since Actions runs it *around* the script), and `env:` / `container.env:` / `with:` values, with YAML aliases resolved — so metadata is not a finding. Flag any diff that adds one of those to `.github/workflows/**`, or that reintroduces a `ci`/`ci:*` root script. `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. The canonical CI-vs-local table is in the root README ("Two tiers: GitHub CI and the local gate").
- **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.
- **A smoke must assert the thing survives, not that it started.** `smoke:tui` runs the Ink TUI under a pseudoterminal (`scripts/lib/pty.mjs`) and requires it to still be running `SMOKE_TUI_SURVIVE_MS` after its first frame (`scripts/lib/render-smoke.mjs`) — because it previously settled OK on first paint while the TUI was exiting 1 ~40ms later with `Raw mode is not supported`, and won that race on every machine (#2147). Flag anything that collapses the survival wait back into "resolve on the marker", and any new smoke whose only assertion is that a process printed something once. A smoke walks only its own happy path, so the branch that matters — started, then died — belongs in a `scripts/lib/*.test.mjs` driven against a stub (as with `announced-child.mjs` #2000 and `ensure-test-servers.mjs` #2111), not in the smoke.
- **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). **GitHub CI runs Chromium only; Firefox runs in the local pre-push gate** via `smoke:web:firefox` (#2086 — a CI job was trialled and removed for never disagreeing with Chromium across a dozen runs, and moved into `npm run ci` instead). They exist 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, as is **`smoke:web:tabs`** (#2148), the connected-flow smoke for the Tools / Resources / Prompts tabs: those are ordinary React and Mantine rather than the sandbox, so it is deliberately **outside `ENGINE_SMOKES`** and pins `chromium` on **both** halves of its npm script — install and launch. Flag a supposedly single-engine script that reads ambient `SMOKE_BROWSER` (an ambient value must not be able to redirect a gated tier), or one whose install and launch engines can disagree.
- **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). **GitHub CI runs Chromium only; Firefox runs in the local pre-push gate** via `smoke:web:firefox` (#2086 — a CI job was trialled and removed for never disagreeing with Chromium across a dozen runs, and moved into `npm run local:gate` instead). They exist 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, as is **`smoke:web:tabs`** (#2148), the connected-flow smoke for the Tools / Resources / Prompts tabs: those are ordinary React and Mantine rather than the sandbox, so it is deliberately **outside `ENGINE_SMOKES`** and pins `chromium` on **both** halves of its npm script — install and launch. Flag a supposedly single-engine script that reads ambient `SMOKE_BROWSER` (an ambient value must not be able to redirect a gated tier), or one whose install and launch engines can disagree.
- **A browser-driven smoke asserts `data-*` attributes, never visible copy** (#2148). Each screen exposes a readiness contract — `data-app-status` for Apps, and `data-tool-count` / `data-call-status` and their Resources / Prompts equivalents for the core tabs, tabulated in `clients/web/README.md`. Copy-based waits fail on the next rewording, and as an opaque timeout rather than a mismatch. Two things to flag: a new smoke matching on text or a heading, and an assertion that stops at an **RPC status** where the issue asked for something to *render* — `data-read-status="ok"` flips the moment `resources/read` resolves and stays true with the preview panel deleted, so the rendered-panel testid must be asserted alongside it. Each attribute name is pinned by a screen unit test so a rename fails there, not in a five-minute gate. Shared drive steps belong in `scripts/lib/` (the deep link lives in `deep-link-connect.mjs`): `parseDeepLink` *ignores* a link it cannot validate rather than reporting one, so a copied-and-drifted link fails as a silent connect timeout.
- **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_.

8
.github/workflows/main.yml vendored
View file

@ -88,6 +88,14 @@ jobs:
run: npx playwright install --with-deps chromium
- name: Run cross-client smokes
# NOTE: this workflow runs GitHub CI's tier only. The LOCAL pre-push
# gate is `npm run local:gate`, and it is a strict superset — do not add
# it, a `local:*` script, `smoke:web:firefox`/`smoke:web:webkit`,
# `smoke:web:engine`, or a non-Chromium `SMOKE_BROWSER` to any workflow.
# `npm run test:scripts` fails if you do (scripts/lib/workflow-gate.mjs,
# #2146). `npm run smoke` and `smoke:web:chromium` belong here and are
# not affected. The canonical CI-vs-local table is in the root README.
#
# End-to-end smokes through the built launcher (--help dispatch + prod
# CLI/web). Not part of any client's `validate`: it needs the
# cli/tui/launcher bundles, which `validate` above already built

39
AGENTS.md
View file

File diff suppressed because one or more lines are too long

38
README.md
View file

@ -109,7 +109,7 @@ Publishing `core/` as its own package (e.g. for third parties to build on) is de
The v2 web client is built from **presentational ("dumb") components** — they accept data and callbacks as props and contain only display logic, with no direct data fetching or client state. State comes from the `@inspector/core` hooks, wired in near the top of the tree. This keeps components isolated, testable, and documentable.
That approach is what makes **Storybook** first-class here: every screen and element component has a `*.stories.tsx` file (96+ stories) that renders it against fixture props. Storybook **play functions** double as interaction tests, run headless in CI (`npm run ci:storybook`, Chromium via Playwright).
That approach is what makes **Storybook** first-class here: every screen and element component has a `*.stories.tsx` file (96+ stories) that renders it against fixture props. Storybook **play functions** double as interaction tests, run headless in GitHub CI (`test:storybook`, Chromium via Playwright) and in the local gate (`npm run local:storybook`, which installs the browser first).
Styling follows a strict Mantine-first convention (theme variants and component props over CSS classes, `--inspector-*` CSS custom properties over raw color literals). The full rules live in [`AGENTS.md`](./AGENTS.md) under **React instructions** — read them before touching web UI. Element components live in `clients/web/src/components/elements/`; theme variants in `clients/web/src/theme/`.
@ -482,18 +482,40 @@ Individual clients: `build:web`, `build:cli`, `build:tui`, `build:launcher`. The
Each client self-validates from its own folder; the root scripts chain them. There is **no** aggregate root `test` script — use `validate` (fast) or `coverage` (the gate).
### Two tiers: GitHub CI and the local gate
**This is the canonical description of the CI-vs-local split — everything else in the repo that mentions it points here.**
| Tier | How it runs | What it covers |
| --- | --- | --- |
| **GitHub CI** (`.github/workflows/main.yml`) | Automatically, on every push | `npm install`, then `validate`, `verify:build-gate`, `verify:bundle-externals`, `smoke` (which includes `smoke:web:chromium`), `test:storybook` — plus `coverage` in a parallel job ([#2159](https://github.com/modelcontextprotocol/inspector/issues/2159)) |
| **The local gate** (`npm run local:gate`) | By hand, before you push | Every check above (the install is yours to run), **plus** the Firefox engine pass (`smoke:web:firefox`), and `smoke:tui` for real rather than self-skipped |
The local gate is a **strict superset**, not a mirror. Two of its steps have no GitHub CI counterpart, each for its own reason:
| Local-only step | Why it is local-only |
| --- | --- |
| `smoke:web:firefox` | Trialled as a CI job and removed ([#2086](https://github.com/modelcontextprotocol/inspector/issues/2086)): across a dozen runs it never once disagreed with Chromium, and `playwright install --with-deps` carries a real flake surface. Kept in front of a human about to push instead. `smoke:web:webkit` is worse still — it fails two of the three smokes for reasons nobody has identified — so it is in neither tier. See [Supported browsers](#supported-browsers). |
| `smoke:tui` | The Ink TUI needs a real TTY. It _is_ invoked in CI via `npm run smoke` and self-skips there on `process.env.CI`, so it needs no guarding — it handles itself. |
So the direction that matters holds: **passing `npm run local:gate` means CI's gates will pass.** The reverse does not.
**The gate is deliberately not named `ci`** ([#2146](https://github.com/modelcontextprotocol/inspector/issues/2146)). That name collided with `npm ci` — a built-in that clean-installs from the lockfile, does **not** run this script, and is one keystroke away — and it invited the cross-engine passes into GitHub CI by looking like the thing a workflow ought to call. There is **no back-compat alias**: `npm run ci` now fails with npm's missing-script error, which prints the available scripts.
That is the readable half, and prose rots. The enforced half is `scripts/lib/workflow-gate.mjs`, run by `npm run test:scripts` over every file under `.github/workflows/`. It fails the suite if a workflow invokes a `local:*` script, a non-Chromium engine pass (`smoke:web:firefox`, `smoke:web:webkit`), or `smoke:web:engine` (whose engine comes from the environment and so cannot be read off the workflow at all), or sets `SMOKE_BROWSER` to anything but a literal `chromium` (including setting it to *nothing*, which is present-and-empty rather than unset, and is rejected rather than defaulted). That last rule respects **where** it is written — an `env:` entry whose key is `SMOKE_BROWSER` (case-insensitively, since Windows environment variables are), or an assignment in a `run:` script (POSIX, cmd, a `>> $GITHUB_ENV` append, or PowerShell — the default shell on a Windows runner) — so a script that merely *prints* the variable's name is not read as setting it. It treats any name it cannot **read** the same way — a workflow expression (`smoke:web:${{ matrix.browser }}`) or a shell variable the script expands itself (`smoke:web:$ENGINE`, `%ENGINE%`) — since a value the workflow can vary may well be `firefox`. It deliberately does **not** forbid `npm run smoke`, `smoke:web:chromium`, or `smoke:tui` — those belong in CI and are there today. It **parses** the workflow and descends the schema's executable paths — workflow `env`/`defaults.run.shell`, job `env`/`container.env`/`with`/`defaults.run.shell`, step `run`/`shell`/`env`/`with` — a custom `shell:` counts, since Actions runs it *around* the script — with aliases resolved — rather than matching text or key names, so it reads only what actually executes — so a step *name* explaining the rule is not mistaken for an invocation of it, and the workflow can still document itself.
| Script | What it does |
| ----------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `npm run validate` | Runs the three durable guards first — `verify:format-coverage` (every tracked source file is format-gated), `verify:typecheck-coverage` (every one lands in a tsconfig project), `verify:dep-lockstep` (no dependency reaching one `tsc` program from two installs skews across them) — then `test:scripts` (the guards' own parser unit tests), then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui/launcher; web typechecks via `tsc -b` inside its `build`) + `build` + fast unit tests. The quick inner-loop check. |
| `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project and covers the shared `core/` runtime (including `core/json` and `core/client`). |
| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web — the TUI one runs under a **pseudoterminal** and asserts the Ink app is still running two seconds after its first frame, not merely that it painted one, [#2147](https://github.com/modelcontextprotocol/inspector/issues/2147)), plus three headless-browser smokes: a boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests), and an **MCP Apps** smoke (`smoke:web:app`) that drives connect → open app → `data-app-status="ready"` against a composable App server, covering the sandbox proxy and UI-protocol bridge, and an **app-rendered elicitation** smoke (`smoke:web:elicit`) that drives one end to end — call the tool, answer inside the sandboxed app, see the app's `ElicitResult` reach the server — and then the same tool against a server that never advertised the capability, which must fall back to the native elicitation form. Those three take their engine from `SMOKE_BROWSER` — see [Supported browsers](#supported-browsers). A fourth, **`smoke:web:tabs`**, drives the core tabs against one server on one browser launch — Tools runs a tool and asserts its `structuredContent` section, Resources reads a resource with templates listed, Prompts fetches one ([#2148](https://github.com/modelcontextprotocol/inspector/issues/2148)) — asserting the `data-*` contract in [the web README](./clients/web/README.md#core-tab-automation-contract) rather than visible copy. It is **Chromium-only**: those tabs are ordinary React and Mantine, so unlike the sandbox they are not engine-sensitive. |
| `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run ci`. |
| `npm run verify:bundle-externals` | Guards the must-not-bundle invariant (#2067): for each tsup-bundled client it reads the **built** `build/` output and fails if any package that must stay external was inlined anyway. Candidates are the union of the client's own `external` array **and the root manifest's `dependencies`** — the latter because #2067 was a *missing* `external` entry, which a self-referential check would have passed. Detection is via esbuild's `// <path>` module banners, so it covers both shapes — a separate `<pkg>-HASH.js` chunk (what a dynamically `import()`ed CommonJS package produces) and a statically-imported package folded straight into `index.js`, which emits no chunk at all. A build with no banners fails as such rather than passing clean, so enabling `minify` cannot silently retire the check. `undici` was declared only in the root and `clients/cli` manifests, and tsup auto-externalizes only what the _nearest_ manifest declares, so the web and TUI bundles inlined 1.05MB of it — and CommonJS inlined into an ESM bundle throws `Dynamic require of "assert" is not supported` on first use, from a specifier no user-side install can satisfy. Reads the output rather than the config because those two disagreed for four releases. Part of `npm run ci`. |
| `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run local:gate` and of GitHub CI. |
| `npm run verify:bundle-externals` | Guards the must-not-bundle invariant (#2067): for each tsup-bundled client it reads the **built** `build/` output and fails if any package that must stay external was inlined anyway. Candidates are the union of the client's own `external` array **and the root manifest's `dependencies`** — the latter because #2067 was a *missing* `external` entry, which a self-referential check would have passed. Detection is via esbuild's `// <path>` module banners, so it covers both shapes — a separate `<pkg>-HASH.js` chunk (what a dynamically `import()`ed CommonJS package produces) and a statically-imported package folded straight into `index.js`, which emits no chunk at all. A build with no banners fails as such rather than passing clean, so enabling `minify` cannot silently retire the check. `undici` was declared only in the root and `clients/cli` manifests, and tsup auto-externalizes only what the _nearest_ manifest declares, so the web and TUI bundles inlined 1.05MB of it — and CommonJS inlined into an ESM bundle throws `Dynamic require of "assert" is not supported` on first use, from a specifier no user-side install can satisfy. Reads the output rather than the config because those two disagreed for four releases. Part of `npm run local:gate` and of GitHub CI. |
| `npm run verify:format-coverage` | Parses the `format:check` globs out of every `package.json` (only those reachable from `validate`), enumerates all tracked source files, and **fails** listing any not covered by a glob — the durable guard for the "every first-party source file is format-gated" invariant (#1792). Runs first in `validate`. |
| `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs`, `scripts/lib/tsc-program.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode, plus two suites over shared `scripts/lib` helpers that no smoke can check itself: `resolve-node-bin.test.mjs` — the cross-platform bin resolver (#1939), pinned against the real `bin`/`exports` shapes of the packages the scripts actually spawn — and `announced-child.test.mjs` — the spawn/readiness ownership helper (#2000), which drives real `node -e` children to prove a child that never announces is still published to the caller before the timeout throws, and so is reachable by teardown rather than orphaned. Four more do the same: `mcp-app-flow.test.mjs` covers the shared MCP Apps flow (#2003) — the deep link's two CSRF gates and `appArgs` encoding, plus `driveAppFlow`'s failure branches against a stand-in page, all of which are dead code from the happy-path smokes' point of view and would otherwise surface only as opaque timeouts; `render-smoke.test.mjs` and `pty.test.mjs` cover the TUI boot harness ([#2147](https://github.com/modelcontextprotocol/inspector/issues/2147)) — the former driving real `node -e` stubs to prove that a child which paints the marker and *then* dies is a **failure**, which the old harness reported as OK and which no fixed TUI can reproduce; the latter pinning the three mutually-incompatible `script(1)` invocations, where a wrong guess stops the pseudoterminal from being allocated at all; and `ensure-test-servers.test.mjs` pins the [#2111](https://github.com/modelcontextprotocol/inspector/issues/2111) invariant — that `test-servers/build` is rebuilt **even when it already exists** — which no smoke can assert about itself, since one driving a stale fixture reports a product failure rather than a staleness one. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. |
| `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs`, `scripts/lib/tsc-program.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode, plus two suites over shared `scripts/lib` helpers that no smoke can check itself: `resolve-node-bin.test.mjs` — the cross-platform bin resolver (#1939), pinned against the real `bin`/`exports` shapes of the packages the scripts actually spawn — and `announced-child.test.mjs` — the spawn/readiness ownership helper (#2000), which drives real `node -e` children to prove a child that never announces is still published to the caller before the timeout throws, and so is reachable by teardown rather than orphaned. Four more do the same: `mcp-app-flow.test.mjs` covers the shared MCP Apps flow (#2003) — the deep link's two CSRF gates and `appArgs` encoding, plus `driveAppFlow`'s failure branches against a stand-in page, all of which are dead code from the happy-path smokes' point of view and would otherwise surface only as opaque timeouts; `render-smoke.test.mjs` and `pty.test.mjs` cover the TUI boot harness ([#2147](https://github.com/modelcontextprotocol/inspector/issues/2147)) — the former driving real `node -e` stubs to prove that a child which paints the marker and *then* dies is a **failure**, which the old harness reported as OK and which no fixed TUI can reproduce; the latter pinning the three mutually-incompatible `script(1)` invocations, where a wrong guess stops the pseudoterminal from being allocated at all; and `ensure-test-servers.test.mjs` pins the [#2111](https://github.com/modelcontextprotocol/inspector/issues/2111) invariant — that `test-servers/build` is rebuilt **even when it already exists** — which no smoke can assert about itself, since one driving a stale fixture reports a product failure rather than a staleness one. `workflow-gate.test.mjs` is a different shape again ([#2146](https://github.com/modelcontextprotocol/inspector/issues/2146)): besides the table of parser cases it runs the parser over the repo's real `.github/workflows/**`, so a workflow that invokes the local-only gate or a non-Chromium engine pass fails here rather than in a CI run nobody expected to be red. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. |
| `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. |
| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. Runs in `validate`.
| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → Storybook. A strict superset of GitHub CI — `smoke:web:firefox` (and `smoke:tui`) run only here. |
| `npm run local:gate` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. A strict superset of GitHub CI — see [Two tiers](#two-tiers-github-ci-and-the-local-gate). Named `local:` rather than `ci` on purpose (#2146); there is no `npm run ci` alias. |
| `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). |
Per-client scripts exist too (`validate:web`, `coverage:cli`, `smoke:tui`, …), plus root `validate:core` / `format:core` for the shared `core/` package, `format:scripts` for the root `scripts/` tooling, and `format:shared` / `lint:shared` for the root "shared" surface (`test-servers/src/**`, `vitest.shared.mts`, the root `eslint.config.js`). Run `npm run format` before committing — the root `format` fixes `core/`, the root `scripts/`, the shared surface, and every client; `validate` runs the non-fixing `format:check` and fails CI on any unformatted file.
@ -513,13 +535,13 @@ SMOKE_BROWSER=webkit npm run smoke:web:app # one smoke, one engine
SMOKE_BROWSER=firefox npm run smoke:web:engine # all three smokes, one engine
```
Unset, the engine is `chromium`, so `npm run smoke` is unchanged. **`npm run ci` — the mandatory pre-push gate — additionally runs all three smokes under Firefox** via `smoke:web:firefox`; **GitHub CI does not.** An unrecognized `SMOKE_BROWSER` is an error, not a fallback: a silent fallback would report a green Chromium run for a command that asked for `webkit`. A missing browser binary fails naming the engine and a remedy that works from where you are — `npm run smoke:web:<engine>`, or `npx playwright install --with-deps <engine>` run from `clients/web`, where Playwright is pinned.
Unset, the engine is `chromium`, so `npm run smoke` is unchanged. **`npm run local:gate` — the mandatory pre-push gate — additionally runs all three smokes under Firefox** via `smoke:web:firefox`; **GitHub CI does not.** An unrecognized `SMOKE_BROWSER` is an error, not a fallback: a silent fallback would report a green Chromium run for a command that asked for `webkit`. A missing browser binary fails naming the engine and a remedy that works from where you are — `npm run smoke:web:<engine>`, or `npx playwright install --with-deps <engine>` run from `clients/web`, where Playwright is pinned.
**Firefox passes all three smokes. WebKit fails the two App smokes**, for reasons nobody has identified. Two things are known: it does **not** reproduce in real Safari (an MCP App opens there normally), and an isolated repro of the mechanism it was first blamed on did not reproduce it under Playwright's WebKit either. So it reads as a property of that particular build rather than a bug users hit, and chasing it further was judged not worth the effort — treat a WebKit failure as unexplained rather than as a defect until someone has looked.
**Why Firefox is in the pre-push gate rather than in CI.** A GitHub Actions job was trialled and was cheap — about two minutes, in parallel with the 15-minute `build` job, so no added wall-clock. It was dropped anyway, on the honest count: across a dozen runs it never once disagreed with Chromium, so it spent runner minutes on every push from every branch to re-confirm a result already in hand, and carried a real flake surface (`playwright install --with-deps` runs `apt-get update`, which fails whenever a third-party repo in the runner image breaks).
Putting it in `npm run ci` instead keeps the check where it is worth most — in front of a human about to push a change they can still reason about — and pays for it once per push rather than once per CI run. If a cross-engine regression ever reaches `v2/main` because someone skipped the gate, that is the evidence for restoring the CI job; it is a one-job diff.
Putting it in `npm run local:gate` instead keeps the check where it is worth most — in front of a human about to push a change they can still reason about — and pays for it once per push rather than once per CI run. If a cross-engine regression ever reaches `v2/main` because someone skipped the gate, that is the evidence for restoring the CI job; it is a one-job diff.
**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.
@ -543,7 +565,7 @@ The root `package.json` `"files"` allowlist is the source of truth for the tarba
### `npm run pack:verify` — publish smoke against the real tarball
The `smoke:*` scripts run against the in-repo build tree, which is **not** the published package. `npm run pack:verify` (`scripts/pack-and-verify.mjs`) closes that gap: it builds, `npm pack`s the publishable tarball (asserting no source maps ship and that the runtime-required files are present), installs the tarball into a **clean throwaway consumer** — a fresh temp directory where it runs a real `npm install <tgz>` (pulls runtime deps, runs `postinstall`), exactly as `npx @modelcontextprotocol/inspector` would — and drives the installed `mcp-inspector` bin end to end: `--help` dispatch, a real `--cli tools/list` over stdio, a prod `--web` boot that must serve `/` from the shipped `dist`, and — riding that same boot — an **MCP App rendered in headless Chromium** through the shipped sandbox proxy, connect → open app → `data-app-status="ready"` ([#2003](https://github.com/modelcontextprotocol/inspector/issues/2003)). That last step shares its flow with `smoke:web:app` via `scripts/lib/mcp-app-flow.mjs` rather than copying it, since the deep-link shape is the part that rots silently; the *client* comes from the install while the App test server stays a repo fixture. It catches "works in `--dev`, breaks under `npx …`" path/packaging failures. It requires network access (the install pulls deps), so it is a local / release check, **not** part of the fast `validate`/`ci` loop.
The `smoke:*` scripts run against the in-repo build tree, which is **not** the published package. `npm run pack:verify` (`scripts/pack-and-verify.mjs`) closes that gap: it builds, `npm pack`s the publishable tarball (asserting no source maps ship and that the runtime-required files are present), installs the tarball into a **clean throwaway consumer** — a fresh temp directory where it runs a real `npm install <tgz>` (pulls runtime deps, runs `postinstall`), exactly as `npx @modelcontextprotocol/inspector` would — and drives the installed `mcp-inspector` bin end to end: `--help` dispatch, a real `--cli tools/list` over stdio, a prod `--web` boot that must serve `/` from the shipped `dist`, and — riding that same boot — an **MCP App rendered in headless Chromium** through the shipped sandbox proxy, connect → open app → `data-app-status="ready"` ([#2003](https://github.com/modelcontextprotocol/inspector/issues/2003)). That last step shares its flow with `smoke:web:app` via `scripts/lib/mcp-app-flow.mjs` rather than copying it, since the deep-link shape is the part that rots silently; the *client* comes from the install while the App test server stays a repo fixture. It catches "works in `--dev`, breaks under `npx …`" path/packaging failures. It requires network access (the install pulls deps), so it is a local / release check, **not** part of the fast `validate` loop or of `local:gate`.
### Cutting a release

2
clients/cli/README.md
View file

@ -371,7 +371,7 @@ The CLI's `test` / `test:coverage` **build the binary first** (out-of-process
`e2e.test.ts` spawns it). `validate` is `format:check && lint && typecheck && test`
with no separate `build` step (`pretest` builds). Repo-root `validate:cli`
delegates here; the coverage gate is `npm run coverage` / `coverage:cli` (also in
`npm run ci`), matching AGENTS.md.
`npm run local:gate`), matching AGENTS.md.
Tests run the CLI **in-process** (importing `runCli()`) so `src/` is measured
under coverage, with a thin out-of-process spawn layer for the real binary. See

2
clients/web/.storybook/preview.tsx
View file

@ -76,7 +76,7 @@ const preview: Preview = {
},
},
a11y: {
// `'error'` fails the Storybook play-function tests (part of `npm run ci`)
// `'error'` fails the Storybook play-function tests (part of `npm run local:gate`)
// on any axe violation, so the zero-violation state this PR reached is
// enforced going forward rather than being a point-in-time result.
test: "error",

4
clients/web/README.md
View file

@ -2,7 +2,7 @@
The browser incarnation of the Inspector: a **Vite + React + [Mantine](https://mantine.dev)** single-page app backed by a small **Node (Hono)** server. The SPA is presentational — it renders data and fires callbacks; all MCP state comes from the shared `@inspector/core` hooks. The backend proxies MCP connections, serves the built SPA, and exposes `/api/*`.
This README covers what's specific to the web client. For the repo-wide picture (the `@inspector/core` shared package, the "dumb components" philosophy, the top-level `validate`/`coverage`/`ci` scripts, and publishing), see the [root README](../../README.md).
This README covers what's specific to the web client. For the repo-wide picture (the `@inspector/core` shared package, the "dumb components" philosophy, the top-level `validate`/`coverage`/`local:gate` scripts, and publishing), see the [root README](../../README.md).
## Two halves: `src/` (browser) and `server/` (Node)
@ -319,7 +319,7 @@ npm run build:storybook # static build
npm run test:storybook # run every story's play function in headless Chromium
```
Storybook is first-class here because the components are presentational — each renders against fixture props. **Play functions double as interaction tests** and run headless in real Chromium via `@vitest/browser-playwright` + `@storybook/addon-vitest` (the `storybook` Vitest project). They're part of `npm run ci` (which installs the Chromium binary first) but kept out of the fast `validate` loop since they need the browser.
Storybook is first-class here because the components are presentational — each renders against fixture props. **Play functions double as interaction tests** and run headless in real Chromium via `@vitest/browser-playwright` + `@storybook/addon-vitest` (the `storybook` Vitest project). GitHub CI runs them directly (`npm run test:storybook`); the local pre-push gate runs them through `npm run local:storybook`, which installs the Chromium binary first. Either way they are kept out of the fast `validate` loop, since they need the browser.
## Auth token

2
clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx
View file

@ -552,7 +552,7 @@ describe("AppRenderer", () => {
// pending here. Observed failing exactly once that way, with 0 calls.
//
// The default 1000ms then turned out not to be enough either — it failed
// twice in a row in `npm run ci` (at ~1021ms, still 0 calls) while
// twice in a row in `npm run local:gate` (at ~1021ms, still 0 calls) while
// passing every time this file runs alone, which is the signature of a
// budget that is too tight rather than of a broken observer. Raised
// rather than retried: a `waitFor` that is generous costs nothing on the

2
clients/web/src/test/integration/mcp/inspectorClient.test.ts
View file

@ -108,7 +108,7 @@ async function getTool(client: InspectorClient, name: string): Promise<Tool> {
* legitimate thing to exercise, but `disconnect()` closes the SDK client, which
* rejects every pending request with "Connection closed" — and a floating
* promise makes that an *unhandled* rejection, which vitest counts as a run
* error and fails `npm run ci` even though every test passes (#1947).
* error and fails `npm run local:gate` even though every test passes (#1947).
*
* Attach the handler at call time (not after the assertions) so there is no
* window in which the rejection can escape, then finish through

2
docs/inspector-roadmap-2026-h2.md
View file

@ -514,7 +514,7 @@ _The items whose shape we cannot yet commit to, plus the multiplier._
### Standing commitments across all phases
- **Weekly milestone cadence** and the `npm run ci` gate are unchanged.
- **Weekly milestone cadence** and the pre-push gate (`npm run local:gate`, renamed off `ci` in #2146) are unchanged.
- **Bug and triage capacity is reserved, not scheduled.** The board's Incoming queue keeps
flowing regardless of phase.
- **WG liaison**: attend Transports, Agents, Triggers, Interceptors, and Server Card sessions

2
eslint.config.js
View file

@ -30,7 +30,7 @@ const typeAware = {
// an awaited one minus four characters, and the unhandled rejection it
// produces surfaces in a different test, in a different file, as a stack
// pointing at SDK internals — which is how #1947 came to fail the whole
// `npm run ci` chain from two un-held `callTool` promises.
// `npm run local:gate` chain from two un-held `callTool` promises.
"@typescript-eslint/no-floating-promises": "error",
},
};

4
package.json
View file

@ -38,8 +38,8 @@
"build:tui": "cd clients/tui && npm run build",
"build:web": "cd clients/web && npm run build",
"build:launcher": "cd clients/launcher && npm run build",
"ci": "npm run validate && npm run coverage && npm run verify:build-gate && npm run verify:bundle-externals && npm run smoke && npm run smoke:web:firefox && npm run ci:storybook",
"ci:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook",
"local:gate": "npm run validate && npm run coverage && npm run verify:build-gate && npm run verify:bundle-externals && npm run smoke && npm run smoke:web:firefox && npm run local:storybook",
"local:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook",
"verify:build-gate": "node scripts/verify-build-gate.mjs",
"verify:bundle-externals": "node scripts/verify-bundle-externals.mjs",
"verify:typecheck-coverage": "node scripts/verify-typecheck-coverage.mjs",

2
scripts/lib/announced-child.mjs
View file

@ -3,7 +3,7 @@
*
* Extracted from `scripts/smoke-web-app.mjs` so the failure path can be tested
* (#2000). A smoke script only ever exercises its own happy path: the real MCP
* test server always announces, so nothing in `npm run ci` proved that a child
* test server always announces, so nothing in `npm run local:gate` proved that a child
* which stays alive *through* the readiness timeout is still reachable by the
* caller's teardown. That is precisely the case that used to orphan a live
* server holding its port, and it is invisible to the smoke itself.

2
scripts/lib/announced-child.test.mjs
View file

@ -4,7 +4,7 @@
* The invariant under test is the one a smoke script can never check itself:
* a child that stays alive *through* the readiness timeout must still be
* reachable by the caller's teardown. `smoke:web:app`'s happy path always gets
* the announcement, so a regression here would be silent in `npm run ci` and
* the announcement, so a regression here would be silent in `npm run local:gate` and
* would surface only as a stray process holding a port on a later run.
*
* Every case drives a real `node -e` child rather than a spy, so the assertion

506
scripts/lib/workflow-gate.mjs Normal file
View file

@ -0,0 +1,506 @@
/**
* The guard that keeps the LOCAL pre-push gate out of GitHub CI (#2146).
*
* Two tiers run the same checks and are deliberately not the same set:
*
* - **GitHub CI** (`.github/workflows/**`) runs `validate`, `coverage`, the
* two verify gates, `smoke` and the Storybook tests. `npm run smoke` covers
* `smoke:launcher`, `smoke:cli`, `smoke:tui`, `smoke:web` and
* `smoke:web:chromium` — all of which BELONG there.
* - **`npm run local:gate`**, the pre-push gate, is a strict superset: it adds
* the **Firefox** engine pass, and `smoke:tui` really runs there rather than
* self-skipping. WebKit is on demand and belongs to neither tier.
*
* The narrow thing that must never reach CI is a **non-Chromium engine pass**.
* `smoke:web:firefox` was placed in the local gate on purpose (#2086,
* #2133) after a CI job was trialled and removed for never once disagreeing
* with Chromium, and `smoke:web:webkit` fails two of the three smokes outright
* for reasons nobody has identified. Confidence in the cross-engine runs is not
* high enough to let them break CI.
*
* Renaming the gate off `ci` removed the *invitation* to add it to a workflow.
* This removes the *possibility*: it is the only part of #2146 that cannot rot,
* since everything else there is prose.
*
* WHAT IT FORBIDS, and nothing more:
*
* 1. Invoking a `local:*` script. That namespace means local-only by
* construction, so the rule covers `local:storybook` and any future
* sibling without being edited.
* 2. Invoking a non-Chromium engine pass (`smoke:web:firefox`,
* `smoke:web:webkit`). Derived from `SUPPORTED_BROWSERS`, so adding an
* engine extends the guard rather than opening a hole in it.
* 3. Invoking `smoke:web:engine`, whose engine comes from the environment and
* therefore cannot be read off the workflow at all.
* 4. Setting `SMOKE_BROWSER` to anything but a literal `chromium` — the back
* door that would redirect an otherwise innocent `npm run smoke`. This one
* is KIND-AWARE: an `env:` entry counts only when `SMOKE_BROWSER` is
* literally its key, and a `run:` script only on an assignment — in any of
* the dialects a step can be written in, POSIX and PowerShell alike. A blanket text match rejected `echo
* "SMOKE_BROWSER: firefox"`, which sets nothing (Copilot). A shell
* assignment inside an `echo` is indistinguishable without parsing the
* shell, and inside a `run:` block erring toward the finding is the safe
* direction.
* 5. Naming either family through a workflow **expression** —
* `npm run smoke:web:${{ matrix.browser }}`, `npm run local:${{ … }}`, or
* `SMOKE_BROWSER: ${{ matrix.browser }}`. This is rule 3's reason applied
* consistently: a matrix whose values the guard cannot read is a matrix
* that may well contain `firefox`, so an unreadable name is treated as a
* forbidden one rather than waved through (Copilot). Note the limit — a
* fully opaque `npm run ${{ matrix.script }}` names no family at all and
* is not detectable here; the guard covers the constructions that spell
* out a forbidden one.
*
* WHAT IT MUST NOT FORBID: `npm run smoke`, `smoke:web:chromium`, `smoke:tui`.
* Those belong in CI and are there today — `smoke:tui` self-skips under
* `process.env.CI` on its own, so it needs no guarding.
*
* ONLY EXECUTABLE POSITIONS ARE SCANNED, found by parsing the file rather than
* by matching lines: `run:` scalars (inline and block), a custom `shell:`
* command template (step-level, and `defaults.run.shell` at workflow and job
* level), and the values of an `env:`, `container.env:` or `with:` mapping. Everything else in a workflow is metadata that
* runs nothing, so scanning it produces findings that are simply false:
* `name: Explain why smoke:web:firefox stays local` executes nothing, and
* failing the suite on it would contradict the "what it forbids, and nothing
* more" contract above (Copilot). Comments fall out for the same reason — and
* the workflow's value is largely in its prose, so a guard that made the two
* tiers undocumentable in the file implementing one of them would be traded for
* the wrong thing.
*
* That is a real limit, stated rather than hidden: an execution vector outside
* those positions — a custom action that reads a script name from somewhere
* else, say — is not seen. The alternative, scanning everything, was
* measured against this repo's own workflow and produces false positives on
* ordinary step names, which is the faster way to get a guard deleted.
*/
import {
LineCounter,
isAlias,
isMap,
isScalar,
isSeq,
parseAllDocuments,
} from "yaml";
import {
BROWSER_ENV_VAR,
SUPPORTED_BROWSERS,
DEFAULT_BROWSER,
} from "./headless-browser.mjs";
/** Scripts whose `local:` prefix declares them local-only. */
export const LOCAL_SCRIPT_PREFIX = "local:";
/**
* Engine passes that must never run in CI: every supported engine except the
* default, plus the env-driven runner whose engine is unknowable statically.
*/
export const FORBIDDEN_ENGINE_SCRIPTS = [
...SUPPORTED_BROWSERS.filter((b) => b !== DEFAULT_BROWSER).map(
(b) => `smoke:web:${b}`,
),
"smoke:web:engine",
];
/** Reasons a finding can carry, kept as constants so tests pin them. */
export const VIOLATION = {
LOCAL_SCRIPT: "local-only-script",
ENGINE_SCRIPT: "non-chromium-engine-pass",
BROWSER_ENV: "smoke-browser-override",
};
// The name may be spelled out (`local:gate`) or built from an expression
// (`local:${{ matrix.task }}`) — both are `local:` scripts, and neither may be
// invoked from a workflow.
//
// A name the guard cannot read: a GitHub expression (`${{ … }}`) or a shell
// variable the script expands itself — `$ENGINE`, `${ENGINE}`, `$env:ENGINE`,
// or cmd's `%ENGINE%` (Copilot). All of them resolve to *some* engine at run
// time, and the guard has no way to know which, so all of them are read as a
// forbidden one for the same reason `smoke:web:engine` is.
//
// It keys off the leading `$`/`%` and then runs to the end of the line, rather
// than trying to find a matching `}}`. An expression body can itself contain
// braces — `${{ format('{0}', matrix.browser) }}` — so a `[^}]*\}\}` body stops
// at the first `}` and misses exactly the case worth catching (Copilot). A
// prefix cannot be defeated by nesting, and since these are display strings the
// greedy tail costs nothing.
const UNREADABLE = String.raw`[$%][^\n]*`;
const LOCAL_SCRIPT_RE = new RegExp(
String.raw`\blocal:(?:[a-z0-9][a-z0-9:-]*|${UNREADABLE})`,
"gi",
);
// Same reasoning one level down: `smoke:web:` followed by an expression could
// resolve to any engine, so it is read as a forbidden one.
const DYNAMIC_ENGINE_RE = new RegExp(
String.raw`\bsmoke:web:${UNREADABLE}`,
"gi",
);
/**
* The ways a `run:` script can set the variable, one entry per shell dialect.
*
* The YAML `env:` spelling is deliberately absent — it is recognized
* structurally, by key, so that a script merely PRINTING `SMOKE_BROWSER:
* firefox` is not read as setting it (Copilot).
*
* More than one dialect is needed because `shell:` and the runner's OS decide
* which one a step is written in, and **PowerShell is the default on a Windows
* runner** — so a POSIX-only pattern is silent on exactly the workflow least
* likely to be reviewed closely (Copilot). Every one of them may match an EMPTY
* value, because assigning nothing sets the empty string, which
* `resolveBrowserName` rejects as present-but-empty rather than treating as
* unset.
*/
/**
* The value half of an assignment: a quoted string, or a bare run of
* non-delimiter characters.
*
* `\S*` was wrong in the direction that matters least often and hurts most:
* for `echo "SMOKE_BROWSER=chromium" >> $GITHUB_ENV` it captured `chromium"`,
* which is not `chromium`, so the guard rejected a **permitted** assignment
* (Copilot). A false positive on the allowed value is worse than one on a
* forbidden one — it fails a workflow that is doing the right thing.
*
* The bare arm therefore stops at a quote or a `;` (PowerShell's separator)
* rather than running to whitespace, and the quoted arms keep a value that
* really is quoted intact for `stripQuotes` to unwrap. Empty stays matchable —
* `SMOKE_BROWSER=` sets the empty string, which is its own finding.
*/
const ASSIGNED_VALUE = String.raw`("[^"]*"|'[^']*'|[^\s"';]*)`;
const BROWSER_ASSIGNMENTS = [
// POSIX `SMOKE_BROWSER=firefox`. Also covers cmd's `set SMOKE_BROWSER=…` and
// the GitHub-native `echo "SMOKE_BROWSER=…" >> $GITHUB_ENV`, since both carry
// the same `NAME=value` substring.
// Case-insensitive for the same Windows reason as the `env:` key above.
{
re: new RegExp(String.raw`\b${BROWSER_ENV_VAR}=${ASSIGNED_VALUE}`, "gi"),
value: 1,
},
// PowerShell `$env:SMOKE_BROWSER = "firefox"`, and its `${env:NAME}` form.
{
re: new RegExp(
String.raw`\$\{?env:${BROWSER_ENV_VAR}\}?\s*=\s*${ASSIGNED_VALUE}`,
"gi",
),
value: 1,
},
// PowerShell's Env: DRIVE, the other standard way to write the environment
// there: `Set-Item Env:SMOKE_BROWSER firefox`, or with `-Path`/`-Value`
// (Copilot). The value may be positional or named.
{
re: new RegExp(
String.raw`\b(?:Set|New)-Item(?:Property)?\b[^\n]*?\bEnv:${BROWSER_ENV_VAR}\b\s*(?:-Value\s+)?${ASSIGNED_VALUE}`,
"gi",
),
value: 1,
},
// GitHub's MULTILINE environment-file syntax: `SMOKE_BROWSER<<EOF`, the value
// on following lines, then the delimiter. Reported as unreadable rather than
// parsed (Copilot): the value can arrive through any number of shapes — a
// brace group redirected to $GITHUB_ENV, a heredoc nested in another heredoc
// — and a parser that handled some of them would be a coverage claim that
// holds only for the shapes someone thought of. Nobody spells `chromium` this
// way; if they did, `SMOKE_BROWSER=chromium` is the fix.
{
re: new RegExp(String.raw`\b${BROWSER_ENV_VAR}\s*<<[^\n]*`, "gi"),
unreadable: true,
},
// .NET, reachable from PowerShell: SetEnvironmentVariable("NAME", "value").
// The value arm accepts an UNQUOTED second argument as well — `$engine`, or a
// `${{ }}` expression — because that spelling sets the environment just the
// same, and an engine the guard cannot read is forbidden for the reason
// `smoke:web:engine` is (Copilot). Bare values stop at `,` or `)` so a third
// argument (`"Process"`) is not swallowed.
{
re: new RegExp(
String.raw`SetEnvironmentVariable\s*\(\s*["']${BROWSER_ENV_VAR}["']\s*,\s*("[^"]*"|'[^']*'|[^,)]*)`,
"gi",
),
value: 1,
},
];
// A shell assignment inside a `run:` script can still be quoted; a YAML value
// arrives already unquoted from the parser, so this is a no-op there.
function stripQuotes(value) {
const trimmed = value.trim().replace(/[,;]+$/, "");
const quoted = /^(["'])(.*)\1$/.exec(trimmed);
return quoted ? quoted[2] : trimmed;
}
/**
* The executable scalars of a workflow, with the line each came from.
*
* Parsed with the `yaml` package rather than by hand. The first two rounds of
* this guard did hand-roll it, and both were wrong in the same direction — a
* regex that stopped at the first `}`, then an opener that could not see
* `run: | # explanation` or the flow mapping `env: { SMOKE_BROWSER: firefox }`
* (Copilot, twice). Each miss is a workflow that invokes a forbidden pass while
* `test:scripts` stays green, which is worse than no guard, because it reports
* a coverage that is not there. YAML has too many spellings of the same thing
* to recognize by indentation; a real parser knows all of them.
*
* `yaml` is already a root **dependency**, so this adds no install — and
* `test:scripts` staying dependency-free was always about not adding one, not
* about refusing what the repo has. This file is not in the published `files`
* allowlist, so it changes nothing about the tarball.
*
* The walk is STRUCTURAL — it descends the schema's executable paths
* (workflow `env`/`defaults.run.shell`, job `env`/`container.env`/`with`/
* `defaults.run.shell`, step `run`/`shell`/`env`/`with`) rather than
* matching pairs by key name anywhere in the tree. Key-name matching looks
* equivalent and is not: workflow input ids are user-defined, so an input
* legitimately named `env` puts its own `description` and `default` in front of
* the rules, and a doc string mentioning `SMOKE_BROWSER: firefox` fails the
* gate (Copilot). A guard that fails on documentation of itself does not
* survive contact with a contributor.
*
* ALIASES ARE RESOLVED. An alias node carries no `.value`, so reading one as a
* scalar yields the empty string — and `run: *command`, with the command
* anchored in a metadata scalar the walk never visits, would invoke anything at
* all while the guard saw nothing (Copilot). Findings are reported at the line
* where the alias is USED, not where its anchor was defined, since that is the
* line to change.
*
* A parse error THROWS rather than yielding no regions: a workflow the guard
* cannot read must not pass as a workflow with nothing in it.
*
* A multi-line `run:` block is one region carrying the whole script, reported
* at the line of its `run:` key rather than at the offending line inside it.
* The finding also prints what it matched, so that is still actionable.
*
* @param {string} text
* @param {string} [file] only for the parse-error message
* @returns {Array<{line: number, kind: string, text: string}>}
*/
export function extractExecutableRegions(text, file = "<workflow>") {
const lineCounter = new LineCounter();
const docs = parseAllDocuments(text, { lineCounter });
const regions = [];
for (const doc of docs) {
if (doc.errors.length > 0) {
throw new Error(
`workflow-gate: could not parse ${file}: ${doc.errors[0].message}`,
);
}
const lineOf = (node) => lineCounter.linePos(node.range[0]).line;
const deref = (node) => (isAlias(node) ? node.resolve(doc) : node);
/** A `run:` command — one scalar, reported where it is written or aliased. */
const addCommand = (node) => {
if (node == null) return;
const resolved = deref(node);
if (!isScalar(resolved)) return;
regions.push({
line: lineOf(node),
kind: "run",
text: String(resolved.value ?? ""),
});
};
/**
* An `env:` or `with:` mapping. Each entry is rebuilt as `NAME: value` so
* the rules read it the way they read a shell assignment — and so an entry
* with NO value (valid YAML, and an empty-string override) still reaches
* them.
*/
const addMapping = (kind, node) => {
if (node == null) return;
const map = deref(node);
if (!isMap(map)) return;
for (const entry of map.items) {
if (!isScalar(entry.key)) continue;
const value = deref(entry.value);
const text = isScalar(value) ? String(value.value ?? "") : "";
regions.push({
line: lineOf(entry.key),
kind,
name: String(entry.key.value),
value: text,
text: `${String(entry.key.value)}: ${text}`,
});
}
};
const at = (node, key) => (isMap(node) ? node.get(key, true) : undefined);
/**
* `defaults.run.shell` — a custom `shell:` is a COMMAND TEMPLATE that
* Actions runs around every script (`{0}` is the script path), so
* `shell: npm run local:gate && bash {0}` invokes the gate while the `run:`
* value beside it looks harmless (Copilot). An ordinary `bash`/`pwsh`/`cmd`
* matches none of the rules, so naming it here costs nothing.
*/
const addDefaultShell = (node) =>
addCommand(at(deref(at(deref(node), "run")), "shell"));
const root = deref(doc.contents);
if (!isMap(root)) continue;
addMapping("env", at(root, "env"));
addDefaultShell(at(root, "defaults"));
const jobs = deref(at(root, "jobs"));
if (!isMap(jobs)) continue;
for (const jobEntry of jobs.items) {
const job = deref(jobEntry.value);
if (!isMap(job)) continue;
addMapping("env", at(job, "env"));
// A job-level `with:` is the input block of a reusable-workflow call.
addMapping("with", at(job, "with"));
// `container.env` is job-wide too: every step running in the container
// sees it, so an override there redirects `npm run smoke` exactly as a
// job-level `env` would (Copilot). `container:` may also be a bare image
// string, which `addMapping` ignores.
addMapping("env", at(deref(at(job, "container")), "env"));
addDefaultShell(at(job, "defaults"));
const steps = deref(at(job, "steps"));
if (!isSeq(steps)) continue;
for (const stepNode of steps.items) {
const step = deref(stepNode);
if (!isMap(step)) continue;
addCommand(at(step, "run"));
addCommand(at(step, "shell"));
addMapping("env", at(step, "env"));
addMapping("with", at(step, "with"));
}
}
}
return regions;
}
/**
* Scan one workflow file's text for invocations that must stay local-only.
*
* Pure: takes text, returns findings. `file` is carried through only so a
* caller can report where a finding came from.
*
* @param {string} text
* @param {string} [file]
* @returns {Array<{file: string, line: number, rule: string, match: string, message: string}>}
*/
export function findWorkflowViolations(text, file = "<workflow>") {
const findings = [];
for (const region of extractExecutableRegions(text, file)) {
const { line: lineNumber, kind } = region;
// Inside a `run:` block the parser hands back the shell comments too, so
// the "a comment invokes nothing" rule has to be applied here now that it
// no longer falls out of skipping YAML comment lines (Copilot).
const line = region.text
.split("\n")
.filter((l) => !l.trimStart().startsWith("#"))
.join("\n");
for (const match of line.matchAll(LOCAL_SCRIPT_RE)) {
findings.push({
file,
line: lineNumber,
rule: VIOLATION.LOCAL_SCRIPT,
match: match[0],
message:
`\`${match[0]}\` is a local-only script — the \`${LOCAL_SCRIPT_PREFIX}\` prefix means ` +
`it runs in the pre-push gate, never in GitHub CI.`,
});
}
for (const script of FORBIDDEN_ENGINE_SCRIPTS) {
// Word-boundary on the right so `smoke:web:chromium` can never be read as
// a prefix of a forbidden name (and vice versa).
// `:` is not a regex metacharacter, so the name needs no escaping.
const re = new RegExp(`\\b${script}\\b`, "g");
for (const match of line.matchAll(re)) {
findings.push({
file,
line: lineNumber,
rule: VIOLATION.ENGINE_SCRIPT,
match: match[0],
message:
`\`${script}\` is a non-Chromium engine pass and belongs to the local gate only ` +
`(#2086). GitHub CI runs \`npm run smoke\`, which covers \`smoke:web:chromium\`.`,
});
}
}
for (const match of line.matchAll(DYNAMIC_ENGINE_RE)) {
findings.push({
file,
line: lineNumber,
rule: VIOLATION.ENGINE_SCRIPT,
match: match[0],
message:
`\`${match[0]}\` builds an engine-pass name from an expression or a shell variable, ` +
`so the guard cannot tell which engine it runs — and a value a workflow can vary may ` +
`well be \`firefox\`. Name \`smoke:web:${DEFAULT_BROWSER}\` outright, or just run ` +
`\`npm run smoke\`.`,
});
}
// The browser rule is the one that must respect WHICH position it is in.
const overrides =
kind === "env"
? // Compared case-INSENSITIVELY: Windows environment variables are, so
// `smoke_browser: firefox` on a windows-latest job sets the very
// variable Node then reads as SMOKE_BROWSER (Copilot). On a POSIX
// runner that spelling would be a different variable and harmless, so
// this is stricter there than it strictly needs to be — the right
// direction for a guard, and the alternative is a rule whose verdict
// depends on a `runs-on` that may itself be an expression.
region.name.toUpperCase() === BROWSER_ENV_VAR.toUpperCase()
? [{ value: region.value, match: region.text.trim() }]
: []
: kind === "run"
? BROWSER_ASSIGNMENTS.flatMap(({ re, value, unreadable }) =>
[...line.matchAll(re)].map((m) => ({
value: unreadable ? null : m[value],
unreadable,
match: m[0].trim(),
})),
)
: // A `with:` value is an action input, not an environment. Whatever
// it names is covered by the script rules above.
[];
for (const override of overrides) {
const value = override.unreadable ? null : stripQuotes(override.value);
if (value === DEFAULT_BROWSER) continue;
// Setting it to nothing is not the same as leaving it out: the variable
// is then present and empty, which `resolveBrowserName` rejects outright
// rather than falling back to the default.
const described =
value === null
? "a multi-line value this guard cannot read"
: value === ""
? "an empty value"
: `\`${value}\``;
findings.push({
file,
line: lineNumber,
rule: VIOLATION.BROWSER_ENV,
match: override.match,
message:
`${BROWSER_ENV_VAR} must not be set to ${described} in a workflow — only ` +
`\`${DEFAULT_BROWSER}\` runs in GitHub CI, an expression cannot be ` +
`checked statically, and an empty value is rejected rather than ` +
`defaulted. Leave it unset; the default is \`${DEFAULT_BROWSER}\`.`,
});
}
}
return findings;
}
/**
* Render findings as a single message, for an assertion failure or a CLI.
*
* @param {ReturnType<typeof findWorkflowViolations>} findings
* @returns {string}
*/
export function formatWorkflowViolations(findings) {
return findings
.map((f) => ` ${f.file}:${f.line} [${f.rule}] ${f.message}`)
.join("\n");
}

689
scripts/lib/workflow-gate.test.mjs Normal file
View file

@ -0,0 +1,689 @@
/**
* The local-gate-out-of-CI guard (#2146), in two halves.
*
* The table-driven half pins one case per rule the parser encodes — including
* the negative cases, which are the ones that matter most here: a guard that
* also rejected `npm run smoke` or `smoke:web:chromium` would be "fixed" by
* deleting it, taking the real protection with it.
*
* The second half runs the parser over the repository's ACTUAL workflow files.
* That is the assertion the issue asks for: it fails the moment a workflow
* invokes the pre-push gate, a non-Chromium engine pass, or `smoke:web:engine`,
* or points `SMOKE_BROWSER` at something else.
*/
import assert from "node:assert/strict";
import { readdirSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { describe, it } from "node:test";
import {
FORBIDDEN_ENGINE_SCRIPTS,
VIOLATION,
extractExecutableRegions,
findWorkflowViolations,
formatWorkflowViolations,
} from "./workflow-gate.mjs";
const repoRoot = join(import.meta.dirname, "..", "..");
const workflowDir = join(repoRoot, ".github", "workflows");
/**
* A minimal but SCHEMA-VALID workflow around some step YAML.
*
* The fixtures have to be real workflows now that the walk is structural
* rather than key-name based: a bare `- run: …` fragment sits in no job and no
* step, so it is correctly invisible, and a table built out of fragments would
* pass while asserting nothing.
*/
const workflow = (steps) =>
[
"on: push",
"jobs:",
" build:",
" runs-on: ubuntu-latest",
" steps:",
steps,
].join("\n");
describe("findWorkflowViolations", () => {
const cases = [
{
name: "flags the pre-push gate itself",
text: workflow(" - run: npm run local:gate"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "flags any other local: script, without being edited for it",
text: workflow(" - run: npm run local:storybook"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "flags the Firefox engine pass",
text: workflow(" - run: npm run smoke:web:firefox"),
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags the WebKit engine pass",
text: workflow(" - run: npm run smoke:web:webkit"),
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags the env-driven runner, whose engine cannot be read statically",
text: workflow(" - run: npm run smoke:web:engine"),
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
// Copilot, first review: the matrix form is exactly how a forbidden
// engine reaches CI without anyone typing its name.
name: "flags an engine pass whose name is built from an expression",
text: workflow(" - run: npm run smoke:web:${{ matrix.browser }}"),
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags a local: script whose name is built from an expression",
text: workflow(" - run: npm run local:${{ matrix.task }}"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
// Copilot, second review: an expression body can itself contain braces,
// so a brace-balanced regex stops at the first `}` and misses the one
// case worth catching. The rule keys off the `${{` prefix instead.
name: "flags an expression whose own body contains braces",
text: workflow(
" - run: npm run smoke:web:${{ format('{0}', matrix.browser) }}",
),
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags a command inside a block scalar",
text: workflow(" - run: |\n npm run local:gate"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
// Copilot, third review: a hand-rolled opener could not see either of
// these, and each is a forbidden invocation that would have passed.
name: "flags a block scalar opened with a trailing comment",
text: workflow(
" - run: | # why this step exists\n npm run local:gate",
),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
// Copilot, fourth review: an alias carries no `.value`, so reading one as
// a scalar sees an empty string — with the command anchored somewhere the
// walk never visits, this invoked the gate while the guard saw nothing.
name: "flags a command reached through a YAML alias",
text: workflow(
[
" - name: &cmd npm run local:gate",
" run: echo documenting the gate",
" - run: *cmd",
].join("\n"),
),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "flags a command passed as an action input",
text: workflow(" - with:\n args: npm run local:gate"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "flags SMOKE_BROWSER pointed at another engine",
text: workflow(
" - env:\n SMOKE_BROWSER: firefox\n run: npm run smoke",
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags a quoted SMOKE_BROWSER value",
text: workflow(
' - env:\n SMOKE_BROWSER: "webkit"\n run: npm run smoke',
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags a shell-assigned SMOKE_BROWSER",
text: workflow(" - run: SMOKE_BROWSER=firefox npm run smoke"),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags an expression, which cannot be checked statically",
text: workflow(
" - env:\n SMOKE_BROWSER: ${{ matrix.browser }}\n run: npm run smoke",
),
rules: [VIOLATION.BROWSER_ENV],
},
{
// Copilot, third review: the flow spelling of the same mapping.
name: "flags an env override written as a flow mapping",
text: workflow(
" - env: { SMOKE_BROWSER: firefox }\n run: npm run smoke",
),
rules: [VIOLATION.BROWSER_ENV],
},
{
// Present-but-empty is not unset: `resolveBrowserName` rejects it rather
// than falling back to chromium, so a workflow must not write it either.
name: "flags SMOKE_BROWSER set to nothing at all",
text: workflow(
" - env:\n SMOKE_BROWSER:\n run: npm run smoke",
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags an empty shell assignment too",
text: workflow(" - run: SMOKE_BROWSER= npm run smoke"),
rules: [VIOLATION.BROWSER_ENV],
},
{
// Copilot, seventh review: a shell resolves the name itself, so the
// engine is just as unreadable as it is behind a `${{ }}` expression.
name: "flags an engine pass named through a shell variable",
text: workflow(
" - env:\n ENGINE: firefox\n run: npm run smoke:web:$ENGINE",
),
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags the braced and cmd spellings of that too",
text: workflow(
" - run: npm run smoke:web:${ENGINE}\n - run: npm run smoke:web:%ENGINE%",
),
rules: [VIOLATION.ENGINE_SCRIPT, VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags a local: script named through a shell variable",
text: workflow(" - run: npm run local:$TASK"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
// Copilot, seventh review: Windows environment variables are
// case-insensitive, so this sets the very variable Node reads as
// SMOKE_BROWSER.
name: "flags a lower-cased env key, which Windows treats as the same var",
text: workflow(
" - env: { smoke_browser: firefox }\n run: npm run smoke",
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "allows a lower-cased key set to chromium",
text: workflow(
" - env: { smoke_browser: chromium }\n run: npm run smoke",
),
rules: [],
},
{
// Copilot, sixth review: PowerShell is the DEFAULT shell on a Windows
// runner, so a POSIX-only pattern is silent on exactly the workflow least
// likely to be read closely.
name: "flags a PowerShell assignment",
text: workflow(
' - run: |\n $env:SMOKE_BROWSER = "firefox"\n npm run smoke',
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags the PowerShell ${env:NAME} spelling too",
text: workflow(
' - run: ${env:SMOKE_BROWSER}="webkit"; npm run smoke',
),
rules: [VIOLATION.BROWSER_ENV],
},
{
// Copilot, ninth review: an unquoted second argument sets the environment
// just the same, and an engine the guard cannot read is forbidden for the
// reason `smoke:web:engine` is.
name: "flags a SetEnvironmentVariable call with a dynamic value",
text: workflow(
' - run: |\n [Environment]::SetEnvironmentVariable("SMOKE_BROWSER", $engine)\n npm run smoke',
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "allows a SetEnvironmentVariable call pinning chromium",
text: workflow(
' - run: |\n [Environment]::SetEnvironmentVariable("SMOKE_BROWSER", "chromium", "Process")\n npm run smoke',
),
rules: [],
},
{
name: "flags a .NET SetEnvironmentVariable call",
text: workflow(
' - run: |\n [Environment]::SetEnvironmentVariable("SMOKE_BROWSER", "firefox")\n npm run smoke',
),
rules: [VIOLATION.BROWSER_ENV],
},
{
// Copilot, eighth review: the bare-value pattern used to run to
// whitespace, so a QUOTED assignment captured the closing quote —
// rejecting a permitted `chromium`. A false positive on the allowed value
// fails a workflow that is doing the right thing, which is the worse
// direction to be wrong in.
name: "allows a quoted chromium appended to $GITHUB_ENV",
text: workflow(
' - run: echo "SMOKE_BROWSER=chromium" >> $GITHUB_ENV',
),
rules: [],
},
{
name: "allows a quoted chromium in a cmd-style set",
text: workflow(' - run: set "SMOKE_BROWSER=chromium"'),
rules: [],
},
{
name: "allows a single-quoted chromium in PowerShell",
text: workflow(
" - run: $env:SMOKE_BROWSER = 'chromium'; npm run smoke",
),
rules: [],
},
{
// cmd's `set NAME=value` and the GitHub-native `>> $GITHUB_ENV` append
// both carry the POSIX `NAME=value` substring, so one pattern covers all
// three — worth pinning, since that is not obvious from reading it.
name: "flags a cmd-style set",
text: workflow(" - run: set SMOKE_BROWSER=firefox && npm run smoke"),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags an append to $GITHUB_ENV",
text: workflow(
' - run: echo "SMOKE_BROWSER=firefox" >> $GITHUB_ENV',
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "allows a PowerShell assignment of chromium",
text: workflow(
' - run: |\n $env:SMOKE_BROWSER = "chromium"\n npm run smoke',
),
rules: [],
},
{
// Copilot, ninth review: a custom `shell:` is a COMMAND TEMPLATE Actions
// runs around every script, so the invocation hides beside a `run:` value
// that looks harmless.
name: "flags a command in a step's shell template",
text: [
"on: push",
"jobs:",
" build:",
" steps:",
" - shell: npm run local:gate && bash {0}",
" run: echo hello",
].join("\n"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "flags one in a job's defaults.run.shell",
text: [
"on: push",
"jobs:",
" build:",
" defaults:",
" run:",
" shell: npm run smoke:web:firefox && bash {0}",
" steps:",
" - run: echo hello",
].join("\n"),
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags one in the workflow-level defaults.run.shell",
text: [
"on: push",
"defaults:",
" run:",
" shell: npm run local:gate && bash {0}",
"jobs:",
" build:",
" steps:",
" - run: echo hello",
].join("\n"),
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "leaves an ordinary shell name alone",
text: workflow(" - shell: pwsh\n run: npm run smoke"),
rules: [],
},
{
// Copilot, eleventh review: PowerShell's Env: drive is the other standard
// way to write the environment there.
name: "flags a Set-Item write to the Env: drive",
text: workflow(
" - run: Set-Item Env:SMOKE_BROWSER firefox; npm run smoke",
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags the named -Path/-Value spelling too",
text: workflow(
' - run: New-Item -Path Env:SMOKE_BROWSER -Value "firefox"',
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "allows a Set-Item write pinning chromium",
text: workflow(
' - run: Set-Item -Path Env:SMOKE_BROWSER -Value "chromium"',
),
rules: [],
},
{
// Reading the variable is not setting it — the same distinction as
// `echo "SMOKE_BROWSER: firefox"` from round 5.
name: "does not flag a Get-Item read of the same variable",
text: workflow(" - run: Get-Item Env:SMOKE_BROWSER; npm run smoke"),
rules: [],
},
{
// Copilot, tenth review: GitHub's multiline environment-file syntax.
// Reported as unreadable rather than parsed — see the comment on the
// pattern for why parsing it would be a coverage claim that only holds
// for the shapes someone thought of.
name: "flags a multi-line $GITHUB_ENV override",
text: workflow(
[
" - run: |",
' echo "SMOKE_BROWSER<<EOF" >> $GITHUB_ENV',
' echo "firefox" >> $GITHUB_ENV',
' echo "EOF" >> $GITHUB_ENV',
].join("\n"),
),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags a workflow-level env override",
text: [
"on: push",
"env:",
" SMOKE_BROWSER: firefox",
"jobs:",
" build:",
" steps:",
" - run: npm run smoke",
].join("\n"),
rules: [VIOLATION.BROWSER_ENV],
},
{
// Copilot, fifth review: `container.env` is job-wide too — every step in
// the container sees it, so it redirects `npm run smoke` exactly as a
// job-level `env` would.
name: "flags a SMOKE_BROWSER override under container.env",
text: [
"on: push",
"jobs:",
" build:",
" container:",
" image: node:22",
" env:",
" SMOKE_BROWSER: firefox",
" steps:",
" - run: npm run smoke",
].join("\n"),
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "does not trip over a container given as a bare image string",
text: [
"on: push",
"jobs:",
" build:",
" container: node:22",
" steps:",
" - run: npm run smoke",
].join("\n"),
rules: [],
},
{
// Copilot, fifth review: a blanket text match rejected a script that
// merely PRINTS the variable name, which sets nothing.
name: "does not read a script printing the variable as setting it",
text: workflow(
' - run: |\n echo "SMOKE_BROWSER: firefox"\n npm run smoke',
),
rules: [],
},
{
name: "does not read an action input mentioning it as setting it",
text: workflow(
' - with:\n message: "SMOKE_BROWSER: firefox"',
),
rules: [],
},
{
name: "allows SMOKE_BROWSER set explicitly to chromium",
text: workflow(
" - env:\n SMOKE_BROWSER: chromium\n run: npm run smoke",
),
rules: [],
},
{
name: "allows the smoke suite, which belongs in CI",
text: workflow(" - run: npm run smoke"),
rules: [],
},
{
name: "allows the Chromium engine pass, which belongs in CI",
text: workflow(" - run: npm run smoke:web:chromium"),
rules: [],
},
{
// #2148's tab smoke pins Chromium on both halves of its npm script and is
// deliberately outside ENGINE_SMOKES — it is not an engine pass, and the
// guard must not read `smoke:web:` as forbidden by itself.
name: "allows the Chromium-only tab smoke",
text: workflow(" - run: npm run smoke:web:tabs"),
rules: [],
},
{
name: "allows smoke:tui, which self-skips under CI on its own",
text: workflow(" - run: npm run smoke:tui"),
rules: [],
},
{
name: "allows the rest of the CI chain",
text: workflow(
[
" - run: npm run validate",
" - run: npm run coverage",
" - run: npm run verify:build-gate",
" - run: npm run verify:bundle-externals",
" - run: npm run test:storybook",
].join("\n"),
),
rules: [],
},
{
// Copilot, second review: metadata executes nothing, so a finding there
// is simply false — and the fastest way to get a guard deleted.
name: "does not read a step name as an invocation",
text: workflow(
" - name: Explain why smoke:web:firefox stays local\n run: npm run smoke",
),
rules: [],
},
{
name: "does not read an `if:` condition as one either",
text: workflow(
" - if: github.event_name == 'push'\n run: npm run smoke",
),
rules: [],
},
{
// Copilot, fourth review: workflow input ids are user-defined, so an
// input legitimately named `env` would have its own docs scanned by a
// key-name match — failing the gate on documentation of the gate.
name: "does not scan a workflow input that happens to be named env",
text: [
"on:",
" workflow_dispatch:",
" inputs:",
" env:",
" description: why smoke:web:firefox stays local",
" default: npm run local:gate",
"jobs:",
" build:",
" steps:",
" - run: npm run smoke",
].join("\n"),
rules: [],
},
{
name: "does not read a similarly-named key as the override",
text: workflow(
" - env:\n SMOKE_BROWSER_DOC: firefox\n run: npm run smoke",
),
rules: [],
},
{
name: "does not read a YAML comment as an invocation",
text: workflow(
" # smoke:web:firefox runs in npm run local:gate, never here\n - run: npm run smoke",
),
rules: [],
},
{
// Copilot, third review: this fixture used to be a YAML comment, which
// the parser strips — so it asserted nothing. Inside a `run:` block the
// comment text really does survive into the scalar, which is where the
// "a comment invokes nothing" rule has to be applied.
name: "does not read a shell comment inside a run block as one either",
text: workflow(
" - run: |\n # SMOKE_BROWSER=webkit is local-only\n npm run smoke",
),
rules: [],
},
];
for (const { name, text, rules } of cases) {
it(name, () => {
const found = findWorkflowViolations(text, "case.yml");
assert.deepEqual(
found.map((f) => f.rule),
rules,
formatWorkflowViolations(found),
);
});
}
it("reports the line a finding came from", () => {
const [finding] = findWorkflowViolations(
workflow(" - run: npm run local:gate"),
"main.yml",
);
assert.equal(finding.line, 6);
assert.equal(finding.file, "main.yml");
assert.equal(finding.match, "local:gate");
});
it("names the file in a parse error, rather than a placeholder", () => {
// The filename is passed in for diagnostics; forgetting to forward it left
// a malformed real workflow reported as `<workflow>` (Copilot).
assert.throws(
() => findWorkflowViolations("jobs:\n - a\n b: c\n", "main.yml"),
/could not parse main\.yml/,
);
});
it("derives the forbidden engines rather than listing them by hand", () => {
// Adding a fourth engine to SUPPORTED_BROWSERS must extend the guard, not
// open a hole in it — which is why this reads the derived constant.
assert.deepEqual(FORBIDDEN_ENGINE_SCRIPTS, [
"smoke:web:firefox",
"smoke:web:webkit",
"smoke:web:engine",
]);
});
});
describe("the gate's name", () => {
const scripts = JSON.parse(
readFileSync(join(repoRoot, "package.json"), "utf8"),
).scripts;
it("is `local:gate`, and runs the local-only Storybook step", () => {
assert.ok(scripts["local:gate"], "the pre-push gate must exist");
assert.match(scripts["local:gate"], /local:storybook/);
});
it("has no `ci` alias, not even a deprecated one", () => {
// The rename is only worth anything if the old name is GONE (#2146): an
// alias preserves exactly the association being scrubbed, and leaves
// something copy-pasteable for a workflow author. Nothing else notices a
// reintroduced `ci`, so assert it here (Copilot).
const revived = Object.keys(scripts).filter(
(name) => name === "ci" || name.startsWith("ci:"),
);
assert.deepEqual(
revived,
[],
`no root script may be named \`ci\` or \`ci:*\` — found ${revived.join(", ")}`,
);
});
});
describe("extractExecutableRegions", () => {
it("descends the executable paths and skips the metadata beside them", () => {
const regions = extractExecutableRegions(
[
"on: push",
"jobs:",
" build:",
" steps:",
" - name: smoke:web:firefox is local-only",
" env:",
" SMOKE_BROWSER: chromium",
" run: npm run smoke",
].join("\n"),
);
// The step `name:` is absent, and the two executable values are not.
// Order follows the walk (a step's `run` before its `env`), not the file.
assert.deepEqual(
regions.map((r) => [r.kind, r.text.trim()]),
[
["run", "npm run smoke"],
["env", "SMOKE_BROWSER: chromium"],
],
);
});
it("throws on a workflow it cannot parse, rather than finding nothing", () => {
// A file that does not parse must not read as a file with nothing in it —
// that is the one failure mode where an unreadable workflow passes.
assert.throws(
() => extractExecutableRegions("jobs:\n - a\n b: c\n", "broken.yml"),
/could not parse broken\.yml/,
);
});
});
describe(".github/workflows", () => {
const files = readdirSync(workflowDir).filter((f) => /\.ya?ml$/.test(f));
it("has workflow files to check", () => {
// Deny-by-default: an empty glob would otherwise make the check below pass
// vacuously after a rename or a move of the workflow directory.
assert.ok(
files.length > 0,
`no workflow files found in ${workflowDir} — the guard below would pass vacuously`,
);
});
it("never invokes the local pre-push gate or a non-Chromium engine pass", () => {
const findings = files.flatMap((file) =>
findWorkflowViolations(
readFileSync(join(workflowDir, file), "utf8"),
file,
),
);
assert.deepEqual(
findings,
[],
"GitHub CI must not run the local-only gate or a non-Chromium engine " +
`pass (#2146):\n${formatWorkflowViolations(findings)}`,
);
});
});

6
scripts/run-engine-smokes.test.mjs
View file

@ -73,7 +73,7 @@ describe("every engine tier consumes ENGINE_SMOKES", () => {
it("the Firefox tier goes through the runner and is in the pre-push gate", () => {
assert.match(scripts["smoke:web:firefox"], /run-engine-smokes\.mjs/);
assert.match(scripts.ci, /smoke:web:firefox/);
assert.match(scripts["local:gate"], /smoke:web:firefox/);
});
it("every supported engine has a `smoke:web:<engine>` script", () => {
@ -94,7 +94,7 @@ describe("every engine tier consumes ENGINE_SMOKES", () => {
// `smoke:web:tabs` is not engine-sensitive and deliberately stays out of
// ENGINE_SMOKES — but "Chromium-only" is a claim the script has to keep.
// Without the literal engine it followed ambient `SMOKE_BROWSER`, so
// `SMOKE_BROWSER=firefox npm run ci` would have run it under Firefox
// `SMOKE_BROWSER=firefox npm run local:gate` would have run it under Firefox
// (Copilot, #2148). Both halves are pinned because they must AGREE: an
// install of one engine and a launch of another surfaces as a confusing
// "Executable doesn't exist" rather than as the wiring error it is.
@ -109,7 +109,7 @@ describe("every engine tier consumes ENGINE_SMOKES", () => {
it("each gated tier names its engine explicitly rather than reading the env", () => {
// An ambient SMOKE_BROWSER must not be able to redirect a gate: without the
// literal engine, `SMOKE_BROWSER=firefox npm run ci` would run Firefox twice
// literal engine, `SMOKE_BROWSER=firefox npm run local:gate` would run Firefox twice
// and never exercise Chromium at all.
assert.match(
scripts["smoke:web:chromium"],

4
scripts/smoke-web-app.mjs
View file

@ -31,7 +31,7 @@
* file in the tarball packlist and on disk after a real install — and, since
* #2003, drives phase 1's flow against the **installed** bin. Keep both:
* pack:verify covers the published artifact, this covers the repo tree on every
* `npm run ci`, where pack:verify (network-bound, local/release-only) does not
* `npm run local:gate`, where pack:verify (network-bound, local/release-only) does not
* run — and this is the only one of the two that drives phase 2. Neither
* subsumes the other.
*
@ -47,7 +47,7 @@
* `webkit`). Three tiers, deliberately (#2086):
*
* - **GitHub CI** runs this smoke in **Chromium** only.
* - **`npm run ci`**, the local pre-push gate, runs it in **Chromium and
* - **`npm run local:gate`**, the local pre-push gate, runs it in **Chromium and
* Firefox** — the Firefox pass is `smoke:web:firefox`, and it is the one
* gate step with no GitHub CI counterpart.
* - **WebKit is on demand only**: `SMOKE_BROWSER=webkit npm run smoke:web:app`

2
scripts/smoke-web-browser.mjs
View file

@ -47,7 +47,7 @@
* 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` (#2086). GitHub CI runs this in **Chromium**
* only; `npm run ci`, the local pre-push gate, also runs it in **Firefox** via
* only; `npm run local:gate`, the local pre-push gate, also runs it in **Firefox** via
* `smoke:web:firefox`; **WebKit is on demand only**. This smoke passes in all
* three — it is the two App smokes that fail under WebKit (see their headers).
*

2
scripts/smoke-web-elicitation.mjs
View file

@ -28,7 +28,7 @@
* `webkit`). Three tiers, deliberately (#2086):
*
* - **GitHub CI** runs this smoke in **Chromium** only.
* - **`npm run ci`**, the local pre-push gate, runs it in **Chromium and
* - **`npm run local:gate`**, the local pre-push gate, runs it in **Chromium and
* Firefox** — the Firefox pass is `smoke:web:firefox`, and it is the one
* gate step with no GitHub CI counterpart.
* - **WebKit is on demand only**: `SMOKE_BROWSER=webkit npm run smoke:web:elicit`

Some files were not shown because too many files have changed in this diff Show more