mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-07 22:44:16 +00:00
fix(cli): keep json preflight stdout machine-readable
This commit is contained in:
@@ -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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -39,14 +39,27 @@ async function getConfigSnapshot() {
|
||||
export async function ensureConfigReady(params: {
|
||||
runtime: RuntimeEnv;
|
||||
commandPath?: string[];
|
||||
suppressDoctorStdout?: boolean;
|
||||
}): Promise<void> {
|
||||
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();
|
||||
|
||||
@@ -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("<path>")
|
||||
.argument("<value>")
|
||||
.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"],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
|
||||
77
src/cli/route.test.ts
Normal file
77
src/cli/route.test.ts
Normal file
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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) {
|
||||
|
||||
44
test/cli-json-stdout.e2e.test.ts
Normal file
44
test/cli-json-stdout.e2e.test.ts
Normal file
@@ -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-" },
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user