P0: non-atomic batch failure has no normative behavior (forbid partial success + REQUIRE statementIndex on non-atomic + state that preceding statements persist)

[ssilvius]

Severity: P0. Reviewed at 82b715c. Three clauses, filed together because clause 1 alone leaves a client strictly worse off than the status quo.

The hole

A non-atomic batch that fails partway through has no normative behavior anywhere in SPEC.md.

• S6.2 (SPEC.md:139) covers only the atomic case: "If an atomic batch fails partway through, the response is the error envelope in section 7, not a partial results array."
• The only statement of the non-atomic rule lives at conformance/README.md:51 (B-4), which grants servers a permission SPEC.md never grants.
• SPEC.md:13 says the spec prose is the normative content. The conformance doc is inventing normative content in a document with no authority to make it.

A client written against SPEC.md alone cannot know what a failed non-atomic batch returns.

Clause 1 -- forbid partial-success responses for non-atomic batches

Both reference servers already fail closed (worker runBatch and DO runBatch both throw out of the non-atomic loop rather than returning the partial out array), so this codifies shipped behavior. Add to S6.2 alongside the existing atomic sentence.

The alternative considered and rejected: a result | error union per statement. Forbid is cheaper and matches both reference servers, but only if clauses 2 and 3 ride with it -- see below.

Clause 2 -- error.statementIndex REQUIRED for non-atomic batch errors

Today error.statementIndex is OPTIONAL (SPEC.md:157). Fail-closed plus an optional index means the client learns that it failed and cannot learn how far it got. That combination is worse than the union.

Scope this to the non-atomic path only. Verified against both reference servers at 82b715c:

• examples/cloudflare-worker-to-d1/src/index.ts -- the atomic branch is db.batch(prepared) and throws bare. Only the non-atomic for loop attaches (wrapped as { statementIndex?: number }).statementIndex = i.
• examples/cloudflare-durable-object/src/index.ts -- same shape. ctx.storage.transactionSync(...) throws bare; only the non-atomic loop stamps the index.
• Both outer catches are conditional (if (statementIndex !== undefined) error.statementIndex = statementIndex), so on an atomic failure the key is simply absent.

A blanket "REQUIRED for batch errors" would make both reference servers non-conforming the day it lands, and D1 batch failure does not surface a per-statement index to attach. Non-atomic is also the only case where the client needs it: on an atomic failure nothing persisted, so there is nothing to locate.

Scoping credit: smugglr, from the applier seat.

Considered and not filed: a SHOULD-emit on the atomic path for diagnostics. Zero consumers need it, D1 cannot satisfy it, and an unsatisfiable SHOULD is surface for its own sake.

Clause 3 -- state that preceding statements remain committed

This is the sentence nobody has, and it is the one a migration applier requires. Proposed text, verbatim from the consumer that needs it:

In a non-atomic batch, statements preceding the failing statement have been executed and their effects persist. Clients MUST NOT treat a non-atomic batch error as no-statements-applied.

Why it is load-bearing: in conformance/README.md, B-4's "fail closed" sits directly under B-3's "no rows persisted." Read together they imply nothing-applied. That is false, and it is exactly the false belief that corrupts a migration ledger on replay -- a client that re-drives a partially-applied batch believing nothing landed will double-apply every statement before the failure point.

Acceptance

• S6.2 states the non-atomic failure rule normatively (clause 1).
• S7 marks error.statementIndex REQUIRED for non-atomic batch errors, OPTIONAL otherwise.
• S6.2 (or S4.2) carries the clause-3 sentence.
• conformance/README.md B-4 is rewritten to test the spec rule rather than grant a permission; the partial-success permission is removed.
• Both reference servers still conform with no code change.

Origin: codex spec review, verified against source by platform and smugglr. Related: the "when not rejected" qualifier at SPEC.md:206 points at a rejection mechanism that does not exist -- filed separately.

[ssilvius]

Cross-reference: the "when not rejected" gap mentioned at the bottom of this issue is filed as #3.

The two are related but separable. #3 is about a server that cannot execute atomically having no conforming way to say so (a MUST qualified by a rejection mechanism that does not exist). This issue is about what happens on the NON-atomic path, which is unspecified regardless of how #3 resolves.

Full cluster from the v0.1 review: #1 #2 (P0), #3 #4 #5 (P1), #6 #7 (P2), #8 (positioning decision for @sean, edits deliberately unmade).

[ssilvius]

SCOPE AMENDMENT, recorded here rather than by rewriting the criteria above, deliberately: the five acceptance criteria are left byte-identical because the legion pr write-check gate on PR #9 validated a criterion-by-criterion mapping against them. Editing them after the gate passed would make the gate record a claim about criteria that no longer exist.

ONE CRITERION IS NARROWER THAN IT READS. "Both reference servers still conform with no code change" holds exactly as written, and I verified it before writing any spec text: both runBatch implementations throw out of the non-atomic loop and discard the accumulated results, so neither has ever returned 200-with-partial.

But it covers the SERVERS only. examples/reference-client.ts DOES change in PR #9, and it has to: S10.2 gains a client MUST (treat statements before error.statementIndex as applied) and a client that discards how far the batch got cannot satisfy it. The client now carries appliedCount, where undefined means unknown and never zero -- a caller that reads zero replays a batch that partly applied, which is the failure mode clause 3 exists to prevent.

ALSO NOT IN THE CRITERIA, and in the PR: conformance/README.md gains B-5 (a three-statement non-atomic batch, verifying the statement AFTER the failure did not execute), and S10.1/S10.2 each gain one conformance item. S10 is by construction a restatement of S2-S9, so a new normative obligation absent from the checklist is a gap in the checklist.

AND ONE THING THIS ISSUE GOT RIGHT THAT I THEN GOT WRONG. The body above records the atomic-path SHOULD as considered-and-rejected ("an unsatisfiable SHOULD is surface for its own sake"). I implemented one anyway, from a reflection that predated this card, and reverted it in e86d06c. My qualifier -- "SHOULD include it where the underlying engine surfaces it" -- was a normative keyword gated on a condition the spec does not define, which is issue #3s exact defect reproduced inside the fix for #1. The card was right; I did not re-read it before implementing it.