Skip to content

Commit 21914c3

Browse files
fix(objectql)!: a multi-valued reference element is held to the single value's stored form (#22908) (#22947)
Fixes #22908 Clause-②: yes (narrowing) ## What changed `packages/objectql/src/validation/record-validator.ts`, the multi-value branch of `validateOne`. It returned `null` early for `lookup` / `user` / `file` / `image` ("reference integrity is handled elsewhere"), so an element of a `multiple: true` field was never parsed against its stored form. Those types now fall through to the ADR-0104 value-shape arm that a single value goes through. That arm parses the whole array with `valueSchemaFor(def, 'stored')`, which is already `z.array(element)` for a multi-valued field (`ReferenceIdValueSchema` for `lookup` / `user`, `FileReferenceIdValueSchema` for `file` / `image`). The result is the same refusal (`invalid_type`, message key `invalid_value_shape`), under the same posture, with the same warn-first sink. No second shape rule was added, as triage 6115825886 binds. - One predicate, `isValueShapeJudgedType`, now answers "does the ADR-0104 arm judge this type" in all three places that ask it: the arm, the multi-value branch that hands an element to it, and `valueShapeViolation` (the scan's predicate). Before this, the scan and the branch could disagree about which types those are. - `valueShapeDetail` names the index of the wrong element on a multi-valued field (`at index 1: ...`), so a refusal names the field and the index. The write path and `os migrate value-shapes` share this one reading. `repeater` / `vector` details are unchanged. - System principal: the shape check is the validator's, which already runs for an `isSystem` write, so the system door now refuses too. `assertReferencesResolve` is untouched (read-only): its `isSystem` latitude on whether an id EXISTS stays as #4551 left it, and a test pins that. - `file` / `image` with `multiple: true`: measured and given their own stored form, the opaque `sys_file` id, under the media posture (`mediaValueShapeStrict` / the file-references flag). A verified media flag does not open reference elements, and the value-shapes flag does not open media elements (pinned). One changeset, `.changeset/22908-multi-reference-element-shape.md`: `@objectstack/objectql` `minor`, BREAKING (accept-set narrowing), ADR-0087 `not-required (no-migration-prescription)`, FROM / TO stated. ## Premise, checked on `origin/main` `018b077fbf` before any edit - At the validator: with `valueShapeStrict: true`, `{ team: [{ id: 'x' }], watchers: [false] }` on a multi-valued `lookup` / `user` was ADMITTED, while the single-valued control `{ owner: { id: 'x' } }` was refused `invalid_type`. - At claim, read-only (triage's item). Does ADR-0104's value-shapes scan report a stored multi-valued element of the wrong shape? Yes. `scanValueShapes` over rows `team: [{ id }]`, `team: [false]`, `watchers: [42]` reported `team` count 2 and `watchers` count 1, plus a single-valued control row, with `blocking` 4. So no gap card is owed. But at BASE the scan counted values the strict write path admitted, which contradicts the scan's own header ("count exactly what strict mode would reject"). This change closes that disagreement, and a test pins the two in agreement. The media half has its own evidence: `backfill-file-references.ts` converts arrays element by element. ## Pins (tests beside the code) - `packages/objectql/src/validation/multi-reference-element-shape.test.ts` (validator level): for `lookup` / `user` with `multiple: true`, the three bad elements (boolean, number, embedded record) are refused on insert and update, naming the field and index 0. The wrong element at index 2 is named as index 2. CONTROLS: the single-valued rows refuse the same three with no index; a well-formed id array and `[]` are accepted; a non-array is still the array refusal. A readonly multi-valued reference is judged by its shape, as a readonly single value is. Warn-first admits the element and reports it to the sink (`value-shape` gate for references, `media` for files). The gate split holds, and the `OS_ALLOW_LAX_VALUE_SHAPES` hatch still works. `file` / `image` elements are held to the `sys_file` id form. The scan predicate and the strict write path agree on every probe. - `packages/objectql/src/engine-multi-reference-element-shape.test.ts` (engine write path, strict posture): the three bad elements on `lookup` and `user` with `multiple: true`, through a caller's context (the engine call the REST data routes make) and the system principal, on insert and update. Each is refused as a `ValidationError` with `code` `VALIDATION_FAILED` and `fields[0]` `{ field, code: 'invalid_type' }`, and nothing is stored or changed. CONTROLS: the single-valued rows are unchanged on both doors; well-formed id arrays are accepted on both doors; and a well-formed id that names no row is `reference_not_found` for the caller but still written by the system principal (the `isSystem` existence latitude is kept). ## The real door: PR #22907's reference-shape family (one-off, not committed) `runReferenceShapeProofs` from `@objectstack/verify` (a real `bootStack`, the REST data routes as the signed-in caller, and `hooks.run` with `{ system: true }`) over a fixture with `accounts` (lookup, multiple), `watchers` (user, multiple), and single-valued `account` / `assignee` controls. It ran as a scratch test file that was deleted afterwards. It resolves `@objectstack/objectql` through its `exports`, so through the built `dist/`. - Fixed build: 4 fields, 4 proven, 0 holes; 48 probes, 48 refused `invalid_type` (REST answers `400`), 0 admitted, 0 unattributed. - Ablated build (below): the two multi-valued fields read `holes`, 8 of 12 admitted each. That reproduces the card's measurement: REST embedded record admitted `201` / `200`; REST boolean / number refused only as `reference_not_found` `400`; system door all admitted. The single-valued controls stay proven. Summary: 2 proven, 2 holes, 32 refused, 16 admitted. ## Ablation (one-off; `scripts/ablation-replace.mjs` in WRAP mode, plus an EXIT/INT/TERM trap) - Mutation: the early return restored as `if (isValueShapeJudgedType(t) && String('ABLATION-22908-MARKER')) return null;` ahead of the option arm. Anchor 1 to 0, blob `671387d31cbd` to `7cc631fd7095`. On disk: marker 1, the early-return line 1. - objectql pins (src-resolved): 43 failed, 10 passed (53). The 10 that pass are the controls. - `pnpm --filter @objectstack/objectql build`, then `scripts/ablation-dist-preflight.mjs`: the marker is present in 4 dist files. The real-door reading is above. - Restore: `git checkout HEAD -- ABS_PATH` by the tool. Blob after restore equals HEAD (`671387d31cbd`), `git diff HEAD` is empty, `git status` is clean. Rebuilt; `--absent` preflight: the marker is absent from all 14 dist files. objectql pins 53/53 pass; the real-door reading is 48/48 refused. - First attempt was a no-op: the tool refused before the child ran, because my replacement still contained the anchor (count 1 to 1). It restored to the HEAD blob and took no reading. The second attempt is the one reported. ## Local verification All read at the final head, `55cf89916e`, after the last commit. - `pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2`: 405 files passed, 7911 tests passed (through `os-verify-lock`, `VERDICT command-exit 0`). - `pnpm --filter @objectstack/objectql typecheck` (`tsc --noEmit` and `check:test-typecheck`): exit 0, test layer held at 40 files / 234 errors / 65 pinned signatures, with no new signature. Both new test files are in the `tsconfig.test.json` program (`--listFiles`); `tsconfig.json` excludes tests. - Gates: the 69 commands derived at dispatch for the declared surface all exit 0. Re-derived on the real diff (`dispatch-gates --commands`, change set from the merge base): 66 families, all inside the 69. `dispatch-gates --ran` with recorded exit codes: 66 derived, 66 run, 0 NOT-MEASURED, 0 UNRUN (a derived zero). The 3 extra are engine-surface families: `check:error-status-conformance`, `check:stack-collection-maps`, `check:swallow-census-controls`. One gate went red during development, `check:objectql-double-limit`, which was right: the new engine test's stub `find` ignored the caller's `limit`. It now applies the bound after the filter. - The dist-reading gates (`check:dts-closure`, `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:published-files`, `check:sourcemap-no-sources-content`, `check-dts-emitted --self-test`) ran after `pnpm --filter @objectstack/objectql build`. - Lint, narrowed and proven: eslint's own config (`calculateConfigForFile`) applies to all 3 changed `.ts` files, and none is ignored. `eslint --no-inline-config --format json` over them gave 3 files, 0 errors, 0 warnings. The config enables no type-aware linting (no `parserOptions.project`, no typed rules), so this diff cannot move the verdict on any untouched file. The repo-wide `pnpm lint` is CI's. - Consumers: no export, spec key or response shape changes, so no import-side suites were run. A grep of downstream writers of multi-valued arrays (`plugin-approvals` attachments, `service-storage` gallery, `rest` and `runtime` fixtures, the showcase seed) found id elements only. CI runs the full set. ## Acceptance notes (observations; not filed) - The spec's authoring-time default-value gate (`actionableValueIssueMessage`, `packages/spec/src/data/default-value-shape.ts`) is a deliberately separate reading. It does not name the index for a multi-valued literal default. That is polish, not a defect class; carrier: none. - `assertReferencesResolve`'s note "already-expanded objects are skipped" now describes a path reachable only under warn-first. On a strict deployment the validator refuses an expanded element first, for both cardinalities, as it already did for a single value. - Not measured: what the console (`objectui`) writes into a multi-valued lookup / user / file / image on save. No `objectui` checkout was available here. If a console editor wrote expanded records back as elements, a strict deployment would now refuse that save. The single-valued field has refused that shape under the same posture all along. - ⛔ Not folded here, per triage: making `os verify --reference-shapes` the default. The one-off reading above is the input for that separate call. `examples/app-showcase` itself was not run through the CLI. --- _Generated by [Claude Code](https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e5687cb commit 21914c3

