From cc99e74c899e68c2d2268226c02fee98707ad8b4 Mon Sep 17 00:00:00 2001
From: Randall Stillwell <me@randallstillwell.com>
Date: Tue, 26 May 2026 22:50:58 -0500
Subject: [PATCH 1/2] refactor(db): collapse @neondatabase/serverless onto
 @vercel/postgres + delete lib/database.js
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Convoy: single-sql-client (P1 quality, launch sequence step 8)
Addresses: AGENTS.md Gotcha #1, .convoys/ship-readiness.md P1 #8

## Decisions

- D1: Caller inventory = 2 files (1 source + 1 test), not "~3 based on graph".
  Only pages/api/auth-utils.js imports `db`; test/api/auth-utils.test.js mocks
  it purely to satisfy the import graph (the 5 tests exercise
  generateToken/verifyToken, not isAdmin/getUserById).
- D2: Migrate both call sites (isAdmin, getUserById) to @vercel/postgres
  tagged-template SQL. Queries are SELECT-only, single-table,
  single-numeric-parameter — byte-equivalent translation; same result shape
  ({rows, rowCount}); no transaction or pool semantics differ.
- D3: KEEP @neondatabase/serverless as a dep. 11 scripts/* files still use
  `neon()` directly (setup-neon-db.js, migrations/, reset-db.js, 8 historical
  add-*/fix-*/seed-* jobs). They are out of scope per the no-go-zones rule
  and the convoy spec; purging the dep entirely would be its own convoy
  (queued as `purge-neondatabase-serverless-fully`, blocked on migration-tool).
- D4: sql.unsafe audit — NOT a real injection vector with current callers
  (userId comes from a verified JWT, is a numeric SERIAL id). Security
  finding: NO. Pure refactor + foot-gun removal that prevents the FUTURE
  caller that would have been the incident.
- D5: Test mock cleanup — drop the now-unneeded `vi.mock('../../lib/database.js')`
  call + unused `vi` import. Test count + assertions unchanged (5/5).

## Per-file changes

- pages/api/auth-utils.js: swap `import { db } from '../../lib/database.js'`
  for `import { sql } from '@vercel/postgres'`; rewrite isAdmin's
  `db.query(SELECT … WHERE id = $1, [userId])` and getUserById's same shape
  to `sql\`SELECT … WHERE id = ${userId}\``. Same try/catch, same
  result.rows[0] access, same error returns.
- test/api/auth-utils.test.js: drop vi.mock for lib/database.js + the unused
  `vi` import. 5/5 tests still pass.
- lib/database.js: DELETED (47 lines removed; manual-interpolation + sql.unsafe
  wrapper is gone).
- .convoys/single-sql-client.md: NEW (the convoy file documenting all
  decisions + caller inventory + verification + risks + follow-ups).

## Verification

