mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 13:26:04 -06:00
fix(models): --agent is silently ignored by models aliases and models scan (#126864)
* fix(models): reject --agent on aliases and scan instead of ignoring it models aliases list/add/remove and models scan never read the parent --agent option, so an operator scoping one of them to an agent got no feedback that the flag did nothing, and a typo'd or nonexistent agent id was never validated. They only read or write agents.defaults.*, so there is no agent-scoped path for the flag to feed. Reuse the existing set/set-image guard rather than inventing a new mechanism: rename it to rejectAgentScopedModelCommand, give each global-only command an accurate scope phrase, and call it before the command runs. fallbacks/image-fallbacks are deliberately untouched; they are tracked separately in #106346. Fixes #126597 * fix(models): drop release-owned changelog entry * fix(models): simplify global-only scope message Review flagged the production LOC delta. Collapse the runtime scope map into a type-only union and one accurate message covering both the read (aliases list) and write commands; net production delta is now +20. * fix(models): state the guard scope accurately for scan Review finding: the shared message claimed every guarded command 'only reads or writes global model config', but models scan --no-probe prints the catalog and returns before any config update (scan.ts:283). Say what is true of all six instead: they are global and never agent-scoped. Docs line updated to match. --------- Co-authored-by: sashankh <saisashankhd@microsoft.com>
This commit is contained in:
+1
-1
@@ -28,7 +28,7 @@ openclaw models set-image <model-or-alias>
|
||||
openclaw models scan
|
||||
```
|
||||
|
||||
`status`, `list`, and `auth` subcommands accept `--agent <id>` to target a configured agent; `scan`, `aliases`, and `fallbacks`/`image-fallbacks` always use the configured default agent, and `set`/`set-image` reject `--agent` outright. When omitted, `--agent`-aware commands use `OPENCLAW_AGENT_DIR` if set, otherwise the configured default agent.
|
||||
`status`, `list`, and `auth` subcommands accept `--agent <id>` to target a configured agent; `fallbacks`/`image-fallbacks` always use the configured default agent, and `set`, `set-image`, `scan`, and `aliases` reject `--agent` outright because they are global and never agent-scoped. When omitted, `--agent`-aware commands use `OPENCLAW_AGENT_DIR` if set, otherwise the configured default agent.
|
||||
|
||||
`models set` and `models set-image` require the provider to be declared by an installed plugin or configured under `models.providers`. An unknown provider exits nonzero without changing config. If the provider is known but the model is absent from the local catalog, the command saves the selection and prints a warning because newly released and self-hosted models may not be cataloged yet. `openclaw doctor --json` reports configured unknown providers; add `--severity-min info` to also see active models that the local catalog cannot confirm.
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ describe("models cli lazy runtime boundary", () => {
|
||||
runtimeLoaded();
|
||||
return {
|
||||
defaultRuntime: {},
|
||||
rejectAgentScopedModelWrite: vi.fn(),
|
||||
rejectAgentScopedModelCommand: vi.fn(),
|
||||
resolveModelAgentOption: vi.fn(),
|
||||
runModelsCommand: vi.fn(),
|
||||
};
|
||||
@@ -53,7 +53,7 @@ describe("models cli lazy runtime boundary", () => {
|
||||
runtimeLoaded();
|
||||
return {
|
||||
defaultRuntime,
|
||||
rejectAgentScopedModelWrite: vi.fn(),
|
||||
rejectAgentScopedModelCommand: vi.fn(),
|
||||
resolveModelAgentOption,
|
||||
runModelsCommand,
|
||||
};
|
||||
|
||||
@@ -20,16 +20,27 @@ export function resolveModelAgentOption(
|
||||
);
|
||||
}
|
||||
|
||||
export function rejectAgentScopedModelWrite(
|
||||
/** `models` subcommands that operate on global state only, never per-agent. */
|
||||
export type GlobalOnlyModelCommandName =
|
||||
| "set"
|
||||
| "set-image"
|
||||
| "scan"
|
||||
| "aliases list"
|
||||
| "aliases add"
|
||||
| "aliases remove";
|
||||
|
||||
export function rejectAgentScopedModelCommand(
|
||||
command: Command,
|
||||
commandName: "set" | "set-image",
|
||||
commandName: GlobalOnlyModelCommandName,
|
||||
): void {
|
||||
// Write commands update global defaults; accepting --agent here would imply per-agent mutation.
|
||||
// None of these resolve an agent, so accepting --agent would imply a scope that
|
||||
// does not exist. Kept scope-neutral: `scan --no-probe` returns after printing
|
||||
// the catalog without writing config at all.
|
||||
const agent = resolveOptionFromCommand<string>(command, "agent");
|
||||
if (!agent) {
|
||||
return;
|
||||
}
|
||||
throw new Error(
|
||||
`openclaw models ${commandName} does not support --agent; it only updates global model defaults. Remove --agent, or run ${formatCliCommand("openclaw agents list")} and set the per-agent model in agent config.`,
|
||||
`openclaw models ${commandName} does not support --agent; it is global and never agent-scoped. Remove --agent, or run ${formatCliCommand("openclaw agents list")} and set the per-agent model in agent config.`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -12,6 +12,10 @@ const mocks = vi.hoisted(() => ({
|
||||
modelsSetCommand: vi.fn().mockResolvedValue(undefined),
|
||||
modelsSetImageCommand: vi.fn().mockResolvedValue(undefined),
|
||||
noopAsync: vi.fn(async () => undefined),
|
||||
modelsAliasesAddCommand: vi.fn().mockResolvedValue(undefined),
|
||||
modelsAliasesListCommand: vi.fn().mockResolvedValue(undefined),
|
||||
modelsAliasesRemoveCommand: vi.fn().mockResolvedValue(undefined),
|
||||
modelsScanCommand: vi.fn().mockResolvedValue(undefined),
|
||||
modelsAuthAddCommand: vi.fn().mockResolvedValue(undefined),
|
||||
modelsAuthListCommand: vi.fn().mockResolvedValue(undefined),
|
||||
modelsAuthLoginCommand: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -25,6 +29,9 @@ const mocks = vi.hoisted(() => ({
|
||||
}));
|
||||
|
||||
const {
|
||||
modelsAliasesAddCommand,
|
||||
modelsAliasesListCommand,
|
||||
modelsAliasesRemoveCommand,
|
||||
modelsAuthAddCommand,
|
||||
modelsAuthListCommand,
|
||||
modelsAuthLoginCommand,
|
||||
@@ -35,6 +42,7 @@ const {
|
||||
modelsAuthPasteApiKeyCommand,
|
||||
modelsAuthPasteTokenCommand,
|
||||
modelsAuthSetupTokenCommand,
|
||||
modelsScanCommand,
|
||||
modelsSetCommand,
|
||||
modelsSetImageCommand,
|
||||
modelsStatusCommand,
|
||||
@@ -65,9 +73,9 @@ vi.mock("../commands/models/auth-order.js", () => ({
|
||||
modelsAuthOrderSetCommand: mocks.modelsAuthOrderSetCommand,
|
||||
}));
|
||||
vi.mock("../commands/models/aliases.js", () => ({
|
||||
modelsAliasesAddCommand: mocks.noopAsync,
|
||||
modelsAliasesListCommand: mocks.noopAsync,
|
||||
modelsAliasesRemoveCommand: mocks.noopAsync,
|
||||
modelsAliasesAddCommand: mocks.modelsAliasesAddCommand,
|
||||
modelsAliasesListCommand: mocks.modelsAliasesListCommand,
|
||||
modelsAliasesRemoveCommand: mocks.modelsAliasesRemoveCommand,
|
||||
}));
|
||||
vi.mock("../commands/models/fallbacks.js", () => ({
|
||||
modelsFallbacksAddCommand: mocks.noopAsync,
|
||||
@@ -82,7 +90,7 @@ vi.mock("../commands/models/image-fallbacks.js", () => ({
|
||||
modelsImageFallbacksRemoveCommand: mocks.noopAsync,
|
||||
}));
|
||||
vi.mock("../commands/models/scan.js", () => ({
|
||||
modelsScanCommand: mocks.noopAsync,
|
||||
modelsScanCommand: mocks.modelsScanCommand,
|
||||
}));
|
||||
vi.mock("../commands/models/set.js", () => ({
|
||||
modelsSetCommand: mocks.modelsSetCommand,
|
||||
@@ -94,6 +102,10 @@ vi.mock("../commands/models/set-image.js", () => ({
|
||||
describe("models cli", () => {
|
||||
beforeEach(() => {
|
||||
mocks.modelsListCommand.mockClear();
|
||||
modelsAliasesAddCommand.mockClear();
|
||||
modelsAliasesListCommand.mockClear();
|
||||
modelsAliasesRemoveCommand.mockClear();
|
||||
modelsScanCommand.mockClear();
|
||||
modelsAuthAddCommand.mockClear();
|
||||
modelsAuthListCommand.mockClear();
|
||||
modelsAuthLoginCommand.mockClear();
|
||||
@@ -488,12 +500,49 @@ describe("models cli", () => {
|
||||
args: ["models", "--agent", "poe", "set-image", "openai/gpt-image-1"],
|
||||
command: modelsSetImageCommand,
|
||||
},
|
||||
{
|
||||
label: "aliases list",
|
||||
args: ["models", "--agent", "poe", "aliases", "list"],
|
||||
command: modelsAliasesListCommand,
|
||||
},
|
||||
{
|
||||
label: "aliases add",
|
||||
args: ["models", "--agent", "poe", "aliases", "add", "zzz", "soraka/grok-4.6"],
|
||||
command: modelsAliasesAddCommand,
|
||||
},
|
||||
{
|
||||
label: "aliases remove",
|
||||
args: ["models", "--agent", "poe", "aliases", "remove", "zzz"],
|
||||
command: modelsAliasesRemoveCommand,
|
||||
},
|
||||
{
|
||||
label: "scan",
|
||||
args: ["models", "--agent", "poe", "scan", "--no-probe", "--no-input"],
|
||||
command: modelsScanCommand,
|
||||
},
|
||||
])("rejects parent --agent for models $label", async ({ args, command }) => {
|
||||
await expect(runModelsCommand(args)).rejects.toThrow("does not support --agent");
|
||||
|
||||
expect(command).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "aliases list",
|
||||
args: ["models", "aliases", "list"],
|
||||
command: modelsAliasesListCommand,
|
||||
},
|
||||
{
|
||||
label: "scan",
|
||||
args: ["models", "scan", "--no-probe", "--no-input"],
|
||||
command: modelsScanCommand,
|
||||
},
|
||||
])("still runs models $label without --agent", async ({ args, command }) => {
|
||||
await runModelsCommand(args);
|
||||
|
||||
expect(command).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows help for models auth without error exit", async () => {
|
||||
const program = new Command();
|
||||
program.exitOverride();
|
||||
|
||||
+25
-14
@@ -142,7 +142,7 @@ export function registerModelsCli(program: Command) {
|
||||
.argument("<model>", "Model id or alias")
|
||||
.action(async (model: string, _opts: unknown, command: Command) => {
|
||||
const runtime = await loadModelsRuntime();
|
||||
runtime.rejectAgentScopedModelWrite(command, "set");
|
||||
runtime.rejectAgentScopedModelCommand(command, "set");
|
||||
await runtime.runModelsCommand(async () => {
|
||||
const { modelsSetCommand } = await import("../commands/models/set.js");
|
||||
await modelsSetCommand(model, runtime.defaultRuntime);
|
||||
@@ -155,7 +155,7 @@ export function registerModelsCli(program: Command) {
|
||||
.argument("<model>", "Model id or alias")
|
||||
.action(async (model: string, _opts: unknown, command: Command) => {
|
||||
const runtime = await loadModelsRuntime();
|
||||
runtime.rejectAgentScopedModelWrite(command, "set-image");
|
||||
runtime.rejectAgentScopedModelCommand(command, "set-image");
|
||||
await runtime.runModelsCommand(async () => {
|
||||
const { modelsSetImageCommand } = await import("../commands/models/set-image.js");
|
||||
await modelsSetImageCommand(model, runtime.defaultRuntime);
|
||||
@@ -169,10 +169,15 @@ export function registerModelsCli(program: Command) {
|
||||
.description("List model aliases")
|
||||
.option("--json", "Output JSON", false)
|
||||
.option("--plain", "Plain output", false)
|
||||
.action(async (opts) => {
|
||||
await withModelsRuntime(async ({ defaultRuntime }) => {
|
||||
.action(async (opts, command: Command) => {
|
||||
const runtime = await loadModelsRuntime();
|
||||
runtime.rejectAgentScopedModelCommand(command, "aliases list");
|
||||
await runtime.runModelsCommand(async () => {
|
||||
const { modelsAliasesListCommand } = await loadModelsAliasesCommands();
|
||||
await modelsAliasesListCommand({ ...opts, json: hasJsonOutput(opts) }, defaultRuntime);
|
||||
await modelsAliasesListCommand(
|
||||
{ ...opts, json: hasJsonOutput(opts) },
|
||||
runtime.defaultRuntime,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -181,10 +186,12 @@ export function registerModelsCli(program: Command) {
|
||||
.description("Add or update a model alias")
|
||||
.argument("<alias>", "Alias name")
|
||||
.argument("<model>", "Model id or alias")
|
||||
.action(async (alias: string, model: string) => {
|
||||
await withModelsRuntime(async ({ defaultRuntime }) => {
|
||||
.action(async (alias: string, model: string, _opts: unknown, command: Command) => {
|
||||
const runtime = await loadModelsRuntime();
|
||||
runtime.rejectAgentScopedModelCommand(command, "aliases add");
|
||||
await runtime.runModelsCommand(async () => {
|
||||
const { modelsAliasesAddCommand } = await loadModelsAliasesCommands();
|
||||
await modelsAliasesAddCommand(alias, model, defaultRuntime);
|
||||
await modelsAliasesAddCommand(alias, model, runtime.defaultRuntime);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -192,10 +199,12 @@ export function registerModelsCli(program: Command) {
|
||||
.command("remove")
|
||||
.description("Remove a model alias")
|
||||
.argument("<alias>", "Alias name")
|
||||
.action(async (alias: string) => {
|
||||
await withModelsRuntime(async ({ defaultRuntime }) => {
|
||||
.action(async (alias: string, _opts: unknown, command: Command) => {
|
||||
const runtime = await loadModelsRuntime();
|
||||
runtime.rejectAgentScopedModelCommand(command, "aliases remove");
|
||||
await runtime.runModelsCommand(async () => {
|
||||
const { modelsAliasesRemoveCommand } = await loadModelsAliasesCommands();
|
||||
await modelsAliasesRemoveCommand(alias, defaultRuntime);
|
||||
await modelsAliasesRemoveCommand(alias, runtime.defaultRuntime);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -286,10 +295,12 @@ export function registerModelsCli(program: Command) {
|
||||
.option("--set-default", "Set agents.defaults.model to the first selection", false)
|
||||
.option("--set-image", "Set agents.defaults.imageModel to the first image selection", false)
|
||||
.option("--json", "Output JSON", false)
|
||||
.action(async (opts) => {
|
||||
await withModelsRuntime(async ({ defaultRuntime }) => {
|
||||
.action(async (opts, command: Command) => {
|
||||
const runtime = await loadModelsRuntime();
|
||||
runtime.rejectAgentScopedModelCommand(command, "scan");
|
||||
await runtime.runModelsCommand(async () => {
|
||||
const { modelsScanCommand } = await import("../commands/models/scan.js");
|
||||
await modelsScanCommand({ ...opts, json: hasJsonOutput(opts) }, defaultRuntime);
|
||||
await modelsScanCommand({ ...opts, json: hasJsonOutput(opts) }, runtime.defaultRuntime);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user