capabilities: mark rows migrate refuses as refused

[Kiran01bm]

The capability matrix no longer says migrate runs four operations it always refuses, and a test now keeps every migrate: supported row honest.

Why

capabilities --json is what an orchestrator reads to decide what to submit. ATTACH PARTITION, DETACH PARTITION [CONCURRENTLY], DROP INDEX, and REINDEX were published as T1 with migrate: supported, but migrate refuses each of them with exit 2, so a consumer trusting the matrix submits statements that can never run.

What

• attach-partition and detach-partition-concurrently become T2 (planned), migrate: refused with the reason migrate actually emits (not-native-safe-rewrite-required, unsupported-statement).
• drop-index and reindex become T3, migrate: refused with index-statement; the notes keep the --accept-blocking exit-3 path.
• diff is not_applicable for attach, detach, and reindex, which a desired schema cannot express; drop-index keeps diff: supported.
• docs/capabilities.md regenerated; docs/limitations.md and the README synced.
• TestMigrateSupportedRowsExecute runs a representative statement for every migrate: supported row through migrate.Run against Postgres and fails if any is refused; a new supported row with no statement mapped fails even under SKIP_INTEGRATION.

Before / after

Only the published matrix changes; migrate behaves exactly as before. The example is the attach row, which an orchestrator would most likely act on.

ALTER TABLE events ATTACH PARTITION events_low FOR VALUES FROM (0) TO (1000)

Before                                       After

capabilities --json says:                    capabilities --json says:
attach-partition, tier t1,                   attach-partition, tier t2,
migrate: supported                           migrate: refused,
                                             reason not-native-safe-rewrite-required
        │                                            │
        ▼                                            ▼
the orchestrator trusts it and               the orchestrator reads the refusal
submits the statement                        and does not submit it
        │                                            │
        ▼                                            ▼
migrate refuses it, exit 2:                  nothing is submitted that migrate
the matrix promised something                would refuse: the matrix and
migrate never does                           migrate agree

The same holds for DETACH PARTITION, DROP INDEX, and REINDEX.

Release note: capability matrix corrections (pg-sprite capabilities --json)

Four existing rows published migrate: supported for statements migrate refuses by default. They now match the shipped behaviour:

• attach-partition: T1 → T2 (🟡), migrate: refused (not-native-safe-rewrite-required), diff: supported → not_applicable.
• detach-partition-concurrently: T1 → T2 (🟡), migrate: refused (unsupported-statement), diff: supported → not_applicable.
• drop-index: T1 → T3 (❌), migrate: refused (index-statement); diff stays supported.
• reindex: T1 → T3 (❌), migrate: refused (index-statement), diff: supported → not_applicable.

Two rows now publish the refusal_reason that migrate actually emits: add-column-with-inline-unique-primary-key-references-check is not-native-safe-rewrite-required (was unsupported-statement), and enum-value-rename-removal is unsupported-statement (was backend-unavailable).

This corrects wrong data; the JSON schema is unchanged. A consumer that selects rows by tier == "t1", front_doors.migrate == "supported", front_doors.diff == "supported", or a specific refusal_reason gets a different set of rows after upgrading. The ❌ mark now reads "no online mechanism pg-sprite will drive": either PostgreSQL has none, or the only online form is a direct statement the operator runs (DROP INDEX CONCURRENTLY, REINDEX ... CONCURRENTLY).

diff is not_applicable for attach and detach because a desired file describes state, not a transition, so diff never plans an ATTACH or DETACH. A desired CREATE TABLE ... PARTITION OF is refused under the create-table-partition-of row (diff: refused).

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-17T04:26:13.219004Z	8d52f94	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

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

[aparajon]

🤖 Review 1/2 (adversarial correctness) of 8d52f94: 0 blocking, 4 non-blocking.

I read the whole diff (YAML, generated Markdown, limitations.md, README, the new integration test) against migrate's routing, the statement gate, and the sequence executor, built the head binary, and drove all four flipped rows (plus a sample of the other migrate: refused rows) through migrate.Run and the CLI on PostgreSQL 16. CI is green on every PostgreSQL major, so the version-gated row (18) is covered there even though I only had 16 locally.

