fix(billing): prevent & contain duplicate DevPass subscriptions#2697
fix(billing): prevent & contain duplicate DevPass subscriptions#2697steebchen wants to merge 3 commits into
Conversation
A failed PaymentIntent for a subscription invoice (Pro / DevPass / chat plan) was recorded as a `credit_topup` transaction, producing a phantom "Credit top-up failed via Stripe" row on the customer's billing history even though they never initiated a credit purchase. handlePaymentIntentSucceeded already ignores subscription invoice intents (they lack `baseAmount` metadata), but handlePaymentIntentFailed only skipped end-user wallet top-ups and treated every other failed intent as a credit top-up. Gate the credit_topup transaction insert on the same discriminator: real top-ups always set `baseAmount` (manual + auto) or carry a pending `transactionId`; subscription invoice intents carry neither. Subscription-failure tracking (paymentFailure row + dunning email) and dev/chat plan credit freezes are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughDev-plan setup is hardened with stale-subscription cancellation and duplicate-card cleanup before charging. Two Stripe webhook handlers fix critical bugs: ChangesStripe Webhook Handler Fixes and Setup Cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A superseded DevPass/chat subscription — e.g. an abandoned first checkout attempt whose payment never completed and that Stripe later marks `incomplete_expired` — still carries `subscriptionType` metadata. handleSubscriptionUpdated detected the plan type from that metadata alone, so the stale subscription's expiry ran the dev-plan branch and froze the *active* plan's credits (pinning the limit to current usage), silently throttling a healthy subscriber to their already-spent amount. Gate subscription.updated handling on the event's subscription being the org's current dev/chat plan subscription, mirroring the id check handleInvoicePaymentFailed already performs. Stale events for a different subscription id are ignored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Setup-mode checkout creates one subscription per setup session, so a retried checkout after a failed first attempt left the customer with two DevPass subscriptions (a dangling incomplete one plus the active one) and two PaymentMethod objects for the same card. The dangling subscription later expired and corrupted the active plan's credit state. When finalizing a session, before creating/activating its subscription: cancel any stale incomplete/past_due DevPass subscriptions from prior attempts (matched on metadata, excluding the current session so it stays safe under the finalize/webhook race), and detach duplicate copies of the card by fingerprint, keeping the one being used. Combined with the id-gating in handleSubscriptionUpdated, a customer ends up with exactly one DevPass subscription and one card. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The incident
A PRO DevPass customer reported their dashboard allowance was "only $20" (vs the expected ~$237 = $79 × 3), showing "Allowance reached for this billing cycle" after spending $19.67 — plus a baffling failed $79 "Credit Topup" they never initiated.
Stripe data showed the customer's checkout produced two subscriptions and two copies of the same card:
The $237 was granted correctly by the active subscription, the card is valid, and the subscription is healthy. The duplicate first attempt then corrupted everything. This PR fixes the duplicate at the source and contains the two ways a stale subscription leaked into billing state.
Fix 1 (root cause) — prevent duplicates being created
Setup-mode checkout creates one subscription per setup session, and the pre-checkout guard only blocks fully activated plans (
dev-plans.ts:200) — so a retried checkout after a failed first attempt creates a second subscription, and each session saves the card as a fresh PaymentMethod (within-org fingerprint dedup was absent).In
finalizeDevPlanSetupSession, before creating/activating this session's subscription:incomplete/past_dueDevPass subscriptions from prior attempts (matched on metadata; the current session is excluded viasetupSessionId, keeping it safe under the finalize/webhook race).Result: a customer ends up with exactly one DevPass subscription and one card.
Fix 2 (the "$20") — stale subscription froze the active plan
handleSubscriptionUpdateddetected the plan type from event metadata alone:~24h after checkout the abandoned first subscription flipped to
incomplete_expired; itscustomer.subscription.updatedstill carriedsubscriptionType: dev_plan, so the dev-plan branch ran for a subscription that wasn't the org's active one →freezeDevPlanCreditspinneddevPlanCreditsLimitto the $19.67 already spent. The dashboard renders that as "$20" viatoFixed(0), $0.00 remaining / 100% used — matching the screenshot.handleInvoicePaymentFailedalready gated on the matching id; this mirrors it forsubscription.updated.Fix 3 — subscription invoice failure mislabeled as a credit top-up
The first attempt's failed $79 fired
payment_intent.payment_failed.handlePaymentIntentFailedrecorded acredit_topupfor every non-wallet failed intent — including subscription invoices — producing the phantom "Credit top-up failed" row. The success handler already guards this viabaseAmount === undefined; the failure handler now does too (only records a top-up whenbaseAmountis set or a pendingtransactionIdexists). Subscription-failure tracking (paymentFailure row + dunning email) is preserved.Immediate customer remediation (data, not code)
Their subscription is active and card valid (an earlier "update your card" diagnosis was wrong). Unfreeze: set
devPlanCreditsFrozen=falseand restoredevPlanCreditsLimitfromdevPlanCreditsLimitBeforeFreeze($237). With Fix 2, asubscription.updatedon the active sub would also auto-restore viarestoreDevPlanCredits.Tests
stripe.spec.ts(13 passing, +3 cases):incomplete_expiredsubscription update → does not freeze the active plan or touch cancel flagscredit_topuprow, but failure tracking still runsbaseAmountset) →credit_topupfailed row recordedThe Fix 1 cleanup helpers call Stripe directly; consistent with the existing
finalizeDevPlanSetupSession(no Stripe-mock coverage), each Stripe call is individually guarded so a single failure can't abort activation.Follow-up (not in this PR)
Dashboard messaging: a frozen account shows "Allowance reached… Upgrade to keep coding" rather than a payment-failure prompt;
devPlanCreditsFrozenisn't exposed by/dev-plans/status.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Tests