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

* wip(driver-sql,platform-objects): #12131 option D — delivery table id.type + sys_migration rider

* fix(driver-sql,platform-objects): builtin-column delivery table speaks the spec field-type vocabulary (#12131)

Author: os-zhuang

.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 }),
 });