fix(cli): route plugin logs to stderr during --json output

This commit is contained in:
Charles Dusek
2026-03-22 14:21:56 -05:00
committed by Peter Steinberger
parent 46a455d9e3
commit 0e1da034c2
4 changed files with 115 additions and 8 deletions

View File

@@ -1,5 +1,6 @@
import { Command } from "commander";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { loggingState } from "../../logging/state.js";
import { setCommandJsonMode } from "./json-mode.js";
const setVerboseMock = vi.fn();
@@ -58,6 +59,7 @@ let originalProcessArgv: string[];
let originalProcessTitle: string;
let originalNodeNoWarnings: string | undefined;
let originalHideBanner: string | undefined;
let originalForceStderr: boolean;
beforeAll(async () => {
({ registerPreActionHooks } = await import("./preaction.js"));
@@ -76,6 +78,8 @@ beforeEach(() => {
originalProcessTitle = process.title;
originalNodeNoWarnings = process.env.NODE_NO_WARNINGS;
originalHideBanner = process.env.OPENCLAW_HIDE_BANNER;
originalForceStderr = loggingState.forceConsoleToStderr;
loggingState.forceConsoleToStderr = false;
delete process.env.NODE_NO_WARNINGS;
delete process.env.OPENCLAW_HIDE_BANNER;
});
@@ -83,6 +87,7 @@ beforeEach(() => {
afterEach(() => {
process.argv = originalProcessArgv;
process.title = originalProcessTitle;
loggingState.forceConsoleToStderr = originalForceStderr;
if (originalNodeNoWarnings === undefined) {
delete process.env.NODE_NO_WARNINGS;
} else {
@@ -340,6 +345,39 @@ describe("registerPreActionHooks", () => {
expect(ensureConfigReadyMock).not.toHaveBeenCalled();
});
it("routes logs to stderr during plugin loading in --json mode and restores after", async () => {
let stderrDuringPluginLoad = false;
ensurePluginRegistryLoadedMock.mockImplementation(() => {
stderrDuringPluginLoad = loggingState.forceConsoleToStderr;
});
await runPreAction({
parseArgv: ["agents"],
processArgv: ["node", "openclaw", "agents", "--json"],
});
expect(ensurePluginRegistryLoadedMock).toHaveBeenCalled();
expect(stderrDuringPluginLoad).toBe(true);
// Flag must be restored after plugin loading completes
expect(loggingState.forceConsoleToStderr).toBe(false);
});
it("does not route logs to stderr during plugin loading without --json", async () => {
let stderrDuringPluginLoad = false;
ensurePluginRegistryLoadedMock.mockImplementation(() => {
stderrDuringPluginLoad = loggingState.forceConsoleToStderr;
});
await runPreAction({
parseArgv: ["agents"],
processArgv: ["node", "openclaw", "agents"],
});
expect(ensurePluginRegistryLoadedMock).toHaveBeenCalled();
expect(stderrDuringPluginLoad).toBe(false);
expect(loggingState.forceConsoleToStderr).toBe(false);
});
beforeAll(() => {
program = buildProgram();
const hooks = (

View File

@@ -3,6 +3,7 @@ import { setVerbose } from "../../globals.js";
import { isTruthyEnvValue } from "../../infra/env.js";
import { routeLogsToStderr } from "../../logging/console.js";
import type { LogLevel } from "../../logging/levels.js";
import { loggingState } from "../../logging/state.js";
import { defaultRuntime } from "../../runtime.js";
import { getCommandPathWithRootOptions, getVerboseFlag, hasHelpOrVersion } from "../argv.js";
import { emitCliBanner } from "../banner.js";
@@ -138,10 +139,21 @@ export function registerPreActionHooks(program: Command, programVersion: string)
commandPath,
...(jsonOutputMode ? { suppressDoctorStdout: true } : {}),
});
// Load plugins for commands that need channel access
<<<<<<< HEAD
// Load plugins for commands that need channel access.
// When --json output is active, temporarily route logs to stderr so plugin
// registration messages don't corrupt the JSON payload on stdout.
if (shouldLoadPluginsForCommand(commandPath, jsonOutputMode)) {
const { ensurePluginRegistryLoaded } = await loadPluginRegistryModule();
ensurePluginRegistryLoaded({ scope: resolvePluginRegistryScope(commandPath) });
const prev = loggingState.forceConsoleToStderr;
if (jsonOutputMode) {
loggingState.forceConsoleToStderr = true;
}
try {
ensurePluginRegistryLoaded({ scope: resolvePluginRegistryScope(commandPath) });
} finally {
loggingState.forceConsoleToStderr = prev;
}
}
});
}

View File

@@ -34,7 +34,11 @@ vi.mock("../runtime.js", () => ({
describe("tryRouteCli", () => {
let tryRouteCli: typeof import("./route.js").tryRouteCli;
// After vi.resetModules(), reimported modules get fresh loggingState.
// Capture the same reference that route.js uses.
let loggingState: typeof import("../logging/state.js").loggingState;
let originalDisableRouteFirst: string | undefined;
let originalForceStderr: boolean;
beforeEach(async () => {
vi.clearAllMocks();
@@ -42,6 +46,9 @@ describe("tryRouteCli", () => {
delete process.env.OPENCLAW_DISABLE_ROUTE_FIRST;
vi.resetModules();
({ tryRouteCli } = await import("./route.js"));
({ loggingState } = await import("../logging/state.js"));
originalForceStderr = loggingState.forceConsoleToStderr;
loggingState.forceConsoleToStderr = false;
findRoutedCommandMock.mockReturnValue({
loadPlugins: (argv: string[]) => !argv.includes("--json"),
run: runRouteMock,
@@ -49,6 +56,9 @@ describe("tryRouteCli", () => {
});
afterEach(() => {
if (loggingState) {
loggingState.forceConsoleToStderr = originalForceStderr;
}
if (originalDisableRouteFirst === undefined) {
delete process.env.OPENCLAW_DISABLE_ROUTE_FIRST;
} else {
@@ -78,6 +88,44 @@ describe("tryRouteCli", () => {
expect(ensurePluginRegistryLoadedMock).toHaveBeenCalledWith({ scope: "channels" });
});
it("routes logs to stderr during plugin loading in --json mode and restores after", async () => {
findRoutedCommandMock.mockReturnValue({
loadPlugins: true,
run: runRouteMock,
});
// Capture the value inside the mock callback using the same loggingState
// reference that route.js sees (both imported after vi.resetModules()).
const captured: boolean[] = [];
ensurePluginRegistryLoadedMock.mockImplementation(() => {
captured.push(loggingState.forceConsoleToStderr);
});
await tryRouteCli(["node", "openclaw", "agents", "--json"]);
expect(ensurePluginRegistryLoadedMock).toHaveBeenCalled();
expect(captured[0]).toBe(true);
expect(loggingState.forceConsoleToStderr).toBe(false);
});
it("does not route logs to stderr during plugin loading without --json", async () => {
findRoutedCommandMock.mockReturnValue({
loadPlugins: true,
run: runRouteMock,
});
const captured: boolean[] = [];
ensurePluginRegistryLoadedMock.mockImplementation(() => {
captured.push(loggingState.forceConsoleToStderr);
});
await tryRouteCli(["node", "openclaw", "agents"]);
expect(ensurePluginRegistryLoadedMock).toHaveBeenCalled();
expect(captured[0]).toBe(false);
expect(loggingState.forceConsoleToStderr).toBe(false);
});
it("routes status when root options precede the command", async () => {
await expect(tryRouteCli(["node", "openclaw", "--log-level", "debug", "status"])).resolves.toBe(
true,

View File

@@ -1,4 +1,5 @@
import { isTruthyEnvValue } from "../infra/env.js";
import { loggingState } from "../logging/state.js";
import { defaultRuntime } from "../runtime.js";
import { VERSION } from "../version.js";
import { getCommandPathWithRootOptions, hasFlag, hasHelpOrVersion } from "./argv.js";
@@ -22,12 +23,20 @@ async function prepareRoutedCommand(params: {
typeof params.loadPlugins === "function" ? params.loadPlugins(params.argv) : params.loadPlugins;
if (shouldLoadPlugins) {
const { ensurePluginRegistryLoaded } = await import("./plugin-registry.js");
ensurePluginRegistryLoaded({
scope:
params.commandPath[0] === "status" || params.commandPath[0] === "health"
? "channels"
: "all",
});
const prev = loggingState.forceConsoleToStderr;
if (suppressDoctorStdout) {
loggingState.forceConsoleToStderr = true;
}
try {
ensurePluginRegistryLoaded({
scope:
params.commandPath[0] === "status" || params.commandPath[0] === "health"
? "channels"
: "all",
});
} finally {
loggingState.forceConsoleToStderr = prev;
}
}
}