- npm run lint → 128 problems (baseline preserved, no regression)
- npm run test:run → 21/21 pass (vitest)
- Grep "lib/database" --type js -l → 0 hits anywhere
- Grep "@neondatabase/serverless" --type js -l → still matches the 11
  scripts/* sites (expected; out of scope per D3)
- node --check pages/api/auth-utils.js → exit 0

## Scope note

This convoy collapses the lib/database.js abstraction onto the canonical
@vercel/postgres surface for pages/api/**. It does NOT eliminate
@neondatabase/serverless from the dependency tree — that would require
migrating the scripts/* helpers, which is out of scope here (no-go-zones
rule + convoy spec). Queued as a follow-up.

## Live smoke

Deferred. The two migrated functions (isAdmin, getUserById) are only
reachable via pages/api/admin/index.js which requires an admin Bearer
token and a populated users table in prod Neon. Byte-equivalent SQL +
identical result shape gives high confidence; rollback is a single-commit
revert if a post-merge admin action 500s.

Co-authored-by: Cursor <cursoragent@cursor.com>
---
 .convoys/single-sql-client.md | 434 ++++++++++++++++++++++++++++++++++
 lib/database.js               |  47 ----
 pages/api/auth-utils.js       |  24 +-
 test/api/auth-utils.test.js   |   6 +-
 4 files changed, 447 insertions(+), 64 deletions(-)
 create mode 100644 .convoys/single-sql-client.md
 delete mode 100644 lib/database.js

diff --git a/.convoys/single-sql-client.md b/.convoys/single-sql-client.md
new file mode 100644
index 0000000..c5c97b5
--- /dev/null
+++ b/.convoys/single-sql-client.md
@@ -0,0 +1,434 @@
+---
+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/lib/database.js b/lib/database.js
deleted file mode 100644
index 8d0b00a..0000000
--- a/lib/database.js
+++ /dev/null
@@ -1,47 +0,0 @@
-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/api/auth-utils.js b/pages/api/auth-utils.js
index 3a92321..30eed94 100644
--- a/pages/api/auth-utils.js
+++ b/pages/api/auth-utils.js
@@ -1,5 +1,5 @@
 import jwt from 'jsonwebtoken';
-import { db } from '../../lib/database.js';
+import { sql } from '@vercel/postgres';
 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 db.query(`
-      SELECT role FROM users WHERE id = $1
-    `, [userId]);
+    const result = await sql`
+      SELECT role FROM users WHERE id = ${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 db.query(`
-      SELECT id, email, role, created_at FROM users WHERE id = $1
-    `, [userId]);
+    const result = await sql`
+      SELECT id, email, role, created_at FROM users WHERE id = ${userId}
+    `;
     return result.rows[0];
   } catch (error) {
     console.error('Error getting user:', error);
     return null;
   }
-} 
\ No newline at end of file
+}
diff --git a/test/api/auth-utils.test.js b/test/api/auth-utils.test.js
index 88128b3..81f0ea5 100644
--- a/test/api/auth-utils.test.js
+++ b/test/api/auth-utils.test.js
@@ -1,10 +1,6 @@
-import { describe, expect, it, vi } from 'vitest';
+import { describe, expect, it } 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';
 
-- 
2.45.2


From ba954629ed7f1e8165add48f35ad5dc0e226bdc5 Mon Sep 17 00:00:00 2001
From: varutasu <104105839+varutasu@users.noreply.github.com>
Date: Tue, 26 May 2026 22:51:22 -0500
Subject: [PATCH 2/2] ci(workflows): exclude pages/api/** from visual-diff path
 filter (#26)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `Screenshot diff` workflow's `paths:` filter included `pages/**`
which matches `pages/api/**` too, so API-only PRs triggered the
visual-diff workflow even though they can't possibly move a single
rendered pixel. PR #19 (cors-tighten) and PR #20 (add-rate-limiting)
both empirically hit this — each was an API-only sweep, and each
burned ~55s of CI runtime on a `Screenshot diff` job that
`continue-on-error: true` then swallowed. Documented as a queued
follow-up in `.convoys/ship-readiness.md` § Queued convoys, ratified
for fix in this convoy.

The fix is a single negated-glob entry inserted immediately after
`pages/**` in the `paths:` list. GitHub Actions evaluates `paths:`
with minimatch and supports `!`-prefixed exclusions per the
published path-filter cheatsheet, but the order matters: a `!pattern`
only takes effect if it appears AFTER an include that already
matched the path. Keeping `!pages/api/**` second in the list (right
after `pages/**`, before all the other includes) is the canonical
shape. All five existing entries are preserved verbatim; only the
one exclusion entry plus an inline comment explaining the ordering
rule and the empirical motivation is added.

`preview-smoke.yml` is intentionally untouched — verified its `on:`
block has no `paths:` filter at all (it triggers on every PR
targeting main, with skip-via-PR-body-directive in the gate job),
so there's no false-positive shape to fix there. Smoke SHOULD run on
every PR including API-only ones because changes to `pages/api/**`
can break the home redirect + sign-in + `/api/health` endpoints the
smoke spec exercises.

Co-authored-by: Cursor <cursoragent@cursor.com>
---
 .convoys/tighten-visual-diff-path-filter.md | 270 ++++++++++++++++++++
 .github/workflows/visual-diff.yml           |   9 +
 2 files changed, 279 insertions(+)
 create mode 100644 .convoys/tighten-visual-diff-path-filter.md

diff --git a/.convoys/tighten-visual-diff-path-filter.md b/.convoys/tighten-visual-diff-path-filter.md
new file mode 100644
index 0000000..7c0796e
--- /dev/null
+++ b/.convoys/tighten-visual-diff-path-filter.md
@@ -0,0 +1,270 @@
+# 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/<non-api>/<api-like>` 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: `<TBD>`
+- PR: `<TBD>`
+- Diff stat: `<TBD>` (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/.github/workflows/visual-diff.yml b/.github/workflows/visual-diff.yml
index f6771e9..744233c 100644
--- a/.github/workflows/visual-diff.yml
+++ b/.github/workflows/visual-diff.yml
@@ -9,6 +9,15 @@ 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'
-- 
2.45.2