refactor(plugins): make provider discovery runtime explicit

This commit is contained in:
Vincent Koc
2026-04-26 11:50:48 -07:00
parent 71e361af8a
commit 522eedc754
5 changed files with 38 additions and 16 deletions

View File

@@ -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,

View File

@@ -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<typeof import("../../plugins/provider-discovery.js")>();
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,

View File

@@ -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,

View File

@@ -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",

View File

@@ -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<ProviderPlugin[]> {