From 522eedc754370ecc7229556fac6c74ecc627a07f Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 26 Apr 2026 11:50:48 -0700 Subject: [PATCH] refactor(plugins): make provider discovery runtime explicit --- .../models-config.providers.implicit.ts | 4 +-- .../models/list.provider-catalog.test.ts | 25 +++++++++++-------- src/commands/models/list.provider-catalog.ts | 6 ++--- src/plugins/provider-discovery.test.ts | 13 ++++++++++ src/plugins/provider-discovery.ts | 6 +++++ 5 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/agents/models-config.providers.implicit.ts b/src/agents/models-config.providers.implicit.ts index 777ecbb7dca..635fd2fed74 100644 --- a/src/agents/models-config.providers.implicit.ts +++ b/src/agents/models-config.providers.implicit.ts @@ -4,7 +4,7 @@ import { createSubsystemLogger } from "../logging/subsystem.js"; import { groupPluginDiscoveryProvidersByOrder, normalizePluginDiscoveryResult, - resolvePluginDiscoveryProviders, + resolveRuntimePluginDiscoveryProviders, runProviderCatalog, } from "../plugins/provider-discovery.js"; import { resolveOwningPluginIdsForProvider } from "../plugins/providers.js"; @@ -359,7 +359,7 @@ export async function resolveImplicitProviders( resolveProviderApiKey: createProviderApiKeyResolver(env, getAuthStore, params.config), resolveProviderAuth: createProviderAuthResolver(env, getAuthStore, params.config), }; - const discoveryProviders = await resolvePluginDiscoveryProviders({ + const discoveryProviders = await resolveRuntimePluginDiscoveryProviders({ config: params.config, workspaceDir: params.workspaceDir, env, diff --git a/src/commands/models/list.provider-catalog.test.ts b/src/commands/models/list.provider-catalog.test.ts index 9752770dfa0..67b4a3a074c 100644 --- a/src/commands/models/list.provider-catalog.test.ts +++ b/src/commands/models/list.provider-catalog.test.ts @@ -11,7 +11,7 @@ const providerDiscoveryMocks = vi.hoisted(() => ({ resolveProviderOwners: vi.fn(), resolveBundledProviderCompatPluginIds: vi.fn(), resolveOwningPluginIdsForProvider: vi.fn(), - resolvePluginDiscoveryProviders: vi.fn(), + resolveRuntimePluginDiscoveryProviders: vi.fn(), resolveProviderContractPluginIdsForProviderAlias: vi.fn(), })); @@ -38,7 +38,8 @@ vi.mock("../../plugins/provider-discovery.js", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - resolvePluginDiscoveryProviders: providerDiscoveryMocks.resolvePluginDiscoveryProviders, + resolveRuntimePluginDiscoveryProviders: + providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders, }; }); @@ -145,7 +146,7 @@ describe("loadProviderCatalogModelsForList", () => { providerDiscoveryMocks.resolveProviderContractPluginIdsForProviderAlias.mockImplementation( (provider: string) => (provider === "azure-openai-responses" ? ["openai"] : undefined), ); - providerDiscoveryMocks.resolvePluginDiscoveryProviders.mockImplementation( + providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders.mockImplementation( async ({ onlyPluginIds }: { onlyPluginIds?: string[] }) => defaultProviders.filter((provider) => onlyPluginIds?.includes(provider.pluginId)), ); @@ -183,7 +184,7 @@ describe("loadProviderCatalogModelsForList", () => { staticOnly: true, }); - expect(providerDiscoveryMocks.resolvePluginDiscoveryProviders).toHaveBeenCalledWith( + expect(providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders).toHaveBeenCalledWith( expect.objectContaining({ onlyPluginIds: ["moonshot"], requireCompleteDiscoveryEntryCoverage: true, @@ -227,7 +228,7 @@ describe("loadProviderCatalogModelsForList", () => { }); it("returns an empty catalog when a static provider catalog throws", async () => { - providerDiscoveryMocks.resolvePluginDiscoveryProviders.mockResolvedValueOnce([ + providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders.mockResolvedValueOnce([ { id: "moonshot", pluginId: "moonshot", @@ -251,7 +252,9 @@ describe("loadProviderCatalogModelsForList", () => { }); it("only skips registry for providers with actual static catalogs", async () => { - providerDiscoveryMocks.resolvePluginDiscoveryProviders.mockResolvedValue([catalogOnlyProvider]); + providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders.mockResolvedValue([ + catalogOnlyProvider, + ]); await expect( hasProviderStaticCatalogForFilter({ @@ -261,7 +264,7 @@ describe("loadProviderCatalogModelsForList", () => { }), ).resolves.toBe(false); - expect(providerDiscoveryMocks.resolvePluginDiscoveryProviders).toHaveBeenCalledWith( + expect(providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders).toHaveBeenCalledWith( expect.objectContaining({ onlyPluginIds: ["ollama"], requireCompleteDiscoveryEntryCoverage: true, @@ -271,7 +274,7 @@ describe("loadProviderCatalogModelsForList", () => { }); it("does not skip registry when a bundled provider has no lightweight static entry", async () => { - providerDiscoveryMocks.resolvePluginDiscoveryProviders.mockResolvedValueOnce([]); + providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders.mockResolvedValueOnce([]); await expect( hasProviderStaticCatalogForFilter({ @@ -297,7 +300,7 @@ describe("loadProviderCatalogModelsForList", () => { }), ).resolves.toBe(false); - expect(providerDiscoveryMocks.resolvePluginDiscoveryProviders).not.toHaveBeenCalled(); + expect(providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders).not.toHaveBeenCalled(); }); it("recognizes bundled provider hook aliases before the unknown-provider short-circuit", async () => { @@ -317,7 +320,7 @@ describe("loadProviderCatalogModelsForList", () => { provider: { baseUrl: "https://workspace.example/v1", models: [] }, })); providerDiscoveryMocks.resolveBundledProviderCompatPluginIds.mockReturnValue(["bundled-demo"]); - providerDiscoveryMocks.resolvePluginDiscoveryProviders.mockResolvedValue([ + providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders.mockResolvedValue([ { id: "bundled-demo", pluginId: "bundled-demo", @@ -342,7 +345,7 @@ describe("loadProviderCatalogModelsForList", () => { ...baseParams, }); - expect(providerDiscoveryMocks.resolvePluginDiscoveryProviders).toHaveBeenCalledWith( + expect(providerDiscoveryMocks.resolveRuntimePluginDiscoveryProviders).toHaveBeenCalledWith( expect.objectContaining({ onlyPluginIds: ["bundled-demo"], includeUntrustedWorkspacePlugins: false, diff --git a/src/commands/models/list.provider-catalog.ts b/src/commands/models/list.provider-catalog.ts index 14491a15beb..dc8a5ea6c68 100644 --- a/src/commands/models/list.provider-catalog.ts +++ b/src/commands/models/list.provider-catalog.ts @@ -13,7 +13,7 @@ import { import { groupPluginDiscoveryProvidersByOrder, normalizePluginDiscoveryResult, - resolvePluginDiscoveryProviders, + resolveRuntimePluginDiscoveryProviders, runProviderStaticCatalog, } from "../../plugins/provider-discovery.js"; import { @@ -157,7 +157,7 @@ export async function hasProviderStaticCatalogForFilter(params: { if (scopedPluginIds.length === 0) { return false; } - const providers = await resolvePluginDiscoveryProviders({ + const providers = await resolveRuntimePluginDiscoveryProviders({ config: params.cfg, env, onlyPluginIds: scopedPluginIds, @@ -227,7 +227,7 @@ export async function loadProviderCatalogModelsForList(params: { } const providers = ( - await resolvePluginDiscoveryProviders({ + await resolveRuntimePluginDiscoveryProviders({ config: params.cfg, env, onlyPluginIds: scopedPluginIds, diff --git a/src/plugins/provider-discovery.test.ts b/src/plugins/provider-discovery.test.ts index 8c9680c089b..65e4bddbf22 100644 --- a/src/plugins/provider-discovery.test.ts +++ b/src/plugins/provider-discovery.test.ts @@ -166,6 +166,19 @@ async function expectProviderCatalogResult(params: { } describe("resolveInstalledPluginProviderContributionIds", () => { + it("keeps current production callers off the ambiguous runtime-discovery alias", () => { + const callerPaths = [ + "src/agents/models-config.providers.implicit.ts", + "src/commands/models/list.provider-catalog.ts", + ]; + + for (const callerPath of callerPaths) { + expect(fs.readFileSync(path.join(process.cwd(), callerPath), "utf-8")).not.toContain( + "resolvePluginDiscoveryProviders", + ); + } + }); + it("reads provider ids from the installed plugin index without importing runtime entries", () => { const candidate = createProviderContributionCandidate({ pluginId: "demo", diff --git a/src/plugins/provider-discovery.ts b/src/plugins/provider-discovery.ts index 3455676f40e..5168ed924cc 100644 --- a/src/plugins/provider-discovery.ts +++ b/src/plugins/provider-discovery.ts @@ -79,6 +79,12 @@ export async function resolveRuntimePluginDiscoveryProviders( .filter((provider) => resolveProviderCatalogOrderHook(provider)); } +/** + * @deprecated Runtime-backed provider discovery must be explicit at call sites. + * Use `resolveRuntimePluginDiscoveryProviders(...)` for paths that intentionally + * import provider plugin runtime, or `resolveInstalledPluginProviderContributionIds(...)` + * for cold installed-index reads. + */ export async function resolvePluginDiscoveryProviders( params: ResolveRuntimePluginDiscoveryProvidersParams, ): Promise {