mirror of https://github.com/openclaw/openclaw.git
fix(matrix): align outbound direct-room selection (#56076)
Merged via squash.
Prepared head SHA: bbd9afdd5c
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Reviewed-by: @gumadeiras
This commit is contained in:
parent
6455606b90
commit
b6ead2dd3b
|
|
@ -101,6 +101,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Plugins/diffs: stage bundled `@pierre/diffs` runtime dependencies during packaged updates so the bundled diff viewer keeps loading after global installs and updates. (#56077) Thanks @gumadeiras.
|
||||
- Plugins/diffs: load bundled Pierre themes without JSON module imports so diff rendering keeps working on newer Node builds. (#45869) thanks @NickHood1984.
|
||||
- Plugins/uninstall: remove owned `channels.<id>` config when uninstalling channel plugins, and keep the uninstall preview aligned with explicit channel ownership so built-in channels and shared keys stay intact. (#35915) Thanks @wbxl2000.
|
||||
- Plugins/Matrix: prefer explicit DM signals when choosing outbound direct rooms and routing unmapped verification summaries, so strict 2-person fallback rooms do not outrank the real DM. (#56076) thanks @gumadeiras
|
||||
|
||||
## 2026.3.24
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ function createClient(overrides: Partial<MatrixClient> = {}): MatrixClient {
|
|||
getAccountData: vi.fn(async () => undefined),
|
||||
getJoinedRooms: vi.fn(async () => [] as string[]),
|
||||
getJoinedRoomMembers: vi.fn(async () => [] as string[]),
|
||||
getRoomStateEvent: vi.fn(async () => ({})),
|
||||
setAccountData: vi.fn(async () => undefined),
|
||||
createDirectRoom: vi.fn(async () => "!created:example.org"),
|
||||
...overrides,
|
||||
|
|
@ -62,6 +63,27 @@ describe("inspectMatrixDirectRooms", () => {
|
|||
expect(result.activeRoomId).toBe("!fresh:example.org");
|
||||
expect(result.discoveredStrictRoomIds).toEqual(["!fresh:example.org"]);
|
||||
});
|
||||
|
||||
it("prefers discovered rooms marked direct in 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",
|
||||
})),
|
||||
});
|
||||
|
||||
const result = await inspectMatrixDirectRooms({
|
||||
client,
|
||||
remoteUserId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result.activeRoomId).toBe("!explicit:example.org");
|
||||
expect(result.discoveredStrictRoomIds).toEqual([
|
||||
"!fallback:example.org",
|
||||
"!explicit:example.org",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("repairMatrixDirectRooms", () => {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,4 @@
|
|||
import {
|
||||
isStrictDirectMembership,
|
||||
isStrictDirectRoom,
|
||||
readJoinedMatrixMembers,
|
||||
} from "./direct-room.js";
|
||||
import { inspectMatrixDirectRoomEvidence } from "./direct-room.js";
|
||||
import type { MatrixClient } from "./sdk.js";
|
||||
import { EventType, type MatrixDirectAccountData } from "./send/types.js";
|
||||
import { isMatrixQualifiedUserId } from "./target-ids.js";
|
||||
|
|
@ -11,6 +7,7 @@ export type MatrixDirectRoomCandidate = {
|
|||
roomId: string;
|
||||
joinedMembers: string[] | null;
|
||||
strict: boolean;
|
||||
explicit: boolean;
|
||||
source: "account-data" | "joined";
|
||||
};
|
||||
|
||||
|
|
@ -86,17 +83,17 @@ async function classifyDirectRoomCandidate(params: {
|
|||
selfUserId: string | null;
|
||||
source: "account-data" | "joined";
|
||||
}): Promise<MatrixDirectRoomCandidate> {
|
||||
const joinedMembers = await readJoinedMatrixMembers(params.client, params.roomId);
|
||||
const evidence = await inspectMatrixDirectRoomEvidence({
|
||||
client: params.client,
|
||||
roomId: params.roomId,
|
||||
remoteUserId: params.remoteUserId,
|
||||
selfUserId: params.selfUserId,
|
||||
});
|
||||
return {
|
||||
roomId: params.roomId,
|
||||
joinedMembers,
|
||||
strict:
|
||||
joinedMembers !== null &&
|
||||
isStrictDirectMembership({
|
||||
selfUserId: params.selfUserId,
|
||||
remoteUserId: params.remoteUserId,
|
||||
joinedMembers,
|
||||
}),
|
||||
joinedMembers: evidence.joinedMembers,
|
||||
strict: evidence.strict,
|
||||
explicit: evidence.strict && (params.source === "account-data" || evidence.viaMemberState),
|
||||
source: params.source,
|
||||
};
|
||||
}
|
||||
|
|
@ -167,22 +164,24 @@ export async function inspectMatrixDirectRooms(params: {
|
|||
joinedRooms = [];
|
||||
}
|
||||
}
|
||||
const discoveredStrictRoomIds: string[] = [];
|
||||
const discoveredStrictRooms: MatrixDirectRoomCandidate[] = [];
|
||||
for (const roomId of normalizeRoomIdList(joinedRooms)) {
|
||||
if (mappedRoomIds.includes(roomId)) {
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
await isStrictDirectRoom({
|
||||
client: params.client,
|
||||
roomId,
|
||||
remoteUserId,
|
||||
selfUserId,
|
||||
})
|
||||
) {
|
||||
discoveredStrictRoomIds.push(roomId);
|
||||
const candidate = await classifyDirectRoomCandidate({
|
||||
client: params.client,
|
||||
roomId,
|
||||
remoteUserId,
|
||||
selfUserId,
|
||||
source: "joined",
|
||||
});
|
||||
if (candidate.strict) {
|
||||
discoveredStrictRooms.push(candidate);
|
||||
}
|
||||
}
|
||||
const discoveredStrictRoomIds = discoveredStrictRooms.map((room) => room.roomId);
|
||||
const discoveredExplicit = discoveredStrictRooms.find((room) => room.explicit);
|
||||
|
||||
return {
|
||||
selfUserId,
|
||||
|
|
@ -190,7 +189,8 @@ export async function inspectMatrixDirectRooms(params: {
|
|||
mappedRoomIds,
|
||||
mappedRooms,
|
||||
discoveredStrictRoomIds,
|
||||
activeRoomId: mappedStrict?.roomId ?? discoveredStrictRoomIds[0] ?? null,
|
||||
activeRoomId:
|
||||
mappedStrict?.roomId ?? discoveredExplicit?.roomId ?? discoveredStrictRoomIds[0] ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,43 @@
|
|||
import { describe, expect, it, vi } from "vitest";
|
||||
import { inspectMatrixDirectRoomEvidence } from "./direct-room.js";
|
||||
import type { MatrixClient } from "./sdk.js";
|
||||
|
||||
function createClient(overrides: Partial<MatrixClient> = {}): MatrixClient {
|
||||
return {
|
||||
getUserId: vi.fn(async () => "@bot:example.org"),
|
||||
getJoinedRoomMembers: vi.fn(async () => ["@bot:example.org", "@alice:example.org"]),
|
||||
getRoomStateEvent: vi.fn(async () => ({})),
|
||||
...overrides,
|
||||
} as unknown as MatrixClient;
|
||||
}
|
||||
|
||||
describe("inspectMatrixDirectRoomEvidence", () => {
|
||||
it("does not retry getUserId when callers explicitly pass a missing self user", async () => {
|
||||
const getUserId = vi.fn(async () => "@bot:example.org");
|
||||
const client = createClient({ getUserId });
|
||||
|
||||
const result = await inspectMatrixDirectRoomEvidence({
|
||||
client,
|
||||
roomId: "!dm:example.org",
|
||||
remoteUserId: "@alice:example.org",
|
||||
selfUserId: null,
|
||||
});
|
||||
|
||||
expect(getUserId).not.toHaveBeenCalled();
|
||||
expect(result.strict).toBe(false);
|
||||
});
|
||||
|
||||
it("resolves selfUserId when callers leave it undefined", async () => {
|
||||
const getUserId = vi.fn(async () => "@bot:example.org");
|
||||
const client = createClient({ getUserId });
|
||||
|
||||
const result = await inspectMatrixDirectRoomEvidence({
|
||||
client,
|
||||
roomId: "!dm:example.org",
|
||||
remoteUserId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(getUserId).toHaveBeenCalledTimes(1);
|
||||
expect(result.strict).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
@ -62,22 +62,56 @@ export async function hasDirectMatrixMemberFlag(
|
|||
}
|
||||
}
|
||||
|
||||
export type MatrixDirectRoomEvidence = {
|
||||
joinedMembers: string[] | null;
|
||||
strict: boolean;
|
||||
viaMemberState: boolean;
|
||||
};
|
||||
|
||||
export async function inspectMatrixDirectRoomEvidence(params: {
|
||||
client: MatrixClient;
|
||||
roomId: string;
|
||||
remoteUserId: string;
|
||||
selfUserId?: string | null;
|
||||
}): Promise<MatrixDirectRoomEvidence> {
|
||||
const selfUserId =
|
||||
params.selfUserId !== undefined
|
||||
? trimMaybeString(params.selfUserId)
|
||||
: trimMaybeString(await params.client.getUserId().catch(() => null));
|
||||
const joinedMembers = await readJoinedMatrixMembers(params.client, params.roomId);
|
||||
const strict = isStrictDirectMembership({
|
||||
selfUserId,
|
||||
remoteUserId: params.remoteUserId,
|
||||
joinedMembers,
|
||||
});
|
||||
if (!strict) {
|
||||
return {
|
||||
joinedMembers,
|
||||
strict: false,
|
||||
viaMemberState: false,
|
||||
};
|
||||
}
|
||||
return {
|
||||
joinedMembers,
|
||||
strict,
|
||||
viaMemberState:
|
||||
(await hasDirectMatrixMemberFlag(params.client, params.roomId, params.remoteUserId)) ||
|
||||
(await hasDirectMatrixMemberFlag(params.client, params.roomId, selfUserId)),
|
||||
};
|
||||
}
|
||||
|
||||
export async function isStrictDirectRoom(params: {
|
||||
client: MatrixClient;
|
||||
roomId: string;
|
||||
remoteUserId: string;
|
||||
selfUserId?: string | null;
|
||||
}): Promise<boolean> {
|
||||
const selfUserId =
|
||||
trimMaybeString(params.selfUserId) ??
|
||||
trimMaybeString(await params.client.getUserId().catch(() => null));
|
||||
if (!selfUserId) {
|
||||
return false;
|
||||
}
|
||||
const joinedMembers = await readJoinedMatrixMembers(params.client, params.roomId);
|
||||
return isStrictDirectMembership({
|
||||
selfUserId,
|
||||
remoteUserId: params.remoteUserId,
|
||||
joinedMembers,
|
||||
});
|
||||
return (
|
||||
await inspectMatrixDirectRoomEvidence({
|
||||
client: params.client,
|
||||
roomId: params.roomId,
|
||||
remoteUserId: params.remoteUserId,
|
||||
selfUserId: params.selfUserId,
|
||||
})
|
||||
).strict;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -28,7 +28,9 @@ function createHarness(params?: {
|
|||
dmEnabled?: boolean;
|
||||
dmPolicy?: "open" | "pairing" | "allowlist" | "disabled";
|
||||
storeAllowFrom?: string[];
|
||||
accountDataByType?: Record<string, unknown>;
|
||||
joinedMembersByRoom?: Record<string, string[]>;
|
||||
memberStateByRoomUser?: Record<string, Record<string, { is_direct?: boolean }>>;
|
||||
verifications?: Array<{
|
||||
id: string;
|
||||
transactionId?: string;
|
||||
|
|
@ -88,7 +90,20 @@ function createHarness(params?: {
|
|||
async (roomId: string) =>
|
||||
params?.joinedMembersByRoom?.[roomId] ?? ["@bot:example.org", "@alice:example.org"],
|
||||
),
|
||||
getJoinedRooms: vi.fn(async () => Object.keys(params?.joinedMembersByRoom ?? {})),
|
||||
getJoinedRooms: vi.fn(async () =>
|
||||
Object.keys(params?.joinedMembersByRoom ?? {}).length > 0
|
||||
? Object.keys(params?.joinedMembersByRoom ?? {})
|
||||
: ["!room:example.org"],
|
||||
),
|
||||
getAccountData: vi.fn(
|
||||
async (eventType: string) =>
|
||||
(params?.accountDataByType?.[eventType] as Record<string, unknown> | undefined) ??
|
||||
undefined,
|
||||
),
|
||||
getRoomStateEvent: vi.fn(
|
||||
async (roomId: string, _eventType: string, stateKey: string) =>
|
||||
params?.memberStateByRoomUser?.[roomId]?.[stateKey] ?? {},
|
||||
),
|
||||
...(params?.cryptoAvailable === false
|
||||
? {}
|
||||
: {
|
||||
|
|
@ -683,7 +698,7 @@ describe("registerMatrixMonitorEvents verification routing", () => {
|
|||
expect(body).toContain("SAS decimal: 4321 8765 2109");
|
||||
});
|
||||
|
||||
it("prefers the most recent verification DM over the canonical active DM for unmapped SAS summaries", async () => {
|
||||
it("prefers the canonical active DM over the most recent verification room for unmapped SAS summaries", async () => {
|
||||
const { sendMessage, roomEventListener, verificationSummaryListener } = createHarness({
|
||||
joinedMembersByRoom: {
|
||||
"!dm-active:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
|
|
@ -748,7 +763,7 @@ describe("registerMatrixMonitorEvents verification routing", () => {
|
|||
"SAS decimal: 2468 1357 9753",
|
||||
),
|
||||
);
|
||||
expect((sasCall?.[0] ?? "") as string).toBe("!dm-current:example.org");
|
||||
expect((sasCall?.[0] ?? "") as string).toBe("!dm-active:example.org");
|
||||
});
|
||||
|
||||
it("retries SAS notice lookup when start arrives before SAS payload is available", async () => {
|
||||
|
|
@ -851,6 +866,115 @@ describe("registerMatrixMonitorEvents verification routing", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("routes unmapped verification summaries to the room marked direct in member state", async () => {
|
||||
const { sendMessage, verificationSummaryListener } = createHarness({
|
||||
joinedMembersByRoom: {
|
||||
"!fallback:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
"!dm:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
},
|
||||
memberStateByRoomUser: {
|
||||
"!dm:example.org": {
|
||||
"@alice:example.org": { is_direct: true },
|
||||
},
|
||||
},
|
||||
});
|
||||
if (!verificationSummaryListener) {
|
||||
throw new Error("verification.summary listener was not registered");
|
||||
}
|
||||
|
||||
verificationSummaryListener({
|
||||
id: "verification-explicit-room",
|
||||
otherUserId: "@alice:example.org",
|
||||
isSelfVerification: false,
|
||||
initiatedByMe: false,
|
||||
phase: 3,
|
||||
phaseName: "started",
|
||||
pending: true,
|
||||
methods: ["m.sas.v1"],
|
||||
canAccept: false,
|
||||
hasSas: true,
|
||||
sas: {
|
||||
decimal: [6158, 1986, 3513],
|
||||
emoji: [
|
||||
["🎁", "Gift"],
|
||||
["🌍", "Globe"],
|
||||
["🐴", "Horse"],
|
||||
],
|
||||
},
|
||||
hasReciprocateQr: false,
|
||||
completed: false,
|
||||
createdAt: new Date("2026-02-25T21:42:54.000Z").toISOString(),
|
||||
updatedAt: new Date("2026-02-25T21:42:55.000Z").toISOString(),
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(sendMessage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect((sendMessage.mock.calls as unknown[][])[0]?.[0]).toBe("!dm:example.org");
|
||||
});
|
||||
|
||||
it("prefers the active direct room over a stale remembered strict room for unmapped summaries", async () => {
|
||||
const { sendMessage, roomEventListener, verificationSummaryListener } = createHarness({
|
||||
joinedMembersByRoom: {
|
||||
"!fallback:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
"!dm:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
},
|
||||
memberStateByRoomUser: {
|
||||
"!dm:example.org": {
|
||||
"@alice:example.org": { is_direct: true },
|
||||
},
|
||||
},
|
||||
});
|
||||
if (!verificationSummaryListener) {
|
||||
throw new Error("verification.summary listener was not registered");
|
||||
}
|
||||
|
||||
roomEventListener("!fallback:example.org", {
|
||||
event_id: "$start-fallback",
|
||||
sender: "@alice:example.org",
|
||||
type: "m.key.verification.start",
|
||||
origin_server_ts: Date.now(),
|
||||
content: {
|
||||
"m.relates_to": { event_id: "$req-fallback" },
|
||||
},
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(sendMessage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
sendMessage.mockClear();
|
||||
|
||||
verificationSummaryListener({
|
||||
id: "verification-stale-room",
|
||||
otherUserId: "@alice:example.org",
|
||||
isSelfVerification: false,
|
||||
initiatedByMe: false,
|
||||
phase: 3,
|
||||
phaseName: "started",
|
||||
pending: true,
|
||||
methods: ["m.sas.v1"],
|
||||
canAccept: false,
|
||||
hasSas: true,
|
||||
sas: {
|
||||
decimal: [6158, 1986, 3513],
|
||||
emoji: [
|
||||
["🎁", "Gift"],
|
||||
["🌍", "Globe"],
|
||||
["🐴", "Horse"],
|
||||
],
|
||||
},
|
||||
hasReciprocateQr: false,
|
||||
completed: false,
|
||||
createdAt: new Date("2026-02-25T21:42:54.000Z").toISOString(),
|
||||
updatedAt: new Date("2026-02-25T21:42:55.000Z").toISOString(),
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(sendMessage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect((sendMessage.mock.calls as unknown[][])[0]?.[0]).toBe("!dm:example.org");
|
||||
});
|
||||
|
||||
it("does not emit duplicate SAS notices for the same verification payload", async () => {
|
||||
const { sendMessage, roomEventListener, listVerifications } = createHarness({
|
||||
verifications: [
|
||||
|
|
|
|||
|
|
@ -219,13 +219,11 @@ async function resolveVerificationSummaryForSignal(
|
|||
// Only fall back by user inside the active DM with that user. Otherwise a
|
||||
// spoofed verification event in an unrelated room can leak the current SAS
|
||||
// prompt into that room.
|
||||
if (
|
||||
!(await isStrictDirectRoom({
|
||||
client,
|
||||
roomId: params.roomId,
|
||||
remoteUserId: params.senderId,
|
||||
}))
|
||||
) {
|
||||
const inspection = await inspectMatrixDirectRooms({
|
||||
client,
|
||||
remoteUserId: params.senderId,
|
||||
}).catch(() => null);
|
||||
if (trimMaybeString(inspection?.activeRoomId) !== params.roomId) {
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -364,6 +362,14 @@ export function createMatrixVerificationEventRouter(params: {
|
|||
const verificationFlowRooms = new Map<string, string>();
|
||||
const verificationUserRooms = new Map<string, string>();
|
||||
|
||||
async function resolveActiveDirectRoomId(remoteUserId: string): Promise<string | null> {
|
||||
const inspection = await inspectMatrixDirectRooms({
|
||||
client: params.client,
|
||||
remoteUserId,
|
||||
}).catch(() => null);
|
||||
return trimMaybeString(inspection?.activeRoomId);
|
||||
}
|
||||
|
||||
function shouldEmitVerificationEventNotice(event: MatrixRawEvent): boolean {
|
||||
const eventTs =
|
||||
typeof event.origin_server_ts === "number" && Number.isFinite(event.origin_server_ts)
|
||||
|
|
@ -421,6 +427,13 @@ export function createMatrixVerificationEventRouter(params: {
|
|||
return null;
|
||||
}
|
||||
const recentRoomId = trimMaybeString(verificationUserRooms.get(remoteUserId));
|
||||
const activeRoomId = await resolveActiveDirectRoomId(remoteUserId);
|
||||
if (recentRoomId && activeRoomId && recentRoomId === activeRoomId) {
|
||||
return recentRoomId;
|
||||
}
|
||||
if (activeRoomId) {
|
||||
return activeRoomId;
|
||||
}
|
||||
if (
|
||||
recentRoomId &&
|
||||
(await isStrictDirectRoom({
|
||||
|
|
@ -431,11 +444,7 @@ export function createMatrixVerificationEventRouter(params: {
|
|||
) {
|
||||
return recentRoomId;
|
||||
}
|
||||
const inspection = await inspectMatrixDirectRooms({
|
||||
client: params.client,
|
||||
remoteUserId,
|
||||
}).catch(() => null);
|
||||
return trimMaybeString(inspection?.activeRoomId);
|
||||
return null;
|
||||
}
|
||||
|
||||
async function routeVerificationSummary(summary: MatrixVerificationSummaryLike): Promise<void> {
|
||||
|
|
|
|||
|
|
@ -51,6 +51,30 @@ describe("resolveMatrixRoomId", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("prefers joined rooms marked direct in member state over plain strict rooms", 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", "!explicit:example.org"]),
|
||||
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,
|
||||
})),
|
||||
setAccountData: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as MatrixClient;
|
||||
|
||||
const resolved = await resolveMatrixRoomId(client, userId);
|
||||
|
||||
expect(resolved).toBe("!explicit:example.org");
|
||||
expect(client.setAccountData).toHaveBeenCalledWith(
|
||||
EventType.Direct,
|
||||
expect.objectContaining({ [userId]: ["!explicit: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