diff: report an index left invalid by an unfinished build as an advisory

README.md

@@ -142,7 +142,8 @@ refusal — never a silently wrong or incomplete result:
   with the desired definition: it plans as one online `VALIDATE CONSTRAINT`, and
   refuses with `validated-constraint-drift` if the constraint changed after planning.
 - **Invalid-index recovery has no CLI verb yet** — a failed concurrent index
-  build's leftover is reported with a typed state, and the proven removal
+  build's leftover is reported with a typed state (and named by `diff` as a
+  typed plan advisory), and the proven removal
   (`executor.RebuildAbandonedIndex`, or `executor.DropAbandonedIndex` when
   the caller must not rebuild) is library-only; from the CLI the
   [runbook](docs/invalid-index-recovery.md) applies.

demo/README.md

@@ -13,8 +13,8 @@ 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), 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                  |
+| `diff`    | The declarative front door: a routed convergence plan for an existing table, for a missing one, the plan advisories for a table carrying an invalid index (the rebuild the desired file names, and the leftover it does not), 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` — except the table seeded with an invalid index, whose baseline re-diffs to its one rebuild | 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`, and a `DROP COLUMN` refused as `destructive-change` (exit code 2) until `--allow-destructive` names the table | yes (seeded tables) |
 
@@ -38,7 +38,9 @@ collapses the `SET NOT NULL` sequence turns the job red), statement counts —
 and on exit codes (`0` success, `2` refusal, `3` accepted-blocking commit,
 `1` lint gate and mismatched acknowledgement). The `pull`
 step has no JSON mode, so check mode asserts only its exit code and exported
-file count, then uses each `diff --json` report to assert zero statements. It never
+file count, then uses each `diff --json` report to assert zero statements (one
+`create-index` plus its `invalid-index-rebuild` advisory for the table seeded
+with an invalid index). It never
 asserts on human-facing prose, which is free to change. CI runs
 this as the `demo` job ("smoke test (built pg-sprite artifact)"): the
 built `bin/pg-sprite` exercised end-to-end against compose PostgreSQL.

demo/desired/contacts_no_index.sql

@@ -0,0 +1,4 @@
+CREATE TABLE contacts (
+    id    bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
+    email text NOT NULL
+);

demo/desired/contacts_rebuild.sql

@@ -0,0 +1,6 @@
+CREATE TABLE contacts (
+    id    bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
+    email text NOT NULL
+);
+
+CREATE UNIQUE INDEX contacts_email_key ON contacts (email);

demo/seed.sql

@@ -2,7 +2,7 @@
 -- recreates everything, so every `make demo` starts from the same state.
 -- Identity (not serial) keys so the declarative diff never meets a
 -- sequence-backed default it must refuse.
