fix(subscription): refuse a scheduled change past the target plan's cycle ceiling; restate the retried row in one write
Merged
EV
14d ago+275+275+012345678901234567890123456789-24-24-01234567890123456789
Follow-up to #11565, for two CodeRabbit findings that were posted as outside-diff comments in review summaries (not inline threads). I missed them before merging.
What
- A target plan whose cycle ceiling is already used. When a
NEXT_PERIODchange takes hold,executeCycleadopts the target plan before billing. But themaxBillingCyclesgate inbillCycleruns earlier, on the plan being left. A target that bills at most N cycles, already used, could therefore be billed again, andrecordSuccessonly expired the subscription after the order was raised.
Fix:scheduledChangeAtrefuses the change withSUBSCRIPTION_CYCLE_CEILING_REACHED. It is cleared, thesubscription.scheduled-change-refusedevent is emitted, and the period opens on the current plan. - Re-cut and reprice in one transaction. When a change took hold at the retry of a failed attempt,
applyScheduledChangemoved the row's end on its transaction, andexecuteCyclerepriced the row after that committed. A payment recorded in between (POST :id/payāmarkPaid) settled the row without its amount, so the later reprice skipped the paid row and left the new period on the old amount.
Fix:SubscriptionBillingService.restateUnsettledreplacesrecutUnsettled. It is one write on the change's transaction that sets the end (when the cadence moves) together with the amount, gross and discount, priced withpriceCycleon the lines, plan and metadata the change opens the period on. The write is predicated on the row being PENDING or FAILED:- a payment that wins first makes it match nothing, and the refusal (
SUBSCRIPTION_BILLING_CYCLE_SETTLED) rolls the whole change back; - a payment that read the row before the commit records itself over the restated amount.
executeCycle'srepriceUnsettledthen finds nothing left to restate.
- a payment that wins first makes it match nothing, and the refusal (
Tests
- refuses a change to a plan whose cycle ceiling the subscription has already used: the event carries the reason, and the renewal is on the old plan;
- rolls the change back when a payment settles the retried row before the change commits: 409, plan, lines, key, amount and end all unchanged, and no new order.
Review round 1 (Greptile, Copilot), fixed in be452e49774:
repriceUnsettledreads the row again and refuses a settled one before its matching-price fast path, so a payment recorded after the restate isn't charged again;scheduledPlanTermsincludesmaxBillingCycles, so a ceiling lowered between the read and the plan lock is caught.
Both have specs that fail on the previous commit.
The whole plugin passes 418/418 locally (Node 26); the two graphql resolver suites only load in CI. The spec doesn't compile against develop, because restateUnsettled is new.
š¤ Generated with Claude Code