Skip to content

fix(metadata-core,objectql): update 的 data.id 同过标量测试,载荷里的算子对象不再被当成主键 (#5748) - #5919

Merged
baozhoutao merged 3 commits into
mainfrom
claude/issue-5748-update-data-id-scalar-test
Aug 6, 2026
Merged

fix(metadata-core,objectql): update 的 data.id 同过标量测试,载荷里的算子对象不再被当成主键 (#5748)#5919
baozhoutao merged 3 commits into
mainfrom
claude/issue-5748-update-data-id-scalar-test

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Fixes #5748

前提复核(先证再改)

issue 正文的两条事实在 origin/main全部成立,但落点描述已过期 —— 正文写的是
「必须两个文件一起改」(packages/objectql/src/engine.ts + packages/objectql/src/engine-update-dispatch.ts),
而 PR #5871(同日)已把共享判定沉到 packages/metadata-core/src/engine-update-dispatch.ts,
objectql 的同名文件只剩 re-export shim。实测确认 engine.ts纯消费方,没有残留取 id 逻辑:

packages/objectql/src/engine.ts:5376
    const dispatch = resolveEngineUpdateDispatch(
      data as EngineUpdateDispatchData,
      options as EngineUpdateDispatchInput | undefined,
    );
    const id: any = dispatch.kind === 'by-id' ? dispatch.id : undefined;

所以本 PR 没有动 engine.ts(在飞 #5699 同文件),也没有动 shim —— 判定改一处,
engine.ts 与全部 fake engine 自动跟随,这正是 #5480 抽取判定时要的效果。

改了什么

ObjectQL.update(object, data, options) 问「这次调用是否指名了一行」时看两个地方,
而这两处此前用的是两套规则:

于是同一个算子对象,写在 where.id 里被识别成谓词,写在 data.id 里却被绑进
driver.update(object, id, …)主键位置,而调用方显式写的 multi: true
被无声吞掉 —— declared ≠ enforced,而且就压在 #5393 刚给 flow update_record
补上的 multi 批量意图键下面一层。

现在 data.idwhere.id 共用同一个标量测试(asScalarId,模块内定义一次,
两半都走它)。非标量 data.id 不算 id,因此不再盖住任何东西:判定按
where.idmultireject 的原有阶梯继续往下走。

行为差异逐条(FROM → TO)

标量 data.id 的按 id 写法完全不受影响 —— 这是仓内每一个调用方用的写法(见下节扫描)。

调用 FROM TO
update(o, { id: 'rec_1', …f }) by-id 'rec_1' 不变
update(o, { id: 'rec_1', …f }, { multi: true }) by-id 'rec_1' 不变(标量 data.id 仍先于 multi)
update(o, { id: 'rec_1', …f }, { where: { id: 'rec_2' } }) by-id 'rec_1' 不变(标量 data.id 仍先于 where)
update(o, { id: 0, …f }, { multi: true }) multi 不变(真值判定,0 不标识行)
update(o, { id: { $in: [...] }, …f }, { multi: true }) by-id,算子对象绑进主键位 multi —— 声明的批量意图被执行
update(o, { id: ['a','b'], …f }, { multi: true }) by-id,数组绑进主键位 multi
update(o, { id: { $in: [...] }, …f })( multi) by-id,算子对象绑进主键位 reject,消息不变
update(o, { id: { $in: [...] }, …f }, { multi: false }) 同上 reject
update(o, { id: null, …f }) reject(null 本就是假值) 不变
update(o, { id: { $in: [...] }, …f }, { where: { id: 'rec_1' } }) by-id,绑的是算子对象 by-id,绑的是 'rec_1'

最后一格是唯一「判定不变、绑定值变了」的一格。它也是这次改动里最容易被绿掉的一格:
前后都是 by-id,只看 expect 的用例两边都过。所以 ENGINE_UPDATE_DISPATCH_CASES
新增了可选的 expectId,把落进主键位的值本身钉住;真引擎侧的
observeEngine 也改成回报 recording driver 收到的那个 id,而不只是回报走了哪个分支。

ENGINE_UPDATE_REJECT_MESSAGE(Update requires an ID or options.multi=true)、
导出符号、类型签名均无变化

裁决与必答项

维护者 2026-08-06 10:39Z 裁 A(data.id 同过标量测试,与 where.id 统一),
取代 08:28Z 的裁 B 评论。

B 的顾虑 —— 「把算子对象写进载荷大概率是写错了位置,A 会把一次笔误静默变成一次真的
批量写」—— 不需要第二条错误消息来处置,阶梯本身就处置了:非标量 data.id 落回
阶梯后,和其他任何「没指名一行」的调用一样需要显式声明的 multi。没有 multi
就是现有的那条响亮 reject,且驱动一次都没被碰到。这条按裁决要求单独立为一条测试:

✓ a NON-SCALAR data.id with NO multi is REJECTED, never silently promoted to a bulk write (#5748)

它对 {$in} / {$ne} / 数组 / null × undefined / {} / {multi:false} /
{where:{tenant}} 共 16 组做笛卡尔断言(判定 + assertEngineUpdateDispatch 抛错),
再用真引擎断言 driver 调用列表为 []

调用方扫描

grep -rn -E "\.update\([^)]*\{[^}]*\bid\s*:" packages/ examples/ —— 30 处生产代码 +
测试,每一处的 data.id 都是标量(row.id / record.id / 'rec_1' / userId …):
plugin-auth/objectql-adapter.ts(4 处 { ...patch, id: record.id })、
plugin-security/normalize-managed-by.tsplugin-sharing/primary-bu-projection.ts
plugin-approvalsplugin-reportsplugin-emailmetadata-protocol/seed-loader.ts
engine.ts 自身的 roll-up / referential-integrity 回写等。全部落在上表「不变」行。

可达性来自外部载荷:flow 的 update_record 把作者字段直接铺进 data
(crud-nodes.ts:409 data.update(objectName, fields, { where: filter, multi })),
REST 的 PATCH body 同理 —— AI 生成的元数据把 id 写进字段集合是完全可能的形状(PD #12)。

反向验证(方向先定,再跑)

预测:。把 asScalarId(data.id) 还原成 data.id、重建 metadata-core,
新增/翻面的钉子应当全红 —— 而且不只是判定红,真引擎侧必须一起红,否则说明钉子
钉的是判定自己的复读而不是生产者。实测 11 条红,方向与预测一致:

× real engine agrees with the predicate: operator object in data.id WITH multi:true … → multi
× real engine agrees with the predicate: operator object in data.id, NO multi — rejected … → reject
× real engine agrees with the predicate: operator object in data.id, scalar where.id — the WHERE id wins … → by-id
× real engine agrees with the predicate: array data.id with multi:true → multi
× real engine agrees with the predicate: array data.id, no multi → reject
× real engine agrees with the predicate: operator object in data.id, multi explicitly false → reject
× never binds a non-scalar into the primary-key position, on ANY case (#5748)
× a NON-SCALAR data.id is not an id, so it no longer outranks a declared multi:true (#5748)
× a NON-SCALAR data.id falls through to where.id, which is bound instead of the operator (#5748)
× a NON-SCALAR data.id with NO multi is REJECTED, never silently promoted to a bulk write (#5748)
× the scalar test is ONE rule: the same value verdicts alike in data.id and where.id (#5748)
 Test Files  1 failed (1)      Tests  11 failed | 25 passed (36)

恢复后 36/36 绿。

钉子的处置(翻面,不是删)

  • data.id outranks where and multi, and is NOT scalar-tested (the producer's rule, verbatim)
    → 拆成两条,两条都继续承重:
    a SCALAR data.id still outranks where and multi (…untouched by #5748)(旧语义里仍然为真的那一半,继续钉住)
    a NON-SCALAR data.id is not an id, so it no longer outranks a declared multi:true (#5748)(翻面的那一半)。
  • ENGINE_UPDATE_DISPATCH_CASESdata.id wins over an explicit multi:true 用的是标量 'rec_1',
    新语义下判定不变 —— 所以它不是翻面项,而是「改名 + 加 expectId」:
    a SCALAR data.id still wins over an explicit multi:true,并钉住绑定值就是 'rec_1'
    真正翻面的是新增的非标量各例。
  • 新增全集不变式 never binds a non-scalar into the primary-key position, on ANY case ——
    对 CASES 全表跑真引擎,凡 by-id 就断言落进主键位的值 typeof ∈ {string, number, bigint}。
    这条是把 issue 的伤害面(算子对象被绑成主键)直接钉成性质,而不是逐例列举。

顺带修正的过期表述

scripts/engine-double-contract.baseline.json 的 88 条 update 切片 DEBT 条目里,
每条都以同一句结尾:「…stricter on the one it invents (data.id, which the producer
takes verbatim when truthy, ahead of both where and multi)」。这句描述的正是本 PR
删掉的那条规则,留着就是 88 处对着读者说假话。统一替换成新的表述(一次机械替换,
逐字相同),门禁计数不变

验证

pnpm --filter @objectstack/metadata-core test        → Test Files 8 passed,   Tests 103 passed
pnpm --filter @objectstack/objectql test             → Test Files 127 passed, Tests 2097 passed
pnpm --filter @objectstack/metadata-protocol test    → Test Files 48 passed,  Tests 471 passed
pnpm --filter @objectstack/service-automation test   → Test Files 64 passed,  Tests 762 passed
pnpm --filter @objectstack/rest test                 → Test Files 58 passed,  Tests 824 passed
pnpm --filter @objectstack/metadata-core --filter @objectstack/objectql typecheck  → Done / Done
node scripts/check-engine-double-contract.mjs        → OK — 64 pinned, 139 DEBT, 2 exempt(与 main 一致,无计数变化)
node scripts/check-engine-double-contract.mjs --self-test → OK
node scripts/check-nul-bytes.mjs                     → OK (5709 files)

metadata-protocol 的 13 个 fake engine 接线测试(update() 开头调
assertEngineUpdateDispatch)全绿,无 fixture 命中非标量 data.id 形状,
因此没有需要按新语义修正的期望值。

git merge origin/main(无 rebase)后重跑上述三个包 + 两个门禁,结果同上。

🤖 Generated with Claude Code

https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We


Generated by Claude Code

claude added 2 commits August 6, 2026 11:31
…atch, so a payload operator object is not bound as a primary key (#5748)

`ObjectQL.update(object, data, options)` asked "does this call name one row?"
of two places under two different rules: `options.where.id` went through a
scalar test (an operator object / array / `null` is a multi-row predicate, not
an id -- #4434 / #4550), while `data.id` was taken VERBATIM whenever truthy,
ahead of both `where` and `options.multi`.

So the same operator object was a predicate in `where` and a primary key in
the payload: `update(o, { id: { $in: ['a','b'] }, title: 'x' }, { multi: true })`
dispatched `by-id` with `{$in: [...]}` bound into `driver.update`'s primary-key
position, and the caller's explicit `multi: true` was swallowed with no
diagnostic -- declared != enforced, one layer under the `multi` intent key
#5393 had just added to the flow `update_record` node.

`data.id` now goes through the SAME scalar test as `where.id`, defined once as
`asScalarId` and reached by both halves, so a non-scalar payload id names no
row and stops shadowing the ladder below it: it falls through to `where.id`,
then `multi`, then `reject`. A scalar `data.id` is untouched -- it still
outranks `where` and `multi`, which is the common, legal
`update(o, { id, ...fields })` spelling every in-repo caller uses.

Ruled by the maintainer (2026-08-06) as option A over option B's "reject any
non-scalar `data.id`". B's objection -- that an operator object in the payload
is most likely a typo the author meant for `where`, and that A would promote it
into a real bulk write -- is answered by the ladder rather than by a second
error message: with no declared `multi`, a non-scalar `data.id` is the existing
loud reject and nothing reaches the driver. That is pinned as its own test.

`ENGINE_UPDATE_DISPATCH_CASES` gains an optional `expectId`, because the
verdict alone cannot separate "picked an id" from "picked the RIGHT id": an
operator `data.id` beside a scalar `where.id` is `by-id` before and after, and
only the bound value says which source won. Reverse-verified: restoring the
verbatim `data.id` read turns 11 of these red against the real engine.

Fixes #5748

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

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 6, 2026 12:55pm

Request Review

@github-actions github-actions Bot added the size/l label Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-core.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-core)
  • content/docs/plugins/packages.mdx (via @objectstack/metadata-core)
  • content/docs/releases/v12.mdx (via @objectstack/metadata-core)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…ate-data-id-scalar-test

# Conflicts:
#	scripts/engine-double-contract.baseline.json
@github-actions github-actions Bot added size/m and removed size/l labels Aug 6, 2026
@baozhoutao
baozhoutao added this pull request to the merge queue Aug 6, 2026
Merged via the queue into main with commit 946a131 Aug 6, 2026
24 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-5748-update-data-id-scalar-test branch August 6, 2026 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ObjectQL.update 的 data.id 不做标量测试 —— 载荷里的算子对象被当成主键绑定,且盖过显式 options.multi: true

2 participants