fix(driver-memory): make bulkCreate all-or-nothing so a refused batch leaves no surviving prefix

.changeset/bulk-create-all-or-nothing.md

@@ -0,0 +1,22 @@
+---
+"@objectstack/driver-memory": patch
+---
+
+fix(driver-memory): `bulkCreate` refuses before writing, so a rejected batch leaves no surviving prefix (#13340)
+
+`InMemoryDriver.bulkCreate` was `Promise.all(dataArray.map(data => this.create(...)))`, and
+`create` writes into the table synchronously. So a batch was **not atomic**: when any row was
+refused, every row accepted *before* it stayed in the store, and the caller got a rejection
+describing a batch that had partly landed. Measured on a two-row table, a refused two-row
+batch left **three** rows behind, which made retrying the same array unsafe for reasons that
+had nothing to do with constraints.
+
+`bulkCreate` now builds and checks every row — against the table **and against the rest of
+the batch** — before pushing any of them. That is the posture `updateMany` has had since
+#13197, one method over, and the one `driver-sql` gets from sending a batch as a single
+insert; the two batch doors of this driver no longer give opposite answers to "is a batch
+atomic?".
+
+`create`'s own single-row path is unchanged, and this does not give the store a primary key:
+an undeclared duplicate `id` is still not a constraint violation, so such a batch still lands
+in full.

packages/drivers/driver-memory/src/memory-bulk-create-atomicity.test.ts

@@ -0,0 +1,205 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#13340] `bulkCreate` is ALL-OR-NOTHING: a refused row leaves the table
+ * exactly as it found it.
+ *
+ * ## The defect this pins, and why the obvious assertion would not have caught it
+ *
+ * `bulkCreate` used to be one line — `Promise.all(dataArray.map(data =>
+ * this.create(object, data, options)))` — and `create` writes into the table
+ * synchronously. So when any row of a batch was refused, every row accepted
+ * BEFORE it stayed in the store and the caller got a rejection describing a
+ * batch that had partly landed. Measured on `main` before the fix, on a
+ * two-row table whose incoming batch collides with itself:
+ *
+ * ```
+ * before: 2 rows
+ * bulkCreate([A9/Z, A9/Z]) -> rejects UNIQUE_VIOLATION / 409
+ * after: 3 rows <- the FIRST batch row landed. That is the defect.
+ * ```
+ *
+ * ⛔ Asserting "the refusal still happens" proves NOTHING here — the refusal
+ * was already correct, and #13197 / #13239 already pin it. That assertion was
+ * green throughout the defect's entire life. **The discriminating fact is that
+ * THE ROW COUNT DOES NOT MOVE**, so every test below reads the store after the
+ * refusal rather than stopping at the envelope.
+ *
+ * The fix is `updateMany`'s posture, one method over: #13197 made that method
+ * prepare and check every row before mutating any of it, precisely so a
+ * half-applied batch cannot happen. The two batch paths of this driver now
+ * give the SAME answer to "is a batch atomic?" — they disagreed for as long as
+ * `bulkCreate` existed, seven lines apart in one file.
+ *
+ * It is also the batch posture of the family this driver stands in for:
+ * `driver-sql` sends the whole batch as one insert, so a constraint failure
+ * there leaves the table untouched.
+ *
+ * ## What this does NOT claim
+ *
+ * Atomicity here is the driver refusing before it writes — not a transaction.
+ * There is no rollback: nothing is written until the whole batch has been
+ * checked. And the store still has **no primary key** (see the boundary
+ * describe at the bottom): an undeclared duplicate `id` is not a constraint
+ * violation on this driver, so such a batch lands in full. Atomicity is about
+ * what happens when a row IS refused, not about what gets refused.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { InMemoryDriver } from './memory-driver.js';
+
+interface WireBearingError extends Error {
+ code?: string;
+ status?: number;
+}
+
+/** Run `fn`, requiring it to reject; hand back the rejection for inspection. */
+async function refusalOf(fn: () => Promise<unknown>): Promise<WireBearingError> {
+ try {
+ await fn();
+ } catch (e) {
+ return e as WireBearingError;
+ }
+ throw new Error('expected the driver to refuse this write, but it resolved');
+}
+
+const DOC_SCHEMA = {
+ name: 'doc',
+ fields: {
+ id: { type: 'text' },
+ doc_no: { type: 'text', unique: 'global' },
+ title: { type: 'text' },
+ },
+} as any;
+
+describe('[#13340] bulkCreate refuses BEFORE writing — no surviving prefix', () => {
+ let driver: InMemoryDriver;
+
+ beforeEach(async () => {
+ driver = new InMemoryDriver();
+ await driver.syncSchema('doc', DOC_SCHEMA);
+ await driver.create('doc', { id: '1', doc_no: 'D-0001' });
+ await driver.create('doc', { id: '2', doc_no: 'D-0002' });
+ });
+
+ it('a batch colliding with ITSELF leaves the row count where it was', async () => {
+ // The card's own measured reading, inverted: this used to leave 3 rows.
+ const err = await refusalOf(() =>
+ driver.bulkCreate('doc', [
+ { id: 'a', doc_no: 'D-0100' },
+ { id: 'b', doc_no: 'D-0100' },
+ ]),
+ );
+ expect(err.code).toBe('UNIQUE_VIOLATION');
+ expect(err.status).toBe(409);
+
+ expect(await driver.count('doc')).toBe(2);
+ // Named explicitly: it is the row ACCEPTED BEFORE the refusal that used to
+ // survive, so its absence is the whole fix.
+ expect(await driver.find('doc', { fields: ['id'], where: { id: 'a' } })).toHaveLength(0);
+ expect(await driver.find('doc', { fields: ['id'], where: { id: 'b' } })).toHaveLength(0);
+ });
+
+ it('a batch colliding with a STORED row leaves the row count where it was', async () => {
+ // The collision is in the LAST row, so rows 'a' and 'b' were both accepted
+ // before the refusal — two survivors under the old behaviour, not one.
+ const err = await refusalOf(() =>
+ driver.bulkCreate('doc', [
+ { id: 'a', doc_no: 'D-0100' },
+ { id: 'b', doc_no: 'D-0101' },
+ { id: 'c', doc_no: 'D-0001' },
+ ]),
+ );
+ expect(err.code).toBe('UNIQUE_VIOLATION');
+ expect(err.status).toBe(409);
+
+ expect(await driver.count('doc')).toBe(2);
+ const survivors = await driver.find('doc', { fields: ['id'] });
+ expect(survivors.map((r: any) => r.id).sort()).toEqual(['1', '2']);
+ });
+
+ it('the FIRST row colliding refuses the batch too — the check is not order-dependent', async () => {
+ const err = await refusalOf(() =>
+ driver.bulkCreate('doc', [
+ { id: 'a', doc_no: 'D-0001' },
+ { id: 'b', doc_no: 'D-0200' },
+ ]),
+ );
+ expect(err.code).toBe('UNIQUE_VIOLATION');
+ expect(await driver.count('doc')).toBe(2);
+ // The row AFTER the refusal must not land either — a check that bailed out
+ // of the loop but had already pushed would still pass the count above if
+ // it pushed nothing, so name the later row directly.
+ expect(await driver.find('doc', { fields: ['id'], where: { id: 'b' } })).toHaveLength(0);
+ });
+
+ it('a clean batch still lands in FULL and returns every row', async () => {
+ // The non-vacuity control for the controls: a fix that refused everything
+ // would pass every assertion above.
+ const out = await driver.bulkCreate('doc', [
+ { id: 'a', doc_no: 'D-0100' },
+ { id: 'b', doc_no: 'D-0101' },
+ ]);
+ expect(out).toHaveLength(2);
+ expect(out.map((r: any) => r.doc_no)).toEqual(['D-0100', 'D-0101']);
+ expect(await driver.count('doc')).toBe(4);
+ });
+
+ it('an empty batch is a no-op that resolves to an empty array', async () => {
+ expect(await driver.bulkCreate('doc', [])).toEqual([]);
+ expect(await driver.count('doc')).toBe(2);
+ });
+
+ it('the two batch doors of this driver now AGREE that a batch is atomic', async () => {
+ // The card's actual complaint: `bulkCreate` and `updateMany` gave opposite
+ // answers to one question, in one file, seven lines apart. Both are asked
+ // here so the pair cannot drift apart again silently.
+ const createErr = await refusalOf(() =>
+ driver.bulkCreate('doc', [
+ { id: 'a', doc_no: 'D-0100' },
+ { id: 'b', doc_no: 'D-0100' },
+ ]),
+ );
+ const updateErr = await refusalOf(() => driver.updateMany('doc', { where: {} }, { doc_no: 'D-0009' }));
+
+ expect(createErr.code).toBe('UNIQUE_VIOLATION');
+ expect(updateErr.code).toBe('UNIQUE_VIOLATION');
+ // Neither door moved the store.
+ expect(await driver.count('doc')).toBe(2);
+ const rows = await driver.find('doc', { fields: ['id', 'doc_no'], orderBy: [{ field: 'id', order: 'asc' }] });
+ expect(rows.map((r: any) => r.doc_no)).toEqual(['D-0001', 'D-0002']);
+ });
+});
+
+describe('[#13340] the boundary — atomicity did NOT give this store a primary key', () => {
+ it('an UNDECLARED duplicate `id` is still not a violation, so that batch lands in full', async () => {
+ // Load-bearing for the docstring on `InMemoryDriver`, which tells readers
+ // this store has no primary key and that `bulkCreate` lands two rows with
+ // the same `id` where a SQL driver raises. #13340 did NOT change that: the
+ // batch is refused only when a DECLARED constraint refuses a row, and
+ // `id` here declares nothing. Without this test the atomicity work above
+ // reads as "bulkCreate now rejects duplicate ids", which it does not.
+ const driver = new InMemoryDriver();
+ await driver.syncSchema('note', {
+ name: 'note',
+ fields: { id: { type: 'text' }, body: { type: 'text' } },
+ } as any);
+
+ const out = await driver.bulkCreate('note', [
+ { id: 'dup', body: 'first' },
+ { id: 'dup', body: 'second' },
+ ]);
+
+ expect(out).toHaveLength(2);
+ expect(await driver.count('note')).toBe(2);
+ // ...and a read returns BOTH, exactly as the docstring says.
+ expect(await driver.find('note', { fields: ['id', 'body'], where: { id: 'dup' } })).toHaveLength(2);
+ });
+
+ it('an object never passed through syncSchema is unconstrained, batch included', async () => {
+ const driver = new InMemoryDriver();
+ const out = await driver.bulkCreate('undeclared', [{ id: 'x', v: 1 }, { id: 'x', v: 2 }]);
+ expect(out).toHaveLength(2);
+ expect(await driver.count('undeclared')).toBe(2);
+ });
+});

packages/drivers/driver-memory/src/memory-declared-index-unique.test.ts

@@ -473,19 +473,26 @@ describe('[#13239] every write path is checked, not just create', () => {
  ]),
  );
  expectUniqueViolationEnvelope(err, 'account_id', 'code');
