diff --git a/.changeset/bulk-write-before-hook-per-row-contract.md b/.changeset/bulk-write-before-hook-per-row-contract.md new file mode 100644 index 0000000000..cf3d3cf2a5 --- /dev/null +++ b/.changeset/bulk-write-before-hook-per-row-contract.md @@ -0,0 +1,44 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): 定形 `multi: true` 批量写在 `before*` 阶段的按行 hook 契约(#6462) + +#5574 的维护者裁决(2026-08-06,方案 B)把「批量写按行语义为平台契约」(#4800 / +#4862,after 侧已由 #5038 交付)延伸到 `before` 型 hook,并指定 **contract-first +拆分:spec 契约子单先行,engine 实现随后**。本次变更是该拆分的 spec 半边 —— 只落 +契约、pin 测试与 ADR 附录,**engine 一行未动**。 + +**为什么需要这条契约。** 谓词写路径上 `ctx.previous` 在 before 阶段从未被绑定,于是 +每一个按守卫写法写出来的 hook —— `if (ctx.previous?.locked) throw` —— 在批量写上 +静默放行。hotcrm 实测:一次批量编辑绕过全部 15 个守卫 hook,把单行路径会拒绝的 +`readonly: true` 字段写成 `null`。失效方向是 fail-open,而让它静默的可选链正是 AI +会写出的形状。 + +**新增契约面** `@objectstack/spec/data` → `bulk-write-hook-conformance.ts`: + +- `BULK_WRITE_HOOK_DISPATCH_CONTRACT` —— 四个写事件的按行派发表(before/after × + update/delete),逐条声明 per-row 上下文携带哪些键、载荷作用域,以及 **`delivered` + 标记**:after 半边由 #5038 交付(`true`),before 半边为已裁未交(`false`, + engine 半边 = #5574 engine 卡)。契约先行必须能被读出「尚未交付」,否则它就是一次 + declared ≠ enforced。 +- `MAX_BULK_PER_ROW_HOOK_ROWS` / `BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE` / + `resolveBulkPerRowHookBudget()` —— `assertBulkPerRowHookBudget` 先例的契约级表述: + **两个阶段共用一个上限**,超限在**首次按行派发之前**整单拒绝(什么也不写、一个 + handler 也不跑),永不降级成「整批一次派发」。 + +**载荷可改写语义(裁决必答项 1)的答案是:载荷仍然只有一份,作用域是整批。** 每个 +per-row 上下文拿到的是**同一个** payload,而不是逐行副本 —— 于是 N 份载荷不可能分叉, +没有合并步骤,谓词写永远不会被拆成 N 次单行写(仍是一次 `updateMany`、一个受影响行数 +#4639)。逐行副本 + 「一致则合并、分叉则拒绝」这条更显然的路线被实测证据否掉:objectql +自带的 `sys_stamp_audit_update` 注册在 `'*'` 上,且在**每行**的 stamp 内部读 +`new Date()`,跨毫秒的两行 `updated_at` 天然不同 —— 该规则会非确定性地拒绝正常批量写。 + +**行为不变。** 本次不改任何 Zod schema 的接受面,不新增可授权键,不动 engine:今天能 +通过校验的元数据,改动后逐字节仍然通过。`hook.zod.ts` 的 `input` 形状表继续描述引擎 +**当前**的构造(由 objectql 对真实派发钉住),只是补了一条指向新契约的前瞻说明 —— +表与引擎不允许抢跑,这正是 #5273 的教训。 + +ADR-0058 新增 **Addendum II**,同时收纳裁决指定的 #5748 半边(`data.id` 与 +`where.id` 统一走标量测试,已由 PR #5919 交付),并把 Addendum I 中「`before*` hooks +are NOT per row」一段标记为 **SUPERSEDED**(原文保留 —— 被推翻的决策本身是记录)。 diff --git a/docs/adr/0058-expression-and-predicate-surface.md b/docs/adr/0058-expression-and-predicate-surface.md index 6e6e34e65d..1a97ce35c3 100644 --- a/docs/adr/0058-expression-and-predicate-surface.md +++ b/docs/adr/0058-expression-and-predicate-surface.md @@ -119,7 +119,13 @@ > Falling back to one dispatch for the batch would skip the hook for N-1 rows > silently — the failure shape this whole family exists to abolish. > -> **`before*` hooks are NOT per row, and that is not a version gap.** A +> **`before*` hooks are NOT per row, and that is not a version gap.** +> ⚠️ **SUPERSEDED by Addendum II below (#5574, ruling B).** The maintainer +> reversed the "not a version gap" half on measured evidence: `before*` hooks +> ARE dispatched per row on a predicate write. What survives verbatim is the +> reason given here for the payload — one `updateMany` carries one payload — +> which Addendum II keeps as its D3 rather than overturning. Kept in place, not +> rewritten, because a reversed decision is a record (Prime Directive #13). A > `beforeUpdate` / `beforeDelete` fires once for the whole batch because it may > still rewrite the payload, and one `updateMany` carries one payload — there is > nothing per-row to hand it. So #5037's `HookConditionError` and its @@ -142,6 +148,153 @@ --- +> **Addendum II (2026-08, #5748 / #5574 / #6462) — BULK-WRITE, part two: which +> writes ARE bulk writes, and what a bulk write does to the `before*` phase.** +> _Contract recorded here. The `data.id` half is **implemented** (#5748 / PR +> #5919). The `before*` half is the **spec** side of a deliberate contract-first +> split (#6462); the engine side is #5574's engine card, `Blocked-by` it._ +> +> Addendum I answered "what is a record-scoped declaration evaluated over when +> one write touches N rows" for the `after*` phase. Two questions it left are +> answered here, because a maintainer ruled on both on 2026-08-06 and directed +> them into one appendix. +> +> --- +> +> ### Part A — which writes are bulk writes (#5748, delivered) +> +> `ObjectQL.update(object, data, options)` took its id from two sources with two +> different rules: `where.id` went through a scalar test (an operator object, an +> array or `null` is a predicate, not an id), while `data.id` went through none +> at all and outranked both `where` and an explicit `options.multi`. So +> `update(o, { id: { $in: ['a','b'] }, title: 'x' }, { multi: true })` bound a +> serialized operator object as a **primary key** and silently discarded the +> declared bulk intent. +> +> **The decision (ruling A, 2026-08-06).** One scalar test, defined once and +> reused on both sides: a non-scalar `data.id` is not an id, so the same call +> now dispatches to `updateMany` and honours the declared `multi: true`. Route B +> (reject non-scalar `data.id` outright) was considered and refused — it would +> have made "the author put the predicate in the wrong slot" fatal for a call +> whose intent is unambiguous once the two halves agree. The concern behind B +> was converted into a required test instead, and #5919 landed 16 assertions of +> it: a non-scalar `data.id` **without** `multi: true` rejects with zero driver +> calls, so a typo is never silently promoted into a real batch write. +> +> Two consequences landed after it, same rule one layer on: a value already +> ruled not-a-primary-key does not get to sit in the primary-key column of the +> SET clause either — stripped with a warning on the predicate branch (#6262) +> and on the by-id branch (#6435). +> +> Why this belongs in the bulk-write appendix and not in a dispatch ADR of its +> own: everything Addendum I says is conditioned on a write BEING a predicate +> write. A rule that decides that question silently, and differently depending +> on which slot the caller used, is the entry to this whole surface. +> +> --- +> +> ### Part B — the `before*` phase is per row too (#5574 ruling B; spec half #6462) +> +> **What Addendum I got right and what it got wrong.** It reasoned: one +> `updateMany` carries one payload, a `before*` hook may rewrite the payload, +> therefore there is nothing per-row to hand it, therefore the dispatch is +> batch-scoped. The premise is true; the conclusion does not follow. The payload +> is not the only thing a `before*` handler reads — `previous` is — and binding +> a per-row pre-image needs no per-row payload at all. +> +> **The measured harm (#5574).** On the predicate path `ctx.previous` was never +> assigned in the before phase, so every guard hook written the way guards are +> written — `if (ctx.previous?.locked) throw` — passed silently. A hotcrm +> deployment measured all **15** of its guard hooks bypassed by one batch edit, +> including writing `null` into a `readonly: true` field that the single-id path +> refuses. The failure direction is fail-OPEN, and the optional chaining that +> makes it silent is exactly what an AI writes. The alternative on the table +> (option A: document the limitation) was refused for that reason — it puts the +> hole in the manual and leaves it in the product. +> +> **The decision.** A predicate write dispatches `beforeUpdate` / `beforeDelete` +> **once per matched row**, on a single-record-shaped context, replacing the +> single batch dispatch — the same move #5038 made for `after*`, held to the +> same yardstick. The full clause set, with the reasoning that does not fit +> here, is `packages/spec/src/data/bulk-write-hook-conformance.ts` (D1–D7), and +> `BULK_WRITE_HOOK_DISPATCH_CONTRACT` is that table machine-readable, carrying a +> `delivered` flag per event so the contract-first gap cannot read as delivered. +> The load-bearing clauses: +> +> - **Per-row context (D1/D2).** `input.id` names the row, `previous` is its +> pre-image, `input.options` is still the caller's bag (the PHASE rule is +> unchanged), `result` stays absent — the before phase has no post-state. +> Zero matched rows is zero dispatches. +> - **The payload stays BATCH-scoped, and that IS the merge rule (D3).** Every +> per-row context carries the one payload, not a copy. A rewrite therefore +> applies to the whole batch whoever made it, rewrites accumulate in dispatch +> order, N post-hook payloads cannot diverge, nothing is reconciled, and no +> predicate write is ever split into N single-row writes. One `updateMany`, +> one affected count (#4639), one aggregate `data.records.updated`. A rewrite +> *conditioned* on the row is out of contract: it widens to every matched row +> rather than scoping itself. Per-row `previous` is supplied so a guard can +> REFUSE, not so a rewrite can be aimed. +> - **`input.id` stops being a reroute lever, on this path only (D4).** A +> per-row context arrives with `id` already bound and the dispatch decided, so +> rebinding it retargets nothing; it is refused rather than ignored, because a +> silent no-op is the failure this family exists to abolish. +> - **One ceiling, both phases (D6).** `MAX_BULK_PER_ROW_HOOK_ROWS` (10 000) +> governs `before*` exactly as it governs `after*`, and the check runs before +> the FIRST per-row dispatch — so an over-ceiling batch runs zero handlers and +> writes nothing, rather than running 10 001 and then throwing. Still a +> refusal, never a downgrade. `resolveBulkPerRowHookBudget` is the rule, +> executable, and the engine half replaces its open-coded copy with it. +> - **One read, reused (D7).** The row set is read once, with the composed AST +> the write binds, and serves validation (#3106), the `readonlyWhen` strip +> (#3042) and both per-row dispatches. The ruling forbids a second fetch in as +> many words. +> +> **Why per-row payload COPIES plus a reconciliation rule was rejected.** It is +> the obvious alternative and it is defeated by a measured fact, so the evidence +> is recorded rather than left to be rediscovered: objectql's own +> `sys_stamp_audit_update` builtin is registered on `'*'` and reads +> `new Date().toISOString()` **inside** the per-record stamp. Under per-row +> dispatch that is one clock read per row, so rows either side of a millisecond +> boundary carry different `updated_at` values — a converge-or-refuse rule would +> refuse honest batches non-deterministically, and a converge-or-split rule +> would shatter one `updateMany` into N writes for the same reason. Beside that, +> nothing measured needs divergent payloads: every `beforeUpdate` payload +> rewrite in the repo (the audit stamp, plugin-pinyin-search's companion +> projection, service-storage's copy-on-claim) is row-invariant. Refusing +> divergence also stays reversible in the safe direction — a later ADR can relax +> it, while a write that has learned to split itself cannot be un-split. +> +> **The consequences, priced as this appendix's predecessor requires.** +> +> - **`bulk_write_previous_unbound` becomes unreachable.** #5038 retired +> `HookConditionError` for after-type hooks and Addendum I kept it alive +> "rescoped to the before dispatch". Once the engine half lands there is no +> dispatch left without a bound `previous`, and `isPredicateBulkWrite` +> (`hook-wrappers.ts`) — whose whole test is "no `input.id` and `multi`" — +> answers `false` for the before phase too. Both `HookConditionLimitation` +> members then have neither producer nor reachable consumer, which is an +> ADR-0049 enforce-or-remove item for the engine card, not a gap. +> - **The demand becomes effectively universal, and that is #5846's bill.** The +> engine gates the row-set read on `hasHooksFor('beforeUpdate', object)`, and +> objectql registers `sys_stamp_audit_update` and `sys_fetch_previous_update` +> on `'*'` — so the gate is true for every object wherever the plugin is +> loaded. Making those builtins express in their registration what their +> handlers already decide at run time is #5846's, already scoped there. +> - **The dispatch ladder must be resolved BEFORE the before phase**, since the +> row set has to be read to build the per-row contexts. That reorders a seam +> #5846 also owns (its (a) direction moves the prior read ahead of +> `beforeUpdate` and binds it), and it interacts with one existing capability +> — a `beforeUpdate` handler that CLEARS `input.id` on a by-id call currently +> converts the write into a predicate write. Deliberately **not** settled +> here: it is a live-behaviour question on a seam another card owns, and this +> appendix will not presume the answer. The engine half and #5846 settle it +> together, in one edit to one ordering, and record it as an amendment. +> - **`scripts/adr-anchors.json`'s `hook-wrappers.ts` invariant still describes +> the batch dispatch.** It is TRUE today and must move with the engine half, +> not before it. + +--- + ## TL;DR ObjectStack exposes **~50 authorable declarations** that hold an expression — formulas, visibility/required/readonly predicates, validation rules, hook conditions, flow/edge conditions, sharing-rule conditions, RLS `using`/`check`, action/view/app visibility, notification/ETL/export/sync/connector conditions — and they all funnel through **one authoring primitive** (`ExpressionInputSchema` → `{ dialect: 'cel', source }`, helpers `cel`/`F`/`P`). The authoring surface is already unified and clean. diff --git a/packages/spec/api-surface/data.json b/packages/spec/api-surface/data.json index ba22d16e62..449efec319 100644 --- a/packages/spec/api-surface/data.json +++ b/packages/spec/api-surface/data.json @@ -33,10 +33,16 @@ "AutonumberToken (type)", "BOOLEAN_VALUE_TYPES (const)", "BUILTIN_DRIVER_IDS (const)", + "BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE (const)", + "BULK_WRITE_HOOK_DISPATCH_CONTRACT (const)", "BaseEngineOptions (type)", "BaseEngineOptionsSchema (const)", "BaseValidationRuleShape (interface)", "BuiltinDriverId (type)", + "BulkPerRowHookBudgetVerdict (type)", + "BulkWriteHookDispatchContractEntry (interface)", + "BulkWriteHookPhase (type)", + "BulkWritePayloadScope (type)", "CALENDAR_DATE_TYPES (const)", "CLOCK_TIME_TYPES (const)", "COMPUTED_VALUE_TYPES (const)", @@ -299,6 +305,7 @@ "LocationCoordinatesSchema (const)", "LocationValueSchema (const)", "LogicalOperatorKey (type)", + "MAX_BULK_PER_ROW_HOOK_ROWS (const)", "MEASURE_FIELD_TYPES (const)", "MULTI_CAPABLE_TYPES (const)", "MULTI_OPTION_TYPES (const)", @@ -606,6 +613,7 @@ "referenceTargetOf (function)", "referencedFields (function)", "renderAutonumber (function)", + "resolveBulkPerRowHookBudget (function)", "resolveCrudAffordances (function)", "resolveDisplayField (function)", "resolveDriverId (function)", diff --git a/packages/spec/src/data/bulk-write-hook-conformance.test.ts b/packages/spec/src/data/bulk-write-hook-conformance.test.ts new file mode 100644 index 0000000000..e4ca4e986c --- /dev/null +++ b/packages/spec/src/data/bulk-write-hook-conformance.test.ts @@ -0,0 +1,206 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Pins the bulk-write hook dispatch contract (ADR-0058 Addendum II, #6462). + * + * These assertions are over the CONTRACT, not over an engine dispatch — spec + * cannot execute one, and must not depend on objectql to try. The engine-side + * half of the same table is pinned against a real dispatch by + * `packages/objectql/src/hook-input-shape-contract.test.ts`; the two meet at + * the event names and the per-row context keys, which is what "same yardstick + * as the after side" means in practice. + * + * The load-bearing case here is `delivered`. Two entries are false today + * because the ruling ordered a contract-first split, and a contract that says + * "not yet" has exactly one way to stay honest: a pin that goes RED the moment + * the engine catches up, so the flag is flipped in the same PR that earns it + * rather than years later by someone re-reading this file. That is deliberately + * the inverse of a normal pin — it is asserting an absence, and it is supposed + * to fail one day. + */ +import { describe, expect, it } from 'vitest'; + +import { + BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE, + BULK_WRITE_HOOK_DISPATCH_CONTRACT, + MAX_BULK_PER_ROW_HOOK_ROWS, + resolveBulkPerRowHookBudget, +} from './bulk-write-hook-conformance'; +import { HookEvent } from './hook.zod'; + +const byEvent = (event: string) => { + const entry = BULK_WRITE_HOOK_DISPATCH_CONTRACT.find((e) => e.event === event); + if (!entry) throw new Error(`no contract entry for ${event}`); + return entry; +}; + +describe('bulk-write hook dispatch contract — the table', () => { + it('covers exactly the four write events a predicate write can dispatch', () => { + // Cross-checked against `HookEvent` rather than hardcoded twice: a new + // write event added to the vocabulary without a row here would be a + // silent hole in the contract, which is the #5273 shape (a table and a + // producer that stopped agreeing) pointed the other way. + const writeEvents = HookEvent.options.filter( + (e) => /^(before|after)(Update|Delete)$/.test(e), + ); + expect([...writeEvents].sort()).toEqual( + BULK_WRITE_HOOK_DISPATCH_CONTRACT.map((e) => e.event).sort(), + ); + // Insert is deliberately absent: `insertMany` carries N payloads, so + // batch insert has dispatched one context per row since #2922 and was + // never part of the bulk-write question. + expect(BULK_WRITE_HOOK_DISPATCH_CONTRACT.map((e) => e.event)).not.toContain('beforeInsert'); + }); + + it('binds a per-row `previous` on every entry — in BOTH phases (D1/D2)', () => { + // The whole ruling in one assertion. The measured harm on #5574 was a + // guard hook reading `previous?.x` on a bulk write, where `previous` + // was permanently undefined, so the guard passed silently and a + // readonly field could be nulled across a batch. + for (const entry of BULK_WRITE_HOOK_DISPATCH_CONTRACT) { + expect(entry.contextKeys).toContain('previous'); + expect(entry.contextKeys).toContain('id'); + expect(entry.contextKeys).toContain('options'); + } + }); + + it('gives `result` to the after phase only (D2)', () => { + for (const entry of BULK_WRITE_HOOK_DISPATCH_CONTRACT) { + expect(entry.contextKeys.includes('result')).toBe( + entry.phase === 'after' && entry.event === 'afterUpdate', + ); + } + // A bulk DELETE has no post-state, so its per-row context sets no + // `result` and consumers fall back to `previous` — Addendum I's wording, + // unchanged by Addendum II. + expect(byEvent('afterDelete').contextKeys).not.toContain('result'); + }); + + it('scopes the update payload to the BATCH and gives delete none (D3)', () => { + expect(byEvent('beforeUpdate').payloadScope).toBe('batch'); + expect(byEvent('afterUpdate').payloadScope).toBe('batch'); + expect(byEvent('beforeDelete').payloadScope).toBe('none'); + expect(byEvent('afterDelete').payloadScope).toBe('none'); + + // The merge rule, stated as the absence of a merge: no entry declares a + // per-row payload, so N post-hook payloads cannot diverge, nothing is + // reconciled and no predicate write is ever split into N single-row + // writes. One `updateMany`, one affected count (#4639). + const scopes = new Set(BULK_WRITE_HOOK_DISPATCH_CONTRACT.map((e) => e.payloadScope)); + expect([...scopes].sort()).toEqual(['batch', 'none']); + + // And the payload key rides exactly the entries that have one. + for (const entry of BULK_WRITE_HOOK_DISPATCH_CONTRACT) { + expect(entry.contextKeys.includes('data')).toBe(entry.payloadScope === 'batch'); + } + }); + + it('holds the before and after halves to the same shape (the "same yardstick" test)', () => { + // Per event kind, the before entry's keys must be the after entry's + // minus `result` — which is the equivalence D5 declares, expressed over + // the table instead of over prose. + for (const kind of ['Update', 'Delete'] as const) { + const before = byEvent(`before${kind}`); + const after = byEvent(`after${kind}`); + expect([...before.contextKeys].sort()).toEqual( + [...after.contextKeys].filter((k) => k !== 'result').sort(), + ); + expect(before.payloadScope).toBe(after.payloadScope); + } + }); +}); + +describe('bulk-write hook dispatch contract — delivery status', () => { + it('records the after half as DELIVERED by #5038', () => { + expect(byEvent('afterUpdate')).toMatchObject({ delivered: true, engineDeliveryIssue: 5038 }); + expect(byEvent('afterDelete')).toMatchObject({ delivered: true, engineDeliveryIssue: 5038 }); + }); + + it('records the before half as CONTRACTED but not yet delivered (#5574 engine half)', () => { + // ⚠️ This case is meant to go red exactly once. When the engine half of + // #5574 lands per-row `before*` dispatch, flip both `delivered` flags + // and this expectation in that PR — and move `hook.zod.ts`'s + // `HookContextSchema.input` shape table with it, since that table + // describes the engine as built and is pinned against a real dispatch + // in objectql. A green run here after the engine lands would mean the + // contract and the producer had drifted apart in the direction nobody + // checks (#5273). + expect(byEvent('beforeUpdate')).toMatchObject({ delivered: false, engineDeliveryIssue: 5574 }); + expect(byEvent('beforeDelete')).toMatchObject({ delivered: false, engineDeliveryIssue: 5574 }); + + const undelivered = BULK_WRITE_HOOK_DISPATCH_CONTRACT.filter((e) => !e.delivered); + expect(undelivered.map((e) => e.event)).toEqual(['beforeUpdate', 'beforeDelete']); + expect(undelivered.every((e) => e.phase === 'before')).toBe(true); + }); +}); + +describe('bulk-write per-row hook budget (D6)', () => { + it('carries the ceiling objectql enforces today', () => { + // Receipt: `ObjectQL.MAX_BULK_PER_ROW_HOOK_ROWS = 10_000` + // (`packages/objectql/src/engine.ts`, #5038). Two definitions until the + // engine half reads this one; this number is what keeps them agreeing + // meanwhile, so changing it here without changing it there is the drift + // to catch. + expect(MAX_BULK_PER_ROW_HOOK_ROWS).toBe(10_000); + }); + + it('admits a batch AT the ceiling and refuses one over it', () => { + const at = resolveBulkPerRowHookBudget({ + object: 'task', event: 'beforeUpdate', matched: MAX_BULK_PER_ROW_HOOK_ROWS, + }); + expect(at.kind).toBe('ok'); + + const over = resolveBulkPerRowHookBudget({ + object: 'task', event: 'beforeUpdate', matched: MAX_BULK_PER_ROW_HOOK_ROWS + 1, + }); + expect(over.kind).toBe('refused'); + }); + + it('admits an empty match set — zero rows is zero dispatches, not a refusal (D1)', () => { + expect(resolveBulkPerRowHookBudget({ object: 'task', event: 'beforeUpdate', matched: 0 }).kind) + .toBe('ok'); + }); + + it('applies the SAME ceiling to both phases (D6)', () => { + const matched = MAX_BULK_PER_ROW_HOOK_ROWS + 1; + for (const event of BULK_WRITE_HOOK_DISPATCH_CONTRACT.map((e) => e.event)) { + const verdict = resolveBulkPerRowHookBudget({ object: 'task', event, matched }); + expect(verdict.kind).toBe('refused'); + if (verdict.kind !== 'refused') throw new Error('unreachable'); + expect(verdict.limit).toBe(MAX_BULK_PER_ROW_HOOK_ROWS); + } + }); + + it('refuses TOTALLY, and says so — never a downgrade to one dispatch', () => { + const verdict = resolveBulkPerRowHookBudget({ + object: 'task', event: 'beforeUpdate', matched: 12_345, + }); + if (verdict.kind !== 'refused') throw new Error('expected a refusal'); + expect(verdict.code).toBe(BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE); + expect(verdict.object).toBe('task'); + expect(verdict.event).toBe('beforeUpdate'); + expect(verdict.matched).toBe(12_345); + // The three facts an operator needs, pinned because a message that + // stops saying "nothing was written" turns a refusal into a mystery. + expect(verdict.message).toContain('Nothing was written'); + expect(verdict.message).toContain('NOT silently downgraded'); + expect(verdict.message).toContain('12345 rows'); + expect(verdict.message).toContain('10000-row ceiling'); + // Both routes out, named. + expect(verdict.message).toContain('Narrow the predicate'); + expect(verdict.message).toContain('remove the'); + }); + + it('is pure — same input, same verdict, no clock', () => { + const args = { object: 'task', event: 'afterUpdate', matched: 20_000 } as const; + expect(resolveBulkPerRowHookBudget(args)).toEqual(resolveBulkPerRowHookBudget(args)); + }); + + it('never mints an ADR-0112 wire code', () => { + // The refusal travels on the thrown error's own property bag. Naming it + // a `StandardErrorCode` member would put an unregistered code on the + // REST envelope by side effect — the failure the closed vocabulary + // exists to prevent. + expect(BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE).toMatch(/^ERR_/); + }); +}); diff --git a/packages/spec/src/data/bulk-write-hook-conformance.ts b/packages/spec/src/data/bulk-write-hook-conformance.ts new file mode 100644 index 0000000000..3fbddadd2d --- /dev/null +++ b/packages/spec/src/data/bulk-write-hook-conformance.ts @@ -0,0 +1,320 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The **bulk-write hook dispatch contract** — what a predicate (`multi: true`) + * update/delete hands a lifecycle hook, in BOTH phases, stated once so the + * engine, the record-change trigger and every guard hook are held to the same + * yardstick. + * + * ADR-0058's bulk-write addendum owns this decision. Addendum I (#4800 / + * #4862 / #5037 / #5038) settled the `after*` half and #5038 delivered it. + * Addendum II (#5574's maintainer ruling B, 2026-08-06, landed by #6462) + * extends the same per-row semantics to the `before*` half, and this module is + * that ruling's contract face — the spec half of a deliberate contract-first + * split, with the engine half tracked as #5574's engine card. + * + * # Why a module and not one more paragraph of TSDoc + * + * `HookContextSchema.input` in `hook.zod.ts` carries the per-event shape table, + * and it describes **what the engine builds today** — pinned against a real + * dispatch by `packages/objectql/src/hook-input-shape-contract.test.ts`, which + * is where the #5273 drift (three keys the table named and no producer set) was + * finally made discoverable. That table must therefore never run ahead of the + * engine. + * + * A contract-first ruling needs the opposite: a place to state the target + * before the producer agrees, loudly enough that "not delivered yet" cannot be + * mistaken for "delivered". So the two live apart and say different things. + * {@link BULK_WRITE_HOOK_DISPATCH_CONTRACT} carries a `delivered` flag per + * event, and `bulk-write-hook-conformance.test.ts` pins exactly which entries + * are still false — so the engine half cannot land without turning this file's + * pin red and flipping the flag in the same PR. + * + * # The decision, in full + * + * **D1 — dispatch.** On a predicate write, a `before*` event dispatches ONCE + * PER MATCHED ROW, replacing the single batch dispatch, exactly as `after*` + * has since #5038. Rows outer, hooks inner (batch INSERT's order, #2922), so a + * handler observes one whole record at a time. Zero matched rows is zero + * record changes and therefore zero dispatches — `[]` is meaningful and + * distinct from "no row set was read". + * + * **D2 — per-row context shape.** `input.id` names that row; `previous` is + * that row's pre-image; `input.options` is the CALLER's bag (`where` and + * `multi` visible — the PHASE rule in `hook.zod.ts` is unchanged, and the + * `before*` phase still reads the pre-merge view). `result` stays absent: + * the before phase has no post-state, and a value assigned to `ctx.result` + * there is overwritten by the write's own result before any `after*` handler + * sees it. The `after*` per-row `result` (`row` composed with the payload) is + * #5038's and untouched. + * + * **D3 — the payload is BATCH-scoped, and that is the merge rule.** There is + * exactly ONE payload for a predicate update — `driver.updateMany` takes one + * SET clause for N rows — and every per-row `beforeUpdate` context carries THAT + * payload, not a per-row copy. So: + * + * - a rewrite takes effect on the WHOLE batch, whichever row's dispatch made + * it, and rewrites accumulate across the N dispatches in dispatch order; + * - N post-hook payloads cannot diverge, so there is no reconciliation step, + * no payload is discarded, and no predicate write is ever split into N + * single-row writes. One `updateMany`, one affected count (#4639), one + * aggregate `data.records.updated` — the write's own contract is untouched, + * exactly as #5038 left it; + * - a rewrite CONDITIONED on the row (`ctx.previous`, `ctx.input.id`) is + * therefore **outside this 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, not so a rewrite can be aimed + * at one row. The three supported routes for row-specific work are: throw + * (which is what the guard case wants), write through `ctx.api` per row, or + * have the CALLER paginate the batch into by-id updates. + * + * On a predicate DELETE this clause is vacuous — a delete context carries an + * id and no payload — which is why `payloadScope` is `'none'` there. + * + * **D4 — `input.id` is not a reroute lever on this path.** On the batch + * dispatch it was: `input.id` was present-but-`undefined`, and binding it moved + * the write onto the single-id branch. A per-row context arrives with `id` + * ALREADY bound to its row, and the dispatch decision was made before the row + * set was read — so rebinding it retargets nothing. A silent no-op is the + * failure this family exists to abolish, so the engine half refuses the + * rebinding rather than ignoring it. This is the ONE respect in which a per-row + * `before*` context is not interchangeable with a single-id one; everything + * else in D5 is. + * + * **D5 — equivalence with the single-id path.** A `before*` handler that reads + * `previous` / `input.id` / `input.data` and either throws or applies a + * ROW-INVARIANT rewrite sees the same context and produces the same outcome + * whether the same N rows arrive as N by-id writes or one predicate write. + * The declared, deliberate differences are exactly four: `input.options` + * carries `multi` and `where`; the write resolves an affected COUNT and names + * no row (#4639); one aggregate realtime event replaces N per-record ones; and + * D4. + * + * **D6 — the budget is one ceiling for both phases.** + * {@link MAX_BULK_PER_ROW_HOOK_ROWS} governs `before*` and `after*` alike, and + * over it the write is REFUSED — before any per-row dispatch and before the + * driver call, so nothing is written and no handler ran for a batch that was + * going to be refused anyway. Never a downgrade to one dispatch for the batch: + * that would skip the hook for N-1 rows and say nothing, which is the shape + * #4649 / #4775 / #5038 all exist to abolish. {@link resolveBulkPerRowHookBudget} + * is that rule, executable. + * + * **D7 — one read, reused.** The matched row set is read ONCE per predicate + * write, with the middleware-composed AST the write itself binds (#2982), and + * serves per-row validation (#3106), the `readonlyWhen` strip (#3042), the + * per-row `before*` dispatch (D1) and the per-row `after*` dispatch (#5038). + * A second fetch is refused by the ruling in as many words. One corollary the + * engine half must honour rather than rediscover: because the engine binds + * `previous` BEFORE dispatching, objectql's `sys_fetch_previous_update` + * builtin (`plugin.ts`, `object: '*'`, priority 5) short-circuits on its own + * `!ctx.previous` guard and issues no `findOne` — which is what keeps "no + * second fetch" true in the one deployment shape where it would otherwise be + * false N times over. + * + * # What this contract deliberately does NOT do, and why + * + * The obvious alternative was per-row payload COPIES plus a reconciliation + * rule — converge on one payload when the N copies agree, refuse (or split the + * write) when they do not. It was rejected on measured evidence, and the + * evidence is worth keeping because it is not recoverable by reading the + * design: + * + * - **A value-based reconciliation is defeated by the clock.** objectql's own + * `sys_stamp_audit_update` builtin is registered on `'*'`, so it runs in + * essentially every deployment, and it computes `new Date().toISOString()` + * INSIDE the per-record stamp (`applyToRecord` in `plugin.ts`). 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. A + * converge-or-refuse rule would then refuse honest batches + * non-deterministically, and a converge-or-split rule would shatter one + * `updateMany` into N single-row writes for the same reason. Both fail in + * the direction nobody can debug. + * - **Nothing measured needs divergent payloads.** Every `beforeUpdate` + * payload rewrite in this repo is row-invariant: the audit stamp + * (session + clock), plugin-pinyin-search's companion projection (derived + * from the payload's own text), service-storage's copy-on-claim (derived + * from the payload plus the referenced `sys_file` rows — and it already + * declines to claim on a bulk write for want of a definite owner). Building + * a divergence mechanism for zero measured producers is capability + * expansion without business pull. + * - **It stays reversible in the safe direction.** Refusing divergence today + * can be relaxed by a later ADR if the pull ever appears; a write that has + * already learned to split itself cannot be un-split without breaking + * whoever came to depend on it. + * + * The residual hazard is named rather than hidden: D3 hands authors per-row + * `previous` and a batch-scoped payload, so a row-conditional rewrite is + * *expressible* and wrong. That is a contract statement, not an enforcement — + * no static rule can decide whether a rewrite is row-invariant — so it belongs + * in the authoring docs and, if it ever earns one, an advisory lint over hook + * bodies (`packages/lint`'s `validate-hook-body-writes` is the existing seam). + * Naming an unenforceable clause is the honest half of ADR-0049, not a breach + * of it: the alternative was to leave the same hazard undocumented. + * + * @see docs/adr/0058-expression-and-predicate-surface.md — Addendum II + * @see HookContextSchema in `data/hook.zod.ts` — the per-event shape the engine + * builds TODAY, pinned in objectql + */ + +/** A lifecycle phase, as the bulk-write contract distinguishes them. */ +export type BulkWriteHookPhase = 'before' | 'after'; + +/** Whether a per-row context carries a mutable write payload, and whose it is. */ +export type BulkWritePayloadScope = + /** One payload for the whole batch; a rewrite applies to every matched row (D3). */ + | 'batch' + /** No payload on this event at all (delete carries an id and options only). */ + | 'none'; + +/** + * One row of the dispatch contract: what a predicate write hands this event. + */ +export interface BulkWriteHookDispatchContractEntry { + /** The lifecycle event this row is about. */ + readonly event: 'beforeUpdate' | 'beforeDelete' | 'afterUpdate' | 'afterDelete'; + /** Which phase it belongs to. */ + readonly phase: BulkWriteHookPhase; + /** + * The keys the PER-ROW context carries, in the order the shape table in + * `hook.zod.ts` lists them. `previous` is that row's pre-image on every + * entry — that is the whole point of the contract. + */ + readonly contextKeys: readonly ('id' | 'data' | 'options' | 'previous' | 'result')[]; + /** {@link BulkWritePayloadScope} — the D3 answer for this event. */ + readonly payloadScope: BulkWritePayloadScope; + /** + * Has the ENGINE delivered per-row dispatch for this event? + * + * `false` is not a gap to route around — it is a contract-first split + * recorded in the open: the ruling required the spec half to land first. + * A consumer that must branch on live behaviour reads the engine, never + * this flag; the flag exists so the flip is a visible, pinned edit. + */ + readonly delivered: boolean; + /** Issue that DELIVERED it (when `delivered`), else the one that will. */ + readonly engineDeliveryIssue: number; +} + +/** + * The four write events a predicate (`multi: true`) write dispatches per + * matched row, with what each per-row context carries. + * + * Read as a table, `after*` and `before*` side by side, because "same + * yardstick as the after side" is the ruling's own test of this contract. + */ +export const BULK_WRITE_HOOK_DISPATCH_CONTRACT: readonly BulkWriteHookDispatchContractEntry[] = [ + { + event: 'beforeUpdate', + phase: 'before', + contextKeys: ['id', 'data', 'options', 'previous'], + payloadScope: 'batch', + delivered: false, + engineDeliveryIssue: 5574, + }, + { + event: 'beforeDelete', + phase: 'before', + contextKeys: ['id', 'options', 'previous'], + payloadScope: 'none', + delivered: false, + engineDeliveryIssue: 5574, + }, + { + event: 'afterUpdate', + phase: 'after', + contextKeys: ['id', 'data', 'options', 'previous', 'result'], + payloadScope: 'batch', + delivered: true, + engineDeliveryIssue: 5038, + }, + { + event: 'afterDelete', + phase: 'after', + contextKeys: ['id', 'options', 'previous'], + payloadScope: 'none', + delivered: true, + engineDeliveryIssue: 5038, + }, +]; + +/** + * Ceiling on the matched-row set one predicate write may fire per-row hooks + * over — D6, and the same number for both phases. + * + * The cost it bounds is the one ADR-0058's Addendum I told the implementation + * to price: a hook that used to run once per batch runs once per row, so a + * notification hook sends N messages. Unbounded, one `multi: true` update over + * a whole table becomes an unbounded fan-out of handler executions inside a + * single write. + * + * ⚠️ Two definitions today, one on purpose and only until the engine half + * lands: objectql holds the same literal as `ObjectQL.MAX_BULK_PER_ROW_HOOK_ROWS` + * (`engine.ts`, #5038) and open-codes the refusal. The engine half of #5574 + * replaces that static and that message with this module, at which point the + * ceiling has one definition again. Until then they agree because + * `bulk-write-hook-conformance.test.ts` pins the number, not because anything + * structural makes them. + */ +export const MAX_BULK_PER_ROW_HOOK_ROWS = 10_000; + +/** + * The code carried by a budget refusal. + * + * Deliberately an `ERR_`-prefixed operational code on the thrown error's own + * property bag, NOT an ADR-0112 wire code: `StandardErrorCode` and + * `ERROR_CODE_LEDGER` are a closed vocabulary that the REST layer promotes onto + * the response envelope, and minting a member of it by side effect is the exact + * `declared != enforced` shape that vocabulary exists to prevent. Same reasoning + * that keeps `HookConditionLimitation` off `error.code` in objectql. + */ +export const BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE = 'ERR_BULK_PER_ROW_HOOK_LIMIT'; + +/** A budget verdict: proceed, or refuse the whole write. */ +export type BulkPerRowHookBudgetVerdict = + | { readonly kind: 'ok'; readonly matched: number; readonly limit: number } + | { + readonly kind: 'refused'; + readonly code: typeof BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE; + readonly object: string; + readonly event: string; + readonly matched: number; + readonly limit: number; + readonly message: string; + }; + +/** + * D6, executable: may a predicate write on `object` dispatch `event` once per + * each of `matched` rows? + * + * A refusal is total — the write does not proceed in any reduced form. The + * message names the count, the ceiling, both routes out, and the one thing a + * reader most needs to be told: that nothing was written and that the write was + * not quietly downgraded to a single dispatch for the batch. + * + * Pure and total: no clock, no I/O, no throw. The engine raises; the contract + * decides. + */ +export function resolveBulkPerRowHookBudget(input: { + readonly object: string; + readonly event: string; + readonly matched: number; +}): BulkPerRowHookBudgetVerdict { + const { object, event, matched } = input; + const limit = MAX_BULK_PER_ROW_HOOK_ROWS; + if (matched <= limit) return { kind: 'ok', matched, limit }; + return { + kind: 'refused', + code: BULK_PER_ROW_HOOK_LIMIT_ERROR_CODE, + object, + event, + matched, + limit, + message: + `Refusing the bulk write on '${object}': it matches ${matched} rows, and '${event}' hooks are ` + + `contracted to fire PER ROW on a predicate write (ADR-0058, bulk-write addendum), which is ` + + `over the ${limit}-row ceiling for one write. Nothing was written. ` + + `Narrow the predicate so the batch matches fewer rows (paginate the write), or remove the ` + + `'${event}' hook from this object. The write is NOT silently downgraded to one hook call for ` + + `the batch — that would skip the hook for ${matched - 1} rows without saying so.`, + }; +} diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index e81a13c77c..4a0f2e4c04 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -362,6 +362,20 @@ export const HookContextSchema = lazySchema(() => z.object({ * there — binding it is precisely the test the engine dispatches on, so a * `before*` handler that sets it REROUTES the write onto the single-id path. * + * ⚠️ CONTRACTED TO CHANGE — the two `before` rows above and the paragraph + * just above them describe the engine as it is TODAY, which is what this + * table is for. #5574's maintainer ruling (2026-08-06, option B) extends the + * per-row bulk-write contract from the `after*` phase to the `before*` phase: + * a predicate write will dispatch `beforeUpdate`/`beforeDelete` once per + * matched row, each carrying that row's `previous` and `id`, over a payload + * that stays BATCH-scoped. The contract is stated — with its budget ceiling + * and its `delivered` flags — in `data/bulk-write-hook-conformance.ts` + * (ADR-0058 Addendum II); the engine half is #5574's engine card, and it + * moves these rows in the same PR that makes them false. Until then, a + * `before*` handler on a bulk write has NO `previous`: a guard written as + * `previous?.x` passes silently on every batch, which is the measured harm + * the ruling is about. + * * The row-scoping predicate a bulk write EXECUTES is not reachable from * `input` at all. It is the composed `ast`, which lives on the * engine-internal `OperationContext.ast` (#2982) so that the filters diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index ddbbeedc2b..eb52b38fcd 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -50,6 +50,14 @@ export * from './autonumber-format'; export * from './validation.zod'; export * from './hook.zod'; export * from './hook-body.zod'; +// The bulk-write hook dispatch contract (ADR-0058 Addendum II) — what a +// predicate (`multi: true`) write hands a lifecycle hook in BOTH phases: per-row +// dispatch, per-row `previous`, a batch-scoped payload, and one budget ceiling +// for before and after alike. Stated apart from `hook.zod.ts`'s shape table on +// purpose: that table describes the engine as BUILT (pinned in objectql), this +// one states the contract the engine is being brought to, with a `delivered` +// flag per event so "not yet" can never read as "yes". +export * from './bulk-write-hook-conformance'; export * from './mapping.zod'; export * from './data-engine.zod'; export * from './driver.zod'; diff --git a/scripts/adr-anchors.json b/scripts/adr-anchors.json index 5858cce456..30c12a1ac2 100644 --- a/scripts/adr-anchors.json +++ b/scripts/adr-anchors.json @@ -24,6 +24,13 @@ ], "invariant": "An unevaluable hook `condition` ABORTS the operation (ADR-0058's write-path addendum, #4775) — `onError` never sees it and cannot soften it back into a silent skip. On a predicate (`multi: true`) bulk write the rejection must name the LIMITATION, not the author. Since #5038 that rejection is scoped to the BATCH dispatch — the `before*` phase, which fires once for N rows because it may still rewrite the shared payload; after-hooks are dispatched per row with `previous` bound (ADR-0058's bulk-write addendum), so they never reach it. The message must therefore name the phase as the reason and point at the matching `after*` event, and must NOT promise an expiry that already happened. The discriminator is `limitation`, deliberately NOT `code` — ADR-0112 makes `error.code` a closed wire vocabulary and the REST layer promotes a thrown error's `.code` onto the envelope, so naming it `code` would mint an unregistered wire code by side effect." }, + { + "file": "packages/spec/src/data/bulk-write-hook-conformance.ts", + "adrs": [ + "ADR-0058" + ], + "invariant": "A predicate (`multi: true`) write dispatches lifecycle hooks PER MATCHED ROW in BOTH phases (ADR-0058 Addendum I for `after*`, Addendum II for `before*`), each context carrying that row's `previous` — the fix for a guard hook reading `previous?.x` and passing silently on every batch. The write's own payload stays BATCH-scoped: one `updateMany` carries one payload, so a `before*` rewrite applies to every matched row, N post-hook payloads cannot diverge, nothing is reconciled and no predicate write is ever split into N single-row writes (one affected count, #4639). Per-row `previous` exists so a guard can REFUSE, not so a rewrite can be aimed at one row. One ceiling (`MAX_BULK_PER_ROW_HOOK_ROWS`) governs both phases and is checked BEFORE the first dispatch; exceeding it REFUSES the write and is never downgraded to one dispatch for the batch. Per-row payload copies plus a converge-or-refuse rule were rejected on measured evidence: objectql's `'*'`-registered audit stamp reads the clock inside the per-record stamp, so rows either side of a millisecond boundary would diverge and refuse honest batches non-deterministically." + }, { "file": "packages/spec/src/identity/membership-role.ts", "adrs": [