Skip to content

Commit 2810695

Browse files
os-zhuangclaude
andauthored
fix(objectql,metadata-core): refuse a by-id update whose scalar where.id names a different row than the payload id (#11229)
* 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 * chore(spec): regenerate error-code reference docs; add the #11142 changeset The UPDATE_ID_MISMATCH ledger entry lands in the generated content/docs/references pages (check:generated --fix, only the artifact it proved stale), and the breaking-changeset carries the ADR-0087 disposition marker (not-required: no authorable surface moves; the refused shape is a self-contradictory input whose fix is a per-site intent decision). Part of #11142 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 38cf397 commit 2810695

9 files changed

Lines changed: 371 additions & 22 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/metadata-core": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/spec": minor
5+
---
6+
7+
**BREAKING (accept-set tightening)**: a by-id `update` whose truthy scalar `options.where.id` names a DIFFERENT row than the truthy scalar payload `data.id` is now refused loudly — `UPDATE_ID_MISMATCH`, HTTP 400 — instead of silently binding the payload row and discarding the `where.id` predicate (#11142).
8+
9+
`update(obj, { id: 'rec_1', title: 'x' }, { where: { id: 'rec_2' } })` used to write `rec_1` with no diagnostic: the caller declared "update rec_1 where id = rec_2" — a condition that can never hold — and the by-id path dropped the losing spelling exactly the way #11009's extra `where` keys were dropped. This was the one unhonoured-predicate shape #11009's refusal deliberately left standing, because refusing it partially reverses the #5748-pinned verdict (`a SCALAR data.id still wins over a scalar where.id`). The maintainer ruling on #11142 (2026-08-23) authorizes that reversal for the UNEQUAL truthy scalar shape only.
10+
11+
What changes, per call shape (`resolveEngineUpdateDispatch`, so every pinned test double inherits the same verdict):
12+
13+
- `data.id === where.id` (both truthy scalars) is **unchanged** — by-id. This is the normal REST spelling: the ingress folds the path id into the payload, so redundant-but-agreeing pairs are routine.
14+
- `data.id` and `where.id` both truthy scalars and **different** — including differing only in type, e.g. `42` beside `'42'` — now **throws** `UPDATE_ID_MISMATCH` with `status: 400`, naming both ids. A declared `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 the condition silently ignored.
15+
- A **falsy** scalar `where.id` (`0`, `''`) beside a payload id is unchanged (a falsy id identifies no row on this ladder, so there is no second row address to conflict with), and a **non-scalar** `where.id` (`{ $in: [...] }`, an array, `null`) beside a payload id keeps its #5748 by-id verdict — widening over either is a separate decision, deliberately not taken here.
16+
17+
A caller hitting the new refusal wrote two row addresses and meant one of them; each fix is a one-line edit at the call site: make the two ids equal (or drop `where.id`) to keep addressing the row by the payload id, or remove `id` from the payload to address the row by `where.id`. The refusal is decorated with `code: 'UPDATE_ID_MISMATCH'` and `status: 400` on the thrown error (registered in the ADR-0112 error-code ledger; the spec's `ErrorCode` union gains the member), so REST callers get a located 400 instead of a sanitised 500, and doubles pinned to `assertEngineUpdateDispatch` throw the identical envelope.
18+
19+
<!-- 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 was a self-contradictory input whose declared condition was never evaluated; deciding which of the two ids the caller meant is a per-site intent decision a mechanical rewrite must not make, and the refusal text itself names both call-site fixes. -->

content/docs/references/api/contract.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data);
2727

2828
| Property | Type | Required | Description |
2929
| :--- | :--- | :--- | :--- |
30-
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +284 more>` || Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
30+
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +285 more>` || Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
3131
| **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112, #9106) |
3232
| **message** | `string` || Readable error message |
3333
| **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim (#9934). Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution (#3821) for anything unmarked. Status-agnostic; never replaces `message`. |
@@ -321,6 +321,7 @@ const result = ApiErrorSchema.parse(data);
321321
* `UNSUPPORTED`
322322
* `UNSUPPORTED_QUERY_PARAM`
323323
* `UNSUPPORTED_TRANSFORM`
324+
* `UPDATE_ID_MISMATCH`
324325
* `UPLOAD_SESSION_EXPIRED`
325326
* `UPLOAD_SESSION_NOT_FOUND`
326327
* `USER_ALREADY_EXISTS`

content/docs/references/api/error-code-ledger.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ const result = ErrorCode.parse(data);
425425
* `UNSUPPORTED`
426426
* `UNSUPPORTED_QUERY_PARAM`
427427
* `UNSUPPORTED_TRANSFORM`
428+
* `UPDATE_ID_MISMATCH`
428429
* `UPLOAD_SESSION_EXPIRED`
429430
* `UPLOAD_SESSION_NOT_FOUND`
430431
* `USER_ALREADY_EXISTS`

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

0 commit comments

Comments
 (0)