Skip to content

Commit 9a86949

Browse files
os-zhuangclaude
andauthored
feat(spec,metadata-protocol): ADR-0118 — transactions reach plugin space through the contract, and atomic stops lying (#4612) (#4623)
* 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 * docs(api): true up the batch `atomic` docs that D4 made wrong (#4612) The docs-drift check flagged the hand-written API docs, and three of them were genuinely stale — they documented the bug ADR-0118 D4 fixes: - `wire-format.mdx` stated it outright: "By default (`options.atomic: true`) processing stops at the first failing record — records already written earlier in the same batch are **not** rolled back, since there is no wrapping database transaction." Accurate before, wrong on both halves now. - `client-sdk.mdx`'s options table listed `atomic` default `true` with "Rollback entire batch on any failure" — the default moved, and the rollback claim only became true with this change. - `data-api.mdx` described `atomic` as the default on `/batch` and `deleteMany`. Each now describes what the route actually does, including the 501 refusal and the rolled-back response shape (`succeeded: 0`, ROLLED_BACK / NOT_ATTEMPTED). Kept honest about scope: `deleteMany` and `updateMany` did NOT get D4's treatment, so `deleteMany`'s note now says plainly that its `atomic` only stops the run and rolls nothing back (#4620 tracks the fix). Documenting the fixed behaviour on routes that still carry the bug would have replaced one inaccuracy with another. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 73d701f commit 9a86949

15 files changed

Lines changed: 1062 additions & 25 deletions
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/api/client-sdk.mdx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,9 +504,13 @@ The `find` method accepts an options object with **canonical** (recommended) fie
504504

505505
| Property | Type | Default | Description |
506506
|:---------|:-----|:--------|:------------|
507-
| `atomic` | `boolean` | `true` | Rollback entire batch on any failure |
507+
| `atomic` | `boolean` | `false` | Run the batch in one transaction and roll every write back on the first failure. Refused with `501 NOT_IMPLEMENTED` where the driver cannot roll back, rather than degrading to best-effort. Takes precedence over `continueOnError` |
508508
| `returnRecords` | `boolean` | `false` | Include full records in response |
509-
| `continueOnError` | `boolean` | `false` | Continue after errors (when atomic=false) |
509+
| `continueOnError` | `boolean` | `false` | Continue after errors (when atomic is false) |
510+
511+
A rolled-back atomic batch reports `succeeded: 0`, with each row carrying
512+
`ROLLED_BACK:`, the causal error, or `NOT_ATTEMPTED:` — no row is reported as a
513+
success, because none of them survived.
510514

511515
---
512516

content/docs/api/data-api.mdx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ Execute a batch operation (create / update / upsert / delete) on multiple record
228228

229229
**Response**: `BatchUpdateResponse` with `succeeded`, `failed`, `total`, and a per-record `results` array. Each entry in `results` has `id`, `success`, an optional `errors` array, and optional `data` (the full record, present when `returnRecords` is `true`).
230230

231+
`options.atomic` defaults to `false` (sequential best-effort, stopping at the first failure). Set it to `true` and the whole batch runs inside one transaction: the first failure rolls back every prior write, and the response reports `succeeded: 0` with each row marked `ROLLED_BACK:`, the causal error, or `NOT_ATTEMPTED:`. A deployment whose driver cannot roll back rejects an atomic request with `501 NOT_IMPLEMENTED` instead of running it best-effort — probe `capabilities.transactionalBatch` on `/discovery` first. `atomic` takes precedence over `continueOnError`.
232+
231233
### `POST /data/:object/createMany`
232234

233235
Batch create multiple records.
@@ -260,16 +262,20 @@ ignored, on this route and on `deleteMany`.
260262

261263
Batch delete records by ID list.
262264

263-
**Body**: `{ "ids": ["1", "2", "3"], "options": { "atomic": true } }``options` is
265+
**Body**: `{ "ids": ["1", "2", "3"], "options": { "continueOnError": true } }``options` is
264266
the same `BatchOptions` bag `/batch` takes. The body is validated against the
265267
contract and unknown keys are dropped: the id list is the *only* thing that
266268
selects rows, so no body key can widen the delete into a filter.
267269

268270
**Response**: `BatchUpdateResponse` — one `results` entry per id. Records are
269271
deleted one at a time by primary key, so each honours `deleteBehavior`
270-
(`cascade` / `set_null` / `restrict`) on relations pointing at it. `atomic`
271-
(default) stops the run at the first failure; `atomic: false` with
272-
`continueOnError: true` processes the remaining ids and reports the failures.
272+
(`cascade` / `set_null` / `restrict`) on relations pointing at it. The run stops
273+
at the first failure; `continueOnError: true` processes the remaining ids and
274+
reports the failures instead.
275+
276+
Note that on this route `atomic` only stops the run — deletes already performed
277+
are **not** rolled back. Unlike `/batch`, `deleteMany` has no wrapping
278+
transaction yet.
273279

274280
### Batch size
275281

content/docs/api/wire-format.mdx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,11 @@ release group); any code reading them directly should move to `error.code`.
471471

472472
**`POST /api/v1/data/task/batch`**
473473

474-
Process many records of a **single** operation type in one request. The body carries one `operation` (`create`, `update`, `upsert`, or `delete`) plus a `records` array. By default (`options.atomic: true`) processing stops at the first failing record — records already written earlier in the same batch are **not** rolled back, since there is no wrapping database transaction. Set `options.atomic: false` (with `options.continueOnError: true`) to keep processing every record and collect a full partial-success report.
474+
Process many records of a **single** operation type in one request. The body carries one `operation` (`create`, `update`, `upsert`, or `delete`) plus a `records` array.
475+
476+
By default (`options.atomic` omitted or `false`) processing stops at the first failing record, and records written earlier in the same batch are **not** rolled back — there is no wrapping transaction. Add `options.continueOnError: true` to keep going instead and collect a full partial-success report.
477+
478+
Send `options.atomic: true` to run the whole batch inside one database transaction: the first failure rolls back every prior write, and the response reports zero successes with each row marked `ROLLED_BACK:`, the causal error, or `NOT_ATTEMPTED:`. A deployment whose driver cannot roll back **rejects** an atomic request with `501 NOT_IMPLEMENTED` rather than quietly running it best-effort — probe `capabilities.transactionalBatch` on `/discovery` to know in advance. `atomic` takes precedence over `continueOnError`.
475479

476480
### Request
477481

@@ -483,7 +487,7 @@ Process many records of a **single** operation type in one request. The body car
483487
{ "id": "tsk_01HQ4B8C0E4G6H9K3L5M", "data": { "status": "done" } }
484488
],
485489
"options": {
486-
"atomic": true,
490+
"atomic": false,
487491
"continueOnError": false
488492
}
489493
}
@@ -509,7 +513,7 @@ The response is the `BatchUpdateResponse` envelope: a top-level `success` flag p
509513

510514
### Partial Failure Response
511515

512-
When `options.atomic: false` and some records fail, the failing entries carry a single `error` message string (not an array):
516+
When the batch is not atomic and some records fail, the failing entries carry a single `error` message string (not an array). An atomic batch never returns this shape — it either commits everything or reports every row as failed:
513517

514518
```json
515519
{

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)