-DROP TABLE IF EXISTS users, orders, widgets CASCADE;
+DROP TABLE IF EXISTS users, orders, widgets, contacts CASCADE;
 
 CREATE TABLE users (
     id     bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
@@ -33,6 +33,24 @@ FROM generate_series(1, 5000) g;
 -- desired-state file describes, so the diff tour's plans do not see it.
 CREATE INDEX orders_total_idx ON orders (total);
 
+-- A table carrying the leftover of a failed concurrent index build: the
+-- unique build below runs over duplicate emails, so it fails after creating
+-- its catalog entry and leaves contacts_email_key behind as an invalid
+-- index. The diff tour shows the plan's advisory for it, and the pull tour
+-- shows the freshly exported baseline re-diffing to that one rebuild.
+-- CONCURRENTLY cannot run inside a transaction, so the failure is tolerated
+-- here rather than caught in a DO block.
+CREATE TABLE contacts (
+    id    bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
+    email text NOT NULL
+);
+
+INSERT INTO contacts (email) VALUES ('dup@example.com'), ('dup@example.com');
+
+\set ON_ERROR_STOP 0
+CREATE UNIQUE INDEX CONCURRENTLY contacts_email_key ON contacts (email);
+\set ON_ERROR_STOP 1
+
 -- Dedicated fixture for atomic desired-state row security.
 CREATE SCHEMA IF NOT EXISTS demo_security;
 DROP TABLE IF EXISTS demo_security.demo_rls;

demo/tour.sh

@@ -78,7 +78,7 @@ dry_run() {
         else
             assert_eq "dry-run refusal exit of [$sql]" 2 "$status"
         fi
-        assert_eq "plan format_version of [$sql]" 14 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "plan format_version of [$sql]" 15 "$(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")"
@@ -254,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]" 14 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "diff format_version of [$desired]" 15 "$(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"*) ;;
@@ -265,6 +265,33 @@ diff_plan() {
     fi
 }
 
+# diff_advisory desired_file statement_count advisory_kind
+#
+# A desired file for the seeded table that carries an invalid index (the
+# leftover of the failed unique build in seed.sql). The diff never drops an
+# unfinished build, so the plan carries an advisory naming it instead
+# (plan.Report.Advisories, schemadiff.AdvisoryKinds()): the rebuild kind when
+# the desired file names the index, the undeclared kind when it does not —
+# in which case the plan has zero statements but is not silent about the
+# leftover. The advisory sits outside the statements and the fingerprint, so
+# the exit code stays 0.
+diff_advisory() {
+    local desired="$1" count="$2" kind="$3" out status=0
+    step "diff --desired $desired (expect advisory)"
+    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]" 15 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "diff disposition of [$desired]" execute "$(jq -r '.disposition' <<<"$out")"
+        assert_eq "statement count of [$desired]" "$count" "$(jq -r '.statements | length' <<<"$out")"
+        assert_eq "advisory count of [$desired]" 1 "$(jq -r '.advisories | length' <<<"$out")"
+        assert_eq "advisory kind of [$desired]" "$kind" "$(jq -r '.advisories[0].kind' <<<"$out")"
+        assert_eq "advisory index of [$desired]" contacts_email_key "$(jq -r '.advisories[0].index' <<<"$out")"
+    else
+        "$PGS" diff --url "$PG_DSN" --desired "$desired" --schema public || echo "(exit $?)"
+    fi
+}
+
 # diff_refused desired_file cause
 #
 # A greenfield desired file whose create shape the create path refuses: the