- // The DUPLICATE did not land — that is the constraint, and it is what this
- // asserts. ⚠️ `bulkCreate` is `Promise.all(map(create))`, so rows accepted
- // BEFORE the refusal stay: the batch is not atomic. That is older than
- // either uniqueness card (any mid-batch `create` failure has always left a
- // partial batch) and identical on the field surface, so it is recorded here
- // as a known boundary rather than silently re-baselined — never "the
- // constraint half-applied".
+ // [#13340] ⛔ The refusal is NOT what this test is for — the refusal was
+ // already correct, and #13197/#13239 already pin it. What it asserts is
+ // that THE ROW COUNT DOES NOT MOVE: the batch is all-or-nothing.
+ //
+ // This assertion was INVERTED in place, not re-baselined. It used to read
+ // `toHaveLength(1)` / `toBe(3)` and recorded the opposite behaviour as a
+ // known boundary: `bulkCreate` was `Promise.all(map(create))`, so the row
+ // accepted BEFORE the refusal stayed in the store and a refused 2-row
+ // batch left a 2-row table holding THREE rows. #13340 gave `bulkCreate`
+ // the check-then-push posture `updateMany` has had since #13197, so
+ // neither row lands now. The old numbers are kept in this comment
+ // deliberately: they are the discriminating reading, and an assertion
+ // that only checked "the refusal still happens" would pass in both
+ // worlds.
  const colliding = await driver.find('ledger', {
  fields: ['id'],
  where: { account_id: 'A9', code: 'Z' },
  });
