fix: include manifestRegistry in cache key

kagura-agent · kagura-agent · commit dc6761455413 · 2026-05-19T17:30:03.000+08:00
Address ClawSweeper P2 review finding: the WeakMap cache was keyed
only on (config, env), ignoring manifestRegistry. Several gateway
callers pass explicit registries, so the cache could return stale
results when called with the same config/env but different registries.

Cache is now a three-level structure: config → env → registry → result.
Calls without a manifestRegistry use undefined as the Map key,
ensuring they are cached separately from calls with an explicit registry.

Added two new test cases:
- recomputes when manifestRegistry reference changes
- caches separately for calls with and without manifestRegistry
diff --git a/src/config/plugin-auto-enable.apply.test.ts b/src/config/plugin-auto-enable.apply.test.ts
@@ -1,8 +1,12 @@
 import { describe, expect, it, vi } from "vitest";
+import { afterEach } from "vitest";
+import type { PluginManifestRegistry } from "../plugins/manifest-registry.js";
 import { applyPluginAutoEnable } from "./plugin-auto-enable.js";
-import { makeIsolatedEnv, resetPluginAutoEnableTestState } from "./plugin-auto-enable.test-helpers.js";
+import {
+  makeIsolatedEnv,
+  resetPluginAutoEnableTestState,
+} from "./plugin-auto-enable.test-helpers.js";
 import type { OpenClawConfig } from "./types.openclaw.js";
-import { afterEach } from "vitest";
 
 vi.mock("../channels/plugins/configured-state.js", async (importOriginal) => {
   const actual = await importOriginal<typeof import("../channels/plugins/configured-state.js")>();
@@ -52,6 +56,31 @@ describe("applyPluginAutoEnable caching", () => {
     expect(result1).not.toBe(result2);
   });
 
+  it("recomputes when manifestRegistry reference changes", () => {
+    const config: OpenClawConfig = {};
+    const env = makeIsolatedEnv();
+    const registry1: PluginManifestRegistry = { plugins: [], diagnostics: [] };
+    const registry2: PluginManifestRegistry = { plugins: [], diagnostics: [] };
+    const result1 = applyPluginAutoEnable({ config, env, manifestRegistry: registry1 });
+    const result2 = applyPluginAutoEnable({ config, env, manifestRegistry: registry2 });
+    // Different registry references should produce separate cache entries
+    expect(result1).not.toBe(result2);
+    expect(result1).toEqual(result2);
+  });
+
+  it("caches separately for calls with and without manifestRegistry", () => {
+    const config: OpenClawConfig = {};
+    const env = makeIsolatedEnv();
+    const registry: PluginManifestRegistry = { plugins: [], diagnostics: [] };
+    const withoutRegistry = applyPluginAutoEnable({ config, env });
+    const withRegistry = applyPluginAutoEnable({ config, env, manifestRegistry: registry });
+    // Should be separate cache entries
+    expect(withoutRegistry).not.toBe(withRegistry);
+    // But same config/env/registry should hit cache
+    const withRegistryAgain = applyPluginAutoEnable({ config, env, manifestRegistry: registry });
+    expect(withRegistry).toBe(withRegistryAgain);
+  });
+
   it("cached calls are faster than uncached calls", () => {
     const env = makeIsolatedEnv();
     // First call with config1 — uncached
diff --git a/src/config/plugin-auto-enable.apply.ts b/src/config/plugin-auto-enable.apply.ts
@@ -40,7 +40,10 @@ export function materializePluginAutoEnableCandidates(params: {
   });
 }
 
-const autoEnableCache = new WeakMap<object, WeakMap<object, PluginAutoEnableResult>>();
+const autoEnableCache = new WeakMap<
+  object,
+  WeakMap<object, Map<PluginManifestRegistry | undefined, PluginAutoEnableResult>>
+>();
 
 export function applyPluginAutoEnable(params: {
   config?: OpenClawConfig;
@@ -50,19 +53,28 @@ export function applyPluginAutoEnable(params: {
   const config = params.config;
   const env = params.env;
   if (config && env) {
-    let inner = autoEnableCache.get(config);
-    if (inner) {
-      const hit = inner.get(env);
-      if (hit) {
-        return hit;
+    const registryKey = params.manifestRegistry;
+    let envMap = autoEnableCache.get(config);
+    if (envMap) {
+      const registryMap = envMap.get(env);
+      if (registryMap) {
+        const hit = registryMap.get(registryKey);
+        if (hit) {
+          return hit;
+        }
       }
     }
     const result = computeAutoEnable(params);
-    if (!inner) {
-      inner = new WeakMap();
-      autoEnableCache.set(config, inner);
+    if (!envMap) {
+      envMap = new WeakMap();
+      autoEnableCache.set(config, envMap);
     }
-    inner.set(env, result);
+    let registryMap = envMap.get(env);
+    if (!registryMap) {
+      registryMap = new Map();
+      envMap.set(env, registryMap);
+    }
+    registryMap.set(registryKey, result);
     return result;
   }
   return computeAutoEnable(params);
