Two things Stripe was doing on its own, without us.
**A renewal produced no invoice and no mail.** InvoiceMail went out from
exactly one place — the first purchase — so a customer paying every month
for a year received one invoice, for month one. Stripe already says when
the money lands: invoice.paid with billing_reason subscription_cycle. What
was missing was the document.
A renewal has no Order behind it, and one was not invented for the
occasion: an Order is something a customer bought, and writing a purchase
nobody made would corrupt the record of what they have actually ordered.
IssueInvoice::forBilledPeriod() issues from the CONTRACT instead, at its
frozen net price, for the term Stripe billed. Idempotent against the Stripe
invoice id, which is unique in the database rather than checked in PHP —
and because the number is drawn inside the same transaction as the row, a
redelivery takes its number back with it and the series keeps its sequence.
Nothing in the paperwork can fail the webhook: issuing and mailing are both
caught and logged, exactly as confirmByMail() does.
Only a cycle renewal gets a document. The checkout's own invoice already
has one; an upgrade's proration was worked out against Stripe's boundaries
and would not match a document written from our snapshot; a charge raised
by hand is for something this catalogue cannot describe. All three stay in
the register, and are logged once as money no document of ours covers.
**After a plan change, Stripe went on billing the old price.** There was no
way to move a subscription onto another price at all, so a customer who
upgraded paid for the smaller package every month afterwards. The client
can now swap one, the item id it needs is learnt from Stripe's own events
(and asked for once, for contracts older than the column), and the
behaviour is chosen per direction: an upgrade settles immediately, so the
cycle invoice stays exactly the contract price the renewal document states;
a downgrade lands at the period boundary and settles nothing.
A granted contract has no Stripe subscription and is left untouched. A
change that reaches the machine and not Stripe is never rolled back — it is
parked on the contract with the error and the behaviour it needed, logged
as an error, and retried hourly by clupilot:sync-stripe-subscriptions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two ways of charging a customer for something they did not get.
A downgrade was due when `subscriptions.current_period_end` said so, and
Stripe pushes that column forward on every renewal. So a downgrade booked in
March was deferred to the end of April by April's renewal, and to the end of
May by May's — each time by a whole term, and each time the customer was
billed again for the package they had asked to leave. The due date was being
re-derived from a moving target on every scheduler tick.
It is decided once now, at the moment the customer decides, and stamped onto
the contract: `pending_plan` and `pending_effective_at`, the two columns that
have been in the schema since the first migration and were written by nothing
at all. The shop stamps them, PlanChange reads them instead of the period end
when a move is already booked, and clupilot:apply-due-plan-changes finds
contracts by the stamped date rather than orders by a date it recomputes.
The order stays what it always was — the customer's own record of the request,
what the cart shows them and what they remove to change their mind — so it is
still consumed with the change, and removing it unbooks the change. What it no
longer does is decide when. A second downgrade replaces the first rather than
queueing behind it; an upgrade clears one, or it would come due months later
and undo the bigger package; a contract cancelled before the date takes the
booking with it; and a customer with no contract can no longer place a
downgrade at all, which was accepted before and could never be carried out.
A booked change is now visible where the contract is read: on the plan card in
the portal, because the cart entry disappears when it is paid for and the
booking does not, and on the customer row in the console, because that is
where an operator answers "what is this customer on".
The second defect is the same money in one step: BookAddon guarded duplicates
per ORDER — the unique index on (order_id, addon_key) — so two orders for the
same module on one contract both went through. A stale tab or a double click
bought priority support twice.
But storage is deliberately sold in packs, and AddonCatalogue sums the
quantities for exactly that reason, so "refuse the second" is only right for
some of them. Which is which is declared beside each price in
config/provisioning.php (`sold_as`), because it is a commercial decision about
each module and not something to infer from a key: storage is a quantity;
off-site backups are on or off, support is prioritised or it is not, Collabora
Pro is one licence, and a machine answers to one own domain. An undeclared
module counts as an entitlement — the cheaper of the two mistakes — and a test
refuses to let one ship undeclared.
Enforced in the action, where the portal, the console's grant screen and a
webhook all pass, with the sentence the customer is shown rather than a
developer's. A retried webhook for the SAME order still gets its one booking
back; that is one purchase arriving twice, not two purchases. The portal stops
offering what would be refused — booked, or already waiting in the cart — and
goes on offering the pack.
What is missing is on the Stripe side and is not invented here: nothing moves
the subscription's item onto the new Price, so the renewal after a plan change
still bills the old plan at Stripe. StripeClient only creates and archives
catalogue objects, and swapping a price also needs the subscription ITEM id,
which nothing stores.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two findings from reviewing all five phases together rather than one at a time.
The webhook held every event a handler answered `null` to — but `null` also
means "already recorded" and "deliberately skipped", which is every checkout's
own invoice and every redelivered renewal. Their contract is right there, so
`replayHeldFor()` could never come back for them, and ordinary Stripe traffic
silted up the holding area until the weekly prune. It now holds only what it
genuinely cannot match — and if the contract appeared in the meantime, applies
the event instead of dropping it: the creation race is narrow, but losing a
cancellation to it would leave us serving someone who had left.
Also fixes a rollback that predates this work and blocks `migrate:fresh` on
MariaDB entirely: dropping the unique index on customers.user_id fails while
the foreign key added one migration earlier still depends on it. Verified by
building all 33 migrations from nothing on MariaDB, rolling every one of them
back, and building them again.
458 tests green. Codex clean on the full branch diff.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A Product per plan family and a Price per priced row of a published version,
plus the four subscribed webhook events that were arriving and being ignored.
`stripe:sync-catalogue` mirrors the catalogue. Idempotent twice over: a stored
id is skipped, and each call carries an idempotency key derived from our own
row, so a crash between Stripe creating a Price and us recording its id gives
back the same object rather than a second one. That matters more here than
usual — a Stripe Price cannot be edited or deleted, so a duplicate is
permanent. Drafts are not synced at all: a version that has promised nothing
has no business in a price list. --dry-run shows what would be created.
The webhook now handles invoice.paid, invoice.payment_failed,
customer.subscription.updated and customer.subscription.deleted. It never
re-derives an amount: the invoice is the authority for what was charged, and
recomputing it from our catalogue would produce a second, disagreeing answer.
A failed payment is recorded and nothing else — Stripe runs the dunning
schedule, and cutting a customer off on the first failure would punish an
expired card as though it were a refusal to pay.
Only a cycle renewal moves the term on. Stripe also sends paid invoices for
prorations and manual charges, and the checkout's own invoice — which is
already in the register as the purchase, and would otherwise double every
customer's first payment.
Stripe does not guarantee delivery order, which is the source of most of the
care here:
- state changes are judged and written in one transaction with the row held,
so two deliveries cannot both pass a check made on a stale copy;
- an older event never overwrites a newer one, with a rank breaking ties
between events sharing a second — their timestamps have one-second
resolution, and a failed attempt and its successful retry routinely do;
- a term is never shortened, and an ended contract is never revived;
- an event for a contract that does not exist yet is HELD and replayed the
moment it appears, rather than acknowledged and forgotten. A cancellation
dropped that way would leave us serving someone who had already left. Held
rows that never match are pruned after a week.
Register entries are keyed by what they are about — the invoice, the attempt,
the subscription — with a unique index doing the work, because a check two
concurrent deliveries can both pass is not idempotency.
PlanChange is now documented as the preview shown before someone confirms, not
the invoice: Stripe's proration accounts for tax, existing credit and the exact
second the change lands, and ours cannot.
Not run against the live account — `stripe:sync-catalogue` creates objects that
cannot be deleted, so that is the owner's call. The dry run lists 12.
457 tests green. Codex review clean after seven rounds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>