deckhearth/.convoys/fix-layout-default-user/brief-1-layout-default-null-and-test.md

---
convoy: fix-layout-default-user
brief_number: 1
depends_on: []
files:
  - components/Layout.js
  - test/components/Layout.test.js
  - package.json
  - package-lock.json
cross_brief_commitments:
  - brief: 2
    description: |
      Brief 2 changes the seven pages that today either omit the `user` prop or
      pass a hardcoded maintainer-email object. Brief 2 assumes Brief 1's new
      logged-out branch (default `user = null`, "Sign in" CTA in
      `UserProfileDropdown`) is in place — without it, those pages would render
      "Sign in" before their own auth state resolved, but Layout's default
      would silently rewrite that back to the maintainer email. Ship Brief 1
      first in the diff.
---

# Brief 1: Default Layout's `user` to null + render a logged-out branch + lock the contract with a vitest test

## Goal (1 sentence)

Change `components/Layout.js`'s `user` default from `{ email: 'me@randallstillwell.com', role: 'user' }` to `null`, replace `UserProfileDropdown`'s avatar+email+menu with a "Sign in" link to `/login` when `user === null`, and add a vitest test under `test/components/Layout.test.js` that locks in the contract by asserting the rendered tree never contains `me@randallstillwell.com` for the no-user / null-user branches.

## Files in scope (do not edit anything else)

- `components/Layout.js` — modified (default-prop fix + logged-out `UserProfileDropdown` branch).
- `test/components/Layout.test.js` — new (regression test).
- `package.json` — modified (add `jsdom` and `@testing-library/react` to `devDependencies`).
- `package-lock.json` — regenerated by `npm install`.

## Conventions to follow

