fix(dm): resolve bridged DM metadata

src/app/hooks/useRoomMeta.test.tsx

@@ -1,11 +1,13 @@
 import { describe, it, expect } from 'vitest';
 import { renderHook, act } from '@testing-library/react';
 import { EventEmitter } from 'events';
+import { Provider as JotaiProvider, createStore } from 'jotai';
 import type { PropsWithChildren } from 'react';
 import type { MatrixClient, MatrixEvent, Room } from '$types/matrix-sdk';
-import { RoomStateEvent } from '$types/matrix-sdk';
+import { EventType, RoomStateEvent } from '$types/matrix-sdk';
+import { mDirectAtom } from '$state/mDirectList';
 import { MatrixClientProvider } from './useMatrixClient';
-import { useRoomAvatar } from './useRoomMeta';
+import { useRoomAvatar, useRoomName } from './useRoomMeta';
 
 const AVATAR_MXC = 'mxc://server/abc';
 
@@ -74,34 +76,147 @@ describe('useRoomAvatar', () => {
   });
 
   it('updates a DM avatar when its member state arrives after the sidebar rendered', () => {
-    const member = {
-      getMxcAvatarUrl: () => AVATAR_MXC,
-      membership: 'join',
-      userId: '@alice:server',
-    };
-    const { room, client, wrapper } = makeRoom('!dm:server');
+    const { room, wrapper } = makeRoom('!dm:server');
     const roomEvents = new EventEmitter();
-    let members = [{ membership: 'join', userId: '@me:server' }];
+    let fallbackMember: { getMxcAvatarUrl: () => string } | undefined;
     Object.assign(room, {
-      getMember: (userId: string) => members.find((roomMember) => roomMember.userId === userId),
-      getMembers: () => members,
-      getAvatarFallbackMember: () => undefined,
+      getAvatarFallbackMember: () => fallbackMember,
       on: roomEvents.on.bind(roomEvents),
       removeListener: roomEvents.removeListener.bind(roomEvents),
     });
-    Object.assign(client, {
-      getAccountData: () => ({ getContent: () => ({ '@alice:server': [room.roomId] }) }),
-      getUserId: () => '@me:server',
-    });
 
     const { result } = renderHook(() => useRoomAvatar(room, true), { wrapper });
     expect(result.current).toBeUndefined();
 
-    members = [{ membership: 'join', userId: '@me:server' }, member];
+    fallbackMember = { getMxcAvatarUrl: () => AVATAR_MXC };
     act(() => {
       roomEvents.emit(RoomStateEvent.Members);
     });
 
     expect(result.current).toBe(AVATAR_MXC);
   });
 });
+
+const makeNameRoom = (
+  roomId: string,
+  {
+    mDirects = {},
+    members = {},
+    fallbackMember,
+    functionalMemberIds = [],
+    name = 'Room',
+    nameEventId,
+  }: {
+    mDirects?: Record<string, string[]>;
+    members?: Record<string, { rawDisplayName?: string; membership?: string }>;
+    fallbackMember?: { userId: string; rawDisplayName?: string; membership?: string };
+    functionalMemberIds?: string[];
+    name?: string;
+    nameEventId?: string;
+  }
+) => {
+  const client = new EventEmitter();
+  const stateEvents: Record<string, MatrixEvent | undefined> = {
+    [EventType.RoomName]: nameEventId
+      ? ({
+          getId: () => nameEventId,
+          getContent: () => ({ name }),
+        } as unknown as MatrixEvent)
+      : undefined,
+    'io.element.functional_members': functionalMemberIds.length
+      ? ({ getContent: () => ({ service_members: functionalMemberIds }) } as unknown as MatrixEvent)
+      : undefined,
+  };
+  const room = {
+    roomId,
+    client,
+    name,
+    guessDMUserId: () => '',
+    recalculate: () => {},
+    getLiveTimeline: () => ({
+      getState: () => ({ getStateEvents: (type: string) => stateEvents[type] }),
+    }),
+    getMember: (userId: string) => members[userId],
+    getMembers: () => [{ userId: '@me:server', membership: 'join' }, ...Object.values(members)],
+    getAvatarFallbackMember: () => fallbackMember,
+    on: client.on.bind(client),
+    removeListener: client.removeListener.bind(client),
+  } as unknown as Room;
+  Object.assign(client, {
+    getAccountData: () => ({ getContent: () => mDirects }),
+    getUserId: () => '@me:server',
+  });
+
+  const store = createStore();
+  store.set(mDirectAtom, { type: 'INITIALIZE', rooms: new Set(Object.values(mDirects).flat()) });
+
+  const wrapper = ({ children }: PropsWithChildren) => (
+    <JotaiProvider store={store}>
+      <MatrixClientProvider value={client as unknown as MatrixClient}>
+        {children}
+      </MatrixClientProvider>
+    </JotaiProvider>
+  );
+  return { room, client, wrapper };
+};
+
+describe('useRoomName', () => {
+  const fakeNameEventId = '$fake-sliding-sync-name-event-!dm:server';
+
+  it('prefers the counterpart over the sliding-sync server name for a bridged DM', () => {
+    const { room, wrapper } = makeNameRoom('!dm:server', {
+      mDirects: { '@alice:server': ['!dm:server'] },
+      members: {
+        '@alice:server': { rawDisplayName: 'Alexia', membership: 'join' },
+        '@signal-service:server': { membership: 'join' },
+      },
+      functionalMemberIds: ['@signal-service:server'],
+      name: 'Alexia, Signal Bridge bot',
+      nameEventId: fakeNameEventId,
+    });
+
+    const { result } = renderHook(() => useRoomName(room), { wrapper });
+
+    expect(result.current).toBe('Alexia');
+  });
+
+  it('falls back to the hero member name when sliding sync has no member state', () => {
+    const { room, wrapper } = makeNameRoom('!dm:server', {
+      mDirects: { '@alice:server': ['!dm:server'] },
+      functionalMemberIds: ['@signal-service:server'],
+      fallbackMember: { userId: '@alice:server', rawDisplayName: 'Alexia', membership: 'join' },
+      name: 'Alexia, Signal Bridge bot',
+      nameEventId: fakeNameEventId,
+    });
+
+    const { result } = renderHook(() => useRoomName(room), { wrapper });
+
+    expect(result.current).toBe('Alexia');
+  });
+
+  it('keeps a genuine m.room.name on a DM', () => {
+    const { room, wrapper } = makeNameRoom('!dm:server', {
+      mDirects: { '@alice:server': ['!dm:server'] },
+      members: {
+        '@alice:server': { rawDisplayName: 'Alexia', membership: 'join' },
+      },
+      name: 'Movie Club',
+      nameEventId: '$real-name-event:server',
+    });
+
+    const { result } = renderHook(() => useRoomName(room), { wrapper });
+
+    expect(result.current).toBe('Movie Club');
+  });
+
+  it('keeps the sliding-sync server name for non-DM rooms', () => {
+    const { room, wrapper } = makeNameRoom('!room:server', {
+      name: 'Bridge Lobby',
+      nameEventId: '$fake-sliding-sync-name-event-!room:server',
+    });
+
+    const { result } = renderHook(() => useRoomName(room), { wrapper });
+
+    expect(result.current).toBe('Bridge Lobby');
+  });
+});

