Skip to content

Commit 5d16379

Browse files
os-zhuangclaude
andauthored
fix(objectql,metadata-core): refuse a by-id update whose non-scalar where.id predicate stands beside the payload id (#11230) (#11432)
update(obj, { id: 'rec_1', title: 'x' }, { where: { id: { $in: ['a','b'] } }, multi: true }) used to write exactly one row — rec_1 — with no diagnostic: the payload id outranked `where` and `multi` alike, so the declared row SET and the declared bulk intent were BOTH discarded, and rec_1 need not even have been a member of the set. The last silent member of the dropped-declaration family (#5748 payload operator-objects, #11009 extra where keys, #11142 unequal scalar where.id). Closing it reverses the REMAINING half of the #5748-pinned verdict 'a SCALAR data.id still outranks where and multi', which the maintainer ruling on #11230 (2026-08-23) authorizes. resolveEngineUpdateDispatch now rejects the shape with a message naming the payload id and the KIND of predicate the caller wrote, carrying the SAME ADR-0112 envelope #11142 declares — UPDATE_ID_MISMATCH + status 400. One ledger member for one defect class, two messages: the two shapes share a remedy (drop one of the two row-address spellings), so a second registered code would buy no consumer a branch it could act on, while growing the closed `code` vocabulary every AI-authored consumer must switch over (ADR-0112 D3/D4). No new error code is registered; the ledger's existing entry is widened in place. Pin-reversal discipline: both pins asserting the old verdict are INVERTED in place, never deleted and never relaxed — 'a SCALAR data.id still outranks where and multi' (its first expectation was exactly this shape) and 'a NON-SCALAR where.id keeps its #5748 verdict', the boundary #11142 deliberately left standing. The boundaries that survive gain their own pins: falsy scalar where.id, a `where` declaring no id, and the no-payload-id ladder. Fixes #11230 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4ed3053 commit 5d16379

8 files changed

Lines changed: 437 additions & 52 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@objectstack/metadata-core": minor
3+
"@objectstack/objectql": minor
4+
---
5+
6+
**BREAKING (accept-set tightening)**: a by-id `update` whose bound truthy scalar payload `data.id` stands beside a DECLARED but non-scalar `options.where.id``{ $in: [...] }`, an array, `null` — is now refused loudly (`UPDATE_ID_MISMATCH`, HTTP 400) instead of silently binding the payload row and discarding both the id predicate and any declared `multi: true` (#11230).
7+
8+
`update(obj, { id: 'rec_1', title: 'x' }, { where: { id: { $in: ['a', 'b'] } }, multi: true })` used to write exactly one row — `rec_1` — with no diagnostic: the payload id outranked `where` and `multi` alike (#5748), so the declared row SET and the declared bulk intent were both dropped, and `rec_1` need not even have been a member of the set. This was the LAST silent member of the dropped-declaration family (#5748 payload operator-objects, #11009 extra `where` keys, #11142 unequal scalar `where.id`); closing it reverses the remaining half of the #5748-pinned verdict `a SCALAR data.id still outranks where and multi`, which the maintainer ruling on #11230 (2026-08-23) authorizes.
9+
10+
What changes, per call shape (`resolveEngineUpdateDispatch`, so every pinned test double inherits the same verdict):
11+
12+
- A truthy scalar `data.id` beside a **non-scalar** `where.id` — an operator object, an array, `null`, or an explicitly-`undefined` `id` key — now **throws** `UPDATE_ID_MISMATCH` with `status: 400`, naming the payload id and the KIND of predicate the caller wrote. `multi: true` does not rescue the call (the payload id outranks `multi` per #5748, so the contradiction stands). Previously the write landed on the payload row with both declarations silently ignored.
13+
- Boundaries that do **not** move: a **falsy** scalar `where.id` (`0`, `''`) is a scalar and keeps its #11142 verdict (by-id); a `where` that declares **no** `id` key at all (`{}`, or no `where`) is untouched; and with **no** scalar payload id the ladder is exactly as #5748 left it (`multi` when declared, otherwise `reject`) — the refusal lives only on the payload-sourced by-id arm.
14+
- The refusal shares the #11142 error code deliberately — one ADR-0112 ledger member for one defect class, two messages. No new code is registered.
15+
16+
A caller hitting the new refusal declared a row address and a row-set predicate in one call and meant one of them; each fix is a one-line edit at the call site: drop `id` from the payload to write EVERY row the predicate selects (`update(object, fields, { where: { id: { $in: [...] } }, multi: true })`), or drop `where.id` to write the single row the payload names (`update(object, { id, ...fields })`). The refusal text names both. Measured before shipping: **no in-repo call site constructs the pair** — every production `where.id` predicate (the outbox sweeps) carries a payload with no `id` — so the in-repo blast radius is nil; an external SDK caller can still write it, and today that silently drops both declarations.
17+
18+
<!-- adr-0087: not-required (no-migration-prescription) No authorable surface is removed or renamed — no spec key, no export, no config field changes spelling, so `objectstack migrate meta` has nothing to rewrite and no ledger entry could serve an upgrader. The newly-refused call shape is a self-contradictory input whose declared condition was never evaluated; deciding whether the caller meant the payload row or the predicate's row set is a per-site intent decision a mechanical rewrite must not make, and the refusal text itself names both call-site fixes. -->

packages/metadata-core/src/engine-update-dispatch.ts

Lines changed: 174 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,30 @@
8787
* never identifies a row anywhere on this ladder (point 3 below), so there
8888
* is no second row address to disagree with.
8989
* - A NON-scalar `where.id` (`{ $in: [...] }`, an array, `null`) beside a
90-
* scalar payload id keeps its #5748 verdict (`by-id`, payload wins) — that
91-
* pin was not reversed, and widening over it is a separate decision, not a
92-
* rider here (#6435's own boundary, one shape over).
90+
* scalar payload id kept its #5748 verdict here (`by-id`, payload wins) —
91+
* #11142 deliberately did not reach it. #11230 does; see the next clause.
92+
*
93+
* **[#11230] A third overriding clause, on the same PAYLOAD-sourced by-id
94+
* arm:** a `where.id` that is DECLARED but is not a scalar id at all
95+
* (`{ $in: [...] }`, an array, `null`) beside a bound scalar payload id is
96+
* refused too — the same `UPDATE_ID_MISMATCH` / 400 envelope, because it is
97+
* the same defect one step further out. The by-id path binds one primary key
98+
* and evaluates no predicate, so the declared row-SET was discarded with no
99+
* diagnostic, and a declared `multi: true` went with it: the caller asked for
100+
* a bulk write over `{ $in: [...] }` and got one silent single-row write.
101+
* This reverses the REMAINING half of the #5748 pin (`a SCALAR data.id still
102+
* outranks where and multi`) — maintainer ruling on #11230, 2026-08-23 — and
103+
* closes the last silent arm of the #5748 / #11009 / #11142 family. The
104+
* boundaries that do NOT move:
105+
*
106+
* - A FALSY scalar `where.id` (`0`, `''`) IS a scalar, so it is not this
107+
* clause's shape at all and keeps its #11142 verdict (`by-id`, nothing to
108+
* conflict with — point 3 below).
109+
* - A `where` with no `id` key (`{}`, no `where` at all, or a `where` that is
110+
* not an object) declares no id predicate and is untouched.
111+
* - With NO scalar id in the payload, a non-scalar `where.id` never reached
112+
* this arm in the first place: it falls down the ladder to `multi`, then
113+
* `reject`, exactly as #5748 left it.
93114
*
94115
* Three things about that list are load-bearing and easy to get wrong when
95116
* copying it by hand — which is the whole argument for importing it instead:
@@ -104,11 +125,13 @@
104125
* 2. **`data.id` outranks `where.id`, but only when it IS an id.** The payload
105126
* is read first, so a scalar `data.id` still wins over `where` and over an
106127
* explicit `multi: true`; `update(o, { id: 'rec_1', … }, { multi: true })`
107-
* is one by-id write, unchanged. Since #11142, "wins over `where`" no
108-
* longer includes silently overriding a truthy scalar `where.id` that
109-
* names a DIFFERENT row — that call is refused (see the clause above);
110-
* what stays is precedence, not the silent drop. What a non-scalar
111-
* `data.id` no longer does
128+
* is one by-id write, unchanged. Since #11142 and #11230, "wins over
129+
* `where`" no longer includes silently overriding a DECLARED `where.id`
130+
* that is not the bound id — a truthy scalar naming a different row
131+
* (#11142) or a non-scalar predicate over a row set (#11230). Both are
132+
* refused (see the two clauses above); what stays is precedence over a
133+
* `where` that declares no id and over `multi`, never a silent drop. What
134+
* a non-scalar `data.id` no longer does
112135
* is *outrank* anything: it is not an id, so the decision falls through to
113136
* `where.id`, then to `multi`, then to `reject` — exactly the ladder a
114137
* non-scalar `where.id` falls down. Until objectstack#5748 the payload half
@@ -152,17 +175,27 @@ import {
152175
export const ENGINE_UPDATE_REJECT_MESSAGE = 'Update requires an ID or options.multi=true';
153176

154177
/**
155-
* [#11142] The `error.code` of the conflicting-id refusal: a by-id update
156-
* whose truthy scalar `options.where.id` names a different row than the bound
157-
* payload `data.id`. Registered in the spec's `ERROR_CODE_LEDGER` (ADR-0112)
158-
* under `@objectstack/objectql`, the production thrower; travels with
159-
* {@link ENGINE_UPDATE_ID_CONFLICT_STATUS} on the thrown error's own property
160-
* bag (the `recordNotFoundError` convention), so the REST boundary's
161-
* status/code passthrough answers 400 instead of a sanitised 500.
178+
* [#11142/#11230] The `error.code` of the conflicting-id refusals: a by-id
179+
* update whose bound payload `data.id` stands beside an `options.where.id`
180+
* that is not that same id — a truthy scalar naming a DIFFERENT row (#11142),
181+
* or a non-scalar predicate over a row SET (#11230). Registered in the spec's
182+
* `ERROR_CODE_LEDGER` (ADR-0112) under `@objectstack/objectql`, the production
183+
* thrower; travels with {@link ENGINE_UPDATE_ID_CONFLICT_STATUS} on the thrown
184+
* error's own property bag (the `recordNotFoundError` convention), so the REST
185+
* boundary's status/code passthrough answers 400 instead of a sanitised 500.
186+
*
187+
* **One code, two messages, deliberately (#11230).** The two shapes are one
188+
* defect class — the call declares the row address twice and the two
189+
* declarations cannot both hold — and the caller's remedy is the same act in
190+
* both: drop one of the two spellings. A second ledger code would buy a
191+
* consumer no branch it could act on differently, while adding a member to the
192+
* closed `error.code` vocabulary every AI-authored consumer must switch over
193+
* (ADR-0112 D3/D4: registration is deliberate, and the catalog stays small).
194+
* The two messages carry the difference, which is where ADR-0112 puts it.
162195
*/
163196
export const ENGINE_UPDATE_ID_CONFLICT_CODE = 'UPDATE_ID_MISMATCH';
164197

165-
/** [#11142] The HTTP status the conflicting-id refusal declares: a caller error, 400. */
198+
/** [#11142/#11230] The HTTP status the conflicting-id refusals declare: a caller error, 400. */
166199
export const ENGINE_UPDATE_ID_CONFLICT_STATUS = 400;
167200

168201
/** What `ObjectQLEngine.update` will do with a given `(data, options)` pair. */
@@ -173,10 +206,11 @@ export type EngineUpdateDispatch =
173206
| { readonly kind: 'multi' }
174207
/**
175208
* Neither — the engine throws `ENGINE_UPDATE_REJECT_MESSAGE`, the #11009
176-
* unhonoured-predicate message, or (#11142) the conflicting-id message.
209+
* unhonoured-predicate message, or one of the two conflicting-id messages
210+
* (#11142 unequal scalar, #11230 non-scalar predicate).
177211
*
178212
* `code`/`status` are present only when the refusal declares an ADR-0112
179-
* envelope of its own (today: the #11142 conflict,
213+
* envelope of its own (today: the #11142 / #11230 conflicting-id refusals,
180214
* {@link ENGINE_UPDATE_ID_CONFLICT_CODE} / 400). The #5748 / #11009
181215
* refusals deliberately stay undecorated — adding an envelope to them is a
182216
* wire-contract change this module must not make by side effect. Throwers
@@ -236,10 +270,33 @@ function asScalarId(value: unknown): string | number | bigint | undefined {
236270
export function scalarUpdateId(
237271
options?: EngineUpdateDispatchInput | null,
238272
): string | number | bigint | undefined {
273+
const declared = declaredWhereId(options);
274+
return declared.present ? asScalarId(declared.value) : undefined;
275+
}
276+
277+
/**
278+
* [#11230] Did the caller DECLARE `options.where.id`, and with what raw value?
279+
*
280+
* {@link scalarUpdateId} collapses "no `id` key at all" and "an `id` key
281+
* holding something that is not a primary key" into the same `undefined`,
282+
* because for *picking a row* they are the same answer. The #11230 refusal
283+
* needs them apart: `{ where: {} }` declares nothing, while
284+
* `{ where: { id: { $in: [...] } } }` declares a predicate the by-id path
285+
* would silently discard — and the refusal message has to quote it.
286+
*
287+
* Private on purpose, and `scalarUpdateId` is re-expressed on top of it rather
288+
* than duplicating the lookup: the module header's point is that one rule gets
289+
* one definition, and a second public spelling of "what is in `where.id`" is
290+
* how that stops being true.
291+
*/
292+
function declaredWhereId(
293+
options?: EngineUpdateDispatchInput | null,
294+
): { readonly present: boolean; readonly value: unknown } {
239295
const where = options?.where;
240-
if (!where || typeof where !== 'object') return undefined;
241-
if (!('id' in (where as Record<string, unknown>))) return undefined;
242-
return asScalarId((where as Record<string, unknown>).id);
296+
if (!where || typeof where !== 'object') return { present: false, value: undefined };
297+
const record = where as Record<string, unknown>;
298+
if (!('id' in record)) return { present: false, value: undefined };
299+
return { present: true, value: record.id };
243300
}
244301

245302
/**
@@ -280,6 +337,64 @@ export function engineUpdateIdConflictMessage(
280337
);
281338
}
282339

340+
/**
341+
* [#11230] How a DECLARED but non-scalar `where.id` is named inside its
342+
* refusal message: by kind, never by dumping the value.
343+
*
344+
* The reader needs to recognise what they wrote (`{ $in: [...] }`, an array,
345+
* `null`), not to read it back — and a caller's predicate can be arbitrarily
346+
* large or cyclic, so `JSON.stringify` here would trade a diagnostic for a
347+
* second failure mode. Operator objects name their keys, which is the half a
348+
* reader actually scans for.
349+
*/
350+
function spellDeclaredWhereId(value: unknown): string {
351+
if (value === null) return 'null';
352+
if (value === undefined) return 'undefined';
353+
if (Array.isArray(value)) {
354+
return `an array of ${value.length} value${value.length === 1 ? '' : 's'}`;
355+
}
356+
if (typeof value === 'object') {
357+
const keys = Object.keys(value as Record<string, unknown>);
358+
return keys.length > 0
359+
? `an operator object (${keys.map((k) => `'${k}'`).join(', ')})`
360+
: 'an operator object with no keys';
361+
}
362+
return `a ${typeof value}`;
363+
}
364+
365+
/**
366+
* [#11230] The message a by-id update is refused with when its bound scalar
367+
* payload `data.id` stands beside a DECLARED `options.where.id` that is not a
368+
* primary key at all — an operator object, an array, `null`.
369+
*
370+
* The last silent member of the #5748 / #11009 / #11142 dropped-declaration
371+
* family, and the widest of them: the by-id path discarded the declared row
372+
* SET *and* any declared `multi: true`, so a caller who asked to update every
373+
* row matching `{ $in: [...] }` got one silent single-row write to a row the
374+
* predicate may not even select. Refusing it reverses the REMAINING half of
375+
* the #5748 pin (`a SCALAR data.id still outranks where and multi`) —
376+
* maintainer ruling on #11230, 2026-08-23.
377+
*
378+
* Shares {@link ENGINE_UPDATE_ID_CONFLICT_CODE} with the #11142 message rather
379+
* than registering a second ledger code: same defect class, same remedy (drop
380+
* one of the two row-address spellings), and the difference between them is
381+
* exactly what a message is for. See that constant's note.
382+
*/
383+
export function engineUpdateIdPredicateConflictMessage(
384+
payloadId: string | number | bigint,
385+
whereId: unknown,
386+
): string {
387+
return (
388+
`Update binds the payload id ${spellScalarId(payloadId)} as the row address, but options.where.id ` +
389+
`declares ${spellDeclaredWhereId(whereId)} — a predicate over a SET of rows, not one primary key. ` +
390+
`The by-id path binds ONLY that one id and never evaluates a predicate, so the write would land on ` +
391+
`${spellScalarId(payloadId)} with the where.id predicate — and any declared multi:true — silently ` +
392+
`ignored (#11230). To write EVERY row the predicate selects, drop id from the payload and declare ` +
393+
`the predicate path: update(object, fields, { where: { id: <predicate> }, multi: true }). To write ` +
394+
`the single row ${spellScalarId(payloadId)}, drop where.id: update(object, { id, ...fields }).`
395+
);
396+
}
397+
283398
/**
284399
* The one spelling of "throw a `reject` verdict" (#11142) — used by
285400
* {@link assertEngineUpdateDispatch} and by `ObjectQL.update` itself, so a
@@ -380,6 +495,29 @@ export function resolveEngineUpdateDispatch(
380495
status: ENGINE_UPDATE_ID_CONFLICT_STATUS,
381496
};
382497
}
498+
// [#11230] The same defect one step further out, and the last silent
499+
// arm of this family. `where.id` is DECLARED but is not a primary key
500+
// at all — `{ $in: [...] }`, an array, `null` — so it is a predicate
501+
// over a SET of rows, and the by-id path evaluates no predicate. The
502+
// call used to dispatch `by-id` on the payload id and discard BOTH the
503+
// row set and any declared `multi: true` with no diagnostic: the
504+
// #5748-pinned verdict `a SCALAR data.id still outranks where and
505+
// multi`, whose REMAINING half the maintainer ruling on #11230
506+
// (2026-08-23) reverses. `conflictingWhereId === undefined` means the
507+
// scalar test above found no id; `declared.present` separates the two
508+
// reasons it could not — an absent `id` key (nothing was declared,
509+
// by-id stands) from a declared non-scalar (refused here).
510+
if (conflictingWhereId === undefined) {
511+
const declared = declaredWhereId(options);
512+
if (declared.present) {
513+
return {
514+
kind: 'reject',
515+
message: engineUpdateIdPredicateConflictMessage(payloadId, declared.value),
516+
code: ENGINE_UPDATE_ID_CONFLICT_CODE,
517+
status: ENGINE_UPDATE_ID_CONFLICT_STATUS,
518+
};
519+
}
520+
}
383521
}
384522
return { kind: 'by-id', id };
385523
}
@@ -478,6 +616,21 @@ export const ENGINE_UPDATE_DISPATCH_CASES: readonly EngineUpdateDispatchCase[] =
478616
// `multi` (#5748), so the call is still a by-id write carrying a where.id it
479617
// can never honour.
480618
{ what: 'a SCALAR data.id beside a DIFFERENT scalar where.id and multi:true — still refused, the payload id outranks multi (#11142)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: 'rec_2' }, multi: true }, expect: 'reject' },
619+
// [#11230] The OTHER half of the reversed #5748 pin, and the last silent arm
620+
// of this family: a DECLARED but NON-SCALAR `where.id` beside a scalar
621+
// payload id. It never had a row of its own in this case-set — the assertion
622+
// that pinned it lived in `engine-update-dispatch.test.ts` ("a SCALAR
623+
// data.id still outranks where and multi", first expectation) and is flipped
624+
// there. It gets rows HERE now, so every double bound to this case-set
625+
// inherits the refusal rather than each one re-deriving it.
626+
{ what: 'a SCALAR data.id beside an $in where.id and multi:true — refused; the row SET and the declared bulk intent were BOTH silently dropped (#11230 reverses the remaining half of the #5748 pin)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: { $in: ['a', 'b'] } }, multi: true }, expect: 'reject' },
627+
{ what: 'a SCALAR data.id beside an $in where.id, no multi — refused (#11230)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: { $in: ['a', 'b'] } } }, expect: 'reject' },
628+
{ what: 'a SCALAR data.id beside an ARRAY where.id — refused (#11230)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: ['a', 'b'] } }, expect: 'reject' },
629+
{ what: 'a SCALAR data.id beside a NULL where.id — refused (#11230)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: null } }, expect: 'reject' },
630+
// [#11230] The boundary that does NOT move: a FALSY scalar `where.id` IS a
631+
// scalar, so it is not this refusal's shape at all and keeps the #11142
632+
// verdict. Pinned so the reversal cannot creep over the truthiness rule.
633+
{ what: 'a SCALAR data.id beside a FALSY scalar where.id (0) — still by-id, a falsy id identifies no row (the #11142 boundary, untouched by #11230)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: 0 } }, expect: 'by-id', expectId: 'rec_1' },
481634
// ── The payload's scalar test (objectstack#5748). A non-scalar `data.id`
482635
// names no row, so it stops shadowing everything under it: the decision
483636
// falls through to `where.id`, then `multi`, then `reject`. Before #5748

0 commit comments

Comments
 (0)