diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 71e9ec4c9..d1763cbae 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -93,9 +93,11 @@ jobs: # 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. + # boots the prod web bundle in headless chromium (#1615); smoke:web:app + # goes further and drives connect → open app → widget ready against a + # composable MCP App server (#1859). Both reuse the chromium installed + # above. 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 diff --git a/AGENTS.md b/AGENTS.md index b86db3801..a4185baaf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -454,10 +454,11 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab - **The `__tests__` dirs are typechecked too (#1791).** The src-only `tsconfig.json` excludes `**/*.test.*`, so each of cli, tui, and launcher carries a **`tsconfig.test.json`** — extending the build config, `noEmit`, including `__tests__/**/*` (only the tests root the project; tsc pulls in the `src` they import, and the src-only config already validates all of `src` without the test-only aliases) and adding the test-only path aliases that resolve what vitest resolves via `vitest.shared.mts`. The alias set differs per client: **cli's is the widest** (`@modelcontextprotocol/inspector-test-server` → `test-servers/src`, the `@inspector/core/*` deep paths, express/vitest — cli is the only one importing the test-server package); **tui's** carries only the `@inspector/core/*` + react/vitest redirects; **launcher's** has **no** `paths` at all — it's a plain `rootDir: "."` sibling of the build config (whose `rootDir: ./src` is what rejects the tests). Each client's `typecheck` script runs **both** projects (`tsc -p tsconfig.json && tsc -p tsconfig.test.json`) so running it standalone means the same thing everywhere (launcher's `build` also `tsc`s `src`, but `typecheck` doesn't rely on that). cli additionally carries `@types/express` (devDep) so the transitively-aliased test-server source typechecks, mirroring `clients/web` (cli's `tsconfig.test.json` also names `test-servers/src/server-composable.ts` explicitly — a bin entry the barrel doesn't import, so nothing else gives it a tsc pass). The client **config files** are typechecked too: cli's/tui's (`vitest.config.ts`, `tsup.config.ts`, tui `dev.ts`) are folded into each src `tsconfig.json`'s `include`; launcher's `vitest.config.ts` goes in its `tsconfig.test.json` instead (again the `rootDir: ./src` reason). Note the gate checks mock **implementations and return types** (typing a `vi.fn()` against a real signature keeps its `mockResolvedValue`/impl in sync) but **not** `toHaveBeenCalledWith(...)` arguments — vitest types those to accept anything regardless of the mock's type parameter. **`npm run verify:typecheck-coverage`** (`scripts/verify-typecheck-coverage.mjs`, run as the second step of `validate` right after `verify:format-coverage`) is the durable guard for this invariant: it runs each client's `typecheck` projects with `tsc --listFilesOnly`, unions them, and fails on any tracked `.ts`/`.tsx`/`.mts`/`.cts` that lands in no project — for every gated Node client, which it discovers from disk (each `clients/*` is enrolled through its `typecheck` script's projects, or — for a `tsc -b` client like `clients/web` with no `typecheck` script — through its `tsconfig.json` `references`), so a new client is covered without editing the guard — the typecheck analog of `verify:format-coverage`, since a project only reaches the files its `include` names plus their transitive imports, so a new top-level file (launcher especially, whose build `rootDir: ./src` rejects package-root files) can otherwise fall out silently. Like its sibling it also asserts the gate is *wired* (each client's typecheck pass is reachable from its `validate` — its `typecheck` script for cli/tui/launcher, or a real `tsc -b` for web — and the root chain runs each client's `validate`), so it can't stay green while measuring a pass nothing invokes. It asserts the same of **`test:scripts`** — its own parser tests — on three axes: reachable from the root `validate`, a **non-empty** tracked `scripts/**/*.{test,spec}.*` set, and **every one of those files matched by a glob harvested across the scripts reachable from `test:scripts`** (so a delegating `test:scripts` still measures correctly). The third axis exists because `node --test` silently *skips* a file its glob misses and still exits 0 — a rename to `*.spec.mjs` would shrink the suite with a green run. Beyond the clients it also covers, **deny-by-default**, the first-party TS no client owns — everything tracked outside `clients/*` (`test-servers/src/**`, the root `vitest.shared.mts`, **all of `core/`**, and any new top-level TS location) must land in the *global* union of client projects (cli aliases the test-server source; web's enrolled projects include `core/`). So a `core` `*.tsx` web's `include` doesn't reach, or an unimported `test-servers/src` bin entry, can't ship uncompiled-but-unchecked. The one "listed but unchecked" tier the guard structurally can't see — a per-file `// @ts-nocheck` — is owned by a different gate: `@typescript-eslint/ban-ts-comment` rejects it across every surface (`lint:core`, `lint:shared`, and each client's `eslint .`). The guard's own pure parsers (`scripts/lib/npm-scripts.mjs` + the exported helpers of `verify-typecheck-coverage.mjs`, whose execution is behind a `main()` so importing it for tests doesn't run it) are **unit-tested** — `npm run test:scripts` (node's built-in `node --test`, in `validate`; the root has no vitest harness by design) runs table-driven cases, one per rule the guard's parsers encode, and the guard itself enforces that this stays wired (above). - 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`. (#1778, #1789, #1792) `clients/web`'s `format`/`format:check` covers `src`, `server`, `.storybook`, and its top-level configs (the uniform `*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` glob — `vite.config.ts`, `tsup.runner.config.ts`, `eslint.config.js`, …), not just `src`, so the Node backend, Storybook config, and Vite/build config are prettier-gated too; `clients/launcher`'s covers `src`, `__tests__`, `scripts`, and its top-level configs (the `*.` top-level glob is non-recursive, so each nested dir — `.storybook`, `scripts` — is named explicitly). The `verify:format-coverage` guard (#1792) enforces that this coverage stays complete. - **`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 `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` 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` / `smoke:web:app`, 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). +- `smoke:web:app` (`scripts/smoke-web-app.mjs`, #1859) goes one step further again: `smoke:web:browser` stops at first paint and never connects to a server, so the Apps tab, the sandbox controller, and the UI-protocol bridge were unexercised by any smoke. This one boots the same prod `--web` server, spawns the `mcp-app-http.json` composable test server (the `mcp_app_demo` tool + its `mcp_app_demo_widget` UI resource), and drives the whole **connect → open app → widget ready** chain through a single deep-link navigate (`?serverUrl=…&autoConnect=&openApp=…&appArgs=…&autoOpen=`). The assertion is the `data-app-status="ready"` contract from [clients/web/README.md](clients/web/README.md) — the renderer reports `ready` only once the widget has loaded inside the sandbox iframe *and* fired `notifications/initialized` back through the bridge, so one attribute covers the sandbox proxy being served, the UI resource loading, and the handshake completing. Two mechanics are load-bearing and easy to get wrong: the test server announces readiness on **stderr** (`console.error` in `server-composable.ts`), so both child streams are piped and scanned — watching stdout alone times out with an empty diagnostic; and its bound port is **not** the config's, because `createTestServerHttp` resolves through `findAvailablePort()`, which walks upward when the configured port is taken — so the smoke parses the announced URL rather than assuming `3130`. **Scope note:** this runs against the repo build tree like every other smoke, so it would *not* have caught #1859 itself (a packaging failure — the file is always present in-repo); `pack:verify` owns that dimension. It does carry a cheap structural pre-check that the proxy page exists at the path `sandbox-controller.ts` resolves, so a move/rename fails fast with a clear cause instead of an opaque render timeout. - **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 ` 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. diff --git a/README.md b/README.md index a597b665e..137927642 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | Config | Demonstrates | Issue | | ----------------------------------------- | -------------------------------------------------- | ---------------------------------------------------------------------- | +| `mcp-app-http.json` **(legacy era)** | An MCP App (UI resource + app tool) in the Apps tab | [#1859](https://github.com/modelcontextprotocol/inspector/issues/1859) | | `modern-mrtr-http.json` | A single MRTR round-trip | — | | `mrtr-showcase-http.json` | Every MRTR preset in one server | — | | `modern-network-http.json` | Network tab: `Mcp-*` headers + error taxonomy | [#1628](https://github.com/modelcontextprotocol/inspector/issues/1628) | @@ -142,6 +143,14 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) | | `tasks-{legacy,modern}-http.json` | Tasks, both eras | [#1631](https://github.com/modelcontextprotocol/inspector/issues/1631) | +#### MCP Apps + +`mcp-app-http.json` serves the `mcp_app_demo` tool (`_meta.ui.resourceUri`) alongside its `mcp_app_demo_widget` UI resource, so the **Apps** tab has a real App to render. It is a plain streamable-HTTP server — connect with the **default (legacy)** protocol era, not Modern. + +Open the Apps tab, select `mcp_app_demo`, give it a title and click **Open App**: the widget renders inside the sandbox iframe and exercises the host-side UI protocol surface — host-context render, `size-changed`, `ui/message`, and a log line into the **App logs** panel. Because the widget is served through the sandbox proxy page, this config is also what reproduces [#1859](https://github.com/modelcontextprotocol/inspector/issues/1859) (a missing `clients/web/static/sandbox_proxy.html` surfaces here as a "Sandbox not loaded" message in place of the widget) — a failure that only ever appeared in an installed package, never in the repo. + +For the scripted version of the same flow (`--app-info` probe → deep link → rendered widget), see [Reviewing an MCP App](./docs/mcp-app-review.md). + #### MRTR `modern-mrtr-http.json` serves the `mrtr_confirm` tool (preset `mrtr_confirm`, `createMrtrTool`) over the modern leg. Its handler returns `inputRequired(...)` embedding a form elicitation, so invoking it produces a real round-trip: `input_required` → the client fulfils the embedded elicitation and retries with a new id → `complete`. @@ -256,7 +265,7 @@ Each client self-validates from its own folder; the root scripts chain them. The | --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `npm run validate` | Runs `verify:format-coverage` (asserts every tracked source file is format-gated) first, then `validate:core` (the shared `core/` `format:check` + `lint` gate), 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 smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus two headless-Chromium 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. | | `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: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` + the exported helpers of `verify-typecheck-coverage.mjs`), one case per rule they encode. 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. | @@ -278,6 +287,7 @@ The root `package.json` `"files"` allowlist is the source of truth for the tarba - **No source maps.** The client bundlers set `sourcemap: false` (`clients/{cli,tui}/tsup.config.ts`, `clients/web/tsup.runner.config.ts`); Vite and the launcher's `tsc` already emit none. Maps are ~half the unpacked size and aren't needed at runtime — debug via `npm run dev` on the source. - **`clients/web/build` ships via `clients/web/.npmignore`.** `clients/web/.gitignore` lists `build/`, and npm's packlist honors that nested `.gitignore` over the root `"files"` allowlist — so the prod web-server runner was silently missing from the tarball while `clients/web/dist` slipped through (its `.gitignore` only lists `dist-ssr`). `clients/web/.npmignore` overrides the `.gitignore` for publishing so both `build/` (runner) and `dist/` (SPA) ship. The other clients don't need this — none ship a nested `.gitignore`. +- **`clients/web/static` ships the MCP Apps sandbox proxy.** `clients/web/static/sandbox_proxy.html` is a committed source file (not a build artifact), read from disk at runtime by `clients/web/server/sandbox-controller.ts` as `/../static/sandbox_proxy.html`. It was missing from the root `"files"` allowlist entirely, so every published build failed the Apps tab with **"Sandbox not loaded"** ([#1859](https://github.com/modelcontextprotocol/inspector/issues/1859)) while working fine in the repo. Because the path is resolved *relative to* `clients/web/build`, the directory must ship at that exact location — `pack:verify` asserts both the tarball entry and the installed-on-disk path. - **A single version number, read from the root `package.json`.** The Inspector ships as one package with one version, so only the **root** `package.json` carries a `version` — the four `clients/*/package.json`s deliberately have none. Every Node client (CLI, TUI, and the web backend) resolves the version through the shared `readInspectorVersion()` reader in `core/node/version.ts`, which walks up to the root manifest (always present in the tarball). No client `package.json` is read at runtime, so none needs to ship. The web **browser** can't read the filesystem; it gets its version from the backend via `GET /api/config` (see [#1639](https://github.com/modelcontextprotocol/inspector/issues/1639)). ### `npm run pack:verify` — publish smoke against the real tarball diff --git a/package.json b/package.json index d9472940e..d3467cf6d 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "clients/launcher/build", "clients/web/build", "clients/web/dist", + "clients/web/static", "clients/cli/build", "clients/tui/build", "scripts/install-clients.mjs" @@ -63,11 +64,12 @@ "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 && npm run smoke:web:browser", + "smoke": "npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web && npm run smoke:web:browser && npm run smoke:web:app", "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:web:app": "cd clients/web && npx playwright install chromium && node ../../scripts/smoke-web-app.mjs", "smoke:launcher": "node scripts/smoke-launcher.mjs", "pack:verify": "node scripts/pack-and-verify.mjs", "prepack": "npm run build", diff --git a/scripts/pack-and-verify.mjs b/scripts/pack-and-verify.mjs index c19bbfe4d..ff1dd310e 100644 --- a/scripts/pack-and-verify.mjs +++ b/scripts/pack-and-verify.mjs @@ -22,8 +22,8 @@ * * 1. builds every client (`npm run build`); * 2. packs the publishable tarball (`npm pack`) and inspects its file list — - * asserting NO source maps ship and that `clients/web/{build,dist}` are - * both present (the two packaging fixes this work landed); + * asserting NO source maps ship and that `clients/web/{build,dist,static}` + * are all present (the packaging fixes this work landed); * 3. installs that tarball into a fresh temp dir (real `npm install `, * which runs the package's `postinstall`); * 4. runs the installed `mcp-inspector` bin: `--help`, `--cli`/`--tui` help @@ -164,26 +164,31 @@ if (maps.length > 0) { } // 2b. Runtime files that are easy to omit from the packlist and only fail once -// installed: both web artifacts — the prod server runner (build) AND the SPA -// (dist). `clients/web/build` was previously dropped by the nested -// .gitignore. (The version the CLI/TUI report is read from the root -// package.json — always shipped — via readInspectorVersion(), so no client -// package.json needs to ship; that read is exercised by driving the bin in -// step 4.) +// installed: the web artifacts — the prod server runner (build), the SPA +// (dist), and the MCP Apps sandbox proxy page (static). `clients/web/build` +// was previously dropped by the nested .gitignore; `clients/web/static` was +// never listed in the root "files" allowlist at all, so the Apps tab failed +// with "Sandbox not loaded" on every published build (#1859). None of these +// are checked-in-tree failures — only an installed tarball reveals them. +// (The version the CLI/TUI report is read from the root package.json — +// always shipped — via readInspectorVersion(), so no client package.json +// needs to ship; that read is exercised by driving the bin in step 4.) for (const required of [ "clients/web/build/index.js", "clients/web/dist/index.html", + "clients/web/static/sandbox_proxy.html", ]) { if (!tarredPaths.includes(required)) { fail( `expected \`${required}\` in the published tarball but it is missing — ` + - `check the "files" field in clients/web/package.json`, + `check the "files" field in the root package.json (and that ` + + `clients/web/.npmignore does not exclude it)`, ); } } console.log( `pack:verify — tarball OK: ${tarredPaths.length} files, no source maps, ` + - `clients/web/{build,dist} present (${(packInfo.unpackedSize / 1048576).toFixed(2)} MB unpacked)`, + `clients/web/{build,dist,static} present (${(packInfo.unpackedSize / 1048576).toFixed(2)} MB unpacked)`, ); // --------------------------------------------------------------------------- @@ -229,10 +234,14 @@ try { if (!existsSync(bin)) { fail(`installed \`mcp-inspector\` bin not found at ${bin}`); } - // Confirm the two packaging fixes survived install onto disk. + // Confirm the packaging fixes survived install onto disk. The sandbox proxy + // is resolved at runtime as `/../static/sandbox_proxy.html`, so its + // position *relative to* clients/web/build is what matters, not just presence + // in the tarball (#1859). for (const required of [ join(installedPkg, "clients", "web", "build", "index.js"), join(installedPkg, "clients", "web", "dist", "index.html"), + join(installedPkg, "clients", "web", "static", "sandbox_proxy.html"), join(installedPkg, "clients", "launcher", "build", "index.js"), ]) { if (!existsSync(required)) { diff --git a/scripts/smoke-web-app.mjs b/scripts/smoke-web-app.mjs new file mode 100644 index 000000000..73ca5f03b --- /dev/null +++ b/scripts/smoke-web-app.mjs @@ -0,0 +1,353 @@ +#!/usr/bin/env node +/** + * Headless-browser MCP Apps smoke for the prod web client (#1859). + * + * `smoke:web:browser` proves the bundle boots and paints its first frame. It + * stops there — it never connects to a server, so everything downstream of the + * connect (the Apps tab, the sandbox controller, the UI-protocol bridge) is + * unexercised by any smoke. This closes that gap: it drives the full + * **connect → open app → widget ready** chain against a real MCP App server. + * + * The assertion is the `data-app-status="ready"` contract documented in + * clients/web/README.md ("MCP Apps screen automation contract"): the renderer + * only reports `ready` once the widget has loaded inside the sandbox iframe and + * fired `notifications/initialized` back through the bridge. So a single + * attribute covers the whole path — sandbox controller serving the proxy page, + * the proxy loading the UI resource, and the bridge completing its handshake. + * + * ── What this does and does NOT catch ─────────────────────────────────────── + * + * This runs against the **repo build tree**, like every other `smoke:*`. That + * matters for the bug that motivated it: #1859 was a *packaging* failure — + * `clients/web/static/sandbox_proxy.html` was missing from the published + * tarball's "files" allowlist. In the repo that file is always present, so this + * smoke would have stayed green through that entire bug. + * + * The packaging dimension is owned by `npm run pack:verify`, which asserts the + * file both in the tarball packlist and on disk after a real install. Keep both: + * pack:verify proves the file *ships*, this proves the App path *works*. Neither + * subsumes the other, and the failure this one is positioned to catch is a + * regression in the sandbox/bridge code itself — which pack:verify, driving only + * `GET /`, would not notice. + * + * As a cheap extra, this does assert the proxy page exists at the location the + * runtime resolves it from (`clients/web/build/../static/…`, see + * server/sandbox-controller.ts) — which catches the file being *moved or + * renamed* without its reader being updated, a repo-tree failure pack:verify + * would only find later. + * + * Playwright is resolved with a `createRequire` based at clients/web/package.json + * rather than a bare `import("playwright")` — a bare ESM specifier resolves + * relative to scripts/, not the cwd, so `cd clients/web` in the npm script would + * NOT make it resolvable. Same gotcha as smoke:web:browser; see its header. + * + * Expects `clients/web/dist` and `clients/launcher/build` to be built first — + * the validate / CI ordering guarantees this. `test-servers/build` is built on + * demand if missing, as in smoke:cli. + */ + +import { spawn, spawnSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { createRequire } from "node:module"; +import { setTimeout as delay } from "node:timers/promises"; +import { join, resolve } from "node:path"; +import { startProdWebServer } from "./lib/prod-web-server.mjs"; +import { stopChild } from "./lib/child-cleanup.mjs"; + +const repoRoot = resolve(import.meta.dirname, ".."); +const requireFromWeb = createRequire( + resolve(repoRoot, "clients/web/package.json"), +); + +const composableServer = join( + repoRoot, + "test-servers", + "build", + "server-composable.js", +); +const appConfig = join( + repoRoot, + "test-servers", + "configs", + "mcp-app-http.json", +); +// The path clients/web/server/sandbox-controller.ts resolves at runtime, from +// the built runner at clients/web/build/. Kept in sync with the `join(__dirname, +// "../static/sandbox_proxy.html")` there. +const sandboxProxyPage = join( + repoRoot, + "clients", + "web", + "static", + "sandbox_proxy.html", +); + +const HOST = "127.0.0.1"; +// Distinct from smoke:web (6299) and smoke:web:browser (6298) so a prior smoke +// whose port is still bound — slow teardown, TIME_WAIT, or a parallel run — +// can't EADDRINUSE this one. The three run back-to-back in `npm run smoke`. +const PORT = process.env.SMOKE_WEB_APP_PORT ?? "6297"; +const TOKEN = "smoke-web-app-token"; +const APP_TOOL = "mcp_app_demo"; +// Console messages that are the async half of the uncaught-crash class (an +// unhandled rejection or a failed dynamic import). Hard failures; every other +// console error is a diagnostic, so benign font-CDN / React-warning noise can't +// flake CI. Kept identical to smoke-web-browser.mjs, which documents the +// reasoning at length. +const FATAL_CONSOLE = /^Uncaught\b|Failed to fetch dynamically imported module/; +// The URL the test server announces on startup. NOT derived from the config's +// port: createTestServerHttp resolves its port with findAvailablePort(), which +// walks UPWARD from the configured value when it's taken — so the config port is +// a starting hint, not a guarantee, and assuming it makes this smoke fail +// whenever anything else holds that port. The announced line is authoritative. +let mcpUrl = null; + +let mcpServer = null; +let browser = null; +const server = startProdWebServer({ host: HOST, port: PORT, token: TOKEN }); + +async function shutdown() { + if (browser) { + try { + await browser.close(); + } catch { + // best-effort + } + browser = null; + } + server.stop(); + if (mcpServer) { + const child = mcpServer; + mcpServer = null; + await stopChild(child, { label: "smoke:web:app", what: "MCP test server" }); + } +} + +async function fail(message) { + console.error(`smoke:web:app FAILED — ${message}`); + await shutdown(); + process.exit(1); +} + +/** Build the composable test server bundle if it isn't present yet. */ +function ensureTestServer() { + if (existsSync(composableServer)) return; + console.log( + "smoke:web:app — building test-servers (missing build output)...", + ); + const r = spawnSync("npx", ["tsc", "-p", "test-servers", "--noCheck"], { + cwd: repoRoot, + stdio: "inherit", + }); + if (r.status !== 0 || !existsSync(composableServer)) { + throw new Error( + "could not build the test servers (test-servers/build/server-composable.js). " + + "Run `npm run test-servers:build` from clients/web.", + ); + } +} + +/** + * Spawn the MCP App test server and wait for it to announce its URL. + * + * Both stdio channels are piped and scanned: server-composable.ts announces + * readiness with `console.error`, so watching stdout alone never matches and + * this times out with an empty diagnostic. Piping both also keeps the child's + * noise out of the smoke's own output while still making it available in the + * failure message. + */ +async function startMcpServer() { + const child = spawn( + process.execPath, + [composableServer, "--config", appConfig], + { cwd: repoRoot, stdio: ["ignore", "pipe", "pipe"] }, + ); + let out = ""; + child.stdout.on("data", (d) => (out += d)); + child.stderr.on("data", (d) => (out += d)); + let exited = false; + let spawnError = null; + // A spawn failure (e.g. an unbuilt/renamed entry) emits `error`, NOT `exit` — + // and with no `error` listener Node throws it uncaught, replacing this smoke's + // diagnostic with a raw stack. `close` is listened to alongside `exit` for the + // same reason: it fires in cases `exit` does not, so a child that dies without + // an exit event can't leave the poll below spinning for the full 30s. + child.on("error", (err) => (spawnError = err)); + child.on("exit", () => (exited = true)); + child.on("close", () => (exited = true)); + + for (let attempt = 0; attempt < 120; attempt++) { + // Take the port the server actually bound, not the one we asked for. + const announced = out.match(/listening at (http:\/\/\S+)/i); + if (announced) return { child, url: announced[1] }; + if (spawnError) { + throw new Error( + `could not spawn the MCP test server (${composableServer}): ${spawnError.message}`, + ); + } + if (exited) throw new Error(`MCP test server exited early:\n${out}`); + await delay(250); + } + throw new Error(`MCP test server did not start within 30s:\n${out}`); +} + +/** base64url(JSON) — the appArgs encoding the deep link expects. */ +function encodeAppArgs(args) { + return Buffer.from(JSON.stringify(args)) + .toString("base64") + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); +} + +async function loadChromium() { + let chromium; + try { + ({ chromium } = requireFromWeb("playwright")); + } catch (err) { + // Not resolvable means devDependencies are missing — fixed by `npm install` + // at the repo root, NOT by `playwright install` (which fetches binaries). + throw new Error( + `could not resolve the Playwright package from clients/web — run \`npm install\` at the repo root (${err instanceof Error ? err.message : String(err)})`, + ); + } + try { + return await chromium.launch({ headless: true }); + } catch (err) { + throw new Error( + `chromium failed to launch — on a bare Linux box run \`npx playwright install --with-deps chromium\` for the system libraries (${err instanceof Error ? err.message : String(err)})`, + ); + } +} + +try { + // Cheap structural check first: the sandbox proxy page must exist where the + // runtime looks for it. Fails fast with a clear cause instead of surfacing as + // an opaque "app never reached ready" 30s timeout below. + if (!existsSync(sandboxProxyPage)) { + await fail( + `sandbox proxy page missing at ${sandboxProxyPage} — clients/web/server/sandbox-controller.ts ` + + `reads it as \`join(__dirname, "../static/sandbox_proxy.html")\`; if it moved, update both ` + + `(and the "files" allowlist in the root package.json, see #1859)`, + ); + } + + ensureTestServer(); + ({ child: mcpServer, url: mcpUrl } = await startMcpServer()); + await server.waitForReady(); + browser = await loadChromium(); + const page = await browser.newPage(); + + // Uncaught *synchronous* page errors. Their *async* twin — an unhandled + // rejection or a failed dynamic import — is not a `pageerror`; Chromium + // reports it on the console channel instead, so both are captured and both + // are hard failures. Same split as smoke:web:browser; see FATAL_CONSOLE there. + const pageErrors = []; + const consoleErrors = []; + page.on("pageerror", (err) => + pageErrors.push(err instanceof Error ? err.message : String(err)), + ); + page.on("console", (msg) => { + if (msg.type() === "error") consoleErrors.push(msg.text()); + }); + const fatalConsole = () => consoleErrors.filter((m) => FATAL_CONSOLE.test(m)); + + // Deep link: connect, switch to the Apps tab, pre-select the app tool, and + // fire "Open App". autoConnect/autoOpen must equal the session token (CSRF + // gate). Shape owned by clients/web/README.md#deep-link-auto-connect. + const url = + `${server.baseUrl}/?serverUrl=${encodeURIComponent(mcpUrl)}` + + `&transport=http&autoConnect=${TOKEN}&openApp=${APP_TOOL}` + + `&appArgs=${encodeAppArgs({ title: "smoke:web:app" })}&autoOpen=${TOKEN}`; + + const drive = async () => { + const response = await page.goto(url, { + waitUntil: "domcontentloaded", + timeout: 30_000, + }); + if (!response || !response.ok()) { + throw new Error( + `GET / returned HTTP ${response ? response.status() : "no response"}`, + ); + } + + // 1. The deep link must be accepted (not rejected by the token gate). + const status = page.locator('[data-testid="connection-status"]'); + await status.waitFor({ state: "attached", timeout: 30_000 }); + const deeplink = await status.getAttribute("data-deeplink"); + if (deeplink !== "parsed") { + throw new Error( + `deep link was not accepted (data-deeplink="${deeplink}") — expected "parsed"`, + ); + } + + // 2. Connected to the test server. + await page + .locator('[data-testid="connection-status"][data-status="connected"]') + .waitFor({ state: "attached", timeout: 45_000 }); + + // 3. The widget rendered inside the sandbox and completed its handshake. + // This is the load-bearing assertion — see the header comment. + try { + await page + .locator('[data-testid="apps-form"][data-app-status="ready"]') + .waitFor({ state: "attached", timeout: 45_000 }); + } catch { + const form = page.locator('[data-testid="apps-form"]'); + const appStatus = (await form.count()) + ? await form.getAttribute("data-app-status") + : "(no apps-form)"; + const appError = (await form.count()) + ? await form.getAttribute("data-app-error") + : null; + throw new Error( + `app never reached data-app-status="ready" (last: "${appStatus}"` + + `${appError ? `, data-app-error="${appError}"` : ""}) — the sandbox proxy ` + + `or the UI-protocol bridge failed to complete`, + ); + } + }; + + // Race against launcher death so a mid-run server crash is reported as the + // real cause instead of a downstream timeout. + try { + await Promise.race([server.whenChildExits(), drive()]); + } catch (err) { + const diagnostics = [ + ...pageErrors, + ...fatalConsole().map((m) => `console: ${m}`), + ]; + await fail( + `${err instanceof Error ? err.message : String(err)}${ + diagnostics.length + ? ` — page diagnostics: ${diagnostics.join("; ")}` + : "" + }`, + ); + } + + // Hard failures: any uncaught sync page error, plus the console errors that + // are the async half of the same class. + const fatal = [...pageErrors, ...fatalConsole()]; + if (fatal.length > 0) { + await fail(`app logged uncaught error(s): ${fatal.join("; ")}`); + } + + // Non-fatal console errors: surface them so a real problem isn't invisible, + // without failing on benign subresource/warning noise. + const benignConsole = consoleErrors.filter((m) => !FATAL_CONSOLE.test(m)); + if (benignConsole.length > 0) { + console.log( + `smoke:web:app note — ${benignConsole.length} non-fatal console error(s): ${benignConsole.join("; ")}`, + ); + } + + console.log( + `smoke:web:app OK — connected to ${mcpUrl}, opened "${APP_TOOL}", ` + + `widget reached data-app-status="ready" through the sandbox proxy`, + ); + await shutdown(); + process.exit(0); +} catch (err) { + await fail(err instanceof Error ? err.message : String(err)); +} diff --git a/test-servers/configs/mcp-app-http.json b/test-servers/configs/mcp-app-http.json new file mode 100644 index 000000000..445966acc --- /dev/null +++ b/test-servers/configs/mcp-app-http.json @@ -0,0 +1,12 @@ +{ + "serverInfo": { + "name": "mcp-app-showcase", + "version": "1.0.0" + }, + "tools": [{ "preset": "echo" }, { "preset": "mcp_app_demo" }], + "resources": [{ "preset": "mcp_app_demo_widget" }], + "transport": { + "type": "streamable-http", + "port": 3130 + } +}