From e80c66a571625ac097779d7b228619056cce9fa7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 11:28:14 +0000 Subject: [PATCH] fix(mattermost): refine probe and onboarding flows --- extensions/mattermost/src/channel.test.ts | 72 +++++++------- .../mattermost/src/mattermost/client.ts | 2 +- .../mattermost/src/mattermost/probe.test.ts | 97 +++++++++++++++++++ extensions/mattermost/src/mattermost/probe.ts | 14 +-- extensions/mattermost/src/onboarding.ts | 64 ++++++------ 5 files changed, 163 insertions(+), 86 deletions(-) create mode 100644 extensions/mattermost/src/mattermost/probe.test.ts diff --git a/extensions/mattermost/src/channel.test.ts b/extensions/mattermost/src/channel.test.ts index cd60f4fe65a..9cb5df2b846 100644 --- a/extensions/mattermost/src/channel.test.ts +++ b/extensions/mattermost/src/channel.test.ts @@ -54,6 +54,25 @@ describe("mattermostPlugin", () => { resetMattermostReactionBotUserCacheForTests(); }); + const runReactAction = async (params: Record, fetchMode: "add" | "remove") => { + const cfg = createMattermostTestConfig(); + const fetchImpl = createMattermostReactionFetchMock({ + mode: fetchMode, + postId: "POST1", + emojiName: "thumbsup", + }); + + return await withMockedGlobalFetch(fetchImpl as unknown as typeof fetch, async () => { + return await mattermostPlugin.actions?.handleAction?.({ + channel: "mattermost", + action: "react", + params, + cfg, + accountId: "default", + } as any); + }); + }; + it("exposes react when mattermost is configured", () => { const cfg: OpenClawConfig = { channels: { @@ -152,51 +171,32 @@ describe("mattermostPlugin", () => { }); it("handles react by calling Mattermost reactions API", async () => { - const cfg = createMattermostTestConfig(); - const fetchImpl = createMattermostReactionFetchMock({ - mode: "add", - postId: "POST1", - emojiName: "thumbsup", - }); - - const result = await withMockedGlobalFetch(fetchImpl as unknown as typeof fetch, async () => { - const result = await mattermostPlugin.actions?.handleAction?.({ - channel: "mattermost", - action: "react", - params: { messageId: "POST1", emoji: "thumbsup" }, - cfg, - accountId: "default", - } as any); - - return result; - }); + const result = await runReactAction({ messageId: "POST1", emoji: "thumbsup" }, "add"); 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 = createMattermostTestConfig(); - const fetchImpl = createMattermostReactionFetchMock({ - mode: "add", - postId: "POST1", - emojiName: "thumbsup", - }); - - const result = await withMockedGlobalFetch(fetchImpl as unknown as typeof fetch, async () => { - const result = await mattermostPlugin.actions?.handleAction?.({ - channel: "mattermost", - action: "react", - params: { messageId: "POST1", emoji: "thumbsup", remove: "true" }, - cfg, - accountId: "default", - } as any); - - return result; - }); + const result = await runReactAction( + { messageId: "POST1", emoji: "thumbsup", remove: "true" }, + "add", + ); expect(result?.content).toEqual([{ type: "text", text: "Reacted with :thumbsup: on POST1" }]); }); + + it("removes reaction when remove flag is boolean true", async () => { + const result = await runReactAction( + { messageId: "POST1", emoji: "thumbsup", remove: true }, + "remove", + ); + + expect(result?.content).toEqual([ + { type: "text", text: "Removed reaction :thumbsup: from POST1" }, + ]); + expect(result?.details).toEqual({}); + }); }); describe("config", () => { diff --git a/extensions/mattermost/src/mattermost/client.ts b/extensions/mattermost/src/mattermost/client.ts index f0a0fd26adc..826212c9eb8 100644 --- a/extensions/mattermost/src/mattermost/client.ts +++ b/extensions/mattermost/src/mattermost/client.ts @@ -58,7 +58,7 @@ function buildMattermostApiUrl(baseUrl: string, path: string): string { return `${normalized}/api/v4${suffix}`; } -async function readMattermostError(res: Response): Promise { +export async function readMattermostError(res: Response): Promise { const contentType = res.headers.get("content-type") ?? ""; if (contentType.includes("application/json")) { const data = (await res.json()) as { message?: string } | undefined; diff --git a/extensions/mattermost/src/mattermost/probe.test.ts b/extensions/mattermost/src/mattermost/probe.test.ts new file mode 100644 index 00000000000..887ac576a85 --- /dev/null +++ b/extensions/mattermost/src/mattermost/probe.test.ts @@ -0,0 +1,97 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { probeMattermost } from "./probe.js"; + +const mockFetch = vi.fn(); + +describe("probeMattermost", () => { + beforeEach(() => { + vi.stubGlobal("fetch", mockFetch); + mockFetch.mockReset(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("returns baseUrl missing for empty base URL", async () => { + await expect(probeMattermost(" ", "token")).resolves.toEqual({ + ok: false, + error: "baseUrl missing", + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("normalizes base URL and returns bot info", async () => { + mockFetch.mockResolvedValueOnce( + new Response(JSON.stringify({ id: "bot-1", username: "clawbot" }), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + + const result = await probeMattermost("https://mm.example.com/api/v4/", "bot-token"); + + expect(mockFetch).toHaveBeenCalledWith( + "https://mm.example.com/api/v4/users/me", + expect.objectContaining({ + headers: { Authorization: "Bearer bot-token" }, + }), + ); + expect(result).toEqual( + expect.objectContaining({ + ok: true, + status: 200, + bot: { id: "bot-1", username: "clawbot" }, + }), + ); + expect(result.elapsedMs).toBeGreaterThanOrEqual(0); + }); + + it("returns API error details from JSON response", async () => { + mockFetch.mockResolvedValueOnce( + new Response(JSON.stringify({ message: "invalid auth token" }), { + status: 401, + statusText: "Unauthorized", + headers: { "content-type": "application/json" }, + }), + ); + + await expect(probeMattermost("https://mm.example.com", "bad-token")).resolves.toEqual( + expect.objectContaining({ + ok: false, + status: 401, + error: "invalid auth token", + }), + ); + }); + + it("falls back to statusText when error body is empty", async () => { + mockFetch.mockResolvedValueOnce( + new Response("", { + status: 403, + statusText: "Forbidden", + headers: { "content-type": "text/plain" }, + }), + ); + + await expect(probeMattermost("https://mm.example.com", "token")).resolves.toEqual( + expect.objectContaining({ + ok: false, + status: 403, + error: "Forbidden", + }), + ); + }); + + it("returns fetch error when request throws", async () => { + mockFetch.mockRejectedValueOnce(new Error("network down")); + + await expect(probeMattermost("https://mm.example.com", "token")).resolves.toEqual( + expect.objectContaining({ + ok: false, + status: null, + error: "network down", + }), + ); + }); +}); diff --git a/extensions/mattermost/src/mattermost/probe.ts b/extensions/mattermost/src/mattermost/probe.ts index cb468ec14db..eda98b21c0e 100644 --- a/extensions/mattermost/src/mattermost/probe.ts +++ b/extensions/mattermost/src/mattermost/probe.ts @@ -1,5 +1,5 @@ import type { BaseProbeResult } from "openclaw/plugin-sdk"; -import { normalizeMattermostBaseUrl, type MattermostUser } from "./client.js"; +import { normalizeMattermostBaseUrl, readMattermostError, type MattermostUser } from "./client.js"; export type MattermostProbe = BaseProbeResult & { status?: number | null; @@ -7,18 +7,6 @@ export type MattermostProbe = BaseProbeResult & { bot?: MattermostUser; }; -async function readMattermostError(res: Response): Promise { - const contentType = res.headers.get("content-type") ?? ""; - if (contentType.includes("application/json")) { - const data = (await res.json()) as { message?: string } | undefined; - if (data?.message) { - return data.message; - } - return JSON.stringify(data); - } - return await res.text(); -} - export async function probeMattermost( baseUrl: string, botToken: string, diff --git a/extensions/mattermost/src/onboarding.ts b/extensions/mattermost/src/onboarding.ts index 9f90f1f2ab8..358d3f43f7f 100644 --- a/extensions/mattermost/src/onboarding.ts +++ b/extensions/mattermost/src/onboarding.ts @@ -22,6 +22,25 @@ async function noteMattermostSetup(prompter: WizardPrompter): Promise { ); } +async function promptMattermostCredentials(prompter: WizardPrompter): Promise<{ + botToken: string; + baseUrl: string; +}> { + const botToken = String( + await prompter.text({ + message: "Enter Mattermost bot token", + validate: (value) => (value?.trim() ? undefined : "Required"), + }), + ).trim(); + const baseUrl = String( + await prompter.text({ + message: "Enter Mattermost base URL", + validate: (value) => (value?.trim() ? undefined : "Required"), + }), + ).trim(); + return { botToken, baseUrl }; +} + export const mattermostOnboardingAdapter: ChannelOnboardingAdapter = { channel, getStatus: async ({ cfg }) => { @@ -90,18 +109,9 @@ export const mattermostOnboardingAdapter: ChannelOnboardingAdapter = { }, }; } else { - botToken = String( - await prompter.text({ - message: "Enter Mattermost bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - baseUrl = String( - await prompter.text({ - message: "Enter Mattermost base URL", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); + const entered = await promptMattermostCredentials(prompter); + botToken = entered.botToken; + baseUrl = entered.baseUrl; } } else if (accountConfigured) { const keep = await prompter.confirm({ @@ -109,32 +119,14 @@ export const mattermostOnboardingAdapter: ChannelOnboardingAdapter = { initialValue: true, }); if (!keep) { - botToken = String( - await prompter.text({ - message: "Enter Mattermost bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - baseUrl = String( - await prompter.text({ - message: "Enter Mattermost base URL", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); + const entered = await promptMattermostCredentials(prompter); + botToken = entered.botToken; + baseUrl = entered.baseUrl; } } else { - botToken = String( - await prompter.text({ - message: "Enter Mattermost bot token", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); - baseUrl = String( - await prompter.text({ - message: "Enter Mattermost base URL", - validate: (value) => (value?.trim() ? undefined : "Required"), - }), - ).trim(); + const entered = await promptMattermostCredentials(prompter); + botToken = entered.botToken; + baseUrl = entered.baseUrl; } if (botToken || baseUrl) {