Compare commits

...

13 Commits

Author SHA1 Message Date
nexxo ecfced9257 Merge branch 'main' into claude/cool-sammet-368fee
tests / pest (push) Failing after 9m11s Details
tests / assets (push) Successful in 20s Details
tests / release (push) Has been skipped Details
2026-07-30 19:28:55 +02:00
nexxo 79654333a8 Name the recurring filter, and stop the spec contradicting itself
Three sentences left over from the wave before, all of the kind this branch
exists to end.

- "Every active Price" was one word short. activePricesFor() sends
  `type: recurring` as well as `active: true`, so a one-time Price is never
  listed — which made the hand-built-payment-link sentence false for a one-time
  link: neither listed nor archived. Both places say `recurring` now, and the
  docblock says why the filter is not a gap: createPrice() always sends
  `recurring[interval]` and is the only call that mints a Price at one of our
  Products, so the filter can only ever hide somebody else's one-time Price.
  A filter left out of a sentence is a gap the next reader goes looking for.
- The sweep test file still opened "The orphans nobody can adopt" — the exact
  narrowing the command's own docblock had to be corrected away from. It happens
  to describe the fixtures, which are all metadata-less, so it now says that
  instead of claiming it of the command.
- The design doc called --archive "harmlos" seven lines above the correction
  establishing that it is not, for an adoptable orphan. A document that
  contradicts itself seven lines apart teaches nobody anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 19:24:18 +02:00
nexxo a43be4186a Say what the sweep did not check, before telling anyone to archive
stripe:sweep-orphan-prices described itself as listing "what AdoptStripePrice
will not take over". It computes nothing of the sort: it lists every active
Price at one of our Products that no register row knows, adoptable or not — the
only question it asks is whether a row holds the id. And then it invited the
destructive path, with no word anywhere about running stripe:sync-catalogue
first.

Following that advice can make a plan unsellable. Archive an orphan the next
sync would have adopted and the sync can no longer see it: activePricesFor()
asks Stripe for active prices only, so adoption returns null and createPrice()
fires under the same key the crashed run used. For twenty-four hours Stripe
answers a repeated key by replaying the stored response — the archived Price's
id, in the body it had while it was live. The register writes it down as live,
and the checkout is refused. This is the same class of defect the branch exists
against: a comment that was eloquent and wrong.

- The docblock now says what the command lists, names the twenty-four-hour
  replay so nobody has to rediscover it, and the ordering requirement is in
  $description and printed on every path that found something, --archive
  included. The reassurance that archiving is harmless is now scoped to what
  THIS codebase sells; a payment link an operator built by hand against one of
  our Products is withdrawn all the same, and says so.
- stripe:sync-catalogue --dry-run says "created or adopted" again. A dry run
  counts intents without calling ensure(), so it cannot know which Stripe
  already holds — and a dry run is the first thing an operator runs after an
  interrupted one. The live line keeps both figures, where they are knowable.
- The sweep's plan side gets a test: an orphan at a family Product is reported
  and a known plan Price never is. Deleting either half of the lookup turns it
  red; the five tests it joins all stayed green for the products() half.
- The throttle test pinned "once" and neither "per price" nor "per day". A key
  mutated to a constant would have silenced every orphan after the first and
  kept it green — the silent no-op the branch is built against. It now plants a
  second orphan, and travels a day to hear the first one again.
- Two of my own documents corrected. The spec sentence claiming an adoptable
  orphan never appears in this list is what the implementation faithfully built;
  it holds only if a sync has run since the orphan appeared, which nothing
  enforced. And the two adoption classes do not carry the same run log:
  $adoptions on the Price side, $duplicates on the Product side.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 19:12:57 +02:00
nexxo 571acd0013 Check Stripe is configured on every path the sweep actually uses
The dry-run guard was copied from stripe:sync-catalogue, where it is right:
that command's --dry-run touches nothing, because everything it compares
already lives in our own tables. This command has no such local-only mode —
the report IS a live activePricesFor() call, dry-run or not — so `! $dryRun &&`
made the guard fire only on the one path that would have worked anyway. A
--dry-run against an unconfigured account fell straight into
HttpStripeClient's own exception instead of the clean error and self::FAILURE.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 18:48:49 +02:00
nexxo 277a6bab83 Give the operator something to do about an orphan nobody can adopt
The recognition step refuses a price that proves nothing about itself, and that
is right — adopting a stranger's price is worse than minting a second one. It
left an operator with a log line and no way to act on it.

Reports by default and touches nothing. With --archive the orphans stop being
sold, which is harmless for the reason it always is here: Stripe goes on billing
every subscription already on an archived price, and nothing is sold on a price
no row knows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 18:36:06 +02:00
nexxo 048e5ba81f Subtract only what was counted, not everything a singleton adopted
A figure may only subtract what it counted as an intent in the first place. The
family loop counts a Product before knowing whether it will be adopted or
minted, so an adoption there could honestly be subtracted back out — but
syncModules() has never counted a module's own Product as an intent at all; it
only counts a module's Prices. Reading AdoptStripeProduct::adoptions as one
run-wide delta could not tell the two apart, so a module Product adopted from
an interrupted run silently inflated "adopted" and shrank "created" by exactly
one, for a Product the command never claimed to have made in the first place.

The fix counts the family side locally, at the one call site that already
counts the intent, and leaves a module's Product out of both figures entirely —
adopted or minted, it was never counted, so neither number may move for it.
AdoptStripeProduct::$adoptions is gone with it: nothing reads a singleton-wide
total that cannot be attributed to one side or the other, and keeping it around
unread would be exactly the kind of state this codebase does not leave lying
about.

The duplicate-product report had the same shape of bug one line down: it read
straight off the singleton's list with no before/after snapshot, so a second
handle() call in one process would reprint a duplicate an earlier run already
named. Sliced to what this run itself added, the same way the counters beside
it already were.

Also: the unread `$charged` line in the adoption test that TDD had already
exercised as dead weight, and its now-unused PlanPrices import.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 18:20:27 +02:00
nexxo df7da334ee Say what was adopted, not that it was created
The count is taken before ensure() runs, so the sweep reported objects created in
Stripe when it had made none — and after the next interrupted run, that line is
the first thing a human reads. Telling the two apart is the whole point of the
recognition step.

Comparing the price id before and after the call does not distinguish them
either: there is no id before, in either case. AdoptStripePrice counts its own
adoptions instead, which is why it and its product sibling are now singletons —
resolved per call, a counter on the instance would never pass one.

Duplicate products are named in the report as well as the log. We deliberately do
not deactivate them, so only a person can resolve one, and a person reads this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 17:56:13 +02:00
nexxo d6ec09a9b4 Recognise the product Stripe already has
The gap beside the guard. A run that creates a product and dies before storing
its id leaves an orphan exactly as a price does, and past the key's twenty-four
hours the next run makes a second one — after which activePricesFor() is asked
about the wrong product, price recognition goes blind for the whole family, and
every orphaned price on the first product is unreachable.

Two things are unlike the price side, deliberately. No money gate: a product
carries no amount, so its metadata is the whole of the proof. And a duplicate is
reported, never deactivated — archiving a price is provably harmless because
Stripe keeps billing subscriptions already on it, but deactivating a product
makes its prices unsellable and contracts can be running on those.

Silent about strangers, too. This asks for every active product on the account,
so an operator selling something else through it is an ordinary state, not a
finding worth a line on every run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 17:39:15 +02:00
nexxo fcea7ff3a8 Let the catalogue ask Stripe what products it already has
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>
2026-07-30 17:25:38 +02:00
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 7d66a01a40 Break the follow-ups into six testable steps
Six tasks, each ending on a green billing folder and its own commit: move the
money gate into the class that promises it, four fixes that make the fake behave
like Stripe, ask Stripe for its products, recognise an orphaned one, count
adopted apart from created, and hand the operator a sweep for the orphans nobody
can adopt.

Self-review dropped one item the spec had carried over from the last review: the
zero guard in PlanPrices::ensure(). Its only purpose was to keep a tiered price
unreachable, and the gate move rejects those directly — so it would be a
behaviour change with no remaining benefit, and it would make a free plan
unsellable. Recorded as deliberately not built, with the reason, rather than
quietly dropped.

Also corrected: a test that counted every product where the run mints one per
family, and a step that named the test it replaces only by description.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 16:56:08 +02:00
nexxo 3ceec8fe5e Design the nine things the reviews found and nobody built
The one that matters is orphan PRODUCTS. A price orphan is recognised now; a
product orphan is not, and it is worse — a second product makes every
activePricesFor() ask about the wrong one, so price recognition goes blind for
that whole family and the guard disables itself through the gap beside it.

Duplicated products are reported, not deactivated. That is the owner's call and
it is the careful one: an archived PRICE is provably harmless, which is what the
grandfathering rule has always rested on, but deactivating a PRODUCT makes its
prices unsellable and contracts can be running on those. What was proven for one
is not proven for the other.

The money gate moves into the class whose docblock promises it. Today
AdoptStripePrice can compare three properties and the other four are filtered in
one client, which the fake cannot express — so no test can reach the promise the
class makes.

Also: a sweep command for orphans nobody can adopt, a day's throttle on the
warning that currently fires on every customer booking, and six mechanical
fixes.

Self-review caught one of my own: the plan to tell "created" from "adopted" by
comparing the price id before and after the call cannot work — there is no id
before, in either case. AdoptStripePrice counts its own adoptions instead, which
needs it bound as a singleton, and the spec now says why.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 16:45:39 +02:00
17 changed files with 3145 additions and 194 deletions

View File

@ -0,0 +1,156 @@
<?php
namespace App\Console\Commands;
use App\Models\PlanFamily;
use App\Models\StripeAddonPrice;
use App\Models\StripePlanPrice;
use App\Services\Stripe\StripeClient;
use Illuminate\Console\Command;
/**
* The Prices at our Products that no row of ours knows.
*
* App\Services\Billing\AdoptStripePrice takes over an orphan it can prove is
* ours. What it will not take over a Price carrying none of our metadata, or
* one that charges in a way we never mint it names in the log and leaves,
* because adopting a stranger's Price is worse than minting a second one. That
* was the right call and it left an operator with a log line and no way to act
* on it.
*
* **What this lists is wider than that log line.** Every active RECURRING Price
* at one of our Products that no row of ours knows adoptable or not. The only
* question asked below is whether a register row holds the id; nothing here
* re-asks AdoptStripePrice whether it would have taken the Price over.
*
* Recurring, because activePricesFor() sends `type: recurring` as well as
* `active: true`. That filter can hide somebody else's one-time Price from the
* list; it can never hide one of ours. createPrice() always sends
* `recurring[interval]`, and it is the only call that mints a Price at one of
* our Products at all the setup fee is priced inline at the checkout, under an
* ad-hoc Product of its own. So the filter errs toward listing too little, never
* toward missing an orphan of ours.
*
* **So stripe:sync-catalogue runs first.** Until it has, an orphan listed here
* may be one the next sync would simply have adopted, and --archive on that one
* is worse than it looks. Archiving it makes it invisible to the very step that
* would have rescued it: activePricesFor() asks Stripe for ACTIVE prices only,
* so adoption finds nothing and the sync falls through to createPrice() under
* the same idempotency key the crashed run used, because the key is built from
* what is being sold and everything the call puts on the wire (see
* App\Services\Stripe\IdempotencyKey), and nothing about either has changed. For
* twenty-four hours Stripe answers a repeated key by replaying the response it
* stored rather than doing anything: the archived Price's id, in the body it had
* while it was still live. The register writes that id down as the live Price
* for the row, and a checkout on it is refused.
*
* Without --archive this reports and touches nothing. With it, the orphans are
* archived at Stripe, which is harmless for everything THIS codebase sells, for
* the reason it always is here: Stripe goes on billing every subscription
* already on an archived Price, it only stops being SOLD and nothing this
* codebase sells is sold on a Price no row knows. That reasoning reaches no
* further than this codebase. A Price an operator wired up by hand a
* SUBSCRIPTION payment link built in Stripe's own dashboard against one of our
* Products is exactly the Price carrying none of our metadata described above,
* and --archive withdraws it too. A one-time link is the other side of the
* recurring filter: neither listed nor archived, and so neither protected nor
* disturbed by this command.
*/
class SweepOrphanStripePrices extends Command
{
protected $signature = 'stripe:sweep-orphan-prices
{--archive : Stop selling the orphans instead of only listing them}
{--dry-run : Show what would be archived without touching Stripe}';
protected $description = 'List the Stripe prices at our products that no row knows, and optionally archive them — run stripe:sync-catalogue first';
public function handle(StripeClient $stripe): int
{
$dryRun = (bool) $this->option('dry-run');
$archive = (bool) $this->option('archive');
// Checked on EVERY path, unlike stripe:sync-catalogue's --dry-run,
// which touches nothing because everything it compares is already in
// our own tables. This command has no local-only mode: the report —
// the whole point of it, dry-run or not — is activePricesFor(), a
// live call. Skipping this check under --dry-run would not skip the
// call, only the graceful error in front of it.
if (! $stripe->isConfigured()) {
$this->error('Stripe is not configured (STRIPE_SECRET is empty). Nothing was read.');
return self::FAILURE;
}
$known = StripeAddonPrice::query()->pluck('stripe_price_id')
->merge(StripePlanPrice::query()->pluck('stripe_price_id'))
->filter()
->flip();
$found = 0;
foreach ($this->products() as $productId) {
foreach ($stripe->activePricesFor($productId) as $price) {
if ($known->has($price['id'])) {
continue;
}
$found++;
$this->line(sprintf(
' orphan %s %d %s %s product %s %s',
$price['id'],
$price['unit_amount'],
$price['currency'],
$price['interval'],
$productId,
$price['metadata'] === [] ? 'no metadata' : json_encode($price['metadata']),
));
if ($archive && ! $dryRun) {
$stripe->archivePrice($price['id']);
}
}
}
$this->newLine();
if ($found === 0) {
$this->info('Every active price at our products is accounted for.');
return self::SUCCESS;
}
// Said on every path that found something, including after --archive
// has already run: the only question asked above is whether a row holds
// the id, so an orphan in this list may be one stripe:sync-catalogue
// would have adopted. Archiving that one is the failure this class's
// docblock describes, and an operator who has just done it needs to
// know as much as one who is about to.
$this->warn(' Run stripe:sync-catalogue first — an orphan listed here may be one the next sync would adopt.');
$this->info(match (true) {
$dryRun && $archive => "{$found} orphan(s) would be archived. Run without --dry-run to archive them.",
$archive => "{$found} orphan(s) archived. Existing subscriptions on them keep billing.",
default => "{$found} orphan(s) found. Once the sync has run, --archive stops selling what is still listed.",
});
return self::SUCCESS;
}
/**
* Every Stripe Product this platform owns: one per plan family, one per
* module. Read from our own rows rather than from Stripe, because a Product
* we do not know is not one whose Prices we can judge.
*
* @return array<int, string>
*/
private function products(): array
{
return PlanFamily::query()->whereNotNull('stripe_product_id')->pluck('stripe_product_id')
->merge(StripeAddonPrice::query()->pluck('stripe_product_id'))
->filter()
->unique()
->values()
->all();
}
}

View File

@ -8,6 +8,8 @@ use App\Models\PlanVersion;
use App\Models\Subscription;
use App\Services\Billing\AddonCatalogue;
use App\Services\Billing\AddonPrices;
use App\Services\Billing\AdoptStripePrice;
use App\Services\Billing\AdoptStripeProduct;
use App\Services\Billing\PlanPrices;
use App\Services\Billing\TaxTreatment;
use App\Services\Stripe\StripeClient;
@ -83,7 +85,27 @@ class SyncStripeCatalogue extends Command
return self::FAILURE;
}
// Resolved once, as singletons: each counts across the WHOLE run, and a
// count taken before ensure()/the adoption call runs can never itself
// distinguish "created" from "adopted" — see AdoptStripePrice.
$adoptPrices = app(AdoptStripePrice::class);
$adoptProducts = app(AdoptStripeProduct::class);
$adoptedPricesBefore = $adoptPrices->adoptions;
// Duplicates accumulate on the SAME singleton across the whole process,
// not only this run — without this, a second handle() in one process
// would print a duplicate a previous run already reported.
$duplicatesBefore = count($adoptProducts->duplicates);
$created = 0;
// Counted here, locally, rather than on AdoptStripeProduct itself: the
// very same class also adopts a MODULE's Product, from
// AddonPrices::product() inside syncModules() below, and a module's
// Product is never counted as an intent at all — unlike a family's,
// which gets its own "product …" line and $created++ right where this
// is incremented. A single counter on the class could not tell the two
// apart; counting only at the one call site that also counts the
// intent avoids the question entirely.
$adoptedFamilyProducts = 0;
foreach (PlanFamily::query()->with('versions.prices')->orderBy('tier')->get() as $family) {
$published = $family->versions->filter(fn (PlanVersion $version) => $version->isPublished());
@ -101,32 +123,32 @@ class SyncStripeCatalogue extends Command
$created++;
if (! $dryRun) {
$productId = $stripe->createProduct(
$metadata = ['plan_family' => $family->key, 'plan_family_id' => (string) $family->id];
// Asked BEFORE minting, for the same reason the Price side
// asks: a run that died between Stripe's create and our
// update left a Product this row does not know, and the key
// below stops protecting it after twenty-four hours.
// plan_family_id is what proves such a Product is this
// family's.
$productId = $adoptProducts($metadata, ['plan_family_id']);
if ($productId !== null) {
// The intent was counted three lines up, so — unlike a
// module's Product — an adoption here may honestly be
// subtracted back out of it below.
$adoptedFamilyProducts++;
}
$productId ??= $stripe->createProduct(
$family->name,
['plan_family' => $family->key, 'plan_family_id' => (string) $family->id],
$metadata,
// Covers a RETRY, for twenty-four hours, and nothing
// after that: Stripe forgets a key at the end of them.
// A crash between Stripe creating the Product and us
// storing its id therefore leaves an orphan, and the
// next run past the expiry makes a second Product for
// this family. There is no recognition step for Products
// — App\Services\Billing\AdoptStripePrice covers Prices
// only — so nothing here ever asks Stripe what Products
// it already has. A KNOWN GAP, not something this key
// closes, and a worse one than a duplicate Price:
// activePricesFor() would then be asked about the new
// Product, and the Price recognition goes blind for the
// whole family.
//
// IdempotencyKey::forProduct() folds the name and the
// metadata into what goes on the wire, so a renamed
// family under an unstored id now mints a second Product
// where it used to answer HTTP 400 for a day. That is
// the trade this branch chose deliberately: a blockade
// reaches a paying customer, a duplicate Product does
// not.
// beyond them. What stops a second Product for one
// family is the adoption step above.
idempotencyKey: "clupilot-product-{$family->id}",
);
$family->update(['stripe_product_id' => $productId]);
}
}
@ -142,22 +164,45 @@ class SyncStripeCatalogue extends Command
$this->newLine();
// $adopted may only subtract what $created actually counted as an
// intent. Every price is one, family or module — syncPrice() and
// syncModules() both increment $created before knowing whether ensure()
// will mint or adopt, so the full price-adoption delta is honest. A
// family's Product is one too, counted above, so $adoptedFamilyProducts
// is honest for the same reason. A module's Product is NOT one — it
// prints no line of its own and increments nothing in syncModules(),
// however AdoptStripeProduct answers it — so its adoption stays out of
// both figures entirely. Subtracting it would report fewer objects
// created than Stripe actually gained.
$adopted = ($adoptPrices->adoptions - $adoptedPricesBefore) + $adoptedFamilyProducts;
$minted = max(0, $created - $adopted);
// Sliced to what THIS run added — the list lives on the same
// process-wide singleton as the count above, so without the same
// before/after care a second handle() in one process would print a
// duplicate a previous run already reported.
foreach (array_slice($adoptProducts->duplicates, $duplicatesBefore) as $duplicate) {
// Named here as well as in the log: an operator running the sweep
// reads this, and a second Product for one family is something only
// a person can resolve — we deliberately do not deactivate it.
$this->warn(" duplicate product {$duplicate} — left active, a product's prices become unsellable if it is deactivated");
}
if ($created === 0) {
$this->info('Stripe is already in step with the catalogue.');
return self::SUCCESS;
}
// "or adopted", because the count is taken BEFORE ensure() runs and
// ensure() may find the Price already at Stripe and adopt it instead of
// creating anything — see App\Services\Billing\AdoptStripePrice. Telling
// which of the two happened is the whole purpose of that step, so the
// first thing an operator reads after an interrupted run must not assert
// a creation that did not take place. The log entry adoption writes names
// the Price it took over.
// "or adopted" on the dry run, and only there. A dry run counts intents
// without ever calling ensure(), so it cannot know which of them Stripe
// already holds a Price for — and a dry run is the first thing an
// operator runs after an interrupted one, which is exactly when some of
// these will be adopted rather than minted. The live line below has
// both figures because by then the adoption step has answered.
$this->info($dryRun
? "{$created} object(s) would be created or adopted. Run without --dry-run to do it."
: "{$created} object(s) created or adopted in Stripe.");
? "{$created} object(s) would be created or adopted. Run without --dry-run to create them."
: "{$minted} object(s) created, {$adopted} adopted in Stripe.");
return self::SUCCESS;
}

View File

@ -13,6 +13,8 @@ use App\Mail\Transport\MailboxTransport;
use App\Models\Customer;
use App\Models\MaintenanceNotification;
use App\Provisioning\PipelineRegistry;
use App\Services\Billing\AdoptStripePrice;
use App\Services\Billing\AdoptStripeProduct;
use App\Services\Billing\CustomDomainAccess;
use App\Services\Dns\FileHostDnsDirectory;
use App\Services\Dns\HetznerDnsClient;
@ -57,6 +59,17 @@ class AppServiceProvider extends ServiceProvider
config('provisioning.pipelines', []),
));
// Both singletons because each carries a record across one run that the
// caller reads only after it finishes: which products were duplicates,
// and how many prices were adopted rather than created. Resolved per
// call they would each start empty on every read. PlanPrices is
// deliberately NOT a singleton — SyncStripeCatalogue asks the container
// for it once per catalogue row — which is exactly why a record like
// this has to live on the adoption classes instead: on a per-call
// instance it could never carry across the row it started on.
$this->app->singleton(AdoptStripeProduct::class);
$this->app->singleton(AdoptStripePrice::class);
// Real I/O implementations; tests swap in fakes via app()->instance().
$this->app->bind(RemoteShell::class, PhpseclibRemoteShell::class);
$this->app->bind(WireguardHub::class, LocalWireguardHub::class);

View File

@ -284,14 +284,19 @@ final class AddonPrices
return $existing;
}
return $this->stripe->createProduct(
$metadata = ['addon' => $addonKey];
// Asked before minting. A run that created this Product and died before
// any row carried its id leaves an orphan — and a second Product would
// make activePricesFor() ask about the wrong one, blinding the Price
// recognition for this module entirely.
$adopted = app(AdoptStripeProduct::class)($metadata, ['addon']);
return $adopted ?? $this->stripe->createProduct(
app(AddonCatalogue::class)->name($addonKey),
['addon' => $addonKey],
// A retry's guard for twenty-four hours and nothing beyond them, the
// same as the plan side's in SyncStripeCatalogue — and the same known
// gap: there is no recognition step for Products, so a run that
// created this one and died before any row carried its id leaves an
// orphan nothing here will ever ask Stripe about.
$metadata,
// A retry's guard for twenty-four hours and nothing beyond them;
// what stops a second Product is the adoption step above.
idempotencyKey: "clupilot-addon-product-{$addonKey}",
);
}

View File

