From 665ce6301738b946a1f8b1cd7f208d1bd2b86351 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 14 May 2026 13:01:02 -0400 Subject: [PATCH 1/2] fix(status): explain cloudflared-stopped reason and surface Connected/Inference fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nemoclaw status` previously printed `● cloudflared (stopped)` in three distinct failure modes (no PID file, garbage PID, dead/wrong-process PID) with no cause and no remediation — exactly the symptom #2604 reported. The doctor already distinguished the three modes; the status renderer just never picked up the same logic. Extract the shared check into a new `readCloudflaredState(pidDir)` in src/lib/tunnel/services.ts that returns a discriminated union, and have both `showStatus()` and the doctor's `cloudflaredDoctorCheck` consume it. showStatus now emits a yellow/red marker plus a one-line remediation: ● cloudflared (stopped) start when needed with `nemoclaw tunnel start` ● cloudflared (stale PID file) run `nemoclaw tunnel stop` and start it again if you need a public tunnel ● cloudflared (stale PID 999999999) run `nemoclaw tunnel stop` to clean up the service state Bare `nemoclaw status` also surfaces the configured Inference (provider / model) and Connected (active-session count) as labeled fields under each sandbox row, matching what was previously only available via the per- sandbox `nemoclaw status`. `getActiveSessionCount` is wired through `buildStatusCommandDeps`, mirroring the cached SSH-process probe already used by `buildListCommandDeps`. Fixes #2604 Signed-off-by: Prekshi Vyas --- src/lib/actions/sandbox/doctor.ts | 89 +++++------------------- src/lib/inventory/index.test.ts | 110 ++++++++++++++++++++++++++++++ src/lib/inventory/index.ts | 26 +++++++ src/lib/status-command-deps.ts | 28 ++++++++ src/lib/tunnel/services.test.ts | 83 +++++++++++++++++++++- src/lib/tunnel/services.ts | 97 +++++++++++++++++++++++--- 6 files changed, 349 insertions(+), 84 deletions(-) diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index 7f9dc51fcd6..af576106381 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -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"; @@ -283,77 +283,22 @@ function staleCloudflaredPidCheck(pid: number): DoctorCheck { }; } -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})`, + }; } } diff --git a/src/lib/inventory/index.test.ts b/src/lib/inventory/index.test.ts index ad45c4cf98d..a6702586291 100644 --- a/src/lib/inventory/index.test.ts +++ b/src/lib/inventory/index.test.ts @@ -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 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; diff --git a/src/lib/inventory/index.ts b/src/lib/inventory/index.ts index ad7b3457c3c..a8a45cf5eae 100644 --- a/src/lib/inventory/index.ts +++ b/src/lib/inventory/index.ts @@ -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 @@ -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 status` to see provider and + // connection state. + if (provider || model) { + const parts = [provider, model].filter(Boolean).join(" / "); + log(` Inference: ${parts}`); + } + if (deps.getActiveSessionCount) { + const count = deps.getActiveSessionCount(sb.name); + if (count !== null) { + log( + ` Connected: ${count > 0 ? `yes (${count} session${count > 1 ? "s" : ""})` : "no"}`, + ); + } + } } log(""); } diff --git a/src/lib/status-command-deps.ts b/src/lib/status-command-deps.ts index c7b7798ce68..dcb20832cf3 100644 --- a/src/lib/status-command-deps.ts +++ b/src/lib/status-command-deps.ts @@ -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( @@ -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: () => @@ -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), diff --git a/src/lib/tunnel/services.test.ts b/src/lib/tunnel/services.test.ts index bb0ec2d5ae2..baee1bc29a5 100644 --- a/src/lib/tunnel/services.test.ts +++ b/src/lib/tunnel/services.test.ts @@ -8,7 +8,12 @@ import { join, resolve } from "node:path"; import { tmpdir } from "node:os"; // Import from compiled dist/ so coverage is attributed correctly. -import { getServiceStatuses, showStatus, stopAll } from "../../../dist/lib/tunnel/services"; +import { + getServiceStatuses, + readCloudflaredState, + showStatus, + stopAll, +} from "../../../dist/lib/tunnel/services"; const ollamaProxyDistPath = resolve( import.meta.dirname, @@ -124,6 +129,82 @@ describe("showStatus", () => { expect(output).not.toContain("Public URL"); logSpy.mockRestore(); }); + + // #2604: stopped/stale modes must each emit a recovery hint. The old + // showStatus printed bare "(stopped)" in all three failure modes with no + // context, which is the user-facing symptom of the bug. + it("prints `tunnel start` remediation when the PID file is missing (stopped)", () => { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + showStatus({ pidDir }); + const output = logSpy.mock.calls.map((c) => c[0]).join("\n"); + expect(output).toContain("(stopped)"); + expect(output).toContain("nemoclaw tunnel start"); + logSpy.mockRestore(); + }); + + it("prints `tunnel stop` remediation when the PID file holds garbage (stale-pid-file)", () => { + writeFileSync(join(pidDir, "cloudflared.pid"), "not-a-number"); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + showStatus({ pidDir }); + const output = logSpy.mock.calls.map((c) => c[0]).join("\n"); + expect(output).toContain("(stale PID file)"); + expect(output).toContain("nemoclaw tunnel stop"); + logSpy.mockRestore(); + }); + + it("prints `tunnel stop` remediation when the PID points at a dead process (stale-pid-process)", () => { + writeFileSync(join(pidDir, "cloudflared.pid"), "999999999"); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + showStatus({ pidDir }); + const output = logSpy.mock.calls.map((c) => c[0]).join("\n"); + expect(output).toContain("(stale PID 999999999)"); + expect(output).toContain("nemoclaw tunnel stop"); + logSpy.mockRestore(); + }); +}); + +// #2604: readCloudflaredState is the shared source of truth used by both +// showStatus and the doctor's cloudflared check. Tests below exercise each +// branch of the discriminated union. +describe("readCloudflaredState", () => { + let pidDir: string; + + beforeEach(() => { + pidDir = mkdtempSync(join(tmpdir(), "nemoclaw-svc-state-test-")); + }); + + afterEach(() => { + rmSync(pidDir, { recursive: true, force: true }); + }); + + it("returns stopped when no PID file exists", () => { + expect(readCloudflaredState(pidDir)).toEqual({ kind: "stopped" }); + }); + + it("returns stopped when the PID file is empty", () => { + writeFileSync(join(pidDir, "cloudflared.pid"), ""); + expect(readCloudflaredState(pidDir)).toEqual({ kind: "stopped" }); + }); + + it("returns stale-pid-file when contents are not parseable as a positive integer", () => { + writeFileSync(join(pidDir, "cloudflared.pid"), "not-a-number"); + expect(readCloudflaredState(pidDir)).toEqual({ kind: "stale-pid-file" }); + }); + + it("returns stale-pid-process when the PID is dead (kernel ESRCH)", () => { + // PID > max(int32) is virtually guaranteed dead on macOS/Linux. + writeFileSync(join(pidDir, "cloudflared.pid"), "999999999"); + const state = readCloudflaredState(pidDir); + expect(state.kind).toBe("stale-pid-process"); + if (state.kind === "stale-pid-process") expect(state.pid).toBe(999999999); + }); + + it("returns stale-pid-process when the PID points at a different process", () => { + // Use this test process's own PID — guaranteed alive, but not cloudflared. + writeFileSync(join(pidDir, "cloudflared.pid"), String(process.pid)); + const state = readCloudflaredState(pidDir); + expect(state.kind).toBe("stale-pid-process"); + }); }); describe("stopAll", () => { diff --git a/src/lib/tunnel/services.ts b/src/lib/tunnel/services.ts index 04f4c0438eb..f25a3e2048a 100644 --- a/src/lib/tunnel/services.ts +++ b/src/lib/tunnel/services.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execSync, spawn, spawnSync } from "node:child_process"; +import { execFileSync, execSync, spawn, spawnSync } from "node:child_process"; import { chmodSync, closeSync, @@ -13,9 +13,9 @@ import { writeFileSync, unlinkSync, } from "node:fs"; -import { join } from "node:path"; +import { basename, join } from "node:path"; -import { AGENT_PRODUCT_NAME, CLI_DISPLAY_NAME } from "../cli/branding"; +import { AGENT_PRODUCT_NAME, CLI_DISPLAY_NAME, CLI_NAME } from "../cli/branding"; import { renderBox } from "../cli/banner"; import { dockerSpawnSync } from "../adapters/docker"; import { DASHBOARD_PORT } from "../core/ports"; @@ -95,6 +95,66 @@ function isRunning(pidDir: string, name: string): boolean { return isAlive(pid); } +// --------------------------------------------------------------------------- +// Cloudflared state — finer-grained than isRunning() so callers (status, +// doctor) can distinguish stopped / stale-pid-file / stale-pid-process and +// emit a targeted remediation. Issue #2604. +// --------------------------------------------------------------------------- + +export type CloudflaredState = + | { kind: "running"; pid: number } + | { kind: "stopped" } + | { kind: "stale-pid-file" } + | { kind: "stale-pid-process"; pid: number }; + +function readProcessCommandLine(pid: number): string | null { + if (process.platform === "win32") return null; + try { + return 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 commandLineNamesCloudflared(commandLine: string): boolean { + return commandLine + .split(/\0|\s+/) + .filter(Boolean) + .some((token) => basename(token) === "cloudflared"); +} + +export function readCloudflaredState(pidDir: string): CloudflaredState { + const pidFile = join(pidDir, "cloudflared.pid"); + if (!existsSync(pidFile)) return { kind: "stopped" }; + let raw: string; + try { + raw = readFileSync(pidFile, "utf-8").trim(); + } catch { + return { kind: "stopped" }; + } + if (raw.length === 0) return { kind: "stopped" }; + const pid = Number(raw); + if (!Number.isFinite(pid) || pid <= 0) return { kind: "stale-pid-file" }; + try { + process.kill(pid, 0); + } catch { + return { kind: "stale-pid-process", pid }; + } + const cmdline = readProcessCommandLine(pid); + if (cmdline !== null && !commandLineNamesCloudflared(cmdline)) { + return { kind: "stale-pid-process", pid }; + } + return { kind: "running", pid }; +} + function writePid(pidDir: string, name: string, pid: number): void { writeFileSync(join(pidDir, `${name}.pid`), String(pid)); } @@ -227,19 +287,34 @@ export function showStatus(opts: ServiceOptions = {}): void { ensurePidDir(pidDir); console.log(""); - for (const svc of SERVICE_NAMES) { - if (isRunning(pidDir, svc)) { - const pid = readPid(pidDir, svc); - console.log(` ${GREEN}●${NC} ${svc} (PID ${String(pid)})`); - } else { - console.log(` ${RED}●${NC} ${svc} (stopped)`); - } + const state = readCloudflaredState(pidDir); + // #2604: distinguish stopped / stale-pid-file / stale-pid-process and + // surface the matching remediation. The previous "(stopped)" line was + // emitted in all three failure modes with no recovery hint. + switch (state.kind) { + case "running": + console.log(` ${GREEN}●${NC} cloudflared (PID ${String(state.pid)})`); + break; + case "stopped": + console.log(` ${RED}●${NC} cloudflared (stopped)`); + console.log(` start when needed with \`${CLI_NAME} tunnel start\``); + break; + case "stale-pid-file": + console.log(` ${YELLOW}●${NC} cloudflared (stale PID file)`); + console.log( + ` run \`${CLI_NAME} tunnel stop\` and start it again if you need a public tunnel`, + ); + break; + case "stale-pid-process": + console.log(` ${YELLOW}●${NC} cloudflared (stale PID ${String(state.pid)})`); + console.log(` run \`${CLI_NAME} tunnel stop\` to clean up the service state`); + break; } console.log(""); // Only show tunnel URL if cloudflared is actually running const logFile = join(pidDir, "cloudflared.log"); - if (isRunning(pidDir, "cloudflared") && existsSync(logFile)) { + if (state.kind === "running" && existsSync(logFile)) { const log = readFileSync(logFile, "utf-8"); const match = /https:\/\/[a-z0-9-]*\.trycloudflare\.com/.exec(log); if (match) { From f1d8833b3bf697178ea6d5c68f147253f69094ea Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Thu, 14 May 2026 14:36:19 -0400 Subject: [PATCH 2/2] fix(status): reword cloudflared remediation as 'no cloudflared process; run tunnel start to restart' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Initial PR copied doctor's existing hint wording, which says "run `nemoclaw tunnel stop`" for stale-pid states. In the bare `nemoclaw status` context after `pkill cloudflared`, the user wants the tunnel back — they don't want a "stop" command for something already not running. Reviewer comments on #2604 (wangericnv 2026-05-11, cv 2026- 05-14) both expected the shape `no cloudflared process; restart with ...` — a cause phrase plus a single-command recovery. Reword all three failure modes to that shape and point at `nemoclaw tunnel start` (which already handles stale PID files — `isRunning()` returns false, `startService()` proceeds and overwrites the file). Apply the same wording in doctor.ts so the two diagnostic paths stay consistent. Update services.test.ts assertions to lock in the new phrasing; doctor JSON tests in cli.test.ts only assert status/detail and remain unaffected. Signed-off-by: Prekshi Vyas --- src/lib/actions/sandbox/doctor.ts | 6 +++--- src/lib/tunnel/services.test.ts | 20 +++++++++++++------- src/lib/tunnel/services.ts | 8 +++++--- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index af576106381..c97785fdb4d 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -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`, }; } @@ -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`, }; } @@ -279,7 +279,7 @@ 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`, }; } diff --git a/src/lib/tunnel/services.test.ts b/src/lib/tunnel/services.test.ts index baee1bc29a5..5c937cd2e23 100644 --- a/src/lib/tunnel/services.test.ts +++ b/src/lib/tunnel/services.test.ts @@ -130,35 +130,41 @@ describe("showStatus", () => { logSpy.mockRestore(); }); - // #2604: stopped/stale modes must each emit a recovery hint. The old - // showStatus printed bare "(stopped)" in all three failure modes with no - // context, which is the user-facing symptom of the bug. + // #2604: wangericnv and Carlos (issue comments 2026-05-11, 2026-05-14) both + // asked for a "no cloudflared process; restart with ..." shape — a cause + // phrase plus a single-command recovery. All three failure modes surface + // "no cloudflared process" and point at `nemoclaw tunnel start`, which + // overwrites a stale PID file when isRunning() is false (see startService). it("prints `tunnel start` remediation when the PID file is missing (stopped)", () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); showStatus({ pidDir }); const output = logSpy.mock.calls.map((c) => c[0]).join("\n"); expect(output).toContain("(stopped)"); + expect(output).toContain("no cloudflared process"); expect(output).toContain("nemoclaw tunnel start"); logSpy.mockRestore(); }); - it("prints `tunnel stop` remediation when the PID file holds garbage (stale-pid-file)", () => { + it("prints `tunnel start` remediation when the PID file holds garbage (stale-pid-file)", () => { writeFileSync(join(pidDir, "cloudflared.pid"), "not-a-number"); const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); showStatus({ pidDir }); const output = logSpy.mock.calls.map((c) => c[0]).join("\n"); expect(output).toContain("(stale PID file)"); - expect(output).toContain("nemoclaw tunnel stop"); + expect(output).toContain("no cloudflared process"); + expect(output).toContain("nemoclaw tunnel start"); logSpy.mockRestore(); }); - it("prints `tunnel stop` remediation when the PID points at a dead process (stale-pid-process)", () => { + it("prints `tunnel start` remediation when the PID points at a dead process (stale-pid-process)", () => { writeFileSync(join(pidDir, "cloudflared.pid"), "999999999"); const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); showStatus({ pidDir }); const output = logSpy.mock.calls.map((c) => c[0]).join("\n"); expect(output).toContain("(stale PID 999999999)"); - expect(output).toContain("nemoclaw tunnel stop"); + expect(output).toContain("no cloudflared process"); + expect(output).toContain("PID 999999999 is dead or not cloudflared"); + expect(output).toContain("nemoclaw tunnel start"); logSpy.mockRestore(); }); }); diff --git a/src/lib/tunnel/services.ts b/src/lib/tunnel/services.ts index f25a3e2048a..fad391b781c 100644 --- a/src/lib/tunnel/services.ts +++ b/src/lib/tunnel/services.ts @@ -297,17 +297,19 @@ export function showStatus(opts: ServiceOptions = {}): void { break; case "stopped": console.log(` ${RED}●${NC} cloudflared (stopped)`); - console.log(` start when needed with \`${CLI_NAME} tunnel start\``); + console.log(` no cloudflared process; run \`${CLI_NAME} tunnel start\` to start it`); break; case "stale-pid-file": console.log(` ${YELLOW}●${NC} cloudflared (stale PID file)`); console.log( - ` run \`${CLI_NAME} tunnel stop\` and start it again if you need a public tunnel`, + ` no cloudflared process (stored PID is invalid); run \`${CLI_NAME} tunnel start\` to restart it`, ); break; case "stale-pid-process": console.log(` ${YELLOW}●${NC} cloudflared (stale PID ${String(state.pid)})`); - console.log(` run \`${CLI_NAME} tunnel stop\` to clean up the service state`); + console.log( + ` no cloudflared process (PID ${String(state.pid)} is dead or not cloudflared); run \`${CLI_NAME} tunnel start\` to restart it`, + ); break; } console.log("");