From a109572bc844b4f4aca3651a8f681865515d5ba6 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Fri, 11 Sep 2026 11:13:10 +1000 Subject: [PATCH 1/3] feat: classify accepted-blocking refusal eligibility Keep future blocking execution fail-closed by admitting only typed refusal shapes whose lock risk can be meaningfully bounded. Distinguish one-relation index maintenance at the parse-only gate so broader forms remain ineligible. --- docs/lock-budgeted-passthrough.md | 2 +- pkg/migrate/refusal_registry.go | 50 +++++++++++++++- pkg/migrate/refusal_registry_test.go | 88 ++++++++++++++++++++++++---- pkg/migrate/verdicts.go | 2 +- pkg/plan/plan_test.go | 13 ++++ pkg/plan/refusal.go | 23 +++++--- pkg/statement/statement.go | 31 ++++++++++ pkg/statement/statement_test.go | 10 ++-- pkg/verdict/verdict.go | 44 ++++++++++++++ pkg/verdict/verdict_test.go | 6 +- 10 files changed, 240 insertions(+), 29 deletions(-) diff --git a/docs/lock-budgeted-passthrough.md b/docs/lock-budgeted-passthrough.md index 5763cb6..8507396 100644 --- a/docs/lock-budgeted-passthrough.md +++ b/docs/lock-budgeted-passthrough.md @@ -373,7 +373,7 @@ Sequence implementation as follows: cause or refusal site. The `index-statement` site distinguishes the single-relation forms from `DROP INDEX a, b` and `REINDEX SCHEMA`, `DATABASE`, and `SYSTEM`, so the registry can admit the former and refuse the latter without a database. Its completeness tests make new - values ineligible by default and prove that render text is never consulted. + values ineligible by default and prove that render text is never consulted. *(done)* 2. Add the executor path through engine-owned bounded sessions, requiring explicit non-zero `statement_timeout` and non-zero `lock_timeout`. At this step add lock-budget invariants to [the invariant registry](invariants.md): every passthrough statement runs in an diff --git a/pkg/migrate/refusal_registry.go b/pkg/migrate/refusal_registry.go index 56046d2..53ea9fe 100644 --- a/pkg/migrate/refusal_registry.go +++ b/pkg/migrate/refusal_registry.go @@ -102,7 +102,7 @@ func siteRefusals() []siteRefusal { // (ALTER TABLE, CREATE INDEX). concurrent distinguishes the already-safe // maintenance forms — which pg-sprite need not wrap — from the plain forms // it refuses in favor of their concurrent idiom. -func gateRefusal(kind statement.Kind, concurrent bool) (verdict.Refusal, bool) { +func gateRefusal(kind statement.Kind, concurrent bool, target statement.IndexTarget) (verdict.Refusal, bool) { switch kind { case statement.KindAlterTable, statement.KindCreateIndex: return verdict.Refusal{}, false @@ -110,7 +110,11 @@ func gateRefusal(kind statement.Kind, concurrent bool) (verdict.Refusal, bool) { if concurrent { return verdict.NoOnlineSafetyProblem(verdict.ReasonIndexStatement, verdict.OwnerDirectOperator), true } - return verdict.ByDesign(verdict.ReasonIndexStatement), true + site := verdict.RefusalSiteIndexOther + if target == statement.IndexTargetSingleRelation { + site = verdict.RefusalSiteIndexSingleRelation + } + return verdict.ByDesign(verdict.ReasonIndexStatement).WithSite(site), true case statement.KindCreateTable: return verdict.NoOnlineSafetyProblem(verdict.ReasonUnsupportedStatement, verdict.OwnerDeclarativeFrontDoor), true case statement.KindDataChange: @@ -201,6 +205,48 @@ func partitionRefusal(cause preflight.PartitionRefusalCause) (verdict.Refusal, b return plan.PartitionRefusal(cause) } +// AcceptedBlockingEligible reports whether a typed refusal is in the closed +// accepted-blocking registry. Unknown combinations fail closed. +func AcceptedBlockingEligible(r verdict.Refusal) bool { + eligible, decided := acceptedBlockingDecision(r) + return decided && eligible +} + +// acceptedBlockingDecision is total over the registered reason and +// cause/site vocabulary. The bool pair is eligibility and whether the key +// has an explicit decision; completeness tests reject undecided additions. +func acceptedBlockingDecision(r verdict.Refusal) (bool, bool) { + switch r.Reason() { + case verdict.ReasonIndexStatement: + switch r.Site() { + case verdict.RefusalSiteIndexSingleRelation: + return r.Class() == verdict.ClassByDesign, true + case verdict.RefusalSiteIndexOther, "": + return false, true + default: + return false, false + } + case verdict.ReasonUnsupportedPartitionedParent: + switch r.Cause() { + case verdict.CauseParentBlockingIndexBuild: + return r.Class() == verdict.ClassCapabilityBoundary, true + case verdict.CauseParentConcurrentIndexBuild, verdict.CauseParentIndexAdoption, + verdict.CauseParentNotValidForeignKey: + return false, true + default: + return false, false + } + case verdict.ReasonUnsupportedStatement, verdict.ReasonTableTooLarge, + verdict.ReasonInsufficientPrivileges, verdict.ReasonBudgetExceeded, + verdict.ReasonRewriteRequired, verdict.ReasonBackendUnavailable, + verdict.ReasonDestructiveChange, verdict.ReasonPlanFingerprintMismatch, + verdict.ReasonCreateCollision: + return false, true + default: + return false, false + } +} + // isInSentinelSet reports whether err matches any sentinel in set. func isInSentinelSet(err error, set []error) bool { for _, sentinel := range set { diff --git a/pkg/migrate/refusal_registry_test.go b/pkg/migrate/refusal_registry_test.go index 24c4b03..5d5e74a 100644 --- a/pkg/migrate/refusal_registry_test.go +++ b/pkg/migrate/refusal_registry_test.go @@ -35,7 +35,7 @@ func deriveRefusalKeys() (keys []classifiedKey, admitted []string) { keys = append(keys, classifiedKey{"create-shape:" + string(c), r, ok}) } for _, c := range preflight.PartitionRefusalCauses() { - r, ok := plan.PartitionRefusal(c) + r, ok := partitionRefusal(c) keys = append(keys, classifiedKey{"partition:" + string(c), r, ok}) } for _, err := range admissionSentinels() { @@ -48,13 +48,22 @@ func deriveRefusalKeys() (keys []classifiedKey, admitted []string) { } for _, k := range statement.Kinds() { for _, concurrent := range []bool{false, true} { - r, ok := gateRefusal(k, concurrent) - key := fmt.Sprintf("gate:%s/concurrent=%t", k, concurrent) - if !ok { - admitted = append(admitted, key) - continue + targets := []statement.IndexTarget{statement.IndexTargetNone} + if !concurrent && (k == statement.KindDropIndex || k == statement.KindReindex) { + targets = []statement.IndexTarget{statement.IndexTargetSingleRelation, statement.IndexTargetOther} + } + for _, target := range targets { + r, ok := gateRefusal(k, concurrent, target) + key := fmt.Sprintf("gate:%s/concurrent=%t", k, concurrent) + if len(targets) > 1 { + key = fmt.Sprintf("%s/target=%d", key, target) + } + if !ok { + admitted = append(admitted, key) + continue + } + keys = append(keys, classifiedKey{key, r, ok}) } - keys = append(keys, classifiedKey{key, r, ok}) } } for _, s := range siteRefusals() { @@ -105,6 +114,9 @@ func TestRefusalRegistryIsComplete(t *testing.T) { // and owner rule. _, err := verdict.NewRefusal(k.refusal.Class(), k.refusal.Reason(), k.refusal.Owner()) require.NoError(t, err) + _, decided := acceptedBlockingDecision(k.refusal) + require.True(t, decided, "eligibility registry has no explicit decision for class=%q reason=%q cause=%q site=%q", + k.refusal.Class(), k.refusal.Reason(), k.refusal.Cause(), k.refusal.Site()) classified[k.refusal.Reason()] = true }) } @@ -126,15 +138,15 @@ func TestRefusalRegistryCorrespondence(t *testing.T) { assert.Equal(t, verdict.ClassByDesign, class(plan.CreateShapeRefusal(executor.CreateShapeIfNotExists))) assert.Equal(t, verdict.ClassCapabilityBoundary, class(plan.CreateShapeRefusal(executor.CreateShapePartitionOf))) assert.Equal(t, verdict.ClassInvariantViolation, class(plan.CreateShapeRefusal(executor.CreateShapeMultipleOperations))) - assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindDataChange, false))) + assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindDataChange, false, statement.IndexTargetNone))) // unsupported-partitioned-parent spans three. assert.Equal(t, verdict.ClassCapabilityBoundary, class(plan.PartitionRefusal(preflight.PartitionCauseConcurrentIndexBuild))) assert.Equal(t, verdict.ClassByDesign, class(plan.PartitionRefusal(preflight.PartitionCauseIndexAdoption))) assert.Equal(t, verdict.ClassEnvironmental, class(plan.PartitionRefusal(preflight.PartitionCauseNotValidForeignKey))) // index-statement: the plain form is refused by design, the concurrent // form is the operator's to run. - assert.Equal(t, verdict.ClassByDesign, class(gateRefusal(statement.KindDropIndex, false))) - assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindReindex, true))) + assert.Equal(t, verdict.ClassByDesign, class(gateRefusal(statement.KindDropIndex, false, statement.IndexTargetSingleRelation))) + assert.Equal(t, verdict.ClassNoOnlineSafetyProblem, class(gateRefusal(statement.KindReindex, true, statement.IndexTargetSingleRelation))) // Owners: each no-online-safety-problem kind names who runs it. for kind, owner := range map[statement.Kind]verdict.Owner{ statement.KindDataChange: verdict.OwnerDataChangeRunner, @@ -143,11 +155,11 @@ func TestRefusalRegistryCorrespondence(t *testing.T) { statement.KindCreateTable: verdict.OwnerDeclarativeFrontDoor, statement.KindDropIndex: verdict.OwnerDirectOperator, } { - r, ok := gateRefusal(kind, kind == statement.KindDropIndex) + r, ok := gateRefusal(kind, kind == statement.KindDropIndex, statement.IndexTargetSingleRelation) require.True(t, ok, kind) assert.Equal(t, owner, r.Owner(), kind) } - r, ok := gateRefusal(statement.KindOther, false) + r, ok := gateRefusal(statement.KindOther, false, statement.IndexTargetNone) require.True(t, ok) assert.Equal(t, verdict.ClassCapabilityBoundary, r.Class(), "unnamed grammar is a boundary, not someone else's work") assert.Empty(t, r.Owner()) @@ -168,6 +180,58 @@ func TestRefusalRegistryCorrespondence(t *testing.T) { assert.Equal(t, fromCause, fromSentinel) } +func TestAcceptedBlockingEligibleRowsArePinned(t *testing.T) { + index, ok := gateRefusal(statement.KindDropIndex, false, statement.IndexTargetSingleRelation) + require.True(t, ok) + parent, ok := partitionRefusal(preflight.PartitionCauseBlockingIndexBuild) + require.True(t, ok) + + assert.True(t, AcceptedBlockingEligible(index)) + assert.True(t, AcceptedBlockingEligible(parent)) + assert.False(t, AcceptedBlockingEligible(rewriteRequiredRefusal())) + assert.False(t, AcceptedBlockingEligible(backendUnavailableRefusal())) +} + +func TestIndexStatementAcceptedBlockingEligibility(t *testing.T) { + tests := []struct { + name string + sql string + want bool + }{ + {"drop one index", "DROP INDEX app.i", true}, + {"reindex index", "REINDEX INDEX app.i", true}, + {"reindex table", "REINDEX TABLE app.t", true}, + {"drop multiple indexes", "DROP INDEX app.i, app.j", false}, + {"reindex schema", "REINDEX SCHEMA app", false}, + {"reindex database", "REINDEX DATABASE app", false}, + {"reindex system", "REINDEX SYSTEM app", false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + st, err := statement.ParseOne(tc.sql) + require.NoError(t, err) + r, ok := gateRefusal(st.Kind(), st.Concurrent(), st.IndexTarget()) + require.True(t, ok) + assert.Equal(t, tc.want, AcceptedBlockingEligible(r)) + }) + } +} + +func TestAcceptedBlockingEligibilityIgnoresRenderedText(t *testing.T) { + r, ok := gateRefusal(statement.KindDropIndex, false, statement.IndexTargetSingleRelation) + require.True(t, ok) + v := verdict.Verdict{Detail: "first explanation", SaferIdiom: "first rendering"}.WithRefusal(r) + before := AcceptedBlockingEligible(r) + v.Detail = "completely different" + v.SaferIdiom = "different rendering" + after := AcceptedBlockingEligible(r) + + assert.Equal(t, "completely different", v.Detail) + assert.Equal(t, "different rendering", v.SaferIdiom) + assert.True(t, before) + assert.Equal(t, before, after) +} + // Membership and classification are one walk: an error outside the sentinel // set, or a sentinel wrapped in a step error (execution started), is not an // admission refusal. diff --git a/pkg/migrate/verdicts.go b/pkg/migrate/verdicts.go index e837286..b3de380 100644 --- a/pkg/migrate/verdicts.go +++ b/pkg/migrate/verdicts.go @@ -21,7 +21,7 @@ import ( // statements are never executed. Gate needs no database, so a caller can // refuse before dialing; [Run] re-checks it regardless. func Gate(st statement.Statement) (verdict.Verdict, bool) { - r, refused := gateRefusal(st.Kind(), st.Concurrent()) + r, refused := gateRefusal(st.Kind(), st.Concurrent(), st.IndexTarget()) if !refused { return verdict.Verdict{}, false } diff --git a/pkg/plan/plan_test.go b/pkg/plan/plan_test.go index b0575f4..9e82dd8 100644 --- a/pkg/plan/plan_test.go +++ b/pkg/plan/plan_test.go @@ -205,6 +205,19 @@ func TestRefuseUnsupportedPartitionedParentClassFollowsCause(t *testing.T) { } } +// The refusal proof carries the preflight cause under the verdict package's +// own typed vocabulary. The two constant sets spell the same wire tokens, and +// this pins that correspondence for every cause preflight registers: a cause +// added on one side without the other is a registry hole, not a silent +// no-cause refusal. +func TestPartitionRefusalCarriesItsCause(t *testing.T) { + for _, cause := range preflight.PartitionRefusalCauses() { + r, ok := plan.PartitionRefusal(cause) + require.True(t, ok, cause) + assert.Equal(t, string(cause), string(r.Cause()), cause) + } +} + func TestRefuseUnsupportedPartitionedParentFailsClosed(t *testing.T) { r := plan.Report{Disposition: router.DispositionExecute, Statements: []plan.Statement{{Disposition: router.DispositionExecute}}} require.Error(t, plan.RefuseUnsupportedPartitionedParent(&r, nil), "positional length mismatch") diff --git a/pkg/plan/refusal.go b/pkg/plan/refusal.go index 38ac4a9..e35fe17 100644 --- a/pkg/plan/refusal.go +++ b/pkg/plan/refusal.go @@ -44,21 +44,30 @@ func CreateShapeRefusal(cause executor.CreateShapeCause) (verdict.Refusal, bool) // PartitionRefusal classifies a partitioned-parent refusal by its cause. The // closed key set is preflight.PartitionRefusalCauses(); ok is false outside // it. The four causes span three classes, which is why the plan carries the -// cause rather than a bare refused flag. +// cause rather than a bare refused flag. The refusal keeps its typed cause, +// so every consumer of the proof — both front doors and the accepted-blocking +// eligibility registry — reads one narrowing instead of re-deriving it from +// the preflight error. func PartitionRefusal(cause preflight.PartitionRefusalCause) (verdict.Refusal, bool) { switch cause { - case preflight.PartitionCauseConcurrentIndexBuild, preflight.PartitionCauseBlockingIndexBuild: - // The partition-aware concurrent index flow is a planned capability; - // refusing the blocking substitute is the policy half of the same gap. - return verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent), true + case preflight.PartitionCauseConcurrentIndexBuild: + // The partition-aware concurrent index flow is a planned capability. + return verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentConcurrentIndexBuild), true + case preflight.PartitionCauseBlockingIndexBuild: + // Refusing the blocking substitute is the policy half of the same gap. + return verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentBlockingIndexBuild), true case preflight.PartitionCauseIndexAdoption: // No supported PostgreSQL version adopts an index as a constraint on // a partitioned parent; waiting for an engine release waits for nothing. - return verdict.ByDesign(verdict.ReasonUnsupportedPartitionedParent), true + return verdict.ByDesign(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentIndexAdoption), true case preflight.PartitionCauseNotValidForeignKey: // The same statement runs on a newer server; the unblocking action is // a server upgrade, not an engine release. - return verdict.Environmental(verdict.ReasonUnsupportedPartitionedParent), true + return verdict.Environmental(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentNotValidForeignKey), true default: return verdict.Refusal{}, false } diff --git a/pkg/statement/statement.go b/pkg/statement/statement.go index e014d47..826926a 100644 --- a/pkg/statement/statement.go +++ b/pkg/statement/statement.go @@ -89,9 +89,25 @@ type Statement struct { schema string table string concurrent bool + indexTarget IndexTarget buildsIndex bool } +// IndexTarget identifies whether an index-maintenance statement names one +// relation. The gate uses this parse-only fact to keep forms requiring more +// than one lock acknowledgement out of accepted-blocking eligibility. +type IndexTarget int + +const ( + // IndexTargetNone is carried by statements outside index maintenance. + IndexTargetNone IndexTarget = iota + // IndexTargetSingleRelation covers one DROP INDEX and REINDEX INDEX or TABLE. + IndexTargetSingleRelation + // IndexTargetOther covers multi-index DROP and REINDEX scopes that do not + // name one relation: SCHEMA, DATABASE, and SYSTEM. + IndexTargetOther +) + // SQL returns the original statement text as submitted. func (s Statement) SQL() string { return s.sql } @@ -112,6 +128,9 @@ func (s Statement) Table() string { return s.table } // It is always false for non-index kinds. func (s Statement) Concurrent() bool { return s.concurrent } +// IndexTarget returns the typed target shape of an index-maintenance statement. +func (s Statement) IndexTarget() IndexTarget { return s.indexTarget } + // BuildsIndex reports whether executing the statement creates a new index: // every CREATE INDEX, and the ALTER TABLE shapes that build one as a side // effect — ADD CONSTRAINT UNIQUE / PRIMARY KEY / EXCLUDE without USING @@ -218,12 +237,24 @@ func ParseOne(sql string) (Statement, error) { if node.GetDropStmt().GetRemoveType() == pganalyze.ObjectType_OBJECT_INDEX { st.kind = KindDropIndex st.concurrent = node.GetDropStmt().GetConcurrent() + if len(node.GetDropStmt().GetObjects()) == 1 { + st.indexTarget = IndexTargetSingleRelation + } else { + st.indexTarget = IndexTargetOther + } } else { st.kind = KindCatalogWork } case node.GetReindexStmt() != nil: st.kind = KindReindex st.concurrent = reindexConcurrently(node.GetReindexStmt()) + switch node.GetReindexStmt().GetKind() { + case pganalyze.ReindexObjectType_REINDEX_OBJECT_INDEX, + pganalyze.ReindexObjectType_REINDEX_OBJECT_TABLE: + st.indexTarget = IndexTargetSingleRelation + default: + st.indexTarget = IndexTargetOther + } default: // Parsed statements that are recognized catalog operations are direct // operator work. Truly unknown grammar nodes retain KindOther. diff --git a/pkg/statement/statement_test.go b/pkg/statement/statement_test.go index 1a546b5..defb14c 100644 --- a/pkg/statement/statement_test.go +++ b/pkg/statement/statement_test.go @@ -99,27 +99,27 @@ func TestParseOneKinds(t *testing.T) { { name: "drop index", sql: "DROP INDEX idx_users_email", - want: Statement{kind: KindDropIndex}, + want: Statement{kind: KindDropIndex, indexTarget: IndexTargetSingleRelation}, }, { name: "drop index concurrently", sql: "DROP INDEX CONCURRENTLY idx_users_email", - want: Statement{kind: KindDropIndex, concurrent: true}, + want: Statement{kind: KindDropIndex, concurrent: true, indexTarget: IndexTargetSingleRelation}, }, { name: "reindex table", sql: "REINDEX TABLE users", - want: Statement{kind: KindReindex}, + want: Statement{kind: KindReindex, indexTarget: IndexTargetSingleRelation}, }, { name: "reindex index", sql: "REINDEX INDEX idx_users_email", - want: Statement{kind: KindReindex}, + want: Statement{kind: KindReindex, indexTarget: IndexTargetSingleRelation}, }, { name: "reindex table concurrently", sql: "REINDEX TABLE CONCURRENTLY users", - want: Statement{kind: KindReindex, concurrent: true}, + want: Statement{kind: KindReindex, concurrent: true, indexTarget: IndexTargetSingleRelation}, }, { name: "alter index parses as AlterTableStmt but is not a table target", diff --git a/pkg/verdict/verdict.go b/pkg/verdict/verdict.go index fab574e..7e9b05b 100644 --- a/pkg/verdict/verdict.go +++ b/pkg/verdict/verdict.go @@ -185,8 +185,23 @@ type Refusal struct { class Class reason Reason owner Owner + cause Cause + site RefusalSite } +// RefusalSite is a typed refusal-site discriminator used when a reason spans +// statement shapes but has no underlying cause. +type RefusalSite string + +const ( + // RefusalSiteIndexSingleRelation is the plain one-relation DROP INDEX, + // REINDEX INDEX, or REINDEX TABLE gate site. + RefusalSiteIndexSingleRelation RefusalSite = "index-statement-single-relation" + // RefusalSiteIndexOther is a multi-relation DROP INDEX or a REINDEX scope + // that does not identify one relation. + RefusalSiteIndexOther RefusalSite = "index-statement-other" +) + // NewRefusal validates and constructs a refusal proof. It rejects a reason // outside Reasons(), a class outside Classes(), an owner outside Owners(), // and an owner that is absent when the class is no-online-safety-problem or @@ -257,6 +272,24 @@ func (r Refusal) Reason() Reason { return r.reason } // no-online-safety-problem. func (r Refusal) Owner() Owner { return r.owner } +// Cause returns the typed cause that narrows the refusal, when one exists. +func (r Refusal) Cause() Cause { return r.cause } + +// Site returns the typed refusal site that narrows the refusal, when one exists. +func (r Refusal) Site() RefusalSite { return r.site } + +// WithCause returns r narrowed by a typed cause. +func (r Refusal) WithCause(cause Cause) Refusal { + r.cause = cause + return r +} + +// WithSite returns r narrowed by a typed refusal site. +func (r Refusal) WithSite(site RefusalSite) Refusal { + r.site = site + return r +} + // IsZero reports whether r was never constructed through NewRefusal. func (r Refusal) IsZero() bool { return r == Refusal{} } @@ -299,6 +332,17 @@ const ( // CauseStatementBudget: the statement ran past statement_timeout and was // cancelled; the change needs a rewrite. CauseStatementBudget Cause = "statement-budget" + // CauseParentBlockingIndexBuild identifies a blocking index build on a + // partitioned parent. + CauseParentBlockingIndexBuild Cause = "parent-blocking-index-build" + // CauseParentConcurrentIndexBuild identifies a concurrent index build on + // a partitioned parent. + CauseParentConcurrentIndexBuild Cause = "parent-concurrent-index-build" + // CauseParentIndexAdoption identifies index adoption on a partitioned parent. + CauseParentIndexAdoption Cause = "parent-index-adoption" + // CauseParentNotValidForeignKey identifies a NOT VALID foreign key on a + // partitioned parent. + CauseParentNotValidForeignKey Cause = "parent-not-valid-foreign-key" ) // Verdict is the structured outcome of one migrate invocation. diff --git a/pkg/verdict/verdict_test.go b/pkg/verdict/verdict_test.go index b197a2f..6b5a605 100644 --- a/pkg/verdict/verdict_test.go +++ b/pkg/verdict/verdict_test.go @@ -163,7 +163,11 @@ func TestJSONOmitsEmptyOptionalFields(t *testing.T) { // Reason and Cause values are the machine contract automation switches on: // flat kebab-case tokens, no spaces or colons — prose belongs in Detail. func TestReasonAndCauseTokensAreFlat(t *testing.T) { - toks := []string{string(CauseLockBudget), string(CauseStatementBudget)} + toks := []string{ + string(CauseLockBudget), string(CauseStatementBudget), + string(CauseParentBlockingIndexBuild), string(CauseParentConcurrentIndexBuild), + string(CauseParentIndexAdoption), string(CauseParentNotValidForeignKey), + } for _, r := range Reasons() { toks = append(toks, string(r)) } From 81daca74a1a55b97dce156c1850ecff56487171f Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Fri, 11 Sep 2026 13:36:10 +1000 Subject: [PATCH 2/3] verdict: carry the refusal proof through WithRefusal and Refusal() Preserve refusal sites and causes on in-process verdicts so accepted-blocking eligibility sees the same typed proof produced by the gate. Verdicts decoded from JSON reconstruct the serialized fields but cannot recover refusal sites, so site-keyed eligibility deliberately fails closed. --- docs/invariants.md | 5 +++ docs/lock-budgeted-passthrough.md | 5 +++ internal/cli/color_test.go | 3 ++ pkg/migrate/refusal_registry_test.go | 52 ++++++++++++++++++++++------ pkg/migrate/verdicts.go | 11 ++++-- pkg/statement/statement.go | 19 ++++++++++ pkg/verdict/verdict.go | 30 +++++++++++++--- pkg/verdict/verdict_test.go | 13 ++++++- 8 files changed, 118 insertions(+), 20 deletions(-) diff --git a/docs/invariants.md b/docs/invariants.md index 5dc1669..5b77a46 100644 --- a/docs/invariants.md +++ b/docs/invariants.md @@ -418,6 +418,11 @@ Each refusal is a preflight **error with a stated reason** — never a warning, fields; the classification registry (`pkg/plan/refusal.go`, `pkg/migrate/refusal_registry.go`) is checked for completeness against the production closed sets by `TestRefusalRegistryIsComplete`. *Source:* [refusal classes](refusal-classes.md). +- **RF-8** — An in-process refused verdict retains its full typed refusal proof. A verdict + decoded from JSON reconstructs class, reason, owner, and cause, but cannot recover the + unexported refusal site; eligibility decisions keyed by site therefore fail closed for + decoded verdicts. *Enforced:* `Verdict.WithRefusal`, `Verdict.Refusal`, and the accepted- + blocking eligibility gate-path tests. *Source:* [lock-budgeted passthrough](lock-budgeted-passthrough.md). ## Orchestration / control-plane (OC) diff --git a/docs/lock-budgeted-passthrough.md b/docs/lock-budgeted-passthrough.md index 8507396..2268a34 100644 --- a/docs/lock-budgeted-passthrough.md +++ b/docs/lock-budgeted-passthrough.md @@ -102,6 +102,11 @@ it is acquired. | 6 | Eligibility is selected from a closed registry keyed by typed class, reason, and cause or refusal site. No renderer text or SQL substring participates in the decision. | | 7 | A dry-run reports the original refusal and a per-statement `blocking_passthrough_eligible` boolean, but executes nothing even when the flag is present. | +The full typed refusal proof is available while its verdict remains in process. JSON carries +class, reason, owner, and cause, but not the internal refusal site. Calling `Refusal()` on a +decoded verdict therefore reconstructs the JSON fields but cannot restore the site, and any +site-keyed accepted-blocking eligibility decision fails closed. + “Prints before execution” is an ordering requirement for human output and an information requirement for JSON. Human mode prints the refusal analysis, then a separate acceptance line, then starts the session. JSON remains one final machine-readable object; its executed verdict diff --git a/internal/cli/color_test.go b/internal/cli/color_test.go index 601abf2..5259e4e 100644 --- a/internal/cli/color_test.go +++ b/internal/cli/color_test.go @@ -182,6 +182,9 @@ func fullVerdict(t *testing.T) verdict.Verdict { } rv := reflect.ValueOf(v) for i := range rv.NumField() { + if rv.Type().Field(i).PkgPath != "" { + continue + } require.False(t, rv.Field(i).IsZero(), "Verdict field %s is zero in the all-fields fixture; set it so the renderer parity lock covers it", rv.Type().Field(i).Name) diff --git a/pkg/migrate/refusal_registry_test.go b/pkg/migrate/refusal_registry_test.go index 5d5e74a..8553bf5 100644 --- a/pkg/migrate/refusal_registry_test.go +++ b/pkg/migrate/refusal_registry_test.go @@ -1,6 +1,7 @@ package migrate import ( + "encoding/json" "errors" "fmt" "testing" @@ -50,13 +51,13 @@ func deriveRefusalKeys() (keys []classifiedKey, admitted []string) { for _, concurrent := range []bool{false, true} { targets := []statement.IndexTarget{statement.IndexTargetNone} if !concurrent && (k == statement.KindDropIndex || k == statement.KindReindex) { - targets = []statement.IndexTarget{statement.IndexTargetSingleRelation, statement.IndexTargetOther} + targets = statement.IndexTargets() } for _, target := range targets { r, ok := gateRefusal(k, concurrent, target) key := fmt.Sprintf("gate:%s/concurrent=%t", k, concurrent) if len(targets) > 1 { - key = fmt.Sprintf("%s/target=%d", key, target) + key = fmt.Sprintf("%s/target=%s", key, target) } if !ok { admitted = append(admitted, key) @@ -217,19 +218,48 @@ func TestIndexStatementAcceptedBlockingEligibility(t *testing.T) { } } +func TestGateVerdictAcceptedBlockingEligibility(t *testing.T) { + for _, tc := range []struct { + name string + sql string + want bool + }{ + {"single relation", "DROP INDEX app.i", true}, + {"multiple relations", "DROP INDEX app.i, app.j", false}, + } { + t.Run(tc.name, func(t *testing.T) { + st, err := statement.ParseOne(tc.sql) + require.NoError(t, err) + v, refused := Gate(st) + require.True(t, refused) + r, err := v.Refusal() + require.NoError(t, err) + assert.Equal(t, tc.want, AcceptedBlockingEligible(r)) + + if tc.want { + encoded, err := json.Marshal(v) + require.NoError(t, err) + var decoded verdict.Verdict + require.NoError(t, json.Unmarshal(encoded, &decoded)) + decodedRefusal, err := decoded.Refusal() + require.NoError(t, err) + assert.False(t, AcceptedBlockingEligible(decodedRefusal)) + } + }) + } +} + func TestAcceptedBlockingEligibilityIgnoresRenderedText(t *testing.T) { r, ok := gateRefusal(statement.KindDropIndex, false, statement.IndexTargetSingleRelation) require.True(t, ok) - v := verdict.Verdict{Detail: "first explanation", SaferIdiom: "first rendering"}.WithRefusal(r) - before := AcceptedBlockingEligible(r) - v.Detail = "completely different" - v.SaferIdiom = "different rendering" - after := AcceptedBlockingEligible(r) + first, err := verdict.Verdict{Detail: "first explanation", SaferIdiom: "first rendering"}.WithRefusal(r).Refusal() + require.NoError(t, err) + second, err := verdict.Verdict{Detail: "completely different", SaferIdiom: "different rendering"}.WithRefusal(r).Refusal() + require.NoError(t, err) - assert.Equal(t, "completely different", v.Detail) - assert.Equal(t, "different rendering", v.SaferIdiom) - assert.True(t, before) - assert.Equal(t, before, after) + assert.Equal(t, first, second) + assert.Equal(t, AcceptedBlockingEligible(first), AcceptedBlockingEligible(second)) + assert.True(t, AcceptedBlockingEligible(first)) } // Membership and classification are one walk: an error outside the sentinel diff --git a/pkg/migrate/verdicts.go b/pkg/migrate/verdicts.go index b3de380..79faf84 100644 --- a/pkg/migrate/verdicts.go +++ b/pkg/migrate/verdicts.go @@ -170,14 +170,20 @@ func admissionRefusalVerdict(st statement.Statement, err error, r verdict.Refusa // different strategy, while a blind attempt that ran past its budget is // doing rewrite work. A refused forced attempt still records the override. func budgetVerdict(st statement.Statement, budgetErr *executor.BudgetError, forced, online bool) verdict.Verdict { + cause := verdict.CauseNone + switch budgetErr.Cause { + case executor.CauseLock: + cause = verdict.CauseLockBudget + case executor.CauseStatement: + cause = verdict.CauseStatementBudget + } v := verdict.Verdict{ Statement: st.SQL(), Table: qualified(st), Forced: forced, - }.WithRefusal(budgetExceededRefusal()) + }.WithRefusal(budgetExceededRefusal().WithCause(cause)) switch budgetErr.Cause { case executor.CauseLock: - v.Cause = verdict.CauseLockBudget v.Attempts = budgetErr.Attempts if budgetErr.Attempts > 1 { v.Detail = fmt.Sprintf("the lock was not granted within the %s lock budget on any of %d bounded "+ @@ -188,7 +194,6 @@ func budgetVerdict(st statement.Statement, budgetErr *executor.BudgetError, forc v.Detail = fmt.Sprintf("the lock was not granted within the %s lock budget: the table is too "+ "contended right now; nothing was executed", budgetErr.Budget) case executor.CauseStatement: - v.Cause = verdict.CauseStatementBudget if online { v.Detail = fmt.Sprintf("cancelled after the %s budget: the statement already is the safe online "+ "idiom — the work needs more time, not a different strategy; retry with a larger budget for "+ diff --git a/pkg/statement/statement.go b/pkg/statement/statement.go index 826926a..9bae702 100644 --- a/pkg/statement/statement.go +++ b/pkg/statement/statement.go @@ -108,6 +108,25 @@ const ( IndexTargetOther ) +// IndexTargets returns the closed set of index-maintenance target shapes. +func IndexTargets() []IndexTarget { + return []IndexTarget{IndexTargetSingleRelation, IndexTargetOther} +} + +// String returns the stable name of an index target shape. +func (t IndexTarget) String() string { + switch t { + case IndexTargetNone: + return "none" + case IndexTargetSingleRelation: + return "single-relation" + case IndexTargetOther: + return "other" + default: + return fmt.Sprintf("IndexTarget(%d)", t) + } +} + // SQL returns the original statement text as submitted. func (s Statement) SQL() string { return s.sql } diff --git a/pkg/verdict/verdict.go b/pkg/verdict/verdict.go index 7e9b05b..3978c40 100644 --- a/pkg/verdict/verdict.go +++ b/pkg/verdict/verdict.go @@ -294,15 +294,18 @@ func (r Refusal) WithSite(site RefusalSite) Refusal { func (r Refusal) IsZero() bool { return r == Refusal{} } // WithRefusal returns v as a refused verdict carrying r's reason, class, -// and owner. It is the one path from a classified refusal onto the verdict -// contract, so a site that forgets to classify has no Reason to set. +// owner, cause, and full in-process proof. It is the one path from a +// classified refusal onto the verdict contract, so a site that forgets to +// classify has no Reason to set. func (v Verdict) WithRefusal(r Refusal) Verdict { - // INV: RF-7 — the verdict's class and owner come from the proof, never - // from the site. + // INV: RF-7, RF-8 — the verdict's refusal fields and in-process proof + // come from the proof, never from the site. v.Outcome = OutcomeRefused v.Reason = r.reason v.Class = r.class v.Owner = r.owner + v.Cause = r.cause + v.proof = r return v } @@ -311,11 +314,26 @@ func (v Verdict) WithRefusal(r Refusal) Verdict { // class and owner through the same one path instead of copying fields. 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. +// An in-process verdict retains its full proof. A verdict decoded from JSON +// reconstructs class, reason, owner, and cause, but cannot recover its refusal +// site; site-keyed eligibility therefore fails closed after JSON decoding. func (v Verdict) Refusal() (Refusal, error) { if v.Outcome != OutcomeRefused { return Refusal{}, fmt.Errorf("verdict outcome is %q, not %q", v.Outcome, OutcomeRefused) } - return NewRefusal(v.Class, v.Reason, v.Owner) + // INV: RF-8 — preserve the full in-process proof; decoded verdicts can + // reconstruct only the refusal fields represented in JSON. + if !v.proof.IsZero() { + return v.proof, nil + } + r, err := NewRefusal(v.Class, v.Reason, v.Owner) + if err != nil { + return Refusal{}, err + } + if v.Cause != CauseNone { + r = r.WithCause(v.Cause) + } + return r, nil } // Cause narrows ReasonBudgetExceeded to the budget that was exceeded, so @@ -347,6 +365,8 @@ const ( // Verdict is the structured outcome of one migrate invocation. type Verdict struct { + proof Refusal + // Outcome is what happened. Outcome Outcome `json:"outcome"` // Reason is the typed refusal cause; empty when executed. diff --git a/pkg/verdict/verdict_test.go b/pkg/verdict/verdict_test.go index 6b5a605..581683c 100644 --- a/pkg/verdict/verdict_test.go +++ b/pkg/verdict/verdict_test.go @@ -78,11 +78,22 @@ func TestWithRefusalStampsOutcomeReasonClassOwner(t *testing.T) { } func TestRefusalRoundTripsThroughVerdict(t *testing.T) { - want := NoOnlineSafetyProblem(ReasonUnsupportedStatement, OwnerProvisioning) + want := ByDesign(ReasonIndexStatement). + WithCause(CauseStatementBudget). + WithSite(RefusalSiteIndexSingleRelation) got, err := Verdict{Statement: "GRANT SELECT ON t TO r"}.WithRefusal(want).Refusal() require.NoError(t, err) assert.Equal(t, want, got) + encoded, err := json.Marshal(Verdict{}.WithRefusal(want)) + require.NoError(t, err) + var decoded Verdict + require.NoError(t, json.Unmarshal(encoded, &decoded)) + got, err = decoded.Refusal() + require.NoError(t, err) + assert.Equal(t, CauseStatementBudget, got.Cause()) + assert.Equal(t, RefusalSite(""), got.Site()) + _, err = Verdict{Outcome: OutcomeExecuted}.Refusal() require.Error(t, err, "an executed verdict carries no refusal") From 588436bd33bcc9cc421a5cd8f332317a923df231 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Fri, 11 Sep 2026 14:20:20 +1000 Subject: [PATCH 3/3] verdict: validate the in-process proof and pin the eligible set from both sides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refusal() returned a non-zero in-process proof without the RF-7 validation the decoded path applies, so a Refusal built outside the constructors (for example Refusal{}.WithSite(...)) or a per-class constructor with an unknown reason reached callers unchecked. It now re-validates the proof through NewRefusal and errors when the verdict's exported class, reason, owner, or cause no longer match the proof WithRefusal stamped them from, so the refusal a consumer reads is the one an eligibility decision consumes. The eligibility registry tests now pin the eligible set from both sides: walking every key the completeness harness derives, exactly the single-relation DROP INDEX and REINDEX gate keys and the blocking parent index build are eligible. New tests prove the default arms fail closed and undecided for an unknown reason, site, or cause, and that each eligible row requires its class. pkg/statement pins IndexTargets() against the declared IndexTarget constants the way Kinds() is pinned. RF-8 and the passthrough design now state the shipped boundary precisely: the site-keyed row fails closed after JSON decoding, the cause-keyed row is decidable from the wire fields, and eligibility is consumed only from the proof of the verdict the same front-door invocation produced. 🤖 Generated with Amp (Claude Opus 4.6) --- docs/invariants.md | 16 ++++-- docs/lock-budgeted-passthrough.md | 14 +++-- pkg/migrate/refusal_registry_test.go | 82 ++++++++++++++++++++++++++-- pkg/statement/statement_test.go | 51 ++++++++++++++--- pkg/verdict/verdict.go | 46 ++++++++++++++-- pkg/verdict/verdict_test.go | 50 +++++++++++++++++ 6 files changed, 233 insertions(+), 26 deletions(-) diff --git a/docs/invariants.md b/docs/invariants.md index 5b77a46..17a82c5 100644 --- a/docs/invariants.md +++ b/docs/invariants.md @@ -418,11 +418,17 @@ Each refusal is a preflight **error with a stated reason** — never a warning, fields; the classification registry (`pkg/plan/refusal.go`, `pkg/migrate/refusal_registry.go`) is checked for completeness against the production closed sets by `TestRefusalRegistryIsComplete`. *Source:* [refusal classes](refusal-classes.md). -- **RF-8** — An in-process refused verdict retains its full typed refusal proof. A verdict - decoded from JSON reconstructs class, reason, owner, and cause, but cannot recover the - unexported refusal site; eligibility decisions keyed by site therefore fail closed for - decoded verdicts. *Enforced:* `Verdict.WithRefusal`, `Verdict.Refusal`, and the accepted- - blocking eligibility gate-path tests. *Source:* [lock-budgeted passthrough](lock-budgeted-passthrough.md). +- **RF-8** — An in-process refused verdict retains its full typed refusal proof, and + `Verdict.Refusal` hands that proof out only when it validates under RF-7 and still matches + the verdict's exported refusal fields. A verdict decoded from JSON reconstructs class, + reason, owner, and cause, but cannot recover the unexported refusal site: the site-keyed + eligibility row fails closed for a decoded verdict, while the cause-keyed row is decidable + from the wire fields. Accepted-blocking eligibility is therefore consumed only from the + proof of the verdict the same front-door invocation produced, never from a decoded verdict. + *Enforced:* `Verdict.WithRefusal`, `Verdict.Refusal`, + `TestRefusalRejectsUnvalidatedProof`, `TestRefusalRejectsFieldsDivergingFromProof`, and + `TestGateVerdictAcceptedBlockingEligibility`, which pins that the site-keyed row is eligible + in process and ineligible after a JSON round trip. *Source:* [lock-budgeted passthrough](lock-budgeted-passthrough.md). ## Orchestration / control-plane (OC) diff --git a/docs/lock-budgeted-passthrough.md b/docs/lock-budgeted-passthrough.md index 2268a34..9a694e0 100644 --- a/docs/lock-budgeted-passthrough.md +++ b/docs/lock-budgeted-passthrough.md @@ -102,10 +102,16 @@ it is acquired. | 6 | Eligibility is selected from a closed registry keyed by typed class, reason, and cause or refusal site. No renderer text or SQL substring participates in the decision. | | 7 | A dry-run reports the original refusal and a per-statement `blocking_passthrough_eligible` boolean, but executes nothing even when the flag is present. | -The full typed refusal proof is available while its verdict remains in process. JSON carries -class, reason, owner, and cause, but not the internal refusal site. Calling `Refusal()` on a -decoded verdict therefore reconstructs the JSON fields but cannot restore the site, and any -site-keyed accepted-blocking eligibility decision fails closed. +The full typed refusal proof is available while its verdict remains in process, and +`Refusal()` returns it only when it validates as a classified refusal and still matches the +verdict's exported fields. JSON carries class, reason, owner, and cause, but not the internal +refusal site. Calling `Refusal()` on a decoded verdict therefore reconstructs the JSON fields +but cannot restore the site: the site-keyed row (single-relation `index-statement`) fails +closed after decoding, while the cause-keyed row (`unsupported-partitioned-parent` with +`parent-blocking-index-build`) is decidable from the wire fields. Decoding is not the +fail-closed boundary; the front door is. Eligibility is consumed only from the proof of the +verdict the same `migrate` or `diff` invocation produced, never from a verdict read back +from JSON. “Prints before execution” is an ordering requirement for human output and an information requirement for JSON. Human mode prints the refusal analysis, then a separate acceptance line, diff --git a/pkg/migrate/refusal_registry_test.go b/pkg/migrate/refusal_registry_test.go index 8553bf5..9b435e7 100644 --- a/pkg/migrate/refusal_registry_test.go +++ b/pkg/migrate/refusal_registry_test.go @@ -181,18 +181,88 @@ func TestRefusalRegistryCorrespondence(t *testing.T) { assert.Equal(t, fromCause, fromSentinel) } +// The eligible set is pinned from both sides over the same production walk +// the completeness harness uses: exactly the single-relation DROP INDEX and +// REINDEX gate keys and the blocking parent index build are eligible, and +// every other key the closed sets name — the other partition causes, the +// concurrent and multi-relation index forms, every create-shape cause, +// admission sentinel, site refusal, and the route refusal — is ineligible. +// A registry that flipped a sibling cause or dropped a class check would +// change this set. func TestAcceptedBlockingEligibleRowsArePinned(t *testing.T) { - index, ok := gateRefusal(statement.KindDropIndex, false, statement.IndexTargetSingleRelation) - require.True(t, ok) - parent, ok := partitionRefusal(preflight.PartitionCauseBlockingIndexBuild) - require.True(t, ok) + keys, _ := deriveRefusalKeys() + require.NotEmpty(t, keys) - assert.True(t, AcceptedBlockingEligible(index)) - assert.True(t, AcceptedBlockingEligible(parent)) + var eligible []string + for _, k := range keys { + if AcceptedBlockingEligible(k.refusal) { + eligible = append(eligible, k.key) + } + } + assert.ElementsMatch(t, []string{ + "gate:DROP INDEX/concurrent=false/target=single-relation", + "gate:REINDEX/concurrent=false/target=single-relation", + "partition:" + string(preflight.PartitionCauseBlockingIndexBuild), + }, eligible) + + for _, cause := range preflight.PartitionRefusalCauses() { + r, ok := partitionRefusal(cause) + require.True(t, ok, cause) + assert.Equal(t, cause == preflight.PartitionCauseBlockingIndexBuild, AcceptedBlockingEligible(r), cause) + } assert.False(t, AcceptedBlockingEligible(rewriteRequiredRefusal())) assert.False(t, AcceptedBlockingEligible(backendUnavailableRefusal())) } +// The registry's default arms are fail-closed, not merely undecided: a +// reason, site, or cause outside the vocabulary it enumerates is ineligible +// and reported as undecided, so the completeness harness rejects it. The +// per-class constructors do not validate their reason, which is how a +// vocabulary addition reaches the registry before it is classified. +func TestAcceptedBlockingUnknownKeysFailClosed(t *testing.T) { + tests := []struct { + name string + refusal verdict.Refusal + }{ + {"unknown reason", verdict.ByDesign("brand-new-reason")}, + {"unknown site under index-statement", verdict.ByDesign(verdict.ReasonIndexStatement).WithSite("brand-new-site")}, + {"unknown cause under partitioned parent", verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent).WithCause("brand-new-cause")}, + {"no cause under partitioned parent", verdict.CapabilityBoundary(verdict.ReasonUnsupportedPartitionedParent)}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + eligible, decided := acceptedBlockingDecision(tc.refusal) + assert.False(t, eligible) + assert.False(t, decided) + assert.False(t, AcceptedBlockingEligible(tc.refusal)) + }) + } +} + +// Each eligible row is keyed on its class as well as its reason and +// discriminator: the same reason and site, or reason and cause, under a +// different class is a decided ineligible key, not an eligible one. +func TestAcceptedBlockingRowsRequireTheirClass(t *testing.T) { + tests := []struct { + name string + refusal verdict.Refusal + }{ + {"index site outside by-design", + verdict.NoOnlineSafetyProblem(verdict.ReasonIndexStatement, verdict.OwnerDirectOperator). + WithSite(verdict.RefusalSiteIndexSingleRelation)}, + {"parent cause outside capability-boundary", + verdict.ByDesign(verdict.ReasonUnsupportedPartitionedParent). + WithCause(verdict.CauseParentBlockingIndexBuild)}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + eligible, decided := acceptedBlockingDecision(tc.refusal) + assert.False(t, eligible) + assert.True(t, decided) + }) + } +} + func TestIndexStatementAcceptedBlockingEligibility(t *testing.T) { tests := []struct { name string diff --git a/pkg/statement/statement_test.go b/pkg/statement/statement_test.go index defb14c..68597ac 100644 --- a/pkg/statement/statement_test.go +++ b/pkg/statement/statement_test.go @@ -354,9 +354,46 @@ func TestKindsIsClosedAndNamed(t *testing.T) { "Kinds() must enumerate exactly the Kind constants statement.go declares") } +// IndexTargets() is the closed set the eligibility registry walks, so it +// must name every declared target shape except IndexTargetNone, which +// marks statements outside index maintenance rather than a shape of one. +// Each named shape has its own String() rather than the numeric fallback. +func TestIndexTargetsIsClosedAndNamed(t *testing.T) { + targets := IndexTargets() + assert.NotContains(t, targets, IndexTargetNone, "the non-shape is not a target the registry decides") + + declared := declaredIotaValues(t, "IndexTargetNone") + delete(declared, int(IndexTargetNone)) + enumerated := make(map[int]struct{}, len(targets)) + names := map[string]bool{} + for _, target := range targets { + _, dup := enumerated[int(target)] + assert.False(t, dup, "duplicate index target %d", target) + enumerated[int(target)] = struct{}{} + assert.NotContains(t, target.String(), "IndexTarget(", "target %d falls through to the numeric name", target) + assert.False(t, names[target.String()], "duplicate index target name %q", target.String()) + names[target.String()] = true + } + assert.Equal(t, declared, enumerated, + "IndexTargets() must enumerate exactly the IndexTarget constants statement.go declares after IndexTargetNone") + assert.Equal(t, "none", IndexTargetNone.String()) +} + // declaredKinds parses statement.go and returns the value of every constant // in the iota block that KindOther opens. func declaredKinds(t *testing.T) map[Kind]struct{} { + t.Helper() + values := declaredIotaValues(t, "KindOther") + declared := make(map[Kind]struct{}, len(values)) + for v := range values { + declared[Kind(v)] = struct{}{} + } + return declared +} + +// declaredIotaValues parses statement.go and returns the value of every +// constant in the iota block that the named anchor constant opens. +func declaredIotaValues(t *testing.T, anchor string) map[int]struct{} { t.Helper() fset := token.NewFileSet() file, err := parser.ParseFile(fset, "statement.go", nil, parser.SkipObjectResolution) @@ -368,25 +405,25 @@ func declaredKinds(t *testing.T) map[Kind]struct{} { continue } first, ok := gen.Specs[0].(*ast.ValueSpec) - if !ok || len(first.Names) != 1 || first.Names[0].Name != "KindOther" { + if !ok || len(first.Names) != 1 || first.Names[0].Name != anchor { continue } - declared := make(map[Kind]struct{}, len(gen.Specs)) + declared := make(map[int]struct{}, len(gen.Specs)) for i, spec := range gen.Specs { vs, ok := spec.(*ast.ValueSpec) require.True(t, ok) - require.Len(t, vs.Names, 1, "one Kind per line in the iota block") + require.Len(t, vs.Names, 1, "one constant per line in the %s iota block", anchor) if i == 0 { - require.Len(t, vs.Values, 1, "KindOther opens the iota block") + require.Len(t, vs.Values, 1, "%s opens the iota block", anchor) ident, ok := vs.Values[0].(*ast.Ident) - require.True(t, ok && ident.Name == "iota", "KindOther is the iota anchor") + require.True(t, ok && ident.Name == "iota", "%s is the iota anchor", anchor) } else { require.Empty(t, vs.Values, "%s takes its value from iota", vs.Names[0].Name) } - declared[Kind(i)] = struct{}{} + declared[i] = struct{}{} } return declared } - t.Fatal("statement.go declares no const block opened by KindOther") + t.Fatalf("statement.go declares no const block opened by %s", anchor) return nil } diff --git a/pkg/verdict/verdict.go b/pkg/verdict/verdict.go index 3978c40..12311ee 100644 --- a/pkg/verdict/verdict.go +++ b/pkg/verdict/verdict.go @@ -314,9 +314,13 @@ func (v Verdict) WithRefusal(r Refusal) Verdict { // class and owner through the same one path instead of copying fields. 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. -// An in-process verdict retains its full proof. A verdict decoded from JSON -// reconstructs class, reason, owner, and cause, but cannot recover its refusal -// site; site-keyed eligibility therefore fails closed after JSON decoding. +// An in-process verdict returns its full proof, re-validated the same way +// and checked against the exported refusal fields, so a proof that never +// passed the constructors, or fields rewritten after WithRefusal, cannot +// reach an eligibility decision. A verdict decoded from JSON reconstructs +// class, reason, owner, and cause, but cannot recover its refusal site; +// site-keyed eligibility therefore fails closed after JSON decoding, while +// cause-keyed eligibility is decidable from the JSON fields. func (v Verdict) Refusal() (Refusal, error) { if v.Outcome != OutcomeRefused { return Refusal{}, fmt.Errorf("verdict outcome is %q, not %q", v.Outcome, OutcomeRefused) @@ -324,7 +328,7 @@ func (v Verdict) Refusal() (Refusal, error) { // INV: RF-8 — preserve the full in-process proof; decoded verdicts can // reconstruct only the refusal fields represented in JSON. if !v.proof.IsZero() { - return v.proof, nil + return v.provenRefusal() } r, err := NewRefusal(v.Class, v.Reason, v.Owner) if err != nil { @@ -336,6 +340,35 @@ func (v Verdict) Refusal() (Refusal, error) { return r, nil } +// provenRefusal returns the in-process proof once it validates under RF-7 +// and agrees with the verdict's exported refusal fields. Both checks are +// needed: the per-class constructors do not validate their reason, and the +// exported fields are what a consumer reads while the proof is what an +// eligibility decision consumes. +func (v Verdict) provenRefusal() (Refusal, error) { + // INV: RF-7, RF-8 — a proof reaches a consumer only when it is a valid + // classified refusal and the verdict still describes it. + if _, err := NewRefusal(v.proof.class, v.proof.reason, v.proof.owner); err != nil { + return Refusal{}, fmt.Errorf("verdict refusal proof: %w", err) + } + if v.refusalFieldsDivergeFromProof() { + return Refusal{}, fmt.Errorf( + "verdict refusal fields class=%q reason=%q owner=%q cause=%q diverge from proof class=%q reason=%q owner=%q cause=%q", + v.Class, v.Reason, v.Owner, v.Cause, + v.proof.class, v.proof.reason, v.proof.owner, v.proof.cause) + } + return v.proof, nil +} + +// refusalFieldsDivergeFromProof reports whether any exported refusal field +// no longer matches the proof WithRefusal stamped it from. +func (v Verdict) refusalFieldsDivergeFromProof() bool { + return v.Class != v.proof.class || + v.Reason != v.proof.reason || + v.Owner != v.proof.owner || + v.Cause != v.proof.cause +} + // Cause narrows ReasonBudgetExceeded to the budget that was exceeded, so // automation can branch on which limit fired without parsing prose. type Cause string @@ -365,6 +398,11 @@ const ( // Verdict is the structured outcome of one migrate invocation. type Verdict struct { + // proof is the full in-process refusal WithRefusal stamped the exported + // fields from. It is deliberately outside the JSON contract: the refusal + // site it carries is not a wire field, so it does not survive decoding, + // and a decoded verdict never compares equal to the in-process verdict it + // was encoded from. Refusal() is the only reader. proof Refusal // Outcome is what happened. diff --git a/pkg/verdict/verdict_test.go b/pkg/verdict/verdict_test.go index 581683c..69dc3db 100644 --- a/pkg/verdict/verdict_test.go +++ b/pkg/verdict/verdict_test.go @@ -104,6 +104,56 @@ func TestRefusalRoundTripsThroughVerdict(t *testing.T) { require.Error(t, err, "an owner outside no-online-safety-problem violates RF-7") } +// The in-process fast path validates the proof the same way the decoded +// path validates the JSON fields: a non-zero proof that never passed the +// constructors is rejected rather than returned as-is. +func TestRefusalRejectsUnvalidatedProof(t *testing.T) { + tests := []struct { + name string + proof Refusal + }{ + {"site on the zero refusal", Refusal{}.WithSite(RefusalSiteIndexSingleRelation)}, + {"cause on the zero refusal", Refusal{}.WithCause(CauseLockBudget)}, + {"reason outside Reasons()", ByDesign("brand-new-reason")}, + {"owner on a class that carries none", Refusal{class: ClassByDesign, reason: ReasonIndexStatement, owner: OwnerDirectOperator}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.False(t, tc.proof.IsZero(), "the fixture must take the in-process path") + _, err := Verdict{}.WithRefusal(tc.proof).Refusal() + assert.Error(t, err) + }) + } +} + +// The exported refusal fields are what a consumer reads; the proof is what +// an eligibility decision consumes. A verdict whose fields were rewritten +// after WithRefusal describes a different refusal than it proves, and +// Refusal() refuses to hand out either. +func TestRefusalRejectsFieldsDivergingFromProof(t *testing.T) { + proof := CapabilityBoundary(ReasonUnsupportedPartitionedParent).WithCause(CauseParentBlockingIndexBuild) + tests := []struct { + name string + mutate func(*Verdict) + }{ + {"class", func(v *Verdict) { v.Class = ClassByDesign }}, + {"reason", func(v *Verdict) { v.Reason = ReasonIndexStatement }}, + {"owner", func(v *Verdict) { v.Owner = OwnerDirectOperator }}, + {"cause", func(v *Verdict) { v.Cause = CauseParentConcurrentIndexBuild }}, + {"cause cleared", func(v *Verdict) { v.Cause = CauseNone }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + v := Verdict{}.WithRefusal(proof) + _, err := v.Refusal() + require.NoError(t, err, "the unmodified verdict agrees with its proof") + tc.mutate(&v) + _, err = v.Refusal() + assert.ErrorContains(t, err, "diverge from proof") + }) + } +} + func TestJSONRoundTrip(t *testing.T) { v := Verdict{ Outcome: OutcomeRefused,