fix(objectql,metadata-core,service-messaging): refuse a by-id write whose where carries predicate keys the by-id path drops (#11009)

.changeset/by-id-unhonoured-predicate-refusal.md

@@ -0,0 +1,21 @@
+---
+"@objectstack/metadata-core": minor
+"@objectstack/objectql": minor
+"@objectstack/service-messaging": minor
+---
+
+**BREAKING (accept-set tightening)**: a by-id `update`/`delete` whose `options.where` carries predicate keys beyond `id` is now refused loudly instead of silently dropping the predicate (#11009).
+
+The by-id dispatch routes to `driver.update(object, id, …)` / `driver.delete(object, id, …)`, which bind ONLY the primary key — every other `where` key was discarded with no diagnostic. A compare-and-set written as `{ where: { id, status: { $in: [...] } }, multi: false }` therefore evaluated to nothing and the write landed unconditionally, reading exactly like a working conditional write. Measured on `better-sqlite3` through a real `ObjectQL` + `SqlDriver`: `SqlHttpOutbox.redeliver`'s terminal-status guard was inert, so a delivery row claimed `in_flight` mid-redeliver was reset anyway and the redelivery reported success while the in-flight attempt kept running.
+
+What changes, per call shape (`resolveEngineUpdateDispatch` / `resolveEngineDeleteDispatch`, so every pinned test double inherits the same verdicts):
+
+- A `where` naming a scalar `id` **and nothing else** is unchanged — by-id, with or without `multi: true` (the `LifecycleService` guarded-reap idiom keeps its per-record cascade path).
+- A `where` carrying a scalar `id` **plus other keys**, with a declared `multi: true` (id sourced from `where`): now routes to the **predicate path** (`driver.updateMany` / `driver.deleteMany`), which compiles EVERY `where` key — the compare-and-set spelling. Previously this dispatched by-id and dropped the extra keys.
+- The same shape **without** `multi: true` — and any by-id call via a scalar `data.id` beside extra `where` keys, `multi` or not (the payload id outranks `multi` per #5748 and cannot be demoted onto the predicate path): now **throws**, naming the keys the by-id path would have dropped. Previously it succeeded with the condition ignored.
+
+A caller hitting the new refusal decides which of two things it meant, and each is a one-line edit at the call site: declare the predicate path (`multi: true`) so the full `where` is honoured and the result is the matched count, or drop the extra `where` keys to keep an unconditional single-row write. Flow authors reach this through `update_record` / `delete_record` nodes whose `filter` names `id` plus other keys without declaring `multi` — those configs were silently unconditional before and refuse loudly now.
+
+`SqlHttpOutbox.redeliver` itself now rides the predicate path, and `MemoryHttpOutbox.redeliver` re-checks terminal status after its guard, so both `IHttpOutbox` implementations agree: a row claimed between redeliver's read and its write is NOT reset, and `redeliver` reports `DELIVERY_NOT_ELIGIBLE` instead of success.
+
+<!-- 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 shapes were silently broken before this change (their declared condition was never evaluated); the refusal text itself names the two call-site choices, and choosing between them is a per-site intent decision (conditional vs unconditional write) that a mechanical rewrite must not make. -->

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

@@ -54,7 +54,9 @@
  * `objectql -> metadata-core` and `metadata-protocol -> metadata-core` both
  * pre-date this change, and this package's own dependencies are
  * `{ @objectstack/spec, zod }` — no `objectql`, so no new edge and no new cycle.
- * This module importing nothing at all is what makes that free.
+ * This module importing nothing from outside its own package is what makes
+ * that free (its one sibling import, the #11009 refusal shared with the
+ * update twin, adds no package edge).
  *
  * `@objectstack/objectql` re-exports every symbol below from its original path,
  * so the 24+ call sites already pinned to it, and the public API, are unchanged.
@@ -72,6 +74,16 @@
  * - otherwise → **`reject`**. The call names neither one row nor a bulk
  * intent, and the engine throws rather than guessing.
  *
+ * **[#11009] One overriding clause on the by-id arm** (byte-for-byte the
+ * update twin's, minus the payload door `delete` does not have): the by-id
+ * route binds ONLY the primary key, so a `where` carrying any key besides
+ * `id` is a predicate the by-id path would silently discard. Such a call is
+ * never dispatched `by-id` — with `multi` truthy it is `multi` (the full
+ * `where`, scalar id included, rides the AST to `driver.deleteMany` — the
+ * compare-and-set spelling), otherwise it is `reject`, naming the dropped
+ * keys. A PURE-id `where` (`{ where: { id } }`) is untouched: it stays
+ * `by-id` even beside `multi: true` (LifecycleService's guarded-reap idiom).
+ *
  * Two halves of that first line are load-bearing, and a hand-written double
  * drops one or the other — which is the whole argument for importing this
  * instead of copying it:
@@ -112,6 +124,11 @@
  * @see scripts/check-engine-double-contract.mjs — the gate that keeps doubles on it.
  */
 
+import {
+ engineByIdUnhonouredPredicateMessage,
+ unhonouredByIdPredicateKeys,
+} from './engine-dispatch-unhonoured-predicate.js';
+
 /** The message `delete()` throws when a call identifies neither one row nor a bulk intent. */
 export const ENGINE_DELETE_REJECT_MESSAGE = 'Delete requires an ID or options.multi=true';
 
@@ -177,7 +194,32 @@ export function resolveEngineDeleteDispatch(
  // `!== undefined`, so a falsy scalar id (`0`, `''`) is not an identifying
  // call and falls down the same ladder as a non-scalar one. See header
  // point 2, and the twin's point 3 (objectstack#5747 / objectstack#5748).
- if (id) return { kind: 'by-id', id };
+ if (id) {
+ // [#11009] The by-id route binds ONLY the primary key, so a `where`
+ // carrying any key besides `id` is a predicate the by-id path would
+ // silently discard (the guard evaluated to nothing — the shape the update
+ // twin measured on `SqlHttpOutbox.redeliver`). Byte-for-byte the twin's
+ // rule, minus the payload arm `delete` does not have:
+ //
+ // - `multi` truthy → `multi`: a declared predicate call; the full
+ // `where` (scalar id included, as an equality term) rides the AST to
+ // `driver.deleteMany`. The compare-and-set spelling. ⚠️ A PURE-id
+ // `where` never reaches this branch and stays by-id even under
+ // `multi: true` — LifecycleService's guarded reap depends on that for
+ // per-record cascade handling, and the predicate path deliberately
+ // trades cascade for an honoured predicate.
+ // - otherwise → `reject`, loudly naming the keys that would have been
+ // dropped.
+ const unhonoured = unhonouredByIdPredicateKeys(options?.where);
+ if (unhonoured.length > 0) {
+ if (options?.multi) return { kind: 'multi' };
+ return {
+ kind: 'reject',
+ message: engineByIdUnhonouredPredicateMessage('Delete', unhonoured),
+ };
+ }
+ return { kind: 'by-id', id };
+ }
  if (options?.multi) return { kind: 'multi' };
  return { kind: 'reject', message: ENGINE_DELETE_REJECT_MESSAGE };
 }
@@ -225,10 +267,18 @@ export interface EngineDeleteDispatchCase {
 export const ENGINE_DELETE_DISPATCH_CASES: readonly EngineDeleteDispatchCase[] = [
  { what: 'scalar string id', options: { where: { id: 'rec_1' } }, expect: 'by-id' },
  { what: 'scalar number id', options: { where: { id: 42 } }, expect: 'by-id' },
- { what: 'scalar id alongside other predicates', options: { where: { id: 'rec_1', tenant: 't1' } }, expect: 'by-id' },
+ // [#11009] A PURE-id `where` stays by-id even under a declared `multi` —
+ // there is no predicate the by-id path could drop, and LifecycleService's
+ // guarded reap relies on this shape for per-record cascade handling
+ // (`engine-data-events.test.ts` pins the event contract of the same shape).
+ { what: 'scalar where.id with multi:true and NOTHING else in where — still one by-id delete (#11009)', options: { where: { id: 'rec_1' }, multi: true }, expect: 'by-id' },
  { what: 'multi with a predicate', options: { where: { rule_id: 'r1' }, multi: true }, expect: 'multi' },
  { what: 'multi with no predicate at all', options: { multi: true }, expect: 'multi' },
  { what: 'multi alongside an $in id set', options: { where: { id: { $in: ['a', 'b'] } }, multi: true }, expect: 'multi' },
+ // [#11009] The compare-and-set spelling: a scalar `where.id` beside real
+ // predicate keys WITH a declared `multi` is a predicate call — every key
+ // rides the AST to `driver.deleteMany`, so the condition is honoured.
+ { what: 'scalar where.id + extra predicate keys + multi:true — the predicate path honours ALL of it (#11009)', options: { where: { id: 'rec_1', status: 'stale' }, multi: true }, expect: 'multi' },
  // ── The FALSY scalars (objectstack#5747). `0` and `''` are scalars, so
  // `scalarDeleteId` returns them — but the engine's `if (input.id)` is a
  // truthiness test, so neither identifies a row. With a declared bulk
@@ -253,4 +303,10 @@ export const ENGINE_DELETE_DISPATCH_CASES: readonly EngineDeleteDispatchCase[] =
  { what: 'empty where, no multi', options: { where: {} }, expect: 'reject' },
  { what: 'no options at all', options: undefined, expect: 'reject' },
  { what: 'multi explicitly false with a predicate', options: { where: { rule_id: 'r1' }, multi: false }, expect: 'reject' },
+ // ── [#11009] The unhonoured-predicate refusals — the delete twin of the
+ // update-side cases. Each used to dispatch `by-id` and silently DISCARD
+ // every `where` key other than `id`; now the refusal names the dropped
+ // keys and prescribes the predicate path (`multi: true`).
+ { what: 'scalar where.id alongside other predicates, NO multi — the guard would be silently dropped (#11009)', options: { where: { id: 'rec_1', tenant: 't1' } }, expect: 'reject' },
+ { what: 'scalar where.id + a CAS operator predicate, multi explicitly false (#11009)', options: { where: { id: 'rec_1', status: { $in: ['done'] } }, multi: false }, expect: 'reject' },
 ];

packages/metadata-core/src/engine-dispatch-unhonoured-predicate.ts

@@ -0,0 +1,77 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The shared half of the objectstack#11009 refusal, imported by BOTH write
+ * dispatch modules (`engine-update-dispatch.ts` / `engine-delete-dispatch.ts`)
+ * so the two verbs cannot drift apart on what an unhonoured by-id predicate
+ * is, or on the words that refuse one.
+ *
+ * ## The defect this refuses
+ *
+ * A by-id dispatch routes to `driver.update(object, id, …)` /
+ * `driver.delete(object, id, …)` — entry points that bind ONLY the primary
+ * key (plus tenant scope). Every other key the caller wrote into
+ * `options.where` is silently discarded there: a compare-and-set guard
+ * (`{ where: { id, status: { $in: [...] } } }`) evaluates to nothing, the
+ * write lands unconditionally, and nothing reports that the declared
+ * condition never ran. Measured on `better-sqlite3` through a real
+ * `ObjectQL` + `SqlDriver` (objectstack#11009); the concrete victim was
+ * `SqlHttpOutbox.redeliver`'s terminal-status guard, which this repo's
+ * dispatcher could race and overwrite mid-flight while `redeliver` reported
+ * success.
+ *
+ * The refusal is the same answer this dispatch family has ruled twice before
+ * (objectstack#5748's operator-object `data.id`, objectstack#6435), and the
+ * same one `ENGINE_UPDATE_OPTION_KEYS` gives unknown option keys (#4371): a
+ * declaration the engine will not honour is refused at authoring time, never
+ * evaluated to nothing.
+ *
+ * ## What is NOT refused
+ *
+ * - A pure primary-key `where` (`{ where: { id } }`), with or without
+ * `multi` — the by-id path honours it in full. `multi: true` beside a
+ * pure-id `where` stays a BY-ID call: `LifecycleService`'s guarded reap
+ * relies on exactly that shape for per-record cascade handling, and
+ * `engine-data-events.test.ts` pins it.
+ * - A `where` carrying extra keys WITH a declared `multi: true` (id sourced
+ * from `where`) — that is a real predicate call, routed to the predicate
+ * path (`driver.updateMany` / `driver.deleteMany`), where `applyFilters`
+ * compiles EVERY key. This is the compare-and-set spelling the refusal
+ * message prescribes.
+ */
+
+/**
+ * The `options.where` keys a by-id dispatch would silently discard: every own
+ * enumerable key other than `id`.
+ *
+ * `null`-valued keys COUNT, deliberately — unlike the options bag (where
+ * `null` is a withdrawal, #4371), a `null` inside `where` is a real predicate
+ * (`status IS NULL`), so dropping it loses declared intent.
+ */
+export function unhonouredByIdPredicateKeys(where: unknown): string[] {
+ if (!where || typeof where !== 'object') return [];
+ return Object.keys(where as Record<string, unknown>).filter((k) => k !== 'id');
+}
+
+/**
+ * The message a by-id dispatch carrying unhonoured predicate keys is refused
+ * with — one composer for both verbs, and deliberately blind to WHICH source
+ * supplied the id (`data.id` or `where.id`): the #5748 symmetry property says
+ * the same call shape verdicts alike through either door, so the words must
+ * too.
+ */
+export function engineByIdUnhonouredPredicateMessage(
+ verb: 'Update' | 'Delete',
+ keys: readonly string[],
+): string {
+ const list = keys.map((k) => `'${k}'`).join(', ');
+ return (
+ `${verb} names one row by primary key, but options.where also carries ` +
+ `${keys.length > 1 ? 'predicate keys' : 'the predicate key'} ${list}. The by-id path binds ONLY ` +
+ `the id — the driver never evaluates the remaining predicate — so the call would succeed with ` +
+ `the declared condition silently ignored (#11009). For a conditional (compare-and-set) write, ` +
+ `declare the predicate path, which honours EVERY where key: { where: { id, ${keys.join(', ')} }, ` +
+ `multi: true } writes at most the one row matching ALL predicates and reports the matched count. ` +
+ `For an unconditional single-row write, drop the extra where keys.`
+ );
+}