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 <noreply@anthropic.com>

app/Services/Billing/PlanPrices.php

@@ -40,7 +40,10 @@ use App\Services\Stripe\StripeClient;
  */
 final class PlanPrices
 {
-    public function __construct(private readonly StripeClient $stripe) {}
+    public function __construct(
+        private readonly StripeClient $stripe,
+        private readonly AdoptStripePrice $adopt,
+    ) {}
 
     /** What Stripe is asked to take for this row from a customer treated so. */
     public static function chargedCents(PlanPrice $price, TaxTreatment $treatment): int
@@ -131,30 +134,49 @@ final class PlanPrices
         $existing = $this->registered($price, $treatment, includeArchived: true);
 
         $priceId = $existing?->stripe_price_id;
+        $interval = $price->term === Subscription::TERM_YEARLY ? 'year' : 'month';
 
         if ($priceId === null) {
-            $priceId = $this->stripe->createPrice(
+            $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: $price->term === Subscription::TERM_YEARLY ? 'year' : 'month',
-                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',
-                ],
-                // The CHARGED amount is part of the key, so a run after a rate
-                // change cannot replay the Price minted at the old figure. So is
-                // the treatment, and it has to be: at a rate of nought the two
-                // Prices are the same amount, and one key would have Stripe hand
-                // back the same object for both — which the register, where a
-                // Price id is unique, would refuse to record twice.
+                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' : ''),
             );

tests/Feature/Billing/StripePriceAdoptionTest.php

@@ -1,10 +1,13 @@
 <?php
 
+use App\Models\PlanPrice;
 use App\Models\StripeAddonPrice;
+use App\Models\StripePlanPrice;
 use App\Models\Subscription;
 use App\Models\SubscriptionAddon;
 use App\Services\Billing\AddonPrices;
 use App\Services\Billing\AdoptStripePrice;
+use App\Services\Billing\PlanPrices;
 use App\Services\Billing\TaxTreatment;
 use App\Services\Stripe\FakeStripeClient;
 use App\Services\Stripe\StripeClient;
@@ -299,3 +302,54 @@ it('leaves a frozen booking on the price it was sold at', function () {
             'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(),
         ))->toBe($sold);
 });
+
+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');
+});