fix(catalog): preserve cached suppression semantics

vincentkoc · vincentkoc · commit a2247a5aec6b · 2026-04-29T02:36:54.000-07:00
diff --git a/src/agents/model-suppression.test.ts b/src/agents/model-suppression.test.ts
@@ -1,19 +1,23 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
 const mocks = vi.hoisted(() => ({
-  resolveManifestBuiltInModelSuppression: vi.fn(),
   buildManifestBuiltInModelSuppressionResolver: vi.fn(),
+  resolveManifestBuiltInModelSuppression: vi.fn(),
 }));
 
 vi.mock("../plugins/manifest-model-suppression.js", () => ({
-  resolveManifestBuiltInModelSuppression: mocks.resolveManifestBuiltInModelSuppression,
   buildManifestBuiltInModelSuppressionResolver: mocks.buildManifestBuiltInModelSuppressionResolver,
+  resolveManifestBuiltInModelSuppression: mocks.resolveManifestBuiltInModelSuppression,
 }));
 
-import { buildShouldSuppressBuiltInModel, shouldSuppressBuiltInModel } from "./model-suppression.js";
+import {
+  buildShouldSuppressBuiltInModel,
+  shouldSuppressBuiltInModel,
+} from "./model-suppression.js";
 
 describe("model suppression", () => {
   beforeEach(() => {
+    mocks.buildManifestBuiltInModelSuppressionResolver.mockReset();
     mocks.resolveManifestBuiltInModelSuppression.mockReset();
   });
 
@@ -51,18 +55,42 @@ describe("model suppression", () => {
       mocks.buildManifestBuiltInModelSuppressionResolver.mockReset();
     });
 
-    it("normalizes provider aliases before checking suppressions", () => {
-      const resolverMock = vi.fn().mockReturnValue({ suppress: true });
-      mocks.buildManifestBuiltInModelSuppressionResolver.mockReturnValueOnce(resolverMock);
+    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 predicate = buildShouldSuppressBuiltInModel({ config: {} });
-      
-      expect(predicate({ provider: "bedrock", id: "anthropic.claude-3-5-sonnet" })).toBe(true);
+      const shouldSuppress = buildShouldSuppressBuiltInModel({});
 
-      expect(resolverMock).toHaveBeenCalledOnce();
-      expect(resolverMock).toHaveBeenCalledWith(
-        expect.objectContaining({ provider: "amazon-bedrock", id: "anthropic.claude-3-5-sonnet" })
-      );
+      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
@@ -72,19 +72,18 @@ export function buildSuppressedBuiltInModelError(params: {
 
 export function buildShouldSuppressBuiltInModel(params: {
   config?: OpenClawConfig;
-}): (input: {
-  provider?: string | null;
-  id?: string | null;
-  baseUrl?: string | null;
-}) => boolean {
+}): (input: { provider?: string | null; id?: string | null; baseUrl?: string | null }) => boolean {
   const resolver = buildManifestBuiltInModelSuppressionResolver({
     config: params.config,
     env: process.env,
   });
 
   return (input) => {
-    return (
-      resolver({ ...input, provider: normalizeProviderId(input.provider ?? "") })?.suppress ?? false
-    );
+    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
@@ -114,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: [],
