diff --git a/.changeset/transaction-same-origin-audit-carve-out.md b/.changeset/transaction-same-origin-audit-carve-out.md new file mode 100644 index 0000000000..94c652052f --- /dev/null +++ b/.changeset/transaction-same-origin-audit-carve-out.md @@ -0,0 +1,57 @@ +--- +"@objectstack/objectql": minor +--- + +fix(objectql)!: 事务句柄不再跨数据源穿透 —— 业务写响亮拒绝、系统账本移出事务落盘 (#5351, #5696) + +**这是行为变化,升级前请读完。** 只影响**注册了第二个数据源**的部署;单数据源部署 +(绝大多数)**行为一字未变**,不拒绝、不 carve-out、不打日志。 + +## 修的是什么 + +`buildDriverOptions` 此前把 ambient 事务句柄**无条件**塞进每一次 driver 调用,不问 +即将收到它的是哪个 driver。于是被路由到别处的对象(`setDatasourceMapping`、显式 +`datasource:` 绑定,或 ADR-0057 §3.6 的 lifecycle 分流)拿到的是**默认库那条连接的 +事务对象**,knex 的 `.transacting(trx)` 把语句发到了错误的库。 + +实测后果(#5351,一次真实 boot):`sys_audit_log` 被 §3.6 路由到 `telemetry` 数据源, +insert 尝试 52 次、成功 50 次、失败 2 次,失败的两次堆栈**全部**带 knex 的 +`trxClient.query` 帧,报 `no such table: sys_audit_log`。也就是说 —— **凡是在事务中执行 +的被审计写入,合规审计行全部静默丢失**:业务写成功、接口 200、数据在盘上,只有「谁做的」 +那一行没了,且无人重试。契约 TSDoc 原先写这类写入「在事务外执行」,比实际情况乐观。 + +## 三条新行为 + +1. **句柄不再跨驱动**。事务句柄只交给开启它的那个 driver(按**实例身份**比对)。读操作 + 同样覆盖 —— 它们此前也在错误的连接上跑,而且连诊断都没有。 +2. **业务写跨驱动 → 拒绝**。抛 `CrossDatasourceTransactionWriteError` + (`code: 'ERR_CROSS_DATASOURCE_TRANSACTION_WRITE'`),在任何 hook / 默认值 / 校验之前, + **一行都没写**。⛔ 这不是跨库原子性:`IDataDriver` 没有两阶段提交,本次刻意不做 + (#4619 原文已排除)。 + **升级须知**:此前这类写入会静默部分提交(而且是在错的连接上)。现在它会失败。两条修法, + 错误消息里都写了 —— 要么让一个 `transaction()` 里写的对象都留在同一个数据源(移动对象, + 或删掉把它路由走的 `datasourceMapping` 规则),要么把工作拆成按数据源的独立单元,由调用方 + 自己对账。 +3. **系统账本移出事务执行(carve-out)**。`lifecycle.class` 为 `audit` / `telemetry` / + `event` 的只追加账本**不拒绝**,而是在自己的连接上、事务之外执行 —— 所以审计行**真正 + 落盘**,插件作者写普通 `afterInsert` 钩子零负担。 + ⚠️ **孤儿行语义**:这些行会**在业务事务回滚后留下** —— 一条审计行可能描述一次被撤销的 + 写入。这是维护者 2026-08-06 明确接受的代价:对只追加的合规账本,「多记一条可对账的行」 + 优于「已提交的写入却少一行」,而后者正是此前在发的版本。判别式是对象**声明**的 + `lifecycle.class`(不是它被哪种机制路由走的),ADR-0067 / ADR-0119 的 2026-08-06 修订 + 记录了全部理由与边界。 + +## 同时移除 + +PR #5724 为跨数据源写入加的那条 `error` 级日志随之退休 —— 已经没有「静默穿错连接」可报了。 +carve-out 路径改为 `debug` 级、每事务每数据源一次:它现在是**声明过的正常行为**,在分流部署 +里每一次被审计的事务写入都会发生,挂在 `error`/`warn` 上只会训练读者跳过真正的持久性告警。 + +## 已知边界(明确不覆盖,非疏漏) + +引擎无法归属的事务句柄不参与同源校验:`ScopedContext` 的离散 +`beginTransaction`/`commit`/`rollback` 三件套(跨 `setImmediate` 显式穿句柄,不走 txStore), +以及外部调用方自带的 `execCtx.transaction`。这类句柄是不透明的 driver 对象,没有回指其属主的 +引用,猜测(比如假定它属于默认驱动)会在一边误拒合法的单库工作、在另一边误放真正被覆盖的写入。 +这条路径保持 #5351 之前的行为,已作为决定钉进测试,收口需要 `IDataDriver` 暴露句柄属主 +(另单跟踪)。 diff --git a/docs/adr/0067-commit-history-and-rollback-for-ai-authoring.md b/docs/adr/0067-commit-history-and-rollback-for-ai-authoring.md index 5f9c994970..b19176ddc5 100644 --- a/docs/adr/0067-commit-history-and-rollback-for-ai-authoring.md +++ b/docs/adr/0067-commit-history-and-rollback-for-ai-authoring.md @@ -1,6 +1,6 @@ # ADR-0067: Commit history and rollback for AI authoring — turns become atomic, revertible commits -**Status**: Accepted (2026-06-24; completed 2026-07-16) — fully implemented: commit grouping (`sys_metadata_commit`), `revertCommit`/`rollbackToPackageCommit`/`listCommits`, REST routes; **Decision-2 landed via #3066**: `publishPackageDrafts` runs every promotion + the commit record inside ONE `engine.transaction()` (two-phase — side effects post-commit), so a commit cannot half-land; `engine.transaction()` joins ambient transactions to make nested repository writes participate. Locked by `protocol-publish-package-drafts.test.ts` (all-or-nothing + rollback tracking) and `engine-ambient-transaction.test.ts`. +**Status**: Accepted (2026-06-24; completed 2026-07-16) · **Amended** (2026-08-06, #5351/#5696 — the D2 join is now distinguishable (`owned`), and Decision-2's atomicity is enforced as a ONE-datasource promise, with audit rows carved out and possibly orphaned; see the Amendment at the end) — fully implemented: commit grouping (`sys_metadata_commit`), `revertCommit`/`rollbackToPackageCommit`/`listCommits`, REST routes; **Decision-2 landed via #3066**: `publishPackageDrafts` runs every promotion + the commit record inside ONE `engine.transaction()` (two-phase — side effects post-commit), so a commit cannot half-land; `engine.transaction()` joins ambient transactions to make nested repository writes participate. Locked by `protocol-publish-package-drafts.test.ts` (all-or-nothing + rollback tracking) and `engine-ambient-transaction.test.ts`. **Deciders**: ObjectStack Protocol Architects **Builds on / amends**: [ADR-0045](./0045-additive-materialization-and-visibility-gate.md) (**amended**: ADR-0045 keeps a *draft + human-confirm* gate on mutations as the safety mechanism; this ADR replaces *confirm-before* with *revert-after* for everything except irreversible data loss, and unifies the two authoring regimes under one primitive — the commit), [ADR-0027](./0027-metadata-authoring-lifecycle.md) (draft workspace — retained as a *review affordance*, demoted from *safety mechanism*), [ADR-0033](./0033-ai-assisted-metadata-authoring.md) ("AI never publishes — it drafts" → **AI commits; commits are revertible**), [ADR-0034](./0034-transactional-writes-and-ambient-transaction.md) (per-write transaction — **extended to span a whole turn**), [ADR-0038](./0038-build-verification-loop.md) (machine gate — runs per commit, before it lands) **Consumers**: `@objectstack/objectql` (commit grouping, atomic turn-apply, `revertCommit`, history query — built on the existing `sys_metadata_history` + `restoreVersion`), `@objectstack/runtime` + `@objectstack/rest` (commit/revert routes), `../cloud/service-ai-studio` (turn = commit; auto-commit policy; data-loss confirmation), `../objectui` (commit timeline + "revert to here") @@ -160,3 +160,65 @@ Acceptance, browser-level: *build an app (commit 1) → ask the AI to change it 2. **Reverting an additive commit that holds real user data** → **allowed, tiered** (§Decision-5): silent when only sample data; typed-confirmation + auto-snapshot escape hatch when user-entered rows exist. Reversibility-by-recovery, not safety-by-prohibition — hard-blocking ("export first") is paternalistic and contradicts the friction-removal goal. 3. **Per-turn full-bundle snapshot** → **rejected** (Option B); `sys_metadata_history` is the commit substrate, `sys_package_version` is reserved for named restore points, cut in v1.1, not per turn. 4. **Does the draft workspace go away?** → **No, but it is demoted.** ADR-0027's draft + `?preview=draft` diff review stays as a *review affordance* (governed orgs, optional preview); it is no longer the *safety mechanism* for the default path. Revertibility is. + +--- + +## Amendment (2026-08-06, #5351 / #5696) — the D2 join is now distinguishable, and "a commit cannot half-land" is scoped to one datasource + +Decision-2 ("commits are atomic") and its join rule — `engine.transaction()` +JOINS an already-open ambient transaction so the outermost caller owns the one +and only commit/rollback — are both **unchanged and reaffirmed**. A nested +`begin` would take a second connection (a deadlock on the single-connection +SQLite pool) and would not be covered by the outer rollback, which is the exact +half-landing the join prevents. Two things around it are amended. + +### The join is now distinguishable by the callback (#5696 point 3) + +The join was correct but **silent**: a nested caller could not tell whether it +owned the transaction it was running in, and a helper whose own contract reads +"this all rolls back together" was making a promise it might not control. The +callback's second argument now carries `{ owned: boolean }` — `true` when this +call opened the transaction, `false` when it joined an outer one or ran on the +no-transaction degrade path. Nothing about who commits changed; only whether the +callback can find out. + +### "A commit cannot half-land" is a promise about ONE datasource + +Decision-2's atomicity was always scoped to the driver the transaction was +opened on — ADR-0119 D1 says so — but the engine did not enforce that scope, and +the gap was worse than the wording suggested: a write routed to another +datasource was handed the FIRST driver's transaction handle and executed on the +wrong connection entirely (measured in #5351; see ADR-0119's 2026-08-06 +amendment for the full mechanism and evidence). Since that amendment: + +- the handle never reaches a driver that does not own it; +- a **business** write that would cross drivers inside a transaction is + **refused** by name, so a metadata commit spanning two datasources fails + loudly at the first crossing instead of half-landing invisibly — which is + Decision-2's own goal, now actually enforced rather than assumed; +- **append-only system ledgers** (`lifecycle.class` audit / telemetry / event, + routed away by ADR-0057 §3.6) are **carved out**: executed outside the + transaction, and therefore surviving its rollback. + +### Orphan rows are a deliberate consequence of the carve-out + +A reverted commit — `revertCommit` / `rollbackToPackageCommit` — restores the +metadata, but the **audit rows written during the reverted work remain**. They +are not undone, and this record now says so rather than leaving it to be +discovered. + +That is the correct direction of error here, and it fits this ADR's own design +center. History is append-only by construction — Decision-3 makes a revert a +*new forward commit* precisely so the record of what happened is never rewritten +— so an audit row describing work that was later undone is consistent with how +this ADR already treats history, not an exception to it. The alternative was +refusing the audit write, which an audit hook's `try/catch` turns straight back +into a dropped row; a spurious row is reconcilable against the commit history, +a missing row for a write that DID commit is not recoverable at all. + +**Decided by**: the maintainer, 2026-08-06, on #5351 (plan A) and #5696. +**Mechanism, limits and evidence**: ADR-0119's amendment of the same date — +including the one path the same-origin gate declines to judge (handles the +engine never opened and cannot attribute). **Implemented in** +`packages/objectql/src/engine.ts`; pinned by +`engine-transaction-same-origin.test.ts`. diff --git a/docs/adr/0119-plugin-reachable-transactions-and-honest-atomic-batch.md b/docs/adr/0119-plugin-reachable-transactions-and-honest-atomic-batch.md index 6ecd0d5778..731ddf008f 100644 --- a/docs/adr/0119-plugin-reachable-transactions-and-honest-atomic-batch.md +++ b/docs/adr/0119-plugin-reachable-transactions-and-honest-atomic-batch.md @@ -1,6 +1,6 @@ # ADR-0119: Multi-write atomicity is reachable through the contract, `atomic` means atomic or refuses, and migrations too big for one transaction get a journal runner -**Status**: Accepted (2026-08-02) — D1/D4 implemented in [#4623](https://github.com/objectstack-ai/objectstack/pull/4623): D1 in `packages/spec/src/contracts/objectql-engine.ts` (test `packages/objectql/src/protocol-batch-atomic.test.ts`), D4 in `packages/metadata-protocol/src/protocol.ts` (test `packages/metadata-protocol/src/protocol.batch-atomic.test.ts`). D2 tracked in [#4617](https://github.com/objectstack-ai/objectstack/issues/4617); D3 tracked in [#4618](https://github.com/objectstack-ai/objectstack/issues/4618) — neither is implemented, so this record is *not* wholly "implemented". +**Status**: Accepted (2026-08-02) · **Amended** (2026-08-06, #5351/#5696 — D1's two caveats decided; see "Amendment (2026-08-06)" at the end) — D1/D4 implemented in [#4623](https://github.com/objectstack-ai/objectstack/pull/4623): D1 in `packages/spec/src/contracts/objectql-engine.ts` (test `packages/objectql/src/protocol-batch-atomic.test.ts`), D4 in `packages/metadata-protocol/src/protocol.ts` (test `packages/metadata-protocol/src/protocol.batch-atomic.test.ts`). D2 tracked in [#4617](https://github.com/objectstack-ai/objectstack/issues/4617); D3 tracked in [#4618](https://github.com/objectstack-ai/objectstack/issues/4618) — neither is implemented, so this record is *not* wholly "implemented". **Renumbered**: published for one day as ADR-0118. Renumbered to 0119 because [ADR-0118 (非用户 actor 的平台契约)](./0118-non-user-actor-contract.md) merged first (10:37 vs 12:11 on 2026-08-02) and holds the number. Citations of "ADR-0118 D1/D2/D3/D4" written before 2026-08-03 mean this record. **Deciders**: ObjectStack Protocol Architects **Builds on**: [ADR-0034](./0034-transactional-writes-and-ambient-transaction.md) (the ambient `AsyncLocalStorage` transaction D1 declares — this ADR adds no mechanism to it), [ADR-0067](./0067-commit-history-and-rollback-for-ai-authoring.md) (D2 — the join-don't-nest rule that makes an outer transaction the sole owner of commit/rollback), [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove — the disposition method applied to `batch?` in D3 and to the `atomic` flag in D4), [ADR-0087](./0087-metadata-protocol-upgrade-contract.md) (D3's replayable migration chain — the metadata-side analogue of the data-side runner D2 specifies), [ADR-0008](./0008-metadata-repository-and-change-log.md) (the JSONL change log — the journal shape D2 deliberately does *not* reuse), [ADR-0060](./0060-conformance-ledger-platform-pattern.md) (framework-owned ledger pattern — the precedent for `sys_migration_journal`), [ADR-0117](./0117-owning-business-unit-record-stamp.md) (D8 — backfill plus a fail-closed enable gate, the migration posture D2 and D4 both inherit), [ADR-0078](./0078-no-silently-inert-metadata.md) (no silently inert declarations — why D2 rejects a pluggable journal store) @@ -186,3 +186,142 @@ the human-readable cause and causal row index. The D4 invariant is unchanged — a rolled-back batch reports zero successes and every row says what happened to it; only the encoding moved from a prefix convention a client had to regex to a code a client branches on. + +--- + +## Amendment (2026-08-06, #5351 / #5696) — D1's two caveats are decided: the handle never crosses drivers, business writes across drivers are refused, and system ledgers are carved out + +D1 wrote two caveats into the contract TSDoc and said plainly that "declaring a +caveat is not fixing it; tightening these is #4619." Both are now tightened. +This amendment records what they became and, as importantly, the one thing that +turned out to be **factually wrong** in the original wording. + +### The caveat text was wrong about what actually happened + +D1's TSDoc said that objects routed elsewhere by `setDatasourceMapping` "are +written outside" the transaction. They were not. `buildDriverOptions` lifted the +ambient handle onto **every** driver call without asking which driver was about +to receive it, so the second driver was handed the FIRST driver's transaction +object. On the in-memory doubles that is invisible; on knex it means +`.transacting(trx)` sends the statement down the owner's connection, into a +database that may not even contain the table. + +That is what #5351 measured on a real boot: `sys_audit_log` — routed to the +dedicated `telemetry` datasource by ADR-0057 §3.6 — took 52 insert attempts, 50 +succeeded, and the 2 failures were exactly the 2 whose stack carried a knex +`trxClient.query` frame, failing `no such table: sys_audit_log` against the +primary database. Every audited write performed **inside a transaction** lost +its compliance row, silently, with no retry: the business write succeeded, the +API returned 200, and only the record of who did it was gone. PR #5724 +reproduced the same handle crossing on pure in-memory doubles with no lifecycle +routing at all (`expected { __trx: 'primary' } to be undefined`), proving the +defect belongs to the transaction seam and not to audit or to SQL. + +### D1-R1 — a transaction handle never reaches a driver that does not own it + +`TransactionScope` (#4619 / PR #5724) already records the owning driver by +**instance identity** — names collide transiently in `registerDriver`, and +identity is what decides which connection a statement rides. +`buildDriverOptions` now consults it: the handle is threaded only when the +resolved driver IS the owner. This is structural and verb-agnostic — it covers +reads as well as writes, which had the same defect and no diagnostic of their +own. + +### D1-R2 — a cross-driver BUSINESS write inside a transaction is refused + +`CrossDatasourceTransactionWriteError` (`ERR_CROSS_DATASOURCE_TRANSACTION_WRITE`), +thrown at the top of `insert`/`update`/`delete` before any hook, default or +validation runs, so a refusal has cost the caller nothing. The message names +both datasources and both remedies: keep one transaction on one datasource, or +split into per-datasource units the caller reconciles. + +This is #5696 point 2 and it is **not** cross-driver atomicity. Two-phase commit +is not in `IDataDriver` and is deliberately out of scope (#4619 excludes it); +opening a companion transaction on the second driver would replace a known +durability risk with a worse one — two stores that can contradict each other +when the second commit fails. + +### D1-R3 — append-only SYSTEM LEDGERS are carved out, and may be orphaned + +Objects whose `lifecycle.class` is `audit`, `telemetry` or `event` — the +append-only ledgers ADR-0057 §3.6 routes to a dedicated datasource — are +**executed outside** the ambient transaction, on their own connection, rather +than refused. They therefore **survive a rollback** of the business +transaction: an audit row may describe a write that was undone. + +That cost is accepted deliberately, and the direction matters more than the +count. For an append-only compliance ledger a spurious row is a **reconcilable** +nuisance — it can be checked against the business data that is or is not there +— while a **missing** row for a write that DID commit is an unrecoverable +compliance hole, and the missing row is what was shipping. Refusal is also not +available here even in principle: the write is made by an `afterInsert` audit +hook whose `try/catch` turns any refusal back into a log line and drops the row +exactly as before. That coupling is why #5696's refusal and this carve-out had +to land in one batch rather than in PR order. + +The discriminator is the object's **declared** `lifecycle.class`, not the +routing mechanism that moved it. An audit ledger pinned to its own datasource by +an explicit `datasource:` binding is the same append-only ledger with the same +reason to be carved out; judging by mechanism would make a compliance guarantee +depend on which of three equivalent configurations an operator happened to +write. The class tuple lives in exactly one place +(`ObjectQL.SYSTEM_LEDGER_LIFECYCLE_CLASSES`), read by both the ADR-0057 §3.6 +routing step and this gate, because two copies would drift by one class and lose +precisely the row this change exists to save. + +### D1-R4 — `opts.require` makes the degrade a caller's choice + +The other caveat — a driver with no `beginTransaction` runs the callback with no +transaction and no rollback — keeps its default behaviour exactly (warn once, +#4619). `transaction(cb, base, { require: true })` turns it into a throw +(`TransactionUnsupportedError`, `ERR_TRANSACTION_UNSUPPORTED`) for callers whose +only reason to open a transaction is the rollback. This generalizes D4's +real-or-refused posture from `batchData` into the primitive itself. + +### D1-R5 — the callback is told whether it owns the transaction + +`transaction(cb, base, opts?)` passes `{ owned: boolean }` as the callback's +second argument: `true` when this call opened the transaction, `false` when it +JOINED an outer one (ADR-0067 D2) or ran on the degrade path where there is no +transaction to own. D4's rollback guarantee, and any caller guarantee phrased as +"this whole unit rolls back together", holds only for the owner; before this +signal a joined callback had no way to know which it was. + +### The declared LIMIT of the same-origin gate + +The gate judges only handles it can attribute. `TransactionScope` exists for +every transaction the engine opens, and an explicitly-threaded handle is matched +back to the store entry by identity — so the dominant path (`transaction()` +hands you `trxCtx`, you thread it as `{ context: trxCtx }`) is covered. + +**Not covered**, by decision rather than oversight: `ScopedContext`'s discrete +`beginTransaction`/`commit`/`rollback` trio, which threads its handle across +`setImmediate` boundaries where AsyncLocalStorage does not survive and therefore +never populates `txStore`; and any handle an outside caller obtained elsewhere +and passed in as `execCtx.transaction`. For those the engine holds an opaque +driver object with no back-reference to its owner, so there is no honest +comparison available. Guessing — assuming an unattributed handle belongs to the +default driver — would refuse legitimate single-datasource work on one side and +carve out genuinely-covered writes on the other. The pre-#5351 behaviour stands +on that path, is pinned as such in +`packages/objectql/src/engine-transaction-same-origin.test.ts`, and closing it +requires handle ownership to become discoverable on `IDataDriver` — a +driver-contract change, tracked separately. + +### Consumer impact + +Single-datasource deployments — the overwhelming majority — see no change of any +kind: no refusal, no carve-out, and nothing logged. The gate is reachable only +where a second datasource is registered AND an object routes to it AND a +transaction is open. + +**Decided by**: the maintainer, 2026-08-06, on #5351 (plan A + this revision) +and #5696 (P2, batched forward to the same implementation). **Implemented in**: +`packages/spec/src/contracts/objectql-engine.ts`, +`packages/objectql/src/engine.ts`, +`packages/objectql/src/transaction-errors.ts`. **Pinned by**: +`engine-transaction-same-origin.test.ts` (18 cases) and +`engine-transaction-contract.test.ts` (15 cases). The `error`-level split +diagnostic PR #5724 added is retired by D1-R2/R3 — there is no longer a split to +report — and its section of `engine-transaction-observability.test.ts` moved to +the same-origin file, re-asked against the decided verdict. diff --git a/packages/objectql/src/engine-transaction-observability.test.ts b/packages/objectql/src/engine-transaction-observability.test.ts index 7e1fff6c12..e10c3a9f8f 100644 --- a/packages/objectql/src/engine-transaction-observability.test.ts +++ b/packages/objectql/src/engine-transaction-observability.test.ts @@ -14,11 +14,17 @@ // committed reported nothing at all. These tests pin the diagnostics that make // the two DISCOVERABLE. // -// What they deliberately do NOT pin is any change of behaviour. Refusing the -// degrade (`opts.require`) or refusing the cross-driver write would tighten the -// declared contract in `packages/spec/src/contracts/objectql-engine.ts`; that -// half of #4619 is spec-lane work and is not done here. So every assertion -// below is of the form "the same thing happens, and now it is also said". +// ⚠️ SCOPE UPDATE (#5351 / #5696, 2026-08-06 ruling). This file originally +// pinned BOTH caveats and pinned neither's behaviour, on the standing note that +// tightening either would change the declared contract and was spec-lane work. +// That work landed. Caveat 2's subject is gone — a cross-driver business write +// is now REFUSED and a system ledger is CARVED OUT, so there is no split left +// to report — and its section has moved wholesale to +// `engine-transaction-same-origin.test.ts`, re-asked against the decided +// verdict rather than re-spelled. Caveat 1 is untouched and still lives here: +// the degrade still happens and still warns once; `opts.require` (#5696) made +// refusing it a caller's OPTION without changing the default this section +// covers, and that option is pinned in `engine-transaction-contract.test.ts`. import { describe, it, expect } from 'vitest'; import { ObjectQL } from './engine.js'; @@ -129,7 +135,7 @@ describe('transaction() degrade with no beginTransaction warns once (#4619)', () const driver = makeDriver('memory', { transactional: false }); engine.registerDriver(driver, true); await engine.init(); - engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any); + engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any, '__test__'); return { rec, engine, driver }; } @@ -191,7 +197,7 @@ describe('transaction() degrade with no beginTransaction warns once (#4619)', () const engine = new ObjectQL({ logger: rec.logger } as any); engine.registerDriver(makeDriver('memory'), true); await engine.init(); - engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any); + engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any, '__test__'); await engine.transaction(async () => { await engine.insert('thing', { name: 'A' }); @@ -219,181 +225,29 @@ describe('transaction() degrade with no beginTransaction warns once (#4619)', () }); // --------------------------------------------------------------------------- -// 2. Default-driver-only → loud cross-datasource routing diagnostic +// 2. Default-driver-only — MOVED. The subject of this section was decided. // --------------------------------------------------------------------------- - -describe('a write inside transaction() routed off the default datasource is reported at error (#4619)', () => { - async function twoDatasourceEngine() { - const rec = recordingLogger(); - const engine = new ObjectQL({ logger: rec.logger } as any); - const primary = makeDriver('primary'); - const ledgerDs = makeDriver('ledger_db'); - const archiveDs = makeDriver('archive_db'); - engine.registerDriver(ledgerDs); - engine.registerDriver(archiveDs); - engine.registerDriver(primary, true); // default - await engine.init(); - // `ledger` and `archive` live elsewhere; `thing` stays on the default. - engine.setDatasourceMapping([ - { objectPattern: 'ledger', datasource: 'ledger_db' }, - { objectPattern: 'archive', datasource: 'archive_db' }, - ]); - engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any); - engine.registry.registerObject({ name: 'ledger', fields: { name: { type: 'text' } } } as any); - engine.registry.registerObject({ name: 'archive', fields: { name: { type: 'text' } } } as any); - const splits = () => matching(rec.at('error'), 'running OUTSIDE the transaction'); - return { rec, engine, primary, ledgerDs, archiveDs, splits }; - } - - it('reports the split — which object, which datasource, and that it is outside the transaction', async () => { - const { engine, ledgerDs, splits } = await twoDatasourceEngine(); - - await engine.transaction(async () => { - await engine.insert('thing', { name: 'covered' }); - await engine.insert('ledger', { name: 'NOT covered' }); - }); - - const found = splits(); - expect(found).toHaveLength(1); - expect(found[0].message).toContain("insert of 'ledger'"); - expect(found[0].message).toContain("datasource 'ledger_db'"); - expect(found[0].message).toContain("default datasource 'primary'"); - // The consequence, concretely, and the fix — both owed by an `error`. - expect(found[0].message).toContain('rolling the transaction back will NOT undo it'); - expect(found[0].message).toContain('datasourceMapping'); - expect(meta(found[0])).toMatchObject({ - object: 'ledger', - operation: 'insert', - datasource: 'ledger_db', - transactionDatasource: 'primary', - }); - - // BEHAVIOUR IS UNCHANGED: the write still went to the mapped datasource, - // exactly as before. This PR reports; refusing is the spec half. - expect(ledgerDs.writes).toHaveLength(1); - expect(ledgerDs.writes[0]).toMatchObject({ object: 'ledger', op: 'create' }); - - // And it is worse than "written without a transaction", which is what the - // contract's caveat says. `buildDriverOptions` reads the ambient handle - // with no idea which driver is about to receive it, so `ledger_db`'s driver - // is handed `primary`'s transaction object. Pinned here as OBSERVED, not - // endorsed — it predates this change (nothing here touches - // `buildDriverOptions`) and is filed separately; the assertion exists so - // that whoever fixes it sees this test, rather than a silent shift under a - // `toBeUndefined()` that was only ever a guess. - expect(ledgerDs.writes[0].transaction).toEqual({ __trx: 'primary' }); - }); - - it('does not fire for writes that stay on the default datasource', async () => { - const { engine, primary, splits } = await twoDatasourceEngine(); - - await engine.transaction(async () => { - await engine.insert('thing', { name: 'A' }); - await engine.insert('thing', { name: 'B' }); - }); - - expect(splits()).toHaveLength(0); - // and those writes really did ride the transaction - expect(primary.writes).toHaveLength(2); - expect(primary.writes[0].transaction).toBeTruthy(); - }); - - it('does not fire for a mapped write made OUTSIDE any transaction — no false positive', async () => { - const { engine, splits } = await twoDatasourceEngine(); - - // Routing an object to another datasource is a normal, supported thing to - // do. It is only a problem while a transaction is open and claiming to - // cover the work, so an ordinary write must stay silent. - const seeded = await engine.insert('ledger', { name: 'plain write' }); - await engine.update('ledger', { id: seeded.id, name: 'renamed' }); - await engine.delete('ledger', { where: { id: seeded.id } } as any); - - expect(splits()).toHaveLength(0); - }); - - it('says it once per transaction per datasource, and separately for a second datasource', async () => { - const { engine, splits } = await twoDatasourceEngine(); - - await engine.transaction(async () => { - await engine.insert('ledger', { name: 'a' }); - await engine.insert('ledger', { name: 'b' }); - await engine.insert('ledger', { name: 'c' }); - await engine.insert('archive', { name: 'd' }); - await engine.insert('archive', { name: 'e' }); - }); - - // Three writes to `ledger_db` + two to `archive_db` = two splits, not five. - // AGENTS.md: "say it once, at the first degradation, not once per failed - // write" — a 500-row batch off the default datasource is ONE split. - const found = splits(); - expect(found).toHaveLength(2); - expect(found.map((r) => meta(r).datasource).sort()).toEqual(['archive_db', 'ledger_db']); - }); - - it('re-reports in a NEW transaction — the budget is per transaction, not per engine', async () => { - const { engine, splits } = await twoDatasourceEngine(); - - await engine.transaction(async () => { await engine.insert('ledger', { name: 'first' }); }); - await engine.transaction(async () => { await engine.insert('ledger', { name: 'second' }); }); - - // Two units of work, two partial commits, two reports: each one is a - // separate atomicity claim that did not hold. - expect(splits()).toHaveLength(2); - }); - - it('covers update and delete, not just insert', async () => { - const { engine, splits } = await twoDatasourceEngine(); - - const seeded = await engine.insert('ledger', { name: 'seed' }); - - await engine.transaction(async () => { - await engine.update('ledger', { id: seeded.id, name: 'renamed' }); - }); - await engine.transaction(async () => { - await engine.delete('ledger', { where: { id: seeded.id } } as any); - }); - - const found = splits(); - expect(found).toHaveLength(2); - expect(found.map((r) => meta(r).operation)).toEqual(['update', 'delete']); - }); - - it('fires for a nested transaction() that JOINED the outer one (ADR-0067 D2)', async () => { - const { engine, splits } = await twoDatasourceEngine(); - - // A joined nested call does not open its own transaction — it runs inside - // the outer one's ambient scope, so the outer owner is what the write is - // measured against. The write is just as uncovered as at the top level. - await engine.transaction(async () => { - await engine.transaction(async () => { - await engine.insert('ledger', { name: 'nested' }); - }); - }); - - expect(splits()).toHaveLength(1); - expect(meta(splits()[0])).toMatchObject({ transactionDatasource: 'primary' }); - }); - - it('fires from ScopedContext.transaction too (ctx.api.transaction in a sandboxed hook body)', async () => { - const { engine, splits } = await twoDatasourceEngine(); - - const scoped = (engine as any).createContext({ userId: 'u1' }); - await scoped.transaction(async () => { - await engine.insert('ledger', { name: 'from sandbox' }); - }); - - expect(splits()).toHaveLength(1); - expect(meta(splits()[0])).toMatchObject({ datasource: 'ledger_db', transactionDatasource: 'primary' }); - }); - - it('stays silent after the transaction closes — the scope does not leak', async () => { - const { engine, splits } = await twoDatasourceEngine(); - - await engine.transaction(async () => { - await engine.insert('thing', { name: 'covered' }); - }); - await engine.insert('ledger', { name: 'after' }); - - expect(splits()).toHaveLength(0); - }); -}); +// +// This file used to carry nine tests pinning the `error`-level diagnostic that +// PR #5724 added for a write routed off the transaction's datasource, under the +// standing note that reporting the split "does not fix it: refusing, or +// committing across drivers, would change the declared contract". +// +// The 2026-08-06 maintainer ruling on #5351 changed that contract. There is no +// longer a diagnostic to pin, because the engine no longer lets the write +// happen the way the diagnostic described: a BUSINESS write across drivers is +// refused (`CrossDatasourceTransactionWriteError`), and an append-only SYSTEM +// LEDGER is carved out and executed outside the transaction, with neither ever +// receiving the owner's handle. +// +// So this section did not become wrong — its subject moved. Every one of its +// facts is re-asked against the decided verdict in +// `engine-transaction-same-origin.test.ts`: the once-per-transaction budget +// (now on the carve-out note, since a refusal throws and a throw is never +// deduplicated), update/delete coverage, the joined-nested case, the +// `ScopedContext` surface, "no false positive outside a transaction", and "the +// scope does not leak". Leaving stubs behind here would pin nothing twice. +// +// Section 1 above stays exactly as it was: the warn-once degrade IS still +// observability-only, and `opts.require` (#5696) made it a caller's choice +// without changing the default it reports on. diff --git a/packages/objectql/src/engine-transaction-same-origin.test.ts b/packages/objectql/src/engine-transaction-same-origin.test.ts new file mode 100644 index 0000000000..5388e9fe27 --- /dev/null +++ b/packages/objectql/src/engine-transaction-same-origin.test.ts @@ -0,0 +1,528 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #5351 + #5696 point 2 — the SAME-ORIGIN transaction gate, and the two answers +// the 2026-08-06 maintainer ruling gave it. +// +// The defect (#5351, reproduced independently in PR #5724): `buildDriverOptions` +// lifted the ambient transaction handle onto EVERY driver call with no idea +// which driver was about to receive it. A write routed elsewhere — by +// `setDatasourceMapping`, by an explicit `datasource:` binding, or by ADR-0057 +// §3.6 lifecycle-class separation — was handed the DEFAULT driver's handle and +// executed on the wrong connection. On knex/SQLite that is `no such table` +// against a database that never held the object, which is how every compliance +// audit row written inside a transaction was being lost: 52 insert attempts, 50 +// successes, and the 2 failures were exactly the 2 with a `trxClient.query` +// frame in their stack. +// +// The ruling is two answers, because the two kinds of write fail in opposite +// directions: +// +// BUSINESS writes are REFUSED (#5696 point 2) — no two-phase commit exists, +// so the honest move is to make the caller choose instead of committing part +// of a "unit of work" they can neither see nor undo. +// +// APPEND-ONLY SYSTEM LEDGERS are CARVED OUT (#5351 plan A) — audit / +// telemetry / event rows execute OUTSIDE the transaction, on their own +// connection, and therefore SURVIVE a rollback. The orphan row is the +// deliberate cost: for an append-only ledger a spurious row is reconcilable, +// while a missing row for a write that DID commit is an unrecoverable +// compliance hole. +// +// Why they had to land together: refusal alone would be "loud but not fixed". +// An audit hook's try/catch eats the refusal and logs it, and the compliance +// row is lost exactly as before — only now with a different log line. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL, ScopedContext } from './engine.js'; +import { CrossDatasourceTransactionWriteError } from './transaction-errors.js'; + +interface Recorded { + level: 'debug' | 'info' | 'warn' | 'error'; + message: string; + args: unknown[]; +} + +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); + }, + }; +} + +/** + * 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 meta(r: Recorded): Record { + return (r.args.find((a) => a !== undefined && typeof a === 'object' && !(a instanceof Error)) ?? + {}) as Record; +} + +/** One write the driver double saw — the shape its `writes` array carries. */ +type RecordedWrite = { object: string; op: 'create' | 'update' | 'delete'; transaction: unknown }; + +function makeDriver(name: string) { + const writes: RecordedWrite[] = []; + const reads: Array<{ object: string; transaction: unknown }> = []; + const rows = new Map>(); + let nextId = 0; + const driver: any = { + name, + version: '0.0.0', + supports: {}, + writes, + reads, + rows, + async connect() {}, + async disconnect() {}, + async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, _ast: any, options: any) { + reads.push({ object, transaction: options?.transaction }); + return Array.from(rows.values()); + }, + async findOne(object: string, ast: any, options: any) { + reads.push({ object, transaction: options?.transaction }); + 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() {}, + // A transaction handle is a per-connection object. The double names its + // driver so a handle arriving at the WRONG driver is visible in one glance — + // which is precisely the defect #5351 measured. + beginTransaction: async () => ({ __trx: name }), + commit: async () => {}, + rollback: async () => {}, + }; + return driver; +} + +/** + * The real #5351 topology: a primary business datasource plus the dedicated + * `telemetry` datasource ADR-0057 §3.6 routes lifecycle-classed system data to, + * plus a plain business object routed away by an ordinary mapping rule. + */ +async function splitEngine() { + const rec = recordingLogger(); + const engine = new ObjectQL({ logger: rec.logger } as any); + const primary = makeDriver('primary'); + const telemetry = makeDriver(ObjectQL.LIFECYCLE_DATASOURCE); + const ledgerDb = makeDriver('ledger_db'); + engine.registerDriver(telemetry); + engine.registerDriver(ledgerDb); + engine.registerDriver(primary, true); // default + await engine.init(); + // `ledger` is an ordinary BUSINESS object that a mapping rule routes away. + engine.setDatasourceMapping([{ objectPattern: 'ledger', datasource: 'ledger_db' }]); + engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any, '__test__'); + engine.registry.registerObject({ name: 'ledger', fields: { name: { type: 'text' } } } as any, '__test__'); + // The three append-only system ledgers, routed by lifecycle class alone. + engine.registry.registerObject({ + name: 'sys_audit_log', + lifecycle: { class: 'audit', retention: { maxAge: '90d' } }, + fields: { action: { type: 'text' } }, + } as any, '__test__'); + engine.registry.registerObject({ + name: 'sys_activity', + lifecycle: { class: 'telemetry', retention: { maxAge: '14d' } }, + fields: { action: { type: 'text' } }, + } as any, '__test__'); + engine.registry.registerObject({ + name: 'sys_bus_event', + lifecycle: { class: 'event', ttl: { field: 'created_at', expireAfter: '6h' } }, + fields: { action: { type: 'text' } }, + } as any, '__test__'); + const carveOutNotes = () => + rec.at('debug').filter((r) => r.message.includes('executing it OUTSIDE the transaction')); + return { rec, engine, primary, telemetry, ledgerDb, carveOutNotes }; +} + +// --------------------------------------------------------------------------- +// (a) The system-ledger carve-out — the compliance row actually lands +// --------------------------------------------------------------------------- + +describe('an audit-shaped write inside a cross-origin transaction LANDS, outside it (#5351)', () => { + it('reaches the telemetry datasource with NO foreign transaction handle', async () => { + const { engine, telemetry, primary } = await splitEngine(); + + await engine.transaction(async () => { + await engine.insert('thing', { name: 'business row' }); + // The shape of an `afterInsert` audit hook: an ordinary insert of an + // audit-class object, made by an author who knows nothing about routing. + await engine.insert('sys_audit_log', { action: 'insert thing' }); + }); + + // It landed — this is the whole point. Before #5351 this write was handed + // `primary`'s handle and, on a real SQL driver, never reached this store. + expect(telemetry.writes).toHaveLength(1); + expect(telemetry.writes[0]).toMatchObject({ object: 'sys_audit_log', op: 'create' }); + // And it landed WITHOUT another driver's connection. Asserting `undefined` + // rather than "not primary's": there is no handle this driver could + // legitimately have been given, since the engine opened no transaction here. + expect(telemetry.writes[0].transaction).toBeUndefined(); + // The business write is unaffected and still rides its own transaction. + expect(primary.writes).toHaveLength(1); + expect(primary.writes[0].transaction).toEqual({ __trx: 'primary' }); + }); + + it('covers all three system-ledger classes, not just audit', async () => { + const { engine, telemetry } = await splitEngine(); + + await engine.transaction(async () => { + await engine.insert('sys_audit_log', { action: 'a' }); + await engine.insert('sys_activity', { action: 'b' }); + await engine.insert('sys_bus_event', { action: 'c' }); + }); + + expect(telemetry.writes.map((w: RecordedWrite) => w.object)).toEqual(['sys_audit_log', 'sys_activity', 'sys_bus_event']); + expect(telemetry.writes.every((w: RecordedWrite) => w.transaction === undefined)).toBe(true); + }); + + it('SURVIVES the rollback of the business transaction — the orphan row, pinned', async () => { + const { engine, telemetry, primary } = await splitEngine(); + let rolledBack = false; + primary.rollback = async () => { rolledBack = true; }; + + await expect( + engine.transaction(async () => { + await engine.insert('thing', { name: 'doomed' }); + await engine.insert('sys_audit_log', { action: 'insert thing' }); + throw new Error('business failure'); + }), + ).rejects.toThrow('business failure'); + + expect(rolledBack).toBe(true); + // The decided cost of plan A, asserted rather than left implicit: an audit + // row describing a write that was undone. For an append-only compliance + // ledger this is the correct direction of error — a spurious row can be + // reconciled against the business data, a MISSING row for a write that did + // commit cannot be recovered at all. ADR-0067/ADR-0119 record it. + expect(telemetry.writes).toHaveLength(1); + expect(telemetry.writes[0]).toMatchObject({ object: 'sys_audit_log' }); + }); + + it('notes the carve-out at debug — once per transaction per datasource', async () => { + const { engine, carveOutNotes, rec } = await splitEngine(); + + await engine.transaction(async () => { + await engine.insert('sys_audit_log', { action: 'a' }); + await engine.insert('sys_audit_log', { action: 'b' }); + await engine.insert('sys_activity', { action: 'c' }); + }); + + const notes = carveOutNotes(); + expect(notes).toHaveLength(1); + expect(notes[0].message).toContain("insert of 'sys_audit_log'"); + expect(notes[0].message).toContain('SURVIVE a rollback'); + expect(meta(notes[0])).toMatchObject({ + object: 'sys_audit_log', + operation: 'insert', + datasource: ObjectQL.LIFECYCLE_DATASOURCE, + transactionDatasource: 'primary', + }); + // NOT `warn`, NOT `error`. This is now DECLARED behaviour that fires on + // every audited write of every transaction in a lifecycle-split deployment; + // AGENTS.md's judgment question asks whether something claimed persisted + // has failed to land, and here it lands. Escalating it would train readers + // to skim the levels that carry real durability failures. + expect(rec.at('error')).toHaveLength(0); + expect(rec.at('warn')).toHaveLength(0); + }); + + it('re-notes in a NEW transaction — the budget is per transaction', async () => { + const { engine, carveOutNotes } = await splitEngine(); + + await engine.transaction(async () => { await engine.insert('sys_audit_log', { action: 'a' }); }); + await engine.transaction(async () => { await engine.insert('sys_audit_log', { action: 'b' }); }); + + expect(carveOutNotes()).toHaveLength(2); + }); + + it('carves out update and delete of a system ledger too, not just insert', async () => { + const { engine, telemetry } = await splitEngine(); + + const seeded = await engine.insert('sys_audit_log', { action: 'seed' }); + await engine.transaction(async () => { + await engine.update('sys_audit_log', { id: seeded.id, action: 'amended' }); + await engine.delete('sys_audit_log', { where: { id: seeded.id } } as any); + }); + + const inTxn = telemetry.writes.slice(1) as RecordedWrite[]; + expect(inTxn.map((w) => w.op)).toEqual(['update', 'delete']); + expect(inTxn.every((w) => w.transaction === undefined)).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// (b) Business writes across drivers — refused, by name +// --------------------------------------------------------------------------- + +describe('a BUSINESS write across drivers inside a transaction is refused (#5696 point 2)', () => { + it('throws CrossDatasourceTransactionWriteError naming both datasources and the fix', async () => { + const { engine } = await splitEngine(); + + const err = await rejection( + engine.transaction(async () => { await engine.insert('ledger', { name: 'NOT covered' }); }), + ); + + expect(err).toBeInstanceOf(CrossDatasourceTransactionWriteError); + expect(err.code).toBe('ERR_CROSS_DATASOURCE_TRANSACTION_WRITE'); + expect(err).toMatchObject({ + object: 'ledger', + operation: 'insert', + datasource: 'ledger_db', + transactionDatasource: 'primary', + }); + // A refusal owes the caller the remedy — both routes out, concretely. + expect(err.message).toContain('datasourceMapping'); + expect(err.message).toContain('split the work into per-datasource units'); + expect(err.message).toContain('Nothing was written.'); + }); + + it('writes nothing, on either datasource, and rolls the transaction back', async () => { + const { engine, primary, ledgerDb } = await splitEngine(); + let rolledBack = false; + primary.rollback = async () => { rolledBack = true; }; + + await expect( + engine.transaction(async () => { + await engine.insert('thing', { name: 'first' }); + await engine.insert('ledger', { name: 'refused' }); + }), + ).rejects.toBeInstanceOf(CrossDatasourceTransactionWriteError); + + // Refused before any hook, default or validation ran — so the cross-driver + // store is untouched, and the covered write is undone by the rollback the + // rejection triggers. This is the difference from the pre-v17 behaviour: + // the caller now learns before a partial commit exists, not after. + expect(ledgerDb.writes).toHaveLength(0); + expect(rolledBack).toBe(true); + }); + + it('refuses update and delete too', async () => { + const { engine } = await splitEngine(); + const seeded = await engine.insert('ledger', { name: 'seed' }); + + await expect( + engine.transaction(async () => { await engine.update('ledger', { id: seeded.id, name: 'x' }); }), + ).rejects.toMatchObject({ operation: 'update', code: 'ERR_CROSS_DATASOURCE_TRANSACTION_WRITE' }); + + await expect( + engine.transaction(async () => { await engine.delete('ledger', { where: { id: seeded.id } } as any); }), + ).rejects.toMatchObject({ operation: 'delete', code: 'ERR_CROSS_DATASOURCE_TRANSACTION_WRITE' }); + }); + + it('refuses from a nested transaction() that JOINED the outer one (ADR-0067 D2)', async () => { + const { engine } = await splitEngine(); + + // A joined nested call opens nothing — it runs in the outer scope, so the + // OUTER owner is what the write is measured against. Just as uncovered as + // at the top level, and refused the same way. + await expect( + engine.transaction(async () => { + await engine.transaction(async () => { await engine.insert('ledger', { name: 'nested' }); }); + }), + ).rejects.toMatchObject({ transactionDatasource: 'primary' }); + }); + + it('refuses from ScopedContext.transaction too (ctx.api.transaction in a sandboxed hook body)', async () => { + const { engine } = await splitEngine(); + const scoped = (engine as any).createContext({ userId: 'u1' }) as ScopedContext; + + await expect( + scoped.transaction(async () => { await engine.insert('ledger', { name: 'from sandbox' }); }), + ).rejects.toBeInstanceOf(CrossDatasourceTransactionWriteError); + }); + + it('does NOT refuse a mapped write made outside any transaction — no false positive', async () => { + const { engine, ledgerDb } = await splitEngine(); + + // Routing a business object to another datasource is normal and supported. + // It is only a problem while a transaction is open and claiming to cover + // the work. + const seeded = await engine.insert('ledger', { name: 'plain write' }); + await engine.update('ledger', { id: seeded.id, name: 'renamed' }); + await engine.delete('ledger', { where: { id: seeded.id } } as any); + + expect(ledgerDb.writes.map((w: RecordedWrite) => w.op)).toEqual(['create', 'update', 'delete']); + }); + + it('stays quiet after the transaction closes — the scope does not leak', async () => { + const { engine, ledgerDb } = await splitEngine(); + + await engine.transaction(async () => { await engine.insert('thing', { name: 'covered' }); }); + await expect(engine.insert('ledger', { name: 'after' })).resolves.toBeTruthy(); + expect(ledgerDb.writes).toHaveLength(1); + }); +}); + +// --------------------------------------------------------------------------- +// Reads: never the wrong connection, never refused +// --------------------------------------------------------------------------- + +describe('a cross-driver READ inside a transaction loses the handle but is not refused (#5351)', () => { + it('does not hand the other driver the transaction handle', async () => { + const { engine, ledgerDb, primary } = await splitEngine(); + + await engine.transaction(async () => { + await engine.find('thing', {}); + await engine.find('ledger', {}); + }); + + // A read has no atomicity claim to break, so refusing it would only break + // legitimate work (reference checks against a routed object, expand reads). + // But it must not run on a connection its driver does not own — that was + // the same wrong-connection defect, one verb over. + // + // Both lengths asserted first: `reads.at(-1)?.transaction` on an EMPTY + // array is `undefined`, so the handle assertion alone would pass for the + // reason "no read happened" rather than "the read was clean". + expect(ledgerDb.reads).toHaveLength(1); + expect(primary.reads).toHaveLength(1); + expect(ledgerDb.reads[0].transaction).toBeUndefined(); + expect(primary.reads[0].transaction).toEqual({ __trx: 'primary' }); + }); +}); + +// --------------------------------------------------------------------------- +// (e) Single-datasource deployments: nothing changes at all +// --------------------------------------------------------------------------- + +describe('a same-origin (single datasource) transaction is untouched (#5351 regression guard)', () => { + async function singleEngine() { + const rec = recordingLogger(); + const engine = new ObjectQL({ logger: rec.logger } as any); + const primary = makeDriver('primary'); + engine.registerDriver(primary, true); + await engine.init(); + engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } } as any, '__test__'); + // Audit-classed, but with NO telemetry datasource registered it resolves to + // the default driver like everything else — ADR-0057 §3.6 is opt-in by the + // datasource's existence. The carve-out must not fire on it. + engine.registry.registerObject({ + name: 'sys_audit_log', + lifecycle: { class: 'audit', retention: { maxAge: '90d' } }, + fields: { action: { type: 'text' } }, + } as any, '__test__'); + return { rec, engine, primary }; + } + + it('every write still rides the one transaction, audit rows included', async () => { + const { engine, primary, rec } = await singleEngine(); + + await engine.transaction(async () => { + await engine.insert('thing', { name: 'a' }); + await engine.insert('sys_audit_log', { action: 'insert thing' }); + }); + + expect(primary.writes).toHaveLength(2); + expect(primary.writes.every((w: RecordedWrite) => JSON.stringify(w.transaction) === JSON.stringify({ __trx: 'primary' }))).toBe(true); + // No carve-out, no refusal, and nothing said: the gate is invisible to the + // deployments that make up the overwhelming majority. + expect(rec.at('error')).toHaveLength(0); + expect(rec.at('warn')).toHaveLength(0); + expect(rec.at('debug').filter((r) => r.message.includes('OUTSIDE the transaction'))).toHaveLength(0); + }); + + it('rolls audit rows back with the business rows, exactly as before', async () => { + const { engine, primary } = await singleEngine(); + let rolledBack = false; + primary.rollback = async () => { rolledBack = true; }; + + await expect( + engine.transaction(async () => { + await engine.insert('sys_audit_log', { action: 'doomed' }); + throw new Error('boom'); + }), + ).rejects.toThrow('boom'); + + // No orphan-row semantics here — there is nothing to be orphaned FROM. The + // carve-out's cost is paid only by deployments that actually split. + expect(rolledBack).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// The gate's declared limit — handles the engine cannot attribute +// --------------------------------------------------------------------------- + +describe('the same-origin gate declines to judge a handle it cannot attribute (#5351 boundary)', () => { + it('covers an explicitly-threaded handle when it IS the ambient one', async () => { + const { engine, ledgerDb } = await splitEngine(); + + // The dominant explicit path: `transaction()` hands the caller `trxCtx` and + // the caller threads it back as `{ context: trxCtx }`. Identity-matching + // that handle against the store entry is what keeps this path covered — a + // caller cannot escape the gate by passing the context the engine gave it. + await expect( + engine.transaction(async (trxCtx) => { + await engine.insert('ledger', { name: 'threaded' }, { context: trxCtx } as any); + }), + ).rejects.toBeInstanceOf(CrossDatasourceTransactionWriteError); + expect(ledgerDb.writes).toHaveLength(0); + }); + + it('does NOT cover a foreign handle passed in from outside — declared, not overlooked', async () => { + const { engine, ledgerDb } = await splitEngine(); + + // The sandbox runner's discrete `beginTransaction`/`commit`/`rollback` trio + // threads its handle across `setImmediate` boundaries where + // AsyncLocalStorage does not survive, so it never populates txStore and the + // engine holds an opaque driver object with no back-reference to its owner. + // There is no honest comparison to make, and guessing would refuse + // legitimate single-datasource work. So the pre-#5351 behaviour stands on + // this path — pinned here so the limit is a recorded decision rather than a + // gap someone discovers. Closing it needs handle ownership to become + // discoverable on `IDataDriver`; filed separately. + const foreign = { __trx: 'somebody elses handle' }; + await engine.insert('ledger', { name: 'unattributed' }, { context: { transaction: foreign } } as any); + + expect(ledgerDb.writes).toHaveLength(1); + expect(ledgerDb.writes[0].transaction).toBe(foreign); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index c4310717f3..1c99bdc1e2 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -43,7 +43,7 @@ import { resolveFilterTokens, } from '@objectstack/core'; import { SummaryRecomputeError, type SummaryRecomputeFailure } from './summary-errors.js'; -import { TransactionUnsupportedError } from './transaction-errors.js'; +import { CrossDatasourceTransactionWriteError, TransactionUnsupportedError } from './transaction-errors.js'; import { aggregateSummaryValue, summaryEmptySetValue, @@ -915,10 +915,16 @@ function eventMatchedCount(value: unknown): number | undefined { * What the engine knows about the transaction it opened, beyond the handle * itself (#4619, ADR-0119 D1 follow-up). * - * Purely an OBSERVABILITY record: nothing here changes which driver a write is - * routed to, whether a transaction is opened, or what is committed. It exists - * so the write path can tell a caller that a write it believes is inside the - * transaction is not — the one thing today's engine cannot say. + * It began as a pure OBSERVABILITY record — #4619 could say a write was outside + * the transaction but deliberately did not act on it, because acting changes + * ADR-0119 D1's declared contract and that was a decision for the maintainer, + * not for the PR that found the defect. Since the 2026-08-06 ruling on #5351 + * this record is LOAD-BEARING: `enforceTransactionOrigin` refuses a + * cross-driver business write on it, and `transactionCoversDriverFor` uses it + * to keep a transaction handle from ever reaching a driver that does not own + * it. Routing itself is still untouched — a write goes exactly where `getDriver` + * sends it; what changed is whether it goes there carrying someone else's + * connection. */ interface TransactionScope { /** @@ -931,9 +937,11 @@ interface TransactionScope { /** The datasource name that driver is registered under — for the message. */ readonly datasource: string; /** - * Datasources already reported for THIS transaction. AGENTS.md's - * "say it once, at the first degradation, not once per failed write" — a - * 500-row batch routed elsewhere is one split, not 500. + * Datasources already noted for THIS transaction. AGENTS.md's "say it once, + * at the first degradation, not once per failed write" — a 500-row audit + * batch carved out of this transaction is one note, not 500. Only the + * carve-out path consumes the budget; a refused business write throws, and a + * throw is never deduplicated. */ readonly reportedOutOfScope: Set; } @@ -955,7 +963,8 @@ export class ObjectQL implements IObjectQLEngine { * that is part of the declared contract (ADR-0119 D1) — so a write routed * elsewhere by `setDatasourceMapping` runs OUTSIDE it and cannot be rolled * back with it. Carrying the owner here is what lets the write path SAY so - * ({@link reportWriteOutsideTransaction}); it changes no routing. + * ({@link enforceTransactionOrigin}) — and, since #5351, to DECIDE what + * happens to it: a business write is refused, a system ledger is carved out. * * Absent on the sandbox runner's explicitly-threaded handles (the * `beginTransaction`/`commit`/`rollback` trio does not use this store at @@ -1757,7 +1766,17 @@ export class ObjectQL implements IObjectQLEngine { const tx = execCtx?.transaction !== undefined ? execCtx.transaction : this.txStore.getStore()?.transaction; - const hasTx = tx !== undefined; + // [#5351] SAME-ORIGIN GATE. A transaction handle is a property of ONE + // driver's connection; handing it to a different driver does not put that + // driver's statement inside the transaction, it executes the statement on + // the WRONG CONNECTION — measured on knex/SQLite as `no such table` against + // a database that never held the object. The write path's + // `enforceTransactionOrigin` has already refused a business write by the + // time we get here (and let a system ledger through by decision), so this + // is the structural half: whatever survives to here, the handle only ever + // reaches the driver that owns it. It covers READS too, which have no gate + // of their own and were riding the same wrong connection. + const hasTx = tx !== undefined && this.transactionCoversDriverFor(object, tx); const hasTenant = execCtx?.tenantId !== undefined && !isTenancyDisabled(this._registry.getObject(object)); @@ -1805,6 +1824,45 @@ export class ObjectQL implements IObjectQLEngine { return opts; } + /** + * Does the open transaction `tx` actually cover the driver `object` resolves + * to? — the same-origin question, asked by instance IDENTITY (#5351). + * + * Answers `true` in two cases: the resolved driver IS the transaction's + * owner, or the engine cannot tell who the owner is. The second case is the + * DECLARED LIMIT of this gate, not an oversight, and it is exactly one + * shape: a handle the engine never opened and cannot attribute. + * + * `TransactionScope` (#5724) records the owner for every transaction the + * engine opens, and `transaction()` also threads that same handle down as + * `execCtx.transaction`, so the dominant explicit-threading path is covered + * by identity-matching the handle back to the store entry. What is NOT + * covered: + * + * - `ScopedContext`'s discrete `beginTransaction`/`commit`/`rollback` trio, + * which threads the handle across `setImmediate` boundaries where + * AsyncLocalStorage does not survive, and so never populates txStore; + * - any handle an outside caller obtained elsewhere and passed in as + * `execCtx.transaction`. + * + * For those the engine holds an opaque driver object with no back-reference + * to its driver, so there is no honest comparison to make. Guessing — say, + * assuming an unattributed handle belongs to the default driver — would + * refuse legitimate single-datasource work on one side and carve out writes + * that were genuinely covered on the other. So the gate declines to judge and + * the pre-#5351 behaviour stands on that path: recorded in the ADR-0067/0119 + * revision, and closable only by making handle ownership discoverable on + * `IDataDriver` (filed separately). + */ + private transactionCoversDriverFor(object: string, tx: unknown): boolean { + const store = this.txStore.getStore(); + // The scope describes the handle in the store. An explicitly-threaded + // handle is covered by it only when it IS that handle. + const scope = store !== undefined && tx === store.transaction ? store.scope : undefined; + if (!scope) return true; + return this.getDriver(object) === scope.driver; + } + /** * Resolve the `NOW()` runtime token into the value the field's declared type * actually stores (#4597). @@ -3306,6 +3364,50 @@ export class ObjectQL implements IObjectQLEngine { */ static readonly LIFECYCLE_DATASOURCE = 'telemetry'; + /** + * The lifecycle classes that make an object an APPEND-ONLY SYSTEM LEDGER — + * the audit trail, telemetry, and the event log (ADR-0057 §3.6). + * + * One constant, read by both places that must agree (#5351): + * + * 1. {@link getDriver} step 3 — which objects lifecycle-class separation + * routes to the dedicated datasource; + * 2. {@link enforceTransactionOrigin} — which cross-datasource writes are + * CARVED OUT of an ambient transaction instead of refused. + * + * Two hand-written copies of this tuple would drift by one class and produce + * the worst outcome available: an object routed away by rule 1 and refused by + * rule 2 loses exactly the compliance row this whole change exists to save. + * + * `transient` is deliberately absent, matching step 3: those objects stay on + * the primary, so they never reach the gate at all. + */ + static readonly SYSTEM_LEDGER_LIFECYCLE_CLASSES: ReadonlySet = new Set([ + 'audit', + 'telemetry', + 'event', + ]); + + /** + * Is `objectName` an append-only system ledger? — the #5351 carve-out's + * discriminator, and deliberately a property of the object's DECLARATION + * (`lifecycle.class`) rather than of the deployment's routing. + * + * Why the declaration and not "was it routed by step 3": an audit ledger + * pinned to its own datasource by an explicit `datasource:` binding, or by a + * `datasourceMapping` rule, is the same append-only compliance ledger with + * the same reason to be carved out — the routing mechanism is an operator's + * choice, the class is the author's statement about what the data IS. Judging + * by the mechanism would make the carve-out depend on which of three + * equivalent configurations a deployment happened to use. + */ + private isSystemLedgerObject(objectName: string): boolean { + const lifecycleClass = ( + this._registry.getObject(objectName) as { lifecycle?: { class?: string } } | undefined + )?.lifecycle?.class; + return lifecycleClass !== undefined && ObjectQL.SYSTEM_LEDGER_LIFECYCLE_CLASSES.has(lifecycleClass); + } + /** * Helper to get the target driver * @@ -3394,7 +3496,8 @@ export class ObjectQL implements IObjectQLEngine { // the engine — splitting their storage would split their brain. const lifecycleClass = (object as { lifecycle?: { class?: string } } | undefined)?.lifecycle?.class; if ( - (lifecycleClass === 'telemetry' || lifecycleClass === 'event' || lifecycleClass === 'audit') && + lifecycleClass !== undefined && + ObjectQL.SYSTEM_LEDGER_LIFECYCLE_CLASSES.has(lifecycleClass) && this.drivers.has(ObjectQL.LIFECYCLE_DATASOURCE) ) { return this.drivers.get(ObjectQL.LIFECYCLE_DATASOURCE)!; @@ -5133,8 +5236,10 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Insert operation starting', { object, isBatch: Array.isArray(data) }); this.assertWriteAllowed(object, 'insert'); const driver = this.getDriver(object); - // #4619 — diagnostic only, changes nothing about where this write goes. - this.reportWriteOutsideTransaction(object, driver, 'insert'); + // [#5351/#5696] Same-origin gate: refuse a cross-driver BUSINESS write, + // carve an append-only system ledger out of the transaction. Before any + // hook, default or validation runs, so a refusal costs nothing. + this.enforceTransactionOrigin(object, driver, 'insert'); const opCtx: OperationContext = { object, @@ -5497,8 +5602,10 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Update operation starting', { object }); this.assertWriteAllowed(object, 'update'); const driver = this.getDriver(object); - // #4619 — diagnostic only, changes nothing about where this write goes. - this.reportWriteOutsideTransaction(object, driver, 'update'); + // [#5351/#5696] Same-origin gate: refuse a cross-driver BUSINESS write, + // carve an append-only system ledger out of the transaction. Before any + // hook, default or validation runs, so a refusal costs nothing. + this.enforceTransactionOrigin(object, driver, 'update'); // Fold the `filter` alias into `where` FIRST (#4346): everything below — // token resolution, the by-id fast path, the #2982 AST seeding — reads @@ -6150,8 +6257,10 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Delete operation starting', { object }); this.assertWriteAllowed(object, 'delete'); const driver = this.getDriver(object); - // #4619 — diagnostic only, changes nothing about where this write goes. - this.reportWriteOutsideTransaction(object, driver, 'delete'); + // [#5351/#5696] Same-origin gate: refuse a cross-driver BUSINESS write, + // carve an append-only system ledger out of the transaction. Before any + // hook, default or validation runs, so a refusal costs nothing. + this.enforceTransactionOrigin(object, driver, 'delete'); // Fold the `filter` alias into `where` first — same reasoning as update() // above (#4346): unfolded, a `multi: true` delete with `{ filter }` had no @@ -6643,10 +6752,18 @@ export class ObjectQL implements IObjectQLEngine { * 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 reported at `error` - * from the write path (#4619, {@link reportWriteOutsideTransaction}). + * - The transaction covers ONE driver's connection — the default one — as + * ADR-0119 D1 declared and this engine still provides (no two-phase + * commit). What a write routed elsewhere gets is decided by + * {@link enforceTransactionOrigin} (#5351 / #5696, 2026-08-06 ruling): a + * BUSINESS write is refused with + * {@link CrossDatasourceTransactionWriteError}; an append-only SYSTEM + * LEDGER (`lifecycle.class` audit / telemetry / event) is carved out and + * executed OUTSIDE the transaction, so it survives a rollback — the orphan + * row is the deliberate direction of error for a compliance ledger. Either + * way the other driver never receives this transaction's handle, which is + * what the pre-v17 engine did and what put statements on the wrong + * connection entirely. * - 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 @@ -6787,51 +6904,76 @@ export class ObjectQL implements IObjectQLEngine { } /** - * A write inside an open `transaction()` was routed to a driver that - * transaction does not cover (#4619). - * - * `transaction()` opens on the DEFAULT datasource only — declared behaviour - * (ADR-0119 D1) — so an object that `setDatasourceMapping` (or an explicit - * `datasource:` binding, or lifecycle-class separation) routes elsewhere is - * written on another connection entirely. It commits immediately, the - * transaction's rollback cannot reach it, and today NOTHING says so: a failed - * "atomic" multi-datasource write reverts one store, keeps the other, and - * returns a clean rejection either way. - * - * `error`, per AGENTS.md's judgment question — after the degradation the - * system looks entirely normal from the outside while a write it claimed was - * part of an atomic unit has landed on its own. This is the durability class, - * not the functional one. - * - * Diagnostic ONLY: the write still goes exactly where routing sent it. - * Refusing the cross-driver write would change the declared contract and - * belongs to #4619's spec half. + * A write inside an open `transaction()` resolved to a driver that + * transaction does not cover — decide what happens to it (#5351, #5696). + * + * Called at the TOP of `insert`/`update`/`delete`, before hooks, validation + * or defaults run: a refusal here has cost the caller nothing. + * + * This seam replaced `reportWriteOutsideTransaction` (#4619 / PR #5724), + * which reported the split at `error` and let the write proceed with the + * owner's handle. Reporting was the right first move — it made the defect + * audible without pre-empting a decision that changes ADR-0119 D1's declared + * contract for every multi-datasource deployment. The 2026-08-06 maintainer + * ruling made that decision, and it is TWO answers, not one, because the two + * kinds of write fail in opposite directions: + * + * - **Business writes are REFUSED** ({@link CrossDatasourceTransactionWriteError}, + * #5696 point 2). The caller opened a transaction and asked for one unit of + * work; there is no way to give them one across two drivers (no two-phase + * commit on `IDataDriver`, deliberately out of scope). Silently committing + * part of it — which is what the pre-v17 engine did, on the wrong + * connection at that — is the outcome a caller can neither detect nor undo. + * Refusing hands them the choice: one datasource per transaction, or + * per-datasource units they reconcile themselves. + * - **Append-only system ledgers are CARVED OUT** (#5351): audit / telemetry + * / event rows execute OUTSIDE the transaction, on their own connection, + * with no foreign handle. They therefore survive a rollback of the business + * transaction — an "orphan row" describing a write that was undone. For an + * append-only compliance ledger that is the correct direction of error: + * a spurious row is reconcilable, a MISSING row for a write that did + * commit is an unrecoverable compliance hole, and the missing row is what + * shipped before this change. It is also what lets a plugin author write an + * ordinary `afterInsert` audit hook with no knowledge that datasource + * routing exists — refusing here would be swallowed by that hook's + * try/catch and lose the row exactly as before. + * + * No `error` log survives on either path. The refusal IS the report, louder + * than any line; and the carve-out is now DECLARED behaviour that fires on + * every audited write of every transaction in a lifecycle-split deployment — + * logging it at `error`, or even `warn`, would train readers to skim the + * levels that carry real durability failures, which AGENTS.md names as the + * mirror-image mistake. It is recorded at `debug`, once per transaction per + * datasource, for the operator who is asking why an audit row outlived a + * rolled-back write; the durable answer lives in ADR-0067/ADR-0119. */ - private reportWriteOutsideTransaction( + private enforceTransactionOrigin( objectName: string, driver: IDataDriver, operation: 'insert' | 'update' | 'delete', ): void { const scope = this.txStore.getStore()?.scope; // No engine-owned transaction in scope (or a handle threaded explicitly by - // the sandbox trio, which this store never sees) — nothing to be outside of. + // the sandbox trio, which this store never sees) — nothing to be outside + // of. See `transactionCoversDriverFor` for why that limit is declared. if (!scope) return; // Identity, not name: this is about riding the same connection. if (driver === scope.driver) return; const target = this.datasourceNameOf(driver); + + if (!this.isSystemLedgerObject(objectName)) { + throw new CrossDatasourceTransactionWriteError(objectName, operation, target, scope.datasource); + } + if (scope.reportedOutOfScope.has(target)) return; scope.reportedOutOfScope.add(target); - this.logger.error( - `${operation} of '${objectName}' inside transaction() is routed to datasource '${target}', but the ` + - `transaction was opened on the default datasource '${scope.datasource}' and covers only that one — ` + - 'so this write is running OUTSIDE the transaction. It commits on its own the moment it executes, and ' + - "rolling the transaction back will NOT undo it: a failed \"atomic\" unit of work reverts " + - `'${scope.datasource}' while these rows stay behind in '${target}', and the caller is told only that ` + - 'the whole thing failed. Keep every object written inside one transaction() on the default ' + - 'datasource (move the object, or drop the datasourceMapping rule that routes it away), or split the ' + - 'work into per-datasource units and have the caller reconcile them explicitly — cross-driver ' + - 'atomicity is not something this engine provides. Reported once per transaction per datasource.', - undefined, + this.logger.debug( + `${operation} of '${objectName}' inside transaction() is routed to datasource '${target}' while the ` + + `transaction is open on '${scope.datasource}' — executing it OUTSIDE the transaction, on its own ` + + 'connection (ADR-0057 §3.6 system ledger, carved out by #5351). It commits independently and will ' + + 'SURVIVE a rollback of this transaction: an audit/telemetry/event row may describe a write that was ' + + 'undone. That is the decided direction of error for an append-only ledger — an extra reconcilable ' + + 'row beats a missing row for a write that did commit. Said once per transaction per datasource.', { object: objectName, operation, datasource: target, transactionDatasource: scope.datasource }, ); } diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 0c4cc70b8f..f54841f5b9 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -71,6 +71,11 @@ export type { InsertManyRowOutcome } from './engine.js'; // 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'; +// [#5351/#5696] Thrown when a BUSINESS write inside an open transaction() +// resolves to a driver that transaction does not cover. Append-only system +// ledgers (lifecycle.class audit/telemetry/event) are carved out and never +// raise it. Narrow on the class in-process; `code` crosses package boundaries. +export { CrossDatasourceTransactionWriteError } 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 index f8cd07c8ed..2fc244be26 100644 --- a/packages/objectql/src/transaction-errors.ts +++ b/packages/objectql/src/transaction-errors.ts @@ -36,3 +36,50 @@ export class TransactionUnsupportedError extends Error { this.name = 'TransactionUnsupportedError'; } } + +/** + * A BUSINESS write inside an open `transaction()` resolved to a driver that + * transaction does not cover (#5696 point 2, decided together with #5351 by the + * 2026-08-06 maintainer ruling). + * + * `transaction()` opens on ONE driver and covers only that driver's connection. + * Until v17 a write routed elsewhere was handed the open transaction's handle + * anyway, so it executed on the WRONG connection — measured on a real SQL + * driver as `no such table` against a database that never held the object + * (#5351). Reporting it (#4619 / PR #5724) made it audible; this error is the + * decided behaviour: refuse, by name, before anything runs, rather than + * partially commit in silence. + * + * Deliberately NOT what this does: open a companion transaction on the second + * driver. Cross-driver atomicity needs two-phase commit, which `IDataDriver` + * does not have, and faking it would swap one durability risk for a worse one + * (#4619 excludes it explicitly). + * + * Append-only SYSTEM ledgers (`lifecycle.class` of `audit` / `telemetry` / + * `event`) never reach this error — they are carved out and executed outside + * the transaction on their own connection. See `enforceTransactionOrigin` and + * `isSystemLedgerObject` in the engine. + */ +export class CrossDatasourceTransactionWriteError extends Error { + readonly code = 'ERR_CROSS_DATASOURCE_TRANSACTION_WRITE' as const; + + constructor( + public readonly object: string, + public readonly operation: 'insert' | 'update' | 'delete', + public readonly datasource: string, + public readonly transactionDatasource: string, + ) { + super( + `${operation} of '${object}' inside transaction() resolves to datasource '${datasource}', but the ` + + `transaction is open on '${transactionDatasource}' and covers only that connection — refused. ` + + 'Executing it anyway would either run the statement on the wrong connection (the pre-v17 defect: ' + + 'the row lands in a database that may not even have the table) or commit it on its own, so a ' + + 'rollback of this transaction would leave it behind while the caller is told only that the whole ' + + 'unit failed. Fix it one of two ways: keep every object written inside one transaction() on one ' + + `datasource (move the object, or drop the datasourceMapping rule that routes '${object}' to ` + + `'${datasource}'), or split the work into per-datasource units and have the caller reconcile them ` + + 'explicitly — cross-driver atomicity is not something this engine provides. Nothing was written.', + ); + this.name = 'CrossDatasourceTransactionWriteError'; + } +} diff --git a/scripts/adr-anchors.json b/scripts/adr-anchors.json index 11760f35fb..5858cce456 100644 --- a/scripts/adr-anchors.json +++ b/scripts/adr-anchors.json @@ -136,9 +136,10 @@ { "file": "packages/spec/src/contracts/objectql-engine.ts", "adrs": [ + "ADR-0067", "ADR-0119" ], - "invariant": "`transaction` is DECLARED on the `objectql` slot contract — plugin space reaches ADR-0034's ambient transaction by name, not through `as unknown as` casts. Required, not optional, per this file's own rule. Its two caveats (default-driver only; the callback runs with NO transaction when the driver lacks `beginTransaction`) are part of the declared meaning, so a caller that cannot lose atomicity silently must fail closed rather than assume it held." + "invariant": "`transaction` is DECLARED on the `objectql` slot contract — plugin space reaches ADR-0034's ambient transaction by name, not through `as unknown as` casts. Required, not optional, per this file's own rule. Its two ADR-0119 D1 caveats are no longer plain caveats (2026-08-06 ruling on #5351/#5696): the no-`beginTransaction` degrade stays the DEFAULT but `opts.require: true` refuses it, and single-driver coverage is now enforced rather than merely stated — a cross-driver BUSINESS write inside a transaction is refused by name, an append-only SYSTEM LEDGER (`lifecycle.class` audit/telemetry/event) is carved out and executed outside it (surviving rollback: the orphan row is the decided cost for a compliance ledger), and no driver is ever handed a transaction handle it does not own. The callback's `owned` argument tells it whether it opened the transaction or JOINED an outer one (ADR-0067 D2). Do not re-describe the old 'routed elsewhere are written outside it' text: measurement disproved it — they were written on the OWNER's connection." }, { "file": "packages/metadata-protocol/src/protocol.ts", @@ -250,7 +251,16 @@ "invariant": "The concrete instance of D11's registration-ownership rule: the `i18n` slot is MULTI-PROVIDER — I18nServicePlugin when service-i18n is installed, else the AppPlugin in-memory fallback auto-registered for stacks that declare translation bundles — so the dispatcher registers this route, not the provider. Moving `/i18n` registration into service-i18n, the obvious-looking 'the owning package should own its route' cleanup, 404s every stack served by the other provider. The extracted body also keeps the legacy matching semantics deliberately (`match: 'prefix'`, so `/i18nxx` matches too, exactly as the old `startsWith` chain did): normalizing that edge is a behaviour change for the http-conformance suite to re-pin, not a tidy-up to slip into an unrelated diff. The 501 on an unserveable slot is D12's honest-capability rule and is narrow on purpose — both real in-memory providers self-declare `degraded` with `handlerReady: true` and keep serving, so the gate closes only on an occupant that would answer with invented strings." }, { - "file": "packages/spec/src/type-alias-convention.pin.test.ts", + "file": "packages/objectql/src/engine.ts", + "adrs": [ + "ADR-0057", + "ADR-0067", + "ADR-0119" + ], + "invariant": "The transaction seam is same-origin by construction (#5351/#5696, 2026-08-06 ruling). `buildDriverOptions` threads the ambient handle ONLY to the driver that owns it, compared by instance identity via `TransactionScope` — handing it to another driver does not put that driver inside the transaction, it runs the statement on the wrong connection. `enforceTransactionOrigin` then decides the write: an append-only system ledger (`lifecycle.class` in `SYSTEM_LEDGER_LIFECYCLE_CLASSES`, the same tuple ADR-0057 §3.6 routing reads — never a second copy) is CARVED OUT and executed outside the transaction, surviving rollback; any other cross-driver write is REFUSED before hooks run. Never add a companion transaction on the second driver (no two-phase commit in IDataDriver) and never restore the unconditional handle lift." + }, + { +"file": "packages/spec/src/type-alias-convention.pin.test.ts", "adrs": [ "ADR-0122" ],