[Kiran01bm]

A DROP INDEX run under --accept-blocking now also needs --allow-destructive naming the same table; without it the drop is refused with destructive-change and the refusal lists the checks to make first.

Why

--accept-blocking acknowledges the ACCESS EXCLUSIVE lock a plain DROP INDEX takes, but nothing acknowledged losing the index itself. Dropping an index the application still reads through, or one a foreign key or a uniqueness guarantee relies on, is a common mistake, and it is not cheap to undo: the fix is a full index build. The planner already treats every index drop as destructive and migrate --desired refuses it; the accepted-blocking path was the one place an index could be dropped without acknowledging the loss. This applies the "make mistakes hard to commit" principle to every index drop, unique or not.

What

• On the accepted-blocking path, once the owning table resolves, a DROP INDEX needs Options.AllowDestructive / --allow-destructive naming that table. Without it: refused, reason destructive-change, class by-design, exit 2, nothing executed. The detail asks the operator to confirm idx_scan is 0 on the primary and every replica over a full application cycle, that no foreign key relies on the index, and that nothing needs a unique index's guarantee. A mismatched acknowledgement is ErrAllowDestructiveMismatch.
• --allow-destructive and --accept-blocking now combine (they were mutually exclusive). REINDEX INDEX / REINDEX TABLE discard nothing and need only --accept-blocking.
• An acknowledged drop's verdict carries allowed_destructive: true beside blocking_passthrough: true, and the loss is audited at warn level.
• RF-10, AB-4, the CLI help, README, limitations, capabilities, refusal classes, the passthrough design, SchemaBot integration notes, the CLI output example and the demo tour are updated.

Behaviour change for callers: a DROP INDEX passed with --accept-blocking alone, which committed before, is now refused with destructive-change. Add --allow-destructive SCHEMA.TABLE (the same table) to keep running it.

Before / after

The same accepted drop of a plain, non-unique index the application may still use. The lock acknowledgement is unchanged; what changes is that the loss now needs its own.

pg-sprite migrate --alter 'DROP INDEX app.orders_created_idx' --accept-blocking app.orders --statement-timeout 10m

Before                                           After

gate refuses: index-statement                    gate refuses: index-statement
--accept-blocking names app.orders  ✓            --accept-blocking names app.orders  ✓
                                                 --allow-destructive given?  no
                                                   → refused: destructive-change, exit 2
                                                     "confirm idx_scan is 0 on the primary
                                                      and every replica, no foreign key
                                                      relies on it, ... then rerun with
                                                      --allow-destructive app.orders"
                                                     the index is still there
DROP INDEX runs under the budgets, exit 3
the index is gone, verdict says nothing          with --allow-destructive app.orders added:
about the loss                                   DROP INDEX runs under the budgets, exit 3,
                                                 verdict carries allowed_destructive: true

REINDEX INDEX and REINDEX TABLE behave exactly as before.

🤖 Generated with Amp (Claude Opus 4.6)

[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-28T23:16:22.442227Z We couldn’t complete this review. Comment @codex review to try again.	8f7ef16	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.

[JashLal]

🤖 Reviewed by Jash's AI agent at 8f7ef16d37ae77eb7a673c54fba690917092ef03. No concrete correctness blocker found in the full diff.

The new gate in pkg/migrate/accept_blocking.go:74 validates the destructive acknowledgement against the resolved owning table before entering execution. Missing acknowledgement for DROP INDEX produces the typed refusal, while the mismatch path returns an error without execution. The new integration cases explicitly check that the index remains present. The successful drop records AllowedDestructive, and the audit now receives the owning table rather than deriving a table name from an index statement. The CLI grammar, rendered example, capability documentation, and caller migration guidance consistently describe the additional acknowledgement.

Non-blocking test suggestion at pkg/migrate/accept_blocking_integration_test.go:22: the shared helper now supplies AllowDestructive for every accepted statement, including the existing REINDEX cases. Please consider an explicit REINDEX INDEX/TABLE case with AllowDestructive empty that still succeeds and leaves AllowedDestructive false. The implementation appears to preserve that behavior, but having the helper supply both flags stops these cases from guarding the documented promise that REINDEX still needs only the blocking acknowledgement.

Validation: all 16 reported CI checks pass, including PostgreSQL 14–18. I did not run a local build, tests, or the demo. Exact-head surrounding source was unavailable through the contents API, so unchanged table resolution, execution/locking helpers, and repository instructions outside the diff were not independently verified.

[JashLal]

🤖 Approving on Jash's behalf after the full-diff review at 8f7ef16. No concrete blocker found; the optional REINDEX-without-destructive-acknowledgement test and surrounding-source verification limits are recorded in the review. No local build or tests were run. Generated by Codex.

[Kiran01bm]

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

No blockers were raised; the one test suggestion is fixed in the follow-up commit.

#	Finding	Status	Explanation
C1-F1	The shared helper now passes AllowDestructive for every accepted statement, so no case guards that REINDEX still needs only the blocking acknowledgement	fixed	New subtest REINDEX needs only the blocking acknowledgement in TestRunAcceptBlockingExecutesEligibleRefusals runs REINDEX INDEX and REINDEX TABLE with AllowDestructive empty and asserts the accepted outcome, allowed_destructive false, and the index kept. It fails if the destructive gate is applied to every accepted statement.
C1-F2	Unchanged table resolution and execution/locking helpers not verified at the head	no action	The PR does not change lockedTable or executor.ExecuteAcceptedBlocking; the new gate runs after lockedTable resolves the owning table and before the executor is called, and its refusal and mismatch paths return before any execution (pkg/migrate/accept_blocking.go).

The verdict, CI status, and approval body in review 5691388310 are confirmations; no action.

Source: block/pg-sprite#276, review comment 6484323671 and review 5691388310 at head 8f7ef16d; fixes in the follow-up commit