From fb5d8a9cd14f94c7c7ffad520962e0d24df8cb4a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 20:18:41 +0000 Subject: [PATCH] perf(slack): memoize allow-from and mention paths --- src/slack/monitor/auth.test.ts | 35 +++++- src/slack/monitor/auth.ts | 110 ++++++++++++++++++- src/slack/monitor/context.ts | 9 +- src/slack/monitor/message-handler/prepare.ts | 23 +++- 4 files changed, 166 insertions(+), 11 deletions(-) diff --git a/src/slack/monitor/auth.test.ts b/src/slack/monitor/auth.test.ts index ca9ac2025..20a46756c 100644 --- a/src/slack/monitor/auth.test.ts +++ b/src/slack/monitor/auth.test.ts @@ -7,17 +7,27 @@ vi.mock("../../pairing/pairing-store.js", () => ({ readChannelAllowFromStore: (...args: unknown[]) => readChannelAllowFromStoreMock(...args), })); -import { resolveSlackEffectiveAllowFrom } from "./auth.js"; +import { clearSlackAllowFromCacheForTest, resolveSlackEffectiveAllowFrom } from "./auth.js"; function makeSlackCtx(allowFrom: string[]): SlackMonitorContext { return { allowFrom, + accountId: "main", + dmPolicy: "pairing", } as unknown as SlackMonitorContext; } describe("resolveSlackEffectiveAllowFrom", () => { + const prevTtl = process.env.OPENCLAW_SLACK_PAIRING_ALLOWFROM_CACHE_TTL_MS; + beforeEach(() => { readChannelAllowFromStoreMock.mockReset(); + clearSlackAllowFromCacheForTest(); + if (prevTtl === undefined) { + delete process.env.OPENCLAW_SLACK_PAIRING_ALLOWFROM_CACHE_TTL_MS; + } else { + process.env.OPENCLAW_SLACK_PAIRING_ALLOWFROM_CACHE_TTL_MS = prevTtl; + } }); it("falls back to channel config allowFrom when pairing store throws", async () => { @@ -37,4 +47,27 @@ describe("resolveSlackEffectiveAllowFrom", () => { expect(effective.allowFrom).toEqual(["u1"]); expect(effective.allowFromLower).toEqual(["u1"]); }); + + it("memoizes pairing-store allowFrom reads within TTL", async () => { + readChannelAllowFromStoreMock.mockResolvedValue(["u2"]); + const ctx = makeSlackCtx(["u1"]); + + const first = await resolveSlackEffectiveAllowFrom(ctx, { includePairingStore: true }); + const second = await resolveSlackEffectiveAllowFrom(ctx, { includePairingStore: true }); + + expect(first.allowFrom).toEqual(["u1", "u2"]); + expect(second.allowFrom).toEqual(["u1", "u2"]); + expect(readChannelAllowFromStoreMock).toHaveBeenCalledTimes(1); + }); + + it("refreshes pairing-store allowFrom when cache TTL is zero", async () => { + process.env.OPENCLAW_SLACK_PAIRING_ALLOWFROM_CACHE_TTL_MS = "0"; + readChannelAllowFromStoreMock.mockResolvedValue(["u2"]); + const ctx = makeSlackCtx(["u1"]); + + await resolveSlackEffectiveAllowFrom(ctx, { includePairingStore: true }); + await resolveSlackEffectiveAllowFrom(ctx, { includePairingStore: true }); + + expect(readChannelAllowFromStoreMock).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/slack/monitor/auth.ts b/src/slack/monitor/auth.ts index 0b5ba9469..7706c0fb5 100644 --- a/src/slack/monitor/auth.ts +++ b/src/slack/monitor/auth.ts @@ -8,13 +8,89 @@ import { import { resolveSlackChannelConfig } from "./channel-config.js"; import { normalizeSlackChannelType, type SlackMonitorContext } from "./context.js"; +type ResolvedAllowFromLists = { + allowFrom: string[]; + allowFromLower: string[]; +}; + +type SlackAllowFromCacheState = { + baseSignature?: string; + base?: ResolvedAllowFromLists; + pairingKey?: string; + pairing?: ResolvedAllowFromLists; + pairingExpiresAtMs?: number; + pairingPending?: Promise; +}; + +let slackAllowFromCache = new WeakMap(); +const DEFAULT_PAIRING_ALLOW_FROM_CACHE_TTL_MS = 5000; + +function getPairingAllowFromCacheTtlMs(): number { + const raw = process.env.OPENCLAW_SLACK_PAIRING_ALLOWFROM_CACHE_TTL_MS?.trim(); + if (!raw) { + return DEFAULT_PAIRING_ALLOW_FROM_CACHE_TTL_MS; + } + const parsed = Number(raw); + if (!Number.isFinite(parsed)) { + return DEFAULT_PAIRING_ALLOW_FROM_CACHE_TTL_MS; + } + return Math.max(0, Math.floor(parsed)); +} + +function getAllowFromCacheState(ctx: SlackMonitorContext): SlackAllowFromCacheState { + const existing = slackAllowFromCache.get(ctx); + if (existing) { + return existing; + } + const next: SlackAllowFromCacheState = {}; + slackAllowFromCache.set(ctx, next); + return next; +} + +function buildBaseAllowFrom(ctx: SlackMonitorContext): ResolvedAllowFromLists { + const allowFrom = normalizeAllowList(ctx.allowFrom); + return { + allowFrom, + allowFromLower: normalizeAllowListLower(allowFrom), + }; +} + export async function resolveSlackEffectiveAllowFrom( ctx: SlackMonitorContext, options?: { includePairingStore?: boolean }, ) { const includePairingStore = options?.includePairingStore === true; - let storeAllowFrom: string[] = []; - if (includePairingStore) { + const cache = getAllowFromCacheState(ctx); + const baseSignature = JSON.stringify(ctx.allowFrom); + if (cache.baseSignature !== baseSignature || !cache.base) { + cache.baseSignature = baseSignature; + cache.base = buildBaseAllowFrom(ctx); + cache.pairing = undefined; + cache.pairingKey = undefined; + cache.pairingExpiresAtMs = undefined; + cache.pairingPending = undefined; + } + if (!includePairingStore) { + return cache.base; + } + + const ttlMs = getPairingAllowFromCacheTtlMs(); + const nowMs = Date.now(); + const pairingKey = `${ctx.accountId}:${ctx.dmPolicy}`; + if ( + ttlMs > 0 && + cache.pairing && + cache.pairingKey === pairingKey && + (cache.pairingExpiresAtMs ?? 0) >= nowMs + ) { + return cache.pairing; + } + if (cache.pairingPending && cache.pairingKey === pairingKey) { + return await cache.pairingPending; + } + + const pairingPending = (async (): Promise => { + let storeAllowFrom: string[] = []; try { const resolved = await readStoreAllowFromForDmPolicy({ provider: "slack", @@ -25,10 +101,34 @@ export async function resolveSlackEffectiveAllowFrom( } catch { storeAllowFrom = []; } + const allowFrom = normalizeAllowList([...(cache.base?.allowFrom ?? []), ...storeAllowFrom]); + return { + allowFrom, + allowFromLower: normalizeAllowListLower(allowFrom), + }; + })(); + + cache.pairingKey = pairingKey; + cache.pairingPending = pairingPending; + try { + const resolved = await pairingPending; + if (ttlMs > 0) { + cache.pairing = resolved; + cache.pairingExpiresAtMs = nowMs + ttlMs; + } else { + cache.pairing = undefined; + cache.pairingExpiresAtMs = undefined; + } + return resolved; + } finally { + if (cache.pairingPending === pairingPending) { + cache.pairingPending = undefined; + } } - const allowFrom = normalizeAllowList([...ctx.allowFrom, ...storeAllowFrom]); - const allowFromLower = normalizeAllowListLower(allowFrom); - return { allowFrom, allowFromLower }; +} + +export function clearSlackAllowFromCacheForTest(): void { + slackAllowFromCache = new WeakMap(); } export function isSlackSenderAllowListed(params: { diff --git a/src/slack/monitor/context.ts b/src/slack/monitor/context.ts index 63fa3907f..553f18671 100644 --- a/src/slack/monitor/context.ts +++ b/src/slack/monitor/context.ts @@ -170,7 +170,9 @@ export function createSlackMonitorContext(params: { const allowFrom = normalizeAllowList(params.allowFrom); const groupDmChannels = normalizeAllowList(params.groupDmChannels); + const groupDmChannelsLower = normalizeAllowListLower(groupDmChannels); const defaultRequireMention = params.defaultRequireMention ?? true; + const hasChannelAllowlistConfig = Object.keys(params.channelsConfig ?? {}).length > 0; const markMessageSeen = (channelId: string | undefined, ts?: string) => { if (!channelId || !ts) { @@ -308,7 +310,6 @@ export function createSlackMonitorContext(params: { } if (isGroupDm && groupDmChannels.length > 0) { - const allowList = normalizeAllowListLower(groupDmChannels); const candidates = [ p.channelId, p.channelName ? `#${p.channelName}` : undefined, @@ -318,7 +319,8 @@ export function createSlackMonitorContext(params: { .filter((value): value is string => Boolean(value)) .map((value) => value.toLowerCase()); const permitted = - allowList.includes("*") || candidates.some((candidate) => allowList.includes(candidate)); + groupDmChannelsLower.includes("*") || + candidates.some((candidate) => groupDmChannelsLower.includes(candidate)); if (!permitted) { return false; } @@ -333,8 +335,7 @@ export function createSlackMonitorContext(params: { }); const channelMatchMeta = formatAllowlistMatchMeta(channelConfig); const channelAllowed = channelConfig?.allowed !== false; - const channelAllowlistConfigured = - Boolean(params.channelsConfig) && Object.keys(params.channelsConfig ?? {}).length > 0; + const channelAllowlistConfigured = hasChannelAllowlistConfig; if ( !isSlackChannelAllowedByPolicy({ groupPolicy: params.groupPolicy, diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index 2819d5e59..b83b19c18 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -51,6 +51,27 @@ import { import { resolveSlackRoomContextHints } from "../room-context.js"; import type { PreparedSlackMessage } from "./types.js"; +const mentionRegexCache = new WeakMap>(); + +function resolveCachedMentionRegexes( + ctx: SlackMonitorContext, + agentId: string | undefined, +): RegExp[] { + const key = agentId?.trim() || "__default__"; + let byAgent = mentionRegexCache.get(ctx); + if (!byAgent) { + byAgent = new Map(); + mentionRegexCache.set(ctx, byAgent); + } + const cached = byAgent.get(key); + if (cached) { + return cached; + } + const built = buildMentionRegexes(ctx.cfg, agentId); + byAgent.set(key, built); + return built; +} + export async function prepareSlackMessage(params: { ctx: SlackMonitorContext; account: ResolvedSlackAccount; @@ -205,7 +226,7 @@ export async function prepareSlackMessage(params: { const historyKey = isThreadReply && ctx.threadHistoryScope === "thread" ? sessionKey : message.channel; - const mentionRegexes = buildMentionRegexes(cfg, route.agentId); + const mentionRegexes = resolveCachedMentionRegexes(ctx, route.agentId); const hasAnyMention = /<@[^>]+>/.test(message.text ?? ""); const explicitlyMentioned = Boolean( ctx.botUserId && message.text?.includes(`<@${ctx.botUserId}>`),