fix(plugins): narrow provider hook reentry guard

This commit is contained in:
Peter Steinberger
2026-04-06 17:56:27 +01:00
parent 309154085b
commit 725cbcc362
6 changed files with 283 additions and 78 deletions

View File

@@ -503,6 +503,10 @@ export function resolveRuntimePluginRegistry(
return getCompatibleActivePluginRegistry(options) ?? loadOpenClawPlugins(options);
}
export function resolvePluginRegistryLoadCacheKey(options: PluginLoadOptions = {}): string {
return resolvePluginLoadCacheContext(options).cacheKey;
}
export function resolveCompatibleRuntimePluginRegistry(
options?: PluginLoadOptions,
): PluginRegistry | undefined {

View File

@@ -1,5 +1,6 @@
import type { AgentMessage } from "@mariozechner/pi-agent-core";
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { ModelProviderConfig } from "../config/types.js";
import {
expectAugmentedCodexCatalog,
expectCodexBuiltInSuppression,
@@ -17,10 +18,13 @@ import type {
} from "./types.js";
type ResolvePluginProviders = typeof import("./providers.runtime.js").resolvePluginProviders;
type IsPluginProvidersLoadInFlight =
typeof import("./providers.runtime.js").isPluginProvidersLoadInFlight;
type ResolveCatalogHookProviderPluginIds =
typeof import("./providers.js").resolveCatalogHookProviderPluginIds;
const resolvePluginProvidersMock = vi.fn<ResolvePluginProviders>((_) => [] as ProviderPlugin[]);
const isPluginProvidersLoadInFlightMock = vi.fn<IsPluginProvidersLoadInFlight>((_) => false);
const resolveCatalogHookProviderPluginIdsMock = vi.fn<ResolveCatalogHookProviderPluginIds>(
(_) => [] as string[],
);
@@ -231,6 +235,8 @@ describe("provider-runtime", () => {
}));
vi.doMock("./providers.runtime.js", () => ({
resolvePluginProviders: (params: unknown) => resolvePluginProvidersMock(params as never),
isPluginProvidersLoadInFlight: (params: unknown) =>
isPluginProvidersLoadInFlightMock(params as never),
}));
({
augmentModelCatalogWithProviderPlugins,
@@ -283,6 +289,8 @@ describe("provider-runtime", () => {
resetProviderRuntimeHookCacheForTest();
resolvePluginProvidersMock.mockReset();
resolvePluginProvidersMock.mockReturnValue([]);
isPluginProvidersLoadInFlightMock.mockReset();
isPluginProvidersLoadInFlightMock.mockReturnValue(false);
resolveCatalogHookProviderPluginIdsMock.mockReset();
resolveCatalogHookProviderPluginIdsMock.mockReturnValue([]);
});
@@ -1280,4 +1288,107 @@ describe("provider-runtime", () => {
}),
);
});
it("does not stack-overflow when provider hook resolution reenters the same plugin load", () => {
let providerLoadInFlight = false;
isPluginProvidersLoadInFlightMock.mockImplementation(() => providerLoadInFlight);
resolvePluginProvidersMock.mockImplementation(() => {
providerLoadInFlight = true;
try {
const reentrantResult = normalizeProviderConfigWithPlugin({
provider: "reentrant-provider",
context: {
provider: "reentrant-provider",
providerConfig: {
baseUrl: "https://example.com",
api: "openai-completions",
models: [],
},
},
});
expect(reentrantResult).toBeUndefined();
return [];
} finally {
providerLoadInFlight = false;
}
});
const result = normalizeProviderConfigWithPlugin({
provider: "demo",
context: {
provider: "demo",
providerConfig: { baseUrl: "https://example.com", api: "openai-completions", models: [] },
},
});
expect(result).toBeUndefined();
expect(resolvePluginProvidersMock).toHaveBeenCalledTimes(2);
});
it("keeps cached provider hook results available during a nested provider load", () => {
const cachedNormalizedConfig: ModelProviderConfig = {
baseUrl: "https://cached.example.com",
api: "openai-completions",
models: [],
};
let providerLoadInFlight = false;
isPluginProvidersLoadInFlightMock.mockImplementation(() => providerLoadInFlight);
resolvePluginProvidersMock.mockImplementation((params) => {
const providerRef = params?.providerRefs?.[0];
if (providerRef === "cached-provider") {
return [
{
id: "cached-provider",
label: "Cached Provider",
auth: [],
normalizeConfig: () => cachedNormalizedConfig,
},
];
}
providerLoadInFlight = true;
try {
const reentrantResult = normalizeProviderConfigWithPlugin({
provider: "cached-provider",
context: {
provider: "cached-provider",
providerConfig: {
baseUrl: "https://example.com",
api: "openai-completions",
models: [],
},
},
});
expect(reentrantResult).toBe(cachedNormalizedConfig);
return [];
} finally {
providerLoadInFlight = false;
}
});
expect(
normalizeProviderConfigWithPlugin({
provider: "cached-provider",
context: {
provider: "cached-provider",
providerConfig: { baseUrl: "https://example.com", api: "openai-completions", models: [] },
},
}),
).toBe(cachedNormalizedConfig);
expect(
normalizeProviderConfigWithPlugin({
provider: "outer-provider",
context: {
provider: "outer-provider",
providerConfig: {
baseUrl: "https://outer.example.com",
api: "openai-completions",
models: [],
},
},
}),
).toBeUndefined();
expect(resolvePluginProvidersMock).toHaveBeenCalledTimes(3);
});
});

