Skip to content

fix(metadata-protocol): deleteMany names the id entry it refuses - #29198

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-t9p12i
Nov 2, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-t9p12i

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread
Fixes #29137

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:批量删除(deleteMany)的 ids 里有空字符串、null 或对象时,仍然 400 拒收,但 fields 现在点名出错的那一项(ids.N),空字符串报 required,其余报 invalid_type。
  • 为什么改:原来不论哪种都只回一句「ids 必须是记录 id 数组」且 fields 为空,ids 明明是数组,调用方找不到是哪一项出错。
  • 风险与回滚:只改拒收时的句子和 fields,拒收与放行的范围不变;回滚即 revert 本 PR。
  • 先看:packages/metadata-protocol/src/protocol-batch-data.ts 的 deleteManyData 拒收分支和 deleteManyIdEntryField。
  • 两分钟验证:pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.delete-many.test.ts,应全部通过。
  • 需要你做的:无,席位落地。

What changes

deleteManyData (the bulk delete door of @objectstack/metadata-protocol) used one check, with one sentence and no fields, for two different faults: ids is not an array, and ids is an array with an entry that is not a record id. It now splits them:

  • ids not an array: unchanged, the sentence "requires 'ids' to be an array of record ids" and no fields.
  • ids an array with bad entries: still 400 VALIDATION_FAILED, now with one fields[] row per offending entry, { field: 'ids.N', code, message }, the shape the REST ingress already gives [null] (zodIssuesToFields). An empty string answers required (the same fault the batch door's rowRequiredIdError calls "Record id is required"), anything else that is not a string or number (null, an operator object, a boolean) answers invalid_type. The sentence names the first offending entry, and says how many were refused when more than one.

What is refused does not move: the fail-closed rule for non-scalar entries is unchanged, and a whitespace-only id still reaches the row loop and reports RECORD_NOT_FOUND for its row. No new error code (the codes are existing members of the ADR-0114 field-level catalog), no packages/spec edit.

Tests

  • packages/metadata-protocol/src/protocol.delete-many.test.ts: new cases for [''], ['a', ''], [{"$ne":null}] (still refused, now named), [null] (same pair as the ingress), several bad entries named at once, a non-array (old sentence, no fields), and [' '] (still reaches the row loop).
  • packages/rest/src/rest-delete-many-ingress.test.ts: a new block through the real protocol at the REST door: {"ids":["a",""]} answers 400 with fields naming ids.1 as required, and [null] keeps the ingress answer ids.0 / invalid_type.
  • No existing test was edited.

Verification

Base fea2dbb7, head 9e074bf5.

Base reading (REST door in-process, real protocol, fake engine)
{"ids":[""]}           400 "requires 'ids' to be an array of record ids"  fields: []
{"ids":["a",""]}       400 "requires 'ids' to be an array of record ids"  fields: []
{"ids":[{"$ne":null}]} 400 "Invalid deleteMany request"  fields: [ids.0 invalid_type]  (ingress)
{"ids":"a"}            400 "Invalid deleteMany request"  fields: [ids invalid_type]    (ingress)
{"ids":[null]}         400 "Invalid deleteMany request"  fields: [ids.0 invalid_type]  (ingress)
{"ids":["  "]}         200 row RECORD_NOT_FOUND

In-process (protocol called directly) every one of the first five got the array sentence and no fields. Premise holds.

Suites
  • pnpm --filter @objectstack/metadata-protocol test: Test Files 297 passed, 4 skipped; Tests 22069 passed, 26 skipped. typecheck: exit 0.
  • REST deleteMany data-door tests (the 17 packages/rest/src/*.test.ts files that mention deleteMany): Test Files 17 passed; Tests 1049 passed.
  • pnpm exec eslint --no-inline-config on the three touched source/test files: exit 0.
Ablation (two legs, each rebuilt, restored, git diff HEAD empty)
  • Leg 1, drop err.fields = idFields: 5 protocol cases red (expected undefined to deeply equal [ { field: 'ids.0', ... } ]) and the REST case red (expected [] to deeply equal [ [ 'ids.1', 'required' ] ]).
  • Leg 2, accept an empty string as an id: 3 protocol cases red and the REST case red (expected 200 to be 400).
  • Restored through ablation-replace.mjs --restore (blob equals HEAD), rebuilt, ablation-dist-preflight --absent passed.
Gates

dispatch-gates --commands derived 60 commands; 58 exited 0. check-plugin-teardown-shape --self-test exited 3 on a shallow clone, then 0 after fetching its pinned fixture commit. check:dual-build-cjs-loads exited 3 (PREREQUISITE NOT MET: needs the whole workspace built): NOT MEASURED locally, CI runs it. --ran reconciliation: 60 derived, 60 run, 0 UNRUN.

Acceptance notes

  • Choice of required for an empty string (rather than min_length, which a z.string().min(1) ingress would spell) follows the batch door's "Record id is required" for the same fault. Flagged to the seat as an open question.

Generated by Claude Code

A deleteMany whose ids array holds an empty string, null or an operator
object is still refused with 400 VALIDATION_FAILED, but the answer now
names each offending entry in fields[] as ids.N (required for an empty
string, invalid_type otherwise), the shape the REST ingress already gives
[null]. A non-array ids keeps the old sentence.

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

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 692e0dc8b936ad37311352868083d78745c76a21.

⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: VALIDATION_FAILED (literal, 38 pages)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 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; 101 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 692e0dc8b936ad37311352868083d78745c76a21 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from dc385fa4996e997ee23f793743aa6faad200b48e — the merge of head 9e074bf5c993869678459b60406304591572c1a0 into base 692e0dc8b936ad37311352868083d78745c76a21, 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 dc385fa4996e997ee23f793743aa6faad200b48e && git checkout dc385fa4996e997ee23f793743aa6faad200b48e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 692e0dc8b936ad37311352868083d78745c76a21 9e074bf5c993869678459b60406304591572c1a0 && git checkout -B drift-repro 692e0dc8b936ad37311352868083d78745c76a21 && git merge --no-ff 9e074bf5c993869678459b60406304591572c1a0

node scripts/docs-audit/affected-docs.mjs --json 692e0dc8b936ad37311352868083d78745c76a21

⚠️ 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 692e0dc8b936ad37311352868083d78745c76a21 → 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:server#1 · os-steve · session_01ADHkmQjPiXgNoSaEpqeGvY

席位核对 · ACCEPT · domain:server seat 1 · os-steve · session_01ADHkmQjPiXgNoSaEpqeGvY · 2026-11-02T09:13Z

Head 9e074bf5c9, report 6573344719 (on #29137), claim 6572652287. Clause-②: no, no arm: the accept set is unchanged, so no contract review is owed. The seat read the diff itself.

item reading
Shape Draft, base main, Fixes #29137. 4 files (+145 / −1): deleteManyData and a private deleteManyIdEntryField in metadata-protocol/src/protocol-batch-data.ts, pins in protocol.delete-many.test.ts and rest/src/rest-delete-many-ingress.test.ts (declared deviation: the REST door pin), a @objectstack/metadata-protocol patch changeset. Metadata-protocol: none. No packages/spec/**, no new error code, no gate. Not governed.
Rule The one check is split: a non-array ids keeps its sentence and empty fields; an array with a non-id entry is still 400 VALIDATION_FAILED, now with one fields[] row per entry {field: 'ids.N', code, message} (required for "", invalid_type for null / object / boolean), the shape the REST ingress gives [null]. isScalarId is unchanged, so what is refused is unchanged (fail-closed for {"$ne":null} kept); [" "] still reaches the row loop.
Premise Measured on base by the dev: [""] and ["a",""] answered the array sentence with fields: [].
Evidence metadata-protocol 22069 pass / 26 skipped, typecheck 0; 17 REST deleteMany files 1049 pass; ablation red on both legs (protocol and REST through dist), restore clean. Gates 60 derived; check:dual-build-cjs-loads NOT MEASURED locally (needs a full build), left to CI. CI on this head is running; auto-merge waits for every check green or an expected skip.
Open question required vs min_length for "": the seat takes required (the dev's recommendation; it matches the batch door's "Record id is required" and is in the ADR-0114 field catalog).
Out of scope none.
Landing 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 2, 2026 09:14
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 2, 2026 09:15
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 761fef4 Nov 2, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-t9p12i branch November 2, 2026 10: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

1 participant