fix(cli): preserve human output for flag-like model option values (#129911)

* fix(cli): preserve human output for flag-like model option values

* test(cli): isolate model output preaction regressions
This commit is contained in:
Peter Steinberger
2026-08-25 23:31:52 -07:00
committed by GitHub
parent bde3494a02
commit b4c936ea82
6 changed files with 170 additions and 8 deletions
+62
View File
@@ -196,6 +196,68 @@ describe("models cli", () => {
expect(detected).toBe(false);
});
it.each(["--plain", "--json"])(
"does not treat required provider value %s as a model output flag",
async (provider) => {
const program = createProgram();
let jsonMode = true;
program.hook("preAction", (_command, actionCommand) => {
jsonMode = isCommandJsonOutputMode(actionCommand, process.argv);
});
const originalArgv = process.argv;
process.argv = ["node", "openclaw", "models", "auth", "list", "--provider", provider];
try {
await program.parseAsync(["models", "auth", "list", "--provider", provider], {
from: "user",
});
} finally {
process.argv = originalArgv;
}
expect(jsonMode).toBe(false);
expectCommandOptions(modelsAuthListCommand, { provider, json: false });
},
);
it.each([
{
name: "an ignored parent status alias and a JSON-looking provider value",
args: ["models", "--status-json", "auth", "list", "--provider", "--json"],
provider: "--json",
json: false,
},
{
name: "a real JSON flag after a status-alias-looking provider value",
args: ["models", "auth", "list", "--provider", "--status-json", "--json"],
provider: "--status-json",
json: true,
},
{
name: "a real JSON flag before a plain-looking provider value",
args: ["models", "auth", "list", "--json", "--provider", "--plain"],
provider: "--plain",
json: true,
},
])("classifies $name by its actual Commander role", async ({ args, provider, json }) => {
const program = createProgram();
let jsonMode = !json;
program.hook("preAction", (_command, actionCommand) => {
jsonMode = isCommandJsonOutputMode(actionCommand, process.argv);
});
const originalArgv = process.argv;
process.argv = ["node", "openclaw", ...args];
try {
await program.parseAsync(args, { from: "user" });
} finally {
process.argv = originalArgv;
}
expect(jsonMode).toBe(json);
expectCommandOptions(modelsAuthListCommand, { provider, json });
});
it.each([
["aliases list --plain", ["models", "aliases", "list", "--plain"]],
["fallbacks list --plain", ["models", "fallbacks", "list", "--plain"]],
+3 -1
View File
@@ -53,7 +53,9 @@ export function registerModelsCli(program: Command) {
);
const hasJsonOutput = (opts?: { json?: boolean }): boolean =>
Boolean(opts?.json || models.opts<{ json?: boolean }>().json);
setCommandJsonMode(models, "output", ({ argv }) => isModelsStatusJsonOutput(argv));
setCommandJsonMode(models, "output", ({ argv, command }) =>
isModelsStatusJsonOutput(argv, command),
);
models
.command("list")
+14 -6
View File
@@ -1,20 +1,28 @@
import type { Command } from "commander";
import { hasMachineOutputOption } from "./machine-output-argv.js";
import { resolveModelsParentCommandPath } from "./parent-command-path.js";
import { hasCommanderOptionToken } from "./program/commander-parse-facts.js";
function hasModelsOutputOption(argv: readonly string[], token: string, command?: Command): boolean {
return command
? hasCommanderOptionToken(command, argv, new Set([token]), "flag")
: hasMachineOutputOption(argv, token);
}
/** Resolve the parent-command alias for `models status --json`. */
export function isModelsStatusJsonOutput(argv: readonly string[]): boolean {
export function isModelsStatusJsonOutput(argv: readonly string[], command?: Command): boolean {
return (
hasMachineOutputOption(argv, "--json") ||
hasModelsOutputOption(argv, "--json", command) ||
(resolveModelsParentCommandPath(argv)?.length === 1 &&
hasMachineOutputOption(argv, "--status-json"))
hasModelsOutputOption(argv, "--status-json", command))
);
}
export function isModelsPlainMachineOutput(argv: readonly string[]): boolean {
export function isModelsPlainMachineOutput(argv: readonly string[], command?: Command): boolean {
const commandPath = resolveModelsParentCommandPath(argv);
return (
commandPath !== null &&
(hasMachineOutputOption(argv, "--plain") ||
(commandPath.length === 1 && hasMachineOutputOption(argv, "--status-plain")))
(hasModelsOutputOption(argv, "--plain", command) ||
(commandPath.length === 1 && hasModelsOutputOption(argv, "--status-plain", command)))
);
}
@@ -0,0 +1,66 @@
import { Command } from "commander";
import { afterEach, describe, expect, it, vi } from "vitest";
import { loggingState } from "../../logging/state.js";
const mocks = vi.hoisted(() => ({
ensureConfigReady: vi.fn(async () => {}),
routeLogsToStderr: vi.fn(),
}));
vi.mock("../../globals.js", () => ({ setVerbose: vi.fn() }));
vi.mock("../../runtime.js", () => ({
defaultRuntime: { error: vi.fn(), exit: vi.fn(), log: vi.fn() },
}));
vi.mock("../../logging/console.js", () => ({
routeLogsToStderr: mocks.routeLogsToStderr,
}));
vi.mock("../banner.js", () => ({ emitCliBanner: vi.fn() }));
vi.mock("../cli-name.js", () => ({ resolveCliName: () => "openclaw" }));
vi.mock("./config-guard.js", () => ({ ensureConfigReady: mocks.ensureConfigReady }));
vi.mock("../plugin-registry.js", () => ({ ensurePluginRegistryLoaded: vi.fn() }));
const originalArgv = [...process.argv];
const originalTitle = process.title;
const originalForceStderr = loggingState.forceConsoleToStderr;
const originalEarlyConsoleRoutingRestore = loggingState.earlyConsoleRoutingRestore;
describe("preaction model output owner", () => {
afterEach(() => {
vi.clearAllMocks();
process.argv = originalArgv;
process.title = originalTitle;
loggingState.forceConsoleToStderr = originalForceStderr;
loggingState.earlyConsoleRoutingRestore = originalEarlyConsoleRoutingRestore;
});
it.each([
{
name: "plain-looking provider value",
args: ["models", "auth", "list", "--provider", "--plain"],
},
{
name: "ignored parent plain alias and plain-looking provider value",
args: ["models", "--status-plain", "auth", "list", "--provider", "--plain"],
},
])("restores human stdout for $name", async ({ args }) => {
const program = new Command().name("openclaw").enablePositionalOptions();
program
.command("models")
.option("--status-plain")
.command("auth")
.command("list")
.option("--provider <id>")
.action(() => {});
const { registerPreActionHooks } = await import("./preaction.js");
registerPreActionHooks(program, "test");
loggingState.forceConsoleToStderr = true;
loggingState.earlyConsoleRoutingRestore = false;
process.argv = ["node", "openclaw", ...args];
await program.parseAsync(process.argv);
expect(loggingState.forceConsoleToStderr).toBe(false);
expect(mocks.routeLogsToStderr).not.toHaveBeenCalled();
});
});
+1 -1
View File
@@ -131,7 +131,7 @@ export function registerPreActionHooks(program: Command, programVersion: string)
return;
}
const jsonOutputMode = isCommandJsonOutputMode(actionCommand, argv);
const machineOutputMode = jsonOutputMode || isModelsPlainMachineOutput(argv);
const machineOutputMode = jsonOutputMode || isModelsPlainMachineOutput(argv, actionCommand);
applyResolvedCommandOutputMode(jsonOutputMode, machineOutputMode);
const { commandPath, startupPolicy } = resolveCliExecutionStartupContext({
argv,
+24
View File
@@ -781,6 +781,30 @@ describe("cli json stdout contract", () => {
);
});
it.each(["--plain", "--json"])(
"keeps human auth-list output on stdout when provider value is %s",
async (provider) => {
await withTempHome(
async (tempHome) => {
const result = runBuiltCli(tempHome, ["models", "auth", "list", "--provider", provider], {
CI: "1",
NO_COLOR: "1",
OPENCLAW_CONFIG_PATH: path.join(tempHome, "missing-openclaw.json"),
OPENCLAW_STATE_DIR: path.join(tempHome, "isolated-state"),
});
expect(result.status, result.stderr).toBe(0);
expect(result.stdout).toContain("Agent: main\n");
expect(result.stdout).toContain(`Provider: ${provider}\n`);
expect(result.stdout).toContain("Profiles: (none)\n");
expect(result.stderr).not.toContain("Agent: main");
expect(result.stderr).not.toContain(`Provider: ${provider}`);
},
{ prefix: "openclaw-models-output-option-value-e2e-" },
);
},
);
it("preserves model catalog refresh success payloads and persisted rows", async () => {
await withTempHome(
async (tempHome) => {