refactor(cli): collapse model fallback command wrappers (#130536)

This commit is contained in:
Peter Steinberger
2026-08-26 17:58:12 -07:00
committed by GitHub
parent 4c4c06aa35
commit a13f4d29ee
7 changed files with 188 additions and 230 deletions
+5 -11
View File
@@ -77,17 +77,11 @@ vi.mock("../commands/models/aliases.js", () => ({
modelsAliasesListCommand: mocks.modelsAliasesListCommand,
modelsAliasesRemoveCommand: mocks.modelsAliasesRemoveCommand,
}));
vi.mock("../commands/models/fallbacks.js", () => ({
modelsFallbacksAddCommand: mocks.noopAsync,
modelsFallbacksClearCommand: mocks.noopAsync,
modelsFallbacksListCommand: mocks.noopAsync,
modelsFallbacksRemoveCommand: mocks.noopAsync,
}));
vi.mock("../commands/models/image-fallbacks.js", () => ({
modelsImageFallbacksAddCommand: mocks.noopAsync,
modelsImageFallbacksClearCommand: mocks.noopAsync,
modelsImageFallbacksListCommand: mocks.noopAsync,
modelsImageFallbacksRemoveCommand: mocks.noopAsync,
vi.mock("../commands/models/fallbacks-shared.js", () => ({
addFallbackCommand: mocks.noopAsync,
clearFallbacksCommand: mocks.noopAsync,
listFallbacksCommand: mocks.noopAsync,
removeFallbackCommand: mocks.noopAsync,
}));
vi.mock("../commands/models/scan.js", () => ({
modelsScanCommand: mocks.modelsScanCommand,
+25 -30
View File
@@ -21,10 +21,7 @@ const loadModelsStatusCommands = createModuleLoader(
);
const loadModelsAliasesCommands = createModuleLoader(() => import("../commands/models/aliases.js"));
const loadModelsFallbacksCommands = createModuleLoader(
() => import("../commands/models/fallbacks.js"),
);
const loadModelsImageFallbacksCommands = createModuleLoader(
() => import("../commands/models/image-fallbacks.js"),
() => import("../commands/models/fallbacks-shared.js"),
);
const loadModelsAuthCommands = createModuleLoader(() => import("../commands/models/auth.js"));
const loadModelsAuthOrderCommands = createModuleLoader(
@@ -216,34 +213,25 @@ export function registerModelsCli(program: Command) {
modelType: "model",
noun: "fallback",
article: "a",
load: async () => {
const commands = await loadModelsFallbacksCommands();
return {
list: commands.modelsFallbacksListCommand,
add: commands.modelsFallbacksAddCommand,
remove: commands.modelsFallbacksRemoveCommand,
clear: commands.modelsFallbacksClearCommand,
};
},
key: "model",
label: "Fallbacks",
notFoundLabel: "Fallback",
clearedMessage: "Fallback list cleared.",
},
{
name: "image-fallbacks",
modelType: "image model",
noun: "image fallback",
article: "an",
load: async () => {
const commands = await loadModelsImageFallbacksCommands();
return {
list: commands.modelsImageFallbacksListCommand,
add: commands.modelsImageFallbacksAddCommand,
remove: commands.modelsImageFallbacksRemoveCommand,
clear: commands.modelsImageFallbacksClearCommand,
};
},
key: "imageModel",
label: "Image fallbacks",
notFoundLabel: "Image fallback",
clearedMessage: "Image fallback list cleared.",
},
] as const;
for (const { name, modelType, noun, article, load } of fallbackGroups) {
for (const params of fallbackGroups) {
const { name, modelType, noun, article } = params;
const group = models.command(name).description(`Manage ${modelType} fallback list`);
group
@@ -253,20 +241,27 @@ export function registerModelsCli(program: Command) {
.option("--plain", "Plain output", false)
.action(async (opts) => {
await withModelsRuntime(async ({ defaultRuntime }) => {
const commands = await load();
await commands.list({ ...opts, json: hasJsonOutput(opts) }, defaultRuntime);
const { listFallbacksCommand } = await loadModelsFallbacksCommands();
await listFallbacksCommand(
params,
{ ...opts, json: hasJsonOutput(opts) },
defaultRuntime,
);
});
});
for (const action of ["add", "remove"] as const) {
for (const [action, handler] of [
["add", "addFallbackCommand"],
["remove", "removeFallbackCommand"],
] as const) {
group
.command(action)
.description(`${action === "add" ? "Add" : "Remove"} ${article} ${noun} model`)
.argument("<model>", "Model id or alias")
.action(async (model: string) => {
await withModelsRuntime(async ({ defaultRuntime }) => {
const commands = await load();
await commands[action](model, defaultRuntime);
const commands = await loadModelsFallbacksCommands();
await commands[handler](params, model, defaultRuntime);
});
});
}
@@ -276,8 +271,8 @@ export function registerModelsCli(program: Command) {
.description(`Clear all ${noun} models`)
.action(async () => {
await withModelsRuntime(async ({ defaultRuntime }) => {
const commands = await load();
await commands.clear(defaultRuntime);
const { clearFallbacksCommand } = await loadModelsFallbacksCommands();
await clearFallbacksCommand(params, defaultRuntime);
});
});
}
+99 -31
View File
@@ -1,8 +1,11 @@
// Models set e2e tests cover persisted model selection updates through command handlers.
import { beforeEach, describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { createModelVisibilityPolicy } from "../agents/model-visibility-policy.js";
import { registerModelsCli } from "../cli/models-cli.js";
import { stampConfigWriteMetadata } from "../config/io.meta.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { defaultRuntime } from "../runtime.js";
import { runRegisteredCli } from "../test-utils/command-runner.js";
const mocks = vi.hoisted(() => ({
currentConfig: {} as Record<string, unknown>,
@@ -26,7 +29,6 @@ vi.mock("../config/config.js", async () => {
};
});
import { modelsFallbacksAddCommand } from "./models/fallbacks.js";
import { modelsSetImageCommand } from "./models/set-image.js";
import { modelsSetCommand } from "./models/set.js";
@@ -56,12 +58,29 @@ function expectWrittenPrimaryModel(model: string) {
});
}
const fallbackGroups = [
{ name: "fallbacks", key: "model", label: "Fallbacks", singular: "Fallback" },
{
name: "image-fallbacks",
key: "imageModel",
label: "Image fallbacks",
singular: "Image fallback",
},
] as const;
async function runFallbackCommand(name: string, ...args: string[]) {
await runRegisteredCli({ register: registerModelsCli, argv: ["models", name, ...args] });
}
describe("models set + fallbacks", () => {
beforeEach(() => {
mocks.currentConfig = {};
mocks.writtenConfig = undefined;
vi.spyOn(defaultRuntime, "log").mockImplementation(() => {});
});
afterEach(() => vi.restoreAllMocks());
it("normalizes z.ai provider in models set", async () => {
mockConfigSnapshot({});
const runtime = makeRuntime();
@@ -149,38 +168,87 @@ describe("models set + fallbacks", () => {
expect(policy.allows({ provider: "openai", model: "gpt-5.6-sol" })).toBe(true);
});
it("normalizes z-ai provider in models fallbacks add", async () => {
mockConfigSnapshot({ agents: { defaults: { model: { fallbacks: [] } } } });
const runtime = makeRuntime();
it.each(fallbackGroups)(
"normalizes z-ai provider in models $name add",
async ({ name, key, label }) => {
mockConfigSnapshot({ agents: { defaults: { [key]: { fallbacks: [] } } } });
await modelsFallbacksAddCommand("z-ai/glm-4.7", runtime);
await runFallbackCommand(name, "add", "z-ai/glm-4.7");
const written = getWrittenConfig();
expect(written.agents).toEqual({
defaults: {
model: { fallbacks: ["zai/glm-4.7"] },
models: { "zai/glm-4.7": {} },
},
});
});
it("preserves primary when adding fallbacks to string defaults.model", async () => {
mockConfigSnapshot({ agents: { defaults: { model: "openai/gpt-4.1-mini" } } });
const runtime = makeRuntime();
await modelsFallbacksAddCommand("anthropic/claude-opus-4-6", runtime);
const written = getWrittenConfig();
expect(written.agents).toEqual({
defaults: {
model: {
primary: "openai/gpt-4.1-mini",
fallbacks: ["anthropic/claude-opus-4-6"],
const written = getWrittenConfig();
expect(written.agents).toEqual({
defaults: {
[key]: { fallbacks: ["zai/glm-4.7"] },
models: { "zai/glm-4.7": {} },
},
models: { "anthropic/claude-opus-4-6": {} },
},
});
});
});
expect(defaultRuntime.log).toHaveBeenLastCalledWith(`${label}: zai/glm-4.7`);
},
);
it.each(fallbackGroups)(
"preserves string primary when adding models $name",
async ({ name, key }) => {
mockConfigSnapshot({ agents: { defaults: { [key]: "openai/gpt-4.1-mini" } } });
await runFallbackCommand(name, "add", "anthropic/claude-opus-4-6");
const written = getWrittenConfig();
expect(written.agents).toEqual({
defaults: {
[key]: {
primary: "openai/gpt-4.1-mini",
fallbacks: ["anthropic/claude-opus-4-6"],
},
models: { "anthropic/claude-opus-4-6": {} },
},
});
},
);
it.each(fallbackGroups)(
"removes aliases and clears only models $name",
async ({ name, key, label, singular }) => {
const siblingKey = key === "model" ? "imageModel" : "model";
const primary = "openai/gpt-5.6-luna";
const sibling = { primary, fallbacks: [primary] };
mockConfigSnapshot({
agents: {
defaults: {
[key]: { primary, fallbacks: ["backup", primary] },
[siblingKey]: sibling,
models: { "zai/glm-4.7": { alias: "backup" } },
},
},
});
await runFallbackCommand(name, "remove", "z-ai/glm-4.7");
expect(getWrittenConfig().agents?.defaults?.[key]).toEqual({ primary, fallbacks: [primary] });
expect(getWrittenConfig().agents?.defaults?.[siblingKey]).toEqual(sibling);
expect(defaultRuntime.log).toHaveBeenLastCalledWith(`${label}: ${primary}`);
mocks.currentConfig = getWrittenConfig();
await runFallbackCommand(name, "clear");
expect(getWrittenConfig().agents?.defaults?.[key]).toEqual({ primary, fallbacks: [] });
expect(getWrittenConfig().agents?.defaults?.[siblingKey]).toEqual(sibling);
expect(defaultRuntime.log).toHaveBeenLastCalledWith(`${singular} list cleared.`);
mockConfigSnapshot(getWrittenConfig());
const error = vi.spyOn(defaultRuntime, "error").mockImplementation(() => {});
const exit = vi.spyOn(defaultRuntime, "exit").mockImplementation(() => {
throw new Error("CLI exit");
});
await expect(runFallbackCommand(name, "remove", "backup")).rejects.toThrow("CLI exit");
expect(error).toHaveBeenCalledWith(
expect.stringContaining(`${singular} not found: zai/glm-4.7.`),
);
expect(error).toHaveBeenCalledWith(expect.stringContaining(`models ${name} list`));
expect(exit).toHaveBeenCalledWith(1);
expect(mocks.writtenConfig).toBeUndefined();
},
);
it("normalizes provider casing in models set", async () => {
mockConfigSnapshot({});
+57 -60
View File
@@ -1,6 +1,7 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { RuntimeEnv } from "../../runtime.js";
import { listFallbacksCommand } from "./fallbacks-shared.js";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { registerModelsCli } from "../../cli/models-cli.js";
import { defaultRuntime } from "../../runtime.js";
import { runRegisteredCli } from "../../test-utils/command-runner.js";
const mocks = vi.hoisted(() => ({
loadModelsConfig: vi.fn(),
@@ -10,25 +11,22 @@ vi.mock("./load-config.js", () => ({
loadModelsConfig: mocks.loadModelsConfig,
}));
describe("listFallbacksCommand", () => {
describe.each([
{
name: "fallbacks",
label: "Fallbacks",
key: "model" as const,
model: "anthropic/claude-sonnet-4-6",
},
{
name: "image-fallbacks",
label: "Image fallbacks",
key: "imageModel" as const,
model: "openai/gpt-image-1",
},
])("models $name list", (testCase) => {
beforeEach(() => {
mocks.loadModelsConfig.mockReset();
});
it.each([
{
label: "Fallbacks",
key: "model" as const,
commandName: "models fallbacks list",
model: "anthropic/claude-sonnet-4-6",
},
{
label: "Image fallbacks",
key: "imageModel" as const,
commandName: "models image-fallbacks list",
model: "openai/gpt-image-1",
},
])("attributes $label diagnostics to the real CLI command", async (testCase) => {
mocks.loadModelsConfig.mockResolvedValue({
agents: {
defaults: {
@@ -36,54 +34,53 @@ describe("listFallbacksCommand", () => {
},
},
});
const runtime = {
log: vi.fn(),
error: vi.fn(),
exit: vi.fn(),
} satisfies RuntimeEnv;
vi.spyOn(defaultRuntime, "log").mockImplementation(() => {});
vi.spyOn(defaultRuntime, "writeStdout").mockImplementation(() => {});
vi.spyOn(defaultRuntime, "writeJson").mockImplementation(() => {});
});
await listFallbacksCommand(
{ label: testCase.label, key: testCase.key },
{ json: true },
runtime,
);
afterEach(() => vi.restoreAllMocks());
it.each([
["--json", testCase.name, "list"],
[testCase.name, "list", "--json"],
])("writes JSON and attributes diagnostics for %s %s %s", async (...args) => {
await runRegisteredCli({ register: registerModelsCli, argv: ["models", ...args] });
expect(mocks.loadModelsConfig).toHaveBeenCalledWith({
commandName: testCase.commandName,
runtime,
});
expect(runtime.log).toHaveBeenCalledOnce();
expect(JSON.parse(runtime.log.mock.calls[0]?.[0] as string)).toEqual({
fallbacks: [testCase.model],
commandName: `models ${testCase.name} list`,
runtime: defaultRuntime,
});
expect(vi.mocked(defaultRuntime.writeJson).mock.calls.map(([value]) => value)).toEqual([
{
fallbacks: [testCase.model],
},
]);
expect(defaultRuntime.log).not.toHaveBeenCalled();
});
it.each([
{ label: "Fallbacks", key: "model" as const, model: "anthropic/claude-sonnet-4-6" },
{ label: "Image fallbacks", key: "imageModel" as const, model: "openai/gpt-image-1" },
])("writes populated plain $label directly to stdout", async (testCase) => {
mocks.loadModelsConfig.mockResolvedValue({
agents: {
defaults: {
[testCase.key]: { fallbacks: [testCase.model] },
},
},
it("writes populated plain output directly to stdout", async () => {
await runRegisteredCli({
register: registerModelsCli,
argv: ["models", testCase.name, "list", "--plain"],
});
const runtime = {
log: vi.fn(),
error: vi.fn(),
exit: vi.fn(),
writeStdout: vi.fn(),
writeJson: vi.fn(),
};
await listFallbacksCommand(
{ label: testCase.label, key: testCase.key },
{ plain: true },
runtime,
);
expect(defaultRuntime.writeStdout).toHaveBeenCalledExactlyOnceWith(testCase.model);
expect(defaultRuntime.log).not.toHaveBeenCalled();
});
expect(runtime.writeStdout).toHaveBeenCalledExactlyOnceWith(testCase.model);
expect(runtime.log).not.toHaveBeenCalled();
it.each([false, true])("preserves human output (empty: %s)", async (empty) => {
if (empty) {
mocks.loadModelsConfig.mockResolvedValue({});
}
await runRegisteredCli({
register: registerModelsCli,
argv: ["models", testCase.name, "list"],
});
expect(vi.mocked(defaultRuntime.log).mock.calls).toEqual([
[`${testCase.label} (${empty ? 0 : 1}):`],
[empty ? "- none" : `- ${testCase.model}`],
]);
});
});
+2 -4
View File
@@ -85,7 +85,6 @@ export async function addFallbackCommand(
params: {
label: string;
key: DefaultsFallbackKey;
logPrefix: string;
},
modelRaw: string,
runtime: RuntimeEnv,
@@ -110,7 +109,7 @@ export async function addFallbackCommand(
});
logConfigUpdated(runtime);
runtime.log(`${params.logPrefix}: ${getFallbacks(updated, params.key).join(", ")}`);
runtime.log(`${params.label}: ${getFallbacks(updated, params.key).join(", ")}`);
}
/** Removes a fallback model by resolving aliases to the canonical provider/model key. */
@@ -119,7 +118,6 @@ export async function removeFallbackCommand(
label: string;
key: DefaultsFallbackKey;
notFoundLabel: string;
logPrefix: string;
},
modelRaw: string,
runtime: RuntimeEnv,
@@ -156,7 +154,7 @@ export async function removeFallbackCommand(
});
logConfigUpdated(runtime);
runtime.log(`${params.logPrefix}: ${getFallbacks(updated, params.key).join(", ")}`);
runtime.log(`${params.label}: ${getFallbacks(updated, params.key).join(", ")}`);
}
/** Clears all fallback model refs for the selected defaults key. */
-47
View File
@@ -1,47 +0,0 @@
/** Commands for managing default text model fallbacks. */
import type { RuntimeEnv } from "../../runtime.js";
import {
addFallbackCommand,
clearFallbacksCommand,
listFallbacksCommand,
removeFallbackCommand,
} from "./fallbacks-shared.js";
/** Lists configured text model fallbacks. */
export async function modelsFallbacksListCommand(
opts: { json?: boolean; plain?: boolean },
runtime: RuntimeEnv,
) {
return await listFallbacksCommand({ label: "Fallbacks", key: "model" }, opts, runtime);
}
/** Adds a text model fallback. */
export async function modelsFallbacksAddCommand(modelRaw: string, runtime: RuntimeEnv) {
return await addFallbackCommand(
{ label: "Fallbacks", key: "model", logPrefix: "Fallbacks" },
modelRaw,
runtime,
);
}
/** Removes a text model fallback. */
export async function modelsFallbacksRemoveCommand(modelRaw: string, runtime: RuntimeEnv) {
return await removeFallbackCommand(
{
label: "Fallbacks",
key: "model",
notFoundLabel: "Fallback",
logPrefix: "Fallbacks",
},
modelRaw,
runtime,
);
}
/** Clears all text model fallbacks. */
export async function modelsFallbacksClearCommand(runtime: RuntimeEnv) {
return await clearFallbacksCommand(
{ key: "model", clearedMessage: "Fallback list cleared." },
runtime,
);
}
-47
View File
@@ -1,47 +0,0 @@
/** Commands for managing default image model fallbacks. */
import type { RuntimeEnv } from "../../runtime.js";
import {
addFallbackCommand,
clearFallbacksCommand,
listFallbacksCommand,
removeFallbackCommand,
} from "./fallbacks-shared.js";
/** Lists configured image model fallbacks. */
export async function modelsImageFallbacksListCommand(
opts: { json?: boolean; plain?: boolean },
runtime: RuntimeEnv,
) {
return await listFallbacksCommand({ label: "Image fallbacks", key: "imageModel" }, opts, runtime);
}
/** Adds an image model fallback. */
export async function modelsImageFallbacksAddCommand(modelRaw: string, runtime: RuntimeEnv) {
return await addFallbackCommand(
{ label: "Image fallbacks", key: "imageModel", logPrefix: "Image fallbacks" },
modelRaw,
runtime,
);
}
/** Removes an image model fallback. */
export async function modelsImageFallbacksRemoveCommand(modelRaw: string, runtime: RuntimeEnv) {
return await removeFallbackCommand(
{
label: "Image fallbacks",
key: "imageModel",
notFoundLabel: "Image fallback",
logPrefix: "Image fallbacks",
},
modelRaw,
runtime,
);
}
/** Clears all image model fallbacks. */
export async function modelsImageFallbacksClearCommand(runtime: RuntimeEnv) {
return await clearFallbacksCommand(
{ key: "imageModel", clearedMessage: "Image fallback list cleared." },
runtime,
);
}