fix: improve matrix direct room resolution (#1436) (thanks @sibbl) (#1486)

* fix: improve matrix direct room resolution (#1436) (thanks @sibbl)

* docs: update changelog for matrix fix (#1486) (thanks @sibbl)

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 <final> tags and error turns intact.

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");
+  });
+});

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<string, string>();
+
+async function persistDirectRoom(
+  client: MatrixClient,
+  userId: string,
+  roomId: string,
+): Promise<void> {
+  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)`);
 }