fix(migrate-ts): thread dialect into verify/replay/drift diff + harden stripCheckWrapper (NOT VALID)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: dmealing

server/typescript/packages/migrate-ts/src/check-expr-compare.ts

@@ -22,3 +22,15 @@ export function checkExprEquals(a: string | undefined, b: string | undefined): b
   if (a === undefined || b === undefined) return false;
   return normalizeCheckExpr(a) === normalizeCheckExpr(b);
 }
+
+/**
+ * `CHECK (<expr>)` → `<expr>` (balanced outer wrapper); returns input unchanged
+ * if there is no CHECK wrapper. Tolerates a trailing constraint modifier suffix
+ * (`pg_get_constraintdef` can return `CHECK (<expr>) NOT VALID`) so the wrapper
+ * still strips cleanly to the inner expression instead of falling through to the
+ * unchanged-input fallback (which would cause spurious drop+add churn).
+ */
+export function stripCheckWrapper(def: string): string {
+  const m = /^\s*CHECK\s*\((.*)\)(?:\s+NOT\s+VALID)?\s*$/is.exec(def);
+  return m ? m[1]!.trim() : def.trim();
+}

server/typescript/packages/migrate-ts/src/drift/classify.ts

@@ -1,6 +1,6 @@
 // src/drift/classify.ts
 import { diff } from "../diff/index.js";
-import type { Change, DiffResult, SchemaSnapshot } from "../types.js";
+import type { Change, Dialect, DiffResult, SchemaSnapshot } from "../types.js";
 
 /**
  * Change kinds that represent an object present in the live DB but absent from
@@ -42,7 +42,12 @@ export function classifyDrift(changes: Change[]): DriftClassification {
 export async function driftAgainstSnapshot(
   snapshot: SchemaSnapshot,
   actual: SchemaSnapshot,
+  dialect?: Dialect,
 ): Promise<DriftClassification> {
-  const result: DiffResult = await diff({ expected: snapshot, actual });
+  const result: DiffResult = await diff({
+    expected: snapshot,
+    actual,
+    ...(dialect !== undefined ? { dialect } : {}),
+  });
   return classifyDrift(result.changes);
 }

server/typescript/packages/migrate-ts/src/drift/drift.ts

@@ -59,6 +59,7 @@ export async function computeDrift(
   return diff({
     expected,
     actual,
+    dialect,
     allow: opts?.allow ?? {},
     ...(opts?.ignoreTables !== undefined ? { ignoreTables: opts.ignoreTables } : {}),
   });

server/typescript/packages/migrate-ts/src/introspect/postgres.ts

@@ -31,6 +31,7 @@ import { sql } from "kysely";
 import type { SchemaSnapshot, TableDescriptor, ColumnDescriptor, ColumnDefault, IndexDescriptor, FkDescriptor, FkAction, ViewDescriptor, CheckDescriptor } from "../types.js";
 import type { SqlType } from "../sql-type.js";
 import { MIGRATIONS_TABLE } from "../apply/ledger.js";
+import { stripCheckWrapper } from "../check-expr-compare.js";
 
 // ---------------------------------------------------------------------------
 // Public API
@@ -459,9 +460,3 @@ async function readPgChecks(k: RawKysely, schema: string, table: string): Promis
     return []; // pg-mem: pg_constraint unsupported
   }
 }
-
-/** `CHECK (<expr>)` → `<expr>` (balanced outer wrapper); returns input unchanged if no wrapper. */
-function stripCheckWrapper(def: string): string {
-  const m = /^\s*CHECK\s*\((.*)\)\s*$/is.exec(def);
-  return m ? m[1]!.trim() : def.trim();
-}

server/typescript/packages/migrate-ts/src/verify/replay.ts

@@ -35,7 +35,7 @@ export async function verifyReplay(args: VerifyReplayArgs): Promise<VerifyReplay
     ...introspected,
     tables: introspected.tables.filter((t) => t.name !== MIGRATIONS_TABLE),
   };
