fix(driver-memory): bulkUpdate's touched-row set must agree with its own id resolution (#13917)

* fix(driver-memory): bulkUpdate's touched-row set must agree with its own id resolution

`bulkUpdate` resolved each id with a loose `==` (matching `update`, one
method up) but built its untouched-row set `settled` from the CALLER's ids
with a strict `Set.has`. `IDataDriver.bulkUpdate` declares
`id: string | number`, so a caller may name a stored `1` as `'1'` — and then
the two lookups disagreed: `findIndex` resolved the row and updated it, while
`settled` still carried that row's PRE-image. The row was represented twice
in the projected set handed to `assertUnique` — once with the value it was
vacating, once with the value it was taking — so a batch that merely HANDS a
unique value from one row to another was refused with a false
`UNIQUE_VIOLATION` / 409. `exceptId` does not help, since it only excludes
the row currently being checked, never a sibling row of the same batch.

Resolve every id to its table index first, then derive the touched set from
the RESOLVED rows' own ids. Both lookups now read the same stored value and
cannot drift apart — the property `updateMany` gets for free by drawing its
`targetIds` from table rows. The loose resolution is deliberately preserved:
narrowing it to `===` would silently change which ids resolve at all, well
beyond this defect.

`bulkDelete` needs no change, and this is checked rather than assumed: it has
exactly ONE id comparison, and dedups on the RESOLVED table index rather than
on caller input, so a mixed-type or repeated id collapses to one index by
construction. Keying that set on caller ids instead would splice one index
twice and take a neighbouring row with it — pinned by test.

Regression test uses mixed id types, alongside a positive control with
consistent id types so it cannot pass vacuously.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L

* docs(driver-memory): cite the issue this fix closes, not the PR that introduced the defect

Neighbouring comments in this file cite the ISSUE (`[#13435]`, `[#13197]`,
`[#13340]`); this one cited #13875, which is the PR that introduced the
defect. Comment text only — no behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L

* test(driver-memory): cite the issue the new block pins, matching the rest of the file

Comment and describe-title text only — no assertion changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: zhuangjianguo

.changeset/driver-memory-bulkupdate-id-type-agreement.md