src/app/hooks/useRoomMeta.ts

@@ -1,6 +1,6 @@
 import { useAtomValue } from 'jotai';
 import { useEffect, useState } from 'react';
-import type { RoomJoinRulesEventContent, Room } from '$types/matrix-sdk';
+import type { MatrixEvent, RoomJoinRulesEventContent, Room } from '$types/matrix-sdk';
 import { RoomEvent, RoomStateEvent, EventType } from '$types/matrix-sdk';
 
 import { mDirectAtom } from '$state/mDirectList';
@@ -9,21 +9,26 @@ import { useMatrixClient } from './useMatrixClient';
 import { useStateEvent } from './useStateEvent';
 import { useNickname } from './useNickname';
 
+// Sliding sync's ensureNameEvent injects the server-computed room name with this
+// fake event id; it counts bridge bots, so DMs rank it below the member name.
+const isServerGeneratedRoomName = (event?: MatrixEvent): boolean =>
+  event?.getId()?.startsWith('$fake-sliding-sync-name-event-') === true;
+
 const getRoomDisplayName = (
   roomName: string,
   stateName: unknown,
+  stateNameIsServerGenerated: boolean,
   isDmTagged: boolean,
   dmNickname?: string,
   dmOtherMemberName?: string
 ): string => {
   if (isDmTagged && dmNickname) return dmNickname;
-  if (typeof stateName === 'string' && stateName) return stateName;
+  if (typeof stateName === 'string' && stateName && !stateNameIsServerGenerated) return stateName;
   if (isDmTagged && dmOtherMemberName) return dmOtherMemberName;
   return roomName;
 };
 
 export const useRoomAvatar = (room: Room, dm?: boolean): string | undefined => {
-  const mx = useMatrixClient();
   const avatarEvent = useStateEvent(room, EventType.RoomAvatar);
   const [, refreshDmAvatar] = useState(0);
 
@@ -39,7 +44,7 @@ export const useRoomAvatar = (room: Room, dm?: boolean): string | undefined => {
   }, [room, dm]);
 
   if (dm) {
-    return getDmOtherMember(mx, room)?.getMxcAvatarUrl();
+    return room.getAvatarFallbackMember()?.getMxcAvatarUrl();
   }
   const content = avatarEvent?.getContent();
   const avatarMxc = content && typeof content.url === 'string' ? content.url : undefined;
