Skip to content

Commit 488b66c

Browse files
os-zhuangclaude
andauthored
fix(objectql): make transaction()'s two declared caveats observable (#4619) (#5724)
`ObjectQL.transaction()` carries two caveats that are part of its DECLARED meaning (ADR-0119 D1, packages/spec/src/contracts/objectql-engine.ts), not hidden behaviour: it degrades to no-transaction/no-rollback when the default driver has no `beginTransaction`, and it covers the DEFAULT datasource only, so an object routed elsewhere by `setDatasourceMapping` is written outside it. Declaring them is not the same as being able to observe them, and both were completely mute. A caller asking for atomicity and not getting it had no way to find out; a multi-datasource "atomic" unit of work that partially committed reported nothing — one store reverted, the other kept its rows, and the caller saw only that the whole thing failed. Same shape as `batchData`'s `atomic` flag being a lie for as long as it was (ADR-0119 D4). Behaviour is unchanged. This is the observability half only: - `warnTransactionUnsupported` — `warn`, once per driver per engine instance, on the degrade path. Names the driver, the consequence and the fix. `warn` rather than `error` per AGENTS.md's judgment question: at that moment nothing has been lost, a capability is simply absent, which is the `if (!capability)` composition branch that section keeps at `warn`. - `reportWriteOutsideTransaction` — `error`, once per transaction per datasource, when an insert/update/delete inside an open transaction resolves to a driver that transaction does not cover. `error` because afterwards the system looks entirely normal from the outside while a write it claimed was part of an atomic unit has landed on its own: the durability class. - `TransactionScope` on the ambient `txStore` entry records WHICH driver owns the open transaction. Observability record only — it routes nothing. Both surfaces report through the same engine-side helpers, so the direct (`engine.transaction`) and sandboxed (`ScopedContext.transaction`, reached as `ctx.api.transaction`) paths share one budget and one wording. Tightening either caveat — an `opts.require` that throws, refusing a cross-driver write, or an owned-vs-joined signal in the callback signature — changes the contract's declared semantics and is deliberately NOT done here; that half of #4619 belongs to `packages/spec`. Note for #5351: the `TransactionScope.driver` added here is exactly the "txStore records the owning driver" prerequisite its recommended option A names. This PR only reports the mismatch; it does not gate the handle. Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx Co-authored-by: Claude <noreply@anthropic.com>
1 parent d8f65fe commit 488b66c

3 files changed

Lines changed: 664 additions & 7 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): say out loud when `transaction()` is not giving you a transaction (#4619)
6+
7+
`ObjectQL.transaction()` carries two caveats that are part of its **declared**
8+
meaning (ADR-0119 D1, `packages/spec/src/contracts/objectql-engine.ts`), not
9+
hidden behaviour:
10+
11+
1. when the default driver has no `beginTransaction`, the callback runs with no
12+
transaction and no rollback;
13+
2. the transaction covers the **default** datasource only, so an object routed
14+
elsewhere by `setDatasourceMapping` is written outside it.
15+
16+
Declaring them is not the same as being able to observe them, and both were
17+
completely mute. A caller asking for atomicity and not getting it had no way to
18+
find out; a multi-datasource "atomic" unit of work that partially committed
19+
reported nothing at all — one store reverted, the other kept its rows, and the
20+
caller saw only that the whole thing failed. That is the same shape as
21+
`batchData`'s `atomic` flag being a lie for as long as it was (ADR-0119 D4).
22+
23+
**Nothing about what the engine does has changed.** Both caveats still hold
24+
exactly as declared; this release only makes them discoverable.
25+
26+
- **`warn`, once per driver per engine instance**, when `transaction()` (or
27+
`ctx.api.transaction()` in a sandboxed hook/action body) degrades because the
28+
driver has no `beginTransaction`. The line names the driver, the consequence
29+
— writes commit as they execute, so a later throw leaves the earlier ones
30+
persisted while the call rejects as if nothing had landed — and the fix.
31+
`warn` rather than `error` on purpose: at that moment nothing has been lost,
32+
a capability is simply absent, which is the functional-degradation branch
33+
AGENTS.md keeps at `warn`. Once per driver because the drivers that reach
34+
this path (test doubles, foreign engines) reach it on *every* call.
35+
36+
- **`error`, once per transaction per datasource**, when an `insert`/`update`/
37+
`delete` inside an open `transaction()` is routed to a driver that
38+
transaction does not cover. The line names the object, the datasource it went
39+
to, the datasource the transaction was opened on, and says the write commits
40+
on its own and will survive the rollback. `error` per AGENTS.md's judgment
41+
question: afterwards the system looks entirely normal from the outside while
42+
a write it claimed was part of an atomic unit has landed by itself — the
43+
durability class, not the functional one.
44+
45+
Both diagnostics are reported by the engine, so the direct
46+
(`engine.transaction`) and sandboxed (`ScopedContext.transaction`) surfaces
47+
share one budget and one wording rather than drifting apart.
48+
49+
Tightening either caveat — an `opts.require` that throws instead of degrading,
50+
refusing a cross-driver write, or surfacing an owned-vs-joined signal to the
51+
callback — would change the contract's declared semantics and is deliberately
52+
**not** done here; that half of #4619 is tracked separately against
53+
`packages/spec`.

0 commit comments

Comments
 (0)