From 4de44e7ab098f1a1da204460c97bf6c8d3786023 Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Thu, 30 Jul 2026 11:02:49 +0200
Subject: [PATCH] Recognise the price Stripe already has
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <noreply@anthropic.com>
---
 app/Services/Billing/AdoptStripePrice.php     | 170 ++++++++++++++++++
 .../Billing/StripePriceAdoptionTest.php       | 132 ++++++++++++++
 2 files changed, 302 insertions(+)
 create mode 100644 app/Services/Billing/AdoptStripePrice.php
 create mode 100644 tests/Feature/Billing/StripePriceAdoptionTest.php

diff --git a/app/Services/Billing/AdoptStripePrice.php b/app/Services/Billing/AdoptStripePrice.php
new file mode 100644
index 0000000..fd2dd1d
--- /dev/null
+++ b/app/Services/Billing/AdoptStripePrice.php
@@ -0,0 +1,170 @@
+<?php
+
+namespace App\Services\Billing;
+
+use App\Services\Stripe\StripeClient;
+use Illuminate\Support\Facades\Log;
+
+/**
+ * The Stripe Price we were about to create and 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 idempotency key covers the next twenty-four hours; after that it
+ * covers nothing, and the run that follows makes a SECOND live Price for the
+ * same money — precisely the duplicate the key was there to prevent. It happened
+ * on 2026-07-29 with priority_support at 3480 EUR.
+ *
+ * So before minting, we ask. Nothing else in the catalogue ever did.
+ *
+ * **What may be taken over, and why so narrowly.** Same amount, same currency,
+ * same interval — a Price at another figure would move money, and moving money
+ * is what this must never do: a running contract keeps the Price it was sold on
+ * and every booking stays frozen at what it cost that day. Then proof: the
+ * candidate must not contradict our metadata, and it must confirm at least one
+ * identifying key of it. An active Price at the right money on our own Product
+ * with no metadata at all is what a person clicking through Stripe's dashboard
+ * leaves behind, and adopting that would be worse than minting a second one.
+ * Finally it must be unclaimed — a Price id belongs to one row, which is what
+ * the unique index on both registers now enforces.
+ *
+ * **Several orphans.** The oldest is adopted, because it is the one a lost row
+ * is likeliest to have been billing on, and the others are archived at Stripe.
+ * Archiving stops a Price being SOLD and leaves every subscription on it exactly
+ * where it was — the same grandfathering archiveSuperseded() has always relied
+ * on — so the rule "one live Price per figure" is restored without touching a
+ * single contract.
+ *
+ * **Reported through the log, not the console.** AddonPrices::ensure() runs
+ * inside a customer's module booking as well as in the sweep, and there is no
+ * command line there.
+ */
+final class AdoptStripePrice
+{
+    public function __construct(private readonly StripeClient $stripe) {}
+
+    /**
+     * The id of an existing Stripe Price to use instead of creating one, or null.
+     *
+     * @param  array<string, string>  $metadata  what the create call would send
+     * @param  array<int, string>  $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<string, string>  $found
+     * @param  array<string, string>  $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<string, string>  $found
+     * @param  array<string, string>  $expected
+     * @param  array<int, string>  $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;
+    }
+}
diff --git a/tests/Feature/Billing/StripePriceAdoptionTest.php b/tests/Feature/Billing/StripePriceAdoptionTest.php
new file mode 100644
index 0000000..68690d1
--- /dev/null
+++ b/tests/Feature/Billing/StripePriceAdoptionTest.php
@@ -0,0 +1,132 @@
+<?php
+
+use App\Services\Billing\AdoptStripePrice;
+use App\Services\Stripe\FakeStripeClient;
+use App\Services\Stripe\StripeClient;
+use Illuminate\Support\Facades\Log;
+
+/**
+ * A run that dies between Stripe's create and our insert leaves an orphan — and
+ * the next run must recognise it instead of minting a second live Price for the
+ * same money.
+ *
+ * On 2026-07-29 23:11 a sweep created price_1TygdEC7u8NpJ8pOt3nsoyYw
+ * (priority_support, 3480 EUR, monthly) and died before the row was written.
+ * The idempotency key covered the next twenty-four hours and then stopped
+ * covering anything; after that, the guard against a duplicate was nothing at
+ * all. This is that guard.
+ *
+ * What may be adopted is narrow on purpose: same money, same interval, same
+ * currency, proof in the metadata that the Price is ours, and no row claiming
+ * it already. Adopting somebody else's Price would be a worse failure than
+ * minting a second one, and adopting a Price at a DIFFERENT amount would move
+ * money — which nothing here is allowed to do.
+ */
+beforeEach(function () {
+    $this->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');
+});