CluPilotCloud/docs/superpowers/specs/2026-07-30-stripe-price-ado...

360 lines
19 KiB
Markdown

# 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<int, array{id: string, unit_amount: int, currency: string, interval: string, created: int, metadata: array<string, string>}> */
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
Zwei Regeldatei-Tests nach Repo-Sitte, getrennt danach, **gegen wen** sie prüfen:
- `tests/Feature/Billing/StripeIdempotencyKeyTest.php` — was auf der Leitung
landet. Läuft über `Http::fake()` gegen `HttpStripeClient`, weil geprüft werden
muss, was **gesendet** wird, nicht was der Fake sich merkt. Hier steht auch,
dass der Fake Stripes 400 überhaupt nachbildet.
- `tests/Feature/Billing/StripePriceAdoptionTest.php` — die Übernahmeregeln,
beide Seiten, über den `FakeStripeClient`.
| 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 |
## 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/StripeIdempotencyKeyTest.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
---
## 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.
### Die eine echte Lücke: Waisen-**Produkte**
`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: `SyncStripeCatalogue::handle()` (Produkt anlegen, ID danach in einem
zweiten Schreibvorgang speichern) und `AddonPrices::product()`. Beide Kommentare
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
`AdoptStripePrice` verspricht in seinem Kopfkommentar, keine Übernahme könne
Geld verschieben — kann aber nur Betrag, Währung und Intervall vergleichen. Was
sonst noch entscheidet, was ein Preis verrechnet (`interval_count`,
`usage_type`, `transform_quantity`, `billing_scheme`), filtert
`HttpStripeClient::activePricesFor()`. Zwei Folgen: der `FakeStripeClient` kann
diese Felder gar nicht ausdrücken, also kann **kein Test von
`AdoptStripePrice` das Geld-Tor abdecken**; und eine dritte Implementierung von
`StripeClient` ließe es still fallen. Die Prüfung gehört in die Klasse, die sie
zusagt.
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.
### 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.
- **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.
- **`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.
- **`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.
- **`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.
- **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.
### Zwei fremde, vorbestehende Testfehler, die 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.