fix(metadata-protocol): updateMany classifies an id-less row as a caller error, matching batchData (#5100) (#5200)

runUpdateManyLoop lacked the !record.id guard #4793 gave batchData's
update branch: the same malformed row answered VALIDATION_FAILED/400 on
one by-id face and RECORD_NOT_FOUND/404 ('Record undefined not found')
on the other, with the unguarded path handing { id: undefined } to the
probe and the write — a reading each driver decides for itself. The
guard fires before any engine round-trip, both faces now give one
classification (#4620), pinned by a cross-face parity test. record.data
handling aligned to the batch branch's || {} in passing.

Dormant over REST (UpdateManyRecordSchema requires id, #3939); the
change is observable only to in-process protocol callers.

Proven red-first: both new cases fail unguarded (RECORD_NOT_FOUND where
VALIDATION_FAILED is asserted). After: metadata-protocol 348 tests,
typecheck across 82 dependent tasks all green.


Claude-Session: https://claude.ai/code/session_01BotUP49pqhvqGY393n2HfU

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

Author: os-zhuang

.changeset/updatemany-idless-row-guard.md

@@ -0,0 +1,21 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(metadata-protocol): `updateMany` classifies an id-less row as a caller error, matching `batchData`'s update branch (#5100)
+
+`runUpdateManyLoop` lacked the `!record.id` guard #4793 gave `runBatchDataLoop`'s
+update branch, so the two by-id update faces classified the same malformed row
+differently: `VALIDATION_FAILED`/400 on batch, but on `updateMany` the row fell
+through to the #5088 existence probe as `{ id: undefined }` and came back
+`RECORD_NOT_FOUND`/404 with `undefined` interpolated into the message — a
+request-shape error reported as a data-state one, with the row's fate left to
+each driver's undefined-where-key handling.
+
+Not reachable over REST (`UpdateManyRecordSchema` requires `id`, #3939) — the
+change is observable only to in-process callers of the protocol method, whose
+id-less rows now answer `VALIDATION_FAILED`/400 (`Record id is required for
+update`) before any engine round-trip, identically on both faces (#4620: one
+classification per file, enforced by a cross-face parity test). `record.data`
+handling is aligned to the batch branch's `record.data || {}` in the same
+change.

packages/metadata-protocol/src/protocol.bulk-record-not-found.test.ts

@@ -421,6 +421,52 @@ describe('[#5088] batchData delete — the driver`s return decides, as in delete
     });
 });
 
+describe('[#5100] an id-less row is a CALLER error on both by-id update faces', () => {
+    it('updateMany: VALIDATION_FAILED/400 before any engine read or write', async () => {
+        const t = makeStoreEngine();
+        const p = new ObjectStackProtocolImplementation(t.engine);
+
+        const res: any = await p.updateManyData({
+            object: 'showcase_task',
+            records: [{ data: { progress: 1 } }],
+            options: { continueOnError: true },
+        } as any);
+
+        expect(res.succeeded).toBe(0);
+        expect(res.failed).toBe(1);
+        expect(res.results[0].errors?.[0]?.code).toBe('VALIDATION_FAILED');
+        expect(res.results[0].errors?.[0]?.httpStatus).toBe(400);
+        expect(res.results[0].errors?.[0]?.message).toBe('Record id is required for update');
+        // A missing id is a request-shape error, not a data-state one — and it
+        // must fail BEFORE any engine round-trip: unguarded, the row reached
+        // the #5088 probe as `{ id: undefined }`, whose reading is up to the
+        // driver's undefined-where-key handling, and came back as a 404 with
+        // `undefined` interpolated into the message.
+        expect(res.results[0].errors?.[0]?.message).not.toContain('not found');
+        expect(t.findOne).not.toHaveBeenCalled();
+        expect(t.update).not.toHaveBeenCalled();
+    });
+
+    it('the two by-id update faces give ONE classification for the same malformed row (#4620)', async () => {
+        const t = makeStoreEngine();
+        const p = new ObjectStackProtocolImplementation(t.engine);
+
+        const many: any = await p.updateManyData({
+            object: 'showcase_task',
+            records: [{ data: { progress: 1 } }],
+        } as any);
+        const batch: any = await p.batchData({
+            object: 'showcase_task',
+            request: { operation: 'update', records: [{ data: { progress: 1 } }] },
+        } as any);
+
+        expect(many.results[0].errors[0].code).toBe(batch.results[0].errors[0].code);
+        expect(many.results[0].errors[0].message).toBe(batch.results[0].errors[0].message);
+        expect(many.results[0].errors[0].httpStatus).toBe(batch.results[0].errors[0].httpStatus);
+        expect(batch.results[0].errors[0].code).toBe('VALIDATION_FAILED');
+    });
+});
+
 describe('[#5088] the three by-id write faces answer the SAME thing', () => {
     it('single-record PATCH, updateMany and batchData produce one message for one missing id', async () => {
         const t = makeStoreEngine();

packages/metadata-protocol/src/protocol.ts

@@ -5935,6 +5935,17 @@ export class ObjectStackProtocolImplementation implements
                 //     (#2948) / `readonlyWhen` (#3042) strips that single-write now
                 //     surfaces (#3431) happened silently here. Collect per row.
                 //
+                // [#5100] Same guard as `runBatchDataLoop`'s update branch
+                // (#4793), same classification: an id-less row is a CALLER
+                // error — VALIDATION_FAILED/400 — not a data-state one. The
+                // REST entrance already rejects it (`UpdateManyRecordSchema`
+                // requires `id`, #3939), but that invariant lives two packages
+                // away; unguarded, an in-process caller's malformed row
+                // reached the probe and the write as `{ id: undefined }`,
+                // whose reading is up to each driver's undefined-where-key
+                // handling — at best a 404 with `undefined` interpolated into
+                // the message, at worst a where-clause with no id at all.
+                if (!record.id) throw rowRequiredIdError('update');
                 // [#5088] Third gap, the same shape: no existence gate. A row
                 // naming no record went straight into `engine.update`, so the
                 // hook pipeline ran over a payload-only record and the row came
@@ -5945,7 +5956,7 @@ export class ObjectStackProtocolImplementation implements
                 const dropped: DroppedFieldsEvent[] = [];
                 const opts: any = { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
                 if (context !== undefined) opts.context = context;
-                const updated = await this.engine.update(object, record.data, opts);
+                const updated = await this.engine.update(object, record.data || {}, opts);
                 results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
                 succeeded++;
             } catch (err: any) {