feat: classify accepted-blocking refusal eligibility

[Kiran01bm]

Add a closed typed registry for accepted-blocking eligibility while preserving the current refusal-only behavior.

Why

Future bounded blocking execution needs a narrow, reviewable admission decision that cannot depend on SQL or rendered explanation text.

What

• Add typed cause and refusal-site discriminators to refusal proofs.
• Admit only one-relation index maintenance and blocking partitioned-parent index builds.
• Distinguish single- and multi-relation index forms at the parse-only gate.
• Attach the partitioned-parent cause once, in plan.PartitionRefusal, so both front doors carry the same typed cause (constants pinned to preflight's by test).
• Add completeness, eligibility, statement-shape, and rendering-independence tests.

Before / after

Before
┌─────────┐   ┌─────────┐
│ refusal │──▶│ exit 2  │
└─────────┘   └─────────┘

After
┌─────────┐   ┌─────────────────────────────┐   ┌────────────────────────┐
│ refusal │──▶│ typed eligibility decision  │──▶│ exit 2; nothing executes│
└─────────┘   │ recorded                    │   └────────────────────────┘
              └─────────────────────────────┘

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 1/2 — the eligibility registry. The proof plumbing, RF-8, and the docs are in 2/2.

Reviewed 2c2dfc69..81daca74 — 12 files, +347/−38, two commits. This is the decision that will later admit a refused statement to blocking execution, so I did not read the switch — I mutated it. Fourteen mutations across ./pkg/{verdict,statement,plan,migrate}/..., base green.

Eight caught, and the parse side is genuinely well pinned.

Mutation	Result
DROP INDEX multi-object check removed (always single-relation)	caught — TestIndexStatementAcceptedBlockingEligibility/drop_multiple_indexes, TestGateVerdictAcceptedBlockingEligibility/multiple_relations
REINDEX SCHEMA/DATABASE/SYSTEM treated as single-relation	caught — TestIndexStatementAcceptedBlockingEligibility/{reindex_schema,reindex_database,reindex_system}
empty site (decoded verdict) becomes eligible	caught — TestGateVerdictAcceptedBlockingEligibility/single_relation
gateRefusal stops attaching the site	caught — 4 tests incl. TestAcceptedBlockingEligibleRowsArePinned
PartitionRefusal drops the blocking-build cause	caught — TestPartitionRefusalCarriesItsCause, TestRefusalRegistryIsComplete/partition:parent-blocking-index-build
WithRefusal stops storing the proof	caught — TestRefusalRoundTripsThroughVerdict + 3
WithRefusal stops copying cause onto the verdict	caught — TestBudgetVerdict/*, TestExecRefusal
Refusal() proof fast-path removed	caught — TestAcceptedBlockingEligibilityIgnoresRenderedText + 3
the three ineligible parent causes become eligible	survived
ReasonIndexStatement class check dropped	survived
parent-cause class check dropped	survived
unknown reason becomes decided	survived
unknown site becomes decided	survived
IndexTargets() drops the Other shape	survived

Deriving the single-relation fact from len(GetObjects()) and the REINDEX object type rather than from the rendered SQL is the right call, and the default: IndexTargetOther arm means a future ReindexObjectType lands ineligible without anyone remembering to think about it. TestIndexStatementAcceptedBlockingEligibility driving real ParseOne input rather than hand-built Statement literals is what makes that checkable end to end — it is the test I would have asked for.

---

1. The three parent causes that must stay ineligible are not pinned, and they are the dangerous half of the row

refusal_registry.go:233-235:

case verdict.CauseParentConcurrentIndexBuild, verdict.CauseParentIndexAdoption,
	verdict.CauseParentNotValidForeignKey:
	return false, true

Change false to true and the entire suite stays green. TestAcceptedBlockingEligibleRowsArePinned:184 pins the two eligible rows and two unrelated reasons (rewriteRequiredRefusal, backendUnavailableRefusal), so the four unsupported-partitioned-parent causes are covered only for the one that is eligible. The other three — the ones where eligibility is the wrong answer — are asserted nowhere.

That asymmetry is backwards for a registry whose thesis is that admitting a row is the consequential direction. Concretely, for parent-concurrent-index-build: preflight/partition.go:16 defines it as a step attempts a concurrent index build on the parent, and plan/refusal.go classifies it CapabilityBoundary because "the partition-aware concurrent index flow is a planned capability." Admitting that row to accepted-blocking would take a statement the user wrote as CONCURRENTLY and run it through the blocking path under an acknowledgement the user gave about a different statement — an ACCESS EXCLUSIVE hold on a partitioned parent, which is precisely what RF-6 exists to forbid and what parent-blocking-index-build is separately enumerated for. The distinction between those two causes is the entire reason PartitionRefusal was split into four arms in this very diff; nothing tests that the split is load-bearing.

parent-index-adoption and parent-not-valid-foreign-key are milder (the server refuses them outright) but belong in the same table for the same reason.

Extending TestAcceptedBlockingEligibleRowsArePinned into a table over all of preflight.PartitionRefusalCauses() with an expected boolean per cause is a handful of lines, mirrors TestPartitionRefusalCarriesItsCause right next door, and turns "only one of four is eligible" from a fact about today's switch into a pinned property. The same shape covers statement.Kinds() × IndexTargets().

2. Both fail-closed defaults are unenforced, which is the property the registry is named for

refusal_registry.go:216-217 promises "completeness tests reject undecided additions," and :208-209 promises "Unknown combinations fail closed." Two mutations say neither is checked:

• :245-246 (default: return false, false over reasons) → return false, true: green.
• :226-227 (default: return false, false over sites) → return false, true: green.

Eligibility itself still fails closed either way, since AcceptedBlockingEligible:211 requires decided && eligible — so this is not a safety hole today. What breaks is the gate that is supposed to catch the next change. TestRefusalRegistryIsComplete:118 asserts decided == true for every registered key; with the default flipped, a newly added reason or site satisfies that assertion without anyone deciding anything, and the row silently becomes a permanent (false, true). The test proves the registry covers what exists; nothing proves it notices what does not.

The missing direction is one assertion: a synthetic reason and a synthetic site outside the vocabulary must come back decided == false. I verified both do today (acceptedBlockingDecision on ByDesign(ReasonIndexStatement).WithSite("brand-new-site") returns eligible=false decided=false) — the behavior is right, it just has no test holding it there. Given that decided exists solely to make this check possible, it is worth the two lines.

3. IndexTargets() drops out of the completeness walk without noticing

statement.go:112 returns {SingleRelation, Other} and deriveRefusalKeys uses it to fan the gate keys out. Truncating it to {SingleRelation} leaves the suite green: the multi-relation gate key stops being walked by TestRefusalRegistryIsComplete, and the only thing still covering it is the parse-driven table in TestIndexStatementAcceptedBlockingEligibility.

That is a real backstop, so this is the mildest of the three — but the whole point of a …s() enumerator in this codebase is that the completeness test fails when someone adds a value and forgets the registry, and here it also has to fail when someone removes one. assert.Len(t, statement.IndexTargets(), 2) is the usual answer; better is asserting it covers every non-None value, which also catches an addition.

---

One behavior worth deciding rather than noting

AcceptedBlockingEligible has no callers yet — step 2 builds them — so everything above is about the shape the executor will inherit rather than a live defect. That makes now the cheap moment for the parent-cause table in finding 1: once the executor keys off this function, "which of the four is eligible" becomes a question about blocking DDL on a partitioned parent rather than a question about a switch statement.

Rollout marker on lock-budgeted-passthrough.md:381 correctly flips step 1 to *(done)* and leaves step 2 unmarked, which matches what shipped here.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 2/2 — the refusal proof, RF-8, and RF-7. The registry itself is in 1/2.

Carrying the narrowing on the Refusal rather than re-deriving it downstream is the right move, and Verdict.WithRefusal staying the single door onto the verdict's refusal fields is what keeps RF-7 true while the proof gets richer. Two findings, both about what the new proof field changes that the invariant text does not yet say.

1. RF-8's fail-closed property covers one of the registry's two eligible rows

RF-8 (invariants.md:421) states it precisely — "eligibility decisions keyed by site therefore fail closed for decoded verdicts" — and the lock-budgeted-passthrough.md paragraph repeats the same qualifier. Both are literally true. The trouble is what a reader takes away from an entry whose first sentence is "An in-process refused verdict retains its full typed refusal proof": that a decoded verdict cannot be eligible. Half the registry says otherwise, and it is the half that matters more.

I ran both eligible rows through a real encode/decode:

Eligible row	Keyed by	In-process	After json.Marshal → Unmarshal → Refusal()
index-statement / single-relation	site (unexported)	eligible	ineligible — site is ""
unsupported-partitioned-parent / parent-blocking-index-build	cause (json:"cause")	eligible	still eligible

And because Cause is an exported JSON field, the second row is reconstructible from hand-written JSON with no pg-sprite code in the loop at all — {"outcome":"refused","reason":"unsupported-partitioned-parent","class":"capability-boundary","cause":"parent-blocking-index-build"} decodes to an eligible refusal. So the two rows have materially different trust properties: one is unforgeable because its key never leaves the process, the other is whatever the JSON says.

Whether that is acceptable depends on a decision this PR does not have to make — whether the step-2 executor ever takes eligibility from a decoded verdict, or only from one it produced in-process. But the entry should say which, because the next reader will key that decision off RF-8 and the current wording reads as a blanket guarantee. Two candidate shapes:

• If decoded verdicts are never an eligibility input, say so in the rule — eligibility is decided only on an in-process proof — which is a stronger and simpler invariant than the site/cause split, and makes the unexported field a mechanism rather than the rule.
• If they are, then the rule is that the fail-closed guarantee is partial, and RF-8 should name the cause-keyed rows as the exception rather than leaving them to be discovered.

The current text is the one shape that reads as a guarantee while being a description of two different behaviors. Worth noting this also interacts with the registry's own framing at refusal_registry.go:208-209 ("Unknown combinations fail closed") — true of unknown combinations, and the cause-keyed row is a known one.

2. The proof fast-path bypasses the validation Refusal() still documents, and that is RF-7's enforcement point

verdict.go:326 returns v.proof before reaching NewRefusal. The doc comment three lines above (:315-316) is unchanged and still says:

It fails on a verdict that is not refused, or whose reason, class, and owner do not validate together — a verdict this build cannot have produced.

For an in-process verdict that second clause no longer holds, and the new WithSite / WithCause methods make it reachable in one call:

bad := Refusal{}.WithSite(RefusalSiteIndexSingleRelation)   // never through NewRefusal
v := Verdict{}.WithRefusal(bad)
got, err := v.Refusal()
// err = <nil>, got.Class() = "", got.Reason() = ""

Before this PR the same fields produced unknown refusal reason "" class="". IsZero() does not catch it because WithSite is what makes the struct non-zero, so any Refusal{}.WithX(...) is a validated-looking proof that was never validated.

This lands on RF-7 rather than RF-8. RF-7's Enforced: line says "verdict.NewRefusal and the per-class constructors are the only way to build a verdict.Refusal" — WithCause/WithSite are new ways to obtain one that skip both, and Refusal() is where that used to be caught on the way out. The entry's text is unaffected (every refusal the production paths build still carries exactly one class and the right owner), so the disposition is upholds — but the enforcement sentence is now describing a door that has a second key.

Cheapest fix keeps the fast path and re-validates it:

if !v.proof.IsZero() {
	if _, err := NewRefusal(v.proof.class, v.proof.reason, v.proof.owner); err != nil {
		return Refusal{}, err
	}
	return v.proof, nil
}

That restores the doc comment's claim, keeps RF-7's enforcement sentence accurate without editing it, and costs one validation on a path that runs once per refusal. The alternative — making WithCause/WithSite no-ops on a zero receiver — is cheaper at runtime but leaves a silently-dropped narrowing, which is worse.

A related latent case in the same area: Refusal() now answers from the proof while JSON renders v.Class / v.Reason, so a verdict whose exported fields are mutated after WithRefusal gives two different answers with no error. Nothing does that today, and this diff actually moves away from it by deleting the post-WithRefusal v.Cause = assignments in budgetVerdict. Re-validating above does not catch this one (the stale proof is internally valid); if it is worth closing, comparing the proof against the exported fields is the check, and it is probably only worth it if the executor ends up trusting one over the other.

Smaller notes

• RF-8's *Enforced:* line names "the accepted-blocking eligibility gate-path tests" without naming one. Every other entry in the family points at a specific symbol or test (RF-7 names TestRefusalRegistryIsComplete), and a pointer that resolves by description is the kind that rots invisibly. TestGateVerdictAcceptedBlockingEligibility is the test that actually holds the decoded-verdict half — it is the one to name.
• The proof field changes Verdict equality semantics. It is comparable so nothing breaks, and nothing currently compares whole Verdict values, but a future reflect.DeepEqual between a produced verdict and a JSON round-trip of itself will now differ on a field neither side can see. The color_test.go:185 PkgPath != "" skip is the right adaptation for the all-fields fixture; worth a line on the field itself saying it is deliberately outside the JSON contract and outside value equality.
• IndexTarget.String()'s default returns IndexTarget(%d) rather than a stable token, which is right for a value that should never be rendered — consistent with how the other typed vocabularies here behave.
• The lock-budgeted-passthrough.md paragraph and the Refusal() doc comment say the same three sentences. That is fine and probably deliberate — but if the RF-8 wording changes per finding 1, all three sites move together.

---

Approving. Finding 2 is the one I would fix before merge: it is a four-line change, and it restores a claim that RF-7 leans on. Finding 1 is a wording decision rather than a code change, but it should be made now rather than by whoever writes the executor.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Approving on 81daca74. Fourteen mutations, eight caught — the parse side (len(GetObjects()), the REINDEX object type, the default: IndexTargetOther arm) is well pinned, and driving the table through real ParseOne input is what makes it checkable end to end. Six survived, detailed in 1/2 and 2/2. The two I would act on: the three unsupported-partitioned-parent causes that must stay ineligible are asserted nowhere, so flipping them to eligible ships green; and Refusal()'s proof fast-path now skips the validation its own doc comment still promises, which is reachable in one call via the new WithSite/WithCause and is the point where RF-7's "NewRefusal is the only way to build a Refusal" was enforced. Also worth deciding now rather than in step 2: RF-8's fail-closed guarantee holds for the site-keyed eligible row but not the cause-keyed one, which survives JSON decoding and is reconstructible from hand-written JSON.

This stamp was left by Claude Code (claude-opus-5).

[Kiran01bm]

🤖 Adversarial review response — created by Kiran's code review agent (Amp, Claude Opus 4.6) — block/pg-sprite pull/105, follow-up commit

Verdict: all five findings fixed in one follow-up commit; the RF-8 wording decision and the proof-divergence check are the two calls to veto.

#	Concern	Status	Explanation
C1-F1	Only the eligible rows are pinned; the three parent causes that must stay ineligible are not.	fixed	TestAcceptedBlockingEligibleRowsArePinned now walks every key deriveRefusalKeys() produces (the same production walk the RF-7 completeness harness uses) and asserts the eligible set is exactly gate:DROP INDEX/concurrent=false/target=single-relation, gate:REINDEX/concurrent=false/target=single-relation, and partition:parent-blocking-index-build. A second loop iterates preflight.PartitionRefusalCauses() and asserts eligible iff the cause is PartitionCauseBlockingIndexBuild. Flipping the three sibling causes to eligible now fails the test.
C1-F2	Both fail-closed default arms are unenforced.	fixed	New TestAcceptedBlockingUnknownKeysFailClosed builds refusals through the per-class constructors (which do not validate their reason) with an unknown reason, an unknown site under index-statement, and an unknown or absent cause under unsupported-partitioned-parent, and asserts each is ineligible and undecided. New TestAcceptedBlockingRowsRequireTheirClass pins that the same site or cause under a different class is decided-ineligible, which catches dropping either r.Class() == check. Each of the three return false, false → return false, true mutations and both class-check mutations now fail a test.
C1-F3	IndexTargets() can drop a value without the completeness walk noticing.	fixed	TestIndexTargetsIsClosedAndNamed in pkg/statement mirrors TestKindsIsClosedAndNamed: it parses the IndexTargetNone iota block from statement.go and asserts IndexTargets() enumerates exactly every declared constant except IndexTargetNone, with a distinct non-fallback String() for each. declaredKinds was generalised into declaredIotaValues(t, anchor) so both tests share the parser. Truncating IndexTargets() to one value now fails this test and the pinned-set test in C1-F1.
C2-F1	RF-8's fail-closed claim covers only the site-keyed row; the cause-keyed row survives JSON decoding.	fixed	Decision taken: describe the shipped boundary rather than move the mechanism. RF-8, the passthrough paragraph, and the Refusal() doc comment now say that the site-keyed row fails closed after decoding while the cause-keyed row is decidable from the wire fields, and that eligibility is consumed only from the proof of the verdict the same migrate/diff invocation produced, never from a decoded verdict. This matches the design's decision rows 2 and 6 and the front-door sequence at lock-budgeted-passthrough.md ("gate refuses, registry says eligible, flag present, dial"), where a decoded verdict is never an input. All three sites changed together.
C2-F2	The proof fast path returns v.proof without the RF-7 validation the doc comment promises.	fixed	Refusal() now routes a non-zero proof through provenRefusal(), which re-validates via NewRefusal(proof.class, proof.reason, proof.owner) and additionally errors when the verdict's exported Class/Reason/Owner/Cause diverge from the proof (refusalFieldsDivergeFromProof, a named helper for the four-term predicate). TestRefusalRejectsUnvalidatedProof covers Refusal{}.WithSite(...), Refusal{}.WithCause(...), ByDesign("brand-new-reason"), and an owner on a class that carries none; TestRefusalRejectsFieldsDivergingFromProof mutates each exported field after WithRefusal. The doc comment was corrected to describe what the method now does.
C2 note	RF-8 Enforced: names "the gate-path tests" without naming one.	fixed	RF-8 now names TestRefusalRejectsUnvalidatedProof, TestRefusalRejectsFieldsDivergingFromProof, and TestGateVerdictAcceptedBlockingEligibility (with what the last one pins).
C2 note	The proof field silently changes Verdict equality; worth a line on the field.	fixed	The field comment now states it is outside the JSON contract, does not survive decoding, that a decoded verdict therefore never compares equal to its in-process source, and that Refusal() is its only reader.
C2 note	IndexTarget.String() falls back to IndexTarget(%d).	no action	Agreed as consistent with the other typed vocabularies; the new closed-set test asserts no enumerated value reaches that fallback.
C1 closing	Decide the parent-cause table now rather than when the executor lands.	fixed	The four-cause table is now pinned in code (C1-F1) and the design table already documents the decision per cause; no wording change was needed there.

Decisions to veto

• RF-8 wording: I kept the site as the mechanism and stated the partial fail-closed property honestly (site row fails closed on decode, cause row does not) with the rule that eligibility is consumed only from the in-process proof. The alternative, making the cause-keyed row also fail closed after decoding, would require a wire-invisible marker on Refusal and was not taken.
• Proof-divergence check: Refusal() now errors when a verdict's exported refusal fields differ from its proof. No production code mutates those fields after WithRefusal (the earlier budgetVerdict post-assignments were removed in this PR), so this is a pure hardening of the step-2 seam; drop refusalFieldsDivergeFromProof if you would rather keep Refusal() to RF-7 validation only.

Verification

gofmt, make lint (0 issues), go vet, and SKIP_INTEGRATION=1 go test ./pkg/... all pass. Ten mutations were applied one at a time and each was caught: flip parent causes eligible; drop the index-statement class check; drop the parent class check; make the reason, site, and cause default arms decided; truncate IndexTargets(); return v.proof unvalidated; skip the divergence check; drop Cause from the divergence predicate. Verdict JSON and exit codes are unchanged, so make demo-check was not needed.

Source: block/pg-sprite#105, review comments 5629181195 and 5629181647 and review 5174802719 at head 81daca74