From 3075d0aaf34273b2ca1cdaa8507f97eb7085efe8 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 10 Jul 2026 12:36:26 -0400 Subject: [PATCH] build(publish): fix v2 single-package bundle and add pack:verify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prepare the single `@modelcontextprotocol/inspector` tarball for publishing and add a script that verifies the real published package end to end. - Add `scripts/pack-and-verify.mjs` (`npm run pack:verify`): build, `npm pack`, install the tarball into a clean consumer, and drive the installed `mcp-inspector` bin (web/cli/tui) — catching "works in --dev, breaks under npx" packaging/path failures the in-repo smokes miss. Local/release tool (install needs network), not wired into the fast validate/ci loop. - Disable source maps in the cli/tui/web-runner tsup configs (~halves the unpacked payload; runtime doesn't need them). - Add `clients/web/.npmignore` so `clients/web/build` (prod web-server runner) ships — npm's packlist was honoring the nested `.gitignore` `build/` entry over the root "files" allowlist, dropping the runner while `dist/` slipped through. - Ship `clients/cli/package.json` and `clients/tui/package.json`: both bundles read their own package.json at runtime (CLI client identity, TUI header), so the installed CLI crashed on connect and the installed TUI crashed on launch without them. Document the packaging invariants and pack:verify in the launcher README. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- clients/cli/tsup.config.ts | 4 +- clients/launcher/README.md | 14 +- clients/tui/tsup.config.ts | 4 +- clients/web/.npmignore | 19 ++ clients/web/tsup.runner.config.ts | 4 +- package.json | 3 + scripts/pack-and-verify.mjs | 327 ++++++++++++++++++++++++++++++ 7 files changed, 371 insertions(+), 4 deletions(-) create mode 100644 clients/web/.npmignore create mode 100644 scripts/pack-and-verify.mjs diff --git a/clients/cli/tsup.config.ts b/clients/cli/tsup.config.ts index 63278f07..752ab878 100644 --- a/clients/cli/tsup.config.ts +++ b/clients/cli/tsup.config.ts @@ -10,7 +10,9 @@ export default defineConfig({ format: ['esm'], outDir: 'build', clean: true, - sourcemap: true, + // No source maps in the published bundle — they roughly double the on-disk + // size and aren't needed at runtime (debug via `npm run dev` on the source). + sourcemap: false, target: 'node22', platform: 'node', // Bundle core source; leave npm deps external. diff --git a/clients/launcher/README.md b/clients/launcher/README.md index 085dc7ee..42b42b11 100644 --- a/clients/launcher/README.md +++ b/clients/launcher/README.md @@ -96,7 +96,19 @@ repo-root `validate:launcher` simply delegates here (`cd clients/launcher && npm ## Publishing -The root `@modelcontextprotocol/inspector` package ships as one fat tarball: `npm run build` at the repo root builds all clients, then `prepack` runs before `npm publish`. Runtime dependencies are declared on the root `package.json`; client builds bundle `@inspector/core` and externalize npm packages resolved from the root install. +The root `@modelcontextprotocol/inspector` package ships as one fat tarball with a **single version number** (no separate `-web` / `-cli` / `-tui` / `-core` packages): `npm run build` at the repo root builds all clients, then `prepack` runs before `npm publish`. Runtime dependencies are declared on the root `package.json`; client builds bundle `@inspector/core` and externalize npm packages resolved from the root install. + +### What ships, and the packaging invariants + +The root `package.json` `"files"` allowlist is the source of truth for the tarball. A few non-obvious entries exist because they are read **at runtime** or were silently dropped by the packlist — do not remove them without re-running `npm run pack:verify`: + +- **No source maps.** The client bundlers set `sourcemap: false` (`clients/cli/tsup.config.ts`, `clients/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` is packed 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/cli/package.json` and `clients/tui/package.json` ship** because those bundles read their own `package.json` at runtime for the client identity / TUI header (`name`, `description`, `version`) relative to the bundle location. Without them the installed CLI crashes on connect and the installed TUI crashes on launch — even though both work in-repo, where the files are present. + +### `npm run pack:verify` — publish smoke against the real tarball + +The `smoke:*` scripts run against the in-repo build tree, which is **not** the published package. `npm run pack:verify` (`scripts/pack-and-verify.mjs`) closes that gap: it builds, `npm pack`s the publishable tarball (asserting no source maps ship and that `clients/web/{build,dist}` + the two client `package.json`s are present), installs the tarball into a clean throwaway consumer (real `npm install `, runs `postinstall`), and drives the installed `mcp-inspector` bin end to end — `--help` dispatch, a real `--cli` `tools/list` over stdio, and a prod `--web` boot that must serve `/` from the shipped `dist`. It catches "works in `--dev`, breaks under `npx @modelcontextprotocol/inspector`" path/packaging failures (exactly how the two `package.json` reads and the `web/build` omission above were found). It requires network access (the install pulls runtime deps), so it is a local / release check, not part of the fast `validate` loop. ## Architecture diff --git a/clients/tui/tsup.config.ts b/clients/tui/tsup.config.ts index 568d97d7..c998221a 100644 --- a/clients/tui/tsup.config.ts +++ b/clients/tui/tsup.config.ts @@ -10,7 +10,9 @@ export default defineConfig({ format: ['esm'], outDir: 'build', clean: true, - sourcemap: true, + // No source maps in the published bundle — they roughly double the on-disk + // size and aren't needed at runtime (debug via `npm run dev` on the source). + sourcemap: false, target: 'node22', platform: 'node', noExternal: [/^@inspector\/core/], diff --git a/clients/web/.npmignore b/clients/web/.npmignore new file mode 100644 index 00000000..133cadc2 --- /dev/null +++ b/clients/web/.npmignore @@ -0,0 +1,19 @@ +# npm's packlist uses THIS file instead of .gitignore when deciding what to +# publish from clients/web. It exists to fix a packaging bug: .gitignore here +# lists `build/` (a git artifact we never commit), and npm honored that +# exclusion even though the root package.json "files" field explicitly lists +# `clients/web/build`. The result was the prod web-server runner silently +# missing from the published @modelcontextprotocol/inspector tarball while +# `clients/web/dist` slipped through — so `mcp-inspector --web` had no server to +# load. (The other clients don't hit this because none of them ship a nested +# .gitignore.) +# +# Crucially this file does NOT list `build` or `dist`, so both the prod runner +# (build/) and the SPA (dist/) are packed. The root "files" allowlist already +# restricts publishing to those two directories, so everything else in +# clients/web (src, configs, node_modules, coverage, storybook-static) stays out +# regardless — the entries below are just belt-and-suspenders. +node_modules +coverage +storybook-static +dist-ssr diff --git a/clients/web/tsup.runner.config.ts b/clients/web/tsup.runner.config.ts index 6841de44..a0fb8a2e 100644 --- a/clients/web/tsup.runner.config.ts +++ b/clients/web/tsup.runner.config.ts @@ -10,7 +10,9 @@ export default defineConfig({ format: ['esm'], outDir: 'build', clean: true, - sourcemap: true, + // No source maps in the published bundle — they roughly double the on-disk + // size and aren't needed at runtime (debug via `npm run dev` on the source). + sourcemap: false, target: 'node22', platform: 'node', noExternal: [/^@inspector\/core/], diff --git a/package.json b/package.json index 3d069649..359605f2 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,9 @@ "clients/web/build", "clients/web/dist", "clients/cli/build", + "clients/cli/package.json", "clients/tui/build", + "clients/tui/package.json", "scripts/install-clients.mjs" ], "scripts": { @@ -54,6 +56,7 @@ "smoke:tui": "node scripts/smoke-tui.mjs", "smoke:web": "node scripts/smoke-web.mjs", "smoke:launcher": "node scripts/smoke-launcher.mjs", + "pack:verify": "node scripts/pack-and-verify.mjs", "prepack": "npm run build", "postinstall": "node scripts/install-clients.mjs" }, diff --git a/scripts/pack-and-verify.mjs b/scripts/pack-and-verify.mjs new file mode 100644 index 00000000..aa44f51f --- /dev/null +++ b/scripts/pack-and-verify.mjs @@ -0,0 +1,327 @@ +#!/usr/bin/env node +/** + * Pack-and-verify: build the exact tarball npm would publish, install it into a + * clean throwaway consumer, and drive the real `mcp-inspector` bin against that + * installed package — web, cli, and tui (#1636). + * + * Why this exists, and why the `smoke:*` scripts aren't enough: the smokes run + * against the in-repo build tree (`clients//build`, `clients/web/dist`) via the + * launcher's relative paths. That tree is NOT the published package. We have + * repeatedly been bitten by things that work in `--dev` / in-repo yet break for + * a real consumer running `npx @modelcontextprotocol/inspector …`, because: + * + * - the `files` allowlist (root + each nested client `package.json`) silently + * omits something the runtime needs (this is exactly how `clients/web/build` + * was missing from the tarball while `clients/web/dist` slipped through); + * - path resolution that is correct relative to the repo differs once the code + * lives under `node_modules/@modelcontextprotocol/inspector/…`; + * - the `postinstall` client-install cascade misbehaves for an end user. + * + * This script closes that gap by exercising the package the way npm publishes + * and a user installs it, end to end. It: + * + * 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); + * 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 + * dispatch, a real `--cli` `tools/list` over stdio, and a prod `--web` boot + * that must serve `/` (HTTP 200) with the injected auth-token global from + * the shipped `dist` — all from the INSTALLED location, not the repo. + * + * Exits non-zero on the first failure. Requires network access (step 3 pulls the + * package's runtime dependencies from the registry) — it is a local / release + * verification tool, not part of the fast `validate` loop. + * + * Usage: `npm run pack:verify` (from the repo root). + */ + +import { spawn, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, existsSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; + +const repoRoot = resolve(import.meta.dirname, ".."); +const testServer = join(repoRoot, "test-servers", "build", "test-server-stdio.js"); + +// Mirrors INSPECTOR_API_TOKEN_GLOBAL in core/mcp/remote/constants.ts; kept as a +// literal because this plain .mjs script can't import the TS source. +const TOKEN_GLOBAL = "__INSPECTOR_API_TOKEN__"; + +function fail(message) { + console.error(`\npack:verify FAILED — ${message}`); + process.exit(1); +} + +function step(message) { + console.log(`\npack:verify — ${message}`); +} + +/** Run a command to completion, inheriting stdio. Returns the exit status. */ +function runInherit(command, args, cwd = repoRoot) { + const r = spawnSync(command, args, { + cwd, + stdio: "inherit", + shell: process.platform === "win32", + }); + return r.status; +} + +/** Build the bundled stdio test server if it isn't present yet. */ +function ensureTestServer() { + if (existsSync(testServer)) return; + step("building test-servers (missing build output)..."); + const status = runInherit("npx", ["tsc", "-p", "test-servers", "--noCheck"]); + if (status !== 0 || !existsSync(testServer)) { + fail("could not build the stdio test server (test-servers/build/test-server-stdio.js)"); + } +} + +// --------------------------------------------------------------------------- +// 1. Build every client exactly as the publish would. +// --------------------------------------------------------------------------- +step("building all clients (npm run build)..."); +if (runInherit("npm", ["run", "build"]) !== 0) { + fail("`npm run build` failed"); +} + +// --------------------------------------------------------------------------- +// 2. Pack the publishable tarball and inspect its file list. +// --ignore-scripts: we just built, so skip the prepack rebuild AND keep +// stdout clean JSON (prepack build output would otherwise pollute it). +// --------------------------------------------------------------------------- +step("packing the publishable tarball (npm pack)..."); +const pack = spawnSync( + "npm", + ["pack", "--json", "--ignore-scripts", "--pack-destination", tmpdir()], + { cwd: repoRoot, encoding: "utf8" }, +); +if (pack.status !== 0) { + fail(`\`npm pack\` failed:\n${pack.stderr || pack.stdout}`); +} +let packInfo; +try { + packInfo = JSON.parse(pack.stdout)[0]; +} catch { + fail(`could not parse \`npm pack --json\` output:\n${pack.stdout}`); +} +const tarball = join(tmpdir(), packInfo.filename.replace(/^.*[/\\]/, "")); +if (!existsSync(tarball)) { + fail(`packed tarball not found at ${tarball}`); +} +const tarredPaths = packInfo.files.map((f) => f.path); + +// 2a. No source maps in the published bundle. +const maps = tarredPaths.filter((p) => p.endsWith(".map")); +if (maps.length > 0) { + fail( + `${maps.length} source map(s) leaked into the tarball — they should be disabled ` + + `in the client bundlers:\n ${maps.slice(0, 10).join("\n ")}`, + ); +} + +// 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) — plus the TUI's package.json, which the TUI bundle reads at +// runtime for its header (name/description/version). `clients/web/build` was +// previously dropped by the nested .gitignore; `clients/tui/package.json` +// was never listed, so the installed TUI crashed on launch. +for (const required of [ + "clients/web/build/index.js", + "clients/web/dist/index.html", + "clients/cli/package.json", + "clients/tui/package.json", +]) { + if (!tarredPaths.includes(required)) { + fail( + `expected \`${required}\` in the published tarball but it is missing — ` + + `check the "files" field in clients/web/package.json`, + ); + } +} +console.log( + `pack:verify — tarball OK: ${tarredPaths.length} files, no source maps, ` + + `clients/web/{build,dist} present (${(packInfo.unpackedSize / 1048576).toFixed(2)} MB unpacked)`, +); + +// --------------------------------------------------------------------------- +// 3. Install the tarball into a clean throwaway consumer (real npm install, +// runs the package's postinstall). +// --------------------------------------------------------------------------- +const work = mkdtempSync(join(tmpdir(), "pack-verify-")); +let cleanupTarball = true; +try { + ensureTestServer(); + step(`installing the tarball into a clean consumer at ${work} (pulls runtime deps)...`); + writeFileSync( + join(work, "package.json"), + JSON.stringify({ name: "pack-verify-consumer", private: true, version: "0.0.0" }), + ); + const install = runInherit( + "npm", + ["install", tarball, "--no-audit", "--no-fund", "--loglevel", "error"], + work, + ); + if (install !== 0) { + fail("`npm install ` into the throwaway consumer failed"); + } + + const installedPkg = join(work, "node_modules", "@modelcontextprotocol", "inspector"); + const bin = join(work, "node_modules", ".bin", process.platform === "win32" ? "mcp-inspector.cmd" : "mcp-inspector"); + if (!existsSync(bin)) { + fail(`installed \`mcp-inspector\` bin not found at ${bin}`); + } + // Confirm the two packaging fixes survived install onto disk. + for (const required of [ + join(installedPkg, "clients", "web", "build", "index.js"), + join(installedPkg, "clients", "web", "dist", "index.html"), + join(installedPkg, "clients", "launcher", "build", "index.js"), + join(installedPkg, "clients", "cli", "package.json"), + join(installedPkg, "clients", "tui", "package.json"), + ]) { + if (!existsSync(required)) { + fail(`expected installed file missing: ${required}`); + } + } + + /** Run the installed bin. Returns { status, output }. */ + const runBin = (args, extraEnv = {}) => { + const r = spawnSync(bin, args, { + cwd: work, + encoding: "utf8", + env: { ...process.env, ...extraEnv }, + shell: process.platform === "win32", + }); + return { status: r.status, output: `${r.stdout ?? ""}${r.stderr ?? ""}`, stdout: r.stdout ?? "" }; + }; + + // --------------------------------------------------------------------------- + // 4. Drive the installed bin. + // --------------------------------------------------------------------------- + // 4a. Help dispatch for each mode — proves the launcher resolved every client + // build from its installed location. + step("verifying help dispatch (--help, --cli --help, --tui --help)..."); + const helpChecks = [ + { args: ["--help"], marker: "Mode flags (--web, --cli, --tui)", label: "launcher --help" }, + { args: ["--cli", "--help"], marker: "Usage: inspector-cli", label: "--cli dispatch" }, + { args: ["--tui", "--help"], marker: "Usage: mcp-inspector-tui", label: "--tui dispatch" }, + ]; + for (const { args, marker, label } of helpChecks) { + const r = runBin(args); + if (r.status !== 0) { + fail(`\`${args.join(" ")}\` exited ${r.status}\n${r.output.slice(0, 800)}`); + } + if (!r.output.includes(marker)) { + fail(`\`${args.join(" ")}\` (${label}) did not print "${marker}"\n${r.output.slice(0, 800)}`); + } + } + + // 4b. Real CLI connect over stdio from the installed package: tools/list must + // return the bundled test server's tools. Exercises launcher → cli → core + // → stdio transport path from node_modules. + step("verifying `--cli` tools/list over stdio from the installed package..."); + const catalogPath = join(work, "catalog.json"); + writeFileSync( + catalogPath, + JSON.stringify({ + mcpServers: { + test: { type: "stdio", command: process.execPath, args: [testServer] }, + }, + }), + ); + const list = runBin(["--cli", "--catalog", catalogPath, "--server", "test", "--method", "tools/list"]); + if (list.status !== 0) { + fail(`\`--cli … tools/list\` exited ${list.status}\n${list.output}`); + } + let parsed; + try { + parsed = JSON.parse(list.stdout); + } catch { + fail(`\`--cli … tools/list\` did not return JSON on stdout:\n${list.stdout}`); + } + if (!(parsed.tools ?? []).some((t) => t.name === "echo")) { + fail(`\`--cli … tools/list\` missing expected "echo" tool`); + } + + // 4c. Prod `--web` boot from the installed package — THE critical packaging + // path: the runner must locate and serve the shipped `dist` (not rebuild + // it) and inject the auth token. Run non-blocking and poll `/`. + await verifyWeb(bin, work); + + console.log( + "\npack:verify OK — published tarball installs clean and the real bin drives " + + "web (prod / served dist), cli (stdio tools/list), and tui (help) end to end.", + ); +} catch (err) { + cleanupTarball = true; + fail(err instanceof Error ? err.message : String(err)); +} finally { + rmSync(work, { recursive: true, force: true }); + if (cleanupTarball) rmSync(tarball, { force: true }); +} + +/** + * Boot the installed `mcp-inspector --web` (prod), wait for it to listen, and + * assert `GET /` returns 200 with the injected token global served from the + * shipped `dist`. Kills the server before returning. + */ +async function verifyWeb(bin, cwd) { + step("verifying prod `--web` serves the shipped dist from the installed package..."); + const host = "127.0.0.1"; + const port = process.env.PACK_VERIFY_WEB_PORT ?? "6399"; + const token = "pack-verify-token"; + const child = spawn(bin, ["--web"], { + cwd, + env: { + ...process.env, + CLIENT_PORT: port, + HOST: host, + MCP_INSPECTOR_API_TOKEN: token, + MCP_AUTO_OPEN_ENABLED: "false", + }, + stdio: ["ignore", "inherit", "inherit"], + shell: process.platform === "win32", + }); + + let exited = false; + let exitCode = null; + child.on("exit", (code) => { + exited = true; + exitCode = code; + }); + const stop = () => { + if (!exited) child.kill("SIGTERM"); + }; + + try { + let res = null; + for (let attempt = 0; attempt < 120 && !res; attempt++) { + if (exited) { + fail(`\`--web\` exited (code ${exitCode}) before serving — see output above`); + } + try { + res = await fetch(`http://${host}:${port}/`); + } catch { + await delay(500); + } + } + if (!res) { + fail("`--web` server did not start within 60s"); + } + if (res.status !== 200) { + fail(`\`--web\` GET / returned HTTP ${res.status}, expected 200`); + } + const body = await res.text(); + if (!body.includes(TOKEN_GLOBAL)) { + fail(`\`--web\` served HTML is missing the ${TOKEN_GLOBAL} global (token not injected)`); + } + if (!body.includes(token)) { + fail("`--web` served HTML is missing the injected auth-token value"); + } + } finally { + stop(); + } +}