Verdict: the four flips are correct. Under default options migrate refuses each one with exactly the token the row now publishes (probe output below), and the tier/class pairing agrees with RF-7: ATTACH is capability-boundary (T2), DETACH is capability-boundary (T2), a plain DROP INDEX/REINDEX is by-design (T3 ❌). This upholds RF-5 (a refused row is never presented as runnable) and the AGENTS.md rule that a capability change updates the matrix, limitations.md, and the README together. The supported-to-executes test does what it says: I reverted each of the four rows to its base block one at a time and TestMigrateSupportedRowsExecute failed every time with the engine's own refusal in the message; removing a claim fails even under SKIP_INTEGRATION=1. Nothing here weakens an invariant, and I found nothing blocking.

Probe (PostgreSQL 16, default options)	Outcome	Reason	Class
ATTACH PARTITION	refused	not-native-safe-rewrite-required	capability-boundary
DETACH PARTITION and DETACH PARTITION ... CONCURRENTLY	refused	unsupported-statement	capability-boundary
plain DROP INDEX / REINDEX INDEX	refused	index-statement	by-design
DROP INDEX CONCURRENTLY / REINDEX INDEX CONCURRENTLY	refused	index-statement	no-online-safety-problem, owner direct-operator

Blocking

None.

Non-blocking

1. Nothing guards the reverse direction, and two rows already published a reason the engine does not emit. The new test proves supported rows execute. No test proves a migrate: refused row is refused, or with the published refusal_reason, so the refusal half of the matrix can drift silently. Two mutants survived the whole pkg/capabilities suite: setting the attach row's refusal_reason to index-statement, and re-publishing drop-column as migrate: refused (T2) with the Markdown regenerated. Driving the refused rows through migrate.Run also found two existing rows whose token is not what migrate emits (both pre-existing, not touched by this PR, so non-blocking):

• add-column-with-inline-unique-primary-key-references-check publishes unsupported-statement; ADD COLUMN code int UNIQUE (and PRIMARY KEY, REFERENCES, CHECK) is refused as not-native-safe-rewrite-required.
• enum-value-rename-removal publishes backend-unavailable; ALTER TYPE ... RENAME VALUE is refused as unsupported-statement (DROP VALUE does not parse at all).

TestPartitionedParentRowsCarryTheEngineReason already treats the published token as something consumers filter on, so the same guard belongs on every refused row that has a representative statement. Suggested fix: a migrateRefusals map next to migrateClaims, asserted as below, and correct the two tokens (or the engine) where it fails.

Test that passes for the four rows this PR flips, fails on the two drifted rows at 8d52f94, and kills the wrong-token mutant

// migrateRefusals maps rows published as `migrate: refused` to statements the
// migrate front door must refuse with the row's refusal_reason.
var migrateRefusals = map[string]migrateClaim{
	"attach-partition": {
		setup: `CREATE TABLE %[1]s.events (id bigint NOT NULL, PRIMARY KEY (id)) PARTITION BY RANGE (id);
			CREATE TABLE %[1]s.events_low (id bigint NOT NULL, PRIMARY KEY (id))`,
		statements: []string{`ALTER TABLE %[1]s.events ATTACH PARTITION %[1]s.events_low FOR VALUES FROM (0) TO (1000)`},
	},
	"detach-partition-concurrently": {
		setup: `CREATE TABLE %[1]s.events (id bigint NOT NULL, PRIMARY KEY (id)) PARTITION BY RANGE (id);
			CREATE TABLE %[1]s.events_low PARTITION OF %[1]s.events FOR VALUES FROM (0) TO (1000)`,
		statements: []string{
			`ALTER TABLE %[1]s.events DETACH PARTITION %[1]s.events_low`,
			`ALTER TABLE %[1]s.events DETACH PARTITION %[1]s.events_low CONCURRENTLY`,
		},
	},
	"drop-index": {
		setup: `CREATE TABLE %[1]s.accounts (id bigint PRIMARY KEY, email text NOT NULL);
			CREATE INDEX accounts_email_idx ON %[1]s.accounts (email)`,
		statements: []string{
			`DROP INDEX %[1]s.accounts_email_idx`,
			`DROP INDEX CONCURRENTLY %[1]s.accounts_email_idx`,
		},
	},
	"reindex": {
		setup: `CREATE TABLE %[1]s.accounts (id bigint PRIMARY KEY, email text NOT NULL);
			CREATE INDEX accounts_email_idx ON %[1]s.accounts (email)`,
		statements: []string{
			`REINDEX INDEX %[1]s.accounts_email_idx`,
			`REINDEX INDEX CONCURRENTLY %[1]s.accounts_email_idx`,
		},
	},
	"enum-value-rename-removal": {
		setup:      `CREATE TYPE %[1]s.mood AS ENUM ('happy', 'sad')`,
		statements: []string{`ALTER TYPE %[1]s.mood RENAME VALUE 'sad' TO 'blue'`},
	},
	"add-column-with-inline-unique-primary-key-references-check": {
		setup:      `CREATE TABLE %[1]s.accounts (id bigint PRIMARY KEY)`,
		statements: []string{`ALTER TABLE %[1]s.accounts ADD COLUMN code int UNIQUE`},
	},
}


