fix(checkout): never confirm a pay-upfront order whose payment never started - #224
Conversation
…started
2026-07-29, LekkerWeed: a production backend task exited mid-request, the
ALB returned a 502 in 87ms, and the PayCloud mint threw. The catch around
it swallowed the error, execution continued to log('SUCCESS'), and the
storefront — seeing no payUrl — fell through to the order-confirmation
screen AND called clearCart().
The customer was shown a placed order, lost her basket, and no payment was
ever taken. Dr Green order ccdeea34 sat PENDING with stock committed.
The swallow was deliberate ('never fail the order if minting fails — still
payable via the email-link flow on admin approval') and is correct for
email-link stores, where an order with no mint is the normal state. It is
wrong for DIRECT stores, which are pay-upfront: there, no mint means no
checkout happened.
Server (orders/submit):
- retry the mint 3× with backoff. The failure was transient — the
replacement task was healthy ~2 minutes later — so one blip should never
strand a pay-upfront order.
- on final failure with directPayEnabled, return paymentStartFailed:true.
The Dr Green order is deliberately kept: it stays payable via
/orders/{id}/pay or the email-link fallback, and the abandoned-order
sweep releases its stock if nobody pays. Flag is always false on
email-link stores.
Storefront (checkout page):
- paymentStartFailed no longer shows the confirmation and no longer clears
the cart. It says nothing has been charged and offers Retry payment,
which re-mints against the EXISTING order via /orders/{id}/pay rather
than creating a second one.
Tests pin both halves: retry-then-succeed, give-up-and-flag, email-link
stores never flagged, and the storefront branch that used to confirm a
phantom order.
📝 WalkthroughWalkthroughThe order submission route retries direct checkout minting and reports when payment startup fails. The checkout page preserves the order and offers payment retry, while unit tests cover retry, failure, email-link, and routing behavior. ChangesDirect payment recovery
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CheckoutPage
participant OrdersSubmitRoute
participant DrGreen
CheckoutPage->>OrdersSubmitRoute: Submit direct-pay order
OrdersSubmitRoute->>DrGreen: Create direct checkout
DrGreen-->>OrdersSubmitRoute: payUrl or mint failure
OrdersSubmitRoute-->>CheckoutPage: order data and paymentStartFailed
CheckoutPage->>DrGreen: Retry payment for pending order
DrGreen-->>CheckoutPage: payUrl or settled response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nextjs_space/app/store/[slug]/checkout/page.tsx (1)
719-741: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMain "Place Order" button stays clickable while a payment retry is pending, allowing duplicate orders.
Once
paymentStartFailedsetspendingPaymentOrderId(lines 193-199), the address form and "Place Order" button remain fully interactive. The customer can click "Place Order" again, resubmitting/orders/submitand creating a second Dr Green order while the first one is still unpaid — the opposite of the PR's goal of retrying against the existing order.🐛 Proposed fix
<Button type="submit" size="lg" className="w-full h-12 text-sm font-semibold text-white" - disabled={isSubmitting} + disabled={isSubmitting || !!pendingPaymentOrderId} style={{Alternatively, hide the whole form and only show the retry CTA once
pendingPaymentOrderIdis set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nextjs_space/app/store/`[slug]/checkout/page.tsx around lines 719 - 741, Disable the checkout form and main “Place Order” Button when pendingPaymentOrderId is set, in addition to the existing isSubmitting state. Update the form controls and submit button around the checkout flow so they cannot resubmit /orders/submit while a payment retry is pending, preserving the retry CTA for the existing pending order.
🧹 Nitpick comments (1)
nextjs_space/tests/unit/direct-pay-no-phantom-order.test.ts (1)
21-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTests exercise a hand-mirrored copy of the production logic, not the real code.
mintWithRetryanddecideare intentionally documented reimplementations of the logic inroute.tsandcheckout/page.tsx. That's pragmatic given the route's dependencies, but it means a future change to the real retry loop or storefront branching (including the missing-payUrl-on-success gap flagged onroute.ts) can drift out of sync with these tests without failing them.Consider extracting the retry loop (and the storefront decision) into small, exported, dependency-free helper functions that both the route/page and these tests import, so the tests validate the actual production code path.
Also applies to: 95-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nextjs_space/tests/unit/direct-pay-no-phantom-order.test.ts` around lines 21 - 49, Extract the retry logic currently mirrored by mintWithRetry and the storefront branching represented by decide into small exported, dependency-free helpers. Update route.ts and checkout/page.tsx to use these helpers, then import them in the tests instead of maintaining duplicate implementations, preserving the existing retry behavior and missing-payUrl handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nextjs_space/app/api/store/`[slug]/orders/submit/route.ts:
- Around line 214-237: Validate checkout.payUrl immediately after
createDirectCheckout returns and only break the mint retry loop when it is
non-empty; treat a missing or empty value as a failed attempt so the existing
retry/failure path sets paymentStartFailed and prevents clearing the cart
without a payment URL. Update the relevant direct-pay test coverage to include a
mint response with an empty payUrl.
In `@nextjs_space/app/store/`[slug]/checkout/page.tsx:
- Around line 42-74: The retryPayment flow must preserve the settled order’s
details and render the checkout success confirmation when the payment endpoint
returns paid: true, rather than clearing the cart and falling through to the
empty-cart view; reuse the existing orderResult state or persist equivalent
paid-order data, while leaving payUrl handling unchanged. Update
submit/route.ts’s paymentStartFailed fallback so responses clearly distinguish
paid orders from payUrl responses and submission errors.
---
Outside diff comments:
In `@nextjs_space/app/store/`[slug]/checkout/page.tsx:
- Around line 719-741: Disable the checkout form and main “Place Order” Button
when pendingPaymentOrderId is set, in addition to the existing isSubmitting
state. Update the form controls and submit button around the checkout flow so
they cannot resubmit /orders/submit while a payment retry is pending, preserving
the retry CTA for the existing pending order.
---
Nitpick comments:
In `@nextjs_space/tests/unit/direct-pay-no-phantom-order.test.ts`:
- Around line 21-49: Extract the retry logic currently mirrored by mintWithRetry
and the storefront branching represented by decide into small exported,
dependency-free helpers. Update route.ts and checkout/page.tsx to use these
helpers, then import them in the tests instead of maintaining duplicate
implementations, preserving the existing retry behavior and missing-payUrl
handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b6618a5-3011-4f6c-a7eb-96ff0f019552
📒 Files selected for processing (3)
nextjs_space/app/api/store/[slug]/orders/submit/route.tsnextjs_space/app/store/[slug]/checkout/page.tsxnextjs_space/tests/unit/direct-pay-no-phantom-order.test.ts
| const checkout = await createDirectCheckout({ | ||
| drGreenOrderId: orderResponse.drGreenOrderId, | ||
| // Tenant hosts ({slug}.budstacks.io / custom domains) serve the | ||
| // store at root; the legacy "/store/<slug>" path 404s there. | ||
| returnUrl: storefrontUrl( | ||
| origin, | ||
| host, | ||
| slug, | ||
| `/payment/return/${orderResponse.orderId}`, | ||
| ), | ||
| apiKey: drGreenConfig.apiKey, | ||
| secretKey: drGreenConfig.secretKey, | ||
| apiUrl: drGreenConfig.apiUrl, | ||
| // US-008: the shopper's IP becomes PayCloud's term_ip fraud hint — | ||
| // without it the transaction is attributed to this server's egress. | ||
| customerIp: getPublicClientIp(request.headers), | ||
| }); | ||
| payUrl = checkout.payUrl; | ||
| log("DIRECT_CHECKOUT", { | ||
| hasPayUrl: !!payUrl, | ||
| expiresAt: checkout.expiresAt, | ||
| attempt, | ||
| }); | ||
| break; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate payUrl before treating a mint attempt as successful.
createDirectCheckout's own implementation notes Dr Green responses are "sometimes wrapped in { data }, sometimes raw" — i.e. parsing ambiguity is a known risk. If a malformed/edge-case response resolves with an empty or missing payUrl, this loop still treats it as success (break), leaving paymentStartFailed = false. The storefront then falls through to setOrderResult(...); clearCart() (checkout/page.tsx data.order?.payUrl check fails, paymentStartFailed is falsy) — reproducing the exact incident this PR fixes: a placed-order confirmation and cleared cart with no way to pay.
🐛 Proposed fix
const checkout = await createDirectCheckout({
...
});
- payUrl = checkout.payUrl;
+ if (!checkout.payUrl) {
+ throw new Error("createDirectCheckout returned no payUrl");
+ }
+ payUrl = checkout.payUrl;
log("DIRECT_CHECKOUT", {
hasPayUrl: !!payUrl,
expiresAt: checkout.expiresAt,
attempt,
});
break;Consider adding a corresponding test case in direct-pay-no-phantom-order.test.ts (mint resolves with an empty payUrl) alongside the existing retry/failure cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const checkout = await createDirectCheckout({ | |
| drGreenOrderId: orderResponse.drGreenOrderId, | |
| // Tenant hosts ({slug}.budstacks.io / custom domains) serve the | |
| // store at root; the legacy "/store/<slug>" path 404s there. | |
| returnUrl: storefrontUrl( | |
| origin, | |
| host, | |
| slug, | |
| `/payment/return/${orderResponse.orderId}`, | |
| ), | |
| apiKey: drGreenConfig.apiKey, | |
| secretKey: drGreenConfig.secretKey, | |
| apiUrl: drGreenConfig.apiUrl, | |
| // US-008: the shopper's IP becomes PayCloud's term_ip fraud hint — | |
| // without it the transaction is attributed to this server's egress. | |
| customerIp: getPublicClientIp(request.headers), | |
| }); | |
| payUrl = checkout.payUrl; | |
| log("DIRECT_CHECKOUT", { | |
| hasPayUrl: !!payUrl, | |
| expiresAt: checkout.expiresAt, | |
| attempt, | |
| }); | |
| break; | |
| const checkout = await createDirectCheckout({ | |
| drGreenOrderId: orderResponse.drGreenOrderId, | |
| // Tenant hosts ({slug}.budstacks.io / custom domains) serve the | |
| // store at root; the legacy "/store/<slug>" path 404s there. | |
| returnUrl: storefrontUrl( | |
| origin, | |
| host, | |
| slug, | |
| `/payment/return/${orderResponse.orderId}`, | |
| ), | |
| apiKey: drGreenConfig.apiKey, | |
| secretKey: drGreenConfig.secretKey, | |
| apiUrl: drGreenConfig.apiUrl, | |
| // US-008: the shopper's IP becomes PayCloud's term_ip fraud hint — | |
| // without it the transaction is attributed to this server's egress. | |
| customerIp: getPublicClientIp(request.headers), | |
| }); | |
| if (!checkout.payUrl) { | |
| throw new Error("createDirectCheckout returned no payUrl"); | |
| } | |
| payUrl = checkout.payUrl; | |
| log("DIRECT_CHECKOUT", { | |
| hasPayUrl: !!payUrl, | |
| expiresAt: checkout.expiresAt, | |
| attempt, | |
| }); | |
| break; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nextjs_space/app/api/store/`[slug]/orders/submit/route.ts around lines 214 -
237, Validate checkout.payUrl immediately after createDirectCheckout returns and
only break the mint retry loop when it is non-empty; treat a missing or empty
value as a failed attempt so the existing retry/failure path sets
paymentStartFailed and prevents clearing the cart without a payment URL. Update
the relevant direct-pay test coverage to include a mint response with an empty
payUrl.
| const retryPayment = async () => { | ||
| if (!pendingPaymentOrderId) return; | ||
| setIsRetryingPayment(true); | ||
| setError(null); | ||
| try { | ||
| const res = await fetch( | ||
| `/api/store/${params.slug}/orders/${pendingPaymentOrderId}/pay`, | ||
| { method: "POST" }, | ||
| ); | ||
| const data = await res.json(); | ||
| if (!res.ok) throw new Error(data.error || "Could not start payment"); | ||
| if (data.payUrl) { | ||
| window.location.href = data.payUrl; | ||
| return; | ||
| } | ||
| if (data.paid) { | ||
| // Already settled (e.g. the first mint actually succeeded upstream). | ||
| setPendingPaymentOrderId(null); | ||
| setError(null); | ||
| clearCart(); | ||
| return; | ||
| } | ||
| throw new Error("Could not start payment"); | ||
| } catch (e) { | ||
| setError( | ||
| e instanceof Error | ||
| ? `${e.message}. Nothing has been charged.` | ||
| : "Could not start payment. Nothing has been charged.", | ||
| ); | ||
| } finally { | ||
| setIsRetryingPayment(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -p 'orders/\[id\]|orders/\[orderId\]' nextjs_space/app/api/store
rg -n -A 20 'export (async )?function POST' nextjs_space/app/api/store/'[slug]'/orders/'[id]'/pay/route.ts 2>/dev/null
fd -e ts pay --full-path nextjs_space/app/api/storeRepository: AutomatosAI/budstack-saas
Length of output: 802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== checkout file outline =="
ast-grep outline nextjs_space/app/store/[slug]/checkout/page.tsx --view expanded || true
echo "== checkout lines 1-120 =="
sed -n '1,120p' nextjs_space/app/store/[slug]/checkout/page.tsx
echo "== checkout lines 200-320 =="
sed -n '200,320p' nextjs_space/app/store/[slug]/checkout/page.tsx
echo "== pay route =="
sed -n '1,260p' nextjs_space/app/api/store/[slug]/orders/[orderId]/pay/route.ts
echo "== payment status route (for contract context maybe) =="
sed -n '1,220p' nextjs_space/app/api/store/[slug]/orders/[orderId]/payment-status/route.ts
echo "== all references to data.paid / payUrl / pendingPaymentOrderId =="
rg -n "data\.paid|payUrl|pendingPaymentOrderId|setOrderResult|OrderResult|items\.length" nextjs_space/app nextjs_space/app/api -SRepository: AutomatosAI/budstack-saas
Length of output: 19458
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== submit post/payment status relevant sections =="
sed -n '220,300p' nextjs_space/app/api/store/[slug]/orders/submit/route.ts
echo "== return page relevant state/contracts =="
sed -n '1,160p' nextjs_space/app/store/[slug]/payment/return/[orderId]/page.tsxRepository: AutomatosAI/budstack-saas
Length of output: 9751
Show the success state when retry reports a settled order.
POST /orders/{id}/pay returns { paid: true } only when the order is already PAID; clearing the cart leaves this checkout page rendering with items.length === 0 and no orderResult, so users see “Your cart is empty. Add items before checking out.” Persist the paid order details (or reuse the original order result) and render the confirmation instead. Also update submit/route.ts’s paymentStartFailed fallback so the contract for { paid } vs { payUrl }/submit error is not ambiguous.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nextjs_space/app/store/`[slug]/checkout/page.tsx around lines 42 - 74, The
retryPayment flow must preserve the settled order’s details and render the
checkout success confirmation when the payment endpoint returns paid: true,
rather than clearing the cart and falling through to the empty-cart view; reuse
the existing orderResult state or persist equivalent paid-order data, while
leaving payUrl handling unchanged. Update submit/route.ts’s paymentStartFailed
fallback so responses clearly distinguish paid orders from payUrl responses and
submission errors.
2 participants
What happened today
LekkerWeed, 15:14 BST. A production backend task exited (
EssentialContainerExited, code 1). The ALB returned a 502 in 87ms toPOST /payments/checkout. The mint threw.The storefront then did this:
The customer was shown a placed order, lost her cart, and no payment was ever taken. Dr Green order
ccdeea34sat PENDING with stock committed. The comment directly above that code even says "the order is not really placed until payment succeeds" — but it only guards thepayUrlbranch.Why the swallow existed
It's deliberate, and it's right — for one of the two checkout routes:
The old code treated both identically.
directPayEnabledalready distinguishes them — this change uses it.Server —
orders/submitdirectPayEnabled, returnpaymentStartFailed: true.The Dr Green order is deliberately kept, not cancelled — it stays payable via
/orders/{id}/payor the email-link fallback, and the abandoned-order sweep releases its stock if nobody ever pays. The flag is alwaysfalseon email-link stores.Storefront — checkout page
paymentStartFailedno longer shows the confirmation and no longer clears the cart. It states that nothing has been charged and offers Retry payment, which re-mints against the existing order via/orders/{id}/payrather than creating a second one.Tests
tests/unit/direct-pay-no-phantom-order.test.ts— retry-then-succeed (the 502 case), give-up-and-flag after 3 attempts, email-link stores never flagged, first-attempt success doesn't retry, plus the storefront branch that used to confirm a phantom order.Not fixed here
ccdeea34still needs a decision — re-mint her a payment link, or void it and release stock.Summary by CodeRabbit
New Features
Bug Fixes
Tests