Merge b82e05f33c1d2d8e919afa35664dccce7ec32e3e into 598f1826d8b2bc969aace2c6459824737667218c

src/auto-reply/command-auth.owner-allow-from-cache.test.ts

@@ -0,0 +1,165 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+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";
+
+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);
+  });
+
+  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"]);
+  });
+});

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,6 +21,11 @@ export type CommandAuthorization = {
   to?: string;
 };
 
+const ownerAllowFromListCache = new WeakMap<
+  OpenClawConfig,
+  WeakMap<ReadonlyArray<string | number>, Map<string, string[]>>
+>();
+
 function resolveProviderFromContext(ctx: MsgContext, cfg: OpenClawConfig): ChannelId | undefined {
   const explicitMessageChannel =
     normalizeMessageChannel(ctx.Provider) ??
@@ -116,6 +122,12 @@ function resolveOwnerAllowFromList(params: {
   if (!Array.isArray(raw) || raw.length === 0) {
     return [];
   }
+  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[] = [];
   for (const entry of raw) {
     const trimmed = String(entry ?? "").trim();
@@ -139,12 +151,24 @@ function resolveOwnerAllowFromList(params: {
     }
     filtered.push(trimmed);
   }
-  return formatAllowFromList({
+  const formatted = formatAllowFromList({
     plugin: params.plugin,
     cfg: params.cfg,
     accountId: params.accountId,
     allowFrom: filtered,
   });
+  let cachedByConfig = ownerAllowFromListCache.get(params.cfg);
+  if (!cachedByConfig) {
+    cachedByConfig = new WeakMap<ReadonlyArray<string | number>, Map<string, string[]>>();
+    ownerAllowFromListCache.set(params.cfg, cachedByConfig);
+  }
+  let cachedByKey = cachedByConfig.get(raw);
+  if (!cachedByKey) {
+    cachedByKey = new Map<string, string[]>();
+    cachedByConfig.set(raw, cachedByKey);
+  }
+  cachedByKey.set(cacheKey, formatted);
+  return formatted;
 }
 
 /**
@@ -244,16 +268,14 @@ function resolveSenderCandidates(params: {
     pushCandidate(params.from);
   }
 
-  const normalized: string[] = [];
+  const normalized = new Set<string>();
   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 +361,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 +400,8 @@ export function resolveCommandAuthorization(params: {
             : ownerCandidatesForCommands,
     ),
   );
+  const ownerListSet = new Set(ownerList);
+  const ownerCandidatesForCommandsSet = new Set(ownerCandidatesForCommands);
 
   const senderCandidates = resolveSenderCandidates({
     plugin,
@@ -386,11 +413,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 +447,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 {