Skip to content

Commit 2bed4c3

Browse files
claude[bot]claude
andauthored
fix(objectql)!: refuse a field whose type is absent or not a FieldType member at the registration door (#16319) (#17444)
* wip(#16319): refuse a field with no/unknown type at the registration door Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * wip(#16319): narrow the driver refusal to an absent type; triage objectql fixtures Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * test(#16319): pin the registration-door refusal, the boot seam and the two downstream defences Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * chore(#16319): changeset with the BREAKING note for the refused field-type class Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * fix(#16319): type the new tests' engine query options so the erasure ratchet holds Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * chore(#16319): delete the four test-typecheck-debt entries the fixture triage graduated Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * fix(#16319): spell the cross-field fixture's text columns `text`, the member the door admits `CROSS_FIELD_OBJECT_FIELDS` declared `stage` and `owner` as `type: 'string'`. `string` is absent from `FieldType`, so this PR's own registration door refused the whole `cross_field_deal` object and the runtime suite's `beforeAll` threw before its subject ran — CI shard `Test Core (3/6)`, 1 failed | 250 passed. `text` is not a re-typing. `canonicalizeSqlType('varchar(255)')` is `'text'` and `suggestFieldTypeForSqlType('varchar(255)')` is `'text'`, both pinned in `spec/data/type-compat.test.ts`, so `text` is the spelling of the column `string` was already producing. Matches `id` / `organization_id` in the same map and the text columns of `CROSS_FIELD_OFFSET_OBJECT_FIELDS` below it. The emitted column does move `varchar(255)` -> `TEXT` (measured on sqlite-wasm), which is inert here: no index keys either column, `initObjects` is passed no indexes, and the longest value in the corpus is four characters. All four `initObjects` consumers stay green. A static census over the tree — `fields:` regions resolved through cross-package imports, classified as ObjectStack field maps, `type` checked against the `FieldType` union read out of `field.zod.ts` — reports these two as the only non-member declarations reaching a `registerObject` door. After this change that count is zero; the 108 that remain all reach `driver.initObjects` only, which the changeset's scope section deliberately leaves alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * fix(#16319): spell the schedule dogfood fixture's `touched` flag `boolean` `packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts` declared `touched: { type: 'checkbox' }`. `checkbox` is not a `FieldType` member — the enum spells the multi-value checkbox GROUP `checkboxes` and the scalar flag `boolean` — so this PR's registration door refuses the whole `sched_org_target` declaration and both #16659 pins fail at `bootStack` on `[sqlite-wasm]` and `[memory]`. Neither side is wrong alone. The declaration landed in ecdfc94 (#17334), after this branch's merge base `d57611dfd3`, where nothing refused it; the door is this card's deliverable. The merge queue tests the PR merged into current `main`, which is the first place the two meet — so this is the merge, not a defect in either diff. `boolean` and not `checkboxes`: the fixture's `update_record` node writes `touched: true`, and both pins read it back as a scalar (`r.touched === true || r.touched === 1`, `Boolean(r.touched)`). `checkboxes` stores a JSON array of option values, whose EMPTY set is truthy — the sweep pin's differential would then report rows nothing touched as touched. The member is chosen from the enum's own grouping and from what the tests assert, not from the refusal message's Levenshtein suggestion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0ee32ed commit 2bed4c3

19 files changed

Lines changed: 1293 additions & 55 deletions
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/metadata-core": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/metadata-protocol": minor
5+
"@objectstack/driver-sql": minor
6+
"@objectstack/cli": minor
7+
---
8+
9+
fix(objectql)!: a field whose `type` is absent or is not a `FieldType` member is refused at the registration door, and every downstream family default becomes a refusal (#16319)
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) nothing an author can write is removed or renamed, and no conversion could repair these bodies: a field with no `type` carries no statement of intent for a conversion to rewrite, which is exactly the finding — the platform cannot know whether the author meant a bounded VARCHAR or an unbounded TEXT, and the two producers guessed differently. The remedy is a human decision per field, so it is prescribed in prose and in the refusal text rather than registered as a mechanical rewrite. -->
12+
13+
**BREAKING** for stored metadata only: an object whose declaration carries a field with no `type`, or with a `type` that is not a `FieldType` member, **no longer loads**. Shipped as `minor` under the repo's launch-window convention. Maintainer ruling, 2026-09-10, verbatim: 「16319 一个没写 type(或拼错)的字段 应该禁止加载。这个才是合理的吧?其他同意」.
14+
15+
**What you have to do.** Nothing, unless a `sys_metadata` row in your deployment carries such a field. If one does, the startup log names it at `error` level — object, field and reason — and the row is left untouched and still reachable: open it in Studio and give the field a real `FieldType` member, or delete it (`DELETE /api/v1/metadata/object/NAME`). Nothing that passes `FieldSchema` is affected: it has always required `type` and always refused a non-member, so only the doors that skip Zod could ever deliver one.
16+
17+
## What was wrong
18+
19+
One declaration produced two different columns. Measured on live PostgreSQL 16.13, driving all three producers from one object:
20+
21+
| declaration | driver | `os generate migration --format sql` | `--format ts` |
22+
|:---|:---|:---|:---|
23+
| `{ maxLength: 100 }`, no `type` | `character varying(100)` | `TEXT` | `TEXT` |
24+
| `{ type: 'this_is_not_a_field_type', maxLength: 100 }` | `character varying(255)` | `TEXT` | `TEXT` |
25+
26+
`SqlDriver.createColumn` read `field.type || 'string'`, which heads its STRING-family arm and sizes the column from the declared `maxLength` (knex's 255 without one). All four generator loops in `os generate` read `String(fieldDef.type || 'text')`, which heads the TEXT family — unbounded unless the column is keyed. Both directions of harm are in the first row: the platform refuses a 101-character value that both generated tables accept, and a table generated from the same object accepts values the platform will not store.
27+
28+
## What it does now
29+
30+
- **One point of closure, at the registration door.** `SchemaRegistry.registerObject` refuses the WHOLE object declaration, with the ADR-0112 envelope (`INVALID_METADATA` + `422`), naming the object, the field and the reason — and offering the spec's own "did you mean?" for a mis-spelling. ⛔ The offending field is never dropped on its own: an object loaded one field short reports success at every authoring surface while the column is never created and every read of it answers `undefined`. Every door goes through this one — declared stacks, package and plugin manifests, `saveMetaItem`, the `sys_metadata` boot rehydration, and raw `registerObject` calls — and all three contributor kinds (`own`, `overlay`, `extend`) are judged, because `ObjectSchema.fields` and `ObjectExtensionSchema.fields` are both `z.record(z.string(), FieldSchema)`.
31+
- **The startup policy is revised for this class.** `loadMetaFromDb`'s 「Registered anyway so it stays serveable and fixable」 no longer applies to it. The row does not register; the startup log states the consequence and the fix once, at `error`. The row itself is untouched, and the metadata API's raw-row path still lists it, still serves it with the offending field visible, still accepts a corrected write, and still deletes it — pinned, because a refused row that vanished from Studio would be unfixable.
32+
- **Downstream guesses become refusals.** `createColumn` refuses a field that declares no `type` instead of building `varchar(255)` for it. All four `os generate` loops — both migration formats and both `os generate types` loops — refuse an absent or non-member `type` and generate nothing for that object, rather than emitting a table one column short. `fieldTypeToSql`'s docblock is rewritten in the same stroke: its `TEXT` miss branch is now dead residue of a total table, ⛔ not a family default to route anything new to.
33+
34+
## Scope, stated rather than left to be inferred
35+
36+
`SqlDriver.createColumn` refuses `type` ABSENCE, not `FieldType` MEMBERSHIP. Membership is refused for the whole object at the registration door, which fronts every route into `syncSchema`, so a non-member cannot reach the driver from a runtime at all. `driver-sql`'s own test corpus declares 388 non-member spellings across ~100 files that drive `initObjects` directly, and `'string'` is a declared `case` arm of that switch whose column shape differs from every member's — so closing that half is a corpus migration with column consequences, deliberately not folded into this change. A pin holds the boundary in both directions.
37+
38+
ONE fixture in that corpus is migrated here, because it is the one that crosses the door. `CROSS_FIELD_OBJECT_FIELDS` — exported from this package's root, so a published export and not only a local literal — declared `stage` and `owner` as `'string'`. Four of its five consumers hand it to `driver.initObjects`, which the paragraph above leaves alone; the fifth hands it to `ql.registerObject`, which now refuses the whole object. Both fields are re-spelled `'text'`. That is not a re-typing: `canonicalizeSqlType('varchar(255)')` is `'text'` and `suggestFieldTypeForSqlType('varchar(255)')` is `'text'`, both pinned in `spec/data/type-compat.test.ts`, so `'text'` is the spelling of the column `'string'` was already producing. It does move the emitted column from `varchar(255)` to `TEXT` (measured on sqlite-wasm: `stage varchar(255)` becomes `stage text`), which is inert for this fixture — no index keys either column, `initObjects` is passed no indexes, and the corpus's longest value in them is four characters.
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #16319 — no generator in this file guesses a field's column family any more.
5+
*
6+
* MAINTAINER RULING, 2026-09-10 (director seat batch #111 item 2), verbatim:
7+
* 「16319 一个没写 type(或拼错)的字段 应该禁止加载。这个才是合理的吧?其他同意」
8+
* — item 3: 「下游默认值全部改拒绝: … 两个迁移生成器与 `os generate types` 的
9+
* `|| 'text'` 一律改为响亮拒绝 … ⛔ 不再猜族」.
10+
*
11+
* ## What the retired default cost
12+
*
13+
* All four loops read `String(fieldDef.type || 'text')`. That put a typeless
14+
* field in the TEXT family while `SqlDriver.createColumn`'s own `field.type ||
15+
* 'string'` put the SAME declaration in the STRING family. Measured on the card
16+
* against live PostgreSQL 16.13:
17+
*
18+
* ```
19+
* driver sql gen ts gen
20+
* { maxLength: 100 } character varying(100) text text
21+
* { type: 'this_is_not_a_
22+
* field_type', maxLength:100} character varying(255) text text
23+
* ```
24+
*
25+
* Both directions of harm are in the first row: the platform refuses a
26+
* 101-character value that both generated tables accept.
27+
*
28+
* ## Why the generators refuse BOTH shapes and the driver refuses one
29+
*
30+
* A generator reads a config FILE and never touches the registry, so here the
31+
* refusal is the only door there is. `SqlDriver.createColumn` sits behind
32+
* `SchemaRegistry.registerObject`, which refuses a non-member for the whole
33+
* object — the ruling's single point of closure, and what its acceptance list
34+
* means by 「驱动永远到不了」.
35+
*
36+
* ⛔ The file, not the field: emitting a table one column short is the same
37+
* silent loss the ruling refuses at the registration door, one artifact left.
38+
*/
39+
40+
import { describe, it, expect } from 'vitest';
41+
import { readFileSync } from 'node:fs';
42+
import { dirname, resolve } from 'node:path';
43+
import { fileURLToPath } from 'node:url';
44+
import {
45+
generateTypesFromConfig,
46+
generateMigrationSql,
47+
generateMigrationTs,
48+
} from './generate.js';
49+
50+
const HERE = dirname(fileURLToPath(import.meta.url));
51+
52+
const configWith = (probe: Record<string, unknown>) => ({
53+
objects: {
54+
probe_object: {
55+
name: 'probe_object',
56+
label: 'Probe',
57+
fields: {
58+
title: { type: 'text', label: 'Title' },
59+
probe: { label: 'Probe', ...probe },
60+
},
61+
},
62+
},
63+
});
64+
65+
const NO_TYPE = { maxLength: 100 };
66+
const BAD_TYPE = { type: 'this_is_not_a_field_type', maxLength: 100 };
67+
const CONTROL = { type: 'email', maxLength: 100 };
68+
69+
const GENERATORS: Array<[string, (c: Record<string, unknown>) => string]> = [
70+
['os generate types', generateTypesFromConfig],
71+
['os generate migration --format sql', generateMigrationSql],
72+
['os generate migration --format ts', generateMigrationTs],
73+
];
74+
75+
describe('#16319 — every exported generator refuses an undeclarable field `type`', () => {
76+
for (const [label, generate] of GENERATORS) {
77+
it(`${label} refuses a field with NO \`type\`, naming object + field + reason`, () => {
78+
let thrown: any;
79+
try { generate(configWith(NO_TYPE) as any); } catch (e) { thrown = e; }
80+
expect(thrown, `${label} must refuse`).toBeDefined();
81+
expect(thrown.message).toContain("object 'probe_object'");
82+
expect(thrown.message).toContain("field 'probe'");
83+
expect(thrown.message).toContain('declares no `type`');
84+
expect(thrown.message).toContain('Nothing is generated for this object');
85+
});
86+
87+
it(`${label} refuses a NON-MEMBER \`type\` the same way`, () => {
88+
let thrown: any;
89+
try { generate(configWith(BAD_TYPE) as any); } catch (e) { thrown = e; }
90+
expect(thrown, `${label} must refuse`).toBeDefined();
91+
expect(thrown.message).toContain("'this_is_not_a_field_type'");
92+
expect(thrown.message).toContain('is not a member of `FieldType`');
93+
});
94+
95+
it(`POSITIVE CONTROL — ${label} still emits for the same object with a \`FieldType\` member`, () => {
96+
const out = generate(configWith(CONTROL) as any);
97+
// ⛔ Not merely "did not throw": the probe column has to be IN the output,
98+
// or a generator that silently dropped the field would pass this control.
99+
expect(out).toContain('probe');
100+
expect(out).toContain('title');
101+
});
102+
}
103+
104+
it('⛔ refuses the FILE, not the field — no partial artifact is produced', () => {
105+
// Every generator throws before returning, so there is no half-written
106+
// artifact to inspect: the assertion is that nothing comes back at all.
107+
for (const [, generate] of GENERATORS) {
108+
expect(() => generate(configWith(NO_TYPE) as any)).toThrow();
109+
}
110+
});
111+
112+
it('SOURCE PIN — all FOUR loops read `declaredFieldType`, and no `|| \'text\'` default survives', () => {
113+
// The fourth loop (`generateClientFromConfig`) is not exported, so its call
114+
// site is pinned here rather than driven. Reading this package's OWN source
115+
// through an `import.meta.url` seed — the spelling
116+
// `pnpm check:cross-package-test-inputs` recognises.
117+
const src = readFileSync(resolve(HERE, 'generate.ts'), 'utf8');
118+
const callSites = src.match(/const fType = declaredFieldType\(/g) ?? [];
119+
expect(callSites.length).toBe(4);
120+
// The retired default, gone from every executable line. Occurrences that
121+
// remain are prose in comments — the control below proves the probe can see
122+
// a real one, so the zero is a reading and not an empty walk.
123+
const executable = src
124+
.split('\n')
125+
.filter((l) => !l.trimStart().startsWith('//') && !l.trimStart().startsWith('*'));
126+
expect(executable.filter((l) => l.includes("|| 'text'")).length).toBe(0);
127+
expect(executable.filter((l) => l.includes('declaredFieldType(')).length).toBeGreaterThan(0);
128+
});
129+
});

0 commit comments

Comments
 (0)