From 99faf2250d57a734ea98bab477bde1a3227d0f4c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 20:32:30 +0000 Subject: [PATCH 1/8] wip: #14099 divergent key-set refusal + pins --- packages/objectql/src/engine.ts | 86 +++- packages/objectql/src/index.ts | 16 + .../multi-update-hook-key-divergence.test.ts | 455 ++++++++++++++++++ .../src/multi-update-hook-key-divergence.ts | 194 ++++++++ .../spec/src/api/error-code-ledger.zod.ts | 13 + 5 files changed, 756 insertions(+), 8 deletions(-) create mode 100644 packages/objectql/src/multi-update-hook-key-divergence.test.ts create mode 100644 packages/objectql/src/multi-update-hook-key-divergence.ts diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 37ab4cb63c..1a5cc05eba 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -199,6 +199,11 @@ import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, str // SAME value. Armed and sealed in `update()`; the module owns the argument for // why neither end may move. import { recordHookPayloadWrites } from './hook-write-provenance.js'; +import { + divergingHookPayloadKeys, + MultiUpdateHookKeyDivergenceError, + type PerRowHookWrittenKeys, +} from './multi-update-hook-key-divergence.js'; import type { HookWriteRecording } from './hook-write-provenance.js'; import { resolveMasterDetailRelation } from './master-detail.js'; // [#6457] The master-detail header a `parent`-scoped predicate reads is made @@ -2928,10 +2933,27 @@ export class ObjectQL implements IObjectQLEngine { * A rewrite CONDITIONED on the row (`ctx.previous`, `ctx.input.id`) is * outside the contract: it does not scope itself to the row it was decided * on, it widens to every matched row. Per-row `previous` is supplied so a - * guard can REFUSE the write (throw), not so a rewrite can be aimed. That is - * a contract statement, not an enforcement — no static rule can decide - * whether a rewrite is row-invariant — and the ADR names it as such rather - * than hiding it. + * guard can REFUSE the write (throw), not so a rewrite can be aimed. + * + * ## D3, ENFORCED — divergent key sets refuse the batch [#14099] + * + * That last paragraph used to end "a contract statement, not an enforcement + * — no static rule can decide whether a rewrite is row-invariant". The second + * half is still true and the conclusion no longer follows: nothing STATIC can + * decide it, but the dispatch can MEASURE it. #14088's recorder is armed once + * more per row here, so each dispatch reports the set of payload keys THAT + * row's hook chain assigned; if two rows disagree, the batch is refused whole + * before any write ({@link MultiUpdateHookKeyDivergenceError}). + * + * Maintainer ruling of 2026-09-02, on the corruption `duly` measured against + * published 17.2.0 — a `completed_at` transition stamp moved an + * already-completed row's timestamp, silently, on a two-row batch. The + * criterion is the key SET and never the values, which is what lets the + * clock-reading audit stamp through: every row writes `updated_at`, so an + * honest batch is never refused non-deterministically. The full argument, + * both rejected value-comparison variants, and the blind spot the ruling + * carries openly (same key, per-row VALUES) live on + * `multi-update-hook-key-divergence.ts`. * * ## D4 — `input.id` is not a reroute lever here * @@ -2948,10 +2970,31 @@ export class ObjectQL implements IObjectQLEngine { ): Promise { const schema = this._registry.getObject(object); const carriesPayload = event === 'beforeUpdate'; + // [#14099] D3's enforcement half — one recorded key set per row. + // `undefined` entries are rows whose recording cannot speak (a hook + // REPLACED the payload object); `divergingHookPayloadKeys` abstains on + // those rather than reading them as an empty set. + const perRowHookWrittenKeys: PerRowHookWrittenKeys[] = []; for (let index = 0; index < rows.length; index++) { const row = rows[index]; const rowId = (row as { id?: unknown }).id; const options = (batchCtx.input as { options?: unknown }).options; + // D3: THE payload, read fresh so a previous row's REPLACEMENT is what + // this row sees. Never a copy. + const batchPayload = (batchCtx.input as { data?: unknown }).data; + // [#14099] The #14088 recorder, armed a SECOND time and per row, nested + // over the batch-scoped recording `update()` already armed at its entry. + // Nesting is what makes both readings true at once: a write through this + // view lands on the outer recording's view, which lands on the real + // payload — so the outer record (which the readonly strips read for + // provenance) still sees every hook write, while this one sees only the + // writes THIS row's chain made. A fresh recording per row is the whole + // point: one recording across the loop would accumulate the union and + // could never tell two rows apart. + const rowRecording = + carriesPayload && batchPayload !== null && typeof batchPayload === 'object' + ? recordHookPayloadWrites(batchPayload as Record) + : undefined; const rowCtx = { ...batchCtx, event, @@ -2961,10 +3004,8 @@ export class ObjectQL implements IObjectQLEngine { // context is a fresh object, so a stash written on the context itself // dies with the row that held it. dispatch: { ...(batchCtx.dispatch as object), index } as HookContext['dispatch'], - // D3: THE payload, read fresh so a previous row's REPLACEMENT is what - // this row sees. Never a copy. input: carriesPayload - ? { id: rowId, data: (batchCtx.input as { data?: unknown }).data, options } + ? { id: rowId, data: rowRecording?.payload ?? batchPayload, options } : { id: rowId, options }, previous: coerceBooleanFields(schema as any, row as any), // D2: no post-state in the before phase. @@ -2975,7 +3016,15 @@ export class ObjectQL implements IObjectQLEngine { // D3, the accumulate half — see the class doc above. if (carriesPayload) { - (batchCtx.input as { data?: unknown }).data = (rowCtx.input as { data?: unknown }).data; + // Sealing does the accumulate write-back AND closes this row's record. + // It is what keeps a recording VIEW out of `batchCtx.input.data` — the + // next row, the outer seal and eventually the driver must all see the + // raw payload (or the hook's replacement), never a proxy of it. + const sealed = rowRecording?.seal((rowCtx.input as { data?: unknown }).data); + (batchCtx.input as { data?: unknown }).data = sealed + ? sealed.data + : (rowCtx.input as { data?: unknown }).data; + if (rowRecording) perRowHookWrittenKeys.push(sealed?.hookWrittenKeys); } // D4. const observed = (rowCtx.input as { id?: unknown }).id; @@ -2985,6 +3034,27 @@ export class ObjectQL implements IObjectQLEngine { }); } } + + // [#14099] The refusal, ruled 2026-09-02 (recommendation C). Placed after + // the loop and not inside it, for two reasons that are both about the + // envelope rather than about cost: the diverging set is `union \ + // intersection` over EVERY row, so the message names every offending key + // instead of the first pair to disagree, and it is order-independent — the + // same batch answers the same way whatever order the driver returned the + // matched rows in. + // + // ⭐ Still BEFORE any write, which is the load-bearing half. This method is + // called from `update()`'s predicate branch ahead of the hook-write seal, + // both readonly strips, `evaluateValidationRules` and every + // `driver.updateMany` — and it runs outside `update()`'s own `try`, so the + // envelope reaches the caller undecorated. Not "after the first row", not + // "inside a transaction that then rolls back": nothing was written. + if (carriesPayload) { + const diverging = divergingHookPayloadKeys(perRowHookWrittenKeys); + if (diverging.length > 0) { + throw new MultiUpdateHookKeyDivergenceError(object, diverging, rows.length); + } + } } /** diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 23a82f0568..e3b62d07ec 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -110,6 +110,22 @@ export { ReadonlyFieldRejectedError } from './readonly-strict-errors.js'; // 'DUPLICATE_RECORD'` is the boundary-crossing identity, the class is the // in-process convenience. export { DuplicateRecordError, DUPLICATE_RECORD_CODE } from './duplicate-record-error.js'; +// [#14099] Thrown by `engine.update` when a `multi: true` batch's per-row +// `beforeUpdate` dispatches assigned DIFFERENT sets of payload keys — the +// enforcement half of ADR-0058 Addendum II D3. Exported for the same reason as +// its neighbour above: an application whose hook has to switch to a per-row +// `ctx.api` write (or to by-id updates) needs to NAME the condition, and +// `code === 'MULTI_UPDATE_HOOK_KEY_DIVERGENCE'` is the boundary-crossing +// identity. `divergingHookPayloadKeys` rides along because it is the whole +// criterion, pure and total, and a consumer reasoning about the refusal should +// be able to read it rather than re-derive it. +export { + MultiUpdateHookKeyDivergenceError, + MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE, + MULTI_UPDATE_HOOK_KEY_DIVERGENCE_STATUS, + divergingHookPayloadKeys, +} from './multi-update-hook-key-divergence.js'; +export type { PerRowHookWrittenKeys } from './multi-update-hook-key-divergence.js'; // Boot guard: thrown by `ObjectQL.init()` when a registered driver's connect() // fails (framework#3741). Hosts that boot the engine themselves can catch it to // render their own "database unreachable" message. diff --git a/packages/objectql/src/multi-update-hook-key-divergence.test.ts b/packages/objectql/src/multi-update-hook-key-divergence.test.ts new file mode 100644 index 0000000000..12e62a1151 --- /dev/null +++ b/packages/objectql/src/multi-update-hook-key-divergence.test.ts @@ -0,0 +1,455 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14099] A `multi: true` update whose per-row `beforeUpdate` dispatches + * assigned DIFFERENT sets of payload keys is refused whole, before any write. + * + * The defect these pins retire was measured downstream against published + * 17.2.0: two `duly_task` rows — one open, one completed earlier — updated in + * one `multi: true` call with a `completed_at` transition stamp bound to + * `beforeUpdate`. The already-completed row's `completed_at` MOVED, from + * `…:26.560Z` to `…:26.571Z`, without transitioning and without an error. The + * corrupted row is byte-for-byte indistinguishable from one genuinely completed + * late, which is what turns a compliant record into a breach in every on-time + * measure reading the column. + * + * Maintainer ruling, 2026-09-02 (recommendation C): keep D3 — the payload stays + * batch-scoped and the engine never splits its own write — and ENFORCE it, by + * recording each row's assigned key set and refusing the batch when two rows + * disagree. ⛔ The criterion is the key SET and never the values; the three + * pins the ruling named are §1, §2 and §3 below, and §3 is the regression guard + * for the whole platform (if the audit-stamp-only batch ever moves, the refusal + * is over-firing on a hook registered in essentially every deployment). + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { bindHooksToEngine } from './hook-binder.js'; +import { + MultiUpdateHookKeyDivergenceError, + MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE, + MULTI_UPDATE_HOOK_KEY_DIVERGENCE_STATUS, + divergingHookPayloadKeys, +} from './multi-update-hook-key-divergence.js'; +import type { Hook, HookContext } from '@objectstack/spec/data'; + +const TASK_FIELDS = { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + status: { name: 'status', label: 'Status', type: 'text' as const }, + completed_at: { name: 'completed_at', label: 'Completed at', type: 'text' as const }, + priority: { name: 'priority', label: 'Priority', type: 'text' as const }, + updated_at: { name: 'updated_at', label: 'Updated at', type: 'text' as const }, +}; +const taskObject = { name: 'task', label: 'Task', fields: TASK_FIELDS }; +const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; + +/** The hook the card measured, and the shape the docs and the showcase teach. */ +const transitionStamp = (at: string) => (ctx: HookContext) => { + const prev = (ctx as any).previous as Record | undefined; + const data = (ctx.input as any).data as Record; + if (prev?.status !== 'done' && data.status === 'done') data.completed_at = at; +}; + +function hook(name: string, event: string, handler: (ctx: HookContext) => void): Hook { + return { name, object: 'task', events: [event], priority: 100, handler } as unknown as Hook; +} + +function makeStubDriver(): any { + const store = new Map>(); + let nextId = 0; + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + if (v && typeof v === 'object' && Array.isArray((v as any).$in)) { + if (!(v as any).$in.some((x: unknown) => x === row[k])) return false; + continue; + } + const expected = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; + if ((row[k] ?? null) !== (expected ?? null)) return false; + } + return true; + }; + const d: any = { + name: 'memory', version: '0.0.0', supports: {}, + store, + /** Single-row writes — a predicate write must never be split into N (D3). */ + updateCalls: 0, + /** Every payload `updateMany` was handed, so a pin can read the SET clause. */ + updateManyPayloads: [] as Record[], + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, async syncSchema() {}, + async find(_o: string, ast: any) { + return Array.from(store.values()).filter((r) => matches(r, ast?.where)); + }, + async findOne(_o: string, ast: any) { + for (const r of store.values()) if (matches(r, ast?.where)) return r; + return null; + }, + async create(_o: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; store.set(id, row); return row; + }, + async update(_o: string, id: string, data: Record) { + d.updateCalls += 1; + const cur = store.get(id); if (!cur) return null; + const u = { ...cur, ...data, id }; store.set(id, u); return u; + }, + async updateMany(_o: string, ast: any, data: Record) { + d.updateManyPayloads.push({ ...data }); + const rows = Array.from(store.values()).filter((r) => matches(r, ast?.where)); + for (const r of rows) store.set(r.id as string, { ...r, ...data, id: r.id }); + return rows.length; + }, + async delete(_o: string, id: string) { return store.delete(id); }, + async deleteMany(_o: string, ast: any) { + const rows = Array.from(store.values()).filter((r) => matches(r, ast?.where)); + for (const r of rows) store.delete(r.id as string); + return rows.length; + }, + async count(_o: string) { return store.size; }, + async bulkCreate(_o: string, rows: any[]) { return Promise.all(rows.map((r) => d.create(_o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async upsert(_o: string, data: any) { return d.create(_o, data); }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return d; +} + +async function boot(hooks: Hook[]): Promise<{ engine: ObjectQL; driver: any }> { + const engine = new ObjectQL(); + const driver = makeStubDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(taskObject); + if (hooks.length > 0) { + bindHooksToEngine(engine, hooks, { packageId: 'app:test', logger: silentLogger }); + } + return { engine, driver }; +} + +/** The stored rows, as a stable, comparable snapshot. */ +const snapshot = (driver: any): string => + JSON.stringify( + [...(driver.store as Map>).values()] + .sort((a, b) => String(a.id).localeCompare(String(b.id))), + ); + +/* ──────────────────────────────────────────────────────────────────────────── + * 1. The card's own fixture — REFUSED, with the envelope naming `completed_at` + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#14099] the mixed batch the card measured is refused', () => { + async function mixedBatch() { + const { engine, driver } = await boot([ + hook('stamp_completion', 'beforeUpdate', transitionStamp('2026-09-01T00:00:26.571Z')), + ]); + await engine.insert('task', [ + { id: 'open', title: 'a', status: 'todo', completed_at: null }, + { id: 'already', title: 'b', status: 'done', completed_at: '2026-09-01T00:00:26.560Z' }, + ] as any); + return { engine, driver }; + } + + const run = (engine: ObjectQL) => + engine.update('task', { status: 'done' }, { + multi: true, where: { id: { $in: ['open', 'already'] } }, + } as any); + + it('refuses with the ADR-0112 envelope — asserted by `code` and `status`', async () => { + const { engine } = await mixedBatch(); + // ⛔ Never a bare `toThrow()`: an un-fixed engine throws nothing at all here, + // and a *different* refusal would satisfy one. The envelope is the pin. + const err = await run(engine).then( + () => { throw new Error('expected the batch to be refused'); }, + (e: unknown) => e as MultiUpdateHookKeyDivergenceError, + ); + expect(err).toBeInstanceOf(MultiUpdateHookKeyDivergenceError); + expect(err.code).toBe(MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE); + expect(err.code).toBe('MULTI_UPDATE_HOOK_KEY_DIVERGENCE'); + expect(err.status).toBe(MULTI_UPDATE_HOOK_KEY_DIVERGENCE_STATUS); + expect(err.status).toBe(400); + }); + + it('names the object, the diverging key and the prescription', async () => { + const { engine } = await mixedBatch(); + const err = await run(engine).catch((e) => e as MultiUpdateHookKeyDivergenceError); + expect(err.object).toBe('task'); + expect(err.keys).toEqual(['completed_at']); + expect(err.rows).toBe(2); + // The message a user-facing surface renders names the object and the key… + expect(err.message).toContain("'task'"); + expect(err.message).toContain("'completed_at'"); + expect(err.message).toContain('Nothing was written'); + // …and the developer half carries both routes out, which is the whole point + // of refusing rather than corrupting. + expect(err.developerMessage).toContain('ctx.api'); + expect(err.developerMessage).toContain('by id'); + }); + + it('refuses BEFORE any write — no row changed, and no driver write ran', async () => { + const { engine, driver } = await mixedBatch(); + const before = snapshot(driver); + await run(engine).catch(() => {}); + // Not "after the first row", not "inside a transaction that then rolls + // back": the driver was never asked to write anything. + expect(driver.updateManyPayloads).toEqual([]); + expect(driver.updateCalls).toBe(0); + expect(snapshot(driver)).toBe(before); + // Specifically: the row the card watched still holds its ORIGINAL instant. + expect(driver.store.get('already').completed_at).toBe('2026-09-01T00:00:26.560Z'); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 2. An all-transition batch PROCEEDS as one `updateMany` + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#14099] an honest batch still proceeds as ONE updateMany (D3 intact)', () => { + it('every row transitions ⇒ one write, and every row is stamped', async () => { + const { engine, driver } = await boot([ + hook('stamp_completion', 'beforeUpdate', transitionStamp('2026-09-02T10:00:00.000Z')), + ]); + await engine.insert('task', [ + { id: 'a', title: 'a', status: 'todo', completed_at: null }, + { id: 'b', title: 'b', status: 'todo', completed_at: null }, + { id: 'c', title: 'c', status: 'todo', completed_at: null }, + ] as any); + + await engine.update('task', { status: 'done' }, { + multi: true, where: { status: 'todo' }, + } as any); + + // ONE `updateMany`, never split into N single-row writes. + expect(driver.updateManyPayloads).toHaveLength(1); + expect(driver.updateCalls).toBe(0); + expect(driver.updateManyPayloads[0].completed_at).toBe('2026-09-02T10:00:00.000Z'); + for (const id of ['a', 'b', 'c']) { + expect(driver.store.get(id).status).toBe('done'); + expect(driver.store.get(id).completed_at).toBe('2026-09-02T10:00:00.000Z'); + } + }); + + it('a batch of ONE row is never refused — there is nothing to diverge from', async () => { + const { engine, driver } = await boot([ + hook('stamp_completion', 'beforeUpdate', transitionStamp('2026-09-02T10:00:00.000Z')), + ]); + await engine.insert('task', [{ id: 'solo', title: 'a', status: 'done' }] as any); + await engine.update('task', { status: 'done' }, { + multi: true, where: { status: 'done' }, + } as any); + expect(driver.updateManyPayloads).toHaveLength(1); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 3. The audit-stamp-only batch is BYTE-IDENTICAL before and after + * + * The platform-wide regression guard. `sys_stamp_audit_update` is registered on + * `'*'`, so it runs in essentially every deployment: if the refusal ever fires + * on a batch whose only payload rewrite is that stamp, every bulk update on + * every ObjectStack install starts erroring. It reads the clock ONCE PER ROW, + * so the VALUES diverge across rows on a perfectly honest batch — which is + * precisely why the ruled criterion is the key SET. + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#14099] a row-invariant rewrite is never refused, however its values differ', () => { + /** The audit stamp's shape: same key on every row, a fresh clock read each time. */ + const perRowClockStamp = () => { + let tick = 0; + return (ctx: HookContext) => { + tick += 1; + (ctx.input as any).data.updated_at = `2026-09-02T10:00:00.${String(tick).padStart(3, '0')}Z`; + }; + }; + + it('same key, DIFFERENT value per row ⇒ proceeds (the values are not the criterion)', async () => { + const { engine, driver } = await boot([ + hook('audit_stamp', 'beforeUpdate', perRowClockStamp()), + ]); + await engine.insert('task', [ + { id: 'a', title: 'a', status: 'todo' }, + { id: 'b', title: 'b', status: 'done' }, + { id: 'c', title: 'c', status: 'blocked' }, + ] as any); + + await engine.update('task', { title: 'renamed' }, { + multi: true, where: {}, + } as any); + + expect(driver.updateManyPayloads).toHaveLength(1); + // D3, unchanged: the LAST dispatch's value is the one the batch carries. + expect(driver.updateManyPayloads[0].updated_at).toBe('2026-09-02T10:00:00.003Z'); + for (const id of ['a', 'b', 'c']) { + expect(driver.store.get(id).title).toBe('renamed'); + expect(driver.store.get(id).updated_at).toBe('2026-09-02T10:00:00.003Z'); + } + }); + + it('BYTE-IDENTICAL: the audit-stamp-only batch writes exactly what it wrote before #14099', async () => { + // Both engines run the same hook over the same rows; the pin is that the + // stored bytes are the same object graph either way, so "the refusal + // over-fires" cannot hide as a subtle payload difference. + const expected = JSON.stringify([ + { id: 'a', title: 'renamed', status: 'todo', updated_at: '2026-09-02T10:00:00.001Z' }, + { id: 'b', title: 'renamed', status: 'done', updated_at: '2026-09-02T10:00:00.001Z' }, + ]); + const { engine, driver } = await boot([ + hook('audit_stamp', 'beforeUpdate', (ctx) => { + (ctx.input as any).data.updated_at = '2026-09-02T10:00:00.001Z'; + }), + ]); + await engine.insert('task', [ + { id: 'a', title: 'a', status: 'todo' }, + { id: 'b', title: 'b', status: 'done' }, + ] as any); + await engine.update('task', { title: 'renamed' }, { multi: true, where: {} } as any); + expect(snapshot(driver)).toBe(expected); + }); + + it('a hook that writes NOTHING on any row is never refused', async () => { + const { engine, driver } = await boot([hook('inert', 'beforeUpdate', () => {})]); + await engine.insert('task', [ + { id: 'a', title: 'a', status: 'todo' }, + { id: 'b', title: 'b', status: 'done' }, + ] as any); + await engine.update('task', { title: 'x' }, { multi: true, where: {} } as any); + expect(driver.updateManyPayloads).toHaveLength(1); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 4. The blind spot the ruling carries OPENLY — pinned as intended, not fixed + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#14099] same key + per-row VALUES still passes — D3’s declared cost', () => { + it('a per-row derived value is NOT refused, and row 1’s value reaches every row', async () => { + // ⚠️ This is the ruling's named blind spot, pinned so it cannot change by + // accident in either direction. It is filed as its own finding with a + // measured instance; ⛔ it is NOT widened into this card, and ⛔ the fix is + // NOT a value comparison — see `multi-update-hook-key-divergence.ts` for + // the two measurements that rejected one. + const { engine, driver } = await boot([ + hook('derive_priority', 'beforeUpdate', (ctx) => { + const prev = (ctx as any).previous as Record; + (ctx.input as any).data.priority = prev.status === 'blocked' ? 'high' : 'low'; + }), + ]); + await engine.insert('task', [ + { id: 'a', title: 'a', status: 'blocked' }, + { id: 'b', title: 'b', status: 'todo' }, + ] as any); + + await engine.update('task', { title: 'x' }, { multi: true, where: {} } as any); + + expect(driver.updateManyPayloads).toHaveLength(1); + // The LAST row's derivation wins and lands on both rows. Row `a` is + // `blocked` and should have been `high`; it is not. + expect(driver.store.get('a').priority).toBe('low'); + expect(driver.store.get('b').priority).toBe('low'); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 5. What the recorder cannot say, it does not say + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#14099] a hook that REPLACES the payload abstains rather than being refused', () => { + it('replacement ⇒ no attributable record ⇒ the batch is not refused', async () => { + // `hook-write-provenance.ts`'s KNOWN LIMIT: a replaced payload's keys are + // indistinguishable from the caller's, so the recording says "cannot say". + // Refusing on a measurement never taken would be a fabricated verdict, so + // the pre-#14099 behaviour stands — the same fail-safe direction #14088 + // chose for the same limit. + const { engine, driver } = await boot([ + hook('replaces', 'beforeUpdate', (ctx) => { + const prev = (ctx as any).previous as Record; + (ctx.input as any).data = prev.status === 'todo' + ? { ...(ctx.input as any).data, completed_at: 'x' } + : { ...(ctx.input as any).data }; + }), + ]); + await engine.insert('task', [ + { id: 'a', title: 'a', status: 'todo' }, + { id: 'b', title: 'b', status: 'done' }, + ] as any); + await engine.update('task', { title: 'x' }, { multi: true, where: {} } as any); + expect(driver.updateManyPayloads).toHaveLength(1); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 6. Neighbouring contracts, unchanged + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#14099] the refusal is scoped to the predicate UPDATE path', () => { + it('a by-id update with the same hook is untouched — one row, one payload', async () => { + const { engine, driver } = await boot([ + hook('stamp_completion', 'beforeUpdate', transitionStamp('2026-09-02T11:00:00.000Z')), + ]); + await engine.insert('task', [{ id: 'a', title: 'a', status: 'todo' }] as any); + await engine.update('task', { id: 'a', status: 'done' } as any); + expect(driver.store.get('a').completed_at).toBe('2026-09-02T11:00:00.000Z'); + }); + + it('a predicate DELETE is untouched — that event carries no payload at all', async () => { + const seen: unknown[] = []; + const { engine, driver } = await boot([ + hook('guard', 'beforeDelete', (ctx) => { seen.push((ctx.input as any).id); }), + ]); + await engine.insert('task', [ + { id: 'a', title: 'a', status: 'todo' }, + { id: 'b', title: 'b', status: 'done' }, + ] as any); + await engine.delete('task', { multi: true, where: {} } as any); + expect(seen).toHaveLength(2); + expect(driver.store.size).toBe(0); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 7. The criterion itself — pure, total, order-independent + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#14099] divergingHookPayloadKeys', () => { + const S = (...k: string[]) => new Set(k); + + it('agreeing rows diverge on nothing', () => { + expect(divergingHookPayloadKeys([S('a', 'b'), S('b', 'a'), S('a', 'b')])).toEqual([]); + }); + + it('reports `union \\ intersection`, sorted — every offending key, not the first pair', () => { + expect(divergingHookPayloadKeys([S('x'), S('y'), S('x', 'y', 'z')])).toEqual(['x', 'y', 'z']); + expect(divergingHookPayloadKeys([S('shared', 'a'), S('shared')])).toEqual(['a']); + }); + + it('is ORDER-INDEPENDENT — the same batch answers the same whatever order the rows came in', () => { + const rows = [S('a'), S(), S('a', 'b')]; + const answer = divergingHookPayloadKeys(rows); + expect(divergingHookPayloadKeys([...rows].reverse())).toEqual(answer); + expect(divergingHookPayloadKeys([rows[1], rows[2], rows[0]])).toEqual(answer); + }); + + it('fewer than two recorded rows cannot diverge', () => { + expect(divergingHookPayloadKeys([])).toEqual([]); + expect(divergingHookPayloadKeys([S('a')])).toEqual([]); + }); + + it('`undefined` ABSTAINS — it is "cannot say", never an empty set', () => { + // Reading `undefined` as `{}` would refuse this batch on evidence that does + // not exist. Both recorded rows agree, so nothing diverges. + expect(divergingHookPayloadKeys([S('a'), undefined, S('a')])).toEqual([]); + expect(divergingHookPayloadKeys([undefined, undefined])).toEqual([]); + // …and rows that CAN speak are still compared with each other. + expect(divergingHookPayloadKeys([S('a'), undefined, S('b')])).toEqual(['a', 'b']); + }); + + it('a key DELETED on one row and assigned on another diverges', () => { + // `recordHookPayloadWrites` drops a deleted key from the record, so this is + // the shape a `delete ctx.input.data.x` on some rows produces. + expect(divergingHookPayloadKeys([S('x'), S()])).toEqual(['x']); + }); +}); diff --git a/packages/objectql/src/multi-update-hook-key-divergence.ts b/packages/objectql/src/multi-update-hook-key-divergence.ts new file mode 100644 index 0000000000..acd77ca4fd --- /dev/null +++ b/packages/objectql/src/multi-update-hook-key-divergence.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14099] The refusal that ends "one hook-mutated payload, applied to every + * matched row" — ADR-0058 Addendum II D3, ENFORCED rather than merely stated. + * + * ## The corruption this retires + * + * `driver.updateMany` takes ONE `SET` clause for N rows, so D3 makes the + * predicate write's payload batch-scoped and hands every per-row `before*` + * dispatch THE payload rather than a copy. The addendum names the residual + * hazard in as many words: a rewrite CONDITIONED on the row + * (`ctx.previous`, `ctx.input.id`) "does not scope itself to the row it was + * decided on, it widens to every matched row" — and until this module it said + * so as "a contract statement, not an enforcement". + * + * Measured downstream, against published 17.2.0: `duly_task` carries a + * `readonly` `completed_at` stamped by a `beforeUpdate` hook on the transition + * into `done`. Two rows — one open, one completed earlier — updated in one + * `multi: true` call. The already-done row's `completed_at` MOVED, from + * `…:26.560Z` to `…:26.571Z`. It did not transition. Nothing errored, and the + * corrupted row is byte-for-byte indistinguishable from one that really was + * completed late — which is what turns a compliant record into a breach in + * every on-time measure that reads the column. The whole class is exposed: + * `approved_at`, `closed_at`, `shipped_at`, `first_responded_at`. + * + * ## The criterion is the KEY SET, never the values + * + * Maintainer ruling of 2026-09-02 (recommendation C, on the decision batch that + * kept Addendum II D3 standing): + * + * > On a `multi: true` update the engine keeps dispatching the before-phase + * > hooks per row with the per-row pre-image and records, per row, the set of + * > payload keys the hook chain assigned. If the recorded key sets differ + * > between any two rows, the whole batch is refused before any write … If + * > every row's key set is identical, the batch proceeds as one `updateMany` + * > with the batch payload, exactly as D3 says. The criterion is the key set, + * > never the values, so the audit stamp's per-row clock reads cannot make an + * > honest batch non-deterministic. + * + * ⛔ Do not "tighten" this to compare VALUES. That variant was rejected on + * measurement twice over, and both measurements are recoverable only here: + * + * - objectql's own `sys_stamp_audit_update` builtin is registered on `'*'`, + * so it runs in essentially every deployment, and it reads the clock INSIDE + * the per-record stamp. Under per-row dispatch that is one clock read per + * row, so two rows either side of a millisecond boundary carry different + * `updated_at` VALUES on an entirely honest batch. A value comparison would + * refuse it non-deterministically — the failure direction nobody can debug. + * - A value comparison also re-breaks #14088's own row: a hook that + * deliberately writes the value the caller also sent (`completed_at: null` + * on a reopen, against a caller that round-tripped the record) is + * indistinguishable, by value, from a hook that never touched the key. That + * is the defect `hook-write-provenance.ts` exists to remove; a value test + * here would reintroduce it one seam over. + * + * The key set is what {@link recordHookPayloadWrites} already records, so this + * refusal adds no new instrument — it reads the #14088 recorder, armed per row. + * + * ## What is NOT covered, named rather than hidden + * + * A hook that writes the SAME key on every row but with per-row VALUES (a + * per-row derived priority, say) passes this test and still applies the first + * row's value to every matched row. That is D3's cost by design; the ruling + * carries it openly and points at the prescription below as the exit. This + * module does not widen to cover it, and a future author reaching for a value + * comparison to close it must re-read the two measurements above first. + * + * ## Divergence is `union \ intersection`, and rows that cannot speak abstain + * + * The diverging keys are every key some row's recording holds and some other + * row's does not — order-independent by construction, so the envelope names + * the same keys whatever order the driver returned the matched rows in. + * + * A row whose recording is `undefined` — a hook REPLACED `ctx.input.data` + * rather than mutating it, the KNOWN LIMIT `hook-write-provenance.ts` + * documents — is EXCLUDED from the comparison rather than treated as an empty + * set. `undefined` means "this call cannot say", and refusing a batch on a + * measurement that was never taken would be a fabricated verdict. That keeps + * the pre-#14099 behaviour for payload-replacing hooks, which is the same + * fail-safe direction #14088 chose for the same limit: keep the old bug rather + * than act on evidence that does not exist. + */ + +/** + * One row's recorded key set, as {@link SealedHookWrites.hookWrittenKeys} + * hands it back: the keys that row's hook chain assigned, or `undefined` when + * the call has no attributable record. + */ +export type PerRowHookWrittenKeys = ReadonlySet | undefined; + +/** + * The keys whose presence in the hook chain's writes DIFFERS across the rows + * of one predicate update, sorted; `[]` when every row that could be recorded + * agreed (which includes the cases of one row, and of no row able to speak). + * + * Pure and total — it never throws and never reads the engine. The engine + * raises; the contract decides. + */ +export function divergingHookPayloadKeys(perRow: readonly PerRowHookWrittenKeys[]): string[] { + const recorded = perRow.filter((s): s is ReadonlySet => s !== undefined); + if (recorded.length < 2) return []; + const union = new Set(); + for (const set of recorded) for (const key of set) union.add(key); + const diverging: string[] = []; + for (const key of union) { + if (!recorded.every((set) => set.has(key))) diverging.push(key); + } + return diverging.sort(); +} + +/** + * The wire code, registered in the spec's `ERROR_CODE_LEDGER` under + * `@objectstack/objectql`. + * + * ⚠️ A REGISTERED ADR-0112 code rather than the `ERR_`-prefixed operational + * kind its `HookTargetRebindError` neighbour uses, and the difference is not + * stylistic: the whole value of this refusal is that the application RECOGNISES + * it and takes the prescription. An unregistered spelling demotes off + * `error.code` at the dispatcher door and rides `declaredCode` instead, which + * is exactly the wrong place for the one code `duly` and `hotcrm` have to + * branch on. `FILE_FIELD_BULK_WRITE_REFUSED` — the same seam, the same shape of + * refusal, one predicate write over — made the same call for the same reason. + */ +export const MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE = 'MULTI_UPDATE_HOOK_KEY_DIVERGENCE' as const; + +/** + * `400`, taken from `FileFieldBulkWriteError`'s reasoning verbatim because the + * verdict is the same one: the write is expressible and permitted, it just + * cannot be expressed as ONE payload, and the remedy belongs to the caller. + * Not `409` — nothing about the stored rows is in conflict, and nothing changes + * if the caller retries later. + */ +export const MULTI_UPDATE_HOOK_KEY_DIVERGENCE_STATUS = 400 as const; + +/** + * The ADR-0112 envelope a `multi: true` update raises when its per-row + * `beforeUpdate` dispatches assigned DIFFERENT sets of payload keys. + * + * Thrown from the per-row before-phase, which runs outside `update()`'s own + * `try` block, so it reaches the caller intact — and, decisively, BEFORE the + * payload is sealed, before both readonly strips, before validation and before + * any `driver.updateMany`. Nothing is written: not the first row, not a + * transaction that then rolls back. + */ +export class MultiUpdateHookKeyDivergenceError extends Error { + override readonly name = 'MultiUpdateHookKeyDivergenceError'; + readonly code = MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE; + readonly status = MULTI_UPDATE_HOOK_KEY_DIVERGENCE_STATUS; + /** The object the refused batch targeted. */ + readonly object: string; + /** The keys whose presence differed across rows, sorted. */ + readonly keys: readonly string[]; + /** How many rows the predicate matched — the batch that was refused whole. */ + readonly rows: number; + /** The remedy half, addressed to the hook's author rather than to a user. */ + readonly developerMessage: string; + + constructor(object: string, keys: readonly string[], rows: number) { + super(buildMessage(object, keys, rows)); + this.object = object; + this.keys = [...keys]; + this.rows = rows; + this.developerMessage = + `A predicate update sends ONE 'SET' clause to the driver, so there is exactly one payload ` + + `for all ${rows} matched records (ADR-0058 Addendum II D3) — whatever a 'beforeUpdate' handler ` + + `writes for one row is applied to every row. The handler wrote ` + + `${keys.map((k) => `'${k}'`).join(', ')} for some of these records and not for others, which ` + + `means it is deciding per record; the engine refuses rather than applying one record's ` + + `derived value to all of them. Two supported ways to express it: write the affected records ` + + `from INSIDE the handler with 'ctx.api' (a per-row write, which the handler's own sandbox ` + + `signals — 'ctx.dispatch.mode', 'ctx.input.id' — let it aim), or have the caller issue the ` + + `updates by id. Branch on \`code === '${MULTI_UPDATE_HOOK_KEY_DIVERGENCE_CODE}'\` (ADR-0112) ` + + `to detect this.`; + } +} + +/** + * The user-facing sentence. + * + * ⛔ It must not begin with a SQL verb — `@objectstack/rest`'s importer runs + * row errors through `sanitizeRowError`, whose SQL backstop replaces any + * message STARTING with `insert`/`update`/`delete` with generic text. The same + * constraint `DuplicateRecordError`'s message records, measured there. + */ +function buildMessage(object: string, keys: readonly string[], rows: number): string { + const named = keys.map((k) => `'${k}'`).join(', '); + return ( + `Refusing a multi-record update on '${object}': its 'beforeUpdate' handlers wrote ${named} for ` + + `some of the ${rows} matched records and not for others, and a predicate update has one payload ` + + `for every record — so one record's value would have been written to all of them. Nothing was ` + + `written. Write those records individually, from inside the handler with 'ctx.api' or by id.` + ); +} diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index d72e9be969..2954a3f116 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -535,6 +535,19 @@ 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', + // [#14099] a `multi: true` update whose per-row `beforeUpdate` dispatches + // assigned DIFFERENT sets of payload keys — a transition stamp + // (`completed_at` on the move into `done`) is the measured shape. One `SET` + // clause serves N rows (ADR-0058 Addendum II D3), so the engine refuses the + // batch whole, before any write, instead of applying one row's derived + // value to every matched row. Registered rather than left as an `ERR_` + // operational code precisely because the prescription is the application's + // to take: it branches on this to switch to a per-row `ctx.api` write or + // by-id updates. Not a VALIDATION_ERROR synonym — the payload and the + // predicate are both valid; the batch SHAPE cannot carry a per-row + // decision. `MultiUpdateHookKeyDivergenceError`, + // `multi-update-hook-key-divergence.ts`. + 'MULTI_UPDATE_HOOK_KEY_DIVERGENCE', // [#11142/#11230] a by-id update carried an `options.where.id` that is not // the bound payload `data.id` — a truthy scalar naming a DIFFERENT row // (#11142), or a non-scalar predicate over a row SET (#11230, which also From 6212e97408961dda6edff8c8505e140928045949 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 20:34:35 +0000 Subject: [PATCH 2/8] wip: changeset --- ...ulti-update-hook-key-divergence-refusal.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 .changeset/multi-update-hook-key-divergence-refusal.md diff --git a/.changeset/multi-update-hook-key-divergence-refusal.md b/.changeset/multi-update-hook-key-divergence-refusal.md new file mode 100644 index 0000000000..27efdcf7d5 --- /dev/null +++ b/.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. + + From ea65b6c0590e9907a2a973995f1393a154ed8fae Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 20:35:09 +0000 Subject: [PATCH 3/8] wip: census re-anchor --- content/docs/permissions/system-context.mdx | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index bce3f2ceb7..b5932ab52d 100644 --- a/content/docs/permissions/system-context.mdx +++ b/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:11274` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11457` | +| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10009` | | 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:10057`, `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:5876` | +| 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:3720`, `:3730`, `:3757` | | 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:6574` | +| 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:12069` | +| 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:11998` | ### 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:3527` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 63 | `objectql/src/engine.ts:14418` | 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:9992`–`10009` | | "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` | From 9657baaec4cc3cd5f0f3016859c360f4bd4be0f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 20:45:06 +0000 Subject: [PATCH 4/8] wip: regenerate spec reference docs for the new ledger code --- content/docs/references/api/contract.mdx | 3 ++- content/docs/references/api/error-code-ledger.mdx | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/content/docs/references/api/contract.mdx b/content/docs/references/api/contract.mdx index 88ad133f51..867fef07ac 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' \| … +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` diff --git a/content/docs/references/api/error-code-ledger.mdx b/content/docs/references/api/error-code-ledger.mdx index 5f419edcec..44312c13df 100644 --- a/content/docs/references/api/error-code-ledger.mdx +++ b/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` From 1c525be631693c1f0ff24de37ea61919feb0c4f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:07:02 +0000 Subject: [PATCH 5/8] test: honour the caller's bound in the new stub driver's find (check:objectql-double-limit) --- .../src/multi-update-hook-key-divergence.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/objectql/src/multi-update-hook-key-divergence.test.ts b/packages/objectql/src/multi-update-hook-key-divergence.test.ts index 12e62a1151..3bf790617c 100644 --- a/packages/objectql/src/multi-update-hook-key-divergence.test.ts +++ b/packages/objectql/src/multi-update-hook-key-divergence.test.ts @@ -80,8 +80,15 @@ function makeStubDriver(): any { updateManyPayloads: [] as Record[], async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, async syncSchema() {}, - async find(_o: string, ast: any) { - return Array.from(store.values()).filter((r) => matches(r, ast?.where)); + async find(_o: string, ast: any, opts?: any) { + // The caller's bound is applied AFTER the filter and by PRESENCE — a + // limit-blind double silently answers past a bound the engine set, which + // is what `check:objectql-double-limit` exists to keep out of new fakes. + const rows = Array.from(store.values()).filter((r) => matches(r, ast?.where)); + const limit = typeof ast?.limit === 'number' + ? ast.limit + : typeof opts?.limit === 'number' ? opts.limit : undefined; + return typeof limit === 'number' ? rows.slice(0, limit) : rows; }, async findOne(_o: string, ast: any) { for (const r of store.values()) if (matches(r, ast?.where)) return r; From 94b48862eca3950b93d41a569334de6ca991f45f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:17:19 +0000 Subject: [PATCH 6/8] refactor: one loop-scoped recording with per-row windows, so D3's payload identity pin holds unchanged --- .../src/bulk-write-per-row-hooks.test.ts | 22 +++-- packages/objectql/src/engine.ts | 89 +++++++++++-------- .../objectql/src/hook-write-provenance.ts | 43 ++++++++- packages/objectql/src/index.ts | 1 - .../multi-update-hook-key-divergence.test.ts | 14 +-- .../src/multi-update-hook-key-divergence.ts | 56 ++++++------ 6 files changed, 146 insertions(+), 79 deletions(-) diff --git a/packages/objectql/src/bulk-write-per-row-hooks.test.ts b/packages/objectql/src/bulk-write-per-row-hooks.test.ts index 76550d9b1a..8a668c68c3 100644 --- a/packages/objectql/src/bulk-write-per-row-hooks.test.ts +++ b/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 () => { diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 1a5cc05eba..116d840815 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -202,7 +202,6 @@ import { recordHookPayloadWrites } from './hook-write-provenance.js'; import { divergingHookPayloadKeys, MultiUpdateHookKeyDivergenceError, - type PerRowHookWrittenKeys, } from './multi-update-hook-key-divergence.js'; import type { HookWriteRecording } from './hook-write-provenance.js'; import { resolveMasterDetailRelation } from './master-detail.js'; @@ -2970,31 +2969,34 @@ export class ObjectQL implements IObjectQLEngine { ): Promise { const schema = this._registry.getObject(object); const carriesPayload = event === 'beforeUpdate'; - // [#14099] D3's enforcement half — one recorded key set per row. - // `undefined` entries are rows whose recording cannot speak (a hook - // REPLACED the payload object); `divergingHookPayloadKeys` abstains on - // those rather than reading them as an empty set. - const perRowHookWrittenKeys: PerRowHookWrittenKeys[] = []; + // ── [#14099] D3's ENFORCEMENT half: one observation window per row ────── + // + // The #14088 recorder, armed a SECOND time for this loop and NESTED over + // the batch-scoped recording `update()` already armed at its entry. The + // nesting is what makes both readings true at once: a write through this + // view lands on the outer recording's view, which lands on the real + // payload — so the outer record (the one the read-only strips read for + // provenance) still sees every hook write, while `closeWindow()` gives + // this loop what only it needs: the keys assigned by ONE row's chain. + // + // ⭐ ONE recording for the whole loop, not one per row, and that is a + // contract point rather than a saving: every per-row context must carry + // THE SAME payload object (D3, pinned by reference identity in + // `bulk-write-per-row-hooks.test.ts`). A fresh view per row would still be + // write-through onto one payload — no copy, nothing to reconcile — but it + // would hand each row a DIFFERENT object, which is a difference an author + // can observe and which this contract says is not there. + const batchPayloadAtEntry = (batchCtx.input as { data?: unknown }).data; + const hookWrites = + carriesPayload && batchPayloadAtEntry !== null && typeof batchPayloadAtEntry === 'object' + ? recordHookPayloadWrites(batchPayloadAtEntry as Record) + : undefined; + if (hookWrites) (batchCtx.input as { data?: unknown }).data = hookWrites.payload; + const perRowHookWrittenKeys: ReadonlySet[] = []; for (let index = 0; index < rows.length; index++) { const row = rows[index]; const rowId = (row as { id?: unknown }).id; const options = (batchCtx.input as { options?: unknown }).options; - // D3: THE payload, read fresh so a previous row's REPLACEMENT is what - // this row sees. Never a copy. - const batchPayload = (batchCtx.input as { data?: unknown }).data; - // [#14099] The #14088 recorder, armed a SECOND time and per row, nested - // over the batch-scoped recording `update()` already armed at its entry. - // Nesting is what makes both readings true at once: a write through this - // view lands on the outer recording's view, which lands on the real - // payload — so the outer record (which the readonly strips read for - // provenance) still sees every hook write, while this one sees only the - // writes THIS row's chain made. A fresh recording per row is the whole - // point: one recording across the loop would accumulate the union and - // could never tell two rows apart. - const rowRecording = - carriesPayload && batchPayload !== null && typeof batchPayload === 'object' - ? recordHookPayloadWrites(batchPayload as Record) - : undefined; const rowCtx = { ...batchCtx, event, @@ -3004,8 +3006,10 @@ export class ObjectQL implements IObjectQLEngine { // context is a fresh object, so a stash written on the context itself // dies with the row that held it. dispatch: { ...(batchCtx.dispatch as object), index } as HookContext['dispatch'], + // D3: THE payload, read fresh so a previous row's REPLACEMENT is what + // this row sees. Never a copy. input: carriesPayload - ? { id: rowId, data: rowRecording?.payload ?? batchPayload, options } + ? { id: rowId, data: (batchCtx.input as { data?: unknown }).data, options } : { id: rowId, options }, previous: coerceBooleanFields(schema as any, row as any), // D2: no post-state in the before phase. @@ -3016,15 +3020,10 @@ export class ObjectQL implements IObjectQLEngine { // D3, the accumulate half — see the class doc above. if (carriesPayload) { - // Sealing does the accumulate write-back AND closes this row's record. - // It is what keeps a recording VIEW out of `batchCtx.input.data` — the - // next row, the outer seal and eventually the driver must all see the - // raw payload (or the hook's replacement), never a proxy of it. - const sealed = rowRecording?.seal((rowCtx.input as { data?: unknown }).data); - (batchCtx.input as { data?: unknown }).data = sealed - ? sealed.data - : (rowCtx.input as { data?: unknown }).data; - if (rowRecording) perRowHookWrittenKeys.push(sealed?.hookWrittenKeys); + (batchCtx.input as { data?: unknown }).data = (rowCtx.input as { data?: unknown }).data; + // [#14099] and the enforcement half: close THIS row's window, so the + // next row opens a fresh one. + if (hookWrites) perRowHookWrittenKeys.push(hookWrites.closeWindow()); } // D4. const observed = (rowCtx.input as { id?: unknown }).id; @@ -3035,21 +3034,37 @@ export class ObjectQL implements IObjectQLEngine { } } + // [#14099] Close this loop's recording and put the underlying payload back + // in `batchCtx.input.data`, so no recording VIEW travels on to the outer + // seal, the strips or a driver. + // + // `hookWrittenKeys` is `undefined` exactly when a hook REPLACED the payload + // object somewhere in this batch (`hook-write-provenance.ts`'s KNOWN + // LIMIT). Then the whole batch ABSTAINS: the windows collected before the + // replacement describe writes the replacement discarded, so refusing on + // them would be a verdict about a payload that is no longer the one being + // written. Abstaining keeps the pre-#14099 behaviour for that shape, which + // is the same fail-safe direction #14088 chose for the same limit. + const sealedLoopWrites = hookWrites?.seal((batchCtx.input as { data?: unknown }).data); + if (sealedLoopWrites) { + (batchCtx.input as { data?: unknown }).data = sealedLoopWrites.data; + } + // [#14099] The refusal, ruled 2026-09-02 (recommendation C). Placed after // the loop and not inside it, for two reasons that are both about the - // envelope rather than about cost: the diverging set is `union \ - // intersection` over EVERY row, so the message names every offending key + // envelope rather than about cost: the diverging set is `union` minus + // `intersection` over EVERY row, so the message names every offending key // instead of the first pair to disagree, and it is order-independent — the // same batch answers the same way whatever order the driver returned the // matched rows in. // // ⭐ Still BEFORE any write, which is the load-bearing half. This method is - // called from `update()`'s predicate branch ahead of the hook-write seal, - // both readonly strips, `evaluateValidationRules` and every + // called from `update()`'s predicate branch ahead of the outer hook-write + // seal, both readonly strips, `evaluateValidationRules` and every // `driver.updateMany` — and it runs outside `update()`'s own `try`, so the // envelope reaches the caller undecorated. Not "after the first row", not // "inside a transaction that then rolls back": nothing was written. - if (carriesPayload) { + if (sealedLoopWrites?.hookWrittenKeys !== undefined) { const diverging = divergingHookPayloadKeys(perRowHookWrittenKeys); if (diverging.length > 0) { throw new MultiUpdateHookKeyDivergenceError(object, diverging, rows.length); diff --git a/packages/objectql/src/hook-write-provenance.ts b/packages/objectql/src/hook-write-provenance.ts index 10fb16e6b0..81d7b30d1b 100644 --- a/packages/objectql/src/hook-write-provenance.ts +++ b/packages/objectql/src/hook-write-provenance.ts @@ -91,6 +91,31 @@ export interface HookWriteRecording { * exactly as it always has. */ readonly payload: Record; + /** + * [#14099] Close the current OBSERVATION WINDOW and open a fresh one: the + * keys assigned since the previous `closeWindow()` — or since arming, for + * the first call — and nothing before that. + * + * The cumulative record {@link HookWriteRecording.seal} returns is NOT + * affected, and that separation is the whole point. One recording answers + * two different questions on the predicate-update path, and the answers must + * not be the same set: + * + * - "did a HOOK assign this key at all?" — cumulative, what the read-only + * strips read for provenance (#14088, the reason this module exists); + * - "did THIS row's dispatch assign it?" — windowed, what ADR-0058 + * Addendum II D3's enforcement compares between rows (#14099). A batch of + * N rows closes N windows; a cumulative set could never tell two rows + * apart, because the second row writing the same key does not grow it. + * + * A window mirrors the cumulative record's own rule on removal: a key + * deleted during the window leaves it, so the window means "the keys holding + * a value this row's chain assigned", never "the names this row touched". + * + * After {@link HookWriteRecording.seal} the recording records nothing more, + * so this returns an empty set rather than a stale one. + */ + closeWindow(): ReadonlySet; /** * Close the recording and hand back the payload the rest of the write must * use. @@ -137,6 +162,12 @@ export interface SealedHookWrites { */ export function recordHookPayloadWrites(target: Record): HookWriteRecording { const written = new Set(); + // [#14099] The windowed twin of `written` — same entries, cleared at each + // `closeWindow()`. Kept beside the cumulative set rather than derived from + // it, because a derivation cannot exist: two rows writing the SAME key grow + // the cumulative set once, so a delta would report the second row as having + // written nothing, which is the exact false reading D3's enforcement turns on. + let windowWrites = new Set(); let sealed = false; const record = (key: string | symbol): void => { @@ -146,6 +177,7 @@ export function recordHookPayloadWrites(target: Record): HookWr // collide with a real key's provenance. if (sealed || typeof key !== 'string') return; written.add(key); + windowWrites.add(key); }; const payload = new Proxy(target, { @@ -173,13 +205,22 @@ export function recordHookPayloadWrites(target: Record): HookWr // records it again. Dropping it here keeps the set meaning "a hook // assigned the value standing on this key" rather than "a hook once // touched this name". - if (ok && typeof key === 'string') written.delete(key); + if (ok && typeof key === 'string') { + written.delete(key); + windowWrites.delete(key); + } return ok; }, }); return { payload, + closeWindow(): ReadonlySet { + if (sealed) return new Set(); + const closed = windowWrites; + windowWrites = new Set(); + return closed; + }, seal(current: unknown): SealedHookWrites { sealed = true; if (current !== payload) { diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index e3b62d07ec..9a6dc785f3 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -125,7 +125,6 @@ export { MULTI_UPDATE_HOOK_KEY_DIVERGENCE_STATUS, divergingHookPayloadKeys, } from './multi-update-hook-key-divergence.js'; -export type { PerRowHookWrittenKeys } from './multi-update-hook-key-divergence.js'; // Boot guard: thrown by `ObjectQL.init()` when a registered driver's connect() // fails (framework#3741). Hosts that boot the engine themselves can catch it to // render their own "database unreachable" message. diff --git a/packages/objectql/src/multi-update-hook-key-divergence.test.ts b/packages/objectql/src/multi-update-hook-key-divergence.test.ts index 3bf790617c..dd7109aac7 100644 --- a/packages/objectql/src/multi-update-hook-key-divergence.test.ts +++ b/packages/objectql/src/multi-update-hook-key-divergence.test.ts @@ -445,13 +445,13 @@ describe('[#14099] divergingHookPayloadKeys', () => { expect(divergingHookPayloadKeys([S('a')])).toEqual([]); }); - it('`undefined` ABSTAINS — it is "cannot say", never an empty set', () => { - // Reading `undefined` as `{}` would refuse this batch on evidence that does - // not exist. Both recorded rows agree, so nothing diverges. - expect(divergingHookPayloadKeys([S('a'), undefined, S('a')])).toEqual([]); - expect(divergingHookPayloadKeys([undefined, undefined])).toEqual([]); - // …and rows that CAN speak are still compared with each other. - expect(divergingHookPayloadKeys([S('a'), undefined, S('b')])).toEqual(['a', 'b']); + it('rows with no writes at all agree with each other', () => { + // The abstention case — a hook REPLACED the payload, so the recording can + // say nothing — never reaches this function: the engine skips the + // comparison outright when its seal returns no record (pinned end-to-end in + // §5). ⛔ So an absent row must never be modelled here as an empty set; + // these are real windows that happen to be empty. + expect(divergingHookPayloadKeys([S(), S(), S()])).toEqual([]); }); it('a key DELETED on one row and assigned on another diverges', () => { diff --git a/packages/objectql/src/multi-update-hook-key-divergence.ts b/packages/objectql/src/multi-update-hook-key-divergence.ts index acd77ca4fd..8a74cd35b6 100644 --- a/packages/objectql/src/multi-update-hook-key-divergence.ts +++ b/packages/objectql/src/multi-update-hook-key-divergence.ts @@ -66,45 +66,45 @@ * module does not widen to cover it, and a future author reaching for a value * comparison to close it must re-read the two measurements above first. * - * ## Divergence is `union \ intersection`, and rows that cannot speak abstain + * ## Divergence is `union` minus `intersection` * - * The diverging keys are every key some row's recording holds and some other + * The diverging keys are every key some row's window holds and some other * row's does not — order-independent by construction, so the envelope names - * the same keys whatever order the driver returned the matched rows in. - * - * A row whose recording is `undefined` — a hook REPLACED `ctx.input.data` - * rather than mutating it, the KNOWN LIMIT `hook-write-provenance.ts` - * documents — is EXCLUDED from the comparison rather than treated as an empty - * set. `undefined` means "this call cannot say", and refusing a batch on a - * measurement that was never taken would be a fabricated verdict. That keeps - * the pre-#14099 behaviour for payload-replacing hooks, which is the same - * fail-safe direction #14088 chose for the same limit: keep the old bug rather - * than act on evidence that does not exist. + * the same keys whatever order the driver returned the matched rows in, and it + * names ALL of them rather than the first pair to disagree. + * + * ## When the recording cannot speak, the batch is not judged at all + * + * That decision does not live here, deliberately: it is the ENGINE that knows + * whether its recording survived the batch. A hook may REPLACE + * `ctx.input.data` rather than mutate it, and the replacement's keys are + * indistinguishable from the caller's (`hook-write-provenance.ts`'s KNOWN + * LIMIT) — so `seal` returns no record, and the engine skips this comparison + * entirely instead of feeding it windows that describe a payload the batch no + * longer writes. Refusing on a measurement that no longer applies would be a + * fabricated verdict; abstaining keeps the pre-#14099 behaviour for that + * shape, which is the same fail-safe direction #14088 chose for the same + * limit. This function therefore takes real key sets only, and reading a + * missing row as an empty set is not a case it can be handed. */ -/** - * One row's recorded key set, as {@link SealedHookWrites.hookWrittenKeys} - * hands it back: the keys that row's hook chain assigned, or `undefined` when - * the call has no attributable record. - */ -export type PerRowHookWrittenKeys = ReadonlySet | undefined; - /** * The keys whose presence in the hook chain's writes DIFFERS across the rows - * of one predicate update, sorted; `[]` when every row that could be recorded - * agreed (which includes the cases of one row, and of no row able to speak). + * of one predicate update, sorted; `[]` when every row agreed (which includes + * a batch of one row, and a batch where no hook wrote anything). * - * Pure and total — it never throws and never reads the engine. The engine - * raises; the contract decides. + * Each entry is one row's OBSERVATION WINDOW — + * {@link HookWriteRecording.closeWindow}'s return, the keys that row's chain + * assigned. Pure and total: it never throws and never reads the engine. The + * engine raises; the contract decides. */ -export function divergingHookPayloadKeys(perRow: readonly PerRowHookWrittenKeys[]): string[] { - const recorded = perRow.filter((s): s is ReadonlySet => s !== undefined); - if (recorded.length < 2) return []; +export function divergingHookPayloadKeys(perRow: readonly ReadonlySet[]): string[] { + if (perRow.length < 2) return []; const union = new Set(); - for (const set of recorded) for (const key of set) union.add(key); + for (const set of perRow) for (const key of set) union.add(key); const diverging: string[] = []; for (const key of union) { - if (!recorded.every((set) => set.has(key))) diverging.push(key); + if (!perRow.every((set) => set.has(key))) diverging.push(key); } return diverging.sort(); } From 1bac457e434deb0e23ffc705126c0824bd6571c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:22:07 +0000 Subject: [PATCH 7/8] docs: re-anchor system-context census after the engine refactor --- content/docs/permissions/system-context.mdx | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b5932ab52d..3f5b2606ac 100644 --- a/content/docs/permissions/system-context.mdx +++ b/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:11274` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11457` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10009` | +| 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:10057`, `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:5876` | -| 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:3720`, `:3730`, `:3757` | +| 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:6574` | -| 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:12069` | -| 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:11998` | +| 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:3527` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14418` | 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:9992`–`10009` | +| "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` | From a59f92f375c62e54ecb87a47f80ed0e892caacb3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 23:13:13 +0000 Subject: [PATCH 8/8] docs(objectql): name the row whose value actually survives the divergence blind spot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new module's "What is NOT covered" paragraph said the residue applies "the first row's value to every matched row", and the residue pin's own title said "row 1's value". Both are contradicted by the assertions directly beneath them: per-row rewrites accumulate onto ONE payload in dispatch order, so the LAST assignment to a key is what the single SET clause carries. The pins already measure it — `bulk-write-per-row-hooks.test.ts`'s D3 case reads `['stamped-2','stamped-2']`, and the residue pin reads `low` on the row whose own dispatch derived `high`. Prose only; no behaviour, no assertion and no exported symbol changes. The ruling's verbatim quotation is untouched — the correction is stated beside it, naming what the ruling said and what the engine does, because a docblock that names the wrong row sends the next author hunting for a per-row seam that does not exist. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../multi-update-hook-key-divergence.test.ts | 2 +- .../src/multi-update-hook-key-divergence.ts | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/objectql/src/multi-update-hook-key-divergence.test.ts b/packages/objectql/src/multi-update-hook-key-divergence.test.ts index dd7109aac7..9abe179c3f 100644 --- a/packages/objectql/src/multi-update-hook-key-divergence.test.ts +++ b/packages/objectql/src/multi-update-hook-key-divergence.test.ts @@ -333,7 +333,7 @@ describe('[#14099] a row-invariant rewrite is never refused, however its values * ──────────────────────────────────────────────────────────────────────────── */ describe('[#14099] same key + per-row VALUES still passes — D3’s declared cost', () => { - it('a per-row derived value is NOT refused, and row 1’s value reaches every row', async () => { + it('a per-row derived value is NOT refused, and the LAST dispatch’s value reaches every row', async () => { // ⚠️ This is the ruling's named blind spot, pinned so it cannot change by // accident in either direction. It is filed as its own finding with a // measured instance; ⛔ it is NOT widened into this card, and ⛔ the fix is diff --git a/packages/objectql/src/multi-update-hook-key-divergence.ts b/packages/objectql/src/multi-update-hook-key-divergence.ts index 8a74cd35b6..0301b0d25b 100644 --- a/packages/objectql/src/multi-update-hook-key-divergence.ts +++ b/packages/objectql/src/multi-update-hook-key-divergence.ts @@ -60,11 +60,20 @@ * ## What is NOT covered, named rather than hidden * * A hook that writes the SAME key on every row but with per-row VALUES (a - * per-row derived priority, say) passes this test and still applies the first - * row's value to every matched row. That is D3's cost by design; the ruling - * carries it openly and points at the prescription below as the exit. This - * module does not widen to cover it, and a future author reaching for a value - * comparison to close it must re-read the two measurements above first. + * per-row derived priority, say) passes this test and still applies ONE row's + * value to every matched row. The ruling's own sentence says "the first row's + * value"; the engine's MEASURED behaviour is the LAST dispatch's, because the + * per-row rewrites accumulate onto one payload in dispatch order and the last + * assignment to a key is what the single `SET` clause carries. Pinned in both + * suites — `bulk-write-per-row-hooks.test.ts`'s D3 case reads + * `['stamped-2','stamped-2']`, and this module's own residue pin reads `low` + * on the row whose own dispatch derived `high`. Which row wins changes nothing + * about the ruling's verdict; it is corrected here because a docblock naming + * the wrong row sends the next author hunting for a per-row seam that does not + * exist. That is D3's cost by design; the ruling carries it openly and points + * at the prescription below as the exit. This module does not widen to cover + * it, and a future author reaching for a value comparison to close it must + * re-read the two measurements above first. * * ## Divergence is `union` minus `intersection` *