fix(plugins): avoid helper reentry loads

This commit is contained in:
Peter Steinberger
2026-04-06 18:19:15 +01:00
parent 348cd6b17a
commit 510fca655a
4 changed files with 118 additions and 12 deletions

View File

@@ -13,6 +13,7 @@ import {
clearPluginLoaderCache,
loadOpenClawPlugins,
PluginLoadReentryError,
resolveRuntimePluginRegistry,
} from "./loader.js";
import {
cleanupPluginLoaderFixturesForTest,
@@ -1254,6 +1255,60 @@ module.exports = { id: "throws-after-import", register() {} };`,
}
},
},
{
label: "lets resolveRuntimePluginRegistry short-circuit during same snapshot load",
run: () => {
useNoBundledPlugins();
const marker = "__openclaw_runtime_registry_reentry_marker";
const resolverMarker = "__openclaw_runtime_registry_reentry_fn";
Reflect.deleteProperty(globalThis, marker);
Reflect.set(
globalThis,
resolverMarker,
(options: Parameters<typeof resolveRuntimePluginRegistry>[0]) =>
resolveRuntimePluginRegistry(options),
);
const pluginDir = makeTempDir();
const pluginFile = path.join(pluginDir, "runtime-registry-reentry.cjs");
const nestedOptions = {
cache: false,
activate: false,
workspaceDir: pluginDir,
config: {
plugins: {
load: { paths: [pluginFile] },
allow: ["runtime-registry-reentry"],
},
},
} satisfies Parameters<typeof loadOpenClawPlugins>[0];
writePlugin({
id: "runtime-registry-reentry",
dir: pluginDir,
filename: "runtime-registry-reentry.cjs",
body: `module.exports = {
id: "runtime-registry-reentry",
register() {
const registry = globalThis.${resolverMarker}(${JSON.stringify(nestedOptions)});
globalThis.${marker} = registry === undefined ? "undefined" : "loaded";
},
};`,
});
const registry = loadOpenClawPlugins(nestedOptions);
try {
expect(Reflect.get(globalThis, marker)).toBe("undefined");
expect(
registry.plugins.find((entry) => entry.id === "runtime-registry-reentry"),
).toMatchObject({
status: "loaded",
});
} finally {
Reflect.deleteProperty(globalThis, marker);
Reflect.deleteProperty(globalThis, resolverMarker);
}
},
},
{
label: "keeps scoped plugin loads in a separate cache entry",
run: () => {

View File

@@ -512,7 +512,17 @@ export function resolveRuntimePluginRegistry(
if (!options || !hasExplicitCompatibilityInputs(options)) {
return getCompatibleActivePluginRegistry();
}
return getCompatibleActivePluginRegistry(options) ?? loadOpenClawPlugins(options);
const compatible = getCompatibleActivePluginRegistry(options);
if (compatible) {
return compatible;
}
// Helper/runtime callers should not recurse into the same snapshot load while
// plugin registration is still in flight. Let direct loadOpenClawPlugins(...)
// callers surface the hard error instead.
if (isPluginRegistryLoadInFlight(options)) {
return undefined;
}
return loadOpenClawPlugins(options);
}
export function resolvePluginRegistryLoadCacheKey(options: PluginLoadOptions = {}): string {

View File

@@ -133,6 +133,33 @@ describe("resolvePluginWebFetchProviders", () => {
expect(loadOpenClawPluginsMock).toHaveBeenCalledTimes(1);
});
it("does not force a fresh snapshot load when the same web-provider load is already in flight", () => {
const inFlightSpy = vi
.spyOn(loaderModule, "isPluginRegistryLoadInFlight")
.mockReturnValue(true);
loadOpenClawPluginsMock.mockImplementation(() => {
throw new Error("resolvePluginWebFetchProviders should not bypass the in-flight guard");
});
const providers = resolvePluginWebFetchProviders({
config: createFirecrawlAllowConfig(),
bundledAllowlistCompat: true,
workspaceDir: DEFAULT_WORKSPACE,
env: createWebFetchEnv(),
});
expect(providers).toEqual([]);
expect(inFlightSpy).toHaveBeenCalledWith(
expect.objectContaining({
activate: false,
cache: false,
onlyPluginIds: ["firecrawl"],
workspaceDir: DEFAULT_WORKSPACE,
}),
);
expect(loadOpenClawPluginsMock).not.toHaveBeenCalled();
});
it("reuses a compatible active registry for snapshot resolution when config is provided", () => {
const env = createWebFetchEnv();
const rawConfig = createFirecrawlAllowConfig();

View File

@@ -6,6 +6,7 @@ import {
shouldUsePluginSnapshotCache,
} from "./cache-controls.js";
import {
isPluginRegistryLoadInFlight,
loadOpenClawPlugins,
resolveCompatibleRuntimePluginRegistry,
resolveRuntimePluginRegistry,
@@ -170,15 +171,10 @@ export function resolvePluginWebProviders<TEntry>(
return cached.providers;
}
}
const loadOptions = resolveWebProviderLoadOptions(params, deps);
const resolved = deps.mapRegistryProviders({
registry:
resolveCompatibleRuntimePluginRegistry(loadOptions) ?? loadOpenClawPlugins(loadOptions),
onlyPluginIds: params.onlyPluginIds,
});
if (cacheOwnerConfig && shouldMemoizeSnapshot) {
const memoizeSnapshot = (providers: TEntry[]) => {
if (!cacheOwnerConfig || !shouldMemoizeSnapshot) {
return;
}
const ttlMs = resolvePluginSnapshotCacheTtlMs(env);
let configCache = deps.snapshotCache.get(cacheOwnerConfig);
if (!configCache) {
@@ -195,10 +191,28 @@ export function resolvePluginWebProviders<TEntry>(
}
envCache.set(cacheKey, {
expiresAt: Date.now() + ttlMs,
providers: resolved,
providers,
});
}
};
const loadOptions = resolveWebProviderLoadOptions(params, deps);
const compatible = resolveCompatibleRuntimePluginRegistry(loadOptions);
if (compatible) {
const resolved = deps.mapRegistryProviders({
registry: compatible,
onlyPluginIds: params.onlyPluginIds,
});
memoizeSnapshot(resolved);
return resolved;
}
if (isPluginRegistryLoadInFlight(loadOptions)) {
return [];
}
const resolved = deps.mapRegistryProviders({
registry: loadOpenClawPlugins(loadOptions),
onlyPluginIds: params.onlyPluginIds,
});
memoizeSnapshot(resolved);
return resolved;
}