Skip to content

Commit e7b6ff5

Browse files
committed
fix(objectql,metadata-core): refuse a by-id update whose scalar where.id names a different row than the payload id (#11142)
update(obj, { id: 'rec_1', ... }, { where: { id: 'rec_2' } }) used to bind rec_1 and silently discard the where.id predicate — the one unhonoured-predicate shape #11009's refusal deliberately left standing, because refusing it reverses the #5748-pinned verdict 'a SCALAR data.id still wins over a scalar where.id'. The maintainer ruling on #11142 (2026-08-23, option A) authorizes that reversal for the UNEQUAL truthy scalar shape only. resolveEngineUpdateDispatch now rejects the conflict with a message naming both ids, decorated code UPDATE_ID_MISMATCH + status 400 (the recordNotFoundError convention; code registered in the ADR-0112 ledger under @objectstack/objectql). Both throwers — assertEngineUpdateDispatch (every pinned fake) and ObjectQL.update — go through one shared engineUpdateDispatchRejectError, so fakes and the real engine refuse identically. The #5748/#11009 refusals stay plain Errors, byte-identical. Pin-reversal discipline: the interrupted pin flips to a refusal pin in ENGINE_UPDATE_DISPATCH_CASES and in the #6435 contrast pins — never deleted; the equal-ids spelling (REST folds the path id into the payload) gains its own passing pin; falsy and non-scalar where.id boundaries keep their pre-existing verdicts, pinned so the refusal cannot creep past the ruled scope. Fixes #11142 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
1 parent 46644e2 commit e7b6ff5

6 files changed

Lines changed: 349 additions & 21 deletions

File tree

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

Lines changed: 155 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,27 @@
7070
* stays `by-id` even beside `multi: true` (LifecycleService's guarded-reap
7171
* idiom — pinned in the case-set below).
7272
*
73+
* **[#11142] A second overriding clause, on the PAYLOAD-sourced by-id arm:**
74+
* a truthy scalar `options.where.id` naming a DIFFERENT row than the bound
75+
* payload id is refused (`UPDATE_ID_MISMATCH`, 400) rather than silently
76+
* discarded. `where.id` is a declared predicate exactly like #11009's extra
77+
* keys — the caller wrote "update <data.id> where id = <where.id>", a
78+
* condition that can never hold — and the by-id path would have dropped it
79+
* with no diagnostic. This deliberately reverses the #5748-pinned verdict
80+
* (`a SCALAR data.id still wins over a scalar where.id`) for the UNEQUAL
81+
* shape ONLY (maintainer ruling on #11142, 2026-08-23):
82+
*
83+
* - `data.id === where.id` stays `by-id`, untouched — the REST ingress folds
84+
* the path id into the payload, so the redundant-but-agreeing spelling is
85+
* a normal one (pinned below).
86+
* - A FALSY scalar `where.id` (`0`, `''`) conflicts with nothing: a falsy id
87+
* never identifies a row anywhere on this ladder (point 3 below), so there
88+
* is no second row address to disagree with.
89+
* - 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).
93+
*
7394
* Three things about that list are load-bearing and easy to get wrong when
7495
* copying it by hand — which is the whole argument for importing it instead:
7596
*
@@ -83,7 +104,11 @@
83104
* 2. **`data.id` outranks `where.id`, but only when it IS an id.** The payload
84105
* is read first, so a scalar `data.id` still wins over `where` and over an
85106
* explicit `multi: true`; `update(o, { id: 'rec_1', … }, { multi: true })`
86-
* is one by-id write, unchanged. What a non-scalar `data.id` no longer does
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
87112
* is *outrank* anything: it is not an id, so the decision falls through to
88113
* `where.id`, then to `multi`, then to `reject` — exactly the ladder a
89114
* non-scalar `where.id` falls down. Until objectstack#5748 the payload half
@@ -126,14 +151,39 @@ import {
126151
/** The message `update()` throws when a call identifies neither one row nor a bulk intent. */
127152
export const ENGINE_UPDATE_REJECT_MESSAGE = 'Update requires an ID or options.multi=true';
128153

154+
/**
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.
162+
*/
163+
export const ENGINE_UPDATE_ID_CONFLICT_CODE = 'UPDATE_ID_MISMATCH';
164+
165+
/** [#11142] The HTTP status the conflicting-id refusal declares: a caller error, 400. */
166+
export const ENGINE_UPDATE_ID_CONFLICT_STATUS = 400;
167+
129168
/** What `ObjectQLEngine.update` will do with a given `(data, options)` pair. */
130169
export type EngineUpdateDispatch =
131170
/** A truthy scalar `data.id`, or a truthy scalar `where.id` — `driver.update`. */
132171
| { readonly kind: 'by-id'; readonly id: unknown }
133172
/** No single id but `options.multi` — `driver.updateMany` with the composed AST. */
134173
| { readonly kind: 'multi' }
135-
/** Neither — the engine throws `ENGINE_UPDATE_REJECT_MESSAGE`. */
136-
| { readonly kind: 'reject'; readonly message: string };
174+
/**
175+
* Neither — the engine throws `ENGINE_UPDATE_REJECT_MESSAGE`, the #11009
176+
* unhonoured-predicate message, or (#11142) the conflicting-id message.
177+
*
178+
* `code`/`status` are present only when the refusal declares an ADR-0112
179+
* envelope of its own (today: the #11142 conflict,
180+
* {@link ENGINE_UPDATE_ID_CONFLICT_CODE} / 400). The #5748 / #11009
181+
* refusals deliberately stay undecorated — adding an envelope to them is a
182+
* wire-contract change this module must not make by side effect. Throwers
183+
* go through {@link engineUpdateDispatchRejectError} so the decoration has
184+
* one spelling.
185+
*/
186+
| { readonly kind: 'reject'; readonly message: string; readonly code?: string; readonly status?: number };
137187

138188
/** The subset of `EngineUpdateOptions` the dispatch decision actually reads. */
139189
export interface EngineUpdateDispatchInput {
@@ -192,6 +242,60 @@ export function scalarUpdateId(
192242
return asScalarId((where as Record<string, unknown>).id);
193243
}
194244

245+
/**
246+
* How a scalar id is quoted inside a refusal message: strings keep their
247+
* quotes so `'42'` (a string) and `42` (a number) stay visibly different —
248+
* the strict-identity conflict test below treats them as different ids, and
249+
* the message must let a reader see why. `JSON.stringify` is not used because
250+
* it throws on `bigint`.
251+
*/
252+
function spellScalarId(value: string | number | bigint): string {
253+
return typeof value === 'string' ? `'${value}'` : String(value);
254+
}
255+
256+
/**
257+
* [#11142] The message a by-id update is refused with when its truthy scalar
258+
* `options.where.id` names a different row than the bound payload `data.id`.
259+
*
260+
* The same defect class as the #11009 refusal one clause over: a declared
261+
* predicate the by-id path would silently discard — here the predicate IS the
262+
* primary key, spelled twice with two different values, a condition that can
263+
* never hold. Refusing it deliberately reverses the #5748-pinned verdict for
264+
* the UNEQUAL shape only (maintainer ruling on #11142, 2026-08-23); the
265+
* equal-ids spelling (the REST ingress folds the path id into the payload)
266+
* stays honoured.
267+
*/
268+
export function engineUpdateIdConflictMessage(
269+
payloadId: string | number | bigint,
270+
whereId: string | number | bigint,
271+
): string {
272+
return (
273+
`Update binds the payload id ${spellScalarId(payloadId)} as the row address, but options.where.id ` +
274+
`names a DIFFERENT row: ${spellScalarId(whereId)}. The by-id path binds ONLY one primary key — the ` +
275+
`losing spelling is never evaluated — so the write would land on ${spellScalarId(payloadId)} with the ` +
276+
`where.id condition silently ignored (#11142). Two ids are the same row only when they are identical, ` +
277+
`type included. If both spellings mean one row, make them equal; otherwise drop one: ` +
278+
`update(object, { id, ...fields }) addresses the row by the payload id, and ` +
279+
`update(object, fields, { where: { id } }) — with no id in the payload — addresses it by where.id.`
280+
);
281+
}
282+
283+
/**
284+
* The one spelling of "throw a `reject` verdict" (#11142) — used by
285+
* {@link assertEngineUpdateDispatch} and by `ObjectQL.update` itself, so a
286+
* pinned fake's refusal carries exactly the envelope the real engine's does.
287+
* Rejects without a declared `code`/`status` (the #5748 / #11009 family) stay
288+
* plain `Error`s, byte-identical to what both threw before this helper.
289+
*/
290+
export function engineUpdateDispatchRejectError(
291+
reject: Extract<EngineUpdateDispatch, { kind: 'reject' }>,
292+
): Error {
293+
const err = new Error(reject.message) as Error & { code?: string; status?: number };
294+
if (reject.code !== undefined) err.code = reject.code;
295+
if (reject.status !== undefined) err.status = reject.status;
296+
return err;
297+
}
298+
195299
/**
196300
* Decide what `ObjectQLEngine.update` does with `(data, options)`, without
197301
* doing it.
@@ -251,6 +355,32 @@ export function resolveEngineUpdateDispatch(
251355
message: engineByIdUnhonouredPredicateMessage('Update', unhonoured),
252356
};
253357
}
358+
// [#11142] The payload id won the ladder, and `where.id` — the only
359+
// `where` key left after the #11009 check above — is itself a truthy
360+
// scalar naming a DIFFERENT row. That is a predicate the by-id path would
361+
// silently discard, exactly like #11009's extra keys: "update <payload id>
362+
// where id = <where.id>" can never hold, and the write used to land on the
363+
// payload row with no diagnostic (the #5748-pinned verdict, reversed for
364+
// this UNEQUAL shape only by the maintainer ruling on #11142). Strict
365+
// identity on purpose: `42` and `'42'` are two ids until the caller says
366+
// otherwise, and a coercing comparison here would be the lenient-consumer
367+
// move Prime Directive #12 forbids. A declared `multi: true` cannot
368+
// rescue the call — the payload id outranks `multi` (#5748), so the
369+
// conflict stands wherever the flag sits. Falsy and non-scalar `where.id`
370+
// shapes never reach this check with a conflict verdict: a falsy scalar
371+
// identifies no row (header point 3), and a non-scalar keeps its #5748
372+
// by-id verdict (widening over it is a separate decision).
373+
if (payloadId) {
374+
const conflictingWhereId = scalarUpdateId(options);
375+
if (conflictingWhereId && conflictingWhereId !== payloadId) {
376+
return {
377+
kind: 'reject',
378+
message: engineUpdateIdConflictMessage(payloadId, conflictingWhereId),
379+
code: ENGINE_UPDATE_ID_CONFLICT_CODE,
380+
status: ENGINE_UPDATE_ID_CONFLICT_STATUS,
381+
};
382+
}
383+
}
254384
return { kind: 'by-id', id };
255385
}
256386
if (options?.multi) return { kind: 'multi' };
@@ -277,7 +407,11 @@ export function assertEngineUpdateDispatch(
277407
options?: EngineUpdateDispatchInput | null,
278408
): Exclude<EngineUpdateDispatch, { kind: 'reject' }> {
279409
const dispatch = resolveEngineUpdateDispatch(data, options);
280-
if (dispatch.kind === 'reject') throw new Error(dispatch.message);
410+
// [#11142] Through the shared helper, so a refusal that declares an
411+
// ADR-0112 `code`/`status` (the conflicting-id reject) carries it here
412+
// exactly as it does from the real engine; undecorated rejects throw the
413+
// same plain `Error` they always have.
414+
if (dispatch.kind === 'reject') throw engineUpdateDispatchRejectError(dispatch);
281415
return dispatch;
282416
}
283417

@@ -327,7 +461,23 @@ export const ENGINE_UPDATE_DISPATCH_CASES: readonly EngineUpdateDispatchCase[] =
327461
// spelling and objectstack#5748 left it exactly as it was.
328462
{ what: 'id carried in the data payload, no where at all', data: { id: 'rec_1', title: 'x' }, options: undefined, expect: 'by-id', expectId: 'rec_1' },
329463
{ what: 'a SCALAR data.id still wins over an explicit multi:true', data: { id: 'rec_1', title: 'x' }, options: { multi: true }, expect: 'by-id', expectId: 'rec_1' },
330-
{ what: 'a SCALAR data.id still wins over a scalar where.id', data: { id: 'rec_1', title: 'x' }, options: { where: { id: 'rec_2' } }, expect: 'by-id', expectId: 'rec_1' },
464+
// [#11142] The REVERSED #5748 pin. This row read
465+
// `expect: 'by-id', expectId: 'rec_1'` from #5748 until the maintainer
466+
// ruling on #11142 (2026-08-23) flipped the UNEQUAL shape to a refusal: a
467+
// truthy scalar `where.id` naming a different row than the payload id is a
468+
// predicate the by-id path would silently discard — the last silent member
469+
// of the #5748/#11009 dropped-declaration family. The pin flips, it does
470+
// not disappear; the EQUAL spelling keeps its own passing pin right below.
471+
{ what: 'a SCALAR data.id beside a DIFFERENT scalar where.id — refused, no longer silently wins (#11142 reverses the #5748 pin for the unequal shape)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: 'rec_2' } }, expect: 'reject' },
472+
// [#11142] The equal-ids spelling stays honoured: the REST ingress folds
473+
// the path id into the payload (`{ ...data, id: request.id }` beside
474+
// `where: { id: request.id }`), so redundant-but-agreeing is a NORMAL
475+
// spelling, not a conflict.
476+
{ what: 'data.id === where.id — the redundant-but-agreeing spelling (REST folds the path id into the payload) stays by-id (#11142)', data: { id: 'rec_1', title: 'x' }, options: { where: { id: 'rec_1' } }, expect: 'by-id', expectId: 'rec_1' },
477+
// [#11142] `multi: true` cannot rescue the conflict: the payload id outranks
478+
// `multi` (#5748), so the call is still a by-id write carrying a where.id it
479+
// can never honour.
480+
{ 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' },
331481
// ── The payload's scalar test (objectstack#5748). A non-scalar `data.id`
332482
// names no row, so it stops shadowing everything under it: the decision
333483
// falls through to `where.id`, then `multi`, then `reject`. Before #5748

‎packages/objectql/src/engine-update-by-id-payload-id.test.ts‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,12 @@
6262
import { describe, it, expect } from 'vitest';
6363
import type { EngineUpdateOptions } from '@objectstack/spec/data';
6464
import { ObjectQL } from './engine.js';
65-
import { assertEngineUpdateDispatch } from './engine-update-dispatch.js';
65+
import {
66+
assertEngineUpdateDispatch,
67+
ENGINE_UPDATE_ID_CONFLICT_CODE,
68+
ENGINE_UPDATE_ID_CONFLICT_STATUS,
69+
engineUpdateIdConflictMessage,
70+
} from './engine-update-dispatch.js';
6671

6772
interface RecordedCall {
6873
readonly fn: 'update' | 'updateMany';
@@ -271,15 +276,39 @@ describe('#6435 — the contrast pins: what this change deliberately does NOT to
271276
expect(call.data).toEqual({ id: 'rec_1', title: 'x' });
272277
});
273278

274-
it('a truthy scalar data.id that DISAGREES with where.id still wins, payload as sent', async () => {
275-
// `ENGINE_UPDATE_DISPATCH_CASES`: "a SCALAR data.id still wins over a
276-
// scalar where.id" ⇒ bound id `rec_1`, not `rec_2`. Unchanged here.
277-
const call = await observeWrite(
278-
{ id: 'rec_1', title: 'x' },
279-
{ where: { id: 'rec_2' } },
280-
{ fn: 'update', boundId: 'rec_1' },
281-
);
282-
expect(call.data).toEqual({ id: 'rec_1', title: 'x' });
279+
it('a truthy scalar data.id that DISAGREES with where.id is REFUSED — the #11142 reversal of the #5748 pin', async () => {
280+
// FLIPPED, not deleted (#11142, maintainer ruling 2026-08-23). This pin
281+
// read "still wins, payload as sent" — `ENGINE_UPDATE_DISPATCH_CASES`'s
282+
// `a SCALAR data.id still wins over a scalar where.id`, bound id `rec_1`
283+
// — from #5748 until the UNEQUAL shape was ruled a refusal: the losing
284+
// `where.id` was a declared predicate the by-id path silently discarded.
285+
// The refusal carries the ADR-0112 envelope halves, and NOTHING reaches
286+
// the driver. The equal-ids contrast pin above is the surviving half of
287+
// the old behaviour.
288+
const message = engineUpdateIdConflictMessage('rec_1', 'rec_2');
289+
let caught: any;
290+
try {
291+
assertEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: 'rec_2' } });
292+
} catch (e) {
293+
caught = e;
294+
}
295+
expect(caught, 'expected the conflicting-id refusal, but the dispatch resolved').toBeDefined();
296+
expect(caught.code).toBe(ENGINE_UPDATE_ID_CONFLICT_CODE);
297+
expect(caught.status).toBe(ENGINE_UPDATE_ID_CONFLICT_STATUS);
298+
expect(caught.message).toBe(message);
299+
300+
const { engine, calls } = await makeEngine();
301+
let engineCaught: any;
302+
try {
303+
await engine.update('task', { id: 'rec_1', title: 'x' }, { where: { id: 'rec_2' } });
304+
} catch (e) {
305+
engineCaught = e;
306+
}
307+
expect(engineCaught, 'expected the real engine to refuse too').toBeDefined();
308+
expect(engineCaught.code).toBe(ENGINE_UPDATE_ID_CONFLICT_CODE);
309+
expect(engineCaught.status).toBe(ENGINE_UPDATE_ID_CONFLICT_STATUS);
310+
expect(engineCaught.message).toBe(message);
311+
expect(calls, 'a refused write reaches no driver entry point').toEqual([]);
283312
});
284313

285314
it('the MULTI arm is exactly as PR #6433 left it', async () => {

0 commit comments

Comments
 (0)