fix(app): never lower the read-state unread watermark (#2704)

fluxer_app/src/features/messaging/commands/MessageCommands.tsx

@@ -279,7 +279,6 @@ function handleMessageFetchSuccess(
 		channelId,
 		isAfter: pageState.isAfter,
 		messages,
-		tailProbeWatermarkId: tailProbe?.watermarkMessageId ?? null,
 	});
 	MessageReferences.handleMessagesFetchSuccess(channelId, messages);
 	void requestMissingGuildMembers(channelId, messages);

fluxer_app/src/features/read_state/state/ReadStates.test.ts

@@ -118,55 +118,32 @@ describe('ReadStates unread invariant', () => {
 		expect(ReadStates.hasUnread(channelId)).toBe(true);
 	});
 
-	it('still lets its own probe lower a watermark a passive update raised', () => {
+	it('never lowers the watermark when an after page comes back empty', () => {
 		const {channelId} = seedReadChannel();
 		loadedMessages.push({id: ID.ack, author: {id: 'someone'}});
 		ReadStates.handlePassiveLastMessageUpdates({[channelId]: ID.newer}, 'guild-1');
-		ReadStates.handleLoadMessages({channelId, isAfter: true, messages: [], tailProbeWatermarkId: ID.newer});
-		expect(ReadStates.lastMessageId(channelId)).toBe(ID.ack);
-		expect(ReadStates.hasUnread(channelId)).toBe(false);
+		ReadStates.handleLoadMessages({channelId, isAfter: true, messages: []});
+		expect(ReadStates.lastMessageId(channelId)).toBe(ID.newer);
+		expect(ReadStates.hasUnread(channelId)).toBe(true);
 	});
 
-	it('lowers a watermark its own probe finds nothing behind', () => {
-		const {channelId, state} = seedReadChannel();
-		state.lastMessageId = ID.newer;
-		loadedMessages.push({id: ID.ack, author: {id: 'someone'}});
-		ReadStates.handleLoadMessages({channelId, isAfter: true, messages: [], tailProbeWatermarkId: ID.newer});
-		expect(ReadStates.lastMessageId(channelId)).toBe(ID.ack);
-		expect(ReadStates.hasUnread(channelId)).toBe(false);
-	});
-
-	it('keeps a watermark that is ahead when an ordinary after page comes back empty', () => {
+	it('keeps a watermark that points at a message no longer in the channel', () => {
 		const {channelId, state} = seedReadChannel();
 		state.lastMessageId = ID.newer;
 		loadedMessages.push({id: ID.ack, author: {id: 'someone'}});
 		ReadStates.handleLoadMessages({channelId, isAfter: true, messages: []});
 		expect(ReadStates.lastMessageId(channelId)).toBe(ID.newer);
+		expect(ReadStates.hasUnread(channelId)).toBe(true);
 	});
 
-	it('keeps a watermark that advanced while its own probe was in flight', () => {
+	it('acks up to the watermark so a deleted newest message cannot keep the channel unread', () => {
 		const {channelId, state} = seedReadChannel();
 		state.lastMessageId = ID.newer;
 		loadedMessages.push({id: ID.ack, author: {id: 'someone'}});
-		ReadStates.handleLoadMessages({channelId, isAfter: true, messages: [], tailProbeWatermarkId: ID.ack});
-		expect(ReadStates.lastMessageId(channelId)).toBe(ID.newer);
-	});
-
-	it('keeps a watermark that is ahead when the page was not an after page', () => {
-		const {channelId, state} = seedReadChannel();
-		state.lastMessageId = ID.newer;
-		loadedMessages.push({id: ID.ack, author: {id: 'someone'}});
-		ReadStates.handleLoadMessages({channelId, messages: [], tailProbeWatermarkId: ID.newer});
-		expect(ReadStates.lastMessageId(channelId)).toBe(ID.newer);
-	});
-
-	it('keeps a watermark that is ahead when the window is not at the live edge', () => {
-		const {channelId, state} = seedReadChannel();
-		hasNewestMessages = false;
-		state.lastMessageId = ID.newer;
-		loadedMessages.push({id: ID.ack, author: {id: 'someone'}});
-		ReadStates.handleLoadMessages({channelId, isAfter: true, messages: [], tailProbeWatermarkId: ID.newer});
-		expect(ReadStates.lastMessageId(channelId)).toBe(ID.newer);
+		ReadStates.handleLoadMessages({channelId, isAfter: true, messages: []});
+		ReadStates.handleChannelAckWithStickyUnread({channelId});
+		expect(ReadStates.ackMessageId(channelId)).toBe(ID.newer);
+		expect(ReadStates.hasUnread(channelId)).toBe(false);
 	});
 
 	it('anchors the divider when a window is loaded whose ack sits outside it', () => {

fluxer_app/src/features/read_state/state/ReadStates.ts

@@ -439,12 +439,7 @@ class ReadStates {
 		});
 	}
 
-	handleLoadMessages(action: {
-		channelId: string;
-		isAfter?: boolean;
-		messages: Array<WireMessage>;
-		tailProbeWatermarkId?: string | null;
-	}): void {
+	handleLoadMessages(action: {channelId: string; isAfter?: boolean; messages: Array<WireMessage>}): void {
 		const state = this.get(action.channelId);
 		state.messagesLoaded = true;
 		const messages = Messages.getMessages(action.channelId);
@@ -453,17 +448,6 @@ class ReadStates {
 			state.lastMessageId = newestMessage.id;
 		}
 		const landedOnNewestWindow = messages.hasNewestMessages();
-		if (
-			action.isAfter &&
-			action.tailProbeWatermarkId != null &&
-			action.tailProbeWatermarkId === state.lastMessageId &&
-			action.messages.length === 0 &&
-			landedOnNewestWindow &&
-			newestMessage != null &&
-			isNewerMessageId(state.lastMessageId, newestMessage.id)
-		) {
-			state.lastMessageId = newestMessage.id;
-		}
 		const landedOnAck = state.ackMessageId != null && messages.jumpDestinationId === state.ackMessageId;
 		if (state.hasUnread() || landedOnNewestWindow || landedOnAck) {
 			state.rebuild();

fluxer_app/src/features/read_state/state/ReadStatesDeletedTailMessage.test.ts

@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {Endpoints} from '@app/features/app/constants/Endpoints';
+import {Channel} from '@app/features/channel/models/Channel';
+import {ACK_BATCH_DELAY_MS, type GatewayReadState} from '@app/features/read_state/state/read_states/shared';
+import {ChannelTypes} from '@fluxer/constants/src/ChannelConstants';
+import type {Channel as WireChannel} from '@fluxer/schema/src/domains/channel/ChannelSchemas';
+import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
+
+const channels = new Map<string, Channel>();
+let pinnedToEnd = false;
+let automaticAck = false;
+let hasNewest = true;
+const loadedMessages: Array<{id: string; author: {id: string}}> = [];
+
+vi.mock('@app/features/app/state/RuntimeConfig', () => ({default: {localInstanceDomain: 'fluxer.test'}}));
+vi.mock('@app/features/channel/state/Channels', () => ({default: {getChannel: (id: string) => channels.get(id)}}));
+vi.mock('@app/features/messaging/state/MessagingMessages', () => ({
+	default: {
+		getMessages: () => ({
+			get hasMoreBefore() {
+				return false;
+			},
+			get length() {
+				return loadedMessages.length;
+			},
+			jumpDestinationId: null,
+			hasNewestMessages: () => hasNewest,
+			has: (id: string) => loadedMessages.some((m) => m.id === id),
+			last: () => loadedMessages[loadedMessages.length - 1],
+			forEachBuffered: (cb: (m: unknown) => void) => {
+				for (const m of loadedMessages) cb(m);
+			},
+		}),
+	},
+}));
+vi.mock('@app/features/user/state/Users', () => ({
+	default: {getCurrentUser: () => ({id: 'me'}), cacheUsers: () => {}},
+}));
+vi.mock('@app/features/relationship/state/Relationships', () => ({default: {isBlocked: () => false}}));
+vi.mock('@app/features/member/state/GuildMembers', () => ({default: {getMember: () => null}}));
+vi.mock('@app/features/user/state/UserGuildSettings', () => ({
+	default: {
+		isEveryoneMentionSuppressed: () => false,
+		isRoleMentionSuppressed: () => false,
+		isGuildOrChannelMuted: () => false,
+	},
+}));
+vi.mock('@app/features/ui/state/Dimension', () => ({default: {channelPinnedToEnd: () => pinnedToEnd}}));
+vi.mock('@app/features/notification/state/NotificationAutoAck', () => ({
+	default: {isAutomaticAckEnabled: () => automaticAck, disableForChannel: () => {}},
+}));
+vi.mock('@app/features/platform/transport/RestTransport', () => ({
+	http: {post: vi.fn(async () => ({body: {read_states: []}})), get: vi.fn()},
+}));
+
+const {default: ReadStates} = await import('@app/features/read_state/state/ReadStates');
+const {http} = await import('@app/features/platform/transport/RestTransport');
+
+const CHANNEL = '1431490439357088063';
+const PHANTOM = '1547836192152621056';
+const REAL = '1546984669772255232';
+const OLD_ACK = '1546000000000000000';
+
+function guildText(lastMessageId: string): WireChannel {
+	return {id: CHANNEL, type: ChannelTypes.GUILD_TEXT, guild_id: '1431490056488128806', last_message_id: lastMessageId};
+}
+
+function readState(ackMessageId: string): GatewayReadState {
+	return {id: CHANNEL, last_message_id: ackMessageId, mention_count: 0, version: '1'};
+}
+
+function ready(ackMessageId: string): void {
+	channels.clear();
+	channels.set(CHANNEL, new Channel(guildText(PHANTOM)));
+	ReadStates.handleGatewayReady({readState: [readState(ackMessageId)], channels: [guildText(PHANTOM)]});
+}
+
+function lastAckedMessageId(): string | null {
+	const calls = vi.mocked(http.post).mock.calls;
+	if (calls.length === 0) return null;
+	const body = calls[calls.length - 1][1] as {body: {read_states: Array<{message_id: string}>}};
+	return body.body.read_states[0].message_id;
+}
+
+describe('channel whose newest message was deleted', () => {
+	beforeEach(() => {
+		vi.useFakeTimers();
+		loadedMessages.length = 0;
+		pinnedToEnd = false;
+		automaticAck = false;
+		hasNewest = true;
+		vi.mocked(http.post).mockClear();
+	});
+
+	afterEach(() => {
+		ReadStates.clearAll();
+		vi.useRealTimers();
+	});
+
+	it('acks up to the channel watermark rather than the newest surviving message', async () => {
+		ready(OLD_ACK);
+		expect(ReadStates.hasUnread(CHANNEL)).toBe(true);
+		loadedMessages.push({id: REAL, author: {id: 'tuna'}});
+		ReadStates.handleLoadMessages({channelId: CHANNEL, isAfter: false, messages: [{id: REAL}] as never});
+		ReadStates.handleLoadMessages({channelId: CHANNEL, isAfter: true, messages: []});
+		expect(ReadStates.lastMessageId(CHANNEL)).toBe(PHANTOM);
+		pinnedToEnd = true;
+		automaticAck = true;
+		ReadStates.handleChannelAckWithStickyUnread({channelId: CHANNEL});
+		await vi.advanceTimersByTimeAsync(ACK_BATCH_DELAY_MS);
+		expect(http.post).toHaveBeenCalledWith(Endpoints.READ_STATES_ACK, {
+			body: {read_states: [{channel_id: CHANNEL, message_id: PHANTOM}]},
+		});
+		expect(ReadStates.hasUnread(CHANNEL)).toBe(false);
+	});
+
+	it('stays read after a reload', async () => {
+		ready(OLD_ACK);
+		loadedMessages.push({id: REAL, author: {id: 'tuna'}});
+		ReadStates.handleLoadMessages({channelId: CHANNEL, isAfter: false, messages: [{id: REAL}] as never});
+		ReadStates.handleLoadMessages({channelId: CHANNEL, isAfter: true, messages: []});
+		pinnedToEnd = true;
+		automaticAck = true;
+		ReadStates.handleChannelAckWithStickyUnread({channelId: CHANNEL});
+		await vi.advanceTimersByTimeAsync(ACK_BATCH_DELAY_MS);
+		const acked = lastAckedMessageId();
+		expect(acked).not.toBeNull();
+		ReadStates.clearAll();
+		ready(acked as string);
+		expect(ReadStates.hasUnread(CHANNEL)).toBe(false);
+	});
+});