AI

Architectural Review: PAY-402 — Partial Refund Support

1. Problem Statement

PAY-402 requests a service method for partial refunds with three invariants: amount cap, observability, and idempotency.

Cross-referencing the codebase against DOMAIN_MODEL.md and ENGINEERING_STANDARDS.md reveals that the core implementation already exists in PaymentService.create_refund().

However, the review surfaced one critical design flaw and two gaps.

2. Context Validation — Invariant Checklist

3. Findings

Finding 1 — CRITICAL: Stripe HTTP call inside transaction.atomic()

The Stripe Refund.create() call at services.py#L77 executes inside the transaction.atomic() block that holds a SELECT FOR UPDATE row lock on the PaymentIntent.

Risks:

• DB connection exhaustion: Row lock held for the full Stripe round-trip (200ms–30s). Under load, this starves the connection pool.

• Phantom refund on timeout: If the HTTP call times out, StripeError is raised, the DB transaction rolls back (Refund row deleted), but Stripe may have actually processed the refund. On the next user retry, a new trace_id is generated → the idempotency key offers no protection → double refund.

Required fix:

Two-phase approach — create the Refund row and release the lock first, then call Stripe outside the transaction, then update status in a second atomic block.

Finding 2 — GAP: Webhook handler incomplete

Per IMPLEMENTATION_STATE.md, the webhook handler is ~50% complete. The view at views.py#L78 only listens for charge.refund.updated but not charge.refunded (the initial refund success event from Stripe). This means refunds that succeed synchronously may never transition from PENDING to SUCCEEDED in our system.

Finding 3 — GAP: No concurrent-request test

Tests validate sequential partial refunds but no test exercises two simultaneous refund requests racing for the same PaymentIntent, which is the scenario the select_for_update() lock is meant to protect.

4. Visual Logic — Proposed Two-Phase Refund Flow

sequenceDiagram
    participant V as View
    participant S as PaymentService
    participant DB as PostgreSQL
    participant St as Stripe API

    V->>S: create_refund(pi_id, amount)

    rect rgb(230, 245, 255)
        note right of S: Phase 1 — Lock, validate, persist
        S->>DB: BEGIN + SELECT FOR UPDATE (PaymentIntent)
        DB-->>S: PaymentIntent row (locked)
        S->>DB: SUM(non-failed refunds)
        DB-->>S: already_refunded
        S->>S: Validate amount cap
        S->>DB: INSERT Refund (status=CREATED)
        S->>DB: COMMIT (lock released)
    end

    rect rgb(255, 245, 230)
        note right of S: Phase 2 — External call (no lock held)
        S->>St: Refund.create(idempotency_key=trace_id)
        St-->>S: stripe_refund_id
    end

    rect rgb(230, 255, 230)
        note right of S: Phase 3 — Update status
        S->>DB: BEGIN + SELECT FOR UPDATE (Refund)
        S->>DB: UPDATE status=PENDING, stripe_refund_id
        S->>DB: COMMIT
    end

    S-->>V: Refund object

Note: In practice, the first pass won’t always catch everything — the win is that the workflow forces an audit against your invariants before any code ships.

5. Developer Plan

The Developer should execute these tasks in order:

6. Context Patch

ai/context/IMPLEMENTATION_STATE.md — proposed update after Developer completes work:

# Implementation State: Refund Feature

## Current Status
- Service Layer: PaymentService.create_refund() refactored to two-phase (lock → call → update)
- Data Layer: Refund model migrated and active
- Webhooks: `charge.refunded` + `charge.refund.updated` both handled
- Idempotency: trace_id used as Stripe idempotency_key; timeout paths enqueue reconciliation

## Resolved
- PAY-402: Partial refund support (amount cap, observability, idempotency)
- Stripe call moved outside transaction.atomic() to prevent lock contention

## Next Steps
1. Implement Celery reconciliation task for ambiguous Stripe timeouts
2. Add integration tests with Stripe sandbox (blocked on QA API keys)
3. Update observability spans for webhook failures

Operational Constraint

Per the Architect role: no implementation code is produced at this stage. The Developer should take this plan and implement against the DEVELOPER.md role constraints, adhering to STYLE_GUIDE.md and BOUNDARIES.md.

AI

PAY-402 Implementation Summary

Files Modified

payments/services.py — Two-Phase Refund

Refactored create_refund to eliminate the critical flaw of holding a DB row lock during the Stripe HTTP call.

Error handling:

• APIConnectionError (ambiguous) → Refund stays CREATED, enqueues reconcile_refund_with_stripe Celery task.
• Other StripeError (definitive) → Refund marked FAILED (persisted, not deleted), raises RefundGatewayError.

payments/tasks.py — Reconciliation Task

New reconcile_refund_with_stripe task (max 5 retries, 120s delay):

• Re-issues the Stripe call using the same idempotency key (trace_id), so Stripe deduplicates.
• Connection error → retries.
• Definitive Stripe error → marks refund FAILED.
• Success → maps Stripe's status to internal status and persists.

payments/views.py — Webhook Gap Closed

Added "charge.refunded" to handled_event_types alongside "charge.refund.updated", ensuring refunds that succeed synchronously on Stripe's side are captured by our webhook handler.

payments/tests.py — Updated & New Tests

Updated tests:

New test classes:

1. ai/context/IMPLEMENTATION_STATE.md — Context Patch

Updated to reflect resolved status of PAY-402, two-phase architecture, and completed webhook coverage.