From 95114b1d67b64f2c1e4fe523b80289429eae37fb Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Mon, 16 Feb 2026 16:37:14 +0100
Subject: [PATCH] Fix race condition between concurrent Event and Mention
 notifier jobs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When concurrent NotifyRecipientsJobs process an Event and a Mention for
the same user+card, Rails' dirty tracking can skip writing source_type
in the UPDATE if it hasn't changed from the stale in-memory value,
leaving source_type and source_id mismatched (e.g. source_type='Event'
with a Mention's source_id, resulting in a nil source).

Force source_type to always be included in the UPDATE via
source_type_will_change! to prevent this.

Here's a sample timeline of this race condition happening in the real
world (with simplified IDs):

Two `NotifyRecipientsJob` involving notification `03fklpu`, same card,
same user, same comment — enqueued within 50ms of each other:

1. Both jobs load notification `03fklpu` — it has source_type='Mention'
   (from a previous job)
2. `EventNotifier` writes at 21:22:36.051: ```sql SET
   source_type='Event', source_id=<event_id>, unread_count=1 ```
3. `source_type` included because it changed ('Mention' → 'Event')
4. `MentionNotifier` writes at 21:22:36.057 (~6ms later): ```sql SET
   source_id=<mention_id>, unread_count=1 ```
5. No `source_type`! It was `'Mention'` when loaded and `'Mention'` is
   what it's setting → not dirty → skipped
6. Final DB state: `source_type='Event'` (from step 2, untouched),
   `source_id=<mention_id>` (from step 3)

`Notification.source` now does `Event.find(<mention_id>) → nil.`

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 app/models/notifier.rb                        |  6 +++
 test/models/notifier/mention_notifier_test.rb | 43 +++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/app/models/notifier.rb b/app/models/notifier.rb
index f00987ab8..f6f356fa4 100644
--- a/app/models/notifier.rb
+++ b/app/models/notifier.rb
@@ -23,6 +23,12 @@ class Notifier
         end
 
         unless notification.previously_new_record?
+          # Always include source_type in the update to prevent a race condition between
+          # concurrent Event and Mention notifier jobs: without this, Rails' dirty tracking
+          # may skip source_type when it hasn't changed from the stale in-memory value,
+          # even though another job has since modified it in the database, leaving
+          # source_type and source_id mismatched.
+          notification.source_type_will_change!
           notification.update!(source: source, creator: creator, read_at: nil, unread_count: notification.unread_count + 1)
         end
 
diff --git a/test/models/notifier/mention_notifier_test.rb b/test/models/notifier/mention_notifier_test.rb
index 833f55ec8..2d0b274c0 100644
--- a/test/models/notifier/mention_notifier_test.rb
+++ b/test/models/notifier/mention_notifier_test.rb
@@ -24,4 +24,47 @@ class Notifier::EventNotifierTest < ActiveSupport::TestCase
       Notifier.for(events(:layout_commented)).notify
     end
   end
+
+  test "updates source_type correctly even when a concurrent job modifies it between load and save" do
+    # Start with a notification sourced from a Mention for kevin on the layout card
+    notifications(:layout_commented_kevin).destroy
+    mention = users(:kevin).mentioned_by(users(:david), at: comments(:layout_overflowing_david))
+    notification = Notification.create!(
+      user: users(:kevin), card: cards(:layout),
+      source: mention, creator: users(:david), unread_count: 1
+    )
+
+    # Override create_or_find_by to simulate a concurrent EventNotifier updating
+    # source_type in the database after the notification is loaded but before
+    # the MentionNotifier's update! runs — reproducing the race condition where
+    # Rails' dirty tracking skips source_type because it hasn't changed from
+    # the stale in-memory value.
+    Notification.class_eval do
+      class << self
+        alias_method :original_create_or_find_by, :create_or_find_by
+
+        def create_or_find_by(...)
+          original_create_or_find_by(...).tap do |record|
+            unless record.previously_new_record?
+              where(id: record.id).update_all(source_type: "Event")
+            end
+          end
+        end
+      end
+    end
+
+    new_mention = users(:kevin).mentioned_by(users(:jz), at: comments(:layout_overflowing_david))
+    Notifier.for(new_mention).notify
+
+    notification.reload
+    assert_equal "Mention", notification.source_type
+    assert_equal new_mention, notification.source
+  ensure
+    Notification.class_eval do
+      class << self
+        alias_method :create_or_find_by, :original_create_or_find_by
+        remove_method :original_create_or_find_by
+      end
+    end
+  end
 end