mattermost: fix DM media upload for unprefixed user IDs (#29925)

Merged via squash.

Prepared head SHA: 5cffcb072c
Co-authored-by: teconomix <6959299+teconomix@users.noreply.github.com>
Co-authored-by: mukhtharcm <56378562+mukhtharcm@users.noreply.github.com>
Reviewed-by: @mukhtharcm

extensions/mattermost/src/channel.ts

@@ -35,6 +35,7 @@ import { monitorMattermostProvider } from "./mattermost/monitor.js";
 import { probeMattermost } from "./mattermost/probe.js";
 import { addMattermostReaction, removeMattermostReaction } from "./mattermost/reactions.js";
 import { sendMessageMattermost } from "./mattermost/send.js";
+import { resolveMattermostOpaqueTarget } from "./mattermost/target-resolution.js";
 import { looksLikeMattermostTargetId, normalizeMattermostMessagingTarget } from "./normalize.js";
 import { mattermostOnboardingAdapter } from "./onboarding.js";
 import { getMattermostRuntime } from "./runtime.js";
@@ -340,6 +341,21 @@ export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = {
     targetResolver: {
       looksLikeId: looksLikeMattermostTargetId,
       hint: "<channelId|user:ID|channel:ID>",
+      resolveTarget: async ({ cfg, accountId, input }) => {
+        const resolved = await resolveMattermostOpaqueTarget({
+          input,
+          cfg,
+          accountId,
+        });
+        if (!resolved) {
+          return null;
+        }
+        return {
+          to: resolved.to,
+          kind: resolved.kind,
+          source: "directory",
+        };
+      },
     },
   },
   outbound: {

extensions/mattermost/src/mattermost/send.test.ts

@@ -1,5 +1,6 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { parseMattermostTarget, sendMessageMattermost } from "./send.js";
+import { resetMattermostOpaqueTargetCacheForTests } from "./target-resolution.js";
 
 const mockState = vi.hoisted(() => ({
   loadConfig: vi.fn(() => ({})),
@@ -14,6 +15,7 @@ const mockState = vi.hoisted(() => ({
   createMattermostPost: vi.fn(),
   fetchMattermostChannelByName: vi.fn(),
   fetchMattermostMe: vi.fn(),
+  fetchMattermostUser: vi.fn(),
   fetchMattermostUserTeams: vi.fn(),
   fetchMattermostUserByUsername: vi.fn(),
   normalizeMattermostBaseUrl: vi.fn((input: string | undefined) => input?.trim() ?? ""),
@@ -34,6 +36,7 @@ vi.mock("./client.js", () => ({
   createMattermostPost: mockState.createMattermostPost,
   fetchMattermostChannelByName: mockState.fetchMattermostChannelByName,
   fetchMattermostMe: mockState.fetchMattermostMe,
+  fetchMattermostUser: mockState.fetchMattermostUser,
   fetchMattermostUserTeams: mockState.fetchMattermostUserTeams,
   fetchMattermostUserByUsername: mockState.fetchMattermostUserByUsername,
   normalizeMattermostBaseUrl: mockState.normalizeMattermostBaseUrl,
@@ -77,9 +80,11 @@ describe("sendMessageMattermost", () => {
     mockState.createMattermostPost.mockReset();
     mockState.fetchMattermostChannelByName.mockReset();
     mockState.fetchMattermostMe.mockReset();
+    mockState.fetchMattermostUser.mockReset();
     mockState.fetchMattermostUserTeams.mockReset();
     mockState.fetchMattermostUserByUsername.mockReset();
     mockState.uploadMattermostFile.mockReset();
+    resetMattermostOpaqueTargetCacheForTests();
     mockState.createMattermostClient.mockReturnValue({});
     mockState.createMattermostPost.mockResolvedValue({ id: "post-1" });
     mockState.fetchMattermostMe.mockResolvedValue({ id: "bot-user" });
@@ -182,6 +187,61 @@ describe("sendMessageMattermost", () => {
       }),
     );
   });
+
+  it("resolves a bare Mattermost user id as a DM target before upload", async () => {
+    const userId = "dthcxgoxhifn3pwh65cut3ud3w";
+    mockState.fetchMattermostUser.mockResolvedValueOnce({ id: userId });
+    mockState.createMattermostDirectChannel.mockResolvedValueOnce({ id: "dm-channel-1" });
+    mockState.loadOutboundMediaFromUrl.mockResolvedValueOnce({
+      buffer: Buffer.from("media-bytes"),
+      fileName: "photo.png",
+      contentType: "image/png",
+      kind: "image",
+    });
+
+    const result = await sendMessageMattermost(userId, "hello", {
+      mediaUrl: "file:///tmp/agent-workspace/photo.png",
+      mediaLocalRoots: ["/tmp/agent-workspace"],
+    });
+
+    expect(mockState.fetchMattermostUser).toHaveBeenCalledWith({}, userId);
+    expect(mockState.createMattermostDirectChannel).toHaveBeenCalledWith({}, ["bot-user", userId]);
+    expect(mockState.uploadMattermostFile).toHaveBeenCalledWith(
+      {},
+      expect.objectContaining({
+        channelId: "dm-channel-1",
+      }),
+    );
+    expect(result.channelId).toBe("dm-channel-1");
+  });
+
+  it("falls back to a channel target when bare Mattermost id is not a user", async () => {
+    const channelId = "aaaaaaaaaaaaaaaaaaaaaaaaaa";
+    mockState.fetchMattermostUser.mockRejectedValueOnce(
+      new Error("Mattermost API 404 Not Found: user not found"),
+    );
+    mockState.loadOutboundMediaFromUrl.mockResolvedValueOnce({
+      buffer: Buffer.from("media-bytes"),
+      fileName: "photo.png",
+      contentType: "image/png",
+      kind: "image",
+    });
+
+    const result = await sendMessageMattermost(channelId, "hello", {
+      mediaUrl: "file:///tmp/agent-workspace/photo.png",
+      mediaLocalRoots: ["/tmp/agent-workspace"],
+    });
+
+    expect(mockState.fetchMattermostUser).toHaveBeenCalledWith({}, channelId);
+    expect(mockState.createMattermostDirectChannel).not.toHaveBeenCalled();
+    expect(mockState.uploadMattermostFile).toHaveBeenCalledWith(
+      {},
+      expect.objectContaining({
+        channelId,
+      }),
+    );
+    expect(result.channelId).toBe(channelId);
+  });
 });
 
 describe("parseMattermostTarget", () => {
@@ -266,3 +326,110 @@ describe("parseMattermostTarget", () => {
     expect(parseMattermostTarget("Mattermost:QRS")).toEqual({ kind: "user", id: "QRS" });
   });
 });
+
+// Each test uses a unique (token, id) pair to avoid module-level cache collisions.
+// userIdResolutionCache and dmChannelCache are module singletons that survive across tests.
+// Using unique cache keys per test ensures full isolation without needing a cache reset API.
+describe("sendMessageMattermost user-first resolution", () => {
+  function makeAccount(token: string) {
+    return {
+      accountId: "default",
+      botToken: token,
+      baseUrl: "https://mattermost.example.com",
+    };
+  }
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockState.createMattermostClient.mockReturnValue({});
+    mockState.createMattermostPost.mockResolvedValue({ id: "post-id" });
+    mockState.createMattermostDirectChannel.mockResolvedValue({ id: "dm-channel-id" });
+    mockState.fetchMattermostMe.mockResolvedValue({ id: "bot-id" });
+  });
+
+  it("resolves unprefixed 26-char id as user and sends via DM channel", async () => {
+    // Unique token + id to avoid cache pollution from other tests
+    const userId = "aaaaaa1111111111aaaaaa1111"; // 26 chars
+    mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-user-dm-t1"));
+    mockState.fetchMattermostUser.mockResolvedValueOnce({ id: userId });
+
+    const res = await sendMessageMattermost(userId, "hello");
+
+    expect(mockState.fetchMattermostUser).toHaveBeenCalledTimes(1);
+    expect(mockState.createMattermostDirectChannel).toHaveBeenCalledTimes(1);
+    const params = mockState.createMattermostPost.mock.calls[0]?.[1];
+    expect(params.channelId).toBe("dm-channel-id");
+    expect(res.channelId).toBe("dm-channel-id");
+    expect(res.messageId).toBe("post-id");
+  });
+
+  it("falls back to channel id when user lookup returns 404", async () => {
+    // Unique token + id for this test
+    const channelId = "bbbbbb2222222222bbbbbb2222"; // 26 chars
+    mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-404-t2"));
+    const err = new Error("Mattermost API 404: user not found");
+    mockState.fetchMattermostUser.mockRejectedValueOnce(err);
+
+    const res = await sendMessageMattermost(channelId, "hello");
+
+    expect(mockState.fetchMattermostUser).toHaveBeenCalledTimes(1);
+    expect(mockState.createMattermostDirectChannel).not.toHaveBeenCalled();
+    const params = mockState.createMattermostPost.mock.calls[0]?.[1];
+    expect(params.channelId).toBe(channelId);
+    expect(res.channelId).toBe(channelId);
+  });
+
+  it("falls back to channel id without caching negative result on transient error", async () => {
+    // Two unique tokens so each call has its own cache namespace
+    const userId = "cccccc3333333333cccccc3333"; // 26 chars
+    const tokenA = "token-transient-t3a";
+    const tokenB = "token-transient-t3b";
+    const transientErr = new Error("Mattermost API 503: service unavailable");
+
+    // First call: transient error → fall back to channel id, do NOT cache negative
+    mockState.resolveMattermostAccount.mockReturnValue(makeAccount(tokenA));
+    mockState.fetchMattermostUser.mockRejectedValueOnce(transientErr);
+
+    const res1 = await sendMessageMattermost(userId, "first");
+    expect(res1.channelId).toBe(userId);
+
+    // Second call with a different token (new cache key) → retries user lookup
+    vi.clearAllMocks();
+    mockState.createMattermostClient.mockReturnValue({});
+    mockState.createMattermostPost.mockResolvedValue({ id: "post-id-2" });
+    mockState.createMattermostDirectChannel.mockResolvedValue({ id: "dm-channel-id" });
+    mockState.fetchMattermostMe.mockResolvedValue({ id: "bot-id" });
+    mockState.resolveMattermostAccount.mockReturnValue(makeAccount(tokenB));
+    mockState.fetchMattermostUser.mockResolvedValueOnce({ id: userId });
+
+    const res2 = await sendMessageMattermost(userId, "second");
+    expect(mockState.fetchMattermostUser).toHaveBeenCalledTimes(1);
+    expect(res2.channelId).toBe("dm-channel-id");
+  });
+
+  it("does not apply user-first resolution for explicit user: prefix", async () => {
+    // Unique token + id — explicit user: prefix bypasses probe, goes straight to DM
+    const userId = "dddddd4444444444dddddd4444"; // 26 chars
+    mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-explicit-user-t4"));
+
+    const res = await sendMessageMattermost(`user:${userId}`, "hello");
+
+    expect(mockState.fetchMattermostUser).not.toHaveBeenCalled();
+    expect(mockState.createMattermostDirectChannel).toHaveBeenCalledTimes(1);
+    expect(res.channelId).toBe("dm-channel-id");
+  });
+
+  it("does not apply user-first resolution for explicit channel: prefix", async () => {
+    // Unique token + id — explicit channel: prefix, no probe, no DM
+    const chanId = "eeeeee5555555555eeeeee5555"; // 26 chars
+    mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-explicit-chan-t5"));
+
+    const res = await sendMessageMattermost(`channel:${chanId}`, "hello");
+
+    expect(mockState.fetchMattermostUser).not.toHaveBeenCalled();
+    expect(mockState.createMattermostDirectChannel).not.toHaveBeenCalled();
+    const params = mockState.createMattermostPost.mock.calls[0]?.[1];
+    expect(params.channelId).toBe(chanId);
+    expect(res.channelId).toBe(chanId);
+  });
+});

extensions/mattermost/src/mattermost/send.ts

@@ -19,6 +19,7 @@ import {
   setInteractionSecret,
   type MattermostInteractiveButtonInput,
 } from "./interactions.js";
+import { isMattermostId, resolveMattermostOpaqueTarget } from "./target-resolution.js";
 
 export type MattermostSendOpts = {
   cfg?: OpenClawConfig;
@@ -50,6 +51,7 @@ type MattermostTarget =
 const botUserCache = new Map<string, MattermostUser>();
 const userByNameCache = new Map<string, MattermostUser>();
 const channelByNameCache = new Map<string, string>();
+const dmChannelCache = new Map<string, string>();
 
 const getCore = () => getMattermostRuntime();
 
@@ -66,12 +68,6 @@ function normalizeMessage(text: string, mediaUrl?: string): string {
 function isHttpUrl(value: string): boolean {
   return /^https?:\/\//i.test(value);
 }
-
-/** Mattermost IDs are 26-character lowercase alphanumeric strings. */
-function isMattermostId(value: string): boolean {
-  return /^[a-z0-9]{26}$/.test(value);
-}
-
 export function parseMattermostTarget(raw: string): MattermostTarget {
   const trimmed = raw.trim();
   if (!trimmed) {
@@ -208,12 +204,18 @@ async function resolveTargetChannelId(params: {
         token: params.token,
         username: params.target.username ?? "",
       });
+  const dmKey = `${cacheKey(params.baseUrl, params.token)}::dm::${userId}`;
+  const cachedDm = dmChannelCache.get(dmKey);
+  if (cachedDm) {
+    return cachedDm;
+  }
   const botUser = await resolveBotUser(params.baseUrl, params.token);
   const client = createMattermostClient({
     baseUrl: params.baseUrl,
     botToken: params.token,
   });
   const channel = await createMattermostDirectChannel(client, [botUser.id, userId]);
+  dmChannelCache.set(dmKey, channel.id);
   return channel.id;
 }
 
@@ -248,7 +250,18 @@ async function resolveMattermostSendContext(
     );
   }
 
-  const target = parseMattermostTarget(to);
+  const trimmedTo = to?.trim() ?? "";
+  const opaqueTarget = await resolveMattermostOpaqueTarget({
+    input: trimmedTo,
+    token,
+    baseUrl,
+  });
+  const target =
+    opaqueTarget?.kind === "user"
+      ? { kind: "user" as const, id: opaqueTarget.id }
+      : opaqueTarget?.kind === "channel"
+        ? { kind: "channel" as const, id: opaqueTarget.id }
+        : parseMattermostTarget(trimmedTo);
   const channelId = await resolveTargetChannelId({
     target,
     baseUrl,

extensions/mattermost/src/mattermost/target-resolution.ts

@@ -0,0 +1,97 @@
+import type { OpenClawConfig } from "openclaw/plugin-sdk/mattermost";
+import { resolveMattermostAccount } from "./accounts.js";
+import {
+  createMattermostClient,
+  fetchMattermostUser,
+  normalizeMattermostBaseUrl,
+} from "./client.js";
+
+export type MattermostOpaqueTargetResolution = {
+  kind: "user" | "channel";
+  id: string;
+  to: string;
+};
+
+const mattermostOpaqueTargetCache = new Map<string, boolean>();
+
+function cacheKey(baseUrl: string, token: string, id: string): string {
+  return `${baseUrl}::${token}::${id}`;
+}
+
+/** Mattermost IDs are 26-character lowercase alphanumeric strings. */
+export function isMattermostId(value: string): boolean {
+  return /^[a-z0-9]{26}$/.test(value);
+}
+
+export function isExplicitMattermostTarget(raw: string): boolean {
+  const trimmed = raw.trim();
+  if (!trimmed) {
+    return false;
+  }
+  return (
+    /^(channel|user|mattermost):/i.test(trimmed) ||
+    trimmed.startsWith("@") ||
+    trimmed.startsWith("#")
+  );
+}
+
+export function parseMattermostApiStatus(err: unknown): number | undefined {
+  if (!err || typeof err !== "object") {
+    return undefined;
+  }
+  const msg = "message" in err ? String((err as { message?: unknown }).message ?? "") : "";
+  const match = /Mattermost API (\d{3})\b/.exec(msg);
+  if (!match) {
+    return undefined;
+  }
+  const code = Number(match[1]);
+  return Number.isFinite(code) ? code : undefined;
+}
+
+export async function resolveMattermostOpaqueTarget(params: {
+  input: string;
+  cfg?: OpenClawConfig;
+  accountId?: string | null;
+  token?: string;
+  baseUrl?: string;
+}): Promise<MattermostOpaqueTargetResolution | null> {
+  const input = params.input.trim();
+  if (!input || isExplicitMattermostTarget(input) || !isMattermostId(input)) {
+    return null;
+  }
+
+  const account =
+    params.cfg && (!params.token || !params.baseUrl)
+      ? resolveMattermostAccount({ cfg: params.cfg, accountId: params.accountId })
+      : null;
+  const token = params.token?.trim() || account?.botToken?.trim();
+  const baseUrl = normalizeMattermostBaseUrl(params.baseUrl ?? account?.baseUrl);
+  if (!token || !baseUrl) {
+    return null;
+  }
+
+  const key = cacheKey(baseUrl, token, input);
+  const cached = mattermostOpaqueTargetCache.get(key);
+  if (cached === true) {
+    return { kind: "user", id: input, to: `user:${input}` };
+  }
+  if (cached === false) {
+    return { kind: "channel", id: input, to: `channel:${input}` };
+  }
+
+  const client = createMattermostClient({ baseUrl, botToken: token });
+  try {
+    await fetchMattermostUser(client, input);
+    mattermostOpaqueTargetCache.set(key, true);
+    return { kind: "user", id: input, to: `user:${input}` };
+  } catch (err) {
+    if (parseMattermostApiStatus(err) === 404) {
+      mattermostOpaqueTargetCache.set(key, false);
+    }
+    return { kind: "channel", id: input, to: `channel:${input}` };
+  }
+}
+
+export function resetMattermostOpaqueTargetCacheForTests(): void {
+  mattermostOpaqueTargetCache.clear();
+}