From 7d66a01a405df9b5642b0c1b878971a004e3e3bf Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 16:56:08 +0200 Subject: [PATCH] Break the follow-ups into six testable steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six tasks, each ending on a green billing folder and its own commit: move the money gate into the class that promises it, four fixes that make the fake behave like Stripe, ask Stripe for its products, recognise an orphaned one, count adopted apart from created, and hand the operator a sweep for the orphans nobody can adopt. Self-review dropped one item the spec had carried over from the last review: the zero guard in PlanPrices::ensure(). Its only purpose was to keep a tiered price unreachable, and the gate move rejects those directly — so it would be a behaviour change with no remaining benefit, and it would make a free plan unsellable. Recorded as deliberately not built, with the reason, rather than quietly dropped. Also corrected: a test that counted every product where the run mints one per family, and a step that named the test it replaces only by description. Co-Authored-By: Claude Opus 5 --- .../2026-07-30-stripe-adoption-followups.md | 1680 +++++++++++++++++ ...-07-30-stripe-adoption-followups-design.md | 28 +- 2 files changed, 1702 insertions(+), 6 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-30-stripe-adoption-followups.md 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`,