From f70125e44c1e7407e86fc10cdc2a9849664bfa27 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 19 Aug 2026 15:58:42 +0200 Subject: [PATCH] fix(chat): treat a newest known message id of 0 as unknown isPlausibleLastReadMessageId rejects a read position more than 10,000 above the conversation's cached newest known message id, guarding against a hash-derived placeholder id being mistaken for a real read position. A federated conversation's cached lastMessage can carry an id of 0 when it was never populated - a real server-assigned message id is never 0 or negative, so that can only mean the newest known id isn't actually known, not that message ids near 0 are the real ceiling. Without this, every real read position in such a room gets rejected as implausible forever: localLastReadMessage never advances past 0, updateRemoteLastReadMessageIfNeeded never finds a reason to send a marker, and the conversation never gets marked as read after leaving the chat. Treat a newest known id of 0 (or negative) the same as null, and name the resulting reference point explicitly so the guard's intent - is there a trustworthy ceiling to judge the candidate against at all - is readable from the code rather than from the comparison operator. Assisted-by: Claude Sonnet 5 Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/chat/viewmodels/ChatViewModel.kt | 12 ++++++++++-- .../talk/chat/viewmodels/ChatViewModelTest.kt | 7 +++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 3bbcfc3716..df460c1488 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -2422,8 +2422,16 @@ class ChatViewModel @AssistedInject constructor( private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 10_000L - fun isPlausibleLastReadMessageId(messageId: Int, newestKnownRealMessageId: Long?): Boolean = - newestKnownRealMessageId == null || messageId <= newestKnownRealMessageId + PLAUSIBLE_MESSAGE_ID_BUFFER + /** + * A real server-assigned message id is always positive, so a null, zero or negative + * [newestKnownRealMessageId] is never a trustworthy ceiling to judge [messageId] against - + * e.g. a federated conversation's cached lastMessage can carry an id of 0 when it was + * never populated. Without a trustworthy ceiling, [messageId] is accepted unchecked here. + */ + fun isPlausibleLastReadMessageId(messageId: Int, newestKnownRealMessageId: Long?): Boolean { + val trustworthyCeiling = newestKnownRealMessageId?.takeIf { it > 0 } ?: return true + return messageId <= trustworthyCeiling + PLAUSIBLE_MESSAGE_ID_BUFFER + } } sealed class OutOfOfficeUIState { diff --git a/app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt b/app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt index 84c649eba3..12f124eefb 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt @@ -61,6 +61,13 @@ class ChatViewModelTest { ) } + @Test + fun `isPlausibleLastReadMessageId treats a newest known id of 0 as unknown`() { + // A federated conversation's cached lastMessage can carry an id of 0 - a real message id + // is never 0, so this must not be treated as a real ceiling near the start of the room. + assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 136556, newestKnownRealMessageId = 0L)) + } + // The unread marker latch: the marker position must only be derived from the visible window // when the window provably reaches back to the unread boundary — otherwise a window of // only-unread messages (e.g. after a capped fetch of the newest messages) would place the