diff --git a/CHANGELOG.md b/CHANGELOG.md index 113d75119..1bba27755 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ Docs: https://docs.clawd.bot - Doctor: honor CLAWDBOT_GATEWAY_TOKEN for auth checks and security audit token reuse. (#1448) Thanks @azade-c. - Agents: make tool summaries more readable and only show optional params when set. - Agents: honor SOUL.md guidance even when the file is nested or path-qualified. (#1434) Thanks @neooriginal. +- Matrix (plugin): persist m.direct for resolved DMs and harden room fallback. (#1436, #1486) Thanks @sibbl. - CLI: prefer `~` for home paths in output. - Mattermost (plugin): enforce pairing/allowlist gating, keep @username targets, and clarify plugin-only docs. (#1428) Thanks @damoahdominic. - Agents: centralize transcript sanitization in the runner; keep tags and error turns intact. diff --git a/extensions/matrix/src/matrix/send/targets.test.ts b/extensions/matrix/src/matrix/send/targets.test.ts new file mode 100644 index 000000000..18499f895 --- /dev/null +++ b/extensions/matrix/src/matrix/send/targets.test.ts @@ -0,0 +1,102 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { MatrixClient } from "matrix-bot-sdk"; +import { EventType } from "./types.js"; + +let resolveMatrixRoomId: typeof import("./targets.js").resolveMatrixRoomId; +let normalizeThreadId: typeof import("./targets.js").normalizeThreadId; + +beforeEach(async () => { + vi.resetModules(); + ({ resolveMatrixRoomId, normalizeThreadId } = await import("./targets.js")); +}); + +describe("resolveMatrixRoomId", () => { + it("uses m.direct when available", async () => { + const userId = "@user:example.org"; + const client = { + getAccountData: vi.fn().mockResolvedValue({ + [userId]: ["!room:example.org"], + }), + getJoinedRooms: vi.fn(), + getJoinedRoomMembers: vi.fn(), + setAccountData: vi.fn(), + } as unknown as MatrixClient; + + const roomId = await resolveMatrixRoomId(client, userId); + + expect(roomId).toBe("!room:example.org"); + expect(client.getJoinedRooms).not.toHaveBeenCalled(); + expect(client.setAccountData).not.toHaveBeenCalled(); + }); + + it("falls back to joined rooms and persists m.direct", async () => { + const userId = "@fallback:example.org"; + const roomId = "!room:example.org"; + const setAccountData = vi.fn().mockResolvedValue(undefined); + const client = { + getAccountData: vi.fn().mockRejectedValue(new Error("nope")), + getJoinedRooms: vi.fn().mockResolvedValue([roomId]), + getJoinedRoomMembers: vi.fn().mockResolvedValue([ + "@bot:example.org", + userId, + ]), + setAccountData, + } as unknown as MatrixClient; + + const resolved = await resolveMatrixRoomId(client, userId); + + expect(resolved).toBe(roomId); + expect(setAccountData).toHaveBeenCalledWith( + EventType.Direct, + expect.objectContaining({ [userId]: [roomId] }), + ); + }); + + it("continues when a room member lookup fails", async () => { + const userId = "@continue:example.org"; + const roomId = "!good:example.org"; + const setAccountData = vi.fn().mockResolvedValue(undefined); + const getJoinedRoomMembers = vi + .fn() + .mockRejectedValueOnce(new Error("boom")) + .mockResolvedValueOnce(["@bot:example.org", userId]); + const client = { + getAccountData: vi.fn().mockRejectedValue(new Error("nope")), + getJoinedRooms: vi.fn().mockResolvedValue(["!bad:example.org", roomId]), + getJoinedRoomMembers, + setAccountData, + } as unknown as MatrixClient; + + const resolved = await resolveMatrixRoomId(client, userId); + + expect(resolved).toBe(roomId); + expect(setAccountData).toHaveBeenCalled(); + }); + + it("allows larger rooms when no 1:1 match exists", async () => { + const userId = "@group:example.org"; + const roomId = "!group:example.org"; + const client = { + getAccountData: vi.fn().mockRejectedValue(new Error("nope")), + getJoinedRooms: vi.fn().mockResolvedValue([roomId]), + getJoinedRoomMembers: vi.fn().mockResolvedValue([ + "@bot:example.org", + userId, + "@extra:example.org", + ]), + setAccountData: vi.fn().mockResolvedValue(undefined), + } as unknown as MatrixClient; + + const resolved = await resolveMatrixRoomId(client, userId); + + expect(resolved).toBe(roomId); + }); +}); + +describe("normalizeThreadId", () => { + it("returns null for empty thread ids", () => { + expect(normalizeThreadId(" ")).toBeNull(); + expect(normalizeThreadId("$thread")).toBe("$thread"); + }); +}); diff --git a/extensions/matrix/src/matrix/send/targets.ts b/extensions/matrix/src/matrix/send/targets.ts index 33286d7d9..dde734ba2 100644 --- a/extensions/matrix/src/matrix/send/targets.ts +++ b/extensions/matrix/src/matrix/send/targets.ts @@ -12,7 +12,38 @@ function normalizeTarget(raw: string): string { export function normalizeThreadId(raw?: string | number | null): string | null { if (raw === undefined || raw === null) return null; - return String(raw).trim(); + const trimmed = String(raw).trim(); + return trimmed ? trimmed : null; +} + +const directRoomCache = new Map(); + +async function persistDirectRoom( + client: MatrixClient, + userId: string, + roomId: string, +): Promise { + let directContent: MatrixDirectAccountData | null = null; + try { + directContent = (await client.getAccountData( + EventType.Direct, + )) as MatrixDirectAccountData | null; + } catch { + // Ignore fetch errors and fall back to an empty map. + } + const existing = + directContent && !Array.isArray(directContent) ? directContent : {}; + const current = Array.isArray(existing[userId]) ? existing[userId] : []; + if (current[0] === roomId) return; + const next = [roomId, ...current.filter((id) => id !== roomId)]; + try { + await client.setAccountData(EventType.Direct, { + ...existing, + [userId]: next, + }); + } catch { + // Ignore persistence errors. + } } async function resolveDirectRoomId( @@ -26,6 +57,9 @@ async function resolveDirectRoomId( ); } + const cached = directRoomCache.get(trimmed); + if (cached) return cached; + // 1) Fast path: use account data (m.direct) for *this* logged-in user (the bot). try { const directContent = (await client.getAccountData( @@ -34,26 +68,47 @@ async function resolveDirectRoomId( const list = Array.isArray(directContent?.[trimmed]) ? directContent[trimmed] : []; - if (list.length > 0) return list[0]; + if (list.length > 0) { + directRoomCache.set(trimmed, list[0]); + return list[0]; + } } catch { // Ignore and fall back. } // 2) Fallback: look for an existing joined room that looks like a 1:1 with the user. // Many clients only maintain m.direct for *their own* account data, so relying on it is brittle. + let fallbackRoom: string | null = null; try { const rooms = await client.getJoinedRooms(); for (const roomId of rooms) { - const members = await client.getJoinedRoomMembers(roomId); - // Heuristic: a classic DM has exactly two joined members and includes the target. - if (members.length === 2 && members.includes(trimmed)) { + let members: string[]; + try { + members = await client.getJoinedRoomMembers(roomId); + } catch { + continue; + } + if (!members.includes(trimmed)) continue; + // Prefer classic 1:1 rooms, but allow larger rooms if requested. + if (members.length === 2) { + directRoomCache.set(trimmed, roomId); + await persistDirectRoom(client, trimmed, roomId); return roomId; } + if (!fallbackRoom) { + fallbackRoom = roomId; + } } } catch { // Ignore and fall back. } + if (fallbackRoom) { + directRoomCache.set(trimmed, fallbackRoom); + await persistDirectRoom(client, trimmed, fallbackRoom); + return fallbackRoom; + } + throw new Error(`No direct room found for ${trimmed} (m.direct missing)`); }