fix(transfers): prevent aborted transaction during concurrent auto-matching

app/models/family/auto_transfer_matchable.rb

@@ -43,14 +43,7 @@ def auto_match_transfers!(account: nil)
         next if used_transaction_ids.include?(match.inflow_transaction_id) ||
                used_transaction_ids.include?(match.outflow_transaction_id)
 
-        begin
-          Transfer.find_or_create_by!(
-            inflow_transaction_id: match.inflow_transaction_id,
-            outflow_transaction_id: match.outflow_transaction_id,
-          )
-        rescue ActiveRecord::RecordNotUnique
-          # Another concurrent job created the transfer; safe to ignore
-        end
+        find_or_create_transfer!(match)
 
         inflow_transaction = transactions_by_id.fetch(match.inflow_transaction_id)
         outflow_transaction = transactions_by_id.fetch(match.outflow_transaction_id)
@@ -80,6 +73,22 @@ def auto_match_transfers!(account: nil)
   end
 
   private
+    # Isolate the insert in a savepoint so losing the unique-index race to a
+    # concurrent job rolls back only this statement, not the surrounding transaction.
+    def find_or_create_transfer!(match)
+      Transfer.transaction(requires_new: true) do
+        Transfer.find_or_create_by!(
+          inflow_transaction_id: match.inflow_transaction_id,
+          outflow_transaction_id: match.outflow_transaction_id,
+        )
+      end
+    rescue ActiveRecord::RecordNotUnique
+      # Lost the insert race; savepoint rolled back, transaction intact.
+    rescue ActiveRecord::RecordInvalid => e
+      # Same race caught by the uniqueness validation; re-raise anything else.
+      raise unless %i[inflow_transaction_id outflow_transaction_id].any? { |attr| e.record.errors.of_kind?(attr, :taken) }
+    end
+
     def coerce_transfer_match_date_window!(value)
       Integer(value)
     rescue ArgumentError, TypeError

test/models/family/auto_transfer_matchable_test.rb

@@ -19,6 +19,73 @@ class Family::AutoTransferMatchableTest < ActiveSupport::TestCase
     end
   end
 
+  test "concurrent unique-index race does not poison the surrounding transaction" do
+    outflow_entry = create_transaction(date: 1.day.ago.to_date, account: @depository, amount: 500)
+    inflow_entry = create_transaction(date: Date.current, account: @credit_card, amount: -500)
+    inflow_id = inflow_entry.entryable_id
+
+    # An existing transfer we can collide with to trigger a real unique violation,
+    # mimicking a concurrent job that won the race. A genuine failing INSERT (not a
+    # synthetic raise) is what aborts the transaction, so only this reproduces the bug.
+    rival_out = create_transaction(date: 1.day.ago.to_date, account: @depository, amount: 250)
+    rival_in = create_transaction(date: Date.current, account: @credit_card, amount: -250)
+    rival = Transfer.create!(inflow_transaction_id: rival_in.entryable_id, outflow_transaction_id: rival_out.entryable_id)
+
+    original = Transfer.method(:find_or_create_by!)
+    Transfer.singleton_class.send(:define_method, :find_or_create_by!) do |attributes|
+      if attributes[:inflow_transaction_id] == inflow_id
+        insert!(inflow_transaction_id: rival.inflow_transaction_id, outflow_transaction_id: rival.outflow_transaction_id)
+      else
+        original.call(attributes)
+      end
+    end
+
+    begin
+      assert_nothing_raised { @family.auto_match_transfers! }
+    ensure
+      Transfer.singleton_class.send(:remove_method, :find_or_create_by!)
+    end
+
+    inflow_entry.reload
+    outflow_entry.reload
+
+    # Surrounding transaction stayed healthy, so kinds were still written.
+    assert_equal "funds_movement", inflow_entry.entryable.kind
+    assert_equal "cc_payment", outflow_entry.entryable.kind
+  end
+
+  test "validation-path race during matching does not abort the run" do
+    outflow_entry = create_transaction(date: 1.day.ago.to_date, account: @depository, amount: 500)
+    inflow_entry = create_transaction(date: Date.current, account: @credit_card, amount: -500)
+
+    # A concurrent job already matched one of these transactions, so the uniqueness
+    # validation rejects the insert with :taken (a plain raise, no aborted SQL).
+    invalid = Transfer.new(inflow_transaction_id: inflow_entry.entryable_id, outflow_transaction_id: outflow_entry.entryable_id)
+    invalid.errors.add(:inflow_transaction_id, :taken)
+    Transfer.stubs(:find_or_create_by!).raises(ActiveRecord::RecordInvalid.new(invalid))
+
+    assert_nothing_raised { @family.auto_match_transfers! }
+
+    inflow_entry.reload
+    outflow_entry.reload
+
+    # The duplicate was swallowed, so the loop kept running and wrote kinds.
+    assert_equal "funds_movement", inflow_entry.entryable.kind
+    assert_equal "cc_payment", outflow_entry.entryable.kind
+  end
+
+  test "non-uniqueness validation failure during matching is not swallowed" do
+    create_transaction(date: 1.day.ago.to_date, account: @depository, amount: 500)
+    create_transaction(date: Date.current, account: @credit_card, amount: -500)
+
+    # A genuine (non-race) validation failure must surface, not be skipped silently.
+    invalid = Transfer.new
+    invalid.errors.add(:base, :different_accounts)
+    Transfer.stubs(:find_or_create_by!).raises(ActiveRecord::RecordInvalid.new(invalid))
+
+    assert_raises(ActiveRecord::RecordInvalid) { @family.auto_match_transfers! }
+  end
+
   test "auto-matches multi-currency transfers" do
     load_exchange_prices
     create_transaction(date: 1.day.ago.to_date, account: @depository, amount: 500)