fix(driver-sql): varchar differ expects what createColumn would emit (#12732)

.changeset/varchar-differ-expects-what-createcolumn-emits.md

@@ -0,0 +1,64 @@
+---
+'@objectstack/driver-sql': minor
+---
+
+fix(driver-sql): the varchar differ now expects what `createColumn` would actually emit, instead of a different rule (#12732)
+
+The managed-schema drift differ's varchar-length branch expected
+`varchar(field.maxLength)` for any bounded field, over a **pre-existing**
+column dialect `postgres`/`mysql` enforce. `SqlDriver.createColumn` does not
+build that for every bounded field — and disagreed with the differ in two
+measured directions:
+
+**An already-serving deployment stopped booting.** An UNKEYED, bounded
+text-family field (`text` / `richtext` / `signature` / `markdown` / …)
+reported `narrow_varchar` at severity `error`, category `destructive` — the
+one category `runArtifactBootMigrationGate` refuses a boot for — demanding
+the column be narrowed to a shape `createColumn` would never build: unkeyed,
+it leaves the column TEXT (`keyableTextLength` returns `null` unkeyed). The
+trigger was an ordinary, correct-looking edit: adding `maxLength: 50` to a
+legacy `varchar(255)` text column. The divergence changed no behaviour at
+all — the write seam already enforces the declared bound — so the refusal
+was over nothing.
+
+**A `safe`, dev-auto-reconcilable finding planned DDL MySQL refuses
+outright.** A base string-family field (`email` / `url` / `password` / …)
+bounded past `SqlDriver.MAX_VARCHAR_CHARS` (16383) reported `widen_varchar`
+at `warning`/`safe`, planning `ALTER … varchar(100000)` — `ERROR 1074 Column
+length too big` on MySQL, while Postgres accepted it: the dialect-divergent
+enforcement this package's conformance matrices exist to close.
+`declaredVarcharLength` returns `null` above the ceiling for the same reason
+`createColumn` never emits that DDL.
+
+This guard had already been patched at the call site three times for the
+same defect class (#11431 for `multiple: true`; #11794/#11875 for genuine
+TEXT columns) — each time by adding one more condition. This is that defect
+arriving a fourth time, through a column spelled `varchar` because an older
+release created it. Rather than a fourth patch, the branch now asks
+`SqlDriver.varcharColumnChars(field, keyed)` — the emitter's own read-only
+mirror of `createColumn`'s switch, already pinned against `columnInfo()` for
+every `FieldType` — what width `createColumn` would actually build. `null`
+means "the emitter would not make this a varchar," and the branch does not
+fire. Keyedness (`indexedKeyColumns()`, #11374) is threaded from
+`SqlDriver.detectTableDrift` into `diffManagedTable`, since a KEYED bounded
+text-family field legitimately takes `varchar(maxLength)` — the fix
+suppresses the false positive, not the branch itself; a keyed field over the
+same shape still reports.
+
+Graded `minor` rather than `patch`, mirroring the sibling drift-op change for
+#12121 in the opposite direction: an already-serving deployment that
+currently fails to boot over Case A will boot after this upgrade, and a
+`widen_varchar` currently eligible for dev auto-reconcile over Case B will no
+longer be planned — both are user-visible behaviour changes for an existing
+deployment (`os migrate plan`, `os migrate apply`'s counts, the boot-time
+`[schema-drift]` warn), not merely an internal correctness detail. `diffManagedTable`'s exported args object gains two **optional** parameters
+(`keyedColumns`, `varcharColumnChars`); omitting either keeps the pre-#12732
+behaviour unconditionally; this is additive to the object type and — unlike
+#12121's `DriftOp` union member — does not add a case any consumer's
+exhaustive switch must handle, so it is not itself a reason to grade higher
+than `minor`. Nothing is removed, renamed, or newly rejected, so this is not
+a breaking change. The category question (whether Case A's `destructive`
+should become a report) is deliberately **not** addressed here: the fix
+makes the false-positive stop firing entirely, so there is nothing left to
+downgrade, and downgrading it as a separate act would be gate-weakening the
+triage seat did not authorise.

packages/drivers/driver-sql/src/schema-drift.12732-varchar-emitter-parity.test.ts

@@ -0,0 +1,206 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #12732 — the varchar differ's expected width disagreed with what
+ * `createColumn` would actually emit, in two measured directions, over a
+ * PRE-EXISTING `varchar(255)` column, dialect `postgres`:
+ *
+ * **Case A** — an UNKEYED, bounded TEXT-family field (`text` / `richtext` /
+ * `signature` / `markdown` / …) reported `narrow_varchar` at severity
+ * `error`, category `destructive` — a boot-refusing finding
+ * (`runArtifactBootMigrationGate` refuses only `destructive`) demanding the
+ * column be narrowed to a shape `createColumn` would never build: unkeyed,
+ * `keyableTextLength` returns `null` and the emitter leaves the column TEXT.
+ *
+ * **Case B** — a base string-family field (`email` / `url` / `password` /
+ * …) bounded PAST `SqlDriver.MAX_VARCHAR_CHARS` (16383) reported
+ * `widen_varchar` at `warning`/`safe`, planning `ALTER … varchar(100000)` —
+ * DDL MySQL refuses outright (`ERROR 1074`). `declaredVarcharLength` returns
+ * `null` above the ceiling and the emitter leaves the column TEXT instead.
+ *
+ * ## The fix
+ *
+ * One predicate, not a third patch at the call site (after #11431 for
+ * `multiple: true` and #11794/#11875 for genuine TEXT columns): ask
+ * `SqlDriver.varcharColumnChars(field, keyed)` — the emitter's own read-only
+ * mirror of `createColumn`'s switch, already pinned against `columnInfo()`
+ * for every `FieldType` by `sql-driver-11565-row-byte-budget.test.ts` — what
+ * width `createColumn` would actually build. `null` means "the emitter would
+ * not make this a varchar", and the branch below simply does not fire.
+ *
+ * Keyedness matters: a KEYED bounded text-family field legitimately takes
+ * `varchar(maxLength)` (`keyableTextLength` returns non-null when keyed), so
+ * suppressing Case A unconditionally would be a second, opposite defect —
+ * silence over a real divergence. `keyedColumns` (from `indexedKeyColumns()`)
+ * is threaded through so the differ's expectation, not only the DDL, agrees
+ * with keyed columns.
+ *
+ * ## What each block below is worth
+ *
+ * 1. **Case A and Case B fixed** — both stop firing, at the exact
+ * `maxLength` values the issue measured, over `postgres` AND `mysql` (the
+ * only two `enforcesVarcharLength` dialects).
+ * 2. **Case A's keyed counter-case** — the fix must not suppress the branch
+ * unconditionally; a keyed bounded text field still gets `varchar(N)`.
+ * 3. **Case B's ceiling boundary** — `16383` (the last legal width) must
+ * still fire; only `16384` and above are suppressed. A predicate that
+ * suppressed the boundary too would silently widen the tolerance.
+ * 4. **The additive-default pin** — a caller that does not thread
+ * `varcharColumnChars` (an existing direct caller of the exported
+ * function this module cannot enumerate) keeps the PRE-#12732 behaviour
+ * unconditionally, so this change cannot silently alter a caller nobody
+ * updated. Documented on the parameter itself; pinned here as behaviour.
+ * 5. **Unaffected shapes untouched** — the differ's other varchar branches
+ * (already-agreeing columns, SQLite, JSON columns) stay silent, so this
+ * predicate is a narrowing, not a broadening of what already worked.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { SqlDriver } from './sql-driver.js';
+import { diffManagedTable, type FieldDef, type PhysicalColumn, type SqlDialectName } from './schema-drift.js';
+import { dialectCell } from './live-dialect-matrix.testkit.js';
+
+const T = 'os12732_probe';
+const C = 'body';
+
+const staleColumn = (maxLength = 255): PhysicalColumn[] => [
+ { name: C, type: 'varchar', nullable: true, maxLength },
+];
+
+/** The two dialects that physically enforce a varchar width (SQLite does not). */
+const ENFORCING: readonly SqlDialectName[] = ['postgres', 'mysql'];
+
+// A driver instance is only ever used here for its `varcharColumnChars`
+// mirror — never connected, never queried. Dialect is irrelevant to that
+// method (confirmed by reading it: no `this.dialectName` branch), so a
+// single SQLite-configured instance serves every dialect row below, exactly
+// as `sql-driver-11565-row-byte-budget.test.ts` and
+// `schema-drift.unbounded-text-column.test.ts` already do.
+const driver = new SqlDriver(dialectCell('sqlite').config());
+const mirror = (field: FieldDef, keyed?: { unique: boolean }) =>
+ (driver as any).varcharColumnChars(field, keyed) as number | null;
+
+const diffWithMirror = (
+ field: Record<string, unknown>,
+ dialect: SqlDialectName,
+ keyedColumns?: ReadonlyMap<string, { unique: boolean }>,
+ maxLength = 255,
+) =>
+ diffManagedTable({
+ table: T,
+ fields: { [C]: field } as never,
+ columns: staleColumn(maxLength),
+ dialect,
+ keyedColumns,
+ varcharColumnChars: mirror,
+ });
+
+describe('diffManagedTable — expects what createColumn would emit (#12732)', () => {
+ describe('Case A — unkeyed, bounded text-family field', () => {
+ it('stops firing narrow_varchar once the emitter mirror is threaded, on both enforcing dialects', () => {
+ for (const dialect of ENFORCING) {
+ for (const type of ['text', 'richtext', 'signature', 'markdown']) {
+ const found = diffWithMirror({ type, maxLength: 50 }, dialect);
+ expect(found, `${type} on ${dialect}`).toHaveLength(0);
+ }
+ }
+ });
+
+ it('still fires — correctly — when the SAME field is KEYED, at the width createColumn would build', () => {
+ const keyed = new Map([[C, { unique: true }]]);
+ for (const dialect of ENFORCING) {
+ for (const type of ['text', 'richtext', 'signature', 'markdown']) {
+ const found = diffWithMirror({ type, maxLength: 50 }, dialect, keyed);
+ expect(found, `${type} on ${dialect}`).toHaveLength(1);
+ expect(found[0].op.type, type).toBe('narrow_varchar');
+ expect(found[0].severity, type).toBe('error');
+ expect(found[0].category, type).toBe('destructive');
+ expect(found[0].expected, type).toBe('varchar(50)');
+ }
+ }
+ });
+
+ it('a field NOT in the keyedColumns map is treated as unkeyed, not as "unknown"', () => {
+ // Only a DIFFERENT field is keyed — `body` itself must still read as
+ // unkeyed, proving the lookup is per-field, not "any key present".
+ const keyed = new Map([['other_column', { unique: true }]]);
+ const found = diffWithMirror({ type: 'text', maxLength: 50 }, 'postgres', keyed);
+ expect(found).toHaveLength(0);
+ });
+ });
+
+ describe('Case B — base string-family field bounded past the varchar ceiling', () => {
+ it('stops firing widen_varchar above MAX_VARCHAR_CHARS (16383), on both enforcing dialects', () => {
+ for (const dialect of ENFORCING) {
+ for (const type of ['email', 'url', 'password']) {
+ for (const maxLength of [100000, 16384]) {
+ const found = diffWithMirror({ type, maxLength }, dialect);
+ expect(found, `${type}@${maxLength} on ${dialect}`).toHaveLength(0);
+ }
+ }
+ }
+ });
+
+ it('still fires at the boundary — 16383, the last legal width — unaffected by the fix', () => {
+ for (const dialect of ENFORCING) {
+ const found = diffWithMirror({ type: 'email', maxLength: 16383 }, dialect);
+ expect(found, dialect).toHaveLength(1);
+ expect(found[0].op).toMatchObject({ type: 'widen_varchar', to: 16383, from: 255 });
+ expect(found[0].severity).toBe('warning');
+ expect(found[0].category).toBe('safe');
+ }
+ });
+ });
+
+ describe('additive default — a caller that does not thread the new args keeps pre-#12732 behaviour', () => {
+ it('Case A still fires when varcharColumnChars is omitted entirely', () => {
+ const found = diffManagedTable({
+ table: T,
+ fields: { [C]: { type: 'text', maxLength: 50 } } as never,
+ columns: staleColumn(255),
+ dialect: 'postgres',
+ });
+ expect(found).toHaveLength(1);
+ expect(found[0].op.type).toBe('narrow_varchar');
+ });
+
+ it('Case B still fires when varcharColumnChars is omitted entirely', () => {
+ const found = diffManagedTable({
+ table: T,
+ fields: { [C]: { type: 'email', maxLength: 100000 } } as never,
+ columns: staleColumn(255),
+ dialect: 'postgres',
+ });
+ expect(found).toHaveLength(1);
+ expect(found[0].op.type).toBe('widen_varchar');
+ });
+
+ it('omitting keyedColumns alone (mirror threaded) reads every field as unkeyed', () => {
+ // Same as the no-args case for Case A, via the OTHER omission path.
+ const found = diffWithMirror({ type: 'text', maxLength: 50 }, 'postgres', undefined);
+ expect(found).toHaveLength(0);
+ });
+ });
+
+ describe('unaffected shapes stay silent (no broadening)', () => {
+ it('an already-agreeing base string-family column reports nothing', () => {
+ for (const type of ['string', 'email', 'url', 'phone', 'password']) {
+ expect(diffWithMirror({ type }, 'postgres')).toHaveLength(0);
+ }
+ });
+
+ it('SQLite (no length enforcement) reports nothing regardless of mirror', () => {
+ expect(diffWithMirror({ type: 'text', maxLength: 50 }, 'sqlite')).toHaveLength(0);
+ expect(diffWithMirror({ type: 'email', maxLength: 100000 }, 'sqlite')).toHaveLength(0);
+ });
+ });
+
+ describe('the mirror itself, as a control', () => {
+ it('confirms the two directions this fix relies on', () => {
+ expect(mirror({ type: 'text', maxLength: 50 })).toBeNull(); // unkeyed text-family
+ expect(mirror({ type: 'text', maxLength: 50 }, { unique: true })).toBe(50); // keyed
+ expect(mirror({ type: 'email', maxLength: 100000 })).toBeNull(); // past ceiling
+ expect(mirror({ type: 'email', maxLength: 16383 })).toBe(16383); // at ceiling
+ });
+ });
+});

packages/drivers/driver-sql/src/schema-drift.ts

@@ -668,8 +668,29 @@ export function diffManagedTable(args: {
  fields: Record<string, FieldDef>;
  columns: PhysicalColumn[];
  dialect: SqlDialectName;
+ /**
+ * Which columns an index KEYS ON (#11374), keyed by field name — the exact
+ * map {@link indexedKeyColumns} builds. Consulted ONLY by the varchar-length
+ * branch below, through {@link varcharColumnChars}, to answer the same
+ * question `createColumn` asks before it sizes a text-family column.
+ * Omitted (the default), a field reads as unkeyed — the same default the
+ * emitter itself takes when nobody supplies one.
+ */
+ keyedColumns?: ReadonlyMap<string, { unique: boolean }>;
+ /**
+ * The emitter's OWN read-only mirror — `SqlDriver.varcharColumnChars`,
+ * bound by the caller — asked "what `varchar(n)` would `createColumn`
+ * actually build for this field, or `null` if it would not build a varchar
+ * at all?" (#12732). `SqlDriver.detectTableDrift` supplies it on every real
+ * call; a caller that doesn't (this module cannot reach `sql-driver.ts` —
+ * see {@link UNBOUNDED_TEXT_FIELD_TYPES}'s note on the cycle) keeps this
+ * branch's pre-#12732 behaviour, unconditional on `declaredMaxLength`
+ * alone — an intentionally additive default so no existing direct caller of
+ * this exported function changes shape under it.
+ */
+ varcharColumnChars?: (field: FieldDef, keyed?: { unique: boolean }) => number | null;
 }): ManagedDriftEntry[] {
- const { table, fields, columns, dialect } = args;
+ const { table, fields, columns, dialect, keyedColumns, varcharColumnChars } = args;
  const out: ManagedDriftEntry[] = [];
 
  const columnsByName = new Map(columns.map((c) => [c.name, c]));
@@ -899,10 +920,39 @@ export function diffManagedTable(args: {
  typeof field.maxLength === 'number' && Number.isInteger(field.maxLength) && field.maxLength > 0
  ? field.maxLength
  : undefined;
+ // ── #12732: would `createColumn` even make this a VARCHAR? ───────
+ //
+ // `declaredMaxLength` alone answers "did the author write a bound?" — it
+ // says nothing about whether the emitter honours that bound as a varchar
+ // width at all, and it disagreed with `createColumn` in two measured
+ // directions: an UNKEYED bounded text-family field (`text` / `richtext` /
+ // `signature` / `markdown` / …) is TEXT regardless of its declared bound
+ // (`keyableTextLength` returns `null` unkeyed), and a base string-family
+ // field (`email` / `url` / `password` / …) bounded PAST the varchar
+ // ceiling is TEXT too (`declaredVarcharLength` returns `null` above
+ // `MAX_VARCHAR_CHARS`). Both were nonetheless diffed as `varchar(N)`
+ // against the physical column — a `narrow_varchar` at `destructive` for
+ // the first (refusing the boot of an already-serving deployment over a
+ // divergence the write seam already enforces), a `widen_varchar` at
+ // `safe` for the second (planning `ALTER … varchar(100000)`, DDL MySQL
+ // refuses outright).
+ //
+ // Asking `varcharColumnChars` — the emitter's own read-only mirror,
+ // rather than a second copy of its switch — answers both at once: `null`
+ // means "the emitter would not make this a varchar", which is the honest
+ // expectation in both directions, so the branch below simply does not
+ // fire. See the parameter doc above for why an OMITTED mirror leaves this
+ // `true` rather than `false` — additive, not a silent behaviour change
+ // for a caller that hasn't been threaded the new input yet.
+ const emitterWouldVarchar =
+ varcharColumnChars === undefined || declaredMaxLength === undefined
+ ? true
+ : varcharColumnChars(field, keyedColumns?.get(fieldName)) !== null;
  if (
  enforcesVarcharLength(dialect) &&
  !declaresJsonColumn &&
  declaredMaxLength !== undefined &&
+ emitterWouldVarchar &&
  isCharacterColumn(col.type) &&
  typeof col.maxLength === 'number' &&
  declaredMaxLength !== col.maxLength