fix(subscription): finish a charged billing cycle once, whichever pass gets there

Merged
EV
13d ago
+2331-323

Stacked on #11570 (base fix/subscription-billing-charge-claim, merged up to its tip 24ac0458444, which includes develop with #11565 and #11602). It is retargeted to develop once #11570 merges; the diff here is this change alone.

What

A billing cycle the run charged is now finished once, by whichever pass gets there: the run itself right after recording the charge, a later pass that meets the cycle, or the payment of an invoiced cycle. Follow-up to #11570 (charge claim), which withdrew a first recovery attempt after Copilot's round-2 review; this PR implements what that review asked for.

The bug

executeCycle recorded the outcome (markPaid / markInvoiced), then raiseCycleOrder wrote the subscription's bookkeeping (setup fee, deferred credit), then recordSuccess moved the calendar and told the renewal — three separate writes. A pass cut off anywhere in between (a crash, a failed bookkeeping write, or a markPaid whose read-back threw) left the cycle PAID while nextBillingAt stayed on its period. Every later pass hit the raised-cycle branch, replayed the cycle and completed the subscription.bill key on it, so the subscription was never billed again and subscription.renewed was never emitted.

The change

finishCycle replaces recordSuccess and the bookkeeping writes in raiseCycleOrder:

  • One statement. Bookkeeping and calendar are one write of the subscription, in a transaction with the subscription.activated / renewed / expired outbox events. A refused event rolls the calendar back, and the next pass tells it.
  • Once. The decision is read from a subscription whose calendar still points at the cycle (isFinishable), and the write is predicated on that read's version, not on a wildcard. When two passes race, the second gets the version conflict, reads again, finds the calendar past the cycle, and writes and tells nothing.
  • The row's own period. The calendar moves to the cycle's persisted periodStart/periodEnd, not to a period recomputed from the current plan's cadence.
  • Consumed-credit identity. The run records what the order carried on the cycle, in the same statement as the outcome: metadata.cycleBookkeeping = { credit, setupFeePlanId }. The finish subtracts that credit from pendingCredit, so a credit applyChange added later is kept, and it records the applied cycle in the new reserved key bookkeptCycleId, so nothing is applied twice. The run always writes cycleBookkeeping whole, so a value a correction put in an uncharged cycle's metadata never counts.
  • Invoiced cycles (Q7-W4). Only the bookkeeping is applied; the calendar stays deferred. payCycle → settleDeferredCalendar applies any bookkeeping that is still owed, in the same write that moves the calendar.
  • Retryable after an unrecorded outcome. An error while recording the outcome of an order the gateway raised now propagates instead of becoming a failed charge. That error can be markPaid's read-back throwing after its write committed. The period key stays open, and the next pass finishes the cycle; if the write itself failed, the next pass charges again under the same attempt key, so the order path's idempotency hands back the same order.
  • Recovery. executeCycle's raised branch, and billCycle's REPLAYED branch, now finish a cycle the run charged but never finished (leftUnfinished) instead of replaying it. That covers a PAID cycle with an orderId that never held its calendar for payment, and an INVOICED cycle that holds its calendar (bookkeeping only). The REPLAYED branch also unsticks subscriptions whose key an earlier build already completed.

Behaviour changes worth a look

  • A subscription cancelled or expired while its cycle was being charged is left as it ended: the cycle stays PAID and a warning is logged. Before, recordSuccess wrote ACTIVE and a new nextBillingAt, which revived the subscription and billed it again the next month.
  • A subscription paused while its cycle was being charged keeps its pause, and its calendar moves to the paid period (as settleDeferredCalendar already did). Before, the pause was dropped.
  • A paid cycle that held its calendar for payment is left to the payment to finish (payCycle). The run no longer moves that calendar a second time with a wildcard write.

Out of scope

  • A cycle an operator paid without an order is still replayed. It is pinned by a spec, and it needs an owner decision.
  • A PAID cycle that still holds CALENDAR_DEFERRED because payCycle was interrupted is finished by retrying the payment, as before.

