Repository navigation
feat(devpass): 3DS flow for SCA-required upgrades - #3208
Conversation
Stripe throws subscription_payment_intent_requires_action when error_if_incomplete hits a card that demands 3DS authentication. The tier-change catch blocks only recognized card_declined and invoice_payment_required, so this expected user-facing outcome fell through to the generic 500 and an error-level log, paging on-call for what is a billing prompt, not a server fault. Handle it like the other payment failures: warn-level log and a 402 telling the user to update their payment method. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R6gxRwHbBXgbJdHGnw2yHM
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughChanges3DS tier upgrade
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BillingClient
participant DevPlansChangeTier
participant Stripe
participant DevPlansStatus
BillingClient->>DevPlansChangeTier: submit tier change
DevPlansChangeTier->>Stripe: update subscription
Stripe-->>DevPlansChangeTier: require payment authentication
DevPlansChangeTier->>Stripe: retry pending subscription update
DevPlansChangeTier-->>BillingClient: return clientSecret
BillingClient->>Stripe: confirm card payment
BillingClient->>DevPlansStatus: poll for updated tier
DevPlansStatus-->>BillingClient: return tier status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
When the atomic error_if_incomplete upgrade charge fails with subscription_payment_intent_requires_action, re-issue the change as a Stripe pending update (pending_if_incomplete): nothing applies until the invoice is paid, and Stripe voids the invoice and discards the update if the customer abandons the challenge. The endpoint returns the payment intent client secret; the billing dashboard confirms it with stripe.confirmCardPayment and polls status until the webhook's existing subscription_update fallback applies the tier change, rollover, transaction row and invoice email idempotently. Previously SCA-challenged cards could never complete an upgrade in-place and got a generic failure toast. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R6gxRwHbBXgbJdHGnw2yHM
steebchen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/src/routes/dev-plans.ts (1)
1239-1245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate Stripe error-code extraction.
The same manual
typeof error === "object" && error !== null && "code" in errornarrowing is repeated here and again at Line 1497-1500. Consider extracting a sharedgetStripeErrorCode(error: unknown): string | undefinedhelper to avoid the duplication.♻️ Proposed helper
+function getStripeErrorCode(error: unknown): string | undefined { + return typeof error === "object" && error !== null && "code" in error + ? String((error as { code?: unknown }).code) + : undefined; +} + } catch (updateError) { - const updateErrCode = - typeof updateError === "object" && - updateError !== null && - "code" in updateError - ? String((updateError as { code?: unknown }).code) - : undefined; + const updateErrCode = getStripeErrorCode(updateError);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/dev-plans.ts` around lines 1239 - 1245, Extract the repeated Stripe error-code narrowing into a shared getStripeErrorCode(error: unknown): string | undefined helper, then replace the inline extraction near updateErrCode and the corresponding logic near the later Stripe error handling with calls to that helper. Preserve the existing undefined behavior when no code is present and the current comparisons unchanged.apps/code/src/app/dashboard/(main)/billing/BillingClient.tsx (1)
151-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated post-success handling between the two branches.
The
invalidateInvoices/invalidateStatus/posthog-capture sequence is repeated almost verbatim in both therequires_actionbranch and the normal-success branch. Consider factoring it into a small shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/code/src/app/dashboard/`(main)/billing/BillingClient.tsx around lines 151 - 172, Extract the duplicated post-success sequence into a shared helper near the relevant billing logic, including invoice/status invalidation and the conditional PostHog capture. Invoke that helper from both the requires_action path and the immediate-success path, preserving each branch’s existing toast behavior and comments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/dev-plans.ts`:
- Around line 1219-1299: The pending tier-change path in the subscription update
catch block must retain the upgrade lease while customer authentication is
outstanding. Update the flow around getSubscriptionPaymentConfirmation and the
requires_action response so releaseTierChangeLease is not called when returning
a clientSecret; release the lease only after authentication succeeds or the
pending update is otherwise finalized or discarded, while preserving existing
lease cleanup for non-pending paths.
---
Nitpick comments:
In `@apps/api/src/routes/dev-plans.ts`:
- Around line 1239-1245: Extract the repeated Stripe error-code narrowing into a
shared getStripeErrorCode(error: unknown): string | undefined helper, then
replace the inline extraction near updateErrCode and the corresponding logic
near the later Stripe error handling with calls to that helper. Preserve the
existing undefined behavior when no code is present and the current comparisons
unchanged.
In `@apps/code/src/app/dashboard/`(main)/billing/BillingClient.tsx:
- Around line 151-172: Extract the duplicated post-success sequence into a
shared helper near the relevant billing logic, including invoice/status
invalidation and the conditional PostHog capture. Invoke that helper from both
the requires_action path and the immediate-success path, preserving each
branch’s existing toast behavior and comments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eae7a3a5-6704-4b9c-a7f2-60b484a0d449
📒 Files selected for processing (5)
apps/api/src/routes/chat-plans.tsapps/api/src/routes/dev-plans.spec.tsapps/api/src/routes/dev-plans.tsapps/api/src/stripe.tsapps/code/src/app/dashboard/(main)/billing/BillingClient.tsx
| let updated: Stripe.Subscription; | ||
| try { | ||
| updated = await getStripe().subscriptions.update(subscriptionId, { | ||
| items: [ | ||
| { | ||
| id: subscriptionItemId, | ||
| price: newPriceId, | ||
| }, | ||
| ], | ||
| proration_behavior: "none", | ||
| billing_cycle_anchor: "now", | ||
| payment_behavior: "error_if_incomplete", | ||
| expand: ["latest_invoice.payment_intent"], | ||
| metadata: { | ||
| ...subscription.metadata, | ||
| devPlan: newTier, | ||
| devPlanCycle: existingCycle, | ||
| }, | ||
| }); | ||
| } catch (updateError) { | ||
| const updateErrCode = | ||
| typeof updateError === "object" && | ||
| updateError !== null && | ||
| "code" in updateError | ||
| ? String((updateError as { code?: unknown }).code) | ||
| : undefined; | ||
| if (updateErrCode !== "subscription_payment_intent_requires_action") { | ||
| throw updateError; | ||
| } | ||
| // The bank demands 3DS/SCA authentication, which only the customer can | ||
| // complete in the browser. Re-issue the update as a Stripe pending | ||
| // update (`pending_if_incomplete`): nothing — price, cycle anchor, or | ||
| // metadata — is applied until the invoice is paid, and if the customer | ||
| // abandons the challenge Stripe voids the invoice and discards the | ||
| // update after at most 23 hours, leaving the subscription untouched. | ||
| // The client confirms the returned payment intent secret with | ||
| // Stripe.js; on success the `invoice.payment_succeeded` webhook's | ||
| // `subscription_update` fallback applies the tier change, credit | ||
| // rollover, transaction row and invoice email idempotently. | ||
| const pending = await getStripe().subscriptions.update(subscriptionId, { | ||
| items: [ | ||
| { | ||
| id: subscriptionItemId, | ||
| price: newPriceId, | ||
| }, | ||
| ], | ||
| proration_behavior: "none", | ||
| billing_cycle_anchor: "now", | ||
| payment_behavior: "pending_if_incomplete", | ||
| expand: ["latest_invoice.payment_intent"], | ||
| metadata: { | ||
| ...subscription.metadata, | ||
| devPlan: newTier, | ||
| devPlanCycle: existingCycle, | ||
| }, | ||
| }); | ||
| const { clientSecret } = | ||
| await getSubscriptionPaymentConfirmation(pending); | ||
| if (!clientSecret) { | ||
| // No confirmable payment intent to hand to the client; surface the | ||
| // original failure through the shared catch below. | ||
| throw updateError; | ||
| } | ||
| logger.info("Dev plan tier change pending customer authentication", { | ||
| organizationId: personalOrg.id, | ||
| subscriptionId, | ||
| newTier, | ||
| }); | ||
| // Release the lease: the charge is no longer in flight server-side, and | ||
| // the webhook completes (or Stripe discards) the change out-of-band. | ||
| if (claimedLeaseThisCall) { | ||
| await releaseTierChangeLease(personalOrg.id); | ||
| } | ||
| return c.json( | ||
| { | ||
| id: subscriptionItemId, | ||
| price: newPriceId, | ||
| status: "requires_action" as const, | ||
| clientSecret, | ||
| }, | ||
| ], | ||
| proration_behavior: "none", | ||
| billing_cycle_anchor: "now", | ||
| payment_behavior: "error_if_incomplete", | ||
| expand: ["latest_invoice.payment_intent"], | ||
| metadata: { | ||
| ...subscription.metadata, | ||
| devPlan: newTier, | ||
| devPlanCycle: existingCycle, | ||
| }, | ||
| }); | ||
| 200, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Stripe subscription pending update attribute allow-list billing_cycle_anchor supported list
💡 Result:
Stripe supports billing_cycle_anchor as a field within subscription pending updates [1][2][3]. When you update a subscription using payment_behavior=pending_if_incomplete, Stripe allows you to specify a set of attributes that will be applied to the subscription once the latest invoice is paid [4][5]. The billing_cycle_anchor is explicitly included in the allow-list of attributes supported for these pending updates [1][3]. When a pending update containing this attribute is applied, it determines the date of the first full invoice and, for plans with month or year intervals, the day of the month for subsequent invoices [2][6][7]. For reference, the following attributes are generally supported for pending updates via the Update a Subscription endpoint: - billing_cycle_anchor [1][3] - items (including price and quantity) [1][3] - proration_behavior [1][3] - proration_date [1][3] - payment_behavior [1][3] - discounts [1][3] - coupon [1][3] - promotion_code [1][3] - trial_end [1][3] - trial_from_plan [1][3] - metadata [1][3] - add_invoice_items [1][3] - expand [1][3] To utilize this, you must set payment_behavior to pending_if_incomplete in your update request [4][5]. If the payment for the current invoice fails, Stripe creates a pending update that only applies if the payment eventually succeeds [5]. Once applied, the pending update can be inspected on the Subscription object under the pending_update attribute [2][6].
Citations:
- 1: https://docs.stripe.com/billing/subscriptions/pending-updates
- 2: https://docs.stripe.com/api/subscriptions/object
- 3: https://docs.stripe.com/billing/subscriptions/pending-updates.md
- 4: https://docs.stripe.com/api/subscriptions/update?api-version=2025-10-29.clover
- 5: https://docs.stripe.com/api/subscriptions/update?api-version=2026-03-25.preview
- 6: https://docs.stripe.com/api/subscriptions/object?api-version=2025-07-30.basil
- 7: https://docs.stripe.com/api/subscriptions/object?api-version=2024-06-20
🌐 Web query:
stripe pending updates duplicate subscription update payment behavior pending_if_incomplete existing pending update behavior
💡 Result:
When you use payment_behavior=pending_if_incomplete for a subscription update, Stripe attempts to charge the customer if the update requires payment (e.g., due to proration) [1][2]. If the payment succeeds, the subscription is updated immediately [1]. If the payment fails, the update is not applied, and Stripe instead creates a pending update, which is stored in the pending_update hash on the subscription object [1][3]. If you attempt to apply a new update to a subscription that already has an existing pending update, the following behavior occurs: 1. Replacement: The new update replaces the existing values within the pending_update hash [1][3]. 2. Invoice Handling: Stripe voids the invoice associated with the previous pending update and creates a new invoice that reflects the new set of updates [1][3]. 3. Resolution: If the payment for this new invoice succeeds, the most recent updates (those in the updated pending_update hash) are applied to the subscription [1][3]. 4. Failure: If the payment for the new invoice fails, a new pending update is generated with a new expiry date, replacing the previous one [1][3]. You can track these states by checking the pending_update hash on the subscription object or by listening for webhook events such as customer.subscription.updated [1][3]. Additionally, you can include metadata in these updates, which will be stored in pending_update.metadata and merged into the subscription metadata upon successful resolution of the update [4].
Citations:
- 1: https://docs.stripe.com/billing/subscriptions/pending-updates
- 2: https://docs.stripe.com/api/subscriptions/update
- 3: https://docs.stripe.com/billing/subscriptions/pending-updates.md
- 4: https://docs.stripe.com/changelog/dahlia/2026-05-27/pending-subscription-update-metadata
🌐 Web query:
stripe api subscriptions update with payment_behavior pending_if_incomplete already pending update
💡 Result:
When you update a Stripe subscription using payment_behavior=pending_if_incomplete, Stripe creates a pending update if the required payment fails [1][2]. A subscription can only have one pending update at a time [3]. If you attempt to apply a new update to a subscription that already has a pending update, the behavior depends on the specific changes being requested and how you construct the API call: 1. Pending Update Lifecycle: The pending update is held in the pending_update hash on the Subscription object [2][3]. It is only applied if the associated invoice is paid [2][3]. If the update expires or is discarded, the pending_update hash is cleared. 2. Replacing or Updating Pending Changes: Generally, sending a new update request for a subscription that already has a pending update will attempt to overwrite or merge the changes into the existing pending update, depending on the parameters provided. Stripe's API documentation indicates that for supported attributes (such as items, quantities, or metadata), new updates are generally merged or reconciled within the pending update state [2][4]. 3. Handling Conflicts: If you send a request that conflicts with the existing pending state or includes unsupported attributes, the API may return an error. Because Stripe explicitly manages the pending_update hash as a state, it is recommended to first check if pending_update is populated on the Subscription object before sending a new update [2]. If you need to change the intent of an existing pending update, you should typically perform a new update call with the desired parameters. If the update is rejected because of the existing state, you may need to either wait for the current pending update to be resolved (paid) or expire/cancel the existing update if your workflow allows it. For the most robust integration, listen for customer.subscription.updated webhooks to track the lifecycle of the pending_update hash and ensure your application state remains synchronized with Stripe [2].
Citations:
- 1: https://docs.stripe.com/api/subscriptions/update
- 2: https://docs.stripe.com/billing/subscriptions/pending-updates
- 3: https://docs.stripe.com/api/subscriptions/object?api-version=2024-09-30.acacia
- 4: https://docs.stripe.com/changelog/dahlia/2026-05-27/pending-subscription-update-metadata
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file around the lease/race area and surrounding logic.
if [ -f apps/api/src/routes/dev-plans.ts ]; then
echo "=== file stats ==="
wc -l apps/api/src/routes/dev-plans.ts
echo
echo "=== relevant lease/race comments and line references ==="
rg -n "lease|pending_update|pending_update_exists|subscription_update|charge|billing_cycle|devPlanExpires|payment_intent_requires_action|releaseTierChangeLease|change-tier|tier" apps/api/src/routes/dev-plans.ts | head -n 220
echo
echo "=== surrounding code 1100-1325 ==="
sed -n '1100,1325p' apps/api/src/routes/dev-plans.ts | nl -ba -v1100
else
echo "apps/api/src/routes/dev-plans.ts not found"
fiRepository: theopenco/llmgateway
Length of output: 9466
Keep the in-flight upgrade lease held until subscription authentication succeeds.
billing_cycle_anchor: "now" is supported on Stripe subscription pending updates, but the lease is released as soon as the 3DS clientSecret is returned. A second /change-tier request before the customer completes/authenticates payment can still acquire the lease and call Stripe again, potentially creating another pending update/invoice while one is already in flight. Keep the upgrade lease held during the 3DA wait, or otherwise track in-flight client-side authentication to avoid reopening the double-charge/race window.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/routes/dev-plans.ts` around lines 1219 - 1299, The pending
tier-change path in the subscription update catch block must retain the upgrade
lease while customer authentication is outstanding. Update the flow around
getSubscriptionPaymentConfirmation and the requires_action response so
releaseTierChangeLease is not called when returning a clientSecret; release the
lease only after authentication succeeds or the pending update is otherwise
finalized or discarded, while preserving existing lease cleanup for non-pending
paths.
Extract the repeated Stripe error-code narrowing into a shared getStripeErrorCode() helper (dev-plans.ts), and factor the duplicated post-tier-change refresh (invoice/status invalidation + analytics capture) into refreshAfterTierChange() in the billing dashboard. Also document why the upgrade lease is released when returning the 3DS client secret rather than held until the webhook resolves: holding it would block the common abandon-then-retry path and lock further tier changes for the 15-minute staleness window (the completion webhook never touches the lease), while a concurrent retry cannot double-charge — a subscription holds at most one pending update, so re-issuing voids the prior invoice and replaces it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
Production alert (7/14,
llmgateway-api): a dev plan tier upgrade failed with a500 Failed to change dev plan tierlogged at ERROR severity. The underlying Stripe error wassubscription_payment_intent_requires_action— the cardholder's bank required 3DS/SCA authentication to approve the upgrade charge.The upgrade path uses
payment_behavior: "error_if_incomplete", so Stripe throws and leaves the subscription (and our DB) untouched — nothing was corrupted. But:card_declinedandinvoice_payment_required, so the SCA error fell through to a generic 500 +logger.error, paging on-call for a user-billing outcome, andrequires_action/clientSecretflow existed only for initial subscription checkout, so banks that challenge every off-session charge left users unable to upgrade at all.Fix
1. Error classification (dev plans + chat plans)
subscription_payment_intent_requires_actionis handled like the other expected payment failures: warn-level log and a 402 with an actionable message. This remains the terminal behavior for chat plans and for any dev-plan case where no confirmable payment intent can be produced.2. In-place 3DS flow for dev plan tier upgrades
dev-plans.ts): when the atomicerror_if_incompleteupdate throws the SCA error, the change is re-issued as a Stripe pending update (payment_behavior: "pending_if_incomplete"). Nothing — price, cycle anchor, or metadata — applies until the invoice is paid, and if the customer abandons the challenge Stripe voids the invoice and discards the update after at most 23h, leaving the subscription untouched. The endpoint returns{ status: "requires_action", clientSecret }(new union member on the 200 response) and releases the upgrade lease.BillingClient.tsx): onrequires_action, confirms the payment intent withstripe.confirmCardPayment()(same pattern as the finalize flow), then polls/dev-plans/statusuntil the tier lands.invoice.payment_succeededwebhook fallback forsubscription_updateinvoices applies the tier change, credit rollover, transaction row, and invoice email idempotently — no webhook changes needed.invoice.payment_failedfreeze handler re-fetches the live subscription and only freezes onpast_due/unpaid/incompletestatuses; a pending update keeps the subscriptionactive, so failed/abandoned 3DS attempts cannot freeze the user's current paid cycle.Testing
requires_action+ client secret, re-issues the update aspending_if_incomplete, applies nothing locally, records no transaction, and releases the lease.pnpm buildpasses (typed clients regenerated from the updated OpenAPI spec).Follow-up
Chat plan tier upgrades keep the 402-only behavior for SCA cards; extending the in-place 3DS flow there needs the equivalent UI work in the playground billing surface.
🤖 Generated with Claude Code
https://claude.ai/code/session_01R6gxRwHbBXgbJdHGnw2yHM
Summary by CodeRabbit
New Features
Bug Fixes
Tests