From 4f4c02bf9fc61a95b7febeb6a826037d83db1ae0 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 3 Sep 2026 14:51:46 -0700 Subject: [PATCH 01/11] fix(cli): add global host and gateway doctor Signed-off-by: Prekshi Vyas --- docs/reference/commands.mdx | 21 +++ src/commands/doctor.test.ts | 89 +++++++++ src/commands/doctor.ts | 29 +++ src/commands/sandbox/doctor.ts | 6 +- src/lib/actions/doctor.test.ts | 174 +++++++++++++++++ src/lib/actions/sandbox/doctor-report.test.ts | 27 ++- src/lib/actions/sandbox/doctor-report.ts | 34 +++- .../actions/sandbox/doctor-system-checks.ts | 4 + src/lib/actions/sandbox/doctor.ts | 178 ++++++++++++------ src/lib/cli/doctor-command-support.ts | 5 + src/lib/cli/public-dispatch.ts | 19 ++ src/lib/cli/public-display-defaults.ts | 7 + test/cli/dispatch-basics.test.ts | 49 ++++- test/cli/root-help.test.ts | 2 + .../cli/command-registry.test.ts | 4 +- 15 files changed, 575 insertions(+), 73 deletions(-) create mode 100644 src/commands/doctor.test.ts create mode 100644 src/commands/doctor.ts create mode 100644 src/lib/actions/doctor.test.ts create mode 100644 src/lib/cli/doctor-command-support.ts diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index b73954e9705..6207e113580 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1569,6 +1569,7 @@ If the sandbox is running an older Deep Agents Code version than this NemoClaw r ### `$$nemoclaw doctor` Run a focused health check for one sandbox and the host services it depends on. The command checks the local CLI build, Docker daemon, OpenShell CLI, NemoClaw gateway container, gateway port mapping, live sandbox state, inference route, configured-provider model invocation, Ollama reachability, and the cloudflared tunnel state. +Use `$$nemoclaw doctor` when you need only the host and gateway checks or when no sandbox exists yet. For gateway-based agents, it also reports messaging channel conflicts within the selected @@ -3144,6 +3145,26 @@ This command remains as a compatibility alias to `$$nemoclaw onboard` and accept $$nemoclaw setup-spark ``` +### `$$nemoclaw doctor` + +Run read-only checks for the host and the NemoClaw gateway without selecting a sandbox. +The command works before you create a sandbox. +It checks the NemoClaw CLI build, the selected host runtime provider, the OpenShell CLI, the sandbox registry, and the gateway selected by `NEMOCLAW_GATEWAY_PORT`. +It does not start, select, restart, or repair a gateway. +It does not run sandbox, inference, messaging, agent-version, config-permission, or agent-service checks. +Use `$$nemoclaw doctor` when you need those sandbox checks. + +```bash +$$nemoclaw doctor [--json] +``` + +The command exits nonzero when a required check fails. +Pass `--json` for a redacted, schema-versioned report with `scope: "global"` and no sandbox field. + +| Flag | Description | +| --- | --- | +| `--json` | Emit the global report as JSON. | + ### `$$nemoclaw debug` Collect diagnostics for bug reports. Gathers system info, Docker state, gateway logs, and sandbox status into a summary or tarball. Use `--sandbox ` to target a specific sandbox, `--quick` for a smaller snapshot, or `--output ` to save a tarball that you can attach to an issue. diff --git a/src/commands/doctor.test.ts b/src/commands/doctor.test.ts new file mode 100644 index 00000000000..8dc0b7afe96 --- /dev/null +++ b/src/commands/doctor.test.ts @@ -0,0 +1,89 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { testTimeoutOptions } from "../../test/helpers/timeouts"; + +const mocks = vi.hoisted(() => ({ + runGlobalDoctor: vi.fn(), +})); + +vi.mock("../lib/actions/sandbox/doctor", () => ({ + runGlobalDoctor: mocks.runGlobalDoctor, +})); + +import DoctorCommand from "./doctor"; + +const rootDir = process.cwd(); + +describe("global doctor command", () => { + beforeEach(() => { + vi.clearAllMocks(); + process.exitCode = undefined; + mocks.runGlobalDoctor.mockResolvedValue({ + schemaVersion: 1, + scope: "global", + status: "ok", + failed: 0, + warnings: 0, + checks: [], + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + process.exitCode = undefined; + }); + + it( + "runs the read-only text diagnosis without a sandbox (#10212)", + testTimeoutOptions(30_000), + async () => { + await DoctorCommand.run([], rootDir); + + expect(mocks.runGlobalDoctor).toHaveBeenCalledWith(); + expect(process.exitCode).toBeUndefined(); + }, + ); + + it("returns redacted JSON and a nonzero status for failed checks (#10212)", async () => { + mocks.runGlobalDoctor.mockResolvedValueOnce({ + schemaVersion: 1, + scope: "global", + status: "fail", + failed: 1, + warnings: 0, + checks: [ + { + group: "Gateway", + label: "OpenShell status", + status: "fail", + detail: "Authorization: Bearer sk-abc123DEF456ghi789", + }, + ], + }); + + const report = (await DoctorCommand.run(["--json"], rootDir)) as { + checks: Array<{ detail: string }>; + scope: string; + }; + + expect(mocks.runGlobalDoctor).toHaveBeenCalledWith({ quiet: true }); + expect(process.exitCode).toBe(1); + expect(report.scope).toBe("global"); + expect(report.checks[0]?.detail).toBe("Authorization: Bearer "); + expect(JSON.stringify(report)).not.toContain("sk-abc123DEF456ghi789"); + }); + + it("rejects global --fix before running health checks (#10212)", async () => { + await expect(DoctorCommand.run(["--fix"], rootDir)).rejects.toThrow(/fix/i); + + expect(mocks.runGlobalDoctor).not.toHaveBeenCalled(); + }); + + it("shows global help without running health checks (#10212)", async () => { + await expect(DoctorCommand.run(["--help"], rootDir)).rejects.toThrow(/EEXIT: 0/); + + expect(mocks.runGlobalDoctor).not.toHaveBeenCalled(); + }); +}); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts new file mode 100644 index 00000000000..d69054ab099 --- /dev/null +++ b/src/commands/doctor.ts @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { Command, Flags } from "@oclif/core"; +import { runGlobalDoctor } from "../lib/actions/sandbox/doctor"; +import { redactForLog, withStdoutRedirectedToStderr } from "../lib/cli/doctor-command-support"; + +export default class DoctorCommand extends Command { + static baseFlags = { help: Flags.help({ char: "h" }) }; + static id = "doctor"; + static strict = true; + static enableJsonFlag = true; + static summary = "Diagnose host and gateway health"; + static description = + "Run read-only host, runtime provider, OpenShell CLI, sandbox registry, and NemoClaw gateway checks. Use ` doctor` for one sandbox."; + static usage = ["doctor [--json]"]; + static examples = ["<%= config.bin %> doctor", "<%= config.bin %> doctor --json"]; + static flags = {}; + + public async run(): Promise { + await this.parse(DoctorCommand); + const json = this.jsonEnabled(); + const report = json + ? await withStdoutRedirectedToStderr(() => runGlobalDoctor({ quiet: true })) + : await runGlobalDoctor(); + if (report.failed > 0) process.exitCode = 1; + return json ? redactForLog(report) : undefined; + } +} diff --git a/src/commands/sandbox/doctor.ts b/src/commands/sandbox/doctor.ts index d98e55e5a24..0a1d004e1bd 100644 --- a/src/commands/sandbox/doctor.ts +++ b/src/commands/sandbox/doctor.ts @@ -3,9 +3,11 @@ import { Args, Flags } from "@oclif/core"; import { runSandboxDoctor } from "../../lib/actions/sandbox/doctor"; +import { + redactForLog, + withStdoutRedirectedToStderr, +} from "../../lib/cli/doctor-command-support"; import { NemoClawCommand } from "../../lib/cli/nemoclaw-oclif-command"; -import { withStdoutRedirectedToStderr } from "../../lib/cli/stdout-guard"; -import { redactForLog } from "../../lib/security/redact"; export default class SandboxDoctorCliCommand extends NemoClawCommand { static id = "sandbox:doctor"; diff --git a/src/lib/actions/doctor.test.ts b/src/lib/actions/doctor.test.ts new file mode 100644 index 00000000000..1230d5033f8 --- /dev/null +++ b/src/lib/actions/doctor.test.ts @@ -0,0 +1,174 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + dockerInspectGateway: vi.fn(), + getNamedGatewayLifecycleState: vi.fn(), + inspectHost: vi.fn(), + listSandboxes: vi.fn(), + recoverNamedGatewayRuntime: vi.fn(), + resolveOpenshell: vi.fn(), + shouldInspectLegacyGatewayContainer: vi.fn(), +})); + +vi.mock("../adapters/openshell/resolve", () => ({ + resolveOpenshell: mocks.resolveOpenshell, +})); + +vi.mock("../gateway-runtime-action", () => ({ + getNamedGatewayLifecycleState: mocks.getNamedGatewayLifecycleState, + recoverNamedGatewayRuntime: mocks.recoverNamedGatewayRuntime, +})); + +vi.mock("../onboard/gateway-binding", () => ({ + resolveGatewayName: () => "nemoclaw", + resolveSandboxGatewayName: vi.fn(), +})); + +vi.mock("../onboard/runtime-provider/access", () => ({ + CURRENT_RUNTIME_PROVIDER_BUNDLES: [], + RuntimeProviderSelectionError: class RuntimeProviderSelectionError extends Error {}, + requireRuntimeProviderBundle: vi.fn(), + resolveCurrentRuntimeProviderBundle: () => ({ + preflightDoctor: { inspectHost: mocks.inspectHost }, + }), +})); + +vi.mock("../runner", () => ({ ROOT: "/repo" })); + +vi.mock("../state/registry", () => ({ + listSandboxes: mocks.listSandboxes, +})); + +vi.mock("./sandbox/doctor-lifecycle-registration", () => ({ + buildPortableRuntimeCheck: () => null, +})); + +vi.mock("./sandbox/doctor-system-checks", () => ({ + dockerInspectGateway: mocks.dockerInspectGateway, + gatewayDoctorStartHint: () => + "Start the gateway again with `nemoclaw onboard`. Then retry this command.", + oneLine: (value = "") => String(value).replace(/\s+/g, " ").trim(), + shouldInspectLegacyGatewayContainer: mocks.shouldInspectLegacyGatewayContainer, +})); + +import { runGlobalDoctor } from "./sandbox/doctor"; + +describe("global doctor action", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(fs, "existsSync").mockReturnValue(true); + mocks.resolveOpenshell.mockReturnValue("/usr/bin/openshell"); + mocks.inspectHost.mockReturnValue({ + group: "Host", + label: "Runtime provider", + status: "ok", + detail: "available", + }); + mocks.listSandboxes.mockReturnValue({ sandboxes: [], defaultSandbox: null }); + mocks.getNamedGatewayLifecycleState.mockReturnValue({ + state: "healthy_named", + status: "Status: Connected", + gatewayInfo: "Gateway: nemoclaw", + activeGateway: "nemoclaw", + }); + mocks.shouldInspectLegacyGatewayContainer.mockReturnValue(false); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("checks host and gateway health with zero sandboxes without recovery (#10212)", async () => { + const report = await runGlobalDoctor({ quiet: true }); + + expect(report).toMatchObject({ + schemaVersion: 1, + scope: "global", + status: "ok", + failed: 0, + warnings: 0, + }); + expect(report).not.toHaveProperty("sandbox"); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ group: "Host", label: "CLI build", status: "ok" }), + expect.objectContaining({ group: "Host", label: "Runtime provider", status: "ok" }), + expect.objectContaining({ group: "Host", label: "OpenShell CLI", status: "ok" }), + expect.objectContaining({ group: "Host", label: "Sandbox registry", status: "ok" }), + expect.objectContaining({ group: "Gateway", label: "OpenShell status", status: "ok" }), + ]), + ); + expect(report.checks.some((check) => check.group === "Sandbox")).toBe(false); + expect(report.checks.some((check) => check.group === "Inference")).toBe(false); + expect(report.checks.some((check) => check.group === "Messaging")).toBe(false); + expect(report.checks.some((check) => check.group === "Local services")).toBe(false); + expect(mocks.getNamedGatewayLifecycleState).toHaveBeenCalledWith("nemoclaw", { + ignoreProbeErrors: true, + }); + expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); + }); + + it("reports a registry read failure without exposing its error text (#10212)", async () => { + mocks.listSandboxes.mockImplementationOnce(() => { + throw new Error("Authorization: Bearer sk-secret-value in /private/registry.json"); + }); + + const report = await runGlobalDoctor({ quiet: true }); + const registryCheck = report.checks.find((check) => check.label === "Sandbox registry"); + + expect(report.status).toBe("fail"); + expect(registryCheck).toMatchObject({ + status: "fail", + detail: "could not read the host sandbox registry", + }); + expect(JSON.stringify(report)).not.toContain("sk-secret-value"); + expect(JSON.stringify(report)).not.toContain("/private/registry.json"); + }); + + it("reports skipped gateway health when the OpenShell CLI is missing (#10212)", async () => { + mocks.resolveOpenshell.mockReturnValueOnce(null); + + const report = await runGlobalDoctor({ quiet: true }); + + expect(report.status).toBe("fail"); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ label: "OpenShell CLI", status: "fail" }), + expect.objectContaining({ + group: "Gateway", + label: "OpenShell status", + status: "fail", + detail: "skipped because the OpenShell CLI is not installed", + }), + ]), + ); + expect(mocks.getNamedGatewayLifecycleState).not.toHaveBeenCalled(); + expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); + }); + + it("renders actionable text without naming a sandbox (#10212)", async () => { + mocks.getNamedGatewayLifecycleState.mockReturnValueOnce({ + state: "missing_named", + status: "Status: Disconnected", + gatewayInfo: "", + activeGateway: null, + }); + const lines: string[] = []; + vi.spyOn(console, "log").mockImplementation((line = "") => lines.push(String(line))); + + const report = await runGlobalDoctor(); + const output = lines.join("\n"); + + expect(report.status).toBe("fail"); + expect(output).toContain("NemoClaw doctor"); + expect(output).not.toContain("NemoClaw doctor:"); + expect(output).toContain( + "Start the gateway again with `nemoclaw onboard`. Then retry this command.", + ); + }); +}); diff --git a/src/lib/actions/sandbox/doctor-report.test.ts b/src/lib/actions/sandbox/doctor-report.test.ts index 3cc0cae8503..2ca0c4b1b17 100644 --- a/src/lib/actions/sandbox/doctor-report.test.ts +++ b/src/lib/actions/sandbox/doctor-report.test.ts @@ -2,7 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 import { afterEach, describe, expect, it, vi } from "vitest"; -import { buildDoctorReport, type DoctorCheck, renderDoctorReport } from "./doctor-report"; +import { + buildDoctorReport, + buildGlobalDoctorReport, + type DoctorCheck, + renderDoctorReport, +} from "./doctor-report"; function check(status: DoctorCheck["status"], group = "Host"): DoctorCheck { return { group, label: `${status} check`, status, detail: `${status} detail` }; @@ -68,4 +73,24 @@ describe("doctor reports", () => { expect(output).toContain("hint: inspect the custom probe"); expect(output).toContain("healthy with 1 warning(s)"); }); + + it("renders a global report without a sandbox field or heading suffix (#10212)", () => { + const lines: string[] = []; + vi.spyOn(console, "log").mockImplementation((line = "") => lines.push(String(line))); + const report = buildGlobalDoctorReport([check("ok")]); + + expect(report).toEqual({ + schemaVersion: 1, + scope: "global", + status: "ok", + failed: 0, + warnings: 0, + checks: [check("ok")], + }); + expect(report).not.toHaveProperty("sandbox"); + expect(renderDoctorReport(report, false)).toBe(0); + const output = lines.join("\n"); + expect(output).toContain("NemoClaw doctor"); + expect(output).not.toContain("NemoClaw doctor:"); + }); }); diff --git a/src/lib/actions/sandbox/doctor-report.ts b/src/lib/actions/sandbox/doctor-report.ts index a5254250786..5109f1699fe 100644 --- a/src/lib/actions/sandbox/doctor-report.ts +++ b/src/lib/actions/sandbox/doctor-report.ts @@ -25,6 +25,17 @@ export type DoctorReport = { checks: DoctorCheck[]; }; +export type GlobalDoctorReport = { + schemaVersion: 1; + scope: "global"; + status: DoctorReportStatus; + failed: number; + warnings: number; + checks: DoctorCheck[]; +}; + +type RenderableDoctorReport = DoctorReport | GlobalDoctorReport; + function summarizeChecks(checks: DoctorCheck[]): { status: DoctorReportStatus; failed: number; @@ -49,6 +60,18 @@ export function buildDoctorReport(sandboxName: string, checks: DoctorCheck[]): D }; } +export function buildGlobalDoctorReport(checks: DoctorCheck[]): GlobalDoctorReport { + const summary = summarizeChecks(checks); + return { + schemaVersion: 1, + scope: "global", + status: summary.status, + failed: summary.failed, + warnings: summary.warned, + checks, + }; +} + function statusLabel(status: DoctorStatus): string { switch (status) { case "ok": @@ -62,7 +85,7 @@ function statusLabel(status: DoctorStatus): string { } } -function orderedGroups(report: DoctorReport): string[] { +function orderedGroups(report: RenderableDoctorReport): string[] { const preferred = ["Host", "Gateway", "Sandbox", "Inference", "Messaging", "Local services"]; const remaining = report.checks .map((check) => check.group) @@ -70,7 +93,7 @@ function orderedGroups(report: DoctorReport): string[] { return [...preferred, ...remaining]; } -function renderCheckGroups(report: DoctorReport): void { +function renderCheckGroups(report: RenderableDoctorReport): void { for (const group of orderedGroups(report)) { const checks = report.checks.filter((check) => check.group === group); if (checks.length === 0) continue; @@ -83,7 +106,7 @@ function renderCheckGroups(report: DoctorReport): void { } } -function renderSummary(report: DoctorReport): void { +function renderSummary(report: RenderableDoctorReport): void { if (report.status === "ok") { console.log(` Summary: ${G}healthy${R}`); return; @@ -97,7 +120,7 @@ function renderSummary(report: DoctorReport): void { ); } -export function renderDoctorReport(report: DoctorReport, asJson: boolean): number { +export function renderDoctorReport(report: RenderableDoctorReport, asJson: boolean): number { if (asJson) { // Parity with `sandbox status --json` (#4310): this console.log egress // bypasses the oclif logJson redaction boundary (#3657), so route the @@ -109,7 +132,8 @@ export function renderDoctorReport(report: DoctorReport, asJson: boolean): numbe } console.log(""); - console.log(` ${B}${CLI_DISPLAY_NAME} doctor:${R} ${report.sandbox}`); + const target = "sandbox" in report ? `: ${report.sandbox}` : ""; + console.log(` ${B}${CLI_DISPLAY_NAME} doctor${target}${R}`); renderCheckGroups(report); console.log(""); renderSummary(report); diff --git a/src/lib/actions/sandbox/doctor-system-checks.ts b/src/lib/actions/sandbox/doctor-system-checks.ts index 3ad56d794d7..fa8eeb59d90 100644 --- a/src/lib/actions/sandbox/doctor-system-checks.ts +++ b/src/lib/actions/sandbox/doctor-system-checks.ts @@ -25,6 +25,10 @@ import type { DoctorCheck } from "./doctor-report"; export const withSandboxDoctorLifecycleLock = withMcpLifecycleLock; +export function gatewayDoctorStartHint(gatewayName: string): string { + return `${gatewayStartGuidance(gatewayName)} Then retry this command.`; +} + export function inspectSandboxDoctorPortableAuthority( sandboxName: string, readRegistry: (sandboxName: string) => SandboxEntry | null, diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index 74b8ac78494..f266294c77b 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -55,13 +55,16 @@ import { import { collectMessagingDoctorChecks } from "./doctor-messaging"; import { buildDoctorReport, + buildGlobalDoctorReport, type DoctorCheck, type DoctorReport, + type GlobalDoctorReport, renderDoctorReport, } from "./doctor-report"; import { cloudflaredDoctorCheck, dockerInspectGateway, + gatewayDoctorStartHint, inspectSandboxDoctorPortableAuthority, ollamaDoctorCheck, oneLine, @@ -81,11 +84,23 @@ type DoctorIntent = { wantsFix: boolean; }; -type GatewayProbe = { +type DoctorHostProbe = { + checks: DoctorCheck[]; + openshellBin: string | null; +}; + +type DoctorGatewayProbe = { checks: DoctorCheck[]; connected: boolean; }; +type DoctorGatewayProbeOptions = { + gatewayPort: number; + ignoreProbeErrors?: boolean; + recoverGateway: boolean; + unavailableHint?: string; +}; + type SandboxProbe = { checks: DoctorCheck[]; reachable: boolean; @@ -181,15 +196,11 @@ function runtimeHostCheck(sb: SandboxEntry | null | undefined): DoctorCheck { } } -function collectHostChecks(sb: SandboxEntry | null | undefined): { - checks: DoctorCheck[]; - openshellBin: ReturnType; -} { - const cli = cliBuildCheck(); +function collectDoctorHostChecks(sb: SandboxEntry | null | undefined): DoctorHostProbe { const openshellBin = resolveOpenshell(); return { checks: [ - cli, + cliBuildCheck(), runtimeHostCheck(sb), { group: "Host", @@ -203,19 +214,49 @@ function collectHostChecks(sb: SandboxEntry | null | undefined): { }; } -async function collectGatewayChecks( +async function gatewayLifecycle(gatewayName: string, options: DoctorGatewayProbeOptions) { + if (!options.recoverGateway) { + return options.ignoreProbeErrors === undefined + ? getNamedGatewayLifecycleState(gatewayName) + : getNamedGatewayLifecycleState(gatewayName, { + ignoreProbeErrors: options.ignoreProbeErrors, + }); + } + const recovery = await recoverNamedGatewayRuntime({ gatewayName }); + return recovery.after || recovery.before; +} + +async function probeOpenShellGateway( + gatewayName: string, + options: DoctorGatewayProbeOptions, +): Promise<{ check: DoctorCheck; connected: boolean }> { + const lifecycle = await gatewayLifecycle(gatewayName, options); + const cleanStatus = oneLine(stripOpenShellCliAnsi(lifecycle?.status || "")); + const connected = lifecycle?.state === "healthy_named"; + return { + connected, + check: { + group: "Gateway", + label: "OpenShell status", + status: connected ? "ok" : "fail", + detail: connected + ? `connected to ${gatewayName}` + : oneLine(cleanStatus || lifecycle?.gatewayInfo || `not connected to ${gatewayName}`), + hint: connected + ? undefined + : lifecycle?.state === "connected_other" || !options.unavailableHint + ? `run \`openshell gateway select ${gatewayName}\` and retry` + : options.unavailableHint, + }, + }; +} + +async function collectDoctorGatewayChecks( gatewayName: string, sb: SandboxEntry | null | undefined, - openshellBin: ReturnType, - recoverGateway: boolean, -): Promise { - // #10223: the fail-only branch at the call site emits this label when the - // registered gateway binding cannot be resolved. This branch runs both - // when it resolved from a real sandbox entry and when the caller falls - // back to the ambient default gateway for an unregistered sandbox name - // (resolveDoctorGatewayName). Only the former is an actual registered - // binding, so gate the ok check on a real sandbox entry, or an - // unregistered sandbox name would misreport one that does not exist. + openshellBin: string | null, + options: DoctorGatewayProbeOptions, +): Promise { const checks: DoctorCheck[] = sb ? [ { @@ -227,7 +268,7 @@ async function collectGatewayChecks( ] : []; const gateway = openshellBin - ? await probeOpenShellGateway(gatewayName, recoverGateway) + ? await probeOpenShellGateway(gatewayName, options) : { check: null, connected: false }; if (gateway.check) checks.push(gateway.check); if (shouldInspectLegacyGatewayContainer(sb)) { @@ -238,43 +279,13 @@ async function collectGatewayChecks( namedGatewayConnected: gateway.connected, gatewayName, }, - sb?.gatewayPort ?? GATEWAY_PORT, + sb?.gatewayPort ?? options.gatewayPort, ), ); } return { checks, connected: gateway.connected }; } -async function gatewayLifecycle(gatewayName: string, recoverGateway: boolean) { - if (!recoverGateway) return getNamedGatewayLifecycleState(gatewayName); - const recovery = await recoverNamedGatewayRuntime({ gatewayName }); - return recovery.after || recovery.before; -} - -async function probeOpenShellGateway( - gatewayName: string, - recoverGateway: boolean, -): Promise<{ - check: DoctorCheck; - connected: boolean; -}> { - const lifecycle = await gatewayLifecycle(gatewayName, recoverGateway); - const cleanStatus = stripOpenShellCliAnsi(lifecycle?.status || ""); - const connected = lifecycle?.state === "healthy_named"; - return { - connected, - check: { - group: "Gateway", - label: "OpenShell status", - status: connected ? "ok" : "fail", - detail: connected - ? `connected to ${gatewayName}` - : oneLine(cleanStatus || lifecycle?.gatewayInfo || `not connected to ${gatewayName}`), - hint: connected ? undefined : `run \`openshell gateway select ${gatewayName}\` and retry`, - }, - }; -} - function liveSandboxDetail( sandboxName: string, present: boolean, @@ -362,7 +373,7 @@ async function liveSandboxCheck(sandboxName: string, gatewayName: string): Promi async function collectSandboxReadinessChecks( sandboxName: string, gatewayName: string | null, - openshellBin: ReturnType, + openshellBin: string | null, openshellConnected: boolean, ): Promise { if (gatewayName && openshellBin && openshellConnected) { @@ -385,7 +396,7 @@ async function collectSandboxReadinessChecks( function resolveInferenceRoute( sb: SandboxEntry | null | undefined, - openshellBin: ReturnType, + openshellBin: string | null, openshellConnected: boolean, gatewayName: string | null, ): DoctorInferenceRoute { @@ -502,9 +513,12 @@ async function collectDoctorChecks( gatewayName: string | null, intent: DoctorIntent, ): Promise { - const host = collectHostChecks(sb); - const gateway: GatewayProbe = gatewayName - ? await collectGatewayChecks(gatewayName, sb, host.openshellBin, !intent.asJson) + const host = collectDoctorHostChecks(sb); + const gateway: DoctorGatewayProbe = gatewayName + ? await collectDoctorGatewayChecks(gatewayName, sb, host.openshellBin, { + gatewayPort: sb?.gatewayPort ?? GATEWAY_PORT, + recoverGateway: !intent.asJson, + }) : { connected: false, checks: [ @@ -549,6 +563,60 @@ function resolveDoctorGatewayName(sb: SandboxEntry | null | undefined): string | } } +function registryReadabilityCheck(): DoctorCheck { + try { + const count = registry.listSandboxes().sandboxes.length; + return { + group: "Host", + label: "Sandbox registry", + status: "ok", + detail: `readable (${count} registered sandbox${count === 1 ? "" : "es"})`, + }; + } catch { + return { + group: "Host", + label: "Sandbox registry", + status: "fail", + detail: "could not read the host sandbox registry", + hint: "check the registry file permissions and JSON, then retry", + }; + } +} + +function unavailableGatewayCheck(): DoctorCheck { + return { + group: "Gateway", + label: "OpenShell status", + status: "fail", + detail: "skipped because the OpenShell CLI is not installed", + hint: "install OpenShell, then retry", + }; +} + +export async function runGlobalDoctor( + options: { quiet?: boolean } = {}, +): Promise { + const host = collectDoctorHostChecks(null); + const gatewayName = resolveGatewayName(GATEWAY_PORT); + const gatewayChecks = host.openshellBin + ? ( + await collectDoctorGatewayChecks(gatewayName, null, host.openshellBin, { + gatewayPort: GATEWAY_PORT, + ignoreProbeErrors: true, + recoverGateway: false, + unavailableHint: gatewayDoctorStartHint(gatewayName), + }) + ).checks + : [unavailableGatewayCheck()]; + const report = buildGlobalDoctorReport([ + ...host.checks, + registryReadabilityCheck(), + ...gatewayChecks, + ]); + if (!options.quiet) renderDoctorReport(report, false); + return report; +} + export async function runSandboxDoctor( sandboxName: string, args: string[] = [], diff --git a/src/lib/cli/doctor-command-support.ts b/src/lib/cli/doctor-command-support.ts new file mode 100644 index 00000000000..4a745ad3569 --- /dev/null +++ b/src/lib/cli/doctor-command-support.ts @@ -0,0 +1,5 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export { withStdoutRedirectedToStderr } from "./stdout-guard"; +export { redactForLog } from "../security/redact"; diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index f278fa53d2e..73674363e76 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -436,6 +436,25 @@ async function dispatchNormalizedArgv(normalized: NormalizedArgv, argv: string[] } async function dispatchGlobalArgv(normalized: NormalizedGlobalArgv): Promise { + if ( + normalized.command === "doctor" && + normalized.args[0] && + isKnownSandboxAction(normalized.args[0]) + ) { + const [action, ...actionArgs] = normalized.args; + await dispatchSandboxArgv( + { + kind: "sandbox", + sandboxName: "doctor", + action, + actionArgs, + connectHelpRequested: + action === "connect" && actionArgs.some((arg) => arg === "--help" || arg === "-h"), + }, + [normalized.command, ...normalized.args], + ); + return; + } if (normalized.command === "status") { const sandboxName = findGlobalStatusSandboxArgument(normalized.args); if (sandboxName) printGlobalStatusScopeHint(sandboxName, normalized.args); diff --git a/src/lib/cli/public-display-defaults.ts b/src/lib/cli/public-display-defaults.ts index 36ff826babf..bbb3f2f6278 100644 --- a/src/lib/cli/public-display-defaults.ts +++ b/src/lib/cli/public-display-defaults.ts @@ -62,6 +62,13 @@ const PUBLIC_DISPLAY_LAYOUT: Record = { flags: "[--quick] [--output FILE|-o FILE] [--sandbox NAME]", }, ], + doctor: [ + { + group: "Troubleshooting", + order: 36, + flags: "[--json]", + }, + ], gc: [ { group: "Cleanup", diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index b15104ab3f0..9470b855c44 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -647,17 +647,48 @@ describe("CLI dispatch", () => { ); }); + it("dispatches the global doctor without a sandbox name (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById, stderr }) => { + await dispatchCli(["doctor"]); + + expect(runOclifCommandById).toHaveBeenCalledWith( + "doctor", + [], + expect.objectContaining({ rootDir: process.cwd() }), + ); + expect(recoverRegistryEntries).not.toHaveBeenCalled(); + expect(stderr).toEqual([]); + }, + ); + }); + + it("dispatches global doctor flags without reading sandbox state (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById }) => { + await dispatchCli(["doctor", "--json"]); + + expect(runOclifCommandById).toHaveBeenCalledWith( + "doctor", + ["--json"], + expect.objectContaining({ rootDir: process.cwd() }), + ); + expect(recoverRegistryEntries).not.toHaveBeenCalled(); + }, + ); + }); + it("reports the sandbox-first grammar without recovering a bare action (#10212)", async () => { await withDirectPublicDispatch( async ({ dispatchCli, exitSpy, recoverRegistryEntries, stderr }) => { - await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); + await expect(dispatchCli(["destroy"])).rejects.toThrow("process.exit:1"); const output = stderr.join("\n"); expect(recoverRegistryEntries).not.toHaveBeenCalled(); - expect(output).toContain("'doctor' is a sandbox command. It needs a sandbox name."); - expect(output).toContain("Run: nemoclaw doctor"); + expect(output).toContain("'destroy' is a sandbox command. It needs a sandbox name."); + expect(output).toContain("Run: nemoclaw destroy"); expect(output).toContain("Run 'nemoclaw onboard' to create one."); - expect(output).not.toContain("Sandbox 'doctor' does not exist."); + expect(output).not.toContain("Sandbox 'destroy' does not exist."); expect(exitSpy).toHaveBeenCalledWith(1); }, ); @@ -666,9 +697,9 @@ describe("CLI dispatch", () => { it.each([ { input: "flag argument", - argv: ["doctor", "--json"], - route: "doctor", - forbidden: /--json/, + argv: ["logs", "--follow"], + route: "logs", + forbidden: /--follow/, }, { input: "credential-bearing argument", @@ -740,7 +771,7 @@ describe("CLI dispatch", () => { it("lists registered sandboxes in the sandbox-first grammar hint (#10212)", async () => { await withDirectPublicDispatch( async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); + await expect(dispatchCli(["destroy"])).rejects.toThrow("process.exit:1"); const output = stderr.join("\n"); expect(output).toContain("Registered sandboxes: alpha, beta"); @@ -753,7 +784,7 @@ describe("CLI dispatch", () => { it("reports pending setup instead of new onboarding for a bare sandbox action (#10212)", async () => { await withDirectPublicDispatch( async ({ dispatchCli, stderr }) => { - await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1"); + await expect(dispatchCli(["destroy"])).rejects.toThrow("process.exit:1"); const output = stderr.join("\n"); expect(output).toContain("Sandbox setup is still pending: alpha"); diff --git a/test/cli/root-help.test.ts b/test/cli/root-help.test.ts index ee8890cd775..4aa96823b72 100644 --- a/test/cli/root-help.test.ts +++ b/test/cli/root-help.test.ts @@ -32,6 +32,8 @@ describe("root help", () => { expect(output).toContain("sandbox commands start with a sandbox name"); expect(output).toContain("nemoclaw status"); expect(output).toContain("nemoclaw status"); + expect(output).toContain("nemoclaw doctor"); + expect(output).toContain("nemoclaw doctor"); }); it("describes onboard agent selection and the global agent runtime list", () => { diff --git a/test/package-contract/cli/command-registry.test.ts b/test/package-contract/cli/command-registry.test.ts index 9000ad55395..7052bd6d84d 100644 --- a/test/package-contract/cli/command-registry.test.ts +++ b/test/package-contract/cli/command-registry.test.ts @@ -44,6 +44,7 @@ describe("command-registry", () => { expect(usages).toContain("nemoclaw tunnel stop"); expect(usages).toContain("nemoclaw tunnel status"); expect(usages).toContain("nemoclaw status"); + expect(usages).toContain("nemoclaw doctor"); }); it.each(globalCommands())("$usage has global scope", (cmd) => { @@ -169,7 +170,7 @@ describe("command-registry", () => { }); describe("globalCommandTokens()", () => { - it("returns the exact set of 29 tokens matching the global dispatch commands", () => { + it("returns the exact set of 30 tokens matching the global dispatch commands", () => { const tokens = globalCommandTokens(); const expected = new Set([ "agents", @@ -187,6 +188,7 @@ describe("command-registry", () => { "stop", "tunnel", "status", + "doctor", "debug", "uninstall", "credentials", From b764829d90be65c949e882f7d50c93b8369c6b0e Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 3 Sep 2026 17:04:22 -0700 Subject: [PATCH 02/11] fix(cli): keep global doctor read-only and redacted Signed-off-by: Prekshi Vyas --- src/lib/actions/sandbox/doctor-report.test.ts | 19 ++++++++++++++++++ src/lib/actions/sandbox/doctor-report.ts | 9 +++++---- src/lib/cli/public-dispatch.ts | 4 ++++ test/cli/dispatch-basics.test.ts | 20 ++++++++++++++++--- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/lib/actions/sandbox/doctor-report.test.ts b/src/lib/actions/sandbox/doctor-report.test.ts index 2ca0c4b1b17..e06f0a9ff12 100644 --- a/src/lib/actions/sandbox/doctor-report.test.ts +++ b/src/lib/actions/sandbox/doctor-report.test.ts @@ -60,6 +60,25 @@ describe("doctor reports", () => { expect(JSON.stringify(printed)).not.toContain("sk-abc123DEF456ghi789"); }); + it("redacts token-shaped values from the text report", () => { + const lines: string[] = []; + vi.spyOn(console, "log").mockImplementation((line = "") => lines.push(String(line))); + const report = buildGlobalDoctorReport([ + { + group: "Gateway", + label: "OpenShell status", + status: "fail", + detail: "connect failed: Authorization: Bearer sk-abc123DEF456ghi789 (HTTP 401)", + }, + ]); + + expect(renderDoctorReport(report, false)).toBe(1); + const output = lines.join("\n"); + expect(output).toContain("Authorization: Bearer "); + expect(output).toContain("[fail]"); + expect(output).not.toContain("sk-abc123DEF456ghi789"); + }); + it("renders preferred groups first, preserves extra-group order, and includes hints", () => { const lines: string[] = []; vi.spyOn(console, "log").mockImplementation((line = "") => lines.push(String(line))); diff --git a/src/lib/actions/sandbox/doctor-report.ts b/src/lib/actions/sandbox/doctor-report.ts index 5109f1699fe..bab6ca327bb 100644 --- a/src/lib/actions/sandbox/doctor-report.ts +++ b/src/lib/actions/sandbox/doctor-report.ts @@ -121,22 +121,23 @@ function renderSummary(report: RenderableDoctorReport): void { } export function renderDoctorReport(report: RenderableDoctorReport, asJson: boolean): number { + const displayReport = redactForLog(report) as RenderableDoctorReport; if (asJson) { // Parity with `sandbox status --json` (#4310): this console.log egress // bypasses the oclif logJson redaction boundary (#3657), so route the // machine-readable report through the centralized redactForLog source of // truth before check details (subprocess stderr, probe errors) reach // stdout. - console.log(JSON.stringify(redactForLog(report), null, 2)); + console.log(JSON.stringify(displayReport, null, 2)); return report.failed > 0 ? 1 : 0; } console.log(""); - const target = "sandbox" in report ? `: ${report.sandbox}` : ""; + const target = "sandbox" in displayReport ? `: ${displayReport.sandbox}` : ""; console.log(` ${B}${CLI_DISPLAY_NAME} doctor${target}${R}`); - renderCheckGroups(report); + renderCheckGroups(displayReport); console.log(""); - renderSummary(report); + renderSummary(displayReport); console.log(""); return report.failed > 0 ? 1 : 0; } diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index 73674363e76..71d3ec5c64c 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -128,6 +128,10 @@ function isMigrationRecoveryInvocation(argv: readonly string[]): boolean { if (argv[0] === "sandbox") { return MIGRATION_RECOVERY_SANDBOX_ACTIONS.has(argv[1] ?? ""); } + if (argv[0] === "doctor") { + const action = argv[1]; + return !action || !isKnownSandboxAction(action); + } return ( argv.length > 1 && !GLOBAL_COMMANDS.has(argv[0] ?? "") && diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index 9470b855c44..e8800d3fbe7 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -649,7 +649,13 @@ describe("CLI dispatch", () => { it("dispatches the global doctor without a sandbox name (#10212)", async () => { await withDirectPublicDispatch( - async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById, stderr }) => { + async ({ + dispatchCli, + migrateLegacyPortState, + recoverRegistryEntries, + runOclifCommandById, + stderr, + }) => { await dispatchCli(["doctor"]); expect(runOclifCommandById).toHaveBeenCalledWith( @@ -657,6 +663,7 @@ describe("CLI dispatch", () => { [], expect.objectContaining({ rootDir: process.cwd() }), ); + expect(migrateLegacyPortState).not.toHaveBeenCalled(); expect(recoverRegistryEntries).not.toHaveBeenCalled(); expect(stderr).toEqual([]); }, @@ -665,7 +672,12 @@ describe("CLI dispatch", () => { it("dispatches global doctor flags without reading sandbox state (#10212)", async () => { await withDirectPublicDispatch( - async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById }) => { + async ({ + dispatchCli, + migrateLegacyPortState, + recoverRegistryEntries, + runOclifCommandById, + }) => { await dispatchCli(["doctor", "--json"]); expect(runOclifCommandById).toHaveBeenCalledWith( @@ -673,6 +685,7 @@ describe("CLI dispatch", () => { ["--json"], expect.objectContaining({ rootDir: process.cwd() }), ); + expect(migrateLegacyPortState).not.toHaveBeenCalled(); expect(recoverRegistryEntries).not.toHaveBeenCalled(); }, ); @@ -823,9 +836,10 @@ describe("CLI dispatch", () => { it("keeps the name-first grammar for a sandbox literally named doctor (#10212)", async () => { await withDirectPublicDispatch( - async ({ dispatchCli, runOclifCommandById, stderr }) => { + async ({ dispatchCli, migrateLegacyPortState, runOclifCommandById, stderr }) => { await dispatchCli(["doctor", "status"]); + expect(migrateLegacyPortState).toHaveBeenCalledTimes(1); expect(runOclifCommandById).toHaveBeenCalledWith( "sandbox:status", ["doctor"], From b044994f369c45b24922f248cf4fd1540cf02588 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 3 Sep 2026 17:37:14 -0700 Subject: [PATCH 03/11] fix(cli): centralize global doctor routing Signed-off-by: Prekshi Vyas --- src/lib/cli/argv-normalizer.test.ts | 53 ++++++++++++----------- src/lib/cli/argv-normalizer.ts | 12 +++++- src/lib/cli/public-dispatch.ts | 46 ++++++-------------- test/cli/dispatch-basics.test.ts | 67 ++++++++++++++++++++++++++++- 4 files changed, 117 insertions(+), 61 deletions(-) diff --git a/src/lib/cli/argv-normalizer.test.ts b/src/lib/cli/argv-normalizer.test.ts index 850bda285a1..89ae7753481 100644 --- a/src/lib/cli/argv-normalizer.test.ts +++ b/src/lib/cli/argv-normalizer.test.ts @@ -5,42 +5,42 @@ import { describe, expect, it } from "vitest"; import { normalizeArgv, suggestCommand } from "./argv-normalizer"; -const globalCommands = new Set(["list", "status", "onboard", "--version"]); +const globalCommands = new Set(["list", "status", "onboard", "doctor", "--version"]); const isConnectFlag = (arg: string | undefined) => arg === "--probe-only" || arg === "--help"; +const normalizerOptions = { + globalCommands, + isSandboxAction: (arg: string | undefined) => ["status", "policy-add"].includes(arg ?? ""), + isSandboxConnectFlag: isConnectFlag, +}; describe("normalizeArgv", () => { it("normalizes root help aliases", () => { - expect(normalizeArgv([], { globalCommands, isSandboxConnectFlag: isConnectFlag })).toEqual({ + expect(normalizeArgv([], normalizerOptions)).toEqual({ kind: "rootHelp", }); - expect( - normalizeArgv(["--help"], { globalCommands, isSandboxConnectFlag: isConnectFlag }), - ).toEqual({ + expect(normalizeArgv(["--help"], normalizerOptions)).toEqual({ kind: "rootHelp", }); }); it("normalizes internal dump commands", () => { expect( - normalizeArgv(["--dump-commands"], { globalCommands, isSandboxConnectFlag: isConnectFlag }), + normalizeArgv(["--dump-commands"], normalizerOptions), ).toEqual({ kind: "dumpCommands" }); expect( - normalizeArgv(["--dump-command-flags"], { - globalCommands, - isSandboxConnectFlag: isConnectFlag, - }), + normalizeArgv(["--dump-command-flags"], normalizerOptions), ).toEqual({ kind: "dumpCommandFlags" }); }); it("normalizes global commands", () => { expect( - normalizeArgv(["list", "--json"], { globalCommands, isSandboxConnectFlag: isConnectFlag }), + normalizeArgv(["list", "--json"], normalizerOptions), ).toEqual({ kind: "global", command: "list", args: ["--json"] }); }); it("normalizes explicit sandbox actions", () => { expect( - normalizeArgv(["alpha", "status"], { globalCommands, isSandboxConnectFlag: isConnectFlag }), + normalizeArgv(["alpha", "status"], normalizerOptions), ).toEqual({ kind: "sandbox", sandboxName: "alpha", @@ -52,7 +52,7 @@ describe("normalizeArgv", () => { it("normalizes bare and implicit connect invocations", () => { expect( - normalizeArgv(["alpha"], { globalCommands, isSandboxConnectFlag: isConnectFlag }), + normalizeArgv(["alpha"], normalizerOptions), ).toEqual({ kind: "sandbox", sandboxName: "alpha", @@ -61,10 +61,7 @@ describe("normalizeArgv", () => { connectHelpRequested: false, }); expect( - normalizeArgv(["alpha", "--probe-only"], { - globalCommands, - isSandboxConnectFlag: isConnectFlag, - }), + normalizeArgv(["alpha", "--probe-only"], normalizerOptions), ).toEqual({ kind: "sandbox", sandboxName: "alpha", @@ -76,10 +73,7 @@ describe("normalizeArgv", () => { it("tracks connect help requests", () => { expect( - normalizeArgv(["alpha", "connect", "--help"], { - globalCommands, - isSandboxConnectFlag: isConnectFlag, - }), + normalizeArgv(["alpha", "connect", "--help"], normalizerOptions), ).toMatchObject({ kind: "sandbox", sandboxName: "alpha", @@ -88,10 +82,7 @@ describe("normalizeArgv", () => { connectHelpRequested: true, }); expect( - normalizeArgv(["alpha", "--help"], { - globalCommands, - isSandboxConnectFlag: isConnectFlag, - }), + normalizeArgv(["alpha", "--help"], normalizerOptions), ).toMatchObject({ kind: "sandbox", sandboxName: "alpha", @@ -100,6 +91,18 @@ describe("normalizeArgv", () => { connectHelpRequested: true, }); }); + + it.each([ + { argv: ["doctor"], kind: "global", action: undefined }, + { argv: ["doctor", "--json"], kind: "global", action: undefined }, + { argv: ["doctor", "status"], kind: "sandbox", action: "status" }, + { argv: ["doctor", "policy-add"], kind: "sandbox", action: "policy-add" }, + ])("classifies $argv from one doctor scope rule", ({ argv, kind, action }) => { + expect(normalizeArgv(argv, normalizerOptions)).toMatchObject({ + kind, + ...(action ? { sandboxName: "doctor", action } : {}), + }); + }); }); describe("suggestCommand", () => { diff --git a/src/lib/cli/argv-normalizer.ts b/src/lib/cli/argv-normalizer.ts index cca96b9cedc..2c83b741542 100644 --- a/src/lib/cli/argv-normalizer.ts +++ b/src/lib/cli/argv-normalizer.ts @@ -22,9 +22,19 @@ export type NormalizedArgv = export type NormalizeArgvOptions = { globalCommands: ReadonlySet; + isSandboxAction: (arg: string | undefined) => boolean; isSandboxConnectFlag: (arg: string | undefined) => boolean; }; +export function isGlobalCommandInvocation( + argv: readonly string[], + opts: NormalizeArgvOptions, +): boolean { + const [command, firstArg] = argv; + if (!command || !opts.globalCommands.has(command)) return false; + return command !== "doctor" || !firstArg || !opts.isSandboxAction(firstArg); +} + export function normalizeArgv(argv: readonly string[], opts: NormalizeArgvOptions): NormalizedArgv { const [cmd, ...args] = argv; @@ -40,7 +50,7 @@ export function normalizeArgv(argv: readonly string[], opts: NormalizeArgvOption return { kind: "dumpCommandFlags" }; } - if (opts.globalCommands.has(cmd)) { + if (isGlobalCommandInvocation(argv, opts)) { return { kind: "global", command: cmd, args }; } diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index 71d3ec5c64c..f29c36b09f6 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -24,9 +24,11 @@ const { import { migrateLegacyPortState } from "../state/legacy-port-migration"; import { + isGlobalCommandInvocation, type NormalizedArgv, type NormalizedGlobalArgv, type NormalizedSandboxArgv, + type NormalizeArgvOptions, normalizeArgv, suggestCommand, } from "./argv-normalizer"; @@ -44,6 +46,11 @@ import { const GLOBAL_COMMANDS = globalCommandTokens(); const NATIVE_OCLIF_NAMESPACES = new Set(["internal", "sandbox"]); const MIGRATION_RECOVERY_SANDBOX_ACTIONS = new Set(["doctor", "recover"]); +const PUBLIC_ARGV_OPTIONS: NormalizeArgvOptions = { + globalCommands: GLOBAL_COMMANDS, + isSandboxAction: isKnownSandboxAction, + isSandboxConnectFlag: isPublicSandboxConnectFlag, +}; type RegistryModule = typeof import("../state/registry"); type RegistryRecoveryModule = typeof import("../registry-recovery-action"); @@ -128,15 +135,8 @@ function isMigrationRecoveryInvocation(argv: readonly string[]): boolean { if (argv[0] === "sandbox") { return MIGRATION_RECOVERY_SANDBOX_ACTIONS.has(argv[1] ?? ""); } - if (argv[0] === "doctor") { - const action = argv[1]; - return !action || !isKnownSandboxAction(action); - } - return ( - argv.length > 1 && - !GLOBAL_COMMANDS.has(argv[0] ?? "") && - MIGRATION_RECOVERY_SANDBOX_ACTIONS.has(argv[1] ?? "") - ); + if (isGlobalCommandInvocation(argv, PUBLIC_ARGV_OPTIONS)) return argv[0] === "doctor"; + return argv.length > 1 && MIGRATION_RECOVERY_SANDBOX_ACTIONS.has(argv[1] ?? ""); } function sandboxRegistrationNames(): { published: string[]; pending: string[] } { @@ -215,8 +215,8 @@ function printOpenShellCommandHint(hint: OpenShellCommandHint): never { process.exit(1); } -function isKnownSandboxAction(action: string): boolean { - return sandboxActionList().includes(action); +function isKnownSandboxAction(action: string | undefined): boolean { + return typeof action === "string" && sandboxActionList().includes(action); } function validSandboxActionsText(): string { @@ -440,25 +440,6 @@ async function dispatchNormalizedArgv(normalized: NormalizedArgv, argv: string[] } async function dispatchGlobalArgv(normalized: NormalizedGlobalArgv): Promise { - if ( - normalized.command === "doctor" && - normalized.args[0] && - isKnownSandboxAction(normalized.args[0]) - ) { - const [action, ...actionArgs] = normalized.args; - await dispatchSandboxArgv( - { - kind: "sandbox", - sandboxName: "doctor", - action, - actionArgs, - connectHelpRequested: - action === "connect" && actionArgs.some((arg) => arg === "--help" || arg === "-h"), - }, - [normalized.command, ...normalized.args], - ); - return; - } if (normalized.command === "status") { const sandboxName = findGlobalStatusSandboxArgument(normalized.args); if (sandboxName) printGlobalStatusScopeHint(sandboxName, normalized.args); @@ -598,10 +579,7 @@ export async function dispatchCli(argv: string[] = process.argv.slice(2)): Promi } await dispatchNormalizedArgv( - normalizeArgv(argv, { - globalCommands: GLOBAL_COMMANDS, - isSandboxConnectFlag: isPublicSandboxConnectFlag, - }), + normalizeArgv(argv, PUBLIC_ARGV_OPTIONS), argv, ); } diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index e8800d3fbe7..a4773608321 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execSync } from "node:child_process"; +import { execSync, spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -292,6 +292,7 @@ describe("CLI dispatch", () => { expect( normalizeArgv(["-h"], { globalCommands: globalCommandTokens(), + isSandboxAction: () => false, isSandboxConnectFlag: () => false, }), ).toEqual({ kind: "rootHelp" }); @@ -670,6 +671,70 @@ describe("CLI dispatch", () => { ); }); + it( + "emits one redacted JSON report through the public global doctor route (#10212)", + testTimeoutOptions(35_000), + () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-global-doctor-json-")); + const localBin = path.join(home, "bin"); + const registryDir = path.join(home, ".nemoclaw"); + const openshellBin = path.join(localBin, "openshell"); + const openshellLog = path.join(home, "openshell-args"); + const registryFile = path.join(registryDir, "sandboxes.json"); + fs.mkdirSync(localBin, { recursive: true }); + fs.mkdirSync(registryDir, { recursive: true }); + const emptyRegistry = JSON.stringify({ sandboxes: {}, defaultSandbox: null }); + fs.writeFileSync(registryFile, emptyRegistry, { mode: 0o600 }); + fs.writeFileSync( + openshellBin, + [ + "#!/bin/sh", + `printf '%s\\n' "$*" >> ${JSON.stringify(openshellLog)}`, + 'if [ "$1" = "status" ]; then', + " printf 'Status: Disconnected\\nGateway: nemoclaw\\nAuthorization: Bearer sk-abc123DEF456ghi789\\n'", + " exit 1", + "fi", + 'if [ "$1" = "gateway" ] && [ "$2" = "info" ]; then', + " printf 'Gateway: nemoclaw\\n'", + " exit 0", + "fi", + "exit 97", + ].join("\n"), + { mode: 0o755 }, + ); + fs.writeFileSync(path.join(localBin, "docker"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + + try { + const result = spawnSync(process.execPath, [CLI, "doctor", "--json"], { + encoding: "utf8", + env: { + ...process.env, + HOME: home, + NEMOCLAW_GATEWAY_RUNTIME: "docker", + NEMOCLAW_OPENSHELL_BIN: openshellBin, + PATH: `${localBin}:${process.env.PATH || ""}`, + }, + stdio: ["ignore", "pipe", "pipe"], + timeout: 30_000, + }); + + expect(result.status).toBe(1); + expect(result.stderr).toBe(""); + const report = JSON.parse(result.stdout) as Record; + expect(report).toMatchObject({ schemaVersion: 1, scope: "global", status: "fail" }); + expect(report).not.toHaveProperty("sandbox"); + expect(result.stdout).not.toContain("sk-abc123DEF456ghi789"); + expect(fs.readFileSync(registryFile, "utf8")).toBe(emptyRegistry); + expect(fs.readFileSync(openshellLog, "utf8").trim().split("\n")).toEqual([ + "status", + "gateway info -g nemoclaw", + ]); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }, + ); + it("dispatches global doctor flags without reading sandbox state (#10212)", async () => { await withDirectPublicDispatch( async ({ From 081763f94f57f3e7cee59a04f8e05a3b3ac41da5 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 3 Sep 2026 17:57:16 -0700 Subject: [PATCH 04/11] fix(cli): report invalid gateway management in doctor --- src/lib/actions/doctor.test.ts | 48 +++++++++++++++++++++++++++++-- src/lib/actions/sandbox/doctor.ts | 41 ++++++++++++++++++++++---- 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/lib/actions/doctor.test.ts b/src/lib/actions/doctor.test.ts index 1230d5033f8..7c809a66cc2 100644 --- a/src/lib/actions/doctor.test.ts +++ b/src/lib/actions/doctor.test.ts @@ -7,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ dockerInspectGateway: vi.fn(), + gatewayDoctorStartHint: vi.fn(), getNamedGatewayLifecycleState: vi.fn(), inspectHost: vi.fn(), listSandboxes: vi.fn(), @@ -50,8 +51,7 @@ vi.mock("./sandbox/doctor-lifecycle-registration", () => ({ vi.mock("./sandbox/doctor-system-checks", () => ({ dockerInspectGateway: mocks.dockerInspectGateway, - gatewayDoctorStartHint: () => - "Start the gateway again with `nemoclaw onboard`. Then retry this command.", + gatewayDoctorStartHint: mocks.gatewayDoctorStartHint, oneLine: (value = "") => String(value).replace(/\s+/g, " ").trim(), shouldInspectLegacyGatewayContainer: mocks.shouldInspectLegacyGatewayContainer, })); @@ -70,6 +70,9 @@ describe("global doctor action", () => { detail: "available", }); mocks.listSandboxes.mockReturnValue({ sandboxes: [], defaultSandbox: null }); + mocks.gatewayDoctorStartHint.mockReturnValue( + "Start the gateway again with `nemoclaw onboard`. Then retry this command.", + ); mocks.getNamedGatewayLifecycleState.mockReturnValue({ state: "healthy_named", status: "Status: Connected", @@ -151,6 +154,47 @@ describe("global doctor action", () => { expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); }); + it("reports invalid gateway management without hiding gateway health (#10212)", async () => { + mocks.gatewayDoctorStartHint.mockImplementationOnce(() => { + throw new Error("invalid declaration at /private/gateway-management.json"); + }); + mocks.getNamedGatewayLifecycleState.mockReturnValueOnce({ + state: "missing_named", + status: "Status: Disconnected", + gatewayInfo: "", + activeGateway: null, + }); + + const report = await runGlobalDoctor({ quiet: true }); + + expect(report).toMatchObject({ + schemaVersion: 1, + scope: "global", + status: "fail", + }); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + group: "Gateway", + label: "Gateway management", + status: "fail", + hint: "check the gateway-management declaration file permissions and JSON, then retry", + }), + expect.objectContaining({ + group: "Gateway", + label: "OpenShell status", + status: "fail", + hint: "check the gateway-management declaration file permissions and JSON, then retry", + }), + ]), + ); + expect(JSON.stringify(report)).not.toContain("/private/gateway-management.json"); + expect(mocks.getNamedGatewayLifecycleState).toHaveBeenCalledWith("nemoclaw", { + ignoreProbeErrors: true, + }); + expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); + }); + it("renders actionable text without naming a sandbox (#10212)", async () => { mocks.getNamedGatewayLifecycleState.mockReturnValueOnce({ state: "missing_named", diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index f266294c77b..6f66d7f7670 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -593,21 +593,52 @@ function unavailableGatewayCheck(): DoctorCheck { }; } +function globalGatewayGuidance(gatewayName: string): { + checks: DoctorCheck[]; + unavailableHint: string; +} { + try { + return { checks: [], unavailableHint: gatewayDoctorStartHint(gatewayName) }; + } catch { + const hint = + "check the gateway-management declaration file permissions and JSON, then retry"; + return { + checks: [ + { + group: "Gateway", + label: "Gateway management", + status: "fail", + detail: "could not resolve the gateway lifecycle owner", + hint, + }, + ], + unavailableHint: hint, + }; + } +} + export async function runGlobalDoctor( options: { quiet?: boolean } = {}, ): Promise { const host = collectDoctorHostChecks(null); const gatewayName = resolveGatewayName(GATEWAY_PORT); - const gatewayChecks = host.openshellBin - ? ( + let gatewayChecks: DoctorCheck[]; + if (host.openshellBin) { + const guidance = globalGatewayGuidance(gatewayName); + gatewayChecks = [ + ...guidance.checks, + ...( await collectDoctorGatewayChecks(gatewayName, null, host.openshellBin, { gatewayPort: GATEWAY_PORT, ignoreProbeErrors: true, recoverGateway: false, - unavailableHint: gatewayDoctorStartHint(gatewayName), + unavailableHint: guidance.unavailableHint, }) - ).checks - : [unavailableGatewayCheck()]; + ).checks, + ]; + } else { + gatewayChecks = [unavailableGatewayCheck()]; + } const report = buildGlobalDoctorReport([ ...host.checks, registryReadabilityCheck(), From 965eab220bd6d6240c54e4801569d34bd1d5a324 Mon Sep 17 00:00:00 2001 From: San Dang Date: Fri, 4 Sep 2026 16:00:08 +0700 Subject: [PATCH 05/11] fix(cli): address global doctor review findings Signed-off-by: San Dang --- ci/source-architecture-budget.json | 2 +- src/commands/doctor.ts | 5 +-- src/lib/actions/doctor.test.ts | 31 ++++++++++++++++ src/lib/actions/sandbox/doctor.ts | 8 ++-- src/lib/cli/argv-normalizer.test.ts | 16 ++++++++ src/lib/cli/argv-normalizer.ts | 5 ++- src/lib/cli/public-dispatch.ts | 11 ++++++ test/cli/dispatch-basics.test.ts | 39 ++++++++++++++++++++ test/support/public-dispatch-test-harness.ts | 7 +++- 9 files changed, 114 insertions(+), 10 deletions(-) diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index 8b64b6d931e..e6d8fb3321c 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -13,7 +13,7 @@ "src/lib/adapters/openshell/timeouts.ts": 33, "src/lib/agent/defs.ts": 32, "src/lib/cli/branding.ts": 80, - "src/lib/cli/nemoclaw-oclif-command.ts": 103, + "src/lib/cli/nemoclaw-oclif-command.ts": 104, "src/lib/cli/terminal-style.ts": 42, "src/lib/core/json-types.ts": 34, "src/lib/core/ports.ts": 89, diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index d69054ab099..7bc7abc4e29 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1,12 +1,11 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { Command, Flags } from "@oclif/core"; import { runGlobalDoctor } from "../lib/actions/sandbox/doctor"; import { redactForLog, withStdoutRedirectedToStderr } from "../lib/cli/doctor-command-support"; +import { NemoClawCommand } from "../lib/cli/nemoclaw-oclif-command"; -export default class DoctorCommand extends Command { - static baseFlags = { help: Flags.help({ char: "h" }) }; +export default class DoctorCommand extends NemoClawCommand { static id = "doctor"; static strict = true; static enableJsonFlag = true; diff --git a/src/lib/actions/doctor.test.ts b/src/lib/actions/doctor.test.ts index 7c809a66cc2..9aa3cd31f09 100644 --- a/src/lib/actions/doctor.test.ts +++ b/src/lib/actions/doctor.test.ts @@ -154,6 +154,37 @@ describe("global doctor action", () => { expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); }); + it("reports invalid gateway management when the OpenShell CLI is missing (#10212)", async () => { + mocks.resolveOpenshell.mockReturnValueOnce(null); + mocks.gatewayDoctorStartHint.mockImplementationOnce(() => { + throw new Error("Authorization: Bearer sk-secret-value in /private/gateway-management.json"); + }); + + const report = await runGlobalDoctor({ quiet: true }); + + expect(report.status).toBe("fail"); + expect(report.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + group: "Gateway", + label: "Gateway management", + status: "fail", + detail: "could not resolve the gateway lifecycle owner", + }), + expect.objectContaining({ + group: "Gateway", + label: "OpenShell status", + status: "fail", + detail: "skipped because the OpenShell CLI is not installed", + }), + ]), + ); + expect(JSON.stringify(report)).not.toContain("sk-secret-value"); + expect(JSON.stringify(report)).not.toContain("/private/gateway-management.json"); + expect(mocks.getNamedGatewayLifecycleState).not.toHaveBeenCalled(); + expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); + }); + it("reports invalid gateway management without hiding gateway health (#10212)", async () => { mocks.gatewayDoctorStartHint.mockImplementationOnce(() => { throw new Error("invalid declaration at /private/gateway-management.json"); diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index 6f66d7f7670..b76388fd553 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -622,11 +622,11 @@ export async function runGlobalDoctor( ): Promise { const host = collectDoctorHostChecks(null); const gatewayName = resolveGatewayName(GATEWAY_PORT); - let gatewayChecks: DoctorCheck[]; + const guidance = globalGatewayGuidance(gatewayName); + let gatewayChecks: DoctorCheck[] = [...guidance.checks]; if (host.openshellBin) { - const guidance = globalGatewayGuidance(gatewayName); gatewayChecks = [ - ...guidance.checks, + ...gatewayChecks, ...( await collectDoctorGatewayChecks(gatewayName, null, host.openshellBin, { gatewayPort: GATEWAY_PORT, @@ -637,7 +637,7 @@ export async function runGlobalDoctor( ).checks, ]; } else { - gatewayChecks = [unavailableGatewayCheck()]; + gatewayChecks.push(unavailableGatewayCheck()); } const report = buildGlobalDoctorReport([ ...host.checks, diff --git a/src/lib/cli/argv-normalizer.test.ts b/src/lib/cli/argv-normalizer.test.ts index 89ae7753481..539a89e2b5d 100644 --- a/src/lib/cli/argv-normalizer.test.ts +++ b/src/lib/cli/argv-normalizer.test.ts @@ -9,6 +9,7 @@ const globalCommands = new Set(["list", "status", "onboard", "doctor", "--versio const isConnectFlag = (arg: string | undefined) => arg === "--probe-only" || arg === "--help"; const normalizerOptions = { globalCommands, + isRegisteredSandbox: () => false, isSandboxAction: (arg: string | undefined) => ["status", "policy-add"].includes(arg ?? ""), isSandboxConnectFlag: isConnectFlag, }; @@ -103,6 +104,21 @@ describe("normalizeArgv", () => { ...(action ? { sandboxName: "doctor", action } : {}), }); }); + + it("preserves bare connect for a registered sandbox named doctor (#10212)", () => { + expect( + normalizeArgv(["doctor"], { + ...normalizerOptions, + isRegisteredSandbox: (name) => name === "doctor", + }), + ).toEqual({ + kind: "sandbox", + sandboxName: "doctor", + action: "connect", + actionArgs: [], + connectHelpRequested: false, + }); + }); }); describe("suggestCommand", () => { diff --git a/src/lib/cli/argv-normalizer.ts b/src/lib/cli/argv-normalizer.ts index 2c83b741542..e059ed54d18 100644 --- a/src/lib/cli/argv-normalizer.ts +++ b/src/lib/cli/argv-normalizer.ts @@ -22,6 +22,7 @@ export type NormalizedArgv = export type NormalizeArgvOptions = { globalCommands: ReadonlySet; + isRegisteredSandbox: (name: string) => boolean; isSandboxAction: (arg: string | undefined) => boolean; isSandboxConnectFlag: (arg: string | undefined) => boolean; }; @@ -32,7 +33,9 @@ export function isGlobalCommandInvocation( ): boolean { const [command, firstArg] = argv; if (!command || !opts.globalCommands.has(command)) return false; - return command !== "doctor" || !firstArg || !opts.isSandboxAction(firstArg); + if (command !== "doctor") return true; + if (firstArg) return !opts.isSandboxAction(firstArg); + return !opts.isRegisteredSandbox(command); } export function normalizeArgv(argv: readonly string[], opts: NormalizeArgvOptions): NormalizedArgv { diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index f29c36b09f6..f1fbb935063 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -48,6 +48,7 @@ const NATIVE_OCLIF_NAMESPACES = new Set(["internal", "sandbox"]); const MIGRATION_RECOVERY_SANDBOX_ACTIONS = new Set(["doctor", "recover"]); const PUBLIC_ARGV_OPTIONS: NormalizeArgvOptions = { globalCommands: GLOBAL_COMMANDS, + isRegisteredSandbox: hasRegisteredSandbox, isSandboxAction: isKnownSandboxAction, isSandboxConnectFlag: isPublicSandboxConnectFlag, }; @@ -79,6 +80,16 @@ function isPublicSandboxConnectFlag(arg: string | undefined): boolean { return sandboxConnect().isSandboxConnectFlag(arg); } +function hasRegisteredSandbox(name: string): boolean { + try { + return registry().getSandbox(name) !== null; + } catch { + // Global doctor owns the registry-readability diagnostic. If dispatch + // cannot inspect the registry, keep routing the bare token there. + return false; + } +} + // ── Commands ───────────────────────────────────────────────────── function oclifRunOptions(publicSandboxName?: string) { diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index a4773608321..22bb4e65b6d 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -292,6 +292,7 @@ describe("CLI dispatch", () => { expect( normalizeArgv(["-h"], { globalCommands: globalCommandTokens(), + isRegisteredSandbox: () => false, isSandboxAction: () => false, isSandboxConnectFlag: () => false, }), @@ -671,6 +672,27 @@ describe("CLI dispatch", () => { ); }); + it("dispatches global doctor when the sandbox registry is unreadable (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, migrateLegacyPortState, runOclifCommandById, stderr }) => { + await dispatchCli(["doctor"]); + + expect(runOclifCommandById).toHaveBeenCalledWith( + "doctor", + [], + expect.objectContaining({ rootDir: process.cwd() }), + ); + expect(migrateLegacyPortState).not.toHaveBeenCalled(); + expect(stderr).toEqual([]); + }, + { + registryReadError: new Error( + "Authorization: Bearer sk-secret-value in /private/sandboxes.json", + ), + }, + ); + }); + it( "emits one redacted JSON report through the public global doctor route (#10212)", testTimeoutOptions(35_000), @@ -916,6 +938,23 @@ describe("CLI dispatch", () => { ); }); + it("keeps bare connect for a sandbox literally named doctor (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, migrateLegacyPortState, runOclifCommandById, stderr }) => { + await dispatchCli(["doctor"]); + + expect(migrateLegacyPortState).toHaveBeenCalledTimes(1); + expect(runOclifCommandById).toHaveBeenCalledWith( + "sandbox:connect", + ["doctor"], + expect.anything(), + ); + expect(stderr).toEqual([]); + }, + { sandboxNames: ["doctor"] }, + ); + }); + it("recovers a live sandbox named after an action before reporting scope (#10212)", async () => { await withDirectPublicDispatch( async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById, sandboxes, stderr }) => { diff --git a/test/support/public-dispatch-test-harness.ts b/test/support/public-dispatch-test-harness.ts index af1ff6500e3..cfafd5ced6e 100644 --- a/test/support/public-dispatch-test-harness.ts +++ b/test/support/public-dispatch-test-harness.ts @@ -30,6 +30,8 @@ type DirectPublicDispatchOptions = { connectFlags?: readonly string[]; /** Error injected by the pre-dispatch legacy-state migration seam. */ migrationError?: Error; + /** Error injected by sandbox registry lookups. */ + registryReadError?: Error; }; const requireCache = require.cache as Record; @@ -82,7 +84,10 @@ export async function withDirectPublicDispatch( }, ]), ); - const getSandbox = vi.fn((name: string) => sandboxes.get(name) ?? null); + const getSandbox = vi.fn((name: string) => { + if (options.registryReadError) throw options.registryReadError; + return sandboxes.get(name) ?? null; + }); const isPublishedSandboxRegistration = vi.fn( (sandbox: SandboxStub) => sandbox.pendingRouteReservation !== true, ); From 6f10104e1316d58992642c0515085730e1e0cf89 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Fri, 4 Sep 2026 08:14:54 -0700 Subject: [PATCH 06/11] fix(cli): preserve global doctor boundaries Signed-off-by: Aaron Erickson --- ci/source-architecture-budget.json | 4 +- src/commands/doctor.test.ts | 3 +- src/commands/doctor.ts | 6 +-- src/commands/sandbox/doctor.ts | 9 ++--- src/lib/actions/sandbox/doctor-report.ts | 6 ++- src/lib/actions/sandbox/doctor.ts | 1 + src/lib/cli/argv-normalizer.test.ts | 13 +++++-- src/lib/cli/argv-normalizer.ts | 5 ++- src/lib/cli/doctor-command-support.ts | 5 --- test/cli/dispatch-basics.test.ts | 41 ++++++++++++++------ test/support/public-dispatch-test-harness.ts | 5 ++- 11 files changed, 61 insertions(+), 37 deletions(-) delete mode 100644 src/lib/cli/doctor-command-support.ts diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index e6d8fb3321c..4d6fedf9f44 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -13,7 +13,7 @@ "src/lib/adapters/openshell/timeouts.ts": 33, "src/lib/agent/defs.ts": 32, "src/lib/cli/branding.ts": 80, - "src/lib/cli/nemoclaw-oclif-command.ts": 104, + "src/lib/cli/nemoclaw-oclif-command.ts": 105, "src/lib/cli/terminal-style.ts": 42, "src/lib/core/json-types.ts": 34, "src/lib/core/ports.ts": 89, @@ -25,7 +25,7 @@ "src/lib/messaging/channels/index.ts": 25, "src/lib/onboard/gateway-binding.ts": 53, "src/lib/runner.ts": 84, - "src/lib/security/redact.ts": 53, + "src/lib/security/redact.ts": 52, "src/lib/state/mcp-lifecycle-lock.ts": 21, "src/lib/state/onboard-session.ts": 35, "src/lib/state/registry.ts": 96, diff --git a/src/commands/doctor.test.ts b/src/commands/doctor.test.ts index 8dc0b7afe96..db6b4564754 100644 --- a/src/commands/doctor.test.ts +++ b/src/commands/doctor.test.ts @@ -8,7 +8,8 @@ const mocks = vi.hoisted(() => ({ runGlobalDoctor: vi.fn(), })); -vi.mock("../lib/actions/sandbox/doctor", () => ({ +vi.mock("../lib/actions/sandbox/doctor", async (importOriginal) => ({ + ...(await importOriginal()), runGlobalDoctor: mocks.runGlobalDoctor, })); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 7bc7abc4e29..9179643da2d 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1,9 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { runGlobalDoctor } from "../lib/actions/sandbox/doctor"; -import { redactForLog, withStdoutRedirectedToStderr } from "../lib/cli/doctor-command-support"; +import { redactDoctorReport, runGlobalDoctor } from "../lib/actions/sandbox/doctor"; import { NemoClawCommand } from "../lib/cli/nemoclaw-oclif-command"; +import { withStdoutRedirectedToStderr } from "../lib/cli/stdout-guard"; export default class DoctorCommand extends NemoClawCommand { static id = "doctor"; @@ -23,6 +23,6 @@ export default class DoctorCommand extends NemoClawCommand { ? await withStdoutRedirectedToStderr(() => runGlobalDoctor({ quiet: true })) : await runGlobalDoctor(); if (report.failed > 0) process.exitCode = 1; - return json ? redactForLog(report) : undefined; + return json ? redactDoctorReport(report) : undefined; } } diff --git a/src/commands/sandbox/doctor.ts b/src/commands/sandbox/doctor.ts index 0a1d004e1bd..fe216b6bc03 100644 --- a/src/commands/sandbox/doctor.ts +++ b/src/commands/sandbox/doctor.ts @@ -2,12 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 import { Args, Flags } from "@oclif/core"; -import { runSandboxDoctor } from "../../lib/actions/sandbox/doctor"; -import { - redactForLog, - withStdoutRedirectedToStderr, -} from "../../lib/cli/doctor-command-support"; +import { redactDoctorReport, runSandboxDoctor } from "../../lib/actions/sandbox/doctor"; import { NemoClawCommand } from "../../lib/cli/nemoclaw-oclif-command"; +import { withStdoutRedirectedToStderr } from "../../lib/cli/stdout-guard"; export default class SandboxDoctorCliCommand extends NemoClawCommand { static id = "sandbox:doctor"; @@ -55,7 +52,7 @@ export default class SandboxDoctorCliCommand extends NemoClawCommand { // report itself so programmatic consumers of the resolved value — not // just the logJson-printed stdout (#3657) — never see token-shaped // values in check details. - return redactForLog(report); + return redactDoctorReport(report); } const doctorArgs = flags.fix ? ["--fix"] : []; await runSandboxDoctor(args.sandboxName, doctorArgs, { quietJson: false }); diff --git a/src/lib/actions/sandbox/doctor-report.ts b/src/lib/actions/sandbox/doctor-report.ts index bab6ca327bb..c01cc82e984 100644 --- a/src/lib/actions/sandbox/doctor-report.ts +++ b/src/lib/actions/sandbox/doctor-report.ts @@ -36,6 +36,10 @@ export type GlobalDoctorReport = { type RenderableDoctorReport = DoctorReport | GlobalDoctorReport; +export function redactDoctorReport(report: T): T { + return redactForLog(report) as T; +} + function summarizeChecks(checks: DoctorCheck[]): { status: DoctorReportStatus; failed: number; @@ -121,7 +125,7 @@ function renderSummary(report: RenderableDoctorReport): void { } export function renderDoctorReport(report: RenderableDoctorReport, asJson: boolean): number { - const displayReport = redactForLog(report) as RenderableDoctorReport; + const displayReport = redactDoctorReport(report); if (asJson) { // Parity with `sandbox status --json` (#4310): this console.log egress // bypasses the oclif logJson redaction boundary (#3657), so route the diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index b76388fd553..2b162d073f2 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -74,6 +74,7 @@ import { import { buildToolScopeChecks } from "./doctor-tool-scope"; export type { DoctorCheck, DoctorReport } from "./doctor-report"; +export { redactDoctorReport } from "./doctor-report"; type RunSandboxDoctorOptions = { quietJson?: boolean; diff --git a/src/lib/cli/argv-normalizer.test.ts b/src/lib/cli/argv-normalizer.test.ts index 539a89e2b5d..30726376af4 100644 --- a/src/lib/cli/argv-normalizer.test.ts +++ b/src/lib/cli/argv-normalizer.test.ts @@ -105,9 +105,14 @@ describe("normalizeArgv", () => { }); }); - it("preserves bare connect for a registered sandbox named doctor (#10212)", () => { + it.each([ + { label: "bare", firstArg: undefined, connectHelpRequested: false }, + { label: "help", firstArg: "--help", connectHelpRequested: true }, + { label: "probe-only", firstArg: "--probe-only", connectHelpRequested: false }, + ])("preserves $label connect for a registered sandbox named doctor (#10212)", (testCase) => { + const argv = testCase.firstArg ? ["doctor", testCase.firstArg] : ["doctor"]; expect( - normalizeArgv(["doctor"], { + normalizeArgv(argv, { ...normalizerOptions, isRegisteredSandbox: (name) => name === "doctor", }), @@ -115,8 +120,8 @@ describe("normalizeArgv", () => { kind: "sandbox", sandboxName: "doctor", action: "connect", - actionArgs: [], - connectHelpRequested: false, + actionArgs: testCase.firstArg ? [testCase.firstArg] : [], + connectHelpRequested: testCase.connectHelpRequested, }); }); }); diff --git a/src/lib/cli/argv-normalizer.ts b/src/lib/cli/argv-normalizer.ts index e059ed54d18..bfd85dbe605 100644 --- a/src/lib/cli/argv-normalizer.ts +++ b/src/lib/cli/argv-normalizer.ts @@ -34,8 +34,9 @@ export function isGlobalCommandInvocation( const [command, firstArg] = argv; if (!command || !opts.globalCommands.has(command)) return false; if (command !== "doctor") return true; - if (firstArg) return !opts.isSandboxAction(firstArg); - return !opts.isRegisteredSandbox(command); + if (!firstArg) return !opts.isRegisteredSandbox(command); + if (opts.isSandboxConnectFlag(firstArg) && opts.isRegisteredSandbox(command)) return false; + return !opts.isSandboxAction(firstArg); } export function normalizeArgv(argv: readonly string[], opts: NormalizeArgvOptions): NormalizedArgv { diff --git a/src/lib/cli/doctor-command-support.ts b/src/lib/cli/doctor-command-support.ts deleted file mode 100644 index 4a745ad3569..00000000000 --- a/src/lib/cli/doctor-command-support.ts +++ /dev/null @@ -1,5 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -export { withStdoutRedirectedToStderr } from "./stdout-guard"; -export { redactForLog } from "../security/redact"; diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index 22bb4e65b6d..ee8d8363159 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -938,20 +938,37 @@ describe("CLI dispatch", () => { ); }); - it("keeps bare connect for a sandbox literally named doctor (#10212)", async () => { + it.each([ + { label: "bare", args: [] as string[], migrationCalls: 1, helpCalls: 0 }, + { label: "help", args: ["--help"], migrationCalls: 0, helpCalls: 1 }, + { label: "probe-only", args: ["--probe-only"], migrationCalls: 1, helpCalls: 0 }, + ])("keeps $label connect for a sandbox literally named doctor (#10212)", async (testCase) => { await withDirectPublicDispatch( - async ({ dispatchCli, migrateLegacyPortState, runOclifCommandById, stderr }) => { - await dispatchCli(["doctor"]); - - expect(migrateLegacyPortState).toHaveBeenCalledTimes(1); - expect(runOclifCommandById).toHaveBeenCalledWith( - "sandbox:connect", - ["doctor"], - expect.anything(), - ); - expect(stderr).toEqual([]); + async ({ + dispatchCli, + migrateLegacyPortState, + printSandboxConnectHelp, + runOclifCommandById, + stderr, + }) => { + await dispatchCli(["doctor", ...testCase.args]); + + expect({ + migrationCalls: migrateLegacyPortState.mock.calls.length, + helpCalls: printSandboxConnectHelp.mock.calls.length, + oclifCall: runOclifCommandById.mock.calls[0]?.slice(0, 2) ?? null, + stderr, + }).toEqual({ + migrationCalls: testCase.migrationCalls, + helpCalls: testCase.helpCalls, + oclifCall: + testCase.helpCalls > 0 + ? null + : ["sandbox:connect", ["doctor", ...testCase.args]], + stderr: [], + }); }, - { sandboxNames: ["doctor"] }, + { sandboxNames: ["doctor"], connectFlags: ["--help", "--probe-only"] }, ); }); diff --git a/test/support/public-dispatch-test-harness.ts b/test/support/public-dispatch-test-harness.ts index cfafd5ced6e..ba1764ca35b 100644 --- a/test/support/public-dispatch-test-harness.ts +++ b/test/support/public-dispatch-test-harness.ts @@ -12,6 +12,7 @@ export type DirectPublicDispatchHarness = { getSandbox: ReturnType; listSandboxes: ReturnType; migrateLegacyPortState: ReturnType; + printSandboxConnectHelp: ReturnType; recoverRegistryEntries: ReturnType; resetObservedCalls: () => void; runOclifArgv: ReturnType; @@ -115,6 +116,7 @@ export async function withDirectPublicDispatch( }); const runOclifArgv = vi.fn(async () => undefined); const runOclifCommandById = vi.fn(async () => undefined); + const printSandboxConnectHelp = vi.fn(); const stderr: string[] = []; const previousExitCode = process.exitCode; const errorSpy = vi.spyOn(console, "error").mockImplementation((...args: unknown[]) => { @@ -150,7 +152,7 @@ export async function withDirectPublicDispatch( typeof arg === "string" ? connectFlags.has(arg) : false, ), parseSandboxConnectArgs: vi.fn(), - printSandboxConnectHelp: vi.fn(), + printSandboxConnectHelp, }); try { @@ -165,6 +167,7 @@ export async function withDirectPublicDispatch( getSandbox, listSandboxes, migrateLegacyPortState, + printSandboxConnectHelp, recoverRegistryEntries, resetObservedCalls, runOclifArgv, From cba85aaf2e8e1f71aaf67ea1d7bbd769f6fa9ce3 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Fri, 4 Sep 2026 08:33:02 -0700 Subject: [PATCH 07/11] fix(cli): migrate colliding doctor probe route Signed-off-by: Aaron Erickson --- src/lib/cli/argv-normalizer.test.ts | 2 ++ src/lib/cli/argv-normalizer.ts | 5 ++++- test/cli/dispatch-basics.test.ts | 28 ++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/lib/cli/argv-normalizer.test.ts b/src/lib/cli/argv-normalizer.test.ts index 30726376af4..181415f5f18 100644 --- a/src/lib/cli/argv-normalizer.test.ts +++ b/src/lib/cli/argv-normalizer.test.ts @@ -95,7 +95,9 @@ describe("normalizeArgv", () => { it.each([ { argv: ["doctor"], kind: "global", action: undefined }, + { argv: ["doctor", "--help"], kind: "global", action: undefined }, { argv: ["doctor", "--json"], kind: "global", action: undefined }, + { argv: ["doctor", "--probe-only"], kind: "sandbox", action: "connect" }, { argv: ["doctor", "status"], kind: "sandbox", action: "status" }, { argv: ["doctor", "policy-add"], kind: "sandbox", action: "policy-add" }, ])("classifies $argv from one doctor scope rule", ({ argv, kind, action }) => { diff --git a/src/lib/cli/argv-normalizer.ts b/src/lib/cli/argv-normalizer.ts index bfd85dbe605..3d15980f872 100644 --- a/src/lib/cli/argv-normalizer.ts +++ b/src/lib/cli/argv-normalizer.ts @@ -35,7 +35,10 @@ export function isGlobalCommandInvocation( if (!command || !opts.globalCommands.has(command)) return false; if (command !== "doctor") return true; if (!firstArg) return !opts.isRegisteredSandbox(command); - if (opts.isSandboxConnectFlag(firstArg) && opts.isRegisteredSandbox(command)) return false; + if (opts.isSandboxConnectFlag(firstArg)) { + const isHelpFlag = firstArg === "--help" || firstArg === "-h"; + if (!isHelpFlag || opts.isRegisteredSandbox(command)) return false; + } return !opts.isSandboxAction(firstArg); } diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index ee8d8363159..153e3cc80bb 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -972,6 +972,34 @@ describe("CLI dispatch", () => { ); }); + it("migrates a legacy sandbox named doctor before probe-only connect (#10212)", async () => { + await withDirectPublicDispatch( + async ({ dispatchCli, migrateLegacyPortState, runOclifCommandById, sandboxes, stderr }) => { + migrateLegacyPortState.mockImplementation(() => { + sandboxes.set("doctor", { name: "doctor" }); + return { + migratedSandboxNames: ["doctor"], + migratedSession: false, + warnings: [], + }; + }); + + await dispatchCli(["doctor", "--probe-only"]); + + expect({ + migrationCalls: migrateLegacyPortState.mock.calls.length, + oclifCall: runOclifCommandById.mock.calls[0]?.slice(0, 2) ?? null, + stderr, + }).toEqual({ + migrationCalls: 1, + oclifCall: ["sandbox:connect", ["doctor", "--probe-only"]], + stderr: [expect.stringContaining("Migrated legacy state")], + }); + }, + { connectFlags: ["--probe-only"] }, + ); + }); + it("recovers a live sandbox named after an action before reporting scope (#10212)", async () => { await withDirectPublicDispatch( async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById, sandboxes, stderr }) => { From 3098bac3c43bdebc4e4fd4462398f298a50d1825 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Fri, 4 Sep 2026 08:49:28 -0700 Subject: [PATCH 08/11] fix(cli): disambiguate global doctor text output Signed-off-by: Aaron Erickson --- docs/reference/commands.mdx | 6 ++- src/commands/doctor.test.ts | 2 +- src/commands/doctor.ts | 16 ++++++-- src/lib/cli/argv-normalizer.test.ts | 1 + src/lib/cli/public-display-defaults.ts | 2 +- test/cli/dispatch-basics.test.ts | 53 ++++++++++++++------------ 6 files changed, 49 insertions(+), 31 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index c4b2c33a3cd..6d2fbc2b060 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1596,7 +1596,7 @@ If the sandbox is running an older Deep Agents Code version than this NemoClaw r ### `$$nemoclaw doctor` Run a focused health check for one sandbox and the host services it depends on. The command checks the local CLI build, Docker daemon, OpenShell CLI, NemoClaw gateway container, gateway port mapping, live sandbox state, inference route, configured-provider model invocation, Ollama reachability, and the cloudflared tunnel state. -Use `$$nemoclaw doctor` when you need only the host and gateway checks or when no sandbox exists yet. +Use `$$nemoclaw doctor` when you need only the host and gateway checks or when no sandbox exists yet. If a sandbox is named `doctor`, use `$$nemoclaw doctor --text` to select the global text report explicitly; bare `$$nemoclaw doctor` keeps the existing name-first sandbox connection behavior. For gateway-based agents, it also reports messaging channel conflicts within the selected @@ -3180,9 +3180,10 @@ It checks the NemoClaw CLI build, the selected host runtime provider, the OpenSh It does not start, select, restart, or repair a gateway. It does not run sandbox, inference, messaging, agent-version, config-permission, or agent-service checks. Use `$$nemoclaw doctor` when you need those sandbox checks. +If a sandbox is named `doctor`, bare `$$nemoclaw doctor` connects to that sandbox under the name-first grammar. Use `$$nemoclaw doctor --text` for the global human-readable report or `$$nemoclaw doctor --json` for the global JSON report. ```bash -$$nemoclaw doctor [--json] +$$nemoclaw doctor [--json | --text] ``` The command exits nonzero when a required check fails. @@ -3191,6 +3192,7 @@ Pass `--json` for a redacted, schema-versioned report with `scope: "global"` and | Flag | Description | | --- | --- | | `--json` | Emit the global report as JSON. | +| `--text` | Emit the global human-readable report explicitly. | ### `$$nemoclaw debug` diff --git a/src/commands/doctor.test.ts b/src/commands/doctor.test.ts index db6b4564754..b129e17e0a5 100644 --- a/src/commands/doctor.test.ts +++ b/src/commands/doctor.test.ts @@ -40,7 +40,7 @@ describe("global doctor command", () => { "runs the read-only text diagnosis without a sandbox (#10212)", testTimeoutOptions(30_000), async () => { - await DoctorCommand.run([], rootDir); + await DoctorCommand.run(["--text"], rootDir); expect(mocks.runGlobalDoctor).toHaveBeenCalledWith(); expect(process.exitCode).toBeUndefined(); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 9179643da2d..d5d797c007d 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { Flags } from "@oclif/core"; import { redactDoctorReport, runGlobalDoctor } from "../lib/actions/sandbox/doctor"; import { NemoClawCommand } from "../lib/cli/nemoclaw-oclif-command"; import { withStdoutRedirectedToStderr } from "../lib/cli/stdout-guard"; @@ -12,9 +13,18 @@ export default class DoctorCommand extends NemoClawCommand { static summary = "Diagnose host and gateway health"; static description = "Run read-only host, runtime provider, OpenShell CLI, sandbox registry, and NemoClaw gateway checks. Use ` doctor` for one sandbox."; - static usage = ["doctor [--json]"]; - static examples = ["<%= config.bin %> doctor", "<%= config.bin %> doctor --json"]; - static flags = {}; + static usage = ["doctor [--json|--text]"]; + static examples = [ + "<%= config.bin %> doctor", + "<%= config.bin %> doctor --text", + "<%= config.bin %> doctor --json", + ]; + static flags = { + text: Flags.boolean({ + description: "Emit the human-readable report explicitly", + exclusive: ["json"], + }), + }; public async run(): Promise { await this.parse(DoctorCommand); diff --git a/src/lib/cli/argv-normalizer.test.ts b/src/lib/cli/argv-normalizer.test.ts index 181415f5f18..09ff1d7318a 100644 --- a/src/lib/cli/argv-normalizer.test.ts +++ b/src/lib/cli/argv-normalizer.test.ts @@ -97,6 +97,7 @@ describe("normalizeArgv", () => { { argv: ["doctor"], kind: "global", action: undefined }, { argv: ["doctor", "--help"], kind: "global", action: undefined }, { argv: ["doctor", "--json"], kind: "global", action: undefined }, + { argv: ["doctor", "--text"], kind: "global", action: undefined }, { argv: ["doctor", "--probe-only"], kind: "sandbox", action: "connect" }, { argv: ["doctor", "status"], kind: "sandbox", action: "status" }, { argv: ["doctor", "policy-add"], kind: "sandbox", action: "policy-add" }, diff --git a/src/lib/cli/public-display-defaults.ts b/src/lib/cli/public-display-defaults.ts index bbb3f2f6278..7d7ca865b85 100644 --- a/src/lib/cli/public-display-defaults.ts +++ b/src/lib/cli/public-display-defaults.ts @@ -66,7 +66,7 @@ const PUBLIC_DISPLAY_LAYOUT: Record = { { group: "Troubleshooting", order: 36, - flags: "[--json]", + flags: "[--json|--text]", }, ], gc: [ diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index 153e3cc80bb..d52fb248ddf 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -694,7 +694,7 @@ describe("CLI dispatch", () => { }); it( - "emits one redacted JSON report through the public global doctor route (#10212)", + "emits one redacted JSON report for the selected non-default gateway (#10212)", testTimeoutOptions(35_000), () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-global-doctor-json-")); @@ -716,8 +716,8 @@ describe("CLI dispatch", () => { " printf 'Status: Disconnected\\nGateway: nemoclaw\\nAuthorization: Bearer sk-abc123DEF456ghi789\\n'", " exit 1", "fi", - 'if [ "$1" = "gateway" ] && [ "$2" = "info" ]; then', - " printf 'Gateway: nemoclaw\\n'", + 'if [ "$1" = "gateway" ] && [ "$2" = "info" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw-19080" ]; then', + " printf 'Gateway: nemoclaw-19080\\n'", " exit 0", "fi", "exit 97", @@ -732,6 +732,7 @@ describe("CLI dispatch", () => { env: { ...process.env, HOME: home, + NEMOCLAW_GATEWAY_PORT: "19080", NEMOCLAW_GATEWAY_RUNTIME: "docker", NEMOCLAW_OPENSHELL_BIN: openshellBin, PATH: `${localBin}:${process.env.PATH || ""}`, @@ -749,7 +750,7 @@ describe("CLI dispatch", () => { expect(fs.readFileSync(registryFile, "utf8")).toBe(emptyRegistry); expect(fs.readFileSync(openshellLog, "utf8").trim().split("\n")).toEqual([ "status", - "gateway info -g nemoclaw", + "gateway info -g nemoclaw-19080", ]); } finally { fs.rmSync(home, { recursive: true, force: true }); @@ -757,26 +758,30 @@ describe("CLI dispatch", () => { }, ); - it("dispatches global doctor flags without reading sandbox state (#10212)", async () => { - await withDirectPublicDispatch( - async ({ - dispatchCli, - migrateLegacyPortState, - recoverRegistryEntries, - runOclifCommandById, - }) => { - await dispatchCli(["doctor", "--json"]); - - expect(runOclifCommandById).toHaveBeenCalledWith( - "doctor", - ["--json"], - expect.objectContaining({ rootDir: process.cwd() }), - ); - expect(migrateLegacyPortState).not.toHaveBeenCalled(); - expect(recoverRegistryEntries).not.toHaveBeenCalled(); - }, - ); - }); + it.each(["--json", "--text"])( + "dispatches global doctor %s without reading sandbox state (#10212)", + async (flag) => { + await withDirectPublicDispatch( + async ({ + dispatchCli, + migrateLegacyPortState, + recoverRegistryEntries, + runOclifCommandById, + }) => { + await dispatchCli(["doctor", flag]); + + expect(runOclifCommandById).toHaveBeenCalledWith( + "doctor", + [flag], + expect.objectContaining({ rootDir: process.cwd() }), + ); + expect(migrateLegacyPortState).not.toHaveBeenCalled(); + expect(recoverRegistryEntries).not.toHaveBeenCalled(); + }, + { sandboxNames: ["doctor"] }, + ); + }, + ); it("reports the sandbox-first grammar without recovering a bare action (#10212)", async () => { await withDirectPublicDispatch( From f9fc6c84460a4040f4e352153c95254d03f7846a Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Fri, 4 Sep 2026 09:07:58 -0700 Subject: [PATCH 09/11] fix(cli): preserve legacy doctor sandbox routing Signed-off-by: Aaron Erickson --- src/lib/cli/argv-normalizer.test.ts | 11 ++++++++-- src/lib/cli/public-dispatch.ts | 16 +++++++++++++-- src/lib/state/legacy-port-migration.test.ts | 21 +++++++++++++++++++- src/lib/state/legacy-port-migration.ts | 21 ++++++++++++++++++++ test/cli/dispatch-basics.test.ts | 11 ++++++---- test/support/public-dispatch-test-harness.ts | 6 +++++- 6 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/lib/cli/argv-normalizer.test.ts b/src/lib/cli/argv-normalizer.test.ts index 09ff1d7318a..26dd08327ea 100644 --- a/src/lib/cli/argv-normalizer.test.ts +++ b/src/lib/cli/argv-normalizer.test.ts @@ -98,11 +98,18 @@ describe("normalizeArgv", () => { { argv: ["doctor", "--help"], kind: "global", action: undefined }, { argv: ["doctor", "--json"], kind: "global", action: undefined }, { argv: ["doctor", "--text"], kind: "global", action: undefined }, + { argv: ["doctor", "--json"], kind: "global", action: undefined, registered: true }, + { argv: ["doctor", "--text"], kind: "global", action: undefined, registered: true }, { argv: ["doctor", "--probe-only"], kind: "sandbox", action: "connect" }, { argv: ["doctor", "status"], kind: "sandbox", action: "status" }, { argv: ["doctor", "policy-add"], kind: "sandbox", action: "policy-add" }, - ])("classifies $argv from one doctor scope rule", ({ argv, kind, action }) => { - expect(normalizeArgv(argv, normalizerOptions)).toMatchObject({ + ])("classifies $argv from one doctor scope rule", ({ argv, kind, action, registered }) => { + expect( + normalizeArgv(argv, { + ...normalizerOptions, + isRegisteredSandbox: () => registered === true, + }), + ).toMatchObject({ kind, ...(action ? { sandboxName: "doctor", action } : {}), }); diff --git a/src/lib/cli/public-dispatch.ts b/src/lib/cli/public-dispatch.ts index f1fbb935063..4ddf07c8148 100644 --- a/src/lib/cli/public-dispatch.ts +++ b/src/lib/cli/public-dispatch.ts @@ -22,7 +22,10 @@ const { sandboxActionTokensForDispatch, } = require("./command-registry"); -import { migrateLegacyPortState } from "../state/legacy-port-migration"; +import { + hasMigratableLegacySandbox, + migrateLegacyPortState, +} from "../state/legacy-port-migration"; import { isGlobalCommandInvocation, type NormalizedArgv, @@ -48,7 +51,7 @@ const NATIVE_OCLIF_NAMESPACES = new Set(["internal", "sandbox"]); const MIGRATION_RECOVERY_SANDBOX_ACTIONS = new Set(["doctor", "recover"]); const PUBLIC_ARGV_OPTIONS: NormalizeArgvOptions = { globalCommands: GLOBAL_COMMANDS, - isRegisteredSandbox: hasRegisteredSandbox, + isRegisteredSandbox: hasRegisteredOrMigratableSandbox, isSandboxAction: isKnownSandboxAction, isSandboxConnectFlag: isPublicSandboxConnectFlag, }; @@ -90,6 +93,15 @@ function hasRegisteredSandbox(name: string): boolean { } } +function hasRegisteredOrMigratableSandbox(name: string): boolean { + if (hasRegisteredSandbox(name)) return true; + try { + return hasMigratableLegacySandbox(name); + } catch { + return false; + } +} + // ── Commands ───────────────────────────────────────────────────── function oclifRunOptions(publicSandboxName?: string) { diff --git a/src/lib/state/legacy-port-migration.test.ts b/src/lib/state/legacy-port-migration.test.ts index 1adfbc007b3..5c1b92df88e 100644 --- a/src/lib/state/legacy-port-migration.test.ts +++ b/src/lib/state/legacy-port-migration.test.ts @@ -11,7 +11,7 @@ import { type OnboardEntryOptionsDeps, resolveOnboardEntryOptions, } from "../onboard/entry-options"; -import { migrateLegacyPortState } from "./legacy-port-migration"; +import { hasMigratableLegacySandbox, migrateLegacyPortState } from "./legacy-port-migration"; import { listRetainedSandboxRecoveryRecords, recordRetainedSandboxRecovery, @@ -92,6 +92,25 @@ afterEach(() => { }); describe("legacy non-default gateway state migration", () => { + it("detects an exact migratable sandbox without changing legacy state", () => { + const home = makeHome(); + const registryFile = path.join(home, ".nemoclaw", "sandboxes.json"); + writeJson(registryFile, { + defaultSandbox: "doctor", + sandboxes: { + doctor: { name: "doctor", gatewayName: "nemoclaw-9123", gatewayPort: 9123 }, + }, + }); + const before = fs.readFileSync(registryFile, "utf8"); + + expect({ + selected: hasMigratableLegacySandbox("doctor", { home, gatewayPort: 9123 }), + otherPort: hasMigratableLegacySandbox("doctor", { home, gatewayPort: 9124 }), + missing: hasMigratableLegacySandbox("missing", { home, gatewayPort: 9123 }), + unchanged: fs.readFileSync(registryFile, "utf8") === before, + }).toEqual({ selected: true, otherPort: false, missing: false, unchanged: true }); + }); + it("partitions a recovery-only session and mixed-gateway recovery authority", () => { const home = makeHome(); const shared = path.join(home, ".nemoclaw"); diff --git a/src/lib/state/legacy-port-migration.ts b/src/lib/state/legacy-port-migration.ts index be3c0017999..1611edc04b5 100644 --- a/src/lib/state/legacy-port-migration.ts +++ b/src/lib/state/legacy-port-migration.ts @@ -67,6 +67,27 @@ export interface LegacyPortMigrationResult { warnings: string[]; } +/** Read-only collision check used before deciding whether public argv needs migration. */ +export function hasMigratableLegacySandbox( + sandboxName: string, + options: { gatewayPort?: number; home?: string } = {}, +): boolean { + const gatewayPort = options.gatewayPort ?? GATEWAY_PORT; + if (gatewayPort === DEFAULT_GATEWAY_PORT) return false; + const home = path.resolve(options.home || resolveHome()); + const sharedRoot = nemoclawStateRoot(home, DEFAULT_GATEWAY_PORT); + const pendingIntent = readMigrationIntent(home, sharedRoot); + if ( + pendingIntent?.metadata.gatewayPort === gatewayPort && + pendingIntent.metadata.selectedSandboxNames.includes(sandboxName) + ) { + return true; + } + const legacyRegistry = readGatewayRegistryFile(home, path.join(sharedRoot, "sandboxes.json")); + const entry = legacyRegistry?.sandboxes[sandboxName]; + return entry ? registryEntryGatewayPort(entry) === gatewayPort : false; +} + interface LegacyPortMigrationIntentMetadata { version: typeof MIGRATION_INTENT_VERSION; gatewayPort: number; diff --git a/test/cli/dispatch-basics.test.ts b/test/cli/dispatch-basics.test.ts index d52fb248ddf..601fbfa92ff 100644 --- a/test/cli/dispatch-basics.test.ts +++ b/test/cli/dispatch-basics.test.ts @@ -977,7 +977,10 @@ describe("CLI dispatch", () => { ); }); - it("migrates a legacy sandbox named doctor before probe-only connect (#10212)", async () => { + it.each([ + { label: "bare", args: [] as string[] }, + { label: "probe-only", args: ["--probe-only"] }, + ])("migrates a legacy sandbox named doctor before $label connect (#10212)", async (testCase) => { await withDirectPublicDispatch( async ({ dispatchCli, migrateLegacyPortState, runOclifCommandById, sandboxes, stderr }) => { migrateLegacyPortState.mockImplementation(() => { @@ -989,7 +992,7 @@ describe("CLI dispatch", () => { }; }); - await dispatchCli(["doctor", "--probe-only"]); + await dispatchCli(["doctor", ...testCase.args]); expect({ migrationCalls: migrateLegacyPortState.mock.calls.length, @@ -997,11 +1000,11 @@ describe("CLI dispatch", () => { stderr, }).toEqual({ migrationCalls: 1, - oclifCall: ["sandbox:connect", ["doctor", "--probe-only"]], + oclifCall: ["sandbox:connect", ["doctor", ...testCase.args]], stderr: [expect.stringContaining("Migrated legacy state")], }); }, - { connectFlags: ["--probe-only"] }, + { connectFlags: ["--probe-only"], migratableSandboxNames: ["doctor"] }, ); }); diff --git a/test/support/public-dispatch-test-harness.ts b/test/support/public-dispatch-test-harness.ts index ba1764ca35b..16552eab5ea 100644 --- a/test/support/public-dispatch-test-harness.ts +++ b/test/support/public-dispatch-test-harness.ts @@ -29,6 +29,8 @@ type DirectPublicDispatchOptions = { pendingSandboxNames?: readonly string[]; /** Args the sandbox-connect stub treats as connect flags (default: none). */ connectFlags?: readonly string[]; + /** Legacy sandbox rows that belong to the selected gateway but are not migrated yet. */ + migratableSandboxNames?: readonly string[]; /** Error injected by the pre-dispatch legacy-state migration seam. */ migrationError?: Error; /** Error injected by sandbox registry lookups. */ @@ -114,6 +116,8 @@ export async function withDirectPublicDispatch( if (options.migrationError) throw options.migrationError; return { migratedSandboxNames: [], migratedSession: false, warnings: [] }; }); + const migratableSandboxNames = new Set(options.migratableSandboxNames ?? []); + const hasMigratableLegacySandbox = vi.fn((name: string) => migratableSandboxNames.has(name)); const runOclifArgv = vi.fn(async () => undefined); const runOclifCommandById = vi.fn(async () => undefined); const printSandboxConnectHelp = vi.fn(); @@ -143,7 +147,7 @@ export async function withDirectPublicDispatch( isPublishedSandboxRegistration, listSandboxes, }); - cacheModule(legacyPortMigrationPath, { migrateLegacyPortState }); + cacheModule(legacyPortMigrationPath, { hasMigratableLegacySandbox, migrateLegacyPortState }); cacheModule(registryRecoveryPath, { recoverRegistryEntries }); cacheModule(oclifRunnerPath, { runOclifArgv, runOclifCommandById }); const connectFlags = new Set(options.connectFlags ?? []); From 6a85a8fa1c216fd865c1c519b45088b85abb41ac Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Fri, 4 Sep 2026 09:25:45 -0700 Subject: [PATCH 10/11] test(cli): preserve doctor report exports in adapter mock Signed-off-by: Aaron Erickson --- src/commands/sandbox/oclif-command-adapters.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands/sandbox/oclif-command-adapters.test.ts b/src/commands/sandbox/oclif-command-adapters.test.ts index 6ab0d1829f2..d2197a7d4d9 100644 --- a/src/commands/sandbox/oclif-command-adapters.test.ts +++ b/src/commands/sandbox/oclif-command-adapters.test.ts @@ -90,7 +90,8 @@ vi.mock("../../lib/sandbox/config", () => ({ SandboxConfigError: mocks.SandboxConfigError, })); -vi.mock("../../lib/actions/sandbox/doctor", () => ({ +vi.mock("../../lib/actions/sandbox/doctor", async (importOriginal) => ({ + ...(await importOriginal()), runSandboxDoctor: mocks.runSandboxDoctor, })); From 36349c3147cc6d18b731a9f2756968d4c4f5ef4b Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Fri, 4 Sep 2026 09:43:41 -0700 Subject: [PATCH 11/11] test(cli): cover pending doctor migration intent Signed-off-by: Aaron Erickson --- src/lib/state/legacy-port-migration.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib/state/legacy-port-migration.test.ts b/src/lib/state/legacy-port-migration.test.ts index 5c1b92df88e..63733dcef1c 100644 --- a/src/lib/state/legacy-port-migration.test.ts +++ b/src/lib/state/legacy-port-migration.test.ts @@ -361,7 +361,12 @@ describe("legacy non-default gateway state migration", () => { path.join(selected, "retained-sandbox-recovery.json"), ).map((record) => record.sandboxName), ).toEqual(["port-box"]); - expect(fs.existsSync(path.join(shared, ".gateway-state-migration"))).toBe(true); + const pendingIntent = path.join(shared, ".gateway-state-migration", "intent.json"); + const pendingIntentBefore = fs.readFileSync(pendingIntent, "utf8"); + expect({ + recognized: hasMigratableLegacySandbox("port-box", { home, gatewayPort: 9123 }), + unchanged: fs.readFileSync(pendingIntent, "utf8") === pendingIntentBefore, + }).toEqual({ recognized: true, unchanged: true }); expect(() => migrateLegacyPortState({ home, gatewayPort: 8080 })).toThrow( /recoverable migration for gateway port 9123 is pending/, );