From f6241d3ed84c123f13f5fb7314459ea4f6effbc3 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 14 Aug 2026 15:12:32 +0200 Subject: [PATCH] Stop attributing main timeline notification counts to a thread When topping up the summarised counts with push actions that have not been rotated yet, the main timeline branch incremented `counts`, the variable left over from the loop that read the summary rows, rather than the main timeline's own counts. Whichever thread the summary query returned last would absorb the main timeline's un-rotated notifications. The room total is unaffected, and the unique index on `event_push_summary` means real thread IDs (event IDs, starting with `$`) sort before `main`, so in practice `main` is returned last and the bug is masked. The test pins it with a synthetic summary row whose thread ID sorts after `main`. --- changelog.d/20111.bugfix | 1 + .../databases/main/event_push_actions.py | 23 +++---- tests/storage/test_event_push_actions.py | 65 +++++++++++++++++++ 3 files changed, 74 insertions(+), 15 deletions(-) create mode 100644 changelog.d/20111.bugfix diff --git a/changelog.d/20111.bugfix b/changelog.d/20111.bugfix new file mode 100644 index 0000000000..13ebff2eae --- /dev/null +++ b/changelog.d/20111.bugfix @@ -0,0 +1 @@ +Fix notification counts for the main timeline sometimes being attributed to a thread. diff --git a/synapse/storage/databases/main/event_push_actions.py b/synapse/storage/databases/main/event_push_actions.py index 9c5fd35906..86af1ac449 100644 --- a/synapse/storage/databases/main/event_push_actions.py +++ b/synapse/storage/databases/main/event_push_actions.py @@ -704,21 +704,14 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas if thread_id not in summarised_threads: continue - if thread_id == MAIN_TIMELINE: - counts.notify_count += notif_count - counts.unread_count += unread_count - elif thread_id in thread_counts: - thread_counts[thread_id].notify_count += notif_count - thread_counts[thread_id].unread_count += unread_count - else: - # Previous thread summaries of 0 are discarded above. - # - # TODO If empty summaries are deleted this can be removed. - thread_counts[thread_id] = NotifCounts( - notify_count=notif_count, - unread_count=unread_count, - highlight_count=0, - ) + # Note that previous thread summaries of 0 are discarded above, so this + # may create a new entry. + # + # TODO If empty summaries are deleted this can use `thread_counts` + # directly. + counts = _get_thread(thread_id) + counts.notify_count += notif_count + counts.unread_count += unread_count # Finally we need to count push actions that aren't included in the # summary returned above. This might be due to recent events that haven't diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index bfe7049fb2..a5285ea2c5 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -472,6 +472,71 @@ class EventPushActionsStoreTestCase(HomeserverTestCase): self.get_success(self.store._rotate_notifs()) _assert_badge(1) + def _send_message( + self, + room_id: str, + tok: str, + thread_root: str | None = None, + highlight_for: str | None = None, + ) -> str: + """Send a message, optionally into a thread and/or mentioning a user.""" + content: JsonDict = { + "msgtype": "m.text", + "body": highlight_for if highlight_for is not None else "msg", + } + if thread_root is not None: + content["m.relates_to"] = { + "rel_type": RelationTypes.THREAD, + "event_id": thread_root, + } + return self.helper.send_event( + room_id, type="m.room.message", content=content, tok=tok + )["event_id"] + + def test_count_aggregation_main_timeline_top_up_stays_on_main_timeline( + self, + ) -> None: + """ + Regression test: the top up of un-rotated push actions for the main timeline + must be added to the main timeline, not to whichever thread the summary query + happened to return last. + """ + user_id, _, _, other_token, room_id = self._create_users_and_room() + + self._send_message(room_id, other_token) + self.get_success(self.store._rotate_notifs()) + + # A summary row for a thread whose ID sorts after "main", so that it, rather + # than the main timeline, is the last row the summary query returns. + self.get_success( + self.store.db_pool.simple_insert( + table="event_push_summary", + values={ + "user_id": user_id, + "room_id": room_id, + "thread_id": "zzz-thread", + "notif_count": 1, + "unread_count": 0, + "stream_ordering": 100000, + "last_receipt_stream_ordering": None, + }, + ) + ) + + # A main timeline notification which has not been rotated yet. + self._send_message(room_id, other_token) + + counts = self.get_success( + self.store.db_pool.runInteraction( + "get-unread-counts", + self.store._get_unread_counts_by_receipt_txn, + room_id, + user_id, + ) + ) + self.assertEqual(counts.main_timeline, NotifCounts(notify_count=2)) + self.assertEqual(counts.threads, {"zzz-thread": NotifCounts(notify_count=1)}) + def test_count_aggregation_threads(self) -> None: """ This is essentially the same test as test_count_aggregation, but adds