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

build(web): fail vite build on a browser-externalized Node built-in (#1769) (#1777)

Browse source 
* build(web): fail vite build on a browser-externalized Node built-in (#1769)

Vite 8 (rolldown) externalizes a node:* / Node built-in that reaches the
browser graph with a warning and ships a `module.exports = {}` stub — the
build still succeeds, shipping a broken bundle. The runtime browser smoke
(smoke:web:browser, #1615) only catches the subset where that stub is *called*
at module init; a stub imported but never called ships silently and nothing
gates it.

Add a Vite build plugin that turns that specific browser-externalization
warning into a hard `vite build` error, moving detection upstream into
`npm run build` / `validate` and additionally catching the never-called case
the runtime smoke can't see.

- clients/web/server/browser-externalized-builtin-gate.ts: Vite-agnostic
  detection + error + per-build state machine (unit-tested to the ≥90 gate).
- vite.config.ts: thin Vite wiring. A throw inside rolldown's onLog is
  swallowed (the one hook where it doesn't abort — verified against vite@8.0.0),
  so the gate records the warning in onLog and re-throws in buildEnd. Scoped to
  `vite build` (apply: 'build'); the Node runner build (tsup) is unaffected.
- scripts/verify-build-gate.mjs + verify:build-gate (in npm run ci and the
  GitHub workflow): runs a real build with a node:fs probe and asserts it fails
  via the gate — the only check that catches the warning phrasing drifting in a
  future Vite bump and silently disabling the message-keyed gate.

There is no stable log `code` on the rolldown warning (only message + plugin),
so the gate keys off the documented message phrasing; the drift guard above is
what keeps that safe across Vite versions. smoke:web:browser stays as the
runtime backstop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): harden the browser-externalized-builtin gate per review (#1769)

Review follow-ups on PR #1777:

- Per-build state: add gate.reset() called from the plugin's buildStart, so a
  `vite build --watch` rebuild doesn't inherit a prior build's recorded warning
  (the plugin instance is reused across rebuilds). Makes the docblock true.
- buildEnd(error) now only asserts on the success path — rolldown also calls
  buildEnd(error) when the build already failed for another reason, and throwing
  then would mask that real error with the #1769 message.
- verify-build-gate.mjs: drop the shell `rm -rf` cache-clear entirely (verified
  the gate fires reliably without it — the earlier "caching" symptom was the
  onLog-swallow behavior, not a stale cache); add a result.error spawn-failure
  check so a missing `npx` isn't misdiagnosed as "not via the gate"; add
  SIGINT/SIGTERM handlers so an interrupt during the multi-minute build still
  restores the mutated entry; add a progress log before the spawn.
- Match the double-quote style of the sibling server/ files.
- Docs: list browser-externalized-builtin-gate.ts in the AGENTS.md server/ block
  and the clients/web/README server/ list (with why a build helper lives under
  server/), and add verify-build-gate to the root README scripts/ line.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-2 review hardening for the #1769 build gate

- Scope the gate to the browser (`client`) environment via applyToEnvironment,
  so a future SSR/node environment built from this config isn't failed for a
  legitimate node:* import — browser-only intent is now structural.
- Report ALL externalized built-ins in one build, deduped, instead of only the
  first offender (N leaks no longer means N fix-and-rebuild cycles).
  browserExternalizedBuiltinError now takes string[]; gate uses a Set.
- verify-build-gate.mjs: pin Vite with `npx --no-install` so the drift check
  can never silently run a registry-fetched Vite instead of the repo-pinned one
  (the whole point is version fidelity); append the probe instead of prepending
  (ES imports hoist, and appending won't demote a future leading directive);
  clarify the signal-handler comment (handlers are a backstop — a blocked
  spawnSync tears down via the process group first).
- AGENTS.md: update both `ci` pipeline enumerations to include verify:build-gate
  and drop the now-false "Storybook is the only CI step left out" claim.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-3 review polish for the #1769 build gate

- Error message: name the transitive-dependency remedy (a resolve.alias to a
  browser shim) — "remove the import / gate behind the backend" doesn't apply
  when the node:* import lives in node_modules.
- isBrowserExternalizedBuiltinLog is now a `message is string` type guard, so
  recordLog drops its redundant `!== undefined` clause while still typing the
  Set.add() — a real one-liner, not a coverage-only removal.
- verify-build-gate.mjs: docblock now states WHY it builds the real config +
  entry (catches config-level regressions — plugin removed, onwarn suppression
  added — that a temp config would miss), so the minute-long build isn't later
  "optimized" into a temp entry and silently lose that coverage. Guard the
  entry read so a renamed main.tsx fails through fail() with an actionable
  message. On a botched restore, save the captured original to a sidecar .bak
  and point there instead of recommending `git checkout --`, which would
  discard uncommitted edits.
- AGENTS.md: fix the muddled CI-steps sentence (GitHub CI runs the whole chain
  as separate steps, Storybook last) and note the applyToEnvironment client-only
  scoping in the gate prose.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-4 review polish — sharper verify-build-gate diagnostics (#1769)

All in scripts/verify-build-gate.mjs's failure paths:

- Split the "build SUCCEEDED" failure into three diagnoses keyed off the
  captured output, so it names the right file: warning present but build passed
  → plugin not applying (removed from `plugins`, `applyToEnvironment` env-name
  mismatch, or an `onwarn` suppression); node:fs present but phrase absent →
  phrasing drift, update the constant; neither → probe never reached the graph.
  The old message hard-coded "phrasing drifted", which misdirects for the
  config-regression class this script exists to catch. Added applyToEnvironment
  to the docblock's config-regression list too.
- Bound the build with a 10-min spawnSync timeout (+ SIGKILL); a timeout sets
  result.error (ETIMEDOUT) and reports via the existing branch, instead of
  burning to the GitHub job's 360-min default with no output.
- Guard the probe write so a read-only/EACCES checkout fails through fail()
  with an actionable message instead of escaping as a raw stack.
- SIGTERM now exits 143 (was 130, which is SIGINT's code).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-5 review polish — self-consistent verify-build-gate (#1769)

All in scripts/verify-build-gate.mjs:

- Drift guard: assert the gate source file still contains the mirrored
  KNOWN_PHRASE literal, failing fast if the two drift. Without it, a stale
  KNOWN_PHRASE after a phrasing update would make the three-way diagnosis
  misreport a plugin-not-applying regression as a phrasing drift — the exact
  misdirection the diagnosis exists to avoid.
- Make the .bak safety net reachable on the path that needs it most: restoreEntry
  now catches its own write failure and routes to a shared saveBackupAndFail
  helper, instead of letting the write escape the finally as a raw stack (which
  skipped the .bak fallback and left the probe injected in the entry).
- Add the fourth "plugin isn't applying" cause to the diagnosis: a future Vite
  emitting the warning before the client env's buildStart reset clears it (same
  symptom as the other three).
- Flag once, on the failing-check path, that clients/web/dist holds a probe
  build so a local debugger doesn't serve it unaware.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-6 review polish — verify-build-gate literal contracts (#1769)

All in scripts/verify-build-gate.mjs; one theme — the literals the script
asserts on can each go stale independently of what they mirror:

- Success key: assert on the gate's distinctive thrown-error prefix
  "Build failed (#1769)", not a bare "#1769". The issue number also appears in
  vite.config.ts comments, so a code frame from an unrelated build error there
  could otherwise make the check report OK with the gate dead — the one
  direction it must never get wrong (a false pass hiding a dead gate).
- Drift guard now anchors on the BROWSER_EXTERNALIZED_BUILTIN_PHRASE assignment
  (regex) instead of a whole-file substring match, so the old wording lingering
  in a comment can't mask a changed constant; and its readFileSync of the gate
  module is guarded so a moved file fails through fail() actionably.
- Extract PROBE_MODULE ("node:fs") used to build both the PROBE and the
  diagnosis check + messages, so changing the probe module can't silently
  misroute a phrasing-drift failure into "probe never reached the graph".
- Comment the load-bearing diagnosis order (phrase before module-name).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-7 review polish — drift-guard both mirrored literals (#1769)

- Extract ERROR_PREFIX ("Build failed (#1769)") and drift-guard it against the
  gate module too, and use it as the success key. Previously only KNOWN_PHRASE
  was guarded; the prefix is mirrored across the same TS/.mjs boundary for the
  same reason, so rewording the gate's thrown error would make a correctly-firing
  gate report "the build broke for another reason" (benign direction, but the
  same misdirection the other guards close).
- Write the restore-failure .bak under the OS temp dir, not next to the entry:
  a sidecar inside src/ is untracked and a `git add -A` (likely while recovering)
  would sweep it into a commit.
- Branch-(b) remediation now says update BOTH the gate constant AND KNOWN_PHRASE,
  so a phrasing-drift fix lands in one pass instead of tripping the drift guard
  on the next run.
- Note next to BROWSER_EXTERNALIZED_BUILTIN_PHRASE that it must stay a single
  string literal (the script's drift guard anchors a regex on the assignment).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-8 review polish — safe recovery path + structural onLog order (#1769)

- saveBackupAndFail no longer claims a backup it may not have written: it tracks
  whether the write succeeded and only points the user at the .bak in that case
  (otherwise it says the probe is still injected and to restore from VCS). Uses
  mkdtempSync for a unique dir instead of a fixed tmp filename, so a
  pre-existing/foreign-owned /tmp file can't make the recovery message point at
  someone else's content.
- Guard the restore-verification readFileSync too, routing a failure through
  saveBackupAndFail so the whole restore path is uniformly safe (a raw throw
  there would skip the backup net).
- vite.config.ts: enforce: 'pre' on the gate plugin so it runs ahead of the
  normal-plugin group in the onLog chain — a plugin whose onLog returns false
  filters the log for later plugins, so trailing the array would let a future
  log-filtering plugin silently blind the gate. Makes "the gate sees every log"
  structural rather than array-position-dependent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-9 review polish — prove the gate fired on the probe (#1769)

The success key confirmed the #1769 gate fired, but not that OUR probe tripped
it. If the repo already leaked a Node built-in, the gate would fire on that and
the script would report OK even if the probe was tree-shaken or entryPath went
stale — branch (c)'s failure silently inverted into a pass. Add an assertion
that PROBE_MODULE appears among the gate's embedded offender warnings. Unreachable
inside `npm run ci` (validate's build:web fails first on a pre-existing leak),
but this script is a documented standalone command — run exactly when debugging
a leak, the case where this mattered.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-10 review polish — roll back a partial probe write (#1769)

The probe writeFileSync uses the default 'w' flag (truncates at open), so a
mid-write ENOSPC/EIO could leave clients/web/src/main.tsx empty or half-written
— and its catch called fail() directly, the one path on this script that could
destroy the entry rather than merely dirty it (every other read/write is already
routed into the recovery net). Call restoreEntry() in that catch to roll the
partial write back (falling through to the .bak net if the restore itself can't
write), and correct the comment that claimed a write failure happens "before any
mutation". Also widen the enforce:'pre' comment to note it orders ahead of Vite's
core plugins too (harmless — the gate defines no resolve/load/transform).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-11 review polish — don't let the rollback swallow the write error (#1769)

Round 10's restoreEntry() in the probe-write catch swallowed the primary error:
restoreEntry() on its own write failure calls saveBackupAndFail → fail → exit,
so the "could not write the probe" diagnosis never ran when the rollback also
failed — which is exactly the read-only-checkout / ENOSPC case (both writes fail
at open). On a read-only checkout nothing was ever written, yet the developer
was told the restore failed and pointed at a /tmp backup of an untouched file.

Roll back inline instead: attempt the restore without exiting, track whether it
succeeded, then always report the PRIMARY write error, appending "restore from
version control" only when the rollback didn't take. Also pin the
"Build failed (#1769)" error prefix as contiguous in a single string fragment
(a comment symmetric with the phrase pin), since verify-build-gate.mjs
drift-guards on it as ERROR_PREFIX.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* build(web): round-12 review polish — don't let the pin comment defeat its own guard (#1769)

The round-11 pin comment quoted "Build failed (#1769)" verbatim, so the prefix
appeared twice in the gate module and verify-build-gate.mjs's whole-file
includes(ERROR_PREFIX) drift guard passed on the comment alone — rewording the
thrown error would have kept the guard green while a correctly-firing gate
reported "the build broke for another reason." Reworded the comment to not repeat
the full lead (matching how the phrase pin avoids repeating its phrase); the
literal now occurs exactly once, restoring the guard's meaning.

Also make the probe-write recovery message factual instead of inferred: read the
entry back and only warn "restore from version control" when it actually differs
from the original. On a read-only checkout both writes fail at open, so the file
is untouched and the old hedge needlessly implied damage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall 2026-07-25 19:45:11 -04:00 • committed by GitHub
commit 07d6316fba
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 655 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -53,6 +53,15 @@ jobs:
# running the integration suite twice.
run: npm run coverage
- name: Verify the browser-externalized-builtin build gate (#1769)
# Runs a real `vite build` with a Node built-in forced into the browser
# graph and asserts the build FAILS via the #1769 gate. The unit tests
# cover the detection logic against a captured message; only this real
# build catches the risk the issue calls out — that the Vite warning
# phrasing drifts across releases, silently disabling the message-keyed
# gate. It restores the mutated entry afterward (see the script).
run: npm run verify:build-gate
# Playwright chromium is installed BEFORE the smokes because
# `smoke:web:browser` (the headless-browser boot smoke, #1615) drives the
# prod web bundle in chromium — restoring/installing it here lets that

9
AGENTS.md
View file

@ -16,7 +16,9 @@ inspector/
│ │ │ # web-server-config.ts (env parsing + initial-config payload + banner),
│ │ │ # sandbox-controller.ts (MCP Apps sandbox HTTP server),
│ │ │ # inject-auth-token.ts (embeds the API token into served index.html),
│ │ │ # vite-base-config.ts (shared optimizeDeps exclusions)
│ │ │ # vite-base-config.ts (shared optimizeDeps exclusions),
│ │ │ # browser-externalized-builtin-gate.ts (build-gate logic that fails
│ │ │ # `vite build` on a browser-externalized Node built-in — #1769)
│ │ └── static/ # sandbox_proxy.html (served by sandbox-controller for MCP Apps tab)
│ ├── cli/ # CLI client (tsup bundle, @inspector/core alias)
│ ├── tui/ # TUI client (Ink + React, tsup bundle)
@ -210,16 +212,17 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i
### Mandatory pre-push gate
- ALWAYS do `npm run format` before committing — the **root** `format` auto-fixes `core/` (`format:core`) and every client's scope in one shot. `validate` runs `format:check` (the non-fixing variant, including `format:check:core`) and will fail in CI on any unformatted file, so always run the auto-fixer first rather than letting `format:check` catch it.
- **`npm run ci` is the mandatory pre-push command** — it mirrors `.github/workflows/main.yml` (minus `npm install`): `validate` → `coverage` → `smoke` → Storybook play-function tests (installs Playwright chromium if needed). It now runs **`npm run coverage`**, the per-file ≥90 gate (lines/statements/functions/branches) that CI enforces — so `npm run ci` is a true superset of GitHub CI, and passing it locally means CI's gates will pass. Expect several minutes. **`npm run validate`** remains the fast inner-loop check during development (unit tests only — no coverage gate, no smoke, no Storybook), but it is **NOT** an acceptable substitute for `npm run ci` before pushing: `validate` runs `test`, not `test:coverage`, so it does **zero** coverage gating. Skipping the gate is how a push passes every fast local check and still fails CI (this exact gap broke PR #1601 on a function-coverage regression).
- **`npm run ci` is the mandatory pre-push command** — it mirrors `.github/workflows/main.yml` (minus `npm install`): `validate` → `coverage` → `verify:build-gate` (the #1769 browser-externalized-builtin build gate) → `smoke` → Storybook play-function tests (installs Playwright chromium if needed). It now runs **`npm run coverage`**, the per-file ≥90 gate (lines/statements/functions/branches) that CI enforces — so `npm run ci` is a true superset of GitHub CI, and passing it locally means CI's gates will pass. Expect several minutes. **`npm run validate`** remains the fast inner-loop check during development (unit tests only — no coverage gate, no smoke, no Storybook), but it is **NOT** an acceptable substitute for `npm run ci` before pushing: `validate` runs `test`, not `test:coverage`, so it does **zero** coverage gating. Skipping the gate is how a push passes every fast local check and still fails CI (this exact gap broke PR #1601 on a function-coverage regression).
- ALWAYS do `npm run format` before committing, then **`npm run ci`** before pushing. From the repo root, `validate` runs the **`core/` gate first** (`validate:core`) and then chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each client delegates to its own `npm run validate` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies).
- **`validate:core` is the shared-code format + lint gate (#1689).** Each client's `prettier`/`eslint` is scoped to its own dir, so nothing reached `core/` before — `validate:core` closes that: it runs `format:check:core` (`prettier --check "core/**/*.{ts,tsx}"`) + `lint:core` (`eslint "core/**/*.{ts,tsx}"` via the **root** `eslint.config.js`). Use `npm run format:core` to auto-fix. The root carries prettier/eslint as devDependencies for this; `core/` is isomorphic (browser + Node globals, no JSX today — the `{ts,tsx}` glob future-proofs against a `core/**/*.tsx`). The root `eslint.config.js` honors an `_`-prefix as the intentionally-unused marker (`argsIgnorePattern`/`varsIgnorePattern`/`caughtErrorsIgnorePattern: '^_'`).
- **cli and tui now typecheck their `src` (#1689).** Their `build`/`test` run through esbuild (no type check), so each has a `typecheck` script (`tsc --noEmit -p tsconfig.json`) folded into `validate`. Their `tsconfig.json` matches `clients/web/tsconfig.app.json`'s module/lib *resolution* options — DOM lib, `moduleResolution: bundler`, and **no** `noUncheckedIndexedAccess` (web's app config does not extend `tsconfig.base`, so re-enabling it would surface `core/` issues web never gates) — so the imported `core/` sources are validated the same way web validates them. It does **not** mirror web's extra strictness flags (`noUnusedLocals`, `verbatimModuleSyntax`, ES2023 target, …), so cli/tui's own `src` is checked slightly more loosely than web's. `core/` itself still typechecks through web's `tsc -b`.
- The one CLI nuance: `clients/cli`'s out-of-process `e2e.test.ts` spawns the built binary, so its `test` **builds first** via `pretest` (`test-servers:build && build`). To avoid building it twice, `clients/cli`'s `validate` folds that in — it is `format:check && lint && typecheck && test` with **no** separate `build` step (the other clients, whose tests don't spawn their bundle, keep an explicit `build`). `validate:web`/`validate:tui`/`validate:launcher` are the uniform `format:check && lint && (typecheck &&) build && test`.
- **`npm run coverage`** is the per-file ≥90 gate and is now part of `npm run ci` — never treat it as optional before a push. It supersedes the old standalone `test:integration` step: web's `test:coverage` runs the `unit` **and** `integration` projects under v8 instrumentation, so `coverage` both enforces the ≥90 gate and exercises the same web integration paths CI covers.
- **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web` / `smoke:web:browser`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the `coverage` gate (which also covers the web integration project), then `smoke` (with Playwright chromium installed just before it, since `smoke:web:browser` needs it). Storybook is the only CI step left out (see below).
- **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web` / `smoke:web:browser`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the `coverage` gate (which also covers the web integration project), then `verify:build-gate` (the #1769 build gate — see below), then `smoke` (with Playwright chromium installed just before it, since `smoke:web:browser` needs it). GitHub CI runs this same chain as separate workflow steps, with the Storybook play-function tests last (see below).
- `smoke:launcher` (`scripts/smoke-launcher.mjs`) runs the built launcher with `--help`, `--cli --help`, and `--tui --help`, asserting each exits 0 and prints that mode's usage banner (which also proves the launcher resolved and loaded the right client build). It's the cheap dispatch check before the heavier prod smokes below.
- `smoke:web` (`scripts/smoke-web.mjs`) starts `mcp-inspector --web` (prod, no `--dev`) against the built `clients/web/dist` and asserts `GET /` serves the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. Prod `--web` serves from `clients/web/dist`, which ships in the published package but is absent in a fresh checkout — the runner builds it on demand (`build:client` = `vite build`) on first launch, or exits with an actionable error if that build can't run (see `clients/web/server/ensure-web-build.ts` and the launcher README). `--dev` runs Vite directly and never needs `dist`. It shares the spawn/readiness/teardown helper (`scripts/lib/prod-web-server.mjs`) with `smoke:web:browser`, so the two can't drift.
- `smoke:web:browser` (`scripts/smoke-web-browser.mjs`, #1615) goes a step further than `smoke:web`: it boots the same prod `--web` server and then actually **runs** the bundle in headless Chromium (Playwright — already a `clients/web` devDependency for the Storybook tests), asserting the app renders its first meaningful frame (the "Add Servers" control) with **no uncaught error**. `smoke:web` only checks the served HTML, so a Node built-in reaching the browser bundle slipped through it; this smoke catches that regression as a *class* (e.g. #1612). The mechanism is the uncaught error, not a magic string: under Vite the excluded module becomes an empty stub and the first *call* into it (e.g. `fs.readFileSync(...)` during a transitive module's init) throws a `TypeError` that aborts app mount. A *synchronous* such throw fires `pageerror`; its *async* twin (the same `TypeError` via `await`/`.then()`, or a failed dynamic import) is logged on the console channel as `Uncaught (in promise) …` / `Failed to fetch dynamically imported module` — the smoke hard-fails on both. The literal `Module "…" has been externalized` text is, **in a prod build**, a build-time warning (`vite build` / `npm run build`), not a runtime message, so the browser never sees it (under `npm run dev` Vite's stub is instead a `Proxy` that `console.warn`s that string at runtime); and an externalized import that is never *called* ships a harmless `{}` and is invisible here by design. Every *other* console error is printed as a diagnostic, not a failure (so a benign font-CDN or React-warning `console.error` doesn't flake CI). Playwright is resolved via `createRequire` based at `clients/web/package.json` — a bare `import("playwright")` would resolve relative to `scripts/`, not the cwd, so it can't be reached that way (it only appears to work when an ancestor `node_modules` carries playwright, and fails in CI, which has none). The npm script's `cd clients/web` exists only so `npx playwright install chromium` finds the local playwright bin (a no-op when already installed).
- **The build gate for the browser-externalized-builtin class (#1769)** is the earlier, more complete companion to `smoke:web:browser`. A Vite plugin in `clients/web/vite.config.ts` (logic in `clients/web/server/browser-externalized-builtin-gate.ts`, unit-tested) turns Vite 8's *browser-externalization warning* (`Module "node:*" has been externalized for browser compatibility`) into a hard `vite build` error, so a Node built-in in the browser graph now **fails `npm run build` / `validate`** instead of shipping a `{}` stub. This catches **both** the *called-at-init* case (which `smoke:web:browser` also catches, but later/at runtime) **and** the *imported-but-never-called* case (the `{}` stub that is invisible to the runtime smoke "by design" — see above). Because rolldown **swallows a throw inside `onLog`** (the one hook where a thrown error doesn't abort — verified against vite@8.0.0), the plugin *records* the warning in `onLog` and re-throws in `buildEnd`. There is **no stable log `code`**, so the gate keys off the documented message phrasing; `npm run verify:build-gate` (`scripts/verify-build-gate.mjs`, in `npm run ci` and the GitHub workflow) runs a real build with a `node:fs` probe forced into `src/main.tsx` and asserts the build fails via the gate — the only check that catches the message phrasing **drifting** in a future Vite bump and silently disabling the gate. The gate is scoped to `vite build` (`apply: 'build'`) — never `vite dev` or the vitest projects — **and** to the browser (`client`) environment (`applyToEnvironment`), so a future SSR/node environment built from this config isn't failed for a legitimate `node:*` import; the Node runner build (tsup, `build:runner`) is a separate config where built-ins are legitimate. `smoke:web:browser` stays as the runtime backstop for crashes the build can't reason about.
- `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog <temp>` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. **`smoke:tui` is local-only: it self-skips when `process.env.CI` is set**, because the Ink TUI needs a real TTY (raw mode) that headless CI lacks — so run it (via `npm run smoke`) on your own machine before pushing. Both build `test-servers/build` on demand if it's missing.
- Storybook play-function tests (`clients/web` `test:storybook`) run in headless Chromium via `@vitest/browser-playwright` (~10s). They are part of `npm run ci` (which installs Playwright chromium first); kept out of `validate` because they need the browser binary and are slower than the unit suite.

5
README.md
View file

@ -35,7 +35,7 @@ inspector/
│ ├── react/ # React hooks over the state stores
│ └── storage/ # File I/O helpers for the OAuth persist backends
├── test-servers/ # Composable MCP test servers + fixtures used by integration tests
├── scripts/ # Root build/verify tooling (install cascade, smokes, pack:verify)
├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, pack:verify)
├── specification/ # Design/build specifications
├── AGENTS.md # Contribution rules for agents AND humans (see below)
└── README.md # You are here
@ -143,7 +143,8 @@ Each client self-validates from its own folder; the root scripts chain them. The
| `npm run validate` | Runs `validate:core` (the shared `core/` `format:check` + `lint` gate) first, then per client: `format:check` + `lint` + **`typecheck`** (cli/tui only) + `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), plus a headless-Chromium 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). |
| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `smoke` → Storybook. A true superset of GitHub 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 ci`. |
| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook. A true superset of GitHub CI. |
| `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. Run `npm run format` before committing — the root `format` fixes `core/` and every client; `validate` runs the non-fixing `format:check` and fails CI on any unformatted file.

1
clients/web/README.md
View file

@ -19,6 +19,7 @@ The `server/` directory holds the Node-only backend:
- **`web-server-config.ts`** — env parsing, the `GET /api/config` payload, the startup banner.
- **`inject-auth-token.ts`** — embeds the API token into the served `index.html` (see [Auth token](#auth-token)).
- **`sandbox-controller.ts`** — the MCP Apps sandbox HTTP server; **`ensure-web-build.ts`** — builds `dist/` on demand for prod `--web`; **`vite-base-config.ts`** — shared `optimizeDeps` exclusions.
- **`browser-externalized-builtin-gate.ts`** — Vite-agnostic build-gate logic that fails `vite build` when a Node built-in reaches the browser bundle (#1769); the thin Vite plugin wiring lives in `vite.config.ts`. It sits under `server/` (rather than `src/`) as the home for Node-only, build-time tooling — it's imported by the Vite config, never by the browser — alongside the other `vite-*` config helpers here.
## Development

104
clients/web/server/browser-externalized-builtin-gate.ts Normal file
View file

@ -0,0 +1,104 @@
/**
* Build-gate logic for #1769: fail a browser `vite build` when a Node built-in
* reaches the browser graph.
*
* Vite 8 (rolldown) *externalizes* a `node:*` / bare Node built-in that leaks
* into the browser bundle — it emits a warning and ships a `module.exports = {}`
* stub, and the build still succeeds. That green build ships a broken bundle
* (#1615). The runtime browser smoke (`smoke:web:browser`) only catches the
* subset where the stub is *called* at module init (CASE 1); a stub that's
* imported but never called ships silently (CASE 2) and nothing gates it.
*
* Promoting that specific warning to a hard build error moves CASE 1 detection
* upstream into `npm run build` / `validate` and additionally catches CASE 2.
*
* This module holds the Vite-agnostic pieces so they can be unit-tested without
* standing up a build; the thin Vite `Plugin` that wires them into `onLog` /
* `buildEnd` lives in `vite.config.ts`, scoped there to the browser (`client`)
* environment. The reason the throw can't happen in `onLog` directly (rolldown
* swallows it there) is documented at that call site.
*/
// The phrase Vite 8 (rolldown) emits when a Node built-in was externalized for
// the browser. There is no stable rollup `code` on this rolldown log — verified
// against the pinned vite@8.0.0, the log carries only `message` +
// `plugin: "rolldown:vite-resolve"` — so the gate keys off this documented,
// user-facing phrasing, whose troubleshooting anchor
// (`module-externalized-for-browser-compatibility`) Vite treats as stable.
// The `verify:build-gate` script exercises a real build so a phrasing drift in a
// future Vite bump fails CI here rather than silently disabling the gate. That
// script also drift-guards its mirrored copy by regex against this assignment —
// keep it a single string literal (no concatenation/template) or the guard will
// report a false drift.
export const BROWSER_EXTERNALIZED_BUILTIN_PHRASE =
"has been externalized for browser compatibility";
// True for a build log announcing a browser-externalized Node built-in. Typed
// as a `message is string` guard: a match implies a defined string, so callers
// can add it to a `Set<string>` without a separate `undefined` check.
export function isBrowserExternalizedBuiltinLog(
message: string | undefined,
): message is string {
return message?.includes(BROWSER_EXTERNALIZED_BUILTIN_PHRASE) ?? false;
}
/**
* The actionable error a browser-externalized Node built-in fails the build
* with. Takes every matched warning so a build that leaks several built-ins
* reports them all in one pass, rather than one-per-rebuild.
*/
export function browserExternalizedBuiltinError(messages: string[]): Error {
const list = messages.map((m) => ` - ${m}`).join("\n");
// Keep the error's `#1769` lead contiguous in a single string fragment:
// scripts/verify-build-gate.mjs mirrors that lead as ERROR_PREFIX (its success
// key) and drift-guards on it as a whole-file substring, so it must occur
// exactly ONCE here — do not quote the full lead in this comment, and don't
// reflow it across fragments, or the guard would pass on a copy and mask a
// reworded error.
return new Error(
"Build failed (#1769): a Node built-in reached the browser bundle and was " +
"externalized to an empty stub, which ships a broken bundle. Remove the " +
"node:* / Node built-in import(s) from the browser graph (or gate them " +
"behind the Node-only dev backend). If an import comes from a dependency " +
"(not first-party code — see the module path in the warning below), add a " +
"`resolve.alias` in clients/web/vite.config.ts pointing it at a browser " +
"shim.\n\nOriginal Vite warning(s):\n" +
list,
);
}
/** Records browser-externalization build logs and fails the build if any were seen. */
export interface BrowserExternalizedBuiltinGate {
/** Feed each build log's message here; matching ones are collected (deduped). */
recordLog(message: string | undefined): void;
/** Throw {@link browserExternalizedBuiltinError} if any match was recorded. */
assertClean(): void;
/** Clear recorded state so a rebuild (e.g. `vite build --watch`) starts fresh. */
reset(): void;
}
/**
* A per-build detector: `recordLog` is called for every build log (from the
* plugin's `onLog`), `assertClean` is called once resolution is complete (from
* the plugin's `buildEnd`) and throws — listing every offender — if any Node
* built-in was externalized. The plugin `reset`s it in `buildStart` so a
* watch-mode rebuild doesn't inherit a previous build's recorded warnings.
*/
export function createBrowserExternalizedBuiltinGate(): BrowserExternalizedBuiltinGate {
const externalized = new Set<string>();
return {
recordLog(message) {
if (isBrowserExternalizedBuiltinLog(message)) {
externalized.add(message);
}
},
assertClean() {
if (externalized.size > 0) {
throw browserExternalizedBuiltinError([...externalized]);
}
},
reset() {
externalized.clear();
},
};
}

120
clients/web/src/test/integration/server/browser-externalized-builtin-gate.test.ts Normal file
View file

@ -0,0 +1,120 @@
import { describe, it, expect } from "vitest";
import {
BROWSER_EXTERNALIZED_BUILTIN_PHRASE,
isBrowserExternalizedBuiltinLog,
browserExternalizedBuiltinError,
createBrowserExternalizedBuiltinGate,
} from "../../../../server/browser-externalized-builtin-gate.js";
// Run `fn`, expect it to throw an Error, and return the thrown message —
// typed, so assertions on the message don't reach through an `any` catch var.
function messageFromThrow(fn: () => void): string {
try {
fn();
} catch (err) {
if (err instanceof Error) return err.message;
throw err;
}
throw new Error("expected the call to throw, but it did not");
}
// A real message captured from vite@8.0.0's build log (see the gate module).
const REAL_MESSAGE =
'Module "node:fs" has been externalized for browser compatibility, ' +
'imported by "/app/src/main.tsx". See https://vite.dev/guide/troubleshooting.html' +
"#module-externalized-for-browser-compatibility for more details.";
describe("isBrowserExternalizedBuiltinLog", () => {
it("matches the real browser-externalization build message", () => {
expect(isBrowserExternalizedBuiltinLog(REAL_MESSAGE)).toBe(true);
// Sanity: the captured message actually contains the phrase we key off.
expect(REAL_MESSAGE).toContain(BROWSER_EXTERNALIZED_BUILTIN_PHRASE);
});
it("does not match an unrelated build warning", () => {
expect(
isBrowserExternalizedBuiltinLog(
"Some chunks are larger than 500 kB after minification.",
),
).toBe(false);
});
it("does not match an absent message", () => {
expect(isBrowserExternalizedBuiltinLog(undefined)).toBe(false);
});
});
describe("browserExternalizedBuiltinError", () => {
it("builds an actionable #1769 error embedding every original warning", () => {
const second = REAL_MESSAGE.replace("node:fs", "node:path");
const err = browserExternalizedBuiltinError([REAL_MESSAGE, second]);
expect(err).toBeInstanceOf(Error);
expect(err.message).toContain("#1769");
expect(err.message).toContain("externalized to an empty stub");
// Every offending warning is listed so all leaks are visible in one pass.
expect(err.message).toContain(REAL_MESSAGE);
expect(err.message).toContain(second);
});
});
describe("createBrowserExternalizedBuiltinGate", () => {
it("does not throw when no externalization was recorded", () => {
const gate = createBrowserExternalizedBuiltinGate();
expect(() => gate.assertClean()).not.toThrow();
});
it("throws #1769 in assertClean when a matching log was recorded", () => {
const gate = createBrowserExternalizedBuiltinGate();
gate.recordLog(REAL_MESSAGE);
expect(() => gate.assertClean()).toThrow(/#1769/);
// The recorded message is surfaced in the failure.
expect(() => gate.assertClean()).toThrow(REAL_MESSAGE);
});
it("ignores non-matching and absent logs", () => {
const gate = createBrowserExternalizedBuiltinGate();
gate.recordLog("unrelated warning");
gate.recordLog(undefined);
expect(() => gate.assertClean()).not.toThrow();
});
it("reports every distinct offender when several are recorded", () => {
const gate = createBrowserExternalizedBuiltinGate();
const other = REAL_MESSAGE.replace("node:fs", "node:path");
gate.recordLog(other);
gate.recordLog(REAL_MESSAGE);
// Both leaks surface in one failure rather than one-per-rebuild.
const message = messageFromThrow(() => gate.assertClean());
expect(message).toContain(other);
expect(message).toContain(REAL_MESSAGE);
});
it("dedupes a repeated offender message", () => {
const gate = createBrowserExternalizedBuiltinGate();
gate.recordLog(REAL_MESSAGE);
gate.recordLog(REAL_MESSAGE);
// Listed once, not twice.
const message = messageFromThrow(() => gate.assertClean());
const occurrences = message.split(REAL_MESSAGE).length - 1;
expect(occurrences).toBe(1);
});
it("reset() clears a recorded warning so a rebuild starts clean", () => {
const gate = createBrowserExternalizedBuiltinGate();
gate.recordLog(REAL_MESSAGE);
gate.reset();
// Post-reset the gate is clean...
expect(() => gate.assertClean()).not.toThrow();
// ...and can record + fail again on the next build.
gate.recordLog(REAL_MESSAGE);
expect(() => gate.assertClean()).toThrow(/#1769/);
});
it("keeps state per instance", () => {
const dirty = createBrowserExternalizedBuiltinGate();
dirty.recordLog(REAL_MESSAGE);
const clean = createBrowserExternalizedBuiltinGate();
expect(() => clean.assertClean()).not.toThrow();
expect(() => dirty.assertClean()).toThrow(/#1769/);
});
});

62
clients/web/vite.config.ts
View file

@ -1,5 +1,5 @@
/// <reference types="vitest/config" />
import { defineConfig } from 'vite';
import { defineConfig, type Plugin } from 'vite';
import react from '@vitejs/plugin-react';
// https://vite.dev/config/
@ -10,6 +10,7 @@ import { playwright } from '@vitest/browser-playwright';
import { honoMiddlewarePlugin } from './server/vite-hono-plugin';
import { getViteBaseConfig, getViteDevOptimizeDeps } from './server/vite-base-config';
import { buildWebServerConfigFromEnv } from './server/web-server-config';
import { createBrowserExternalizedBuiltinGate } from './server/browser-externalized-builtin-gate';
import { vitestSharedPaths } from '../../vitest.shared.mts';
const dirname = typeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url));
const { repoRoot, sharedDedupe, nodeModulesAliases, projectResolve, sharedAliases } =
@ -27,6 +28,57 @@ const { repoRoot, sharedDedupe, nodeModulesAliases, projectResolve, sharedAliase
// include AND fail to be excluded from unit, silently landing under happy-dom.
const integrationGlob = 'clients/web/src/test/integration/**/*.test.{ts,tsx}';
// Fail `vite build` when a Node built-in lands in the browser bundle (#1769).
// Vite 8 otherwise only *warns* and ships a `module.exports = {}` stub, so a
// broken bundle builds green; the runtime browser smoke (`smoke:web:browser`,
// #1615) only catches the subset where that stub is *called* at module init.
// The detection + error logic lives in `browser-externalized-builtin-gate.ts`
// (Vite-agnostic, unit-tested); this is only the Vite wiring.
//
// Throwing directly in `onLog` does NOT abort a rolldown build (it's the one
// hook where a thrown error is swallowed — verified against vite@8.0.0), so the
// gate *records* the warning in `onLog` and re-throws in `buildEnd`, which runs
// after module resolution (by when the warning has fired) and where a throw
// aborts the build with a non-zero exit.
//
// `apply: 'build'` scopes it to `vite build` (never `vite dev` or the vitest
// projects), and `applyToEnvironment` narrows it to the **browser** (`client`)
// environment so a future SSR/node environment built from this config isn't
// failed for a legitimate `node:*` import — the browser-only intent is
// structural, not incidental. (The Node runner build is a separate tsup config.)
//
// `buildStart` resets the gate so a `vite build --watch` rebuild doesn't inherit
// a prior build's recorded warnings (the plugin instance is reused across
// rebuilds). `buildEnd` only asserts on the *success* path: rolldown also calls
// `buildEnd(error)` when the build already failed for another reason, and
// throwing then would mask that real error with the #1769 message.
function browserExternalizedBuiltinGate(): Plugin {
const gate = createBrowserExternalizedBuiltinGate();
return {
name: 'inspector:fail-on-browser-externalized-builtin',
apply: 'build',
applyToEnvironment: (environment) => environment.name === 'client',
// `enforce: 'pre'` runs this ahead of the normal-plugin group (and Vite's
// core plugins) in the `onLog` chain: a plugin whose `onLog` returns `false`
// filters that log for every later plugin, so trailing the array would let a
// future log-filtering plugin silently blind the gate. Ordering the gate
// first makes "it sees every log" structural rather than dependent on array
// position. Harmless to the emitting `rolldown:vite-resolve` plugin — the gate
// defines no resolve/load/transform, and `onLog` delivery doesn't depend on
// the emitter's position.
enforce: 'pre',
buildStart() {
gate.reset();
},
onLog(_level, log) {
gate.recordLog(log.message);
},
buildEnd(error) {
if (!error) gate.assertClean();
},
};
}
// More info at: https://storybook.js.org/docs/next/writing-tests/integrations/vitest-addon
export default defineConfig(({ command }) => {
const isDevServer = command === "serve" && !process.env.VITEST;
@ -46,7 +98,13 @@ export default defineConfig(({ command }) => {
// resolve it from core/, reviving the benign `UNRESOLVED_IMPORT` warnings
// that #1491 eliminated at the source (by removing the old stream filter and
// build-time onwarn suppressions rather than re-hiding the symptom).
plugins: [react(), honoMiddlewarePlugin(buildWebServerConfigFromEnv())],
// `browserExternalizedBuiltinGate` fails `vite build` if a Node built-in
// reaches the browser bundle (#1769) — see its definition above.
plugins: [
react(),
honoMiddlewarePlugin(buildWebServerConfigFromEnv()),
browserExternalizedBuiltinGate(),
],
// Shared optimizeDeps exclusions so node-only packages
// (`@modelcontextprotocol/client/stdio`, `cross-spawn`, `which`)
// consumed by the dev backend aren't scanned for browser pre-bundling.

3
package.json
View file

@ -37,8 +37,9 @@
"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 smoke && npm run ci:storybook",
"ci": "npm run validate && npm run coverage && npm run verify:build-gate && npm run smoke && npm run ci:storybook",
"ci:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook",
"verify:build-gate": "node scripts/verify-build-gate.mjs",
"validate": "npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher",
"validate:core": "npm run format:check:core && npm run lint:core",
"lint:core": "eslint \"core/**/*.{ts,tsx}\"",

350
scripts/verify-build-gate.mjs Normal file
View file

@ -0,0 +1,350 @@
#!/usr/bin/env node
/**
* Verifies the browser-externalized-builtin build gate (#1769).
*
* The gate (clients/web/vite.config.ts +
* clients/web/server/browser-externalized-builtin-gate.ts) fails `vite build`
* when a Node built-in reaches the browser graph — Vite 8 otherwise only warns
* and ships a `{}` stub, so the broken bundle builds green. The unit tests cover
* the detection logic against a *captured* message string; only a real build can
* prove the gate still fires against the *live* Vite version, catching the one
* risk the issue calls out: the warning phrasing drifts across Vite releases
* (8.0.x → 8.1.x), silently disabling a message-keyed gate.
*
* This temporarily injects a `node:fs` import into the browser entry
* (src/main.tsx), runs `vite build`, and asserts the build FAILS with the #1769
* error, then restores the entry. Run from `npm run ci`.
*
* Why the REAL config + entry (and not a fast throwaway temp entry / generated
* config): building the actual `clients/web` config is what catches config-level
* regressions — the plugin being deleted from the `plugins` array, its
* `applyToEnvironment` no longer matching the browser environment's name, or a
* `build.rollupOptions.onwarn` suppression added above it. A temp config would
* keep passing through all of those, degrading this from "the gate works in this
* repo" to "the gate's string still matches the live Vite." That fidelity is the
* point, and it's why this accepts a full (~minute) app build and a
* source-mutation-with-restore rather than something cheaper. Do NOT "optimize"
* it into a temp entry — that silently loses the config-regression coverage.
*/
import { spawnSync } from "node:child_process";
import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { fileURLToPath } from "node:url";
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
const webDir = path.join(repoRoot, "clients/web");
// Hardcoded browser entry. If it's ever renamed, the guarded read below fails
// with an actionable message rather than a raw ENOENT stack.
const entryPath = path.join(webDir, "src/main.tsx");
const gatePath = path.join(
webDir,
"server/browser-externalized-builtin-gate.ts",
);
// The two literals this .mjs mirrors from the gate module (it can't import the
// TS source). KNOWN_PHRASE tells apart the ways a passing build can mean the gate
// broke (see the diagnoses below); ERROR_PREFIX is the success key — the
// distinctive lead of the gate's thrown error, so an unrelated build error whose
// output merely mentions "#1769" (it's in vite.config.ts comments) can't report
// OK. The drift guard below keeps BOTH honest against the source.
const KNOWN_PHRASE = "has been externalized for browser compatibility";
const ERROR_PREFIX = "Build failed (#1769)";
// The Node built-in the probe imports. Single source of truth: the diagnosis
// below tests the output for this exact name, so deriving both from one constant
// keeps a phrasing-drift failure from being misrouted as "probe never reached
// the graph" if the probe module ever changes.
const PROBE_MODULE = "node:fs";
// A namespace import + guarded use so the built-in isn't tree-shaken before Vite
// externalizes it (a bare side-effect import can be dropped). `__never__` is
// never truthy, so the reference survives to build time without running.
// Appended (not prepended): ES imports hoist, so this still externalizes at
// resolve time, and appending won't demote a leading directive (e.g. a future
// `"use client"`) the way prepending would.
const PROBE =
`\nimport * as __nodeBuiltinProbe from "${PROBE_MODULE}";\n` +
"if (globalThis.__never__) console.log(__nodeBuiltinProbe);\n";
function fail(message, detail) {
console.error(`verify:build-gate FAILED — ${message}`);
if (detail) console.error(detail);
process.exit(1);
}
// Escape a literal for safe interpolation into a RegExp.
function escapeRegExp(literal) {
return literal.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}
// Write the captured original to a backup and fail — the honest remedy when the
// in-place restore can't be trusted, since it preserves any uncommitted edits
// the developer had (unlike `git checkout --`). The backup goes in a fresh
// `mkdtempSync` dir, NOT next to the entry (a sidecar inside `src/` is untracked
// and a recovery-time `git add -A` would commit it) and NOT a fixed tmp filename
// (a pre-existing/foreign-owned file would make us point the user at someone
// else's content). The message only claims a backup when the write succeeded —
// this path runs during recovery, so it must not misdirect.
function saveBackupAndFail(reason) {
let savedTo;
try {
const dir = mkdtempSync(path.join(tmpdir(), "verify-build-gate-"));
savedTo = path.join(dir, path.basename(entryPath));
writeFileSync(savedTo, original);
} catch {
savedTo = undefined;
}
const remedy = savedTo
? `pre-run contents were saved to ${savedTo}; restore from there ` +
`(it preserves uncommitted edits, unlike 'git checkout --')`
: `a backup could NOT be written — the entry still has the probe injected; ` +
`restore it from version control (this discards uncommitted edits to it) ` +
`once the filesystem is writable`;
fail(`${reason} — ${remedy}`);
}
let original;
try {
original = readFileSync(entryPath, "utf8");
} catch (err) {
fail(
`could not read the browser entry ${path.relative(repoRoot, entryPath)} ` +
`(${err.message}) — if it was renamed, update entryPath in this script`,
);
}
// Fail fast if either mirrored literal drifted from the source of truth: a stale
// KNOWN_PHRASE would make the three-way diagnosis below misreport a
// plugin-not-applying regression as a phrasing drift, and a stale ERROR_PREFIX
// would make a correctly-firing gate report "the build broke for another
// reason" (both exactly the misdirection this script exists to avoid). Anchor
// the phrase on the *assignment* — matching the whole file would let the old
// wording lingering in a comment mask a changed constant; the prefix is
// distinctive enough that a plain substring is fine.
let gateSource;
try {
gateSource = readFileSync(gatePath, "utf8");
} catch (err) {
fail(
`could not read the gate module ${path.relative(repoRoot, gatePath)} ` +
`(${err.message}) — if it moved, update gatePath in this script`,
);
}
const gateRel = path.relative(repoRoot, gatePath);
const phraseAssignment = new RegExp(
`BROWSER_EXTERNALIZED_BUILTIN_PHRASE\\s*=\\s*["'\`]${escapeRegExp(KNOWN_PHRASE)}["'\`]`,
);
if (!phraseAssignment.test(gateSource)) {
fail(
`KNOWN_PHRASE here no longer matches the BROWSER_EXTERNALIZED_BUILTIN_PHRASE ` +
`assignment in ${gateRel} — the mirrored literals drifted; re-sync them.`,
);
}
if (!gateSource.includes(ERROR_PREFIX)) {
fail(
`ERROR_PREFIX here ("${ERROR_PREFIX}") no longer appears in ${gateRel} — the ` +
`gate's thrown error was reworded; re-sync ERROR_PREFIX with it.`,
);
}
let restored = false;
// Restore the entry in place. On failure the sidecar-`.bak` path is the reachable
// safety net — letting the write escape the `finally` would skip it and leave the
// probe injected in the entry (the worst outcome this script can produce).
function restoreEntry() {
if (restored) return;
try {
writeFileSync(entryPath, original);
restored = true;
} catch (err) {
saveBackupAndFail(
`failed to restore ${path.relative(repoRoot, entryPath)} (${err.message})`,
);
}
}
// A `finally` doesn't run on Ctrl-C during the multi-minute build; restore the
// mutated entry on a signal too so an interrupt never leaves the tree dirty.
// While `spawnSync` blocks, a Ctrl-C reaches `vite` via the shared process
// group (the child dies, `spawnSync` returns, the `finally` restores) and these
// handlers run afterward as a backstop — e.g. for a `kill <pid>` that targets
// only this process, where the queued handler is the sole restore path.
for (const signal of ["SIGINT", "SIGTERM"]) {
process.on(signal, () => {
restoreEntry();
// Conventional 128 + signal number: SIGINT → 130, SIGTERM → 143.
process.exit(signal === "SIGINT" ? 130 : 143);
});
}
// Mutate the entry (guarded), THEN wrap only the build in the restore-`finally`
// — so a write failure fails actionably before the build, and any partial write
// is rolled back rather than escaping as a raw stack. `writeFileSync`'s default
// `'w'` flag truncates at open, so a mid-write ENOSPC/EIO can leave the entry
// empty or half-written; `restoreEntry()` puts the captured original back (and
// itself falls through to the `.bak` net if it can't write).
try {
writeFileSync(entryPath, original + PROBE);
} catch (err) {
// Roll back a possible partial write ('w' truncates at open), but inline —
// NOT via restoreEntry(), whose own saveBackupAndFail→exit on a failing
// rollback would swallow this primary write error (the actual diagnosis) and,
// on a read-only checkout where nothing was ever written, misdirect the
// developer to a /tmp backup of an untouched file.
let rolledBack = false;
try {
writeFileSync(entryPath, original);
restored = rolledBack = true;
} catch {
// Reported via the damage check below.
}
// Only warn of damage when the entry actually differs from the original — on a
// read-only checkout (EACCES/EROFS, the likeliest trigger) both writes fail at
// open so the file is untouched, and "restore from version control" would
// needlessly discard uncommitted edits. Read failing → assume the worst.
let damaged = false;
if (!rolledBack) {
try {
damaged = readFileSync(entryPath, "utf8") !== original;
} catch {
damaged = true;
}
}
fail(
`could not write the probe into ${path.relative(repoRoot, entryPath)} (${err.message})` +
(damaged
? " — the entry is truncated or partially written; restore it from version control"
: ""),
);
}
let result;
try {
console.log(
"verify:build-gate: running a real `vite build` with a node:fs probe (takes a minute)…",
);
// `--no-install` pins to the locally installed (repo-pinned) Vite: the whole
// point is proving the message-keyed gate fires against THIS Vite, so `npx`
// must never silently fetch a different version from the registry when
// clients/web/node_modules is missing/partial. A missing local bin then
// surfaces via the `result.error` check below. `timeout` bounds a hung build:
// spawnSync sets `result.error` (ETIMEDOUT) on timeout, so the same branch
// reports it — otherwise a hang would burn to the GitHub job's 360-min default
// with no output (this step captures rather than inherits stdio).
result = spawnSync("npx", ["--no-install", "vite", "build"], {
cwd: webDir,
encoding: "utf8",
timeout: 10 * 60_000,
killSignal: "SIGKILL",
});
} finally {
// Always restore the entry, even if the build spawn threw.
restoreEntry();
}
// Guard against a botched restore that wrote *something* other than the original
// (distinct from restoreEntry's write throwing, which it handles itself). The
// verifying read is itself routed through saveBackupAndFail so the restore path
// is uniformly safe — a raw throw here would skip the backup net.
let afterRestore;
try {
afterRestore = readFileSync(entryPath, "utf8");
} catch (err) {
saveBackupAndFail(
`could not re-read ${path.relative(repoRoot, entryPath)} to verify the restore (${err.message})`,
);
}
if (afterRestore !== original) {
saveBackupAndFail(
`failed to restore ${path.relative(repoRoot, entryPath)} (contents differ)`,
);
}
// A spawn failure (e.g. `npx` missing) leaves `status` null with no output —
// surface it as itself rather than falling through to the "not via the gate"
// diagnosis, which would send someone chasing a build regression that isn't real.
if (result.error) {
fail(`could not run \`vite build\` (${result.error.message})`);
}
const output = `${result.stdout ?? ""}\n${result.stderr ?? ""}`;
if (result.status === 0) {
// A passing build with a Node built-in in the browser graph means the gate
// broke — but in distinct ways, each pointing at a different file. The
// captured output distinguishes them (Vite prints the warning at the default
// log level, and no build script passes `--logLevel`, so its presence is
// reliable). All three paths completed the build, so `clients/web/dist` now
// holds a probe bundle — harmless (the probe never runs) and overwritten by
// the next `validate`/`build`; flagged once here so a local debugger doesn't
// serve it via `npm run web` unaware.
console.error(
"verify:build-gate: note — clients/web/dist now holds a probe build; run a normal build before serving it.",
);
// Order matters: a phrase match implies the probe reached the graph, so it
// must be checked before the module-name fallback — don't reorder these.
if (output.includes(KNOWN_PHRASE)) {
fail(
"vite build SUCCEEDED but Vite DID emit the externalization warning — the " +
"gate plugin isn't applying. In clients/web/vite.config.ts the plugin may " +
"have been removed from `plugins`, its `applyToEnvironment` may no longer " +
"match the browser environment's name, a `build.rollupOptions.onwarn` " +
"suppression was added above it, or a future Vite emitted the warning " +
"before the client environment's `buildStart` reset (which then cleared it).",
output,
);
}
if (output.includes(PROBE_MODULE)) {
fail(
"vite build SUCCEEDED and the warning phrasing drifted — the probe reached " +
`the graph (${PROBE_MODULE} is named) but the known phrase is absent. Update ` +
"BOTH BROWSER_EXTERNALIZED_BUILTIN_PHRASE in browser-externalized-builtin-gate.ts " +
"AND KNOWN_PHRASE in this script to the new Vite wording (the drift guard " +
"requires they stay in sync).",
output,
);
}
fail(
"vite build SUCCEEDED and the probe never reached the browser graph (neither " +
`the known phrase nor ${PROBE_MODULE} appears in the output) — the entry may ` +
"have moved or the probe was tree-shaken. Check this script's PROBE / entryPath.",
output,
);
}
// Assert on the gate's distinctive thrown-error prefix, NOT a bare "#1769":
// that issue number also appears in clients/web/vite.config.ts comments, so a
// code frame from an unrelated build error there could otherwise make this
// report OK with the gate dead. ERROR_PREFIX + KNOWN_PHRASE are the script's
// contract with the gate module, and both are drift-guarded above.
if (!output.includes(ERROR_PREFIX)) {
fail(
"vite build failed, but not via the #1769 gate — the build broke for another " +
"reason, so this check no longer proves the gate works.",
output,
);
}
// The gate fired — but confirm it fired on OUR probe, not a pre-existing leak.
// The gate's error embeds every original Vite warning, and those name the module,
// so PROBE_MODULE must appear. Without this, a repo that already leaks a built-in
// would report OK even if the probe was tree-shaken or entryPath went stale —
// branch (c)'s failure, silently inverted into a pass. (Unreachable inside
// `npm run ci`: `validate`'s `build:web` fails first on a pre-existing leak — but
// this script is a documented standalone command, run exactly when debugging one.)
if (!output.includes(PROBE_MODULE)) {
fail(
`the #1769 gate fired, but ${PROBE_MODULE} isn't among the offenders — it ` +
`tripped on a pre-existing leak, so this run doesn't prove the probe reached ` +
`the graph (was it tree-shaken, or has entryPath gone stale?).`,
output,
);
}
console.log(
"verify:build-gate OK — vite build fails on a Node built-in in the browser graph (#1769 gate fired).",
);
process.exit(0);