One real conflict, in SyncStripeCatalogue::handle(), and both sides were right.
Main's parallel work resolves the adoption singletons and takes "before" counts
so a run can tell created from adopted. The operating-mode work refuses a
catalogue whose stored objects belong to the other mode, and records which mode
the catalogue now belongs to.
Kept both, with the mode guard first. It REFUSES, so it must not run behind
anything that has already built state; the counters only need to be in place
before the create loop, and they still are. Reversing that order would have the
command resolve singletons and take counts on a catalogue it is about to reject.
2017 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three real conflicts, and one file that did not conflict and mattered more.
Overview::notices(): both sides added notices. Kept all of them, and pointed
main's mail check at MailboxTransport::NON_DELIVERING — its own comment already
named the constant while the code carried a copy of the list.
billing.blade.php: main wrapped the page in a contract branch. The "payment is
not set up" error moved OUTSIDE it, because the customer most likely to meet
that message is the one buying for the first time, who has no contract yet and
would never have seen it from inside the @else.
ConsoleReportsRealDataTest: kept both sides rather than choosing. The renamed
test and its docblock explain why admin.systems_ok can no longer be asserted on
a bare install; main's mail pin still keeps "clean" clean in the mail
dimension. Picking one would have quietly weakened the other's claim.
HostStepsTest: git combined main's config()->set('provisioning.dns.zone',
'clupilot.com') with this branch's assertion on clupilot.cloud, and produced a
test that contradicted itself with no marker. Main's approach is the better one
— it pins the zone in the test and asserts the SHAPE of the name rather than
this box's domain — so its assertion stands.
HttpStripeClient merged silently and correctly: secret() still throws,
isConfigured() still reads the vault directly. Had it taken main's
filled($this->secret()), six callers that ask in order NOT to get an exception
would have become exception throwers, and the suite would have stayed green.
CheckoutWithoutStripeKeyTest is the lock that would have caught it.
StripeIdempotencyKeyTest (new on main) leaned on the environment fallback for
stripe.secret. That entry is strict now — no fallback in either direction, so
the .env cannot be a back door for a live key in test mode. It stores a real
vault row instead.
1966 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The price half of this question was answered; the product half was left open and
named as a known gap. It is the more consequential one: a second product makes
every activePricesFor() ask about the wrong one, so price recognition goes blind
for that whole family and every orphan on the first product is unreachable.
The whole active list comes back with no metadata filter, because Stripe cannot
search metadata and an account holds a handful of products. Deciding which are
ours belongs to the next commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
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>
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>
"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>
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>
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>
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>
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>