fix: tighten empty plugin registry reuse

This commit is contained in:
Peter Steinberger
2026-05-25 15:39:03 +01:00
parent e7ad116b9b
commit 026cfb6ba1
4 changed files with 265 additions and 16 deletions

View File

@@ -41,6 +41,62 @@ describe("getLoadedRuntimePluginRegistry", () => {
).toBe(emptyRegistry);
});
it("does not treat disabled plugin records as an empty plugin scope", () => {
const disabledRegistry = createEmptyPluginRegistry();
disabledRegistry.plugins.push({
id: "disabled",
status: "disabled",
} as never);
setActivePluginRegistry(disabledRegistry, "disabled", "default", "/tmp/ws");
expect(
getLoadedRuntimePluginRegistry({
workspaceDir: "/tmp/ws",
requiredPluginIds: [],
}),
).toBeUndefined();
});
it("does not treat diagnostics as loaded plugin records", () => {
const failedRegistry = createEmptyPluginRegistry();
failedRegistry.plugins.push({
id: "failed",
status: "error",
} as never);
failedRegistry.diagnostics.push({
level: "error",
pluginId: "failed",
message: "failed to load",
} as never);
setActivePluginRegistry(failedRegistry, "failed", "default", "/tmp/ws");
expect(
getLoadedRuntimePluginRegistry({
workspaceDir: "/tmp/ws",
requiredPluginIds: ["failed"],
}),
).toBeUndefined();
});
it("does not treat setup-only registrations as loaded plugin records", () => {
const setupRegistry = createEmptyPluginRegistry();
setupRegistry.plugins.push({
id: "setup-only",
status: "disabled",
} as never);
setupRegistry.channelSetups.push({
pluginId: "setup-only",
} as never);
setActivePluginRegistry(setupRegistry, "setup-only", "default", "/tmp/ws");
expect(
getLoadedRuntimePluginRegistry({
workspaceDir: "/tmp/ws",
requiredPluginIds: ["setup-only"],
}),
).toBeUndefined();
});
it("does not reuse workspace-agnostic registries for workspace-specific requests", () => {
setActivePluginRegistry(createRegistryWithPlugin("demo"), "demo");

View File

@@ -22,15 +22,17 @@ function normalizeRequiredPluginIds(ids?: readonly string[]): string[] | undefin
);
}
function registryContainsPluginIds(
export function registryContainsRuntimePluginIds(
registry: PluginRegistry,
pluginIds: readonly string[] | undefined,
): boolean {
if (pluginIds === undefined) {
return true;
}
const present = new Set<string>();
const loaded = new Set<string>();
for (const plugin of registry.plugins ?? []) {
present.add(plugin.id);
if (plugin.status === undefined || plugin.status === "loaded") {
loaded.add(plugin.id);
}
@@ -43,13 +45,13 @@ function registryContainsPluginIds(
if (entry && typeof entry === "object" && "pluginId" in entry) {
const pluginId = entry.pluginId;
if (typeof pluginId === "string" && pluginId.length > 0) {
loaded.add(pluginId);
present.add(pluginId);
}
}
}
}
if (pluginIds.length === 0) {
return loaded.size === 0;
return present.size === 0;
}
return pluginIds.every((pluginId) => loaded.has(pluginId));
}
@@ -83,7 +85,7 @@ export function getLoadedRuntimePluginRegistry(
);
if (surface === "active" && params.loadOptions && requiredPluginIds?.length !== 0) {
const compatible = resolveCompatibleRuntimePluginRegistry(params.loadOptions);
if (!compatible || !registryContainsPluginIds(compatible, requiredPluginIds)) {
if (!compatible || !registryContainsRuntimePluginIds(compatible, requiredPluginIds)) {
return undefined;
}
return compatible;
@@ -98,7 +100,7 @@ export function getLoadedRuntimePluginRegistry(
if (!registry) {
return undefined;
}
if (!registryContainsPluginIds(registry, requiredPluginIds)) {
if (!registryContainsRuntimePluginIds(registry, requiredPluginIds)) {
return undefined;
}
return registry;

View File

@@ -7,6 +7,8 @@ const mocks = vi.hoisted(() => ({
vi.fn<typeof import("../loader.js").resolveCompatibleRuntimePluginRegistry>(),
resolveRuntimePluginRegistry: vi.fn<typeof import("../loader.js").resolveRuntimePluginRegistry>(),
getActivePluginRegistry: vi.fn<typeof import("../runtime.js").getActivePluginRegistry>(),
getActivePluginRegistryWorkspaceDir:
vi.fn<typeof import("../runtime.js").getActivePluginRegistryWorkspaceDir>(),
resolveConfiguredChannelPluginIds:
vi.fn<typeof import("../channel-plugin-ids.js").resolveConfiguredChannelPluginIds>(),
resolveDiscoverableScopedChannelPluginIds:
@@ -76,7 +78,9 @@ vi.mock("../runtime.js", () => ({
getActivePluginHttpRouteRegistry: () => null,
getActivePluginRegistry: (...args: Parameters<typeof mocks.getActivePluginRegistry>) =>
mocks.getActivePluginRegistry(...args),
getActivePluginRegistryWorkspaceDir: () => undefined,
getActivePluginRegistryWorkspaceDir: (
...args: Parameters<typeof mocks.getActivePluginRegistryWorkspaceDir>
) => mocks.getActivePluginRegistryWorkspaceDir(...args),
}));
vi.mock("../channel-plugin-ids.js", () => ({
@@ -119,6 +123,7 @@ describe("ensurePluginRegistryLoaded", () => {
mocks.resolveCompatibleRuntimePluginRegistry.mockReset();
mocks.resolveRuntimePluginRegistry.mockReset();
mocks.getActivePluginRegistry.mockReset();
mocks.getActivePluginRegistryWorkspaceDir.mockReset();
mocks.resolveConfiguredChannelPluginIds.mockReset();
mocks.resolveDiscoverableScopedChannelPluginIds.mockReset();
mocks.resolveChannelPluginIds.mockReset();
@@ -129,6 +134,7 @@ describe("ensurePluginRegistryLoaded", () => {
resetPluginRegistryLoadedForTests();
mocks.getActivePluginRegistry.mockReturnValue(null);
mocks.getActivePluginRegistryWorkspaceDir.mockReturnValue(undefined);
mocks.resolveCompatibleRuntimePluginRegistry.mockReturnValue(undefined);
mocks.loadOpenClawPlugins.mockReturnValue(createEmptyPluginRegistry());
mocks.resolveRuntimePluginRegistry.mockImplementation(
@@ -333,6 +339,33 @@ describe("ensurePluginRegistryLoaded", () => {
expect(load.workspaceDir).toBe("/resolved-workspace");
});
it("does not reuse non-empty all-scope registries without loader compatibility", () => {
mocks.resolveEffectivePluginIds.mockReturnValue(["demo"]);
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { allow: ["demo"] } } as never,
});
const activeRegistry = createEmptyPluginRegistry();
activeRegistry.plugins.push({
id: "demo",
source: "/tmp/demo.js",
origin: "workspace",
enabled: true,
status: "loaded",
} as never);
mocks.getActivePluginRegistry.mockReturnValue(activeRegistry);
mocks.getActivePluginRegistryWorkspaceDir.mockReturnValue("/resolved-workspace");
mocks.loadOpenClawPlugins.mockClear();
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { allow: ["demo"], entries: { demo: { value: "changed" } } } } as never,
});
expect(loadOptions().onlyPluginIds).toEqual(["demo"]);
});
it("preserves empty all-scope loads instead of widening to all discovered plugins", () => {
mocks.resolveEffectivePluginIds.mockReturnValue([]);
@@ -344,6 +377,149 @@ describe("ensurePluginRegistryLoaded", () => {
expect(loadOptions().onlyPluginIds).toEqual([]);
});
it("reuses an active empty registry for repeated empty all-scope loads", () => {
mocks.resolveEffectivePluginIds.mockReturnValue([]);
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
const emptyRegistry = createEmptyPluginRegistry();
mocks.getActivePluginRegistry.mockReturnValue(emptyRegistry);
mocks.getActivePluginRegistryWorkspaceDir.mockReturnValue("/resolved-workspace");
mocks.loadOpenClawPlugins.mockClear();
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
expect(mocks.loadOpenClawPlugins).not.toHaveBeenCalled();
});
it("does not reuse an empty active registry from another workspace", () => {
mocks.resolveEffectivePluginIds.mockReturnValue([]);
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
const emptyRegistry = createEmptyPluginRegistry();
mocks.getActivePluginRegistry.mockReturnValue(emptyRegistry);
mocks.getActivePluginRegistryWorkspaceDir.mockReturnValue("/other-workspace");
mocks.loadOpenClawPlugins.mockClear();
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
expect(loadOptions().onlyPluginIds).toEqual([]);
});
it("does not reuse a non-empty active registry for empty all-scope loads", () => {
mocks.resolveEffectivePluginIds.mockReturnValue([]);
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
const staleRegistry = createEmptyPluginRegistry();
staleRegistry.plugins.push({
id: "stale",
source: "/tmp/stale.js",
origin: "workspace",
enabled: true,
status: "loaded",
} as never);
mocks.getActivePluginRegistry.mockReturnValue(staleRegistry);
mocks.loadOpenClawPlugins.mockClear();
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
expect(loadOptions().onlyPluginIds).toEqual([]);
});
it("does not reuse a disabled-record registry for empty all-scope loads", () => {
mocks.resolveEffectivePluginIds.mockReturnValue([]);
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
const disabledRegistry = createEmptyPluginRegistry();
disabledRegistry.plugins.push({
id: "disabled",
source: "/tmp/disabled.js",
origin: "workspace",
enabled: false,
status: "disabled",
} as never);
mocks.getActivePluginRegistry.mockReturnValue(disabledRegistry);
mocks.getActivePluginRegistryWorkspaceDir.mockReturnValue("/resolved-workspace");
mocks.loadOpenClawPlugins.mockClear();
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
});
expect(loadOptions().onlyPluginIds).toEqual([]);
});
it("does not reuse a failed diagnostic registry for explicit plugin scopes", () => {
const failedRegistry = createEmptyPluginRegistry();
failedRegistry.plugins.push({
id: "failed",
source: "/tmp/failed.js",
origin: "workspace",
enabled: true,
status: "error",
} as never);
failedRegistry.diagnostics.push({
level: "error",
pluginId: "failed",
message: "failed to load",
} as never);
mocks.getActivePluginRegistry.mockReturnValue(failedRegistry);
mocks.getActivePluginRegistryWorkspaceDir.mockReturnValue("/resolved-workspace");
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
onlyPluginIds: ["failed"],
});
expect(loadOptions().onlyPluginIds).toEqual(["failed"]);
});
it("does not reuse a setup-only registry for explicit plugin scopes", () => {
const setupRegistry = createEmptyPluginRegistry();
setupRegistry.plugins.push({
id: "setup-only",
source: "/tmp/setup-only.js",
origin: "workspace",
enabled: false,
status: "disabled",
} as never);
setupRegistry.channelSetups.push({
pluginId: "setup-only",
} as never);
mocks.getActivePluginRegistry.mockReturnValue(setupRegistry);
mocks.getActivePluginRegistryWorkspaceDir.mockReturnValue("/resolved-workspace");
ensurePluginRegistryLoaded({
scope: "all",
config: { plugins: { enabled: true } } as never,
onlyPluginIds: ["setup-only"],
});
expect(loadOptions().onlyPluginIds).toEqual(["setup-only"]);
});
it("reuses a compatible active registry instead of forcing a broad reload", () => {
const activeRegistry = createEmptyPluginRegistry();
activeRegistry.plugins.push({

View File

@@ -1,6 +1,9 @@
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { withActivatedPluginIds } from "../activation-context.js";
import { getLoadedRuntimePluginRegistry } from "../active-runtime-registry.js";
import {
getLoadedRuntimePluginRegistry,
registryContainsRuntimePluginIds,
} from "../active-runtime-registry.js";
import {
resolveChannelPluginIds,
resolveConfiguredChannelPluginIds,
@@ -13,7 +16,7 @@ import {
hasNonEmptyPluginIdScope,
normalizePluginIdScope,
} from "../plugin-scope.js";
import { getActivePluginRegistry } from "../runtime.js";
import { getActivePluginRegistry, getActivePluginRegistryWorkspaceDir } from "../runtime.js";
import {
buildPluginRuntimeLoadOptionsFromValues,
resolvePluginRuntimeLoadContext,
@@ -42,18 +45,17 @@ function activeRegistrySatisfiesScope(
active: ReturnType<typeof getActivePluginRegistry>,
expectedChannelPluginIds: readonly string[],
requestedPluginIds: readonly string[] | undefined,
requestedWorkspaceDir: string | undefined,
): boolean {
if (!active) {
return false;
}
if (requestedPluginIds !== undefined) {
if (requestedPluginIds.length === 0) {
const activeWorkspaceDir = getActivePluginRegistryWorkspaceDir();
if (requestedWorkspaceDir !== undefined && activeWorkspaceDir !== requestedWorkspaceDir) {
return false;
}
const activePluginIds = new Set(
active.plugins.filter((plugin) => plugin.status === "loaded").map((plugin) => plugin.id),
);
return requestedPluginIds.every((pluginId) => activePluginIds.has(pluginId));
return registryContainsRuntimePluginIds(active, requestedPluginIds);
}
const activeChannelPluginIds = new Set(active.channels.map((entry) => entry.plugin.id));
switch (scope) {
@@ -155,17 +157,30 @@ export function ensurePluginRegistryLoaded(options?: {
? (requestedPluginIds ?? [])
: resolveScopePluginIds({ scope, context });
const active = getActivePluginRegistry();
const requestedPluginIdsForScope = scope === "all" ? expectedPluginIds : undefined;
const requestedPluginIdsForScope =
scope === "all" && expectedPluginIds.length === 0 ? expectedPluginIds : undefined;
if (
!scopedLoad &&
scopeRank(pluginRegistryLoaded) >= scopeRank(scope) &&
activeRegistrySatisfiesScope(scope, active, expectedPluginIds, requestedPluginIdsForScope)
activeRegistrySatisfiesScope(
scope,
active,
expectedPluginIds,
requestedPluginIdsForScope,
context.workspaceDir,
)
) {
return;
}
if (
(pluginRegistryLoaded === "none" || scopedLoad) &&
activeRegistrySatisfiesScope(scope, active, expectedPluginIds, requestedPluginIds)
activeRegistrySatisfiesScope(
scope,
active,
expectedPluginIds,
requestedPluginIds,
context.workspaceDir,
)
) {
if (!scopedLoad) {
pluginRegistryLoaded = scope;