diff --git a/CLAUDE.md b/CLAUDE.md index 742d8195..61f3840e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,10 +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. +**Declare multi-column uniqueness as `uniqueIndex('uq_…').on(a, b)`, never `unique('uq_…').on(a, b)`** — +drizzle-kit mis-diffs named composite unique *constraints* and re-creates them on every push, which used +to stop `db:push` on an unanswerable truncate prompt. Same for any foreign key whose generated name would +exceed Postgres's 63-character identifier limit: name it explicitly. See `src/databases/CLAUDE.md` → +"Composite keys" before adding either. ## Security Model diff --git a/src/databases/CLAUDE.md b/src/databases/CLAUDE.md index b3ee3c0b..5b2902a5 100644 --- a/src/databases/CLAUDE.md +++ b/src/databases/CLAUDE.md @@ -49,45 +49,53 @@ 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. +### Composite keys: use `uniqueIndex`, and name any long foreign key -**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. +**Declare a multi-column uniqueness rule as `uniqueIndex('uq_…').on(a, b)`, never `unique('uq_…').on(a, b)`.** -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. +drizzle-kit mis-diffs named composite unique CONSTRAINTS. It reads them back from the database, compares +them against a schema that declares exactly the same name, columns and order, decides they differ, and +emits a `DROP CONSTRAINT` + `ADD CONSTRAINT` pair — on every push, forever. Fifteen of them made +`db:push` plan 32 statements against a database that already matched, and `ADD UNIQUE` on a populated +table is a data-risk statement, so push stopped on an interactive *"do you want to truncate?"* prompt +that could never be resolved by answering it. -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. +Reproduced on a database drizzle had itself created seconds earlier, so it is not drift and not +something your change caused. Single-column `.unique()` is diffed correctly and is unaffected — only the +table-level `unique('name').on(...)` form. Unique indexes are diffed on a different code path and are +stable. Verified on drizzle-kit 0.31.9 / drizzle-orm 0.45.1. -**The rules, in order of how much damage getting them wrong does:** +A unique index enforces exactly what the constraint did, and `onConflictDoUpdate({ target: [...] })` +accepts it as an arbiter. The one thing it cannot do is be the target of a foreign key — Postgres +requires a unique *constraint* there. Nothing here has a composite foreign-key target; check before +adding one. -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: +**Name a foreign key explicitly when drizzle's generated name would exceed 63 characters.** Postgres +truncates identifiers at 63 and stores the shortened form, so drizzle keeps comparing against its own +longer version and re-creates the constraint on every push. +`user_integrations_server_integration_id_server_integrations_id_fk` was 65, and is now declared with +`foreignKey({ name: 'fk_user_integrations_server_integration', … })`. + +**Known remaining churn, harmless:** a composite `primaryKey` has the same diffing bug and there is no +index form to escape to — a primary key must be a constraint. `music_now_playing` therefore drops and +re-adds `pk_music_now_playing` on every push. Two statements, silent, no prompt even with rows in the +table, data unaffected. Naming it explicitly does not help. Leave it. + +**Rules that still apply:** + +1. **Never answer "Yes, truncate the table"** if a prompt ever does appear. It destroys rows and fixes + nothing — whatever is being re-created gets re-created next push regardless. +2. **Never delete a constraint from the schema to quiet a diff.** The schema is right. Removing a + uniqueness rule would break the upserts that depend on it. +3. **Do not use `--force`.** It auto-accepts data-loss statements and nobody has established which + branch it takes at a truncate prompt. +4. **Read the plan before applying it:** `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. + two `pk_music_now_playing` lines. Anything else is your change. -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. +To test a schema change without touching `officer_dev`: `createdb officer_scratch`, then +`POSTGRES_URL=postgresql://postgres:postgres@127.0.0.1:5432/officer_scratch bunx drizzle-kit push +--config=drizzle.config.ts`. Push twice — the second run tells you whether your declaration is stable. ## Type naming diff --git a/src/databases/officer_db/src/schema/dashboards.ts b/src/databases/officer_db/src/schema/dashboards.ts index 22c72672..58498a86 100644 --- a/src/databases/officer_db/src/schema/dashboards.ts +++ b/src/databases/officer_db/src/schema/dashboards.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, text, integer, timestamp, jsonb, unique } from 'drizzle-orm/pg-core'; +import { pgTable, serial, text, integer, timestamp, jsonb, uniqueIndex } from 'drizzle-orm/pg-core'; import { users } from './auth'; export const dashboards = pgTable( @@ -17,7 +17,7 @@ export const dashboards = pgTable( createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, - (table) => [unique('uq_dashboards_user_id').on(table.userId, table.id)], + (table) => [uniqueIndex('uq_dashboards_user_id').on(table.userId, table.id)], ); export const screens = pgTable( @@ -33,7 +33,7 @@ export const screens = pgTable( hostTerminals: jsonb('host_terminals').notNull().default({}), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, - (table) => [unique('uq_screens_user_name').on(table.userId, table.name)], + (table) => [uniqueIndex('uq_screens_user_name').on(table.userId, table.name)], ); export const dashboardDefaults = pgTable('dashboard_defaults', { diff --git a/src/databases/officer_db/src/schema/email.ts b/src/databases/officer_db/src/schema/email.ts index 8c7faf84..6512f183 100644 --- a/src/databases/officer_db/src/schema/email.ts +++ b/src/databases/officer_db/src/schema/email.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, boolean, timestamp, jsonb, unique } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, boolean, timestamp, jsonb, uniqueIndex } from 'drizzle-orm/pg-core'; import { users } from './auth'; export const emailAccounts = pgTable( @@ -22,5 +22,5 @@ export const emailAccounts = pgTable( syncMeta: jsonb('sync_meta').notNull().default({}), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, - (table) => [unique('uq_email_accounts_user_email').on(table.userId, table.email)], + (table) => [uniqueIndex('uq_email_accounts_user_email').on(table.userId, table.email)], ); diff --git a/src/databases/officer_db/src/schema/headscale.ts b/src/databases/officer_db/src/schema/headscale.ts index ce333771..fa23a1a2 100644 --- a/src/databases/officer_db/src/schema/headscale.ts +++ b/src/databases/officer_db/src/schema/headscale.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, boolean, timestamp, unique, uniqueIndex } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, boolean, timestamp, uniqueIndex } from 'drizzle-orm/pg-core'; import { sql } from 'drizzle-orm'; import { users } from './auth'; @@ -38,7 +38,7 @@ export const headscaleServers = pgTable( }, (t) => [ // One registration per URL — re-registering the same server should be an edit, not a duplicate. - unique('uq_headscale_servers_user_url').on(t.userId, t.url), + uniqueIndex('uq_headscale_servers_user_url').on(t.userId, t.url), // At most one active server per owner, enforced by the DB rather than by convention: a partial unique // index over the active rows only. setActiveHeadscaleServer still clears the others in a transaction, // but a bug there fails loudly here instead of silently leaving two servers active. diff --git a/src/databases/officer_db/src/schema/invoiceshelf.ts b/src/databases/officer_db/src/schema/invoiceshelf.ts index 58702917..d8bff028 100644 --- a/src/databases/officer_db/src/schema/invoiceshelf.ts +++ b/src/databases/officer_db/src/schema/invoiceshelf.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, boolean, timestamp, unique, uniqueIndex } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, boolean, timestamp, uniqueIndex } from 'drizzle-orm/pg-core'; import { sql } from 'drizzle-orm'; import { users } from './auth'; @@ -47,7 +47,7 @@ export const invoiceshelfAccounts = pgTable( updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, (t) => [ - unique('uq_invoiceshelf_accounts_user_label').on(t.userId, t.label), + uniqueIndex('uq_invoiceshelf_accounts_user_label').on(t.userId, t.label), // At most one active account per owner, enforced by the DB rather than by convention: a partial unique // index over the active rows only. setActiveInvoiceshelfAccount still clears the others in a transaction, // but a bug there fails loudly here instead of silently leaving two active and the UI picking one. diff --git a/src/databases/officer_db/src/schema/music.ts b/src/databases/officer_db/src/schema/music.ts index 6d598846..ece25fd3 100644 --- a/src/databases/officer_db/src/schema/music.ts +++ b/src/databases/officer_db/src/schema/music.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, real, timestamp, unique, index, primaryKey } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, real, timestamp, index, primaryKey, uniqueIndex } from 'drizzle-orm/pg-core'; import { users } from './auth'; // Per-user music favorites. `key` is an opaque path the app supplies and the server never interprets: @@ -17,7 +17,7 @@ export const musicFavorites = pgTable( createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), }, (t) => [ - unique('uq_music_favorites_user_kind_key').on(t.userId, t.kind, t.key), + uniqueIndex('uq_music_favorites_user_kind_key').on(t.userId, t.kind, t.key), index('idx_music_favorites_user_kind').on(t.userId, t.kind), ], ); @@ -36,7 +36,7 @@ export const musicPlaylists = pgTable( createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, - (t) => [unique('uq_music_playlists_user_name').on(t.userId, t.name)], + (t) => [uniqueIndex('uq_music_playlists_user_name').on(t.userId, t.name)], ); // Ordered track entries of a playlist. `position` is 0-based; deletes cascade from the playlist. @@ -76,5 +76,5 @@ export const musicNowPlaying = pgTable( positionSec: real('position_sec').notNull().default(0), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, - (t) => [primaryKey({ columns: [t.userId, t.device] })], + (t) => [primaryKey({ name: 'pk_music_now_playing', columns: [t.userId, t.device] })], ); diff --git a/src/databases/officer_db/src/schema/notify.ts b/src/databases/officer_db/src/schema/notify.ts index a349834d..df2add43 100644 --- a/src/databases/officer_db/src/schema/notify.ts +++ b/src/databases/officer_db/src/schema/notify.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, timestamp, unique, index, check } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, timestamp, index, check, uniqueIndex } from 'drizzle-orm/pg-core'; import { sql } from 'drizzle-orm'; import { users } from './auth'; @@ -42,7 +42,7 @@ export const pushDevices = pgTable( (table) => [ // One row per token per app. The app re-registers on every launch because tokens rotate silently, so // registration is an upsert on this pair rather than an insert. - unique('uq_push_devices_token_bundle').on(table.token, table.bundleId), + uniqueIndex('uq_push_devices_token_bundle').on(table.token, table.bundleId), index('idx_push_devices_user').on(table.userId), // The schema is the source of truth for what may exist, not just its shape — see // src/databases/CLAUDE.md. sql.raw because an interpolated string binds as a parameter, which diff --git a/src/databases/officer_db/src/schema/photos.ts b/src/databases/officer_db/src/schema/photos.ts index 7cf6b888..3105524e 100644 --- a/src/databases/officer_db/src/schema/photos.ts +++ b/src/databases/officer_db/src/schema/photos.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, boolean, timestamp, unique, uniqueIndex } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, boolean, timestamp, uniqueIndex } from 'drizzle-orm/pg-core'; import { sql } from 'drizzle-orm'; import { users } from './auth'; @@ -38,7 +38,7 @@ export const photosConfig = pgTable( (t) => [ // Labels are how the owner tells two accounts apart — duplicates would make the switcher useless. Not // unique on url: several keys against one instance is the whole point. - unique('uq_photos_config_user_label').on(t.userId, t.label), + uniqueIndex('uq_photos_config_user_label').on(t.userId, t.label), // At most one active account per owner, enforced by the DB rather than by convention: a partial unique // index over the active rows only. setActivePhotosAccount still clears the others in a transaction, but a // bug there fails loudly here instead of silently leaving two accounts active and the UI picking one. diff --git a/src/databases/officer_db/src/schema/service-connections.ts b/src/databases/officer_db/src/schema/service-connections.ts index edd0c11e..98c6a701 100644 --- a/src/databases/officer_db/src/schema/service-connections.ts +++ b/src/databases/officer_db/src/schema/service-connections.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, timestamp, unique } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, timestamp, uniqueIndex } from 'drizzle-orm/pg-core'; import { users } from './auth'; // Where a self-hosted service lives, and what it takes to talk to it — for services the owner has exactly @@ -45,5 +45,5 @@ export const serviceConnections = pgTable( }, // One connection per service per owner. This is what makes the whole "no active flag" simplification safe: // there is never a second row to choose between, so nothing can be ambiguous about which one is in use. - (t) => [unique('uq_service_connections_user_service').on(t.userId, t.service)], + (t) => [uniqueIndex('uq_service_connections_user_service').on(t.userId, t.service)], ); diff --git a/src/databases/officer_db/src/schema/soulseek.ts b/src/databases/officer_db/src/schema/soulseek.ts index 614aa81e..d50b4924 100644 --- a/src/databases/officer_db/src/schema/soulseek.ts +++ b/src/databases/officer_db/src/schema/soulseek.ts @@ -5,10 +5,10 @@ import { bigint, text, timestamp, - unique, index, jsonb, foreignKey, + uniqueIndex, } from 'drizzle-orm/pg-core'; import { users } from './auth'; @@ -31,7 +31,7 @@ export const soulseekFavorites = pgTable( createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), }, // Also the lookup index — (user_id, username) covers "all of a user's favourites" by leftmost prefix. - (t) => [unique('uq_soulseek_favorites_user_username').on(t.userId, t.username)], + (t) => [uniqueIndex('uq_soulseek_favorites_user_username').on(t.userId, t.username)], ); // ── Cached share trees ── @@ -62,7 +62,7 @@ export const soulseekBrowseSnapshots = pgTable( startedAt: timestamp('started_at', { withTimezone: true }).notNull().defaultNow(), completedAt: timestamp('completed_at', { withTimezone: true }), }, - (t) => [unique('uq_soulseek_browse_snapshots_user_username').on(t.userId, t.username)], + (t) => [uniqueIndex('uq_soulseek_browse_snapshots_user_username').on(t.userId, t.username)], ); // One row per FOLDER, with that folder's files inlined as jsonb — not a row per file, which would be diff --git a/src/databases/officer_db/src/schema/user-data.ts b/src/databases/officer_db/src/schema/user-data.ts index d9298451..0a47d92b 100644 --- a/src/databases/officer_db/src/schema/user-data.ts +++ b/src/databases/officer_db/src/schema/user-data.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, timestamp, jsonb, unique, check } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, timestamp, jsonb, check, uniqueIndex, foreignKey } from 'drizzle-orm/pg-core'; import { users } from './auth'; import { sql } from 'drizzle-orm'; import { serverIntegrations } from './server'; @@ -30,15 +30,22 @@ export const userIntegrations = pgTable( .notNull() .references(() => users.id, { onDelete: 'cascade' }), provider: text('provider').notNull(), - serverIntegrationId: integer('server_integration_id').references(() => serverIntegrations.id, { - onDelete: 'set null', - }), + // Named explicitly. Drizzle's generated name would be + // `user_integrations_server_integration_id_server_integrations_id_fk` — 65 characters, and Postgres + // truncates identifiers at 63. It stores the truncated form, drizzle keeps comparing against its + // own 65-char version, and every `db:push` drops and re-adds the constraint forever. + serverIntegrationId: integer('server_integration_id'), config: jsonb('config').notNull().default({}), createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, (table) => [ - unique('uq_user_integrations_user_provider').on(table.userId, table.provider), + foreignKey({ + name: 'fk_user_integrations_server_integration', + columns: [table.serverIntegrationId], + foreignColumns: [serverIntegrations.id], + }).onDelete('set null'), + uniqueIndex('uq_user_integrations_user_provider').on(table.userId, table.provider), check( 'ck_user_integrations_provider', // sql.raw — see the note on ck_server_integrations_provider; an interpolated JS string becomes a diff --git a/src/databases/officer_db/src/schema/wallet.ts b/src/databases/officer_db/src/schema/wallet.ts index d2346744..a00134fc 100644 --- a/src/databases/officer_db/src/schema/wallet.ts +++ b/src/databases/officer_db/src/schema/wallet.ts @@ -1,4 +1,4 @@ -import { pgTable, serial, integer, text, boolean, timestamp, jsonb, unique, uniqueIndex } from 'drizzle-orm/pg-core'; +import { pgTable, serial, integer, text, boolean, timestamp, jsonb, uniqueIndex } from 'drizzle-orm/pg-core'; import { sql } from 'drizzle-orm'; import { users } from './auth'; @@ -55,7 +55,7 @@ export const walletWallets = pgTable( updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(), }, (t) => [ - unique('uq_wallet_wallets_user_name').on(t.userId, t.name), + uniqueIndex('uq_wallet_wallets_user_name').on(t.userId, t.name), // At most one active wallet per owner, enforced by the DB rather than convention — a partial unique // index over active rows only, mirroring uq_headscale_servers_one_active. uniqueIndex('uq_wallet_wallets_one_active') @@ -80,7 +80,7 @@ export const walletLabels = pgTable( label: text('label').notNull(), createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), }, - (t) => [unique('uq_wallet_labels_wallet_kind_ref').on(t.walletId, t.kind, t.ref)], + (t) => [uniqueIndex('uq_wallet_labels_wallet_kind_ref').on(t.walletId, t.kind, t.ref)], ); // Frozen UTXOs, excluded from automatic coin selection. Separate from labels because this one affects @@ -102,7 +102,7 @@ export const walletFrozenUtxos = pgTable( reason: text('reason'), createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), }, - (t) => [unique('uq_wallet_frozen_utxos_wallet_outpoint').on(t.walletId, t.outpoint)], + (t) => [uniqueIndex('uq_wallet_frozen_utxos_wallet_outpoint').on(t.walletId, t.outpoint)], ); /**