diff --git a/CHANGELOG.md b/CHANGELOG.md index d15b9f275..3e2cfe4d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,13 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] -Shared-enum cross-package hardening (**#246** + its sibling **#259**). When cut this releases -as a coordinated PATCH — the loader change (#246) lands in all five ports, the Kotlin codegen -changes (#246 Bug 1, #259) land on Maven Central; no metadata vocabulary changes, byte-identical -output for any model that doesn't hit the specific cross-package/two-hop enum shapes below. +Shared-enum cross-package hardening (**#246** + its sibling **#259**), plus an **npm-only** +migrate-ts fix (**#258**). When cut this releases as a coordinated PATCH — the loader change +(#246) lands in all five ports, the Kotlin codegen changes (#246 Bug 1, #259) land on Maven +Central, and #258 lands on npm only (`migrate-ts` + `cli`; schema/migrate is TS-owned, ADR-0015); +no metadata vocabulary changes, byte-identical output for any model that doesn't hit the specific +cross-package/two-hop enum shapes below (and, for #258, any migration that isn't a primary-key +move). - **#246 — a `field.enum` may now be shared across packages, and a conflicting redeclaration is rejected instead of silently dropped.** Two independent fixes: @@ -48,6 +51,35 @@ documented as out-of-scope in the design spec Kotlin `enumTypeName` collapse gaining the `isAbstract` leg the other ports already carry (so a root-level *concrete* enum extended with own `@values` gets a per-field enum on every port). +### Fixed — migrate refuses a primary-key move instead of silently dropping the PK (#258) + +**npm-only** (`migrate-ts` + `cli`; PyPI / NuGet / Maven Central unchanged — schema migrations are +TS-owned, ADR-0015). The diff/emit has no primary-key change kind, so adopting an existing database +(`--from-db`) whose `PRIMARY KEY` differs from the metadata identity degraded **silently** into an +add-column + drop-column: the old PK column and its constraint were dropped, the new column was +never made PK, leaving the table with **no primary key**, so every foreign key referencing it +failed at apply (`there is no unique constraint matching given keys for referenced table`). Only +observable when adopting an existing DB whose PK disagrees with the metadata — a greenfield +`create-table` carries its PK inline. Follow-on from #255, which is what let the apply clear the +column drops and reach the FK stage where this surfaced. + +Migration generation now detects the move and throws a new `PrimaryKeyChangeError` (naming the +table and both PKs) instead of emitting the un-appliable SQL — detect-and-refuse, the #226→#241 arc +for D1 FK cascades being the precedent (auto-migrating the PK remains a follow-up). The check runs +**after** rename detection, mapping live PK column names through any detected `rename-column` for +the table, so a PK column that was merely renamed (the engine preserves the PK through `RENAME +COLUMN`) is not mistaken for a move. It is gated by a `DiffArgs.refusePrimaryKeyChange` flag set +only by the migration-generation paths (the online `meta migrate --db` diff call and the offline +`planOffline`); the read-only `meta verify`/drift path does **not** set it, so `verify` keeps +reporting PK drift rather than throwing. The CLI catches `PrimaryKeyChangeError` at both throw +sites (online + offline, including the D1 path) and emits a structured error + exit 1. + +Byte-identical for any migration that is not a primary-key move (the full `migrate-ts` suite passes +unchanged). Gated by 5 unit tests (refuse on a move; no-refuse on an unchanged PK; no-refuse on a +resolved PK-column rename; no-throw without the flag) plus a real-Postgres integration round-trip +(gated on `MIGRATE_TS_PG_URL`) that reproduces the original failure — a live +`user_profiles PK(user_id)` with a referencing FK — and asserts the refusal fires. + ## [0.20.10] — 2026-08-02 **Coordinated PATCH** — npm `0.20.10` · PyPI `0.19.9` · NuGet `0.19.7` · Maven Central `7.11.7`. diff --git a/docs/bugs/2026-08-02-no-primary-key-change-kind.md b/docs/bugs/2026-08-02-no-primary-key-change-kind.md index a94b320d5..88dbce7af 100644 --- a/docs/bugs/2026-08-02-no-primary-key-change-kind.md +++ b/docs/bugs/2026-08-02-no-primary-key-change-kind.md @@ -5,7 +5,22 @@ title: "migrate: no primary-key change kind, so moving a table's PK leaves it wi labels: bug --- -> **Filed as** https://github.com/metaobjectsdev/metaobjects/issues/258 (2026-08-02). Open. Follow-on from #255. +> **Filed as** https://github.com/metaobjectsdev/metaobjects/issues/258 (2026-08-02). **RESOLVED +> (detect-and-refuse).** Follow-on from #255. +> +> Migration generation now refuses a primary-key move instead of emitting un-appliable SQL — the +> second of the two approaches proposed below, chosen deliberately over auto-migrating. It landed +> in `272ee9d5` ("fix(#258): migrate refuses a primary-key move instead of silently dropping the +> PK") and was extended to the D1 path in `737d3244`. The diff throws a new `PrimaryKeyChangeError` +> (naming the table and both PKs) when an existing table's live `PRIMARY KEY` differs from the +> metadata identity; the check runs after rename detection, so a PK column that was merely renamed +> (the engine preserves the PK through `RENAME COLUMN`) is not mistaken for a move. It is gated by +> a `DiffArgs.refusePrimaryKeyChange` flag that only the migration-generation paths set (the online +> `meta migrate --db` diff call and the offline `planOffline`); the read-only `meta verify`/drift +> path does **not** set it, so `verify` keeps reporting PK drift rather than throwing. Gated by 5 +> unit tests plus a real-Postgres round-trip on the genuine reproduction. Auto-migrating the PK — +> the `add-primary-key`/`drop-primary-key` change kinds and staging proposed in "Suggested fix" +> below — remains a follow-up. Kept as a written record of the failure mode. **Affected port(s):** TypeScript (diff + emit; shared migration engine, so all ports) **Package + version:** `@metaobjectsdev/cli` + `@metaobjectsdev/migrate-ts` 0.20.10 diff --git a/docs/features/migrations-and-drift.md b/docs/features/migrations-and-drift.md index 317cb768f..30998f26d 100644 --- a/docs/features/migrations-and-drift.md +++ b/docs/features/migrations-and-drift.md @@ -97,6 +97,32 @@ cycle, rebuild the tables, then restore it) or break the cycle in your metadata. self-referencing table (a table whose own foreign key targets itself) is not a cycle in this sense and is handled by the cascade like any other rebuild. +#### A moved primary key (adoption-time refusal) + +The diff/emit has no `add-primary-key` / `drop-primary-key` change kind, so an **existing** +table whose live `PRIMARY KEY` differs from the metadata identity cannot be expressed as a +migration. When adopting such a database (`--from-db`), `meta migrate` now **refuses at +generation time** instead of emitting un-appliable SQL — detect-and-refuse, the same arc as +[#226](https://github.com/metaobjectsdev/metaobjects/issues/226)→[#241](https://github.com/metaobjectsdev/metaobjects/issues/241) +for the D1 foreign-key rebuilds above. It throws a `PrimaryKeyChangeError` (naming the table +and both PKs), the CLI catches it and exits 1 +([#258](https://github.com/metaobjectsdev/metaobjects/issues/258)). + +Previously the move degraded **silently** into an add-column + drop-column: the old PK +column and its constraint were dropped while the new column was never made primary key, +leaving the table with no primary key, so every foreign key referencing it failed at apply +(`there is no unique constraint matching given keys`). This surfaces only when **adopting** +an existing database whose PK disagrees with the metadata — a greenfield `create-table` +carries its primary key inline. + +The check is engine-wide (`postgres` / `sqlite` / `d1` — the diff is shared) and runs +**after** rename detection, mapping live PK column names through any detected +`rename-column` change, so a primary-key column that was merely **renamed** (the engine +preserves the PK through `RENAME COLUMN`) is not mistaken for a move. The read-only +`meta verify` / drift path does not set the refusal flag, so `verify` keeps **reporting** +primary-key drift rather than throwing. Auto-migrating the move (adding the +`add-primary-key` / `drop-primary-key` change kinds) is a documented future follow-up. + ### Java Schema migrations for Java projects are owned by the **TypeScript toolchain** diff --git a/server/typescript/packages/cli/src/commands/migrate.ts b/server/typescript/packages/cli/src/commands/migrate.ts index f0976ccdd..4652720c4 100644 --- a/server/typescript/packages/cli/src/commands/migrate.ts +++ b/server/typescript/packages/cli/src/commands/migrate.ts @@ -25,6 +25,7 @@ import { readSnapshot, writeSnapshot, BlockedChangesError, + PrimaryKeyChangeError, renderD1, writeMigrationD1, introspectD1, @@ -389,6 +390,10 @@ export async function migrateCommand( actual, dialect: kysely.dialect, allow: tokensToAllowOptions(config.allow), + // #258 — adopting a live DB whose PRIMARY KEY differs from the metadata identity + // has no expressible migration; refuse loudly instead of emitting SQL that drops + // the constraint and breaks referencing FKs at apply. + refusePrimaryKeyChange: true, // #208 §7 — declared-@unmanaged objects are external: exclude them from the // actual side so migrate proposes neither create nor drop for them. unmanagedNames: collectUnmanagedNames(metadata), @@ -398,6 +403,13 @@ export async function migrateCommand( }, }); } catch (err) { + // #258 — a primary-key move has no expressible migration; refuse loudly. + if (err instanceof PrimaryKeyChangeError) { + log.error(`migrate: ${err.message}`); + emitStructuredError(`migrate: ${err.message}`, "align the primary key manually, or reconcile the metadata identity to match the live table", fmt); + await kysely.close(); + return 1; + } // diff() throws when onAmbiguous returns "abort" — surface as exit 1 // with the collected ambiguity list. if ((err as Error).message.includes("aborted by onAmbiguous")) { @@ -809,6 +821,12 @@ export async function runOfflineGenerate( }, }); } catch (err) { + // #258 — a primary-key move has no expressible migration; refuse loudly. + if (err instanceof PrimaryKeyChangeError) { + log.error(`migrate: ${err.message}`); + emitStructuredError(`migrate: ${err.message}`, "align the primary key manually, or reconcile the metadata identity to match the live table", fmt); + return 1; + } if ((err as Error).message.includes("aborted by onAmbiguous")) { log.error(`migrate: ambiguous rename/drop detected; re-run with --on-ambiguous rename|drop-add`); return 1; @@ -912,7 +930,7 @@ async function runD1Migrate( config: ResolvedMigrateConfig, metaRoot: string, runner: WranglerRunner, - _fmt: OutputFormat = "text", + fmt: OutputFormat = "text", ): Promise { // 1. Resolve wrangler.toml + binding. const wranglerConfigPath = config.d1.wranglerConfigPath @@ -1012,6 +1030,10 @@ async function runD1Migrate( // @constraintName models churning and enum @values changes silent on D1. dialect: "d1", allow: tokensToAllowOptions(config.allow), + // #258 — adopting a live D1 DB whose PRIMARY KEY differs from the metadata identity + // has no expressible migration; refuse loudly instead of emitting SQL that drops + // the constraint and breaks referencing FKs at apply (same failure as the online path). + refusePrimaryKeyChange: true, // #208 §7 — declared-@unmanaged objects are external (see the online path above). unmanagedNames: collectUnmanagedNames(metadata), onAmbiguous: async (a) => { @@ -1020,6 +1042,12 @@ async function runD1Migrate( }, }); } catch (err) { + // #258 — a primary-key move has no expressible migration; refuse loudly. + if (err instanceof PrimaryKeyChangeError) { + log.error(`migrate: ${err.message}`); + emitStructuredError(`migrate: ${err.message}`, "align the primary key manually, or reconcile the metadata identity to match the live table", fmt); + return 1; + } if ((err as Error).message.includes("aborted by onAmbiguous")) { const entries = ambiguousToEntries(collectedAmbiguous); for (const e of entries) { diff --git a/server/typescript/packages/migrate-ts/src/diff/index.ts b/server/typescript/packages/migrate-ts/src/diff/index.ts index ae778df08..823cf18c1 100644 --- a/server/typescript/packages/migrate-ts/src/diff/index.ts +++ b/server/typescript/packages/migrate-ts/src/diff/index.ts @@ -8,6 +8,7 @@ import type { import type { SqlType } from "../sql-type.js"; import { sqlTypeEquals } from "../sql-type.js"; import { applyStatus } from "./status.js"; +import { PrimaryKeyChangeError } from "../errors.js"; import { detectColumnRenames, detectTableRenames } from "./rename-heuristic.js"; import { viewSqlEquals } from "../view-sql-compare.js"; import { viewReplaceIsLegal } from "../view-column-types.js"; @@ -58,6 +59,16 @@ export interface DiffArgs { unmanagedNames?: string[]; /** Dialect; CHECK-constraint evolution on existing tables is emitted for postgres only. */ dialect?: Dialect; + /** + * #258 — refuse (throw {@link PrimaryKeyChangeError}) when an existing table's live + * PRIMARY KEY differs from the metadata identity. There is no primary-key change kind + * in the emitter, so such a move would silently degrade into add-column + drop-column + * and leave the table with no PK, breaking referencing FKs at apply time. Set by the + * migration-generation path (snapshot/plan.ts); left unset by the read-only drift/verify + * path so `meta verify` keeps reporting drift rather than throwing. Off by default — + * existing callers are byte-identical. + */ + refusePrimaryKeyChange?: boolean; } const ALLOWED: ChangeStatus = { state: "allowed" }; @@ -266,10 +277,51 @@ export async function diff( delete (c as Aug)._columns; } + // #258: refuse a primary-key MOVE at generation time. There is no primary-key change + // kind, so a table whose live PK differs from the metadata identity would degrade into + // an add-column + drop-column and lose the constraint (breaking referencing FKs at + // apply). Runs after rename detection so a PK column that was merely RENAMED (PK + // preserved by the engine) is not mistaken for a move. Gated by refusePrimaryKeyChange + // so only migration generation refuses; the read-only drift/verify path is unchanged. + if (args.refusePrimaryKeyChange === true) { + for (const [id, expectedTable] of expectedTables) { + const actualTable = actualTables.get(id); + if (actualTable === undefined) continue; // create-table: PK is inline, not a move + assertPrimaryKeyUnchanged(expectedTable, actualTable, changes); + } + } + applyStatus(changes, args.allow ?? {}); return { changes, blocked: changes.filter((c) => c.status.state === "blocked") }; } +/** + * #258 — throw {@link PrimaryKeyChangeError} when a table's live PRIMARY KEY differs from + * the metadata identity. Live PK column names are first mapped through any detected + * `rename-column` for this table, so a renamed PK column (the engine preserves the PK + * through a `RENAME COLUMN`) is not treated as a move. A genuine move — a PK column added + * or dropped, or the key repointed to different columns — has no expressible migration and + * is refused. + */ +function assertPrimaryKeyUnchanged( + expected: TableDescriptor, + actual: TableDescriptor, + changes: Change[], +): void { + const wantId = tableIdentity(expected); + const renamed = new Map(); + for (const c of changes) { + if (c.kind === "rename-column" && tableIdentity({ name: c.table, ...schemaSpread(c.schema) }) === wantId) { + renamed.set(c.from, c.to); + } + } + const livePk = actual.primaryKey.map((col) => renamed.get(col) ?? col); + const wantPk = expected.primaryKey; + const unchanged = livePk.length === wantPk.length && livePk.every((col, i) => col === wantPk[i]); + if (unchanged) return; + throw new PrimaryKeyChangeError(expected.name, actual.primaryKey, expected.primaryKey, expected.schema); +} + function isDiffArgs(x: DiffArgs | SchemaSnapshot): x is DiffArgs { return "expected" in x && "actual" in x; } diff --git a/server/typescript/packages/migrate-ts/src/errors.ts b/server/typescript/packages/migrate-ts/src/errors.ts index 330cadbc4..ae7089688 100644 --- a/server/typescript/packages/migrate-ts/src/errors.ts +++ b/server/typescript/packages/migrate-ts/src/errors.ts @@ -93,3 +93,37 @@ export class BlockedChangesError extends Error { this.enableHints = hints; } } + +/** + * #258 — a table whose live PRIMARY KEY differs from the metadata identity cannot be + * migrated: the diff/emit has no primary-key change kind, so the difference degrades + * silently into an add-column + drop-column (the old PK column is dropped, the new one + * is never made PK), leaving the table with no primary key and breaking every foreign + * key that references it at apply time. Migration generation detects the move and throws + * this instead of emitting un-appliable SQL (detect-and-refuse; the #226→#241 arc for D1 + * FK cascades is the precedent). A pure column RENAME is NOT a key move — the engine + * preserves the PK through a `RENAME COLUMN` — and does not trigger this. + */ +export class PrimaryKeyChangeError extends Error { + override readonly name = "PrimaryKeyChangeError"; + readonly table: string; + readonly livePrimaryKey: string[]; + readonly expectedPrimaryKey: string[]; + readonly schema?: string; + + constructor(table: string, livePrimaryKey: string[], expectedPrimaryKey: string[], schema?: string) { + const qualified = schema !== undefined ? `${schema}.${table}` : table; + const fmt = (cols: string[]) => (cols.length > 0 ? `PRIMARY KEY (${cols.join(", ")})` : "no primary key"); + super( + `primary key of "${qualified}" differs from the live database: live ${fmt(livePrimaryKey)} vs ` + + `metadata ${fmt(expectedPrimaryKey)}. migrate cannot express a primary-key change (there is no ` + + `add/drop-primary-key change kind), so this would silently drop the constraint and break every ` + + `foreign key that references this table. Align the primary key manually — or reconcile the metadata ` + + `identity to match the live table — before migrating.`, + ); + this.table = table; + this.livePrimaryKey = livePrimaryKey; + this.expectedPrimaryKey = expectedPrimaryKey; + if (schema !== undefined) this.schema = schema; + } +} diff --git a/server/typescript/packages/migrate-ts/src/index.ts b/server/typescript/packages/migrate-ts/src/index.ts index 20d29be71..d95a3dc9a 100644 --- a/server/typescript/packages/migrate-ts/src/index.ts +++ b/server/typescript/packages/migrate-ts/src/index.ts @@ -31,7 +31,7 @@ export { planOffline, baselineFromMetadata } from "./snapshot/plan.js"; export type { PlanOfflineArgs, PlanOfflineResult } from "./snapshot/plan.js"; // Errors -export { BlockedChangesError, SetNullNotNullableError } from "./errors.js"; +export { BlockedChangesError, SetNullNotNullableError, PrimaryKeyChangeError } from "./errors.js"; // SqlType helpers (rarely needed but useful for advanced consumers) export { isWidening, sqlTypeEquals } from "./sql-type.js"; diff --git a/server/typescript/packages/migrate-ts/src/snapshot/plan.ts b/server/typescript/packages/migrate-ts/src/snapshot/plan.ts index c4d4ee61d..f681d52e7 100644 --- a/server/typescript/packages/migrate-ts/src/snapshot/plan.ts +++ b/server/typescript/packages/migrate-ts/src/snapshot/plan.ts @@ -38,6 +38,10 @@ export async function planOffline(args: PlanOfflineArgs): Promise + ({ name, sqlType, nullable: false, ...(identity ? { identity } : {}) }); + +const UUID = { kind: "uuid" } as const; +const BIGINT = { kind: "integer", bits: 64 } as const; + +// live table has PRIMARY KEY (user_id bigint); metadata identity is id (uuid). +const liveUserIdPk = (): SchemaSnapshot => ({ + tables: [table("user_profiles", [col("user_id", BIGINT)], ["user_id"])], + views: [], +}); +const metadataIdPk = (): SchemaSnapshot => ({ + tables: [table("user_profiles", [col("id", UUID, "uuid")], ["id"])], + views: [], +}); + +describe("diff — #258 primary-key move detect-and-refuse", () => { + test("REFUSES when the live PK differs from the metadata identity (add/drop, not a rename)", async () => { + await expect( + diff({ expected: metadataIdPk(), actual: liveUserIdPk(), refusePrimaryKeyChange: true }), + ).rejects.toBeInstanceOf(PrimaryKeyChangeError); + }); + + test("the refusal names the table and both primary keys", async () => { + try { + await diff({ expected: metadataIdPk(), actual: liveUserIdPk(), refusePrimaryKeyChange: true }); + throw new Error("expected diff to refuse"); + } catch (e) { + expect(e).toBeInstanceOf(PrimaryKeyChangeError); + const err = e as PrimaryKeyChangeError; + expect(err.table).toBe("user_profiles"); + expect(err.livePrimaryKey).toEqual(["user_id"]); + expect(err.expectedPrimaryKey).toEqual(["id"]); + expect(err.message).toContain("user_profiles"); + } + }); + + test("does NOT refuse when the primary key is unchanged", async () => { + const same = (): SchemaSnapshot => ({ + tables: [table("user_profiles", [col("id", UUID, "uuid")], ["id"])], + views: [], + }); + const r = await diff({ expected: same(), actual: same(), refusePrimaryKeyChange: true }); + expect(r.changes).toEqual([]); + }); + + test("does NOT refuse a PK-column RENAME (rename resolved → PK preserved by the engine)", async () => { + const actual: SchemaSnapshot = { tables: [table("t", [col("user_id", BIGINT)], ["user_id"])], views: [] }; + const expected: SchemaSnapshot = { tables: [table("t", [col("userId", BIGINT)], ["userId"])], views: [] }; + const r = await diff({ + expected, + actual, + refusePrimaryKeyChange: true, + allow: { dropColumn: true }, + onAmbiguous: async () => "rename", + }); + // The renamed PK column is a rename-column change; the PK is preserved, so no refusal. + expect(r.changes.some((c) => c.kind === "rename-column")).toBe(true); + }); + + test("without the flag, diff does not throw — the verify/drift path is unchanged", async () => { + const r = await diff({ expected: metadataIdPk(), actual: liveUserIdPk(), allow: { dropColumn: true } }); + expect(r).toBeDefined(); + // The un-fixed behavior: silent add-column + drop-column, PK move lost. + expect(r.changes.some((c) => c.kind === "add-column")).toBe(true); + expect(r.changes.some((c) => c.kind === "drop-column")).toBe(true); + }); +}); diff --git a/server/typescript/packages/migrate-ts/test/integration/pg-primary-key-refuse-258.test.ts b/server/typescript/packages/migrate-ts/test/integration/pg-primary-key-refuse-258.test.ts new file mode 100644 index 000000000..6533f07f9 --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/integration/pg-primary-key-refuse-258.test.ts @@ -0,0 +1,122 @@ +/** + * Real-Postgres gate for #258 — "adopting a database whose PRIMARY KEY differs + * from the metadata identity." + * + * Root cause: the diff/emit has no primary-key change kind, so a table whose live + * PK moved (e.g. live `PRIMARY KEY (user_id)`, metadata identity `id`) degrades + * silently into an add-column `id` + drop-column `user_id`: the old PK column (and + * its constraint) is dropped, the new column is never made PK, and the table is left + * with NO primary key — so every foreign key that references it is rejected at apply + * ("there is no unique constraint matching given keys for referenced table …"). + * + * The fix is DETECT-AND-REFUSE: migration generation compares the introspected PK to + * the metadata identity and throws PrimaryKeyChangeError instead of emitting the + * un-appliable SQL. This test proves the refusal fires against the REAL introspected + * schema (a unit assertion on hand-built snapshots is not sufficient evidence that + * introspection reads the live PK correctly) — and that WITHOUT the guard the diff + * still produces the silent add/drop pair, so the guard is what closes the gap. + * + * Gated on MIGRATE_TS_PG_URL like every other pg integration test here; skips cleanly + * when unset. + */ + +import { test, expect, beforeAll, afterAll, describe } from "bun:test"; +import { Pool } from "pg"; +import { Kysely, PostgresDialect, sql } from "kysely"; +import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata"; +import { buildExpectedSchema } from "../../src/expected-schema.js"; +import { introspectPostgres } from "../../src/introspect/postgres.js"; +import { diff } from "../../src/diff/index.js"; +import { PrimaryKeyChangeError } from "../../src/errors.js"; + +const PG_URL = process.env["MIGRATE_TS_PG_URL"]; +const realDescribe = PG_URL ? describe : describe.skip; + +// Metadata: user_profiles keyed on `id` (uuid). The LIVE table (raw SQL below) is +// instead keyed on `user_id` — the PK MOVE the bug is about. +const META = JSON.stringify({ + "metadata.root": { + package: "acme", + children: [ + { + "object.entity": { + name: "UserProfile", + children: [ + { "source.rdb": { "@table": "user_profiles" } }, + { "field.uuid": { name: "id" } }, + { "field.string": { name: "authUserId", "@required": true } }, + { "identity.primary": { name: "pk", "@fields": ["id"] } }, + ], + }, + }, + ], + }, +}); + +let k: Kysely>; +let pool: Pool; + +if (PG_URL) { + beforeAll(() => { + pool = new Pool({ connectionString: PG_URL }); + k = new Kysely>({ dialect: new PostgresDialect({ pool }) }); + }); + afterAll(async () => { + await cleanup(); + await k.destroy(); + }); +} + +async function cleanup(): Promise { + await sql.raw(`DROP TABLE IF EXISTS "agent_configs" CASCADE`).execute(k); + await sql.raw(`DROP TABLE IF EXISTS "user_profiles" CASCADE`).execute(k); +} + +async function loadRoot(json: string) { + return (await new MetaDataLoader().load([new InMemoryStringSource(json)])).root; +} + +realDescribe("PG #258 — a live primary-key move is refused, not silently dropped", () => { + test("adopting a DB whose PRIMARY KEY differs from the metadata identity refuses instead of losing the PK", async () => { + await cleanup(); + + // LIVE schema: user_profiles keyed on user_id, with a second table whose FK + // references it — the exact shape where a silent PK drop breaks the FK at apply. + await sql + .raw(`CREATE TABLE "user_profiles" ("user_id" bigint PRIMARY KEY, "auth_user_id" text NOT NULL)`) + .execute(k); + await sql + .raw( + `CREATE TABLE "agent_configs" ("id" bigint PRIMARY KEY, ` + + `"created_by" bigint NOT NULL REFERENCES "user_profiles" ("user_id"))`, + ) + .execute(k); + + const root = await loadRoot(META); + const expected = buildExpectedSchema(root, { dialect: "postgres" }); + const actual = await introspectPostgres(k); + + // Introspection read the live PK as user_id (the premise of the whole bug). + const liveUP = actual.tables.find((t) => t.name === "user_profiles"); + expect(liveUP?.primaryKey).toEqual(["user_id"]); + + // WITHOUT the guard: the diff silently degrades into add-column id + drop-column + // user_id — the PK is dropped and never re-added (the un-appliable migration). + const unsafe = await diff({ expected, actual, dialect: "postgres", allow: { dropColumn: true } }); + expect(unsafe.changes.some((c) => c.kind === "add-column" && c.column.name === "id")).toBe(true); + expect(unsafe.changes.some((c) => c.kind === "drop-column" && c.column === "user_id")).toBe(true); + + // WITH the guard (migration generation): refuse loudly, so no bad SQL is emitted. + let refused: unknown; + try { + await diff({ expected, actual, dialect: "postgres", refusePrimaryKeyChange: true, allow: { dropColumn: true } }); + } catch (e) { + refused = e; + } + expect(refused).toBeInstanceOf(PrimaryKeyChangeError); + const err = refused as PrimaryKeyChangeError; + expect(err.table).toBe("user_profiles"); + expect(err.livePrimaryKey).toEqual(["user_id"]); + expect(err.expectedPrimaryKey).toEqual(["id"]); + }); +});