Skip to content

Commit 8dd98bf

Browse files
os-zhuangclaude
andauthored
feat(objectql)!: retire delete()'s by-id beforeDelete REPOINT, aligning with update() (#6752) (#7482)
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 Claude-Session: https://claude.ai/code/session_01CxyoeX26ZjnnwzbegYvTHW Co-authored-by: Claude <noreply@anthropic.com>
1 parent b3430af commit 8dd98bf

9 files changed

Lines changed: 443 additions & 81 deletions
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
---
2+
"@objectstack/objectql": major
3+
"@objectstack/spec": patch
4+
---
5+
6+
<!-- adr-0087: registered delete-by-id-before-hook-repoint-retired -->
7+
8+
feat(objectql)!: retire `delete()`'s by-id `beforeDelete` REPOINT, aligning it with `update()` (#6752)
9+
10+
A `beforeDelete` handler on a **by-id** `delete()` may no longer move the
11+
delete onto a different row by assigning `ctx.input.id`. The rebind is
12+
**refused** with `HookTargetRebindError` / `ERR_HOOK_TARGET_REBIND`
13+
(`path: 'by-id'`) — exactly what the `update()` twin and both per-row paths
14+
already raised. Nothing is deleted, and `afterDelete` and the roll-up
15+
recompute never run.
16+
17+
**The rule is now one line, on both verbs: a by-id target is immutable in a
18+
`before*` handler.**
19+
20+
| | CLEARED id | REBOUND to another id |
21+
| ------------------ | -------------- | ------------------------- |
22+
| `update()` by-id | refused | refused |
23+
| `delete()` by-id | refused | **refused** (was honoured)|
24+
| either, per-row | refused (D4) | refused (D4) |
25+
26+
**Removed keys and their prescriptions (FROM → TO):**
27+
28+
| Wrote | Write instead |
29+
| --- | --- |
30+
| `beforeDelete` handler: `ctx.input.id = otherId` | `await ctx.ql.delete(object, otherId)` for that row explicitly, and let the addressed delete proceed — or `throw` from the handler to stop it |
31+
| `beforeDelete` handler repointing to delete a set | have the **caller** pass `{ multi: true, where: … }` |
32+
33+
Writing the **same** id back is unaffected and stays legal: the check is
34+
`input.id !== id`, the `update()` check verbatim, so a handler that reads the
35+
id or assigns it to itself is not caught.
36+
37+
**This removes a capability that WORKED, and the reasoning has to be read that
38+
way.** `delete()` had a re-resolution for a repointed target since #5272: it
39+
re-read the new target's pre-image and rebound `previous`, so `afterDelete` and
40+
the summary recompute saw the row actually deleted. Nothing stale ever leaked,
41+
and the case that retires a rebind on `update()` — the write landing on a row
42+
whose pre-image, `readonlyWhen` locks and validation rules were never evaluated
43+
— did not apply to it. That is why #5574's engine half (PR #6697) deliberately
44+
left the asymmetry standing and filed it as its own question.
45+
46+
The 2026-08-09 maintainer ruling on #6752 retires it anyway, on three measured
47+
axes:
48+
49+
- **Compatibility cost, measured: zero.** A repository-wide grep for assignments
50+
into a hook's `input.id`, re-run on this PR's base rather than inherited,
51+
finds six sites and all six are this family's own pins. No consumer anywhere
52+
repoints — not in the framework, plugins, examples or docs.
53+
- **One rule beats two correct rules.** Two verbs answering the same slot
54+
differently is something every hook author must hold in memory, and the
55+
justification for the split lived in an ADR, not at the call site.
56+
- **The surface is a footgun independent of the mechanism.** "A hook silently
57+
redirects which row gets deleted" is a top-grade hazard for authored — and
58+
especially AI-authored — handlers. Correctness of a mechanism does not justify
59+
the surface it exposes.
60+
61+
Aligning the other way — building `update()` the same re-resolution — stays
62+
excluded by #5574's recorded ruling ("do not silently pick re-resolution
63+
instead").
64+
65+
The re-read block in `delete()`'s by-id branch is **deleted, not bypassed**: its
66+
guard was `input.id !== id && input.id`, precisely the case the refusal now
67+
throws on, so it became unreachable the moment the refusal landed. The single
68+
pre-dispatch pre-image read that binds `previous` for `beforeDelete` is a
69+
different read and is untouched.
70+
71+
Recorded as **ADR-0058 Amendment II.2**; the `hook-target-rebind-errors.ts`
72+
"what this does NOT cover" section is gone, because there is no longer an
73+
exception to remember. The #5272 pin asserting the repoint was honoured is
74+
**flipped to assert the refusal**, not deleted, with a new negative control
75+
pinning that a same-id rewrite stays legal.
76+
77+
Supersedes the scope note in the pending `bulk-write-before-hooks-per-row`
78+
changeset ("a `beforeDelete` handler that repoints the target is unaffected"),
79+
which described PR #6697's deliberate carve-out and is closed by this change in
80+
the same release.

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

