diff --git a/.changeset/transaction-contract-require-and-owned.md b/.changeset/transaction-contract-require-and-owned.md new file mode 100644 index 0000000000..1306867857 --- /dev/null +++ b/.changeset/transaction-contract-require-and-owned.md @@ -0,0 +1,42 @@ +--- +"@objectstack/spec": minor +"@objectstack/objectql": minor +"@objectstack/core": minor +--- + +feat(spec,objectql): `engine.transaction` 契约收紧第一批 —— `opts.require` fail-closed 与 `owned` 信号 (#5696) + +`IObjectQLEngine.transaction` 的声明面(`packages/spec/src/contracts/objectql-engine.ts`, +ADR-0119 D1)此前把「默认驱动之外的对象写在事务外」与「驱动没有 `beginTransaction` +时静默降级」写成**声明语义**的一部分。#4619 把这两条降级变得可观测(PR #5724),本次 +把其中两条收紧为调用方可选的契约,并同步修订 TSDoc 的事实性偏差。 + +**新增(可选,默认行为完全不变):** + +- `transaction(cb, base, { require: true })` —— 驱动没有 `beginTransaction` 时 + **抛 `TransactionUnsupportedError`(`code: 'ERR_TRANSACTION_UNSUPPORTED'`)**, + 而不是静默降级成「无事务、无回滚」。在回调运行**之前**拒绝,所以调用方收到错误时 + 一行都还没写。这是把 `batchData` 的 atomic 门(ADR-0119 D4)泛化成通用能力: + 只为「开事务的唯一理由就是回滚」的调用方而设,不传 `require` 的行为一字未变 + (仍然降级 + warn-once)。 +- 回调的**第二个参数** `{ owned: boolean }` —— `true` 表示本次调用开启了事务并拥有 + 提交/回滚,`false` 表示它 **join** 了外层已开的 ambient 事务(ADR-0067 D2), + 或者处在降级路径上(那里根本没有事务可拥有)。join 语义本身正确且保留;缺的是 + 调用方**无从分辨**,而「整体一起回滚」这类担保只在 owned 时成立。单参数回调不受影响。 + +两点在 `ctx.api.transaction`(`ScopedContext.transaction`,沙箱 hook/action 体)上 +同样生效 —— 同一个原语的第二份实现不该变成第二种方言。 + +**契约文本修订:** transaction 的 TSDoc 原先写「路由到别处的对象在事务**外**写入」, +实测不符 —— 引擎无条件把 ambient 事务句柄穿给了目标驱动,语句在**错误的连接**上执行 +(#5351 在真 SQL driver 上实测为 `no such table`)。TSDoc 已按实测改写,并声明了随后 +落地的两条语义:业务写跨驱动**响亮拒绝**、系统账本(`lifecycle.class` 为 +`audit`/`telemetry`/`event`)**移出事务执行**。 + +**类型面:** `@objectstack/core` 的 `EngineWithTransaction` 从「手抄签名」改为 +`transaction: IObjectQLEngine['transaction']`,窄接口可以窄,但不能与真签名漂移。 +新导出 `EngineTransactionOptions` / `EngineTransactionInfo`(spec `contracts` 命名空间, +经 `@objectstack/core` 转出)。 + +升级须知:无破坏性变更。既有调用点全部保持原行为;要 fail-closed 的调用方显式传 +`{ require: true }`。 diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 72f071a1d5..c02358876d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -66,5 +66,7 @@ export type { IDataEngine, IObjectQLEngine, EngineSchemaRegistryView, + EngineTransactionOptions, + EngineTransactionInfo, IDataDriver, } from '@objectstack/spec/contracts'; diff --git a/packages/core/src/utils/migration-journal.ts b/packages/core/src/utils/migration-journal.ts index ab77d2a344..1e534e81e2 100644 --- a/packages/core/src/utils/migration-journal.ts +++ b/packages/core/src/utils/migration-journal.ts @@ -99,9 +99,18 @@ export function engineCanRollBack(engine: T): engine is T & EngineWithTransac return !defaultDriver || typeof (defaultDriver as { beginTransaction?: unknown }).beginTransaction === 'function'; } -/** What {@link engineCanRollBack} proves is present. Mirrors `IObjectQLEngine['transaction']`. */ +/** + * What {@link engineCanRollBack} proves is present. + * + * Typed FROM the contract rather than transcribed from it (#5696): a hand-copy + * mirrors the signature only until the contract moves, and this one had already + * started to — it predates `opts.require` and the callback's `owned` argument. + * ADR-0119 D1 blessed exactly this shape for the narrow host surfaces + * (`transaction?: IObjectQLEngine['transaction']`); a *narrow* surface may stay + * narrow, but it may not drift from the real signature. + */ export interface EngineWithTransaction { - transaction(callback: (trxCtx: any) => Promise, baseContext?: any): Promise; + transaction: IObjectQLEngine['transaction']; } /** What a forward/compensate callback is told about the chunk it is running. */ diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-endpoint-gate.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-endpoint-gate.test.ts index 8f2268f842..c19f53b460 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-endpoint-gate.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-endpoint-gate.test.ts @@ -169,8 +169,8 @@ function makeStubEngine(namespace?: string) { rows.delete(found.key); return { deleted: 1 }; }, - async transaction(cb: (ctx: any) => Promise): Promise { - return cb(undefined); + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); }, registry: { registerItem: () => {}, diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts index a5d4d6b350..7d9eb840f8 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts @@ -166,8 +166,8 @@ function makeStubEngine() { rows.delete(found.key); return { deleted: 1 }; }, - async transaction(cb: (ctx: any) => Promise): Promise { - return cb(undefined); + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); }, registry: { registerItem: () => {}, diff --git a/packages/metadata-protocol/src/protocol.read-decorations.test.ts b/packages/metadata-protocol/src/protocol.read-decorations.test.ts index cfd2820075..8756190fed 100644 --- a/packages/metadata-protocol/src/protocol.read-decorations.test.ts +++ b/packages/metadata-protocol/src/protocol.read-decorations.test.ts @@ -92,7 +92,7 @@ function makeStubEngine() { assertEngineDeleteDispatch(opts); return { deleted: 0 }; }, - async transaction(cb: (ctx: any) => Promise): Promise { return cb(undefined); }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { return cb(undefined, { owned: true }); }, async syncObjectSchema() { /* no DDL in this stub */ }, registry: { listItems: () => [], diff --git a/packages/metadata-protocol/src/sys-metadata-repository.draft-drain.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.draft-drain.test.ts index 77bfbb1f22..02c903a5b4 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.draft-drain.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.draft-drain.test.ts @@ -180,13 +180,13 @@ function makeFakeEngine() { rows.delete(found.key); return { deleted: 1 }; }, - async transaction(cb: (ctx: any) => Promise): Promise { + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { const rowsSnapshot = new Map(Array.from(rows, ([k, r]) => [k, { ...r }] as const)); const historySnapshot = historyRows.map((h) => ({ ...h })); const outer = pendingRollback; pendingRollback = rowsSnapshot; try { - return await cb({ txn: true }); + return await cb({ txn: true }, { owned: true }); } catch (err) { rows.clear(); for (const [k, r] of rowsSnapshot) rows.set(k, r); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.history-counters.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.history-counters.test.ts index c5b9b27c6c..428f04beac 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.history-counters.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.history-counters.test.ts @@ -158,11 +158,11 @@ function makeFakeEngine() { rows.delete(found.key); return { deleted: 1 }; }, - async transaction(cb: (ctx: any) => Promise): Promise { + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { const rowsSnapshot = new Map(Array.from(rows, ([k, r]) => [k, { ...r }] as const)); const historySnapshot = historyRows.map((h) => ({ ...h })); try { - return await cb({ txn: true }); + return await cb({ txn: true }, { owned: true }); } catch (err) { // ACID: a txn body that throws commits nothing at all. rows.clear(); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.recorded-by.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.recorded-by.test.ts index a0cc471023..3eadcc5cac 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.recorded-by.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.recorded-by.test.ts @@ -96,7 +96,7 @@ function makeFakeEngine() { rows.delete(found.key); return { deleted: 1 }; }, - async transaction(cb: (ctx: any) => Promise): Promise { return cb(undefined); }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { return cb(undefined, { owned: true }); }, }; } diff --git a/packages/objectql/src/engine-transaction-contract.test.ts b/packages/objectql/src/engine-transaction-contract.test.ts new file mode 100644 index 0000000000..b45f0fe030 --- /dev/null +++ b/packages/objectql/src/engine-transaction-contract.test.ts @@ -0,0 +1,324 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #5696 (the tightening half of #4619) — points 1 and 3 of the contract +// revision, pinned against the engine that implements them: +// +// 1. `opts.require: true` — the ADR-0119 D1 degrade (driver with no +// `beginTransaction` runs the callback with no transaction and no +// rollback) becomes a THROW for callers that cannot tolerate it. Fail +// closed, BEFORE anything is written, generalizing `batchData`'s atomic +// gate (ADR-0119 D4). +// 3. `owned` — the callback is told whether it OPENED the transaction or +// JOINED an outer one (ADR-0067 D2). The join is correct and stays; what +// was missing is that the callback could not tell, so it could not know +// whether its own "this all rolls back together" promise held. +// +// Point 2 (cross-driver business writes refused) is NOT here: it is coupled by +// the 2026-08-06 ruling to #5351's system-write carve-out and lands with it, in +// `engine-transaction-same-origin.test.ts`. Landing the refusal alone would be +// "loud but not fixed" — the audit hook's try/catch eats the refusal and the +// compliance row is lost exactly as before. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL, ScopedContext } from './engine.js'; +import { TransactionUnsupportedError } from './transaction-errors.js'; + +interface Recorded { + level: 'debug' | 'info' | 'warn' | 'error'; + message: string; + args: unknown[]; +} + +/** + * The error a call rejected with, typed as `E`. + * + * `promise.catch((e) => e as E)` types the result as `E | `, so + * every property read on it is a type error. This narrows to the rejection and + * fails loudly if the call did NOT reject — which a bare `.catch()` would + * silently let through as a passing test. + */ +async function rejection(p: Promise): Promise { + try { + await p; + } catch (e) { + return e as E; + } + throw new Error('expected the call to reject, but it resolved'); +} + +function recordingLogger() { + const records: Recorded[] = []; + const push = (level: Recorded['level']) => (message: string, ...args: unknown[]) => + void records.push({ level, message: String(message), args }); + return { + records, + logger: { debug: push('debug'), info: push('info'), warn: push('warn'), error: push('error') }, + at(level: Recorded['level']) { + return records.filter((r) => r.level === level); + }, + }; +} + +/** + * `transactional: false` makes a driver WITHOUT `beginTransaction` — the shape + * the degrade path exists for (test doubles, foreign engines; every in-tree + * driver implements it). + */ +function makeDriver(name: string, opts: { transactional?: boolean } = {}) { + const writes: Array<{ object: string; op: 'create' | 'update' | 'delete'; transaction: unknown }> = []; + const rows = new Map>(); + let nextId = 0; + const driver: any = { + name, + version: '0.0.0', + supports: {}, + writes, + async connect() {}, + async disconnect() {}, + async checkHealth() { return true; }, + async execute() { return null; }, + async find() { return Array.from(rows.values()); }, + async findOne(_o: string, ast: any) { + const id = ast?.where?.find?.((c: any) => c?.field === 'id')?.value; + if (id !== undefined) return rows.get(String(id)) ?? null; + for (const r of rows.values()) return r; + return null; + }, + async create(object: string, data: Record, options: any) { + writes.push({ object, op: 'create', transaction: options?.transaction }); + nextId += 1; + const id = (data.id as string) ?? `${name}_${nextId}`; + const row = { ...data, id }; + rows.set(id, row); + return row; + }, + async update(object: string, id: string, data: Record, options: any) { + writes.push({ object, op: 'update', transaction: options?.transaction }); + const row = { ...rows.get(String(id)), ...data, id }; + rows.set(String(id), row); + return row; + }, + async delete(object: string, id: string, options: any) { + writes.push({ object, op: 'delete', transaction: options?.transaction }); + return rows.delete(String(id)); + }, + async count() { return 0; }, + async bulkCreate(object: string, batch: Record[]) { + return Promise.all(batch.map((r) => this.create(object, r, undefined))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async syncSchema() {}, + }; + if (opts.transactional !== false) { + driver.beginTransaction = async () => ({ __trx: name }); + driver.commit = async () => {}; + driver.rollback = async () => {}; + } + return driver; +} + +async function engineWith(opts: { transactional: boolean }) { + const rec = recordingLogger(); + const engine = new ObjectQL({ logger: rec.logger } as any); + const driver = makeDriver('primary', { transactional: opts.transactional }); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any, '__test__'); + return { rec, engine, driver }; +} + +// --------------------------------------------------------------------------- +// 1. `opts.require: true` — fail closed instead of degrading +// --------------------------------------------------------------------------- + +describe('transaction({ require: true }) refuses a driver that cannot roll back (#5696)', () => { + it('throws TransactionUnsupportedError instead of running the callback', async () => { + const { engine } = await engineWith({ transactional: false }); + let ran = false; + + await expect( + engine.transaction(async () => { ran = true; }, undefined, { require: true }), + ).rejects.toBeInstanceOf(TransactionUnsupportedError); + + // Refused BEFORE the callback — the whole point of failing closed is that + // nothing has been written when the caller finds out. + expect(ran).toBe(false); + }); + + it('carries the boundary-crossing code, the datasource, and the fix', async () => { + const { engine } = await engineWith({ transactional: false }); + + const err = await rejection( + engine.transaction(async () => 'unreachable', undefined, { require: true }), + ); + + expect(err.code).toBe('ERR_TRANSACTION_UNSUPPORTED'); + expect(err.datasource).toBe('primary'); + expect(err.message).toContain("driver 'primary' has no beginTransaction"); + expect(err.message).toContain('nothing has been written'); + // An error that refuses owes the reader the remedy, both halves of it. + expect(err.message).toContain('Register a driver that implements beginTransaction'); + expect(err.message).toContain('drop `require`'); + }); + + it('writes nothing — the refusal is not a rollback, it is a non-start', async () => { + const { engine, driver } = await engineWith({ transactional: false }); + + await engine + .transaction(async () => { await engine.insert('thing', { name: 'x' }); }, undefined, { require: true }) + .catch(() => undefined); + + expect(driver.writes).toHaveLength(0); + }); + + it('is silent about the degrade it refused — the throw IS the report', async () => { + const { engine, rec } = await engineWith({ transactional: false }); + + await engine.transaction(async () => 1, undefined, { require: true }).catch(() => undefined); + + // The warn-once budget exists for callers who KEEP GOING without a + // transaction. This caller did not: it was told by rejection, which is + // louder than any log line, so emitting the warn too would be noise. + expect(rec.at('warn')).toHaveLength(0); + }); + + it('does nothing at all when the driver CAN transact', async () => { + const { engine, driver, rec } = await engineWith({ transactional: true }); + + const out = await engine.transaction( + async () => { await engine.insert('thing', { name: 'ok' }); return 'done'; }, + undefined, + { require: true }, + ); + + expect(out).toBe('done'); + expect(driver.writes).toHaveLength(1); + expect(driver.writes[0].transaction).toEqual({ __trx: 'primary' }); + expect(rec.at('warn')).toHaveLength(0); + }); + + it('leaves the DEFAULT unchanged — no `require` still degrades and warns (ADR-0119 D1)', async () => { + const { engine, rec } = await engineWith({ transactional: false }); + let ran = false; + + // Regression guard: `require` is opt-in. Making the degrade throw for + // everyone would fail-close every deployment whose driver cannot transact, + // which is the change this option exists to AVOID making globally. + await engine.transaction(async () => { ran = true; }); + + expect(ran).toBe(true); + expect(rec.at('warn').filter((r) => r.message.includes('has no beginTransaction'))).toHaveLength(1); + }); + + it('honours `require: false` as the default, not as a second spelling of true', async () => { + const { engine } = await engineWith({ transactional: false }); + await expect(engine.transaction(async () => 'ran', undefined, { require: false })).resolves.toBe('ran'); + }); +}); + +// --------------------------------------------------------------------------- +// 3. `owned` — opened by me, or joined from an outer owner? +// --------------------------------------------------------------------------- + +describe('transaction() tells its callback whether it OWNS the transaction (#5696, ADR-0067 D2)', () => { + it('owned: true for the call that opened it', async () => { + const { engine } = await engineWith({ transactional: true }); + const seen: boolean[] = []; + + await engine.transaction(async (_ctx, info) => { seen.push(info.owned); }); + + expect(seen).toEqual([true]); + }); + + it('owned: false for a nested call that JOINED it', async () => { + const { engine, driver } = await engineWith({ transactional: true }); + const seen: boolean[] = []; + + await engine.transaction(async (_outerCtx, outer) => { + seen.push(outer.owned); + await engine.transaction(async (_innerCtx, inner) => { + seen.push(inner.owned); + await engine.insert('thing', { name: 'nested' }); + }); + }); + + expect(seen).toEqual([true, false]); + // The join itself is unchanged: ONE transaction, and the nested write rode + // the outer owner's handle. + expect(driver.writes).toHaveLength(1); + expect(driver.writes[0].transaction).toEqual({ __trx: 'primary' }); + }); + + it('owned: false on the degrade path — there is no transaction to own', async () => { + const { engine } = await engineWith({ transactional: false }); + let owned: boolean | undefined; + + await engine.transaction(async (_ctx, info) => { owned = info.owned; }); + + // Not a lie by omission: `owned: false` says "you do not own a rollback", + // which is exactly true here. A caller that needs to distinguish "someone + // else owns it" from "nobody does" passes `require: true` and never + // reaches this branch at all. + expect(owned).toBe(false); + }); + + it('does not disturb one-argument callbacks', async () => { + const { engine } = await engineWith({ transactional: true }); + const legacy = async (ctx: any) => { + expect(ctx.transaction).toEqual({ __trx: 'primary' }); + return 'legacy'; + }; + await expect(engine.transaction(legacy)).resolves.toBe('legacy'); + }); + + it('still threads baseContext when the third argument is present', async () => { + const { engine } = await engineWith({ transactional: true }); + + const ctx = await engine.transaction( + async (trxCtx) => trxCtx, + { userId: 'u1', isSystem: true }, + { require: true }, + ); + + expect(ctx).toMatchObject({ userId: 'u1', isSystem: true, transaction: { __trx: 'primary' } }); + }); +}); + +// --------------------------------------------------------------------------- +// The sandbox surface is the same primitive, not a second dialect +// --------------------------------------------------------------------------- + +describe('ScopedContext.transaction (ctx.api.transaction) carries the same two points (#5696)', () => { + function scopedOf(engine: ObjectQL): ScopedContext { + return (engine as any).createContext({ userId: 'u1' }) as ScopedContext; + } + + it('refuses under require: true, with the same error', async () => { + const { engine } = await engineWith({ transactional: false }); + let ran = false; + + await expect( + scopedOf(engine).transaction(async () => { ran = true; }, { require: true }), + ).rejects.toBeInstanceOf(TransactionUnsupportedError); + expect(ran).toBe(false); + }); + + it('still degrades without require — behaviour unchanged', async () => { + const { engine } = await engineWith({ transactional: false }); + await expect(scopedOf(engine).transaction(async () => 'ran')).resolves.toBe('ran'); + }); + + it('reports owned: true (this surface always opens) and false when degraded', async () => { + const withTx = await engineWith({ transactional: true }); + const withoutTx = await engineWith({ transactional: false }); + let ownedOpen: boolean | undefined; + let ownedDegraded: boolean | undefined; + + await scopedOf(withTx.engine).transaction(async (_ctx, info) => { ownedOpen = info.owned; }); + await scopedOf(withoutTx.engine).transaction(async (_ctx, info) => { ownedDegraded = info.owned; }); + + expect(ownedOpen).toBe(true); + expect(ownedDegraded).toBe(false); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 9ab6a1a733..c4310717f3 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -33,6 +33,8 @@ import { IDataDriver, IDataEngine, type IObjectQLEngine, + type EngineTransactionInfo, + type EngineTransactionOptions, Logger, createLogger, withTransientRetry, @@ -41,6 +43,7 @@ import { resolveFilterTokens, } from '@objectstack/core'; import { SummaryRecomputeError, type SummaryRecomputeFailure } from './summary-errors.js'; +import { TransactionUnsupportedError } from './transaction-errors.js'; import { aggregateSummaryValue, summaryEmptySetValue, @@ -6635,24 +6638,31 @@ export class ObjectQL implements IObjectQLEngine { * the API safe to call on drivers without ACID support (e.g. the * in-memory driver in tests). It is DECLARED behaviour (ADR-0119 D1), not * a bug to be discovered — but since v17 it is no longer *silent*: the - * degrade warns once per driver (#4619, {@link warnTransactionUnsupported}). + * degrade warns once per driver (#4619, {@link warnTransactionUnsupported}), + * and a caller who cannot live with it says so with `opts.require: true`, + * which THROWS {@link TransactionUnsupportedError} instead (#5696 point 1). * - On callback success the transaction is committed; on any thrown error * it is rolled back and the original error is re-thrown. * - The transaction covers the DEFAULT datasource only — also declared * (ADR-0119 D1). A write that `setDatasourceMapping` routes elsewhere runs - * OUTSIDE it and survives the rollback; that split is now reported at - * `error` from the write path (#4619, - * {@link reportWriteOutsideTransaction}). Reporting it does not fix it: - * refusing, or committing across drivers, would change the declared - * contract and is tracked by #4619's spec half. + * OUTSIDE it and survives the rollback; that split is reported at `error` + * from the write path (#4619, {@link reportWriteOutsideTransaction}). + * - The callback's SECOND argument says whether this call owns the + * transaction (#5696 point 3): `owned: true` when this call opened it, + * `false` when it JOINED an outer one (ADR-0067 D2) — and `false` on the + * degrade path too, where there is no transaction to own. A callback whose + * own guarantees are phrased as "this all rolls back together" only holds + * that promise when it owns the transaction; before this signal it had no + * way to tell. One-argument callbacks are unaffected. * * Use case: multi-step operations that must be atomic (e.g. CRM * `convertLead`, which creates an account + contact + opportunity + flips * the lead in a single unit of work). */ async transaction( - callback: (trxCtx: any) => Promise, + callback: (trxCtx: any, info: EngineTransactionInfo) => Promise, baseContext?: any, + opts?: EngineTransactionOptions, ): Promise { // ADR-0067 D2 — JOIN an already-open ambient transaction instead of // opening a nested driver transaction. A nested begin would acquire a @@ -6664,15 +6674,29 @@ export class ObjectQL implements IObjectQLEngine { // sys-metadata repository's `withTxn`, hook-driven writes, …). const ambient = this.txStore.getStore(); if (ambient?.transaction) { - return callback({ ...(baseContext ?? {}), transaction: ambient.transaction }); + // JOINED, not owned: some outer caller decides commit vs rollback (#5696). + return callback( + { ...(baseContext ?? {}), transaction: ambient.transaction }, + { owned: false }, + ); } const driver = this.defaultDriver ? this.drivers.get(this.defaultDriver) : undefined; const drv = driver as any; if (!drv?.beginTransaction) { + const datasource = this.defaultDriver ?? drv?.name; + if (opts?.require === true) { + // Fail CLOSED (#5696 point 1): the caller declared it cannot tolerate + // running without a rollback, so refuse BEFORE the callback writes + // anything rather than degrade behind a warning it may never read. + // Generalizes `batchData`'s atomic gate (ADR-0119 D4). + throw new TransactionUnsupportedError(datasource ?? ''); + } // Declared degrade (ADR-0119 D1) — behaviour unchanged, but no longer // mute: the caller asked for atomicity and is not getting it (#4619). - this.warnTransactionUnsupported(this.defaultDriver ?? drv?.name); - return callback(baseContext); + this.warnTransactionUnsupported(datasource); + // `owned: false` — honest: there is no transaction here to own, and no + // rollback the callback may promise on the strength of it. + return callback(baseContext, { owned: false }); } const trx = await drv.beginTransaction(); const trxCtx = { ...(baseContext ?? {}), transaction: trx }; @@ -6681,7 +6705,7 @@ export class ObjectQL implements IObjectQLEngine { // queries during writes reuse this transaction's connection (ADR-0034). const result = await this.txStore.run( { transaction: trx, scope: this.newTransactionScope(driver!) }, - () => callback(trxCtx), + () => callback(trxCtx, { owned: true }), ); if (drv.commit) await drv.commit(trx); else if (drv.commitTransaction) await drv.commitTransaction(trx); @@ -7274,8 +7298,16 @@ export class ScopedContext { * caveats report through the SAME engine-side helpers the engine's own * `transaction()` uses, so the sandbox surface is no quieter than the direct * one and "say it once" holds across both. + * + * `opts.require` and the callback's `owned` argument (#5696) are honoured + * here for the same reason: a second implementation of one primitive must not + * become a second DIALECT of it. A hook body that fails closed through + * `ctx.api.transaction` gets the same refusal the engine's own surface gives. */ - async transaction(callback: (trxCtx: ScopedContext) => Promise): Promise { + async transaction( + callback: (trxCtx: ScopedContext, info: EngineTransactionInfo) => Promise, + opts?: EngineTransactionOptions, + ): Promise { const engine = this.engine as any; // Find the default driver for transaction support @@ -7284,11 +7316,16 @@ export class ScopedContext { : undefined; if (!driver?.beginTransaction) { + const datasource = engine.defaultDriver ?? driver?.name; + if (opts?.require === true) { + // Same fail-closed refusal as the engine surface (#5696 point 1). + throw new TransactionUnsupportedError(datasource ?? ''); + } // No transaction support — execute directly. Declared (ADR-0119 D1), but // said out loud since #4619: the caller asked for atomicity and the // callback is about to run without any. - engine.warnTransactionUnsupported?.(engine.defaultDriver ?? driver?.name); - return callback(this); + engine.warnTransactionUnsupported?.(datasource); + return callback(this, { owned: false }); } const trx = await driver.beginTransaction(); @@ -7309,7 +7346,9 @@ export class ScopedContext { txStore ? txStore.run({ transaction: trx, scope }, fn) : fn(); try { - const result = await runIn(() => callback(trxCtx)); + // This surface always OPENS (it has no ADR-0067 D2 join branch of its + // own), so a callback that reaches here owns the outcome. + const result = await runIn(() => callback(trxCtx, { owned: true })); if (driver.commit) await driver.commit(trx); else if (driver.commitTransaction) await driver.commitTransaction(trx); return result; diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 6fce791553..0c4cc70b8f 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -67,6 +67,10 @@ export type { DatasourceUnavailableKind, } from './driver-connect-errors.js'; export type { InsertManyRowOutcome } from './engine.js'; +// [#5696] Thrown by `transaction(cb, base, { require: true })` when the +// datasource cannot give a real transaction. Exported so a caller that fails +// closed can narrow on the class; `code` is the boundary-crossing identity. +export { TransactionUnsupportedError } from './transaction-errors.js'; // [#4550] The delete-dispatch contract, exported so a TEST DOUBLE that stands // in for the engine can import the producer's own decision rather than diff --git a/packages/objectql/src/transaction-errors.ts b/packages/objectql/src/transaction-errors.ts new file mode 100644 index 0000000000..f8cd07c8ed --- /dev/null +++ b/packages/objectql/src/transaction-errors.ts @@ -0,0 +1,38 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Errors thrown by the engine's transaction seam (#5696 — the tightening half + * of #4619, revising ADR-0119 D1). + * + * Errors here identify themselves by a `code` field rather than by + * `instanceof`, for the reason `DriverConnectError` already records: the check + * has to survive crossing a package boundary, where two copies of this module + * can exist. + */ + +/** + * `transaction(cb, base, { require: true })` was called on a datasource whose + * driver has no `beginTransaction` (#5696 point 1). + * + * Without `require` this path DEGRADES — the callback runs with no transaction + * and no rollback, warning once (ADR-0119 D1, unchanged). The degrade is right + * for callers who can live without atomicity and wrong for callers whose only + * reason to open a transaction is the rollback; `require: true` is how the + * second kind says so, and this error is what it gets. It is thrown BEFORE the + * callback runs, so nothing has been written when it surfaces — the fail-closed + * posture `batchData`'s atomic gate established (ADR-0119 D4). + */ +export class TransactionUnsupportedError extends Error { + readonly code = 'ERR_TRANSACTION_UNSUPPORTED' as const; + + constructor(public readonly datasource: string) { + super( + `transaction({ require: true }) cannot be honoured: driver '${datasource}' has no beginTransaction, ` + + 'so the callback would run with NO transaction and NO rollback — every write committing as it ' + + 'executes, and a later throw leaving the earlier ones persisted. Refused before running anything, ' + + 'so nothing has been written. Register a driver that implements beginTransaction for this ' + + 'datasource, or drop `require` if this caller can genuinely tolerate losing atomicity.', + ); + this.name = 'TransactionUnsupportedError'; + } +} diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index da9e5c5ee2..18c17ce892 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -72,6 +72,8 @@ "EmailAttachment (interface)", "EmailDeliveryStatus (type)", "EngineSchemaRegistryView (interface)", + "EngineTransactionInfo (interface)", + "EngineTransactionOptions (interface)", "ExecuteUpgradeInput (interface)", "ExplainAccessRequest (interface)", "ExportJobDownload (interface)", diff --git a/packages/spec/src/contracts/objectql-engine.ts b/packages/spec/src/contracts/objectql-engine.ts index f5f4f0461f..928c1de04d 100644 --- a/packages/spec/src/contracts/objectql-engine.ts +++ b/packages/spec/src/contracts/objectql-engine.ts @@ -107,6 +107,55 @@ export interface EngineSchemaRegistryView { ): unknown; } +/** + * Options for {@link IObjectQLEngine.transaction} (#5696 — the tightening half + * of #4619, revising ADR-0119 D1). + * + * One member today, deliberately: the surface grows when a consumer proves it + * needs more, the same evidence bar this file's header sets for members. + */ +export interface EngineTransactionOptions { + /** + * Fail CLOSED when the datasource cannot give a real transaction. + * + * Default (`undefined` / `false`) keeps ADR-0119 D1's declared degrade: a + * driver without `beginTransaction` runs the callback with no transaction + * and no rollback, warning once. That degrade is right for callers who can + * live without atomicity (test doubles, in-memory drivers) and wrong for + * callers whose whole reason to open a transaction is the rollback. + * + * With `require: true` the engine THROWS instead of degrading, before the + * callback runs — so a caller that cannot tolerate losing atomicity states + * it once, at the call site, instead of re-deriving `batchData`'s probe. + * That probe is the precedent being generalized here (ADR-0119 D4, cited in + * older text as ADR-0118 D4 — see that ADR's renumbering note): an `atomic` + * request refuses rather than silently running best-effort. + */ + require?: boolean; +} + +/** + * What {@link IObjectQLEngine.transaction} tells its callback about the + * transaction the callback is running in (#5696). + */ +export interface EngineTransactionInfo { + /** + * `true` when THIS call opened the transaction and therefore owns its + * commit/rollback; `false` when it JOINED an already-open ambient one + * (ADR-0067 D2) and some outer caller owns the outcome. + * + * The join is correct and stays — a nested `begin` would take a second + * connection (deadlocking a single-connection SQLite pool) and would not be + * covered by the outer rollback. What was missing is that the callback + * could not TELL: a joined callback's `throw` unwinds work the outer owner + * may still commit or roll back on its own terms, and guarantees phrased + * as "this whole unit rolls back together" (`batchData`'s rollback + * response, ADR-0119 D4) hold only for the owner. A callback that must not + * promise what it does not control reads `owned` and says so. + */ + owned: boolean; +} + /** * The full ObjectQL engine, as the `objectql` slot's consumers use it. * @@ -193,7 +242,7 @@ export interface IObjectQLEngine extends IDataEngine { /** Drop the memoized migration-flag reads (the attestation may race a fast boot's first read). */ invalidateDataMigrationFlags(): void; - // ── Transactions (ADR-0119 D1) ─────────────────────────────────────── + // ── Transactions (ADR-0119 D1, revised by #5696/#5351) ─────────────── /** * Run `callback` inside ONE driver transaction — the ADR-0034 ambient * transaction. The callback receives a context carrying the handle, which @@ -213,18 +262,60 @@ export interface IObjectQLEngine extends IDataEngine { * header; callers that tolerate test doubles keep their runtime * `typeof === 'function'` probes, which types do not replace. * - * TWO CAVEATS ARE PART OF THE DECLARED MEANING (ADR-0119 D1), not - * behaviour to be discovered: this covers the DEFAULT driver only — objects - * routed elsewhere by `setDatasourceMapping` are written outside it — and - * when that driver has no `beginTransaction` the callback runs with NO - * transaction and NO rollback. A caller that cannot tolerate silently - * losing atomicity must fail closed itself rather than assume it held; see - * `batchData`'s atomic gate (ADR-0119 D4). Tightening both is tracked by - * the ADR's follow-up. + * ## The transaction still covers ONE datasource — but no longer silently + * + * A transaction is opened on the DEFAULT driver and covers only that + * driver's connection; cross-driver atomicity is NOT provided (no + * two-phase commit — deliberately out of scope, #4619). What changes with + * #5696/#5351 is what happens to a write inside the transaction that + * routes somewhere else. Until v17 the engine handed the OTHER driver the + * default driver's transaction handle unconditionally, so the statement + * executed on the wrong connection — the text here used to say such writes + * ran "outside" the transaction, which measurement disproved (#5351: on a + * real SQL driver the write reached a database that has no such table, and + * the row was lost with only a log line behind it). The engine now compares + * the resolved driver against the transaction's OWNER (by instance + * identity) and takes one of two paths: + * + * - **Business writes are REFUSED**, loudly and by name, instead of + * silently partially committing. The caller chooses explicitly: keep the + * objects of one transaction on one datasource, or split the work into + * per-datasource units and reconcile them. Refusing is the point — a + * caller who asked for atomicity must not be handed best-effort without + * being told (the same posture as `batchData`'s atomic gate). + * - **System writes carved out (#5351)**: objects whose `lifecycle.class` + * is `audit` / `telemetry` / `event` — the append-only ledgers ADR-0057 + * §3.6 routes to a dedicated datasource — are executed OUTSIDE the + * ambient transaction, on their own connection, with NO handle from + * another driver. They therefore SURVIVE a rollback of the business + * transaction ("orphan rows"): an audit row may describe a write that was + * rolled back. That is the deliberate direction of error for an + * append-only compliance ledger — an extra reconcilable row beats a + * missing row for a write that DID commit — and it is what lets a hook + * author write an ordinary `afterInsert` audit hook without knowing + * datasource routing exists. Recorded in the ADR-0067/ADR-0119 revision. + * + * ## The degrade is now a caller's choice, not a fixed caveat + * + * When the default driver has no `beginTransaction`, the callback still + * runs with NO transaction and NO rollback (warning once) — unchanged, and + * still declared. `opts.require: true` turns that degrade into a THROW for + * callers who cannot tolerate it; see {@link EngineTransactionOptions}. + * + * ## The callback is told whether it owns the transaction + * + * The second callback argument carries `owned` — `true` when this call + * opened the transaction, `false` when it joined an outer one (ADR-0067 + * D2). See {@link EngineTransactionInfo}. Existing one-argument callbacks + * are unaffected. * * `trxCtx`/`baseContext` are the engine-local execution-context shape, left * loose here per this file's edge-typing rule; consumers narrow at the call * site. */ - transaction(callback: (trxCtx: any) => Promise, baseContext?: any): Promise; + transaction( + callback: (trxCtx: any, info: EngineTransactionInfo) => Promise, + baseContext?: any, + opts?: EngineTransactionOptions, + ): Promise; }