From 7d9172d5e093435d37ed3e00e476a4eaf84a252b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 24 Apr 2026 07:13:39 +0100 Subject: [PATCH] perf: speed up slow extension tests --- .../monitor/message-handler/prepare.test.ts | 199 +++++++++++++++- .../prepare.thread-context-allowlist.test.ts | 223 ------------------ extensions/whatsapp/src/session.test.ts | 45 ++-- 3 files changed, 219 insertions(+), 248 deletions(-) delete mode 100644 extensions/slack/src/monitor/message-handler/prepare.thread-context-allowlist.test.ts diff --git a/extensions/slack/src/monitor/message-handler/prepare.test.ts b/extensions/slack/src/monitor/message-handler/prepare.test.ts index 775d64724e5..08d2c8bfbb7 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test.ts @@ -10,10 +10,11 @@ import { } from "openclaw/plugin-sdk/conversation-runtime"; import { resolveAgentRoute } from "openclaw/plugin-sdk/routing"; import { resolveThreadSessionKeys } from "openclaw/plugin-sdk/routing"; -import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { ResolvedSlackAccount } from "../../accounts.js"; import type { SlackMessageEvent } from "../../types.js"; import type { SlackMonitorContext } from "../context.js"; +import { resetSlackThreadStarterCacheForTest } from "../media.js"; import { resolveSlackMessageContent } from "./prepare-content.js"; import { prepareSlackMessage } from "./prepare.js"; import { @@ -29,6 +30,10 @@ describe("slack prepareSlackMessage inbound contract", () => { storeFixture.setup(); }); + beforeEach(() => { + resetSlackThreadStarterCacheForTest(); + }); + afterAll(() => { storeFixture.cleanup(); }); @@ -126,6 +131,102 @@ describe("slack prepareSlackMessage inbound contract", () => { return prepareMessageWith(ctx, createThreadAccount(), createThreadReplyMessage(overrides)); } + type ThreadContextAllowlistCaseParams = { + channel: string; + channelType: SlackMessageEvent["channel_type"]; + user: string; + userName: string; + starterText: string; + followUpText: string; + startTs: string; + replyTs: string; + followUpTs: string; + currentTs: string; + channelsConfig?: Parameters[0]["channelsConfig"]; + resolveChannelName?: (channelId: string) => Promise<{ + name?: string; + type?: SlackMessageEvent["channel_type"]; + topic?: string; + purpose?: string; + }>; + }; + + async function prepareThreadContextAllowlistCase(params: ThreadContextAllowlistCaseParams) { + const { storePath } = storeFixture.makeTmpStorePath(); + const replies = vi + .fn() + .mockResolvedValueOnce({ + messages: [{ text: params.starterText, user: params.user, ts: params.startTs }], + }) + .mockResolvedValueOnce({ + messages: [ + { text: params.starterText, user: params.user, ts: params.startTs }, + { text: "assistant reply", bot_id: "B1", ts: params.replyTs }, + { text: params.followUpText, user: params.user, ts: params.followUpTs }, + { text: "current message", user: params.user, ts: params.currentTs }, + ], + response_metadata: { next_cursor: "" }, + }); + const ctx = createInboundSlackCtx({ + cfg: { + session: { store: storePath }, + channels: { + slack: { + enabled: true, + replyToMode: "all", + groupPolicy: "open", + contextVisibility: "allowlist", + }, + }, + } as OpenClawConfig, + appClient: { conversations: { replies } } as unknown as App["client"], + defaultRequireMention: false, + replyToMode: "all", + channelsConfig: params.channelsConfig, + }); + ctx.allowFrom = ["u-owner"]; + ctx.resolveUserName = async (id: string) => ({ + name: id === params.user ? params.userName : "Owner", + }); + if (params.resolveChannelName) { + ctx.resolveChannelName = params.resolveChannelName; + } + + const prepared = await prepareSlackMessage({ + ctx, + account: createSlackAccount({ + replyToMode: "all", + thread: { initialHistoryLimit: 20 }, + }), + message: { + channel: params.channel, + channel_type: params.channelType, + user: params.user, + text: "current message", + ts: params.currentTs, + thread_ts: params.startTs, + } as SlackMessageEvent, + opts: { source: "message" }, + }); + + return { prepared, replies }; + } + + function expectThreadContextAllowsHumanHistory( + prepared: Awaited>, + replies: ReturnType, + starterText: string, + followUpText: string, + ) { + expect(prepared).toBeTruthy(); + expect(prepared!.ctxPayload.ThreadStarterBody).toBe(starterText); + expect(prepared!.ctxPayload.ThreadHistoryBody).toContain(starterText); + expect(prepared!.ctxPayload.ThreadHistoryBody).toContain(followUpText); + expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply"); + expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message"); + expect(replies).toHaveBeenCalledTimes(2); + } + function createDmScopeMainSlackCtx(): SlackMonitorContext { const slackCtx = createInboundSlackCtx({ cfg: { @@ -502,6 +603,102 @@ describe("slack prepareSlackMessage inbound contract", () => { expect(replies).toHaveBeenCalledTimes(2); }); + it("uses room users allowlist for thread context filtering", async () => { + const { prepared, replies } = await prepareThreadContextAllowlistCase({ + channel: "C123", + channelType: "channel", + user: "U1", + userName: "Alice", + starterText: "starter from room user", + followUpText: "allowed follow-up", + startTs: "100.000", + replyTs: "100.500", + followUpTs: "100.800", + currentTs: "101.000", + channelsConfig: { + C123: { + users: ["U1"], + requireMention: false, + }, + }, + resolveChannelName: async () => ({ name: "general", type: "channel" }), + }); + + expectThreadContextAllowsHumanHistory( + prepared, + replies, + "starter from room user", + "allowed follow-up", + ); + }); + + it("does not apply the owner allowlist to open-room thread context", async () => { + const { prepared, replies } = await prepareThreadContextAllowlistCase({ + channel: "C124", + channelType: "channel", + user: "U2", + userName: "Bob", + starterText: "starter from open room", + followUpText: "open-room follow-up", + startTs: "200.000", + replyTs: "200.500", + followUpTs: "200.800", + currentTs: "201.000", + channelsConfig: { + C124: { + requireMention: false, + }, + }, + resolveChannelName: async () => ({ name: "general", type: "channel" }), + }); + + expectThreadContextAllowsHumanHistory( + prepared, + replies, + "starter from open room", + "open-room follow-up", + ); + }); + + it("does not apply the owner allowlist to open DMs when dmPolicy is open", async () => { + const { prepared, replies } = await prepareThreadContextAllowlistCase({ + channel: "D300", + channelType: "im", + user: "U3", + userName: "Dana", + starterText: "starter from open dm", + followUpText: "dm follow-up", + startTs: "300.000", + replyTs: "300.500", + followUpTs: "300.800", + currentTs: "301.000", + }); + + expectThreadContextAllowsHumanHistory( + prepared, + replies, + "starter from open dm", + "dm follow-up", + ); + }); + + it("does not apply the owner allowlist to MPIM thread context", async () => { + const { prepared, replies } = await prepareThreadContextAllowlistCase({ + channel: "G400", + channelType: "mpim", + user: "U4", + userName: "Evan", + starterText: "starter from mpim", + followUpText: "mpim follow-up", + startTs: "400.000", + replyTs: "400.500", + followUpTs: "400.800", + currentTs: "401.000", + }); + + expectThreadContextAllowsHumanHistory(prepared, replies, "starter from mpim", "mpim follow-up"); + }); + it("skips loading thread history when thread session already exists in store (bloat fix)", async () => { const { storePath } = storeFixture.makeTmpStorePath(); const cfg = { diff --git a/extensions/slack/src/monitor/message-handler/prepare.thread-context-allowlist.test.ts b/extensions/slack/src/monitor/message-handler/prepare.thread-context-allowlist.test.ts deleted file mode 100644 index 359447c811d..00000000000 --- a/extensions/slack/src/monitor/message-handler/prepare.thread-context-allowlist.test.ts +++ /dev/null @@ -1,223 +0,0 @@ -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import type { App } from "@slack/bolt"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; -import { afterAll, describe, expect, it, vi } from "vitest"; -import type { SlackMessageEvent } from "../../types.js"; - -const [{ prepareSlackMessage }, helpers] = await Promise.all([ - import("./prepare.js"), - import("./prepare.test-helpers.js"), -]); -const { createInboundSlackTestContext, createSlackTestAccount } = helpers; -let fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-slack-room-thread-context-")); -let caseId = 0; - -function makeTmpStorePath() { - if (!fixtureRoot) { - throw new Error("fixtureRoot missing"); - } - const dir = path.join(fixtureRoot, `case-${caseId++}`); - fs.mkdirSync(dir); - return path.join(dir, "sessions.json"); -} - -type ThreadContextCaseParams = { - channel: string; - channelType: SlackMessageEvent["channel_type"]; - user: string; - userName: string; - starterText: string; - followUpText: string; - startTs: string; - replyTs: string; - followUpTs: string; - currentTs: string; - channelsConfig?: Parameters[0]["channelsConfig"]; - resolveChannelName?: (channelId: string) => Promise<{ - name?: string; - type?: SlackMessageEvent["channel_type"]; - topic?: string; - purpose?: string; - }>; -}; - -async function prepareThreadContextCase(params: ThreadContextCaseParams) { - const replies = vi - .fn() - .mockResolvedValueOnce({ - messages: [{ text: params.starterText, user: params.user, ts: params.startTs }], - }) - .mockResolvedValueOnce({ - messages: [ - { text: params.starterText, user: params.user, ts: params.startTs }, - { text: "assistant reply", bot_id: "B1", ts: params.replyTs }, - { text: params.followUpText, user: params.user, ts: params.followUpTs }, - { text: "current message", user: params.user, ts: params.currentTs }, - ], - response_metadata: { next_cursor: "" }, - }); - const ctx = createInboundSlackTestContext({ - cfg: { - session: { store: makeTmpStorePath() }, - channels: { - slack: { - enabled: true, - replyToMode: "all", - groupPolicy: "open", - contextVisibility: "allowlist", - }, - }, - } as OpenClawConfig, - appClient: { conversations: { replies } } as unknown as App["client"], - defaultRequireMention: false, - replyToMode: "all", - channelsConfig: params.channelsConfig, - }); - ctx.allowFrom = ["u-owner"]; - ctx.resolveUserName = async (id: string) => ({ - name: id === params.user ? params.userName : "Owner", - }); - if (params.resolveChannelName) { - ctx.resolveChannelName = params.resolveChannelName; - } - - const prepared = await prepareSlackMessage({ - ctx, - account: createSlackTestAccount({ - replyToMode: "all", - thread: { initialHistoryLimit: 20 }, - }), - message: { - channel: params.channel, - channel_type: params.channelType, - user: params.user, - text: "current message", - ts: params.currentTs, - thread_ts: params.startTs, - } as SlackMessageEvent, - opts: { source: "message" }, - }); - - return { prepared, replies }; -} - -describe("prepareSlackMessage thread context allowlists", () => { - afterAll(() => { - if (fixtureRoot) { - fs.rmSync(fixtureRoot, { - recursive: true, - force: true, - maxRetries: 5, - retryDelay: 50, - }); - fixtureRoot = ""; - } - }); - - it("uses room users allowlist for thread context filtering", async () => { - const { prepared, replies } = await prepareThreadContextCase({ - channel: "C123", - channelType: "channel", - user: "U1", - userName: "Alice", - starterText: "starter from room user", - followUpText: "allowed follow-up", - startTs: "100.000", - replyTs: "100.500", - followUpTs: "100.800", - currentTs: "101.000", - channelsConfig: { - C123: { - users: ["U1"], - requireMention: false, - }, - }, - resolveChannelName: async () => ({ name: "general", type: "channel" }), - }); - - expect(prepared).toBeTruthy(); - expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from room user"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from room user"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("allowed follow-up"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message"); - expect(replies).toHaveBeenCalledTimes(2); - }); - - it("does not apply the owner allowlist to open-room thread context", async () => { - const { prepared, replies } = await prepareThreadContextCase({ - channel: "C124", - channelType: "channel", - user: "U2", - userName: "Bob", - starterText: "starter from open room", - followUpText: "open-room follow-up", - startTs: "200.000", - replyTs: "200.500", - followUpTs: "200.800", - currentTs: "201.000", - channelsConfig: { - C124: { - requireMention: false, - }, - }, - resolveChannelName: async () => ({ name: "general", type: "channel" }), - }); - - expect(prepared).toBeTruthy(); - expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from open room"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from open room"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("open-room follow-up"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message"); - expect(replies).toHaveBeenCalledTimes(2); - }); - - it("does not apply the owner allowlist to open DMs when dmPolicy is open", async () => { - const { prepared, replies } = await prepareThreadContextCase({ - channel: "D300", - channelType: "im", - user: "U3", - userName: "Dana", - starterText: "starter from open dm", - followUpText: "dm follow-up", - startTs: "300.000", - replyTs: "300.500", - followUpTs: "300.800", - currentTs: "301.000", - }); - - expect(prepared).toBeTruthy(); - expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from open dm"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from open dm"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("dm follow-up"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message"); - expect(replies).toHaveBeenCalledTimes(2); - }); - - it("does not apply the owner allowlist to MPIM thread context", async () => { - const { prepared, replies } = await prepareThreadContextCase({ - channel: "G400", - channelType: "mpim", - user: "U4", - userName: "Evan", - starterText: "starter from mpim", - followUpText: "mpim follow-up", - startTs: "400.000", - replyTs: "400.500", - followUpTs: "400.800", - currentTs: "401.000", - }); - - expect(prepared).toBeTruthy(); - expect(prepared!.ctxPayload.ThreadStarterBody).toBe("starter from mpim"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("starter from mpim"); - expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("mpim follow-up"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("assistant reply"); - expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message"); - expect(replies).toHaveBeenCalledTimes(2); - }); -}); diff --git a/extensions/whatsapp/src/session.test.ts b/extensions/whatsapp/src/session.test.ts index 4b107678c30..b24bf1e87c3 100644 --- a/extensions/whatsapp/src/session.test.ts +++ b/extensions/whatsapp/src/session.test.ts @@ -4,6 +4,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { resetLogger, setLoggerOverride } from "openclaw/plugin-sdk/runtime-env"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { enqueueCredsSave } from "./creds-persistence.js"; import { baileys, getLastSocket, resetBaileysMocks, resetLoadConfigMock } from "./test-helpers.js"; const useMultiFileAuthStateMock = vi.mocked(baileys.useMultiFileAuthState); @@ -367,33 +368,29 @@ describe("web session", () => { const authDirA = createTempAuthDir("openclaw-wa-a"); const authDirB = createTempAuthDir("openclaw-wa-b"); - const openMock = mockFsOpenForCredsWrites({ - onTempWrite: async (filePath) => { - if (filePath.startsWith(authDirA)) { - inFlightA += 1; - await gateA; - inFlightA -= 1; - return; - } - if (filePath.startsWith(authDirB)) { - inFlightB += 1; - await gateB; - inFlightB -= 1; - } + const onError = vi.fn(); + + enqueueCredsSave( + authDirA, + async () => { + inFlightA += 1; + await gateA; + inFlightA -= 1; }, - }); - - await createWaSocket(false, false, { authDir: authDirA }); - const sockA = getLastSocket(); - await createWaSocket(false, false, { authDir: authDirB }); - const sockB = getLastSocket(); - - sockA.ev.emit("creds.update", {}); - sockB.ev.emit("creds.update", {}); + onError, + ); + enqueueCredsSave( + authDirB, + async () => { + inFlightB += 1; + await gateB; + inFlightB -= 1; + }, + onError, + ); await flushCredsUpdate(); - expect(openMock.tempHandles).toHaveLength(2); expect(inFlightA).toBe(1); expect(inFlightB).toBe(1); @@ -403,7 +400,7 @@ describe("web session", () => { expect(inFlightA).toBe(0); expect(inFlightB).toBe(0); - openMock.restore(); + expect(onError).not.toHaveBeenCalled(); }); it("rotates creds backup when creds.json is valid JSON", async () => {