deckhearth/.convoys/fix-layout-default-user.md

---
name: fix-layout-default-user
classification: convoy
success_metric: |
  components/Layout.js's user prop defaults to null. Every page in pages/**
  that renders Layout either passes a user prop explicitly OR relies on the
  new logged-out rendering branch. No real email address remains in any
  component default-prop. Manual smoke: load an unauthenticated session on
  a page that previously impersonated the maintainer; confirm the navbar /
  profile dropdown reads "Sign in" rather than "me@randallstillwell.com".
skip:
  - role-design-system-auditor   # no design-token changes
  - role-ia-architect            # no URL / IA changes
  - browser-smoke                # local smoke is fine for this scope
status: open
created: 2026-05-23
parent: ship-readiness
addresses: P0 #7
depends_on:
  - bump-next-js (shipped)
  - fix-auth-bypass (shipped)
  - drop-public-setup (shipped)
---

# Fix Layout default user

Close P0 #7 from `.convoys/ship-readiness.md` (the **last** remaining P0
ship-blocker). `components/Layout.js` line 562 defaults the `user` prop to
`{ email: 'me@randallstillwell.com', role: 'user' }` — any page that renders
`Layout` without passing `user` explicitly displays the maintainer's real
email and impersonates them as the logged-in user.

## Scope (verbatim from ship-readiness P0 #7)

- **`components/Layout.js`** — change the `Layout({ children, user = {...} })`
  default to `user = null`. Add a logged-out rendering branch (navbar /
  profile dropdown / mobile menu) that handles `user === null` cleanly —
  typically "Sign in" CTA replacing the user avatar + dropdown.
- **17 pages in `pages/**`** that import Layout (confirmed via grep):
  ```
  pages/scanner.js
  pages/collection/[identifier].js
  pages/card/[id].js
  pages/my-cards.js
  pages/cards.js
  pages/deck-builder.js
  pages/deck/[id].js
  pages/decks.js
  pages/dashboard.js
  pages/community/collections.js
  pages/collections.js
  pages/settings.js
  pages/profile.js
  pages/invite/decline.js
  pages/invite/accept.js
  pages/admin/card-import.js
  pages/admin/card-editor.js
  ```
  For each: confirm it passes `user` explicitly OR triage that it should
  use the new logged-out branch (e.g. public pages like card/[id].js,
  community/collections.js may legitimately render Layout for anonymous
  visitors).

## Out of scope

