diff --git a/docs/superpowers/plans/2026-07-30-stripe-price-adoption.md b/docs/superpowers/plans/2026-07-30-stripe-price-adoption.md new file mode 100644 index 0000000..5bfa20f --- /dev/null +++ b/docs/superpowers/plans/2026-07-30-stripe-price-adoption.md @@ -0,0 +1,1839 @@ +# Stripe Price Adoption — 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:** Ein Abgleich, der zwischen Stripes Anlage und unserem Schreiben abbricht, erkennt den vorhandenen Preis beim nächsten Lauf wieder statt einen zweiten anzulegen — und eine Änderung an den Metadaten von `createPrice` blockiert Abgleich und Kundenbuchung nie wieder mit HTTP 400. + +**Architecture:** Zwei neue Bauteile. `App\Services\Stripe\IdempotencyKey` faltet einen Fingerabdruck der tatsächlich gesendeten Parameter in den Schlüssel — angewandt **nur** auf `createPrice`/`createProduct`, nie auf Geldbewegungen. `App\Services\Billing\AdoptStripePrice` fragt Stripe vor dem Anlegen nach den aktiven Preisen des Produkts und übernimmt einen beweisbar eigenen. Beide Preis-Dienste (`AddonPrices`, `PlanPrices`) bekommen den Wiedererkennungsschritt zwischen Tabellen-Blick und `createPrice`. Dazu ein eindeutiger Index auf `stripe_addon_prices.stripe_price_id` als Netz darunter. + +**Tech Stack:** Laravel 11, Pest, Livewire (nicht berührt), Stripe REST über `Illuminate\Http\Client`, MariaDB in Produktion / SQLite `:memory:` in Tests. + +**Spec:** `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md` + +## Global Constraints + +- **Commit-Disziplin (Nutzervorgabe, nicht verhandelbar):** Eine zweite Session arbeitet im selben Git-Index. **Immer** `git add -- ` und `git commit -F - -- `. **Nie** `git add -A`, `git add .`, `git commit -a`. +- **Sprache:** Code, Kommentare, Log-Meldungen und Commit-Botschaften **englisch**. Spec/Plan/Handoff-Dokumente **deutsch**. Kommentare erklären das *Warum*, nicht das *Was* — das ist die Hausschrift dieses Repos, siehe die vorhandenen Klassenkommentare in `app/Services/Billing/`. +- **Bestehende Verträge behalten ihre alten Preise.** Neue Preise gelten nur für neu abschließende Kunden. Keine Änderung dieses Plans darf einen laufenden Vertrag auf einen anderen Betrag bewegen. +- **Jede Buchung friert ihren Preis ein.** Ein später gebuchtes Modul kommt zum heutigen Preis; eine Stornierung zieht den damaligen Betrag ab. +- **Der Fingerabdruck ist ausschließlich für `createPrice` und `createProduct`.** Bei `refund`, `cancelSubscription`, `addSubscriptionItem` und `createCheckoutSession` muss Stripes HTTP 400 bei geänderten Parametern erhalten bleiben — dort ist ein Duplikat doppelt abgebuchtes Kundengeld. +- **`TaxTreatment` bleibt die einzige Steuerinstanz.** `automatic_tax` bleibt aus. +- **Kein Testlauf mit `--filter` als Abschluss:** jede Task endet mit dem vollen Billing-Ordner (`php artisan test --testsuite=Feature tests/Feature/Billing`), bevor committet wird. +- **Zahlen für Tests, aus dem echten Katalog:** `priority_support` = `2900` Netto (`config/provisioning.php:308`), Steuersatz 20 % → `3480` verrechnet monatlich, `34800` Netto/`41760` verrechnet jährlich. `3480 EUR monatlich` ist genau der Preis aus dem Vorfall (`price_1TygdEC7u8NpJ8pOt3nsoyYw`). + +--- + +## Dateistruktur + +**Neu** + +| Datei | Verantwortung | +|---|---| +| `app/Services/Stripe/IdempotencyKey.php` | Bildet Header-Werte: sprechender Schlüssel + Fingerabdruck der Aufrufparameter. Die **einzige** Stelle, an der ein Fingerabdruck entsteht — beide Clients müssen denselben Wert bilden. | +| `app/Services/Billing/AdoptStripePrice.php` | Der Wiedererkennungsschritt und seine fünf Bedingungen. Weiß nichts über Module oder Pakete; bekommt Produkt, Betrag, Metadaten und eine Frage „ist diese ID schon beansprucht?" gestellt. | +| `database/migrations/2026_07_31_210000_one_row_per_stripe_price.php` | Entdoppelt `stripe_addon_prices` und macht `stripe_price_id` eindeutig. | +| `tests/Feature/Billing/StripeIdempotencyKeyTest.php` | Was auf der Leitung landet (`Http::fake` gegen `HttpStripeClient`) und dass der Fake Stripe nachbildet. | +| `tests/Feature/Billing/StripePriceAdoptionTest.php` | Die Übernahmeregeln, beide Seiten, plus der eindeutige Index und die eingefrorene Buchung. | + +**Geändert** + +| Datei | Änderung | +|---|---| +| `app/Services/Stripe/StripeClient.php` | Zwei Methoden am Vertrag: `activePricesFor()`, `updatePriceMetadata()`. | +| `app/Services/Stripe/HttpStripeClient.php` | Beide Methoden umgesetzt; Fingerabdruck an `createPrice`/`createProduct`. | +| `app/Services/Stripe/FakeStripeClient.php` | Beide Methoden; `created` an den Preisen; `plantPrice()` für Tests; Schlüssel-Ledger vergleicht Parameter. | +| `app/Services/Billing/AddonPrices.php` | Wiedererkennung vor `createPrice`; Schlüssel-Kommentar berichtigt. | +| `app/Services/Billing/PlanPrices.php` | Dasselbe. | +| `docs/handoffs/2026-07-30-real-run-handoff.md` | Block-D-Punkt zur Eindeutigkeit als erledigt markieren. | + +**Reihenfolge und Abhängigkeit:** Task 1 (Schlüssel) und Task 2 (Client-Methoden) sind unabhängig voneinander. Task 3 braucht Task 2. Task 4 und 5 brauchen Task 3. Task 6 ist unabhängig, kommt aber zuletzt, weil ihr Test die Übernahme aus Task 4 mitprüft. + +--- + +### Task 1: Der Idempotenz-Schlüssel trägt, was gesendet wird + +**Files:** +- Create: `app/Services/Stripe/IdempotencyKey.php` +- Modify: `app/Services/Stripe/HttpStripeClient.php:87-121` (`createProduct`, `createPrice`) +- Modify: `app/Services/Stripe/FakeStripeClient.php:37-38` (Ledger-Eigenschaft), `:151-193` (`createProduct`, `createPrice`), `:313`, `:337-350` (`cancelSubscription`, `refund`) +- Test: `tests/Feature/Billing/StripeIdempotencyKeyTest.php` + +**Interfaces:** +- Consumes: nichts. +- Produces: + - `IdempotencyKey::forPrice(?string $key, string $productId, int $amountCents, string $currency, string $interval, array $metadata): ?string` + - `IdempotencyKey::forProduct(?string $key, string $name, array $metadata): ?string` + - `IdempotencyKey::fingerprint(array $parameters): string` — acht Hex-Zeichen + - `FakeStripeClient::$keys` wechselt von `array` zu `array`. **Kein Test liest die Eigenschaft** (geprüft), nur der Fake selbst an vier Stellen. + +- [ ] **Step 1: Write the failing test** + +`tests/Feature/Billing/StripeIdempotencyKeyTest.php`: + +```php +set('services.stripe.secret', 'sk_test_plan_task_one'); +}); + +it('sends a different key once the metadata changes', function () { + Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]); + + $client = new HttpStripeClient; + $spoken = 'clupilot-addon-price-priority_support-month-3480-EUR'; + + // The call as it stood before 9da1358, and the call after it: same money, + // same interval, one metadata field more. + $client->createPrice('prod_1', 3480, 'EUR', 'month', + ['addon' => 'priority_support'], $spoken); + + $client->createPrice('prod_1', 3480, 'EUR', 'month', + ['addon' => 'priority_support', 'tax_treatment' => 'domestic'], $spoken); + + $sent = collect(Http::recorded()) + ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) + ->all(); + + expect($sent[0])->toStartWith($spoken) + ->and($sent[1])->toStartWith($spoken) + ->and($sent[1])->not->toBe($sent[0]); +}); + +it('sends the same key for the very same call', function () { + Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]); + + $client = new HttpStripeClient; + + foreach ([1, 2] as $ignored) { + $client->createPrice('prod_1', 3480, 'EUR', 'month', + ['addon' => 'priority_support'], 'clupilot-addon-price'); + } + + $sent = collect(Http::recorded()) + ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) + ->unique() + ->all(); + + expect($sent)->toHaveCount(1); +}); + +it('fingerprints the product call too, where the same trap was waiting', function () { + Http::fake(['api.stripe.com/*' => Http::response(['id' => 'prod_x'])]); + + $client = new HttpStripeClient; + + $client->createProduct('Priority Support', ['addon' => 'priority_support'], 'clupilot-addon-product-x'); + $client->createProduct('Priority Support', ['addon' => 'priority_support', 'sold_as' => 'entitlement'], 'clupilot-addon-product-x'); + + $sent = collect(Http::recorded()) + ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) + ->all(); + + expect($sent[1])->not->toBe($sent[0]); +}); + +it('leaves the money calls their bare key, so Stripe still refuses a changed one', function () { + Http::fake(['api.stripe.com/*' => Http::response(['id' => 'x'])]); + + $client = new HttpStripeClient; + + $client->refund('pi_1', 500, 'clupilot-refund-7'); + $client->cancelSubscription('sub_1', 'at_period_end', 'clupilot-cancel-7'); + $client->addSubscriptionItem('sub_1', 'price_1', 1, 'none', 'clupilot-item-7'); + + $sent = collect(Http::recorded()) + ->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null) + ->all(); + + expect($sent)->toBe(['clupilot-refund-7', 'clupilot-cancel-7', 'clupilot-item-7']); +}); + +it('reproduces the refusal Stripe makes, which the fake used to swallow', function () { + $fake = new FakeStripeClient; + + $fake->refund('pi_1', 500, 'clupilot-refund-7'); + + // Same key, different amount. Stripe answers 400; the fake said nothing and + // replayed the first refund's id, which is how a test could pass over the + // very failure that stopped production. + expect(fn () => $fake->refund('pi_1', 900, 'clupilot-refund-7')) + ->toThrow(RuntimeException::class, 'same parameters'); +}); + +it('mints a second price rather than blocking when the metadata moved', function () { + $fake = new FakeStripeClient; + + $first = $fake->createPrice('prod_1', 3480, 'EUR', 'month', + ['addon' => 'priority_support'], 'clupilot-addon-price'); + + $second = $fake->createPrice('prod_1', 3480, 'EUR', 'month', + ['addon' => 'priority_support', 'tax_treatment' => 'domestic'], 'clupilot-addon-price'); + + // Two objects, no exception. That the second one is not WANTED is the job of + // AdoptStripePrice, not of the key — see StripePriceAdoptionTest. + expect($second)->not->toBe($first); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +Expected: FAIL — `Class "App\Services\Stripe\IdempotencyKey" not found` erscheint noch nicht, weil der Test die Klasse nicht direkt anfasst; stattdessen schlagen die Erwartungen fehl (`$sent[1]` **ist** gleich `$sent[0]`), und der Refund-Test schlägt fehl, weil der Fake nicht wirft. + +- [ ] **Step 3: Write `IdempotencyKey`** + +`app/Services/Stripe/IdempotencyKey.php`: + +```php + $productId, + 'unit_amount' => $amountCents, + 'currency' => strtolower($currency), + 'interval' => $interval, + 'metadata' => $metadata, + ]); + } + + /** The key for a Product, for the same reason and against the same trap. */ + public static function forProduct(?string $key, string $name, array $metadata): ?string + { + return self::with($key, ['name' => $name, 'metadata' => $metadata]); + } + + /** + * Eight hex characters over the parameters, canonically ordered. + * + * Ordered, because PHP keeps insertion order and two callers writing the + * same metadata in a different order would otherwise send two keys for one + * call — which would mint two Prices and be a worse bug than the one this + * closes. + */ + public static function fingerprint(array $parameters): string + { + return substr(sha1(self::canonical($parameters)), 0, 8); + } + + private static function with(?string $key, array $parameters): ?string + { + // Null stays null: a caller who sends no key wants no replay, and + // inventing one here would change what the call means. + return $key === null ? null : $key.'-'.self::fingerprint($parameters); + } + + private static function canonical(array $parameters): string + { + ksort($parameters); + + foreach ($parameters as $name => $value) { + $parameters[$name] = is_array($value) ? self::canonical($value) : (string) $value; + } + + return json_encode($parameters, JSON_THROW_ON_ERROR); + } +} +``` + +- [ ] **Step 4: Wire it into `HttpStripeClient`** + +`app/Services/Stripe/HttpStripeClient.php` — `createProduct` (ab `:87`): + +```php + public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string + { + return (string) $this->request(IdempotencyKey::forProduct($idempotencyKey, $name, $metadata)) + ->asForm() + ->post($this->url('products'), array_filter([ + 'name' => $name, + ...$this->flatten('metadata', $metadata), + ])) + ->throw() + ->json('id'); + } +``` + +`createPrice` (ab `:99`) — nur die erste Zeile des `return` ändert sich: + +```php + return (string) $this->request(IdempotencyKey::forPrice( + $idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata, + )) + ->asForm() + ->post($this->url('prices'), [ +``` + +Und den Kommentar in `request()` (`:344-351`) ergänzen — er behauptet heute, der Schlüssel schütze einen Retry, und verschweigt, dass er einen geänderten Aufruf abwürgt: + +```php + // Stripe replays the original response for a repeated key instead of + // creating a second object — but ONLY for a call repeated exactly; a + // changed parameter under a used key is HTTP 400 for twenty-four hours. + // The catalogue calls therefore fold a fingerprint of their parameters + // into the key (see IdempotencyKey); the money calls deliberately do + // not. Their keys expire after 24 hours, so this protects a retry, not + // a sync re-run next week — for which the stored ids and + // AdoptStripePrice are the guard. +``` + +`use App\Services\Stripe\IdempotencyKey;` ist **nicht** nötig — dieselbe Namespace. + +- [ ] **Step 5: Make the fake faithful** + +`app/Services/Stripe/FakeStripeClient.php` — Eigenschaft ersetzen (`:37-38`): + +```php + /** + * Idempotency key → what was first answered for it, and a fingerprint of + * what it was first sent with. + * + * The fingerprint is the half that was missing. This ledger replayed the + * first id for a repeated key without ever looking at what the second call + * was asking for — so it was unrealistic in precisely the way that mattered, + * and no test could see the 2026-07-29 blockade. + * + * @var array + */ + public array $keys = []; +``` + +Zwei private Helfer am Ende der Klasse, neben `failIfAsked()`: + +```php + /** + * The id Stripe already answered for this key, or null when it is new. + * + * Throws on a key that comes back with different parameters, which is what + * Stripe does — with this wording — and what makes a test able to see the + * failure at all. + */ + private function replay(?string $key, array $parameters): ?string + { + if ($key === null || ! isset($this->keys[$key])) { + return null; + } + + if ($this->keys[$key]['fingerprint'] !== IdempotencyKey::fingerprint($parameters)) { + throw new RuntimeException( + 'Keys for idempotent requests can only be used with the same parameters they were first used with.', + ); + } + + return $this->keys[$key]['id']; + } + + private function rememberKey(?string $key, string $id, array $parameters): void + { + if ($key !== null) { + $this->keys[$key] = ['id' => $id, 'fingerprint' => IdempotencyKey::fingerprint($parameters)]; + } + } +``` + +`createProduct` (`:151-166`) — Schlüssel **mit Fingerabdruck**, genau wie der HTTP-Client, sonst prüft der Test etwas anderes als die Produktion sendet: + +```php + public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string + { + $key = IdempotencyKey::forProduct($idempotencyKey, $name, $metadata); + $parameters = ['name' => $name, 'metadata' => $metadata]; + + // Replays the first answer for a repeated key, as Stripe does. + $replayed = $this->replay($key, $parameters); + + if ($replayed !== null) { + return $replayed; + } + + $id = 'prod_'.substr(sha1($name.count($this->products)), 0, 12); + $this->products[$id] = ['name' => $name, 'metadata' => $metadata]; + + $this->rememberKey($key, $id, $parameters); + + return $id; + } +``` + +`createPrice` (`:168-194`) — dasselbe Muster, plus `created` am gespeicherten Preis (Task 3 sortiert danach): + +```php + public function createPrice( + string $productId, + int $amountCents, + string $currency, + string $interval, + array $metadata = [], + ?string $idempotencyKey = null, + ): string { + $key = IdempotencyKey::forPrice($idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata); + $parameters = [ + 'product' => $productId, + 'unit_amount' => $amountCents, + 'currency' => strtolower($currency), + 'interval' => $interval, + 'metadata' => $metadata, + ]; + + $replayed = $this->replay($key, $parameters); + + if ($replayed !== null) { + return $replayed; + } + + $id = 'price_'.substr(sha1($productId.$amountCents.$interval.count($this->prices)), 0, 12); + $this->prices[$id] = [ + 'product' => $productId, + 'amount' => $amountCents, + 'currency' => $currency, + 'interval' => $interval, + 'metadata' => $metadata, + // Stripe stamps every object with its creation time and + // AdoptStripePrice takes the OLDEST of several orphans. A counter + // is enough and beats a clock: it cannot tie. + 'created' => count($this->prices) + 1, + ]; + + $this->rememberKey($key, $id, $parameters); + + return $id; + } +``` + +Das `@var` der Eigenschaft `$prices` (`:22`) um `created: int` ergänzen. + +`cancelSubscription` (`:313`, `:324`) und `refund` (`:337`, `:350`) auf `replay()`/`rememberKey()` umstellen — **ohne** `IdempotencyKey`, mit dem rohen Schlüssel: + +```php + // No fingerprint: for a refund, a changed parameter under a used key is + // exactly what must fail loudly. See IdempotencyKey. + $parameters = ['payment' => $paymentReference, 'amount' => (string) $amountCents]; + $replayed = $this->replay($idempotencyKey, $parameters); + + if ($replayed !== null) { + return $replayed; + } +``` + +und nach dem Anlegen `$this->rememberKey($idempotencyKey, $id, $parameters);`. Für `cancelSubscription` sind die Parameter `['subscription' => $subscriptionId, 'when' => $when]`, die gemerkte „id" bleibt `$subscriptionId` wie heute. + +- [ ] **Step 6: Run the test to verify it passes** + +```bash +php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +Expected: PASS, sieben Tests. + +- [ ] **Step 7: Run the whole billing folder — the fake is used by a dozen files** + +```bash +php artisan test tests/Feature/Billing +``` + +Expected: PASS. Wenn hier etwas fällt, ist es ein Test, der einen Schlüssel zweimal mit verschiedenen Parametern schickte und das bisher nicht merkte — **das ist ein echter Fund**, nicht ein zu unterdrückender Testfehler: den Aufrufer korrigieren, nicht den Fake. + +- [ ] **Step 8: Commit** + +```bash +git add -- app/Services/Stripe/IdempotencyKey.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +```bash +git commit -F - -- app/Services/Stripe/IdempotencyKey.php app/Services/Stripe/HttpStripeClient.php app/Services/Stripe/FakeStripeClient.php tests/Feature/Billing/StripeIdempotencyKeyTest.php <<'MSG' +Put in the key everything the call actually sends + +A key says "I already sent this call", not "this is what the object is". The +catalogue calls sent metadata that the key knew nothing about, so adding the +tax_treatment field in 9da1358 poisoned yesterday's key for a day — and +AddonPrices::ensure() runs inside a customer's module booking, not only in the +sweep. + +createPrice and createProduct now fold a fingerprint of their parameters into +the key. refund, cancelSubscription, addSubscriptionItem and the checkout +deliberately do not: there a second object is the customer's money taken twice, +and Stripe's refusal is the thing worth keeping. + +The fake could not see any of this. Its ledger replayed the first id for a +repeated key without ever comparing what the second call asked for. + +Co-Authored-By: Claude Opus 5 +MSG +``` + +--- + +### Task 2: Stripe fragen, was es schon hat + +**Files:** +- Modify: `app/Services/Stripe/StripeClient.php:118-140` (zwischen `createPrice` und `archivePrice`) +- Modify: `app/Services/Stripe/HttpStripeClient.php` (nach `activatePrice`, `:143`) +- Modify: `app/Services/Stripe/FakeStripeClient.php` (nach `activatePrice`, `:201-212`) +- Test: `tests/Feature/Billing/StripeIdempotencyKeyTest.php` (zwei Tests angehängt — dieselbe Datei, weil es wieder um das geht, was auf der Leitung landet) + +**Interfaces:** +- Consumes: nichts aus Task 1. +- Produces: + - `StripeClient::activePricesFor(string $productId): array` — Liste von + `array{id: string, unit_amount: int, currency: string, interval: string, created: int, metadata: array}`. + **`currency` kommt GROSSGESCHRIEBEN zurück** (Stripe liefert klein; unsere Tabellen halten groß), `interval` ist `month`/`year`. + - `StripeClient::updatePriceMetadata(string $priceId, array $metadata): void` + - `FakeStripeClient::plantPrice(string $id, string $productId, int $amountCents, string $currency, string $interval, array $metadata, int $created = 1): void` + - `FakeStripeClient::$metadataUpdates` — `array}>` + +- [ ] **Step 1: Write the failing test** + +An `tests/Feature/Billing/StripeIdempotencyKeyTest.php` anhängen: + +```php +it('pages through every active price of a product', function () { + Http::fake([ + 'api.stripe.com/*' => Http::sequence() + ->push([ + 'data' => [ + ['id' => 'price_a', 'unit_amount' => 3480, 'currency' => 'eur', + 'created' => 100, 'recurring' => ['interval' => 'month'], + 'metadata' => ['addon' => 'priority_support']], + ['id' => 'price_b', 'unit_amount' => 41760, 'currency' => 'eur', + 'created' => 101, 'recurring' => ['interval' => 'year'], 'metadata' => []], + ], + 'has_more' => true, + ]) + ->push([ + 'data' => [ + ['id' => 'price_c', 'unit_amount' => 2900, 'currency' => 'eur', + 'created' => 102, 'recurring' => ['interval' => 'month'], + 'metadata' => ['addon' => 'priority_support', 'tax_treatment' => 'reverse_charge']], + ], + 'has_more' => false, + ]), + ]); + + $prices = (new HttpStripeClient)->activePricesFor('prod_1'); + + expect($prices)->toHaveCount(3) + ->and($prices[0])->toBe([ + 'id' => 'price_a', + 'unit_amount' => 3480, + // Upper case, because that is how our own tables hold it and the + // comparison in AdoptStripePrice must not have to remember which + // side is which. + 'currency' => 'EUR', + 'interval' => 'month', + 'created' => 100, + 'metadata' => ['addon' => 'priority_support'], + ]) + ->and($prices[2]['id'])->toBe('price_c'); + + // The second page has to be asked for, or this reintroduces the very gap it + // exists to close — a family product accumulates prices across versions, + // terms, treatments and every rate change. + Http::assertSent(fn ($request) => str_contains($request->url(), 'starting_after=price_b')); + + // Archived prices are none of our business here: we are looking for + // something to SELL on. + Http::assertSent(fn ($request) => str_contains($request->url(), 'active=true')); +}); + +it('writes metadata onto a price that already exists', function () { + Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_a'])]); + + (new HttpStripeClient)->updatePriceMetadata('price_a', ['addon' => 'priority_support']); + + Http::assertSent(fn ($request) => $request->url() === 'https://api.stripe.com/v1/prices/price_a' + && $request['metadata[addon]'] === 'priority_support'); +}); + +it('lets the fake answer with the prices it holds, minus the archived ones', function () { + $fake = new FakeStripeClient; + + $kept = $fake->createPrice('prod_1', 3480, 'EUR', 'month', ['addon' => 'priority_support']); + $gone = $fake->createPrice('prod_1', 2900, 'EUR', 'month', ['addon' => 'priority_support']); + $other = $fake->createPrice('prod_2', 3480, 'EUR', 'month', []); + $fake->archivePrice($gone); + + $fake->plantPrice('price_orphan', 'prod_1', 3480, 'EUR', 'month', + ['addon' => 'priority_support'], created: 0); + + $found = collect($fake->activePricesFor('prod_1'))->pluck('id')->all(); + + expect($found)->toContain($kept, 'price_orphan') + ->and($found)->not->toContain($gone, $other); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +Expected: FAIL — `Call to undefined method App\Services\Stripe\HttpStripeClient::activePricesFor()`. + +- [ ] **Step 3: Add both methods to the contract** + +`app/Services/Stripe/StripeClient.php`, zwischen `createPrice` und `archivePrice`: + +```php + /** + * Every active recurring Price of a Product, as Stripe holds them. + * + * The half of the catalogue mirror that was missing: nothing here ever asked + * Stripe what it already had. A run that created a Price and died before the + * row was written left an orphan our table never learned about, and once the + * idempotency key expired the next run made a SECOND live Price for the same + * money — the exact duplicate the key exists to prevent. See + * App\Services\Billing\AdoptStripePrice, which is the only caller. + * + * 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; + + /** + * Write metadata onto a Price that already exists. + * + * Metadata is one of the few fields a Stripe Price lets you change — the + * same property activatePrice() rests on, and the reason the metadata format + * is NOT part of a Price's identity. An adopted orphan is brought up to + * today's metadata rather than replaced, which is exactly what was done by + * hand after 2026-07-29 and keeps Stripe's own dashboard readable. + */ + public function updatePriceMetadata(string $priceId, array $metadata): void; +``` + +- [ ] **Step 4: Implement in `HttpStripeClient`** (nach `activatePrice`) + +```php + public function activePricesFor(string $productId): array + { + $prices = []; + $after = null; + + // Paged through to the end, for the same reason invoiceLines() is: Stripe + // caps a page at a hundred, and a family Product collects a Price per + // version, term, treatment and rate change. Stopping at the first page + // would leave an orphan unfound and mint the duplicate anyway. + do { + $page = $this->request() + ->get($this->url('prices'), array_filter([ + 'product' => $productId, + 'active' => 'true', + 'type' => 'recurring', + 'limit' => 100, + 'starting_after' => $after, + ], fn ($value) => $value !== null)) + ->throw() + ->json(); + + $data = (array) ($page['data'] ?? []); + + foreach ($data as $price) { + $prices[] = [ + 'id' => (string) ($price['id'] ?? ''), + 'unit_amount' => (int) ($price['unit_amount'] ?? 0), + 'currency' => strtoupper((string) ($price['currency'] ?? '')), + 'interval' => (string) ($price['recurring']['interval'] ?? ''), + 'created' => (int) ($price['created'] ?? 0), + 'metadata' => array_map( + fn ($value) => (string) $value, + (array) ($price['metadata'] ?? []), + ), + ]; + } + + $after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? null); + } while (($page['has_more'] ?? false) === true && $after !== null); + + return $prices; + } + + public function updatePriceMetadata(string $priceId, array $metadata): void + { + $this->request() + ->asForm() + ->post($this->url('prices/'.$priceId), $this->flatten('metadata', $metadata)) + ->throw(); + } +``` + +- [ ] **Step 5: Implement in `FakeStripeClient`** (nach `activatePrice`) + +```php + public function activePricesFor(string $productId): array + { + // No failIfAsked(): createPrice(), the call this one stands in front of, + // does not fail either, and a test that scripts an outage for a refund + // must not have its catalogue sync change behaviour underneath it. + $found = []; + + foreach ($this->prices as $id => $price) { + if ($price['product'] !== $productId || in_array($id, $this->archived, true)) { + continue; + } + + $found[] = [ + 'id' => $id, + 'unit_amount' => $price['amount'], + 'currency' => strtoupper($price['currency']), + 'interval' => $price['interval'], + 'created' => $price['created'], + 'metadata' => $price['metadata'], + ]; + } + + return $found; + } + + public function updatePriceMetadata(string $priceId, array $metadata): void + { + if (isset($this->prices[$priceId])) { + $this->prices[$priceId]['metadata'] = $metadata; + } + + $this->metadataUpdates[] = ['price' => $priceId, 'metadata' => $metadata]; + } + + /** + * A Price that exists at Stripe and in no table of ours — the orphan a run + * leaves behind when it dies between Stripe's create and our insert. + * + * Here rather than in a test file because it is the state the whole adoption + * step exists for, and every test that needs it would otherwise reach into + * $prices and write the shape by hand. + */ + public function plantPrice( + string $id, + string $productId, + int $amountCents, + string $currency, + string $interval, + array $metadata = [], + int $created = 1, + ): void { + $this->prices[$id] = [ + 'product' => $productId, + 'amount' => $amountCents, + 'currency' => $currency, + 'interval' => $interval, + 'metadata' => $metadata, + 'created' => $created, + ]; + } +``` + +Eigenschaft dazu, neben `$activated`: + +```php + /** + * Every metadata write, in order, so a test can assert that an adopted + * orphan was brought up to today's metadata instead of being replaced. + * + * @var array}> + */ + public array $metadataUpdates = []; +``` + +- [ ] **Step 6: Run the tests** + +```bash +php artisan test tests/Feature/Billing/StripeIdempotencyKeyTest.php +``` + +Expected: PASS, zehn Tests. + +- [ ] **Step 7: Run the billing folder** + +```bash +php artisan test tests/Feature/Billing +``` + +Expected: PASS. + +- [ ] **Step 8: 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 it already has + +Nothing here could. The client could create, archive and unarchive a Price and +had no way to list one, so a run that died between Stripe's create and our +insert left an orphan our table never learned about — and once the key expired, +the next run made a second live Price for the same money. + +Paged to the end like invoiceLines(), because a family Product collects a Price +per version, term, treatment and rate change, and stopping at the first page +would leave the orphan unfound and mint the duplicate anyway. + +updatePriceMetadata() alongside it: metadata is one of the few fields a Price +lets you change, which is why the metadata format is not part of a Price's +identity and why an adopted orphan is brought up to date rather than replaced. + +Co-Authored-By: Claude Opus 5 +MSG +``` + +--- + +### Task 3: `AdoptStripePrice` — die fünf Bedingungen + +**Files:** +- Create: `app/Services/Billing/AdoptStripePrice.php` +- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` + +**Interfaces:** +- Consumes: `StripeClient::activePricesFor()`, `updatePriceMetadata()`, `archivePrice()` aus Task 2. `FakeStripeClient::plantPrice()`, `$metadataUpdates`. +- Produces: + +```php +public function __invoke( + string $productId, + int $amountCents, + string $currency, + string $interval, + array $metadata, + array $identifying, + callable $claimed, // fn (string $priceId): bool +): ?string +``` + + Gibt die übernommene Preis-ID zurück oder `null`, wenn nichts zu übernehmen ist. `$identifying` sind die Metadatenschlüssel, die einen Preis als unseren ausweisen (`['addon']` bzw. `['plan_price_id']`). + +- [ ] **Step 1: Write the failing test** + +`tests/Feature/Billing/StripePriceAdoptionTest.php`: + +```php +stripe = new FakeStripeClient; + app()->instance(StripeClient::class, $this->stripe); +}); + +/** The module metadata as AddonPrices sends it today. */ +function moduleMetadata(string $treatment = 'domestic'): array +{ + return ['addon' => 'priority_support', 'tax_treatment' => $treatment]; +} + +/** Ask the adoption step the question AddonPrices asks it. */ +function adoptModulePrice(?array $metadata = null, ?callable $claimed = null): ?string +{ + return app(AdoptStripePrice::class)( + productId: 'prod_support', + amountCents: 3480, + currency: 'EUR', + interval: 'month', + metadata: $metadata ?? moduleMetadata(), + identifying: ['addon'], + claimed: $claimed ?? fn (string $id) => false, + ); +} + +it('adopts the orphan of 2026-07-29 instead of minting a second price', function () { + // The state that morning: the Price exists at Stripe, carries the metadata + // of the code that made it — WITHOUT tax_treatment, which 9da1358 added + // afterwards — and no row of ours knows it. + $this->stripe->plantPrice('price_1TygdEC7u8NpJ8pOt3nsoyYw', 'prod_support', + 3480, 'EUR', 'month', ['addon' => 'priority_support']); + + expect(adoptModulePrice())->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw'); + + // Brought up to today's metadata rather than replaced: metadata is mutable + // at Stripe, the amount is not, which is the whole reason the format is no + // part of a Price's identity. + expect($this->stripe->metadataUpdates)->toBe([[ + 'price' => 'price_1TygdEC7u8NpJ8pOt3nsoyYw', + 'metadata' => moduleMetadata(), + ]]); +}); + +it('leaves the metadata alone when it already says the right thing', function () { + $this->stripe->plantPrice('price_ok', 'prod_support', 3480, 'EUR', 'month', moduleMetadata()); + + expect(adoptModulePrice())->toBe('price_ok') + ->and($this->stripe->metadataUpdates)->toBe([]); +}); + +it('adopts nothing when the amount, currency or interval differ', function () { + $this->stripe->plantPrice('price_cheaper', 'prod_support', 2900, 'EUR', 'month', moduleMetadata()); + $this->stripe->plantPrice('price_yearly', 'prod_support', 3480, 'EUR', 'year', moduleMetadata()); + $this->stripe->plantPrice('price_dollars', 'prod_support', 3480, 'USD', 'month', moduleMetadata()); + + expect(adoptModulePrice())->toBeNull(); +}); + +it('refuses a price nothing proves is ours, and says so', function () { + Log::spy(); + + // What a person clicking through Stripe's own dashboard leaves behind: the + // right money on our product, and not one word about what it is for. + $this->stripe->plantPrice('price_by_hand', 'prod_support', 3480, 'EUR', 'month', []); + + expect(adoptModulePrice())->toBeNull(); + + Log::shouldHaveReceived('warning')->once(); +}); + +it('passes silently over another of our own prices', function () { + Log::spy(); + + // At a VAT rate of nought both treatments are the same amount, so the + // reverse-charge Price sits at the domestic one's money — and contradicts on + // tax_treatment. That is not a mystery worth a warning; it is a Price of + // ours that is not the one being asked for. + $this->stripe->plantPrice('price_rc', 'prod_support', 3480, 'EUR', 'month', + moduleMetadata('reverse_charge')); + + expect(adoptModulePrice())->toBeNull(); + + Log::shouldNotHaveReceived('warning'); +}); + +it('never hands out a price a row already claims', function () { + $this->stripe->plantPrice('price_taken', 'prod_support', 3480, 'EUR', 'month', moduleMetadata()); + + expect(adoptModulePrice(claimed: fn (string $id) => $id === 'price_taken'))->toBeNull(); +}); + +it('adopts the oldest of several orphans and stops selling the rest', function () { + Log::spy(); + + $this->stripe->plantPrice('price_second', 'prod_support', 3480, 'EUR', 'month', + ['addon' => 'priority_support'], created: 200); + $this->stripe->plantPrice('price_first', 'prod_support', 3480, 'EUR', 'month', + ['addon' => 'priority_support'], created: 100); + $this->stripe->plantPrice('price_third', 'prod_support', 3480, 'EUR', 'month', + ['addon' => 'priority_support'], created: 300); + + // The oldest, because it is the one a lost row is likeliest to have been + // billing on. + expect(adoptModulePrice())->toBe('price_first') + ->and($this->stripe->archived)->toBe(['price_second', 'price_third']); + + Log::shouldHaveReceived('warning'); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: FAIL — `Target class [App\Services\Billing\AdoptStripePrice] does not exist.` + +- [ ] **Step 3: Write the implementation** + +`app/Services/Billing/AdoptStripePrice.php`: + +```php + $metadata what the create call would send + * @param array $identifying metadata keys that mark a Price as ours + * @param callable(string): bool $claimed is this Price id already in our register? + */ + public function __invoke( + string $productId, + int $amountCents, + string $currency, + string $interval, + array $metadata, + array $identifying, + callable $claimed, + ): ?string { + $candidates = []; + + foreach ($this->stripe->activePricesFor($productId) as $price) { + if ($price['unit_amount'] !== $amountCents + || $price['currency'] !== strtoupper($currency) + || $price['interval'] !== $interval) { + continue; + } + + if ($claimed($price['id'])) { + continue; + } + + // Contradicts on something it carries: another Price of ours, not a + // mystery. Silent — at a VAT rate of nought the two treatments share + // an amount, and this would otherwise warn on every sweep. + if ($this->contradicts($price['metadata'], $metadata)) { + continue; + } + + if (! $this->confirms($price['metadata'], $metadata, $identifying)) { + 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; + } + + $candidates[] = $price; + } + + if ($candidates === []) { + return null; + } + + usort($candidates, fn (array $a, array $b) => $a['created'] <=> $b['created']); + + $adopted = array_shift($candidates); + + foreach ($candidates as $duplicate) { + $this->stripe->archivePrice($duplicate['id']); + + Log::warning('stripe: stopped selling a duplicate price for one figure', [ + 'price' => $duplicate['id'], + 'adopted' => $adopted['id'], + 'product' => $productId, + 'amount_cents' => $amountCents, + ]); + } + + // Only when it differs, so a sweep over a healthy catalogue makes no + // writes at Stripe at all. + if ($adopted['metadata'] !== $metadata) { + $this->stripe->updatePriceMetadata($adopted['id'], $metadata); + } + + Log::info('stripe: adopted an existing price instead of creating a second one', [ + 'price' => $adopted['id'], + 'product' => $productId, + 'amount_cents' => $amountCents, + 'currency' => strtoupper($currency), + 'interval' => $interval, + ]); + + return $adopted['id']; + } + + /** + * Does this Price say something about itself that we do not? + * + * @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; + } + + /** + * Does it prove it is ours? + * + * Failing to contradict is not enough — an empty metadata bag contradicts + * nothing. At least one identifying key has to be there and agree. + * + * @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: Run the test to verify it passes** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: PASS, acht Tests. + +- [ ] **Step 5: Commit** + +```bash +git add -- app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +```bash +git commit -F - -- app/Services/Billing/AdoptStripePrice.php tests/Feature/Billing/StripePriceAdoptionTest.php <<'MSG' +Recognise the price Stripe already has + +An abandoned run leaves an orphan: Stripe made the Price, our insert never +happened, and the table that decides everything afterwards does not know it +exists. The key covers a day; after that the next run makes a second live Price +for the same money. + +What may be taken over is narrow. Same amount, currency and interval — a Price +at another figure would move money, and nothing here may: a running contract +keeps the Price it was sold on, and a booking stays frozen at what it cost that +day. Plus proof in the metadata, because an unexplained active Price at the +right money is what somebody clicking through Stripe's dashboard leaves behind, +and adopting that is worse than minting a second one. + +Several orphans: the oldest is adopted — likeliest to be the one a lost row was +billing on — and the rest are archived, which stops them being sold and moves +nobody. + +Co-Authored-By: Claude Opus 5 +MSG +``` + +--- + +### Task 4: Die Modulseite benutzt den Wiedererkennungsschritt + +**Files:** +- Modify: `app/Services/Billing/AddonPrices.php:47` (Konstruktor), `:126-159` (der Anlege-Pfad in `ensure`) +- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` (angehängt) + +**Interfaces:** +- Consumes: `AdoptStripePrice::__invoke()` aus Task 3. +- Produces: `AddonPrices::__construct(StripeClient $stripe, AdoptStripePrice $adopt)`. **Geprüft: niemand konstruiert `AddonPrices` mit `new`** (weder App noch Tests), der Container löst auf. + +- [ ] **Step 1: Write the failing test** + +An `tests/Feature/Billing/StripePriceAdoptionTest.php` anhängen: + +```php +use App\Models\StripeAddonPrice; +use App\Models\Subscription; +use App\Models\SubscriptionAddon; +use App\Services\Billing\AddonPrices; +use App\Services\Billing\TaxTreatment; +use Illuminate\Contracts\Console\Kernel; + +it('takes the orphan over instead of minting a second module price', function () { + // The product exists because a previous run got that far; the Price exists + // because the run that made it died before the insert. + $this->stripe->plantPrice('price_1TygdEC7u8NpJ8pOt3nsoyYw', 'prod_support', + 3480, 'EUR', 'month', ['addon' => 'priority_support']); + StripeAddonPrice::query()->create([ + 'addon_key' => 'priority_support', 'reverse_charge' => false, + 'amount_cents' => 41760, 'net_cents' => 34800, 'currency' => 'EUR', + 'interval' => 'year', 'stripe_product_id' => 'prod_support', + 'stripe_price_id' => 'price_yearly_already_known', + ]); + + $before = count($this->stripe->prices); + + $id = app(AddonPrices::class)->ensure( + 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), + ); + + expect($id)->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw') + // Nothing new at Stripe: the orphan was taken over, not replaced. + ->and(count($this->stripe->prices))->toBe($before) + ->and(StripeAddonPrice::query() + ->where('addon_key', 'priority_support') + ->where('interval', 'month') + ->where('reverse_charge', false) + ->value('stripe_price_id'))->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw'); +}); + +it('does not block a booking because the metadata format moved', function () { + // 2026-07-29, exactly: orphan with the old metadata, code with the new. This + // is the call that answered HTTP 400 for twenty-four hours. + $this->stripe->plantPrice('price_1TygdEC7u8NpJ8pOt3nsoyYw', 'prod_support', + 3480, 'EUR', 'month', ['addon' => 'priority_support']); + StripeAddonPrice::query()->create([ + 'addon_key' => 'priority_support', 'reverse_charge' => false, + 'amount_cents' => 41760, 'net_cents' => 34800, 'currency' => 'EUR', + 'interval' => 'year', 'stripe_product_id' => 'prod_support', + 'stripe_price_id' => 'price_yearly_already_known', + ]); + + $id = app(AddonPrices::class)->ensure( + 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), + ); + + expect($id)->toBe('price_1TygdEC7u8NpJ8pOt3nsoyYw') + ->and($this->stripe->metadataUpdates)->toHaveCount(1); +}); + +it('leaves a frozen booking on the price it was sold at', function () { + app(Kernel::class)->call('stripe:sync-catalogue'); + + $sold = app(AddonPrices::class)->liveFor( + 'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(), + ); + + $subscription = Subscription::factory()->plan('team')->create(); + $booking = SubscriptionAddon::query()->create([ + 'subscription_id' => $subscription->id, + 'addon_key' => 'priority_support', + // Net, per month, per unit — frozen at booking. `currency` and + // `booked_at` are both NOT NULL (2026_07_26_060000), and `uuid` fills + // itself through the model's uniqueIds(). + 'price_cents' => 2900, + 'currency' => 'EUR', + 'quantity' => 1, + 'booked_at' => now(), + 'stripe_price_id' => $sold, + ]); + + // The catalogue moves, and somebody has already left an orphan at the new + // figure. Neither may reach a booking that is already frozen. + config()->set('provisioning.addons.priority_support.price_cents', 3900); + $this->stripe->plantPrice('price_orphan_new_figure', 'prod_support', + 4680, 'EUR', 'month', ['addon' => 'priority_support']); + + app(Kernel::class)->call('stripe:sync-catalogue'); + + expect($booking->refresh()->stripe_price_id)->toBe($sold) + ->and($this->stripe->archived)->not->toContain($sold); +}); +``` + +**Zum letzten Test:** `SubscriptionAddon::booted()` wirft bei jeder Änderung an `FROZEN` (`subscription_id`, `addon_key`, `price_cents`, `currency`, `quantity`, `booked_at`) — die Regel „eine Buchung friert ihren Preis ein" steht also schon im Modell. `stripe_price_id` gehört nicht dazu und ist damit änderbar; genau deshalb muss ein Test sagen, dass die Übernahme sie nicht anfasst. `Subscription::factory()->plan('team')` ist die Form aus `tests/Feature/Billing/ReverseChargePriceTest.php:220`. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: FAIL — `ensure()` legt einen neuen Preis an, `count($this->stripe->prices)` ist um eins gewachsen, und die zurückgegebene ID ist nicht die der Waise. + +- [ ] **Step 3: Wire the adoption into `AddonPrices`** + +Konstruktor (`:47`): + +```php + public function __construct( + private readonly StripeClient $stripe, + private readonly AdoptStripePrice $adopt, + ) {} +``` + +Der Anlege-Pfad in `ensure()` (`:126-159`) wird zu: + +```php + $productId = $this->product($addonKey); + + $metadata = [ + // Read back when a Stripe invoice line has to be turned into + // wording a customer can read — see StripeInvoiceLines. + 'addon' => $addonKey, + // Which of the module's two Prices this is, for anyone reading + // Stripe's own dashboard, where they would otherwise differ only + // by an amount. + 'tax_treatment' => $reverseCharge ? 'reverse_charge' : 'domestic', + ]; + + // Asked BEFORE minting, because a run that died between Stripe's create + // and our insert left a Price no table of ours knows — and the key below + // stops protecting it after twenty-four hours. See AdoptStripePrice for + // what may be taken over and why so narrowly. + $priceId = ($this->adopt)( + productId: $productId, + amountCents: $amount, + currency: $currency, + interval: $interval, + metadata: $metadata, + identifying: ['addon'], + claimed: fn (string $id) => StripeAddonPrice::query() + ->where('stripe_price_id', $id) + ->exists(), + ); + + $priceId ??= $this->stripe->createPrice( + productId: $productId, + amountCents: $amount, + currency: $currency, + interval: $interval, + metadata: $metadata, + // Says "I have already sent this call", nothing more. The amount is + // the CHARGED one and the treatment is in there because at a rate of + // nought the two Prices are the same amount — but what stops a + // second Price for one figure is AdoptStripePrice above, not this: + // Stripe forgets a key after twenty-four hours. The metadata is + // folded in by IdempotencyKey inside the client, so changing the + // format below can never again refuse the call for a day. + idempotencyKey: "clupilot-addon-price-{$addonKey}-{$interval}-{$amount}-{$currency}" + .($reverseCharge ? '-rc' : ''), + ); + + $this->remember($addonKey, $reverseCharge, $amount, $netCents, $currency, $interval, $productId, $priceId); + $this->archiveSuperseded($addonKey, $reverseCharge, $netCents, $currency, $interval, $amount); + + return $priceId; +``` + +`use App\Services\Billing\AdoptStripePrice;` ist nicht nötig (gleiche Namespace). + +Den Kommentar in `remember()` (`:297-301`) um den zweiten Grund ergänzen, aus dem die Ausnahme kommen kann: + +```php + } catch (UniqueConstraintViolationException) { + // Two bookings of the same module landed together. Stripe replayed + // one Price for both — the idempotency key saw to that — so there is + // nothing to correct here beyond letting the first row stand. The + // same catch now also covers a Price id another row already claims, + // which the unique index refuses: the caller is handed the id + // regardless, because refusing would fail a customer's booking over + // a register that is one row short. + } +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: PASS, elf Tests. + +- [ ] **Step 5: Run the billing folder** + +```bash +php artisan test tests/Feature/Billing +``` + +Expected: PASS. Ein Fehler in `StripeAddonBillingTest` oder `AddonCancellationTest` bedeutet, dass der Wiedererkennungsschritt einen Preis übernimmt, wo ein Test einen neuen erwartet — dann prüfen, ob die Erwartung des Tests oder die Bedingung in `AdoptStripePrice` falsch ist, **bevor** irgendetwas geändert wird. + +- [ ] **Step 6: Commit** + +```bash +git add -- app/Services/Billing/AddonPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +```bash +git commit -F - -- app/Services/Billing/AddonPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php <<'MSG' +Ask before minting a module price + +The sweep that died on 2026-07-29 left price_1TygdEC7u8NpJ8pOt3nsoyYw at Stripe +and no row here. This is the step that finds it: same money, same interval, our +metadata on it, unclaimed — taken over and brought up to date instead of +duplicated. + +The comment on the key was the root of it. It read "keyed on what the Price IS", +and a key is not that: identity is product, amount, currency and interval, and +recognising those is AdoptStripePrice's job. Stripe forgets a key after a day; +it never was the guard against a duplicate. + +A booking stays where it was sold. Adoption only ever matches an identical +amount, so a frozen booking cannot be moved by it — there is a test that says so. + +Co-Authored-By: Claude Opus 5 +MSG +``` + +--- + +### Task 5: Die Paketseite benutzt ihn ebenso + +**Files:** +- Modify: `app/Services/Billing/PlanPrices.php:43` (Konstruktor), `:130-160` (der Anlege-Zweig in `ensure`) +- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` (angehängt) + +**Interfaces:** +- Consumes: `AdoptStripePrice::__invoke()` aus Task 3. +- Produces: `PlanPrices::__construct(StripeClient $stripe, AdoptStripePrice $adopt)`. **Geprüft: niemand konstruiert `PlanPrices` mit `new`.** + +- [ ] **Step 1: Write the failing test** + +An `tests/Feature/Billing/StripePriceAdoptionTest.php` anhängen: + +```php +use App\Models\PlanPrice; +use App\Models\StripePlanPrice; +use App\Services\Billing\PlanPrices; + +it('takes over an orphaned package price', function () { + // A catalogue mirrored once, so families have Products and rows have Prices. + app(Kernel::class)->call('stripe:sync-catalogue'); + + $row = PlanPrice::query()->firstOrFail(); + $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); + + // The register loses its row and Stripe keeps the Price: a run that died + // between the two, seen from the next run's point of view. + $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(); + + $before = count($this->stripe->prices); + + $id = app(PlanPrices::class)->ensure($row->refresh(), TaxTreatment::domestic()); + + expect($id)->toBe($orphan) + ->and(count($this->stripe->prices))->toBe($before) + ->and(StripePlanPrice::query() + ->where('plan_price_id', $row->id) + ->where('reverse_charge', false) + ->where('charged_cents', $charged) + ->value('stripe_price_id'))->toBe($orphan) + // The pointer for the ordinary domestic sale is written as before. + ->and((string) $row->refresh()->stripe_price_id)->toBe($orphan); +}); + +it('does not take a package price belonging to another catalogue row', function () { + app(Kernel::class)->call('stripe:sync-catalogue'); + + $row = PlanPrice::query()->firstOrFail(); + $product = (string) $row->version->family->stripe_product_id; + $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic()); + $interval = $row->term === Subscription::TERM_YEARLY ? 'year' : 'month'; + + StripePlanPrice::query()->where('plan_price_id', $row->id)->delete(); + + // Same product, same money, same interval — and plan_price_id says it is a + // DIFFERENT row's Price. One Product carries every version and term of a + // family, so this is the ordinary case, not an exotic one. + $this->stripe->plantPrice('price_other_row', $product, $charged, (string) $row->currency, + $interval, ['plan_price_id' => (string) ($row->id + 1000), 'tax_treatment' => 'domestic']); + + $id = app(PlanPrices::class)->ensure($row->refresh(), TaxTreatment::domestic()); + + expect($id)->not->toBe('price_other_row'); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: FAIL im ersten der beiden — `ensure()` legt einen zweiten Preis an, `count($this->stripe->prices)` wächst. + +- [ ] **Step 3: Wire the adoption into `PlanPrices`** + +Konstruktor (`:43`): + +```php + public function __construct( + private readonly StripeClient $stripe, + private readonly AdoptStripePrice $adopt, + ) {} +``` + +Der Zweig `if ($priceId === null)` in `ensure()` (`:135-160`): + +```php + $interval = $price->term === Subscription::TERM_YEARLY ? 'year' : 'month'; + + if ($priceId === null) { + $metadata = [ + 'plan_family' => $family->key, + 'plan_version' => (string) $version->version, + 'plan_version_id' => (string) $version->id, + 'plan_price_id' => (string) $price->id, + // Read back by anything that has a Price id and needs to know + // what kind of sale it was — and by a person looking at + // Stripe's own dashboard, where two Prices on one Product + // would otherwise differ only by an amount. + 'tax_treatment' => $treatment->reverseCharge ? 'reverse_charge' : 'domestic', + ]; + + // Asked BEFORE minting: a run that died between Stripe's create and + // our insert left a Price the register does not know, and the key + // below stops protecting it after twenty-four hours. `plan_price_id` + // is what proves such a Price is this row's — one Product carries + // every version and term of a family, so the amount alone would not. + $priceId = ($this->adopt)( + productId: (string) $productId, + amountCents: $charged, + currency: (string) $price->currency, + interval: $interval, + metadata: $metadata, + identifying: ['plan_price_id'], + claimed: fn (string $id) => StripePlanPrice::query() + ->where('stripe_price_id', $id) + ->exists(), + ); + + $priceId ??= $this->stripe->createPrice( + productId: (string) $productId, + amountCents: $charged, + currency: (string) $price->currency, + interval: $interval, + metadata: $metadata, + // Says "I have already sent this call", nothing more — the + // metadata is folded in by IdempotencyKey inside the client, so + // changing the format above can never again refuse the call for + // a day. What stops a second Price for one figure is the + // adoption step above; Stripe forgets a key after 24 hours. + idempotencyKey: "clupilot-price-{$price->id}-{$charged}" + .($treatment->reverseCharge ? '-rc' : ''), + ); + } elseif ($existing?->archived_at !== null) { +``` + +Der bestehende `elseif`-Zweig und alles darunter bleiben unverändert. + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: PASS, dreizehn Tests. + +- [ ] **Step 5: Run the billing folder** + +```bash +php artisan test tests/Feature/Billing +``` + +Expected: PASS. `ReverseChargePriceTest` ist hier der wichtigste Zeuge: `:342-347` zählt genau 16 Register-Zeilen und 16 verschiedene Preis-IDs nach einem doppelten Sync-Lauf. Bleibt das grün, hat der Wiedererkennungsschritt nichts verdoppelt und nichts zusammengelegt. + +- [ ] **Step 6: Commit** + +```bash +git add -- app/Services/Billing/PlanPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +```bash +git commit -F - -- app/Services/Billing/PlanPrices.php tests/Feature/Billing/StripePriceAdoptionTest.php <<'MSG' +Ask before minting a package price + +Same defect, same fix, one difference: a family Product carries every version +and every term, so the amount alone does not say which row a Price belongs to. +plan_price_id in the metadata does, and it is what has to agree before a Price +is taken over. + +Co-Authored-By: Claude Opus 5 +MSG +``` + +--- + +### Task 6: Eine Zeile pro Stripe-Preis + +**Files:** +- Create: `database/migrations/2026_07_31_210000_one_row_per_stripe_price.php` +- Modify: `docs/handoffs/2026-07-30-real-run-handoff.md:175-176` +- Test: `tests/Feature/Billing/StripePriceAdoptionTest.php` (angehängt) + +**Interfaces:** +- Consumes: nichts. +- Produces: eindeutiger Index `stripe_addon_prices_price_unique` auf `stripe_addon_prices.stripe_price_id`. + +- [ ] **Step 1: Write the failing test** + +An `tests/Feature/Billing/StripePriceAdoptionTest.php` anhängen: + +```php +use Illuminate\Database\UniqueConstraintViolationException; + +it('lets no two module rows claim one stripe price', function () { + $shared = [ + 'addon_key' => 'priority_support', 'net_cents' => 2900, 'currency' => 'EUR', + 'stripe_product_id' => 'prod_support', 'stripe_price_id' => 'price_shared', + ]; + + StripeAddonPrice::query()->create([...$shared, + 'reverse_charge' => false, 'amount_cents' => 3480, 'interval' => 'month']); + + // Two rows on one Price is what Block D warned about: archiving the one + // would withdraw the Price the other is still selling. + expect(fn () => StripeAddonPrice::query()->create([...$shared, + 'reverse_charge' => true, 'amount_cents' => 2900, 'interval' => 'month'])) + ->toThrow(UniqueConstraintViolationException::class); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: FAIL — keine Ausnahme, die zweite Zeile entsteht. + +- [ ] **Step 3: Write the migration** + +`database/migrations/2026_07_31_210000_one_row_per_stripe_price.php`: + +```php +select('stripe_price_id') + ->groupBy('stripe_price_id') + ->havingRaw('count(*) > 1') + ->pluck('stripe_price_id'); + + foreach ($shared as $priceId) { + // The lowest id stays — the row that was written first, which is the + // one anything already billing is likeliest to have been reading. + $keep = DB::table('stripe_addon_prices')->where('stripe_price_id', $priceId)->min('id'); + + DB::table('stripe_addon_prices') + ->where('stripe_price_id', $priceId) + ->where('id', '!=', $keep) + ->delete(); + } + + Schema::table('stripe_addon_prices', function (Blueprint $table) { + $table->unique('stripe_price_id', 'stripe_addon_prices_price_unique'); + }); + } + + public function down(): void + { + Schema::table('stripe_addon_prices', function (Blueprint $table) { + $table->dropUnique('stripe_addon_prices_price_unique'); + }); + } +}; +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +php artisan test tests/Feature/Billing/StripePriceAdoptionTest.php +``` + +Expected: PASS, vierzehn Tests. + +- [ ] **Step 5: Prove the deduplication itself, on a real migration run** + +```bash +php artisan migrate:fresh --env=testing && php artisan migrate:status | tail -3 +``` + +Expected: die neue Migration steht als `Ran`. (Sie hat auf einer leeren Tabelle nichts zu entdoppeln — der Test in Step 1 prüft den Index, und der Entdopplungszweig ist gegen eine Live-Tabelle geschrieben, in der es vermutlich keine Doppel gibt.) + +- [ ] **Step 6: Mark the Block D item done** + +`docs/handoffs/2026-07-30-real-run-handoff.md:175-176` ersetzen: + +```markdown +- ~~`stripe_addon_prices.stripe_price_id` ist **nicht** eindeutig (Plan-Seite schon): + zwei Zeilen könnten einen Preis teilen, Archivieren würde den anderen mitentziehen.~~ + **Erledigt** — eindeutig seit `2026_07_31_210000_one_row_per_stripe_price`; zugleich + das Netz unter dem Wiedererkennungsschritt, siehe + `docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md`. +``` + +- [ ] **Step 7: Run the full suite — this is a schema change** + +```bash +php artisan test +``` + +Expected: PASS. Eine `UniqueConstraintViolationException` in einem fremden Test bedeutet, dass dort zwei Modul-Zeilen absichtlich eine Preis-ID teilten — das ist ein Fund, der gemeldet und nicht durch Zurücknehmen des Index behoben wird. + +- [ ] **Step 8: Commit** + +```bash +git add -- database/migrations/2026_07_31_210000_one_row_per_stripe_price.php tests/Feature/Billing/StripePriceAdoptionTest.php docs/handoffs/2026-07-30-real-run-handoff.md +``` + +```bash +git commit -F - -- database/migrations/2026_07_31_210000_one_row_per_stripe_price.php tests/Feature/Billing/StripePriceAdoptionTest.php docs/handoffs/2026-07-30-real-run-handoff.md <<'MSG' +One row per Stripe price on the module side too + +The plan register has had this since 2026_07_30_110000. The module register +never did, so two rows could claim one Price and archiving the one would have +withdrawn the Price the other was still selling — Block D of the real-run +handoff. + +It is also the net under the adoption step: at a VAT rate of nought both +treatments charge the same amount, and the check that refuses an already-claimed +Price is then the only thing keeping the two apart. + +Duplicates are deleted, not archived. An archived row goes on claiming the id +and a unique index knows nothing about archived_at; the row rebuilds itself on +the next ensure(), and subscription_addons holds the Stripe id as text rather +than a foreign key, so no booking loses its price. + +Co-Authored-By: Claude Opus 5 +MSG +``` + +--- + +## Nach dem letzten Task + +- [ ] **Voller Testlauf** (nicht nur `tests/Feature/Billing`): + +```bash +php artisan test +``` + +- [ ] **Ein Review über den Gesamtdiff**, dann **eine** Fix-Runde, dann **ein** Re-Review über den Fix-Diff — und danach werden offene Befunde mit schriftlicher Begründung geparkt (R22.1). Ein zweiter Re-Review desselben Befundes ist verboten. +- [ ] **Nicht geplant, bewusst offen** (Spec §12): ein Aufräum-Kommando für Waisen, die niemand übernehmen kann. Sie stehen mit ihrer ID im Log (`stripe: left an unexplained active price alone…`). Als Folgepunkt in den Merge Request. diff --git a/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md b/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md index a2d2491..1f3987a 100644 --- a/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md +++ b/docs/superpowers/specs/2026-07-30-stripe-price-adoption-design.md @@ -219,8 +219,14 @@ diese Prüfung je gerissen wird. ## 11. Tests -Ein Regeldatei-Test nach Repo-Sitte: -`tests/Feature/Billing/StripePriceAdoptionTest.php` +Zwei Regeldatei-Tests nach Repo-Sitte, getrennt danach, **gegen wen** sie prüfen: + +- `tests/Feature/Billing/StripeIdempotencyKeyTest.php` — was auf der Leitung + landet. Läuft über `Http::fake()` gegen `HttpStripeClient`, weil geprüft werden + muss, was **gesendet** wird, nicht was der Fake sich merkt. Hier steht auch, + dass der Fake Stripes 400 überhaupt nachbildet. +- `tests/Feature/Billing/StripePriceAdoptionTest.php` — die Übernahmeregeln, + beide Seiten, über den `FakeStripeClient`. | Prüfung | Deckt ab | |---|---| @@ -235,9 +241,6 @@ Ein Regeldatei-Test nach Repo-Sitte: | Der Fake wirft bei gleichem Schlüssel mit anderen Parametern | §9 | | Zwei Zeilen können keine `stripe_price_id` teilen | §10 | -Der Header-Test läuft über `Http::fake()` gegen `HttpStripeClient`, nicht gegen -den Fake: geprüft werden muss, was **gesendet** wird. - ## 12. Nicht Teil dieses Vorhabens - **Ein Aufräum-Kommando** für Waisen, die niemand mehr übernehmen kann (etwa @@ -255,6 +258,7 @@ den Fake: geprüft werden muss, was **gesendet** wird. - `app/Services/Billing/AdoptStripePrice.php` - `app/Services/Stripe/IdempotencyKey.php` - `database/migrations/2026_07_31_210000_one_row_per_stripe_price.php` +- `tests/Feature/Billing/StripeIdempotencyKeyTest.php` - `tests/Feature/Billing/StripePriceAdoptionTest.php` **Geändert**