Skip to content

Commit 4df2a98

Browse files
os-muskclaude
andauthored
fix(objects): stop declaring maxLength on the platform's own id column (#16936)
`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 70f7d6d 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. Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg Co-authored-by: Claude <noreply@anthropic.com>
1 parent bbf7614 commit 4df2a98

5 files changed

Lines changed: 201 additions & 3 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/metadata-core": patch
3+
"@objectstack/service-messaging": patch
4+
---
5+
6+
`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.
7+
8+
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)`.
9+
10+
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.
11+
12+
**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.
13+
14+
⛔ 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.
15+
16+
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.
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15335 — this package's own shipped declarations must not trip the
5+
* builtin-column diagnostic.
6+
*
7+
* `initObjects` emits `id`, `created_at` and `updated_at` itself and skips any
8+
* declared field colliding with one, so the STORAGE half of such a declaration
9+
* is discarded. #12015 made that loud instead of silent; #12131 then cleared 45
10+
* system objects that were declaring `id` as `text`. `sys_metadata_commit`
11+
* survived both because its residue was an attribute (`maxLength: 64`) rather
12+
* than a type — measured on `origin/main` at 70f7d6d735, it was one of only two
13+
* declarations left in the whole repo that still tripped the diagnostic, and it
14+
* warned on every clean boot about the platform's own table.
15+
*
16+
* ⛔ The remedy is NOT to quiet the diagnostic — it is working as designed and is
17+
* exactly what #12015 was filed for. What this pin holds is the other side: the
18+
* platform's own declarations stay clean, so a boot that prints one of these
19+
* warnings is always about code the reader wrote.
20+
*
21+
* The authoritative classification of "storage" vs "presentation" keys lives in
22+
* one table, `FIELD_KEY_STORAGE_CLASS` in `@objectstack/driver-sql`'s
23+
* `builtin-column-collision.ts`, and is pinned there against `FieldSchema.shape`.
24+
* ⛔ This file deliberately does not copy that table — metadata-core does not
25+
* depend on the driver, and a hand-copied second list is how the two halves
26+
* drift. It pins the one attribute this card removed, over EVERY object the
27+
* package ships, with a positive control in the same run so a green result is a
28+
* measurement rather than an empty loop.
29+
*/
30+
31+
import { describe, expect, it } from 'vitest';
32+
import * as objects from './index.js';
33+
34+
/** The three columns `initObjects` emits itself, so a declaration on them loses its storage half. */
35+
const PLATFORM_EMITTED_COLUMNS = ['id', 'created_at', 'updated_at'] as const;
36+
37+
/** `maxLength` on a platform-emitted column: declared, discarded, and warned about on every boot. */
38+
function declaredBoundsOnBuiltins(schema: {
39+
name?: unknown;
40+
fields?: Record<string, unknown>;
41+
}): string[] {
42+
const found: string[] = [];
43+
for (const column of PLATFORM_EMITTED_COLUMNS) {
44+
const declaration = schema.fields?.[column] as Record<string, unknown> | undefined;
45+
if (declaration && declaration.maxLength !== undefined) {
46+
found.push(`${String(schema.name)}.${column} declares maxLength: ${String(declaration.maxLength)}`);
47+
}
48+
}
49+
return found;
50+
}
51+
52+
const SHIPPED: Array<[string, { name: string; fields: Record<string, unknown> }]> = [];
53+
for (const [exportName, value] of Object.entries(objects as Record<string, unknown>)) {
54+
const schema = value as { name?: unknown; fields?: unknown };
55+
if (typeof schema?.name !== 'string') continue;
56+
if (typeof schema?.fields !== 'object' || schema.fields === null) continue;
57+
SHIPPED.push([exportName, { name: schema.name, fields: schema.fields as Record<string, unknown> }]);
58+
}
59+
60+
describe('#15335 — metadata-core declares no storage attribute the platform cannot deliver', () => {
61+
it('ships object schemas at all — the loop below is otherwise vacuous', () => {
62+
expect(SHIPPED.length).toBeGreaterThan(0);
63+
expect(SHIPPED.map(([, schema]) => schema.name)).toContain('sys_metadata_commit');
64+
});
65+
66+
it('detects the shape it forbids — positive control, same predicate, same run', () => {
67+
expect(
68+
declaredBoundsOnBuiltins({ name: 'synthetic', fields: { id: { type: 'text', maxLength: 64 } } }),
69+
).toEqual(['synthetic.id declares maxLength: 64']);
70+
// …and stays silent on the honoured half, so it is a detector and not a blanket.
71+
expect(
72+
declaredBoundsOnBuiltins({
73+
name: 'synthetic',
74+
fields: { id: { type: 'text', label: 'ID', required: true, readonly: true } },
75+
}),
76+
).toEqual([]);
77+
});
78+
79+
for (const [exportName, schema] of SHIPPED) {
80+
it(`${exportName} (${schema.name}) declares no maxLength on a platform-emitted column`, () => {
81+
expect(declaredBoundsOnBuiltins(schema)).toEqual([]);
82+
});
83+
}
84+
});

packages/metadata-core/src/objects/sys-metadata-commit.object.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,18 @@ export const SysMetadataCommitObject = ObjectSchema.create({
3131
description: 'Package-scoped commit log grouping a turn’s metadata changes (ADR-0067).',
3232

3333
fields: {
34-
/** Primary Key — the commit id. */
34+
/**
35+
* Primary Key — the commit id.
36+
*
37+
* ⛔ No `maxLength`: the platform emits this column itself (`varchar(255)`),
38+
* so a declared bound is discarded and the SQL driver says so on every boot
39+
* (#12015). The clean shape is the one every other system object already
40+
* carries after #12131 — declare the honoured half only.
41+
*/
3542
id: Field.text({
3643
label: 'ID',
3744
required: true,
3845
readonly: true,
39-
maxLength: 64,
4046
}),
4147

4248
/** The app/package this commit belongs to (the unit a user reverts). */
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15335 — this package's own shipped declarations must not trip the
5+
* builtin-column diagnostic. The sibling half of this pin lives in
6+
* `@objectstack/metadata-core`, whose `sys_metadata_commit` carried the same
7+
* residue; the contract asserted here is identical.
8+
*
9+
* `initObjects` emits `id`, `created_at` and `updated_at` itself and skips any
10+
* declared field colliding with one, so the STORAGE half of such a declaration
11+
* is discarded. #12015 made that loud instead of silent; #12131 then cleared 45
12+
* system objects declaring `id` as `text`. `sys_http_delivery` survived both
13+
* because its residue was an attribute (`maxLength: 64`) rather than a type —
14+
* measured on `origin/main` at 70f7d6d735, it was one of only two declarations
15+
* left in the whole repo that still tripped the diagnostic, and it warned on
16+
* every clean boot about the platform's own table.
17+
*
18+
* ⛔ The remedy is NOT to quiet the diagnostic — it is working as designed and is
19+
* exactly what #12015 was filed for. What this pin holds is the other side: the
20+
* platform's own declarations stay clean, so a boot that prints one of these
21+
* warnings is always about code the reader wrote.
22+
*
23+
* The authoritative classification of "storage" vs "presentation" keys lives in
24+
* one table, `FIELD_KEY_STORAGE_CLASS` in `@objectstack/driver-sql`'s
25+
* `builtin-column-collision.ts`, and is pinned there against `FieldSchema.shape`.
26+
* ⛔ This file deliberately does not copy that table — service-messaging does not
27+
* depend on the driver, and a hand-copied second list is how the two halves
28+
* drift. It pins the one attribute this card removed, over EVERY object the
29+
* package ships, with a positive control in the same run so a green result is a
30+
* measurement rather than an empty loop.
31+
*/
32+
33+
import { describe, expect, it } from 'vitest';
34+
import * as objects from './index.js';
35+
36+
/** The three columns `initObjects` emits itself, so a declaration on them loses its storage half. */
37+
const PLATFORM_EMITTED_COLUMNS = ['id', 'created_at', 'updated_at'] as const;
38+
39+
/** `maxLength` on a platform-emitted column: declared, discarded, and warned about on every boot. */
40+
function declaredBoundsOnBuiltins(schema: {
41+
name?: unknown;
42+
fields?: Record<string, unknown>;
43+
}): string[] {
44+
const found: string[] = [];
45+
for (const column of PLATFORM_EMITTED_COLUMNS) {
46+
const declaration = schema.fields?.[column] as Record<string, unknown> | undefined;
47+
if (declaration && declaration.maxLength !== undefined) {
48+
found.push(`${String(schema.name)}.${column} declares maxLength: ${String(declaration.maxLength)}`);
49+
}
50+
}
51+
return found;
52+
}
53+
54+
const SHIPPED: Array<[string, { name: string; fields: Record<string, unknown> }]> = [];
55+
for (const [exportName, value] of Object.entries(objects as Record<string, unknown>)) {
56+
const schema = value as { name?: unknown; fields?: unknown };
57+
if (typeof schema?.name !== 'string') continue;
58+
if (typeof schema?.fields !== 'object' || schema.fields === null) continue;
59+
SHIPPED.push([exportName, { name: schema.name, fields: schema.fields as Record<string, unknown> }]);
60+
}
61+
62+
describe('#15335 — service-messaging declares no storage attribute the platform cannot deliver', () => {
63+
it('ships object schemas at all — the loop below is otherwise vacuous', () => {
64+
expect(SHIPPED.length).toBeGreaterThan(0);
65+
expect(SHIPPED.map(([, schema]) => schema.name)).toContain('sys_http_delivery');
66+
});
67+
68+
it('detects the shape it forbids — positive control, same predicate, same run', () => {
69+
expect(
70+
declaredBoundsOnBuiltins({ name: 'synthetic', fields: { id: { type: 'text', maxLength: 64 } } }),
71+
).toEqual(['synthetic.id declares maxLength: 64']);
72+
// …and stays silent on the honoured half, so it is a detector and not a blanket.
73+
expect(
74+
declaredBoundsOnBuiltins({
75+
name: 'synthetic',
76+
fields: { id: { type: 'text', label: 'ID', required: true, readonly: false } },
77+
}),
78+
).toEqual([]);
79+
});
80+
81+
for (const [exportName, schema] of SHIPPED) {
82+
it(`${exportName} (${schema.name}) declares no maxLength on a platform-emitted column`, () => {
83+
expect(declaredBoundsOnBuiltins(schema)).toEqual([]);
84+
});
85+
}
86+
});

packages/services/service-messaging/src/objects/http-delivery.object.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,16 @@ export const HttpDelivery = ObjectSchema.create({
8080
},
8181

8282
fields: {
83+
/**
84+
* ⛔ No `maxLength`: the platform emits this column itself
85+
* (`varchar(255)`), so a declared bound is discarded and the SQL driver
86+
* says so on every boot (#12015). The clean shape is the one every other
87+
* system object already carries after #12131 — declare the honoured half
88+
* only.
89+
*/
8390
id: Field.text({
8491
label: 'Delivery ID',
8592
required: true,
86-
maxLength: 64,
8793
description: 'UUID — also doubles as the receiver-side idempotency key',
8894
}),
8995

0 commit comments

Comments
 (0)