@ -3,6 +3,7 @@
namespace App\Services\Billing;
use App\Services\Stripe\StripeClient;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Log;
/**
@ -41,6 +42,16 @@ use Illuminate\Support\Facades\Log;
*/
final class AdoptStripePrice
{
/**
* How many Prices this run took over rather than minted.
*
* Read by stripe:sync-catalogue, which counts an intent BEFORE calling
* ensure() and so cannot tell the two apart on its own comparing the Price
* id before and after does not distinguish them either, because there is no
* id before in either case.
*/
public int $adoptions = 0;
public function __construct(private readonly StripeClient $stripe) {}
/**
@ -68,9 +79,22 @@ final class AdoptStripePrice
$candidates = [];
foreach ($this->stripe->activePricesFor($productId) as $price) {
// The money gate, and the whole of it — this class promises that no
// adoption can move money, and until these four moved here it could
// compare only the first three. A Price at our figure with
// interval_count 3 bills quarterly; one with transform_quantity
// divide_by 10 charges a customer holding three packs for one; a
// metered or tiered one does not charge an amount at all. Stripe's
// dashboard copies metadata when it duplicates a Price, so such a
// Price can carry our own identifying key and be indistinguishable
// from ours by every other test here.
if ($price['unit_amount'] !== $amountCents
|| $price['currency'] !== strtoupper($currency)
|| $price['interval'] !== $interval) {
|| $price['interval'] !== $interval
|| $price['interval_count'] !== 1
|| $price['usage_type'] !== 'licensed'
|| $price['transform_quantity']
|| $price['billing_scheme'] !== 'per_unit') {
continue;
}
@ -86,6 +110,14 @@ final class AdoptStripePrice
}
if (! $this->confirms($price['metadata'], $metadata, $identifying)) {
// Once a day per Price. An orphan nobody can adopt is a STATE,
// not an event, and this runs in a customer's module booking as
// well as in the sweep — without a throttle it writes a line
// every time anybody books anything, for as long as the orphan
// exists, and the clean-up command that would end it is a
// separate one. Cache::add is this repo's throttle; see
// App\Livewire\Billing.
if (Cache::add('stripe:unexplained-price:'.$price['id'], true, now()->addDay())) {
Log::warning('stripe: left an unexplained active price alone rather than adopting it', [
'price' => $price['id'],
'product' => $productId,
@ -93,6 +125,7 @@ final class AdoptStripePrice
'currency' => strtoupper($currency),
'interval' => $interval,
]);
}
continue;
}
@ -104,7 +137,9 @@ final class AdoptStripePrice
return null;
}
usort($candidates, fn (array $a, array $b) => $a['created'] <=> $b['created']);
// Oldest first, and on a tie the lower id — so the same two orphans give
// the same answer on every run, whatever order Stripe lists them in.
usort($candidates, fn (array $a, array $b) => [$a['created'], $a['id']] <=> [$b['created'], $b['id']]);
$adopted = array_shift($candidates);
@ -145,6 +180,8 @@ final class AdoptStripePrice
'interval' => $interval,
]);
$this->adoptions++;
return $adopted['id'];
}

View File

@ -0,0 +1,137 @@
<?php
namespace App\Services\Billing;
use App\Services\Stripe\StripeClient;
use Illuminate\Support\Facades\Log;
/**
* The Stripe Product we were about to create and Stripe already has.
*
* The sibling of AdoptStripePrice, one level up, and the gap that made that one
* fragile. A run that creates a Product and dies before storing its id leaves an
* orphan; the next run past the idempotency key's twenty-four hours makes a
* second Product and from then on activePricesFor() is asked about the new
* one, so the Price recognition goes blind for the whole family and every
* orphaned Price on the first Product is unreachable.
*
* **No money gate.** A Product carries no amount, no interval and no recurrence,
* so there is nothing here that could move money and nothing to compare but the
* metadata. That is the whole of the proof: a Product must CONFIRM an
* identifying key of ours and contradict none it carries.
*
* **A duplicate is reported, never deactivated.** This is the one place the
* Price side's reasoning does not carry over. Archiving a Price is provably
* harmless Stripe goes on billing every subscription already on it, which is
* what the grandfathering rule has always rested on. Deactivating a Product
* makes its PRICES unsellable, and contracts can be running on those. What was
* proven for one is not proven for the other, so it is not done.
*
* **Silent about strangers.** Unlike the Price side, which asks only about our
* own Product and warns when something unexplained sits there, this asks for
* every active Product on the account. An operator selling something else
* through the same account is an ordinary state, not a finding, and warning
* about it would fire on every run for ever.
*/
final class AdoptStripeProduct
{
/**
* Products that matched but were not taken, oldest-first order having
* decided against them. Read by stripe:sync-catalogue for its report a
* log line alone is not seen by the operator running the sweep.
*
* @var array<int, string>
*/
public array $duplicates = [];
public function __construct(private readonly StripeClient $stripe) {}
/**
* The id of an existing Stripe Product to use instead of creating one.
*
* @param array<string, string> $metadata what the create call would send
* @param array<int, string> $identifying metadata keys that mark a Product as ours
*/
public function __invoke(array $metadata, array $identifying): ?string
{
$metadata = array_map(fn ($value) => (string) $value, $metadata);
$candidates = [];
foreach ($this->stripe->activeProducts() as $product) {
if ($this->contradicts($product['metadata'], $metadata)) {
continue;
}
if (! $this->confirms($product['metadata'], $metadata, $identifying)) {
continue;
}
$candidates[] = $product;
}
if ($candidates === []) {
return null;
}
// Oldest first, and on a tie the lower id, so two runs agree. The tie is
// not merely theoretical: createProduct()'s counter and plantProduct()'s
// default both start at 1, so a minted Product and a planted orphan can
// share a `created` — the same collision AdoptStripePrice's usort breaks
// the same way, documented at FakeStripeClient::createPrice().
usort($candidates, fn (array $a, array $b) => [$a['created'], $a['id']] <=> [$b['created'], $b['id']]);
$adopted = array_shift($candidates);
foreach ($candidates as $duplicate) {
$this->duplicates[] = $duplicate['id'];
Log::warning('stripe: a second product claims to be the same thing — left alone, not deactivated', [
'product' => $duplicate['id'],
'adopted' => $adopted['id'],
'metadata' => $metadata,
]);
}
Log::info('stripe: adopted an existing product instead of creating a second one', [
'product' => $adopted['id'],
'metadata' => $metadata,
]);
return $adopted['id'];
}
/**
* @param array<string, string> $found
* @param array<string, string> $expected
*/
private function contradicts(array $found, array $expected): bool
{
foreach ($expected as $key => $value) {
if (array_key_exists($key, $found) && $found[$key] !== $value) {
return true;
}
}
return false;
}
/**
* Failing to contradict is not enough an empty metadata bag contradicts
* nothing, and this list holds every Product on the account.
*
* @param array<string, string> $found
* @param array<string, string> $expected
* @param array<int, string> $identifying
*/
private function confirms(array $found, array $expected, array $identifying): bool
{
foreach ($identifying as $key) {
if (isset($found[$key]) && $found[$key] === ($expected[$key] ?? null)) {
return true;
}
}
return false;
}
}

View File

@ -16,10 +16,10 @@ class FakeStripeClient implements StripeClient
{
public bool $configured = true;
/** @var array<int, array{name: string, metadata: array}> */
/** @var array<int, array{name: string, metadata: array, created: int}> */
public array $products = [];
/** @var array<int, array{product: string, amount: int, currency: string, interval: string, metadata: array, created: int}> */
/** @var array<int, array{product: string, amount: int, currency: string, interval: string, interval_count: int, usage_type: string, transform_quantity: bool, billing_scheme: string, metadata: array, created: int}> */
public array $prices = [];
/** @var array<int, string> */
@ -184,7 +184,14 @@ class FakeStripeClient implements StripeClient
}
$id = 'prod_'.substr(sha1($name.count($this->products)), 0, 12);
$this->products[$id] = ['name' => $name, 'metadata' => $metadata];
$this->products[$id] = [
'name' => $name,
'metadata' => $metadata,
// Stripe stamps a creation time and AdoptStripeProduct takes the
// OLDEST of several. A counter cannot drift the way a clock in a
// test can.
'created' => count($this->products) + 1,
];
$this->rememberKey($key, $id, $parameters);
@ -218,15 +225,24 @@ class FakeStripeClient implements StripeClient
'amount' => $amountCents,
'currency' => $currency,
'interval' => $interval,
// Stripe's defaults, and the only thing createPrice() can produce:
// it sends none of these four, so every Price this platform mints
// carries exactly them.
'interval_count' => 1,
'usage_type' => 'licensed',
'transform_quantity' => false,
'billing_scheme' => 'per_unit',
'metadata' => $metadata,
// Stripe stamps every object with its creation time and
// AdoptStripePrice takes the OLDEST of several orphans. Counted
// rather than clocked, because two Prices minted in the same second
// would share a timestamp — but the two counters are independent:
// plantPrice() stamps `created: 1` of its own accord, which ties with
// a Price minted here while this list was still empty. Any test whose
// outcome depends on which of two prices is older passes `created:`
// itself, as the adoption tests do.
// a Price minted here while this list was still empty. A tie is
// possible, not merely theoretical — which is why AdoptStripePrice's
// usort breaks one by price id rather than leaving it to Stripe's
// list order, and a test can plant two prices at the same `created`
// on purpose to prove it.
'created' => count($this->prices) + 1,
];
@ -258,15 +274,6 @@ class FakeStripeClient implements StripeClient
// No failIfAsked(): createPrice(), the call this one stands in front of,
// does not fail either, and a test that scripts an outage for a refund
// must not have its catalogue sync change behaviour underneath it.
//
// None of HttpStripeClient's charging-property filter either — the four
// fields it names (interval_count, usage_type, transform_quantity,
// billing_scheme). This fake only ever holds prices its own
// createPrice()/plantPrice() put here, and neither takes an argument for
// any of them: there is nothing non-standard a test could plant, so the
// filter would have nothing to do. Not an oversight. What that filter
// does is proven where it lives, over Http::fake — see
// tests/Feature/Billing/StripeIdempotencyKeyTest.php.
$found = [];
foreach ($this->prices as $id => $price) {
@ -279,18 +286,57 @@ class FakeStripeClient implements StripeClient
'unit_amount' => $price['amount'],
'currency' => strtoupper($price['currency']),
'interval' => $price['interval'],
'interval_count' => $price['interval_count'],
'usage_type' => $price['usage_type'],
'transform_quantity' => $price['transform_quantity'],
'billing_scheme' => $price['billing_scheme'],
'created' => $price['created'],
'metadata' => $price['metadata'],
// Cast like the wire does: Stripe hands metadata back as
// strings, and an un-cast int here would let a test pass where
// production's strict === fails.
'metadata' => array_map(fn ($value) => (string) $value, $price['metadata']),
];
}
return $found;
}
public function activeProducts(): array
{
$found = [];
foreach ($this->products as $id => $product) {
$found[] = [
'id' => $id,
'name' => $product['name'],
'created' => $product['created'],
'metadata' => array_map(fn ($value) => (string) $value, $product['metadata']),
];
}
return $found;
}
/**
* A Product that exists at Stripe and in no row of ours the orphan a run
* leaves when it dies between Stripe's create and our storing its id.
*/
public function plantProduct(string $id, string $name, array $metadata = [], int $created = 1): void
{
$this->products[$id] = ['name' => $name, 'metadata' => $metadata, 'created' => $created];
}
public function updatePriceMetadata(string $priceId, array $metadata): void
{
if (isset($this->prices[$priceId])) {
$this->prices[$priceId]['metadata'] = $metadata;
// Merged, not replaced — Stripe leaves a key you do not send where
// it is. A fake that replaced would let a test prove the opposite of
// production, and "write only when it differs" in AdoptStripePrice
// is built around exactly this merging.
$this->prices[$priceId]['metadata'] = [
...$this->prices[$priceId]['metadata'],
...$metadata,
];
}
$this->metadataUpdates[] = ['price' => $priceId, 'metadata' => $metadata];
@ -312,12 +358,23 @@ class FakeStripeClient implements StripeClient
string $interval,
array $metadata = [],
int $created = 1,
int $intervalCount = 1,
string $usageType = 'licensed',
bool $transformQuantity = false,
string $billingScheme = 'per_unit',
): void {
$this->prices[$id] = [
'product' => $productId,
'amount' => $amountCents,
'currency' => $currency,
'interval' => $interval,
// Defaulted to what we could have minted ourselves, so a test that
// does not care says nothing — and a test about the money gate says
// exactly which property it is about.
'interval_count' => $intervalCount,
'usage_type' => $usageType,
'transform_quantity' => $transformQuantity,
'billing_scheme' => $billingScheme,
'metadata' => $metadata,
'created' => $created,
];

View File

@ -170,43 +170,24 @@ class HttpStripeClient implements StripeClient
foreach ($data as $price) {
$recurring = (array) ($price['recurring'] ?? []);
// Only prices we could have minted ourselves. createPrice() sends
// product, unit_amount, currency, recurring[interval] and metadata
// and nothing else, so Stripe defaults every other field that
// decides what a Price CHARGES — and anything but those defaults
// is a Price this platform did not write. Skipped here rather than
// returned, because AdoptStripePrice compares amount, currency and
// interval only and trusts that triple as the whole of what a
// Price charges:
//
// - interval_count multiplies the period — 3 bills every three
// months — while `interval` still reads 'month';
// - usage_type other than 'licensed' bills metered usage;
// - a transform_quantity divides the quantity before charging,
// and modules are billed BY quantity — SyncStripeAddonItems
// sums a pack into one item at quantity n — so divide_by 10
// would charge a customer holding three for one;
// - billing_scheme 'tiered' keeps the money in tiers, leaving
// unit_amount null. Read as 0 below, so such a Price fails the
// amount match only while the caller's own figure is not 0 —
// and PlanPrices::ensure() has no zero guard, so that is not a
// condition to rest on.
//
// An ABSENT key is Stripe's default and no reason to reject.
// Read the other way round this filter would refuse every
// legitimate price and turn recognition into a permanent no-op.
if ((int) ($recurring['interval_count'] ?? 1) !== 1
|| ($recurring['usage_type'] ?? 'licensed') !== 'licensed'
|| (array) ($price['transform_quantity'] ?? []) !== []
|| ($price['billing_scheme'] ?? 'per_unit') !== 'per_unit') {
continue;
}
$prices[] = [
'id' => (string) ($price['id'] ?? ''),
'unit_amount' => (int) ($price['unit_amount'] ?? 0),
'currency' => strtoupper((string) ($price['currency'] ?? '')),
'interval' => (string) ($recurring['interval'] ?? ''),
// The four properties that decide what a Price CHARGES
// beyond its amount. createPrice() sends none of them, so
// Stripe defaults every one — and an ABSENT key is that
// default, never a reason to reject. Reported rather than
// filtered: the class that promises adoption cannot move
// money is AdoptStripePrice, and it could not keep that
// promise while these lived here, where FakeStripeClient
// cannot express them and no test of that class could reach
// them.
'interval_count' => (int) ($recurring['interval_count'] ?? 1),
'usage_type' => (string) ($recurring['usage_type'] ?? 'licensed'),
'transform_quantity' => (array) ($price['transform_quantity'] ?? []) !== [],
'billing_scheme' => (string) ($price['billing_scheme'] ?? 'per_unit'),
'created' => (int) ($price['created'] ?? 0),
'metadata' => array_map(
fn ($value) => (string) $value,
@ -216,11 +197,66 @@ class HttpStripeClient implements StripeClient
}
$after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? null);
} while (($page['has_more'] ?? false) === true && $after !== null);
// invoiceLines() stops here, and there that costs a short document.
// Here it costs an unseen orphan and a second live Price for one
// figure — the incident. So it is an error, not a stopping point.
if (($page['has_more'] ?? false) === true && $after === null) {
throw new RuntimeException(
'Stripe reported another page of prices and the last item of this one carried no id; '
.'refusing to stop half-read.'
);
}
} while (($page['has_more'] ?? false) === true);
return $prices;
}
public function activeProducts(): array
{
$products = [];
$after = null;
do {
$page = $this->request()
->get($this->url('products'), array_filter([
'active' => 'true',
'limit' => 100,
'starting_after' => $after,
], fn ($value) => $value !== null))
->throw()
->json();
$data = (array) ($page['data'] ?? []);
foreach ($data as $product) {
$products[] = [
'id' => (string) ($product['id'] ?? ''),
'name' => (string) ($product['name'] ?? ''),
'created' => (int) ($product['created'] ?? 0),
'metadata' => array_map(
fn ($value) => (string) $value,
(array) ($product['metadata'] ?? []),
),
];
}
$after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? null);
// Same reason as activePricesFor(): a half-read list here means a
// Product we own goes unrecognised, and the next run mints a second
// one — which blinds the Price recognition for the whole family.
if (($page['has_more'] ?? false) === true && $after === null) {
throw new RuntimeException(
'Stripe reported another page of products and the last item of this one carried no id; '
.'refusing to stop half-read.'
);
}
} while (($page['has_more'] ?? false) === true);
return $products;
}
public function updatePriceMetadata(string $priceId, array $metadata): void
{
$this->request()

View File

@ -125,42 +125,50 @@ interface StripeClient
): string;
/**
* Every active recurring Price of a Product, as Stripe holds them.
* Every active recurring Price of a Product, as Stripe holds them, with the
* properties that decide what each one CHARGES.
*
* The half of the catalogue mirror that was missing: nothing here ever asked
* Stripe what it already had. A run that created a Price and died before the
* row was written left an orphan our table never learned about, and once the
* idempotency key expired the next run made a SECOND live Price for the same
* money the exact duplicate the key exists to prevent. See
* App\Services\Billing\AdoptStripePrice, which is the only caller.
* `interval_count` multiplies the period 3 bills every three months while
* `interval` still reads 'month'. `usage_type` other than 'licensed' bills
* metered usage. `transform_quantity` divides the quantity before charging,
* and modules are billed BY quantity, so a divide_by of 10 would charge a
* customer holding three for one. `billing_scheme` 'tiered' keeps the money
* in tiers and leaves `unit_amount` null, which reads as 0 here.
*
* All four are REPORTED, not filtered. Which of them is acceptable is the
* caller's decision, and the caller is App\Services\Billing\AdoptStripePrice
* the class whose docblock promises that adoption cannot move money. That
* promise has to be testable in the class that makes it, and it was not
* while these fields were dropped here: FakeStripeClient cannot express
* them.
*
* An absent key is Stripe's own default — 1, 'licensed', none, 'per_unit'.
*
* Active ones only, because the question being asked is "is there something
* here I can sell on?". `currency` comes back UPPER CASE, the way our own
* tables hold it.
*
* FOUR properties are checked, and they are named because the list is what
* the contract is: `recurring.interval_count` is 1, `recurring.usage_type`
* is `licensed`, there is no `transform_quantity`, and `billing_scheme` is
* `per_unit`. Those are Stripe's own defaults, and createPrice() sets none of
* the four so anything else is a Price this platform did not write, and is
* filtered out here rather than returned.
*
* The reason it is filtered at all: AdoptStripePrice compares the amount,
* currency and interval of the shape below and trusts that triple as the
* WHOLE of what a Price charges. A Price billed every three months, on
* metered usage, dividing the quantity before charging, or pricing in tiers
* would otherwise pass that triple on `interval => 'month'` alone and be
* adopted as if it charged our figure per unit per month which is adoption
* moving money, the one thing nothing here may do.
*
* Not a list of everything Stripe can put on a Price: it is the list of
* fields known to change what one CHARGES. A fifth would have to be added
* here, at the boundary, before the shape below is handed on.
*
* @return array<int, array{id: string, unit_amount: int, currency: string, interval: string, created: int, metadata: array<string, string>}>
* @return array<int, array{id: string, unit_amount: int, currency: string, interval: string, interval_count: int, usage_type: string, transform_quantity: bool, billing_scheme: string, created: int, metadata: array<string, string>}>
*/
public function activePricesFor(string $productId): array;
/**
* Every active Product on the account, with its metadata.
*
* The other half of the orphan question, and the more consequential one. A
* run that created a Product and died before storing its id leaves an orphan
* exactly as a Price does but a second Product is worse than a duplicate
* Price, because activePricesFor() is then asked about the wrong one and the
* Price recognition goes blind for that whole family.
*
* The WHOLE active list, with no metadata filter: Stripe cannot search
* metadata, and an account holds a handful of Products. Which of them is
* ours is App\Services\Billing\AdoptStripeProduct's decision.
*
* @return array<int, array{id: string, name: string, created: int, metadata: array<string, string>}>
*/
public function activeProducts(): array;
/**
* Write metadata onto a Price that already exists.
*

View File

@ -223,6 +223,23 @@ Kette an einer anderen Stelle, und mehrere brechen sie **nach** der Zahlung.
`stripe:reprice-subscriptions` (je zuerst `--dry-run`). Der Abgleich legt jetzt
**zwei** Preise je Paket an; bis er lief, kann ein geprüfter EU-Firmenkunde nicht
bestellen — das ist Absicht.
- `stripe:sweep-orphan-prices` listet jeden **aktiven wiederkehrenden**
Stripe-Preis an unseren Produkten, den **keine** Zeile in
`stripe_addon_prices` oder `stripe_plan_prices` kennt — mit ID, Betrag,
Währung, Intervall und Metadaten. Einmalige Preise sieht es nicht; von uns
angelegt ist nie einer davon, `createPrice()` schickt immer
`recurring[interval]`.
Ohne Argument wird nichts angefasst; mit `--archive` werden die aufgelisteten
Preise bei Stripe stillgelegt, also **nicht mehr verkauft** (laufende Abos
darauf verrechnet Stripe weiter). **Zuerst `stripe:sync-catalogue` laufen
lassen:** das Kommando prüft nur, ob eine Zeile die ID kennt, nicht ob der
Wiedererkennungsschritt die Waise übernommen hätte. Eine übernehmbare Waise
vorher stillzulegen macht sie für den Abgleich unsichtbar (er fragt Stripe nur
nach aktiven Preisen) — er fällt dann auf `createPrice()` mit demselben
Idempotenz-Schlüssel zurück, den der abgebrochene Lauf benutzt hat, und Stripe
antwortet 24 Stunden lang mit der gespeicherten Antwort: der ID des
stillgelegten Preises. Danach steht im Register ein Preis, auf den Stripe
keinen Checkout mehr öffnet.
---

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,279 @@
# Spec — Die offenen Punkte der Preis-Wiedererkennung
**Datum:** 2026-07-30
**Status:** entworfen, noch nicht umgesetzt
**Vorgänger:** `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md`, §14
---
## 1. Ausgangslage
Die Preis-Wiedererkennung ist gebaut und gemergt: ein Lauf, der zwischen Stripes
Anlage und unserem Schreiben abbricht, hinterlässt keine zweite Preis-Zeile
mehr, und eine Änderung an den Metadaten blockiert nichts mehr für 24 Stunden.
§14 des Vorgängers hat aufgeschrieben, was die Prüfungen dabei fanden und
niemand behoben hat. Das wird hier gebaut. Drei Punkte mit Entwurfsentscheidung,
sechs mechanische.
## 2. Waisen-**Produkte** — die eine echte Lücke
`activePricesFor()` erkennt einen verwaisten *Preis* wieder. Für ein verwaistes
**Produkt** gibt es kein Gegenstück, und dort ist der Schaden größer: ein
zweites Produkt lässt jedes `activePricesFor($neueId)` leer zurückkommen. Damit
ist die Wiedererkennung für dieses Modul oder diese Familie **dauerhaft blind**
und jede Waise am ersten Produkt unerreichbar. Der Zweig deaktiviert seine eigene
Sicherung durch die Lücke, die er nicht geschlossen hat.
Betroffen sind zwei Stellen, beide mit demselben Muster — Produkt anlegen, ID in
einem **zweiten** Schreibvorgang speichern:
- `SyncStripeCatalogue::handle()` für Paketfamilien (`$family->update([…])`),
- `AddonPrices::product()` für Module (die ID landet erst über `remember()` in
einer Preiszeile).
### Wie es gebaut wird
`StripeClient::activeProducts(): array` — alle aktiven Produkte des Kontos, nach
dem Blätter-Muster von `activePricesFor()`. **Ohne** Filter auf Metadaten: Stripe
kann danach nicht suchen, und ein Konto hat eine Handvoll Produkte, keine
Tausende.
`App\Services\Billing\AdoptStripeProduct` — der Wiedererkennungsschritt, nach
dem Vorbild von `AdoptStripePrice`, aber deutlich kürzer. Ein Produkt hat **keinen
Betrag**, also gibt es hier **kein Geld-Tor**: der Beweis liegt allein in den
Metadaten. Übernommen wird ein Produkt, das
1. bei Stripe **aktiv** ist,
2. **mindestens einen tragenden Metadatenschlüssel bestätigt**
`plan_family_id` bei Familien, `addon` bei Modulen — und
3. **keinem** unserer Metadatenschlüssel widerspricht, den es trägt.
Eine Beanspruchungs-Prüfung wie bei den Preisen braucht es nicht: die
tragenden Schlüssel sind je Familie und je Modul eindeutig, ein Produkt kann
also gar nicht zu zweien gehören.
### Doppelte Produkte werden gemeldet, nicht stillgelegt
Entscheidung des Auftraggebers vom 2026-07-30, und sie ist die vorsichtige:
**das älteste wird übernommen, die übrigen werden gemeldet und nicht angefasst.**
Der Grund ist ein echter Unterschied zur Preis-Seite. Ein archivierter *Preis*
ist nachweislich harmlos — Stripe verrechnet laufende Abos weiter, nur verkauft
wird er nicht mehr, und darauf beruht die Grandfathering-Regel dieses Projekts
schon lange. Ein stillgelegtes *Produkt* macht dagegen **seine Preise
unverkäuflich**, und auf denen können Verträge laufen. Was bei Preisen bewiesen
harmlos war, ist hier nicht bewiesen — also wird es nicht getan.
Gemeldet wird über `Log::warning` **und** in der Ausgabe von
`stripe:sync-catalogue`, weil die Produkt-Anlage im Gegensatz zu `ensure()` nur
im Kommando läuft und ein Betreiber dort hinsieht.
## 3. Das Geld-Tor zieht dorthin, wo es zugesagt wird
`AdoptStripePrice` verspricht im Kopfkommentar, keine Übernahme könne Geld
verschieben — vergleicht aber nur Betrag, Währung und Intervall. Die vier
übrigen Eigenschaften, die entscheiden, was ein Preis verrechnet
(`interval_count`, `usage_type`, `transform_quantity`, `billing_scheme`),
filtert `HttpStripeClient::activePricesFor()`.
Zwei Folgen, und die zweite ist die schlimmere:
- der `FakeStripeClient` kann diese Felder gar nicht ausdrücken, also kann
**kein Test von `AdoptStripePrice` das Geld-Tor erreichen** — genau die
Zusage, um die es geht;
- eine dritte Implementierung von `StripeClient` ließe die Prüfung still
fallen.
### Wie es gebaut wird
`activePricesFor()` **filtert nicht mehr**, sondern gibt die vier Eigenschaften
im Rückgabe-Array mit: `interval_count`, `usage_type`, `transform_quantity`
(als `bool`: trägt der Preis eine?), `billing_scheme`. Absente Schlüssel werden
weiterhin auf Stripes Vorgaben gesetzt — `1`, `licensed`, `false`, `per_unit`
und das bleibt die Richtung, auf die es ankommt: andersherum gelesen würde der
Filter jeden legitimen Preis ablehnen und die Wiedererkennung still zu einem
Nichts machen.
`AdoptStripePrice` prüft sie neben Betrag, Währung und Intervall. Der
`FakeStripeClient` bekommt die vier Felder in `plantPrice()`, damit ein Test
einen vierteljährlichen oder mengenteilenden Preis überhaupt hinstellen kann.
`StripeClient::activePricesFor()`s Vertrag wird ehrlich: er liefert aktive,
wiederkehrende Preise **mit** ihren preisbestimmenden Eigenschaften; was davon
zulässig ist, entscheidet der Aufrufer.
### Die Null-Schranke in `PlanPrices` wird **nicht** gebaut
Der Abschluss-Review hatte sie vorgeschlagen, und §14 des Vorgängers hat sie
notiert: `AddonPrices::ensure()` hat eine `<= 0`-Schranke, `PlanPrices::ensure()`
nicht, und das sei das Einzige, was einen `billing_scheme: tiered`-Preis (bei dem
Stripe `unit_amount: null` liefert, hier als 0 gelesen) auf der Paketseite
erreichbar macht.
Das stimmte — **bis zu diesem Umzug**. Sobald `AdoptStripePrice` selbst
`billing_scheme !== 'per_unit'` ablehnt, ist ein gestufter Preis unabhängig vom
Betrag des Aufrufers ausgeschlossen. Die Schranke hätte dann keinen Zweck mehr,
wäre aber eine Verhaltensänderung: ein Paket zu null — ein kostenloser Tarif,
heute nicht im Katalog, aber jederzeit anlegbar — bekäme keinen Stripe-Preis
mehr und wäre nicht verkäuflich. Eine Schranke ohne Nutzen, die einen künftigen
Fall bricht, wird nicht eingebaut.
Der §14-Eintrag wird entsprechend abgehakt: nicht gebaut, und warum.
## 4. `stripe:sweep-orphan-prices` — das Aufräum-Kommando
Der Vorgänger hat es bewusst ausgelassen (§12) und dafür die Log-Zeile
begründet. Die Log-Zeile allein reicht nicht: sie nennt die Waise, aber niemand
kann etwas mit ihr tun, ohne von Hand in Stripes Oberfläche zu gehen.
- **Ohne Argument:** ein Bericht. Für jedes unserer Produkte werden die aktiven
**wiederkehrenden** Preise geholt und die aufgelistet, die **keine Zeile** in
`stripe_addon_prices` oder `stripe_plan_prices` kennt — mit ID, Betrag,
Währung, Intervall und Metadaten. Nichts wird angefasst.
- **Mit `--archive`:** dieselbe Liste wird stillgelegt. Für alles, was **dieses
Projekt** verkauft, ist das harmlos, und zwar aus demselben Grund wie überall
sonst hier: ein archivierter Preis verrechnet laufende Abos weiter, er wird
nur nicht mehr verkauft. Für eine **übernehmbare** Waise gilt das ausdrücklich
**nicht** — siehe die Berichtigung unten.
- **`--dry-run`** wie bei `stripe:sync-catalogue`, damit die beiden Kommandos
sich gleich anfühlen.
**Berichtigt am 2026-07-30.** Hier stand: „Eine Waise, die der
Wiedererkennungsschritt hätte übernehmen können, taucht hier gar nicht erst auf
— sie wurde beim letzten Abgleich übernommen." Das stimmt nur, **wenn seit dem
Auftauchen der Waise ein Abgleich gelaufen ist**, und genau diese Vorbedingung
erzwingt das Kommando nicht. Es fragt ausschließlich, ob eine Zeile die ID
kennt; es fragt `AdoptStripePrice` nicht, ob die Waise übernehmbar wäre. Die
Liste ist deshalb **weiter** als das, was §12 „nicht übernehmbar" nannte, und
die Implementierung hat den falschen Satz getreu nachgebaut.
Die Vorbedingung lautet: **zuerst `stripe:sync-catalogue`, dann dieses
Kommando.** Sie steht im Klassenkommentar, in `$description`, in der
Berichtsausgabe und im Betriebsteil des Handoffs — nicht als Ordnungsliebe,
sondern weil `--archive` auf eine übernehmbare Waise sie für den Abgleich
unsichtbar macht (`activePricesFor()` fragt nur aktive Preise ab). Der Abgleich
fällt dann auf `createPrice()` mit demselben Idempotenz-Schlüssel zurück, den
der abgebrochene Lauf benutzt hat, und Stripe antwortet 24 Stunden lang mit der
gespeicherten Antwort — der ID des stillgelegten Preises, im Zustand von
damals. Das Register hält den Tarif dann für live auf einem Preis, auf den
Stripe keinen Checkout öffnet.
## 5. Die Warnung wird gedrosselt
`AdoptStripePrice` meldet einen unerklärlichen aktiven Preis mit
`Log::warning` — bei **jedem** Abgleich und **jeder** Kundenbuchung, solange die
Waise existiert. Der Widerspruchs-Pfad wurde aus genau diesem Grund still
gestellt; der Bestätigungs-Pfad bekam dieselbe Überlegung nicht.
Höchstens einmal pro Preis-ID und Tag, über `Cache::add()` — dasselbe Muster wie
`App\Livewire\Billing:377` und `App\Livewire\Admin\Vpn:329`. Die Drosselung gilt
**nur** dieser wiederkehrenden Warnung, nicht der Meldung über ein doppeltes
Produkt und nicht der über einen archivierten Doppel-Preis: die stehen für ein
Ereignis, nicht für einen Zustand.
## 6. Die sechs mechanischen Punkte
Ohne Entwurfsentscheidung — bauen, Test, weiter (R22.5):
1. `FakeStripeClient::updatePriceMetadata()` **ersetzt**, wo Stripe
**zusammenführt**. Bestehende Schlüssel bleiben künftig stehen.
2. `FakeStripeClient::activePricesFor()` gibt Metadaten ungecastet zurück, wo
`HttpStripeClient` auf String castet. Angleichen.
3. `created` im Fake kann gleichstehen: `createPrice()` stempelt
`count($prices) + 1`, `plantPrice()` hat die Vorgabe `1`. Gleichstand wird
über die Preis-ID aufgelöst, damit die Wahl über Seitengrenzen hinweg
reproduzierbar ist.
4. `HttpStripeClient::activePricesFor()` bricht das Blättern **still** ab, wenn
dem letzten Eintrag einer nicht-letzten Seite die `id` fehlt. Künftig wirft
es. Begründung im Kommentar: bei `invoiceLines()` entsteht dadurch ein zu
kurzer Belegtext, hier eine ungesehene Waise und ein zweiter aktiver Preis —
also der Vorfall selbst.
5. `stripe:sync-catalogue` zählt „angelegt" und „übernommen" **getrennt** statt
die Wortwahl zu entschärfen.
Der naheliegende Weg funktioniert nicht: die Preis-ID vor und nach dem
Aufruf zu vergleichen unterscheidet die beiden Fälle *nicht* — vorher gibt es
in beiden keine, nachher in beiden eine. Stattdessen zählt
`AdoptStripePrice` seine eigenen Übernahmen mit, und das Kommando liest den
Zähler am Anfang und am Ende. Dafür wird die Klasse in `AppServiceProvider`
als **Singleton** gebunden: sie wird heute je Aufruf frisch aufgelöst
(`SyncStripeCatalogue` holt `PlanPrices` pro Zeile neu), ein Zähler auf der
Instanz käme also nie über eins hinaus. Sie hat sonst keinen Zustand, und
der Zähler ist das, was den Bericht wahr macht.
6. ~~`PlanPrices::ensure()` bekommt die `<= 0`-Schranke~~**entfällt**, siehe
§3: der Umzug des Geld-Tors nimmt ihr den Zweck, und sie würde einen
kostenlosen Tarif unverkäuflich machen.
## 7. Was nicht dazugehört
- **Der sprunghafte `PlanCatalogueTest`.** Ursachensuche, keine Bauarbeit, und
sie braucht ein anderes Vorgehen als dieser Plan.
- **`CLUPILOT_DNS_ZONE` in `phpunit.xml` festnageln.** Fremder Fehler,
inzwischen anders gelöst — die zweite Session hat die Zone im Test selbst
festgenagelt (`a842512`).
- **Alles, was laufende Verträge bewegt.** Wie im Vorgänger: dieser Entwurf
bewegt keinen einzigen.
## 8. Tests
`tests/Feature/Billing/StripeProductAdoptionTest.php` (neu)
| Prüfung | Deckt ab |
|---|---|
| Verwaistes Produkt bei Stripe → wird übernommen, `createProduct` **nicht** aufgerufen (Familie **und** Modul) | §2 |
| Produkt ohne unsere Metadaten → **nicht** übernommen, neues angelegt, Warnung | §2, Bedingung 2 |
| Produkt, das einem Metadatenschlüssel widerspricht → nicht übernommen | §2, Bedingung 3 |
| Zwei passende Produkte → ältestes übernommen, das andere **gemeldet und nicht stillgelegt** | §2 |
An `StripePriceAdoptionTest.php` angehängt:
| Prüfung | Deckt ab |
|---|---|
| Ein Preis mit `interval_count: 3` wird **von `AdoptStripePrice`** abgelehnt — über den Fake, nicht über `Http::fake` | §3 |
| Dasselbe für `transform_quantity`, `usage_type: metered`, `billing_scheme: tiered` | §3 |
| Zweite Warnung zur selben Preis-ID innerhalb eines Tages unterbleibt | §5 |
| Gleichstand bei `created` → die kleinere Preis-ID gewinnt, zweimal gelaufen dasselbe Ergebnis | §6.3 |
| Ein Lauf, der eine Waise übernimmt, meldet „übernommen" und **nicht** „angelegt" | §6.5 |
`tests/Feature/Billing/SweepOrphanPricesTest.php` (neu)
| Prüfung | Deckt ab |
|---|---|
| Bericht nennt die Waise und lässt sie in Ruhe | §4 |
| `--archive` legt sie still, eine Zeile mit Preis bleibt unberührt | §4 |
| `--dry-run` mit `--archive` legt nichts still | §4 |
Und an `StripeIdempotencyKeyTest.php`: Blättern **wirft** bei fehlender `id`
(§6.4); der Fake führt Metadaten zusammen statt sie zu ersetzen (§6.1).
## 9. Berührte Dateien
**Neu**
- `app/Services/Billing/AdoptStripeProduct.php`
- `app/Console/Commands/SweepOrphanStripePrices.php`
- `tests/Feature/Billing/StripeProductAdoptionTest.php`
- `tests/Feature/Billing/SweepOrphanPricesTest.php`
**Geändert**
- `app/Services/Stripe/StripeClient.php``activeProducts()`, Vertrag von
`activePricesFor()`
- `app/Services/Stripe/HttpStripeClient.php` — Produkte auflisten, Filter
auflösen, Blättern wirft
- `app/Services/Stripe/FakeStripeClient.php` — Produkte auflisten, vier Felder
in `plantPrice()`, Metadaten zusammenführen, `created`-Gleichstand
- `app/Services/Billing/AdoptStripePrice.php` — die vier Eigenschaften prüfen,
Warnung drosseln
- `app/Services/Billing/AddonPrices.php` — Produkt-Wiedererkennung in
`product()`
- `app/Providers/AppServiceProvider.php` — beide Adoptions-Klassen als
Singleton, damit ihr Lauf-Protokoll über den ganzen Lauf trägt. Es heißt in
den beiden Klassen **nicht** gleich: `AdoptStripePrice::$adoptions` zählt die
übernommenen Preise, `AdoptStripeProduct::$duplicates` sammelt die IDs der
doppelten Produkte.
- `app/Console/Commands/SyncStripeCatalogue.php` — Produkt-Wiedererkennung,
getrennte Zählung, Kommentar berichtigt
- `tests/Feature/Billing/StripePriceAdoptionTest.php`,
`StripeIdempotencyKeyTest.php` — angehängt
- `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` — §14
abhaken, was hier erledigt wird

View File

@ -277,10 +277,11 @@ Zwei Regeldatei-Tests nach Repo-Sitte, getrennt danach, **gegen wen** sie prüfe
## 14. Folgepunkte, die aus der Umsetzung entstanden sind
Nachgetragen nach dem Abschluss-Review. Alle Befunde sind offengelegt, keiner
blockiert den Merge — aber keiner ist erledigt.
Nachgetragen nach dem Abschluss-Review. Die Fortsetzung steht in
`docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md`; jeder
Punkt unten ist abgehakt, mit der Datei, die ihn schließt.
### Die eine echte Lücke: Waisen-**Produkte**
### Die eine echte Lücke: Waisen-**Produkte** — erledigt
`activePricesFor()` erkennt einen verwaisten *Preis* wieder. Für ein verwaistes
**Produkt** gibt es kein Gegenstück, und dort ist der Schaden größer: ein
@ -295,7 +296,11 @@ benennen die Lücke jetzt; der Fix wäre ein `productsFor()` nach dem Muster von
`activePricesFor()`, das Stripe nach den Produkten fragt und über die Metadaten
wiedererkennt.
### Das Geld-Tor sitzt an der falschen Stelle
**Geschlossen durch:** `App\Services\Billing\AdoptStripeProduct`, beide
Verdrahtungen (`SyncStripeCatalogue::handle()` für eine Familie,
`AddonPrices::product()` für ein Modul).
### Das Geld-Tor sitzt an der falschen Stelle — erledigt
`AdoptStripePrice` verspricht in seinem Kopfkommentar, keine Übernahme könne
Geld verschieben — kann aber nur Betrag, Währung und Intervall vergleichen. Was
@ -307,53 +312,62 @@ diese Felder gar nicht ausdrücken, also kann **kein Test von
`StripeClient` ließe es still fallen. Die Prüfung gehört in die Klasse, die sie
zusagt.
**Geschlossen durch:** die Prüfung sitzt jetzt in `AdoptStripePrice` selbst;
`activePricesFor()`s Vertrag liefert die vier preisbestimmenden Eigenschaften,
statt selbst zu filtern.
Verwandt: `PlanPrices::ensure()` hat keine `<= 0`-Schranke, wo
`AddonPrices::ensure()` eine hat. Das ist das Einzige, was einen
`billing_scheme: tiered`-Preis (bei dem Stripe `unit_amount: null` liefert, hier
als 0 gelesen) auf der Paketseite überhaupt erreichbar macht.
**Bewusst nicht gebaut.** Begründung in der neuen Spec §3: sobald
`AdoptStripePrice` selbst `billing_scheme !== 'per_unit'` ablehnt, hat die
Schranke keinen Zweck mehr — sie würde nur noch ein Paket zu null (ein
kostenloser Tarif, heute nicht im Katalog, aber jederzeit anlegbar) treffen und
es unverkäuflich machen.
### Kleineres, benannt statt vergessen
- **Der Abgleich zählt „angelegt", auch wo er übernommen hat.** Die Wortwahl der
Konsolenausgabe ist entschärft („created or adopted"), gezählt wird weiterhin
vor dem Aufruf. Eine echte Trennung der beiden Zahlen fehlt — und das ist die
eine Zahl, die ein Mensch nach einem Vorfall zuerst liest.
**Erledigt** — getrennte Zählung in `SyncStripeCatalogue::handle()`.
- **Die Warnung über einen unerklärlichen Preis hat keine Drosselung.** Sie
feuert bei jedem Abgleich *und* jeder Kundenbuchung, solange die Waise
existiert — und weil das Aufräum-Kommando (§12) ein Folgepunkt ist, auf
unbestimmte Zeit. Der Widerspruchs-Pfad wurde aus genau diesem Grund still
gestellt; der Bestätigungs-Pfad bekam dieselbe Überlegung nicht.
**Erledigt** — einmal pro Preis und Tag, über `Cache::add()`.
- **`FakeStripeClient::updatePriceMetadata()` ersetzt, wo Stripe zusammenführt**,
und `activePricesFor()` gibt Metadaten ungecastet zurück, wo der HTTP-Client
auf String castet. Heute nicht beobachtbar, aber es ist genau die Sorte
Untreue des Fakes, gegen die §9 geschrieben wurde.
**Erledigt.**
- **`created` im Fake kann gleichstehen**: `createPrice()` stempelt
`count($prices) + 1`, `plantPrice()` hat die Vorgabe `1`. Der Kommentar sagt
das jetzt zu, statt es zu bestreiten; ein Gleichstand-Tiebreak auf die ID
fehlt weiter.
**Erledigt** — Gleichstand wird über die Preis-ID aufgelöst.
- **`HttpStripeClient::activePricesFor()` bricht das Blättern still ab**, wenn
dem letzten Eintrag einer nicht-letzten Seite die `id` fehlt — getreue Kopie
desselben Verhaltens in `invoiceLines()`. Die Folge hat sich aber geändert: in
`invoiceLines()` entsteht ein zu kurzer Belegtext, hier eine ungesehene Waise
und ein zweiter aktiver Preis, also der Vorfall. Wer es anfasst, soll werfen
statt anhalten.
**Erledigt** — wirft jetzt, für Preise und Produkte.
- **Der Idempotenz-Fingerabdruck und der POST-Rumpf müssen zusammenbleiben.**
Ein Feld, das künftig in `createPrice()` gesendet, aber nicht in
`IdempotencyKey::priceParameters()` aufgenommen wird, öffnet den Vorfall
wieder. Der Test „does not block a booking because the metadata format moved"
wacht über diese Nahtstelle — mit einem gesetzten Ledger-Eintrag, weil der
Zustand heute nicht mehr fahrbar ist.
**Bleibt offen** — es ist eine Naht, kein Fehler; der Test darüber existiert.
### Zwei fremde, vorbestehende Testfehler, die dieser Zweig aufgedeckt hat
### Ein fremder, vorbestehender Testfehler, den dieser Zweig aufgedeckt hat
Keiner davon wird von diesem Zweig verursacht.
1. **`HostStepsTest.php:1064`** schreibt `fsn-01.node.clupilot.com` hart hinein,
während `.env:107` `CLUPILOT_DNS_ZONE=clupilot.cloud` setzt und `phpunit.xml`
neun andere Variablen mit `force="true"` festnagelt, diese aber nicht. Eine
Zeile in `phpunit.xml`. Derselbe Punkt steht als offene Aufgabe in
`docs/handoffs/2026-07-30-real-run-handoff.md`.
2. **`PlanCatalogueTest.php`** ist einmal ausgefallen und lief beim
Wiederholen durch. „Beim zweiten Mal grün" ist keine Diagnose, und ein
sprunghafter Fehler in den Katalogpreis-Tests liegt im Wirkungskreis dieses
Zweigs, auch wenn er nicht von ihm kommt.
**`PlanCatalogueTest.php`** ist einmal ausgefallen und lief beim Wiederholen
durch. „Beim zweiten Mal grün" ist keine Diagnose, und ein sprunghafter Fehler
in den Katalogpreis-Tests liegt im Wirkungskreis dieses Zweigs, auch wenn er
nicht von ihm kommt. **Bleibt offen** — Ursachensuche, eigene Sitzung.

View File

@ -155,6 +155,13 @@ it('pages through every active price of a product', function () {
// side is which.
'currency' => 'EUR',
'interval' => 'month',
// Stripe's own defaults, absent from every price in this response —
// reported here rather than filtered, which is the whole point of
// this test's neighbour below.
'interval_count' => 1,
'usage_type' => 'licensed',
'transform_quantity' => false,
'billing_scheme' => 'per_unit',
'created' => 100,
'metadata' => ['addon' => 'priority_support'],
])
@ -170,73 +177,40 @@ it('pages through every active price of a product', function () {
Http::assertSent(fn ($request) => str_contains($request->url(), 'active=true'));
});
it('skips a price it could not have created itself, so the money gate never trusts a partial recurrence', function () {
it('reports the properties that decide what a price charges, and filters none of them', function () {
Http::fake([
'api.stripe.com/*' => Http::response([
'data' => [
['id' => 'price_monthly', 'unit_amount' => 3480, 'currency' => 'eur',
'created' => 100, 'recurring' => ['interval' => 'month'],
'metadata' => ['addon' => 'priority_support']],
['id' => 'price_ordinary', 'unit_amount' => 3480, 'currency' => 'eur',
'created' => 100, 'recurring' => ['interval' => 'month'], 'metadata' => []],
['id' => 'price_quarterly', 'unit_amount' => 3480, 'currency' => 'eur',
'created' => 101, 'recurring' => ['interval' => 'month', 'interval_count' => 3],
'metadata' => ['addon' => 'priority_support']],
['id' => 'price_metered', 'unit_amount' => 3480, 'currency' => 'eur',
'created' => 102, 'recurring' => ['interval' => 'month', 'usage_type' => 'metered'],
'metadata' => ['addon' => 'priority_support']],
],
'has_more' => false,
]),
]);
$prices = (new HttpStripeClient)->activePricesFor('prod_1');
// A hand-duplicated dashboard price keeps our metadata, so nothing
// downstream could tell it apart, and the module would bill quarterly.
//
// None of the three planted prices carries `transform_quantity` or
// `billing_scheme` at all, and the ordinary one still comes back — so this
// also holds the other direction of the next test's filter: an absent key is
// Stripe's default, not a reason to reject.
expect(collect($prices)->pluck('id')->all())->toBe(['price_monthly']);
});
it('skips a price that would charge our figure for the wrong quantity', function () {
Http::fake([
'api.stripe.com/*' => Http::response([
'data' => [
// Spelled out as Stripe actually sends them for an ordinary
// price: transform_quantity null, billing_scheme per_unit.
['id' => 'price_ordinary', 'unit_amount' => 3480, 'currency' => 'eur',
'created' => 100, 'recurring' => ['interval' => 'month'],
'transform_quantity' => null, 'billing_scheme' => 'per_unit',
'metadata' => ['addon' => 'priority_support']],
'metadata' => []],
['id' => 'price_divided', 'unit_amount' => 3480, 'currency' => 'eur',
'created' => 101, 'recurring' => ['interval' => 'month'],
'transform_quantity' => ['divide_by' => 10, 'round' => 'up'],
'billing_scheme' => 'per_unit',
'metadata' => ['addon' => 'priority_support']],
['id' => 'price_tiered', 'unit_amount' => null, 'currency' => 'eur',
'created' => 102, 'recurring' => ['interval' => 'month'],
'billing_scheme' => 'tiered',
'metadata' => ['addon' => 'priority_support']],
'transform_quantity' => ['divide_by' => 10, 'round' => 'up'], 'metadata' => []],
['id' => 'price_tiered', 'unit_amount' => null, 'currency' => 'eur',
'created' => 103, 'recurring' => ['interval' => 'month', 'usage_type' => 'metered'],
'billing_scheme' => 'tiered', 'metadata' => []],
],
'has_more' => false,
]),
]);
$prices = (new HttpStripeClient)->activePricesFor('prod_1');
$prices = collect((new HttpStripeClient)->activePricesFor('prod_1'))->keyBy('id');
// The concrete harm: modules are billed BY quantity — SyncStripeAddonItems
// sums a pack into ONE item at quantity n — so price_divided is our exact
// figure, on our Product, carrying our `addon` key, and adoption would take
// it. A customer holding three would then be charged ceil(3/10) = 1.
//
// price_tiered is refused by name rather than by luck: Stripe reports
// unit_amount null for a tiered price, activePricesFor() casts that to 0, and
// the amount match alone only rejects it while the caller's own figure is not
// 0 — which PlanPrices::ensure(), unlike AddonPrices::ensure(), does not
// guarantee.
expect(collect($prices)->pluck('id')->all())->toBe(['price_ordinary']);
// All four come back. Deciding which are usable is AdoptStripePrice's job —
// it is the class that promises adoption cannot move money, and it could not
// keep that promise while the fields lived only here.
expect($prices)->toHaveCount(4)
->and($prices['price_ordinary'])->toMatchArray([
'interval_count' => 1, 'usage_type' => 'licensed',
'transform_quantity' => false, 'billing_scheme' => 'per_unit',
])
->and($prices['price_quarterly']['interval_count'])->toBe(3)
->and($prices['price_divided']['transform_quantity'])->toBeTrue()
->and($prices['price_tiered']['usage_type'])->toBe('metered')
->and($prices['price_tiered']['billing_scheme'])->toBe('tiered');
});
it('writes metadata onto a price that already exists', function () {
@ -264,3 +238,92 @@ it('lets the fake answer with the prices it holds, minus the archived ones', fun
expect($found)->toContain($kept, 'price_orphan')
->and($found)->not->toContain($gone, $other);
});
it('merges metadata onto a price the way Stripe does, rather than replacing it', function () {
$fake = new FakeStripeClient;
$fake->plantPrice('price_a', 'prod_1', 3480, 'EUR', 'month',
['addon' => 'priority_support', 'internal_note' => 'kept']);
$fake->updatePriceMetadata('price_a', ['tax_treatment' => 'domestic']);
// Stripe merges: a key you do not send stays. A fake that replaced would let
// a test prove the opposite of production — and it is exactly this merging
// that AdoptStripePrice's "write only when it differs" comparison is built
// around.
expect($fake->activePricesFor('prod_1')[0]['metadata'])->toBe([
'addon' => 'priority_support',
'internal_note' => 'kept',
'tax_treatment' => 'domestic',
]);
});
it('hands metadata back as strings, the way the wire does', function () {
$fake = new FakeStripeClient;
$fake->plantPrice('price_a', 'prod_1', 3480, 'EUR', 'month', ['plan_price_id' => 42]);
// HttpStripeClient casts; the fake did not. An un-cast int is what makes
// confirms()'s strict === fail forever for that price.
expect($fake->activePricesFor('prod_1')[0]['metadata']['plan_price_id'])->toBe('42');
});
it('refuses to stop half-read when Stripe says there is another page', function () {
Http::fake([
'api.stripe.com/*' => Http::response([
// has_more, and the last item carries no id to page after. Stopping
// here leaves an orphan unseen — which is a second live Price for one
// figure, the incident this whole feature exists to end.
'data' => [['unit_amount' => 3480, 'currency' => 'eur', 'recurring' => ['interval' => 'month']]],
'has_more' => true,
]),
]);
expect(fn () => (new HttpStripeClient)->activePricesFor('prod_1'))
->toThrow(RuntimeException::class, 'half-read');
});
it('pages through every active product of the account', function () {
Http::fake([
'api.stripe.com/*' => Http::sequence()
->push([
'data' => [
['id' => 'prod_a', 'name' => 'Team', 'created' => 100,
'metadata' => ['plan_family' => 'team', 'plan_family_id' => '3']],
['id' => 'prod_b', 'name' => 'Priority Support', 'created' => 101,
'metadata' => ['addon' => 'priority_support']],
],
'has_more' => true,
])
->push([
'data' => [['id' => 'prod_c', 'name' => 'Etwas anderes', 'created' => 102, 'metadata' => []]],
'has_more' => false,
]),
]);
$products = (new HttpStripeClient)->activeProducts();
expect($products)->toHaveCount(3)
->and($products[0])->toBe([
'id' => 'prod_a',
'name' => 'Team',
'created' => 100,
'metadata' => ['plan_family' => 'team', 'plan_family_id' => '3'],
])
->and($products[2]['id'])->toBe('prod_c');
// No metadata filter is sent: Stripe cannot search by it, so the whole
// active list comes back and the matching happens here. An account holds a
// handful of products, not thousands.
Http::assertSent(fn ($request) => str_contains($request->url(), 'active=true')
&& ! str_contains($request->url(), 'metadata'));
Http::assertSent(fn ($request) => str_contains($request->url(), 'starting_after=prod_b'));
});
it('lets the fake answer with the products it holds', function () {
$fake = new FakeStripeClient;
$minted = $fake->createProduct('Team', ['plan_family_id' => '3']);
$fake->plantProduct('prod_orphan', 'Team', ['plan_family_id' => '3'], created: 0);
expect(collect($fake->activeProducts())->pluck('id')->all())
->toContain($minted, 'prod_orphan');
});

View File

@ -610,3 +610,100 @@ it('keeps only the lowest-id row in every group of duplicates, and leaves the re
'created_at' => now(), 'updated_at' => now(),
]))->toThrow(UniqueConstraintViolationException::class);
});
it('refuses a price whose recurrence is not one we could have minted', function () {
// The money gate, asked of AdoptStripePrice itself rather than of the
// client's filter. Until this moved, these four properties were rejected
// inside HttpStripeClient — and FakeStripeClient could not express them, so
// no test of this class could reach the promise its docblock makes.
$this->stripe->plantPrice('price_quarterly', 'prod_support', 3480, 'EUR', 'month',
moduleMetadata(), intervalCount: 3);
expect(adoptModulePrice())->toBeNull();
});
it('refuses a price that meters usage instead of selling a licence', function () {
$this->stripe->plantPrice('price_metered', 'prod_support', 3480, 'EUR', 'month',
moduleMetadata(), usageType: 'metered');
expect(adoptModulePrice())->toBeNull();
});
it('refuses a price that divides the quantity before charging', function () {
// Modules are billed BY quantity — SyncStripeAddonItems sums a pack into one
// item at quantity n — so a divide_by price would charge a customer holding
// three for one.
$this->stripe->plantPrice('price_divided', 'prod_support', 3480, 'EUR', 'month',
moduleMetadata(), transformQuantity: true);
expect(adoptModulePrice())->toBeNull();
});
it('refuses a price that keeps its money in tiers', function () {
$this->stripe->plantPrice('price_tiered', 'prod_support', 3480, 'EUR', 'month',
moduleMetadata(), billingScheme: 'tiered');
expect(adoptModulePrice())->toBeNull();
});
it('still adopts a price that carries every one of Stripe\'s defaults', function () {
// The direction that matters more than the four above: read the wrong way
// round, the gate refuses every legitimate price and recognition becomes a
// permanent no-op that nothing would ever alert on.
$this->stripe->plantPrice('price_ordinary', 'prod_support', 3480, 'EUR', 'month', moduleMetadata());
expect(adoptModulePrice())->toBe('price_ordinary');
});
it('warns about an unexplained price once a day, per price, not once a booking', function () {
Log::spy();
$this->stripe->plantPrice('price_by_hand', 'prod_support', 3480, 'EUR', 'month', []);
// ensure() runs inside a customer's module booking, so without a throttle
// this line is written every time anybody books anything.
adoptModulePrice();
adoptModulePrice();
adoptModulePrice();
Log::shouldHaveReceived('warning')->once();
// PER PRICE. A second unexplained orphan is a second thing to say, not a
// repeat of the first — and "once" alone does not pin that: a key mutated
// to a constant would silence every orphan after this file's first one and
// leave the assertion above green. That is the silent no-op this whole
// branch is built against.
$this->stripe->plantPrice('price_by_hand_too', 'prod_support', 3480, 'EUR', 'month', []);
adoptModulePrice();
Log::shouldHaveReceived('warning')->twice();
// PER DAY. An orphan is a STATE, so the line is due again tomorrow — the
// throttle exists to thin it out, not to say it once and never again. Past
// the TTL by a minute rather than exactly on it, so this asserts the expiry
// and not a boundary comparison.
$this->travelTo(now()->addDay()->addMinute());
adoptModulePrice();
// Asked of the FIRST price by name: after the travel both orphans warn
// again, and a count over all warnings could not tell "price_by_hand spoke
// twice" from "price_by_hand_too spoke twice and the first stayed silent".
Log::shouldHaveReceived('warning')
->withArgs(fn (string $message, array $context) => $context['price'] === 'price_by_hand')
->twice();
});
it('breaks a tie on created by the price id, so two runs agree', function () {
// plantPrice defaults created to 1 and createPrice stamps count+1, so a
// planted orphan can tie with a minted price. A tie decided by Stripe's list
// order would adopt one price today and the other tomorrow.
$this->stripe->plantPrice('price_bbb', 'prod_support', 3480, 'EUR', 'month',
moduleMetadata(), created: 7);
$this->stripe->plantPrice('price_aaa', 'prod_support', 3480, 'EUR', 'month',
moduleMetadata(), created: 7);
expect(adoptModulePrice())->toBe('price_aaa')
->and($this->stripe->archived)->toBe(['price_bbb']);
});

View File

@ -0,0 +1,184 @@
<?php
use App\Models\PlanFamily;
use App\Models\PlanPrice;
use App\Models\StripeAddonPrice;
use App\Models\StripePlanPrice;
use App\Models\Subscription;
use App\Services\Billing\AddonPrices;
use App\Services\Billing\AdoptStripeProduct;
use App\Services\Billing\TaxTreatment;
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\StripeClient;
use Illuminate\Contracts\Console\Kernel;
use Illuminate\Support\Facades\Log;
/**
* A run that dies between Stripe creating a PRODUCT and us storing its id leaves
* an orphan too and that one is worse than an orphaned Price.
*
* A second Product does not merely duplicate something. Every activePricesFor()
* afterwards is asked about the new Product, so the Price recognition built for
* exactly this failure goes blind for the whole family, and every orphaned Price
* on the first Product becomes unreachable. The guard disables itself through
* the gap beside it.
*
* Two things are deliberately unlike the Price side. There is no money gate: a
* Product carries no amount, so the metadata is the whole of the proof. And a
* duplicate is REPORTED, never deactivated an archived Price is provably
* harmless because Stripe goes on billing subscriptions already on it, but
* deactivating a Product makes its Prices unsellable, and contracts can be
* running on those.
*/
beforeEach(function () {
$this->stripe = new FakeStripeClient;
app()->instance(StripeClient::class, $this->stripe);
});
it('adopts an orphaned family product instead of minting a second one', function () {
// A run that got as far as creating the Product and died before storing its
// id: Stripe has it, plan_families does not.
$family = PlanFamily::query()->orderBy('tier')->firstOrFail();
$family->update(['stripe_product_id' => null]);
$this->stripe->plantProduct('prod_orphan_family', $family->name, [
'plan_family' => $family->key,
'plan_family_id' => (string) $family->id,
]);
app(Kernel::class)->call('stripe:sync-catalogue');
// Counting every product would prove nothing — the same run mints one for
// each of the OTHER families. What must hold is that this family's id is
// carried by exactly one product, and that it is the planted one.
expect($family->refresh()->stripe_product_id)->toBe('prod_orphan_family')
->and(collect($this->stripe->products)
->filter(fn (array $p) => ($p['metadata']['plan_family_id'] ?? null) === (string) $family->id)
->keys()->all())->toBe(['prod_orphan_family']);
});
it('adopts an orphaned module product instead of minting a second one', function () {
$this->stripe->plantProduct('prod_orphan_module', 'Priority Support', [
'addon' => 'priority_support',
]);
$before = count($this->stripe->products);
app(AddonPrices::class)->ensure(
'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(),
);
expect(StripeAddonPrice::query()
->where('addon_key', 'priority_support')
->value('stripe_product_id'))->toBe('prod_orphan_module')
->and(count($this->stripe->products))->toBe($before);
});
it('adopts nothing a product does not prove about itself', function () {
// What an operator selling something else through the same Stripe account
// leaves lying around. Silent, unlike the Price side: we list every product
// on the account, so warning about each foreign one would fire on every run
// for as long as it exists.
Log::spy();
$this->stripe->plantProduct('prod_stranger', 'Etwas ganz anderes', []);
expect(app(AdoptStripeProduct::class)(
['addon' => 'priority_support'], ['addon'],
))->toBeNull();
Log::shouldNotHaveReceived('warning');
});
it('adopts nothing that contradicts what we would have written', function () {
$this->stripe->plantProduct('prod_other_module', 'Collabora Pro', ['addon' => 'collabora_pro']);
expect(app(AdoptStripeProduct::class)(
['addon' => 'priority_support'], ['addon'],
))->toBeNull();
});
it('takes the oldest of two matching products and leaves the other alone', function () {
Log::spy();
$this->stripe->plantProduct('prod_second', 'Priority Support', ['addon' => 'priority_support'], created: 200);
$this->stripe->plantProduct('prod_first', 'Priority Support', ['addon' => 'priority_support'], created: 100);
$adopt = app(AdoptStripeProduct::class);
expect($adopt(['addon' => 'priority_support'], ['addon']))->toBe('prod_first')
// NOT deactivated. Its prices would become unsellable, and a contract
// can be running on one of them — the asymmetry with archivePrice().
->and($adopt->duplicates)->toBe(['prod_second']);
Log::shouldHaveReceived('warning')->once();
});
it('says it adopted, not that it created', function () {
// The one signal a human reads after an incident. The count is taken before
// ensure() runs, so without this the run reports objects created in Stripe
// when it made none — and telling those two apart is the whole point of the
// recognition step.
app(Kernel::class)->call('stripe:sync-catalogue');
$row = PlanPrice::query()->firstOrFail();
$orphan = (string) StripePlanPrice::query()
->where('plan_price_id', $row->id)->where('reverse_charge', false)
->value('stripe_price_id');
StripePlanPrice::query()->where('stripe_price_id', $orphan)->delete();
$this->stripe->keys = [];
$this->artisan('stripe:sync-catalogue')
->expectsOutputToContain('1 adopted')
->assertSuccessful();
});
it('names a duplicate product in its report, without touching it', function () {
// Nothing has been synced yet, same as the orphan test above — the family
// has no product of its own, and two stray ones claiming to be it are the
// whole of what Stripe has for it.
$family = PlanFamily::query()->orderBy('tier')->firstOrFail();
$family->update(['stripe_product_id' => null]);
$metadata = ['plan_family' => $family->key, 'plan_family_id' => (string) $family->id];
$this->stripe->plantProduct('prod_first', $family->name, $metadata, created: 100);
$this->stripe->plantProduct('prod_second', $family->name, $metadata, created: 200);
$this->artisan('stripe:sync-catalogue')
->expectsOutputToContain('prod_second')
->assertSuccessful();
expect($family->refresh()->stripe_product_id)->toBe('prod_first');
});
it('does not fold a module Product adoption into the created/minted count', function () {
// A module's prices are all genuinely new, and its Product exists at
// Stripe already — unlinked from our own register, the same shape as a
// family's orphaned Product one level up (AddonPrices::product() asks
// AdoptStripeProduct exactly as the family loop does). The difference that
// makes this dangerous: syncModules() below counts a module's PRICES as
// intents, but it has never counted a module's PRODUCT as one — adopted or
// minted, that Product prints no line and increments nothing — so
// adopting it here must neither inflate "adopted" nor shrink "created".
$this->stripe->plantProduct('prod_priority_support_orphan', 'Priority Support', [
'addon' => 'priority_support',
]);
// Computed from the catalogue rather than hardcoded, the way
// ReverseChargePriceTest sizes a rate-change sweep: one Product per family,
// two treatments per priced row, and monthly+yearly at both treatments for
// every module — all of it newly minted here, nothing pre-existing to
// adopt except the one Product just planted above.
$modules = count(array_merge(array_keys((array) config('provisioning.addons')), ['storage']));
$expectedCreated = PlanFamily::query()->count() + PlanPrice::query()->count() * 2 + $modules * 4;
$this->artisan('stripe:sync-catalogue')
->expectsOutputToContain("{$expectedCreated} object(s) created, 0 adopted in Stripe.")
->assertSuccessful();
// And the orphan really was taken over rather than merely ignored — the
// Product this test is about, not a no-op that would pass it by accident.
expect(StripeAddonPrice::query()
->where('addon_key', 'priority_support')
->value('stripe_product_id'))->toBe('prod_priority_support_orphan');
});

