fix(migrate-ts): refuse a primary-key move instead of silently dropping the PK (#258)

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`.

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

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**

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<number> {
   // 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) {

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<string, string>();
+  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;
 }

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;
+  }
+}

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";

server/typescript/packages/migrate-ts/src/snapshot/plan.ts

@@ -38,6 +38,10 @@ export async function planOffline(args: PlanOfflineArgs): Promise<PlanOfflineRes
     expected: nextSnapshot,
     actual: args.snapshot,
     dialect: args.dialect,
+    // #258 — migration generation refuses a primary-key MOVE (there is no primary-key
+    // change kind to express it; it would otherwise silently drop the constraint). The
+    // read-only verify/drift path does NOT set this, so `meta verify` still reports drift.
+    refusePrimaryKeyChange: true,
     // #208 §7 — exclude declared-@unmanaged objects from the actual (snapshot) side too,
     // so the OFFLINE generate path never proposes DROP for an external table that a
     // `baseline --from-db` captured into the snapshot (parity with the online/verify paths).