From 1c14764a082233dd90996a548a64c8a6218918b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 07:52:19 +0000 Subject: [PATCH 1/9] fix(objectql): an aborting after* hook rolls its write back A plain write opened no unit of work, so an after* hook whose onError is abort (the default) refused the caller while the row stayed stored. The write door now opens a unit for exactly that case. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- .../src/engine-after-hook-abort-unit.test.ts | 167 ++++++++++++++++++ packages/objectql/src/engine.ts | 79 ++++++++- .../hook-after-abort-rollback.dogfood.test.ts | 105 +++++++++++ 3 files changed, 347 insertions(+), 4 deletions(-) create mode 100644 packages/objectql/src/engine-after-hook-abort-unit.test.ts create mode 100644 packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts diff --git a/packages/objectql/src/engine-after-hook-abort-unit.test.ts b/packages/objectql/src/engine-after-hook-abort-unit.test.ts new file mode 100644 index 00000000000..362fca6803a --- /dev/null +++ b/packages/objectql/src/engine-after-hook-abort-unit.test.ts @@ -0,0 +1,167 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #22782 — an `after*` hook whose `onError` is `abort` (the default) rolls its + * write back, as `HookSchema.onError` says ("abort: Rollback transaction"). + * + * Before this, a plain write opened no unit of work, so a throwing + * `afterInsert` refused the caller while the row, and the `ctx.api` write an + * earlier hook made, stayed stored. The write door now opens a unit for exactly + * that case (`inAfterAbortUnit`); every other write opens none, which the + * `begun` ledger below measures. + * + * The driver rolls back for real (a snapshot per transaction): a stub whose + * `rollback()` is a no-op cannot tell "wrapped" from "not wrapped". + */ + +import { describe, it, expect } from 'vitest'; +import type { Hook } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; +import { bindHooksToEngine } from './hook-binder.js'; + +function makeDriver(name: string, opts: { transactional?: boolean; failParentUpdate?: boolean } = {}) { + let stores = new Map>>(); + const storeFor = (o: string) => stores.get(o) ?? (stores.set(o, new Map()), stores.get(o)!); + const matches = (row: Record, where: any) => + !where || Object.entries(where).every(([k, v]) => k.startsWith('$') || row[k] === v); + const begun: unknown[] = []; + const rolledBack: unknown[] = []; + const snapshots = new Map(); + let n = 0; + const driver: any = { + name, version: '0.0.0', supports: {}, begun, rolledBack, + rowsOf: (o: string) => Array.from(storeFor(o).values()), + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async syncSchema() {}, + async find(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); }, + async findOne(o: string, ast: any) { return (await this.find(o, ast))[0] ?? null; }, + async create(o: string, data: Record) { + const row = { ...data, id: (data.id as string) ?? `${o}_${++n}` }; + storeFor(o).set(row.id, row); + return row; + }, + async update(o: string, id: string, data: Record) { + if (opts.failParentUpdate && o === 'inv') throw new Error('ETIMEDOUT'); + const row = { ...storeFor(o).get(String(id)), ...data, id }; + storeFor(o).set(String(id), row); + return row; + }, + async delete(o: string, id: string) { return storeFor(o).delete(String(id)); }, + async count(o: string, ast: any) { return (await this.find(o, ast)).length; }, + async bulkCreate(o: string, rows: Record[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + }; + if (opts.transactional !== false) { + driver.beginTransaction = async () => { + const handle = { trx: begun.length + 1 }; + snapshots.set(handle, new Map(Array.from(stores, ([o, s]) => [o, new Map(Array.from(s, ([k, v]) => [k, { ...v }]))]))); + begun.push(handle); + return handle; + }; + driver.commit = async (h: unknown) => { snapshots.delete(h); }; + driver.rollback = async (h: unknown) => { stores = snapshots.get(h) ?? stores; rolledBack.push(h); }; + } + return driver; +} + +const obj = (name: string, extra: Record = {}) => ({ + name, label: name, fields: { id: { name: 'id', type: 'text', primaryKey: true }, name: { name: 'name', type: 'text' }, ...extra }, +}); +const hook = (name: string, events: string[], handler: (ctx: any) => unknown, extra: Partial = {}) => + ({ name, object: 'case', events, handler, ...extra }) as unknown as Hook; +const boom = () => { throw new Error('refused by the hook'); }; +/** The earlier `afterInsert` the card measured: it writes a note through `ctx.api` first. */ +const noteWriter = hook('note_writer', ['afterInsert'], (ctx) => ctx.api.object('note').insert({ name: 'from-hook' }), { priority: 10 }); + +async function makeEngine(hooks: Hook[], opts: { transactional?: boolean; caseDriver?: string; failParentUpdate?: boolean } = {}) { + const warns: string[] = []; + const log = () => {}; + const engine = new ObjectQL({ logger: { debug: log, info: log, error: log, warn: (m: string) => void warns.push(String(m)) } } as any); + const driver = makeDriver('primary', opts); + engine.registerDriver(driver, true); + const side = makeDriver('side'); + engine.registerDriver(side); + await engine.init(); + engine.registry.registerObject({ ...obj('case'), ...(opts.caseDriver ? { datasource: opts.caseDriver } : {}) } as any, '__test__'); + engine.registry.registerObject(obj('note') as any, '__test__'); + bindHooksToEngine(engine, hooks, { packageId: 'app:t' }); + return { engine, driver, side, warns }; +} + +describe('#22782 — an aborting after* hook rolls its write back', () => { + it('a throwing afterInsert with the default onError: refused, the row and the chain\'s ctx.api write absent', async () => { + const { engine, driver } = await makeEngine([noteWriter, hook('veto', ['afterInsert'], boom, { priority: 20 })]); + await expect(engine.insert('case', { name: 'c1' })).rejects.toThrow('refused by the hook'); + expect(driver.rowsOf('case')).toEqual([]); + expect(driver.rowsOf('note')).toEqual([]); + expect(driver.begun).toHaveLength(1); + expect(driver.rolledBack).toHaveLength(1); + }); + + it('afterUpdate and afterDelete: the stored row is what it was before the refused write', async () => { + const { engine, driver } = await makeEngine([hook('veto', ['afterUpdate', 'afterDelete'], boom)]); + const row = await engine.insert('case', { name: 'before' }); + await expect(engine.update('case', { name: 'after' }, { where: { id: row.id } } as any)).rejects.toThrow('refused by the hook'); + await expect(engine.delete('case', { where: { id: row.id } } as any)).rejects.toThrow('refused by the hook'); + expect(driver.rowsOf('case')).toMatchObject([{ id: row.id, name: 'before' }]); + expect(driver.begun).toHaveLength(2); + }); + + it('CONTROL onError: \'log\' — the row is stored and no unit is opened', async () => { + const { engine, driver } = await makeEngine([hook('veto', ['afterInsert'], boom, { onError: 'log' })]); + await engine.insert('case', { name: 'c1' }); + expect(driver.rowsOf('case')).toMatchObject([{ name: 'c1' }]); + expect(driver.begun).toHaveLength(0); + }); + + it('CONTROL a throwing before* hook — refused, nothing stored, no unit opened', async () => { + const { engine, driver } = await makeEngine([hook('veto', ['beforeInsert'], boom)]); + await expect(engine.insert('case', { name: 'c1' })).rejects.toThrow('refused by the hook'); + expect(driver.rowsOf('case')).toEqual([]); + expect(driver.begun).toHaveLength(0); + }); + + it('CONTROL inside a caller-opened transaction() — it joins (one unit), and the caller decides', async () => { + const { engine, driver } = await makeEngine([hook('veto', ['afterInsert'], boom)]); + await engine.transaction(async (trx: any) => { + await expect(engine.insert('case', { name: 'kept' }, { context: trx })).rejects.toThrow('refused by the hook'); + }); + expect(driver.begun).toHaveLength(1); + expect(driver.rolledBack).toHaveLength(0); + expect(driver.rowsOf('case')).toMatchObject([{ name: 'kept' }]); + }); + + it('no unit for a hook that cannot abort the write: async, code-registered, or skipped automations', async () => { + const { engine, driver } = await makeEngine([hook('fire_and_forget', ['afterInsert'], () => {}, { async: true })]); + engine.registerHook('afterInsert', async () => {}, { object: 'case' }); + await engine.insert('case', { name: 'c1' }); + const vetoed = await makeEngine([hook('veto', ['afterInsert'], boom)]); + await vetoed.engine.insert('case', { name: 'c2' }, { context: { isSystem: true, skipAutomations: true } as any }); + expect(driver.begun).toHaveLength(0); + expect(vetoed.driver.begun).toHaveLength(0); + }); + + it('no unit where one cannot hold — off the default driver, or a driver with no transactions — and no warning', async () => { + const off = await makeEngine([hook('veto', ['afterInsert'], boom)], { caseDriver: 'side' }); + await expect(off.engine.insert('case', { name: 'c1' })).rejects.toThrow('refused by the hook'); + expect(off.side.rowsOf('case')).toMatchObject([{ name: 'c1' }]); + const plain = await makeEngine([hook('veto', ['afterInsert'], boom)], { transactional: false }); + await expect(plain.engine.insert('case', { name: 'c2' })).rejects.toThrow('refused by the hook'); + expect(plain.driver.rowsOf('case')).toMatchObject([{ name: 'c2' }]); + expect([...off.driver.begun, ...off.side.begun]).toHaveLength(0); + expect(plain.warns.filter((w) => w.includes('transaction'))).toEqual([]); + }); + + it('a roll-up failure inside the unit still means "the records WERE written": committed, then thrown', async () => { + const { engine, driver } = await makeEngine([], { failParentUpdate: true }); + (engine as any).summaryRetryOptions = { sleep: async () => {}, backoffBaseMs: 0 }; + engine.registry.registerObject({ name: 'inv', fields: { total: { type: 'summary', summaryOperations: { object: 'inv_line', field: 'amount', function: 'sum' } } } } as any); + engine.registry.registerObject({ name: 'inv_line', fields: { amount: { type: 'number' }, inv: { type: 'master_detail', reference: 'inv' } } } as any); + bindHooksToEngine(engine, [{ name: 'observe', object: 'inv_line', events: ['afterInsert'], handler: () => {} } as unknown as Hook], { packageId: 'app:s' }); + const parent = await engine.insert('inv', {}); + await expect(engine.insert('inv_line', { inv: parent.id, amount: 1 })).rejects.toMatchObject({ code: 'ERR_SUMMARY_RECOMPUTE' }); + expect(driver.begun).toHaveLength(1); + expect(driver.rolledBack).toHaveLength(0); + expect(driver.rowsOf('inv_line')).toHaveLength(1); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 9905baf9fb8..c23ff7d2f2d 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -400,12 +400,16 @@ export interface AdmittedValueShapeViolationTally { * 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: + * Four ordinary paths open such a unit around the dispatch: + * - the write door itself, when the object has a blocking metadata-bound + * `after*` hook whose `onError` is `abort` — so that abort rolls the row + * back as `HookSchema.onError` says (#22782, `inAfterAbortUnit`); * - 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; + * PARENT's `afterDelete` is outside that wrap by construction (inside the + * door's own unit when the first path opened one); 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 @@ -4677,6 +4681,58 @@ export class ObjectQL implements IObjectQLEngine { return entries.some((entry) => hookMatchesObject(entry, object)); } + /** + * [#22782] Run `write` inside a unit of work when, and only when, its + * `after*` dispatch can ABORT and no unit is open yet. + * + * `HookSchema.onError` says `abort` rolls the transaction back. The three + * paths {@link DISPATCHABLE_HOOK_EVENTS} names open one; a plain write opened + * none, so a throwing `afterInsert` refused the caller while the row, and + * every `ctx.api` write the chain made before it, stayed stored. The unit is + * opened for exactly the case that needs it: a metadata-bound hook (`meta`, + * the only entries carrying a declared `onError`) on `event` for `object` + * that is blocking (not `async`) and whose `onError` is `abort` (the + * default), with automations not skipped. Hook count and order are untouched + * (#7413); only the boundary around them moves. + * + * No unit — today's path — when one is already open (the ambient store or an + * explicitly threaded handle: the caller's unit is the one that rolls back), + * when `object` is not on the default driver (`transaction()` covers that + * driver alone, so the object's own write would be refused), or when that + * driver declares no transactions (ADR-0119 D1; asking would only warn). + * + * ⚠️ `SummaryRecomputeError` is the one failure that says "the records WERE + * written", so it is rethrown AFTER the commit, never through the rollback. + */ + private async inAfterAbortUnit( + event: 'afterInsert' | 'afterUpdate' | 'afterDelete', + object: string, + context: ExecutionContext | undefined, + write: () => Promise, + ): Promise { + if (context?.transaction !== undefined || this.txStore.getStore()?.transaction !== undefined) return write(); + if ((context as { skipAutomations?: unknown } | undefined)?.skipAutomations) return write(); + const aborting = (this.hooks.get(event) ?? []).some((entry) => + entry.meta && !entry.meta.async && (entry.meta.onError ?? 'abort') === 'abort' && hookMatchesObject(entry, object)); + if (!aborting) return write(); + const defaultDriver = this.defaultDriver ? this.drivers.get(this.defaultDriver) : undefined; + let onDefault = false; + try { onDefault = this.getDriver(object) === defaultDriver; } catch { /* the write itself reports it */ } + if (!onDefault || !driverSupportsTransactions(defaultDriver)) return write(); + let written: SummaryRecomputeError | undefined; + const result = await this.transaction(async () => { + try { + return await write(); + } catch (e) { + if (!(e instanceof SummaryRecomputeError)) throw e; + written = e; + return undefined as T; + } + }); + if (written) throw written; + return result; + } + /** * [#5038] The per-row after-hook contexts a predicate (`multi: true`) write * dispatches, one per matched row. @@ -13990,6 +14046,11 @@ export class ObjectQL implements IObjectQLEngine { */ async insert(object: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise { object = this.resolveObjectName(object); + // [#22782] An aborting `afterInsert` rolls this write back — see `inAfterAbortUnit`. + return this.inAfterAbortUnit('afterInsert', object, options?.context, () => this.insertInUnit(object, data, options)); + } + + private async insertInUnit(object: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise { this.logger.debug('Insert operation starting', { object, isBatch: Array.isArray(data) }); this.assertWriteAllowed(object, 'insert'); // [#8844] The organization set is about to change. Dropped BEFORE the write @@ -15164,7 +15225,12 @@ export class ObjectQL implements IObjectQLEngine { * a violation raised by a nested driver call in there is not this object's. */ async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null> { - object = this.resolveObjectName(object); + object = this.resolveObjectName(object); + // [#22782] An aborting `afterUpdate` rolls this write back — see `inAfterAbortUnit`. + return this.inAfterAbortUnit('afterUpdate', object, options?.context, () => this.updateInUnit(object, data, options)); + } + + private async updateInUnit(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null> { this.logger.debug('Update operation starting', { object }); this.assertWriteAllowed(object, 'update'); // [#8844] See the insert face — an organization row that changes identity @@ -18144,6 +18210,11 @@ export class ObjectQL implements IObjectQLEngine { async delete(object: string, options?: EngineDeleteOptions): Promise { object = this.resolveObjectName(object); + // [#22782] An aborting `afterDelete` rolls this write back — see `inAfterAbortUnit`. + return this.inAfterAbortUnit('afterDelete', object, options?.context, () => this.deleteInUnit(object, options)); + } + + private async deleteInUnit(object: string, options?: EngineDeleteOptions): Promise { this.logger.debug('Delete operation starting', { object }); this.assertWriteAllowed(object, 'delete'); // [#8844] See the insert face — deleting the install's only organization diff --git a/packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts b/packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts new file mode 100644 index 00000000000..bb907d237b8 --- /dev/null +++ b/packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts @@ -0,0 +1,105 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #22782 — `HookSchema.onError`: "abort: Rollback transaction (if blocking)", +// default `abort`. Measured on a booted stack before the fix: an `afterInsert` +// hook that threw refused the caller while the row stayed stored, and so did +// the row an earlier `afterInsert` hook wrote through `ctx.api` — a plain write +// opened no unit of work, so the abort had nothing to roll back. A client that +// retries the refused create makes a duplicate. +// +// Pinned on the two published doors the card names: REST `POST /data/:object` +// (`createData`) and the engine door (`hooks.run`), each read back through the +// engine. The three controls are the triage's: `onError: 'log'` stores the +// row; a caller-opened `transaction()` stays the caller's to decide; a throwing +// `before*` hook refuses exactly as before. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; + +const CASE = 'har_case'; +const NOTE = 'har_note'; +const LOGGED = 'har_logged'; + +const named = (name: string) => ObjectSchema.create({ + name, label: name, sharingModel: 'public_read_write', + fields: { name: Field.text({ label: 'Name', required: true }), case_name: Field.text({ label: 'Case' }) }, +}); +const veto = (prefix: string) => (ctx: any) => { + if (String(ctx.input?.data?.name ?? '').startsWith(prefix)) throw new Error(`refused: ${prefix}`); +}; + +const harStack = defineStack({ + manifest: { + id: 'com.dogfood.hook-after-abort-rollback', namespace: 'har', version: '0.0.0', type: 'app', + name: 'After-hook abort rollback fixture', description: 'An afterInsert abort, its log twin, and a beforeInsert veto.', + }, + objects: [named(CASE), named(NOTE), named(LOGGED)], + hooks: [ + // The earlier hook of the card's measurement: it writes through ctx.api first. + { name: 'har_note_writer', object: CASE, events: ['afterInsert'], priority: 10, + handler: (ctx: any) => ctx.api.object(NOTE).insert({ name: 'note', case_name: ctx.input.data.name }) }, + { name: 'har_after_veto', object: CASE, events: ['afterInsert'], priority: 20, handler: veto('veto') }, + { name: 'har_before_veto', object: CASE, events: ['beforeInsert'], handler: veto('block') }, + { name: 'har_logged_veto', object: LOGGED, events: ['afterInsert'], onError: 'log', handler: veto('veto') }, + ], +} as never); + +describe('an aborting afterInsert rolls the write back (#22782)', () => { + let stack: VerifyStack; + let token: string; + const uniq = (prefix: string) => `${prefix}-${Math.random().toString(36).slice(2, 8)}`; + const stored = async (object: string, name: string) => (await stack.rows(object, { name })).length; + const notes = async (name: string) => (await stack.rows(NOTE, { case_name: name })).length; + + beforeAll(async () => { + stack = await bootStack(harStack); + token = await stack.signIn(); + }, 120_000); + afterAll(async () => { await stack?.stop(); }); + + it('REST createData: the refused create leaves neither the row nor the hook chain\'s ctx.api write', async () => { + const name = uniq('veto'); + const res = await stack.apiAs(token, 'POST', `/data/${CASE}`, { name }); + expect(res.status, await res.clone().text()).toBe(400); + expect((await res.json() as any).error).toBe('refused: veto'); + expect(await stored(CASE, name)).toBe(0); + expect(await notes(name)).toBe(0); + }); + + it('the engine door (hooks.run): refused, and nothing stored', async () => { + const name = uniq('veto'); + await expect(stack.hooks.run(CASE, 'insert', { name }, { system: true })).rejects.toThrow('refused: veto'); + expect(await stored(CASE, name)).toBe(0); + expect(await notes(name)).toBe(0); + }); + + it('positive control: an insert no hook refuses stores the row and the note', async () => { + const name = uniq('ok'); + expect((await stack.apiAs(token, 'POST', `/data/${CASE}`, { name })).status).toBe(201); + expect([await stored(CASE, name), await notes(name)]).toEqual([1, 1]); + }); + + it('CONTROL onError: \'log\' — the hook throws and the row is stored', async () => { + const name = uniq('veto'); + expect((await stack.apiAs(token, 'POST', `/data/${LOGGED}`, { name })).status).toBe(201); + expect(await stored(LOGGED, name)).toBe(1); + }); + + it('CONTROL a caller-opened transaction(): the caller that swallows the refusal and commits keeps the row', async () => { + const name = uniq('veto'); + const ql = await stack.kernel.getServiceAsync('objectql'); + await ql.transaction(async (trx: any) => { + await expect(ql.insert(CASE, { name }, { context: { ...trx, isSystem: true } })).rejects.toThrow('refused: veto'); + }); + expect([await stored(CASE, name), await notes(name)]).toEqual([1, 1]); + }); + + it('CONTROL a throwing beforeInsert: refused, nothing stored', async () => { + const name = uniq('block'); + const res = await stack.apiAs(token, 'POST', `/data/${CASE}`, { name }); + expect(res.status).toBe(400); + expect(await stored(CASE, name)).toBe(0); + }); +}); From c84dd8c955f0799653cb8b82742871af71ae65c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 07:57:10 +0000 Subject: [PATCH 2/9] fix(objectql): a unit's ambient handle closes when its callback settles Work a unit starts and does not await (an async: true hook) inherited the finished handle and lost its write. Measured on a booted stack. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- packages/objectql/src/engine.ts | 13 ++++++--- .../hook-after-abort-rollback.dogfood.test.ts | 27 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index c23ff7d2f2d..c93d867a356 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -19522,13 +19522,18 @@ export class ObjectQL implements IObjectQLEngine { } const trx = await drv.beginTransaction(); const trxCtx = { ...(baseContext ?? {}), transaction: trx }; + const ambientEntry: { transaction: unknown; scope?: TransactionScope } = + { transaction: trx, scope: this.newTransactionScope(driver!) }; try { // Run the callback inside the ambient transaction store so internal // queries during writes reuse this transaction's connection (ADR-0034). - const result = await this.txStore.run( - { transaction: trx, scope: this.newTransactionScope(driver!) }, - () => callback(trxCtx, { owned: true }), - ); + // [#22782] The entry is CLOSED once the callback settles, before the + // commit: work it started and did not await (an `async: true` hook) + // inherits this store, and finding the handle there it ran on a finished + // transaction ("Transaction query already complete") and was lost. Closed, + // it runs on its own connection, as it would outside any unit. + const result = await this.txStore.run(ambientEntry, () => callback(trxCtx, { owned: true })) + .finally(() => { ambientEntry.transaction = undefined; ambientEntry.scope = undefined; }); if (drv.commit) await drv.commit(trx); else if (drv.commitTransaction) await drv.commitTransaction(trx); return result; diff --git a/packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts b/packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts index bb907d237b8..22991429189 100644 --- a/packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts +++ b/packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts @@ -11,7 +11,13 @@ // (`createData`) and the engine door (`hooks.run`), each read back through the // engine. The three controls are the triage's: `onError: 'log'` stores the // row; a caller-opened `transaction()` stays the caller's to decide; a throwing -// `before*` hook refuses exactly as before. +// `before*` hook refuses exactly as before. A plain `Error` from a function +// hook answers REST's sanitised `500 INTERNAL_ERROR` (its terminal branch) on +// both sides of the fix; what this file pins is what is STORED. +// +// One more case the unit made necessary: an `async: true` hook is started +// inside the unit and writes after it commits. It used to inherit the finished +// handle and lose its write ("Transaction query already complete", measured). import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { bootStack, type VerifyStack } from '@objectstack/verify'; @@ -43,6 +49,11 @@ const harStack = defineStack({ { name: 'har_after_veto', object: CASE, events: ['afterInsert'], priority: 20, handler: veto('veto') }, { name: 'har_before_veto', object: CASE, events: ['beforeInsert'], handler: veto('block') }, { name: 'har_logged_veto', object: LOGGED, events: ['afterInsert'], onError: 'log', handler: veto('veto') }, + { name: 'har_late_note', object: CASE, events: ['afterInsert'], async: true, onError: 'log', + handler: async (ctx: any) => { + await new Promise((r) => setTimeout(r, 40)); + await ctx.api.object(NOTE).insert({ name: 'late', case_name: ctx.input.data.name }); + } }, ], } as never); @@ -51,7 +62,8 @@ describe('an aborting afterInsert rolls the write back (#22782)', () => { let token: string; const uniq = (prefix: string) => `${prefix}-${Math.random().toString(36).slice(2, 8)}`; const stored = async (object: string, name: string) => (await stack.rows(object, { name })).length; - const notes = async (name: string) => (await stack.rows(NOTE, { case_name: name })).length; + const notes = async (name: string, note = 'note') => (await stack.rows(NOTE, { case_name: name, name: note })).length; + const refusal = async (res: Response) => ({ status: res.status, code: ((await res.json()) as any).code }); beforeAll(async () => { stack = await bootStack(harStack); @@ -61,9 +73,7 @@ describe('an aborting afterInsert rolls the write back (#22782)', () => { it('REST createData: the refused create leaves neither the row nor the hook chain\'s ctx.api write', async () => { const name = uniq('veto'); - const res = await stack.apiAs(token, 'POST', `/data/${CASE}`, { name }); - expect(res.status, await res.clone().text()).toBe(400); - expect((await res.json() as any).error).toBe('refused: veto'); + expect(await refusal(await stack.apiAs(token, 'POST', `/data/${CASE}`, { name }))).toEqual({ status: 500, code: 'INTERNAL_ERROR' }); expect(await stored(CASE, name)).toBe(0); expect(await notes(name)).toBe(0); }); @@ -75,10 +85,12 @@ describe('an aborting afterInsert rolls the write back (#22782)', () => { expect(await notes(name)).toBe(0); }); - it('positive control: an insert no hook refuses stores the row and the note', async () => { + it('positive control: an insert no hook refuses stores the row, the note, and the async hook\'s late note', async () => { const name = uniq('ok'); expect((await stack.apiAs(token, 'POST', `/data/${CASE}`, { name })).status).toBe(201); expect([await stored(CASE, name), await notes(name)]).toEqual([1, 1]); + for (let i = 0; i < 40 && (await notes(name, 'late')) === 0; i++) await new Promise((r) => setTimeout(r, 50)); + expect(await notes(name, 'late')).toBe(1); }); it('CONTROL onError: \'log\' — the hook throws and the row is stored', async () => { @@ -98,8 +110,7 @@ describe('an aborting afterInsert rolls the write back (#22782)', () => { it('CONTROL a throwing beforeInsert: refused, nothing stored', async () => { const name = uniq('block'); - const res = await stack.apiAs(token, 'POST', `/data/${CASE}`, { name }); - expect(res.status).toBe(400); + expect(await refusal(await stack.apiAs(token, 'POST', `/data/${CASE}`, { name }))).toEqual({ status: 500, code: 'INTERNAL_ERROR' }); expect(await stored(CASE, name)).toBe(0); }); }); From 31b78915dc8331e58a96407eb59cb1899c9f3b2d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 08:10:54 +0000 Subject: [PATCH 3/9] docs(objectql): changeset and hooks page for the after-hook abort unit Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- ...82-objectql-after-hook-abort-rolls-back.md | 18 ++++++++ content/docs/automation/hooks.mdx | 5 ++- packages/objectql/src/engine.ts | 43 ++++++++----------- 3 files changed, 38 insertions(+), 28 deletions(-) create mode 100644 .changeset/22782-objectql-after-hook-abort-rolls-back.md diff --git a/.changeset/22782-objectql-after-hook-abort-rolls-back.md b/.changeset/22782-objectql-after-hook-abort-rolls-back.md new file mode 100644 index 00000000000..8856832ea70 --- /dev/null +++ b/.changeset/22782-objectql-after-hook-abort-rolls-back.md @@ -0,0 +1,18 @@ +--- +'@objectstack/objectql': minor +--- + +An `after*` hook whose `onError` is `abort` (the default) now rolls its write back on the default write door, as `HookSchema.onError` says ("abort: Rollback transaction (if blocking)") + +Clause-②: yes (narrowing) + + + +**BREAKING** stored-outcome narrowing, shipped as `minor` under the repo's launch-window convention for breaking changes (ADR-0131 D3, D9). + +- **FROM:** `insert` / `update` / `delete` (REST `createData` / `updateData` / `deleteData`, `ctx.api`, the engine door) on an object with a blocking (not `async`) `after*` hook bound from metadata whose `onError` is `abort` or omitted. When that hook threw, the caller was refused, but the row stayed stored, changed or deleted, and so did every write the hook chain had already made through `ctx.api`. A client that retried the refused create made a duplicate. +- **TO:** that write runs inside a unit of work, so the abort rolls back the row and the hook chain's engine writes together. The caller is refused exactly as before. Hook count and order are unchanged. +- **Unchanged:** a write already inside a unit (a caller's `transaction()`, an atomic batch or cascade) joins it, and that unit's owner still decides. No unit is opened for an `onError: 'log'` hook, an `async: true` hook, a hook registered in code with `registerHook` (no declared `onError`), a write with `skipAutomations`, an object whose datasource is not the default one, or a default driver with no transactions (ADR-0119 D1, such as a libSQL remote transport). On those the row stays stored after an abort, as before. +- **Inside the new unit, the rules of any unit apply.** An `after*` hook on such an object that writes a business row to an object on another datasource is now refused with `CrossDatasourceTransactionWriteError`, which the abort turns into a refused write. A MongoDB default store needs a replica set for this unit, as it does for `transaction()` already. If either applies to you, mark the hook `onError: 'log'` or `async: true`, or keep the objects on one datasource. +- **Work a unit starts and does not await** (an `async: true` hook) no longer inherits the unit's finished transaction handle. It used to fail with "Transaction query already complete" and lose its write. It now runs on its own connection, as it would outside any unit. +- **Cost:** one transaction per write on the objects above only. Measured on the in-process SQLite driver: about 1.36x the per-insert time of the same object with an `onError: 'log'` hook. Objects without such a hook pay one map lookup. diff --git a/content/docs/automation/hooks.mdx b/content/docs/automation/hooks.mdx index 3169f6201a2..c83fa539b83 100644 --- a/content/docs/automation/hooks.mdx +++ b/content/docs/automation/hooks.mdx @@ -292,11 +292,12 @@ has been requested and will happen unless this unit of work is undone**. 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: +Four 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 | +| Any write to an object with a blocking `after*` hook whose `onError` is `abort` (the default) | The write and every `after*` hook it dispatches, so that hook's abort rolls the row back. Not opened on an object outside the default datasource, or on a driver with no transactions: there the row stays stored after an abort | +| A by-id `delete()` that cascades to dependent records | Each **cascaded child's** `afterDelete`. The parent's own `afterDelete` runs after that unit closes, unless the row above opened one around the whole delete | | `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 | diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index c93d867a356..4beedd90153 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -4682,27 +4682,20 @@ export class ObjectQL implements IObjectQLEngine { } /** - * [#22782] Run `write` inside a unit of work when, and only when, its - * `after*` dispatch can ABORT and no unit is open yet. - * - * `HookSchema.onError` says `abort` rolls the transaction back. The three - * paths {@link DISPATCHABLE_HOOK_EVENTS} names open one; a plain write opened - * none, so a throwing `afterInsert` refused the caller while the row, and - * every `ctx.api` write the chain made before it, stayed stored. The unit is - * opened for exactly the case that needs it: a metadata-bound hook (`meta`, - * the only entries carrying a declared `onError`) on `event` for `object` - * that is blocking (not `async`) and whose `onError` is `abort` (the - * default), with automations not skipped. Hook count and order are untouched - * (#7413); only the boundary around them moves. - * - * No unit — today's path — when one is already open (the ambient store or an - * explicitly threaded handle: the caller's unit is the one that rolls back), - * when `object` is not on the default driver (`transaction()` covers that - * driver alone, so the object's own write would be refused), or when that - * driver declares no transactions (ADR-0119 D1; asking would only warn). - * - * ⚠️ `SummaryRecomputeError` is the one failure that says "the records WERE - * written", so it is rethrown AFTER the commit, never through the rollback. + * [#22782] Run `write` inside a unit of work when its `after*` dispatch can + * ABORT and no unit is open, so the abort rolls back what `HookSchema.onError` + * says it does: the row and every `ctx.api` write the chain made. "Can abort" + * is a metadata-bound hook (`meta` — the only entries with a declared + * `onError`) on `event` for `object`, blocking (not `async`), `onError` + * `abort` (the default), automations not skipped. Hook count and order are + * untouched (#7413); only the boundary around them moves. + * + * No unit — the pre-#22782 path — when one is open (ambient or threaded: the + * caller's unit decides), when `object` is off the default driver + * (`transaction()` covers that one alone and would refuse the write), or when + * that driver has no transactions (ADR-0119 D1; asking would only warn). + * `SummaryRecomputeError` means "the records WERE written": it is rethrown + * after the commit, never through the rollback. */ private async inAfterAbortUnit( event: 'afterInsert' | 'afterUpdate' | 'afterDelete', @@ -19527,11 +19520,9 @@ export class ObjectQL implements IObjectQLEngine { try { // Run the callback inside the ambient transaction store so internal // queries during writes reuse this transaction's connection (ADR-0034). - // [#22782] The entry is CLOSED once the callback settles, before the - // commit: work it started and did not await (an `async: true` hook) - // inherits this store, and finding the handle there it ran on a finished - // transaction ("Transaction query already complete") and was lost. Closed, - // it runs on its own connection, as it would outside any unit. + // [#22782] CLOSED once the callback settles, before the commit: work it + // started and did not await (an `async: true` hook) inherits this store, + // ran on the finished handle and lost its write. Now it runs unenrolled. const result = await this.txStore.run(ambientEntry, () => callback(trxCtx, { owned: true })) .finally(() => { ambientEntry.transaction = undefined; ambientEntry.scope = undefined; }); if (drv.commit) await drv.commit(trx); From d572e29ec87516eda323f37d3019e1ba4a178b7c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 08:33:42 +0000 Subject: [PATCH 4/9] refactor(objectql): keep the write bodies in insert/update/delete; the door re-enters its own unit The system-context census anchors elevation reads by symbol, so the bodies keep their names: the door asks opensAfterAbortUnit, and the unit re-enters the same method, which joins it. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- packages/objectql/src/engine.ts | 62 ++++++++++++++++----------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 4beedd90153..1fef9c8fc2a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -403,7 +403,7 @@ export interface AdmittedValueShapeViolationTally { * Four ordinary paths open such a unit around the dispatch: * - the write door itself, when the object has a blocking metadata-bound * `after*` hook whose `onError` is `abort` — so that abort rolls the row - * back as `HookSchema.onError` says (#22782, `inAfterAbortUnit`); + * back as `HookSchema.onError` says (#22782, `opensAfterAbortUnit`); * - 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 @@ -4682,9 +4682,9 @@ export class ObjectQL implements IObjectQLEngine { } /** - * [#22782] Run `write` inside a unit of work when its `after*` dispatch can - * ABORT and no unit is open, so the abort rolls back what `HookSchema.onError` - * says it does: the row and every `ctx.api` write the chain made. "Can abort" + * [#22782] Does this write need its own unit of work? Yes when its `after*` + * dispatch can ABORT and no unit is open, so the abort rolls back what + * `HookSchema.onError` says: the row and every `ctx.api` write the chain made. "Can abort" * is a metadata-bound hook (`meta` — the only entries with a declared * `onError`) on `event` for `object`, blocking (not `async`), `onError` * `abort` (the default), automations not skipped. Hook count and order are @@ -4697,21 +4697,22 @@ export class ObjectQL implements IObjectQLEngine { * `SummaryRecomputeError` means "the records WERE written": it is rethrown * after the commit, never through the rollback. */ - private async inAfterAbortUnit( - event: 'afterInsert' | 'afterUpdate' | 'afterDelete', - object: string, - context: ExecutionContext | undefined, - write: () => Promise, - ): Promise { - if (context?.transaction !== undefined || this.txStore.getStore()?.transaction !== undefined) return write(); - if ((context as { skipAutomations?: unknown } | undefined)?.skipAutomations) return write(); + private opensAfterAbortUnit(event: 'afterInsert' | 'afterUpdate' | 'afterDelete', object: string, context: ExecutionContext | undefined): boolean { + if (context?.transaction !== undefined || this.txStore.getStore()?.transaction !== undefined) return false; + if ((context as { skipAutomations?: unknown } | undefined)?.skipAutomations) return false; const aborting = (this.hooks.get(event) ?? []).some((entry) => entry.meta && !entry.meta.async && (entry.meta.onError ?? 'abort') === 'abort' && hookMatchesObject(entry, object)); - if (!aborting) return write(); + if (!aborting) return false; const defaultDriver = this.defaultDriver ? this.drivers.get(this.defaultDriver) : undefined; - let onDefault = false; - try { onDefault = this.getDriver(object) === defaultDriver; } catch { /* the write itself reports it */ } - if (!onDefault || !driverSupportsTransactions(defaultDriver)) return write(); + try { + return this.getDriver(object) === defaultDriver && driverSupportsTransactions(defaultDriver); + } catch { + return false; // the write itself reports an unroutable object + } + } + + /** [#22782] The unit {@link opensAfterAbortUnit} asked for; `write` re-enters the door, which joins it. */ + private async inAfterAbortUnit(write: () => Promise): Promise { let written: SummaryRecomputeError | undefined; const result = await this.transaction(async () => { try { @@ -14039,11 +14040,10 @@ export class ObjectQL implements IObjectQLEngine { */ async insert(object: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise { object = this.resolveObjectName(object); - // [#22782] An aborting `afterInsert` rolls this write back — see `inAfterAbortUnit`. - return this.inAfterAbortUnit('afterInsert', object, options?.context, () => this.insertInUnit(object, data, options)); - } - - private async insertInUnit(object: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise { + // [#22782] An aborting `afterInsert` rolls this write back — see `opensAfterAbortUnit`. + if (this.opensAfterAbortUnit('afterInsert', object, options?.context)) { + return this.inAfterAbortUnit(() => this.insert(object, data, options)); + } this.logger.debug('Insert operation starting', { object, isBatch: Array.isArray(data) }); this.assertWriteAllowed(object, 'insert'); // [#8844] The organization set is about to change. Dropped BEFORE the write @@ -15218,12 +15218,11 @@ export class ObjectQL implements IObjectQLEngine { * a violation raised by a nested driver call in there is not this object's. */ async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null> { - object = this.resolveObjectName(object); - // [#22782] An aborting `afterUpdate` rolls this write back — see `inAfterAbortUnit`. - return this.inAfterAbortUnit('afterUpdate', object, options?.context, () => this.updateInUnit(object, data, options)); - } - - private async updateInUnit(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise | number | null> { + object = this.resolveObjectName(object); + // [#22782] An aborting `afterUpdate` rolls this write back — see `opensAfterAbortUnit`. + if (this.opensAfterAbortUnit('afterUpdate', object, options?.context)) { + return this.inAfterAbortUnit(() => this.update(object, data, options)); + } this.logger.debug('Update operation starting', { object }); this.assertWriteAllowed(object, 'update'); // [#8844] See the insert face — an organization row that changes identity @@ -18203,11 +18202,10 @@ export class ObjectQL implements IObjectQLEngine { async delete(object: string, options?: EngineDeleteOptions): Promise { object = this.resolveObjectName(object); - // [#22782] An aborting `afterDelete` rolls this write back — see `inAfterAbortUnit`. - return this.inAfterAbortUnit('afterDelete', object, options?.context, () => this.deleteInUnit(object, options)); - } - - private async deleteInUnit(object: string, options?: EngineDeleteOptions): Promise { + // [#22782] An aborting `afterDelete` rolls this write back — see `opensAfterAbortUnit`. + if (this.opensAfterAbortUnit('afterDelete', object, options?.context)) { + return this.inAfterAbortUnit(() => this.delete(object, options)); + } this.logger.debug('Delete operation starting', { object }); this.assertWriteAllowed(object, 'delete'); // [#8844] See the insert face — deleting the install's only organization From 1c880cd1cbb4ee5b824da21a48dcfa006237e139 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 09:17:45 +0000 Subject: [PATCH 5/9] test(objectql): the after-hook unit test's driver double honours the caller's limit Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- packages/objectql/src/engine-after-hook-abort-unit.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/objectql/src/engine-after-hook-abort-unit.test.ts b/packages/objectql/src/engine-after-hook-abort-unit.test.ts index 362fca6803a..fdae4550fa3 100644 --- a/packages/objectql/src/engine-after-hook-abort-unit.test.ts +++ b/packages/objectql/src/engine-after-hook-abort-unit.test.ts @@ -33,7 +33,10 @@ function makeDriver(name: string, opts: { transactional?: boolean; failParentUpd rowsOf: (o: string) => Array.from(storeFor(o).values()), async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, async syncSchema() {}, - async find(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); }, + async find(o: string, ast: any) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, async findOne(o: string, ast: any) { return (await this.find(o, ast))[0] ?? null; }, async create(o: string, data: Record) { const row = { ...data, id: (data.id as string) ?? `${o}_${++n}` }; From 536cd0e1b57a8c38e428911f832043820ed32bc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 10:04:33 +0000 Subject: [PATCH 6/9] docs(spec): onError and HookEvent say where an after* abort rolls back Text only: HookEvent's JSDoc gains the write door's unit as the fourth way a write ends up inside a unit of work; onError's TSDoc and describe() say where abort rolls back and what it means elsewhere. No shape, key, default or enum change. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- packages/spec/src/data/hook.zod.ts | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index 2ec3e511e54..321bf811dc5 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -151,7 +151,12 @@ function refuseBodyOnStoredMetadataTarget( * 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: + * Four ordinary ways a write ends up inside such a unit: + * - the write door itself, when the object has a blocking (not `async`) + * `after*` hook whose `onError` is `abort` (the default) — limited to an + * object on the default datasource whose driver declares transactions, so + * that hook's abort rolls the row back (#22782); see `onError` below for + * what `abort` means where this unit is not opened; * - 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 @@ -419,10 +424,23 @@ export const HookSchema = lazySchema(() => strictObject( /** * Error Policy * What to do if the hook throws an exception? - * - abort: Rollback transaction (if blocking) + * - abort: refuse the operation. A blocking (not `async`) hook's abort rolls + * the write back — its row and the `ctx.api` writes the hook chain made + * through the engine — whenever the write runs inside a unit of work: a + * caller's `transaction()`, an atomic batch or cascade, or the unit the + * write door opens for a blocking `abort` `after*` hook (#22782). The door + * opens that unit only for an object on the default datasource whose + * driver declares transactions. Elsewhere — an object on another + * datasource, or a driver without transactions — an `after*` hook's abort + * refuses the caller while the row stays stored. A `before*` hook's abort + * refuses the write before it is issued, so nothing is stored either way. * - log: Log error and continue */ - onError: z.enum(['abort', 'log']).default('abort').describe('Error handling strategy'), + onError: z.enum(['abort', 'log']).default('abort').describe( + 'Error handling strategy. abort: refuse the operation; a blocking hook\'s abort rolls the write back inside a unit of work, ' + + 'which the write door opens for an after* hook on an object on the default datasource whose driver declares transactions ' + + '(elsewhere the caller is refused and the row stays stored). log: log the error and continue', + ), /** * Execution identity for the hook's `ctx.api` data operations (#14010; From ad052838775103fbb2cc772eaeece7a754a342e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 10:16:49 +0000 Subject: [PATCH 7/9] docs(spec): regenerate the hook reference page; spec patch changeset for the onError text Regenerated with check:generated --fix (gen:docs only, the one artifact it proved stale). Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- .changeset/22782-spec-hook-onerror-abort-text.md | 12 ++++++++++++ content/docs/references/data/hook.mdx | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 .changeset/22782-spec-hook-onerror-abort-text.md diff --git a/.changeset/22782-spec-hook-onerror-abort-text.md b/.changeset/22782-spec-hook-onerror-abort-text.md new file mode 100644 index 00000000000..c583ceb1b10 --- /dev/null +++ b/.changeset/22782-spec-hook-onerror-abort-text.md @@ -0,0 +1,12 @@ +--- +'@objectstack/spec': patch +--- + +`HookSchema.onError` and `HookEvent` now say where an `after*` hook's `abort` rolls the write back, and what it means elsewhere + +Clause-②: no + +Text only: no key, shape, default or enum changes, and every hook that parsed before parses the same. + +- **`onError`** (its TSDoc and `describe()`): `abort` refuses the operation. A blocking hook's abort rolls the write back whenever the write runs inside a unit of work. Those units are a caller's `transaction()`, an atomic batch or cascade, and the unit the write door now opens for a blocking `abort` `after*` hook on an object on the default datasource whose driver declares transactions (`@objectstack/objectql`, same release). On an object on another datasource, or a driver without transactions, an `after*` hook's abort refuses the caller while the row stays stored. +- **`HookEvent`'s JSDoc** listed three ways a write ends up inside a unit of work. It now lists four: the write door's own unit is the new one. diff --git a/content/docs/references/data/hook.mdx b/content/docs/references/data/hook.mdx index 34145cfdcdf..e1a712d27ec 100644 --- a/content/docs/references/data/hook.mdx +++ b/content/docs/references/data/hook.mdx @@ -41,7 +41,7 @@ const result = HookSchema.parse(data); | **retryPolicy** | `{ maxRetries?: number; backoffMs?: number }` | optional | Retry policy for failed hook executions | | **timeoutMs** | `number` | optional | Maximum execution time in milliseconds before the hook is aborted | | **timeout** | `never` | optional | [REMOVED] `hook.timeout` was removed in @objectstack/spec 17 — its unit (milliseconds) lived only in the description, beside a body-level `timeoutMs` and a `retryPolicy.backoffMs` that spell theirs, so the same number read as two conventions on one surface. Rename the key to `timeoutMs`; the value (milliseconds) is unchanged. Run `os migrate meta --from 17` to list the mechanical edits for existing sources; `--write` applies the ones it can prove, and you apply the rest by hand. | -| **onError** | `Enum<'abort' \| 'log'>` | optional (default: `"abort"`) | Error handling strategy | +| **onError** | `Enum<'abort' \| 'log'>` | optional (default: `"abort"`) | Error handling strategy. abort: refuse the operation; a blocking hook's abort rolls the write back inside a unit of work, which the write door opens for an after* hook on an object on the default datasource whose driver declares transactions (elsewhere the caller is refused and the row stays stored). log: log the error and continue | | **runAs** | `Enum<'system' \| 'user' \| 'inherit'>` | optional (default: `"inherit"`) | Execution identity for the hook's ctx.api data operations: system = elevated (bypasses RLS), user = the triggering user (RLS-respecting), inherit = the context of the write that fired the hook (the pre-runAs behaviour; the default). A hook with no trigger user has no identity to scope to, so under user its ctx.api data operations are REFUSED — declare system to make the elevation explicit. This covers any hook fired by a write that carried no user (an isSystem plugin/service write; a system-elevated flow node). Scope: ctx.api only — condition evaluation, the readonly strip on ctx.input, ctx.session and async are unchanged. | | **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | | **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | From 0b6b8275d161059992c14d084c9af8f041d81f76 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 11:38:27 +0000 Subject: [PATCH 8/9] test(runtime): the sandbox's no-host-transaction throw case pins the door's unit A sandboxed afterInsert body declaring no onError aborts, so the write door opens the unit, the body's ctx.api.transaction joins it and the abort rolls the outer row back too. The #6406 subject stays asserted: one begin, the body's write on that handle, one rollback (the owner's). The sibling case's comment says the begin is now the door's; its assertions are unchanged. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- ...82-objectql-after-hook-abort-rolls-back.md | 1 + ...ansaction-ambient-join.integration.test.ts | 36 +++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/.changeset/22782-objectql-after-hook-abort-rolls-back.md b/.changeset/22782-objectql-after-hook-abort-rolls-back.md index 8856832ea70..525f985ed83 100644 --- a/.changeset/22782-objectql-after-hook-abort-rolls-back.md +++ b/.changeset/22782-objectql-after-hook-abort-rolls-back.md @@ -14,5 +14,6 @@ Clause-②: yes (narrowing) - **TO:** that write runs inside a unit of work, so the abort rolls back the row and the hook chain's engine writes together. The caller is refused exactly as before. Hook count and order are unchanged. - **Unchanged:** a write already inside a unit (a caller's `transaction()`, an atomic batch or cascade) joins it, and that unit's owner still decides. No unit is opened for an `onError: 'log'` hook, an `async: true` hook, a hook registered in code with `registerHook` (no declared `onError`), a write with `skipAutomations`, an object whose datasource is not the default one, or a default driver with no transactions (ADR-0119 D1, such as a libSQL remote transport). On those the row stays stored after an abort, as before. - **Inside the new unit, the rules of any unit apply.** An `after*` hook on such an object that writes a business row to an object on another datasource is now refused with `CrossDatasourceTransactionWriteError`, which the abort turns into a refused write. A MongoDB default store needs a replica set for this unit, as it does for `transaction()` already. If either applies to you, mark the hook `onError: 'log'` or `async: true`, or keep the objects on one datasource. +- **A sandboxed hook body's own `ctx.api.transaction`** on such an object, when no transaction is open around the write, used to open and own its transaction, so a throw from it rolled back only the body's own writes and the triggering row stayed stored. It now joins the door's unit, as it joins any open unit, and the abort rolls back the triggering row too. - **Work a unit starts and does not await** (an `async: true` hook) no longer inherits the unit's finished transaction handle. It used to fail with "Transaction query already complete" and lose its write. It now runs on its own connection, as it would outside any unit. - **Cost:** one transaction per write on the objects above only. Measured on the in-process SQLite driver: about 1.36x the per-insert time of the same object with an `onError: 'log'` hook. Objects without such a hook pay one map lookup. diff --git a/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts b/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts index 862929daad4..d4feef2fd81 100644 --- a/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts +++ b/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts @@ -211,23 +211,47 @@ describe('#6406 sandbox ctx.api.transaction joins the host transaction (real eng await engine.insert('thing', { name: 'outer' }); - // The outer write is not in any transaction; the body's is, and owns it. + // Since #22782 the begin belongs to the WRITE DOOR, not the body: the hook + // declares no `onError`, so its `afterInsert` can abort and the door opens a + // unit around the outer write. The body's `ctx.api.transaction` JOINS that + // unit (#6406) and the door commits it once. Before #22782 the outer write + // was in no transaction and the body opened and owned this one. expect(seen.begins).toHaveLength(1); expect(seen.commits).toHaveLength(1); expect(seen.commits[0]).toBe(seen.begins[0]); expect(committedNames('thing')).toEqual(['inner', 'outer']); }, 30000); - it('with NO host transaction a throwing body still ROLLS BACK its own — unchanged', async () => { + /** + * #22782 (triage 6106449072): an `after*` hook whose `onError` is `abort` — the + * default, which this body's hook takes — rolls the write back on the default + * write door, as `HookSchema.onError` says. + * + * FROM: the outer write ran in no unit; the body opened and owned its own + * transaction and rolled back only `inner`, so `outer` stayed + * committed while the caller was refused. + * TO: the door opens a unit around the outer write; the body's + * `ctx.api.transaction` JOINS it, its `__txRollback` ABSTAINS for the + * joined handle and re-throws, and the abort rolls the door's unit + * back — `outer` and `inner` together. + * + * The #6406 subject is unchanged and asserted here: one begin (no second + * connection), the body's write on that same handle, and exactly one + * rollback — the owner's, never the joined body's. + */ + it("with NO host transaction the door's unit owns the begin, the body joins it, and the abort rolls the outer row back too (#22782)", async () => { await wire(THROWING_TX_BODY); await expect(engine.insert('thing', { name: 'outer' })).rejects.toThrow(/body boom/); - expect(seen.begins).toHaveLength(1); - expect(seen.rollbacks).toHaveLength(1); + expect(seen.begins).toHaveLength(1); // one begin: no second connection + expect(seen.creates).toHaveLength(2); + expect(seen.creates[0].transaction).toBe(seen.begins[0]); // the outer write is inside the door's unit + expect(seen.creates[1].transaction).toBe(seen.begins[0]); // the body's write JOINED it + expect(seen.rollbacks).toHaveLength(1); // the joined rollback abstained + expect(seen.rollbacks[0]).toBe(seen.begins[0]); // the one rollback is the owner's expect(seen.commits).toHaveLength(0); - // The body's write is discarded; the outer one was never transactional. - expect(committedNames('thing')).toEqual(['outer']); + expect(committedNames('thing')).toEqual([]); // FROM ['outer'] — the refused write left its row }, 30000); /** From a1ed7388631a7376b9df230401550a1be5b8de6f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:31:25 +0000 Subject: [PATCH 9/9] docs: after* abort now rolls the write back on the default door (data-flow, events); changeset cites ADR-0087's launch-window exemption data-flow.mdx's lifecycle table said an after* hook cannot abort (record is saved); events.mdx gains where the rollback holds. The changeset's provenance for the minor-for-breaking convention named ADR-0131, which decides organization ownership; it now names ADR-0087's ratified launch-window exemption (amended 2026-09-13). The runtime sibling case's title states the door's unit; its assertions are unchanged. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude --- .changeset/22782-objectql-after-hook-abort-rolls-back.md | 2 +- content/docs/api/data-flow.mdx | 2 +- content/docs/kernel/events.mdx | 2 +- .../src/sandbox/transaction-ambient-join.integration.test.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.changeset/22782-objectql-after-hook-abort-rolls-back.md b/.changeset/22782-objectql-after-hook-abort-rolls-back.md index 525f985ed83..f02b523a08c 100644 --- a/.changeset/22782-objectql-after-hook-abort-rolls-back.md +++ b/.changeset/22782-objectql-after-hook-abort-rolls-back.md @@ -8,7 +8,7 @@ Clause-②: yes (narrowing) -**BREAKING** stored-outcome narrowing, shipped as `minor` under the repo's launch-window convention for breaking changes (ADR-0131 D3, D9). +**BREAKING** stored-outcome narrowing, shipped as `minor` under the repo's launch-window convention for breaking changes (ADR-0087, the ratified pre-launch launch-window exemption, as amended 2026-09-13, #18003). - **FROM:** `insert` / `update` / `delete` (REST `createData` / `updateData` / `deleteData`, `ctx.api`, the engine door) on an object with a blocking (not `async`) `after*` hook bound from metadata whose `onError` is `abort` or omitted. When that hook threw, the caller was refused, but the row stayed stored, changed or deleted, and so did every write the hook chain had already made through `ctx.api`. A client that retried the refused create made a duplicate. - **TO:** that write runs inside a unit of work, so the abort rolls back the row and the hook chain's engine writes together. The caller is refused exactly as before. Hook count and order are unchanged. diff --git a/content/docs/api/data-flow.mdx b/content/docs/api/data-flow.mdx index 4fd7b8427cf..a20b0fe2582 100644 --- a/content/docs/api/data-flow.mdx +++ b/content/docs/api/data-flow.mdx @@ -311,7 +311,7 @@ The lifecycle events are defined by the `HookEvent` enum (8 events). Reads fire | Hook | Phase | Can Modify? | Can Abort? | |:---|:---|:---|:---| | `beforeInsert/Update/Delete` | Pre-persist | ✅ Yes (mutate `input`) | ✅ Yes (throw with `onError: 'abort'`) | -| `afterInsert/Update/Delete` | Post-persist | ✅ Yes (mutate `result`) | ❌ No (record is saved) | +| `afterInsert/Update/Delete` | Post-persist, before the unit of work commits | ✅ Yes (mutate `result`) | ✅ Yes (a blocking hook that throws with `onError: 'abort'`, the default): the write is refused and rolled back. On an object outside the default datasource, or on a driver with no transactions, the caller is refused but the row stays stored — see [After hooks run inside the unit of work](/docs/automation/hooks#after-hooks-run-inside-the-unit-of-work) | **Side Effects:** Place side effects (emails, notifications, external API calls) in `after*` hooks. Set `async: true` so they run fire-and-forget without blocking the operation, and use `onError: 'log'` so a failing side effect doesn't roll back the write. diff --git a/content/docs/kernel/events.mdx b/content/docs/kernel/events.mdx index 12716d6e3bb..40ea34b09c1 100644 --- a/content/docs/kernel/events.mdx +++ b/content/docs/kernel/events.mdx @@ -112,7 +112,7 @@ engine.registerHook('beforeInsert', async (ctx) => { Hooks declared as **`Hook` metadata** additionally honour an **`onError`** policy (`'abort'` | `'log'`, default `'abort'`). It is a metadata field — `registerHook()` has no `onError` option: -- `onError: 'abort'` (default) — the error propagates, rolling back the transaction (when the hook is blocking) and cancelling the operation. +- `onError: 'abort'` (default) — the error propagates, rolling back the transaction (when the hook is blocking) and cancelling the operation. For an `after*` hook the write door opens that transaction itself when none is open, but only for an object on the default datasource whose driver declares transactions; elsewhere the operation is refused while its row stays stored. - `onError: 'log'` — the error is logged and swallowed; the operation is **not** cancelled. ### Ordering diff --git a/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts b/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts index d4feef2fd81..fa2d3472464 100644 --- a/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts +++ b/packages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts @@ -206,7 +206,7 @@ describe('#6406 sandbox ctx.api.transaction joins the host transaction (real eng expect(committedNames('thing')).toEqual([]); }, 30000); - it('with NO host transaction the sandbox still OPENS one — unchanged behaviour', async () => { + it("with NO host transaction the door's unit owns the one begin, the sandbox body joins it, and the door commits both rows (#22782)", async () => { await wire(TX_BODY); await engine.insert('thing', { name: 'outer' });