Skip to content

Commit 2b2edaa

Browse files
w-sssgumadeiras
w-sss
and
gumadeiras
authored
fix(matrix): correct DM classification with three-tier is_direct logic and 2-member guard (#57124)
Merged via squash. Prepared head SHA: e2ff0d5 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
1 parent dd9d0bd commit 2b2edaa

8 files changed

Lines changed: 148 additions & 26 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ Docs: https://docs.openclaw.ai
112112
- 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.
113113
- 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.
114114
- 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.
115+
- 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.
115116

116117
## 2026.3.28
117118

‎extensions/matrix/src/matrix/direct-management.test.ts‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,15 @@ describe("inspectMatrixDirectRooms", () => {
6464
expect(result.discoveredStrictRoomIds).toEqual(["!fresh:example.org"]);
6565
});
6666

67-
it("prefers discovered rooms marked direct in member state over plain strict rooms", async () => {
67+
it("prefers discovered rooms marked direct in local member state over plain strict rooms", async () => {
6868
const client = createClient({
6969
getJoinedRooms: vi.fn(async () => ["!fallback:example.org", "!explicit:example.org"]),
7070
getJoinedRoomMembers: vi.fn(async () => ["@bot:example.org", "@alice:example.org"]),
71-
getRoomStateEvent: vi.fn(async (roomId: string, _eventType: string, userId: string) => ({
72-
is_direct: roomId === "!explicit:example.org" && userId === "@alice:example.org",
73-
})),
71+
getRoomStateEvent: vi.fn(async (roomId: string, _eventType: string, userId: string) =>
72+
roomId === "!explicit:example.org" && userId === "@bot:example.org"
73+
? { is_direct: true }
74+
: {},
75+
),
7476
});
7577

7678
const result = await inspectMatrixDirectRooms({
@@ -84,6 +86,43 @@ describe("inspectMatrixDirectRooms", () => {
8486
"!explicit:example.org",
8587
]);
8688
});
89+
90+
it("ignores remote member-state direct flags when ranking discovered rooms", async () => {
91+
const client = createClient({
92+
getJoinedRooms: vi.fn(async () => ["!fallback:example.org", "!remote-marked:example.org"]),
93+
getJoinedRoomMembers: vi.fn(async () => ["@bot:example.org", "@alice:example.org"]),
94+
getRoomStateEvent: vi.fn(async (roomId: string, _eventType: string, userId: string) =>
95+
roomId === "!remote-marked:example.org" && userId === "@alice:example.org"
96+
? { is_direct: true }
97+
: {},
98+
),
99+
});
100+
101+
const result = await inspectMatrixDirectRooms({
102+
client,
103+
remoteUserId: "@alice:example.org",
104+
});
105+
106+
expect(result.activeRoomId).toBe("!fallback:example.org");
107+
});
108+
109+
it("does not treat discovered rooms with local is_direct false as active DMs", async () => {
110+
const client = createClient({
111+
getJoinedRooms: vi.fn(async () => ["!blocked:example.org"]),
112+
getJoinedRoomMembers: vi.fn(async () => ["@bot:example.org", "@alice:example.org"]),
113+
getRoomStateEvent: vi.fn(async (_roomId: string, _eventType: string, userId: string) => ({
114+
is_direct: userId === "@bot:example.org" ? false : undefined,
115+
})),
116+
});
117+
118+
const result = await inspectMatrixDirectRooms({
119+
client,
120+
remoteUserId: "@alice:example.org",
121+
});
122+
123+
expect(result.activeRoomId).toBeNull();
124+
expect(result.discoveredStrictRoomIds).toEqual([]);
125+
});
87126
});
88127

89128
describe("repairMatrixDirectRooms", () => {

‎extensions/matrix/src/matrix/direct-management.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,12 @@ async function classifyDirectRoomCandidate(params: {
9292
return {
9393
roomId: params.roomId,
9494
joinedMembers: evidence.joinedMembers,
95-
strict: evidence.strict,
96-
explicit: evidence.strict && (params.source === "account-data" || evidence.viaMemberState),
95+
strict:
96+
evidence.strict && (params.source === "account-data" || evidence.memberStateFlag !== false),
97+
explicit:
98+
evidence.strict &&
99+
(params.source === "account-data" || evidence.memberStateFlag !== false) &&
100+
(params.source === "account-data" || evidence.viaMemberState),
97101
source: params.source,
98102
};
99103
}

‎extensions/matrix/src/matrix/direct-room.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,22 @@ describe("inspectMatrixDirectRoomEvidence", () => {
4040
expect(getUserId).toHaveBeenCalledTimes(1);
4141
expect(result.strict).toBe(true);
4242
});
43+
44+
it("records only the local member-state direct flag", async () => {
45+
const client = createClient({
46+
getRoomStateEvent: vi.fn(async (_roomId: string, _eventType: string, stateKey: string) =>
47+
stateKey === "@bot:example.org" ? { is_direct: false } : { is_direct: true },
48+
),
49+
});
50+
51+
const result = await inspectMatrixDirectRoomEvidence({
52+
client,
53+
roomId: "!dm:example.org",
54+
remoteUserId: "@alice:example.org",
55+
});
56+
57+
expect(result.strict).toBe(true);
58+
expect(result.memberStateFlag).toBe(false);
59+
expect(result.viaMemberState).toBe(false);
60+
});
4361
});

‎extensions/matrix/src/matrix/direct-room.ts‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,23 +49,33 @@ export async function hasDirectMatrixMemberFlag(
4949
client: MatrixClient,
5050
roomId: string,
5151
userId?: string | null,
52-
): Promise<boolean> {
52+
): Promise<boolean | null> {
5353
const normalizedUserId = trimMaybeString(userId);
5454
if (!normalizedUserId) {
55-
return false;
55+
return null;
5656
}
5757
try {
5858
const state = await client.getRoomStateEvent(roomId, "m.room.member", normalizedUserId);
59-
return state?.is_direct === true;
59+
// Return true if is_direct is explicitly true, false if explicitly false, null if absent
60+
if (state?.is_direct === true) {
61+
return true;
62+
}
63+
if (state?.is_direct === false) {
64+
return false;
65+
}
66+
// is_direct field is absent from the membership event
67+
return null;
6068
} catch {
61-
return false;
69+
// API/network error - treat as unavailable
70+
return null;
6271
}
6372
}
6473

6574
export type MatrixDirectRoomEvidence = {
6675
joinedMembers: string[] | null;
6776
strict: boolean;
6877
viaMemberState: boolean;
78+
memberStateFlag: boolean | null;
6979
};
7080

7181
export async function inspectMatrixDirectRoomEvidence(params: {
@@ -89,14 +99,15 @@ export async function inspectMatrixDirectRoomEvidence(params: {
8999
joinedMembers,
90100
strict: false,
91101
viaMemberState: false,
102+
memberStateFlag: null,
92103
};
93104
}
105+
const memberStateFlag = await hasDirectMatrixMemberFlag(params.client, params.roomId, selfUserId);
94106
return {
95107
joinedMembers,
96108
strict,
97-
viaMemberState:
98-
(await hasDirectMatrixMemberFlag(params.client, params.roomId, params.remoteUserId)) ||
99-
(await hasDirectMatrixMemberFlag(params.client, params.roomId, selfUserId)),
109+
viaMemberState: memberStateFlag === true,
110+
memberStateFlag,
100111
};
101112
}
102113

‎extensions/matrix/src/matrix/monitor/direct.test.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,10 @@ describe("createDirectRoomTracker", () => {
145145
expect(client.getRoomStateEvent).not.toHaveBeenCalled();
146146
});
147147

148-
it("treats sender is_direct member state as a DM signal", async () => {
148+
it("does not treat sender is_direct member state as a DM signal", async () => {
149149
const client = createMockClient({
150150
isDm: false,
151+
dmCacheAvailable: true,
151152
stateEvents: {
152153
"!room:example.org|m.room.member|@alice:example.org": { is_direct: true },
153154
},
@@ -159,7 +160,7 @@ describe("createDirectRoomTracker", () => {
159160
roomId: "!room:example.org",
160161
senderId: "@alice:example.org",
161162
}),
162-
).resolves.toBe(true);
163+
).resolves.toBe(false);
163164
});
164165

165166
it("treats self is_direct member state as a DM signal", async () => {
@@ -179,6 +180,24 @@ describe("createDirectRoomTracker", () => {
179180
).resolves.toBe(true);
180181
});
181182

183+
it("treats self is_direct false member state as a non-DM signal", async () => {
184+
const client = createMockClient({
185+
isDm: false,
186+
dmCacheAvailable: false,
187+
stateEvents: {
188+
"!room:example.org|m.room.member|@bot:example.org": { is_direct: false },
189+
},
190+
});
191+
const tracker = createDirectRoomTracker(client);
192+
193+
await expect(
194+
tracker.isDirectMessage({
195+
roomId: "!room:example.org",
196+
senderId: "@alice:example.org",
197+
}),
198+
).resolves.toBe(false);
199+
});
200+
182201
it("does not classify 2-member rooms whose sender is not a joined member when falling back", async () => {
183202
const client = createMockClient({
184203
isDm: false,

‎extensions/matrix/src/matrix/monitor/direct.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
3737
let hasSeededDmCache = false;
3838
let cachedSelfUserId: string | null = null;
3939
const joinedMembersCache = new Map<string, { members: string[]; ts: number }>();
40-
const directMemberFlagCache = new Map<string, { isDirect: boolean; ts: number }>();
40+
const directMemberFlagCache = new Map<string, { isDirect: boolean | null; ts: number }>();
4141

4242
const ensureSelfUserId = async (): Promise<string | null> => {
4343
if (cachedSelfUserId) {
@@ -82,10 +82,10 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
8282
const resolveDirectMemberFlag = async (
8383
roomId: string,
8484
userId?: string | null,
85-
): Promise<boolean> => {
85+
): Promise<boolean | null> => {
8686
const normalizedUserId = userId?.trim();
8787
if (!normalizedUserId) {
88-
return false;
88+
return null;
8989
}
9090
const cacheKey = `${roomId}\n${normalizedUserId}`;
9191
const cached = directMemberFlagCache.get(cacheKey);
@@ -134,13 +134,15 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
134134
}
135135

136136
if (strictDirectMembership) {
137-
const directViaState =
138-
(await resolveDirectMemberFlag(roomId, senderId)) ||
139-
(await resolveDirectMemberFlag(roomId, selfUserId));
140-
if (directViaState) {
137+
const directViaSelf = await resolveDirectMemberFlag(roomId, selfUserId);
138+
if (directViaSelf === true) {
141139
log(`matrix: dm detected via member state room=${roomId}`);
142140
return true;
143141
}
142+
if (directViaSelf === false) {
143+
log(`matrix: dm rejected via member state room=${roomId}`);
144+
return false;
145+
}
144146

145147
if (!hasSeededDmCache) {
146148
log(

‎extensions/matrix/src/matrix/send/targets.test.ts‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe("resolveMatrixRoomId", () => {
5151
);
5252
});
5353

54-
it("prefers joined rooms marked direct in member state over plain strict rooms", async () => {
54+
it("prefers joined rooms marked direct in local member state over plain strict rooms", async () => {
5555
const userId = "@fallback:example.org";
5656
const client = {
5757
getAccountData: vi.fn().mockRejectedValue(new Error("nope")),
@@ -60,9 +60,11 @@ describe("resolveMatrixRoomId", () => {
6060
getJoinedRoomMembers: vi.fn().mockResolvedValue(["@bot:example.org", userId]),
6161
getRoomStateEvent: vi
6262
.fn()
63-
.mockImplementation(async (roomId: string, _eventType: string, stateKey: string) => ({
64-
is_direct: roomId === "!explicit:example.org" && stateKey === userId,
65-
})),
63+
.mockImplementation(async (roomId: string, _eventType: string, stateKey: string) =>
64+
roomId === "!explicit:example.org" && stateKey === "@bot:example.org"
65+
? { is_direct: true }
66+
: {},
67+
),
6668
setAccountData: vi.fn().mockResolvedValue(undefined),
6769
} as unknown as MatrixClient;
6870

@@ -75,6 +77,32 @@ describe("resolveMatrixRoomId", () => {
7577
);
7678
});
7779

80+
it("ignores remote member-state direct flags when resolving a direct room", async () => {
81+
const userId = "@fallback:example.org";
82+
const client = {
83+
getAccountData: vi.fn().mockRejectedValue(new Error("nope")),
84+
getUserId: vi.fn().mockResolvedValue("@bot:example.org"),
85+
getJoinedRooms: vi
86+
.fn()
87+
.mockResolvedValue(["!fallback:example.org", "!remote-marked:example.org"]),
88+
getJoinedRoomMembers: vi.fn().mockResolvedValue(["@bot:example.org", userId]),
89+
getRoomStateEvent: vi
90+
.fn()
91+
.mockImplementation(async (roomId: string, _eventType: string, stateKey: string) =>
92+
roomId === "!remote-marked:example.org" && stateKey === userId ? { is_direct: true } : {},
93+
),
94+
setAccountData: vi.fn().mockResolvedValue(undefined),
95+
} as unknown as MatrixClient;
96+
97+
const resolved = await resolveMatrixRoomId(client, userId);
98+
99+
expect(resolved).toBe("!fallback:example.org");
100+
expect(client.setAccountData).toHaveBeenCalledWith(
101+
EventType.Direct,
102+
expect.objectContaining({ [userId]: ["!fallback:example.org"] }),
103+
);
104+
});
105+
78106
it("continues when a room member lookup fails", async () => {
79107
const userId = "@continue:example.org";
80108
const roomId = "!good:example.org";

0 commit comments

Comments
 (0)