Uh oh!
There was an error while loading. Please reload this page.
fix(service-storage): tombstone attachments on a predicate delete - #10951
Conversation
The beforeDelete -> afterDelete file-id hand-off rode on the hook context itself, on the premise that the engine passes the same HookContext to both events. Since #5574 (ADR-0058 Addendum II D1/D2) a predicate write dispatches one fresh context per matched row in each phase, so on a `multi: true` delete the stash never arrived, the orphan list was empty, and no tombstone was ever written -- leaving the file at status='committed' with its bytes stranded permanently, since an untombstoned orphan matches neither declared sweep policy on sys_file. Read the departed id from `ctx.previous.file_id` instead, which the engine binds on both phases and both dispatch paths (by-id unconditionally since #7867, per-row from the batch's doomed-row read). That is the slot the update verb's detach leg already reads, so the module now carries one mechanism for "what file did this join row point at before?" rather than two. The `beforeDelete` registration has no work left and is removed, together with its `MULTI_DELETE_RESOLVE_LIMIT` limb. That limb's unreachability was measured, not assumed: both branches of the live handler were counted while every delete shape the engine offers was driven through the wired engine, and the sibling id-branch (the positive control) fired on all six dispatchable shapes while the where-branch fired zero times -- including on `where: { id: { $in: [...] } }`, the batch-shaped delete it was written for. Fixes#10240 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
📓 Docs Drift CheckThis PR changes 1 package(s): 11 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 — 6 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 9d413315fcf89b6f75a51bc508b5b4b7524388ab && git checkout 9d413315fcf89b6f75a51bc508b5b4b7524388ab
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c2b97c2a188d3a5798f5ee224a943ccb413c6396 3fecc3eaaaa18b162765eb6c2da7672d681e8696 && git checkout -B drift-repro c2b97c2a188d3a5798f5ee224a943ccb413c6396 && git merge --no-ff 3fecc3eaaaa18b162765eb6c2da7672d681e8696
node scripts/docs-audit/affected-docs.mjs --json c2b97c2a188d3a5798f5ee224a943ccb413c6396
|
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 32524701294 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
跨 PR 相同签名（24h，按失败测试文件聚合）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes#10240
What was broken
installAttachmentLifecycleHookshanded file ids frombeforeDeletetoafterDeleteon the hook context itself (ctx['__attachmentFileIds']), on the premise stated in its own comment: "the engine passes the SAME HookContext object to both events". That was true of the pre-#5574 batch dispatch. Since ADR-0058 Addendum II (D1/D2) a predicate (multi: true) write dispatches one fresh context per matched row in each phase —dispatchPerRowBeforeHooks/buildPerRowAfterContextsin objectql'sengine.ts, which says so outright: "a per-row context is a fresh object, so a stash written on the context itself dies with the row that held it".So on a predicate delete the stash never arrived, the orphan list was empty, and no tombstone was ever written. The bytes were stranded permanently rather than late:
sys_file's declared lifecycle nominates a sweep candidate only viattl { field: 'deleted_at' }orretention { onlyWhen: { status: 'pending' } }, and an untombstoned orphan matches neither — so the reap guard is never asked about it.The premise, measured before touching anything
Both verbs, one tree, one run, on the wired
ObjectQLengine:delete(where: { id })recordstatus: "deleted",deleted_atsetdelete({ multi: true, where })per-rowstatus: "committed"— no tombstoneupdate(where: { id })re-pointrecordstatus: "deleted",deleted_atsetupdate({ multi: true, where })re-pointper-rowstatus: "deleted",deleted_atsetThe two update rows are green because the update verb's detach leg had already reached this conclusion; the delete verb is the half that was still leaking. Those four cases are now a pinned test (
both verbs now behave alike on BOTH dispatch paths), so the table cannot quietly stop being true.The repair
Read the departed id from
ctx.previous.file_id, which the engine binds to the row's pre-image on both phases and both dispatch paths — by-id unconditionally since #7867 (it is the read that also produces the 404, so it is never skipped), per-row from the batch's single doomed-row read. This is the slot the update verb's detach leg already reads, so the module now carries one mechanism for "what file did this join row point at before?" instead of two that drift apart.The
beforeDeleteregistration existed only to write the stash, so it is removed. A test pins its absence: a secondbeforeDeleteonsys_attachmentreappearing means the stash mechanism came back with it.The
MULTI_DELETE_RESOLVE_LIMITlimb — measured unreachable, not assumedThe old
beforeDeletecarried a second branch (else if (ctx.input.options.where)) resolving the doomed set itself under a 1000-row cap, for a batch-shaped context binding noinput.id. Triage asked for confirmation rather than assumption, so both branches of the live handler were instrumented with counters and every delete shape the engine offers was driven through the wired engine. The sibling id-branch is the positive control — it proves the instrument can see a hit:where: { id: { $in: [..] } }where: {}(match-all)where(unscoped)whereDelete requires an ID or options.multi=trueZero hits on the branch under test, while the control fires on the very predicate path that branch was written for — including on
where: { id: { $in: [...] } }, the batch-shaped delete it existed to serve. The mechanism agrees: all three sites that dispatchbeforeDeletebindinput.idto a scalar, and the unscoped-multi dispatch (#9719) reaches only registrations declaringdispatchUnscopedMultiWrite, which this file never did — and by definition carries nowhereat all. The limb is removed; the instrumentation was reverted byte-identically (git hash-objectverified) before the fix was written.Pins, in both directions
Ablation: reverting the handler to lose the id on
dispatch.mode === 'per-row'turns exactly the four predicate-delete pins red (expected [] to deeply equal [ 'f1' ],expected status "committed" to match "deleted") and leaves the by-id, update and wiring pins green.Upgrade note
Files already stranded by the old behaviour are not retro-actively tombstoned by this change — the repair is forward-only. That backlog is tracked separately in #10950, which is out of scope here and intentionally left for its own review: backfilling writes tombstones that become irreversible byte deletes 30 days later, which does not belong in a hook-context bug fix. The changeset says the same thing for upgraders.
Verification
Final sha
3fecc3eaa, clean tree.@objectstack/service-storagesuite: 405 passed (24 files).node scripts/pm/dispatch-gates.mjs(no path arguments) on the final commit; all 18 families green, exit codes captured before any pipe.check:i18nfirst answeredPREREQUISITE NOT MET — the workspace CLI is not built … Nothing was checked; recorded as not-measured, the CLI was built, and it then reportedcheck-i18n-bundles: OK (9 package(s) — all bundles in sync…).check:type-check-debt --re-measuregreen:33 ledger entr(ies) re-measured … none above its recorded number. Its note that@objectstack/plugin-authTEST_DEBT records 109 while tsc now reports 97 was left alone —--lowerwas not run.check:route-envelope,check:dispatcher-error-vocabulary) was run explicitly with--self-test, both green. The path derivation did not name either of them.Generated by Claude Code