From ca53e87f7aa9561ec4e9d424b9490a662a57dccb Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Tue, 11 Aug 2026 03:59:04 +0000 Subject: [PATCH] docs(spec,objectql): declare after* hooks fire inside the unit of work (#7477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `afterInsert`/`afterUpdate`/`afterDelete` are dispatched before the enclosing transaction commits. What that guarantees was never written down, so it is now declared per the maintainer ruling on #7477 (Option 1): an `after*` hook means "the write has been requested and will happen unless this unit of work is undone", not "the write happened" — a hook with side effects outside the engine is responsible for tolerating a rollback. Zero behaviour change. The statement lands as JSDoc on `HookEvent` and `HookEventType` in @objectstack/spec, on `DISPATCHABLE_HOOK_EVENTS`, `HookHandler` and `triggerHooks` in @objectstack/objectql, and as a new section on content/docs/automation/hooks.mdx. The existing #7413 pin already asserted this ordering; its comment now records the ruling instead of leaving the question open — its assertions are unchanged. --- .../after-hook-in-transaction-semantics.md | 45 ++++++++++++++ content/docs/automation/hooks.mdx | 49 ++++++++++++++- .../src/engine-cascade-delete-atomic.test.ts | 15 ++++- packages/objectql/src/engine.ts | 59 +++++++++++++++++++ packages/spec/src/data/hook.zod.ts | 40 +++++++++++++ 5 files changed, 204 insertions(+), 4 deletions(-) create mode 100644 .changeset/after-hook-in-transaction-semantics.md diff --git a/.changeset/after-hook-in-transaction-semantics.md b/.changeset/after-hook-in-transaction-semantics.md new file mode 100644 index 0000000000..e6f6172f8f --- /dev/null +++ b/.changeset/after-hook-in-transaction-semantics.md @@ -0,0 +1,45 @@ +--- +"@objectstack/spec": patch +"@objectstack/objectql": patch +--- + +docs(spec,objectql): declare that `after*` hooks fire inside the unit of work (#7477) + +`afterInsert` / `afterUpdate` / `afterDelete` are dispatched **before** the +enclosing transaction commits. What that guarantees has never been written +down, and the two readings differ in exactly the case that matters — so it is +now declared, on the API surface and in the docs, per the maintainer ruling on +#7477. + +**The declared meaning:** an `after*` hook means *"the write has been requested +and will happen unless this unit of work is undone"* — not *"the write +happened"*. A later refusal inside the same unit rolls the row back after the +hook has already run. + +Three ordinary operations put a write inside such a unit: + +- a by-id `delete()` whose cascade is atomic — each **cascaded child's** + `afterDelete` fires inside the wrap the parent opened (#7413); the parent's + own `afterDelete` runs after that unit closes and is unaffected; +- `batchData` / `deleteManyData` with `atomic: true` — every member's `after*` + fires inside one transaction that aborts on the first failure (#4620); +- any caller that wrapped the write in `engine.transaction()` / + `ctx.api.transaction()`. + +**What it means for a handler.** Effects routed back through the engine +(`ctx.api`, `ctx.ql`) join the same transaction and roll back with everything +else — that is what makes an in-engine audit or projection hook correct. +Effects that leave the engine — webhooks, notifications, external index +updates, file deletion — are the handler's own responsibility to make +rollback-tolerant: idempotent and reconcilable, or handed to a worker that +re-reads the record instead of trusting the event alone. + +**No behaviour change.** Nothing about when a hook fires moved; the alternative +(deferring `after*` to commit) was considered and rejected in the same ruling, +because it would push a handler's own `ctx.api` writes outside the transaction +the write ran in. The statement lands as JSDoc on `HookEvent` and +`HookEventType` in `@objectstack/spec`, on `DISPATCHABLE_HOOK_EVENTS`, +`HookHandler` and `triggerHooks` in `@objectstack/objectql`, and as a new +section on the Hooks documentation page. The existing #7413 pin already +asserted this ordering; its comment now records the ruling instead of leaving +the question open. diff --git a/content/docs/automation/hooks.mdx b/content/docs/automation/hooks.mdx index a45c79e4c7..0f7dd1a07a 100644 --- a/content/docs/automation/hooks.mdx +++ b/content/docs/automation/hooks.mdx @@ -169,8 +169,11 @@ export const AccountBeforeWrite: Hook = { ## After Hook -React after a record is persisted. Use `ctx.previous` for the pre-change -snapshot and `ctx.api.object('x')` for cross-object writes: +React after a record is written — but before the enclosing unit of work +commits, so read [After hooks run inside the unit of +work](#after-hooks-run-inside-the-unit-of-work) before giving one a side effect +outside the engine. Use `ctx.previous` for the pre-change snapshot and +`ctx.api.object('x')` for cross-object writes: ```typescript export const OpportunityAfterUpdate: Hook = { @@ -195,6 +198,45 @@ export const OpportunityAfterUpdate: Hook = { }; ``` +## After hooks run inside the unit of work + +An `after*` hook does **not** mean "the write happened". It means **the write +has been requested and will happen unless this unit of work is undone**. +`afterInsert`, `afterUpdate` and `afterDelete` are dispatched *before* the +enclosing transaction commits, so a later refusal in the same unit can roll the +row back after your handler has already run. + +Three ordinary operations put a write inside such a unit: + +| Operation | What is inside the transaction | +| :--- | :--- | +| A by-id `delete()` that cascades to dependent records | Each **cascaded child's** `afterDelete`. The parent's own `afterDelete` runs after that unit closes, so it is unaffected | +| `batchData` / `deleteManyData` with `atomic: true` | Every member's `after*` — the batch aborts and rolls back on the first failure | +| Any write you wrapped yourself in `ctx.api.transaction(...)` or `engine.transaction(...)` | Everything in the callback | + +What this means when you write a handler: + +- **Effects that go back through the engine are safe.** Writes made with + `ctx.api.object('x')` join the same transaction and roll back with + everything else — that is what makes an in-engine audit or projection hook + correct in the first place. +- **Effects that leave the engine are yours to make rollback-tolerant.** A + webhook, a notification, an email, an external search-index update or a file + deletion has already gone out when the rollback happens, announcing a change + that did not survive. Make the effect idempotent and reconcilable, or hand it + to a worker that re-reads the record before acting rather than trusting the + event on its own. + +Before hooks carry no such caveat: they run before the write is issued, and +throwing from one refuses the operation outright. + + +This is a deliberate, ruled semantics ([#7477](https://github.com/objectstack-ai/objectstack/issues/7477)), +not an implementation detail awaiting a fix. Deferring `after*` to commit time +would move a handler's own `ctx.api` writes *outside* the transaction the write +ran in, which is a worse guarantee than the one documented here. + + ## Hook Context `handler` receives a `HookContext` with these fields: @@ -233,6 +275,9 @@ ctx = { - Trigger unbounded cascades of writes - Perform heavy/long-running work inline in a hook - Mutate `ctx.result` in before hooks (it is only populated for after hooks) +- Treat an `after*` hook as proof the write committed — it fires inside the + unit of work, so an un-retractable external effect there can outlive a + rollback (see [above](#after-hooks-run-inside-the-unit-of-work)) ## Related business logic diff --git a/packages/objectql/src/engine-cascade-delete-atomic.test.ts b/packages/objectql/src/engine-cascade-delete-atomic.test.ts index 13dee7f029..a51bacc20a 100644 --- a/packages/objectql/src/engine-cascade-delete-atomic.test.ts +++ b/packages/objectql/src/engine-cascade-delete-atomic.test.ts @@ -501,8 +501,19 @@ describe('hook firing is unchanged by the transaction wrap (#7413)', () => { // every atomic write path in this engine — `runAtomicBatch` (#4620) fires // per-row delete hooks inside the same rollback-able scope — and it is the // strictly better half of the trade: before this card the hook fired AND - // the row stayed gone. Re-timing `afterDelete` to fire after commit is a - // separate question, filed rather than folded in here. + // the row stayed gone. + // + // [#7477] The re-timing question this comment used to leave open ("filed + // rather than folded in here") has since been RULED, and the answer is the + // shape asserted below: `after*` fires INSIDE the unit of work, meaning + // "the write has been requested and will happen unless this unit is + // undone" — a hook with side effects outside the engine is responsible for + // tolerating the rollback. So this expectation is no longer the status quo + // pinned pending a decision; it is the decided contract, and changing it + // needs the ruling reopened rather than a test update. The author-facing + // statement lives on `HookEvent` (`@objectstack/spec`'s `data/hook.zod.ts`), + // on `DISPATCHABLE_HOOK_EVENTS` and `HookHandler` in `engine.ts`, and in + // `content/docs/automation/hooks.mdx`. expect(events).toEqual([ 'parent:beforeDelete', 'kid:beforeDelete', diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 8ff7d3bc80..0ce09e2517 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -193,6 +193,39 @@ export interface AdmittedValueShapeViolationTally { * events cover both single-id and bulk (`multi: true`) writes (#3195). A hook * subscribing to anything outside this set would silently never fire, so * `registerHook` warns rather than accepting it blindly. + * + * ## WHEN `after*` fires, relative to the commit (#7477) + * + * `afterInsert`/`afterUpdate`/`afterDelete` are dispatched INSIDE the unit of + * work, before the enclosing transaction (if any) commits. The declared + * meaning is **"the write has been requested and will happen unless this unit + * of work is undone"** — not "the write happened". This is the ruled semantics + * (#7477, 2026-08-11), not an accident of the current call sites: an `after*` + * dispatch is deliberately NOT deferred to commit, because deferring it would + * push a handler's own `ctx.api` writes outside the transaction the write ran + * in, and an in-engine audit hook depends on landing inside it. + * + * Three ordinary paths open such a unit around the dispatch: + * - a by-id {@link ObjectQL.delete} whose cascade is `'atomic'` — each + * dependent's own `afterDelete` fires inside the wrap the parent opened, + * and the parent's row removal can still refuse afterwards (#7413). The + * PARENT's `afterDelete` is outside that wrap by construction, so it is + * unaffected; the cascaded CHILDREN's are not; + * - `runAtomicBatch` in `@objectstack/metadata-protocol` — + * `batchData`/`deleteManyData` with `atomic: true` runs every member's + * `after*` inside one transaction that aborts on the first failure + * (#4620); + * - any caller that opened `transaction()` / `ctx.api.transaction()` around + * the write itself. + * + * A rollback on any of those leaves a hook that fired for a row that still + * exists. Effects routed back through this engine roll back with it and are + * therefore safe; effects that leave the engine — webhooks, notifications, + * external index updates, file deletion — are the HANDLER's responsibility to + * make rollback-tolerant (idempotent and reconcilable, or re-checked against + * the row by a worker rather than trusted from the event alone). Documented + * for authors on `HookEvent` in `@objectstack/spec/data` and in + * `content/docs/automation/hooks.mdx`. */ const DISPATCHABLE_HOOK_EVENTS: ReadonlySet = new Set([ 'beforeFind', 'afterFind', @@ -895,6 +928,21 @@ function hydrateWriteFormulas( applyFormulaPlan(plan, records, execCtx); } +/** + * A hook body, as registered through {@link ObjectQL.registerHook} or bound + * from metadata by `bindHooksToEngine`. + * + * ## `after*` handlers run INSIDE the unit of work (#7477) + * + * An `afterInsert` / `afterUpdate` / `afterDelete` handler is dispatched + * before the enclosing transaction commits. The guarantee it may rely on is + * **"the write has been requested and will happen unless this unit of work is + * undone"** — not "the write happened": a later refusal in the same unit rolls + * the row back after this handler has already run. See + * {@link DISPATCHABLE_HOOK_EVENTS} for the full statement and for the paths + * that open such a unit; a handler whose side effects leave the engine is the + * one that has to tolerate it. + */ export type HookHandler = (context: HookContext) => Promise | void; /** @@ -1935,6 +1983,17 @@ export class ObjectQL implements IObjectQLEngine { return (this as any)._hookMetricsRecorder; } + /** + * Dispatch `event` to every registered handler that covers `context.object`, + * in priority order, awaiting each in turn. + * + * ⚠️ This runs wherever the caller calls it — it does NOT wait for a commit. + * An `after*` dispatch made from inside an open transaction therefore fires + * for a write that can still be rolled back; that is the declared semantics + * (#7477), stated in full on {@link DISPATCHABLE_HOOK_EVENTS}. Anything + * added here that defers a dispatch past the enclosing unit of work would be + * changing that ruling, not implementing it. + */ public async triggerHooks(event: string, context: HookContext) { const entries = this.hooks.get(event) || []; diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index fcf9fcdb19..db51ace607 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -68,6 +68,41 @@ const hookTargetError = + "`object: 'account'` or `object: ['account', 'contact']` — or, if firing on " + "every object really is the intent, write the wildcard explicitly: `object: '*'`."; +/** + * The lifecycle events a hook can subscribe to. + * + * ## `after*` fires INSIDE the unit of work, not after it commits (#7477) + * + * `afterInsert` / `afterUpdate` / `afterDelete` mean **"the write has been + * requested and will happen unless this unit of work is undone"** — NOT "the + * write happened". They are dispatched before the enclosing transaction (if + * there is one) commits, so a later refusal can roll the row back after the + * hook has already run. Ruled on #7477 (2026-08-11) as the declared semantics, + * not an implementation detail to be re-timed later. + * + * Three ordinary ways a write ends up inside such a unit: + * - a by-id `delete()` whose cascade is atomic — each dependent's own + * `afterDelete` runs inside the wrap the parent opened (#7413); + * - a `batchData`/`deleteManyData` call with `atomic: true` — every member's + * `after*` runs inside one transaction that aborts on the first failure + * (#4620); + * - any caller that opened `engine.transaction()` / `ctx.api.transaction()` + * around the write itself. + * + * What that means for a handler: + * - **Effects through the same engine are safe.** `ctx.api` / `ctx.ql` writes + * join the same transaction and roll back with everything else — which is + * exactly what makes an in-engine audit hook correct. + * - **Effects OUTSIDE the engine are the hook's own responsibility to make + * rollback-tolerant** — webhooks, notifications, external index updates, + * file deletion, email. On a rollback the row survives and the + * announcement has already gone out. Make such an effect idempotent and + * reconcilable, or enqueue it for a worker that re-reads the row before + * acting rather than trusting the event alone. + * + * The `before*` events carry no such caveat: they run before the write is + * issued, and throwing from one refuses the operation. + */ export const HookEvent = z.enum([ // Read — one event per read, regardless of shape. `beforeFind`/`afterFind` // fire for BOTH `find` and `findOne` (the event attaches to record @@ -847,6 +882,11 @@ export type Hook = z.input; /** Post-parse shape of {@link Hook} — defaults applied, transforms run (ADR-0122). */ export type HookParsed = z.infer; export type ResolvedHook = z.output; +/** + * One lifecycle event name. See {@link HookEvent} for the timing each one + * carries — in particular that `after*` fires INSIDE the unit of work, before + * the enclosing transaction commits (#7477). + */ export type HookEventType = z.input; export type HookContext = z.input; /**