feat(objectql)!: retire delete()'s by-id beforeDelete REPOINT, aligning with update() (#6752) - #7482
Merged
Merged
Conversation
…ng with update() (#6752) A `beforeDelete` handler on a by-id `delete()` may no longer move the delete onto another row by assigning `ctx.input.id`. It is refused with `HookTargetRebindError` / `ERR_HOOK_TARGET_REBIND` (`path: 'by-id'`) — the same refusal the `update()` twin and both per-row paths (D4) already raised. Nothing is deleted; `afterDelete` and the roll-up recompute never run. The rule across both verbs is now one line: a by-id target is immutable in a `before*` handler. This removes a capability that WORKED. #5272 RE-RESOLVED a repointed target, re-reading its pre-image and rebinding `previous`, so nothing stale ever reached a consumer — which is why #5574's engine half (PR #6697) deliberately left the asymmetry standing rather than folding a behaviour removal into an ordering change, and filed it as #6752. The 2026-08-09 maintainer ruling on that card retires it on three measured axes: compatibility cost zero (the repo-wide grep for assignments into a hook's `input.id`, re-run on this branch's base, finds six sites and all six are this family's own pins), one rule beats two individually-correct rules an author must memorize, and "a hook silently redirects which row gets deleted" is a top-grade footgun for authored handlers however correctly the redirect is implemented. Aligning the other way — building `update()` the same re-resolution — stays excluded by #5574's recorded ruling. The four recorded landing points move together: * ADR-0058 Amendment II.1's scope table now shows the REBOUND column uniform, with the carve-out pointing forward to the new Amendment II.2, which records the ruling and its three axes. * `hook-target-rebind-errors.ts`'s "what this error does NOT cover" section is gone — there is no exception left to remember — replaced by the full four-cell coverage table and the record of why the last cell arrived late. The REBOUND message text was written for `update()` only and would have misdescribed the delete path; it now states the rule for both verbs. * `engine.ts`'s re-read block is DELETED, not merely bypassed: its guard was `input.id !== id && input.id`, exactly the case the refusal now throws on, so it became unreachable code. The single pre-dispatch pre-image read that binds `previous` for `beforeDelete` is a different read and is untouched. * The D4 §7 pin is FLIPPED to assert the refusal with its full envelope (code, name, object, event, path, expectedId, observedId, message), plus that neither row was deleted and `afterDelete` never ran. Flipped, not deleted — the case is the record of a ruling. A new negative control pins that a same-id rewrite stays legal, which is the measurement the `input.id !== id` check rests on. Registers the ADR-0087 step-17 semantic entry `delete-by-id-before-hook-repoint-retired` (a runtime hook surface: no `sys_metadata` row for the D2 chain to rewrite, and the intent behind a repoint is not recoverable, so it is a D3 semantic TODO). Artifacts regenerated (`gen:migration-registry`, `gen:spec-changes`, `gen:upgrade-guide`). Refs #6752, #5272, #5574, PR #6697, ADR-0058, ADR-0087 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxyoeX26ZjnnwzbegYvTHW
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/l
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 109 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
os-zhuang
marked this pull request as ready for review
August 10, 2026 19:24
This was referenced Aug 10, 2026
hotlong
approved these changes
Aug 11, 2026
This was referenced Aug 11, 2026
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
3 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.
Closes #6752.
What this is
Route 2 of the 2026-08-09 maintainer ruling on #6752: a
beforeDeletehandler on a by-id
delete()may no longer move the delete onto another rowby assigning
ctx.input.id. The rebind is refused withHookTargetRebindError/ERR_HOOK_TARGET_REBIND,path: 'by-id'— exactly what theupdate()twin(ADR-0058 Amendment II.1, PR #6697) and both per-row paths (D4) already raised.
Nothing is deleted;
afterDeleteand the roll-up recompute never run.The rule across both verbs is now one line: a by-id target is immutable in a
before*handler.update()by-iddelete()by-idWriting the same id back stays legal — the check is
input.id !== id, theupdate()check verbatim.⛔ Route 3 (building
update()the same re-resolution) stays excluded by #5574'srecorded ruling; this PR does not reopen it.
This removes a capability that WORKED
Worth stating plainly, because the diff reads like a bug fix and is not one.
#5272's re-resolution was internally correct: it re-read the new target's
pre-image and rebound
previous, soafterDeleteand the summary recompute sawthe row actually deleted and nothing stale ever leaked. The case that retires a
rebind on
update()— the write landing on a row whose pre-image,readonlyWhenlocks and validation rules were never evaluated — genuinely did not apply to it.
That is why PR #6697 left the asymmetry standing rather than folding a behaviour
removal into an ordering change.
The ruling retires it anyway, on three measured axes: compatibility cost zero,
one rule beats two individually-correct rules an author must memorize, and "a
hook silently redirects which row gets deleted" is a top-grade footgun for
authored — especially AI-authored — handlers however correctly the redirect is
implemented.
The compatibility grep, re-run on this branch's base (not inherited)
Six sites, all of them tests in
packages/objectql/src/. No consumer anywhererepoints — not in the framework, plugins, examples or docs.
bulk-write-per-row-hooks.test.ts:675bulk-write-per-row-hooks.test.ts:695update()by-id CLEARED refusal pin (control)bulk-write-per-row-hooks.test.ts:715delete()by-id CLEARED refusal pin (control)bulk-write-per-row-hooks.test.ts:746bulk-write-per-row-hooks.test.ts:767engine.test.ts:1107input\.id\s*=withinput.id ===excluded, which misses the
(ctx.input as any).id =cast form every pin inbulk-write-per-row-hooks.test.tsuses — the card reported one hit where thereare six. The conclusion is unchanged and now rests on a grep that can see the
pins: every additional hit is a test of this very family, so the measured
consumer count is still zero.
The four landing points, all moved together
delete()-by-id REBOUND cellmoves from "honoured (单记录 delete 从不绑定
hookContext.previous—— 契约声明「for update/delete」,引擎只在 update 分支赋值;#5038 之后批量 delete 反而比单记录 delete 更完整 #5272's re-read, unchanged)" to refused, and thecarve-out paragraph now points forward instead of deferring. New Amendment
II.2 records the ruling, its three axes, the replacement routes, and that
route 3 stays excluded.
hook-target-rebind-errors.ts's NOT-covered section — deleted, becausethere is no longer an exception to remember. Replaced by the full four-cell
coverage table plus the record of why the last cell arrived late (the
mechanism it removed was not broken).
➕ Not on the card, found by reading: the REBOUND branch of the error
message was written for
update()only — it claimed honouring a rebind "wouldwrite a row that none of those checks ever saw", which was never true on the
delete path. Left alone it would have shipped a false explanation to the first
author who hit the new refusal. Restated for both verbs.
engine.ts's re-read block — measured dead, so removed, not justre-commented. Its guard was
wantsPreImage && input.id !== id && input.id,which is precisely the case the refusal now throws on, so it is unreachable
the moment the refusal lands. The single pre-dispatch pre-image read that
binds
previousforbeforeDelete(单 id update 把同一行前置状态读了 3 次(engine 前置行门 + sys_fetch_previous_update + plugin-audit captureBefore),且后两次不受任何按对象需求门约束 #5846 (a) / feat(objectql): dispatch before* hooks per matched row on a predicate bulk write (#5574, #5846) #6697) is a different readand is untouched —
readPreImage/bindPreImagesurvive with one call siteeach.
bulk-write-per-row-hooks.test.ts) — flipped, neverdeleted.
#5272's pins, each disposition named
bulk-write-per-row-hooks.test.ts§7still HONOURS a by-id beforeDelete REPOINTREFUSES a by-id beforeDelete REPOINT — the last cell, retired by #6752. Same handler, same fixtures; now asserts the full envelope (code,name,object,event,path,expectedId,observedId,message), that neither row was deleted, and thatafterDeletenever ran. Its comment records the flip as a ruling, not a fix, and warns against "restoring symmetry" via route 3.leaves a by-id beforeDelete that rewrites the SAME id aloneinput.id !== idcheck rests on. Same-id rewrite still deletes the addressed row andafterDeletestill describes it. Measured legal before this PR and after.hook-condition-previous-scope.test.ts§[#5272] a single-record delete binds previouspreviousbinding, not the repoint. Passes unchanged.engine-delete-prior-read-scope.test.ts(per-object read gate, #5272/#6697)plugin-audit/audit-bound-previous.test.ts:300single-id delete() pays exactly ONE findOneengine.test.ts:1107(#2982 fail-closed CLEARED)update()twin + CLEARED column + per-row D4packages/specIS touched — declaring it loudlyThree files, all ADR-0087 ledger machinery, no schema and no export surface
changed (
gen:api-surfacereports zero drift):packages/spec/src/migrations/entries/semantic/17.delete-by-id-before-hook-repoint-retired.ts— new entrypackages/spec/src/migrations/registry.ts— generated region,gen:migration-registrypackages/spec/spec-changes.json— generated,gen:spec-changesdocs/protocol-upgrade-guide.md, generated bygen:upgrade-guide)Flagging for the #6017 declaration the PM files.
Changeset + gate reading
.changeset/retire-delete-by-id-before-hook-repoint.md,@objectstack/objectql: major+@objectstack/spec: patch, carrying<!-- adr-0087: registered delete-by-id-before-hook-repoint-retired -->.check-changeset-no-major.mjs. Thebrief warned the fixed group ships breaking as
minor. Read against the gate:that rule is currently suspended — Changesets is in RC pre-release mode
(
tag: rc), where amajoronly ever producesX.0.0-rc.N, so the guard standsaside by design and says so.
majoris therefore both correct and green here,matching the immediate precedent (
708431313,registerhook-empty-target-refusal).If the RC window exits before this lands, the bump must drop to
minor.The ADR-0087 disposition is
registeredrather thannot-requiredbecause theother three are mechanically false:
@objectstack/objectqlis published (notunpublished), no prior id covers this surface (notalready-registered), andthe changeset ships rewrite instructions, so
no-migration-prescriptionis aself-contradiction the gate refuses. The entry is semantic (D3), not a D2
conversion, for the reason
hook-context-session-roles-retiredandhook-register-empty-object-target-refusedare at the same step: aHookContextis never persisted, so there is no
sys_metadatarow to rewrite, and the intentbehind a repoint is not recoverable — only the author knows whether it meant
"delete that row instead" or "delete that row too".
Gates run locally — all
completed: success@objectstack/objectqlsuite@objectstack/specsuite@objectstack/plugin-auditsuite@objectstack/objectql typechecktsc --noEmit)check:adr-anchorscheck:durability-log-levelcheck:engine-double-contractcheck:stack-collection-mapscheck-engine-split-ratio.mjscheck-changeset-no-major.mjscheck-adr-0087-registration.mjscheck-empty-changeset.mjsspec check:migration-registryspec check:spec-changesspec check:upgrade-guidespec gen:api-surfaceplugin-auditshowed 7 failed suites — traced to a stale@objectstack/objectqlbuild in the worktree (Failed to resolve entry for package), not to this diff. Green after rebuilding. Recorded so nobody re-derives it.One thing left deliberately alone
The pending
.changeset/bulk-write-before-hooks-per-row.md(PR #6697's) says "abeforeDeletehandler that repoints the target is unaffected" — true of thatPR, false of the release both ship in. I did not edit another card's pending
changeset: it is their artifact and a needless conflict magnet. My changeset
states the supersession instead. Flagging it so the release compiler can reconcile
the two entries.
Refs #6752, #5272, #5574, PR #6697, ADR-0058, ADR-0087.
Generated by Claude Code