Review rounds (all threads answered and resolved)

  • Round 1 (Copilot 5, Greptile 2), 136e2bd8b9f:

    • Each attempt's order request is frozen on the cycle (metadata.attemptRequest) before the order path is called, and a takeover resends it verbatim. The order path then replays the original order instead of answering ORDER_SUBSCRIPTION_KEY_REUSED for changed lines, which had been recorded as a failed charge and charged again.
    • The service's own cycle keys are reserved at create and correct, and reset by the run's outcome writes.
    • For legacy cycles without cycleBookkeeping, the consumed credit is read from its adjustment.
    • Refunded run-paid cycles are finished.
    • resume keeps the calendar on a cycle that is being charged.

  • Round 2 (Copilot 5, Greptile 2, CodeRabbit 1), 59c431d9b45:

    • Corrections take the charge claim for their write.
    • A frozen attempt is replayed without new adjustments, and it holds the calendar (in resume, the preview and the opening period).
    • A legacy inferred credit is subtracted only when pendingCredit.at is no later than the consumption; otherwise it is kept and logged for reconciliation.

  • Round 3 (Copilot 2), 6b7093332b7:

    • resume predicates its write on the version it decided from.
    • Activation refuses (SUBSCRIPTION_OPENING_CYCLE_CHANGED) to move an opening cycle with an unrecorded attempt, or one charged and not finished.

  • Rounds 4–5 (CodeRabbit, Greptile), 15e6086f97b and c1a50bd42e7: an operator's pay or waive of a PENDING/FAILED cycle takes the charge claim, re-reads under it, refuses an unrecorded attempt, and writes on the claim's token.

  • Merge of #11570's tip 0582eb94a00 (develop with #11565 and #11602):

    • executeCycle claims first, then applies the scheduled change and reprices. A replay of a frozen attempt skips both, along with the adjustments.
    • markWaived keeps #11570's restatement refusal (SUBSCRIPTION_BILLING_CYCLE_RESTATED), now checked under the fence.
    • A pass meeting a raised (paid, invoiced or refunded) cycle reconciles its discount rows to the row's own metadata.discount (asked for on #11602).

  • Merge of #11570's tip 24ac0458444: raiseCycleOrder keeps this branch's shape. On ORDER_SUBSCRIPTION_KEY_REUSED (#11570), the pass also drops the request it froze (unfreezeAttempt): the order path refused that request as not the attempt's own, so leaving it in place would stop an operator from reconciling the cycle.

Specs (subscription.service.spec.ts, new describe "a charged cycle is finished once")

Each case shows one charge, one calendar step and one subscription.renewed per paid cycle, and no credit granted twice:

  • A finish whose write failed: the key stays IN_PROGRESS, the next pass finishes the cycle without charging, the setup fee is remembered, and the next period bills as a renewal.
  • markPaid commits but its read-back throws: no failure is recorded, the key stays open, and the next pass finishes the cycle.
  • A pass takes the key over while the charging pass is finishing: one activated, one renewed, and one calendar step.
  • The outbox refuses the renewal: the calendar rolls back, and the next pass tells the renewal once.
  • The plan changes to a yearly cadence before recovery: the calendar moves one month, from the row's own period.
  • A credit of −30 is consumed and another −10 is deferred before recovery: −10 is left, the next cycle gets −10, and 40 is granted in total.
  • A credit deferred while the run is charging is kept on the run's own finish.
  • A cycleBookkeeping value that a correction wrote is overwritten by the run's own.
  • An invoiced cycle: a later pass applies its bookkeeping without moving the calendar, and its payment then moves the calendar once and leaves a newer credit alone.
  • An invoiced cycle whose bookkeeping never ran: the payment applies it with the calendar.
  • A key that an earlier build completed on a stuck cycle: the next run finishes the cycle.
  • An operator-paid cycle without an order is replayed as before.
  • A subscription cancelled during the charge is not revived.

The existing spec "does not turn a failed bookkeeping write…" spied on the removed rememberSetupFee. It now fails the finish's transaction instead, and it also asserts that the later pass finishes the cycle.

🤖 Generated with Claude Code