From 2c1be64d977d9a107a891733ec5aa978bf7902b8 Mon Sep 17 00:00:00 2001 From: Shakker Date: Tue, 28 Apr 2026 02:37:32 +0100 Subject: [PATCH] fix: keep refreshable manifest catalogs registry backed --- src/commands/models.list.e2e.test.ts | 20 +++-- .../list.list-command.forward-compat.test.ts | 79 +++++++++++++++++-- .../models/list.manifest-catalog.test.ts | 12 +-- src/commands/models/list.manifest-catalog.ts | 43 +++++++--- src/commands/models/list.source-plan.test.ts | 38 +++++---- src/commands/models/list.source-plan.ts | 32 ++++---- 6 files changed, 162 insertions(+), 62 deletions(-) diff --git a/src/commands/models.list.e2e.test.ts b/src/commands/models.list.e2e.test.ts index d77c110f4f1..37223f34bd3 100644 --- a/src/commands/models.list.e2e.test.ts +++ b/src/commands/models.list.e2e.test.ts @@ -22,6 +22,9 @@ const loadProviderCatalogModelsForList = vi.fn<() => Promise [], ); const loadStaticManifestCatalogRowsForList = vi.fn<() => Array>>(() => []); +const loadSupplementalManifestCatalogRowsForList = vi.fn<() => Array>>( + () => [], +); const loadProviderIndexCatalogRowsForList = vi.fn<() => Array>>(() => []); const hasProviderStaticCatalogForFilter = vi.fn().mockResolvedValue(false); const shouldSuppressBuiltInModel = vi.fn().mockReturnValue(false); @@ -196,6 +199,7 @@ vi.mock("./models/list.provider-catalog.js", async (importOriginal) => { vi.mock("./models/list.manifest-catalog.js", () => ({ loadStaticManifestCatalogRowsForList, + loadSupplementalManifestCatalogRowsForList, })); vi.mock("./models/list.provider-index-catalog.js", () => ({ @@ -254,6 +258,8 @@ beforeEach(() => { loadProviderCatalogModelsForList.mockResolvedValue([]); loadStaticManifestCatalogRowsForList.mockReset(); loadStaticManifestCatalogRowsForList.mockReturnValue([]); + loadSupplementalManifestCatalogRowsForList.mockReset(); + loadSupplementalManifestCatalogRowsForList.mockReturnValue([]); loadProviderIndexCatalogRowsForList.mockReset(); loadProviderIndexCatalogRowsForList.mockReturnValue([]); hasProviderStaticCatalogForFilter.mockReset(); @@ -565,13 +571,13 @@ describe("models list/status", () => { }); it("filters stale spark rows from models list and registry views", async () => { - shouldSuppressBuiltInModel.mockImplementation( - ({ provider, id }: { provider?: string | null; id?: string | null }) => - id === "gpt-5.3-codex-spark" && - (provider === "openai" || - provider === "azure-openai-responses" || - provider === "openai-codex"), - ); + const suppressSpark = ({ provider, id }: { provider?: string | null; id?: string | null }) => + id === "gpt-5.3-codex-spark" && + (provider === "openai" || + provider === "azure-openai-responses" || + provider === "openai-codex"); + shouldSuppressBuiltInModel.mockImplementation(suppressSpark); + shouldSuppressBuiltInModelFromManifest.mockImplementation(suppressSpark); setDefaultModel("openai/gpt-5.5"); modelRegistryState.models = [OPENAI_MODEL, OPENAI_SPARK_MODEL, AZURE_OPENAI_SPARK_MODEL]; modelRegistryState.available = [OPENAI_MODEL, OPENAI_SPARK_MODEL, AZURE_OPENAI_SPARK_MODEL]; diff --git a/src/commands/models/list.list-command.forward-compat.test.ts b/src/commands/models/list.list-command.forward-compat.test.ts index 9363e003bdf..f7ec57d337c 100644 --- a/src/commands/models/list.list-command.forward-compat.test.ts +++ b/src/commands/models/list.list-command.forward-compat.test.ts @@ -49,7 +49,8 @@ const mocks = vi.hoisted(() => { loadModelRegistry: vi.fn(), loadModelCatalog: vi.fn(), loadProviderCatalogModelsForList: vi.fn(), - loadManifestCatalogRowsForList: vi.fn(), + loadStaticManifestCatalogRowsForList: vi.fn(), + loadSupplementalManifestCatalogRowsForList: vi.fn(), loadProviderIndexCatalogRowsForList: vi.fn(), hasProviderStaticCatalogForFilter: vi.fn(), resolveConfiguredEntries: vi.fn(), @@ -78,7 +79,8 @@ function resetMocks() { }); mocks.loadModelCatalog.mockResolvedValue([]); mocks.loadProviderCatalogModelsForList.mockResolvedValue([]); - mocks.loadManifestCatalogRowsForList.mockReturnValue([]); + mocks.loadStaticManifestCatalogRowsForList.mockReturnValue([]); + mocks.loadSupplementalManifestCatalogRowsForList.mockReturnValue([]); mocks.loadProviderIndexCatalogRowsForList.mockReturnValue([]); mocks.hasProviderStaticCatalogForFilter.mockResolvedValue(false); mocks.resolveConfiguredEntries.mockReturnValue({ @@ -143,7 +145,8 @@ function installModelsListCommandForwardCompatMocks() { })); vi.doMock("./list.manifest-catalog.js", () => ({ - loadManifestCatalogRowsForList: mocks.loadManifestCatalogRowsForList, + loadStaticManifestCatalogRowsForList: mocks.loadStaticManifestCatalogRowsForList, + loadSupplementalManifestCatalogRowsForList: mocks.loadSupplementalManifestCatalogRowsForList, })); vi.doMock("./list.provider-index-catalog.js", () => ({ @@ -525,7 +528,7 @@ describe("modelsListCommand forward-compat", () => { it("uses manifest catalog rows before provider runtime catalog rows", async () => { mocks.resolveConfiguredEntries.mockReturnValueOnce({ entries: [] }); - mocks.loadManifestCatalogRowsForList.mockReturnValueOnce([ + mocks.loadStaticManifestCatalogRowsForList.mockReturnValueOnce([ { provider: "moonshot", id: "kimi-k2.6", @@ -555,6 +558,72 @@ describe("modelsListCommand forward-compat", () => { ]); }); + it("keeps refreshable manifest catalog rows on the registry-backed provider path", async () => { + mocks.resolveConfiguredEntries.mockReturnValueOnce({ entries: [] }); + mocks.loadSupplementalManifestCatalogRowsForList.mockReturnValueOnce([ + { + provider: "openai", + id: "gpt-5.5-pro", + ref: "openai/gpt-5.5-pro", + mergeKey: "openai::gpt-5.5-pro", + name: "gpt-5.5-pro", + source: "manifest", + input: ["text", "image"], + reasoning: true, + status: "available", + baseUrl: "https://api.openai.com/v1", + contextWindow: 1_000_000, + }, + ]); + mocks.loadModelRegistry.mockResolvedValueOnce({ + models: [ + { + provider: "openai", + id: "gpt-5.4", + name: "GPT-5.4", + api: "openai-responses", + baseUrl: "https://api.openai.com/v1", + input: ["text", "image"], + contextWindow: 1_050_000, + maxTokens: 128_000, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + }, + ], + availableKeys: new Set(), + registry: { + getAll: () => [], + }, + }); + mocks.resolveModelWithRegistry.mockImplementation( + ({ provider, modelId }: { provider: string; modelId: string }) => + provider === "openai" && modelId === "gpt-5.4" + ? { + provider, + id: modelId, + name: "GPT-5.4", + api: "openai-responses", + baseUrl: "https://api.openai.com/v1", + input: ["text", "image"], + contextWindow: 1_050_000, + maxTokens: 128_000, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + } + : undefined, + ); + const runtime = createRuntime(); + + await modelsListCommand({ all: true, provider: "openai", json: true }, runtime as never); + + expect(mocks.loadModelRegistry).toHaveBeenCalledWith(mocks.resolvedConfig, { + providerFilter: "openai", + normalizeModels: true, + }); + expect(lastPrintedRows<{ key: string }>()).toEqual([ + expect.objectContaining({ key: "openai/gpt-5.4" }), + expect.objectContaining({ key: "openai/gpt-5.5-pro" }), + ]); + }); + it("uses provider index preview rows when an installable provider is not installed", async () => { mocks.resolveConfiguredEntries.mockReturnValueOnce({ entries: [] }); mocks.loadProviderIndexCatalogRowsForList.mockReturnValueOnce([ @@ -596,7 +665,7 @@ describe("modelsListCommand forward-compat", () => { getAll: () => [{ ...OPENAI_CODEX_MODEL }], }, }); - mocks.loadManifestCatalogRowsForList.mockReturnValueOnce([ + mocks.loadSupplementalManifestCatalogRowsForList.mockReturnValueOnce([ { provider: "moonshot", id: "kimi-k2.6", diff --git a/src/commands/models/list.manifest-catalog.test.ts b/src/commands/models/list.manifest-catalog.test.ts index 868edb6a811..7fc693baeac 100644 --- a/src/commands/models/list.manifest-catalog.test.ts +++ b/src/commands/models/list.manifest-catalog.test.ts @@ -50,7 +50,7 @@ const openrouterPlugin = { }; describe("loadStaticManifestCatalogRowsForList", () => { - it("loads listable manifest catalog rows without a provider filter", async () => { + it("loads only static manifest catalog rows without a provider filter", async () => { const { loadStaticManifestCatalogRowsForList } = await import("./list.manifest-catalog.js"); const index = { plugins: [], diagnostics: [] }; mocks.loadPluginRegistrySnapshot.mockReturnValueOnce(index); @@ -63,7 +63,7 @@ describe("loadStaticManifestCatalogRowsForList", () => { loadStaticManifestCatalogRowsForList({ cfg: {}, }).map((row) => row.ref), - ).toEqual(["moonshot/kimi-k2.6", "openrouter/auto"]); + ).toEqual(["moonshot/kimi-k2.6"]); expect(mocks.loadPluginManifestRegistryForInstalledIndex).toHaveBeenCalledWith({ index, config: {}, @@ -71,8 +71,9 @@ describe("loadStaticManifestCatalogRowsForList", () => { }); }); - it("can load refreshable manifest rows for broad registry-backed lists", async () => { - const { loadManifestCatalogRowsForList } = await import("./list.manifest-catalog.js"); + it("loads refreshable manifest rows as registry-backed supplements", async () => { + const { loadSupplementalManifestCatalogRowsForList } = + await import("./list.manifest-catalog.js"); mocks.loadPluginRegistrySnapshot.mockReturnValueOnce({ plugins: [], diagnostics: [] }); mocks.loadPluginManifestRegistryForInstalledIndex.mockReturnValueOnce({ plugins: [openrouterPlugin, moonshotPlugin], @@ -80,9 +81,8 @@ describe("loadStaticManifestCatalogRowsForList", () => { }); expect( - loadManifestCatalogRowsForList({ + loadSupplementalManifestCatalogRowsForList({ cfg: {}, - staticOnly: false, }).map((row) => row.ref), ).toEqual(["moonshot/kimi-k2.6", "openrouter/auto"]); }); diff --git a/src/commands/models/list.manifest-catalog.ts b/src/commands/models/list.manifest-catalog.ts index fe0a1921f86..8b44d09a3f7 100644 --- a/src/commands/models/list.manifest-catalog.ts +++ b/src/commands/models/list.manifest-catalog.ts @@ -13,13 +13,15 @@ import { type PluginRegistrySnapshot, } from "../../plugins/plugin-registry.js"; +type ManifestCatalogRowsForListMode = "static-authoritative" | "supplemental"; + function loadManifestCatalogRowsForPluginIds(params: { cfg: OpenClawConfig; env?: NodeJS.ProcessEnv; index: PluginRegistrySnapshot; + mode: ManifestCatalogRowsForListMode; pluginIds?: readonly string[]; providerFilter?: string; - staticOnly?: boolean; }): readonly NormalizedModelCatalogRow[] { if (params.pluginIds && params.pluginIds.length === 0) { return []; @@ -34,16 +36,19 @@ function loadManifestCatalogRowsForPluginIds(params: { registry, ...(params.providerFilter ? { providerFilter: params.providerFilter } : {}), }); - if (params.staticOnly === false) { - return plan.rows; - } - const listableProviders = new Set( - plan.entries.filter((entry) => entry.discovery !== "runtime").map((entry) => entry.provider), + const eligibleProviders = new Set( + plan.entries + .filter((entry) => + params.mode === "static-authoritative" + ? entry.discovery === "static" + : entry.discovery !== "runtime", + ) + .map((entry) => entry.provider), ); - if (listableProviders.size === 0) { + if (eligibleProviders.size === 0) { return []; } - return plan.rows.filter((row) => listableProviders.has(row.provider)); + return plan.rows.filter((row) => eligibleProviders.has(row.provider)); } function resolveConventionModelCatalogPluginIds(params: { @@ -85,11 +90,12 @@ export function loadManifestCatalogRowsForList(params: { cfg: OpenClawConfig; providerFilter?: string; env?: NodeJS.ProcessEnv; - staticOnly?: boolean; + mode?: ManifestCatalogRowsForListMode; }): readonly NormalizedModelCatalogRow[] { const providerFilter = params.providerFilter ? normalizeModelCatalogProviderId(params.providerFilter) : undefined; + const mode = params.mode ?? "static-authoritative"; const index = loadPluginRegistrySnapshot({ config: params.cfg, env: params.env, @@ -99,20 +105,20 @@ export function loadManifestCatalogRowsForList(params: { cfg: params.cfg, env: params.env, index, - staticOnly: params.staticOnly, + mode, }); } const conventionRows = loadManifestCatalogRowsForPluginIds({ cfg: params.cfg, env: params.env, index, + mode, pluginIds: resolveConventionModelCatalogPluginIds({ cfg: params.cfg, index, providerFilter, }), providerFilter, - staticOnly: params.staticOnly, }); if (conventionRows.length > 0) { return conventionRows; @@ -121,13 +127,13 @@ export function loadManifestCatalogRowsForList(params: { cfg: params.cfg, env: params.env, index, + mode, pluginIds: resolveDeclaredModelCatalogPluginIds({ cfg: params.cfg, index, providerFilter, }), providerFilter, - staticOnly: params.staticOnly, }); } @@ -138,6 +144,17 @@ export function loadStaticManifestCatalogRowsForList(params: { }): readonly NormalizedModelCatalogRow[] { return loadManifestCatalogRowsForList({ ...params, - staticOnly: true, + mode: "static-authoritative", + }); +} + +export function loadSupplementalManifestCatalogRowsForList(params: { + cfg: OpenClawConfig; + providerFilter?: string; + env?: NodeJS.ProcessEnv; +}): readonly NormalizedModelCatalogRow[] { + return loadManifestCatalogRowsForList({ + ...params, + mode: "supplemental", }); } diff --git a/src/commands/models/list.source-plan.test.ts b/src/commands/models/list.source-plan.test.ts index c85174af48b..22e0ca2d55c 100644 --- a/src/commands/models/list.source-plan.test.ts +++ b/src/commands/models/list.source-plan.test.ts @@ -1,13 +1,15 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ - loadManifestCatalogRowsForList: vi.fn(), + loadStaticManifestCatalogRowsForList: vi.fn(), + loadSupplementalManifestCatalogRowsForList: vi.fn(), loadProviderIndexCatalogRowsForList: vi.fn(), hasProviderStaticCatalogForFilter: vi.fn(), })); vi.mock("./list.manifest-catalog.js", () => ({ - loadManifestCatalogRowsForList: mocks.loadManifestCatalogRowsForList, + loadStaticManifestCatalogRowsForList: mocks.loadStaticManifestCatalogRowsForList, + loadSupplementalManifestCatalogRowsForList: mocks.loadSupplementalManifestCatalogRowsForList, })); vi.mock("./list.provider-index-catalog.js", () => ({ @@ -33,14 +35,15 @@ const catalogRow = { describe("planAllModelListSources", () => { beforeEach(() => { vi.clearAllMocks(); - mocks.loadManifestCatalogRowsForList.mockReturnValue([]); + mocks.loadStaticManifestCatalogRowsForList.mockReturnValue([]); + mocks.loadSupplementalManifestCatalogRowsForList.mockReturnValue([]); mocks.loadProviderIndexCatalogRowsForList.mockReturnValue([]); mocks.hasProviderStaticCatalogForFilter.mockResolvedValue(false); }); it("uses installed manifest rows before provider index or runtime catalog sources", async () => { const { planAllModelListSources } = await import("./list.source-plan.js"); - mocks.loadManifestCatalogRowsForList.mockReturnValueOnce([catalogRow]); + mocks.loadStaticManifestCatalogRowsForList.mockReturnValueOnce([catalogRow]); const plan = await planAllModelListSources({ all: true, @@ -54,11 +57,11 @@ describe("planAllModelListSources", () => { skipRuntimeModelSuppression: true, }); expect(plan.manifestCatalogRows).toEqual([catalogRow]); - expect(mocks.loadManifestCatalogRowsForList).toHaveBeenCalledWith({ + expect(mocks.loadStaticManifestCatalogRowsForList).toHaveBeenCalledWith({ cfg: {}, providerFilter: "moonshot", - staticOnly: true, }); + expect(mocks.loadSupplementalManifestCatalogRowsForList).not.toHaveBeenCalled(); expect(mocks.loadProviderIndexCatalogRowsForList).not.toHaveBeenCalled(); expect(mocks.hasProviderStaticCatalogForFilter).not.toHaveBeenCalled(); }); @@ -83,9 +86,9 @@ describe("planAllModelListSources", () => { expect(mocks.hasProviderStaticCatalogForFilter).not.toHaveBeenCalled(); }); - it("uses provider-filtered refreshable manifest rows without loading the registry", async () => { + it("keeps provider-filtered refreshable manifest rows registry-backed", async () => { const { planAllModelListSources } = await import("./list.source-plan.js"); - mocks.loadManifestCatalogRowsForList.mockReturnValueOnce([catalogRow]); + mocks.loadSupplementalManifestCatalogRowsForList.mockReturnValueOnce([catalogRow]); const plan = await planAllModelListSources({ all: true, @@ -94,15 +97,18 @@ describe("planAllModelListSources", () => { }); expect(plan).toMatchObject({ - kind: "manifest", - requiresInitialRegistry: false, - skipRuntimeModelSuppression: true, + kind: "registry", + requiresInitialRegistry: true, + skipRuntimeModelSuppression: false, }); expect(plan.manifestCatalogRows).toEqual([catalogRow]); - expect(mocks.loadManifestCatalogRowsForList).toHaveBeenCalledWith({ + expect(mocks.loadStaticManifestCatalogRowsForList).toHaveBeenCalledWith({ + cfg: {}, + providerFilter: "openai", + }); + expect(mocks.loadSupplementalManifestCatalogRowsForList).toHaveBeenCalledWith({ cfg: {}, providerFilter: "openai", - staticOnly: true, }); expect(mocks.loadProviderIndexCatalogRowsForList).not.toHaveBeenCalled(); }); @@ -127,7 +133,7 @@ describe("planAllModelListSources", () => { it("keeps broad all-model lists on the registry path with cheap catalog supplements", async () => { const { planAllModelListSources } = await import("./list.source-plan.js"); const providerIndexRow = { ...catalogRow, source: "provider-index" }; - mocks.loadManifestCatalogRowsForList.mockReturnValueOnce([catalogRow]); + mocks.loadSupplementalManifestCatalogRowsForList.mockReturnValueOnce([catalogRow]); mocks.loadProviderIndexCatalogRowsForList.mockReturnValueOnce([providerIndexRow]); const plan = await planAllModelListSources({ @@ -142,10 +148,10 @@ describe("planAllModelListSources", () => { }); expect(plan.manifestCatalogRows).toEqual([catalogRow]); expect(plan.providerIndexCatalogRows).toEqual([providerIndexRow]); - expect(mocks.loadManifestCatalogRowsForList).toHaveBeenCalledWith({ + expect(mocks.loadSupplementalManifestCatalogRowsForList).toHaveBeenCalledWith({ cfg: {}, - staticOnly: false, }); + expect(mocks.loadStaticManifestCatalogRowsForList).not.toHaveBeenCalled(); expect(mocks.hasProviderStaticCatalogForFilter).not.toHaveBeenCalled(); }); diff --git a/src/commands/models/list.source-plan.ts b/src/commands/models/list.source-plan.ts index 7eace278cfe..293f246c067 100644 --- a/src/commands/models/list.source-plan.ts +++ b/src/commands/models/list.source-plan.ts @@ -51,26 +51,16 @@ export async function planAllModelListSources(params: { return createRegistryModelListSourcePlan(); } - const { loadManifestCatalogRowsForList } = await import("./list.manifest-catalog.js"); - const staticManifestCatalogRows = loadManifestCatalogRowsForList({ - cfg: params.cfg, - ...(params.providerFilter ? { providerFilter: params.providerFilter } : {}), - staticOnly: Boolean(params.providerFilter), - }); - const manifestCatalogRows = - params.providerFilter && staticManifestCatalogRows.length === 0 - ? loadManifestCatalogRowsForList({ - cfg: params.cfg, - providerFilter: params.providerFilter, - staticOnly: false, - }) - : staticManifestCatalogRows; + const { loadStaticManifestCatalogRowsForList, loadSupplementalManifestCatalogRowsForList } = + await import("./list.manifest-catalog.js"); if (!params.providerFilter) { const { loadProviderIndexCatalogRowsForList } = await import("./list.provider-index-catalog.js"); return createSourcePlan({ kind: "registry", - manifestCatalogRows, + manifestCatalogRows: loadSupplementalManifestCatalogRowsForList({ + cfg: params.cfg, + }), providerIndexCatalogRows: loadProviderIndexCatalogRowsForList({ cfg: params.cfg, }), @@ -78,6 +68,18 @@ export async function planAllModelListSources(params: { }); } + const staticManifestCatalogRows = loadStaticManifestCatalogRowsForList({ + cfg: params.cfg, + providerFilter: params.providerFilter, + }); + const manifestCatalogRows = + staticManifestCatalogRows.length === 0 + ? loadSupplementalManifestCatalogRowsForList({ + cfg: params.cfg, + providerFilter: params.providerFilter, + }) + : staticManifestCatalogRows; + if (manifestCatalogRows.length > 0) { if (staticManifestCatalogRows.length === 0) { return createSourcePlan({