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-24

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

  1. A target plan whose cycle ceiling is already used. When a NEXT_PERIOD change takes hold, executeCycle adopts the target plan before billing. But the maxBillingCycles gate in billCycle runs earlier, on the plan being left. A target that bills at most N cycles, already used, could therefore be billed again, and recordSuccess only expired the subscription after the order was raised.
    Fix: scheduledChangeAt refuses the change with SUBSCRIPTION_CYCLE_CEILING_REACHED. It is cleared, the subscription.scheduled-change-refused event is emitted, and the period opens on the current plan.
  2. Re-cut and reprice in one transaction. When a change took hold at the retry of a failed attempt, applyScheduledChange moved the row's end on its transaction, and executeCycle repriced 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.restateUnsettled replaces recutUnsettled. 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 with priceCycle on 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's repriceUnsettled then finds nothing left to restate.

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:

  • repriceUnsettled reads 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;
  • scheduledPlanTerms includes maxBillingCycles, 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