From 2b2edaa01db51c6670eef3381a191d5808711f05 Mon Sep 17 00:00:00 2001 From: end <1598099293@qq.com> Date: Tue, 31 Mar 2026 06:56:00 +0800 Subject: [PATCH] fix(matrix): correct DM classification with three-tier is_direct logic and 2-member guard (#57124) Merged via squash. Prepared head SHA: e2ff0d5e961890b95bdc3f994c2381de4d2dfe24 Co-authored-by: w-sss <204439273+w-sss@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + .../src/matrix/direct-management.test.ts | 47 +++++++++++++++++-- .../matrix/src/matrix/direct-management.ts | 8 +++- .../matrix/src/matrix/direct-room.test.ts | 18 +++++++ extensions/matrix/src/matrix/direct-room.ts | 25 +++++++--- .../matrix/src/matrix/monitor/direct.test.ts | 23 ++++++++- .../matrix/src/matrix/monitor/direct.ts | 16 ++++--- .../matrix/src/matrix/send/targets.test.ts | 36 ++++++++++++-- 8 files changed, 148 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df5bb2226fc..9e543975c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -112,6 +112,7 @@ Docs: https://docs.openclaw.ai - Diffs: fall back to plain text when `lang` hints are invalid during diff render and viewer hydration, so bad or stale language values no longer break the diff viewer. (#57902) Thanks @gumadeiras. - Doctor/plugins: skip false Matrix legacy-helper warnings when no migration plans exist, and keep bundled `enabledByDefault` plugins in the gateway startup set. (#57931) Thanks @dinakars777. - Matrix/CLI send: start one-off Matrix send clients before outbound delivery so `openclaw message send --channel matrix` restores E2EE in encrypted rooms instead of sending plain events. (#57936) Thanks @gumadeiras. +- Matrix/direct rooms: stop trusting remote `is_direct`, honor explicit local `is_direct: false` for discovered DM candidates, and avoid extra member-state lookups for shared rooms so DM routing and repair stay aligned. (#57124) Thanks @w-sss. ## 2026.3.28 diff --git a/extensions/matrix/src/matrix/direct-management.test.ts b/extensions/matrix/src/matrix/direct-management.test.ts index c3c6eb7c459..42b12fabe4e 100644 --- a/extensions/matrix/src/matrix/direct-management.test.ts +++ b/extensions/matrix/src/matrix/direct-management.test.ts @@ -64,13 +64,15 @@ describe("inspectMatrixDirectRooms", () => { expect(result.discoveredStrictRoomIds).toEqual(["!fresh:example.org"]); }); - it("prefers discovered rooms marked direct in member state over plain strict rooms", async () => { + it("prefers discovered rooms marked direct in local member state over plain strict rooms", async () => { const client = createClient({ getJoinedRooms: vi.fn(async () => ["!fallback:example.org", "!explicit:example.org"]), getJoinedRoomMembers: vi.fn(async () => ["@bot:example.org", "@alice:example.org"]), - getRoomStateEvent: vi.fn(async (roomId: string, _eventType: string, userId: string) => ({ - is_direct: roomId === "!explicit:example.org" && userId === "@alice:example.org", - })), + getRoomStateEvent: vi.fn(async (roomId: string, _eventType: string, userId: string) => + roomId === "!explicit:example.org" && userId === "@bot:example.org" + ? { is_direct: true } + : {}, + ), }); const result = await inspectMatrixDirectRooms({ @@ -84,6 +86,43 @@ describe("inspectMatrixDirectRooms", () => { "!explicit:example.org", ]); }); + + it("ignores remote member-state direct flags when ranking discovered rooms", async () => { + const client = createClient({ + getJoinedRooms: vi.fn(async () => ["!fallback:example.org", "!remote-marked:example.org"]), + getJoinedRoomMembers: vi.fn(async () => ["@bot:example.org", "@alice:example.org"]), + getRoomStateEvent: vi.fn(async (roomId: string, _eventType: string, userId: string) => + roomId === "!remote-marked:example.org" && userId === "@alice:example.org" + ? { is_direct: true } + : {}, + ), + }); + + const result = await inspectMatrixDirectRooms({ + client, + remoteUserId: "@alice:example.org", + }); + + expect(result.activeRoomId).toBe("!fallback:example.org"); + }); + + it("does not treat discovered rooms with local is_direct false as active DMs", async () => { + const client = createClient({ + getJoinedRooms: vi.fn(async () => ["!blocked:example.org"]), + getJoinedRoomMembers: vi.fn(async () => ["@bot:example.org", "@alice:example.org"]), + getRoomStateEvent: vi.fn(async (_roomId: string, _eventType: string, userId: string) => ({ + is_direct: userId === "@bot:example.org" ? false : undefined, + })), + }); + + const result = await inspectMatrixDirectRooms({ + client, + remoteUserId: "@alice:example.org", + }); + + expect(result.activeRoomId).toBeNull(); + expect(result.discoveredStrictRoomIds).toEqual([]); + }); }); describe("repairMatrixDirectRooms", () => { diff --git a/extensions/matrix/src/matrix/direct-management.ts b/extensions/matrix/src/matrix/direct-management.ts index 2ed6c8366dd..db63018c62b 100644 --- a/extensions/matrix/src/matrix/direct-management.ts +++ b/extensions/matrix/src/matrix/direct-management.ts @@ -92,8 +92,12 @@ async function classifyDirectRoomCandidate(params: { return { roomId: params.roomId, joinedMembers: evidence.joinedMembers, - strict: evidence.strict, - explicit: evidence.strict && (params.source === "account-data" || evidence.viaMemberState), + strict: + evidence.strict && (params.source === "account-data" || evidence.memberStateFlag !== false), + explicit: + evidence.strict && + (params.source === "account-data" || evidence.memberStateFlag !== false) && + (params.source === "account-data" || evidence.viaMemberState), source: params.source, }; } diff --git a/extensions/matrix/src/matrix/direct-room.test.ts b/extensions/matrix/src/matrix/direct-room.test.ts index dfc482bcc48..0179d2a1e65 100644 --- a/extensions/matrix/src/matrix/direct-room.test.ts +++ b/extensions/matrix/src/matrix/direct-room.test.ts @@ -40,4 +40,22 @@ describe("inspectMatrixDirectRoomEvidence", () => { expect(getUserId).toHaveBeenCalledTimes(1); expect(result.strict).toBe(true); }); + + it("records only the local member-state direct flag", async () => { + const client = createClient({ + getRoomStateEvent: vi.fn(async (_roomId: string, _eventType: string, stateKey: string) => + stateKey === "@bot:example.org" ? { is_direct: false } : { is_direct: true }, + ), + }); + + const result = await inspectMatrixDirectRoomEvidence({ + client, + roomId: "!dm:example.org", + remoteUserId: "@alice:example.org", + }); + + expect(result.strict).toBe(true); + expect(result.memberStateFlag).toBe(false); + expect(result.viaMemberState).toBe(false); + }); }); diff --git a/extensions/matrix/src/matrix/direct-room.ts b/extensions/matrix/src/matrix/direct-room.ts index cc11671c162..b3bc71e5872 100644 --- a/extensions/matrix/src/matrix/direct-room.ts +++ b/extensions/matrix/src/matrix/direct-room.ts @@ -49,16 +49,25 @@ export async function hasDirectMatrixMemberFlag( client: MatrixClient, roomId: string, userId?: string | null, -): Promise { +): Promise { const normalizedUserId = trimMaybeString(userId); if (!normalizedUserId) { - return false; + return null; } try { const state = await client.getRoomStateEvent(roomId, "m.room.member", normalizedUserId); - return state?.is_direct === true; + // Return true if is_direct is explicitly true, false if explicitly false, null if absent + if (state?.is_direct === true) { + return true; + } + if (state?.is_direct === false) { + return false; + } + // is_direct field is absent from the membership event + return null; } catch { - return false; + // API/network error - treat as unavailable + return null; } } @@ -66,6 +75,7 @@ export type MatrixDirectRoomEvidence = { joinedMembers: string[] | null; strict: boolean; viaMemberState: boolean; + memberStateFlag: boolean | null; }; export async function inspectMatrixDirectRoomEvidence(params: { @@ -89,14 +99,15 @@ export async function inspectMatrixDirectRoomEvidence(params: { joinedMembers, strict: false, viaMemberState: false, + memberStateFlag: null, }; } + const memberStateFlag = await hasDirectMatrixMemberFlag(params.client, params.roomId, selfUserId); return { joinedMembers, strict, - viaMemberState: - (await hasDirectMatrixMemberFlag(params.client, params.roomId, params.remoteUserId)) || - (await hasDirectMatrixMemberFlag(params.client, params.roomId, selfUserId)), + viaMemberState: memberStateFlag === true, + memberStateFlag, }; } diff --git a/extensions/matrix/src/matrix/monitor/direct.test.ts b/extensions/matrix/src/matrix/monitor/direct.test.ts index 36d53322c3e..ded86edc496 100644 --- a/extensions/matrix/src/matrix/monitor/direct.test.ts +++ b/extensions/matrix/src/matrix/monitor/direct.test.ts @@ -145,9 +145,10 @@ describe("createDirectRoomTracker", () => { expect(client.getRoomStateEvent).not.toHaveBeenCalled(); }); - it("treats sender is_direct member state as a DM signal", async () => { + it("does not treat sender is_direct member state as a DM signal", async () => { const client = createMockClient({ isDm: false, + dmCacheAvailable: true, stateEvents: { "!room:example.org|m.room.member|@alice:example.org": { is_direct: true }, }, @@ -159,7 +160,7 @@ describe("createDirectRoomTracker", () => { roomId: "!room:example.org", senderId: "@alice:example.org", }), - ).resolves.toBe(true); + ).resolves.toBe(false); }); it("treats self is_direct member state as a DM signal", async () => { @@ -179,6 +180,24 @@ describe("createDirectRoomTracker", () => { ).resolves.toBe(true); }); + it("treats self is_direct false member state as a non-DM signal", async () => { + const client = createMockClient({ + isDm: false, + dmCacheAvailable: false, + stateEvents: { + "!room:example.org|m.room.member|@bot:example.org": { is_direct: false }, + }, + }); + 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, diff --git a/extensions/matrix/src/matrix/monitor/direct.ts b/extensions/matrix/src/matrix/monitor/direct.ts index d976a794c81..39e21fccab1 100644 --- a/extensions/matrix/src/matrix/monitor/direct.ts +++ b/extensions/matrix/src/matrix/monitor/direct.ts @@ -37,7 +37,7 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr let hasSeededDmCache = false; let cachedSelfUserId: string | null = null; const joinedMembersCache = new Map(); - const directMemberFlagCache = new Map(); + const directMemberFlagCache = new Map(); const ensureSelfUserId = async (): Promise => { if (cachedSelfUserId) { @@ -82,10 +82,10 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr const resolveDirectMemberFlag = async ( roomId: string, userId?: string | null, - ): Promise => { + ): Promise => { const normalizedUserId = userId?.trim(); if (!normalizedUserId) { - return false; + return null; } const cacheKey = `${roomId}\n${normalizedUserId}`; const cached = directMemberFlagCache.get(cacheKey); @@ -134,13 +134,15 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr } if (strictDirectMembership) { - const directViaState = - (await resolveDirectMemberFlag(roomId, senderId)) || - (await resolveDirectMemberFlag(roomId, selfUserId)); - if (directViaState) { + const directViaSelf = await resolveDirectMemberFlag(roomId, selfUserId); + if (directViaSelf === true) { log(`matrix: dm detected via member state room=${roomId}`); return true; } + if (directViaSelf === false) { + log(`matrix: dm rejected via member state room=${roomId}`); + return false; + } if (!hasSeededDmCache) { log( diff --git a/extensions/matrix/src/matrix/send/targets.test.ts b/extensions/matrix/src/matrix/send/targets.test.ts index 1308dec7619..149139bf842 100644 --- a/extensions/matrix/src/matrix/send/targets.test.ts +++ b/extensions/matrix/src/matrix/send/targets.test.ts @@ -51,7 +51,7 @@ describe("resolveMatrixRoomId", () => { ); }); - it("prefers joined rooms marked direct in member state over plain strict rooms", async () => { + it("prefers joined rooms marked direct in local member state over plain strict rooms", async () => { const userId = "@fallback:example.org"; const client = { getAccountData: vi.fn().mockRejectedValue(new Error("nope")), @@ -60,9 +60,11 @@ describe("resolveMatrixRoomId", () => { getJoinedRoomMembers: vi.fn().mockResolvedValue(["@bot:example.org", userId]), getRoomStateEvent: vi .fn() - .mockImplementation(async (roomId: string, _eventType: string, stateKey: string) => ({ - is_direct: roomId === "!explicit:example.org" && stateKey === userId, - })), + .mockImplementation(async (roomId: string, _eventType: string, stateKey: string) => + roomId === "!explicit:example.org" && stateKey === "@bot:example.org" + ? { is_direct: true } + : {}, + ), setAccountData: vi.fn().mockResolvedValue(undefined), } as unknown as MatrixClient; @@ -75,6 +77,32 @@ describe("resolveMatrixRoomId", () => { ); }); + it("ignores remote member-state direct flags when resolving a direct room", async () => { + const userId = "@fallback:example.org"; + const client = { + getAccountData: vi.fn().mockRejectedValue(new Error("nope")), + getUserId: vi.fn().mockResolvedValue("@bot:example.org"), + getJoinedRooms: vi + .fn() + .mockResolvedValue(["!fallback:example.org", "!remote-marked:example.org"]), + getJoinedRoomMembers: vi.fn().mockResolvedValue(["@bot:example.org", userId]), + getRoomStateEvent: vi + .fn() + .mockImplementation(async (roomId: string, _eventType: string, stateKey: string) => + roomId === "!remote-marked:example.org" && stateKey === userId ? { is_direct: true } : {}, + ), + setAccountData: vi.fn().mockResolvedValue(undefined), + } as unknown as MatrixClient; + + const resolved = await resolveMatrixRoomId(client, userId); + + expect(resolved).toBe("!fallback:example.org"); + expect(client.setAccountData).toHaveBeenCalledWith( + EventType.Direct, + expect.objectContaining({ [userId]: ["!fallback:example.org"] }), + ); + }); + it("continues when a room member lookup fails", async () => { const userId = "@continue:example.org"; const roomId = "!good:example.org";