verdict: classify every refusal with a typed class and owner

[Kiran01bm]

Stamp every refusal verdict with a typed class (and an owner where there is no online-safety problem to solve), classified by a registry keyed on the typed cause or refusal site, and carry both through the plan report as format_version 4.

Why

A refusal reason names the immediate cause but not the kind of boundary reached: unsupported-statement alone covers a data backfill (another tool's job), an imperative CREATE TABLE (the declarative front door's job), CREATE INDEX IF NOT EXISTS (permanently refused, with a safer idiom), and a parse/route incoherence (pg-sprite's own defect). unsupported-partitioned-parent spans three classes along its PartitionRefusalCause. Consumers were rebuilding that split from reason strings; docs/refusal-classes.md decided the vocabulary and this change makes the engine emit it.

What

• pkg/verdict: Refusal proof type with unexported fields. NewRefusal(class, reason, owner) rejects a zero/unknown class, unknown reason, or an owner outside no-online-safety-problem; per-class constructors (CapabilityBoundary, NoOnlineSafetyProblem, ByDesign, Environmental, InvariantViolation) are total for valid inputs — no panic path. Verdict.WithRefusal is the one place a verdict gets outcome: refused + reason + class + owner together (// INV: RF-7); Verdict.Refusal() reconstructs the proof from a refused verdict so an aggregator (the desired-state loop) propagates class and owner through that same path instead of copying fields.
• Registry, two halves: pkg/plan/refusal.go classifies plan-side keys (CreateShapeRefusal(cause), PartitionRefusal(cause), RouteRefusal()); pkg/migrate/refusal_registry.go classifies statement kinds at the gate, both admission sentinel sets, and each imperative refusal site. Nothing is derived from the reason string.
• TestRefusalRegistryIsComplete derives its keys from production closed sets (executor.CreateShapeCauses(), preflight.PartitionRefusalCauses(), statement.Kinds() × concurrent, the sentinel sets, a walk of the site refusals) and pins a sentinel subset so a broken deriver cannot pass on an empty set. pkg/statement gains Kinds(), whose closed-set test parses the iota block with go/ast so a kind appended to the source but left out of Kinds() fails; kinds are split so KindOther no longer absorbs provisioning / catalog-work / data-change statements.
• Plan refusals fail closed: RefuseUnsupportedPartitionedParent(report, causes) error replaces the bool form and returns executor.ErrInvariantViolation on a length mismatch or unclassified cause; a plan statement refused without a class keeps its reason and is reported invariant-violation. The desired-state loop returns ErrInvariantViolation if a refused statement verdict carries no valid class.
• Plan report contract: format_version 4. class and owner join Report (present exactly when the aggregate reason is) and Statement (present exactly when disposition is refuse; excluded from the fingerprint). docs/plan-report.md documents both fields and both closed vocabularies; TestDocListsEveryVocabularyValue walks verdict.Classes()/Owners(), and pkg/verdict/docs_test.go guards the same values against docs/refusal-classes.md.
• CLI: the run verdict text renderer prints class: / owner:; the plan text renderers (migrate --dry-run, diff) print a refusal class: …; owner: … note under each refused statement's error diagnostic. JSON gains class / owner on verdicts, plan statements, and plan reports (omitempty). demo/tour.sh asserts format_version 4 and the class alongside reason and cause.
• Docs: RF-7 registered in docs/invariants.md; docs/refusal-classes.md rewritten from proposal to shipped behavior; docs/limitations.md and README no longer claim every refusal is an online-safety gap; the partitioned-parent NOT VALID FK matrix row corrected to ✅ with a server-version precondition (environmental on 14–17), matching the cause table.

Exit codes and every existing reason string are unchanged.

Before / after

One refused statement, seen through both front doors.

$ pg-sprite migrate --alter 'UPDATE app.t SET v = 1' --json

before                                       after
{                                            {
  "outcome": "refused",                        "outcome": "refused",
  "reason": "unsupported-statement",           "reason": "unsupported-statement",
  "detail": "only ALTER TABLE and ..."         "class": "no-online-safety-problem",
}                                              "owner": "data-change-runner",
                                               "detail": "only ALTER TABLE and ..."
                                             }

$ pg-sprite migrate --alter 'CREATE INDEX events_created_idx ON events (created)' --dry-run
  (events is a partitioned parent)

before                                       after
error[unsupported-partitioned-parent]:       error[unsupported-partitioned-parent]:
  refused — the target is a partitioned        refused — the target is a partitioned
  table ...                                    table ...
                                             note:
                                               refusal class: capability-boundary

  --json: "format_version": 3, no class        --json: "format_version": 4,
                                                       "class": "capability-boundary"
                                                       on the report and the statement

Desired-state run whose planned statement is refused at execution time
(table crossed the size policy after planning):

before                                       after
{ "outcome": "refused",                      { "outcome": "refused",
  "reason": "not-native-safe-table-too-        "reason": "not-native-safe-table-too-
            large",                                      large",
  "verdicts": [{ ..., "class":                 "class": "environmental",
                 "environmental" }] }          "verdicts": [{ ..., "class":
                                                               "environmental" }] }

CREATE INDEX ... on partitioned parent   -> class: capability-boundary
ADD CONSTRAINT ... USING INDEX on parent -> class: by-design
ADD FOREIGN KEY ... NOT VALID on parent  -> class: environmental (PG < 18)
CREATE INDEX IF NOT EXISTS (greenfield)  -> class: by-design
plan statement refused, cause unknown    -> class: invariant-violation (fail closed)

Refs: docs/refusal-classes.md, docs/invariants.md RF-7, docs/plan-report.md

🤖 Drafted with Amp (Claude Opus 4.6); reviewed and edited by the author.

[JashLal]

Reviewed the full diff. The design holds together well: verdict.Refusal as a proof type with unexported fields, WithRefusal/DesiredResult.refused as the only paths onto the verdict contract, and TestRefusalRegistryIsComplete deriving its keys from production closed sets (with sentinel pinning against a vacuous walk) rather than a shadow list. Fail-closed handling on unclassified causes and the RefuseUnsupportedPartitionedParent signature change are consistent across all three callers, docs guards were updated, and CI is green on PG 14–18. Fingerprints and reason strings are unchanged as claimed.

Non-blocking observations:

1. ParseOne classifies DROP TABLE (any non-index DROP) as KindCatalogWork, so the gate refuses it with no-online-safety-problem / direct-operator. Defensible under the vocabulary (destructiveness ≠ lock safety), but the verdict now nudges an operator to run a destructive statement directly, and the detail text still only says "only ALTER TABLE and CREATE INDEX statements are supported". Worth a follow-up either to the detail wording or a destructive-aware kind.
2. Verdict.String() and writeVerdictText print class: unconditionally for refused verdicts, so a verdict decoded from an older producer's JSON renders an empty class line. Cosmetic.
3. In refuseStatements, the report-level stamp takes the current mutator's refusal even when the selected statement was already refused with a different classification, so the report class can disagree with the first statement's class. Same shape as the pre-existing reason behavior, just noting it now extends to class/owner.

[chatgpt-codex-connector]

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

[Kiran01bm]

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

All five findings addressed in f7fa089: the blocking RF-7 gap on the desired-state path is closed through one path, the plan report moves to format_version 4 with documented and guarded class/owner vocabularies, the Kinds() closed-set test parses the const block, the plan text renderers print class and owner, and Classes()/Owners() get a docs guard.

#	Finding	Status	Explanation
1	Desired-state convergence loop drops Class/Owner on an execution-time refusal (blocking)	Fixed	Verdict.Refusal() reconstructs the classified refusal from a refused verdict by re-validating through NewRefusal, so the aggregate is stamped through the same result.refused(...) path every other site uses instead of copying fields. A refused verdict that does not validate (a state this build cannot produce) returns an executor.ErrInvariantViolation-wrapped failed result rather than an unclassified refusal. TestRunDesired/stops at an execution-time refusal now asserts res.Class == environmental, class/owner equal to the refusing verdict's, and "class":"environmental" on the wire; reverting the fix to Reason-only fails it. TestRefusalRoundTripsThroughVerdict covers the accessor, including the not-refused error.
2	class/owner land in the plan-report contract without a format_version bump or doc rows	Fixed	FormatVersion = 4, comment updated with what v4 added. docs/plan-report.md: "current version is 4" (guarded by TestDocStatesCurrentFormatVersion), class/owner rows in Report fields (present exactly when reason is, i.e. target-dependent refusals) and Statement fields (present exactly when disposition is refuse; excluded from the fingerprint), new ### Classes and ### Owners vocabulary sections copied from docs/refusal-classes.md, and the versioning paragraph lists classes/owners among the pinned vocabularies. TestDocListsEveryVocabularyValue now walks verdict.Classes() and verdict.Owners(). Every format_version 3 pin moved to 4: demo/tour.sh (4 asserts), plan_test.go JSON-shape fixtures and comments, the example JSON in docs/plan-report.md, docs/cli-output-examples.md, and the two docs/schemabot-integration.md references now read "3 and later". make demo-check passes against the built binary.
3	Kinds() closed-set test is a length check, so a kind appended past KindCatalogWork ships unclassified	Fixed	TestKindsIsClosedAndNamed now parses statement.go with go/ast (same pattern as TestPartitionRefusalCausesEnumerateEveryDeclaredCause), locates the const block KindOther Kind = iota opens, checks each later spec takes its value from iota, and asserts Kinds() as a set equals exactly the declared values. It also asserts every non-KindOther kind has a String() other than "other", so a kind missing from the switch cannot hide under the catch-all name. Mutation check: appending a scratch KindTruncate to the iota block fails the test with "Kinds() must enumerate exactly the Kind constants statement.go declares".
4	Plan-report text renderer prints neither class nor owner, contradicting docs/refusal-classes.md	Fixed	writeRefusal's DispositionRefuse arm now emits a note: line refusal class: <class> plus ; owner: <owner> when set, immediately after the leading error diagnostic, for both migrate --dry-run and diff text output (they share the writer). A statement the plan leaves unclassified prints no note rather than a blank one. Tests: the partitioned-parent and planner-refusal dry-run cases and the greenfield diff case assert the note; a new TestDryRunTextRefusalClassAndOwner covers the owner branch and the unclassified-prints-nothing branch.
5	Classes()/Owners() have no docs guard	Fixed	TestRefusalClassesDocListsEveryClassAndOwner in pkg/verdict/docs_test.go walks both accessors and requires a `

Decisions to veto: bumped format_version to 4 rather than treating class/owner as an additive v3 change — the doc mandates a bump for any new closed vocabulary, and downstream consumers pin the version. The plan report still carries no class on rewrite-required / unavailable statements (those dispositions have no refusal class in the plan contract today), so the text renderer prints none for them; that is a pre-existing gap, not introduced here.

Source: #97, scratch review scratch/code-reviews/pg-sprite-pr97-review.md at head 28c63ad; fixes in f7fa089.

[aparajon]

🤖 Adversarial correctness review, requested by Armand and performed by Armand's agent. Reviewed at head f7fa089.

Verdict: correct and safe to land. The proof type, the two registry halves, and the fail-closed paths hold under attack: every refusal site now reaches the verdict through WithRefusal, the desired-state loop reconstructs the class through Verdict.Refusal() and fails closed into ErrInvariantViolation when it cannot, and the fingerprint excludes class/owner. The one substantive finding is upstream of the registry: the parse boundary that feeds it assigns kinds from a hand-enumerated node list, so sibling statements of the same matrix row land in different classes and the JSON contradicts docs/refusal-classes.md. Nothing blocks; @JashLal's three observations above are not repeated here.

Findings

1. The kind classifier splits siblings across classes, and the doc's own tables say otherwise. Driving statement.ParseOne + migrate.Gate over a spread of statements at head: CREATE ROLE r → no-online-safety-problem/provisioning but DROP ROLE r → capability-boundary; CREATE POLICY / CREATE PUBLICATION → provisioning but DROP POLICY / DROP PUBLICATION → direct-operator and DROP SUBSCRIPTION → capability-boundary; CREATE VIEW, CREATE EXTENSION, CREATE SEQUENCE, DROP TYPE, DROP DOMAIN, DROP SCHEMA → direct-operator but ALTER VIEW, ALTER EXTENSION, ALTER SEQUENCE, ALTER INDEX, CREATE TYPE, CREATE DOMAIN, CREATE SCHEMA, CREATE MATERIALIZED VIEW, REFRESH MATERIALIZED VIEW, TRUNCATE → capability-boundary. Two mechanisms: the DropStmt branch maps every non-index drop to KindCatalogWork regardless of RemoveType (so a policy or publication drop skips the provisioning row), and the statements that parse as their own node types (DropRoleStmt, DropSubscriptionStmt, DropOwnedStmt, CreateSchemaStmt, CreateDomainStmt, CreateEnumStmt, CompositeTypeStmt, AlterSeqStmt, AlterEnumStmt, AlterExtensionStmt, CreateTableAsStmt, RefreshMatViewStmt, TruncateStmt) plus the non-table AlterTableStmt/RenameStmt/AlterObjectSchemaStmt branches all fall to KindOther, whose registry entry means "the engine may learn to route it" — false for every one of them. docs/refusal-classes.md routes "roles, policies, publications, subscriptions" to provisioning and "views, extensions, standalone sequences, and other catalog work the matrix marks ⚪" to direct-operator; docs/capabilities.md marks "enum/domain type creation and drop" (⚪) and "materialized views (create and replace)" (⚪). TestRefusalRegistryIsComplete cannot see this because its key is the kind, and the statement_test.go pins for ALTER VIEW / ALTER INDEX / ALTER SEQUENCE as KindOther were written to assert "not a table target" before KindOther carried a class. This is the class-level face of @JashLal's observation 1: the same enumeration gap, but here it points a newcomer at "wait for a capability" for CREATE SCHEMA.

2. (nit) RF-7's Enforced line claims a type guarantee the type does not quite give. WithRefusal accepts the zero Refusal unchecked, and Verdict's Class/Owner/Reason stay exported (JSON decoding and tests set them directly), so "the only way a verdict acquires its refusal fields" is a convention that the registry constructors' totality and the completeness test uphold, not something the compiler does. The desired-state loop is covered because Refusal() re-validates through NewRefusal.

3. (doc nit) A sentence written before the split survived it. docs/refusal-classes.md still says the imperative front door refuses through "a single catch-all in pkg/migrate/verdicts.go (statement.KindOther), which today cannot tell a backfill from a GRANT" and that "classifying that site means the parse boundary … distinguishes the kinds". At head it does, so the paragraph describes the state this PR removes.

Action items

1. (Finding 1) In ParseOne, route the DropStmt branch by RemoveType (OBJECT_POLICY, OBJECT_PUBLICATION, OBJECT_SUBSCRIPTION, OBJECT_ROLE → KindProvisioning; everything else non-index → KindCatalogWork), add DropRoleStmt / DropSubscriptionStmt / DropOwnedStmt → KindProvisioning, add the create/alter node types listed in the finding → KindCatalogWork, and let the three non-table ALTER/RENAME/SET SCHEMA branches return KindCatalogWork instead of the zero kind. Decide TRUNCATE explicitly (data change with ACCESS EXCLUSIVE, so data-change-runner or direct-operator, not "wait for the engine").
2. (Finding 1) Decide what KindOther means now that it has a class. Nothing the gate refuses by kind is on the engine's route roadmap, so either default every parsed-but-unnamed statement to KindCatalogWork and reserve capability-boundary at the gate for nothing, or add a pinned sibling-symmetry test (CREATE / ALTER / DROP of one object family classify alike) so the enumeration cannot drift again. Update the statement_test.go pins either way.
3. (Finding 3) Rewrite the "one site still covers several rows" paragraph in docs/refusal-classes.md to describe the shipped split.
4. (optional, Finding 2) Either soften RF-7's Enforced wording to name the convention and the tests, or have WithRefusal route a zero proof to invariant-violation so the type does what the sentence says.

Verified (tried to break, couldn't)

Built, vetted, and ran the full ./pkg/... ./internal/... suite locally at f7fa089 (green, including the Docker-backed executor and preflight packages); grepped for any remaining direct Outcome: OutcomeRefused construction outside verdict.go and DesiredResult.refused (none, so every refusal reaches the contract through WithRefusal); walked execRefusal and confirmed the invalid-index path stays a non-refusal as documented; confirmed RunDesired turns an unreconstructable refused verdict into an ErrInvariantViolation failure rather than an unclassified refusal; read plan.Fingerprint (hashes SQL, route, backend, disposition, exec SQL only, so class/owner cannot move a pinned plan); checked refuseStatements keeps first-refusal-wins for statement and report and that RefuseUnsupportedPartitionedParent's length check fails closed before any statement is marked, with its one caller in diffplan updated; confirmed exit codes and every reason token are unchanged; CI green on all 15 checks including PostgreSQL 14–18; no Codex or Copilot threads open.

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

[aparajon]

🤖 Two-lens product review (OSS adoption, SchemaBot integration), requested by Armand and performed by Armand's agent. Reviewed at head f7fa089.

OSS adoption ease

1. The class note assumes the reader already knows the vocabulary. migrate --dry-run and diff now print note: refusal class: by-design and the run verdict prints class: capability-boundary, but neither line says where the five words are defined or what the reader should do with them. A first-time user meets capability-boundary with no pointer to the "consumer action" column that gives it meaning. Suggest the note (or the class: line) carry the routing verb from the doc's table, for example refusal class: by-design (use the safer idiom), or point at docs/refusal-classes.md once per report.

2. Finding 1 of the correctness review is worst for exactly this reader. CREATE SCHEMA app refused as capability-boundary tells someone evaluating the tool to wait for an engine release that will never come. Fixing the kind enumeration is an adoption fix as much as a correctness one.

SchemaBot integration

1. The format_version 4 bump is lockstep-safe. SchemaBot's Postgres engine compares report.FormatVersion against the linked library's pgplan.FormatVersion and persists no plan report across deploys, so bumping the dependency moves producer and check together; a stale report cannot exist, and an unrecognized version already renders a blocked placeholder. No fork or shim needed.

2. class/owner give the engine the seam it has been missing. SchemaBot's engine maps Disposition to a blocked execution mode with its own wording and never reads reason, so today every refusal reads as "blocked" with a per-disposition sentence. class is the typed switch that wording wants: no-online-safety-problem + owner → "this is a data change / provisioning, not a schema change; run it through that tooling", by-design → surface safer_idiom, capability-boundary → "not supported by the engine yet", environmental → "retry after fixing the environment", invariant-violation → fail the apply and log. That is a SchemaBot-side follow-up; nothing here needs to change for it, and the closed Classes()/Owners() accessors mean the engine can pin its switch as exhaustive.

3. Fail-closed direction is right for the consumer. An unclassified refusal in the desired-state loop surfaces as an ErrInvariantViolation error, not a refusal verdict, so the consumer sees a failed run rather than a refusal it might route somewhere plausible. That matches how SchemaBot treats engine errors.

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

[aparajon]

🤖 Approving on Armand's behalf after the adversarial correctness review above. The findings there are yours to pick up as follow-ups — flagging them, not gating on them.

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