// A row published as `migrate: refused` must be refused by migrate.Run with
// exactly the published refusal_reason.
func TestMigrateRefusedRowsRefuse(t *testing.T) {
	rows, err := capabilities.Rows()
	require.NoError(t, err)
	byID := map[string]capabilities.Row{}
	for _, row := range rows {
		byID[row.ID] = row
	}

	url := testutil.StartPostgres(t)
	pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: url})
	require.NoError(t, err)
	defer pool.Close()
	for id, claim := range migrateRefusals {
		t.Run(id, func(t *testing.T) {
			row, ok := byID[id]
			require.True(t, ok, "row %s is not in the registry", id)
			require.Equal(t, capabilities.DoorRefused, row.FrontDoors.Migrate)
			schema := testutil.NewSchema(t, pool)
			_, err := pool.Exec(t.Context(), fmt.Sprintf(claim.setup, schema))
			require.NoError(t, err)
			for _, sql := range claim.statements {
				sql = fmt.Sprintf(sql, schema)
				st, err := statement.ParseOne(sql)
				require.NoError(t, err)
				v, err := migrate.Run(t.Context(), pool, st, migrate.DefaultOptions())
				require.NoError(t, err, sql)
				assert.Equal(t, verdict.OutcomeRefused, v.Outcome, sql)
				assert.Equal(t, row.RefusalReason, v.Reason, "%s: the matrix publishes a different reason than migrate emits", sql)
			}
		})
	}
}

(Lives in package capabilities_test beside front_door_integration_test.go and reuses its migrateClaim type and imports.)

--- FAIL: TestMigrateRefusedRowsRefuse/enum-value-rename-removal (expected backend-unavailable, actual unsupported-statement) and --- FAIL: TestMigrateRefusedRowsRefuse/add-column-with-inline-unique-primary-key-references-check (expected unsupported-statement, actual not-native-safe-rewrite-required) on 8d52f94; attach-partition, detach-partition-concurrently, drop-index, reindex all PASS. With the attach row's token mutated to index-statement the attach subtest fails (expected index-statement, actual not-native-safe-rewrite-required). Run locally against PostgreSQL 16.

2. migrateClaims still carries statements for the four rows the PR just flipped to refused. The map's own comment says it covers "every capability row whose imperative migrate door is published as supported" (L32-L35), but the entries at drop-index L165, reindex L177, attach-partition L189, and detach-partition-concurrently L203 are statements migrate refuses. They are never executed, because the loop at L231 only visits supported rows, and nothing ties the map's keys to supported rows, so a stale entry (or the mutant above that flips drop-column to refused) is invisible. Suggested fix: delete the four entries (the refusal map above is where those statements belong) and assert that every key is a supported row.

Test that fails on 8d52f94 until the four entries move

