chore(ci): render an MCP App from the installed tarball in pack:verify (#2003)
Nothing covered the intersection #1859 fell through. `smoke:web:app` drives the App path but against the repo build tree; `pack:verify` runs against the installed tarball but only asserted the sandbox proxy page *exists*. So a packaging regression that ships the file present-but-unreachable — a rename with a stale reader, a wrong position relative to clients/web/build, a future "files"/.npmignore edit — passes both. Extract the drive-the-App-flow half of the smoke into scripts/lib/mcp-app-flow.mjs and call it from both, rather than copying it: the deep-link shape is the part that rots silently, and a drifted link fails as an opaque timeout rather than a mismatch. pack:verify then drives connect -> open app -> data-app-status="ready" in headless Chromium against the installed `--web` boot it already starts. The client under test comes from the install; the App test server stays a repo fixture. Also isolates pack:verify's `--web` catalog (the #1977 treatment the smokes already have). Untidy while the step only did `GET /`; the App deep link persists a server row, which would make it destructive and non-deterministic against the developer's real ~/.mcp-inspector/mcp.json. Unit tests cover what neither consumer can check itself — both CSRF gates and the appArgs encoding on the deep link, and driveAppFlow's failure branches against a stand-in page, all dead code from a happy-path smoke's view. Verified: pack:verify passes end to end against the real tarball, and fails at the new step alone when sandbox-controller.ts is pointed at a renamed proxy page (the tarball and on-disk assertions both still pass — the exact gap this closes). Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
a92a73003b
commit
17c33080f9
2 changed files with 676 additions and 242 deletions
282
scripts/lib/mcp-app-flow.mjs
Normal file
282
scripts/lib/mcp-app-flow.mjs
Normal file
|
|
@ -0,0 +1,282 @@
|
|||
/**
|
||||
* The MCP Apps drive-the-App-flow half of `smoke:web:app`, extracted so
|
||||
* `pack:verify` can run the same flow against the **installed tarball** (#2003).
|
||||
*
|
||||
* Two checks bracketed the bug that motivated all of this (#1859, the missing
|
||||
* `clients/web/static/sandbox_proxy.html`) without either one catching it:
|
||||
*
|
||||
* | check | runs against | drives the App path? |
|
||||
* | --------------- | ------------------ | -------------------- |
|
||||
* | `smoke:web:app` | the repo build tree | yes |
|
||||
* | `pack:verify` | the installed tarball | no — only `GET /` |
|
||||
*
|
||||
* So a packaging regression that leaves the file present-but-unreachable — a
|
||||
* rename with a stale reader, a wrong relative position, a future `.npmignore`
|
||||
* or `"files"` edit — passes both: the smoke never installs, and `pack:verify`
|
||||
* only asserted the path *exists*. Sharing the flow is what closes it, and it
|
||||
* has to be *shared* rather than copied: the deep-link shape is the part that
|
||||
* silently rots, and two copies would drift without either failing.
|
||||
*
|
||||
* Note the split of responsibilities that survives here. The **client** under
|
||||
* test comes from wherever the caller booted it (repo tree or install); the
|
||||
* **test server** is always a repo fixture — it is not in the tarball and
|
||||
* should not be.
|
||||
*
|
||||
* 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 a `cd clients/web`
|
||||
* in the npm script would NOT make it resolvable. Same gotcha as
|
||||
* smoke-web-browser.mjs; see its header.
|
||||
*/
|
||||
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import { join } from "node:path";
|
||||
import { startAnnouncedChild } from "./announced-child.mjs";
|
||||
import { resolveNodeBin } from "./resolve-node-bin.mjs";
|
||||
|
||||
/** The App tool the `mcp-app-http.json` fixture serves. */
|
||||
export 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.
|
||||
*/
|
||||
export const FATAL_CONSOLE =
|
||||
/^Uncaught\b|Failed to fetch dynamically imported module/;
|
||||
|
||||
/** Path to the composable test server build, relative to the repo root. */
|
||||
export function composableServerPath(repoRoot) {
|
||||
return join(repoRoot, "test-servers", "build", "server-composable.js");
|
||||
}
|
||||
|
||||
/** Path to a `test-servers/configs/<name>.json` fixture. */
|
||||
export function testServerConfigPath(repoRoot, name) {
|
||||
return join(repoRoot, "test-servers", "configs", `${name}.json`);
|
||||
}
|
||||
|
||||
/**
|
||||
* The path `clients/web/server/sandbox-controller.ts` resolves at runtime, given
|
||||
* the directory holding the built web runner. Kept in sync with the
|
||||
* `join(__dirname, "../static/sandbox_proxy.html")` there — the position
|
||||
* *relative to* the runner is what matters, which is why callers pass the
|
||||
* runner dir rather than a package root.
|
||||
*/
|
||||
export function sandboxProxyPageFor(webBuildDir) {
|
||||
return join(webBuildDir, "..", "static", "sandbox_proxy.html");
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the composable test server bundle if it isn't present yet.
|
||||
*
|
||||
* The root-installed tsc is run via this Node — `npx` is a `.cmd` shim on
|
||||
* Windows that a shell-free spawnSync can't start (ENOENT — #1939).
|
||||
*/
|
||||
export function ensureComposableTestServer(repoRoot, label) {
|
||||
const entry = composableServerPath(repoRoot);
|
||||
if (existsSync(entry)) return entry;
|
||||
console.log(`${label} — building test-servers (missing build output)...`);
|
||||
const r = spawnSync(
|
||||
process.execPath,
|
||||
[
|
||||
resolveNodeBin("typescript", "tsc", repoRoot),
|
||||
"-p",
|
||||
"test-servers",
|
||||
"--noCheck",
|
||||
],
|
||||
{ cwd: repoRoot, stdio: "inherit" },
|
||||
);
|
||||
if (r.status !== 0 || !existsSync(entry)) {
|
||||
throw new Error(
|
||||
"could not build the test servers (test-servers/build/server-composable.js). " +
|
||||
"Run `npm run test-servers:build` from clients/web.",
|
||||
);
|
||||
}
|
||||
return entry;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn a composable test server and wait for it to announce its URL.
|
||||
*
|
||||
* The announced line is authoritative rather than the config's port:
|
||||
* `createTestServerHttp` resolves through `findAvailablePort()`, which walks
|
||||
* upward when the configured value is taken — so the config port is a starting
|
||||
* hint, not a guarantee, and assuming it fails whenever anything else holds it.
|
||||
* The announcement goes to **stderr** (`console.error` in
|
||||
* server-composable.ts), which is why `startAnnouncedChild` scans both channels.
|
||||
*
|
||||
* `onSpawn` publishes the child before the readiness wait, so the caller's
|
||||
* teardown reaches it on every throw path — the timeout included (#2000).
|
||||
*/
|
||||
export async function startMcpAppServer({
|
||||
repoRoot,
|
||||
config = "mcp-app-http",
|
||||
onSpawn,
|
||||
label,
|
||||
}) {
|
||||
const entry = ensureComposableTestServer(repoRoot, label);
|
||||
const { match } = await startAnnouncedChild({
|
||||
command: process.execPath,
|
||||
args: [entry, "--config", testServerConfigPath(repoRoot, config)],
|
||||
cwd: repoRoot,
|
||||
pattern: /listening at (http:\/\/\S+)/i,
|
||||
onSpawn,
|
||||
what: `MCP test server (${entry})`,
|
||||
});
|
||||
return match[1];
|
||||
}
|
||||
|
||||
/** base64url(JSON) — the `appArgs` encoding the deep link expects. */
|
||||
export function encodeAppArgs(args) {
|
||||
return Buffer.from(JSON.stringify(args))
|
||||
.toString("base64")
|
||||
.replace(/\+/g, "-")
|
||||
.replace(/\//g, "_")
|
||||
.replace(/=+$/, "");
|
||||
}
|
||||
|
||||
/**
|
||||
* The deep link that connects, switches to the Apps tab, pre-selects the app
|
||||
* tool, and fires "Open App". `autoConnect`/`autoOpen` must equal the session
|
||||
* token (CSRF gate). Shape owned by clients/web/README.md#deep-link-auto-connect.
|
||||
*/
|
||||
export function buildAppDeepLink({
|
||||
baseUrl,
|
||||
mcpUrl,
|
||||
token,
|
||||
appTool = APP_TOOL,
|
||||
appArgs = {},
|
||||
}) {
|
||||
return (
|
||||
`${baseUrl}/?serverUrl=${encodeURIComponent(mcpUrl)}` +
|
||||
`&transport=http&autoConnect=${token}&openApp=${appTool}` +
|
||||
`&appArgs=${encodeAppArgs(appArgs)}&autoOpen=${token}`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Attach the two error channels a headless page reports on.
|
||||
*
|
||||
* An uncaught *synchronous* page error fires `pageerror`; its async twin — an
|
||||
* unhandled rejection or a failed dynamic import — is not a `pageerror` at all,
|
||||
* Chromium reports it on the console channel instead. Both are captured; only
|
||||
* `fatal()` is a failure, so ordinary console noise stays a diagnostic.
|
||||
*/
|
||||
export function attachPageDiagnostics(page) {
|
||||
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());
|
||||
});
|
||||
return {
|
||||
pageErrors,
|
||||
consoleErrors,
|
||||
fatalConsole: () => consoleErrors.filter((m) => FATAL_CONSOLE.test(m)),
|
||||
benignConsole: () => consoleErrors.filter((m) => !FATAL_CONSOLE.test(m)),
|
||||
fatal: () => [
|
||||
...pageErrors,
|
||||
...consoleErrors.filter((m) => FATAL_CONSOLE.test(m)),
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
/** Launch headless Chromium, resolved from the web client's install. */
|
||||
export async function loadChromium(repoRoot) {
|
||||
const requireFromWeb = createRequire(
|
||||
join(repoRoot, "clients", "web", "package.json"),
|
||||
);
|
||||
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)})`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Drive **connect → open app → widget ready** on an already-open page.
|
||||
*
|
||||
* The load-bearing assertion is the `data-app-status="ready"` contract
|
||||
* documented in clients/web/README.md ("MCP Apps screen automation contract"):
|
||||
* 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 whole path — the sandbox controller
|
||||
* serving the proxy page, the proxy loading the UI resource, and the bridge
|
||||
* completing its handshake.
|
||||
*
|
||||
* Throws with the last observed `data-app-status` / `data-app-error` rather than
|
||||
* a bare timeout, so a failure names its stage.
|
||||
*
|
||||
* Takes only the small slice of Playwright's `page` it uses (`goto`, `locator`
|
||||
* → `waitFor`/`getAttribute`/`count`), which is what lets its failure paths be
|
||||
* unit-tested against a stand-in — a smoke only ever exercises its happy path.
|
||||
*/
|
||||
export async function driveAppFlow({
|
||||
page,
|
||||
url,
|
||||
gotoTimeoutMs = 30_000,
|
||||
connectTimeoutMs = 45_000,
|
||||
readyTimeoutMs = 45_000,
|
||||
}) {
|
||||
const response = await page.goto(url, {
|
||||
waitUntil: "domcontentloaded",
|
||||
timeout: gotoTimeoutMs,
|
||||
});
|
||||
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: gotoTimeoutMs });
|
||||
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: connectTimeoutMs });
|
||||
|
||||
// 3. The widget rendered inside the sandbox and completed its handshake.
|
||||
try {
|
||||
await page
|
||||
.locator('[data-testid="apps-form"][data-app-status="ready"]')
|
||||
.waitFor({ state: "attached", timeout: readyTimeoutMs });
|
||||
} catch {
|
||||
const form = page.locator('[data-testid="apps-form"]');
|
||||
const present = (await form.count()) > 0;
|
||||
const appStatus = present
|
||||
? await form.getAttribute("data-app-status")
|
||||
: "(no apps-form)";
|
||||
const appError = present ? 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`,
|
||||
);
|
||||
}
|
||||
}
|
||||
166
scripts/lib/mcp-app-flow.test.mjs
Normal file
166
scripts/lib/mcp-app-flow.test.mjs
Normal file
|
|
@ -0,0 +1,166 @@
|
|||
/**
|
||||
* Unit tests for the shared MCP Apps flow (#2003).
|
||||
*
|
||||
* These cover the halves neither consumer can check itself. `smoke:web:app` and
|
||||
* `pack:verify` only ever exercise the happy path — the widget reaches `ready`,
|
||||
* so every failure branch below is dead code from their point of view, and the
|
||||
* deep-link shape is asserted by nothing at all once it is built rather than
|
||||
* inlined. Both are exactly the parts that rot silently: a mangled `appArgs`
|
||||
* encoding or a dropped `autoOpen` token produces a page that simply never
|
||||
* connects, reported as an opaque timeout.
|
||||
*
|
||||
* `driveAppFlow` is driven against a stand-in page rather than Chromium — it
|
||||
* takes only `goto` and `locator(...)` → `waitFor`/`getAttribute`/`count`,
|
||||
* which is what makes that possible.
|
||||
*/
|
||||
|
||||
import assert from "node:assert/strict";
|
||||
import { describe, it } from "node:test";
|
||||
import {
|
||||
buildAppDeepLink,
|
||||
driveAppFlow,
|
||||
encodeAppArgs,
|
||||
sandboxProxyPageFor,
|
||||
} from "./mcp-app-flow.mjs";
|
||||
|
||||
/** Minimal Playwright `page` stand-in. `plan` maps a selector to its behavior. */
|
||||
function fakePage(plan, { gotoStatus = 200 } = {}) {
|
||||
return {
|
||||
goto: async () => ({
|
||||
ok: () => gotoStatus === 200,
|
||||
status: () => gotoStatus,
|
||||
}),
|
||||
locator: (selector) => {
|
||||
const entry = plan[selector] ?? {};
|
||||
return {
|
||||
waitFor: async () => {
|
||||
if (entry.waitFails) throw new Error(`timeout: ${selector}`);
|
||||
},
|
||||
getAttribute: async (name) => entry.attrs?.[name] ?? null,
|
||||
count: async () => entry.count ?? 0,
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const STATUS = '[data-testid="connection-status"]';
|
||||
const CONNECTED = '[data-testid="connection-status"][data-status="connected"]';
|
||||
const FORM = '[data-testid="apps-form"]';
|
||||
const READY = '[data-testid="apps-form"][data-app-status="ready"]';
|
||||
|
||||
/** The plan under which every stage succeeds. */
|
||||
const happyPlan = () => ({
|
||||
[STATUS]: { attrs: { "data-deeplink": "parsed" } },
|
||||
[CONNECTED]: {},
|
||||
[READY]: {},
|
||||
});
|
||||
|
||||
describe("encodeAppArgs", () => {
|
||||
it("encodes JSON as base64url with no padding", () => {
|
||||
const encoded = encodeAppArgs({ title: "a/b+c?" });
|
||||
assert.doesNotMatch(encoded, /[+/=]/);
|
||||
assert.deepEqual(JSON.parse(Buffer.from(encoded, "base64url").toString()), {
|
||||
title: "a/b+c?",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildAppDeepLink", () => {
|
||||
const link = buildAppDeepLink({
|
||||
baseUrl: "http://127.0.0.1:6399",
|
||||
mcpUrl: "http://127.0.0.1:3130/mcp",
|
||||
token: "tok",
|
||||
appArgs: { title: "t" },
|
||||
});
|
||||
const params = new URL(link).searchParams;
|
||||
|
||||
it("percent-encodes the server URL so its own query/scheme can't leak", () => {
|
||||
assert.ok(link.includes(encodeURIComponent("http://127.0.0.1:3130/mcp")));
|
||||
assert.equal(params.get("serverUrl"), "http://127.0.0.1:3130/mcp");
|
||||
});
|
||||
|
||||
it("carries the session token on BOTH gates", () => {
|
||||
// autoConnect and autoOpen are separate CSRF gates; dropping either leaves
|
||||
// the page silently idle rather than erroring.
|
||||
assert.equal(params.get("autoConnect"), "tok");
|
||||
assert.equal(params.get("autoOpen"), "tok");
|
||||
});
|
||||
|
||||
it("defaults to the fixture's app tool and round-trips appArgs", () => {
|
||||
assert.equal(params.get("openApp"), "mcp_app_demo");
|
||||
assert.equal(params.get("transport"), "http");
|
||||
assert.deepEqual(
|
||||
JSON.parse(Buffer.from(params.get("appArgs"), "base64url").toString()),
|
||||
{ title: "t" },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("sandboxProxyPageFor", () => {
|
||||
it("resolves relative to the runner dir, as sandbox-controller.ts does", () => {
|
||||
assert.match(
|
||||
sandboxProxyPageFor("/pkg/clients/web/build").replace(/\\/g, "/"),
|
||||
/\/pkg\/clients\/web\/(build\/\.\.\/)?static\/sandbox_proxy\.html$/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("driveAppFlow", () => {
|
||||
it("resolves when every stage succeeds", async () => {
|
||||
await driveAppFlow({ page: fakePage(happyPlan()), url: "http://x/" });
|
||||
});
|
||||
|
||||
it("reports a non-200 document instead of waiting on selectors", async () => {
|
||||
await assert.rejects(
|
||||
driveAppFlow({
|
||||
page: fakePage(happyPlan(), { gotoStatus: 500 }),
|
||||
url: "http://x/",
|
||||
}),
|
||||
/GET \/ returned HTTP 500/,
|
||||
);
|
||||
});
|
||||
|
||||
it("names a rejected deep link rather than timing out on connect", async () => {
|
||||
// The token gate refusing the link is the failure a bad `autoConnect`
|
||||
// produces; without this branch it surfaces as a 45s connect timeout.
|
||||
const plan = happyPlan();
|
||||
plan[STATUS] = { attrs: { "data-deeplink": "rejected" } };
|
||||
await assert.rejects(
|
||||
driveAppFlow({ page: fakePage(plan), url: "http://x/" }),
|
||||
/deep link was not accepted \(data-deeplink="rejected"\)/,
|
||||
);
|
||||
});
|
||||
|
||||
it("surfaces the last app status and error when ready never arrives", async () => {
|
||||
const plan = happyPlan();
|
||||
plan[READY] = { waitFails: true };
|
||||
plan[FORM] = {
|
||||
count: 1,
|
||||
attrs: {
|
||||
"data-app-status": "error",
|
||||
"data-app-error": "Sandbox not loaded",
|
||||
},
|
||||
};
|
||||
await assert.rejects(
|
||||
driveAppFlow({ page: fakePage(plan), url: "http://x/" }),
|
||||
(err) => {
|
||||
// The #1859 shape: the widget never renders because the proxy page the
|
||||
// runtime reads is not where it was shipped. The message has to name
|
||||
// that, not just "timeout".
|
||||
assert.match(err.message, /last: "error"/);
|
||||
assert.match(err.message, /data-app-error="Sandbox not loaded"/);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("says so when the apps form never mounted at all", async () => {
|
||||
const plan = happyPlan();
|
||||
plan[READY] = { waitFails: true };
|
||||
plan[FORM] = { count: 0 };
|
||||
await assert.rejects(
|
||||
driveAppFlow({ page: fakePage(plan), url: "http://x/" }),
|
||||
/last: "\(no apps-form\)"/,
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue