From 4ed09553ce3bd20197fbeeba6da0e461954825ed Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Mon, 3 Aug 2026 18:24:47 +0200 Subject: [PATCH 1/3] fix(sync): keep room state and read receipts flowing under sliding sync --- src/app/utils/notifications.test.ts | 106 ++++++++++++++ src/client/presenceSync.ts | 3 + src/client/slidingSync.test.ts | 130 ++++++++++++------ src/client/slidingSync.ts | 118 ++++++++-------- .../slidingSyncExpandedTimeline.test.ts | 120 ++++++++++++++++ tests/e2e/fixtures/continuwuity.ts | 17 +++ tests/e2e/sliding-sync-room-state.spec.ts | 98 +++++++++++++ 7 files changed, 495 insertions(+), 97 deletions(-) create mode 100644 src/app/utils/notifications.test.ts create mode 100644 src/client/slidingSyncExpandedTimeline.test.ts create mode 100644 tests/e2e/sliding-sync-room-state.spec.ts diff --git a/src/app/utils/notifications.test.ts b/src/app/utils/notifications.test.ts new file mode 100644 index 0000000000..9053200010 --- /dev/null +++ b/src/app/utils/notifications.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { MatrixClient, MatrixEvent } from '$types/matrix-sdk'; +import { ReceiptType } from '$types/matrix-sdk'; +import { markAsRead } from './notifications'; + +vi.mock('@tauri-apps/api/core', () => ({ isTauri: () => false })); + +const userId = '@me:example.com'; +const roomId = '!room:example.com'; + +const event = (id: string, sending = false): MatrixEvent => + ({ + getId: () => id, + isSending: () => sending, + }) as unknown as MatrixEvent; + +const makeMx = (events: MatrixEvent[], readUpTo: string | null) => { + const setRoomReadMarkers = vi + .fn< + ( + roomId: string, + eventId: string, + read?: MatrixEvent, + fullyRead?: MatrixEvent + ) => Promise + >() + .mockResolvedValue(); + const sendReadReceipt = vi + .fn<(event: MatrixEvent, receiptType: ReceiptType) => Promise>() + .mockResolvedValue(); + const room = { + getLiveTimeline: () => ({ getEvents: () => events }), + getEventReadUpTo: () => readUpTo, + }; + + return { + mx: { + getRoom: (id: string) => (id === roomId ? room : null), + getUserId: () => userId, + setRoomReadMarkers, + sendReadReceipt, + } as unknown as MatrixClient, + setRoomReadMarkers, + sendReadReceipt, + }; +}; + +// The live timeline is in timeline order by contract, so the newest event is last. +// Ordering defects belong in markExpandedTimelinesLimited, not here. +describe('markAsRead', () => { + it('marks read up to the last event in the timeline', async () => { + const { mx, setRoomReadMarkers, sendReadReceipt } = makeMx( + [event('$older'), event('$newest')], + null + ); + + await markAsRead(mx, roomId, false); + + expect(setRoomReadMarkers).toHaveBeenCalledWith(roomId, '$newest', expect.anything()); + expect(sendReadReceipt).toHaveBeenCalledWith(expect.anything(), ReceiptType.Read); + expect(sendReadReceipt.mock.calls[0]?.[0].getId()).toBe('$newest'); + }); + + it('does nothing when the last event is already read', async () => { + const { mx, setRoomReadMarkers, sendReadReceipt } = makeMx( + [event('$older'), event('$newest')], + '$newest' + ); + + await markAsRead(mx, roomId, false); + + expect(setRoomReadMarkers).not.toHaveBeenCalled(); + expect(sendReadReceipt).not.toHaveBeenCalled(); + }); + + it('ignores events that are still sending', async () => { + const { mx, sendReadReceipt } = makeMx([event('$confirmed'), event('$local', true)], null); + + await markAsRead(mx, roomId, false); + + expect(sendReadReceipt.mock.calls[0]?.[0].getId()).toBe('$confirmed'); + }); + + it('sends a private receipt when reads are hidden', async () => { + const { mx, setRoomReadMarkers, sendReadReceipt } = makeMx([event('$newest')], null); + + await markAsRead(mx, roomId, true); + + expect(setRoomReadMarkers).toHaveBeenCalledWith( + roomId, + '$newest', + undefined, + expect.anything() + ); + expect(sendReadReceipt).toHaveBeenCalledWith(expect.anything(), ReceiptType.ReadPrivate); + }); + + it('does nothing for an empty timeline', async () => { + const { mx, setRoomReadMarkers, sendReadReceipt } = makeMx([], null); + + await markAsRead(mx, roomId, false); + + expect(setRoomReadMarkers).not.toHaveBeenCalled(); + expect(sendReadReceipt).not.toHaveBeenCalled(); + }); +}); diff --git a/src/client/presenceSync.ts b/src/client/presenceSync.ts index 4d2b9a6d8a..1cb5b946f1 100644 --- a/src/client/presenceSync.ts +++ b/src/client/presenceSync.ts @@ -76,6 +76,9 @@ export class PresenceSyncManager { this.abortController?.abort(); } + // MSC4186 has no presence extension and a filter cannot exclude to_device, so this + // poll drains the same per-device queue as the to_device extension. Dropping these + // would lose room keys. private async processCrypto(response: PresenceSyncResponse): Promise { const crypto = this.mx.getCrypto() as CryptoBackend | undefined; if (!crypto) return; diff --git a/src/client/slidingSync.test.ts b/src/client/slidingSync.test.ts index 3324f4b002..7571a25818 100644 --- a/src/client/slidingSync.test.ts +++ b/src/client/slidingSync.test.ts @@ -6,7 +6,11 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { MatrixClient, MatrixEvent, MSC3575List } from '$types/matrix-sdk'; import { EventType, KnownMembership, SlidingSyncEvent, SlidingSyncState } from '$types/matrix-sdk'; -import { scopeEphemeralExtensions, SlidingSyncManager } from './slidingSync'; +import { + markExpandedTimelinesLimited, + scopeTypingExtension, + SlidingSyncManager, +} from './slidingSync'; import type { SlidingSyncSidebarCache } from './slidingSyncSidebarCache'; // ── vi.hoisted mocks ───────────────────────────────────────────────────────── @@ -162,7 +166,6 @@ describe('SlidingSyncManager initial request', () => { const lists = mocks.slidingSyncConstructorArgs?.[1] as Map; const joined = lists.get('joined'); - const updates = lists.get('updates'); const defaultSubscription = mocks.slidingSyncConstructorArgs?.[2] as { timeline_limit: number; required_state: string[][]; @@ -173,12 +176,7 @@ describe('SlidingSyncManager initial request', () => { expect(joined?.required_state).toHaveLength(10); expect(joined?.required_state).toContainEqual([EventType.RoomJoinRules, '']); expect(joined?.required_state).not.toContainEqual(['m.space.child', '*']); - expect(updates).toMatchObject({ - ranges: [[0, 29]], - timeline_limit: 1, - required_state: [[EventType.RoomMember, '$ME']], - filters: { is_invite: false }, - }); + expect([...lists.keys()]).toEqual(['joined', 'invites']); expect(defaultSubscription.timeline_limit).toBe(50); expect(defaultSubscription.required_state).toContainEqual([EventType.RoomMember, '$LAZY']); expect(defaultSubscription.required_state).not.toContainEqual([EventType.RoomMember, '*']); @@ -449,43 +447,39 @@ describe('SlidingSyncManager initial request', () => { }); }); - it('keeps full lightweight event coverage after narrowing the detailed joined list', () => { - const listRanges = new Map([ - ['joined', [[0, 29]]], - ['updates', [[0, 29]]], - ]); + // Narrowing it stops state deltas reaching every room but those in the window. + it('keeps the joined list covering every room once hydration completes', () => { + let joinedRange: [number, number][] = [[0, 29]]; const manager = makeManager(makeMockMx()); mocks.slidingSyncInstance.getListData.mockImplementation((key: string) => - key === 'joined' || key === 'updates' ? ({ joinedCount: 45 } as never) : null + key === 'joined' ? ({ joinedCount: 45 } as never) : null ); - mocks.slidingSyncInstance.getListParams.mockImplementation( - (key: string) => ({ ranges: listRanges.get(key) ?? [[0, 29]] }) as never + mocks.slidingSyncInstance.getListParams.mockImplementation((key: string) => + key === 'joined' ? ({ ranges: joinedRange } as never) : null ); mocks.slidingSyncInstance.setListRanges.mockImplementation((key, ranges) => { - if (key === 'joined' || key === 'updates') { - listRanges.set(key, ranges as [number, number][]); - } + if (key === 'joined') joinedRange = ranges as [number, number][]; }); manager.attach(); fireLifecycle(SlidingSyncState.Complete); - expect(listRanges.get('joined')).toEqual([[0, 44]]); - expect(listRanges.get('updates')).toEqual([[0, 44]]); + expect(joinedRange).toEqual([[0, 44]]); fireLifecycle(SlidingSyncState.Complete); fireLifecycle(SlidingSyncState.Complete); - expect(listRanges.get('joined')).toEqual([[0, 2]]); - expect(listRanges.get('updates')).toEqual([[0, 44]]); + expect(joinedRange).toEqual([[0, 44]]); }); - it('keeps detailed coverage when the homeserver does not provide the updates list', () => { + // A rename does not bump a room, so it never sorts back into a stale window. + it('extends the joined list when rooms are joined after hydration', () => { + let joinedCount = 45; let joinedRange: [number, number][] = [[0, 29]]; const manager = makeManager(makeMockMx()); mocks.slidingSyncInstance.getListData.mockImplementation((key: string) => - key === 'joined' ? ({ joinedCount: 45 } as never) : null + key === 'joined' ? ({ joinedCount } as never) : null ); mocks.slidingSyncInstance.getListParams.mockImplementation((key: string) => - key === 'joined' ? ({ ranges: joinedRange } as never) : ({ ranges: [[0, 29]] } as never) + key === 'joined' ? ({ ranges: joinedRange } as never) : null ); mocks.slidingSyncInstance.setListRanges.mockImplementation((key, ranges) => { if (key === 'joined') joinedRange = ranges as [number, number][]; @@ -494,8 +488,12 @@ describe('SlidingSyncManager initial request', () => { fireLifecycle(SlidingSyncState.Complete); fireLifecycle(SlidingSyncState.Complete); - expect(joinedRange).toEqual([[0, 44]]); + + joinedCount = 48; + fireLifecycle(SlidingSyncState.Complete); + + expect(joinedRange).toEqual([[0, 47]]); }); }); @@ -623,6 +621,22 @@ describe('SlidingSyncManager room subscription coordination', () => { ); }); + it('drops the image-pack subscription for a room that was left', async () => { + const manager = makeManager(makeMockMx()); + const roomId = '!pack:example.com'; + (manager as unknown as { listsFullyLoaded: boolean }).listsFullyLoaded = true; + + manager.setImagePackSubscriptions([roomId]); + expect(mocks.slidingSyncInstance.modifyRoomSubscriptions).toHaveBeenLastCalledWith( + new Set([roomId]) + ); + + manager.reconcileRoomMembership(roomId, KnownMembership.Leave); + await Promise.resolve(); + + expect(mocks.slidingSyncInstance.modifyRoomSubscriptions).toHaveBeenLastCalledWith(new Set()); + }); + it('keeps space subscriptions active for future hierarchy changes', async () => { const manager = makeManager(makeMockMx()); const roomId = '!space:example.com'; @@ -806,15 +820,15 @@ describe('SlidingSyncManager room subscription coordination', () => { }); }); -describe('scopeEphemeralExtensions', () => { - it('limits typing and receipts to active rooms without changing other extensions', () => { +describe('scopeTypingExtension', () => { + it('limits typing to active rooms without changing other extensions', () => { const extensions = { typing: { enabled: true }, receipts: { enabled: true }, account_data: { enabled: true }, }; - scopeEphemeralExtensions(extensions, ['!space:example.com', '!room:example.com']); + scopeTypingExtension(extensions, ['!space:example.com', '!room:example.com']); expect(extensions).toEqual({ typing: { @@ -822,25 +836,63 @@ describe('scopeEphemeralExtensions', () => { lists: [], rooms: ['!space:example.com', '!room:example.com'], }, - receipts: { - enabled: true, - lists: [], - rooms: ['!space:example.com', '!room:example.com'], - }, + receipts: { enabled: true }, account_data: { enabled: true }, }); }); - it('uses an empty room scope when no timeline is open', () => { + it('leaves receipts unscoped so every room keeps receiving read state', () => { const extensions = { typing: { enabled: true, lists: ['joined'], rooms: ['!old:example.com'] }, - receipts: { enabled: true, lists: ['joined'], rooms: ['!old:example.com'] }, + receipts: { enabled: true }, }; - scopeEphemeralExtensions(extensions, []); + scopeTypingExtension(extensions, []); expect(extensions.typing).toMatchObject({ lists: [], rooms: [] }); - expect(extensions.receipts).toMatchObject({ lists: [], rooms: [] }); + expect(extensions.receipts).toEqual({ enabled: true }); + }); +}); + +describe('markExpandedTimelinesLimited', () => { + it('marks an expanded timeline limited so the SDK reconciles the gap', () => { + const resp = { + rooms: { + '!dm:example.com': { unstable_expanded_timeline: true, prev_batch: 't1-2' }, + }, + } as unknown as Parameters[0]; + + markExpandedTimelinesLimited(resp); + + expect(resp?.rooms['!dm:example.com']).toMatchObject({ limited: true }); + }); + + it('leaves rooms without an expanded timeline untouched', () => { + const resp = { + rooms: { + '!quiet:example.com': { limited: false, prev_batch: 't1-2' }, + }, + } as unknown as Parameters[0]; + + markExpandedTimelinesLimited(resp); + + expect(resp?.rooms['!quiet:example.com']).toMatchObject({ limited: false }); + }); + + it('keeps an expanded timeline unlimited when there is no pagination token', () => { + const resp = { + rooms: { + '!dm:example.com': { unstable_expanded_timeline: true }, + }, + } as unknown as Parameters[0]; + + markExpandedTimelinesLimited(resp); + + expect(resp?.rooms['!dm:example.com']).not.toHaveProperty('limited'); + }); + + it('tolerates a response without rooms', () => { + expect(() => markExpandedTimelinesLimited(null)).not.toThrow(); }); }); diff --git a/src/client/slidingSync.ts b/src/client/slidingSync.ts index 3eec08431e..36e31cc0b3 100644 --- a/src/client/slidingSync.ts +++ b/src/client/slidingSync.ts @@ -34,14 +34,10 @@ const debugLog = createDebugLogger('slidingSync'); const LIST_JOINED = 'joined'; const LIST_INVITES = 'invites'; -const LIST_UPDATES = 'updates'; const LIST_TIMELINE_LIMIT = 1; const LIST_PAGE_SIZE = 30; -const STEADY_STATE_DETAILED_ROOMS = 3; const DEFAULT_POLL_TIMEOUT_MS = 45000; -const LIST_SORT_ORDER = ['by_recency', 'by_name']; - const ACTIVE_ROOM_SUBSCRIPTION_KEY = 'active_room'; const SIDEBAR_ROOM_SUBSCRIPTION_KEY = 'sidebar_room'; const SPACE_SUBSCRIPTION_KEY = 'space'; @@ -59,7 +55,6 @@ type OptimisticJoin = { export type PartialSlidingSyncRequest = { filters?: MSC3575List['filters']; - sort?: string[]; ranges?: [number, number][]; }; @@ -147,8 +142,9 @@ const buildSelfJoinEvent = ( }; }; +// The wildcard state key makes Synapse fall back to StateFilter.all() and return +// the room's whole state on the initial send, so this is not as cheap as it looks. const buildListRequiredState = (): MSC3575RoomSubscription['required_state'] => [ - // first sync limited solely to what's needed to render rooms [EventType.RoomAvatar, ''], [EventType.RoomTombstone, ''], [EventType.RoomEncryption, ''], @@ -242,7 +238,6 @@ const buildLists = (): Map => { lists.set(LIST_JOINED, { ranges: [[0, LIST_PAGE_SIZE - 1]], - sort: LIST_SORT_ORDER, timeline_limit: LIST_TIMELINE_LIMIT, required_state: listRequiredState, filters: { is_invite: false }, @@ -250,20 +245,11 @@ const buildLists = (): Map => { lists.set(LIST_INVITES, { ranges: [[0, LIST_PAGE_SIZE - 1]], - sort: LIST_SORT_ORDER, timeline_limit: LIST_TIMELINE_LIMIT, required_state: listRequiredState, filters: { is_invite: true }, }); - lists.set(LIST_UPDATES, { - ranges: [[0, LIST_PAGE_SIZE - 1]], - sort: LIST_SORT_ORDER, - timeline_limit: LIST_TIMELINE_LIMIT, - required_state: [[EventType.RoomMember, MSC3575_STATE_KEY_ME]], - filters: { is_invite: false }, - }); - return lists; }; @@ -278,21 +264,38 @@ type RoomScopedExtension = { rooms?: string[]; }; -export const scopeEphemeralExtensions = ( +// Receipts stay unscoped on purpose: they drive unread state for every room in +// the sidebar, and a room that never receives one reads as permanently unread. +export const scopeTypingExtension = ( extensions: object | undefined, roomIds: readonly string[] ): void => { if (!extensions) return; - const extensionMap = extensions as Record; - ['typing', 'receipts'].forEach((name) => { - const extension = extensionMap[name]; - if (!extension || typeof extension !== 'object') return; + const typing = (extensions as Record).typing; + if (!typing || typeof typing !== 'object') return; - const scopedExtension = extension as RoomScopedExtension; - scopedExtension.lists = []; - scopedExtension.rooms = [...roomIds]; - }); + const scopedTyping = typing as RoomScopedExtension; + scopedTyping.lists = []; + scopedTyping.rooms = [...roomIds]; +}; + +type ExpandedTimelineRoomData = MSC3575RoomData & { unstable_expanded_timeline?: boolean }; + +// Synapse flags history re-sent for a raised timeline_limit only as +// `unstable_expanded_timeline`, but the SDK reconciles a gap on `limited`/`initial`, +// so without this it appends that history after the newest event. Drop once the SDK +// reads the flag. +export const markExpandedTimelinesLimited = (resp: MSC3575SlidingSyncResponse | null): void => { + if (!resp?.rooms) return; + + for (const roomData of Object.values(resp.rooms)) { + const expanded = roomData as ExpandedTimelineRoomData; + // Without a token the SDK would clear the back-pagination token. + if (expanded.unstable_expanded_timeline === true && typeof expanded.prev_batch === 'string') { + expanded.limited = true; + } + } }; export class SlidingSyncManager { @@ -531,7 +534,7 @@ export class SlidingSyncManager { const currentCount = listData?.joinedCount ?? 0; const previousCount = this.previousListCounts.get(key) ?? 0; - if (key !== LIST_UPDATES) totalRoomCount += currentCount; + totalRoomCount += currentCount; if (currentCount !== previousCount) { changes[key] = { @@ -583,6 +586,7 @@ export class SlidingSyncManager { } this.expandListsByPage(); + this.ensureListCoverage(); Sentry.metrics.distribution('sable.sync.processing_ms', syncDuration, { attributes: { transport: 'sliding' }, @@ -620,11 +624,10 @@ export class SlidingSyncManager { if (member.membership !== KnownMembership.Leave && member.membership !== KnownMembership.Ban) return; this.sidebarCache.removeRoom(member.roomId); - const removedSpaceSubscription = this.spaceSubscriptions.delete(member.roomId); - const removedSidebarSubscription = this.sidebarRoomSubscriptions.delete(member.roomId); + const removedPassiveSubscription = this.removePassiveSubscriptions(member.roomId); if (this.activeRoomSubscriptions.has(member.roomId)) { this.unsubscribeFromRoom(member.roomId); - } else if (removedSpaceSubscription || removedSidebarSubscription) { + } else if (removedPassiveSubscription) { this.queueRoomSubscriptionSync(); } }; @@ -937,7 +940,6 @@ export class SlidingSyncManager { this.hydrationStatusListeners.forEach((listener) => listener(false)); this.reconcileSidebarCacheMembership(); globalThis.setTimeout(() => this.flushDeferredSubscriptions(), 0); - this.applySteadyStateListRanges(); log.log(`Sliding Sync all lists fully loaded for ${this.mx.getUserId()}`); const totalRooms = (this.slidingSync.getListData(LIST_JOINED)?.joinedCount ?? 0) + @@ -979,31 +981,23 @@ export class SlidingSyncManager { } } - private applySteadyStateListRanges(): void { - const joinedList = this.slidingSync.getListParams(LIST_JOINED); - const currentEnd = getListEndIndex(joinedList); - const steadyStateEnd = Math.min(currentEnd, STEADY_STATE_DETAILED_ROOMS - 1); - if (steadyStateEnd < 0 || steadyStateEnd === currentEnd) return; - - const joinedCount = this.slidingSync.getListData(LIST_JOINED)?.joinedCount ?? 0; - const updatesCount = this.slidingSync.getListData(LIST_UPDATES)?.joinedCount ?? 0; - const updatesConfirmedEnd = this.confirmedListRangeEnds.get(LIST_UPDATES) ?? -1; - if (updatesCount !== joinedCount || updatesConfirmedEnd < joinedCount - 1) { - debugLog.warn('sync', 'Kept detailed joined list fully covered: updates list unavailable', { - joinedCount, - updatesCount, - updatesConfirmedEnd, - }); - return; - } + // Paging stops once every list is covered, but the counts keep growing. A state + // change does not bump a room, so a room joined mid-session would otherwise sit + // outside the window and stop receiving state deltas until the next restart. + private ensureListCoverage(): void { + if (!this.initialListHydrationCompleted) return; - this.slidingSync.setListRanges(LIST_JOINED, [[0, steadyStateEnd]]); - this.requestedListRangeEnds.set(LIST_JOINED, steadyStateEnd); - debugLog.info('sync', 'Reduced detailed joined list to steady-state window', { - previousEnd: currentEnd, - newEnd: steadyStateEnd, - retainedDetailedRooms: steadyStateEnd + 1, - updatesCoverageEnd: updatesConfirmedEnd, + this.listKeys.forEach((key) => { + const knownCount = this.slidingSync.getListData(key)?.joinedCount ?? 0; + const desiredEnd = knownCount - 1; + if (desiredEnd <= getListEndIndex(this.slidingSync.getListParams(key))) return; + + this.slidingSync.setListRanges(key, [[0, desiredEnd]]); + this.requestedListRangeEnds.set(key, desiredEnd); + debugLog.info('sync', `Extended list "${key}" to cover newly joined rooms`, { + list: key, + newEnd: desiredEnd, + }); }); } @@ -1012,7 +1006,6 @@ export class SlidingSyncManager { if (!list) { list = { ranges: [[0, LIST_PAGE_SIZE - 1]], - sort: LIST_SORT_ORDER, timeline_limit: LIST_TIMELINE_LIMIT, required_state: buildListRequiredState(), ...updateArgs, @@ -1275,17 +1268,26 @@ export class SlidingSyncManager { if (membership === KnownMembership.Leave) { this.optimisticallyJoinedRoomIds.delete(roomId); this.sidebarCache.removeRoom(roomId); - const removedSpaceSubscription = this.spaceSubscriptions.delete(roomId); - const removedSidebarSubscription = this.sidebarRoomSubscriptions.delete(roomId); + const removedPassiveSubscription = this.removePassiveSubscriptions(roomId); if (this.activeRoomSubscriptions.has(roomId)) { this.unsubscribeFromRoom(roomId); - } else if (removedSpaceSubscription || removedSidebarSubscription) { + } else if (removedPassiveSubscription) { this.queueRoomSubscriptionSync(); } this.mx.store.removeRoom(roomId); } } + // Includes the deferred sets so a later flush cannot resubscribe a room we left. + private removePassiveSubscriptions(roomId: string): boolean { + const removedSpace = this.spaceSubscriptions.delete(roomId); + const removedSidebar = this.sidebarRoomSubscriptions.delete(roomId); + const removedImagePack = this.imagePackRoomSubscriptions.delete(roomId); + this.deferredSpaceSubscriptions.delete(roomId); + this.deferredImagePackSubscriptions?.delete(roomId); + return removedSpace || removedSidebar || removedImagePack; + } + private flushDeferredSubscriptions(): void { if (this.disposed || !this.listsFullyLoaded) return; diff --git a/src/client/slidingSyncExpandedTimeline.test.ts b/src/client/slidingSyncExpandedTimeline.test.ts new file mode 100644 index 0000000000..15f61ba57e --- /dev/null +++ b/src/client/slidingSyncExpandedTimeline.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from 'vitest'; +import { SlidingSyncSdk } from 'matrix-js-sdk/lib/sliding-sync-sdk'; +import type { MatrixClient, MSC3575RoomData, MSC3575SlidingSyncResponse } from '$types/matrix-sdk'; +import type { Logger } from 'matrix-js-sdk/lib/logger'; +import { createClient, EventTimeline } from '$types/matrix-sdk'; +import { markExpandedTimelinesLimited } from './slidingSync'; + +// Drives the real matrix-js-sdk room-data path, so this pins the SDK's actual gap +// reconciliation rather than our assumptions about it. + +const userId = '@me:example.com'; +const roomId = '!dm:example.com'; + +type RoomDataHandler = (roomId: string, data: MSC3575RoomData) => Promise; + +const silentLogger: Logger = { + trace: () => {}, + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + getChild: () => silentLogger, +}; + +const makeSdk = (): { mx: MatrixClient; deliver: RoomDataHandler } => { + const mx = createClient({ baseUrl: 'https://example.com', userId, accessToken: 'token' }); + + let roomDataHandler: RoomDataHandler | undefined; + const slidingSyncStub = { + on: (event: string, handler: unknown) => { + if (event === 'SlidingSync.RoomData') roomDataHandler = handler as RoomDataHandler; + }, + registerExtension: () => {}, + }; + + new SlidingSyncSdk(slidingSyncStub as never, mx, {}, { logger: silentLogger }); + if (!roomDataHandler) throw new Error('SlidingSyncSdk did not subscribe to room data'); + + return { mx, deliver: roomDataHandler }; +}; + +const message = (id: string, ts: number) => ({ + type: 'm.room.message', + event_id: id, + sender: '@them:example.com', + origin_server_ts: ts, + content: { msgtype: 'm.text', body: id }, +}); + +/** What a list sends at timeline_limit: 1 the first time it sees a room. */ +const initialRoomData = (newest: ReturnType): MSC3575RoomData => + ({ + initial: true, + required_state: [], + timeline: [newest], + limited: true, + prev_batch: 't1-0', + }) as unknown as MSC3575RoomData; + +/** What Synapse sends for a raised timeline_limit: history from the top of the + * room, `initial` and `limited` both unset. */ +const expandedResponse = (timeline: ReturnType[]): MSC3575SlidingSyncResponse => + ({ + pos: 'p2', + rooms: { + [roomId]: { + unstable_expanded_timeline: true, + required_state: [], + timeline, + prev_batch: 't1-5', + }, + }, + }) as unknown as MSC3575SlidingSyncResponse; + +const timelineIds = (mx: MatrixClient): string[] => + mx + .getRoom(roomId)! + .getLiveTimeline() + .getEvents() + .map((event) => event.getId()!); + +const history = [message('$e1', 100), message('$e2', 200), message('$e3', 300)]; +const newest = history[history.length - 1]!; + +describe('expanded timeline handling in matrix-js-sdk', () => { + it('appends the expanded history out of order when only Synapse flags it', async () => { + const { mx, deliver } = makeSdk(); + await deliver(roomId, initialRoomData(newest)); + expect(timelineIds(mx)).toEqual(['$e3']); + + const resp = expandedResponse(history); + await deliver(roomId, resp.rooms[roomId]!); + + expect(timelineIds(mx)).toEqual(['$e3', '$e1', '$e2']); + }); + + it('reconciles the gap into a correctly ordered timeline once marked limited', async () => { + const { mx, deliver } = makeSdk(); + await deliver(roomId, initialRoomData(newest)); + + const resp = expandedResponse(history); + markExpandedTimelinesLimited(resp); + await deliver(roomId, resp.rooms[roomId]!); + + expect(timelineIds(mx)).toEqual(['$e1', '$e2', '$e3']); + }); + + it('keeps a usable back-pagination token after reconciling', async () => { + const { mx, deliver } = makeSdk(); + await deliver(roomId, initialRoomData(newest)); + + const resp = expandedResponse(history); + markExpandedTimelinesLimited(resp); + await deliver(roomId, resp.rooms[roomId]!); + + expect(mx.getRoom(roomId)!.getLiveTimeline().getPaginationToken(EventTimeline.BACKWARDS)).toBe( + 't1-5' + ); + }); +}); diff --git a/tests/e2e/fixtures/continuwuity.ts b/tests/e2e/fixtures/continuwuity.ts index cdb942818a..32fdedbe99 100644 --- a/tests/e2e/fixtures/continuwuity.ts +++ b/tests/e2e/fixtures/continuwuity.ts @@ -112,6 +112,23 @@ export async function sendText( return ((await res.json()) as { event_id: string }).event_id; } +export async function setRoomName( + baseUrl: string, + token: string, + roomId: string, + name: string +): Promise { + const url = `${baseUrl}/_matrix/client/v3/rooms/${encodeURIComponent(roomId)}/state/m.room.name/`; + const res = await fetch(url, { + method: 'PUT', + headers: { authorization: `Bearer ${token}` }, + body: JSON.stringify({ name }), + }); + if (!res.ok) { + throw new Error(`setRoomName failed: ${res.status} ${await res.text()}`); + } +} + export async function sendMessage( baseUrl: string, token: string, diff --git a/tests/e2e/sliding-sync-room-state.spec.ts b/tests/e2e/sliding-sync-room-state.spec.ts new file mode 100644 index 0000000000..05a1793e06 --- /dev/null +++ b/tests/e2e/sliding-sync-room-state.spec.ts @@ -0,0 +1,98 @@ +import { readFile } from 'node:fs/promises'; +import { test, expect, type Page } from '@playwright/test'; +import { createRoom, registerUser, sendText, setRoomName } from './fixtures/continuwuity'; +import { AppShell, CLIENT_READY_TIMEOUT } from './pages/AppShell'; + +const PASSWORD = 'test-passw0rd'; + +// More rooms than the window the joined list used to shrink to. +const FILLER_ROOM_COUNT = 5; + +type InjectedSession = { + baseUrl: string; + userId: string; + deviceId: string; + accessToken: string; + slidingSyncOptIn?: boolean; +}; + +async function homeserverBaseUrl(storageStatePath: string): Promise { + const state = JSON.parse(await readFile(storageStatePath, 'utf8')) as { + origins: { localStorage: { name: string; value: string }[] }[]; + }; + const entry = state.origins[0]!.localStorage.find((item) => item.name === 'matrixSessions')!; + return (JSON.parse(entry.value) as InjectedSession[])[0]!.baseUrl; +} + +async function loginAsFreshUser( + page: Page, + baseUrl: string, + name: string +): Promise<{ accessToken: string }> { + const user = await registerUser(baseUrl, name, PASSWORD); + const session: InjectedSession = { + baseUrl, + userId: user.userId, + deviceId: user.deviceId, + accessToken: user.accessToken, + slidingSyncOptIn: true, + }; + await page.addInitScript((injected: InjectedSession) => { + localStorage.setItem('matrixSessions', JSON.stringify([injected])); + localStorage.setItem('matrixActiveSession', JSON.stringify(injected.userId)); + localStorage.setItem('dismissNotice', 'true'); + }, session); + return user; +} + +test.describe('sliding sync room state', () => { + // Regression guard for #1161 / #1389. + test('applies a rename to a room that is neither open nor recently active', async ({ + page, + }, testInfo) => { + test.skip(testInfo.project.name !== 'desktop', 'desktop-focused'); + test.setTimeout(300_000); + const storageStatePath = testInfo.project.use.storageState as string; + const hsBaseUrl = await homeserverBaseUrl(storageStatePath); + const tag = `state-${process.pid}-${Date.now().toString(36)}`; + const app = new AppShell(page); + const user = await loginAsFreshUser(page, hsBaseUrl, `${tag}-u`); + + const staleName = `${tag} Stale`; + const renamedName = `${tag} Renamed`; + const stale = await createRoom(hsBaseUrl, user.accessToken, { + name: staleName, + preset: 'private_chat', + }); + + // Only these get recent activity, so the target sorts last by recency. + const active: string[] = []; + for (let i = 0; i < FILLER_ROOM_COUNT; i += 1) { + active.push( + // oxlint-disable-next-line no-await-in-loop + await createRoom(hsBaseUrl, user.accessToken, { + name: `${tag} Active ${i}`, + preset: 'private_chat', + }) + ); + } + for (let i = 0; i < active.length; i += 1) { + // oxlint-disable-next-line no-await-in-loop + await sendText(hsBaseUrl, user.accessToken, active[i]!, `${tag}-bump-${i}`, i + 1); + } + + await page.goto('/'); + await expect(app.room(staleName)).toBeVisible({ timeout: CLIENT_READY_TIMEOUT }); + + // An active subscription would fetch state regardless of the list config. + await app.openRoom(`${tag} Active 0`); + await expect(page.getByText(`${tag}-bump-0`, { exact: true })).toBeVisible({ + timeout: CLIENT_READY_TIMEOUT, + }); + + await setRoomName(hsBaseUrl, user.accessToken, stale, renamedName); + + await expect(app.room(renamedName)).toBeVisible({ timeout: CLIENT_READY_TIMEOUT }); + await expect(page.getByText(staleName, { exact: true })).toHaveCount(0); + }); +}); From ee0c6391c5a83acbe5c9496b006a27eaa6c744cd Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Mon, 3 Aug 2026 18:24:47 +0200 Subject: [PATCH 2/3] fix(sync): isolate sliding sync connections per client and gate on server support --- src/client/initMatrix.test.ts | 88 ++++++++++++++++++++++++++++++++++- src/client/initMatrix.ts | 56 ++++++++++++++++++++-- src/client/versionsCache.ts | 7 +++ 3 files changed, 144 insertions(+), 7 deletions(-) diff --git a/src/client/initMatrix.test.ts b/src/client/initMatrix.test.ts index db141dd664..a73c3ac832 100644 --- a/src/client/initMatrix.test.ts +++ b/src/client/initMatrix.test.ts @@ -1,7 +1,8 @@ -import { beforeEach, describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { MatrixClient } from '$types/matrix-sdk'; import type { Session } from '$state/sessions'; import { ACTIVE_SESSION_KEY, MATRIX_SESSIONS_KEY } from '$state/sessions'; -import { ownsActiveMediaSession } from './initMatrix'; +import { newSlidingSyncConnId, ownsActiveMediaSession, supportsSlidingSync } from './initMatrix'; const alice = { userId: '@alice:example.org' } as Session; const bob = { userId: '@bob:example.org' } as Session; @@ -24,3 +25,86 @@ describe('ownsActiveMediaSession', () => { expect(ownsActiveMediaSession(alice)).toBe(true); }); }); + +describe('newSlidingSyncConnId', () => { + it('gives every client instance its own id', () => { + const ids = new Set(Array.from({ length: 50 }, () => newSlidingSyncConnId())); + + expect(ids.size).toBe(50); + }); +}); + +describe('supportsSlidingSync', () => { + const baseUrl = 'https://matrix.example.org'; + const versionsKey = `sable.versionsCache.${baseUrl}|${alice.userId}`; + + const makeMx = ( + doesServerSupportUnstableFeature: (feature: string) => Promise + ): MatrixClient => + ({ + doesServerSupportUnstableFeature, + getSafeUserId: () => alice.userId, + }) as unknown as MatrixClient; + + beforeEach(() => { + localStorage.clear(); + }); + + const cacheFeature = (supported: boolean) => + localStorage.setItem( + versionsKey, + JSON.stringify({ + versions: [], + unstable_features: { 'org.matrix.simplified_msc3575': supported }, + fetchedAt: Date.now(), + }) + ); + + it('reports support when the homeserver advertises simplified sliding sync', async () => { + const check = vi.fn<(feature: string) => Promise>().mockResolvedValue(true); + + await expect(supportsSlidingSync(makeMx(check), baseUrl)).resolves.toBe(true); + expect(check).toHaveBeenCalledWith('org.matrix.simplified_msc3575'); + }); + + it('reports no support when the homeserver does not advertise it', async () => { + await expect( + supportsSlidingSync( + makeMx(() => Promise.resolve(false)), + baseUrl + ) + ).resolves.toBe(false); + }); + + // Classic sync still works; opting in against a server that cannot serve it does not. + it('falls back to classic sync when the capability was never confirmed', async () => { + await expect( + supportsSlidingSync( + makeMx(() => Promise.reject(new Error('offline'))), + baseUrl + ) + ).resolves.toBe(false); + }); + + it('keeps sliding sync when a previously confirmed capability is cached', async () => { + cacheFeature(true); + + await expect( + supportsSlidingSync( + makeMx(() => Promise.reject(new Error('offline'))), + baseUrl + ) + ).resolves.toBe(true); + }); + + it('does not resurrect sliding sync from a cached negative', async () => { + cacheFeature(false); + + await expect( + supportsSlidingSync( + makeMx(() => Promise.reject(new Error('offline'))), + baseUrl + ) + ).resolves.toBe(false); + }); +}); diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index 74a4967491..d363bf3d4b 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -32,7 +32,11 @@ import { assertAuthMetadataIssuer, createSessionTokenRefresher } from './oidcTok import { revokeOAuthToken } from './oauthTokenRevocation'; import { clearSecretStorageKeys, cryptoCallbacks } from './secretStorageKeys'; import type { SlidingSyncDiagnostics } from './slidingSync'; -import { scopeEphemeralExtensions, SlidingSyncManager } from './slidingSync'; +import { + markExpandedTimelinesLimited, + scopeTypingExtension, + SlidingSyncManager, +} from './slidingSync'; import { PresenceSyncManager } from './presenceSync'; import { SlidingSyncSidebarCache } from './slidingSyncSidebarCache'; import { clearCachedUserProfiles } from './userProfileCache'; @@ -41,6 +45,7 @@ import { revalidateVersionsCache, clearCachedVersions, cacheVersionsFromClient, + wasUnstableFeatureCached, } from './versionsCache'; const log = createLogger('initMatrix'); @@ -139,24 +144,29 @@ type SlidingSyncRequestWithConnId = MSC3575SlidingSyncRequest & { conn_id?: string; }; -const SLIDING_SYNC_CONN_ID = 'sable-main'; +// Synapse keys connection state on (user, device, conn_id) and keeps only the two +// latest positions, so two clients sharing an id invalidate each other's `pos`. +export const newSlidingSyncConnId = (): string => + `sable-${globalThis.crypto?.randomUUID?.().slice(0, 8) ?? `${Date.now()}-${Math.random().toString(16).slice(2)}`}`; function installSlidingSyncRequestPatch(mx: MatrixClient, manager: SlidingSyncManager): void { slidingSyncRequestCleanupByClient.get(mx)?.(); + const connId = newSlidingSyncConnId(); const mxWritable = mx as MatrixClientWithWritableSlidingSync; const original = mx.slidingSync.bind(mx) as SlidingSyncMethod; mxWritable.slidingSync = async (reqBody, baseUrl, abortSignal) => { const req = reqBody as SlidingSyncRequestWithConnId; if (req.conn_id === undefined) { - req.conn_id = SLIDING_SYNC_CONN_ID; + req.conn_id = connId; } const roomIds = manager.getActiveRoomSubscriptionIds(); - scopeEphemeralExtensions(req.extensions, roomIds); + scopeTypingExtension(req.extensions, roomIds); // Must run before the SDK processes the response. const response = await original(reqBody, baseUrl, abortSignal); + markExpandedTimelinesLimited(response); manager.sanitizeOptimisticJoinResponse(response); return response; }; @@ -365,6 +375,34 @@ export type ClientSyncDiagnostics = { sliding?: SlidingSyncDiagnostics; }; +const SLIDING_SYNC_UNSTABLE_FEATURE = 'org.matrix.simplified_msc3575'; + +const SLIDING_SYNC_CAPABILITY_TIMEOUT_MS = 5000; + +// The SDK retries an unsupported sync endpoint forever instead of erroring, so an +// unconfirmed capability falls back to classic sync. A cached confirmation keeps +// sliding sync through one bad lookup. /versions has no timeout of its own. +export const supportsSlidingSync = async (mx: MatrixClient, baseUrl: string): Promise => { + const timeout = new Promise((resolve) => { + globalThis.setTimeout(() => resolve(undefined), SLIDING_SYNC_CAPABILITY_TIMEOUT_MS); + }); + + let confirmed: boolean | undefined; + try { + confirmed = await Promise.race([ + mx.doesServerSupportUnstableFeature(SLIDING_SYNC_UNSTABLE_FEATURE), + timeout, + ]); + } catch { + confirmed = undefined; + } + + return ( + confirmed ?? + wasUnstableFeatureCached(baseUrl, mx.getSafeUserId(), SLIDING_SYNC_UNSTABLE_FEATURE) + ); +}; + const disposeSlidingSync = (mx: MatrixClient): void => { membershipActionCleanupByClient.get(mx)?.(); const manager = slidingSyncByClient.get(mx); @@ -428,7 +466,15 @@ export const startClient = async (mx: MatrixClient, config?: StartClientConfig): disposePresenceSync(mx); const baseUrl = config?.baseUrl ?? mx.baseUrl; - const useSliding = config?.sessionSlidingSyncOptIn === true; + const optedIntoSliding = config?.sessionSlidingSyncOptIn === true; + const useSliding = optedIntoSliding && (await supportsSlidingSync(mx, baseUrl)); + + if (optedIntoSliding && !useSliding) { + debugLog.warn('sync', 'Homeserver does not support sliding sync; using classic sync', { + userId: mx.getUserId(), + baseUrl, + }); + } debugLog.info('sync', 'Starting Matrix client', { userId: mx.getUserId() }); diff --git a/src/client/versionsCache.ts b/src/client/versionsCache.ts index 0953a682ed..9726f57ec0 100644 --- a/src/client/versionsCache.ts +++ b/src/client/versionsCache.ts @@ -113,6 +113,13 @@ export const revalidateVersionsCache = async ( } }; +/** Separates "the server does not support this" from "we could not ask". */ +export const wasUnstableFeatureCached = ( + baseUrl: string, + userId: string, + feature: string +): boolean => readCache(baseUrl, userId)?.unstable_features?.[feature] === true; + /** Clear the cached versions for a session (used on logout). */ export const clearCachedVersions = (baseUrl: string, userId: string): void => { try { From 6bc2b766c2a4c5b60dbad69da495bfa84af59386 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Mon, 3 Aug 2026 20:16:03 +0200 Subject: [PATCH 3/3] fix(sync): keep response corrections from wedging the sync loop --- src/app/utils/notifications.test.ts | 4 +- src/client/initMatrix.test.ts | 14 +++-- src/client/initMatrix.ts | 56 ++++++++++++++----- src/client/slidingSync.ts | 8 +-- .../slidingSyncExpandedTimeline.test.ts | 4 +- src/client/versionsCache.ts | 2 +- tests/e2e/sliding-sync-room-state.spec.ts | 11 +++- 7 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/app/utils/notifications.test.ts b/src/app/utils/notifications.test.ts index 9053200010..ded5ab1beb 100644 --- a/src/app/utils/notifications.test.ts +++ b/src/app/utils/notifications.test.ts @@ -45,8 +45,8 @@ const makeMx = (events: MatrixEvent[], readUpTo: string | null) => { }; }; -// The live timeline is in timeline order by contract, so the newest event is last. -// Ordering defects belong in markExpandedTimelinesLimited, not here. +// The live timeline is in timeline order, so the newest event is last. Ordering +// defects belong in the sliding-sync layer, not here. describe('markAsRead', () => { it('marks read up to the last event in the timeline', async () => { const { mx, setRoomReadMarkers, sendReadReceipt } = makeMx( diff --git a/src/client/initMatrix.test.ts b/src/client/initMatrix.test.ts index a73c3ac832..4c0aca6d3d 100644 --- a/src/client/initMatrix.test.ts +++ b/src/client/initMatrix.test.ts @@ -63,7 +63,10 @@ describe('supportsSlidingSync', () => { it('reports support when the homeserver advertises simplified sliding sync', async () => { const check = vi.fn<(feature: string) => Promise>().mockResolvedValue(true); - await expect(supportsSlidingSync(makeMx(check), baseUrl)).resolves.toBe(true); + await expect(supportsSlidingSync(makeMx(check), baseUrl)).resolves.toEqual({ + supported: true, + reason: 'advertised', + }); expect(check).toHaveBeenCalledWith('org.matrix.simplified_msc3575'); }); @@ -73,17 +76,18 @@ describe('supportsSlidingSync', () => { makeMx(() => Promise.resolve(false)), baseUrl ) - ).resolves.toBe(false); + ).resolves.toEqual({ supported: false, reason: 'unadvertised' }); }); // Classic sync still works; opting in against a server that cannot serve it does not. + // The reason must stay distinguishable: we never established the server's answer. it('falls back to classic sync when the capability was never confirmed', async () => { await expect( supportsSlidingSync( makeMx(() => Promise.reject(new Error('offline'))), baseUrl ) - ).resolves.toBe(false); + ).resolves.toEqual({ supported: false, reason: 'unknown' }); }); it('keeps sliding sync when a previously confirmed capability is cached', async () => { @@ -94,7 +98,7 @@ describe('supportsSlidingSync', () => { makeMx(() => Promise.reject(new Error('offline'))), baseUrl ) - ).resolves.toBe(true); + ).resolves.toEqual({ supported: true, reason: 'cached' }); }); it('does not resurrect sliding sync from a cached negative', async () => { @@ -105,6 +109,6 @@ describe('supportsSlidingSync', () => { makeMx(() => Promise.reject(new Error('offline'))), baseUrl ) - ).resolves.toBe(false); + ).resolves.toEqual({ supported: false, reason: 'unknown' }); }); }); diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index d363bf3d4b..470812056b 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -164,10 +164,18 @@ function installSlidingSyncRequestPatch(mx: MatrixClient, manager: SlidingSyncMa const roomIds = manager.getActiveRoomSubscriptionIds(); scopeTypingExtension(req.extensions, roomIds); - // Must run before the SDK processes the response. const response = await original(reqBody, baseUrl, abortSignal); - markExpandedTimelinesLimited(response); - manager.sanitizeOptimisticJoinResponse(response); + // Must run before the SDK processes the response. A throw would reach the SDK's + // loop, which drops the response and retries the same `pos` forever. + try { + markExpandedTimelinesLimited(response); + manager.sanitizeOptimisticJoinResponse(response); + } catch (error) { + Sentry.captureException(error, { tags: { area: 'sliding_sync_response' } }); + debugLog.error('sync', 'Failed to prepare sliding sync response', { + error: error instanceof Error ? error.message : String(error), + }); + } return response; }; @@ -379,12 +387,19 @@ const SLIDING_SYNC_UNSTABLE_FEATURE = 'org.matrix.simplified_msc3575'; const SLIDING_SYNC_CAPABILITY_TIMEOUT_MS = 5000; -// The SDK retries an unsupported sync endpoint forever instead of erroring, so an -// unconfirmed capability falls back to classic sync. A cached confirmation keeps -// sliding sync through one bad lookup. /versions has no timeout of its own. -export const supportsSlidingSync = async (mx: MatrixClient, baseUrl: string): Promise => { +// The SDK retries an unsupported endpoint forever instead of erroring, so anything +// short of a confirmation falls back to classic sync. /versions has no timeout of +// its own, hence the race. +export const supportsSlidingSync = async ( + mx: MatrixClient, + baseUrl: string +): Promise<{ + supported: boolean; + reason: 'advertised' | 'unadvertised' | 'cached' | 'unknown'; +}> => { + let timer: ReturnType | undefined; const timeout = new Promise((resolve) => { - globalThis.setTimeout(() => resolve(undefined), SLIDING_SYNC_CAPABILITY_TIMEOUT_MS); + timer = globalThis.setTimeout(() => resolve(undefined), SLIDING_SYNC_CAPABILITY_TIMEOUT_MS); }); let confirmed: boolean | undefined; @@ -395,12 +410,20 @@ export const supportsSlidingSync = async (mx: MatrixClient, baseUrl: string): Pr ]); } catch { confirmed = undefined; + } finally { + globalThis.clearTimeout(timer); } - return ( - confirmed ?? - wasUnstableFeatureCached(baseUrl, mx.getSafeUserId(), SLIDING_SYNC_UNSTABLE_FEATURE) + if (confirmed !== undefined) { + return { supported: confirmed, reason: confirmed ? 'advertised' : 'unadvertised' }; + } + // We never reached /versions, so a previous confirmation is all we have to go on. + const cached = wasUnstableFeatureCached( + baseUrl, + mx.getSafeUserId(), + SLIDING_SYNC_UNSTABLE_FEATURE ); + return { supported: cached, reason: cached ? 'cached' : 'unknown' }; }; const disposeSlidingSync = (mx: MatrixClient): void => { @@ -467,12 +490,19 @@ export const startClient = async (mx: MatrixClient, config?: StartClientConfig): const baseUrl = config?.baseUrl ?? mx.baseUrl; const optedIntoSliding = config?.sessionSlidingSyncOptIn === true; - const useSliding = optedIntoSliding && (await supportsSlidingSync(mx, baseUrl)); + const slidingSupport = optedIntoSliding + ? await supportsSlidingSync(mx, baseUrl) + : { supported: false, reason: 'unadvertised' as const }; + const useSliding = optedIntoSliding && slidingSupport.supported; if (optedIntoSliding && !useSliding) { - debugLog.warn('sync', 'Homeserver does not support sliding sync; using classic sync', { + debugLog.warn('sync', 'Falling back to classic sync', { userId: mx.getUserId(), baseUrl, + reason: slidingSupport.reason, + }); + Sentry.metrics.count('sable.sync.transport_downgrade', 1, { + attributes: { reason: slidingSupport.reason }, }); } diff --git a/src/client/slidingSync.ts b/src/client/slidingSync.ts index 36e31cc0b3..d95047f5e5 100644 --- a/src/client/slidingSync.ts +++ b/src/client/slidingSync.ts @@ -284,8 +284,7 @@ type ExpandedTimelineRoomData = MSC3575RoomData & { unstable_expanded_timeline?: // Synapse flags history re-sent for a raised timeline_limit only as // `unstable_expanded_timeline`, but the SDK reconciles a gap on `limited`/`initial`, -// so without this it appends that history after the newest event. Drop once the SDK -// reads the flag. +// so without this it lands after the newest event. Drop once the SDK reads the flag. export const markExpandedTimelinesLimited = (resp: MSC3575SlidingSyncResponse | null): void => { if (!resp?.rooms) return; @@ -981,9 +980,8 @@ export class SlidingSyncManager { } } - // Paging stops once every list is covered, but the counts keep growing. A state - // change does not bump a room, so a room joined mid-session would otherwise sit - // outside the window and stop receiving state deltas until the next restart. + // Paging stops once every list is covered, but the counts keep growing, and a state + // change does not bump a room back into the window. private ensureListCoverage(): void { if (!this.initialListHydrationCompleted) return; diff --git a/src/client/slidingSyncExpandedTimeline.test.ts b/src/client/slidingSyncExpandedTimeline.test.ts index 15f61ba57e..5f699e8d37 100644 --- a/src/client/slidingSyncExpandedTimeline.test.ts +++ b/src/client/slidingSyncExpandedTimeline.test.ts @@ -5,8 +5,8 @@ import type { Logger } from 'matrix-js-sdk/lib/logger'; import { createClient, EventTimeline } from '$types/matrix-sdk'; import { markExpandedTimelinesLimited } from './slidingSync'; -// Drives the real matrix-js-sdk room-data path, so this pins the SDK's actual gap -// reconciliation rather than our assumptions about it. +// Drives the real matrix-js-sdk room-data path, pinning the SDK's actual behaviour +// rather than our assumptions about it. const userId = '@me:example.com'; const roomId = '!dm:example.com'; diff --git a/src/client/versionsCache.ts b/src/client/versionsCache.ts index 9726f57ec0..ba0a3084d2 100644 --- a/src/client/versionsCache.ts +++ b/src/client/versionsCache.ts @@ -113,7 +113,7 @@ export const revalidateVersionsCache = async ( } }; -/** Separates "the server does not support this" from "we could not ask". */ +/** True only when a cached /versions payload advertised the feature. */ export const wasUnstableFeatureCached = ( baseUrl: string, userId: string, diff --git a/tests/e2e/sliding-sync-room-state.spec.ts b/tests/e2e/sliding-sync-room-state.spec.ts index 05a1793e06..eeb3eba48e 100644 --- a/tests/e2e/sliding-sync-room-state.spec.ts +++ b/tests/e2e/sliding-sync-room-state.spec.ts @@ -46,7 +46,8 @@ async function loginAsFreshUser( } test.describe('sliding sync room state', () => { - // Regression guard for #1161 / #1389. + // Regression guard for #1389: reintroducing the post-hydration narrowing of the + // joined list fails this. Rooms outside the server's own window are not covered. test('applies a rename to a room that is neither open nor recently active', async ({ page, }, testInfo) => { @@ -81,7 +82,15 @@ test.describe('sliding sync room state', () => { await sendText(hsBaseUrl, user.accessToken, active[i]!, `${tag}-bump-${i}`, i + 1); } + // Proves the client chose sliding sync and the homeserver accepted the request. + const slidingSyncAccepted = page.waitForResponse( + (response) => + response.url().includes('/org.matrix.simplified_msc3575/sync') && response.status() === 200, + { timeout: CLIENT_READY_TIMEOUT } + ); + await page.goto('/'); + await slidingSyncAccepted; await expect(app.room(staleName)).toBeVisible({ timeout: CLIENT_READY_TIMEOUT }); // An active subscription would fetch state regardless of the list config.