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.
This commit is contained in:
peter 2026-03-25 19:41:34 -07:00 committed by Gustavo Madeira Santana
parent 11952457af
commit eaebed0e38
No known key found for this signature in database
2 changed files with 116 additions and 118 deletions

View File

@ -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) {

View File

@ -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<string[] | null> => {
@ -82,34 +78,34 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
},
isDirectMessage: async (params: DirectMessageCheck): Promise<boolean> => {
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(