From 0a50eb03431ef1690b401d5233cd68b62a557c1b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 23:38:48 +0000 Subject: [PATCH] refactor: share models command helpers --- src/commands/models/list.configured.ts | 40 ++++++----------- src/commands/models/list.probe.ts | 37 +++++++--------- src/commands/models/load-config.test.ts | 58 ++++++++++--------------- 3 files changed, 53 insertions(+), 82 deletions(-) diff --git a/src/commands/models/list.configured.ts b/src/commands/models/list.configured.ts index fed70a4fe47..d83dd9d7f1b 100644 --- a/src/commands/models/list.configured.ts +++ b/src/commands/models/list.configured.ts @@ -39,6 +39,17 @@ export function resolveConfiguredEntries(cfg: OpenClawConfig) { tagsByKey.get(key)?.add(tag); }; + const addResolvedModelRef = (raw: string, tag: string) => { + const resolved = resolveModelRefFromString({ + raw, + defaultProvider: DEFAULT_PROVIDER, + aliasIndex, + }); + if (resolved) { + addEntry(resolved.ref, tag); + } + }; + addEntry(resolvedDefault, "default"); const modelFallbacks = resolveAgentModelFallbackValues(cfg.agents?.defaults?.model); @@ -46,38 +57,15 @@ export function resolveConfiguredEntries(cfg: OpenClawConfig) { const imagePrimary = resolveAgentModelPrimaryValue(cfg.agents?.defaults?.imageModel) ?? ""; modelFallbacks.forEach((raw, idx) => { - const resolved = resolveModelRefFromString({ - raw: String(raw ?? ""), - defaultProvider: DEFAULT_PROVIDER, - aliasIndex, - }); - if (!resolved) { - return; - } - addEntry(resolved.ref, `fallback#${idx + 1}`); + addResolvedModelRef(String(raw ?? ""), `fallback#${idx + 1}`); }); if (imagePrimary) { - const resolved = resolveModelRefFromString({ - raw: imagePrimary, - defaultProvider: DEFAULT_PROVIDER, - aliasIndex, - }); - if (resolved) { - addEntry(resolved.ref, "image"); - } + addResolvedModelRef(imagePrimary, "image"); } imageFallbacks.forEach((raw, idx) => { - const resolved = resolveModelRefFromString({ - raw: String(raw ?? ""), - defaultProvider: DEFAULT_PROVIDER, - aliasIndex, - }); - if (!resolved) { - return; - } - addEntry(resolved.ref, `img-fallback#${idx + 1}`); + addResolvedModelRef(String(raw ?? ""), `img-fallback#${idx + 1}`); }); for (const key of Object.keys(cfg.agents?.defaults?.models ?? {})) { diff --git a/src/commands/models/list.probe.ts b/src/commands/models/list.probe.ts index 5311b004ce2..91418c5a1c2 100644 --- a/src/commands/models/list.probe.ts +++ b/src/commands/models/list.probe.ts @@ -437,6 +437,17 @@ async function probeTarget(params: { await fs.mkdir(sessionDir, { recursive: true }); const start = Date.now(); + const buildResult = (status: AuthProbeResult["status"], error?: string): AuthProbeResult => ({ + provider: target.provider, + model: `${target.model.provider}/${target.model.model}`, + profileId: target.profileId, + label: target.label, + source: target.source, + mode: target.mode, + status, + ...(error ? { error } : {}), + latencyMs: Date.now() - start, + }); try { await runEmbeddedPiAgent({ sessionId, @@ -458,29 +469,13 @@ async function probeTarget(params: { verboseLevel: "off", streamParams: { maxTokens }, }); - return { - provider: target.provider, - model: `${target.model.provider}/${target.model.model}`, - profileId: target.profileId, - label: target.label, - source: target.source, - mode: target.mode, - status: "ok", - latencyMs: Date.now() - start, - }; + return buildResult("ok"); } catch (err) { const described = describeFailoverError(err); - return { - provider: target.provider, - model: `${target.model.provider}/${target.model.model}`, - profileId: target.profileId, - label: target.label, - source: target.source, - mode: target.mode, - status: mapFailoverReasonToProbeStatus(described.reason), - error: redactSecrets(described.message), - latencyMs: Date.now() - start, - }; + return buildResult( + mapFailoverReasonToProbeStatus(described.reason), + redactSecrets(described.message), + ); } } diff --git a/src/commands/models/load-config.test.ts b/src/commands/models/load-config.test.ts index b8969fd4681..2d35c012a49 100644 --- a/src/commands/models/load-config.test.ts +++ b/src/commands/models/load-config.test.ts @@ -25,6 +25,27 @@ vi.mock("../../cli/command-secret-targets.js", () => ({ import { loadModelsConfig, loadModelsConfigWithSource } from "./load-config.js"; describe("models load-config", () => { + const runtimeConfig = { + models: { providers: { openai: { apiKey: "sk-runtime" } } }, // pragma: allowlist secret + }; + const resolvedConfig = { + models: { providers: { openai: { apiKey: "sk-resolved" } } }, // pragma: allowlist secret + }; + const targetIds = new Set(["models.providers.*.apiKey"]); + + function mockResolvedConfigFlow(params: { sourceConfig: unknown; diagnostics: string[] }) { + mocks.loadConfig.mockReturnValue(runtimeConfig); + mocks.readConfigFileSnapshotForWrite.mockResolvedValue({ + snapshot: { valid: true, resolved: params.sourceConfig }, + writeOptions: {}, + }); + mocks.getModelsCommandSecretTargetIds.mockReturnValue(targetIds); + mocks.resolveCommandSecretRefsViaGateway.mockResolvedValue({ + resolvedConfig, + diagnostics: params.diagnostics, + }); + } + beforeEach(() => { vi.clearAllMocks(); }); @@ -39,25 +60,9 @@ describe("models load-config", () => { }, }, }; - const runtimeConfig = { - models: { providers: { openai: { apiKey: "sk-runtime" } } }, // pragma: allowlist secret - }; - const resolvedConfig = { - models: { providers: { openai: { apiKey: "sk-resolved" } } }, // pragma: allowlist secret - }; - const targetIds = new Set(["models.providers.*.apiKey"]); const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() }; - mocks.loadConfig.mockReturnValue(runtimeConfig); - mocks.readConfigFileSnapshotForWrite.mockResolvedValue({ - snapshot: { valid: true, resolved: sourceConfig }, - writeOptions: {}, - }); - mocks.getModelsCommandSecretTargetIds.mockReturnValue(targetIds); - mocks.resolveCommandSecretRefsViaGateway.mockResolvedValue({ - resolvedConfig, - diagnostics: ["diag-one", "diag-two"], - }); + mockResolvedConfigFlow({ sourceConfig, diagnostics: ["diag-one", "diag-two"] }); const result = await loadModelsConfigWithSource({ commandName: "models list", runtime }); @@ -78,24 +83,7 @@ describe("models load-config", () => { it("loadModelsConfig returns resolved config while preserving runtime snapshot behavior", async () => { const sourceConfig = { models: { providers: {} } }; - const runtimeConfig = { - models: { providers: { openai: { apiKey: "sk-runtime" } } }, // pragma: allowlist secret - }; - const resolvedConfig = { - models: { providers: { openai: { apiKey: "sk-resolved" } } }, // pragma: allowlist secret - }; - const targetIds = new Set(["models.providers.*.apiKey"]); - - mocks.loadConfig.mockReturnValue(runtimeConfig); - mocks.readConfigFileSnapshotForWrite.mockResolvedValue({ - snapshot: { valid: true, resolved: sourceConfig }, - writeOptions: {}, - }); - mocks.getModelsCommandSecretTargetIds.mockReturnValue(targetIds); - mocks.resolveCommandSecretRefsViaGateway.mockResolvedValue({ - resolvedConfig, - diagnostics: [], - }); + mockResolvedConfigFlow({ sourceConfig, diagnostics: [] }); await expect(loadModelsConfig({ commandName: "models list" })).resolves.toBe(resolvedConfig); expect(mocks.setRuntimeConfigSnapshot).toHaveBeenCalledWith(resolvedConfig, sourceConfig);