Skip to content

Commit f02b767

Browse files
vincentkocvincentkoc
committed
fix(plugins): ignore throwing provider policy hooks
1 parent 4d49a76 commit f02b767

4 files changed

Lines changed: 265 additions & 12 deletions

File tree

‎src/plugins/provider-public-artifacts.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ describe("provider public artifacts", () => {
2424
vi.doUnmock("./bundled-dir.js");
2525
vi.doUnmock("./manifest-registry.js");
2626
vi.doUnmock("./public-surface-loader.js");
27+
vi.doUnmock("../logging/subsystem.js");
2728
vi.resetModules();
2829
});
2930

@@ -314,4 +315,61 @@ describe("provider public artifacts", () => {
314315
artifactBasename: "provider-policy-api.js",
315316
});
316317
});
318+
319+
it("ignores throwing bundled provider policy hooks without poisoning callers", async () => {
320+
const warn = vi.fn();
321+
let shouldThrow = true;
322+
const loadBundledPluginPublicArtifactModuleSync = vi.fn(() => ({
323+
normalizeConfig: (ctx: { providerConfig: ModelProviderConfig }) => {
324+
if (shouldThrow) {
325+
throw new Error("fuzzplugin provider policy exploded");
326+
}
327+
return {
328+
...ctx.providerConfig,
329+
baseUrl: "https://recovered.example/v1",
330+
};
331+
},
332+
}));
333+
vi.doMock("../logging/subsystem.js", () => ({
334+
createSubsystemLogger: () => ({ warn }),
335+
}));
336+
vi.doMock("./public-surface-loader.js", () => ({
337+
loadBundledPluginPublicArtifactModuleSync,
338+
}));
339+
340+
const {
341+
consumeBundledProviderPolicyHookFailure,
342+
resolveBundledProviderPolicySurface: resolvePolicySurface,
343+
} = await importFreshModule<typeof import("./provider-public-artifacts.js")>(
344+
import.meta.url,
345+
"./provider-public-artifacts.js?scope=throwing-policy-hook",
346+
);
347+
348+
const providerConfig: ModelProviderConfig = {
349+
baseUrl: "https://api.fuzzplugin.example/v1",
350+
api: "openai-completions",
351+
models: [],
352+
};
353+
const surface = resolvePolicySurface("fuzzplugin");
354+
355+
expect(
356+
surface?.normalizeConfig?.({
357+
provider: "fuzzplugin",
358+
providerConfig,
359+
}),
360+
).toBeUndefined();
361+
expect(warn).toHaveBeenCalledWith(expect.stringContaining("fuzzplugin.normalizeConfig failed"));
362+
expect(warn).toHaveBeenCalledWith(
363+
expect.stringContaining("fuzzplugin provider policy exploded"),
364+
);
365+
366+
shouldThrow = false;
367+
expect(
368+
surface?.normalizeConfig?.({
369+
provider: "fuzzplugin",
370+
providerConfig,
371+
})?.baseUrl,
372+
).toBe("https://recovered.example/v1");
373+
expect(consumeBundledProviderPolicyHookFailure(surface?.normalizeConfig)).toBe(false);
374+
});
317375
});

