Review payment refund orchestration

Review exercise · TypeScript

Review brief

Advanced45 min

A commerce service is adding partial refunds through a payment provider that first accepts work and later confirms it by signed webhook. Client requests may be retried, multiple operators may refund the same payment concurrently, and provider calls can have ambiguous outcomes.

Behavioral contract

A client `requestKey` is unique per merchant and maps every retry to one stored refund and one result. The refund ID is stable and is also the provider idempotency key on every attempt.
Refund amounts are positive integer minor units in the captured payment currency. Under a payment-scoped database lock, the sum of pending and succeeded refunds plus the new amount must not exceed the captured amount.
Creating the pending refund and appending its provider command happen in one database transaction. A durable dispatcher performs provider calls; request handling never leaves a refund without a retryable command.
Provider `accepted` means processing has started, not that money was refunded. A refund becomes `succeeded` only after a verified `refund.succeeded` webhook; rejection or ambiguous delivery remains pending or transitions to an explicitly retryable failure.
Every provider attempt uses the stored refund ID as its idempotency key. Generating a new key for a retry can create multiple external refunds.
The webhook verifier authenticates the raw payload and signature. Claiming its stable event ID and applying the refund transition commit atomically; a crash cannot leave an event permanently claimed without its transition, and replayed delivery has no additional effect.
Caller authorization to the merchant and provider signature cryptography are supplied by upstream middleware and the verifier respectively; review the orchestration after those boundaries.

Review instructions

1Review request retries, simultaneous partial refunds, crash points around provider delivery, and repeated webhooks.
2Distinguish a pending provider acknowledgement from final financial success.
3Prioritize no more than six findings and treat integer minor-unit arithmetic as intentional.

Learning objectives

• Enforce cumulative money invariants under concurrency
• Preserve stable identity through durable provider delivery
• Separate provider acceptance from confirmed refund success

src/refund-types.tsTypeScript

@@ -0,0 +1,80 @@

1+export interface PaymentRecord {

2+ id: string;

3+ merchantId: string;

4+ capturedAmountMinor: number;

5+ currency: string;

6+}

8+export interface RefundRecord {

9+ id: string;

10+ merchantId: string;

11+ paymentId: string;

12+ requestKey: string;

13+ amountMinor: number;

14+ currency: string;

15+ status: "pending" | "succeeded" | "failed";

16+}

17+

18+export interface RefundRepository {

19+ findPayment(paymentId: string, transaction?: object): Promise<PaymentRecord | null>;

20+ findByRequestKey(

21+ merchantId: string,

22+ requestKey: string,

23+ transaction?: object,

24+ ): Promise<RefundRecord | null>;

25+ listForPayment(paymentId: string, transaction?: object): Promise<RefundRecord[]>;

26+ create(refund: RefundRecord, transaction?: object): Promise<void>;

27+ markSucceeded(

28+ refundId: string,

29+ providerRefundId: string,

30+ transaction?: object,

31+ ): Promise<void>;

32+}

33+

34+export interface RefundCommandOutbox {

35+ enqueue(

36+ command: { refundId: string; paymentId: string; amountMinor: number; currency: string },

37+ transaction?: object,

38+ ): Promise<void>;

39+}

40+

41+export interface PaymentProvider {

42+ startRefund(input: {

43+ paymentId: string;

44+ amountMinor: number;

45+ currency: string;

46+ idempotencyKey: string;

47+ }): Promise<

48+ | { status: "accepted"; providerRefundId: string }

49+ | { status: "rejected"; reason: string }

50+ >;

51+}

52+

53+export interface RefundDependencies {

54+ refunds: RefundRepository;

55+ commands: RefundCommandOutbox;

56+ provider: PaymentProvider;

57+ ids: { next(): string };

58+ database: {

59+ withPaymentLock<T>(paymentId: string, work: (transaction: object) => Promise<T>): Promise<T>;

60+ };

61+}

62+

63+export interface RefundWebhookEvent {

64+ eventId: string;

65+ type: "refund.succeeded";

66+ refundId: string;

67+ providerRefundId: string;

68+}

69+

70+export interface RefundWebhookVerifier {

71+ verify(rawBody: string, signature: string): Promise<RefundWebhookEvent>;

72+}

73+

74+export interface WebhookEventStore {

75+ claimOnce(eventId: string): Promise<boolean>;

76+ claimAndApply(

77+ eventId: string,

78+ apply: (transaction: object) => Promise<void>,

79+ ): Promise<boolean>;

80+}

Reference solution

The findings below are ordered by priority.

Findings

1. Concurrent partial refunds can exceed the captured amount

Location: src/refund-types.ts:18-31,53-60 and src/refund-service.ts:26-32,34-43

The service reads totals and creates a refund without withPaymentLock, so two requests can observe the same remaining balance and both pass. It also sums only succeeded refunds, allowing any already pending amount to be spent again even without concurrency.

Under the payment-scoped database lock, pass the supplied transaction through the repository reads, sum both pending and succeeded amounts, enforce the limit, and create the new pending refund before releasing the transaction.

2. Retried client requests create new refunds

Location: src/refund-types.ts:18-31,53-60 and src/refund-service.ts:16-43

requestKey is copied into a new record but never looked up, and every call generates a new refund ID. A timeout followed by an ordinary client retry can therefore reserve and send the same requested refund more than once.

Within the locked transaction, look up the merchant-scoped request key through the transaction-aware repository method and return its existing refund when present. Back the check with a unique constraint to resolve concurrent identical requests safely.

3. The provider call has no durable handoff

Location: src/refund-service.ts:43-50

The refund row is committed and then the request handler calls the provider directly; the command outbox and database transaction are unused. A crash after create but before or during startRefund leaves a pending refund with no durable work to resume after an ambiguous outcome.

Create the pending refund and enqueue its command in the same transaction. Let a durable dispatcher retry provider delivery and record attempt outcomes.

4. Provider retries receive a new idempotency identity

Location: src/refund-service.ts:45-50

The provider key comes from a second ids.next() call instead of the stored refund ID. Any retry that reruns this code uses another key, so the provider may execute multiple external refunds for one local record.

Use refund.id unchanged as the provider idempotency key on every dispatcher attempt.

5. Provider acceptance is recorded as financial success

Location: src/refund-service.ts:51-58

An accepted result only means processing started, but the service immediately marks the refund succeeded and returns that status. Later provider rejection would leave local accounting and customer-visible state claiming money was refunded when it was not.

Keep the refund pending after acceptance. Transition to succeeded only from the verified success webhook, with an explicit retryable failure path for rejection or ambiguous delivery.

6. Replayed webhooks apply the transition repeatedly

Location: src/refund-types.ts:70-80 and src/refund-webhook.ts:14-17

Signature verification is correctly awaited, but the stable eventId is never passed to claimOnce; the event store is merely referenced with void. Provider webhook retries can therefore repeat the success transition and any ledger or notification effects attached to it.

Use claimAndApply to claim the verified event ID and mark the refund succeeded in one transaction. If the callback fails, the claim rolls back and delivery remains retryable; if the event was already committed, return without reapplying the transition.

Reasonable non-findings

Integer minor-unit arithmetic is the required representation and avoids floating-point rounding.
Returning payment not found for both absence and merchant mismatch avoids revealing another merchant's payment.
Verification of the raw webhook body is correctly delegated to the supplied verifier; the defect is replay handling after authentication.