// A representative statement is only evidence for a row the matrix publishes
// as supported.
func TestMigrateClaimsBelongToSupportedRows(t *testing.T) {
	rows, err := capabilities.Rows()
	require.NoError(t, err)
	for id := range migrateClaims {
		i := slices.IndexFunc(rows, func(r capabilities.Row) bool { return r.ID == id })
		require.GreaterOrEqual(t, i, 0, "row %s is not in the registry", id)
		assert.Equal(t, capabilities.DoorSupported, rows[i].FrontDoors.Migrate, "migrateClaims has statements for row %s, which is not published as migrate: supported", id)
	}
}

--- FAIL: TestMigrateClaimsBelongToSupportedRows on 8d52f94 (migrateClaims has statements for row drop-index, reindex, attach-partition, detach-partition-concurrently, none of which is published as migrate: supported). With drop-column mutated to refused it fails on drop-column. Run locally.

3. --force runs ATTACH and plain DETACH, but the new text says nothing executes and sends the operator outside migrate. Attach notes say "Typed refusal, nothing executed", limitations.md L17 says to "attach outside migrate", and L18 says DETACH is refused "before anything executes". The refusal is the unforced path. The attach refusal is not-native-safe-rewrite-required, which --force SCHEMA.TABLE overrides, so the blocking form does run, and so does a plain DETACH PARTITION (only DETACH ... CONCURRENTLY stays refused under --force). That is correct under RF-5 (a loud, typed, audited acknowledgement), but a consumer or operator reading the row learns neither that the override exists nor that it takes the blocking lock. Other rows in the same file already name --force where it does or does not apply. Suggested fix: add one clause to each of the two rows and the two limitations.md lines. Probe (not wired into a test, so no test block):

Probe output on 8d52f94 (PostgreSQL 16, `Options.Force = "SCHEMA.TABLE"`)

attach (force)            outcome=executed-natively  detail=forced: the submitted form ran as-is under budgets (lock 3s, statement 30s), overriding the engine's routing
detach plain (force)      outcome=executed-natively  detail=forced: the submitted form ran as-is under budgets ...
detach concurrently (force) outcome=refused reason=unsupported-statement

4. migrate --dry-run and diff still tell the consumer the refused operations will run. The matrix is now right, but two other surfaces of the same binary contradict it, and capabilities.md L85-L86 promises that migrate and its dry-run "exit with code 0 only when the change is executable through an online-safe path". Both are pre-existing; the PR's DETACH note mentions the dry-run rewrite but not the exit code:

• migrate --alter 'ALTER TABLE public.ev DETACH PARTITION public.ev1' --dry-run prints "pg-sprite will run a safer online sequence instead" and "1 step to run, 0 refused" and exits 0; the real run exits 2 (refused (unsupported-statement), class capability-boundary). The same holds for DETACH ... CONCURRENTLY. (ATTACH is consistent: dry-run reports "1 refused".) safer-sequences.md records the DETACH gap as a known limitation, so a CI gate that trusts the dry-run exit code is exactly what the matrix row should warn about.
• diff against a live CREATE INDEX that the desired file omits prints DROP INDEX public.accounts_email_idx;, "pg-sprite migrate will run a safer online sequence instead: DROP INDEX CONCURRENTLY", "1 step to run, 0 refused", and "run each statement via pg-sprite migrate --alter '…', which refuses blocking forms and substitutes safer online sequences". migrate --alter refuses that statement (index-statement, exit 2). The drop-index row's diff: supported is accurate about what diff emits, but the loop it advertises (diff, then migrate) cannot complete.

Suggested fix: either make the dry-run apply sequence admission and the diff hint name the DROP INDEX CONCURRENTLY direct-run path, or, at minimum, state both facts in the rows' notes so a consumer does not treat the dry-run exit code as a submit gate. Not a regression from this PR.

Verified

