From 647fc7bfecec3ab0a9663accaf8142b9d73a90b5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 5 Apr 2026 09:32:56 +0100 Subject: [PATCH] refactor(plugins): unify explicit provider ownership loading --- src/commands/models/auth.test.ts | 19 ++------ src/plugins/provider-runtime.test.ts | 53 +++------------------- src/plugins/provider-runtime.ts | 27 ++++------- src/plugins/providers.runtime.ts | 68 ++++++++++++++++++++++++++-- src/plugins/providers.test.ts | 23 ++++++++++ 5 files changed, 106 insertions(+), 84 deletions(-) diff --git a/src/commands/models/auth.test.ts b/src/commands/models/auth.test.ts index 55b46566470..0a3fde3133e 100644 --- a/src/commands/models/auth.test.ts +++ b/src/commands/models/auth.test.ts @@ -14,7 +14,6 @@ const mocks = vi.hoisted(() => ({ resolveAgentWorkspaceDir: vi.fn(), resolveDefaultAgentWorkspaceDir: vi.fn(), upsertAuthProfile: vi.fn(), - resolveOwningPluginIdsForProvider: vi.fn(), resolvePluginProviders: vi.fn(), createClackPrompter: vi.fn(), loadValidConfigOrThrow: vi.fn(), @@ -55,10 +54,6 @@ vi.mock("../../plugins/providers.runtime.js", () => ({ resolvePluginProviders: mocks.resolvePluginProviders, })); -vi.mock("../../plugins/providers.js", () => ({ - resolveOwningPluginIdsForProvider: mocks.resolveOwningPluginIdsForProvider, -})); - vi.mock("../../wizard/clack-prompter.js", () => ({ createClackPrompter: mocks.createClackPrompter, })); @@ -153,7 +148,6 @@ describe("modelsAuthLoginCommand", () => { mocks.resolveAgentWorkspaceDir.mockReturnValue("/tmp/openclaw/workspace"); mocks.resolveDefaultAgentWorkspaceDir.mockReturnValue("/tmp/openclaw/workspace"); mocks.loadValidConfigOrThrow.mockImplementation(async () => currentConfig); - mocks.resolveOwningPluginIdsForProvider.mockReturnValue(undefined); mocks.updateConfig.mockImplementation( async (mutator: (cfg: OpenClawConfig) => OpenClawConfig) => { lastUpdatedConfig = mutator(currentConfig); @@ -299,10 +293,9 @@ describe("modelsAuthLoginCommand", () => { }, }, }); - mocks.resolveOwningPluginIdsForProvider.mockReturnValue(["anthropic"]); mocks.resolvePluginProviders.mockImplementation( - (params: { activate?: boolean; onlyPluginIds?: string[] } | undefined) => - params?.activate === true && params?.onlyPluginIds?.[0] === "anthropic" + (params: { activate?: boolean; providerRefs?: string[] } | undefined) => + params?.activate === true && params?.providerRefs?.[0] === "anthropic" ? [ { id: "anthropic", @@ -325,19 +318,13 @@ describe("modelsAuthLoginCommand", () => { runtime, ); - expect(mocks.resolveOwningPluginIdsForProvider).toHaveBeenCalledWith({ - provider: "anthropic", - config: {}, - workspaceDir: "/tmp/openclaw/workspace", - env: process.env, - }); expect(mocks.resolvePluginProviders).toHaveBeenCalledWith( expect.objectContaining({ config: {}, workspaceDir: "/tmp/openclaw/workspace", bundledProviderAllowlistCompat: true, bundledProviderVitestCompat: true, - onlyPluginIds: ["anthropic"], + providerRefs: ["anthropic"], activate: true, }), ); diff --git a/src/plugins/provider-runtime.test.ts b/src/plugins/provider-runtime.test.ts index 91f94e6eba6..988d6668768 100644 --- a/src/plugins/provider-runtime.test.ts +++ b/src/plugins/provider-runtime.test.ts @@ -18,16 +18,11 @@ import type { type ResolvePluginProviders = typeof import("./providers.runtime.js").resolvePluginProviders; type ResolveCatalogHookProviderPluginIds = typeof import("./providers.js").resolveCatalogHookProviderPluginIds; -type ResolveOwningPluginIdsForProvider = - typeof import("./providers.js").resolveOwningPluginIdsForProvider; const resolvePluginProvidersMock = vi.fn((_) => [] as ProviderPlugin[]); const resolveCatalogHookProviderPluginIdsMock = vi.fn( (_) => [] as string[], ); -const resolveOwningPluginIdsForProviderMock = vi.fn( - (_) => undefined as string[] | undefined, -); let augmentModelCatalogWithProviderPlugins: typeof import("./provider-runtime.js").augmentModelCatalogWithProviderPlugins; let buildProviderAuthDoctorHintWithPlugin: typeof import("./provider-runtime.js").buildProviderAuthDoctorHintWithPlugin; @@ -141,30 +136,17 @@ function createOpenAiCatalogProviderPlugin( }; } -function expectProviderRuntimePluginLoad(params: { - provider: string; - expectedPluginId?: string; - expectedOnlyPluginIds?: string[]; -}) { +function expectProviderRuntimePluginLoad(params: { provider: string; expectedPluginId?: string }) { const plugin = resolveProviderRuntimePlugin({ provider: params.provider }); expect(plugin?.id).toBe(params.expectedPluginId); - expect(resolveOwningPluginIdsForProviderMock).toHaveBeenCalledWith( + expect(resolvePluginProvidersMock).toHaveBeenCalledWith( expect.objectContaining({ - provider: params.provider, + providerRefs: [params.provider], + bundledProviderAllowlistCompat: true, + bundledProviderVitestCompat: true, }), ); - if (params.expectedOnlyPluginIds) { - expect(resolvePluginProvidersMock).toHaveBeenCalledWith( - expect.objectContaining({ - onlyPluginIds: params.expectedOnlyPluginIds, - bundledProviderAllowlistCompat: true, - bundledProviderVitestCompat: true, - }), - ); - } else { - expect(resolvePluginProvidersMock).not.toHaveBeenCalled(); - } } function createDemoRuntimeContext>( @@ -244,8 +226,6 @@ describe("provider-runtime", () => { vi.doMock("./providers.js", () => ({ resolveCatalogHookProviderPluginIds: (params: unknown) => resolveCatalogHookProviderPluginIdsMock(params as never), - resolveOwningPluginIdsForProvider: (params: unknown) => - resolveOwningPluginIdsForProviderMock(params as never), })); vi.doMock("./providers.runtime.js", () => ({ resolvePluginProviders: (params: unknown) => resolvePluginProvidersMock(params as never), @@ -302,12 +282,9 @@ describe("provider-runtime", () => { resolvePluginProvidersMock.mockReturnValue([]); resolveCatalogHookProviderPluginIdsMock.mockReset(); resolveCatalogHookProviderPluginIdsMock.mockReturnValue([]); - resolveOwningPluginIdsForProviderMock.mockReset(); - resolveOwningPluginIdsForProviderMock.mockReturnValue(undefined); }); it("matches providers by alias for runtime hook lookup", () => { - resolveOwningPluginIdsForProviderMock.mockReturnValue(["openrouter"]); resolvePluginProvidersMock.mockReturnValue([ { id: "openrouter", @@ -320,11 +297,10 @@ describe("provider-runtime", () => { expectProviderRuntimePluginLoad({ provider: "Open Router", expectedPluginId: "openrouter", - expectedOnlyPluginIds: ["openrouter"], }); }); - it("skips plugin loading when the provider has no owning plugin", () => { + it("returns no runtime plugin when the provider has no owning plugin", () => { expectProviderRuntimePluginLoad({ provider: "anthropic", }); @@ -350,11 +326,6 @@ describe("provider-runtime", () => { }, }), ).toBe("gemini-3.1-flash-lite-preview"); - expect(resolveOwningPluginIdsForProviderMock).toHaveBeenCalledWith( - expect.objectContaining({ - provider: "google-vertex", - }), - ); expect(resolvePluginProvidersMock).toHaveBeenCalledTimes(1); }); @@ -390,7 +361,6 @@ describe("provider-runtime", () => { }); it("resolves provider config defaults through owner plugins", () => { - resolveOwningPluginIdsForProviderMock.mockReturnValue(["anthropic"]); resolvePluginProvidersMock.mockReturnValue([ { id: "anthropic", @@ -523,7 +493,6 @@ describe("provider-runtime", () => { }, }, } as { plugins: { entries: { demo: { enabled: boolean } } } }; - resolveOwningPluginIdsForProviderMock.mockReturnValue(["demo"]); resolvePluginProvidersMock.mockImplementation((params) => { const runtimeConfig = params?.config as typeof config | undefined; const enabled = runtimeConfig?.plugins?.entries?.demo?.enabled === true; @@ -560,15 +529,6 @@ describe("provider-runtime", () => { it("dispatches runtime hooks for the matched provider", async () => { resolveCatalogHookProviderPluginIdsMock.mockReturnValue(["openai"]); - resolveOwningPluginIdsForProviderMock.mockImplementation((params) => { - if (params.provider === "demo") { - return ["demo"]; - } - if (params.provider === "openai") { - return ["openai"]; - } - return undefined; - }); const prepareDynamicModel = vi.fn(async () => undefined); const createStreamFn = vi.fn(() => vi.fn()); const createEmbeddingProvider = vi.fn(async () => ({ @@ -1126,7 +1086,6 @@ describe("provider-runtime", () => { }); it("merges compat contributions from owner and foreign provider plugins", () => { - resolveOwningPluginIdsForProviderMock.mockReturnValue(["openrouter"]); resolvePluginProvidersMock.mockImplementation((params) => { const onlyPluginIds = params.onlyPluginIds ?? []; const plugins: ProviderPlugin[] = [ diff --git a/src/plugins/provider-runtime.ts b/src/plugins/provider-runtime.ts index 64404b3e57e..22aae91a0d3 100644 --- a/src/plugins/provider-runtime.ts +++ b/src/plugins/provider-runtime.ts @@ -2,10 +2,7 @@ import type { AuthProfileCredential, OAuthCredential } from "../agents/auth-prof import { normalizeProviderId } from "../agents/provider-id.js"; import type { OpenClawConfig } from "../config/config.js"; import type { ModelProviderConfig } from "../config/types.js"; -import { - resolveCatalogHookProviderPluginIds, - resolveOwningPluginIdsForProvider, -} from "./providers.js"; +import { resolveCatalogHookProviderPluginIds } from "./providers.js"; import { resolvePluginProviders } from "./providers.runtime.js"; import { resolvePluginCacheInputs } from "./roots.js"; import { getActivePluginRegistryWorkspaceDirFromState } from "./runtime-state.js"; @@ -104,13 +101,14 @@ function buildHookProviderCacheKey(params: { config?: OpenClawConfig; workspaceDir?: string; onlyPluginIds?: string[]; + providerRefs?: string[]; env?: NodeJS.ProcessEnv; }) { const { roots } = resolvePluginCacheInputs({ workspaceDir: params.workspaceDir, env: params.env, }); - return `${roots.workspace ?? ""}::${roots.global}::${roots.stock ?? ""}::${JSON.stringify(params.config ?? null)}::${JSON.stringify(params.onlyPluginIds ?? [])}`; + return `${roots.workspace ?? ""}::${roots.global}::${roots.stock ?? ""}::${JSON.stringify(params.config ?? null)}::${JSON.stringify(params.onlyPluginIds ?? [])}::${JSON.stringify(params.providerRefs ?? [])}`; } export function clearProviderRuntimeHookCache(): void { @@ -133,6 +131,7 @@ function resolveProviderPluginsForHooks(params: { workspaceDir?: string; env?: NodeJS.ProcessEnv; onlyPluginIds?: string[]; + providerRefs?: string[]; }): ProviderPlugin[] { const env = params.env ?? process.env; const workspaceDir = params.workspaceDir ?? getActivePluginRegistryWorkspaceDirFromState(); @@ -144,6 +143,7 @@ function resolveProviderPluginsForHooks(params: { config: params.config, workspaceDir, onlyPluginIds: params.onlyPluginIds, + providerRefs: params.providerRefs, env, }); const cached = cacheBucket.get(cacheKey); @@ -190,20 +190,11 @@ export function resolveProviderRuntimePlugin(params: { workspaceDir?: string; env?: NodeJS.ProcessEnv; }): ProviderPlugin | undefined { - const workspaceDir = params.workspaceDir ?? getActivePluginRegistryWorkspaceDirFromState(); - const owningPluginIds = resolveOwningPluginIdsForProvider({ - provider: params.provider, - config: params.config, - workspaceDir, - env: params.env, - }); - if (!owningPluginIds || owningPluginIds.length === 0) { - return undefined; - } return resolveProviderPluginsForHooks({ - ...params, - workspaceDir, - onlyPluginIds: owningPluginIds, + config: params.config, + workspaceDir: params.workspaceDir ?? getActivePluginRegistryWorkspaceDirFromState(), + env: params.env, + providerRefs: [params.provider], }).find((plugin) => matchesProviderId(plugin, params.provider)); } diff --git a/src/plugins/providers.runtime.ts b/src/plugins/providers.runtime.ts index 4f15df071d5..c4daa3ba1f9 100644 --- a/src/plugins/providers.runtime.ts +++ b/src/plugins/providers.runtime.ts @@ -11,6 +11,7 @@ import { resolveDiscoveredProviderPluginIds, resolveEnabledProviderPluginIds, resolveBundledProviderCompatPluginIds, + resolveOwningPluginIdsForProvider, resolveOwningPluginIdsForModelRefs, withBundledProviderVitestCompat, } from "./providers.js"; @@ -19,6 +20,41 @@ import type { ProviderPlugin } from "./types.js"; const log = createSubsystemLogger("plugins"); +function withRuntimeActivatedPluginIds(params: { + config?: PluginLoadOptions["config"]; + pluginIds: readonly string[]; +}): PluginLoadOptions["config"] { + if (params.pluginIds.length === 0) { + return params.config; + } + const allow = new Set(params.config?.plugins?.allow ?? []); + const entries = { + ...params.config?.plugins?.entries, + }; + for (const pluginId of [...params.pluginIds].toSorted((left, right) => + left.localeCompare(right), + )) { + const normalized = pluginId.trim(); + if (!normalized) { + continue; + } + allow.add(normalized); + entries[normalized] = { + ...entries[normalized], + enabled: true, + }; + } + return { + ...params.config, + plugins: { + ...params.config?.plugins, + ...(allow.size > 0 + ? { allow: [...allow].toSorted((left, right) => left.localeCompare(right)) } + : {}), + entries, + }, + }; +} export function resolvePluginProviders(params: { config?: PluginLoadOptions["config"]; workspaceDir?: string; @@ -27,6 +63,7 @@ export function resolvePluginProviders(params: { bundledProviderAllowlistCompat?: boolean; bundledProviderVitestCompat?: boolean; onlyPluginIds?: string[]; + providerRefs?: readonly string[]; modelRefs?: readonly string[]; activate?: boolean; cache?: boolean; @@ -35,6 +72,21 @@ export function resolvePluginProviders(params: { }): ProviderPlugin[] { const env = params.env ?? process.env; const workspaceDir = params.workspaceDir ?? getActivePluginRegistryWorkspaceDir(); + const providerOwnedPluginIds = params.providerRefs?.length + ? [ + ...new Set( + params.providerRefs.flatMap( + (provider) => + resolveOwningPluginIdsForProvider({ + provider, + config: params.config, + workspaceDir, + env, + }) ?? [], + ), + ), + ] + : []; const modelOwnedPluginIds = params.modelRefs?.length ? resolveOwningPluginIdsForModelRefs({ models: params.modelRefs, @@ -44,12 +96,22 @@ export function resolvePluginProviders(params: { }) : []; const requestedPluginIds = - params.onlyPluginIds || modelOwnedPluginIds.length > 0 - ? [...new Set([...(params.onlyPluginIds ?? []), ...modelOwnedPluginIds])] + params.onlyPluginIds || + params.providerRefs?.length || + params.modelRefs?.length || + providerOwnedPluginIds.length > 0 || + modelOwnedPluginIds.length > 0 + ? [ + ...new Set([ + ...(params.onlyPluginIds ?? []), + ...providerOwnedPluginIds, + ...modelOwnedPluginIds, + ]), + ].toSorted((left, right) => left.localeCompare(right)) : undefined; const runtimeConfig = withActivatedPluginIds({ config: params.config, - pluginIds: modelOwnedPluginIds, + pluginIds: [...providerOwnedPluginIds, ...modelOwnedPluginIds], }); if (params.mode === "setup") { const providerPluginIds = resolveDiscoveredProviderPluginIds({ diff --git a/src/plugins/providers.test.ts b/src/plugins/providers.test.ts index ad8904a8e2b..07c8e7a1af7 100644 --- a/src/plugins/providers.test.ts +++ b/src/plugins/providers.test.ts @@ -589,7 +589,30 @@ describe("resolvePluginProviders", () => { }), ); }); + it("activates owning plugins for explicit provider refs", () => { + setOwningProviderManifestPlugins(); + resolvePluginProviders({ + config: {}, + providerRefs: ["openai-codex"], + activate: true, + }); + + expect(resolveRuntimePluginRegistryMock).toHaveBeenCalledWith( + expect.objectContaining({ + onlyPluginIds: ["openai"], + activate: true, + config: expect.objectContaining({ + plugins: expect.objectContaining({ + allow: ["openai"], + entries: { + openai: { enabled: true }, + }, + }), + }), + }), + ); + }); it.each([ { provider: "minimax-portal",