From 7bd2f391b06e02030703509f8eb04a1b035bf6c3 Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:03:11 +0800 Subject: [PATCH 1/3] perf(commands): cache owner allowlist resolution --- ...ommand-auth.owner-allow-from-cache.test.ts | 63 +++++++++++++++++++ src/auto-reply/command-auth.ts | 56 +++++++++++------ 2 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 src/auto-reply/command-auth.owner-allow-from-cache.test.ts diff --git a/src/auto-reply/command-auth.owner-allow-from-cache.test.ts b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts new file mode 100644 index 00000000000..4cd25adb8a3 --- /dev/null +++ b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts @@ -0,0 +1,63 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; +import type { OpenClawConfig } from "../config/config.js"; +import { createOutboundTestPlugin, createTestRegistry } from "../test-utils/channel-plugins.js"; +import { resolveCommandAuthorization } from "./command-auth.js"; +import type { MsgContext } from "./templating.js"; + +const largeListFormatter = vi.fn(); + +describe("resolveCommandAuthorization owner allowlist hot path", () => { + beforeEach(() => { + largeListFormatter.mockReset(); + const plugin = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + plugin.config = { + ...plugin.config, + formatAllowFrom: ({ allowFrom }) => { + if (allowFrom.length > 1) { + largeListFormatter(); + } + return allowFrom + .map((entry) => String(entry ?? "").trim()) + .filter((entry) => entry.length > 0); + }, + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "discord", plugin, source: "test" }]), + ); + }); + + afterEach(() => { + setActivePluginRegistry(createTestRegistry()); + }); + + it("reuses processed ownerAllowFrom for repeated authorizations on the same config snapshot", () => { + const cfg = { + channels: { discord: {} }, + commands: { ownerAllowFrom: ["123", "456", "789"] }, + } as OpenClawConfig; + + const ctx = { + Provider: "discord", + Surface: "discord", + From: "discord:123", + SenderId: "123", + } as MsgContext; + + resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + + expect(largeListFormatter).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index 44e9f5f30db..1ccec930e96 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -20,6 +20,8 @@ export type CommandAuthorization = { to?: string; }; +const ownerAllowFromListCache = new WeakMap, Map>(); + function resolveProviderFromContext(ctx: MsgContext, cfg: OpenClawConfig): ChannelId | undefined { const explicitMessageChannel = normalizeMessageChannel(ctx.Provider) ?? @@ -116,6 +118,11 @@ function resolveOwnerAllowFromList(params: { if (!Array.isArray(raw) || raw.length === 0) { return []; } + const cacheKey = `${params.plugin?.id ?? ""}\u0000${params.accountId ?? ""}\u0000${params.providerId ?? ""}`; + const cached = ownerAllowFromListCache.get(raw)?.get(cacheKey); + if (cached) { + return cached; + } const filtered: string[] = []; for (const entry of raw) { const trimmed = String(entry ?? "").trim(); @@ -139,12 +146,19 @@ function resolveOwnerAllowFromList(params: { } filtered.push(trimmed); } - return formatAllowFromList({ + const formatted = formatAllowFromList({ plugin: params.plugin, cfg: params.cfg, accountId: params.accountId, allowFrom: filtered, }); + let cachedByKey = ownerAllowFromListCache.get(raw); + if (!cachedByKey) { + cachedByKey = new Map(); + ownerAllowFromListCache.set(raw, cachedByKey); + } + cachedByKey.set(cacheKey, formatted); + return formatted; } /** @@ -244,16 +258,14 @@ function resolveSenderCandidates(params: { pushCandidate(params.from); } - const normalized: string[] = []; + const normalized = new Set(); for (const sender of candidates) { const entries = normalizeAllowFromEntry({ plugin, cfg, accountId, value: sender }); for (const entry of entries) { - if (!normalized.includes(entry)) { - normalized.push(entry); - } + normalized.add(entry); } } - return normalized; + return [...normalized]; } function resolveFallbackAllowFrom(params: { @@ -339,13 +351,16 @@ export function resolveCommandAuthorization(params: { providerId, allowFrom: cfg.commands?.ownerAllowFrom, }); - const contextOwnerAllowFromList = resolveOwnerAllowFromList({ - plugin, - cfg, - accountId: ctx.AccountId, - providerId, - allowFrom: ctx.OwnerAllowFrom, - }); + const contextOwnerAllowFromList = + Array.isArray(ctx.OwnerAllowFrom) && ctx.OwnerAllowFrom.length > 0 + ? resolveOwnerAllowFromList({ + plugin, + cfg, + accountId: ctx.AccountId, + providerId, + allowFrom: ctx.OwnerAllowFrom, + }) + : []; const allowAll = allowFromList.length === 0 || allowFromList.some((entry) => entry.trim() === "*"); @@ -375,6 +390,8 @@ export function resolveCommandAuthorization(params: { : ownerCandidatesForCommands, ), ); + const ownerListSet = new Set(ownerList); + const ownerCandidatesForCommandsSet = new Set(ownerCandidatesForCommands); const senderCandidates = resolveSenderCandidates({ plugin, @@ -386,11 +403,11 @@ export function resolveCommandAuthorization(params: { from, chatType: ctx.ChatType, }); - const matchedSender = ownerList.length - ? senderCandidates.find((candidate) => ownerList.includes(candidate)) + const matchedSender = ownerListSet.size + ? senderCandidates.find((candidate) => ownerListSet.has(candidate)) : undefined; - const matchedCommandOwner = ownerCandidatesForCommands.length - ? senderCandidates.find((candidate) => ownerCandidatesForCommands.includes(candidate)) + const matchedCommandOwner = ownerCandidatesForCommandsSet.size + ? senderCandidates.find((candidate) => ownerCandidatesForCommandsSet.has(candidate)) : undefined; const senderId = matchedSender ?? senderCandidates[0]; @@ -420,8 +437,9 @@ export function resolveCommandAuthorization(params: { if (commandsAllowFromList !== null) { // commands.allowFrom is configured - use it for authorization const commandsAllowAll = commandsAllowFromList.some((entry) => entry.trim() === "*"); - const matchedCommandsAllowFrom = commandsAllowFromList.length - ? senderCandidates.find((candidate) => commandsAllowFromList.includes(candidate)) + const commandsAllowFromSet = new Set(commandsAllowFromList); + const matchedCommandsAllowFrom = commandsAllowFromSet.size + ? senderCandidates.find((candidate) => commandsAllowFromSet.has(candidate)) : undefined; isAuthorizedSender = commandsAllowAll || Boolean(matchedCommandsAllowFrom); } else { From 10da4efff999f00a6ac5e5181fdcd433eba56c1c Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:22:40 +0800 Subject: [PATCH 2/3] fix(commands): scope owner allowlist cache to config and registry --- ...ommand-auth.owner-allow-from-cache.test.ts | 102 ++++++++++++++++++ src/auto-reply/command-auth.ts | 22 ++-- 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/src/auto-reply/command-auth.owner-allow-from-cache.test.ts b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts index 4cd25adb8a3..fb1f3af9414 100644 --- a/src/auto-reply/command-auth.owner-allow-from-cache.test.ts +++ b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts @@ -60,4 +60,106 @@ describe("resolveCommandAuthorization owner allowlist hot path", () => { expect(largeListFormatter).toHaveBeenCalledTimes(1); }); + + it("does not reuse cached ownerAllowFrom across config snapshots sharing the same array", () => { + const sharedOwnerAllowFrom = ["123", "456", "789"]; + const cfgA = { + channels: { discord: {} }, + commands: { ownerAllowFrom: sharedOwnerAllowFrom }, + testVariant: "A", + } as OpenClawConfig; + const cfgB = { + channels: { discord: {} }, + commands: { ownerAllowFrom: sharedOwnerAllowFrom }, + testVariant: "B", + } as OpenClawConfig; + + const plugin = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + plugin.config = { + ...plugin.config, + formatAllowFrom: ({ cfg, allowFrom }) => + allowFrom.map((entry) => `${(cfg as { testVariant?: string }).testVariant}:${String(entry)}`), + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "discord", plugin, source: "test" }]), + "owner-cache-config-a", + ); + + const ctx = { + Provider: "discord", + Surface: "discord", + From: "discord:123", + SenderId: "123", + } as MsgContext; + + const authA = resolveCommandAuthorization({ + ctx, + cfg: cfgA, + commandAuthorized: true, + }); + const authB = resolveCommandAuthorization({ + ctx, + cfg: cfgB, + commandAuthorized: true, + }); + + expect(authA.ownerList).toEqual(["A:123", "A:456", "A:789"]); + expect(authB.ownerList).toEqual(["B:123", "B:456", "B:789"]); + }); + + it("does not reuse cached ownerAllowFrom across plugin registry reloads", () => { + const sharedOwnerAllowFrom = ["123", "456", "789"]; + const cfg = { + channels: { discord: {} }, + commands: { ownerAllowFrom: sharedOwnerAllowFrom }, + } as OpenClawConfig; + const ctx = { + Provider: "discord", + Surface: "discord", + From: "discord:123", + SenderId: "123", + } as MsgContext; + + const pluginA = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + pluginA.config = { + ...pluginA.config, + formatAllowFrom: ({ allowFrom }) => allowFrom.map((entry) => `A:${String(entry)}`), + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "discord", plugin: pluginA, source: "test" }]), + "owner-cache-registry-a", + ); + const authA = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + + const pluginB = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + pluginB.config = { + ...pluginB.config, + formatAllowFrom: ({ allowFrom }) => allowFrom.map((entry) => `B:${String(entry)}`), + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "discord", plugin: pluginB, source: "test" }]), + "owner-cache-registry-b", + ); + const authB = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + + expect(authA.ownerList).toEqual(["A:123", "A:456", "A:789"]); + expect(authB.ownerList).toEqual(["B:123", "B:456", "B:789"]); + }); }); diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index 1ccec930e96..75f9d95140c 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -2,6 +2,7 @@ import { getChannelPlugin, listChannelPlugins } from "../channels/plugins/index. import type { ChannelId, ChannelPlugin } from "../channels/plugins/types.js"; import { normalizeAnyChannelId } from "../channels/registry.js"; import type { OpenClawConfig } from "../config/config.js"; +import { getActivePluginRegistryVersion } from "../plugins/runtime.js"; import { normalizeStringEntries } from "../shared/string-normalization.js"; import { INTERNAL_MESSAGE_CHANNEL, @@ -20,7 +21,10 @@ export type CommandAuthorization = { to?: string; }; -const ownerAllowFromListCache = new WeakMap, Map>(); +const ownerAllowFromListCache = new WeakMap< + OpenClawConfig, + WeakMap, Map> +>(); function resolveProviderFromContext(ctx: MsgContext, cfg: OpenClawConfig): ChannelId | undefined { const explicitMessageChannel = @@ -118,9 +122,10 @@ function resolveOwnerAllowFromList(params: { if (!Array.isArray(raw) || raw.length === 0) { return []; } - const cacheKey = `${params.plugin?.id ?? ""}\u0000${params.accountId ?? ""}\u0000${params.providerId ?? ""}`; - const cached = ownerAllowFromListCache.get(raw)?.get(cacheKey); - if (cached) { + const registryVersion = getActivePluginRegistryVersion(); + const cacheKey = `${registryVersion}\u0000${params.plugin?.id ?? ""}\u0000${params.accountId ?? ""}\u0000${params.providerId ?? ""}`; + const cached = ownerAllowFromListCache.get(params.cfg)?.get(raw)?.get(cacheKey); + if (cached !== undefined) { return cached; } const filtered: string[] = []; @@ -152,10 +157,15 @@ function resolveOwnerAllowFromList(params: { accountId: params.accountId, allowFrom: filtered, }); - let cachedByKey = ownerAllowFromListCache.get(raw); + let cachedByConfig = ownerAllowFromListCache.get(params.cfg); + if (!cachedByConfig) { + cachedByConfig = new WeakMap, Map>(); + ownerAllowFromListCache.set(params.cfg, cachedByConfig); + } + let cachedByKey = cachedByConfig.get(raw); if (!cachedByKey) { cachedByKey = new Map(); - ownerAllowFromListCache.set(raw, cachedByKey); + cachedByConfig.set(raw, cachedByKey); } cachedByKey.set(cacheKey, formatted); return formatted; From b82e05f33c1d2d8e919afa35664dccce7ec32e3e Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Fri, 20 Mar 2026 11:10:24 +0800 Subject: [PATCH 3/3] style(commands): format owner allowlist cache test --- .../command-auth.owner-allow-from-cache.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/auto-reply/command-auth.owner-allow-from-cache.test.ts b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts index fb1f3af9414..23023d7aa35 100644 --- a/src/auto-reply/command-auth.owner-allow-from-cache.test.ts +++ b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { setActivePluginRegistry } from "../plugins/runtime.js"; import type { OpenClawConfig } from "../config/config.js"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; import { createOutboundTestPlugin, createTestRegistry } from "../test-utils/channel-plugins.js"; import { resolveCommandAuthorization } from "./command-auth.js"; import type { MsgContext } from "./templating.js"; @@ -25,9 +25,7 @@ describe("resolveCommandAuthorization owner allowlist hot path", () => { .filter((entry) => entry.length > 0); }, }; - setActivePluginRegistry( - createTestRegistry([{ pluginId: "discord", plugin, source: "test" }]), - ); + setActivePluginRegistry(createTestRegistry([{ pluginId: "discord", plugin, source: "test" }])); }); afterEach(() => { @@ -81,7 +79,9 @@ describe("resolveCommandAuthorization owner allowlist hot path", () => { plugin.config = { ...plugin.config, formatAllowFrom: ({ cfg, allowFrom }) => - allowFrom.map((entry) => `${(cfg as { testVariant?: string }).testVariant}:${String(entry)}`), + allowFrom.map( + (entry) => `${(cfg as { testVariant?: string }).testVariant}:${String(entry)}`, + ), }; setActivePluginRegistry( createTestRegistry([{ pluginId: "discord", plugin, source: "test" }]),