Summary
Split out of #7413 (PR #7476) rather than folded into it, per that card's pre-dispatch assessment: "if afterDelete semantics need re-timing (fire-after-commit), that may be its own card — file, don't fold, unless it is inseparable." It is separable, and it is not a regression introduced by #7413 — it is a pre-existing property of every atomic write path in the engine, which #7413 merely extends to one more path.
afterDelete fires inside the transaction, before the commit. When that transaction rolls back, the hook has already fired for a row that still exists once the dust settles. A hook that emits an event, writes to an external system, or sends a notification has therefore announced a deletion that did not happen.
Where
Two paths, same shape:
The parent's own afterDelete on the by-id path is not affected — #7413 places the wrap between the parent's beforeDelete and afterDelete deliberately, so the parent's after-phase runs only once its own delete has committed. This is about the cascaded children and the batch members.
Why it is not simply a bug in #7413
In both paths the alternative was strictly worse. Before #7413 the cascaded child's afterDelete fired and the row stayed deleted while the caller was told the operation failed — so the hook was consistent with the data and both were wrong. After #7413 the data is right and the hook is the only thing left inconsistent. That is a strict improvement, and it is the same trade #4620 already accepted for the batch path.
What has never been decided is whether afterDelete (and afterInsert/afterUpdate, which have the same shape) should mean "the write happened" or "the write has been requested and will happen unless this unit of work is undone". Today it means the second and is documented as neither.
The fork
- Declare the current semantics.
after* fires inside the unit of work; a hook with external side effects is responsible for tolerating a rollback. Cheapest, and arguably correct for hooks that write through the same engine (their writes roll back too, which is what makes an in-engine audit hook work at all).
- Re-time to fire after commit.
after* hooks queue during the transaction and dispatch once it commits. Correct for external side effects, but it changes what ctx.api writes inside an after* hook mean — they would land outside the transaction, which is a different behaviour change on a much wider surface than this issue's.
- Let the hook declare it — an authorable
timing: 'in-transaction' | 'after-commit'. Most expressive, most spec surface.
Option 2 in particular is not a local edit: plugin-audit, service-storage's file-reference lifecycle and plugin-sharing's record-share cascade all register after* hooks today and would each need re-reading against the new timing.
Impact
Silent for hooks whose only effect is through the engine (they roll back with everything else). Real for hooks with effects outside it — notifications, webhooks, external index updates, file deletions — where a rolled-back atomic batch or a refused cascade delete produces an announcement of a deletion that never happened. The blast radius grew slightly with #7413, since the by-id cascade is a far more ordinary operation than an explicit atomic: true batch.
Prior art
Filed unassigned — recording the finding, not claiming it.
Summary
Split out of #7413 (PR #7476) rather than folded into it, per that card's pre-dispatch assessment: "if
afterDeletesemantics need re-timing (fire-after-commit), that may be its own card — file, don't fold, unless it is inseparable." It is separable, and it is not a regression introduced by #7413 — it is a pre-existing property of every atomic write path in the engine, which #7413 merely extends to one more path.afterDeletefires inside the transaction, before the commit. When that transaction rolls back, the hook has already fired for a row that still exists once the dust settles. A hook that emits an event, writes to an external system, or sends a notification has therefore announced a deletion that did not happen.Where
Two paths, same shape:
runAtomicBatchinpackages/metadata-protocol/src/protocol.ts—deleteManyData/batchDatawithatomic: true(fix(metadata-protocol): deleteManyData has the same fake-atomic as batchData, updateManyData ignores atomic entirely #4620). The per-id loop callsengine.delete()insideengine.transaction(), and each of those fires the object'sbeforeDelete/afterDelete. A batch that aborts on record i rolls back records0..i-1whoseafterDeletehooks have already run.ObjectQL.delete's by-id cascade inpackages/objectql/src/engine.ts, as of ObjectQL.delete's single-id cascade is not transactional — a refusal mid-cascade leaves earlier children deleted while the response says the delete failed #7413.cascadeDeleteRelationsrecursively re-entersthis.delete()per dependent, so each cascaded child'safterDeletefires inside the wrap; a later refusal rolls the rows back underneath it.The parent's own
afterDeleteon the by-id path is not affected — #7413 places the wrap between the parent'sbeforeDeleteandafterDeletedeliberately, so the parent's after-phase runs only once its own delete has committed. This is about the cascaded children and the batch members.Why it is not simply a bug in #7413
In both paths the alternative was strictly worse. Before #7413 the cascaded child's
afterDeletefired and the row stayed deleted while the caller was told the operation failed — so the hook was consistent with the data and both were wrong. After #7413 the data is right and the hook is the only thing left inconsistent. That is a strict improvement, and it is the same trade #4620 already accepted for the batch path.What has never been decided is whether
afterDelete(andafterInsert/afterUpdate, which have the same shape) should mean "the write happened" or "the write has been requested and will happen unless this unit of work is undone". Today it means the second and is documented as neither.The fork
after*fires inside the unit of work; a hook with external side effects is responsible for tolerating a rollback. Cheapest, and arguably correct for hooks that write through the same engine (their writes roll back too, which is what makes an in-engine audit hook work at all).after*hooks queue during the transaction and dispatch once it commits. Correct for external side effects, but it changes whatctx.apiwrites inside anafter*hook mean — they would land outside the transaction, which is a different behaviour change on a much wider surface than this issue's.timing: 'in-transaction' | 'after-commit'. Most expressive, most spec surface.Option 2 in particular is not a local edit:
plugin-audit,service-storage's file-reference lifecycle andplugin-sharing's record-share cascade all registerafter*hooks today and would each need re-reading against the new timing.Impact
Silent for hooks whose only effect is through the engine (they roll back with everything else). Real for hooks with effects outside it — notifications, webhooks, external index updates, file deletions — where a rolled-back atomic batch or a refused cascade delete produces an announcement of a deletion that never happened. The blast radius grew slightly with #7413, since the by-id cascade is a far more ordinary operation than an explicit
atomic: truebatch.Prior art
ObjectQL.delete's by-id cascade is one unit of work (#7413) #7476 — extends the same timing to the by-id cascade; pins hook count and order unchanged on both success and refusal, precisely so this question stays open rather than being silently answered.Filed unassigned — recording the finding, not claiming it.