diff --git a/CLAUDE.md b/CLAUDE.md index a1420893..742d8195 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,6 +100,11 @@ database and alters it directly. **`drizzle-kit migrate` has never been run here history was deleted because it had drifted from the real schema. Treat the schema code, not those files, as the source of truth. +**`push` is interactive and asks the same question on every run** — it plans 16 statements against a +database that already matches the schema, because drizzle-kit mis-diffs named composite unique +constraints. **Never answer "Yes, truncate the table", and never delete a constraint from the schema to +silence it.** Read `src/databases/CLAUDE.md` → "push is interactive" before running it. + ## Security Model The perimeter is one credential, so the guards matter: diff --git a/src/databases/CLAUDE.md b/src/databases/CLAUDE.md index 6ac33e41..b3ee3c0b 100644 --- a/src/databases/CLAUDE.md +++ b/src/databases/CLAUDE.md @@ -49,6 +49,46 @@ check( Adding a constraint fails while existing rows violate it. That is the point: push refusing tells you the database has drifted, instead of quietly accepting it. Clean the rows, then push. +### `push` is interactive, and it asks the same question every time. Do not "fix" it. + +**Read this before running `bun db:push`.** It plans 16 statements on a database that already matches the +schema, and it will plan them again on the next push, and the one after. This is a drizzle-kit diffing +bug, not drift you introduced and not something your change caused. + +It drops and re-adds **every named composite unique constraint** — all 14 of them, `uq_screens_user_name` +through `uq_wallet_labels_wallet_kind_ref`. Single-column `.unique()` is diffed correctly and left alone; +only the `unique('name').on(a, b)` form is affected. Verified on drizzle-kit 0.31.9 / drizzle-orm 0.45.1: +the names, columns and column order in the database are identical to what the schema declares. + +It also drops and re-adds one foreign key, for a different and fully understood reason — +`user_integrations_server_integration_id_server_integrations_id_fk` is **65 characters**, Postgres +truncates identifiers at **63**, so drizzle compares its generated name against the stored, truncated one +and always sees a difference. + +**The rules, in order of how much damage getting them wrong does:** + +1. **Never answer "Yes, truncate the table."** The prompt appears because `ADD UNIQUE` against a + populated table is a data-risk statement. Truncating destroys the rows AND does not help: the + constraint is dropped and re-added on the next push regardless of whether the table is empty. + Always take `No, add the constraint without truncating the table` — it is the highlighted default, + and it succeeds whenever the data has no duplicates. +2. **Never delete a constraint from the schema to silence the prompt.** The schema is right; the diff is + wrong. Removing `unique(...)` to make push quiet would drop a real constraint that upserts depend on — + `onConflictDoUpdate({ target: [...] })` requires it to exist. +3. **Do not reach for `--force`.** It auto-accepts data-loss statements, and which branch it takes at the + truncate prompt has not been established here. Find out on a scratch database before ever pointing it + at `officer_dev`. +4. **Check what it is actually planning** before answering anything: + `bunx drizzle-kit push --config=drizzle.config.ts --verbose` prints every statement first. Expect the + 16 above. Anything else is your change, and worth reading. + +Because of all this, push cannot currently be automated or run unattended — it needs a TTY. Piping input +does not work; the prompt reads the terminal directly. + +**If you are fixing this properly**, the leads are: name the over-long foreign key explicitly so it fits +in 63 characters, and try `uniqueIndex('name').on(a, b)` in place of `unique('name').on(a, b)` — indexes +are diffed on a different code path and may sidestep the bug. Test on a scratch database, not this one. + ## Type naming ```ts diff --git a/src/databases/officer_db/src/index.ts b/src/databases/officer_db/src/index.ts index c47f6e6d..bb0ddc3f 100644 --- a/src/databases/officer_db/src/index.ts +++ b/src/databases/officer_db/src/index.ts @@ -217,6 +217,11 @@ export type { } from './queries/wallet'; export type { WalletChainSnapshot } from './schema/wallet'; +// Exported as a value, not just a type: the API and the UI need to enumerate the roles, and the +// column definition is the only place that list should exist. +export { USER_ROLES } from './schema/auth'; +export type { UserRole } from './schema/auth'; + export { db } from './db'; export * as schema from './schema'; diff --git a/src/databases/officer_db/src/schema/auth.ts b/src/databases/officer_db/src/schema/auth.ts index 0da3e526..b6bb32cd 100644 --- a/src/databases/officer_db/src/schema/auth.ts +++ b/src/databases/officer_db/src/schema/auth.ts @@ -1,5 +1,17 @@ import { pgTable, serial, text, integer, timestamp, index } from 'drizzle-orm/pg-core'; +// Roles, in descending authority. Exported as a value so the API and the UI can enumerate them from +// the same place the column is defined — a second hand-written list of role names is how they drift. +// +// A text column with a TS enum rather than a Postgres enum type, matching `status` below: the set is +// still settling, and adding a role should be a one-line schema change that `db:push` applies, not an +// ALTER TYPE. Postgres will not reject an unknown string — the TS type is the guard. +// +// 'Super Admin' is the server owner. There is exactly one, and it is not a role that gets handed out: +// see super-admin.ts, which still resolves the owner independently of this column. +export const USER_ROLES = ['Super Admin', 'Admin', 'Member', 'Developer'] as const; +export type UserRole = (typeof USER_ROLES)[number]; + export const users = pgTable('users', { id: serial('id').primaryKey(), email: text('email').notNull().unique(), @@ -7,6 +19,9 @@ export const users = pgTable('users', { status: text('status', { enum: ['Unverified', 'Active', 'Prospect', 'Invited', 'Blocked', 'Banned', 'Deleted'] }) .notNull() .default('Unverified'), + // Defaults to the least privileged role so a row created by any path that does not think about + // authorisation cannot accidentally mint an admin. + role: text('role', { enum: USER_ROLES }).notNull().default('Member'), name: text('name'), username: text('username').unique(), avatar: text('avatar'),