fix(stripe): skip refunds on disputed and already-refunded charges

app/jobs/credit_notes/refunds/stripe_create_job.rb

@@ -5,6 +5,13 @@ module Refunds
     class StripeCreateJob < ApplicationJob
       queue_as "providers"
 
+      # NOTE: safe to retry because stripe deduplicates on the idempotency key that
+      #       Stripe::Refund.create sends (the credit note id), so a replayed request returns
+      #       the original refund instead of issuing a second one. We enforce nothing on our
+      #       side, and stripe only honours that key for 24h; the backoff below tops out around
+      #       20 minutes, so raising `attempts` much further would void the guarantee.
+      retry_on(*PaymentProviders::StripeProvider::TRANSIENT_ERRORS, wait: :polynomially_longer, attempts: 6)
+
       def perform(credit_note)
         result = CreditNotes::Refunds::StripeService.new(credit_note).create
         result.raise_if_error!

app/jobs/payment_providers/stripe/handle_event_job.rb

@@ -13,8 +13,7 @@ class HandleEventJob < ApplicationJob
 
       # NOTE: Sometimes, the stripe webhook is received before the DB update of the impacted resource
       retry_on BaseService::NotFoundFailure
-      retry_on ::Stripe::RateLimitError, wait: :polynomially_longer, attempts: 6, jitter: 0.75
-      retry_on ::Stripe::APIConnectionError, wait: :polynomially_longer, attempts: 6, jitter: 0.75
+      retry_on(*PaymentProviders::StripeProvider::TRANSIENT_ERRORS, wait: :polynomially_longer, attempts: 6, jitter: 0.75)
       retry_on BaseService::LockAcquisitionFailure, ActiveRecord::Deadlocked, attempts: MAX_LOCK_RETRY_ATTEMPTS, wait: random_lock_retry_delay
 
       def perform(organization:, event:)

app/models/invoice.rb

@@ -515,6 +515,18 @@ def mark_as_dispute_lost!(timestamp = Time.current)
     save!
   end
 
+  # NOTE: mirrors stripe's `is_charge_refundable`: set while a dispute prevents a refund,
+  #       cleared as soon as the charge becomes refundable again.
+  def mark_refund_as_blocked!(timestamp = Time.current)
+    self.payment_refund_blocked_at ||= timestamp
+    save!
+  end
+
+  def mark_refund_as_unblocked!
+    self.payment_refund_blocked_at = nil
+    save!
+  end
+
   def should_sync_invoice?
     !self_billed && finalized? && customer.integration_customers.accounting_kind.any? { |c| c.integration.sync_invoices }
   end
@@ -737,6 +749,7 @@ def set_finalized_at
 #  payment_dispute_lost_at                 :datetime
 #  payment_due_date                        :date
 #  payment_overdue                         :boolean          default(FALSE)
+#  payment_refund_blocked_at               :datetime
 #  payment_status                          :integer          default("pending"), not null
 #  payment_term                            :jsonb
 #  payment_term_source                     :string

app/models/payment_providers/stripe_provider.rb

@@ -19,6 +19,8 @@ class StripeProvider < BaseProvider
       charge.refund.updated
       customer.updated
       charge.dispute.closed
