From 6af80a20b0bd966c12032a582057a7e5d69f5df0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 4 Aug 2026 14:22:50 +0000 Subject: [PATCH] db: add the user role column, and write down how push actually behaves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `role` to users as a text column with a TS enum, matching how `status` is done — no pgEnum, so adding a role stays a one-line schema change rather than an ALTER TYPE while the set is still settling. USER_ROLES and UserRole are exported from officerdb so the API and UI enumerate them from the column definition rather than a second hand-written list. Defaults to 'Member', the least privileged, so a row created by a path that does not think about authorisation cannot mint an admin. NOT YET PUSHED — the column is in the schema, not in the database. The larger half of this commit is a rule for anyone running `bun db:push`, because the first time you run it, it looks like your change broke something. It plans 16 statements against a database that already matches the schema, and plans them again on the next run. Two separate causes, both diagnosed here: - it drops and re-adds every NAMED COMPOSITE unique constraint — all 14, from uq_screens_user_name to uq_wallet_labels_wallet_kind_ref. Single-column .unique() diffs correctly and is untouched; only unique('name').on(a, b) is affected. Names, columns and order in the database are identical to what the schema declares. drizzle-kit 0.31.9 / drizzle-orm 0.45.1. - it drops and re-adds one foreign key because user_integrations_server_integration_id_server_integrations_id_fk is 65 characters and Postgres truncates identifiers at 63, so drizzle compares its generated name against the stored, truncated one and always differs. The rules, ranked by how much damage getting them wrong does: never answer "Yes, truncate the table" — it destroys rows and does not help, since the constraint is re-added next push either way; never delete a constraint from the schema to silence the prompt, because upserts depend on it existing; do not reach for --force until someone has established on a scratch database which branch it takes. Run with --verbose first and read what it is actually planning. Pointers added to platform/CLAUDE.md and the workspace root CLAUDE.md, since those are what a session reads before it ever opens the database directory. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 5 +++ src/databases/CLAUDE.md | 40 +++++++++++++++++++++ src/databases/officer_db/src/index.ts | 5 +++ src/databases/officer_db/src/schema/auth.ts | 15 ++++++++ 4 files changed, 65 insertions(+) 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'),