diff --git a/.changeset/hook-previous-row-invariant-rewrite.md b/.changeset/hook-previous-row-invariant-rewrite.md new file mode 100644 index 0000000000..8b354b95ab --- /dev/null +++ b/.changeset/hook-previous-row-invariant-rewrite.md @@ -0,0 +1,16 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): `HookContext` admits a row-invariant-in-effect rewrite by per-row `previous` on a predicate write, kept safe by the engine's key-divergence refusal (#16074) + +The `hook.zod.ts` contract said that on a predicate (`multi: true`) write the per-row `previous` is supplied *so a guard can REFUSE (throw), not so a rewrite can be aimed*. Three shipped `beforeUpdate` provenance stamps (`sys_email_template`, `sys_sharing_rule`, `sys_webhook`) read `ctx.previous` per row and write `customized: true` conditioned on it — inside the letter of what the engine allows, outside the stated purpose of the input they use. Maintainer ruling (recorded by the director seat, decision batch #59, 2026-09-06), option 1: **the contract admits the shape.** + +The amended D3 clause (`HookContextSchema.input` TSDoc, mirrored in `bulk-write-hook-conformance.ts`) now states: + +- Per-row `previous` is supplied so a guard can REFUSE, **and** so a `before*` hook can make a **row-invariant-in-effect rewrite** — one whose written KEY SET is the same on every matched row **and is assigned in place** (`ctx.input.data.customized = true`, not a wholesale replacement of `ctx.input.data`). +- What makes that shape safe is the engine's `MULTI_UPDATE_HOOK_KEY_DIVERGENCE` refusal (#14099): the dispatch records, per row, the payload keys that row's hook chain assigned **in place**, and if any two rows disagree the whole batch is refused **before any write**. In place is the condition the refusal rests on: a hook that REPLACES `ctx.input.data` leaves the dispatch unable to attribute keys, so the comparison is skipped and the batch is not judged at all. +- What an operator sees when it fires: an ADR-0112 envelope with `status: 400`, `code: 'MULTI_UPDATE_HOOK_KEY_DIVERGENCE'`, `keys` (the sorted keys some rows' hooks wrote and others did not, e.g. `['customized']`), `rows` (how many rows the predicate matched), `object`, and a message that says "Nothing was written" before naming the remedy. A bulk edit over rows that already disagree on the stamp's condition is refused whole rather than half-stamped; that is the engine working, not the hooks misbehaving, and the remedy is the caller's — write those rows by id, or from inside the handler through `ctx.api`. +- Three shapes the rule does **not** admit: a rewrite whose written key set differs across rows (that is the refusal itself); the same key written with a per-row VALUE — the engine judges key sets, never values, so that shape clears the check and applies the last dispatch's value to every row; and a row-conditioned REPLACEMENT of `ctx.input.data`, which silences the recording above so that shape is judged by nothing at all. All three stay out of contract. + +Purely additive at the contract: no schema key, type or accept set of `HookContextSchema` itself changes, and the engine's behaviour is unchanged — the three stamps become conforming by amendment, and the rule for the next hook author is written down where the contract lives. Option 2 (change the hooks to stop aiming by `previous`) was not adopted: #15302 measured that declining on a predicate write leaves unstamped exactly the rows the next boot overwrites, turning a visible 400 into silent loss of an admin edit. diff --git a/docs/adr/0058-expression-and-predicate-surface.md b/docs/adr/0058-expression-and-predicate-surface.md index 7b614b6973..c8e73126bf 100644 --- a/docs/adr/0058-expression-and-predicate-surface.md +++ b/docs/adr/0058-expression-and-predicate-surface.md @@ -233,7 +233,7 @@ > 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. +> REFUSE, not so a rewrite can be aimed. **→ Amended in Amendment II.3 below.** > - **`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 @@ -439,6 +439,73 @@ --- +> **Amendment II.3 (2026-09, #16074 maintainer ruling, decision batch #59) — +> a ROW-INVARIANT-IN-EFFECT rewrite is ADMITTED, and the ENGINE is what makes +> it safe.** +> _Amends D3's closing sentences in Addendum II above by ADDITION: the 2026-08 +> text is left standing as the dated record of what was decided then, carrying +> a forward pointer to here. This block RECORDS a decision already taken; it +> takes none._ +> +> **The ruling.** Maintainer reply, verbatim: 「16063 c, 其他同意」 +> (2026-09-06, decision batch #59, recorded on this card #16074 in comment +> `5560086928`). Option 1 adopted: the CONTRACT admits the shape. The three +> shipped provenance stamps are made conforming by amending the contract, not +> by being changed to fit it. +> +> **What the rule now is.** On a predicate write (`multi: true`), per-row +> `previous` is supplied so a guard can REFUSE (throw) **and** so a `before*` +> hook can make a **row-invariant-in-effect** rewrite: one whose written KEY +> SET is the same on every matched row AND is assigned IN PLACE +> (`ctx.input.data.customized = true`). The shape that ships is the worked +> example — a provenance stamp writing `customized: true` on every row whose +> `previous.managed_by` is package-seeded. D3's merge rule is untouched: the +> payload stays BATCH-scoped, so what "row-invariant in effect" buys is the +> right to DECIDE per row while writing the same keys for all of them. +> +> **The mechanism that makes it safe is the ENGINE, not the hook.** The +> dispatch's `MULTI_UPDATE_HOOK_KEY_DIVERGENCE` refusal (#14099) records, per +> row, the payload keys that row's hook chain assigned IN PLACE, and if any two +> rows disagree it refuses the WHOLE batch BEFORE any write — nothing is +> written, not the first row. In-place is the condition the refusal rests on, +> which is why the admitted shape carries it: a hook that REPLACES +> `ctx.input.data` hands the dispatch a fresh object whose keys it cannot +> attribute, the recording yields nothing, and the comparison is SKIPPED — the +> batch is not judged at all. To an operator that refusal is an ADR-0112 +> envelope, `status: 400`, carrying `keys` (the sorted keys some rows' hooks +> wrote and other rows' did not) and `rows` (how many rows the predicate +> matched). So an author does not have to be TRUSTED to be row-invariant; a +> hook that is not gets a loud, whole-batch 400 instead of a half-stamped +> table. +> +> **The three shapes the rule does NOT admit.** +> +> - A rewrite whose written KEY SET differs across rows. That IS the refusal +> above: out of contract, and the engine says so before anything is written. +> - The same key written with a per-row VALUE. The engine judges KEY SETS, +> never values — the clock-reading audit stamp has to pass — so this shape +> CLEARS the divergence check and applies the LAST dispatch's value to every +> matched row. It stays out of contract and stays unenforced. +> - A row-conditioned REPLACEMENT of `ctx.input.data`. The recording cannot +> attribute a replacement's keys, so the batch ABSTAINS and no refusal fires +> at all. It stays out of contract and stays unenforced, for the same reason +> as the shape above: what admits the new shape is the refusal, and the +> refusal does not reach here. +> +> **Why option 2 (change the three stamps) was not adopted.** #15302 measured +> the cost of the alternative: a stamp that DECLINES on a predicate write +> leaves unstamped exactly the rows the next boot overwrites, so it converts a +> visible 400 into the silent loss of an admin edit. +> +> **Where the contract text lives now.** The D3 bullet of +> `packages/spec/src/data/hook.zod.ts#HookContextSchema`'s `input` clause set, +> mirrored in `packages/spec/src/data/bulk-write-hook-conformance.ts` — both +> amended in PR #17249. The refusal's own class, and the value-comparison +> variants that were rejected with it, are +> `packages/objectql/src/multi-update-hook-key-divergence.ts`. + +--- + ## 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/src/data/bulk-write-hook-conformance.ts b/packages/spec/src/data/bulk-write-hook-conformance.ts index 407f4d6c03..5d9a136a97 100644 --- a/packages/spec/src/data/bulk-write-hook-conformance.ts +++ b/packages/spec/src/data/bulk-write-hook-conformance.ts @@ -66,13 +66,26 @@ * 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. + * - a rewrite CONDITIONED on the row (`ctx.previous`, `ctx.input.id`) + * therefore cannot 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, and — ruled on #16074 — so a hook can make a + * ROW-INVARIANT-IN-EFFECT rewrite: one whose written KEY SET is the same on + * every matched row AND is assigned IN PLACE. The engine's + * `MULTI_UPDATE_HOOK_KEY_DIVERGENCE` refusal (#14099; `400`, naming the + * diverging `keys` and the matched `rows`) is what makes that shape safe — + * when two rows' hook chains assign different key sets in place the WHOLE + * batch is refused before any write. In-place is the condition the refusal + * rests on, not a detail of spelling: a hook that REPLACES `ctx.input.data` + * rather than assigning into it leaves the dispatch unable to attribute + * keys, so the recording yields nothing and the batch is NOT JUDGED at all. + * A rewrite aimed at ONE row — a per-row key set; the same key with a + * per-row value, which the key-set test cannot see; or a row-conditioned + * REPLACEMENT of the payload, which the recording cannot see — is still + * **outside this contract**; `hook.zod.ts` carries the operator-facing + * shape of the refusal. 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. @@ -150,12 +163,17 @@ * * 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. + * *expressible*. No static rule can decide whether a rewrite is row-invariant, + * but since #14099 the dispatch MEASURES the half that matters — the key set + * each row's chain writes — and refuses divergence, which is what lets #16074 + * admit the row-invariant-in-effect shape above. That measurement has two + * blind spots, and both stay unenforced: the same key with per-row VALUES, and + * a payload REPLACED wholesale rather than assigned into, whose keys the + * dispatch cannot attribute at all. Both belong in the authoring docs and, if + * they ever earn 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 diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index c92bd21baa..e7af547091 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -554,10 +554,45 @@ export const HookContextSchema = lazySchema(() => z.object({ * context carries THE one payload, not a copy — `driver.updateMany` takes * one SET clause for N rows — so a rewrite applies to the whole batch * whichever row's dispatch made it, and rewrites accumulate in dispatch - * order. A rewrite CONDITIONED on the row is therefore out of contract: - * it widens to every matched row instead of scoping itself. Per-row - * `previous` is supplied so a guard can REFUSE (throw), not so a rewrite - * can be aimed. + * order. A rewrite CONDITIONED on the row therefore cannot scope itself: + * whatever one row's dispatch writes is written to every matched row. + * Per-row `previous` is supplied so a guard can REFUSE (throw), and — + * ruled on #16074 — so a `before*` hook can make a + * ROW-INVARIANT-IN-EFFECT rewrite: one whose written KEY SET is the same + * on every matched row AND is assigned IN PLACE + * (`ctx.input.data.customized = true`), such as a provenance stamp that + * writes `customized: true` on every row whose `previous.managed_by` is + * package-seeded. What makes that shape safe is not the hook but the + * engine's `MULTI_UPDATE_HOOK_KEY_DIVERGENCE` refusal (#14099): the + * dispatch records, per row, the payload keys that row's hook chain + * assigned IN PLACE, and if any two rows disagree the WHOLE batch is + * refused before any write — nothing is written, not the first row. + * In-place is the load-bearing half of that condition, not a detail of + * spelling: a hook that REPLACES `ctx.input.data` (assigning + * `ctx.input.data = { ...ctx.input.data, customized: true }`) hands the + * dispatch a fresh object whose keys it cannot attribute, so the + * recording yields nothing and the comparison is SKIPPED — the batch is + * not judged at all, rather than judged and passed. To an operator that + * refusal is an ADR-0112 envelope with `status: 400` and + * `code: 'MULTI_UPDATE_HOOK_KEY_DIVERGENCE'`, carrying `keys` — the sorted + * keys some rows' hooks wrote and other rows' did not (for the stamp above, + * `['customized']`) — and `rows` — how many rows the predicate matched + * (`2` for a two-row batch) — plus `object` naming the target and a + * message that says "Nothing was written" and then names the remedy. So a + * bulk edit over rows that ALREADY disagree on the stamp's condition (one + * row still package-managed, one already customized) is refused whole + * rather than half-stamped; that is the engine working, not the hooks + * misbehaving, and the remedy is the caller's: write those rows by id, or + * from inside the handler through `ctx.api`. Three shapes this rule does + * NOT admit: a rewrite whose written key set differs across rows (that IS + * the refusal above); a rewrite that writes the same key with a per-row + * VALUE — the engine judges key sets, never values (the clock-reading + * audit stamp must pass), so that shape clears the check and applies the + * LAST dispatch's value to every row; and a row-conditioned REPLACEMENT + * of `ctx.input.data`, which silences the recording described above, so + * that shape is judged by nothing at all. All three stay out of contract. + * The refusal's class and both rejected value-comparison variants are + * recorded on `packages/objectql/src/multi-update-hook-key-divergence.ts`. * - `input.id` is NOT a reroute lever (D4). It used to be: on the batch * dispatch `input.id` was present-but-`undefined`, and binding it moved * the write onto the single-id path. A per-row context arrives with `id` diff --git a/scripts/adr-anchors/packages__spec__src__data__bulk-write-hook-conformance.ts.json b/scripts/adr-anchors/packages__spec__src__data__bulk-write-hook-conformance.ts.json index 8855b05079..f3485a3d22 100644 --- a/scripts/adr-anchors/packages__spec__src__data__bulk-write-hook-conformance.ts.json +++ b/scripts/adr-anchors/packages__spec__src__data__bulk-write-hook-conformance.ts.json @@ -3,5 +3,5 @@ "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." + "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 and so a hook can make a row-invariant-in-effect rewrite (the same written key set on every row, assigned IN PLACE) — never so a rewrite can be aimed at one row: the engine's `MULTI_UPDATE_HOOK_KEY_DIVERGENCE` refusal (400, `keys`, `rows`) rejects the whole batch when two rows' hook chains write different key sets in place, and ABSTAINS — judging the batch not at all — when a hook REPLACES `ctx.input.data` instead of assigning into it. 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." }