From e19bd33f81543aa6ab9834b9a78fa9a5af15417c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 14:20:14 +0000 Subject: [PATCH 1/4] wip(driver-sql): report an unbounded text-family field over a pre-existing varchar --- .../drivers/driver-sql/src/schema-drift.ts | 159 ++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 3bf6c09c2e..538015936d 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -171,6 +171,38 @@ export type DriftOp = * can show the divergence without re-deriving it from the message. */ | { type: 'manual_column_type_change'; table: string; column: string; to: string; from: string } + /** + * REPORT ONLY (#12121). The column is a `varchar(n)` under a TEXT-family + * field that declared NO usable bound — the shape `createColumn` emits as + * TEXT — so the column caps writes the declaration allows, and **the platform + * deliberately does not change it**. + * + * ⛔ A DISTINCT op rather than a second use of `manual_column_type_change`, + * for a measured reason and not a stylistic one: `os migrate + * multi-value-columns` selects its ENTIRE population by + * `op.type === 'manual_column_type_change'` (`isStaleMultiValueColumn`, "the + * only op this command touches") and then recovers the dialect by matching the + * finding's message against {@link manualJsonConversionSql}. Reusing that op + * would hand this finding to a command whose remedy converts the column to + * **json** — the wrong column type for a signature — and, since this message + * embeds no json statement, the command would file it under + * `remedy_not_recognized`: one refusal line per finding, on every run, for a + * divergence that command has no business with. + * + * ⛔ There is NO reconciler arm, deliberately. `applyDriftOpInPlace` matches + * no case, so `applyMigrationEntries` reports the entry as skipped and logs + * it. Whether ObjectStack should run the `ALTER` itself is a separate decision + * with hazards this differ must not pre-empt: on MySQL a `MODIFY` restates the + * WHOLE column definition (silently dropping a NOT NULL or DEFAULT that is not + * repeated) and a TEXT column cannot carry a plain index without a prefix + * length, so the conversion can turn a working table into one whose declared + * index no longer exists. + * + * `from` is the physical type word and `to` is always `'text'`, spelled the way + * `manual_column_type_change` spells them so a renderer showing `from → to` + * needs no new arm. + */ + | { type: 'manual_widen_varchar_to_text'; table: string; column: string; to: 'text'; from: string } /** * Retire the legacy platform-wide UNIQUE index on a now-tenant-scoped field * and put the composite `(tenantField, field)` in its place (#3696). The two @@ -483,6 +515,46 @@ function acceptsStringifiedJson(type: string | undefined): boolean { return /char|text/i.test(String(type ?? '')); } +/** + * The field types whose column is TEXT whenever the field declares no usable + * `maxLength` — `createColumn`'s text-family case (#11794 / #11875). + * + * ⚠️ Read the scope precisely: these are the types for which the emitter's + * answer is TEXT **regardless of whether an index keys the column**. That + * independence is what licenses the #12121 branch below to report on this shape + * without being told which columns are keyed, and it is not an assumption — it + * falls out of the emitter's own expression, `keyable = keyed ? + * keyableTextLength(field) : null`, whose keyed arm returns `null` for a field + * with no positive-integer bound. A text-family field that DID declare a keyable + * bound takes `varchar(maxLength)` when keyed, which is exactly why that branch + * is gated on the declaration being ABSENT and never fires for it. + * + * ⛔ NOT derived from the spec's `BOUNDED_STRING_FIELD_TYPES`, for the reason + * `sql-driver-12017-bounded-string-spec-parity.test.ts` argues in full: that set + * answers "may this type declare a bound?" and carries no varchar/TEXT + * partition, so deriving would have to INVENT an answer for every future member + * at the one seam where the maintainer has actually ruled per type. It is a + * hand-written list and is therefore PINNED rather than trusted — + * `schema-drift.12121-unbounded-text-column.test.ts` probes the driver's OWN + * dispatch (`varcharColumnChars`) over every `FieldType` the spec declares and + * asserts set equality in both directions, plus the keyed-and-unkeyed `null` + * above for every member. A type entering or leaving `createColumn`'s text + * family reds there by name. + * + * ⚠️ It lives here rather than being imported from `sql-driver.ts` because the + * dependency runs the other way — `sql-driver.ts` imports this module — so an + * import would be a cycle. The pin is what stands in for a shared constant. + * + * ⛔ Module-exported so this package's own suites can pin it, and deliberately + * NOT added to `index.ts` — the same call {@link MULTI_VALUE_COLUMN_REMEDY_COMMAND} + * makes: nothing outside this package has a question this set answers. + */ +export const UNBOUNDED_TEXT_FIELD_TYPES: ReadonlySet = new Set([ + 'text', 'textarea', 'html', 'markdown', 'richtext', 'code', + // #11875 — joined the family once the write seam enforced their declared bound. + 'signature', 'qrcode', +]); + /** * Does a multi-value field's JSON column carry its type on THIS dialect — i.e. * does a stale textual column silently corrupt the value (#11535)? @@ -863,6 +935,93 @@ export function diffManagedTable(args: { }); } } + + // ── an UNBOUNDED text-family field over a pre-existing varchar (#12121) ── + // + // The exact COMPLEMENT of the branch above: that one REQUIRES + // `declaredMaxLength !== undefined`, so on a pre-existing table the two + // partition the text family by whether its author wrote a number. + // + // Until this branch existed the undeclared half was reported by NOTHING, and + // that half is the common case. Measured on the pre-fix tree, one + // `diffManagedTable` call per type: a `text` / `textarea` / `html` / + // `markdown` / `richtext` / `code` / `signature` / `qrcode` field with no + // `maxLength` over a `character varying(255)` column returned **zero** + // entries on both enforcing dialects, while `{ type: 'signature', maxLength: + // 4096 }` over the same column returned `widen_varchar` in the same run — so + // the differ was working and this shape was simply invisible to it. + // + // What that silence costs: after #11875/#12119 a NEWLY created column for + // these types is TEXT and holds a data URI correctly, but the additive sync + // never revisits an existing column, so a deployment upgrading into that + // release gets no change AND no diagnostic. The server keeps refusing the + // same write, and the refusal is a poor substitute for a report: the live + // probe behind `objectql`'s `driver-fault-redaction.ts` measured Postgres's + // `22001` as identifier-only and naming the TYPE rather than the column + // (`value too long for type character varying(255)`), MySQL's `1406` as + // `Data too long for column 'label' at row 1`. Meanwhile every + // drift-reporting road in the platform — `os migrate plan`, `os migrate + // apply`, the artifact-pinned boot gate, the boot-time `[schema-drift]` warn + // — reads THIS function, so the one place that could have named the column + // and the cause named nothing at all. + // + // ## Why this needs no keyed-column input + // + // `createColumn` sizes a text-family column as `keyed ? + // keyableTextLength(field) : null`, and `keyableTextLength` returns `null` + // for a field with no positive-integer bound. So for the fields this branch + // SELECTS the emitter answers TEXT whether or not an index keys them: the + // differ does not have to know, and cannot be wrong about it. Pinned as such + // — see {@link UNBOUNDED_TEXT_FIELD_TYPES}. + // + // ## Severity `error`, category `needs_confirm` — and the category is the + // ## load-bearing half, exactly as it is for the base-type branch above + // + // ⛔ Do NOT "correct" `needs_confirm` to `destructive` to match how bad it + // sounds. `runArtifactBootMigrationGate` refuses a boot for `category === + // 'destructive'` and for nothing else, and every database this finding + // describes is ALREADY SERVING — that is the premise of the report. A + // `destructive` spelling would convert a deployment that merely refuses + // over-long values into a crash-loop on its next restart. `safe` is wrong in + // the other direction: dev auto-reconcile applies `safe` entries unattended + // and there is no arm to apply. + // + // `severity` is read by NO gate — it is render weight — and `error` is the + // honest weight for the same reason the base-type branch takes it: there is + // no automatic repair, so the operator has to act. + if ( + enforcesVarcharLength(dialect) && + !declaresJsonColumn && + declaredMaxLength === undefined && + UNBOUNDED_TEXT_FIELD_TYPES.has(field.type || 'string') && + isCharacterColumn(col.type) && + typeof col.maxLength === 'number' + ) { + out.push({ + kind: 'type_mismatch', + remoteName: table, + table, + column: fieldName, + expected: 'text', + actual: `varchar(${col.maxLength})`, + severity: 'error', + category: 'needs_confirm', + op: { type: 'manual_widen_varchar_to_text', table, column: fieldName, to: 'text', from: col.type }, + message: + `${table}.${fieldName}: metadata declares \`${field.type || 'string'}\` with no ` + + `\`maxLength\`, so ObjectStack creates this column as TEXT — but the existing column is ` + + `\`varchar(${col.maxLength})\` and the additive sync never changes a column's type. The ` + + `column still caps at ${col.maxLength} characters, so the server refuses longer values the ` + + `declaration ALLOWS (Postgres 22001, MySQL ER_DATA_TOO_LONG) — a data URI in a ` + + `\`signature\`/\`qrcode\` field, or an ordinary rich-text body, is routinely past it ` + + `(#12121). ObjectStack does NOT migrate this column: "os migrate apply" reports this entry ` + + `as skipped. Two operator routes — declare a \`maxLength\` this dialect can express, which ` + + `turns this into the widen op "os migrate apply" performs; or convert the column to TEXT by ` + + `hand, with a backup taken first, restating the FULL column definition on MySQL (MODIFY ` + + `drops a NOT NULL or DEFAULT you do not repeat) and dropping any index that keys the column ` + + `first, since MySQL cannot key a TEXT column without a prefix length.`, + }); + } } // ── orphaned columns (physical column, no metadata field) ────────── From 4953795a5fbf78255135235f33fd9bc2b2a67e05 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 14:24:46 +0000 Subject: [PATCH 2/4] test(driver-sql): pin the unbounded text-family drift op, its silences, and the set --- .../drivers/driver-sql/src/schema-drift.ts | 2 +- ...schema-drift.unbounded-text-column.test.ts | 312 ++++++++++++++++++ 2 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 packages/drivers/driver-sql/src/schema-drift.unbounded-text-column.test.ts diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 538015936d..3c521e01d1 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -535,7 +535,7 @@ function acceptsStringifiedJson(type: string | undefined): boolean { * partition, so deriving would have to INVENT an answer for every future member * at the one seam where the maintainer has actually ruled per type. It is a * hand-written list and is therefore PINNED rather than trusted — - * `schema-drift.12121-unbounded-text-column.test.ts` probes the driver's OWN + * `schema-drift.unbounded-text-column.test.ts` probes the driver's OWN * dispatch (`varcharColumnChars`) over every `FieldType` the spec declares and * asserts set equality in both directions, plus the keyed-and-unkeyed `null` * above for every member. A type entering or leaving `createColumn`'s text diff --git a/packages/drivers/driver-sql/src/schema-drift.unbounded-text-column.test.ts b/packages/drivers/driver-sql/src/schema-drift.unbounded-text-column.test.ts new file mode 100644 index 0000000000..724c91f40c --- /dev/null +++ b/packages/drivers/driver-sql/src/schema-drift.unbounded-text-column.test.ts @@ -0,0 +1,312 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12121 — a TEXT-family field that declares NO bound, over a PRE-EXISTING + * `varchar` column, was reported by nothing. + * + * ## The defect, as measured on the pre-fix tree + * + * The varchar differ's whole branch required `declaredMaxLength !== undefined`. + * On a pre-existing table that split the text family in two by whether its + * author had written a number: + * + * ``` + * Field.signature({ maxLength: 4096 }) over varchar(255) -> widen_varchar ✅ reported + * Field.signature() — no bound over varchar(255) -> (nothing) ⛔ silent + * ``` + * + * One `diffManagedTable` call per type on the pre-fix tree, dialect `postgres`, + * column `varchar(255)`: `text` / `textarea` / `html` / `markdown` / `richtext` / + * `code` / `signature` / `qrcode` with no `maxLength` each returned **zero** + * entries — and `{ type: 'signature', maxLength: 4096 }` over the same column + * returned exactly one `widen_varchar` in the same run. So the differ was + * working; this shape was simply invisible to it. + * + * ⭐ **A drift op that reports nothing is indistinguishable from no drift**, + * which is the whole hazard: after #11875/#12119 a NEWLY created column for + * these types is TEXT and holds a data URI correctly, but the additive sync + * never revisits an existing one — so a deployment upgrading into that release + * gets no change AND no diagnostic, while the server keeps refusing the same + * write. That refusal is a poor substitute for a report: the live probe behind + * `objectql`'s `driver-fault-redaction.ts` measured Postgres's `22001` as + * identifier-only and naming the TYPE rather than the column (`value too long + * for type character varying(255)`). + * + * ## What each block below is worth + * + * 1. **The emission**, over every member, with the declared-bound row as the + * POSITIVE CONTROL in the same run — the thing that proves the differ can + * emit at all here, so a green count is a measurement rather than a mood. + * 2. **The silences**, which are what make the emission a detector rather than + * a blanket. Each one is a shape where the emitter and the column already + * AGREE, so a finding would be a false positive. + * 3. **The set pin.** `UNBOUNDED_TEXT_FIELD_TYPES` is a hand-written list, and + * a hand-written copy of `createColumn`'s case list is the exact defect + * #11794 was filed about. It is therefore held equal to the driver's OWN + * dispatch over every `FieldType` the spec declares, both directions, with + * this file writing down neither list. + * 4. **The keyedness-independence pin**, which is the licence for the branch's + * predicate: it decides without being told which columns are keyed, and that + * is only sound because the emitter answers TEXT for these fields either way. + * 5. **The two couplings that would be silent if broken** — the boot gate reads + * `category`, and `os migrate multi-value-columns` selects its entire + * population by `op.type`. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { FieldType } from '@objectstack/spec/data'; +import { SqlDriver } from './sql-driver.js'; +import { + diffManagedTable, + UNBOUNDED_TEXT_FIELD_TYPES, + type ManagedDriftEntry, + type PhysicalColumn, + type SqlDialectName, +} from './schema-drift.js'; +import { dialectCell } from './live-dialect-matrix.testkit.js'; + +const T = 'os12121_probe'; +const C = 'body'; + +/** The column a long-lived deployment still carries, in a dialect's own spelling. */ +const staleColumn = (type: string, maxLength?: number): PhysicalColumn[] => [ + { name: C, type, nullable: true, ...(maxLength === undefined ? {} : { maxLength }) }, +]; + +const diffBody = ( + field: Record, + columns: PhysicalColumn[], + dialect: SqlDialectName = 'postgres', +): ManagedDriftEntry[] => diffManagedTable({ table: T, fields: { [C]: field } as never, columns, dialect }); + +/** The two dialects that physically enforce a varchar width (SQLite does not). */ +const ENFORCING: readonly SqlDialectName[] = ['postgres', 'mysql']; + +describe('diffManagedTable — an unbounded text-family field over a pre-existing varchar (#12121)', () => { + let driver: SqlDriver; + afterEach(async () => { + await driver?.disconnect().catch(() => {}); + }); + + /** + * ⭐ THE ASSERTION. Every member of the text family, declaring no bound, over + * a `varchar(255)` a previous release created — reported exactly once, on + * both enforcing dialects. + * + * The declared-bound row rides along as the POSITIVE CONTROL: it is the case + * that was ALREADY reported before this branch existed, so its `widen_varchar` + * in this same run is what distinguishes "the differ found the new shape" + * from "the differ is emitting for everything". Both counts are exact — a + * `toBeGreaterThan(0)` here would pass just as well on a differ that reported + * every column in the table. + */ + it('reports every member exactly once, with the declared-bound row as the control in the same run', () => { + const members = [...UNBOUNDED_TEXT_FIELD_TYPES].sort(); + expect(members.length).toBeGreaterThan(5); // the set is real, not an empty loop + + for (const dialect of ENFORCING) { + for (const type of members) { + const found = diffBody({ type }, staleColumn('varchar', 255), dialect); + expect(found, `${type} on ${dialect}`).toHaveLength(1); + const [d] = found; + expect(d.op.type).toBe('manual_widen_varchar_to_text'); + expect(d.kind).toBe('type_mismatch'); + expect(d.expected).toBe('text'); + expect(d.actual).toBe('varchar(255)'); + expect(d.op).toMatchObject({ table: T, column: C, to: 'text', from: 'varchar' }); + + // The message has to carry what an operator acts on: which declaration, + // how wide the column actually is, and that nothing runs by itself. + expect(d.message).toContain(`${T}.${C}`); + expect(d.message).toContain(type); + expect(d.message).toContain('varchar(255)'); + expect(d.message).toContain('os migrate apply'); + // ⛔ NOT the multi-value command: its remedy converts the column to + // `json`, which is the wrong column type for every member here. + expect(d.message).not.toContain('multi-value-columns'); + } + + // ── POSITIVE CONTROL, same shape, same run: a declared bound is still + // the pre-existing `widen_varchar`, untouched by this branch. + const control = diffBody({ type: 'signature', maxLength: 4096 }, staleColumn('varchar', 255), dialect); + expect(control, `control on ${dialect}`).toHaveLength(1); + expect(control[0].op).toMatchObject({ type: 'widen_varchar', to: 4096, from: 255 }); + expect(control[0].severity).toBe('warning'); + expect(control[0].category).toBe('safe'); + } + }); + + /** + * The silences — every one of them a shape where the emitter and the physical + * column already agree, so a finding would be a false positive rather than a + * detection. Without this block the branch above is satisfied by a differ that + * simply reports more. + */ + it('stays silent wherever the emitter and the column already agree', () => { + // A column the current driver would create: TEXT. Nothing to report. + expect(diffBody({ type: 'signature' }, staleColumn('text'))).toHaveLength(0); + + // MySQL reports a TEXT column's `character_maximum_length` as 65535 — the + // #11431 defect. `isCharacterColumn` is what keeps it out, and this is the + // case that would re-open it one door to the left. + expect(diffBody({ type: 'signature' }, staleColumn('text', 65535), 'mysql')).toHaveLength(0); + + // SQLite records a declared type and enforces nothing, so a `varchar(255)` + // there refuses no value the declaration allows — the same exclusion + // `enforcesVarcharLength` already makes for widen/narrow. + expect(diffBody({ type: 'signature' }, staleColumn('varchar', 255), 'sqlite')).toHaveLength(0); + + // The varchar family. `createColumn` gives an unbounded `string` / `email` / + // `url` / `phone` / `password` knex's varchar(255) — which is exactly the + // column on disk, so there is no divergence. + for (const type of ['string', 'email', 'url', 'phone', 'password']) { + expect(diffBody({ type }, staleColumn('varchar', 255)), type).toHaveLength(0); + } + expect(diffBody({}, staleColumn('varchar', 255))).toHaveLength(0); // untyped -> 'string' + + // A multi-value field is a `json` column whatever its element type would + // have been, and the base-type branch (#11535) already owns that shape. + // Reporting it here too would give one column two contradictory remedies. + const multi = diffBody({ type: 'signature', multiple: true }, staleColumn('varchar', 255)); + expect(multi).toHaveLength(1); + expect(multi[0].op.type).toBe('manual_column_type_change'); + }); + + /** + * ⭐ THE SET PIN. `UNBOUNDED_TEXT_FIELD_TYPES` === the types the driver's own + * dispatch puts in `createColumn`'s text-family branch, over the spec's whole + * `FieldType` vocabulary. + * + * Both directions are load-bearing, and neither list is written down here: + * + * - `⊇` — a type that JOINS the emitter's text family (as `signature` and + * `qrcode` did at #11875) and is not added here goes back to being + * silent, which is this card's defect re-entering by the door it came in. + * - `⊆` — a type listed here that the emitter does NOT make TEXT would be + * reported as needing a conversion to a column shape the platform would + * never create: a finding an operator can act on and be left with drift. + * + * The classification PROBES the driver rather than restating its cases — the + * technique `sql-driver-12017-bounded-string-spec-parity.test.ts` introduced, + * and the reason that file carries no case list either. + */ + it('holds the set equal to the driver text-family branch over every FieldType', () => { + driver = new SqlDriver(dialectCell('sqlite').config()); + const types = FieldType.options as readonly string[]; + expect(types.length).toBeGreaterThan(40); // the spec registry really was read + + const textFamily = types.filter((t) => shapeOf(driver, t) === 'text-family').sort(); + const declared = [...UNBOUNDED_TEXT_FIELD_TYPES].sort(); + + // Non-vacuity: the probe resolved branches OTHER than the one under test, + // so an equality between two empty-ish sets cannot pass for a measurement. + const shapes = [...new Set(types.map((t) => shapeOf(driver, t)))]; + expect(shapes).toContain('varchar-from-declaration'); + expect(shapes).toContain('default-varchar-255'); + expect(shapes).toContain('not-sized-from-metadata'); + + expect(textFamily).toEqual(declared); + }); + + /** + * ⭐ THE LICENCE for the branch's predicate. `diffManagedTable` is never told + * which columns an index keys, and the branch decides anyway. That is sound + * only because, for a text-family field with NO usable bound, the emitter + * answers TEXT either way: + * + * createColumn: keyable = keyed ? keyableTextLength(field) : null + * keyableTextLength(no positive-integer maxLength) === null + * + * Measured here instead of read off the source, and paired with its CONTRAST: + * the same type WITH a keyable bound takes `varchar(bound)` when keyed, which + * is precisely why the branch is gated on the declaration being ABSENT. Drop + * that gate and a keyed, bounded text column — a column the emitter itself + * would create as `varchar(n)` — starts being reported as drift. + */ + it('pins that an unbounded text-family field is TEXT whether or not it is keyed', () => { + driver = new SqlDriver(dialectCell('sqlite').config()); + const chars = (field: unknown, keyed?: { unique: boolean }) => + (driver as unknown as { varcharColumnChars(f: unknown, k?: { unique: boolean }): number | null }) + .varcharColumnChars(field, keyed); + + for (const type of UNBOUNDED_TEXT_FIELD_TYPES) { + // No bound at all, and the malformed spellings the emitter treats as none + // (#11431) — the differ's own `declaredMaxLength` uses the same predicate. + for (const field of [{ type }, { type, maxLength: 0 }, { type, maxLength: 12.5 }]) { + expect(chars(field, undefined), `${type} unkeyed`).toBeNull(); + expect(chars(field, { unique: false }), `${type} keyed`).toBeNull(); + expect(chars(field, { unique: true }), `${type} unique`).toBeNull(); + } + // ⚠️ THE CONTRAST — a keyable bound IS honoured when keyed, so the gate on + // "no declaration" is doing real work and is not a redundant condition. + expect(chars({ type, maxLength: 700 }, { unique: true }), `${type} bounded+keyed`).toBe(700); + } + }); + + /** + * The two couplings that fail SILENTLY if this entry is spelled wrong. Neither + * is visible from the emission assertions above. + */ + it('cannot refuse a boot, and cannot be claimed by "os migrate multi-value-columns"', () => { + const [d] = diffBody({ type: 'richtext' }, staleColumn('varchar', 255)); + + // `runArtifactBootMigrationGate` refuses a boot for `category === + // 'destructive'` and nothing else. Every database this finding describes is + // ALREADY SERVING, so `destructive` would turn a deployment that merely + // refuses over-long values into a crash-loop on its next restart. `safe` is + // wrong the other way: dev auto-reconcile applies those unattended, and + // there is no reconciler arm to apply. + expect(d.category).toBe('needs_confirm'); + expect(d.severity).toBe('error'); + + // `os migrate multi-value-columns` selects its ENTIRE population by + // `op.type === 'manual_column_type_change'` and then recovers the dialect by + // matching the message against `manualJsonConversionSql`. Sharing that op + // would hand this finding to a command whose remedy makes the column `json`, + // and — the message carrying no json statement — have it refused as + // `remedy_not_recognized` on every run. + expect(d.op.type).not.toBe('manual_column_type_change'); + }); +}); + +/** + * The bound every probe declares: past `DEFAULT_STRING_VARCHAR_CHARS` (255) so + * an honoured declaration is distinguishable from the catch-all, and inside both + * `MAX_KEYABLE_VARCHAR_CHARS` (768) and `MAX_VARCHAR_CHARS` (16383) so neither + * ceiling turns the answer into `null` and mis-files the type. + */ +const PROBE_CHARS = 700; + +type ColumnShape = + | 'varchar-from-declaration' + | 'text-family' + | 'default-varchar-255' + | 'not-sized-from-metadata'; + +/** + * Which branch of `createColumn`'s switch a type takes, read off the driver's + * own dispatch. The keyed probe is what separates `text-family` from + * `not-sized-from-metadata` at all — unkeyed, both answer `null`. + * + * An answer outside the table is a THROW, never a default bucket: a switch that + * grew a fifth behaviour must be classified on purpose, and filing it silently + * under "not sized" would be this card's own defect committed by its own guard. + */ +function shapeOf(driver: SqlDriver, type: string): ColumnShape { + const dflt = (SqlDriver as unknown as { DEFAULT_STRING_VARCHAR_CHARS: number }).DEFAULT_STRING_VARCHAR_CHARS; + const mirror = (keyed?: { unique: boolean }) => + (driver as unknown as { varcharColumnChars(f: unknown, k?: { unique: boolean }): number | null }) + .varcharColumnChars({ type, maxLength: PROBE_CHARS }, keyed); + const unkeyed = mirror(undefined); + const keyed = mirror({ unique: false }); + + if (unkeyed === PROBE_CHARS && keyed === PROBE_CHARS) return 'varchar-from-declaration'; + if (unkeyed === null && keyed === PROBE_CHARS) return 'text-family'; + if (unkeyed === dflt && keyed === dflt) return 'default-varchar-255'; + if (unkeyed === null && keyed === null) return 'not-sized-from-metadata'; + throw new Error( + `#12121: varcharColumnChars answered (unkeyed=${String(unkeyed)}, keyed=${String(keyed)}) for ` + + `type '${type}' at maxLength ${PROBE_CHARS} — a branch this guard does not classify. ` + + `Classify it on purpose rather than widening a bucket.`, + ); +} From 1a30fa37fdf9bcc7444690f748a60326eb4920e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 14:32:42 +0000 Subject: [PATCH 3/4] chore(changeset): unbounded text-family column drift report --- ...nbounded-text-column-over-stale-varchar.md | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 .changeset/drift-unbounded-text-column-over-stale-varchar.md diff --git a/.changeset/drift-unbounded-text-column-over-stale-varchar.md b/.changeset/drift-unbounded-text-column-over-stale-varchar.md new file mode 100644 index 0000000000..6807b4bf08 --- /dev/null +++ b/.changeset/drift-unbounded-text-column-over-stale-varchar.md @@ -0,0 +1,68 @@ +--- +'@objectstack/driver-sql': minor +--- + +Report an **unbounded** text-family field left on a pre-existing `varchar` +column, instead of leaving the operator with a refused write and no diagnostic + +After #11875/#12119 a **newly created** `signature` / `qrcode` column is TEXT and +holds a data URI correctly. `initObjects` is additive-only, so on a database +created by an earlier release nothing is missing, nothing is added, and the old +`varchar(255)` column is kept forever — the boundary #12119's own changeset +states in as many words. What was not stated is what the drift reporter did +about it, and the answer was **nothing**. + +The varchar differ's entire branch required `declaredMaxLength !== undefined`, so +on a pre-existing table it split the text family by whether its author had +written a number: + +``` +Field.signature({ maxLength: 4096 }) over varchar(255) -> widen_varchar reported +Field.signature() — no bound over varchar(255) -> (nothing) silent +``` + +The second row is the common case. Measured on the pre-fix tree, one +`diffManagedTable` call per type on dialect `postgres` against a `varchar(255)` +column: `text` / `textarea` / `html` / `markdown` / `richtext` / `code` / +`signature` / `qrcode` with no `maxLength` each returned **zero** entries, while +`{ type: 'signature', maxLength: 4096 }` over the same column returned exactly +one `widen_varchar` in the same run — so the differ was working and this shape +was simply invisible to it. An upgrading deployment therefore saw no change and +no diagnostic, while the server kept refusing the same write; and the refusal is +a poor substitute for a report, because the live probe behind objectql's +`driver-fault-redaction.ts` measured Postgres's `22001` as identifier-only and +naming the **type** rather than the column (`value too long for type character +varying(255)`). + +The divergence is now **detected and reported** under a new report-only +`manual_widen_varchar_to_text` op, naming the declared type, the physical width, +the consequence, and both operator routes. Same `declared ≠ enforced` shape as +the #11374 / #11431 / #11875 family, closed one door further along — at the +migration seam rather than the authoring or write seam. + +**Nothing is migrated for you, and nothing new is refused.** There is no +reconciler arm: `os migrate apply` reports the entry as skipped, exactly as it +does for `manual_column_type_change`. The entry is `category: 'needs_confirm'`, +so the artifact-pinned boot gate — which refuses a boot for `destructive` and +nothing else — is unaffected: a deployment that merely refuses over-long values +must not become a crash-loop on its next restart. Dev auto-reconcile takes +`safe` only, so it never applies this unattended either. SQLite is excluded: it +enforces no declared width, so there is no divergence to report. + +`manual_widen_varchar_to_text` is a **distinct** op rather than a second use of +`manual_column_type_change`, for a measured reason: `os migrate +multi-value-columns` selects its entire population by +`op.type === 'manual_column_type_change'` and recovers the dialect by matching +the message against `manualJsonConversionSql`, so sharing the op would hand this +finding to a command whose remedy makes the column `json` — and, the message +carrying no json statement, have it refused as `remedy_not_recognized` on every +run. + +Graded `minor` rather than `patch` on two counts, matching the sibling drift-op +addition that shipped for #11535: `detectManagedDrift` emits a finding on +existing deployments where it previously emitted none (visible in `os migrate +plan`, in `os migrate apply`'s skipped count and in the boot-time +`[schema-drift]` warn), and the exported `DriftOp` union gains a member, which is +additive for producers but widens a type any consumer switching exhaustively +over it must account for. Nothing is removed, renamed or newly rejected, so it is +not a breaking change. From 038193327cd022fe69e3044a8649607ad99d043f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 15:07:28 +0000 Subject: [PATCH 4/4] docs(cli): correct the drift-op superlative this PR falsifies (#12121) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `manual_widen_varchar_to_text` is, by design, a second drift op `os migrate apply` never applies — schema-drift.ts's own comments say so. Two sites in cli.mdx claimed it was the only one: - The `os migrate` command table (:546) said `apply` never reconciles "the one drift op" that `multi-value-columns` migrates. - The `#### os migrate multi-value-columns` section opener (:670) made the same "the one drift op" claim. Both now say "one of two" and name the new op next to `manual_column_type_change`, without documenting it at length here — that belongs to the driver, not this command's doc section. Also reworded the `needs_confirm` category table's "Applied by" cell (:635): unlike `manual_column_type_change` (applied by `os migrate multi-value-columns --apply`), `manual_widen_varchar_to_text` has no applier at all — nothing in the CLI references it. Left silently as `os migrate apply` it would read as false for this one entry. PM rework request: PR #12733 comment 5441049227. --- content/docs/deployment/cli.mdx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 08475fb96f..c9dc4dba1a 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -543,7 +543,7 @@ diverges from the live schema, and the physical column wins at write time. |---------|-------------| | `os migrate plan` | Dry-run: show how the database has drifted from metadata, categorised safe / needs-confirm / destructive (no changes applied) | | `os migrate apply` | Reconcile the database to metadata. Applies loosening changes; destructive ones require `--allow-destructive` | -| `os migrate multi-value-columns` | Migrate a stale `varchar`/`text` column to `json` where the field declares `multiple: true` — the one drift op `apply` never reconciles for you. Dry run by default; `--apply` runs the statement the finding prints | +| `os migrate multi-value-columns` | Migrate a stale `varchar`/`text` column to `json` where the field declares `multiple: true` — one of two drift ops `apply` never reconciles for you. Dry run by default; `--apply` runs the statement the finding prints | ```bash os migrate plan # Preview drift (no changes) @@ -632,7 +632,7 @@ occupancy on its own. | Category | Examples | Applied by | |----------|----------|------------| | `safe` | relax `NOT NULL` → nullable, widen a `varchar`, create a declared index, replace a legacy installation-wide unique with its per-organization composite | `os migrate apply` (and dev auto-reconcile) | -| `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` | +| `needs_confirm` | non-narrowing type change, rebuild a non-unique index whose columns changed | `os migrate apply` — except `manual_widen_varchar_to_text`, which nothing applies | | `destructive` | drop an orphaned column or index, tighten `NOT NULL`, narrow a type, rebuild an index as `UNIQUE` | `os migrate apply --allow-destructive` | #### Index drift @@ -667,7 +667,7 @@ it reconciles via a table rebuild (copy → swap) that preserves your data. #### `os migrate multi-value-columns` -The one drift op `os migrate apply` will **never** apply for you. +`os migrate apply` will **never** apply this drift op — and it isn't the only one: `manual_widen_varchar_to_text` (an unbounded text-family field left on a pre-existing `varchar` column) is also never applied, but has no `os migrate` subcommand of its own. This section covers the op that does. A field that gains `multiple: true` over a database that already exists keeps its old `varchar` / `text` column: the additive sync adds columns, and never