Skip to content

Commit 707f8c0

Browse files
committed
fix(sdk): include delayed messages in MAM catch-up cursor to prevent backward queries
1 parent 8b2eb27 commit 707f8c0

7 files changed

Lines changed: 183 additions & 43 deletions

File tree

apps/fluux/src-tauri/tauri.conf.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
],
6969
"macOS": {
7070
"minimumSystemVersion": "10.13",
71-
"bundleVersion": "e40b1fa",
71+
"bundleVersion": "12987ef",
7272
"entitlements": "Entitlements.plist",
7373
"signingIdentity": null
7474
},

packages/fluux-sdk/src/core/chatSideEffects.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,74 @@ describe('setupChatSideEffects', () => {
235235
})
236236
})
237237

238+
describe('catch-up with delayed messages', () => {
239+
it('should use forward query from newest delayed message (not fall back to backward)', async () => {
240+
// This test validates the fix for GitHub issue #135:
241+
// Messages sent from another client while Fluux is closed were not shown
242+
// because the catch-up cursor skipped delayed messages, causing a backward
243+
// query whose prepend-based merge put newer messages at the wrong position.
244+
const delayedTimestamp = new Date('2026-02-15T10:00:00Z')
245+
246+
connectionStore.getState().setServerInfo({
247+
identities: [],
248+
domain: 'example.com',
249+
features: [NS_MAM],
250+
})
251+
252+
chatStore.getState().addConversation({
253+
id: 'contact@example.com',
254+
name: 'contact@example.com',
255+
type: 'chat',
256+
lastMessage: undefined,
257+
unreadCount: 0,
258+
})
259+
260+
chatStore.getState().setActiveConversation('contact@example.com')
261+
262+
// Mock loadMessagesFromCache to populate with ONLY delayed messages
263+
// (simulates a conversation populated entirely via previous MAM catch-ups)
264+
const delayedMsg = {
265+
type: 'chat' as const,
266+
id: 'delayed-msg-1',
267+
conversationId: 'contact@example.com',
268+
from: 'contact@example.com',
269+
body: 'Previous MAM message',
270+
timestamp: delayedTimestamp,
271+
isOutgoing: false,
272+
isDelayed: true,
273+
}
274+
const loadSpy = vi.spyOn(chatStore.getState(), 'loadMessagesFromCache')
275+
.mockImplementation(async (conversationId: string) => {
276+
chatStore.getState().addMessage({
277+
...delayedMsg,
278+
conversationId,
279+
})
280+
return [delayedMsg]
281+
})
282+
283+
connectionStore.getState().setStatus('disconnected')
284+
cleanup = setupChatSideEffects(mockClient)
285+
286+
simulateFreshSession(mockClient)
287+
288+
await vi.waitFor(() => {
289+
expect(mockClient.chat.queryMAM).toHaveBeenCalledWith(
290+
expect.objectContaining({
291+
with: 'contact@example.com',
292+
start: expect.any(String),
293+
})
294+
)
295+
})
296+
297+
// Verify it used a forward query from the delayed message's timestamp
298+
// (previously this would have been a backward query without 'start')
299+
const call = (mockClient.chat.queryMAM as ReturnType<typeof vi.fn>).mock.calls[0][0]
300+
expect(new Date(call.start).getTime()).toBe(delayedTimestamp.getTime() + 1)
301+
302+
loadSpy.mockRestore()
303+
})
304+
})
305+
238306
describe('SM resumption', () => {
239307
it('should NOT trigger MAM catchup on SM resumption for active conversation', async () => {
240308
connectionStore.getState().setServerInfo({

packages/fluux-sdk/src/core/chatSideEffects.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,19 @@ import { logInfo } from './logger'
1818
import { getDomain } from './jid'
1919

2020
/**
21-
* Find the newest message that was NOT delivered with delay (offline).
22-
* Delayed messages should not advance the MAM catch-up cursor.
21+
* Find the newest message in the array (regardless of delay status).
22+
*
23+
* Used as the catch-up cursor for MAM forward queries. Previously this
24+
* skipped delayed messages, but that caused a subtle bug: when ALL cached
25+
* messages are delayed (common after previous MAM catch-ups), the cursor
26+
* returned undefined, triggering a backward query. The backward query's
27+
* merge (`prependOlderMessages`) incorrectly placed newer messages (sent
28+
* from another client while offline) at the top of the list, making them
29+
* invisible to the user and preventing the sidebar lastMessage update.
2330
*/
24-
function findNewestLiveMessage(messages: Array<{ isDelayed?: boolean; timestamp?: Date }>): { timestamp: Date } | undefined {
31+
function findNewestMessage(messages: Array<{ timestamp?: Date }>): { timestamp: Date } | undefined {
2532
for (let i = messages.length - 1; i >= 0; i--) {
26-
if (!messages[i].isDelayed && messages[i].timestamp) return messages[i] as { timestamp: Date }
33+
if (messages[i].timestamp) return messages[i] as { timestamp: Date }
2734
}
2835
return undefined
2936
}
@@ -102,14 +109,11 @@ export function setupChatSideEffects(
102109

103110
// Re-read messages after cache load (store was mutated)
104111
const cachedMessages = chatStore.getState().messages.get(conversationId) || []
105-
// Use the newest non-delayed (live) message as the catch-up cursor.
106-
// Delayed messages (offline delivery) should not advance the cursor,
107-
// otherwise MAM catch-up skips the gap where originals of corrections live.
108-
const newestLiveMessage = findNewestLiveMessage(cachedMessages)
112+
const newestMessage = findNewestMessage(cachedMessages)
109113

110114
const queryOptions: { with: string; start?: string } = { with: conversation.id }
111-
if (newestLiveMessage?.timestamp) {
112-
const startTime = new Date(newestLiveMessage.timestamp.getTime() + 1)
115+
if (newestMessage?.timestamp) {
116+
const startTime = new Date(newestMessage.timestamp.getTime() + 1)
113117
queryOptions.start = startTime.toISOString()
114118
}
115119

packages/fluux-sdk/src/core/modules/MAM.catchup.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ describe('MAM Background Catch-Up', () => {
388388
expect(mamQuery).toBeDefined()
389389
})
390390

391-
it('should fall back to backward query when all cached messages are delayed', async () => {
391+
it('should use forward query from newest delayed message as catch-up cursor', async () => {
392392
await connectClient()
393393

394394
vi.mocked(mockStores.chat.getAllConversations).mockReturnValue([
@@ -415,10 +415,12 @@ describe('MAM Background Catch-Up', () => {
415415
await waitForAsyncOps(20, 100)
416416
await catchUpPromise
417417

418-
// Should fall back to backward query (before="") since no live messages exist
418+
// Should use forward query from the newest delayed message timestamp
419+
// (not fall back to backward query) so merge uses full sort and
420+
// correctly positions messages sent from other clients while offline
419421
expect(emitSDKSpy).toHaveBeenCalledWith('chat:mam-messages', expect.objectContaining({
420422
conversationId: 'alice@example.com',
421-
direction: 'backward',
423+
direction: 'forward',
422424
}))
423425
})
424426
})

packages/fluux-sdk/src/core/modules/MAM.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,17 @@ interface UnresolvedModifications {
8787
}
8888

8989
/**
90-
* Find the newest message in an array that was NOT delivered with delay (offline).
91-
* Delayed messages should not advance the MAM catch-up cursor, otherwise the
92-
* forward query skips the gap where originals of corrections may live.
90+
* Find the newest message in an array (regardless of delay status).
91+
*
92+
* Used as the catch-up cursor for MAM forward queries. Including delayed
93+
* messages ensures the catch-up always uses a forward query, which merges
94+
* correctly via full sort. Previously skipping delayed messages caused
95+
* backward queries whose prepend-based merge put newer messages (sent
96+
* from another client while offline) at the wrong position.
9397
*/
94-
function findNewestLiveMessage(messages: Array<{ isDelayed?: boolean; timestamp?: Date }>): { timestamp: Date } | undefined {
98+
function findNewestMessage(messages: Array<{ timestamp?: Date }>): { timestamp: Date } | undefined {
9599
for (let i = messages.length - 1; i >= 0; i--) {
96-
if (!messages[i].isDelayed && messages[i].timestamp) return messages[i] as { timestamp: Date }
100+
if (messages[i].timestamp) return messages[i] as { timestamp: Date }
97101
}
98102
return undefined
99103
}
@@ -819,21 +823,18 @@ export class MAM extends BaseModule {
819823
if (this.deps.stores?.connection.getStatus() !== 'online') return
820824

821825
const messages = conv.messages || []
822-
// Use the newest non-delayed (live) message as the catch-up cursor.
823-
// Delayed messages (offline delivery) should not advance the cursor,
824-
// otherwise MAM catch-up skips the gap where originals of corrections live.
825-
const newestLiveMessage = findNewestLiveMessage(messages)
826-
827-
if (newestLiveMessage?.timestamp) {
828-
// Forward query from the last live message
829-
const startTime = new Date(newestLiveMessage.timestamp.getTime() + 1)
826+
const newestMessage = findNewestMessage(messages)
827+
828+
if (newestMessage?.timestamp) {
829+
// Forward query from the last known message
830+
const startTime = new Date(newestMessage.timestamp.getTime() + 1)
830831
await this.queryArchive({
831832
with: conv.id,
832833
start: startTime.toISOString(),
833834
max: 100,
834835
})
835836
} else {
836-
// No live messages (all delayed or empty) — fetch latest from MAM
837+
// No messages (empty) — fetch latest from MAM
837838
await this.queryArchive({
838839
with: conv.id,
839840
before: '',
@@ -947,19 +948,18 @@ export class MAM extends BaseModule {
947948
// Re-read room after cache load (store was mutated)
948949
const updatedRoom = this.deps.stores?.room.getRoom(room.jid)
949950
const messages = updatedRoom?.messages || []
950-
// Use the newest non-delayed (live) message as the catch-up cursor.
951-
const newestLiveMessage = findNewestLiveMessage(messages)
951+
const newestMessage = findNewestMessage(messages)
952952

953-
if (newestLiveMessage?.timestamp) {
954-
// Forward query from the last live message
955-
const startTime = new Date(newestLiveMessage.timestamp.getTime() + 1)
953+
if (newestMessage?.timestamp) {
954+
// Forward query from the last known message
955+
const startTime = new Date(newestMessage.timestamp.getTime() + 1)
956956
await this.queryRoomArchive({
957957
roomJid: room.jid,
958958
start: startTime.toISOString(),
959959
max: 100,
960960
})
961961
} else {
962-
// No live messages (all delayed or empty) — fetch latest from MAM
962+
// No messages (empty) — fetch latest from MAM
963963
await this.queryRoomArchive({
964964
roomJid: room.jid,
965965
before: '',

packages/fluux-sdk/src/core/roomSideEffects.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@ import { roomStore, connectionStore } from '../stores'
2020
import { logInfo } from './logger'
2121

2222
/**
23-
* Find the newest message that was NOT delivered with delay (offline).
24-
* Delayed messages should not advance the MAM catch-up cursor.
23+
* Find the newest message in the array (regardless of delay status).
24+
*
25+
* Used as the catch-up cursor for MAM forward queries. Including delayed
26+
* messages ensures the catch-up always uses a forward query, which merges
27+
* correctly via full sort.
2528
*/
26-
function findNewestLiveMessage(messages: Array<{ isDelayed?: boolean; timestamp?: Date }>): { timestamp: Date } | undefined {
29+
function findNewestMessage(messages: Array<{ timestamp?: Date }>): { timestamp: Date } | undefined {
2730
for (let i = messages.length - 1; i >= 0; i--) {
28-
if (!messages[i].isDelayed && messages[i].timestamp) return messages[i] as { timestamp: Date }
31+
if (messages[i].timestamp) return messages[i] as { timestamp: Date }
2932
}
3033
return undefined
3134
}
@@ -113,12 +116,11 @@ export function setupRoomSideEffects(
113116
// Re-read room after cache load (store was mutated)
114117
const roomAfterCache = roomStore.getState().rooms.get(roomJid)
115118
const messages = roomAfterCache?.messages || []
116-
// Use the newest non-delayed (live) message as the catch-up cursor.
117-
const newestLiveMessage = findNewestLiveMessage(messages)
119+
const newestMessage = findNewestMessage(messages)
118120

119-
if (newestLiveMessage?.timestamp) {
120-
// Query for messages AFTER the newest live message (catchup)
121-
const startTime = new Date(newestLiveMessage.timestamp.getTime() + 1)
121+
if (newestMessage?.timestamp) {
122+
// Query for messages AFTER the newest known message (catchup)
123+
const startTime = new Date(newestMessage.timestamp.getTime() + 1)
122124
await client.chat.queryRoomMAM({
123125
roomJid,
124126
start: startTime.toISOString(),

packages/fluux-sdk/src/stores/shared/messageArrayUtils.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,5 +687,69 @@ describe('messageArrayUtils', () => {
687687
expect(merged[2].id).toBe('msg-4')
688688
expect(merged[3].id).toBe('msg-5')
689689
})
690+
691+
it('should place newer messages at wrong position (known limitation for backward queries)', () => {
692+
// This test documents a known limitation of prependOlderMessages:
693+
// If "older" messages are actually NEWER than existing ones, they end up
694+
// at the wrong position (prepended before existing instead of appended after).
695+
//
696+
// This is why catch-up queries must use forward direction (with 'start' filter)
697+
// when cached messages exist — mergeAndProcessMessages does a full sort.
698+
const existing = [
699+
createMessage('msg-1', 'Old cached', new Date('2024-01-15T10:00:00Z')),
700+
createMessage('msg-2', 'Old cached', new Date('2024-01-15T11:00:00Z')),
701+
]
702+
703+
// "Older" messages that are actually newer (e.g., sent from another client while offline)
704+
const newer = [
705+
createMessage('msg-3', 'Sent from Gajim', new Date('2024-01-15T12:00:00Z')),
706+
]
707+
708+
const { merged } = prependOlderMessages(existing, newer, (m) => [m.id])
709+
710+
// BUG: msg-3 (newer) is placed BEFORE msg-1 and msg-2 (older)
711+
// This is why the catch-up cursor fix is essential — it prevents this scenario
712+
// by always using forward queries (mergeAndProcessMessages) when cached msgs exist
713+
expect(merged[0].id).toBe('msg-3') // Wrong: should be at end
714+
expect(merged[1].id).toBe('msg-1')
715+
expect(merged[2].id).toBe('msg-2')
716+
})
717+
})
718+
719+
describe('mergeAndProcessMessages correctly handles catch-up scenario', () => {
720+
it('should sort messages sent from another client while offline into correct position', () => {
721+
// Simulates the catch-up scenario with forward direction (mergeAndProcessMessages):
722+
// - Existing messages loaded from IndexedDB cache (all delayed from previous MAM)
723+
// - MAM catch-up returns messages sent from another client while Fluux was offline
724+
const existing = [
725+
createMessage('msg-1', 'Previous session', new Date('2024-01-15T10:00:00Z')),
726+
createMessage('msg-2', 'Previous session', new Date('2024-01-15T11:00:00Z')),
727+
]
728+
729+
const fromMAM = [
730+
// Message sent by user from another client while Fluux was closed
731+
createMessage('msg-3', '[OMEMO encrypted]', new Date('2024-01-15T12:00:00Z')),
732+
// Reply received while Fluux was closed
733+
createMessage('msg-4', '[OMEMO encrypted]', new Date('2024-01-15T13:00:00Z')),
734+
]
735+
736+
const { merged, newMessages } = mergeAndProcessMessages(
737+
existing,
738+
fromMAM,
739+
(m) => [m.id]
740+
)
741+
742+
// Full sort ensures correct chronological order
743+
expect(merged).toHaveLength(4)
744+
expect(merged[0].id).toBe('msg-1')
745+
expect(merged[1].id).toBe('msg-2')
746+
expect(merged[2].id).toBe('msg-3') // Correctly at position 3 (not prepended)
747+
expect(merged[3].id).toBe('msg-4') // Correctly at position 4 (newest)
748+
expect(newMessages).toHaveLength(2)
749+
750+
// The last message in the array should be the newest — this is used for
751+
// lastMessage sidebar preview in mergeMAMMessages
752+
expect(merged[merged.length - 1].id).toBe('msg-4')
753+
})
690754
})
691755
})

0 commit comments

Comments
 (0)