test(ci): guard the booking payment intent uniqueness (#2529)

* test(ci): guard the booking payment intent uniqueness

Invoice.providerPaymentIntentId is the entire idempotency guarantee of the
Stripe appointment-booking webhook. handleWebhookEvent never reads event.id,
so a redelivered event re-runs the whole dispatch, and this unique index is
the only thing stopping two concurrent deliveries minting two invoices for
one payment.

The jest suite cannot assert that, and it is worth being precise about why:
it mocks Prisma, so it exercises the handler's decision logic and never the
constraint those decisions rest on. Dropping the index leaves every one of
those tests green.

Asserted here instead, against the real Postgres this gate already runs. Two
rows with one intent must be refused; two rows with a NULL intent must still
coexist, because every invoice not raised by that webhook carries NULL and
Postgres keeps NULLs distinct.

Verified by extracting this step body and running it against postgres:16
-alpine with the full migration history: it passes with the index, fails with
'Two invoices were accepted for the same Stripe payment intent' once the index
is dropped, and passes again on restore. The trap clears its rows either way.

* test(ci): assert the intent collision is the unique index, not any error

Review caught that the first version of this guard proved less than it
claimed. It inserted two rows carrying one intent in a single statement and
treated any failure as success, so a later migration that dropped the index
while adding a check constraint, foreign key or trigger rejecting the test
value would still have reported uniqueness intact. A typo in the guard's own
SQL would have done the same. That is the exact failure shape the guard exists
to prevent, in the guard.

Now asserted three ways. The catalog must show the index present and
indisunique, which catches a drop directly and independently of any error
text. The rows go in as two statements, so the first must succeed before the
second is required to fail. And the failure must name both 'duplicate key
value violates unique constraint' and Invoice_providerPaymentIntentId_key, so
an unrelated rejection cannot pass for this one.

Verified against postgres:16-alpine with the full migration history, including
the reviewer's scenario: index dropped, a CHECK constraint rejecting the test
value. The previous guard printed 'The booking payment intent is unique' and
exited 0 on that state. This one exits 1. Baseline still passes and the trap
clears its rows either way.

* test(ci): bind the uniqueness check to the table it protects

Ran an adversarial pass over this step rather than waiting to be told a third
time that it proves less than it claims. Six angles, 23 candidate attacks,
seven confirmed after a judge threw out the ones that were wrong about
Postgres 16.

The catalog check now returns four facts and expects 1|0|1|1, because they
fail in four different ways and one count would hide which:

  a. "Invoice" is still a plain table in public, not a view or a partitioned
     parent.
  b. It has no children. A partition or an INHERITS child holds invoices the
     parent's index does not cover, and the old check could not see one.
  c. The index is on THAT table (indrelid), live rather than a half-built
     CONCURRENTLY corpse (indisvalid/indisready), unconditional, and keyed on
     that column alone.
  d. It is the ONLY unique index over the column. A second one, NULLS NOT
     DISTINCT, ends the NULL coexistence every other invoice path depends on
     without touching the first.

Two smaller ones. The index name is matched with its closing quote, so a
suffixed sibling cannot pass for it. And the NULL rows are inserted without
naming the column, the way every non-webhook path writes, so a DEFAULT on it
fails here instead of in production.

Measured on postgres:16-alpine with the full history. Previous step vs this
one: INHERITS child 0 -> 1, second unique index 0 -> 1, DEFAULT '' 0 -> 1.
Baseline stays 0 for both. The relocated-index case was already failing, but
only because this table's dropped-column slots put the attnum at 39 while a
LIKE-rebuilt copy puts it at 31; pinning indrelid makes that deliberate rather
than an accident of history.

What this still cannot see, and the comment says so: the column could stop
being written entirely and the index would stay unique and unused. That needs
a backend test that mints an invoice through StripeService, not a catalog
query.

---------

Co-authored-by: Ankit Upadhyay <ankit@dunexploration.com>

Author: ankit-yc

.github/workflows/_migration.yaml

@@ -223,6 +223,153 @@ jobs:
 
           echo "Clinical term search survives a null synonym list and a quoted synonym."
 
+      # Invoice.providerPaymentIntentId is the whole idempotency guarantee of the
+      # Stripe appointment-booking webhook. handleWebhookEvent never reads
+      # event.id - there is no processed-event table, no cache and no lock - so a
+      # redelivered event re-runs the entire dispatch, and this unique index is
+      # the only thing that stops two concurrent deliveries minting two invoices
+      # for one payment.
+      #
+      # It is asserted here because the jest suite CANNOT assert it: that suite
+      # mocks Prisma, so it exercises the handler's decision logic and never the
+      # constraint the decisions rely on. Verified against a real Postgres that
+      # dropping this index lets both racers insert, producing exactly the
+      # duplicate-invoice bug the design exists to prevent.
+      #
+      # The nullable half matters just as much: every invoice not raised by that
+      # webhook has a NULL here, and Postgres keeps NULLs distinct in a unique
+      # index, so they must all continue to coexist.
+      - name: Check the booking payment intent stays unique
+        shell: bash
+        env:
+          PGPASSWORD: ${{ github.run_id }}
+        run: |
+          set -euo pipefail
+
+          psql_ci() { psql -h localhost -U postgres -d yosemite_ci "$@"; }
+
+          cleanup() {
+            psql_ci -q -c "DELETE FROM \"Invoice\" WHERE \"currency\" LIKE 'ci-guard%';" || true
+          }
+          trap cleanup EXIT
+
+          # Four facts, because they fail in four different ways and a single
+          # count would hide which one broke. Expected: 1|0|1|1.
+          #
+          #   a. "Invoice" is still a plain table in public - not a view, and not
+          #      a partitioned parent.
+          #   b. It has no children. A partition or an INHERITS child would hold
+          #      invoices the parent's index does not cover.
+          #   c. The index protects THIS table (indrelid), is live rather than a
+          #      half-built corpse (indisvalid/indisready), is unconditional
+          #      (indpred), and is keyed on that one column (indnkeyatts/indkey).
+          #      Every one of those can be false while the name still exists.
+          #   d. It is the ONLY unique index over the column. A second one -
+          #      NULLS NOT DISTINCT, or composite and partial - would end the
+          #      NULL coexistence that every non-webhook invoice depends on,
+          #      without touching this one.
+          #
+          # What this CANNOT see: the column could stop being written at all, and
+          # the index would stay unique and unused. That belongs in a backend
+          # test that mints an invoice through StripeService, not here.
+          intent_attnum="(SELECT attnum FROM pg_attribute
+             WHERE attrelid = to_regclass('public.\"Invoice\"')
+               AND attname = 'providerPaymentIntentId' AND NOT attisdropped)"
+
+          facts=$(psql_ci -tAc "
+            SELECT
+              (SELECT count(*) FROM pg_class
+                 WHERE oid = to_regclass('public.\"Invoice\"') AND relkind = 'r')
+              || '|' ||
+              (SELECT count(*) FROM pg_inherits
+                 WHERE inhparent = to_regclass('public.\"Invoice\"'))
+              || '|' ||
+              (SELECT count(*) FROM pg_index i
+                 JOIN pg_class c ON c.oid = i.indexrelid
+                WHERE c.relname = 'Invoice_providerPaymentIntentId_key'
+                  AND i.indrelid = to_regclass('public.\"Invoice\"')
+                  AND i.indisunique AND i.indisvalid AND i.indisready
+                  AND i.indpred IS NULL
+                  AND i.indnkeyatts = 1
+                  AND i.indkey[0] = $intent_attnum)
+              || '|' ||
+              (SELECT count(*) FROM pg_index i
+                WHERE i.indrelid = to_regclass('public.\"Invoice\"')
+                  AND i.indisunique
+                  AND $intent_attnum = ANY((i.indkey::int2[])[0:i.indnkeyatts-1]));")
+
+          if [ "$facts" != "1|0|1|1" ]; then
+            echo "::error::The uniqueness guarantee on Invoice.providerPaymentIntentId has changed."
+            echo "Expected 1|0|1|1, got: $facts"
+            echo "  1st: \"Invoice\" is a plain table in public"
+            echo "  2nd: it has no partitions or inheritance children"
+            echo "  3rd: Invoice_providerPaymentIntentId_key is on THAT table, valid,"
+            echo "       unconditional, and keyed on providerPaymentIntentId alone"
+            echo "  4th: it is the only unique index over that column"
+            echo "That index is the only thing stopping a redelivered booking webhook"
+            echo "from minting a second invoice for one payment, because nothing"
+            echo "upstream deduplicates by Stripe event id."
+            exit 1
+          fi
+
+          # Every non-intent value differs between the two colliding rows, so a
+          # composite unique index on any other column would not refuse them and
+          # this would correctly fail rather than pass by accident.
+          row() {
+            echo "('$1', '[]'::jsonb, $3, $3, 'ci-guard-$4', $2, now())"
+          }
+
+          insert_rows() {
+            psql_ci -v ON_ERROR_STOP=1 -q -c "
+              INSERT INTO \"Invoice\" (\"id\", \"items\", \"subtotal\", \"totalAmount\", \"currency\", \"providerPaymentIntentId\", \"updatedAt\")
+              VALUES $1;"
+          }
+
+          # One good row, then the collision. Split in two on purpose: a single
+          # two-row statement fails for any reason at all - a check constraint, a
+          # trigger, a typo in this SQL - and "the insert failed" would then read
+          # as "uniqueness is intact" while measuring nothing.
+          insert_rows "$(row 00000000-0000-4000-8000-0000000pi001 "'pi_ci_guard'" 11 a)"
+
+          if err=$(insert_rows "$(row 00000000-0000-4000-8000-0000000pi002 "'pi_ci_guard'" 22 b)" 2>&1); then
+            echo "::error::Two invoices were accepted for the same Stripe payment intent."
+            exit 1
+          fi
+
+          # And by THAT index. The name is matched with its closing quote, so a
+          # suffixed sibling such as ..._key_v2 cannot pass for it.
+          if ! grep -q 'duplicate key value violates unique constraint "Invoice_providerPaymentIntentId_key"' <<<"$err"; then
+            echo "::error::The duplicate intent was rejected, but not by the uniqueness guarantee."
+            echo "Something else refused the row, so this proves nothing about whether a"
+            echo "redelivered booking webhook can mint a second invoice."
+            echo "--- what Postgres actually said ---"
+            echo "$err"
+            exit 1
+          fi
+
+          # The nullable half. Written WITHOUT naming the column, which is how
+          # every non-webhook path inserts, so a DEFAULT or a BEFORE trigger on
+          # it fails here rather than in production.
+          if err=$(psql_ci -v ON_ERROR_STOP=1 -q -c "
+            INSERT INTO \"Invoice\" (\"id\", \"items\", \"subtotal\", \"totalAmount\", \"currency\", \"updatedAt\")
+            VALUES ('00000000-0000-4000-8000-0000000pi003', '[]'::jsonb, 33, 33, 'ci-guard-c', now()),
+                   ('00000000-0000-4000-8000-0000000pi004', '[]'::jsonb, 44, 44, 'ci-guard-d', now());" 2>&1); then
+            nulls=$(psql_ci -tAc "SELECT count(*) FROM \"Invoice\" WHERE \"currency\" LIKE 'ci-guard%' AND \"providerPaymentIntentId\" IS NULL;")
+          else
+            nulls="insert failed: $err"
+          fi
+
+          if [ "$nulls" != "2" ]; then
+            echo "::error::Invoices with no payment intent stopped coexisting."
+            echo "NULLs must stay distinct in this unique index, and the column must"
+            echo "stay genuinely null when nobody sets it, or every invoice not raised"
+            echo "by the booking webhook collides with every other one."
+            echo "Expected 2 rows, got: $nulls"
+            exit 1
+          fi
+
+          echo "The booking payment intent is unique, and NULLs still coexist."
+
       # The Supabase advisor caught public."OrganizationDocumentAcknowledgements"
       # shipping with RLS off. The cause was structural: RLS was only ever enabled
       # by hand in the dashboard, so every new table was unprotected until someone