Skip to content

fix(metadata-protocol): an update by id answers the same refusal whether or not the id exists, as delete does - #28615

Merged
objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/harden-update-refusal-order
Nov 1, 2026
Merged

objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/harden-update-refusal-order

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Clause-②: no
Metadata-protocol: none
Skills-docs: none — no file under skills/** teaches what a by-id update answers for an id that names no record (a git grep of skills/ for RECORD_NOT_FOUND names only the approvals record lock), so no skill line changes. The Update Record section of content/docs/protocol/kernel/http-protocol.mdx states the order.

维护者速读

  • 改了什么:按 id 更新记录时,先判断"这次更新允不允许",再查"记录在不在"——与按 id 删除的顺序一致。覆盖单条 PATCH、批量 updateMany、batch(update)三个入口,以及经它们更新的所有入口(dispatcher /data、/mcp HTTP 工具、声明式更新动作)。
  • 为什么改:QA 第三轮发现的安全加固(细节按披露裁决不公开)
  • 风险与回滚:不允许更新该对象的调用方(无编辑权限,或外部数据源只读)对不存在的 id 现在收到与存在的 id 相同的 403,原来是 404;有权限的调用方、成功更新、405、乐观锁(If-Match)、upsert 与系统上下文写入均不变。回滚 = revert 本 PR。
  • 先看:① packages/metadata-protocol/src/protocol.ts 的 updateData——删掉写前的存在性探测,改为引擎返回 null 后再查存在;② packages/metadata-protocol/src/protocol-batch-data.ts 的两处 update 分支(同一改法);③ packages/qa/dogfood/test/update-by-id-refusal-order.dogfood.test.ts。
  • 两分钟验证:pnpm --filter @objectstack/dogfood exec vitest run test/update-by-id-refusal-order.dogfood.test.ts —— 预期 16/16 通过(四类对象上 PATCH、批量更新、DELETE 对"存在/不存在"的 id 给出同一个回答,另有编辑权限、行级范围、管理员三组对照)。
  • 需要你做的:无,席位落地。

What changes

An update by id now answers the same refusal whether or not the id exists, as delete does.

  • updateData (PATCH /api/v1/data/:object/:id, and every door that updates through it: the runtime dispatcher's /data, the /mcp HTTP tools, callData, declarative update actions) no longer reads the record before calling engine.update. The engine judges the write first — the security middleware's object- and row-level verdicts, the external datasource's write guard — and then its own by-id not-found gate (inside the middleware chain, ahead of every before* hook) answers 404 RECORD_NOT_FOUND to a caller the write is open to. The protocol asks existence (the system-context probe) only when the engine resolved a null readback: that keeps the 404 for an engine that resolves null for a missing row, and keeps 200 { record: null } for an update that moved its row out of the caller's row scope.
  • Optimistic concurrency on updateData is asked the way deleteData asks it: assertVersionMatch, a probe only when the caller sent a token.
  • updateManyData and batchData's update operation: the same — no probe ahead of the engine, the existence check after a null readback, per row.
  • Docs: the Update Record section of content/docs/protocol/kernel/http-protocol.mdx states the order.

Unchanged, and already one answer for both ids for a caller refused the operation: DELETE on every door and the cross-object POST /api/v1/batch (the engine judges first), and clone and the record field-access read, which load the record through the read door first (each measured at the HTTP door on the stock showcase). The action-by-id record load, the MCP stdio bridge's update_record / delete_record and callData's no-protocol fallback likewise read the record as the caller, through the read door, before any write, on their update and delete arms alike (read in code).

Tests

  • HTTP door (dogfood, stock showcase) — packages/qa/dogfood/test/update-by-id-refusal-order.dogfood.test.ts, 16 cases. A plain member (baseline only) on showcase_project_membership, sys_user_permission_set, showcase_ext_customer and showcase_ext_order: PATCH, updateMany and batch (update) answer the same status and body for an id that names a row and one that names none; DELETE keeps its one answer; a stale If-Match keeps one status and code. Controls: a member holding the edit grant updates (200) and gets 404 RECORD_NOT_FOUND for no row; a member with edit and a row-level restriction (showcase_private_note) gets one not-found answer for a foreign row and for no row and still updates its own; the administrator updates (200), gets 404 for no row on an ordinary object, and gets EXTERNAL_WRITE_FORBIDDEN for both ids on a read-only federated object, as its DELETE does; refused writes leave the row untouched.
  • Engine — packages/objectql/src/engine-write-not-found-gate.test.ts gains section 5: a refusal the middleware chain answers comes before the by-id not-found gate, for update and delete, and nothing reaches the driver.
  • Protocol — protocol.record-not-found.test.ts: a refused write answers the same for a real and a missing id on updateData and deleteData with no read ahead of the engine; the post-write existence check is asked as the system and keeps the 200 of an update that moved its row out of scope. protocol.bulk-record-not-found.test.ts: the same parity on updateManyData and batchData.

Verification

All readings below were taken on head 0acc8a19b (this branch, origin/main merged at 258fb1e94) unless a line names another commit (the metadata-protocol, runtime and dogfood rows were read on d0d3d0be0; 0acc8a19b changes only packages/objectql/src/protocol-data.test.ts); wall times are from a shared container. CI on 0acc8a19b: 50 check-runs, 38 success, 12 skipped, none failed — all eight Test Core shards, TypeScript Type Check, Lint · ESLint and Check Changeset among them.

Touched packages — tests and typecheck
pnpm --filter @objectstack/metadata-protocol test
  Test Files  289 passed | 4 skipped (293)   Tests  21898 passed | 26 skipped (21924)
pnpm --filter @objectstack/objectql test                                   (on 0acc8a19b)
  Test Files  510 passed (510)   Tests  9879 passed (9879)
pnpm --filter @objectstack/runtime exec vitest run src/domains/data-conflict-caller-visibility.test.ts src/plugin-driver-external-object.spike.test.ts
  Test Files  2 passed (2)   Tests  28 passed (28)
pnpm --filter @objectstack/dogfood exec vitest run test/update-by-id-refusal-order.dogfood.test.ts
  Test Files  1 passed (1)   Tests  16 passed (16)
pnpm --filter @objectstack/dogfood exec vitest run test/showcase-anonymous-deny-surfaces.dogfood.test.ts
  Test Files  1 passed (1)   Tests  75 passed (75)

Other suites that drive updateData through a real protocol, on 0acc8a19b: client client.test.ts 219, organizations create-explicit-organization-wall.test.ts 45, plugin-approvals record-lock-batch-row-status.integration.test.ts 4, http-conformance hono-declared-envelope.conformance.test.ts 20, service-automation write-nodes-stored-metadata-family-refusal.integration.test.ts 17, service-messaging (two notification-template files) 38 — all passed.

Consumer suites, on 5a4fb7a95 (the change itself is the same there; later commits touch tests and one equivalent spelling of the post-write check): pnpm --filter @objectstack/runtime test 499 files / 8092 tests passed; pnpm --filter @objectstack/rest test 362 files / 6904 tests passed.

pnpm --filter PKG typecheck exit 0 for metadata-protocol, objectql, runtime and dogfood; eslint --no-inline-config over the 10 touched .ts files exit 0.

Ablation of the reorder — one-shot, restored

Mutation (scripts/ablation-replace.mjs, anchor hit 1 → 0, blob 78abbab6ef4b → c387c962240a): an existence probe put back ahead of engine.update in updateData. ablation-dist-preflight found the marker in dist/index.js and dist/index.cjs after the rebuild.

  • dogfood pin under the mutation: 6 of 16 red — the four PATCH families, the stale If-Match case and the administrator's federated-object case (the bulk doors stay green: they were not mutated);
  • protocol.record-not-found.test.ts under the mutation: 4 of 18 red.

Restore: blob back to the HEAD blob 78abbab6ef4b, git diff HEAD empty, whole-tree git status --porcelain empty, rebuilt; ablation-dist-preflight --absent marker absent from all 24 built files; dogfood pin 16/16 and the unit file 18/18 green.

Gates — dispatch-gates.mjs --commands on 0acc8a1

93 derived families, run locally, every one exit 0 (pnpm check:type-check-debt under the verify lock). dispatch-gates.mjs --ran with every exit code recorded: 93 derived, 93 run, 0 NOT-MEASURED, 0 UNRUN. Among them:

check-system-context-census: OK — 163 elevation read sites in 22 packages across 90 files …
check-doc-anchors: 851 internal #fragment link(s) across 481 source file(s) all resolve to a real heading
check-engine-double-contract: OK — 1149 pinned, 120 in the DEBT ledger, 3 exempt.
check-nul-bytes: OK (scanned 23283 text file(s) … no raw ASCII control bytes)
check-changeset-no-major: This diff introduces no `major` bump.
check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
Full dogfood battery, under the verify lock

On 924d21987: 363 files passed, 3 skipped, 1 failed — showcase-anonymous-deny-surfaces.dogfood.test.ts › "the SAME answer the REST data surface gives for that same id", which compared the action door's ghost-id answer with a member's PATCH on showcase_task, an object that member may not edit. That is this change's consequence: the comparison now uses the data read (the surface the action's record load goes through) and asserts the member's PATCH is the write refusal. The repaired file is 75/75 on d0d3d0be0.

Acceptance notes

  • Fixture triage. The bulk and batch-conformance doubles answered a missing id with an unclassified throw (the write pipeline's old answer); they now refuse it the way ObjectQL's by-id gate does — after the write is judged, before any hook — and "the write pipeline never ran for that row" is asserted on a pipeline spy instead of "the engine was never called". The engine-layer guarantee is pinned by the engine's own not-found gate tests. update-path-id-wins and two runtime tests change their read counts: a tokenless PATCH, like a tokenless DELETE, reads nothing ahead of the engine, and a refused PATCH on a plugin driver costs the remote no read.
  • packages/objectql/src/protocol-data.test.ts (OCC cases). A PATCH with no token or a blank one now asserts zero reads ahead of the engine; a missing id is answered RECORD_NOT_FOUND by the engine's not-found gate (the double rejects with the shared envelope) or, for an engine that resolves a null readback, by the system-context existence check after the write — in both, a token never turns the missing row into a 409.
  • Admin on a read-only federated object. An unknown id now answers the write guard's 403 EXTERNAL_WRITE_FORBIDDEN, the answer its DELETE already gave, where it answered 404.

Generated by Claude Code

… id exists

WIP: the HTTP-door cases, measured before the change.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
…her or not the id exists, as delete does

The by-id update doors (updateData, updateManyData, batchData update)
no longer ask existence ahead of the engine. The engine judges the
write first and refuses a row that is not there itself; the protocol
asks existence only after the engine resolved a null readback.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
The engine pin: a refusal the middleware chain answers comes before the
by-id not-found gate, for update as for delete. The dogfood pin: on the
stock showcase, PATCH, the bulk update doors and DELETE answer one
refusal for an id that names a row and for one that names none, with
the edit-grant, row-scope and administrator controls.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
The batch conformance double refuses a missing id the way ObjectQL's
by-id gate does; a refused PATCH on a plugin driver costs the remote no
read; a tokenless PATCH, like a tokenless DELETE, reads nothing ahead of
the engine.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
… after the write

The existence check below the write throws recordNotFoundError in the
seam itself, and the protocol test drives its refused-caller and
out-of-scope cases through the file's one engine double.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
The action loads its record through the read door, so the matching data
surface is GET by id. A member without edit on the object gets the write
refusal on PATCH whatever the id, now that an update by id is judged
before the record is looked up.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
A PATCH with no token, or a blank one, reads nothing ahead of the
engine. A missing id is answered RECORD_NOT_FOUND by the engine's
not-found gate, or by the existence check after a null readback, and a
token never turns it into a conflict.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 4 documentable anchor(s).

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

  • content/docs/concepts/metadata-lifecycle.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
  • content/docs/kernel/contracts/data-engine.mdx (via updateData (symbol, a method of class ObjectStackProtocolImplementation))
  • content/docs/kernel/services-checklist.mdx (via updateData (symbol, a method of class ObjectStackProtocolImplementation))

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

  • content/docs/releases/v16.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))

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 cross-cutting symbol(s) contributed no route anchor: updateData (4 routes)
  • the SDK route bridge reached 60 of 214 client-bound route-ledger rows — the other 154 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 154: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 99 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 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 7449e479c452394223ff5ab3a7af41c76a880608 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 88d048ed0614a5ab30d5d81a6c8680a7c4173f67 — the merge of head 0acc8a19bedce53504aae3c85b2215a8a5757fb3 into base 7449e479c452394223ff5ab3a7af41c76a880608, 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 88d048ed0614a5ab30d5d81a6c8680a7c4173f67 && git checkout 88d048ed0614a5ab30d5d81a6c8680a7c4173f67
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7449e479c452394223ff5ab3a7af41c76a880608 0acc8a19bedce53504aae3c85b2215a8a5757fb3 && git checkout -B drift-repro 7449e479c452394223ff5ab3a7af41c76a880608 && git merge --no-ff 0acc8a19bedce53504aae3c85b2215a8a5757fb3

node scripts/docs-audit/affected-docs.mjs --json 7449e479c452394223ff5ab3a7af41c76a880608

⚠️ 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 7449e479c452394223ff5ab3a7af41c76a880608 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

ACCEPT → head 0acc8a19bedce53504aae3c85b2215a8a5757fb3 · 2026-11-01T08:56Z

维护者速读

  • 改了什么:按 id 更新记录时,先判断这次更新是否允许,再查记录在不在,和按 id 删除的顺序一致。覆盖单条 PATCH、updateMany、batch(update)三个入口,以及经过它们的 dispatcher /data、/mcp HTTP 工具和声明式更新动作。
  • 为什么改:QA 第三轮发现的安全加固(细节按披露裁决不公开)。
  • 风险与回滚:没有编辑权限的调用方,或写入外部只读数据源的调用方,对不存在的 id 现在也收到 403,原来是 404。有权限的调用方仍然收到 404。成功更新、405、乐观锁、upsert 和系统上下文写入都不变。回滚即 revert 本 PR,不涉及数据迁移。
  • 先看:packages/metadata-protocol/src/protocol.ts 的 updateData,以及 protocol-batch-data.ts 的两处 update 分支。
  • 两分钟验证:pnpm --filter @objectstack/dogfood exec vitest run test/update-by-id-refusal-order.dogfood.test.ts,预期 16/16 通过。
  • 需要你做的:无,席位落地。

席位意见:收。席位逐行读了 protocol.ts、protocol-batch-data.ts、changeset 和文档段,确认三点:

  • 存在性探测从引擎调用之前,移到了引擎返回 null 之后。
  • 写操作被移出调用方行范围后仍回 200 { record: null },原有语义保留。
  • OCC 改用 deleteData 同一个 assertVersionMatch。

席位核对

  1. 形态:draft、目标 main;按披露裁决不建卡,正文没有关闭关键词。正文首行是签名,Clause-②、Metadata-protocol、Skills-docs 三行都在。
  2. 前提实测:基线 bff315cd4 上 dogfood 有 9/15 变红,复现了问题。ablation 把探测放回引擎之前,dogfood 有 6/16 变红、单元测试有 4/18 变红;恢复后全绿,dist 里的标记也已清除。
  3. 门禁:本地 dispatch-gates 共 93 项,全部 exit 0。objectql 全套 510 个文件、9879 个测试通过。本 head 的 CI 共 50 项:38 success、12 skipped、0 失败;Lint & Repo Gates、TypeScript Type Check、Test Core(8 个分片全部)和 Check Changeset 都是 success。上一个 head d0d3d0be0 上 Test Core (7/8) 失败,原因是 protocol-data.test.ts 里 3 个用例还停在旧的探测顺序上。返工只改了这一个测试文件,没动源码;改后的断言更严:无版本号时引擎调用前不读记录,所有探测都是系统上下文,事后的存在性检查一定排在 update 之后。
  4. 契约面:没有碰契约四面。Clause-②: no,这是一种拒绝换成另一种拒绝(404 换成 403),接受集不变,没有改 spec,也没有改受管文本。所以不欠达档复核,按「CI + 席位自读」处理。
  5. 范围外去向:带 If-Match 失效版本的拒绝文案、upsert 按 id 的存在性分叉,这两条按披露裁决私下处理,不上 GitHub。ADR-0142:206 有一句过时的背景描述,属于受管面,本 PR 不碰。
  6. 落地:不属于受管面,794 行,低于 3000 行上限 → pr_ready + automerge_enable。

Generated by Claude Code

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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant