Give the operator something to do about an orphan nobody can adopt
The recognition step refuses a price that proves nothing about itself, and that is right — adopting a stranger's price is worse than minting a second one. It left an operator with a log line and no way to act on it. Reports by default and touches nothing. With --archive the orphans stop being sold, which is harmless for the reason it always is here: Stripe goes on billing every subscription already on an archived price, and nothing is sold on a price no row knows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feature/host-bootstrap
parent
048e5ba81f
commit
277a6bab83
|
|
@ -0,0 +1,109 @@
|
|||
<?php
|
||||
|
||||
namespace App\Console\Commands;
|
||||
|
||||
use App\Models\PlanFamily;
|
||||
use App\Models\StripeAddonPrice;
|
||||
use App\Models\StripePlanPrice;
|
||||
use App\Services\Stripe\StripeClient;
|
||||
use Illuminate\Console\Command;
|
||||
|
||||
/**
|
||||
* The Prices at our Products that no row of ours knows.
|
||||
*
|
||||
* App\Services\Billing\AdoptStripePrice takes over an orphan it can prove is
|
||||
* ours. What it will not take over — a Price carrying none of our metadata, or
|
||||
* one that charges in a way we never mint — it names in the log and leaves,
|
||||
* because adopting a stranger's Price is worse than minting a second one. That
|
||||
* was the right call and it left an operator with a log line and no way to act
|
||||
* on it.
|
||||
*
|
||||
* Without --archive this reports and touches nothing. With it, the orphans are
|
||||
* archived at Stripe, which is harmless for the reason it always is in this
|
||||
* codebase: Stripe goes on billing every subscription already on an archived
|
||||
* Price. It stops being SOLD, and nothing here is sold on a Price no row knows.
|
||||
*/
|
||||
class SweepOrphanStripePrices extends Command
|
||||
{
|
||||
protected $signature = 'stripe:sweep-orphan-prices
|
||||
{--archive : Stop selling the orphans instead of only listing them}
|
||||
{--dry-run : Show what would be archived without touching Stripe}';
|
||||
|
||||
protected $description = 'List the Stripe prices at our products that no row knows, and optionally archive them';
|
||||
|
||||
public function handle(StripeClient $stripe): int
|
||||
{
|
||||
$dryRun = (bool) $this->option('dry-run');
|
||||
$archive = (bool) $this->option('archive');
|
||||
|
||||
if (! $dryRun && ! $stripe->isConfigured()) {
|
||||
$this->error('Stripe is not configured (STRIPE_SECRET is empty). Nothing was read.');
|
||||
|
||||
return self::FAILURE;
|
||||
}
|
||||
|
||||
$known = StripeAddonPrice::query()->pluck('stripe_price_id')
|
||||
->merge(StripePlanPrice::query()->pluck('stripe_price_id'))
|
||||
->filter()
|
||||
->flip();
|
||||
|
||||
$found = 0;
|
||||
|
||||
foreach ($this->products() as $productId) {
|
||||
foreach ($stripe->activePricesFor($productId) as $price) {
|
||||
if ($known->has($price['id'])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$found++;
|
||||
|
||||
$this->line(sprintf(
|
||||
' orphan %s %d %s %s product %s %s',
|
||||
$price['id'],
|
||||
$price['unit_amount'],
|
||||
$price['currency'],
|
||||
$price['interval'],
|
||||
$productId,
|
||||
$price['metadata'] === [] ? 'no metadata' : json_encode($price['metadata']),
|
||||
));
|
||||
|
||||
if ($archive && ! $dryRun) {
|
||||
$stripe->archivePrice($price['id']);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$this->newLine();
|
||||
|
||||
if ($found === 0) {
|
||||
$this->info('Every active price at our products is accounted for.');
|
||||
|
||||
return self::SUCCESS;
|
||||
}
|
||||
|
||||
$this->info(match (true) {
|
||||
$dryRun && $archive => "{$found} orphan(s) would be archived. Run without --dry-run to archive them.",
|
||||
$archive => "{$found} orphan(s) archived. Existing subscriptions on them keep billing.",
|
||||
default => "{$found} orphan(s) found. Run with --archive to stop selling them.",
|
||||
});
|
||||
|
||||
return self::SUCCESS;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every Stripe Product this platform owns: one per plan family, one per
|
||||
* module. Read from our own rows rather than from Stripe, because a Product
|
||||
* we do not know is not one whose Prices we can judge.
|
||||
*
|
||||
* @return array<int, string>
|
||||
*/
|
||||
private function products(): array
|
||||
{
|
||||
return PlanFamily::query()->whereNotNull('stripe_product_id')->pluck('stripe_product_id')
|
||||
->merge(StripeAddonPrice::query()->pluck('stripe_product_id'))
|
||||
->filter()
|
||||
->unique()
|
||||
->values()
|
||||
->all();
|
||||
}
|
||||
}
|
||||
|
|
@ -250,7 +250,7 @@ Und an `StripeIdempotencyKeyTest.php`: Blättern **wirft** bei fehlender `id`
|
|||
- `app/Services/Billing/AddonPrices.php` — Produkt-Wiedererkennung in
|
||||
`product()`
|
||||
- `app/Providers/AppServiceProvider.php` — beide Adoptions-Klassen als
|
||||
Singleton, damit ihr Lauf-Protokoll (`$duplicates`, `$adoptions`) trägt
|
||||
Singleton, damit ihr Lauf-Protokoll (`$duplicates`) trägt
|
||||
- `app/Console/Commands/SyncStripeCatalogue.php` — Produkt-Wiedererkennung,
|
||||
getrennte Zählung, Kommentar berichtigt
|
||||
- `tests/Feature/Billing/StripePriceAdoptionTest.php`,
|
||||
|
|
|
|||
|
|
@ -277,10 +277,11 @@ Zwei Regeldatei-Tests nach Repo-Sitte, getrennt danach, **gegen wen** sie prüfe
|
|||
|
||||
## 14. Folgepunkte, die aus der Umsetzung entstanden sind
|
||||
|
||||
Nachgetragen nach dem Abschluss-Review. Alle Befunde sind offengelegt, keiner
|
||||
blockiert den Merge — aber keiner ist erledigt.
|
||||
Nachgetragen nach dem Abschluss-Review. Die Fortsetzung steht in
|
||||
`docs/superpowers/specs/2026-07-30-stripe-adoption-followups-design.md`; jeder
|
||||
Punkt unten ist abgehakt, mit der Datei, die ihn schließt.
|
||||
|
||||
### Die eine echte Lücke: Waisen-**Produkte**
|
||||
### Die eine echte Lücke: Waisen-**Produkte** — erledigt
|
||||
|
||||
`activePricesFor()` erkennt einen verwaisten *Preis* wieder. Für ein verwaistes
|
||||
**Produkt** gibt es kein Gegenstück, und dort ist der Schaden größer: ein
|
||||
|
|
@ -295,7 +296,11 @@ benennen die Lücke jetzt; der Fix wäre ein `productsFor()` nach dem Muster von
|
|||
`activePricesFor()`, das Stripe nach den Produkten fragt und über die Metadaten
|
||||
wiedererkennt.
|
||||
|
||||
### Das Geld-Tor sitzt an der falschen Stelle
|
||||
**Geschlossen durch:** `App\Services\Billing\AdoptStripeProduct`, beide
|
||||
Verdrahtungen (`SyncStripeCatalogue::handle()` für eine Familie,
|
||||
`AddonPrices::product()` für ein Modul).
|
||||
|
||||
### Das Geld-Tor sitzt an der falschen Stelle — erledigt
|
||||
|
||||
`AdoptStripePrice` verspricht in seinem Kopfkommentar, keine Übernahme könne
|
||||
Geld verschieben — kann aber nur Betrag, Währung und Intervall vergleichen. Was
|
||||
|
|
@ -307,53 +312,62 @@ diese Felder gar nicht ausdrücken, also kann **kein Test von
|
|||
`StripeClient` ließe es still fallen. Die Prüfung gehört in die Klasse, die sie
|
||||
zusagt.
|
||||
|
||||
**Geschlossen durch:** die Prüfung sitzt jetzt in `AdoptStripePrice` selbst;
|
||||
`activePricesFor()`s Vertrag liefert die vier preisbestimmenden Eigenschaften,
|
||||
statt selbst zu filtern.
|
||||
|
||||
Verwandt: `PlanPrices::ensure()` hat keine `<= 0`-Schranke, wo
|
||||
`AddonPrices::ensure()` eine hat. Das ist das Einzige, was einen
|
||||
`billing_scheme: tiered`-Preis (bei dem Stripe `unit_amount: null` liefert, hier
|
||||
als 0 gelesen) auf der Paketseite überhaupt erreichbar macht.
|
||||
|
||||
**Bewusst nicht gebaut.** Begründung in der neuen Spec §3: sobald
|
||||
`AdoptStripePrice` selbst `billing_scheme !== 'per_unit'` ablehnt, hat die
|
||||
Schranke keinen Zweck mehr — sie würde nur noch ein Paket zu null (ein
|
||||
kostenloser Tarif, heute nicht im Katalog, aber jederzeit anlegbar) treffen und
|
||||
es unverkäuflich machen.
|
||||
|
||||
### Kleineres, benannt statt vergessen
|
||||
|
||||
- **Der Abgleich zählt „angelegt", auch wo er übernommen hat.** Die Wortwahl der
|
||||
Konsolenausgabe ist entschärft („created or adopted"), gezählt wird weiterhin
|
||||
vor dem Aufruf. Eine echte Trennung der beiden Zahlen fehlt — und das ist die
|
||||
eine Zahl, die ein Mensch nach einem Vorfall zuerst liest.
|
||||
**Erledigt** — getrennte Zählung in `SyncStripeCatalogue::handle()`.
|
||||
- **Die Warnung über einen unerklärlichen Preis hat keine Drosselung.** Sie
|
||||
feuert bei jedem Abgleich *und* jeder Kundenbuchung, solange die Waise
|
||||
existiert — und weil das Aufräum-Kommando (§12) ein Folgepunkt ist, auf
|
||||
unbestimmte Zeit. Der Widerspruchs-Pfad wurde aus genau diesem Grund still
|
||||
gestellt; der Bestätigungs-Pfad bekam dieselbe Überlegung nicht.
|
||||
**Erledigt** — einmal pro Preis und Tag, über `Cache::add()`.
|
||||
- **`FakeStripeClient::updatePriceMetadata()` ersetzt, wo Stripe zusammenführt**,
|
||||
und `activePricesFor()` gibt Metadaten ungecastet zurück, wo der HTTP-Client
|
||||
auf String castet. Heute nicht beobachtbar, aber es ist genau die Sorte
|
||||
Untreue des Fakes, gegen die §9 geschrieben wurde.
|
||||
**Erledigt.**
|
||||
- **`created` im Fake kann gleichstehen**: `createPrice()` stempelt
|
||||
`count($prices) + 1`, `plantPrice()` hat die Vorgabe `1`. Der Kommentar sagt
|
||||
das jetzt zu, statt es zu bestreiten; ein Gleichstand-Tiebreak auf die ID
|
||||
fehlt weiter.
|
||||
**Erledigt** — Gleichstand wird über die Preis-ID aufgelöst.
|
||||
- **`HttpStripeClient::activePricesFor()` bricht das Blättern still ab**, wenn
|
||||
dem letzten Eintrag einer nicht-letzten Seite die `id` fehlt — getreue Kopie
|
||||
desselben Verhaltens in `invoiceLines()`. Die Folge hat sich aber geändert: in
|
||||
`invoiceLines()` entsteht ein zu kurzer Belegtext, hier eine ungesehene Waise
|
||||
und ein zweiter aktiver Preis, also der Vorfall. Wer es anfasst, soll werfen
|
||||
statt anhalten.
|
||||
**Erledigt** — wirft jetzt, für Preise und Produkte.
|
||||
- **Der Idempotenz-Fingerabdruck und der POST-Rumpf müssen zusammenbleiben.**
|
||||
Ein Feld, das künftig in `createPrice()` gesendet, aber nicht in
|
||||
`IdempotencyKey::priceParameters()` aufgenommen wird, öffnet den Vorfall
|
||||
wieder. Der Test „does not block a booking because the metadata format moved"
|
||||
wacht über diese Nahtstelle — mit einem gesetzten Ledger-Eintrag, weil der
|
||||
Zustand heute nicht mehr fahrbar ist.
|
||||
**Bleibt offen** — es ist eine Naht, kein Fehler; der Test darüber existiert.
|
||||
|
||||
### Zwei fremde, vorbestehende Testfehler, die dieser Zweig aufgedeckt hat
|
||||
### Ein fremder, vorbestehender Testfehler, den dieser Zweig aufgedeckt hat
|
||||
|
||||
Keiner davon wird von diesem Zweig verursacht.
|
||||
|
||||
1. **`HostStepsTest.php:1064`** schreibt `fsn-01.node.clupilot.com` hart hinein,
|
||||
während `.env:107` `CLUPILOT_DNS_ZONE=clupilot.cloud` setzt und `phpunit.xml`
|
||||
neun andere Variablen mit `force="true"` festnagelt, diese aber nicht. Eine
|
||||
Zeile in `phpunit.xml`. Derselbe Punkt steht als offene Aufgabe in
|
||||
`docs/handoffs/2026-07-30-real-run-handoff.md`.
|
||||
2. **`PlanCatalogueTest.php`** ist einmal ausgefallen und lief beim
|
||||
Wiederholen durch. „Beim zweiten Mal grün" ist keine Diagnose, und ein
|
||||
sprunghafter Fehler in den Katalogpreis-Tests liegt im Wirkungskreis dieses
|
||||
Zweigs, auch wenn er nicht von ihm kommt.
|
||||
**`PlanCatalogueTest.php`** ist einmal ausgefallen und lief beim Wiederholen
|
||||
durch. „Beim zweiten Mal grün" ist keine Diagnose, und ein sprunghafter Fehler
|
||||
in den Katalogpreis-Tests liegt im Wirkungskreis dieses Zweigs, auch wenn er
|
||||
nicht von ihm kommt. **Bleibt offen** — Ursachensuche, eigene Sitzung.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,66 @@
|
|||
<?php
|
||||
|
||||
use App\Models\StripeAddonPrice;
|
||||
use App\Services\Stripe\FakeStripeClient;
|
||||
use App\Services\Stripe\StripeClient;
|
||||
use Illuminate\Contracts\Console\Kernel;
|
||||
|
||||
/**
|
||||
* The orphans nobody can adopt.
|
||||
*
|
||||
* A Price at one of our Products that no row knows, and that AdoptStripePrice
|
||||
* refuses — because nothing in its metadata proves it is ours, or because it
|
||||
* charges in a way we never mint. The recognition step names it in the log and
|
||||
* leaves it, deliberately: adopting a stranger's Price is worse than minting a
|
||||
* second one. This is the other half of that decision, the one an operator can
|
||||
* act with.
|
||||
*/
|
||||
beforeEach(function () {
|
||||
$this->stripe = new FakeStripeClient;
|
||||
app()->instance(StripeClient::class, $this->stripe);
|
||||
app(Kernel::class)->call('stripe:sync-catalogue');
|
||||
|
||||
$this->product = (string) StripeAddonPrice::query()
|
||||
->where('addon_key', 'priority_support')
|
||||
->value('stripe_product_id');
|
||||
|
||||
$this->known = (string) StripeAddonPrice::query()
|
||||
->where('addon_key', 'priority_support')
|
||||
->where('interval', 'month')
|
||||
->where('reverse_charge', false)
|
||||
->value('stripe_price_id');
|
||||
|
||||
$this->stripe->plantPrice('price_orphan', $this->product, 9900, 'EUR', 'month', []);
|
||||
});
|
||||
|
||||
it('reports the orphan and leaves it alone', function () {
|
||||
$this->artisan('stripe:sweep-orphan-prices')
|
||||
->expectsOutputToContain('price_orphan')
|
||||
->assertSuccessful();
|
||||
|
||||
expect($this->stripe->archived)->toBe([]);
|
||||
});
|
||||
|
||||
it('never reports a price a row knows', function () {
|
||||
$this->artisan('stripe:sweep-orphan-prices')
|
||||
->doesntExpectOutputToContain($this->known)
|
||||
->assertSuccessful();
|
||||
});
|
||||
|
||||
it('stops selling the orphan when asked to', function () {
|
||||
$this->artisan('stripe:sweep-orphan-prices', ['--archive' => true])
|
||||
->assertSuccessful();
|
||||
|
||||
// Archiving is harmless for the same reason it always is here: Stripe goes
|
||||
// on billing every subscription already on an archived Price, it merely
|
||||
// stops being sold.
|
||||
expect($this->stripe->archived)->toBe(['price_orphan']);
|
||||
});
|
||||
|
||||
it('touches nothing on a dry run, even when asked to archive', function () {
|
||||
$this->artisan('stripe:sweep-orphan-prices', ['--archive' => true, '--dry-run' => true])
|
||||
->expectsOutputToContain('price_orphan')
|
||||
->assertSuccessful();
|
||||
|
||||
expect($this->stripe->archived)->toBe([]);
|
||||
});
|
||||
Loading…
Reference in New Issue