luckberg/familysync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(12): gap-closure plans 05-07 for 6 UAT gaps

Browse Source
This commit is contained in:
Lucas Berger
2026-06-15 21:05:45 -04:00
parent dc7f8d2aa9
commit f0fb31348d
4 changed files with 442 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.planning/phases/12-initial-setup-wizard/12-05-PLAN.md
+143
View File
@@ -0,0 +1,143 @@
---
phase: 12-initial-setup-wizard
plan: 05
type: execute
wave: 2
depends_on: ["12-06"]
files_modified:
- apps/pwa/src/routes/SetupPage.tsx
- apps/pwa/src/routes/SetupPage.test.tsx
autonomous: true
gap_closure: true
requirements: [SETUP-01]
must_haves:
truths:
- "The Instance step intro copy no longer contains the DB-vs-env-file aside"
- "A read-only, disabled DB-name field renders directly under the App URL field on the Instance step"
- "Navigating Back from the Calendar step to the Instance step preserves all previously entered field values"
artifacts:
- path: "apps/pwa/src/routes/SetupPage.tsx"
provides: "Instance step copy trimmed; read-only DB-name field; field state lifted so Back preserves values"
contains: "readOnly"
key_links:
- from: "SetupPage Instance step"
to: "GET /api/setup/status dbName"
via: "fetchSetupStatus().dbName populates the read-only field"
pattern: "dbName"
- from: "SetupPage parent (step owner)"
to: "Step2Config fields"
via: "field values lifted to SetupPage (or sessionStorage) and passed as props"
pattern: "appUrl|oidcIssuer|oidcClientId|vapidPublicKey"
---
<objective>
Close UAT gaps 1, 3 (frontend), and 4 — all on the PWA Instance step (`SetupPage.tsx`).
Gap 1 (cosmetic): the Instance step intro `<p>` contains "These are written to the database — not your environment file." — an implementation aside the user wants dropped.
Gap 3 (minor, frontend half): the "database connection verified" row has no on-screen referent. Add a read-only, greyed-out/disabled field showing the env-derived DB name (from `GET /api/setup/status` `dbName`, added in Plan 06), positioned directly under the App URL field. Keep the existing DB validation row as-is.
Gap 4 (minor): each wizard step holds its field values in its own local `useState` and unmounts on navigation, so going Back from the Calendar step to the Instance step loses all entered config. Lift Instance (and Calendar) field values into `SetupPage` (or persist to sessionStorage) so Back preserves them.
Purpose: First-run operator can navigate Back without re-typing; the DB row makes sense; no confusing implementation copy.
Output: Instance step with trimmed copy, a read-only DB-name field, and persistent field values across Back navigation.
</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/12-initial-setup-wizard/12-UAT.md
@.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md
# Files to edit
@apps/pwa/src/routes/SetupPage.tsx
@apps/pwa/src/routes/SetupPage.test.tsx
# Contract this plan consumes (added by Plan 06)
@apps/pwa/src/api/client.ts
</context>
<tasks>
<task type="auto">
<name>Task 1: Drop the DB-vs-env-file aside + add read-only DB-name field (gaps 1, 3-frontend)</name>
<files>apps/pwa/src/routes/SetupPage.tsx, apps/pwa/src/routes/SetupPage.test.tsx</files>
<action>
Gap 1 — In `Step2Config` (apps/pwa/src/routes/SetupPage.tsx, the intro `<p>` at ~line 547-558), remove the sentence "These are written to the database — not your environment file." Keep the first sentence ("Enter your instance's connection details.") and the surrounding paragraph styling intact.
Gap 3 (frontend) — Render a read-only, disabled field showing the env-derived DB name directly under the App URL field block (the App URL `<div>` ends ~line 575, just before the OIDC Issuer block):
- Fetch the DB name from the status endpoint. Import `fetchSetupStatus` from '../api/client.js' (already exported) and read `dbName` from its response (the `dbName?: string | null` field added by Plan 06). Use `useQuery({ queryKey: ['setupStatus'], queryFn: fetchSetupStatus, staleTime: 0, retry: false })` inside Step2Config (or lift the query to SetupPage and pass `dbName` as a prop — executor's choice, but keep it self-contained to the Instance step).
- Render a labelled input mirroring the existing field markup (reuse `labelStyle`, `inputStyle(false)`, `helperStyle`): label "Database" (or "Database name"), value = the fetched dbName (fallback to an empty string / a "—" placeholder while loading or if null), with `readOnly` AND `disabled` set, a greyed-out appearance (set the input's `background`/`color` to a muted token, e.g. `var(--color-surface-dim)` / `var(--color-text-secondary)`), and `aria-readonly="true"`. Helper text: explains this is configured via the server's Docker environment (DB_HOST/DB_PORT/DB_USER/DB_PASSWORD), not entered here — so the "database connection verified" row below has a referent. NEVER render DB_HOST/DB_USER/DB_PASSWORD — only the name.
- Do NOT change the existing DB ValidationRow ("Database connection verified.") — keep it as-is per the UAT "missing" note.
In apps/pwa/src/routes/SetupPage.test.tsx: assert the dropped sentence is no longer present (query the Instance step text and assert "not your environment file" is absent), and assert the read-only DB-name field renders disabled/readOnly with the mocked dbName. Mock `fetchSetupStatus` (or the client module) to return `{ setupComplete: false, dbName: 'familysync' }`.
</action>
<verify>
<automated>cd apps/pwa && pnpm test -- SetupPage 2>&1 | tail -20</automated>
</verify>
<done>Instance step intro no longer contains "not your environment file"; a disabled+readOnly DB-name field (value from status dbName) renders under App URL; the existing DB validation row is unchanged; SetupPage.test.tsx GREEN.</done>
</task>
<task type="auto">
<name>Task 2: Preserve wizard field values across Back navigation (gap 4)</name>
<files>apps/pwa/src/routes/SetupPage.tsx, apps/pwa/src/routes/SetupPage.test.tsx</files>
<action>
Lift the Instance-step field values (appUrl, oidcIssuer, oidcClientId, vapidPublicKey) and the Calendar-step field values (email, plus credential-verified flag if needed for UX) out of the per-step local `useState` so they survive step unmount/remount.
Recommended approach (state lifted to the SetupPage parent — matches the existing "parent owns `step`" structure):
- In `SetupPage` (the component owning `useState<WizardStep>`), add state for the Instance fields: `appUrl`, `oidcIssuer`, `oidcClientId`, `vapidPublicKey` (the app password is sensitive — do NOT lift/persist the password value; only the non-secret email may be lifted if convenient, but the password must stay local and cleared on unmount per T-12-15).
- Pass these values + their setters down to `Step2Config` as props; replace the component-local `useState('')` declarations (~lines 439-442) with the props. Validation/mutation logic stays inside Step2Config.
- Ensure that when navigating Back from Step 3 → Step 2, the Instance fields are still populated (because the parent now holds them). When navigating Back from Step 2 → Step 1 and forward again, values also persist.
Alternative (sessionStorage) is acceptable if simpler, but MUST NOT persist the Fastmail app password (T-12-15) — only the non-secret Instance fields. Prefer the lifted-state approach.
Security: the Fastmail app password (Step 3) is NOT lifted and NOT persisted to sessionStorage — it remains in Step3Credential local state and is cleared on unmount (T-12-15 preserved).
In apps/pwa/src/routes/SetupPage.test.tsx: add a test that fills the Instance fields, advances to the Calendar step, navigates Back, and asserts the Instance field values are still present (inputs retain their values). Add an assertion that the password field is NOT persisted across navigation (re-mount of Step 3 starts empty).
</action>
<verify>
<automated>cd apps/pwa && pnpm test -- SetupPage 2>&1 | tail -20</automated>
</verify>
<done>Filling the Instance step, advancing, then clicking Back restores all four Instance field values; the Fastmail app password is never persisted across navigation; SetupPage.test.tsx GREEN.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| operator input → wizard state | Non-secret config + a sensitive app password are entered here |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-12-15 | Information Disclosure | Step3 app password | mitigate | App password stays in Step3 local state; NOT lifted to parent, NOT written to sessionStorage; cleared on unmount; field remains type="password" |
| T-12-14 | Tampering (XSS) | Instance/DB-name copy | mitigate | All new copy + dbName rendered as plain-text JSX children; no dangerouslySetInnerHTML (grep returns 0) |
| T-12-3DB | Information Disclosure | DB-name field | mitigate | Only the dbName from status is rendered; DB_HOST/DB_USER/DB_PASSWORD never fetched or shown |
</threat_model>
<verification>
- `cd apps/pwa && pnpm test -- SetupPage` GREEN
- `grep -n "not your environment file" apps/pwa/src/routes/SetupPage.tsx` returns nothing
- `grep -c "dangerouslySetInnerHTML" apps/pwa/src/routes/SetupPage.tsx` is 0
- `grep -nE "sessionStorage|localStorage" apps/pwa/src/routes/SetupPage.tsx` — if present, confirm no password/appPassword key is written
- `cd apps/pwa && pnpm typecheck` clean
</verification>
<success_criteria>
- Gap 1 closed: implementation aside removed.
- Gap 3 (frontend) closed: read-only DB-name field gives the DB validation row a referent.
- Gap 4 closed: Back navigation preserves Instance field values; app password never persisted.
</success_criteria>
<output>
Create `.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md` when done
</output>
.planning/phases/12-initial-setup-wizard/12-06-PLAN.md
+146
View File
@@ -0,0 +1,146 @@
---
phase: 12-initial-setup-wizard
plan: 06
type: execute
wave: 1
depends_on: []
files_modified:
- apps/api/src/routes/setup.ts
- apps/api/tests/routes/setup.test.ts
- apps/pwa/src/api/client.ts
autonomous: true
gap_closure: true
requirements: [SETUP-02]
must_haves:
truths:
- "Entering a wrong/invalid VAPID public key in the wizard fails the VAPID validation row"
- "POST /api/setup/validate/vapid returns 400 when the submitted vapid_public_key does not match the env VAPID_PUBLIC_KEY"
- "The GET /api/setup/status response exposes the non-secret env DB name (no secrets)"
- "VAPID_PRIVATE_KEY is never returned in any response (T-12-06 preserved)"
artifacts:
- path: "apps/api/src/routes/setup.ts"
provides: "validate/vapid asserts submitted key matches env public key; status returns dbName"
contains: "VAPID_PUBLIC_KEY"
- path: "apps/pwa/src/api/client.ts"
provides: "SetupStatusResponse.dbName field"
contains: "dbName"
key_links:
- from: "POST /api/setup/validate/vapid"
to: "app_config.vapid_public_key"
via: "compare submitted key against process.env.VAPID_PUBLIC_KEY"
pattern: "vapid_public_key"
- from: "GET /api/setup/status"
to: "process.env.DB_NAME"
via: "non-secret DB name surfaced in response"
pattern: "dbName"
---
<objective>
Close UAT gaps 2 and 3 on the backend setup-route surface.
Gap 2 (major): `POST /api/setup/validate/vapid` validates the *env* VAPID pair via `webpush.setVapidDetails` but never compares against the wizard-entered `vapid_public_key`. An operator typed `BH123` (clearly invalid) and the row still went green because the env pair was valid. The fix: assert the submitted/persisted `vapid_public_key` equals `process.env.VAPID_PUBLIC_KEY` (the public half of the configured pair) so a wrong key fails the row and gates Continue.
Gap 3 (minor, backend half): the DB connection is configured via Docker env (DB_HOST/PORT/USER/PASSWORD), not collected in the wizard, so the "database connection verified" row has no on-screen referent. Surface the **non-secret** DB name so the PWA (Plan 05) can render a read-only field giving that row a referent.
Purpose: A wrong VAPID key must fail (push silently breaks in production otherwise — SETUP-02); the DB row must reference something visible.
Output: `validate/vapid` rejects mismatched keys; `GET /api/setup/status` returns `{ setupComplete, dbName }`.
</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/12-initial-setup-wizard/12-UAT.md
@.planning/phases/12-initial-setup-wizard/12-02-SUMMARY.md
# Files to edit (already in context for the planner; executor should read before editing)
@apps/api/src/routes/setup.ts
@apps/api/src/api/../tests/routes/setup.test.ts
@apps/pwa/src/api/client.ts
</context>
<tasks>
<task type="auto" tdd="true">
<name>Task 1: validate/vapid asserts submitted key matches env public key (gap 2)</name>
<files>apps/api/src/routes/setup.ts, apps/api/tests/routes/setup.test.ts</files>
<behavior>
- When VAPID_PRIVATE_KEY/VAPID_PUBLIC_KEY env are set AND app_config.vapid_public_key equals process.env.VAPID_PUBLIC_KEY → 200 { ok: true } (existing happy path preserved).
- When app_config.vapid_public_key is present but does NOT equal process.env.VAPID_PUBLIC_KEY (e.g. "BH123") → 400 { ok: false } with a non-echoing error message; the response NEVER contains VAPID_PRIVATE_KEY.
- When app_config.vapid_public_key row is absent → 400 { ok: false } (cannot validate without the operator-submitted key).
- When env VAPID keys are missing → existing 400 path preserved.
- When setup is locked → existing 423 path preserved (isSetupLocked() first).
</behavior>
<action>
In the `POST /validate/vapid` handler (apps/api/src/routes/setup.ts, currently ~line 202), after the existing `isSetupLocked()` 423 guard and the existing env-presence check, add an equality assertion BEFORE the `webpush.setVapidDetails` structural check:
- Read the operator-submitted public key from app_config: SELECT value FROM app_config WHERE key = 'vapid_public_key' (use the existing `db.select({ value: appConfig.value }).from(appConfig).where(eq(appConfig.key, 'vapid_public_key')).limit(1)` idiom already used by the validate/oidc handler).
- If that row is absent OR its value !== process.env.VAPID_PUBLIC_KEY, return 400 { ok: false, error: 'VAPID public key does not match the configured key pair. Paste the exact VAPID_PUBLIC_KEY printed by `npm run generate-secrets`.' }. This is the gap-2 assertion: a wrong key now fails the row.
- Keep the existing `webpush.setVapidDetails(subject, publicKey, privateKey)` structural check AFTER the equality check, still reading BOTH keys ONLY from process.env. Do NOT read VAPID_PRIVATE_KEY from app_config and NEVER return it (T-12-06 / D-01 preserved — the equality compares the submitted PUBLIC key to the env PUBLIC key only).
In apps/api/tests/routes/setup.test.ts, extend the validate/vapid suite (RED first): add a test that mocks app_config.vapid_public_key returning a value different from process.env.VAPID_PUBLIC_KEY and asserts a 400 plus that the JSON body has no key matching VAPID_PRIVATE_KEY; update the existing happy-path test so the mocked app_config value equals process.env.VAPID_PUBLIC_KEY (otherwise it would now 400). Add a test for the absent-row → 400 case.
</action>
<verify>
<automated>cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm test -- setup 2>&1 | tail -20</automated>
</verify>
<done>validate/vapid returns 400 for a mismatched/absent submitted key and 200 only when the submitted key equals process.env.VAPID_PUBLIC_KEY; no response path returns VAPID_PRIVATE_KEY; setup.test.ts vapid suite GREEN.</done>
</task>
<task type="auto">
<name>Task 2: Expose non-secret DB name via GET /api/setup/status (gap 3 backend)</name>
<files>apps/api/src/routes/setup.ts, apps/api/tests/routes/setup.test.ts, apps/pwa/src/api/client.ts</files>
<action>
In the `GET /status` handler (apps/api/src/routes/setup.ts, ~line 86), include the non-secret DB name in the response alongside the existing `setupComplete`. Source the name from `process.env.DB_NAME` (the Drizzle/mysql2 connection uses DB_HOST/DB_PORT/DB_USER/DB_PASSWORD/DB_NAME — confirm the exact env var name by grepping apps/api/src/db/client.ts; use whatever that file reads for the database name). Return `{ setupComplete, dbName }` where dbName is `process.env.DB_NAME ?? null`.
ONLY the database NAME is surfaced — never DB_HOST, DB_USER, or DB_PASSWORD (those are connection secrets/topology; the name alone is the on-screen referent the operator asked for). Do NOT add DB_PASSWORD or any secret to any response.
In apps/pwa/src/api/client.ts, add `dbName?: string | null` to the `SetupStatusResponse` interface (~line 538) so the PWA (Plan 05) consumes a typed field. No other client.ts changes.
In apps/api/tests/routes/setup.test.ts, update the GET /status test(s) to assert the response includes `dbName` reflecting the mocked/process env DB name (set process.env.DB_NAME in the test or assert the key is present).
</action>
<verify>
<automated>cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm test -- setup 2>&1 | tail -15 && cd ../pwa && pnpm typecheck 2>&1 | tail -5</automated>
</verify>
<done>GET /api/setup/status returns `{ setupComplete, dbName }` with the non-secret DB name (no DB_PASSWORD/DB_HOST/DB_USER); SetupStatusResponse carries `dbName?: string | null`; api setup.test.ts GREEN; pwa typecheck clean.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| pre-auth client → /api/setup/* | Unauthenticated operator input crosses here before OIDC is configured |
| process.env → response body | Secret env vars must not leak into pre-auth JSON |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-12-06 | Information Disclosure | validate/vapid | mitigate | VAPID_PRIVATE_KEY read ONLY from process.env, never compared/returned; equality check uses PUBLIC keys only; test asserts no VAPID_PRIVATE_KEY in body |
| T-12-3DB | Information Disclosure | GET /status dbName | mitigate | Only process.env.DB_NAME surfaced; DB_HOST/DB_USER/DB_PASSWORD never added to any response (grep-checked) |
| T-12-04 | Tampering/Replay | all setup routes | mitigate | isSetupLocked() remains the first await in every handler (unchanged) |
</threat_model>
<verification>
- `cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm test -- setup` GREEN
- `grep -nE "VAPID_PRIVATE_KEY" apps/api/src/routes/setup.ts` shows it only inside the env-only structural check, never in a response/compare against app_config
- `grep -nE "DB_PASSWORD|DB_HOST|DB_USER" apps/api/src/routes/setup.ts | grep -i "status\|c.json"` returns nothing (no secret/topology in status response)
- `cd apps/pwa && pnpm typecheck` clean
</verification>
<success_criteria>
- Gap 2 closed: a wrong wizard-entered VAPID public key fails validate/vapid (400) and therefore gates Continue.
- Gap 3 backend closed: status exposes the non-secret DB name for the PWA read-only field.
- No secret (VAPID_PRIVATE_KEY, DB_PASSWORD) appears in any response.
</success_criteria>
<output>
Create `.planning/phases/12-initial-setup-wizard/12-06-SUMMARY.md` when done
</output>
.planning/phases/12-initial-setup-wizard/12-07-PLAN.md
+146
View File
@@ -0,0 +1,146 @@
---
phase: 12-initial-setup-wizard
plan: 07
type: execute
wave: 1
depends_on: []
files_modified:
- apps/pwa/src/App.tsx
- apps/pwa/src/App.test.tsx
- apps/pwa/src/components/SetupBanner.tsx
autonomous: true
gap_closure: true
requirements: [SETUP-01, SETUP-04]
must_haves:
truths:
- "After setup is complete, manually visiting /setup shows the 'already complete' surface (or redirects away) — not the wizard"
- "After completing the wizard (incl. Fastmail credential) and landing in the app, the /calendar 'Set up your calendar' banner does NOT show for the operator"
- "The setup gate respects the loading state to avoid a flash of the wizard"
artifacts:
- path: "apps/pwa/src/App.tsx"
provides: "/setup route gated on setupComplete (alreadyLocked or redirect); ['me'] freshness reconciled with wizard completion"
contains: "alreadyLocked"
key_links:
- from: "App.tsx /setup route"
to: "setupQuery.data.setupComplete"
via: "alreadyLocked={setupComplete === true} or Navigate to /calendar"
pattern: "alreadyLocked|setupComplete"
- from: "SetupBanner needsProviderSetup"
to: "['me'] query freshness"
via: "['me'] refetched after wizard completion so the banner reflects the claimed credential"
pattern: "needsProviderSetup|invalidateQueries"
---
<objective>
Close UAT gaps 5 and 6 — both on the PWA app-shell gate (`App.tsx`), with the banner component (`SetupBanner.tsx`).
Gap 5 (major): `App.tsx` renders `<Route path="/setup" element={<SetupPage />} />` with no `alreadyLocked` prop and no `setupComplete` check. The `*` gate only redirects OTHER routes TO /setup when incomplete; there is no reverse guard. When `setupComplete === true`, manually visiting /setup still mounts the full wizard. The backend already 423s mutations, so this is purely a frontend gating gap. Fix: gate the /setup route on `setupComplete` — pass `alreadyLocked={setupComplete === true}` (SetupPage already supports this prop → renders Surface 8 "Setup already complete") or `Navigate` to /calendar; respect `setupLoading` to avoid a flash.
Gap 6 (major, root_cause PRELIMINARY): after finishing the wizard and landing on /calendar, the "Set up your calendar / Set up now" banner still shows. Task 1 is an investigation step to confirm the mechanism before prescribing the exact fix; Task 2 implements the confirmed fix.
Purpose: A completed instance must not re-expose the wizard, and must not nag the operator to set up a calendar they already configured in the wizard.
Output: /setup route gated post-completion; ['me'] reconciled so the banner does not show after wizard completion.
</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/12-initial-setup-wizard/12-UAT.md
@.planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md
@.planning/phases/12-initial-setup-wizard/12-04-SUMMARY.md
# Files to edit + investigate
@apps/pwa/src/App.tsx
@apps/pwa/src/App.test.tsx
@apps/pwa/src/components/SetupBanner.tsx
# Reference (do not edit unless Task 1 investigation proves a backend linking gap)
@apps/api/src/routes/me.ts
@apps/api/src/routes/setup.ts
@apps/api/src/auth/user.ts
</context>
<tasks>
<task type="auto">
<name>Task 1: Gate the /setup route on setupComplete (gap 5)</name>
<files>apps/pwa/src/App.tsx, apps/pwa/src/App.test.tsx</files>
<action>
In apps/pwa/src/App.tsx, the `<Route path="/setup" element={<SetupPage />} />` (line ~139) currently mounts the wizard unconditionally. Add a reverse guard using the already-present `setupComplete` / `setupLoading` values (derived at lines ~132-133 from `setupQuery`):
- While `setupLoading` is true → render the existing no-flash placeholder (`<div aria-hidden="true" />`) for the /setup element (do not show the wizard before status resolves).
- When `setupComplete === true` → render `<SetupPage alreadyLocked={true} />` (SetupPage already supports the `alreadyLocked` prop → Surface 8 "Setup already complete"). Using the prop (rather than Navigate) keeps the operator on /setup with a clear terminal surface, matching the UAT expectation that manual /setup navigation shows the "already complete" surface. (Navigate to /calendar is an acceptable alternative if the executor finds the prop path conflicts with routing — but the prop path is preferred and already wired/tested in SetupPage.)
- When `setupComplete === false` (or undefined post-load) → render `<SetupPage />` (the active wizard) as today.
Implement this by replacing the static `element={<SetupPage />}` with an inline conditional element expression mirroring the existing `*`-route gate style.
In apps/pwa/src/App.test.tsx: add a test that mocks `fetchSetupStatus` → `{ setupComplete: true }`, navigates to /setup (set `window.history.pushState({}, '', '/setup')` in the test per the existing URL-isolation pattern), and asserts the "Setup already complete" surface renders (and the active wizard's Step 1 heading "Welcome to FamilySync Setup" does NOT). Keep the existing setupComplete:false → wizard test passing.
</action>
<verify>
<automated>cd apps/pwa && pnpm test -- App 2>&1 | tail -20</automated>
</verify>
<done>Visiting /setup with setupComplete===true renders the "Setup already complete" surface (not the wizard); setupComplete===false still renders the wizard; loading state shows no wizard flash; App.test.tsx GREEN.</done>
</task>
<task type="auto">
<name>Task 2: Diagnose + fix the persistent calendar banner (gap 6)</name>
<files>apps/pwa/src/App.tsx, apps/pwa/src/components/SetupBanner.tsx, apps/pwa/src/App.test.tsx</files>
<action>
STEP A — Investigate/confirm root cause (the UAT root_cause is PRELIMINARY). Determine which of two mechanisms causes the banner to persist after wizard completion. Use the code already in context plus a focused trace:
- Mechanism (i) — claiming/linking gap: the wizard credential is stored against the unclaimed user (oidcIss=null), and first OIDC login does NOT bind it to the operator, so `needsProviderSetup` stays true. Verify by reading `apps/api/src/auth/user.ts` upsertUser first-login-claims branch: confirm whether the claim `db.update(users)...where(eq(users.id, unclaimed.id))` PRESERVES the same users.id (so the member_credentials row keyed on userId stays linked → needsProviderSetup=false). The 12-03-SUMMARY and the claim branch indicate the id IS preserved and is_admin/credential link is retained — i.e. mechanism (i) is NOT the cause. CONFIRM this by reading user.ts directly; if confirmed, the credential IS linked and needsProviderSetup is correctly false after first login.
- Mechanism (ii) — ['me'] staleness/refetch gap: `meQuery` uses `staleTime: 5 * 60 * 1000` (App.tsx ~line 88 and SetupBanner.tsx ~line 40). If `['me']` was populated BEFORE wizard completion / first claim (e.g. an earlier visit), the cached `needsProviderSetup=true` is served for up to 5 minutes after the operator authenticates post-wizard, so the banner shows even though the DB now says false.
Record the confirmed mechanism in the SUMMARY. The expected finding (per the claim-branch evidence) is mechanism (ii): a ['me'] freshness/refetch gap, NOT a linking gap.
STEP B — Implement the fix for the CONFIRMED mechanism:
- If mechanism (ii) (expected): ensure `['me']` is fresh on entry to the authenticated app shell after wizard completion. Preferred: invalidate/refetch `['me']` when the app transitions into the `setupComplete===true` shell, OR reduce the staleness window so the post-auth boot refetches member status. Concretely — when the setup gate resolves to the completed shell (the `setupComplete===true` branch in App.tsx), trigger a one-shot `queryClient.invalidateQueries({ queryKey: ['me'] })` (guarded so it does not loop), or set the `['me']` query's `staleTime` to 0 for the boot fetch so `needsProviderSetup` reflects the just-claimed credential. Keep the SetupBanner's success-only dismissal contract intact (do NOT add an X/dismiss button — the banner must still clear via needsProviderSetup=false).
- If STEP A instead confirms mechanism (i) (a real linking gap): the fix belongs on the backend — adjust the claim/credential linking in apps/api/src/auth/user.ts or apps/api/src/routes/setup.ts so the operator's claimed user owns the wizard-stored credential (needsProviderSetup=false). In that case add apps/api/src/auth/user.ts (+ its test) to files_modified and add a backend regression test asserting the claimed user has a credential.
Do NOT add a dismiss button to SetupBanner.tsx (T-05-24 / success-only contract). The banner must continue to clear ONLY via needsProviderSetup becoming false.
In apps/pwa/src/App.test.tsx (or SetupBanner test): add a regression test for the confirmed mechanism. For mechanism (ii): assert that on the completed-shell boot, ['me'] is refetched (or staleTime is 0) such that a `needsProviderSetup=false` response hides the banner; assert the SetupBanner is absent when needsProviderSetup is false.
</action>
<verify>
<automated>cd apps/pwa && pnpm test -- App SetupBanner 2>&1 | tail -25</automated>
</verify>
<done>Root cause confirmed and documented; the fix ensures `needsProviderSetup` reflects the wizard-claimed credential on app entry so the "Set up your calendar" banner does NOT show post-wizard; no dismiss button added; tests GREEN.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| client routing → setup surface | Frontend gating of /setup; backend already enforces 423 |
| ['me'] cache → UI gating | Stale member status must not mislead UX (banner) |
## STRIDE Threat Register
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-12-04 | Tampering/Replay | /setup post-completion | mitigate | Frontend reverse-gate renders Surface 8; backend 423 on mutations remains the authoritative boundary (unchanged) |
| T-12-10 | Spoofing | first-login claim | accept | Claim query is `oidcIss IS NULL AND claimed=false LIMIT 1`; investigation confirms id-preserving link; no change unless mechanism (i) found |
| T-05-24 | Tampering (XSS) | SetupBanner | mitigate | No dismiss button added; copy stays plain-text JSX; success-only dismissal contract preserved |
</threat_model>
<verification>
- `cd apps/pwa && pnpm test -- App SetupBanner` GREEN
- `grep -nE "alreadyLocked" apps/pwa/src/App.tsx` shows the /setup route is gated on setupComplete
- `grep -nE "X button|dismiss|onClose.*banner|aria-label=\"Dismiss\"" apps/pwa/src/components/SetupBanner.tsx` returns nothing new (no dismiss added)
- SUMMARY documents the confirmed gap-6 mechanism (i vs ii)
- `cd apps/pwa && pnpm typecheck` clean
</verification>
<success_criteria>
- Gap 5 closed: /setup post-completion shows the "already complete" surface, not the wizard.
- Gap 6 closed: the calendar banner does not show after the operator completes the wizard; root cause documented; success-only banner contract preserved.
</success_criteria>
<output>
Create `.planning/phases/12-initial-setup-wizard/12-07-SUMMARY.md` when done
</output>