4 files changed

Lines changed: 490 additions & 31 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
fix(objectql)!: each element of a multi-valued `lookup` / `user` / `file` / `image` field is held to the single value's stored form — an embedded record, a boolean or a number as an element answers `invalid_type` (#22908)
6+
7+
Clause-②: yes (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export or stored shape is removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite. The stored form a multi-valued reference or media field declares (`valueSchemaFor`: an array of record ids, or of `sys_file` ids) is unchanged. What narrows is the write path's accept-set, which now refuses an element that form already excluded. Stored rows holding such an element are application data, reported by `os migrate value-shapes` (references) and `os migrate files-to-references` (media) as before. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no exported declaration is removed or narrowed (not runtime-interface-only or type-surface-only). -->
10+
11+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
12+
13+
**FROM.** On a `lookup`, `user`, `file` or `image` field declared `multiple: true`, the record validator checked that the value was an array and stopped there. So an element of the wrong shape was stored:
14+
15+
- REST `POST` / `PATCH /api/v1/data/:object`, as a signed-in caller: an embedded record `[{ id }]` answered `201` / `200` and was stored verbatim. `[false]` and `[42]` were refused only by the reference-existence check, as `reference_not_found`.
16+
- A system write (`isSystem`: seed replay, hooks, package install): `[false]`, `[42]` and `[{ id }]` were all stored verbatim.
17+
- The single-valued field refused the same three `invalid_type` on both doors.
18+
19+
**TO.** Each element is parsed against the single value's stored form (ADR-0104 D1, `valueSchemaFor`): a record id for `lookup` / `user`, an opaque `sys_file` id for `file` / `image`. The deployment's value-shape posture decides, the same posture a single value answers to:
20+
21+
- **Strict** (a fresh store is born attested; or after `os migrate value-shapes --apply` for references, `os migrate files-to-references --apply` for media; or `OS_DATA_VALUE_SHAPE_STRICT_ENABLED=1`): the write is refused `400 VALIDATION_FAILED`, with a `fields[]` entry `{ field, code: 'invalid_type' }` whose detail names the index of the wrong element (`at index 1: …`).
22+
- **Warn-first** (an unverified deployment, or `OS_ALLOW_LAX_VALUE_SHAPES=1` / `OS_ALLOW_LAX_MEDIA_VALUES=1`): the write is admitted, logged and reported as an admitted ADR-0104 violation, exactly as a single value is.
23+
- The shape check runs for a system write too. Whether a referenced id EXISTS is unchanged: a system write still skips that check.
24+
- `os migrate value-shapes` already counted these elements; its report detail now names the index too, because the scan and the write path share one reading.
25+
26+
**Fix, if a write relied on the old answer:** write the ids as the elements, never the record a read with `$expand` returns — `team: ['acc_1', 'acc_2']`, not `team: [{ id: 'acc_1', name: 'Acme' }]`. To find stored rows that hold such an element, run `os migrate value-shapes` (a dry run writes nothing).
Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#22908] A multi-valued reference element of the wrong shape is refused on
5+
* the engine's write path, on both doors: a caller's write (the one the REST
6+
* data routes make) and the system principal's.
7+
*
8+
* Measured on `main` before the fix, on a deployment whose value-shape posture
9+
* is strict: `[{ id }]` was stored on a caller's insert and update (the
10+
* existence check skips an object-typed element), `[false]` / `[42]` reached
11+
* that check and came back `reference_not_found`, and the system principal —
12+
* for whom the existence check returns at once — stored all three verbatim.
13+
* The single-valued fields refused all three `invalid_type` on both doors.
14+
*
15+
* The SHAPE check is the record validator's, and it runs for a system write as
16+
* it does for a single value. Whether a referenced id EXISTS stays
17+
* `assertReferencesResolve`'s question, whose `isSystem` latitude is untouched:
18+
* the last case below pins that a well-formed id the existence check would
19+
* refuse a caller is still written by the system principal.
20+
*/
21+
22+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
23+
import { ObjectQL } from './engine.js';
24+
25+
const STRICT = 'OS_DATA_VALUE_SHAPE_STRICT_ENABLED';
26+
27+
const account = {
28+
name: 'ms_account',
29+
label: 'Account',
30+
fields: {
31+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
32+
name: { name: 'name', label: 'Name', type: 'text' as const },
33+
},
34+
};
35+
36+
const sysUser = {
37+
name: 'sys_user',
38+
label: 'User',
39+
fields: {
40+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
41+
name: { name: 'name', label: 'Name', type: 'text' as const },
42+
},
43+
};
44+
45+
const project = {
46+
name: 'ms_project',
47+
label: 'Project',
48+
fields: {
49+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
50+
title: { name: 'title', label: 'Title', type: 'text' as const },
51+
team: { name: 'team', label: 'Team', type: 'lookup' as const, reference: 'ms_account', multiple: true },
52+
watchers: { name: 'watchers', label: 'Watchers', type: 'user' as const, reference: 'sys_user', multiple: true },
53+
owner: { name: 'owner', label: 'Owner', type: 'lookup' as const, reference: 'ms_account' },
54+
assignee: { name: 'assignee', label: 'Assignee', type: 'user' as const, reference: 'sys_user' },
55+
},
56+
};
57+
58+
/** The card's three wrong shapes. */
59+
const WRONG: Record<string, unknown> = { boolean: false, number: 42, 'embedded-record': { id: 'acc_1' } };
60+
61+
const DOORS = {
62+
caller: { userId: 'usr_1' },
63+
system: { isSystem: true },
64+
} as const;
65+
66+
function makeStubDriver() {
67+
const stores = new Map<string, Map<string, Record<string, unknown>>>();
68+
const storeFor = (obj: string) => {
69+
let s = stores.get(obj);
70+
if (!s) { s = new Map(); stores.set(obj, s); }
71+
return s;
72+
};
73+
let nextId = 0;
74+
const matches = (row: Record<string, unknown>, where: any): boolean => {
75+
if (!where || typeof where !== 'object') return true;
76+
for (const [k, v] of Object.entries(where)) {
77+
if (k.startsWith('$')) continue;
78+
const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
79+
if ((row[k] ?? null) !== (expected ?? null)) return false;
80+
}
81+
return true;
82+
};
83+
const driver: any = {
84+
name: 'memory', version: '0.0.0', supports: {} as any,
85+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
86+
async execute() { return null; },
87+
async find(object: string, ast: any) {
88+
const rows = Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where));
89+
return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
90+
},
91+
async findOne(object: string, ast: any) {
92+
for (const r of storeFor(object).values()) if (matches(r, ast?.where)) return r;
93+
return null;
94+
},
95+
async create(object: string, data: Record<string, unknown>) {
96+
nextId += 1;
97+
const id = (data.id as string) ?? `r_${nextId}`;
98+
const row = { ...data, id };
99+
storeFor(object).set(id, row);
100+
return row;
101+
},
102+
async update(object: string, id: string, data: Record<string, unknown>) {
103+
const s = storeFor(object);
104+
const cur = s.get(id);
105+
if (!cur) return null;
106+
const next = { ...cur, ...data, id };
107+
s.set(id, next);
108+
return next;
109+
},
110+
async delete(object: string, id: string) { return storeFor(object).delete(id); },
111+
async count(object: string, ast: any) { return (await this.find(object, ast)).length; },
112+
async bulkCreate(object: string, rows: Record<string, unknown>[]) {
113+
return Promise.all(rows.map((r) => this.create(object, r)));
114+
},
115+
async bulkUpdate() { return []; },
116+
async bulkDelete() {},
117+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
118+
async commit() {}, async rollback() {},
119+
};
120+
return { driver, stores };
121+
}
122+
123+
async function refusalOf(run: () => Promise<unknown>): Promise<any> {
124+
try {
125+
await run();
126+
} catch (e) {
127+
return e;
128+
}
129+
throw new Error('expected the write to be refused, but it was admitted');
130+
}
131+
132+
describe('[#22908] multi-valued reference elements on the engine write path (strict posture)', () => {
133+
let engine: ObjectQL;
134+
let stores: Map<string, Map<string, Record<string, unknown>>>;
135+
let prior: string | undefined;
136+
137+
const projects = () => stores.get('ms_project') ?? new Map<string, Record<string, unknown>>();
138+
139+
beforeEach(async () => {
140+
prior = process.env[STRICT];
141+
process.env[STRICT] = '1';
142+
engine = new ObjectQL();
143+
const stub = makeStubDriver();
144+
stores = stub.stores;
145+
engine.registerDriver(stub.driver, true);
146+
await engine.init();
147+
engine.registry.registerObject(account);
148+
engine.registry.registerObject(sysUser);
149+
engine.registry.registerObject(project);
150+
const sys = { context: { isSystem: true } } as any;
151+
await engine.insert('ms_account', { id: 'acc_1', name: 'Acme' }, sys);
152+
await engine.insert('ms_account', { id: 'acc_2', name: 'Globex' }, sys);
153+
await engine.insert('sys_user', { id: 'usr_1', name: 'Ada' }, sys);
154+
await engine.insert('ms_project', { id: 'prj_1', title: 'Base', team: ['acc_1'], watchers: ['usr_1'] }, sys);
155+
});
156+
157+
afterEach(() => {
158+
if (prior === undefined) delete process.env[STRICT];
159+
else process.env[STRICT] = prior;
160+
});
161+
162+
for (const [door, context] of Object.entries(DOORS)) {
163+
for (const field of ['team', 'watchers'] as const) {
164+
for (const [kind, element] of Object.entries(WRONG)) {
165+
it(`${door} insert: ${field} = [${kind}] answers invalid_type naming the field, and nothing is stored`, async () => {
166+
const before = projects().size;
167+
const err = await refusalOf(() =>
168+
engine.insert('ms_project', { title: 'T', [field]: [element] }, { context } as any),
169+
);
170+
expect(err.name).toBe('ValidationError');
171+
expect(err.code).toBe('VALIDATION_FAILED');
172+
expect(err.fields).toHaveLength(1);
173+
expect(err.fields[0]).toMatchObject({ field, code: 'invalid_type' });
174+
expect(projects().size).toBe(before);
175+
});
176+
177+
it(`${door} update: ${field} = [${kind}] answers invalid_type naming the field, and the row is unchanged`, async () => {
178+
const held = projects().get('prj_1')?.[field];
179+
const err = await refusalOf(() =>
180+
engine.update('ms_project', { [field]: [element] }, { where: { id: 'prj_1' }, context } as any),
181+
);
182+
expect(err.name).toBe('ValidationError');
183+
expect(err.code).toBe('VALIDATION_FAILED');
184+
expect(err.fields).toHaveLength(1);
185+
expect(err.fields[0]).toMatchObject({ field, code: 'invalid_type' });
186+
expect(projects().get('prj_1')?.[field]).toEqual(held);
187+
});
188+
}
189+
}
190+
191+
it(`${door} CONTROL: the single-valued rows refuse the same three, unchanged`, async () => {
192+
for (const field of ['owner', 'assignee'] as const) {
193+
for (const element of Object.values(WRONG)) {
194+
const err = await refusalOf(() =>
195+
engine.insert('ms_project', { title: 'T', [field]: element }, { context } as any),
196+
);
197+
expect(err.fields[0]).toMatchObject({ field, code: 'invalid_type' });
198+
}
199+
}
200+
});
201+
202+
it(`${door} CONTROL: a well-formed id array is accepted on insert and on update`, async () => {
203+
const row: any = await engine.insert(
204+
'ms_project', { id: `prj_ok_${door}`, title: 'T', team: ['acc_1', 'acc_2'], watchers: ['usr_1'] }, { context } as any,
205+
);
206+
expect(row.team).toEqual(['acc_1', 'acc_2']);
207+
await engine.update('ms_project', { team: ['acc_2'], watchers: ['usr_1'] }, { where: { id: 'prj_1' }, context } as any);
208+
expect(projects().get('prj_1')).toMatchObject({ team: ['acc_2'], watchers: ['usr_1'] });
209+
});
210+
}
211+
212+
it('the existence check keeps its isSystem latitude: a well-formed id that names no row is the caller\'s refusal only', async () => {
213+
const err = await refusalOf(() =>
214+
engine.insert('ms_project', { title: 'T', team: ['acc_missing'] }, { context: DOORS.caller } as any),
215+
);
216+
expect(err.fields[0]).toMatchObject({ field: 'team', code: 'reference_not_found' });
217+
const row: any = await engine.insert('ms_project', { title: 'T', team: ['acc_missing'] }, { context: DOORS.system } as any);
218+
expect(row.team).toEqual(['acc_missing']);
219+
});
220+
});

0 commit comments

Comments
 (0)