feat(objectql): dispatch before* hooks per matched row on a predicate bulk write (#5574, #5846) - #6697
Merged
Merged
Conversation
… 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
… 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
…ous_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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 117 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
github-actions
Bot
added
documentation
…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
os-zhuang
marked this pull request as ready for review
August 8, 2026 15:21
This was referenced Aug 8, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5574
Fixes #5846
Engine half of #5574 (maintainer ruling B, 2026-08-06) together with the (a) half of #5846 — one ordering change, so one PR. The spec half landed in #6516; this delivers the producer and flips its
deliveredmarkers.What changed
A predicate (
multi: true) update or delete now dispatchesbeforeUpdate/beforeDeleteonce per matched row, on a single-record-shaped context carrying that row'sidandprevious— the move #5038 made for theafter*phase, held to the same yardstick. The normative contract ispackages/spec/src/data/bulk-write-hook-conformance.ts(D1–D7) and ADR-0058 Addendum II.The harm.
ctx.previouswas never bound in the before phase of a predicate write, so every guard written the way guards are written —if (ctx.previous?.locked) throw— passed silently on every batch. Fail-open, and invisible. One measured deployment had all 15 of its guard hooks bypassed by a single batch edit.previousis that row's pre-image,resultabsent,input.optionsstill the caller's bag. Zero matched rows ⇒ zero dispatches.input.data, which the loop writes back. OneupdateMany, one affected count (#4639). Never split into N writes.input.idstops being a reroute lever on the per-row path; a rebind is refused, not ignored.resolveBulkPerRowHookBudget;ObjectQL.MAX_BULK_PER_ROW_HOOK_ROWSis now a re-export.readonlyWhenstrip (#3042) and both per-row dispatches.#5846 (a).
update()reads its prior row before dispatchingbeforeUpdateand bindspreviousthere — the shapedelete()has had since #5272 — so both phases share one read.sys_fetch_previous_updateis retired: its!ctx.previousguard is now permanently false. A by-id update on a kernel used to read the same row three times; this removes one and makes the engine's read the single producer.ADR-0049 enforce-or-remove.
HookConditionLimitation(both members),isPredicateBulkWriteand thepredicateBulkWriteflag are retired — with no batch-scoped dispatch left they have neither producer nor reachable consumer. Aprevious-readingbefore*condition on a bulk write now evaluates as authored, per row.HookConditionErroritself is unchanged: an unevaluable condition still aborts (#4775).ADR-0058 Amendment II.1 — the
input.idlever, settled preciselyThe dispatch ladder is now resolved before the before phase (it has to be: per-row contexts are built from the matched row set), so the id slot can no longer steer the write. Rather than ignore an assignment (a silent no-op) or honour it blindly, the write is rejected with
HookTargetRebindError(ERR_HOOK_TARGET_REBIND), whose message names the retired capability and its three replacements.update()by-iddelete()by-idClearing is uniform, and not a discretionary choice — it worked by falling through to the predicate branch, and that branch is now chosen before any handler runs. That is the capability the ruling names.
Rebinding is deliberately not uniform. The case against honouring a rebind is that the write lands on a row whose pre-image,
readonlyWhenlocks and validation rules were never evaluated — and ondelete()that is simply not true: #5272 already re-resolves the new target, re-reading its pre-image and rebindingpreviousbeforeafterDeleteor 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. Soupdate()refuses anddelete()keeps honouring, until the delete-side repoint is ruled on as its own question — filed as #6752, deliberately not ridden in on an ordering change. The asymmetry is pinned inbulk-write-per-row-hooks.test.ts§7 D4 so neither direction gets tidied up by a later reader.A security regression found and fixed in the same PR
plugin-auth's last-administrator break-glass guard resolved its target set as "a scalar
input.idif there is one, otherwise the caller's predicate". That was sound only while a predicate write'sbefore*leftinput.idpresent-but-undefined. Under per-row dispatch amultiban of every administrator arrives as N individually legitimate by-id bans — banning one admin out of three leaves two — so the batch passed with no refusal anywhere and locked the environment out.resolveTargetIdsnow asksoptions.multifirst; the id is consulted only when the write really is by-id.input.optionsis the caller's bag duringbefore*,multiincluded, and the contract preserves that deliberately.The other six
input.options.wherereaders in the repo (service-storage ×3, plugin-sharing, plugin-approvals, plugin-audit) were each reviewed: all are per-row permission/reconcile questions, for which the per-row view is exact rather than an upper bound, so they are correct or strictly better. plugin-auth was the only guard whose predicate is a global invariant over a population, which is what a per-row view cannot see.Test placement
D1–D7 extend
packages/objectql/src/bulk-write-per-row-hooks.test.ts(§7) rather than opening a new file: "the before half diverged from the after half" is exactly the failure that would go unnoticed, and side-by-side is where it gets caught. The spec conformance table is flipped inbulk-write-hook-conformance.test.ts, andhook-input-shape-contract.test.tsmoves with the engine it pins.Blocks whose subject this change reverses are kept in place, inverted and annotated, not deleted — a reversed decision is a record (PD #13). That covers
hook-condition-bulk-previous.test.ts§a/§d,hook-condition-fail-loud.test.ts§5, andengine-update-prior-read-scope.test.ts's "beforeUpdateis not a reader of THIS read".Reverse verification
Predicted red list written before running (predicted → actual, signature-level):
deliveredmarkercarries NO result, which also fails on the dispatch count). plugin-auth predicted GREEN and was green — the guard fix is dispatch-granularity insensitive, as claimed.No prediction was green-when-predicted-red.
Gates
Every
check:*step enumerated from.github/workflows/lint.ymlwas run one by one — 58 steps, including the two added by #6672 / #6661 after this branch was cut. Two went red earlier and were fixed in5553684: the query-options erasure ratchet (engine.ts fell 12 → 9, ratcheted down; the new cases were typed rather than raising the test ceiling) andcheck:adr-anchors(hook-wrappers.tslost its ADR-0112 reference — the rule outlived the branch it anchored, so it is restated where it now applies).Not in scope
delete()'s by-id repoint — split out to delete() by-id 仍允许 beforeDelete 重定向 input.id —— 与 update() 侧的 ADR-0058 Amendment II.1 不对称，需定夺是否一并退休 #6752 rather than retired here.plugin-audit'scaptureBeforeredundant read — the identity lane's follow-up, deliberately untouched.sys_fetch_previous_delete— now provably circular (it holds open the very gate whose read makes it redundant). Retiring it stays delete() 的按对象前置行门在任何 kernel 托管引擎上恒真 —— ObjectQLPlugin 自带的 sys_fetch_previous_delete 以 object: '*' 注册在 beforeDelete #5929's card, which owns the gate's per-object honesty; the measurement is recorded inplugin.tsso that card need not rediscover it.🤖 Generated with Claude Code
https://claude.ai/code/session_013ZgeUyxzRnXzNCq8vizVoQ