fix(schema): stop declaring ON DELETE actions the database does not have (BS#2239)

[jakebromberg]

Closes #2239

Problem

Five foreign keys declared a referential action in shared/database/src/schema.ts that the deployed database has never had. The constraints are all plain NO ACTION (pg_constraint.confdeltype = 'a'), verified in both the CI database built from the full migration chain (a ci:db-init from empty) and production — so this is not a local-only artifact.

constraint	schema.ts declared	database has
show_djs.show_id	onDelete: 'cascade'	NO ACTION
artist_library_crossreference.artist_id	onDelete: 'cascade'	NO ACTION
genre_artist_crossreference.artist_id	onDelete: 'cascade'	NO ACTION
genre_artist_crossreference.genre_id	onDelete: 'cascade'	NO ACTION
schedule.specialty_id	onDelete: 'set null'	NO ACTION

drizzle-kit generate structurally cannot see this. The snapshot already records the schema-source values, so once schema.ts and the snapshot agree with each other, no catch-up migration is emitted — the drift never surfaces on its own, no matter how many times the authoring loop runs. The database is the only witness.

Direction: schema.ts was corrected to match the database, not the reverse

One migration is included and it deliberately contains no DDL. The database already matches the corrected declarations; this PR removes five false declarations from schema.ts and changes nothing about the deployed schema. Migration 0155 exists solely to re-sync the Drizzle snapshot — see "The snapshot had to be re-synced too" below.

The two directions are not symmetric:

• De-declaring is a zero-runtime-change edit. Nothing in production deletes these parent rows. Audited across apps/ and shared/: no Drizzle .delete(shows | artists | genres | specialty_shows) and no raw DELETE FROM against any of them, outside tests. Removing the declarations ends the falsehood immediately and forces any future delete path to make its cleanup explicit, in the PR that actually needs it.
• Adding the cascades would arm five destructive cascades across tables holding decades of flowsheet and library history, for zero current callers, to close a discrepancy that has never caused a production failure.

show_djs.show_id is the case with the clearest semantics — a show_djs row is meaningless without its show — and if a show-delete path is ever built, that PR should add the cascade deliberately. Today it would be arming a gun for a caller that does not exist.

This is deliberately the opposite direction from #1126

tests/integration/fk-on-delete-flowsheet-rotation-reviews.spec.js (#1126) documents the identical bug class in a different five FKs, and resolved it by patching the database to match schema.ts (migrations 0094/0097). This PR goes the other way, for the reason above.

The inconsistency is intentional. Each de-declared FK in schema.ts carries a comment saying so and pointing at #2239, so a future reader does not "fix" it back.

The guard is general, not another allowlist

The #1126 spec hard-codes its own five constraints. A different five drifted underneath it anyway. That is empirical evidence that a hand-listed regression spec does not prevent recurrence of this class — nothing forces a new or edited FK onto the list.

tests/integration/fk-on-delete-general-guard.spec.js therefore enumerates every foreign key Drizzle knows about (via getTableConfig over every exported table in @wxyc/database, covering public where better-auth's tables live and the domain schema) and compares each declared onDelete against the live database's pg_constraint.confdeltype. Any divergence fails, in either direction — including a FK the database has that schema.ts never declared. No list to remember to update.

The #1126 spec is left intact: it also exercises real delete behaviour, which this guard does not replace.

Two implementation details that are load-bearing

Both look like they could be simplified. Neither can.

1. FKs are matched by (schema, table, columns), not by constraint name. Two naming quirks make name-string matching produce false drift:

• Postgres silently truncates identifiers over 63 bytes, so Drizzle's un-truncated getName() never matches what is actually stored for the long auth_oauth_* constraints.
• Migration 0067 created flowsheet_linkage_review's FK with an inline REFERENCES … ON DELETE CASCADE column constraint, which Postgres named by its own <table>_<column>_fkey convention rather than Drizzle's <table>_<column>_<reftable>_<refcolumn>_fk.

Neither is real drift — both constraints match schema.ts. They are naming artifacts, and the column tuple is what Postgres actually enforces identity by.

2. The declaration collection runs in a plain node subprocess (tests/utils/collect-declared-foreign-keys.js), not inside Jest. Requiring drizzle-orm/pg-core and @wxyc/database in the same Jest file flips the latter's resolution from the built dist/ to the TypeScript source, which then pulls in a Jest-automocked drizzle-orm whose mock file fails to parse under the integration config's non-TS transform. Confirmed by isolating the two requires: either one alone resolves correctly, and adding the other in the same file (in either order) flips it. The subprocess sidesteps the interaction entirely rather than depending on it.

Verification

tests/integration/fk-on-delete-general-guard.spec.js passes against the CI database.

The guard was also proven to fail on reintroduced drift — a guard that has never been seen fail is not a guard. Putting { onDelete: 'cascade' } back on show_djs.show_id, rebuilding @wxyc/database, and re-running produced:

table: "wxyc_schema.show_djs(show_id)"
database: "show_djs_show_id_shows_id_fk: a (NO ACTION)"
schema.ts declares: "show_djs_show_id_shows_id_fk: c (CASCADE)"

It names the constraint and both sides. Then reverted and rebuilt.

Both FK specs run together green: 2 suites, 5 tests — this guard plus #1126's fk-on-delete-flowsheet-rotation-reviews.spec.js.

The full integration suite was not re-run locally: an earlier failed ci:testmock run leaves a poisoned database volume behind, and re-running against it produces a large number of red suites that are pure artifact. GitHub Actions CI is the authoritative clean-environment check for this PR.

Files

• shared/database/src/schema.ts — five onDelete clauses removed, each replaced with a comment recording the Five foreign keys declare an ON DELETE action the database does not have (drizzle-kit cannot see it) #2239 decision and the deliberate divergence from FK ON DELETE drifted: flowsheet, rotation, reviews mismatch schema vs DB #1126.
• tests/integration/fk-on-delete-general-guard.spec.js — new general guard.
• tests/utils/collect-declared-foreign-keys.js — new subprocess helper that emits every declared FK as JSON.

No schema change and no production behaviour change. The one migration included (0155) carries the snapshot re-sync and no DDL whatsoever.

The snapshot had to be re-synced too (review follow-up)

Code review caught a real hole in the first version of this PR, and it is worth recording because it is the same bug class the PR is about.

De-declaring the five onDelete clauses left schema.ts and meta/0154_snapshot.json disagreeing where they had previously agreed. Drizzle diffs schema.ts against the latest snapshot, so the next drizzle:generate — for some unrelated change, run by someone not thinking about foreign keys — would have silently bundled five DROP CONSTRAINT + five ADD CONSTRAINT statements into that migration. Re-adding a foreign key takes ACCESS EXCLUSIVE on both tables plus a full validation scan of the child; on artist_library_crossreference and genre_artist_crossreference that is a prod-sized scan riding along inside a migration nobody reviewed for it.

That is exactly this issue's own failure mode, one level up: the PR argues that recorded state must match reality, and in fixing schema.ts it left the snapshot lying. Documenting the divergence here instead of fixing it would have re-created the same someone-has-to-remember dependency #2239 exists to remove.

Migration 0155 fixes it and carries no DDL. The statements drizzle-kit wanted to emit drop each constraint and re-add it with the exact semantics it already has — all five are plain NO ACTION in production and in a CI database built from this chain from empty. The database needs no change; only Drizzle's bookkeeping does, and that lives in meta/0155_snapshot.json next to it. The .sql file is a comment block explaining why it is empty and instructing future readers not to "restore" the DDL.

Two supporting facts, both verified rather than assumed:

• scripts/validate-migrations.mjs passes at 153 entries / 0 warnings. Check 8 (constraint-adding migrations want a precondition guard) stays silent precisely because there is no constraint DDL — keeping the generated statements would have tripped it.
• Check 10 of that same validator already recommends "folding the pair into a single no-op migration" for a net-no-op CREATE/DROP, so a statement-less migration is a shape this repo's tooling already reasons in, not a novel pattern.

Two smaller review fixes

• The guard's join key could drop a constraint. Both Maps built from identityKey() — the schema.ts side and the database side — used a bare Map.set, so two foreign keys over the same (schema, table, columns) would silently evict one another and one would vanish from the guard. That is the same "coverage rots" failure this spec was written to prevent, so it must not be reintroduced by the matching key itself. setUnique() now refuses the overwrite and names both the side and the key. Review flagged only the schema.ts side; the database side had it too. No such pair exists today — the guard passing is the proof.
• Explicit test timeout. jest.config.json sets testTimeout: 30000, but jest.parallel.config.json (test:integration:parallel, ci:test:parallel) sets none and falls back to Jest's 5 s default. This spec opens with a synchronous execFileSync that boots Node and requires both drizzle-orm/pg-core and the built @wxyc/database; because it blocks the worker, Jest cannot interrupt it and only reports the timeout after the fact. Pinned so both configs behave identically.

The snapshot file is ~6.5k lines. That is a generated full-schema dump, not hand-written diff surface.

[github-actions]

Schema constraint shape report

no new constraints detected in this diff (uniqueIndex, .unique(), SET NOT NULL, CHECK, FK)

[jakebromberg]

Pushed 76401cef addressing three review findings. Body updated — the previous "no migration is needed and none is included" claim is now wrong and has been corrected.

The substantive one: de-declaring the five onDelete clauses left schema.ts and meta/0154_snapshot.json disagreeing where they had previously agreed. Drizzle diffs against the latest snapshot, so the next drizzle:generate — for an unrelated change, by someone not thinking about foreign keys — would have silently folded five DROP CONSTRAINT + five ADD CONSTRAINT into that migration, and re-adding a FK means ACCESS EXCLUSIVE plus a full child-table validation scan. On artist_library_crossreference and genre_artist_crossreference that is a prod-sized scan inside a migration nobody reviewed for it.

Worth naming plainly: that is this issue's own failure mode one level up. The PR argues recorded state must match reality, and in fixing schema.ts it left the snapshot lying. Writing the divergence down instead of fixing it would have rebuilt the same someone-has-to-remember dependency #2239 exists to remove.

Migration 0155 carries the snapshot and no DDL. All five constraints are already plain NO ACTION in prod and in a CI database built from this chain from empty, so the statements drizzle-kit wanted would drop each constraint and re-add it with the semantics it already has. lint:migrations passes at 153 entries / 0 warnings — check 8 stays silent precisely because there is no constraint DDL.

Also fixed: both identityKey() Maps used a bare Map.set, so two FKs over the same (schema, table, columns) would silently evict one another and drop out of the guard — the same coverage-rots failure the spec exists to prevent, reintroduced by its own join key. Review flagged one side; both had it. And an explicit test timeout, since jest.parallel.config.json sets none and falls back to 5s while the spec opens with a blocking execFileSync.

CI green on 76401cef, including Migration Dry-Run against prod-shaped data.