From e7b6ff5ba42d385e26833ebbfa24185dd58ec8b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 03:42:50 +0000 Subject: [PATCH 1/2] fix(objectql,metadata-core): refuse a by-id update whose scalar where.id names a different row than the payload id (#11142) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .../src/engine-update-dispatch.ts | 160 +++++++++++++++++- .../engine-update-by-id-payload-id.test.ts | 49 ++++-- .../src/engine-update-dispatch.test.ts | 118 ++++++++++++- .../objectql/src/engine-update-dispatch.ts | 8 + packages/objectql/src/engine.ts | 24 ++- .../spec/src/api/error-code-ledger.zod.ts | 11 ++ 6 files changed, 349 insertions(+), 21 deletions(-) diff --git a/packages/metadata-core/src/engine-update-dispatch.ts b/packages/metadata-core/src/engine-update-dispatch.ts index 0eeba2ee9e..384067f187 100644 --- a/packages/metadata-core/src/engine-update-dispatch.ts +++ b/packages/metadata-core/src/engine-update-dispatch.ts @@ -70,6 +70,27 @@ * stays `by-id` even beside `multi: true` (LifecycleService's guarded-reap * idiom — pinned in the case-set below). * + * **[#11142] A second overriding clause, on the PAYLOAD-sourced by-id arm:** + * a truthy scalar `options.where.id` naming a DIFFERENT row than the bound + * payload id is refused (`UPDATE_ID_MISMATCH`, 400) rather than silently + * discarded. `where.id` is a declared predicate exactly like #11009's extra + * keys — the caller wrote "update where id = ", a + * condition that can never hold — and the by-id path would have dropped it + * with no diagnostic. This deliberately reverses the #5748-pinned verdict + * (`a SCALAR data.id still wins over a scalar where.id`) for the UNEQUAL + * shape ONLY (maintainer ruling on #11142, 2026-08-23): + * + * - `data.id === where.id` stays `by-id`, untouched — the REST ingress folds + * the path id into the payload, so the redundant-but-agreeing spelling is + * a normal one (pinned below). + * - A FALSY scalar `where.id` (`0`, `''`) conflicts with nothing: a falsy id + * never identifies a row anywhere on this ladder (point 3 below), so there + * is no second row address to disagree with. + * - A NON-scalar `where.id` (`{ $in: [...] }`, an array, `null`) beside a + * scalar payload id keeps its #5748 verdict (`by-id`, payload wins) — that + * pin was not reversed, and widening over it is a separate decision, not a + * rider here (#6435's own boundary, one shape over). + * * Three things about that list are load-bearing and easy to get wrong when * copying it by hand — which is the whole argument for importing it instead: * @@ -83,7 +104,11 @@ * 2. **`data.id` outranks `where.id`, but only when it IS an id.** The payload * is read first, so a scalar `data.id` still wins over `where` and over an * explicit `multi: true`; `update(o, { id: 'rec_1', … }, { multi: true })` - * is one by-id write, unchanged. What a non-scalar `data.id` no longer does + * is one by-id write, unchanged. Since #11142, "wins over `where`" no + * longer includes silently overriding a truthy scalar `where.id` that + * names a DIFFERENT row — that call is refused (see the clause above); + * what stays is precedence, not the silent drop. What a non-scalar + * `data.id` no longer does * is *outrank* anything: it is not an id, so the decision falls through to * `where.id`, then to `multi`, then to `reject` — exactly the ladder a * non-scalar `where.id` falls down. Until objectstack#5748 the payload half @@ -126,14 +151,39 @@ import { /** The message `update()` throws when a call identifies neither one row nor a bulk intent. */ export const ENGINE_UPDATE_REJECT_MESSAGE = 'Update requires an ID or options.multi=true'; +/** + * [#11142] The `error.code` of the conflicting-id refusal: a by-id update + * whose truthy scalar `options.where.id` names a different row than the bound + * payload `data.id`. Registered in the spec's `ERROR_CODE_LEDGER` (ADR-0112) + * under `@objectstack/objectql`, the production thrower; travels with + * {@link ENGINE_UPDATE_ID_CONFLICT_STATUS} on the thrown error's own property + * bag (the `recordNotFoundError` convention), so the REST boundary's + * status/code passthrough answers 400 instead of a sanitised 500. + */ +export const ENGINE_UPDATE_ID_CONFLICT_CODE = 'UPDATE_ID_MISMATCH'; + +/** [#11142] The HTTP status the conflicting-id refusal declares: a caller error, 400. */ +export const ENGINE_UPDATE_ID_CONFLICT_STATUS = 400; + /** What `ObjectQLEngine.update` will do with a given `(data, options)` pair. */ export type EngineUpdateDispatch = /** A truthy scalar `data.id`, or a truthy scalar `where.id` — `driver.update`. */ | { readonly kind: 'by-id'; readonly id: unknown } /** No single id but `options.multi` — `driver.updateMany` with the composed AST. */ | { readonly kind: 'multi' } - /** Neither — the engine throws `ENGINE_UPDATE_REJECT_MESSAGE`. */ - | { readonly kind: 'reject'; readonly message: string }; + /** + * Neither — the engine throws `ENGINE_UPDATE_REJECT_MESSAGE`, the #11009 + * unhonoured-predicate message, or (#11142) the conflicting-id message. + * + * `code`/`status` are present only when the refusal declares an ADR-0112 + * envelope of its own (today: the #11142 conflict, + * {@link ENGINE_UPDATE_ID_CONFLICT_CODE} / 400). The #5748 / #11009 + * refusals deliberately stay undecorated — adding an envelope to them is a + * wire-contract change this module must not make by side effect. Throwers + * go through {@link engineUpdateDispatchRejectError} so the decoration has + * one spelling. + */ + | { readonly kind: 'reject'; readonly message: string; readonly code?: string; readonly status?: number }; /** The subset of `EngineUpdateOptions` the dispatch decision actually reads. */ export interface EngineUpdateDispatchInput { @@ -192,6 +242,60 @@ export function scalarUpdateId( return asScalarId((where as Record).id); } +/** + * How a scalar id is quoted inside a refusal message: strings keep their + * quotes so `'42'` (a string) and `42` (a number) stay visibly different — + * the strict-identity conflict test below treats them as different ids, and + * the message must let a reader see why. `JSON.stringify` is not used because + * it throws on `bigint`. + */ +function spellScalarId(value: string | number | bigint): string { + return typeof value === 'string' ? `'${value}'` : String(value); +} + +/** + * [#11142] The message a by-id update is refused with when its truthy scalar + * `options.where.id` names a different row than the bound payload `data.id`. + * + * The same defect class as the #11009 refusal one clause over: a declared + * predicate the by-id path would silently discard — here the predicate IS the + * primary key, spelled twice with two different values, a condition that can + * never hold. Refusing it deliberately reverses the #5748-pinned verdict for + * the UNEQUAL shape only (maintainer ruling on #11142, 2026-08-23); the + * equal-ids spelling (the REST ingress folds the path id into the payload) + * stays honoured. + */ +export function engineUpdateIdConflictMessage( + payloadId: string | number | bigint, + whereId: string | number | bigint, +): string { + return ( + `Update binds the payload id ${spellScalarId(payloadId)} as the row address, but options.where.id ` + + `names a DIFFERENT row: ${spellScalarId(whereId)}. The by-id path binds ONLY one primary key — the ` + + `losing spelling is never evaluated — so the write would land on ${spellScalarId(payloadId)} with the ` + + `where.id condition silently ignored (#11142). Two ids are the same row only when they are identical, ` + + `type included. If both spellings mean one row, make them equal; otherwise drop one: ` + + `update(object, { id, ...fields }) addresses the row by the payload id, and ` + + `update(object, fields, { where: { id } }) — with no id in the payload — addresses it by where.id.` + ); +} + +/** + * The one spelling of "throw a `reject` verdict" (#11142) — used by + * {@link assertEngineUpdateDispatch} and by `ObjectQL.update` itself, so a + * pinned fake's refusal carries exactly the envelope the real engine's does. + * Rejects without a declared `code`/`status` (the #5748 / #11009 family) stay + * plain `Error`s, byte-identical to what both threw before this helper. + */ +export function engineUpdateDispatchRejectError( + reject: Extract, +): Error { + const err = new Error(reject.message) as Error & { code?: string; status?: number }; + if (reject.code !== undefined) err.code = reject.code; + if (reject.status !== undefined) err.status = reject.status; + return err; +} + /** * Decide what `ObjectQLEngine.update` does with `(data, options)`, without * doing it. @@ -251,6 +355,32 @@ export function resolveEngineUpdateDispatch( message: engineByIdUnhonouredPredicateMessage('Update', unhonoured), }; } + // [#11142] The payload id won the ladder, and `where.id` — the only + // `where` key left after the #11009 check above — is itself a truthy + // scalar naming a DIFFERENT row. That is a predicate the by-id path would + // silently discard, exactly like #11009's extra keys: "update + // where id = " can never hold, and the write used to land on the + // payload row with no diagnostic (the #5748-pinned verdict, reversed for + // this UNEQUAL shape only by the maintainer ruling on #11142). Strict + // identity on purpose: `42` and `'42'` are two ids until the caller says + // otherwise, and a coercing comparison here would be the lenient-consumer + // move Prime Directive #12 forbids. A declared `multi: true` cannot + // rescue the call — the payload id outranks `multi` (#5748), so the + // conflict stands wherever the flag sits. Falsy and non-scalar `where.id` + // shapes never reach this check with a conflict verdict: a falsy scalar + // identifies no row (header point 3), and a non-scalar keeps its #5748 + // by-id verdict (widening over it is a separate decision). + if (payloadId) { + const conflictingWhereId = scalarUpdateId(options); + if (conflictingWhereId && conflictingWhereId !== payloadId) { + return { + kind: 'reject', + message: engineUpdateIdConflictMessage(payloadId, conflictingWhereId), + code: ENGINE_UPDATE_ID_CONFLICT_CODE, + status: ENGINE_UPDATE_ID_CONFLICT_STATUS, + }; + } + } return { kind: 'by-id', id }; } if (options?.multi) return { kind: 'multi' }; @@ -277,7 +407,11 @@ export function assertEngineUpdateDispatch( options?: EngineUpdateDispatchInput | null, ): Exclude { const dispatch = resolveEngineUpdateDispatch(data, options); - if (dispatch.kind === 'reject') throw new Error(dispatch.message); + // [#11142] Through the shared helper, so a refusal that declares an + // ADR-0112 `code`/`status` (the conflicting-id reject) carries it here + // exactly as it does from the real engine; undecorated rejects throw the + // same plain `Error` they always have. + if (dispatch.kind === 'reject') throw engineUpdateDispatchRejectError(dispatch); return dispatch; } @@ -327,7 +461,23 @@ export const ENGINE_UPDATE_DISPATCH_CASES: readonly EngineUpdateDispatchCase[] = // spelling and objectstack#5748 left it exactly as it was. { 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' }, { 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' }, - { 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' }, + // [#11142] The REVERSED #5748 pin. This row read + // `expect: 'by-id', expectId: 'rec_1'` from #5748 until the maintainer + // ruling on #11142 (2026-08-23) flipped the UNEQUAL shape to a refusal: a + // truthy scalar `where.id` naming a different row than the payload id is a + // predicate the by-id path would silently discard — the last silent member + // of the #5748/#11009 dropped-declaration family. The pin flips, it does + // not disappear; the EQUAL spelling keeps its own passing pin right below. + { 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' }, + // [#11142] The equal-ids spelling stays honoured: the REST ingress folds + // the path id into the payload (`{ ...data, id: request.id }` beside + // `where: { id: request.id }`), so redundant-but-agreeing is a NORMAL + // spelling, not a conflict. + { 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' }, + // [#11142] `multi: true` cannot rescue the conflict: the payload id outranks + // `multi` (#5748), so the call is still a by-id write carrying a where.id it + // can never honour. + { 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' }, // ── The payload's scalar test (objectstack#5748). A non-scalar `data.id` // names no row, so it stops shadowing everything under it: the decision // falls through to `where.id`, then `multi`, then `reject`. Before #5748 diff --git a/packages/objectql/src/engine-update-by-id-payload-id.test.ts b/packages/objectql/src/engine-update-by-id-payload-id.test.ts index 3d4fe2ba29..9f13540de9 100644 --- a/packages/objectql/src/engine-update-by-id-payload-id.test.ts +++ b/packages/objectql/src/engine-update-by-id-payload-id.test.ts @@ -62,7 +62,12 @@ import { describe, it, expect } from 'vitest'; import type { EngineUpdateOptions } from '@objectstack/spec/data'; import { ObjectQL } from './engine.js'; -import { assertEngineUpdateDispatch } from './engine-update-dispatch.js'; +import { + assertEngineUpdateDispatch, + ENGINE_UPDATE_ID_CONFLICT_CODE, + ENGINE_UPDATE_ID_CONFLICT_STATUS, + engineUpdateIdConflictMessage, +} from './engine-update-dispatch.js'; interface RecordedCall { readonly fn: 'update' | 'updateMany'; @@ -271,15 +276,39 @@ describe('#6435 — the contrast pins: what this change deliberately does NOT to expect(call.data).toEqual({ id: 'rec_1', title: 'x' }); }); - it('a truthy scalar data.id that DISAGREES with where.id still wins, payload as sent', async () => { - // `ENGINE_UPDATE_DISPATCH_CASES`: "a SCALAR data.id still wins over a - // scalar where.id" ⇒ bound id `rec_1`, not `rec_2`. Unchanged here. - const call = await observeWrite( - { id: 'rec_1', title: 'x' }, - { where: { id: 'rec_2' } }, - { fn: 'update', boundId: 'rec_1' }, - ); - expect(call.data).toEqual({ id: 'rec_1', title: 'x' }); + it('a truthy scalar data.id that DISAGREES with where.id is REFUSED — the #11142 reversal of the #5748 pin', async () => { + // FLIPPED, not deleted (#11142, maintainer ruling 2026-08-23). This pin + // read "still wins, payload as sent" — `ENGINE_UPDATE_DISPATCH_CASES`'s + // `a SCALAR data.id still wins over a scalar where.id`, bound id `rec_1` + // — from #5748 until the UNEQUAL shape was ruled a refusal: the losing + // `where.id` was a declared predicate the by-id path silently discarded. + // The refusal carries the ADR-0112 envelope halves, and NOTHING reaches + // the driver. The equal-ids contrast pin above is the surviving half of + // the old behaviour. + const message = engineUpdateIdConflictMessage('rec_1', 'rec_2'); + let caught: any; + try { + assertEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: 'rec_2' } }); + } catch (e) { + caught = e; + } + expect(caught, 'expected the conflicting-id refusal, but the dispatch resolved').toBeDefined(); + expect(caught.code).toBe(ENGINE_UPDATE_ID_CONFLICT_CODE); + expect(caught.status).toBe(ENGINE_UPDATE_ID_CONFLICT_STATUS); + expect(caught.message).toBe(message); + + const { engine, calls } = await makeEngine(); + let engineCaught: any; + try { + await engine.update('task', { id: 'rec_1', title: 'x' }, { where: { id: 'rec_2' } }); + } catch (e) { + engineCaught = e; + } + expect(engineCaught, 'expected the real engine to refuse too').toBeDefined(); + expect(engineCaught.code).toBe(ENGINE_UPDATE_ID_CONFLICT_CODE); + expect(engineCaught.status).toBe(ENGINE_UPDATE_ID_CONFLICT_STATUS); + expect(engineCaught.message).toBe(message); + expect(calls, 'a refused write reaches no driver entry point').toEqual([]); }); it('the MULTI arm is exactly as PR #6433 left it', async () => { diff --git a/packages/objectql/src/engine-update-dispatch.test.ts b/packages/objectql/src/engine-update-dispatch.test.ts index 8eab7a7f30..58a3eefdf0 100644 --- a/packages/objectql/src/engine-update-dispatch.test.ts +++ b/packages/objectql/src/engine-update-dispatch.test.ts @@ -21,10 +21,13 @@ import { ObjectQL } from './engine.js'; import { ENGINE_UPDATE_DISPATCH_CASES, ENGINE_UPDATE_REJECT_MESSAGE, + ENGINE_UPDATE_ID_CONFLICT_CODE, + ENGINE_UPDATE_ID_CONFLICT_STATUS, resolveEngineUpdateDispatch, assertEngineUpdateDispatch, scalarUpdateId, engineByIdUnhonouredPredicateMessage, + engineUpdateIdConflictMessage, unhonouredByIdPredicateKeys, } from './engine-update-dispatch.js'; @@ -195,7 +198,11 @@ describe('engine update dispatch — the shared predicate IS the engine (#5480)' it('a SCALAR data.id still outranks where and multi (the common legal spelling, untouched by #5748)', () => { expect(resolveEngineUpdateDispatch({ id: 'rec_1' }, { where: { id: { $in: ['a'] } }, multi: true })) .toEqual({ kind: 'by-id', id: 'rec_1' }); - expect(resolveEngineUpdateDispatch({ id: 'rec_1' }, { where: { id: 'rec_2' } })) + // [#11142] `{ id: 'rec_1' }` beside `{ where: { id: 'rec_2' } }` used to + // sit here as a by-id assertion; the UNEQUAL scalar pair is now refused + // (the reversed #5748 pin — see the #11142 describe below). The EQUAL + // pair stays the honoured spelling: + expect(resolveEngineUpdateDispatch({ id: 'rec_1' }, { where: { id: 'rec_1' } })) .toEqual({ kind: 'by-id', id: 'rec_1' }); expect(resolveEngineUpdateDispatch({ id: 42, title: 'x' }, { multi: true })) .toEqual({ kind: 'by-id', id: 42 }); @@ -328,3 +335,112 @@ describe('engine update dispatch — the shared predicate IS the engine (#5480)' expect(() => resolveEngineUpdateDispatch(undefined as any, { multi: true })).toThrow(TypeError); }); }); + +// ── [#11142] The reversed #5748 pin: a truthy scalar `where.id` naming a +// DIFFERENT row than the bound payload id is refused, never silently +// dropped. Maintainer ruling 2026-08-23 — the UNEQUAL shape only; the +// equal-ids spelling (REST folds the path id into the payload) stays +// honoured, and the falsy / non-scalar `where.id` boundaries keep their +// pre-existing verdicts (truthiness rule; un-reversed #5748 pin). +describe('[#11142] a conflicting scalar where.id beside the payload id is refused', () => { + const data = { id: 'rec_1', title: 'x' }; + const options = { where: { id: 'rec_2' } }; + const message = engineUpdateIdConflictMessage('rec_1', 'rec_2'); + + it('the predicate refuses with the declared message, code and status', () => { + expect(resolveEngineUpdateDispatch(data, options)).toEqual({ + kind: 'reject', + message, + code: ENGINE_UPDATE_ID_CONFLICT_CODE, + status: ENGINE_UPDATE_ID_CONFLICT_STATUS, + }); + // The envelope halves are contract (ADR-0112): `code` is registered in the + // spec ledger, `status` rides the REST boundary's declared-status + // passthrough. Pinned by value so a rename or a demotion to 500 is a + // deliberate act here, never drift. + expect(ENGINE_UPDATE_ID_CONFLICT_CODE).toBe('UPDATE_ID_MISMATCH'); + expect(ENGINE_UPDATE_ID_CONFLICT_STATUS).toBe(400); + }); + + it('assertEngineUpdateDispatch throws it carrying code + status (every pinned fake inherits this)', () => { + let caught: any; + try { + assertEngineUpdateDispatch(data, options); + } catch (e) { + caught = e; + } + expect(caught, 'expected the conflicting-id refusal, but the call resolved').toBeDefined(); + expect(caught.code).toBe(ENGINE_UPDATE_ID_CONFLICT_CODE); + expect(caught.status).toBe(ENGINE_UPDATE_ID_CONFLICT_STATUS); + expect(caught.message).toBe(message); + }); + + it('the REAL engine throws the same envelope and NOTHING reaches the driver', async () => { + const { engine, calls } = await makeEngine(); + let caught: any; + try { + await engine.update('task', data as any, options as any); + } catch (e) { + caught = e; + } + expect(caught, 'expected the conflicting-id refusal, but the call resolved').toBeDefined(); + expect(caught.code).toBe(ENGINE_UPDATE_ID_CONFLICT_CODE); + expect(caught.status).toBe(ENGINE_UPDATE_ID_CONFLICT_STATUS); + expect(caught.message).toBe(message); + // The refused write wrote NOTHING — the entire point: the pre-#11142 + // behaviour wrote rec_1 with the rec_2 condition silently ignored. + expect(calls).toEqual([]); + }); + + it('multi:true cannot rescue the conflict — the payload id outranks multi (#5748), so the contradiction stands', () => { + const verdict = resolveEngineUpdateDispatch(data, { where: { id: 'rec_2' }, multi: true }); + expect(verdict.kind).toBe('reject'); + expect((verdict as { message?: string }).message).toBe(message); + }); + + it('EQUAL ids stay by-id — the REST spelling (path id folded into the payload) is untouched', async () => { + expect(resolveEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: 'rec_1' } })) + .toEqual({ kind: 'by-id', id: 'rec_1' }); + const observed = await observeEngine({ id: 'rec_1', title: 'x' }, { where: { id: 'rec_1' } }); + expect(observed.kind).toBe('by-id'); + expect(observed.boundId).toBe('rec_1'); + }); + + it('identity is STRICT — a string id and a number id are two ids, and the message shows the type difference', () => { + const verdict = resolveEngineUpdateDispatch({ id: 42, title: 'x' }, { where: { id: '42' } }); + expect(verdict.kind).toBe('reject'); + // `'42'` (quoted) vs `42` (bare) — the reader can see the mismatch is one + // of type, not of row name. A coercing comparison here would be the + // lenient-consumer move Prime Directive #12 forbids. + expect((verdict as { message?: string }).message).toBe(engineUpdateIdConflictMessage(42, '42')); + expect((verdict as { message?: string }).message).toContain("'42'"); + }); + + it('a FALSY scalar where.id conflicts with nothing — a falsy id identifies no row on this ladder (out of the #11142 ruled scope)', () => { + // NOT part of the reversal: `0` / `''` never identify a row (header + // point 3), so there is no second row address to contradict. Pinned so + // the refusal cannot creep over the truthiness boundary without a ruling. + expect(resolveEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: 0 } })) + .toEqual({ kind: 'by-id', id: 'rec_1' }); + expect(resolveEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: '' } })) + .toEqual({ kind: 'by-id', id: 'rec_1' }); + }); + + it('a NON-SCALAR where.id keeps its #5748 verdict — the payload id wins (out of the #11142 ruled scope)', () => { + // The un-reversed neighbour pin, restated from the ruling's own boundary: + // widening the refusal over operator-object / array / null `where.id` + // beside a scalar payload id is a separate decision, not a rider here. + expect(resolveEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: { $in: ['a'] } }, multi: true })) + .toEqual({ kind: 'by-id', id: 'rec_1' }); + expect(resolveEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: null } })) + .toEqual({ kind: 'by-id', id: 'rec_1' }); + }); + + it('the #11009 refusal keeps PRIORITY on a where that also carries extra keys (its message names them, unchanged)', () => { + // Both defects at once: `where` carries a conflicting id AND extra keys. + // The #11009 unhonoured-keys refusal fires first, byte-identical to + // before this change — no existing #11009 pin moves. + expect(resolveEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: 'rec_2', tenant: 't1' } })) + .toEqual({ kind: 'reject', message: engineByIdUnhonouredPredicateMessage('Update', ['tenant']) }); + }); +}); diff --git a/packages/objectql/src/engine-update-dispatch.ts b/packages/objectql/src/engine-update-dispatch.ts index ed012397df..d8b33df40e 100644 --- a/packages/objectql/src/engine-update-dispatch.ts +++ b/packages/objectql/src/engine-update-dispatch.ts @@ -26,6 +26,14 @@ export { // once, here, so `objectql` callers can quote the refusal verbatim. engineByIdUnhonouredPredicateMessage, unhonouredByIdPredicateKeys, + // [#11142] The conflicting-id refusal (a truthy scalar `where.id` naming a + // DIFFERENT row than the bound payload id): its message composer, its + // declared ADR-0112 code/status, and the one reject thrower the real engine + // and every pinned fake share. + ENGINE_UPDATE_ID_CONFLICT_CODE, + ENGINE_UPDATE_ID_CONFLICT_STATUS, + engineUpdateIdConflictMessage, + engineUpdateDispatchRejectError, } from '@objectstack/metadata-core'; export type { diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index ebb9c13e2a..bf1f6d0d07 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -194,6 +194,11 @@ import { import { resolveEngineUpdateDispatch, ENGINE_UPDATE_REJECT_MESSAGE, + // [#11142] The one reject thrower, shared with assertEngineUpdateDispatch, + // so a refusal that declares an ADR-0112 code/status (the conflicting-id + // reject) carries it identically from the real engine and from every + // pinned fake. + engineUpdateDispatchRejectError, type EngineUpdateDispatchData, type EngineUpdateDispatchInput, } from './engine-update-dispatch.js'; @@ -9569,7 +9574,13 @@ export class ObjectQL implements IObjectQLEngine { // the one non-reject way in here — a `multi` verdict against a // driver with no `updateMany`, which stays the generic refusal it // has always been. - throw new Error(dispatch.kind === 'reject' ? dispatch.message : ENGINE_UPDATE_REJECT_MESSAGE); + // [#11142] Rejects throw through the dispatch module's own + // thrower so a verdict carrying a declared code/status (the + // conflicting-id refusal) reaches the caller — and the REST + // boundary's status passthrough — decorated; the #5748/#11009 + // family stays a plain Error, byte-identical to before. + if (dispatch.kind === 'reject') throw engineUpdateDispatchRejectError(dispatch); + throw new Error(ENGINE_UPDATE_REJECT_MESSAGE); } // [#6966] The ladder verdict, stated on the contract. Bound HERE and @@ -9856,10 +9867,13 @@ export class ObjectQL implements IObjectQLEngine { // // - A TRUTHY SCALAR `data.id` is left exactly as it is. There // the payload's `id` IS the bound key (it outranks `where` — - // same case-set), so the write is `SET id = 'rec_1' WHERE id - // = 'rec_1'`: a same-value no-op, redundant rather than - // damaging, and long-standing behaviour. Widening the strip - // to cover it is a separate decision, not a rider here. + // same case-set; since #11142 a truthy scalar `where.id` + // naming a DIFFERENT row is refused at dispatch and never + // reaches this branch), so the write is `SET id = 'rec_1' + // WHERE id = 'rec_1'`: a same-value no-op, redundant rather + // than damaging, and long-standing behaviour. Widening the + // strip to cover it is a separate decision, not a rider + // here. // - Rejecting the call instead (#6435's route B) would reverse // the `expect: 'by-id'` verdict the case-set states today — // a partial rollback of #5748's ruling A, i.e. a maintainer diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index b940646963..bb9d5b6193 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -479,6 +479,17 @@ export const ERROR_CODE_LEDGER = { // been written (`TransactionUnsupportedError`, `transaction-errors.ts`; // ADR-0119 D1/D4 fail-closed posture). Same #8087-gate family. 'ERR_TRANSACTION_UNSUPPORTED', + // [#11142] a by-id update carried a truthy scalar `options.where.id` naming + // a DIFFERENT row than the bound payload `data.id` — a condition that can + // never hold, refused 400 at dispatch instead of silently writing the + // payload row (the reversed #5748 pin; equal ids — the REST path-id fold — + // stay honoured). Stamped by `@objectstack/metadata-core`'s + // `engineUpdateDispatchRejectError`, thrown in production by + // `ObjectQL.update` (`engine.ts`), hence registered here. Not a + // VALIDATION_ERROR synonym: the payload parses fine — the two row + // addresses contradict each other, the same mismatch class as + // QUERY_OBJECT_MISMATCH one layer up. + 'UPDATE_ID_MISMATCH', 'VALIDATION_FAILED', ], '@objectstack/core': [ From 12bac63b352468c923f3b3049e38defa0cd07697 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 04:03:26 +0000 Subject: [PATCH 2/2] 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 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .changeset/where-id-conflict-refusal.md | 19 +++++++++++++++++++ content/docs/references/api/contract.mdx | 3 ++- .../docs/references/api/error-code-ledger.mdx | 1 + 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 .changeset/where-id-conflict-refusal.md diff --git a/.changeset/where-id-conflict-refusal.md b/.changeset/where-id-conflict-refusal.md new file mode 100644 index 0000000000..09785bbbab --- /dev/null +++ b/.changeset/where-id-conflict-refusal.md @@ -0,0 +1,19 @@ +--- +"@objectstack/metadata-core": minor +"@objectstack/objectql": minor +"@objectstack/spec": minor +--- + +**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). + +`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. + +What changes, per call shape (`resolveEngineUpdateDispatch`, so every pinned test double inherits the same verdict): + +- `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. +- `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. +- 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. + +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. + + diff --git a/content/docs/references/api/contract.mdx b/content/docs/references/api/contract.mdx index 4f4f0a3680..6afae2efd7 100644 --- a/content/docs/references/api/contract.mdx +++ b/content/docs/references/api/contract.mdx @@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **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) | +| **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) | | **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) | | **message** | `string` | ✅ | Readable error message | | **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); * `UNSUPPORTED` * `UNSUPPORTED_QUERY_PARAM` * `UNSUPPORTED_TRANSFORM` +* `UPDATE_ID_MISMATCH` * `UPLOAD_SESSION_EXPIRED` * `UPLOAD_SESSION_NOT_FOUND` * `USER_ALREADY_EXISTS` diff --git a/content/docs/references/api/error-code-ledger.mdx b/content/docs/references/api/error-code-ledger.mdx index eb774d8487..ae46388da3 100644 --- a/content/docs/references/api/error-code-ledger.mdx +++ b/content/docs/references/api/error-code-ledger.mdx @@ -425,6 +425,7 @@ const result = ErrorCode.parse(data); * `UNSUPPORTED` * `UNSUPPORTED_QUERY_PARAM` * `UNSUPPORTED_TRANSFORM` +* `UPDATE_ID_MISMATCH` * `UPLOAD_SESSION_EXPIRED` * `UPLOAD_SESSION_NOT_FOUND` * `USER_ALREADY_EXISTS`