Skip to content

Commit 7236bb0

Browse files
committed
feat(spec,metadata-protocol): ADR-0118 — transactions reach plugin space through the contract, and atomic stops lying (#4612)
#4612 asks the platform to choose a debt: transactionalize the engine, bless a shared migration-journal primitive, or rule both out. Scoping it found the first option's expensive half was paid three majors ago — ADR-0034's ambient transaction is implemented, and hooks/validation/internal reads already join it — while the cheap half was never done. So this records ADR-0118, which splits the ask along where the work actually is. D1 (here) — `transaction` joins `IObjectQLEngine`. The mechanism ships; only the declaration was missing, so the metadata protocol's atomic publish, its `transactionalBatch` probe, and the sys-metadata repository each reached it through `as unknown as` casts — unchecked claims about a class none of them import. The contract's own evidence bar was met three times over. Its caveats (default-driver only; runs without a transaction when the driver lacks `beginTransaction`) go into the TSDoc as declared meaning, not behaviour to be discovered. Narrow host surfaces now type their optional member as `IObjectQLEngine['transaction']` so they cannot drift from it. D4 (here) — `batchData`'s `atomic` opened no transaction. It broke a loop: prior writes stayed committed while the response reported them `success: true` under the one flag guaranteeing they were undone — #4346's class, silent and destructive. It now runs the batch inside one `engine.transaction()`, and a failure rolls back everything and says so (`succeeded: 0`, rows marked ROLLED_BACK / NOT_ATTEMPTED). A runtime that cannot roll back is REFUSED with 501, never degraded — silent degradation is how the flag came to lie. `BatchOptionsSchema.atomic`'s declared `.default(true)` is aligned down to the enforced `false`, so opting in is explicit and no existing caller's failure semantics change underneath them. D2 (#4617) specifies a framework-owned migration-journal runner: transactions cannot span a million-row backfill, a whole-DB-snapshot driver, a multi- datasource write, or a process crash, so the four queued consumers need journal + compensation + resume on top of transactions, not instead of them. D3 (#4618) retires `IDataEngine.batch?` — declared, implemented by nobody, called by nobody. Caveat hardening is #4619; the sibling fake-atomics in `deleteManyData`/`updateManyData` are #4620. Tests: 10 unit pins on the atomic contract (rollback shape, 501 refusal with zero writes attempted, precedence, non-atomic regression) and 6 integration pins over a real engine + transacting driver, including that an internal read during the batch binds to the open transaction — the no-deadlock coverage whose absence ADR-0034 was written about. Closes #4612 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx
1 parent 0c681c3 commit 7236bb0

12 files changed

Lines changed: 1039 additions & 16 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata-protocol": minor
4+
---
5+
6+
feat(spec,metadata-protocol): `IObjectQLEngine.transaction` joins the slot contract, and `batchData`'s `atomic` flag becomes real — rollback or refusal, never silent best-effort (ADR-0118 D1/D4, #4612)
7+
8+
**D1 — the contract fix.** `ObjectQL.transaction()` — ADR-0034's ambient
9+
transaction, shipped since v8.0.0 — was reachable from plugin space only
10+
through `as unknown as` casts: the metadata protocol's atomic publish and its
11+
`transactionalBatch` discovery probe, and the sys-metadata repository's
12+
`withTxn`, each declared a private structural slice of an engine none of them
13+
import. It is now declared on `IObjectQLEngine`, required per that contract's
14+
own rule, with its caveats written into the TSDoc as part of the declared
15+
meaning rather than left to be discovered: it covers the **default driver
16+
only**, and when that driver has no `beginTransaction` the callback runs with
17+
no transaction and no rollback. `MetadataHostEngine` and the sys-metadata
18+
repository's engine surface now type their optional member as
19+
`IObjectQLEngine['transaction']`, so a narrow host surface can no longer drift
20+
from the real signature. Runtime `typeof === 'function'` probes stay — that is
21+
test-double defence the type system does not replace.
22+
23+
**D4 — the honesty fix.** `batchData`'s `options.atomic` promised "rollback
24+
entire batch on any failure (transaction mode)" and delivered a `break`
25+
statement. Every write before the failure stayed committed, and — the part that
26+
did the real damage — the response reported those rows `success: true` under
27+
the one flag whose job is to guarantee they were undone.
28+
29+
Now an explicitly atomic batch runs inside ONE `engine.transaction()`: the
30+
first failure rolls back every prior write, and the response says so
31+
(`succeeded: 0`, with rows marked `ROLLED_BACK:` / the causal error /
32+
`NOT_ATTEMPTED:`, and no row reporting success). On a runtime that cannot roll
33+
back — no `transaction()`, or a default driver without `beginTransaction` — an
34+
atomic request is **refused** with `501 NOT_IMPLEMENTED` rather than silently
35+
degrading, matching the cross-object `/batch` route. `atomic` takes precedence
36+
over `continueOnError`, whose own description already scoped it to
37+
`atomic=false`. In atomic mode the upsert path no longer falls back to an
38+
insert when its update throws: inside an aborted transaction that fallback can
39+
only fail with a secondary error that buries the real cause.
40+
41+
**Aligned declaration.** `BatchOptionsSchema.atomic` declared `.default(true)`
42+
while no enforcement site delivered atomicity — and the REST route forwards the
43+
original request body rather than the parsed output, so the declared default
44+
never reached the loop at all. The default is now `false`: the declaration is
45+
aligned down to what every site already does, rather than up to what none of
46+
them did. Honouring the old `true` would have silently flipped the failure
47+
semantics of every existing batch caller and hard-failed ordinary batches on
48+
any driver that cannot transact. Callers who were explicitly sending
49+
`atomic: true` now get what they always asked for; callers sending nothing keep
50+
today's behaviour exactly.
51+
52+
If you were passing `atomic: true` and relying on partial results surviving a
53+
failure, that was the bug — switch to `atomic: false` (or omit it) for
54+
best-effort semantics.
55+
56+
ADR-0118 also rules on two items landing separately: D2 specifies a
57+
framework-owned migration-journal runner for multi-step migrations too large
58+
for one transaction, and D3 retires the declared-but-unimplemented
59+
`IDataEngine.batch?`.

content/docs/references/api/batch.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ const result = BatchConfigSchema.parse(data);
8686

8787
| Property | Type | Required | Description |
8888
| :--- | :--- | :--- | :--- |
89-
| **atomic** | `boolean` || If true, rollback entire batch on any failure (transaction mode) |
89+
| **atomic** | `boolean` || Opt-in all-or-nothing. When explicitly true the whole batch runs inside ONE engine transaction: the first failure rolls back every prior write, and the response reports zero successes with rows marked ROLLED_BACK / NOT_ATTEMPTED. A runtime that cannot roll back REFUSES the request (501 NOT_IMPLEMENTED) rather than silently degrading to best-effort — probe `capabilities.transactionalBatch` on /discovery first. Takes precedence over continueOnError. Default false: sequential best-effort. |
9090
| **returnRecords** | `boolean` || If true, return full record data in response |
9191
| **continueOnError** | `boolean` || If true (and atomic=false), continue processing remaining records after errors |
9292
| **validateOnly** | `any` | optional | [REMOVED] `options.validateOnly` was removed from BatchOptions in @objectstack/spec (#4052). It was never implemented: the batch surfaces persisted regardless, so a "dry-run" would have silently executed. There is no dry-run today — drop the key. If you need to preview a batch without writing, open an issue so it can be designed (no-commit cascade / constraint semantics) and reintroduced as a flag that actually holds. |

0 commit comments

Comments
 (0)