Skip to content

A stopped or rolled-back bulk batch now names a causal row that did not fail, and calls the real error "unknown" — live on main since #19432 #19452

Description

@huangyiirene

What is wrong

A stopped or rolled-back bulk batch names a causal row that did not fail, and reports its error as 「unknown error」 while the row that actually threw is sitting in the same array.

⚠️ This is LIVE on origin/main today, reachable through deleteMany since PR #19432 merged at b3f7fdc4be.

The mechanism, verified verbatim on origin/main

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

Both builders encode one invariant: !success ⇒ this row failed, and it carries errors[0].

PR #19432 (card #19412) broke that invariant on purpose and correctly: a row that MATCHED but was deliberately NOT removed now answers success: false with ⛔ no errors entry — because a surviving record is an outcome, ⛔ not a fault.

findIndex(r => !r.success) can now land on that survivor, errors?.[0]?.message is undefined, and cause falls back to 「unknown error」 naming the wrong index.

Reproduced (by the os-dev agent on #19433, on its branch, and it reproduces identically on origin/main)

  • Non-atomic ['survivor', 'missing', 'other'] delete batch ⇒ row 2 answers NOT_ATTEMPTED, message 「record 0 failed — unknown error; the batch stopped there.」 ⚠️ Row 1 is what threw.
  • Atomic batch ⇒ a rolled-back row answers ROLLED_BACK 「record 1 failed — unknown error」 for a row that survived rather than failed.

⚠️ This is the reviewing seat's miss, and it is recorded as such

The domain:engine PM seat reviewed PR #19432, explicitly ruled the errors[] question (option A — no per-row code, because a surviving record is an outcome), and ⛔ did not ask who consumes !success.

⭐ The ruling was right. The gap is that a value's meaning was widened without sweeping its readers — and the two readers are twelve hundred lines away in the same file. ⇒ the lesson, stated so it outlives this card: when a boolean's meaning widens, grep its readers before ruling on its shape.

Why it is a card and not a rider on #19433

The os-dev agent on #19433 found it, reproduced it, and ⛔ declined the in-place fix, naming both failing legs of the bounded exemption: a different defect class from its card, and a fix shape that is ⛔ not pinned — it edits builders that all three bulk faces read, adding a verification surface that card does not carry. ⭐ That refusal is correct.

Acceptance

  1. The causal row is located by something that means 「this row failed」, ⛔ not by !success. ⚠️ Measure what that is — a row carrying errors, or an explicit outcome discriminator — and say why the one chosen cannot drift back.
  2. ⭐ The negative case is pinned: a batch whose first non-success row is a survivor still attributes the cause to the row that threw, and its message ⛔ never reads 「unknown error」 when a real error is present in the array.
  3. Both builders are covered — ⛔ fixing one leaves the other live.
  4. ⚠️ All three bulk faces read these builders. State which are covered by the pin and which are reasoned about.

⚠️ Serial

packages/metadata-protocol/src/protocol.ts is a hot file — this family has held up to three concurrent claims. ⛔ Clear the region, not the file. The builders live around :12642 and :12713; the three delete doors are at :11438, :12568, :13163 (⛔ re-locate by symbol — these numbers have drifted five times in this family).

Dedupe words (⛔ the filer does not dedupe)

reconcileStoppedBatch causal row findIndex not success · NOT_ATTEMPTED message names a surviving row · ROLLED_BACK unknown error cause misattribution · batch stopped cause index wrong row · buildRolledBackBatchResponse findIndex surviving row

Provenance

Surfaced by the os-dev agent on #19433 (PR #19451) and independently verified on origin/main by the domain:engine execution PM seat — both findIndex(r => !r.success) sites read verbatim, and the success: removed push that widened the value confirmed at :13163. ⚠️ Introduced by PR #19432, which that same seat reviewed and carried to merge.


Generated by Claude Code

Activity

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions