Repository navigation
feat(db): Drizzle tooling, and the tables everything else hangs off - #44
Willi363363 wants to merge 1 commit into
Conversation
Steps 2.1 and 2.2 of phase 2, in one change: the first half of 2.1's criterion needs a table to migrate, since `drizzle-kit migrate` on an empty schema does nothing and proves nothing. One driver for every environment. `postgres.js` speaks plain Postgres, which is what Neon serves over TCP and what a container serves in a test, so the code that runs in production is the code the tests exercise. A separate serverless driver would mean two paths and only one of them tested; phase 9 revisits that if a deployment needs the HTTP driver. The Better Auth core schema was read rather than remembered, which was worth doing: `account` carries an `issuer` column in a compound unique with `account_id`, and I would have left it out. Phase 5 configures the adapter instead of migrating on arrival. `profile` is a separate table because `user` belongs to Better Auth: adding columns to it means the adapter and the migration disagree the day its core schema changes. The integration tests run against a real Postgres, migrated from the committed migrations — the pitfall being that a suite running against a database somebody already migrated proves the queries and nothing about the schema. They assert on SQLSTATE rather than on error text: `postgres.js` reports "Failed query: …" and keeps the class in a property, so matching the message would have passed for any failure at all, including a typo in the query. Drizzle wraps the driver's error, so the code is read by walking the `cause` chain. Absent locally, `DATABASE_URL` skips those tests; absent in CI, it fails them. A suite that quietly skips its integration tests on the machine that decides whether to merge reports green for work it never did. Two things `drizzle-kit` made necessary. It pulls in `tsx`, which is a peer of Vite, which gave pnpm reason to hand two packages two instances of Vitest — and `vitest.base.ts` exported a *type* across package boundaries, so every package re-exporting it stopped typechecking for a reason unrelated to its own code. The baseline is a plain object now. And drizzle writes files without a trailing newline, which the hygiene check refuses, so `generate` normalises them; its snapshots are kept out of Prettier, since drizzle reads them back to compute the next diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01533WTTbgmfCaDP5tmK8Gkv
PR Summary by QodoAdd Drizzle-based DB package, initial auth/profile schema, and CI migrations
AI Description
Diagram
High-Level Assessment
Files changed (25)
|
Code Review by Qodo
1. Windows migration generation fails
|
| import { readdirSync, readFileSync, statSync, writeFileSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
|
|
||
| const MIGRATIONS = new URL('../migrations/', import.meta.url).pathname; |
There was a problem hiding this comment.
1. Windows migration generation fails 🐞 Bug ☼ Reliability
The normalisation script passes new URL(...).pathname directly to Node filesystem APIs, which produces a URL pathname such as /C:/repo/... rather than a native Windows path. Consequently `pnpm --filter @wikifake/db generate` fails while scanning the generated migrations on Windows.
Agent Prompt
## Issue description
`normalise-migrations.ts` derives a filesystem directory with `URL.pathname`. On Windows this is not a valid native path, so the `generate` script fails during migration normalisation.
## Issue Context
Use Node's `fileURLToPath()` for file-URL-to-path conversion, matching the existing migration test helper.
## Fix Focus Areas
- packages/db/scripts/normalise-migrations.ts[8-11]
- packages/db/src/testing/database.ts[8-12]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Closed by the stack collapse, not abandoned. Every commit of this pull request is in Three commit messages were reworded on the way — a This description and its review thread stay readable here. |
Steps 2.1 and 2.2 of
plans/rewrite/phase-02-data.md. Phase 2 opens here.feat/rewrite-phase-2branches off the top of phase 1 (#43), since phase 2depends on it and nothing is merged yet. This is a step branch onto it.
Why the two steps are one change
2.1's criterion is "
drizzle-kit migrateruns on a fresh database". On anempty schema it does nothing and proves nothing, so the tooling is only really
verified by the first table. The phase sheet says so now.
One driver, one code path
postgres.jsspeaks plain Postgres — what Neon serves over TCP and what acontainer serves in a test. So the code that runs in production is the code
the tests exercise. A serverless HTTP driver alongside would mean two paths
with one of them tested; phase 9 revisits it if a deployment needs one.
The Better Auth schema was read, not remembered
Worth doing:
accountcarries anissuercolumn, in a compound unique withaccount_id, and I would have left it out. Getting that wrong means phase 5migrating on arrival instead of configuring an adapter.
profileis a separate table on purpose.userbelongs to Better Auth, andadding columns to it means the adapter and the migration disagree the day its
core schema changes.
The integration tests run against a real Postgres
Migrated from the committed migrations, on a fresh database. The pitfall the
sheet names is exactly this: a suite running against a database somebody already
migrated proves the queries and nothing about the schema.
They assert on SQLSTATE, not on error text:
My first version matched
/unique|duplicate/on the message and failed — theconstraints were working, but
postgres.jsreports"Failed query: …"and keepsthe class in a property. Matching the message would have passed for any
failure, including a typo in the query. Drizzle wraps the driver's error, so the
code is read by walking the
causechain rather than off the top error.Also asserted: a deleted account takes its sessions, provider links and profile
with it — a session pointing at nobody is a way in.
A skip that cannot hide in CI
DATABASE_URLabsent locally skips the integration tests; absent in CI itfails them. A suite that quietly skips its integration tests on the machine
that decides whether to merge reports green for work it never did.
CI gets a
postgres:17-alpineservice, and runsdrizzle-kit migratebeforethe tests — the exit gate asks for the CLI on a never-migrated database, and a
programmatic migrator passing is not the same proof.
Two things drizzle-kit made necessary
Vitest split in two.
drizzle-kitpulls intsx, which is a peer of Vite,which gave pnpm reason to hand two packages two instances of Vitest. Since
vitest.base.tsexported a type across package boundaries, every packagere-exporting it stopped typechecking — for a reason with nothing to do with its
own code:
The baseline is a plain object now. Vitest reads it identically, a typo still
fails loudly (no test found), and the type no longer crosses a package boundary.
Trailing newlines. drizzle writes files without one and the hygiene check
refuses that, so
generatenormalises them afterwards. Its snapshots are keptout of Prettier: drizzle reads them back to compute the next diff, and
reformatting one is how a migration chain starts disagreeing with the database it
describes.
Also fixed:
turbo.jsondid not passDATABASE_URLthrough, so the integrationtests were silently skipped under
pnpm testeven with the variable set.Caught because 8 tests reported as skipped when they should have run.
Checks
On Node 22, against a container Postgres 17, replaying CI's exact sequence on a
brand-new database:
Three new dependencies, all named by
00-overview.md's decision table:drizzle-orm,postgres, anddrizzle-kit(dev).Not run: the Python backend tests — no
pytesthere. This change touches noPython; CI covers them.
Next
2.3 (game tables, with the negative assertion on in-progress reads), 2.4 (audit),
2.5 (
llm_calland the cost queries), 2.6 (seed).🤖 Generated with Claude Code
https://claude.ai/code/session_01533WTTbgmfCaDP5tmK8Gkv