Skip to content

fix(metadata-protocol): batchData upsert fork asks existence, not caller visibility (#5099) - #5187

Merged
os-zhuang merged 1 commit into
mainfrom
claude/updatemany-hooks-nonexistent-id-p6pxb8
Aug 4, 2026
Merged

fix(metadata-protocol): batchData upsert fork asks existence, not caller visibility (#5099)#5187
os-zhuang merged 1 commit into
mainfrom
claude/updatemany-hooks-nonexistent-id-p6pxb8

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #5099

维护者已拍板方案 1(存在性探针),裁定与实现约束记录在 #5099 的裁定评论

问题

runBatchDataLoop 的 upsert 分岔用调用者上下文findOne 判「这条记录在不在」——那是 RLS/sharing 会收窄的读(#3455)。于是一个确实存在、但在调用者读作用域之外的 id 走了 insert 分支:

同文件 probeRecord 的注释(#4435)把「存在性 ≠ 可见性」写成承重点,#5088 刚把它落到 update / delete 写面——upsert 是全文件唯一反着读的分岔(#4620:一份口径,不是三份碰巧一致)。

修法

  • 分岔改用与单记录路径、update/delete 写面同一只 probeRecord(系统上下文,只问存在性)。授权不前移:作用域外既有行走 update 分支后,由 engine.updatefix(security)[P0]: enforce RLS on by-id writes — close member-edits-others'-records hole (#1985) #1994 的前像检查裁决——行的失败答案是写策略自己的答案(掩蔽式部署下即与直接 by-id update 相同的 404),不是 duplicate-key。rls-by-id-write 证明的可红性不受影响(探针不做授权判断)。
  • 非 atomic 的盲插 fallback 一并移除(两臂统一)。存在性既已在写前判定,"update 失败 → 盲插"只剩一种作用:把真实的 update 失败埋进"插入一行刚被证明存在的记录"必然产生的 duplicate-key 里——这正是 ADR-0119 D4 在 atomic 臂禁 fallback 用的同一条掩蔽论证。update 失败的行现在如实报告该失败。runBatchDataLoop 的 doc comment 同步改写(atomic 现在只改变一件事)。
  • 存在性预言机没有变宽:旧行为的 duplicate-key 同样向调用者暴露"该 id 存在于你作用域之外"。

刻意不动:missing id 仍插入(#5088 的既有 pin upsert is deliberately UNTOUCHED: a missing id still inserts 原样通过);写仍带调用者上下文(只有探针是系统读,#3455 回归有 pin);atomic 响应形状不变。

#5038 的边界

#5038(批量写按行语义,在飞)的文件面是 packages/objectql + service-automation,改的是 predicatemulti: true)写的 hook/flow 触发粒度;本 PR 只动 packages/metadata-protocol/src/protocol.tsby-id upsert 分岔,两条路径的边界即 #4800 分诊所划。无共享文件。

先证红再信绿

新增 protocol.upsert-existence.test.ts(7 例:作用域外既有 id、探针系统上下文、真实 update 失败不被掩蔽、missing id 仍插入、可见 id 正常更新、atomic causal 行、混合批次)。fake engine 带调用者作用域读 + #1994 式 by-id 写策略(作用域外掩蔽为 404)+ 主键唯一约束。

只保留测试、不改 protocol.ts 时:

Tests  5 failed | 2 passed (7)
AssertionError: expected "vi.fn()" to not be called at all, but actually been called 2 times   // 双重插入
AssertionError: expected false to be true            // 探针带的是调用者上下文
AssertionError: expected "vi.fn()" to not be called at all, but actually been called 1 times   // fallback 掩蔽 update 失败
AssertionError: expected [ 'ROLLED_BACK', 'INTERNAL_ERROR' ] to deeply equal [ 'ROLLED_BACK', 'RECORD_NOT_FOUND' ]
AssertionError: expected 'INTERNAL_ERROR' to be 'RECORD_NOT_FOUND'

修复后:@objectstack/metadata-protocol 39 文件 / 346 用例全绿(含 batch-atomic 对 atomic 臂"真实错误存活、无 fallback"的既有 pin),@objectstack/rest 608 用例全绿turbo typecheck --filter=...@objectstack/metadata-protocol 81 任务全过

备注

🤖 Generated with Claude Code

https://claude.ai/code/session_01BotUP49pqhvqGY393n2HfU


Generated by Claude Code

…ler visibility (#5099)

The update-or-insert fork read findOne under the caller's context — the
read RLS/sharing narrows — so an existing row outside the caller's scope
took the insert arm: duplicate-key on a constrained store (an
authorization/update scenario reported as a key collision), a second row
on an unconstrained one. The fork now uses the same system-context
existence probe as every other by-id face (#4620), and write
authorization stays #1994's decision inside engine.update.

The non-atomic blind-insert fallback is removed with it: with existence
decided before the fork it could only mask a real update failure behind
a duplicate-key error — the same masking ADR-0119 D4 forbade on the
atomic arm.

Proven red-first: 5 of 7 new cases fail on the old fork (insert called
twice for an out-of-scope existing id; update failures masked). After:
metadata-protocol 39 files / 346 tests, rest 608 tests, typecheck across
81 dependent tasks all green.

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

vercel Bot commented Aug 4, 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 4, 2026 8:15am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

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.

@os-zhuang
os-zhuang marked this pull request as ready for review August 4, 2026 08:28
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 4, 2026
Merged via the queue into main with commit e96ad55 Aug 4, 2026
24 checks passed
@os-zhuang
os-zhuang deleted the claude/updatemany-hooks-nonexistent-id-p6pxb8 branch August 4, 2026 08:40
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

2 participants