• Every cited line checked against 8d52f94; the head is a merge of main into the PR branch, and the diff against the merge base is the six files listed.
• Generated Markdown is in sync (TestCheckedInMarkdownIsGenerated), the summary counts (15 / 26 / 14 / 4 of 59) match the YAML, and capabilities --json serializes the four rows as described.
• Every other imperative migrate: refused row I could drive without a fixture (volatile default, constrained domain, stored generated column, SET EXPRESSION, general type change, SET TABLESPACE, EXCLUDE, index build on a partitioned parent, USING INDEX on a partitioned parent, PARTITION OF, DROP TABLE, views, triggers, extensions, grants, sequences, publications, DML, enum and function creation) is refused with its published token apart from the two in finding 1.

Mutant	Caught by
attach-partition reverted to its base block (migrate: supported)	TestMigrateSupportedRowsExecute
detach-partition-concurrently reverted to base	TestMigrateSupportedRowsExecute
drop-index reverted to base	TestMigrateSupportedRowsExecute
reindex reverted to base	TestMigrateSupportedRowsExecute
drop-column claim removed from migrateClaims (SKIP_INTEGRATION=1)	TestMigrateSupportedRowsExecute (no representative statement)
a nonexistent id added to declarativeOnlyClaims	survives (harmless; the list is not checked against the registry)
attach row's refusal_reason changed to index-statement (Markdown regenerated)	survives the whole suite (finding 1)
drop-column re-published as migrate: refused, T2 (Markdown regenerated)	survives the whole suite (finding 1, 2)

Mutations ran in a throwaway worktree; nothing was pushed.

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

[aparajon]

🤖 Review 2/2 (OSS adoption and integration ease, for 8d52f94): 0 blocking, 3 non-blocking.

This is the second lens: what an adopter or an importer (an orchestrator reading pg-sprite capabilities --json, or a human reading capabilities.md) experiences. Correctness is in comment 1/2. The headline is good for adoption: a consumer that trusted the old matrix would have submitted statements that can never run, and the published support claim now matches what migrate does by default. The remaining items are about the vocabulary the four flips stretch, and about telling consumers what changed.

Blocking

None.

Non-blocking

1. The ❌ mark now has two meanings, and one row spans two refusal classes. capabilities-contract.md L33-L36 defines ❌ as "out of scope because PostgreSQL has no online mechanism", and the legend at capabilities.md L196, the engine-path legend near L157, and the jq comment at L60-L61 all repeat it. DROP INDEX and REINDEX are now ❌ although PostgreSQL has CONCURRENTLY forms for both. The PR patches one paragraph (L115-L118) and rewrites the generated summary to "no online mechanism for pg-sprite to build on", so the page now says two different things about the same mark, and a consumer who filters status_mark == "❌" to mean "waiting is pointless, PostgreSQL has no answer" will include rows that have one. Two related edges:

• The contract's mark-to-class table, L214-L217 is one-to-one (T3 ❌ is by-design). These rows refuse a plain form as by-design but a concurrent form as no-online-safety-problem with owner direct-operator (comment 1/2, probe table), so a consumer deriving the route from the row gets the right answer for the plain form and the wrong one for the concurrent form.
• The concurrent-form detail text says pg-sprite "does not drive this maintenance form yet" (verdicts.go L49), while capabilities.md L127 states that T3 rows deliberately have no roadmap item. "Yet" reads as a T2 promise.

Suggested fix: pick one reading and make it uniform. Either extend the ❌ definition in the three hand-written places and the contract to "no online mechanism pg-sprite will drive: PostgreSQL has none, or the only online form is a direct statement the operator runs", and drop "yet" from the detail; or split each row into a plain and a concurrent row so each carries one class. The first is a docs-and-string change.

2. "migrate never runs this statement" is contradicted by the same cell. The drop-index and reindex notes, and limitations.md L16 ("migrate never runs index maintenance"), go on to say a single-relation plain form runs under --accept-blocking and exits 3. Both are true only if "runs" means "runs online-safely". An adopter scanning the matrix for "can this tool run REINDEX for me" reads the first sentence and misses the escape hatch, or reads the second and distrusts the first. Suggested fix: "migrate never runs this statement online-safely".