-  const classification = await driftAgainstSnapshot(args.snapshot, actual);
+  const classification = await driftAgainstSnapshot(args.snapshot, actual, args.dialect);
   return {
     ...classification,
     ok: classification.drift.length === 0 && classification.unmanaged.length === 0,

server/typescript/packages/migrate-ts/test/check-evolution/drift-dialect.test.ts

@@ -0,0 +1,31 @@
+import { describe, test, expect } from "bun:test";
+import type { SchemaSnapshot, TableDescriptor, CheckDescriptor } from "../../src/types.js";
+import { driftAgainstSnapshot } from "../../src/drift/classify.js";
+
+const CHK = (name: string, expression: string): CheckDescriptor => ({ name, expression });
+function tbl(checks: CheckDescriptor[]): TableDescriptor {
+  return {
+    name: "orders",
+    columns: [{ name: "qty", sqlType: { kind: "integer", bits: 32 }, nullable: false }],
+    indexes: [], foreignKeys: [], checks, primaryKey: ["qty"],
+  };
+}
+const snap = (checks: CheckDescriptor[]): SchemaSnapshot => ({ tables: [tbl(checks)], views: [] });
+const C0 = CHK("orders_qty_numeric_chk", "qty >= 1");
+const C1 = CHK("orders_qty_max_chk", "qty <= 100");
+
+describe("driftAgainstSnapshot — dialect-threaded CHECK evolution", () => {
+  test("with dialect=postgres: snapshot has 2 checks, DB has 1 → reports check drift", async () => {
+    // expected (snapshot) = 2 checks; actual (DB) = 1 check → the missing modeled
+    // check surfaces as add-check drift (postgres-gated diffTableChecks).
+    const r = await driftAgainstSnapshot(snap([C0, C1]), snap([C0]), "postgres");
+    const checkChanges = [...r.drift, ...r.unmanaged].filter((c) => c.kind.endsWith("-check"));
+    expect(checkChanges.length).toBeGreaterThan(0);
+  });
+
+  test("without dialect (back-compat): same snapshots → no check drift", async () => {
+    const r = await driftAgainstSnapshot(snap([C0, C1]), snap([C0]));
+    const checkChanges = [...r.drift, ...r.unmanaged].filter((c) => c.kind.endsWith("-check"));
+    expect(checkChanges.length).toBe(0);
+  });
+});

server/typescript/packages/migrate-ts/test/check-evolution/normalize.test.ts

@@ -1,5 +1,5 @@
 import { describe, test, expect } from "bun:test";
-import { normalizeCheckExpr, checkExprEquals } from "../../src/check-expr-compare.js";
+import { normalizeCheckExpr, checkExprEquals, stripCheckWrapper } from "../../src/check-expr-compare.js";
 
 describe("normalizeCheckExpr", () => {
   test("strips parens, collapses whitespace, lowercases", () => {
@@ -18,3 +18,21 @@ describe("normalizeCheckExpr", () => {
     expect(checkExprEquals(undefined, "x")).toBe(false);
   });
 });
+
+describe("stripCheckWrapper", () => {
+  test("strips the CHECK(...) wrapper", () => {
+    expect(stripCheckWrapper("CHECK (qty >= 1)")).toBe("qty >= 1");
+  });
+  test("preserves inner expression with multiple terms", () => {
+    expect(stripCheckWrapper("CHECK (qty >= 1 AND qty <= 100)")).toBe("qty >= 1 AND qty <= 100");
+  });
+  test("tolerates a trailing NOT VALID modifier", () => {
+    expect(stripCheckWrapper("CHECK (qty >= 1) NOT VALID")).toBe("qty >= 1");
+  });
+  test("preserves a regex expression with parens", () => {
+    expect(stripCheckWrapper("CHECK (slug ~ '^(a|b)$')")).toBe("slug ~ '^(a|b)$'");
+  });
+  test("returns a non-CHECK string unchanged", () => {
+    expect(stripCheckWrapper("qty >= 1")).toBe("qty >= 1");
+  });
+});