@@ -0,0 +1,29 @@
+---
+"@objectstack/driver-memory": patch
+---
+
+fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)
+
+`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
+resolves an id to a row with a loose comparison — the way `update` and
+`delete` always have — so naming a stored `1` as `'1'` finds the same row.
+The all-or-nothing rework shipped one release earlier then built its
+untouched-row set from the *caller's* ids using strict `Set` membership, so
+for a mixed-type id the two lookups disagreed: the row was resolved and
+updated, yet also stayed in the untouched set carrying its **pre-image**. It
+faced the uniqueness check twice — once with the value it was vacating, once
+with the value it was taking — and a batch that merely HANDS a unique value
+from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.
+
+`bulkUpdate` now resolves every id to its table index first and derives the
+touched set from the *resolved rows' own ids*, so both lookups read the same
+stored value and cannot drift apart — the property the sibling `updateMany`
+gets for free by drawing its target ids from table rows. The loose resolution
+is deliberately preserved: tightening it would silently change which ids
+resolve at all, a far wider behaviour change than this defect.
+
+A genuine collision is still refused, and the stored id keeps its own type —
+naming a row with a differently-typed id does not restamp it. `bulkDelete`
+needed no change: it has exactly one id comparison and dedups on the resolved
+table index rather than on caller input, so a mixed-type or repeated id
+collapses to a single index by construction.

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

@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
     expect(await driver.count('doc')).toBe(3);
   });
 });
+
+/**
+ * [#13911] The batch's two id lookups must AGREE.
+ *
+ * `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
+ * legitimately name a row with an id whose JS type differs from the stored
+ * row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
+ * that `'1'` still finds stored `1`. The first cut of #13435 then built its
+ * untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
+ * so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
+ * got updated) while `settled` still carried that row's PRE-image. The row sat
+ * in the projected check set twice — once stale, once pending — and a batch
+ * that merely MOVES a unique value between rows was refused with a false
+ * `UNIQUE_VIOLATION`.
+ *
+ * The sibling `updateMany` never had this gap: its `targetIds` come from table
+ * ROWS and its `findIndex` is strict `===`, so both sides agree by
+ * construction. The fix restores that property here the other way round —
+ * keeping the loose resolution (narrowing it would silently change which ids
+ * resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
+ *
+ * ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
+ * only asserted "a collision still refuses" would pass against the defect.
+ */
+describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
+  let driver: InMemoryDriver;
+
+  /** Numeric stored ids — the caller may still name them as strings. */
+  beforeEach(async () => {
+    driver = new InMemoryDriver();
+    await driver.syncSchema('doc', DOC_SCHEMA);
+    await driver.create('doc', { id: 1, doc_no: 'D-0001', title: 'One' });
+    await driver.create('doc', { id: 2, doc_no: 'D-0002', title: 'Two' });
+  });
+
+  it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
+    // Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
+    // unusual — it is here so the mixed-type case below cannot pass vacuously.
+    const out = await driver.bulkUpdate('doc', [
+      { id: 1, data: { doc_no: 'D-0900' } },
+      { id: 2, data: { doc_no: 'D-0001' } },
+    ]);
+
+    expect(out).toHaveLength(2);
+    const rows = await snapshot(driver, 'doc');
+    expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
+      [1, 'D-0900'],
+      [2, 'D-0001'],
+    ]);
+  });
+
+  it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
+    // Identical to the control except the first id is a string. It resolves
+    // (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
+    // must therefore NOT collide. Against the defect this threw a false
+    // UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
+    const out = await driver.bulkUpdate('doc', [
+      { id: '1', data: { doc_no: 'D-0900' } },
+      { id: 2, data: { doc_no: 'D-0001' } },
+    ]);
+
+    expect(out).toHaveLength(2);
+    const rows = await snapshot(driver, 'doc');
+    expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
+      [1, 'D-0900'],
+      [2, 'D-0001'],
+    ]);
+  });
+
+  it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
+    await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);
+
+    const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
+    expect(row.id).toBe(1);
+    expect(row.title).toBe('Renamed');
+  });
+
+  it('a REAL collision is still refused when the id types are mixed', async () => {
+    // The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
+    // it is a genuine violation however the caller spelled row 1's id.
+    const before = await snapshot(driver, 'doc');
+
+    const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));
+
+    expect(err.code).toBe('UNIQUE_VIOLATION');
+    expect(err.status).toBe(409);
+    expect(await snapshot(driver, 'doc')).toEqual(before);
+  });
+
+  it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
+    await driver.bulkDelete('doc', ['1']);
+
+    const rows = await snapshot(driver, 'doc');
+    expect(rows.map((r: any) => r.id)).toEqual([2]);
+  });
+
+  it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
+    // `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
+    // Keying the set on caller input instead would make '1' and 1 two entries
+    // and splice index 0 twice — taking row 2 with it.
+    await driver.bulkDelete('doc', ['1', 1]);
+
+    const rows = await snapshot(driver, 'doc');
+    expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
+  });
+});

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

@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
     this.logger.debug('BulkUpdate operation', { object, count: updates.length });
 
     const table = this.getTable(object);
-    const touchedIds = new Set(updates.map((u) => u.id));
+
+    // [#13911] Resolve every id to its table row FIRST, then draw the touched
+    // set from the RESOLVED rows' OWN ids — never from caller input. Ids are
+    // resolved with a loose `==` (matching `update`, one method up), but a
+    // `Set` membership test is always strict, so a caller naming a stored `1`
+    // as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
+    // `string | number` — used to satisfy the resolving lookup while failing
+    // the `settled` one. That row was then updated AND left in `settled`
+    // carrying its PRE-image, so it faced the uniqueness check twice and a
+    // batch merely HANDING a unique value from one row to another was refused
+    // with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
+    // value, so they cannot disagree — the property `updateMany` gets for free
+    // by drawing its `targetIds` from table rows.
+    const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
+    const touchedIds = new Set(
+      resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
+    );
     const settled = table.filter((r) => !touchedIds.has(r.id));
 
     const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
     const pending: Record<string, any>[] = [];
 
-    for (const u of updates) {
-      const index = table.findIndex((r) => r.id == u.id);
+    // Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
+    // resolving a second time here is what let the two lookups drift apart.
+    for (let position = 0; position < updates.length; position++) {
+      const u = updates[position];
+      const index = resolvedIndexes[position];
       if (index === -1) {
         if (this.config.strictMode) {
           this.logger.warn('Record not found for bulk update', { object, id: u.id });