refactor(mattermost): dedupe reaction flow and test fixtures

extensions/mattermost/src/channel.test.ts

@@ -1,7 +1,13 @@
 import type { OpenClawConfig } from "openclaw/plugin-sdk";
 import { createReplyPrefixOptions } from "openclaw/plugin-sdk";
-import { describe, expect, it, vi } from "vitest";
+import { beforeEach, describe, expect, it } from "vitest";
 import { mattermostPlugin } from "./channel.js";
+import { resetMattermostReactionBotUserCacheForTests } from "./mattermost/reactions.js";
+import {
+  createMattermostReactionFetchMock,
+  createMattermostTestConfig,
+  withMockedGlobalFetch,
+} from "./mattermost/reactions.test-helpers.js";
 
 describe("mattermostPlugin", () => {
   describe("messaging", () => {
@@ -44,6 +50,10 @@ describe("mattermostPlugin", () => {
   });
 
   describe("messageActions", () => {
+    beforeEach(() => {
+      resetMattermostReactionBotUserCacheForTests();
+    });
+
     it("exposes react when mattermost is configured", () => {
       const cfg: OpenClawConfig = {
         channels: {
@@ -142,41 +152,14 @@ describe("mattermostPlugin", () => {
     });
 
     it("handles react by calling Mattermost reactions API", async () => {
-      const cfg: OpenClawConfig = {
-        channels: {
-          mattermost: {
-            enabled: true,
-            botToken: "test-token",
-            baseUrl: "https://chat.example.com",
-          },
-        },
-      };
-
-      const fetchImpl = vi.fn(async (url: any, init?: any) => {
-        if (String(url).endsWith("/api/v4/users/me")) {
-          return new Response(JSON.stringify({ id: "BOT123" }), {
-            status: 200,
-            headers: { "content-type": "application/json" },
-          });
-        }
-        if (String(url).endsWith("/api/v4/reactions")) {
-          expect(init?.method).toBe("POST");
-          expect(JSON.parse(init?.body)).toEqual({
-            user_id: "BOT123",
-            post_id: "POST1",
-            emoji_name: "thumbsup",
-          });
-          return new Response(JSON.stringify({ ok: true }), {
-            status: 201,
-            headers: { "content-type": "application/json" },
-          });
-        }
-        throw new Error(`unexpected url: ${url}`);
+      const cfg = createMattermostTestConfig();
+      const fetchImpl = createMattermostReactionFetchMock({
+        mode: "add",
+        postId: "POST1",
+        emojiName: "thumbsup",
       });
 
-      const prevFetch = globalThis.fetch;
-      (globalThis as any).fetch = fetchImpl;
-      try {
+      const result = await withMockedGlobalFetch(fetchImpl as unknown as typeof fetch, async () => {
         const result = await mattermostPlugin.actions?.handleAction?.({
           channel: "mattermost",
           action: "react",
@@ -185,51 +168,22 @@ describe("mattermostPlugin", () => {
           accountId: "default",
         } as any);
 
-        expect(result?.content).toEqual([
-          { type: "text", text: "Reacted with :thumbsup: on POST1" },
-        ]);
-        expect(result?.details).toEqual({});
-      } finally {
-        (globalThis as any).fetch = prevFetch;
-      }
+        return result;
+      });
+
+      expect(result?.content).toEqual([{ type: "text", text: "Reacted with :thumbsup: on POST1" }]);
+      expect(result?.details).toEqual({});
     });
 
     it("only treats boolean remove flag as removal", async () => {
-      const cfg: OpenClawConfig = {
-        channels: {
-          mattermost: {
-            enabled: true,
-            botToken: "test-token",
-            baseUrl: "https://chat.example.com",
-          },
-        },
-      };
-
-      const fetchImpl = vi.fn(async (url: any, init?: any) => {
-        if (String(url).endsWith("/api/v4/users/me")) {
-          return new Response(JSON.stringify({ id: "BOT123" }), {
-            status: 200,
-            headers: { "content-type": "application/json" },
-          });
-        }
-        if (String(url).endsWith("/api/v4/reactions")) {
-          expect(init?.method).toBe("POST");
-          expect(JSON.parse(init?.body)).toEqual({
-            user_id: "BOT123",
-            post_id: "POST1",
-            emoji_name: "thumbsup",
-          });
-          return new Response(JSON.stringify({ ok: true }), {
-            status: 201,
-            headers: { "content-type": "application/json" },
-          });
-        }
-        throw new Error(`unexpected url: ${url}`);
+      const cfg = createMattermostTestConfig();
+      const fetchImpl = createMattermostReactionFetchMock({
+        mode: "add",
+        postId: "POST1",
+        emojiName: "thumbsup",
       });
 
-      const prevFetch = globalThis.fetch;
-      (globalThis as any).fetch = fetchImpl;
-      try {
+      const result = await withMockedGlobalFetch(fetchImpl as unknown as typeof fetch, async () => {
         const result = await mattermostPlugin.actions?.handleAction?.({
           channel: "mattermost",
           action: "react",
@@ -238,12 +192,10 @@ describe("mattermostPlugin", () => {
           accountId: "default",
         } as any);
 
-        expect(result?.content).toEqual([
-          { type: "text", text: "Reacted with :thumbsup: on POST1" },
-        ]);
-      } finally {
-        (globalThis as any).fetch = prevFetch;
-      }
+        return result;
+      });
+
+      expect(result?.content).toEqual([{ type: "text", text: "Reacted with :thumbsup: on POST1" }]);
     });
   });
 

extensions/mattermost/src/mattermost/reactions.test-helpers.ts

@@ -0,0 +1,83 @@
+import type { OpenClawConfig } from "openclaw/plugin-sdk";
+import { expect, vi } from "vitest";
+
+export function createMattermostTestConfig(): OpenClawConfig {
+  return {
+    channels: {
+      mattermost: {
+        enabled: true,
+        botToken: "test-token",
+        baseUrl: "https://chat.example.com",
+      },
+    },
+  };
+}
+
+export function createMattermostReactionFetchMock(params: {
+  postId: string;
+  emojiName: string;
+  mode: "add" | "remove" | "both";
+  userId?: string;
+  status?: number;
+  body?: unknown;
+}) {
+  const userId = params.userId ?? "BOT123";
+  const mode = params.mode;
+  const allowAdd = mode === "add" || mode === "both";
+  const allowRemove = mode === "remove" || mode === "both";
+  const addStatus = params.status ?? 201;
+  const removeStatus = params.status ?? 204;
+  const removePath = `/api/v4/users/${userId}/posts/${params.postId}/reactions/${encodeURIComponent(params.emojiName)}`;
+
+  return vi.fn(async (url: any, init?: any) => {
+    if (String(url).endsWith("/api/v4/users/me")) {
+      return new Response(JSON.stringify({ id: userId }), {
+        status: 200,
+        headers: { "content-type": "application/json" },
+      });
+    }
+
+    if (allowAdd && String(url).endsWith("/api/v4/reactions")) {
+      expect(init?.method).toBe("POST");
+      expect(JSON.parse(init?.body)).toEqual({
+        user_id: userId,
+        post_id: params.postId,
+        emoji_name: params.emojiName,
+      });
+
+      const responseBody = params.body === undefined ? { ok: true } : params.body;
+      return new Response(
+        responseBody === null ? null : JSON.stringify(responseBody),
+        responseBody === null
+          ? { status: addStatus, headers: { "content-type": "text/plain" } }
+          : { status: addStatus, headers: { "content-type": "application/json" } },
+      );
+    }
+
+    if (allowRemove && String(url).endsWith(removePath)) {
+      expect(init?.method).toBe("DELETE");
+      const responseBody = params.body === undefined ? null : params.body;
+      return new Response(
+        responseBody === null ? null : JSON.stringify(responseBody),
+        responseBody === null
+          ? { status: removeStatus, headers: { "content-type": "text/plain" } }
+          : { status: removeStatus, headers: { "content-type": "application/json" } },
+      );
+    }
+
+    throw new Error(`unexpected url: ${url}`);
+  });
+}
+
+export async function withMockedGlobalFetch<T>(
+  fetchImpl: typeof fetch,
+  run: () => Promise<T>,
+): Promise<T> {
+  const prevFetch = globalThis.fetch;
+  (globalThis as any).fetch = fetchImpl;
+  try {
+    return await run();
+  } finally {
+    (globalThis as any).fetch = prevFetch;
+  }
+}

extensions/mattermost/src/mattermost/reactions.test.ts

@@ -1,45 +1,28 @@
-import type { OpenClawConfig } from "openclaw/plugin-sdk";
-import { describe, expect, it, vi } from "vitest";
-import { addMattermostReaction, removeMattermostReaction } from "./reactions.js";
-
-function createCfg(): OpenClawConfig {
-  return {
-    channels: {
-      mattermost: {
-        enabled: true,
-        botToken: "test-token",
-        baseUrl: "https://chat.example.com",
-      },
-    },
-  };
-}
+import { beforeEach, describe, expect, it } from "vitest";
+import {
+  addMattermostReaction,
+  removeMattermostReaction,
+  resetMattermostReactionBotUserCacheForTests,
+} from "./reactions.js";
+import {
+  createMattermostReactionFetchMock,
+  createMattermostTestConfig,
+} from "./reactions.test-helpers.js";
 
 describe("mattermost reactions", () => {
+  beforeEach(() => {
+    resetMattermostReactionBotUserCacheForTests();
+  });
+
   it("adds reactions by calling /users/me then POST /reactions", async () => {
-    const fetchMock = vi.fn(async (url: any, init?: any) => {
-      if (String(url).endsWith("/api/v4/users/me")) {
-        return new Response(JSON.stringify({ id: "BOT123" }), {
-          status: 200,
-          headers: { "content-type": "application/json" },
-        });
-      }
-      if (String(url).endsWith("/api/v4/reactions")) {
-        expect(init?.method).toBe("POST");
-        expect(JSON.parse(init?.body)).toEqual({
-          user_id: "BOT123",
-          post_id: "POST1",
-          emoji_name: "thumbsup",
-        });
-        return new Response(JSON.stringify({ ok: true }), {
-          status: 201,
-          headers: { "content-type": "application/json" },
-        });
-      }
-      throw new Error(`unexpected url: ${url}`);
+    const fetchMock = createMattermostReactionFetchMock({
+      mode: "add",
+      postId: "POST1",
+      emojiName: "thumbsup",
     });
 
     const result = await addMattermostReaction({
-      cfg: createCfg(),
+      cfg: createMattermostTestConfig(),
       postId: "POST1",
       emojiName: "thumbsup",
       fetchImpl: fetchMock as unknown as typeof fetch,
@@ -50,24 +33,16 @@ describe("mattermost reactions", () => {
   });
 
   it("returns a Result error when add reaction API call fails", async () => {
-    const fetchMock = vi.fn(async (url: any) => {
-      if (String(url).endsWith("/api/v4/users/me")) {
-        return new Response(JSON.stringify({ id: "BOT123" }), {
-          status: 200,
-          headers: { "content-type": "application/json" },
-        });
-      }
-      if (String(url).endsWith("/api/v4/reactions")) {
-        return new Response(JSON.stringify({ id: "err", message: "boom" }), {
-          status: 500,
-          headers: { "content-type": "application/json" },
-        });
-      }
-      throw new Error(`unexpected url: ${url}`);
+    const fetchMock = createMattermostReactionFetchMock({
+      mode: "add",
+      postId: "POST1",
+      emojiName: "thumbsup",
+      status: 500,
+      body: { id: "err", message: "boom" },
     });
 
     const result = await addMattermostReaction({
-      cfg: createCfg(),
+      cfg: createMattermostTestConfig(),
       postId: "POST1",
       emojiName: "thumbsup",
       fetchImpl: fetchMock as unknown as typeof fetch,
@@ -80,25 +55,14 @@ describe("mattermost reactions", () => {
   });
 
   it("removes reactions by calling /users/me then DELETE /users/:id/posts/:postId/reactions/:emoji", async () => {
-    const fetchMock = vi.fn(async (url: any, init?: any) => {
-      if (String(url).endsWith("/api/v4/users/me")) {
-        return new Response(JSON.stringify({ id: "BOT123" }), {
-          status: 200,
-          headers: { "content-type": "application/json" },
-        });
-      }
-      if (String(url).endsWith("/api/v4/users/BOT123/posts/POST1/reactions/thumbsup")) {
-        expect(init?.method).toBe("DELETE");
-        return new Response(null, {
-          status: 204,
-          headers: { "content-type": "text/plain" },
-        });
-      }
-      throw new Error(`unexpected url: ${url}`);
+    const fetchMock = createMattermostReactionFetchMock({
+      mode: "remove",
+      postId: "POST1",
+      emojiName: "thumbsup",
     });
 
     const result = await removeMattermostReaction({
-      cfg: createCfg(),
+      cfg: createMattermostTestConfig(),
       postId: "POST1",
       emojiName: "thumbsup",
       fetchImpl: fetchMock as unknown as typeof fetch,
@@ -107,4 +71,33 @@ describe("mattermost reactions", () => {
     expect(result).toEqual({ ok: true });
     expect(fetchMock).toHaveBeenCalled();
   });
+
+  it("caches the bot user id across reaction mutations", async () => {
+    const fetchMock = createMattermostReactionFetchMock({
+      mode: "both",
+      postId: "POST1",
+      emojiName: "thumbsup",
+    });
+
+    const cfg = createMattermostTestConfig();
+    const addResult = await addMattermostReaction({
+      cfg,
+      postId: "POST1",
+      emojiName: "thumbsup",
+      fetchImpl: fetchMock as unknown as typeof fetch,
+    });
+    const removeResult = await removeMattermostReaction({
+      cfg,
+      postId: "POST1",
+      emojiName: "thumbsup",
+      fetchImpl: fetchMock as unknown as typeof fetch,
+    });
+
+    const usersMeCalls = fetchMock.mock.calls.filter((call) =>
+      String(call[0]).endsWith("/api/v4/users/me"),
+    );
+    expect(addResult).toEqual({ ok: true });
+    expect(removeResult).toEqual({ ok: true });
+    expect(usersMeCalls).toHaveLength(1);
+  });
 });

extensions/mattermost/src/mattermost/reactions.ts

@@ -3,6 +3,15 @@ import { resolveMattermostAccount } from "./accounts.js";
 import { createMattermostClient, fetchMattermostMe, type MattermostClient } from "./client.js";
 
 type Result = { ok: true } | { ok: false; error: string };
+type ReactionParams = {
+  cfg: OpenClawConfig;
+  postId: string;
+  emojiName: string;
+  accountId?: string | null;
+  fetchImpl?: typeof fetch;
+};
+type ReactionMutation = (client: MattermostClient, params: MutationPayload) => Promise<void>;
+type MutationPayload = { userId: string; postId: string; emojiName: string };
 
 const BOT_USER_CACHE_TTL_MS = 10 * 60_000;
 const botUserIdCache = new Map<string, { userId: string; expiresAt: number }>();
@@ -31,36 +40,10 @@ export async function addMattermostReaction(params: {
   accountId?: string | null;
   fetchImpl?: typeof fetch;
 }): Promise<Result> {
-  const resolved = resolveMattermostAccount({ cfg: params.cfg, accountId: params.accountId });
-  const baseUrl = resolved.baseUrl?.trim();
-  const botToken = resolved.botToken?.trim();
-  if (!baseUrl || !botToken) {
-    return { ok: false, error: "Mattermost botToken/baseUrl missing." };
-  }
-
-  const client = createMattermostClient({
-    baseUrl,
-    botToken,
-    fetchImpl: params.fetchImpl,
+  return runMattermostReaction(params, {
+    action: "add",
+    mutation: createReaction,
   });
-
-  const cacheKey = `${baseUrl}:${botToken}`;
-  const userId = await resolveBotUserId(client, cacheKey);
-  if (!userId) {
-    return { ok: false, error: "Mattermost reactions failed: could not resolve bot user id." };
-  }
-
-  try {
-    await createReaction(client, {
-      userId,
-      postId: params.postId,
-      emojiName: params.emojiName,
-    });
-  } catch (err) {
-    return { ok: false, error: `Mattermost add reaction failed: ${String(err)}` };
-  }
-
-  return { ok: true };
 }
 
 export async function removeMattermostReaction(params: {
@@ -70,6 +53,23 @@ export async function removeMattermostReaction(params: {
   accountId?: string | null;
   fetchImpl?: typeof fetch;
 }): Promise<Result> {
+  return runMattermostReaction(params, {
+    action: "remove",
+    mutation: deleteReaction,
+  });
+}
+
+export function resetMattermostReactionBotUserCacheForTests(): void {
+  botUserIdCache.clear();
+}
+
+async function runMattermostReaction(
+  params: ReactionParams,
+  options: {
+    action: "add" | "remove";
+    mutation: ReactionMutation;
+  },
+): Promise<Result> {
   const resolved = resolveMattermostAccount({ cfg: params.cfg, accountId: params.accountId });
   const baseUrl = resolved.baseUrl?.trim();
   const botToken = resolved.botToken?.trim();
@@ -90,22 +90,19 @@ export async function removeMattermostReaction(params: {
   }
 
   try {
-    await deleteReaction(client, {
+    await options.mutation(client, {
       userId,
       postId: params.postId,
       emojiName: params.emojiName,
     });
   } catch (err) {
-    return { ok: false, error: `Mattermost remove reaction failed: ${String(err)}` };
+    return { ok: false, error: `Mattermost ${options.action} reaction failed: ${String(err)}` };
   }
 
   return { ok: true };
 }
 
-async function createReaction(
-  client: MattermostClient,
-  params: { userId: string; postId: string; emojiName: string },
-): Promise<void> {
+async function createReaction(client: MattermostClient, params: MutationPayload): Promise<void> {
   await client.request<Record<string, unknown>>("/reactions", {
     method: "POST",
     body: JSON.stringify({
@@ -116,10 +113,7 @@ async function createReaction(
   });
 }
 
-async function deleteReaction(
-  client: MattermostClient,
-  params: { userId: string; postId: string; emojiName: string },
-): Promise<void> {
+async function deleteReaction(client: MattermostClient, params: MutationPayload): Promise<void> {
   const emoji = encodeURIComponent(params.emojiName);
   await client.request<unknown>(
     `/users/${params.userId}/posts/${params.postId}/reactions/${emoji}`,