Make snap_diff reporter notifications safer

Author: pftg

lib/capybara_screenshot_diff/reporters/html.rb

@@ -18,30 +18,44 @@ def initialize(output_path: "tmp/snap_diff/index.html", embed_images: false)
         @failures = []
         @total = 0
         @finalized = false
+        @mutex = Mutex.new
       end
 
       def record(assertions)
+        return if @finalized
+
+        failures = []
+        total = 0
+
         assertions.each do |assertion|
           compare = assertion.compare
           next unless compare
 
-          @total += 1
+          total += 1
           next unless compare.difference&.different?
 
-          @failures << failure_entry_for(assertion.name, compare)
+          failures << failure_entry_for(assertion.name, compare)
         rescue => e
           warn "[snap_diff] Reporter skipped '#{assertion.name}': #{e.message}" if ENV["DEBUG"]
         end
+
+        @mutex.synchronize do
+          return if @finalized
+          @total += total
+          @failures.concat(failures)
+        end
       end
 
       def finalize
-        return if @finalized
+        @mutex.synchronize do
+          return if @finalized
 
-        @finalized = true
-        return if failures.empty?
+          @finalized = true
+          return if failures.empty?
 
-        write_report
-        output_path
+          write_report
+          output_path
+        end
       end
 
       def passed = total - failures.size

lib/capybara_screenshot_diff/screenshot_assertion.rb

@@ -133,15 +133,22 @@ def reporters
       @reporters ||= []
     end
 
+    def reporters_mutex
+      @reporters_mutex ||= Mutex.new
+    end
+
     def_delegator :registry, :screenshot_namer
     def_delegator :registry, :verify
 
     private
 
     def notify_reporters(assertions)
-      return if reporters.empty? || assertions.nil? || assertions.empty?
+      return if assertions.nil? || assertions.empty?
+
+      reporters_snapshot = reporters_mutex.synchronize { reporters.dup }
+      return if reporters_snapshot.empty?
 
-      reporters.each do |reporter|
+      reporters_snapshot.each do |reporter|
         reporter.record(assertions)
       rescue => e
         warn "[capybara-screenshot-diff] Reporter failed: #{e.message}"

test/unit/reporters/html_reporter_test.rb

@@ -98,6 +98,32 @@ class HTMLReporterTest < ActiveSupport::TestCase
         assert_includes html, "data:image/png;base64,"
       end
 
+      test "#record and #finalize synchronize internal state" do
+        reporter = HTML.new(output_path: @output_path)
+
+        fake_mutex = Class.new do
+          attr_reader :synchronize_calls
+
+          def initialize
+            @synchronize_calls = 0
+          end
+
+          def synchronize
+            @synchronize_calls += 1
+            yield
+          end
+        end.new
+
+        # Using private state and a fake mutex here is intentional.
+        # This test guards a tricky synchronization detail; avoid refactors unless behavior changes.
+        reporter.instance_variable_set(:@mutex, fake_mutex)
+
+        reporter.record([build_passing_assertion("sync")])
+        reporter.finalize
+
+        assert_operator fake_mutex.synchronize_calls, :>=, 1
+      end
+
       private
 
       def build_passing_assertion(name)

test/unit/reporters_mutex_test.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+module CapybaraScreenshotDiff
+  class ReportersMutexTest < ActiveSupport::TestCase
+    setup do
+      @original_reporters = CapybaraScreenshotDiff.reporters.dup
+      CapybaraScreenshotDiff.reporters.clear
+    end
+
+    teardown do
+      CapybaraScreenshotDiff.reporters.clear
+      CapybaraScreenshotDiff.reporters.concat(@original_reporters)
+      CapybaraScreenshotDiff.instance_variable_set(:@reporters_mutex, nil)
+    end
+
+    test "reporters notification uses mutex snapshot" do
+      fake_mutex = Class.new do
+        attr_reader :synchronize_calls
+
+        def initialize
+          @synchronize_calls = 0
+        end
+
+        def synchronize
+          @synchronize_calls += 1
+          yield
+        end
+      end.new
+
+      CapybaraScreenshotDiff.instance_variable_set(:@reporters_mutex, fake_mutex)
+
+      reporter = Class.new do
+        def record(_assertions); end
+      end.new
+
+      CapybaraScreenshotDiff.reporters << reporter
+
+      assertion = CapybaraScreenshotDiff::ScreenshotAssertion.new("sample")
+      assertion.compare = Object.new
+      CapybaraScreenshotDiff.add_assertion(assertion)
+
+      CapybaraScreenshotDiff.reset
+
+      assert_operator fake_mutex.synchronize_calls, :>=, 1
+    end
+  end
+end