Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,11 @@ jobs:
# running the integration suite twice.
run: npm run coverage

- name: Run cross-client smokes
# 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
# (smoke:web builds clients/web/dist on demand — #1486). smoke:tui
# self-skips here — the Ink TUI needs a real TTY (raw mode) that headless
# CI lacks, so its boot/render check is local-only.
run: npm run smoke

# 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
# smoke reuse the cache instead of downloading its own copy. The Storybook
# step below reuses the same install.
- name: Cache Playwright browsers
uses: actions/cache@v4
with:
Expand All @@ -72,6 +68,16 @@ jobs:
working-directory: ./clients/web
run: npx playwright install --with-deps chromium

- name: Run cross-client smokes
# 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
# (smoke:web builds clients/web/dist on demand — #1486). smoke:web:browser
# boots the prod web bundle in headless chromium (#1615). smoke:tui
# self-skips here — the Ink TUI needs a real TTY (raw mode) that headless
# CI lacks, so its boot/render check is local-only.
run: npm run smoke

- name: Run Storybook play-function tests
working-directory: ./clients/web
run: npm run test:storybook
Expand Down
5 changes: 3 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,10 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i
- **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`, 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`. 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 `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: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`.
- `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).
- `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.

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ 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). |
| `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 pack:verify` | Publish smoke — see [Publishing](#publishing). |

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@
"coverage:tui": "cd clients/tui && npm run test:coverage",
"coverage:web": "cd clients/web && npm run test:coverage",
"coverage:launcher": "cd clients/launcher && npm run test:coverage",
"smoke": "npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web",
"smoke": "npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web && npm run smoke:web:browser",
"smoke:cli": "node scripts/smoke-cli.mjs",
"smoke:tui": "node scripts/smoke-tui.mjs",
"smoke:web": "node scripts/smoke-web.mjs",
"smoke:web:browser": "cd clients/web && npx playwright install chromium && node ../../scripts/smoke-web-browser.mjs",
"smoke:launcher": "node scripts/smoke-launcher.mjs",
"pack:verify": "node scripts/pack-and-verify.mjs",
"prepack": "npm run build",
Expand Down
128 changes: 128 additions & 0 deletions scripts/lib/prod-web-server.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/**
* Shared boot/readiness helper for the prod web smokes.
*
* Both `scripts/smoke-web.mjs` (serves-the-HTML check) and
* `scripts/smoke-web-browser.mjs` (runs-the-bundle check, #1615) boot the *same*
* prod `mcp-inspector --web` server, so the spawn + readiness-poll boilerplate
* lives here once instead of being copy-pasted (and drifting) in each script.
*
* Repo-root paths are derived from import.meta.url, so a caller's cwd (e.g.
* `smoke:web:browser` does `cd clients/web` first so its `npx playwright
* install` finds the local bin) doesn't affect which launcher/build tree runs.
*/

import { spawn } from "node:child_process";
import { setTimeout as delay } from "node:timers/promises";
import { fileURLToPath } from "node:url";
import { dirname, resolve } from "node:path";

const libDir = dirname(fileURLToPath(import.meta.url));
const repoRoot = resolve(libDir, "..", "..");
const launcherEntry = resolve(repoRoot, "clients/launcher/build/index.js");

/**
* Spawn `mcp-inspector --web` (prod, no `--dev`) against the built
* `clients/web/dist` and return handles for readiness + teardown.
*
* @param {object} opts
* @param {string} opts.host
* @param {string} opts.port
* @param {string} opts.token value injected as MCP_INSPECTOR_API_TOKEN
*/
export function startProdWebServer({ host, port, token }) {
const baseUrl = `http://${host}:${port}`;

const child = spawn(process.execPath, [launcherEntry, "--web"], {
env: {
...process.env,
CLIENT_PORT: port,
HOST: host,
MCP_INSPECTOR_API_TOKEN: token,
// Don't pop a browser in CI.
MCP_AUTO_OPEN_ENABLED: "false",
},
stdio: ["ignore", "inherit", "inherit"],
});

let exited = false;
let exitCode = null;
let childError = null;
child.on("exit", (code) => {
exited = true;
exitCode = code;
});
// A child 'error' event covers a spawn failure (e.g. a missing launcher
// entry) but ALSO a failed kill / failed send where the process is still
// alive. Without a listener Node throws it uncaught with a raw stack instead
// of the smoke's `… FAILED —` line. Record it, but deliberately do NOT set
// `exited` — otherwise `stop()` (which guards on `exited`) would skip the
// SIGTERM and orphan a still-running launcher on the port.
child.on("error", (err) => {
childError = err;
});

// Boot has failed if the process exited or emitted an error.
const bootFailed = () => exited || childError !== null;

function bootFailure(phase) {
return new Error(
childError
? `launcher process error: ${childError.message}`
: `launcher exited (code ${exitCode}) ${phase} — see output above`,
);
}

/**
* Poll `GET /` until the server answers with an ok status. Keeps polling on a
* non-ok status (a warming server may legitimately answer 503), but records
* the last one so a server that boots yet never returns ok (e.g. a broken
* `dist` answering 500) reports that status instead of a bare timeout. Rejects
* early if the launcher exits before serving. Resolves with the first ok
* Response.
*/
async function waitForReady({ attempts = 120, intervalMs = 500 } = {}) {
let lastStatus = null;
for (let attempt = 0; attempt < attempts; attempt++) {
if (bootFailed()) throw bootFailure("before serving");
try {
const res = await fetch(`${baseUrl}/`);
if (res.ok) return res;
lastStatus = res.status;
} catch {
// not listening yet
}
await delay(intervalMs);
}
throw new Error(
`server did not start within ${(attempts * intervalMs) / 1000}s${
lastStatus !== null ? ` (last response: HTTP ${lastStatus})` : ""
}`,
);
}

/**
* A promise that rejects when the launcher process dies — race it against
* page-load work so a mid-flight server death is reported as the real cause
* instead of a downstream render timeout. Never resolves; if the server stays
* up it simply stays pending until the process exits.
*/
function whenChildExits() {
return new Promise((_resolve, reject) => {
const onDeath = () => reject(bootFailure("mid-run"));
if (bootFailed()) onDeath();
else {
child.once("exit", onDeath);
child.once("error", onDeath);
}
});
}

return {
baseUrl,
waitForReady,
whenChildExits,
stop: () => {
if (!exited) child.kill("SIGTERM");
},
};
}
Loading
Loading