fix(objectql,spec): refuse a multi: true update whose per-row beforeUpdate hooks write divergent key sets (#14099)

.changeset/multi-update-hook-key-divergence-refusal.md

@@ -0,0 +1,81 @@
+---
+"@objectstack/objectql": minor
+"@objectstack/spec": minor
+---
+
+fix(objectql,spec): refuse a `multi: true` update whose per-row `beforeUpdate` hooks write divergent key sets (#14099)
+
+**BREAKING** accept-set narrowing on a published write path, shipped as `minor`
+under the repo's launch-window convention for breaking changes. A `multi: true`
+update that succeeds today is REFUSED when its `beforeUpdate` handlers assign
+different sets of payload keys to different matched rows.
+
+**What it fixes.** `driver.updateMany` takes one `SET` clause for N rows, so a
+predicate update has exactly one payload (ADR-0058 Addendum II D3) — whatever a
+`beforeUpdate` handler writes for one row was applied to every matched row. The
+transition stamp is the shape this breaks, and it is the standard way to record
+when a record entered a state:
+
+```ts
+// beforeUpdate — correct per record, silently wrong on a batch
+if (previous.status !== 'done' && next.status === 'done') patch.completed_at = now;
+```
+
+Measured against published `17.2.0`: two rows, one open and one completed
+earlier, updated in a single `multi: true` call. The already-completed row's
+`completed_at` moved from `…:26.560Z` to `…:26.571Z`. It never transitioned,
+nothing errored, and the corrupted row is byte-for-byte indistinguishable from
+one genuinely completed late — so every on-time measure reading the column turns
+a compliant record into a breach, with no audit entry and nothing in the data
+that shows it happened. The whole class is exposed: `approved_at`, `closed_at`,
+`shipped_at`, `first_responded_at`.
+
+**What changed.** The engine still dispatches the before phase once per matched
+row with that row's pre-image, and D3 still stands — the payload stays
+batch-scoped and the engine never splits its own write. It now also RECORDS,
+per row, the set of payload keys that row's hook chain assigned (the #14088
+provenance recorder, armed once more per row). If two rows disagree, the whole
+batch is refused before any write — not after the first row, not inside a
+transaction that then rolls back — with the ADR-0112 envelope
+`MULTI_UPDATE_HOOK_KEY_DIVERGENCE` (HTTP `400`,
+`MultiUpdateHookKeyDivergenceError`), naming the object, the diverging keys and
+the remedy. When every row's key set is identical the batch proceeds as one
+`updateMany`, exactly as before.
+
+**The criterion is the key SET, never the values.** That is what keeps honest
+batches honest: objectql's own `sys_stamp_audit_update` builtin is registered on
+`'*'` and reads the clock inside the per-record stamp, so an ordinary bulk
+update writes `updated_at` on every row with different values. Every in-repo
+`beforeUpdate` payload rewrite was measured on a mixed batch before this shipped
+— the audit stamp (`['updated_at','updated_by']` on every row), plugin-pinyin's
+companion projection (`['__search']` on every row) and service-storage's
+copy-on-claim (`[]` on every row) — and all three are row-invariant, so none of
+them is refused.
+
+**Migration — how to write a per-record rewrite on a batch.** Two supported
+routes, both available in this release:
+
+1. **Route 2, from inside the handler.** Write the affected records with
+   `ctx.api`, aimed with the per-row signals the hook sandbox now carries
+   (`ctx.dispatch.mode === 'per-row'`, `ctx.input.id`, `ctx.input.options`),
+   and leave the batch payload alone. ⚠️ Those signals are NOT in `17.2.0` —
+   they land in this same release, which is why the refusal and its
+   prescription ship together rather than the refusal arriving first.
+2. **By-id updates from the caller.** Issue the updates per record when the
+   value genuinely differs per record.
+
+`objectstack-ai/hotcrm` and `objectstack-ai/duly` both carry hooks of this
+shape and should take route 1: `duly`'s `duly_task.completed_at` stamp is the
+measured instance, and hotcrm's `previous`-reading handlers are the same family.
+
+**Known limit, carried openly rather than hidden.** A handler that writes the
+SAME key on every row but with a per-row VALUE (a per-row derived priority, say)
+still passes this test, and still applies the last dispatch's value to every
+matched row. That is D3's declared cost; the two routes above are the exit for
+it, and it is tracked as its own finding. ⛔ It is deliberately NOT closed by
+comparing values: a value comparison refuses honest audit-stamp batches
+non-deterministically (one clock read per row) and re-opens #14088's own
+`completed_at: null` row, where a hook that writes the value the caller also
+sent is indistinguishable from a hook that never touched the key.
+
+<!-- adr-0087: not-required (no-migration-prescription) A runtime accept-set narrowing on the engine's predicate-update path: no authorable metadata key is removed, renamed or re-shaped, so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. The affected artifact is HOOK BODY CODE, whose per-row intent no mechanical rewrite can infer — choosing between a `ctx.api` per-row write and by-id updates is an authoring decision. The refusal itself is the notification channel, raised at the write site with the object, the diverging keys and both routes in the envelope. -->

content/docs/permissions/system-context.mdx

@@ -109,18 +109,18 @@ that silently does not happen.
 
 | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
 |:--|:---|:---|:---|:---|
-| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11204` |
-| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11387` |
-| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9939` |
+| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11289` |
+| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11472` |
+| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10024` |
 | 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` |
-| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9987`, `readonly-strict-errors.ts:66` |
-| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5806` |
-| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3650`, `:3660`, `:3687` |
+| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10072`, `readonly-strict-errors.ts:66` |
+| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5891` |
+| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3735`, `:3745`, `:3772` |
 | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` |
 | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` |
-| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6504` |
-| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11999` |
-| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11928` |
+| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6589` |
+| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:12084` |
+| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:12013` |
 
 ### 3. Sharing (`plugin-sharing`)
 
@@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them.
 
 | # | Site | Package | What it does |
 |:--|:---|:---|:---|
-| 62 | `objectql/src/engine.ts:3457` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
-| 63 | `objectql/src/engine.ts:14348` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
+| 62 | `objectql/src/engine.ts:3542` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
+| 63 | `objectql/src/engine.ts:14433` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
 | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report |
 | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across |
 
@@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs.
 |:---|:---|:---|
 | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:685` |
 | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) |
-| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9922`–`9939` |
+| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10007`–`10024` |
 | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) |
 | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` |
 | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` |

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' \| … +291 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' \| … +292 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) |
 | **message** | `string` | ✅ | Readable error message |
 | **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim. Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution for anything unmarked. Status-agnostic; never replaces `message`. |
@@ -220,6 +220,7 @@ const result = ApiErrorSchema.parse(data);
 * `METADATA_CONFLICT`
 * `METADATA_NOT_FOUND`
 * `METADATA_SCHEMA_INVALID`
+* `MULTI_UPDATE_HOOK_KEY_DIVERGENCE`
 * `NAMESPACE_PREFIX`
 * `NEEDS_PASSWORD`
 * `NODE_FAILURE`

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

@@ -336,6 +336,7 @@ const result = ErrorCode.parse(data);
 * `METADATA_CONFLICT`
 * `METADATA_NOT_FOUND`
 * `METADATA_SCHEMA_INVALID`
+* `MULTI_UPDATE_HOOK_KEY_DIVERGENCE`
 * `NAMESPACE_PREFIX`
 * `NEEDS_PASSWORD`
 * `NODE_FAILURE`

packages/objectql/src/bulk-write-per-row-hooks.test.ts

@@ -614,9 +614,19 @@ describe('[#5574 / D3] the payload stays BATCH-scoped, and that IS the merge rul
   });
 
   it('a rewrite made on ONE row’s dispatch applies to the WHOLE batch', async () => {
-    let firstOnly = true;
+    // [#14099] The fixture — never the contract — changed here. It used to
+    // stamp `owner` on the FIRST row only (`if (firstOnly) …`), which is now
+    // REFUSED: writing a key for some matched rows and not others is precisely
+    // the divergence D3's enforcement rejects before any write, because it can
+    // only mean the handler is deciding per record. The contract this case
+    // pins is untouched and is still the reason the refusal exists — one
+    // `updateMany` carries one SET clause, so whichever dispatch produces a
+    // value, EVERY row gets it. So the handler writes the same key on every
+    // row, and the batch still carries ONE value: the last dispatch's.
+    let dispatches = 0;
     const { engine } = await boot([hook('stamp', 'beforeUpdate', (ctx) => {
-      if (firstOnly) { (ctx.input as any).data.owner = 'stamped'; firstOnly = false; }
+      dispatches += 1;
+      (ctx.input as any).data.owner = `stamped-${dispatches}`;
     })]);
     await seedTasks(engine, [
       { title: 'a', status: 'todo', owner: 'u1' },
@@ -625,10 +635,12 @@ describe('[#5574 / D3] the payload stays BATCH-scoped, and that IS the merge rul
 
     await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } });
 
-    // Both rows got it, including the one whose dispatch did not make it. This
-    // is the contract, not a leak: one `updateMany` carries one SET clause.
+    // Both rows carry the SECOND dispatch's value, including the row whose own
+    // dispatch produced `stamped-1`. That is the contract, not a leak — and it
+    // is the residual hazard #14099's ruling names openly and does not close:
+    // same key, per-row values still applies one row's value to all of them.
     const rows: any[] = await engine.find('task', {});
-    expect(rows.map((r) => r.owner)).toEqual(['stamped', 'stamped']);
+    expect(rows.map((r) => r.owner)).toEqual(['stamped-2', 'stamped-2']);
   });
 
   it('rewrites ACCUMULATE in dispatch order, including a REPLACED payload', async () => {