From dcf4d3c99153aa5d3c77c7206be228d7ba43db48 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 13 Sep 2026 22:47:09 -0700 Subject: [PATCH] perf(onboard): reuse provider install catalog during setup (#147712) Reuse one install-choice catalog for deprecated and selected-choice lookup, and retain only the final stable catalog sort. Preserve trust filters, blank/rejected paths, deprecated precedence, and choice ordering. Validate 45 owner tests, the commands/plugins-platform type graphs, and actual-owner catalog/result/trace parity across the bounded synthetic corpus. --- .../auth-choice.plugin-providers.test.ts | 104 +++++++++++++----- .../local/auth-choice.plugin-providers.ts | 23 ++-- src/plugins/provider-install-catalog.test.ts | 51 +++++++++ src/plugins/provider-install-catalog.ts | 36 +++--- 4 files changed, 156 insertions(+), 58 deletions(-) diff --git a/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.test.ts b/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.test.ts index a3c96f424f04..6495f1f496ec 100644 --- a/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.test.ts +++ b/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.test.ts @@ -30,11 +30,13 @@ const resolveManifestProviderAuthChoice = vi.hoisted(() => vi.fn(() => undefined vi.mock("../../../plugins/provider-auth-choices.js", () => ({ resolveManifestProviderAuthChoice, })); -const resolveProviderInstallCatalogEntry = vi.hoisted(() => vi.fn(() => undefined)); -const resolveDeprecatedProviderInstallCatalogEntry = vi.hoisted(() => vi.fn(() => undefined)); +const resolveProviderInstallCatalogEntries = vi.hoisted(() => + vi.fn< + typeof import("../../../plugins/provider-install-catalog.js").resolveProviderInstallCatalogEntries + >(() => []), +); vi.mock("../../../plugins/provider-install-catalog.js", () => ({ - resolveDeprecatedProviderInstallCatalogEntry, - resolveProviderInstallCatalogEntry, + resolveProviderInstallCatalogEntries, })); const ensureOnboardingPluginInstalled = vi.hoisted(() => vi.fn()); vi.mock("../../onboarding-plugin-install.js", () => ({ @@ -56,8 +58,7 @@ beforeEach(() => { vi.clearAllMocks(); resolvePreferredProviderForAuthChoice.mockResolvedValue(undefined); resolveManifestProviderAuthChoice.mockReturnValue(undefined); - resolveDeprecatedProviderInstallCatalogEntry.mockReturnValue(undefined); - resolveProviderInstallCatalogEntry.mockReturnValue(undefined); + resolveProviderInstallCatalogEntries.mockReturnValue([]); ensureOnboardingPluginInstalled.mockResolvedValue(undefined); resolveOwningPluginIdsForProvider.mockReturnValue(undefined as never); resolveProviderPluginChoice.mockReturnValue(undefined); @@ -321,6 +322,7 @@ describe("applyNonInteractivePluginProviderChoice", () => { }), ); expect(result).toEqual({ plugins: { allow: ["vllm"] } }); + expect(resolveProviderInstallCatalogEntries).not.toHaveBeenCalled(); }); it.each([false, true])( @@ -380,7 +382,7 @@ describe("applyNonInteractivePluginProviderChoice", () => { }, ); - it("installs an official catalog provider before applying a cold auth choice", async () => { + it("installs an official catalog provider for a cold auth choice with whitespace", async () => { const runtime = createRuntime(); const runNonInteractive = vi.fn(async ({ config }: { config: OpenClawConfig }) => ({ ...config, @@ -391,16 +393,21 @@ describe("applyNonInteractivePluginProviderChoice", () => { }, })); const provider = { id: "groq", pluginId: "groq", label: "Groq" }; - resolveProviderInstallCatalogEntry.mockReturnValue({ - pluginId: "groq", - providerId: "groq", - label: "Groq", - origin: "bundled", - install: { - npmSpec: "@openclaw/groq-provider", - defaultChoice: "npm", + resolveProviderInstallCatalogEntries.mockReturnValue([ + { + pluginId: "groq", + providerId: "groq", + methodId: "api-key", + choiceId: "groq-api-key", + choiceLabel: "Groq API key", + label: "Groq", + origin: "bundled", + install: { + npmSpec: "@openclaw/groq-provider", + defaultChoice: "npm", + }, }, - } as never); + ]); ensureOnboardingPluginInstalled.mockResolvedValue({ cfg: { plugins: { @@ -421,7 +428,7 @@ describe("applyNonInteractivePluginProviderChoice", () => { const result = await applyNonInteractivePluginProviderChoice({ nextConfig: { agents: { defaults: {} } } as OpenClawConfig, - authChoice: "groq-api-key", + authChoice: " groq-api-key ", opts: { groqApiKey: "groq-key" } as never, runtime: runtime as never, baseConfig: { agents: { defaults: {} } } as OpenClawConfig, @@ -430,12 +437,11 @@ describe("applyNonInteractivePluginProviderChoice", () => { toApiKeyCredential: vi.fn(), }); - expect(resolveProviderInstallCatalogEntry).toHaveBeenCalledWith( - "groq-api-key", - expect.objectContaining({ - includeUntrustedWorkspacePlugins: false, - }), - ); + expect(resolveProviderInstallCatalogEntries).toHaveBeenCalledExactlyOnceWith({ + config: { agents: { defaults: {} } }, + workspaceDir: target.workspaceDir, + includeUntrustedWorkspacePlugins: false, + }); expect(ensureOnboardingPluginInstalled).toHaveBeenCalledWith( expect.objectContaining({ cfg: { agents: { defaults: {} } }, @@ -468,11 +474,22 @@ describe("applyNonInteractivePluginProviderChoice", () => { }); }); - it("guides deprecated official auth choices before their plugin is installed", async () => { + it("guides deprecated official auth choices before a current catalog match can install", async () => { const runtime = createRuntime(); - resolveDeprecatedProviderInstallCatalogEntry.mockReturnValue({ + const choice = { + pluginId: "qwen", + providerId: "qwen", + methodId: "api-key", choiceId: "qwen-api-key", - } as never); + choiceLabel: "Qwen API key", + label: "Qwen", + origin: "bundled" as const, + install: { npmSpec: "@openclaw/qwen-provider" }, + }; + resolveProviderInstallCatalogEntries.mockReturnValue([ + { ...choice, choiceId: "modelstudio-api-key" }, + { ...choice, deprecatedChoiceIds: ["modelstudio-api-key"] }, + ]); const result = await applyNonInteractivePluginProviderChoice({ nextConfig: { agents: { defaults: {} } } as OpenClawConfig, @@ -492,9 +509,39 @@ describe("applyNonInteractivePluginProviderChoice", () => { ); expect(runtime.exit).toHaveBeenCalledWith(1); expect(ensureOnboardingPluginInstalled).not.toHaveBeenCalled(); - expect(resolveProviderInstallCatalogEntry).not.toHaveBeenCalled(); + expect(resolveProviderInstallCatalogEntries).toHaveBeenCalledOnce(); }); + it.each([ + { authChoice: "", catalogReads: 0 }, + { authChoice: " ", catalogReads: 0 }, + { authChoice: "unknown-choice", catalogReads: 1 }, + ])( + "leaves unmatched auth choice %j without setup effects", + async ({ authChoice, catalogReads }) => { + const runtime = createRuntime(); + const config: OpenClawConfig = { agents: { defaults: {} } }; + const resolveApiKey = vi.fn(); + const result = await applyNonInteractivePluginProviderChoice({ + nextConfig: config, + authChoice, + opts: {}, + runtime, + baseConfig: config, + target, + resolveApiKey, + toApiKeyCredential: vi.fn(), + }); + + expect(result).toBeUndefined(); + expect(resolveProviderInstallCatalogEntries).toHaveBeenCalledTimes(catalogReads); + expect(ensureOnboardingPluginInstalled).not.toHaveBeenCalled(); + expect(resolveApiKey).not.toHaveBeenCalled(); + expect(runtime.error).not.toHaveBeenCalled(); + expect(runtime.exit).not.toHaveBeenCalled(); + }, + ); + it.each([false, true])( "rejects an unmatched provider-plugin auth choice while honoring json=%s", async (json) => { @@ -513,6 +560,7 @@ describe("applyNonInteractivePluginProviderChoice", () => { expect(result).toBeNull(); expect(resolvePreferredProviderForAuthChoice).not.toHaveBeenCalled(); + expect(resolveProviderInstallCatalogEntries).not.toHaveBeenCalled(); expectRuntimeErrorIncludes( runtime, 'Auth choice "provider-plugin:workspace-provider:api-key" was not matched to a trusted provider plugin.', @@ -559,6 +607,7 @@ describe("applyNonInteractivePluginProviderChoice", () => { expectRuntimeErrorIncludes(runtime, '"provider-plugin:"'); expect(resolvePluginProvidersCore).not.toHaveBeenCalled(); expect(resolvePreferredProviderForAuthChoice).not.toHaveBeenCalled(); + expect(resolveProviderInstallCatalogEntries).not.toHaveBeenCalled(); if (json) { expect(runtime.log).toHaveBeenCalledOnce(); expect(JSON.parse(String(runtime.log.mock.calls[0]?.[0]))).toEqual({ @@ -601,6 +650,7 @@ describe("applyNonInteractivePluginProviderChoice", () => { expect(resolveProviderPluginChoice).toHaveBeenCalledTimes(1); expect(resolvePluginProvidersCore).toHaveBeenCalledTimes(1); expect(mockCall(resolveManifestProviderAuthChoice, 0)[0]).toBe("workspace-provider-api-key"); + expect(resolveProviderInstallCatalogEntries).not.toHaveBeenCalled(); const trustedManifestInput = mockArg(resolveManifestProviderAuthChoice, 0, 1); expect(trustedManifestInput.includeUntrustedWorkspacePlugins).toBe(false); expect(mockCall(resolveManifestProviderAuthChoice, 1)[0]).toBe("workspace-provider-api-key"); diff --git a/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.ts b/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.ts index f6ce2ab6b08e..f0e711f0c9c1 100644 --- a/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.ts +++ b/src/commands/onboard-non-interactive/local/auth-choice.plugin-providers.ts @@ -16,10 +16,7 @@ import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import { enablePluginWithCapabilityConsent } from "../../../plugins/enable.js"; import { resolvePreferredProviderForAuthChoice } from "../../../plugins/provider-auth-choice-preference.js"; import { resolveManifestProviderAuthChoice } from "../../../plugins/provider-auth-choices.js"; -import { - resolveDeprecatedProviderInstallCatalogEntry, - resolveProviderInstallCatalogEntry, -} from "../../../plugins/provider-install-catalog.js"; +import { resolveProviderInstallCatalogEntries } from "../../../plugins/provider-install-catalog.js"; import type { ProviderAuthOptionBag, ProviderNonInteractiveApiKeyCredentialParams, @@ -164,23 +161,25 @@ export async function applyNonInteractivePluginProviderChoice(params: { ].join("\n"), ); } - const installCatalogParams = { + const normalizedChoiceId = params.authChoice.trim(); + if (!normalizedChoiceId) { + return undefined; + } + const installCatalog = resolveProviderInstallCatalogEntries({ config: nextConfig, workspaceDir, includeUntrustedWorkspacePlugins: false, - }; - const deprecatedInstallCatalogEntry = resolveDeprecatedProviderInstallCatalogEntry( - params.authChoice, - installCatalogParams, + }); + const deprecatedInstallCatalogEntry = installCatalog.find((entry) => + entry.deprecatedChoiceIds?.includes(normalizedChoiceId), ); if (deprecatedInstallCatalogEntry) { return reject( `${JSON.stringify(params.authChoice)} is no longer supported. Use --auth-choice ${JSON.stringify(deprecatedInstallCatalogEntry.choiceId)} instead.`, ); } - const installCatalogEntry = resolveProviderInstallCatalogEntry( - params.authChoice, - installCatalogParams, + const installCatalogEntry = installCatalog.find( + (entry) => entry.choiceId === normalizedChoiceId, ); if (!installCatalogEntry) { return undefined; diff --git a/src/plugins/provider-install-catalog.test.ts b/src/plugins/provider-install-catalog.test.ts index 4bf9de7e6137..75ee7c4694f3 100644 --- a/src/plugins/provider-install-catalog.test.ts +++ b/src/plugins/provider-install-catalog.test.ts @@ -227,6 +227,57 @@ describe("provider install catalog", () => { ]); }); + it("keeps stable label order and installed-choice priority when merging official entries", () => { + loadPluginRegistrySnapshot.mockReturnValue( + registrySnapshot({ plugins: [{ ...vllmPluginWithPackageInstall(), origin: "bundled" }] }), + ); + const choice = (choiceId: string, choiceLabel: string) => ({ + pluginId: "vllm", + providerId: "vllm", + methodId: "api-key", + choiceId, + choiceLabel, + }); + resolveManifestProviderAuthChoices.mockReturnValue([ + choice("last", "Zulu"), + choice("same-first", "Same"), + choice("same-second", "Same"), + choice("first", "Alpha"), + ]); + listOfficialExternalProviderCatalogEntries.mockReturnValue([ + { + name: "@openclaw/qwen-provider", + openclaw: { + plugin: { id: "qwen", label: "Qwen" }, + install: { npmSpec: "@openclaw/qwen-provider" }, + providers: [ + { + id: "qwen", + name: "Qwen", + authChoices: [ + { method: "api-key", choiceId: "same-official", choiceLabel: "Same" }, + { method: "api-key", choiceId: "same-first", choiceLabel: "A shadow" }, + ], + }, + ], + }, + }, + ]); + + expect( + resolveProviderInstallCatalogEntries().map(({ choiceId, pluginId }) => ({ + choiceId, + pluginId, + })), + ).toEqual([ + { choiceId: "first", pluginId: "vllm" }, + { choiceId: "same-first", pluginId: "vllm" }, + { choiceId: "same-second", pluginId: "vllm" }, + { choiceId: "same-official", pluginId: "qwen" }, + { choiceId: "last", pluginId: "vllm" }, + ]); + }); + it("prefers durable install records over package-authored install intent", () => { loadPluginRegistrySnapshot.mockReturnValue( registrySnapshot({ diff --git a/src/plugins/provider-install-catalog.ts b/src/plugins/provider-install-catalog.ts index 44945194f583..81c21e4f9228 100644 --- a/src/plugins/provider-install-catalog.ts +++ b/src/plugins/provider-install-catalog.ts @@ -311,25 +311,23 @@ export function resolveProviderInstallCatalogEntries( const installParams = params ?? {}; const { installedPluginIds, installsByPluginId } = resolvePreferredInstallsByPluginId(installParams); - const manifestEntries = resolveManifestProviderAuthChoices(params) - .flatMap((choice) => { - const install = installsByPluginId.get(choice.pluginId); - if (!install) { - return []; - } - return [ - { - ...choice, - label: choice.groupLabel ?? choice.choiceLabel, - origin: install.origin, - install: install.install, - installSource: describePluginInstallSource(install.install, { - expectedPackageName: install.packageName, - }), - } satisfies ProviderInstallCatalogEntry, - ]; - }) - .toSorted((left, right) => left.choiceLabel.localeCompare(right.choiceLabel)); + const manifestEntries = resolveManifestProviderAuthChoices(params).flatMap((choice) => { + const install = installsByPluginId.get(choice.pluginId); + if (!install) { + return []; + } + return [ + { + ...choice, + label: choice.groupLabel ?? choice.choiceLabel, + origin: install.origin, + install: install.install, + installSource: describePluginInstallSource(install.install, { + expectedPackageName: install.packageName, + }), + } satisfies ProviderInstallCatalogEntry, + ]; + }); const seenChoiceIds = new Set(manifestEntries.map((entry) => entry.choiceId)); const officialEntries = resolveOfficialExternalProviderInstallCatalogEntries({ installedPluginIds,