From 473bb3655dd45418faf63652808e7b0ede8ad0d0 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Wed, 4 Nov 2026 11:29:37 +0000 Subject: [PATCH 1/4] fix(checkout): refund when completion throws anything after capture completeOrRefund() only caught PaymentConfirmationException. Any other throw from complete() (an order build failure, a placement listener, a database fault) left captured money with no order. The exception also escaped the reconcile sweep, aborting it for every later session. It never consumed an attempt, so the session never stalled. A captured hold was re-captured on the next pass and reopened with the money kept. Every failure now reports and takes the refund (or hold void) path. A session a concurrent completion already moved to Completed is re-read first and never refunded. Spec 0010 section E.2 amended first. --- spec/0010-cart-session-reconciliation.md | 13 +- src/Actions/ReconcileCheckoutSession.php | 16 ++ tests/Feature/CompletionFailureRefundTest.php | 158 ++++++++++++++++++ tests/Feature/PaymentHoldTest.php | 18 ++ 4 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 tests/Feature/CompletionFailureRefundTest.php diff --git a/spec/0010-cart-session-reconciliation.md b/spec/0010-cart-session-reconciliation.md index 21c47f8..057d913 100644 --- a/spec/0010-cart-session-reconciliation.md +++ b/spec/0010-cart-session-reconciliation.md @@ -329,7 +329,14 @@ for before an order is created: **idempotent keyed on the session `uuid`** (create-or-fetch, §B). 3. **Mismatch or failure → the refund invariant.** If the live cart changed during the gateway window, or order creation fails (e.g. stock gone under the - `ValidateCartForOrderCreation` re-check): + `ValidateCartForOrderCreation` re-check), the money goes back as below. Any + exception `complete()` throws + counts as a failure, not only a confirmation mismatch: an order build, a + placement listener or a database fault that rolls the completion back leaves + money with no order, so it is reported and resolves here. The one exception + is a session another process already moved out of `PaymentProcessing` (a + concurrent completion that won the guarded write): it is re-read first, and + a completed session is never refunded. - **Payment not captured** — abort/void the intent via the gateway driver; only once the void is **gateway-confirmed** does the session transition `→ Open` (re-synced, `CheckoutCompletionFailed`); the customer re-confirms @@ -580,7 +587,9 @@ states (`Expired`, etc.); the table does not duplicate those. order-creation transaction; a duplicate callback or double-submitted synchronous completion yields exactly one order; a post-capture mismatch or order-creation failure refunds the charge and returns the session to `Open` - (`CheckoutCompletionFailed`) — no charge survives without an order. + (`CheckoutCompletionFailed`) — no charge survives without an order. Any + exception from `complete()` takes the same refund (or hold void) path, except + when a re-read shows a concurrent process already completed the session. - Invalidating a `PaymentProcessing` session voids the intent first; an unabortable intent leaves the session in `PaymentProcessing` for §E.2 — never a terminal state with an unresolved charge. diff --git a/src/Actions/ReconcileCheckoutSession.php b/src/Actions/ReconcileCheckoutSession.php index 0cdd9d9..58f7971 100644 --- a/src/Actions/ReconcileCheckoutSession.php +++ b/src/Actions/ReconcileCheckoutSession.php @@ -11,6 +11,7 @@ use Lunar\Checkout\Exceptions\PaymentConfirmationException; use Lunar\Checkout\Models\CheckoutSession; use Lunar\Checkout\States\CheckoutSession\Cancelled; +use Lunar\Checkout\States\CheckoutSession\Completed; use Lunar\Checkout\States\CheckoutSession\Open; use Lunar\Checkout\States\CheckoutSession\PaymentProcessing; use Lunar\Checkout\Support\PaymentIntentGateway; @@ -161,6 +162,21 @@ private function completeOrRefund( return 'completed'; } catch (PaymentConfirmationException) { // The cart no longer matches what the customer paid for. + } catch (\Throwable $e) { + // Any other failure rolled the completion back and left money + // with no order (spec 0010 §E.2), unless a concurrent completion + // won the guarded write, whose order the money now belongs to. + $session->refresh(); + + if ($session->status instanceof Completed) { + return 'completed'; + } + + if (! $session->status instanceof PaymentProcessing) { + return 'not-applicable'; + } + + report($e); } // Money back: captured funds refund; an uncaptured hold voids. A diff --git a/tests/Feature/CompletionFailureRefundTest.php b/tests/Feature/CompletionFailureRefundTest.php new file mode 100644 index 0000000..e9136bb --- /dev/null +++ b/tests/Feature/CompletionFailureRefundTest.php @@ -0,0 +1,158 @@ + */ + public static array $refunds = []; + + public function createIntent(Cart $cart): PaymentIntentDescriptor + { + return new PaymentIntentDescriptor('pi_captured_'.$cart->id); + } + + public function fetchIntent(string $reference): PaymentIntentStatus + { + return PaymentIntentStatus::Captured; + } + + public function voidIntent(string $reference): void {} + + public function refundIntent(string $reference, int $amountMinor, string $idempotencyKey): PaymentRefund + { + static::$refunds[] = ['reference' => $reference, 'amount' => $amountMinor]; + + return new PaymentRefund(success: true, reference: 'refund_'.$reference); + } +} + +class CapturedIntentMethod extends AbstractPaymentMethod +{ + public function handle(): string + { + return 'captured-card'; + } + + public function label(): string + { + return 'Card'; + } + + public function driver(): string + { + return 'captured-intent'; + } + + public function requiresIntent(): bool + { + return true; + } + + public function component(): string + { + return 'stripe-card'; + } +} + +beforeEach(function () { + CapturedIntentGateway::$refunds = []; + + Payments::extend('captured-intent', fn () => app(CapturedIntentGateway::class)); + app(PaymentMethodRegistry::class)->add(CapturedIntentMethod::class); +}); + +function capturedSession(): CheckoutSession +{ + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = CheckoutCart::session($cart); + + $session->forceFill([ + 'status' => PaymentProcessing::$name, + 'payment_intent_ref' => 'pi_captured_pinned', + 'payment_processing_at' => now(), + 'cart_fingerprint' => CheckoutCart::fingerprint($session), + 'meta' => ['payment_method' => 'captured-intent', 'payment_handle' => 'captured-card'], + ])->save(); + + return $session->refresh(); +} + +it('refunds and reopens when the order cannot be built after the money was captured', function () { + Exceptions::fake(); + $session = capturedSession(); + + config()->set('lunar.orders.pipelines.creation', [ + fn () => throw new RuntimeException('order pipeline down'), + ]); + + $outcome = app(ReconcilesCheckoutSession::class)->execute($session); + + expect($outcome)->toBe('refunded') + ->and(CapturedIntentGateway::$refunds)->toBe([['reference' => 'pi_captured_pinned', 'amount' => $session->amount_total]]) + ->and($session->refresh()->status)->toBeInstanceOf(Open::class) + ->and(Order::query()->whereNotNull('placed_at')->count())->toBe(0); + + Exceptions::assertReported(fn (RuntimeException $e) => $e->getMessage() === 'order pipeline down'); +}); + +it('keeps the sweep going past a session whose completion throws', function () { + Exceptions::fake(); + $failing = capturedSession(); + $failing->forceFill(['payment_processing_at' => now()->subHours(2)])->save(); + + config()->set('lunar.orders.pipelines.creation', [ + fn () => throw new RuntimeException('order pipeline down'), + ]); + + $this->artisan('lunar:checkout:reconcile')->assertSuccessful(); + + expect($failing->refresh()->status)->toBeInstanceOf(Open::class) + ->and(CapturedIntentGateway::$refunds)->toHaveCount(1); +}); + +it('never refunds a session a concurrent completion already placed', function () { + $session = capturedSession(); + + $real = app(CheckoutDriver::class); + $proxy = Mockery::mock($real)->makePartial(); + $proxy->shouldReceive('complete')->andReturnUsing(function (CheckoutSession $s) use ($real) { + // The other process (the webhook) wins the guarded write first. + $real->complete($s->replicate()->forceFill(['id' => $s->id])->setRelations([])); + + throw new CheckoutSessionConflictException('frozen'); + }); + app()->instance(CheckoutDriver::class, $proxy); + + $outcome = app(ReconcilesCheckoutSession::class)->execute($session); + + expect($outcome)->toBe('completed') + ->and(CapturedIntentGateway::$refunds)->toBeEmpty() + ->and($session->refresh()->status)->toBeInstanceOf(Completed::class) + ->and(Order::query()->whereNotNull('placed_at')->count())->toBe(1); +}); diff --git a/tests/Feature/PaymentHoldTest.php b/tests/Feature/PaymentHoldTest.php index bc60b5c..22eaa8e 100644 --- a/tests/Feature/PaymentHoldTest.php +++ b/tests/Feature/PaymentHoldTest.php @@ -2,6 +2,7 @@ use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Artisan; +use Illuminate\Support\Facades\Exceptions; use Lunar\Checkout\Contracts\PaymentMethodRegistry; use Lunar\Checkout\Models\CheckoutSession; use Lunar\Checkout\PaymentMethods\AbstractPaymentMethod; @@ -549,6 +550,23 @@ function mintPinnedHoldSessionWithoutHoldSupport(): CheckoutSession expect($session->refresh()->status)->toBeInstanceOf(Open::class); }); +it('refunds the captured hold when completion throws for any reason', function () { + Exceptions::fake(); + $session = mintPinnedHoldSession(); + FakeHoldGateway::$fetchStatus = PaymentIntentStatus::RequiresCapture; + + config()->set('lunar.orders.pipelines.creation', [ + fn () => throw new RuntimeException('order pipeline down'), + ]); + + $this->get(route('lunar.checkout.processing', $session->uuid)); + + expect(FakeHoldGateway::$captureCalls)->toHaveCount(1) + ->and(FakeHoldGateway::$refundCalls)->toHaveCount(1) + ->and(FakeHoldGateway::$voidedReferences)->toBeEmpty() + ->and($session->refresh()->status)->toBeInstanceOf(Open::class); +}); + it('release reopens a pinned hold session keeping the hold intact', function () { $session = mintPinnedHoldSession(); FakeHoldGateway::$fetchStatus = PaymentIntentStatus::RequiresCapture; From 8663abd2497914d459b77849ad2f1c7c8fb70da3 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Wed, 4 Nov 2026 11:57:15 +0000 Subject: [PATCH 2/4] fix(checkout): return money under the session lock; retry transient faults Review of the first cut: a refund decided on an unlocked re-read could race a webhook completion still mid-transaction and refund a placed order. Money now goes back under the session row lock, held through the reopen, and the async completion's guarded write accepts only PaymentProcessing (sync only Open), so a reopened session is never completed. Deadlocks and lost connections retry via recordAttempt unless this pass captured a hold. An uncaptured policy-mode authorisation is voided, not refunded. The exception is reported before any early return. --- spec/0010-cart-session-reconciliation.md | 27 +++-- src/Actions/ReconcileCheckoutSession.php | 114 ++++++++++++------ src/Drivers/LunarCheckoutDriver.php | 5 +- tests/Feature/CompletionFailureRefundTest.php | 33 +++++ tests/Feature/PaymentHoldTest.php | 17 +++ 5 files changed, 146 insertions(+), 50 deletions(-) diff --git a/spec/0010-cart-session-reconciliation.md b/spec/0010-cart-session-reconciliation.md index 057d913..d4c9de5 100644 --- a/spec/0010-cart-session-reconciliation.md +++ b/spec/0010-cart-session-reconciliation.md @@ -330,13 +330,20 @@ for before an order is created: 3. **Mismatch or failure → the refund invariant.** If the live cart changed during the gateway window, or order creation fails (e.g. stock gone under the `ValidateCartForOrderCreation` re-check), the money goes back as below. Any - exception `complete()` throws - counts as a failure, not only a confirmation mismatch: an order build, a - placement listener or a database fault that rolls the completion back leaves - money with no order, so it is reported and resolves here. The one exception - is a session another process already moved out of `PaymentProcessing` (a - concurrent completion that won the guarded write): it is re-read first, and - a completed session is never refunded. + exception `complete()` throws counts as a failure, not only a confirmation + mismatch: an order build, a placement listener or a database fault that + rolls the completion back leaves money with no order, so it is reported and + resolves here. A deadlock or a lost database connection is the exception: + it records a reconciliation attempt and retries on a later pass, unless this + pass captured a hold, which a retry cannot capture again. Money goes back + under the **session row lock**, held until the session is back in `Open`: + a concurrent completion either committed first (the session reads + `Completed` and nothing is returned) or waits on its guarded write and then + finds the session no longer `PaymentProcessing`. The async completion's + guarded write accepts only `PaymentProcessing`, and the synchronous one only + `Open`, so a session reopened mid-completion is never completed. An + authorisation that was never captured (a hold, or a policy-mode + `requires_capture` intent) is voided rather than refunded. - **Payment not captured** — abort/void the intent via the gateway driver; only once the void is **gateway-confirmed** does the session transition `→ Open` (re-synced, `CheckoutCompletionFailed`); the customer re-confirms @@ -588,8 +595,10 @@ states (`Expired`, etc.); the table does not duplicate those. completion yields exactly one order; a post-capture mismatch or order-creation failure refunds the charge and returns the session to `Open` (`CheckoutCompletionFailed`) — no charge survives without an order. Any - exception from `complete()` takes the same refund (or hold void) path, except - when a re-read shows a concurrent process already completed the session. + exception from `complete()` takes the same refund (or void, when nothing was + captured) path under the session row lock, except a deadlock or lost + connection with no hold captured on that pass, which retries; a session a + concurrent completion placed is never refunded. - Invalidating a `PaymentProcessing` session voids the intent first; an unabortable intent leaves the session in `PaymentProcessing` for §E.2 — never a terminal state with an unresolved charge. diff --git a/src/Actions/ReconcileCheckoutSession.php b/src/Actions/ReconcileCheckoutSession.php index 58f7971..644edbb 100644 --- a/src/Actions/ReconcileCheckoutSession.php +++ b/src/Actions/ReconcileCheckoutSession.php @@ -3,6 +3,9 @@ namespace Lunar\Checkout\Actions; use Illuminate\Contracts\Events\Dispatcher; +use Illuminate\Database\ConnectionInterface; +use Illuminate\Database\DetectsConcurrencyErrors; +use Illuminate\Database\DetectsLostConnections; use Lunar\Checkout\Contracts\Actions\ReconcilesCheckoutSession; use Lunar\Checkout\Contracts\CheckoutDriver; use Lunar\Checkout\Events\CheckoutCompletionFailed; @@ -30,10 +33,14 @@ */ final class ReconcileCheckoutSession implements ReconcilesCheckoutSession { + use DetectsConcurrencyErrors; + use DetectsLostConnections; + public function __construct( private CheckoutDriver $driver, private PaymentIntentGateway $gateways, private Dispatcher $events, + private ConnectionInterface $db, ) {} public function execute(CheckoutSession $session, ?string $resolve = null): string @@ -65,7 +72,7 @@ public function execute(CheckoutSession $session, ?string $resolve = null): stri PaymentIntentStatus::Pending => 'pending', PaymentIntentStatus::Captured, - PaymentIntentStatus::RequiresCapture => $this->completeOrRefund($session, $gateway, $reference), + PaymentIntentStatus::RequiresCapture => $this->completeOrRefund($session, $gateway, $reference, $status), PaymentIntentStatus::Voided, PaymentIntentStatus::Failed => $this->reopen($session, 'intent_'.$status->value), @@ -100,7 +107,7 @@ public function release(CheckoutSession $session): string // moved: the customer may keep editing and re-confirm, so // reopen with the hold intact instead of forcing completion. if (! ($session->isHoldMode() && $status === PaymentIntentStatus::RequiresCapture)) { - return $this->completeOrRefund($session, $gateway, $reference); + return $this->completeOrRefund($session, $gateway, $reference, $status); } } } @@ -121,14 +128,16 @@ public function release(CheckoutSession $session): string /** * Captured money resolves to exactly one of: an order, or money back. - * Money back is flavour-aware: an authorise-only hold that was never - * captured voids; anything actually captured (a standard intent, or a - * hold captured by this very call) refunds. + * Money back is flavour-aware: an authorisation that was never captured + * (a hold, or a policy-mode requires_capture intent) voids; anything + * actually captured (a standard intent, or a hold captured by this very + * call) refunds. */ private function completeOrRefund( CheckoutSession $session, SupportsPaymentIntents $gateway, string $reference, + PaymentIntentStatus $status, ): string { // An authorise-only hold is not money moved yet: capture it now, in // the one idempotent place every completion path funnels through, so @@ -164,56 +173,81 @@ private function completeOrRefund( // The cart no longer matches what the customer paid for. } catch (\Throwable $e) { // Any other failure rolled the completion back and left money - // with no order (spec 0010 §E.2), unless a concurrent completion - // won the guarded write, whose order the money now belongs to. - $session->refresh(); + // with no order (spec 0010 §E.2). A deadlock or a dropped + // connection is retried on a later pass instead, unless this + // call captured a hold: a retry cannot capture it again. + report($e); + + if (! $holdCaptured && ($this->causedByConcurrencyError($e) || $this->causedByLostConnection($e))) { + return $this->recordAttempt($session); + } + } + + return $this->moneyBack($session, $gateway, $reference, $holdCaptured || $status === PaymentIntentStatus::Captured); + } + + /** + * Under the session row lock, held until the session is reopened: a + * concurrent completion (the webhook) either committed first, and is + * seen here, or waits on its guarded write and then finds the session + * no longer PaymentProcessing. Money is never returned for an order + * that gets placed. + */ + private function moneyBack( + CheckoutSession $session, + SupportsPaymentIntents $gateway, + string $reference, + bool $captured, + ): string { + return $this->db->transaction(function () use ($session, $gateway, $reference, $captured): string { + $locked = CheckoutSession::query()->whereKey($session->getKey())->lockForUpdate()->firstOrFail(); - if ($session->status instanceof Completed) { + if ($locked->status instanceof Completed) { return 'completed'; } - if (! $session->status instanceof PaymentProcessing) { + if (! $locked->status instanceof PaymentProcessing) { return 'not-applicable'; } - report($e); - } + // Nothing was captured, so there is nothing to refund: release + // the authorisation. A hold-mode session with no capture here + // means $gateway lacked SupportsPaymentHolds (a + // misconfiguration); voiding is still the safe answer. + if (! $captured) { + try { + $gateway->voidIntent($reference); + } catch (PaymentIntentException) { + $this->events->dispatch(new CheckoutCompletionFailed($session, 'void-failed')); + + return $this->recordAttempt($session); + } + + $holdMode = $session->isHoldMode(); + + $this->reopen($session, $holdMode ? 'hold_voided' : 'voided'); + $this->events->dispatch(new CheckoutCompletionFailed($session, $holdMode ? 'hold-voided' : 'voided')); + + return 'voided'; + } - // Money back: captured funds refund; an uncaptured hold voids. A - // hold-mode session that reaches here with $holdCaptured still false - // means $gateway lacked SupportsPaymentHolds (a misconfiguration); - // voiding is still the safe answer. - if ($session->isHoldMode() && ! $holdCaptured) { try { - $gateway->voidIntent($reference); + $refund = $gateway->refundIntent( + $reference, + $session->amount_total, + 'lunar-checkout-refund:'.$reference, + ); } catch (PaymentIntentException) { - $this->events->dispatch(new CheckoutCompletionFailed($session, 'void-failed')); + $this->events->dispatch(new CheckoutCompletionFailed($session, 'refund-failed')); return $this->recordAttempt($session); } - $this->reopen($session, 'hold_voided'); - $this->events->dispatch(new CheckoutCompletionFailed($session, 'hold-voided')); + $this->reopen($session, 'refunded'); + $this->events->dispatch(new CheckoutCompletionFailed($session, 'refunded', $refund->reference)); - return 'voided'; - } - - try { - $refund = $gateway->refundIntent( - $reference, - $session->amount_total, - 'lunar-checkout-refund:'.$reference, - ); - } catch (PaymentIntentException) { - $this->events->dispatch(new CheckoutCompletionFailed($session, 'refund-failed')); - - return $this->recordAttempt($session); - } - - $this->reopen($session, 'refunded'); - $this->events->dispatch(new CheckoutCompletionFailed($session, 'refunded', $refund->reference)); - - return 'refunded'; + return 'refunded'; + }); } private function resolveByOperator( diff --git a/src/Drivers/LunarCheckoutDriver.php b/src/Drivers/LunarCheckoutDriver.php index 5ab131d..b382bc8 100644 --- a/src/Drivers/LunarCheckoutDriver.php +++ b/src/Drivers/LunarCheckoutDriver.php @@ -220,8 +220,11 @@ public function complete(CheckoutSession $session, ?string $confirmationToken = * the first. All of it sits in one transaction under the cart lock, * so nothing observes the session completed without its order. */ + // Each path completes only from the status it verified: a session + // that reconcile reopened (refunded) while this ran must not be + // completed by the async path that read it PaymentProcessing. $transitioned = $session->transitionGuarded( - [Open::$name, PaymentProcessing::$name], + [$isSync ? Open::$name : PaymentProcessing::$name], Completed::$name, $attributes, ); diff --git a/tests/Feature/CompletionFailureRefundTest.php b/tests/Feature/CompletionFailureRefundTest.php index e9136bb..740c7ca 100644 --- a/tests/Feature/CompletionFailureRefundTest.php +++ b/tests/Feature/CompletionFailureRefundTest.php @@ -1,5 +1,6 @@ and($session->refresh()->status)->toBeInstanceOf(Completed::class) ->and(Order::query()->whereNotNull('placed_at')->count())->toBe(1); }); + +it('retries a deadlocked completion on a later pass instead of refunding', function () { + Exceptions::fake(); + $session = capturedSession(); + + config()->set('lunar.orders.pipelines.creation', [ + fn () => throw new RuntimeException('SQLSTATE[40001]: Deadlock found when trying to get lock; try restarting transaction'), + ]); + + $outcome = app(ReconcilesCheckoutSession::class)->execute($session); + + expect($outcome)->toBe('retry') + ->and(CapturedIntentGateway::$refunds)->toBeEmpty() + ->and($session->refresh()->status)->toBeInstanceOf(PaymentProcessing::class) + ->and($session->reconciliation_attempts)->toBe(1); + + Exceptions::assertReported(DeadlockException::class); +}); + +it('never completes a session reconcile reopened while the async completion ran', function () { + $session = capturedSession(); + $stale = CheckoutSession::query()->findOrFail($session->id); + + // Reconcile refunded and reopened it after this copy was read. + $session->forceFill(['status' => Open::$name, 'payment_intent_ref' => null])->save(); + + expect(fn () => app(CheckoutDriver::class)->complete($stale)) + ->toThrow(CheckoutSessionConflictException::class); + + expect($session->refresh()->status)->toBeInstanceOf(Open::class) + ->and(Order::query()->whereNotNull('placed_at')->count())->toBe(0); +}); diff --git a/tests/Feature/PaymentHoldTest.php b/tests/Feature/PaymentHoldTest.php index 22eaa8e..031ebc6 100644 --- a/tests/Feature/PaymentHoldTest.php +++ b/tests/Feature/PaymentHoldTest.php @@ -567,6 +567,23 @@ function mintPinnedHoldSessionWithoutHoldSupport(): CheckoutSession ->and($session->refresh()->status)->toBeInstanceOf(Open::class); }); +it('voids, never refunds, a policy-mode authorisation whose completion throws', function () { + Exceptions::fake(); + $session = mintPinnedStandardSession(); + FakeHoldGateway::$fetchStatus = PaymentIntentStatus::RequiresCapture; + + config()->set('lunar.orders.pipelines.creation', [ + fn () => throw new RuntimeException('order pipeline down'), + ]); + + $this->get(route('lunar.checkout.processing', $session->uuid)); + + expect(FakeHoldGateway::$voidedReferences)->toBe(['pi_fake_pinned']) + ->and(FakeHoldGateway::$refundCalls)->toBeEmpty() + ->and(FakeHoldGateway::$captureCalls)->toBeEmpty() + ->and($session->refresh()->status)->toBeInstanceOf(Open::class); +}); + it('release reopens a pinned hold session keeping the hold intact', function () { $session = mintPinnedHoldSession(); FakeHoldGateway::$fetchStatus = PaymentIntentStatus::RequiresCapture; From c6abb18e3c024233dd1939ffb05aa980dbd2bcd7 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Wed, 4 Nov 2026 12:06:11 +0000 Subject: [PATCH 3/4] fix(checkout): fire completion-failed after the reopen commits; never refund a placed order Round two of review: a listener throwing inside the money-back transaction rolled back the reopen after the refund, so the next pass could place the order against refunded money. Events now dispatch after commit. Under the lock, money also stays put when the gateway already placed the cart's order or the session was re-pinned against a new intent; both record an attempt for the operator. The lock runs on the session model's own connection. --- spec/0010-cart-session-reconciliation.md | 11 +++- src/Actions/ReconcileCheckoutSession.php | 51 ++++++++++------- .../Actions/ReconcilesCheckoutSession.php | 2 +- src/Events/CheckoutCompletionFailed.php | 3 +- src/Http/Controllers/CheckoutController.php | 2 +- tests/Feature/CompletionFailureRefundTest.php | 55 +++++++++++++++++++ 6 files changed, 98 insertions(+), 26 deletions(-) diff --git a/spec/0010-cart-session-reconciliation.md b/spec/0010-cart-session-reconciliation.md index d4c9de5..d71c431 100644 --- a/spec/0010-cart-session-reconciliation.md +++ b/spec/0010-cart-session-reconciliation.md @@ -341,9 +341,14 @@ for before an order is created: `Completed` and nothing is returned) or waits on its guarded write and then finds the session no longer `PaymentProcessing`. The async completion's guarded write accepts only `PaymentProcessing`, and the synchronous one only - `Open`, so a session reopened mid-completion is never completed. An - authorisation that was never captured (a hold, or a policy-mode - `requires_capture` intent) is voided rather than refunded. + `Open`, so a session reopened mid-completion is never completed. Under the + same lock, money stays put when the session was re-pinned against another + intent meanwhile, or when the gateway's own `authorize()` already placed an + order for the cart (the failure came after placement): that records an + attempt and stalls to the operator instead. `CheckoutCompletionFailed` + fires only after the reopen commits. An authorisation that was never + captured (a hold, or a policy-mode `requires_capture` intent) is voided + rather than refunded. - **Payment not captured** — abort/void the intent via the gateway driver; only once the void is **gateway-confirmed** does the session transition `→ Open` (re-synced, `CheckoutCompletionFailed`); the customer re-confirms diff --git a/src/Actions/ReconcileCheckoutSession.php b/src/Actions/ReconcileCheckoutSession.php index 644edbb..33df9ba 100644 --- a/src/Actions/ReconcileCheckoutSession.php +++ b/src/Actions/ReconcileCheckoutSession.php @@ -3,7 +3,6 @@ namespace Lunar\Checkout\Actions; use Illuminate\Contracts\Events\Dispatcher; -use Illuminate\Database\ConnectionInterface; use Illuminate\Database\DetectsConcurrencyErrors; use Illuminate\Database\DetectsLostConnections; use Lunar\Checkout\Contracts\Actions\ReconcilesCheckoutSession; @@ -22,6 +21,7 @@ use Lunar\Core\Contracts\SupportsPaymentIntents; use Lunar\Core\Enums\PaymentIntentStatus; use Lunar\Core\Exceptions\PaymentIntentException; +use Lunar\Core\Models\Order; /** * Bounded resolution of a `PaymentProcessing` session (spec 0010 §F): the @@ -40,7 +40,6 @@ public function __construct( private CheckoutDriver $driver, private PaymentIntentGateway $gateways, private Dispatcher $events, - private ConnectionInterface $db, ) {} public function execute(CheckoutSession $session, ?string $resolve = null): string @@ -190,8 +189,9 @@ private function completeOrRefund( * Under the session row lock, held until the session is reopened: a * concurrent completion (the webhook) either committed first, and is * seen here, or waits on its guarded write and then finds the session - * no longer PaymentProcessing. Money is never returned for an order - * that gets placed. + * no longer PaymentProcessing. Events fire only after the reopen commits, + * so a throwing listener cannot roll back a reopen whose money has + * already gone back. */ private function moneyBack( CheckoutSession $session, @@ -199,15 +199,24 @@ private function moneyBack( string $reference, bool $captured, ): string { - return $this->db->transaction(function () use ($session, $gateway, $reference, $captured): string { + [$outcome, $event] = $session->getConnection()->transaction(function () use ($session, $gateway, $reference, $captured): array { $locked = CheckoutSession::query()->whereKey($session->getKey())->lockForUpdate()->firstOrFail(); if ($locked->status instanceof Completed) { - return 'completed'; + return ['completed', null]; } - if (! $locked->status instanceof PaymentProcessing) { - return 'not-applicable'; + // Another process reopened it, or reopened and re-pinned it + // against a new intent: this pass's money decision is stale. + if (! $locked->status instanceof PaymentProcessing || $locked->payment_intent_ref !== $reference) { + return ['not-applicable', null]; + } + + // The gateway's own authorize() placed an order for this cart + // before complete() failed: returning the money would leave a + // placed order unpaid. The operator resolves it. + if (Order::query()->where('cart_id', (int) $session->cart_reference)->whereNotNull('placed_at')->exists()) { + return ['order-placed', null]; } // Nothing was captured, so there is nothing to refund: release @@ -218,17 +227,13 @@ private function moneyBack( try { $gateway->voidIntent($reference); } catch (PaymentIntentException) { - $this->events->dispatch(new CheckoutCompletionFailed($session, 'void-failed')); - - return $this->recordAttempt($session); + return ['void-failed', new CheckoutCompletionFailed($session, 'void-failed')]; } $holdMode = $session->isHoldMode(); + $this->reopen($session, 'voided'); - $this->reopen($session, $holdMode ? 'hold_voided' : 'voided'); - $this->events->dispatch(new CheckoutCompletionFailed($session, $holdMode ? 'hold-voided' : 'voided')); - - return 'voided'; + return ['voided', new CheckoutCompletionFailed($session, $holdMode ? 'hold-voided' : 'voided')]; } try { @@ -238,16 +243,22 @@ private function moneyBack( 'lunar-checkout-refund:'.$reference, ); } catch (PaymentIntentException) { - $this->events->dispatch(new CheckoutCompletionFailed($session, 'refund-failed')); - - return $this->recordAttempt($session); + return ['refund-failed', new CheckoutCompletionFailed($session, 'refund-failed')]; } $this->reopen($session, 'refunded'); - $this->events->dispatch(new CheckoutCompletionFailed($session, 'refunded', $refund->reference)); - return 'refunded'; + return ['refunded', new CheckoutCompletionFailed($session, 'refunded', $refund->reference)]; }); + + if ($event !== null) { + $this->events->dispatch($event); + } + + return match ($outcome) { + 'void-failed', 'refund-failed', 'order-placed' => $this->recordAttempt($session), + default => $outcome, + }; } private function resolveByOperator( diff --git a/src/Contracts/Actions/ReconcilesCheckoutSession.php b/src/Contracts/Actions/ReconcilesCheckoutSession.php index 8f6f9a8..917b4c6 100644 --- a/src/Contracts/Actions/ReconcilesCheckoutSession.php +++ b/src/Contracts/Actions/ReconcilesCheckoutSession.php @@ -26,7 +26,7 @@ public function execute(CheckoutSession $session, ?string $resolve = null): stri * unconfirmable outcome stays frozen for the reconciliation sweep. * * Returns a stable outcome code: `released`, `completed`, `refunded`, - * `unconfirmed` or `not-applicable`. + * `voided`, `unconfirmed` or `not-applicable`. */ public function release(CheckoutSession $session): string; } diff --git a/src/Events/CheckoutCompletionFailed.php b/src/Events/CheckoutCompletionFailed.php index 1c1f92f..9d4ad0d 100644 --- a/src/Events/CheckoutCompletionFailed.php +++ b/src/Events/CheckoutCompletionFailed.php @@ -8,7 +8,8 @@ * The §E.2 completion re-verify or order creation failed, or a misdirected * gateway success event was refunded (spec 0010 §E.2). `$reason` is a stable * machine code distinguishing `refunded` / `refund-pending` / `refund-failed` - * (plus `verify-mismatch` when nothing was captured yet). + * (plus `verify-mismatch` when nothing was captured yet, and `hold-voided` / + * `voided` / `void-failed` when an uncaptured authorisation was released). */ class CheckoutCompletionFailed extends CheckoutSessionEvent { diff --git a/src/Http/Controllers/CheckoutController.php b/src/Http/Controllers/CheckoutController.php index 5971f47..42cc549 100644 --- a/src/Http/Controllers/CheckoutController.php +++ b/src/Http/Controllers/CheckoutController.php @@ -1275,7 +1275,7 @@ public function releasePayment(CheckoutSessionModel $session, ReconcilesCheckout return response()->json([ 'outcome' => $outcome, - 'released' => in_array($outcome, ['released', 'refunded'], true), + 'released' => in_array($outcome, ['released', 'refunded', 'voided'], true), // The retry pins against the live fingerprint, so hand it over. 'fingerprint' => $checkoutDriver->fingerprint($session), ]); diff --git a/tests/Feature/CompletionFailureRefundTest.php b/tests/Feature/CompletionFailureRefundTest.php index 740c7ca..baf9cec 100644 --- a/tests/Feature/CompletionFailureRefundTest.php +++ b/tests/Feature/CompletionFailureRefundTest.php @@ -2,10 +2,13 @@ use Illuminate\Database\DeadlockException; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Exceptions; use Lunar\Checkout\Contracts\Actions\ReconcilesCheckoutSession; use Lunar\Checkout\Contracts\CheckoutDriver; use Lunar\Checkout\Contracts\PaymentMethodRegistry; +use Lunar\Checkout\Events\CheckoutCompletionFailed; +use Lunar\Checkout\Events\OrderPlacing; use Lunar\Checkout\Exceptions\CheckoutSessionConflictException; use Lunar\Checkout\Models\CheckoutSession; use Lunar\Checkout\PaymentMethods\AbstractPaymentMethod; @@ -189,3 +192,55 @@ function capturedSession(): CheckoutSession expect($session->refresh()->status)->toBeInstanceOf(Open::class) ->and(Order::query()->whereNotNull('placed_at')->count())->toBe(0); }); + +it('keeps the session reopened when a listener throws after the refund', function () { + Exceptions::fake(); + $session = capturedSession(); + + config()->set('lunar.orders.pipelines.creation', [ + fn () => throw new RuntimeException('order pipeline down'), + ]); + Event::listen(CheckoutCompletionFailed::class, fn () => throw new LogicException('listener down')); + + expect(fn () => app(ReconcilesCheckoutSession::class)->execute($session)) + ->toThrow(LogicException::class, 'listener down'); + + expect(CapturedIntentGateway::$refunds)->toHaveCount(1) + ->and($session->refresh()->status)->toBeInstanceOf(Open::class) + ->and($session->payment_intent_ref)->toBeNull(); +}); + +it('never refunds money whose order the gateway already placed', function () { + Exceptions::fake(); + $session = capturedSession(); + Cart::query()->findOrFail((int) $session->cart_reference)->createOrder()->update(['placed_at' => now()]); + + Event::listen(OrderPlacing::class, fn () => throw new LogicException('placement vetoed')); + + $outcome = app(ReconcilesCheckoutSession::class)->execute($session); + + expect($outcome)->toBe('retry') + ->and(CapturedIntentGateway::$refunds)->toBeEmpty() + ->and($session->refresh()->status)->toBeInstanceOf(PaymentProcessing::class) + ->and($session->reconciliation_attempts)->toBe(1); +}); + +it('leaves a session alone that was re-pinned against a new intent meanwhile', function () { + Exceptions::fake(); + $session = capturedSession(); + + $real = app(CheckoutDriver::class); + $proxy = Mockery::mock($real)->makePartial(); + $proxy->shouldReceive('complete')->andReturnUsing(function (CheckoutSession $s) { + CheckoutSession::query()->whereKey($s->id)->update(['payment_intent_ref' => 'pi_newer']); + + throw new RuntimeException('order pipeline down'); + }); + app()->instance(CheckoutDriver::class, $proxy); + + $outcome = app(ReconcilesCheckoutSession::class)->execute($session); + + expect($outcome)->toBe('not-applicable') + ->and(CapturedIntentGateway::$refunds)->toBeEmpty() + ->and($session->refresh()->payment_intent_ref)->toBe('pi_newer'); +}); From 39653161662d600665ec44c07a450b16ebb34ff8 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Wed, 4 Nov 2026 12:14:07 +0000 Subject: [PATCH 4/4] fix(checkout): scope the placed-order guard to this payment; count attempts first Round three of review: the placed-order guard matched an order an earlier checkout of the same cart placed, stalling a refund that should happen. It now only counts orders placed since the pin. The attempt is recorded before CompletionFailed listeners run and only while the session is still PaymentProcessing. Deadlock retry now covers captured holds too, since captureHold() is idempotent per reference. --- spec/0010-cart-session-reconciliation.md | 21 ++++++--- src/Actions/ReconcileCheckoutSession.php | 44 ++++++++++++++----- tests/Feature/CompletionFailureRefundTest.php | 35 +++++++++++++++ 3 files changed, 82 insertions(+), 18 deletions(-) diff --git a/spec/0010-cart-session-reconciliation.md b/spec/0010-cart-session-reconciliation.md index d71c431..44e8461 100644 --- a/spec/0010-cart-session-reconciliation.md +++ b/spec/0010-cart-session-reconciliation.md @@ -334,8 +334,9 @@ for before an order is created: mismatch: an order build, a placement listener or a database fault that rolls the completion back leaves money with no order, so it is reported and resolves here. A deadlock or a lost database connection is the exception: - it records a reconciliation attempt and retries on a later pass, unless this - pass captured a hold, which a retry cannot capture again. Money goes back + it records a reconciliation attempt and retries on a later pass (a captured + hold survives the retry, because `captureHold()` is idempotent per + reference). Money goes back under the **session row lock**, held until the session is back in `Open`: a concurrent completion either committed first (the session reads `Completed` and nothing is returned) or waits on its guarded write and then @@ -343,9 +344,14 @@ for before an order is created: guarded write accepts only `PaymentProcessing`, and the synchronous one only `Open`, so a session reopened mid-completion is never completed. Under the same lock, money stays put when the session was re-pinned against another - intent meanwhile, or when the gateway's own `authorize()` already placed an - order for the cart (the failure came after placement): that records an - attempt and stalls to the operator instead. `CheckoutCompletionFailed` + intent meanwhile, or when the gateway's own `authorize()` placed an order + for the cart since the pin (the failure came after placement): that records + an attempt and stalls to the operator instead. An attempt is only counted + while the session is still `PaymentProcessing`, and before any listener + runs. Residual: a gateway that places the order in its own transaction + without touching the session row (Stripe's `authorize()` does) can still + commit that placement while the refund is in flight, so "never refunded" + holds against checkout's own completion, not against every gateway's. `CheckoutCompletionFailed` fires only after the reopen commits. An authorisation that was never captured (a hold, or a policy-mode `requires_capture` intent) is voided rather than refunded. @@ -602,8 +608,9 @@ states (`Expired`, etc.); the table does not duplicate those. (`CheckoutCompletionFailed`) — no charge survives without an order. Any exception from `complete()` takes the same refund (or void, when nothing was captured) path under the session row lock, except a deadlock or lost - connection with no hold captured on that pass, which retries; a session a - concurrent completion placed is never refunded. + connection, which retries; a session checkout's own concurrent completion + placed is never refunded, and an order the gateway placed since the pin + stalls to the operator instead of being refunded. - Invalidating a `PaymentProcessing` session voids the intent first; an unabortable intent leaves the session in `PaymentProcessing` for §E.2 — never a terminal state with an unresolved charge. diff --git a/src/Actions/ReconcileCheckoutSession.php b/src/Actions/ReconcileCheckoutSession.php index 33df9ba..7a13ffa 100644 --- a/src/Actions/ReconcileCheckoutSession.php +++ b/src/Actions/ReconcileCheckoutSession.php @@ -173,11 +173,11 @@ private function completeOrRefund( } catch (\Throwable $e) { // Any other failure rolled the completion back and left money // with no order (spec 0010 §E.2). A deadlock or a dropped - // connection is retried on a later pass instead, unless this - // call captured a hold: a retry cannot capture it again. + // connection is retried on a later pass instead; captureHold() + // is idempotent per reference, so a captured hold survives that. report($e); - if (! $holdCaptured && ($this->causedByConcurrencyError($e) || $this->causedByLostConnection($e))) { + if ($this->causedByConcurrencyError($e) || $this->causedByLostConnection($e)) { return $this->recordAttempt($session); } } @@ -213,9 +213,17 @@ private function moneyBack( } // The gateway's own authorize() placed an order for this cart - // before complete() failed: returning the money would leave a - // placed order unpaid. The operator resolves it. - if (Order::query()->where('cart_id', (int) $session->cart_reference)->whereNotNull('placed_at')->exists()) { + // since the pin, before complete() failed: returning the money + // would leave a placed order unpaid. The operator resolves it. + // An order an earlier checkout of the same cart placed does not + // count. + $placedForThisPayment = Order::query() + ->where('cart_id', (int) $session->cart_reference) + ->whereNotNull('placed_at') + ->when($locked->payment_processing_at, fn ($query, $pinnedAt) => $query->where('placed_at', '>=', $pinnedAt)) + ->exists(); + + if ($placedForThisPayment) { return ['order-placed', null]; } @@ -251,14 +259,18 @@ private function moneyBack( return ['refunded', new CheckoutCompletionFailed($session, 'refunded', $refund->reference)]; }); + // Count the attempt before any listener runs, so a throwing + // listener cannot stop the session reaching its stall. + $result = match ($outcome) { + 'void-failed', 'refund-failed', 'order-placed' => $this->recordAttempt($session), + default => $outcome, + }; + if ($event !== null) { $this->events->dispatch($event); } - return match ($outcome) { - 'void-failed', 'refund-failed', 'order-placed' => $this->recordAttempt($session), - default => $outcome, - }; + return $result; } private function resolveByOperator( @@ -362,9 +374,19 @@ private function recordAttempt(CheckoutSession $session): string { $maxAttempts = (int) config('lunar.checkout.reconciliation.max_attempts', 5); - $session->increment('reconciliation_attempts'); + // Only a session still awaiting resolution counts an attempt: one a + // concurrent completion or reopen moved on must not raise a stall. + $counted = CheckoutSession::query() + ->whereKey($session->getKey()) + ->where('status', PaymentProcessing::$name) + ->increment('reconciliation_attempts'); + $session->refresh(); + if ($counted === 0) { + return 'not-applicable'; + } + if ($session->reconciliation_attempts < $maxAttempts) { return 'retry'; } diff --git a/tests/Feature/CompletionFailureRefundTest.php b/tests/Feature/CompletionFailureRefundTest.php index baf9cec..e092317 100644 --- a/tests/Feature/CompletionFailureRefundTest.php +++ b/tests/Feature/CompletionFailureRefundTest.php @@ -20,6 +20,7 @@ use Lunar\Core\DataObjects\PaymentIntentDescriptor; use Lunar\Core\DataObjects\PaymentRefund; use Lunar\Core\Enums\PaymentIntentStatus; +use Lunar\Core\Exceptions\PaymentIntentException; use Lunar\Core\Facades\CartSession; use Lunar\Core\Facades\Payments; use Lunar\Core\Models\Cart; @@ -35,6 +36,8 @@ class CapturedIntentGateway extends OfflinePayment implements CreatesPaymentInte /** @var array */ public static array $refunds = []; + public static bool $refundFails = false; + public function createIntent(Cart $cart): PaymentIntentDescriptor { return new PaymentIntentDescriptor('pi_captured_'.$cart->id); @@ -49,6 +52,10 @@ public function voidIntent(string $reference): void {} public function refundIntent(string $reference, int $amountMinor, string $idempotencyKey): PaymentRefund { + if (static::$refundFails) { + throw new PaymentIntentException('gateway refund failed'); + } + static::$refunds[] = ['reference' => $reference, 'amount' => $amountMinor]; return new PaymentRefund(success: true, reference: 'refund_'.$reference); @@ -85,6 +92,7 @@ public function component(): string beforeEach(function () { CapturedIntentGateway::$refunds = []; + CapturedIntentGateway::$refundFails = false; Payments::extend('captured-intent', fn () => app(CapturedIntentGateway::class)); app(PaymentMethodRegistry::class)->add(CapturedIntentMethod::class); @@ -244,3 +252,30 @@ function capturedSession(): CheckoutSession ->and(CapturedIntentGateway::$refunds)->toBeEmpty() ->and($session->refresh()->payment_intent_ref)->toBe('pi_newer'); }); + +it('still refunds when the only placed order on the cart predates this payment', function () { + $session = capturedSession(); + Cart::query()->findOrFail((int) $session->cart_reference)->createOrder()->update(['placed_at' => now()->subDay()]); + $session->forceFill(['cart_fingerprint' => 'stale'])->save(); + + $outcome = app(ReconcilesCheckoutSession::class)->execute($session->refresh()); + + expect($outcome)->toBe('refunded') + ->and(CapturedIntentGateway::$refunds)->toHaveCount(1) + ->and($session->refresh()->status)->toBeInstanceOf(Open::class); +}); + +it('counts the attempt even when a listener throws on a failed refund', function () { + Exceptions::fake(); + CapturedIntentGateway::$refundFails = true; + $session = capturedSession(); + $session->forceFill(['cart_fingerprint' => 'stale'])->save(); + + Event::listen(CheckoutCompletionFailed::class, fn () => throw new LogicException('listener down')); + + expect(fn () => app(ReconcilesCheckoutSession::class)->execute($session->refresh())) + ->toThrow(LogicException::class, 'listener down'); + + expect($session->refresh()->status)->toBeInstanceOf(PaymentProcessing::class) + ->and($session->reconciliation_attempts)->toBe(1); +});