From 8dac217ce94e14a67ad2fc65837e518a2fbe0669 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 20:54:27 -0700 Subject: [PATCH] refactor(models): consolidate prepared catalog construction (#130654) * refactor(models): consolidate prepared catalog construction * test(models): avoid shadowing registry fixture identifiers --- .../model.static-catalog.prepared.test.ts | 63 +++++++++- .../model.static-catalog.ts | 80 ++++-------- ...prepared-model-runtime.catalog-contract.ts | 4 +- ...epared-model-runtime.configured-catalog.ts | 15 +-- ...red-model-runtime.configured-completion.ts | 17 +-- .../prepared-model-runtime.configured.ts | 11 +- src/agents/prepared-model-runtime.facts.ts | 27 ++-- ...pared-model-runtime.startup-static.test.ts | 43 +++++-- src/agents/sessions/model-registry.test.ts | 118 +++++++++--------- src/agents/sessions/model-registry.ts | 33 ++--- src/agents/sessions/resolve-config-value.ts | 5 - 11 files changed, 212 insertions(+), 204 deletions(-) diff --git a/src/agents/embedded-agent-runner/model.static-catalog.prepared.test.ts b/src/agents/embedded-agent-runner/model.static-catalog.prepared.test.ts index ce1451abd00f..16767b59ffc3 100644 --- a/src/agents/embedded-agent-runner/model.static-catalog.prepared.test.ts +++ b/src/agents/embedded-agent-runner/model.static-catalog.prepared.test.ts @@ -1,6 +1,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { PluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.types.js"; -import { getModelProviderRequestRouteFacts } from "../provider-request-config.js"; +import { getModelProviderLocalService } from "../provider-local-service.js"; +import { + getModelProviderRequestRouteFacts, + getModelProviderRequestTransport, +} from "../provider-request-config.js"; const mocks = vi.hoisted(() => ({ loadPluginManifestRegistryCore: vi.fn(), @@ -196,18 +200,42 @@ describe("prepared bundled provider static catalogs", () => { ); }); - it("projects prepared rows without rerunning hooks", async () => { + it("projects heterogeneous prepared rows without rerunning hooks or resolving empty providers", async () => { mocks.resolveRuntimePluginDiscoveryProviders.mockResolvedValue([provider]); mocks.normalizePluginDiscoveryResult.mockReturnValue({ google: { + api: "fixture-api", + baseUrl: "https://fixture.example/v1", + authHeader: false, + maxTokens: 4096, + request: { headers: { "X-Catalog": "prepared" } }, + localService: { command: "fixture-service" }, models: [ { id: "gemini-3.1-pro-preview", name: "Gemini Pro", contextWindow: 1_048_576, + reasoning: false, + input: ["text", "image"], + cost: { input: 0.5 }, + maxTokens: 0, + }, + { + id: "fallback-model", + name: "", + baseUrl: "", + input: [], + contextWindow: 0, + contextTokens: 0, }, ], }, + empty: { + request: { + headers: { "X-Unused": { source: "env", provider: "default", id: "UNUSED_HEADER" } }, + }, + models: [], + }, }); const metadataSnapshot = createMetadataSnapshot(["google"]); @@ -223,12 +251,37 @@ describe("prepared bundled provider static catalogs", () => { expect.objectContaining({ id: "gemini-3.1-pro-preview", provider: "google", + api: "fixture-api", + baseUrl: "https://fixture.example/v1", + authHeader: false, + reasoning: false, + input: ["text", "image"], + cost: { input: 0.5 }, contextWindow: 1_048_576, + maxTokens: 0, + }), + expect.objectContaining({ + id: "fallback-model", + name: "fallback-model", + provider: "google", + api: "fixture-api", + baseUrl: "", + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 0, + contextTokens: 0, + maxTokens: 4096, }), ]); - expect(getModelProviderRequestRouteFacts(models[0]!)?.providerMetadataOwners).toBe( - metadataSnapshot.owners, - ); + for (const model of models) { + expect(getModelProviderRequestRouteFacts(model)?.providerMetadataOwners).toBe( + metadataSnapshot.owners, + ); + expect(getModelProviderRequestTransport(model)).toEqual({ + headers: { "X-Catalog": "prepared" }, + }); + expect(getModelProviderLocalService(model)).toEqual({ command: "fixture-service" }); + } expect(mocks.resolveRuntimePluginDiscoveryProviders).toHaveBeenCalledOnce(); expect(mocks.runProviderStaticCatalog).not.toHaveBeenCalled(); }); diff --git a/src/agents/embedded-agent-runner/model.static-catalog.ts b/src/agents/embedded-agent-runner/model.static-catalog.ts index 99f267998761..9a5f9793d13e 100644 --- a/src/agents/embedded-agent-runner/model.static-catalog.ts +++ b/src/agents/embedded-agent-runner/model.static-catalog.ts @@ -27,7 +27,7 @@ import { resolveOwningPluginIdsForProviderRef, } from "../../plugins/providers.js"; import { DEFAULT_CONTEXT_TOKENS } from "../defaults.js"; -import { buildInlineProviderModels } from "./model.inline-provider.js"; +import { buildInlineProviderModels, type InlineModelEntry } from "./model.inline-provider.js"; import { createStaticModelIdMatcher, staticModelIdMatches, @@ -97,38 +97,22 @@ function modelFromStaticCatalogRow(row: NormalizedModelCatalogRow): ProviderRunt }; } -function modelFromProviderStaticCatalog(params: { - provider: string; - providerConfig: ModelProviderConfig; - model: ModelProviderConfig["models"][number]; - providerMetadataOwners?: PluginMetadataSnapshot["owners"]; -}): ProviderRuntimeModel { - const [model] = buildInlineProviderModels( - { - [params.provider]: { ...params.providerConfig, models: [params.model] }, - }, - { providerMetadataOwners: params.providerMetadataOwners }, - ); +function completeProviderStaticCatalogModel( + model: InlineModelEntry, + providerConfig: ModelProviderConfig, +): ProviderRuntimeModel { return { ...model, - id: model?.id ?? params.model.id, - name: model?.name || params.model.name || params.model.id, - provider: params.provider, - api: model?.api ?? params.model.api ?? params.providerConfig.api ?? "openai-responses", - baseUrl: model?.baseUrl ?? params.model.baseUrl ?? params.providerConfig.baseUrl ?? "", - reasoning: model?.reasoning ?? params.model.reasoning ?? false, - input: normalizeStaticCatalogInput(model?.input ?? params.model.input), - cost: model?.cost ?? normalizeStaticCatalogCost(params.model.cost), - contextWindow: model?.contextWindow ?? params.model.contextWindow ?? DEFAULT_CONTEXT_TOKENS, - contextTokens: model?.contextTokens ?? params.model.contextTokens, - maxTokens: - model?.maxTokens ?? - params.model.maxTokens ?? - params.providerConfig.maxTokens ?? - DEFAULT_CONTEXT_TOKENS, - ...(params.providerConfig.authHeader !== undefined - ? { authHeader: params.providerConfig.authHeader } - : {}), + name: model.name || model.id, + api: model.api ?? providerConfig.api ?? "openai-responses", + baseUrl: model.baseUrl ?? "", + reasoning: model.reasoning ?? false, + input: normalizeStaticCatalogInput(model.input), + cost: model.cost ?? normalizeStaticCatalogCost(undefined), + contextWindow: model.contextWindow ?? DEFAULT_CONTEXT_TOKENS, + contextTokens: model.contextTokens, + maxTokens: model.maxTokens ?? DEFAULT_CONTEXT_TOKENS, + ...(providerConfig.authHeader !== undefined ? { authHeader: providerConfig.authHeader } : {}), }; } @@ -485,21 +469,20 @@ async function loadBundledProviderStaticCatalogModels(params: { }); for (const [providerIdRaw, providerConfig] of Object.entries(normalized)) { const provider = normalizeProviderId(providerIdRaw); - if (!provider || !Array.isArray(providerConfig.models)) { + // Empty catalogs never resolve request secrets or transport settings. + if ( + !provider || + !Array.isArray(providerConfig.models) || + providerConfig.models.length === 0 + ) { continue; } const models = modelsByProvider.get(provider) ?? []; models.push( - ...providerConfig.models.map((model) => - modelFromProviderStaticCatalog({ - provider, - providerConfig, - model, - ...(params.providerMetadataOwners - ? { providerMetadataOwners: params.providerMetadataOwners } - : {}), - }), - ), + ...buildInlineProviderModels( + { [provider]: providerConfig }, + { providerMetadataOwners: params.providerMetadataOwners }, + ).map((model) => completeProviderStaticCatalogModel(model, providerConfig)), ); modelsByProvider.set(provider, models); } @@ -639,17 +622,6 @@ function createScopedBundledProviderStaticCatalogModelResolver( }; } -/** - * Prepares bundled provider static-catalog lookup. - * Each provider hook runs at most once for the resolver lifetime. - */ -function createBundledProviderStaticCatalogModelResolver( - params: BundledProviderStaticCatalogResolverParams = {}, -): (lookup: BundledStaticCatalogLookup) => Promise { - const resolveModel = createScopedBundledProviderStaticCatalogModelResolver(params); - return async (lookup) => await resolveModel(lookup); -} - function resolveOwnedNestedProviderLookup(params: { lookup: BundledStaticCatalogLookup; resolverParams: BundledProviderStaticCatalogResolverParams; @@ -735,5 +707,5 @@ export async function resolveBundledProviderStaticCatalogModel(params: { env?: NodeJS.ProcessEnv; metadataSnapshot?: PluginMetadataSnapshot; }): Promise { - return createBundledProviderStaticCatalogModelResolver(params)(params); + return createScopedBundledProviderStaticCatalogModelResolver(params)(params); } diff --git a/src/agents/prepared-model-runtime.catalog-contract.ts b/src/agents/prepared-model-runtime.catalog-contract.ts index 75533b7e5238..c86813f57c39 100644 --- a/src/agents/prepared-model-runtime.catalog-contract.ts +++ b/src/agents/prepared-model-runtime.catalog-contract.ts @@ -1,4 +1,4 @@ -import type { ConfiguredModelRef } from "@openclaw/model-catalog-core/configured-model-refs"; +import type { ModelCatalogRef } from "@openclaw/model-catalog-core/model-catalog-refs"; import type { ProviderCatalogOutcome } from "../plugins/provider-catalog.types.js"; import type { AuthProfileStore } from "./auth-profiles/types.js"; import type { InlineModelEntry } from "./embedded-agent-runner/model.inline-provider.js"; @@ -19,7 +19,7 @@ export type PreparedModelRuntimeAgentBaseFacts = { templateAuthStorage: AuthStorage; credentials: Readonly; providerIds: string[]; - configuredModelRefs: readonly ConfiguredModelRef[]; + configuredModelRefs: readonly ModelCatalogRef[]; }; export type PreparedModelRuntimeAgentFacts = PreparedModelRuntimeAgentBaseFacts & { diff --git a/src/agents/prepared-model-runtime.configured-catalog.ts b/src/agents/prepared-model-runtime.configured-catalog.ts index 02b4168ea2a1..8cbb17fce0e8 100644 --- a/src/agents/prepared-model-runtime.configured-catalog.ts +++ b/src/agents/prepared-model-runtime.configured-catalog.ts @@ -1,4 +1,4 @@ -import type { ConfiguredModelRef } from "@openclaw/model-catalog-core/configured-model-refs"; +import type { ModelCatalogRef } from "@openclaw/model-catalog-core/model-catalog-refs"; import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id"; import type { InlineModelEntry } from "./embedded-agent-runner/model.inline-provider.js"; import type { ModelCatalogEntry } from "./model-catalog.js"; @@ -11,7 +11,7 @@ import { import type { ModelRegistry } from "./sessions/model-registry.js"; type ConfiguredCatalogAgentFacts = { - configuredModelRefs: readonly ConfiguredModelRef[]; + configuredModelRefs: readonly ModelCatalogRef[]; runtimeCapabilityModels: readonly PreparedRuntimeCapabilityModel[]; }; @@ -50,16 +50,7 @@ function createConfiguredModelCatalogSnapshot(params: { for (const configured of params.configuredRuntimeModels) { addEntry(toStaticCatalogEntry(configured.model)); } - for (const { value } of params.agentFacts.configuredModelRefs) { - const separator = value.indexOf("/"); - if (separator <= 0 || separator >= value.length - 1) { - continue; - } - const provider = normalizeProviderId(value.slice(0, separator)); - const modelId = value.slice(separator + 1).trim(); - if (!provider || !modelId) { - continue; - } + for (const { provider, modelId } of params.agentFacts.configuredModelRefs) { const model = params.templateModelRegistry.find(provider, modelId); if (model) { addEntry(toStaticCatalogEntry(model)); diff --git a/src/agents/prepared-model-runtime.configured-completion.ts b/src/agents/prepared-model-runtime.configured-completion.ts index 3a3bc85c4cf0..bde9c1199d8b 100644 --- a/src/agents/prepared-model-runtime.configured-completion.ts +++ b/src/agents/prepared-model-runtime.configured-completion.ts @@ -1,13 +1,12 @@ -import type { ConfiguredModelRef } from "@openclaw/model-catalog-core/configured-model-refs"; import { buildModelCatalogMergeKey, - parseModelCatalogRef, + type ModelCatalogRef, } from "@openclaw/model-catalog-core/model-catalog-refs"; import type { ProviderRuntimeModel } from "../plugins/provider-runtime-model.types.js"; import type { PreparedConfiguredRuntimeModel } from "./prepared-model-runtime.configured.js"; export function completeConfiguredRuntimeModels(params: { - configuredModelRefs: readonly ConfiguredModelRef[]; + configuredModelRefs: readonly ModelCatalogRef[]; configuredRuntimeModels: readonly PreparedConfiguredRuntimeModel[]; resolveDynamicModel: (lookup: { provider: string; @@ -22,20 +21,16 @@ export function completeConfiguredRuntimeModels(params: { ); const completed: PreparedConfiguredRuntimeModel[] = []; const seen = new Set(); - for (const { value } of params.configuredModelRefs) { - const parsed = parseModelCatalogRef(value); - if (!parsed) { - continue; - } - const key = buildModelCatalogMergeKey(parsed.provider, parsed.modelId); + for (const ref of params.configuredModelRefs) { + const key = buildModelCatalogMergeKey(ref.provider, ref.modelId); if (seen.has(key)) { continue; } seen.add(key); const prepared = existing.get(key); - const model = prepared?.model ?? params.resolveDynamicModel(parsed); + const model = prepared?.model ?? params.resolveDynamicModel(ref); if (model) { - completed.push({ provider: parsed.provider, modelId: parsed.modelId, model }); + completed.push({ ...ref, model }); } } return completed; diff --git a/src/agents/prepared-model-runtime.configured.ts b/src/agents/prepared-model-runtime.configured.ts index 0a231fd38144..7be4555f5169 100644 --- a/src/agents/prepared-model-runtime.configured.ts +++ b/src/agents/prepared-model-runtime.configured.ts @@ -5,6 +5,7 @@ import { import { buildModelCatalogMergeKey, parseModelCatalogRef, + type ModelCatalogRef, } from "@openclaw/model-catalog-core/model-catalog-refs"; import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id"; import { MODEL_APIS } from "../config/types.models.js"; @@ -163,8 +164,7 @@ export function collectConfiguredProviderIdsNeedingStaticCatalog(params: { } export function prepareConfiguredRuntimeModels(params: { - config: OpenClawConfig; - configuredModelRefs?: readonly ConfiguredModelRef[]; + configuredModelRefs: readonly ModelCatalogRef[]; metadataSnapshot: PluginMetadataSnapshot; preparedStaticProviderCatalog?: PreparedProviderStaticCatalog; providerStaticModels: readonly ProviderRuntimeModel[]; @@ -176,12 +176,7 @@ export function prepareConfiguredRuntimeModels(params: { }): PreparedConfiguredRuntimeModel[] { const prepared: PreparedConfiguredRuntimeModel[] = []; const seen = new Set(); - for (const { value } of params.configuredModelRefs ?? collectConfiguredModelRefs(params.config)) { - const parsed = parseModelCatalogRef(value); - if (!parsed) { - continue; - } - const { modelId, provider } = parsed; + for (const { modelId, provider } of params.configuredModelRefs) { const key = buildModelCatalogMergeKey(provider, modelId); if (seen.has(key)) { continue; diff --git a/src/agents/prepared-model-runtime.facts.ts b/src/agents/prepared-model-runtime.facts.ts index 7b58a0a35f7a..9e78d8a7a70b 100644 --- a/src/agents/prepared-model-runtime.facts.ts +++ b/src/agents/prepared-model-runtime.facts.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import path from "node:path"; import { performance } from "node:perf_hooks"; +import { parseModelCatalogRef } from "@openclaw/model-catalog-core/model-catalog-refs"; import { findNormalizedProviderValue, normalizeProviderId, @@ -113,7 +114,7 @@ function prepareAgentFacts( }); const credentials = authFacts.credentials; const templateAuthStorage = authFacts.authStorage; - const configuredModelRefs = collectPreparedModelRuntimeConfiguredRefs( + const rawConfiguredModelRefs = collectPreparedModelRuntimeConfiguredRefs( input.config, input.agentId, ); @@ -123,7 +124,12 @@ function prepareAgentFacts( authStore: authFacts.store, templateAuthStorage, credentials, - configuredModelRefs, + // Keep order and case-distinct refs: registry lookup remains exact-case even + // where static/dynamic completion deduplicates case-insensitive merge keys. + configuredModelRefs: rawConfiguredModelRefs.flatMap(({ value }) => { + const ref = parseModelCatalogRef(value); + return ref ? [ref] : []; + }), // Gateway startup prepares only providers named by config/model selection. An unrelated // stored credential must not pull that provider's complete catalog into the admission path. providerIds: [ @@ -132,7 +138,7 @@ function prepareAgentFacts( input.config, credentials, catalogMode === "live", - configuredModelRefs, + rawConfiguredModelRefs, ), ...parseConfiguredModelVisibilityEntries({ cfg: input.config, @@ -344,7 +350,6 @@ export async function prepareWorkspaceBuildGroup( const agentFacts: PreparedModelRuntimeAgentFacts[] = []; for (const facts of agentBaseFacts) { const configuredRuntimeModels = prepareConfiguredRuntimeModels({ - config: facts.input.config, configuredModelRefs: facts.configuredModelRefs, metadataSnapshot: pluginMetadataSnapshot, ...(preparedStaticProviderCatalog ? { preparedStaticProviderCatalog } : {}), @@ -373,18 +378,8 @@ export async function prepareWorkspaceBuildGroup( } const configuredGeneratedCatalogPluginIds = [ ...new Set( - facts.configuredModelRefs.flatMap(({ value }) => { - const separator = value.indexOf("/"); - if (separator <= 0 || separator >= value.length - 1) { - return []; - } - const provider = normalizeProviderId(value.slice(0, separator)); - const modelId = value.slice(separator + 1).trim(); - if ( - !provider || - !modelId || - configuredEntryKeys.has(modelCatalogEntryKey({ provider, id: modelId })) - ) { + facts.configuredModelRefs.flatMap(({ provider, modelId }) => { + if (configuredEntryKeys.has(modelCatalogEntryKey({ provider, id: modelId }))) { return []; } const pluginId = resolvePluginModelCatalogOwnerPluginId({ diff --git a/src/agents/prepared-model-runtime.startup-static.test.ts b/src/agents/prepared-model-runtime.startup-static.test.ts index 18e3191ea768..72b3d86c1ed0 100644 --- a/src/agents/prepared-model-runtime.startup-static.test.ts +++ b/src/agents/prepared-model-runtime.startup-static.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { createEmptyPluginRegistry } from "../plugins/registry-empty.js"; +import type { ModelRegistry } from "./sessions/model-registry.js"; type CreateStaticCatalogResolver = typeof import("./embedded-agent-runner/model.static-catalog.js").createBundledStaticCatalogModelResolver; @@ -29,7 +30,7 @@ const mocks = vi.hoisted(() => { const modelRegistry = { fork: vi.fn((nextAuthStorage: unknown) => ({ authStorage: nextAuthStorage })), getAll: vi.fn(() => []), - find: vi.fn(() => null), + find: vi.fn(() => undefined), }; const resolveSyntheticAuth = vi.fn(() => ({ apiKey: "synthetic-openai-key", @@ -227,6 +228,7 @@ beforeEach(() => { .mockReset() .mockReturnValue(createEmptyPluginRegistry()); vi.clearAllMocks(); + mocks.modelRegistry.find.mockReset(); mocks.resolveStaticCatalogModel.mockReturnValue(undefined); }); @@ -524,6 +526,22 @@ describe("prepared model runtime Gateway catalog mode", () => { source: "test", }); mocks.loadAgentRuntimePluginRegistryHandle.mockReturnValue(registry); + mocks.modelRegistry.find.mockImplementation((registryProvider, registryModelId) => + registryProvider === "registry-only" && registryModelId === "MIXED" + ? { + provider: registryProvider, + id: registryModelId, + name: "Exact-case registry model", + api: "openai-responses", + baseUrl: "https://registry.invalid/v1", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 32_000, + maxTokens: 4096, + } + : undefined, + ); const providerConfig = { api: "openai-responses" as const, baseUrl: "https://configured.fixture.invalid/v1", @@ -535,7 +553,14 @@ describe("prepared model runtime Gateway catalog mode", () => { defaults: { model: { primary: `${provider}/${modelId}`, - fallbacks: ["openai/gpt-5.5", `${provider}/${modelId}`], + fallbacks: [ + "openai/gpt-5.5", + `${provider}/${modelId}`, + "registry-only/mixed", + "REGISTRY-ONLY/MIXED", + "bare-alias", + "provider-only/", + ], }, }, }, @@ -578,24 +603,24 @@ describe("prepared model runtime Gateway catalog mode", () => { api: "openai-responses", baseUrl: "https://fixture.invalid/v1", }); - for (const entries of [ - snapshot?.modelCatalog.entries, - snapshot?.modelCatalog.routeVariants, - snapshot?.modelCatalog.staticEntries, - ]) { + for (const entries of [snapshot?.modelCatalog.entries, snapshot?.modelCatalog.routeVariants]) { expect(entries?.map((entry) => `${entry.provider}/${entry.id}`)).toEqual([ `${provider}/${modelId}`, "openai/gpt-5.5", + "registry-only/MIXED", ]); } + expect( + snapshot?.modelCatalog.staticEntries?.map((entry) => `${entry.provider}/${entry.id}`), + ).toEqual([`${provider}/${modelId}`, "openai/gpt-5.5"]); expect( snapshot?.modelCatalog.staticEntries?.find((entry) => entry.provider === "openai") ?.thinkingLevelMap, ).toEqual({ off: null, max: "max" }); expect(mocks.prepareStaticCatalog).toHaveBeenCalledWith( expect.objectContaining({ - providerDiscoveryProviderIds: [provider, "openai"], - staticCatalogProviderIds: [provider, "openai"], + providerDiscoveryProviderIds: [provider, "openai", "provider-only", "registry-only"], + staticCatalogProviderIds: [provider, "openai", "registry-only"], }), ); expect(mocks.discoverModels).toHaveBeenCalledOnce(); diff --git a/src/agents/sessions/model-registry.test.ts b/src/agents/sessions/model-registry.test.ts index 6e369dbed3a1..514c2b4db307 100644 --- a/src/agents/sessions/model-registry.test.ts +++ b/src/agents/sessions/model-registry.test.ts @@ -693,70 +693,76 @@ describe("ModelRegistry models.json auth", () => { expect(availableRefs).toContain("nvidia/explicit-empty"); }); - it("isolates invalid SQLite-cached plugin catalogs from valid models", () => { - const modelsPath = writeModelsJsonWithPluginCatalogs({ - root: { - providers: { - custom: { - baseUrl: "https://models.example/v1", - api: "openai-responses", - apiKey: "CUSTOM_API_KEY", - models: [{ id: "root-model", name: "Root Model" }], - }, - }, - }, - pluginCatalogs: [ - { - pluginRelativePath: join("plugins", "google", PLUGIN_MODEL_CATALOG_FILE), - pluginCatalog: { - generatedBy: PLUGIN_MODEL_CATALOG_GENERATED_BY, - providers: { - "google-vertex": { - baseUrl: "https://us-central1-aiplatform.googleapis.com/v1", - api: "google-vertex", - apiKey: "GOOGLE_API_KEY", - models: [ - { - id: "gemini-3.1-pro-preview", - name: "Gemini 3.1 Pro", - contextWindow: 0, - }, - ], - }, + it.each(["persisted", "captured"] as const)( + "isolates invalid %s plugin catalogs from valid models", + (source) => { + const modelsPath = writeModelsJsonWithPluginCatalogs({ + root: { + providers: { + custom: { + baseUrl: "https://models.example/v1", + api: "openai-responses", + apiKey: "CUSTOM_API_KEY", + models: [{ id: "root-model", name: "Root Model" }], }, }, }, - { - pluginRelativePath: join("plugins", "zai", PLUGIN_MODEL_CATALOG_FILE), - pluginCatalog: { - generatedBy: PLUGIN_MODEL_CATALOG_GENERATED_BY, - providers: { - zai: { - baseUrl: "https://api.z.ai/api/paas/v4", - api: "openai-completions", - apiKey: "ZAI_API_KEY", - models: [{ id: "glm-5.1", name: "GLM 5.1" }], + pluginCatalogs: [ + { + pluginRelativePath: join("plugins", "google", PLUGIN_MODEL_CATALOG_FILE), + pluginCatalog: { + generatedBy: PLUGIN_MODEL_CATALOG_GENERATED_BY, + providers: { + "google-vertex": { + baseUrl: "https://us-central1-aiplatform.googleapis.com/v1", + api: "google-vertex", + apiKey: "GOOGLE_API_KEY", + models: [ + { + id: "gemini-3.1-pro-preview", + name: "Gemini 3.1 Pro", + contextWindow: 0, + }, + ], + }, }, }, }, - }, - ], - }); + { + pluginRelativePath: join("plugins", "zai", PLUGIN_MODEL_CATALOG_FILE), + pluginCatalog: { + generatedBy: PLUGIN_MODEL_CATALOG_GENERATED_BY, + providers: { + zai: { + baseUrl: "https://api.z.ai/api/paas/v4", + api: "openai-completions", + apiKey: "ZAI_API_KEY", + models: [{ id: "glm-5.1", name: "GLM 5.1" }], + }, + }, + }, + }, + ], + }); - const registry = ModelRegistry.create(AuthStorage.inMemory(), modelsPath, { - pluginMetadataSnapshot: pluginOwnerSnapshotEntries([ - { providerId: "google-vertex", pluginId: "google" }, - { providerId: "zai", pluginId: "zai" }, - ]), - }); + const registry = ModelRegistry.create(AuthStorage.inMemory(), modelsPath, { + ...(source === "captured" + ? { pluginCatalogs: listPersistedPluginModelCatalogs(dirname(modelsPath)) } + : {}), + pluginMetadataSnapshot: pluginOwnerSnapshotEntries([ + { providerId: "google-vertex", pluginId: "google" }, + { providerId: "zai", pluginId: "zai" }, + ]), + }); - expect(registry.getError()).toContain( - "Provider google-vertex, model gemini-3.1-pro-preview: invalid contextWindow", - ); - expect(registry.find("custom", "root-model")?.name).toBe("Root Model"); - expect(registry.find("zai", "glm-5.1")?.name).toBe("GLM 5.1"); - expect(registry.find("google-vertex", "gemini-3.1-pro-preview")).toBeUndefined(); - }); + expect(registry.getError()).toContain( + "Provider google-vertex, model gemini-3.1-pro-preview: invalid contextWindow", + ); + expect(registry.find("custom", "root-model")?.name).toBe("Root Model"); + expect(registry.find("zai", "glm-5.1")?.name).toBe("GLM 5.1"); + expect(registry.find("google-vertex", "gemini-3.1-pro-preview")).toBeUndefined(); + }, + ); it("repairs missing-api generated rows before repeated registry loads", () => { const modelsPath = writeModelsJsonWithPluginCatalogs({ diff --git a/src/agents/sessions/model-registry.ts b/src/agents/sessions/model-registry.ts index c12162e732d9..2fb660e53f2d 100644 --- a/src/agents/sessions/model-registry.ts +++ b/src/agents/sessions/model-registry.ts @@ -38,7 +38,6 @@ import { } from "./model-registry-runtime.js"; import { BUILT_IN_PROVIDER_DISPLAY_NAMES } from "./provider-display-names.js"; import { - clearConfigValueCache, resolveConfigValueOrThrow, resolveConfigValueUncached, resolveHeadersOrThrow, @@ -312,9 +311,6 @@ function mergeCompat( return merged as Model["compat"]; } -/** Clear the config value command cache. Exported for testing. */ -export const clearApiKeyCache = clearConfigValueCache; - /** * Model registry - loads and manages models, resolves API keys via AuthStorage. */ @@ -583,13 +579,9 @@ export class ModelRegistry { if (options.includePluginCatalogs !== false) { let pluginCatalogs: readonly PersistedPluginModelCatalog[] = []; try { - if (this.pluginCatalogs) { - pluginCatalogs = this.pluginCatalogs; - } else { - const loaded = loadPersistedPluginModelCatalogs(dirname(modelsJsonPath)); - pluginCatalogs = loaded.catalogs; - pluginCatalogErrors.push(...loaded.warnings); - } + const loaded = loadPersistedPluginModelCatalogs(dirname(modelsJsonPath)); + pluginCatalogs = loaded.catalogs; + pluginCatalogErrors.push(...loaded.warnings); } catch (error) { pluginCatalogErrors.push( `Failed to load generated plugin model catalogs: ${ @@ -597,21 +589,10 @@ export class ModelRegistry { }`, ); } - for (const pluginCatalog of pluginCatalogs) { - const pluginResult = this.loadCustomModels( - `sqlite:plugin-model-catalog/${pluginCatalog.pluginId}`, - { - catalogPluginId: pluginCatalog.pluginId, - contents: pluginCatalog.contents, - includePluginCatalogs: false, - requireGeneratedCatalog: true, - }, - ); - if (pluginResult.error) { - pluginCatalogErrors.push(pluginResult.error); - continue; - } - models.push(...pluginResult.models); + const pluginResult = this.loadCapturedPluginCatalogs(pluginCatalogs); + models.push(...pluginResult.models); + if (pluginResult.error) { + pluginCatalogErrors.push(pluginResult.error); } } diff --git a/src/agents/sessions/resolve-config-value.ts b/src/agents/sessions/resolve-config-value.ts index 2e891488c67b..423b635ce829 100644 --- a/src/agents/sessions/resolve-config-value.ts +++ b/src/agents/sessions/resolve-config-value.ts @@ -135,8 +135,3 @@ export function resolveHeadersOrThrow( } return Object.keys(resolved).length > 0 ? resolved : undefined; } - -/** Clear the config value command cache. Exported for testing. */ -export function clearConfigValueCache(): void { - commandResultCache.clear(); -}