View File

@ -0,0 +1,123 @@
<?php
use App\Models\PlanPrice;
use App\Models\StripeAddonPrice;
use App\Models\StripePlanPrice;
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\StripeClient;
use Illuminate\Contracts\Console\Kernel;
/**
* The orphans an operator can finally act on.
*
* The command lists EVERY active recurring Price at one of our Products that no
* row knows adoptable or not; the only question it asks is whether a register
* row holds the id. That is wider than the log line AdoptStripePrice writes, and
* it is why stripe:sync-catalogue has to run first: see the command's own
* docblock, which had to be corrected away from the narrower claim.
*
* The fixtures below are all at the narrow end anyway planted with no metadata
* at all, so nothing could have adopted them, which is what a person clicking
* through Stripe's dashboard leaves behind. That keeps each test about the
* sweep's own question (does a row know this id?) rather than about what the
* recognition step would have done with it.
*/
beforeEach(function () {
$this->stripe = new FakeStripeClient;
app()->instance(StripeClient::class, $this->stripe);
app(Kernel::class)->call('stripe:sync-catalogue');
$this->product = (string) StripeAddonPrice::query()
->where('addon_key', 'priority_support')
->value('stripe_product_id');
$this->known = (string) StripeAddonPrice::query()
->where('addon_key', 'priority_support')
->where('interval', 'month')
->where('reverse_charge', false)
->value('stripe_price_id');
$this->stripe->plantPrice('price_orphan', $this->product, 9900, 'EUR', 'month', []);
});
it('reports the orphan and leaves it alone', function () {
$this->artisan('stripe:sweep-orphan-prices')
->expectsOutputToContain('price_orphan')
->assertSuccessful();
expect($this->stripe->archived)->toBe([]);
});
it('never reports a price a row knows', function () {
$this->artisan('stripe:sweep-orphan-prices')
->doesntExpectOutputToContain($this->known)
->assertSuccessful();
});
it('sees the plan side too: a family product orphan is reported, a known plan price never', function () {
// Every other test in this file plants its orphan under the MODULE Product,
// so both halves of the plan side went untested: the PlanFamily half of
// products() and the StripePlanPrice half of the $known set. Blind to
// either, the command misses the LARGER half — a family Product collects a
// Price per version, term, treatment and rate change, a module Product four.
//
// Both assertions sit in one test on purpose: each fails for one of the two
// lines. Drop the PlanFamily pluck from products() and the family Product is
// never visited, so the orphan below goes unreported. Drop the
// StripePlanPrice pluck from $known and the row's own Price becomes an
// orphan the command names.
$row = PlanPrice::query()->firstOrFail();
$familyProduct = (string) $row->version->family->stripe_product_id;
$knownPlanPrice = (string) StripePlanPrice::query()
->where('plan_price_id', $row->id)
->where('reverse_charge', false)
->value('stripe_price_id');
// At the family's Product and at an amount no row of ours holds — what the
// sync leaves behind when it dies between Stripe's create and our insert,
// seen from the sweep's side.
$this->stripe->plantPrice('price_family_orphan', $familyProduct,
12300, (string) $row->currency, 'month', []);
$this->artisan('stripe:sweep-orphan-prices')
->expectsOutputToContain('price_family_orphan')
->doesntExpectOutputToContain($knownPlanPrice)
->assertSuccessful();
expect($familyProduct)->not->toBe('')
->and($knownPlanPrice)->not->toBe('')
// Nothing was touched: this is the reporting path, not --archive.
->and($this->stripe->archived)->toBe([]);
});
it('stops selling the orphan when asked to', function () {
$this->artisan('stripe:sweep-orphan-prices', ['--archive' => true])
->assertSuccessful();
// Archiving is harmless for the same reason it always is here: Stripe goes
// on billing every subscription already on an archived Price, it merely
// stops being sold.
expect($this->stripe->archived)->toBe(['price_orphan']);
});
it('touches nothing on a dry run, even when asked to archive', function () {
$this->artisan('stripe:sweep-orphan-prices', ['--archive' => true, '--dry-run' => true])
->expectsOutputToContain('price_orphan')
->assertSuccessful();
expect($this->stripe->archived)->toBe([]);
});
it('fails cleanly on a dry run when Stripe is not configured, rather than throwing', function () {
// Unlike stripe:sync-catalogue, this command has no local-only path: the
// report itself is a live activePricesFor() call, dry-run or not. A guard
// that only fired outside --dry-run would let this fall straight into
// HttpStripeClient's own "not configured" exception instead of the clean
// error below.
$this->stripe->configured = false;
$this->artisan('stripe:sweep-orphan-prices', ['--dry-run' => true])
->expectsOutputToContain('Stripe is not configured (STRIPE_SECRET is empty). Nothing was read.')
->assertFailed();
});