From eaebed0e389dc90501a490ea47763e525ca03471 Mon Sep 17 00:00:00 2001 From: peter <251383182+private-peter@users.noreply.github.com> Date: Wed, 25 Mar 2026 19:41:34 -0700 Subject: [PATCH] fix(matrix): only use 2-member DM fallback when dm refresh fails Issue #54772 reports that two separate 2-person Matrix rooms with the same participants can both get routed as DMs. The regression comes from treating strict 2-member membership as a general fallback even when the DM cache is available and says the room is not a DM. Move the catch from refreshDmCache() into isDirectMessage(). When the refresh succeeds, keep the existing client.dms.isDm(roomId) && isStrictDirectMembership(...) gate so non-DM 2-person rooms stay grouped. Only when the refresh fails do we fall back to the exact 2-member heuristic and keep the warning log. --- .../matrix/src/matrix/monitor/direct.test.ts | 186 +++++++++--------- .../matrix/src/matrix/monitor/direct.ts | 48 +++-- 2 files changed, 116 insertions(+), 118 deletions(-) diff --git a/extensions/matrix/src/matrix/monitor/direct.test.ts b/extensions/matrix/src/matrix/monitor/direct.test.ts index e7250683a97..95528da110d 100644 --- a/extensions/matrix/src/matrix/monitor/direct.test.ts +++ b/extensions/matrix/src/matrix/monitor/direct.test.ts @@ -30,73 +30,6 @@ describe("createDirectRoomTracker", () => { }); it("treats m.direct rooms as DMs", async () => { - const tracker = createDirectRoomTracker(createMockClient({ isDm: true })); - await expect( - tracker.isDirectMessage({ - roomId: "!room:example.org", - senderId: "@alice:example.org", - }), - ).resolves.toBe(true); - }); - - it("does not trust stale m.direct classifications for shared rooms", async () => { - const tracker = createDirectRoomTracker( - createMockClient({ - isDm: true, - members: ["@alice:example.org", "@bot:example.org", "@extra:example.org"], - }), - ); - await expect( - tracker.isDirectMessage({ - roomId: "!room:example.org", - senderId: "@alice:example.org", - }), - ).resolves.toBe(false); - }); - - it("classifies 2-member rooms as DMs when direct metadata is missing", async () => { - const client = createMockClient({ isDm: false }); - const tracker = createDirectRoomTracker(client); - await expect( - tracker.isDirectMessage({ - roomId: "!room:example.org", - senderId: "@alice:example.org", - }), - ).resolves.toBe(true); - expect(client.getJoinedRoomMembers).toHaveBeenCalledWith("!room:example.org"); - }); - - it("does not classify rooms with extra members as DMs", async () => { - const tracker = createDirectRoomTracker( - createMockClient({ - isDm: false, - members: ["@alice:example.org", "@bot:example.org", "@observer:example.org"], - }), - ); - await expect( - tracker.isDirectMessage({ - roomId: "!room:example.org", - senderId: "@alice:example.org", - }), - ).resolves.toBe(false); - }); - - it("does not classify 2-member rooms whose sender is not a joined member as DMs", async () => { - const tracker = createDirectRoomTracker( - createMockClient({ - isDm: false, - members: ["@mallory:example.org", "@bot:example.org"], - }), - ); - await expect( - tracker.isDirectMessage({ - roomId: "!room:example.org", - senderId: "@alice:example.org", - }), - ).resolves.toBe(false); - }); - - it("re-checks room membership after invalidation when a DM gains extra members", async () => { const client = createMockClient({ isDm: true }); const tracker = createDirectRoomTracker(client); @@ -107,8 +40,100 @@ describe("createDirectRoomTracker", () => { }), ).resolves.toBe(true); - client.__setMembers(["@alice:example.org", "@bot:example.org", "@mallory:example.org"]); + expect(client.getJoinedRoomMembers).toHaveBeenCalledWith("!room:example.org"); + }); + it("does not trust stale m.direct classifications for shared rooms", async () => { + const client = createMockClient({ + isDm: true, + members: ["@alice:example.org", "@bot:example.org", "@extra:example.org"], + }); + const tracker = createDirectRoomTracker(client); + + await expect( + tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }), + ).resolves.toBe(false); + + expect(client.getJoinedRoomMembers).toHaveBeenCalledWith("!room:example.org"); + }); + + it("does not classify 2-member rooms as DMs when the dm cache refresh succeeds", async () => { + const client = createMockClient({ isDm: false }); + const tracker = createDirectRoomTracker(client); + + await expect( + tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }), + ).resolves.toBe(false); + + expect(client.getJoinedRoomMembers).toHaveBeenCalledWith("!room:example.org"); + }); + + it("falls back to strict 2-member membership when dm cache refresh fails", async () => { + const client = createMockClient({ isDm: false }); + client.dms.update.mockRejectedValue(new Error("dm cache unavailable")); + const tracker = createDirectRoomTracker(client); + + await expect( + tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }), + ).resolves.toBe(true); + + expect(client.getJoinedRoomMembers).toHaveBeenCalledWith("!room:example.org"); + }); + + it("does not classify rooms with extra members as DMs when falling back", async () => { + const client = createMockClient({ + isDm: false, + members: ["@alice:example.org", "@bot:example.org", "@observer:example.org"], + }); + client.dms.update.mockRejectedValue(new Error("dm cache unavailable")); + const tracker = createDirectRoomTracker(client); + + await expect( + tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }), + ).resolves.toBe(false); + }); + + it("does not classify 2-member rooms whose sender is not a joined member when falling back", async () => { + const client = createMockClient({ + isDm: false, + members: ["@mallory:example.org", "@bot:example.org"], + }); + client.dms.update.mockRejectedValue(new Error("dm cache unavailable")); + const tracker = createDirectRoomTracker(client); + + await expect( + tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }), + ).resolves.toBe(false); + }); + + it("re-checks room membership after invalidation when fallback membership changes", async () => { + const client = createMockClient({ isDm: false }); + client.dms.update.mockRejectedValue(new Error("dm cache unavailable")); + const tracker = createDirectRoomTracker(client); + + await expect( + tracker.isDirectMessage({ + roomId: "!room:example.org", + senderId: "@alice:example.org", + }), + ).resolves.toBe(true); + + client.__setMembers(["@alice:example.org", "@bot:example.org", "@mallory:example.org"]); tracker.invalidateRoom("!room:example.org"); await expect( @@ -119,32 +144,9 @@ describe("createDirectRoomTracker", () => { ).resolves.toBe(false); }); - it("still recognizes exact 2-member rooms when member state also claims is_direct", async () => { - const tracker = createDirectRoomTracker(createMockClient({})); - await expect( - tracker.isDirectMessage({ - roomId: "!room:example.org", - senderId: "@alice:example.org", - }), - ).resolves.toBe(true); - }); - - it("ignores member-state is_direct when the room is not a strict DM", async () => { - const tracker = createDirectRoomTracker( - createMockClient({ - members: ["@alice:example.org", "@bot:example.org", "@observer:example.org"], - }), - ); - await expect( - tracker.isDirectMessage({ - roomId: "!room:example.org", - senderId: "@alice:example.org", - }), - ).resolves.toBe(false); - }); - it("bounds joined-room membership cache size", async () => { const client = createMockClient({ isDm: false }); + client.dms.update.mockRejectedValue(new Error("dm cache unavailable")); const tracker = createDirectRoomTracker(client); for (let i = 0; i <= 1024; i += 1) { diff --git a/extensions/matrix/src/matrix/monitor/direct.ts b/extensions/matrix/src/matrix/monitor/direct.ts index c40967a05d6..a44d1c6c92c 100644 --- a/extensions/matrix/src/matrix/monitor/direct.ts +++ b/extensions/matrix/src/matrix/monitor/direct.ts @@ -48,11 +48,7 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr return; } lastDmUpdateMs = now; - try { - await client.dms.update(); - } catch (err) { - log(`matrix: dm cache refresh failed (${String(err)})`); - } + await client.dms.update(); }; const resolveJoinedMembers = async (roomId: string): Promise => { @@ -82,34 +78,34 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr }, isDirectMessage: async (params: DirectMessageCheck): Promise => { const { roomId, senderId } = params; - await refreshDmCache(); const selfUserId = params.selfUserId ?? (await ensureSelfUserId()); const joinedMembers = await resolveJoinedMembers(roomId); + const strictDirectMembership = isStrictDirectMembership({ + selfUserId, + remoteUserId: senderId, + joinedMembers, + }); + let refreshFailed = false; - if (client.dms.isDm(roomId)) { - const directViaAccountData = Boolean( - isStrictDirectMembership({ - selfUserId, - remoteUserId: senderId, - joinedMembers, - }), - ); - if (directViaAccountData) { + try { + await refreshDmCache(); + } catch (err) { + log(`matrix: dm cache refresh failed (${String(err)})`); + refreshFailed = true; + } + + if (refreshFailed) { + if (strictDirectMembership) { + log(`matrix: dm detected via exact 2-member room room=${roomId}`); + return true; + } + } else if (client.dms.isDm(roomId)) { + if (strictDirectMembership) { log(`matrix: dm detected via m.direct room=${roomId}`); return true; } log(`matrix: ignoring stale m.direct classification room=${roomId}`); - } - - if ( - isStrictDirectMembership({ - selfUserId, - remoteUserId: senderId, - joinedMembers, - }) - ) { - log(`matrix: dm detected via exact 2-member room room=${roomId}`); - return true; + return false; } log(