diff --git a/.changeset/17883-generate-migration-file-family-width.md b/.changeset/17883-generate-migration-file-family-width.md new file mode 100644 index 0000000000..f8ffcb8d95 --- /dev/null +++ b/.changeset/17883-generate-migration-file-family-width.md @@ -0,0 +1,16 @@ +--- +"@objectstack/cli": patch +--- + +`os generate migration` gives the file family — `file` / `image` / `avatar` / `video` / `audio` — the **same column width in both formats**. The typescript format emitted a bare `table.string(name)`, knex's `varchar(255)`, while `--format sql` emitted `VARCHAR(2048)` for the same field, so one command answered one field with two widths depending on the flag (#17883). + +2048 is not a new number: ADR-0104 ruled the generator's `VARCHAR(2048)` the end-state for this family, `driver-sql` moved to it (`MEDIA_ID_VARCHAR_CHARS`, #15989), and `os migrate files-to-references --apply` retypes the column to `varchar(2048)`. The typescript format was the one producer left at 255 — so a deployment scaffolded from it declared a width the migration it will later run retypes away from. + +```diff +- table.string('cover_image').nullable(); ++ table.string('cover_image', 2048).nullable(); +``` + +- **No regeneration is required of anyone.** `syncSchema` / `initObjects` are additive and never alter an existing column's type, and a `sys_file` id is far shorter than 255, so nothing stored today is at risk either way. What moves is the **declared** width of tables generated from now on. +- **The width is now read from the sql format's own entry** instead of being retyped beside it, so the two formats cannot drift apart again; `generate-file-reference-width.pin.test.ts` measures both against `driver-sql`'s constant, which is what stops the two halves from "meeting in the middle" at some third value. +- ⛔ **Nothing outside the family moved.** The `text` family, the reference types the file family used to share an arm with (`lookup` / `master_detail` / `user` / `tree`), `autonumber`, and every `--format sql` answer are byte-identical. 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 c0cad70440..9da764822b 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 @@ -93,12 +93,14 @@ * `FIELD_TYPE_MAP` and a scalar column in `FIELD_TYPE_SQL_MAP` is a * contradiction whoever is right, and `autonumber` was exactly that. * - * ⚠️ Still NOT asserted, deliberately: the FILE_REFERENCE_TYPES family. Those - * five ARE in the driver's `JSON_COLUMN_TYPES` today while this generator gives - * them a varchar — but that is #14657's ADR-0104 D3 answer against a driver - * that is still pre-D3, i.e. a decision about which side moves, not a wrong - * value to correct. It is recorded below as a measured divergence so it cannot - * be mistaken for coverage, and filed rather than fixed here. + * ⚰️ The FILE_REFERENCE_TYPES family used to be excluded from all of this, + * deliberately: those five were in the driver's `JSON_COLUMN_TYPES` while this + * generator gave them a varchar — #14657's ADR-0104 D3 answer against a driver + * that was still pre-D3, i.e. a decision about which side moves rather than a + * wrong value to correct. The ruling on #15041 decided it and #15989 landed it, + * so the exclusion below became coverage. [#17883] The width the two migration + * formats emit for the family is a second question and has a pin of its own — + * `generate-file-reference-width.pin.test.ts`. * * The runtime fallbacks (`|| 'unknown'`, `|| 'TEXT'`, `default:`) stay and are * NOT dead: they answer a `type` string that is not a `FieldType` at all, which @@ -375,9 +377,18 @@ function sqlColumn(type: string): string | null { return m ? m[1] : null; } -/** The `table.x('f_type')` call one field type contributes, or `null` for none. */ +/** + * The `table.x('f_type'[, …])` call one field type contributes, or `null` for + * none. + * + * [#17883] The argument list is part of the call and is captured with it. The + * reader used to stop at the name, so a SIZED call — `table.string(name, 2048)`, + * which is what the file family takes since #17883 — read as no column at all, + * and every assertion below would have been answering a question about absence + * instead of one about the column. + */ function tsColumn(type: string): string | null { - const m = TS_OUT.match(new RegExp(`^ {4}(table\\.\\w+\\('f_${type}'\\)).*$`, 'm')); + const m = TS_OUT.match(new RegExp(`^ {4}(table\\.\\w+\\('f_${type}'(?:, [^)]*)?\\)).*$`, 'm')); return m ? m[1] : null; } @@ -669,7 +680,12 @@ describe('#14828 — the SQL answers are the platform’s, not this file’s inv for (const type of FILE_REFERENCE_TYPES) { expect(sqlColumn(type)).toBe('VARCHAR(2048)'); - expect(tsColumn(type)).toBe(`table.string('f_${type}')`); + // [#17883] Was a bare `table.string('f_type')` — knex's varchar(255), + // which left THIS generator's two formats disagreeing about the family + // after the driver had already moved to 2048. The width agreement itself + // is `generate-file-reference-width.pin.test.ts`; what stays here is the + // spelling, so the two pins fail in different sentences. + expect(tsColumn(type)).toBe(`table.string('f_${type}', 2048)`); } }); }); diff --git a/packages/cli/src/commands/generate-file-reference-width.pin.test.ts b/packages/cli/src/commands/generate-file-reference-width.pin.test.ts new file mode 100644 index 0000000000..f94e979bfe --- /dev/null +++ b/packages/cli/src/commands/generate-file-reference-width.pin.test.ts @@ -0,0 +1,195 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * THE #17883 PIN: one `os generate migration`, one column width for the file + * family — whichever `--format` the author picked. + * + * ## The defect + * + * `generate.ts` carries two migration formats, and they answered the same + * FILE_REFERENCE_TYPES field with two different widths: + * + * ``` + * --format sql f_file VARCHAR(2048) FIELD_TYPE_SQL_MAP + * typescript f_file table.string('f_file') knex's varchar(255) + * ``` + * + * `generate.ts` states that second equality itself, in as many words on the + * `autonumber` entry: "`table.string(name)` = knex's `varchar(255)` + * (`DEFAULT_STRING_VARCHAR_CHARS`)". The typescript half reached it by riding + * the REFERENCE_VALUE_TYPES arm, whose derivation is the TARGET row's `id` + * column — a derivation that was never the file family's, whose value is an + * opaque `sys_file` id. + * + * ## Why the typescript half is the side that moves + * + * 2048 is not a width invented to settle a disagreement. It is the width + * ADR-0104 ruled and the rest of the tree has already shipped: + * + * - ADR-0104, recording the maintainer ruling on #15041: "The driver is the + * side that moves; the generator's `VARCHAR(2048)` already states the ruled + * end-state and stands." + * - #15989 moved `driver-sql`: `MEDIA_ID_VARCHAR_CHARS = 2048` and + * `table.string(name, MEDIA_ID_VARCHAR_CHARS)` on the moved arm. + * - `os migrate files-to-references --apply` retypes the column to + * `varchar(2048)` (`MEDIA_ID_MOVE_WIDTH`, transcribed from the same width). + * + * So a deployment scaffolded from the typescript format was the one place left + * standing at 255 — its declared width disagreeing with the width the migration + * it will later run retypes to. ⛔ The repair is the typescript half joining + * the shipped target, never the two halves meeting in the middle, which is why + * the sweep below asserts the AGREED width against the driver's own constant + * rather than merely asserting the two formats equal. Two formats that had both + * drifted to 255 would satisfy an equality-only pin perfectly. + * + * ## The control, and where it comes from + * + * ⛔ Not invented here: `generate.ts` documents one. Its `text` entry carries a + * measured row for `f_text` — "sql gen f_text varchar(255) / ts gen f_text + * varchar(255)" — a NON-file family whose two halves already agree. Measured + * before and after the repair, `f_text` and every other non-family column in + * the same emission are byte-identical; only the five family rows move. The + * sweep pins that shape: the family agrees at the driver's width, and the + * neighbours it used to share an arm with keep the unsized call. + * + * ## What this pin does NOT assert + * + * Nothing about live DDL. `generate-string-family-width.pin.test.ts` carries + * the measured-on-PostgreSQL evidence for the character families; this file + * measures agreement between two emitters and the driver's own stated width, + * which is the whole content of the card. And nothing about the JSON-vs-varchar + * question that #15989 settled — `generate-field-type-vocabulary.pin.test.ts` + * holds that one, including the control that reddens if the driver reverts. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES } from '@objectstack/spec/data'; +import { describe, expect, it } from 'vitest'; + +import { generateMigrationSql, generateMigrationTs } from './generate.js'; + +/** `packages/drivers/driver-sql/src` — declared for `@objectstack/cli#test` in `turbo.json`. */ +const DRIVER_SQL_SRC = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../../drivers/driver-sql/src'); +const SQL_DRIVER_SOURCE = fs.readFileSync(path.join(DRIVER_SQL_SRC, 'sql-driver.ts'), 'utf8'); + +/** The control the generator documents, plus the arm the family used to ride. */ +const CONTROL_TYPES = ['text', ...REFERENCE_VALUE_TYPES, 'autonumber'] as const; + +function probeConfig(): Record { + const fields: Record> = {}; + for (const type of [...FILE_REFERENCE_TYPES, ...CONTROL_TYPES]) fields[`f_${type}`] = { type }; + return { objects: { probe: { name: 'probe', label: 'Probe', fields } } }; +} + +const SQL_OUT = generateMigrationSql(probeConfig()); +const TS_OUT = generateMigrationTs(probeConfig()); + +/** The column declaration one field type contributes to the sql format. */ +function sqlColumn(type: string): string | null { + const m = SQL_OUT.match(new RegExp(`^ {2}"f_${type}" (.+?),?$`, 'm')); + return m ? m[1] : null; +} + +/** The `table.x('f_type'[, …])` call one field type contributes to the typescript format. */ +function tsColumn(type: string): string | null { + const m = TS_OUT.match(new RegExp(`^ {4}(table\\.\\w+\\('f_${type}'(?:, [^)]*)?\\)).*$`, 'm')); + return m ? m[1] : null; +} + +/** The character width one format states, or `null` when it states none. */ +function sqlWidth(type: string): number | null { + const m = /^VARCHAR\((\d+)\)$/.exec(sqlColumn(type) ?? ''); + return m ? Number(m[1]) : null; +} + +/** + * The width the typescript format's call carries. `null` for a bare + * `table.string(name)` — which is a WIDTH, knex's 255, and is exactly the + * answer this card is about; the sweep names it rather than reading it as + * absence. + */ +function tsWidth(type: string): number | null { + const m = /^table\.string\('f_[a-z_]+', (\d+)\)$/.exec(tsColumn(type) ?? ''); + return m ? Number(m[1]) : null; +} + +/** The driver's own width for a moved media column, read where it is declared. */ +function driverMediaChars(): number { + const m = SQL_DRIVER_SOURCE.match(/const MEDIA_ID_VARCHAR_CHARS = (\d+);/); + if (!m) { + throw new Error( + 'MEDIA_ID_VARCHAR_CHARS is no longer declared in driver-sql/src/sql-driver.ts. That constant ' + + 'is the width #15989 moved the driver to, and this pin has nothing to measure the generators ' + + 'against without it — re-read ADR-0104 and #15989 before re-anchoring it.', + ); + } + return Number(m[1]); +} + +describe('#17883 — both migration formats give the file family one width', () => { + it('control — the probe really emitted both formats, and the readers discriminate', () => { + // Non-vacuity: readers that matched nothing would make every `toBe(null)` + // below pass while measuring literally nothing. + expect(FILE_REFERENCE_TYPES.size).toBe(5); + expect(SQL_OUT).toContain('CREATE TABLE IF NOT EXISTS "probe"'); + expect(TS_OUT).toContain("db.schema.createTable('probe'"); + expect(sqlColumn('file')).not.toBeNull(); + expect(tsColumn('file')).not.toBeNull(); + // …and they tell two different answers apart, in both formats. + expect(sqlColumn('text')).not.toBe(sqlColumn('file')); + expect(tsColumn('text')).not.toBe(tsColumn('file')); + // A width reader that always answered `null` would make the sweep vacuous. + expect(sqlWidth('lookup')).not.toBeNull(); + expect(sqlWidth('text')).toBeNull(); + }); + + it('the driver states a media width this pin can anchor to', () => { + // The anchor is READ, never transcribed: a literal here would be a third + // copy of the number the card exists to stop copying. + expect(driverMediaChars()).toBeGreaterThan(255); + expect(SQL_DRIVER_SOURCE).toContain('table.string(name, MEDIA_ID_VARCHAR_CHARS)'); + }); + + for (const type of FILE_REFERENCE_TYPES) { + it(`${type} takes the same width in both formats, at the shipped target`, () => { + const width = driverMediaChars(); + expect( + sqlWidth(type), + `os generate migration --format sql no longer states the ruled end-state width for ${type}. ` + + 'ADR-0104 ruled that the GENERATOR states it and the driver moves to it — so a change here ' + + 'is the two halves meeting in the middle, which #17883 forbids.', + ).toBe(width); + expect( + tsWidth(type), + `os generate migration (typescript) gave ${type} a different width from the sql format. A ` + + 'bare `table.string(name)` reads as null here and is knex\'s varchar(255) — the #17883 ' + + 'defect exactly: one command, one field, two widths depending on --format.', + ).toBe(width); + // Stated once more as the emitted text, so a reader of a failure sees the + // two lines rather than two numbers. + expect(sqlColumn(type)).toBe(`VARCHAR(${width})`); + expect(tsColumn(type)).toBe(`table.string('f_${type}', ${width})`); + }); + } + + it('the control does not move: the non-file families keep their own answers', () => { + // `f_text` is the control `generate.ts` documents on its own `text` entry — + // a non-file family whose two halves already agree. ⛔ Not invented here. + expect(sqlColumn('text')).toBe('TEXT'); + expect(tsColumn('text')).toBe("table.text('f_text')"); + + // And the arm the family used to ride: a reference column holds the target + // row's id, `table.string('id').primary()` = knex's default width, so it + // takes the UNSIZED call. If the repair had widened this class too, it + // would have carried the file family's width into columns that never + // wanted it. + for (const type of [...REFERENCE_VALUE_TYPES, 'autonumber']) { + expect(tsWidth(type), `${type} was widened along with the file family`).toBeNull(); + expect(tsColumn(type)).toBe(`table.string('f_${type}')`); + expect(sqlWidth(type)).not.toBe(driverMediaChars()); + } + }); +}); diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 1fccbeffad..8d8c34a0f7 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -1372,13 +1372,36 @@ async function runClientGeneration(configPath: string | undefined, flags: { outp * other two moved would have manufactured a fresh within-file * contradiction of exactly the kind this card exists to close. * - * ⛔ NOT in scope here, and filed rather than mirrored: the FILE_REFERENCE_TYPES - * family (`file` / `image` / `avatar` / `video` / `audio`) is in the driver's - * `JSON_COLUMN_TYPES` today while this table gives it `VARCHAR(2048)`. That is - * a real disagreement, but it is #14657's ADR-0104 D3 answer against a driver - * that is still pre-D3 — a decision about which side moves, not a wrong value - * to correct. `generate-field-type-vocabulary.pin.test.ts` names the exclusion - * so it cannot be mistaken for coverage. + * ## #17883 — the FILE_REFERENCE_TYPES exclusion is closed + * + * This paragraph used to hold the family (`file` / `image` / `avatar` / + * `video` / `audio`) out of scope, "filed rather than mirrored": the family was + * in the driver's `JSON_COLUMN_TYPES` while this table gave it `VARCHAR(2048)`, + * which was #14657's ADR-0104 D3 answer against a driver that was still pre-D3 + * — a decision about which side moves, not a wrong value to correct. + * + * It was decided, and it landed. ADR-0104 records the ruling: "The driver is + * the side that moves; the generator's `VARCHAR(2048)` already states the ruled + * end-state and stands." #15989 moved the driver — the family left + * `JSON_COLUMN_TYPES`, and `createColumn` now builds + * `table.string(name, MEDIA_ID_VARCHAR_CHARS)` at 2048. + * + * What that left was the same fork one level down and INSIDE this file, which + * is the defect #17883 names: the typescript format below spelled the family's + * column as a bare `table.string(name)` — knex's `varchar(255)`, as the + * `autonumber` note above states in as many words — so ONE `os generate + * migration` answered ONE field with `varchar(2048)` under `--format sql` and + * `varchar(255)` as typescript, and a deployment scaffolded from the typescript + * half stood at a width `os migrate files-to-references --apply` retypes away + * from. The typescript format now reads its width off THIS table — see + * {@link fileReferenceVarcharChars} — so the two formats of one command cannot + * answer the same field differently again. + * + * ⛔ The five entries below did NOT move, and must not: 2048 is the + * already-shipped target that the driver (`MEDIA_ID_VARCHAR_CHARS`) and the + * `os migrate files-to-references` retype (`MEDIA_ID_MOVE_WIDTH`) were brought + * to. The repair is the typescript half joining it, ⛔ never the two halves + * meeting in the middle. */ const FIELD_TYPE_SQL_MAP: Record = { // #16091 — TEXT, not VARCHAR(255). `text` heads the SAME text-family arm as @@ -1909,6 +1932,37 @@ function declaredVarchar(maxLength: unknown): VarcharAnswer { return n > MAX_VARCHAR_CHARS ? { kind: 'unbounded' } : { kind: 'sized', chars: n }; } +/** + * The `varchar(n)` width a FILE_REFERENCE_TYPES column takes, READ from this + * file's own SQL vocabulary rather than transcribed beside it (#17883). + * + * ⛔ Never a second literal. The width is a decision this file already carries + * once — {@link FIELD_TYPE_SQL_MAP}'s `VARCHAR(2048)`, which ADR-0104 calls the + * ruled end-state and which `driver-sql` moved to in #15989 — and a copy of + * `2048` in the typescript format would be free to drift from it exactly as the + * bare `table.string(name)` it replaces did. One source, so "the two formats of + * one command agree" is true BY CONSTRUCTION and not by a reviewer noticing. + * + * It throws rather than falling back, for the same reason {@link numericSqlType} + * does: a file-reference entry this reader cannot parse means the SQL half has + * changed shape, and the only wrong answer is a plausible width emitted anyway. + * `generate-file-reference-width.pin.test.ts` measures both halves against each + * other, so the parting is named in CI before it can reach an author. + */ +function fileReferenceVarcharChars(fieldType: string): number { + const stated = FIELD_TYPE_SQL_MAP[fieldType]; + const width = typeof stated === 'string' ? /^VARCHAR\((\d+)\)$/.exec(stated) : null; + if (!width) { + throw new Error( + `generate: FIELD_TYPE_SQL_MAP states no VARCHAR width for the file-reference type ` + + `'${fieldType}' (it says ${JSON.stringify(stated)}), so the typescript format has no ` + + 'width to agree with. Restore the entry, or stop routing this type through the ' + + 'file-reference arm.', + ); + } + return Number(width[1]); +} + /** * `schema-drift.ts`'s `isUniqueScopeDeclared` — the FIELD-level unique * vocabulary. @@ -2697,8 +2751,9 @@ export function generateMigrationTs(config: Record): string { // `user` references sys_user, whose id is a text identifier (not a uuid), // so store it as a string column — consistent with the runtime sql-driver. // #14657 — `tree` is the same REFERENCE_VALUE_TYPES class pointing at the - // object's own id, and the FILE_REFERENCE_TYPES class stores an opaque - // `sys_file` id string (ADR-0104 D3). `autonumber` is a RENDERED string + // object's own id. (FILE_REFERENCE_TYPES rode this arm too until #17883 + // gave it its own below: its value is not another row's id, and the sql + // format states a width of its own for it.) `autonumber` is a RENDERED string // (prefix + counter + suffix), which is both what `FIELD_TYPE_MAP` says // and what `driver-sql` emits — a SERIAL could not hold `INV-0001`. // @@ -2716,10 +2771,30 @@ export function generateMigrationTs(config: Record): string { // for type uuid` on the very first insert. case 'lookup': case 'master_detail': case 'user': case 'tree': - case 'image': case 'file': case 'avatar': case 'video': case 'audio': case 'autonumber': colMethod = `table.string('${fieldName}')`; break; + // #17883 — FILE_REFERENCE_TYPES takes an arm of its own, at the width + // the sql format above already states for it. + // + // It used to ride the reference arm, and that arm's derivation was + // never this family's: a reference column holds the TARGET row's `id`, + // which `driver-sql` emits as `table.string('id').primary()` = knex's + // varchar(255), while a file-reference column holds an opaque + // `sys_file` id and the sql format states `VARCHAR(2048)` for it. So + // ONE `os generate migration` gave one field two widths depending on + // `--format`, and the typescript one was the outlier: ADR-0104 ruled + // the generator's 2048 the end-state, #15989 moved `driver-sql` to it + // (`table.string(name, MEDIA_ID_VARCHAR_CHARS)`), and + // `os migrate files-to-references --apply` retypes to it + // (`MEDIA_ID_MOVE_WIDTH`) — a typescript-scaffolded deployment was the + // one place left standing at 255. + // + // ⛔ The width is READ, never retyped here: see + // {@link fileReferenceVarcharChars}. + case 'image': case 'file': case 'avatar': case 'video': case 'audio': + colMethod = `table.string('${fieldName}', ${fileReferenceVarcharChars(fType)})`; + break; default: // Reachable only through the UNVALIDATED authoring door — a `type` // that is not a `FieldType` at all. Every real member is cased above,