From 54828f4759c54044b67a5b5366ef1fae6874241a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 17:03:58 +0000 Subject: [PATCH] fix(objects): stop declaring maxLength on the platform's own id column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sys_metadata_commit` and `sys_http_delivery` were the last two shipped declarations that made a clean boot warn about the platform's own tables. `initObjects` emits `id`, `created_at` and `updated_at` itself and skips any declared field colliding with one, so the storage half of such a declaration is discarded before it reaches DDL. #12015 made that discard loud instead of silent; #12131 then cleared 45 system objects declaring `id` as `text`. These two survived that sweep because their residue was an attribute (`maxLength: 64`) rather than a type — nothing can honour a bound on a column the platform emits as varchar(255). Measured on origin/main at 70f7d6d735 over all 112 `*.object.ts` files (117 object declarations, 215 fields declared on a platform-emitted builtin column): exactly 2 declarations still tripped the diagnostic, and both are these. Fed through the real SqlDriver DDL path — one create pass, then one alter pass over the now-existing tables — they produced 4 `[sql-driver]` collision warning blocks; after this change the same two runs produce 0, with the 215-field sweep unchanged so the empty result is a measurement rather than an empty loop. The diagnostic itself is untouched, on purpose: it is working as designed and is what #12015 was filed for. Each package gains an in-package pin holding every object schema it ships to that shape, with a positive control in the same run. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --- .../system-object-id-declares-no-maxlength.md | 16 ++++ .../builtin-column-storage-attributes.test.ts | 84 ++++++++++++++++++ .../src/objects/sys-metadata-commit.object.ts | 10 ++- .../builtin-column-storage-attributes.test.ts | 86 +++++++++++++++++++ .../src/objects/http-delivery.object.ts | 8 +- 5 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 .changeset/system-object-id-declares-no-maxlength.md create mode 100644 packages/metadata-core/src/objects/builtin-column-storage-attributes.test.ts create mode 100644 packages/services/service-messaging/src/objects/builtin-column-storage-attributes.test.ts diff --git a/.changeset/system-object-id-declares-no-maxlength.md b/.changeset/system-object-id-declares-no-maxlength.md new file mode 100644 index 0000000000..093138625c --- /dev/null +++ b/.changeset/system-object-id-declares-no-maxlength.md @@ -0,0 +1,16 @@ +--- +"@objectstack/metadata-core": patch +"@objectstack/service-messaging": patch +--- + +`sys_metadata_commit` and `sys_http_delivery` stop declaring `maxLength: 64` on `id` — the last two platform-shipped declarations that made a clean boot warn about the platform's own tables. + +The driver emits `id`, `created_at` and `updated_at` itself and skips any declared field colliding with one, so the STORAGE half of such a declaration is discarded. #12015 made that discard loud instead of silent, and #12131 then cleared 45 system objects declaring `id` as `text`. These two survived that sweep because their residue was an attribute rather than a type: nothing can honour a `maxLength: 64` on a column the platform emits as `varchar(255)`. + +Measured on `origin/main` at `70f7d6d735`, over all 112 `*.object.ts` files in the repo (117 object declarations, 215 fields declared on a platform-emitted builtin column): exactly **2** declarations still tripped the diagnostic, and both are these. Fed through the real `SqlDriver` DDL path — one create pass, then one alter pass over the now-existing tables — those two produced **4** `[sql-driver]` collision warning blocks; after this change the same two runs produce **0**, with the 215-field sweep unchanged so the empty result is a measurement and not an empty loop. + +**No behaviour changes.** The attribute was already being discarded before it reached DDL, so the physical columns, the accept set and every write path are byte-for-byte what they were. What changes is that the platform's own declarations no longer trip a diagnostic aimed at author code. + +⛔ The warning itself is untouched, on purpose. It is working as designed — it is the diagnostic #12015 was filed for, because a declared `id` used to be discarded in silence. Quieting, suppressing or narrowing it was never the remedy; the platform's declarations getting clean is. + +Each package gains an in-package pin (`builtin-column-storage-attributes.test.ts`) holding every object schema it ships to that shape, with a positive control in the same run. diff --git a/packages/metadata-core/src/objects/builtin-column-storage-attributes.test.ts b/packages/metadata-core/src/objects/builtin-column-storage-attributes.test.ts new file mode 100644 index 0000000000..c405332e17 --- /dev/null +++ b/packages/metadata-core/src/objects/builtin-column-storage-attributes.test.ts @@ -0,0 +1,84 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15335 — this package's own shipped declarations must not trip the + * builtin-column diagnostic. + * + * `initObjects` emits `id`, `created_at` and `updated_at` itself and skips any + * declared field colliding with one, so the STORAGE half of such a declaration + * is discarded. #12015 made that loud instead of silent; #12131 then cleared 45 + * system objects that were declaring `id` as `text`. `sys_metadata_commit` + * survived both because its residue was an attribute (`maxLength: 64`) rather + * than a type — measured on `origin/main` at 70f7d6d735, it was one of only two + * declarations left in the whole repo that still tripped the diagnostic, and it + * warned on every clean boot about the platform's own table. + * + * ⛔ The remedy is NOT to quiet the diagnostic — it is working as designed and is + * exactly what #12015 was filed for. What this pin holds is the other side: the + * platform's own declarations stay clean, so a boot that prints one of these + * warnings is always about code the reader wrote. + * + * The authoritative classification of "storage" vs "presentation" keys lives in + * one table, `FIELD_KEY_STORAGE_CLASS` in `@objectstack/driver-sql`'s + * `builtin-column-collision.ts`, and is pinned there against `FieldSchema.shape`. + * ⛔ This file deliberately does not copy that table — metadata-core does not + * depend on the driver, and a hand-copied second list is how the two halves + * drift. It pins the one attribute this card removed, over EVERY object the + * package ships, with a positive control in the same run so a green result is a + * measurement rather than an empty loop. + */ + +import { describe, expect, it } from 'vitest'; +import * as objects from './index.js'; + +/** The three columns `initObjects` emits itself, so a declaration on them loses its storage half. */ +const PLATFORM_EMITTED_COLUMNS = ['id', 'created_at', 'updated_at'] as const; + +/** `maxLength` on a platform-emitted column: declared, discarded, and warned about on every boot. */ +function declaredBoundsOnBuiltins(schema: { + name?: unknown; + fields?: Record; +}): string[] { + const found: string[] = []; + for (const column of PLATFORM_EMITTED_COLUMNS) { + const declaration = schema.fields?.[column] as Record | undefined; + if (declaration && declaration.maxLength !== undefined) { + found.push(`${String(schema.name)}.${column} declares maxLength: ${String(declaration.maxLength)}`); + } + } + return found; +} + +const SHIPPED: Array<[string, { name: string; fields: Record }]> = []; +for (const [exportName, value] of Object.entries(objects as Record)) { + const schema = value as { name?: unknown; fields?: unknown }; + if (typeof schema?.name !== 'string') continue; + if (typeof schema?.fields !== 'object' || schema.fields === null) continue; + SHIPPED.push([exportName, { name: schema.name, fields: schema.fields as Record }]); +} + +describe('#15335 — metadata-core declares no storage attribute the platform cannot deliver', () => { + it('ships object schemas at all — the loop below is otherwise vacuous', () => { + expect(SHIPPED.length).toBeGreaterThan(0); + expect(SHIPPED.map(([, schema]) => schema.name)).toContain('sys_metadata_commit'); + }); + + it('detects the shape it forbids — positive control, same predicate, same run', () => { + expect( + declaredBoundsOnBuiltins({ name: 'synthetic', fields: { id: { type: 'text', maxLength: 64 } } }), + ).toEqual(['synthetic.id declares maxLength: 64']); + // …and stays silent on the honoured half, so it is a detector and not a blanket. + expect( + declaredBoundsOnBuiltins({ + name: 'synthetic', + fields: { id: { type: 'text', label: 'ID', required: true, readonly: true } }, + }), + ).toEqual([]); + }); + + for (const [exportName, schema] of SHIPPED) { + it(`${exportName} (${schema.name}) declares no maxLength on a platform-emitted column`, () => { + expect(declaredBoundsOnBuiltins(schema)).toEqual([]); + }); + } +}); diff --git a/packages/metadata-core/src/objects/sys-metadata-commit.object.ts b/packages/metadata-core/src/objects/sys-metadata-commit.object.ts index 7781fe47cf..993a4cdf84 100644 --- a/packages/metadata-core/src/objects/sys-metadata-commit.object.ts +++ b/packages/metadata-core/src/objects/sys-metadata-commit.object.ts @@ -31,12 +31,18 @@ export const SysMetadataCommitObject = ObjectSchema.create({ description: 'Package-scoped commit log grouping a turn’s metadata changes (ADR-0067).', fields: { - /** Primary Key — the commit id. */ + /** + * Primary Key — the commit id. + * + * ⛔ No `maxLength`: the platform emits this column itself (`varchar(255)`), + * so a declared bound is discarded and the SQL driver says so on every boot + * (#12015). The clean shape is the one every other system object already + * carries after #12131 — declare the honoured half only. + */ id: Field.text({ label: 'ID', required: true, readonly: true, - maxLength: 64, }), /** The app/package this commit belongs to (the unit a user reverts). */ diff --git a/packages/services/service-messaging/src/objects/builtin-column-storage-attributes.test.ts b/packages/services/service-messaging/src/objects/builtin-column-storage-attributes.test.ts new file mode 100644 index 0000000000..8f9d9470b1 --- /dev/null +++ b/packages/services/service-messaging/src/objects/builtin-column-storage-attributes.test.ts @@ -0,0 +1,86 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15335 — this package's own shipped declarations must not trip the + * builtin-column diagnostic. The sibling half of this pin lives in + * `@objectstack/metadata-core`, whose `sys_metadata_commit` carried the same + * residue; the contract asserted here is identical. + * + * `initObjects` emits `id`, `created_at` and `updated_at` itself and skips any + * declared field colliding with one, so the STORAGE half of such a declaration + * is discarded. #12015 made that loud instead of silent; #12131 then cleared 45 + * system objects declaring `id` as `text`. `sys_http_delivery` survived both + * because its residue was an attribute (`maxLength: 64`) rather than a type — + * measured on `origin/main` at 70f7d6d735, it was one of only two declarations + * left in the whole repo that still tripped the diagnostic, and it warned on + * every clean boot about the platform's own table. + * + * ⛔ The remedy is NOT to quiet the diagnostic — it is working as designed and is + * exactly what #12015 was filed for. What this pin holds is the other side: the + * platform's own declarations stay clean, so a boot that prints one of these + * warnings is always about code the reader wrote. + * + * The authoritative classification of "storage" vs "presentation" keys lives in + * one table, `FIELD_KEY_STORAGE_CLASS` in `@objectstack/driver-sql`'s + * `builtin-column-collision.ts`, and is pinned there against `FieldSchema.shape`. + * ⛔ This file deliberately does not copy that table — service-messaging does not + * depend on the driver, and a hand-copied second list is how the two halves + * drift. It pins the one attribute this card removed, over EVERY object the + * package ships, with a positive control in the same run so a green result is a + * measurement rather than an empty loop. + */ + +import { describe, expect, it } from 'vitest'; +import * as objects from './index.js'; + +/** The three columns `initObjects` emits itself, so a declaration on them loses its storage half. */ +const PLATFORM_EMITTED_COLUMNS = ['id', 'created_at', 'updated_at'] as const; + +/** `maxLength` on a platform-emitted column: declared, discarded, and warned about on every boot. */ +function declaredBoundsOnBuiltins(schema: { + name?: unknown; + fields?: Record; +}): string[] { + const found: string[] = []; + for (const column of PLATFORM_EMITTED_COLUMNS) { + const declaration = schema.fields?.[column] as Record | undefined; + if (declaration && declaration.maxLength !== undefined) { + found.push(`${String(schema.name)}.${column} declares maxLength: ${String(declaration.maxLength)}`); + } + } + return found; +} + +const SHIPPED: Array<[string, { name: string; fields: Record }]> = []; +for (const [exportName, value] of Object.entries(objects as Record)) { + const schema = value as { name?: unknown; fields?: unknown }; + if (typeof schema?.name !== 'string') continue; + if (typeof schema?.fields !== 'object' || schema.fields === null) continue; + SHIPPED.push([exportName, { name: schema.name, fields: schema.fields as Record }]); +} + +describe('#15335 — service-messaging declares no storage attribute the platform cannot deliver', () => { + it('ships object schemas at all — the loop below is otherwise vacuous', () => { + expect(SHIPPED.length).toBeGreaterThan(0); + expect(SHIPPED.map(([, schema]) => schema.name)).toContain('sys_http_delivery'); + }); + + it('detects the shape it forbids — positive control, same predicate, same run', () => { + expect( + declaredBoundsOnBuiltins({ name: 'synthetic', fields: { id: { type: 'text', maxLength: 64 } } }), + ).toEqual(['synthetic.id declares maxLength: 64']); + // …and stays silent on the honoured half, so it is a detector and not a blanket. + expect( + declaredBoundsOnBuiltins({ + name: 'synthetic', + fields: { id: { type: 'text', label: 'ID', required: true, readonly: false } }, + }), + ).toEqual([]); + }); + + for (const [exportName, schema] of SHIPPED) { + it(`${exportName} (${schema.name}) declares no maxLength on a platform-emitted column`, () => { + expect(declaredBoundsOnBuiltins(schema)).toEqual([]); + }); + } +}); diff --git a/packages/services/service-messaging/src/objects/http-delivery.object.ts b/packages/services/service-messaging/src/objects/http-delivery.object.ts index 940c990bdd..acc40e2958 100644 --- a/packages/services/service-messaging/src/objects/http-delivery.object.ts +++ b/packages/services/service-messaging/src/objects/http-delivery.object.ts @@ -80,10 +80,16 @@ export const HttpDelivery = ObjectSchema.create({ }, fields: { + /** + * ⛔ No `maxLength`: the platform emits this column itself + * (`varchar(255)`), so a declared bound is discarded and the SQL driver + * says so on every boot (#12015). The clean shape is the one every other + * system object already carries after #12131 — declare the honoured half + * only. + */ id: Field.text({ label: 'Delivery ID', required: true, - maxLength: 64, description: 'UUID — also doubles as the receiver-side idempotency key', }),