From 98b872e35788cfaeef61378eba7c60217d1a68d4 Mon Sep 17 00:00:00 2001 From: latenighthackathon Date: Tue, 2 Jun 2026 04:41:09 +0000 Subject: [PATCH 1/2] fix(status): keep stdout clean for status --json during gateway recovery (#4643) status --json builds its report by reconciling the gateway, and on a recoverable gateway error that reconcile streams human startup progress ("[2/8] Starting OpenShell gateway", "Starting gateway cluster...") to stdout via console.log. On the --json path those lines land ahead of the JSON document and make stdout unparseable. Build the report under a stdout guard that redirects writes to stderr, so the machine-readable document on stdout stays valid while recovery progress stays visible on stderr. The plain (non --json) status view is unchanged. Fixes #4643 Signed-off-by: latenighthackathon --- src/lib/actions/sandbox/status-snapshot.ts | 32 ++++++++++-- src/lib/cli/stdout-guard.ts | 28 ++++++++++ test/sandbox-status-json-stdout.test.ts | 59 +++++++++++++++++++++ test/stdout-guard.test.ts | 61 ++++++++++++++++++++++ 4 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 src/lib/cli/stdout-guard.ts create mode 100644 test/sandbox-status-json-stdout.test.ts create mode 100644 test/stdout-guard.test.ts diff --git a/src/lib/actions/sandbox/status-snapshot.ts b/src/lib/actions/sandbox/status-snapshot.ts index 6073fc5237..705c2c75dd 100644 --- a/src/lib/actions/sandbox/status-snapshot.ts +++ b/src/lib/actions/sandbox/status-snapshot.ts @@ -18,6 +18,7 @@ import { import { parseSandboxPhase } from "../../state/gateway"; import * as registry from "../../state/registry"; import { getSandboxDockerRuntime } from "./docker-health"; +import { withStdoutRedirectedToStderr } from "../../cli/stdout-guard"; import type { SandboxGatewayState } from "./gateway-state"; import { getReconciledSandboxGatewayState, @@ -106,8 +107,13 @@ export interface SandboxStatusSnapshot { inferenceHealth: ProviderHealthStatus | null; } +type ReconcileSandboxGatewayState = ( + sandboxName: string, +) => Promise; + interface CollectSandboxStatusSnapshotDeps { probeProviderHealthImpl?: ProbeProviderHealth; + reconcile?: ReconcileSandboxGatewayState; } export async function collectSandboxStatusSnapshot( @@ -117,12 +123,16 @@ export async function collectSandboxStatusSnapshot( deps?: CollectSandboxStatusSnapshotDeps; } = {}, ): Promise { + const reconcile = + opts.deps?.reconcile ?? + ((name: string) => + getReconciledSandboxGatewayState(name, { + getState: getSandboxGatewayStateForStatus, + })); const sb = registry.getSandbox(sandboxName); let lookup: SandboxGatewayState; try { - lookup = await getReconciledSandboxGatewayState(sandboxName, { - getState: getSandboxGatewayStateForStatus, - }); + lookup = await reconcile(sandboxName); } catch (err) { const message = err instanceof Error ? err.message : String(err); lookup = { @@ -190,10 +200,26 @@ export async function collectSandboxStatusSnapshot( export async function getSandboxStatusReport( sandboxName: string, + deps: CollectSandboxStatusSnapshotDeps = {}, +): Promise { + // The report is the machine-readable (--json) payload the CLI prints on + // stdout. Building it reconciles the gateway, and that path prints human + // progress to stdout via console.log (step(), gateway-start streaming). + // Redirect any such writes to stderr while the report is built so stdout + // carries only the JSON document. + return withStdoutRedirectedToStderr(() => + buildSandboxStatusReport(sandboxName, deps), + ); +} + +async function buildSandboxStatusReport( + sandboxName: string, + deps: CollectSandboxStatusSnapshotDeps, ): Promise { const preflight = await getSandboxStatusPreflight(registry.getSandbox(sandboxName)); const snapshot = await collectSandboxStatusSnapshot(sandboxName, { suppressInferenceProbe: preflight.suppressInferenceProbe, + deps, }); const { sb, lookup, rpcIssue, currentModel, currentProvider, inferenceHealth } = snapshot; const dockerRuntime = diff --git a/src/lib/cli/stdout-guard.ts b/src/lib/cli/stdout-guard.ts new file mode 100644 index 0000000000..5a263bee3d --- /dev/null +++ b/src/lib/cli/stdout-guard.ts @@ -0,0 +1,28 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Run `fn` with writes to `process.stdout` sent to `process.stderr` instead, + * restoring the original stdout writer afterwards (even if `fn` throws). + * + * Machine-readable command paths (`--json`) emit a structured document on + * stdout. Some shared code they call prints human-facing progress to stdout + * via `console.log` (for example, `status` reconciles the gateway and the + * recovery path streams gateway-start progress). On a `--json` path that + * progress would interleave with the JSON document and make stdout + * unparseable, so it is redirected to stderr, where it stays visible to a + * human without corrupting the machine output. + */ +export async function withStdoutRedirectedToStderr( + fn: () => Promise, +): Promise { + const originalStdoutWrite = process.stdout.write; + process.stdout.write = process.stderr.write.bind( + process.stderr, + ) as typeof process.stdout.write; + try { + return await fn(); + } finally { + process.stdout.write = originalStdoutWrite; + } +} diff --git a/test/sandbox-status-json-stdout.test.ts b/test/sandbox-status-json-stdout.test.ts new file mode 100644 index 0000000000..4983c053fe --- /dev/null +++ b/test/sandbox-status-json-stdout.test.ts @@ -0,0 +1,59 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { getSandboxStatusReport } from "../dist/lib/actions/sandbox/status-snapshot.js"; + +// `sandbox status --json` builds a machine-readable report through +// getSandboxStatusReport, which reconciles the gateway. When the gateway needs +// recovery, the reconcile path prints human progress to stdout (step(), +// streamGatewayStart, "Waiting for gateway health...", etc.). We inject a +// reconcile that writes that progress and assert the --json report builder +// keeps stdout clean; otherwise the JSON document on stdout is unparseable. +// Writes go through process.stdout.write directly (what console.log delegates +// to), so the test targets the exact stream the builder must keep clean. +describe("sandbox status --json keeps stdout clean during gateway recovery", () => { + let originalWrite: typeof process.stdout.write; + let captured: string[]; + + beforeEach(() => { + captured = []; + originalWrite = process.stdout.write.bind(process.stdout); + process.stdout.write = ((chunk: unknown, ...rest: unknown[]): boolean => { + captured.push(typeof chunk === "string" ? chunk : String(chunk)); + const cb = rest.find((a) => typeof a === "function") as undefined | (() => void); + if (cb) cb(); + return true; + }) as typeof process.stdout.write; + }); + + afterEach(() => { + process.stdout.write = originalWrite; + }); + + it("does not leak reconcile/recovery progress onto stdout (it would corrupt --json)", async () => { + const report = await getSandboxStatusReport("ghost-sandbox", { + reconcile: async () => { + process.stdout.write("\n [2/8] Starting OpenShell gateway\n"); + process.stdout.write(" Starting gateway cluster...\n"); + process.stdout.write(" Waiting for gateway health...\n"); + return { + state: "gateway_unreachable_after_restart", + output: "Gateway: nemoclaw\nStatus: unreachable", + }; + }, + }); + process.stdout.write = originalWrite; + + const onStdout = captured.join(""); + expect(onStdout).not.toContain("Starting OpenShell gateway"); + expect(onStdout).not.toContain("Starting gateway cluster"); + expect(onStdout).toBe(""); + + expect(report.schemaVersion).toBe(1); + expect(report.name).toBe("ghost-sandbox"); + expect(report.found).toBe(false); + expect(report.gatewayState).toBe("gateway_unreachable_after_restart"); + }); +}); diff --git a/test/stdout-guard.test.ts b/test/stdout-guard.test.ts new file mode 100644 index 0000000000..83d460c784 --- /dev/null +++ b/test/stdout-guard.test.ts @@ -0,0 +1,61 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, describe, expect, it } from "vitest"; + +import { withStdoutRedirectedToStderr } from "../dist/lib/cli/stdout-guard.js"; + +describe("withStdoutRedirectedToStderr", () => { + let restore: (() => void) | null = null; + + afterEach(() => { + if (restore) restore(); + restore = null; + }); + + it("sends stdout writes to stderr while the callback runs, and returns its value", async () => { + const out: string[] = []; + const err: string[] = []; + const origOut = process.stdout.write.bind(process.stdout); + const origErr = process.stderr.write.bind(process.stderr); + process.stdout.write = ((c: unknown) => { + out.push(String(c)); + return true; + }) as typeof process.stdout.write; + process.stderr.write = ((c: unknown) => { + err.push(String(c)); + return true; + }) as typeof process.stderr.write; + restore = () => { + process.stdout.write = origOut; + process.stderr.write = origErr; + }; + + const result = await withStdoutRedirectedToStderr(async () => { + process.stdout.write("progress line\n"); + return 42; + }); + + restore(); + restore = null; + expect(result).toBe(42); + expect(out.join("")).toBe(""); + expect(err.join("")).toContain("progress line"); + }); + + it("restores the original stdout writer after the callback resolves", async () => { + const before = process.stdout.write; + await withStdoutRedirectedToStderr(async () => undefined); + expect(process.stdout.write).toBe(before); + }); + + it("restores the original stdout writer even when the callback throws", async () => { + const before = process.stdout.write; + await expect( + withStdoutRedirectedToStderr(async () => { + throw new Error("boom"); + }), + ).rejects.toThrow("boom"); + expect(process.stdout.write).toBe(before); + }); +}); From 0525a45d7c284474eec921dbe6921db663a966f2 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Wed, 3 Jun 2026 10:50:12 -0700 Subject: [PATCH 2/2] fix(cli): keep more json commands stdout-clean --- .../global-oclif-command-adapters.test.ts | 45 ++++++++++++++++++ src/commands/list.ts | 8 +++- src/commands/sandbox/doctor.ts | 14 +++--- .../sandbox/oclif-command-adapters.test.ts | 46 +++++++++++++++++++ 4 files changed, 105 insertions(+), 8 deletions(-) diff --git a/src/commands/global-oclif-command-adapters.test.ts b/src/commands/global-oclif-command-adapters.test.ts index f2e1e56bd8..acef377a9d 100644 --- a/src/commands/global-oclif-command-adapters.test.ts +++ b/src/commands/global-oclif-command-adapters.test.ts @@ -117,6 +117,51 @@ describe("global oclif command adapters", () => { ); }); + it("keeps list --json stdout clean while inventory recovery prints progress", async () => { + const report = { + schemaVersion: 1, + defaultSandbox: null, + recovery: { recoveredFromSession: false, recoveredFromGateway: 0 }, + lastOnboardedSandbox: null, + sandboxes: [], + }; + mocks.getSandboxInventory.mockImplementationOnce(async () => { + process.stdout.write(" Starting OpenShell gateway\n"); + return report; + }); + + const out: string[] = []; + const err: string[] = []; + const origOut = process.stdout.write; + const origErr = process.stderr.write; + process.stdout.write = ((chunk: unknown, ...rest: unknown[]): boolean => { + out.push(typeof chunk === "string" ? chunk : String(chunk)); + const cb = rest.find((arg) => typeof arg === "function") as undefined | (() => void); + if (cb) cb(); + return true; + }) as typeof process.stdout.write; + process.stderr.write = ((chunk: unknown, ...rest: unknown[]): boolean => { + err.push(typeof chunk === "string" ? chunk : String(chunk)); + const cb = rest.find((arg) => typeof arg === "function") as undefined | (() => void); + if (cb) cb(); + return true; + }) as typeof process.stderr.write; + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + try { + await ListCommand.run(["--json"], rootDir); + } finally { + process.stdout.write = origOut; + process.stderr.write = origErr; + } + + const stdout = out.join(""); + expect(stdout).not.toContain("Starting OpenShell gateway"); + expect(stdout).toBe(""); + expect(JSON.parse(String(log.mock.calls.at(-1)?.[0]))).toEqual(report); + expect(err.join("")).toContain("Starting OpenShell gateway"); + }); + it("runs status through status helpers", async () => { await StatusCommand.run([], rootDir); diff --git a/src/commands/list.ts b/src/commands/list.ts index d1953a4785..948b81c4c3 100644 --- a/src/commands/list.ts +++ b/src/commands/list.ts @@ -3,6 +3,7 @@ import { getSandboxInventory, renderSandboxInventoryText } from "../lib/inventory"; import { NemoClawCommand } from "../lib/cli/nemoclaw-oclif-command"; +import { withStdoutRedirectedToStderr } from "../lib/cli/stdout-guard"; import { buildListCommandDeps } from "../lib/list-command-deps"; export default class ListCommand extends NemoClawCommand { @@ -19,8 +20,11 @@ export default class ListCommand extends NemoClawCommand { public async run(): Promise { await this.parse(ListCommand); const deps = buildListCommandDeps(); - const inventory = await getSandboxInventory(deps); - if (this.jsonEnabled()) { + const json = this.jsonEnabled(); + const inventory = json + ? await withStdoutRedirectedToStderr(() => getSandboxInventory(deps)) + : await getSandboxInventory(deps); + if (json) { return inventory; } diff --git a/src/commands/sandbox/doctor.ts b/src/commands/sandbox/doctor.ts index 44220834dc..40fe47eba2 100644 --- a/src/commands/sandbox/doctor.ts +++ b/src/commands/sandbox/doctor.ts @@ -3,6 +3,7 @@ import { Args } from "@oclif/core"; import { NemoClawCommand } from "../../lib/cli/nemoclaw-oclif-command"; +import { withStdoutRedirectedToStderr } from "../../lib/cli/stdout-guard"; import { runSandboxDoctor } from "../../lib/actions/sandbox/doctor"; @@ -25,12 +26,13 @@ export default class SandboxDoctorCliCommand extends NemoClawCommand { public async run(): Promise { const { args } = await this.parse(SandboxDoctorCliCommand); - const report = await runSandboxDoctor( - args.sandboxName, - this.jsonEnabled() ? ["--json"] : [], - { quietJson: this.jsonEnabled() }, - ); - if (this.jsonEnabled()) { + const json = this.jsonEnabled(); + const report = json + ? await withStdoutRedirectedToStderr(() => + runSandboxDoctor(args.sandboxName, ["--json"], { quietJson: true }), + ) + : await runSandboxDoctor(args.sandboxName, [], { quietJson: false }); + if (json) { if (report && report.failed > 0) process.exitCode = 1; return report; } diff --git a/src/commands/sandbox/oclif-command-adapters.test.ts b/src/commands/sandbox/oclif-command-adapters.test.ts index 88b90f999c..bbc47cea3e 100644 --- a/src/commands/sandbox/oclif-command-adapters.test.ts +++ b/src/commands/sandbox/oclif-command-adapters.test.ts @@ -186,4 +186,50 @@ describe("sandbox oclif command adapters", () => { expect(mocks.shieldsUp).toHaveBeenCalledWith("alpha"); expect(mocks.shieldsStatus).toHaveBeenCalledWith("alpha"); }); + + it("keeps doctor --json stdout clean while diagnostics recovery prints progress", async () => { + const report = { + schemaVersion: 1, + sandbox: "alpha", + status: "ok", + failed: 0, + warnings: 0, + checks: [], + }; + mocks.runSandboxDoctor.mockImplementationOnce(async () => { + process.stdout.write(" Starting OpenShell gateway\n"); + return report; + }); + + const out: string[] = []; + const err: string[] = []; + const origOut = process.stdout.write; + const origErr = process.stderr.write; + process.stdout.write = ((chunk: unknown, ...rest: unknown[]): boolean => { + out.push(typeof chunk === "string" ? chunk : String(chunk)); + const cb = rest.find((arg) => typeof arg === "function") as undefined | (() => void); + if (cb) cb(); + return true; + }) as typeof process.stdout.write; + process.stderr.write = ((chunk: unknown, ...rest: unknown[]): boolean => { + err.push(typeof chunk === "string" ? chunk : String(chunk)); + const cb = rest.find((arg) => typeof arg === "function") as undefined | (() => void); + if (cb) cb(); + return true; + }) as typeof process.stderr.write; + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + try { + await SandboxDoctorCliCommand.run(["alpha", "--json"], rootDir); + } finally { + process.stdout.write = origOut; + process.stderr.write = origErr; + } + + const stdout = out.join(""); + expect(stdout).not.toContain("Starting OpenShell gateway"); + expect(stdout).toBe(""); + expect(JSON.parse(String(log.mock.calls.at(-1)?.[0]))).toEqual(report); + expect(err.join("")).toContain("Starting OpenShell gateway"); + }); });