Say what was adopted, not that it was created

The count is taken before ensure() runs, so the sweep reported objects created in
Stripe when it had made none — and after the next interrupted run, that line is
the first thing a human reads. Telling the two apart is the whole point of the
recognition step.

Comparing the price id before and after the call does not distinguish them
either: there is no id before, in either case. AdoptStripePrice counts its own
adoptions instead, which is why it and its product sibling are now singletons —
resolved per call, a counter on the instance would never pass one.

Duplicate products are named in the report as well as the log. We deliberately do
not deactivate them, so only a person can resolve one, and a person reads this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

app/Console/Commands/SyncStripeCatalogue.php

@@ -8,6 +8,7 @@ use App\Models\PlanVersion;
 use App\Models\Subscription;
 use App\Services\Billing\AddonCatalogue;
 use App\Services\Billing\AddonPrices;
+use App\Services\Billing\AdoptStripePrice;
 use App\Services\Billing\AdoptStripeProduct;
 use App\Services\Billing\PlanPrices;
 use App\Services\Billing\TaxTreatment;
@@ -84,6 +85,14 @@ class SyncStripeCatalogue extends Command
             return self::FAILURE;
         }
 
+        // Resolved once, as singletons: each counts across the WHOLE run, and a
+        // count taken before ensure()/the adoption call runs can never itself
+        // distinguish "created" from "adopted" — see AdoptStripePrice.
+        $adoptPrices = app(AdoptStripePrice::class);
+        $adoptProducts = app(AdoptStripeProduct::class);
+        $adoptedPricesBefore = $adoptPrices->adoptions;
+        $adoptedProductsBefore = $adoptProducts->adoptions;
+
         $created = 0;
 
         foreach (PlanFamily::query()->with('versions.prices')->orderBy('tier')->get() as $family) {
@@ -110,7 +119,7 @@ class SyncStripeCatalogue extends Command
                     // below stops protecting it after twenty-four hours.
                     // plan_family_id is what proves such a Product is this
                     // family's.
-                    $productId = app(AdoptStripeProduct::class)($metadata, ['plan_family_id']);
+                    $productId = $adoptProducts($metadata, ['plan_family_id']);
 
                     $productId ??= $stripe->createProduct(
                         $family->name,
@@ -136,22 +145,33 @@ class SyncStripeCatalogue extends Command
 
         $this->newLine();
 
+        // Both counted the same way and for the same reason: $created is an
+        // INTENT taken before we know whether ensure()/the adoption call will
+        // mint or adopt, for a price as well as for a family's product — see
+        // AdoptStripePrice. Combined into one "adopted" figure because an
+        // operator reading the summary line cares whether Stripe made anything
+        // NEW, not which kind of object it would have been.
+        $adoptedPrices = $adoptPrices->adoptions - $adoptedPricesBefore;
+        $adoptedProducts = $adoptProducts->adoptions - $adoptedProductsBefore;
+        $adopted = $adoptedPrices + $adoptedProducts;
+        $minted = max(0, $created - $adopted);
+
+        foreach ($adoptProducts->duplicates as $duplicate) {
+            // Named here as well as in the log: an operator running the sweep
+            // reads this, and a second Product for one family is something only
+            // a person can resolve — we deliberately do not deactivate it.
+            $this->warn("  duplicate product {$duplicate} — left active, a product's prices become unsellable if it is deactivated");
+        }
+
         if ($created === 0) {
             $this->info('Stripe is already in step with the catalogue.');
 
             return self::SUCCESS;
         }
 
-        // "or adopted", because the count is taken BEFORE ensure() runs and
-        // ensure() may find the Price already at Stripe and adopt it instead of
-        // creating anything — see App\Services\Billing\AdoptStripePrice. Telling
-        // which of the two happened is the whole purpose of that step, so the
-        // first thing an operator reads after an interrupted run must not assert
-        // a creation that did not take place. The log entry adoption writes names
-        // the Price it took over.
         $this->info($dryRun
-            ? "{$created} object(s) would be created or adopted. Run without --dry-run to do it."
-            : "{$created} object(s) created or adopted in Stripe.");
+            ? "{$created} object(s) would be created. Run without --dry-run to create them."
+            : "{$minted} object(s) created, {$adopted} adopted in Stripe.");
 
         return self::SUCCESS;
     }

app/Providers/AppServiceProvider.php

@@ -13,6 +13,7 @@ use App\Mail\Transport\MailboxTransport;
 use App\Models\Customer;
 use App\Models\MaintenanceNotification;
 use App\Provisioning\PipelineRegistry;
+use App\Services\Billing\AdoptStripePrice;
 use App\Services\Billing\AdoptStripeProduct;
 use App\Services\Billing\CustomDomainAccess;
 use App\Services\Dns\FileHostDnsDirectory;
@@ -58,12 +59,15 @@ class AppServiceProvider extends ServiceProvider
             config('provisioning.pipelines', []),
         ));
 
-        // Singletons because both carry a record across one run that the caller
-        // reads afterwards: which products were duplicates, and how many prices
-        // were adopted rather than created. Resolved per call they would each
-        // start empty — SyncStripeCatalogue asks the container for PlanPrices
-        // once per catalogue row.
+        // Both singletons because each carries a record across one run that the
+        // caller reads only after it finishes: which products were duplicates,
+        // and how many products and prices were adopted rather than created.
+        // Resolved per call they would each start empty on every read. PlanPrices
+        // is deliberately NOT a singleton — SyncStripeCatalogue asks the container
+        // for it once per catalogue row — which is exactly why the counter has to
+        // live here instead: on a per-call instance it could never pass one.
         $this->app->singleton(AdoptStripeProduct::class);
+        $this->app->singleton(AdoptStripePrice::class);
 
         // Real I/O implementations; tests swap in fakes via app()->instance().
         $this->app->bind(RemoteShell::class, PhpseclibRemoteShell::class);

app/Services/Billing/AdoptStripePrice.php

@@ -42,6 +42,16 @@ use Illuminate\Support\Facades\Log;
  */
 final class AdoptStripePrice
 {
+    /**
+     * How many Prices this run took over rather than minted.
+     *
+     * Read by stripe:sync-catalogue, which counts an intent BEFORE calling
+     * ensure() and so cannot tell the two apart on its own — comparing the Price
+     * id before and after does not distinguish them either, because there is no
+     * id before in either case.
+     */
+    public int $adoptions = 0;
+
     public function __construct(private readonly StripeClient $stripe) {}
 
     /**
@@ -170,6 +180,8 @@ final class AdoptStripePrice
             'interval' => $interval,
         ]);
 
+        $this->adoptions++;
+
         return $adopted['id'];
     }
 

app/Services/Billing/AdoptStripeProduct.php

@@ -44,6 +44,16 @@ final class AdoptStripeProduct
      */
     public array $duplicates = [];
 
+    /**
+     * How many Products this run took over rather than minted.
+     *
+     * Mirrors AdoptStripePrice::$adoptions, and for the same reason:
+     * stripe:sync-catalogue counts an intent to create a Product BEFORE it
+     * knows whether this class will hand back an existing one, so a Product
+     * that was adopted must not silently count as one that was created.
+     */
+    public int $adoptions = 0;
+
     public function __construct(private readonly StripeClient $stripe) {}
 
     /**
@@ -98,6 +108,8 @@ final class AdoptStripeProduct
             'metadata' => $metadata,
         ]);
 
+        $this->adoptions++;
+
         return $adopted['id'];
     }
 

tests/Feature/Billing/StripeProductAdoptionTest.php

@@ -1,10 +1,13 @@
 <?php
 
 use App\Models\PlanFamily;
+use App\Models\PlanPrice;
 use App\Models\StripeAddonPrice;
+use App\Models\StripePlanPrice;
 use App\Models\Subscription;
 use App\Services\Billing\AddonPrices;
 use App\Services\Billing\AdoptStripeProduct;
+use App\Services\Billing\PlanPrices;
 use App\Services\Billing\TaxTreatment;
 use App\Services\Stripe\FakeStripeClient;
 use App\Services\Stripe\StripeClient;
@@ -111,3 +114,41 @@ it('takes the oldest of two matching products and leaves the other alone', funct
 
     Log::shouldHaveReceived('warning')->once();
 });
+
+it('says it adopted, not that it created', function () {
+    // The one signal a human reads after an incident. The count is taken before
+    // ensure() runs, so without this the run reports objects created in Stripe
+    // when it made none — and telling those two apart is the whole point of the
+    // recognition step.
+    app(Kernel::class)->call('stripe:sync-catalogue');
+
+    $row = PlanPrice::query()->firstOrFail();
+    $charged = PlanPrices::chargedCents($row, TaxTreatment::domestic());
+    $orphan = (string) StripePlanPrice::query()
+        ->where('plan_price_id', $row->id)->where('reverse_charge', false)
+        ->value('stripe_price_id');
+    StripePlanPrice::query()->where('stripe_price_id', $orphan)->delete();
+    $this->stripe->keys = [];
+
+    $this->artisan('stripe:sync-catalogue')
+        ->expectsOutputToContain('1 adopted')
+        ->assertSuccessful();
+});
+
+it('names a duplicate product in its report, without touching it', function () {
+    // Nothing has been synced yet, same as the orphan test above — the family
+    // has no product of its own, and two stray ones claiming to be it are the
+    // whole of what Stripe has for it.
+    $family = PlanFamily::query()->orderBy('tier')->firstOrFail();
+    $family->update(['stripe_product_id' => null]);
+
+    $metadata = ['plan_family' => $family->key, 'plan_family_id' => (string) $family->id];
+    $this->stripe->plantProduct('prod_first', $family->name, $metadata, created: 100);
+    $this->stripe->plantProduct('prod_second', $family->name, $metadata, created: 200);
+
+    $this->artisan('stripe:sync-catalogue')
+        ->expectsOutputToContain('prod_second')
+        ->assertSuccessful();
+
+    expect($family->refresh()->stripe_product_id)->toBe('prod_first');
+});