‎src/plugins/provider-public-artifacts.ts‎

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id";
22
import type { ModelProviderConfig } from "../config/types.js";
33
import type { OpenClawConfig } from "../config/types.openclaw.js";
4+
import { formatErrorMessage } from "../infra/errors.js";
5+
import { createSubsystemLogger } from "../logging/subsystem.js";
46
import { resolveBundledPluginsDir } from "./bundled-dir.js";
57
import { loadPluginManifestRegistry, type PluginManifestRegistry } from "./manifest-registry.js";
68
import type {
@@ -16,6 +18,8 @@ import { loadBundledPluginPublicArtifactModuleSync } from "./public-surface-load
1618

1719
const PROVIDER_POLICY_ARTIFACT_CANDIDATES = ["provider-policy-api.js"] as const;
1820
const providerPolicySurfaceByPluginId = new Map<string, BundledProviderPolicySurface | null>();
21+
const providerPolicyHookFailures = new WeakSet<object>();
22+
const log = createSubsystemLogger("plugins/provider-policy");
1923

2024
export type BundledProviderPolicySurface = {
2125
normalizeConfig?: (ctx: ProviderNormalizeConfigContext) => ModelProviderConfig | null | undefined;
@@ -39,6 +43,81 @@ function hasProviderPolicyHook(
3943
);
4044
}
4145

46+
function wrapProviderPolicyHook<TContext, TResult>(params: {
47+
pluginId: string;
48+
hookName: keyof BundledProviderPolicySurface;
49+
hook: ((ctx: TContext) => TResult) | undefined;
50+
}): ((ctx: TContext) => TResult | undefined) | undefined {
51+
if (!params.hook) {
52+
return undefined;
53+
}
54+
const hook = params.hook;
55+
const wrappedHook = (ctx: TContext): TResult | undefined => {
56+
providerPolicyHookFailures.delete(wrappedHook);
57+
try {
58+
return hook(ctx);
59+
} catch (error) {
60+
providerPolicyHookFailures.add(wrappedHook);
61+
log.warn(
62+
`bundled provider policy hook ${params.pluginId}.${params.hookName} failed; ignoring hook: ${formatErrorMessage(error)}`,
63+
);
64+
return undefined;
65+
}
66+
};
67+
return wrappedHook;
68+
}
69+
70+
export function consumeBundledProviderPolicyHookFailure(hook: object | undefined): boolean {
71+
if (!hook) {
72+
return false;
73+
}
74+
if (!providerPolicyHookFailures.has(hook)) {
75+
return false;
76+
}
77+
providerPolicyHookFailures.delete(hook);
78+
return true;
79+
}
80+
81+
function wrapBundledProviderPolicySurface(params: {
82+
pluginId: string;
83+
surface: BundledProviderPolicySurface;
84+
}): BundledProviderPolicySurface {
85+
const wrapped: BundledProviderPolicySurface = {};
86+
const normalizeConfig = wrapProviderPolicyHook({
87+
pluginId: params.pluginId,
88+
hookName: "normalizeConfig",
89+
hook: params.surface.normalizeConfig,
90+
});
91+
if (normalizeConfig) {
92+
wrapped.normalizeConfig = normalizeConfig;
93+
}
94+
const applyConfigDefaults = wrapProviderPolicyHook({
95+
pluginId: params.pluginId,
96+
hookName: "applyConfigDefaults",
97+
hook: params.surface.applyConfigDefaults,
98+
});
99+
if (applyConfigDefaults) {
100+
wrapped.applyConfigDefaults = applyConfigDefaults;
101+
}
102+
const resolveConfigApiKey = wrapProviderPolicyHook({
103+
pluginId: params.pluginId,
104+
hookName: "resolveConfigApiKey",
105+
hook: params.surface.resolveConfigApiKey,
106+
});
107+
if (resolveConfigApiKey) {
108+
wrapped.resolveConfigApiKey = resolveConfigApiKey;
109+
}
110+
const resolveThinkingProfile = wrapProviderPolicyHook({
111+
pluginId: params.pluginId,
112+
hookName: "resolveThinkingProfile",
113+
hook: params.surface.resolveThinkingProfile,
114+
});
115+
if (resolveThinkingProfile) {
116+
wrapped.resolveThinkingProfile = resolveThinkingProfile;
117+
}
118+
return wrapped;
119+
}
120+
42121
function tryLoadBundledProviderPolicySurface(
43122
pluginId: string,
44123
): BundledProviderPolicySurface | null {
@@ -54,8 +133,12 @@ function tryLoadBundledProviderPolicySurface(
54133
artifactBasename,
55134
});
56135
if (hasProviderPolicyHook(mod)) {
57-
providerPolicySurfaceByPluginId.set(cacheKey, mod);
58-
return mod;
136+
const surface = wrapBundledProviderPolicySurface({
137+
pluginId,
138+
surface: mod,
139+
});
140+
providerPolicySurfaceByPluginId.set(cacheKey, surface);
141+
return surface;
59142
}
60143
} catch (error) {
61144
if (

‎src/plugins/provider-runtime.test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ type ResolveOwningPluginIdsForProvider =
2929
typeof import("./providers.js").resolveOwningPluginIdsForProvider;
3030
type ResolveBundledProviderPolicySurface =
3131
typeof import("./provider-public-artifacts.js").resolveBundledProviderPolicySurface;
32+
type ConsumeBundledProviderPolicyHookFailure =
33+
typeof import("./provider-public-artifacts.js").consumeBundledProviderPolicyHookFailure;
3234

3335
const resolvePluginProvidersMock = vi.fn<ResolvePluginProviders>((_) => [] as ProviderPlugin[]);
3436
const isPluginProvidersLoadInFlightMock = vi.fn<IsPluginProvidersLoadInFlight>((_) => false);
@@ -45,6 +47,9 @@ const resolveOwningPluginIdsForProviderMock = vi.fn<ResolveOwningPluginIdsForPro
4547
const resolveBundledProviderPolicySurfaceMock = vi.fn<ResolveBundledProviderPolicySurface>(
4648
(_) => null,
4749
);
50+
const consumeBundledProviderPolicyHookFailureMock = vi.fn<ConsumeBundledProviderPolicyHookFailure>(
51+
() => false,
52+
);
4853
const providerRuntimeWarnMock = vi.fn();
4954

5055
let augmentModelCatalogWithProviderPlugins: typeof import("./provider-runtime.js").augmentModelCatalogWithProviderPlugins;
@@ -281,6 +286,8 @@ describe("provider-runtime", () => {
281286
beforeAll(async () => {
282287
vi.resetModules();
283288
vi.doMock("./provider-public-artifacts.js", () => ({
289+
consumeBundledProviderPolicyHookFailure: (hook: object | undefined) =>
290+
consumeBundledProviderPolicyHookFailureMock(hook),
284291
resolveBundledProviderPolicySurface: (provider: string) =>
285292
resolveBundledProviderPolicySurfaceMock(provider),
286293
}));
@@ -379,6 +386,8 @@ describe("provider-runtime", () => {
379386
resolveOwningPluginIdsForProviderMock.mockReturnValue(undefined);
380387
resolveBundledProviderPolicySurfaceMock.mockReset();
381388
resolveBundledProviderPolicySurfaceMock.mockReturnValue(null);
389+
consumeBundledProviderPolicyHookFailureMock.mockReset();
390+
consumeBundledProviderPolicyHookFailureMock.mockReturnValue(false);
382391
providerRuntimeWarnMock.mockReset();
383392
});
384393

@@ -1383,6 +1392,91 @@ describe("provider-runtime", () => {
13831392
expect(resolvePluginProvidersMock).not.toHaveBeenCalled();
13841393
});
13851394

1395+
it("falls back to runtime provider hooks after bundled policy hook failures", () => {
1396+
const providerConfig: ModelProviderConfig = {
1397+
baseUrl: "https://api.fuzzplugin.example/v1",
1398+
api: "openai-completions",
1399+
models: [],
1400+
};
1401+
const normalizeConfig = vi.fn(() => undefined);
1402+
const resolveConfigApiKey = vi.fn(() => undefined);
1403+
const resolveThinkingProfile = vi.fn(() => undefined);
1404+
const applyConfigDefaults = vi.fn(() => undefined);
1405+
resolveBundledProviderPolicySurfaceMock.mockReturnValue({
1406+
normalizeConfig,
1407+
resolveConfigApiKey,
1408+
resolveThinkingProfile,
1409+
applyConfigDefaults,
1410+
});
1411+
consumeBundledProviderPolicyHookFailureMock.mockImplementation(
1412+
(hook) =>
1413+
hook === normalizeConfig ||
1414+
hook === resolveConfigApiKey ||
1415+
hook === resolveThinkingProfile ||
1416+
hook === applyConfigDefaults,
1417+
);
1418+
resolvePluginProvidersMock.mockReturnValue([
1419+
{
1420+
id: "fuzzplugin",
1421+
label: "Fuzz Plugin",
1422+
auth: [],
1423+
normalizeConfig: ({ providerConfig: config }) => ({
1424+
...config,
1425+
baseUrl: "https://runtime.example.com/v1",
1426+
}),
1427+
resolveConfigApiKey: () => "runtime-api-key",
1428+
resolveThinkingProfile: () => ({
1429+
levels: [{ id: "low" }],
1430+
defaultLevel: "low",
1431+
}),
1432+
applyConfigDefaults: ({ config }) => ({
1433+
...config,
1434+
pluginDefaultsApplied: true,
1435+
}),
1436+
},
1437+
]);
1438+
1439+
expect(
1440+
normalizeProviderConfigWithPlugin({
1441+
provider: "fuzzplugin",
1442+
context: {
1443+
provider: "fuzzplugin",
1444+
providerConfig,
1445+
},
1446+
})?.baseUrl,
1447+
).toBe("https://runtime.example.com/v1");
1448+
expect(
1449+
resolveProviderConfigApiKeyWithPlugin({
1450+
provider: "fuzzplugin",
1451+
context: {
1452+
provider: "fuzzplugin",
1453+
providerConfig,
1454+
env: {},
1455+
},
1456+
}),
1457+
).toBe("runtime-api-key");
1458+
expect(
1459+
resolveProviderThinkingProfile({
1460+
provider: "fuzzplugin",
1461+
context: {
1462+
provider: "fuzzplugin",
1463+
modelId: "fuzz-model",
1464+
reasoning: true,
1465+
},
1466+
}),
1467+
).toEqual({ levels: [{ id: "low" }], defaultLevel: "low" });
1468+
expect(
1469+
applyProviderConfigDefaultsWithPlugin({
1470+
provider: "fuzzplugin",
1471+
context: {
1472+
provider: "fuzzplugin",
1473+
config: {},
1474+
env: {},
1475+
},
1476+
}),
1477+
).toEqual({ pluginDefaultsApplied: true });
1478+
});
1479+
13861480
it("resolves thinking profiles from bundled policy surface before runtime plugins", () => {
13871481
const resolveThinkingProfile = vi.fn(() => ({
13881482
levels: [{ id: "off" as const }],

‎src/plugins/provider-runtime.ts‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ import {
3232
type ProviderRuntimePluginHandle,
3333
wrapProviderStreamFn,
3434
} from "./provider-hook-runtime.js";
35-
import { resolveBundledProviderPolicySurface } from "./provider-public-artifacts.js";
35+
import {
36+
consumeBundledProviderPolicyHookFailure,
37+
resolveBundledProviderPolicySurface,
38+
} from "./provider-public-artifacts.js";
3639
import type { ProviderRuntimeModel } from "./provider-runtime-model.types.js";
3740
import type { ProviderThinkingProfile } from "./provider-thinking.types.js";
3841
import {
@@ -414,9 +417,12 @@ export function normalizeProviderConfigWithPlugin(params: {
414417
const hasConfigChange = (normalized: ModelProviderConfig) =>
415418
normalized !== params.context.providerConfig;
416419
const bundledSurface = resolveBundledProviderPolicySurface(params.provider);
417-
if (bundledSurface?.normalizeConfig) {
418-
const normalized = bundledSurface.normalizeConfig(params.context);
419-
return normalized && hasConfigChange(normalized) ? normalized : undefined;
420+
const bundledNormalizeConfig = bundledSurface?.normalizeConfig;
421+
if (bundledNormalizeConfig) {
422+
const normalized = bundledNormalizeConfig(params.context);
423+
if (!consumeBundledProviderPolicyHookFailure(bundledNormalizeConfig)) {
424+
return normalized && hasConfigChange(normalized) ? normalized : undefined;
425+
}
420426
}
421427
if (!hasExplicitProviderRuntimePluginActivation(params)) {
422428
return undefined;
@@ -455,8 +461,12 @@ export function resolveProviderConfigApiKeyWithPlugin(params: {
455461
allowRuntimePluginLoad?: boolean;
456462
}): string | undefined {
457463
const bundledSurface = resolveBundledProviderPolicySurface(params.provider);
458-
if (bundledSurface?.resolveConfigApiKey) {
459-
return normalizeOptionalString(bundledSurface.resolveConfigApiKey(params.context));
464+
const bundledResolveConfigApiKey = bundledSurface?.resolveConfigApiKey;
465+
if (bundledResolveConfigApiKey) {
466+
const apiKey = normalizeOptionalString(bundledResolveConfigApiKey(params.context));
467+
if (!consumeBundledProviderPolicyHookFailure(bundledResolveConfigApiKey)) {
468+
return apiKey;
469+
}
460470
}
461471
if (params.allowRuntimePluginLoad === false) {
462472
return undefined;
@@ -766,8 +776,12 @@ export function resolveProviderThinkingProfile(params: {
766776
context: ProviderDefaultThinkingPolicyContext;
767777
}): ProviderThinkingProfile | null | undefined {
768778
const bundledSurface = resolveBundledProviderPolicySurface(params.provider);
769-
if (bundledSurface?.resolveThinkingProfile) {
770-
return bundledSurface.resolveThinkingProfile(params.context) ?? undefined;
779+
const bundledResolveThinkingProfile = bundledSurface?.resolveThinkingProfile;
780+
if (bundledResolveThinkingProfile) {
781+
const profile = bundledResolveThinkingProfile(params.context) ?? undefined;
782+
if (!consumeBundledProviderPolicyHookFailure(bundledResolveThinkingProfile)) {
783+
return profile;
784+
}
771785
}
772786
return resolveProviderRuntimePlugin(params)?.resolveThinkingProfile?.(params.context);
773787
}
@@ -790,8 +804,12 @@ export function applyProviderConfigDefaultsWithPlugin(params: {
790804
context: ProviderApplyConfigDefaultsContext;
791805
}) {
792806
const bundledSurface = resolveBundledProviderPolicySurface(params.provider);
793-
if (bundledSurface?.applyConfigDefaults) {
794-
return bundledSurface.applyConfigDefaults(params.context) ?? undefined;
807+
const bundledApplyConfigDefaults = bundledSurface?.applyConfigDefaults;
808+
if (bundledApplyConfigDefaults) {
809+
const config = bundledApplyConfigDefaults(params.context) ?? undefined;
810+
if (!consumeBundledProviderPolicyHookFailure(bundledApplyConfigDefaults)) {
811+
return config;
812+
}
795813
}
796814
return resolveProviderRuntimePlugin(params)?.applyConfigDefaults?.(params.context) ?? undefined;
797815
}

0 commit comments

Comments
 (0)