Convoy: single-sql-client (P1 quality, launch sequence step 8)
Addresses: AGENTS.md Gotcha #1, .convoys/ship-readiness.md P1 #8
## Decisions
- D1: Caller inventory = 2 files (1 source + 1 test), not "~3 based on graph".
Only pages/api/auth-utils.js imports `db`; test/api/auth-utils.test.js mocks
it purely to satisfy the import graph (the 5 tests exercise
generateToken/verifyToken, not isAdmin/getUserById).
- D2: Migrate both call sites (isAdmin, getUserById) to @vercel/postgres
tagged-template SQL. Queries are SELECT-only, single-table,
single-numeric-parameter — byte-equivalent translation; same result shape
({rows, rowCount}); no transaction or pool semantics differ.
- D3: KEEP @neondatabase/serverless as a dep. 11 scripts/* files still use
`neon()` directly (setup-neon-db.js, migrations/, reset-db.js, 8 historical
add-*/fix-*/seed-* jobs). They are out of scope per the no-go-zones rule
and the convoy spec; purging the dep entirely would be its own convoy
(queued as `purge-neondatabase-serverless-fully`, blocked on migration-tool).
- D4: sql.unsafe audit — NOT a real injection vector with current callers
(userId comes from a verified JWT, is a numeric SERIAL id). Security
finding: NO. Pure refactor + foot-gun removal that prevents the FUTURE
caller that would have been the incident.
- D5: Test mock cleanup — drop the now-unneeded `vi.mock('../../lib/database.js')`
call + unused `vi` import. Test count + assertions unchanged (5/5).
## Per-file changes
- pages/api/auth-utils.js: swap `import { db } from '../../lib/database.js'`
for `import { sql } from '@vercel/postgres'`; rewrite isAdmin's
`db.query(SELECT … WHERE id = $1, [userId])` and getUserById's same shape
to `sql\`SELECT … WHERE id = ${userId}\``. Same try/catch, same
result.rows[0] access, same error returns.
- test/api/auth-utils.test.js: drop vi.mock for lib/database.js + the unused
`vi` import. 5/5 tests still pass.
- lib/database.js: DELETED (47 lines removed; manual-interpolation + sql.unsafe
wrapper is gone).
- .convoys/single-sql-client.md: NEW (the convoy file documenting all
decisions + caller inventory + verification + risks + follow-ups).
## Verification
- npm run lint → 128 problems (baseline preserved, no regression)
- npm run test:run → 21/21 pass (vitest)
- Grep "lib/database" --type js -l → 0 hits anywhere
- Grep "@neondatabase/serverless" --type js -l → still matches the 11
scripts/* sites (expected; out of scope per D3)
- node --check pages/api/auth-utils.js → exit 0
## Scope note
This convoy collapses the lib/database.js abstraction onto the canonical
@vercel/postgres surface for pages/api/**. It does NOT eliminate
@neondatabase/serverless from the dependency tree — that would require
migrating the scripts/* helpers, which is out of scope here (no-go-zones
rule + convoy spec). Queued as a follow-up.
## Live smoke
Deferred. The two migrated functions (isAdmin, getUserById) are only
reachable via pages/api/admin/index.js which requires an admin Bearer
token and a populated users table in prod Neon. Byte-equivalent SQL +
identical result shape gives high confidence; rollback is a single-commit
revert if a post-merge admin action 500s.
Co-authored-by: Cursor <cursoragent@cursor.com>
* 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>
Closes AGENTS.md gotcha #11 (well, the relevant half of it — "Testing:
None yet" line in §6 is now stale).
Installs vitest@^3.2.4 (single devDep, no UI / coverage / jsdom) and
adds 16 unit tests across 3 files that lock in post-Brief-1/2/4
behavior:
test/lib/auth-secret.test.js (3 tests)
- JWT_SECRET exports the env value
- JWT_TOKEN_TTL is canonical 24h
- Module throws at load when JWT_SECRET is empty
test/lib/permission-middleware.test.js (8 tests)
- getUserFromRequest returns null for: missing header, non-Bearer
scheme, malformed token, wrong-secret token, expired token,
valid-token-no-user-row
- Returns user object for valid token + user row
- Brief 2 regression lock: does NOT return the synthetic admin
shape { userId: 1, email: 'admin@tcgvault.com', role: 'admin' }
when no Authorization header is present
test/api/auth-utils.test.js (5 tests)
- generateToken issues 24h JWT (exp - iat === 86400)
- Payload includes userId, email, role
- verifyToken round-trips valid tokens
- Returns null for malformed / wrong-secret tokens
CI: re-enabled the previously commented-out test: job in
.github/workflows/ci.yml. Blocking (no || true wrapper) — vitest is
the first runner in this repo and we want CI red on test regression.
JWT_SECRET is set via a CI-only fake; production secret is unaffected.
Rate-limit (Brief 4) coverage deferred to a future expand-auth-tests
convoy per architect's call (R11). package.json has "type": "module"
so vitest's default Vite-based transform handles .js ESM out of the
box — no transform config needed.
Convoy: fix-auth-bypass / Brief 5 (last brief)
Co-authored-by: Cursor <cursoragent@cursor.com>
varutasu