[alecritson]

Why

Glenn wants an order to exist before any money is taken. Lunar spec 0077 ("Placement", lunarphp/lunar#2745) now states that rule. Before this PR, checkout created the order only after payment, inside complete(). Now the Lunar driver drafts the cart's order when Pay pins the session, and completion places that same draft.

What changed

Specs first. spec/0010-cart-session-reconciliation.md section E now says the Lunar driver drafts the cart's order inside the pin. Section E.2 is retitled "drafted at the pin; re-verify, then place" and records that a refunded or voided attempt leaves an unplaced draft, which the next Pay reuses. The acceptance checks cover all of this. spec/0004-checkout-session.md no longer says the order is created only at completion. The CheckoutDriver contract is unchanged. The session still stores no order id until Completed.

LunarCheckoutDriver::assertReadyForPayment(). The token check, Gate 2, the guarded pin UPDATE and $cart->createOrder() now run in one $this->db->transaction, against one lockForUpdate() read of the cart row (the same read complete() takes). So the draft holds exactly what the pinned fingerprint covers.

• If drafting throws, the pin rolls back and Pay fails with nothing pinned.
• If the pin affects 0 rows (expired session, a concurrent Pay that lost, a supersede), the driver throws before drafting.
• Step 0 (the pinned currency and channel check) stays outside the transaction. On a mismatch it voids the gateway intent and cancels the session, and a rollback must not undo the cancel while the void stands.

complete(): the draft step changed, against the brief. The brief said re-running createOrder() in complete() would refresh the draft. It did not re-run it: $cart->completedOrder ?: $cart->draftOrder ?: $cart->createOrder() placed any existing draft exactly as built. That breaks a synchronous retry:

1. Pay by card drafts the order at quantity 1.
2. The cart changes, so completion refunds and reopens the session.
3. The customer retries with bank transfer (the synchronous path).
4. complete() places the stale quantity-1 draft.

The line is now $cart->completedOrder ?? ($isSync ? null : $cart->draftOrder) ?? $cart->createOrder().

• Async path: uses the draft the pin built, without rebuilding it. The fingerprint match proves the draft holds what was paid for, and skipping the rebuild means no order build runs after capture. (The first cut also rebuilt on the async path. Review showed that rebuild could throw after capture with no refund, so 7b8e7dc dropped it.)
• Sync path: there was no pin, so createOrder() reuses the draft and rebuilds it from the cart just verified.

A test covers each path. With the original line, the synchronous-retry test fails with Failed asserting that 1 is identical to 2.

Holds. completeOrRefund() calls captureHold() only after the pin, so the draft already exists by then. A test asserts it.

Changed assertions in existing suites

• PayBoundaryTest, "still pins for confirmation when the method needs an intent": this test asserted Order::count() === 0 after the pin, which is the old "no order before completion" rule. Renamed it to "pins for confirmation with the order drafted but unplaced". It now asserts one order with placed_at null and no order_reference on the session.
• PaymentHoldTest: FakeHoldGateway::captureHold() now also records the ids of unplaced orders when it is called, so the new hold test can prove the draft existed before capture. No existing assertion changed.

CheckoutSessionIntegrityTest, OrderPlacingEventTest, WebhookFirstPlacementTest and PaymentRecordingTest pass unchanged.

New tests

tests/Feature/DraftOrderAtPayGateTest.php:

• Pay on a gateway method drafts exactly one unplaced order before any confirmation.
• No draft is created on a stale token, an expired session, or a concurrent Pay that lost the pin.
• A draft that fails to build leaves the session Open and unpinned.
• Completion places the same draft (same id), with the Transaction row on it.
• Webhook-first completion places the same draft, with no second order.
• After a refund the draft stays unplaced, and the retry reuses the same id, rebuilt.
• A synchronous retry after a refund places the rebuilt draft, not the stale one.

PaymentHoldTest:

• The draft exists before captureHold(), and completion places it.
• After a void the draft stays unplaced, and the retry reuses the same id.

Against main, 7 of the 9 new driver tests fail. The 2 that pass assert that a rejected pin drafts nothing, which main satisfies trivially.

Known trade-offs (accepted)

A fresh self-review found two consequences of drafting at Pay. Both are accepted.

1. Coupon uses are spent at Pay. Core's CreateOrder calls markAsUsed() when it builds the draft, so an abandoned Pay can use up a limited code. Accepted. The Pay route is throttled (checkout-pay: 10 a minute and 30 an hour per IP, in 9270a49) so it can't be done cheaply on purpose. Counting the use at placement is filed in core as Count discount uses when an order is placed, not when its draft is created lunar#2818.
2. Stale drafts. Fixed for checkout in bb7eb1a. The pin stamps the draft with the checkout fingerprint, and complete() uses the draft as built only when that stamp matches the fingerprint it just verified. Otherwise it rebuilds the draft, which covers a payment confirmed with no Pay step. Remaining in core: Stripe's authorize() reads $cart->draftOrder directly, so a payment confirmed with no Pay after a cart change still meets the stale draft there, and Stripe's total check rejects it. The shipped UI always calls Pay first (useCheckout.js lines 591 then 616).

Also noted, low: core's MarkAsNewCustomer job now runs when the draft is built rather than at placement.

Notes for review

• Core strict-mode gap (lunarphp/lunar). Rebuilding a draft whose line quantity changed goes through OrderLineObserver::updating, which calls OrderLineQuantity::validate(). That lazy-loads $orderLine->order. CreateOrderLines also lazy-loads ProductVariant::values through getOption() on a cart with more than one line. With Model::preventLazyLoading() on, as in this suite and in many apps' local environments, either case throws. Production with lazy loading allowed is unaffected. The two retry tests switch strict mode off, with a comment explaining why. This should be fixed in core.
• Pre-existing gap, unchanged here. completeOrRefund() only catches PaymentConfirmationException. If createOrder() inside complete() throws a cart validation exception after money was captured (stock gone, for example), nothing refunds. main has the same exposure, because it always called createOrder() at async completion. Worth a follow-up.
• CheckoutPaymentConfirmationFailed is now dispatched inside the pin transaction. Nothing in this package listens to it. An app listener that writes to the database would see that write rolled back with the rejection.
• Out of scope, as briefed: migrating to 0077's renamed interfaces.

Checks

composer pint was applied. composer test (at 9270a49): 329 passed, 1 skipped, and PHPStan reported no errors.

[alecritson]

I've left 3 comments: Draft rebuild after capture can throw with no refund; Unplaced drafts accumulate for carts that pin but never pay; Tests disable lazy-loading strictness globally and never restore it.

Additional findings

• Tests disable lazy-loading strictness globally and never restore it (tests/Feature/DraftOrderAtPayGateTest.php:498) · Low · Full review

  doubleTheQuantity() and the new PaymentHoldTest case call Model::preventLazyLoading(false) and never turn it back on. This is a static flag, so it persists for the rest of the process. Later tests then run without strict mode, which can hide lazy-load regressions in unrelated suites, and results depend on test order. Restore it in an afterEach, or wrap the call in a helper that resets it, for example by capturing the previous state and restoring it.

[alecritson]

Draft rebuild after capture can throw with no refund
Medium · Full review

complete() now always calls createOrder(), which rebuilds the existing draft. The PR description says this rebuild throws under Model::preventLazyLoading() when a line quantity changed or the cart has more than one line. That setting is common in local and staging apps. On the async path this runs after money was captured, and completeOrRefund() only catches PaymentConfirmationException. A failure here leaves a charge with no placed order, which breaks the spec's refund invariant. The description calls the uncaught-exception gap pre-existing, but this PR widens it. Before, createOrder() only ran when no draft existed. Now it runs on every completion that has one.

Suggested fix: catch any Throwable from the rebuild in completeOrRefund() and refund. Alternatively, skip the rebuild when the live fingerprint already equals the pinned one, and rebuild only on the synchronous retry path where the draft may be stale. The description says the async rebuild changes nothing in that case.

[alecritson]

Fixed in 7b8e7dc, using your second option. The async path now adopts the draft the pin built and skips the rebuild. The fingerprint match proves that draft holds what was paid for, so no order build runs after capture. Only the sync path, where nothing was pinned, still rebuilds through createOrder(). That path is needed so a sync retry never places a stale draft. A new test breaks the order creation pipeline after the pin, and completion still places the draft with no refund. The same test fails on 1f2328a. The broader catch in completeOrRefund() is going into its own PR.

[alecritson]

Unplaced drafts accumulate for carts that pin but never pay
Low · Full review

Every Pay on a gateway method now creates an Order row. Abandoned PaymentProcessing sessions leave unplaced drafts attached to carts. Core's admin, reports and stock logic may count or list orders with placed_at null. The spec says nothing about cleaning these up when a session expires or is pruned, or about excluding them from queries. Consider a follow-up that removes or marks drafts when the session expires. At minimum, document that consumers must filter on placed_at.

[alecritson]

Partly addressed in 7b8e7dc. createOrder() reuses the cart's draft, so a cart never has more than one. Spec 0010 section E now says that a pin that is never paid leaves that one draft with placed_at null, and that anything listing or counting orders must filter on placed_at. That is the same split as core's draftOrder() / completedOrder(). Removing drafts when a session expires or is pruned is left for a follow-up. It needs a decision on whether a draft that a late webhook could still place is safe to delete.

[alecritson]

On the third finding (lazy-load strictness never restored): no change. The flag does not persist between tests. tests/TestCase.php calls Model::preventLazyLoading() in getEnvironmentSetUp(), and Testbench runs that for every test when it boots a fresh app. I checked with a throwaway probe: test A turned strictness off, and test B, which ran next in the same process, saw Model::preventsLazyLoading() as true. Test order cannot leak it.

[TheAaronLawrence]

I've left 2 comments: Async completion can place a stale draft when payment is confirmed without Pay; Pay now consumes coupon uses, allowing limited-code exhaustion.

Additional findings

• Async completion can place a stale draft when payment is confirmed without Pay (src/Drivers/LunarCheckoutDriver.php:218) · Medium · Full review

  On the async path complete() and core's authorize() take $cart->draftOrder as it is. After a refund and reopen, the draft holds the old cart, and only Pay rebuilds it. A payment confirmed with no intervening Pay, such as a custom client, a webhook-first confirmation or a replayed intent, would place an order that doesn't match the cart or the amount captured. The description acknowledges this and relies on the shipped UI always calling Pay. The backend should not depend on client behaviour for this. Options: have complete() compare the draft's totals or fingerprint against the pinned session and rebuild or refund on a mismatch, or invalidate or rebuild the draft when the session is reopened.

[TheAaronLawrence]

Pay now consumes coupon uses, allowing limited-code exhaustion
Medium · Security

Drafting at Pay means core's CreateOrder calls markAsUsed() before any payment exists. Anyone who can create a cart can apply a limited-use or single-use code, press Pay, and abandon the session, which burns a use for no payment. Repeating this with fresh carts can exhaust a promotion. The description accepts this as "rare", but it is cheap to do deliberately, and the draft is never cleaned up. Consider releasing the coupon use when a draft is orphaned (on session expiry or prune), consuming it at placement instead, or at least rate-limiting Pay. Add a scheduled cleanup for stale unplaced drafts.

[alecritson]

Kept as an accepted trade-off, with two mitigations in 9270a49.

• Throttle. The Pay route now uses a checkout-pay limiter: 10 a minute and 30 an hour per IP, the same pattern as the lookup endpoints. Pressing Pay and abandoning across fresh carts now costs real time per use. A real customer retries a handful of times at most. Test: the 11th Pay inside a minute gets a 429 and no draft is created.
• Root cause, filed in core as Count discount uses when an order is placed, not when its draft is created lunar#2818. Count the use when the order is placed, not when the draft is built. Releasing a use is not possible from checkout, because core has no reverse of markAsUsed().

Spec 0010 section E now records the trade-off and the throttle. I did not add the draft cleanup job. Without a way to release a use it would not give the use back, and a draft that a late webhook could still place is not safe to delete blindly. That belongs with the core change.

[alecritson]

Re: async completion placing a stale draft. Agreed that the backend should not rely on the client calling Pay. Fixed in bb7eb1a.

• The pin now stamps the draft with the checkout fingerprint (meta.checkout_fingerprint).
• complete() uses the draft as built only when that stamp equals the fingerprint it just verified. Any other draft is rebuilt by createOrder() from the verified cart. That covers a draft left by an earlier failed attempt, and a draft on a session that was pinned without the gate (a payment the gateway confirmed with no Pay step).
• New test: Pay, change the cart, refund, then pin the session without the gate (the way the webhook listener pins it) and complete. The same order id is placed, with the new quantity and the paid total. Against the previous commit that test places the stale quantity-1 draft.

One part can't be fixed in this repo. Core Stripe's authorize() reads $cart->draftOrder directly (StripePaymentType.php:355) and runs its intent-vs-order total check against it. So a payment confirmed with no Pay after a cart change still meets the stale draft inside core, and the check rejects it. That needs a core change, for example core gateways rebuilding a draft whose fingerprint differs from the cart. Spec 0010 section E.2 now records this boundary.