From 374076b5a8c535bcb31aa0cbb324f0eee93e0b7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8B=BC=E5=93=A5?= <238804951+luoyanglang@users.noreply.github.com> Date: Wed, 24 Jun 2026 07:22:29 -0700 Subject: [PATCH] fix(plugins): retain plugin tool registry after replacement (#82562) Merged via squash. Prepared head SHA: 1bcbbbfbc1a3cfd44367145afeacf17715fa7adc Co-authored-by: luoyanglang <238804951+luoyanglang@users.noreply.github.com> Co-authored-by: vincentkoc <25068+vincentkoc@users.noreply.github.com> Reviewed-by: @vincentkoc --- src/plugins/runtime/load-context.test.ts | 4 ++ src/plugins/runtime/load-context.ts | 53 +++++++++++++----- .../standalone-runtime-registry-loader.ts | 4 +- src/plugins/tools.optional.test.ts | 56 +++++++++++++++++++ src/plugins/tools.ts | 31 ++++++++-- 5 files changed, 127 insertions(+), 21 deletions(-) diff --git a/src/plugins/runtime/load-context.test.ts b/src/plugins/runtime/load-context.test.ts index 604af3553157..1212d2a3c810 100644 --- a/src/plugins/runtime/load-context.test.ts +++ b/src/plugins/runtime/load-context.test.ts @@ -21,6 +21,7 @@ const metadataSnapshot = { workspaceDir: "/resolved-workspace", }; const loadPluginMetadataSnapshotMock = vi.fn(() => metadataSnapshot); +const isPluginMetadataSnapshotCompatibleMock = vi.fn(() => true); const getCurrentPluginMetadataSnapshotMock = vi.fn(() => undefined); const setCurrentPluginMetadataSnapshotMock = vi.fn(); const clearCurrentPluginMetadataSnapshotMock = vi.fn(); @@ -45,6 +46,7 @@ vi.mock("../../agents/agent-scope.js", () => ({ })); vi.mock("../plugin-metadata-snapshot.js", () => ({ + isPluginMetadataSnapshotCompatible: isPluginMetadataSnapshotCompatibleMock, loadPluginMetadataSnapshot: loadPluginMetadataSnapshotMock, resolvePluginMetadataSnapshot: loadPluginMetadataSnapshotMock, })); @@ -69,6 +71,8 @@ describe("resolvePluginRuntimeLoadContext", () => { applyPluginAutoEnableMock.mockReset(); getCurrentPluginMetadataSnapshotMock.mockReset(); getCurrentPluginMetadataSnapshotMock.mockReturnValue(undefined); + isPluginMetadataSnapshotCompatibleMock.mockReset(); + isPluginMetadataSnapshotCompatibleMock.mockReturnValue(true); loadPluginMetadataSnapshotMock.mockClear(); getCurrentPluginMetadataSnapshotMock.mockClear(); setCurrentPluginMetadataSnapshotMock.mockClear(); diff --git a/src/plugins/runtime/load-context.ts b/src/plugins/runtime/load-context.ts index 382e25a7e8ef..b42168c5c778 100644 --- a/src/plugins/runtime/load-context.ts +++ b/src/plugins/runtime/load-context.ts @@ -14,7 +14,10 @@ import { import { extractPluginInstallRecordsFromInstalledPluginIndex } from "../installed-plugin-index-install-records.js"; import type { PluginLoadOptions } from "../loader.js"; import type { PluginManifestRegistry } from "../manifest-registry.js"; -import { resolvePluginMetadataSnapshot } from "../plugin-metadata-snapshot.js"; +import { + isPluginMetadataSnapshotCompatible, + resolvePluginMetadataSnapshot, +} from "../plugin-metadata-snapshot.js"; import type { PluginLogger } from "../types.js"; const log = createSubsystemLogger("plugins"); @@ -73,18 +76,16 @@ export function resolvePluginRuntimeLoadContext( const rawConfig = options?.config ?? getRuntimeConfig(); const rawWorkspaceDir = options?.workspaceDir ?? resolveAgentWorkspaceDir(rawConfig, resolveDefaultAgentId(rawConfig)); - const metadataSnapshot = options?.manifestRegistry - ? undefined - : resolvePluginMetadataSnapshot({ - config: rawConfig, - env, - workspaceDir: rawWorkspaceDir, - allowWorkspaceScopedCurrent: true, - }); - const manifestRegistry = options?.manifestRegistry ?? metadataSnapshot?.manifestRegistry; - const installRecords = metadataSnapshot - ? extractPluginInstallRecordsFromInstalledPluginIndex(metadataSnapshot.index) - : undefined; + const initialMetadataSnapshot = + options?.manifestRegistry === undefined + ? resolvePluginMetadataSnapshot({ + config: rawConfig, + env, + workspaceDir: rawWorkspaceDir, + allowWorkspaceScopedCurrent: true, + }) + : undefined; + const manifestRegistry = options?.manifestRegistry ?? initialMetadataSnapshot?.manifestRegistry; const activationSourceConfig = resolvePluginActivationSourceConfig({ config: rawConfig, activationSourceConfig: options?.activationSourceConfig, @@ -93,11 +94,33 @@ export function resolvePluginRuntimeLoadContext( config: rawConfig, env, manifestRegistry, - discovery: metadataSnapshot?.discovery, + discovery: initialMetadataSnapshot?.discovery, }); const config = autoEnabled.config; const workspaceDir = options?.workspaceDir ?? resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config)); + const metadataSnapshot = + options?.manifestRegistry !== undefined + ? undefined + : initialMetadataSnapshot && + isPluginMetadataSnapshotCompatible({ + snapshot: initialMetadataSnapshot, + config, + env, + workspaceDir, + }) + ? initialMetadataSnapshot + : resolvePluginMetadataSnapshot({ + config, + env, + workspaceDir, + allowWorkspaceScopedCurrent: true, + ...(initialMetadataSnapshot ? { index: initialMetadataSnapshot.index } : {}), + }); + const finalManifestRegistry = options?.manifestRegistry ?? metadataSnapshot?.manifestRegistry; + const installRecords = metadataSnapshot + ? extractPluginInstallRecordsFromInstalledPluginIndex(metadataSnapshot.index) + : undefined; if (metadataSnapshot) { // Reusable snapshots stay available to later manifest-policy lookups for this runtime load. if (isReusableCurrentPluginMetadataSnapshot(metadataSnapshot)) { @@ -119,7 +142,7 @@ export function resolvePluginRuntimeLoadContext( workspaceDir, env, logger: options?.logger ?? createPluginRuntimeLoaderLogger(), - manifestRegistry, + ...(finalManifestRegistry ? { manifestRegistry: finalManifestRegistry } : {}), installRecords, }; } diff --git a/src/plugins/runtime/standalone-runtime-registry-loader.ts b/src/plugins/runtime/standalone-runtime-registry-loader.ts index 0e1c7d7c5b0a..edf431004c2e 100644 --- a/src/plugins/runtime/standalone-runtime-registry-loader.ts +++ b/src/plugins/runtime/standalone-runtime-registry-loader.ts @@ -27,7 +27,7 @@ function resolveRuntimeSubagentMode( return "default"; } -function installStandaloneRegistry( +function installStandaloneRuntimePluginRegistry( registry: PluginRegistry, params: { loadOptions: PluginLoadOptions; @@ -99,7 +99,7 @@ export function ensureStandaloneRuntimePluginRegistryLoaded(params: { return registry; } - installStandaloneRegistry(registry, { + installStandaloneRuntimePluginRegistry(registry, { loadOptions: params.loadOptions, surface, }); diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index 57490d8b025c..2724f721e22b 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -2147,6 +2147,62 @@ describe("resolvePluginTools optional tools", () => { expect(factory).toHaveBeenCalledTimes(2); }); + it("retains cold-loaded plugin tools for cached descriptor execution after active registry replacement", async () => { + const factory = vi.fn(() => makeTool("cached_lifecycle_tool")); + const gatewayRegistry = setRegistry([ + { + pluginId: "cache-lifecycle-test", + optional: false, + source: "/tmp/cache-lifecycle-test.js", + names: ["cached_lifecycle_tool"], + factory, + }, + ]); + const first = resolvePluginTools( + createResolveToolsParams({ + toolAllowlist: ["cached_lifecycle_tool"], + allowGatewaySubagentBinding: true, + }), + ); + const [tool] = resolvePluginTools( + createResolveToolsParams({ + toolAllowlist: ["cached_lifecycle_tool"], + allowGatewaySubagentBinding: true, + }), + ); + expectResolvedToolNames(first, ["cached_lifecycle_tool"]); + expect(tool?.name).toBe("cached_lifecycle_tool"); + expect(factory).toHaveBeenCalledTimes(1); + + const unrelatedEntry: MockRegistryToolEntry = { + pluginId: "unrelated-live", + optional: false, + source: "/tmp/unrelated-live.js", + names: ["unrelated_live_tool"], + factory: () => makeTool("unrelated_live_tool"), + }; + const replacementRegistry = createToolRegistry([unrelatedEntry]); + replacementRegistry.plugins.push({ id: "cache-lifecycle-test", status: "loaded" }); + setActivePluginRegistry?.(replacementRegistry as never, "provider-runtime", "default", "/tmp"); + resolveRuntimePluginRegistryMock.mockReturnValue(undefined); + loadOpenClawPluginsMock.mockReset(); + loadOpenClawPluginsMock + .mockReturnValueOnce(gatewayRegistry) + .mockReturnValue(createToolRegistry([])); + + await expect(tool?.execute("call-1", {}, undefined)).resolves.toEqual({ + content: [{ type: "text", text: "ok" }], + }); + await expect(tool?.execute("call-2", {}, undefined)).resolves.toEqual({ + content: [{ type: "text", text: "ok" }], + }); + expect(loadOpenClawPluginsMock).toHaveBeenCalledTimes(1); + expect(getActivePluginRegistry?.()).toBe(replacementRegistry); + expect(getActivePluginRegistry?.()?.tools.map((entry) => entry.pluginId)).toContain( + "unrelated-live", + ); + }); + it("does not reuse cached plugin tool descriptors across sandbox context changes", () => { const factory = vi.fn((rawCtx: unknown) => { const ctx = rawCtx as { sandboxed?: boolean }; diff --git a/src/plugins/tools.ts b/src/plugins/tools.ts index 9ebc29576766..b2dcd4a92718 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -31,16 +31,21 @@ import { capturePluginToolDescriptor, createPluginToolDescriptorConfigCacheKeyMemo, readCachedPluginToolDescriptors, + resetPluginToolDescriptorCache as resetCachedPluginToolDescriptors, type CachedPluginToolDescriptor, type PluginToolDescriptorConfigCacheKeyMemo, writeCachedPluginToolDescriptors, } from "./tool-descriptor-cache.js"; import type { OpenClawPluginToolContext } from "./types.js"; -export { - resetPluginToolDescriptorCache, - resetPluginToolDescriptorCache as resetPluginToolFactoryCache, -} from "./tool-descriptor-cache.js"; +let cachedDescriptorRuntimeRegistries = new WeakMap(); + +export function resetPluginToolDescriptorCache(): void { + resetCachedPluginToolDescriptors(); + cachedDescriptorRuntimeRegistries = new WeakMap(); +} + +export { resetPluginToolDescriptorCache as resetPluginToolFactoryCache }; /** MCP bridge metadata attached to plugin tools surfaced through agent tool lists. */ export type PluginToolMcpMeta = { @@ -692,6 +697,10 @@ function createCachedDescriptorPluginTool(params: { const registry = resolvePluginToolRegistry({ loadOptions, onlyPluginIds: [pluginId], + retainedRegistry: cachedDescriptorRuntimeRegistries.get(params.descriptor), + onRetainRegistry: (retainedRegistry) => { + cachedDescriptorRuntimeRegistries.set(params.descriptor, retainedRegistry); + }, }); const candidates = registry?.tools.filter((candidate) => candidate.pluginId === pluginId); if (!candidates || candidates.length === 0) { @@ -899,6 +908,8 @@ function resolveCachedPluginTools(params: { function resolvePluginToolRegistry(params: { loadOptions: PluginLoadOptions; onlyPluginIds?: readonly string[]; + retainedRegistry?: PluginRegistry; + onRetainRegistry?: (registry: PluginRegistry) => void; }) { const lookup = { env: params.loadOptions.env, @@ -924,7 +935,16 @@ function resolvePluginToolRegistry(params: { return activeRegistry; } + if (registryHasScopedPluginTools(params.retainedRegistry, params.onlyPluginIds)) { + return params.retainedRegistry; + } + const forceStandaloneLoad = Boolean(channelRegistry || activeRegistry); + const shouldRetainColdLoadedToolRegistry = + forceStandaloneLoad && + params.loadOptions.activate === false && + params.loadOptions.toolDiscovery === true && + params.onRetainRegistry !== undefined; const standaloneRegistry = ensureStandaloneRuntimePluginRegistryLoaded({ surface: "active", forceLoad: forceStandaloneLoad, @@ -933,6 +953,9 @@ function resolvePluginToolRegistry(params: { loadOptions: params.loadOptions, }); if (registryHasScopedPluginTools(standaloneRegistry, params.onlyPluginIds)) { + if (shouldRetainColdLoadedToolRegistry) { + params.onRetainRegistry?.(standaloneRegistry); + } return standaloneRegistry; } return standaloneRegistry ?? channelRegistry ?? activeRegistry;