ad0e254324
Close stale convoy frontmatter for merged scanner, lint, and hygiene work; record P1 #11.5 and queued follow-ups as RESOLVED with PR references. Co-authored-by: Cursor <cursoragent@cursor.com>
560 lines
24 KiB
Markdown
560 lines
24 KiB
Markdown
---
|
|
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: shipped
|
|
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
|
|
|
|
Single squash commit `c403ea4` (PR #30, merged 2026-05-27T03:54:01Z
|
|
UTC / local 2026-05-26). Parent-owned end-to-end per the "Owns" line
|
|
— no architect, no implementer subagent dispatched. Mirror-the-pattern
|
|
fix exactly as planned; no mid-execution surprises that would have
|
|
forced an architect bounce. **AGENTS.md Gotcha #1 flipped from open
|
|
→ RESOLVED in the post-convoy doc-writer pass** (see
|
|
`.convoys/ship-readiness.md` § P1 #8 entry post-flip and the AGENTS.md
|
|
sweep in this same wave).
|
|
|
|
**Diff: 4 files, +447 / -64.** The 447-addition figure is dominated
|
|
by `.convoys/single-sql-client.md` (~330 lines for the planning
|
|
document, committed atomically with the fix). Actual source-file diff
|
|
is small: `pages/api/auth-utils.js` +5 / -7 (import swap + 2 query
|
|
shape conversions); `test/api/auth-utils.test.js` +1 / -5 (drop the
|
|
now-unused `vi.mock` + unused `vi` import); `lib/database.js` 0 / -47
|
|
(deletion).
|
|
|
|
**The change shipped exactly as designed:**
|
|
|
|
1. **`lib/database.js` deleted.** 47-line file; no replacement. The
|
|
`DatabaseAdapter` 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.
|
|
2. **`pages/api/auth-utils.js`** swept (the only source caller).
|
|
- Import: `import { db } from '../../lib/database.js'` → `import
|
|
{ sql } from '@vercel/postgres'`.
|
|
- `isAdmin(userId)`: `await db.query(\`SELECT role FROM users
|
|
WHERE id = $1\`, [userId])` → `` await sql`SELECT role FROM
|
|
users WHERE id = ${userId}` ``.
|
|
- `getUserById(userId)`: `await db.query(\`SELECT id, email, role,
|
|
created_at FROM users WHERE id = $1\`, [userId])` → `` 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.
|
|
- SQL is byte-equivalent (single-parameter numeric `userId`);
|
|
result shape is identical (`{ rows, rowCount }` from
|
|
`@vercel/postgres` matches what `DatabaseAdapter.query` was
|
|
returning).
|
|
3. **`test/api/auth-utils.test.js`** mock cleanup. Dropped
|
|
`vi.mock('../../lib/database.js', () => ({ db: { query: vi.fn() } }))`
|
|
(no longer needed because `auth-utils.js` no longer imports from
|
|
`lib/database.js`) and the unused `vi` import. The 5 test bodies
|
|
(2 `generateToken` + 3 `verifyToken`) are unchanged. Test count
|
|
stays at 21/21.
|
|
|
|
**`@neondatabase/serverless` retained as a runtime dep**, per D3 of
|
|
this convoy file. 11 `scripts/*` helpers still import `neon()`
|
|
directly (`scripts/setup-neon-db.js`, `scripts/migrations/2026-05-24-rename-admin-email.js`,
|
|
`scripts/reset-db.js`, plus 8 historical `add-*` / `fix-*` / `seed-*`
|
|
jobs); all out of scope per the no-go-zones rule. The dep-purge is
|
|
tracked as the newly queued `purge-neondatabase-serverless-fully`
|
|
follow-up (now **unblocked** by PR #32 `migration-tool` — the
|
|
migration helpers go through `node-pg-migrate`'s `pg` client, not
|
|
`@neondatabase/serverless`, so the only remaining direct `neon()`
|
|
consumers post-PR-#32 are `setup-neon-db.js`, `reset-db.js`, and the
|
|
historical graveyard).
|
|
|
|
**Verification (all gates green at merge):**
|
|
- `node --check pages/api/auth-utils.js` → exit 0
|
|
- `npm run lint` → 125 problems (post-PR-#31 baseline preserved;
|
|
zero regression). The `lib/database.js` deletion did not change
|
|
lint count because the file was already lint-clean.
|
|
- `npm run test:run` → 21/21 pass
|
|
- `rg "lib/database" --type js -l` → 0 hits anywhere (no `pages/**`,
|
|
no `lib/**`, no `scripts/**`, no `test/**`) — confirms no missed
|
|
importer (R2 mitigation)
|
|
- `rg "@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)
|
|
- CI on PR #30: Lint ✓ | Vitest 21/21 ✓ | Playwright smoke 3/3 ✓ |
|
|
`forbidden-endpoints` ✓ | `forbidden-cors-headers` ✓ | Vercel
|
|
preview deploy ✓ | Aggregate gate ✓
|
|
- **`Screenshot diff`: NOT triggered.** PR #30's diff is
|
|
`pages/api/**` + `lib/**` + `test/**` + `.convoys/**` — the
|
|
post-PR-#26 `!pages/api/**` exclusion correctly held. **This PR is
|
|
the first empirical confirmation that the
|
|
`tighten-visual-diff-path-filter` (PR #26, `ba95462`) exclusion
|
|
fires as documented** — the post-merge success criterion that
|
|
PR #26's § Verification plan deferred until "the next API-only PR
|
|
after this merges". `Screenshot diff` did NOT appear in PR #30's
|
|
Checks tab; the convoy-file as-shipped block of PR #26 now records
|
|
this confirmation.
|
|
|
|
**Live runtime smoke deferred** per § Verification plan — 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. Running a live curl smoke would require
|
|
seeding the admin user with a known password (operator-only flow per
|
|
`drop-public-setup`'s contract) and minting a token. The
|
|
byte-equivalent SQL semantics (D2) plus identical `try/catch` +
|
|
`result.rows[0]` access shape gives high confidence the migration is
|
|
correct; if a post-merge Vercel preview admin action 500s, rollback
|
|
is a single-commit revert.
|
|
|
|
**Cross-validation finding (continues the lineage).** The `Playwright
|
|
smoke` 3/3 PASS confirms the deployed preview is unaffected by the
|
|
lib-database deletion — the smoke spec doesn't exercise `isAdmin` /
|
|
`getUserById`, but the auth surface that smoke does exercise
|
|
(`/api/health`, sign-in render) is correctly insensitive to the
|
|
DatabaseAdapter removal. **Eighth consecutive convoy** where the
|
|
same 3-test smoke spec defends the auth surface through a sweeping
|
|
change (PR #15 → #19 → #20 → #21 → #25 → #32 → #27 → this PR).
|
|
|
|
**Operator action required going forward:** **none.** No env-var
|
|
change; no schema change; no infra change. The migration is purely a
|
|
client swap; the existing `POSTGRES_URL` contract is preserved
|
|
verbatim (both `lib/database.js` and `@vercel/postgres` read the same
|
|
`POSTGRES_URL`).
|
|
|
|
**Spec deviation:** none of substance. The convoy spec's "~3 files
|
|
based on graph" estimate was loose; architect grep (D1) confirmed
|
|
exactly 2 source-tree callers (1 source + 1 test mock). No deviation
|
|
flag needed; the convoy is even smaller than estimated. The post-flip
|
|
ship-readiness P1 #8 entry records this fact explicitly.
|
|
|
|
**Doc surface flipped atomically (in the post-convoy doc-writer pass):**
|
|
`AGENTS.md` Gotcha #1 → RESOLVED with the as-shipped paragraph
|
|
(tool deletion + retained-dep caveat + the 11-script `neon()` direct
|
|
consumers list); `AGENTS.md` § 3 "DB access" bullet trimmed (the
|
|
warning about `lib/database.js` is gone — the file doesn't exist);
|
|
`.cursor/rules/db-and-schema.mdc` § Clients section refreshed to
|
|
match (handled in the doc-writer commit alongside this convoy in
|
|
the 7-wave cleanup).
|
|
|
|
**Surfaced follow-ups (newly queued in `.convoys/ship-readiness.md`):**
|
|
- `purge-neondatabase-serverless-fully` (P3 polish; unblocked by PR #32).
|
|
- `lint-against-lib-database` (P3 polish; from this convoy's
|
|
pre-existing § Follow-ups list — would prevent re-introduction).
|
|
|
|
## 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).
|