Uh oh!
There was an error while loading. Please reload this page.
fix(objectql): restore the #4757 unscoped multi-delete refusal on sys_attachment through the wired engine - #9797
Conversation
…e refusal through the wired engine The predicate path's per-row dispatch (#5038/#5574) binds input.id on every beforeDelete dispatch, so sys_attachment's declared #4757 refusal of a predicate-less multi: true delete always took the handler's by-id branch — and a zero-match predicate dispatched nothing at all. The refusal could not fire through ObjectQL.delete on exactly the shape it refuses. Engine: a new opt-in registration declaration, dispatchUnscopedMultiDelete (beforeDelete only, refused elsewhere), makes ObjectQL.delete's predicate branch dispatch the whole-operation context ONCE — before the matched-row read, zero-match included — to registrations that declared for it. Binding input.id on that context is refused (HookTargetRebindError, path 'unscoped-multi'), mirroring D4/#6752. Undeclared objects see no new dispatch. service-storage: the sys_attachment access-hook registration declares the flag, making the handler's own #4757 branch reachable again with its declared envelope (ATTACHMENT_DELETE_DENIED, 403). The unit block that pinned the whole-operation shape by direct handler call — green while the wired engine did the opposite — is re-pointed at a real ObjectQL engine. Fixes#9719 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
check:type-check-debt re-measured @objectstack/objectql at +3 over its frozen 355: two 1-arg registerObject calls and one unused loop variable, all in the new dispatch test. Fixed at the source (registerObject gets its packageId; the seed loop no longer declares an unused row) — the ledger number is back to exactly 355. The service-storage harness carried the same two idioms latently (its test layer is not ledger-measured today) — fixed the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
…coped-multi-delete-refusal
📓 Docs Drift CheckThis PR changes 2 package(s): 8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 16 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b9d8338c8e39faa4e98625b117bf7cfdeb3f50c3 && git checkout b9d8338c8e39faa4e98625b117bf7cfdeb3f50c3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 185c7bd51ac92d934ba1ad813eb01ebb6e2ca55b 8f430db06fc41346607fa6c3913730bbf442efed && git checkout -B drift-repro 185c7bd51ac92d934ba1ad813eb01ebb6e2ca55b && git merge --no-ff 8f430db06fc41346607fa6c3913730bbf442efed
node scripts/docs-audit/affected-docs.mjs --json 185c7bd51ac92d934ba1ad813eb01ebb6e2ca55b
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#9719
What was broken
sys_attachment's declared #4757 refusal — amulti: truedelete carrying no id and nowheremust be refused outright — could not fire throughObjectQL.delete. Re-measured on this tree before writing code, all four premises from the claim comment held, plus the fourth limb:attachment-access-hooks.tswith its declared envelope (ATTACHMENT_DELETE_DENIED, 403);input.idon every predicate dispatch;beforeDeletedispatch sites are the by-id branch and the per-row fan-out;doomed.lengthbeing positive, so an unscoped delete matching nothing dispatches nothing — a caller probing against an empty table sees success and ships the unscoped delete.Limb 4 is why the refusal cannot live solely in the handler, and it is measured in this PR's ablation (below), not assumed.
Ruling and route
Triage's adjudication (issue comment
5331638200): restore the declared refusal forsys_attachment(option 1), re-point the false-green unit block in the same PR; option 3 not available, option 2 (generalize to every guarded object) not commissioned.Both PM-suggested mechanisms were measured; the shipped shape is the second one made literal — declared, not inferred:
optionsonly — rejected by measurement: it rides the per-row fan-out, so the zero-match limb still never dispatches (the ablation leg reproduces exactly this: the empty-table pin stays a resolve, with no per-row dispatch).buildSession"session-worthy" judgment plus the handler's own gates, and the error vocabulary is the consumer's; a second engine-side copy of either can drift from the one the handler already declares and unit-tests.registerMiddlewareis an optional member of the lifecycle-engine surface (silent fail-open where absent), it would re-implement the session-worthy judgment from raw context, and it leaves the handler's declared branch permanently dead — the exact declared-vs-wired gap this card exists to kill.Shipped:
registerHookgains an opt-in declaration,dispatchUnscopedMultiDelete(valid onbeforeDeleteonly; refused loudly on any other event — ADR-0078, no silently inert declaration).ObjectQL.delete's predicate branch dispatches the whole-operation context ONCE — before the matched-row read, zero-match included — to declaring registrations, when the caller's raw predicate is absent ornull(read frominput.options.where, the upper-bound slothook.zod.tsblesses forbefore*handlers; never from the AST, which middleware may have narrowed). The dispatched context is derived,dispatch: { mode: 'record', index: 0 }with the batchscopeidentity-shared, so everyhook.zod.tssentence about the batch-level context stays true. Bindinginput.idon it is refused (HookTargetRebindError, new path'unscoped-multi') — same rule as D4/#6752, never a silent no-op.The engine stays neutral: undeclared registrations, scoped deletes (including the match-all
where: {}), by-id deletes, and every other object see zero new dispatches — pinned. The product behaviour change is enacted solely by thesys_attachmentregistration declaring the flag, which makes the handler's own #4757 branch reachable again with its declared envelope. Option 2 is not taken; generalizing remains a separate decision card.Test re-point (the false green, killed)
The
unscoped multi-delete (no id, no where) — #4757block inattachment-access-hooks.test.tscalled the handler directly with a hand-rolled whole-operation context and stayed green while the wired engine wiped the table. It is replaced by a wired block driving a realObjectQL+ in-memory driver + this module's installer (ql.delete('sys_attachment', …)end to end), pinning:{ multi: true }from the uploader of EVERY row rejects withcode: 'ATTACHMENT_DELETE_DENIED',status: 403, message first-sentenceRefusing an unscoped multi-delete of attachments…— and the rows survive;where: nullthe same way;where: {}on the same empty table resolves — the refusal measures the shape, not the emptiness);Cannot delete attachment…message) still firing through the wire;where, and match-allwhere: {}deletes still resolve; system-context and context-less programmatic unscoped deletes still bypass.Engine-side pins (new
engine-unscoped-multi-delete-dispatch.test.ts, 13 cases against the real engine): dispatch condition, whole-operation shape, ordering (refusal before any driver call — zero reads), zero-match limb with its no-flag positive control, neutrality for scoped/by-id/undeclared, scope identity, rebind refusal, and the registration-time flag validation.Note the wired service-storage block consumes
@objectstack/objectqlthrough dist (deliberately un-aliased, per theKNOWN_UNALIASED_TEST_IMPORTSledger) — rebuild objectql before trusting a local verdict from it; the test file says so in place.Verification
Union run at
8f430db0(post-merge of origin/main, workspace rebuilt):@objectstack/objectql: 219 files / 3870 tests green;typecheckgreen.@objectstack/service-storage: 24 files / 360 tests green.check:durability-log-level·check:stack-collection-maps·check:test-source-alias·check:type-source-resolution·check:nul-bytes·check:i18n·check:query-options-erasure·check:type-check-coverage·check:engine-double-contract·check:where-matcher(both new stub matchers conform by refusing combinators) ·check:changeset-gate-self-tests·check:objectui-changeset·check-adr-0087-registration·check-changeset-no-major·check-empty-changeset·check-engine-split-ratio(report-only) ·check-affected-docs. The changeset five were named by re-runningdispatch-gates.mjsagainst the real diff (additions beyond the dispatch list, as ordered).check:type-check-debt(re-measure): green at the final HEAD as well — 33 ledger entries re-measured, none above its recorded number, surplus none;@objectstack/objectqlsits exactly at its frozen 355 after the excursion fix below. (At first report time this re-run was still queued on the shared verify lock and was honestly recorded in_progress; this line records its completed reading.)ablation-dist-preflight.mjs, markerdispatchUnscopedMultiDeleteHooks): withengine.ts+hook-target-rebind-errors.tsrestored to origin/main and objectql rebuilt (marker proven ABSENT from all built files), the three wired unscoped pins go red by resolving —promise resolved "2"on the uploader-of-every-row case is the issue's measured wipe, reproduced — while the other 26 stay green; fix restored and rebuilt (marker proven PRESENT), 29/29 green.check:type-check-debtexcursion: the first re-measure read@objectstack/objectqlat +3 over its frozen 355 — all three in the new test file (two 1-argregisterObjectcalls, one unused loop variable). Fixed at the source; re-measured back to exactly 355. No ledger entry was raised.Read-couplings observed (declared in the claim):
check-durability-degradation-log-level.mjschanged on main mid-run (#9657's lane) and was re-run green at the final HEAD under its new detection.check:engine-double-contract(#9680's lane) showed no shift across my runs.Declared-surface deviations (claim comment
5334565259)The claim declared
engine.tsto region level plus the two attachment files. The commissioned mechanism mechanically entails four adjacencies, none in contact with any concurrent card (#9476 is insrc/validation/record-validator.ts; the devx cards touch scripts only):engine.tsHookEntry+registerHook(~:1310, ~:2095) — the declaration has to live on the registration;engine.tsnew privatedispatchUnscopedMultiDeleteHooksbesidedispatchPerRowBeforeHooks;hook-target-rebind-errors.ts— one union member + one message branch, so the retired-lever refusal does not lie about the seam it fired on (the claim's own "if the context-shape contract must move" clause);attachment-lifecycle.ts— theAttachmentLifecycleEngine.registerHookoptions type carries the flag so the registration type-checks without a cast.Changeset
@objectstack/objectqlminor (new opt-in registration declaration),@objectstack/service-storagepatch (guard restoration). Not declared-breaking: the accept set does not expand — this is a declared = enforced restoration (triage's boundary test), so no ADR-0087 disposition marker is owed.Generated by Claude Code