Skip to content

fix: payment_failed immediately sets expired, add precedence parens#2512

Open
fix2015 wants to merge 1 commit into
different-ai:devfrom
fix2015:fix/payment-failed-status-and-precedence
Open

fix: payment_failed immediately sets expired, add precedence parens#2512
fix2015 wants to merge 1 commit into
different-ai:devfrom
fix2015:fix/payment-failed-status-and-precedence

Conversation

@fix2015

@fix2015 fix2015 commented Jul 6, 2026

Copy link
Copy Markdown

Two issues in the billing code:

  • The `invoice.payment_failed` webhook handler was setting the subscription status to `"expired"` on the very first failed payment. Stripe retries payments multiple times over days/weeks via its dunning cycle — immediately killing the subscription before Stripe even retries means a single transient card decline locks people out. Changed to `"past_due"` which is the standard status for a failed payment that's still being retried.

  • Added explicit parentheses to the tier check in `readInferenceMetadata` — the `&&` vs `||` precedence was technically correct but ambiguous enough that a future edit could easily change the logic without realizing it.

- invoice.payment_failed handler was setting subscription status to
  "expired" on the very first failed payment attempt. Stripe retries
  multiple times over days — setting expired immediately kills the
  subscription before Stripe's dunning even starts. Changed to
  "past_due" which matches Stripe's own terminology.
- added explicit parentheses to the tier check in readInferenceMetadata
  to make the operator precedence unambiguous
@vercel

vercel Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
openwork-landing Ready Ready Preview, Comment, Open in v0 Jul 6, 2026 8:13pm

@vercel

vercel Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@fix2015 is attempting to deploy a commit to the Different AI Team on Vercel.

A member of the Team first needs to authorize it.

@Pablosinyores Pablosinyores left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the dunning behavior — killing a subscription on the first invoice.payment_failed is wrong given Stripe's retry cycle. A few things worth thinking through before this lands, because I don't think the diff fully delivers the "don't lock people out" goal yet.

past_due is a valid status, no schema risk. It's in OrgSubscriptionStatus and handled by subscriptionStatus(), so the DB/type side is fine.

But access gating doesn't treat past_due as still-active. Access is granted only for ACTIVE_STATUSES = {active, trialing} (lines 248/253/512/556/595), and past_due is explicitly a member of EXPIRED_STATUSES (line 25). So during the retry window a past_due org is denied exactly like expired — the user is still locked out. The rename is a correctness/labeling win (accurate status, and it lets a later customer.subscription.updated cleanly restore active on a successful retry), but it isn't a grace period on its own. Might be worth saying that in the description so the behavioral impact isn't over-credited.

The inference case has a real recovery gap. The handler still calls setInferenceEnabled({ enabled: false }) immediately on payment_failed, and nothing re-enables it when the payment later succeeds: upsertOrgSubscriptionFromStripe only ever disables inference (line ~330, on EXPIRED_STATUSES), and the only re-enable path is checkout.session.completed (line ~658). There's no invoice.paid / invoice.payment_succeeded case, so recovery arrives via customer.subscription.updated → the status flips back to active but the inference flag stays off. Net: an inference user who hits a single transient decline loses inference until they re-checkout — the exact "locks people out" symptom this PR is aiming at. If the goal is to make a transient decline non-fatal, re-enabling inference on recovery (an ACTIVE_STATUSES branch in the upsert, or handling invoice.payment_succeeded) is probably the change that actually does it.

Parens change — reads clearer and it's a safe no-op (&& already binds tighter than ||, so behavior is identical). Fine as-is.

Not blocking the status rename itself — it's strictly more accurate and low-risk — just flagging that the described user-facing benefit needs the inference re-enable / gating piece to be real.

@Pablosinyores Pablosinyores left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The billing fix is the right call — invoice.payment_failed firing on the first retry shouldn't terminate the subscription, since Stripe's dunning cycle keeps retrying for days and the customer usually recovers. Moving to past_due there is correct.

Two things worth confirming so the state machine stays coherent:

  1. Where does past_due become terminal? With this change, a subscription that genuinely fails all retries needs something else to move it to expired — presumably customer.subscription.deleted (or subscription.updated with status: "canceled") once Stripe gives up. Worth double-checking that handler exists and lands on expired, otherwise a permanently-failed sub sits in past_due forever and never actually expires.

  2. How does access control read past_due? If entitlement checks gate on status === "active", a past_due user is cut off immediately anyway (so the dunning grace is only cosmetic); if they gate on status !== "expired", the user keeps full access through the whole retry window. Either may be intended, but it's the behavioral question this change really turns on — worth a line in the PR on which one you want during dunning.

On the precedence parens in inference.ts: good for readability, but note it doesn't change evaluation — && already binds tighter than ||, so a || b && c was already parsed as a || (b && c). So the guard behaved correctly before; this is a clarity improvement, not a behavior fix (worth saying so it isn't mistaken for the second half of a bug).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants