feat(web): build dist on demand for prod --web + end-to-end CI smoke (#1486)

[cliffhall]

Closes #1486.

Problem

Prod mcp-inspector --web (no --dev) serves static assets from clients/web/dist/, which only exists after npm run build. In a fresh dev checkout dist/ is absent and the prod path served a broken page (serveIndexHtml ENOENT on index.html). The launcher smoke test only checked --help / --cli --help / --tui --help, so the prod web path was never exercised in CI.

Changes

Build on demand (or fail clearly):

• clients/web/server/ensure-web-build.ts (new) — when dist/index.html is missing, builds it on demand via npm run build:client (vite build). If the build can't run (e.g. a stripped published install with no dev deps), throws an actionable error pointing at npm run build / --dev instead of serving a broken page.
• run-web.ts — calls ensureWebBuild() in the prod path before startHonoServer; --dev skips it (Vite serves directly).
• clients/web/package.json — added build:client (vite build, assets-only).

End-to-end CI coverage:

• scripts/smoke-web.mjs + smoke:web (new) — starts mcp-inspector --web against built dist, asserts GET / returns HTTP 200 with the injected __INSPECTOR_API_TOKEN__, then shuts down.
• Wired into validate:launcher and a new .github/workflows/main.yml step.

Docs: launcher README ("Production web build" section) + AGENTS.md.

Acceptance criteria

• --web works after build-on-demand, or fails with a clear, actionable error when dist/ is missing (no silent broken page).
• CI exercises the prod web launcher path end-to-end against a built dist/ (serves index.html 200 with injected token).
• Behavior documented in AGENTS.md / launcher README.

Testing

• New ensure-web-build.test.ts (8 tests) covers injected seams and real defaults (existsSync/spawnSync/console.log).
• Extended run-web.test.ts: verifies the helper is invoked in prod, skipped in --dev, and that a build failure exits with the actionable error.
• Full web suite: 2391 tests pass, coverage gate clean.
• Verified smoke:web end-to-end, and confirmed build-on-demand by removing dist and watching --web auto-run vite build and serve 200.

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 42s —— View job

---

Code Review — PR #1497

• Read the diff against origin/v2/main
• Review new ensure-web-build.ts + tests
• Review run-web.ts changes
• Review smoke-web.mjs + CI wiring
• Review docs/package.json changes
• Post review feedback

---

Solid, well-scoped change. The build-on-demand seam is clean, the actionable-error path is exactly the right call (no silent broken page), and the testing is thorough — injected seams for logic plus default-dependency tests so the real existsSync/spawnSync/console.log path is exercised. The end-to-end smoke meaningfully closes the gap the --help-only smoke left. No blocking issues.

Things I like

• Right build target. build:client = vite build only (no tsc -b/build:runner) is correct: by the time ensureWebBuild runs, the runner is already executing, so only the static dist/ assets are needed. Skipping tsc -b also avoids a typecheck failure blocking the asset build.
• status !== 0 || !exists(indexHtml) double-check (ensure-web-build.ts:60) correctly catches the partial/aborted-build case where the child exits 0 but produces no index.html. Nice, and explicitly tested.
• Windows handled — shell: process.platform === "win32" for the npm→npm.cmd case.
• --dev skip is verified by an explicit negative assertion in the test, not just left implicit.

Minor suggestions (non-blocking)

1. spawnSync error is dropped. runViteBuild (ensure-web-build.ts:33-40) returns only result.status and discards result.error. On a spawn failure (e.g. npm not on PATH → ENOENT), status is null and the user gets the generic actionable message, but the underlying cause never surfaces. Consider logging result.error?.message before throwing to aid debugging.

   const result = spawnSync("npm", ["run", "build:client"], { ... });
   if (result.error) log(`build:client failed to spawn: ${result.error.message}`);
   return result.status;

   Fix this →

2. Smoke timeout vs. cold build-on-demand. scripts/smoke-web.mjs:62 retries GET / for ~60s. In CI/validate dist/ is pre-built so ensureWebBuild is a no-op and 60s is plenty. But the script's own header comment says it covers the case where "dist is missing the launcher's build-on-demand path builds it" — a cold vite build can exceed 60s, in which case the smoke would fail with the misleading "server did not start within 60s". Since the script never actually runs without a pre-built dist in practice, this is theoretical, but the comment slightly oversells. Either drop that line from the comment or bump the budget for the build-on-demand scenario.

3. ensureWebBuild blocks the event loop via spawnSync with stdio: "inherit". Fine for a one-shot CLI startup (and the inherited stdio gives the user live build output), just noting it's intentional.

Things I verified

• The thrown error is correctly caught by the existing try/catch in runWeb (run-web.ts:264-269) and surfaced via console.error("Error:", …) + process.exit(1) — covered by the new "exits with an actionable error" test.
• validate:launcher now ends with smoke:web, and validate:web (which builds dist/) runs earlier in the validate chain, so the ordering dependency holds. The CI step is placed after "Run Build", so dist/ and clients/launcher/build both exist. ✅
• Docs (AGENTS.md, launcher README) accurately describe the new behavior.

Nice work — the suggestions above are all polish, nothing needs to change to merge.
• feat/web-build-on-demand-1486

[cliffhall]

Thanks for the review! Addressed both actionable suggestions in b1b39bc:

1. spawnSync error dropped — fixed. runViteBuild now logs result.error.message before returning when the child can't be spawned (e.g. npm not on PATH → ENOENT, where status is null), so the underlying cause surfaces ahead of the generic actionable message. The build seam gained a log parameter, and there's a new test (surfaces the spawn error when the build can't start (ENOENT)) asserting the message is emitted.

2. Smoke timeout vs. cold build comment — fixed. Reworded the scripts/smoke-web.mjs header so it no longer implies the readiness wait covers a cold build-on-demand vite build. The wait is sized for an already-built dist (which validate/CI guarantee); the build-on-demand fallback isn't a scenario this script targets, and the comment now says so rather than overselling.

3. spawnSync blocks the event loop — acknowledged, no change. Intentional, as you noted: it's a one-shot CLI startup and the inherited stdio gives the user live build output. Left as-is.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 40s —— View job

---

Re-review — PR #1497

• Read updated diff against origin/v2/main
• Review follow-up commit b1b39bc (spawn-error logging, smoke comment)
• Post review feedback

---

Both actionable suggestions from the prior review are addressed cleanly in b1b39bc4. Re-reviewed the full diff — no remaining issues, ready to merge.

Verified the follow-ups

1. Spawn-error surfacing ✅ — runViteBuild now takes a log seam and emits Web build failed to start: ${result.error.message} before returning, so an ENOENT (npm not on PATH) surfaces its cause ahead of the generic actionable error (ensure-web-build.ts:40-42). The build seam signature ((webRoot, log) => number | null) and the EnsureWebBuildDeps.log injection are consistent, and the no-op path still never logs. Covered by the new surfaces the spawn error when the build can't start (ENOENT) test, which asserts both the throw and the logged message via the real default spawnSync/console.log seams (ensure-web-build.test.ts:124-146). Good — this exercises the production path, not just the injected one.

2. Smoke comment reworded ✅ — the scripts/smoke-web.mjs header no longer implies the 60s readiness wait covers a cold build-on-demand vite build; it now states the wait is sized for an already-built dist (which validate/CI guarantee) and explicitly scopes out the build-on-demand-on-startup case (smoke-web.mjs:13-16). Matches the actual loop budget (120 attempts × 500ms = 60s, message at line 76).

3. spawnSync blocking — acknowledged as intentional; agreed, correct for one-shot CLI startup with inherited live build output.

Other things I re-checked

• The thrown error is still caught by runWeb's try/catch and surfaced via console.error("Error:", …) + process.exit(1) (run-web.ts:264-269); ensureWebBuild runs only on the prod branch, after --dev short-circuits to Vite (run-web.ts:256-262).
• Test coverage for ensure-web-build.ts is comprehensive across both injected and default seams: no-op, build-then-proceed, non-zero exit, null status, exit-0-but-no-index.html, and the new ENOENT case.
• log defaults to console.log and is threaded through both seams; no any, no suppressions — consistent with the TS guidelines.

Nice, tight follow-up. Nothing blocking.
• feat/web-build-on-demand-1486

[cliffhall]

Thanks for the re-review — glad the follow-ups landed cleanly. No further changes; this is ready to merge.