Commit Graph

8 Commits (4b99c6d5c6ef6639dca6e6f7ecd2929402cfb1e6)

Author SHA1 Message Date
nexxo 4b99c6d5c6 Make the fake behave like Stripe where it matters
Three ways it did not. It replaced metadata where Stripe merges — and merging is
the property AdoptStripePrice's "write only when it differs" comparison is built
around. It handed metadata back uncast where the wire returns strings, so a test
could pass on an int that production's strict === rejects forever. And a tie on
`created` fell to insertion order, so two runs could adopt different prices.

Paging now throws instead of stopping when Stripe says there is another page and
the last item carries no id. invoiceLines() stops there and it costs a short
document; here it costs an unseen orphan and a second live price for one figure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 17:14:46 +02:00
nexxo 40532c6e02 Put the money gate in the class that promises it
AdoptStripePrice says in its own docblock that no adoption can move money, and
could compare three properties. The four that also decide what a Price charges
were filtered in HttpStripeClient — where FakeStripeClient cannot express them,
so no test of the class could reach the promise it makes, and a third client
implementation would drop the check in silence.

The client reports them now and the caller decides. Absent keys stay Stripe's
defaults, which is the direction that matters: read the other way round the gate
refuses every legitimate price and recognition becomes a no-op nothing alerts on.

And the unexplained-orphan warning is a state, not an event. It fired on every
sweep and every customer booking for as long as the orphan existed; once a day
per price is enough to act on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 17:04:25 +02:00
nexxo 865fd16f58 Put the price recognition back, all 3526 lines of it
2321854 removed it. That commit's own work — discarding an unconfirmed
registration and sweeping abandoned ones — is untouched and correct; what went
with it was every file of the Stripe price adoption merged an hour earlier:
AdoptStripePrice, IdempotencyKey, the unique-index migration, both test files,
the spec, the plan, and the edits in six more.

Nothing was lost. The commits stayed in history and the files stayed on disk,
untracked, which is what a staged deletion from a stale tree leaves behind.
Restored from 4eb90c8, the reviewed head, by explicit path — the fourteen paths
2321854 damaged and not one more, so the ten files that commit legitimately
added or changed keep exactly what it gave them.

This is the failure the repo's own rule exists to prevent: stage by path, never
`git add -A`, because a second session's index does not know what a first one
merged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 16:35:55 +02:00
nexxo 2321854967 Discard an unconfirmed registration, and sweep the abandoned ones
tests / pest (push) Failing after 8m17s Details
tests / assets (push) Successful in 19s Details
tests / release (push) Has been skipped Details
"Abmelden und neu registrieren" was advice that could not work. Signing out
frees nothing: the address is still held by the unique index, so registering
again with the address somebody meant fails — and every abandoned attempt
stayed in `users` for ever. Reported exactly that way, and it was right.

The button discards the account instead. Only an UNCONFIRMED one, and only
one with nothing behind it: the webhook creates a customer's login from a
paid checkout, so a user can sit on this page unconfirmed while already
having a contract, and deleting that is data loss dressed up as a
convenience. It refuses and says where to go. A confirmed account is closed,
not discarded — that is ConfirmCloseAccount, with an invoice history behind
it.

R23: confirmed in a modal. The modal mutates nothing; it dispatches the event
the page listens for, so the checks stay in the one place they already are
rather than being duplicated where they could drift.

And nothing has to be discarded by hand. clupilot:prune-unverified removes
registrations nobody confirmed after five days — long enough for somebody who
signed up on a Friday and found the mail in a spam folder on Monday, short
enough that a typo does not hold the correct address for a month. It skips a
confirmed account whatever its age, and any account with a customer record
behind it, matched on user_id AND on the address, because either link means
somebody is a customer. Every removal is logged with the address: a line that
disappears is what somebody asks about later, and a count answers nothing.

The deadline is said on the page as well as in the mail. Somebody who never
received the mail is looking at the page, and the deadline is the reason they
do not have to do anything about an attempt they abandon.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 14:08:02 +02:00
nexxo 3cf16ecd63 Name every property that decides what a price charges
The money gate compared amount, currency and interval and left the rest to a
filter that excluded two fields. A recurring, licensed, monthly price with
transform_quantity divide_by 10, at our exact figure, on our product, carrying
our addon key passed all five conditions. Modules bill BY quantity — a pack is
one item at quantity n — so a customer holding three would have been charged
one. A tiered price was excluded only by accident: Stripe reports unit_amount
null for it and we read that as 0, which fails the amount match unless the
caller's own figure is 0, and PlanPrices::ensure() has no zero guard.

