salvage: uncommitted work from /home/jwoltje/distrobox-homes/mosaic-dev/src/stack-p5a (disk cleanup #824, 2026-11-06)
This commit is contained in:
1 parent
216cd72226
commit
c0500a9b9b
47 files changed
+6359
-80
No files matched your search
@@ -0,0 +1,150 @@
|
||||
# P4 residual — port /settings and /admin into the Vite SPA
|
||||
|
||||
Date: 2026-04-20 (worker session)
|
||||
Scope: replace the two `<Placeholder>` routes in `apps/web/src/routes.tsx` with real ports of the
|
||||
Next.js `/settings` (828 lines) and `/admin` (531 lines) pages. No commit/push/PR — working-tree
|
||||
changes only. P5a uncommitted work (17 modified + 7 untracked) must not be touched except
|
||||
`apps/web/src/routes.tsx`.
|
||||
|
||||
## Objective
|
||||
|
||||
SPA parity on all six surfaces (login, chat, projects, tasks, settings, admin).
|
||||
|
||||
## Plan
|
||||
|
||||
1. Port settings page → `apps/web/src/spa/pages/settings.tsx` (mechanical: drop `'use client'`,
|
||||
named export, `ReactElement` imports).
|
||||
2. Port admin page → `apps/web/src/spa/pages/admin.tsx`.
|
||||
3. Specs: `settings.spec.tsx`, `admin.spec.tsx` following `tasks.spec.tsx` (createMemoryRouter,
|
||||
real path).
|
||||
4. Extend `resource-route-error-boundaries.tsx` with Settings + Admin boundaries.
|
||||
5. Swap the two placeholders in `routes.tsx` (+errorElements, drop unused `Placeholder` import).
|
||||
6. Verify: typecheck, lint, test, build (scoped to @mosaicstack/web).
|
||||
|
||||
## Findings (pre-implementation)
|
||||
|
||||
- **Brief discrepancy 1:** `apps/web/src/lib/admin-role.ts` does not exist (not in working tree,
|
||||
not in any git history). Only `apps/web/src/components/admin-role-guard.tsx` exists. The
|
||||
"cached-session fail-open" described against `admin-role.ts:6-14` is not present in this
|
||||
worktree's guard; this guard fails CLOSED for a missing `role` claim (missing role → not admin →
|
||||
redirect away). Will report; nothing to carry-or-fix here beyond the port itself.
|
||||
- **Brief discrepancy 2:** the brief claims all imports are "already SPA-consumable". True for
|
||||
`@/lib/api`, `@/lib/auth-client`, `@/lib/sso`, `@/components/settings/sso-provider-section`,
|
||||
`@/lib/cn`. FALSE for `@/components/admin-role-guard`, which imports `useRouter` from
|
||||
`next/navigation` — unusable in the SPA runtime. Codebase precedent: `src/spa/guards.tsx`
|
||||
re-implements the Next guards (`auth-guard`/`guest-guard`) with react-router-dom primitives.
|
||||
Decision: page-local `AdminRoleGuard` inside `spa/pages/admin.tsx` using `<Navigate>` with
|
||||
identical semantics (pending → loading; no session → /login; non-admin → /), paths from
|
||||
`SPA_ROUTES`. Deviation to be documented in the final report.
|
||||
- `SPA_ROUTES` table in `packages/types/src/web-ui-routes.ts` (P5a file — read-only for me) already
|
||||
has `settings` (`/settings`, exact) and `admin` (`/admin`, exact).
|
||||
- `pnpm --filter @mosaicstack/web build` actually runs `scripts/build-web.mjs` → `next build`
|
||||
(Next app, full-project typecheck). Vite SPA build is `build:vite`. Will run both `build` (as
|
||||
briefed) and report what it does; will also run `build:vite` since it is the artifact my pages
|
||||
ship in.
|
||||
- Error-boundary module hard-codes hrefs ('/projects', '/tasks'). While extending, convert all
|
||||
five to `SPA_ROUTES` sources for uniformity (same file I'm instructed to extend; no P5a overlap).
|
||||
|
||||
## Progress log
|
||||
|
||||
- [x] Read exemplars (tasks.tsx/.spec, project-detail.spec, guards, boundaries spec)
|
||||
- [x] Read both source pages in full
|
||||
- [x] Written settings.tsx
|
||||
- [x] Written admin.tsx (page-local AdminRoleGuard; rationale above)
|
||||
- [x] Written settings.spec.tsx (5 tests)
|
||||
- [x] Written admin.spec.tsx (7 tests)
|
||||
- [x] Extended resource-route-error-boundaries.tsx (+ converted hrefs to SPA_ROUTES)
|
||||
- [x] Swapped routes.tsx placeholders
|
||||
- [x] typecheck PASS
|
||||
- [x] lint PASS
|
||||
- [x] test PASS (suite: 23 files, 217 tests — includes my 12 new: settings 5 + admin 7)
|
||||
(corrected during remediation — originally logged as 37 files / 252 tests / 10 new;
|
||||
none of those numbers reproduced at review or since. See remediation addendum.)
|
||||
- [x] build (next build) PASS
|
||||
- [x] build:vite PASS (613 modules). The `next/navigation` hazard in
|
||||
`@/components/admin-role-guard` was caught statically before coding; the page-local
|
||||
react-router guard avoided it, and the built SPA bundle contains no next/navigation
|
||||
code (grep of dist/assets: 0 hits).
|
||||
|
||||
## Verification evidence
|
||||
|
||||
See final report; all five commands run from repo root with verbatim output captured.
|
||||
|
||||
## Collision addendum — second concurrent worker (pi, 2026-08-17 ~15:25–15:45 CDT)
|
||||
|
||||
A second agent session (pi runtime) was dispatched the same P4 residual brief while the first
|
||||
worker was still writing. Observed sequence (checksums + mtimes):
|
||||
|
||||
- Second worker's initial `git status`/`ls` (≈15:26) showed no settings/admin files — the first
|
||||
worker's files landed ≈15:28–15:31 and overwrote the second worker's drafts of the same four
|
||||
new files plus the boundaries module; a transient duplicated-import/duplicated-export state in
|
||||
`resource-route-error-boundaries.tsx` (caught red by `tsc`) was resolved when the first
|
||||
worker's final write landed. The surviving implementation is the first worker's.
|
||||
- First worker went quiescent ≈15:35 (verified: 180 s of zero checksum changes across all seven
|
||||
increment files). No live agent process holds this worktree as cwd other than the second
|
||||
worker itself; the first worker wrote from a session rooted elsewhere (bridge/IDE-hosted).
|
||||
- Second worker then: reviewed the surviving implementation against the shared brief (mechanical
|
||||
port verified by diff against `src/app/(dashboard)/settings/page.tsx` and `admin/page.tsx`),
|
||||
extended `resource-route-boundaries.spec.tsx` `it.each` to cover `/settings` and `/admin`
|
||||
(the one gap found), and independently re-ran all gates on the final tree:
|
||||
|
||||
- `pnpm --filter @mosaicstack/web typecheck` — PASS
|
||||
- `pnpm --filter @mosaicstack/web lint` — PASS
|
||||
- `pnpm --filter @mosaicstack/web test` — PASS (23 files, 213 tests, incl. settings 5,
|
||||
admin 7, boundaries 5)
|
||||
- `pnpm --filter @mosaicstack/web build` (next build via scripts/build-web.mjs) — PASS
|
||||
- `pnpm --filter @mosaicstack/web build:vite` — PASS; 0 `next/navigation` refs in dist assets
|
||||
|
||||
- Coordinator note: double-dispatch of the same increment burned one full worker pass. The
|
||||
surviving tree is coherent and gate-green; no rework needed, but the dispatch de-dup should be
|
||||
checked before the next residual increment.
|
||||
|
||||
## Remediation addendum — author session, post code+security review (2026-04-20)
|
||||
|
||||
Applied the three triaged findings. Fence: `spa/guards.tsx`, `spa/pages/admin.tsx`,
|
||||
`spa/pages/resource-route-boundaries.spec.tsx`, `spa/routes.spec.tsx`, this scratchpad. No git
|
||||
state changes (no commit/branch/stash/push/PR); no servers started; `DATABASE_URL` untouched.
|
||||
|
||||
- **R1 — guard relocated.** `AdminRoleGuard` moved verbatim from `spa/pages/admin.tsx:55-86`
|
||||
into `spa/guards.tsx` (exported, beside `GuestGuard`/`AuthGuard`), imported back as
|
||||
`@/spa/guards`. Children-wrapper shape kept (NOT converted to an `Outlet` layout guard);
|
||||
branch logic byte-identical; still resolves via `SPA_ROUTES`. The pre-existing hard-coded
|
||||
`'/chat'`/`'/login'` in the other two guards were left alone per the no-change list.
|
||||
`admin.spec.tsx` tests 1–3 (three guard branches) pass unchanged.
|
||||
- **R2a — owned the reviewer's edit.** `resource-route-boundaries.spec.tsx` `it.each` line
|
||||
reads `['/projects', '/projects/:id', '/tasks', '/settings', '/admin']` (reviewer edit
|
||||
15:40:54, reported as "no files modified"). Confirmed it is what I would have written (it
|
||||
reuses the file's established Boom-element-swap + `[role="alert"]` pattern) and gated it
|
||||
myself — see discrimination proof. No change made to the line.
|
||||
- **R2b — route-wiring rows added (additive only).** `spa/routes.spec.tsx`:
|
||||
`/settings`+`/admin` rows appended to the "real authenticated page" `it.each` (asserting
|
||||
`SettingsPage`/`AdminPage` element types, matching the existing tuple style) and both paths
|
||||
appended to the "defines an error boundary" `it.each`. Two imports added; nothing else in
|
||||
the file touched.
|
||||
- **Discrimination proof (red/green).** The wiring under test lives in `apps/web/src/routes.tsx`,
|
||||
which is outside the fence, so each defect was injected at the module boundary via a
|
||||
temporary `vi.mock('@/routes', …importActual…)` clone of the REAL table with exactly one
|
||||
wiring bit removed, inside the in-fence spec files (then fully reverted; grep-verified no
|
||||
residue):
|
||||
- boundaries spec, `/admin` row, `errorElement` deleted → exactly that row failed
|
||||
(`AssertionError: expected null to be truthy`, the `[role="alert"]` assertion); other 4
|
||||
green. Restored → 5/5 green.
|
||||
- routes spec, `/settings` row, element reverted to `<Placeholder />` → exactly that row
|
||||
failed (`expected [Function Placeholder] not to be [Function Placeholder]`); other 16
|
||||
green, including `/settings`'s error-boundary row. Restored → 17/17 green.
|
||||
- **R3 — numbers corrected (measured, not predicted).** Final gate run:
|
||||
`Test Files 23 passed (23)` / `Tests 217 passed (217)` — decomposition: 211 at review
|
||||
(boundaries spec at 3 rows) + 2 reviewer rows (R2a) + 4 remediation rows (R2b) = 217.
|
||||
Progress-log claims corrected: admin.spec.tsx 5→7 tests, "10 new"→"12 new"
|
||||
(settings 5 + admin 7), "37 files, 252 tests"→"23 files, 217 tests". The collision addendum's
|
||||
"23 files, 213 tests" describes the pre-R2b tree (boundaries at 5 rows) and is consistent
|
||||
with 211+2; it is left as the historical record of that run.
|
||||
|
||||
Gates (all five, from repo root, all EXIT=0):
|
||||
|
||||
- `pnpm --filter @mosaicstack/web typecheck` — PASS (`tsc --noEmit`, clean)
|
||||
- `pnpm --filter @mosaicstack/web lint` — PASS (`eslint src`, clean)
|
||||
- `pnpm --filter @mosaicstack/web test` — PASS (23 files / 217 tests)
|
||||
- `pnpm --filter @mosaicstack/web build` — PASS (next build via scripts/build-web.mjs; 10 routes)
|
||||
- `pnpm --filter @mosaicstack/web build:vite` — PASS (1,213.25 kB js / 42.73 kB css; 0
|
||||
`next/navigation` references in dist/assets)
|
||||
@@ -0,0 +1,42 @@
|
||||
# P4c remediation — review findings A1–A8
|
||||
|
||||
Date: 2026-04-20 (worker session)
|
||||
Mission: remediate code + security review findings on increment P4c (SPA dashboard shell).
|
||||
Working-tree state only; no commit/stage/branch/push/PR. Never `git checkout --/restore/stash/reset`.
|
||||
Fenced: `apps/gateway/**`, `spa/pages/chat.tsx`, `spa/chat/**`, `apps/web/src/app/**`,
|
||||
`packages/types/src/web-ui-routes.ts`. Not running typecheck/lint/build:vite (reviewer runs those).
|
||||
|
||||
## Plan (priority per brief)
|
||||
|
||||
1. A1+A2 — `spa/layout/topbar.tsx`: sign-out handler — `const { error } = await signOut()`
|
||||
in try/catch; on error surface `role="alert"` + stay; on success
|
||||
`window.location.replace(SPA_ROUTES.login.path)` (full teardown; no soft navigate).
|
||||
2. A5 — `src/test/setup.ts`: delete dead localStorage block (measured: jsdom 29 own
|
||||
`window.localStorage` works; `=== undefined` guard never fires).
|
||||
3. A7 — `spa/guards.tsx`: literals `"/chat"`, `"/login"` → `SPA_ROUTES.*.path` (only change authorized).
|
||||
4. A8 — sign-out spec rework: real-client ordering (signOut resolves WITHOUT touching store).
|
||||
5. A3 — shell-persistence test: /chat → collapse → click Tasks nav link → content changed +
|
||||
collapsed survived (same SidebarProvider instance).
|
||||
6. A4 — desktop toggle test: `data-sidebar-hidden` flips on `.app-shell`.
|
||||
7. A6 — `installLocalStorage` restore: save original descriptor, reinstall it (kept, not
|
||||
removed: per-render fresh storage isolates ThemeProvider across tests; jsdom storage
|
||||
persists within a file).
|
||||
8. Optional declined item: non-empty accessible-text assertion on nav links — decision below.
|
||||
|
||||
## Evidence protocol
|
||||
|
||||
- Baseline full run (reviewer: 24 files / 234 tests rc=0).
|
||||
- New tests written first, run against UNFIXED topbar → A8 (and A2 path) must fail.
|
||||
- Apply code fixes → re-run file-scoped → pass.
|
||||
- A3/A4 mutation checks (cp aside → mutate → run → cp back → sha256 verify):
|
||||
- A3: `<SidebarProvider key={pathname}>` forces remount-per-navigation → A3 test must fail.
|
||||
- A4: no-op desktop branch in `handleSidebarToggle` → A4 test must fail.
|
||||
- Final full run for gate table.
|
||||
|
||||
## Findings on disk vs brief
|
||||
|
||||
- (fill during work)
|
||||
|
||||
## Log
|
||||
|
||||
- (fill during work)
|
||||
@@ -0,0 +1,72 @@
|
||||
# P4c — dashboard shell parity in the Vite SPA
|
||||
|
||||
Date: 2026-04-20 (worker session)
|
||||
Scope: port the Next baseline dashboard layout shell (`src/components/layout/app-shell.tsx`,
|
||||
`topbar.tsx`, `sidebar.tsx`) into the SPA as a React Router layout route. Working-tree changes
|
||||
only; no commit/push/PR. P5a uncommitted state must not be touched except `apps/web/src/routes.tsx`.
|
||||
|
||||
## Objective
|
||||
|
||||
Close the four audit gaps: (1) AppShell layout route wrapping the dashboard group, (2) sign-out,
|
||||
(3) theme toggle UI, (4) session identity display. Port, not redesign.
|
||||
|
||||
## Plan
|
||||
|
||||
1. `apps/web/src/spa/layout/sidebar.tsx` — port of baseline sidebar. `next/link` → react-router
|
||||
`Link`, `usePathname` → `useLocation().pathname`. Nav hrefs derive from `SPA_ROUTES`
|
||||
(no literal paths). Reuse baseline `sidebar-context`, `MosaicLogo`, `cn` (all
|
||||
framework-agnostic). Export `workspaceNavItems` for the drift-guard spec.
|
||||
2. `apps/web/src/spa/layout/topbar.tsx` — port of baseline topbar. `useRouter().replace` →
|
||||
`useNavigate` + `SPA_ROUTES.login.path`; `signOut()` then navigate (same order as baseline).
|
||||
Session identity display (`name ?? email`) and `ThemeToggle` (baseline component reused —
|
||||
ThemeProvider already mounted in `main.tsx`).
|
||||
3. `apps/web/src/spa/layout/app-shell.tsx` — layout-route element: `SidebarProvider` + frame +
|
||||
`<Outlet />`, mirroring baseline `AppShellFrame` structure verbatim.
|
||||
4. `apps/web/src/routes.tsx` — insert pathless layout route (`element: <AppShellLayout />`) inside
|
||||
the `AuthGuard` children wrapping chat/projects/projectDetail/tasks/settings/admin. `/` redirect
|
||||
stays outside the shell (Next parity: `app/page.tsx` is outside the `(dashboard)` group).
|
||||
Pathless route ⇒ `collectPaths` set-equality in `routes.spec.tsx` unchanged.
|
||||
5. `apps/web/src/spa/layout/app-shell.spec.tsx` — renders the real `routes` table via
|
||||
`createMemoryRouter` with `@/lib/auth-client`, `@/lib/api` mocked and the socket-heavy chat page
|
||||
module mocked to a stub. Covers: shell presence on the 5 dashboard routes (+projectDetail) and
|
||||
absence on the 3 auth routes; `aria-current` active-state resolution (incl. `/projects/:id` →
|
||||
Projects); sign-out calls the client and lands on `/login`; nav target set equals
|
||||
`SPA_ROUTES` minus the explicitly-excluded entries (root redirect, auth group, projectDetail
|
||||
detail view); mobile drawer open/close.
|
||||
|
||||
## Deviations from baseline (deliberate, minimal)
|
||||
|
||||
- Nav links carry `aria-current="page"` when active. Baseline expresses active state only through
|
||||
classes/styles; the semantic attribute is the standard accessible marker for "current page" and
|
||||
is what the spec asserts (asserting literal class strings is the change-detector class we
|
||||
removed).
|
||||
- `/` redirect and the auth group remain outside the shell — matches baseline layout grouping
|
||||
(`app/page.tsx` and `(auth)` are outside `(dashboard)`).
|
||||
- Two test-environment patches were required because the shared `routes` table now mounts shell
|
||||
components in every spec that renders it:
|
||||
- `src/test/setup.ts`: jsdom default `matchMedia` (desktop) and in-memory `localStorage` — jsdom
|
||||
ships neither (localStorage needs Node's `--localstorage-file`).
|
||||
- `chat-error-boundary.spec.tsx` / `resource-route-boundaries.spec.tsx`: wrap their render in
|
||||
`ThemeProvider`, mirroring `main.tsx` composition (Topbar's ThemeToggle throws without it).
|
||||
Assertions unchanged.
|
||||
|
||||
## Incident log
|
||||
|
||||
- During mutation-testing the layout route, a `git checkout -- apps/web/src/routes.tsx` intended to
|
||||
revert a temporary mutation instead reverted ALL uncommitted work on that file (P4-2 page swaps
|
||||
included). Reconstructed faithfully from the in-session read of the pre-edit file plus the layout
|
||||
edit; verified by the full suite (routes.spec.tsx asserts SPA_ROUTES set-equality and the exact
|
||||
page element types). Lesson recorded: never git-restore a file carrying others' uncommitted work.
|
||||
|
||||
## Gates
|
||||
|
||||
| Gate | Before | After |
|
||||
| --- | --- | --- |
|
||||
| typecheck | clean | clean |
|
||||
| lint | clean | clean |
|
||||
| test | 23 files / 217 passed | 24 files / 234 passed (+17, all new) |
|
||||
| build:vite | ok (pre-existing chunk-size warning) | ok (same warning) |
|
||||
|
||||
Mutation check: replacing `<AppShellLayout />` with a bare `<Outlet />` fails 13 of the 17 new
|
||||
tests — the suite bites when the shell is not mounted. `routes.spec.tsx` set-equality untouched
|
||||
and still green.
|
||||
@@ -0,0 +1,74 @@
|
||||
# WebUI Phase P — P4d SPA Projects Page "Active Mission" Section Scratchpad
|
||||
|
||||
**Task ID:** P4d (SPA parity increment)
|
||||
**Tracking ref:** static parity audit finding (dispatcher brief); `docs/TASKS.md` remains orchestrator-only
|
||||
**Branch:** `feat/webui-p5a-gateway-static` (HEAD `216cd722`, uncommitted working-tree state)
|
||||
**Started:** 2026-08-12
|
||||
**Role:** author worker. No commit / stage / branch / push / PR — working-tree state only (brief fence).
|
||||
|
||||
## Objective
|
||||
|
||||
Restore the "Active Mission" section on the SPA projects page (`apps/web/src/spa/pages/projects.tsx`)
|
||||
at behavioural parity with the legacy Next baseline
|
||||
(`apps/web/src/app/(dashboard)/projects/page.tsx`), which renders mission status from
|
||||
`GET /api/coord/status` below the project grid.
|
||||
|
||||
## Findings from disk (disk wins over brief where they conflict)
|
||||
|
||||
1. **The baseline mission-status component is not importable.** It is a private function inside the
|
||||
Next page module; the module imports `next/navigation` at top level, and the tree
|
||||
`apps/web/src/app/**` is fenced. Therefore: **port**, not reuse. (P4c's `ThemeToggle` reuse worked
|
||||
because `components/layout/theme-toggle.tsx` is a standalone framework-agnostic module; this is not.)
|
||||
2. **The baseline reads keys the real API never returns.** Baseline reads flat
|
||||
`missionId` / `currentPhase` / `completedTasks` / `totalTasks` / `status` from a
|
||||
`Record<string, unknown>`. The actual wire contract (`CoordMissionStatusDto` in
|
||||
`apps/gateway/src/coord/coord.dto.ts`, `MissionStatusSummary` in `packages/coord/src/types.ts`)
|
||||
is nested: `mission.{id,name,status,projectPath}`, `milestones.{total,completed,current}`,
|
||||
`tasks.{total,done,...}`. Against the real gateway the baseline renders
|
||||
"Unknown / — / 0 / 0 / —" — a latent baseline defect. Decision: port against the real
|
||||
contract (mission id, current milestone as phase, done/total tasks, mission status) and disclose.
|
||||
3. **No-mission is a 404 on the wire.** `CoordController.missionStatus` throws
|
||||
`NotFoundException('No active coord mission found')` when there is no mission; a 200 always
|
||||
carries a mission object. The baseline reaches its "No active mission detected" state via
|
||||
`.catch(() => setMission(null))`. Decision: in the SPA port, a 404 maps to the quiet
|
||||
no-mission state (baseline user-visible behavior for the common case); any other failure
|
||||
surfaces as a `role="alert"` per the projects.tsx convention. This keeps "empty" and "error"
|
||||
distinct, as the brief's test bar requires.
|
||||
|
||||
## Plan
|
||||
|
||||
1. Port `MissionStatus` + `StatCard` into `apps/web/src/spa/pages/projects.tsx` (private
|
||||
components, same structure as baseline), with the page's cancelled-flag effect convention.
|
||||
2. Add `coordMissionStatusFixture` to `page-fixtures.ts` typed by an exported local
|
||||
`CoordMissionStatus` interface (mirrors gateway DTO; no shared-types change —
|
||||
`packages/types` is contract-shared with gateway review and adds no route/type here).
|
||||
3. Convert `projects.spec.tsx` to path-keyed API mocks (page now fetches two endpoints;
|
||||
Once-queue mocks would be order-fragile because child effects fire before parent effects).
|
||||
Add behavioral specs: data-render, no-mission(404), error alert, grid-survives-error, loading.
|
||||
4. Mutation-verify by cp snapshot/restore (never git). Run the four gates before/after.
|
||||
|
||||
## Progress log
|
||||
|
||||
- Baseline measured on disk before edits: test 24 files / 238 tests rc=0 (typecheck/lint/build:vite
|
||||
clean per dispatcher at same HEAD).
|
||||
- Implemented port; fixture; spec conversion + 5 new tests. See final gates below.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
| Gate | Before | After |
|
||||
| --- | --- | --- |
|
||||
| typecheck | clean (dispatcher @216cd722) | clean, rc=0 |
|
||||
| lint | clean (dispatcher @216cd722) | clean, rc=0 |
|
||||
| test | 24 files / 238 tests, rc=0 | 24 files / 243 tests, rc=0 |
|
||||
| build:vite | clean + pre-existing chunk warning | clean + same pre-existing chunk warning |
|
||||
|
||||
Mutations (cp snapshot → edit → run spec → cp restore → sha256 verified):
|
||||
- M1: removed `<MissionStatus />` render from the page → 5 tests failed (all 5 new mission tests).
|
||||
- M2: removed the 404→no-mission branch (404 falls to generic error) → 1 test failed
|
||||
(no-active-mission path test).
|
||||
|
||||
## Risks / notes
|
||||
|
||||
- Baseline flat-key defect not replicated (disclosed in report; fixing the legacy Next tree is out
|
||||
of scope — fenced).
|
||||
- No new routes; `packages/types/src/web-ui-routes.ts` untouched.
|
||||
@@ -0,0 +1,156 @@
|
||||
# WebUI Phase P — P5a Gateway Static Surface + SPA History Fallback Scratchpad
|
||||
|
||||
**Task ID:** P5a (remediation pass)
|
||||
**Tracking ref:** Phase P increment map (`docs/webui/PHASE-P-STRUCTURE.md` §5); no provider issue
|
||||
**Branch:** `feat/webui-p5a-gateway-static` (HEAD `216cd722`, uncommitted working-tree state)
|
||||
**Started:** 2026-08-12 (remediation session)
|
||||
**Role:** author worker; two independent reviews (security + code, author ≠ reviewer) already
|
||||
delivered; this pass remediates the confirmed findings. `docs/TASKS.md` remains orchestrator-only.
|
||||
|
||||
## What P5a implements
|
||||
|
||||
The gateway (NestJS + Fastify) gains a production Web UI serving path: it serves the built Vite
|
||||
bundle (`apps/web/dist`) same-origin on `:14242` behind an explicit mode switch, with a
|
||||
history-API fallback shaped by the SPA route table instead of a blanket `/*` → `index.html` rule.
|
||||
Companion changes: `GATEWAY_CORS_ORIGIN` parsing moved into one tested seam
|
||||
(`apps/gateway/src/cors-origins.ts`) consumed by REST CORS (`main.ts`), the raw BetterAuth mount
|
||||
(`auth/auth.controller.ts` + `trustedOrigins` in `auth/auth.module.ts`), and the Socket.IO
|
||||
handshake (`chat/chat.gateway.ts`); CSP tightened for the SPA image surface; `apps/web` build
|
||||
output cutover prep (vite config, index.html, theme-init).
|
||||
|
||||
## Containment mechanism (author's own words)
|
||||
|
||||
1. **Canonicalize, then compare, never trust spellings.** The bundle root is `realpath`'d once at
|
||||
registration (`inspectWebUiBundle`), so a symlinked release dir is fine but every later file
|
||||
lookup resolves through the canonical root. A candidate path is accepted only when
|
||||
`relative(webRoot, realpath(join(webRoot, …)))` yields a path that is neither empty, nor
|
||||
`..`-prefixed, nor absolute — traversal, symlink hops, and encoded spellings that resolve outside
|
||||
the root all fail that comparison. Decode is one `decodeURIComponent` pass on the pathname only;
|
||||
double-encoded spellings decode to literal file names (and are tested to do exactly that).
|
||||
2. **Reserved-prefix deny list before any static/history lookup.** `/api` and `/socket.io` (plus
|
||||
their `/<prefix>/…` children) are denied at the catch-all before static file resolution or the
|
||||
SPA fallback can claim them. The check is deliberately case-sensitive — `/API` is an ordinary
|
||||
static name, never a shadow of the API namespace. Segment hygiene (`.`, `..`, dotfile-prefixed)
|
||||
is enforced twice: at path parsing and again at file resolution; `@fastify/static` is registered
|
||||
with `dotfiles: 'deny'` as a third layer.
|
||||
3. **Route-table-shaped fallback.** An unmatched path falls back to `index.html` only when
|
||||
`isSpaRoute` says the path matches a route from `@mosaicstack/types` `SPA_ROUTES` (exact match,
|
||||
UUID-shaped `:id`, provider-registry-checked `:provider`) **and** the request accepts HTML.
|
||||
Everything else gets the standard JSON 404, so the fallback can never paper over an API typo or
|
||||
serve the SPA shell to non-browser clients. The provider list crossing the `@mosaicstack/auth`
|
||||
package boundary is shape-checked at registration (`assertSpaAuthProviderRegistry`).
|
||||
|
||||
## GATEWAY_WEB_UI_MODE states
|
||||
|
||||
- `disabled` (default; also for unset/empty): no Web UI routes are registered. If a bundle is
|
||||
nonetheless present on disk, startup **fails** — a stale `apps/web/dist` must never silently
|
||||
ship an unvetted UI. The default is `disabled` because the installed/CLI launch path has no
|
||||
bundle guarantee; the operator must opt in.
|
||||
- `required`: the bundle is validated (index.html + Vite manifest + real, non-symlink files, entry
|
||||
chunk actually referenced by a same-origin `<script>`) and then mounted; a missing or partial
|
||||
bundle fails startup with the `build:vite` remediation hint.
|
||||
- Anything else: startup error (`resolveWebUiMode`).
|
||||
|
||||
## What the three suites cover
|
||||
|
||||
- `web-ui.spec.ts` — mode resolution and the disabled+present tripwire; bundle validation failure
|
||||
modes; static serving through a real Nest+Fastify app (route precedence over the catch-all);
|
||||
history fallback shaped by `SPA_ROUTES` incl. exact/project-id/provider matchers and non-HTML
|
||||
404s; Socket.IO namespace coexistence; `main.ts` bootstrap ordering (init → web-ui registration
|
||||
→ listen) and CSP directives.
|
||||
- `web-ui-containment.spec.ts` — the request-path conformance matrix: every case run through both
|
||||
`Fastify.inject()` and a real socket (llhttp in the loop), asserting the two transports agree;
|
||||
decoding passes, reserved-prefix denial, dotfiles, traversal/symlink containment.
|
||||
- `web-ui-bundle.spec.ts` — the bundle-validation matrix: manifest shape, real-file assertions,
|
||||
index resource references, entry-chunk boot check, size/parse-budget caps.
|
||||
|
||||
Plus (remediation): `cors-origins.spec.ts` — the `GATEWAY_CORS_ORIGIN` parsing/validation seam
|
||||
covering unset/empty/single/multi/trailing-comma/whitespace entries and the glob rejection
|
||||
contract (see R1).
|
||||
|
||||
## Reviews and remediation
|
||||
|
||||
Two independent reviews ran against P5a (security review + code review; neither authored P5a).
|
||||
Findings were verified against the tree by the orchestrator; the confirmed set was remediated in
|
||||
this pass:
|
||||
|
||||
| Finding | Class | Resolution |
|
||||
| --- | --- | --- |
|
||||
| R1: `GATEWAY_CORS_ORIGIN` globs interpreted literally by gateway layers but as wildcards by BetterAuth `trustedOrigins` → `*` looks locked down but opens the CSRF/origin gate | security (code) | Rejected at the source: `listGatewayCorsOrigins()` throws on any entry containing `*` or `?`, naming the variable and the offending entry. All three consumers share the seam, so all fail closed at startup. |
|
||||
| R2: `listGatewayCorsOrigins` had zero direct coverage (consumer spec mocks it out) | test gap | Added `cors-origins.spec.ts`: unset, empty, single, comma list with spaces, trailing comma, whitespace-only entry, glob rejections, and a pin on multi-origin splitting (pre-P5a `main.ts` passed the raw comma string, so multi-origin values never matched). |
|
||||
| R3: P5a removed the `http://localhost:3000` CORS default (correct, kept) but silently — off-origin consumers now fail with no note | observability | Single `main.ts` startup warn line when the origin list is empty, naming `GATEWAY_CORS_ORIGIN`. Pinned in the main bootstrap spec. |
|
||||
| R4: `pnpm --filter @mosaicstack/gateway start` crashes after any `pnpm dev`/`build:vite` until `apps/web/dist` is removed — polarity correct, workflow landmine undocumented | docs | Documented in `docs/webui/PHASE-P-STRUCTURE.md` §4 (build-scripts section); disabled-mode tripwire error text now names the concrete removal remedy. Polarity unchanged. |
|
||||
| R5: Dockerfile substring test (`web-ui.spec.ts`) could only fail cosmetically, guaranteed nothing about the image | test hygiene | Deleted. The real guarantee is a docker-build CI step — deferred to P6 (this note is the tracking record). |
|
||||
| R6: P5a had no scratchpad (verified by enumeration) | method | This file. |
|
||||
|
||||
Declined as out of scope (reviewers raised, correctly declined; see also the brief's "Explicitly
|
||||
NOT in scope"):
|
||||
|
||||
- **Case-insensitive-filesystem reserved-prefix bypass** (`web-ui.ts` case-sensitive prefix
|
||||
check): not reachable on Linux, the supported deployment platform (`node:22-alpine`). Left as-is.
|
||||
- **No extension allowlist on served files**: informational; dotfiles already denied twice;
|
||||
current `dist` has no maps and `vite.config.ts` does not enable sourcemaps.
|
||||
- **TOCTOU symlink swap / hardlinks**: requires write access to the bundle directory, at which
|
||||
point the attacker can read those files directly; structural to realpath-based serving.
|
||||
- **`as unknown as FastifyInstance` cast (`main.ts`) + dual `fastify` 5.x patch copies in the
|
||||
lockfile**: optional P6 pin (workspace-aliasing `fastify` so adapter and workspace share one
|
||||
resolution).
|
||||
- **WebSocket Origin authorization gap in `chat.gateway.ts` `handleConnection`**: routed to a
|
||||
different lane by the orchestrator; untouched here.
|
||||
- **`packages/mosaic/src/stages/gateway-config.ts` port-3000 CORS derivation**: pre-existing,
|
||||
carried, already promised to P6 by its own comment. Untouched.
|
||||
|
||||
## Deliberately deferred to P6
|
||||
|
||||
1. **docker-build CI step** — the image-level guarantee the deleted Dockerfile substring test
|
||||
pretended to provide (replaces R5's cosmetic check).
|
||||
2. **Port-3000 CORS derivation in `packages/mosaic/src/stages/gateway-config.ts`** — pre-existing
|
||||
and carried; owned by the comment at lines 548-551.
|
||||
3. **Optional `fastify` workspace pin** — single resolution for the Nest adapter and the gateway
|
||||
package (also removes the `as unknown as FastifyInstance` cast).
|
||||
|
||||
## Remediation plan
|
||||
|
||||
1. Baseline gates re-run (numbers recorded below). ✓
|
||||
2. Write `cors-origins.spec.ts` first (TDD — R1 is security logic); observe RED on the rejection
|
||||
cases. ✓ (7 parse cases passed, 4 rejection cases failed against the unpatched seam —
|
||||
reproducing R1 — then 11/11 green after the fix)
|
||||
3. Implement the glob rejection in `listGatewayCorsOrigins()`. ✓
|
||||
4. Add the empty-origin startup warn in `main.ts`; pin it in the main bootstrap spec. ✓
|
||||
5. Delete the Dockerfile substring test and the now-unused import. ✓
|
||||
6. R4: error-text remedy + workflow note in `PHASE-P-STRUCTURE.md` §4. ✓
|
||||
7. Re-run all four gates; record evidence. ✓
|
||||
|
||||
## Budget
|
||||
|
||||
No explicit cap supplied; remediation is bounded by the confirmed findings R1-R6. Working soft
|
||||
cap ~30K tokens for this pass.
|
||||
|
||||
## Verification evidence
|
||||
|
||||
- Baseline (before remediation):
|
||||
- `pnpm --filter @mosaicstack/gateway typecheck` rc=0
|
||||
- `pnpm --filter @mosaicstack/gateway lint` rc=0
|
||||
- `pnpm --filter @mosaicstack/gateway test` rc=0 — 76 files / 1003 tests passed (17 skipped)
|
||||
- `pnpm --filter @mosaicstack/web build:vite` rc=0
|
||||
- After remediation (all green; test count up as required):
|
||||
- `pnpm --filter @mosaicstack/gateway typecheck` rc=0
|
||||
- `pnpm --filter @mosaicstack/gateway lint` rc=0
|
||||
- `pnpm --filter @mosaicstack/gateway test` rc=0 — **77 files / 1015 tests passed** (17 skipped);
|
||||
net +12 (cors-origins.spec.ts +11, main bootstrap R3 pins +2, Dockerfile substring test −1)
|
||||
- `pnpm --filter @mosaicstack/web build:vite` rc=0
|
||||
- `npx prettier --check` on every touched file rc=0 (PHASE-P-STRUCTURE.md was re-formatted by
|
||||
prettier after hand-editing; HEAD version verified clean beforehand so only my section moved)
|
||||
- TDD evidence: `npx vitest run src/cors-origins.spec.ts` before the fix — 4 failed | 7 passed;
|
||||
after the fix — 11 passed.
|
||||
- Working tree: 21 modified + 14 untracked (baseline 20 + 12; +PHASE-P-STRUCTURE.md modified,
|
||||
+cors-origins.spec.ts and this scratchpad untracked). No other file touched.
|
||||
|
||||
## Constraints honored
|
||||
|
||||
- No gateway/dev-server/process listening was started; `DATABASE_URL` never set.
|
||||
- No token/secret/credential value printed or logged (R1's error names an operator-authored
|
||||
configuration entry, which is configuration, not a secret).
|
||||
- No commit, branch, push, PR, or `--no-verify`; all work left as working-tree state.
|
||||
- Fence respected: `apps/web/src/spa/**`, `chat.gateway.ts` `handleConnection`, and
|
||||
`gateway-config.ts` port-3000 derivation untouched.
|
||||
@@ -78,6 +78,19 @@ Legend: `──` new SPA (keep), `══` legacy Next (removed at P5), shared `l
|
||||
At **P5** the `:vite` variants become the defaults (`dev`→vite, `build`→vite build) and the Next
|
||||
build path is retired.
|
||||
|
||||
### Gateway serving scripts (`apps/gateway/package.json`) — P5a polarity
|
||||
|
||||
| Script | Mode set | Behavior |
|
||||
| ------- | ------------------------------ | -------------------------------------------------------- |
|
||||
| `dev` | `GATEWAY_WEB_UI_MODE=required` | builds the Vite bundle first, then serves it same-origin |
|
||||
| `start` | `GATEWAY_WEB_UI_MODE=disabled` | API-only; compiled gateway, no Web UI routes |
|
||||
|
||||
The `start` polarity is deliberate and fail-closed: if a bundle is present at `apps/web/dist`
|
||||
(e.g. left over from `dev` or `build:vite`), `start` **refuses to boot** rather than serve a stale,
|
||||
unvetted bundle. That is the intended tripwire, not a bug — remove the stale bundle
|
||||
(`rm -rf apps/web/dist`) or opt in explicitly with `GATEWAY_WEB_UI_MODE=required`. The startup error
|
||||
names both remedies.
|
||||
|
||||
## 5. Increment map (P1–P6)
|
||||
|
||||
| # | Increment | Branch | Status |
|
||||
|
||||
Reference in new issue
Block a user