Files
familysync/.planning/phases/06-ux-polish/06-05-PLAN.md
T

239 lines
23 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
phase: 06-ux-polish
plan: 05
type: execute
wave: 1
depends_on: []
files_modified:
- apps/pwa/src/api/client.ts
- apps/pwa/src/api/client.test.ts
- apps/pwa/src/components/AuthSplash.tsx
- apps/pwa/src/components/CalendarShell.tsx
- apps/pwa/src/main.tsx
- apps/pwa/src/store/calendarStore.ts
autonomous: false
requirements: []
must_haves:
truths:
- "Unauthenticated cold load shows a single neutral 'Signing you in' splash — no calendar shell, skeleton, or 'Sign-in required' flash before Authelia (D-10, success criterion 5)"
- "A session that expires mid-use (401 / opaqueredirect from ANY query or mutation) shows a 'Session expired' interstitial and cleanly redirects to /api/login instead of hanging (D-11, success criterion 4)"
- "Every PWA fetch wrapper detects 401/opaqueredirect and throws a typed SessionExpiredError (D-11)"
artifacts:
- path: "apps/pwa/src/api/client.ts"
provides: "SessionExpiredError class + consistent redirect:'manual' + handleAuthResponse across all fetch wrappers; recurrenceUntil/recurrenceCount on CreateEventPayload; hasRrule on CalendarOccurrence"
contains: "class SessionExpiredError"
- path: "apps/pwa/src/components/AuthSplash.tsx"
provides: "full-screen neutral auth interstitial (loading / redirecting / dead-end states)"
contains: "AuthSplash"
- path: "apps/pwa/src/main.tsx"
provides: "QueryClient wired with QueryCache+MutationCache onError that arms the session-expiry interstitial"
contains: "MutationCache"
key_links:
- from: "apps/pwa/src/main.tsx"
to: "apps/pwa/src/store/calendarStore.ts"
via: "QueryCache/MutationCache onError → setSessionExpired(true) on SessionExpiredError"
pattern: "SessionExpiredError"
- from: "apps/pwa/src/components/CalendarShell.tsx"
to: "apps/pwa/src/components/AuthSplash.tsx"
via: "meQuery.isLoading/isError and sessionExpired flag render AuthSplash instead of calendar/alert"
pattern: "AuthSplash"
---
<objective>
Smooth the entire auth flow (D-10 + D-11) — the security-relevant slice. A single refactor serves both: gate the app render on auth state so nothing paints before Authelia (999.2), and centralize session-expiry detection so a timed-out session redirects cleanly instead of hanging (999.3).
This plan is the SOLE owner of `apps/pwa/src/api/client.ts`. To keep file ownership exclusive across the wave, it also lands the two non-auth type additions other plans depend on (consumed, not edited, elsewhere):
- `recurrenceUntil?` / `recurrenceCount?` on `CreateEventPayload` (D-06 — the API contract is in Plan 02; the EventForm UI in Plan 06 sends these).
- `hasRrule: boolean` on the client mirror of `CalendarOccurrence` (D-08 — server source-of-truth is Plan 03; the series-edit prompt in Plan 06 reads it). Per PATTERNS Pitfall 4, the mirror must match `expand.ts` exactly.
Purpose: Auth gating and session-expiry are the phase's highest-severity items (999.3 is "high"). The typed `SessionExpiredError` detection is pure I/O logic → TDD; the splash/interstitial rendering is glue → standard tasks verified with playwright-cli.
Output: `SessionExpiredError` + consistent `redirect:'manual'` in all fetch wrappers; `AuthSplash` component; gated `CalendarShell`; global QueryCache/MutationCache error handler in `main.tsx`; `sessionExpired` flag in the store.
</objective>
<execution_context>
@$HOME/.claude/gsd-core/workflows/execute-plan.md
@$HOME/.claude/gsd-core/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/06-ux-polish/06-RESEARCH.md
@.planning/phases/06-ux-polish/06-PATTERNS.md
@.planning/phases/06-ux-polish/06-UI-SPEC.md
@apps/pwa/src/components/SkeletonCalendar.tsx
@apps/pwa/src/lib/loginRedirect.ts
</context>
<artifacts_this_plan_produces>
NEW symbols introduced here (exclude from drift/convergence checks):
- `class SessionExpiredError extends Error` in `apps/pwa/src/api/client.ts`
- `handleAuthResponse(res, label)` helper in `client.ts`
- `recurrenceUntil?: string` + `recurrenceCount?: number` on `CreateEventPayload` (client.ts)
- `hasRrule: boolean` on the client-side `CalendarOccurrence` (client.ts mirror of expand.ts)
- `AuthSplash` component (`apps/pwa/src/components/AuthSplash.tsx`) with `state: 'loading' | 'redirecting' | 'dead-end'`
- `sessionExpired` boolean + `setSessionExpired` action in the Zustand store (`calendarStore.ts`)
- QueryCache/MutationCache `onError` wiring in `main.tsx`
</artifacts_this_plan_produces>
<context_note_tanstack_v5>
RESEARCH flagged the TanStack Query v5 global-error API as an unverified assumption (A3). It is now RESOLVED via Context7 (`/tanstack/query`): in v5 the global handler is supplied by constructing `new QueryCache({ onError })` and `new MutationCache({ onError })` and passing them into `new QueryClient({ queryCache, mutationCache })`. These `onError` callbacks always fire (unlike `defaultOptions.onError`, which was removed). Do NOT use `defaultOptions.onError`. The executor MUST still run one Context7 `query-docs` confirmation against `/tanstack/query` for the exact `QueryCache`/`MutationCache` constructor signature in version 5.101.0 before coding Task 3, then implement per the confirmed API.
</context_note_tanstack_v5>
<tasks>
<task type="tdd" tdd="true">
<name>Task 1: TDD — SessionExpiredError detection across all fetch wrappers (client.ts)</name>
<files>apps/pwa/src/api/client.ts, apps/pwa/src/api/client.test.ts</files>
<read_first>
- apps/pwa/src/api/client.ts — `fetchMe` (lines 2853) is the model: `redirect:'manual'` + `if (res.type === 'opaqueredirect' || res.status === 401)`; the wrappers to generalize: `fetchEvents` (106), `createEvent` (173), `updateEvent` (194), `deleteEvent` (218), `fetchSyncStatus` (254), `fetchWritableCalendars` (273); `RecurrencePreset` (130), `CreateEventPayload` (136148), `CalendarOccurrence` (7191)
- apps/pwa/src/api/client.test.ts (or, if thin, apps/pwa/src/lib/loginRedirect.test.ts as the role analog) — how to mock `fetch` to return `{ type:'opaqueredirect', status:0 }` and `{ status:401 }`
- .planning/phases/06-ux-polish/06-RESEARCH.md §"Focus 6 — D-11" + §"Code Examples — D-11 typed error" — the exact `SessionExpiredError` class with `Object.setPrototypeOf`
- .planning/phases/06-ux-polish/06-PATTERNS.md §"apps/pwa/src/api/client.ts" + §"Shared Patterns — Auth detection" — handleAuthResponse helper + per-wrapper application
- apps/api/src/broker/expand.ts (from Plan 03) — the authoritative `CalendarOccurrence.hasRrule` field this client type must mirror exactly
</read_first>
<behavior>
- fetchEvents with a mocked `{ type:'opaqueredirect', status:0 }` response → throws `SessionExpiredError` (instanceof check passes).
- fetchEvents with a mocked `{ status:401 }` response → throws `SessionExpiredError`.
- createEvent / updateEvent / deleteEvent with a mocked 401 → each throws `SessionExpiredError`.
- A normal non-auth error (e.g. 500) → throws a generic Error, NOT SessionExpiredError (so ret/ error UI still distinguishes).
- fetchMe's existing opaqueredirect/401 path now also throws SessionExpiredError (unified) — its existing callers (CalendarShell meQuery.isError) continue to work.
</behavior>
<action>
RED: in client.test.ts add cases mocking opaqueredirect and 401 for `fetchEvents`, `createEvent`, `updateEvent`, `deleteEvent` (and a 500 negative case), asserting `instanceof SessionExpiredError`. Run → RED (no such class / wrappers don't detect). Commit `test(06-05): add failing SessionExpiredError detection tests`.
GREEN: add the `SessionExpiredError` class (with `Object.setPrototypeOf(this, SessionExpiredError.prototype)` per RESEARCH) and a `handleAuthResponse(res, label)` helper that throws `SessionExpiredError` on `opaqueredirect || 401` and a generic Error on other non-ok. Add `redirect:'manual'` + `handleAuthResponse(...)` to EVERY fetch wrapper, mirroring `fetchMe`. Also (same file, same commit — exclusive ownership): add `recurrenceUntil?: string` and `recurrenceCount?: number` to `CreateEventPayload`, and add `hasRrule: boolean` to `CalendarOccurrence` matching the Plan 03 `expand.ts` field exactly (Pitfall 4 — atomic mirror). Run → GREEN. Commit `feat(06-05): centralize session-expiry detection and extend client types`.
</action>
<verify>
<automated>cd apps/pwa && pnpm test -- run api/client</automated>
</verify>
<acceptance_criteria>
- `SessionExpiredError` exported; all six fetch wrappers use `redirect:'manual'` + throw it on 401/opaqueredirect (grep: each wrapper references handleAuthResponse).
- 500 (non-auth) does NOT produce SessionExpiredError.
- `CreateEventPayload` has `recurrenceUntil?` + `recurrenceCount?`; `CalendarOccurrence` has `hasRrule: boolean` matching expand.ts.
- `test(06-05)` precedes `feat(06-05)` (RED→GREEN).
</acceptance_criteria>
<done>Typed session-expiry detection unified across all wrappers; client types extended; client suite green; RED→GREEN order present.</done>
</task>
<task type="auto">
<name>Task 2: AuthSplash component + gate CalendarShell render on auth state (D-10)</name>
<files>apps/pwa/src/components/AuthSplash.tsx, apps/pwa/src/components/CalendarShell.tsx</files>
<read_first>
- apps/pwa/src/components/SkeletonCalendar.tsx — full-screen centered layout + inline-style approach to mirror for AuthSplash (PATTERNS §"No Analog Found")
- apps/pwa/src/components/CalendarShell.tsx — current optimistic render: the `meQuery.isError` "Sign-in required" branch (lines ~220235), `isInitialLoading`/SkeletonCalendar, the `maybeRedirectToLogin()` effect (~197) and `clearLoginRedirect()` effect (~205), import block (~4456)
- apps/pwa/src/lib/loginRedirect.ts — `maybeRedirectToLogin()` / `clearLoginRedirect()` one-shot guard semantics
- .planning/phases/06-ux-polish/06-UI-SPEC.md §"Surface 1" (auth splash states + copy + role="status") and §"Brand Assets — In-App Logo Usage" (lockup on splash) and §"Copywriting Contract" (exact copy: heading "Signing you in", body "Taking you to the sign-in page…", dead-end "Sign-in required. Tap here to try again.")
- .planning/phases/06-ux-polish/06-PATTERNS.md §"apps/pwa/src/components/CalendarShell.tsx" — exact branch replacement
</read_first>
<action>
Create `AuthSplash.tsx`: a full-screen centered column (height:100dvh, `--color-surface` bg) with a Loader2 spinner (24px, `--color-member-0`, global `spin`), heading (18px/600) and body (15px/400, `--color-text-secondary`), `role="status"` + `aria-label="Signing you in"`. Accept a `state` prop driving copy per UI-SPEC Surface 1: `loading`/`redirecting` show the spinner + "Signing you in" / "Taking you to the sign-in page…"; `dead-end` shows "Sign-in required. Tap here to try again." with a tap handler (no spinner) that calls `clearLoginRedirect()` then `maybeRedirectToLogin()`. Render the `logo-lockup.svg` above the spinner only if the asset exists; otherwise omit gracefully (brand assets are a separate concern — do not block on them). In `CalendarShell.tsx`, REPLACE the `meQuery.isError` "Sign-in required" block with: early-return `<AuthSplash state="loading" />` when `meQuery.isLoading` (so no skeleton paints pre-auth), and `<AuthSplash state="redirecting" />` when `meQuery.isError` (the existing `maybeRedirectToLogin()` effect still fires). Keep both existing auth effects unchanged. Reserve the `dead-end` state for the one-shot-guard fall-through (guard already set). Do NOT render CalendarContent/SkeletonCalendar until `meQuery.isSuccess`. Commit `feat(06-05): gate app render behind AuthSplash (no pre-auth flash)`.
</action>
<verify>
<automated>cd apps/pwa && pnpm test -- run components/CalendarShell</automated>
</verify>
<acceptance_criteria>
- `AuthSplash` renders loading/redirecting/dead-end with the exact UI-SPEC copy and `role="status"`.
- CalendarShell returns AuthSplash for isLoading and isError; the "Sign-in required" `role="alert"` block is gone; CalendarContent/skeleton render only on isSuccess.
- Existing CalendarShell tests pass (update any test asserting the old "Sign-in required" alert to assert the splash instead).
</acceptance_criteria>
<done>No calendar/skeleton/alert paints before auth; neutral splash covers loading + redirecting; dead-end reserved for guard fall-through.</done>
</task>
<task type="auto">
<name>Task 3: Global session-expiry handler + interstitial wiring (D-11)</name>
<files>apps/pwa/src/main.tsx, apps/pwa/src/store/calendarStore.ts, apps/pwa/src/components/CalendarShell.tsx</files>
<read_first>
- apps/pwa/src/main.tsx — current `new QueryClient({...})` (lines ~1732) and QueryClientProvider mount
- apps/pwa/src/store/calendarStore.ts — existing Zustand `create(...)` shape to add `sessionExpired`/`setSessionExpired`
- apps/pwa/src/lib/loginRedirect.ts — `clearLoginRedirect()` must run BEFORE `maybeRedirectToLogin()` in the expiry path (re-arm the one-shot guard)
- .planning/phases/06-ux-polish/06-UI-SPEC.md §"Surface 2" — interstitial copy ("Session expired" / "Signing you back in…"), ≤2s before redirect, no dismiss button, 1.5s delay before `window.location.href='/api/login'`
- .planning/phases/06-ux-polish/06-RESEARCH.md §"Focus 6 Part 2" + §"Pitfall 5" and this plan's <context_note_tanstack_v5> — the v5 QueryCache/MutationCache onError API (NOT defaultOptions.onError)
- Context7 `/tanstack/query` — confirm the exact v5.101.0 `QueryCache`/`MutationCache` constructor + `onError` signature before coding (mandatory per planning context)
</read_first>
<action>
Run the Context7 confirmation first, then: add `sessionExpired: boolean` (default false) and `setSessionExpired(v)` to the Zustand store. In `main.tsx`, construct the `QueryClient` with `queryCache: new QueryCache({ onError })` and `mutationCache: new MutationCache({ onError })`, where each `onError(error)` checks `error instanceof SessionExpiredError` and calls `setSessionExpired(true)` (read the store action outside React via the store's `getState`/imperative setter pattern already used in the codebase). In `CalendarShell.tsx` (or the app root above the calendar), when `sessionExpired` is true render `<AuthSplash state="redirecting" />` with the Surface-2 copy ("Session expired" / "Signing you back in…"), and on mount of that state run `clearLoginRedirect()` then schedule `maybeRedirectToLogin()` after ~1.5s. Re-use `AuthSplash` (extend it with the session-expired copy variant rather than creating a second component — keep one interstitial component). In-flight write replay is explicitly OUT (D-11 nice-to-have, deferred per RESEARCH Open Question 3) — surfacing a clean re-auth is sufficient. Commit `feat(06-05): global session-expiry interstitial via QueryCache/MutationCache onError`.
</action>
<verify>
<automated>cd apps/pwa && pnpm test -- run 2>&1 | tail -3</automated>
</verify>
<acceptance_criteria>
- `main.tsx` uses `new QueryCache({onError})` + `new MutationCache({onError})` (NOT `defaultOptions.onError`); both route `SessionExpiredError` to `setSessionExpired(true)`.
- Store exposes `sessionExpired` + `setSessionExpired`.
- When `sessionExpired` is true the app shows the "Session expired / Signing you back in…" interstitial and fires `clearLoginRedirect()` then `maybeRedirectToLogin()` after a short delay.
- Full PWA suite still green.
</acceptance_criteria>
<done>Any query/mutation 401 surfaces the interstitial and cleanly re-auths; one-shot guard re-armed; v5 API confirmed via Context7.</done>
</task>
<task type="checkpoint:human-verify" gate="blocking">
<name>Task 4: playwright-cli — no pre-auth flash on cold load; clean session-expiry redirect</name>
<files>(verification only — no files modified)</files>
<action>
Verification task (no code changes). Using the playwright-cli skill against desktop Chromium (dev stack host-side per docs/deployment.md, DEV_AUTH_BYPASS=true): (1) cold-load the app with no session cookie and confirm the FIRST painted frame is the neutral "Signing you in" splash — never the calendar shell, SkeletonCalendar, or a "Sign-in required" alert — then it navigates toward /api/login; (2) with an authenticated session, intercept a subsequent /api/events (or a mutation) to return 401/opaque redirect, trigger it, and confirm the "Session expired / Signing you back in…" interstitial appears then redirects within ~2s (no hang, no generic error); (3) confirm the dead-end "Sign-in required. Tap here to try again." state only appears after the one-shot guard has already fired. If any change appears to affect iOS-Safari standalone redirect behavior, flag it for the iOS human checkpoint per 06-VALIDATION.md. This is a blocking human-verify checkpoint — pause for operator confirmation.
</action>
<read_first>
- .claude/skills/playwright-cli/SKILL.md — drive desktop Chromium, clear cookies, intercept/stub responses (force a 401)
- docs/deployment.md §"Running locally (host-side, no Docker)" — dev run command
- apps/pwa/src/components/AuthSplash.tsx + CalendarShell.tsx — the surfaces under test
</read_first>
<what-built>
Auth-gated render: AuthSplash replaces the optimistic calendar/skeleton/alert on cold load; a global QueryCache/MutationCache error handler surfaces a "Session expired" interstitial and redirects on any mid-use 401.
</what-built>
<how-to-verify>
1. Cold load (D-10): in desktop Chromium with no session cookie, load the app via playwright-cli. Observe the FIRST painted frame is the neutral "Signing you in" splash — NOT the calendar shell, NOT the SkeletonCalendar, NOT a "Sign-in required" alert — then it navigates toward /api/login. Capture the sequence to confirm no calendar/alert flash.
2. Session expiry (D-11): with an authenticated session loaded, intercept a subsequent `/api/events` (or a mutation) to return 401 / an opaque redirect, trigger that request, and observe the "Session expired / Signing you back in…" interstitial appears (no hang, no generic "couldn't load events"), followed by navigation to /api/login within ~2s.
3. Confirm the dead-end "Sign-in required. Tap here to try again." state only appears after the one-shot guard has already fired (not on the first attempt).
NOTE: iOS-Safari standalone cold-load/redirect is the documented exception — if any change appears to affect standalone redirect behavior, flag it for the iOS human checkpoint per 06-VALIDATION.md Manual-Only table.
</how-to-verify>
<verify>
<human-check>Cold load shows only the splash (no calendar/skeleton/alert flash); mid-use 401 shows the session-expired interstitial then redirects cleanly.</human-check>
</verify>
<resume-signal>Type "approved" or describe the flash/hang observed.</resume-signal>
<acceptance_criteria>
- No calendar shell, skeleton, or "Sign-in required" alert paints before the redirect on cold load.
- A mid-use 401 produces the interstitial + clean redirect, not a hang or generic error.
</acceptance_criteria>
<done>Cold-load flash eliminated and mid-use session-expiry redirect verified live in desktop Chromium.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| browser → OIDC IdP (Authelia) | Unauthenticated/expired requests cross to the IdP via a full-page navigation to `/api/login`; the `redirect:'manual'` XHR boundary keeps cross-origin IdP redirects from being silently followed. |
| browser → API (`/api/*`) | Any query/mutation may receive a 401/opaqueredirect when the session has expired; this is the boundary where session state is enforced. |
| client render gate | The point where authenticated calendar content is allowed to paint — must occur only after `meQuery.isSuccess`. |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-06-05-info | Information Disclosure | CalendarShell pre-auth render (D-10) | mitigate | Gate render on `meQuery.isSuccess`; AuthSplash (no app data) is the only thing painted while auth is unknown. Eliminates the 999.2 flash of calendar shell/skeleton — itself a minor disclosure of app structure before auth. (ASVS V2.) |
| T-06-05-redirect | Tampering (open redirect / loop) | maybeRedirectToLogin one-shot guard re-arm (D-11) | mitigate | Redirect target is the fixed internal `/api/login` string — never derived from user input or a `returnTo`/`next` param, so no open-redirect vector. The one-shot `familysync.loginRedirectAttempted` guard prevents a redirect loop; it is re-armed via `clearLoginRedirect()` only on a genuine session-expiry transition (or successful `/api/me`), bounding re-auth attempts to one per expiry. |
| T-06-05-session | Spoofing | SessionExpiredError detection (D-11) | mitigate | Detection is `res.type==='opaqueredirect' || res.status===401` only — it never trusts a response body to decide auth state. Session remains server-enforced via the existing Authelia httpOnly same-origin cookie contract; the client merely reacts to the server's 401/redirect. No token is read or stored client-side. (ASVS V3.) |
| T-06-05-inflight | Repudiation / data loss | in-flight write on expiry | accept | In-flight write replay is deferred (D-11 nice-to-have, RESEARCH Open Question 3). A write that hits an expired session surfaces a clear re-auth instead of silently succeeding; the user re-submits after re-auth. Acceptable for a two-user household; documented, not silent. |
| T-06-05-SC | Tampering | npm installs | accept | No package installs (zero new deps — RESEARCH Package Legitimacy Audit n/a). |
</threat_model>
<verification>
- `cd apps/pwa && pnpm test -- run api/client components/CalendarShell` green; full `pnpm --filter @familysync/pwa test` green.
- `grep -n "class SessionExpiredError" apps/pwa/src/api/client.ts` present; `grep -n "MutationCache" apps/pwa/src/main.tsx` present; `grep -n "defaultOptions" apps/pwa/src/main.tsx` does NOT show an `onError` (v5 correctness).
- `grep -n "hasRrule" apps/pwa/src/api/client.ts` and `recurrenceUntil` present (type mirrors landed).
- playwright-cli: no pre-auth flash; clean mid-use redirect.
</verification>
<success_criteria>
- D-10 (success criterion 5): unauthenticated cold load shows only the neutral splash.
- D-11 (success criterion 4): mid-use session expiry redirects cleanly via a global handler.
- Client type contract for D-06 (payload) and D-08 (hasRrule mirror) is in place for Plan 06.
- `client.ts` ownership is exclusive to this plan (no other Wave-1 plan edits it).
</success_criteria>
<output>
Create `.planning/phases/06-ux-polish/06-05-SUMMARY.md` when done (RED/GREEN notes for Task 1, the confirmed TanStack v5 API used, and the playwright-cli observations).
</output>