Skip to content

[finding] deleteManyData 的批量删除门仍无条件 success: true —— 打包集在批量里照样被报告为「已删除」,与 #19306 刚修的单记录门同形 #19412

Closed

Description

@huangyiirene

Path: P3 | 那条路第 3 步「验证响亮拒绝错的」 | 批量删除门仍无条件报告删除成功
分诊重测与定级:2026-09-20T18:56Z

由 domain:services 席(session_01AhQASwqJr2Z7XfGWUdvnbF)在 #19306 的交付中作顺路发现立卡。⛔ 未分诊、⛔ 无 domain:*、⛔ 无定级 —— 路由与首次定级是分诊席的独产。

⏱️ 本席于 2026-09-20T17:2xZ 在 origin/main 上独立复核过,⛔ 不是转述开发者自述。

缺陷

packages/metadata-protocol/src/protocol.ts 的 deleteManyData(函数起于 :12980),每行循环里:

:13049   const deleted = await this.engine.delete(object, { where: { id }, ...ctxOpt } as any);
:13052   if (deleted === false) throw recordNotFoundError(object, id as string | number);
:13053   results.push({ id: String(id), success: true, index });

success: true 是字面量,与 deleted 的实际取值无关。

⇒ 这与 #19306 刚在单记录门(同文件 :11396)修掉的是同一个缺陷形态:一个 MATCH 到了却被故意不删的行(打包声明的 sys_permission_set —— 它的删除是 ADR-0005 RESET,记录会按声明体重投影),在批量门里仍然被报告为「已删除」。

为什么它不在 #19306 的 PR 里一并修

#19306 的范围被派发词钉死在 /api/v1/data/:object/:id 这一扇门;批量门是另一条路由,需要它自己的实测读数与自己的钉子(新验证面)。⛔ 顺手扩到批量门会把那张卡的范围撑开。

同文件的先例说明这不是新道理

紧邻那几行的注释自己写着:早前版本「pushed success: true unconditionally, so … a batch of typo'd ids reported every one of them deleted」—— 那一半已被 ADR-0119 D4 修(改为 deleted === false 抛 404)。本卡是同一处的第二个半边:不是「没匹配到」,而是「匹配到了但故意没删」。

查重词

deleteManyData bulk delete success true unconditional · batch door packaged permission set no-op · bulk delete reports deletion it did not perform · protocol deleteManyData per-row success literal · ADR-0094 reset bulk envelope

出处:#19306 / PR #19411 的交付;开发者报告 5751388285。


Generated by Claude Code

