Skip to content

fix(objectql,metadata-core): refuse a by-id update whose scalar where.id names a different row than the payload id - #11229

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-11142-where-id-conflict-refusal
Aug 23, 2026
Merged

fix(objectql,metadata-core): refuse a by-id update whose scalar where.id names a different row than the payload id#11229
os-zhuang merged 3 commits into
mainfrom
claude/issue-11142-where-id-conflict-refusal

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11142

Implements the maintainer ruling recorded on the card (2026-08-23, verbatim 「其他同意你的意见」, adopting option A): a by-id update whose truthy scalar options.where.id names a different row than the truthy scalar payload data.id is refused loudly — UPDATE_ID_MISMATCH, status 400 — instead of silently binding the payload row and discarding the where.id predicate. This deliberately reverses the #5748-pinned verdict a SCALAR data.id still wins over a scalar where.id for the unequal shape only; the equal-ids spelling (the REST ingress folds the path id into the payload) stays honoured and now carries its own passing pin.

Where the refusal actually lives (the seat's open question)

The refusal is raised in the dispatch resolution layer itselfresolveEngineUpdateDispatch in packages/metadata-core/src/engine-update-dispatch.ts returns a reject verdict carrying message + declared code/status. There is no second refusal-raising file on the by-id path: the production throw is packages/objectql/src/engine.ts's existing dispatch-reject throw site, now routed through a new shared thrower (engineUpdateDispatchRejectError, also used by assertEngineUpdateDispatch) so the real engine and every pinned fake throw the identical decorated error. This confirms the dispatch-layer assumption: it is the same place the #11009 refusal fires, not a per-driver check.

Landed surface vs the declared prediction — wider by a few mechanical files, so the seat can size the hold: packages/metadata-core/src/engine-update-dispatch.ts and packages/objectql/src/engine.ts as predicted, plus packages/objectql/src/engine-update-dispatch.ts (re-export shim), packages/objectql/src/engine-update-dispatch.test.ts, packages/objectql/src/engine-update-by-id-payload-id.test.ts (a second #5748-era pin of the same shape lived there and flips with the case table), packages/spec/src/api/error-code-ledger.zod.ts (ADR-0112 code registration) + its two regenerated reference-doc pages, and the changeset.

The change, per call shape

Pin-reversal discipline: the interrupted pin flips, it is not deleted — the case-table row keeps its position with expect: 'reject' and a comment citing the ruling, and the #6435 contrast pin a truthy scalar data.id that DISAGREES with where.id still wins becomes a refusal pin asserting code+status and that nothing reaches the driver.

UPDATE_ID_MISMATCH is registered in the ADR-0112 ledger under @objectstack/objectql (the production thrower; the stamp itself lives in metadata-core's shared thrower — noted in the ledger comment). Not a VALIDATION_ERROR synonym (the admission gate agrees): the payload parses fine — two row addresses contradict each other, the same mismatch class as QUERY_OBJECT_MISMATCH. The #5748 / #11009 refusals deliberately stay plain undecorated Errors.

Before/after on the flipped pin (measured, not asserted)

Probe: resolveEngineUpdateDispatch({ id: 'rec_1', title: 'x' }, { where: { id: 'rec_2' } }) resolved through the built@objectstack/metadata-core dist from a dependent package.

  • Before (dispatch module restored to origin/main, package rebuilt; mutation proved on disk both ways — anchored greps on the source UPDATE_ID_MISMATCH=0, 11142=0, then ablation-dist-preflight metadata-core 'UPDATE_ID_MISMATCH' --absent: marker absent from all 18 built files): verdict {"kind":"by-id","id":"rec_1"} — the silent drop. The flipped-pin suites go red against that build: Test Files 2 failed (2), failing exactly at the A scalar data.id beside a DIFFERENT scalar where.id silently drops the where.id predicate — the one unhonoured-predicate shape #11009's refusal deliberately left standing #11142 surface (TypeError: engineUpdateIdConflictMessage is not a function at the flipped tests).
  • After (module restored to HEAD — git status clean against HEAD is the byte-identity proof — package rebuilt, preflight: marker present in all 4 built files): verdict {"kind":"reject","code":"UPDATE_ID_MISMATCH","status":400}; equal pair stays {"kind":"by-id","id":"rec_1"}; both suites green (65/65). The mutation script carried a trap ... EXIT INT TERM restore.

Non-REST ingress survey (the ruling's recorded confidence gap)

Method: a repo-wide scan of non-test TypeScript under packages/ for .update( call windows that pass options.where (34 windows across 1898 files), each classified by reading the call site; plus a sweep of shipped flow configs in examples/ and skills/. Controls proving the queries were live: the call-site scan re-found every already-known site (protocol.ts:10335/10367, stdio-data-bridge.ts:395, action-execution.ts:216/1078, crud-nodes.ts:424, engine.tsupdateById); the flow-config sweep's control string update_record hit 9 files before the fields-key filter was applied.

Claim established (stated at its real strength): no non-REST ingress in this repository relies on payload-priority-under-conflict. Every ingress that supplies both id sources is in one of two groups:

  1. Fold-equal by construction (both ids are the same variable): protocol.updateData ({ ...data, id: request.id } beside where: { id: request.id } — every REST PATCH), REST batch update ({ ...data, id }, no where at all), objectql scoped-repo updateById, service-storage metadata-store (file + upload-session), service-automation suspended-run-store (save + recordTerminal), the sys_notification migration. Unaffected — the equal carve-out exists for exactly this pattern.
  2. Verbatim payload beside where: { id } (an unequal pair can only come from the caller writing contradictory ids): MCP stdio bridge (stdio-data-bridge.ts:395), runtime callData ObjectQL fallback (action-execution.ts:216), the action engine facade (action-execution.ts:1078), protocol bulkData update and upsert arms (protocol.ts:10335/10367), and the flow update_record executor (author-written fields beside filter). Today an unequal pair in this group silently writes the payload row — in the stdio bridge, callData fallback and bulkData arms that write lands after an existence probe that judged the where row, i.e. the same cross-row shape the REST PATCH /data/:object/:id:请求体里的标量 id 压过路径 :id,存在性探测/OCC 判在一行、写落在另一行、响应报第三个说法 #6479 fix removed from updateData. After this change those calls refuse loudly. That is the ruling's premise confirmed by measurement, not contradicted — no fork-back needed.
  3. Predicate-shaped where.id sites (outbox { id: { $in: ids } } sweeps) are outside the refusal's condition (non-scalar), and no shipped flow/example/skill writes id into update_record's fields (all use filter: { id: ... } only).

Boundary of the claim: measured over this repository's packages/, examples/, skills/. Sibling repos (objectui reaches data through REST, which folds; cloud) and third-party SDK consumers were not measured — "none exists anywhere" is not claimed.

Verification

Union run at the final head 60bcad871 (merge of origin/main @ ee2ff455a, which gained no commits touching this surface; premise re-verified against that main — pin string present, unequal shape still silently honoured):

  • @objectstack/metadata-core tests: Test Files 11 passed (11) / Tests 165 passed (165); typecheck Done (its program includes tests).
  • @objectstack/objectql full suite: Test Files 229 passed (229) / Tests 4060 passed (4060); typecheck Done. Flipped-pin files verbose: 65/65 with every A scalar data.id beside a DIFFERENT scalar where.id silently drops the where.id predicate — the one unhonoured-predicate shape #11009's refusal deliberately left standing #11142 test listed.
  • Downstream fake-pinned consumers: @objectstack/metadata-protocol137 passed | 2 skipped / 1879 passed | 10 skipped; @objectstack/mcp20 passed / 207 passed; @objectstack/service-messaging26 passed / 259 passed (after building objectql+driver-sql dists — the first run's 7 reds were the fresh-worktree unbuilt-dist trap, not the diff). Direction: these are downstream consumers of the tightened contract (--filter on the named packages; the '...@pkg' prefix-filter full sweep is CI's).
  • Spec: check:generated green after --fix regenerated the one proved-stale artifact (check:docs → the two error-code reference pages); ledger admission suite 2 files / 70 tests green (synonym gate accepts the code).
  • Derived gate union (node scripts/pm/dispatch-gates.mjs, change set derived by the script itself — 9 paths): all matched families run at 60bcad871, every one exit 0, including check:engine-double-contract (366 rows held), check:error-code-casing, check:dispatcher-error-vocabulary (OK — 21 unregistered code-stamping site(s), all classified; the new code is registered so it is not reported), check:nul-bytes (OK (scanned 6494 text file(s) ...)), check:where-matcher, check:query-options-erasure, check:cross-package-test-inputs, check:type-check-coverage, check:durability-log-level, check:slot-lookup (ratchet holds), check:adr-0087-registration ([BREAKING] not-required (no-migration-prescription) accepted), check:changeset-no-major (minor only, launch-window convention), and the doc/content families the regenerated pages triggered.
  • Declared narrowings (each with its evidence, CI runs the full form):
    • check:type-check-debt --re-measure (workspace-wide) was reproduced narrowly for the one ledger entry my diff can move: the gate's own remeasureProject construction was replicated for @objectstack/objectql (extends its tsconfig, test-exclusion removed, default typeRoots); tsc's own output counts 354 non-TS6059 errors == the recorded ledger value 354, and the only 2 diagnostics naming my edited files are on pre-existing lines my diff does not touch. Invariance for untouched packages: the generated program is per-package and my diff adds no test files elsewhere.
    • check-dev-prereqs reds locally with The workspace is not built — 52 of 67 packages — a local build-completeness precondition of this worktree, not a diff finding; the spec-dist freshness half it guards is built and current here.
    • Full-farm suites of the remaining ~60 downstream packages: CI's, per the standing local-verification scope.

Changeset: minor (breaking declared in body — launch-window convention), ADR-0087 disposition marker included; skip-changeset not applicable (real changeset present).

Wire status (measured, answering the PM's question)

The declared status: 400 is what the wire actually produces, measured in-process on the real RestServer route handlers over the real protocol over the real engine (scratch test under the verify lock, deleted after):

  • PATCH /api/v1/data/task/rec_1 with body { id: 'rec_2' }200 on the path row — the single-record door cannot produce the conflict at all (the REST PATCH /data/:object/:id:请求体里的标量 id 压过路径 :id,存在性探测/OCC 判在一行、写落在另一行、响应报第三个说法 #6479 fold rewrites the body id to the path id before the engine sees it).
  • The doors that can carry the conflict over REST — POST /data/:object/updateMany and batchData's update/upsert arms — answer the row with success: false, errors[0].code = 'UPDATE_ID_MISMATCH', errors[0].httpStatus = 400 (toRowApiError parses the code against the ADR-0112 union; the ledger registration in this PR is what makes it survive).
  • mapDataError over the real engine throw (the classification every single-record data-route catch shares — pinned by the [rest] Hook refusals carrying an explicit statusCode are not mapped by /api/v1/data — they leak as HTTP 500 INTERNAL_ERROR with no located guidance #7525 suite): status: 400, body carries the refusal message + code: 'UPDATE_ID_MISMATCH' + object. Mechanism: classifyDataError's generic declared-status passthrough (declaredHttpStatus reads status/statusCode, 400–599; thrownCodeFields narrows the code against the registered union) — the recordNotFoundError pattern. content/docs/api/error-handling-server.mdx:11's "small fixed set" sentence predates that passthrough and understates it; doc untouched here per scope.
  • Dispatcher door (callData/actions): read, not measured — errorFromThrown reads .status/.statusCode, so 400 + code there too.

Generated by Claude Code

….id names a different row than the payload id (#11142)
update(obj, { id: 'rec_1', ... }, { where: { id: 'rec_2' } }) used to bind
rec_1 and silently discard the where.id predicate — the one
unhonoured-predicate shape #11009's refusal deliberately left standing,
because refusing it reverses the #5748-pinned verdict 'a SCALAR data.id
still wins over a scalar where.id'. The maintainer ruling on #11142
(2026-08-23, option A) authorizes that reversal for the UNEQUAL truthy
scalar shape only.
resolveEngineUpdateDispatch now rejects the conflict with a message naming
both ids, decorated code UPDATE_ID_MISMATCH + status 400 (the
recordNotFoundError convention; code registered in the ADR-0112 ledger
under @objectstack/objectql). Both throwers — assertEngineUpdateDispatch
(every pinned fake) and ObjectQL.update — go through one shared
engineUpdateDispatchRejectError, so fakes and the real engine refuse
identically. The #5748/#11009 refusals stay plain Errors, byte-identical.
Pin-reversal discipline: the interrupted pin flips to a refusal pin in
ENGINE_UPDATE_DISPATCH_CASES and in the #6435 contrast pins — never
deleted; the equal-ids spelling (REST folds the path id into the payload)
gains its own passing pin; falsy and non-scalar where.id boundaries keep
their pre-existing verdicts, pinned so the refusal cannot creep past the
ruled scope.
Fixes#11142
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
…ngeset
The UPDATE_ID_MISMATCH ledger entry lands in the generated
content/docs/references pages (check:generated --fix, only the artifact it
proved stale), and the breaking-changeset carries the ADR-0087 disposition
marker (not-required: no authorable surface moves; the refused shape is a
self-contradictory input whose fix is a per-site intent decision).
Part of #11142
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/metadata-core, @objectstack/objectql, @objectstack/spec, touching 12 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx(via ERROR_CODE_LEDGER (symbol))
  • content/docs/api/error-handling-server.mdx(via ERROR_CODE_LEDGER (symbol))
  • content/docs/kernel/contracts/data-engine.mdx(via ERROR_CODE_LEDGER (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via ERROR_CODE_LEDGER (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/objectql/src/engine-update-dispatch.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 127 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json dd84ddd796d32a1deb9b8b71165c9bce5ddac30apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 6a2a8151ae1a530a903ec29a743a8be924c89f07 — the merge of head 60bcad871a5496826d290dfb38281d62f01e9c19 into base dd84ddd796d32a1deb9b8b71165c9bce5ddac30a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6a2a8151ae1a530a903ec29a743a8be924c89f07 && git checkout 6a2a8151ae1a530a903ec29a743a8be924c89f07
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dd84ddd796d32a1deb9b8b71165c9bce5ddac30a 60bcad871a5496826d290dfb38281d62f01e9c19 && git checkout -B drift-repro dd84ddd796d32a1deb9b8b71165c9bce5ddac30a && git merge --no-ff 60bcad871a5496826d290dfb38281d62f01e9c19
node scripts/docs-audit/affected-docs.mjs --json dd84ddd796d32a1deb9b8b71165c9bce5ddac30a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs dd84ddd796d32a1deb9b8b71165c9bce5ddac30a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Answers to the PM's wire-status question (measured, then read — in that order).

1. Is error-handling-server.mdx:11 still accurate? Partially — stale in exactly the direction that matters here. Read from packages/rest/src/error-response.ts on this branch (post-merge of current main): the flat { error, code } shape claim holds, and the structured fixed-set branches exist as described. But since the #5582 / #7525 / #9232 changes, classifyDataError ends in a generic declared-status passthrough the sentence does not mention: declaredHttpStatus reads error.status / error.statusCode over the 400–599 band; a 4xx keeps its status, keeps its message (truncated, never replaced — #5423), and carries its code via thrownCodeFields, which narrows against the ADR-0112 union — a registered code rides error-body code verbatim, an unregistered spelling is demoted to declaredCode. So arbitrary thrown errors are still not auto-translated, but a declared-status error with a registered code does reach the wire with both halves. That is the recordNotFoundError pattern (404), and it is exactly what UPDATE_ID_MISMATCH + status: 400 was built to ride. I did not edit the doc in this PR (per the PM's lean-no on widening).

2. What does the wire actually produce after this change? Measured in-process on the realRestServer route handlers over the realObjectStackProtocolImplementation over the realObjectQL engine (only the storage driver is an in-memory echo; scratch test run under the verify lock, then deleted — not committed):

3. Fix needed in packages/rest?None. Nothing surfaces as anything other than 400 with the code on the body, at every door that can reach the conflict. No widening done, none needed, nothing to route.

Net: the refusal is loud on the wire, not just on the error object — and the single-record PATCH route is additionally immune by construction. Adding this as a section to the PR body next.


Generated by Claude Code


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-zhuang@claude