perf(slack): memoize allow-from and mention paths
This commit is contained in:
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<ResolvedAllowFromLists>;
|
||||
};
|
||||
|
||||
let slackAllowFromCache = new WeakMap<SlackMonitorContext, SlackAllowFromCacheState>();
|
||||
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<ResolvedAllowFromLists> => {
|
||||
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<SlackMonitorContext, SlackAllowFromCacheState>();
|
||||
}
|
||||
|
||||
export function isSlackSenderAllowListed(params: {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -51,6 +51,27 @@ import {
|
||||
import { resolveSlackRoomContextHints } from "../room-context.js";
|
||||
import type { PreparedSlackMessage } from "./types.js";
|
||||
|
||||
const mentionRegexCache = new WeakMap<SlackMonitorContext, Map<string, RegExp[]>>();
|
||||
|
||||
function resolveCachedMentionRegexes(
|
||||
ctx: SlackMonitorContext,
|
||||
agentId: string | undefined,
|
||||
): RegExp[] {
|
||||
const key = agentId?.trim() || "__default__";
|
||||
let byAgent = mentionRegexCache.get(ctx);
|
||||
if (!byAgent) {
|
||||
byAgent = new Map<string, RegExp[]>();
|
||||
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}>`),
|
||||
|
||||
Reference in New Issue
Block a user