Fix/sliding sync room state and unread

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<void>
+    >()
+    .mockResolvedValue();
+  const sendReadReceipt = vi
+    .fn<(event: MatrixEvent, receiptType: ReceiptType) => Promise<void>>()
+    .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, 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(
+      [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();
+  });
+});

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,90 @@ 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<boolean>
+  ): 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<boolean>>().mockResolvedValue(true);
+
+    await expect(supportsSlidingSync(makeMx(check), baseUrl)).resolves.toEqual({
+      supported: true,
+      reason: 'advertised',
+    });
+    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.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.toEqual({ supported: false, reason: 'unknown' });
+  });
+
+  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.toEqual({ supported: true, reason: 'cached' });
+  });
+
+  it('does not resurrect sliding sync from a cached negative', async () => {
+    cacheFeature(false);
+
+    await expect(
+      supportsSlidingSync(
+        makeMx(() => Promise.reject(new Error('offline'))),
+        baseUrl
+      )
+    ).resolves.toEqual({ supported: false, reason: 'unknown' });
+  });
+});

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,25 +144,38 @@ 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);
-    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;
   };
 
@@ -365,6 +383,49 @@ 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 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<typeof globalThis.setTimeout> | undefined;
+  const timeout = new Promise<boolean | undefined>((resolve) => {
+    timer = 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;
+  } finally {
+    globalThis.clearTimeout(timer);
+  }
+
+  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 => {
   membershipActionCleanupByClient.get(mx)?.();
   const manager = slidingSyncByClient.get(mx);
@@ -428,7 +489,22 @@ 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 slidingSupport = optedIntoSliding
+    ? await supportsSlidingSync(mx, baseUrl)
+    : { supported: false, reason: 'unadvertised' as const };
+  const useSliding = optedIntoSliding && slidingSupport.supported;
+
+  if (optedIntoSliding && !useSliding) {
+    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 },
+    });
+  }
 
   debugLog.info('sync', 'Starting Matrix client', { userId: mx.getUserId() });
 

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<void> {
     const crypto = this.mx.getCrypto() as CryptoBackend | undefined;
     if (!crypto) return;