activePricesFor() now also skips a non-empty transform_quantity and any
billing_scheme other than per_unit. Absent keys stay Stripe's defaults, which
is the direction that matters: read the other way round the filter would refuse
every legitimate price and turn recognition into a permanent no-op. The
contract docblock names the four properties instead of claiming to cover all of
them, and says the list is not everything Stripe can put on a price — only
what is known to change what one charges.

And a comment that says a gap is covered is worse than the gap. The sync said a
crash between Stripe creating a product and us storing its id gives back the
same product next run: true for twenty-four hours, false afterwards, and the
exact claim this branch was written to refute. There is no recognition step for
products at all, so an interrupted run leaves an orphan and the next run past
the expiry mints a second one — worse than a duplicate price, because
activePricesFor() then looks at the wrong product and recognition goes blind
for the whole family. Written down as a known gap at both createProduct call
sites, with the trade this branch chose: folding name and metadata into the key
means a renamed family now mints a second product where it used to answer 400,
and a blockade reaches a paying customer while a duplicate product does not.

The sweep no longer reports "created" for what it adopted — the count is taken
before ensure() runs, and telling those two apart is the whole point.

The test named after 2026-07-29 could not reproduce it: the fake's key ledger
was empty, so createPrice() could not have thrown and the test could not tell
"adoption prevented the 400" from "no 400 was possible". Seeded now with the
key today's call sends and a fingerprint over the metadata the pre-9da1358 call
sent. Remove the adopt() call from AddonPrices::ensure() and the test dies with
Stripe's own sentence instead of an assertion.

Also: the plan document still instructed migrate:fresh --env=testing. There is
no .env.testing here, so it targets the live development database; struck
through with the reason, not deleted, and a Korrekturen section records the
five defects the plan's own reviews found in it. Four comments corrected —
"below" that meant above, a counter claimed unable to tie that ties with a
planted price, the unique index's second meaning for remember()'s catch, and
that MariaDB DDL is not transactional so the dedupe commits before the index.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 13:50:20 +02:00
nexxo 06c214e597 Make the money gate compare the whole recurrence
activePricesFor() reported interval alone, never interval_count or usage_type.
A Price we could not have created ourselves — billed every three months, or on
metered usage — could carry our own metadata (Stripe's dashboard "duplicate
price" copies it) and pass AdoptStripePrice's amount/currency/interval check on
'month' alone. Adopting it would move money onto a different recurrence, which
is exactly what nothing here may do.

Filtered at the boundary instead of in the predicate: HttpStripeClient now
skips anything besides Stripe's own defaults (interval_count 1, usage_type
licensed), so activePricesFor() can promise callers it returns only prices we
could have minted. FakeStripeClient needed no filter — it only ever holds
prices its own createPrice()/plantPrice() made, all standard — but says so, so
the omission doesn't read as forgotten.

Two more silent-failure paths, closed with the same three lines each:

- The metadata-differs check was a bare !==, which is key-order sensitive and
  blind to Stripe's own merge. A Price carrying an extra key of its own, or
  metadata handed back in a different order, could never compare equal and
  would be rewritten every sweep forever. Compared through
  array_intersect_key() plus ksort() on both sides instead, so only what WE
  would send is what gets compared.
- Metadata is normalised to strings once, up front, so an un-cast value from a
  caller can't make confirms() fail forever and silently turn adoption off for
  that Price.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 11:15:16 +02:00
nexxo ca575e6df1 Let the catalogue ask Stripe what it already has
Nothing here could. The client could create, archive and unarchive a Price and
had no way to list one, so a run that died between Stripe's create and our
insert left an orphan our table never learned about — and once the key expired,
the next run made a second live Price for the same money.

Paged to the end like invoiceLines(), because a family Product collects a Price
per version, term, treatment and rate change, and stopping at the first page
would leave the orphan unfound and mint the duplicate anyway.

updatePriceMetadata() alongside it: metadata is one of the few fields a Price
lets you change, which is why the metadata format is not part of a Price's
identity and why an adopted orphan is brought up to date rather than replaced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 10:54:23 +02:00
nexxo 60e93aafd7 Put in the key everything the call actually sends
A key says "I already sent this call", not "this is what the object is". The
catalogue calls sent metadata that the key knew nothing about, so adding the
tax_treatment field in 9da1358 poisoned yesterday's key for a day — and
AddonPrices::ensure() runs inside a customer's module booking, not only in the
sweep.

createPrice and createProduct now fold a fingerprint of their parameters into
the key. refund, cancelSubscription, addSubscriptionItem and the checkout
deliberately do not: there a second object is the customer's money taken twice,
and Stripe's refusal is the thing worth keeping.

The fake could not see any of this. Its ledger replayed the first id for a
repeated key without ever comparing what the second call asked for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 10:30:32 +02:00