From 461f4345ff0bc77b012bdfa0f51f4cbc8fa07424 Mon Sep 17 00:00:00 2001 From: "MD. MOHIBUR RAHMAN" <35300157+mrpmohiburrahman@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:13:36 +0600 Subject: [PATCH 1/3] fix: do not reset channel unread count on thread read A `message.read` carrying `event.thread` is a thread read. It now early-outs of the channel-scoped read branch in `Channel._handleChannelEvent` instead of advancing the channel read cursor, clearing `unread_messages` and zeroing `unreadCount`. The offline DB gets the matching carve-out so the wrong 0 is not persisted and re-hydrated. Fixes #1676 --- src/channel.ts | 10 ++++ src/offline-support/offline_support_api.ts | 8 +++ test/unit/channel.test.js | 51 +++++++++++++++++++ .../offline_support_api.test.ts | 37 ++++++++++++++ 4 files changed, 106 insertions(+) diff --git a/src/channel.ts b/src/channel.ts index 497febdd4d..6cf4528cf3 100644 --- a/src/channel.ts +++ b/src/channel.ts @@ -2007,6 +2007,16 @@ export class Channel { // delivery-report network sync below is skipped for it. case 'message.read_locally': case 'message.read': + // A thread read (`markRead({ thread_id })`) echoes back an event carrying + // `event.thread`. It concerns that thread alone — `Thread.subscribeRepliesRead` + // applies it — so none of the channel-scoped read state below may move, for any + // user: a reply being read says nothing about the channel messages around it. + // This also skips the delivery sync below; that only delays the next delivery + // report to the following `message.new` / `message.delivered` / channel query, + // which supersede it anyway (the report is a latest-delivered high-water mark). + // `markReadLocally()` never sets `thread`, so the shared placement above the + // `message.read_locally` label is inert for that event, not a behaviour change. + if (event.thread) break; if (event.user?.id && event.created_at) { const previousReadState = channelState.read[event.user.id]; channelState.read[event.user.id] = { diff --git a/src/offline-support/offline_support_api.ts b/src/offline-support/offline_support_api.ts index fdebd645eb..e613c8cfb1 100644 --- a/src/offline-support/offline_support_api.ts +++ b/src/offline-support/offline_support_api.ts @@ -1050,6 +1050,14 @@ export abstract class AbstractOfflineDB implements OfflineDBApi { } if (type === 'message.read' || type === 'notification.mark_read') { + // Mirrors the thread-read carve-out in `Channel._handleChannelEvent`, and like it + // covers `message.read` only — the event `markRead({ thread_id })` echoes back. + // `handleRead` is channel-scoped (cid-keyed), so persisting a thread read would store + // unread_messages: 0 for the whole channel and re-hydrate that 0 on the next restart. + // `notification.mark_read` is deliberately left alone: its in-memory counterpart in + // `StreamChat._handleClientEvent` does not check `thread`, and guarding only this layer + // would let the DB and the in-memory state disagree across a restart. + if (type === 'message.read' && event.thread) return []; return this.handleRead({ event, unreadMessages: 0, execute }); } diff --git a/test/unit/channel.test.js b/test/unit/channel.test.js index 35fb1e6f0e..2f0ac6d2a9 100644 --- a/test/unit/channel.test.js +++ b/test/unit/channel.test.js @@ -991,6 +991,57 @@ describe('Channel _handleChannelEvent', function () { initialReadState.last_delivered_message_id, ); }); + // regression #1676: `markRead({ thread_id })` echoes a `message.read` carrying + // `event.thread`. It concerns the thread only — `Thread.subscribeRepliesRead` picks + // it up off the client event bus — so no channel read state may move. + it('should not touch channel read state for a thread read', () => { + channel.state.unreadCount = initialCountUnread; + channel.state.read[user.id] = initialReadState; + const onMessageRead = vi.spyOn(channel.messageReceiptsTracker, 'onMessageRead'); + const event = { + ...messageReadEvent, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(channel.state.unreadCount).toBe(initialCountUnread); + expect(new Date(channel.state.read[user.id].last_read).getTime()).toBe( + new Date(initialReadState.last_read).getTime(), + ); + expect(channel.state.read[user.id].last_read_message_id).toBe( + initialReadState.last_read_message_id, + ); + expect(channel.state.read[user.id].unread_messages).toBe(initialCountUnread); + expect(onMessageRead).not.toHaveBeenCalled(); + }); + + // The guard is not scoped to the connected user: another user reading a thread reply + // says nothing about the channel messages around it either, so their channel read + // marker (what "seen by" indicators render from) must not advance. + it('should not touch channel read state for another user’s thread read', () => { + const anotherUser = { id: 'another-user' }; + channel.state.unreadCount = initialCountUnread; + channel.state.read[anotherUser.id] = initialReadState; + const onMessageRead = vi.spyOn(channel.messageReceiptsTracker, 'onMessageRead'); + const event = { + ...messageReadEvent, + user: anotherUser, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(channel.state.unreadCount).toBe(initialCountUnread); + expect(new Date(channel.state.read[anotherUser.id].last_read).getTime()).toBe( + new Date(initialReadState.last_read).getTime(), + ); + expect(channel.state.read[anotherUser.id].last_read_message_id).toBe( + initialReadState.last_read_message_id, + ); + expect(channel.state.read[anotherUser.id].unread_messages).toBe(initialCountUnread); + expect(onMessageRead).not.toHaveBeenCalled(); + }); }); describe('message.delivered', () => { diff --git a/test/unit/offline-support/offline_support_api.test.ts b/test/unit/offline-support/offline_support_api.test.ts index da553c5037..2854345f4f 100644 --- a/test/unit/offline-support/offline_support_api.test.ts +++ b/test/unit/offline-support/offline_support_api.test.ts @@ -1915,6 +1915,43 @@ describe('OfflineSupportApi', () => { expect(result).toEqual([]); }); }); + + // regression #1676: a thread read echoes `event.thread`. `handleRead` is cid-keyed, + // so persisting one would store unread_messages: 0 for the whole channel. + describe('thread reads', () => { + const threadEvent = (type: string) => + ({ + ...dummyEvent, + type, + thread: { parent_message_id: 'parent-message-id' }, + }) as unknown as Event; + + it('is a no-op for message.read carrying a thread', async () => { + const event = threadEvent('message.read'); + + const result = await offlineDb.handleEvent({ event }); + + expect(offlineDb.handleRead).not.toHaveBeenCalled(); + expect(result).toEqual([]); + }); + + // The carve-out covers `message.read` only, mirroring `Channel._handleChannelEvent`. + // `notification.mark_read` has no `thread` check in its in-memory counterpart + // (`StreamChat._handleClientEvent`), so guarding it here alone would let the DB and + // the in-memory state disagree across a restart. + it('still persists notification.mark_read carrying a thread', async () => { + const event = threadEvent('notification.mark_read'); + + const result = await offlineDb.handleEvent({ event }); + + expect(offlineDb.handleRead).toHaveBeenCalledWith({ + event, + unreadMessages: 0, + execute: true, + }); + expect(result).toEqual(['read']); + }); + }); }); }); From 2401b753f92e28447d33409cb6136320ca51c557 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Mon, 24 Aug 2026 14:02:09 +0200 Subject: [PATCH 2/3] fix: include notification.mark_read as well --- src/offline-support/offline_support_api.ts | 12 ++---- test/unit/channel.test.js | 39 +++++++++++++++++++ .../offline_support_api.test.ts | 16 +++----- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/offline-support/offline_support_api.ts b/src/offline-support/offline_support_api.ts index e613c8cfb1..f87369c07c 100644 --- a/src/offline-support/offline_support_api.ts +++ b/src/offline-support/offline_support_api.ts @@ -1050,14 +1050,10 @@ export abstract class AbstractOfflineDB implements OfflineDBApi { } if (type === 'message.read' || type === 'notification.mark_read') { - // Mirrors the thread-read carve-out in `Channel._handleChannelEvent`, and like it - // covers `message.read` only — the event `markRead({ thread_id })` echoes back. - // `handleRead` is channel-scoped (cid-keyed), so persisting a thread read would store - // unread_messages: 0 for the whole channel and re-hydrate that 0 on the next restart. - // `notification.mark_read` is deliberately left alone: its in-memory counterpart in - // `StreamChat._handleClientEvent` does not check `thread`, and guarding only this layer - // would let the DB and the in-memory state disagree across a restart. - if (type === 'message.read' && event.thread) return []; + // We make sure not to update channel reads (which is what's stored in + // the offline DB in any case) whenever we receive a read event for a + // a thread specifically. + if (event.thread) return []; return this.handleRead({ event, unreadMessages: 0, execute }); } diff --git a/test/unit/channel.test.js b/test/unit/channel.test.js index 2f0ac6d2a9..c7bc1b4728 100644 --- a/test/unit/channel.test.js +++ b/test/unit/channel.test.js @@ -1042,6 +1042,45 @@ describe('Channel _handleChannelEvent', function () { expect(channel.state.read[anotherUser.id].unread_messages).toBe(initialCountUnread); expect(onMessageRead).not.toHaveBeenCalled(); }); + + // Skipping the case also skips the delivery sync at its tail. Pinned because it is a + // deliberate behaviour change, not an oversight: the next `message.new` / + // `message.delivered` / channel query supersedes the report anyway. + it('should not sync delivery report candidates on a thread read', () => { + const syncDeliveredCandidates = vi.spyOn(client, 'syncDeliveredCandidates'); + const event = { + ...messageReadEvent, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(syncDeliveredCandidates).not.toHaveBeenCalled(); + }); + + // The stored fields above are only half of it: `countUnread()` re-derives the count from + // `read[user].last_read`, so an advanced cursor reproduces the reported 0 even with + // `unreadCount` guarded. This pins the value the integrator actually reads. + it('should keep lastRead() and countUnread() consistent after a thread read', () => { + // An unread channel message sent after the current user's last channel read but before + // the thread read - it must keep counting as unread once the thread is read. + channel.state.addMessagesSorted([ + generateMsg({ date: new Date(1800).toISOString(), user: otherUser }), + ]); + channel.state.read[user.id] = { + ...initialReadState, + last_read: new Date(initialReadState.last_read), + }; + const event = { + ...messageReadEvent, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(channel.lastRead().getTime()).toBe(1500); + expect(channel.countUnread(channel.lastRead())).toBe(1); + }); }); describe('message.delivered', () => { diff --git a/test/unit/offline-support/offline_support_api.test.ts b/test/unit/offline-support/offline_support_api.test.ts index 2854345f4f..f07e8faba4 100644 --- a/test/unit/offline-support/offline_support_api.test.ts +++ b/test/unit/offline-support/offline_support_api.test.ts @@ -1935,21 +1935,15 @@ describe('OfflineSupportApi', () => { expect(result).toEqual([]); }); - // The carve-out covers `message.read` only, mirroring `Channel._handleChannelEvent`. - // `notification.mark_read` has no `thread` check in its in-memory counterpart - // (`StreamChat._handleClientEvent`), so guarding it here alone would let the DB and - // the in-memory state disagree across a restart. - it('still persists notification.mark_read carrying a thread', async () => { + // `handleRead` writes `unread_messages: 0` for this event type too, so a + // thread-scoped `notification.mark_read` would zero the whole channel just the same. + it('is a no-op for notification.mark_read carrying a thread', async () => { const event = threadEvent('notification.mark_read'); const result = await offlineDb.handleEvent({ event }); - expect(offlineDb.handleRead).toHaveBeenCalledWith({ - event, - unreadMessages: 0, - execute: true, - }); - expect(result).toEqual(['read']); + expect(offlineDb.handleRead).not.toHaveBeenCalled(); + expect(result).toEqual([]); }); }); }); From 3a800d9da94d7b41b0ad1077f4b73fed26e2875e Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Mon, 24 Aug 2026 14:16:37 +0200 Subject: [PATCH 3/3] fix: docs and comments --- src/channel.ts | 12 +++--------- test/unit/channel.test.js | 7 +------ .../unit/offline-support/offline_support_api.test.ts | 2 -- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/channel.ts b/src/channel.ts index 6cf4528cf3..c8fb3a589a 100644 --- a/src/channel.ts +++ b/src/channel.ts @@ -2007,15 +2007,9 @@ export class Channel { // delivery-report network sync below is skipped for it. case 'message.read_locally': case 'message.read': - // A thread read (`markRead({ thread_id })`) echoes back an event carrying - // `event.thread`. It concerns that thread alone — `Thread.subscribeRepliesRead` - // applies it — so none of the channel-scoped read state below may move, for any - // user: a reply being read says nothing about the channel messages around it. - // This also skips the delivery sync below; that only delays the next delivery - // report to the following `message.new` / `message.delivered` / channel query, - // which supersede it anyway (the report is a latest-delivered high-water mark). - // `markReadLocally()` never sets `thread`, so the shared placement above the - // `message.read_locally` label is inert for that event, not a behaviour change. + // Thread read events are handled indiscriminately within the reactive `thread` object, we can + // skip them here in order to ensure the channel does not get read incidentally when it should + // not. if (event.thread) break; if (event.user?.id && event.created_at) { const previousReadState = channelState.read[event.user.id]; diff --git a/test/unit/channel.test.js b/test/unit/channel.test.js index c7bc1b4728..4e2da5a3db 100644 --- a/test/unit/channel.test.js +++ b/test/unit/channel.test.js @@ -991,9 +991,7 @@ describe('Channel _handleChannelEvent', function () { initialReadState.last_delivered_message_id, ); }); - // regression #1676: `markRead({ thread_id })` echoes a `message.read` carrying - // `event.thread`. It concerns the thread only — `Thread.subscribeRepliesRead` picks - // it up off the client event bus — so no channel read state may move. + // Tests against this issue: https://github.com/GetStream/stream-chat-js/issues/1676 it('should not touch channel read state for a thread read', () => { channel.state.unreadCount = initialCountUnread; channel.state.read[user.id] = initialReadState; @@ -1016,9 +1014,6 @@ describe('Channel _handleChannelEvent', function () { expect(onMessageRead).not.toHaveBeenCalled(); }); - // The guard is not scoped to the connected user: another user reading a thread reply - // says nothing about the channel messages around it either, so their channel read - // marker (what "seen by" indicators render from) must not advance. it('should not touch channel read state for another user’s thread read', () => { const anotherUser = { id: 'another-user' }; channel.state.unreadCount = initialCountUnread; diff --git a/test/unit/offline-support/offline_support_api.test.ts b/test/unit/offline-support/offline_support_api.test.ts index f07e8faba4..f2ca3cb2d1 100644 --- a/test/unit/offline-support/offline_support_api.test.ts +++ b/test/unit/offline-support/offline_support_api.test.ts @@ -1916,8 +1916,6 @@ describe('OfflineSupportApi', () => { }); }); - // regression #1676: a thread read echoes `event.thread`. `handleRead` is cid-keyed, - // so persisting one would store unread_messages: 0 for the whole channel. describe('thread reads', () => { const threadEvent = (type: string) => ({