chore(smoke): run the web smokes against a throwaway catalog
`startProdWebServer` spread `process.env` without setting `MCP_CATALOG_PATH`, so `smoke:web`, `smoke:web:browser`, and `smoke:web:app` all ran against the developer's real `~/.mcp-inspector/mcp.json` — unlike `smoke:cli` / `smoke:tui`, which have always driven a temp `--catalog`. That made the web smokes both destructive and non-deterministic. `smoke:web:app`'s deep link persists a `deep-link` server row, so a second run finds it already on disk, racing hydration: the deep-link effect misses it in `servers`, POSTs `addServer` anyway, and the backend answers 409. The app swallows that 409 by design (App.tsx:2842) so the smoke still passed, but it reported a spurious non-fatal console error that was really just residue from the previous run. Give the helper its own temp catalog dir and clean it up in `stop()` via `removeSafe`, so cleanup can never turn a passing smoke red. Isolation lives in the shared helper rather than per script, since every web smoke needs it. Only the catalog is redirected — other per-user state under `~/.mcp-inspector` stays shared, since isolating it means redirecting HOME wholesale, which would also move the npx and Playwright caches these smokes depend on. Verified: `smoke:web:app` twice in a row is now clean both times (previously the second run reported the 409), the real catalog's mtime is unchanged, and no temp dirs leak. CI never saw this — a fresh HOME per run made every CI run look like a first run. Closes #1977 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
925b699040
commit
29e3ce95b8
4 changed files with 58 additions and 9 deletions
|
|
@ -1,10 +1,12 @@
|
|||
/**
|
||||
* 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.
|
||||
* `scripts/smoke-web.mjs` (serves-the-HTML check), `scripts/smoke-web-browser.mjs`
|
||||
* (runs-the-bundle check, #1615), and `scripts/smoke-web-app.mjs` (MCP Apps
|
||||
* end-to-end, #1859) all 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. Catalog isolation (#1977) lives here for the same
|
||||
* reason — it is a property every web smoke needs, not one script's concern.
|
||||
*
|
||||
* 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
|
||||
|
|
@ -12,9 +14,13 @@
|
|||
*/
|
||||
|
||||
import { spawn } from "node:child_process";
|
||||
import { mkdtempSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, resolve } from "node:path";
|
||||
import { dirname, join, resolve } from "node:path";
|
||||
|
||||
import { removeSafe } from "./child-cleanup.mjs";
|
||||
|
||||
const libDir = dirname(fileURLToPath(import.meta.url));
|
||||
const repoRoot = resolve(libDir, "..", "..");
|
||||
|
|
@ -24,20 +30,43 @@ 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.
|
||||
*
|
||||
* The server always runs against a **throwaway catalog**, never the developer's
|
||||
* real `~/.mcp-inspector/mcp.json` (#1977). Without `MCP_CATALOG_PATH` the web
|
||||
* backend falls back to that default writable catalog, which made these smokes
|
||||
* both destructive and non-deterministic: `smoke:web:app`'s deep link persists a
|
||||
* `deep-link` server row, so a second run finds it already on disk. The row is
|
||||
* then racing hydration — the deep-link effect misses it in `servers`, POSTs
|
||||
* `addServer` anyway, and the backend answers 409. The app swallows that 409 by
|
||||
* design so the smoke still passes, but it reports a spurious non-fatal console
|
||||
* error that is really just residue from the previous run. Isolating the catalog
|
||||
* makes every run look like the first one, and mirrors `smoke:cli` / `smoke:tui`,
|
||||
* which have always driven a temp `--catalog`.
|
||||
*
|
||||
* Only the catalog is redirected. Other per-user state under `~/.mcp-inspector`
|
||||
* (OAuth tokens, the `storage/` dir) is still shared — isolating that means
|
||||
* redirecting HOME wholesale, which these smokes deliberately do not do, since
|
||||
* HOME also resolves the npx and Playwright caches they depend on.
|
||||
*
|
||||
* @param {object} opts
|
||||
* @param {string} opts.host
|
||||
* @param {string} opts.port
|
||||
* @param {string} opts.token value injected as MCP_INSPECTOR_API_TOKEN
|
||||
* @param {string} [opts.label] prefix for the temp dir + cleanup warnings
|
||||
*/
|
||||
export function startProdWebServer({ host, port, token }) {
|
||||
export function startProdWebServer({ host, port, token, label = "smoke:web" }) {
|
||||
const baseUrl = `http://${host}:${port}`;
|
||||
|
||||
// The file need not exist — the backend seeds an empty catalog on first run.
|
||||
const catalogDir = mkdtempSync(join(tmpdir(), "smoke-web-catalog-"));
|
||||
const catalogPath = join(catalogDir, "catalog.json");
|
||||
|
||||
const child = spawn(process.execPath, [launcherEntry, "--web"], {
|
||||
env: {
|
||||
...process.env,
|
||||
CLIENT_PORT: port,
|
||||
HOST: host,
|
||||
MCP_INSPECTOR_API_TOKEN: token,
|
||||
MCP_CATALOG_PATH: catalogPath,
|
||||
// Don't pop a browser in CI.
|
||||
MCP_AUTO_OPEN_ENABLED: "false",
|
||||
},
|
||||
|
|
@ -119,10 +148,15 @@ export function startProdWebServer({ host, port, token }) {
|
|||
|
||||
return {
|
||||
baseUrl,
|
||||
catalogPath,
|
||||
waitForReady,
|
||||
whenChildExits,
|
||||
stop: () => {
|
||||
if (!exited) child.kill("SIGTERM");
|
||||
// SIGTERM is not awaited, so the server may still hold the catalog for a
|
||||
// moment. `removeSafe` warns rather than throws, so the worst case is a
|
||||
// leaked temp dir the OS reclaims — never a red smoke over cleanup.
|
||||
removeSafe(catalogDir, { label });
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -104,7 +104,12 @@ let mcpUrl = null;
|
|||
|
||||
let mcpServer = null;
|
||||
let browser = null;
|
||||
const server = startProdWebServer({ host: HOST, port: PORT, token: TOKEN });
|
||||
const server = startProdWebServer({
|
||||
host: HOST,
|
||||
port: PORT,
|
||||
token: TOKEN,
|
||||
label: "smoke:web:app",
|
||||
});
|
||||
|
||||
async function shutdown() {
|
||||
if (browser) {
|
||||
|
|
|
|||
|
|
@ -78,7 +78,12 @@ const TOKEN = "smoke-web-browser-token";
|
|||
// the font/CDN flake.
|
||||
const FATAL_CONSOLE = /^Uncaught\b|Failed to fetch dynamically imported module/;
|
||||
|
||||
const server = startProdWebServer({ host: HOST, port: PORT, token: TOKEN });
|
||||
const server = startProdWebServer({
|
||||
host: HOST,
|
||||
port: PORT,
|
||||
token: TOKEN,
|
||||
label: "smoke:web:browser",
|
||||
});
|
||||
let browser = null;
|
||||
|
||||
async function shutdown() {
|
||||
|
|
|
|||
|
|
@ -30,7 +30,12 @@ const TOKEN = "smoke-web-token";
|
|||
// literal because this plain .mjs script can't import the TS source.
|
||||
const TOKEN_GLOBAL = "__INSPECTOR_API_TOKEN__";
|
||||
|
||||
const server = startProdWebServer({ host: HOST, port: PORT, token: TOKEN });
|
||||
const server = startProdWebServer({
|
||||
host: HOST,
|
||||
port: PORT,
|
||||
token: TOKEN,
|
||||
label: "smoke:web",
|
||||
});
|
||||
|
||||
function fail(message) {
|
||||
console.error(`smoke:web FAILED — ${message}`);
|
||||
|
|
|
|||
Loading…
Reference in a new issue