diff --git a/src/cli/models-cli.test.ts b/src/cli/models-cli.test.ts index 891d43a40e9d..3392afd5b5dd 100644 --- a/src/cli/models-cli.test.ts +++ b/src/cli/models-cli.test.ts @@ -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"]], diff --git a/src/cli/models-cli.ts b/src/cli/models-cli.ts index ec49f0a552f8..1260313a91e5 100644 --- a/src/cli/models-cli.ts +++ b/src/cli/models-cli.ts @@ -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") diff --git a/src/cli/models-output-mode.ts b/src/cli/models-output-mode.ts index f8ba7a1f602b..7e5f8dbfa699 100644 --- a/src/cli/models-output-mode.ts +++ b/src/cli/models-output-mode.ts @@ -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))) ); } diff --git a/src/cli/program/preaction-output-mode.test.ts b/src/cli/program/preaction-output-mode.test.ts new file mode 100644 index 000000000000..af16a0d522cc --- /dev/null +++ b/src/cli/program/preaction-output-mode.test.ts @@ -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 ") + .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(); + }); +}); diff --git a/src/cli/program/preaction.ts b/src/cli/program/preaction.ts index b79ece805dae..8c20d76c6f7c 100644 --- a/src/cli/program/preaction.ts +++ b/src/cli/program/preaction.ts @@ -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, diff --git a/test/cli-json-stdout.e2e.test.ts b/test/cli-json-stdout.e2e.test.ts index 7bbbf264fbf6..1e4c3543d021 100644 --- a/test/cli-json-stdout.e2e.test.ts +++ b/test/cli-json-stdout.e2e.test.ts @@ -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) => {