Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 20 additions & 75 deletions src/lib/actions/sandbox/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// SPDX-License-Identifier: Apache-2.0


import { execFileSync, spawnSync } from "node:child_process";
import { spawnSync } from "node:child_process";
import fs from "node:fs";
import path from "node:path";

import { CLI_DISPLAY_NAME, CLI_NAME } from "../../cli/branding";
import { isErrnoException } from "../../core/errno";
import { recoverNamedGatewayRuntime } from "../../gateway-runtime-action";
import { readCloudflaredState } from "../../tunnel/services";
import { probeProviderHealth, type ProviderHealthStatus } from "../../inference/health";
import { probeSandboxInferenceGatewayHealth } from "./process-recovery";
import { parseGatewayInference } from "../../inference/config";
Expand Down Expand Up @@ -259,7 +259,7 @@ function stoppedCloudflaredCheck(): DoctorCheck {
label: "cloudflared",
status: "info",
detail: "stopped",
hint: `start when needed with \`${CLI_NAME} tunnel start\``,
hint: `no cloudflared process; run \`${CLI_NAME} tunnel start\` to start it`,
};
}

Expand All @@ -269,7 +269,7 @@ function staleCloudflaredPidFileCheck(): DoctorCheck {
label: "cloudflared",
status: "warn",
detail: "stale PID file",
hint: `run \`${CLI_NAME} tunnel stop\` and start it again if you need a public tunnel`,
hint: `no cloudflared process (stored PID is invalid); run \`${CLI_NAME} tunnel start\` to restart it`,
};
}

Expand All @@ -279,81 +279,26 @@ function staleCloudflaredPidCheck(pid: number): DoctorCheck {
label: "cloudflared",
status: "warn",
detail: `stale PID ${pid}`,
hint: `run \`${CLI_NAME} tunnel stop\` to clean up the service state`,
hint: `no cloudflared process (PID ${pid} is dead or not cloudflared); run \`${CLI_NAME} tunnel start\` to restart it`,
};
}

function readCloudflaredPidFile(pidFile: string): string | null {
try {
return fs.readFileSync(pidFile, "utf-8").trim();
} catch (error) {
if (isErrnoException(error) && error.code === "ENOENT") {
return null;
}
throw error;
}
}

function commandLineNamesCloudflared(commandLine: string): boolean {
return commandLine
.split(/\0|\s+/)
.filter(Boolean)
.some((token) => path.basename(token) === "cloudflared");
}

function readProcessCommandLine(pid: number): string | null {
if (process.platform === "win32") {
return null;
}
try {
return fs.readFileSync(`/proc/${pid}/cmdline`, "utf-8");
} catch {
try {
return execFileSync("ps", ["-p", String(pid), "-o", "comm=", "-o", "args="], {
encoding: "utf-8",
stdio: ["ignore", "pipe", "ignore"],
timeout: 1000,
});
} catch {
return null;
}
}
}

function isCloudflaredProcess(pid: number): boolean {
const commandLine = readProcessCommandLine(pid);
if (commandLine === null) {
return false;
}
return commandLineNamesCloudflared(commandLine);
}

