chore: merge executor worktree (worktree-agent-ad2593f5ac87f6852)

.planning/phases/12-initial-setup-wizard/12-07-SUMMARY.md

@@ -0,0 +1,127 @@
+---
+phase: 12-initial-setup-wizard
+plan: 07
+subsystem: ui
+tags: [react, tanstack-query, react-router, setup-wizard, pwa, oidc]
+
+# Dependency graph
+requires:
+  - phase: 12-initial-setup-wizard
+    provides: "SetupPage wizard with alreadyLocked Surface 8; /setup route gate; first-login-claim in upsertUser; SetupBanner self-service onboarding (12-03/12-04)"
+provides:
+  - "/setup route reverse-gated on setupComplete — Surface 8 ('Setup already complete') after completion, never re-mounts the wizard"
+  - "['me'] freshness fix (staleTime 0) so the post-wizard 'Set up your calendar' banner clears once the claimed credential is in effect"
+  - "SetupBanner.test.tsx regression coverage for the success-only dismissal contract + stale-cache refetch"
+affects: [setup-wizard, onboarding, pwa-app-shell]
+
+# Tech tracking
+tech-stack:
+  added: []
+  patterns:
+    - "Reverse route-gate: conditional route element keyed on setupComplete/setupLoading mirroring the existing `*`-route gate"
+    - "staleTime 0 on a boot-critical ['me'] query so authenticated-shell entry always reflects fresh server truth (post-claim)"
+
+key-files:
+  created:
+    - apps/pwa/src/components/SetupBanner.test.tsx
+  modified:
+    - apps/pwa/src/App.tsx
+    - apps/pwa/src/App.test.tsx
+    - apps/pwa/src/components/SetupBanner.tsx
+
+key-decisions:
+  - "D-12-07-GAP6-MECH: gap 6 root cause is mechanism (ii) — ['me'] client-cache staleness, NOT a backend linking gap. upsertUser's first-login claim preserves users.id (where eq(users.id, unclaimed.id)), so the wizard-stored CalDAV credential stays linked and the DB reports needsProviderSetup=false. Fix is client-only."
+  - "D-12-07-STALE0: ['me'] staleTime set to 0 in both App.tsx (boot) and SetupBanner.tsx so a pre-claim stale entry is refetched on shell entry; success-only dismissal contract preserved (no dismiss/X button added)."
+  - "D-12-07-LOCKED-PROP: /setup reverse-gate uses SetupPage alreadyLocked prop (Surface 8) rather than Navigate, keeping the operator on /setup with a terminal surface per UAT expectation."
+
+patterns-established:
+  - "Reverse-gate a standalone route by swapping its element via the same loading/complete derivation used by the app-shell gate."
+
+requirements-completed: [SETUP-01, SETUP-04]
+
+# Metrics
+duration: 11min
+completed: 2026-06-16
+---
+
+# Phase 12 Plan 07: UAT Gap-Closure (gaps 5 & 6) Summary
+
+**The /setup wizard no longer re-mounts after completion (shows Surface 8 'Setup already complete'), and the '/calendar' setup banner no longer nags the operator after they finish the wizard — fixed by reverse-gating the route and making the ['me'] query fresh on shell entry.**
+
+## Performance
+
+- **Duration:** ~11 min
+- **Started:** 2026-06-16T21:19Z
+- **Completed:** 2026-06-16T21:24Z
+- **Tasks:** 2
+- **Files modified:** 4 (3 modified, 1 created)
+
+## Accomplishments
+
+- **Gap 5 closed:** `/setup` is now reverse-gated on `setupComplete`. After completion, manually visiting `/setup` renders `SetupPage alreadyLocked` → Surface 8 "Setup already complete" (the backend already 423s setup mutations; this is the matching frontend gate). Loading state renders a no-flash placeholder; `setupComplete===false` still mounts the active wizard.
+- **Gap 6 closed:** the "Set up your calendar" banner no longer persists after the operator completes the wizard. Root cause confirmed as a `['me']` cache-staleness gap (mechanism ii), NOT a backend linking gap. Set `['me']` `staleTime` to 0 in both `App.tsx` (boot) and `SetupBanner.tsx` so a pre-claim stale entry is refetched on entry to the authenticated shell — `needsProviderSetup` then reflects the just-claimed credential and the banner hides.
+- **Regression coverage added:** new `SetupBanner.test.tsx` (3 tests) + 2 new App reverse-gate tests. Full PWA suite green at 258 tests; typecheck clean.
+
+## Gap 6 — Root Cause Investigation (Task 2 Step A)
+
+The UAT `root_cause` was flagged PRELIMINARY. Reading `apps/api/src/auth/user.ts` `upsertUser` confirmed the first-login claim branch (lines ~118-130) updates the unclaimed row via `where(eq(users.id, unclaimed.id))` — it **preserves the same `users.id`**. Because `member_credentials` is keyed on `userId`, the wizard-stored CalDAV credential stays linked to the claimed operator row, so the DB correctly returns `needsProviderSetup=false` after first OIDC login.
+
+**Conclusion: mechanism (i) (a backend claiming/linking gap) is NOT the cause** — consistent with the 12-03 summary and threat-register disposition `T-12-10 = accept`. The cause is **mechanism (ii)**: `['me']` had `staleTime: 5 * 60 * 1000`, so a cache entry populated before the claim (e.g. a pre-auth visit) served `needsProviderSetup=true` for up to 5 minutes after the operator authenticated post-wizard. No backend change was made; the fix is purely client-side cache freshness.
+
+## Task Commits
+
+1. **Task 1: Gate the /setup route on setupComplete (gap 5)** — `fdcb4dc` (feat)
+   - (also carried the App.tsx `['me']` staleTime → 0 edit, staged together; the SetupBanner-side change + its test landed in Task 2)
+2. **Task 2: Diagnose + fix the persistent calendar banner (gap 6)** — `2b3569f` (fix)
+
+## Files Created/Modified
+
+- `apps/pwa/src/App.tsx` — reverse-gated `/setup` route element (loading placeholder / `alreadyLocked` Surface 8 / active wizard); boot `['me']` `staleTime` → 0 with mechanism note.
+- `apps/pwa/src/components/SetupBanner.tsx` — `['me']` `staleTime` 5min → 0 (gap-6 freshness); no dismiss button added; success-only dismissal contract restated in comments.
+- `apps/pwa/src/App.test.tsx` — SetupPage mock now respects `alreadyLocked`; 2 new reverse-gate tests (already-complete surface + active wizard on `/setup`).
+- `apps/pwa/src/components/SetupBanner.test.tsx` — NEW: banner absent when `needsProviderSetup=false`, present (no dismiss button) when true, and stale-cache refetch hides the banner on mount (staleTime 0).
+
+## Decisions Made
+
+- **D-12-07-GAP6-MECH** — gap 6 is mechanism (ii) `['me']` staleness, not a linking gap (evidence: id-preserving claim in `upsertUser`).
+- **D-12-07-STALE0** — `['me']` `staleTime` set to 0 in App.tsx + SetupBanner.tsx; success-only dismissal contract preserved.
+- **D-12-07-LOCKED-PROP** — `/setup` reverse-gate uses the `alreadyLocked` prop (Surface 8), not `Navigate`, per the UAT expectation that manual `/setup` navigation shows the "already complete" surface.
+
+## Deviations from Plan
+
+None — plan executed as written. The plan's expected gap-6 finding (mechanism ii) was confirmed by the Task 2 Step A investigation; the prescribed staleTime fix was applied. No architectural changes; no backend changes required.
+
+## Issues Encountered
+
+- **Pre-existing PWA lint errors (out of scope).** `pnpm lint` in `apps/pwa` reports 22 errors in `src/api/setupClient.contract.test.ts` (`no-unsafe-*`) and `src/routes/SetupPage.test.tsx:152` (`no-unused-vars`). Neither file was touched by this plan; both were last modified in earlier Phase-12 commits. The four files this plan touched lint clean (exit 0). Logged to `.planning/phases/12-initial-setup-wizard/deferred-items.md` and left untouched per the executor SCOPE BOUNDARY rule. Recommend a follow-up lint-cleanup quick task.
+
+## Verification
+
+- `apps/pwa` full suite: **258 tests passed (22 files)**; `App.test.tsx` 8 passed; `SetupBanner.test.tsx` 3 passed.
+- `pnpm typecheck` (apps/pwa): clean.
+- `grep -nE "alreadyLocked" apps/pwa/src/App.tsx` → `/setup` route gated on setupComplete (Surface 8).
+- `grep` for new dismiss/X button in `SetupBanner.tsx` → none added (only contract comments).
+- Touched-files lint: `eslint` over the 4 files → exit 0.
+
+## Known Stubs
+
+None.
+
+## User Setup Required
+
+None — no external service configuration required.
+
+## Next Phase Readiness
+
+- Both UAT major gaps (5 and 6) are closed in code with regression tests. Ready for re-UAT of the post-completion `/setup` surface and the post-wizard calendar banner.
+- Remaining UAT gaps (if any from 12-05/12-06) are tracked in their own gap-closure plans; this plan scoped only gaps 5 & 6.
+- Pre-existing PWA lint debt deferred (see deferred-items.md) — does not block this plan's UI behavior.
+
+## Self-Check: PASSED
+
+- All 5 created/modified files present on disk.
+- All 3 commits (`fdcb4dc`, `2b3569f`, `96c4913`) present in git history.
+
+---
+*Phase: 12-initial-setup-wizard*
+*Completed: 2026-06-16*

