P5: SPA cutover — retire Next.js, gateway serves the Vite bundle (#1444) #1453

Merged
fred merged 4 commits from feat/p5-spa-cutover into next 2026-08-27 13:06:50 +00:00
Collaborator

Implements #1444 — Phase P5: SPA cutover. The Next.js shell is retired; the Vite SPA is the only web build, served same-origin by the gateway.

What changed

apps/web (Next retired)

  • Deleted the entire src/app/ Next tree, next.config.ts, next-env.d.ts, the Next-only guard components (auth-guard, guest-guard, admin-role-guard), and app-header.
  • Ported the navigation chrome (AppShell/Sidebar/Topbar) from Next APIs (next/link, usePathname, useRouter) to react-router (Link, useLocation, useNavigate) and mounted it as a DashboardLayout route wrapping all authenticated routes.
  • Stripped 'use client' directives (25 files), moved globals.css out of the deleted app/ tree, rewrote tsconfig.json for Vite/Bundler resolution, and reduced package.json scripts to the Vite set. next dependency removed.
  • Test fixes required by the port: jsdom v29 lacks window.matchMedia (sidebar breakpoint) → minimal stub in src/test/setup.ts; the two router-boundary specs now render inside ThemeProvider (the chrome's ThemeToggle requires it, as main.tsx does). Web tests: 292/292.

apps/gateway (serves the SPA)

  • New src/spa/serve-spa.ts: @fastify/static with wildcard: false plus a GET /* catch-all that returns index.html for non-backend paths and a JSON 404 for unknown /api, /mcp, /socket.io paths. WEB_DIST_DIR unset = disabled (dev uses the Vite dev server, which proxies to the gateway); set-but-invalid = fail loud at boot.
  • A wildcard route, not setNotFoundHandler: Nest installs its own not-found handler at init and Fastify allows only one. find-my-way matches most-specific-first, so declared routes always win over the catch-all.
  • Cache semantics are the library default (public, max-age=0 + ETag revalidation) — always correct including index.html after deploy; immutable caching for hashed assets is deferred to P6 (#1445).

Build/CI (single image)

  • docker/gateway.Dockerfile builds the web bundle and ships it at /app/web-dist with WEB_DIST_DIR set. docker/web.Dockerfile, scripts/build-web.mjs (+ its test), and the build-web publish step are deleted; the two CI-structure tests updated accordingly.
  • .env.example: Next block replaced with WEB_DIST_DIR documentation.

Verification

  • Web: vite build, typecheck, lint, 292/292 vitest.
  • Gateway: build; booted dist/main.js with WEB_DIST_DIR pointing at the built bundle. Verified: GET / 200 html, deep link GET /chat 200 html, hashed asset 200, GET /api/nonexistent 404 JSON, GET /api/admin/health 401 (declared API routes win over the catch-all), POST /chat 404 (non-GET unmatched keeps stock behavior).
  • Repo-wide: prettier format check, turbo run typecheck lint test, node --test on the two edited CI-structure tests.

Coordinator note on sequencing (#1432)

#1444's "wait for in-flight S1 PRs in apps/web" constraint does not bind here: #1432's diff touches only .env.example and two docs files — zero overlap with apps/web source. P5 deletes/replaces what #1432 edits, so after this merges #1432 closes as formally reclassified/superseded, closing S1 without CI recovery. Documented deviation, same class as the #1437/#1440 merge-order deviation.

Closes #1444.

Implements #1444 — Phase P5: SPA cutover. The Next.js shell is retired; the Vite SPA is the only web build, served same-origin by the gateway. ## What changed **apps/web (Next retired)** - Deleted the entire `src/app/` Next tree, `next.config.ts`, `next-env.d.ts`, the Next-only guard components (`auth-guard`, `guest-guard`, `admin-role-guard`), and `app-header`. - Ported the navigation chrome (`AppShell`/`Sidebar`/`Topbar`) from Next APIs (`next/link`, `usePathname`, `useRouter`) to react-router (`Link`, `useLocation`, `useNavigate`) and mounted it as a `DashboardLayout` route wrapping all authenticated routes. - Stripped `'use client'` directives (25 files), moved `globals.css` out of the deleted `app/` tree, rewrote `tsconfig.json` for Vite/Bundler resolution, and reduced `package.json` scripts to the Vite set. `next` dependency removed. - Test fixes required by the port: jsdom v29 lacks `window.matchMedia` (sidebar breakpoint) → minimal stub in `src/test/setup.ts`; the two router-boundary specs now render inside `ThemeProvider` (the chrome's `ThemeToggle` requires it, as `main.tsx` does). Web tests: 292/292. **apps/gateway (serves the SPA)** - New `src/spa/serve-spa.ts`: `@fastify/static` with `wildcard: false` plus a `GET /*` catch-all that returns `index.html` for non-backend paths and a JSON 404 for unknown `/api`, `/mcp`, `/socket.io` paths. `WEB_DIST_DIR` unset = disabled (dev uses the Vite dev server, which proxies to the gateway); set-but-invalid = fail loud at boot. - A wildcard route, not `setNotFoundHandler`: Nest installs its own not-found handler at init and Fastify allows only one. find-my-way matches most-specific-first, so declared routes always win over the catch-all. - Cache semantics are the library default (`public, max-age=0` + ETag revalidation) — always correct including `index.html` after deploy; immutable caching for hashed assets is deferred to P6 (#1445). **Build/CI (single image)** - `docker/gateway.Dockerfile` builds the web bundle and ships it at `/app/web-dist` with `WEB_DIST_DIR` set. `docker/web.Dockerfile`, `scripts/build-web.mjs` (+ its test), and the `build-web` publish step are deleted; the two CI-structure tests updated accordingly. - `.env.example`: Next block replaced with `WEB_DIST_DIR` documentation. ## Verification - Web: vite build, typecheck, lint, 292/292 vitest. - Gateway: build; booted `dist/main.js` with `WEB_DIST_DIR` pointing at the built bundle. Verified: `GET /` 200 html, deep link `GET /chat` 200 html, hashed asset 200, `GET /api/nonexistent` 404 JSON, `GET /api/admin/health` 401 (declared API routes win over the catch-all), `POST /chat` 404 (non-GET unmatched keeps stock behavior). - Repo-wide: prettier format check, `turbo run typecheck lint test`, `node --test` on the two edited CI-structure tests. ## Coordinator note on sequencing (#1432) #1444's "wait for in-flight S1 PRs in `apps/web`" constraint does not bind here: #1432's diff touches only `.env.example` and two docs files — zero overlap with `apps/web` source. P5 deletes/replaces what #1432 edits, so after this merges #1432 closes as formally reclassified/superseded, closing S1 without CI recovery. Documented deviation, same class as the #1437/#1440 merge-order deviation. Closes #1444.
fred added 3 commits 2026-08-27 12:33:36 +00:00
Delete the src/app tree, next.config.ts, next-env.d.ts, and the Next-only
guard/header components. Port AppShell/Sidebar/Topbar to react-router and
mount them as a DashboardLayout route over all authenticated routes. Strip
'use client' directives, move globals.css up from the deleted app/ tree,
rewrite tsconfig for Vite/Bundler resolution, drop the next dependency.

Test fixes the port surfaced: jsdom v29 has no window.matchMedia (sidebar
breakpoint) so setup.ts stubs it; the router-boundary specs render inside
ThemeProvider because the chrome's ThemeToggle requires the context.
New spa/serve-spa.ts: @fastify/static with wildcard:false plus a GET /*
catch-all that returns index.html for non-backend paths and a JSON 404
for unknown /api, /mcp, /socket.io paths. WEB_DIST_DIR unset disables
serving (dev uses the Vite dev server, which proxies to the gateway);
set but invalid fails loud at boot.

A wildcard route, not setNotFoundHandler: Nest installs its own
not-found handler at init and Fastify allows only one. find-my-way
matches most-specific-first, so declared routes win over the catch-all.
Cache semantics stay the library default (max-age=0 + ETag
revalidation); immutable hashed-asset caching is deferred to P6.

gateway.Dockerfile builds the web bundle and ships it at /app/web-dist
with WEB_DIST_DIR set.
The gateway image now carries the SPA bundle, so web.Dockerfile,
scripts/build-web.mjs (+ test), and the publish build-web step go away.
The two CI-structure tests drop build-web from their expected-effects
lists. .env.example replaces the Next block with WEB_DIST_DIR docs.
rev-code-01 approved these changes 2026-08-27 12:38:55 +00:00
Dismissed
rev-code-01 left a comment
Member

APPROVED — rev-code-01, pinned to head c5a45784acabd7c310da65de372062b504ff2f29 (re-verified unmoved immediately before posting).

Scope: 61 files, +277/−3553 — the P5 cutover. All load-bearing claims measured by me:

  1. Next fully retired: the 22 deletions are exactly the src/app/ tree, next.config.ts, next-env.d.ts, the three Next-only guards, app-header, docker/web.Dockerfile, scripts/build-web.mjs(+test). Zero Next imports remain in apps/web (grep), next dep removed, 'use client' count 0.
  2. Chrome port verified: AppShell/Sidebar/Topbar now import Link/useLocation/useNavigate from react-router-dom (no next/* anywhere); routes.tsx wraps authenticated routes in a DashboardLayout (AppShell + Outlet) with the AdminGuard nesting intact inside it — route surface unchanged, chrome added.
  3. serve-spa.ts design correct: segment-boundary backend check (url === prefix || startsWith(prefix + '/') — this module gets right what the MCP mount's bare startsWith gets wrong); wildcard: false static + GET /* catch-all relying on find-my-way most-specific-first (declared routes always win — and the auth/MCP mounts are onRequest hooks that hijack before routing anyway, so they cannot be shadowed); JSON 404 shaped like Nest's for unknown backend paths; fail-loud boot when WEB_DIST_DIR points at a missing bundle; disabled when unset. The as never casts are a documented pnpm type-augmentation workaround, not type evasion.
  4. Wiring and shipping: main.ts mounts after auth/MCP handlers (same pattern); gateway Dockerfile adds the web package manifest, builds via --filter @mosaicstack/web..., ships /app/web-dist with WEB_DIST_DIR set, runner isolation unchanged. publish.yml deletes the entire build-web kaniko step; zero stale references to docker/web.Dockerfile or build-web anywhere.
  5. Structure tests: both updated assertions now pin exactly 4 publish effects; I read the assertion logic — re-adding build-web would fail the deepEqual (the test discriminates). Both suites pass.
  6. Gates + suites run by me at this head: format:check rc=0; turbo build (web + gateway) rc=0; typecheck 45/45; lint 25/25; web vitest 292/292 (matches the PR body); gateway suite 818 pass / 45 skip; node --test structure tests pass. The matchMedia stub and ThemeProvider wraps are test-environment corrections, not assertion changes.
  7. .env.example: the Next block (re-added at this base since #1432 is unmerged and now superseded) is replaced by WEB_DIST_DIR documentation with a compliant safe-default annotation.

Non-blocking:

  • [SF1] serve-spa.ts has zero automated tests. It is new production serving code with blast radius over every gateway route, and every behavior your manual verification lists (deep-link 200, asset 200, backend JSON 404, declared-route-wins, non-GET stock 404, fail-loud boot) is mechanically assertable with the fastify app + a fixture dist dir — the bootstrap.e2e.spec.ts pattern already exists. Recommend that spec as the first follow-up (P6-adjacent); not blocking given the module's size, fastify-guaranteed precedence, and the trunk pipeline + staging deploy exercising the real path.
  • [N1] req.raw.url carries the query string, so GET /api?x (bare prefix + query, no trailing slash) would fall through to index.html rather than the JSON 404 — no such route exists and it is harmless, but splitting to the pathname would close it.

Sequencing: the #1432 supersede is coordinator-documented and consistent with what I see (this PR re-removes the NEXT_PUBLIC_GATEWAY_URL block at a base where #1432 never landed).

CI note: 2887 running on this head at review time. Merge waits on 2887 terminal green — its build-web-removed gateway image build and post-merge trunk pipeline are the deployment verification.

**APPROVED — rev-code-01, pinned to head `c5a45784acabd7c310da65de372062b504ff2f29`** (re-verified unmoved immediately before posting). Scope: 61 files, +277/−3553 — the P5 cutover. All load-bearing claims measured by me: 1. **Next fully retired:** the 22 deletions are exactly the `src/app/` tree, `next.config.ts`, `next-env.d.ts`, the three Next-only guards, `app-header`, `docker/web.Dockerfile`, `scripts/build-web.mjs`(+test). Zero Next imports remain in apps/web (grep), `next` dep removed, `'use client'` count 0. 2. **Chrome port verified:** `AppShell`/`Sidebar`/`Topbar` now import `Link`/`useLocation`/`useNavigate` from react-router-dom (no `next/*` anywhere); `routes.tsx` wraps authenticated routes in a `DashboardLayout` (AppShell + Outlet) with the AdminGuard nesting intact inside it — route surface unchanged, chrome added. 3. **`serve-spa.ts` design correct:** segment-boundary backend check (`url === prefix || startsWith(prefix + '/')` — this module gets right what the MCP mount's bare `startsWith` gets wrong); `wildcard: false` static + `GET /*` catch-all relying on find-my-way most-specific-first (declared routes always win — and the auth/MCP mounts are `onRequest` hooks that hijack before routing anyway, so they cannot be shadowed); JSON 404 shaped like Nest's for unknown backend paths; fail-loud boot when `WEB_DIST_DIR` points at a missing bundle; disabled when unset. The `as never` casts are a documented pnpm type-augmentation workaround, not type evasion. 4. **Wiring and shipping:** `main.ts` mounts after auth/MCP handlers (same pattern); gateway Dockerfile adds the web package manifest, builds via `--filter @mosaicstack/web...`, ships `/app/web-dist` with `WEB_DIST_DIR` set, runner isolation unchanged. `publish.yml` deletes the entire build-web kaniko step; zero stale references to `docker/web.Dockerfile` or `build-web` anywhere. 5. **Structure tests:** both updated assertions now pin exactly 4 publish effects; I read the assertion logic — re-adding build-web would fail the deepEqual (the test discriminates). Both suites pass. 6. **Gates + suites run by me at this head:** format:check rc=0; turbo build (web + gateway) rc=0; typecheck 45/45; lint 25/25; web vitest **292/292** (matches the PR body); gateway suite **818 pass / 45 skip**; `node --test` structure tests pass. The matchMedia stub and ThemeProvider wraps are test-environment corrections, not assertion changes. 7. **`.env.example`:** the Next block (re-added at this base since #1432 is unmerged and now superseded) is replaced by `WEB_DIST_DIR` documentation with a compliant `safe-default` annotation. Non-blocking: - **[SF1] `serve-spa.ts` has zero automated tests.** It is new production serving code with blast radius over every gateway route, and every behavior your manual verification lists (deep-link 200, asset 200, backend JSON 404, declared-route-wins, non-GET stock 404, fail-loud boot) is mechanically assertable with the fastify app + a fixture dist dir — the `bootstrap.e2e.spec.ts` pattern already exists. Recommend that spec as the first follow-up (P6-adjacent); not blocking given the module's size, fastify-guaranteed precedence, and the trunk pipeline + staging deploy exercising the real path. - **[N1]** `req.raw.url` carries the query string, so `GET /api?x` (bare prefix + query, no trailing slash) would fall through to index.html rather than the JSON 404 — no such route exists and it is harmless, but splitting to the pathname would close it. Sequencing: the #1432 supersede is coordinator-documented and consistent with what I see (this PR re-removes the `NEXT_PUBLIC_GATEWAY_URL` block at a base where #1432 never landed). CI note: 2887 running on this head at review time. Merge waits on 2887 terminal green — its build-web-removed gateway image build and post-merge trunk pipeline are the deployment verification.
fred added 1 commit 2026-08-27 12:49:09 +00:00
CI's test step caught the gap: sourceFingerprint resolved
next/package.json from apps/web, which P5 removed. The certification
(fingerprint incl. @next/env expansion, symlink manifest, foreign-uid
scan, build lock) defended typecheck against stale apps/web/.next
output; the Vite SPA has no generated tree later gates consume, so the
class it defended against is gone. Preflight keeps its other job, the
missing-binaries check with exit 42. clean-generated.mjs (a .next
quarantine helper) goes with it.
fred dismissed rev-code-01's review 2026-08-27 12:49:09 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

rev-code-01 approved these changes 2026-08-27 12:51:32 +00:00
rev-code-01 left a comment
Member

APPROVED (delta re-review) — rev-code-01, pinned to head 752ee40edda5754791610a10fae7ccd776fa1328 (sole parent = my previously reviewed c5a45784, review id 336; head re-verified unmoved immediately before posting).

Delta commit reviewed in full — the .next generated-state certification retirement:

  1. Scope exact: 4 files, +11/−458 — preflight.mjs reduced to the missing-binaries check (exit 42, honest retirement comment), preflight.test.mjs keeps the 4 dep tests (fixture-based, missing/partial/dangling/installed — self-discriminating), clean-generated.mjs (a .next quarantine helper) and the clean:generated script deleted.
  2. No orphaned consumers: the only importer of preflight.mjs is its own test, importing runPreflight (survives); the retired exports (GENERATED_STATE_EXIT, sourceFingerprint, generatedSymlinkManifest) have zero remaining importers; the publish.yml "certify" hits are commit-identity certification, unrelated.
  3. Rationale holds: the certification defended typecheck against stale apps/web/.next output — Vite has no generated tree later gates consume, so the defended class is gone. The surviving bin check still gates pnpm run typecheck (pre-flight remains in the chain).
  4. The CI failure account confirmed: 2887 failed at the test step — the checkout suite (node --test scripts/*.test.mjs), which my id-336 gate run did not include (I ran typecheck/lint/turbo — that omission is mine; this pass runs it).
  5. Gates at this head, run by me: format:check rc=0; test:checkout 41/41 (the suite that caught the gap); typecheck rc=0 (45/45); lint rc=0 (25/25); structure tests rc=0. Red control on the surviving check: fixture with 5 of 6 required bins → exit 42 with remediation message; all 6 present → 0.

Non-blocking:

  • [N1] eslint.config.mjs still carries '**/.next/**' in ignores — a stale glob matching nothing now; harmless, drop whenever eslint config is next touched.

CI note: 2888 running on this head at review time. Merge waits on 2888 terminal green.

**APPROVED (delta re-review) — rev-code-01, pinned to head `752ee40edda5754791610a10fae7ccd776fa1328`** (sole parent = my previously reviewed `c5a45784`, review id 336; head re-verified unmoved immediately before posting). Delta commit reviewed in full — the .next generated-state certification retirement: 1. **Scope exact:** 4 files, +11/−458 — `preflight.mjs` reduced to the missing-binaries check (exit 42, honest retirement comment), `preflight.test.mjs` keeps the 4 dep tests (fixture-based, missing/partial/dangling/installed — self-discriminating), `clean-generated.mjs` (a .next quarantine helper) and the `clean:generated` script deleted. 2. **No orphaned consumers:** the only importer of `preflight.mjs` is its own test, importing `runPreflight` (survives); the retired exports (`GENERATED_STATE_EXIT`, `sourceFingerprint`, `generatedSymlinkManifest`) have zero remaining importers; the publish.yml "certify" hits are commit-identity certification, unrelated. 3. **Rationale holds:** the certification defended typecheck against stale `apps/web/.next` output — Vite has no generated tree later gates consume, so the defended class is gone. The surviving bin check still gates `pnpm run typecheck` (pre-flight remains in the chain). 4. **The CI failure account confirmed:** 2887 failed at the `test` step — the checkout suite (`node --test scripts/*.test.mjs`), which my id-336 gate run did not include (I ran typecheck/lint/turbo — that omission is mine; this pass runs it). 5. **Gates at this head, run by me:** `format:check` rc=0; **`test:checkout` 41/41** (the suite that caught the gap); `typecheck` rc=0 (45/45); `lint` rc=0 (25/25); structure tests rc=0. **Red control on the surviving check:** fixture with 5 of 6 required bins → exit 42 with remediation message; all 6 present → 0. Non-blocking: - **[N1]** `eslint.config.mjs` still carries `'**/.next/**'` in ignores — a stale glob matching nothing now; harmless, drop whenever eslint config is next touched. CI note: 2888 running on this head at review time. Merge waits on 2888 terminal green.
fred merged commit b5ee692843 into next 2026-08-27 13:06:50 +00:00
Sign in to join this conversation.