function cloudflaredDoctorCheck(sandboxName: string): DoctorCheck {
const pidFile = path.join(`/tmp/nemoclaw-services-${sandboxName}`, "cloudflared.pid");
if (!fs.existsSync(pidFile)) {
return stoppedCloudflaredCheck();
}
const rawPid = readCloudflaredPidFile(pidFile);
if (rawPid === null) {
return stoppedCloudflaredCheck();
}
const pid = Number(rawPid);
if (!Number.isFinite(pid) || pid <= 0) {
return staleCloudflaredPidFileCheck();
}
try {
process.kill(pid, 0);
if (!isCloudflaredProcess(pid)) {
return staleCloudflaredPidCheck(pid);
}
return {
group: "Local services",
label: "cloudflared",
status: "ok",
detail: `running (PID ${pid})`,
};
} catch {
return staleCloudflaredPidCheck(pid);
const state = readCloudflaredState(path.join("/tmp", `nemoclaw-services-${sandboxName}`));
switch (state.kind) {
case "stopped":
return stoppedCloudflaredCheck();
case "stale-pid-file":
return staleCloudflaredPidFileCheck();
case "stale-pid-process":
return staleCloudflaredPidCheck(state.pid);
case "running":
return {
group: "Local services",
label: "cloudflared",
status: "ok",
detail: `running (PID ${state.pid})`,
};
}
}

Expand Down
110 changes: 110 additions & 0 deletions src/lib/inventory/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,116 @@ describe("inventory commands", () => {
expect(lines).toContain(" (onboarded: unknown)");
});

// #2604: bare `nemoclaw status` previously only showed the model in parens
// and didn't label provider or connection state. Users had to run the
// per-sandbox `nemoclaw <name> status` to see those fields.
it("emits an Inference line with provider / model under each sandbox row (#2604)", () => {
const lines: string[] = [];
showStatusCommand({
listSandboxes: () => ({
sandboxes: [
{
name: "alpha",
model: "nvidia/nemotron-3-super-120b-a12b",
provider: "nvidia-prod",
},
{ name: "beta", model: "qwen2.5:7b", provider: "ollama-local" },
],
defaultSandbox: "alpha",
}),
getLiveInference: () => null,
showServiceStatus: vi.fn(),
log: (message = "") => lines.push(message),
});

expect(lines).toContain(" Inference: nvidia-prod / nvidia/nemotron-3-super-120b-a12b");
expect(lines).toContain(" Inference: ollama-local / qwen2.5:7b");
});

it("prefers live gateway provider for the default sandbox in the Inference line (#2604)", () => {
const lines: string[] = [];
showStatusCommand({
listSandboxes: () => ({
sandboxes: [
{ name: "alpha", model: "stored-model", provider: "stored-provider" },
],
defaultSandbox: "alpha",
}),
getLiveInference: () => ({ provider: "live-provider", model: "live-model" }),
showServiceStatus: vi.fn(),
log: (message = "") => lines.push(message),
});

expect(lines).toContain(" Inference: live-provider / live-model");
});

it("emits a Connected line per sandbox when getActiveSessionCount is provided (#2604)", () => {
const lines: string[] = [];
showStatusCommand({
listSandboxes: () => ({
sandboxes: [
{ name: "alpha", model: "m" },
{ name: "beta", model: "m" },
],
defaultSandbox: "alpha",
}),
getLiveInference: () => null,
getActiveSessionCount: (name) => (name === "alpha" ? 2 : 0),
showServiceStatus: vi.fn(),
log: (message = "") => lines.push(message),
});

expect(lines).toContain(" Connected: yes (2 sessions)");
expect(lines).toContain(" Connected: no");
});

it("renders `1 session` (singular) when the active count is exactly one (#2604)", () => {
const lines: string[] = [];
showStatusCommand({
listSandboxes: () => ({
sandboxes: [{ name: "alpha", model: "m" }],
defaultSandbox: "alpha",
}),
getLiveInference: () => null,
getActiveSessionCount: () => 1,
showServiceStatus: vi.fn(),
log: (message = "") => lines.push(message),
});

expect(lines).toContain(" Connected: yes (1 session)");
});

it("omits the Connected line when getActiveSessionCount returns null (probe unavailable)", () => {
const lines: string[] = [];
showStatusCommand({
listSandboxes: () => ({
sandboxes: [{ name: "alpha", model: "m" }],
defaultSandbox: "alpha",
}),
getLiveInference: () => null,
getActiveSessionCount: () => null,
showServiceStatus: vi.fn(),
log: (message = "") => lines.push(message),
});

expect(lines.some((l) => l.includes("Connected:"))).toBe(false);
});

it("omits the Connected line when the dep is not wired", () => {
const lines: string[] = [];
showStatusCommand({
listSandboxes: () => ({
sandboxes: [{ name: "alpha", model: "m" }],
defaultSandbox: "alpha",
}),
getLiveInference: () => null,
showServiceStatus: vi.fn(),
log: (message = "") => lines.push(message),
});

expect(lines.some((l) => l.includes("Connected:"))).toBe(false);
});

it("emits a gateway-down diagnostic and sets process.exitCode when the gateway is unhealthy (#3386)", () => {
const previousExitCode = process.exitCode;
process.exitCode = 0;
Expand Down
26 changes: 26 additions & 0 deletions src/lib/inventory/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ export interface ShowStatusCommandDeps {
getLiveInference: () => GatewayInference | null;
showServiceStatus: (options: { sandboxName?: string }) => void;
getServiceStatuses?: (options: { sandboxName?: string }) => StatusServiceRow[];
/**
* Active SSH-session count for a sandbox. When provided, `showStatusCommand`
* emits a `Connected:` line under each sandbox row. Returns null when the
* probe is not available (e.g. no openshell binary); the line is omitted in
* that case. #2604.
*/
getActiveSessionCount?: (sandboxName: string) => number | null;
/**
* Report whether the named NemoClaw gateway is reachable. When omitted,
* `showStatusCommand` keeps its legacy 0-exit behaviour; when provided and
Expand Down Expand Up @@ -402,12 +409,31 @@ export function showStatusCommand(deps: ShowStatusCommandDeps): void {
// Prefer the live gateway model for the default sandbox so `status`
// agrees with `openshell inference get` (#2369).
const liveModel = isDefault && live ? live.model : null;
const liveProvider = isDefault && live ? live.provider : null;
const model = liveModel || sb.model;
const provider = liveProvider || sb.provider;
const portSuffix = sb.dashboardPort != null ? ` :${sb.dashboardPort}` : "";
log(` ${sb.name}${def}${model ? ` (${model})` : ""}${portSuffix}`);
if (isDefault && liveModel && liveModel !== sb.model) {
log(` (onboarded: ${sb.model || "unknown"})`);
}
// #2604: surface the configured Inference (provider/model) and
// Connected (active-session count) as labeled fields. Bare
// `nemoclaw status` previously only had the model in parens above —
// users had to run `nemoclaw <name> status` to see provider and
// connection state.
if (provider || model) {
const parts = [provider, model].filter(Boolean).join(" / ");
log(` Inference: ${parts}`);
Comment on lines +425 to +427

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve provider / model structure in Inference: output.

When only one field exists, the current join emits an ambiguous single value. Keep the fixed two-part format so the output remains consistent and parseable.

Proposed fix
-      if (provider || model) {
-        const parts = [provider, model].filter(Boolean).join(" / ");
-        log(`      Inference: ${parts}`);
-      }
+      if (provider || model) {
+        log(`      Inference: ${provider || "unknown"} / ${model || "unknown"}`);
+      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (provider || model) {
const parts = [provider, model].filter(Boolean).join(" / ");
log(` Inference: ${parts}`);
if (provider || model) {
log(` Inference: ${provider || "unknown"} / ${model || "unknown"}`);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/inventory/index.ts` around lines 425 - 427, The Inference log
currently builds parts via [provider, model].filter(Boolean).join(" / "), which
collapses to a single ambiguous value when one side is missing; change the
construction so it always emits two parts separated by " / " (e.g., `${provider
?? ""} / ${model ?? ""}` or equivalent) and pass that string to log(`     
Inference: ${parts}`) so the output consistently preserves the "provider /
model" structure even when one side is empty.

}
if (deps.getActiveSessionCount) {
const count = deps.getActiveSessionCount(sb.name);
if (count !== null) {
log(
` Connected: ${count > 0 ? `yes (${count} session${count > 1 ? "s" : ""})` : "no"}`,
);
}
}
}
log("");
}
Expand Down
28 changes: 28 additions & 0 deletions src/lib/status-command-deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { captureOpenshellCommand, stripAnsi } from "./adapters/openshell/client"
import { OPENSHELL_PROBE_TIMEOUT_MS } from "./adapters/openshell/timeouts";
import * as registry from "./state/registry";
import { resolveOpenshell } from "./adapters/openshell/resolve";
import { createSystemDeps, parseSshProcesses } from "./state/sandbox-session";
import { getServiceStatuses, showStatus as showServiceStatus } from "./tunnel/services";

function captureOpenshell(
Expand Down Expand Up @@ -158,6 +159,22 @@ function probeGatewayHealth(): GatewayHealth {
}

export function buildStatusCommandDeps(rootDir: string): ShowStatusCommandDeps {
const opsBin = resolveOpenshell();
const sessionDeps = opsBin ? createSystemDeps(opsBin) : null;
// Cache the SSH process probe once per command invocation — avoids
// spawning ps per sandbox row. #2604; mirrors buildListCommandDeps.
let cachedSshOutput: string | null | undefined;
const getCachedSshOutput = (): string | null => {
if (cachedSshOutput === undefined && sessionDeps) {
try {
cachedSshOutput = sessionDeps.getSshProcesses();
} catch {
cachedSshOutput = null;
}
}
return cachedSshOutput ?? null;
};

return {
listSandboxes: () => registry.listSandboxes(),
getLiveInference: () =>
Expand All @@ -171,6 +188,17 @@ export function buildStatusCommandDeps(rootDir: string): ShowStatusCommandDeps {
showServiceStatus,
getServiceStatuses,
getGatewayHealth: probeGatewayHealth,
getActiveSessionCount: sessionDeps
? (name) => {
try {
const sshOutput = getCachedSshOutput();
if (sshOutput === null) return null;
return parseSshProcesses(sshOutput, name).length;
} catch {
return null;
}
}
: undefined,
checkMessagingBridgeHealth: (sandboxName, channels) =>
checkMessagingBridgeHealth(rootDir, sandboxName, channels),
backfillAndFindOverlaps: () => backfillAndFindOverlaps(rootDir),
Expand Down
Loading
Loading