View File

@@ -4,7 +4,7 @@ import type { ProviderSystemPromptContribution } from "../agents/system-prompt-c
import type { OpenClawConfig } from "../config/config.js";
import type { ModelProviderConfig } from "../config/types.js";
import { resolveCatalogHookProviderPluginIds } from "./providers.js";
import { resolvePluginProviders } from "./providers.runtime.js";
import { isPluginProvidersLoadInFlight, resolvePluginProviders } from "./providers.runtime.js";
import { resolvePluginCacheInputs } from "./roots.js";
import { getActivePluginRegistryWorkspaceDirFromState } from "./runtime-state.js";
import type {
@@ -155,6 +155,19 @@ function resolveProviderPluginsForHooks(params: {
if (cached) {
return cached;
}
if (
isPluginProvidersLoadInFlight({
...params,
workspaceDir,
env,
activate: false,
cache: false,
bundledProviderAllowlistCompat: true,
bundledProviderVitestCompat: true,
})
) {
return [];
}
const resolved = resolvePluginProviders({
...params,
workspaceDir,

View File

@@ -2,6 +2,7 @@ import { withActivatedPluginIds } from "./activation-context.js";
import { resolveBundledPluginCompatibleActivationInputs } from "./activation-context.js";
import {
loadOpenClawPlugins,
resolvePluginRegistryLoadCacheKey,
resolveRuntimePluginRegistry,
type PluginLoadOptions,
} from "./loader.js";
@@ -20,21 +21,16 @@ import {
} from "./runtime/load-context.js";
import type { ProviderPlugin } from "./types.js";
export function resolvePluginProviders(params: {
const inFlightProviderPluginLoadKeys = new Set<string>();
function resolvePluginProviderLoadBase(params: {
config?: PluginLoadOptions["config"];
workspaceDir?: string;
/** Use an explicit env when plugin roots should resolve independently from process.env. */
env?: PluginLoadOptions["env"];
bundledProviderAllowlistCompat?: boolean;
bundledProviderVitestCompat?: boolean;
onlyPluginIds?: string[];
providerRefs?: readonly string[];
modelRefs?: readonly string[];
activate?: boolean;
cache?: boolean;
pluginSdkResolution?: PluginLoadOptions["pluginSdkResolution"];
mode?: "runtime" | "setup";
}): ProviderPlugin[] {
}) {
const env = params.env ?? process.env;
const workspaceDir = params.workspaceDir ?? getActivePluginRegistryWorkspaceDir();
const providerOwnedPluginIds = params.providerRefs?.length
@@ -78,47 +74,58 @@ export function resolvePluginProviders(params: {
config: params.config,
pluginIds: [...providerOwnedPluginIds, ...modelOwnedPluginIds],
});
if (params.mode === "setup") {
const providerPluginIds = resolveDiscoveredProviderPluginIds({
config: runtimeConfig,
workspaceDir,
env,
onlyPluginIds: requestedPluginIds,
});
if (providerPluginIds.length === 0) {
return [];
}
const registry = loadOpenClawPlugins(
buildPluginRuntimeLoadOptionsFromValues(
{
config: withActivatedPluginIds({
config: runtimeConfig,
pluginIds: providerPluginIds,
}),
activationSourceConfig: runtimeConfig,
autoEnabledReasons: {},
workspaceDir,
env,
logger: createPluginRuntimeLoaderLogger(),
},
{
onlyPluginIds: providerPluginIds,
pluginSdkResolution: params.pluginSdkResolution,
cache: params.cache ?? false,
activate: params.activate ?? false,
},
),
);
return registry.providers.map((entry) => ({
...entry.provider,
pluginId: entry.pluginId,
}));
}
const activation = resolveBundledPluginCompatibleActivationInputs({
rawConfig: runtimeConfig,
return {
env,
workspaceDir,
onlyPluginIds: requestedPluginIds,
requestedPluginIds,
runtimeConfig,
};
}
function resolveSetupProviderPluginLoadState(
params: Parameters<typeof resolvePluginProviders>[0],
base: ReturnType<typeof resolvePluginProviderLoadBase>,
) {
const providerPluginIds = resolveDiscoveredProviderPluginIds({
config: base.runtimeConfig,
workspaceDir: base.workspaceDir,
env: base.env,
onlyPluginIds: base.requestedPluginIds,
});
if (providerPluginIds.length === 0) {
return undefined;
}
const loadOptions = buildPluginRuntimeLoadOptionsFromValues(
{
config: withActivatedPluginIds({
config: base.runtimeConfig,
pluginIds: providerPluginIds,
}),
activationSourceConfig: base.runtimeConfig,
autoEnabledReasons: {},
workspaceDir: base.workspaceDir,
env: base.env,
logger: createPluginRuntimeLoaderLogger(),
},
{
onlyPluginIds: providerPluginIds,
pluginSdkResolution: params.pluginSdkResolution,
cache: params.cache ?? false,
activate: params.activate ?? false,
},
);
return { loadOptions };
}
function resolveRuntimeProviderPluginLoadState(
params: Parameters<typeof resolvePluginProviders>[0],
base: ReturnType<typeof resolvePluginProviderLoadBase>,
) {
const activation = resolveBundledPluginCompatibleActivationInputs({
rawConfig: base.runtimeConfig,
env: base.env,
workspaceDir: base.workspaceDir,
onlyPluginIds: base.requestedPluginIds,
applyAutoEnable: true,
compatMode: {
allowlist: params.bundledProviderAllowlistCompat,
@@ -131,39 +138,97 @@ export function resolvePluginProviders(params: {
? withBundledProviderVitestCompat({
config: activation.config,
pluginIds: activation.compatPluginIds,
env,
env: base.env,
})
: activation.config;
const providerPluginIds = resolveEnabledProviderPluginIds({
config,
workspaceDir,
env,
onlyPluginIds: requestedPluginIds,
workspaceDir: base.workspaceDir,
env: base.env,
onlyPluginIds: base.requestedPluginIds,
});
const registry = resolveRuntimePluginRegistry(
buildPluginRuntimeLoadOptionsFromValues(
{
config,
activationSourceConfig: activation.activationSourceConfig,
autoEnabledReasons: activation.autoEnabledReasons,
workspaceDir,
env,
logger: createPluginRuntimeLoaderLogger(),
},
{
onlyPluginIds: providerPluginIds,
pluginSdkResolution: params.pluginSdkResolution,
cache: params.cache ?? false,
activate: params.activate ?? false,
},
),
const loadOptions = buildPluginRuntimeLoadOptionsFromValues(
{
config,
activationSourceConfig: activation.activationSourceConfig,
autoEnabledReasons: activation.autoEnabledReasons,
workspaceDir: base.workspaceDir,
env: base.env,
logger: createPluginRuntimeLoaderLogger(),
},
{
onlyPluginIds: providerPluginIds,
pluginSdkResolution: params.pluginSdkResolution,
cache: params.cache ?? false,
activate: params.activate ?? false,
},
);
if (!registry) {
return [];
}
return registry.providers.map((entry) => ({
...entry.provider,
pluginId: entry.pluginId,
}));
return { loadOptions };
}
export function isPluginProvidersLoadInFlight(
params: Parameters<typeof resolvePluginProviders>[0],
): boolean {
const base = resolvePluginProviderLoadBase(params);
const loadState =
params.mode === "setup"
? resolveSetupProviderPluginLoadState(params, base)
: resolveRuntimeProviderPluginLoadState(params, base);
if (!loadState) {
return false;
}
return inFlightProviderPluginLoadKeys.has(
resolvePluginRegistryLoadCacheKey(loadState.loadOptions),
);
}
export function resolvePluginProviders(params: {
config?: PluginLoadOptions["config"];
workspaceDir?: string;
/** Use an explicit env when plugin roots should resolve independently from process.env. */
env?: PluginLoadOptions["env"];
bundledProviderAllowlistCompat?: boolean;
bundledProviderVitestCompat?: boolean;
onlyPluginIds?: string[];
providerRefs?: readonly string[];
modelRefs?: readonly string[];
activate?: boolean;
cache?: boolean;
pluginSdkResolution?: PluginLoadOptions["pluginSdkResolution"];
mode?: "runtime" | "setup";
}): ProviderPlugin[] {
const base = resolvePluginProviderLoadBase(params);
if (params.mode === "setup") {
const loadState = resolveSetupProviderPluginLoadState(params, base);
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 [];
}
return registry.providers.map((entry) => ({
...entry.provider,
pluginId: entry.pluginId,
}));
} finally {
inFlightProviderPluginLoadKeys.delete(loadKey);
}
}

View File

@@ -7,6 +7,8 @@ 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 LoadPluginManifestRegistry =
typeof import("./manifest-registry.js").loadPluginManifestRegistry;
type ApplyPluginAutoEnable = typeof import("../config/plugin-auto-enable.js").applyPluginAutoEnable;
@@ -14,6 +16,9 @@ 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 loadPluginManifestRegistryMock = vi.fn<LoadPluginManifestRegistry>();
const applyPluginAutoEnableMock = vi.fn<ApplyPluginAutoEnable>();
@@ -264,6 +269,8 @@ describe("resolvePluginProviders", () => {
vi.doMock("./loader.js", () => ({
loadOpenClawPlugins: (...args: Parameters<LoadOpenClawPlugins>) =>
loadOpenClawPluginsMock(...args),
resolvePluginRegistryLoadCacheKey: (...args: Parameters<ResolvePluginRegistryLoadCacheKey>) =>
resolvePluginRegistryLoadCacheKeyMock(...args),
resolveRuntimePluginRegistry: (...args: Parameters<ResolveRuntimePluginRegistry>) =>
resolveRuntimePluginRegistryMock(...args),
}));
@@ -295,6 +302,10 @@ describe("resolvePluginProviders", () => {
setActivePluginRegistry(createEmptyPluginRegistry());
resolveRuntimePluginRegistryMock.mockReset();
loadOpenClawPluginsMock.mockReset();
resolvePluginRegistryLoadCacheKeyMock.mockReset();
resolvePluginRegistryLoadCacheKeyMock.mockImplementation((options) =>
JSON.stringify(options ?? {}),
);
const provider: ProviderPlugin = {
id: "demo-provider",
label: "Demo Provider",