mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(matrix): restore robust DM routing without the memberCount heuristic (#19736)
* fix(matrix): remove memberCount heuristic from DM detection The memberCount === 2 check in isDirectMessage() misclassifies 2-person group rooms (admin channels, monitoring rooms) as DMs, routing them to the main session instead of their room-specific session. Matrix already distinguishes DMs from groups at the protocol level via m.direct account data and is_direct member state flags. Both are already checked by client.dms.isDm() and hasDirectFlag(). The memberCount heuristic only adds false positives for 2-person groups. Move resolveMemberCount() below the protocol-level checks so it is only reached for rooms not matched by m.direct or is_direct. This narrows its role to diagnostic logging for confirmed group rooms. Refs: #19739 * fix(matrix): add conservative fallback for broken DM flags Some homeservers (notably Continuwuity) have broken m.direct account data or never set is_direct on invite events. With the memberCount heuristic removed, these DMs are no longer detected. Add a conservative fallback that requires two signals before classifying as DM: memberCount === 2 AND no explicit m.room.name. Group rooms almost always have explicit names; DMs almost never do. Error handling distinguishes M_NOT_FOUND (missing state event, expected for unnamed rooms) from network/auth errors. Non-404 errors fall through to group classification rather than guessing. This is independently revertable — removing this commit restores pure protocol-based detection without any heuristic fallback. * fix(matrix): add parentPeer for DM room binding support Add parentPeer to DM routes so conversations are bindable by room ID while preserving DM trust semantics (secure 1:1, no group restrictions). Suggested by @KirillShchetinin. * fix(matrix): override DM detection for explicitly configured rooms Builds on @robertcorreiro's config-driven approach from #9106. Move resolveMatrixRoomConfig() before the DM check. If a room matches a non-wildcard config entry (matchSource === "direct") and was classified as DM, override the classification to group. This gives users a deterministic escape hatch for misclassified rooms. Wildcards are excluded from the override to avoid breaking DM routing when a "*" catch-all exists. roomConfig is gated behind isRoom so DMs never inherit group settings (skills, systemPrompt, autoReply). This commit is independently droppable if the scope is too broad. * test(matrix): add DM detection and config override tests - 15 unit tests for direct.ts: all detection paths, priority order, M_NOT_FOUND vs network error handling, edge cases (whitespace names, API failures) - 8 unit tests for rooms.ts: matchSource classification, wildcard safety for DM override, direct match priority over wildcard * Changelog: note matrix DM routing follow-up * fix(matrix): preserve DM fallback and room bindings --------- Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -1,65 +1,400 @@
|
||||
import type { MatrixClient } from "@vector-im/matrix-bot-sdk";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createDirectRoomTracker } from "./direct.js";
|
||||
|
||||
function createMockClient(params: {
|
||||
isDm?: boolean;
|
||||
senderDirect?: boolean;
|
||||
selfDirect?: boolean;
|
||||
members?: string[];
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers -- minimal MatrixClient stub
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
type StateEvent = Record<string, unknown>;
|
||||
type DmMap = Record<string, boolean>;
|
||||
|
||||
function createMockClient(opts: {
|
||||
dmRooms?: DmMap;
|
||||
membersByRoom?: Record<string, string[]>;
|
||||
stateEvents?: Record<string, StateEvent>;
|
||||
selfUserId?: string;
|
||||
}) {
|
||||
const members = params.members ?? ["@alice:example.org", "@bot:example.org"];
|
||||
const {
|
||||
dmRooms = {},
|
||||
membersByRoom = {},
|
||||
stateEvents = {},
|
||||
selfUserId = "@bot:example.org",
|
||||
} = opts;
|
||||
|
||||
return {
|
||||
dms: {
|
||||
isDm: (roomId: string) => dmRooms[roomId] ?? false,
|
||||
update: vi.fn().mockResolvedValue(undefined),
|
||||
isDm: vi.fn().mockReturnValue(params.isDm === true),
|
||||
},
|
||||
getUserId: vi.fn().mockResolvedValue("@bot:example.org"),
|
||||
getJoinedRoomMembers: vi.fn().mockResolvedValue(members),
|
||||
getUserId: vi.fn().mockResolvedValue(selfUserId),
|
||||
getJoinedRoomMembers: vi.fn().mockImplementation(async (roomId: string) => {
|
||||
return membersByRoom[roomId] ?? [];
|
||||
}),
|
||||
getRoomStateEvent: vi
|
||||
.fn()
|
||||
.mockImplementation(async (_roomId: string, _event: string, stateKey: string) => {
|
||||
if (stateKey === "@alice:example.org") {
|
||||
return { is_direct: params.senderDirect === true };
|
||||
.mockImplementation(async (roomId: string, eventType: string, stateKey: string) => {
|
||||
const key = `${roomId}|${eventType}|${stateKey}`;
|
||||
const ev = stateEvents[key];
|
||||
if (ev === undefined) {
|
||||
// Simulate real homeserver M_NOT_FOUND response (matches MatrixError shape)
|
||||
const err = new Error(`State event not found: ${key}`) as Error & {
|
||||
errcode?: string;
|
||||
statusCode?: number;
|
||||
};
|
||||
err.errcode = "M_NOT_FOUND";
|
||||
err.statusCode = 404;
|
||||
throw err;
|
||||
}
|
||||
if (stateKey === "@bot:example.org") {
|
||||
return { is_direct: params.selfDirect === true };
|
||||
}
|
||||
return {};
|
||||
return ev;
|
||||
}),
|
||||
} as unknown as MatrixClient;
|
||||
};
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests -- isDirectMessage
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("createDirectRoomTracker", () => {
|
||||
it("treats m.direct rooms as DMs", async () => {
|
||||
const tracker = createDirectRoomTracker(createMockClient({ isDm: true }));
|
||||
await expect(
|
||||
tracker.isDirectMessage({
|
||||
roomId: "!room:example.org",
|
||||
describe("m.direct detection (SDK DM cache)", () => {
|
||||
it("returns true when SDK DM cache marks room as DM", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: { "!dm:example.org": true },
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!dm:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
}),
|
||||
).resolves.toBe(true);
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for rooms not in SDK DM cache (with >2 members)", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!group:example.org": ["@alice:example.org", "@bob:example.org", "@carol:example.org"],
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!group:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
it("does not classify 2-member rooms as DMs without direct flags", async () => {
|
||||
const client = createMockClient({ isDm: false });
|
||||
const tracker = createDirectRoomTracker(client);
|
||||
await expect(
|
||||
tracker.isDirectMessage({
|
||||
describe("is_direct state flag detection", () => {
|
||||
it("returns true when sender's membership has is_direct=true", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: { "!room:example.org": ["@alice:example.org", "@bot:example.org"] },
|
||||
stateEvents: {
|
||||
"!room:example.org|m.room.member|@alice:example.org": { is_direct: true },
|
||||
"!room:example.org|m.room.member|@bot:example.org": { is_direct: false },
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!room:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
}),
|
||||
).resolves.toBe(false);
|
||||
expect(client.getJoinedRoomMembers).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true when bot's own membership has is_direct=true", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: { "!room:example.org": ["@alice:example.org", "@bot:example.org"] },
|
||||
stateEvents: {
|
||||
"!room:example.org|m.room.member|@alice:example.org": { is_direct: false },
|
||||
"!room:example.org|m.room.member|@bot:example.org": { is_direct: true },
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!room:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
selfUserId: "@bot:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it("uses is_direct member flags when present", async () => {
|
||||
const tracker = createDirectRoomTracker(createMockClient({ senderDirect: true }));
|
||||
await expect(
|
||||
tracker.isDirectMessage({
|
||||
describe("conservative fallback (memberCount + room name)", () => {
|
||||
it("returns true for 2-member room WITHOUT a room name (broken flags)", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!broken-dm:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
// is_direct not set on either member (e.g. Continuwuity bug)
|
||||
"!broken-dm:example.org|m.room.member|@alice:example.org": {},
|
||||
"!broken-dm:example.org|m.room.member|@bot:example.org": {},
|
||||
// No m.room.name -> getRoomStateEvent will throw (event not found)
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!broken-dm:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true for 2-member room with empty room name", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!broken-dm:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!broken-dm:example.org|m.room.member|@alice:example.org": {},
|
||||
"!broken-dm:example.org|m.room.member|@bot:example.org": {},
|
||||
"!broken-dm:example.org|m.room.name|": { name: "" },
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!broken-dm:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for 2-member room WITH a room name (named group)", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!named-group:example.org": ["@alice:example.org", "@bob:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!named-group:example.org|m.room.member|@alice:example.org": {},
|
||||
"!named-group:example.org|m.room.member|@bob:example.org": {},
|
||||
"!named-group:example.org|m.room.name|": { name: "Project Alpha" },
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!named-group:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for 3+ member room without any DM signals", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!group:example.org": ["@alice:example.org", "@bob:example.org", "@carol:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!group:example.org|m.room.member|@alice:example.org": {},
|
||||
"!group:example.org|m.room.member|@bob:example.org": {},
|
||||
"!group:example.org|m.room.member|@carol:example.org": {},
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!group:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for 1-member room (self-chat)", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!solo:example.org": ["@bot:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!solo:example.org|m.room.member|@bot:example.org": {},
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!solo:example.org",
|
||||
senderId: "@bot:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("detection priority", () => {
|
||||
it("m.direct takes priority -- skips state and fallback checks", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: { "!dm:example.org": true },
|
||||
membersByRoom: {
|
||||
"!dm:example.org": ["@alice:example.org", "@bob:example.org", "@carol:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!dm:example.org|m.room.name|": { name: "Named Room" },
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!dm:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
// Should not have checked member state or room name
|
||||
expect(client.getRoomStateEvent).not.toHaveBeenCalled();
|
||||
expect(client.getJoinedRoomMembers).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("is_direct takes priority over fallback -- skips member count", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
stateEvents: {
|
||||
"!room:example.org|m.room.member|@alice:example.org": { is_direct: true },
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!room:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
}),
|
||||
).resolves.toBe(true);
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
// Should not have checked member count
|
||||
expect(client.getJoinedRoomMembers).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("edge cases", () => {
|
||||
it("handles member count API failure gracefully", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
stateEvents: {
|
||||
"!failing:example.org|m.room.member|@alice:example.org": {},
|
||||
"!failing:example.org|m.room.member|@bot:example.org": {},
|
||||
},
|
||||
});
|
||||
client.getJoinedRoomMembers.mockRejectedValue(new Error("API unavailable"));
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!failing:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
// Cannot determine member count -> conservative: classify as group
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("treats M_NOT_FOUND for room name as no name (DM)", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!no-name:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!no-name:example.org|m.room.member|@alice:example.org": {},
|
||||
"!no-name:example.org|m.room.member|@bot:example.org": {},
|
||||
// m.room.name not in stateEvents -> mock throws generic Error
|
||||
},
|
||||
});
|
||||
// Override to throw M_NOT_FOUND like a real homeserver
|
||||
const originalImpl = client.getRoomStateEvent.getMockImplementation()!;
|
||||
client.getRoomStateEvent.mockImplementation(
|
||||
async (roomId: string, eventType: string, stateKey: string) => {
|
||||
if (eventType === "m.room.name") {
|
||||
const err = new Error("not found") as Error & {
|
||||
errcode?: string;
|
||||
statusCode?: number;
|
||||
};
|
||||
err.errcode = "M_NOT_FOUND";
|
||||
err.statusCode = 404;
|
||||
throw err;
|
||||
}
|
||||
return originalImpl(roomId, eventType, stateKey);
|
||||
},
|
||||
);
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!no-name:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it("treats non-404 room name errors as unknown (falls through to group)", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!error-room:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!error-room:example.org|m.room.member|@alice:example.org": {},
|
||||
"!error-room:example.org|m.room.member|@bot:example.org": {},
|
||||
},
|
||||
});
|
||||
// Simulate a network/auth error (not M_NOT_FOUND)
|
||||
const originalImpl = client.getRoomStateEvent.getMockImplementation()!;
|
||||
client.getRoomStateEvent.mockImplementation(
|
||||
async (roomId: string, eventType: string, stateKey: string) => {
|
||||
if (eventType === "m.room.name") {
|
||||
throw new Error("Connection refused");
|
||||
}
|
||||
return originalImpl(roomId, eventType, stateKey);
|
||||
},
|
||||
);
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!error-room:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
// Network error -> don't assume DM, classify as group
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("whitespace-only room name is treated as no name", async () => {
|
||||
const client = createMockClient({
|
||||
dmRooms: {},
|
||||
membersByRoom: {
|
||||
"!ws-name:example.org": ["@alice:example.org", "@bot:example.org"],
|
||||
},
|
||||
stateEvents: {
|
||||
"!ws-name:example.org|m.room.member|@alice:example.org": {},
|
||||
"!ws-name:example.org|m.room.member|@bot:example.org": {},
|
||||
"!ws-name:example.org|m.room.name|": { name: " " },
|
||||
},
|
||||
});
|
||||
const tracker = createDirectRoomTracker(client as never);
|
||||
|
||||
const result = await tracker.isDirectMessage({
|
||||
roomId: "!ws-name:example.org",
|
||||
senderId: "@alice:example.org",
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -13,14 +13,22 @@ type DirectRoomTrackerOptions = {
|
||||
|
||||
const DM_CACHE_TTL_MS = 30_000;
|
||||
|
||||
/**
|
||||
* Check if an error is a Matrix M_NOT_FOUND response (missing state event).
|
||||
* The bot-sdk throws MatrixError with errcode/statusCode on the error object.
|
||||
*/
|
||||
function isMatrixNotFoundError(err: unknown): boolean {
|
||||
if (typeof err !== "object" || err === null) return false;
|
||||
const e = err as { errcode?: string; statusCode?: number };
|
||||
return e.errcode === "M_NOT_FOUND" || e.statusCode === 404;
|
||||
}
|
||||
|
||||
export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTrackerOptions = {}) {
|
||||
const log = opts.log ?? (() => {});
|
||||
const includeMemberCountInLogs = opts.includeMemberCountInLogs === true;
|
||||
let lastDmUpdateMs = 0;
|
||||
let cachedSelfUserId: string | null = null;
|
||||
const memberCountCache = includeMemberCountInLogs
|
||||
? new Map<string, { count: number; ts: number }>()
|
||||
: undefined;
|
||||
const memberCountCache = new Map<string, { count: number; ts: number }>();
|
||||
|
||||
const ensureSelfUserId = async (): Promise<string | null> => {
|
||||
if (cachedSelfUserId) {
|
||||
@@ -48,9 +56,6 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
|
||||
};
|
||||
|
||||
const resolveMemberCount = async (roomId: string): Promise<number | null> => {
|
||||
if (!memberCountCache) {
|
||||
return null;
|
||||
}
|
||||
const cached = memberCountCache.get(roomId);
|
||||
const now = Date.now();
|
||||
if (cached && now - cached.ts < DM_CACHE_TTL_MS) {
|
||||
@@ -91,7 +96,6 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check m.room.member state for is_direct flag
|
||||
const selfUserId = params.selfUserId ?? (await ensureSelfUserId());
|
||||
const directViaState =
|
||||
(await hasDirectFlag(roomId, senderId)) || (await hasDirectFlag(roomId, selfUserId ?? ""));
|
||||
@@ -100,16 +104,47 @@ export function createDirectRoomTracker(client: MatrixClient, opts: DirectRoomTr
|
||||
return true;
|
||||
}
|
||||
|
||||
// Member count alone is NOT a reliable DM indicator.
|
||||
// Explicitly configured group rooms with 2 members (e.g. bot + one user)
|
||||
// were being misclassified as DMs, causing messages to be routed through
|
||||
// DM policy instead of group policy and silently dropped.
|
||||
// See: https://github.com/openclaw/openclaw/issues/20145
|
||||
// Conservative fallback: 2-member rooms without an explicit room name are likely
|
||||
// DMs with broken m.direct / is_direct flags. This has been observed on Continuwuity
|
||||
// where m.direct pointed to the wrong room and is_direct was never set on the invite.
|
||||
// Unlike the removed heuristic, this requires two signals (member count + no name)
|
||||
// to avoid false positives on named 2-person group rooms.
|
||||
//
|
||||
// Performance: member count is cached (resolveMemberCount). The room name state
|
||||
// check is not cached but only runs for the subset of 2-member rooms that reach
|
||||
// this fallback path (no m.direct, no is_direct). In typical deployments this is
|
||||
// a small minority of rooms.
|
||||
//
|
||||
// Note: there is a narrow race where a room name is being set concurrently with
|
||||
// this check. The consequence is a one-time misclassification that self-corrects
|
||||
// on the next message (once the state event is synced). This is acceptable given
|
||||
// the alternative of an additional API call on every message.
|
||||
const memberCount = await resolveMemberCount(roomId);
|
||||
if (memberCount === 2) {
|
||||
try {
|
||||
const nameState = await client.getRoomStateEvent(roomId, "m.room.name", "");
|
||||
if (!nameState?.name?.trim()) {
|
||||
log(`matrix: dm detected via fallback (2 members, no room name) room=${roomId}`);
|
||||
return true;
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
// Missing state events (M_NOT_FOUND) are expected for unnamed rooms and
|
||||
// strongly indicate a DM. Any other error (network, auth) is ambiguous,
|
||||
// so we fall through to classify as group rather than guess.
|
||||
if (isMatrixNotFoundError(err)) {
|
||||
log(`matrix: dm detected via fallback (2 members, no room name) room=${roomId}`);
|
||||
return true;
|
||||
}
|
||||
log(
|
||||
`matrix: dm fallback skipped (room name check failed: ${String(err)}) room=${roomId}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (!includeMemberCountInLogs) {
|
||||
log(`matrix: dm check room=${roomId} result=group`);
|
||||
return false;
|
||||
}
|
||||
const memberCount = await resolveMemberCount(roomId);
|
||||
log(`matrix: dm check room=${roomId} result=group members=${memberCount ?? "unknown"}`);
|
||||
return false;
|
||||
},
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import type { MatrixClient } from "@vector-im/matrix-bot-sdk";
|
||||
import type { PluginRuntime, RuntimeEnv, RuntimeLogger } from "openclaw/plugin-sdk/matrix";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createMatrixRoomMessageHandler } from "./handler.js";
|
||||
import {
|
||||
createMatrixRoomMessageHandler,
|
||||
resolveMatrixBaseRouteSession,
|
||||
shouldOverrideMatrixDmToGroup,
|
||||
} from "./handler.js";
|
||||
import { EventType, type MatrixRawEvent } from "./types.js";
|
||||
|
||||
describe("createMatrixRoomMessageHandler BodyForAgent sender label", () => {
|
||||
@@ -18,8 +22,15 @@ describe("createMatrixRoomMessageHandler BodyForAgent sender label", () => {
|
||||
channel: {
|
||||
pairing: {
|
||||
readAllowFromStore: vi.fn().mockResolvedValue([]),
|
||||
upsertPairingRequest: vi.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
routing: {
|
||||
buildAgentSessionKey: vi
|
||||
.fn()
|
||||
.mockImplementation(
|
||||
(params: { agentId: string; channel: string; peer?: { kind: string; id: string } }) =>
|
||||
`agent:${params.agentId}:${params.channel}:${params.peer?.kind ?? "direct"}:${params.peer?.id ?? "unknown"}`,
|
||||
),
|
||||
resolveAgentRoute: vi.fn().mockReturnValue({
|
||||
agentId: "main",
|
||||
accountId: undefined,
|
||||
@@ -139,4 +150,47 @@ describe("createMatrixRoomMessageHandler BodyForAgent sender label", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses room-scoped session keys for DM rooms matched via parentPeer binding", () => {
|
||||
const buildAgentSessionKey = vi
|
||||
.fn()
|
||||
.mockReturnValue("agent:main:matrix:channel:!dmroom:example.org");
|
||||
|
||||
const resolved = resolveMatrixBaseRouteSession({
|
||||
buildAgentSessionKey,
|
||||
baseRoute: {
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
mainSessionKey: "agent:main:main",
|
||||
matchedBy: "binding.peer.parent",
|
||||
},
|
||||
isDirectMessage: true,
|
||||
roomId: "!dmroom:example.org",
|
||||
accountId: undefined,
|
||||
});
|
||||
|
||||
expect(buildAgentSessionKey).toHaveBeenCalledWith({
|
||||
agentId: "main",
|
||||
channel: "matrix",
|
||||
accountId: undefined,
|
||||
peer: { kind: "channel", id: "!dmroom:example.org" },
|
||||
});
|
||||
expect(resolved).toEqual({
|
||||
sessionKey: "agent:main:matrix:channel:!dmroom:example.org",
|
||||
lastRoutePolicy: "session",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not override DMs to groups for explicit allow:false room config", () => {
|
||||
expect(
|
||||
shouldOverrideMatrixDmToGroup({
|
||||
isDirectMessage: true,
|
||||
roomConfigInfo: {
|
||||
config: { allow: false },
|
||||
allowed: false,
|
||||
matchSource: "direct",
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -77,6 +77,56 @@ export type MatrixMonitorHandlerParams = {
|
||||
accountId?: string | null;
|
||||
};
|
||||
|
||||
export function resolveMatrixBaseRouteSession(params: {
|
||||
buildAgentSessionKey: (params: {
|
||||
agentId: string;
|
||||
channel: string;
|
||||
accountId?: string | null;
|
||||
peer?: { kind: "direct" | "channel"; id: string } | null;
|
||||
}) => string;
|
||||
baseRoute: {
|
||||
agentId: string;
|
||||
sessionKey: string;
|
||||
mainSessionKey: string;
|
||||
matchedBy?: string;
|
||||
};
|
||||
isDirectMessage: boolean;
|
||||
roomId: string;
|
||||
accountId?: string | null;
|
||||
}): { sessionKey: string; lastRoutePolicy: "main" | "session" } {
|
||||
const sessionKey =
|
||||
params.isDirectMessage && params.baseRoute.matchedBy === "binding.peer.parent"
|
||||
? params.buildAgentSessionKey({
|
||||
agentId: params.baseRoute.agentId,
|
||||
channel: "matrix",
|
||||
accountId: params.accountId,
|
||||
peer: { kind: "channel", id: params.roomId },
|
||||
})
|
||||
: params.baseRoute.sessionKey;
|
||||
return {
|
||||
sessionKey,
|
||||
lastRoutePolicy: sessionKey === params.baseRoute.mainSessionKey ? "main" : "session",
|
||||
};
|
||||
}
|
||||
|
||||
export function shouldOverrideMatrixDmToGroup(params: {
|
||||
isDirectMessage: boolean;
|
||||
roomConfigInfo?:
|
||||
| {
|
||||
config?: MatrixRoomConfig;
|
||||
allowed: boolean;
|
||||
matchSource?: string;
|
||||
}
|
||||
| undefined;
|
||||
}): boolean {
|
||||
return (
|
||||
params.isDirectMessage === true &&
|
||||
params.roomConfigInfo?.config !== undefined &&
|
||||
params.roomConfigInfo.allowed === true &&
|
||||
params.roomConfigInfo.matchSource === "direct"
|
||||
);
|
||||
}
|
||||
|
||||
export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParams) {
|
||||
const {
|
||||
client,
|
||||
@@ -188,22 +238,37 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
|
||||
}
|
||||
}
|
||||
|
||||
const isDirectMessage = await directTracker.isDirectMessage({
|
||||
let isDirectMessage = await directTracker.isDirectMessage({
|
||||
roomId,
|
||||
senderId,
|
||||
selfUserId,
|
||||
});
|
||||
|
||||
// Resolve room config early so explicitly configured rooms can override DM classification.
|
||||
// This ensures rooms in the groups config are always treated as groups regardless of
|
||||
// member count or protocol-level DM flags. Only explicit matches (not wildcards) trigger
|
||||
// the override to avoid breaking DM routing when a wildcard entry exists. (See #9106)
|
||||
const roomConfigInfo = resolveMatrixRoomConfig({
|
||||
rooms: roomsConfig,
|
||||
roomId,
|
||||
aliases: roomAliases,
|
||||
name: roomName,
|
||||
});
|
||||
if (shouldOverrideMatrixDmToGroup({ isDirectMessage, roomConfigInfo })) {
|
||||
logVerboseMessage(
|
||||
`matrix: overriding DM to group for configured room=${roomId} (${roomConfigInfo.matchKey})`,
|
||||
);
|
||||
isDirectMessage = false;
|
||||
}
|
||||
|
||||
const isRoom = !isDirectMessage;
|
||||
|
||||
const roomConfigInfo = isRoom
|
||||
? resolveMatrixRoomConfig({
|
||||
rooms: roomsConfig,
|
||||
roomId,
|
||||
aliases: roomAliases,
|
||||
name: roomName,
|
||||
})
|
||||
: undefined;
|
||||
const roomConfig = roomConfigInfo?.config;
|
||||
if (isRoom && groupPolicy === "disabled") {
|
||||
return;
|
||||
}
|
||||
// Only expose room config for confirmed group rooms. DMs should never inherit
|
||||
// group settings (skills, systemPrompt, autoReply) even when a wildcard entry exists.
|
||||
const roomConfig = isRoom ? roomConfigInfo?.config : undefined;
|
||||
const roomMatchMeta = roomConfigInfo
|
||||
? `matchKey=${roomConfigInfo.matchKey ?? "none"} matchSource=${
|
||||
roomConfigInfo.matchSource ?? "none"
|
||||
@@ -435,13 +500,24 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
|
||||
kind: isDirectMessage ? "direct" : "channel",
|
||||
id: isDirectMessage ? senderId : roomId,
|
||||
},
|
||||
// For DMs, pass roomId as parentPeer so the conversation is bindable by room ID
|
||||
// while preserving DM trust semantics (secure 1:1, no group restrictions).
|
||||
parentPeer: isDirectMessage ? { kind: "channel", id: roomId } : undefined,
|
||||
});
|
||||
const baseRouteSession = resolveMatrixBaseRouteSession({
|
||||
buildAgentSessionKey: core.channel.routing.buildAgentSessionKey,
|
||||
baseRoute,
|
||||
isDirectMessage,
|
||||
roomId,
|
||||
accountId,
|
||||
});
|
||||
|
||||
const route = {
|
||||
...baseRoute,
|
||||
lastRoutePolicy: baseRouteSession.lastRoutePolicy,
|
||||
sessionKey: threadRootId
|
||||
? `${baseRoute.sessionKey}:thread:${threadRootId}`
|
||||
: baseRoute.sessionKey,
|
||||
? `${baseRouteSession.sessionKey}:thread:${threadRootId}`
|
||||
: baseRouteSession.sessionKey,
|
||||
};
|
||||
|
||||
let threadStarterBody: string | undefined;
|
||||
|
||||
@@ -36,4 +36,89 @@ describe("resolveMatrixRoomConfig", () => {
|
||||
expect(byName.allowed).toBe(false);
|
||||
expect(byName.config).toBeUndefined();
|
||||
});
|
||||
|
||||
describe("matchSource classification", () => {
|
||||
it('returns matchSource="direct" for exact room ID match', () => {
|
||||
const result = resolveMatrixRoomConfig({
|
||||
rooms: { "!room:example.org": { allow: true } },
|
||||
roomId: "!room:example.org",
|
||||
aliases: [],
|
||||
});
|
||||
expect(result.matchSource).toBe("direct");
|
||||
expect(result.config).toBeDefined();
|
||||
});
|
||||
|
||||
it('returns matchSource="direct" for alias match', () => {
|
||||
const result = resolveMatrixRoomConfig({
|
||||
rooms: { "#alias:example.org": { allow: true } },
|
||||
roomId: "!room:example.org",
|
||||
aliases: ["#alias:example.org"],
|
||||
});
|
||||
expect(result.matchSource).toBe("direct");
|
||||
expect(result.config).toBeDefined();
|
||||
});
|
||||
|
||||
it('returns matchSource="wildcard" for wildcard match', () => {
|
||||
const result = resolveMatrixRoomConfig({
|
||||
rooms: { "*": { allow: true } },
|
||||
roomId: "!any:example.org",
|
||||
aliases: [],
|
||||
});
|
||||
expect(result.matchSource).toBe("wildcard");
|
||||
expect(result.config).toBeDefined();
|
||||
});
|
||||
|
||||
it("returns undefined matchSource when no match", () => {
|
||||
const result = resolveMatrixRoomConfig({
|
||||
rooms: { "!other:example.org": { allow: true } },
|
||||
roomId: "!room:example.org",
|
||||
aliases: [],
|
||||
});
|
||||
expect(result.matchSource).toBeUndefined();
|
||||
expect(result.config).toBeUndefined();
|
||||
});
|
||||
|
||||
it("direct match takes priority over wildcard", () => {
|
||||
const result = resolveMatrixRoomConfig({
|
||||
rooms: {
|
||||
"!room:example.org": { allow: true, systemPrompt: "room-specific" },
|
||||
"*": { allow: true, systemPrompt: "generic" },
|
||||
},
|
||||
roomId: "!room:example.org",
|
||||
aliases: [],
|
||||
});
|
||||
expect(result.matchSource).toBe("direct");
|
||||
expect(result.config?.systemPrompt).toBe("room-specific");
|
||||
});
|
||||
});
|
||||
|
||||
describe("DM override safety (matchSource distinction)", () => {
|
||||
// These tests verify the matchSource property that handler.ts uses
|
||||
// to decide whether a configured room should override DM classification.
|
||||
// Only "direct" matches should trigger the override -- never "wildcard".
|
||||
|
||||
it("wildcard config should NOT be usable to override DM classification", () => {
|
||||
const result = resolveMatrixRoomConfig({
|
||||
rooms: { "*": { allow: true, skills: ["general"] } },
|
||||
roomId: "!dm-room:example.org",
|
||||
aliases: [],
|
||||
});
|
||||
// handler.ts checks: matchSource === "direct" -> this is "wildcard", so no override
|
||||
expect(result.matchSource).not.toBe("direct");
|
||||
expect(result.matchSource).toBe("wildcard");
|
||||
});
|
||||
|
||||
it("explicitly configured room should be usable to override DM classification", () => {
|
||||
const result = resolveMatrixRoomConfig({
|
||||
rooms: {
|
||||
"!configured-room:example.org": { allow: true },
|
||||
"*": { allow: true },
|
||||
},
|
||||
roomId: "!configured-room:example.org",
|
||||
aliases: [],
|
||||
});
|
||||
// handler.ts checks: matchSource === "direct" -> this IS "direct", so override is safe
|
||||
expect(result.matchSource).toBe("direct");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user