@@ -282,7 +309,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]" 14 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "diff format_version of [$desired]" 15 "$(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")"
@@ -304,22 +331,38 @@ run_pull() {
         "$PGS" pull --url "$PG_DSN" --schema public --out "$out_dir" >/dev/null || status=$?
         assert_eq "pull exit" 0 "$status"
         file_count=$(find "$out_dir" -maxdepth 1 -type f -name '*.sql' | wc -l | tr -d ' ')
-        # One file per table in demo/seed.sql (orders, users).
-        assert_eq "pulled file count" 2 "$file_count"
+        # One file per table in demo/seed.sql (contacts, orders, users).
+        assert_eq "pulled file count" 3 "$file_count"
     else
         "$PGS" pull --url "$PG_DSN" --schema public --out "$out_dir" || echo "(exit $?)"
     fi
 
+    # Every exported file re-diffs to zero statements — except the one table
+    # carrying an invalid index (contacts, seeded that way): pull renders the
+    # index like any other, so its baseline re-diffs to exactly one
+    # create-index, and the plan's rebuild advisory names the entry. That
+    # lone create is the signal that a just-exported table carries an
+    # unfinished build (docs/pull.md).
     for desired in "$out_dir"/*.sql; do
         step "zero-diff proof: $desired"
         status=0
         if [ "$CHECK" = 1 ]; then
             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]" 14 "$(jq -r '.format_version' <<<"$out")"
+            assert_eq "diff format_version of [$desired]" 15 "$(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")"
+            case "$(basename "$desired")" in
+            contacts.sql)
+                assert_eq "rebuild diff of [$desired]" 1 "$(jq -r '.statements | length' <<<"$out")"
+                assert_eq "rebuild kind of [$desired]" create-index "$(jq -r '.statements[0].kind' <<<"$out")"
+                assert_eq "advisory kind of [$desired]" invalid-index-rebuild "$(jq -r '.advisories[0].kind' <<<"$out")"
+                ;;
+            *)
+                assert_eq "zero diff of [$desired]" 0 "$(jq -r '.statements | length' <<<"$out")"
+                assert_eq "no advisories of [$desired]" 0 "$(jq -r '.advisories | length' <<<"$out")"
+                ;;
+            esac
         else
             "$PGS" diff --url "$PG_DSN" --schema public --desired "$desired" || echo "(exit $?)"
         fi
@@ -370,6 +413,8 @@ run_diff() {
     heading "Declarative front door: routed convergence plans (read-only)"
     diff_plan desired/users_v2.sql 2 "ADD COLUMN bio"
     diff_plan desired/widgets_v1.sql 2 "CREATE TABLE"
+    diff_advisory desired/contacts_rebuild.sql 1 invalid-index-rebuild
+    diff_advisory desired/contacts_no_index.sql 0 invalid-index-undeclared
     diff_refused desired/gadgets_if_not_exists.sql if-not-exists
 }
 

docs/capabilities.md

@@ -264,7 +264,7 @@ or the only online form is a direct statement the operator runs).
 <!-- capabilities:begin declarative_model -->
 | Table shape | Status | Engine path | Online-safety problem? | Behavior and why |
 | --- | --- | --- | --- | --- |
-| Plain tables + their indexes | ✅ | native, as-is | Yes | `diff`, `pull`, and desired-file rendering round-trip the canonical model. The model carries each index's validity (`pg_index.indisvalid`): on a plain table an invalid entry is a concurrent build that did not finish — abandoned, or still running — so a live entry with the desired name and definition does not deliver the desired index, and `diff` plans it as a `create-index` change. `diff` never emits a drop for an invalid entry, whatever the desired file says about its name: a plain `DROP INDEX` blocks the table and cannot tell abandoned debris from a build still in progress. A `pull`ed baseline of such a table therefore re-diffs to that one rebuild rather than to zero. On a partitioned parent an invalid index means unattached partition indexes, not an unfinished build, so validity plays no part in the parent's comparison |
+| Plain tables + their indexes | ✅ | native, as-is | Yes | `diff`, `pull`, and desired-file rendering round-trip the canonical model. The model carries each index's validity (`pg_index.indisvalid`): on a plain table an invalid entry is a concurrent build that did not finish — abandoned, or still running — so a live entry with the desired name and definition does not deliver the desired index, and `diff` plans it as a `create-index` change. `diff` never emits a drop for an invalid entry, whatever the desired file says about its name: a plain `DROP INDEX` blocks the table and cannot tell abandoned debris from a build still in progress. Every invalid entry the diff observes and leaves in place is named on the plan report as an advisory (`invalid-index-rebuild` when the desired file names it, `invalid-index-undeclared` when it does not), outside the statements and the fingerprint, so an empty plan over such a table reads as 'no changes to run' rather than converged. A `pull`ed baseline of such a table therefore re-diffs to that one rebuild rather than to zero. On a partitioned parent an invalid index means unattached partition indexes, not an unfinished build, so validity plays no part in the parent's comparison |
 | A live check constraint or foreign key left `NOT VALID` | ✅ | native, as-is | Yes | The model carries each constraint's validity (`pg_constraint.convalidated`). A live constraint left `NOT VALID` with the desired definition, which an interrupted online add leaves behind, plans as one `VALIDATE CONSTRAINT` statement (`validate-constraint`), which scans under `SHARE UPDATE EXCLUSIVE` while writes keep flowing, instead of a destructive drop and re-add. The statement names only the constraint, so execution reads the constraint again under that same lock and refuses with the permanent `validated-constraint-drift` code, validating nothing, if it was dropped or replaced under the same name since planning. A foreign key plans as `validate-foreign-key`, refused with `declarative-foreign-key` like every declarative foreign-key change. `CREATE TABLE` ignores `NOT VALID`, so a desired file cannot declare it: for a check constraint, `pull` renders the validated definition, and the baseline re-diffs to that one validation rather than to zero. `pull` refuses a table that owns a foreign key, validated or not, with `ErrUnrenderableForeignKey` |
 | Classic table inheritance (`INHERITS`) | 🟡 | native, planned flow | Yes | Typed refusal for both parents and children: the model cannot express inheritance edges, and flattening inherited columns would produce a silently lossy baseline. A desired file that declares `INHERITS` is refused before planning builds anything, since the scratch build would lock the live parent and add an inheritance edge to it |
 | Tables that own **or are referenced by** foreign keys | 🟡 | native, planned flow | Yes | Planned but not executed. A desired file may declare a foreign key to an unqualified table that exists in the target schema; `diff` shows a matching foreign key as no change and an added or dropped one as an `add-foreign-key` or `drop-foreign-key` statement refused with `declarative-foreign-key`, and `migrate --desired` refuses the whole plan. Planning binds each reference to an empty scratch copy of the live table, which needs `SELECT` on it, so planning takes only `ACCESS SHARE` on the live referenced table, the lock a plain `SELECT` takes, and never blocks its writes. Export still refuses both sides — an incoming FK cannot be expressed in the table's own desired file. Declarative FK execution (composite keys as the primary case, two-phase `NOT VALID` → `VALIDATE` execution) is planned |

docs/cli-output-examples.md

@@ -160,7 +160,7 @@ itself.
 ```console
 $ pg-sprite migrate --alter 'ALTER TABLE users ADD COLUMN note text' --dry-run --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "alter",
   "schema": "public",
   "table": "users",
@@ -201,7 +201,7 @@ plans the safer online sequence instead: the decision carries it in
 ```console
 $ pg-sprite migrate --alter 'ALTER TABLE users ADD CONSTRAINT users_email_key UNIQUE (email)' --dry-run --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "alter",
   "schema": "public",
   "table": "users",
@@ -303,7 +303,7 @@ column first, then build the constraint as a separate, named
 ```console
 $ pg-sprite migrate --alter 'ALTER TABLE users ADD COLUMN nickname text UNIQUE' --dry-run --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "alter",
   "schema": "public",
   "table": "users",
@@ -345,7 +345,7 @@ no native SQL to show.
 ```console
 $ pg-sprite migrate --alter 'ALTER TABLE users ALTER COLUMN id TYPE text' --dry-run --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "alter",
   "schema": "public",
   "table": "users",
@@ -405,7 +405,7 @@ column, a relocation — so the plan is `unavailable` and nothing executes.
 ```console
 $ pg-sprite migrate --alter 'ALTER TABLE users ADD COLUMN joined timestamptz DEFAULT clock_timestamp()' --dry-run --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "alter",
   "schema": "public",
   "table": "users",
@@ -442,7 +442,7 @@ The refusal cause is the report-level `reason`.
 ```console
 $ pg-sprite migrate --alter 'CREATE INDEX events_created_idx ON events (created)' --dry-run --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "alter",
   "schema": "public",
   "table": "events",
@@ -507,7 +507,7 @@ table. A mismatched acknowledgement executes nothing and exits 1.
 ```console
 $ pg-sprite migrate --alter 'ALTER TABLE users DROP COLUMN email' --dry-run --allow-destructive public.users --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "alter",
   "schema": "public",
   "table": "users",
@@ -634,7 +634,7 @@ CREATE TABLE users (
 ```console
 $ pg-sprite diff --desired /tmp/users.sql --json
 {
-  "format_version": 14,
+  "format_version": 15,
   "source": "diff",
   "schema": "public",
   "table": "users",