From 6e0d36ec0f4dd824d5e7d065ac7df4384856eeea Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 08:57:03 +0000 Subject: [PATCH] fix(metadata-protocol): updateMany classifies an id-less row as a caller error, matching batchData (#5100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BotUP49pqhvqGY393n2HfU --- .changeset/updatemany-idless-row-guard.md | 21 +++++++++ .../protocol.bulk-record-not-found.test.ts | 46 +++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 13 +++++- 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 .changeset/updatemany-idless-row-guard.md diff --git a/.changeset/updatemany-idless-row-guard.md b/.changeset/updatemany-idless-row-guard.md new file mode 100644 index 0000000000..d6ebacfb34 --- /dev/null +++ b/.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. diff --git a/packages/metadata-protocol/src/protocol.bulk-record-not-found.test.ts b/packages/metadata-protocol/src/protocol.bulk-record-not-found.test.ts index 9b372889fb..84272ae351 100644 --- a/packages/metadata-protocol/src/protocol.bulk-record-not-found.test.ts +++ b/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(); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 75a6f74a9a..65535778d2 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/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) {