From be509faf94c6d4f49cbdb544ddeb604d654880c8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 24 Aug 2026 14:15:23 -0700 Subject: [PATCH] fix(cli): emit JSON parse errors for default-machine commands (#128861) --- src/cli/program/error-output.test.ts | 262 ++++++++++++++++++++++++++- src/cli/program/openclaw-command.ts | 9 +- 2 files changed, 268 insertions(+), 3 deletions(-) diff --git a/src/cli/program/error-output.test.ts b/src/cli/program/error-output.test.ts index 25df06c68931..b878b83c582c 100644 --- a/src/cli/program/error-output.test.ts +++ b/src/cli/program/error-output.test.ts @@ -1,11 +1,17 @@ // Error output tests cover program-level error display and exit messaging. import { CommanderError, InvalidArgumentError } from "commander"; import { describe, expect, it } from "vitest"; -import { ExpectedCliError } from "../failure-output.js"; +import { createCronOutputCommand, isCronMachineOutput } from "../cron-cli/output-mode.js"; +import { isDevicesMachineOutput } from "../devices-output-mode.js"; +import { ExpectedCliError, formatCliJsonFailure } from "../failure-output.js"; import { isJsonOutputModeActive, withConsoleLogsRoutedToStderrForJson, } from "../json-output-mode.js"; +import { isNodesMachineOutput } from "../nodes-cli/output-mode.js"; +import { isProxyMachineOutput } from "../proxy-output-mode.js"; +import { isSkillsMachineOutput } from "../skills-output-mode.js"; +import { isSystemMachineOutput } from "../system-output-mode.js"; import { getCommanderErrorCommandNames, getCommanderErrorCommandPath, @@ -15,6 +21,7 @@ import { createCliUnknownCommandError, formatCliParseErrorOutput, } from "./error-output.js"; +import { setCommandJsonMode } from "./json-mode.js"; import { OpenClawCommand } from "./openclaw-command.js"; import { registerLazyCommand } from "./register-lazy-command.js"; @@ -70,6 +77,259 @@ async function parseLazyGroupError(params: { } describe("formatCliParseErrorOutput", () => { + it.each([ + { + name: "automation lookup", + args: ["cron", "get"], + root: "cron", + children: ["get"], + argument: "", + message: 'Missing required argument "id".', + machineOutput: isCronMachineOutput, + }, + { + name: "profiled automation alias", + args: ["--profile", "work", "automations", "runs"], + root: "cron", + alias: "automations", + children: ["runs"], + requiredOption: "--id ", + message: 'Missing required option "--id ".', + machineOutput: isCronMachineOutput, + }, + { + name: "raw automation scratch", + args: ["cron", "scratch"], + root: "cron", + children: ["scratch"], + argument: "", + message: 'Missing required argument "id".', + machineOutput: isCronMachineOutput, + }, + { + name: "skill verification", + args: ["skills", "verify"], + root: "skills", + children: ["verify"], + argument: "", + message: 'Missing required argument "ref".', + machineOutput: isSkillsMachineOutput, + }, + { + name: "node invocation", + args: ["nodes", "invoke"], + root: "nodes", + children: ["invoke"], + requiredOption: "--node ", + message: 'Missing required option "--node ".', + machineOutput: isNodesMachineOutput, + }, + { + name: "node approval", + args: ["nodes", "approve"], + root: "nodes", + children: ["approve"], + argument: "", + message: 'Missing required argument "requestId".', + machineOutput: isNodesMachineOutput, + }, + { + name: "device rotation", + args: ["devices", "rotate"], + root: "devices", + children: ["rotate"], + requiredOption: "--device ", + message: 'Missing required option "--device ".', + machineOutput: isDevicesMachineOutput, + }, + { + name: "raw proxy blob", + args: ["proxy", "blob"], + root: "proxy", + children: ["blob"], + argument: "", + message: 'Missing required argument "blobId".', + machineOutput: isProxyMachineOutput, + }, + { + name: "nested system heartbeat", + args: ["system", "heartbeat", "last", "--unknown"], + root: "system", + children: ["heartbeat", "last"], + message: 'OpenClaw does not recognize option "--unknown".', + machineOutput: isSystemMachineOutput, + }, + ])("keeps $name parse failures machine-readable by default", async (testCase) => { + const originalArgv = process.argv; + process.argv = ["node", "openclaw", ...testCase.args]; + try { + const program = new OpenClawCommand() + .name("openclaw") + .enablePositionalOptions() + .option("--profile ") + .exitOverride(); + program.configureOutput({ writeErr: () => {} }); + const root = program.command(testCase.root); + if (testCase.alias) { + root.alias(testCase.alias); + } + setCommandJsonMode(root, "output", ({ argv }) => testCase.machineOutput(argv)); + + let command = root; + for (const child of testCase.children) { + command = + testCase.root === "cron" + ? createCronOutputCommand(command, child as "get" | "runs" | "scratch") + : command.command(child).option("--json"); + } + if (testCase.argument) { + command.argument(testCase.argument); + } + if (testCase.requiredOption) { + command.requiredOption(testCase.requiredOption); + } + command.action(() => {}); + + await withConsoleLogsRoutedToStderrForJson( + process.argv, + async () => { + const error = await program.parseAsync(process.argv).catch((cause: unknown) => cause); + + expect(error).toBeInstanceOf(ExpectedCliError); + expect(isJsonOutputModeActive(process.argv)).toBe(true); + expect(formatCliJsonFailure(error)).toEqual({ + ok: false, + error: { + type: "cli_error", + message: expect.stringContaining(testCase.message), + }, + }); + }, + { machineOutput: testCase.machineOutput(process.argv), restoreChanges: true }, + ); + } finally { + process.argv = originalArgv; + } + }); + + it("keeps a consumed JSON spelling machine-readable when the command owns JSON by default", async () => { + const originalArgv = process.argv; + process.argv = ["node", "openclaw", "cron", "status", "--limit", "--json"]; + try { + const program = new OpenClawCommand().name("openclaw").exitOverride(); + program.configureOutput({ writeErr: () => {} }); + const cron = program.command("cron"); + setCommandJsonMode(cron, "output", ({ argv }) => isCronMachineOutput(argv)); + createCronOutputCommand(cron, "status") + .option("--limit ", "Result limit", () => { + throw new InvalidArgumentError("--limit must be a positive integer."); + }) + .action(() => {}); + + await withConsoleLogsRoutedToStderrForJson( + process.argv, + async () => { + const error = await program.parseAsync(process.argv).catch((cause: unknown) => cause); + + expect(error).toBeInstanceOf(ExpectedCliError); + expect(isJsonOutputModeActive(process.argv)).toBe(true); + expect((error as ExpectedCliError).message).toContain( + "--limit must be a positive integer.", + ); + }, + { machineOutput: true, restoreChanges: true }, + ); + } finally { + process.argv = originalArgv; + } + }); + + it.each([ + { name: "human automation", args: ["cron", "show"], root: "cron", child: "show" }, + { + name: "parse-only config", + args: ["config", "set", "gateway.port", "--json"], + root: "config", + child: "set", + }, + { + name: "human skill card", + args: ["skills", "verify", "--card"], + root: "skills", + child: "verify", + }, + ])("keeps $name parse failures on the human error path", async (testCase) => { + const originalArgv = process.argv; + process.argv = ["node", "openclaw", ...testCase.args]; + try { + const program = new OpenClawCommand().name("openclaw").exitOverride(); + program.configureOutput({ writeErr: () => {} }); + const root = program.command(testCase.root); + if (testCase.root === "cron") { + setCommandJsonMode(root, "output", ({ argv }) => isCronMachineOutput(argv)); + } else if (testCase.root === "skills") { + setCommandJsonMode(root, "output", ({ argv }) => isSkillsMachineOutput(argv)); + } + const command = root.command(testCase.child).argument("").option("--json"); + if (testCase.root === "config") { + command.argument(""); + setCommandJsonMode(command, "parse-only", () => true); + } else if (testCase.root === "skills") { + command.option("--card"); + } + command.action(() => {}); + + await withConsoleLogsRoutedToStderrForJson( + process.argv, + async () => { + const error = await program.parseAsync(process.argv).catch((cause: unknown) => cause); + + expect(error).toBeInstanceOf(CommanderError); + expect(error).not.toBeInstanceOf(ExpectedCliError); + expect(isJsonOutputModeActive(process.argv)).toBe(false); + }, + { restoreChanges: true }, + ); + } finally { + process.argv = originalArgv; + } + }); + + it("keeps successful machine-command help outside the JSON failure path", async () => { + const originalArgv = process.argv; + process.argv = ["node", "openclaw", "cron", "get", "--help"]; + let stdout = ""; + try { + const program = new OpenClawCommand().name("openclaw").exitOverride(); + program.configureOutput({ + writeOut: (output) => { + stdout += output; + }, + writeErr: () => {}, + }); + const cron = program.command("cron"); + setCommandJsonMode(cron, "output", ({ argv }) => isCronMachineOutput(argv)); + createCronOutputCommand(cron, "get") + .argument("") + .action(() => {}); + + await withConsoleLogsRoutedToStderrForJson( + process.argv, + async () => { + const error = await program.parseAsync(process.argv).catch((cause: unknown) => cause); + + expect(error).toBeInstanceOf(CommanderError); + expect((error as CommanderError).exitCode).toBe(0); + expect(stdout).toContain("Usage: openclaw cron get"); + expect(isJsonOutputModeActive(process.argv)).toBe(false); + }, + { machineOutput: true, restoreChanges: true }, + ); + } finally { + process.argv = originalArgv; + } + }); + it.each([ { label: "JSON spelling", value: "--json", supportsJson: true }, { label: "true-valued JSON spelling", value: "--json=true", supportsJson: true }, diff --git a/src/cli/program/openclaw-command.ts b/src/cli/program/openclaw-command.ts index eaf154c1c645..11444f4c214c 100644 --- a/src/cli/program/openclaw-command.ts +++ b/src/cli/program/openclaw-command.ts @@ -9,6 +9,7 @@ import { setCommanderErrorCommand, } from "./commander-parse-facts.js"; import { createCliParseError } from "./error-output.js"; +import { isCommandJsonOutputMode } from "./json-mode.js"; // Commander 15 declares this help hook only in its runtime class, not its types. // Declaring it here lets the subclass override and delegate through `super` @@ -32,12 +33,16 @@ export class OpenClawCommand extends Command { if ( error instanceof CommanderError && error.exitCode !== 0 && - isJsonOutputModeActive(process.argv) + (isJsonOutputModeActive(process.argv) || isCommandJsonOutputMode(this, process.argv)) ) { - if (!hasCommanderOptionToken(this, process.argv, new Set(["--json"]), "flag")) { + if ( + !isCommandJsonOutputMode(this, process.argv) && + !hasCommanderOptionToken(this, process.argv, new Set(["--json"]), "flag") + ) { applyResolvedCommandOutputMode(false); throw error; } + applyResolvedCommandOutputMode(true); throw createCliParseError( message, {