From 6a425d189e000d23346a3cdbaf71f522371a2cf2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 21:31:26 +0000 Subject: [PATCH] refactor(channels): dedupe slack telegram and web monitor tests --- .../monitor/message-handler/prepare.test.ts | 67 +----------- src/telegram/accounts.test.ts | 44 +++----- src/telegram/bot-native-command-menu.test.ts | 102 +++++++++--------- src/telegram/fetch.test.ts | 23 ++-- src/telegram/monitor.test.ts | 61 ++++------- src/web/auto-reply/deliver-reply.test.ts | 42 +++----- 6 files changed, 122 insertions(+), 217 deletions(-) diff --git a/src/slack/monitor/message-handler/prepare.test.ts b/src/slack/monitor/message-handler/prepare.test.ts index 4092750b97b..64f59b2e2dd 100644 --- a/src/slack/monitor/message-handler/prepare.test.ts +++ b/src/slack/monitor/message-handler/prepare.test.ts @@ -7,12 +7,14 @@ import { expectInboundContextContract } from "../../../../test/helpers/inbound-c import type { OpenClawConfig } from "../../../config/config.js"; import { resolveAgentRoute } from "../../../routing/resolve-route.js"; import { resolveThreadSessionKeys } from "../../../routing/session-key.js"; -import type { RuntimeEnv } from "../../../runtime.js"; import type { ResolvedSlackAccount } from "../../accounts.js"; import type { SlackMessageEvent } from "../../types.js"; import type { SlackMonitorContext } from "../context.js"; -import { createSlackMonitorContext } from "../context.js"; import { prepareSlackMessage } from "./prepare.js"; +import { + createInboundSlackTestContext as createInboundSlackCtx, + createSlackTestAccount as createSlackAccount, +} from "./prepare.test-helpers.js"; describe("slack prepareSlackMessage inbound contract", () => { let fixtureRoot = ""; @@ -38,53 +40,6 @@ describe("slack prepareSlackMessage inbound contract", () => { } }); - function createInboundSlackCtx(params: { - cfg: OpenClawConfig; - appClient?: App["client"]; - defaultRequireMention?: boolean; - replyToMode?: "off" | "all"; - channelsConfig?: Record; - }) { - return createSlackMonitorContext({ - cfg: params.cfg, - accountId: "default", - botToken: "token", - app: { client: params.appClient ?? {} } as App, - runtime: {} as RuntimeEnv, - botUserId: "B1", - teamId: "T1", - apiAppId: "A1", - historyLimit: 0, - sessionScope: "per-sender", - mainKey: "main", - dmEnabled: true, - dmPolicy: "open", - allowFrom: [], - allowNameMatching: false, - groupDmEnabled: true, - groupDmChannels: [], - defaultRequireMention: params.defaultRequireMention ?? true, - channelsConfig: params.channelsConfig, - groupPolicy: "open", - useAccessGroups: false, - reactionMode: "off", - reactionAllowlist: [], - replyToMode: params.replyToMode ?? "off", - threadHistoryScope: "thread", - threadInheritParent: false, - slashCommand: { - enabled: false, - name: "openclaw", - sessionPrefix: "slack:slash", - ephemeral: true, - }, - textLimit: 4000, - ackReactionScope: "group-mentions", - mediaMaxBytes: 1024, - removeAckAfterReply: false, - }); - } - function createDefaultSlackCtx() { const slackCtx = createInboundSlackCtx({ cfg: { @@ -133,20 +88,6 @@ describe("slack prepareSlackMessage inbound contract", () => { }); } - function createSlackAccount(config: ResolvedSlackAccount["config"] = {}): ResolvedSlackAccount { - return { - accountId: "default", - enabled: true, - botTokenSource: "config", - appTokenSource: "config", - userTokenSource: "none", - config, - replyToMode: config.replyToMode, - replyToModeByChatType: config.replyToModeByChatType, - dm: config.dm, - }; - } - function createSlackMessage(overrides: Partial): SlackMessageEvent { return { ...defaultMessageTemplate, ...overrides } as SlackMessageEvent; } diff --git a/src/telegram/accounts.test.ts b/src/telegram/accounts.test.ts index b53c9ef6ded..33112386d7d 100644 --- a/src/telegram/accounts.test.ts +++ b/src/telegram/accounts.test.ts @@ -227,6 +227,21 @@ describe("resolveTelegramAccount groups inheritance (#30673)", () => { }, }); + const createDefaultAccountGroupsConfig = (includeDevAccount: boolean): OpenClawConfig => ({ + channels: { + telegram: { + groups: { "-100999": { requireMention: true } }, + accounts: { + default: { + botToken: "123:default", + groups: { "-100123": { requireMention: false } }, + }, + ...(includeDevAccount ? { dev: { botToken: "456:dev" } } : {}), + }, + }, + }, + }); + it("inherits channel-level groups in single-account setup", () => { const resolved = resolveTelegramAccount({ cfg: { @@ -265,20 +280,7 @@ describe("resolveTelegramAccount groups inheritance (#30673)", () => { it("uses account-level groups even in multi-account setup", () => { const resolved = resolveTelegramAccount({ - cfg: { - channels: { - telegram: { - groups: { "-100999": { requireMention: true } }, - accounts: { - default: { - botToken: "123:default", - groups: { "-100123": { requireMention: false } }, - }, - dev: { botToken: "456:dev" }, - }, - }, - }, - }, + cfg: createDefaultAccountGroupsConfig(true), accountId: "default", }); @@ -287,19 +289,7 @@ describe("resolveTelegramAccount groups inheritance (#30673)", () => { it("account-level groups takes priority over channel-level in single-account setup", () => { const resolved = resolveTelegramAccount({ - cfg: { - channels: { - telegram: { - groups: { "-100999": { requireMention: true } }, - accounts: { - default: { - botToken: "123:default", - groups: { "-100123": { requireMention: false } }, - }, - }, - }, - }, - }, + cfg: createDefaultAccountGroupsConfig(false), accountId: "default", }); diff --git a/src/telegram/bot-native-command-menu.test.ts b/src/telegram/bot-native-command-menu.test.ts index c249f0ff761..d3fa114944c 100644 --- a/src/telegram/bot-native-command-menu.test.ts +++ b/src/telegram/bot-native-command-menu.test.ts @@ -6,6 +6,31 @@ import { syncTelegramMenuCommands, } from "./bot-native-command-menu.js"; +type SyncMenuOptions = { + deleteMyCommands: ReturnType; + setMyCommands: ReturnType; + commandsToRegister: Parameters[0]["commandsToRegister"]; + accountId: string; + botIdentity: string; + runtimeLog?: ReturnType; +}; + +function syncMenuCommandsWithMocks(options: SyncMenuOptions): void { + syncTelegramMenuCommands({ + bot: { + api: { deleteMyCommands: options.deleteMyCommands, setMyCommands: options.setMyCommands }, + } as unknown as Parameters[0]["bot"], + runtime: { + log: options.runtimeLog ?? vi.fn(), + error: vi.fn(), + exit: vi.fn(), + } as Parameters[0]["runtime"], + commandsToRegister: options.commandsToRegister, + accountId: options.accountId, + botIdentity: options.botIdentity, + }); +} + describe("bot-native-command-menu", () => { it("caps menu entries to Telegram limit", () => { const allCommands = Array.from({ length: 105 }, (_, i) => ({ @@ -91,14 +116,9 @@ describe("bot-native-command-menu", () => { callOrder.push("set"); }); - syncTelegramMenuCommands({ - bot: { - api: { - deleteMyCommands, - setMyCommands, - }, - } as unknown as Parameters[0]["bot"], - runtime: {} as Parameters[0]["runtime"], + syncMenuCommandsWithMocks({ + deleteMyCommands, + setMyCommands, commandsToRegister: [{ command: "cmd", description: "Command" }], accountId: `test-delete-${Date.now()}`, botIdentity: "bot-a", @@ -136,13 +156,10 @@ describe("bot-native-command-menu", () => { const commands = [{ command: "skip_test", description: "Skip test command" }]; // First sync — no cached hash, should call setMyCommands. - syncTelegramMenuCommands({ - bot: { - api: { deleteMyCommands, setMyCommands }, - } as unknown as Parameters[0]["bot"], - runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters< - typeof syncTelegramMenuCommands - >[0]["runtime"], + syncMenuCommandsWithMocks({ + deleteMyCommands, + setMyCommands, + runtimeLog, commandsToRegister: commands, accountId, botIdentity: "bot-a", @@ -153,13 +170,10 @@ describe("bot-native-command-menu", () => { }); // Second sync with the same commands — hash is cached, should skip. - syncTelegramMenuCommands({ - bot: { - api: { deleteMyCommands, setMyCommands }, - } as unknown as Parameters[0]["bot"], - runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters< - typeof syncTelegramMenuCommands - >[0]["runtime"], + syncMenuCommandsWithMocks({ + deleteMyCommands, + setMyCommands, + runtimeLog, commandsToRegister: commands, accountId, botIdentity: "bot-a", @@ -180,26 +194,20 @@ describe("bot-native-command-menu", () => { const accountId = `test-bot-identity-${Date.now()}`; const commands = [{ command: "same", description: "Same" }]; - syncTelegramMenuCommands({ - bot: { api: { deleteMyCommands, setMyCommands } } as unknown as Parameters< - typeof syncTelegramMenuCommands - >[0]["bot"], - runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters< - typeof syncTelegramMenuCommands - >[0]["runtime"], + syncMenuCommandsWithMocks({ + deleteMyCommands, + setMyCommands, + runtimeLog, commandsToRegister: commands, accountId, botIdentity: "token-bot-a", }); await vi.waitFor(() => expect(setMyCommands).toHaveBeenCalledTimes(1)); - syncTelegramMenuCommands({ - bot: { api: { deleteMyCommands, setMyCommands } } as unknown as Parameters< - typeof syncTelegramMenuCommands - >[0]["bot"], - runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters< - typeof syncTelegramMenuCommands - >[0]["runtime"], + syncMenuCommandsWithMocks({ + deleteMyCommands, + setMyCommands, + runtimeLog, commandsToRegister: commands, accountId, botIdentity: "token-bot-b", @@ -217,26 +225,20 @@ describe("bot-native-command-menu", () => { const runtimeLog = vi.fn(); const accountId = `test-empty-delete-fail-${Date.now()}`; - syncTelegramMenuCommands({ - bot: { api: { deleteMyCommands, setMyCommands } } as unknown as Parameters< - typeof syncTelegramMenuCommands - >[0]["bot"], - runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters< - typeof syncTelegramMenuCommands - >[0]["runtime"], + syncMenuCommandsWithMocks({ + deleteMyCommands, + setMyCommands, + runtimeLog, commandsToRegister: [], accountId, botIdentity: "bot-a", }); await vi.waitFor(() => expect(deleteMyCommands).toHaveBeenCalledTimes(1)); - syncTelegramMenuCommands({ - bot: { api: { deleteMyCommands, setMyCommands } } as unknown as Parameters< - typeof syncTelegramMenuCommands - >[0]["bot"], - runtime: { log: runtimeLog, error: vi.fn(), exit: vi.fn() } as Parameters< - typeof syncTelegramMenuCommands - >[0]["runtime"], + syncMenuCommandsWithMocks({ + deleteMyCommands, + setMyCommands, + runtimeLog, commandsToRegister: [], accountId, botIdentity: "bot-a", diff --git a/src/telegram/fetch.test.ts b/src/telegram/fetch.test.ts index 7019b4cb513..95b26d931cb 100644 --- a/src/telegram/fetch.test.ts +++ b/src/telegram/fetch.test.ts @@ -46,6 +46,14 @@ function expectEnvProxyAgentConstructorCall(params: { nth: number; autoSelectFam }); } +function resolveTelegramFetchOrThrow() { + const resolved = resolveTelegramFetch(); + if (!resolved) { + throw new Error("expected resolved fetch"); + } + return resolved; +} + afterEach(() => { resetTelegramFetchStateForTests(); setDefaultAutoSelectFamily.mockReset(); @@ -233,10 +241,7 @@ describe("resolveTelegramFetch", () => { .mockResolvedValueOnce({ ok: true } as Response); globalThis.fetch = fetchMock as unknown as typeof fetch; - const resolved = resolveTelegramFetch(); - if (!resolved) { - throw new Error("expected resolved fetch"); - } + const resolved = resolveTelegramFetchOrThrow(); await resolved("https://api.telegram.org/file/botx/photos/file_1.jpg"); @@ -261,10 +266,7 @@ describe("resolveTelegramFetch", () => { .mockResolvedValueOnce({ ok: true } as Response); globalThis.fetch = fetchMock as unknown as typeof fetch; - const resolved = resolveTelegramFetch(); - if (!resolved) { - throw new Error("expected resolved fetch"); - } + const resolved = resolveTelegramFetchOrThrow(); await resolved("https://api.telegram.org/file/botx/photos/file_1.jpg"); await resolved("https://api.telegram.org/file/botx/photos/file_2.jpg"); @@ -281,10 +283,7 @@ describe("resolveTelegramFetch", () => { const fetchMock = vi.fn().mockRejectedValue(fetchError); globalThis.fetch = fetchMock as unknown as typeof fetch; - const resolved = resolveTelegramFetch(); - if (!resolved) { - throw new Error("expected resolved fetch"); - } + const resolved = resolveTelegramFetchOrThrow(); await expect(resolved("https://api.telegram.org/file/botx/photos/file_3.jpg")).rejects.toThrow( "fetch failed", diff --git a/src/telegram/monitor.test.ts b/src/telegram/monitor.test.ts index afcb4994379..b9b8e473e21 100644 --- a/src/telegram/monitor.test.ts +++ b/src/telegram/monitor.test.ts @@ -83,10 +83,15 @@ const makeRunnerStub = (overrides: Partial = {}): RunnerStub => ({ isRunning: overrides.isRunning ?? (() => false), }); -async function monitorWithAutoAbort( - opts: Omit[0], "abortSignal"> = {}, -) { - const abort = new AbortController(); +function makeRecoverableFetchError() { + return Object.assign(new TypeError("fetch failed"), { + cause: Object.assign(new Error("connect timeout"), { + code: "UND_ERR_CONNECT_TIMEOUT", + }), + }); +} + +function mockRunOnceAndAbort(abort: AbortController) { runSpy.mockImplementationOnce(() => makeRunnerStub({ task: async () => { @@ -94,6 +99,13 @@ async function monitorWithAutoAbort( }, }), ); +} + +async function monitorWithAutoAbort( + opts: Omit[0], "abortSignal"> = {}, +) { + const abort = new AbortController(); + mockRunOnceAndAbort(abort); await monitorTelegramProvider({ token: "tok", ...opts, @@ -254,11 +266,7 @@ describe("monitorTelegramProvider (grammY)", () => { it("retries on recoverable undici fetch errors", async () => { const abort = new AbortController(); - const networkError = Object.assign(new TypeError("fetch failed"), { - cause: Object.assign(new Error("connect timeout"), { - code: "UND_ERR_CONNECT_TIMEOUT", - }), - }); + const networkError = makeRecoverableFetchError(); runSpy .mockImplementationOnce(() => makeRunnerStub({ @@ -305,20 +313,10 @@ describe("monitorTelegramProvider (grammY)", () => { it("retries recoverable deleteWebhook failures before polling", async () => { const abort = new AbortController(); - const cleanupError = Object.assign(new TypeError("fetch failed"), { - cause: Object.assign(new Error("connect timeout"), { - code: "UND_ERR_CONNECT_TIMEOUT", - }), - }); + const cleanupError = makeRecoverableFetchError(); api.deleteWebhook.mockReset(); api.deleteWebhook.mockRejectedValueOnce(cleanupError).mockResolvedValueOnce(true); - runSpy.mockImplementationOnce(() => - makeRunnerStub({ - task: async () => { - abort.abort(); - }, - }), - ); + mockRunOnceAndAbort(abort); await monitorTelegramProvider({ token: "tok", abortSignal: abort.signal }); @@ -330,20 +328,9 @@ describe("monitorTelegramProvider (grammY)", () => { it("retries setup-time recoverable errors before starting polling", async () => { const abort = new AbortController(); - const setupError = Object.assign(new TypeError("fetch failed"), { - cause: Object.assign(new Error("connect timeout"), { - code: "UND_ERR_CONNECT_TIMEOUT", - }), - }); + const setupError = makeRecoverableFetchError(); createTelegramBotErrors.push(setupError); - - runSpy.mockImplementationOnce(() => - makeRunnerStub({ - task: async () => { - abort.abort(); - }, - }), - ); + mockRunOnceAndAbort(abort); await monitorTelegramProvider({ token: "tok", abortSignal: abort.signal }); @@ -354,11 +341,7 @@ describe("monitorTelegramProvider (grammY)", () => { it("awaits runner.stop before retrying after recoverable polling error", async () => { const abort = new AbortController(); - const recoverableError = Object.assign(new TypeError("fetch failed"), { - cause: Object.assign(new Error("connect timeout"), { - code: "UND_ERR_CONNECT_TIMEOUT", - }), - }); + const recoverableError = makeRecoverableFetchError(); let firstStopped = false; const firstStop = vi.fn(async () => { await Promise.resolve(); diff --git a/src/web/auto-reply/deliver-reply.test.ts b/src/web/auto-reply/deliver-reply.test.ts index e3dfe6126bb..6a2810d182a 100644 --- a/src/web/auto-reply/deliver-reply.test.ts +++ b/src/web/auto-reply/deliver-reply.test.ts @@ -69,37 +69,27 @@ const replyLogger = { warn: vi.fn(), }; +async function expectReplySuppressed(replyResult: { text: string; isReasoning?: boolean }) { + const msg = makeMsg(); + await deliverWebReply({ + replyResult, + msg, + maxMediaBytes: 1024 * 1024, + textLimit: 200, + replyLogger, + skipLog: true, + }); + expect(msg.reply).not.toHaveBeenCalled(); + expect(msg.sendMedia).not.toHaveBeenCalled(); +} + describe("deliverWebReply", () => { it("suppresses payloads flagged as reasoning", async () => { - const msg = makeMsg(); - - await deliverWebReply({ - replyResult: { text: "Reasoning:\n_hidden_", isReasoning: true }, - msg, - maxMediaBytes: 1024 * 1024, - textLimit: 200, - replyLogger, - skipLog: true, - }); - - expect(msg.reply).not.toHaveBeenCalled(); - expect(msg.sendMedia).not.toHaveBeenCalled(); + await expectReplySuppressed({ text: "Reasoning:\n_hidden_", isReasoning: true }); }); it("suppresses payloads that start with reasoning prefix text", async () => { - const msg = makeMsg(); - - await deliverWebReply({ - replyResult: { text: " \n Reasoning:\n_hidden_" }, - msg, - maxMediaBytes: 1024 * 1024, - textLimit: 200, - replyLogger, - skipLog: true, - }); - - expect(msg.reply).not.toHaveBeenCalled(); - expect(msg.sendMedia).not.toHaveBeenCalled(); + await expectReplySuppressed({ text: " \n Reasoning:\n_hidden_" }); }); it("does not suppress messages that mention Reasoning: mid-text", async () => {