diff --git a/.convoys/fix-layout-default-user.md b/.convoys/fix-layout-default-user.md new file mode 100644 index 0000000..2165cd6 --- /dev/null +++ b/.convoys/fix-layout-default-user.md @@ -0,0 +1,132 @@ +--- +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.