Lines changed: 77 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -332,30 +332,32 @@
332332
> NAMES the retired capability, so an author whose handler stopped working
333333
> learns what changed instead of watching a write land somewhere unexpected.
334334
>
335-
> **Scope, stated precisely, because the two verbs do NOT answer alike.**
335+
> **Scope, stated precisely. Both verbs now answer alike** — see Amendment II.2,
336+
> which closed the one cell this amendment left open.
336337
>
337338
> | | CLEARED id | REBOUND to another id |
338339
> |---|---|---|
339340
> | `update()` by-id | refused | refused |
340-
> | `delete()` by-id | refused | **honoured** (#5272's re-read, unchanged) |
341+
> | `delete()` by-id | refused | refused (#6752 — was **honoured** as delivered here) |
341342
> | either, per-row | refused (D4) | refused (D4) |
342343
>
343344
> The CLEARED column is uniform because the ladder reorder leaves it no answer
344345
> of its own: clearing worked by falling through to the predicate branch, and
345346
> that branch is chosen before any handler runs. That is the capability this
346347
> amendment retires, and it is the one the ruling names.
347348
>
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.
349+
> The REBOUND column was **not** uniform as this amendment shipped, and the
350+
> asymmetry was principled rather than an oversight. The case against honouring
351+
> a rebind is that the write would land on a row whose pre-image, `readonlyWhen`
352+
> locks and validation rules were never evaluated — and on `delete()` that was
353+
> simply not true: #5272 RE-RESOLVED the new target, re-reading its pre-image
354+
> and rebinding `previous` before `afterDelete` or the summary recompute could
355+
> see it. `update()` has no such mechanism and would have to grow one, which is
356+
> the "silently pick re-resolution instead" this ruling forbids. So `update()`
357+
> refused and `delete()` kept honouring, with the delete-side repoint carved out
358+
> to be ruled on as its own question (#6752) — deliberately NOT folded in here
359+
> as a rider on an ordering change. **→ Ruled on, and retired, in Amendment II.2
360+
> below.**
359361
>
360362
> Premise for the retirement, checked against `origin/main`: the only
361363
> `ctx.input.id` assignment in the whole repository was one engine test forcing
@@ -375,6 +377,68 @@
375377
376378
---
377379

380+
> **Amendment II.2 (2026-08, #6752 maintainer ruling) — `delete()`'s by-id
381+
> REPOINT is RETIRED too. The REBOUND column is now uniform.**
382+
> _Settles the one cell Amendment II.1 carved out by name. The rule across both
383+
> verbs is now sayable in one line: **a by-id target is immutable in a `before*`
384+
> handler.**_
385+
>
386+
> **What changes.** A `beforeDelete` handler that assigns `ctx.input.id` a
387+
> DIFFERENT id no longer moves the delete to that row. It is refused with the
388+
> same `HookTargetRebindError` / `ERR_HOOK_TARGET_REBIND` the `update()` twin
389+
> and both per-row paths (D4) already raise, with `path: 'by-id'` and
390+
> `expectedId` / `observedId` naming both ends of the move. Nothing is deleted;
391+
> `afterDelete` and the summary recompute never run. Writing back the SAME id is
392+
> untouched and still legal — the refusal is `input.id !== id`, the `update()`
393+
> check verbatim, so a handler that reads the id or assigns it to itself is not
394+
> caught.
395+
>
396+
> **This is a behaviour REMOVAL of a mechanism that worked, and the reasoning
397+
> has to be read that way.** #5272's re-resolution was internally correct: it
398+
> re-read the new target's pre-image and rebound `previous`, so nothing stale
399+
> ever reached `afterDelete` or the roll-up. No defect was found in it, and the
400+
> second bullet of the three-options paragraph above genuinely did not apply to
401+
> it. It is retired on three measured axes instead:
402+
>
403+
> - **Compatibility cost, measured: zero.** A repository-wide grep for
404+
> assignments into a hook's `input.id` — re-run against the base of the
405+
> implementing PR, not inherited from the card — finds six sites, all of them
406+
> tests in `packages/objectql/src/`: four are this family's OWN pins (the
407+
> per-row rebind refusal, both by-id CLEARED refusals, the same-id negative
408+
> control), one is the repoint pin being flipped here, and the sixth
409+
> (`engine.test.ts`) clears the id to force the #2982 fail-closed assertion.
410+
> **No consumer anywhere repoints** — not in the framework, not in the
411+
> plugins, not in the examples, not in the docs. The removal cannot break code
412+
> that does not exist.
413+
> - **One rule beats two correct rules.** Two verbs answering the same slot
414+
> differently is a thing every hook author must hold in memory, and the
415+
> justification for the split ("`delete()` re-resolves, `update()` cannot")
416+
> lives in an ADR, not at the call site where a handler is written.
417+
> - **The surface is a footgun independent of the mechanism.** "A hook silently
418+
> redirects which row gets deleted" is a top-grade hazard for authored — and
419+
> especially AI-authored — handlers. Correctness of a mechanism does not
420+
> justify the surface it exposes.
421+
>
422+
> **Route 3 stays excluded.** Aligning the other way — growing `update()` the
423+
> same re-resolution — remains forbidden by #5574's recorded ruling ("do not
424+
> silently pick re-resolution instead"), and this amendment does not reopen it.
425+
> The alignment was only ever going to run in this direction; the open question
426+
> was whether to align at all.
427+
>
428+
> **What replaces it.** Exactly what replaced the CLEARED lever, and for the
429+
> same reasons: to delete a DIFFERENT row, call `ctx.api` / `ctx.ql` for that row
430+
> explicitly; to delete MANY rows, have the caller pass `{ multi: true, where: … }`;
431+
> to stop this delete, throw from the handler.
432+
>
433+
> **The mechanism removed with it.** The re-read block in `delete()`'s by-id
434+
> branch is DELETED, not merely bypassed — its guard was `input.id !== id &&
435+
> input.id`, precisely the case the refusal now throws on, so it became
436+
> unreachable code the moment the refusal landed. The single pre-dispatch
437+
> pre-image read that binds `previous` for `beforeDelete` (#5846 (a) / #6697) is
438+
> a different read and is untouched.
439+
440+
---
441+
378442
## TL;DR
379443

380444
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.

0 commit comments

Comments
 (0)