diff --git a/docs/superpowers/plans/2026-07-30-stripe-adoption-followups.md b/docs/superpowers/plans/2026-07-30-stripe-adoption-followups.md new file mode 100644 index 0000000..9ede35c --- /dev/null +++ b/docs/superpowers/plans/2026-07-30-stripe-adoption-followups.md @@ -0,0 +1,1680 @@ +# Stripe Adoption Follow-ups — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Die neun Punkte aus §14 der Preis-Wiedererkennung bauen — allen voran die Wiedererkennung verwaister Stripe-**Produkte**, ohne die die schon gebaute Preis-Wiedererkennung für eine Familie dauerhaft blind werden kann. + +**Architecture:** Das Geld-Tor zieht aus `HttpStripeClient::activePricesFor()` in `AdoptStripePrice` um — der Client liefert die preisbestimmenden Eigenschaften, der Aufrufer entscheidet, was zulässig ist. Daneben entsteht `AdoptStripeProduct` nach demselben Muster wie `AdoptStripePrice`, aber ohne Geld-Tor (ein Produkt hat keinen Betrag) und ohne Stilllegen von Doppeln (ein stillgelegtes Produkt macht seine Preise unverkäuflich). Dazu ein Aufräum-Kommando, eine Tagesdrosselung der Waisen-Warnung und vier mechanische Korrekturen an Client und Fake. + +**Tech Stack:** Laravel 11, Pest, Stripe REST über `Illuminate\Http\Client`, MariaDB in Produktion / SQLite `:memory:` in Tests. PHP läuft im Container: `docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test …` + +**Spec:** `docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md` +**Vorgänger:** `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` §14 + +## Global Constraints + +- **Commit-Disziplin (Nutzervorgabe, nicht verhandelbar):** eine zweite Session arbeitet im selben Repository. **Immer** `git add -- ` und `git commit -F - -- `. **Nie** `git add -A`, `git add .`, `git commit -a`. Diese Regel wurde am 2026-07-30 gebrochen und hat 3526 Zeilen fertiger Arbeit aus `main` gelöscht. +- **Es wird ausschließlich im Worktree gearbeitet** (`/home/nexxo/clupilot/.claude/worktrees/cool-sammet-368fee`, Zweig `claude/cool-sammet-368fee`). Das Haupt-Repo `/home/nexxo/clupilot` wird **nicht angefasst** — dort läuft die zweite Session. +- **Kein `migrate:fresh`, kein `db:wipe`, kein `--env=testing`.** Es gibt keine `.env.testing`; solche Befehle treffen die laufende Entwicklungsdatenbank. Die Suite migriert sich selbst nach SQLite. +- Code, Kommentare, Log-Meldungen und Commit-Botschaften **englisch**; Spec, Plan und Handoffs **deutsch**. Kommentare erklären das *Warum*. +- **Adoption darf niemals Geld verschieben.** Übernommen wird nur bei identischem Betrag, identischer Währung, identischem Intervall — und nach Task 1 auch nur bei `interval_count` 1, `usage_type` `licensed`, ohne `transform_quantity` und mit `billing_scheme` `per_unit`. +- **Absente Stripe-Felder sind Stripes Vorgaben**: `interval_count` 1, `usage_type` `licensed`, `transform_quantity` keine, `billing_scheme` `per_unit`. Andersherum gelesen würde jede Prüfung jeden legitimen Preis ablehnen und die Wiedererkennung still zu einem Nichts machen. +- **Ein Produkt wird nie stillgelegt.** Ein archivierter *Preis* ist nachweislich harmlos; ein stillgelegtes *Produkt* macht seine Preise unverkäuflich, und darauf können Verträge laufen. +- **Die `<= 0`-Schranke in `PlanPrices::ensure()` wird NICHT gebaut** — siehe Spec §3. Der Umzug des Geld-Tors nimmt ihr den Zweck, und sie würde einen kostenlosen Tarif unverkäuflich machen. +- Jede Task endet mit einem grünen `tests/Feature/Billing`, bevor committet wird. + +--- + +## Dateistruktur + +**Neu** + +| Datei | Verantwortung | +|---|---| +| `app/Services/Billing/AdoptStripeProduct.php` | Erkennt ein verwaistes Stripe-Produkt als unseres wieder. Kein Geld-Tor, kein Stilllegen — nur Metadaten-Beweis, ältestes gewinnt, Doppel werden gemeldet. | +| `app/Console/Commands/SweepOrphanStripePrices.php` | Listet aktive Preise an unseren Produkten, die keine Zeile kennt; auf Wunsch legt es sie still. | +| `tests/Feature/Billing/StripeProductAdoptionTest.php` | Die Produkt-Regeln und beide Verdrahtungen. | +| `tests/Feature/Billing/SweepOrphanPricesTest.php` | Bericht, `--archive`, `--dry-run`. | + +**Geändert** + +| Datei | Änderung | +|---|---| +| `app/Services/Stripe/StripeClient.php` | `activeProducts()`; Vertrag von `activePricesFor()` wird ehrlich | +| `app/Services/Stripe/HttpStripeClient.php` | Filter auflösen, vier Felder mitliefern, Produkte auflisten, Blättern wirft | +| `app/Services/Stripe/FakeStripeClient.php` | vier Felder in `createPrice`/`plantPrice`/`activePricesFor`, Metadaten zusammenführen und casten, `activeProducts()`, `plantProduct()`, `created` an Produkten | +| `app/Services/Billing/AdoptStripePrice.php` | vier Eigenschaften prüfen, Warnung drosseln, Gleichstand über die ID, Übernahmen zählen | +| `app/Services/Billing/AddonPrices.php` | Produkt-Wiedererkennung in `product()` | +| `app/Console/Commands/SyncStripeCatalogue.php` | Produkt-Wiedererkennung, getrennte Zählung | +| `app/Providers/AppServiceProvider.php` | beide Adoptions-Klassen als Singleton | +| `tests/Feature/Billing/StripeIdempotencyKeyTest.php`, `StripePriceAdoptionTest.php` | angehängt bzw. umgehängt | +| `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` | §14 abhaken | + +**Abhängigkeit:** Task 1 und 2 sind unabhängig. Task 3 ist unabhängig. Task 4 braucht 3. Task 5 braucht 4 (Singleton-Muster). Task 6 ist unabhängig, kommt aber zuletzt. + +--- + +### Task 1: Das Geld-Tor zieht dorthin, wo es zugesagt wird + +**Files:** +- Modify: `app/Services/Stripe/HttpStripeClient.php` (`activePricesFor()`, der Filterblock und das Rückgabe-Array) +- Modify: `app/Services/Stripe/FakeStripeClient.php` (`createPrice()`, `activePricesFor()`, `plantPrice()`) +- Modify: `app/Services/Stripe/StripeClient.php` (Vertrag von `activePricesFor()`) +- Modify: `app/Services/Billing/AdoptStripePrice.php` (Geld-Tor, Drosselung) +- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` (neu angehängt), `tests/Feature/Billing/StripeIdempotencyKeyTest.php` (bestehender Test umgestellt) + +**Interfaces:** +- Produces: `activePricesFor()` liefert je Preis zusätzlich + `interval_count: int`, `usage_type: string`, `transform_quantity: bool`, `billing_scheme: string`. + `FakeStripeClient::plantPrice()` bekommt vier optionale Parameter am Ende: + `int $intervalCount = 1, string $usageType = 'licensed', bool $transformQuantity = false, string $billingScheme = 'per_unit'`. + +- [ ] **Step 1: Write the failing test** + +An `tests/Feature/Billing/StripePriceAdoptionTest.php` anhängen: + +```php +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, 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(); +}); +``` + +In `tests/Feature/Billing/StripeIdempotencyKeyTest.php` **zwei bestehende Tests +durch einen ersetzen**. Es sind genau diese beiden, sie prüfen das Wegfiltern und +haben nach dem Umzug keinen Gegenstand mehr: + +- `it('skips a price it could not have created itself, so the money gate never trusts a partial recurrence', …)` (ab `:173`) +- `it('skips a price that would charge our figure for the wrong quantity', …)` (ab `:203`) + +An ihre Stelle tritt einer, der prüft, dass die Eigenschaften **mitgeliefert** +statt weggefiltert werden — das Ablehnen selbst prüfen jetzt die vier neuen +Tests in `StripePriceAdoptionTest.php` oben, also dort, wo die Zusage gemacht +wird: + +```php +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_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' => []], + ['id' => 'price_divided', 'unit_amount' => 3480, 'currency' => 'eur', + 'created' => 102, 'recurring' => ['interval' => 'month'], + '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 = collect((new HttpStripeClient)->activePricesFor('prod_1'))->keyBy('id'); + + // 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'); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +Expected: FAIL — `plantPrice()` kennt die vier benannten Argumente nicht (`Unknown named parameter $intervalCount`), und der umgestellte Client-Test bekommt nur einen Preis zurück statt vier. + +- [ ] **Step 3: `HttpStripeClient::activePricesFor()` filtert nicht mehr** + +Den ganzen `if (…) { continue; }`-Block samt seinem Kommentar **entfernen** und das Rückgabe-Array erweitern: + +```php + $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, + (array) ($price['metadata'] ?? []), + ), + ]; +``` + +- [ ] **Step 4: Den Vertrag ehrlich machen** + +`app/Services/Stripe/StripeClient.php`, Docblock von `activePricesFor()` — die Aufzählung der vier Eigenschaften bleibt, aber die Aussage dreht sich um: + +```php + /** + * Every active recurring Price of a Product, as Stripe holds them, with the + * properties that decide what each one CHARGES. + * + * `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. + * + * @return array}> + */ + public function activePricesFor(string $productId): array; +``` + +- [ ] **Step 5: Der Fake kann die vier Eigenschaften ausdrücken** + +In `FakeStripeClient::createPrice()` das gespeicherte Array um die vier Vorgaben ergänzen: + +```php + $this->prices[$id] = [ + 'product' => $productId, + '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, + 'created' => count($this->prices) + 1, + ]; +``` + +`plantPrice()` bekommt die vier als benannte Parameter mit denselben Vorgaben: + +```php + public function plantPrice( + string $id, + string $productId, + int $amountCents, + string $currency, + 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, + ]; + } +``` + +`activePricesFor()` im Fake gibt sie zurück; der Kommentarabsatz, der erklärt, +warum der Fake nicht filtert, wird **entfernt** — er beschreibt einen Zustand, +den es nicht mehr gibt: + +```php + $found[] = [ + 'id' => $id, + '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'], + ]; +``` + +Das `@var` von `$prices` (`:22`) um die vier Schlüssel ergänzen. + +- [ ] **Step 6: Das Geld-Tor in `AdoptStripePrice`** + +```php + 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_count'] !== 1 + || $price['usage_type'] !== 'licensed' + || $price['transform_quantity'] + || $price['billing_scheme'] !== 'per_unit') { + continue; + } +``` + +- [ ] **Step 7: Die Warnung wird gedrosselt** + +`use Illuminate\Support\Facades\Cache;` ergänzen und den Warn-Zweig umbauen: + +```php + 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 — undrosselt 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, + 'amount_cents' => $amountCents, + 'currency' => strtoupper($currency), + 'interval' => $interval, + ]); + } + + continue; + } +``` + +- [ ] **Step 8: Run the tests** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing +``` + +Expected: PASS. Fällt hier ein bestehender Test, ist das ein echter Fund über den Umzug — melden, nicht den Test aufweichen. + +- [ ] **Step 9: Commit** + +```bash +git add -- app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php app/Services/Stripe/StripeClient.php app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripePriceAdoptionTest.php tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +```bash +git commit -F - -- app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php app/Services/Stripe/StripeClient.php app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripePriceAdoptionTest.php tests/Feature/Billing/StripeIdempotencyKeyTest.php <<'MSG' +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 +MSG +``` + +--- + +### Task 2: Vier mechanische Korrekturen an Client und Fake + +**Files:** +- Modify: `app/Services/Stripe/FakeStripeClient.php` (`updatePriceMetadata()`, `activePricesFor()`) +- Modify: `app/Services/Stripe/HttpStripeClient.php` (`activePricesFor()`, Blätter-Schleife) +- Modify: `app/Services/Billing/AdoptStripePrice.php` (`usort`) +- Test: `tests/Feature/Billing/StripeIdempotencyKeyTest.php`, `tests/Feature/Billing/StripePriceAdoptionTest.php` + +**Interfaces:** +- Consumes: die vier Felder aus Task 1. +- Produces: nichts Neues nach außen. + +- [ ] **Step 1: Write the failing tests** + +An `tests/Feature/Billing/StripeIdempotencyKeyTest.php`: + +```php +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'); +}); +``` + +An `tests/Feature/Billing/StripePriceAdoptionTest.php`: + +```php +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']); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: FAIL — Metadaten werden ersetzt statt zusammengeführt, `42` bleibt Integer, das Blättern wirft nicht, und der Gleichstand fällt auf die Einfügereihenfolge. + +- [ ] **Step 3: Fake führt Metadaten zusammen** + +```php + public function updatePriceMetadata(string $priceId, array $metadata): void + { + if (isset($this->prices[$priceId])) { + // 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]; + } +``` + +- [ ] **Step 4: Fake castet Metadaten** + +Im Rückgabe-Array von `FakeStripeClient::activePricesFor()`: + +```php + // 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']), +``` + +- [ ] **Step 5: Blättern wirft, statt still abzubrechen** + +In `HttpStripeClient::activePricesFor()`, nach dem Setzen von `$after`: + +```php + $after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? 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); +``` + +`use RuntimeException;` steht bereits oben in der Datei. + +- [ ] **Step 6: Gleichstand über die Preis-ID** + +In `AdoptStripePrice`: + +```php + // 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']]); +``` + +Den Kommentar am `created`-Feld des Fakes berichtigen: er darf nicht mehr +behaupten, ein Gleichstand sei unmöglich — er ist möglich und wird jetzt +aufgelöst. + +- [ ] **Step 7: Run the tests** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing +``` + +Expected: PASS. + +- [ ] **Step 8: Commit** + +```bash +git add -- app/Services/Stripe/FakeStripeClient.php app/Services/Stripe/HttpStripeClient.php app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripeIdempotencyKeyTest.php tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +```bash +git commit -F - -- app/Services/Stripe/FakeStripeClient.php app/Services/Stripe/HttpStripeClient.php app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripeIdempotencyKeyTest.php tests/Feature/Billing/StripePriceAdoptionTest.php <<'MSG' +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 +MSG +``` + +--- + +### Task 3: Stripe nach seinen Produkten fragen + +**Files:** +- Modify: `app/Services/Stripe/StripeClient.php` (nach `activePricesFor()`) +- Modify: `app/Services/Stripe/HttpStripeClient.php` +- Modify: `app/Services/Stripe/FakeStripeClient.php` +- Test: `tests/Feature/Billing/StripeIdempotencyKeyTest.php` + +**Interfaces:** +- Produces: + `StripeClient::activeProducts(): array` — Liste von + `array{id: string, name: string, created: int, metadata: array}`. + `FakeStripeClient::plantProduct(string $id, string $name, array $metadata = [], int $created = 1): void`. + `FakeStripeClient::$products` bekommt zusätzlich `created: int`. + +- [ ] **Step 1: Write the failing test** + +```php +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'); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +Expected: FAIL — `Call to undefined method App\Services\Stripe\HttpStripeClient::activeProducts()`. + +- [ ] **Step 3: Den Vertrag ergänzen** + +`app/Services/Stripe/StripeClient.php`, nach `activePricesFor()`: + +```php + /** + * 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}> + */ + public function activeProducts(): array; +``` + +- [ ] **Step 4: Im `HttpStripeClient` umsetzen** + +```php + 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; + } +``` + +- [ ] **Step 5: Im `FakeStripeClient` umsetzen** + +`createProduct()` stempelt einen Zähler mit: + +```php + $id = 'prod_'.substr(sha1($name.count($this->products)), 0, 12); + $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, + ]; +``` + +Das `@var` von `$products` entsprechend erweitern. Dazu: + +```php + 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]; + } +``` + +- [ ] **Step 6: Run the tests** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing +``` + +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add -- app/Services/Stripe/StripeClient.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +```bash +git commit -F - -- app/Services/Stripe/StripeClient.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php <<'MSG' +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 +MSG +``` + +--- + +### Task 4: `AdoptStripeProduct` und beide Verdrahtungen + +**Files:** +- Create: `app/Services/Billing/AdoptStripeProduct.php` +- Modify: `app/Console/Commands/SyncStripeCatalogue.php` (Produkt-Anlage der Familie) +- Modify: `app/Services/Billing/AddonPrices.php` (`product()`) +- Modify: `app/Providers/AppServiceProvider.php` (Singleton-Bindung) +- Test: `tests/Feature/Billing/StripeProductAdoptionTest.php` + +**Interfaces:** +- Consumes: `StripeClient::activeProducts()` aus Task 3, `FakeStripeClient::plantProduct()`. +- Produces: + ```php + public function __invoke(array $metadata, array $identifying): ?string + ``` + Gibt die ID eines zu übernehmenden Produkts zurück oder `null`. + `public array $duplicates = []` — Produkt-IDs, die als Doppel erkannt und **nicht** angefasst wurden, für die Ausgabe des Kommandos. + In `AppServiceProvider::register()` als **Singleton** gebunden, damit `$duplicates` über einen ganzen Lauf trägt. + +- [ ] **Step 1: Write the failing test** + +`tests/Feature/Billing/StripeProductAdoptionTest.php`: + +```php +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(); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing/StripeProductAdoptionTest.php +``` + +Expected: FAIL — `Target class [App\Services\Billing\AdoptStripeProduct] does not exist.` + +- [ ] **Step 3: `AdoptStripeProduct` schreiben** + +```php + + */ + 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 $metadata what the create call would send + * @param array $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. + 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 $found + * @param array $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 $found + * @param array $expected + * @param array $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; + } +} +``` + +- [ ] **Step 4: Als Singleton binden** + +In `app/Providers/AppServiceProvider::register()`, bei den anderen Bindungen: + +```php + // Singletons because both carry a record across one run that the caller + // reads afterwards: which products were duplicates, and how many prices + // were adopted rather than created. Resolved per call they would each + // start empty — SyncStripeCatalogue asks the container for PlanPrices + // once per catalogue row. + $this->app->singleton(AdoptStripeProduct::class); +``` + +`use App\Services\Billing\AdoptStripeProduct;` oben ergänzen. + +- [ ] **Step 5: Die Familie fragt zuerst** + +`SyncStripeCatalogue::handle()`, im Block `if ($productId === null)`: + +```php + if (! $dryRun) { + $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 = app(AdoptStripeProduct::class)($metadata, ['plan_family_id']); + + $productId ??= $stripe->createProduct( + $family->name, + $metadata, + // Covers a RETRY, for twenty-four hours, and nothing + // 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]); + } +``` + +- [ ] **Step 6: Das Modul fragt zuerst** + +`AddonPrices::product()`: + +```php + private function product(string $addonKey): string + { + $existing = StripeAddonPrice::query() + ->where('addon_key', $addonKey) + ->value('stripe_product_id'); + + if (is_string($existing) && $existing !== '') { + return $existing; + } + + $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), + $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}", + ); + } +``` + +- [ ] **Step 7: Run the tests** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing +``` + +Expected: PASS. `ReverseChargePriceTest` ist auch hier der Zeuge: es zählt nach einem doppelten Sync 16 Register-Zeilen und 16 verschiedene Preis-IDs. + +- [ ] **Step 8: Commit** + +```bash +git add -- app/Services/Billing/AdoptStripeProduct.php app/Providers/AppServiceProvider.php app/Console/Commands/SyncStripeCatalogue.php app/Services/Billing/AddonPrices.php tests/Feature/Billing/StripeProductAdoptionTest.php +``` + +```bash +git commit -F - -- app/Services/Billing/AdoptStripeProduct.php app/Providers/AppServiceProvider.php app/Console/Commands/SyncStripeCatalogue.php app/Services/Billing/AddonPrices.php tests/Feature/Billing/StripeProductAdoptionTest.php <<'MSG' +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 +MSG +``` + +--- + +### Task 5: Der Abgleich zählt „angelegt" und „übernommen" getrennt + +**Files:** +- Modify: `app/Services/Billing/AdoptStripePrice.php` (Zähler) +- Modify: `app/Providers/AppServiceProvider.php` (Singleton) +- Modify: `app/Console/Commands/SyncStripeCatalogue.php` (Zählung und Schlusszeile, Doppel-Produkte melden) +- Test: `tests/Feature/Billing/StripeProductAdoptionTest.php` + +**Interfaces:** +- Consumes: `AdoptStripeProduct::$duplicates` aus Task 4. +- Produces: `AdoptStripePrice::$adoptions` (`int`), als Singleton gebunden. + +- [ ] **Step 1: Write the failing test** + +An `tests/Feature/Billing/StripeProductAdoptionTest.php`: + +```php +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(); + $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); + $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 () { + $family = PlanFamily::query()->whereNotNull('stripe_product_id')->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'); +}); +``` + +Die zusätzlichen `use`-Zeilen (`App\Models\PlanPrice`, `App\Models\StripePlanPrice`, `App\Services\Billing\PlanPrices`) oben in der Datei ergänzen. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing/StripeProductAdoptionTest.php +``` + +Expected: FAIL — die Ausgabe sagt „created", und die Doppel-Produkt-ID steht nirgends. + +- [ ] **Step 3: `AdoptStripePrice` zählt seine Übernahmen** + +Eigenschaft und Inkrement: + +```php + /** + * 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; +``` + +und, unmittelbar vor dem `return $adopted['id'];`: + +```php + $this->adoptions++; +``` + +- [ ] **Step 4: Als Singleton binden** + +In `AppServiceProvider::register()`, neben der Bindung aus Task 4: + +```php + $this->app->singleton(AdoptStripePrice::class); +``` + +`use App\Services\Billing\AdoptStripePrice;` oben ergänzen. + +- [ ] **Step 5: Das Kommando zählt und meldet getrennt** + +In `SyncStripeCatalogue::handle()`, ganz am Anfang von `handle()`: + +```php + $adoptPrices = app(AdoptStripePrice::class); + $adoptProducts = app(AdoptStripeProduct::class); + $adoptedBefore = $adoptPrices->adoptions; +``` + +und die Schlussausgabe ersetzen: + +```php + $adopted = $adoptPrices->adoptions - $adoptedBefore; + $minted = max(0, $created - $adopted); + + foreach ($adoptProducts->duplicates 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; + } + + $this->info($dryRun + ? "{$created} object(s) would be created. Run without --dry-run to create them." + : "{$minted} object(s) created, {$adopted} adopted in Stripe."); + + return self::SUCCESS; +``` + +`use App\Services\Billing\AdoptStripePrice;` und `use App\Services\Billing\AdoptStripeProduct;` oben ergänzen. + +- [ ] **Step 6: Run the tests** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing +``` + +Expected: PASS. `ReverseChargePriceTest:335` prüft `'already in step'` — diese Zeile bleibt unverändert. + +- [ ] **Step 7: Commit** + +```bash +git add -- app/Services/Billing/AdoptStripePrice.php app/Providers/AppServiceProvider.php app/Console/Commands/SyncStripeCatalogue.php tests/Feature/Billing/StripeProductAdoptionTest.php +``` + +```bash +git commit -F - -- app/Services/Billing/AdoptStripePrice.php app/Providers/AppServiceProvider.php app/Console/Commands/SyncStripeCatalogue.php tests/Feature/Billing/StripeProductAdoptionTest.php <<'MSG' +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 +MSG +``` + +--- + +### Task 6: `stripe:sweep-orphan-prices` + +**Files:** +- Create: `app/Console/Commands/SweepOrphanStripePrices.php` +- Test: `tests/Feature/Billing/SweepOrphanPricesTest.php` +- Modify: `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` (§14 abhaken) + +**Interfaces:** +- Consumes: `StripeClient::activePricesFor()`, `archivePrice()`. +- Produces: das Kommando `stripe:sweep-orphan-prices {--archive} {--dry-run}`. + +- [ ] **Step 1: Write the failing test** + +`tests/Feature/Billing/SweepOrphanPricesTest.php`: + +```php +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([]); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing/SweepOrphanPricesTest.php +``` + +Expected: FAIL — `The command "stripe:sweep-orphan-prices" does not exist.` + +- [ ] **Step 3: Das Kommando schreiben** + +`app/Console/Commands/SweepOrphanStripePrices.php`: + +```php +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(); + } +} +``` + +- [ ] **Step 4: Run the tests** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test tests/Feature/Billing/SweepOrphanPricesTest.php +``` + +Expected: PASS, vier Tests. + +- [ ] **Step 5: §14 des Vorgängers abhaken** + +In `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` §14 wird +Punkt für Punkt abgehakt, jeweils mit der Datei, die ihn schließt: + +| §14-Eintrag | Ergebnis | +|---|---| +| Waisen-Produkte | **erledigt** — `AdoptStripeProduct`, beide Verdrahtungen | +| Geld-Tor an der falschen Stelle | **erledigt** — geprüft jetzt in `AdoptStripePrice` | +| Null-Schranke in `PlanPrices` | **bewusst nicht gebaut** — Begründung in der neuen Spec §3: der Umzug nimmt ihr den Zweck, und sie würde einen kostenlosen Tarif unverkäuflich machen | +| Abgleich zählt „angelegt" für Übernommenes | **erledigt** — getrennte Zählung | +| Warnung ohne Drosselung | **erledigt** — einmal pro Preis und Tag | +| Fake ersetzt Metadaten / castet nicht | **erledigt** | +| `created` kann gleichstehen | **erledigt** — Gleichstand über die Preis-ID | +| Blättern bricht still ab | **erledigt** — wirft, für Preise und Produkte | +| Fingerabdruck und POST-Rumpf müssen zusammenbleiben | **bleibt offen** — es ist eine Naht, kein Fehler; der Test darüber existiert | +| `HostStepsTest` (DNS-Zone) | **erledigt durch die zweite Session** (`a842512`), Eintrag streichen | +| Sprunghafter `PlanCatalogueTest` | **bleibt offen** — Ursachensuche, eigene Sitzung | + +Der Abschnitt bekommt einen Kopfsatz, der sagt, wo die Fortsetzung steht: +`docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md`. + +- [ ] **Step 6: Voller Testlauf** + +```bash +docker exec -w /var/www/html/.claude/worktrees/cool-sammet-368fee clupilot-app-1 php artisan test +``` + +Expected: PASS. Sollte etwas außerhalb von `tests/Feature/Billing` fallen, ist das ein echter Fund — melden. + +- [ ] **Step 7: Commit** + +```bash +git add -- app/Console/Commands/SweepOrphanStripePrices.php tests/Feature/Billing/SweepOrphanPricesTest.php docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md +``` + +```bash +git commit -F - -- app/Console/Commands/SweepOrphanStripePrices.php tests/Feature/Billing/SweepOrphanPricesTest.php docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md <<'MSG' +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 +MSG +``` + +--- + +## Nach dem letzten Task + +- [ ] **Voller Testlauf im Worktree**, nicht im Haupt-Repo. +- [ ] **Ein Review über den Gesamtdiff**, dann **eine** Fix-Runde, dann **ein** Re-Review über den Fix-Diff (R22.1). Danach werden offene Befunde mit schriftlicher Begründung geparkt. +- [ ] **Nicht mergen, solange die zweite Session arbeitet.** Anweisung des Nutzers vom 2026-07-30. Der Merge nach `main` wartet, bis das Haupt-Repo einen sauberen Baum hat. 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 47d2eba..f246df0 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 @@ -103,10 +103,23 @@ einen vierteljährlichen oder mengenteilenden Preis überhaupt hinstellen kann. wiederkehrende Preise **mit** ihren preisbestimmenden Eigenschaften; was davon zulässig ist, entscheidet der Aufrufer. -**Verwandt, hier mit erledigt:** `PlanPrices::ensure()` bekommt die -`<= 0`-Schranke, die `AddonPrices::ensure()` schon hat. Sie 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. +### 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 @@ -171,7 +184,9 @@ Ohne Entwurfsentscheidung — bauen, Test, weiter (R22.5): (`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 — siehe §3. +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 @@ -234,7 +249,8 @@ Und an `StripeIdempotencyKeyTest.php`: Blättern **wirft** bei fehlender `id` Warnung drosseln - `app/Services/Billing/AddonPrices.php` — Produkt-Wiedererkennung in `product()` -- `app/Services/Billing/PlanPrices.php` — `<= 0`-Schranke +- `app/Providers/AppServiceProvider.php` — beide Adoptions-Klassen als + Singleton, damit ihr Lauf-Protokoll (`$duplicates`, `$adoptions`) trägt - `app/Console/Commands/SyncStripeCatalogue.php` — Produkt-Wiedererkennung, getrennte Zählung, Kommentar berichtigt - `tests/Feature/Billing/StripePriceAdoptionTest.php`,