Merge branch 'v2/main' into v2/chore/2003-pack-verify-app-render
One conflict, scripts/smoke-web-app.mjs: #2056 added a second phase to the smoke (the same app declaring _meta.ui.domain, asserting it renders from the dedicated app origin rather than an opaque one) while this branch extracted the smoke's flow into scripts/lib/mcp-app-flow.mjs. Resolved by keeping both: phase 2 now runs through the shared helper too, which grows three knobs it needs and pack:verify does not — - expectDeepLink: phase 2 re-navigates the SAME page, so the token gate is already proven and a rejected re-navigate would fail at the connect wait regardless; - what: so a phase-2 ready timeout doesn't read as a phase-1 one; - extraDiagnostics: phase 2 appends the frame list, which is what separates "never rendered" from "rendered at the wrong origin". Each is covered by a new unit test, since neither consumer exercises a failure branch. pack:verify deliberately drives phase 1 only. The dedicated origin is served by the same runner the packaging assertions already cover and reads nothing new from disk (app-origin-controller.ts has no file reads), so a second browser launch there would buy no packaging signal. Verified on the merged tree: smoke:web:app passes both phases (dedicated origin http://127.0.0.1:6278/app-document/<id>), and pack:verify passes end to end against the real tarball. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
commit
df938777b1
3 changed files with 2557 additions and 76 deletions
|
|
@ -225,6 +225,18 @@ export async function loadChromium(repoRoot) {
|
|||
* Throws with the last observed `data-app-status` / `data-app-error` rather than
|
||||
* a bare timeout, so a failure names its stage.
|
||||
*
|
||||
* Three knobs exist for `smoke:web:app`'s second phase (#2056), which
|
||||
* re-navigates the *same* page to a second server declaring `_meta.ui.domain`:
|
||||
*
|
||||
* - `expectDeepLink` — the token gate is only worth asserting on the first
|
||||
* navigate. A re-navigate that were rejected fails at the connect wait
|
||||
* anyway, and phase 2's real subject is the origin, not the gate.
|
||||
* - `what` — the noun in the ready-failure message, so a phase-2 timeout does
|
||||
* not read as a phase-1 one.
|
||||
* - `extraDiagnostics` — appended to that message. Phase 2 lists the page's
|
||||
* frames, which is what distinguishes "the app never rendered" from "it
|
||||
* rendered at the wrong origin".
|
||||
*
|
||||
* 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.
|
||||
|
|
@ -232,6 +244,9 @@ export async function loadChromium(repoRoot) {
|
|||
export async function driveAppFlow({
|
||||
page,
|
||||
url,
|
||||
expectDeepLink = true,
|
||||
what = "app",
|
||||
extraDiagnostics = null,
|
||||
gotoTimeoutMs = 30_000,
|
||||
connectTimeoutMs = 45_000,
|
||||
readyTimeoutMs = 45_000,
|
||||
|
|
@ -247,13 +262,15 @@ export async function driveAppFlow({
|
|||
}
|
||||
|
||||
// 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"`,
|
||||
);
|
||||
if (expectDeepLink) {
|
||||
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.
|
||||
|
|
@ -273,10 +290,11 @@ export async function driveAppFlow({
|
|||
? await form.getAttribute("data-app-status")
|
||||
: "(no apps-form)";
|
||||
const appError = present ? await form.getAttribute("data-app-error") : null;
|
||||
const extra = extraDiagnostics ? await extraDiagnostics() : "";
|
||||
throw new Error(
|
||||
`app never reached data-app-status="ready" (last: "${appStatus}"` +
|
||||
`${what} never reached data-app-status="ready" (last: "${appStatus}"` +
|
||||
`${appError ? `, data-app-error="${appError}"` : ""}) — the sandbox proxy ` +
|
||||
`or the UI-protocol bridge failed to complete`,
|
||||
`or the UI-protocol bridge failed to complete${extra ? ` — ${extra}` : ""}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -154,6 +154,42 @@ describe("driveAppFlow", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("skips the deep-link gate when the caller re-navigates an open page", async () => {
|
||||
// smoke:web:app phase 2 (#2056) re-navigates the SAME page to a second
|
||||
// server. The gate was proven on the first navigate, and a rejected
|
||||
// re-navigate fails at the connect wait anyway — so a stale/absent
|
||||
// data-deeplink must not fail the phase.
|
||||
const plan = happyPlan();
|
||||
plan[STATUS] = { attrs: {} };
|
||||
await driveAppFlow({
|
||||
page: fakePage(plan),
|
||||
url: "http://x/",
|
||||
expectDeepLink: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("names the phase and appends its diagnostics on a ready timeout", async () => {
|
||||
// A phase-2 timeout that read as a phase-1 one would send the reader to the
|
||||
// wrong half of the smoke; the frame list is what separates "never
|
||||
// rendered" from "rendered at the wrong origin".
|
||||
const plan = happyPlan();
|
||||
plan[READY] = { waitFails: true };
|
||||
plan[FORM] = { count: 1, attrs: { "data-app-status": "loading" } };
|
||||
await assert.rejects(
|
||||
driveAppFlow({
|
||||
page: fakePage(plan),
|
||||
url: "http://x/",
|
||||
what: "dedicated-origin app",
|
||||
extraDiagnostics: () => "frames: a, b",
|
||||
}),
|
||||
(err) => {
|
||||
assert.match(err.message, /^dedicated-origin app never reached/);
|
||||
assert.match(err.message, /frames: a, b$/);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("says so when the apps form never mounted at all", async () => {
|
||||
const plan = happyPlan();
|
||||
plan[READY] = { waitFails: true };
|
||||
|
|
|
|||
|
|
@ -8,9 +8,16 @@
|
|||
* 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 flow itself lives in `lib/mcp-app-flow.mjs`, shared with `pack:verify`
|
||||
* (#2003) — see that header for why it is shared rather than copied, and for
|
||||
* the `data-app-status="ready"` contract the assertion rests on.
|
||||
* A second phase (#2056) drives the same app declaring `_meta.ui.domain`, and
|
||||
* asserts the thing that field exists for: the app document is served from the
|
||||
* backend's dedicated app-origin listener and runs at a REAL origin, not the
|
||||
* opaque one whose requests carry `Origin: null`. Both phases end at
|
||||
* `data-app-status="ready"`, so only reading the inner frame's own
|
||||
* `location.origin` can tell them apart.
|
||||
*
|
||||
* The flow both phases share lives in `lib/mcp-app-flow.mjs`, shared with
|
||||
* `pack:verify` (#2003) — see that header for why it is shared rather than
|
||||
* copied, and for the `data-app-status="ready"` contract the assertion rests on.
|
||||
*
|
||||
* ── What this does and does NOT catch ───────────────────────────────────────
|
||||
*
|
||||
|
|
@ -22,10 +29,11 @@
|
|||
*
|
||||
* The packaging dimension is owned by `npm run pack:verify`, which asserts the
|
||||
* file in the tarball packlist and on disk after a real install — and, since
|
||||
* #2003, drives this same App flow against the **installed** bin. Keep both:
|
||||
* #2003, drives phase 1's flow against the **installed** bin. Keep both:
|
||||
* pack:verify covers the published artifact, this covers the repo tree on every
|
||||
* `npm run ci`, where pack:verify (network-bound, local/release-only) does not
|
||||
* run. Neither subsumes the other.
|
||||
* run — and this is the only one of the two that drives phase 2. Neither
|
||||
* subsumes the other.
|
||||
*
|
||||
* As a cheap extra, this does assert the proxy page exists at the location the
|
||||
* runtime resolves it from (`clients/web/build/../static/…`, see
|
||||
|
|
@ -66,11 +74,12 @@ const HOST = "127.0.0.1";
|
|||
// 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";
|
||||
// The URL the test server announces on startup — authoritative over the
|
||||
// The URL each test server announces on startup — authoritative over the
|
||||
// config's port; see startMcpAppServer.
|
||||
let mcpUrl = null;
|
||||
|
||||
let mcpServer = null;
|
||||
let domainMcpServer = null;
|
||||
let browser = null;
|
||||
const server = startProdWebServer({
|
||||
host: HOST,
|
||||
|
|
@ -94,6 +103,11 @@ async function shutdown() {
|
|||
mcpServer = null;
|
||||
await stopChild(child, { label: LABEL, what: "MCP test server" });
|
||||
}
|
||||
if (domainMcpServer) {
|
||||
const child = domainMcpServer;
|
||||
domainMcpServer = null;
|
||||
await stopChild(child, { label: LABEL, what: "MCP test server (domain)" });
|
||||
}
|
||||
}
|
||||
|
||||
async function fail(message) {
|
||||
|
|
@ -128,17 +142,87 @@ try {
|
|||
const page = await browser.newPage();
|
||||
const diagnostics = attachPageDiagnostics(page);
|
||||
|
||||
const url = buildAppDeepLink({
|
||||
baseUrl: server.baseUrl,
|
||||
mcpUrl,
|
||||
token: TOKEN,
|
||||
appArgs: { title: LABEL },
|
||||
});
|
||||
const deepLink = (target) =>
|
||||
buildAppDeepLink({
|
||||
baseUrl: server.baseUrl,
|
||||
mcpUrl: target,
|
||||
token: TOKEN,
|
||||
appArgs: { title: LABEL },
|
||||
});
|
||||
|
||||
/** The frames a failure should name, so "never rendered" reads differently
|
||||
* from "rendered at the wrong origin". */
|
||||
const frameList = () =>
|
||||
`frames: ${page
|
||||
.frames()
|
||||
.map((f) => f.url() || "(blank)")
|
||||
.join(", ")}`;
|
||||
|
||||
/**
|
||||
* Phase 2 (#2056): the same app, this time declaring `_meta.ui.domain`.
|
||||
*
|
||||
* Phase 1 proves the default render works; it cannot distinguish it from the
|
||||
* dedicated-origin render, because both end at `data-app-status="ready"`. The
|
||||
* thing #2056 is actually about is the *origin the app document runs at* — an
|
||||
* opaque origin sends `Origin: null`, which no CORS / OAuth-callback /
|
||||
* API-key allowlist can admit. So this phase asserts the two facts that only
|
||||
* hold on the dedicated path: the inner frame was navigated to a published
|
||||
* document on the backend's app-origin listener, and the document's own
|
||||
* `location.origin` is a real origin rather than the literal "null".
|
||||
*
|
||||
* It reuses the running web server and browser page — only a second MCP
|
||||
* server and a second deep-link navigate are added. The deep-link gate is not
|
||||
* re-asserted (`expectDeepLink: false`): it was proven on the first navigate,
|
||||
* and a rejected one would fail at the connect wait regardless.
|
||||
*/
|
||||
const driveDedicatedOrigin = async () => {
|
||||
const domainUrl = await startMcpAppServer({
|
||||
repoRoot,
|
||||
config: "mcp-app-domain-http",
|
||||
onSpawn: (child) => {
|
||||
domainMcpServer = child;
|
||||
},
|
||||
label: LABEL,
|
||||
});
|
||||
await driveAppFlow({
|
||||
page,
|
||||
url: deepLink(domainUrl),
|
||||
expectDeepLink: false,
|
||||
what: "dedicated-origin app",
|
||||
extraDiagnostics: frameList,
|
||||
});
|
||||
|
||||
const appFrame = page
|
||||
.frames()
|
||||
.find((f) => /\/app-document\/[0-9a-f]{32}$/.test(f.url()));
|
||||
if (!appFrame) {
|
||||
throw new Error(
|
||||
"app declaring _meta.ui.domain was not served from the dedicated app " +
|
||||
`origin — no frame at /app-document/<id> (${frameList()})`,
|
||||
);
|
||||
}
|
||||
const origin = await appFrame.evaluate(() => window.location.origin);
|
||||
if (!/^http:\/\/[^/]+$/.test(origin)) {
|
||||
throw new Error(
|
||||
`app document ran at origin "${origin}" — expected a real http origin. ` +
|
||||
"An opaque origin sends `Origin: null`, which is the bug #2056 fixes.",
|
||||
);
|
||||
}
|
||||
return { origin, url: appFrame.url() };
|
||||
};
|
||||
|
||||
// Race against launcher death so a mid-run server crash is reported as the
|
||||
// real cause instead of a downstream timeout.
|
||||
let dedicated = null;
|
||||
try {
|
||||
await Promise.race([server.whenChildExits(), driveAppFlow({ page, url })]);
|
||||
await Promise.race([
|
||||
server.whenChildExits(),
|
||||
driveAppFlow({ page, url: deepLink(mcpUrl) }),
|
||||
]);
|
||||
dedicated = await Promise.race([
|
||||
server.whenChildExits(),
|
||||
driveDedicatedOrigin(),
|
||||
]);
|
||||
} catch (err) {
|
||||
const notes = [
|
||||
...diagnostics.pageErrors,
|
||||
|
|
@ -169,7 +253,9 @@ try {
|
|||
|
||||
console.log(
|
||||
`${LABEL} OK — connected to ${mcpUrl}, opened "${APP_TOOL}", ` +
|
||||
`widget reached data-app-status="ready" through the sandbox proxy`,
|
||||
`widget reached data-app-status="ready" through the sandbox proxy; ` +
|
||||
`and an app declaring _meta.ui.domain rendered from the dedicated origin ` +
|
||||
`${dedicated?.origin} (${dedicated?.url})`,
|
||||
);
|
||||
await shutdown();
|
||||
process.exit(0);
|
||||
|
|
|
|||
Loading…
Reference in a new issue