Skip to content

Commit 374076b

Browse files
luoyanglangvincentkoc
luoyanglang
and
vincentkoc
authored
fix(plugins): retain plugin tool registry after replacement (#82562)
Merged via squash. Prepared head SHA: 1bcbbbf Co-authored-by: luoyanglang <[email protected]> Co-authored-by: vincentkoc <[email protected]> Reviewed-by: @vincentkoc
1 parent 242fbf1 commit 374076b

5 files changed

Lines changed: 127 additions & 21 deletions

File tree

‎src/plugins/runtime/load-context.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const metadataSnapshot = {
2121
workspaceDir: "/resolved-workspace",
2222
};
2323
const loadPluginMetadataSnapshotMock = vi.fn(() => metadataSnapshot);
24+
const isPluginMetadataSnapshotCompatibleMock = vi.fn(() => true);
2425
const getCurrentPluginMetadataSnapshotMock = vi.fn(() => undefined);
2526
const setCurrentPluginMetadataSnapshotMock = vi.fn();
2627
const clearCurrentPluginMetadataSnapshotMock = vi.fn();
@@ -45,6 +46,7 @@ vi.mock("../../agents/agent-scope.js", () => ({
4546
}));
4647

4748
vi.mock("../plugin-metadata-snapshot.js", () => ({
49+
isPluginMetadataSnapshotCompatible: isPluginMetadataSnapshotCompatibleMock,
4850
loadPluginMetadataSnapshot: loadPluginMetadataSnapshotMock,
4951
resolvePluginMetadataSnapshot: loadPluginMetadataSnapshotMock,
5052
}));
@@ -69,6 +71,8 @@ describe("resolvePluginRuntimeLoadContext", () => {
6971
applyPluginAutoEnableMock.mockReset();
7072
getCurrentPluginMetadataSnapshotMock.mockReset();
7173
getCurrentPluginMetadataSnapshotMock.mockReturnValue(undefined);
74+
isPluginMetadataSnapshotCompatibleMock.mockReset();
75+
isPluginMetadataSnapshotCompatibleMock.mockReturnValue(true);
7276
loadPluginMetadataSnapshotMock.mockClear();
7377
getCurrentPluginMetadataSnapshotMock.mockClear();
7478
setCurrentPluginMetadataSnapshotMock.mockClear();

‎src/plugins/runtime/load-context.ts‎

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import {
1414
import { extractPluginInstallRecordsFromInstalledPluginIndex } from "../installed-plugin-index-install-records.js";
1515
import type { PluginLoadOptions } from "../loader.js";
1616
import type { PluginManifestRegistry } from "../manifest-registry.js";
17-
import { resolvePluginMetadataSnapshot } from "../plugin-metadata-snapshot.js";
17+
import {
18+
isPluginMetadataSnapshotCompatible,
19+
resolvePluginMetadataSnapshot,
20+
} from "../plugin-metadata-snapshot.js";
1821
import type { PluginLogger } from "../types.js";
1922

2023
const log = createSubsystemLogger("plugins");
@@ -73,18 +76,16 @@ export function resolvePluginRuntimeLoadContext(
7376
const rawConfig = options?.config ?? getRuntimeConfig();
7477
const rawWorkspaceDir =
7578
options?.workspaceDir ?? resolveAgentWorkspaceDir(rawConfig, resolveDefaultAgentId(rawConfig));
76-
const metadataSnapshot = options?.manifestRegistry
77-
? undefined
78-
: resolvePluginMetadataSnapshot({
79-
config: rawConfig,
80-
env,
81-
workspaceDir: rawWorkspaceDir,
82-
allowWorkspaceScopedCurrent: true,
83-
});
84-
const manifestRegistry = options?.manifestRegistry ?? metadataSnapshot?.manifestRegistry;
85-
const installRecords = metadataSnapshot
86-
? extractPluginInstallRecordsFromInstalledPluginIndex(metadataSnapshot.index)
87-
: undefined;
79+
const initialMetadataSnapshot =
80+
options?.manifestRegistry === undefined
81+
? resolvePluginMetadataSnapshot({
82+
config: rawConfig,
83+
env,
84+
workspaceDir: rawWorkspaceDir,
85+
allowWorkspaceScopedCurrent: true,
86+
})
87+
: undefined;
88+
const manifestRegistry = options?.manifestRegistry ?? initialMetadataSnapshot?.manifestRegistry;
8889
const activationSourceConfig = resolvePluginActivationSourceConfig({
8990
config: rawConfig,
9091
activationSourceConfig: options?.activationSourceConfig,
@@ -93,11 +94,33 @@ export function resolvePluginRuntimeLoadContext(
9394
config: rawConfig,
9495
env,
9596
manifestRegistry,
96-
discovery: metadataSnapshot?.discovery,
97+
discovery: initialMetadataSnapshot?.discovery,
9798
});
9899
const config = autoEnabled.config;
99100
const workspaceDir =
100101
options?.workspaceDir ?? resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config));
102+
const metadataSnapshot =
103+
options?.manifestRegistry !== undefined
104+
? undefined
105+
: initialMetadataSnapshot &&
106+
isPluginMetadataSnapshotCompatible({
107+
snapshot: initialMetadataSnapshot,
108+
config,
109+
env,
110+
workspaceDir,
111+
})
112+
? initialMetadataSnapshot
113+
: resolvePluginMetadataSnapshot({
114+
config,
115+
env,
116+
workspaceDir,
117+
allowWorkspaceScopedCurrent: true,
118+
...(initialMetadataSnapshot ? { index: initialMetadataSnapshot.index } : {}),
119+
});
120+
const finalManifestRegistry = options?.manifestRegistry ?? metadataSnapshot?.manifestRegistry;
121+
const installRecords = metadataSnapshot
122+
? extractPluginInstallRecordsFromInstalledPluginIndex(metadataSnapshot.index)
123+
: undefined;
101124
if (metadataSnapshot) {
102125
// Reusable snapshots stay available to later manifest-policy lookups for this runtime load.
103126
if (isReusableCurrentPluginMetadataSnapshot(metadataSnapshot)) {
@@ -119,7 +142,7 @@ export function resolvePluginRuntimeLoadContext(
119142
workspaceDir,
120143
env,
121144
logger: options?.logger ?? createPluginRuntimeLoaderLogger(),
122-
manifestRegistry,
145+
...(finalManifestRegistry ? { manifestRegistry: finalManifestRegistry } : {}),
123146
installRecords,
124147
};
125148
}

‎src/plugins/runtime/standalone-runtime-registry-loader.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ function resolveRuntimeSubagentMode(
2727
return "default";
2828
}
2929

30-
function installStandaloneRegistry(
30+
function installStandaloneRuntimePluginRegistry(
3131
registry: PluginRegistry,
3232
params: {
3333
loadOptions: PluginLoadOptions;
@@ -99,7 +99,7 @@ export function ensureStandaloneRuntimePluginRegistryLoaded(params: {
9999
return registry;
100100
}
101101

102-
installStandaloneRegistry(registry, {
102+
installStandaloneRuntimePluginRegistry(registry, {
103103
loadOptions: params.loadOptions,
104104
surface,
105105
});

‎src/plugins/tools.optional.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,6 +2147,62 @@ describe("resolvePluginTools optional tools", () => {
21472147
expect(factory).toHaveBeenCalledTimes(2);
21482148
});
21492149

2150+
it("retains cold-loaded plugin tools for cached descriptor execution after active registry replacement", async () => {
2151+
const factory = vi.fn(() => makeTool("cached_lifecycle_tool"));
2152+
const gatewayRegistry = setRegistry([
2153+
{
2154+
pluginId: "cache-lifecycle-test",
2155+
optional: false,
2156+
source: "/tmp/cache-lifecycle-test.js",
2157+
names: ["cached_lifecycle_tool"],
2158+
factory,
2159+
},
2160+
]);
2161+
const first = resolvePluginTools(
2162+
createResolveToolsParams({
2163+
toolAllowlist: ["cached_lifecycle_tool"],
2164+
allowGatewaySubagentBinding: true,
2165+
}),
2166+
);
2167+
const [tool] = resolvePluginTools(
2168+
createResolveToolsParams({
2169+
toolAllowlist: ["cached_lifecycle_tool"],
2170+
allowGatewaySubagentBinding: true,
2171+
}),
2172+
);
2173+
expectResolvedToolNames(first, ["cached_lifecycle_tool"]);
2174+
expect(tool?.name).toBe("cached_lifecycle_tool");
2175+
expect(factory).toHaveBeenCalledTimes(1);
2176+
2177+
const unrelatedEntry: MockRegistryToolEntry = {
2178+
pluginId: "unrelated-live",
2179+
optional: false,
2180+
source: "/tmp/unrelated-live.js",
2181+
names: ["unrelated_live_tool"],
2182+
factory: () => makeTool("unrelated_live_tool"),
2183+
};
2184+
const replacementRegistry = createToolRegistry([unrelatedEntry]);
2185+
replacementRegistry.plugins.push({ id: "cache-lifecycle-test", status: "loaded" });
2186+
setActivePluginRegistry?.(replacementRegistry as never, "provider-runtime", "default", "/tmp");
2187+
resolveRuntimePluginRegistryMock.mockReturnValue(undefined);
2188+
loadOpenClawPluginsMock.mockReset();
2189+
loadOpenClawPluginsMock
2190+
.mockReturnValueOnce(gatewayRegistry)
2191+
.mockReturnValue(createToolRegistry([]));
2192+
2193+
await expect(tool?.execute("call-1", {}, undefined)).resolves.toEqual({
2194+
content: [{ type: "text", text: "ok" }],
2195+
});
2196+
await expect(tool?.execute("call-2", {}, undefined)).resolves.toEqual({
2197+
content: [{ type: "text", text: "ok" }],
2198+
});
2199+
expect(loadOpenClawPluginsMock).toHaveBeenCalledTimes(1);
2200+
expect(getActivePluginRegistry?.()).toBe(replacementRegistry);
2201+
expect(getActivePluginRegistry?.()?.tools.map((entry) => entry.pluginId)).toContain(
2202+
"unrelated-live",
2203+
);
2204+
});
2205+
21502206
it("does not reuse cached plugin tool descriptors across sandbox context changes", () => {
21512207
const factory = vi.fn((rawCtx: unknown) => {
21522208
const ctx = rawCtx as { sandboxed?: boolean };

‎src/plugins/tools.ts‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,21 @@ import {
3131
capturePluginToolDescriptor,
3232
createPluginToolDescriptorConfigCacheKeyMemo,
3333
readCachedPluginToolDescriptors,
34+
resetPluginToolDescriptorCache as resetCachedPluginToolDescriptors,
3435
type CachedPluginToolDescriptor,
3536
type PluginToolDescriptorConfigCacheKeyMemo,
3637
writeCachedPluginToolDescriptors,
3738
} from "./tool-descriptor-cache.js";
3839
import type { OpenClawPluginToolContext } from "./types.js";
3940

40-
export {
41-
resetPluginToolDescriptorCache,
42-
resetPluginToolDescriptorCache as resetPluginToolFactoryCache,
43-
} from "./tool-descriptor-cache.js";
41+
let cachedDescriptorRuntimeRegistries = new WeakMap<CachedPluginToolDescriptor, PluginRegistry>();
42+
43+
export function resetPluginToolDescriptorCache(): void {
44+
resetCachedPluginToolDescriptors();
45+
cachedDescriptorRuntimeRegistries = new WeakMap();
46+
}
47+
48+
export { resetPluginToolDescriptorCache as resetPluginToolFactoryCache };
4449

4550
/** MCP bridge metadata attached to plugin tools surfaced through agent tool lists. */
4651
export type PluginToolMcpMeta = {
@@ -692,6 +697,10 @@ function createCachedDescriptorPluginTool(params: {
692697
const registry = resolvePluginToolRegistry({
693698
loadOptions,
694699
onlyPluginIds: [pluginId],
700+
retainedRegistry: cachedDescriptorRuntimeRegistries.get(params.descriptor),
701+
onRetainRegistry: (retainedRegistry) => {
702+
cachedDescriptorRuntimeRegistries.set(params.descriptor, retainedRegistry);
703+
},
695704
});
696705
const candidates = registry?.tools.filter((candidate) => candidate.pluginId === pluginId);
697706
if (!candidates || candidates.length === 0) {
@@ -899,6 +908,8 @@ function resolveCachedPluginTools(params: {
899908
function resolvePluginToolRegistry(params: {
900909
loadOptions: PluginLoadOptions;
901910
onlyPluginIds?: readonly string[];
911+
retainedRegistry?: PluginRegistry;
912+
onRetainRegistry?: (registry: PluginRegistry) => void;
902913
}) {
903914
const lookup = {
904915
env: params.loadOptions.env,
@@ -924,7 +935,16 @@ function resolvePluginToolRegistry(params: {
924935
return activeRegistry;
925936
}
926937

938+
if (registryHasScopedPluginTools(params.retainedRegistry, params.onlyPluginIds)) {
939+
return params.retainedRegistry;
940+
}
941+
927942
const forceStandaloneLoad = Boolean(channelRegistry || activeRegistry);
943+
const shouldRetainColdLoadedToolRegistry =
944+
forceStandaloneLoad &&
945+
params.loadOptions.activate === false &&
946+
params.loadOptions.toolDiscovery === true &&
947+
params.onRetainRegistry !== undefined;
928948
const standaloneRegistry = ensureStandaloneRuntimePluginRegistryLoaded({
929949
surface: "active",
930950
forceLoad: forceStandaloneLoad,
@@ -933,6 +953,9 @@ function resolvePluginToolRegistry(params: {
933953
loadOptions: params.loadOptions,
934954
});
935955
if (registryHasScopedPluginTools(standaloneRegistry, params.onlyPluginIds)) {
956+
if (shouldRetainColdLoadedToolRegistry) {
957+
params.onRetainRegistry?.(standaloneRegistry);
958+
}
936959
return standaloneRegistry;
937960
}
938961
return standaloneRegistry ?? channelRegistry ?? activeRegistry;

0 commit comments

Comments
 (0)