147 lines
11 KiB
Markdown
147 lines
11 KiB
Markdown
---
|
|
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>
|