fix(plugins): detect reentrant plugin loads

This commit is contained in:
Peter Steinberger
2026-04-06 18:04:48 +01:00
parent f8920e96d0
commit 7d9a6b5572
4 changed files with 710 additions and 635 deletions

View File

@@ -8,7 +8,12 @@ import { withEnv } from "../test-utils/env.js";
import { clearPluginCommands, getPluginCommandSpecs } from "./command-registry-state.js";
import { getGlobalHookRunner, resetGlobalHookRunner } from "./hook-runner-global.js";
import { createHookRunner } from "./hooks.js";
import { __testing, clearPluginLoaderCache, loadOpenClawPlugins } from "./loader.js";
import {
__testing,
clearPluginLoaderCache,
loadOpenClawPlugins,
PluginLoadReentryError,
} from "./loader.js";
import {
cleanupPluginLoaderFixturesForTest,
EMPTY_PLUGIN_SCHEMA,
@@ -1184,6 +1189,71 @@ module.exports = { id: "throws-after-import", register() {} };`,
}
},
},
{
label: "fails loudly when a plugin reenters the same snapshot load during register",
run: () => {
useNoBundledPlugins();
const marker = "__openclaw_loader_reentry_error";
const reenterFnMarker = "__openclaw_loader_reentry_fn";
Reflect.deleteProperty(globalThis, marker);
Reflect.set(
globalThis,
reenterFnMarker,
(options: Parameters<typeof loadOpenClawPlugins>[0]) => loadOpenClawPlugins(options),
);
const pluginDir = makeTempDir();
const pluginFile = path.join(pluginDir, "reentrant-snapshot.cjs");
const nestedOptions = {
cache: false,
activate: false,
workspaceDir: pluginDir,
config: {
plugins: {
load: { paths: [pluginFile] },
allow: ["reentrant-snapshot"],
},
},
} satisfies Parameters<typeof loadOpenClawPlugins>[0];
writePlugin({
id: "reentrant-snapshot",
dir: pluginDir,
filename: "reentrant-snapshot.cjs",
body: `module.exports = {
id: "reentrant-snapshot",
register() {
try {
globalThis.${reenterFnMarker}(${JSON.stringify(nestedOptions)});
} catch (error) {
globalThis.${marker} = {
name: error?.name,
message: String(error?.message ?? error),
};
throw error;
}
},
};`,
});
const registry = loadOpenClawPlugins(nestedOptions);
try {
expect(Reflect.get(globalThis, marker)).toMatchObject({
name: PluginLoadReentryError.name,
message: expect.stringContaining("plugin load reentry detected"),
});
expect(registry.plugins.find((entry) => entry.id === "reentrant-snapshot")).toMatchObject(
{
status: "error",
error: expect.stringContaining("plugin load reentry detected"),
failurePhase: "register",
},
);
} finally {
Reflect.deleteProperty(globalThis, marker);
Reflect.deleteProperty(globalThis, reenterFnMarker);
}
},
},
{
label: "keeps scoped plugin loads in a separate cache entry",
run: () => {

File diff suppressed because it is too large Load Diff

View File

@@ -1,8 +1,8 @@
import { withActivatedPluginIds } from "./activation-context.js";
import { resolveBundledPluginCompatibleActivationInputs } from "./activation-context.js";
import {
isPluginRegistryLoadInFlight,
loadOpenClawPlugins,
resolvePluginRegistryLoadCacheKey,
resolveRuntimePluginRegistry,
type PluginLoadOptions,
} from "./loader.js";
@@ -21,8 +21,6 @@ import {
} from "./runtime/load-context.js";
import type { ProviderPlugin } from "./types.js";
const inFlightProviderPluginLoadKeys = new Set<string>();
function resolvePluginProviderLoadBase(params: {
config?: PluginLoadOptions["config"];
workspaceDir?: string;
@@ -177,9 +175,7 @@ export function isPluginProvidersLoadInFlight(
if (!loadState) {
return false;
}
return inFlightProviderPluginLoadKeys.has(
resolvePluginRegistryLoadCacheKey(loadState.loadOptions),
);
return isPluginRegistryLoadInFlight(loadState.loadOptions);
}
export function resolvePluginProviders(params: {
@@ -203,32 +199,20 @@ export function resolvePluginProviders(params: {
if (!loadState) {
return [];
}
const loadKey = resolvePluginRegistryLoadCacheKey(loadState.loadOptions);
inFlightProviderPluginLoadKeys.add(loadKey);
try {
const registry = loadOpenClawPlugins(loadState.loadOptions);
return registry.providers.map((entry) => ({
...entry.provider,
pluginId: entry.pluginId,
}));
} finally {
inFlightProviderPluginLoadKeys.delete(loadKey);
}
}
const loadState = resolveRuntimeProviderPluginLoadState(params, base);
const loadKey = resolvePluginRegistryLoadCacheKey(loadState.loadOptions);
inFlightProviderPluginLoadKeys.add(loadKey);
try {
const registry = resolveRuntimePluginRegistry(loadState.loadOptions);
if (!registry) {
return [];
}
const registry = loadOpenClawPlugins(loadState.loadOptions);
return registry.providers.map((entry) => ({
...entry.provider,
pluginId: entry.pluginId,
}));
} finally {
inFlightProviderPluginLoadKeys.delete(loadKey);
}
const loadState = resolveRuntimeProviderPluginLoadState(params, base);
const registry = resolveRuntimePluginRegistry(loadState.loadOptions);
if (!registry) {
return [];
}
return registry.providers.map((entry) => ({
...entry.provider,
pluginId: entry.pluginId,
}));
}

View File

@@ -7,8 +7,7 @@ import type { ProviderPlugin } from "./types.js";
type ResolveRuntimePluginRegistry = typeof import("./loader.js").resolveRuntimePluginRegistry;
type LoadOpenClawPlugins = typeof import("./loader.js").loadOpenClawPlugins;
type ResolvePluginRegistryLoadCacheKey =
typeof import("./loader.js").resolvePluginRegistryLoadCacheKey;
type IsPluginRegistryLoadInFlight = typeof import("./loader.js").isPluginRegistryLoadInFlight;
type LoadPluginManifestRegistry =
typeof import("./manifest-registry.js").loadPluginManifestRegistry;
type ApplyPluginAutoEnable = typeof import("../config/plugin-auto-enable.js").applyPluginAutoEnable;
@@ -16,9 +15,7 @@ type SetActivePluginRegistry = typeof import("./runtime.js").setActivePluginRegi
const resolveRuntimePluginRegistryMock = vi.fn<ResolveRuntimePluginRegistry>();
const loadOpenClawPluginsMock = vi.fn<LoadOpenClawPlugins>();
const resolvePluginRegistryLoadCacheKeyMock = vi.fn<ResolvePluginRegistryLoadCacheKey>((options) =>
JSON.stringify(options ?? {}),
);
const isPluginRegistryLoadInFlightMock = vi.fn<IsPluginRegistryLoadInFlight>((_) => false);
const loadPluginManifestRegistryMock = vi.fn<LoadPluginManifestRegistry>();
const applyPluginAutoEnableMock = vi.fn<ApplyPluginAutoEnable>();
@@ -269,8 +266,8 @@ describe("resolvePluginProviders", () => {
vi.doMock("./loader.js", () => ({
loadOpenClawPlugins: (...args: Parameters<LoadOpenClawPlugins>) =>
loadOpenClawPluginsMock(...args),
resolvePluginRegistryLoadCacheKey: (...args: Parameters<ResolvePluginRegistryLoadCacheKey>) =>
resolvePluginRegistryLoadCacheKeyMock(...args),
isPluginRegistryLoadInFlight: (...args: Parameters<IsPluginRegistryLoadInFlight>) =>
isPluginRegistryLoadInFlightMock(...args),
resolveRuntimePluginRegistry: (...args: Parameters<ResolveRuntimePluginRegistry>) =>
resolveRuntimePluginRegistryMock(...args),
}));
@@ -302,10 +299,8 @@ describe("resolvePluginProviders", () => {
setActivePluginRegistry(createEmptyPluginRegistry());
resolveRuntimePluginRegistryMock.mockReset();
loadOpenClawPluginsMock.mockReset();
resolvePluginRegistryLoadCacheKeyMock.mockReset();
resolvePluginRegistryLoadCacheKeyMock.mockImplementation((options) =>
JSON.stringify(options ?? {}),
);
isPluginRegistryLoadInFlightMock.mockReset();
isPluginRegistryLoadInFlightMock.mockReturnValue(false);
const provider: ProviderPlugin = {
id: "demo-provider",
label: "Demo Provider",