@@ -49,28 +54,30 @@ export const useRoomAvatar = (room: Room, dm?: boolean): string | undefined => {
 
 export const useRoomName = (room: Room): string => {
   const mx = useMatrixClient();
-  const dmUserId = room.guessDMUserId();
-  const dmNickname = useNickname(dmUserId || '');
   const mDirects = useAtomValue(mDirectAtom);
   const isDmTagged = mDirects.has(room.roomId);
+  const dmUserId = room.guessDMUserId();
+  const dmNickname = useNickname(dmUserId || '');
   const nameEvent = useStateEvent(room, EventType.RoomName);
   const stateName = nameEvent?.getContent().name;
+  const stateNameIsServerGenerated = isServerGeneratedRoomName(nameEvent);
   const [name, setName] = useState(room.name);
 
   useEffect(() => {
     const updateName = () => {
-      if (room.name === 'Empty room') {
+      if (isDmTagged || room.name === 'Empty room') {
         room.recalculate();
       }
 
       const otherMember = isDmTagged ? getDmOtherMember(mx, room) : undefined;
       const dmOtherMemberName = otherMember
-        ? (getMemberDisplayName(room, otherMember.userId) ?? otherMember.userId)
+        ? (getMemberDisplayName(room, otherMember.userId) ?? otherMember.rawDisplayName)
         : undefined;
 
       const nextName = getRoomDisplayName(
         room.name,
         stateName,
+        stateNameIsServerGenerated,
         isDmTagged,
         dmNickname,
         dmOtherMemberName
@@ -87,7 +94,7 @@ export const useRoomName = (room: Room): string => {
       room.removeListener(RoomEvent.Name, updateName);
       room.removeListener(RoomStateEvent.Members, updateName);
     };
-  }, [room, mx, stateName, dmNickname, isDmTagged]);
+  }, [room, mx, stateName, stateNameIsServerGenerated, dmNickname, isDmTagged]);
 
   return name;
 };

src/app/utils/room/directMember.test.ts

@@ -0,0 +1,52 @@
+import { describe, expect, it } from 'vitest';
+import { isFunctionalMember, resolveDmOtherMember } from './directMember';
+
+const me = { membership: 'join', userId: '@me:example.org' };
+const alice = { membership: 'join', userId: '@alice:example.org' };
+const bridge = { membership: 'join', userId: '@signal-service:example.org' };
+
+describe('resolveDmOtherMember', () => {
+  it('prefers the sole active m.direct member', () => {
+    expect(
+      resolveDmOtherMember({
+        currentUserId: me.userId,
+        directMembers: [alice],
+        members: [me, alice, bridge],
+        functionalMemberIds: [],
+        fallbackMember: bridge,
+      })
+    ).toBe(alice);
+  });
+
+  it('excludes functional members from m.direct and membership fallbacks', () => {
+    expect(
+      resolveDmOtherMember({
+        currentUserId: me.userId,
+        directMembers: [bridge],
+        members: [me, alice, bridge],
+        functionalMemberIds: [bridge.userId],
+        fallbackMember: bridge,
+      })
+    ).toBe(alice);
+  });
+
+  it('does not infer service membership from an MXID', () => {
+    const alicebot = { membership: 'join', userId: '@alicebot:example.org' };
+
+    expect(
+      resolveDmOtherMember({
+        currentUserId: me.userId,
+        directMembers: [alicebot],
+        members: [me, alicebot],
+        functionalMemberIds: [],
+      })
+    ).toBe(alicebot);
+  });
+});
+
+describe('isFunctionalMember', () => {
+  it('can be reused by other member surfaces', () => {
+    expect(isFunctionalMember(bridge, [bridge.userId])).toBe(true);
+    expect(isFunctionalMember(alice, [bridge.userId])).toBe(false);
+  });
+});

src/app/utils/room/directMember.ts

@@ -0,0 +1,41 @@
+export type DirectRoomMember = {
+  userId: string;
+  membership?: string;
+};
+
+type ResolveDmOtherMemberInput<Member extends DirectRoomMember> = {
+  currentUserId: string | null;
+  directMembers: Member[];
+  members: Member[];
+  functionalMemberIds: string[];
+  fallbackMember?: Member;
+};
+
+export const isFunctionalMember = (
+  member: DirectRoomMember,
+  functionalMemberIds: readonly string[]
+): boolean => functionalMemberIds.includes(member.userId);
+
+const isActiveMember = (member: DirectRoomMember): boolean =>
+  member.membership === 'join' || member.membership === 'invite';
+
+export const resolveDmOtherMember = <Member extends DirectRoomMember>({
+  currentUserId,
+  directMembers,
+  members,
+  functionalMemberIds,
+  fallbackMember,
+}: ResolveDmOtherMemberInput<Member>): Member | undefined => {
+  const isFunctional = (member: DirectRoomMember) =>
+    isFunctionalMember(member, functionalMemberIds);
+
+  const directMember = directMembers.filter((member) => !isFunctional(member));
+  if (directMember.length === 1 && isActiveMember(directMember[0]!)) return directMember[0];
+
+  const otherMembers = members.filter(
+    (member) => member.userId !== currentUserId && !isFunctional(member) && isActiveMember(member)
+  );
+  if (otherMembers.length === 1) return otherMembers[0];
+
+  return fallbackMember && !isFunctional(fallbackMember) ? fallbackMember : undefined;
+};