Add stable Stripe and Square refund idempotency keys - #1912
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefund flows now generate deterministic provider-specific idempotency keys. Square and Stripe pass these keys to refund APIs, while Stripe request plumbing supports explicit per-request overrides. Tests cover determinism, propagation, and empty-string behavior. ChangesRefund idempotency
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RefundFlow
participant refundIdempotencyKey
participant StripeClient
participant StripeRequest
participant StripeAPI
RefundFlow->>refundIdempotencyKey: Generate key from provider and intent ID
RefundFlow->>StripeClient: Create refund with idempotency key
StripeClient->>StripeRequest: Send request options
StripeRequest->>StripeAPI: POST refund with Idempotency-Key header
StripeAPI-->>RefundFlow: Refund response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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
🤖 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 `@src/shared/stripe/request.ts`:
- Around line 265-273: Define a named callable type for the async request
function returned by createStripeRequest, including its parameter and Promise<T>
return signature, then annotate the exported createStripeRequest factory with
that type. Preserve the existing request implementation and generic behavior
while replacing implicit public return-type inference.
In `@test/shared/stripe/client.test.ts`:
- Around line 71-88: Update the createStripeClient setup in the refund
idempotency-key test to use maxNetworkRetries: 1 instead of 0, while preserving
the supplied stable-refund-key and exact captured-header assertion so the test
verifies explicit-key precedence over retry-generated keys.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3df8cdff-c141-4253-b7b3-1dcc2b9a01aa
📒 Files selected for processing (11)
scripts/mutation/equivalent-mutants.txtsrc/shared/payment-idempotency.tssrc/shared/square.tssrc/shared/stripe.tssrc/shared/stripe/client.tssrc/shared/stripe/request.tstest/shared/payment-idempotency.test.tstest/shared/square/retrieve-refund.test.tstest/shared/stripe-provider/operations.test.tstest/shared/stripe/client.test.tstest/shared/stripe/request.test.ts
Extract refundHeaderProbe helper for the refund-header-capturing client shared by client.test.ts and request.test.ts, and set maxNetworkRetries to 1 so the explicit-key override is proven to take precedence over the retry-generated default (a zero-retry setup makes the override untestable). 0% cpd and 100% mutation kill held.
- StripeRequestOptions doc referenced refundIdempotencyKey via {@link}
across modules (won't resolve); plain-qualified to its module instead.
- refundHeaderProbe's maxNetworkRetries param was never overridden by any
caller; inline the intentional 1 with a comment and trim the now-duplicated
rationale from the client.test precedence comment.
… rebase After rebasing onto main (absorbing #1909 test hardening, #1910 equivalent-registry cleanup, and #1912 stable Square refund idempotency keys), the three square.ts equivalent-mutant entries moved to 294/684/754 and the Square refundPayment now uses #1912 stable refundIdempotencyKey instead of crypto.randomUUID. Updated the entries to match and the TODO note for #1912 landing (the stable key mitigates the double-pay half of the PENDING-refund redelivery risk). Exhaustive mutation on square.ts is 100% (290 killed, 3 equivalents suppressed).
… rebase After rebasing onto main (absorbing #1909 test hardening, #1910 equivalent-registry cleanup, and #1912 stable Square refund idempotency keys), the three square.ts equivalent-mutant entries moved to 294/684/754 and the Square refundPayment now uses #1912 stable refundIdempotencyKey instead of crypto.randomUUID. Updated the entries to match and the TODO note for #1912 landing (the stable key mitigates the double-pay half of the PENDING-refund redelivery risk). Exhaustive mutation on square.ts is 100% (290 killed, 3 equivalents suppressed).
1 participant
A retried webhook redelivery of the same refund used to reach the payment provider with a fresh idempotency key, so the provider could treat the second call as a new refund rather than a duplicate.
What changed
This adds one small, provider-and-payment-scoped key that is stable across retries, and threads it through the Stripe and Square refund paths.
src/shared/payment-idempotency.ts—refundIdempotencyKey(provider, paymentReference)returns a deterministic SHA-256 base64url key (43 characters, within each provider key-length limit). The same provider-and-payment pair always produces the same key; a different provider for the same payment reference hashes to a different key, so the two never collide.stripe.tsrefundPaymentcomputes this key for the payment intent and passes it toclient.refunds.create. The request transport (stripe/request.ts) gained an optionalidempotencyKeyon its request options (preferred over the per-POST random retry key), and the client surface (stripe/client.ts) forwards the key into theIdempotency-Keyheader.square.tsrefundPaymenthandsrefundIdempotencyKey("square", paymentId)to the SDK refund instead of a freshcrypto.randomUUID().Nothing else about the refund flow changes — the key is the same shape each provider already expected, it is just stable now.
Why this matters
When a webhook is redelivered (Stripe retries on non-2xx; Square retries similarly), a refund retried for the same payment must resolve to one provider-side refund, not a second charge-back. A random per-call key defeats that; a deterministic
(provider, payment)key makes the second call a deduplicate of the first. Different providers hashing to different keys also stops a Stripe refund and a Square refund that happen to share a reference from collapsing onto one provider key.Tests
Direct tests proving the mechanism end to end:
test/shared/payment-idempotency.test.ts— the key is deterministic, differs across payment references, differs across providers for the same reference, and is the SHA-256 base64url value (43 chars).test/shared/stripe/client.test.ts—refunds.create(params, key)sends exactly that value as theIdempotency-Keyheader.test/shared/stripe-provider/operations.test.ts—stripePaymentProvider.refundPayment(intent)passes the SHA-256-derived stable key (asserted as the exact constant).test/shared/square/retrieve-refund.test.ts— strengthened the existing weak "is a string" assertions to the exact stable key, and added a test that two refunds of the same payment reuse one key (webhook redelivery safety).test/shared/stripe/request.test.ts— locks the nullish-coalescing semantics so an explicit empty override is not swallowed into the random retry default.Mutation testing
Targeted exhaustive mutation at 100% kill on the changed source:
payment-idempotency.ts,stripe/client.ts,stripe/request.ts. The Stripe and Square refund call sites are covered by exact-key assertions that kill operator and provider swaps.Notes for review
PR #1905 overlaps the Square provider files. This change is deliberately narrow (one line in
square.tsrefundPayment, and the Stripe side lives in files #1905 does not touch), so rebasing after #1905 moves or merges is straightforward. The twosquare.tsentries inscripts/mutation/equivalent-mutants.txtwere re-pinned to their new line numbers after the added import shifted them down.Summary by CodeRabbit