diff --git a/src/agents/model-catalog.test.ts b/src/agents/model-catalog.test.ts index 0eeca0d6977..60df3fd4ef7 100644 --- a/src/agents/model-catalog.test.ts +++ b/src/agents/model-catalog.test.ts @@ -19,6 +19,11 @@ vi.mock("./model-suppression.runtime.js", () => ({ params.provider === "azure-openai-responses" || params.provider === "openai-codex") && params.id === "gpt-5.3-codex-spark", + buildShouldSuppressBuiltInModel: () => (params: { provider?: string; id?: string }) => + (params.provider === "openai" || + params.provider === "azure-openai-responses" || + params.provider === "openai-codex") && + params.id === "gpt-5.3-codex-spark", })); function mockCatalogImportFailThenRecover() { diff --git a/src/agents/model-catalog.ts b/src/agents/model-catalog.ts index a039b2a6a37..b30bb4e4993 100644 --- a/src/agents/model-catalog.ts +++ b/src/agents/model-catalog.ts @@ -149,7 +149,7 @@ export async function loadModelCatalog(params?: { const piSdk = await importPiSdk(); logStage("pi-sdk-imported"); const agentDir = resolveOpenClawAgentDir(); - const { shouldSuppressBuiltInModel } = await loadModelSuppression(); + const { buildShouldSuppressBuiltInModel } = await loadModelSuppression(); logStage("catalog-deps-ready"); const authStorage = piSdk.discoverAuthStorage( agentDir, @@ -164,6 +164,10 @@ export async function loadModelCatalog(params?: { logStage("registry-ready"); const entries = Array.isArray(registry) ? registry : registry.getAll(); logStage("registry-read", `entries=${entries.length}`); + + const shouldSuppressBuiltInModel = buildShouldSuppressBuiltInModel({ config: cfg }); + logStage("suppress-resolver-ready"); + for (const entry of entries) { const id = normalizeOptionalString(entry?.id) ?? ""; if (!id) { @@ -173,7 +177,7 @@ export async function loadModelCatalog(params?: { if (!provider) { continue; } - if (shouldSuppressBuiltInModel({ provider, id, config: cfg })) { + if (shouldSuppressBuiltInModel({ provider, id })) { continue; } const name = normalizeOptionalString(entry?.name ?? id) || id; diff --git a/src/agents/model-suppression.runtime.ts b/src/agents/model-suppression.runtime.ts index 7d39bf2b8a3..7d76de597db 100644 --- a/src/agents/model-suppression.runtime.ts +++ b/src/agents/model-suppression.runtime.ts @@ -1,10 +1,21 @@ -import { shouldSuppressBuiltInModel as shouldSuppressBuiltInModelImpl } from "./model-suppression.js"; +import { + buildShouldSuppressBuiltInModel as buildShouldSuppressBuiltInModelImpl, + shouldSuppressBuiltInModel as shouldSuppressBuiltInModelImpl, +} from "./model-suppression.js"; type ShouldSuppressBuiltInModel = typeof import("./model-suppression.js").shouldSuppressBuiltInModel; +type BuildShouldSuppressBuiltInModel = + typeof import("./model-suppression.js").buildShouldSuppressBuiltInModel; export function shouldSuppressBuiltInModel( ...args: Parameters ): ReturnType { return shouldSuppressBuiltInModelImpl(...args); } + +export function buildShouldSuppressBuiltInModel( + ...args: Parameters +): ReturnType { + return buildShouldSuppressBuiltInModelImpl(...args); +} diff --git a/src/agents/model-suppression.test.ts b/src/agents/model-suppression.test.ts index 0ad467392d3..3f091144b2b 100644 --- a/src/agents/model-suppression.test.ts +++ b/src/agents/model-suppression.test.ts @@ -1,17 +1,23 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ + buildManifestBuiltInModelSuppressionResolver: vi.fn(), resolveManifestBuiltInModelSuppression: vi.fn(), })); vi.mock("../plugins/manifest-model-suppression.js", () => ({ + buildManifestBuiltInModelSuppressionResolver: mocks.buildManifestBuiltInModelSuppressionResolver, resolveManifestBuiltInModelSuppression: mocks.resolveManifestBuiltInModelSuppression, })); -import { shouldSuppressBuiltInModel } from "./model-suppression.js"; +import { + buildShouldSuppressBuiltInModel, + shouldSuppressBuiltInModel, +} from "./model-suppression.js"; describe("model suppression", () => { beforeEach(() => { + mocks.buildManifestBuiltInModelSuppressionResolver.mockReset(); mocks.resolveManifestBuiltInModelSuppression.mockReset(); }); @@ -43,4 +49,48 @@ describe("model suppression", () => { expect(mocks.resolveManifestBuiltInModelSuppression).toHaveBeenCalledOnce(); }); + + describe("buildShouldSuppressBuiltInModel", () => { + beforeEach(() => { + mocks.buildManifestBuiltInModelSuppressionResolver.mockReset(); + }); + + it("creates a reusable manifest resolver with normalized provider and model ids", () => { + const resolver = vi + .fn() + .mockReturnValueOnce({ suppress: true, errorMessage: "manifest suppression" }) + .mockReturnValueOnce(undefined); + const config = {}; + mocks.buildManifestBuiltInModelSuppressionResolver.mockReturnValueOnce(resolver); + + const shouldSuppress = buildShouldSuppressBuiltInModel({ config }); + + expect(shouldSuppress({ provider: "bedrock", id: "Claude-3" })).toBe(true); + expect(shouldSuppress({ provider: "aws-bedrock", id: "claude-4" })).toBe(false); + expect(mocks.buildManifestBuiltInModelSuppressionResolver).toHaveBeenCalledOnce(); + expect(mocks.buildManifestBuiltInModelSuppressionResolver).toHaveBeenCalledWith({ + config, + env: process.env, + }); + expect(resolver).toHaveBeenNthCalledWith(1, { + provider: "amazon-bedrock", + id: "claude-3", + }); + expect(resolver).toHaveBeenNthCalledWith(2, { + provider: "amazon-bedrock", + id: "claude-4", + }); + }); + + it("does not call the manifest resolver for empty provider or model ids", () => { + const resolver = vi.fn(); + mocks.buildManifestBuiltInModelSuppressionResolver.mockReturnValueOnce(resolver); + + const shouldSuppress = buildShouldSuppressBuiltInModel({}); + + expect(shouldSuppress({ provider: "openai", id: "" })).toBe(false); + expect(shouldSuppress({ provider: "", id: "gpt-5.5" })).toBe(false); + expect(resolver).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/agents/model-suppression.ts b/src/agents/model-suppression.ts index 7ab86ef8321..d95be529f90 100644 --- a/src/agents/model-suppression.ts +++ b/src/agents/model-suppression.ts @@ -1,5 +1,8 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { resolveManifestBuiltInModelSuppression } from "../plugins/manifest-model-suppression.js"; +import { + buildManifestBuiltInModelSuppressionResolver, + resolveManifestBuiltInModelSuppression, +} from "../plugins/manifest-model-suppression.js"; import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; import { normalizeProviderId } from "./provider-id.js"; @@ -66,3 +69,21 @@ export function buildSuppressedBuiltInModelError(params: { }): string | undefined { return resolveBuiltInModelSuppression(params)?.errorMessage; } + +export function buildShouldSuppressBuiltInModel(params: { + config?: OpenClawConfig; +}): (input: { provider?: string | null; id?: string | null; baseUrl?: string | null }) => boolean { + const resolver = buildManifestBuiltInModelSuppressionResolver({ + config: params.config, + env: process.env, + }); + + return (input) => { + const provider = normalizeProviderId(input.provider ?? ""); + const id = normalizeLowercaseStringOrEmpty(input.id); + if (!provider || !id) { + return false; + } + return resolver({ ...input, provider, id })?.suppress ?? false; + }; +} diff --git a/src/plugins/manifest-model-suppression.test.ts b/src/plugins/manifest-model-suppression.test.ts index a755c4c3288..29e281a6aa0 100644 --- a/src/plugins/manifest-model-suppression.test.ts +++ b/src/plugins/manifest-model-suppression.test.ts @@ -9,6 +9,7 @@ vi.mock("./plugin-registry.js", () => ({ })); import { + buildManifestBuiltInModelSuppressionResolver, clearManifestModelSuppressionCacheForTest, resolveManifestBuiltInModelSuppression, } from "./manifest-model-suppression.js"; @@ -46,6 +47,30 @@ describe("manifest model suppression", () => { }); }); + describe("buildManifestBuiltInModelSuppressionResolver", () => { + it("reads planned manifest suppressions once per resolver creation", () => { + const config = { plugins: { entries: { openai: { enabled: true } } } }; + + const resolver = buildManifestBuiltInModelSuppressionResolver({ + config, + env: process.env, + }); + + expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(1); + + resolver({ + provider: "azure-openai-responses", + id: "gpt-5.3-codex-spark", + }); + resolver({ + provider: "azure-openai-responses", + id: "gpt-5.3-codex-spark", + }); + + expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(1); + }); + }); + it("resolves manifest suppressions for declared provider aliases", () => { expect( resolveManifestBuiltInModelSuppression({ @@ -89,6 +114,29 @@ describe("manifest model suppression", () => { expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(2); }); + it("reuses planned manifest suppressions inside a resolver instance", () => { + const config = { plugins: { entries: { openai: { enabled: true } } } }; + + const resolver = buildManifestBuiltInModelSuppressionResolver({ + config, + env: process.env, + }); + + expect( + resolver({ + provider: "azure-openai-responses", + id: "gpt-5.3-codex-spark", + })?.suppress, + ).toBe(true); + expect( + resolver({ + provider: "azure-openai-responses", + id: "gpt-4.1", + }), + ).toBeUndefined(); + expect(mocks.loadPluginManifestRegistryForPluginRegistry).toHaveBeenCalledTimes(1); + }); + it("matches conditional suppressions by base URL host", () => { mocks.loadPluginManifestRegistryForPluginRegistry.mockReturnValue({ diagnostics: [], diff --git a/src/plugins/manifest-model-suppression.ts b/src/plugins/manifest-model-suppression.ts index 98da5285aae..3aa2df28cf4 100644 --- a/src/plugins/manifest-model-suppression.ts +++ b/src/plugins/manifest-model-suppression.ts @@ -103,6 +103,59 @@ export function clearManifestModelSuppressionCacheForTest(): void { // Manifest suppressions are read fresh. Keep the test hook as a no-op. } +export function buildManifestBuiltInModelSuppressionResolver(params: { + config?: OpenClawConfig; + workspaceDir?: string; + env?: NodeJS.ProcessEnv; +}) { + const suppressions = listManifestModelCatalogSuppressions({ + config: params.config, + workspaceDir: params.workspaceDir, + env: params.env ?? process.env, + }); + + return (input: { + provider?: string | null; + id?: string | null; + baseUrl?: string | null; + }) => { + const provider = normalizeLowercaseStringOrEmpty(input.provider); + const modelId = normalizeLowercaseStringOrEmpty(input.id); + if (!provider || !modelId) { + return undefined; + } + const mergeKey = buildModelCatalogMergeKey(provider, modelId); + const suppression = suppressions.find( + (entry) => + entry.mergeKey === mergeKey && + manifestSuppressionMatchesConditions({ + suppression: entry, + provider, + baseUrl: input.baseUrl, + config: params.config, + }), + ); + if (!suppression) { + return undefined; + } + return { + suppress: true, + errorMessage: buildManifestSuppressionError({ + provider, + modelId, + reason: suppression.reason, + }), + }; + }; +} + +/** + * Resolves whether a built-in model should be suppressed based on manifest declarations. + * + * Note: This function instantiates a fresh resolver on every call, which incurs a full + * filesystem scan of the manifest registry. For hot paths (like building the model catalog), + * instantiate and reuse `buildManifestBuiltInModelSuppressionResolver` instead. + */ export function resolveManifestBuiltInModelSuppression(params: { provider?: string | null; id?: string | null; @@ -111,35 +164,14 @@ export function resolveManifestBuiltInModelSuppression(params: { env?: NodeJS.ProcessEnv; baseUrl?: string | null; }) { - const provider = normalizeLowercaseStringOrEmpty(params.provider); - const modelId = normalizeLowercaseStringOrEmpty(params.id); - if (!provider || !modelId) { - return undefined; - } - const mergeKey = buildModelCatalogMergeKey(provider, modelId); - const suppression = listManifestModelCatalogSuppressions({ + const resolver = buildManifestBuiltInModelSuppressionResolver({ config: params.config, workspaceDir: params.workspaceDir, - env: params.env ?? process.env, - }).find( - (entry) => - entry.mergeKey === mergeKey && - manifestSuppressionMatchesConditions({ - suppression: entry, - provider, - baseUrl: params.baseUrl, - config: params.config, - }), - ); - if (!suppression) { - return undefined; - } - return { - suppress: true, - errorMessage: buildManifestSuppressionError({ - provider, - modelId, - reason: suppression.reason, - }), - }; + env: params.env, + }); + return resolver({ + provider: params.provider, + id: params.id, + baseUrl: params.baseUrl, + }); }