fix(driver-sql,platform-objects): the builtin-column delivery table speaks the spec's field-type vocabulary, not knex's builder names (#12131)

.changeset/builtin-column-collision-warning.md

@@ -38,8 +38,11 @@ only when the declaration asks for storage the platform's own column does not de
 (a differing `type`, a `maxLength`, `unique`, `defaultValue`, `storage.notNull`, a
 `multiple` shape…) and stays silent when it does not: `created_at: { type: 'datetime',
 defaultValue: 'NOW()' }` describes precisely what lands, and says nothing.
-`id: { type: 'number' }` — an author expecting a numeric key — still fires, as does
-`id: { type: 'text' }`. The storage/presentation split is one table
+`id: { type: 'number' }` — an author expecting a numeric key — still fires.
+`id: { type: 'text' }` does **not**: varchar(255) canonicalizes to the field type
+`text`, so that declaration asks for precisely what the column delivers (#12131 —
+the delivery table recorded the knex builder name `'string'` there at first, and
+reported all 45 of the platform's own correct `id` declarations as disagreements). The storage/presentation split is one table
 (`builtin-column-collision.ts`) pinned against `FieldSchema.shape`, so a field key
 added later is classified deliberately instead of defaulting into silence.
 

.changeset/builtin-column-delivery-id-type.md

@@ -0,0 +1,46 @@
+---
+"@objectstack/driver-sql": patch
+"@objectstack/platform-objects": patch
+---
+
+fix(driver-sql): the builtin-column delivery table speaks the spec's field-type vocabulary, not knex's builder names (#12131)
+
+`BUILTIN_COLUMN_DELIVERY.id.type` recorded `'string'` — the **knex builder name** from
+`table.string('id').primary()` — and `undeliveredStorageAttributes` compares that value
+with `===` against a declaration's `type`, which is a spec `FieldType`. The two are
+different vocabularies, and `'string'` is not a member of the one being compared: it is
+absent from `FieldType`'s 49 options, `Field.string` is absent from the builder's keys,
+and `FieldSchema` refuses `type: 'string'` outright. So **no declaration could ever
+match it**, and the #12015 diagnostic reported every correct declaration on the
+platform's own key as a disagreement.
+
+Measured on a stock boot of `@objectstack/platform-objects`: **45 warnings, one per
+system object**, each saying `type: 'text' (the column is 'string')` about a
+declaration that was right all along. `varchar` canonicalizes to the field type `text`
+(`canonicalizeSqlType('varchar(255)') === 'text'`, `suggestFieldTypeForSqlType('varchar(255)') === 'text'`,
+`isCompatible('varchar(255)', 'text') === true` — all pinned in `type-compat.test.ts`),
+so `id: Field.text(...)` asks for exactly what the platform's column delivers. The
+delivery table now records `text`, and the 45 lines go silent because they were false,
+not because they were suppressed.
+
+`sys_migration.id`'s `maxLength: 128` was the one **honest** disagreement in that corpus
+— the column is varchar(255) — and it is removed rather than widened to 255. It bound
+nothing in any seam: the DDL discards a declared width on a builtin column name, and
+`validateRecord` skips `id` by name on both the insert and the update path (it is also
+`readonly`). Declaring a width that nothing enforces is the shape enforce-or-remove
+exists to prevent, and the 44 sibling system objects declare none.
+
+The classification pin now holds **every** entry in the delivery table to
+`FieldType.options`, so a builder name written there fails by name instead of surfacing
+as a corpus of false warnings. The fixtures in both #12015 pin files were written
+against the delivery table rather than against the source — `sys_presence.id` was spelled
+`type: 'string'` in the "silent" cases, which is why they passed while the same
+declaration as actually written warned. They now use the shapes as declared, and the
+firing cases declare a type that genuinely disagrees.
+
+**Grade: `patch` for both, and deliberately.** No door moves and no DDL changes: the
+platform still owns `id` / `created_at` / `updated_at`, the emitted column is
+byte-identical, every object that booted before still boots, and `BUILTIN_COLUMN_DELIVERY`
+is internal to the package (it is not re-exported from the package entry). The
+`platform-objects` half removes one metadata key that was measured inert in every seam
+that could read it. What changes is what the driver **says**.

packages/drivers/driver-sql/src/builtin-column-collision.test.ts

@@ -17,10 +17,19 @@
  * own column already provides is NOT a disagreement and must not be
  * reported as one — that is the whole content of the 2026-08-25 narrowing,
  * and the case that makes the message true again.
+ *
+ * ③ **The delivery table speaks the SPEC's vocabulary** (#12131). Its `type`
+ * is compared with `===` against a declaration's `type`, so a knex builder
+ * name there is a cross-vocabulary comparison that no declaration can
+ * satisfy. `id` used to record `'string'` — the `table.string('id')`
+ * builder name — and reported all 45 correct `id: Field.text(...)`
+ * declarations in `@objectstack/platform-objects` as disagreements while
+ * `'string'` was not even authorable. The first case below now holds every
+ * entry to `FieldType`, so the class fails by name rather than by corpus.
  */
 
 import { describe, it, expect } from 'vitest';
-import { FieldSchema } from '@objectstack/spec/data';
+import { FieldSchema, FieldType } from '@objectstack/spec/data';
 import {
  FIELD_KEY_STORAGE_CLASS,
  BUILTIN_COLUMN_DELIVERY,
@@ -60,10 +69,30 @@ describe('the FieldSchema storage/presentation classification (#12015)', () => {
  }
  });
 
+ it('⛔ spells every delivered `type` in the SPEC vocabulary, never a knex builder name (#12131)', () => {
+ // The one that got away: `id` recorded `'string'`, the knex builder name,
+ // and `undeliveredStorageAttributes` compares it with `===` against a
+ // declaration's `type` — a spec `FieldType`. No declaration could match it
+ // (`'string'` is absent from FieldType's 49 members and `FieldSchema`
+ // refuses it), so every correct `id: Field.text(...)` was reported as a
+ // disagreement: 45 of them on a stock boot of platform-objects.
+ for (const [column, delivery] of Object.entries(BUILTIN_COLUMN_DELIVERY)) {
+ expect(
+ FieldType.options as readonly string[],
+ `BUILTIN_COLUMN_DELIVERY.${column}.type must be a spec FieldType, not a builder name`,
+ ).toContain(delivery.type);
+ }
+ });
+
  it('records what each builtin column actually delivers, read off the emitting lines', () => {
- // `table.string('id').primary()` — varchar(255), NOT NULL, unique, no default.
+ // `table.string('id').primary()` — varchar(255), NOT NULL, unique, no
+ // default. varchar canonicalizes to the field type `text`
+ // (`canonicalizeSqlType('varchar(255)') === 'text'`, pinned in
+ // `type-compat.test.ts`), so `text` is what this column DELIVERS — which
+ // is why the platform's own `id: Field.text(...)` declarations agree with
+ // it exactly (#12131).
  expect(BUILTIN_COLUMN_DELIVERY.id).toMatchObject({
- type: 'string', maxLength: 255, unique: true, notNull: true, defaultValue: null,
+ type: 'text', maxLength: 255, unique: true, notNull: true, defaultValue: null,
  });
  // `createAuditTimestampColumn` — a timestamp defaulted to the DB clock, left NULLABLE.
  for (const column of ['created_at', 'updated_at']) {
@@ -76,22 +105,30 @@ describe('the FieldSchema storage/presentation classification (#12015)', () => {
 
 describe('what a declaration on a builtin column name loses (#12015)', () => {
  it('FIRES on the author error the card was filed for', () => {
- // `id: { type: 'number' }` — an author expecting a numeric key.
+ // `id: { type: 'number' }` — an author expecting a numeric key. This is
+ // the real author error; ⛔ NOT `{ type: 'text' }`, which is what the
+ // column delivers (see the silent case below).
  expect(keysOf(undeliveredStorageAttributes('id', { type: 'number' }))).toEqual(['type']);
- // The #11456 fixture's shape.
- expect(keysOf(undeliveredStorageAttributes('id', { type: 'text', name: 'id' }))).toEqual(['type']);
+ expect(keysOf(undeliveredStorageAttributes('id', { type: 'number', name: 'id' }))).toEqual(['type']);
  // …and names what the column really is, not just that something was lost.
- expect(undeliveredStorageAttributes('id', { type: 'text' })[0]).toMatchObject({
- key: 'type', declared: 'text', delivered: 'string',
+ expect(undeliveredStorageAttributes('id', { type: 'number' })[0]).toMatchObject({
+ key: 'type', declared: 'number', delivered: 'text',
  });
  });
 
  it('is SILENT for a presentation-only declaration — the platform honours that half', () => {
  // `sys_presence.id`, verbatim in shape: the population the pre-narrowing
- // warning was false about.
+ // warning was false about. ⚠️ It is `Field.text`, and this fixture used to
+ // spell it `type: 'string'` — matching the delivery table's builder name
+ // rather than the source. That made this case pass while the same
+ // declaration as actually written warned (#12131). Verbatim now.
  expect(
- undeliveredStorageAttributes('id', { type: 'string', label: 'Presence ID', required: true, readonly: true }),
+ undeliveredStorageAttributes('id', { type: 'text', label: 'Presence ID', required: true, readonly: true }),
  ).toEqual([]);
+ // The #11456 fixture's exact shape — the declaration that started #12015.
+ // It asks for precisely what the column delivers, so it is SILENT; it was
+ // reported as a disagreement until the delivery table was corrected.
+ expect(undeliveredStorageAttributes('id', { type: 'text', name: 'id' })).toEqual([]);
  expect(
  undeliveredStorageAttributes('created_at', {
  type: 'datetime', label: 'Created At', defaultValue: 'NOW()', readonly: true,
@@ -100,21 +137,21 @@ describe('what a declaration on a builtin column name loses (#12015)', () => {
  });
 
  it('is SILENT for a storage attribute the column already delivers', () => {
- expect(undeliveredStorageAttributes('id', { type: 'string', maxLength: 255 })).toEqual([]);
- expect(undeliveredStorageAttributes('id', { type: 'string', unique: true })).toEqual([]); // the PK is unique
- expect(undeliveredStorageAttributes('id', { type: 'string', storage: { notNull: true } })).toEqual([]); // the PK is NOT NULL
+ expect(undeliveredStorageAttributes('id', { type: 'text', maxLength: 255 })).toEqual([]);
+ expect(undeliveredStorageAttributes('id', { type: 'text', unique: true })).toEqual([]); // the PK is unique
+ expect(undeliveredStorageAttributes('id', { type: 'text', storage: { notNull: true } })).toEqual([]); // the PK is NOT NULL
  expect(undeliveredStorageAttributes('created_at', { type: 'datetime', defaultValue: 'now()' })).toEqual([]); // token, case-insensitive
  });
 
  it('FIRES for a storage attribute the column does NOT deliver, one entry each', () => {
- expect(keysOf(undeliveredStorageAttributes('id', { type: 'string', maxLength: 12 }))).toEqual(['maxLength']);
- expect(keysOf(undeliveredStorageAttributes('id', { type: 'string', defaultValue: 'NOW()' }))).toEqual(['defaultValue']);
+ expect(keysOf(undeliveredStorageAttributes('id', { type: 'text', maxLength: 12 }))).toEqual(['maxLength']);
+ expect(keysOf(undeliveredStorageAttributes('id', { type: 'text', defaultValue: 'NOW()' }))).toEqual(['defaultValue']);
  // created_at IS nullable and NOT unique — asking for either is a real disagreement.
  expect(keysOf(undeliveredStorageAttributes('created_at', { type: 'datetime', unique: true }))).toEqual(['unique']);
  expect(keysOf(undeliveredStorageAttributes('created_at', { type: 'datetime', storage: { notNull: true } })))
  .toEqual(['storage.notNull']);
  // Several at once, in declaration order.
- expect(keysOf(undeliveredStorageAttributes('id', { type: 'text', maxLength: 12, unique: false })))
+ expect(keysOf(undeliveredStorageAttributes('id', { type: 'number', maxLength: 12, unique: false })))
  .toEqual(['type', 'maxLength']); // `unique: false` asks for nothing
  });
 
@@ -125,7 +162,7 @@ describe('what a declaration on a builtin column name loses (#12015)', () => {
  it('stays silent — never throws — on a key it does not know, and on a malformed declaration', () => {
  // Forward compatibility: an unclassified key cannot invent a warning. The
  // exhaustiveness case above is what makes its arrival visible.
- expect(undeliveredStorageAttributes('id', { type: 'string', someFutureKey: 'x' } as any)).toEqual([]);
+ expect(undeliveredStorageAttributes('id', { type: 'text', someFutureKey: 'x' } as any)).toEqual([]);
  expect(undeliveredStorageAttributes('id', undefined)).toEqual([]);
  expect(undeliveredStorageAttributes('id', null as any)).toEqual([]);
  });

packages/drivers/driver-sql/src/builtin-column-collision.ts

@@ -158,7 +158,8 @@ export const FIELD_KEY_STORAGE_CLASS: Readonly<Record<string, FieldKeyClass>> =
  *
  * - `id` — `table.string('id').primary()`: varchar(255), NOT NULL,
  * PRIMARY KEY (so: unique), no column default (the engine
- * generates the key).
+ * generates the key). varchar canonicalizes to the field
+ * type `text`, which is what the table below records.
  * - `created_at` / `updated_at` — `createAuditTimestampColumn`: a timestamp
  * (MySQL `datetime(3)`) defaulted to the database clock,
  * left NULLABLE and stamped by the driver on every write.
@@ -168,7 +169,24 @@ export const FIELD_KEY_STORAGE_CLASS: Readonly<Record<string, FieldKeyClass>> =
  * defaultValue: 'NOW()' }` describes precisely what lands.
  */
 export interface BuiltinColumnDelivery {
- /** The field type whose column this builtin actually is. */
+ /**
+ * The field type whose column this builtin actually is, spelled in the
+ * SPEC's `FieldType` vocabulary — because that is the vocabulary a
+ * declaration's `type` is written in, and this value is compared against it
+ * with `===`.
+ *
+ * ⛔ NEVER the knex builder name. `id` is emitted by `table.string('id')`,
+ * but knex's `string` IS varchar(255), and `canonicalizeSqlType('varchar(255)')`
+ * is `'text'` — so the field type this column delivers is `'text'`, and
+ * `isCompatible('varchar(255)', 'text')` is EXACT (both pinned in
+ * `type-compat.test.ts`). Spelling the builder name here compares two
+ * vocabularies and reports every correct `id: Field.text(...)` declaration
+ * as a disagreement: measured at 45 false warnings on a stock boot of
+ * `@objectstack/platform-objects`, on declarations that were right all
+ * along. `'string'` is not even authorable — it is absent from `FieldType`'s
+ * members and `FieldSchema` refuses it — so no declaration could have
+ * silenced it.
+ */
  type: string;
  /** Fixed varchar width, when the column is bounded. */
  maxLength?: number;
@@ -181,7 +199,10 @@ export interface BuiltinColumnDelivery {
 }
 
 export const BUILTIN_COLUMN_DELIVERY: Readonly<Record<string, BuiltinColumnDelivery>> = Object.freeze({
- id: Object.freeze({ type: 'string', maxLength: 255, unique: true, notNull: true, defaultValue: null }),
+ // `table.string('id')` is knex's varchar(255); the FIELD TYPE it delivers is
+ // `text` (see the vocabulary note on `BuiltinColumnDelivery.type` — do not
+ // put the builder name back here).
+ id: Object.freeze({ type: 'text', maxLength: 255, unique: true, notNull: true, defaultValue: null }),
  created_at: Object.freeze({ type: 'datetime', unique: false, notNull: false, defaultValue: 'NOW()' as const }),
  updated_at: Object.freeze({ type: 'datetime', unique: false, notNull: false, defaultValue: 'NOW()' as const }),
 });