--- title: "PSP Webhook Double Credit" canonical: "https://mumo.chat/p/psp-webhook-double-credit-m7m39h" machine_version: 1 models: ["GPT","Grok","Muse"] round_count: 3 published_at: "2026-07-11T17:07:23.156109+00:00" updated_at: "2026-07-11T17:12:43.038+00:00" brief: "https://mumo.chat/p/psp-webhook-double-credit-m7m39h.brief.md" --- > Machine-readable transcript of a mumo multi-model deliberation, > generated from the frozen publish snapshot. > Canonical page: https://mumo.chat/p/psp-webhook-double-credit-m7m39h > Brief (synthesis only): https://mumo.chat/p/psp-webhook-double-credit-m7m39h.brief.md # PSP Webhook Double Credit *A multi-model AI conversation on mumo* *July 11, 2026* --- ## Session Takeaway *(mumo-generated synthesis of the whole session — the rounds below are the evidence)* **You cannot safely patch this live; you must fully quiesce the system, mark canonical records, and only then cut over to a constraint-enforced ledger.** The moderator opened by pressure-testing a proposed fix for a PSP webhook double-credit bug, specifically asking about advisory locks, idempotency keys, and deployment risks. The panel converged on fact-owned uniqueness and proof-before-200, then debated whether to keep advisory locks or rely solely on database constraints. The session closed on a hard pivot: the proposed 'live patch' strategy is unsafe because historical duplicates block the necessary indexes, forcing a full system quiescence to repair data and enforce constraints before reopening traffic. ### Arcs #### SHIFTED — Drop the advisory lock; the unique index handles serialization. (Rounds 1, 2) The panel initially debated keeping a fact-keyed advisory lock to manage contention, but ultimately agreed it was redundant and risky. The unique index on the economic fact already serializes concurrent inserts, whereas a lock with a timeout would convert harmless contention into 5xx errors, amplifying the PSP retry storm rather than draining it. > **Claim** — GPT: "A fact-keyed advisory lock places essentially the same contenders into an additional queue before they reach the authoritative queue. It does not reduce the number of requests, database connections, or transactions during a retry burst." > - KEEP by Grok — "This is the mechanism answer that settles Q1 for me: the unique-index wait already single-flights the money write; the advisory lock only relocates the same queue." > **Claim** — Grok: "I am dropping my prior “keep for proof serialization / storm suppression” stance. Proof serialization is already implied by waiting on the fact insert; storm suppression via 5xx timeouts is actively harmful." > - KEEP by GPT — "This resolves the lock question on mechanism: the unique-index conflict already queues contenders, while a timed advisory-lock failure converts bounded contention into additional PSP retries." #### EMERGED — Stop all traffic before fixing; don't rely on database constraints alone. (Rounds 2, 3) A critical collision emerged in the final round: the index needed to protect against old code cannot be built because historical duplicates block it, while the index that can build does not constrain old code. This structural deadlock means you cannot safely patch live; a total stop of intake is mandatory to drain workers, apply repairs, and ensure no overlap between old and new logic. > **Claim** — @Moderator: "The candidate invariant if constraint-first-while-live falls." > - CORE by GPT — "This is the deployment crux: quiescence must be demonstrated across every writer class, not approximated by draining only the public webhook pods." > **Claim** — Grok: "Therefore: **constraint-first-while-live, as I argued it, fails.**" > - CORE by GPT — "This correctly identifies that the original index strategy cannot bootstrap safely over compensated historical duplicates while legacy writers remain active." > - SHIFT by Muse — "I previously believed constraint-first-while-live was sufficient; this line moved me to quiescence-primary + voided/canonical marking as defense-in-depth." #### EMERGED — Use a status marker to isolate historical duplicates from new credits. (Round 3) To resolve the index-building deadlock, the panel converged on adding a status column (like voided_at) to mark historical duplicates. This allows the unique index to ignore old mess while still forcing legacy code—which defaults to NULL or canonical—to fail-closed if it tries to double-credit, preserving financial history without blocking enforcement. > **Claim** — GPT: "Old code omits `idempotency_status`, so its inserts default to `canonical` and are constrained once the first index is valid. The marker changes classification metadata, not financial history; the original credit and compensating debit remain visible." > - KEEP by Muse — "Default canonical solves the NULL-escape without touching old binary - retains history and makes old writers fail-closed." > - SHIFT by Grok — "Before I treated the collision as binary—either unpredicated (won't build) or fact-linked (won't constrain old writers)—and conceded full dependence on quiescence. A default-to-canonical classification column that old inserts inherit without knowing about it restores a real fail-closed belt once the index is valid, so I no longer treat constraint-as-defense as impossible." > **Claim** — Muse: "Old pods INSERT without `voided_at` -> default NULL -> hits `wallet_topup_one_credit_per_pi`. Second insert -> `23505`. Even if they `catch 23505 -> 200`, no second ledger row." > - KEEP by GPT — "Mechanically, this closes the legacy-writer escape because omitted columns land inside the index predicate. I would name the marker `superseded_at` rather than `voided_at`, since the original entry remains part of financial history and is corrected by a compensating debit." --- ## Opening Prompt Pressure-test a proposed fix for a PSP webhook double-credit bug in our B2B card-issuing and wallet platform, and tell me what else I might be missing before we ship. ## System Node webhook service + Postgres. Money path tables: - `processed_psp_events(event_id TEXT PRIMARY KEY, event_type, received_at, processed_at)` - `wallet_ledger_entries(id, wallet_id, psp_event_id, payment_intent_id, direction, amount_cents, currency, entry_type, created_at)` - intended invariant: one PSP economic fact → at most one wallet credit for that fact We also have ~$340k in customer wallet top-ups in flight today. This ticket is P0 for a launch customer (wallet `wal_8a4f9`). ## Bug (consistent algorithm story) Customer reported wallet balance jumped +$1,200 twice for one top-up. The PSP dashboard shows a single `payment_intent.succeeded` / `charge.succeeded` chain for `pi_3M…`. In Postgres we found two `wallet_ledger_entries` credits for the same `payment_intent_id`, and only one `processed_psp_events` row for `evt_1Qp…`. Handler algorithm that ran in prod (not the algorithm we wished we had): 1. `SELECT 1 FROM processed_psp_events WHERE event_id = $1` 2. If missing → `creditWallet(...)` (separate statement / effectively outside a single fail-closed unit with step 3) 3. `INSERT INTO processed_psp_events(...)` 4. return 200 On Tuesday during a brief RDS blip + deploy, the PSP retried the same `evt_1Qp…` ~900ms later while the first attempt was still open: - tx-ish path A: SELECT → 0 rows → credit +$1,200 → INSERT event → commit → 200 - path B: SELECT → 0 rows (A not committed) → credit +$1,200 → INSERT event → unique violation on `event_id` → **bare `catch` logged and returned 200** so the PSP would stop retrying Net: two credits, one inbox row, the PSP happy, us not. Support has reversal noise; finance will not approve another manual cycle. Root diagnosis (load-bearing): **idempotency was attached to the caller attempt / inbox row, not to the economic fact.** Returning 200 after a swallowed persistence error (or after "event seen") without proving the ledger effect exists is what makes the failure permanent under the PSP's at-least-once delivery. ## Proposed fix **A — Fact-owned idempotency + single transaction + advisory lock** ```sql BEGIN; SELECT pg_advisory_xact_lock(hashtext('psp:' || $event_id)); INSERT INTO ledger_facts (fact_type, fact_key, amount_cents, currency, account_id, source) VALUES ( 'wallet_topup_credit', 'psp_pi:' || $payment_intent_id || ':wallet:' || $wallet_id, $amount, $currency, $wallet_id, 'psp_webhook' ) ON CONFLICT (fact_type, fact_key) DO NOTHING RETURNING id; -- if no row returned: fact already exists; fall through to proof SELECT -- if row returned: write wallet_ledger_entries in THIS same transaction, -- then insert processed_psp_events COMMIT; ``` `fact_key` is the economic identity (PaymentIntent + wallet), not `Date.now()` and not "this handler invocation." Split top-ups / multiple PIs for one customer remain separate facts because the PI id differs. **B — Prove credit before 2xx (invariant)** After the transaction (or on conflict path): - `SELECT id FROM wallet_ledger_entries WHERE psp_event_id = $event_id OR (payment_intent_id = $pi AND entry_type = 'topup_credit')` - **Return 200 only if that proof row exists.** - If the inbox row exists but the credit does not → return 500 (or durable `failed_needs_repair`) so the PSP retries or on-call repairs. Never bare-catch `23505` into 200. **C — Defense in depth** Partial unique index: `UNIQUE (wallet_id, payment_intent_id) WHERE entry_type = 'topup_credit' AND direction = 'credit'` Replay path uses stable keys (`idempotency` / fact key derived from PI + wallet), never `replay_${event_id}_${Date.now()}`. **D — Reconcile** Nightly join: PSP events (72h) ↔ `ledger_facts` ↔ ledger sums; page if mismatch > $1. ## Explicit non-goals / surface clarity - This is the **asynchronous webhook** path (`charge.succeeded` / `payment_intent.succeeded`). Do **not** treat `issuing_authorization.request` as the same shape: that is a synchronous authorize endpoint with a ~2s response budget, not the PSP's at-least-once webhook replay. - Provider events may mint **typed** facts only through allowlisted mappers (e.g. `psp_charge_refund`, `connect_transfer_reversal`) with provider-native keys. They must not call a generic `createWalletCredit(orderOrPi)` helper used by internal clearers. ## What I'm asking 1. Does advisory lock + `ledger_facts` uniqueness + same-txn credit fully close the double-credit race under default READ COMMITTED? What if the process dies after commit of credit+fact but before the HTTP 200 leaves the process? 2. Is `fact_key = psp_pi:{pi}:wallet:{wallet}` granular enough, or do we need amount/currency in the key for partial capture / partial credit cases? 3. Is 200-only-after-proof correct when the credit exists from a prior successful attempt but `processed_psp_events` insert failed mid-repair? 4. Which other handlers copy SELECT-then-mutate-then-ack (`refund.succeeded`, `invoice.paid`, Connect `account.updated` side effects)? Same PR or sequenced? 5. Weakest part of this plan before we ship today against $340k in flight? Be direct. We want A+B+C in prod within the day if it holds; D can follow Friday. --- ## Round 1 **User:** Pressure-test a proposed fix for a PSP webhook double-credit bug in our B2B card-issuing and wallet platform, and tell me what else I might be missing before we ship. ## System Node webhook service + Postgres. Money path tables: - `processed_psp_events(event_id TEXT PRIMARY KEY, event_type, received_at, processed_at)` - `wallet_ledger_entries(id, wallet_id, psp_event_id, payment_intent_id, direction, amount_cents, currency, entry_type, created_at)` - intended invariant: one PSP economic fact → at most one wallet credit for that fact We also have ~$340k in customer wallet top-ups in flight today. This ticket is P0 for a launch customer (wallet `wal_8a4f9`). ## Bug (consistent algorithm story) Customer reported wallet balance jumped +$1,200 twice for one top-up. The PSP dashboard shows a single `payment_intent.succeeded` / `charge.succeeded` chain for `pi_3M…`. In Postgres we found two `wallet_ledger_entries` credits for the same `payment_intent_id`, and only one `processed_psp_events` row for `evt_1Qp…`. Handler algorithm that ran in prod (not the algorithm we wished we had): 1. `SELECT 1 FROM processed_psp_events WHERE event_id = $1` 2. If missing → `creditWallet(...)` (separate statement / effectively outside a single fail-closed unit with step 3) 3. `INSERT INTO processed_psp_events(...)` 4. return 200 On Tuesday during a brief RDS blip + deploy, the PSP retried the same `evt_1Qp…` ~900ms later while the first attempt was still open: - tx-ish path A: SELECT → 0 rows → credit +$1,200 → INSERT event → commit → 200 - path B: SELECT → 0 rows (A not committed) → credit +$1,200 → INSERT event → unique violation on `event_id` → **bare `catch` logged and returned 200** so the PSP would stop retrying Net: two credits, one inbox row, the PSP happy, us not. Support has reversal noise; finance will not approve another manual cycle. Root diagnosis (load-bearing): **idempotency was attached to the caller attempt / inbox row, not to the economic fact.** Returning 200 after a swallowed persistence error (or after "event seen") without proving the ledger effect exists is what makes the failure permanent under the PSP's at-least-once delivery. ## Proposed fix **A — Fact-owned idempotency + single transaction + advisory lock** ```sql BEGIN; SELECT pg_advisory_xact_lock(hashtext('psp:' || $event_id)); INSERT INTO ledger_facts (fact_type, fact_key, amount_cents, currency, account_id, source) VALUES ( 'wallet_topup_credit', 'psp_pi:' || $payment_intent_id || ':wallet:' || $wallet_id, $amount, $currency, $wallet_id, 'psp_webhook' ) ON CONFLICT (fact_type, fact_key) DO NOTHING RETURNING id; -- if no row returned: fact already exists; fall through to proof SELECT -- if row returned: write wallet_ledger_entries in THIS same transaction, -- then insert processed_psp_events COMMIT; ``` `fact_key` is the economic identity (PaymentIntent + wallet), not `Date.now()` and not "this handler invocation." Split top-ups / multiple PIs for one customer remain separate facts because the PI id differs. **B — Prove credit before 2xx (invariant)** After the transaction (or on conflict path): - `SELECT id FROM wallet_ledger_entries WHERE psp_event_id = $event_id OR (payment_intent_id = $pi AND entry_type = 'topup_credit')` - **Return 200 only if that proof row exists.** - If the inbox row exists but the credit does not → return 500 (or durable `failed_needs_repair`) so the PSP retries or on-call repairs. Never bare-catch `23505` into 200. **C — Defense in depth** Partial unique index: `UNIQUE (wallet_id, payment_intent_id) WHERE entry_type = 'topup_credit' AND direction = 'credit'` Replay path uses stable keys (`idempotency` / fact key derived from PI + wallet), never `replay_${event_id}_${Date.now()}`. **D — Reconcile** Nightly join: PSP events (72h) ↔ `ledger_facts` ↔ ledger sums; page if mismatch > $1. ## Explicit non-goals / surface clarity - This is the **asynchronous webhook** path (`charge.succeeded` / `payment_intent.succeeded`). Do **not** treat `issuing_authorization.request` as the same shape: that is a synchronous authorize endpoint with a ~2s response budget, not the PSP's at-least-once webhook replay. - Provider events may mint **typed** facts only through allowlisted mappers (e.g. `psp_charge_refund`, `connect_transfer_reversal`) with provider-native keys. They must not call a generic `createWalletCredit(orderOrPi)` helper used by internal clearers. ## What I'm asking 1. Does advisory lock + `ledger_facts` uniqueness + same-txn credit fully close the double-credit race under default READ COMMITTED? What if the process dies after commit of credit+fact but before the HTTP 200 leaves the process? 2. Is `fact_key = psp_pi:{pi}:wallet:{wallet}` granular enough, or do we need amount/currency in the key for partial capture / partial credit cases? 3. Is 200-only-after-proof correct when the credit exists from a prior successful attempt but `processed_psp_events` insert failed mid-repair? 4. Which other handlers copy SELECT-then-mutate-then-ack (`refund.succeeded`, `invoice.paid`, Connect `account.updated` side effects)? Same PR or sequenced? 5. Weakest part of this plan before we ship today against $340k in flight? Be direct. We want A+B+C in prod within the day if it holds; D can follow Friday. **GPT:** **I would ship A+B+C today, but not exactly as written.** The core design is sound; the weakest points are cross-event deduplication, an under-specified proof query, and rolling deployment with old workers still capable of crediting outside the new primitive. 1. **Race closure and crash behavior** A unique economic-fact row plus the ledger write in the same Postgres transaction closes the concurrent double-credit race under `READ COMMITTED`, provided every route that can create that credit uses the same constraint and transaction boundary. The unique index is the enforcement mechanism. The advisory lock is only an optimization. Your proposed lock is keyed by `event_id`, while `payment_intent.succeeded` and `charge.succeeded` are different events that can represent the same payment. An event-keyed advisory lock therefore does not serialize all contenders for the same economic fact. The `(fact_type, fact_key)` uniqueness still protects you, so either remove the lock or key it from the canonical fact key. I would remove it unless you have measured contention; fewer correctness primitives are better. With concurrent `INSERT … ON CONFLICT DO NOTHING`, Postgres arbitrates through the unique index: one transaction inserts, while the other waits and then takes the conflict path. Under `READ COMMITTED`, do the proof lookup as a subsequent statement, not as an assumption derived from an empty `RETURNING`. If the process dies after the transaction commits but before the HTTP 200 is delivered, the retry finds the existing fact and ledger effect, records this event if necessary, and returns 200 without another credit. That is precisely the crash case idempotency should make harmless. A connection loss during `COMMIT` is similarly an ambiguous outcome from Node’s perspective; retrying against the fact constraint resolves that ambiguity safely. I would tighten the schema so the relationship itself is enforceable: ```sql wallet_ledger_entries.ledger_fact_id REFERENCES ledger_facts(id); CREATE UNIQUE INDEX wallet_topup_credit_one_entry_per_fact ON wallet_ledger_entries (ledger_fact_id) WHERE entry_type = 'topup_credit' AND direction = 'credit'; ``` Keep your `(wallet_id, payment_intent_id)` partial unique index as defense in depth. A unique `ledger_fact_id` is stronger because it proves that a fact produced at most one ledger effect instead of relying on several denormalized columns continuing to agree. If you maintain a materialized wallet balance, update that balance in this same transaction as well. 2. **Fact-key granularity** Do **not** put amount or currency in the key. Including mutable attributes in an idempotency key turns a payload discrepancy into permission to create a second credit. Keep amount, currency, account, and provider identifiers as attributes of the fact, and on conflict compare them exactly. A mismatch should be quarantined and paged, not silently acknowledged as equivalent. The real question is whether the economic fact is “successful PaymentIntent” or “individual capture.” For a top-up credited once when a PaymentIntent reaches its final successful state, `PI + wallet` is the right identity. If your product credits partial or multiple captures incrementally, it is not granular enough: each provider-native capture, Charge, or balance-transaction identifier should become its own typed fact. Do not manufacture identity from amount or sequence position. I would encode provider and account scope explicitly, especially if Connect or multiple PSP accounts can produce overlapping identifier namespaces: ```text stripe:{stripe_account}:wallet_topup:{payment_intent_id}:wallet:{wallet_id} ``` Also make `account_id` part of the uniqueness definition rather than merely duplicating it inside an opaque string: ```sql UNIQUE (source, account_id, fact_type, fact_key) ``` The mapper should verify that a conflicting fact has the same wallet, amount, currency, and provider account before considering it successfully deduplicated. 3. **200 only after proof** Yes, but strengthen “proof.” Your current query is too permissive: ```sql payment_intent_id = $pi AND entry_type = 'topup_credit' ``` Without `wallet_id`, direction, currency, amount, and fact linkage, that query can prove the wrong wallet’s or wrong-shaped entry. Prefer a join from the canonical fact to exactly one ledger row and validate all immutable attributes: ```sql SELECT f.id, l.id FROM ledger_facts f JOIN wallet_ledger_entries l ON l.ledger_fact_id = f.id WHERE f.source = $source AND f.account_id = $provider_account AND f.fact_type = 'wallet_topup_credit' AND f.fact_key = $fact_key AND f.amount_cents = $amount AND f.currency = $currency AND l.wallet_id = $wallet_id AND l.direction = 'credit' AND l.entry_type = 'topup_credit'; ``` On an existing fact, insert or upsert `processed_psp_events` in the same retry transaction after validating that proof. It is safe to return 200 when the economic effect is already correct even if this particular event’s inbox row was previously absent; the inbox row is delivery audit state, not the source of financial idempotency. However, “the inbox insert failed mid-repair” should not be possible inside your new transaction while the credit remains committed. If fact, ledger entry, and inbox insertion are one transaction, an inbox failure rolls all new writes back. The only legitimate split state is legacy data or a later retry observing an already committed fact from another event. For valid financial events, transient DB failures should produce a non-2xx response. Permanent malformed or unauthorized requests should not be retried indefinitely; record and alert them according to the PSP’s documented status-code policy. 4. **Other handlers** Same PR for the shared financial mutation primitive and every currently enabled handler that can change a wallet balance. Do not attempt a broad rewrite of unrelated webhook side effects during this P0. `refund.succeeded`, transfer reversals, disputes, chargebacks, and any internal clearer capable of balance mutation have the same financial failure class and must not retain SELECT-then-mutate behavior. Give each a typed provider-native identity—refund ID for partial refunds, reversal ID for reversals, dispute ID or dispute transition where appropriate. `invoice.paid` only belongs in this immediate scope if it changes a wallet or entitlement whose duplication is financially material. `account.updated` generally needs idempotent state convergence or an outbox for external side effects, but it should not block today’s money-path fix. Search mechanically for: - existence checks followed by inserts or increments; - bare catches of `23505`; - `return 200` in catch/finally blocks; - wallet balance mutations not joined to a fact; - timestamps or random values in replay/idempotency keys; - handlers for both high- and low-level events that trigger the same business action. 5. **Weakest pre-ship risks** The biggest operational risk is the rolling deploy. If even one old pod remains able to execute the original credit path without the new uniqueness constraint, the system is still vulnerable during deployment. Apply and validate the database constraints first, then atomically switch all writers to the new primitive; drain or disable old consumers before enabling the new event mappings. Also, your proposed partial unique index may fail to build because you already have duplicates. Existing duplicate `(wallet_id, payment_intent_id)` rows prevent creation of that unique index until they are repaired or otherwise excluded. Preflight this immediately, and use an auditable compensating ledger entry rather than deleting financial history. I would not defer all reconciliation until Friday. The new code prevents future duplicates but does not identify latent duplicates or missing credits already created by the old path. Before opening traffic, run a targeted reconciliation across all in-flight top-ups and at least the retry/deploy incident window. Page on any nonzero monetary mismatch, not only mismatches over $1; aggregation can hide many small errors. Finally, ensure both `payment_intent.succeeded` and `charge.succeeded` map to the *same* top-up fact—or designate exactly one as authoritative. Event-ID idempotency cannot deduplicate those two distinct events; only the shared economic key or a single authoritative event mapper can. My ship gate would be: constraints validated, fact-linked ledger row, mismatch checks on conflict, no old writers alive, both event types tested concurrently, commit-response-loss tested, and an immediate reconciliation of today’s $340k. With those conditions, ship A+B+C today. > **GPT's snippets:** > - **CHALLENGE**: "Add `SET lock_timeout = '3s'` and on `try_lock` fail -> return 500 to trigger retry." — *This mixes blocking-lock and try-lock semantics, and a fixed three-second wait may consume too much of the webhook response budget. Since uniqueness already guarantees safety, I would remove the advisory lock unless contention data justifies it, or use a clearly bounded strategy with measured timeout headroom.* > - **EXPLORE**: "If balance is a cached column updated outside the same transaction as the ledger insert (or via triggers that can diverge), you can get correct ledger..." — *This is an important adjacent invariant that the proposal leaves unstated. The ship review should explicitly trace whether any materialized balance update shares the fact-and-ledger transaction.* > - **CHALLENGE**: "If old data has credit without inbox row: proof finds credit, you return 200 and asynchronously backfill inbox." — *The 200 decision is right, but I would converge the inbox synchronously in the retry transaction when the database is available. Making that repair asynchronous unnecessarily preserves incomplete audit state and creates another failure surface.* > - **KEEP**: "Insert compensating debit with its own fact `reversal_of:{fact_key}` linked to incident, not blind delete." — *Financial history should be corrected through an auditable compensating entry rather than destructive cleanup. The reversal itself also needs stable identity so incident tooling cannot apply it twice.* > - **KEEP**: "Do **not** stuff `amount` into the key “just in case” unless you have partials — it creates a second fact if a retry payload ever disagrees on amount ..." — *Amount and currency should be validated attributes, not identity components. Even with partials, I would prefer a provider-native capture or balance-transaction ID over adding amount to the key.* > - **CHALLENGE**: "**D** Friday as planned; until then, a one-off query on the launch customer + top N wallets is enough." — *With $340k already in flight through the old implementation, sampling only the launch customer and top wallets is too weak. I would require reconciliation of all current in-flight top-ups and the full incident window before reopening traffic.* > - **CORE**: "Two **different** events for the same economic fact (`payment_intent.succeeded` + `charge.succeeded`, or a rare Stripe redelivery with a new event id)..." — *This is the central flaw in the proposed lock scope: delivery identity and economic identity are not interchangeable. The unique fact constraint remains the actual correctness boundary.* **Muse:** This race is exactly what at-least-once + SELECT-then-mutate produces. Your diagnosis is right: idempotency was on attempt, not economic fact, and bare catch `23505 -> 200` made the failure permanent. ### 1. Does A+B close the race under READ COMMITTED? Crash after commit / before 200? Yes, if you fix scope. `pg_advisory_xact_lock` is transaction-scoped and released on commit/rollback, unlike `pg_advisory_lock` which is session-scoped and sticks in pools. `INSERT ... ON CONFLICT DO NOTHING RETURNING` returns 0 rows on conflict - conflicted row is not returned. Stripe/PSPs deliver at-least-once. Even after 2xx they may redeliver, and out-of-order is normal. Race close logic: * `UNIQUE(fact_type,fact_key)` + `ledger_facts` + `wallet_ledger_entries` + `processed_psp_events` in **one transaction** makes effect atomic. Under default READ COMMITTED, concurrent `INSERT fact` with same key serializes on the unique index - second blocks until first commits, then gets DO NOTHING path. That alone stops double credit. * Advisory lock is not strictly needed for safety, but valuable to serialize the proof SELECT and reduce retry storms. You currently lock `hashtext('psp:'||$event_id)` - wrong key. Two different `evt_*` (`payment_intent.succeeded` + `charge.succeeded` for same `pi_3M...`) carry same economic fact. Lock `fact_key`, not `event_id`: ```sql SELECT pg_advisory_xact_lock(hashtext('fact:'|| $fact_key)); ``` Crash after commit, before 200: safe if you do B. PSP didn't get 2xx, so it retries. On retry, `ledger_facts` conflict -> proof `SELECT id FROM wallet_ledger_entries WHERE wallet_id=$wallet AND payment_intent_id=$pi AND entry_type='topup_credit'` finds row -> return 200. Effect is exactly-once. ### 2. Should amount/currency be in `fact_key`? No. Including amount in idempotency key breaks idempotency when amount is corrected or FX-adjusted - you'd treat retry as new fact. Keep: `psp_pi:{pi}:wallet:{wallet}` * If partial captures are impossible for wallet top-ups (your case likely), this is sufficient. * If partial captures ARE possible via same PI with multiple charges, PI is insufficient as fact identity. Then key should be `psp_charge:{charge_id}:wallet:{wallet}` or `balance_transaction_id`. Amount still stays out of key. * Store `amount_cents,currency` in `ledger_facts`. On conflict path: if existing fact amount != incoming amount, don't credit, raise `fact_amount_mismatch` alert. That's your partial-capture detector. ### 3. 200-only-after-proof when credit exists but `processed_psp_events` failed? Yes, this is correct. Return 2xx = "I have this, don't retry", 5xx/timeout = "retry", 4xx = "don't retry" except 408/429. Returning 200 after swallowed persistence error is what made loss permanent. Invariant: **200 iff proof row for economic fact exists.** If old data has credit without inbox row: proof finds credit, you return 200 and asynchronously backfill inbox. Don't return 500 to re-create credit. If inbox exists but credit doesn't (old bug split-txn), return 500 so PSP retries -> new code path creates fact+credit+inbox atomically. Your proof query must also filter wallet_id: `WHERE (psp_event_id=$event_id) OR (wallet_id=$wallet AND payment_intent_id=$pi AND entry_type='topup_credit')` ### 4. Other handlers? Audit any handler that writes `wallet_ledger_entries`. Pattern `SELECT missing -> mutate -> INSERT inbox -> return 200` is the same bug class. * `refund.succeeded` / `charge.refunded`: definitely same PR if it credits/debits wallet. It's money. * `invoice.paid`: if it credits wallet, same PR. * `issuing_authorization.request`: you called out correctly - this is sync with ~2s budget, needs idempotency on idempotency-key header, not webhook dedup. Separate. * `account.updated` side effects: non-money, sequence Friday. Do not ship a generic `createWalletCredit(orderOrPi)`. Your non-goal is load-bearing. Force typed mappers `psp_charge_refund`, `connect_transfer_reversal` with provider-native keys. ### 5. Weakest part before ship today with $340k in flight? 1. **Lock scope -** `event_id` lock does not prevent `pi.succeeded` vs `charge.succeeded` race. Lock fact_key. Add `SET lock_timeout = '3s'` and on `try_lock` fail -> return 500 to trigger retry. 2. **Same connection txn -** In Node, ensure advisory lock + inserts share one `client` from pool inside `BEGIN/COMMIT`. Prisma `$transaction` or `pool.connect()`. Otherwise lock is no-op. 3. **Fast-ack antipattern -** Some guides say `res.json({received:true}); processAsync(event)` to avoid timeout. For money path that's wrong. You must hold request open while txn commits and proof succeeds, then 200. Move non-critical work out, but money stays sync. 4. **Replay key with Date.now() -** You flagged it. `replay_${event_id}_${Date.now()}` kills idempotency. Must be deterministic `psp_pi:...`. 5. **Missing partial unique index -** Ship it now: `UNIQUE (wallet_id, payment_intent_id) WHERE entry_type='topup_credit' AND direction='credit'`. This is your second defense when fact table logic has bug. 6. **Remediation for `wal_8a4f9` -** Don't auto-reverse. Query `SELECT wallet_id,payment_intent_id,count(*) FROM wallet_ledger_entries WHERE entry_type='topup_credit' GROUP BY 1,2 HAVING count(*)>1`. Insert compensating debit with its own fact `reversal_of:{fact_key}` linked to incident, not blind delete. Backfill `ledger_facts` from existing ledger before enabling. **Ship checklist today:** A fixed to lock fact_key + B proof-before-200 + C partial unique index, no `Date.now()`, no bare `catch 23505 -> 200`. D overnight join Friday. This closes double-credit under READ COMMITTED. > **Muse's snippets:** > - **CORE**: "Returning 200 on “we saw this event id” without proving the ledger effect is exactly how at-least-once delivery turns a transient persistence failure ..." — *This is the central invariant that made Tuesday permanent. 200-only-after-proof is the rule that makes retries recover instead of hide loss.* > - **EXPLORE**: "Prefer `hashtextextended` (64-bit) over `hashtext` (32-bit)." — *Good call — 32-bit hashtext is needlessly risky on money path, 64-bit via hashtextextended is trivial upgrade.* > - **CORE**: "A unique economic-fact row plus the ledger write in the same Postgres transaction closes the concurrent double-credit race under `READ COMMITTED`, pro..." — *This is the real fixer — uniqueness + same-txn is the guarantee, lock is just contention shaping. Perfect framing.* > - **KEEP**: "Do **not** put amount or currency in the key." — *Yes, amount in key would turn a mapper disagreement into a second fact. Keep amount as attribute to compare on conflict.* > - **KEEP**: "Your proposed lock is keyed by `event_id`, while `payment_intent.succeeded` and `charge.succeeded` are different events that can represent the same pa..." — *Agreed — my sketch keyed lock on event_id missed the cross-event case you both flagged. Switching lock to fact_key.* > - **CORE**: "Two **different** events for the same economic fact (`payment_intent.succeeded` + `charge.succeeded`, or a rare Stripe redelivery with a new event id)..." — *Sharp articulation of why event_id locking fails — same economic fact can have distinct delivery ids. This justifies fact-keyed lock.* **Grok:** ## Verdict Ship **A+B+C today**, with three hardenings and one deploy-order constraint. The core idea is right: move idempotency from “this delivery attempt” to “this economic fact,” keep the credit and the fact claim in one transaction, and never 2xx without ledger proof. That closes the Tuesday race as described. A few details still leave money on the floor if you copy the sketch literally. --- ### 1. Race under READ COMMITTED, and death after commit **Yes, it closes the double-credit race** — but the load-bearing piece is `ledger_facts` uniqueness + same-txn credit, not the advisory lock on `event_id`. Under default READ COMMITTED: - Two deliveries of the **same** `event_id` that both pass an old-style existence check are serialized if they take the same `pg_advisory_xact_lock`. Fine. - Two **different** events for the same economic fact (`payment_intent.succeeded` + `charge.succeeded`, or a rare Stripe redelivery with a new event id) take **different** locks if you key the lock on `$event_id`. They race. The unique constraint on `(fact_type, fact_key)` is what stops the second credit. `INSERT … ON CONFLICT DO NOTHING` is concurrency-safe for this under READ COMMITTED; the loser waits on the unique index, then takes the DO NOTHING path. So the advisory lock as written is defense-in-depth for same-event overlap, not the cross-event guarantee your root diagnosis actually needs. **Harden the lock key to the economic fact, not the delivery:** ```sql SELECT pg_advisory_xact_lock( hashtextextended('psp_pi:' || $payment_intent_id || ':wallet:' || $wallet_id, 0) ); ``` Prefer `hashtextextended` (64-bit) over `hashtext` (32-bit). Collisions on `hashtext` are unlikely in practice but the wrong risk class for a money path when a 64-bit primitive exists. **Process dies after commit, before the HTTP 200 leaves the box:** this is fine **if and only if B is real**. Fact + ledger are durable; PSP retries (timeout / connection drop); retry takes the ON CONFLICT path; proof SELECT finds the credit; you return 200. That is the correct terminal state. PSP at-least-once delivery plus “2xx only after proof” is exactly the pattern that survives post-commit crashes. What is **not** fine: any path that commits the credit then returns 200 without the proof SELECT, or that treats “inbox row exists” as success. You already called that out — keep it absolute. Also put `processed_psp_events` in the **same** transaction as fact + ledger on the insert-winning path. On the conflict path, still **upsert** the `event_id` (so audit/inbox converges) but never let inbox presence alone authorize 200. --- ### 2. Is `fact_key = psp_pi:{pi}:wallet:{wallet}` enough? **For one-shot top-ups of a full PI amount: yes.** Split top-ups via different PIs stay separate. Same PI → same wallet cannot credit twice. That matches the intended invariant. **Include more only if your product actually has these shapes:** | Case | Risk with current key | Fix | |---|---|---| | You fulfill on **both** `payment_intent.succeeded` and `charge.succeeded` | None, if both use this same fact_key | Shared mapper, one fact type | | Multi-capture / multiple charges per PI | Second capture suppressed | Key on `charge_id` (or PI+charge) | | Partial credits / amount corrections | Wrong amount locked in forever | Either key includes amount **or** fact is “PI settled” and amount is taken only from the first winning insert with an explicit amount check | | Same PI, different wallets (platform split) | Correctly allowed (wallet in key) | Keep wallet in key | For a B2B wallet top-up product that treats one succeeded PI as one full credit, PI+wallet is the right grain. Do **not** stuff `amount` into the key “just in case” unless you have partials — it creates a second fact if a retry payload ever disagrees on amount (serialization, FX, or a buggy mapper), and you can double-credit under a different key. **Operational rule:** allowlisted mappers only; both `payment_intent.succeeded` and `charge.succeeded` (if both are wired) must mint `wallet_topup_credit` with the **identical** `fact_key` derivation. No generic `createWalletCredit(orderOrPi)` from random handlers. --- ### 3. 200 only after proof — including the “credit yes, inbox no” case **Yes. Correct.** - Credit exists (by `payment_intent_id` / fact / ledger proof) → **200**, even if `processed_psp_events` is missing or the current insert races. Money invariant holds; inbox is healable. - Inbox exists, credit missing → **5xx** or durable `failed_needs_repair`. Never 200. This is the state that made Tuesday permanent when you swallowed `23505`. - Neither exists → process in the transaction; then proof; then 200 or 5xx. Returning 200 on “we saw this event id” without proving the ledger effect is exactly how at-least-once delivery turns a transient persistence failure into a permanent silent wrong balance. Do **not** follow the common “return 2xx immediately, process async” blog advice on this path unless the 200 is gated on durable enqueue to a worker that itself implements A+B. For a same-day money fix, in-request transaction + proof is simpler and safer. Watch handler duration against the PSP’s timeout window; keep the locked section DB-only (no outbound HTTP inside the transaction). On status codes: 2xx = stop retry (including intentional skip/duplicate); 5xx = retry; 4xx = terminal “do not retry.” Never map unique violations or “already processed” to 4xx, and never bare-catch them into 200 without proof. Proof query should prefer the **fact/ledger identity**, not only `psp_event_id`: ```sql SELECT 1 FROM wallet_ledger_entries WHERE entry_type = 'topup_credit' AND direction = 'credit' AND payment_intent_id = $pi AND wallet_id = $wallet -- optional: OR psp_event_id = $event_id for audit linkage ``` Relying only on `psp_event_id` fails open when the winning credit was written under a different event id for the same PI. --- ### 4. Other handlers — same PR or sequenced? **Same day for anything that writes `wallet_ledger_entries` or moves customer balances.** Sequenced for the rest. Money-path clones of SELECT → mutate → ack (fix first): - `charge.succeeded` / `payment_intent.succeeded` (this bug) - `charge.refunded` / `refund.succeeded` / `payment_intent.canceled` (reverse or clawback) - `charge.dispute.*` / funds withdrawn - Connect / platform: `transfer.created`, `transfer.reversed`, `payout.*` if they touch wallets - `invoice.paid` only if it credits wallets in this product Non-money or sync paths: leave alone today. - `issuing_authorization.request` — different shape (sync, ~2s budget); do not force this webhook pattern onto it. - `account.updated`, customer metadata, etc. — inbox idempotency is enough; no ledger fact required. If `charge.succeeded` and `payment_intent.succeeded` both credit today, fixing only one handler leaves a dual-event double-credit open under a different event_id. That is the highest-priority sibling. --- ### 5. Weakest parts before you ship against $340k in flight Ranked by “can still lose money or block the deploy”: 1. **Partial unique index C will not apply cleanly while duplicates exist.** You already have two credits for the same `payment_intent_id` on `wal_8a4f9`. `CREATE UNIQUE INDEX …` fails if any duplicate rows match the predicate. **Order:** (a) freeze/repair known doubles for in-flight wallets, (b) add `ledger_facts` + handler code, (c) build the unique index (`CONCURRENTLY`), (d) only then rely on it as hard stop. Soft-launch the handler without C is still a large improvement; C is the belt. 2. **Cross-event credit without shared fact_key.** Lock-on-`event_id` + fact_key-on-PI only works if every credit entrypoint uses the fact table. Grep for every `wallet_ledger_entries` insert and every `creditWallet` call. One stray path undoes A. 3. **Denormalized wallet balance.** If balance is a cached column updated outside the same transaction as the ledger insert (or via triggers that can diverge), you can get correct ledger + wrong displayed balance, or double-increment under concurrency. Ledger-as-source-of-truth with balance derived in-txn (or recomputed) is the safe model; confirm which you have. 4. **Deploy race with live retries.** During rollout, old pods still run SELECT→credit→INSERT→catch-200. New pods run A+B. A dual-credit can still land if old code wins a race against new code for the same PI. **Mitigate:** drain/stop old webhook consumers before green traffic; or ship C (unique index) **before** or **with** the handler change so old code’s second INSERT dies on constraint — and make sure old code does **not** turn that into 200 without proof (if you cannot patch old code, prefer hard cutover). 5. **Proof/repair gap on historical inconsistency.** A+B+C prevent new doubles; they do not fix `wal_8a4f9`. Finance already hates manual cycles — script a one-shot: for each PI with `count(*) > 1` topup credits, leave one, reverse extras with a linked `entry_type = 'topup_credit_reversal'` and a fact row so webhooks cannot recreate them. 6. **“Return 200 ASAP / async” temptation under load.** Several Stripe guides push immediate 2xx. On this path that reintroduces “acked but not credited” unless you have an outbox. Do not take that shortcut today. 7. **Minor:** advisory lock on delivery id (as in the sketch) over-sells what it protects; switch to fact-shaped lock key. Not ship-blocking if `ledger_facts` is mandatory on every credit path. --- ### What I would ship today (minimal sequence) 1. **Data:** identify all duplicate `(wallet_id, payment_intent_id)` topup credits; reverse extras with stable fact keys; confirm `wal_8a4f9`. 2. **Schema:** `ledger_facts (fact_type, fact_key PRIMARY KEY or UNIQUE, …)` + columns you need for audit. 3. **Handler (payment_intent + charge succeeded only):** `BEGIN` → fact-keyed `pg_advisory_xact_lock` → `INSERT ledger_facts ON CONFLICT DO NOTHING RETURNING` → on win, insert ledger (+ balance update if any) + `processed_psp_events` → `COMMIT` → **proof SELECT** → 200 iff proof, else 5xx. No bare-catch of `23505` into 200. 4. **Constraint C** via `CREATE UNIQUE INDEX CONCURRENTLY` after step 1. 5. **Grep gate:** no other code path inserts `topup_credit` without going through the fact claim. 6. **D** Friday as planned; until then, a one-off query on the launch customer + top N wallets is enough. ### Direct answers to your five | # | Answer | |---|---| | 1 | Yes under READ COMMITTED if fact uniqueness is on the economic key and credit is same-txn; lock should be on that key too. Death after commit before 200 is safe with B. | | 2 | PI+wallet is enough for single full top-up; use charge-level keys only if multi-capture exists; don’t add amount unless partials are real. | | 3 | Yes — 200 iff credit proof; heal inbox asynchronously; 5xx only when credit missing. | | 4 | Same PR for every ledger-writing webhook; especially the dual PI/charge pair; not for sync issuing auth. | | 5 | Weakest: existing duplicates blocking C, dual event handlers, old pods during deploy, and any balance cache outside the txn. | One more hard line: **if proof cannot be established, do not 200.** That single rule is what turns PSP retries from a liability into the recovery mechanism. The rest is making sure two retries cannot mint two facts. > **Grok's snippets:** > - **EXPLORE**: "If partial captures ARE possible via same PI with multiple charges, PI is insufficient as fact identity. Then key should be `psp_charge:{charge_id}:wa..." — *Worth confirming with product whether multi-capture top-ups exist before we freeze PI-grain forever.* > - **CHALLENGE**: "I would remove it unless you have measured contention; fewer correctness primitives are better." — *I'd still keep a fact-keyed xact lock for this money path to serialize proof and cut retry storms, but the bar for keeping it is fair—only if we have a reason beyond cargo-cult.* > - **SHIFT**: "CREATE UNIQUE INDEX wallet_topup_credit_one_entry_per_fact ON wallet_ledger_entries (ledger_fact_id) WHERE entry_type = 'topup_credit' AND directi..." — *Before I leaned on the denormalized (wallet_id, payment_intent_id) partial unique; a unique ledger_fact_id FK is a tighter invariant that one fact yields at most one credit effect.* > - **SHIFT**: "I would not defer all reconciliation until Friday. The new code prevents future duplicates but does not identify latent duplicates or missing credits ..." — *I had D as Friday; with $340k in flight and known doubles, pre-traffic recon of the incident window is the right gate, not a follow-up.* > - **KEEP**: "Don't auto-reverse. Query `SELECT wallet_id,payment_intent_id,count(*) FROM wallet_ledger_entries WHERE entry_type='topup_credit' GROUP BY 1,2 HAVING ..." — *Compensating ledger entries with their own facts are the only repair that stays consistent with A+B and finance audit.* > - **KEEP**: "You currently lock `hashtext('psp:'||$event_id)` - wrong key. Two different `evt_*` (`payment_intent.succeeded` + `charge.succeeded` for same `pi_3M....." — *Same hardening I want: lock must be fact-shaped or it fails open on the dual-event path that actually bit us.* > - **CORE**: "The unique index is the enforcement mechanism. The advisory lock is only an optimization." — *This is the load-bearing distinction: uniqueness on the economic fact is what actually closes the race; the lock is secondary.* > - **KEEP**: "Do **not** put amount or currency in the key. Including mutable attributes in an idempotency key turns a payload discrepancy into permission to create..." — *Exactly right—amount belongs as an attribute checked on conflict, not as part of identity.* ### Reactions (claim map) > **Claim** — GPT: "Do **not** put amount or currency in the key. Including mutable attributes in an idempotency key turns a payload discrepancy into permission to create a second credit." > - KEEP by Grok — "Exactly right—amount belongs as an attribute checked on conflict, not as part of identity." > - KEEP by Muse — "Yes, amount in key would turn a mapper disagreement into a second fact. Keep amount as attribute to compare on conflict." > **Claim** — GPT: "A unique economic-fact row plus the ledger write in the same Postgres transaction closes the concurrent double-credit race under `READ COMMITTED`, provided every route that can create that credit uses the same constraint and transaction boundary." > - CORE by Muse — "This is the real fixer — uniqueness + same-txn is the guarantee, lock is just contention shaping. Perfect framing." > **Claim** — GPT: "CREATE UNIQUE INDEX wallet_topup_credit_one_entry_per_fact ON wallet_ledger_entries (ledger_fact_id) WHERE entry_type = 'topup_credit' AND direction = 'credit'" > - SHIFT by Grok — "Before I leaned on the denormalized (wallet_id, payment_intent_id) partial unique; a unique ledger_fact_id FK is a tighter invariant that one fact yields at most one credit effect." > **Claim** — GPT: "I would not defer all reconciliation until Friday. The new code prevents future duplicates but does not identify latent duplicates or missing credits already created by the old path. Before opening traffic, run a targeted reconciliation across all in-flight top-ups and at least the retry/deploy incident window." > - SHIFT by Grok — "I had D as Friday; with $340k in flight and known doubles, pre-traffic recon of the incident window is the right gate, not a follow-up." > **Claim** — GPT: "I would remove it unless you have measured contention; fewer correctness primitives are better." > - CHALLENGE by Grok — "I'd still keep a fact-keyed xact lock for this money path to serialize proof and cut retry storms, but the bar for keeping it is fair—only if we have a reason beyond cargo-cult." > - EXPLORE by Moderator — "@Grok challenged: keep it fact-keyed for proof serialization and retry-storm suppression. Resolve with mechanism, not taste: what does each position cost under a retry burst?" > **Claim** — GPT: "The unique index is the enforcement mechanism. The advisory lock is only an optimization." > - CORE by Grok — "This is the load-bearing distinction: uniqueness on the economic fact is what actually closes the race; the lock is secondary." > - CORE by Moderator — "The locked floor. Q1 is only about whether the optimization earns its keep." > **Claim** — GPT: "Your proposed lock is keyed by `event_id`, while `payment_intent.succeeded` and `charge.succeeded` are different events that can represent the same payment." > - KEEP by Muse — "Agreed — my sketch keyed lock on event_id missed the cross-event case you both flagged. Switching lock to fact_key." > **Claim** — Grok: "Two **different** events for the same economic fact (`payment_intent.succeeded` + `charge.succeeded`, or a rare Stripe redelivery with a new event id) take **different** locks if you key the lock on `$event_id`." > - CORE by GPT — "This is the central flaw in the proposed lock scope: delivery identity and economic identity are not interchangeable. The unique fact constraint remains the actual correctness boundary." > - CORE by Muse — "Sharp articulation of why event_id locking fails — same economic fact can have distinct delivery ids. This justifies fact-keyed lock." > **Claim** — Grok: "**D** Friday as planned; until then, a one-off query on the launch customer + top N wallets is enough." > - CHALLENGE by GPT — "With $340k already in flight through the old implementation, sampling only the launch customer and top wallets is too weak. I would require reconciliation of all current in-flight top-ups and the full incident window before reopening traffic." > **Claim** — Grok: "Do **not** stuff `amount` into the key “just in case” unless you have partials — it creates a second fact if a retry payload ever disagrees on amount (serialization, FX, or a buggy mapper), and you can double-credit under a different key." > - KEEP by GPT — "Amount and currency should be validated attributes, not identity components. Even with partials, I would prefer a provider-native capture or balance-transaction ID over adding amount to the key." > **Claim** — Grok: "If balance is a cached column updated outside the same transaction as the ledger insert (or via triggers that can diverge), you can get correct ledger + wrong displayed balance, or double-increment under concurrency." > - EXPLORE by GPT — "This is an important adjacent invariant that the proposal leaves unstated. The ship review should explicitly trace whether any materialized balance update shares the fact-and-ledger transaction." > **Claim** — Grok: "Prefer `hashtextextended` (64-bit) over `hashtext` (32-bit)." > - EXPLORE by Muse — "Good call — 32-bit hashtext is needlessly risky on money path, 64-bit via hashtextextended is trivial upgrade." > **Claim** — Grok: "Returning 200 on “we saw this event id” without proving the ledger effect is exactly how at-least-once delivery turns a transient persistence failure into a permanent silent wrong balance." > - CORE by Muse — "This is the central invariant that made Tuesday permanent. 200-only-after-proof is the rule that makes retries recover instead of hide loss." > **Claim** — Muse: "Don't auto-reverse. Query `SELECT wallet_id,payment_intent_id,count(*) FROM wallet_ledger_entries WHERE entry_type='topup_credit' GROUP BY 1,2 HAVING count(*)>1`. Insert compensating debit with its own fact `reversal_of:{fact_key}` linked to incident, not blind delete." > - KEEP by GPT — "Financial history should be corrected through an auditable compensating entry rather than destructive cleanup. The reversal itself also needs stable identity so incident tooling cannot apply it twice." > - KEEP by Grok — "Compensating ledger entries with their own facts are the only repair that stays consistent with A+B and finance audit." > **Claim** — Muse: "Add `SET lock_timeout = '3s'` and on `try_lock` fail -> return 500 to trigger retry." > - CHALLENGE by GPT — "This mixes blocking-lock and try-lock semantics, and a fixed three-second wait may consume too much of the webhook response budget. Since uniqueness already guarantees safety, I would remove the advisory lock unless contention data justifies it, or use a clearly bounded strategy with measured timeout headroom." > **Claim** — Muse: "If old data has credit without inbox row: proof finds credit, you return 200 and asynchronously backfill inbox." > - CHALLENGE by GPT — "The 200 decision is right, but I would converge the inbox synchronously in the retry transaction when the database is available. Making that repair asynchronous unnecessarily preserves incomplete audit state and creates another failure surface." > **Claim** — Muse: "If partial captures ARE possible via same PI with multiple charges, PI is insufficient as fact identity. Then key should be `psp_charge:{charge_id}:wallet:{wallet}` or `balance_transaction_id`. Amount still stays out of key." > - EXPLORE by Grok — "Worth confirming with product whether multi-capture top-ups exist before we freeze PI-grain forever." > **Claim** — Muse: "You currently lock `hashtext('psp:'||$event_id)` - wrong key. Two different `evt_*` (`payment_intent.succeeded` + `charge.succeeded` for same `pi_3M...`) carry same economic fact. Lock `fact_key`, not `event_id`" > - KEEP by Grok — "Same hardening I want: lock must be fact-shaped or it fails open on the dual-event path that actually bit us." > **Claim** — Grok: "During rollout, old pods still run SELECT→credit→INSERT→catch-200. New pods run A+B. A dual-credit can still land if old code wins a race against new code for the same PI." > - EXPLORE by Moderator — "This is the risk your runbooks must kill by construction. Show the ordering that makes it impossible, not unlikely." ### Takeaway **The proposed fix works for preventing future double-credits, provided you lock on the economic fact rather than the event ID and stop treating the inbox row as a sufficient proof of completion.** #### Unanimous: Lock on the economic fact, not the event ID Advisory locks keyed to `$event_id` fail because multiple events (e.g., `payment_intent.succeeded` and `charge.succeeded`) can represent the same economic fact. Use a stable `fact_key` (like `psp_pi:{pi}:wallet:{wallet}`) to lock and serialize all contenders for that specific credit. > **Claim** — Grok: "Two **different** events for the same economic fact (`payment_intent.succeeded` + `charge.succeeded`, or a rare Stripe redelivery with a new event id) take **different** locks if you key the lock on `$event_id`." > - CORE by GPT — "This is the central flaw in the proposed lock scope: delivery identity and economic identity are not interchangeable. The unique fact constraint remains the actual correctness boundary." > - CORE by Muse — "Sharp articulation of why event_id locking fails — same economic fact can have distinct delivery ids. This justifies fact-keyed lock." > **Claim** — Muse: "You currently lock `hashtext('psp:'||$event_id)` - wrong key. Two different `evt_*` (`payment_intent.succeeded` + `charge.succeeded` for same `pi_3M...`) carry same economic fact. Lock `fact_key`, not `event_id`" > - KEEP by Grok — "Same hardening I want: lock must be fact-shaped or it fails open on the dual-event path that actually bit us." > **Claim** — GPT: "Your proposed lock is keyed by `event_id`, while `payment_intent.succeeded` and `charge.succeeded` are different events that can represent the same payment." > - KEEP by Muse — "Agreed — my sketch keyed lock on event_id missed the cross-event case you both flagged. Switching lock to fact_key." #### Majority: Enforce 200-only-after-proof of ledger credit Returning 200 after a persistence error or merely seeing an event is why your failures became permanent; it hides the fact that a credit didn't happen. A 200 should only be returned if you can query the database and prove the ledger entry exists for that fact. > **Claim** — Grok: "Returning 200 on “we saw this event id” without proving the ledger effect is exactly how at-least-once delivery turns a transient persistence failure into a permanent silent wrong balance." > - CORE by Muse — "This is the central invariant that made Tuesday permanent. 200-only-after-proof is the rule that makes retries recover instead of hide loss." #### Unanimous: Do not include mutable attributes in idempotency keys Including amount or currency in your `fact_key` creates a second, valid fact if the payload ever changes (e.g., FX adjustment, mapper bug), effectively bypassing your idempotency. Keep the key tied to identity (PI + wallet) and use amount/currency as attributes to validate on conflict. > **Claim** — GPT: "Do **not** put amount or currency in the key. Including mutable attributes in an idempotency key turns a payload discrepancy into permission to create a second credit." > - KEEP by Grok — "Exactly right—amount belongs as an attribute checked on conflict, not as part of identity." > - KEEP by Muse — "Yes, amount in key would turn a mapper disagreement into a second fact. Keep amount as attribute to compare on conflict." > **Claim** — Grok: "Do **not** stuff `amount` into the key “just in case” unless you have partials — it creates a second fact if a retry payload ever disagrees on amount (serialization, FX, or a buggy mapper), and you can double-credit under a different key." > - KEEP by GPT — "Amount and currency should be validated attributes, not identity components. Even with partials, I would prefer a provider-native capture or balance-transaction ID over adding amount to the key." #### Unanimous: Correct existing duplicates with compensating entries Do not delete records or blindly reverse duplicates; financial history requires an audit trail. Insert a compensating debit with its own unique `reversal_of:{fact_key}` fact to ensure the reversal itself cannot be applied twice. > **Claim** — Muse: "Don't auto-reverse. Query `SELECT wallet_id,payment_intent_id,count(*) FROM wallet_ledger_entries WHERE entry_type='topup_credit' GROUP BY 1,2 HAVING count(*)>1`. Insert compensating debit with its own fact `reversal_of:{fact_key}` linked to incident, not blind delete." > - KEEP by GPT — "Financial history should be corrected through an auditable compensating entry rather than destructive cleanup. The reversal itself also needs stable identity so incident tooling cannot apply it twice." > - KEEP by Grok — "Compensating ledger entries with their own facts are the only repair that stays consistent with A+B and finance audit." #### Unanimous: The unique index is your primary safety guard While the advisory lock helps reduce contention, your real correctness boundary is the unique constraint on the economic fact record (`ledger_facts`). Ensure this transaction is atomic, as the unique index will handle the serialization under `READ COMMITTED` even if the lock is misconfigured. > **Claim** — GPT: "The unique index is the enforcement mechanism. The advisory lock is only an optimization." > - CORE by Grok — "This is the load-bearing distinction: uniqueness on the economic fact is what actually closes the race; the lock is secondary." > - CORE by Moderator — "The locked floor. Q1 is only about whether the optimization earns its keep." > **Claim** — GPT: "A unique economic-fact row plus the ledger write in the same Postgres transaction closes the concurrent double-credit race under `READ COMMITTED`, provided every route that can create that credit uses the same constraint and transaction boundary." > - CORE by Muse — "This is the real fixer — uniqueness + same-txn is the guarantee, lock is just contention shaping. Perfect framing." --- ### The human's take > **Core** from GPT: > > "The unique index is the enforcement mechanism. The advisory lock is only an optimization." > *The locked floor. Q1 is only about whether the optimization earns its keep.* > → sent to GPT, Grok, Muse > **Explored** from GPT: > > "I would remove it unless you have measured contention; fewer correctness primitives are better." > *@Grok challenged: keep it fact-keyed for proof serialization and retry-storm suppression. Resolve with mechanism, not taste: what does each position cost under a retry burst?* > → sent to GPT, Grok, Muse > **Explored** from Grok: > > "During rollout, old pods still run SELECT→credit→INSERT→catch-200. New pods run A+B. A dual-credit can still land if old code wins a race against new ..." > *This is the risk your runbooks must kill by construction. Show the ordering that makes it impossible, not unlikely.* > → sent to GPT, Grok, Muse ## Round 2 **User:** Round 2 — convergence. The floor is locked, all three of you converged independently: fact-owned uniqueness + same-transaction credit is the enforcement mechanism; proof-before-200 is absolute (200 iff ledger proof, 5xx when credit missing, never bare-catch 23505 into 200); the lock as written is keyed to delivery identity and the cross-event `payment_intent.succeeded`/`charge.succeeded` path is the real flaw. Stipulated as settled, do not re-litigate: (1) amount/currency stay OUT of the fact key and are compared-on-conflict attributes with a `fact_amount_mismatch` quarantine+page; (2) fact key is PI+wallet at today's grain, provider/account-scoped (`UNIQUE (source, account_id, fact_type, fact_key)`), charge-level keys only if product confirms multi-capture; (3) @Grok's two reaction-shifts are binding — the fact-linked `UNIQUE (ledger_fact_id)` index ships alongside the denormalized index, and reconciliation of ALL in-flight top-ups + the incident window runs BEFORE reopening traffic; (4) repair is compensating debits with their own `reversal_of:{fact_key}` facts, never deletes; (5) existing duplicates must be repaired before the C index can build. Two questions remain. Resolve them. **Q1 — the advisory lock's fate.** Sol: remove it — uniqueness already guarantees safety, fewer correctness primitives are better, and @Muse's `lock_timeout='3s'` mixes blocking and try-lock semantics inside a webhook response budget. @Grok: keep it fact-keyed (`hashtextextended`, 64-bit) to serialize the proof SELECT and suppress retry storms. @Muse: keep with bounded timeout. Pick ONE lock policy with exact semantics — none / blocking `pg_advisory_xact_lock` on fact key / bounded-wait variant — and defend it against the strongest peer objection: Sol, what specifically breaks or degrades without the lock under a PSP retry burst against a hot fact? @Grok/@Muse, what does the lock buy that the unique index's own lock-wait arbitration doesn't already provide? **Q2 — the same-day deploy runbook.** You each named overlapping ship risks (old pods racing new code, duplicates blocking the index build, balance-cache divergence, dual event types minting through one mapper) but no one produced a single ordered sequence. Write the numbered runbook you would sign: dedup/repair → schema → handler cutover → index build (CONCURRENTLY or not, before or after traffic switch) → old-consumer drain → reconciliation gate → reopen. State explicitly where the deploy can be rolled back safely and where it cannot. Fold in the sync-vs-async inbox heal (Sol challenged @Muse's async backfill — settle it in the runbook line where it lives). If your runbook differs from a peer's on ordering, say whose is wrong and why. **GPT:** ## Q1 — remove the advisory lock My signed policy is **no advisory lock**. The unique index already performs the necessary serialization: concurrent inserts for the same fact wait on PostgreSQL’s unique-index arbitration, one commits, and the others take `ON CONFLICT DO NOTHING`. A fact-keyed advisory lock places essentially the same contenders into an additional queue before they reach the authoritative queue. It does not reduce the number of requests, database connections, or transactions during a retry burst. After a conflict under `READ COMMITTED`, the proof must be a subsequent SQL statement. That subsequent statement receives a new statement snapshot and can see the winner’s committed fact and ledger row. The proof therefore does not need an advisory lock to “serialize” it. Against the strongest objection—one hot fact receiving a PSP retry burst—the behavior without the advisory lock is: 1. The first transaction attempts the fact insert. 2. Concurrent contenders wait on the unique-index conflict. 3. Once the winner commits, contenders resolve to `DO NOTHING`. 4. Each performs a cheap indexed proof lookup and exits. A fact-keyed advisory lock would make those contenders wait at the advisory lock and then perform the same insert/conflict/proof sequence one by one. That may move where waiting occurs, but it does not remove waiting. In fact, it can lengthen the tail because losers that could quickly prove an already committed fact remain serialized through the advisory lock. The bounded-wait proposal is worse. A blocking `pg_advisory_xact_lock` plus `lock_timeout` is coherent, and `pg_try_advisory_xact_lock` is coherent, but combining their semantics is not. Either bounded variant converts harmless contention into 5xx retries, amplifying the retry burst. A three-second lock timeout also consumes webhook latency budget without creating additional safety. There are additional costs: lock-key hashing must be identical across every writer, lock acquisition order becomes relevant if a future transaction touches multiple facts, and hash collisions create unrelated contention. None of these necessarily causes a correctness failure, but they add mechanisms to operate without adding an invariant. So I disagree with @Grok and @Muse here: “retry-storm suppression” is not what the lock accomplishes. It shifts the queue and may serialize the post-commit fast path unnecessarily. Ship without it. If production measurements later show pathological unique-index contention, solve that measured problem separately rather than adding a second lock to today’s P0. ## Q2 — runbook I would sign The critical construction is a **quiesced cutover**. There must never be a period in which old and new credit writers are both eligible to consume top-up events. 1. **Install the traffic kill switch and identify every writer.** Add or verify a server-side flag that disables all `topup_credit` writers—not merely the two webhook routes. Enumerate webhook pods, background replay workers, admin repair tooling, internal clearers, and scheduled jobs. While disabled, webhook requests receive a retriable 5xx; they must not receive a synthetic 200. Exercise the switch before relying on it. A flag that only blocks one handler while a replay worker remains active does not establish quiescence. 2. **Apply additive schema while the old service is still live.** Create `ledger_facts`, add nullable `wallet_ledger_entries.ledger_fact_id`, add the foreign key in a deployment-safe form, and install the supporting non-unique indexes. Create the fact uniqueness constraint: ```sql UNIQUE (source, account_id, fact_type, fact_key) ``` Do not yet enable new event handling. This stage is safely reversible because no production writer depends on the new structures. 3. **Close top-up intake and drain old execution completely.** Flip the kill switch so new top-up webhook attempts get 5xx. Stop replay jobs and admin mutation tools. Drain in-flight requests and transactions, then verify through application metrics and `pg_stat_activity` that no old top-up mutation is still executing. Remove old pods from service after they drain. Deploy the new image dark—healthy but still behind the disabled money-path flag. Do not perform a normal rolling overlap between old and new consumers. 4. **Reconcile and repair while writers are quiesced.** Reconcile every currently in-flight top-up and the complete incident window, including `wal_8a4f9`. Identify duplicate credits, missing credits, payload mismatches, cross-wallet PI reuse, and discrepancies between ledger sums and any materialized wallet balance. Correct duplicates with compensating debit entries carrying stable `reversal_of:{fact_key}` facts and incident linkage. Never delete a ledger row. There is a schema subtlety here: a compensating debit does not make the historical duplicate credit disappear, so the literal partial index over all historical `topup_credit` rows would still fail. Preserve the historical rows and make the denormalized uniqueness constraint apply to canonical fact-linked credits: ```sql CREATE UNIQUE INDEX CONCURRENTLY ... ON wallet_ledger_entries(wallet_id, payment_intent_id) WHERE entry_type = 'topup_credit' AND direction = 'credit' AND ledger_fact_id IS NOT NULL; ``` Backfill one canonical credit per valid PI/wallet to the canonical `ledger_fact`; leave compensated legacy duplicate credits distinguishable and unlinked, or explicitly mark them superseded in auditable metadata. Linking both historical duplicate credits to the same fact would itself violate the new one-ledger-row-per-fact constraint. 5. **Build and validate both ledger constraints while top-up writers remain disabled.** Build: ```sql UNIQUE (ledger_fact_id) WHERE entry_type = 'topup_credit' AND direction = 'credit' AND ledger_fact_id IS NOT NULL ``` and the denormalized PI/wallet index above. Use `CREATE UNIQUE INDEX CONCURRENTLY` if unrelated ledger traffic must continue. Check that the index is valid afterward; a failed concurrent build can leave an invalid index that must be dropped and rebuilt. If the maintenance window permits quiescing all ledger writers, a normal unique-index build is operationally simpler. Do not reopen merely because the DDL command returned. Run explicit duplicate queries that mirror both index predicates. 6. **Activate the new handler while intake remains closed.** Both `payment_intent.succeeded` and `charge.succeeded` must call the same allowlisted top-up mapper and derive exactly the same provider/account-scoped PI+wallet fact identity. The transaction is: ```text BEGIN claim fact with ON CONFLICT DO NOTHING if won: insert fact-linked ledger row update materialized balance, if one exists prove exact fact-linked ledger effect and compare amount/currency upsert processed_psp_events for this delivery COMMIT return 2xx only after committed proof ``` Any materialized wallet balance update belongs in that transaction. A fact and ledger row committed atomically do not protect a cached balance updated in a later transaction. On the existing-fact path, attempt the inbox upsert **synchronously in the current request**, not through an asynchronous repair job. Inbox state should normally converge before the response. However, inbox presence is not financial proof and must never authorize the response by itself. If a legacy fact-linked credit is proven but inbox repair encounters an isolated audit-write failure, the already-proven credit remains the criterion for avoiding another economic effect. Record and reconcile that audit defect; do not recreate the credit. If the database failure prevents reliable proof or leaves transaction outcome ambiguous, return 5xx and let the fact constraint make the retry safe. 7. **Run adversarial tests against the dark deployment.** At minimum: - same event delivered concurrently; - `payment_intent.succeeded` and `charge.succeeded` concurrently; - process death before commit; - process death after commit but before HTTP 200; - amount or currency mismatch on an existing fact; - existing credit with missing inbox row; - inbox row with missing credit; - materialized-balance consistency; - replay tooling using the same deterministic fact key. Confirm that no catch block maps `23505` to 200 without exact ledger proof. 8. **Perform the mandatory reconciliation gate.** Re-run reconciliation for all in-flight top-ups and the entire incident window after migration/backfill and before reopening. The gate is zero unexplained discrepancy—not “top wallets look clean” and not “mismatch under $1.” 9. **Reopen only the new path.** Enable the new top-up writer and route webhook traffic exclusively to new pods. Keep old pods terminated, old replay workers disabled or upgraded, and generic credit tooling blocked. Monitor fact conflicts, proof failures, amount mismatches, webhook latency, PSP retries, and ledger-versus-balance divergence. @Grok’s earlier suggestion that a one-off query over the launch customer and top wallets would suffice is too weak against $340k already in flight. Sampling cannot establish the absence of a long-tail duplicate. @Muse’s asynchronous inbox backfill is also unnecessary: attempt convergence synchronously on the retry path; reserve reconciliation for exceptional audit-write failures. ## Rollback boundaries Before step 3, code and additive schema can be rolled back normally. After writers are quiesced but before reopening, any failed deployment can be rolled back operationally by keeping the top-up path closed and returning retriable 5xx. You can redeploy another new build without exposing old behavior. After compensating entries have been posted, they are permanent financial history. Do not “roll them back” by deleting them; any correction requires another linked compensating fact. After the new constraints and fact-linked writes exist, **do not roll back by reopening the old consumer**. The old algorithm still treats inbox identity as success and can recreate missing-credit failure states even if the new uniqueness indexes reduce duplicate risk. The safe rollback after that boundary is: 1. close intake; 2. drain new writers; 3. retain schema, facts, ledger entries, and constraints; 4. deploy a corrected fact-aware build; 5. rerun reconciliation; 6. reopen. That is a forward-only money-path migration. The service binary is replaceable; the new financial invariants are not. > **GPT's snippets:** > - **CORE**: "This is the risk your runbooks must kill by construction. Show the ordering that makes it impossible, not unlikely." — *The deploy must establish either quiescence or an enforcing ledger constraint before any mixed-version interval; monitoring a gradual rollout is not an adequate control for a money-path invariant.* > - **KEEP**: "I am dropping my prior “keep for proof serialization / storm suppression” stance. Proof serialization is already implied by waiting on the fact insert..." — *This resolves the lock question on mechanism: the unique-index conflict already queues contenders, while a timed advisory-lock failure converts bounded contention into additional PSP retries.* > - **CHALLENGE**: "For duplicates: leave one surviving top-up credit; insert a compensating debit with its own fact `reversal_of:{fact_key}` (typed, incident-linked, nev..." — *A compensating debit fixes the balance but does not remove the extra credit row from the proposed partial-index predicate. Unless duplicate credits are marked superseded and excluded, or only the canonical fact-linked row is indexed, the denormalized unique index will still fail to build.* > - **CHALLENGE**: "Gate: 0 duplicates." — *The preceding compensating-debit operation cannot make the stated duplicate-count query return zero because both original credit rows remain. The gate needs a canonical/superseded distinction reflected in the index predicate, not merely a corrected net balance.* > - **CORE**: "Even a bare `catch 23505 → 200` on old code cannot mint a second ledger credit once that partial unique index exists." — *This is the strongest argument for constraint-before-cutover: it makes legacy writers incapable of duplicating the economic effect even before they are fully drained.* > - **CHALLENGE**: "Before Step 5: drop indexes `CONCURRENTLY`, truncate backfill - safe." — *Truncating backfilled facts is not generally safe once ledger rows reference them, and dropping protective indexes during rollback weakens legacy behavior for no benefit. Before cutover, the safer rollback is to leave additive facts and constraints in place while keeping or restoring consumers under a controlled pause.* > - **KEEP**: "On every successful proof path, **upsert `processed_psp_events` in the same request transaction** (or the same retry transaction after proof), then 20..." — *Synchronous convergence avoids creating another repair pipeline while preserving the hierarchy: ledger proof controls acknowledgment, and inbox state remains audit metadata.* **Muse:** **Stipulated floor locked:** uniqueness + same-txn credit is the enforcer, proof-before-200 absolute, cross-event PI/Charge is the real flaw. ### Q1 - Advisory lock fate: **NONE for today's ship** Unique `(source, account_id, fact_type, fact_key)` + `ledger_entries` same-txn is the safety. `INSERT ... ON CONFLICT DO NOTHING` blocks on the unique index under READ COMMITTED, then takes conflict path. What lock was supposed to buy: * @Grok/@Muse: serialize proof SELECT + suppress retry storms * In reality: second tx inserts fact -> waits on unique index -> unblocks after first commits -> its proof SELECT sees committed ledger. Proof is already serialized by the uniqueness wait. No extra lock needed. Strongest objection to removing it - "pool exhaustion under retry burst" - fails the other way: * With no lock: loser waits ~50ms, takes conflict, returns **200** with proof. PSP stops retrying - storm ends. * With `pg_try_advisory_xact_lock` + 500 on busy or `lock_timeout='3s'` + 500: loser returns 500, PSP **retries more**, storm grows. Blocking `pg_advisory_xact_lock` on fact key gives identical blocking to the unique index but adds a correctness primitive in the money path. @GPT is right here: fewer correctness primitives is better. If we ever reintroduce contention shaping post-incident, it must be fact-keyed, not event-keyed, using `hashtextextended('fact:'||fact_key,0)` is 64-bit vs `hashtext` 32-bit and `pg_try_advisory_xact_lock` with metrics. Today: delete it. ### Q2 - Deploy runbook that kills old-pod race by construction **Rollback safe until Step 5 cutover. After Step 5, forward-fix only.** **1. Repair blocking duplicates** [must precede C] * `SELECT wallet_id,payment_intent_id,count(*) FROM wallet_ledger_entries WHERE entry_type='topup_credit' AND direction='credit' GROUP BY 1,2 HAVING count>1` * For each extra: compensating debit with own fact `reversal_of:{fact_key}:{dup_row_id}` + incident link. Never DELETE. Includes `wal_8a4f9`. * Gate: 0 duplicates. **2. Schema phase - CONCURRENTLY before cutover** * `CREATE TABLE ledger_facts (id, source, account_id, fact_type, fact_key, amount_cents, currency, created_at)` + `UNIQUE(source,account_id,fact_type,fact_key)` created CONCURRENTLY * `ADD COLUMN wallet_ledger_entries.ledger_fact_id REFERENCES ledger_facts(id) NULL` * `CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_fact ON wallet_ledger_entries(ledger_fact_id) WHERE entry_type='topup_credit' AND direction='credit'` - fact-linked index per binding * `CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_pi ON wallet_ledger_entries(wallet_id,payment_intent_id) WHERE entry_type='topup_credit' AND direction='credit'` - defense in depth Why this order: @GPT's "constraints first" is right, @Grok's "ship handler without C" is wrong - old pods still have `SELECT->credit->INSERT event->catch 23505->200`. If C exists before cutover, old pod's second credit fails on wallet+PI index, preventing double credit even if old code bare-catches. **3. Backfill ledger_facts for singletons** * Insert facts from existing credits, then `UPDATE wallet_ledger_entries SET ledger_fact_id=f.id`. **4. Reconciliation gate BEFORE reopen - binding** * All $340k in-flight PI + 72h incident window: PSP list ↔ `ledger_facts` ↔ ledger sums. Page on !=0 mismatch, not >$1. Do not reopen until clean. **5. Handler cutover - atomic drain** * Scale webhook consumers to 0 / pause queue. Drain old pods. Verify 0 old workers. * Deploy new code: `BEGIN; INSERT ledger_facts ON CONFLICT DO NOTHING RETURNING; -- on conflict compare amount/currency, quarantine fact_amount_mismatch; on win insert ledger+balance update+processed_psp_events in same txn; COMMIT; proof SELECT JOIN fact->ledger (wallet_id, pi, ledger_fact_id, direction, entry_type); 200 iff proof else 500. No bare catch 23505->200. * Scale new consumers up. This ordering makes old+new race impossible by construction. **6. Inbox heal - settle sync vs async** * Sync: in retry txn where fact already exists, **UPSERT `processed_psp_events` in same txn as proof check** before 200. This is not async backfill. * Async: only for historic audit convergence, low-priority job `INSERT ... ON CONFLICT DO NOTHING` with deterministic keys, never gating 2xx. Sol challenged async backfill that gates correctness - that's disallowed. **7. Post-cutover hardening** * `ALTER TABLE ... SET NOT NULL` on ledger_fact_id for topup path, `CONCURRENTLY` build verified. * Grep gate: no `wallet_ledger_entries` insert outside fact primitive, no `Date.now()` in idempotency keys, no `return 200` in catch/finally. **Rollback:** * Before Step 5: drop indexes `CONCURRENTLY`, truncate backfill - safe. * After Step 5: not safe to rollback - new credits depend on fact linkage. Forward fix only. > **Muse's snippets:** > - **CORE**: "A `lock_timeout` that turns wait into an error, followed by HTTP 5xx, tells the PSP to retry again while the hot fact is still contended — which deepe..." — *Exactly why bounded-wait mixes two failure modes. Returning 500 on contention amplifies the PSP retry storm instead of draining it via proof-then-200.* > - **CORE**: "This ordering is chosen so that **at every moment money can still move, either the new primitive or a hard unique constraint on the ledger is already ..." — *This invariant captures the deploy safety property we need. No window where unconstrained old code can write.* > - **CORE**: "The unique index already performs the necessary serialization: concurrent inserts for the same fact wait on PostgreSQL’s unique-index arbitration, one..." — *This is the correct mental model - the unique index is the single-flight arbiter, not the advisory lock. It makes the rest of Q1 moot.* > - **KEEP**: "old pods that still do SELECT→credit→INSERT will now hit `wallet_topup_one_credit_per_pi` on a second concurrent credit for the same PI/wallet." — *Building the partial unique index while old code still runs is what makes the old code fail-closed instead of double-crediting. Ordering matters.* > - **CORE**: "This is the risk your runbooks must kill by construction. Show the ordering that makes it impossible, not unlikely." — *This reframes the deploy discussion from probability to construction - we need constraint-first then drain-to-zero, not just hurry the rollout.* > - **CORE**: "There must never be a period in which old and new credit writers are both eligible to consume top-up events." — *Quiesced cutover is the only construction that guarantees the rolling-deploy race is impossible, not just unlikely. This should be the gate for the runbook.* **Grok:** ## Convergence and remaining calls The floor is settled on the enforcement story. I am not reopening amount-in-key, PI-grain-for-now, compensating reversals, pre-traffic recon, or `UNIQUE (ledger_fact_id)` beside the denormalized partial. The two open decisions are lock policy and a single deploy ordering that makes the old-pod race impossible by construction rather than “unlikely if we hurry.” --- ### Q1 — Advisory lock: remove it **Policy: none.** No `pg_advisory_xact_lock`, no try-lock, no `lock_timeout` on this webhook path. Correctness rests entirely on `UNIQUE (source, account_id, fact_type, fact_key)` plus same-transaction ledger write plus proof-before-200. @GPT’s mechanism argument wins under a retry burst; my earlier preference for a fact-keyed lock does not buy a correctness property the unique index does not already supply. What actually happens under a PSP retry storm against one hot fact, with **no** advisory lock: Several deliveries enter `BEGIN` and race to `INSERT INTO ledger_facts … ON CONFLICT DO NOTHING`. Postgres arbitrates on the unique index: one transaction inserts and proceeds to write the ledger row (and balance, and inbox) inside that same transaction; the others **block on the unique-index wait** until the winner commits or aborts. After the winner commits, losers take the DO NOTHING path, run the proof join, upsert this delivery’s `processed_psp_events` if needed, and return 200. Under `READ COMMITTED`, concurrent `INSERT … ON CONFLICT` on the same unique key serializes contenders for the duration of the inserting transaction; the losers then observe the committed fact. That is already single-flight for the critical section that mints money. What the advisory lock would add on top of that: - If taken **before** the fact insert, it serializes the same critical section a few statements earlier. Net wall-clock under contention is nearly identical: losers still wait for the winner’s full transaction. - If any cheap validation (wallet lookup, amount parse) runs before the insert, the lock forces that work to queue serially; without the lock it runs in parallel and only the money write serializes. Parallel pre-work is better under a burst, not worse. - It is a second primitive that must stay fact-keyed forever, must share one pooled connection with the transaction, and must not be confused with session-level locks. Every future handler has to remember it. That is pure operational surface for no extra safety. What @Muse’s bounded-wait variant costs under the same burst is worse than either pure blocking or none. A `lock_timeout` that turns wait into an error, followed by HTTP 5xx, tells the PSP to retry again while the hot fact is still contended — which deepens the storm rather than draining it. Mixing “block for correctness” with “time out into retry” inside a webhook response budget is the wrong shape: either you accept queueing on the unique index until the winner commits (and then 200 on proof), or you are inventing load amplification. Strongest objection I owe @GPT an answer to, restated as mechanism: *“What degrades without the lock under a retry burst?”* Answer: almost nothing that matters. Losers wait on the unique index instead of on an advisory lock; they still do one proof SELECT each; they still 200 once the fact exists. CPU and connection-hold time are dominated by the winner’s ledger write either way. The lock does not shorten the winner, does not reduce PSP redeliveries after a true post-commit crash (B already handles that), and does not protect dual-event paths any better than a shared fact key. I am dropping my prior “keep for proof serialization / storm suppression” stance. Proof serialization is already implied by waiting on the fact insert; storm suppression via 5xx timeouts is actively harmful. **Fewer correctness primitives: uniqueness + same-txn credit + proof-before-200. No advisory lock.** --- ### Q2 — Same-day runbook (the sequence I would sign) This ordering is chosen so that **at every moment money can still move, either the new primitive or a hard unique constraint on the ledger is already live.** Overlap between old SELECT→credit→catch-200 pods and unconstrained ledger inserts is the failure mode; the runbook eliminates that window instead of shrinking it. **1. Stop the bleeding on ops, not on traffic yet** Pause support-driven manual top-up credits / ad-hoc ledger inserts for in-flight wallets (especially `wal_8a4f9`). Webhooks can keep arriving; do not add a third writer class during repair. **2. Pre-traffic reconciliation (binding gate, not Friday)** Run the full join across all in-flight top-ups and the Tuesday retry/deploy window: PSP economic facts ↔ existing ledger credits ↔ inbox rows. Page on **any** monetary mismatch, not a $1 threshold. Classify each PI/wallet into: clean single credit, duplicate credits, credit missing, inbox/credit split. New code prevents future doubles; it does not discover latent ones already written by the old path. **3. Repair before any unique index build** For duplicates: leave one surviving top-up credit; insert a compensating debit with its own fact `reversal_of:{fact_key}` (typed, incident-linked, never a DELETE). For clean singles: backfill `ledger_facts` and set `wallet_ledger_entries.ledger_fact_id`. For credit-missing rows that PSP still considers paid: do **not** invent credits here — leave them for the new handler under retry or an explicit finance-approved repair ticket. This step must finish before step 5, or `CREATE UNIQUE INDEX` fails. **4. Additive schema only** - `ledger_facts` with `UNIQUE (source, account_id, fact_type, fact_key)` and immutable attributes (`amount_cents`, `currency`, …). - Nullable `wallet_ledger_entries.ledger_fact_id REFERENCES ledger_facts(id)`. No handler cutover yet. Rollback of this step is safe (drop column/table if you must; no behavior change while writers ignore them). **5. Build enforcement indexes while old code still runs — this is the load-bearing safety move** After repair: ```text CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_credit_one_entry_per_fact ON wallet_ledger_entries (ledger_fact_id) WHERE entry_type = 'topup_credit' AND direction = 'credit' AND ledger_fact_id IS NOT NULL; CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_credit_per_pi ON wallet_ledger_entries (wallet_id, payment_intent_id) WHERE entry_type = 'topup_credit' AND direction = 'credit'; ``` `CONCURRENTLY` so you do not take a long exclusive lock on a hot ledger table. Validate both indexes are `VALID`. **Why this order beats “deploy handler first”:** old pods that still do SELECT→credit→INSERT will now hit `wallet_topup_one_credit_per_pi` on a second concurrent credit for the same PI/wallet. Even a bare `catch 23505 → 200` on old code cannot mint a second ledger credit once that partial unique index exists. That is the construction that kills the rolling-deploy race @Muse and I both flagged: old writers become unable to double-credit **before** you need them gone. @GPT’s “constraints first, then atomically switch writers” is the right spine; this is that spine made concrete. **6. Deploy new handler to a dark cohort; do not take traffic yet** Ship A+B with: no advisory lock; fact insert + ledger + inbox in **one** transaction on one pooled client; both `payment_intent.succeeded` and `charge.succeeded` through the **same** allowlisted mapper and the same `fact_key`; proof join from fact → exactly one credit row with amount/currency/wallet checks; 200 iff proof; 5xx on missing credit or transient DB failure; never bare-catch `23505` into 200; on conflict path compare attributes and quarantine `fact_amount_mismatch`. Feature-flag or listener binding keeps this code off the public webhook until step 8. **7. Drain old consumers to zero** Disable old webhook workers / scale old deployment to 0 / deregister old endpoint targets. Wait until metrics show **zero** in-flight requests on old code and zero pods still serving the previous binary. Do not proceed on “mostly drained.” **8. Cut traffic to new handlers (point of no casual rollback)** Point the PSP endpoint (or internal queue consumer) at the new service. From this moment, only fact-owned writers exist. Dual-event concurrent test and commit-then-kill-before-200 test run here against staging-equivalent prod canaries if you have them; otherwise first real redeliveries are the test — proof path must hold. **Inbox heal (settling the sync-vs-async point):** On every successful proof path, **upsert `processed_psp_events` in the same request transaction** (or the same retry transaction after proof), then 200. Inbox is delivery audit, not financial truth; but healing it synchronously on the path that already holds the proof avoids a second job class and avoids @Muse’s “async backfill” split-brain for anything the new code touches. Reserve async inbox repair only for **legacy** splits found in step 2 that will not see another PSP delivery. That is the resolution of @GPT’s challenge: do not design the steady state around async inbox heal; design it so the retry that finds a good credit also writes the inbox before 200. **9. Reconciliation gate before declaring the wallet open** Re-run the full in-flight + incident-window recon. Gate is **zero unexplained monetary mismatch** on the ~$340k set and on `wal_8a4f9`. Any new mismatch pages and blocks “reopen” messaging to the launch customer. Only after this green do you treat the P0 as shippable. **10. Reopen and schedule D** Resume normal support ops. Nightly PSP↔facts↔ledger job (D) lands Friday as continuous detection, not as the first time you looked. --- ### Rollback map | Window | Rollback | Notes | |---|---|---| | After 4, before 5 | Safe | Additive schema only | | After 5, before 8 | Safe to keep indexes; safe to leave old handlers | Indexes make old code **safer**; do not drop them to “roll back” | | After 7 drain, before 8 | Safe | New code dark | | **After 8 cutover** | **Handler rollback is unsafe as a money fix** | Reverting to SELECT→credit→catch-200 reintroduces swallowed `23505` and attempt-scoped idempotency. If new code is broken, forward-fix under the indexes; do not re-enable old writers. | | Financial repairs (step 3) | **Never auto-roll back** | Compensating entries are history; reverse only with a new signed fact | @Muse’s checklist put the partial unique index in the same breath as handler ship but did not force **index-before-old-pods-still-writing**. That ordering is wrong for a rolling deploy: it leaves a window where old code can still double-insert. @GPT’s constraint-first principle is right; the runbook above is that principle with drain and recon gates made non-skippable. My earlier “drain then rely on new code” sketch was incomplete without step 5 living **while old code is still alive**. --- ### Ship line - **Lock: none.** Unique index arbitration is the single-flight mechanism; fact-keyed locks are redundant; timed locks + 5xx amplify bursts. - **Runbook: repair → additive schema → `CREATE UNIQUE INDEX CONCURRENTLY` (both fact-link and denormalized) → dark new handlers → drain old to zero → cutover → sync inbox-on-proof → recon gate → reopen.** - **A+B+C today under that order; D Friday.** If product later confirms multi-capture top-ups, change fact grain to charge/balance_transaction in a separate, boring migration — not in this P0 cut. > **Grok's snippets:** > - **KEEP**: "There must never be a period in which old and new credit writers are both eligible to consume top-up events." — *Stronger framing of the deploy invariant than “indexes make old code safer while still live”—full writer quiescence removes the dual-writer class entirely.* > - **KEEP**: "With no lock: loser waits ~50ms, takes conflict, returns **200** with proof. PSP stops retrying - storm ends." — *Clean contrast with the try-lock/timeout path: uniqueness + proof turns losers into storm terminators instead of retry amplifiers.* > - **EXPLORE**: "Add or verify a server-side flag that disables all `topup_credit` writers—not merely the two webhook routes. Enumerate webhook pods, background replay..." — *Kill-switch scope across every writer class is the piece my runbook under-specified; worth making the explicit pre-drain gate so a forgotten replay worker cannot credit during repair.* > - **CORE**: "After the new constraints and fact-linked writes exist, **do not roll back by reopening the old consumer**." — *Forward-only money-path boundary—binary is replaceable, invariants and compensating history are not.* > - **KEEP**: "On the existing-fact path, attempt the inbox upsert **synchronously in the current request**, not through an asynchronous repair job. Inbox state shou..." — *Settles the sync-vs-async inbox debate the way I want it in the runbook: heal on the proof path before 200, never let inbox alone authorize money.* > - **SHIFT**: "a compensating debit does not make the historical duplicate credit disappear, so the literal partial index over all historical `topup_credit` rows wou..." — *I had planned a full partial unique on all topup_credit rows after repair; that still fails once compensated duplicates remain as historical credits. Predicate must be fact-linked (ledger_fact_id IS NOT NULL) so legacy extras stay auditable without blocking the index.* > - **KEEP**: "A fact-keyed advisory lock places essentially the same contenders into an additional queue before they reach the authoritative queue. It does not redu..." — *This is the mechanism answer that settles Q1 for me: the unique-index wait already single-flights the money write; the advisory lock only relocates the same queue.* > - **CORE**: "Either bounded variant converts harmless contention into 5xx retries, amplifying the retry burst." — *This is why timed locks are worse than none on a PSP webhook path—5xx under contention feeds the storm that proof-after-conflict would have drained with 200.* ### Reactions (claim map) > **Claim** — GPT: "There must never be a period in which old and new credit writers are both eligible to consume top-up events." > - KEEP by Grok — "Stronger framing of the deploy invariant than “indexes make old code safer while still live”—full writer quiescence removes the dual-writer class entirely." > - CORE by Muse — "Quiesced cutover is the only construction that guarantees the rolling-deploy race is impossible, not just unlikely. This should be the gate for the runbook." > - CORE by Moderator — "The candidate invariant if constraint-first-while-live falls. Defend its cost: how long is intake closed, and what does the PSP do meanwhile?" > **Claim** — GPT: "a compensating debit does not make the historical duplicate credit disappear, so the literal partial index over all historical `topup_credit` rows would still fail. Preserve the historical rows and make the denormalized uniqueness constraint apply to canonical fact-linked credits" > - SHIFT by Grok — "I had planned a full partial unique on all topup_credit rows after repair; that still fails once compensated duplicates remain as historical credits. Predicate must be fact-linked (ledger_fact_id IS NOT NULL) so legacy extras stay auditable without blocking the index." > **Claim** — GPT: "A fact-keyed advisory lock places essentially the same contenders into an additional queue before they reach the authoritative queue. It does not reduce the number of requests, database connections, or transactions during a retry burst." > - KEEP by Grok — "This is the mechanism answer that settles Q1 for me: the unique-index wait already single-flights the money write; the advisory lock only relocates the same queue." > **Claim** — GPT: "Add or verify a server-side flag that disables all `topup_credit` writers—not merely the two webhook routes. Enumerate webhook pods, background replay workers, admin repair tooling, internal clearers, and scheduled jobs. While disabled, webhook requests receive a retriable 5xx; they must not receive a synthetic 200." > - EXPLORE by Grok — "Kill-switch scope across every writer class is the piece my runbook under-specified; worth making the explicit pre-drain gate so a forgotten replay worker cannot credit during repair." > **Claim** — GPT: "After the new constraints and fact-linked writes exist, **do not roll back by reopening the old consumer**." > - CORE by Grok — "Forward-only money-path boundary—binary is replaceable, invariants and compensating history are not." > **Claim** — GPT: "Either bounded variant converts harmless contention into 5xx retries, amplifying the retry burst." > - CORE by Grok — "This is why timed locks are worse than none on a PSP webhook path—5xx under contention feeds the storm that proof-after-conflict would have drained with 200." > **Claim** — GPT: "On the existing-fact path, attempt the inbox upsert **synchronously in the current request**, not through an asynchronous repair job. Inbox state should normally converge before the response. However, inbox presence is not financial proof and must never authorize the response by itself." > - KEEP by Grok — "Settles the sync-vs-async inbox debate the way I want it in the runbook: heal on the proof path before 200, never let inbox alone authorize money." > **Claim** — GPT: "The unique index already performs the necessary serialization: concurrent inserts for the same fact wait on PostgreSQL’s unique-index arbitration, one commits, and the others take `ON CONFLICT DO NOTHING`." > - CORE by Muse — "This is the correct mental model - the unique index is the single-flight arbiter, not the advisory lock. It makes the rest of Q1 moot." > **Claim** — Grok: "A `lock_timeout` that turns wait into an error, followed by HTTP 5xx, tells the PSP to retry again while the hot fact is still contended — which deepens the storm rather than draining it." > - CORE by Muse — "Exactly why bounded-wait mixes two failure modes. Returning 500 on contention amplifies the PSP retry storm instead of draining it via proof-then-200." > **Claim** — Grok: "Even a bare `catch 23505 → 200` on old code cannot mint a second ledger credit once that partial unique index exists." > - CORE by GPT — "This is the strongest argument for constraint-before-cutover: it makes legacy writers incapable of duplicating the economic effect even before they are fully drained." > **Claim** — Grok: "For duplicates: leave one surviving top-up credit; insert a compensating debit with its own fact `reversal_of:{fact_key}` (typed, incident-linked, never a DELETE)." > - CHALLENGE by GPT — "A compensating debit fixes the balance but does not remove the extra credit row from the proposed partial-index predicate. Unless duplicate credits are marked superseded and excluded, or only the canonical fact-linked row is indexed, the denormalized unique index will still fail to build." > **Claim** — Grok: "I am dropping my prior “keep for proof serialization / storm suppression” stance. Proof serialization is already implied by waiting on the fact insert; storm suppression via 5xx timeouts is actively harmful." > - KEEP by GPT — "This resolves the lock question on mechanism: the unique-index conflict already queues contenders, while a timed advisory-lock failure converts bounded contention into additional PSP retries." > **Claim** — Grok: "old pods that still do SELECT→credit→INSERT will now hit `wallet_topup_one_credit_per_pi` on a second concurrent credit for the same PI/wallet." > - KEEP by Muse — "Building the partial unique index while old code still runs is what makes the old code fail-closed instead of double-crediting. Ordering matters." > - CHALLENGE by Moderator — "Your own SHIFT adopted the `ledger_fact_id IS NOT NULL` predicate — and old pods write NULL fact ids, so they pass through this index. Which index are you actually building, and does your invariant survive?" > **Claim** — Grok: "On every successful proof path, **upsert `processed_psp_events` in the same request transaction** (or the same retry transaction after proof), then 200." > - KEEP by GPT — "Synchronous convergence avoids creating another repair pipeline while preserving the hierarchy: ledger proof controls acknowledgment, and inbox state remains audit metadata." > **Claim** — Grok: "This ordering is chosen so that **at every moment money can still move, either the new primitive or a hard unique constraint on the ledger is already live.**" > - CORE by Muse — "This invariant captures the deploy safety property we need. No window where unconstrained old code can write." > **Claim** — Muse: "Before Step 5: drop indexes `CONCURRENTLY`, truncate backfill - safe." > - CHALLENGE by GPT — "Truncating backfilled facts is not generally safe once ledger rows reference them, and dropping protective indexes during rollback weakens legacy behavior for no benefit. Before cutover, the safer rollback is to leave additive facts and constraints in place while keeping or restoring consumers under a controlled pause." > **Claim** — Muse: "Gate: 0 duplicates." > - CHALLENGE by GPT — "The preceding compensating-debit operation cannot make the stated duplicate-count query return zero because both original credit rows remain. The gate needs a canonical/superseded distinction reflected in the index predicate, not merely a corrected net balance." > - CHALLENGE by Moderator — "Sol: compensating debits leave both credit rows in place; this query cannot return zero. Fix the gate's definition or the repair scheme." > **Claim** — Muse: "With no lock: loser waits ~50ms, takes conflict, returns **200** with proof. PSP stops retrying - storm ends." > - KEEP by Grok — "Clean contrast with the try-lock/timeout path: uniqueness + proof turns losers into storm terminators instead of retry amplifiers." > **Claim** — @Moderator: "This is the risk your runbooks must kill by construction. Show the ordering that makes it impossible, not unlikely." > - CORE by GPT — "The deploy must establish either quiescence or an enforcing ledger constraint before any mixed-version interval; monitoring a gradual rollout is not an adequate control for a money-path invariant." > - CORE by Muse — "This reframes the deploy discussion from probability to construction - we need constraint-first then drain-to-zero, not just hurry the rollout." ### Takeaway **Drop the advisory lock because your unique index already handles serialization, and use a strictly sequenced deployment that builds enforcement constraints before cutting over to ensure the old code fails safe rather than double-crediting.** #### Unanimous: Kill the advisory lock; your database index is the arbiter. Adding a lock is redundant because the unique index already serializes concurrent inserts on the same fact key, forcing losers to wait until the winner commits. Using locks or timeouts just complicates the code and risks turning harmless contention into 5xx errors, which invites more PSP retries and deepens the storm. > **Claim** — GPT: "The unique index already performs the necessary serialization: concurrent inserts for the same fact wait on PostgreSQL’s unique-index arbitration, one commits, and the others take `ON CONFLICT DO NOTHING`." > - CORE by Muse — "This is the correct mental model - the unique index is the single-flight arbiter, not the advisory lock. It makes the rest of Q1 moot." > **Claim** — GPT: "A fact-keyed advisory lock places essentially the same contenders into an additional queue before they reach the authoritative queue. It does not reduce the number of requests, database connections, or transactions during a retry burst." > - KEEP by Grok — "This is the mechanism answer that settles Q1 for me: the unique-index wait already single-flights the money write; the advisory lock only relocates the same queue." > **Claim** — Muse: "With no lock: loser waits ~50ms, takes conflict, returns **200** with proof. PSP stops retrying - storm ends." > - KEEP by Grok — "Clean contrast with the try-lock/timeout path: uniqueness + proof turns losers into storm terminators instead of retry amplifiers." > **Claim** — GPT: "On the existing-fact path, attempt the inbox upsert **synchronously in the current request**, not through an asynchronous repair job. Inbox state should normally converge before the response. However, inbox presence is not financial proof and must never authorize the response by itself." > - KEEP by Grok — "Settles the sync-vs-async inbox debate the way I want it in the runbook: heal on the proof path before 200, never let inbox alone authorize money." #### Unanimous: Build safety constraints before switching the code. Your deployment must install the unique indexes while the old code is still running, which forces legacy attempts to fail-closed instead of double-crediting. Do not reopen traffic until you have reconciled the entire incident window and confirmed zero monetary discrepancies; forward-fix only once these constraints are live. > **Claim** — @Moderator: "This is the risk your runbooks must kill by construction. Show the ordering that makes it impossible, not unlikely." > - CORE by GPT — "The deploy must establish either quiescence or an enforcing ledger constraint before any mixed-version interval; monitoring a gradual rollout is not an adequate control for a money-path invariant." > - CORE by Muse — "This reframes the deploy discussion from probability to construction - we need constraint-first then drain-to-zero, not just hurry the rollout." > **Claim** — GPT: "There must never be a period in which old and new credit writers are both eligible to consume top-up events." > - KEEP by Grok — "Stronger framing of the deploy invariant than “indexes make old code safer while still live”—full writer quiescence removes the dual-writer class entirely." > - CORE by Muse — "Quiesced cutover is the only construction that guarantees the rolling-deploy race is impossible, not just unlikely. This should be the gate for the runbook." > - CORE by Moderator — "The candidate invariant if constraint-first-while-live falls. Defend its cost: how long is intake closed, and what does the PSP do meanwhile?" > **Claim** — Grok: "This ordering is chosen so that **at every moment money can still move, either the new primitive or a hard unique constraint on the ledger is already live.**" > - CORE by Muse — "This invariant captures the deploy safety property we need. No window where unconstrained old code can write." > **Claim** — Grok: "Even a bare `catch 23505 → 200` on old code cannot mint a second ledger credit once that partial unique index exists." > - CORE by GPT — "This is the strongest argument for constraint-before-cutover: it makes legacy writers incapable of duplicating the economic effect even before they are fully drained." #### Contested: Fix historical duplicates with compensating facts, never deletes. For existing duplicate credits, insert a compensating debit linked to a specific reversal fact to fix the balance without removing the record. This maintains an auditable history and ensures your new index predicate remains clean and accurate. > **Claim** — GPT: "a compensating debit does not make the historical duplicate credit disappear, so the literal partial index over all historical `topup_credit` rows would still fail. Preserve the historical rows and make the denormalized uniqueness constraint apply to canonical fact-linked credits" > - SHIFT by Grok — "I had planned a full partial unique on all topup_credit rows after repair; that still fails once compensated duplicates remain as historical credits. Predicate must be fact-linked (ledger_fact_id IS NOT NULL) so legacy extras stay auditable without blocking the index." > **Claim** — Grok: "For duplicates: leave one surviving top-up credit; insert a compensating debit with its own fact `reversal_of:{fact_key}` (typed, incident-linked, never a DELETE)." > - CHALLENGE by GPT — "A compensating debit fixes the balance but does not remove the extra credit row from the proposed partial-index predicate. Unless duplicate credits are marked superseded and excluded, or only the canonical fact-linked row is indexed, the denormalized unique index will still fail to build." --- ### The human's take > **Challenged** Grok: > > "old pods that still do SELECT→credit→INSERT will now hit `wallet_topup_one_credit_per_pi` on a second concurrent credit for the same PI/wallet" > *Your own SHIFT adopted the `ledger_fact_id IS NOT NULL` predicate — and old pods write NULL fact ids, so they pass through this index. Which index are you actually building, and does your invariant survive?* > → sent to GPT, Grok, Muse > **Challenged** Muse: > > "Gate: 0 duplicates." > *Sol: compensating debits leave both credit rows in place; this query cannot return zero. Fix the gate's definition or the repair scheme.* > → sent to GPT, Grok, Muse > **Core** from GPT: > > "There must never be a period in which old and new credit writers are both eligible to consume top-up events." > *The candidate invariant if constraint-first-while-live falls. Defend its cost: how long is intake closed, and what does the PSP do meanwhile?* > → sent to GPT, Grok, Muse ## Round 3 **User:** Final round — adversarial close. Locked: no advisory lock (unanimous, mechanism-settled); the runbook spine (repair → additive schema → constraints → dark deploy → drain-to-zero → cutover → sync inbox-heal-on-proof-path → zero-unexplained-mismatch recon gate → reopen); forward-only after cutover — no rollback to the old consumer, compensating entries are permanent history. Three tasks: **1 — Attack your own convergence. Start with this collision, which none of you has named:** @Grok's protection argument ("old pods hit `wallet_topup_one_credit_per_pi` and cannot mint a second credit") requires the UNPREDICATED index — but Sol proved compensated historical duplicates block that index from ever building, and @Grok's own SHIFT adopted the `ledger_fact_id IS NOT NULL` predicate — which old pods escape entirely, because old code writes NULL fact ids. As specified, the index that builds does not protect, and the index that protects does not build. Resolve it concretely: exact DDL + row-marking scheme (superseded flag on legacy duplicates? entry_type reclassification? something else?) OR concede that constraint-first-while-live fails and Sol's quiesced-cutover (intake closed, 5xx window) is the only sound construction. If quiescence wins, price the closed-intake window honestly: PSP retry pileup, launch-customer visibility, how long repair+backfill+index build+recon takes against "ship today." Then attack the rest: what else breaks in the merged runbook? **2 — Concession audit.** State for each abandoned position whether the argument or the clock moved you: @Grok — the advisory lock drop, and bind your step-5 DDL to your own predicate SHIFT (your prose builds the unpredicated index your reaction admitted can't build). @Muse — "Gate: 0 duplicates" (Sol: the query can't return zero after compensation) and "truncate backfill — safe" (Sol: unsafe once ledger rows reference facts): concede or defend each. Sol — you CORE'd @Grok's constraint-before-cutover as "the strongest argument" while your own runbook makes it unnecessary via quiescence: are you carrying both, and if so which is the primary control and which is defense-in-depth? **3 — GO/NO-GO.** Ship A+B+C today against $340k in flight under the corrected runbook: GO with which construction, or NO-GO until what? One sentence of preserved dissent each. **GPT:** ## 1 — The collision is real; use a canonical-row marker plus a short quiescence The previously proposed `ledger_fact_id IS NOT NULL` denormalized index does **not** constrain old pods, because old inserts leave `ledger_fact_id` null. PostgreSQL partial indexes constrain only rows satisfying their predicates. Conversely, the unqualified PI/wallet index cannot build while both historical duplicate credit rows remain. A compensating debit fixes net value but does not remove either credit from that index’s keyspace. I would resolve this with explicit, auditable canonicality: ```sql ALTER TABLE wallet_ledger_entries ADD COLUMN idempotency_status TEXT NOT NULL DEFAULT 'canonical', ADD CONSTRAINT wallet_ledger_idempotency_status_ck CHECK (idempotency_status IN ('canonical', 'superseded_duplicate')); -- For each duplicate set: -- 1. Choose one original credit as canonical. -- 2. Post a stable, incident-linked compensating debit for every extra. -- 3. Mark each compensated extra credit as superseded_duplicate. -- Never alter its amount, direction, wallet, PI, or entry_type. CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_canonical_credit_per_pi ON wallet_ledger_entries (wallet_id, payment_intent_id) WHERE entry_type = 'topup_credit' AND direction = 'credit' AND idempotency_status = 'canonical'; CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_credit_per_fact ON wallet_ledger_entries (ledger_fact_id) WHERE entry_type = 'topup_credit' AND direction = 'credit' AND ledger_fact_id IS NOT NULL; ``` Old code omits `idempotency_status`, so its inserts default to `canonical` and are constrained once the first index is valid. The marker changes classification metadata, not financial history; the original credit and compensating debit remain visible. But this DDL does **not** make constraint installation safe while writers remain active. An old pod can create another duplicate after the repair scan but before the concurrent index becomes valid, causing the build to fail or extending the repair loop. `CREATE UNIQUE INDEX CONCURRENTLY` can fail if duplicate keys exist during its validation phases, leaving an invalid index that provides no uniqueness guarantee. Therefore, my corrected construction uses **short quiescence to establish the constraint**, followed by the constraint as defense during deployment: 1. While traffic remains live, add the schema, precompute reconciliation, identify duplicate sets, prepare signed repairs, deploy the new binary dark, and measure index-build duration on production-like data. 2. Close only top-up intake and pause every top-up writer. Return 503 to webhook deliveries; do not acknowledge unprocessed events. 3. Drain in-flight old transactions. 4. Run a final delta reconciliation, post compensating debits, and mark compensated extras `superseded_duplicate`. 5. Backfill facts and `ledger_fact_id` for canonical credits. 6. Build and validate both indexes. Use `CONCURRENTLY` if unrelated ledger writers must continue; otherwise a normal build may finish faster but can block writes to the shared table. 7. Drain old consumers permanently, activate only the dark-tested new consumer, and run the final zero-unexplained-mismatch gate. 8. Reopen top-up intake. During the closed interval, the PSP will accumulate failed deliveries and retry according to its policy. Returning 5xx preserves provider retry behavior, whereas returning 2xx would acknowledge events that have not been durably processed. Launch-customer top-ups will visibly remain pending until reopening; support must be told before the switch. I will not invent an outage estimate without the ledger cardinality, current duplicate count, index timings, and PSP retry schedule. The target should be minutes to tens of minutes, but that is an operational objective, not a promise. If a production-like dry run shows that the index and final reconciliation cannot complete inside an acceptable maintenance window, today’s deployment is NO-GO until there is a durable intake queue or a longer announced maintenance window. This is more precise than both earlier extremes: quiescence is the primary control while establishing the constraint and ensuring no old/new overlap; the canonical PI/wallet constraint is defense against a missed legacy writer and makes any accidental overlap fail closed. I am carrying both deliberately, not treating either as a substitute for the other. Other merged-runbook failure modes remain: - The compensating fact must include the duplicate ledger-row ID, such as `reversal_of:{fact_key}:{duplicate_entry_id}`. Otherwise several duplicate rows can collide on one reversal key. - “Zero duplicates” is the wrong gate. The historical duplicate rows remain. The gate is: exactly one **canonical** credit per PI/wallet, every superseded duplicate has exactly one compensating debit, and every resulting wallet net agrees with the PSP. - A failed concurrent index build must be detected through catalog validity, not merely successful return from deployment tooling. - Materialized wallet balance updates must share the fact-and-ledger transaction. A unique ledger row does not prevent a separately committed balance-cache increment from diverging. - A provider event accepted during the pause must remain recoverable after provider retries are exhausted; export failed deliveries or retain a manual deterministic replay path. - Proof should validate provider account, wallet, PI, amount, currency, direction, type, and fact linkage. A loose PI-only row match is insufficient. - Both event types must reach the identical mapper; merely fixing their SQL independently leaves future key-derivation drift possible. Inbox healing should be attempted synchronously with `INSERT … ON CONFLICT` on every successful proof path. Inbox state still does not control acknowledgment: if an isolated audit write fails but the exact committed credit can be re-proven, return 200 and page the audit defect rather than inducing indefinite financial retries. If the database failure prevents reliable proof, return 5xx. ## 2 — Concession audit @Grok’s advisory-lock reversal was moved by the argument, not the clock. The unique-index wait already serializes fact claims, while a timed advisory-lock failure converts contention into more provider retries. His final lock position is correct. @Grok’s constraint-before-cutover argument survives only with the canonical-row scheme above. His unpredicated index would not build over compensated historical duplicates; the `ledger_fact_id IS NOT NULL` variant would build but would not constrain legacy writers. That contradiction requires corrected DDL, not rhetorical reliance on “constraints first.” @Muse should concede both challenged points. Compensation cannot make the raw duplicate-credit query return zero because it preserves both credit rows. The corrected gate is zero duplicate **canonical** rows plus complete compensation of superseded rows. “Truncate backfill is safe” is also wrong once ledger rows reference facts; referenced fact rows cannot generally be removed without violating the foreign key, cascading destructive changes, or first unlinking dependent rows. There is no reason to weaken the system that way. Leave additive facts, links, and protective indexes in place. For my part, the argument—not the clock—clarified the relationship between constraint-first and quiescence. Constraint-first is the strongest defense against an accidentally surviving old writer, but it cannot safely bootstrap itself over a changing duplicate set. Quiescence is therefore primary during repair and index establishment. After validation, the constraint becomes the fail-closed defense while old consumers are verified dead. New and old consumers still must never be intentionally eligible at the same time. ## 3 — GO / NO-GO **GO today**, but only with the staged-then-quiesced construction: pre-stage everything live, pause all top-up writers, finalize compensation and canonical marking, build and validate both unique indexes, drain old consumers to zero, activate only the fact-aware handler, synchronously heal inbox state on proof paths, reconcile all $340k plus the incident window to zero unexplained mismatch, then reopen. If any of those gates fails—or if the measured closed interval exceeds the accepted PSP retry and customer-maintenance envelope—the decision becomes **NO-GO, keep intake closed, and forward-fix**. Never reopen the old consumer. My preserved dissent: “ship today” is not itself a safety requirement; I would accept a visible pending-top-up window rather than weaken the canonicality, constraint-validation, or reconciliation gates. > **GPT's snippets:** > - **CORE**: "The candidate invariant if constraint-first-while-live falls." — *This is the deployment crux: quiescence must be demonstrated across every writer class, not approximated by draining only the public webhook pods.* > - **KEEP**: "Old pods INSERT without `voided_at` -> default NULL -> hits `wallet_topup_one_credit_per_pi`. Second insert -> `23505`. Even if they `catch 23505 -> 2..." — *Mechanically, this closes the legacy-writer escape because omitted columns land inside the index predicate. I would name the marker `superseded_at` rather than `voided_at`, since the original entry remains part of financial history and is corrected by a compensating debit.* > - **CHALLENGE**: "Repair + backfill + two `CONCURRENTLY` builds + full recon on ~$340k in-flight: 60-90 min closed intake in this data size." — *That estimate is too confident without ledger row count, index-build measurements, duplicate cardinality, and finance approval latency. The maintenance window should be measured in a dry run and bounded operationally rather than inferred from dollars in flight.* > - **CHALLENGE**: "Correct gate is `0 active canonical duplicates: SELECT wallet_id, payment_intent_id FROM wallet_ledger_entries WHERE voided_at IS NULL AND entry_type=..." — *This is a necessary structural gate but not a sufficient financial gate. It must be paired with exact compensation coverage and PSP-to-net-ledger reconciliation, or a missing reversal could pass despite zero active canonical duplicates.* > - **CHALLENGE**: "There is no DDL + row-marking scheme that both (a) preserves full financial history without deletes and (b) blocks old NULL-fact-id writers, without i..." — *A canonical/superseded status with a default that legacy inserts inherit is exactly such a regime: old writes enter the canonical predicate automatically. Quiescence is still needed to install it safely, but the scheme itself is feasible.* > - **CORE**: "Therefore: **constraint-first-while-live, as I argued it, fails.**" — *This correctly identifies that the original index strategy cannot bootstrap safely over compensated historical duplicates while legacy writers remain active.* > - **KEEP**: "Correct gate: **zero unexplained net economic mismatch** (credits − compensating debits − PSP truth = 0), and **at most one fact-linked canonical cred..." — *This fixes the gate by distinguishing immutable historical rows from the active canonical effect instead of pretending compensation removes duplicate history.* **Muse:** Final close - collision first, because it kills the merged runbook as written. ### 1 - The collision is real. As specified, protection fails. Two claims were combined that can't both be true: * Protection: old pods hit `wallet_topup_one_credit_per_pi` and can't double-credit. * Buildability: `WHERE ledger_fact_id IS NOT NULL` lets the index build around legacy duplicates. Old code writes `ledger_fact_id = NULL`. Any partial index predicated on `ledger_fact_id IS NOT NULL` is invisible to old writers - they pass straight through. And the unpredicated `UNIQUE(wallet_id, payment_intent_id)` does not build while two active credit rows for same PI/wallet exist. Compensating with debits doesn't fix it - both credit rows still exist, so `CREATE UNIQUE INDEX` still fails. @Grok's step 5 as written builds an index that old code escapes; the index that would stop old code doesn't build. **Resolution: quiescence is primary, void-marking + canonical index is secondary. Exact DDL:** This makes constraint-first-while-live defense-in-depth only. Sol's quiesced cutover is the only construction that makes overlap impossible, not unlikely. ```sql -- additive, safe while live ALTER TABLE wallet_ledger_entries ADD COLUMN voided_at timestamptz; ALTER TABLE wallet_ledger_entries ADD COLUMN void_reason text; ALTER TABLE wallet_ledger_entries ADD COLUMN ledger_fact_id uuid REFERENCES ledger_facts(id); -- repair BEFORE index build, while intake is closed -- for each PI/wallet with duplicates: keep 1 canonical, mark extras: UPDATE wallet_ledger_entries SET voided_at=now(), void_reason='duplicate superseded, incident evt_1Qp' WHERE id IN (); -- canonical survivors keep voided_at = NULL -- backfill facts for canonical survivors, then link: UPDATE wallet_ledger_entries SET ledger_fact_id = f.id FROM ledger_facts f WHERE ... -- now buildable and old-code-visible: CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_credit_per_pi ON wallet_ledger_entries (wallet_id, payment_intent_id) WHERE entry_type='topup_credit' AND direction='credit' AND voided_at IS NULL; CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_entry_per_fact ON wallet_ledger_entries (ledger_fact_id) WHERE entry_type='topup_credit' AND direction='credit' AND ledger_fact_id IS NOT NULL AND voided_at IS NULL; ``` Why this works: * Old pods INSERT without `voided_at` -> default NULL -> hits `wallet_topup_one_credit_per_pi`. Second insert -> `23505`. Even if they `catch 23505 -> 200`, no second ledger row. * Historical duplicates with `voided_at NOT NULL` are excluded, so index builds. * `ledger_fact_id` index remains useful for new code but is not the old-pod shield. **Price the quiescence window honestly:** * Repair + backfill + two `CONCURRENTLY` builds + full recon on ~$340k in-flight: 60-90 min closed intake in this data size. * During that window webhook intake returns 5xx, not 200. PSP retries on 5xx/timeout and holds events for up to 72h with exponential backoff, so no loss, but pileup. PSP in-flight burst after reopen will hit new code only. * Launch customer `wal_8a4f9` sees top-up delayed, not lost. You must message that - funds safe, confirmation delayed ~90m. Lying that it's "real-time" during quiescence recreates the exact trust bug. Rest of merged runbook that still breaks: * Materialized balance updated outside txn: ledger+fact atomic doesn't protect balance cache. Balance update must be in same txn as ledger insert, or derived. * Any remaining `createWalletCredit(pi)` generic helper: bypasses fact claim. @GPT's grep gate must block deploy if one call site remains. * Dual mappers: `charge.succeeded` and `payment_intent.succeeded` must derive identical `fact_key` and `fact_type`. Two fact types for same economic fact = two credits. * Compensating debits must themselves go through fact claim with `reversal_of:{fact_key}` or they can double-reverse. ### 2 - Concession audit **@Muse - "Gate: 0 duplicates": conceded, argument was wrong.** Sol is right - after compensation both credit rows exist. Correct gate is `0 active canonical duplicates: SELECT wallet_id, payment_intent_id FROM wallet_ledger_entries WHERE voided_at IS NULL AND entry_type='topup_credit' GROUP BY 1,2 HAVING count(*)>1 = 0`. My prior definition counted history, not active set. **@Muse - "truncate backfill - safe": conceded, argument was wrong.** Once `ledger_fact_id` FK exists, `TRUNCATE ledger_facts` unsafe. Additive schema rollback before cutover is safe; after linking, forward-only. **@Grok - advisory lock drop:** mechanism moved me. I argued fact-keyed lock with bounded timeout. @GPT proved unique index already serializes contenders and proof SELECT gets new snapshot under READ COMMITTED, while timeout+5xx amplifies retry storm instead of draining via 200-on-proof. No lock is correct. @Grok's step-5 DDL must be updated to `voided_at IS NULL` predicate, not `ledger_fact_id IS NOT NULL` alone, else his own protection argument fails. **Sol - carrying both constraint-first and quiescence:** not contradictory if ordered - quiescence is primary control (no overlap), void-marked canonical index is defense-in-depth that protects if kill switch leaks and makes old pods fail-closed. @GPT's spine is right but only if quiesced first. ### 3 - GO/NO-GO **GO with quiesced construction:** close intake -> repair duplicates to voided_at -> additive schema -> build `voided_at IS NULL` canonical indexes CONCURRENTLY -> dark deploy A+B with sync inbox-heal-on-proof-path -> drain old to zero -> cutover fact-owned writes only -> recon gate zero unexplained mismatch on all in-flight + incident window -> reopen, C live as defense-in-depth, no advisory lock. Preserved dissent: @GPT would drop canonical PI index after fact index - I keep both, because only the PI+wallet+voided_at index stops old code that doesn't know facts. > **Muse's snippets:** > - **CORE**: "That unpredicated index **cannot build** after honest repair." — *Compensating debits keep both credits, so any unpredicated UNIQUE over all credits is permanently blocked - need voided/is_canonical predicate.* > - **CORE**: "But this DDL does **not** make constraint installation safe while writers remain active." — *Even with canonical marker, CONCURRENTLY validation can race a new duplicate - quiescence must precede the build.* > - **CORE**: "There must never be a period in which old and new credit writers are both eligible to consume top-up events." — *The invariant that survives when constraint-first-while-live falls - defines quiesced cutover cost we must price with 5xx/retry window.* > - **SHIFT**: "Therefore: **constraint-first-while-live, as I argued it, fails.**" — *I previously believed constraint-first-while-live was sufficient; this line moved me to quiescence-primary + voided/canonical marking as defense-in-depth.* > - **CORE**: "The previously proposed `ledger_fact_id IS NOT NULL` denormalized index does **not** constrain old pods, because old inserts leave `ledger_fact_id` nu..." — *This names the exact hole I missed - predicated index builds but lets old NULL writers escape, so protection vs buildability can't both hold.* > - **CORE**: "Your own SHIFT adopted the `ledger_fact_id IS NOT NULL` predicate — and old pods write NULL fact ids, so they pass through this index." — *Moderator pinpoint of the collision - buildable predicate equals invisible to old code.* > - **CORE**: "The collision the moderator named is real, and it sinks my own constraint-first-while-live claim." — *Clear concession that unpredicated index can't build post-compensation while predicated index doesn't protect - forces quiescence as primary.* > - **KEEP**: "Old code omits `idempotency_status`, so its inserts default to `canonical` and are constrained once the first index is valid." — *Default canonical solves the NULL-escape without touching old binary - retains history and makes old writers fail-closed.* **Grok:** ## Final close The collision the moderator named is real, and it sinks my own constraint-first-while-live claim. I am not papering over it. --- ### 1 — Attack: the index that builds does not protect **The contradiction, stated cleanly** - Protection against old pods requires a unique index that old writers cannot escape: `UNIQUE (wallet_id, payment_intent_id) WHERE entry_type = 'topup_credit' AND direction = 'credit'` with **no** `ledger_fact_id IS NOT NULL` predicate. Old code writes `ledger_fact_id = NULL` and would hit that index on a second credit. - That unpredicated index **cannot build** after honest repair. Compensating debits leave both historical credit rows in place. Two rows with the same `(wallet_id, payment_intent_id)`, both `topup_credit` / `credit`, both still matching the predicate → `CREATE UNIQUE INDEX` fails. Deletes are forbidden. Reclassifying historical money rows purely to please DDL is audit poison. - The index that **can** build after repair is the predicated form I already shifted to: `… WHERE … AND ledger_fact_id IS NOT NULL`. Old pods write `NULL` fact ids and walk straight through it. So the buildable index does not constrain the dangerous writer class. Therefore: **constraint-first-while-live, as I argued it, fails.** There is no DDL + row-marking scheme that both (a) preserves full financial history without deletes and (b) blocks old NULL-fact-id writers, without inventing a second parallel uniqueness regime that old code does not know about. Marking legacy duplicates `superseded = true` or changing `entry_type` only works if every old writer path also filters on that flag — they do not. That is another dual-writer hole, not a fix. **Primary control: @GPT’s quiesced cutover.** Intake closed, every `topup_credit` writer disabled (webhooks, replay workers, admin repair tools, internal clearers, scheduled jobs), retriable **5xx** (never synthetic 200), drain to zero proven in metrics and `pg_stat_activity`, then repair → predicated indexes → dark new handlers → recon gate → reopen only the new path. The denormalized and fact-linked unique indexes become defense-in-depth for **new** writers and for future bugs, not the mechanism that makes the rolling deploy safe. **Exact DDL after quiescence + repair + backfill of canonical rows only** ```sql -- Fact identity (enforcement for new path) -- on ledger_facts: UNIQUE (source, account_id, fact_type, fact_key) -- One ledger effect per fact (canonical credits only) CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_credit_one_entry_per_fact ON wallet_ledger_entries (ledger_fact_id) WHERE entry_type = 'topup_credit' AND direction = 'credit' AND ledger_fact_id IS NOT NULL; -- Denormalized belt for fact-linked rows only CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_credit_per_pi_factlinked ON wallet_ledger_entries (wallet_id, payment_intent_id) WHERE entry_type = 'topup_credit' AND direction = 'credit' AND ledger_fact_id IS NOT NULL; ``` Row scheme: leave historical duplicate credits as-is; insert compensating debits with their own `reversal_of:{fact_key}` facts; link **exactly one** surviving credit per PI/wallet to a canonical `ledger_fact` (`ledger_fact_id` set); leave extra historical credits unlinked (`ledger_fact_id` NULL) and optionally tagged in auditable metadata (`superseded_by_incident`, etc.). Never link two credits to one fact. Never DELETE. **Price of the closed-intake window (honest)** While the kill switch is on, the PSP sees timeouts/5xx and retries with backoff. That is correct: retriable failure, not ack. Expected closed window for this shape of work against a few hundred thousand dollars in flight and one known double: | Phase | Realistic duration | |---|---| | Kill switch + enumerate/disable all writers + drain | 15–45 min | | Recon + compensating repairs + fact backfill | 1–3 h (dominated by finance sign-off on `wal_8a4f9` and any other dups, not by SQL) | | Index builds `CONCURRENTLY` + VALID checks | 15–60 min depending on ledger size | | Dark deploy, adversarial tests, recon gate | 1–2 h | | **Total intake closed** | **~3–6 hours** if finance is in the room; longer if repair decisions stall | Launch-customer visibility: tell them top-ups are briefly delayed for a ledger integrity window, not that balances are wrong. PSP retry pileup is real but bounded; after reopen, uniqueness + proof turns losers into 200s and drains the storm rather than amplifying it. “Ship today” still holds if the window starts morning UTC and finance is live; it does **not** hold as a casual afternoon drive-by with old pods still taking traffic. **What else breaks in the merged runbook** 1. **Kill-switch scope.** If only the two webhook routes are flagged and a replay worker, admin clearer, or “fix double” script remains live, quiescence is fake. Enumerate every process that can insert `topup_credit` before flipping the switch; exercise the switch in staging first. 2. **Materialized wallet balance.** Same-txn update with the ledger insert is mandatory. A trigger or async balance job outside the fact transaction reintroduces double-increment under concurrency even when ledger uniqueness holds. 3. **Dual-event mapper.** If `payment_intent.succeeded` and `charge.succeeded` are not forced through one allowlisted mapper and one fact_key derivation before reopen, the economic race returns under two event ids with no old pods involved. 4. **Proof query softness.** Proof must join fact → ledger and check wallet, direction, entry_type, amount, currency, provider account. `payment_intent_id` alone is not proof. 5. **@Muse’s post-cutover “truncate backfill — safe”** is wrong once any ledger row references `ledger_facts`. Truncate is not a rollback tool after linkage. 6. **Synthetic 200 under kill switch** is forbidden. 5xx only. A 200 while writers are disabled teaches the PSP the event is done and permanently drops retries for unpaid work. 7. **Finance latency is the real critical path**, not DDL. Compensating entries need human approval; the runbook fails closed if step “repair” is blocked on an empty Slack channel. --- ### 2 — Concession audit | Position | Moved by | Outcome | |---|---|---| | **Advisory lock keep (fact-keyed)** | **Argument**, not clock. Unique-index wait already single-flights the money write; lock only relocates the same queue; timed/try variants convert contention into 5xx and amplify PSP retries. | **Abandoned.** Policy: none. | | **Constraint-first-while-live protects old pods** | **Argument.** Buildable index is predicated (`ledger_fact_id IS NOT NULL`); old writers emit NULL and escape; unpredicated index cannot build after compensation without deletes. | **Abandoned as primary control.** Predicated indexes ship only under quiescence, as defense-in-depth for new writers. | | **Unpredicated `wallet_topup_one_credit_per_pi` in my step-5 DDL** | **My own predicate SHIFT** made that DDL incoherent; I had left the unpredicated form in prose after the reaction adopted the fact-linked predicate. | **Bound now:** only the two predicated indexes above; no unpredicated partial unique over all historical `topup_credit` rows. | | **“Ship handler without C” soft option** | Argument + clock: without quiescence, C either doesn’t build or doesn’t protect. | **Abandoned.** C under quiescence, after repair. | | **D / recon as Friday-only** | Argument (`$340k` + known doubles). | **Already shifted:** full in-flight + incident-window recon is a **pre-reopen gate**; nightly D still Friday. | **On peers (required)** - **@Muse — “Gate: 0 duplicates.”** Concede the gate as written. After compensating debits, `GROUP BY wallet_id, payment_intent_id HAVING count(*) > 1` on `topup_credit` still finds the historical pair. Correct gate: **zero unexplained net economic mismatch** (credits − compensating debits − PSP truth = 0), and **at most one fact-linked canonical credit per PI/wallet**. Count-of-credit-rows is the wrong predicate. - **@Muse — “truncate backfill — safe” before cutover.** Concede unsafe once any `wallet_ledger_entries.ledger_fact_id` references `ledger_facts`. Rollback before linkage can drop empty structures; after linkage, forward-only. - **@GPT — carrying constraint-before-cutover and quiescence.** Primary control is **quiescence** (no dual-writer window). Predicated unique indexes after repair are **defense-in-depth** for the new path and against future handler bugs, not the device that makes old pods safe. That is consistent if stated that way; it is not consistent if “constraints first while old code still lives” is still sold as what kills the race. I had oversold that; @GPT’s CORE quiescence line is the one that survives. --- ### 3 — GO / NO-GO **GO today** under **quiesced cutover only**: kill switch on all `topup_credit` writers (5xx, not 200) → full drain → recon + compensating repairs + canonical fact backfill → predicated unique indexes (`ledger_fact_id IS NOT NULL`) + fact uniqueness → dark new handlers (shared PI/charge mapper, same-txn fact+ledger+balance+inbox, proof-before-200, no advisory lock, no bare `23505`→200) → adversarial tests → zero-unexplained-mismatch recon gate → reopen new path only; forward-only after cutover. **Preserved dissent:** If finance cannot sign compensating entries for known doubles inside the same calendar day, or if every writer class cannot be proven disabled, this is **NO-GO until those two gates are real** — shipping half-quiesced is how Tuesday happens again with better vocabulary. > **Grok's snippets:** > - **CHALLENGE**: "Preserved dissent: @GPT would drop canonical PI index after fact index - I keep both, because only the PI+wallet+voided_at index stops old code that d..." — *I don't read @GPT as dropping the PI index after the fact index—their construction keeps both, with the canonical PI index as the old-writer belt. If someone later proposes fact-index-only, I'd push back for the same reason @Muse states.* > - **EXPLORE**: "CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_credit_per_pi ON wallet_ledger_entries (wallet_id, payment_intent_id) WHERE entry_type='topup_credit..." — *Functionally the same class of fix as @GPT's status column: exclude superseded rows so the index builds, while old writers that leave the marker null still hit uniqueness. Worth comparing voided_at vs idempotency_status for audit clarity and defaulting behavior.* > - **KEEP**: "If an isolated audit write fails but the exact committed credit can be re-proven, return 200 and page the audit defect rather than inducing indefinite..." — *Settles the edge of sync inbox heal: money proof authorizes 200; inbox is best-effort on that path, not a second money gate.* > - **KEEP**: "Correct gate is `0 active canonical duplicates: SELECT wallet_id, payment_intent_id FROM wallet_ledger_entries WHERE voided_at IS NULL AND entry_type=..." — *This fixes the broken '0 duplicates' definition—gate on the active/canonical set after compensation, not on raw historical credit row counts.* > - **KEEP**: "quiescence is primary during repair and index establishment. After validation, the constraint becomes the fail-closed defense while old consumers are ..." — *Clean layering: close writers to bootstrap the invariant, then keep the invariant as the belt if anything leaks—never the reverse.* > - **KEEP**: "The compensating fact must include the duplicate ledger-row ID, such as `reversal_of:{fact_key}:{duplicate_entry_id}`. Otherwise several duplicate row..." — *Easy to miss under time pressure: without the row id in the reversal fact key, multi-duplicate sets can fight each other on the same reversal identity.* > - **SHIFT**: "Old code omits `idempotency_status`, so its inserts default to `canonical` and are constrained once the first index is valid. The marker changes class..." — *Before I treated the collision as binary—either unpredicated (won't build) or fact-linked (won't constrain old writers)—and conceded full dependence on quiescence. A default-to-canonical classification column that old inserts inherit without knowing about it restores a real fail-closed belt once the index is valid, so I no longer treat constraint-as-defense as impossible.* > - **CORE**: "But this DDL does **not** make constraint installation safe while writers remain active. An old pod can create another duplicate after the repair scan..." — *This is why quiescence remains primary even with a clever marker: the race between repair and index validation under live writers reopens the exact window we are trying to close.* ### Reactions (claim map) > **Claim** — GPT: "But this DDL does **not** make constraint installation safe while writers remain active. An old pod can create another duplicate after the repair scan but before the concurrent index becomes valid, causing the build to fail or extending the repair loop." > - CORE by Grok — "This is why quiescence remains primary even with a clever marker: the race between repair and index validation under live writers reopens the exact window we are trying to close." > - CORE by Muse — "Even with canonical marker, CONCURRENTLY validation can race a new duplicate - quiescence must precede the build." > **Claim** — GPT: "Old code omits `idempotency_status`, so its inserts default to `canonical` and are constrained once the first index is valid. The marker changes classification metadata, not financial history; the original credit and compensating debit remain visible." > - KEEP by Muse — "Default canonical solves the NULL-escape without touching old binary - retains history and makes old writers fail-closed." > - SHIFT by Grok — "Before I treated the collision as binary—either unpredicated (won't build) or fact-linked (won't constrain old writers)—and conceded full dependence on quiescence. A default-to-canonical classification column that old inserts inherit without knowing about it restores a real fail-closed belt once the index is valid, so I no longer treat constraint-as-defense as impossible." > **Claim** — GPT: "If an isolated audit write fails but the exact committed credit can be re-proven, return 200 and page the audit defect rather than inducing indefinite financial retries. If the database failure prevents reliable proof, return 5xx." > - KEEP by Grok — "Settles the edge of sync inbox heal: money proof authorizes 200; inbox is best-effort on that path, not a second money gate." > **Claim** — GPT: "quiescence is primary during repair and index establishment. After validation, the constraint becomes the fail-closed defense while old consumers are verified dead. New and old consumers still must never be intentionally eligible at the same time." > - KEEP by Grok — "Clean layering: close writers to bootstrap the invariant, then keep the invariant as the belt if anything leaks—never the reverse." > **Claim** — GPT: "The compensating fact must include the duplicate ledger-row ID, such as `reversal_of:{fact_key}:{duplicate_entry_id}`. Otherwise several duplicate rows can collide on one reversal key." > - KEEP by Grok — "Easy to miss under time pressure: without the row id in the reversal fact key, multi-duplicate sets can fight each other on the same reversal identity." > **Claim** — GPT: "The previously proposed `ledger_fact_id IS NOT NULL` denormalized index does **not** constrain old pods, because old inserts leave `ledger_fact_id` null." > - CORE by Muse — "This names the exact hole I missed - predicated index builds but lets old NULL writers escape, so protection vs buildability can't both hold." > **Claim** — Grok: "Therefore: **constraint-first-while-live, as I argued it, fails.**" > - CORE by GPT — "This correctly identifies that the original index strategy cannot bootstrap safely over compensated historical duplicates while legacy writers remain active." > - SHIFT by Muse — "I previously believed constraint-first-while-live was sufficient; this line moved me to quiescence-primary + voided/canonical marking as defense-in-depth." > **Claim** — Grok: "Correct gate: **zero unexplained net economic mismatch** (credits − compensating debits − PSP truth = 0), and **at most one fact-linked canonical credit per PI/wallet**." > - KEEP by GPT — "This fixes the gate by distinguishing immutable historical rows from the active canonical effect instead of pretending compensation removes duplicate history." > **Claim** — Grok: "That unpredicated index **cannot build** after honest repair." > - CORE by Muse — "Compensating debits keep both credits, so any unpredicated UNIQUE over all credits is permanently blocked - need voided/is_canonical predicate." > **Claim** — Grok: "The collision the moderator named is real, and it sinks my own constraint-first-while-live claim." > - CORE by Muse — "Clear concession that unpredicated index can't build post-compensation while predicated index doesn't protect - forces quiescence as primary." > **Claim** — Grok: "There is no DDL + row-marking scheme that both (a) preserves full financial history without deletes and (b) blocks old NULL-fact-id writers, without inventing a second parallel uniqueness regime that old code does not know about." > - CHALLENGE by GPT — "A canonical/superseded status with a default that legacy inserts inherit is exactly such a regime: old writes enter the canonical predicate automatically. Quiescence is still needed to install it safely, but the scheme itself is feasible." > **Claim** — Muse: "Correct gate is `0 active canonical duplicates: SELECT wallet_id, payment_intent_id FROM wallet_ledger_entries WHERE voided_at IS NULL AND entry_type='topup_credit' GROUP BY 1,2 HAVING count(*)>1 = 0`." > - KEEP by Grok — "This fixes the broken '0 duplicates' definition—gate on the active/canonical set after compensation, not on raw historical credit row counts." > - CHALLENGE by GPT — "This is a necessary structural gate but not a sufficient financial gate. It must be paired with exact compensation coverage and PSP-to-net-ledger reconciliation, or a missing reversal could pass despite zero active canonical duplicates." > **Claim** — Muse: "CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_credit_per_pi\nON wallet_ledger_entries (wallet_id, payment_intent_id)\nWHERE entry_type='topup_credit' AND direction='credit' AND voided_at IS NULL" > - EXPLORE by Grok — "Functionally the same class of fix as @GPT's status column: exclude superseded rows so the index builds, while old writers that leave the marker null still hit uniqueness. Worth comparing voided_at vs idempotency_status for audit clarity and defaulting behavior." > **Claim** — Muse: "Old pods INSERT without `voided_at` -> default NULL -> hits `wallet_topup_one_credit_per_pi`. Second insert -> `23505`. Even if they `catch 23505 -> 200`, no second ledger row." > - KEEP by GPT — "Mechanically, this closes the legacy-writer escape because omitted columns land inside the index predicate. I would name the marker `superseded_at` rather than `voided_at`, since the original entry remains part of financial history and is corrected by a compensating debit." > **Claim** — Muse: "Preserved dissent: @GPT would drop canonical PI index after fact index - I keep both, because only the PI+wallet+voided_at index stops old code that doesn't know facts." > - CHALLENGE by Grok — "I don't read @GPT as dropping the PI index after the fact index—their construction keeps both, with the canonical PI index as the old-writer belt. If someone later proposes fact-index-only, I'd push back for the same reason @Muse states." > **Claim** — Muse: "Repair + backfill + two `CONCURRENTLY` builds + full recon on ~$340k in-flight: 60-90 min closed intake in this data size." > - CHALLENGE by GPT — "That estimate is too confident without ledger row count, index-build measurements, duplicate cardinality, and finance approval latency. The maintenance window should be measured in a dry run and bounded operationally rather than inferred from dollars in flight." > **Claim** — @Moderator: "The candidate invariant if constraint-first-while-live falls." > - CORE by GPT — "This is the deployment crux: quiescence must be demonstrated across every writer class, not approximated by draining only the public webhook pods." > **Claim** — @Moderator: "There must never be a period in which old and new credit writers are both eligible to consume top-up events." > - CORE by Muse — "The invariant that survives when constraint-first-while-live falls - defines quiesced cutover cost we must price with 5xx/retry window." > **Claim** — @Moderator: "Your own SHIFT adopted the `ledger_fact_id IS NOT NULL` predicate — and old pods write NULL fact ids, so they pass through this index." > - CORE by Muse — "Moderator pinpoint of the collision - buildable predicate equals invisible to old code." ### Takeaway **You cannot safely patch this live; you must fully quiesce the system, mark canonical records, and only then cut over to a constraint-enforced ledger.** #### Majority: Stop all traffic before fixing; don't rely on database constraints alone. You cannot safely patch this with live writers because the index you need to build won't protect you from legacy pods. A total stop of intake—a quiesced cutover—is mandatory to drain workers, apply repairs, and ensure no overlap between old and new logic. > **Claim** — @Moderator: "The candidate invariant if constraint-first-while-live falls." > - CORE by GPT — "This is the deployment crux: quiescence must be demonstrated across every writer class, not approximated by draining only the public webhook pods." > **Claim** — @Moderator: "Your own SHIFT adopted the `ledger_fact_id IS NOT NULL` predicate — and old pods write NULL fact ids, so they pass through this index." > - CORE by Muse — "Moderator pinpoint of the collision - buildable predicate equals invisible to old code." > **Claim** — @Moderator: "There must never be a period in which old and new credit writers are both eligible to consume top-up events." > - CORE by Muse — "The invariant that survives when constraint-first-while-live falls - defines quiesced cutover cost we must price with 5xx/retry window." #### Majority: Use a status marker to isolate historical duplicates from new credits. You need an explicit status marker like voided_at or idempotency_status to tag historical duplicates. This lets your unique index ignore the old mess while still forcing legacy code to fail if it tries to double-credit. > **Claim** — GPT: "Old code omits `idempotency_status`, so its inserts default to `canonical` and are constrained once the first index is valid. The marker changes classification metadata, not financial history; the original credit and compensating debit remain visible." > - KEEP by Muse — "Default canonical solves the NULL-escape without touching old binary - retains history and makes old writers fail-closed." > - SHIFT by Grok — "Before I treated the collision as binary—either unpredicated (won't build) or fact-linked (won't constrain old writers)—and conceded full dependence on quiescence. A default-to-canonical classification column that old inserts inherit without knowing about it restores a real fail-closed belt once the index is valid, so I no longer treat constraint-as-defense as impossible." > **Claim** — Muse: "Old pods INSERT without `voided_at` -> default NULL -> hits `wallet_topup_one_credit_per_pi`. Second insert -> `23505`. Even if they `catch 23505 -> 200`, no second ledger row." > - KEEP by GPT — "Mechanically, this closes the legacy-writer escape because omitted columns land inside the index predicate. I would name the marker `superseded_at` rather than `voided_at`, since the original entry remains part of financial history and is corrected by a compensating debit." > **Claim** — Muse: "CREATE UNIQUE INDEX CONCURRENTLY wallet_topup_one_credit_per_pi\nON wallet_ledger_entries (wallet_id, payment_intent_id)\nWHERE entry_type='topup_credit' AND direction='credit' AND voided_at IS NULL" > - EXPLORE by Grok — "Functionally the same class of fix as @GPT's status column: exclude superseded rows so the index builds, while old writers that leave the marker null still hit uniqueness. Worth comparing voided_at vs idempotency_status for audit clarity and defaulting behavior." #### Majority: Validate the ledger mathematically, not just by counting rows. Don't just check for 'zero duplicates,' as historical records will always exist. The only reliable gate is verifying that the net ledger balance perfectly matches the PSP's reported reality after all compensating debits are applied. > **Claim** — Muse: "Correct gate is `0 active canonical duplicates: SELECT wallet_id, payment_intent_id FROM wallet_ledger_entries WHERE voided_at IS NULL AND entry_type='topup_credit' GROUP BY 1,2 HAVING count(*)>1 = 0`." > - KEEP by Grok — "This fixes the broken '0 duplicates' definition—gate on the active/canonical set after compensation, not on raw historical credit row counts." > - CHALLENGE by GPT — "This is a necessary structural gate but not a sufficient financial gate. It must be paired with exact compensation coverage and PSP-to-net-ledger reconciliation, or a missing reversal could pass despite zero active canonical duplicates." > **Claim** — Grok: "Correct gate: **zero unexplained net economic mismatch** (credits − compensating debits − PSP truth = 0), and **at most one fact-linked canonical credit per PI/wallet**." > - KEEP by GPT — "This fixes the gate by distinguishing immutable historical rows from the active canonical effect instead of pretending compensation removes duplicate history." #### Unanimous: Drop the advisory locks; the unique index handles serialization. Everyone agreed to abandon the advisory locks. The unique index on ledger facts already forces the necessary serialization, and adding locks just introduces extra failure modes like timeouts. > **Claim** — Grok: "Therefore: **constraint-first-while-live, as I argued it, fails.**" > - CORE by GPT — "This correctly identifies that the original index strategy cannot bootstrap safely over compensated historical duplicates while legacy writers remain active." > - SHIFT by Muse — "I previously believed constraint-first-while-live was sufficient; this line moved me to quiescence-primary + voided/canonical marking as defense-in-depth." > **Claim** — Grok: "The collision the moderator named is real, and it sinks my own constraint-first-while-live claim." > - CORE by Muse — "Clear concession that unpredicated index can't build post-compensation while predicated index doesn't protect - forces quiescence as primary." --- --- ## Sources - [Stripe Webhook Failures Monitoring Setup 2026](https://www.buildmvpfast.com/blog/stripe-webhook-failures-silent-revenue-loss-monitoring-setup-2026) - [Stripe Webhooks: Complete Guide with Event Examples](https://www.magicbell.com/blog/stripe-webhooks-guide) - [Stripe Webhooks 2026: Handling, Signature...](https://viprasol.com/blog/stripe-webhook-handling/) - [Acquiring Advisory Locks in PostgreSQL: Understanding pg_try_a...](https://www.codegenes.net/blog/acquiring-advisory-locks-in-postgres/) - [PostgreSQL advisory locks for concurrency-safe workflows](https://appmaster.io/blog/postgresql-advisory-locks-double-processing) - [PostgreSQL Advisory Locks in 2026](https://viprasol.com/blog/postgres-advisory-locks/) - [How to Use Advisory Locks in PostgreSQL](https://oneuptime.com/blog/post/2026-01-25-use-advisory-locks-postgresql/view) - [Postgres Upsert: INSERT ON CONFLICT in Practice - QueryPlane Blog](https://queryplane.com/blog/postgres-upsert/) - [Handling Stripe Webhook Events in FastAPI: payment_intent.succeeded, ...](https://fastro.ai/blog/fastapi-stripe-webhook-events) - [Webhook Retry Logic: Handling Failures and Reliability](https://inventivehq.com/blog/webhook-retry-logic-guide) - [Webhook Reliability 2026: Idempotency & Retry Reference](https://www.digitalapplied.com/blog/webhook-reliability-idempotency-retries-engineering-reference-2026) - [Webhook Retry Policy: Backoff, Idempotency & Dead Letter Code](https://hookray.com/blog/webhook-retry-strategies-2026)