fix: route logs to stderr in --json output mode#52063
fix: route logs to stderr in --json output mode#52063xl0shk wants to merge 2 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes a bug where log output could leak to stdout when using Changes:
The fix is minimal and targeted, reusing the existing Confidence Score: 5/5
Last reviewed commit: "CLI: route logs to s..." |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 959d66c566
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const suppressDoctorStdout = isJsonOutputMode(commandPath, argv); | ||
| if (suppressDoctorStdout) { | ||
| routeLogsToStderr(); | ||
| } |
There was a problem hiding this comment.
--json command output on stdout
runCli() enables console capture before the Commander path is built (src/cli/run-main.ts:113-116), so calling routeLogsToStderr() here flips every later console.log/runtime.log write onto stderr. Many non-routed JSON commands emit their actual machine-readable payload through runtime.log, for example src/commands/channels/list.ts:122-129. After this change, openclaw channels list --json will leave stdout empty and put the JSON on stderr, which breaks any caller that parses stdout.
Useful? React with 👍 / 👎.
| const suppressDoctorStdout = hasFlag(params.argv, "--json"); | ||
| if (suppressDoctorStdout) { | ||
| routeLogsToStderr(); | ||
| } |
There was a problem hiding this comment.
In the routed path this call only sets loggingState.forceConsoleToStderr, but runCli() does not call enableConsoleCapture() until after tryRouteCli() has already been skipped (src/cli/run-main.ts:109-114). That means routed commands still use the native console.log/defaultRuntime.log, so textual diagnostics continue to leak to stdout. A concrete case is memory status --json: withMemoryManagerForAgent() can emit defaultRuntime.log(error ?? "Memory search disabled.") in src/cli/memory-cli.ts:138 before the JSON payload at line 425, so stdout is still not parseable JSON in that scenario.
Useful? React with 👍 / 👎.
|
The two CI failures are unrelated to this change:
All other 30+ checks pass, including the directly relevant Requesting a re-run of the failed jobs. |
1 participant
Fixes #52032
Summary
routeLogsToStderr()in both CLI execution paths (fast-path routes and Commander preAction hook) when--jsonis detected, so all log output goes to stderr and stdout stays clean for JSON parsing.Root Cause
The CLI has two execution paths that both need the fix:
src/cli/route.ts):prepareRoutedCommand()detected--jsonbut only used it forsuppressDoctorStdout, never calledrouteLogsToStderr().src/cli/program/preaction.ts): Same issue — detected--jsonviaisJsonOutputMode()but didn't callrouteLogsToStderr().The existing
routeLogsToStderr()function (already used incompletion-cli.tsfor the same reason) simply setsloggingState.forceConsoleToStderr = true, which the subsystem logger'swriteConsoleLine()checks to route output to stderr.Changes
src/cli/route.ts: callrouteLogsToStderr()inprepareRoutedCommand()when--jsonflag is presentsrc/cli/program/preaction.ts: callrouteLogsToStderr()in the preAction hook whenisJsonOutputMode()returns truesrc/cli/route.test.tsandsrc/cli/program/preaction.test.tsTest plan
pnpm test -- src/cli/route.test.ts src/cli/program/preaction.test.ts— 15/15 passpnpm check— all pass