fix(cli): keep nodes json stdout clean#84423
Conversation
|
Codex review: needs maintainer review before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: no. live current-main reproduction was run in this read-only review. Source inspection shows the remaining gap: current main carries argv through subcommand registration, but the imported nodes registrar still reads ambient PR rating What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Next step before merge Security Review detailsBest possible solution: Land the focused argv-forwarding follow-up after maintainer review, preserving the existing JSON stderr helper and focused regression coverage. Do we have a high-confidence way to reproduce the issue? No live current-main reproduction was run in this read-only review. Source inspection shows the remaining gap: current main carries argv through subcommand registration, but the imported nodes registrar still reads ambient Is this the best way to solve the issue? Yes. Passing captured argv into Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 1f9ebb9dda36. |
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Sunspot Diff Drake Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
77758d2 to
a935d7d
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Rebased this onto current Verification after rebase:
Current head is signed: |
|
@clawsweeper re-review Current PR head has green Real behavior proof and check-guards, and the existing durable report body says there are no concrete contributor-facing blockers. Please refresh the durable verdict so the headline/labels match the current ready state. |
a935d7d to
59cb1d6
Compare
|
@clawsweeper re-review Rebased onto current origin/main, which includes the QA fixture knip ignore rename that fixes the failing check-dependencies unused-file report. Re-ran the focused CLI tests, oxfmt, git diff check, the full dependency deadcode shard pieces, and autoreview clean. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Added maintainer CLI proof to the PR body. Real proof summary:
{
"pending": [],
"paired": []
}The gateway/plugin startup logs and the scope-upgrade diagnostic were on stderr/gateway logs, not stdout. @clawsweeper re-review |
Summary
nodesCLI registrar soopenclaw nodes ... --jsongets the same protection as other lazy plugin commandsVerification
git diff --check origin/main...HEADpnpm exec oxfmt --check --threads=1 src/cli/json-output-mode.test.ts src/cli/nodes-cli/register.ts src/cli/program/register.subclis-core.ts src/cli/program/register.subclis.test.tsOPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/cli/json-output-mode.test.ts src/cli/program/register.subclis.test.ts --reporter=verbose(21 tests passed)pnpm deadcode:dependenciespnpm deadcode:unused-filespnpm deadcode:report:ci:ts-unused.agents/skills/autoreview/scripts/autoreview --mode branch(clean: no accepted/actionable findings)tbx_01ks1q2yrpg4xxytr9sjfeq3xt:CI=1 NODE_OPTIONS=--max-old-space-size=4096 OPENCLAW_TEST_PROJECTS_PARALLEL=6 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=900000 OPENCLAW_TESTBOX=1 OPENCLAW_TESTBOX_REMOTE_RUN=1 node scripts/run-vitest.mjs src/cli/json-output-mode.test.ts src/cli/program/register.subclis.test.ts src/cli/run-main.exit.test.ts(99 tests passed)OPENCLAW_GATEWAY_TOKEN=<redacted> OPENCLAW_SKIP_CHANNELS=1 node scripts/run-node.mjs --dev gateway --resetthennode scripts/run-node.mjs --dev nodes list --json > nodes-list.stdout.json 2> nodes-list.stderr.txt; command exit 0 andJSON.parse(nodes-list.stdout.json)succeeded (stdoutBytes=36, keyspending,paired).tbx_01ks1qfdc3nxmaw1fdabba4n5x:CI=1 NODE_OPTIONS=--max-old-space-size=4096 OPENCLAW_TEST_PROJECTS_PARALLEL=6 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_TESTBOX=1 OPENCLAW_TESTBOX_REMOTE_RUN=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.e2e.config.ts src/cli/program.nodes-basic.e2e.test.ts test/cli-json-stdout.e2e.test.ts(12 tests passed)Refs #84293
Real behavior proof
Behavior addressed:
openclaw nodes list --jsonno longer lets lazy plugin registration logs write to stdout before JSON output.Real environment tested: Blacksmith Testbox through Crabbox plus WSL-native maintainer worktree after rebase onto current
origin/main.Exact steps or command run after this patch:
OPENCLAW_GATEWAY_TOKEN=<redacted> OPENCLAW_SKIP_CHANNELS=1 node scripts/run-node.mjs --dev gateway --resetin a tempOPENCLAW_HOME, thennode scripts/run-node.mjs --dev nodes list --json > nodes-list.stdout.json 2> nodes-list.stderr.txt, followed byJSON.parse(nodes-list.stdout.json); also ran the focused unit/e2e/Testbox/deadcode proof listed above.Evidence after fix: the real CLI run exited 0; stdout was exactly parseable JSON (
stdoutBytes=36, top-level object keyspending,paired); stderr contained gateway connection/pairing diagnostics and gateway/plugin startup logs stayed outside stdout. Local focused CLI tests passed 21 tests; the exact previously failing deadcode unused-file shard passed after rebasing onto currentorigin/main; Testboxtbx_01ks1q2yrpg4xxytr9sjfeq3xtpassed 99 tests; Testboxtbx_01ks1qfdc3nxmaw1fdabba4n5xpassed 12 e2e tests.Observed result after fix:
nodes-list.stdout.jsoncontained only{ "pending": [], "paired": [] }and parsed successfully; the non-JSON gateway messagescope upgrade pending approvalappeared on stderr, not stdout. JSON-mode registration temporarily forces console logs to stderr, restores prior logging state afterward, andnodeslazy registration receives argv for--jsondetection.What was not tested: a live LCM plugin install inside the original reporter's agent container was not exercised; the regression is covered through the CLI registration path and existing nodes/json stdout e2e suites.