diff --git a/.convoys/cleanup-mobile-nav-dead-props.md b/.convoys/cleanup-mobile-nav-dead-props.md deleted file mode 100644 index 1e75eb1..0000000 --- a/.convoys/cleanup-mobile-nav-dead-props.md +++ /dev/null @@ -1,155 +0,0 @@ -# cleanup-mobile-nav-dead-props (P3 polish — single-prop hygiene) - -**Status:** SHIPPED 2026-05-26 (PR TBD) -**Classification:** hygiene -**Priority:** P3 polish (not a bug, not a security issue; dead-prop -removal is purely a clarity-of-surface cleanup) -**Convoy owner:** parent (no architect — single-line prop removal in -one component + one caller; surfaced and pre-decided in a sibling -convoy) -**Opened:** 2026-05-26 - -## Background - -`components/MobileNavigation.js` accepts `{ user, onMenuOpen }` but -never reads `user.*` — the bottom-bar items (Cards, Decks, Dashboard, -Community, More) are static and don't depend on auth state or role. - -This was originally surfaced as **R8** in the `fix-layout-default-user` -convoy (see `.convoys/fix-layout-default-user.md` § R8 and § "Anything -flagged but not acted on") and deferred there with explicit -instructions: *"If the implementer is tempted to delete the prop, they -MUST stop — that's god-component-split / single-auth-provider -territory."* The deferral was correct for that convoy's scope; it is -no longer needed because the prop is genuinely dead at the current -static-bar reality, and removing it does not require a wider auth -refactor. - -The follow-up was queued as `cleanup-mobile-nav-dead-props` in -`.convoys/ship-readiness.md` § Queued convoys, with a note that it -may fold into `god-component-split` (P2 #13) if that lands first. -God-component-split has not landed; this small hygiene convoy ships -first. - -## Audit results - -Pre-edit audit (the spec's "don't blindly trust the queue entry" -clause): - -1. **Reading `components/MobileNavigation.js`** — the file is 171 - lines. Line 5 destructures `{ user, onMenuOpen }`. Lines 6–170 use - `onMenuOpen` exactly once (line 39, as the `onClick` for the "More" - button). `user` does not appear elsewhere — no `user.email`, - `user.role`, `user.id`, no conditional render gated on `user`, no - pass-through to a child component. The bottom-bar `navigationItems` - array is hardcoded and does not branch on auth state. -2. **`rg '\buser\b' components/MobileNavigation.js`** before edit: 1 - hit (the destructure on line 5). After edit: 0 hits. -3. **`rg "MobileNavigation" components/ pages/ --type js`**: two - import + JSX-callsite pairs in the codebase: - - `components/Layout.js` (active) — line 5 import, line 598-601 JSX - call passing `user={user}` and `onMenuOpen={...}`. - - `components/Layout.js.backup` (no-go-zone per - `.cursor/rules/no-go-zones.mdc` § "Append-only / historical" — - "legacy snapshot; delete with a real PR, never edit") — line 5 - import, line 264 JSX call. Left untouched per the no-go-zone - rule; if/when the `.backup` file is eventually deleted, this dead - call disappears with it. - -Audit verdict: `user` is genuinely dead. Cleanup is safe. - -## The fix - -Two-file, three-line diff: - -1. **`components/MobileNavigation.js` line 5**: remove `user` from the - destructured props. - - Before: `export default function MobileNavigation({ user, onMenuOpen }) {` - - After: `export default function MobileNavigation({ onMenuOpen }) {` -2. **`components/Layout.js` lines 598-601**: remove the `user={user}` - JSX attribute from the only active call site. - - Before: - ``` - setIsMobileMenuOpen(true)} - /> - ``` - - After: - ``` - setIsMobileMenuOpen(true)} - /> - ``` - -No new code. No refactors. No tests added (the component has no -direct test coverage; `test/components/Layout.test.js` tests Layout's -logged-out branch and does not assert on `MobileNavigation`'s prop -shape). - -## Out of scope - -- The unused `import { useState } from 'react'` on - `components/MobileNavigation.js` line 3. The hook is imported but - not called. This is a pre-existing dead import unrelated to the - `user` prop; the convoy spec explicitly forbids "refactor anything - else in MobileNavigation.js (this is a single-prop removal)". A - future hygiene pass can sweep it (or it'll get caught by an - eventual lint-no-unused-imports rule). -- `components/Layout.js.backup` — no-go-zone, untouched. - -## Verification plan - -1. `rg '\buser\b' components/MobileNavigation.js` → 0 hits (post-edit - confirmation that the prop is truly gone, not just renamed). -2. `rg "MobileNavigation" components/ pages/ --type js` → confirm - each active call site passes only `onMenuOpen`. -3. `npm run lint` → 128 problems baseline preserved (no regression - introduced; no new dead-code/unused-var warnings created by the - change). -4. `npm run test:run` → 21/21 pass. Specifically, - `test/components/Layout.test.js` continues to pass — its - regression-lock assertions for the logged-out Layout branch - (Gotcha #8) do not depend on `MobileNavigation`'s prop shape, so - the dead-prop removal is invisible to that suite. -5. `npm run build` skipped — relying on Vercel preview CI. Trade-off: - single-prop removal in a leaf component is extremely low risk of - build-time regression, and the Playwright smoke + visual-diff - workflows on the PR will catch any Layout-rendering issue before - merge. - -## Risks - -- **R1 — A future feature that wants per-user bottom-bar items would - need to re-add the prop.** Hypothetical examples: showing an - unread-count badge on a "Notifications" tab gated on `user.id`, or - hiding the "Community" tab for unauthenticated visitors. **Accepted.** - Re-adding a prop is a one-line change when the feature actually - lands; carrying a dead prop "just in case" obscures the current - surface and adds nothing. The cleanup is correct for the - current static-bar reality; future features pay their own - add-the-prop cost. -- **R2 — `components/Layout.js.backup` still references the old prop - shape.** **Accepted.** The backup is a no-go-zone (per - `.cursor/rules/no-go-zones.mdc`) and is dead code by definition. - Touching it would violate the rule; leaving it as a stale snapshot - is the convention. When the backup is eventually deleted in a - separate convoy, this stale call disappears with it. - -## Acceptance criteria - -- 2 files modified, 3 lines net change (1 line edit + 1 attribute - removal from a multi-line JSX block). -- `npm run lint` exit 1 with 128 problems (baseline preserved). -- `npm run test:run` 21/21 pass. -- `rg '\buser\b' components/MobileNavigation.js` → 0 hits post-edit. - -## Owns - -Parent (single-prop removal in a leaf component; no architect or -implementer subagent required; audit confirms the queue entry's -premise). - -## As-shipped - -_To be filled in post-merge._ diff --git a/.convoys/lint-against-cjs-in-esm-scripts.md b/.convoys/lint-against-cjs-in-esm-scripts.md deleted file mode 100644 index 059477b..0000000 --- a/.convoys/lint-against-cjs-in-esm-scripts.md +++ /dev/null @@ -1,176 +0,0 @@ ---- -name: lint-against-cjs-in-esm-scripts -classification: hygiene -success_metric: future helper scripts that re-introduce CJS `require()` calls under `package.json` "type": "module" fail at lint time, not at first execution -status: open -created: 2026-05-26 ---- - -# lint-against-cjs-in-esm-scripts (P3 polish — parent-owned) - -**Priority:** P3 polish (one-line ESLint rule; no architect required) -**Convoy owner:** parent -**Opened:** 2026-05-26 - -## Background — the recurring bug pattern - -Since `bump-next-js` flipped `package.json` to `"type": "module"`, -any helper script under `scripts/` that uses CJS `require()` throws -`ReferenceError: require is not defined` on Node 22.x at first run. -The same bug has now bitten the repo twice in two convoys: - -1. **`drop-public-setup` Brief 2** (commit `b63b509`, 2026-05-23): - `scripts/setup-neon-db.js` was still CJS post-`bump-next-js`; `npm - run setup-db` was silently broken until Brief 2 swept it to ESM - imports. The convoy retro called this out as "the seed script - silently stopped executing after `bump-next-js`." -2. **`fix-reset-db-script` Brief 1** (commit `3ab9bf8`, PR #25, - 2026-05-26): three `require()` calls in `scripts/reset-db.js` - (lines 10, 12, 142) — same bug, same blast radius (`npm run - reset-db` throws `ReferenceError`), same fix shape (verbatim - mirror of post-`drop-public-setup` `setup-neon-db.js`). - -Both bugs were caught at first run, not at lint time. A small -ESLint rule scoped to `scripts/**/*.js` would have caught both at -PR time and is cheap insurance against a third recurrence. - -## Design decision — `scripts/**` only (NOT all `.js`) - -Two reasonable scopes: - -- **`scripts/**/*.js` (chosen):** matches the actual blast radius — - every observed instance of the bug has been in a helper script. - Per-file-block override in `eslint.config.mjs` via a second flat- - config entry. Zero impact on `pages/api/**` (already correctly - ESM-imported throughout) and zero impact on the root `*.config.js` - files (which are intentionally CJS-shaped and which the next-config - base rules already handle correctly). -- **All `.js` files at repo root (rejected):** broader-than-necessary - blast radius. `pages/api/**` already uses ESM `import` everywhere - (`add-rate-limiting`, `cors-tighten`, and `add-route` skill all - verified this in the last three months). A repo-wide ban would - produce zero true positives outside `scripts/**` today and would - risk breaking config-file shapes that legitimately use CJS - (`postcss.config.js`, `tailwind.config.js` are flagged by - `import/no-anonymous-default-export` today but read CJS-style - exports under the hood — see also Gotcha #9 + #10). - -The scoped rule is a 7-line flat-config block; the broader rule -would require explicit allowlist for every config file, which is -strictly more code and more maintenance. - -## The fix - -Add a new flat-config block at the end of `eslint.config.mjs` (after -the existing `globalIgnores(...)` call, NOT inside the root rules -block) targeting only `scripts/**/*.js`: - -```js -{ - files: ['scripts/**/*.js'], - rules: { - 'no-restricted-syntax': ['error', { - selector: 'CallExpression[callee.name="require"]', - message: 'Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md.', - }], - }, -}, -``` - -The error message points at `.convoys/fix-reset-db-script.md` so that -the next agent / contributor who triggers the rule gets a 1-click path -to the exemplar fix (ESM top-level imports for `dotenv`, `neon`, -`bcrypt`) instead of having to re-derive it. - -`scripts/migrations/**` is already in `globalIgnores` (from -`pick-a-name` Brief 2's migration script) and stays ignored — the -rule does not fire there even though the migration script is ESM and -correctly uses `import` (no need to re-lint files already excluded). - -The rule fires on `CallExpression[callee.name="require"]` — the AST -shape of a plain `require('foo')` call. It does NOT fire on -`createRequire(import.meta.url)` patterns (which use `Module.createRequire`) -should one ever be needed; the AST callee is `createRequire`, not -`require`. If a future helper script legitimately needs CJS interop, -the right path is `await import('foo')` (ESM dynamic import) — the -rule will not block that either. - -## Verification plan - -1. `node --check eslint.config.mjs` → exit 0 (config parses). -2. `npm run lint` → exit 1 with **128 problems (81 errors, 47 - warnings)** — verbatim match of the pre-convoy baseline (no - regression, no new false positives in the current tree). -3. **Negative test (apply, run, revert):** prepend - `const x = require('fs');` to `scripts/reset-db.js`, run - `npm run lint`, confirm exit 1 with the new rule firing at the - expected line/column and the documented message, then revert. -4. `npm run test:run` → 21/21 pass (no test surface touched; runs - only to confirm vitest is still green). -5. `rg "require\(" scripts/ --type js` → 0 hits (sanity check - confirming the current tree is clean and the rule has zero - positives to silence on day 1). - -## Risks - -- **False positives if anyone legitimately needs `require()` in - `scripts/**`.** None today (verified by step 5 — zero `require(` - hits across all helper scripts in the current tree after PR #25 - and the `drop-public-setup` B2 sweep). If a future script - legitimately needs CJS interop (e.g. a dependency that only - exports CJS without an ESM wrapper), the fix is `await - import('foo')` — ESM dynamic import works in any ESM script and - is not flagged by the rule. If that's somehow not viable, the - escape hatch is a per-line `// eslint-disable-next-line - no-restricted-syntax` with a comment explaining why ESM doesn't - work; lint baseline tracking will catch the disable directive in - review. -- **Rule scope drift.** If someone adds a new top-level scripts - directory (`tools/`, `cli/`, etc.) the rule won't fire there. Low - risk — this repo has consolidated on `scripts/` since inception - and there's no signal of a second scripts directory being added. - Tracked here so the next refactor that reshapes the helper-script - layout knows to extend the `files:` glob. -- **ESLint v10 bump.** When `bump-eslint-10` lands (currently - upstream-blocked per Gotcha #10), re-verify this rule's selector - syntax against the v10 AST behavior. `no-restricted-syntax` is a - stable core rule going back to ESLint v1; no v10 deprecation is - expected, but the smoke check is cheap. - -## Acceptance criteria - -- `node --check eslint.config.mjs` exit 0 -- `npm run lint` exit 1 with 128 problems (baseline preserved) -- `npm run test:run` 21/21 pass -- Negative test passes (rule fires on synthetic `require()` insertion, - reverts cleanly to 128 problems after the synthetic edit is - removed) -- Grep: 0 `require(` occurrences in `scripts/**/*.js` (current tree - is clean — rule starts with zero positives to silence) - -## Out of scope - -- Sweeping any other `scripts/**` file — current tree is clean - (verified by the grep step above). The rule is preventative, - not retroactive. -- Broadening the rule to all `.js` files at repo root — see - § Design decision; `pages/api/**` is already correctly ESM and - the config files (`postcss.config.js`, `tailwind.config.js`, - `next.config.js`) intentionally use CJS-style exports that the - next-config base rules handle correctly. -- Bumping any deps (ESLint stays at v9 per Gotcha #10; no - `typescript-eslint` interaction since the rule is a core rule). -- The `purge-weak-creds-from-helpers` follow-up (the - `scripts/create-test-users.js` portion remains queued; this - convoy only adds the lint rule, not the weak-creds sweep). - -## Owns - -Parent (single-file ESLint config edit; no architect or implementer -subagent required — proven-pattern follow-up to PR #25). - -## As-shipped - -_(Stub — doc-writer to fill in post-merge with squash commit SHA, -PR URL, observed lint baseline before/after, CI gate results, and -any deviations from the planned shape.)_ diff --git a/.convoys/purge-weak-creds-from-helpers.md b/.convoys/purge-weak-creds-from-helpers.md deleted file mode 100644 index 4c84ba9..0000000 --- a/.convoys/purge-weak-creds-from-helpers.md +++ /dev/null @@ -1,187 +0,0 @@ -# purge-weak-creds-from-helpers (P2 hygiene — final scope close) - -**Status:** IN-FLIGHT 2026-05-26 -**Priority:** P2 hygiene (not a security blocker; the test-fixture -script is dev-only and the documented passwords were never reachable -from a production code path — but the bug pattern is the same as the -P0-grade weak-creds shape that `drop-public-setup` removed from -`setup-neon-db.js`, so closing it brings the helper-script surface to -zero weak literals) -**Convoy owner:** parent (no architect — proven-pattern mirror; -single-script + single-doc fix following two already-shipped -applications of the same pattern) -**Opened:** 2026-05-26 -**Classification:** hygiene - -## Background — the multi-convoy history that led here - -The original `purge-weak-creds-from-helpers` convoy was queued in -`.convoys/ship-readiness.md` as the umbrella for sweeping every -helper-script + manual-QA-doc reference to the legacy `admin123` / -`alice123` / `bob123` literals and the legacy `@tcgvault.com` email -domain. Its scope has been progressively whittled down by three -already-shipped convoys: - -1. **`drop-public-setup`** (squash `ff80753` Brief 1 + `b63b509` - Brief 2): replaced the hardcoded `admin123` in - `scripts/setup-neon-db.js` with the fail-loud - `ADMIN_INITIAL_PASSWORD` env-var gate; converted the script from - CJS to ESM so `npm run setup-db` actually runs on Node 22.x. Set - the precedent for the env-var + fail-loud + no-echo pattern that - the next two convoys (and this one) mirror verbatim. -2. **`pick-a-name` Brief 2** (squash `9abbab6`, 2026-05-24): swept - every `@tcgvault.com` literal in scripts + docs to `@deckhearth.com` - together with the one-shot migration script. **Email half done.** -3. **`fix-reset-db-script`** (squash `3ab9bf8`, PR #25, 2026-05-26): - second application of the post-`drop-public-setup` pattern, this - time to `scripts/reset-db.js`. Removed the second `admin123` literal - from the codebase, removed the only remaining `Admin Password:` - echo, converted the third CJS-in-ESM script. - -After those three convoys, the remaining weak-credential surface is -exactly two files — the alice/bob test-user fixture script and the -manual-QA doc that pairs with it. Both are addressed here. - -## Remaining scope (this convoy) - -1. **`scripts/create-test-users.js`** — alice + bob fixtures still - hardcode `bcrypt.hash('alice123', 12)` + `bcrypt.hash('bob123', 12)` - and echo the literal passwords to stdout (`console.log('✅ Created - Alice (alice@deckhearth.com / alice123)')`). -2. **`TESTING_GUIDE.md`** — Test Accounts table still documents the - literal passwords for admin + alice + bob. - -## The fix shape — single env var, no echo, ESM-already - -The fix is a verbatim mirror of the post-`drop-public-setup` -`scripts/setup-neon-db.js` pattern and the post-`fix-reset-db-script` -`scripts/reset-db.js` pattern, with one deliberate simplification: - -- **Single env var: `TEST_USERS_PASSWORD`.** Both alice and bob get - the same hashed value. Per-user env vars (`ALICE_PASSWORD`, - `BOB_PASSWORD`) would be unnecessary sprawl for what is a test - fixture surface — these aren't independent identities, they're a - collaborator-flow demo pair. Risk R2 below argues this explicitly. -- **Fail-loud at the top of `createTestUsers()`.** Reads - `process.env.TEST_USERS_PASSWORD`; if unset or whitespace-only, - prints an actionable error (names the var, points at `.env.local`, - suggests `openssl rand -base64 24`, references README's "First-time - admin setup" section) and `process.exit(1)` BEFORE opening any DB - connection. Same wording template as `setup-neon-db.js` lines 20-26 - and `reset-db.js` lines 29-35. -- **No password echo to stdout.** The previous file logged the - literal `alice123` / `bob123` strings in both the per-user creation - line and the final summary block. All four echo lines are deleted; - the new summary line documents *where* the password comes from - (`(passwords from TEST_USERS_PASSWORD)`) without ever printing the - value. -- **ESM already.** Unlike `setup-neon-db.js` and `reset-db.js` at the - start of their respective convoys, `create-test-users.js` was - already top-level ESM (it imports `{ config } from 'dotenv'`, - `{ sql } from '@vercel/postgres'`, `bcrypt from 'bcryptjs'` at the - top of the file). **No CJS→ESM conversion needed.** This convoy is - the first of the three to skip that half of the pattern. -- **`ON CONFLICT (email) DO NOTHING`** is preserved (already in the - original file at lines 19, 28) — defensive against double-run. - -`TESTING_GUIDE.md`'s Test Accounts table is rewritten to (a) remove -the literal passwords from the table, (b) document the env-var source -for each user, and (c) point at README's "First-time admin setup" -section for the `openssl rand -base64 24` generation tip. The two -inline `Password: alice123` / `Password: bob123` snippets later in the -workflow are replaced with `Password: `. - -## Verification plan (static-grep only — script is destructive) - -This convoy does NOT live-test `create-test-users.js`. The script -opens a DB connection and inserts rows; running it against a Neon -branch in CI or in the boot-the-brief loop would be a side-effect -cost we don't need to incur. The verification surface is entirely -static: - -- `node --check scripts/create-test-users.js` → exit 0 -- `npm run lint` → 128 problems (baseline preserved; no regression) -- `npm run test:run` → 21/21 pass -- Grep `scripts/ TESTING_GUIDE.md` for `admin123|password123|test123| - alice123|bob123` → 0 hits on literal passwords -- Grep `scripts/create-test-users.js` for `require(` → 0 hits - (mirror-the-pattern preserves ESM-only; trivially satisfied here - because the file was already ESM) -- Grep `scripts/ TESTING_GUIDE.md` for `TEST_USERS_PASSWORD` → - expect 10 hits (5 in script: docstring + const + error message - body; 5 in TESTING_GUIDE.md: table + two inline workflow snippets + - explanatory paragraph) - -**Live verification deferred to operator.** Optional post-merge -action: set `TEST_USERS_PASSWORD` in `.env.local`, run -`node scripts/create-test-users.js` against a non-prod Neon branch, -verify alice + bob rows insert; then unset the env var and re-run, -verify the script exits 1 with the helpful error message before -opening the DB connection. - -## Risks - -- **R1 — A CI step or doc dep on the literal passwords.** If - `.github/workflows/**` or any other doc (`docs/**`, `TESTING_GUIDE.md` - sibling files, the agent-context-pipeline docs) references - `alice123` / `bob123` / `admin123` as part of an automated test - flow, removing the literal would break it. **Mitigation:** the grep - hunt covered `scripts/` + `TESTING_GUIDE.md`. The broader hits in - `.convoys/**` and `AGENTS.md` are historical convoy narrative and - must NOT be edited (rewriting history). The only live reference - outside this convoy's scope is `pages/login.js` — see "Surfaced - out-of-scope follow-up" below. -- **R2 — Env-var sprawl.** Using a single `TEST_USERS_PASSWORD` for - both alice and bob is intentional. These are test-fixture users - for the collaboration demo flow in TESTING_GUIDE.md; they aren't - modeled as independent identities anywhere in the auth surface, and - giving them per-user passwords would (a) double the env-var - contract for zero security benefit (anyone running this script - already has full DB access) and (b) drift from the - ADMIN_INITIAL_PASSWORD shape that the operator is already trained - on. If future test-user additions need distinct passwords for - realistic concurrency testing, that's a separate concern and a - separate convoy. - -## Surfaced out-of-scope follow-up - -- **`pages/login.js` "Quick Login" buttons still hardcode the legacy - literals.** Lines 172 + 184 invoke - `handleQuickLogin('alice@deckhearth.com', 'alice123')` and - `handleQuickLogin('bob@deckhearth.com', 'bob123')`. These are - client-side dev convenience buttons that ship to production HTML - and reveal the legacy passwords directly to anyone viewing the - login page source. **NOT in scope for this convoy** (the convoy - spec is "scripts + docs only; do NOT touch `pages/**`"). Queue a - follow-up convoy: `purge-quick-login-from-loginpage` (P2 hygiene) - to either (a) delete the Quick Login section entirely or - (b) gate it behind `process.env.NODE_ENV === 'development'`. The - latter still requires a credential source that doesn't ship to - prod HTML — likely a `.env.local`-only `NEXT_PUBLIC_DEV_*` - convention or a dev-only proxy endpoint. Architect-worth. - -## Operator action required - -- **Pre-merge:** none. No schema change. No new infra. -- **Post-merge:** anyone running `node scripts/create-test-users.js` - (or `npm run create-test-users` if such a script exists) must add - `TEST_USERS_PASSWORD=` to their `.env.local` first. - Existing alice + bob rows in already-seeded environments are - **not** rotated by re-running this script — `ON CONFLICT (email) - DO NOTHING` preserves the old hashes. Any environment that ran - `create-test-users.js` before this convoy still has the weak - `alice123` / `bob123` hashes in its DB; operators must rotate - manually via the app (or drop those rows and re-seed). Same - caveat that applies to the `drop-public-setup` admin row — - `setup-neon-db.js`'s and `create-test-users.js`'s idempotency - means they do NOT rotate; they only seed. - -## Owns - -Parent (single-script + single-doc proven-pattern fix; no architect -or implementer subagent required). - -## As-shipped - -(stub — fill in post-merge) diff --git a/.convoys/single-auth-provider.md b/.convoys/single-auth-provider.md new file mode 100644 index 0000000..a2b0b3d --- /dev/null +++ b/.convoys/single-auth-provider.md @@ -0,0 +1,317 @@ +# single-auth-provider (P1 quality — collapse three client auth surfaces onto one) + +**Status:** OPEN 2026-05-26 (this convoy) +**Priority:** P1 quality (launch sequence step 9 — `.convoys/ship-readiness.md` § P1 entry 9) +**Convoy owner:** parent (architect + implementer rolled together — diff is mechanical once the +shape-parity decision is made) +**Branch:** `convoy/single-auth-provider` +**Opened:** 2026-05-26 + +## Background + +The repo carried **three parallel client-side auth implementations** since the early days of the +project. `.convoys/ship-readiness.md` § P1 entry 9 ("Three parallel client-side auth +implementations") is the canonical spec; AGENTS.md § 3 already documented `lib/use-auth.js` as the +canonical surface and instructed new code to avoid the other two. This convoy executes the +collapse. + +The three surfaces: + +1. **`lib/use-auth.js::useAuth`** (the keeper). Hook-only — reads `auth_token` from + `localStorage` on mount, hits `/api/auth/verify`, exposes `{ user, loading, logout, refreshAuth }`. + No React context, no `` wrapper required. +2. **`lib/auth-context.js::{ AuthProvider, useAuth }`** (legacy). Context provider + consumer + hook with the same verify-on-mount semantics, plus `login()` and `register()` helpers that + `pages/login.js` / `pages/signup.js` no longer use (those pages call `/api/auth/{login,register}` + directly and write the token to `localStorage` themselves). Wired in `pages/_app.js` as + ``. +3. **`lib/admin-auth.js::{ AdminProvider, useAdmin, useIsAdmin }`** (legacy). A redundant context + that does the *same* verify-on-mount roundtrip, plus a hook-only `useIsAdmin()` that does its + own verify roundtrip on top of that. `AdminProvider` is **not** wired in `_app.js` (verified + by reading `_app.js` pre-convoy: only `` + ``), so `useAdmin()` + would have thrown at runtime if anyone called it — nobody does. Only `useIsAdmin()` has a + live consumer (`pages/card/[id].js`). + +**Symptom that drove ranking this P1.** When `pages/card/[id].js` mounts, it calls +`useAuth()` from `lib/use-auth.js` AND `useIsAdmin()` from `lib/admin-auth.js`, each issuing its +own `GET /api/auth/verify`. With `AuthProvider` mounted on every page via `_app.js`, that's a +**third** verify roundtrip on the very first page load. Three roundtrips, identical request, +serial cost on a cold connection. Post-convoy: 1 roundtrip per page-load. + +## Decisions + +### D1 — Shape parity check on `lib/use-auth.js`. Verdict: no parity gap; do **not** extend. + +`lib/auth-context.js::useAuth()` exposed `{ user, loading, login, register, logout }`. +`lib/use-auth.js::useAuth()` exposes `{ user, loading, logout, refreshAuth }`. + +The apparent gap is `login` / `register`. Verified-by-grep: **zero call sites** invoke +`useAuth().login(…)` or `useAuth().register(…)` anywhere in `pages/**` or `components/**`. The +only callers of those flows are `pages/login.js` and `pages/signup.js`, both of which `fetch` +`/api/auth/{login,register}` directly and write the returned token to `localStorage`. +`useAuth()`'s `useEffect` then picks up the new token on the next mount (or the page can call +`refreshAuth()` to re-verify in place). + +Conclusion: do **not** add `login` / `register` to `use-auth.js`. The legacy methods were dead +code on the consumer surface; preserving them would be cargo-culting and would re-create a +non-DRY login flow (one in `pages/login.js`, one in the hook). `auth-and-permissions.mdc` § +"Authentication state on the client" was updated to document the post-convoy `useAuth()` shape +and to spell out the `login.js` / `signup.js` direct-fetch pattern. + +### D2 — `useIsAdmin()` migration shape. Verdict: collapse onto the existing `useAuth()` call. + +`pages/card/[id].js` is the **only** consumer of `useIsAdmin()`. The page already called +`useAuth()` from `lib/use-auth.js` at line 13 (added by `fix-layout-default-user` Brief 2). The +migration is: + +```js +// Before +const { user } = useAuth(); +// ... +const { isAdmin, loading: adminLoading } = useIsAdmin(); +// ... usage at line 524: {isAdmin && !adminLoading && (...)} + +// After +const { user, loading: authLoading } = useAuth(); +// ... +const isAdmin = user?.role === 'admin'; +const adminLoading = authLoading; +// ... usage at line 524 unchanged: {isAdmin && !adminLoading && (...)} +``` + +`adminLoading` is kept as a local alias rather than substituting `authLoading` directly at the +call site, to keep the diff minimal and the rendering condition byte-identical. The `loading` +window from `useAuth()` covers exactly the same period (`/api/auth/verify` resolution) that +`useIsAdmin`'s own loading covered, so there is no UX regression. + +### D3 — `pages/_app.js` provider tree. Before / after. + +```jsx +// Before + + + + + + +// After + + + +``` + +`useAuth()` from `lib/use-auth.js` is hook-only — no Provider needed. The `` +wrapper is removed entirely; no replacement Provider is added. `` stays (out of +scope). `` was never in the tree to begin with. + +### D4 — Token-verify roundtrip count. + +Per the spec: pre-convoy a worst-case page mount issued **3** identical `GET /api/auth/verify` +requests: + +1. `` in `_app.js` calls `verifyToken()` on mount. +2. `pages/card/[id].js` calls `useAuth()` from `lib/use-auth.js`, which calls `checkAuth()` on + mount → another verify. +3. The same page calls `useIsAdmin()` from `lib/admin-auth.js`, which calls its inline + `checkAdmin()` on mount → another verify. + +Post-convoy: + +1. `` is gone. +2. `pages/card/[id].js` calls `useAuth()` once → 1 verify. +3. `useIsAdmin()` call site is gone; admin status is computed synchronously from the same + `user` returned by step 2. + +Net: **3 → 1** verify roundtrip on `card/[id].js` mount. Other pages drop from **2 → 1** +(no `useIsAdmin` involved, but `` was). The 1× pattern is the floor; further +reduction would require server-side hydration of the user object, which is a separate +architectural conversation (out of scope; see Follow-ups). + +### D5 — Test impact. Verdict: zero test files modified. + +The 21-test vitest suite covers: + +- `test/lib/auth-secret.test.js` (3) — server-side, untouched by this convoy. +- `test/lib/permission-middleware.test.js` (8) — server-side, untouched. +- `test/api/auth-utils.test.js` (5) — server-side, untouched. +- `test/components/Layout.test.js` (5) — passes `user` as a *prop*, not via any hook. The + legacy `auth-context` and `admin-auth` modules are not imported. Unaffected. + +All four files were `grep`-checked for `auth-context|admin-auth|use-auth` references — zero +hits. No test was written against the legacy hooks themselves; the deletion is risk-free from a +test-suite perspective. Vitest stays green at 21/21 post-convoy. + +## Importer inventory + +Generated via `rg "from ['\"].*lib/auth-context['\"]" --type js` and +`rg "from ['\"].*lib/admin-auth['\"]" --type js` against the worktree (excluding docs / convoys). + +### Importers of `lib/auth-context.js` (6 source files) + +| File | Symbol | Migration | +| --- | --- | --- | +| `pages/_app.js` | `AuthProvider` | Wrapper removed; no replacement (D3) | +| `pages/index.js` | `useAuth` | Path swap → `lib/use-auth.js` | +| `pages/scanner.js` | `useAuth` | Path swap → `lib/use-auth` | +| `pages/decks.js` | `useAuth` | Path swap → `lib/use-auth` | +| `pages/deck/[id].js` | `useAuth` | Path swap → `lib/use-auth` (depth `../../`) | +| `pages/deck-builder.js` | `useAuth` | Path swap → `lib/use-auth` | + +All 5 page-level `useAuth` consumers destructured only `{ user }` or `{ user, loading }` (verified +by grep). No `login` / `register` / other-method consumer found, confirming D1. + +### Importers of `lib/admin-auth.js` (1 source file) + +| File | Symbol | Migration | +| --- | --- | --- | +| `pages/card/[id].js` | `useIsAdmin` | Replaced with `user?.role === 'admin'` from existing `useAuth()` (D2) | + +`AdminProvider` and `useAdmin()` had **zero** importers in the source tree — confirming +they were dead exports. + +### Adjacent doc / config edits + +| File | Change | +| --- | --- | +| `pages/_app.js` | Removed `import { AuthProvider } from '../lib/auth-context.js'` and the wrapper | +| `.github/CODEOWNERS` | Removed the two CODEOWNERS lines for the deleted files | +| `AGENTS.md` § 2 + § 3 | Updated the Auth row of the architecture table and the "Auth (client)" convention bullet to describe the post-convoy single-surface state | +| `.cursor/rules/auth-and-permissions.mdc` | Reframed § "Legacy" to "deleted by this convoy"; updated § "Authentication state on the client" to the post-convoy `useAuth()` shape and the direct-fetch login flow | +| `.cursor/rules/no-go-zones.mdc` | Auth-refactors bullet updated to drop the deleted files | +| `.cursor/skills/add-page/SKILL.md` | Updated checklist bullet + anti-pattern row to refer to the deletion | + +`.convoys/**` and `.convoys/fix-layout-default-user/**` were **not** edited — those are +historical convoy records and are append-only by repo convention. The doc-writer post-convoy +sweep will add the as-shipped section at the bottom of this file plus update +`.convoys/ship-readiness.md` § P1 → entry 9 with the squash commit reference. + +## The fix (per-category translation rules) + +### Category A — `useAuth` from `auth-context` → `useAuth` from `use-auth` + +```js +// before +import { useAuth } from '../lib/auth-context'; // or auth-context.js +// after +import { useAuth } from '../lib/use-auth'; // or use-auth.js +``` + +The destructure pattern (`const { user } = useAuth()` / `const { user, loading } = useAuth()`) +stays byte-identical. No call-site changes. + +### Category B — `AuthProvider` wrapper in `_app.js` + +```jsx +// before +import { AuthProvider } from '../lib/auth-context.js'; +return ( + + + + + +); + +// after +return ( + + + +); +``` + +Plus delete the import line. + +### Category C — `useIsAdmin` in `pages/card/[id].js` + +See D2 for the full diff. Three line-ranges touched: the import block, the `useAuth` destructure, +and the `useIsAdmin` line block. Usage at line 524 is unchanged. + +### Category D — `useAdmin`, `AdminProvider` + +No call sites. No work to do; these symbols disappear when the file is deleted. + +## Verification plan + +1. **`rg "lib/auth-context|lib/admin-auth" --type js`** → expect zero hits in `pages/`, `lib/`, + `components/`. Achieved. +2. **`npm run lint`** → baseline 128 problems pre-convoy → 125 problems post-convoy (3 fewer + errors, since the deleted files contained 3 unused-import / unused-var lints; no new lint + surface introduced). No regression. +3. **`npm run test:run`** → 21/21 pass pre- and post-convoy. Layout test confirmed unaffected. +4. **`npm run build`** → succeeds end-to-end. All 26 pages compile (10 dynamic API routes + 16 + `pages/**` views including `card/[id]`, `_app`, `decks`, `deck/[id]`, `deck-builder`, `scanner`, + `index` — every file modified by the sweep). No SSR-level breakage; importantly no + "useAuth must be used within an AuthProvider" runtime error during static generation, which + would have indicated the page tried to use the legacy context hook unwrapped. +5. **Manual smoke:** _deferred_ — the build pass + vitest pass + zero-hit grep is the gate for + merging; the parent does not have a logged-in admin browser session ready in this + conversation. Documenting in As-shipped post-merge once the operator runs `npm run dev` and + exercises dashboard / profile / settings / collections / cards / admin/card-editor. + +## Risks + +- **R1 — Shape parity gap breaks runtime auth state.** *Mitigated by D1.* The grep audit + confirmed no consumer reads `login` / `register` / any other surface that exists on the + legacy hook but not on `use-auth`. `loading` and `user` were preserved with identical + semantics. +- **R2 — SSR mismatch from removing ``.** *Mitigated.* `lib/use-auth.js` reads + `localStorage` inside a `useEffect`, so SSR sees `user === null, loading === true` and never + touches the browser-only API on the server — same guarded shape as the legacy provider. + `npm run build` confirms no SSR error during static generation. (`auth-context.js`'s + `useEffect` had the same guard, so removing the provider didn't change the SSR surface.) +- **R3 — Missed importer.** *Mitigated.* Post-delete grep over `--type js` returned zero hits. + The deletion would itself surface any missed importer at module-load time during `npm run + build` (Node would throw "Cannot find module"); build succeeded. +- **R4 — Verify-roundtrip dedup creates a regression where a page never re-verifies.** + *Mitigated.* Pre-convoy, three providers each ran their own verify on mount but they did not + coordinate state — one provider's success had no effect on another's loading flag. Post-convoy + we have a single source of truth. Pages that need to re-verify (e.g. after an action that + might have invalidated the token) can call `refreshAuth()` from the same hook; no consumer + currently does this, but the surface is preserved for future use. +- **R5 — Stale `useAuth` cache across components.** *Out of scope; see Follow-ups.* Each + `useAuth()` call site instantiates its own state via `useState`. Two components on the same + page that both call `useAuth` will issue two verify roundtrips and hold two independent + `user` references. This was true pre-convoy too (the legacy `useIsAdmin` was already a + separate verify). Hoisting state into a shared module-level cache or wrapping `useAuth` in a + context (the very thing we just removed!) is a separate decision — see "Follow-ups". + +## As-shipped + +_Stub for doc-writer post-merge:_ + +- Squash commit: `` +- PR: #`` +- Files changed: 13 (2 deletions: `lib/auth-context.js`, `lib/admin-auth.js`; 11 modifications: + `pages/_app.js`, `pages/index.js`, `pages/scanner.js`, `pages/decks.js`, `pages/deck/[id].js`, + `pages/deck-builder.js`, `pages/card/[id].js`, `.github/CODEOWNERS`, `AGENTS.md`, + `.cursor/rules/auth-and-permissions.mdc`, `.cursor/rules/no-go-zones.mdc`, + `.cursor/skills/add-page/SKILL.md`). +- Verify roundtrip count: documented 3 → 1 on `card/[id].js`, 2 → 1 on every other page-load. +- `.convoys/ship-readiness.md` § P1 entry 9 to be marked RESOLVED with this convoy's squash SHA. +- Lint baseline updated 128 → 125 (no regression; 3 fewer errors from deleted unused-import + lines). + +## Follow-ups (out of scope here) + +- **Component-level `useAuth` cache audit.** Two components on the same page that both call + `useAuth()` will issue two verify roundtrips. This was the original motivation for the + legacy context, and was the *one* legitimate thing those providers did right. A future + convoy should consider either (a) returning a shared module-level state via a small + Zustand-style store, (b) reintroducing a thin `` that *only* hoists state + without re-implementing fetch logic, or (c) accepting the duplicate roundtrip as the price of + hook-only simplicity. Today's call sites already deduplicate at the page level (one + `useAuth` per page is the prevailing pattern), so this is a soft optimisation, not a + correctness fix. +- **Rate-limit-aware re-auth on 429.** `lib/use-auth.js`'s `checkAuth` does not currently + back off if `/api/auth/verify` returns 429 (the rate-limiter from + `add-rate-limiting` would only kick in if a single client exceeded + 60 verify calls / minute, which is unrealistic in practice but worth a defensive guard). +- **Server-side hydration of user.** The page-mount verify roundtrip is unavoidable in this + hook-only shape because the token is only readable on the client. Moving to an HTTP-only + cookie + Next.js `getServerSideProps` hydration would eliminate the round-trip entirely + and is a larger architectural conversation that should not piggyback on a quality convoy. +- **Doc-writer cleanup.** Update `.convoys/ship-readiness.md` § P1 → entry 9 with the + RESOLVED stamp + squash SHA; trim the "three parallel surfaces" framing from any other + doc that still mentions it; refresh the "Auth refactors" no-go-zones bullet if any other + files become canonical (none today). diff --git a/.convoys/single-sql-client.md b/.convoys/single-sql-client.md deleted file mode 100644 index c5c97b5..0000000 --- a/.convoys/single-sql-client.md +++ /dev/null @@ -1,434 +0,0 @@ ---- -name: single-sql-client -classification: quality -success_metric: | - `lib/database.js` is deleted; every former caller uses - `@vercel/postgres` tagged templates with identical query - semantics; vitest 21/21 stays green; lint baseline (128 - problems) is preserved; no remaining `lib/database` import - appears anywhere under `pages/` or `lib/`. AGENTS.md Gotcha #1 - / `.convoys/ship-readiness.md` P1 #8 flip → RESOLVED. -skip: - - role-design-system-auditor - - role-a11y-auditor - - role-ux-reviewer - - role-ia-architect -status: open -created: 2026-05-26 -parent: ship-readiness -addresses: P1 #8 (launch sequence step 8) — "Two SQL clients in parallel" -depends_on: [] ---- - -# Convoy: single-sql-client - -Collapse the dual SQL-client problem documented in AGENTS.md -Gotcha #1 by deleting `lib/database.js` and migrating its sole -production caller (`pages/api/auth-utils.js`) onto the canonical -`@vercel/postgres` tagged-template surface. Keep -`@neondatabase/serverless` as a runtime dependency for the -out-of-scope `scripts/**` helpers that already use `neon()` -directly. - -## Background - -`AGENTS.md` Gotcha #1 (and the `.cursor/rules/db-and-schema.mdc` -"Clients" section) document a long-standing dual-client problem: - -- `@neondatabase/serverless` is wrapped by `lib/database.js`'s - `DatabaseAdapter`, which exposes a `db.query(sqlString, - params)` API that **manually interpolates `$1, $2, ...` - placeholders into the SQL string** and then calls - `sql.unsafe(query)` on the result. -- `@vercel/postgres` is used directly by ~30 `pages/api/**` - handlers via tagged-template SQL (`` await sql`SELECT … WHERE - id = ${id}` ``), which is parameterized natively by the driver. - -The manual-interpolation shape in `lib/database.js` is the -documented foot-gun: - -> *Avoid `lib/database.js`'s `db.query(string, params)` API for -> new code — it interpolates params into a string and then -> calls `sql.unsafe()`, which is a SQL-injection vector.* -> — `.cursor/rules/db-and-schema.mdc` § Clients - -The `.convoys/ship-readiness.md` P1 #8 entry tracks the cleanup -as launch sequence step 8 ("touches ~3 files based on graph"). - -## Decisions - -### D1 — Caller inventory: 2 files in scope (1 source + 1 test), not "~3" - -Architect ran `Grep` against the workspace for `lib/database` -imports. Only **2** files touch the module: - -| File | Kind | What it does with `db` | -| --- | --- | --- | -| `pages/api/auth-utils.js` | source | imports `db`, calls `db.query` in `isAdmin(userId)` + `getUserById(userId)` | -| `test/api/auth-utils.test.js` | test | `vi.mock('../../lib/database.js', () => ({ db: { query: vi.fn() } }))` — mock-only; the 5 tests exercise `generateToken` / `verifyToken`, not `isAdmin` / `getUserById`. The mock exists purely to satisfy the import graph | - -The README/AGENTS-quoted "~3 files based on graph" estimate from -ship-readiness P1 #8 was loose. Confirmed exact count = 2. No -deviation flag needed; the convoy is even smaller than estimated. - -### D2 — `@vercel/postgres` tagged templates, byte-equivalent SQL - -Two call sites in `pages/api/auth-utils.js`: - -**Before:** - -```js -import { db } from '../../lib/database.js'; - -export async function isAdmin(userId) { - try { - const result = await db.query(` - SELECT role FROM users WHERE id = $1 - `, [userId]); - return result.rows[0]?.role === 'admin'; - } catch (error) { ... } -} - -export async function getUserById(userId) { - try { - const result = await db.query(` - SELECT id, email, role, created_at FROM users WHERE id = $1 - `, [userId]); - return result.rows[0]; - } catch (error) { ... } -} -``` - -**After:** - -```js -import { sql } from '@vercel/postgres'; - -export async function isAdmin(userId) { - try { - const result = await sql` - SELECT role FROM users WHERE id = ${userId} - `; - return result.rows[0]?.role === 'admin'; - } catch (error) { ... } -} - -export async function getUserById(userId) { - try { - const result = await sql` - SELECT id, email, role, created_at FROM users WHERE id = ${userId} - `; - return result.rows[0]; - } catch (error) { ... } -} -``` - -The two queries are SELECT-only, single-table, single-parameter -(numeric `userId`). The translated tagged-template form produces -**byte-equivalent SQL** with proper parameter binding (instead -of string interpolation + `sql.unsafe`). The `result.rows[0]` -access pattern works identically — `@vercel/postgres` returns -`{ rows, rowCount }` natively, which matches the shape -`DatabaseAdapter.query` was already returning. No -result-handling change needed at the call sites in `isAdmin` / -`getUserById`. No transaction / pool semantics change either: -neither `lib/database.js` nor `@vercel/postgres` uses a -long-lived pool from these call sites (both create a per-request -HTTP connection against Neon's serverless endpoint). - -### D3 — Keep `@neondatabase/serverless` as a dep; SCOPE = `lib/database.js` collapse only - -The convoy seed flagged this explicitly. Architect verified: - -- `@neondatabase/serverless` is imported by **11 files** outside - `lib/database.js`: - - `scripts/setup-neon-db.js` (the DDL bootstrap) - - `scripts/migrations/2026-05-24-rename-admin-email.js` (post-`pick-a-name` admin rename) - - `scripts/reset-db.js` (dev reset) - - 8 other one-off `scripts/add-*.js` / `scripts/fix-*.js` / `scripts/seed-*.js` historical jobs -- None of those go through `lib/database.js`; they all use - `neon(POSTGRES_URL)` directly with tagged-template SQL (which - is the safe shape that `lib/database.js`'s wrapper subverts). - -**Scope decision: keep `@neondatabase/serverless` in -`package.json`.** This convoy is about deleting the -`lib/database.js` abstraction, not about eliminating -`@neondatabase/serverless` from the dependency tree. The -no-go-zones rule explicitly forbids touching -`scripts/setup-neon-db.js`, `scripts/migrations/*`, and the -`scripts/add-*` / `scripts/fix-*` / `scripts/seed-*` historical -jobs. Migrating those to `@vercel/postgres` would also be the -wrong call architecturally — `@vercel/postgres` is tuned for -Vercel's edge / serverless runtime; one-off scripts run from a -developer laptop or CI runner where `@neondatabase/serverless`'s -direct `neon()` shape is more appropriate. - -A future convoy could (a) migrate the scripts to a uniform -client OR (b) extract a thin `scripts/lib/db.js` that wraps -`neon()` once. Either is its own scope; flagged in § Follow-ups. - -### D4 — `sql.unsafe` audit: NOT a real injection vector with current callers (security finding: NO) - -The `lib/database.js` shape is **unsafe-by-default**: it -interpolates raw values into a SQL string and calls -`sql.unsafe()`. In theory, that's a SQL-injection vector for -any caller that passes user-controlled string input. - -Architect audit of the **2 current call sites**: - -- `isAdmin(userId)` — `userId` is sourced from a verified JWT - payload (`decoded.userId` after `verifyToken(token)` succeeds - in `pages/api/admin/index.js`). It's a number from - `users.id` (SERIAL). The `lib/database.js` interpolation path - for numbers is `result = await sql\`${sql.unsafe(query)}\`` - with the number inlined raw — for a numeric SERIAL id, no - injection vector exists in practice. -- `getUserById(userId)` — not currently called by any handler - (`Grep` for `getUserById` returns only the definition site + - the public-API comment in AGENTS.md). Same `userId` shape - semantics apply if it were called. - -**Conclusion: NO real security finding.** This is a pure -refactor + foot-gun-removal convoy. The next convoy that adds a -caller passing user-controlled string input to `db.query` would -have been the security incident; deleting the unsafe surface -prevents that future incident. - -If a future audit surfaces a `lib/database.js`-shaped wrapper -re-introduced in another lib (e.g. `lib/db-helper.js`), this -convoy's lesson is: kill it on sight. See § Follow-ups for a -queued ESLint rule that would catch a `sql.unsafe` re-introduction. - -### D5 — Tests: 1 test file touched, no semantic change - -`test/api/auth-utils.test.js` already mocks `lib/database.js` -purely to satisfy the import graph; the 5 tests exercise -`generateToken` + `verifyToken`, neither of which touches the -DB. **Post-migration:** the mock is no longer needed because -`auth-utils.js` no longer imports from `lib/database.js`. Drop -the `vi.mock('../../lib/database.js', ...)` call and the now-unused -`vi` import. Test count + assertions unchanged: 5/5. - -The other 16 vitest tests (`lib/auth-secret.test.js`, -`lib/permission-middleware.test.js`, -`components/Layout.test.js`) don't touch `lib/database.js`. No -mock drift risk; nothing else to update. - -`@vercel/postgres` does NOT get a new mock in this convoy -(deferred to queued `fill-vitest-handler-coverage`); the -`isAdmin` / `getUserById` functions are still untested at the -unit level, same as pre-migration. The migration is purely a -client swap, not a coverage expansion. - -## Caller inventory (verbatim before/after) - -### `pages/api/auth-utils.js` (only source caller) - -**Imports (before):** -```js -import jwt from 'jsonwebtoken'; -import { db } from '../../lib/database.js'; -import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'; -``` - -**Imports (after):** -```js -import jwt from 'jsonwebtoken'; -import { sql } from '@vercel/postgres'; -import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'; -``` - -**`isAdmin` body (before):** -```js -const result = await db.query(` - SELECT role FROM users WHERE id = $1 -`, [userId]); -``` - -**`isAdmin` body (after):** -```js -const result = await sql` - SELECT role FROM users WHERE id = ${userId} -`; -``` - -**`getUserById` body (before):** -```js -const result = await db.query(` - SELECT id, email, role, created_at FROM users WHERE id = $1 -`, [userId]); -``` - -**`getUserById` body (after):** -```js -const result = await sql` - SELECT id, email, role, created_at FROM users WHERE id = ${userId} -`; -``` - -The `try/catch` shape, the `result.rows[0]` access, the -`?.role === 'admin'` check, and the `console.error` + `return -false`/`return null` error paths are all preserved verbatim. - -### `test/api/auth-utils.test.js` (mock cleanup) - -**Imports (before):** -```js -import { describe, expect, it, vi } from 'vitest'; -import jwt from 'jsonwebtoken'; - -vi.mock('../../lib/database.js', () => ({ - db: { query: vi.fn() }, -})); - -import { JWT_SECRET } from '../../lib/auth-secret.js'; -import { generateToken, verifyToken } from '../../pages/api/auth-utils.js'; -``` - -**Imports (after):** -```js -import { describe, expect, it } from 'vitest'; -import jwt from 'jsonwebtoken'; - -import { JWT_SECRET } from '../../lib/auth-secret.js'; -import { generateToken, verifyToken } from '../../pages/api/auth-utils.js'; -``` - -The 5 test bodies (2 `generateToken` + 3 `verifyToken`) are -unchanged. - -### `lib/database.js` (deleted) - -47-line file. No replacement; the abstraction is gone. The two -callers go straight to `@vercel/postgres` tagged-template SQL, -matching the canonical pattern used by ~30 other `pages/api/**` -handlers already in the tree. - -## The fix (per-file diff shape) - -| File | Change | Lines | -| --- | --- | --- | -| `pages/api/auth-utils.js` | swap `db.query(string, params)` → `sql\`…${userId}\`` in 2 functions; swap import | +5 / -7 | -| `test/api/auth-utils.test.js` | drop `vi.mock('../../lib/database.js', ...)` + unused `vi` import | +1 / -5 | -| `lib/database.js` | DELETE | 0 / -47 | -| `.convoys/single-sql-client.md` | NEW (this file) | +330 / 0 | - -Total: 3 modified files (1 source + 1 test + 1 deletion) + 1 -new convoy doc. - -## Verification plan - -1. `npm run lint` → 128 problems (baseline preserved, no regression) -2. `npm run test:run` → 21/21 pass -3. `Grep "lib/database" --type js -l` → 0 hits anywhere - (no `pages/**`, no `lib/**`, no `scripts/**`, no `test/**`) -4. `Grep "@neondatabase/serverless" --type js -l` → still matches - `scripts/setup-neon-db.js`, `scripts/migrations/2026-05-24-rename-admin-email.js`, - `scripts/reset-db.js`, and the 8 other `scripts/add-*` / `fix-*` / - `seed-*` historical helpers (all out of scope per § D3) -5. `node --check pages/api/auth-utils.js` → exit 0 (syntax) - -**Live runtime smoke deferred.** The two migrated functions -(`isAdmin`, `getUserById`) are only reachable via -`pages/api/admin/index.js` which requires an admin Bearer token -+ a populated `users` table in prod Neon. Running a live curl -smoke against a local `npm run dev` would require seeding the -admin user with a known password (which `setup-neon-db.js` -now requires `ADMIN_INITIAL_PASSWORD` to do — operator-only -flow) and minting a token. Out-of-band for a convoy -that's a pure client swap; the byte-equivalent SQL semantics -(D2) plus the same `try/catch` + `result.rows[0]` access shape -gives high confidence that the migration is correct. If the -post-merge Vercel preview's admin surface 500s on an admin -action, the rollback is a single-commit revert of this convoy. - -## Risks - -- **R1 — Byte-equivalence not guaranteed if `lib/database.js` - has hidden behavior.** Architect re-read all 47 lines of - `lib/database.js` (it's a small file). The only behavior - beyond "interpolate, run SQL, return `{ rows, rowCount }`" is - the `escapedParams.map` quoting for string params — and both - current callers pass numeric `userId`, so the quoting path - isn't exercised. The `raw(sqlString, params)` method is just - an alias for `query(sqlString, params)`; no caller invokes - `raw` (`Grep "\.raw\(" --type js` → zero hits). **Residual - risk: very low.** The migration's byte-equivalent SQL plus - identical result shape (`{ rows: [...], rowCount: N }`) - closes this risk in practice. - -- **R2 — Missed callers.** Mitigated by the post-delete `Grep` - sweep in the verification plan (step 3) — if any file still - imports `lib/database`, the file no longer exists and the - import will throw at module load, failing CI's lint or test - job. The `Grep` sweep also catches `require('../../lib/database')` - CJS shape (zero hits across the tree at architect time; - expected since the repo is `"type": "module"`). - -- **R3 — A future PR re-introduces `lib/database.js` or - another `sql.unsafe`-shaped wrapper.** Mitigated by: - documentation (this convoy file + AGENTS.md Gotcha #1 - doc-writer flip → RESOLVED in a follow-up cleanup pass). - Stronger mitigation would be an ESLint - `no-restricted-imports` rule against `lib/database` or a - `no-restricted-syntax` rule against `sql.unsafe(` — surfaced - as a follow-up below (`lint-against-lib-database`). - -## As-shipped - -*Stub for doc-writer.* Populate after merge with: -- Squash commit SHA + PR number -- Diff stats (file count, line counts) -- CI gate outcomes (lint, vitest, smoke, forbidden-* gates) -- Any operator-action follow-ups -- Cross-validation findings (e.g. smoke continuing to pass) -- Updates needed to AGENTS.md Gotcha #1 + `.cursor/rules/db-and-schema.mdc` § Clients + ship-readiness P1 #8 entry - -## Follow-ups - -- **`lint-against-lib-database`** (priority: P3 polish). Add an - ESLint `no-restricted-imports` rule against `'../**/lib/database'` - (or any path resolving to a `lib/database.js` file), so a future - PR that re-introduces the unsafe wrapper fails at lint time - rather than at runtime. Pairs naturally with the queued - `lint-against-cjs-in-esm-scripts` follow-up (both are - static-source guards added to `eslint.config.mjs`). Small - surface (one entry in the config); could fold into a - `harden-eslint-static-guards` convoy if more such guards - accumulate. - -- **`purge-neondatabase-serverless-fully`** (priority: P3 polish; - blocked on `migration-tool`). Once the `migration-tool` convoy - (P1 #11) lands and replaces the ad-hoc `scripts/add-*` / - `scripts/fix-*` / `scripts/seed-*` shape with a real migration - framework, revisit whether the remaining 11 - `@neondatabase/serverless` import sites can be collapsed onto - `@vercel/postgres`. Caveat: `@vercel/postgres` is tuned for the - Vercel edge runtime and may not be the right choice for - developer-laptop / CI-runner scripts; the right answer may be - "keep the dep, but route all scripts through a single thin - helper" rather than "delete the dep entirely". This is its own - scope; do NOT bolt onto this convoy. - -- **`add-neon-return-shape-rule`** (priority: P3 polish; surfaced - 2026-05-25 in PR #24 + restated in `.convoys/fix-reset-db-script.md` - § Out of scope). Codify the `neon()` (returns `[rows]`) vs - `@vercel/postgres` (`{ rows: [...] }`) return-shape difference - as a `.cursor/rules/db-and-schema.mdc` callout. **Partially - satisfied** by this convoy because the dual-client shape is now - collapsed for `pages/api/**` — only the 11 `scripts/**` callers - still use `neon()` directly, and they all destructure the - array return shape correctly today. The rule would defend - future contributors who don't already know the difference; queue - if a third such bug surfaces. - -## Owns - -- Architect (this convoy file + decisions D1-D5). -- Implementer (mechanical: 2 source-file edits + 1 deletion + - verification gates). Possibly the same agent — the - implementation surface is small enough that the cost of a - fresh implementer subagent boot may exceed the cost of doing - the work in the architect's own turn. Documented per - `fix-reset-db-script` precedent ("Why no architect" / "Why - no implementer" — single-file proven-pattern work). diff --git a/.convoys/tighten-visual-diff-path-filter.md b/.convoys/tighten-visual-diff-path-filter.md deleted file mode 100644 index 7c0796e..0000000 --- a/.convoys/tighten-visual-diff-path-filter.md +++ /dev/null @@ -1,270 +0,0 @@ -# tighten-visual-diff-path-filter (P3 polish — single-file YAML tweak) - -**Status:** OPEN 2026-05-26 (this PR) -**Priority:** P3 polish (CI-cost / signal-noise; not a security or -correctness issue — `Screenshot diff` already swallows its own -"snapshot doesn't exist" failure via `continue-on-error: true`, so -the false-trigger is wasted runtime + a slightly chatty checks tab, -nothing more) -**Convoy owner:** parent (no architect — single-file workflow YAML -tweak following published GitHub Actions path-filter semantics) -**Opened:** 2026-05-26 -**Branch:** `convoy/tighten-visual-diff-path-filter` - -## Background - -`.github/workflows/visual-diff.yml` is supposed to fire only on UI -changes. Its current `paths:` filter: - -```yaml -paths: - - 'pages/**' - - 'components/**' - - 'styles/**' - - 'tailwind.config.js' - - 'postcss.config.js' -``` - -But `pages/**` matches `pages/api/**` too, and the `tcg-vault` repo's -API surface lives entirely under `pages/api/` (Next.js Pages router). -That means **API-only PRs trigger the visual-diff workflow** even -though they can't possibly move a single rendered pixel. The -empirical false-positives: - -- **PR #19 `cors-tighten`** (squash `da50d78`, 2026-05-24) — touched - 24 files all under `pages/api/**`. Triggered `Screenshot diff`, - documented in `.convoys/ship-readiness.md` § P0 #5 As-shipped - metrics: *"Screenshot diff — workflow exited 0 because of - `continue-on-error: true`, but the actual visual test failed with - the documented 'snapshot doesn't exist' error… Triggered on PR #19 - despite this being API-only because its `paths:` filter is - `pages/**` which matches `pages/api/**` too — minor false-positive - queued as `tighten-visual-diff-path-filter`."* -- **PR #20 `add-rate-limiting`** (squash `708ef45`, 2026-05-24) — - touched 6 routes under `pages/api/` (plus `lib/rate-limit.js` and - `pages/admin/card-import.js`). Same false-trigger, same swallow. - Documented in `.convoys/ship-readiness.md` § P0 #6 As-shipped - metrics. - -Cost per false-trigger: **~55s of CI runtime** (the `wait-for-vercel-preview` -step hits its 120s budget against the deployed preview, then -Playwright `npm ci` + `npx playwright install` + the `visual` project -runs — even though the test ultimately can't compare against a -non-existent baseline). The `Screenshot diff` job exits 0 because of -`continue-on-error: true` (Decision-4 end state of -`adopt-playwright-smoke`, until `seed-visual-baselines-on-linux` -lands), but it still posts a "Visual Diff — view run" comment and -clutters the PR Checks tab with a green-but-meaningless run. - -## Design decision — negated-glob `!pages/api/**` - -GitHub Actions evaluates `paths:` with [minimatch](https://github.com/isaacs/minimatch) -and supports `!`-prefixed exclusion patterns per the -[official path-filter cheatsheet](https://docs.github.com/en/actions/using-workflows/triggering-a-workflow#patterns-to-match-file-paths). -Order matters: a `!pattern` only takes effect if it comes AFTER an -include that already matched the path. So the canonical shape is: - -```yaml -paths: - - 'pages/**' - - '!pages/api/**' # must follow 'pages/**' to subtract from it - - 'components/**' - ... -``` - -**Considered alternatives:** - -1. **Per-feature paths** — replace `pages/**` with explicit - subdirectory globs (`pages/!(api)/**` *or* `pages/dashboard.js`, - `pages/cards/**`, `pages/decks/**`, …). Rejected: too verbose, - needs to be touched every time a top-level page is added, - defeats the "trigger on UI changes" intent. -2. **Extglob `pages/!(api)/**`** — would work in bash with extglob - enabled, but minimatch's default options used by GitHub Actions - do NOT enable extglob without a flag we can't set from YAML. - The queue entry explicitly flagged this risk; the negated-glob - shape is the safer documented path. -3. **Move the gate into the `gate:` job** — add a step that diffs - `pages/api/**` and sets `should_run=false` if every changed file - is API-only. Rejected: more code, more surface, doesn't actually - fire faster (the gate job itself spins up a runner). The native - `paths:` filter short-circuits BEFORE any runner spins up, - which is the cheapest possible exclusion. - -The simple negation is sufficient and matches GitHub's published -guidance. - -## The fix - -Single edit in `.github/workflows/visual-diff.yml`. Insert -`!pages/api/**` immediately after `pages/**`, with an inline comment -explaining the ordering rule and the empirical motivation: - -```yaml -paths: - - 'pages/**' - # Exclude API-only edits — they don't render UI, so they can't move - # any visual-diff pixels. Order matters: GitHub Actions evaluates the - # `paths:` list with minimatch and applies `!`-prefixed exclusions - # only after they've already matched a prior include. Keep this entry - # immediately AFTER `pages/**`. - # Surfaced by `tighten-visual-diff-path-filter` after PR #19 - # (cors-tighten) and PR #20 (add-rate-limiting) both falsely - # triggered Screenshot diff at ~55s/PR. - - '!pages/api/**' - - 'components/**' - - 'styles/**' - - 'tailwind.config.js' - - 'postcss.config.js' -``` - -All five existing entries are preserved; only the one exclusion entry -is added. - -### `.github/workflows/preview-smoke.yml` — left untouched - -Verified the sibling workflow's shape: - -```yaml -on: - pull_request: - branches: [main] - types: [labeled, opened, synchronize, reopened] -``` - -`preview-smoke.yml` has **no `paths:` filter at all** — it triggers -on every PR targeting `main` (modulo the in-job `gate:` skip via -`pipeline: skip smoke` in the PR body). This is intentional: a smoke -test that hits the home redirect, the sign-in page, and `/api/health` -SHOULD run on every PR including API-only ones, because changes to -`pages/api/**` can break those routes too. There is no false-positive -shape to fix here. Leaving `preview-smoke.yml` strictly out of scope. - -## Verification plan - -1. **YAML parse** — `python3 -c "import yaml; ..."` confirms the - `paths:` list deserializes to the expected 6-entry list with - `'!pages/api/**'` at index 1 (immediately after `'pages/**'`). - Done at gate time, see § Acceptance criteria. -2. **`npm run lint`** — exits 1 with **128 problems** (baseline - preserved, no regression). YAML files don't go through ESLint; - verification here is just that we didn't accidentally edit a - `.js` source file. -3. **`npm run test:run`** — **21/21 pass**. YAML changes don't touch - any test surface; verification only. -4. **Post-merge CI behavior verification — DEFERRED.** The only true - verification that the `!pages/api/**` exclusion actually fires - the way we expect is observing the **next API-only PR after this - merges** and confirming `Screenshot diff` does NOT appear in its - Checks tab. We document this explicitly here so the doc-writer - pass that closes the convoy can record the next API-only PR's - number + a "Screenshot diff: not triggered" line as the - as-shipped success criterion (mirroring the - `fix-reset-db-script` convoy's Screenshot-diff-not-triggered - line in its as-shipped block). - - We do NOT attempt to live-verify the path filter at convoy time - (e.g. by pushing a throwaway API-only commit to a sacrificial - branch and watching CI). That'd be theater — GitHub's path-filter - semantics are documented and stable, and the YAML parse + the - syntax match against the published cheatsheet is enough - pre-merge confidence for a P3 polish convoy. - -## Acceptance criteria - -- `python3 -c "import yaml; d=yaml.safe_load(open('.github/workflows/visual-diff.yml')); print(d[True]['pull_request']['paths'])"` - → `['pages/**', '!pages/api/**', 'components/**', 'styles/**', 'tailwind.config.js', 'postcss.config.js']` - (order-sensitive) -- `npm run lint` → exit 1 with 128 problems (baseline preserved) -- `npm run test:run` → 21/21 pass -- `.github/workflows/preview-smoke.yml` unchanged in this PR's diff -- No other workflow files touched - -## Risks - -- **R1 — minimatch syntax compatibility.** GitHub Actions uses - minimatch internally; the `!` prefix at the start of a pattern is - documented as the canonical exclusion syntax. If for any reason - Actions rejects this shape on the next workflow load (unlikely — - this exact shape is shown in the published cheatsheet), the - workflow would either fail to register OR silently treat the `!` - pattern as a literal include. **Fallback:** restructure to - per-feature path globs (`pages/dashboard.js`, `pages/cards/**`, - `pages/decks/**`, `pages/deck/**`, `pages/deck-builder.js`, - `pages/scanner.js`, `pages/profile.js`, `pages/settings.js`, - `pages/login.js`, `pages/register.js`, `pages/admin/**`, - `pages/collection/**`, `pages/collections.js`, `pages/community/**`, - `pages/invite/**`, `pages/my-cards.js`, `pages/card/**`, - `pages/_app.js`, `pages/_document.js`, `pages/_error.js`, `pages/index.js`). - More verbose, but unambiguously valid. Track as a follow-up convoy - ONLY if the post-merge verification step (next API-only PR) shows - the exclusion didn't fire. -- **R2 — future `pages//` subdir.** If someone - later adds a directory like `pages/server/**` that contains both - API-style endpoints AND visual UI pages, the simple `!pages/api/**` - exclusion would not catch it, and visual-diff would fire on - changes to that directory. Documented but accepted: the repo - convention for the foreseeable future is "all backend lives under - `pages/api/**`", and the only realistic alternative ("server - components" or similar) would warrant its own paths-filter - revisit at that point. R2 is a "watch this space" risk, not a - blocker. - -## Scope - -- **In scope:** `.github/workflows/visual-diff.yml` only (plus this - convoy planning doc). -- **Out of scope:** any other workflow file. Verified - `preview-smoke.yml` has no `paths:` filter and intentionally fires - on every PR, so no mirror-fix is needed there. - -## Why no architect - -This is a **single-file YAML tweak following published vendor -documentation**. No new precedents; no new decisions; the queue -entry in `.convoys/ship-readiness.md` § Queued convoys already -ratified the negated-glob direction. Parent applies the fix, runs -the bounded checks (YAML parse + lint baseline + vitest), opens the -PR. If anything surprising surfaces (the YAML doesn't parse, -minimatch rejects the syntax), the parent stops and dispatches an -architect mid-execution. - -## Out of scope (queued follow-ups) - -- **`seed-visual-baselines-on-linux`** (priority: P3 polish; **was - already queued** by `adopt-playwright-smoke`) — once visual - baselines are seeded under `tests/visual/__screenshots__/` from a - Linux runner (or the documented Playwright Docker container), the - `Screenshot diff` job will start posting real visual-diff - comparisons and `continue-on-error: true` can be removed. This - convoy's path-filter tightening is orthogonal to baseline seeding — - both are needed eventually, but neither blocks the other. Surfaced - in `AGENTS.md` § 6 Testing. - -## As-shipped - -*(stub — populated by post-merge doc-writer pass)* - -- Squash commit: `` -- PR: `` -- Diff stat: `` (expected: 2 files, +N / -0 — `visual-diff.yml` - +N for the one entry + comment block; this convoy file +M for the - full planning doc) -- Verification at merge: - - YAML parse: paths list includes `!pages/api/**` immediately after - `pages/**` - - Lint: 128 problems (baseline preserved) - - Vitest: 21/21 - - All pre-existing CI gates green at merge -- **Post-merge success criterion** (the deferred verification from - § Verification plan): the next API-only PR after this merges does - NOT show `Screenshot diff` in its Checks tab. Doc-writer to record - that PR's number + the absence of `Screenshot diff` as the - as-shipped success line, mirroring `.convoys/fix-reset-db-script.md`'s - *"Screenshot diff: not triggered (script-only PR — `paths:` filter - excludes `scripts/**`…)"* line. - -## Owns - -Parent (single-file proven-pattern fix; no architect or implementer -subagent required). diff --git a/.cursor/rules/auth-and-permissions.mdc b/.cursor/rules/auth-and-permissions.mdc index 7308787..98f7bbf 100644 --- a/.cursor/rules/auth-and-permissions.mdc +++ b/.cursor/rules/auth-and-permissions.mdc @@ -21,12 +21,15 @@ There are three parallel client-side auth implementations and one server-side he | Client: route protection | `components/ProtectedRoute.js` | | Client: admin route protection | `components/AdminProtected.js` | -## Legacy (do not extend) +## Legacy (deleted by `single-auth-provider`) -- `lib/auth-context.js::AuthProvider` + `useAuth` — older context. Still wired in `pages/_app.js`; left in place for compatibility. Don't add new consumers. -- `lib/admin-auth.js::AdminProvider` + `useAdmin` + `useIsAdmin` — parallel admin context. Same story. - -A convoy is planned to collapse these three into one provider + one hook. +`lib/auth-context.js` and `lib/admin-auth.js` were the two parallel client-side +auth surfaces that lived alongside `lib/use-auth.js`. They were deleted by the +`single-auth-provider` convoy (P1). Do **not** reintroduce a `` +or `` wrapper in `pages/_app.js` — `useAuth()` from +`lib/use-auth.js` is hook-only (reads token from `localStorage` and hits +`/api/auth/verify` on mount) and does not require a context provider. The +`useIsAdmin` semantic is now `const { user } = useAuth(); const isAdmin = user?.role === 'admin'`. ## Token model @@ -48,7 +51,7 @@ When introducing a new permission tier, update both `checkRolePermission`'s hier ## Authentication state on the client -`useAuth()` returns `{ user, loading, login, logout, refresh }`. `user === null` means logged out; `loading === true` means token verification in flight. Always render against `loading === false` before deciding to redirect. +`useAuth()` returns `{ user, loading, logout, refreshAuth }`. `user === null` means logged out; `loading === true` means token verification in flight. Always render against `loading === false` before deciding to redirect. The login / register flows do **not** go through `useAuth` — `pages/login.js` and `pages/signup.js` `fetch` `/api/auth/{login,register}` directly and write the returned token to `localStorage`; `useAuth()` will pick it up on next mount via its `useEffect` → `/api/auth/verify` roundtrip (or call `refreshAuth()` to re-verify in place). ## Server-side authorization patterns diff --git a/.cursor/rules/no-go-zones.mdc b/.cursor/rules/no-go-zones.mdc index 3f47c84..028ae90 100644 --- a/.cursor/rules/no-go-zones.mdc +++ b/.cursor/rules/no-go-zones.mdc @@ -33,5 +33,5 @@ Do not edit, refactor, or quote as context examples. If you think you need to ch ## Editing rules of thumb - **Schema changes:** until a proper migration tool lands, document the change in a new dated script under `scripts/migrations/YYYY-MM-DD-.js` (folder TBD). Do NOT edit `scripts/setup-neon-db.js` in place for any **DDL change** (`CREATE TABLE`, `ALTER`, new columns, constraint changes) — it's idempotent and meant for first-time setup only. **Operational changes are allowed** (env-var gating, error-message hardening, module-system fixes) — `drop-public-setup` set this precedent by adding the `ADMIN_INITIAL_PASSWORD` gate and converting the script to ESM. The distinction: if the change touches DDL strings or `INSERT` semantics, file a migration; if it only touches Node-module behavior or pre-flight validation, edit in place and document why in the convoy. -- **Auth refactors:** `lib/permission-middleware.js`, `pages/api/auth-utils.js`, `lib/auth-context.js`, `lib/admin-auth.js`, and `lib/use-auth.js` form a deliberately documented mess. Tighten them inside a single convoy; don't cherry-pick. +- **Auth refactors:** `lib/permission-middleware.js`, `pages/api/auth-utils.js`, `lib/auth-secret.js`, and `lib/use-auth.js` are the four documented auth surfaces. Tighten them inside a single convoy; don't cherry-pick. (The legacy `lib/auth-context.js` and `lib/admin-auth.js` were deleted by `single-auth-provider`; do not resurrect them.) - **Card-import jobs:** `pages/api/cards/import-*.js` hit external APIs with rate limits. Don't run them ad-hoc against prod data; use staging. diff --git a/.cursor/skills/add-page/SKILL.md b/.cursor/skills/add-page/SKILL.md index 09f7ff9..0180773 100644 --- a/.cursor/skills/add-page/SKILL.md +++ b/.cursor/skills/add-page/SKILL.md @@ -93,7 +93,7 @@ Use Tailwind for layout, spacing, sizing, hover/focus states. Use CSS vars (inli ## Step 6: Check - [ ] Auth wrapper chosen (ProtectedRoute / AdminProtected / public). -- [ ] `useAuth()` from `lib/use-auth.js` (not the legacy `lib/auth-context.js`). +- [ ] `useAuth()` from `lib/use-auth.js` (the only client auth hook; `lib/auth-context.js` and `lib/admin-auth.js` were deleted by the `single-auth-provider` convoy). - [ ] `user` passed to Layout explicitly. - [ ] Colors come from theme tokens, not hex. - [ ] All interactive elements have `aria-label` or visible text. @@ -105,5 +105,5 @@ Use Tailwind for layout, spacing, sizing, hover/focus states. Use CSS vars (inli | --- | --- | | Hardcode hex colors | Use CSS variables | | Default `user = { … }` to a real email | Default to `null` | -| Pull from `lib/auth-context` for new code | Use `lib/use-auth` | +| Reintroduce `lib/auth-context` or `lib/admin-auth` (deleted) | Use `lib/use-auth` | | Render Layout twice on the same page | Single `` at the top | diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 66fc49b..354055a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -8,8 +8,6 @@ pages/api/auth/** @YOUR-GITHUB-HANDLE pages/api/auth-utils.js @YOUR-GITHUB-HANDLE lib/permission-middleware.js @YOUR-GITHUB-HANDLE -lib/auth-context.js @YOUR-GITHUB-HANDLE -lib/admin-auth.js @YOUR-GITHUB-HANDLE lib/use-auth.js @YOUR-GITHUB-HANDLE components/ProtectedRoute.js @YOUR-GITHUB-HANDLE components/AdminProtected.js @YOUR-GITHUB-HANDLE diff --git a/.github/workflows/visual-diff.yml b/.github/workflows/visual-diff.yml index 744233c..f6771e9 100644 --- a/.github/workflows/visual-diff.yml +++ b/.github/workflows/visual-diff.yml @@ -9,15 +9,6 @@ on: branches: [main] paths: - 'pages/**' - # Exclude API-only edits — they don't render UI, so they can't move - # any visual-diff pixels. Order matters: GitHub Actions evaluates the - # `paths:` list with minimatch and applies `!`-prefixed exclusions - # only after they've already matched a prior include. Keep this entry - # immediately AFTER `pages/**`. - # Surfaced by `tighten-visual-diff-path-filter` after PR #19 - # (cors-tighten) and PR #20 (add-rate-limiting) both falsely - # triggered Screenshot diff at ~55s/PR. - - '!pages/api/**' - 'components/**' - 'styles/**' - 'tailwind.config.js' diff --git a/AGENTS.md b/AGENTS.md index a5132ac..0777ef3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ A web app for managing trading-card-game collections (Magic, Pokémon, Lorcana). | Pages router views | `pages/*.js` | Public + auth views; uses `components/Layout.js` | | API routes | `pages/api/**/*.js` | Express-style `handler(req, res)`. **30+ handlers depend on `lib/permission-middleware.js::getUserFromRequest`** | | Shared UI | `components/*.js` | `Layout`, `CardItem`, `CameraScanner`, modal family | -| Auth + DB libs | `lib/*.js` | `auth-context`, `admin-auth`, `use-auth` (three parallel auth surfaces), `database`, `permission-middleware` | +| Auth + DB libs | `lib/*.js` | `use-auth` (canonical client hook — sole surface post-`single-auth-provider`), `database`, `permission-middleware` | | Migration scripts | `scripts/*.js` | 27+ one-off "add column" / "seed" scripts. No formal migration tool | | Card-import jobs | `pages/api/cards/import-*.js`, `scripts/import-*.js` | Scryfall / Lorcana / Pokémon TCG APIs | | Database schema | `scripts/setup-neon-db.js` | Bootstrap SQL DDL — the source of truth until a real migration tool lands | @@ -40,7 +40,7 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560 ## 3. Key conventions - **Auth (server):** `import { getUserFromRequest } from '../../lib/permission-middleware'` → returns `{ userId, email, role }` or `null`. `null` means "send 401" — always early-return when the user is null before doing any work that depends on their identity. -- **Auth (client):** `import { useAuth } from '../lib/use-auth'`. Avoid `lib/auth-context.js` and `lib/admin-auth.js` for new code — they are legacy parallel implementations. +- **Auth (client):** `import { useAuth } from '../lib/use-auth'`. Returns `{ user, loading, logout, refreshAuth }`; `user === null` means logged out, `loading === true` means token verification in flight. There is no client-side admin hook — compute `const isAdmin = user?.role === 'admin'` from the same `useAuth()` call. The legacy `lib/auth-context.js` and `lib/admin-auth.js` were deleted by the `single-auth-provider` convoy; do not reintroduce a `` / `` wrapper in `pages/_app.js`. - **Layout `user` prop:** pages should pass `user` from `useAuth()` to ``. Layout's default is `null` and renders a logged-out "Sign in" CTA when no user is supplied — both paths are valid (some surfaces like `pages/invite/{accept,decline}.js` legitimately render Layout for anonymous visitors). Do not reintroduce a hardcoded user object as a default prop. - **JWT secret + TTL:** `import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'`. This is the only place either value is defined; do not reintroduce literal fallbacks. `JWT_TOKEN_TTL = '24h'` is canonical. - **Auth helper (token mint / verify / password hash):** `import { ... } from '../../pages/api/auth-utils'` (`generateToken`, `verifyToken`, `hashPassword`, `verifyPassword`). Reads the secret + TTL from `lib/auth-secret.js` under the hood. diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index 816879c..502f45e 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -2,21 +2,11 @@ ## 👥 Test Accounts -| User | Email | Role | Password source | -|------|-------|------|-----------------| -| Admin | `admin@deckhearth.com` | Admin | `ADMIN_INITIAL_PASSWORD` env var (seeded by `npm run setup-db`) | -| Alice | `alice@deckhearth.com` | User | `TEST_USERS_PASSWORD` env var (seeded by `node scripts/create-test-users.js`) | -| Bob | `bob@deckhearth.com` | User | `TEST_USERS_PASSWORD` env var (seeded by `node scripts/create-test-users.js`) | - -Both env vars must be set in `.env.local` before running the -corresponding seed script — each script fails loud (exit 1, no DB -connection opened) if its env var is unset. Generate strong values -with `openssl rand -base64 24`; see README.md → "First-time admin -setup" for the canonical env-var pattern. - -Alice + Bob share a single `TEST_USERS_PASSWORD` value because this is -a test-fixture surface; that's intentional and documented in the -`purge-weak-creds-from-helpers` convoy. +| User | Email | Password | Role | +|------|-------|----------|------| +| Admin | `admin@deckhearth.com` | `admin123` | Admin | +| Alice | `alice@deckhearth.com` | `alice123` | User | +| Bob | `bob@deckhearth.com` | `bob123` | User | ## 🃏 Sample Cards Available @@ -31,8 +21,8 @@ a test-fixture surface; that's intentional and documented in the ### 1. **Login as Alice** ``` -Email: alice@deckhearth.com -Password: +Email: alice@deckhearth.com +Password: alice123 ``` ### 2. **Create a Collection** @@ -61,7 +51,7 @@ Password: ### 5. **Switch to Bob's Account** - Logout and login as Bob - Email: `bob@deckhearth.com` -- Password: `` +- Password: `bob123` ### 6. **Accept Invitation (Simulated)** Since we're testing locally, simulate email acceptance: diff --git a/components/Layout.js b/components/Layout.js index 84a14fa..d9472e1 100644 --- a/components/Layout.js +++ b/components/Layout.js @@ -596,6 +596,7 @@ export default function Layout({ children, user = null, showSearch = false }) {
{/* Mobile Navigation - Bottom bar for mobile */} setIsMobileMenuOpen(true)} /> diff --git a/components/MobileNavigation.js b/components/MobileNavigation.js index 56690ae..c492495 100644 --- a/components/MobileNavigation.js +++ b/components/MobileNavigation.js @@ -2,7 +2,7 @@ import { useRouter } from 'next/router'; import Link from 'next/link'; import { useState } from 'react'; -export default function MobileNavigation({ onMenuOpen }) { +export default function MobileNavigation({ user, onMenuOpen }) { const router = useRouter(); // Navigation items for the bottom bar diff --git a/eslint.config.mjs b/eslint.config.mjs index 1705fe9..1bfbd39 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -11,15 +11,6 @@ const eslintConfig = defineConfig([ 'next-env.d.ts', 'scripts/migrations/**', ]), - { - files: ['scripts/**/*.js'], - rules: { - 'no-restricted-syntax': ['error', { - selector: 'CallExpression[callee.name="require"]', - message: 'Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md.', - }], - }, - }, ]); export default eslintConfig; diff --git a/lib/admin-auth.js b/lib/admin-auth.js deleted file mode 100644 index 4a90b65..0000000 --- a/lib/admin-auth.js +++ /dev/null @@ -1,120 +0,0 @@ -import { createContext, useContext, useState, useEffect } from 'react'; - -// Create admin context -const AdminContext = createContext(); - -export function AdminProvider({ children }) { - const [user, setUser] = useState(null); - const [loading, setLoading] = useState(true); - - useEffect(() => { - checkAdminAuth(); - }, []); - - const checkAdminAuth = async () => { - try { - // Get token from localStorage - const token = localStorage.getItem('auth_token'); - - const headers = { - 'Content-Type': 'application/json', - }; - - // Add authorization header if token exists - if (token) { - headers.Authorization = `Bearer ${token}`; - } - - const response = await fetch('/api/auth/verify', { headers }); - if (response.ok) { - const userData = await response.json(); - setUser(userData); - } else { - setUser(null); - // Clear invalid token - if (token) { - localStorage.removeItem('auth_token'); - } - } - } catch (error) { - console.error('Auth check failed:', error); - setUser(null); - } finally { - setLoading(false); - } - }; - - const isAdmin = () => { - return user && user.role === 'admin'; - }; - - const isAuthenticated = () => { - return user !== null; - }; - - const value = { - user, - loading, - isAdmin, - isAuthenticated, - checkAdminAuth - }; - - return ( - - {children} - - ); -} - -export function useAdmin() { - const context = useContext(AdminContext); - if (!context) { - throw new Error('useAdmin must be used within an AdminProvider'); - } - return context; -} - -// Simple hook for checking admin status without context -export function useIsAdmin() { - const [isAdmin, setIsAdmin] = useState(false); - const [loading, setLoading] = useState(true); - - useEffect(() => { - const checkAdmin = async () => { - try { - // Get token from localStorage - const token = localStorage.getItem('auth_token'); - - const headers = { - 'Content-Type': 'application/json', - }; - - // Add authorization header if token exists - if (token) { - headers.Authorization = `Bearer ${token}`; - } - - const response = await fetch('/api/auth/verify', { headers }); - if (response.ok) { - const userData = await response.json(); - setIsAdmin(userData.role === 'admin'); - } else { - setIsAdmin(false); - // Clear invalid token - if (token) { - localStorage.removeItem('auth_token'); - } - } - } catch (error) { - setIsAdmin(false); - } finally { - setLoading(false); - } - }; - - checkAdmin(); - }, []); - - return { isAdmin, loading }; -} \ No newline at end of file diff --git a/lib/auth-context.js b/lib/auth-context.js deleted file mode 100644 index 3b7e8b5..0000000 --- a/lib/auth-context.js +++ /dev/null @@ -1,116 +0,0 @@ -import { createContext, useContext, useState, useEffect } from 'react'; - -const AuthContext = createContext(); - -export function AuthProvider({ children }) { - const [user, setUser] = useState(null); - const [loading, setLoading] = useState(true); - - useEffect(() => { - // Check for existing token on app load - const token = localStorage.getItem('auth_token'); - if (token) { - // Verify token and set user - verifyToken(token); - } else { - setLoading(false); - } - }, []); - - const verifyToken = async (token) => { - try { - const response = await fetch('/api/auth/verify', { - headers: { - 'Authorization': `Bearer ${token}` - } - }); - - if (response.ok) { - const userData = await response.json(); - setUser(userData); // API returns user data directly, not wrapped in .user - } else { - localStorage.removeItem('auth_token'); - } - } catch (error) { - console.error('Token verification failed:', error); - localStorage.removeItem('auth_token'); - } finally { - setLoading(false); - } - }; - - const login = async (email, password) => { - try { - const response = await fetch('/api/auth/login', { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ email, password }), - }); - - const data = await response.json(); - - if (response.ok) { - localStorage.setItem('auth_token', data.token); - setUser(data.user); - return { success: true }; - } else { - return { success: false, error: data.error }; - } - } catch (error) { - return { success: false, error: 'Network error' }; - } - }; - - const register = async (email, password) => { - try { - const response = await fetch('/api/auth/register', { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ email, password }), - }); - - const data = await response.json(); - - if (response.ok) { - localStorage.setItem('auth_token', data.token); - setUser(data.user); - return { success: true }; - } else { - return { success: false, error: data.error }; - } - } catch (error) { - return { success: false, error: 'Network error' }; - } - }; - - const logout = () => { - localStorage.removeItem('auth_token'); - setUser(null); - }; - - const value = { - user, - loading, - login, - register, - logout, - }; - - return ( - - {children} - - ); -} - -export function useAuth() { - const context = useContext(AuthContext); - if (!context) { - throw new Error('useAuth must be used within an AuthProvider'); - } - return context; -} \ No newline at end of file diff --git a/lib/database.js b/lib/database.js new file mode 100644 index 0000000..8d0b00a --- /dev/null +++ b/lib/database.js @@ -0,0 +1,47 @@ +import { neon } from '@neondatabase/serverless'; + +// Get the database URL from environment variables +const sql = neon(process.env.POSTGRES_URL); + +// Database adapter that works with Neon +class DatabaseAdapter { + async query(sqlString, params = []) { + try { + // For Neon, we need to use tagged template literals + // This is a simplified approach - in production you'd want more robust parameter handling + let result; + + if (params.length === 0) { + // No parameters + result = await sql`${sql.unsafe(sqlString)}`; + } else { + // With parameters - this is a simplified approach + // In production, you'd want proper parameter escaping + const escapedParams = params.map(param => + typeof param === 'string' ? `'${param.replace(/'/g, "''")}'` : param + ); + + let query = sqlString; + for (let i = 0; i < escapedParams.length; i++) { + query = query.replace(`$${i + 1}`, escapedParams[i]); + } + + result = await sql`${sql.unsafe(query)}`; + } + + return { + rows: Array.isArray(result) ? result : [result], + rowCount: Array.isArray(result) ? result.length : 1 + }; + } catch (error) { + console.error('Database query error:', error); + throw error; + } + } + + async raw(sqlString, params = []) { + return await this.query(sqlString, params); + } +} + +export const db = new DatabaseAdapter(); \ No newline at end of file diff --git a/pages/_app.js b/pages/_app.js index 671cf63..d85b52f 100644 --- a/pages/_app.js +++ b/pages/_app.js @@ -1,13 +1,10 @@ import '../styles/globals.css'; -import { AuthProvider } from '../lib/auth-context.js'; import { ThemeProvider } from '../lib/theme-context.js'; export default function App({ Component, pageProps }) { return ( - - - + ); -} \ No newline at end of file +} diff --git a/pages/api/auth-utils.js b/pages/api/auth-utils.js index 30eed94..3a92321 100644 --- a/pages/api/auth-utils.js +++ b/pages/api/auth-utils.js @@ -1,5 +1,5 @@ import jwt from 'jsonwebtoken'; -import { sql } from '@vercel/postgres'; +import { db } from '../../lib/database.js'; import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'; export async function hashPassword(password) { @@ -14,10 +14,10 @@ export async function verifyPassword(password, hashedPassword) { export function generateToken(user) { return jwt.sign( - { - userId: user.id, - email: user.email, - role: user.role + { + userId: user.id, + email: user.email, + role: user.role }, JWT_SECRET, { expiresIn: JWT_TOKEN_TTL } @@ -34,9 +34,9 @@ export function verifyToken(token) { export async function isAdmin(userId) { try { - const result = await sql` - SELECT role FROM users WHERE id = ${userId} - `; + const result = await db.query(` + SELECT role FROM users WHERE id = $1 + `, [userId]); return result.rows[0]?.role === 'admin'; } catch (error) { console.error('Error checking admin status:', error); @@ -46,12 +46,12 @@ export async function isAdmin(userId) { export async function getUserById(userId) { try { - const result = await sql` - SELECT id, email, role, created_at FROM users WHERE id = ${userId} - `; + const result = await db.query(` + SELECT id, email, role, created_at FROM users WHERE id = $1 + `, [userId]); return result.rows[0]; } catch (error) { console.error('Error getting user:', error); return null; } -} +} \ No newline at end of file diff --git a/pages/card/[id].js b/pages/card/[id].js index e7d92e3..8de8bc8 100644 --- a/pages/card/[id].js +++ b/pages/card/[id].js @@ -1,7 +1,6 @@ import { useState, useEffect } from 'react'; import { useRouter } from 'next/router'; import Layout from '../../components/Layout'; -import { useIsAdmin } from '../../lib/admin-auth'; import { useAuth } from '../../lib/use-auth'; import CollectionSelectionModal from '../../components/CollectionSelectionModal'; import { ManaCost, ColorIdentity, AdvancedManaCost } from '../../components/ManaSymbols'; @@ -10,7 +9,7 @@ import ManaSymbolSettings from '../../components/ManaSymbolSettings'; export default function CardDetail() { const router = useRouter(); const { id } = router.query; - const { user } = useAuth(); + const { user, loading: authLoading } = useAuth(); const [card, setCard] = useState(null); const [loading, setLoading] = useState(true); @@ -31,8 +30,8 @@ export default function CardDetail() { // Mana symbol settings const [manaSymbolSettings, setManaSymbolSettings] = useState({ useSVG: false }); - // Check admin status - const { isAdmin, loading: adminLoading } = useIsAdmin(); + const isAdmin = user?.role === 'admin'; + const adminLoading = authLoading; // Fetch card data from API useEffect(() => { diff --git a/pages/deck-builder.js b/pages/deck-builder.js index 02328ab..d4e2ec7 100644 --- a/pages/deck-builder.js +++ b/pages/deck-builder.js @@ -4,7 +4,7 @@ import Link from 'next/link'; import Layout from '../components/Layout'; import { ManaCost, ColorIdentity, ColorFilterSymbol } from '../components/ManaSymbols'; import ManaSymbolSettings from '../components/ManaSymbolSettings'; -import { useAuth } from '../lib/auth-context'; +import { useAuth } from '../lib/use-auth'; import { getColorIdentity, getColorSymbol } from '../lib/mana-symbols'; export default function DeckBuilder() { diff --git a/pages/deck/[id].js b/pages/deck/[id].js index 2c975b3..ef7d0bb 100644 --- a/pages/deck/[id].js +++ b/pages/deck/[id].js @@ -3,7 +3,7 @@ import { useRouter } from 'next/router'; import Link from 'next/link'; import Layout from '../../components/Layout'; import { ManaCost, ColorIdentity } from '../../components/ManaSymbols'; -import { useAuth } from '../../lib/auth-context'; +import { useAuth } from '../../lib/use-auth'; import { getColorIdentity } from '../../lib/mana-symbols'; export default function DeckDetail() { diff --git a/pages/decks.js b/pages/decks.js index 33aedf4..e52b615 100644 --- a/pages/decks.js +++ b/pages/decks.js @@ -2,7 +2,7 @@ import { useState, useEffect } from 'react'; import { useRouter } from 'next/router'; import Link from 'next/link'; import Layout from '../components/Layout'; -import { useAuth } from '../lib/auth-context'; +import { useAuth } from '../lib/use-auth'; export default function Decks() { const { user } = useAuth(); diff --git a/pages/index.js b/pages/index.js index 5d93fd3..03a6724 100644 --- a/pages/index.js +++ b/pages/index.js @@ -1,7 +1,7 @@ import { useState, useEffect } from 'react'; import { useRouter } from 'next/router'; import Link from 'next/link'; -import { useAuth } from '../lib/auth-context.js'; +import { useAuth } from '../lib/use-auth.js'; import AnimatedFireLogo from '../components/AnimatedFireLogo'; export default function Home() { diff --git a/pages/scanner.js b/pages/scanner.js index 4fa22b2..345a816 100644 --- a/pages/scanner.js +++ b/pages/scanner.js @@ -5,7 +5,7 @@ import CameraScanner from '../components/CameraScanner'; import OCRSettings from '../components/OCRSettings'; import { ManaCost, ColorIdentity } from '../components/ManaSymbols'; import ManaSymbolSettings from '../components/ManaSymbolSettings'; -import { useAuth } from '../lib/auth-context'; +import { useAuth } from '../lib/use-auth'; export default function Scanner() { const { user } = useAuth(); diff --git a/scripts/create-test-users.js b/scripts/create-test-users.js index abb26cb..e0c45ed 100755 --- a/scripts/create-test-users.js +++ b/scripts/create-test-users.js @@ -1,69 +1,39 @@ #!/usr/bin/env node -/** - * Create Test Users Script - * - * Seeds the alice + bob test-user fixtures used by the manual QA flows - * in TESTING_GUIDE.md. Both users share a single password supplied via - * the TEST_USERS_PASSWORD environment variable — this is a test-fixture - * helper, not a prod auth surface, so per-user env vars would be - * unnecessary sprawl. - * - * Required env (in .env.local): - * POSTGRES_URL — Neon connection string - * TEST_USERS_PASSWORD — strong password applied to every test user - * (generate with `openssl rand -base64 24`) - * - * Mirrors the post-`drop-public-setup` shape of `setup-neon-db.js` - * (commit b63b509) and the post-`fix-reset-db-script` shape of - * `reset-db.js` (commit 3ab9bf8) — same ESM imports, same fail-loud - * env-var check, same no-password-echo convention. Convoy: - * `purge-weak-creds-from-helpers` (2026-05-26). - */ - import { config } from 'dotenv'; import { sql } from '@vercel/postgres'; import bcrypt from 'bcryptjs'; +// Load environment variables config({ path: '.env.local' }); async function createTestUsers() { - const testUsersPassword = process.env.TEST_USERS_PASSWORD; - if (!testUsersPassword || !testUsersPassword.trim()) { - console.error( - '❌ TEST_USERS_PASSWORD environment variable is not set.\n' + - '\n' + - ' Set it in .env.local before running `node scripts/create-test-users.js`.\n' + - ' Generate a strong password with: openssl rand -base64 24\n' + - ' See README.md → "First-time admin setup" for the env-var pattern.\n' - ); - process.exit(1); - } - try { - console.log('👥 Creating test users...\n'); - - const hashedPassword = await bcrypt.hash(testUsersPassword, 12); + console.log('�� Creating test users...\n'); + // Create Alice (collaborator) + const alicePassword = await bcrypt.hash('alice123', 12); await sql` INSERT INTO users (email, password, role) - VALUES ('alice@deckhearth.com', ${hashedPassword}, 'user') + VALUES ('alice@deckhearth.com', ${alicePassword}, 'user') ON CONFLICT (email) DO NOTHING `; - console.log('✅ Created Alice (alice@deckhearth.com)'); + console.log('✅ Created Alice (alice@deckhearth.com / alice123)'); + // Create Bob (collaborator) + const bobPassword = await bcrypt.hash('bob123', 12); await sql` INSERT INTO users (email, password, role) - VALUES ('bob@deckhearth.com', ${hashedPassword}, 'user') + VALUES ('bob@deckhearth.com', ${bobPassword}, 'user') ON CONFLICT (email) DO NOTHING `; - console.log('✅ Created Bob (bob@deckhearth.com)'); + console.log('✅ Created Bob (bob@deckhearth.com / bob123)'); console.log('\n🎉 Test users created successfully!'); - console.log('\n👥 Available Test Accounts (passwords from TEST_USERS_PASSWORD):'); - console.log(' 1. admin@deckhearth.com (Admin — seeded by setup-neon-db.js)'); - console.log(' 2. alice@deckhearth.com (User)'); - console.log(' 3. bob@deckhearth.com (User)'); + console.log('\n👥 Available Test Accounts:'); + console.log(' 1. admin@deckhearth.com / admin123 (Admin)'); + console.log(' 2. alice@deckhearth.com / alice123 (User)'); + console.log(' 3. bob@deckhearth.com / bob123 (User)'); } catch (error) { console.error('❌ Failed to create test users:', error.message); diff --git a/test/api/auth-utils.test.js b/test/api/auth-utils.test.js index 81f0ea5..88128b3 100644 --- a/test/api/auth-utils.test.js +++ b/test/api/auth-utils.test.js @@ -1,6 +1,10 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import jwt from 'jsonwebtoken'; +vi.mock('../../lib/database.js', () => ({ + db: { query: vi.fn() }, +})); + import { JWT_SECRET } from '../../lib/auth-secret.js'; import { generateToken, verifyToken } from '../../pages/api/auth-utils.js';