feat: reconcile async Stripe refunds

[bseverino]

Summary

• Implements EDUN-15350: production-ready async UPI refunds in commerce-coordinator.
• Design: Enable Payment Methods (UPI) - Schema & High-Level Technical Documentation (work item #8).
• Stripe pending is a valid refund initiation: persist a CommerceTools Refund transaction in PENDING, leave return Initial, and do not revoke LMS or emit Order Refunded until Stripe confirms success.
• Shared reconciler (stripe_refund_reconcile) finalizes refund.updated / refund.failed / charge.refunded and forward-path succeeded with Stripe Refund ID as the identity. canceled maps like failed.
• Does not change stripe_payment_finalize.py (EDUN-15347 pay-in path stays independent).

What Was Built

Component	Details
Stripe refund create	refund_payment_intent accepts succeeded and pending; other statuses still fail.
Webhook routing	Existing signed WebhookView handles refund.updated and refund.failed (Refund object). HTTP single-invocation uses Stripe Event ID; refund-ID lock serializes work.
Reconciler	New stripe_refund_reconcile.py: PENDING persist, PENDING→SUCCESS/FAILURE change-state, Dashboard path with no CT Return updates payment only.
Side effects	Persist return REFUNDED before LMS revoke and Segment. Segment Order Refunded uses message_id = Stripe Refund ID. Failure → FAILURE / NotRefunded, no revoke/analytics.
Forward return task	Stripe succeeded uses the same reconciler; Celery autoretries lock/dispatch errors. Pending still skips terminal side effects.

Deploy Verification

Not applicable (no CDK / CloudFormation). Activation still requires Stripe Dashboard allowlist of refund.updated and refund.failed on the coordinator webhook endpoint (ops; not in this PR).

Test Plan

• Unit tests: Stripe client pending/succeeded, webhook routing (Event ID, pending then succeeded, 503 on lock/dispatch), pipeline pending persist, reconciler state machine, forward-task autoretry
• Focused pytest on changed modules
• Full pytest suite (tox py312-django42)
• Stage: UPI refund → CT PENDING + return Initial + no LMS change; refund.updated succeeded → Refunded + revoke + one Segment event
• Stage: refund.failed / canceled → FAILURE + NotRefunded + access preserved
• Replay the same webhook → no extra Order Refunded (stable Segment message_id)
• Confirm Stripe endpoint allowlist includes refund.updated and refund.failed in the target env

NFR Compliance

N/A for this Open edX Django IDA (no new public EDU API / OpenAPI / EDU-Events schema).

Registration Compliance

N/A (no service-discovery, M2M, or QAaS registration changes).

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
commerce_coordinator/apps/commercetools
clients.py					987-994
pipeline.py
stripe_refund_reconcile.py					89, 117, 141-148, 166-172, 176-182, 278, 280-281, 300, 334, 342-354
tasks.py
utils.py
commerce_coordinator/apps/commercetools/catalog_info
edx_utils.py
commerce_coordinator/apps/commercetools/sub_messages
tasks.py					432-435
commerce_coordinator/apps/commercetools/tests
test_clients.py
test_pipeline.py
test_stripe_refund_reconcile.py
test_tasks.py
test_utils.py
commerce_coordinator/apps/commercetools/tests/sub_messages
test_tasks.py
commerce_coordinator/apps/core
views.py
commerce_coordinator/apps/stripe
clients.py
constants.py
views.py					89, 229-234, 238-239
commerce_coordinator/apps/stripe/tests
test_clients.py
test_views.py					653
Project Total

This report was generated by python-coverage-comment-action

[Copilot]

Pull request overview

Adds first-class support for async Stripe refund lifecycles (including pending) for CommerceTools-backed orders by introducing a shared refund reconciler and extending webhook + forward-task flows to use it, so CT state and downstream side effects occur only once refunds are terminal.

Changes:

• Extend Stripe webhook routing to handle refund.updated / refund.failed (Refund object events) and serialize processing by Stripe Refund ID.
• Introduce stripe_refund_reconcile to persist/transition CT refund transactions (PENDING → SUCCESS/FAILURE) and gate terminal side effects (return state, LMS revoke, Segment).
• Update forward “order returned” task + pipeline logic to defer terminal actions for pending and delegate terminal reconciliation to the shared reconciler, with corresponding unit tests.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
commerce_coordinator/apps/stripe/views.py	Routes additional refund webhook event types and introduces refund-ID locking before dispatch.
commerce_coordinator/apps/stripe/tests/test_views.py	Adds coverage for refund object events, lock contention, and dispatch failure behavior.
commerce_coordinator/apps/stripe/tests/test_clients.py	Adds test coverage for pending refund responses.
commerce_coordinator/apps/stripe/constants.py	Adds new Stripe event types and refund statuses.
commerce_coordinator/apps/stripe/clients.py	Accepts pending refunds as a valid initiation result.
commerce_coordinator/apps/commercetools/utils.py	Updates “full refund” detection and adds lookup by refund interaction ID.
commerce_coordinator/apps/commercetools/tests/test_utils.py	Adds tests for pending refund not counting as fully refunded + canceled mapping.
commerce_coordinator/apps/commercetools/tests/test_tasks.py	Adjusts refund task tests to assert reconciler delegation.
commerce_coordinator/apps/commercetools/tests/test_stripe_refund_reconcile.py	New reconciler test suite covering state transitions and side-effect gating.
commerce_coordinator/apps/commercetools/tests/test_pipeline.py	Verifies pending Stripe refunds don’t transition CT returns to REFUNDED.
commerce_coordinator/apps/commercetools/tests/test_clients.py	Adds test for changing CT refund transaction state.
commerce_coordinator/apps/commercetools/tests/sub_messages/test_tasks.py	Updates forward-task tests to integrate reconciler and pending behavior.
commerce_coordinator/apps/commercetools/tasks.py	Delegates refund task behavior to the shared reconciler and expands autoretry conditions.
commerce_coordinator/apps/commercetools/sub_messages/tasks.py	Updates forward flow to defer pending and delegate succeeded Stripe refunds to reconciler.
commerce_coordinator/apps/commercetools/stripe_refund_reconcile.py	New shared reconciler implementing CT transaction persistence/state machine + terminal side effects.
commerce_coordinator/apps/commercetools/pipeline.py	Introduces “refund_pending” signal and defers CT return state transition for async Stripe refunds.
commerce_coordinator/apps/commercetools/clients.py	Adds CT API helper to change refund transaction state.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

commerce_coordinator/apps/commercetools/stripe_refund_reconcile.py:120

• _update_return_state() indexes stripe_refund["payment_intent"], but the Refund TypedDict marks payment_intent as optional and some Stripe refund payloads may omit it. That will raise KeyError during reconciliation and prevent return/payment updates and side effects.

    refund_id = stripe_refund["id"]
    payment = client.get_payment_by_key(payment_intent_id)
    transaction = get_refund_transaction_by_interaction_id(payment, refund_id)

commerce_coordinator/apps/stripe/views.py:236

• The webhook handler acquires refund_reconcile_lock_key(refund_id) and the Celery worker later calls reconcile_stripe_refund(), which acquires the same lock key again. If the task starts executing before the webhook releases the lock (possible on fast/low-latency queues), the worker will hit RefundReconcileInProgressError and autoretry, adding avoidable latency and retry noise.

        lock_key = refund_reconcile_lock_key(stripe_refund['id'])
        if not acquire_task_lock(lock_key, REFUND_RECONCILE_LOCK_EXPIRE):
            logger.warning(
                '[Stripe webhooks] refund %s is already reconciling; returning retryable failure',
                stripe_refund['id'],
            )
            raise StripeWebhookDispatchAPIError

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

commerce_coordinator/apps/commercetools/tasks.py:152

• The refund_from_stripe_task docstring still says it only creates a CT refund transaction, but the task now delegates to reconcile_stripe_refund (which can also update Return state and trigger LMS/Segment side effects on confirmed success). Updating the docstring will help avoid misuse/incorrect assumptions by future callers.

    """
    Celery task for handling a refund registered in the Stripe dashboard.
    Creates a refund payment transaction record via the Commercetools API.

    Args:

commerce_coordinator/apps/commercetools/stripe_refund_reconcile.py:391

• _emit_segment_refund can return without emitting anything when products cannot be resolved (properties["products"] is empty), but reconcile_stripe_refund will still mark the Segment side effect as completed. That can permanently suppress the "Order Refunded" event for that refund. Consider failing fast (or otherwise signaling failure) when no products can be built so the caller doesn’t mark completion incorrectly.

    if properties["products"]:
        properties["title"] = ", ".join(
            item.name["en-US"] for item in selected_line_items
        )

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Suppressed comments (2)

commerce_coordinator/apps/stripe/tests/test_views.py:519

• This test asserts an implementation detail via inspect.getsource(...), which is brittle (whitespace/refactors can break it) and doesn't actually validate runtime behavior. Prefer patching acquire_task_lock during the request and asserting it was not called.

        self.mock_stripe_event.id = "evt_unlocked"
        self.mock_stripe_event.type = StripeEventType.REFUND_UPDATED.value
        self.mock_stripe_event.data.object = refund
        mock_construct_event.return_value = self.mock_stripe_event

commerce_coordinator/apps/commercetools/tasks.py:152

• refund_from_stripe_task now delegates to reconcile_stripe_refund (which can update return state and trigger side effects), but the docstring still says it only creates a refund transaction and its Args list no longer matches the function signature.

    """
    Celery task for handling a refund registered in the Stripe dashboard.
    Creates a refund payment transaction record via the Commercetools API.

    Args:

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

commerce_coordinator/apps/stripe/clients.py:313

• logger.exception(...) is used here without an active exception, which will log an unnecessary stack trace (often NoneType: None). Use logger.error/warning instead so unsuccessful-but-expected statuses (e.g., failed, canceled) don’t look like unhandled exceptions in logs.

            logger.exception('Refund for order [%s] was unsuccessful', order_uuid)

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

commerce_coordinator/apps/commercetools/tasks.py:139

• reconcile_stripe_refund() calls CommercetoolsAPIClient.update_return_payment_state_after_successful_refund(), which wraps CommercetoolsError into openedx_filters.exceptions.OpenEdxFilterException (see commerce_coordinator/apps/commercetools/clients.py:866-869). Because this task’s autoretry_for does not include OpenEdxFilterException, transient CT write failures during return updates will not be retried, and Stripe will not retry because the webhook already acked after enqueueing the task.

    autoretry_for=(
        CommercetoolsError,
        RefundReconcileInProgressError,
        RefundSideEffectDispatchError,
    ),

commerce_coordinator/apps/stripe/views.py:202

• refund.updated / refund.failed events assume event_object.payment_intent is present. If it is missing/None, get_payment_by_key(payment_intent_id) will raise (it calls re.sub on the key), causing a 5xx and repeated Stripe retries. Since the reconciler already supports refund payloads that omit payment_intent, the webhook handler should validate it before calling CommerceTools and fail fast with a 400 (or explicitly skip) instead of raising a TypeError.

            stripe_refund = dict(event_object)
            payment_intent_id = event_object.payment_intent
            client = CommercetoolsAPIClient()
            try:
                payment = client.get_payment_by_key(payment_intent_id)

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.