From 8087a07a4433ae7cf59257cbab3fb843383e25f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 16:37:32 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(objectql):=20ScopedContext.transaction?= =?UTF-8?q?=20=E8=A1=A5=E9=BD=90=20ADR-0067=20D2=20=E7=9A=84=20ambient=20j?= =?UTF-8?q?oin=20(#6168)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectQL.transaction()` 的第一件事一直是 D2 的 join 判定:已有 ambient 事务 就在其中运行回调并报 `owned: false`,不再自开嵌套的 driver 事务。 `ScopedContext.transaction()` —— 同一原语的第二份实现,hook / action 体里的 `ctx.api.transaction(fn)` —— 没有这一支,直接取默认驱动 `beginTransaction()`。 它自己的 TSDoc 称自己是「同一件事的第二份实现」并逐条对齐了 ADR-0119 D1 的 caveat,唯独 join 这条一直没对齐。本次补上:同一分支、同一位置(在驱动查找和 `opts.require` 之前 —— ambient 事务本身就是事务,声明「不能没有事务」的调用者 由 join 满足)。 行为变化:内层从此随外层回滚。此前一个由 `engine.transaction()` 触发的 hook, 其体内的 `ctx.api.transaction(fn)` 会另开一个事务并自行 commit,写入因此 **存活过外层回滚** —— 调用者被告知工作单元已撤销,而其中若干行仍在,无报错也 无日志;同时它占住第二条连接,正是 D2 要避开的单连接池死锁。 join 只读引擎的 ambient store,因此离散 begin/commit/rollback 三件套(刻意不写 该 store)的显式句柄不可见、不会被误当成 ambient —— 该边界连同 #6167 的交叉 引用一并写入 TSDoc。 测试:`engine-ambient-transaction.test.ts` 新增 5 例(含回滚持久性钉子:内层写 在外层回滚后必须消失,由一个 rollback-honest 的 driver 双替按句柄暂存/丢弃来 测),`engine-transaction-contract.test.ts` 新增 2 例契约对齐, `engine-transaction-same-origin.test.ts` 新增 2 例证实 #5351 同源判定在 join 后 走外层属主、拒绝与豁免形状不变。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We --- ...scoped-context-transaction-ambient-join.md | 54 +++++ .../src/engine-ambient-transaction.test.ts | 223 +++++++++++++++++- .../src/engine-transaction-contract.test.ts | 46 +++- .../engine-transaction-same-origin.test.ts | 50 +++- packages/objectql/src/engine.ts | 87 +++++-- 5 files changed, 443 insertions(+), 17 deletions(-) create mode 100644 .changeset/scoped-context-transaction-ambient-join.md diff --git a/.changeset/scoped-context-transaction-ambient-join.md b/.changeset/scoped-context-transaction-ambient-join.md new file mode 100644 index 0000000000..0c245e20ec --- /dev/null +++ b/.changeset/scoped-context-transaction-ambient-join.md @@ -0,0 +1,54 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): `ctx.api.transaction()` joins an open ambient transaction instead of opening a second one (#6168) + +`ObjectQL.transaction()` has always started with the ADR-0067 D2 join: if an +ambient transaction is already open, it runs the callback inside that one and +reports `owned: false` rather than beginning a nested driver transaction. +`ScopedContext.transaction()` — the second implementation of the same +primitive, reached as `ctx.api.transaction(fn)` from hook and action bodies — +did not. It went straight to the default driver and called `beginTransaction()` +unconditionally. + +Its own TSDoc called it "a second implementation of the same thing" and lined +up against ADR-0119 D1's caveats one by one; the join was the single point that +never got aligned. That is now fixed, with the same branch, in the same +position — before the driver lookup and before `opts.require`, because an +ambient transaction *is* a transaction and a caller who declared they cannot run +without one is served by joining it. + +**Behaviour change — a nested sandbox/hook transaction now rolls back with the +outer one.** Previously a hook fired from inside an `engine.transaction()` +whose body called `ctx.api.transaction(fn)` got a **separate** driver +transaction. That transaction committed itself, so its writes **survived a +rollback of the outer one**: the caller was told the unit of work had been +undone while some of its rows were still there, with nothing failing and +nothing logged. It also took a second connection for the duration — the +deadlock ADR-0067 D2 exists to avoid on a single-connection pool such as +SQLite's. After this change the inner call joins, writes on the outer handle, +and is undone by the outer rollback. + +If you have a hook or action body that used `ctx.api.transaction()` inside a +larger transaction *specifically* to get an independently-committing unit — +an audit trail that must outlive a rollback, say — it no longer does. The +supported way to have a write survive a rollback is the ADR-0057 §3.6 system +ledger carve-out (`lifecycle.class` of `audit` / `telemetry` / `event`), which +routes the row to its own datasource and executes it outside the transaction by +decision rather than by accident. + +The callback's `owned` signal (#5696) now reports `false` on this path, as it +already did on the engine surface. It was never wrong before — this surface +really did always open its own transaction — but what it honestly described was +the defect. + +Two limits stay as they are, and are now stated in the method's TSDoc. The join +reads the engine's ambient `AsyncLocalStorage` store only, so the discrete +`beginTransaction`/`commit`/`rollback` trio — which deliberately never +populates that store, because its handle is threaded explicitly across +`setImmediate` boundaries — is invisible to it and is not joined. That is what +keeps the branch from mistaking an explicitly-threaded handle for an ambient +one. The QuickJS sandbox drives its VM-side `ctx.api.transaction(fn)` through +that trio rather than through this method, so a VM-side body is outside this +join; unattributable handles are tracked separately in #6167. diff --git a/packages/objectql/src/engine-ambient-transaction.test.ts b/packages/objectql/src/engine-ambient-transaction.test.ts index 4318169087..a058742f43 100644 --- a/packages/objectql/src/engine-ambient-transaction.test.ts +++ b/packages/objectql/src/engine-ambient-transaction.test.ts @@ -7,7 +7,7 @@ // and deadlock on the single-connection SQLite pool. import { describe, it, expect, beforeEach } from 'vitest'; -import { ObjectQL } from './engine.js'; +import { ObjectQL, ScopedContext } from './engine.js'; function makeRecordingDriver() { const stores = new Map>(); @@ -133,3 +133,224 @@ describe('engine ambient transaction (ADR-0034)', () => { expect(seen.commit.length).toBe(0); }); }); + +// --------------------------------------------------------------------------- +// #6168 — the SECOND implementation of the same primitive joins too +// --------------------------------------------------------------------------- +// +// `ScopedContext.transaction` — what a hook or action body reaches as +// `ctx.api.transaction(fn)` — had no ADR-0067 D2 join branch. Inside an +// `engine.transaction()` it opened a SECOND driver transaction: a second +// connection (the deadlock D2 exists to avoid on a single-connection pool), +// committed independently, and therefore NOT covered by the outer rollback. +// +// The driver below is deliberately ROLLBACK-HONEST where the one above is only +// rollback-RECORDING: writes carrying a handle are staged per handle, `commit` +// flushes them into the committed store and `rollback` discards them. That is +// the difference between pinning "rollback was called" and pinning the fact +// this issue is actually about — whether the inner row is still there +// afterwards. With the join removed, the inner transaction commits its own +// stage and the row SURVIVES the outer rollback; the assertion that fails is +// the one on committed state, in the shape of leftover residue. + +function makeRollbackHonestDriver() { + const committed = new Map>(); + const staged = new Map>(); + const seen = { + begins: [] as unknown[], + commits: [] as unknown[], + rollbacks: [] as unknown[], + creates: [] as Array<{ object: string; transaction: unknown }>, + }; + let nextId = 0; + let nextTrx = 0; + const committedFor = (o: string) => { + let s = committed.get(o); + if (!s) { s = new Map(); committed.set(o, s); } + return s; + }; + const driver: any = { + name: 'memory', + version: '0.0.0', + supports: {}, + async connect() {}, + async disconnect() {}, + async checkHealth() { return true; }, + async execute() { return null; }, + // Reads see COMMITTED state only. Nothing in these cases reads back its own + // uncommitted write, and keeping it simple keeps the durability assertion + // unambiguous: what `find` returns at the end is what actually landed. + async find(object: string) { return Array.from(committedFor(object).values()); }, + async findOne(object: string) { + for (const r of committedFor(object).values()) return r; + return null; + }, + async create(object: string, data: Record, options: any) { + const trx = options?.transaction; + seen.creates.push({ object, transaction: trx }); + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + if (trx === undefined) committedFor(object).set(id, row); + else staged.set(trx, [...(staged.get(trx) ?? []), { object, row }]); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = committedFor(object); + const row = { ...s.get(id), ...data, id }; + s.set(id, row); + return row; + }, + async delete(object: string, id: string) { return committedFor(object).delete(id); }, + async count() { return 0; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r, undefined))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async syncSchema() {}, + async beginTransaction() { + nextTrx += 1; + const handle = { __trx: nextTrx }; + seen.begins.push(handle); + staged.set(handle, []); + return handle; + }, + async commit(trx: unknown) { + seen.commits.push(trx); + for (const { object, row } of staged.get(trx) ?? []) committedFor(object).set(row.id, row); + staged.delete(trx); + }, + async rollback(trx: unknown) { + seen.rollbacks.push(trx); + staged.delete(trx); + }, + }; + const committedNames = (object: string) => + Array.from(committedFor(object).values()).map((r) => r.name).sort(); + return { driver, seen, committedNames }; +} + +describe('ScopedContext.transaction joins the ambient transaction (ADR-0067 D2, #6168)', () => { + let engine: ObjectQL; + let seen: ReturnType['seen']; + let committedNames: ReturnType['committedNames']; + + beforeEach(async () => { + engine = new ObjectQL(); + const d = makeRollbackHonestDriver(); + seen = d.seen; + committedNames = d.committedNames; + engine.registerDriver(d.driver, true); + await engine.init(); + engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any); + }); + + /** + * A hook body's `ctx.api.transaction(fn)`, registered on `thing` and fired by + * a write made inside `engine.transaction()`. This is the real reachability + * path — `HookContext.api` IS a `ScopedContext` (`ObjectQL.buildHookApi`) — + * not a `createContext` stand-in for one. + */ + function hookThatOpensATransaction( + body: (trxCtx: any, info: any) => Promise, + onlyFor = 'outer', + ) { + (engine as any).registerHook( + 'afterInsert', + async (ctx: any) => { + // The inner write fires this hook again; without the guard the join + // would be measured against a recursion, not against the outer call. + if (ctx.result?.name !== onlyFor) return; + await ctx.api.transaction(body); + }, + { object: 'thing' }, + ); + } + + it('joins the outer transaction — same handle, owned: false, no second begin', async () => { + let innerHandle: unknown; + let innerOwned: boolean | undefined; + let outerHandle: unknown; + + hookThatOpensATransaction(async (trxCtx: any, info: any) => { + innerHandle = trxCtx.transactionHandle; + innerOwned = info.owned; + }); + + await engine.transaction(async (ctx: any) => { + outerHandle = ctx.transaction; + await engine.insert('thing', { name: 'outer' }); + }); + + expect(innerOwned).toBe(false); + expect(innerHandle).toBe(outerHandle); + // The whole point of D2: ONE begin, so ONE connection. + expect(seen.begins).toHaveLength(1); + expect(seen.commits).toHaveLength(1); + }); + + it('the joined write is UNDONE by the outer rollback — the durability pin', async () => { + hookThatOpensATransaction(async (trxCtx: any) => { + await trxCtx.object('thing').insert({ name: 'inner' }); + }); + + await expect( + engine.transaction(async () => { + await engine.insert('thing', { name: 'outer' }); + throw new Error('outer boom'); + }), + ).rejects.toThrow('outer boom'); + + // Before #6168 the inner transaction committed itself, so 'inner' was still + // here after the outer rollback: a write the caller was told had been undone + // and had not been. Nothing failed, nothing was logged. + expect(committedNames('thing')).toEqual([]); + expect(seen.rollbacks).toHaveLength(1); + expect(seen.commits).toHaveLength(0); + // Both writes rode the ONE handle the outer call owns. + expect(seen.creates).toHaveLength(2); + expect(seen.creates[1].transaction).toBe(seen.creates[0].transaction); + }); + + it('the joined write commits with the outer one when the outer succeeds', async () => { + hookThatOpensATransaction(async (trxCtx: any) => { + await trxCtx.object('thing').insert({ name: 'inner' }); + }); + + await engine.transaction(async () => { + await engine.insert('thing', { name: 'outer' }); + }); + + expect(committedNames('thing')).toEqual(['inner', 'outer']); + expect(seen.begins).toHaveLength(1); + }); + + it('with NO ambient transaction it still OPENS one — owned: true, unchanged', async () => { + const scoped = (engine as any).createContext({ userId: 'u1' }) as ScopedContext; + let owned: boolean | undefined; + + await scoped.transaction(async (trxCtx: any, info: any) => { + owned = info.owned; + await trxCtx.object('thing').insert({ name: 'standalone' }); + }); + + expect(owned).toBe(true); + expect(seen.begins).toHaveLength(1); + expect(seen.commits).toHaveLength(1); + expect(committedNames('thing')).toEqual(['standalone']); + }); + + it('does not join a transaction that has already closed — the store does not leak', async () => { + await engine.transaction(async () => { + await engine.insert('thing', { name: 'covered' }); + }); + + const scoped = (engine as any).createContext({ userId: 'u1' }) as ScopedContext; + let owned: boolean | undefined; + await scoped.transaction(async (_ctx: any, info: any) => { owned = info.owned; }); + + expect(owned).toBe(true); + expect(seen.begins).toHaveLength(2); // the outer one, then a fresh one + }); +}); diff --git a/packages/objectql/src/engine-transaction-contract.test.ts b/packages/objectql/src/engine-transaction-contract.test.ts index b45f0fe030..0f27995b6a 100644 --- a/packages/objectql/src/engine-transaction-contract.test.ts +++ b/packages/objectql/src/engine-transaction-contract.test.ts @@ -309,7 +309,7 @@ describe('ScopedContext.transaction (ctx.api.transaction) carries the same two p await expect(scopedOf(engine).transaction(async () => 'ran')).resolves.toBe('ran'); }); - it('reports owned: true (this surface always opens) and false when degraded', async () => { + it('reports owned: true when it opens one, and false when degraded', async () => { const withTx = await engineWith({ transactional: true }); const withoutTx = await engineWith({ transactional: false }); let ownedOpen: boolean | undefined; @@ -321,4 +321,48 @@ describe('ScopedContext.transaction (ctx.api.transaction) carries the same two p expect(ownedOpen).toBe(true); expect(ownedDegraded).toBe(false); }); + + // The third point of parity, added by #6168. Until then this was the ONE + // place the second implementation still diverged: it had no ADR-0067 D2 join + // branch, so `owned` reported `true` here forever — honest about a behaviour + // that was not. Durability coverage lives in `engine-ambient-transaction.test.ts` + // (the inner write must be undone by the outer rollback); what is pinned here + // is the CONTRACT parity — same handle, same signal, same ordering. + it('JOINS an ambient transaction rather than opening a second one (ADR-0067 D2, #6168)', async () => { + const { engine, driver } = await engineWith({ transactional: true }); + let outerHandle: unknown; + let innerHandle: unknown; + let innerOwned: boolean | undefined; + + await engine.transaction(async (ctx: any) => { + outerHandle = ctx.transaction; + await scopedOf(engine).transaction(async (trxCtx, info) => { + innerHandle = (trxCtx as unknown as { transactionHandle: unknown }).transactionHandle; + innerOwned = info.owned; + }); + }); + + // `beginTransaction` mints a fresh object per call, so identity is what + // separates "joined the outer handle" from "opened an identical-looking one". + expect(innerOwned).toBe(false); + expect(innerHandle).toBe(outerHandle); + expect(driver.writes).toHaveLength(0); + }); + + it('joins under require: true — an ambient transaction satisfies it, nothing is refused', async () => { + const { engine } = await engineWith({ transactional: true }); + let owned: boolean | undefined; + + // The join sits BEFORE the driver/`require` handling on both surfaces, and + // it must: when there is an ambient transaction there IS a transaction, so + // a caller who declared it cannot run without one is served by joining. + await engine.transaction(async () => { + await scopedOf(engine).transaction( + async (_ctx, info) => { owned = info.owned; }, + { require: true }, + ); + }); + + expect(owned).toBe(false); + }); }); diff --git a/packages/objectql/src/engine-transaction-same-origin.test.ts b/packages/objectql/src/engine-transaction-same-origin.test.ts index 5388e9fe27..da4b26c605 100644 --- a/packages/objectql/src/engine-transaction-same-origin.test.ts +++ b/packages/objectql/src/engine-transaction-same-origin.test.ts @@ -370,7 +370,7 @@ describe('a BUSINESS write across drivers inside a transaction is refused (#5696 ).rejects.toMatchObject({ transactionDatasource: 'primary' }); }); - it('refuses from ScopedContext.transaction too (ctx.api.transaction in a sandboxed hook body)', async () => { + it('refuses from ScopedContext.transaction too (ctx.api.transaction in a hook body)', async () => { const { engine } = await splitEngine(); const scoped = (engine as any).createContext({ userId: 'u1' }) as ScopedContext; @@ -379,6 +379,54 @@ describe('a BUSINESS write across drivers inside a transaction is refused (#5696 ).rejects.toBeInstanceOf(CrossDatasourceTransactionWriteError); }); + it('refuses from a ScopedContext.transaction that JOINED the outer one, against the OUTER owner (#6168)', async () => { + const { engine } = await splitEngine(); + + // #6168 made `ctx.api.transaction` join an ambient transaction instead of + // opening a second one. The joined callback is handed the OUTER handle, so + // the same-origin gate attributes it to the outer scope by identity + // (`transactionCoversDriverFor`) and judges the write exactly as it judges + // a write made directly in the outer call: refused, and refused naming the + // OUTER datasource. The join changes which transaction the write is in, not + // how #5351 measures it — there is no new interaction between the two. + const err = await rejection( + engine.transaction(async (outerCtx: any) => { + const scoped = (engine as any).createContext({ + userId: 'u1', + transaction: outerCtx.transaction, + }) as ScopedContext; + await scoped.transaction(async (trxCtx: any) => { + await trxCtx.object('ledger').insert({ name: 'joined' }); + }); + }), + ); + + expect(err).toBeInstanceOf(CrossDatasourceTransactionWriteError); + expect(err.transactionDatasource).toBe('primary'); + expect(err.operation).toBe('insert'); + }); + + it('carves an audit row out of a JOINED ScopedContext.transaction the same way (#6168)', async () => { + const { engine, telemetry, carveOutNotes } = await splitEngine(); + + // The carve-out half of the same ruling, measured through the joined + // surface: a system ledger still executes OUTSIDE the transaction, on its + // own connection, with no foreign handle. + await engine.transaction(async (outerCtx: any) => { + const scoped = (engine as any).createContext({ + userId: 'u1', + transaction: outerCtx.transaction, + }) as ScopedContext; + await scoped.transaction(async (trxCtx: any) => { + await trxCtx.object('sys_audit_log').insert({ action: 'joined audit' }); + }); + }); + + expect(telemetry.writes).toHaveLength(1); + expect(telemetry.writes[0].transaction).toBeUndefined(); + expect(carveOutNotes()).toHaveLength(1); + }); + it('does NOT refuse a mapped write made outside any transaction — no false positive', async () => { const { engine, ledgerDb } = await splitEngine(); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index e8fa97d196..125f0b1e5b 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -7491,22 +7491,83 @@ export class ScopedContext implements IScopedContext { * Carries BOTH of `ObjectQL.transaction`'s declared caveats (ADR-0119 D1) — * default-datasource-only, and a silent degrade when that driver has no * `beginTransaction` — because it is a second implementation of the same - * thing, reached from `ctx.api.transaction(fn)` in sandboxed hook and action - * bodies. Behaviour is unchanged, and so is the split: since #4619 both - * 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. + * thing, reached from `ctx.api.transaction(fn)` in hook and action bodies. + * Behaviour is unchanged, and so is the split: since #4619 both caveats + * report through the SAME engine-side helpers the engine's own + * `transaction()` uses, so this 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. + * + * And so, since #6168, is the **ADR-0067 D2 join** — the first thing this + * method does, exactly as on the engine surface. It was the one point where + * the second implementation still diverged, and it diverged in the direction + * that costs the most: a hook triggered from inside an `engine.transaction()` + * that called `ctx.api.transaction(fn)` opened a SECOND driver transaction, + * which (a) takes a second connection — the deadlock D2 exists to avoid on a + * single-connection pool like SQLite's — and (b) committed itself, so its + * writes SURVIVED the outer rollback. D2's whole point is that the outermost + * caller owns the one-and-only commit/rollback for every write made through + * nested helpers. The `owned` signal was already honest about this + * (`true` every time, because this surface really did always open); what was + * wrong is the behaviour it was honestly describing. + * + * DECLARED LIMIT, so the next reader does not mistake it for the same + * oversight: the join reads the engine's ambient `txStore` only. The discrete + * `beginTransaction`/`commit`/`rollback` trio below deliberately does not + * populate that store (its handle is threaded explicitly across + * `setImmediate` boundaries where AsyncLocalStorage does not survive), so a + * trio-held handle is invisible here and is NOT joined — which is what keeps + * this branch from mistaking an explicitly-threaded handle for an ambient + * one. The QuickJS sandbox drives `ctx.api.transaction(fn)` through that trio + * rather than through this method, so a VM-side body is outside this join; + * unattributable handles are the same surface #6167 tracks, and closing that + * needs handle ownership to become discoverable on `IDataDriver`. */ async transaction( callback: (trxCtx: ScopedContext, info: EngineTransactionInfo) => Promise, opts?: EngineTransactionOptions, ): Promise { const engine = this.engine as any; + // The engine's ambient transaction store (ADR-0034), reached the `as any` + // way this whole class reaches engine internals. One accessor serves both + // readers below: the D2 join, and the `run` that publishes a transaction + // this call opens. + const txStore = engine?.txStore as + | { + getStore(): { transaction: unknown } | undefined; + run(s: { transaction: unknown; scope?: unknown }, fn: () => R): R; + } + | undefined; + + // ADR-0067 D2 — JOIN an already-open ambient transaction instead of opening + // a nested driver one (#6168). Same first move, same reasons and the same + // shape as `ObjectQL.transaction`: a nested begin would take a second + // connection AND would not be covered by the outer rollback, so the outer + // caller would stop owning the one-and-only commit/rollback. + // + // BEFORE the driver/`require` handling on purpose, mirroring the engine: + // when there is an ambient transaction there IS a transaction, so + // `require: true` is satisfied by joining it and the degrade is not + // reachable. + const ambient = txStore?.getStore(); + if (ambient?.transaction) { + // The handle is threaded EXPLICITLY into the child context, not left to + // the ambient store, for the same reason the engine surface threads it: + // `buildDriverOptions` prefers the explicit handle, and it survives async + // boundaries the store does not. It is identity-equal to the store's + // handle, so `transactionCoversDriverFor` still attributes it to the + // OUTER owner and the #5351 same-origin gate judges it unchanged. + const joinedCtx = new ScopedContext( + { ...this.executionContext, transaction: ambient.transaction }, + this.engine + ); + // JOINED, not owned: the outer caller decides commit vs rollback (#5696). + return callback(joinedCtx, { owned: false }); + } // Find the default driver for transaction support const driver = engine.defaultDriver @@ -7531,21 +7592,19 @@ export class ScopedContext implements IScopedContext { { ...this.executionContext, transaction: trx }, this.engine ); - // Share the engine's ambient transaction store so internal queries during - // writes reuse this transaction's connection (ADR-0034). The store entry - // also carries WHICH driver owns the transaction (#4619) so the write path - // can report a write routed off it; `newTransactionScope` is the engine's, + // Publish this transaction into the engine's ambient store so internal + // queries during writes reuse its connection (ADR-0034) — and so a nested + // `transaction()` on either surface can JOIN it. The store entry also + // carries WHICH driver owns the transaction (#4619) so the write path can + // report a write routed off it; `newTransactionScope` is the engine's, // reached the same `as any` way as `txStore` itself. - const txStore = (this.engine as any)?.txStore as - | { run(s: { transaction: unknown; scope?: unknown }, fn: () => R): R } - | undefined; const scope = engine.newTransactionScope?.(driver); const runIn = (fn: () => Promise): Promise => txStore ? txStore.run({ transaction: trx, scope }, fn) : fn(); try { - // This surface always OPENS (it has no ADR-0067 D2 join branch of its - // own), so a callback that reaches here owns the outcome. + // Reached only with no ambient transaction to join, so this call really + // did open one and the callback owns the outcome (#5696 / #6168). const result = await runIn(() => callback(trxCtx, { owned: true })); if (driver.commit) await driver.commit(trx); else if (driver.commitTransaction) await driver.commitTransaction(trx); From e1e56cce5ae4d3178de21c090449a203f3fa15a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 16:48:47 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(objectql):=20=E5=8D=95=E8=BF=9E?= =?UTF-8?q?=E6=8E=A5=E6=B1=A0=E9=92=89=E5=AD=90=20=E2=80=94=E2=80=94=20joi?= =?UTF-8?q?n=20=E5=90=8E=E5=B5=8C=E5=A5=97=E8=B0=83=E7=94=A8=E4=B8=8D?= =?UTF-8?q?=E5=86=8D=E7=94=B3=E8=AF=B7=E7=AC=AC=E4=BA=8C=E6=9D=A1=E8=BF=9E?= =?UTF-8?q?=E6=8E=A5=20(#6168)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0067 D2 理由的另一半:第二次 `beginTransaction` 会向连接池要第二条连接, 而单连接池(D2 点名的 knex/SQLite 那个)给不出来。真实池在此**阻塞**——那就是 死锁——但如实建模阻塞的测试只能靠 vitest 超时判红:慢,且并行下不稳。 故双替建模「size=1 且带 acquire 超时」的池:第二次签出直接拒绝而不是排队。 拒绝是对挂起的替身;两种建模下被如实测量的都是同一个致因——到底有没有去要 第二条连接。 反向验证:移除 join 支后本例翻红,报 `pool exhausted: no connection available (max=1)`,栈为 registerHook handler → ScopedContext.transaction → beginTransaction,正是 issue 所述机制。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We --- .../src/engine-ambient-transaction.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/packages/objectql/src/engine-ambient-transaction.test.ts b/packages/objectql/src/engine-ambient-transaction.test.ts index a058742f43..d06eda9c68 100644 --- a/packages/objectql/src/engine-ambient-transaction.test.ts +++ b/packages/objectql/src/engine-ambient-transaction.test.ts @@ -341,6 +341,55 @@ describe('ScopedContext.transaction joins the ambient transaction (ADR-0067 D2, expect(committedNames('thing')).toEqual(['standalone']); }); + /** + * The OTHER half of ADR-0067 D2's rationale: the second `beginTransaction` + * asks the pool for a second connection, and on a single-connection pool + * (the knex/SQLite one D2 names) there is no second connection to give. + * + * The real pool BLOCKS there — that is the deadlock — and a test that + * modelled the block faithfully would fail only by hitting vitest's timeout: + * slow, and flaky under parallel load. So this double models a pool of size 1 + * WITH an acquire timeout, which refuses the second checkout instead of + * queueing for it. The refusal is a stand-in for the hang; what is measured + * honestly either way is the thing that causes both — whether a second + * connection is asked for at all. + */ + it('never asks for a second connection — a single-connection pool survives the nested call', async () => { + let checkedOut = false; + const checkouts: number[] = []; + const d = makeRollbackHonestDriver(); + d.driver.beginTransaction = async () => { + if (checkedOut) { + // Where a real single-connection pool would wait forever. + throw new Error('pool exhausted: no connection available (max=1)'); + } + checkedOut = true; + checkouts.push(checkouts.length + 1); + return { __trx: 'only' }; + }; + d.driver.commit = async () => { checkedOut = false; }; + d.driver.rollback = async () => { checkedOut = false; }; + + const oneConn = new ObjectQL(); + oneConn.registerDriver(d.driver, true); + await oneConn.init(); + oneConn.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any); + (oneConn as any).registerHook( + 'afterInsert', + async (ctx: any) => { + if (ctx.result?.name !== 'outer') return; + await ctx.api.transaction(async () => { /* joins — asks for nothing */ }); + }, + { object: 'thing' }, + ); + + await expect( + oneConn.transaction(async () => { await oneConn.insert('thing', { name: 'outer' }); }), + ).resolves.toBeUndefined(); + + expect(checkouts).toHaveLength(1); + }); + it('does not join a transaction that has already closed — the store does not leak', async () => { await engine.transaction(async () => { await engine.insert('thing', { name: 'covered' });