- expect(colliding).toHaveLength(1);
- expect(await driver.count('ledger')).toBe(3);
+ expect(colliding).toHaveLength(0);
+ expect(await driver.count('ledger')).toBe(2);
  });
 
  it('updateMany refuses BEFORE mutating anything — no half-applied batch', async () => {

packages/drivers/driver-memory/src/memory-driver.ts

@@ -202,12 +202,31 @@ interface MemoryTransaction {
  * (`createWithAutonumberResync`) is triggered by the STORE rejecting the
  * duplicate and this store rejected nothing.
  *
+ * Since #13340 {@link bulkCreate} is ALL-OR-NOTHING: it builds and checks every
+ * row — against the table AND against the rest of the batch — before it pushes
+ * any of them, so a refused row leaves the table exactly as it found it. That
+ * is the posture {@link updateMany} has had since #13197, and the one
+ * `driver-sql` gets from sending a batch as a single insert. `bulkCreate` used
+ * to be `Promise.all(map(create))`, where a refusal left every row accepted
+ * BEFORE it standing — a refused 2-row batch on a 2-row table left THREE rows
+ * — which made a caller's retry of the same array unsafe for reasons that had
+ * nothing to do with constraints.
+ *
+ * ⚠️ That is a statement about those TWO doors, not about batches in general.
+ * {@link bulkUpdate} and {@link bulkDelete} are still `Promise.all(map(...))`
+ * and still leave the rows applied before a refusal standing — #13435. Do not
+ * read the paragraph above as covering them.
+ *
  * Everything else is still unenforced. {@link syncSchema} allocates an array,
  * indexes temporal fields and records those unique constraints — and nothing
  * more — so there is no primary key, no `NOT NULL`, no foreign key and no
- * column typing. `bulkCreate` will still happily land two rows with the same
- * `id` (unless `id` itself declares `unique`, or a declared index lists it)
- * where a SQL driver raises a constraint violation, and a read returns both.
+ * column typing. Two rows with the same `id` still land — through `create` and
+ * `bulkCreate` alike, unless `id` itself declares `unique` or a declared index
+ * lists it — where a SQL driver raises a constraint violation, and a read
+ * returns both. ⚠️ Read that as the missing primary key, NOT as a batch that
+ * half-applied: an undeclared duplicate is not a refusal at all, so there is
+ * nothing to refuse and the whole batch lands (pinned in
+ * `memory-bulk-create-atomicity.test.ts`).
  * A declared index WITHOUT `unique` is an access path and buys nothing here:
  * this store is a linear scan.
  *
@@ -611,9 +630,46 @@ export class InMemoryDriver implements IDataDriver {
 
  async bulkCreate(object: string, dataArray: Record<string, any>[], options?: DriverOptions): Promise<Record<string, any>[]> {
  this.logger.debug('BulkCreate operation', { object, count: dataArray.length });
- const results = await Promise.all(dataArray.map(data => this.create(object, data, options)));
- this.logger.debug('BulkCreate completed', { object, count: results.length });
- return results;
+
+ const table = this.getTable(object);
+
+ // [#13340] Build and CHECK every row before pushing ANY of them — the
+ // posture `updateMany` took in #13197, one method over. This used to be
+ // `Promise.all(dataArray.map(data => this.create(...)))`, and `create`
+ // writes into the table synchronously, so every row accepted BEFORE a
+ // refusal stayed in the store: the caller got a rejection describing a
+ // batch that had partly landed, and retrying the same array was unsafe
+ // for reasons that had nothing to do with constraints. Measured on a
+ // 2-row table, a refused 2-row batch left THREE rows behind.
+ //
+ // The pending rows are checked against the table AND against each other,
+ // which a per-row check against `table` alone would miss — nothing is
+ // written yet, so a duplicate WITHIN the batch is invisible to the live
+ // table. Same reason `updateMany` projects its own pending set.
+ //
+ // This is the batch posture of the family this driver stands in for:
+ // `driver-sql` sends the whole batch as one insert, so a constraint
+ // failure there leaves the table untouched. ⭐ `create`'s own single-row
+ // path is unchanged — the record-building expression below is a copy of
+ // it on purpose, so the two doors cannot drift on what a row looks like
+ // (own-key-`undefined` included).
+ const pending: Record<string, any>[] = [];
+ for (const data of dataArray) {
+ const newRecord = this.toStoredRecord(object, {
+ id: data.id || this.generateId(object),
+ ...data,
+ created_at: data.created_at || new Date().toISOString(),
+ updated_at: data.updated_at || new Date().toISOString(),
+ });
+ this.assertUnique(object, newRecord, undefined, [...table, ...pending]);
+ pending.push(newRecord);
+ }
+
+ for (const row of pending) table.push(row);
+
+ if (pending.length > 0) this.markDirty();
+ this.logger.debug('BulkCreate completed', { object, count: pending.length });
+ return pending.map((row) => ({ ...row }));
  }
 
  async updateMany(object: string, query: DriverQuery, data: Record<string, any>, options?: DriverOptions): Promise<number> {

packages/drivers/driver-memory/src/memory-unique-constraint.test.ts

@@ -150,6 +150,13 @@ describe('[#13197] every write path goes through the constraint, not just create
  driver.bulkCreate('doc', [{ id: 'a', doc_no: 'D-0100' }, { id: 'b', doc_no: 'D-0100' }]),
  );
  expectUniqueViolationEnvelope(err, 'doc_no');
+
+ // [#13340] The envelope alone is a VACUOUS control here — the refusal was
+ // already correct before the batch was made atomic, so this assertion
+ // passed while the first row of the batch was still landing. The row
+ // count is what discriminates: 2 before, 2 after, nothing half-applied.
+ expect(await driver.count('doc')).toBe(2);
+ expect(await driver.find('doc', { fields: ['id'], where: { doc_no: 'D-0100' } })).toHaveLength(0);
  });
 
  it('updateMany refuses BEFORE mutating anything — no half-applied batch', async () => {