3. Say what changed for existing consumers. Four existing ids changed value in a published, versioned JSON surface: attach-partition and detach-partition-concurrently moved from T1 to T2, drop-index and reindex moved from T1 to T3, migrate flipped to refused on all four, and diff on attach-partition, detach-partition-concurrently, and reindex flipped from supported to not_applicable. The contract treats a changed meaning as a compatibility boundary (L199-L200); this is a correction of wrong data rather than a schema change, so it does not need one, but a pinned consumer that selects tier == "t1" or diff == "supported" will silently lose rows after upgrading. A line in the release notes naming the four ids and the three diff flips is enough. Related: diff: not_applicable on attach and detach is only partly true. A desired file that lists a partition is rejected by diff at plan time (a desired CREATE TABLE ... PARTITION OF cannot be planned: building it would lock the live partitioned parent), and the create-table-partition-of row publishes diff: refused, so diff can identify these shapes. The contract defines not_applicable as "an operation that a door cannot express" and refused as "that door can identify and reject it" (L112-L114); read strictly, that points at refused for attach and detach. Worth a sentence in the release notes either way.

Verified

• The README sentence, limitations.md rows, capabilities.md table rows, and YAML notes agree with each other on which statement is refused with which token, and with safer-sequences.md and low-level-design.md on the DETACH limitation.
• The AGENTS.md capability-statement rule is met: matrix, limitations.md, and README move together in this PR.
• OSS hygiene: the diff and this comment reference only public repository content.
• Docs-only dispositions against docs/invariants.md: RF-5 upheld, RF-7 upheld (tier and class pairing checked above); no entry is added, weakened, or moved.

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

[aparajon]

🤖 Stamping with comments: 0 blocking, 7 non-blocking (see the two review comments above).

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

[Kiran01bm]

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

Verdict: all 7 non-blocking findings addressed in a follow-up commit that also merges main; one sub-point answered rather than changed.

#	Concern	Status
C1-F1	nothing proves a migrate: refused row is refused with its published refusal_reason; two rows already drifted	fixed — migrateRefusals and TestMigrateRefusedRowsRefuse added as suggested (front_door_refusal_integration_test.go); add-column-with-inline-unique-primary-key-references-check now publishes not-native-safe-rewrite-required and enum-value-rename-removal publishes unsupported-statement; the test fails on the old tokens
C1-F2	migrateClaims still carries statements for the four rows flipped to refused	fixed — the four entries moved to migrateRefusals, and TestMigrateClaimsBelongToSupportedRows fails on any claim for a row not published as supported
C1-F3	--force runs ATTACH and plain DETACH, but the text says nothing executes	fixed — the attach and detach notes and limitations.md now scope "refused, nothing executed" to the unforced path, say --force SCHEMA.TABLE runs the blocking form as submitted under bounded budgets, and that DETACH ... CONCURRENTLY stays refused under --force
C1-F4	migrate --dry-run of DETACH exits 0 and diff's hint for a removed index ends in a refusal	fixed (docs) — the detach row and limitations.md say the dry-run exit code is not a submit gate for it, and the drop-index row says to run DROP INDEX CONCURRENTLY directly instead of the migrate --alter hint; changing the dry-run and hint behaviour is left out of this docs PR
C2-F1	❌ now has two meanings, and drop-index/reindex span two classes	fixed — ❌ now reads "no online mechanism pg-sprite will drive: PostgreSQL has none, or the only online form is a direct statement the operator runs" in the contract and the hand-written places in capabilities.md; the contract notes that the concurrent forms refuse as no-online-safety-problem (direct-operator); the "yet" in the concurrent-form detail is dropped in #189, which owns indexAdvice
C2-F2	"migrate never runs this statement" is contradicted by --accept-blocking in the same cell	fixed — "never runs this statement online-safely" in the drop-index and reindex notes and limitations.md
C2-F3	existing consumers are not told four ids, three diff flips, and two tokens changed	fixed — a release-note section in the PR description lists every changed id and token; diff: not_applicable is kept for attach and detach because a desired file describes state, not a transition, so diff never plans either, and the desired PARTITION OF refusal belongs to create-table-partition-of (diff: refused)

Source: block/pg-sprite#188, review comments 6229736338 and 6229736931 and review 5556242609 at head 8d52f94.