Skip to content

Commit ea1d916

Browse files
os-zhuangclaude
andauthored
feat(objectql): dispatch before* hooks per matched row on a predicate bulk write (#5574, #5846) (#6697)
* feat(objectql): dispatch before* hooks per matched row on a predicate bulk write (#5574, #5846) A `multi: true` update or delete now dispatches `beforeUpdate`/`beforeDelete` once per matched row, on a single-record-shaped context carrying that row's `id` and `previous` — the move #5038 made for the `after*` phase, held to the same yardstick. ADR-0058 Addendum II (ruling B, 2026-08-06) is the contract; `packages/spec/src/data/bulk-write-hook-conformance.ts` states it as D1-D7 and its `delivered` flags flip here. The harm: `ctx.previous` was never bound in the before phase of a predicate write, so every guard written as `if (ctx.previous?.locked) throw` passed silently on every batch — fail-open, and invisible. - D1/D2: one dispatch per matched row; `previous` is that row's pre-image, `result` stays absent, `input.options` is still the caller's bag. Zero matched rows is zero dispatches. - D3: the payload stays BATCH-scoped. Per-row contexts share THE payload object, rewrites apply to the whole batch and accumulate in dispatch order (including a REPLACED `input.data`). One updateMany, one affected count. - D4 + ADR-0058 Amendment II.1: `input.id` stops being a reroute lever, on the by-id path too. Clearing it converted a by-id write into a predicate write; rebinding moved the write to a row whose pre-image, readonlyWhen locks and validation rules were never evaluated. Both now reject with `HookTargetRebindError` (`ERR_HOOK_TARGET_REBIND`), naming the retired capability and its three replacements. - D6: one ceiling for both phases, checked before the FIRST dispatch. The engine's open-coded ceiling and message are replaced by the spec module's `resolveBulkPerRowHookBudget`. - D7: the matched row set is read ONCE and serves validation (#3106), the readonlyWhen strip (#3042) and both per-row dispatches. #5846 (a): `update()` reads its prior row BEFORE dispatching `beforeUpdate` and binds `previous` there, matching `delete()` since #5272, so both phases share one read. `sys_fetch_previous_update` is retired — its `!ctx.previous` guard is now permanently false. ADR-0049: `HookConditionLimitation` (both members), `isPredicateBulkWrite` and `predicateBulkWrite` are retired — a batch-scoped `before*` dispatch no longer exists, leaving them with neither producer nor reachable consumer. A `previous`-reading `before*` condition on a bulk write now evaluates as authored, per row. fix(plugin-auth): the last-administrator break-glass guard resolved its target set as "a scalar `input.id` if there is one, else the predicate", which was sound only while a predicate write's `before*` left `input.id` undefined. Under per-row dispatch a `multi` ban of every administrator arrived as N individually legitimate by-id bans and locked the environment out. `resolveTargetIds` now asks `options.multi` first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ZgeUyxzRnXzNCq8vizVoQ * chore(objectql,scripts): satisfy the erasure ratchet and the ADR-0112 anchor (#5574) - The new bulk-write before-phase cases passed their engine options through `as any`. They are ordinary in-contract calls (`EngineUpdateOptions` / `EngineDeleteOptions` both declare `multi` and `where`), so they are typed instead: the test surface returns to its 263 ceiling rather than being raised for tests that never needed the erasure. - The `update()` restructure removed three `any`-erased engine option sites, so `packages/objectql/src/engine.ts` ratchets 12 → 9 in the baseline. - `hook-wrappers.ts` lost its ADR-0112 reference when the batch-write branch of the condition diagnosis was retired. The rule it anchored did NOT go with the branch — no discriminator on `HookConditionError` is ever `error.code`, for every field the class still carries — so it is restated where it now applies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ZgeUyxzRnXzNCq8vizVoQ * docs(objectql): record the measured #5929 position of sys_fetch_previous_delete The retirement note next door claimed the delete-side builtin's guard was still reachable. It is not, and the reason is worth handing to #5929 rather than leaving to be rediscovered: the builtin is a 'beforeDelete' hook on '*', so it holds open the very demand gate whose read then binds 'previous' before it runs — its only remaining effect is to make itself redundant. Retiring it stays #5929's card, since that card owns the gate's per-object honesty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ZgeUyxzRnXzNCq8vizVoQ * refactor(objectql): split the delete-side by-id repoint out of #5574's settlement (#6752) Amendment II.1 retired the `input.id` reroute lever, and the first cut applied the refusal to `delete()`'s by-id REPOINT as well — folding a behaviour removal into an ordering change. Split out; #5272's re-read is restored verbatim. The two verbs now answer a rebind differently, and the asymmetry is the point rather than a leftover. The case against honouring a rebind is that the write lands on a row whose pre-image, `readonlyWhen` locks and validation rules were never evaluated — and on `delete()` that is simply not true: #5272 already RE-RESOLVES the new target, re-reading its pre-image and rebinding `previous` before `afterDelete` or the summary recompute can see it. `update()` has no such mechanism and would have to grow one, which is the "silently pick re-resolution instead" the ruling forbids. So `update()` refuses and `delete()` keeps honouring, until the repoint is ruled on as its own question. A CLEARED id stays refused on both verbs, and that is not a discretionary choice: it worked by falling through to the predicate branch, and the ladder is now resolved before any handler runs because a per-row `before*` context is built from the matched row set. That is the capability the ruling names. - `engine.ts`: delete() by-id restores the repoint re-read; refuses only a clear. - `hook-target-rebind-errors.ts`, ADR-0058 Amendment II.1 (now a scope table), the changeset: state which cell answers what, and why the row is uneven. - `bulk-write-per-row-hooks.test.ts` §7 D4: the repoint case is inverted into a pin that the REPOINTED row is the one deleted and `afterDelete` sees its pre-image, so neither direction gets "tidied up" by a later reader. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ZgeUyxzRnXzNCq8vizVoQ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 559206b commit ea1d916

25 files changed

Lines changed: 2062 additions & 960 deletions
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
feat(objectql): dispatch `before*` hooks per matched row on a predicate bulk write (#5574, #5846)
7+
8+
A `multi: true` update or delete now dispatches `beforeUpdate` / `beforeDelete`
9+
**once per matched row**, on a single-record-shaped context carrying that row's
10+
`id` and `previous` — the same move #5038 made for the `after*` phase, held to
11+
the same yardstick. ADR-0058 Addendum II (maintainer ruling B, 2026-08-06) is
12+
the contract; `packages/spec/src/data/bulk-write-hook-conformance.ts` states it
13+
as clauses D1–D7, and its `delivered` flags flip with this change.
14+
15+
**The harm this fixes.** `ctx.previous` was never bound in the before phase of a
16+
predicate write, so every guard written the way guards are written —
17+
`if (ctx.previous?.locked) throw` — passed **silently** on every batch. The
18+
failure direction is fail-OPEN and the optional chaining that makes it silent is
19+
exactly what an AI writes. One measured deployment had all 15 of its guard hooks
20+
bypassed by a single batch edit, including writing `null` into a `readonly: true`
21+
field that the single-id path refuses.
22+
23+
**Two visible behaviour changes, both loud.**
24+
25+
- **Guards now fire per row on predicate writes.** A `beforeUpdate` /
26+
`beforeDelete` hook on an object targeted by a `multi: true` write runs N times
27+
instead of once, each time with that row's `previous` bound. Zero matched rows
28+
is zero dispatches. A hook that throws refuses the whole batch before anything
29+
is written. The payload stays **batch-scoped** (D3): every per-row context
30+
carries the one payload, so a rewrite applies to every matched row whichever
31+
row's dispatch made it, rewrites accumulate in dispatch order, and no predicate
32+
write is ever split into N single-row writes — one `updateMany`, one affected
33+
count (#4639), one aggregate event. A rewrite *conditioned* on the row is
34+
therefore out of contract: it widens to the whole batch rather than scoping
35+
itself. Per-row `previous` is supplied so a guard can REFUSE, not so a rewrite
36+
can be aimed.
37+
- **The `input.id` reroute lever is retired and now refuses.** The dispatch
38+
ladder is resolved **before** the before phase — it has to be, since per-row
39+
contexts are built from the matched row set — so the id slot can no longer
40+
steer the write. Rather than ignore an assignment (a silent no-op) or honour
41+
it blindly, the write is rejected with `HookTargetRebindError`
42+
(`ERR_HOOK_TARGET_REBIND`), whose message names the retired capability and the
43+
three supported replacements. Recorded as ADR-0058 Amendment II.1. Precisely:
44+
45+
| | CLEARED id | REBOUND to another id |
46+
|---|---|---|
47+
| `update()` by-id | refused | refused |
48+
| `delete()` by-id | refused | **honoured, unchanged** (#5272's re-read) |
49+
| either, per-row | refused (D4) | refused (D4) |
50+
51+
Clearing is uniform because it worked by falling through to the predicate
52+
branch, and that branch is now chosen before any handler runs. Rebinding is
53+
not uniform, deliberately: the case against honouring it is that the write
54+
lands on a row whose pre-image and rules were never evaluated, and on
55+
`delete()` that is simply not true — #5272 already re-resolves the new target
56+
before `afterDelete` or the summary recompute sees it. `update()` has no such
57+
mechanism and building one would be the "silently pick re-resolution instead"
58+
the ruling forbids. Retiring the delete-side repoint is its own question,
59+
filed as #6752 rather than ridden in on an ordering change.
60+
61+
**Also in this change.**
62+
63+
- **One read, reused (D7).** The matched row set is read ONCE per predicate
64+
write, with the write's own composed AST, and serves per-row validation
65+
(#3106), the `readonlyWhen` strip (#3042) and both per-row dispatches.
66+
- **One ceiling, both phases (D6).** `MAX_BULK_PER_ROW_HOOK_ROWS` (10 000) now
67+
governs `before*` as well as `after*`, checked **before the first dispatch**, so
68+
an over-ceiling batch runs zero handlers and writes nothing — a refusal, never
69+
a downgrade to one dispatch. The engine's open-coded ceiling and refusal
70+
message are replaced by the spec module's `resolveBulkPerRowHookBudget`, so the
71+
number and the wording have one definition again.
72+
- **`update()` binds `previous` before the before phase (#5846 (a)).** The by-id
73+
path reads its prior row ahead of the dispatch, matching `delete()`'s shape
74+
since #5272, so both phases share one read. objectql's
75+
`sys_fetch_previous_update` builtin is **retired**: it existed to bind
76+
`previous` for the before phase behind `if (input.id && !ctx.previous)`, and
77+
that guard is now permanently false. A by-id update on a kernel used to read
78+
the same row three times; this removes one and makes the engine's read the
79+
single producer.
80+
- **`HookConditionLimitation` is retired** (ADR-0049 enforce-or-remove), with
81+
`isPredicateBulkWrite` and the `predicateBulkWrite` flag. Both members
82+
(`bulk_write_previous_unbound`, `bulk_write_stored_state_unavailable`)
83+
described a batch-scoped `before*` dispatch that no longer exists, leaving them
84+
with neither producer nor reachable consumer. A `previous`-reading `before*`
85+
condition on a bulk write now **evaluates as authored**, per row, instead of
86+
rejecting the batch. `HookConditionError` itself is unchanged — an unevaluable
87+
condition still aborts the operation (#4775).
88+
89+
**Migrating.** A handler that cleared `ctx.input.id` — or rebound it on an
90+
`update()` — must instead write through `ctx.api` / `ctx.ql` for the row it
91+
means, have the caller pass `{ multi: true, where: … }`, or throw to refuse the
92+
write. A `beforeDelete` handler that repoints the target is unaffected. A `beforeUpdate` hook
93+
with side effects on an object that receives bulk writes should expect to run
94+
per row; a batch-wide effect belongs in a payload rewrite, which is still
95+
batch-scoped.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(plugin-auth): keep the last-administrator guard exact when `before*` hooks fire per row (#5574)
6+
7+
The break-glass guard resolved a write's target set as "a scalar `input.id` if
8+
there is one, otherwise the caller's predicate". That was sound only because a
9+
predicate (`multi: true`) write's `before*` dispatch left `input.id`
10+
present-but-**undefined**. ADR-0058 Addendum II makes the `before*` phase fire
11+
once per MATCHED ROW, each context naming its own row — so read that way, a
12+
`multi` ban of every administrator arrives as N separate by-id bans, each of
13+
which is legitimately allowed (banning one admin out of three leaves two), and
14+
the batch locked the environment out with no refusal anywhere.
15+
16+
`resolveTargetIds` now asks `options.multi` FIRST: on a predicate write the
17+
target set is the caller's predicate, whichever row the current dispatch names;
18+
the id is consulted only when the write really is by-id. `input.options` is the
19+
caller's bag during `before*``where` and `multi` included — and the contract
20+
preserves that deliberately, so the discriminator the guard needs is unchanged.
21+
22+
All eight guarded halves (#5892 ban, #5941 delete, #5978 standing) are covered
23+
by the existing predicate cases, which went red on the engine change and are now
24+
the pin that a population-scoped invariant survives being asked one row at a
25+
time.

docs/adr/0058-expression-and-predicate-surface.md

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,89 @@
289289
> here: it is a live-behaviour question on a seam another card owns, and this
290290
> appendix will not presume the answer. The engine half and #5846 settle it
291291
> together, in one edit to one ordering, and record it as an amendment.
292+
> **→ Settled in Amendment II.1 below.**
292293
> - **`scripts/adr-anchors.json`'s `hook-wrappers.ts` invariant still describes
293294
> the batch dispatch.** It is TRUE today and must move with the engine half,
294-
> not before it.
295+
> not before it. **→ Moved with #5574's engine half.**
296+
297+
---
298+
299+
> **Amendment II.1 (2026-08, #5574 engine half + #5846 (a)) — the `input.id`
300+
> reroute lever is RETIRED, and refused loudly.**
301+
> _Settles the item Addendum II left open by name. Delivered in the same PR that
302+
> delivered the per-row `before*` dispatch, because it is the same edit to the
303+
> same ordering._
304+
>
305+
> **The capability, stated exactly.** `update()` and `delete()` dispatched their
306+
> `before*` event FIRST and only then read `hookContext.input.id` to choose the
307+
> driver call. The id slot therefore doubled as a control lever: a handler
308+
> assigning `ctx.input.id = undefined` on a by-id call converted the write into
309+
> a PREDICATE write over the caller's `where`; a handler assigning a different
310+
> id moved the write to another row (`delete()` supported that explicitly, by
311+
> re-reading the pre-image for the new target — #5272).
312+
>
313+
> **Why it cannot survive the reorder.** A per-row `before*` context is BUILT
314+
> from the matched row set, so the row set must be in hand before the first
315+
> dispatch, so the branch that decides whether there IS a row set must be
316+
> decided before that. #5846's (a) direction lands in the same edit: the by-id
317+
> path reads its prior row ahead of the dispatch and binds `previous` there. By
318+
> the time any handler runs, the target is settled — `previous`, the
319+
> `readonlyWhen` strip and every validation rule have already been computed
320+
> against the row the ladder chose.
321+
>
322+
> **The three options, and the choice.** *Ignore it* — the assignment retargets
323+
> nothing and says nothing, which is the silent no-op D4 exists to abolish, and
324+
> here the write still lands on the ORIGINAL row. *Honour it by re-resolving*
325+
> the write lands on a row whose pre-image was never read, whose `readonlyWhen`
326+
> locks were never evaluated and whose rules were checked against a different
327+
> record: silently weaker enforcement, aimed by a hook. *Refuse* — chosen. The
328+
> write is rejected with `HookTargetRebindError`
329+
> (`objectql/src/hook-target-rebind-errors.ts`, code `ERR_HOOK_TARGET_REBIND`,
330+
> an `ERR_`-prefixed operational code on the error's own bag and deliberately
331+
> NOT an ADR-0112 wire code, same reasoning as the budget refusal). The message
332+
> NAMES the retired capability, so an author whose handler stopped working
333+
> learns what changed instead of watching a write land somewhere unexpected.
334+
>
335+
> **Scope, stated precisely, because the two verbs do NOT answer alike.**
336+
>
337+
> | | CLEARED id | REBOUND to another id |
338+
> |---|---|---|
339+
> | `update()` by-id | refused | refused |
340+
> | `delete()` by-id | refused | **honoured** (#5272's re-read, unchanged) |
341+
> | either, per-row | refused (D4) | refused (D4) |
342+
>
343+
> The CLEARED column is uniform because the ladder reorder leaves it no answer
344+
> of its own: clearing worked by falling through to the predicate branch, and
345+
> that branch is chosen before any handler runs. That is the capability this
346+
> amendment retires, and it is the one the ruling names.
347+
>
348+
> The REBOUND column is not uniform, and the asymmetry is principled rather
349+
> than an oversight. The case against honouring a rebind is that the write would
350+
> land on a row whose pre-image, `readonlyWhen` locks and validation rules were
351+
> never evaluated — and on `delete()` that is simply not true: #5272 already
352+
> RE-RESOLVES the new target, re-reading its pre-image and rebinding `previous`
353+
> before `afterDelete` or the summary recompute can see it. `update()` has no
354+
> such mechanism and would have to grow one, which is the "silently pick
355+
> re-resolution instead" this ruling forbids. So `update()` refuses and
356+
> `delete()` keeps honouring, until the delete-side repoint is ruled on as its
357+
> own question (#6752) — deliberately NOT folded in here as a rider on an
358+
> ordering change.
359+
>
360+
> Premise for the retirement, checked against `origin/main`: the only
361+
> `ctx.input.id` assignment in the whole repository was one engine test forcing
362+
> the fail-closed AST assertion, which is now the refusal's own pin.
363+
>
364+
> **What replaces it, for each thing it was used for.** Write a different row:
365+
> `ctx.api` / `ctx.ql` for that row explicitly. Write many rows: have the caller
366+
> pass `{ multi: true, where: … }`. Stop this write: throw from the handler —
367+
> the supported way for a `before*` guard to refuse, and the one the per-row
368+
> `previous` binding exists to enable.
369+
>
370+
> **One consequence priced with it.** `ENGINE_UPDATE_REJECT_MESSAGE` /
371+
> `ENGINE_DELETE_REJECT_MESSAGE` used to be raised AFTER the before phase, so a
372+
> handler binding `input.id` could convert a rejecting call into a by-id write.
373+
> That is the same lever pointed the other way; with the ladder resolved first
374+
> the refusal lands before any handler runs and before anything is read.
295375
296376
---
297377

0 commit comments

Comments
 (0)