From 277a6bab830d4f4d70ef9f300a89dc33a47c2178 Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 18:36:06 +0200 Subject: [PATCH] Give the operator something to do about an orphan nobody can adopt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Commands/SweepOrphanStripePrices.php | 109 ++++++++++++++++++ ...-07-30-stripe-adoption-followups-design.md | 2 +- ...2026-07-30-stripe-price-adoption-design.md | 46 +++++--- .../Feature/Billing/SweepOrphanPricesTest.php | 66 +++++++++++ 4 files changed, 206 insertions(+), 17 deletions(-) create mode 100644 app/Console/Commands/SweepOrphanStripePrices.php create mode 100644 tests/Feature/Billing/SweepOrphanPricesTest.php diff --git a/app/Console/Commands/SweepOrphanStripePrices.php b/app/Console/Commands/SweepOrphanStripePrices.php new file mode 100644 index 0000000..edc355f --- /dev/null +++ b/app/Console/Commands/SweepOrphanStripePrices.php @@ -0,0 +1,109 @@ +option('dry-run'); + $archive = (bool) $this->option('archive'); + + if (! $dryRun && ! $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; + } + + $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. Run with --archive to stop selling them.", + }); + + 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 + */ + 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(); + } +} diff --git a/docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md b/docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md index f246df0..89677e8 100644 --- a/docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md +++ b/docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md @@ -250,7 +250,7 @@ Und an `StripeIdempotencyKeyTest.php`: Blättern **wirft** bei fehlender `id` - `app/Services/Billing/AddonPrices.php` — Produkt-Wiedererkennung in `product()` - `app/Providers/AppServiceProvider.php` — beide Adoptions-Klassen als - Singleton, damit ihr Lauf-Protokoll (`$duplicates`, `$adoptions`) trägt + Singleton, damit ihr Lauf-Protokoll (`$duplicates`) trägt - `app/Console/Commands/SyncStripeCatalogue.php` — Produkt-Wiedererkennung, getrennte Zählung, Kommentar berichtigt - `tests/Feature/Billing/StripePriceAdoptionTest.php`, diff --git a/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md b/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md index be50139..d53c1e4 100644 --- a/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md +++ b/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md @@ -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. diff --git a/tests/Feature/Billing/SweepOrphanPricesTest.php b/tests/Feature/Billing/SweepOrphanPricesTest.php new file mode 100644 index 0000000..c33f453 --- /dev/null +++ b/tests/Feature/Billing/SweepOrphanPricesTest.php @@ -0,0 +1,66 @@ +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('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([]); +});