deckhearth/.convoys/fix-layout-default-user/brief-2-page-audit-pass-user-explicit.md
varutasu ca302a89c1
fix(layout+pages): default user=null + page audit sweep (P0 #7) (#15)
* convoy: scope fix-layout-default-user (P0 #7 — Layout maintainer-email leak)

The last remaining P0 ship-blocker from .convoys/ship-readiness.md.
components/Layout.js line 562 defaults the user prop to a real email
address (me@randallstillwell.com); any page that renders Layout without
passing user explicitly impersonates the maintainer.

Scope: components/Layout.js + audit of 17 pages that import Layout
(grep-confirmed list in convoy file). Single PR likely. Auditor cohort
skipped (no design-system, IA, or browser-smoke surface).

Architect to address:
  - Q1: logged-out rendering branch design (navbar, mobile-nav,
        auth-only items treatment)
  - Q2: page audit triage into always-auth / public-or-auth /
        anonymous-allowed buckets
  - Q3: brief decomposition (single brief / 2 briefs in 1 PR / fan-out)
  - Q4: whether to add vitest coverage for the logged-out branch
        (recommend yes — small surface, high regression protection)

Hard out-of-scope: branding (pick-a-name), auth-provider collapse
(single-auth-provider), Layout god-component split (god-component-split).

depends_on: bump-next-js (shipped), fix-auth-bypass (shipped),
            drop-public-setup (shipped)
addresses: P0 #7 from .convoys/ship-readiness.md
parent: ship-readiness

Co-authored-by: Cursor <cursoragent@cursor.com>

* architect(fix-layout-default-user): plan + briefs 1-2 (Layout fix + page audit)

2 briefs, single PR. ~12 files net (down from the 18 in the original scope —
10 of the 17 Layout-importing pages already pass user explicitly).

Brief 1: components/Layout.js default user=null + Sign-in CTA branch in
  UserProfileDropdown when logged out. Adds first jsdom test in the repo
  at test/components/Layout.test.js (Decision D2) with 5 regression-lock
  assertions. devDeps: jsdom@^29, @testing-library/react@^16.

Brief 2: page audit sweep — 7 pages need code changes:
  - Pass user={user} to Layout: scanner.js, deck-builder.js (×4),
    deck/[id].js (×3), decks.js (×3)
  - Replace page-level useState({email: 'me@...'}) → useState(null) +
    null-guards: profile.js, settings.js
  - Replace hardcoded const user = {email: 'me@...'} with useAuth():
    card/[id].js

Discovered second anti-pattern: profile.js, settings.js, card/[id].js
seed page-level state with the maintainer email. Folded into Brief 2 since
success metric "no real email address remains in any component default-prop"
reads naturally to include page-level seed values.

Decisions:
  A1 — Sign-in CTA replaces avatar+email+dropdown when user===null;
       hides auth-only dropdown (Profile/Settings/Logout/Admin);
       keeps public + community nav visible
  B  — Per-page bucket assignment (10 already correct, 7 need fix);
       full per-page table with justification in convoy file
  C2 — Two briefs in one PR (Brief 1 = Layout + test; Brief 2 = page
       sweep depends on Brief 1). C1 buries the conceptual change under
       mechanical edits; C3 is over-orchestrated for this scope
  D2 — vitest lock-in; first jsdom test in repo; same negative-regression
       style as test/lib/permission-middleware.test.js (synthetic-admin
       shape). devDeps jsdom + @testing-library/react

Risks tracked R1-R8. Biggest: R2 (useState(null) null-deref in 3 leaky
pages — mitigated by audit-pass mandate + manual smoke).

MobileNavigation deliberately NOT folded in: its user prop is dead code
(never reads user.*); different bug class; cleanup queued separately to
avoid scope expansion.

Flagged-but-deferred:
  - 4 pages still import useAuth from lib/auth-context.js
    → single-auth-provider (queued P1 #9)
  - Layout headers still render "Deck Hearth" / "DH" branding
    → pick-a-name (queued P1 #12)
  - MobileNavigation dead user prop → cleanup-mobile-nav-dead-props
    or fold into god-component-split

addresses: P0 #7 from .convoys/ship-readiness.md (last P0 ship-blocker)
parent: ship-readiness
Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(layout): default user=null + Sign-in CTA when logged out (Brief 1 of fix-layout-default-user)

Closes the source-side half of P0 #7 from .convoys/ship-readiness.md.
The page-side sweep (Brief 2) follows in a separate commit.

components/Layout.js:
  - Default user prop is now null (was hardcoded to
    { email: 'me@randallstillwell.com', role: 'user' })
  - UserProfileDropdown renders a "Sign in" link to /login when
    user === null instead of the maintainer's email + auth-only menu
    items (Decision A1)
  - All user.* accesses guarded with optional chaining or null checks
  - useState hook stays above the new null-user early return to satisfy
    rules-of-hooks (boot-the-brief caught this on the first try;
    see AGENTS.md Gotcha #11.5)

test/components/Layout.test.js (new):
  - First jsdom test in the repo (Decision D2)
  - 5 regression-lock assertions: no maintainer email ever rendered
    (prop omitted, prop=null), Sign-in link exists with href=/login,
    supplied email renders when prop is set, no "Guest" placeholder
    (locks A1 copy choice)
  - Mocks next/link, next/router (prefetch, replace, events, query),
    and theme-context.useTheme for jsdom safety under Next 16

package.json + package-lock.json:
  - Add jsdom@^29 and @testing-library/react@^16 to devDependencies
  - @testing-library/dom@^10 added explicitly (peer auto-install
    skipped it under npm 11; brief anticipated this fallback)

vitest.config.js (deviation from brief — see PR description):
  - Add esbuild { loader: 'jsx', jsx: 'automatic' } so vitest can
    parse JSX in .js files. Required to import any React component
    written in the repo's Next.js pages-router .js convention
    (AGENTS.md Gotcha #9). The brief said "no change" to this file,
    but JSX-in-.js parsing is a hard prerequisite for the new test
    to import components/Layout.js — the alternatives (rename test
    to .test.jsx; rewrite test in React.createElement) either break
    the test glob or still hit the same Layout.js parse failure.
    Other tests are unaffected (they import non-JSX modules).

Smoke output: see PR description.

addresses: P0 #7 from .convoys/ship-readiness.md (last P0 ship-blocker)
Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(pages): pass user explicitly + null-guard leaky page seeds (Brief 2 of fix-layout-default-user)

Closes the page-side half of P0 #7 from .convoys/ship-readiness.md.
Brief 1 (commit ddf8fd2) handled the Layout-side fix.

Per the architect's per-page bucket table (Decision B in
.convoys/fix-layout-default-user.md), 7 pages needed code changes;
the other 10 of 17 Layout-importing pages already pass `user` correctly.

Pass user={user} to Layout (4 pages, 11 call sites):
  - pages/scanner.js (1 call)
  - pages/decks.js (3 calls)
  - pages/deck-builder.js (4 calls)
  - pages/deck/[id].js (3 calls)
  (All four still import useAuth from lib/auth-context.js — that's
   intentional and stays as-is until the single-auth-provider convoy
   collapses the three parallel auth surfaces.)

Replace leaky page-level seed values with useState(null) + null guards
(2 pages, R2 mitigation):
  - pages/profile.js: useState({email: 'me@...', role: 'user', ...})
                     → useState(null) + ?. on every sync user.* read
                     + early-return guards in getDisplayName/getInitials
                     + conditional render around the "Member since" block
                       so formatDate(undefined) never runs
  - pages/settings.js: same pattern (single user.email reader guarded)

Replace hardcoded const with useAuth from lib/use-auth.js (1 page):
  - pages/card/[id].js: const user = {email: 'me@...'}
                       → const { user } = useAuth() (called unconditionally
                       at the top of the component; rules-of-hooks safe)

Verification:
  - grep 'me@randallstillwell.com' pages/ → 0 hits
  - 21/21 vitest tests pass (16 pre-existing + 5 from Brief 1)
  - npm run lint matches baseline (128 problems pre, 128 post; verified
    via git stash before/after)
  - Manual static read-through of every diff; ReadLints clean on the 7
    files
  - Dev-server smoke: /cards anonymous returned HTTP 200 with 0
    'me@randallstillwell' matches before the user's shared dev server
    became unresponsive mid-session (same dev-server-shared-by-user
    constraint flagged in Brief 1); interactive logged-in smoke is
    parent/operator gated

Flagged-but-deferred (untouched per scope):
  - 4 pages still import useAuth from lib/auth-context.js
    → single-auth-provider (queued P1 #9)
  - components/MobileNavigation.js still receives dead user prop
    → cleanup-mobile-nav-dead-props (or fold into god-component-split)

addresses: P0 #7 from .convoys/ship-readiness.md (last P0 ship-blocker)
Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-24 14:31:37 -05:00

15 KiB

convoy brief_number depends_on files cross_brief_commitments
fix-layout-default-user 2
1
pages/scanner.js
pages/deck-builder.js
pages/deck/[id].js
pages/decks.js
pages/profile.js
pages/settings.js
pages/card/[id].js
brief description
1 Brief 1 changed `components/Layout.js` so the `user` prop defaults to `null` and a logged-out branch in `UserProfileDropdown` renders a "Sign in" CTA. This brief assumes that branch exists — the seven pages below either (a) currently rely on the maintainer-email default by omitting `user`, or (b) seed `user` from a hardcoded `me@randallstillwell.com` initializer. After this brief, every Layout call site sources `user` from `useAuth()` (or remains a legitimate `user={null}` for anonymous-allowed surfaces). Without Brief 1's logged-out branch, this brief would silently re-leak the maintainer email through Layout's default. Ship Brief 1 first in the diff.

Brief 2: Pages — pass user explicitly to Layout, and drop page-level maintainer-email defaults

Goal (1 sentence)

Audit the 17 pages that render Layout and fix the seven that today either omit the user prop (so Layout's default kicks in) or seed user from a hardcoded email: 'me@randallstillwell.com' initializer; after this brief, every page sources user from an auth hook (or passes user={null} deliberately on anonymous-allowed surfaces).

Files in scope (do not edit anything else)

  • pages/scanner.js — modified.
  • pages/deck-builder.js — modified.
  • pages/deck/[id].js — modified.
  • pages/decks.js — modified.
  • pages/profile.js — modified.
  • pages/settings.js — modified.
  • pages/card/[id].js — modified.

Conventions to follow

  • .cursor/rules/auth-and-permissions.mdc § Canonical surface vs Legacy. The canonical client hook is lib/use-auth.js::useAuth. The legacy lib/auth-context.js::useAuth and lib/admin-auth.js::useAdmin are still wired for compatibility but should not gain new consumers. However, four of the seven files in this brief already import useAuth from lib/auth-context.js. Do NOT migrate those imports to lib/use-auth.js here — that's the queued single-auth-provider convoy's job. The migration touches ~30 files and needs a coordinated sweep; cherry-picking four of them would diverge that convoy's plan. Specifically:
    • pages/scanner.js — keep import { useAuth } from '../lib/auth-context';.
    • pages/deck-builder.js — keep import { useAuth } from '../lib/auth-context';.
    • pages/deck/[id].js — keep import { useAuth } from '../../lib/auth-context';.
    • pages/decks.js — keep import { useAuth } from '../lib/auth-context';.
  • .cursor/rules/ui-and-theming.mdc § Component conventions. Pages that render Layout MUST pass user explicitly. Page-level useState({ email: 'me@…' }) initializers fail the same rule even though Layout itself is fixed; clear them.
  • .cursor/rules/no-go-zones.mdc. Do not edit components/Layout.js.backup.
  • Brief size discipline. This brief is seven page-level edits, each small. Do NOT:
    • Touch any file outside files: above. Specifically: no components/**, no lib/**, no test/**, no package.json, no .github/**, no .cursor/rules/**.
    • Add new vitest tests. Brief 1's test/components/Layout.test.js covers the contract at the component boundary; per-page tests would duplicate it.
    • Migrate a page off lib/auth-context.js (see above).
    • Restructure loading / useEffect chains. Each page already has a working data-fetch pattern; the only thing changing is where the seed user comes from.
    • Convert pages to TypeScript (the repo is JavaScript-only — AGENTS.md Gotcha #9).
  • No new dependencies. All seven files use modules already imported elsewhere in the codebase.

Acceptance criteria

The seven files split into two patterns. Apply the corresponding pattern to each.

Pattern A — pages that omit user on Layout (4 files)

These pages already call useAuth() (from lib/auth-context.js) but render <Layout> without forwarding the user reference. Add user={user} to every <Layout> call.

pages/scanner.js

  • Locate line 333: <Layout>.
  • Replace with <Layout user={user}>.
  • No other change to the file. The existing import { useAuth } from '../lib/auth-context'; (line 8) and const { user } = useAuth(); (line 11) stay as-is.
  • Verify the if (!user) router.push('/login') redirect (line 30-35) still runs first. After Brief 1, Layout will render the logged-out shape briefly while user === null and loading === true; once the redirect to /login fires, the user lands on the AuthLayout-rendered login page. Acceptable.

pages/deck-builder.js

  • Replace each of the four <Layout> calls with <Layout user={user}>. Lines (per current grep): 262, 277, 287, 303.
  • No other change. import { useAuth } from '../lib/auth-context'; (line 7) and const { user } = useAuth(); (line 11) stay.

pages/deck/[id].js

  • Replace each of the three <Layout> calls with <Layout user={user}>. Lines: 132, 142, 160.
  • No other change. import { useAuth } from '../../lib/auth-context'; (line 6) and const { user } = useAuth(); (line 10) stay.

pages/decks.js

  • Replace each of the three <Layout> calls with <Layout user={user}>. Lines: 161, 176, 185.
  • No other change. import { useAuth } from '../lib/auth-context'; (line 5) and const { user } = useAuth(); (line 8) stay.

Pattern B — pages with hardcoded maintainer-email initializers (3 files)

These pages seed their user state/const with the maintainer's email at the page level. Replace the initializer with null (and source from useAuth() where the page does not already have an auth hook). Add the necessary null-guards on sync reads.

pages/profile.js

  • Replace lines 10-20 (const [user, setUser] = useState({ email: 'me@randallstillwell.com', role: 'admin', first_name: '', last_name: '', username: '', bio: '', avatar_url: '', favorite_games: ['MTG'], created_at: new Date().toISOString() });) with:
const [user, setUser] = useState(null);
  • Add null-guards on every sync read of user.* in the JSX. The reader functions (getDisplayName, getInitials near lines 239-249) already use optional chaining where it matters (user.first_name, user.last_name, user.username, user.email); confirm they handle null:
    • getDisplayName(): returns user.first_name || user.last_name ... — change to start with if (!user) return ''; so the first sync render is safe.
    • getInitials(): same pattern — if (!user) return ''; at the top.
    • In the JSX (around line 260-450), every user.email, user.role, user.username, user.bio, user.avatar_url, user.first_name, user.last_name, user.created_at access must use user?.* (optional chaining). Most are inside {user && (...)} blocks already; verify each one.
  • The loading branch (line 258-267, if (loading) { return <Layout user={user}>... }) renders <Layout user={user}> — after this brief, user is null during loading, so Layout will show the "Sign in" branch. That's correct; loadUserProfile redirects to /login if no token (line 51-54), so this flash only happens for already-authenticated users while their profile fetches.
  • Verify with rg "user\." pages/profile.js — every match must be inside an optional-chain (user?.) or a {user && ...} guard or if (!user) early-return.
  • Do NOT add an import { useAuth } from '...'. The page's existing pattern (manual fetch from /api/user/profile) stays — adding useAuth here would be a second source of truth and a single-auth-provider migration. The point of this brief is that the seed value is null, not who provides it.

pages/settings.js

  • Replace lines 9-12 (const [user, setUser] = useState({ email: 'me@randallstillwell.com', role: 'admin' });) with:
const [user, setUser] = useState(null);
  • Audit every user.* read in the JSX (notably line 306 value={user.email} in the email read-only field). Wrap each in optional chaining or a {user && (...)} guard. The simplest fix for line 306 is value={user?.email || ''}.
  • The loading branch (line 228-236, if (loading) { return <Layout user={user}>... }) renders <Layout user={user}> — same flow as profile.js. After this brief, the loading flash shows "Sign in" briefly until loadSettings redirects (line 53-55) or resolves with the real user.
  • Do NOT add useAuth. Same reasoning as profile.js.
  • Verify with rg "user\." pages/settings.js — every match must be optional-chain / guard / early-return safe.

pages/card/[id].js

  • Replace lines 13-16 (the hardcoded const user = { email: 'me@randallstillwell.com', role: 'user' };) with:
const { user } = useAuth();
  • Add the import at line 4 (between the existing useIsAdmin import on line 4 and the next line — alphabetical / grouping is the implementer's call):
import { useAuth } from '../../lib/use-auth';
  • lib/use-auth.js is the canonical hook (auth-and-permissions.mdc § Canonical surface). card/[id].js does not currently import any auth hook (the useIsAdmin import is from lib/admin-auth.js, but useIsAdmin returns a boolean, not a user object), so we are adding a fresh consumer rather than migrating one — the "no new consumers of legacy providers" rule is satisfied.
  • Audit every user.* read in the filepages/card/[id].js is 913 lines; run rg "user\." pages/card/[id].js and confirm every match is user?.* or under a {user && (...)} guard. Likely matches: user.userId, user.email, user.role. The page is public-or-authenticated, so each user.* site should already gracefully handle "no user" — the convoy's bucket assignment for this page is Bucket 2 (public-or-auth).
  • The three <Layout user={user}> calls (lines 343, 353, 376) stay — user is now sourced from useAuth() and may be null for anonymous viewers.
  • Verify the page still renders for an anonymous visitor at /card/some-id after the change. (Smoke test below.)

Repo-wide grep verification (run before opening PR)

  • rg "me@randallstillwell" --type js returns hits ONLY in components/Layout.js.backup (no-go zone, untouched). Specifically: NO hits in components/Layout.js, pages/profile.js, pages/settings.js, pages/card/[id].js. (The existing components/Layout.js hit was removed by Brief 1.)
  • rg "<Layout>" pages --type js returns ZERO hits — every Layout opening tag in pages/ includes a user= prop.
  • rg "import.*useAuth.*lib/auth-context" pages --type js returns the SAME four hits as before this brief (pages/scanner.js, pages/deck-builder.js, pages/deck/[id].js, pages/decks.js). No new hits, no removals — confirms we didn't migrate the legacy-provider consumers.
  • rg "import.*useAuth.*lib/use-auth" pages --type js includes pages/card/[id].js (new hit) plus the existing six (pages/dashboard.js, pages/my-cards.js, pages/cards.js, pages/collection/[identifier].js, pages/community/collections.js, pages/collections.js).

Smoke (manual)

Run these in order; paste the relevant output / screenshots into the PR description. Each scenario MUST be tested both logged out (no auth_token in localStorage) and logged in (a real user account).

  • /scanner
    • Logged out: redirected to /login. Layout never renders maintainer email (the redirect fires from useEffect but Layout briefly shows "Sign in" before navigation). DevTools console: no errors.
    • Logged in: navbar shows real user email; scanner UI loads.
  • /deck-builder (no query string, just the bare route)
    • Logged out: Layout shows "Sign in" in the sidebar; the page's own loading / empty state renders without crashing.
    • Logged in: navbar shows real user email; deck builder loads.
  • /deck/<some-id> (use any deck id from npm run dev admin → decks list)
    • Logged out (public deck): Layout shows "Sign in"; the deck detail still renders (this is a Bucket 2 page).
    • Logged in: navbar shows real user email; deck detail loads.
  • /decks
    • Logged out: Layout shows "Sign in"; the page's own auth-gated content (private deck list) shows the appropriate empty / login-prompt state.
    • Logged in: navbar shows real user email; deck list loads.
  • /profile
    • Logged out: redirected to /login by loadUserProfile's 401 branch. Layout briefly shows "Sign in" before redirect. No me@randallstillwell.com flash anywhere — this is the change you are smoke-testing.
    • Logged in: navbar shows real user email; profile loads with real user data.
  • /settings
    • Logged out: redirected to /login. Same flash as profile.
    • Logged in: navbar shows real user email; settings load.
  • /card/<some-id> (use any card id from /cards)
    • Logged out: Layout shows "Sign in"; card detail still renders (this is the public-card-detail page, Bucket 2). No me@randallstillwell.com anywhere on the page. Open DevTools → Console → document.body.innerText.includes('me@randallstillwell.com') → expect false.
    • Logged in: navbar shows real user email; card detail loads, "Add to collection" UI works.
  • Lint. npm run lint exits 0 (or matches the existing pre-PR baseline; do not introduce new lint errors).
  • Tests. npm run test:run is green: 5 (Brief 1) + 24 (pre-existing) = 29 tests pass.

Out of scope (do not do these)

  • No edits to any of the other 10 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). They already pass user correctly per the per-bucket audit in .convoys/fix-layout-default-user.md § Decisions B.
  • No edits to components/**, lib/**, test/**, package.json, package-lock.json, .github/**, .cursor/rules/**.
  • No migration of the four legacy-provider importers (scanner.js, deck-builder.js, deck/[id].js, decks.js) to lib/use-auth.js. That's single-auth-provider.
  • No removal of the dead user prop on MobileNavigation — flagged for follow-up.
  • No vitest tests added — Brief 1's test/components/Layout.test.js is the convoy's single test addition.
  • No AGENTS.md Gotcha #8 update — post-convoy doc-writer pass.
  • No README / CHANGELOG / TESTING_GUIDE updates.

Rationale (≤3 sentences)

The convoy's success metric requires that no real email address remains in any component default-prop AND that every Layout call site passes user correctly; Brief 1 fixed Layout but four pages omit the prop entirely (re-triggering the default if not for Brief 1's null default) and three pages seed page-level state with the maintainer email (a parallel anti-pattern that Brief 1 cannot reach). Splitting these seven mechanical edits into Brief 2 keeps Brief 1's diff focused on the actual fix + test, while this brief sweeps the call sites in one pass. Leaving the four legacy-auth-context imports alone preserves the planned single-auth-provider migration's clean diff — fixing the user-prop default does not require re-architecting which auth hook a page imports.