.planning/phases/12-initial-setup-wizard/deferred-items.md

@@ -0,0 +1,12 @@
+# Deferred Items — Phase 12
+
+Out-of-scope discoveries logged during execution. Not fixed by the originating plan.
+
+## 12-07 — Pre-existing PWA lint errors (out of scope)
+
+Discovered during 12-07 verification (`pnpm lint` in apps/pwa). 22 errors, NOT introduced by 12-07 (the four files 12-07 touched lint clean):
+
+- `apps/pwa/src/api/setupClient.contract.test.ts` — `@typescript-eslint/no-unsafe-*` (any-typed `res.body` access in contract assertions)
+- `apps/pwa/src/routes/SetupPage.test.tsx:152` — `no-unused-vars` (`container` assigned but unused)
+
+Both files were last modified in earlier Phase-12 commits (e.g. 066b69f), confirming pre-existing. Left untouched per the executor SCOPE BOUNDARY rule (only auto-fix issues directly caused by the current task). Recommend a follow-up lint-cleanup quick task.

apps/pwa/src/App.test.tsx

@@ -25,8 +25,17 @@ vi.mock('./components/CalendarShell.js', () => ({
   CalendarShell: () => <div data-testid="calendar-shell">CalendarShell</div>,
 }));
 
+// SetupPage mock respects the `alreadyLocked` prop so the reverse-gate test (gap 5)
+// can distinguish the active wizard (Step 1 "Welcome to FamilySync Setup") from the
+// Surface 8 "Setup already complete" terminal surface. The real SetupPage renders
+// these two surfaces based on this exact prop — see routes/SetupPage.tsx.
 vi.mock('./routes/SetupPage.js', () => ({
-  SetupPage: () => <div data-testid="setup-page">SetupPage</div>,
+  SetupPage: ({ alreadyLocked }: { alreadyLocked?: boolean }) =>
+    alreadyLocked ? (
+      <div data-testid="setup-page">Setup already complete</div>
+    ) : (
+      <div data-testid="setup-page">Welcome to FamilySync Setup</div>
+    ),
 }));
 
 vi.mock('./components/AppNav.js', () => ({
@@ -177,6 +186,41 @@ describe('App — setup-status gate', () => {
       expect(screen.getByTestId('setup-page')).toBeInTheDocument();
     });
   });
+
+  // gap 5 (T-12-04): manually visiting /setup AFTER setup is complete must show the
+  // "Setup already complete" surface (alreadyLocked), NOT re-mount the active wizard.
+  it('renders the "already complete" surface (not the wizard) on /setup when setupComplete is true', async () => {
+    mockFetchSetupStatus.mockResolvedValue({ setupComplete: true });
+    // Navigate directly to /setup (URL-isolation pattern — beforeEach reset to /)
+    window.history.pushState({}, '', '/setup');
+
+    const queryClient = makeQueryClient();
+    renderApp(queryClient);
+
+    await waitFor(() => {
+      expect(screen.getByTestId('setup-page')).toHaveTextContent('Setup already complete');
+    });
+
+    // The active wizard's Step 1 heading must NOT render when setup is complete
+    expect(screen.queryByText('Welcome to FamilySync Setup')).toBeNull();
+    // Standalone wizard surface — no AppNav shell on /setup
+    expect(screen.queryByTestId('app-nav')).toBeNull();
+  });
+
+  // gap 5 counterpart: /setup with setupComplete false still mounts the active wizard.
+  it('renders the active wizard on /setup when setupComplete is false', async () => {
+    mockFetchSetupStatus.mockResolvedValue({ setupComplete: false });
+    window.history.pushState({}, '', '/setup');
+
+    const queryClient = makeQueryClient();
+    renderApp(queryClient);
+
+    await waitFor(() => {
+      expect(screen.getByTestId('setup-page')).toHaveTextContent('Welcome to FamilySync Setup');
+    });
+
+    expect(screen.queryByText('Setup already complete')).toBeNull();
+  });
 });
 
 describe('App — setupStatus and route presence', () => {

apps/pwa/src/App.tsx

@@ -81,11 +81,21 @@ export default function App() {
   // Fetch current user once at the app shell level so AppNav has member data on
   // ALL routes. This is the same query key (['me']) used by CalendarShell, so
   // TanStack Query deduplicates the request — no double fetch.
+  //
+  // gap 6 (mechanism (ii) — ['me'] staleness, NOT a linking gap; see SUMMARY):
+  // the first-login claim in upsertUser (auth/user.ts) preserves the same users.id,
+  // so the wizard-stored CalDAV credential stays linked → needsProviderSetup is
+  // correctly FALSE in the DB after the operator authenticates post-wizard. The bug
+  // was purely client-cache: a ['me'] entry populated BEFORE the claim (e.g. an
+  // earlier pre-auth visit) served a stale needsProviderSetup=true for up to 5
+  // minutes, so the "Set up your calendar" banner kept showing. Set staleTime 0 on
+  // the boot ['me'] query so the authenticated app shell always refetches member
+  // status on entry — needsProviderSetup then reflects the just-claimed credential.
   const meQuery = useQuery({
     queryKey: ['me'],
     queryFn: fetchMe,
     retry: false,
-    staleTime: 5 * 60 * 1000,
+    staleTime: 0,
   });
 
   // isAdmin from /api/me — used for UX gating only (D-03). Server enforces 403.
@@ -135,8 +145,26 @@ export default function App() {
   return (
     <BrowserRouter>
       <Routes>
-        {/* /setup route — standalone wizard, no AppNav/BottomTabBar shell (UI-SPEC §Routing) */}
-        <Route path="/setup" element={<SetupPage />} />
+        {/* /setup route — standalone wizard, no AppNav/BottomTabBar shell (UI-SPEC §Routing).
+            Reverse gate (gap 5, T-12-04): once setup is complete the wizard must NOT re-mount.
+            - While setupQuery is loading → render the no-flash placeholder (no wizard before
+              status resolves), mirroring the `*`-route loading gate below.
+            - setupComplete === true → render SetupPage with alreadyLocked → Surface 8
+              ("Setup already complete"), keeping the operator on /setup with a terminal surface.
+            - setupComplete === false (or undefined post-load) → active wizard, as before.
+            The backend already 423s setup mutations; this is the matching frontend reverse-gate. */}
+        <Route
+          path="/setup"
+          element={
+            setupLoading ? (
+              <div aria-hidden="true" />
+            ) : setupComplete === true ? (
+              <SetupPage alreadyLocked={true} />
+            ) : (
+              <SetupPage />
+            )
+          }
+        />
 
         {/* All other routes are gated on setup completion */}
         <Route

apps/pwa/src/components/SetupBanner.test.tsx

@@ -0,0 +1,130 @@
+/**
+ * SetupBanner.test.tsx — regression tests for the gap-6 ['me'] freshness fix
+ * (Phase 12 Plan 07 Task 2) plus the success-only dismissal contract.
+ *
+ * gap 6 mechanism (ii) — ['me'] staleness, NOT a backend linking gap:
+ *   The first-login claim in upsertUser (apps/api/src/auth/user.ts) preserves the
+ *   same users.id, so the wizard-stored CalDAV credential stays linked and the DB
+ *   correctly reports needsProviderSetup=false after the operator authenticates.
+ *   The bug was purely client-cache: a ['me'] entry populated BEFORE the claim
+ *   (e.g. a pre-auth visit) served a stale needsProviderSetup=true for up to the
+ *   5-minute staleTime, so the "Set up your calendar" banner kept showing.
+ *   Fix: the banner's ['me'] query uses staleTime 0, so a stale cache entry is
+ *   refetched on mount and the banner reflects the just-claimed credential.
+ *
+ * Tests cover:
+ *   - needsProviderSetup=false → banner absent
+ *   - needsProviderSetup=true  → banner present (no dismiss/X button)
+ *   - staleTime 0: a pre-seeded stale ['me'] (needsProviderSetup=true) is refetched
+ *     on mount; once fetchMe resolves needsProviderSetup=false, the banner hides
+ *     (mirrors the post-wizard claim where the DB now reports false)
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { render, screen, waitFor } from '@testing-library/react';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+
+// CredentialSheet is rendered (closed) by SetupBanner — mock it to avoid the
+// real sheet's DOM/focus-trap complexity. We're testing banner visibility only.
+vi.mock('./CredentialSheet.js', () => ({
+  CredentialSheet: () => null,
+}));
+
+vi.mock('../api/client.js', () => ({
+  fetchMe: vi.fn(),
+}));
+
+import { fetchMe } from '../api/client.js';
+import type { Mock } from 'vitest';
+import { SetupBanner } from './SetupBanner.js';
+
+const mockFetchMe = fetchMe as Mock;
+
+function makeQueryClient() {
+  return new QueryClient({
+    defaultOptions: {
+      queries: { retry: false },
+      mutations: { retry: false },
+    },
+  });
+}
+
+function renderBanner(queryClient: QueryClient) {
+  return render(
+    <QueryClientProvider client={queryClient}>
+      <SetupBanner />
+    </QueryClientProvider>,
+  );
+}
+
+function meUser(needsProviderSetup: boolean) {
+  return {
+    user: {
+      id: 1,
+      displayName: 'Test User',
+      color: '#4a90d9',
+      isAdmin: false,
+      needsProviderSetup,
+    },
+  };
+}
+
+describe('SetupBanner', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('does not render when needsProviderSetup is false', async () => {
+    mockFetchMe.mockResolvedValue(meUser(false));
+
+    const queryClient = makeQueryClient();
+    renderBanner(queryClient);
+
+    // Give the query a tick to resolve, then confirm the banner stays absent
+    await new Promise((r) => setTimeout(r, 50));
+    expect(screen.queryByText('Set up your calendar')).toBeNull();
+  });
+
+  it('renders the banner (no dismiss button) when needsProviderSetup is true', async () => {
+    mockFetchMe.mockResolvedValue(meUser(true));
+
+    const queryClient = makeQueryClient();
+    renderBanner(queryClient);
+
+    await waitFor(() => {
+      expect(screen.getByText('Set up your calendar')).toBeInTheDocument();
+    });
+
+    // The ONLY action is "Set up now" — there is NO dismiss/X button (T-05-24).
+    expect(screen.getByRole('button', { name: 'Set up now' })).toBeInTheDocument();
+    expect(screen.queryByRole('button', { name: /dismiss/i })).toBeNull();
+    expect(screen.queryByRole('button', { name: /close/i })).toBeNull();
+  });
+
+  // gap 6 regression: a stale ['me'] cache entry (needsProviderSetup=true, as it
+  // would be from a pre-claim visit) must be refetched on mount because the query
+  // uses staleTime 0. After the fresh fetch resolves needsProviderSetup=false (the
+  // post-wizard-claim DB truth), the banner must hide.
+  it('refetches a stale ["me"] on mount (staleTime 0) and hides the banner once needsProviderSetup is false', async () => {
+    const queryClient = makeQueryClient();
+
+    // Seed a STALE cache entry as if populated before the wizard claim.
+    queryClient.setQueryData(['me'], meUser(true));
+
+    // The server now reports the claimed credential → needsProviderSetup=false.
+    mockFetchMe.mockResolvedValue(meUser(false));
+
+    renderBanner(queryClient);
+
+    // staleTime 0 → the seeded entry is stale → refetch fires on mount.
+    await waitFor(() => {
+      expect(mockFetchMe).toHaveBeenCalled();
+    });
+
+    // After the refetch resolves false, the banner must disappear.
+    await waitFor(() => {
+      expect(screen.queryByText('Set up your calendar')).toBeNull();
+    });
+  });
+});

apps/pwa/src/components/SetupBanner.tsx

@@ -33,11 +33,18 @@ export function SetupBanner() {
   // Use HTMLButtonElement for the ref (assignable to the CredentialSheet's HTMLElement trigger)
   const ctaRef = useRef<HTMLButtonElement>(null);
 
+  // staleTime 0 (gap 6): this banner gates on needsProviderSetup, which is only
+  // correct if ['me'] is fresh on entry to the authenticated shell after the
+  // wizard claim (see App.tsx for the full mechanism note). With a 5-minute
+  // staleTime a pre-claim cache entry kept needsProviderSetup=true and the banner
+  // showed even though the operator already configured the calendar in the wizard.
+  // The success-only dismissal contract is unchanged: this banner still clears
+  // ONLY when needsProviderSetup becomes false (no dismiss/X button).
   const meQuery = useQuery({
     queryKey: ['me'],
     queryFn: fetchMe,
     retry: false,
-    staleTime: 5 * 60 * 1000,
+    staleTime: 0,
   });
 
   // Only show when needsProviderSetup is explicitly true