* feat(web): build dist on demand for prod --web + end-to-end CI smoke (#1486) Prod `mcp-inspector --web` serves static assets from clients/web/dist, which only exists after a build. In a fresh checkout it was absent and the prod path served a broken page (ENOENT on index.html); the launcher smoke test only covered --help, so this path was unverified in CI. - ensure-web-build.ts: build dist on demand (vite build via build:client) when missing, or exit with an actionable error if the build can't run. - run-web.ts: invoke ensureWebBuild before startHonoServer in the prod path (--dev runs Vite directly and skips it). - scripts/smoke-web.mjs + smoke:web: start --web against built dist and assert GET / returns 200 with the injected __INSPECTOR_API_TOKEN__. Wired into validate:launcher and a new CI step. - Docs: launcher README + AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): surface spawn error in build-on-demand; clarify smoke comment Address PR #1497 review: - runViteBuild now logs result.error.message (e.g. ENOENT when npm isn't on PATH) before returning a null status, so the spawn-failure cause surfaces instead of only the generic actionable error. The build seam gains a `log` param; added a test covering the ENOENT path. - Reword scripts/smoke-web.mjs header so it no longer implies the readiness wait covers a cold build-on-demand vite build (it's sized for a pre-built dist, which validate/CI guarantee). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
91f02fd6db
commit
36b48e61a2
2 changed files with 405 additions and 11 deletions
148
clients/web/src/test/integration/server/ensure-web-build.test.ts
Normal file
148
clients/web/src/test/integration/server/ensure-web-build.test.ts
Normal file
|
|
@ -0,0 +1,148 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { join } from "node:path";
|
||||
|
||||
const { existsSync, spawnSync } = vi.hoisted(() => ({
|
||||
existsSync: vi.fn(),
|
||||
spawnSync: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("node:fs", () => ({ existsSync }));
|
||||
vi.mock("node:child_process", () => ({ spawnSync }));
|
||||
|
||||
import { ensureWebBuild } from "../../../../server/ensure-web-build.js";
|
||||
|
||||
const WEB_ROOT = "/repo/clients/web";
|
||||
const DIST_ROOT = join(WEB_ROOT, "dist");
|
||||
const INDEX_HTML = join(DIST_ROOT, "index.html");
|
||||
|
||||
describe("ensureWebBuild", () => {
|
||||
it("is a no-op when the build already exists", () => {
|
||||
const exists = vi.fn().mockReturnValue(true);
|
||||
const build = vi.fn();
|
||||
const log = vi.fn();
|
||||
|
||||
ensureWebBuild(WEB_ROOT, DIST_ROOT, { exists, build, log });
|
||||
|
||||
expect(exists).toHaveBeenCalledWith(INDEX_HTML);
|
||||
expect(build).not.toHaveBeenCalled();
|
||||
expect(log).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("builds on demand when the build is missing, then proceeds", () => {
|
||||
// Missing before the build, present after.
|
||||
const exists = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true);
|
||||
const build = vi.fn().mockReturnValue(0);
|
||||
const log = vi.fn();
|
||||
|
||||
expect(() =>
|
||||
ensureWebBuild(WEB_ROOT, DIST_ROOT, { exists, build, log }),
|
||||
).not.toThrow();
|
||||
|
||||
expect(build).toHaveBeenCalledWith(WEB_ROOT, expect.any(Function));
|
||||
expect(log).toHaveBeenCalledTimes(1);
|
||||
expect(log.mock.calls[0]?.[0]).toContain("No production web build found");
|
||||
});
|
||||
|
||||
it("throws an actionable error when the build exits non-zero", () => {
|
||||
const exists = vi.fn().mockReturnValue(false);
|
||||
const build = vi.fn().mockReturnValue(1);
|
||||
|
||||
expect(() =>
|
||||
ensureWebBuild(WEB_ROOT, DIST_ROOT, { exists, build, log: vi.fn() }),
|
||||
).toThrow(/Could not build the web UI automatically/);
|
||||
});
|
||||
|
||||
it("throws when the build can't even run (null exit status)", () => {
|
||||
const exists = vi.fn().mockReturnValue(false);
|
||||
const build = vi.fn().mockReturnValue(null);
|
||||
|
||||
expect(() =>
|
||||
ensureWebBuild(WEB_ROOT, DIST_ROOT, { exists, build, log: vi.fn() }),
|
||||
).toThrow(/`--dev`/);
|
||||
});
|
||||
|
||||
it("throws when the build succeeds but still produces no index.html", () => {
|
||||
// Exit 0 but the asset is still absent (e.g. a partial/aborted build).
|
||||
const exists = vi.fn().mockReturnValue(false);
|
||||
const build = vi.fn().mockReturnValue(0);
|
||||
|
||||
expect(() =>
|
||||
ensureWebBuild(WEB_ROOT, DIST_ROOT, { exists, build, log: vi.fn() }),
|
||||
).toThrow(/Could not build the web UI/);
|
||||
expect(build).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Default seams (real `existsSync` / `spawnSync` / `console.log`) so the
|
||||
// production code path — not just the injected one — is exercised.
|
||||
describe("with default dependencies", () => {
|
||||
beforeEach(() => {
|
||||
existsSync.mockReset();
|
||||
spawnSync.mockReset();
|
||||
});
|
||||
|
||||
it("is a no-op when index.html already exists (no spawn)", () => {
|
||||
existsSync.mockReturnValue(true);
|
||||
|
||||
ensureWebBuild(WEB_ROOT, DIST_ROOT);
|
||||
|
||||
expect(existsSync).toHaveBeenCalledWith(INDEX_HTML);
|
||||
expect(spawnSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("spawns `npm run build:client` in the web root when missing", () => {
|
||||
existsSync.mockReturnValueOnce(false).mockReturnValueOnce(true);
|
||||
spawnSync.mockReturnValue({ status: 0 });
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
ensureWebBuild(WEB_ROOT, DIST_ROOT);
|
||||
} finally {
|
||||
logSpy.mockRestore();
|
||||
}
|
||||
|
||||
expect(spawnSync).toHaveBeenCalledWith(
|
||||
"npm",
|
||||
["run", "build:client"],
|
||||
expect.objectContaining({ cwd: WEB_ROOT, stdio: "inherit" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the spawned build exits non-zero", () => {
|
||||
existsSync.mockReturnValue(false);
|
||||
spawnSync.mockReturnValue({ status: 1 });
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
expect(() => ensureWebBuild(WEB_ROOT, DIST_ROOT)).toThrow(
|
||||
/Could not build the web UI automatically/,
|
||||
);
|
||||
} finally {
|
||||
logSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("surfaces the spawn error when the build can't start (ENOENT)", () => {
|
||||
existsSync.mockReturnValue(false);
|
||||
spawnSync.mockReturnValue({
|
||||
status: null,
|
||||
error: new Error("spawn npm ENOENT"),
|
||||
});
|
||||
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
expect(() => ensureWebBuild(WEB_ROOT, DIST_ROOT)).toThrow(
|
||||
/Could not build the web UI/,
|
||||
);
|
||||
expect(
|
||||
logSpy.mock.calls.some((c) =>
|
||||
String(c[0]).includes(
|
||||
"Web build failed to start: spawn npm ENOENT",
|
||||
),
|
||||
),
|
||||
).toBe(true);
|
||||
} finally {
|
||||
logSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -4,14 +4,18 @@ import { existsSync } from "node:fs";
|
|||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
const { mockClose, startViteDevServer, startHonoServer } = vi.hoisted(() => {
|
||||
const mockClose = vi.fn().mockResolvedValue(undefined);
|
||||
return {
|
||||
mockClose,
|
||||
startViteDevServer: vi.fn().mockResolvedValue({ close: mockClose }),
|
||||
startHonoServer: vi.fn().mockResolvedValue({ close: mockClose }),
|
||||
};
|
||||
});
|
||||
const { mockClose, startViteDevServer, startHonoServer, ensureWebBuild } =
|
||||
vi.hoisted(() => {
|
||||
const mockClose = vi.fn().mockResolvedValue(undefined);
|
||||
return {
|
||||
mockClose,
|
||||
startViteDevServer: vi.fn().mockResolvedValue({ close: mockClose }),
|
||||
startHonoServer: vi.fn().mockResolvedValue({ close: mockClose }),
|
||||
// No-op by default so prod tests don't shell out to a real build; the
|
||||
// build-on-demand contract is covered in ensure-web-build.test.ts.
|
||||
ensureWebBuild: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../../../../server/start-vite-dev-server.js", () => ({
|
||||
startViteDevServer,
|
||||
|
|
@ -21,6 +25,10 @@ vi.mock("../../../../server/server.js", () => ({
|
|||
startHonoServer,
|
||||
}));
|
||||
|
||||
vi.mock("../../../../server/ensure-web-build.js", () => ({
|
||||
ensureWebBuild,
|
||||
}));
|
||||
|
||||
import * as nodeConfig from "../../../../../../core/mcp/node/config.js";
|
||||
import { runWeb } from "../../../../server/run-web.js";
|
||||
|
||||
|
|
@ -39,6 +47,8 @@ describe("runWeb", () => {
|
|||
beforeEach(() => {
|
||||
startViteDevServer.mockClear();
|
||||
startHonoServer.mockClear();
|
||||
ensureWebBuild.mockClear();
|
||||
ensureWebBuild.mockReset();
|
||||
mockClose.mockClear();
|
||||
signalHandlers.clear();
|
||||
exitCode = undefined;
|
||||
|
|
@ -126,6 +136,32 @@ describe("runWeb", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("ensures the web build before starting Hono in production", async () => {
|
||||
void runWeb(["node", "run-web"]);
|
||||
await expectServerStarted(startHonoServer);
|
||||
|
||||
expect(ensureWebBuild).toHaveBeenCalledTimes(1);
|
||||
// (webRoot, distRoot) — both end at the web client root / its dist dir.
|
||||
const [webRoot, distRoot] = ensureWebBuild.mock.calls[0] as [
|
||||
string,
|
||||
string,
|
||||
];
|
||||
expect(distRoot).toMatch(/dist$/);
|
||||
expect(distRoot.startsWith(webRoot)).toBe(true);
|
||||
});
|
||||
|
||||
it("exits with an actionable error when the web build can't be produced", async () => {
|
||||
ensureWebBuild.mockImplementationOnce(() => {
|
||||
throw new Error("Could not build the web UI automatically.");
|
||||
});
|
||||
|
||||
await expect(runWeb(["node", "run-web"])).rejects.toThrow("process.exit:1");
|
||||
expect(
|
||||
errorLines.some((l) => l.includes("Could not build the web UI")),
|
||||
).toBe(true);
|
||||
expect(startHonoServer).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("starts Vite when --dev is set", async () => {
|
||||
void runWeb(["node", "run-web", "--dev"]);
|
||||
await expectServerStarted(startViteDevServer);
|
||||
|
|
@ -134,6 +170,8 @@ describe("runWeb", () => {
|
|||
expect.objectContaining({ initialMcpConfig: null, writable: true }),
|
||||
);
|
||||
expect(logLines.some((l) => l.includes("development mode"))).toBe(true);
|
||||
// --dev uses Vite directly; no static build is required or attempted.
|
||||
expect(ensureWebBuild).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// --- ad-hoc launch → read-only in-memory session ------------------------
|
||||
|
|
|
|||
Loading…
Reference in a new issue