From 9e4a366ee632b5009622a24a64ba5bb38ee52a64 Mon Sep 17 00:00:00 2001 From: Altay Date: Mon, 23 Feb 2026 12:34:10 +0300 Subject: [PATCH] fix(cli): keep json preflight stdout machine-readable --- src/cli/program/config-guard.test.ts | 43 +++++++++++++++- src/cli/program/config-guard.ts | 21 ++++++-- src/cli/program/preaction.test.ts | 56 +++++++++++++++++++- src/cli/program/preaction.ts | 21 +++++++- src/cli/route.test.ts | 77 ++++++++++++++++++++++++++++ src/cli/route.ts | 9 +++- test/cli-json-stdout.e2e.test.ts | 44 ++++++++++++++++ 7 files changed, 260 insertions(+), 11 deletions(-) create mode 100644 src/cli/route.test.ts create mode 100644 test/cli-json-stdout.e2e.test.ts diff --git a/src/cli/program/config-guard.test.ts b/src/cli/program/config-guard.test.ts index f61590ebae3..d0d2dbf0342 100644 --- a/src/cli/program/config-guard.test.ts +++ b/src/cli/program/config-guard.test.ts @@ -34,10 +34,10 @@ describe("ensureConfigReady", () => { return await import("./config-guard.js"); } - async function runEnsureConfigReady(commandPath: string[]) { + async function runEnsureConfigReady(commandPath: string[], suppressDoctorStdout = false) { const runtime = makeRuntime(); const { ensureConfigReady } = await loadEnsureConfigReady(); - await ensureConfigReady({ runtime: runtime as never, commandPath }); + await ensureConfigReady({ runtime: runtime as never, commandPath, suppressDoctorStdout }); return runtime; } @@ -100,4 +100,43 @@ describe("ensureConfigReady", () => { expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledTimes(1); }); + + it("still runs doctor flow when stdout suppression is enabled", async () => { + await runEnsureConfigReady(["message"], true); + expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledTimes(1); + }); + + it("prevents preflight stdout noise when suppression is enabled", async () => { + const stdoutWrites: string[] = []; + const writeSpy = vi.spyOn(process.stdout, "write").mockImplementation(((chunk: unknown) => { + stdoutWrites.push(String(chunk)); + return true; + }) as typeof process.stdout.write); + loadAndMaybeMigrateDoctorConfigMock.mockImplementation(async () => { + process.stdout.write("Doctor warnings\n"); + }); + try { + await runEnsureConfigReady(["message"], true); + expect(stdoutWrites.join("")).not.toContain("Doctor warnings"); + } finally { + writeSpy.mockRestore(); + } + }); + + it("allows preflight stdout noise when suppression is not enabled", async () => { + const stdoutWrites: string[] = []; + const writeSpy = vi.spyOn(process.stdout, "write").mockImplementation(((chunk: unknown) => { + stdoutWrites.push(String(chunk)); + return true; + }) as typeof process.stdout.write); + loadAndMaybeMigrateDoctorConfigMock.mockImplementation(async () => { + process.stdout.write("Doctor warnings\n"); + }); + try { + await runEnsureConfigReady(["message"], false); + expect(stdoutWrites.join("")).toContain("Doctor warnings"); + } finally { + writeSpy.mockRestore(); + } + }); }); diff --git a/src/cli/program/config-guard.ts b/src/cli/program/config-guard.ts index 10ba913c12d..fac92899e40 100644 --- a/src/cli/program/config-guard.ts +++ b/src/cli/program/config-guard.ts @@ -39,14 +39,27 @@ async function getConfigSnapshot() { export async function ensureConfigReady(params: { runtime: RuntimeEnv; commandPath?: string[]; + suppressDoctorStdout?: boolean; }): Promise { const commandPath = params.commandPath ?? []; if (!didRunDoctorConfigFlow && shouldMigrateStateFromPath(commandPath)) { didRunDoctorConfigFlow = true; - await loadAndMaybeMigrateDoctorConfig({ - options: { nonInteractive: true }, - confirm: async () => false, - }); + const runDoctorConfigFlow = async () => + loadAndMaybeMigrateDoctorConfig({ + options: { nonInteractive: true }, + confirm: async () => false, + }); + if (!params.suppressDoctorStdout) { + await runDoctorConfigFlow(); + } else { + const originalStdoutWrite = process.stdout.write; + process.stdout.write = ((() => true) as unknown) as typeof process.stdout.write; + try { + await runDoctorConfigFlow(); + } finally { + process.stdout.write = originalStdoutWrite; + } + } } const snapshot = await getConfigSnapshot(); diff --git a/src/cli/program/preaction.test.ts b/src/cli/program/preaction.test.ts index a21374be427..ca20c46b52c 100644 --- a/src/cli/program/preaction.test.ts +++ b/src/cli/program/preaction.test.ts @@ -78,7 +78,18 @@ describe("registerPreActionHooks", () => { program.command("doctor").action(async () => {}); program.command("completion").action(async () => {}); program.command("secrets").action(async () => {}); - program.command("update").action(async () => {}); + program + .command("update") + .command("status") + .option("--json") + .action(async () => {}); + const config = program.command("config"); + config + .command("set") + .argument("") + .argument("") + .option("--json") + .action(async () => {}); program.command("channels").action(async () => {}); program.command("directory").action(async () => {}); program.command("agents").action(async () => {}); @@ -87,6 +98,7 @@ describe("registerPreActionHooks", () => { program .command("message") .command("send") + .option("--json") .action(async () => {}); registerPreActionHooks(program, "9.9.9-test"); return program; @@ -194,4 +206,46 @@ describe("registerPreActionHooks", () => { expect(emitCliBannerMock).not.toHaveBeenCalled(); expect(ensureConfigReadyMock).toHaveBeenCalledTimes(1); }); + + it("suppresses doctor stdout for any --json output command", async () => { + await runCommand({ + parseArgv: ["message", "send", "--json"], + processArgv: ["node", "openclaw", "message", "send", "--json"], + }); + + expect(ensureConfigReadyMock).toHaveBeenCalledWith({ + runtime: runtimeMock, + commandPath: ["message", "send"], + suppressDoctorStdout: true, + }); + + vi.clearAllMocks(); + + await runCommand({ + parseArgv: ["update", "status", "--json"], + processArgv: ["node", "openclaw", "update", "status", "--json"], + }); + + expect(ensureConfigReadyMock).toHaveBeenCalledWith({ + runtime: runtimeMock, + commandPath: ["update", "status"], + suppressDoctorStdout: true, + }); + }); + + it("does not treat config set --json (strict-parse alias) as json output mode", async () => { + await runCommand({ + parseArgv: ["config", "set", "gateway.auth.mode", "{bad", "--json"], + processArgv: ["node", "openclaw", "config", "set", "gateway.auth.mode", "{bad", "--json"], + }); + + const firstCall = ensureConfigReadyMock.mock.calls[0]?.[0] as + | { suppressDoctorStdout?: boolean } + | undefined; + expect(firstCall?.suppressDoctorStdout).toBeUndefined(); + expect(ensureConfigReadyMock).toHaveBeenCalledWith({ + runtime: runtimeMock, + commandPath: ["config", "set"], + }); + }); }); diff --git a/src/cli/program/preaction.ts b/src/cli/program/preaction.ts index 2e075e822ea..6871c7cc7d2 100644 --- a/src/cli/program/preaction.ts +++ b/src/cli/program/preaction.ts @@ -3,7 +3,7 @@ import { setVerbose } from "../../globals.js"; import { isTruthyEnvValue } from "../../infra/env.js"; import type { LogLevel } from "../../logging/levels.js"; import { defaultRuntime } from "../../runtime.js"; -import { getCommandPath, getVerboseFlag, hasHelpOrVersion } from "../argv.js"; +import { getCommandPath, getVerboseFlag, hasFlag, hasHelpOrVersion } from "../argv.js"; import { emitCliBanner } from "../banner.js"; import { resolveCliName } from "../cli-name.js"; @@ -30,6 +30,7 @@ const PLUGIN_REQUIRED_COMMANDS = new Set([ "onboard", ]); const CONFIG_GUARD_BYPASS_COMMANDS = new Set(["doctor", "completion", "secrets"]); +const JSON_PARSE_ONLY_COMMANDS = new Set(["config set"]); function getRootCommand(command: Command): Command { let current = command; @@ -51,6 +52,17 @@ function getCliLogLevel(actionCommand: Command): LogLevel | undefined { return typeof logLevel === "string" ? (logLevel as LogLevel) : undefined; } +function isJsonOutputMode(commandPath: string[], argv: string[]): boolean { + if (!hasFlag(argv, "--json")) { + return false; + } + const key = `${commandPath[0] ?? ""} ${commandPath[1] ?? ""}`.trim(); + if (JSON_PARSE_ONLY_COMMANDS.has(key)) { + return false; + } + return true; +} + export function registerPreActionHooks(program: Command, programVersion: string) { program.hook("preAction", async (_thisCommand, actionCommand) => { setProcessTitleForCommand(actionCommand); @@ -79,8 +91,13 @@ export function registerPreActionHooks(program: Command, programVersion: string) if (CONFIG_GUARD_BYPASS_COMMANDS.has(commandPath[0])) { return; } + const suppressDoctorStdout = isJsonOutputMode(commandPath, argv); const { ensureConfigReady } = await import("./config-guard.js"); - await ensureConfigReady({ runtime: defaultRuntime, commandPath }); + await ensureConfigReady({ + runtime: defaultRuntime, + commandPath, + ...(suppressDoctorStdout ? { suppressDoctorStdout: true } : {}), + }); // Load plugins for commands that need channel access if (PLUGIN_REQUIRED_COMMANDS.has(commandPath[0])) { const { ensurePluginRegistryLoaded } = await import("../plugin-registry.js"); diff --git a/src/cli/route.test.ts b/src/cli/route.test.ts new file mode 100644 index 00000000000..024a4d484ef --- /dev/null +++ b/src/cli/route.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const emitCliBannerMock = vi.hoisted(() => vi.fn()); +const ensureConfigReadyMock = vi.hoisted(() => vi.fn(async () => {})); +const ensurePluginRegistryLoadedMock = vi.hoisted(() => vi.fn()); +const findRoutedCommandMock = vi.hoisted(() => vi.fn()); +const runRouteMock = vi.hoisted(() => vi.fn(async () => true)); + +vi.mock("./banner.js", () => ({ + emitCliBanner: emitCliBannerMock, +})); + +vi.mock("./program/config-guard.js", () => ({ + ensureConfigReady: ensureConfigReadyMock, +})); + +vi.mock("./plugin-registry.js", () => ({ + ensurePluginRegistryLoaded: ensurePluginRegistryLoadedMock, +})); + +vi.mock("./program/routes.js", () => ({ + findRoutedCommand: findRoutedCommandMock, +})); + +vi.mock("../runtime.js", () => ({ + defaultRuntime: { error: vi.fn(), log: vi.fn(), exit: vi.fn() }, +})); + +describe("tryRouteCli", () => { + let tryRouteCli: typeof import("./route.js").tryRouteCli; + let originalDisableRouteFirst: string | undefined; + + beforeEach(async () => { + vi.clearAllMocks(); + originalDisableRouteFirst = process.env.OPENCLAW_DISABLE_ROUTE_FIRST; + delete process.env.OPENCLAW_DISABLE_ROUTE_FIRST; + vi.resetModules(); + ({ tryRouteCli } = await import("./route.js")); + findRoutedCommandMock.mockReturnValue({ + loadPlugins: false, + run: runRouteMock, + }); + }); + + afterEach(() => { + if (originalDisableRouteFirst === undefined) { + delete process.env.OPENCLAW_DISABLE_ROUTE_FIRST; + } else { + process.env.OPENCLAW_DISABLE_ROUTE_FIRST = originalDisableRouteFirst; + } + }); + + it("passes suppressDoctorStdout=true for routed --json commands", async () => { + await expect(tryRouteCli(["node", "openclaw", "status", "--json"])).resolves.toBe(true); + + expect(ensureConfigReadyMock).toHaveBeenCalledWith( + expect.objectContaining({ + commandPath: ["status"], + suppressDoctorStdout: true, + }), + ); + }); + + it("does not pass suppressDoctorStdout for routed non-json commands", async () => { + await expect(tryRouteCli(["node", "openclaw", "status"])).resolves.toBe(true); + + expect(ensureConfigReadyMock).toHaveBeenCalledWith( + expect.objectContaining({ + commandPath: ["status"], + }), + ); + const firstCall = ensureConfigReadyMock.mock.calls[0]?.[0] as + | { suppressDoctorStdout?: boolean } + | undefined; + expect(firstCall?.suppressDoctorStdout).toBeUndefined(); + }); +}); diff --git a/src/cli/route.ts b/src/cli/route.ts index e9929e6698b..2d86eeb036c 100644 --- a/src/cli/route.ts +++ b/src/cli/route.ts @@ -1,7 +1,7 @@ import { isTruthyEnvValue } from "../infra/env.js"; import { defaultRuntime } from "../runtime.js"; import { VERSION } from "../version.js"; -import { getCommandPath, hasHelpOrVersion } from "./argv.js"; +import { getCommandPath, hasFlag, hasHelpOrVersion } from "./argv.js"; import { emitCliBanner } from "./banner.js"; import { ensurePluginRegistryLoaded } from "./plugin-registry.js"; import { ensureConfigReady } from "./program/config-guard.js"; @@ -12,8 +12,13 @@ async function prepareRoutedCommand(params: { commandPath: string[]; loadPlugins?: boolean | ((argv: string[]) => boolean); }) { + const suppressDoctorStdout = hasFlag(params.argv, "--json"); emitCliBanner(VERSION, { argv: params.argv }); - await ensureConfigReady({ runtime: defaultRuntime, commandPath: params.commandPath }); + await ensureConfigReady({ + runtime: defaultRuntime, + commandPath: params.commandPath, + ...(suppressDoctorStdout ? { suppressDoctorStdout: true } : {}), + }); const shouldLoadPlugins = typeof params.loadPlugins === "function" ? params.loadPlugins(params.argv) : params.loadPlugins; if (shouldLoadPlugins) { diff --git a/test/cli-json-stdout.e2e.test.ts b/test/cli-json-stdout.e2e.test.ts new file mode 100644 index 00000000000..b3915dbb1af --- /dev/null +++ b/test/cli-json-stdout.e2e.test.ts @@ -0,0 +1,44 @@ +import { spawnSync } from "node:child_process"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { withTempHome } from "./helpers/temp-home.ts"; + +describe("cli json stdout contract", () => { + it("keeps `update status --json` stdout parseable even with legacy doctor preflight inputs", async () => { + await withTempHome( + async (tempHome) => { + const legacyDir = path.join(tempHome, ".clawdbot"); + await fs.mkdir(legacyDir, { recursive: true }); + await fs.writeFile(path.join(legacyDir, "clawdbot.json"), "{}", "utf8"); + + const env = { + ...process.env, + HOME: tempHome, + USERPROFILE: tempHome, + OPENCLAW_TEST_FAST: "1", + }; + delete env.OPENCLAW_HOME; + delete env.OPENCLAW_STATE_DIR; + delete env.OPENCLAW_CONFIG_PATH; + delete env.VITEST; + + const entry = path.resolve(process.cwd(), "openclaw.mjs"); + const result = spawnSync( + process.execPath, + [entry, "update", "status", "--json", "--timeout", "1"], + { cwd: process.cwd(), env, encoding: "utf8" }, + ); + + expect(result.status).toBe(0); + const stdout = result.stdout.trim(); + expect(stdout.length).toBeGreaterThan(0); + expect(() => JSON.parse(stdout)).not.toThrow(); + expect(stdout).not.toContain("Doctor warnings"); + expect(stdout).not.toContain("Doctor changes"); + expect(stdout).not.toContain("Config invalid"); + }, + { prefix: "openclaw-json-e2e-" }, + ); + }); +});