Two PaymentIntents: a Losing webhook Reverts the Winner

The short version

A customer completing a manual-capture card-hold Checkout in two tabs produces two PaymentIntents for the same contract. Only the winner’s PI id gets recorded on the contract row. The succeeding/canceled webhook events for the losing PI still matched the same row, because the UPDATE that transitioned hold state never checked stripe_payment_intent_id — so an event meant for the loser could overwrite a hold that was still live under the winner.

What it looks like

The card-hold rail uses a Checkout with payment_intent.capture_method: manual so charging waits until the application is approved. Before the fix (at 608ee58), claiming the contract row looked like this:

const { data: held } = await admin
  .from("contracts")
  .update({
    stripe_hold_status: "authorized",
    stripe_payment_intent_id: paymentIntentId,
    payment_method: "card",
  })
  .eq("id", contractId)
  .is("stripe_hold_status", null)
  .select("id")
  .maybeSingle();
if (held) {
  // Slack notification, audit log
}
return { contractId };

The UPDATE only succeeds while stripe_hold_status is still null, so if a second checkout.session.completed arrives for the same contract, it loses the claim and held comes back null — quietly, by design. But the losing PaymentIntent itself is untouched on Stripe’s side and stays at requires_capture. Nothing releases it, so the customer’s card can carry an unused hold for up to seven days.

On top of that, the payment_intent.succeeded and payment_intent.canceled handlers narrowed their target row using only this condition (same 608ee58 snapshot):

// payment_intent.succeeded
if (pi.metadata?.kind === "hold") {
  await admin
    .from("contracts")
    .update({ stripe_hold_status: "captured" })
    .eq("id", contractId)
    .eq("stripe_hold_status", "authorized");
}

// payment_intent.canceled
const { data: released } = await admin
  .from("contracts")
  .update({ stripe_hold_status: "released" })
  .eq("id", contractId)
  .eq("stripe_hold_status", "authorized")
  .select("id")
  .maybeSingle();

Neither checks which PaymentIntent the event is actually about — only contracts.id and stripe_hold_status = 'authorized'. When the losing PI auto-expires seven days later and Stripe fires payment_intent.canceled, contractId still matches and the winner’s hold is still authorized, so the condition matches too — and the still-live winning hold gets flipped to released.

Why

stripe_payment_intent_id was already stored on the contract row, but the state-transition UPDATEs never used it as a filter. The claim UPDATE (.is("stripe_hold_status", null)) is exclusion logic for deciding who wins the race — it is not the same thing as verifying which PaymentIntent a later event is actually reporting on. Reusing one UPDATE shape for both jobs meant nothing was left to reject an event that belonged to the PI that had already lost.

Fixing it

Both the succeeded and canceled UPDATEs gained an explicit stripe_payment_intent_id match, so an event for anything other than the currently-recorded PI can no longer hit the row (at 74e2c34a):

// payment_intent.succeeded
if (pi.metadata?.kind === "hold") {
  await admin
    .from("contracts")
    .update({ stripe_hold_status: "captured" })
    .eq("id", contractId)
    .eq("stripe_hold_status", "authorized")
    .eq("stripe_payment_intent_id", pi.id);
}

// payment_intent.canceled
const { data: released } = await admin
  .from("contracts")
  .update({ stripe_hold_status: "released" })
  .eq("id", contractId)
  .eq("stripe_hold_status", "authorized")
  .eq("stripe_payment_intent_id", pi.id)
  .select("id")
  .maybeSingle();

The claim-losing branch of checkout.session.completed, which previously had no else at all, now distinguishes a duplicate delivery of the same PI (stays silent, as before) from a genuinely different PI (a real second tab). Only the latter triggers an immediate best-effort cancel and a Slack alert:

if (held) {
  // winner path
} else {
  const { data: cur } = await admin
    .from("contracts")
    .select("stripe_payment_intent_id")
    .eq("id", contractId)
    .maybeSingle();
  const winner = cur?.stripe_payment_intent_id ?? null;
  if (paymentIntentId && winner !== paymentIntentId) {
    try {
      await stripe.paymentIntents.cancel(paymentIntentId);
    } catch {
      /* already expired/canceled — Stripe releases it on its own */
    }
    await notifySlack(
      `🔴 *Duplicate hold detected* released the second PaymentIntent (winner: ${winner} / released: ${paymentIntentId})`
    );
  }
}

Now a duplicate PI is distinguished from a duplicate delivery, and only the former is released right away — instead of waiting out the seven-day auto-expiry with the customer’s card still held.

Preventing a repeat

This path never reached a real customer. It was flagged 26 minutes after the code was written, by an independent reviewer (a separate agent) requesting changes, and fixed before any production deploy. There’s no record in the source of a new test or static check added specifically for this — what actually stopped this defect was a second review pass before deploy, not a mechanism in the code itself.

Folding two different jobs — “who wins the exclusive claim” and “which PaymentIntent does this event belong to” — into the same UPDATE’s WHERE clause leaves whichever job wasn’t explicitly checked unenforced. An .eq("id", contractId).eq("stripe_hold_status", "authorized") pattern reads as if it selects a unique row, but what it actually selects is “the contract that is currently authorized” — not “the PaymentIntent this specific event is about.” Those are two different claims, and writing a state-transition UPDATE means checking both on purpose.