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

convoy	brief_number	depends_on	files	cross_brief_commitments
fix-layout-default-user	1		components/Layout.js test/components/Layout.test.js package.json package-lock.json	brief description 2 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:

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

After:

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:

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:

// @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.