173 lines
12 KiB
Markdown
173 lines
12 KiB
Markdown
# WebUI Phase P — P2 Data + Auth Scratchpad
|
|
|
|
**Task ID:** WEBUI-P2
|
|
**Tracking ref:** stacked on PR #1143 (`feat/webui-p1-vite-skeleton`); no separate issue specified in the author brief
|
|
**Branch:** `feat/webui-p2-data-auth`
|
|
**Started:** 2026-08-09
|
|
**Role:** P2 author worker (must not modify `docs/TASKS.md`)
|
|
|
|
## Original tasking
|
|
|
|
> Read `/home/jwoltje/briefs/P2-brief.md` and execute it fully in `/home/jwoltje/src/stack-p1` on branch `feat/webui-p2-data-auth`. Run every listed verification gate, commit locally only, and do not push.
|
|
|
|
Container path mapping:
|
|
|
|
- Brief: `/home/jwoltje/distrobox-homes/mosaic-dev/briefs/P2-brief.md`
|
|
- Repo: `/home/jwoltje/distrobox-homes/mosaic-dev/src/stack-p1`
|
|
|
|
## Objective
|
|
|
|
Make the P1 Vite SPA authenticate against the Mosaic Gateway by same-origin relative paths, replace guest-route placeholders with real login/register/SSO callback pages, and enforce guest/authenticated route guards without modifying the parallel Next app tree except for unavoidable shared-library import fixes.
|
|
|
|
## Scope and invariants
|
|
|
|
- All SPA HTTP and Socket.IO access remains origin-relative (`/api/...`, `/api/auth/...`, `/chat`).
|
|
- No `NEXT_PUBLIC_*`, `VITE_*` origin setting, or hard-coded `http://localhost:14242` under `apps/web/src/`.
|
|
- Verify the Gateway BetterAuth mount path from source before choosing auth-client configuration.
|
|
- Delete the legacy static SSO-provider discovery module and test; runtime `/api/sso/providers` is canonical.
|
|
- Preserve the Next build while adding React Router guest pages and session guards.
|
|
- Never push; stage named files only; do not touch the modified `.mosaic/orchestrator/session.lock`.
|
|
|
|
## Plan
|
|
|
|
1. Inspect P1 SPA structure, shared libraries, legacy Next auth pages/components, Gateway auth mount, and current test setup.
|
|
2. Add/adjust tests first for relative API behavior, auth-client origin configuration, relative Socket.IO singleton behavior, and all guard session states; run focused tests and capture expected RED failures.
|
|
3. Implement shared-library relative networking and remove the legacy SSO-provider module/imports.
|
|
4. Port login, register, and provider callback pages to React Router and wire routes.
|
|
5. Implement session-aware guest/auth guards and satisfy focused tests.
|
|
6. Run focused tests, all brief verification gates, invariant searches, and an independent code/security review; remediate and re-run affected gates.
|
|
7. Update this scratchpad with evidence, stage named files only, and commit locally.
|
|
|
|
## Testing strategy
|
|
|
|
- TDD is required because this increment changes authentication/session behavior.
|
|
- Primary situational evidence: jsdom router guard state tests plus guest-page/auth-flow contract tests already present or added as needed.
|
|
- Baseline gates: web tests, Vite build, Next build, typecheck, lint, and root format check exactly as listed in the brief.
|
|
- Browser automation is not required by the brief for P2; if used, it will be headless only.
|
|
|
|
## Budget
|
|
|
|
No explicit user token cap was provided. Working soft cap: **30K tokens**, derived from a multi-file auth/frontend increment with tests, dual builds, review, and remediation. Keep implementation within the brief; no unrelated refactors or dependencies.
|
|
|
|
## Progress / evidence
|
|
|
|
- [x] Loaded mission protocol, active MVP manifest/scratchpad, top-level tasks, PRD, relevant frontend/auth/testing/type/review guides, and matching skills.
|
|
- [x] Resolved host paths to the Distrobox-mapped repo and brief.
|
|
- [x] Confirmed branch `feat/webui-p2-data-auth`, stacked at P1 commit `068d0f9b`.
|
|
- [x] Source and Gateway mount inspection complete.
|
|
- [x] RED tests observed.
|
|
- [x] Implementation complete.
|
|
- [x] Independent review complete and findings remediated.
|
|
- [x] Verification gates complete.
|
|
- [x] Local implementation commit created: `46d68e1f`.
|
|
|
|
### Source decisions
|
|
|
|
- Gateway source mounts BetterAuth at `/api/auth/` in `apps/gateway/src/auth/auth.controller.ts`; `packages/auth/src/auth.ts` configures `basePath: '/api/auth'`.
|
|
- BetterAuth 1.5.5 defaults its browser client to `/api/auth` when `baseURL` is omitted. `src/lib/auth-client.ts` therefore omits `baseURL`, preserving same-origin behavior without encoding any gateway origin.
|
|
- The only `src/app/` edit is the transitional Next provider redirect page. Deleting `src/lib/sso-providers.ts` required that mechanical consumer migration; it now uses the same runtime `/api/sso/providers` discovery and callback sanitizer as the SPA.
|
|
- GuestGuard pending behavior intentionally follows the approved brief: if no session object exists (including pending), render `<Outlet />`; only a present session redirects to `/chat`. AuthGuard alone renders the specified pending treatment.
|
|
|
|
### TDD evidence
|
|
|
|
- Initial focused run: 9 expected failures, proving absolute API/auth/socket origins, P1 guest placeholders, and missing guard redirects/loading behavior.
|
|
- After implementation: focused contract suite 15/15 passed.
|
|
- Security remediation RED: `%2F%0A%2F%2Fevil.example` reached `signIn.oauth2` as an external-normalizing callback target before the fix.
|
|
- Security remediation GREEN: shared `resolveAuthCallbackURL` unit suite 9/9 plus SPA callback regression 1/1 passed; both SPA and Next consumers use the shared helper.
|
|
|
|
### Independent review
|
|
|
|
- Primary Codex wrappers could not run because `jq` is absent; direct Codex fallback then failed authentication with HTTP 401. No review result was claimed from those attempts.
|
|
- Independent Claude Sonnet code review found no implementation-scope blocker, one pending-state UX suggestion, and one pre-existing `api.ts` type-assertion suggestion. The pending-state suggestion was rejected because it contradicts the brief's explicit GuestGuard contract; the API cleanup is outside P2's preserve-contract scope.
|
|
- Independent Claude Sonnet security review found a high-severity control-character open redirect in the callback prefix check shared by the newly ported SPA logic and transitional Next consumer.
|
|
- Remediation centralized WHATWG URL parsing plus exact current-origin comparison in `src/lib/auth-redirect.ts`, returns only path/search/hash, and added the RED-first regression above.
|
|
- Fresh code re-review: `approve`, 0 blockers, 0 should-fix findings.
|
|
- Fresh security re-review: `low`, 0 critical/high/medium/low findings.
|
|
|
|
### Documentation checklist disposition
|
|
|
|
- `docs/PRD.md` exists and P2 aligns to FR-8 / AC-7 authentication requirements.
|
|
- No Gateway endpoint, DTO, permission, or API schema changed; existing `/api/auth/*` and `/api/sso/providers` contracts are consumed unchanged, so OpenAPI/API-index updates are not applicable.
|
|
- Route paths were already present in the P1 route table; P2 replaces placeholders without changing site-map navigation, so `docs/SITEMAP.md` is unchanged.
|
|
- User/admin auth behavior is parity with the still-live Next implementation, not a new workflow. Implementation decisions, failure behavior, testing, and migration compatibility are documented here; no publishing action is in scope.
|
|
- Documentation remains in-repo; no generated publishing output was created.
|
|
|
|
### Final verification evidence
|
|
|
|
Run fresh after remediation from the locations required by the brief:
|
|
|
|
- `apps/web: pnpm test` — PASS: `Test Files 9 passed (9)`; `Tests 27 passed (27)`.
|
|
- `apps/web: pnpm build:vite` — PASS: `✓ 113 modules transformed`; `✓ built in 565ms`.
|
|
- `apps/web: pnpm build` — PASS: `✓ Compiled successfully in 4.6s`; 10/10 static pages generated; dynamic provider/project routes retained.
|
|
- `apps/web: pnpm typecheck` — PASS: `tsc --noEmit` exited 0.
|
|
- `apps/web: pnpm lint` — PASS: `eslint src` exited 0.
|
|
- Root `pnpm format:check` — PASS: `All matched files use Prettier code style!`.
|
|
- Invariant scan — PASS: no `NEXT_PUBLIC_*`, `VITE_*`, `GATEWAY_URL`, `http://localhost:14242`, or `sso-providers` references under `apps/web/src`.
|
|
- `git diff --check` — PASS.
|
|
- All six required gates above were repeated successfully after implementation commit `46d68e1f`, proving the exact committed source tree.
|
|
|
|
## Risks / blockers
|
|
|
|
- `.mosaic/orchestrator/session.lock` is modified by the active harness and must remain unstaged.
|
|
- P2 changes shared `src/lib/` modules consumed by both Vite and Next, so the Next build is a required compatibility gate.
|
|
|
|
## Remediation 1 — independent Node 26 verification failure
|
|
|
|
**Correction received:** 2026-08-09
|
|
|
|
The orchestrator rejected the P2 verification claim after an independent run under Node 26.4.0 produced 2 failed redirect tests and 2 unhandled errors. React Router passed jsdom's realm-local `AbortSignal` to Node 26's native undici `Request`, which rejects non-native signals. Production guard behavior is correct and must not change.
|
|
|
|
### Remediation constraints
|
|
|
|
- Preserve both redirect assertions and all `guards.tsx` behavior; no skips, weakening, or test deletion.
|
|
- Add the smallest Vitest environment repair so global `AbortController` / `AbortSignal` are constructors accepted by native undici `Request`.
|
|
- Run all six required gates, commit named files locally without pushing, leave `.mosaic/orchestrator/session.lock` untouched, then run `apps/web: pnpm test` as the final worktree action.
|
|
- Completion requires zero failed tests, zero test errors, and zero unhandled rejections.
|
|
|
|
### Remediation plan
|
|
|
|
1. Reproduce under Node 26.4.0 if an ephemeral matching runtime is available.
|
|
2. Add a Vitest `setupFiles` module deriving Node-native abort constructors from `node:util` and register it in `apps/web/vitest.config.ts`.
|
|
3. Add a direct regression assertion that a global controller's signal is accepted by native `Request`, while retaining the existing redirect behavior tests unchanged.
|
|
4. Run focused Node 22 and Node 26 tests, independent review, all required gates, and local commit(s).
|
|
5. After every edit/commit/status check is complete, run `pnpm test` from `apps/web` as the last command.
|
|
|
|
### Remediation implementation and evidence
|
|
|
|
- Reproduced under ephemeral Node `v26.4.0`: `Test Files 1 failed (1)`, `Tests 2 failed | 4 passed (6)`, `Errors 2 errors`, with the exact undici `AbortSignal` realm rejection from the remediation brief.
|
|
- Added `apps/web/src/test/setup.ts`, registered through `vitest.config.ts#setupFiles`. It derives Node-native abort constructors from the built-in `node:util.transferableAbortController()` and aligns both `globalThis` and jsdom `window`; it does not replace `Request`, `Response`, or `fetch`.
|
|
- Explicitly pinned Vitest `isolate: true` so the test-only global constructors cannot leak between test-file environments.
|
|
- Added `src/test/setup.spec.ts`, which proves a global controller signal is accepted by Node's native `Request` and that abort propagation remains functional.
|
|
- Existing `guards.spec.tsx` and production `guards.tsx` remain unchanged.
|
|
- Focused Node 26.4.0 remediation run: `Test Files 2 passed (2)` and `Tests 7 passed (7)`, with zero errors/unhandled rejections.
|
|
- Preliminary full Node 26.4.0 run: `Test Files 9 passed (9)` and `Tests 27 passed (27)`, with zero errors/unhandled rejections before the direct setup regression was added.
|
|
- No dependency was added; `node:util` is a Node built-in.
|
|
|
|
### Remediation independent review
|
|
|
|
- First code review: approve, 0 blockers, 0 should-fix; suggested a direct Request regression and version-neutral comment.
|
|
- First security/integrity review: low risk; suggested pinning test isolation explicitly.
|
|
- All suggestions were applied.
|
|
- Fresh code re-review: approve, 0 blockers, 0 should-fix, no findings.
|
|
- Fresh security/integrity re-review: low risk, 0 findings.
|
|
|
|
### Remediation pre-commit gate evidence
|
|
|
|
- Node 26.4.0 `pnpm test`: `Test Files 10 passed (10)`; `Tests 28 passed (28)`; zero errors and zero unhandled rejections.
|
|
- `pnpm typecheck`: `tsc --noEmit` exited 0.
|
|
- `pnpm build:vite`: `✓ 113 modules transformed`; `✓ built in 960ms`.
|
|
- `pnpm build` (Next): `✓ Compiled successfully in 4.8s`; static pages generated 10/10.
|
|
- `pnpm lint`: `eslint src` exited 0.
|
|
- Root `pnpm format:check`: `All matched files use Prettier code style!`.
|
|
- Final post-commit non-test gates and final-action Node 26 `pnpm test` remain required before reporting completion.
|
|
|
|
### Remediation committed verification
|
|
|
|
- Implementation commit: `e16c08aa` (`test(web): align jsdom abort signals with Node`).
|
|
- Post-commit `pnpm typecheck`: `tsc --noEmit` exited 0.
|
|
- Post-commit `pnpm build:vite`: `✓ 113 modules transformed`; `✓ built in 323ms`.
|
|
- Post-commit `pnpm build` (Next): `✓ Compiled successfully in 4.6s`; static pages generated 10/10.
|
|
- Post-commit `pnpm lint`: `eslint src` exited 0.
|
|
- Post-commit root `pnpm format:check`: `All matched files use Prettier code style!`.
|
|
- Final Node 26 `pnpm test` will be the last worktree action and its verbatim output will be reported to the orchestrator.
|