fix(metadata-protocol): a single-record update binds the PATH row, not the body's id (#6479) (#6709)

`updateData` probed existence and validated OCC against the path `:id`, built
`{ where: { id: request.id } }`, and then handed the request body to the engine
verbatim — where a truthy scalar `data.id` outranks `where.id`. A body of
`{"id":"rec_2"}` on `PATCH /data/task/rec_1` therefore probed rec_1,
version-checked rec_1, WROTE rec_2, and answered `id: rec_1` beside rec_2's
readback: a silent cross-row write straight past the caller's own `If-Match`.

The path id is now merged over the payload before dispatch
(`{ ...request.data, id: request.id }`) — the same shape the bulk ingress in
`rest-server.ts` has always used, so the repo's two single-write ingresses give
one answer (#4550 / #4434). Triage ruling A of 2026-08-08; routes B (400 on
mismatch) and C (schema ban) were explicitly rejected, and neither the engine's
payload-first dispatch (#5748) nor its by-id payload strip (#6435) is touched.

A non-record payload (`undefined`, `null`, an array) passes through untouched so
this ingress is never kinder than the producer about a malformed call.


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

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

Author: os-zhuang

.changeset/patch-path-id-wins-over-body-id.md

@@ -0,0 +1,48 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(metadata-protocol): a single-record update binds the row the CALLER named, not the row the body names (#6479)
+
+`PATCH /data/:object/:id` decided which row to write **twice, differently**. The
+protocol's `updateData` probed existence and validated `If-Match` /
+`expectedVersion` against the path `:id`, built `{ where: { id: request.id } }`,
+and then handed the request body to the engine verbatim — where the dispatch
+reads the payload first, so a truthy scalar `data.id` outranks `where.id`.
+
+So `PATCH /data/task/rec_1` with a body of `{"id":"rec_2","title":"x"}`:
+
+- probed **rec_1** for existence (404 gate, #4435);
+- version-checked **rec_1** against the caller's `If-Match`;
+- **wrote rec_2**; and
+- answered `{ id: "rec_1", record: <rec_2's readback> }` — a receipt whose two
+  halves name different rows.
+
+rec_2 was never probed and never version-checked, so the most common client
+shape there is — GET a record, edit a field, PUT the whole body back — performed
+a **silent cross-row write straight past its own optimistic-concurrency check**
+whenever the body carried another row's id (a mis-clicked list row, a stale
+refresh, a generated client that copied the wrong field).
+
+`updateData` now merges the path id over the payload before dispatch
+(`{ ...request.data, id: request.id }`) — the same shape the **bulk** ingress has
+always used for this question (`ql.update(op.object, { ...data, id }, …)`), so the
+two ingresses give one answer instead of two. The probed row, the OCC-checked
+row, the written row and the receipt's `id`/`record` are now the same row: the
+one in the URL.
+
+Nothing else moves:
+
+- **The engine is untouched.** ObjectQL's payload-first dispatch (#5748) and its
+  by-id payload strip (#6435) are unchanged and still correct for a caller who
+  hands ObjectQL a payload and nothing else; this was a gap at the REST/protocol
+  ingress, which had already named the row.
+- **No new rejection, no request-shape change.** A body `id` equal to the path
+  id behaves exactly as before, and a differing one is now simply overridden
+  rather than refused — `UpdateDataRequestSchema` still accepts the same bodies.
+- **Non-record payloads pass through untouched** (`undefined`, `null`, an array),
+  so the engine's own diagnostics for a malformed call still surface unchanged.
+
+Callers that deliberately relied on the body's `id` redirecting a
+single-record PATCH must address the intended row in the URL instead — the bulk
+endpoint has never honoured a body id either.

packages/metadata-protocol/src/protocol.ts

@@ -5883,7 +5883,55 @@ export class ObjectStackProtocolImplementation implements
         // listener never breaks the write (the engine catches + logs).
         const dropped: DroppedFieldsEvent[] = [];
         opts.onFieldsDropped = (e: DroppedFieldsEvent) => { dropped.push(e); };
-        const result = await this.engine.update(request.object, request.data, opts);
+        // [#6479] At THIS ingress the row is the one the caller named — `request.id`,
+        // the path `:id` — and nothing in the payload gets to move it.
+        //
+        // The engine's dispatch reads the PAYLOAD first: a truthy scalar `data.id`
+        // outranks `options.where.id` (`engine-update-dispatch.ts`, case *"a SCALAR
+        // data.id still wins over a scalar where.id"* — `expectId: 'rec_1'`). That
+        // rule is correct and deliberate for a caller who hands ObjectQL a payload
+        // and nothing else (#5748 / PR #5919, ruling A); it is a HOLE here, because
+        // this caller has already named the row twice — in the URL and in `where` —
+        // and the three gates around this line all judge THAT row:
+        //
+        //   probe   → `probeRecord(object, request.id)`      (existence, #4435)
+        //   OCC     → `assertVersionOf(…, request.id, …)`    (If-Match / expectedVersion)
+        //   receipt → `{ id: request.id, record: result }`
+        //
+        // Passing `request.data` verbatim let a body `{"id":"rec_2"}` on
+        // `PATCH /data/task/rec_1` bind rec_2: probed rec_1, OCC-checked rec_1,
+        // WROTE rec_2, and answered `id: rec_1` beside rec_2's readback. rec_2 was
+        // never probed and never version-checked, so a client that GETs a record,
+        // edits it and PUTs the whole body back — with the wrong row's id picked up
+        // from a mis-clicked list or a stale refresh — performed a silent cross-row
+        // write past its own `If-Match`.
+        //
+        // The fix is the shape the BULK ingress has always used for the same
+        // question (`rest-server.ts`, batch `update`: `ql.update(op.object,
+        // { ...data, id }, …)` — the operation's id after the spread, so it wins).
+        // Two ingresses, one answer (#4550 / #4434). It changes no engine verdict:
+        // the call still dispatches `by-id`, on the id `where` already carried.
+        //
+        // Deliberately NOT route B (400 on mismatch) or route C (ban `id` in
+        // `UpdateDataRequestSchema`) — both were rejected by the 2026-08-08 triage
+        // ruling on #6479; B installs a new rejection on a shipped API and C
+        // changes the accepted request shape.
+        //
+        // A non-record payload is passed through UNTOUCHED (`undefined`, `null`, an
+        // array): the engine reads `data.id` unguarded on purpose, so `undefined`
+        // is its `TypeError`, and an ingress that answered a non-record payload
+        // more kindly than the producer would be the very looseness
+        // `engine-update-dispatch.ts` exists to prevent. Those shapes carry no
+        // scalar `id` to outrank `where.id` either, so the invariant holds for them
+        // through `opts.where` alone.
+        const writeData = (
+            request.data !== null
+            && typeof request.data === 'object'
+            && !Array.isArray(request.data)
+        )
+            ? { ...(request.data as Record<string, unknown>), id: request.id }
+            : request.data;
+        const result = await this.engine.update(request.object, writeData, opts);
         return {
             object: request.object,
             id: request.id,