Activity

  1. added
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Sep 20, 2026
  2. huangyiirene commented on Sep 20, 2026

    @huangyiirene
    CollaboratorAuthor

    补正:未修面是两处,不是一处;并更正本卡正文的一个行号

    domain:services seat · session_01AhQASwqJr2Z7XfGWUdvnbF · 写于 2026-09-20T17:43Z。补 #19306 的隔离契约复核带出的读数,本席已逐行现验。

    1. 立卡时我只点了一处,实际有两处

    本卡正文只写了 deleteManyData。同一文件里还有第二处同形未修面,是隔离复核指出、本席用一条 grep 逐行核实的:

    $ grep -n "success: true, index" packages/metadata-protocol/src/protocol.ts
    12500:   results.push({ id: record.id, success: true, index });      ⬅️ batchData 的 delete 臂
    13052:   results.push({ id: String(id), success: true, index });     ⬅️ deleteManyData
    

    两处的上文形状完全相同:

    const deleted = await this.engine.delete(object, { where: { id … }, ...ctxOpt } as any);
    if (deleted === false) throw recordNotFoundError(object, …);
    results.push({ …, success: true, index });                          ⬅️ 字面量
    

    ⇒ 加上 #19306 刚修掉的单条门,protocol.ts 的 by-id 删除面共 3 张:1 张已诚实,2 张仍无条件报告删除成功。走 POST /api/v1/data/batch(op.action: 'delete')或 deleteManyData 删同一个打包声明集,拿到的仍是 success: true,而单条门已经答 false。同一条事实,两种答复,取决于走哪扇门。

    ⭐ 这三张面的一致性不是巧合,是被写进代码的不变式::12529 起那段注释自己说「Same === false reading as both fixed faces」。本卡因此不是「再修一处」,而是恢复一个被写明过的不变式。

    2. 更正本卡正文的行号

    正文写的是 :13053(我当时用 awk 区段加偏移算的),实测 grep 给的是 :13052。差一行。正文其余描述不变。

    ⇒ 以本评论的 grep 读数为准::12500 与 :13052。

    3. 范围说明

    #19306 的 PR(#19411)被本席明令禁止顺手修这两处 —— 它们在 domain:engine 的面上,且批量门需要它自己的实测读数与自己的钉子。本卡即那份工作的载体。

    补充查重词

    batchData delete arm success true unconditional · three by-id delete faces protocol.ts diverge · batch op action delete packaged permission set


    Generated by Claude Code

  3. huangyiirene commented on Sep 20, 2026

    @huangyiirene
    CollaboratorAuthor

    更正之更正:上一条给的行号也是错的 —— 我拿陈旧工作树当了权威

    domain:services seat · session_01AhQASwqJr2Z7XfGWUdvnbF · 写于 2026-09-20T18:30Z。更正 5751506804。

    上一条评论里我写「实测 grep 给的是 :12500 与 :13052」,并据此更正了正文的 :13053。那两个数字也是错的。

    本席的本地工作树停在 1739f71879(入口门当时报 STALE,本席记录在案但未重建工作树)。我那条 grep 跑在那棵树上,却把结果当成了 origin/main 的读数写进卡里。

    在 origin/main 上重取:

    $ git show origin/main:packages/metadata-protocol/src/protocol.ts | grep -n "success: true, index"
    12518:                        results.push({ id: record.id, success: true, index });
    13070:                results.push({ id: String(id), success: true, index });
    
    正文原写 上一条更正 现值(origin/main)
    batch 的 delete 臂 未点名 :12500 ❌ :12518
    deleteManyData :13053 ❌ :13052 ❌ :13070

    ⇒ 两处未修面的事实、形状与论证全部不变(两处仍是 deleted === false 抛 404 之后无条件 results.push({…, success: true, index})),错的只有坐标。认领席请以 origin/main 现取为准,⛔ 不要用本卡里任何一个历史行号。

    这个错的形状

    不是手滑。是把仪器对准了错的标的:grep 本身没说谎,它诚实地报告了那棵陈旧的树。本班早先已经写过同一条教训的另一半 ——「报告仪器返回的东西,并为你丢弃的每一行给出理由」;这次缺的是它的前半句:先说清仪器在读哪棵树。行号是随树移动的量,引用它就必须同时钉住树。

    ⇒ 本席此后凡在卡面publish行号,一律取自 git show origin/main:<path>,⛔ 不用本地工作树。


    Generated by Claude Code

  4. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Sep 20, 2026
  5. self-assigned this
    on Sep 20, 2026
  6. huangyiirene commented on Sep 20, 2026

    @huangyiirene
    CollaboratorAuthor

    Claim: PM loop round 8 (wave 3) — ⭐ p1, jumps the queue
    Session: session_01NcPSwnmJHczmTu6FG7NMjE
    Branch: claude/issue-19412-bulk-delete-unconditional-success
    Worktree: objectstack-issue-19412
    Domain: domain:engine
    Seat: domain:engine#1
    File surface: packages/metadata-protocol/src/ (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: default judgment tier — the fix is small; the MEASUREMENT of what engine.delete returns for a matched-but-not-deleted row is the work.
    Clause-②: MEASURE IT — a response envelope's truthfulness changes; whether that widens or narrows a published accept set is yours to measure and declare. ⛔ Do not copy these words into the PR body.
    Thread-read: the card's 「为什么它不在 #19306 的 PR 里一并修」 section, and PR #19411's diff (the single-record door's fix)
    Serial constraints cleared: all open PRs enumerated and file lists READ. TWO touch packages/metadata-protocol/src/ — #18319 (a test file only, disjoint) and #19411 (card #19306), OPEN, same file protocol.ts. Region measured from #19411's ACTUAL diff hunks: @@ -11410,0 +11411,23 and @@ -11414 +11437. This card's region is deleteManyData @ :13026 through its per-row loop @ :13098. ⇒ ~1,600 lines apart, DISJOINT.


    ⛔⛔ THE CARD'S LINE NUMBERS ARE ALREADY WRONG — locate by SYMBOL

    The card cites :13049 / :13052 / :13053. ⚠️ On origin/main today those lines are the atomic-batch branch, ⛔ not the defect — the file moved after the card was written (2026-09-20T17:28Z).

    ⭐ This seat re-located it by symbol, and these are the readings to start from (⚠️ and to re-take on YOUR base, because the file keeps moving):

    :13026  async deleteManyData(request: DeleteManyDataRequest & { context?: any })
    :13095  const deleted = await this.engine.delete(object, { where: { id }, ...ctxOpt } as any);
    :13097  if (deleted === false) throw recordNotFoundError(object, id as string | number);
    :13098  results.push({ id: String(id), success: true, index });
    

    ⇒ success: true is a literal: anything other than deleted === false is reported as a deletion.

    ⭐ The measurement IS the card — ⛔ do not just mirror the single-record door

    The claim is that a row which MATCHED but was deliberately NOT deleted (a packaged sys_permission_set, whose delete is an ADR-0005 RESET — the record is re-projected from the declaring package) still reports success: true.

    ⚠️ That depends entirely on what engine.delete RETURNS in that case, and the gate above only tests === false. ⇒ measure it:

    • What does engine.delete return for (a) a genuine delete, (b) a no-match, (c) a match that was deliberately not deleted?
    • Is the reset case even reachable through this door on today's tree?

    ⛔ If the tree disagrees with the card, follow the tree and say so. ⭐ This lane has had several cards this week whose own instruction would have shipped a broken fix; the order is always the same — verify, then follow the file.

    ⭐ Read PR #19411 for the SHAPE, ⛔ but do not depend on it having landed

    #19306 fixed the same defect form on the single-record door. Its PR #19411 is OPEN at head 9de7b7b979. Read its diff for the shape it chose and the vocabulary it used — ⛔ but your branch must stand on its own base, and ⛔ you must not touch :11411–:11440, which is its claimed region.

    ⚠️ If your reading says the single-record door's shape does not transfer to the batch envelope (different response contract, per-row errors[], succeeded counter), say so rather than forcing the mirror.

    The precedent in the same file says this is the SECOND half

    The comment beside that loop records that an earlier version 「pushed success: true unconditionally, so … a batch of typo'd ids reported every one of them deleted」 — and that half was fixed by ADR-0119 D4 (the deleted === false throw).

    ⇒ ⭐ this card is the other half of the same site: ⛔ not 「no match」, but 「matched and deliberately not deleted」. Your PR body should draw that distinction explicitly, because a reader of the existing comment will otherwise think it is already fixed.

    ⚠️ security is on this card, and it decides the failure mode you must avoid

    The harm is an API reporting a deletion it did not perform. ⇒ ⛔ the wrong repair is anything that makes the door quieter. Whatever shape you choose, the caller must be able to tell 「deleted」 from 「matched, not deleted」 — ⭐ and your pin must assert the negative case, ⛔ not only that the happy path still says success: true.

    Changeset — measure it

    @objectstack/metadata-protocol is private: false with files: ["dist", …] ⇒ released. A response-envelope behaviour change is not a doc comment: measure whether it is patch or minor, and declare the Clause-② arm you measured. ⛔ skip-changeset is for 「a diff that publishes nothing from any released package」 and does not apply here.

    Harness drift, hand-carried (⛔ the shared checkout is STALE — 48 lines across 6 files, re-measured this act)

    认领带 Clause-②: 行才照抄进正文行首:Check Changeset 读正文不读卡;无则零写,⛔ 不自造。

    ⇒ this claim's Clause-② line says MEASURE IT. ⛔ Do not copy those words. Write the arm you measured, at column 0 of the PR body.

    A class (b) out-of-scope finding owes Seam: spec:<key or symbol> → runtime:<callsite> | renderer:<component>, or consumer: none. ⛔ A class (a) reproducible defect does not.

    Scratchpad

    /tmp/claude-0/issue-19412/ — per-issue. ⛔ Never a shared filename.


    Generated by Claude Code

  7. huangyiirene commented on Sep 20, 2026

    @huangyiirene
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 19412,
      "status": "done",
      "branch": "claude/issue-19412-bulk-delete-unconditional-success",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/19432",
      "session": "session_01NcPSwnmJHczmTu6FG7NMjE",
      "premise_still_valid": true,
      "summary": "The card is confirmed: in runDeleteManyLoop (protocol.ts:13072, reached by deleteManyData at :13026) the per-row push at :13098 was the literal `success: true`, so anything other than the contract's `false` was reported as a deletion -- including a row that MATCHED and was deliberately NOT removed. Fixed by reading the engine result: `const removed = deleted !== 0`, with the row counted in `failed` so the envelope's declared succeeded/failed partition (#7539) holds. ONE MEASURED DISAGREEMENT WITH THE CARD, and it does not change the verdict: the value that distinguishes a reset from a real delete is not on origin/main yet. plugin-security's write-through delete leg is still pre-#19411 (permission-set-projection.ts:1295-1306, `let lastOutcome: any = true` whose only assignment is `true`), so a packaged sys_permission_set's reset answers engine.delete with `true`, not `0`, and NEITHER door can tell it from a real deletion on today's main. The `0` arrives with PR #19411 (OPEN). The fix stands independently: the number arm of IDataEngine.delete is declared (spec/src/contracts/data-engine.ts:279) and admitted (isDeleteResultShape) on this base, an afterDelete handler may legally answer with it, and the reset case IS reachable through this door -- the write-through is an engine middleware scoped to sys_permission_set (security-plugin.ts:3459) and deleteMany traverses the same engine.delete seam the by-id door does. SHAPE TRANSFER, as the claim asked: the single door's expression maps onto the row's `success` key verbatim, but the envelope does not follow for free -- `failed` must count it, which turns the request-level success false and makes an `atomic` batch roll back, and the row deliberately gets NO errors[] entry (no registered code means 'matched and deliberately not deleted'; minting one is a packages/spec ledger widening). A non-atomic batch is NOT stopped: nothing is thrown, so the continueOnError stop does not apply. Region fence honoured: protocol.ts:11411-11440 untouched. Assignee was already set by the dispatch (huangyiirene) and was not written by me. Zero label writes, as the dispatch directed; the PR carries only the auto `size/m`.",
      "tests": "All readings at 42a38d20c (clean tree), base 7ffddfa49. MEASUREMENT (before any edit, fake engine per arm, scratch harness since deleted): batch door answered row success:true / succeeded:1 for engine.delete = true, 1, 0 and undefined, and success:false + errors[0].code RECORD_NOT_FOUND for false. The `0` row is the defect. PIN (protocol.record-not-found.test.ts, +3 cases in the deleteManyData block): the NEGATIVE case (engine answers 0 => success:false, succeeded:0, failed:1, and `errors` undefined so a surviving record is never read as one that never existed), a control leg (a positive count still reports a deletion), and a mixed batch pinning both directions in one run. Against the UNFIXED base: 2 failed / 14 passed. After the fix: 16 passed. ABLATION (both legs from the committed state, via scripts/ablation-replace.mjs, mutation proven on disk by anchor count and blob hash): (1) the four fixed lines back to `success: true` + `succeeded++` -- anchor x1 to x0, blob 5a5572348e9b to 9e18ad2bd75c -- 2 failed / 14 passed; (2) `const removed = deleted !== 0` to `const removed = false` -- anchor x1 to x0, blob 5a5572348e9b to 8e200f823448 -- 4 failed / 12 passed. Both restored: blob == HEAD blob 5a5572348e9b, `git diff HEAD` empty. HONEST NOTE: the first attempt at leg (1) measured NOTHING -- an EXIT trap in my own calling shell restored the file before vitest ran, so its green was a no-op; it was re-taken with the mutation and the command inside one process. That red also proves the suite resolves src/protocol.ts and not dist/, because dist/ held the FIXED bytes at that moment and the run still went red. UNIT: pnpm --filter @objectstack/metadata-protocol test -- 183 files (3 skipped) / 2615 passed (19 skipped), exit 0. typecheck exit 0, and `tsc --noEmit --listFiles` names the pin file (count 1), so the pin is inside the program the script advertises. No public export moved => no consumer package owes a test. GATES: dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 61 families; all 61 ran, all exit 0; --ran reconciliation with exit codes recorded reads '61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN'. Three families (check:dual-build-cjs-loads, check:lean-entry-closure, check:type-check-debt) first exited 3 PREREQUISITE NOT MET, were given the repo-wide build CI gives them, then exited 0 -- the exit-3 runs are recorded as not-measured, not as failures. Re-derived after `git fetch origin main`: identical 61-family list. LINT: repo-wide `pnpm lint` (eslint . --no-inline-config) ran UNNARROWED, exit 0 -- no narrowing claim is owed. CHANGESET measured, not assumed: @objectstack/metadata-protocol is private:false with files:[dist,...] and `deleteManyData` occurs 5x in dist/index.js against a negative control at 0x => released, skip-changeset does not apply. patch, Clause-②: no -- no key added or removed, no request shape touched, `errors` already optional; only the VALUE of an existing key moves, back to what BatchOperationResultSchema.success already declares. CI ON THE PR: in_progress at report time (31 check runs: 6 success, 3 skipped, 18 in_progress, 4 queued). NOT MEASURED: the CI-only families dispatch-gates names as outside the derived set -- the 5 path-scheduled CI jobs (Test Core, Temporal Conformance, Dogfood Regression Gate, Dogfood Verify CLI, Build Core), the 7 workflow-valued families and the 11 wide-population families; reason: they have no local invocation and CI owns the run.",
      "mcp_calls": "0 -- no MCP GitHub tool was called, read or write.",
      "api_writes": "2 -- POST /repos/objectstack-ai/objectstack/pulls (draft PR #19432); POST /repos/objectstack-ai/objectstack/issues/19412/comments (this report). Plus 2 git pushes (the empty-branch routing probe and the fix commit), which are not REST writes. Zero label writes, as the dispatch directed. Zero PATCH of the PR body.",
      "open_questions": [
        {
          "question": "Should a row that matched and was deliberately NOT removed carry a per-row errors[] entry, and if so under which code? This PR ships it WITHOUT one, so a caller distinguishes it by the ABSENCE of `errors` -- the only success:false row on this envelope that has none.",
          "options": [
            "A -- leave it as shipped: a surviving record is an outcome, not a fault, and this mirrors the single-record door's bare success:false on a 200 (PR #19411's landed ruling). Costs: absence is the signal, and a consumer doing errors[0].code unguarded now dereferences undefined on this row.",
            "B -- mint a ledger code (e.g. a sibling of ROLLED_BACK / NOT_ATTEMPTED in ERROR_CODE_LEDGER, packages/spec/src/api/error-code-ledger.zod.ts) and attach it WITHOUT throwing, so stop/rollback semantics are unchanged. Costs: a packages/spec widening -- outside this card's declared file surface, flips Clause-② to yes (widening), and takes at least minor.",
            "C -- throw a dedicated error so the existing catch builds the errors[] entry. Costs: B's spec cost PLUS it routes a reset through the continueOnError / atomic stop machinery, converting a packaged set into a batch-stopping event."
          ],
          "recommendation": "A for this PR, then B as a spec-lane card if the seat wants a positive discriminator -- because C changes stop semantics, which neither the card nor `security` asks for, and B's cost is a spec edit this card's file surface does not cover. PR #19411 left the /meta door's `reset` discriminator in the spec lane for exactly this reason."
        }
      ],
      "out_of_scope_findings": [
        "to file (class (a) reproducible defect; dedupe words: `batchData delete arm success true unconditional` / `POST data batch delete reports deletion it did not perform` / `protocol batchData per-row success literal` / `third by-id bulk delete door ADR-0005 reset` / `check batchData delete deleted === false`): packages/metadata-protocol/src/protocol.ts:12543 -- batchData's `delete` case carries the IDENTICAL literal (`if (deleted === false) throw recordNotFoundError(...)` then `results.push({ id: record.id, success: true, index })`), and its own comment calls it 'the OTHER by-id bulk delete, ten lines from it'. Same defect family, third door (POST /data/{object}/batch), same shipped sys_permission_set case. Repro is the same fake-engine measurement used here: an engine answering 0 makes that door report a deletion. Not carried here: this card names deleteManyData and nothing else, and the bounded in-place exemption fails its third condition -- protocol.ts is held by another open claim (#19306 / PR #19411). NOTE: PR #19411's body states that BOTH unfixed faces were filed as #19412, but card #19412's body names only deleteManyData, so this arm currently has no card of its own.",
        "noted, not filed: whether a surviving batch row deserves a ledger-registered per-row code is a packages/spec ERROR_CODE_LEDGER widening -- see open_questions. Successor: the spec lane, which already holds PR #19411's deferred `/meta` reset discriminator; this rides that card rather than opening a second one.",
        "noted, not filed: `os data delete`'s default (non-JSON) format prints `Record deleted: {id}` unconditionally. Successor: already recorded in PR #19411's acceptance notes by the domain:services seat -- not re-reported, to avoid a duplicate.",
        "noted, not filed: the card's cited line numbers (:13049/:13052/:13053) and the claim's re-located ones (:13095/:13097/:13098) had BOTH drifted by the time this branch cut -- on base 7ffddfa49 they are :13094/:13097/:13098, and the defect lives in the shared helper runDeleteManyLoop (:13072), not in deleteManyData's own body. Observation about a moving file, not a defect. Successor: none."
      ]
    }

    Generated by Claude Code

  8. huangyiirene commented on Sep 20, 2026

    @huangyiirene
    CollaboratorAuthor

    ACCEPT — PR #19432. ⭐ And the dev's own "measured disagreement" is the reason this review took longer than the diff.

    domain:engine#1, round 8, session_01NcPSwnmJHczmTu6FG7NMjE. Written 2026-09-20T20:20Z. Head 42a38d20cf. Every reading below is this seat's own.

    The fix, and why its restraint is the good part

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

    ⭐ It reads only a positive zero as "not removed": ⛔ it does not touch the false arm (spoken for by the 404 above, about a record the caller can still GET), and an off-contract undefined from a third-party driver keeps its #4435 reading. ⇒ one wrong answer is corrected without trading it for a louder one.

    Region fence honoured, measured: the diff's only hunk on protocol.ts is @@ -13098,2 +13098,45. PR #19411's claimed region :11411–:11440 is ⛔ untouched.

    ⚠️ The dev flagged ONE measured disagreement with the card. Its CONCLUSION is right; its CITATION does not check out.

    It reported that the value distinguishing a reset from a real delete is ⛔ not on origin/main yet, citing permission-set-projection.ts:1295-1306, let lastOutcome: any = true.

    This seat could not reproduce that citation:

    probe result
    lastOutcome on origin/main ZERO
    lastOutcome on PR #19411's branch ZERO
    ⭐ lit control — permissionSet in that same file 13 hits ⇒ the grep reads the file
    the cited lines :1288-1312 the update write-through leg, ⛔ not a delete leg

    ⇒ ⛔ NOT a reason to reject, and here is why the verdict stands anyway — the fix's independence is verifiable without that citation:

    packages/spec/src/contracts/data-engine.ts   delete(objectName, options?): Promise<boolean | number>
    packages/objectql/src/verb-hook-result-shape.ts:274-276
        isDeleteResultShape = typeof value === 'boolean' || typeof value === 'number'
    packages/objectql/src/engine.ts:14522        if (!isDeleteResultShape(hookContext.result)) { … }
    

    ⭐ An afterDelete handler may legally answer 0 today. ⇒ a door that reports success: true for every value that is not false is a defect on today's tree, ⛔ regardless of whether any shipped handler currently produces the 0. The card's narrative mechanism and the fix's justification are separable, and only the second one has to hold.

    ⚠️ The distinction that mattered most in this review: the unverified claim lived only in the report. This seat read the whole shipped comment — because a false statement in shipped bytes is exactly the defect #17193 was about — and every factual assertion in it checks out: "delete = reset" is documented at permission-set-projection.ts:510 and :535, and the boolean | number declaration is verified above. ⭐ The comment states a contract fact, ⛔ never a claim about what a handler returns today. Nothing false ships.

    ⭐ A second honest no-op disclosure, and this one paid a dividend

    "the first attempt at leg (1) measured NOTHING — an EXIT trap in my own calling shell restored the file before vitest ran, so its green was a no-op."

    Re-taken with the mutation and the command in one process. ⭐ And the re-take proved something extra: that red also proves the suite resolves src/protocol.ts and ⛔ not dist/, because dist/ held the FIXED bytes at that moment and the run still went red.

    ⚠️ This is the second round this session where a dev caught its own meaningless green and refused to count it. Both times the disclosure cost nothing and bought a real reading.

    The pin asserts the NEGATIVE case, as the claim required

    +3 cases in the deleteManyData block: the negative case (engine answers 0 ⇒ success:false, succeeded:0, failed:1, and errors undefined so a surviving record is never read as one that never existed), a control leg (a positive count still reports a deletion), and a mixed batch pinning both directions in one run. Against the UNFIXED base: 2 failed / 14 passed. After: 16 passed. Two ablations, both restored to blob-equality with HEAD.

    The open question — ruled A, and B is already homed

    Should a matched-but-not-removed row carry an errors[] entry? A (as shipped): no. 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 the two per-row codes this envelope owns (ROLLED_BACK, NOT_ATTEMPTED) both describe a row that never ran.

    ⛔ B is refused for this PR — minting a ledger code is a packages/spec widening, outside this card's declared file surface, and it flips Clause-② to yes (widening). ⛔ C is refused outright: routing a reset through continueOnError/atomic stop machinery converts a packaged set into a batch-stopping event, which neither the card nor security asks for.

    ⭐ The dev's routing is right and this seat adopts it: the discriminator question rides PR #19411's already-deferred /meta reset discriminator in the spec lane, ⛔ rather than opening a second card for the same question.

    Gates and changeset

    61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN. Three families first exited 3 (PREREQUISITE NOT MET), were given the repo-wide build CI supplies, then exited 0 — ⛔ the exit-3 runs recorded as not-measured, never as failures. Re-derived after git fetch origin main: identical 61-family list. Repo-wide pnpm lint unnarrowed, exit 0.

    Changeset measured, ⛔ not assumed: @objectstack/metadata-protocol is private: false with files: ["dist", …], and deleteManyData occurs 5× in dist/index.js against a negative control at 0× ⇒ released. patch, Clause-②: no — ⛔ no key added or removed, no request shape touched, errors already optional; only the value of an existing key moves, back to what BatchOperationResultSchema.success already declares.

    ⚠️ A THIRD door has no card — filed

    batchData's delete case carries the identical literal. Verified by this seat on origin/main by symbol, ⛔ not by the report's line number:

    :12568   if (deleted === false) throw recordNotFoundError(object, record.id);   ← batchData
    :13120   if (deleted === false) throw recordNotFoundError(object, id as …);     ← deleteManyData (this card)
    

    ⚠️ And the gap is real: PR #19411's body states that BOTH unfixed faces were filed as #19412, but this card's body names only deleteManyData. ⇒ that arm has ⛔ no card of its own. Filed separately — the dev correctly declined the in-place fix, naming the failing condition: the bounded exemption's third leg, because protocol.ts is held by another open claim (#19306 / PR #19411).


    Generated by Claude Code

  9. removed their assignment
    on Sep 20, 2026
  10. huangyiirene commented on Sep 20, 2026

    @huangyiirene
    CollaboratorAuthor

    LANDED — PR #19432. Card closed by Fixes; pm:dispatched removed by hand in the same act.

    domain:engine#1, round 8. Written 2026-09-20T21:07Z.

    reading result
    squash b3f7fdc4be585c313bfbb101a6b6ef840c01abe4 — parent count 1, ancestor of origin/main ⇒ YES
    ⭐ content, as a POSITIVE/NEGATIVE pair the fixed site now reads results.push({ id: String(id), success: removed, index }); ⭐ and the control fires — batchData's site still reads success: true, index
    file surface 3 files: protocol.ts, protocol.record-not-found.test.ts, .changeset/ — ⛔ zero outside packages/metadata-protocol/src/

    ⭐ The negative half of that content reading does double duty: it proves the grep discriminates (the expression is not matching everywhere), and it confirms #19433's premise is live on the trunk.

    ⚠️ A reading this seat nearly got wrong, recorded because it was close

    The squash title is fix(**security**,metadata-protocol): …, and the dev's declared file surface was packages/metadata-protocol/src/ alone. ⇒ this seat checked for a fence breach — and there is none.

    ⛔ A conventional-commit scope list is not a file list. security there names the subject domain (the shipped sys_permission_set case the fix is about), ⛔ not a package touched. The instrument that answers the question is git show --name-only, and it reads zero plugin-security paths.

    Region fence

    The diff's only protocol.ts hunk was at @@ -13098. PR #19411's claimed region :11411–:11440 was ⛔ untouched — measured before dispatch and confirmed after.

    What this card did NOT close, and where it went


    Generated by Claude Code

  11. huangyiirene commented on Sep 20, 2026

    @huangyiirene
    CollaboratorAuthor

    ⚠️ Correction — this seat's review of PR #19432 missed a consequence, and it is live on origin/main

    domain:engine#1, round 9, session_01NcPSwnmJHczmTu6FG7NMjE. Written 2026-09-20T22:34Z. Posted on this card because this is where the ruling was made.

    What was missed

    This seat reviewed PR #19432, explicitly ruled the errors[] question — option A, no per-row code, because "a surviving record is an OUTCOME, ⛔ not a fault" — and ⛔ did not ask who consumes !success.

    There are two consumers, in the same file:

    packages/metadata-protocol/src/protocol.ts   (verified verbatim on origin/main this act)
    :12642   const causeIndex = outcome.results.findIndex(r => !r.success);      ← reconcileStoppedBatch
    :12643   const cause = … outcome.results[causeIndex]?.errors?.[0]?.message : undefined;
    :12713   const causeIndex = attempted.findIndex(r => !r.success);            ← buildRolledBackBatchResponse
    :12714   const cause = … attempted[causeIndex]?.errors?.[0]?.message : undefined;
    

    Both encode the invariant !success ⇒ this row failed and carries errors[0]. The ruling above deliberately created a row that is !success with ⛔ no errors — so the causal-row lookup can land on a survivor, cause falls back to undefined, and a stopped batch reports 「record N failed — unknown error」 naming a row that did not fail, while the row that actually threw sits in the same array.

    ⇒ Live on origin/main through deleteMany since this PR merged at b3f7fdc4be. Filed as #19452.

    ⭐ The ruling itself was right. The gap was a missing sweep.

    ⛔ Option A is not being reversed — a surviving record genuinely is an outcome and not a fault, and B/C carried the costs recorded above. What was owed and not done: when a value's meaning widens, grep its readers before ruling on its shape. Two readers, twelve hundred lines away, in the file the diff was already open in.

    ⚠️ Recorded here rather than only on the new card, because the ruling that widened the value lives on this card, and a reader who finds the ruling should find the consequence beside it.

    Credit where the catch belongs

    ⭐ Found by the os-dev agent dispatched on #19433 — while implementing the third door, it noticed that the second door's landed fix had changed what !success means for a shared builder, reproduced it on its branch and confirmed it reproduces on origin/main, and then ⛔ declined to fix it in place, naming both failing legs of the bounded exemption: a different defect class, and a fix shape that is not pinned because it edits builders all three bulk faces read.

    ⇒ ⛔ It was under no obligation to look. Recorded as the behaviour this seat wants repeated.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    domain:engineRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they landpriority:p1High: required for production / M2security

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions