From e2b8f25e78fe5cd2f4819b8c03530b066bad8651 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 12:19:31 +0200 Subject: [PATCH] fix(engine-b): address Codex round 5 (capacity release, activate-last, guest-ip poll, email unique) - Failed run releases the instance (status=failed) so its quota stops counting against host capacity. - CompleteProvisioning activates instance+order only AFTER onboarding tasks + credential delivery succeed. - ConfigureNetwork polls until the guest has an address (no wrong Traefik route). - customers.email unique + race-safe customer resolution in the intake action. Co-Authored-By: Claude Opus 4.8 --- app/Actions/StartCustomerProvisioning.php | 27 ++++++++++++++----- app/Models/Order.php | 7 ++++- .../Steps/Customer/CompleteProvisioning.php | 10 ++++--- .../Steps/Customer/ConfigureNetwork.php | 17 ++++++++---- ...10_add_unique_email_to_customers_table.php | 24 +++++++++++++++++ .../Provisioning/CustomerDataModelTest.php | 8 ++++-- .../CustomerProvisioningEndToEndTest.php | 4 ++- .../Provisioning/CustomerStepsTest.php | 6 +++++ 8 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 database/migrations/2026_07_25_080010_add_unique_email_to_customers_table.php diff --git a/app/Actions/StartCustomerProvisioning.php b/app/Actions/StartCustomerProvisioning.php index d1e03db..df339a2 100644 --- a/app/Actions/StartCustomerProvisioning.php +++ b/app/Actions/StartCustomerProvisioning.php @@ -25,13 +25,10 @@ class StartCustomerProvisioning return null; // already processed } - try { - [$order, $run] = DB::transaction(function () use ($event) { - $customer = Customer::query()->firstOrCreate( - ['email' => $event['email']], - ['name' => $event['name'] ?? $event['email'], 'stripe_customer_id' => $event['stripe_customer_id'] ?? null], - ); + $customer = $this->resolveCustomer($event); + try { + [$order, $run] = DB::transaction(function () use ($event, $customer) { $order = Order::create([ 'customer_id' => $customer->id, 'plan' => $event['plan'], @@ -61,4 +58,22 @@ class StartCustomerProvisioning return $order; } + + /** + * Find or create the customer, race-safe against the unique email index so a + * concurrent first-time purchase can't create two customers for one email. + * + * @param array{email:string,name:?string,stripe_customer_id:?string} $event + */ + private function resolveCustomer(array $event): Customer + { + try { + return Customer::query()->firstOrCreate( + ['email' => $event['email']], + ['name' => $event['name'] ?? $event['email'], 'stripe_customer_id' => $event['stripe_customer_id'] ?? null], + ); + } catch (UniqueConstraintViolationException) { + return Customer::query()->where('email', $event['email'])->firstOrFail(); + } + } } diff --git a/app/Models/Order.php b/app/Models/Order.php index 8151a7b..73485e4 100644 --- a/app/Models/Order.php +++ b/app/Models/Order.php @@ -40,9 +40,14 @@ class Order extends Model implements ProvisioningSubject return $this->morphMany(ProvisioningRun::class, 'subject'); } - /** Runner hook: a failed provisioning run marks the order failed (no auto-refund). */ + /** + * Runner hook: a failed provisioning run marks the order failed (no + * auto-refund) AND releases the reserved instance, so its quota stops + * counting against host capacity. + */ public function onProvisioningFailed(): void { $this->update(['status' => 'failed']); + $this->instance()->update(['status' => 'failed']); } } diff --git a/app/Provisioning/Steps/Customer/CompleteProvisioning.php b/app/Provisioning/Steps/Customer/CompleteProvisioning.php index 2641dbb..9272acc 100644 --- a/app/Provisioning/Steps/Customer/CompleteProvisioning.php +++ b/app/Provisioning/Steps/Customer/CompleteProvisioning.php @@ -25,14 +25,12 @@ class CompleteProvisioning extends CustomerStep $order = $this->order($run); $instance = $this->instance($run); - $instance->update(['status' => 'active']); - $order->update(['status' => 'active']); - + // Do all fallible work FIRST, so a failure can't leave the instance + // active. Onboarding tasks (idempotent) + credential delivery. foreach (['invite_users', 'upload_logo', 'connect_desktop', 'explore_apps'] as $key) { $instance->onboardingTasks()->firstOrCreate(['key' => $key], ['done' => false]); } - // Deliver the initial credentials, then scrub the transient password. $encrypted = $run->context('admin_password'); if ($encrypted !== null) { Notification::route('mail', $order->customer->email)->notify( @@ -41,6 +39,10 @@ class CompleteProvisioning extends CustomerStep $run->forgetContext('admin_password'); } + // Activate LAST — only once everything above succeeded. + $instance->update(['status' => 'active']); + $order->update(['status' => 'active']); + return StepResult::advance(); } } diff --git a/app/Provisioning/Steps/Customer/ConfigureNetwork.php b/app/Provisioning/Steps/Customer/ConfigureNetwork.php index 30cb4a3..0655de5 100644 --- a/app/Provisioning/Steps/Customer/ConfigureNetwork.php +++ b/app/Provisioning/Steps/Customer/ConfigureNetwork.php @@ -17,7 +17,8 @@ class ConfigureNetwork extends CustomerStep public function maxDuration(): int { - return 120; + // Above the 90s guest-ip own-deadline below. + return 150; } public function execute(ProvisioningRun $run): StepResult @@ -29,12 +30,18 @@ class ConfigureNetwork extends CustomerStep $this->guest($pve, $run, 'hostnamectl set-hostname '.escapeshellarg($instance->subdomain)); - // Capture the guest's own address so Traefik can route to the VM (not the host). - $ipOut = trim($pve->guestExec($node, $vmid, "hostname -I")['out-data'] ?? ''); + // Capture the guest's own address so Traefik routes to the VM, not the + // host. Poll until it's available rather than advancing with no address. + $ipOut = trim($pve->guestExec($node, $vmid, 'hostname -I')['out-data'] ?? ''); $guestIp = trim(strtok($ipOut, ' ') ?: ''); - if ($guestIp !== '') { - $instance->update(['guest_ip' => $guestIp]); + if ($guestIp === '') { + if ($run->started_at !== null && $run->started_at->copy()->addSeconds(90)->isPast()) { + return StepResult::fail('guest_ip_unavailable'); + } + + return StepResult::poll(10, 'waiting for the guest to obtain an address'); } + $instance->update(['guest_ip' => $guestIp]); // Allow HTTP/HTTPS, deny everything else (management stays on the tunnel). $pve->applyFirewall($node, $vmid, [ diff --git a/database/migrations/2026_07_25_080010_add_unique_email_to_customers_table.php b/database/migrations/2026_07_25_080010_add_unique_email_to_customers_table.php new file mode 100644 index 0000000..2782e9e --- /dev/null +++ b/database/migrations/2026_07_25_080010_add_unique_email_to_customers_table.php @@ -0,0 +1,24 @@ +unique('email'); + }); + } + + public function down(): void + { + Schema::table('customers', function (Blueprint $table) { + $table->dropUnique(['email']); + }); + } +}; diff --git a/tests/Feature/Provisioning/CustomerDataModelTest.php b/tests/Feature/Provisioning/CustomerDataModelTest.php index 51880ba..f9b41ff 100644 --- a/tests/Feature/Provisioning/CustomerDataModelTest.php +++ b/tests/Feature/Provisioning/CustomerDataModelTest.php @@ -18,12 +18,16 @@ it('resolves an Order as the polymorphic run subject', function () { ->and($order->getRouteKeyName())->toBe('uuid'); }); -it('marks the order failed via the ProvisioningSubject hook', function () { +it('marks the order and releases its instance via the ProvisioningSubject hook', function () { $order = Order::factory()->create(['status' => 'provisioning']); + $instance = Instance::factory()->create([ + 'order_id' => $order->id, 'customer_id' => $order->customer_id, 'status' => 'provisioning', + ]); $order->onProvisioningFailed(); - expect($order->fresh()->status)->toBe('failed'); + expect($order->fresh()->status)->toBe('failed') + ->and($instance->fresh()->status)->toBe('failed'); // quota no longer counts against capacity }); it('enforces a unique stripe_event_id (idempotency)', function () { diff --git a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php index e58137b..7ed2776 100644 --- a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php +++ b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php @@ -14,7 +14,8 @@ it('provisions a paid order all the way to active (mocked)', function () { Notification::fake(); Queue::fake(); // drive the runner manually, one step per advance $s = fakeServices(); - $s['pve']->guestScript('status.php', 0, 'ok'); // deploy health + acceptance probe + $s['pve']->guestScript('status.php', 0, 'ok'); + $s['pve']->guestScript('hostname -I', 0, '10.20.0.9'); // deploy health + acceptance probe // Fake defaults: tasks stopped/OK, guest agent up, cert reachable. Host::factory()->active()->create(['datacenter' => 'fsn', 'node' => 'pve', 'total_gb' => 1000]); @@ -62,6 +63,7 @@ it('does not duplicate external resources when a step re-runs after a crash', fu Queue::fake(); $s = fakeServices(); $s['pve']->guestScript('status.php', 0, 'ok'); + $s['pve']->guestScript('hostname -I', 0, '10.20.0.9'); Host::factory()->active()->create(['datacenter' => 'fsn', 'node' => 'pve', 'total_gb' => 1000]); $order = Order::factory()->create(['status' => 'paid', 'plan' => 'start', 'datacenter' => 'fsn']); diff --git a/tests/Feature/Provisioning/CustomerStepsTest.php b/tests/Feature/Provisioning/CustomerStepsTest.php index 3dfacd3..66d73ad 100644 --- a/tests/Feature/Provisioning/CustomerStepsTest.php +++ b/tests/Feature/Provisioning/CustomerStepsTest.php @@ -186,6 +186,12 @@ it('configures the guest network, captures the ip, and firewall', function () { ->and($s['pve']->firewallCalls)->toContain('101'); }); +it('polls network config until the guest has an address', function () { + fakeServices(); // hostname -I returns empty by default → no address yet + ['run' => $run] = reservedRun(); + expect(app(ConfigureNetwork::class)->execute($run)->type)->toBe('poll'); +}); + // 8. DeployApplicationStack it('deploys the stack, polling until healthy, without persisting the db password', function () { $s = fakeServices();