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 new file mode 100644 index 0000000..a2d2491 --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md @@ -0,0 +1,270 @@ +# Spec — Bestehende Stripe-Preise wiedererkennen, statt sie zu verdoppeln + +**Datum:** 2026-07-30 +**Status:** entworfen, noch nicht umgesetzt +**Vorgänger-Kontext:** `docs/handoffs/2026-07-30-real-run-handoff.md` Block D +(`stripe_addon_prices.stripe_price_id` ist nicht eindeutig) + +--- + +## 1. Der Vorfall + +Ein Lauf am **2026-07-29 23:11** legte bei Stripe +`price_1TygdEC7u8NpJ8pOt3nsoyYw` an (`priority_support`, 3480 EUR, monatlich) und +brach ab, **bevor** die Zeile in `stripe_addon_prices` geschrieben war. Commit +`9da1358` fügte danach das Metadatenfeld `tax_treatment` hinzu. Der nächste +Abgleich schickte **denselben** Idempotenz-Schlüssel mit den **neuen** Metadaten +und bekam von Stripe: + +> HTTP 400 — Keys for idempotent requests can only be used with the same +> parameters they were first used with. + +Der Abgleich war damit hart blockiert und wäre es bis zum Ablauf des Schlüssels +(24 h) geblieben. Von Hand bereinigt: Zeile für den vorhandenen Preis +nachgetragen, Metadaten bei Stripe angeglichen. Die Ursache stand offen. + +## 2. Ausgangsbefund — gemessen, nicht gelesen + +| Befund | Beleg | +|---|---| +| Der Modul-Schlüssel deckt Betrag, Währung, Intervall, Modul und Behandlung ab — **keine** Metadaten | `app/Services/Billing/AddonPrices.php:152` | +| Der Paket-Schlüssel ebenso | `app/Services/Billing/PlanPrices.php:158` | +| Beide senden Metadaten mit, die **nicht** im Schlüssel stehen — u. a. das neue `tax_treatment` | `AddonPrices.php:133-141`, `PlanPrices.php:141-151` | +| `createProduct` hat dieselbe Falle latent: Metadaten mit, Schlüssel ohne | `AddonPrices.php:269-273`, `SyncStripeCatalogue.php:104-111`, `HttpStripeClient.php:87-97` | +| Kein Aufruf fragt Stripe nach **vorhandenen** Preisen. Der Client hat gar keine Methode dafür | `app/Services/Stripe/StripeClient.php` — `createPrice`, `archivePrice`, `activatePrice`, kein Auflisten | +| `ensure()` läuft **nicht nur** im Abgleich, sondern in der **Kundenbuchung** | `BookAddon.php:259` und `:360` → `SyncStripeAddonItems.php:183` und `:291` → `AddonPrices::ensure()` | +| Der Fake spielt einen bekannten Schlüssel zurück, **ohne die Parameter zu vergleichen** | `FakeStripeClient.php:168-193`, Ledger `$keys` (`:38`) | +| `stripe_plan_prices.stripe_price_id` ist eindeutig | Migration `2026_07_30_110000:56` | +| `stripe_addon_prices.stripe_price_id` ist **nicht** eindeutig; eindeutig ist nur das Tupel | Migration `2026_07_31_200000:59-77` | +| Der Client blättert schon woanders durch Stripe-Listen | `HttpStripeClient::invoiceLines()`, `:312-336` | +| Produktion `mariadb`, Tests `sqlite :memory:` | `.env:24`, `phpunit.xml:42-43` | + +**Der 400 ist damit kein Betriebsfehler, sondern ein Kundenfehler.** Ein +Kunde, der ein Modul bucht, während ein Waisen-Preis mit altem Metadatenformat +bei Stripe liegt, bekommt 24 Stunden lang keine Buchung zustande. + +## 3. Zwei getrennte Mängel + +1. **Waise ohne Wiedererkennung.** Ein Abgleich, der zwischen Stripes Anlage und + unserem Schreiben abbricht, hinterlässt einen Preis bei Stripe, den unsere + Tabelle nie erfährt. Nach Ablauf der 24 Stunden legt der nächste Lauf einen + **zweiten aktiven** Preis über dieselbe Summe an — genau das, was der + Schlüssel verhindern soll. +2. **Metadaten im Aufruf, nicht im Schlüssel.** Jede künftige Änderung an den + Metadaten von `createPrice` blockiert den Abgleich für 24 Stunden auf + dieselbe Weise. + +## 4. Gehört das Metadatenformat zur Identität? — Nein + +**Identität** eines Preises ist, was Stripe unveränderlich macht: Produkt, +Betrag, Währung, Intervall — und, weil bei einem Steuersatz von 0 % beide +Behandlungen denselben Betrag haben, welche der beiden es ist. Metadaten sind +bei Stripe **nachträglich änderbar**. Deshalb wird ein wiedererkannter Preis mit +korrigierten Metadaten **übernommen**, nicht durch einen neuen ersetzt. + +Der Idempotenz-Schlüssel behauptet aber gar keine Identität. Er behauptet +„**diesen Aufruf habe ich schon abgeschickt**". Weil Stripe das wörtlich prüft, +müssen **alle gesendeten Parameter** hinein — sonst ist der Schlüssel eine +Lüge, die mit 400 endet. + +Der Kommentar im Code verwechselt beides („Keyed on what the Price IS", +`AddonPrices.php:142-151`). Das ist die eigentliche Ursache und wird +mitkorrigiert: die Identität wandert in den Wiedererkennungsschritt, der +Schlüssel wird auf seine tatsächliche Aussage zurückgeführt. + +## 5. Regeln des Auftraggebers, die dieser Entwurf nicht anfassen darf + +Beide wurden im Gespräch am 2026-07-30 gesetzt und gelten als Prüfstein: + +1. **Bestehende Verträge behalten ihre alten Preise.** Neue Preise gelten nur + für Kunden, die neu abschließen. Daraus folgt die Freigabe zum Archivieren: + ein archivierter Stripe-Preis bleibt für laufende Abos in Kraft, er wird nur + nicht mehr **verkauft** — worauf `archiveSuperseded()` schon heute beruht. +2. **Jede Buchung friert ihren Preis ein.** Ein Paket und zwei Module werden + immer gleich verrechnet; ein später gebuchtes Modul kommt zum heutigen Preis; + eine Stornierung zieht genau den damaligen Betrag ab und wird danach nicht + mehr verrechnet. + +Beides bleibt strukturell gewahrt, weil die Übernahme **ausschließlich** Preise +mit **identischem** Betrag, Währung und Intervall betrifft. Sie kann niemandes +Verrechnung verschieben. §11 schreibt es als Test fest. + +## 6. Bauteil 1 — `App\Services\Billing\AdoptStripePrice` + +Der Wiedererkennungsschritt, **einmal** für beide Seiten (Modul und Paket), weil +zwei Fassungen derselben Regel der Anfang ihres Auseinanderlaufens sind. + +**Stellung im Ablauf:** nach dem Blick in unsere eigene Tabelle, vor +`createPrice`. Also genau dann ein zusätzliches GET, wenn ohnehin ein POST +fällig gewesen wäre — nie im Normalfall. + +``` +ensure() + ├─ Zeile in unserer Tabelle? → ja: wie heute (ggf. reaktivieren) + ├─ AdoptStripePrice: Kandidat bei Stripe? → ja: übernehmen + └─ createPrice(…) → nein: wie heute anlegen +``` + +**Ein Kandidat wird übernommen, wenn er alle fünf Bedingungen erfüllt:** + +1. bei Stripe **aktiv** und an **unserem** Produkt hängend + (`active=true`, `product=…`, `type=recurring`); +2. **exakt** gleich in Betrag, Währung (klein/groß normalisiert) und Intervall; +3. **widerspricht keinem** unserer Metadatenschlüssel, den er trägt; +4. **bestätigt mindestens einen** tragenden Schlüssel — `addon` auf der + Modulseite, `plan_price_id` auf der Paketseite. Ein von Hand im Dashboard + angelegter Preis wird damit nie übernommen; +5. ist **von keiner Zeile beansprucht** (Prüfung gegen die eigene Tabelle, + archivierte Zeilen eingeschlossen). + +**Danach:** Metadaten bei Stripe angleichen (genau das, was von Hand getan +wurde), Zeile schreiben, `archiveSuperseded()` wie im Anlege-Pfad, Preis-ID +zurückgeben. + +**Mehrere Kandidaten:** der **älteste** (`created` aufsteigend) wird übernommen — +er ist am ehesten der, auf dem eine verlorene Zeile schon bilanziert. Die +übrigen werden bei Stripe **archiviert** (Freigabe aus §5.1) und protokolliert. + +**Kein beweisbar eigener Kandidat:** Verhalten wie heute (anlegen), mit Warnung +im Log. Bewusst so: einen fremden Preis zu übernehmen ist schlimmer als einen +zweiten anzulegen, und die Warnung nennt die ID, damit ein Betreiber entscheiden +kann. + +**Meldeweg ist das Log** (`Log::warning`), nicht die Konsolenausgabe: `ensure()` +läuft auch im Web-Request, wo es keine Kommandozeile gibt. + +## 7. Bauteil 2 — `App\Services\Stripe\IdempotencyKey` + +Bildet aus dem sprechenden Schlüssel plus einem kurzen Fingerabdruck der +**tatsächlich gesendeten Parameter** den Header-Wert: + +```php +IdempotencyKey::for('clupilot-addon-price-priority_support-month-3480-EUR', $payload) +// → 'clupilot-addon-price-priority_support-month-3480-EUR-9f3a1c07' +``` + +Kanonisch gebildet (Schlüssel sortiert, dann `sha1`, auf 8 Zeichen gekürzt), +damit dieselbe Nutzlast immer denselben Fingerabdruck ergibt. In **einer** +Support-Klasse, weil `HttpStripeClient` und `FakeStripeClient` denselben Wert +bilden müssen — sonst prüft der Test etwas anderes, als die Produktion sendet. + +**Angewandt nur auf `createPrice` und `createProduct`.** Bewusst **nicht** auf: + +| Aufruf | Warum der 400 dort bleiben muss | +|---|---| +| `refund` | Ein zweiter Refund schickt dem Kunden das Geld zweimal. | +| `cancelSubscription` | Andere Parameter unter demselben Schlüssel heißt: eine andere Kündigungsart als die schon abgeschickte. | +| `addSubscriptionItem` | Ein zweites Item verrechnet dasselbe Modul doppelt. | +| `createCheckoutSession` | Eine zweite Session zu geänderten Parametern ist ein zweiter Kaufvorgang. | + +Die Regel lautet also: **der Fingerabdruck ist für die Katalogseite, wo ein +Duplikat reparierbar und eine Blockade kundenwirksam ist.** Bei +Geldbewegungen ist es umgekehrt. §11 schreibt diese Grenze als Test fest, damit +sie niemand „vereinheitlicht". + +## 8. Zwei neue Client-Methoden + +```php +/** @return array}> */ +public function activePricesFor(string $productId): array; + +public function updatePriceMetadata(string $priceId, array $metadata): void; +``` + +`activePricesFor()` blättert nach dem Muster von `invoiceLines()` +(`HttpStripeClient.php:312-336`) — ein Familien-Produkt sammelt über Versionen, +Terme, Behandlungen und Steuersatzänderungen hinweg leicht mehr als eine Seite, +und beim ersten Seitenende stehenzubleiben wäre dieselbe Lücke in neuer Form. + +`updatePriceMetadata()` benutzt, dass Metadaten eines der wenigen Felder sind, +die ein Stripe-Preis ändern lässt — dieselbe Eigenschaft, auf der +`activatePrice()` schon beruht. + +Beide im `FakeStripeClient` nachgezogen. + +## 9. Der Fake muss Stripe erst einmal nachbilden + +`FakeStripeClient::createPrice` spielt einen bekannten Schlüssel zurück, **ohne +die Parameter zu vergleichen** (`:168-193`). Er ist genau in der Sache +unrealistisch, um die es hier geht — deshalb konnte kein Test den Vorfall sehen. + +Der Ledger `$keys` speichert künftig neben der ID auch die Nutzlast und wirft +bei gleichem Schlüssel mit anderen Parametern — mit Stripes Wortlaut. Das ist +der Regressionstest, der den 29.07. gesehen hätte. + +## 10. Migration — eine Zeile pro Stripe-Preis + +`2026_07_31_210000_one_row_per_stripe_price.php` (nach der letzten vorhandenen +Migration einsortiert): + +1. **Entdoppeln:** Zeilen in `stripe_addon_prices`, die eine + `stripe_price_id` teilen — niedrigste Zeilen-ID bleibt, die übrigen werden + **gelöscht**. Nicht archiviert: eine archivierte Zeile beansprucht die ID + weiter und der eindeutige Index kennt `archived_at` nicht. +2. **Index:** `stripe_price_id` wird eindeutig — wie auf der Paketseite seit + `2026_07_30_110000:56`. + +Löschen ist gefahrlos: die Zeile baut sich beim nächsten `ensure()` über die +Wiedererkennung selbst wieder auf, und `subscription_addons.stripe_price_id` +hält die Stripe-ID als **Text**, nicht als Fremdschlüssel. + +Treiberneutral formuliert (Auswahl über `groupBy`/`having`, Löschen über +IDs) — Produktion ist `mariadb`, Tests laufen auf `sqlite`. + +**Was der Index absichert:** Bei einem Steuersatz von 0 % sind die Beträge +beider Behandlungen gleich. Die Übernahme lehnt einen bereits beanspruchten +Preis dann über §6 Bedingung 5 ab und legt für die zweite Behandlung einen +eigenen an (der Schlüssel trägt `-rc`). Der Index ist das Netz darunter, falls +diese Prüfung je gerissen wird. + +## 11. Tests + +Ein Regeldatei-Test nach Repo-Sitte: +`tests/Feature/Billing/StripePriceAdoptionTest.php` + +| Prüfung | Deckt ab | +|---|---| +| Waise bei Stripe → wird übernommen, `createPrice` **nicht** aufgerufen, Zeile trägt die vorhandene ID (Modul **und** Paket) | Mangel 1 | +| Waise mit **altem** Metadatenformat → wird übernommen, Metadaten bei Stripe angeglichen | der Vorfall selbst | +| Zwei Waisen → älteste übernommen, andere bei Stripe archiviert, Warnung im Log | §6 | +| Aktiver Preis an unserem Produkt **ohne** unsere Metadaten → **nicht** übernommen, neuer angelegt, Warnung | §6 Bedingung 4 | +| Preis-ID schon von einer Zeile beansprucht → **nicht** zweimal vergeben | §6 Bedingung 5 | +| **Eine eingefrorene Buchung behält ihren Betrag**: ein auf altem Preis laufendes Modul wird von einem Abgleich nicht verschoben | §5.2 | +| Gleicher sprechender Schlüssel + geänderte Metadaten → **unterschiedlicher** Header, kein 400 | Mangel 2 | +| Fingerabdruck **nur** bei `createPrice`/`createProduct`, nicht bei `refund`, `cancelSubscription`, `addSubscriptionItem`, `createCheckoutSession` | §7 | +| Der Fake wirft bei gleichem Schlüssel mit anderen Parametern | §9 | +| Zwei Zeilen können keine `stripe_price_id` teilen | §10 | + +Der Header-Test läuft über `Http::fake()` gegen `HttpStripeClient`, nicht gegen +den Fake: geprüft werden muss, was **gesendet** wird. + +## 12. Nicht Teil dieses Vorhabens + +- **Ein Aufräum-Kommando** für Waisen, die niemand mehr übernehmen kann (etwa + ohne unsere Metadaten). Der Log-Eintrag nennt sie; ein Kommando dafür ist ein + Folgepunkt. +- **Die übrigen Block-D-Punkte** aus `docs/handoffs/2026-07-30-real-run-handoff.md`. +- **`stripe:reprice-subscriptions`** und alles, was laufende Verträge bewegt. + Dieser Entwurf bewegt keinen einzigen — siehe §5. +- **Stripes `automatic_tax`** bleibt aus, `TaxTreatment` bleibt die einzige + Steuerinstanz. + +## 13. Berührte Dateien + +**Neu** +- `app/Services/Billing/AdoptStripePrice.php` +- `app/Services/Stripe/IdempotencyKey.php` +- `database/migrations/2026_07_31_210000_one_row_per_stripe_price.php` +- `tests/Feature/Billing/StripePriceAdoptionTest.php` + +**Geändert** +- `app/Services/Stripe/StripeClient.php` — zwei Methoden +- `app/Services/Stripe/HttpStripeClient.php` — Auflisten, Metadaten schreiben, + Fingerabdruck bei den zwei Anlege-Aufrufen +- `app/Services/Stripe/FakeStripeClient.php` — beide Methoden, Ledger mit + Parametervergleich +- `app/Services/Billing/AddonPrices.php` — Wiedererkennung vor `createPrice`, + Kommentar zur Schlüssel-Begründung berichtigt +- `app/Services/Billing/PlanPrices.php` — dasselbe +- `docs/handoffs/2026-07-30-real-run-handoff.md` — Block-D-Punkt als erledigt + markieren