refactor(db): collapse @neondatabase/serverless onto @vercel/postgres + delete lib/database.js #30

Merged

varutasu merged 1 commit from convoy/single-sql-client into main

2026-05-26 23:54:01 -04:00

varutasu commented

2026-05-26 23:51:32 -04:00

(Migrated from github.com)

Summary

Convoy: single-sql-client (P1 quality, launch sequence step 8). Closes the dual SQL-client problem documented in AGENTS.md Gotcha #1 + .convoys/ship-readiness.md P1 #8 by deleting lib/database.js and migrating its sole production caller (pages/api/auth-utils.js) onto the canonical @vercel/postgres tagged-template surface. Keeps @neondatabase/serverless as a runtime dep for the 11 scripts/* helpers that use neon() directly (out of scope per the no-go-zones rule).

Decisions

D1 — Caller inventory: 2 files in scope (1 source + 1 test), not the "~3 based on graph" estimate in ship-readiness P1 #8. Only pages/api/auth-utils.js imports db; test/api/auth-utils.test.js mocks the module purely to satisfy the import graph (the 5 tests exercise generateToken / verifyToken, not isAdmin / getUserById).
D2 — Both call sites migrate to @vercel/postgres tagged templates with byte-equivalent SQL. Single-table SELECTs, single numeric parameter (userId), same { rows, rowCount } result shape, same try/catch + result.rows[0]?.role === 'admin' access pattern. 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/2026-05-24-rename-admin-email.js, reset-db.js, 8 historical add-* / fix-* / seed-* jobs). Migrating those is out of scope per the convoy spec + the no-go-zones rule; queued as purge-neondatabase-serverless-fully (blocked on migration-tool).
D4 — sql.unsafe audit: NOT a real injection vector with current callers. userId is sourced from a verified JWT (decoded.userId after verifyToken(token) succeeds), 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).

Caller inventory

