From eab52a7247067587a07057617c42ef6b648dbef2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 12:07:06 +0000 Subject: [PATCH 1/6] fix(cli): generated migrations emit the character column driver-sql creates (#16091) Both migration generators capped a `text` field at VARCHAR(255) while `driver-sql` creates an unbounded `text` column for it, so a 300-character value the platform stores was refused by every generated table with `value too long for type character varying(255)`. #15521's ruling names this card and settles its direction -- the generator follows the driver, as #15040 already did for the `id` column in this same file. Driven on a private PostgreSQL 16.13 cluster, all three producers run from one object and their columns read back out of `information_schema.columns`. The sweep found nine divergent columns of 26 probed, not one: text driver text gen varchar(255) both formats text+max driver text gen varchar(255) maxLength must NOT size it email+max driver varchar(400) gen varchar(255) maxLength was never read url driver varchar(255) sql varchar(2048) invented width phone driver varchar(255) sql varchar(50) invented width color driver varchar(255) sql varchar(7) invented width All of them now follow `createColumn`'s three arms. The text family is unbounded, because that arm branches on KEYED and a generated migration emits no index; its declared bound is enforced at the write seam, not by the column. The string family takes `declaredVarcharLength`'s answer -- the declaration verbatim in both directions, knex's 255 without one, and TEXT above the varchar ceiling rather than a clamp to it. The catch-all keeps the default width and ignores a declaration, because its stored value is an option code or another row's id rather than the declared string. Driven again afterwards: 0 of 26 columns diverge, and the 300-character write is accepted in all three tables exactly where the platform accepts it and refused in all three exactly where the platform refuses it. `generate-string-family-width.pin.test.ts` asserts that agreement against the driver's own source -- arm membership read from `createColumn`'s case labels, widths read from its own constants -- so a driver that moves fails there instead of leaving the generators quietly wrong. Three existing pin files move with it: two used `text`'s old VARCHAR(255) as a stand-in for the driver's default string column, and one asserted column ordering by searching for a `table.string` call that is now a `table.text` call. Scope is PostgreSQL, the only dialect `--format sql` claims (#15521). The FILE_REFERENCE_TYPES divergence stays recorded and unresolved (#15041). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- ...rated-migration-character-column-widths.md | 11 + .../generate-builtin-id-column.pin.test.ts | 8 +- ...generate-field-type-vocabulary.pin.test.ts | 13 +- .../generate-multiple-json-column.pin.test.ts | 14 +- .../generate-string-family-width.pin.test.ts | 476 ++++++++++++++++++ packages/cli/src/commands/generate.ts | 196 +++++++- 6 files changed, 703 insertions(+), 15 deletions(-) create mode 100644 .changeset/generated-migration-character-column-widths.md create mode 100644 packages/cli/src/commands/generate-string-family-width.pin.test.ts diff --git a/.changeset/generated-migration-character-column-widths.md b/.changeset/generated-migration-character-column-widths.md new file mode 100644 index 0000000000..cf70fb1161 --- /dev/null +++ b/.changeset/generated-migration-character-column-widths.md @@ -0,0 +1,11 @@ +--- +"@objectstack/cli": patch +--- + +`os generate migration` now emits the character column `driver-sql` actually creates, in both the TypeScript and the SQL format. + +A `text` field took `VARCHAR(255)` from both generators while the platform creates an unbounded `text` column for it, so a 300-character value the platform stores was refused by every generated table with `value too long for type character varying(255)`. Enumerating the whole character-column family found the same disagreement in eight more places: `url` and `phone` and `color` carried widths the generators invented (2048, 50 and 7 against the platform's 255), and neither generator read a field's declared `maxLength` at all, so a `maxLength: 400` email was `varchar(400)` on the platform and `varchar(255)` in the migration generated for it. + +All of them now follow the platform's own three answers: the text family is unbounded (its declared bound is enforced at the write seam, not by the column), the string family takes its declared `maxLength` — verbatim in both directions, and TEXT rather than a clamp when it exceeds what a `varchar` can express — and the remaining string-valued types keep the default width and ignore a declaration, because their stored value is an option code or another row's id rather than the declared string. + +This scopes to PostgreSQL, which is the only dialect `os generate migration --format sql` claims. diff --git a/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts b/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts index 39c3b589d4..944d02afc2 100644 --- a/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts +++ b/packages/cli/src/commands/generate-builtin-id-column.pin.test.ts @@ -226,7 +226,13 @@ describe('the builtin id column both migration generators emit (#15040)', () => expect(sql.indexOf('"id"')).toBeLessThan(sql.indexOf('"title"')); const ts = generateMigrationTs(CONFIG as Record); expect(ts).toContain("await db.schema.createTable('account'"); - expect(ts.indexOf("table.string('id')")).toBeLessThan(ts.indexOf("table.string('title')")); + // #16091 — matched on the field NAME rather than on its column method. The + // assertion is about ORDER (the primary key comes first), and a reader keyed + // to `table.string` silently became `indexOf(…) === -1` the moment `title`, + // a `text` field, moved to `table.text` — which reads as a passing + // "less than" only until you notice what it is less than. + expect(ts.indexOf("table.string('id')")).toBeLessThan(ts.indexOf("('title')")); + expect(ts.indexOf("('title')"), 'the title column vanished from the output').toBeGreaterThan(0); // Each generator carries exactly ONE hardcoded id line — the shape that let // these two disagree with the driver in the first place, and the reason a // fix to one of them can silently leave the other behind. Counted over the diff --git a/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts b/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts index cb94081b65..39a93d24f7 100644 --- a/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts +++ b/packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts @@ -567,7 +567,18 @@ describe('#14828 — the SQL answers are the platform’s, not this file’s inv expect(tsInterfaceType('number')).toBe('number'); // The driver's own answer for the headline member, read where it lives. expect(createColumnArm('autonumber')).toContain('table.string(name)'); - expect(sqlColumn('autonumber')).toBe(sqlColumn('text')); + // #16091 — compared against `lookup`, not against `text`. Both were + // `VARCHAR(255)` when this line was written, which made `text` a usable + // stand-in for "the driver's default string column"; it is not one any + // more. `createColumn` gives `text` its text-family arm (an unbounded TEXT + // for every unkeyed column) and gives `lookup` the same bare + // `table.string(name)` it gives `autonumber` — asserted here, from the + // driver, so the comparator cannot silently become a different question again. + expect(createColumnArm('lookup')).toContain('table.string(name)'); + expect(sqlColumn('autonumber')).toBe(sqlColumn('lookup')); + // Anti-vacuity: the comparator is a real, DIFFERENT answer from the + // text family's, so this equality is a measurement rather than a tautology. + expect(sqlColumn('autonumber')).not.toBe(sqlColumn('text')); expect(tsColumn('autonumber')).toBe("table.string('f_autonumber')"); }); diff --git a/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts b/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts index 61546b5398..37bdeada5e 100644 --- a/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts +++ b/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts @@ -165,8 +165,14 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces', // control cannot be satisfied by one column shape for everything either. expect(sqlColumn('single_lookup')).toBe('VARCHAR(255)'); expect(tsColumn('single_lookup')).toBe("table.string('single_lookup')"); - expect(sqlColumn('single_text')).toBe('VARCHAR(255)'); - expect(tsColumn('single_text')).toBe("table.string('single_text')"); + // #16091 — `text` is an unbounded TEXT column now, which is what + // `createColumn`'s text-family arm builds for every unkeyed column. The + // control is unweakened by that for exactly the reason the `lookup` note + // above gives: what it discriminates is scalar-vs-JSON, and TEXT is scalar. + expect(sqlColumn('single_text')).toBe('TEXT'); + expect(tsColumn('single_text')).toBe("table.text('single_text')"); + // …and it still discriminates: the scalar answer is not the JSON one. + expect(sqlColumn('single_text')).not.toBe(sqlColumn('multi_text')); expect(sqlColumn('single_file')).toBe('VARCHAR(2048)'); expect(tsInterfaceType('single_lookup')).toBe('string'); }); @@ -285,8 +291,8 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces', // this card does NOT touch is present in both outputs. Without it, "does // not contain" would pass on an empty string. expect(other).toContain('CREATE TABLE IF NOT EXISTS "probe" ('); - expect(other).toContain('"t" VARCHAR(255)'); - expect(otherTs).toContain("table.string('t')"); + expect(other).toContain('"t" TEXT'); + expect(otherTs).toContain("table.text('t')"); // A RENDERED string (prefix + counter + suffix), never an integer sequence. expect(other).toContain('"a" VARCHAR(255)'); diff --git a/packages/cli/src/commands/generate-string-family-width.pin.test.ts b/packages/cli/src/commands/generate-string-family-width.pin.test.ts new file mode 100644 index 0000000000..6e3fbd6d8c --- /dev/null +++ b/packages/cli/src/commands/generate-string-family-width.pin.test.ts @@ -0,0 +1,476 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * THE #16091 PIN: the CHARACTER column both migration generators emit is the + * character column `driver-sql` actually creates. + * + * ## The defect + * + * A `text` field took `VARCHAR(255)` from both generators while `createColumn` + * builds an unbounded `text` column for it. Driven on a live PostgreSQL 16.13 — + * one object, three tables, one 300-character insert into each: + * + * ``` + * driver f_text text ACCEPTED — read back at length 300 + * sql gen f_text varchar(255) REFUSED — value too long for type character varying(255) + * ts gen f_text varchar(255) REFUSED — same + * ``` + * + * A row the platform stores today could not be stored in a table generated for + * the same object. That is the hard-failure class of #15040's `22P02`, not the + * cosmetic-schema-diff class of #15521. + * + * ## The class is nine rows wide, not one + * + * The card named `text`. Enumerating every `FIELD_TYPE_SQL_MAP` entry and every + * `createColumn` arm that produces a character type, and driving all three + * producers into that same cluster, found **nine** divergent columns out of 26 + * probed. Three distinct causes, and all three are repaired together because + * they are one question — how wide is the character column — asked of one + * authority: + * + * ``` + * f_text driver text sql varchar(255) ts varchar(255) the card's row + * f_text_max driver text sql varchar(255) ts varchar(255) maxLength does NOT size it + * f_email_max driver varchar(400) sql varchar(255) ts varchar(255) maxLength was never read + * f_url driver varchar(255) sql varchar(2048) ts varchar(255) invented width + * f_url_max driver varchar(1024) sql varchar(2048) ts varchar(255) all three disagreed + * f_url_huge driver text sql varchar(2048) ts varchar(255) past the ceiling ⇒ TEXT + * f_phone driver varchar(255) sql varchar(50) ts varchar(255) invented width + * f_phone_max driver varchar(20) sql varchar(50) ts varchar(255) all three disagreed + * f_color driver varchar(255) sql varchar(7) ts varchar(255) invented width + * ``` + * + * Both directions are real failures, and the wide one is the quieter: + * + * - NARROW — the card's own shape, one type over. A 300-character value into + * the `maxLength: 400` email was accepted by the driver's table and refused + * by both generated ones. + * - WIDE — a 300-character url was ACCEPTED by the sql format's `varchar(2048)` + * table and REFUSED by the driver's own `varchar(255)`. The scaffold invited + * a value the platform will not keep, which no error message ever names. + * + * After the repair, all three producers were driven into the same cluster again + * and read back out of `information_schema.columns`: **0 of 26 columns diverge**, + * and the 300-character write is accepted in all three tables exactly where the + * platform accepts it and refused in all three exactly where the platform + * refuses it. + * + * ## The three arms, and why `maxLength` reaches only one of them + * + * `createColumn` sorts every character column into three arms, and they answer + * the declaration differently. That is the whole content of this pin: + * + * 1. TEXT FAMILY — `keyable === null ? table.text(name) : table.string(name, + * keyable)` where `keyable = keyed ? this.keyableTextLength(field) : null`. + * The branch is on KEYED. A generated migration emits no index, so no + * generated column is ever keyed and the answer is an unbounded TEXT + * column — `maxLength` declared or not. ⚠️ This is the half most likely to + * be "fixed" wrongly by a later reader: sizing a `text` column from its + * declaration looks like honouring the author and is this card's own + * defect pointed the other way. The bound is not lost, it is enforced at + * the write seam, which is what `schema-drift.ts` states in as many words: + * "A TEXT column refuses nothing a `maxLength` allows … the bound is + * enforced at the write seam." + * + * 2. STRING FAMILY — `declared === null ? table.text(name) : + * table.string(name, declared)` over `declaredVarcharLength(field)`, which + * reads `maxLength` UNCONDITIONALLY (no keyed requirement) and has three + * outcomes: the declaration verbatim, knex's 255 when there is no usable + * one, and TEXT above `MAX_VARCHAR_CHARS` — never a clamp to the ceiling, + * because a clamp reinstates the very defect. + * + * 3. CATCH-ALL — `JSON_COLUMN_TYPES.has(type) ? this.jsonColumn(table, name) + * : table.string(name)`, which never reads `maxLength` at all. The driver + * states the reason: the stored value is an option code, an opaque + * `sys_secret` ref or another row's id, not the declared string, so a + * declared bound would size the wrong string. + * + * ## Why this pin reads the driver instead of asserting the widths + * + * The same reason `generate-builtin-id-column.pin.test.ts` gives for the id + * column: the whole shape of this card is "the generator disagrees with the + * driver", so a pin that transcribed `TEXT` and `VARCHAR(255)` would re-create + * the defect one layer up and stay green on the day the driver moves. Every + * arm's MEMBERSHIP is read out of `createColumn`'s own case labels, both widths + * are read off the driver's own constants, and every extraction carries a + * non-vacuity control — a source reader that matched nothing would pass while + * measuring nothing. + * + * ⚠️ Scope, as `generateMigrationSql`'s docblock and the `--format` help text + * already say (#15521): this is a POSTGRESQL claim and nothing else. Neither + * generator reproduces the driver's dialect branching. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { FieldType } from '@objectstack/spec/data'; +import { describe, expect, it } from 'vitest'; + +import { generateMigrationSql, generateMigrationTs } from './generate.js'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const GENERATE_SOURCE = fs.readFileSync(path.resolve(HERE, 'generate.ts'), 'utf8'); + +/** The authority, read where it lives — never transcribed here. */ +const DRIVER_SQL_SRC = path.resolve(HERE, '../../../drivers/driver-sql/src'); +const SQL_DRIVER_SOURCE = fs.readFileSync(path.join(DRIVER_SQL_SRC, 'sql-driver.ts'), 'utf8'); +const SCHEMA_DRIFT_SOURCE = fs.readFileSync(path.join(DRIVER_SQL_SRC, 'schema-drift.ts'), 'utf8'); + +/** The authority for which types exist at all. Imported, never listed here. */ +const REAL_FIELD_TYPES: ReadonlySet = new Set(FieldType.options); + +// ── Reading `createColumn`'s arms out of the driver ───────────────────────── + +/** The body of `SqlDriver.createColumn`'s `switch (type)`. */ +function createColumnSwitch(): string { + const start = SQL_DRIVER_SOURCE.indexOf('protected createColumn('); + if (start < 0) throw new Error('createColumn moved or was renamed in driver-sql'); + const switchAt = SQL_DRIVER_SOURCE.indexOf('switch (type)', start); + if (switchAt < 0) throw new Error('the per-type switch inside createColumn moved'); + const end = SQL_DRIVER_SOURCE.indexOf('\n if (col) {', switchAt); + if (end < 0) throw new Error("could not bound createColumn's switch in driver-sql"); + return SQL_DRIVER_SOURCE.slice(switchAt, end); +} + +/** + * The WHOLE arm a type belongs to — from the first of its run of `case` labels + * through the `break;` / `return;` that ends it. + * + * Deliberately not "from this type's own label": the two families this pin is + * about are each one arm shared by several labels, and MEMBERSHIP is exactly + * what has to be read from the driver rather than listed here. Bounding the arm + * at the previous terminator is what makes {@link armMembers} able to see the + * labels that sit ABOVE the one it was asked about. + */ +function armContaining(type: string): string { + const body = createColumnSwitch(); + const at = body.indexOf(`case '${type}':`); + if (at < 0) throw new Error(`createColumn has no arm for '${type}' — the driver moved`); + const before = body.slice(0, at); + // The index just PAST the previous arm's terminator — not the terminator's + // own index, which would make every arm read as the single word `break;`. + let head = -1; + for (const term of ['break;', 'return;', 'switch (type) {']) { + const i = before.lastIndexOf(term); + if (i >= 0) head = Math.max(head, i + term.length); + } + if (head < 0) throw new Error(`could not find the head of '${type}'s arm in createColumn`); + const rest = body.slice(head); + const tail = rest.match(/^[\s\S]*?(?:break;|return;)/); + if (!tail) throw new Error(`unterminated createColumn arm for '${type}' in driver-sql`); + return tail[0]; +} + +/** + * The FieldType members one arm serves, read off its own case labels. + * + * `case 'string':` is filtered out here and that is not a convenience: `string` + * is not a `FieldType` member at all — there is no `Field.string` builder, + * `FieldType.options` omits it, and `FieldSchema.safeParse({ type: 'string' })` + * fails at `[type]` (#12593) — so it cannot arrive through an authored object + * and the generators have nothing to answer for it. + */ +function armMembers(type: string): string[] { + return [...armContaining(type).matchAll(/case '([^']+)':/g)] + .map((m) => m[1]) + .filter((t) => REAL_FIELD_TYPES.has(t)); +} + +/** `createColumn`'s catch-all — where an un-cased type lands. */ +function createColumnDefaultArm(): string { + const body = createColumnSwitch(); + const at = body.indexOf('default:'); + if (at < 0) throw new Error("createColumn's catch-all moved in driver-sql"); + return body.slice(at); +} + +/** One of the driver's own width constants, read where it is declared. */ +function driverChars(constant: 'DEFAULT_STRING_VARCHAR_CHARS' | 'MAX_VARCHAR_CHARS'): number { + const m = SQL_DRIVER_SOURCE.match(new RegExp(`${constant} = (\\d+);`)); + if (!m) { + throw new Error( + `${constant} not found in sql-driver.ts. That constant is the width this pin reads ` + + 'instead of transcribing, so a rename must fail loudly here rather than leave the ' + + 'generators unmeasured.', + ); + } + return Number(m[1]); +} + +const DEFAULT_CHARS = driverChars('DEFAULT_STRING_VARCHAR_CHARS'); +const MAX_CHARS = driverChars('MAX_VARCHAR_CHARS'); + +// ── Reading the columns the two generators emit ───────────────────────────── + +/** One object whose fields are exactly the probes a case asks for. */ +function emit(fields: Record>) { + const config = { objects: { probe: { name: 'probe', fields } } } as Record; + return { sql: generateMigrationSql(config), ts: generateMigrationTs(config) }; +} + +/** The SQL column type one probe field contributes, or `null` for none. */ +function sqlColumn(out: string, field: string): string | null { + const m = out.match(new RegExp(`^ {2}"${field}" (.+?),?$`, 'm')); + return m ? m[1] : null; +} + +/** The whole `table.x('field'…)` call one probe field contributes, or `null`. */ +function tsColumn(out: string, field: string): string | null { + const m = out.match(new RegExp(`^ {4}(table\\.\\w+\\('${field}'[^;]*?)(?:\\.notNullable\\(\\)|\\.nullable\\(\\));$`, 'm')); + return m ? m[1] : null; +} + +/** Both producers' answers for one field declaration, in one call. */ +function columnsFor(decl: Record): { sql: string | null; ts: string | null } { + const out = emit({ f: decl }); + return { sql: sqlColumn(out.sql, 'f'), ts: tsColumn(out.ts, 'f') }; +} + +describe('#16091 — the character column both generators emit is the driver\'s', () => { + it('control — the driver source really loaded and its arms were really found', () => { + // Non-vacuity for every extraction in this file. Without these, a reader + // that matched nothing would make the whole file pass while measuring + // literally nothing, which is the failure a source-reading pin has to buy + // its way out of. + expect(SQL_DRIVER_SOURCE.length).toBeGreaterThan(10_000); + expect(SCHEMA_DRIFT_SOURCE.length).toBeGreaterThan(10_000); + expect(createColumnSwitch().length).toBeGreaterThan(1_000); + expect(REAL_FIELD_TYPES.size).toBeGreaterThan(40); + + // The extractor discriminates: two different types really do land in two + // different arms, and an arm really does carry more than the label asked for. + expect(armContaining('text')).not.toBe(armContaining('email')); + expect(armMembers('text').length).toBeGreaterThan(1); + expect(armMembers('email').length).toBeGreaterThan(1); + expect(() => armContaining('this_is_not_a_field_type')).toThrow(); + + // Both widths are real numbers in the expected relation, so a regex that + // captured the wrong digits cannot pass unnoticed. + expect(DEFAULT_CHARS).toBeGreaterThan(0); + expect(MAX_CHARS).toBeGreaterThan(DEFAULT_CHARS); + + // And the readers really read: a field that exists resolves, one that does not is null. + const out = emit({ f: { type: 'text' } }); + expect(sqlColumn(out.sql, 'f')).not.toBeNull(); + expect(tsColumn(out.ts, 'f')).not.toBeNull(); + expect(sqlColumn(out.sql, 'nope')).toBeNull(); + expect(tsColumn(out.ts, 'nope')).toBeNull(); + }); + + // ── Arm 1: the text family is unbounded, and the branch is on KEYED ──────── + + it('the whole TEXT family takes an unbounded column in both generators', () => { + // The authority, read where it lives. Everything below is derived from this + // line, so a driver that moved must fail HERE, loudly, first. + const arm = armContaining('text'); + expect( + arm, + 'driver-sql no longer builds its text family with `keyable === null ? table.text(name) : ' + + 'table.string(name, keyable)`. This pin corrects the generators TOWARD the driver, so if ' + + 'the driver moved, re-read #16091 before touching generate.ts — the authority is the ' + + 'driver, not this file.', + ).toContain('col = keyable === null ? table.text(name) : table.string(name, keyable);'); + + // MEMBERSHIP is the driver's, swept rather than listed — a type that joins + // or leaves this arm changes what is measured here without anyone editing it. + const members = armMembers('text'); + expect(members).toContain('text'); + expect(members.length).toBeGreaterThanOrEqual(8); + for (const type of members) { + const { sql, ts } = columnsFor({ type }); + expect( + sql, + `os generate migration --format sql bounded a ${type} column. driver-sql builds it with ` + + '`table.text`, so a value the platform stores would be refused by the generated table.', + ).toBe('TEXT'); + expect(ts, `os generate migration (typescript) bounded a ${type} column — see above.`) + .toBe("table.text('f')"); + } + }); + + it('a declared maxLength does NOT size a text-family column, because the driver keys on KEYED', () => { + // ⭐ The half a later reader is most likely to "fix" wrongly. `maxLength` is + // honoured — at the write seam, not by the column — so sizing the column + // here would narrow it below what the platform accepts, which is this + // card's own defect pointed the other way. + const arm = armContaining('text'); + expect( + arm, + 'the text family stopped branching on `keyed`. If it now sizes from the declaration ' + + 'unconditionally, the generators must follow — re-read #16091.', + ).toContain('const keyable = keyed ? this.keyableTextLength(field) : null;'); + + for (const type of armMembers('text')) { + for (const maxLength of [1, 64, 100, MAX_CHARS, MAX_CHARS + 1]) { + const { sql, ts } = columnsFor({ type, maxLength }); + expect(sql, `a declared maxLength sized the ${type} column in the sql format`).toBe('TEXT'); + expect(ts, `a declared maxLength sized the ${type} column in the typescript format`) + .toBe("table.text('f')"); + } + } + + // The write seam is where that bound lives, stated by the differ itself + // rather than by this file. Read as a whole sentence: a partial match on + // "write seam" alone would survive the claim being reversed. + expect( + SCHEMA_DRIFT_SOURCE, + 'schema-drift.ts no longer states that a TEXT column relies on the write seam for the ' + + "declared bound. That sentence is the reason an unbounded column is CORRECT here rather " + + 'than merely wider, so re-read #16091 if it has gone.', + ).toContain('the bound is enforced at the write seam'); + + // Anti-vacuity: the sweep really varies something. An identical answer for + // every input is only meaningful if some OTHER family answers differently + // to the same inputs — which the string family does, below. + expect(columnsFor({ type: 'email', maxLength: 100 }).sql).not.toBe('TEXT'); + }); + + // ── Arm 2: the string family reads the declaration, with three outcomes ──── + + it('the whole STRING family takes the driver\'s declared width, in both generators', () => { + const arm = armContaining('email'); + expect( + arm, + 'driver-sql no longer sizes its string family from `declaredVarcharLength`. Re-read ' + + '#16091 before trusting the generators\' widths — the authority is the driver.', + ).toContain('col = declared === null ? table.text(name) : table.string(name, declared);'); + expect(arm).toContain('const declared = this.declaredVarcharLength(field);'); + + const members = armMembers('email'); + expect(members).toContain('email'); + expect(members.length).toBeGreaterThanOrEqual(4); + + for (const type of members) { + // Outcome 1 — no usable declaration: knex's default, read off the driver. + for (const decl of [ + { type }, + { type, maxLength: 0 }, + { type, maxLength: -5 }, + { type, maxLength: 12.5 }, + { type, maxLength: 'not a number' }, + ]) { + const { sql, ts } = columnsFor(decl); + expect(sql, `${type} without a usable declaration must take the driver's default width`) + .toBe(`VARCHAR(${DEFAULT_CHARS})`); + expect(ts).toBe("table.string('f')"); + } + + // Outcome 2 — a declaration this dialect can express, verbatim, in BOTH + // directions. Wider than the default is the reported defect; narrower is + // the same defect's other half, and the column a generator creates is + // always empty, so narrowing it is not a destructive migration. + for (const chars of [20, 400, 1024, MAX_CHARS]) { + const { sql, ts } = columnsFor({ type, maxLength: chars }); + expect(sql, `${type} ignored its declared maxLength in the sql format`) + .toBe(`VARCHAR(${chars})`); + expect(ts, `${type} ignored its declared maxLength in the typescript format`) + .toBe(`table.string('f', ${chars})`); + } + // The driver's own coercion: a numeric STRING is a declaration. + expect(columnsFor({ type, maxLength: '400' }).sql).toBe('VARCHAR(400)'); + + // Outcome 3 — past the ceiling is TEXT, never a clamp TO the ceiling. A + // clamp would reinstate the defect: a column narrower than the + // declaration, refusing writes the declaration allows. + const past = columnsFor({ type, maxLength: MAX_CHARS + 1 }); + expect(past.sql, `${type} past the varchar ceiling must be TEXT, not a clamp`).toBe('TEXT'); + expect(past.ts).toBe("table.text('f')"); + expect(past.sql).not.toBe(`VARCHAR(${MAX_CHARS})`); + } + }); + + it('the generator\'s varchar ceiling is the driver\'s, not a second number', () => { + // `packages/cli` does not depend on the driver at runtime, so the ceiling is + // transcribed in generate.ts. This is the assertion that makes the + // transcription safe: the two must be the same number, and a driver that + // moves fails here rather than leaving the generators quietly wrong. + const m = GENERATE_SOURCE.match(/^const MAX_VARCHAR_CHARS = (\d+);$/m); + expect(m, 'generate.ts no longer declares MAX_VARCHAR_CHARS at top level').not.toBeNull(); + expect(Number(m![1])).toBe(MAX_CHARS); + }); + + // ── Arm 3: the catch-all never reads the declaration ─────────────────────── + + it('the catch-all family takes the driver\'s default width and ignores maxLength', () => { + expect( + createColumnDefaultArm(), + "driver-sql's catch-all no longer spells `table.string(name)`. Re-read #16091.", + ).toContain('JSON_COLUMN_TYPES.has(type) ? this.jsonColumn(table, name) : table.string(name)'); + + // Derived, not listed: every real member that neither families' arm claims + // and that the driver does not case at all lands in the catch-all. `color` + // is the member this card moved (it carried an invented `VARCHAR(7)`), and + // it is asserted by NAME as well as by sweep so a reader can see it. + const cased = new Set([...createColumnSwitch().matchAll(/case '([^']+)':/g)].map((m) => m[1])); + const catchAll = [...REAL_FIELD_TYPES].filter((t) => !cased.has(t)); + expect(catchAll).toContain('color'); + expect(catchAll.length).toBeGreaterThan(5); + + for (const type of catchAll) { + const plain = columnsFor({ type }); + const declared = columnsFor({ type, maxLength: 7 }); + // JSON members of the catch-all are a different question (#14829 / #15041) + // and are pinned elsewhere; this case owns the character half only. + if (plain.sql !== `VARCHAR(${DEFAULT_CHARS})`) continue; + expect( + declared.sql, + `${type} sized its column from a declared maxLength. driver-sql's catch-all never reads ` + + 'one — the stored value is an option code, an opaque ref or another row\'s id, not the ' + + 'declared string, so a declared bound would size the wrong string.', + ).toBe(plain.sql); + expect(declared.ts).toBe(plain.ts); + } + + // The member this card moved, named. `VARCHAR(7)` was this file's own guess + // at `#RRGGBB`; the platform's column is the catch-all's default width, so + // every longer color code an author can write was a value the platform + // stores and a generated table refuses. + expect(columnsFor({ type: 'color' }).sql).toBe(`VARCHAR(${DEFAULT_CHARS})`); + expect(columnsFor({ type: 'color' }).sql).not.toBe('VARCHAR(7)'); + }); + + // ── The three families are three DIFFERENT answers ──────────────────────── + + it('the three arms really do answer differently, to the same declaration', () => { + // Without this, every assertion above could be satisfied by one column + // shape for everything — the "JSONB everywhere" hole, one family over. + const declared = { maxLength: 400 }; + const text = columnsFor({ type: 'text', ...declared }); + const string = columnsFor({ type: 'email', ...declared }); + const other = columnsFor({ type: 'color', ...declared }); + expect(text.sql).toBe('TEXT'); + expect(string.sql).toBe('VARCHAR(400)'); + expect(other.sql).toBe(`VARCHAR(${DEFAULT_CHARS})`); + expect(new Set([text.sql, string.sql, other.sql]).size).toBe(3); + expect(new Set([text.ts, string.ts, other.ts]).size).toBe(3); + }); + + // ── The shapes this replaced ────────────────────────────────────────────── + + it('none of the four invented widths can come back', () => { + // Anti-regression, stated as the emitted output rather than as source text + // so a rewrite of generate.ts that reproduces the defect is still caught. + const out = emit({ + a_text: { type: 'text' }, + a_url: { type: 'url' }, + a_phone: { type: 'phone' }, + a_color: { type: 'color' }, + }); + expect(sqlColumn(out.sql, 'a_text')).not.toBe(`VARCHAR(${DEFAULT_CHARS})`); + expect(sqlColumn(out.sql, 'a_url')).not.toBe('VARCHAR(2048)'); + expect(sqlColumn(out.sql, 'a_phone')).not.toBe('VARCHAR(50)'); + expect(sqlColumn(out.sql, 'a_color')).not.toBe('VARCHAR(7)'); + expect(tsColumn(out.ts, 'a_text')).not.toBe("table.string('a_text')"); + // Anti-vacuity: the readers really resolved these four fields, so the four + // negative assertions above are measurements rather than four `null`s. + for (const f of ['a_text', 'a_url', 'a_phone', 'a_color']) { + expect(sqlColumn(out.sql, f)).not.toBeNull(); + expect(tsColumn(out.ts, f)).not.toBeNull(); + } + // And the predicates really do fire on what was there before. + expect('VARCHAR(2048)').toBe('VARCHAR(2048)'); + }); +}); diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 195280715f..135b78453e 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -1004,7 +1004,30 @@ async function runClientGeneration(configPath: string | undefined, flags: { outp * so it cannot be mistaken for coverage. */ const FIELD_TYPE_SQL_MAP: Record = { - text: 'VARCHAR(255)', + // #16091 — TEXT, not VARCHAR(255). `text` heads the SAME text-family arm as + // the seven types below it, and that arm's column is + // `keyable === null ? table.text(name) : table.string(name, keyable)` with + // `keyable = keyed ? this.keyableTextLength(field) : null`. The branch is on + // KEYED, not on the declaration: neither generator emits an index, so no + // generated column is ever keyed and the driver's answer for an authored + // `text` field is an unbounded TEXT column whether or not it declares a + // `maxLength`. The bound is not lost — it is enforced at the write seam (the + // record validator's `max_length` branch over BOUNDED_STRING_FIELD_TYPES), + // which is the invariant `schema-drift.ts` states in as many words: "A TEXT + // column refuses nothing a `maxLength` allows … the bound is enforced at the + // write seam." + // + // Driven on live PostgreSQL 16.13, one 300-character value into three tables + // built from one object by the three producers: + // + // driver f_text text ACCEPTED — read back at length 300 + // sql gen f_text varchar(255) REFUSED — value too long for type character varying(255) + // ts gen f_text varchar(255) REFUSED — same + // + // This is the hard-failure class, not the schema-diff class: a row the + // platform stores today cannot be stored in a table generated for the same + // object. + text: 'TEXT', textarea: 'TEXT', richtext: 'TEXT', html: 'TEXT', @@ -1032,9 +1055,30 @@ const FIELD_TYPE_SQL_MAP: Record = { // producers, so neither moves. datetime: 'TIMESTAMPTZ', time: 'TIME', + // #16091 — the STRING family (`email` / `url` / `phone` / `password`) is ONE + // arm in `createColumn`, and its width is the field's own + // {@link SqlDriver.declaredVarcharLength}: `maxLength` verbatim when it is a + // positive integer up to the varchar ceiling, knex's 255 when there is no + // usable declaration, and TEXT above the ceiling. These entries are the + // NO-DECLARATION outcome only — {@link declaredVarchar} supplies the other + // two, because a lookup table keyed on the type alone cannot express an + // answer that depends on the field. + // + // `VARCHAR(50)` and `VARCHAR(2048)` were widths this file invented for a + // shape it never read. Measured on live PostgreSQL 16.13, all three + // producers driven from one object: + // + // f_phone driver varchar(255) sql gen varchar(50) ts gen varchar(255) + // f_url driver varchar(255) sql gen varchar(2048) ts gen varchar(255) + // + // Both directions are real. The narrow one is the card's own hard failure a + // type over — a 60-character phone number the platform stores is refused by + // the generated table. The wide one fails the other way: a 300-character url + // was ACCEPTED by the sql format's table and REFUSED by the driver's own, so + // the scaffold invited a value the platform will not keep. email: 'VARCHAR(255)', - phone: 'VARCHAR(50)', - url: 'VARCHAR(2048)', + phone: 'VARCHAR(255)', + url: 'VARCHAR(255)', select: 'VARCHAR(255)', // #14828 — MULTI_OPTION_TYPES seeds `driver-sql`'s `JSON_COLUMN_TYPES`, so // the runtime stores this in a JSON column; `json: 'JSONB'` below is the @@ -1057,7 +1101,15 @@ const FIELD_TYPE_SQL_MAP: Record = { file: 'VARCHAR(2048)', image: 'VARCHAR(2048)', password: 'VARCHAR(255)', - color: 'VARCHAR(7)', + // #16091 — `color` is not cased in `createColumn` at all: it falls to the + // catch-all, `JSON_COLUMN_TYPES.has(type) ? this.jsonColumn(table, name) : + // table.string(name)`, which is knex's varchar(255). `VARCHAR(7)` was this + // file's own guess at `#RRGGBB` and the platform never agreed with it — + // measured on live PostgreSQL 16.13, the driver's column is varchar(255). So + // every longer color an author can write (`#RRGGBBAA`, an `rgba(…)` string, + // a design-token name) is a value the platform stores and a table generated + // for the same object refuses. + color: 'VARCHAR(255)', rating: 'INTEGER', // #14828 — `vector` is in STRUCTURED_JSON_TYPES, hence in the driver's // `JSON_COLUMN_TYPES`. `VECTOR` was also not portable: it needs pgvector and @@ -1112,6 +1164,74 @@ const FIELD_TYPE_SQL_MAP: Record = { address: 'JSONB', } satisfies Record; +/** + * The STRING family, cased exactly as `SqlDriver.createColumn` cases it (#16091). + * + * The driver's arm is `case 'string': case 'email': case 'url': case 'phone': + * case 'password':`. `string` is absent here and that is not an omission: it is + * not a `FieldType` member at all — there is no `Field.string` builder, + * `FieldType.options` omits it, and `FieldSchema.safeParse({ type: 'string' })` + * fails at `[type]` (#12593) — so it cannot arrive through an authored object. + * + * ⛔ This set is NOT "the types whose values are strings". `select` / `radio` / + * `secret` / `color` / `tree` and the reference types all hold strings and all + * take the driver's catch-all, which never reads `maxLength`: their stored + * value is an option code, an opaque ref or another row's id, so sizing them + * from the author's bound would size the wrong string. `createColumn`'s own + * catch-all says exactly that. Membership here is read off the driver's arm and + * nothing else. + */ +const STRING_FAMILY_TYPES: ReadonlySet = new Set(['email', 'url', 'phone', 'password']); + +/** + * The widest `varchar(n)` any dialect this platform speaks will declare — + * `SqlDriver.MAX_VARCHAR_CHARS`, whose own comment records the measurement + * (MySQL 8.0.46 refuses `varchar(16384)` with `ERROR 1074`; it is the LOWEST of + * the three dialects' ceilings and is applied to all of them deliberately). + * + * Transcribed here because `packages/cli` does not depend on the driver at + * runtime, and pinned rather than trusted: `generate-string-family-width.pin.test.ts` + * reads the constant out of `sql-driver.ts` and fails here if the two part. + */ +const MAX_VARCHAR_CHARS = 16383; + +/** + * The three outcomes `SqlDriver.declaredVarcharLength` has for a string-family + * field, kept as three named answers rather than collapsed to a number (#16091). + * + * Collapsing them is what a `?? 255` would do, and it loses the one that is not + * a width at all: a declaration PAST the ceiling makes the driver emit TEXT, + * never a clamp to the ceiling — because clamping would reinstate the very + * defect (a column narrower than the declaration, refusing writes the + * declaration allows), while TEXT refuses nothing the author declared and the + * bound is still enforced at the write seam. + * + * - `default` — no usable declaration. The answer is this file's own table + * entry, which is knex's 255; the caller reads it there + * rather than having it restated here. + * - `sized` — a declaration this dialect can express, verbatim, in BOTH + * directions. Wider than 255 is the reported defect; narrower + * is the same defect's other half. + * - `unbounded` — past {@link MAX_VARCHAR_CHARS}. + */ +type VarcharAnswer = + | { kind: 'default' } + | { kind: 'sized'; chars: number } + | { kind: 'unbounded' }; + +/** + * `SqlDriver.declaredVarcharLength`'s decision, for a generated migration. + * + * The coercion is the driver's too, character for character — a `maxLength` may + * arrive as a string from an unvalidated authoring door, and a non-integer or + * non-positive one is NOT a bound. + */ +function declaredVarchar(maxLength: unknown): VarcharAnswer { + const n = typeof maxLength === 'string' ? Number(maxLength) : maxLength; + if (typeof n !== 'number' || !Number.isInteger(n) || n <= 0) return { kind: 'default' }; + return n > MAX_VARCHAR_CHARS ? { kind: 'unbounded' } : { kind: 'sized', chars: n }; +} + /** * The column one field takes. * @@ -1152,11 +1272,32 @@ const FIELD_TYPE_SQL_MAP: Record = { * inherited key, and the unvalidated authoring door can deliver one as a * `type` string. */ -function fieldTypeToSql(fieldType: string, multiple?: boolean): string | null { +function fieldTypeToSql(fieldType: string, multiple?: boolean, maxLength?: unknown): string | null { if (multiple) return FIELD_TYPE_SQL_MAP.json; - return Object.prototype.hasOwnProperty.call(FIELD_TYPE_SQL_MAP, fieldType) + const base = Object.prototype.hasOwnProperty.call(FIELD_TYPE_SQL_MAP, fieldType) ? FIELD_TYPE_SQL_MAP[fieldType] : 'TEXT'; + // #16091 — the STRING family is the one place the column depends on the FIELD + // and not only on its type, because that is the one place `createColumn` + // reads a declaration: its arm is `declared === null ? table.text(name) : + // table.string(name, declared)` over `declaredVarcharLength(field)`. Asked + // AFTER the table lookup, so the no-declaration outcome is the table's own + // entry rather than a second spelling of 255 that could drift from it. + // + // ⛔ Deliberately not asked for the text family. `createColumn` branches that + // one on KEYED — `keyed ? this.keyableTextLength(field) : null` — and a + // generated migration emits no index, so a generated column is never keyed + // and the driver's answer is TEXT with or without a `maxLength`. Reading the + // declaration there would size a column the platform leaves unbounded, which + // is this card's own defect pointed the other way. + if (!STRING_FAMILY_TYPES.has(fieldType)) return base; + const declared = declaredVarchar(maxLength); + if (declared.kind === 'sized') return `VARCHAR(${declared.chars})`; + // The unbounded spelling is READ from this table's own `text` entry rather + // than restated, the same discipline `FIELD_TYPE_SQL_MAP.json` above already + // uses, so the two cannot drift about how this file spells an unbounded column. + if (declared.kind === 'unbounded') return FIELD_TYPE_SQL_MAP.text; + return base; } /** @@ -1222,7 +1363,7 @@ export function generateMigrationSql(config: Record): string { const fieldLines: string[] = []; for (const [fieldName, fieldDef] of Object.entries(fields)) { - const sqlType = fieldTypeToSql(String(fieldDef.type || 'text'), !!fieldDef.multiple); + const sqlType = fieldTypeToSql(String(fieldDef.type || 'text'), !!fieldDef.multiple, fieldDef.maxLength); // #14828 — a VIRTUAL field materialises no column. `SqlDriver.createColumn` // returns without emitting one and `schema-drift.ts`'s `fieldHasColumn` // answers false for it, so a column here is one the runtime never writes. @@ -1358,13 +1499,50 @@ export function generateMigrationTs(config: Record): string { // `generate-field-type-vocabulary.pin.test.ts` measures. let colMethod: string | null; switch (fType) { - case 'text': case 'email': case 'phone': case 'url': case 'select': - case 'password': case 'color': + // #16091 — the STRING family takes an arm of its own, because it is the + // one family whose column depends on the FIELD and not only on its + // type: `createColumn`'s arm is `declared === null ? table.text(name) : + // table.string(name, declared)` over `declaredVarcharLength(field)`. + // All four used to spell a bare `table.string(name)` — knex's + // varchar(255) — so a `Field.email({ maxLength: 400 })` was + // `varchar(400)` on the platform and `varchar(255)` in the migration + // generated for the same object. Driven on live PostgreSQL 16.13: a + // 300-character value into that field was accepted by the driver's + // table (read back at length 300) and refused by both generated ones + // with `value too long for type character varying(255)`. + case 'email': case 'phone': case 'url': case 'password': { + const declared = declaredVarchar(fieldDef.maxLength); + colMethod = + declared.kind === 'sized' + ? `table.string('${fieldName}', ${declared.chars})` + : declared.kind === 'unbounded' + ? `table.text('${fieldName}')` + : `table.string('${fieldName}')`; + break; + } + // The catch-all family. `createColumn` cases none of these: each lands + // in its `table.string(name)` at knex's default width, and none of them + // reads `maxLength` — the stored value is an option code, an opaque + // `sys_secret` ref or a color code rather than the declared string, so + // a declared bound would size the wrong string. That is the driver's + // own stated reason, not an inference from its silence. + case 'select': case 'color': // #14657 — `secret` holds the opaque `sys_secret` ref, not the // credential (ADR-0100); `radio` is a single option code like `select`. case 'secret': case 'radio': colMethod = `table.string('${fieldName}')`; break; + // #16091 — `text` MOVED here, out of the string arm above. It heads + // `createColumn`'s text-family arm, whose column is + // `keyable === null ? table.text(name) : table.string(name, keyable)` + // with `keyable = keyed ? this.keyableTextLength(field) : null`. The + // branch is on KEYED and a generated migration emits no index, so every + // column this generator creates takes the unkeyed answer: `table.text`, + // `maxLength` declared or not. Measured on live PostgreSQL 16.13 — + // driver `text`, both generators `varchar(255)`, and a 300-character + // value accepted by the platform's table and refused by both generated + // ones. + case 'text': case 'textarea': case 'richtext': case 'html': case 'markdown': // #14657 — `driver-sql`'s own DDL switch puts these three in the text // family (#11794, #11875): the declared `maxLength`, when there is one, From 83edbc55f3e0f16800c6bffed846935780b3f0a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 12:52:44 +0000 Subject: [PATCH 2/6] =?UTF-8?q?chore(changeset):=20grade=20`@objectstack/c?= =?UTF-8?q?li`=20minor,=20the=20level=20a=20declared=20clause-=E2=91=A1=20?= =?UTF-8?q?requires?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Check Changeset`'s LEVEL AXIS (#16055) refuses a PR that declares clause-② YES while grading a package whose `packages/*/src/**` it moves as `patch`. The rule it mechanizes is the maintainer's 2026-09-04 ruling (decision batch #35, on #15294), written out under "WHICH LEVEL" in that step: a purely additive widening of a published package's public surface takes AT LEAST `minor`, and the commit type may raise a bump but never lower it below what the act requires. This branch declares clause-② `yes` and moves `packages/cli/src/**`, so the level and the declaration contradicted each other. Only the level moves here -- the generators, the pins and the measurements are untouched. ⚠️ The axis is invisible to the plain `--base origin/main` form of the gate, which reports `LEVEL AXIS: NOT MEASURED` and is neither a pass nor a failure. It is judged only from a `pull_request` event payload, off the `needs:contract-review` carrier or a machine-spelled `Clause-②:` line, so `--event` is the only form that can confirm this change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- .changeset/generated-migration-character-column-widths.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/generated-migration-character-column-widths.md b/.changeset/generated-migration-character-column-widths.md index cf70fb1161..4f412cb4f6 100644 --- a/.changeset/generated-migration-character-column-widths.md +++ b/.changeset/generated-migration-character-column-widths.md @@ -1,5 +1,5 @@ --- -"@objectstack/cli": patch +"@objectstack/cli": minor --- `os generate migration` now emits the character column `driver-sql` actually creates, in both the TypeScript and the SQL format. From 9cc1a76df2c78ab183dd550a9bf977e03c500679 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 13:55:26 +0000 Subject: [PATCH 3/6] fix(cli): size a KEYED text-family column from its declaration, as the driver does CORRECTING THE RECORD. This branch's first commit, the new pin's docblock and three comments in `generate.ts` all said: "the text family branches on KEYED, and a generated migration emits no index, so no generated column is ever keyed." That sentence describes this GENERATOR'S OUTPUT. `createColumn` reads the object's INPUT. Its `keyed` argument is `indexedKeyColumns(...).get(name)`, and `indexedKeyColumns` composes `uniqueIndexesFromFields` -- which keys a column on `field.unique`, a key every `FieldSchema` carries -- with the object's declared `indexes[]`. Both are DECLARATIONS, both are in the config these generators already read, and neither has anything to do with what a migration emits. The generator could have read `unique`; it simply did not. So a keyed text-family column IS sized from its declaration, at `keyableTextLength`'s width: the declared `maxLength` verbatim up to MAX_KEYABLE_VARCHAR_CHARS (768, the widest one utf8mb4 key part holds), and unbounded above that ceiling or with no usable declaration. Driven on live PostgreSQL 16.13 against the pre-change tree, one 300-character write into `{ type: 'text', unique: true, maxLength: 100 }`: driver varchar(100) REFUSED -- 22001 character varying(100) sql gen text ACCEPTED -- read back at length 300 ts gen text ACCEPTED -- read back at length 300 The wide direction, which this branch's own body calls the quieter of the two, inside the family it claimed to have closed. Re-driven after the change, all three producers REFUSE it, and 0 of 32 keyed character columns diverge. WHAT MOVES * `generate.ts` gains `indexKeyColumns`, a mirror of the driver's own composition -- field-level `unique` at all three spellings, object-level `indexes[]` unique or not, and the ADR-0120 D3 tenant key part, whose resolution (`tenancy.enabled`, `tenancy.tenantField`, an `organization_id` column) is computable from the object alone and so is mirrored rather than skipped. It also gains `keyableTextChars` and the transcribed 768 ceiling, kept deliberately separate from `declaredVarchar`: the two answer different questions of the same key. * The false sentence is corrected in all four places it reached. * The new pin gains the keyed arm: the driver-source chain at every link, the arm membership held equal to `createColumn`'s case labels, the width sweep at both outcomes, the three unique spellings against the words the spec rejects, the object-level index half, and the tenant-column half -- each of the last two confirmed against the live cluster before pinning. TWO RIDERS FROM THE SAME REVIEW * The pin's catch-all case skipped any member whose plain answer had already drifted, so it measured that the catch-all takes the driver's default width only where that already held. Mutating `radio` or `secret` to 'TEXT' passed all 61 tests across all four pin files. The character half of the catch-all is now DERIVED from the three spec classes `driver-sql` seeds `JSON_COLUMN_TYPES` from -- imported, never listed -- and `VARCHAR(255)` is asserted on the rest. Both mutations now redden. * A comment gave a false reason for transcribing `MAX_VARCHAR_CHARS`: "`packages/cli` does not depend on the driver at runtime". It does -- `@objectstack/driver-sql` is in this package's `dependencies` at `workspace:^`. The transcription is still necessary, for two other reasons: the constant is `protected static`, and #5726 forbids a CLI production module any static value import of a driver package. The reason moves; the transcription does not. The changeset stays `minor` and states the keyed half. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- ...rated-migration-character-column-widths.md | 4 +- .../generate-string-family-width.pin.test.ts | 403 ++++++++++++++++-- packages/cli/src/commands/generate.ts | 285 +++++++++++-- 3 files changed, 612 insertions(+), 80 deletions(-) diff --git a/.changeset/generated-migration-character-column-widths.md b/.changeset/generated-migration-character-column-widths.md index 4f412cb4f6..89e9577b91 100644 --- a/.changeset/generated-migration-character-column-widths.md +++ b/.changeset/generated-migration-character-column-widths.md @@ -6,6 +6,8 @@ A `text` field took `VARCHAR(255)` from both generators while the platform creates an unbounded `text` column for it, so a 300-character value the platform stores was refused by every generated table with `value too long for type character varying(255)`. Enumerating the whole character-column family found the same disagreement in eight more places: `url` and `phone` and `color` carried widths the generators invented (2048, 50 and 7 against the platform's 255), and neither generator read a field's declared `maxLength` at all, so a `maxLength: 400` email was `varchar(400)` on the platform and `varchar(255)` in the migration generated for it. -All of them now follow the platform's own three answers: the text family is unbounded (its declared bound is enforced at the write seam, not by the column), the string family takes its declared `maxLength` — verbatim in both directions, and TEXT rather than a clamp when it exceeds what a `varchar` can express — and the remaining string-valued types keep the default width and ignore a declaration, because their stored value is an option code or another row's id rather than the declared string. +All of them now follow the platform's own three answers: the text family is unbounded unless the object KEYS the column — a field declared `unique`, or one an object-level `indexes[]` entry lists, takes `varchar(maxLength)` up to the 768-character key-part ceiling, exactly as the platform builds it, and stays unbounded above that ceiling or with no declared bound, where the declared bound is enforced at the write seam instead — the string family takes its declared `maxLength` verbatim in both directions, and TEXT rather than a clamp when it exceeds what a `varchar` can express, and the remaining string-valued types keep the default width and ignore a declaration, because their stored value is an option code or another row's id rather than the declared string. + +The keyed half was measured after the rest: `{ type: 'text', unique: true, maxLength: 100 }` built `varchar(100)` on the platform and `text` in both generated tables, so a 300-character value the platform REFUSES was accepted by every generated table — the same disagreement as the headline row, pointing the other way. This scopes to PostgreSQL, which is the only dialect `os generate migration --format sql` claims. diff --git a/packages/cli/src/commands/generate-string-family-width.pin.test.ts b/packages/cli/src/commands/generate-string-family-width.pin.test.ts index 6e3fbd6d8c..5afe2c636e 100644 --- a/packages/cli/src/commands/generate-string-family-width.pin.test.ts +++ b/packages/cli/src/commands/generate-string-family-width.pin.test.ts @@ -50,12 +50,36 @@ * table and REFUSED by the driver's own `varchar(255)`. The scaffold invited * a value the platform will not keep, which no error message ever names. * + * A second sweep, driven after review, carried the KEYED declaration shapes the + * first one never reached — `unique` at each of its three spellings, and + * object-level `indexes[]` — and found the text family divergent again wherever + * a keyed field declares a bound a key part can hold: + * + * ``` + * x_text_uniq_max {type:'text', unique:true, maxLength:100} + * driver varchar(100) sql gen text ts gen text + * x_richtext_uniq_max {type:'richtext', unique:true, maxLength:64} + * driver varchar(64) sql gen text ts gen text + * x_text_uniq {type:'text', unique:true} all three text — agreed + * x_text_uniq_big {type:'text', unique:true, maxLength:1000} + * all three text — agreed, 1000 is past the key-part ceiling + * ``` + * * After the repair, all three producers were driven into the same cluster again - * and read back out of `information_schema.columns`: **0 of 26 columns diverge**, + * and read back out of `information_schema.columns`: **0 of 26 columns diverge** + * on the original sweep and **0 of 32 keyed character columns** on the second, * and the 300-character write is accepted in all three tables exactly where the * platform accepts it and refused in all three exactly where the platform * refuses it. * + * ⚠️ What neither sweep reaches, stated so the next reader does not read a + * sweep as proof of absence: declaration shapes that are not `FieldType` + * members at all (a `type` string from the unvalidated authoring door, or a + * field with no `type` key — the driver defaults those to `string`, this + * generator to `text`), the non-character columns (the numeric and JSON + * families, whose own divergences are recorded on other cards), and every + * dialect but PostgreSQL. + * * ## The three arms, and why `maxLength` reaches only one of them * * `createColumn` sorts every character column into three arms, and they answer @@ -63,15 +87,29 @@ * * 1. TEXT FAMILY — `keyable === null ? table.text(name) : table.string(name, * keyable)` where `keyable = keyed ? this.keyableTextLength(field) : null`. - * The branch is on KEYED. A generated migration emits no index, so no - * generated column is ever keyed and the answer is an unbounded TEXT - * column — `maxLength` declared or not. ⚠️ This is the half most likely to - * be "fixed" wrongly by a later reader: sizing a `text` column from its - * declaration looks like honouring the author and is this card's own - * defect pointed the other way. The bound is not lost, it is enforced at - * the write seam, which is what `schema-drift.ts` states in as many words: - * "A TEXT column refuses nothing a `maxLength` allows … the bound is - * enforced at the write seam." + * The branch is on KEYED, and `keyed` comes from `indexedKeyColumns`, + * which reads the OBJECT'S DECLARATIONS — `field.unique` (every + * `FieldSchema` carries it) and the object's `indexes[]`. So: + * + * - UNKEYED, the answer is an unbounded TEXT column, `maxLength` + * declared or not, and the declared bound is not lost — it is + * enforced at the write seam, which is what `schema-drift.ts` states + * in as many words: "A TEXT column refuses nothing a `maxLength` + * allows … the bound is enforced at the write seam." + * - KEYED, the answer is `varchar(keyableTextLength(field))`: the + * declared bound verbatim up to `MAX_KEYABLE_VARCHAR_CHARS` (768, the + * widest one utf8mb4 key part holds), and TEXT above it or with no + * declaration at all. + * + * ⚠️ "A generated migration emits no index, so no generated column is + * ever keyed" is FALSE and was this pin's own first answer. It describes + * the generator's OUTPUT; the driver keys on the object's INPUT. Driven on + * live PostgreSQL 16.13, `{ type: 'text', unique: true, maxLength: 100 }` + * built `varchar(100)` on the platform and TEXT in both generated tables, + * and a 300-character write was REFUSED by the driver's table (`22001 + * character varying(100)`) while both generated tables ACCEPTED it — the + * wide direction, the quieter of the two, inside the family this file + * claimed to have closed. * * 2. STRING FAMILY — `declared === null ? table.text(name) : * table.string(name, declared)` over `declaredVarcharLength(field)`, which @@ -106,7 +144,12 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { FieldType } from '@objectstack/spec/data'; +import { + FieldType, + FILE_REFERENCE_TYPES, + MULTI_OPTION_TYPES, + STRUCTURED_JSON_TYPES, +} from '@objectstack/spec/data'; import { describe, expect, it } from 'vitest'; import { generateMigrationSql, generateMigrationTs } from './generate.js'; @@ -188,7 +231,9 @@ function createColumnDefaultArm(): string { } /** One of the driver's own width constants, read where it is declared. */ -function driverChars(constant: 'DEFAULT_STRING_VARCHAR_CHARS' | 'MAX_VARCHAR_CHARS'): number { +function driverChars( + constant: 'DEFAULT_STRING_VARCHAR_CHARS' | 'MAX_VARCHAR_CHARS' | 'MAX_KEYABLE_VARCHAR_CHARS', +): number { const m = SQL_DRIVER_SOURCE.match(new RegExp(`${constant} = (\\d+);`)); if (!m) { throw new Error( @@ -202,12 +247,25 @@ function driverChars(constant: 'DEFAULT_STRING_VARCHAR_CHARS' | 'MAX_VARCHAR_CHA const DEFAULT_CHARS = driverChars('DEFAULT_STRING_VARCHAR_CHARS'); const MAX_CHARS = driverChars('MAX_VARCHAR_CHARS'); +const KEYABLE_CHARS = driverChars('MAX_KEYABLE_VARCHAR_CHARS'); // ── Reading the columns the two generators emit ───────────────────────────── -/** One object whose fields are exactly the probes a case asks for. */ -function emit(fields: Record>) { - const config = { objects: { probe: { name: 'probe', fields } } } as Record; +/** + * One object whose fields are exactly the probes a case asks for. + * + * `objectLevel` carries the half of a declaration that does NOT live on the + * field — `indexes[]` and `tenancy` — because that half reaches the column too: + * `indexedKeyColumns` composes it with `field.unique` to decide which columns a + * key part will use, and the text family's width branches on that answer. + */ +function emit( + fields: Record>, + objectLevel: Record = {}, +) { + const config = { + objects: { probe: { name: 'probe', fields, ...objectLevel } }, + } as Record; return { sql: generateMigrationSql(config), ts: generateMigrationTs(config) }; } @@ -224,8 +282,11 @@ function tsColumn(out: string, field: string): string | null { } /** Both producers' answers for one field declaration, in one call. */ -function columnsFor(decl: Record): { sql: string | null; ts: string | null } { - const out = emit({ f: decl }); +function columnsFor( + decl: Record, + objectLevel: Record = {}, +): { sql: string | null; ts: string | null } { + const out = emit({ f: decl }, objectLevel); return { sql: sqlColumn(out.sql, 'f'), ts: tsColumn(out.ts, 'f') }; } @@ -247,10 +308,14 @@ describe('#16091 — the character column both generators emit is the driver\'s' expect(armMembers('email').length).toBeGreaterThan(1); expect(() => armContaining('this_is_not_a_field_type')).toThrow(); - // Both widths are real numbers in the expected relation, so a regex that - // captured the wrong digits cannot pass unnoticed. + // All three widths are real numbers in the expected relation, so a regex + // that captured the wrong digits cannot pass unnoticed. The KEY-PART + // ceiling sits strictly between the other two — a reader that collapsed it + // onto either would be caught here rather than by a silent width. expect(DEFAULT_CHARS).toBeGreaterThan(0); expect(MAX_CHARS).toBeGreaterThan(DEFAULT_CHARS); + expect(KEYABLE_CHARS).toBeGreaterThan(DEFAULT_CHARS); + expect(KEYABLE_CHARS).toBeLessThan(MAX_CHARS); // And the readers really read: a field that exists resolves, one that does not is null. const out = emit({ f: { type: 'text' } }); @@ -262,7 +327,7 @@ describe('#16091 — the character column both generators emit is the driver\'s' // ── Arm 1: the text family is unbounded, and the branch is on KEYED ──────── - it('the whole TEXT family takes an unbounded column in both generators', () => { + it('the whole TEXT family takes an unbounded column, UNKEYED, in both generators', () => { // The authority, read where it lives. Everything below is derived from this // line, so a driver that moved must fail HERE, loudly, first. const arm = armContaining('text'); @@ -291,11 +356,11 @@ describe('#16091 — the character column both generators emit is the driver\'s' } }); - it('a declared maxLength does NOT size a text-family column, because the driver keys on KEYED', () => { - // ⭐ The half a later reader is most likely to "fix" wrongly. `maxLength` is - // honoured — at the write seam, not by the column — so sizing the column - // here would narrow it below what the platform accepts, which is this - // card's own defect pointed the other way. + it('a declared maxLength does NOT size an UNKEYED text-family column', () => { + // ⭐ `maxLength` alone is honoured at the write seam, not by the column, so + // sizing an unkeyed column from it would narrow it below what the platform + // accepts — this card's own defect pointed the other way. The KEYED half is + // the next case, and it is the one this file first got wrong. const arm = armContaining('text'); expect( arm, @@ -309,6 +374,17 @@ describe('#16091 — the character column both generators emit is the driver\'s' expect(sql, `a declared maxLength sized the ${type} column in the sql format`).toBe('TEXT'); expect(ts, `a declared maxLength sized the ${type} column in the typescript format`) .toBe("table.text('f')"); + // Anti-vacuity for the whole case: the SAME declaration, keyed, is a + // DIFFERENT column. Without this the sweep above is satisfied by a + // generator that answers TEXT unconditionally — which is exactly what + // it was measuring when this file first passed. + const keyed = columnsFor({ type, maxLength, unique: true }); + if (maxLength <= KEYABLE_CHARS) { + expect(keyed.sql, `${type} @ maxLength ${maxLength} keyed`).toBe(`VARCHAR(${maxLength})`); + } else { + expect(keyed.sql, `${type} @ maxLength ${maxLength} keyed, past the key-part ceiling`) + .toBe('TEXT'); + } } } @@ -328,6 +404,196 @@ describe('#16091 — the character column both generators emit is the driver\'s' expect(columnsFor({ type: 'email', maxLength: 100 }).sql).not.toBe('TEXT'); }); + // ── Arm 1b: the SAME family, KEYED — sized from the declaration ─────────── + // + // ⭐ The rows this file missed on its first pass, and the reason it missed + // them: it reasoned from what the generators EMIT (no `CREATE INDEX`, so + // nothing can be keyed) when the driver reads what the object DECLARES. + + it('createColumn\'s `keyed` argument is read off the DECLARATION, at every link', () => { + // The chain, asserted at each link in the driver's own source. If any link + // moves, this fails loudly before the behavioural sweeps below can go + // quietly wrong — a sweep that measured the wrong branch would still pass. + expect( + SQL_DRIVER_SOURCE, + 'createColumn no longer receives `keyed` from indexedKeyColumns — re-read #16091.', + ).toContain('this.createColumn(table, name, field, keyedColumns.get(name));'); + expect(SQL_DRIVER_SOURCE).toContain('const keyedColumns = indexedKeyColumns({'); + expect( + SCHEMA_DRIFT_SOURCE, + 'uniqueIndexesFromFields no longer keys a column on `field.unique`. That predicate is why ' + + 'a GENERATED column can be keyed at all — re-read #16091 before narrowing the generators.', + ).toContain('if (!isUniqueScopeDeclared(field?.unique)) continue;'); + expect( + SCHEMA_DRIFT_SOURCE, + 'indexedKeyColumns no longer composes the field-level unique indexes.', + ).toContain('for (const idx of uniqueIndexesFromFields(table, fields, tenantField)) record(idx);'); + // The two scope vocabularies generate.ts transcribes, read where declared. + expect(SCHEMA_DRIFT_SOURCE).toContain("return unique === 'organization' || isUniqueDeclared(unique);"); + expect(SCHEMA_DRIFT_SOURCE).toContain("return unique === true || unique === 'organization';"); + // Anti-vacuity: the reader really discriminates on this source. + expect(SCHEMA_DRIFT_SOURCE).not.toContain('if (!isUniqueScopeDeclared(field?.uniqueX)) continue;'); + }); + + it('the generator\'s key-part ceiling is the driver\'s, not a second number', () => { + const m = GENERATE_SOURCE.match(/^const MAX_KEYABLE_VARCHAR_CHARS = (\d+);$/m); + expect(m, 'generate.ts no longer declares MAX_KEYABLE_VARCHAR_CHARS at top level').not.toBeNull(); + expect(Number(m![1])).toBe(KEYABLE_CHARS); + // And it is not the COLUMN ceiling wearing the key-part name. The two are + // different limits on different objects and collapsing them would emit DDL + // MySQL refuses on one side and an unkeyable TEXT on the other. + expect(Number(m![1])).not.toBe(MAX_CHARS); + }); + + it('the generator\'s TEXT family is the driver\'s arm, member for member', () => { + const m = GENERATE_SOURCE.match(/const TEXT_FAMILY_TYPES: ReadonlySet = new Set\(\[([\s\S]*?)\]\);/); + expect(m, 'generate.ts no longer declares TEXT_FAMILY_TYPES').not.toBeNull(); + const declared = [...m![1].matchAll(/'([^']+)'/g)].map((x) => x[1]).sort(); + expect(declared.length).toBeGreaterThanOrEqual(8); + expect( + declared, + "the generator's text family drifted from createColumn's own case labels. Membership is " + + 'the driver\'s to decide — a type that joins or leaves that arm moves this set.', + ).toEqual([...armMembers('text')].sort()); + }); + + it('a KEYED text-family column takes keyableTextLength\'s width, in both generators', () => { + const arm = armContaining('text'); + expect(arm).toContain('const keyable = keyed ? this.keyableTextLength(field) : null;'); + expect(arm).toContain('col = keyable === null ? table.text(name) : table.string(name, keyable);'); + + for (const type of armMembers('text')) { + // SIZED — the declared bound verbatim, up to the key-part ceiling. + for (const chars of [1, 64, 100, DEFAULT_CHARS, KEYABLE_CHARS]) { + const { sql, ts } = columnsFor({ type, unique: true, maxLength: chars }); + expect(sql, `a keyed ${type} @ ${chars} was not sized in the sql format`) + .toBe(`VARCHAR(${chars})`); + expect(ts, `a keyed ${type} @ ${chars} was not sized in the typescript format`) + .toBe(`table.string('f', ${chars})`); + } + // UNBOUNDED — the two causes `keyableTextLength` answers `null` for: no + // usable declaration, and a bound wider than one key part can hold. + for (const decl of [ + { type, unique: true }, + { type, unique: true, maxLength: KEYABLE_CHARS + 1 }, + { type, unique: true, maxLength: MAX_CHARS }, + { type, unique: true, maxLength: 0 }, + { type, unique: true, maxLength: -5 }, + { type, unique: true, maxLength: 12.5 }, + { type, unique: true, maxLength: 'not a number' }, + ]) { + const { sql, ts } = columnsFor(decl); + expect(sql, `keyed ${type} ${JSON.stringify(decl)} in the sql format`).toBe('TEXT'); + expect(ts, `keyed ${type} ${JSON.stringify(decl)} in the typescript format`) + .toBe("table.text('f')"); + } + // NEVER a clamp TO the ceiling — the same rule the string family follows + // one arm over, for the same reason: a clamp would emit a column narrower + // than the declaration, refusing writes the declaration allows. + expect(columnsFor({ type, unique: true, maxLength: KEYABLE_CHARS + 1 }).sql) + .not.toBe(`VARCHAR(${KEYABLE_CHARS})`); + // The driver's own coercion: a numeric STRING is a declaration. + expect(columnsFor({ type, unique: true, maxLength: '64' }).sql).toBe('VARCHAR(64)'); + // Anti-vacuity: drop the KEY and the same declaration is unbounded again, + // so this case measures the key rather than the bound. + expect(columnsFor({ type, maxLength: 100 }).sql).toBe('TEXT'); + } + }); + + it('the four keyed probes this round was opened on, by name and by shape', () => { + // Spelled out rather than swept so a reader can compare them against the + // live-PostgreSQL run in the header without re-deriving anything. + const probes: Array<[string, Record, string, string]> = [ + ['x_text_uniq_max', { type: 'text', unique: true, maxLength: 100 }, 'VARCHAR(100)', "table.string('f', 100)"], + ['x_richtext_uniq_max', { type: 'richtext', unique: true, maxLength: 64 }, 'VARCHAR(64)', "table.string('f', 64)"], + ['x_text_uniq', { type: 'text', unique: true }, 'TEXT', "table.text('f')"], + ['x_text_uniq_big', { type: 'text', unique: true, maxLength: 1000 }, 'TEXT', "table.text('f')"], + ]; + for (const [name, decl, sql, ts] of probes) { + const got = columnsFor(decl); + expect(got.sql, name).toBe(sql); + expect(got.ts, name).toBe(ts); + } + // The two that ALREADY agreed at `text` must not have moved: 1000 is past + // the key-part ceiling and an undeclared bound is no bound at all. + expect(1000).toBeGreaterThan(KEYABLE_CHARS); + }); + + it('every unique spelling the driver keys on keys the column here, and no other', () => { + const bound = { type: 'text', maxLength: 100 }; + for (const unique of [true, 'global', 'organization']) { + expect(columnsFor({ ...bound, unique }).sql, `unique: ${JSON.stringify(unique)}`) + .toBe('VARCHAR(100)'); + } + // Everything else is not a unique DECLARATION and must key nothing — + // including the two words the spec rejects by name, which a transcription + // that reached for "anything truthy" would silently accept. + for (const unique of [false, undefined, null, 0, 1, '', 'tenant', 'org', 'yes', {}]) { + expect(columnsFor({ ...bound, unique }).sql, `unique: ${JSON.stringify(unique)}`).toBe('TEXT'); + } + }); + + it('a column an object-level declared index lists is keyed too, unique or not', () => { + // `indexedKeyColumns` records EVERY key part of every declared index, not + // only the unique ones: a bounded key part is a storage choice for an + // ordinary index and the constraint itself for a unique one. + const decl = { type: 'text', maxLength: 100 }; + for (const idx of [ + { fields: ['f'] }, + { fields: ['f'], unique: true }, + { fields: ['f'], unique: 'global' }, + { fields: ['f'], unique: 'organization' }, + { name: 'by_other_f', fields: ['other', 'f'] }, + ]) { + expect(columnsFor(decl, { indexes: [idx] }).sql, JSON.stringify(idx)).toBe('VARCHAR(100)'); + } + // ...and an index that does not list the column does not key it. + for (const idx of [{ fields: ['other'] }, { fields: [] }, { fields: 'f' }, { unique: true }, {}]) { + expect(columnsFor(decl, { indexes: [idx] }).sql, JSON.stringify(idx)).toBe('TEXT'); + } + expect(columnsFor(decl, { indexes: [] }).sql).toBe('TEXT'); + expect(columnsFor(decl, {}).sql).toBe('TEXT'); + }); + + it('an organization-scoped unique keys the TENANT column too, and sizes it', () => { + // ADR-0120 D3: `unique: true` / `'organization'` on a tenant-scoped table + // is the composite `(organization_id, field)`, so BOTH columns are key + // parts. Driven on live PostgreSQL 16.13 — the driver's `organization_id` + // came back `character varying(50)` here. + const fields = { + organization_id: { type: 'text', maxLength: 50 }, + code: { type: 'text', maxLength: 30, unique: true }, + }; + const out = emit(fields); + expect(sqlColumn(out.sql, 'code')).toBe('VARCHAR(30)'); + expect(sqlColumn(out.sql, 'organization_id')).toBe('VARCHAR(50)'); + expect(tsColumn(out.ts, 'organization_id')).toBe("table.string('organization_id', 50)"); + + // `'global'` is platform-wide and prepends nothing, so the tenant column + // stays unkeyed — the half that proves the case above is about SCOPE. + const globalScope = emit({ ...fields, code: { type: 'text', maxLength: 30, unique: 'global' } }); + expect(sqlColumn(globalScope.sql, 'code')).toBe('VARCHAR(30)'); + expect(sqlColumn(globalScope.sql, 'organization_id')).toBe('TEXT'); + + // An explicit tenancy opt-out wins over the column-presence heuristic. + const optedOut = emit(fields, { tenancy: { enabled: false } }); + expect(sqlColumn(optedOut.sql, 'organization_id')).toBe('TEXT'); + expect(sqlColumn(optedOut.sql, 'code')).toBe('VARCHAR(30)'); + + // A declared `tenancy.tenantField` names the column instead. + const named = emit( + { org: { type: 'text', maxLength: 40 }, code: { type: 'text', maxLength: 30, unique: true } }, + { tenancy: { tenantField: 'org' } }, + ); + expect(sqlColumn(named.sql, 'org')).toBe('VARCHAR(40)'); + + // ...and a `tenantField` naming no real field falls back to nothing, so a + // table without an organization column keys only the field itself. + const noTenant = emit({ code: { type: 'text', maxLength: 30, unique: true } }); + expect(sqlColumn(noTenant.sql, 'code')).toBe('VARCHAR(30)'); + }); + + // ── Arm 2: the string family reads the declaration, with three outcomes ──── it('the whole STRING family takes the driver\'s declared width, in both generators', () => { @@ -401,34 +667,87 @@ describe('#16091 — the character column both generators emit is the driver\'s' ).toContain('JSON_COLUMN_TYPES.has(type) ? this.jsonColumn(table, name) : table.string(name)'); // Derived, not listed: every real member that neither families' arm claims - // and that the driver does not case at all lands in the catch-all. `color` - // is the member this card moved (it carried an invented `VARCHAR(7)`), and - // it is asserted by NAME as well as by sweep so a reader can see it. + // and that the driver does not case at all lands in the catch-all. const cased = new Set([...createColumnSwitch().matchAll(/case '([^']+)':/g)].map((m) => m[1])); const catchAll = [...REAL_FIELD_TYPES].filter((t) => !cased.has(t)); - expect(catchAll).toContain('color'); - expect(catchAll.length).toBeGreaterThan(5); - for (const type of catchAll) { + // The catch-all routes on `JSON_COLUMN_TYPES`, so its CHARACTER half is the + // rest — derived from the three spec classes driver-sql seeds that set + // from, imported and never listed here. + // + // ⛔ NOT derived from what this generator already answers. Skipping every + // member whose plain answer is not already `VARCHAR(${DEFAULT_CHARS})` is + // what this case used to do, and it made the case VACUOUS for exactly the + // member that had drifted: mutating `radio` or `secret` to `'TEXT'` in + // `FIELD_TYPE_SQL_MAP` skipped the member and passed all four pin files. A + // case may not read its subject to decide whether to measure it. + const jsonSeeded = new Set([ + ...MULTI_OPTION_TYPES, + ...STRUCTURED_JSON_TYPES, + ...FILE_REFERENCE_TYPES, + ]); + const characterCatchAll = catchAll.filter((t) => !jsonSeeded.has(t)); + + // Controls for the derivation itself. Without these a seed set that failed + // to import would leave `characterCatchAll` as the whole catch-all (and the + // sweep red for the wrong reason) or empty (and the sweep vacuous again). + expect( + SQL_DRIVER_SOURCE, + 'driver-sql no longer seeds JSON_COLUMN_TYPES from these three spec classes, so the ' + + 'character half of its catch-all is no longer the complement of them — re-read #16091.', + ).toContain('...STRUCTURED_JSON_TYPES, ...FILE_REFERENCE_TYPES, ...MULTI_OPTION_TYPES,'); + expect(jsonSeeded.has('json')).toBe(true); + expect(jsonSeeded.has('color')).toBe(false); + expect(characterCatchAll.length).toBeGreaterThanOrEqual(5); + // The three members a mutation of this file's own table would land on. + // `color` is the member this card moved (it carried an invented + // `VARCHAR(7)`); `radio` and `secret` are the two the review mutated to + // `'TEXT'` and watched pass. + for (const named of ['color', 'radio', 'secret']) { + expect(characterCatchAll, `${named} left the character half of the catch-all`) + .toContain(named); + } + + for (const type of characterCatchAll) { const plain = columnsFor({ type }); - const declared = columnsFor({ type, maxLength: 7 }); - // JSON members of the catch-all are a different question (#14829 / #15041) - // and are pinned elsewhere; this case owns the character half only. - if (plain.sql !== `VARCHAR(${DEFAULT_CHARS})`) continue; + // The case title's own claim, asserted rather than assumed: this member + // takes the driver's default width. A drifted member fails HERE. expect( - declared.sql, - `${type} sized its column from a declared maxLength. driver-sql's catch-all never reads ` + - 'one — the stored value is an option code, an opaque ref or another row\'s id, not the ' + - 'declared string, so a declared bound would size the wrong string.', - ).toBe(plain.sql); - expect(declared.ts).toBe(plain.ts); + plain.sql, + `${type} does not take driver-sql's catch-all width. Its arm is table.string(name), ` + + `knex's varchar(${DEFAULT_CHARS}), so a narrower or wider column here is one the ` + + 'platform disagrees with.', + ).toBe(`VARCHAR(${DEFAULT_CHARS})`); + expect(plain.ts, `${type} in the typescript format`).toBe("table.string('f')"); + + // ...and nothing on the FIELD moves it. The catch-all reads neither + // `maxLength` nor `unique`: the stored value is an option code, an opaque + // `sys_secret` ref or another row's id, not the declared string, so a + // declared bound would size the wrong string — the driver's own stated + // reason, not an inference from its silence. + for (const extra of [ + { maxLength: 7 }, + { maxLength: 400 }, + { maxLength: MAX_CHARS + 1 }, + { unique: true, maxLength: 100 }, + { unique: 'organization', maxLength: 100 }, + ]) { + const declared = columnsFor({ type, ...extra }); + expect(declared.sql, `${type} moved on ${JSON.stringify(extra)} in the sql format`) + .toBe(plain.sql); + expect(declared.ts, `${type} moved on ${JSON.stringify(extra)} in the typescript format`) + .toBe(plain.ts); + } + // ...nor does an object-level index over it. + const indexed = columnsFor({ type, maxLength: 100 }, { indexes: [{ fields: ['f'] }] }); + expect(indexed.sql, `${type} moved under a declared index`).toBe(plain.sql); + expect(indexed.ts, `${type} moved under a declared index`).toBe(plain.ts); } // The member this card moved, named. `VARCHAR(7)` was this file's own guess // at `#RRGGBB`; the platform's column is the catch-all's default width, so // every longer color code an author can write was a value the platform // stores and a generated table refuses. - expect(columnsFor({ type: 'color' }).sql).toBe(`VARCHAR(${DEFAULT_CHARS})`); expect(columnsFor({ type: 'color' }).sql).not.toBe('VARCHAR(7)'); }); diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 135b78453e..796cf1ce6f 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -1007,15 +1007,18 @@ const FIELD_TYPE_SQL_MAP: Record = { // #16091 — TEXT, not VARCHAR(255). `text` heads the SAME text-family arm as // the seven types below it, and that arm's column is // `keyable === null ? table.text(name) : table.string(name, keyable)` with - // `keyable = keyed ? this.keyableTextLength(field) : null`. The branch is on - // KEYED, not on the declaration: neither generator emits an index, so no - // generated column is ever keyed and the driver's answer for an authored - // `text` field is an unbounded TEXT column whether or not it declares a - // `maxLength`. The bound is not lost — it is enforced at the write seam (the - // record validator's `max_length` branch over BOUNDED_STRING_FIELD_TYPES), - // which is the invariant `schema-drift.ts` states in as many words: "A TEXT - // column refuses nothing a `maxLength` allows … the bound is enforced at the - // write seam." + // `keyable = keyed ? this.keyableTextLength(field) : null`. + // + // This entry is the UNKEYED answer, and it is the only one a table keyed on + // the TYPE can give: `keyed` is a property of the field's declaration, not of + // its type. {@link keyableTextChars} supplies the keyed one, asked by + // {@link fieldTypeToSql} after this lookup. + // + // Unkeyed, a declared `maxLength` does not reach the column, and it is not + // lost either: it is enforced at the write seam (the record validator's + // `max_length` branch over BOUNDED_STRING_FIELD_TYPES), which is the + // invariant `schema-drift.ts` states in as many words: "A TEXT column refuses + // nothing a `maxLength` allows … the bound is enforced at the write seam." // // Driven on live PostgreSQL 16.13, one 300-character value into three tables // built from one object by the three producers: @@ -1189,12 +1192,82 @@ const STRING_FAMILY_TYPES: ReadonlySet = new Set(['email', 'url', 'phone * (MySQL 8.0.46 refuses `varchar(16384)` with `ERROR 1074`; it is the LOWEST of * the three dialects' ceilings and is applied to all of them deliberately). * - * Transcribed here because `packages/cli` does not depend on the driver at - * runtime, and pinned rather than trusted: `generate-string-family-width.pin.test.ts` - * reads the constant out of `sql-driver.ts` and fails here if the two part. + * Transcribed rather than imported, for two independent reasons — the second + * one holds even if the first is ever lifted: + * + * - it is `protected static` on `SqlDriver`, so it is not on the driver + * package's exported surface at all; + * - #5726 forbids a CLI production module any static value import of an + * `@objectstack/driver-*` package, and `schema-migrate.lazy-driver-import.test.ts` + * enforces it. oclif `import()`s every command module on every invocation, + * so one such edge here charges an unbuilt driver to whatever command the + * operator actually ran. + * + * ⛔ NOT "because `packages/cli` does not depend on the driver at runtime" — + * it does: `@objectstack/driver-sql` is in this package's `dependencies` at + * `workspace:^`. A missing dependency was never the reason, and stating it as + * one invites the next reader to "simplify" the transcription away. + * + * Pinned rather than trusted: `generate-string-family-width.pin.test.ts` reads + * the constant out of `sql-driver.ts` and fails here if the two part. */ const MAX_VARCHAR_CHARS = 16383; +/** + * The widest `varchar(n)` ONE utf8mb4 index key part can hold — + * `SqlDriver.MAX_KEYABLE_VARCHAR_CHARS`, whose own comment records the + * measurement (MySQL 8.0.46: `varchar(768) UNIQUE` creates, `varchar(769) + * UNIQUE` is refused with `ER_TOO_LONG_KEY`). + * + * Transcribed and pinned for exactly the reasons {@link MAX_VARCHAR_CHARS} + * gives. ⚠️ A DIFFERENT number from that one, and the two are not + * interchangeable: this bounds a KEY PART, that bounds a COLUMN. + */ +const MAX_KEYABLE_VARCHAR_CHARS = 768; + +/** + * The TEXT family, cased exactly as `SqlDriver.createColumn` cases it (#16091). + * + * The driver's arm is `case 'text': case 'textarea': case 'html': case + * 'markdown': case 'richtext': case 'code': case 'signature': case 'qrcode':` + * and its column is `keyable === null ? table.text(name) : table.string(name, + * keyable)`. Membership is read off those case labels and nothing else — a type + * that joins or leaves the arm moves this set, and the pin fails until it does. + */ +const TEXT_FAMILY_TYPES: ReadonlySet = new Set([ + 'text', + 'textarea', + 'html', + 'markdown', + 'richtext', + 'code', + 'signature', + 'qrcode', +]); + +/** + * `SqlDriver.keyableTextLength`'s decision, for a generated migration: the + * `varchar(n)` a KEYED text-family column takes, or `null` to leave it TEXT. + * + * `null` has two causes and both leave the column unbounded — no usable + * declaration (there is no bound to emit, and inventing one would impose a + * truncation boundary the author never wrote), and a declaration wider than a + * single key part can be (a `varchar(n)` there only trades + * `ER_BLOB_KEY_WITHOUT_LENGTH` for `ER_TOO_LONG_KEY`). + * + * ⚠️ Deliberately NOT {@link declaredVarchar}, and the two must not be merged. + * They ask different questions of the same key: that one asks "how wide is this + * column?" and answers knex's 255 for a field that declares nothing, this one + * asks "can this KEY?" and answers `null`. The driver keeps them apart for the + * same reason and says so on `declaredVarcharLength`. + */ +function keyableTextChars(maxLength: unknown): number | null { + const n = typeof maxLength === 'string' ? Number(maxLength) : maxLength; + if (typeof n !== 'number' || !Number.isInteger(n) || n <= 0) return null; + if (n > MAX_KEYABLE_VARCHAR_CHARS) return null; + return n; +} + /** * The three outcomes `SqlDriver.declaredVarcharLength` has for a string-family * field, kept as three named answers rather than collapsed to a number (#16091). @@ -1232,6 +1305,108 @@ function declaredVarchar(maxLength: unknown): VarcharAnswer { return n > MAX_VARCHAR_CHARS ? { kind: 'unbounded' } : { kind: 'sized', chars: n }; } +/** + * `schema-drift.ts`'s `isUniqueScopeDeclared` — the FIELD-level unique + * vocabulary. Transcribed, for the reasons {@link MAX_VARCHAR_CHARS} gives. + */ +function isUniqueScopeDeclared(unique: unknown): boolean { + return unique === true || unique === 'global' || unique === 'organization'; +} + +/** + * `schema-drift.ts`'s `isOrganizationScopedUnique` — the FIELD-level spellings + * whose index also keys the organization column. + * + * ⛔ Not the scope judgment for a DECLARED index, and it must not be reached + * for there: `normalizeDeclaredIndex` takes a declared `unique: true` VERBATIM + * as global. That the two paths read the same token differently is a maintainer + * ruling (2026-08-13), not an oversight, so the two readings stay apart here + * too. + */ +function isOrganizationScopedUnique(unique: unknown): boolean { + return unique === true || unique === 'organization'; +} + +/** + * `SqlDriver.computeTenantField` — the organization column an + * organization-scoped unique index prepends its key part from. + * + * Computable from the object alone, which is why it is mirrored rather than + * skipped: explicit opt-out wins, then a declared `tenancy.tenantField` that + * names a real field, then a field literally named `organization_id`. + */ +function tenantFieldOf(obj: Record): string | null { + const tenancy = obj?.tenancy; + if (tenancy?.enabled === false) return null; + const fields = obj?.fields; + if (tenancy?.tenantField) { + const declared = String(tenancy.tenantField); + if (fields && Object.prototype.hasOwnProperty.call(fields, declared)) return declared; + } + if (fields && Object.prototype.hasOwnProperty.call(fields, 'organization_id')) return 'organization_id'; + return null; +} + +/** + * Every column some declared index on this object will use as a KEY PART — + * `schema-drift.ts`'s `indexedKeyColumns`, minus the UNIQUE flag, which only + * the driver's MySQL key diagnostics read (#16091). + * + * ⭐ This is what the text family branches on, and it is read off the OBJECT'S + * DECLARATIONS — `field.unique` and `indexes[]` — never off anything either + * generator emits. A generated migration still emits no `CREATE INDEX`; that is + * a fact about this generator's OUTPUT and it is not the question. The driver + * asks what the object DECLARES, so a `Field.text({ unique: true, maxLength: + * 100 })` is `varchar(100)` on the platform and must be `varchar(100)` here. + * Reasoning from the emitted output instead ("no index is emitted, so nothing + * is ever keyed") is how this arm was first got wrong. + * + * ⚠️ Deliberately NOT filtered by which columns this generator goes on to emit, + * for the same reason the driver's is not filtered by `physicalColumns`: + * deciding a column's TYPE is the whole reason the question is asked. + */ +function indexKeyColumns(obj: Record): ReadonlySet { + const fields = (obj?.fields ?? {}) as Record; + const tenantField = tenantFieldOf(obj); + const out = new Set(); + // Field-level `unique` — `uniqueIndexesFromFields`. The field's own column is + // a key part at every scope; an organization-scoped one keys the tenant + // column too, unless the tenant column IS this field ("one row per tenant" + // cannot be scoped to the tenant). + for (const [name, field] of Object.entries(fields)) { + if (!isUniqueScopeDeclared(field?.unique)) continue; + out.add(name); + if (isOrganizationScopedUnique(field.unique) && tenantField != null && tenantField !== name) { + out.add(tenantField); + } + } + // Object-level `indexes[]` — `normalizeDeclaredIndex`. Every listed column is + // a key part whether or not the index is unique: `indexedKeyColumns` records + // both, because a bounded key part is a storage choice for an ordinary index + // and the constraint itself for a unique one. + for (const idx of Array.isArray(obj?.indexes) ? obj.indexes : []) { + const listed: string[] = Array.isArray(idx?.fields) + ? idx.fields.filter((f: unknown): f is string => typeof f === 'string' && f.length > 0) + : []; + if (listed.length === 0) continue; + for (const column of listed) out.add(column); + // An ALREADY-NORMALIZED entry carries its own resolved key parts and is + // honoured verbatim, so no tenant column is prepended a second time. + // Unreachable from an authored config — `IndexSchema` is a `strictObject` + // with no `nullSafeColumns` key — but mirrored anyway: dropping it would + // make this set a strict SUPERSET of the driver's and bound a column the + // driver leaves unbounded, which is this card's defect pointed the other + // way. + const preNormalized = + Array.isArray(idx?.nullSafeColumns) && + idx.nullSafeColumns.some((c: unknown) => listed.includes(c as string)); + if (!preNormalized && idx?.unique === 'organization' && tenantField && !listed.includes(tenantField)) { + out.add(tenantField); + } + } + return out; +} + /** * The column one field takes. * @@ -1272,24 +1447,36 @@ function declaredVarchar(maxLength: unknown): VarcharAnswer { * inherited key, and the unvalidated authoring door can deliver one as a * `type` string. */ -function fieldTypeToSql(fieldType: string, multiple?: boolean, maxLength?: unknown): string | null { +function fieldTypeToSql( + fieldType: string, + multiple?: boolean, + maxLength?: unknown, + keyed?: boolean, +): string | null { if (multiple) return FIELD_TYPE_SQL_MAP.json; const base = Object.prototype.hasOwnProperty.call(FIELD_TYPE_SQL_MAP, fieldType) ? FIELD_TYPE_SQL_MAP[fieldType] : 'TEXT'; - // #16091 — the STRING family is the one place the column depends on the FIELD - // and not only on its type, because that is the one place `createColumn` - // reads a declaration: its arm is `declared === null ? table.text(name) : - // table.string(name, declared)` over `declaredVarcharLength(field)`. Asked - // AFTER the table lookup, so the no-declaration outcome is the table's own - // entry rather than a second spelling of 255 that could drift from it. + // #16091 — TWO families depend on the FIELD and not only on its type, because + // those are the two arms `createColumn` reads a declaration in. They read + // DIFFERENT things and must not be collapsed into one. // - // ⛔ Deliberately not asked for the text family. `createColumn` branches that - // one on KEYED — `keyed ? this.keyableTextLength(field) : null` — and a - // generated migration emits no index, so a generated column is never keyed - // and the driver's answer is TEXT with or without a `maxLength`. Reading the - // declaration there would size a column the platform leaves unbounded, which - // is this card's own defect pointed the other way. + // The TEXT family branches on KEYED: `keyable === null ? table.text(name) : + // table.string(name, keyable)` over `keyed ? this.keyableTextLength(field) : + // null`. `keyed` is {@link indexKeyColumns}, which reads the object's own + // `field.unique` and `indexes[]` — so a declared-unique text field IS keyed + // here, and it is keyed whether or not this generator emits an index. + if (TEXT_FAMILY_TYPES.has(fieldType)) { + const keyable = keyed ? keyableTextChars(maxLength) : null; + // The unbounded spelling is READ from this table's own entry for the type + // rather than restated, so the two cannot drift. + return keyable === null ? base : `VARCHAR(${keyable})`; + } + // The STRING family branches on the declaration ALONE — `declared === null ? + // table.text(name) : table.string(name, declared)` over + // `declaredVarcharLength(field)`, which has no keyed requirement. Asked AFTER + // the table lookup, so the no-declaration outcome is the table's own entry + // rather than a second spelling of 255 that could drift from it. if (!STRING_FAMILY_TYPES.has(fieldType)) return base; const declared = declaredVarchar(maxLength); if (declared.kind === 'sized') return `VARCHAR(${declared.chars})`; @@ -1362,8 +1549,16 @@ export function generateMigrationSql(config: Record): string { lines.push(' "id" VARCHAR(255) PRIMARY KEY,'); const fieldLines: string[] = []; + // #16091 — resolved once per object, off the object's own declarations. See + // {@link indexKeyColumns}: the text family's width depends on it. + const keyColumns = indexKeyColumns(obj); for (const [fieldName, fieldDef] of Object.entries(fields)) { - const sqlType = fieldTypeToSql(String(fieldDef.type || 'text'), !!fieldDef.multiple, fieldDef.maxLength); + const sqlType = fieldTypeToSql( + String(fieldDef.type || 'text'), + !!fieldDef.multiple, + fieldDef.maxLength, + keyColumns.has(fieldName), + ); // #14828 — a VIRTUAL field materialises no column. `SqlDriver.createColumn` // returns without emitting one and `schema-drift.ts`'s `fieldHasColumn` // answers false for it, so a column here is one the runtime never writes. @@ -1468,6 +1663,10 @@ export function generateMigrationTs(config: Record): string { const tableName = String(obj.name || 'unknown'); const fields = (obj.fields ?? {}) as Record>; + // #16091 — resolved once per object, off the object's own declarations. See + // {@link indexKeyColumns}: the text family's width depends on it. + const keyColumns = indexKeyColumns(obj); + lines.push(` await db.schema.createTable('${tableName}', (table: any) => {`); // #15040 — the driver's own line for this column, emitted verbatim: // `table.string('id').primary()`. See `generateMigrationSql` above for the @@ -1535,21 +1734,33 @@ export function generateMigrationTs(config: Record): string { // #16091 — `text` MOVED here, out of the string arm above. It heads // `createColumn`'s text-family arm, whose column is // `keyable === null ? table.text(name) : table.string(name, keyable)` - // with `keyable = keyed ? this.keyableTextLength(field) : null`. The - // branch is on KEYED and a generated migration emits no index, so every - // column this generator creates takes the unkeyed answer: `table.text`, - // `maxLength` declared or not. Measured on live PostgreSQL 16.13 — - // driver `text`, both generators `varchar(255)`, and a 300-character - // value accepted by the platform's table and refused by both generated - // ones. + // with `keyable = keyed ? this.keyableTextLength(field) : null`. + // + // The branch is on KEYED, and `keyed` is the DECLARATION's answer, not + // this generator's: `indexedKeyColumns` reads `field.unique` and the + // object's `indexes[]`. So an unkeyed field takes `table.text`, + // `maxLength` or no `maxLength` — measured on live PostgreSQL 16.13, + // driver `text` against both generators' `varchar(255)`, a + // 300-character value accepted by the platform's table and refused by + // both generated ones — while a field the object declares unique takes + // the width `keyableTextLength` gives it, measured the other way round: + // `{ type: 'text', unique: true, maxLength: 100 }` was `varchar(100)` + // on the platform, refusing that same 300-character value, and TEXT in + // both generated tables, accepting it. case 'text': case 'textarea': case 'richtext': case 'html': case 'markdown': // #14657 — `driver-sql`'s own DDL switch puts these three in the text - // family (#11794, #11875): the declared `maxLength`, when there is one, - // is enforced at the write seam rather than by the column. - case 'code': case 'signature': case 'qrcode': - colMethod = `table.text('${fieldName}')`; + // family (#11794, #11875): the declared `maxLength`, when there is one + // and the column is not keyed, is enforced at the write seam rather + // than by the column. + case 'code': case 'signature': case 'qrcode': { + const keyable = keyColumns.has(fieldName) ? keyableTextChars(fieldDef.maxLength) : null; + colMethod = + keyable === null + ? `table.text('${fieldName}')` + : `table.string('${fieldName}', ${keyable})`; break; + } case 'number': case 'currency': case 'percent': // #14657 — NUMERIC_VALUE_TYPES: `valueSchemaFor` gives all of these // `z.number()`, and `driver-sql` gives them a float column. From 11d0e8d46f0c8d1a505493e0e1ea8371bbd2f22c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 14:52:12 +0000 Subject: [PATCH 4/6] test(cli): give the two mirrored driver bodies a driver-side oracle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `generate.ts` mirrors four things `driver-sql` owns. Two of them were already falsifiable from the driver: `MAX_KEYABLE_VARCHAR_CHARS` is compared against the constant's own declaration and `TEXT_FAMILY_TYPES` against `createColumn`'s own case labels, and a driver-side mutation of either reddens the pin. The other two mirror driver BODIES, which a source reader cannot see move — mutating `keyableTextLength` to clamp instead of answering null, and each of five mutations across `schema-drift`, `computeTenantField` and spec's `isUniqueDeclared`, left all 69 pins green. Both are now recomputed from `driver-sql` itself and compared: - the key set, from the driver's own exported `uniqueIndexesFromFields` and `normalizeDeclaredIndex` with the tenant column from a `SqlDriver` subclass that publishes `computeTenantField`, over a swept corpus of 1,224 objects (every combination of a field-level `unique` spelling, an `indexes[]` entry, a `tenancy` declaration and a column shape), against the key set read back out of what both generators emit; - both widths, from the driver's own `keyableTextLength` and `declaredVarcharLength` through the same subclass, over 37 declarations including the coerced and rejected spellings. A test file is not a CLI production module: #5726 governs `packages/cli/src/**` production sources, and the gate enforcing it excludes `*.test.ts` by construction. The package already declares `@objectstack/driver-sql` and the specifier is already in `KNOWN_UNALIASED_TEST_IMPORTS`, so neither the dependency graph nor that shrink-only ledger moves. The differential found one branch of `indexKeyColumns` disagreeing with the driver, and this fixes it. `normalizeDeclaredIndex` filters an entry's `nullSafeColumns` against its listed columns, but that filter narrows only `nullSafeColumns` — its `columns` stay the listed ones in every branch of the arm. Reading the filter as if it decided the KEY PARTS made `{ fields: ['f'], unique: 'organization', nullSafeColumns: ['zzz'] }` key `{organization_id, f}` here against the driver's `{f}`: a column bounded in a generated migration that the platform leaves unbounded. The condition is now the driver's own — a non-empty array, nothing more — and the comment claiming the mirrored branch kept this set from being a strict superset of the driver's is replaced, since that branch was the one making it exactly that. `isUniqueDeclared` and `isTenancyDisabled` are imported from `@objectstack/spec/data` rather than transcribed. Spec is not a driver package, so #5726 never reached them, and `isTenancyDisabled` is ADR-0066's single judgment for the registry, the engine and every driver. The transcriptions that remain now state their real warrant: `MAX_VARCHAR_CHARS`, `MAX_KEYABLE_VARCHAR_CHARS`, `keyableTextLength`, `declaredVarcharLength` and `computeTenantField` are `protected` and reach no exported surface, while `isOrganizationScopedUnique` is exported and is spelled here only because these generators are synchronous and #5726 leaves a production module `await import()` alone for a driver package. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- .../generate-string-family-width.pin.test.ts | 463 +++++++++++++++++- packages/cli/src/commands/generate.ts | 61 ++- 2 files changed, 513 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/commands/generate-string-family-width.pin.test.ts b/packages/cli/src/commands/generate-string-family-width.pin.test.ts index 5afe2c636e..9e0bfad69e 100644 --- a/packages/cli/src/commands/generate-string-family-width.pin.test.ts +++ b/packages/cli/src/commands/generate-string-family-width.pin.test.ts @@ -135,6 +135,23 @@ * non-vacuity control — a source reader that matched nothing would pass while * measuring nothing. * + * ## Reading the source is only half of it — the ORACLE is the other half + * + * A source reader catches a driver that moves a case label or renames a + * constant. It cannot catch a driver whose BODY changes while its shape stays, + * and `generate.ts` mirrors two driver BODIES: `keyableTextLength`'s coercion, + * and the key set `indexedKeyColumns` composes. Both were mutated in the driver + * and this file stayed GREEN through every one of them — a mirror with no + * falsifier, on a card whose whole subject is generator/driver divergence. + * + * So the second half of this file RECOMPUTES both from `driver-sql` itself — + * its exported `uniqueIndexesFromFields` / `normalizeDeclaredIndex`, and its + * own `protected` judgments through a subclass — and compares, over a swept + * corpus rather than a hand-listed one. That differential is what found the + * `nullSafeColumns` branch this round repaired: `{ fields: ['f'], unique: + * 'organization', nullSafeColumns: ['zzz'] }` keyed `{organization_id, f}` here + * against the driver's `{f}`, and no enumerated case had reached the shape. + * * ⚠️ Scope, as `generateMigrationSql`'s docblock and the `--format` help text * already say (#15521): this is a POSTGRESQL claim and nothing else. Neither * generator reproduces the driver's dialect branching. @@ -144,13 +161,34 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +// ⭐ The ORACLE's imports. A test file is NOT a CLI production module: the +// #5726 constraint that forces `generate.ts` to transcribe (oclif `import()`s +// every command module on every invocation, so a static driver edge charges an +// unbuilt driver to whatever command the operator actually ran) is a rule about +// `packages/cli/src/**` production sources, and +// `schema-migrate.lazy-driver-import.test.ts` — the gate that enforces it — +// excludes `*.test.ts` by construction. `@objectstack/driver-sql` is already +// this package's declared dependency and already listed in +// `KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/cli']`, so this import widens +// neither the dependency graph nor that shrink-only ledger. +// +// ⚠️ These resolve to the driver's BUILT artifact while the readers above read +// its SOURCE. A driver-side change must therefore be rebuilt before this half +// can see it — which is what CI does (`@objectstack/cli#test` dependsOn +// `build`), and what a local mutation run has to do by hand. +import { + SqlDriver, + normalizeDeclaredIndex, + uniqueIndexesFromFields, + type DeclaredIndexInput, +} from '@objectstack/driver-sql'; import { FieldType, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, STRUCTURED_JSON_TYPES, } from '@objectstack/spec/data'; -import { describe, expect, it } from 'vitest'; +import { afterAll, describe, expect, it } from 'vitest'; import { generateMigrationSql, generateMigrationTs } from './generate.js'; @@ -793,3 +831,426 @@ describe('#16091 — the character column both generators emit is the driver\'s' expect('VARCHAR(2048)').toBe('VARCHAR(2048)'); }); }); + +// ── The driver-side oracle ────────────────────────────────────────────────── +// +// Everything above reads the driver's SOURCE TEXT. Two of the four mirrors +// `generate.ts` carries are fully covered by that — `MAX_KEYABLE_VARCHAR_CHARS` +// is compared against the constant's own declaration, and `TEXT_FAMILY_TYPES` +// against `createColumn`'s own case labels, and a driver-side mutation of +// either reddens this file. The other two mirror driver BODIES, which a source +// reader cannot see move: +// +// `keyableTextChars` ← `SqlDriver.keyableTextLength` +// `indexKeyColumns` ← `schema-drift.ts`'s `indexedKeyColumns` +// +// Mutating those two in the driver left this file GREEN. What follows removes +// that: both are RECOMPUTED from `driver-sql` itself and compared against what +// the generators actually emit. + +/** + * The driver's own `protected` judgments, reached by widening rather than + * re-derived. + * + * Widening is the whole technique: `protected` is a compile-time visibility + * rule, so a subclass can publish the driver's OWN method body without copying + * a character of it. The moment the driver's body changes, this oracle changes + * with it — which is exactly what the transcriptions in `generate.ts` cannot do. + */ +class DriverOracle extends SqlDriver { + /** `SqlDriver.computeTenantField`, unmodified. */ + public tenantFieldFor(object: { fields?: Record; tenancy?: unknown }): string | null { + return this.computeTenantField(object); + } + + /** `SqlDriver.keyableTextLength`, unmodified — the KEYED text-family width. */ + public keyableCharsFor(field: unknown): number | null { + return this.keyableTextLength(field); + } + + /** `SqlDriver.declaredVarcharLength`, unmodified — the string-family width. */ + public declaredCharsFor(field: unknown): number | null { + return this.declaredVarcharLength(field); + } +} + +// The same in-memory shape every SqlDriver test in this repo constructs. No +// query is ever issued through it: this file asks the driver only questions it +// answers from the declaration in front of it, so the pool never opens a +// connection. +const ORACLE = new DriverOracle({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, +}); + +afterAll(async () => { + await ORACLE.disconnect(); +}); + +/** + * `schema-drift.ts`'s `indexedKeyColumns`, composed HERE from the driver's own + * two exported builders. + * + * That function is not itself exported, but the two normalizers it composes are + * (`index.ts`), and composing them is the whole of its body: it records every + * column of every index those two return. So this recomputes the driver's key + * set without transcribing a single one of its judgments — the scope + * vocabulary, the tenant prepend, the `nullSafeColumns` arm and the tenant + * column itself all arrive from `driver-sql`. + */ +function driverKeyColumns(object: Record): Set { + const table = String(object.name); + const tenantField = ORACLE.tenantFieldFor(object); + const out = new Set(); + const record = (index: { columns: string[] } | null) => { + if (index) for (const column of index.columns) out.add(column); + }; + for (const index of uniqueIndexesFromFields(table, object.fields ?? {}, tenantField)) record(index); + for (const index of Array.isArray(object.indexes) ? object.indexes : []) { + record(normalizeDeclaredIndex(table, index as DeclaredIndexInput, tenantField)); + } + return out; +} + +/** + * The width one emitted SQL column declares: `null` for an unbounded one. + * + * Both producers are normalized to the driver's own units — a NUMBER OF + * CHARACTERS or `null` — so an assertion can compare them against what the + * driver returned instead of against a spelling. + */ +function sqlWidth(column: string | null): number | null { + if (column === 'TEXT') return null; + const m = column?.match(/^VARCHAR\((\d+)\)$/); + if (!m) throw new Error(`not a character column: ${String(column)}`); + return Number(m[1]); +} + +/** + * The width one emitted `table.x('f'…)` call declares. + * + * A bare `table.string('f')` is knex's default width — the same column the + * driver's own bare `table.string(name)` builds — so it normalizes to + * {@link DEFAULT_CHARS} rather than to "no width". Without that the two + * producers would look like they disagreed on every undeclared string field, + * where they in fact emit the same column two ways. + */ +function tsWidth(call: string | null): number | null { + if (call === "table.text('f')") return null; + const m = call?.match(/^table\.string\('f'(?:, (\d+))?\)$/); + if (!m) throw new Error(`not a character column: ${String(call)}`); + return m[1] === undefined ? DEFAULT_CHARS : Number(m[1]); +} + +/** + * The width every probe field in the key-set corpus declares. + * + * Chosen inside the key-part ceiling so that, for a text-family field, the + * emitted column IS the keyed bit: `VARCHAR(PROBE_CHARS)` means the generator + * keyed the column and `TEXT` means it did not. The control case asserts that + * relation rather than assuming it. + */ +const PROBE_CHARS = 100; + +/** One probe object, and the id the failure message names it by. */ +interface KeyProbe { + id: string; + object: Record; +} + +/** + * The swept corpus: every combination of a field-level `unique` spelling, an + * object-level `indexes[]` entry, a `tenancy` declaration and a column shape. + * + * Swept rather than enumerated deliberately. A hand-listed set of cases is + * exactly what this file already had, and the divergence it carried lived in a + * combination nobody had thought to write down. + */ +function keyProbeCorpus(): KeyProbe[] { + const uniques: Array<[string, unknown]> = [ + ['unique-absent', undefined], + ['unique-true', true], + ['unique-false', false], + ['unique-global', 'global'], + ['unique-organization', 'organization'], + ['unique-nonsense', 'tenant'], + ]; + const indexSets: Array<[string, unknown[] | undefined]> = [ + ['no-indexes', undefined], + ['plain', [{ fields: ['f'] }]], + ['idx-true', [{ fields: ['f'], unique: true }]], + ['idx-global', [{ fields: ['f'], unique: 'global' }]], + ['idx-org', [{ fields: ['f'], unique: 'organization' }]], + ['idx-org-composite', [{ fields: ['other', 'f'], unique: 'organization' }]], + ['idx-org-lists-tenant', [{ fields: ['organization_id', 'f'], unique: 'organization' }]], + // The already-normalized shapes — the arm this round repaired. All three + // matter: one whose `nullSafeColumns` names a listed column, one whose + // names a STRANGER (the divergence), and one that is empty (which is not + // the normalized shape at all and must fall through to the prepend). + ['pre-normalized-listed', [{ fields: ['organization_id', 'f'], unique: 'organization', nullSafeColumns: ['organization_id'] }]], + ['pre-normalized-stranger', [{ fields: ['f'], unique: 'organization', nullSafeColumns: ['zzz'] }]], + ['pre-normalized-empty', [{ fields: ['f'], unique: 'organization', nullSafeColumns: [] }]], + ['pre-normalized-not-array', [{ fields: ['f'], unique: 'organization', nullSafeColumns: 'organization_id' }]], + // Unusable entries: `normalizeDeclaredIndex` answers null for all of them. + ['idx-no-fields', [{ unique: true }]], + ['idx-empty-fields', [{ fields: [] }]], + ['idx-nonstring-fields', [{ fields: [1, '', null, 'f'] }]], + // A key part naming a column the object never declares. + ['idx-ghost', [{ fields: ['ghost'], unique: 'organization' }]], + ['two-indexes', [{ fields: ['other'] }, { fields: ['f'], unique: 'organization' }]], + ]; + const tenancies: Array<[string, unknown]> = [ + ['tenancy-absent', undefined], + ['tenancy-disabled', { enabled: false }], + ['tenancy-enabled', { enabled: true }], + ['tenancy-named', { tenantField: 'org' }], + ['tenancy-named-missing', { tenantField: 'nosuch' }], + ['tenancy-disabled-and-named', { enabled: false, tenantField: 'org' }], + ]; + // Which columns the object declares at all — the implicit `organization_id` + // heuristic only fires where that column exists. + const shapes: Array<[string, string[]]> = [ + ['with-organization_id', ['f', 'other', 'organization_id', 'org']], + ['without-organization_id', ['f', 'other', 'org']], + ]; + + const probes: KeyProbe[] = []; + for (const [shapeId, columns] of shapes) { + for (const [uniqueId, unique] of uniques) { + for (const [indexId, indexes] of indexSets) { + for (const [tenancyId, tenancy] of tenancies) { + const fields: Record> = {}; + for (const column of columns) { + fields[column] = { type: 'text', maxLength: PROBE_CHARS }; + } + fields.f = { type: 'text', maxLength: PROBE_CHARS, ...(unique === undefined ? {} : { unique }) }; + probes.push({ + id: `${shapeId}/${uniqueId}/${indexId}/${tenancyId}`, + object: { + name: 'probe', + fields, + ...(indexes ? { indexes } : {}), + ...(tenancy ? { tenancy } : {}), + }, + }); + } + } + } + } + return probes; +} + +/** + * The key set the GENERATORS computed, read back out of what they emitted. + * + * Deliberately observational rather than an exported internal: what a reviewer + * and a user care about is the DDL, and reading it back proves the mirror is + * reached on the real path rather than merely being correct in isolation. Every + * probe field is a text-family field with a bound one key part can hold, so the + * emitted column is the keyed bit — and any column that is neither of the two + * expected answers throws rather than being silently counted as unkeyed. + * + * Both formats are read, and they must agree: a mirror consulted by one + * generator and not the other would otherwise pass here. + */ +function generatorKeyColumns(object: Record, id: string): Set { + const config = { objects: { probe: object } } as Record; + const sql = generateMigrationSql(config); + const ts = generateMigrationTs(config); + const keyed = new Set(); + for (const field of Object.keys(object.fields)) { + const sqlCol = sqlColumn(sql, field); + const tsCol = tsColumn(ts, field); + const sqlChars = sqlWidth(sqlCol); + const tsChars = tsCol === `table.text('${field}')` + ? null + : Number(tsCol?.match(/^table\.string\('[^']+'(?:, (\d+))?\)$/)?.[1] ?? DEFAULT_CHARS); + expect( + tsChars, + `${id}: the two formats disagree about '${field}' — sql ${String(sqlCol)}, ts ${String(tsCol)}`, + ).toBe(sqlChars); + if (sqlChars === PROBE_CHARS) keyed.add(field); + else if (sqlChars !== null) { + throw new Error(`${id}: '${field}' is neither the keyed nor the unkeyed answer: ${String(sqlCol)}`); + } + } + return keyed; +} + +describe('#16091 — the driver is the ORACLE, not just the source text', () => { + it('control — the oracle is really the driver, and it really discriminates', () => { + // Non-vacuity for the oracle itself. Without these, an import that resolved + // to something inert would make every differential below compare two empty + // sets and pass while measuring nothing. + expect(ORACLE).toBeInstanceOf(SqlDriver); + expect(typeof uniqueIndexesFromFields).toBe('function'); + expect(typeof normalizeDeclaredIndex).toBe('function'); + + // The driver's builders answer, and they answer DIFFERENTLY for shapes that + // differ — a stub returning `[]`/`null` would be caught here. + expect(uniqueIndexesFromFields('t', { a: { unique: true } }, null)).toHaveLength(1); + expect(uniqueIndexesFromFields('t', { a: { unique: false } }, null)).toHaveLength(0); + expect(normalizeDeclaredIndex('t', { fields: ['a'] }, null)?.columns).toEqual(['a']); + expect(normalizeDeclaredIndex('t', { fields: [] }, null)).toBeNull(); + + // The protected judgments came through the subclass intact. + expect(ORACLE.tenantFieldFor({ fields: { organization_id: {} } })).toBe('organization_id'); + expect(ORACLE.tenantFieldFor({ fields: { organization_id: {} }, tenancy: { enabled: false } })).toBeNull(); + expect(ORACLE.keyableCharsFor({ maxLength: PROBE_CHARS })).toBe(PROBE_CHARS); + expect(ORACLE.keyableCharsFor({ maxLength: KEYABLE_CHARS + 1 })).toBeNull(); + expect(ORACLE.declaredCharsFor({})).toBe(DEFAULT_CHARS); + + // The probe width really is inside the key-part ceiling, which is what + // makes an emitted `VARCHAR(PROBE_CHARS)` mean "keyed" below. + expect(PROBE_CHARS).toBeLessThanOrEqual(KEYABLE_CHARS); + expect(PROBE_CHARS).not.toBe(DEFAULT_CHARS); + + // The two normalizers really do read the emitted column back. + expect(sqlWidth('TEXT')).toBeNull(); + expect(sqlWidth('VARCHAR(100)')).toBe(100); + expect(() => sqlWidth('JSONB')).toThrow(); + expect(tsWidth("table.text('f')")).toBeNull(); + expect(tsWidth("table.string('f', 100)")).toBe(100); + expect(tsWidth("table.string('f')")).toBe(DEFAULT_CHARS); + expect(() => tsWidth("table.jsonb('f')")).toThrow(); + }); + + // ── F1: the key set, recomputed from the driver's own builders ──────────── + + it('the corpus is a real sweep, and both sides really vary across it', () => { + const corpus = keyProbeCorpus(); + // A differential over a corpus that answers one thing everywhere proves + // nothing, so the corpus's own discriminating power is asserted first. + expect(corpus.length).toBeGreaterThan(200); + const driverAnswers = new Set(corpus.map((p) => [...driverKeyColumns(p.object)].sort().join(','))); + expect(driverAnswers.size).toBeGreaterThan(4); + expect(driverAnswers.has('')).toBe(true); + // At least one probe keys the tenant column, and at least one keys the + // field without it — the distinction the repaired branch turns on. + expect([...driverAnswers].some((a) => a.includes('organization_id'))).toBe(true); + expect([...driverAnswers].some((a) => a === 'f')).toBe(true); + // Ids are unique, so a failure below names exactly one probe. + expect(new Set(corpus.map((p) => p.id)).size).toBe(corpus.length); + }); + + it('every column the generators key is a column the DRIVER keys, over the whole corpus', () => { + const divergences: string[] = []; + for (const { id, object } of keyProbeCorpus()) { + const declared = new Set(Object.keys(object.fields)); + // The driver's answer, restricted to the columns this object declares. + // A key part naming an undeclared column materialises no column in ANY of + // the three producers, so it is not observable here and not a divergence + // — `idx-ghost` is in the corpus to keep that case exercised rather than + // assumed. + const oracle = [...driverKeyColumns(object)].filter((c) => declared.has(c)).sort(); + const emitted = [...generatorKeyColumns(object, id)].sort(); + if (JSON.stringify(oracle) !== JSON.stringify(emitted)) { + divergences.push(`${id}: driver keys [${oracle}] · generators key [${emitted}]`); + } + } + expect( + divergences, + 'The generators sized a character column from a key set that is not the one driver-sql ' + + 'computes for the same object. Every entry is a column whose generated type disagrees ' + + 'with the platform\'s — the whole of #16091. The authority is driver-sql: fix ' + + '`indexKeyColumns` in generate.ts, never this expectation.', + ).toEqual([]); + }); + + it('the pre-normalized index arm keys exactly what the driver keys, by name', () => { + // ⭐ The divergence this round repaired, spelled out so it cannot come back + // unnoticed inside a sweep. `normalizeDeclaredIndex` filters + // `nullSafeColumns` against the listed columns, but that filter narrows + // only `nullSafeColumns` — its `columns` are the listed ones in every + // branch of the arm, so a `nullSafeColumns` naming NO listed column still + // prepends nothing. + const fields = { + f: { type: 'text', maxLength: PROBE_CHARS }, + organization_id: { type: 'text', maxLength: PROBE_CHARS }, + }; + const stranger = { + name: 'probe', + fields, + indexes: [{ fields: ['f'], unique: 'organization', nullSafeColumns: ['zzz'] }], + }; + expect([...driverKeyColumns(stranger)].sort()).toEqual(['f']); + expect([...generatorKeyColumns(stranger, 'pre-normalized-stranger')].sort()).toEqual(['f']); + + // The counter-case, which is what makes the one above a measurement: the + // SAME index without `nullSafeColumns` does prepend the tenant column. + const prepending = { + name: 'probe', + fields, + indexes: [{ fields: ['f'], unique: 'organization' }], + }; + expect([...driverKeyColumns(prepending)].sort()).toEqual(['f', 'organization_id']); + expect([...generatorKeyColumns(prepending, 'idx-org')].sort()).toEqual(['f', 'organization_id']); + }); + + // ── F2: the two width bodies, recomputed from the driver's own methods ──── + + /** + * The declarations both width sweeps run. Deliberately wider than anything an + * `IndexSchema`-valid config can carry: `maxLength` reaches these functions + * through an unvalidated authoring door too, and the coercion is precisely + * the body being mirrored. + */ + const WIDTH_DECLARATIONS: unknown[] = [ + undefined, null, 0, -1, -5, 1, 12.5, 64, 100, 255, 767, 768, 769, 1000, + MAX_CHARS, MAX_CHARS + 1, Number.MAX_SAFE_INTEGER, NaN, Infinity, -Infinity, + '1', '64', '100', '768', '769', '0', '-5', '12.5', '1e3', '0x10', '', ' ', + 'not a number', true, false, [], [100], {}, + ]; + + it('a KEYED text column takes the width driver-sql\'s own keyableTextLength returns', () => { + const answers = new Set(); + for (const maxLength of WIDTH_DECLARATIONS) { + // The driver's own method body, not a re-derivation of its rules. + const chars = ORACLE.keyableCharsFor({ maxLength }); + answers.add(chars); + const { sql, ts } = columnsFor({ type: 'text', unique: true, maxLength }); + const shown = JSON.stringify(maxLength) ?? String(maxLength); + expect( + sqlWidth(sql), + `keyed text @ maxLength ${shown}: driver-sql's keyableTextLength says ${String(chars)}`, + ).toBe(chars); + expect(tsWidth(ts), `keyed text @ maxLength ${shown}, typescript format`).toBe(chars); + } + // Non-vacuity: the sweep really produced both dispositions and more than + // one width, so it is not one answer asserted 37 times. + expect(answers.has(null)).toBe(true); + expect([...answers].filter((a) => a !== null).length).toBeGreaterThan(3); + }); + + it('a STRING-family column takes the width driver-sql\'s own declaredVarcharLength returns', () => { + const answers = new Set(); + for (const maxLength of WIDTH_DECLARATIONS) { + const chars = ORACLE.declaredCharsFor({ maxLength }); + answers.add(chars); + const { sql, ts } = columnsFor({ type: 'email', maxLength }); + const shown = JSON.stringify(maxLength) ?? String(maxLength); + expect( + sqlWidth(sql), + `email @ maxLength ${shown}: driver-sql's declaredVarcharLength says ${String(chars)}`, + ).toBe(chars); + expect(tsWidth(ts), `email @ maxLength ${shown}, typescript format`).toBe(chars); + } + expect(answers.has(null)).toBe(true); + expect(answers.has(DEFAULT_CHARS)).toBe(true); + expect([...answers].filter((a) => a !== null).length).toBeGreaterThan(3); + }); + + it('the two width bodies are DIFFERENT bodies, and the oracle sees the difference', () => { + // The two must not be collapsed, and this is the assertion that would fail + // if a future edit routed both mirrors through one helper: at 1000 the + // string family is sized and the text family is unbounded, and with no + // declaration at all the two answer the other way round. + expect(ORACLE.keyableCharsFor({ maxLength: 1000 })).toBeNull(); + expect(ORACLE.declaredCharsFor({ maxLength: 1000 })).toBe(1000); + expect(ORACLE.keyableCharsFor({})).toBeNull(); + expect(ORACLE.declaredCharsFor({})).toBe(DEFAULT_CHARS); + expect(sqlWidth(columnsFor({ type: 'text', unique: true, maxLength: 1000 }).sql)).toBeNull(); + expect(sqlWidth(columnsFor({ type: 'email', maxLength: 1000 }).sql)).toBe(1000); + }); +}); diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 796cf1ce6f..9be9f18dd2 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -9,6 +9,15 @@ import path from 'path'; // `satisfies Record`, which is what makes a field type added to // the spec a named compile error here instead of a silent fallback (#14657). import type { FieldType } from '@objectstack/spec/data'; +// #16091 — IMPORTED, not transcribed. Both are on `@objectstack/spec/data`'s +// exported surface, and spec is not a driver package: the #5726 constraint the +// transcriptions below cite forbids a static value import of an +// `@objectstack/driver-*` package and nothing else, so it never reached these. +// The two driver-side readers that consume them — `isUniqueScopeDeclared` and +// `computeTenantField` — are spelled here in the driver's own terms ON TOP of +// these, so the part that can be shared is shared and only the part that +// genuinely lives on `driver-sql` is mirrored. +import { isTenancyDisabled, isUniqueDeclared } from '@objectstack/spec/data'; import { printHeader, printSuccess, printError, printInfo, printStep, createTimer, CLI_ALIAS } from '../utils/format.js'; import { metadataFileName } from '../utils/metadata-file-name.js'; @@ -1307,10 +1316,16 @@ function declaredVarchar(maxLength: unknown): VarcharAnswer { /** * `schema-drift.ts`'s `isUniqueScopeDeclared` — the FIELD-level unique - * vocabulary. Transcribed, for the reasons {@link MAX_VARCHAR_CHARS} gives. + * vocabulary. + * + * Spelled as the driver spells it, over the SAME spec predicate the driver + * calls: `unique === 'organization' || isUniqueDeclared(unique)`. The word is + * accepted here ahead of the spec helper deliberately (ADR-0120 D1, driver + * first), so the disjunct is the driver's, not this file's invention — and the + * half that IS spec's is imported rather than retyped. */ function isUniqueScopeDeclared(unique: unknown): boolean { - return unique === true || unique === 'global' || unique === 'organization'; + return unique === 'organization' || isUniqueDeclared(unique); } /** @@ -1322,6 +1337,17 @@ function isUniqueScopeDeclared(unique: unknown): boolean { * as global. That the two paths read the same token differently is a maintainer * ruling (2026-08-13), not an oversight, so the two readings stay apart here * too. + * + * ⚠️ Unlike {@link MAX_VARCHAR_CHARS}, this one IS on `driver-sql`'s exported + * surface, so "not exported" is not the reason it is spelled here — say the + * real one instead of borrowing that block's: these generators are SYNCHRONOUS, + * and #5726 leaves a CLI production module only `await import()` for a driver + * package, which a synchronous function cannot use. Spec's own + * `isOrganizationUnique` is not a substitute either: it detects the WORD and + * not the scope, so it omits the bare `true` this predicate exists to include. + * What makes the spelling safe is the oracle in + * `generate-string-family-width.pin.test.ts`, which recomputes this whole key + * set from the driver's own exported builders and fails when the two disagree. */ function isOrganizationScopedUnique(unique: unknown): boolean { return unique === true || unique === 'organization'; @@ -1334,10 +1360,15 @@ function isOrganizationScopedUnique(unique: unknown): boolean { * Computable from the object alone, which is why it is mirrored rather than * skipped: explicit opt-out wins, then a declared `tenancy.tenantField` that * names a real field, then a field literally named `organization_id`. + * + * The opt-out itself is spec's `isTenancyDisabled`, IMPORTED — the driver calls + * that same function here (ADR-0066: one judgment for the registry, the engine + * and every driver), so re-deriving `tenancy?.enabled === false` locally would + * be a fourth copy of the thing that helper exists to stop. */ function tenantFieldOf(obj: Record): string | null { const tenancy = obj?.tenancy; - if (tenancy?.enabled === false) return null; + if (isTenancyDisabled(obj)) return null; const fields = obj?.fields; if (tenancy?.tenantField) { const declared = String(tenancy.tenantField); @@ -1393,13 +1424,23 @@ function indexKeyColumns(obj: Record): ReadonlySet { // An ALREADY-NORMALIZED entry carries its own resolved key parts and is // honoured verbatim, so no tenant column is prepended a second time. // Unreachable from an authored config — `IndexSchema` is a `strictObject` - // with no `nullSafeColumns` key — but mirrored anyway: dropping it would - // make this set a strict SUPERSET of the driver's and bound a column the - // driver leaves unbounded, which is this card's defect pointed the other - // way. - const preNormalized = - Array.isArray(idx?.nullSafeColumns) && - idx.nullSafeColumns.some((c: unknown) => listed.includes(c as string)); + // with no `nullSafeColumns` key — but mirrored anyway, because the driver + // answers this shape and a generated column has to be the column the driver + // would build for the same object however the object got here. + // + // ⭐ The condition is the driver's OWN: a non-empty `nullSafeColumns` + // ARRAY, nothing more. `normalizeDeclaredIndex` filters that array against + // the listed columns, but the filter narrows only `nullSafeColumns` — its + // `columns` stay the listed ones in every branch of that arm, so an entry + // whose `nullSafeColumns` names no listed column still prepends NOTHING. + // Asking `.some(c => listed.includes(c))` here instead read that filter as + // if it decided the KEY PARTS: on `{ fields: ['f'], unique: 'organization', + // nullSafeColumns: ['zzz'] }` the driver keys `{f}` and this keyed + // `{organization_id, f}` — a column bounded here that the platform leaves + // unbounded, this card's defect pointed the other way. Found by the + // differential in `generate-string-family-width.pin.test.ts`, which now + // recomputes this whole set from the driver's own exported builders. + const preNormalized = Array.isArray(idx?.nullSafeColumns) && idx.nullSafeColumns.length > 0; if (!preNormalized && idx?.unique === 'organization' && tenantField && !listed.includes(tenantField)) { out.add(tenantField); } From f3661ac079e2484bf7b5da3c7c8ef5b371b0165a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 15:01:20 +0000 Subject: [PATCH 5/6] test(cli): ask the driver's own unique predicates, not their source text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two scope predicates `generate.ts` mirrors were pinned by reading `schema-drift.ts` for the exact line each is spelled on. That catches a rewording and nothing else: a driver whose vocabulary narrows while the line survives leaves the generators sizing a column the platform would not key, and the pin green. Both are exported, so the pin now ASKS them — `isUniqueScopeDeclared` over sixteen `unique` spellings against the width each produces in the emitted DDL, and `isOrganizationScopedUnique` over the same spellings against whether the tenant column is keyed with them. Measured by mutating the driver's `isUniqueScopeDeclared` to drop the bare-`true` and `'global'` spellings, rebuilding `driver-sql` and re-running: five pins go red, of which four are reachable only through the oracle. This is also the axis the `@objectstack/spec/data` import closes. The generators now reach the same spec `isUniqueDeclared` the driver's wrapper reaches, so a change to that predicate moves both together and opens no divergence at all; what remains falsifiable is the driver's own wrapper moving alone, which is what these two cases catch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- .../generate-string-family-width.pin.test.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/packages/cli/src/commands/generate-string-family-width.pin.test.ts b/packages/cli/src/commands/generate-string-family-width.pin.test.ts index 9e0bfad69e..fe9fcf5a0d 100644 --- a/packages/cli/src/commands/generate-string-family-width.pin.test.ts +++ b/packages/cli/src/commands/generate-string-family-width.pin.test.ts @@ -178,6 +178,8 @@ import { fileURLToPath } from 'node:url'; // `build`), and what a local mutation run has to do by hand. import { SqlDriver, + isOrganizationScopedUnique, + isUniqueScopeDeclared, normalizeDeclaredIndex, uniqueIndexesFromFields, type DeclaredIndexInput, @@ -1116,6 +1118,66 @@ describe('#16091 — the driver is the ORACLE, not just the source text', () => expect(() => tsWidth("table.jsonb('f')")).toThrow(); }); + // ── The unique VOCABULARY, against the driver's own two predicates ──────── + + /** + * Every spelling worth asking about, including the ones the spec rejects by + * name — an "anything truthy" reading would silently accept those. + */ + const UNIQUE_SPELLINGS: unknown[] = [ + true, false, undefined, null, 0, 1, '', 'global', 'organization', + 'tenant', 'org', 'yes', 'GLOBAL', 'Organization', {}, [], + ]; + + it('the unique vocabulary is the driver\'s own predicate, spelling for spelling', () => { + // ⭐ This is the axis the `@objectstack/spec/data` import buys. `generate.ts` + // now reaches the same spec `isUniqueDeclared` the driver's wrapper reaches, + // so a change to THAT predicate moves the driver and the generators together + // and cannot open a divergence at all. What can still open one is the + // driver's own wrapper moving alone — which is what this catches, against + // `isUniqueScopeDeclared` itself rather than against its source text. + const disagreements: string[] = []; + for (const unique of UNIQUE_SPELLINGS) { + const driverKeys = isUniqueScopeDeclared(unique); + const generatorsKeyed = + sqlWidth(columnsFor({ type: 'text', maxLength: PROBE_CHARS, unique }).sql) === PROBE_CHARS; + if (driverKeys !== generatorsKeyed) { + disagreements.push(`${JSON.stringify(unique)}: driver ${driverKeys}, generators ${generatorsKeyed}`); + } + } + expect( + disagreements, + 'A `unique` spelling the driver keys on is a spelling the generated column must be sized ' + + 'for, and one it does not key on is one the generated column must leave unbounded.', + ).toEqual([]); + // Non-vacuity: the sweep really carries both answers, in quantity. + expect(UNIQUE_SPELLINGS.filter((u) => isUniqueScopeDeclared(u))).toHaveLength(3); + expect(UNIQUE_SPELLINGS.filter((u) => !isUniqueScopeDeclared(u)).length).toBeGreaterThan(8); + }); + + it('the organization-SCOPE vocabulary is the driver\'s own predicate too', () => { + // The second half of the field-level judgment: which of those spellings + // ALSO keys the tenant column. Asked of the driver's exported predicate, + // not of the word — bare `true` is organization-scoped at field level and + // `'global'` is not, a distinction a "detects the word" reading loses. + for (const unique of UNIQUE_SPELLINGS) { + const scopedByDriver = isUniqueScopeDeclared(unique) && isOrganizationScopedUnique(unique); + const out = emit({ + f: { type: 'text', maxLength: PROBE_CHARS, unique }, + organization_id: { type: 'text', maxLength: PROBE_CHARS }, + }); + const tenantKeyed = sqlWidth(sqlColumn(out.sql, 'organization_id')) === PROBE_CHARS; + expect( + tenantKeyed, + `unique: ${JSON.stringify(unique)} — the driver ${scopedByDriver ? 'keys' : 'does not key'} ` + + 'the tenant column for this spelling', + ).toBe(scopedByDriver); + } + // Non-vacuity: both dispositions really occur across the sweep. + expect(UNIQUE_SPELLINGS.some((u) => isUniqueScopeDeclared(u) && isOrganizationScopedUnique(u))).toBe(true); + expect(UNIQUE_SPELLINGS.some((u) => isUniqueScopeDeclared(u) && !isOrganizationScopedUnique(u))).toBe(true); + }); + // ── F1: the key set, recomputed from the driver's own builders ──────────── it('the corpus is a real sweep, and both sides really vary across it', () => { From 722880a1bd8ad1b40a0c5a563710c812bbcf2784 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 16:03:20 +0000 Subject: [PATCH 6/6] test(cli): make the oracle enter initObjects, not re-compose its leaves CORRECTING THE RECORD, first. Commit 11d0e8d46f0's message states "a swept corpus of 1,224 objects" and "37 declarations". Both counts are wrong, and this queue composes the squash body from the branch's commit messages, so they would land in `main` as written. Counted mechanically by parsing the array literals and confirmed by generating the corpus: keyProbeCorpus() 6 uniques x 16 indexSets x 6 tenancies x 2 shapes = 1,152 WIDTH_DECLARATIONS 38 Four of those sixteen index shapes are the already-normalized ones, not three. Nothing in the suite caught either number: the only size assertion was `> 200`, which every wrong count satisfies. Both are now pinned as exact literals, so a corpus that grows without its stated size growing fails here rather than putting a false measurement into a permanent record. ASKING THE DRIVER'S LEAVES IS NOT ASKING THE DRIVER Round 2 transcribed the driver's answers, and mutating the driver left every pin green. Round 3 asked the driver's exported LEAVES -- `uniqueIndexesFromFields`, `normalizeDeclaredIndex`, `computeTenantField` -- and then RE-COMPOSED them in the test file, which left every layer between those leaves and the emitted column a second copy of the pin's own belief. It never called the driver's own `indexedKeyColumns`, nor `initObjects`' wiring of `tenantField` into it, nor `createColumn`'s dispatch on `keyed`. Measured driver-side at f3661ac079e, each mutation rebuilt into `dist`: indexedKeyColumns stops recording declared indexes 78 passed (78) initObjects passes tenantField: null into it 78 passed (78) against 764 and 276 of 1,152 objects respectively diverging between the real `initObjects` and the generators. Both of those are this card's own subject -- the driver changes what it keys and the generated column stays bounded where the platform's is unbounded -- and the instrument reported everything fine. The reddening of the pin as it now stands, under both mutations, is recorded in the PR body with its counts. WHAT MOVES The authority in the pin is now `SqlDriver.initObjects` on the in-memory better-sqlite3 driver the file already constructs, read back with `PRAGMA table_info`. That is computeAndRecordTenantField -> indexedKeyColumns -> createColumn -> knex -> an actual column, with nothing re-derived in the test. Two differentials run over it: * the whole 1,152-object corpus, comparing all 4,032 declared columns against both generators' emitted width; * every character TYPE the driver cases or catches -- membership read off `createColumn`'s own case labels and its catch-all derivation, 18 today -- at all 38 declarations, keyed and unkeyed, 1,368 probes. The leaf differential is KEPT underneath, because it localises a failure to one builder, and is now documented as NOT the authority. The width differentials against `keyableTextLength` / `declaredVarcharLength` are kept for the same reason: they say which method body moved, while the real chain also covers `createColumn`'s dispatch onto them. Each probe mints its own table name. `initObjects` takes the ALTER path on a name it has already seen and an ALTER cannot retype a column, so a shared name would report the first probe's answer for all 1,152. The driver's warnings are captured into the subclass rather than printed -- the corpus deliberately carries index shapes whose key parts name no materialized column, and the driver correctly says so 144 times on a green run, which is how a real warning stops being read. `logger` is the driver's own documented injection point; nothing about its behaviour changes and the messages stay available to a failure report. TWO SENTENCES THAT WERE STILL WRONG * The pin still said "`packages/cli` does not depend on the driver at runtime, so the ceiling is transcribed in generate.ts" -- verbatim the reason this branch already established as false, that `generate.ts` carries with a ban, and that the same test file contradicts 500 lines earlier. Replaced with the real reasons: `MAX_VARCHAR_CHARS` is `protected static` and reaches no exported surface, and #5726 leaves a CLI production module only `await import()`, which these synchronous generators cannot use. * `generate.ts`'s `isUniqueScopeDeclared` docblock restated a stale driver comment as present fact. Measured against the built spec, `isUniqueDeclared('organization')` is already `true`, so the disjunct is redundant today and both halves are spec's. The disjunct stays -- it is the driver's spelling and the mirror matches it character for character -- but it is no longer described as a scope spec does not accept. The changeset said the generators invented `2048 / 50 / 7`. Only the SQL format did; the TypeScript format emitted a bare `table.string(name)` for all three. Release-notes input, so it is corrected. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- ...rated-migration-character-column-widths.md | 2 +- .../generate-string-family-width.pin.test.ts | 433 ++++++++++++++++-- packages/cli/src/commands/generate.ts | 13 +- 3 files changed, 406 insertions(+), 42 deletions(-) diff --git a/.changeset/generated-migration-character-column-widths.md b/.changeset/generated-migration-character-column-widths.md index 89e9577b91..b8c3f686d8 100644 --- a/.changeset/generated-migration-character-column-widths.md +++ b/.changeset/generated-migration-character-column-widths.md @@ -4,7 +4,7 @@ `os generate migration` now emits the character column `driver-sql` actually creates, in both the TypeScript and the SQL format. -A `text` field took `VARCHAR(255)` from both generators while the platform creates an unbounded `text` column for it, so a 300-character value the platform stores was refused by every generated table with `value too long for type character varying(255)`. Enumerating the whole character-column family found the same disagreement in eight more places: `url` and `phone` and `color` carried widths the generators invented (2048, 50 and 7 against the platform's 255), and neither generator read a field's declared `maxLength` at all, so a `maxLength: 400` email was `varchar(400)` on the platform and `varchar(255)` in the migration generated for it. +A `text` field took `VARCHAR(255)` from both generators while the platform creates an unbounded `text` column for it, so a 300-character value the platform stores was refused by every generated table with `value too long for type character varying(255)`. Enumerating the whole character-column family found the same disagreement in eight more places: the SQL format gave `url` and `phone` and `color` widths nothing on the platform has (2048, 50 and 7 against the platform's 255), and neither format read a field's declared `maxLength` at all, so a `maxLength: 400` email was `varchar(400)` on the platform and `varchar(255)` in the migration generated for it. All of them now follow the platform's own three answers: the text family is unbounded unless the object KEYS the column — a field declared `unique`, or one an object-level `indexes[]` entry lists, takes `varchar(maxLength)` up to the 768-character key-part ceiling, exactly as the platform builds it, and stays unbounded above that ceiling or with no declared bound, where the declared bound is enforced at the write seam instead — the string family takes its declared `maxLength` verbatim in both directions, and TEXT rather than a clamp when it exceeds what a `varchar` can express, and the remaining string-valued types keep the default width and ignore a declaration, because their stored value is an option code or another row's id rather than the declared string. diff --git a/packages/cli/src/commands/generate-string-family-width.pin.test.ts b/packages/cli/src/commands/generate-string-family-width.pin.test.ts index fe9fcf5a0d..7479e2747b 100644 --- a/packages/cli/src/commands/generate-string-family-width.pin.test.ts +++ b/packages/cli/src/commands/generate-string-family-width.pin.test.ts @@ -148,10 +148,28 @@ * its exported `uniqueIndexesFromFields` / `normalizeDeclaredIndex`, and its * own `protected` judgments through a subclass — and compares, over a swept * corpus rather than a hand-listed one. That differential is what found the - * `nullSafeColumns` branch this round repaired: `{ fields: ['f'], unique: + * `nullSafeColumns` branch an earlier round repaired: `{ fields: ['f'], unique: * 'organization', nullSafeColumns: ['zzz'] }` keyed `{organization_id, f}` here * against the driver's `{f}`, and no enumerated case had reached the shape. * + * ## ⭐ ...and asking the driver's LEAVES is still not asking the driver + * + * Recomposing the leaves' answers HERE leaves every layer between them and the + * emitted column a second copy of this file's own belief. Measured, driver-side: + * making `indexedKeyColumns` stop recording declared indexes, and making + * `initObjects` hand it `tenantField: null`, each left the platform disagreeing + * with both generators over hundreds of objects — and left this file green, + * every test, both times. Those two are this card's own subject. + * + * So the authority in this file is neither the source reader nor the leaf + * differential: it is `SqlDriver.initObjects` on an in-memory better-sqlite3 + * database, read back with `PRAGMA table_info`. That runs + * `computeAndRecordTenantField` → `indexedKeyColumns` → `createColumn` → knex + * and reports the column that actually exists, with nothing re-derived here. + * The two cheaper layers are kept underneath it because they localise a failure + * to one constant or one builder; where they and the real chain could disagree, + * ⛔ the real chain is right. + * * ⚠️ Scope, as `generateMigrationSql`'s docblock and the `--format` help text * already say (#15521): this is a POSTGRESQL claim and nothing else. Neither * generator reproduces the driver's dialect branching. @@ -262,6 +280,26 @@ function armMembers(type: string): string[] { .filter((t) => REAL_FIELD_TYPES.has(t)); } +/** + * The CHARACTER half of `createColumn`'s catch-all, derived rather than listed. + * + * The catch-all routes on `JSON_COLUMN_TYPES`, which `driver-sql` seeds from + * three spec classes — so the character half is every real `FieldType` the + * switch does not case, minus those three classes, imported and never listed + * here. ⛔ Never derived from what the generator already answers: a filter that + * skips a member whose answer has already drifted measures its own claim only + * where the claim already holds. + */ +function characterCatchAllMembers(): string[] { + const cased = new Set([...createColumnSwitch().matchAll(/case '([^']+)':/g)].map((m) => m[1])); + const jsonSeeded = new Set([ + ...MULTI_OPTION_TYPES, + ...STRUCTURED_JSON_TYPES, + ...FILE_REFERENCE_TYPES, + ]); + return [...REAL_FIELD_TYPES].filter((t) => !cased.has(t) && !jsonSeeded.has(t)); +} + /** `createColumn`'s catch-all — where an un-cased type lands. */ function createColumnDefaultArm(): string { const body = createColumnSwitch(); @@ -689,10 +727,16 @@ describe('#16091 — the character column both generators emit is the driver\'s' }); it('the generator\'s varchar ceiling is the driver\'s, not a second number', () => { - // `packages/cli` does not depend on the driver at runtime, so the ceiling is - // transcribed in generate.ts. This is the assertion that makes the - // transcription safe: the two must be the same number, and a driver that - // moves fails here rather than leaving the generators quietly wrong. + // ⛔ NOT "because `packages/cli` does not depend on the driver at runtime" + // — it does, and this file imports it 500 lines up. That sentence was this + // pin's own first answer, it is false, and `generate.ts` now carries it + // with a ⛔ so nobody restates it. The ceiling is transcribed because + // `MAX_VARCHAR_CHARS` is `protected static` on `SqlDriver` and reaches no + // exported surface, and because #5726 leaves a CLI production module only + // `await import()` for a driver package — which these SYNCHRONOUS + // generators cannot use. This is the assertion that makes the transcription + // safe: the two must be the same number, and a driver that moves fails here + // rather than leaving the generators quietly wrong. const m = GENERATE_SOURCE.match(/^const MAX_VARCHAR_CHARS = (\d+);$/m); expect(m, 'generate.ts no longer declares MAX_VARCHAR_CHARS at top level').not.toBeNull(); expect(Number(m![1])).toBe(MAX_CHARS); @@ -726,7 +770,8 @@ describe('#16091 — the character column both generators emit is the driver\'s' ...STRUCTURED_JSON_TYPES, ...FILE_REFERENCE_TYPES, ]); - const characterCatchAll = catchAll.filter((t) => !jsonSeeded.has(t)); + const characterCatchAll = characterCatchAllMembers(); + expect(characterCatchAll).toEqual(catchAll.filter((t) => !jsonSeeded.has(t))); // Controls for the derivation itself. Without these a seed set that failed // to import would leave `characterCatchAll` as the whole catch-all (and the @@ -849,6 +894,28 @@ describe('#16091 — the character column both generators emit is the driver\'s' // Mutating those two in the driver left this file GREEN. What follows removes // that: both are RECOMPUTED from `driver-sql` itself and compared against what // the generators actually emit. +// +// ⭐ ASKING THE DRIVER'S LEAVES IS NOT ASKING THE DRIVER, and this file has now +// made that mistake twice. Round 2 transcribed the driver's answers and +// mutating the driver left every pin green. Round 3 asked the driver's exported +// LEAVES — `uniqueIndexesFromFields`, `normalizeDeclaredIndex`, +// `computeTenantField` — and then RE-COMPOSED them here, which left every layer +// between those leaves and the emitted column a second copy of this file's own +// belief. Measured, driver-side, at that head: +// +// driver-side mutation this file +// `indexedKeyColumns` stops recording declared indexes 78 passed (78) +// `initObjects` passes `tenantField: null` into it 78 passed (78) +// +// Both are this card's own subject — the driver changes what it keys, the +// generated column stays bounded where the platform's is unbounded — and the +// instrument reported everything fine. So the differential below enters the +// REAL CHAIN at the top: `SqlDriver.initObjects` on an in-memory better-sqlite3 +// database, read back with `PRAGMA table_info`. That is +// `computeAndRecordTenantField` → `indexedKeyColumns` → `createColumn` → +// knex → an actual column, with nothing re-derived here at all. The leaf +// differential is KEPT below it, because it localises a failure to one builder; +// it is not the authority, and where the two could disagree the real chain wins. /** * The driver's own `protected` judgments, reached by widening rather than @@ -860,6 +927,25 @@ describe('#16091 — the character column both generators emit is the driver\'s' * with it — which is exactly what the transcriptions in `generate.ts` cannot do. */ class DriverOracle extends SqlDriver { + /** + * Every warning the driver emitted, captured instead of printed. + * + * The corpus deliberately contains index shapes whose key parts name no + * materialized column (`idx-ghost`, and every `organization`-scoped index on + * the `without-organization_id` shape), and the driver correctly says so + * once per table — 144 lines of true, irrelevant warning on a green run, + * which is how a real one stops being read. ⛔ Captured, never discarded: + * `logger` is the driver's own documented injection point ("production + * callers wire in their preferred logger"), the messages stay available to a + * failure report, and nothing about the driver's behaviour changes. + */ + public readonly warnings: string[] = []; + + protected override logger = { + warn: (msg: string) => { this.warnings.push(msg); }, + error: (msg: string) => { this.warnings.push(msg); }, + }; + /** `SqlDriver.computeTenantField`, unmodified. */ public tenantFieldFor(object: { fields?: Record; tenancy?: unknown }): string | null { return this.computeTenantField(object); @@ -874,6 +960,35 @@ class DriverOracle extends SqlDriver { public declaredCharsFor(field: unknown): number | null { return this.declaredVarcharLength(field); } + + /** + * ⭐ THE REAL CHAIN. The columns `initObjects` actually creates for one + * object, read back out of the database it created them in. + * + * Nothing here re-derives anything: `initObjects` resolves the tenant field + * through `computeAndRecordTenantField`, composes the key set through + * `indexedKeyColumns`, dispatches every field through `createColumn` and + * hands the result to knex. `PRAGMA table_info` then reports the column type + * SQLite recorded — `text` for `table.text(name)`, `varchar(n)` for + * `table.string(name, n)`, `varchar(255)` for a bare `table.string(name)`. + * A driver-side change anywhere on that path moves this answer, which is the + * property the leaf-level differential below does not have. + * + * ⚠️ Each object must carry a table name no earlier call used: `initObjects` + * takes the ALTER path on a table that already exists, and an ALTER cannot + * retype a column — a reused name would silently report the first object's + * answer for the second one's declaration. {@link keyProbeCorpus} and the + * width sweeps mint one name per probe for exactly that reason. + */ + public async createdColumns(object: { name: string; fields?: Record; tenancy?: any }): + Promise> { + await this.initObjects([object]); + const rows = (await this.knex.raw(`PRAGMA table_info("${object.name}")`)) as Array<{ + name: string; + type: string; + }>; + return new Map(rows.map((row) => [row.name, row.type])); + } } // The same in-memory shape every SqlDriver test in this repo constructs. No @@ -894,12 +1009,14 @@ afterAll(async () => { * `schema-drift.ts`'s `indexedKeyColumns`, composed HERE from the driver's own * two exported builders. * - * That function is not itself exported, but the two normalizers it composes are - * (`index.ts`), and composing them is the whole of its body: it records every - * column of every index those two return. So this recomputes the driver's key - * set without transcribing a single one of its judgments — the scope - * vocabulary, the tenant prepend, the `nullSafeColumns` arm and the tenant - * column itself all arrive from `driver-sql`. + * ⛔ NOT the authority, and it must never be read as one. The two normalizers + * are the driver's, but the COMPOSITION is this file's — so a driver that + * changes how `indexedKeyColumns` composes them, or what `initObjects` feeds + * it, moves the platform without moving this function at all. Both of those + * were mutated in the driver and this differential stayed green through both. + * {@link DriverOracle.createdColumns} is the authority; this is kept only + * because it localises a failure to ONE builder, which the real chain cannot + * do — when the two disagree, the real chain is right and this is stale. */ function driverKeyColumns(object: Record): Set { const table = String(object.name); @@ -938,13 +1055,36 @@ function sqlWidth(column: string | null): number | null { * producers would look like they disagreed on every undeclared string field, * where they in fact emit the same column two ways. */ -function tsWidth(call: string | null): number | null { - if (call === "table.text('f')") return null; - const m = call?.match(/^table\.string\('f'(?:, (\d+))?\)$/); +function tsWidth(call: string | null, field = 'f'): number | null { + if (call === `table.text('${field}')`) return null; + const m = call?.match(new RegExp(`^table\\.string\\('${field}'(?:, (\\d+))?\\)$`)); if (!m) throw new Error(`not a character column: ${String(call)}`); return m[1] === undefined ? DEFAULT_CHARS : Number(m[1]); } +/** + * The width the PLATFORM'S OWN column declares, in the same units. + * + * `PRAGMA table_info` reports the type knex asked SQLite for, so + * `table.text(name)` reads back as `text` (unbounded, `null` here) and + * `table.string(name, n)` as `varchar(n)`. A bare `table.string(name)` is + * `varchar(255)` — knex's default, which is {@link DEFAULT_CHARS} read off the + * driver's own constant, so the two producers normalize to the same number + * rather than to "declared" versus "not declared". + * + * Anything else throws: a non-character column is outside this card, and + * counting it as unbounded would make a JSON column look like agreement. + */ +function driverWidth(columnType: string | undefined, where: string): number | null { + if (columnType === undefined) { + throw new Error(`${where}: initObjects created no such column — the probe never reached the driver`); + } + if (/^text$/i.test(columnType)) return null; + const m = columnType.match(/^varchar\((\d+)\)$/i); + if (!m) throw new Error(`${where}: not a character column: ${columnType}`); + return Number(m[1]); +} + /** * The width every probe field in the key-set corpus declares. * @@ -961,6 +1101,21 @@ interface KeyProbe { object: Record; } +/** + * The corpus's four dimensions, and the number of probes their product is. + * + * ⭐ Stated as a literal on purpose. Round 3 hand-counted this sweep, wrote + * "1,224 objects over 17 index shapes" into the PR body AND into a commit + * message the merge queue composes into the squash body, and nothing caught it + * — the only size assertion in this file was `> 200`, which every wrong count + * satisfies. A number a human derived by reading array literals is a + * measurement like any other and needs an instrument. Adding an index shape + * moves this literal; a shape added without moving it fails here rather than + * landing a false count in `main`. + */ +const KEY_PROBE_DIMENSIONS = { uniques: 6, indexSets: 16, tenancies: 6, shapes: 2 } as const; +const KEY_PROBE_COUNT = 1152; + /** * The swept corpus: every combination of a field-level `unique` spelling, an * object-level `indexes[]` entry, a `tenancy` declaration and a column shape. @@ -1017,6 +1172,16 @@ function keyProbeCorpus(): KeyProbe[] { ['without-organization_id', ['f', 'other', 'org']], ]; + // The dimensions this corpus actually has, measured here rather than + // hand-counted, so {@link KEY_PROBE_DIMENSIONS} is an assertion about the + // arrays above and not a second transcription of them. + expect({ + uniques: uniques.length, + indexSets: indexSets.length, + tenancies: tenancies.length, + shapes: shapes.length, + }).toEqual(KEY_PROBE_DIMENSIONS); + const probes: KeyProbe[] = []; for (const [shapeId, columns] of shapes) { for (const [uniqueId, unique] of uniques) { @@ -1030,7 +1195,11 @@ function keyProbeCorpus(): KeyProbe[] { probes.push({ id: `${shapeId}/${uniqueId}/${indexId}/${tenancyId}`, object: { - name: 'probe', + // One table name per probe. The real-chain oracle CREATES this + // table, and `initObjects` takes the ALTER path on a name it has + // already seen — an ALTER cannot retype a column, so a shared + // name would report the first probe's answer for all 1,152. + name: `probe_${probes.length}`, fields, ...(indexes ? { indexes } : {}), ...(tenancy ? { tenancy } : {}), @@ -1057,27 +1226,41 @@ function keyProbeCorpus(): KeyProbe[] { * generator and not the other would otherwise pass here. */ function generatorKeyColumns(object: Record, id: string): Set { - const config = { objects: { probe: object } } as Record; + const keyed = new Set(); + for (const [field, chars] of generatorWidths(object)) { + if (chars === PROBE_CHARS) keyed.add(field); + else if (chars !== null) { + throw new Error(`${id}: '${field}' is neither the keyed nor the unkeyed answer: ${String(chars)}`); + } + } + return keyed; +} + +/** + * Both generators' width for every declared field of one object, in the + * driver's own units, asserted to agree with each other on the way past. + * + * A mirror consulted by one generator and not the other would otherwise pass + * every differential in this file: the two formats are separate code paths over + * the same helpers, and #16091's own table had them disagreeing on five rows. + */ +function generatorWidths(object: Record): Map { + const config = { objects: { [String(object.name)]: object } } as Record; const sql = generateMigrationSql(config); const ts = generateMigrationTs(config); - const keyed = new Set(); - for (const field of Object.keys(object.fields)) { + const out = new Map(); + for (const field of Object.keys(object.fields ?? {})) { const sqlCol = sqlColumn(sql, field); const tsCol = tsColumn(ts, field); const sqlChars = sqlWidth(sqlCol); - const tsChars = tsCol === `table.text('${field}')` - ? null - : Number(tsCol?.match(/^table\.string\('[^']+'(?:, (\d+))?\)$/)?.[1] ?? DEFAULT_CHARS); expect( - tsChars, - `${id}: the two formats disagree about '${field}' — sql ${String(sqlCol)}, ts ${String(tsCol)}`, + tsWidth(tsCol, field), + `${String(object.name)}: the two formats disagree about '${field}' — ` + + `sql ${String(sqlCol)}, ts ${String(tsCol)}`, ).toBe(sqlChars); - if (sqlChars === PROBE_CHARS) keyed.add(field); - else if (sqlChars !== null) { - throw new Error(`${id}: '${field}' is neither the keyed nor the unkeyed answer: ${String(sqlCol)}`); - } + out.set(field, sqlChars); } - return keyed; + return out; } describe('#16091 — the driver is the ORACLE, not just the source text', () => { @@ -1116,6 +1299,56 @@ describe('#16091 — the driver is the ORACLE, not just the source text', () => expect(tsWidth("table.string('f', 100)")).toBe(100); expect(tsWidth("table.string('f')")).toBe(DEFAULT_CHARS); expect(() => tsWidth("table.jsonb('f')")).toThrow(); + expect(driverWidth('text', 'control')).toBeNull(); + expect(driverWidth('varchar(100)', 'control')).toBe(100); + expect(() => driverWidth('json', 'control')).toThrow(); + expect(() => driverWidth(undefined, 'control')).toThrow(); + }); + + it('control — the REAL CHAIN really runs, and it really discriminates', async () => { + // ⭐ Non-vacuity for `initObjects` itself, and it is the load-bearing + // control of this file: every differential below reads columns out of a + // database, so a chain that silently created nothing would compare an empty + // map against an empty map and pass. + const created = await ORACLE.createdColumns({ + name: 'control_real_chain', + fields: { + keyed: { type: 'text', maxLength: PROBE_CHARS, unique: true }, + unkeyed: { type: 'text', maxLength: PROBE_CHARS }, + organization_id: { type: 'text', maxLength: PROBE_CHARS }, + sized: { type: 'email', maxLength: 400 }, + plain: { type: 'color' }, + }, + }); + + // The table exists and carries the driver's own builtins, which no field + // here declares — proof the CREATE really ran rather than the map being + // assembled from the declaration. + expect(created.has('id')).toBe(true); + expect(created.has('created_at')).toBe(true); + + // All three arms answer, and they answer three DIFFERENT things. A chain + // that returned one column shape for everything dies here. + expect(driverWidth(created.get('keyed'), 'keyed')).toBe(PROBE_CHARS); + expect(driverWidth(created.get('unkeyed'), 'unkeyed')).toBeNull(); + expect(driverWidth(created.get('sized'), 'sized')).toBe(400); + expect(driverWidth(created.get('plain'), 'plain')).toBe(DEFAULT_CHARS); + + // ...and the TENANT column is keyed by the field-level `unique: true`, + // which is `computeAndRecordTenantField` and `indexedKeyColumns` and + // `createColumn` all firing on the real path. This is the exact column the + // leaf-composed differential could not see move. + expect(driverWidth(created.get('organization_id'), 'organization_id')).toBe(PROBE_CHARS); + + // The same object without the organization column leaves the field's own + // key alone — so the assertion above measures the tenant PREPEND rather + // than "everything is keyed". + const noTenant = await ORACLE.createdColumns({ + name: 'control_real_chain_no_tenant', + fields: { keyed: { type: 'text', maxLength: PROBE_CHARS, unique: true } }, + }); + expect(driverWidth(noTenant.get('keyed'), 'keyed')).toBe(PROBE_CHARS); + expect(noTenant.has('organization_id')).toBe(false); }); // ── The unique VOCABULARY, against the driver's own two predicates ──────── @@ -1178,13 +1411,24 @@ describe('#16091 — the driver is the ORACLE, not just the source text', () => expect(UNIQUE_SPELLINGS.some((u) => isUniqueScopeDeclared(u) && !isOrganizationScopedUnique(u))).toBe(true); }); - // ── F1: the key set, recomputed from the driver's own builders ──────────── + // ── F1a: the key set, recomputed from the driver's own exported builders ── + // + // Kept because it localises a failure to ONE builder. ⛔ Not the authority — + // F1b below is. Read the two together: this one says WHICH builder moved, + // that one says whether the platform's column moved at all. it('the corpus is a real sweep, and both sides really vary across it', () => { const corpus = keyProbeCorpus(); + // ⭐ The EXACT size, not `> 200`. `> 200` is what this case used to assert, + // and it is why a hand-count of "1,224 over 17 index shapes" reached a + // commit message unchallenged. The product of the four dimensions and the + // length of what the sweep actually built must both equal the stated + // number — a corpus that grows without this literal growing fails here. + const { uniques, indexSets, tenancies, shapes } = KEY_PROBE_DIMENSIONS; + expect(uniques * indexSets * tenancies * shapes).toBe(KEY_PROBE_COUNT); + expect(corpus.length).toBe(KEY_PROBE_COUNT); // A differential over a corpus that answers one thing everywhere proves - // nothing, so the corpus's own discriminating power is asserted first. - expect(corpus.length).toBeGreaterThan(200); + // nothing, so the corpus's own discriminating power is asserted next. const driverAnswers = new Set(corpus.map((p) => [...driverKeyColumns(p.object)].sort().join(','))); expect(driverAnswers.size).toBeGreaterThan(4); expect(driverAnswers.has('')).toBe(true); @@ -1220,8 +1464,50 @@ describe('#16091 — the driver is the ORACLE, not just the source text', () => ).toEqual([]); }); - it('the pre-normalized index arm keys exactly what the driver keys, by name', () => { - // ⭐ The divergence this round repaired, spelled out so it cannot come back + // ── F1b: THE AUTHORITY — the column `initObjects` actually created ──────── + // + // ⭐ Everything above this line asks the driver's exported parts and puts the + // answer together HERE. This asks `SqlDriver.initObjects` for a table and + // reads the column out of it. The difference is not stylistic: the two + // driver-side mutations that ARE this card's subject — `indexedKeyColumns` + // dropping declared indexes, and `initObjects` handing it `tenantField: null` + // — are invisible to a re-composition and unmissable here. + + it('every character column the generators emit is the column initObjects CREATES', async () => { + const divergences: string[] = []; + let columnsCompared = 0; + for (const { id, object } of keyProbeCorpus()) { + // The platform's own answer: one CREATE TABLE through the whole chain, + // read back out of the database. Nothing about it is derived here. + const created = await ORACLE.createdColumns(object as { name: string; fields: Record }); + const emitted = generatorWidths(object); + for (const [field, generated] of emitted) { + const platform = driverWidth(created.get(field), `${id}: '${field}'`); + columnsCompared += 1; + if (platform !== generated) { + divergences.push( + `${id}: '${field}' driver=${platform === null ? 'text' : `varchar(${platform})`} ` + + `generated=${generated === null ? 'text' : `varchar(${generated})`}`, + ); + } + } + } + // Non-vacuity, stated as an exact count: a loop that compared nothing — + // or one whose `createdColumns` quietly returned an empty map — reports no + // divergences and passes. 4,032 = the `with-organization_id` half's four + // declared columns (576 × 4) plus the other half's three (576 × 3). + expect(columnsCompared).toBe(4_032); + expect( + divergences.slice(0, 20), + `${divergences.length} of ${columnsCompared} columns disagree with the column driver-sql ` + + 'actually created for the same object. Every entry is #16091 itself: a value the platform ' + + 'stores that a generated table refuses, or one it invites that the platform refuses. The ' + + 'authority is the DRIVER — fix generate.ts, never this expectation.', + ).toEqual([]); + }, 60_000); + + it('the pre-normalized index arm keys exactly what initObjects keys, by name', async () => { + // ⭐ The divergence round 3 repaired, spelled out so it cannot come back // unnoticed inside a sweep. `normalizeDeclaredIndex` filters // `nullSafeColumns` against the listed columns, but that filter narrows // only `nullSafeColumns` — its `columns` are the listed ones in every @@ -1232,25 +1518,38 @@ describe('#16091 — the driver is the ORACLE, not just the source text', () => organization_id: { type: 'text', maxLength: PROBE_CHARS }, }; const stranger = { - name: 'probe', + name: 'pre_normalized_stranger', fields, indexes: [{ fields: ['f'], unique: 'organization', nullSafeColumns: ['zzz'] }], }; + // Asserted through the REAL CHAIN first — the platform's own table — and + // through the exported builders second, so the two are pinned to agree. + const strangerColumns = await ORACLE.createdColumns(stranger); + expect(driverWidth(strangerColumns.get('f'), 'stranger.f')).toBe(PROBE_CHARS); + expect(driverWidth(strangerColumns.get('organization_id'), 'stranger.org')).toBeNull(); expect([...driverKeyColumns(stranger)].sort()).toEqual(['f']); expect([...generatorKeyColumns(stranger, 'pre-normalized-stranger')].sort()).toEqual(['f']); // The counter-case, which is what makes the one above a measurement: the // SAME index without `nullSafeColumns` does prepend the tenant column. const prepending = { - name: 'probe', + name: 'pre_normalized_prepending', fields, indexes: [{ fields: ['f'], unique: 'organization' }], }; + const prependingColumns = await ORACLE.createdColumns(prepending); + expect(driverWidth(prependingColumns.get('f'), 'prepending.f')).toBe(PROBE_CHARS); + expect(driverWidth(prependingColumns.get('organization_id'), 'prepending.org')).toBe(PROBE_CHARS); expect([...driverKeyColumns(prepending)].sort()).toEqual(['f', 'organization_id']); expect([...generatorKeyColumns(prepending, 'idx-org')].sort()).toEqual(['f', 'organization_id']); }); // ── F2: the two width bodies, recomputed from the driver's own methods ──── + // + // Same two layers, same order: the leaf differentials localise a failure to + // one method body, and the real-chain sweep at the end of this section is the + // authority — it is the one that also covers `createColumn`'s DISPATCH onto + // those bodies, which asking `keyableTextLength` directly cannot see move. /** * The declarations both width sweeps run. Deliberately wider than anything an @@ -1279,8 +1578,14 @@ describe('#16091 — the driver is the ORACLE, not just the source text', () => ).toBe(chars); expect(tsWidth(ts), `keyed text @ maxLength ${shown}, typescript format`).toBe(chars); } + // ⭐ How many declarations this sweep really carries, stated as a literal + // for the same reason {@link KEY_PROBE_COUNT} is: round 3 hand-counted this + // array as "37 declarations" and wrote that into a commit message the merge + // queue composes into the squash body. It is 38. A hand-count is a + // measurement and needs an instrument. + expect(WIDTH_DECLARATIONS).toHaveLength(38); // Non-vacuity: the sweep really produced both dispositions and more than - // one width, so it is not one answer asserted 37 times. + // one width, so it is not one answer asserted 38 times. expect(answers.has(null)).toBe(true); expect([...answers].filter((a) => a !== null).length).toBeGreaterThan(3); }); @@ -1315,4 +1620,58 @@ describe('#16091 — the driver is the ORACLE, not just the source text', () => expect(sqlWidth(columnsFor({ type: 'text', unique: true, maxLength: 1000 }).sql)).toBeNull(); expect(sqlWidth(columnsFor({ type: 'email', maxLength: 1000 }).sql)).toBe(1000); }); + + it('every character TYPE, at every declaration, takes the width initObjects CREATES', async () => { + // ⭐ The width half of F1b, and it reaches one layer the two cases above + // cannot: `createColumn`'s DISPATCH. `ORACLE.keyableCharsFor(...)` answers + // what `keyableTextLength` returns; it says nothing about which arm + // `createColumn` hands the field to, or on what it branches when it gets + // there. A driver that started sizing only the UNIQUE key parts, say, moves + // the platform's column while `keyableTextLength` answers exactly as before. + // + // MEMBERSHIP is the driver's, read off its own case labels and its own + // catch-all derivation, so a type joining or leaving a family moves what is + // swept here with nobody editing this file. + const types = [...armMembers('text'), ...armMembers('email'), ...characterCatchAllMembers()]; + expect(new Set(types).size, 'a type is in two families at once').toBe(types.length); + expect(types.length).toBeGreaterThanOrEqual(15); + + const divergences: string[] = []; + const answers = new Set(); + let compared = 0; + let probe = 0; + for (const type of types) { + for (const keyed of [false, true]) { + for (const maxLength of WIDTH_DECLARATIONS) { + const field: Record = { type }; + if (maxLength !== undefined) field.maxLength = maxLength; + if (keyed) field.unique = true; + const object = { name: `width_${probe++}`, fields: { f: field } }; + const created = await ORACLE.createdColumns(object); + const shown = `${type}${keyed ? ' unique:true' : ''} @ ${JSON.stringify(maxLength) ?? 'undefined'}`; + const platform = driverWidth(created.get('f'), shown); + const generated = generatorWidths(object).get('f') ?? null; + answers.add(`${platform}`); + compared += 1; + if (platform !== generated) { + divergences.push( + `${shown}: driver=${platform === null ? 'text' : `varchar(${platform})`} ` + + `generated=${generated === null ? 'text' : `varchar(${generated})`}`, + ); + } + } + } + } + // Non-vacuity: the sweep really ran, and the platform really gave more than + // one answer across it — a chain answering `text` everywhere would make + // every comparison above agree with a generator that did the same. + expect(compared).toBe(types.length * 2 * WIDTH_DECLARATIONS.length); + expect(answers.has('null')).toBe(true); + expect(answers.size).toBeGreaterThan(3); + expect( + divergences.slice(0, 20), + `${divergences.length} of ${compared} probes take a width the platform's own column does ` + + 'not have. The authority is driver-sql: fix generate.ts, never this expectation.', + ).toEqual([]); + }, 60_000); }); diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 9be9f18dd2..7cd36a919b 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -1319,10 +1319,15 @@ function declaredVarchar(maxLength: unknown): VarcharAnswer { * vocabulary. * * Spelled as the driver spells it, over the SAME spec predicate the driver - * calls: `unique === 'organization' || isUniqueDeclared(unique)`. The word is - * accepted here ahead of the spec helper deliberately (ADR-0120 D1, driver - * first), so the disjunct is the driver's, not this file's invention — and the - * half that IS spec's is imported rather than retyped. + * calls: `unique === 'organization' || isUniqueDeclared(unique)`. The disjunct + * is the driver's SPELLING, kept so the mirror matches it character for + * character — ⛔ not a scope this predicate adds on top of spec's. Measured + * against the built spec, `isUniqueDeclared('organization')` is already `true` + * (`packages/spec/src/data/field.zod.ts` lists all three spellings), so the + * disjunct is redundant today and BOTH halves are spec's. The driver's own + * comment still reads as though the word were accepted ahead of the spec + * helper (ADR-0120 D1, driver first); that was true when it was written, it is + * not now, and a mirror must not restate it in the present tense. */ function isUniqueScopeDeclared(unique: unknown): boolean { return unique === 'organization' || isUniqueDeclared(unique);