[huangyiirene]

Fixes #19433

Clause-②: no

What was wrong

POST /api/v1/data/{object}/batch with operation: "delete" reported a deletion it did not
perform. This is the third and last of the three by-id delete doors to read the engine's
answer: the single-record DELETE (PR #19411) and deleteMany (PR #19432) already do, and
this branch is the one that fell between them with no card.

Each row pushed success: true as a literal, so every engine result that was not the
driver contract's false was reported as a deletion. The false arm — "no row matched" —
has reported honestly since #4435 and #5088. The other ending had not: a row that matched
and was deliberately not removed was counted in succeeded and shipped an envelope
byte-identical to a batch in which every id really went.

sys_permission_set is the shipped shape of it. Deleting a package-declared set is an
ADR-0005 RESET — plugin-security's write-through tombstones the overlay and the record
re-projects to the declared body instead of vanishing — so the row matched, the write ran,
and the record is still there. On a security-configuration write the envelope told an
operator a permission set was gone while it was still being enforced.

Why this is a defect today — rested on the CONTRACT, not on a shipped handler

IDataEngine.delete declares Promise[boolean | number]
(packages/spec/src/contracts/data-engine.ts; square brackets here for the generic, the
spelling this family's cards use) — the driver boolean for a by-id write, a COUNT of rows
removed otherwise. isDeleteResultShape in packages/objectql/src/verb-hook-result-shape.ts
is a pair of typeof tests admitting boolean and number, and the ADR-0112 hook gate in
packages/objectql/src/engine.ts uses it. So an afterDelete handler — or a non-ObjectQL
IDataEngine — may legally answer 0 today, and a numeric zero is the one value that
positively means "the row is still there".

This PR does not rest on "a shipped handler already returns the 0". That claim was made
on #19412 and the seat could not verify it; the sibling fix stood because the contract
argument is independent, and this one stands the same way.

Located by symbol, because this family's line numbers have drifted four times

grep -n "throw recordNotFoundError" packages/metadata-protocol/src/protocol.ts on this
branch's base (23f1de078):

door	line on this base	state before this PR
single-record deleteData	:11438	reads the engine result (#19306 / PR #19411)
runBatchDataLoop case 'delete'	:12568	the literal — this PR
runDeleteManyLoop	:13120	reads the engine result (#19412 / PR #19432)

The file's own comment beside the batchData site calls it "the OTHER by-id bulk delete, ten
lines from it".

Measured on the unfixed base, before any edit

A fake engine per answer, through batchData with one record:

engine.delete answers	batch door, before	after this PR
true	success: true, succeeded: 1	unchanged
1	success: true, succeeded: 1	unchanged
0	success: true, succeeded: 1, failed: 0	success: false, succeeded: 0, failed: 1, no errors
false	success: false + errors[0].code RECORD_NOT_FOUND, failed: 1	unchanged
undefined	success: true	unchanged

Mixed batch ['a', 'kept', 'b'] where kept answers 0, before: all three success: true,
succeeded: 3. Atomic with the same engine, before: committed, succeeded: 3, kept
silently still present.

The envelope measurement this card owes — what transfers, and what does not

batchData's envelope is its own, so each of these was read off this file rather than
inherited from deleteMany. All three shapes turn out to agree, and the reason is
mechanical: the two surfaces share the machinery.

question	deleteManyData	batchData	why
same succeeded/failed partition (#7539)?	yes	yes	both response builders call the shared reconcileStoppedBatch; buildBatchDataResponse then sets success: failed === 0, total: records.length
an atomic arm, aborting on failed > 0?	yes	yes	runAtomicBatchData delegates to the shared runAtomicBatch, whose abort condition is outcome.failed > 0
does the continueOnError stop apply?	no	no	the stop lives in the loop's catch, and a surviving row throws nothing

Two things about batchData that have no counterpart on deleteMany, and are therefore
pinned separately:

1. runBatchDataLoop is shared by create / update / upsert / delete, and each
   case owns its own succeeded++. Only the delete case is touched; the sibling arms keep
   their counters exactly as they were.
2. returnRecords: false is a batchData-only projection (buildDeleteManyResponse has
   no such flag). It drops data and keeps success / index / errors, so the one key
   this card moves is the key that survives it — pinned.

The fix

The per-row push in case 'delete' reads the engine result instead of being a literal:

const removed = deleted !== 0;
results.push({ id: record.id, success: removed, index });
if (removed) succeeded++;
else failed++;

Not false for this case. That arm is spoken for by the 404 above, about a record this
caller can still GET; answering it here would trade one wrong answer for a louder one. Only
a POSITIVE zero is read as "not removed", and an off-contract undefined from a third-party
driver keeps its #4435 reading.

And the row gets no errors[] entry. A surviving record is an OUTCOME, not a fault; the
single-record door answers the same case with a bare success: false on a 200, and this
envelope's two per-row codes (ROLLED_BACK, NOT_ATTEMPTED) both describe a row that never
ran. Minting a code for this ending is an ERROR_CODE_LEDGER widening in packages/spec and
belongs to the spec lane. That was ruled on #19412 and the ruling is carried here, not
re-opened.

HTTP status is untouched — the route hands the protocol result straight to res.json, so
this stays a 200 whose body reports per row.

On the wire, per row

case	before	after
row removed	success: true, in succeeded	unchanged
matched, not removed (packaged-set reset)	success: true, in succeeded	success: false, in failed, no errors[]
unknown id	success: false, errors[0].code RECORD_NOT_FOUND	unchanged
batch stopped / rolled back	NOT_ATTEMPTED / ROLLED_BACK	unchanged

Because the counters partition results, a surviving row also makes the request-level
success false, and an atomic batch holding one now rolls back rather than committing
under a response that called every row deleted. A non-atomic batch is not stopped by it.

Evidence

All readings at 7b9c6376c (final commit, origin/main merged in), worktree clean, base
23f1de078.

The pin, and that it can fail. protocol.bulk-record-not-found.test.ts gains a
[#19433] block of five cases beside the existing [#5088] one: the NEGATIVE case (engine
answers 0, so success: false, succeeded: 0, failed: 1, no errors key, and the
record is still in the store), a CONTROL leg (a positive count still reports a deletion —
without it the pin can pass vacuously), a mixed batch pinning both directions plus the
partition and the un-stopped run, the atomic arm (the surviving row aborts and the earlier
delete is really undone), and returnRecords: false. Run against the UNFIXED base first:
4 failed / 18 passed. After the fix: 22 passed.

Reverse verification, both legs from the committed state, mutation proven on disk and
restored by blob hash (scripts/ablation-replace.mjs, mutation and command in one process):

mutation	anchor	blob	result
the four fixed lines back to success: true + succeeded++	x1 to x0	aaa92be21a62 to d9c2974fa715	4 failed / 18 passed
const removed = deleted !== 0 to const removed = false	x1 to x0	aaa92be21a62 to 39ef16f7b8ee	7 failed / 15 passed

Both restored: blob equals HEAD's (aaa92be21a62) and git diff HEAD is empty. The first
leg also proves the suite resolves src/protocol.ts and not dist/ — dist/ held the FIXED
bytes at that moment (success: removed occurs twice in dist/index.js) and the run still
went red.

Unit + types. pnpm --filter @objectstack/metadata-protocol test — 183 files
(3 skipped) / 2621 passed (19 skipped), exit 0. typecheck exit 0, and
tsc --noEmit --listFiles names protocol.bulk-record-not-found.test.ts once (negative
control: a name not in the program is listed 0 times), so the pin is inside the program the
script advertises. No public export moved, so no consumer package owes a test here.

Gates. node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack
derived 61 families from this diff; all 61 ran, all exit 0, reconciled with --ran
carrying each exit code — "61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN". Re-derived after
git fetch origin main and again after merging it: identical list, so the new
check-dts-references family that landed on main meanwhile is not one this diff owes.
Three families (check:dual-build-cjs-loads, check:lean-entry-closure,
check:type-check-debt) exited 3 PREREQUISITE NOT MET on the first sweep, were given the
repo-wide build CI gives them, and then exited 0; the exit-3 runs are recorded as not
measured rather than as failures. Repo-wide pnpm lint ran unnarrowed
(eslint . --no-inline-config): exit 0, at this same head.

Changeset — measured, not assumed. @objectstack/metadata-protocol is private: false
with files: ["dist", "README.md", "CHANGELOG.md"], and the edited symbol ships: batchData
occurs 25 times in the built dist, and the moved expression success: removed occurs in
both dist/index.js and dist/index.cjs, against a negative control symbol that occurs 0
times. So a changeset is owed and skip-changeset does not apply. patch: a released
package's bug fix whose published accept set does not move — no schema key added or removed,
no closed-set member, no new published export, no registry entry, no request shape touched.
What changes is the VALUE of an existing key, pulled back to what
BatchOperationResultSchema.success already declares ("Whether this record was processed
successfully"), on a schema whose errors was already optional. Hence Clause-②: no,
measured against this diff rather than copied from #19412's verdict.

Acceptance notes

Noticed while measuring, deliberately not carried here:

• The "causal row" of a stopped or rolled-back batch can now name a row that did not
  fail. reconcileStoppedBatch and buildRolledBackBatchResponse both locate the cause
  with findIndex(r => !r.success), which since PR fix(security,metadata-protocol): deleteMany's per-row success stops reporting a deletion it did not perform #19432 can land on a SURVIVING row — a
  non-success that carries no error. Measured on this branch: a non-atomic
  ['survivor', 'missing', 'other'] delete batch answers row 2 with
  NOT_ATTEMPTED: "record 0 failed — unknown error; the batch stopped there." when it was
  row 1 that threw; an atomic batch answers a rolled-back row with
  ROLLED_BACK: "record 1 failed — unknown error" for a row that survived rather than
  failed. This is already live on origin/main through deleteMany since PR fix(security,metadata-protocol): deleteMany's per-row success stops reporting a deletion it did not perform #19432 landed,
  and it reproduces identically there — so it is a shared-builder defect, not one this PR
  introduces, and fixing it means editing builders all three bulk faces read. Reported for
  filing. No pin in this PR asserts either message.
• makeStoreEngine's fake delete answers { deleted: 1 } — an object, which is
  neither arm of the declared boolean | number. It happens to be !== 0 so the existing
  cases keep their reading, but it is a test double speaking off-contract. Noted, not filed:
  no consumer.
• os data delete's unconditional print was already recorded on PR fix(security,metadata-protocol): a packaged permission set's DELETE stops reporting a deletion it did not perform #19411 and is not
  re-reported here.
• [finding] deleteManyData 的批量删除门仍无条件 success: true —— 打包集在批量里照样被报告为「已删除」,与 #19306 刚修的单记录门同形 #19412 is not addressed here; it landed in PR fix(security,metadata-protocol): deleteMany's per-row success stops reporting a deletion it did not perform #19432 and this branch does not touch the
  runDeleteManyLoop region.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NcPSwnmJHczmTu6FG7NMjE

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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 — 11 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 57ceb9d6d2af35ebf83d0f8cc396cb612540af74 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 57ceb9d6d2af35ebf83d0f8cc396cb612540af74

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