File	Kind	Change
`pages/api/auth-utils.js`	source	imports `db`, calls `db.query` in `isAdmin(userId)` + `getUserById(userId)` → migrated to `sql\`…${userId}``
`test/api/auth-utils.test.js`	test	`vi.mock('../../lib/database.js', …)` → removed
`lib/database.js`	DELETED	47-line abstraction with manual `\$1` → string interpolation + `sql.unsafe(query)` — the foot-gun is gone
`.convoys/single-sql-client.md`	NEW	the convoy file (decisions, caller inventory, verification, risks, follow-ups)

Per-file changes

pages/api/auth-utils.js (+5 / -7):

swap import { db } from '../../lib/database.js' → import { sql } from '@vercel/postgres'
isAdmin: db.query(\SELECT role FROM users WHERE id = $1`, [userId])→sql`SELECT role FROM users WHERE id = ${userId}``
getUserById: same shape, same swap

test/api/auth-utils.test.js (+1 / -5):

drop vi.mock('../../lib/database.js', () => ({ db: { query: vi.fn() } }))
drop now-unused vi import

lib/database.js (0 / -47): DELETED.

.convoys/single-sql-client.md (+434 / 0): NEW convoy file.

Verification

npm run lint → 128 problems (81 errors, 47 warnings) — baseline preserved, no regression
npm run test:run → 21/21 pass (4 test files, 1.18s)
Grep "lib/database" --type js → 0 hits anywhere in JS sources
Grep "@neondatabase/serverless" --type js → still matches scripts/setup-neon-db.js, scripts/migrations/2026-05-24-rename-admin-email.js, scripts/reset-db.js, and 8 other scripts/add-* / fix-* / seed-* historical helpers (expected; out of scope per D3)
node --check pages/api/auth-utils.js → exit 0

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. Byte-equivalent SQL + identical result shape (D2) gives high confidence; rollback is a single-commit revert of this PR if a post-merge admin action 500s.

Risks

R1 — Byte-equivalence not guaranteed if lib/database.js has hidden behavior. Mitigation: architect re-read all 47 lines; only behavior beyond "interpolate, run SQL, return { rows, rowCount }" is string-quoting for string params, but both current callers pass numeric userId (quoting path not exercised). The raw() method is just an alias for query(); no caller invokes raw (Grep "\.raw\(" --type js → zero hits). Residual risk: very low.
R2 — Missed callers. Mitigation: post-delete Grep sweep (verification step 3) — if any file still imports lib/database, the file no longer exists and the import throws at module load, failing CI lint or test.
R3 — A future PR re-introduces lib/database.js or another sql.unsafe-shaped wrapper. Mitigation: documentation (this convoy file + AGENTS.md Gotcha #1 doc-writer flip in a follow-up cleanup pass). Stronger mitigation surfaced as queued follow-up lint-against-lib-database (ESLint no-restricted-imports).

Follow-ups (queued)

lint-against-lib-database (P3) — ESLint no-restricted-imports rule against lib/database re-introduction.
purge-neondatabase-serverless-fully (P3, blocked on migration-tool) — full dep purge once scripts adopt a single client.
add-neon-return-shape-rule (P3) — codify the neon() (returns [rows]) vs @vercel/postgres ({ rows: [...] }) return-shape difference; partially satisfied here because the dual shape is collapsed for pages/api/**.

Conflict-with-parallel-PRs note

Parallel P1 convoys in flight (single-auth-provider, purge-weak-creds-from-helpers, etc.) also touch pages/api/** and other files. This PR's source-side changes are confined to pages/api/auth-utils.js (2 function bodies + 1 import line). Conflicts at merge time should be minimal — the auth-utils import block doesn't overlap with the getUserFromRequest or other auth-context refactors that single-auth-provider is expected to make.

Test plan

npm run lint → 128 problems (baseline)
npm run test:run → 21/21 pass
Grep verifies no remaining lib/database reference in JS sources
Grep verifies @neondatabase/serverless still matches only the 11 scripts/* sites (D3 scope)
CI gates (Lint / Vitest / Playwright smoke / forbidden-endpoints / forbidden-cors-headers / Vercel preview) all green
Optional post-merge: admin user exercises the admin page (/admin) to confirm isAdmin works end-to-end

Made with Cursor

## Summary Convoy: `single-sql-client` (P1 quality, launch sequence step 8). Closes the dual SQL-client problem documented in AGENTS.md Gotcha #1 + `.convoys/ship-readiness.md` P1 #8 by deleting `lib/database.js` and migrating its sole production caller (`pages/api/auth-utils.js`) onto the canonical `@vercel/postgres` tagged-template surface. Keeps `@neondatabase/serverless` as a runtime dep for the 11 `scripts/*` helpers that use `neon()` directly (out of scope per the no-go-zones rule). ## Decisions - **D1** — Caller inventory: 2 files in scope (1 source + 1 test), not the "~3 based on graph" estimate in ship-readiness P1 #8. Only `pages/api/auth-utils.js` imports `db`; `test/api/auth-utils.test.js` mocks the module purely to satisfy the import graph (the 5 tests exercise `generateToken` / `verifyToken`, not `isAdmin` / `getUserById`). - **D2** — Both call sites migrate to `@vercel/postgres` tagged templates with byte-equivalent SQL. Single-table SELECTs, single numeric parameter (`userId`), same `{ rows, rowCount }` result shape, same `try/catch` + `result.rows[0]?.role === 'admin'` access pattern. 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/2026-05-24-rename-admin-email.js`, `reset-db.js`, 8 historical `add-*` / `fix-*` / `seed-*` jobs). Migrating those is out of scope per the convoy spec + the no-go-zones rule; queued as `purge-neondatabase-serverless-fully` (blocked on `migration-tool`). - **D4** — `sql.unsafe` audit: **NOT a real injection vector** with current callers. `userId` is sourced from a verified JWT (`decoded.userId` after `verifyToken(token)` succeeds), 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). ## Caller inventory | File | Kind | Change | | --- | --- | --- | | `pages/api/auth-utils.js` | source | imports `db`, calls `db.query` in `isAdmin(userId)` + `getUserById(userId)` → migrated to `sql\`…${userId}\`` | | `test/api/auth-utils.test.js` | test | `vi.mock('../../lib/database.js', …)` → removed | | `lib/database.js` | DELETED | 47-line abstraction with manual `\$1` → string interpolation + `sql.unsafe(query)` — the foot-gun is gone | | `.convoys/single-sql-client.md` | NEW | the convoy file (decisions, caller inventory, verification, risks, follow-ups) | ## Per-file changes **`pages/api/auth-utils.js`** (+5 / -7): - swap `import { db } from '../../lib/database.js'` → `import { sql } from '@vercel/postgres'` - `isAdmin`: `db.query(\`SELECT role FROM users WHERE id = \$1\`, [userId])` → `sql\`SELECT role FROM users WHERE id = \${userId}\`` - `getUserById`: same shape, same swap **`test/api/auth-utils.test.js`** (+1 / -5): - drop `vi.mock('../../lib/database.js', () => ({ db: { query: vi.fn() } }))` - drop now-unused `vi` import **`lib/database.js`** (0 / -47): DELETED. **`.convoys/single-sql-client.md`** (+434 / 0): NEW convoy file. ## Verification - `npm run lint` → **128 problems (81 errors, 47 warnings)** — baseline preserved, no regression - `npm run test:run` → **21/21 pass** (4 test files, 1.18s) - `Grep "lib/database" --type js` → **0 hits** anywhere in JS sources - `Grep "@neondatabase/serverless" --type js` → still matches `scripts/setup-neon-db.js`, `scripts/migrations/2026-05-24-rename-admin-email.js`, `scripts/reset-db.js`, and 8 other `scripts/add-*` / `fix-*` / `seed-*` historical helpers (expected; out of scope per D3) - `node --check pages/api/auth-utils.js` → exit 0 **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. Byte-equivalent SQL + identical result shape (D2) gives high confidence; rollback is a single-commit revert of this PR if a post-merge admin action 500s. ## Risks - **R1** — Byte-equivalence not guaranteed if `lib/database.js` has hidden behavior. **Mitigation:** architect re-read all 47 lines; only behavior beyond "interpolate, run SQL, return `{ rows, rowCount }`" is string-quoting for string params, but both current callers pass numeric `userId` (quoting path not exercised). The `raw()` method is just an alias for `query()`; no caller invokes `raw` (`Grep "\.raw\(" --type js` → zero hits). Residual risk: very low. - **R2** — Missed callers. **Mitigation:** post-delete `Grep` sweep (verification step 3) — if any file still imports `lib/database`, the file no longer exists and the import throws at module load, failing CI lint or test. - **R3** — A future PR re-introduces `lib/database.js` or another `sql.unsafe`-shaped wrapper. **Mitigation:** documentation (this convoy file + AGENTS.md Gotcha #1 doc-writer flip in a follow-up cleanup pass). Stronger mitigation surfaced as queued follow-up `lint-against-lib-database` (ESLint `no-restricted-imports`). ## Follow-ups (queued) - **`lint-against-lib-database`** (P3) — ESLint `no-restricted-imports` rule against `lib/database` re-introduction. - **`purge-neondatabase-serverless-fully`** (P3, blocked on `migration-tool`) — full dep purge once scripts adopt a single client. - **`add-neon-return-shape-rule`** (P3) — codify the `neon()` (returns `[rows]`) vs `@vercel/postgres` (`{ rows: [...] }`) return-shape difference; partially satisfied here because the dual shape is collapsed for `pages/api/**`. ## Conflict-with-parallel-PRs note Parallel P1 convoys in flight (`single-auth-provider`, `purge-weak-creds-from-helpers`, etc.) also touch `pages/api/**` and other files. This PR's source-side changes are confined to **`pages/api/auth-utils.js`** (2 function bodies + 1 import line). Conflicts at merge time should be minimal — the auth-utils import block doesn't overlap with the `getUserFromRequest` or other auth-context refactors that `single-auth-provider` is expected to make. ## Test plan - [x] `npm run lint` → 128 problems (baseline) - [x] `npm run test:run` → 21/21 pass - [x] Grep verifies no remaining `lib/database` reference in JS sources - [x] Grep verifies `@neondatabase/serverless` still matches only the 11 `scripts/*` sites (D3 scope) - [ ] CI gates (Lint / Vitest / Playwright smoke / forbidden-endpoints / forbidden-cors-headers / Vercel preview) all green - [ ] Optional post-merge: admin user exercises the admin page (`/admin`) to confirm `isAdmin` works end-to-end Made with [Cursor](https://cursor.com)

vercel[bot] commented

2026-05-26 23:51:38 -04:00

(Migrated from github.com)

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
tcg-vault	Ready	Preview, Comment	May 27, 2026 3:51am

[vc]: #9JMEZHKsvMK2g6onwzjb1Ptz9ARtsRAffWfwDXTV7zc=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWNvbnZveS1zaW5nbGUtMjkwMDUyLXJhbmRhbGwtc3RpbGx3ZWxscy1wcm9qZWN0cy52ZXJjZWwuYXBwIn0sImluc3BlY3RvclVybCI6Imh0dHBzOi8vdmVyY2VsLmNvbS9yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMvdGNnLXZhdWx0L0JrZkRXbzJjcU5TWmJYYm5DVDV4NlJxTjR0dFYiLCJwcmV2aWV3VXJsIjoidGNnLXZhdWx0LWdpdC1jb252b3ktc2luZ2xlLTI5MDA1Mi1yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMudmVyY2VsLmFwcCIsIm5leHRDb21taXRTdGF0dXMiOiJERVBMT1lFRCJ9XSwicmVxdWVzdFJldmlld1VybCI6Imh0dHBzOi8vdmVyY2VsLmNvbS92ZXJjZWwtYWdlbnQvcmVxdWVzdC1yZXZpZXc/b3duZXI9dmFydXRhc3UmcmVwbz10Y2ctdmF1bHQmcHI9MzAifQ== The latest updates on your projects. Learn more about [Vercel for GitHub](https://vercel.link/github-learn-more). | Project | Deployment | Actions | Updated (UTC) | | :--- | :----- | :------ | :------ | | [tcg-vault](https://vercel.com/randall-stillwells-projects/tcg-vault) | ![Ready](https://vercel.com/static/status/ready.svg) [Ready](https://vercel.com/randall-stillwells-projects/tcg-vault/BkfDWo2cqNSZbXbnCT5x6RqN4ttV) | [Preview](https://tcg-vault-git-convoy-single-290052-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-convoy-single-290052-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 27, 2026 3:51am | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=30" rel="noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://agents-vade-review.vercel.sh/request-review-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://agents-vade-review.vercel.sh/request-review-light.svg"><img src="https://agents-vade-review.vercel.sh/request-review-light.svg" alt="Request Review"></picture></a>

github-actions[bot] commented

2026-05-26 23:51:41 -04:00

(Migrated from github.com)

Pipeline Health

Build + CI gates

Gate	Status
Vercel build (Preview)	✅ pass
CI: Lint	✅ pass
CI: Schema map fresh	❌ skipped
Preview smoke	✅ pass
Visual diff	⏭ skipped or pending

Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build).

Role reports

Role	Status
Reviewer report	⏳ pending
A11y audit	⏳ pending
Design system audit	⏳ pending

See individual comments above for details. This rollup updates automatically.

## Pipeline Health ### Build + CI gates | Gate | Status | | --- | --- | | Vercel build (Preview) | ✅ pass | | CI: Lint | ✅ pass | | CI: Schema map fresh | ❌ skipped | | Preview smoke | ✅ pass | | Visual diff | ⏭ skipped or pending | _Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build)._ ### Role reports | Role | Status | | --- | --- | | Reviewer report | ⏳ pending | | A11y audit | ⏳ pending | | Design system audit | ⏳ pending | See individual comments above for details. This rollup updates automatically.