mirror of https://github.com/openclaw/openclaw.git
fix(matrix): correct DM classification with three-tier is_direct logic and 2-member guard (#57124)
Merged via squash.
Prepared head SHA: e2ff0d5e96
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
This commit is contained in:
parent
dd9d0bdd8e
commit
2b2edaa01d
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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", () => {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -49,16 +49,25 @@ export async function hasDirectMatrixMemberFlag(
|
|||
client: MatrixClient,
|
||||
roomId: string,
|
||||
userId?: string | null,
|
||||
): Promise<boolean> {
|
||||
): Promise<boolean | null> {
|
||||
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,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
|
|||
let hasSeededDmCache = false;
|
||||
let cachedSelfUserId: string | null = null;
|
||||
const joinedMembersCache = new Map<string, { members: string[]; ts: number }>();
|
||||
const directMemberFlagCache = new Map<string, { isDirect: boolean; ts: number }>();
|
||||
const directMemberFlagCache = new Map<string, { isDirect: boolean | null; ts: number }>();
|
||||
|
||||
const ensureSelfUserId = async (): Promise<string | null> => {
|
||||
if (cachedSelfUserId) {
|
||||
|
|
@ -82,10 +82,10 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
|
|||
const resolveDirectMemberFlag = async (
|
||||
roomId: string,
|
||||
userId?: string | null,
|
||||
): Promise<boolean> => {
|
||||
): Promise<boolean | null> => {
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
|
|
|||
Loading…
Reference in New Issue