From 272ee9d56e3e78e164428a71b0cb7cdbaff8b5a2 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sun, 2 Aug 2026 11:00:21 -0400 Subject: [PATCH 1/4] fix(#258): migrate refuses a primary-key move instead of silently dropping the PK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopting a database whose PRIMARY KEY differs from the metadata identity had no expressible migration: the diff/emit has no primary-key change kind, so a moved PK (e.g. live PRIMARY KEY (user_id), metadata identity id) degraded into an add-column + drop-column — the old PK column and its constraint were dropped, the new column was never made PK, and the table was left with NO primary key, so every referencing foreign key failed at apply. Detect-and-refuse (the #226->#241 arc precedent): migration generation now compares the introspected primary key to the metadata identity and throws PrimaryKeyChangeError with a clear message instead of emitting the un-appliable SQL. 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. Gated by a new DiffArgs.refusePrimaryKeyChange flag set only by the two migration-generation paths (online meta migrate --db and offline planOffline); the read-only drift/verify path is unchanged, so meta verify still reports drift. Auto-migrating a PK move (add/drop-primary-key change kinds) is a later follow-up. npm-only (migrate-ts + cli). Gated by unit tests (refuse on a move, not on an unchanged PK, not on a renamed PK column, and off by default) plus a real-Postgres integration test proving introspection reads the live PK and the refusal fires on the genuine reproduction. Existing meta gen / meta migrate output is byte-identical. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr --- .../packages/cli/src/commands/migrate.ts | 18 +++ .../packages/migrate-ts/src/diff/index.ts | 52 ++++++++ .../packages/migrate-ts/src/errors.ts | 34 +++++ .../packages/migrate-ts/src/index.ts | 2 +- .../packages/migrate-ts/src/snapshot/plan.ts | 4 + .../test/diff-primary-key-refuse.test.ts | 82 ++++++++++++ .../pg-primary-key-refuse-258.test.ts | 122 ++++++++++++++++++ 7 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 server/typescript/packages/migrate-ts/test/diff-primary-key-refuse.test.ts create mode 100644 server/typescript/packages/migrate-ts/test/integration/pg-primary-key-refuse-258.test.ts diff --git a/server/typescript/packages/cli/src/commands/migrate.ts b/server/typescript/packages/cli/src/commands/migrate.ts index f0976ccdd..c0d31841e 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; 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"]); + }); +}); From 737d3244704e8e58af585933932804dfbf99faa5 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sun, 2 Aug 2026 11:40:27 -0400 Subject: [PATCH 2/4] no-mistakes(review): Guard D1 migrate path against primary-key moves (#258) --- .../typescript/packages/cli/src/commands/migrate.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/server/typescript/packages/cli/src/commands/migrate.ts b/server/typescript/packages/cli/src/commands/migrate.ts index c0d31841e..4652720c4 100644 --- a/server/typescript/packages/cli/src/commands/migrate.ts +++ b/server/typescript/packages/cli/src/commands/migrate.ts @@ -930,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 @@ -1030,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) => { @@ -1038,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) { From e0deec4ee7ad7142f0a77e9a9b59dfbfdd128c91 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sun, 2 Aug 2026 11:59:06 -0400 Subject: [PATCH 3/4] no-mistakes(document): Document #258 PK-refuse fix in CHANGELOG and bug doc --- CHANGELOG.md | 40 +++++++++++++++++-- .../2026-08-02-no-primary-key-change-kind.md | 17 +++++++- 2 files changed, 52 insertions(+), 5 deletions(-) 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 From cccbc61ac41e605f4fd9385a5432638eb0b4d5b4 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sun, 2 Aug 2026 12:06:13 -0400 Subject: [PATCH 4/4] no-mistakes(document): Document #258 PK-move refusal in migrate guide --- docs/features/migrations-and-drift.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) 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**