migrate: refuse a destructive statement unless --allow-destructive names the table

README.md

@@ -40,7 +40,8 @@ advisory `suggest` command are implemented. `pg-sprite migrate --alter '…'` cl
 routes the statement, then executes the routed SQL — the planner's safer native sequence by
 default when the submitted form blocks (reported in the verdict's `executed_sql`), a bounded
 optimistic native attempt otherwise. A gated `--force` runs the submitted form as-is under the
-same budgets. Changes without an available backend get a structured refusal (exit code 2).
+same budgets, and a statement that discards data runs only when `--allow-destructive` names
+its table. Changes without an available backend get a structured refusal (exit code 2).
 Desired-state execution — converging a live table onto a `CREATE TABLE` file, including
 creating the table when it does not exist yet — is available through `migrate --desired schema.sql` and
 `migrate.RunDesired` in [`pkg/migrate`](pkg/migrate/desired.go).
@@ -119,6 +120,13 @@ refusal — never a silently wrong or incomplete result:
   `REPLICA IDENTITY` first, then re-plan.
 - **Retyping or dropping a column a view reads is refused** — PostgreSQL would
   refuse it anyway; the plan names the views to drop or redefine first.
+- **Destructive statements need a typed acknowledgement** — `migrate --alter`
+  refuses a statement that drops a column, a constraint, or a `NOT NULL` with
+  `destructive-change` (exit 2) on every route and in `--dry-run`, unless
+  `--allow-destructive SCHEMA.TABLE` names the table; the acknowledged run is
+  audited and its verdict carries `allowed_destructive: true`. A drop that
+  adds the same column back gets the guidance to change its type in place,
+  which keeps its values.
 - **Destructive desired-state plans are refused as a whole** — `migrate --desired`
   creates missing tables and converges supported changes, but never infers permission
   to discard live structure. A new primary key or unique constraint over a live
@@ -197,10 +205,10 @@ the machine-readable shape is in
 **Refuse: no safe path exists, so nothing runs.** A rewrite the copy-and-swap
 backend does not carry yet (a volatile column default, for one) refuses; the
 dry run exits 2 so CI can gate on it without parsing JSON (the full four-code ladder is under
-[Exit codes](#exit-codes)). The exit-code gate stops refusals only —
-a destructive-but-executable change (`DROP COLUMN`) warns and exits 0, so a
-gate that must stop drops checks `.statements[].destructive` in the
-`--json` report. Watch it in
+[Exit codes](#exit-codes)). A destructive change (`DROP COLUMN`) is refused
+the same way, exit 2 with reason `destructive-change`, until
+`--allow-destructive SCHEMA.TABLE` names the table; the acknowledged dry run
+still marks it `.statements[].destructive` in the `--json` report. Watch it in
 [docs/demos/refuse.gif](docs/demos/refuse.gif); the machine-readable shape
 is in [docs/cli-output-examples.md](docs/cli-output-examples.md).
 
@@ -210,7 +218,7 @@ findings exit non-zero, warnings alone pass. Watch it in
 [docs/demos/lint.gif](docs/demos/lint.gif); the machine-readable shape is
 in [docs/cli-output-examples.md](docs/cli-output-examples.md).
 
-More shapes — every disposition as JSON, destructive warnings, and exit
+More shapes — every disposition as JSON, destructive refusals, and exit
 codes — are in [docs/cli-output-examples.md](docs/cli-output-examples.md).
 
 ## Install
@@ -265,7 +273,7 @@ vouch for it as online-safe.
 | Exit | Meaning | Anything committed? | Online-safe? |
 |---|---|---|---|
 | 0 | Executed through an online-safe path; or a dry run, `diff`, or `pull` found nothing to refuse | yes (dry run, `diff`, `pull`: nothing runs) | yes |
-| 1 | Failed: a PostgreSQL error after execution started (rolled back), or an operational or usage error before it — bad flags, an unreachable database, a mismatched `--force` acknowledgement | no; a safer sequence that stopped mid-flight keeps its committed prefix, named in the verdict's `executed_sql`; unknown when the verdict's `code` is `commit-outcome-unknown` — the failed step's `COMMIT` may have landed, so inspect the catalog before retrying | not applicable |
+| 1 | Failed: a PostgreSQL error after execution started (rolled back), or an operational or usage error before it — bad flags, an unreachable database, a mismatched `--force` or `--allow-destructive` acknowledgement | no; a safer sequence that stopped mid-flight keeps its committed prefix, named in the verdict's `executed_sql`; unknown when the verdict's `code` is `commit-outcome-unknown` — the failed step's `COMMIT` may have landed, so inspect the catalog before retrying | not applicable |
 | 2 | Refused: no online-safe path, and the verdict names the typed `reason` (and `class`) automation switches on | no | not applicable |
 | 3 | Committed through the accepted-blocking passthrough — the operator explicitly accepted a blocking form, and the engine ran it under bounded budgets without vouching for online safety | yes | no |
 

SAFETY.md

@@ -57,12 +57,14 @@ re-introspected, re-classified, re-routed, and re-preflighted at execution time,
 the loop wrongly admits still cannot make the core exceed a lock budget or skip a preflight.
 **One admission check has no core backstop: the destructive guard.** The core has no concept
 of destructiveness — `pkg/executor` and `pkg/preflight` never check it — so refusing a
-destructive desired-state plan rests entirely on `RunDesired`'s admission gate and on the
-classifier's `Destructive` derivation in `pkg/planner`, and its failure mode is data loss (a
-falsely-admitted `DROP COLUMN` commits), not a refusal or a bounded failed attempt. Those two
-sites are the exception to the periphery posture: treat `destructiveOp` and the desired-state
-admission gate with the core's review bar — spec-first, test-first, small diffs — even though
-their packages stay periphery for everything else they do.
+destructive desired-state plan rests entirely on `RunDesired`'s admission gate, refusing an
+unacknowledged destructive imperative statement rests entirely on `Run`'s destructive gate
+(RF-10), and both rest on the classifier's `Destructive` derivation in `pkg/planner`. Their
+failure mode is data loss (a falsely-admitted `DROP COLUMN` commits), not a refusal or a
+bounded failed attempt. Those three sites are the exception to the periphery posture: treat
+`destructiveOp`, the desired-state admission gate, and the imperative destructive gate with
+the core's review bar — spec-first, test-first, small diffs — even though their packages stay
+periphery for everything else they do.
 
 ## Rules inside the core
 

demo/README.md

@@ -12,11 +12,11 @@ The tour walks five sections, each runnable on its own via
 
 | Section   | What it shows                                                                                                       | Writes?             |
 | --------- | ------------------------------------------------------------------------------------------------------------------- | ------------------- |
-| `dryrun`  | One statement per planner route, reason, and disposition: metadata-only, fast-default, binary-coercible, the safer idioms, a rewrite-required suggestion, type-rewrite, volatile-default, an app-breaking rename (refused as rewrite-required), destructive, relocation, and a refusal | no                  |
+| `dryrun`  | One statement per planner route, reason, and disposition: metadata-only, fast-default, binary-coercible, the safer idioms, a rewrite-required suggestion, type-rewrite, volatile-default, an app-breaking rename (refused as rewrite-required), a destructive drop (planned under `--allow-destructive`, refused as `destructive-change` without it), relocation, and a refusal | no                  |
 | `diff`    | The declarative front door: a routed convergence plan for an existing table, for a missing one, and a typed create-shape refusal (exit code 2) for a missing table declared `IF NOT EXISTS` | no                  |
 | `pull`    | Existing-database onboarding: export one desired file per demo table, then prove each produces a zero-change `diff` | no                  |
 | `offline` | `lint` (gates on error findings), `suggest` (advises), `fmt` (canonicalizes) — no database                           | no                  |
-| `exec`    | Real executions: a native add, the concurrent index substitution, the four-step `SET NOT NULL` sequence, a genuine type rewrite run through copy-and-swap (`executed-via-copy-and-swap`, exit code 0), a structured refusal (exit code 2) for a rewrite the copy-and-swap engine does not carry yet, and the `--accept-blocking` passthrough: the plain `DROP INDEX` refused, refused again with a mismatched acknowledgement (exit code 1, nothing runs), then committed without online safety (exit code 3) along with a `REINDEX TABLE` | yes (seeded tables) |
+| `exec`    | Real executions: a native add, the concurrent index substitution, the four-step `SET NOT NULL` sequence, a genuine type rewrite run through copy-and-swap (`executed-via-copy-and-swap`, exit code 0), a structured refusal (exit code 2) for a rewrite the copy-and-swap engine does not carry yet, and the `--accept-blocking` passthrough: the plain `DROP INDEX` refused, refused again with a mismatched acknowledgement (exit code 1, nothing runs), then committed without online safety (exit code 3) along with a `REINDEX TABLE`, and a `DROP COLUMN` refused as `destructive-change` (exit code 2) until `--allow-destructive` names the table | yes (seeded tables) |
 
 `make demo` reseeds [seed.sql](seed.sql) first, so every run starts from the
 same state and the tour is rerunnable. The compose database is left running
@@ -31,8 +31,8 @@ make demo-check   # same tour, asserting on --json fields and exit codes; needs
 ```
 
 `CHECK=1` makes the tour assert on the typed JSON contract — plan report
-routes/reasons/destructive flags and `format_version`, verdict outcomes and
-reasons, the substituted `executed_sql` shape (step count plus a
+routes/reasons/destructive flags and `format_version`, verdict outcomes,
+reasons, and classes, the `allowed_destructive` audit field, the substituted `executed_sql` shape (step count plus a
 distinguishing fragment, so a regression that drops `CONCURRENTLY` or
 collapses the `SET NOT NULL` sequence turns the job red), statement counts —
 and on exit codes (`0` success, `2` refusal, `3` accepted-blocking commit,

demo/tour.sh

@@ -51,34 +51,88 @@ assert_eq() {
     fi
 }
 
-# dry_run route reason destructive disposition sql
+# dry_run route reason destructive disposition sql [allow_destructive_table]
 #
 # Demo mode prints the routed plan. Check mode asserts the statement's
 # route, first decision reason, destructive flag, and the report's
 # disposition from the JSON plan report. Dry-run never writes, so these
-# rows can rerun in any order.
+# rows can rerun in any order. A destructive statement dry-runs to a plan
+# only under --allow-destructive, so its row names the table to
+# acknowledge; refused_destructive covers the unacknowledged form.
 #
 # Exit code: an execute-disposition plan exits 0; any plan that would not
 # execute (rewrite-required, unavailable, refuse) exits with the refusal
 # code 2 — the binary under test is always this tree's build, so the
 # contract is pinned exactly.
 dry_run() {
     local route="$1" reason="$2" destructive="$3" disposition="$4" sql="$5" out status=0
+    local ack=()
+    if [ -n "${6:-}" ]; then
+        ack=(--allow-destructive "$6")
+    fi
     step "$sql"
     if [ "$CHECK" = 1 ]; then
-        out=$("$PGS" migrate --url "$PG_DSN" --dry-run --json --alter "$sql") || status=$?
+        out=$("$PGS" migrate --url "$PG_DSN" --dry-run --json --alter "$sql" ${ack[@]+"${ack[@]}"}) || status=$?
         if [ "$disposition" = execute ]; then
             assert_eq "dry-run exit of [$sql]" 0 "$status"
         else
             assert_eq "dry-run refusal exit of [$sql]" 2 "$status"
         fi
-        assert_eq "plan format_version of [$sql]" 13 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "plan format_version of [$sql]" 14 "$(jq -r '.format_version' <<<"$out")"
         assert_eq "disposition of [$sql]" "$disposition" "$(jq -r '.disposition' <<<"$out")"
         assert_eq "route of [$sql]" "$route" "$(jq -r '.statements[0].route' <<<"$out")"
         assert_eq "reason of [$sql]" "$reason" "$(jq -r '.statements[0].decisions[0].reason' <<<"$out")"
         assert_eq "destructive of [$sql]" "$destructive" "$(jq -r '.statements[0].destructive' <<<"$out")"
     else
-        "$PGS" migrate --url "$PG_DSN" --dry-run --alter "$sql" || echo "(exit $?)"
+        "$PGS" migrate --url "$PG_DSN" --dry-run --alter "$sql" ${ack[@]+"${ack[@]}"} || echo "(exit $?)"
+    fi
+}
+
+# refused_destructive sql
+#
+# A statement that discards data runs only when --allow-destructive names
+# its table. Without it, the dry run and the apply both end in the
+# destructive-change verdict (class by-design) and the refusal exit code 2;
+# the dry run prints that verdict, not a plan report, so it carries no
+# format_version. Nothing runs either way, so the row can rerun.
+refused_destructive() {
+    local sql="$1" out status=0
+    step "refuse (destructive, unacknowledged): $sql"
+    if [ "$CHECK" = 1 ]; then
+        out=$("$PGS" migrate --url "$PG_DSN" --dry-run --json --alter "$sql") || status=$?
+        assert_eq "destructive dry-run exit of [$sql]" 2 "$status"
+        assert_eq "destructive dry-run outcome of [$sql]" refused "$(jq -r '.outcome' <<<"$out")"
+        assert_eq "destructive dry-run reason of [$sql]" destructive-change "$(jq -r '.reason' <<<"$out")"
+        assert_eq "destructive dry-run class of [$sql]" by-design "$(jq -r '.class' <<<"$out")"
+        assert_eq "destructive dry-run format_version of [$sql]" null "$(jq -r '.format_version' <<<"$out")"
+        status=0
+        out=$("$PGS" migrate --url "$PG_DSN" --json --alter "$sql") || status=$?
+        assert_eq "destructive exit of [$sql]" 2 "$status"
+        assert_eq "destructive outcome of [$sql]" refused "$(jq -r '.outcome' <<<"$out")"
+        assert_eq "destructive reason of [$sql]" destructive-change "$(jq -r '.reason' <<<"$out")"
+        assert_eq "destructive class of [$sql]" by-design "$(jq -r '.class' <<<"$out")"
+    else
+        "$PGS" migrate --url "$PG_DSN" --alter "$sql"             || echo "(refused, exit $? — expected: a destructive statement needs --allow-destructive naming its table)"
+    fi
+}
+
+# execute_destructive table sql
+#
+# Runs a statement that discards data under the --allow-destructive
+# acknowledgement and requires the executed-natively verdict with exit 0
+# and the allowed_destructive audit field, for the table the
+# acknowledgement named.
+execute_destructive() {
+    local table="$1" sql="$2" out status=0
+    step "execute (allow destructive on $table): $sql"
+    if [ "$CHECK" = 1 ]; then
+        out=$("$PGS" migrate --url "$PG_DSN" --json --alter "$sql" --allow-destructive "$table" 2>/dev/null) || status=$?
+        assert_eq "acknowledged destructive exit of [$sql]" 0 "$status"
+        assert_eq "outcome of [$sql]" executed-natively "$(jq -r '.outcome' <<<"$out")"
+        assert_eq "allowed_destructive of [$sql]" true "$(jq -r '.allowed_destructive' <<<"$out")"
+        assert_eq "table of [$sql]" "$table" "$(jq -r '.table' <<<"$out")"
+    else
+        "$PGS" migrate --url "$PG_DSN" --alter "$sql" --allow-destructive "$table"
     fi
 }
 
@@ -200,7 +254,7 @@ diff_plan() {
     if [ "$CHECK" = 1 ]; then
         out=$("$PGS" diff --url "$PG_DSN" --desired "$desired" --schema public --json) || status=$?
         assert_eq "diff exit of [$desired]" 0 "$status"
-        assert_eq "diff format_version of [$desired]" 13 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "diff format_version of [$desired]" 14 "$(jq -r '.format_version' <<<"$out")"
         assert_eq "statement count of [$desired]" "$count" "$(jq -r '.statements | length' <<<"$out")"
         case "$(jq -r '.statements[0].sql' <<<"$out")" in
         *"$fragment"*) ;;
@@ -228,7 +282,7 @@ diff_refused() {
     if [ "$CHECK" = 1 ]; then
         out=$("$PGS" diff --url "$PG_DSN" --desired "$desired" --schema public --json) || status=$?
         assert_eq "diff refusal exit of [$desired]" 2 "$status"
-        assert_eq "diff format_version of [$desired]" 13 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "diff format_version of [$desired]" 14 "$(jq -r '.format_version' <<<"$out")"
         assert_eq "diff disposition of [$desired]" refuse "$(jq -r '.disposition' <<<"$out")"
         assert_eq "diff reason of [$desired]" unsupported-statement "$(jq -r '.statements[0].reason' <<<"$out")"
         assert_eq "diff class of [$desired]" "$class" "$(jq -r '.statements[0].class' <<<"$out")"
@@ -263,7 +317,7 @@ run_pull() {
             out=$("$PGS" diff --url "$PG_DSN" --schema public --desired "$desired" --json) || status=$?
             assert_eq "diff exit of [$desired]" 0 "$status"
             # Plan report contract version (plan.FormatVersion), same pin as diff_plan.
-            assert_eq "diff format_version of [$desired]" 13 "$(jq -r '.format_version' <<<"$out")"
+            assert_eq "diff format_version of [$desired]" 14 "$(jq -r '.format_version' <<<"$out")"
             assert_eq "diff disposition of [$desired]" execute "$(jq -r '.disposition' <<<"$out")"
             assert_eq "zero diff of [$desired]" 0 "$(jq -r '.statements | length' <<<"$out")"
         else
@@ -289,9 +343,10 @@ run_dryrun() {
     dry_run native         fast-default           false execute     "ALTER TABLE users ADD COLUMN joined timestamptz DEFAULT now()"
     dry_run copy-and-swap  volatile-default       false unavailable "ALTER TABLE users ADD COLUMN started timestamptz DEFAULT clock_timestamp()"
     dry_run native         app-breaking-rename    false rewrite-required "ALTER TABLE users RENAME COLUMN name TO full_name"
-    dry_run native         metadata-only          true  execute     "ALTER TABLE users DROP COLUMN status"
+    dry_run native         metadata-only          true  execute     "ALTER TABLE users DROP COLUMN status" public.users
     dry_run copy-and-swap  relocation             false unavailable "ALTER TABLE users SET TABLESPACE pg_default"
     dry_run refuse         unsupported-operation  false refuse      "ALTER TABLE users ENABLE ROW LEVEL SECURITY"
+    refused_destructive "ALTER TABLE users DROP COLUMN status"
 }
 
 review_rls() {
@@ -359,7 +414,7 @@ run_offline() {
     if [ "$CHECK" = 1 ]; then
         out=$("$PGS" suggest --json risky.sql) || status=$?
         assert_eq "suggest exit" 0 "$status"
-        assert_eq "suggest format_version" 4 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "suggest format_version" 5 "$(jq -r '.format_version' <<<"$out")"
         assert_eq "suggest count" 2 "$(jq -r '.suggestions | length' <<<"$out")"
     else
         "$PGS" suggest risky.sql || echo "(exit $?)"
@@ -433,6 +488,10 @@ run_exec() {
     execute_accept_mismatch public.users    "DROP INDEX orders_total_idx"
     execute_accepted        public.orders   "DROP INDEX orders_total_idx"
     execute_accepted        public.orders   "REINDEX TABLE orders"
+    # A drop discards the column's values, so it is refused until the
+    # acknowledgement names the table, then runs natively.
+    refused_destructive                     "ALTER TABLE users DROP COLUMN status"
+    execute_destructive     public.users    "ALTER TABLE users DROP COLUMN status"
 }
 
 case "$section" in