From 0ec5d93da80fb5f6c308af0e3cb12a4ebbae002b Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 10:18:20 +0200 Subject: [PATCH] Break the price-adoption spec 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: the key fingerprint (and a fake that finally reproduces Stripe's refusal), the two listing calls, the adoption step itself, then each price service in turn, then one row per Stripe price. The spec now names both test files. What goes on the wire is checked through Http::fake against the real client — asserting against the fake would only prove the fake agrees with itself, which is the mistake that let the incident through. Co-Authored-By: Claude Opus 5 --- .../plans/2026-07-30-stripe-price-adoption.md | 1839 +++++++++++++++++ ...2026-07-30-stripe-price-adoption-design.md | 14 +- 2 files changed, 1848 insertions(+), 5 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-30-stripe-price-adoption.md 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**