- **`.cursor/rules/ui-and-theming.mdc` § Component conventions.** Already documents the intent: "Avoid hardcoded default values for `user` props. … New components must default to `null` and render a logged-out state." This brief is the first concrete application of that rule.
- **`.cursor/rules/auth-and-permissions.mdc` § Authentication state on the client.** `useAuth()` returns `{ user, loading, … }` where `user === null` means logged out. Layout's null-user rendering must be safe for that case; do **not** add new logic that throws on `user === null`.
- **`.cursor/rules/no-go-zones.mdc`.** Do not edit `components/Layout.js.backup`. Do not edit anything under `lib/**` or `.github/**`. Do not touch `.cursor/rules/**`.
- **Brief size discipline.** This brief is one component change, one new test, two devDeps. **Do NOT**:
  - Split the Layout god-component (`god-component-split` convoy owns that).
  - Migrate Layout off `lib/auth-context.js` / `lib/admin-auth.js` (Layout doesn't import either today; both legacy providers are queued for `single-auth-provider`).
  - Rename "Deck Hearth" or "DH" to "TCG Vault" (`pick-a-name` convoy).
  - Remove the dead `user` prop on `MobileNavigation` (deferred follow-up, see convoy file § "Anything flagged but not acted on").
  - Touch `components/MobileNavigation.js` at all.
- **Vitest test patterns.** Match `test/lib/permission-middleware.test.js`'s shape: `describe` block per behavior cluster, `vi.mock(...)` for module dependencies, plain `expect()` matchers (no jest-dom required). The new test uses a per-file `// @vitest-environment jsdom` directive at the top (vitest v3 supports this) so `vitest.config.js`'s global `environment: 'node'` does not need to change.

## Acceptance criteria

### `components/Layout.js`

- [ ] **Change the default-prop on line 562.** Before:

```js
export default function Layout({ children, user = { email: 'me@randallstillwell.com', role: 'user' }, showSearch = false }) {
```

  After:

```js
export default function Layout({ children, user = null, showSearch = false }) {
```

  No other change to that line.

- [ ] **Add a logged-out branch to `UserProfileDropdown`.** The branch MUST be placed **after** the existing `useState(false)` call (rules of hooks: hooks must be called in the same order every render — moving the early return above `useState` would throw "Rendered more hooks than during the previous render" the moment `user` flips from `null` to an object on a subsequent render). Verbatim shape — the implementer MAY adjust class names to match neighboring sidebar items, but every prop / behavior must be present:

```js
function UserProfileDropdown({ user, onMobileMenuClose }) {
  // Hook order is fixed for both branches; do not move this below the
  // null-user early return — see rules-of-hooks (AGENTS.md Gotcha #11.5).
  const [isDropdownOpen, setIsDropdownOpen] = useState(false);

  // Logged-out: replace avatar + email + dropdown with a Sign-in CTA.
  if (!user) {
    return (
      <Link href="/login">
        <div
          className="w-full flex items-center px-4 py-3 rounded-2xl transition-all duration-200 focus-within:outline-none focus-within:ring-2 focus-within:ring-offset-2 nav-item-hover cursor-pointer"
          style={{
            backgroundColor: 'transparent',
            color: 'var(--text-primary)',
            '--tw-ring-color': 'var(--accent-ember)',
            '--tw-ring-offset-color': 'var(--bg-secondary)'
          }}
          onClick={onMobileMenuClose}
        >
          <div className="h-8 w-8 logo-container mr-3 flex items-center justify-center">
            <svg className="h-4 w-4 text-white" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true">
              <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M11 16l-4-4m0 0l4-4m-4 4h14m-5 4v1a3 3 0 01-3 3H6a3 3 0 01-3-3V7a3 3 0 013-3h4a3 3 0 013 3v1" />
            </svg>
          </div>
          <span className="font-medium text-sm">Sign in</span>
        </div>
      </Link>
    );
  }

  // Existing dropdown body (profileMenuItems, getProfileIcon, JSX) —
  // unchanged below this line.
}
```

  - The `useState` stays where it is today (currently line 9); the early return is inserted **between** `useState` and the existing `profileMenuItems` declaration. `setIsDropdownOpen` is unused on the null branch; that's fine — React does not warn on unused state setters, and the variable is declared because the hook MUST run.
  - The `<Link href="/login">` is non-negotiable — the test in `test/components/Layout.test.js` queries `getByRole('link', { name: /sign in/i })` and asserts every match has `href="/login"`.
  - `onClick={onMobileMenuClose}` keeps the mobile drawer behavior consistent with the existing items (the dropdown's existing items also call this on click — see line 73-74 of the current file).
  - The SVG is a "log-in" / "arrow-into-box" glyph (mirror of the existing logout SVG at line 39-41). Implementer MAY substitute another inline SVG so long as it is wrapped in `aria-hidden="true"` and the visible label is exactly "Sign in" (the test does a case-insensitive `/sign in/i` match — "Sign In" / "Sign in" / "SIGN IN" all work, but "Login" / "Log in" would fail the assertion).
  - **Lint check.** Run `npm run lint -- components/Layout.js` after the change — confirm zero new `react-hooks/rules-of-hooks` violations. Pre-existing lint baseline issues (per `AGENTS.md` Gotcha #11.5) may be present elsewhere in the file but `react-hooks/rules-of-hooks` should not regress in `UserProfileDropdown`.
- [ ] **Do not modify** `NavigationContent` (lines 127-560). Its `authenticatedNavigation`, `myCollectionNavigation`, `adminNavigation` already gate on `user` correctly; the public + community sections render unconditionally and are correct for both logged-out and logged-in states.
- [ ] **Do not modify** the Layout body (lines 568-793) — the desktop sidebar, mobile drawer, mobile overlay, and main-content wrapper are all unchanged. They pass `user` (now potentially `null`) to `MobileNavigation`, `NavigationContent`, and `UserProfileDropdown`; each of those handles `null` correctly after this brief.
- [ ] **Do not touch** the `'me@randallstillwell.com'` literal anywhere except the line 562 default — there are no other references in `components/Layout.js` (verified by `rg "me@randallstillwell" components/Layout.js` returning a single hit before this brief).
- [ ] **Branding.** Lines 593-596 ("DH" / "Deck Hearth" mobile drawer header) and 686-689 (desktop sidebar header) stay unchanged. `pick-a-name` owns branding.

### `test/components/Layout.test.js` (new)

- [ ] **Create the directory** `test/components/` if it does not exist (it does not as of this brief). The vitest glob `include: ['test/**/*.test.js']` (see `vitest.config.js` line 8) automatically picks the new file up.
- [ ] **Set the per-file environment** with `// @vitest-environment jsdom` as the first line. Do NOT modify `vitest.config.js`'s global `environment: 'node'` — other tests (auth, permission-middleware) run in node and changing the default would force every test through jsdom unnecessarily.
- [ ] **Verbatim shape** — the implementer MAY tighten queries, but every assertion must be present and the file must run green:

```js
// @vitest-environment jsdom
import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, cleanup, screen } from '@testing-library/react';

// Mock next/router so useRouter() does not crash without a RouterContext.
// Layout reads `router.pathname` only; the rest of the surface (`prefetch`,
// `events`, `push`) is for next/link's internals — provide stubs so prefetch
// does not throw when <Link> mounts.
vi.mock('next/router', () => ({
  useRouter: () => ({
    pathname: '/',
    asPath: '/',
    query: {},
    push: vi.fn(),
    replace: vi.fn(),
    prefetch: vi.fn().mockResolvedValue(undefined),
    events: { on: vi.fn(), off: vi.fn(), emit: vi.fn() },
  }),
}));

// Mock next/link to a plain <a>. The real next/link triggers prefetch on
// mount via the router; bypassing it removes a class of jsdom flake without
// changing the rendered DOM that the assertions inspect.
vi.mock('next/link', () => ({
  __esModule: true,
  default: ({ href, children, ...rest }) => {
    // children may be a single element (e.g. a <div>) or text; wrap in <a>.
    return (
      <a href={typeof href === 'string' ? href : ''} {...rest}>
        {children}
      </a>
    );
  },
}));

// Mock the theme context so useTheme() does not require a ThemeProvider.
vi.mock('../../lib/theme-context', () => ({
  useTheme: () => ({ theme: 'light', toggleTheme: vi.fn() }),
}));

import Layout from '../../components/Layout';

describe('Layout — logged-out rendering (regression: P0 #7)', () => {
  afterEach(() => cleanup());

  it('does NOT render the maintainer email when no user prop is passed', () => {
    const { container } = render(<Layout>page body</Layout>);
    expect(container.textContent).not.toContain('me@randallstillwell.com');
  });

  it('does NOT render the maintainer email when user is null', () => {
    const { container } = render(<Layout user={null}>page body</Layout>);
    expect(container.textContent).not.toContain('me@randallstillwell.com');
  });

  it('renders a Sign-in link to /login when user is null', () => {
    render(<Layout user={null}>page body</Layout>);
    const links = screen.getAllByRole('link', { name: /sign in/i });
    expect(links.length).toBeGreaterThanOrEqual(1);
    // Both desktop sidebar + mobile drawer render UserProfileDropdown,
    // so we expect TWO Sign-in links (one per copy).
    for (const link of links) {
      expect(link.getAttribute('href')).toBe('/login');
    }
  });

  it('renders the supplied user email when user is an object', () => {
    const { container } = render(
      <Layout user={{ email: 'foo@bar.com', role: 'user' }}>page body</Layout>
    );
    expect(container.textContent).toContain('foo@bar.com');
    expect(container.textContent).not.toContain('me@randallstillwell.com');
  });

  it('does NOT render a "Guest" placeholder when logged out', () => {
    // Decision A says the logged-out copy is "Sign in", not "Guest".
    // This test prevents a future revert that ships "Guest" as the default
    // (which would still hide the maintainer email but skip the CTA).
    const { container } = render(<Layout user={null}>page body</Layout>);
    expect(container.textContent).not.toContain('Guest');
  });
});
```

- [ ] **All five tests pass** under `npm run test:run`.
- [ ] **The pre-existing 24 tests** (16 in `permission-middleware.test.js`, 3 in `auth-secret.test.js`, 5 in `auth-utils.test.js`) **remain green**. This brief does not touch any file they cover, so the only failure mode is a CI environment regression — investigate and fix before merging.

### `package.json` + `package-lock.json`

- [ ] **Add to `devDependencies`** — single command: `npm install --save-dev jsdom @testing-library/react`. Resolved versions at architect time (2026-05-23): `jsdom@29.1.1`, `@testing-library/react@16.3.2`. Caret ranges are fine (matches the existing devDep style — `vitest@^3.2.4`).
- [ ] **Verify `@testing-library/dom` is present.** `@testing-library/react@16` declares `@testing-library/dom@^10.0.0` as a **peer dependency** (verified by `npm view @testing-library/react peerDependencies`). npm 7+ auto-installs peers, so a single `npm install --save-dev jsdom @testing-library/react` should resolve it transitively. After running install, run `npm ls @testing-library/dom` and confirm a single `^10.x` entry appears. If it does NOT (npm version too old or peer-install was disabled), explicitly add it: `npm install --save-dev @testing-library/dom@^10.0.0`.
- [ ] **Do NOT add `@testing-library/jest-dom`.** The test uses plain `expect().toContain()` / `toBeGreaterThanOrEqual()` matchers — no jest-dom matchers needed. Adding it would expand the dep surface for no acceptance-criteria benefit.
- [ ] **Do NOT add `@types/react`.** The repo is JavaScript-only (`AGENTS.md` Gotcha #9); the React 18/19 type peer on `@testing-library/react` is irrelevant when there is no `tsconfig.json`. npm will warn-and-skip the type peers, which is the documented behavior — leave the warning alone.
- [ ] **Do NOT bump any other dep** while you have `package.json` open. No `next` bump, no `vitest` bump, no react bump. If npm hoists a transitive minor that flips a lockfile entry, that's fine; if it tries to bump a top-level dep, stop and ask.
- [ ] **Verify** `package-lock.json` was regenerated and committed (the `npm install` run produces the lockfile diff; commit it as part of the same change).

### `vitest.config.js`

- [ ] **No change.** The new test file uses `// @vitest-environment jsdom` per-file. The global `environment: 'node'` (line 5) stays so the existing 24 tests do not slow down by going through jsdom unnecessarily.
- [ ] (Sanity check, not a code change.) Confirm the existing `include: ['test/**/*.test.js']` glob (line 8) catches `test/components/Layout.test.js`. It does — the test file matches the recursive glob.

### Smoke (manual, in addition to the vitest run)

Run these in order; paste the relevant output / screenshots into the PR description:

- [ ] **Logged-out smoke.** With no `auth_token` in `localStorage` (DevTools → Application → Local Storage → clear `auth_token`), `npm run dev` and visit `http://localhost:3000/dashboard` (or any page that renders Layout). Expect:
  - The desktop sidebar bottom shows "Sign in" with a small icon, in place of the avatar + email + dropdown chevron.
  - Clicking "Sign in" routes to `/login`.
  - **The string `me@randallstillwell.com` does NOT appear anywhere on the page.** (Open DevTools → Console → run `document.body.innerText.includes('me@randallstillwell.com')` → expect `false`.)
  - The "My Collection", admin, and authenticated-only nav items are hidden.
  - Public + community nav items (Cards, Scanner, Deck Builder placeholder, Community section) are still visible.
- [ ] **Logged-in smoke.** Log in as a non-admin (`admin@tcgvault.com` works since the seed admin has `role='admin'` — use a non-admin signup or temporarily flip the row). Visit `/dashboard`. Expect:
  - The desktop sidebar bottom shows the real user's email + role.
  - Clicking the avatar opens the dropdown with Profile / Settings / Logout (no Admin Panel for non-admin).
- [ ] **Mobile drawer smoke.** Resize Chrome DevTools to a phone preset (iPhone 13). Open the mobile drawer ("More" tap). Expect: the same Sign-in / authenticated states render in the drawer's bottom section as in the desktop sidebar.
- [ ] **Lint.** `npm run lint` exits 0 (or matches the existing pre-PR baseline — pre-existing lint errors are fine; do not introduce new ones).
- [ ] **Tests.** `npm run test:run` is green: 5 new + 24 pre-existing = 29 tests pass.

### Out of scope (do not do these)

- [ ] No edits to any of the 17 pages — that's Brief 2.
- [ ] No edits to `components/MobileNavigation.js` — its dead `user` prop is a deferred cleanup.
- [ ] No edits to `lib/use-auth.js`, `lib/auth-context.js`, `lib/admin-auth.js`, `lib/permission-middleware.js`, or `lib/auth-secret.js`. The auth surface is downstream of this brief.
- [ ] No edits to `vitest.config.js` (the per-file `// @vitest-environment jsdom` directive is the entire mechanism).
- [ ] No new vitest tests beyond `test/components/Layout.test.js`. Brief 2 does not add tests either (Decision D = D2 covers the contract at the component boundary).
- [ ] No `.cursor/rules/*.mdc` updates. The rule (`ui-and-theming.mdc` § Component conventions) already documents the intent.
- [ ] No `AGENTS.md` Gotcha #8 update — that's the post-convoy doc-writer pass.
- [ ] No `README.md` update.
- [ ] No `CHANGELOG.md` (none exists yet — `adopt-keep-a-changelog` is a separate convoy).

## Rationale (≤3 sentences)

The Layout default is the actual P0 #7 bug; this brief fixes it at the source and locks in the contract with a regression test that asserts the maintainer email never reappears for the null-user branches. Splitting the page audit into Brief 2 keeps the conceptually interesting change (Layout + logged-out branch + test harness) reviewable on its own; the page edits are mechanical and benefit from being grouped separately. Adding `jsdom` + `@testing-library/react` as devDeps is the smallest harness that lets vitest exercise React rendering — the tools are common, the surface is two packages, and the value (preventing a recurrence of a default-prop email leak) is high.