- **Branding** — Layout still renders "Deck Hearth" / "DH" while the rest of
  the repo says "TCG Vault". That mismatch is the queued `pick-a-name`
  convoy (P1 #12). Don't fix branding here.
- **Three parallel auth providers** — Layout reads from one of
  `lib/auth-context.js` / `lib/admin-auth.js` / `lib/use-auth.js`. Collapsing
  them is the queued `single-auth-provider` convoy (P1 #9).
- **God-component split** — `components/Layout.js` is 700+ lines. Splitting
  is the queued `god-component-split` convoy (P2 #13). Touch only the user-
  prop default and the logged-out rendering branch in THIS convoy.
- **`MobileNavigation`** — receives `user` from Layout. May need a similar
  default-prop fix if it has the same anti-pattern. Audit during architect
  pass and decide whether to fold in or queue separately.
- **`AGENTS.md` Gotcha #8** — will be marked RESOLVED in the post-convoy
  doc-writer pass; do not pre-emptively edit AGENTS.md here.

## Architect's questions

1. **Logged-out rendering branch design.** When `user === null`, what should
   Layout render?
   - **Q1a:** Navbar / profile dropdown — replace the user avatar + email
     with a "Sign in" link to `/login`?
   - **Q1b:** Mobile bottom-nav — same treatment, or hide the user-only
     items entirely?
   - **Q1c:** Authenticated-only nav items (admin, settings, profile) —
     hide them, or show but link to `/login`?

2. **Page audit triage.** For each of the 17 pages, three buckets:
   - **Always-authenticated** (dashboard, my-cards, profile, settings,
     scanner, admin/*) — must pass `user` explicitly; pages without it
     should add it via `useAuth()`.
   - **Public-or-authenticated** (cards, card/[id], collection/[identifier],
     community/collections, deck/[id], collections, decks) — currently
     show different views based on auth; the Layout user prop should
     come from `useAuth()` either way.
   - **Anonymous-allowed** (invite/decline, invite/accept) — may
     legitimately render Layout without a user; rely on the new
     logged-out branch.

   The architect should produce the exact bucket assignment per page and
   the brief should give the implementer the per-page instruction.

3. **Brief decomposition.** Three options:
   - **Single brief, one PR.** All 18 files (Layout + 17 pages) in one diff.
     Reviewable but big.
   - **Two briefs, one PR.** Brief 1: Layout change + logged-out rendering.
     Brief 2: page audit (depends on Brief 1). Both ship together.
   - **Fan-out by page bucket.** Brief 1: Layout change. Brief 2: always-auth
     pages. Brief 3: public-or-auth pages. Brief 4: anonymous-allowed pages.
     Multitask-friendly via worktrees.

   **Recommend two briefs in one PR** for size + reviewability balance,
   unless the page audit reveals >10 files needing real changes (in which
   case fan-out makes sense).

4. **Test coverage.** Should this convoy add vitest tests that exercise
   Layout's logged-out branch? The fix-auth-bypass convoy added 16 auth
   tests (`test/lib/permission-middleware.test.js`); a similar lock-in for
   the user-prop default could prevent regression.

   **Recommend yes** — a single test that asserts `Layout` renders the
   logged-out shape when `user === undefined` and `user === null` would
   catch any future regression that reintroduces the maintainer-email
   default. Trivial to write; high value.

## Expected size

1-2 briefs, ~18 files total (1 component, 17 pages). Single PR likely.

## Architecture

### File plan

| File | Action | Purpose |
| --- | --- | --- |
| `components/Layout.js` | modified | Default `user` to `null`. Replace `UserProfileDropdown`'s avatar+email+dropdown with a "Sign in" CTA (inline) when `user === null`. The other null-safe paths (`NavigationContent`'s `authenticatedNavigation`, `myCollectionNavigation`, `adminNavigation`) already do the right thing today; do not touch them. |
| `vitest.config.js` | modified | No global change to `environment`; the new test file uses a per-file `// @vitest-environment jsdom` directive. The only edit here is bumping `include: ['test/**/*.test.js']` if the implementer wants test files under `test/components/` (already covered by the existing glob — verify only). |
| `package.json` | modified | Add `jsdom` and `@testing-library/react` to `devDependencies` so the new component test can render the JSX tree. No runtime deps. |
| `package-lock.json` | modified | Regenerated by `npm install`. |
| `test/components/Layout.test.js` | new | Locks in the contract: `<Layout>` (no prop) and `<Layout user={null}>` MUST NOT render `me@randallstillwell.com`; MUST render a "Sign in" affordance; `<Layout user={…}>` MUST render the supplied email. Negative regression against the maintainer-email default. |
| `pages/scanner.js` | modified | Pass `user={user}` to `<Layout>` (line 333). `useAuth` from `lib/auth-context` is already imported. |
| `pages/deck-builder.js` | modified | Pass `user={user}` to all four `<Layout>` calls. `useAuth` from `lib/auth-context` already imported. |
| `pages/deck/[id].js` | modified | Pass `user={user}` to all three `<Layout>` calls. `useAuth` from `lib/auth-context` already imported. |
| `pages/decks.js` | modified | Pass `user={user}` to all three `<Layout>` calls. `useAuth` from `lib/auth-context` already imported. |
| `pages/profile.js` | modified | Page-level `useState` initializer at line 10 hardcodes `email: 'me@randallstillwell.com', role: 'admin'`. Replace the initial value with `null`. Wrap `user.*` reads in `?.` (the page already runs after `loadUserProfile()` resolves; the few sync reads need null-guards). Add `useAuth()` from `lib/use-auth.js` only if the implementer prefers a single source — but the simpler fix is `useState(null)` since the API call already overwrites the state. |
| `pages/settings.js` | modified | Same shape as `profile.js`: replace `useState({ email: 'me@…', role: 'admin' })` with `useState(null)`. The component already redirects to `/login` if no token (line 53-55), so the null window is the loading flash. |
| `pages/card/[id].js` | modified | Replace the hardcoded `const user = { email: 'me@…', role: 'user' }` (lines 13-16) with `const { user } = useAuth()` from `lib/use-auth.js`. This is a public-or-authenticated page (community card detail), so `user === null` is a legitimate state. |

The 10 remaining pages (`pages/dashboard.js`, `pages/my-cards.js`, `pages/cards.js`, `pages/collection/[identifier].js`, `pages/community/collections.js`, `pages/collections.js`, `pages/invite/accept.js`, `pages/invite/decline.js`, `pages/admin/card-import.js`, `pages/admin/card-editor.js`) already pass `user` correctly; **no changes** there. They are listed in the per-bucket assignment below for the audit record.

### API surface

None. This is a frontend component change.

### Schema diff

None.

### Test plan

- **Unit (new):** `test/components/Layout.test.js` (Decision D = D2). Per-file `// @vitest-environment jsdom`. Mocks `next/router` (`useRouter` → `{ pathname: '/' }`) and `lib/theme-context` (`useTheme` → `{ theme: 'light', toggleTheme: vi.fn() }`). Asserts:
  1. `<Layout>page</Layout>` (no `user` prop) renders a tree whose `textContent` does **not** include `me@randallstillwell.com`. (Direct regression on the bug shape.)
  2. `<Layout user={null}>page</Layout>` — same.
  3. `<Layout user={null}>page</Layout>` renders a "Sign in" affordance (`getByText('Sign in')`).
  4. `<Layout user={{ email: 'foo@bar.com', role: 'user' }}>page</Layout>` renders `foo@bar.com` in the navbar.
  5. (Negative regression — explicit) `<Layout>page</Layout>`'s `textContent` does **not** include `Guest` (so we don't accidentally ship a "Guest" placeholder where Decision A says "Sign in" should live).
- **No new tests** for the page audit changes. The Layout test catches the fix at the component boundary; verifying every page individually would duplicate that contract. Manual smoke covers the page-level fixes.
- **Existing tests** (16 in `test/lib/permission-middleware.test.js`, 3 in `test/lib/auth-secret.test.js`, 5 in `test/api/auth-utils.test.js`) must remain green. None of them touch UI; this convoy will not regress them.
- **Manual smoke** (in PR description):
  - With no `auth_token` in `localStorage` (logged out), visit `/scanner`, `/deck-builder`, `/decks`, `/deck/[any-id]`, `/profile`, `/settings`, `/card/[any-id]`. Confirm: navbar shows "Sign in" instead of `me@randallstillwell.com`, no "Profile / Settings / Logout" dropdown, public nav items still visible.
  - With `auth_token` set (logged in as a non-admin), visit each of the same pages. Confirm: navbar shows the real user's email, dropdown opens, "Profile / Settings / Logout" links visible, "Admin Panel" hidden.
  - Log in as admin. Confirm: "Admin Panel" link visible.
  - Visit `/invite/accept?token=anything` and `/invite/decline?token=anything` while logged out. Confirm: layout renders "Sign in", no maintainer email, no crashes (these pages explicitly pass `user={null}` and rely on the new branch).

### Risks

- **R1 — Loading-flash UX regression (low).** `useAuth()` returns `loading: true` with `user === null` until `/api/auth/verify` resolves. During that flash (< 200ms in dev, typically faster in prod), Layout shows "Sign in" before swapping to the authenticated shape. **Mitigation:** This is the same flash that `ProtectedRoute` and `AdminProtected` already produce; their loading branches render `<Layout user={null}>` today (see `components/ProtectedRoute.js` line 29, `components/AdminProtected.js` line 55). The fix keeps the existing UX contract; document in Brief 1 so the implementer doesn't try to "improve" it with a loading skeleton (out of scope).
- **R2 — Page-level useState initializer regressions (medium).** `pages/profile.js`, `pages/settings.js`, and `pages/card/[id].js` initialize a hardcoded user object. Replacing the initializer with `null` means any sync code that reads `user.first_name`, `user.email`, `user.role`, etc. before the API resolves now hits a null-deref. **Mitigation:** Brief 2 enumerates the sync reads per page and adds `?.` / `?? defaults`. Vitest can't catch this at render time without a full mock harness; manual smoke is the gate. The implementer MUST exercise both the loading state and the loaded state on each of those three pages.
- **R3 — `user.role === 'admin'` defaults flip (low).** `profile.js` and `settings.js` initialize `role: 'admin'`. Today, a non-admin user briefly sees admin chrome (Admin Panel link) during the load flash. After the fix, that flash shows "Sign in" (R1) until the API resolves, then the correct role takes over. The "ghost admin" was a worse UX bug already; the fix improves it. No mitigation needed.
- **R4 — `UserProfileDropdown` Sign-in branch breaks the desktop+mobile-shared component (low).** The dropdown is rendered twice (desktop sidebar + mobile drawer). Both must show "Sign in" when logged out. **Mitigation:** Brief 1 makes the change inside `UserProfileDropdown` (a single function) — both call sites pick up the new behavior automatically.
- **R5 — `vitest` test fragility (low).** The test asserts on rendered text. If a future change moves "Sign in" to an icon-only `aria-label`, `getByText` will fail. **Mitigation:** Test against `getByRole('link', { name: /sign in/i })` so the assertion survives icon-only refactors. Brief 1 specifies this query shape.
- **R6 — `jsdom` + `@testing-library/react` adds devDeps (low).** Two new packages (~25 transitive deps; devDep only). **Mitigation:** Brief 1 declares the additions explicitly in `files:` and acceptance criteria; CI's existing `test:` job will pick up the new test automatically (the glob already includes `test/**/*.test.js`).
- **R7 — Three parallel auth providers (existing, not introduced).** 4 of the 7 pages we touch import `useAuth` from `lib/auth-context.js` (the legacy provider) instead of the canonical `lib/use-auth.js`. Brief 2 explicitly says "do not change the auth import — keep the existing `useAuth` source." Migrating these to `lib/use-auth.js` is the queued `single-auth-provider` convoy's job. **Mitigation:** Brief 2 acceptance criteria includes a grep verification that `pages/scanner.js`, `pages/deck-builder.js`, `pages/deck/[id].js`, `pages/decks.js` still import from `lib/auth-context`.
- **R8 — `MobileNavigation` receives `user` but ignores it (cosmetic, deferred).** `components/MobileNavigation.js` accepts `{ user, onMenuOpen }` but does not access any field of `user` (the bottom-bar items are static). The prop is dead. **Decision:** leave it alone in this convoy; flag as a tiny cleanup follow-up. If the implementer is tempted to delete the prop, they MUST stop — that's god-component-split / single-auth-provider territory.

### Decomposition

| Brief # | Title | Files | Depends on | Estimated PR size |
| --- | --- | --- | --- | --- |
| 1 | Layout: default user to null + logged-out branch + vitest lock-in | `components/Layout.js`, `test/components/Layout.test.js` (new), `package.json`, `package-lock.json` | none | ~80 LOC source change in Layout; ~60 LOC new test; package.json adds 2 devDeps |
| 2 | Pages: pass user explicitly + drop page-level maintainer-email defaults | `pages/scanner.js`, `pages/deck-builder.js`, `pages/deck/[id].js`, `pages/decks.js`, `pages/profile.js`, `pages/settings.js`, `pages/card/[id].js` | 1 | ~30-40 LOC across 7 files |

Brief 2 depends on Brief 1 because the page changes assume the new logged-out branch exists. Both ship in the same PR. (Decision C = C2.)

### Slice dependencies (multitask-ready)

```yaml
slice_dependencies:
  - brief: 1
    depends_on: []
    files:
      - components/Layout.js
      - test/components/Layout.test.js
      - package.json
      - package-lock.json
  - brief: 2
    depends_on: [1]
    files:
      - pages/scanner.js
      - pages/deck-builder.js
      - pages/deck/[id].js
      - pages/decks.js
      - pages/profile.js
      - pages/settings.js
      - pages/card/[id].js
```

Briefs 1 and 2 do not share any files. Brief 2 depends on Brief 1 only because its acceptance criteria reference the new logged-out branch. The Conductor MUST sequence them (no `/multitask` parallel fan-out for this convoy).

## Decisions

### Decision A — Logged-out rendering branch design (was Q1)

**A1 — replace user avatar + email + dropdown with a "Sign in" link to `/login`. Hide auth-only nav (already done by existing optional-chains). Public nav items still visible.**

`NavigationContent` is already null-safe: `authenticatedNavigation` (line 140-142), `myCollectionNavigation` (line 145-157), and `adminNavigation` (line 167) all gate on `user`. The only piece that leaks the maintainer email is `UserProfileDropdown`'s avatar (line 100-104), email/role labels (line 107-110), and dropdown menu items (line 11-18 — Profile/Settings/Admin/Logout linking to authenticated routes). Brief 1 short-circuits `UserProfileDropdown` with a `<Link href="/login">` containing a "Sign in" label and a sign-in icon when `user === null`; the rest of the layout continues to work.

A2 (elaborate logged-out CTA with marketing copy) is out of scope per the convoy file ("Branding" → `pick-a-name`; "landing-page-rework" not yet queued). A3 (hide layout chrome entirely) breaks anonymous viewing on `pages/invite/{accept,decline}.js`, `pages/cards.js` (`PublicCardsView`), and any other public surface that legitimately renders Layout for non-authenticated users.

### Decision B — Page audit triage (was Q2)

Per-page bucket assignment (17 pages):

| # | Page | Bucket | `useAuth` source | Already passes `user`? | Action |
| --- | --- | --- | --- | --- | --- |
| 1 | `pages/scanner.js` | 1 (Always-auth) | `lib/auth-context` | No (`<Layout>` line 333) | **Fix** — pass `user={user}` |
| 2 | `pages/dashboard.js` | 1 | `lib/use-auth` | Yes | None |
| 3 | `pages/my-cards.js` | 1 | `lib/use-auth` | Yes | None |
| 4 | `pages/profile.js` | 1 | none (uses local `useState` with maintainer-email init) | Yes (passes the leaky useState user) | **Fix** — replace `useState({email:'me@…'})` with `useState(null)` |
| 5 | `pages/settings.js` | 1 | none (same pattern as profile) | Yes (leaky) | **Fix** — same as profile |
| 6 | `pages/deck-builder.js` | 1 | `lib/auth-context` | No (`<Layout>` ×4) | **Fix** — pass `user={user}` to all four |
| 7 | `pages/decks.js` | 1 | `lib/auth-context` | No (`<Layout>` ×3) | **Fix** — pass `user={user}` to all three |
| 8 | `pages/admin/card-import.js` | 1 (admin) | none (gets `user` from `AdminProtected` render-prop) | Yes | None |
| 9 | `pages/admin/card-editor.js` | 1 (admin) | none (same pattern as card-import) | Yes | None |
| 10 | `pages/collections.js` | 1 | `lib/use-auth` | Yes | None |
| 11 | `pages/cards.js` | 2 (Public-or-auth) | `lib/use-auth` | Yes (`PublicCardsView` passes `user={null}`; `AuthenticatedCards` passes `user={user}`) | None |
| 12 | `pages/card/[id].js` | 2 | none (hardcoded `const user = { email: 'me@…' }`) | Yes (passes the hardcoded const) | **Fix** — `const { user } = useAuth()` from `lib/use-auth` |
| 13 | `pages/collection/[identifier].js` | 2 | `lib/use-auth` | Yes | None |
| 14 | `pages/community/collections.js` | 2 | `lib/use-auth` | Yes | None |
| 15 | `pages/deck/[id].js` | 2 | `lib/auth-context` | No (`<Layout>` ×3) | **Fix** — pass `user={user}` to all three |
| 16 | `pages/invite/accept.js` | 3 (Anonymous-allowed) | none | Yes (`user={null}`) | None |
| 17 | `pages/invite/decline.js` | 3 | none | Yes (`user={null}`) | None |

**Net:** 7 pages need code changes (Brief 2). 10 pages are already correct.

A page-audit grep also surfaced **3 page-level maintainer-email leaks** that the convoy file did not enumerate explicitly: `pages/profile.js`, `pages/settings.js`, and `pages/card/[id].js` initialize their `user` state/const with `email: 'me@randallstillwell.com'`. These are the same bug shape as the Layout default — the convoy's success metric ("No real email address remains in any component default-prop") is satisfied only if these are fixed too. Brief 2 covers them.

### Decision C — Brief decomposition (was Q3)

**C2 — two briefs, one PR.**

Brief 1 ships the Layout change + vitest lock-in (the actual fix). Brief 2 ships the page audit (the cleanup that proves the fix is complete). Brief 2 depends on Brief 1 because its acceptance criteria reference the new logged-out branch.

C1 (single brief, 12 files) makes the diff harder to review — the Layout change is the conceptually interesting piece; lumping it with 7 mechanical page edits buries it. C3 (fan-out by bucket) creates 4 briefs without parallelization benefit, since Brief 2's pages don't share files with one another but DO all depend on Brief 1, so /multitask gives 4× the orchestration cost for the same wall-clock time.

### Decision D — vitest coverage (was Q4)

**D2 — yes, add `test/components/Layout.test.js` with the four assertions enumerated in the test plan.**

The bug was a default-prop value that nobody caught for ~12 months. A single test that asserts "the rendered tree does not contain `me@randallstillwell.com`" makes the regression impossible to reintroduce silently. Cost: two new devDeps (`jsdom`, `@testing-library/react`), one ~60-LOC test file, per-file `// @vitest-environment jsdom` directive (no global vitest config change). Pattern matches `test/lib/permission-middleware.test.js`'s negative-regression-against-old-shape style (the test for the synthetic-admin shape locks in P0 #1's fix).

D1 (no tests, manual smoke only) is what the original bug had. Manual smoke is human-attention-bottlenecked and not a regression gate.

### Anything flagged but not acted on

- **`MobileNavigation`'s unused `user` prop.** The component accepts `{ user, onMenuOpen }` but never reads `user.*` (the bottom-bar items are static — Cards, Decks, Dashboard, Community, More — none gated on auth state or role). The prop is dead. Removing it is a 2-line cleanup, but the convoy spec lists `MobileNavigation` as out-of-scope-or-fold-in territory and the right call here is to defer: a dead prop is harmless, and removing it touches `Layout.js` (the call site) plus `MobileNavigation.js`, expanding the diff. **Follow-up convoy:** `cleanup-mobile-nav-dead-props` (P3 polish; can fold into `god-component-split` if that lands first).
- **Three parallel auth providers.** Brief 2 leaves `pages/scanner.js`, `pages/deck-builder.js`, `pages/deck/[id].js`, `pages/decks.js` importing `useAuth` from `lib/auth-context` (the legacy provider) instead of `lib/use-auth`. The convoy file explicitly defers this to `single-auth-provider`. **Follow-up convoy:** `single-auth-provider` (P1 #9, already queued in `.convoys/ship-readiness.md`).
- **Default branding in Layout headers.** Lines 593-596 (mobile drawer header) and 686-689 (desktop sidebar header) render "DH" + "Deck Hearth". The user-prop fix does not touch branding. **Follow-up convoy:** `pick-a-name` (P1 #12, already queued).
- **`pages/profile.js` and `pages/settings.js` `loading` state design.** Both pages render `<Layout user={user}>` while `loading === true`. After Brief 2, this means `<Layout user={null}>` during loading → "Sign in" briefly visible to a logged-in user reloading the page. This matches `ProtectedRoute`'s existing loading UX and is acceptable. If product wants to suppress the flash, that's a separate UX convoy (`auth-loading-skeleton`).
- **`AGENTS.md` Gotcha #8.** The convoy file says it will be marked RESOLVED in the post-convoy doc-writer pass. Do not edit `AGENTS.md` here.
- **`.cursor/rules/ui-and-theming.mdc` § Component conventions.** The rule already documents the intent: "Avoid hardcoded default values for `user` props … default to `null` and render a logged-out state." No rule update needed.