+      charge.dispute.created
+      charge.dispute.updated
     ].freeze
 
     PROCESSING_STATUSES = %w[
@@ -31,6 +33,22 @@ class StripeProvider < BaseProvider
     FAILED_STATUSES = %w[canceled requires_payment_method].freeze
     SUPPORTED_EU_BANK_TRANSFER_COUNTRIES = %w[BE DE ES FR IE NL].freeze
 
+    # NOTE: retrying one of these can change the answer, so jobs talking to stripe retry them and
+    #       lookups let them propagate rather than acting on data they could not confirm.
+    TRANSIENT_ERRORS = [
+      ::Stripe::APIConnectionError,
+      ::Stripe::APIError,
+      ::Stripe::RateLimitError
+    ].freeze
+
+    # NOTE: retrying one of these never changes the answer, so a lookup that hits one falls back
+    #       to whatever it already has rather than failing the job.
+    PERMANENT_ERRORS = [
+      ::Stripe::AuthenticationError,
+      ::Stripe::InvalidRequestError,
+      ::Stripe::PermissionError
+    ].freeze
+
     validates :secret_key, presence: true
     validates :success_redirect_url, url: true, allow_nil: true, length: {maximum: 1024}
 

app/services/credit_notes/refunds/stripe_service.rb

@@ -8,6 +8,16 @@ class StripeService < BaseService
       include Customers::PaymentProviderFinder
 
       INVALID_PAYMENT_METHOD_ERROR = "charge_not_refundable"
+      CHARGE_DISPUTED_ERROR = "charge_disputed"
+      CHARGE_ALREADY_REFUNDED_ERROR = "charge_already_refunded"
+      # NOTE: lago-specific, stripe rejects an over-refund without an error code
+      INSUFFICIENT_REFUNDABLE_AMOUNT_ERROR = "insufficient_refundable_amount"
+
+      NON_RETRYABLE_ERRORS = [
+        INVALID_PAYMENT_METHOD_ERROR,
+        CHARGE_DISPUTED_ERROR,
+        CHARGE_ALREADY_REFUNDED_ERROR
+      ].freeze
 
       def initialize(credit_note = nil)
         @credit_note = credit_note
@@ -19,7 +29,16 @@ def create
         result.credit_note = credit_note
         return result unless should_process_refund?
 
-        stripe_result = create_stripe_refund
+        blocking_error_code = refund_blocked_error_code
+        # NOTE: an earlier attempt can reach stripe without us ever seeing its response, and the
+        #       refund it created is exactly what the amount check then trips on. Adopt that
+        #       refund rather than failing the credit note for money we already refunded.
+        stripe_result = blocking_error_code ? existing_stripe_refund : create_stripe_refund
+
+        if stripe_result.nil?
+          handle_refund_failure(message: refund_blocked_message(blocking_error_code), code: blocking_error_code)
+          return result
+        end
 
         refund = Refund.new(
           organization_id: credit_note.organization_id,
@@ -44,10 +63,8 @@ def create
       rescue ActiveRecord::RecordInvalid => e
         result.record_validation_failure!(record: e.record)
       rescue ::Stripe::InvalidRequestError => e
-        deliver_error_webhook(message: e.message, code: e.code)
-        update_credit_note_status(:failed)
-        Utils::ActivityLog.produce(credit_note, "credit_note.refund_failure")
-        return result if e.code == INVALID_PAYMENT_METHOD_ERROR
+        handle_refund_failure(message: e.message, code: e.code)
+        return result if NON_RETRYABLE_ERRORS.include?(e.code)
 
         result.service_failure!(code: "stripe_error", message: e.message)
       end
@@ -105,6 +122,97 @@ def payment
         end
       end
 
+      def refund_blocked_error_code
+        return CHARGE_DISPUTED_ERROR if invoice.payment_refund_blocked_at?
+
+        remaining = stripe_refundable_amount_cents
+        # NOTE: nil means stripe could not be asked. The pre-check is an optimisation, never a
+        #       gate: when in doubt we let Stripe::Refund.create decide.
+        return nil if remaining.nil?
+        return CHARGE_ALREADY_REFUNDED_ERROR if remaining <= 0
+        return INSUFFICIENT_REFUNDABLE_AMOUNT_ERROR if remaining < credit_note.refund_amount_cents
+
+        nil
+      end
+
+      def refund_blocked_message(code)
+        case code
+        when CHARGE_DISPUTED_ERROR
+          "The charge is disputed and cannot be refunded"
+        when CHARGE_ALREADY_REFUNDED_ERROR
+          "The charge has already been fully refunded"
+        when INSUFFICIENT_REFUNDABLE_AMOUNT_ERROR
+          "The charge has only #{stripe_refundable_amount_cents} cents left to refund, " \
+            "#{credit_note.refund_amount_cents} are required"
+        end
+      end
+
+      def stripe_refundable_amount_cents
+        return @stripe_refundable_amount_cents if defined?(@stripe_refundable_amount_cents)
+
+        charge = stripe_charge
+        # NOTE: bracket access, stripe objects raise NoMethodError on fields absent from the
+        #       pinned API version.
+        captured = charge && (charge[:amount_captured] || charge[:amount])
+        refunded = charge && charge[:amount_refunded]
+
+        @stripe_refundable_amount_cents = if captured.nil? || refunded.nil?
+          nil
+        else
+          captured - refunded
+        end
+      end
+
+      # NOTE: manual payments carry no provider payment id, and a stripe list filtered on nil
+      #       does not filter at all, it returns the whole account.
+      def stripe_payment_intent_id
+        payment.provider_payment_id.presence
+      end
+
+      def existing_stripe_refund
+        return @existing_stripe_refund if defined?(@existing_stripe_refund)
+        return @existing_stripe_refund = nil if stripe_payment_intent_id.nil?
+
+        refunds = ::Stripe::Refund.list(
+          {payment_intent: stripe_payment_intent_id, limit: 100},
+          {api_key: stripe_api_key}
+        )
+        @existing_stripe_refund = refunds.data.detect do |refund|
+          refund[:metadata] && refund[:metadata][:lago_credit_note_id] == credit_note.id
+        end
+      rescue *PaymentProviders::StripeProvider::PERMANENT_ERRORS => e
+        # NOTE: transient errors are left to propagate so the job retries and can still find the
+        #       refund. Failing here would mark the credit note failed for a refund stripe may
+        #       already hold.
+        Rails.logger.warn("Unable to list stripe refunds for payment #{payment.id}: #{e.message}")
+        @existing_stripe_refund = nil
+      end
+
+      def stripe_charge
+        return @stripe_charge if defined?(@stripe_charge)
+        return @stripe_charge = nil if stripe_payment_intent_id.nil?
+
+        charges = ::Stripe::Charge.list(
+          {payment_intent: stripe_payment_intent_id, limit: 10},
+          {api_key: stripe_api_key}
+        )
+        # NOTE: a payment intent can carry failed attempts alongside the successful charge.
+        @stripe_charge = charges.data.detect { |charge| charge[:status] == "succeeded" }
+      rescue ::Stripe::StripeError => e
+        # NOTE: deliberately broader than PERMANENT_ERRORS. This pre-check only saves a doomed
+        #       call, so proceeding is safe: stripe rejects a bad refund and the rescue below
+        #       handles it without raising. Retrying transient errors here would risk
+        #       dead-queueing a refund that would otherwise have gone through.
+        Rails.logger.warn("Unable to retrieve stripe charge for payment #{payment.id}: #{e.message}")
+        @stripe_charge = nil
+      end
+
+      def handle_refund_failure(message:, code:)
+        deliver_error_webhook(message:, code:)
+        update_credit_note_status(:failed)
+        Utils::ActivityLog.produce(credit_note, "credit_note.refund_failure")
+      end
+
       def stripe_api_key
         stripe_payment_provider.secret_key
       end

app/services/payment_providers/stripe/handle_event_service.rb

@@ -9,6 +9,8 @@ class HandleEventService < BaseService
         "payment_intent.payment_failed" => PaymentProviders::Stripe::Webhooks::PaymentIntentPaymentFailedService,
         "customer.updated" => PaymentProviders::Stripe::Webhooks::CustomerUpdatedService,
         "charge.dispute.closed" => PaymentProviders::Stripe::Webhooks::ChargeDisputeClosedService,
+        "charge.dispute.created" => PaymentProviders::Stripe::Webhooks::ChargeDisputeCreatedService,
+        "charge.dispute.updated" => PaymentProviders::Stripe::Webhooks::ChargeDisputeCreatedService,
         "payment_intent.canceled" => PaymentProviders::Stripe::Webhooks::PaymentIntentPaymentFailedService
       }.freeze
 

app/services/payment_providers/stripe/webhooks/charge_dispute_closed_service.rb

@@ -4,16 +4,22 @@ module PaymentProviders
   module Stripe
     module Webhooks
       class ChargeDisputeClosedService < BaseService
-        def call
-          status = event.data.object.status
-          reason = event.data.object.reason
-          provider_payment_id = event.data.object.payment_intent
+        include DisputeRefundability
 
-          payment = Payment.find_by(provider_payment_id:)
+        def call
           return result unless payment
 
-          if status == "lost"
-            return ::Payments::LoseDisputeService.call(payment:, payment_dispute_lost_at:, reason:)
+          # NOTE: unblock only once no dispute on the payment blocks refunds any more. On a lost
+          #       dispute the charge stays unrefundable, and payment_dispute_lost_at takes over
+          #       as the permanent refund block.
+          ::Payments::CloseDisputeService.call(payment:) if charge_refundable?
+
+          if event.data.object.status == "lost"
+            return ::Payments::LoseDisputeService.call(
+              payment:,
+              payment_dispute_lost_at:,
+              reason: event.data.object.reason
+            )
           end
 
           result

app/services/payment_providers/stripe/webhooks/charge_dispute_created_service.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module PaymentProviders
+  module Stripe
+    module Webhooks
+      class ChargeDisputeCreatedService < BaseService
+        include DisputeRefundability
+
+        def call
+          return result unless payment
+
+          # NOTE: `charge.dispute.created` also fires for inquiries, where stripe still accepts
+          #       refunds. `is_charge_refundable` is the only reliable signal, and it flips
+          #       through `charge.dispute.updated` when an inquiry escalates to a real dispute.
+          if charge_refundable?
+            ::Payments::CloseDisputeService.call(payment:)
+          else
+            ::Payments::OpenDisputeService.call(payment:, payment_refund_blocked_at:)
+          end
+        end
+
+        private
+
+        def payment_refund_blocked_at
+          Time.zone.at(event.created)
+        end
+      end
+    end
+  end
+end

app/services/payment_providers/stripe/webhooks/dispute_refundability.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+module PaymentProviders
+  module Stripe
+    module Webhooks
+      # NOTE: refundability belongs to the charge, not to a single dispute: a payment intent can
+      #       carry several disputes, and refunds stay blocked while any one of them blocks them.
+      #       Reading the current set also makes the transition order-safe, since stripe does not
+      #       guarantee webhook ordering and a replayed event would otherwise apply stale state.
+      module DisputeRefundability
+        private
+
+        def charge_refundable?
+          return event.data.object[:is_charge_refundable] if current_disputes.nil?
+
+          current_disputes.none? { |dispute| dispute[:is_charge_refundable] == false }
+        end
+
+        def current_disputes
+          return @current_disputes if defined?(@current_disputes)
+          return @current_disputes = nil if stripe_api_key.blank?
+
+          @current_disputes = ::Stripe::Dispute.list(
+            {payment_intent: provider_payment_id, limit: 100},
+            {api_key: stripe_api_key}
+          ).data
+        rescue *PaymentProviders::StripeProvider::PERMANENT_ERRORS => e
+          # NOTE: retrying these never changes the answer, so fall back to the payload rather
+          #       than dead-queueing the event. Transient errors are deliberately left to
+          #       propagate: HandleEventJob retries them, and acting on a possibly stale payload
+          #       is what this lookup exists to prevent.
+          Rails.logger.warn("Unable to list stripe disputes for #{provider_payment_id}: #{e.message}")
+          @current_disputes = nil
+        end
+
+        def payment
+          return @payment if defined?(@payment)
+          # NOTE: a dispute on a charge created outside a payment intent carries a null
+          #       payment_intent, and manual payments store a null provider_payment_id, so an
+          #       unguarded lookup would match an unrelated payment in the organization.
+          return @payment = nil if provider_payment_id.blank?
+
+          # NOTE: scoped to the organization, a stripe api key can be shared across several
+          #       of them, and the dispute must not reach another organization's invoices.
+          @payment = Payment.where(organization_id: organization.id).find_by(provider_payment_id:)
+        end
+
+        def provider_payment_id
+          event.data.object.payment_intent
+        end
+
+        def stripe_api_key
+          payment&.payment_provider&.secret_key
+        end
+      end
+    end
+  end
+end

app/services/payments/close_dispute_service.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Payments
+  class CloseDisputeService < BaseService
+    Result = BaseResult[:invoices, :payment]
+
+    def initialize(payment:)
+      @payment = payment
+      @payable = payment&.payable
+      super
+    end
+
+    def call
+      return result.not_found_failure!(resource: "payment") if payment.nil?
+      return result.not_found_failure!(resource: "payable") if payable.nil?
+
+      result.payment = payment
+      invoices = payment.invoices
+
+      ActiveRecord::Base.transaction do
+        invoices.each(&:mark_refund_as_unblocked!)
+      end
+
+      result.invoices = invoices
+      result
+    end
+
+    private
+
+    attr_reader :payment, :payable
+  end
+end