diff --git a/scripts/checks/vitest-project-overlap.mts b/scripts/checks/vitest-project-overlap.mts index 340400860d6..40857afc416 100644 --- a/scripts/checks/vitest-project-overlap.mts +++ b/scripts/checks/vitest-project-overlap.mts @@ -43,6 +43,7 @@ const INSTALLER_INTEGRATION_TESTS = new Set([ "test/install-clone-ref.test.ts", "test/install-express-prompt.test.ts", "test/install-express-wsl-ollama.test.ts", + "test/install-forward-restore-diagnostics.test.ts", "test/install-hermes-forward-restore.test.ts", "test/install-managed-cli-reuse.test.ts", "test/install-openshell-version-pin.test.ts", diff --git a/scripts/install.sh b/scripts/install.sh index 5360c02e66a..47ad79c89f1 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -505,7 +505,7 @@ resolve_hermes_api_port() { } restore_onboard_forward_after_post_checks() { - local sandbox_name agent_name agent_display port openshell_bin openshell_dir attempt selected_state_dir state_dir pid_file watcher_script watcher_pid + local sandbox_name agent_name agent_display port openshell_bin openshell_dir attempt selected_state_dir state_dir pid_file watcher_script watcher_pid start_diagnostic diagnostic_file sandbox_name="$(resolve_default_sandbox_name)" agent_name="$(resolve_onboarded_agent)" agent_display="$(agent_display_name "$agent_name")" @@ -557,6 +557,37 @@ restore_onboard_forward_after_post_checks() { rm -f "$pid_file" fi + redact_forward_start_diagnostic() { + local redactor + redactor="$(resolve_repo_root)/dist/lib/security/redact.js" + if command_exists node && [[ -f "$redactor" ]] \ + && node -e ' + const fs = require("fs"); + const { redactFull } = require(process.argv[1]); + process.stdout.write(redactFull(fs.readFileSync(0, "utf8"))); + ' "$redactor" 2>/dev/null; then + return 0 + fi + printf "" + } + + sanitize_forward_start_diagnostic() { + if command_exists node && node -e ' + const fs = require("fs"); + const encoded = fs.readFileSync(0).toString("latin1") + .replace(/\x1B\][\s\S]*?(?:\x07|\x1B\\|$)/g, "") + .replace(/\x9D[\s\S]*?(?:\x07|\x1B\\|\x9C|$)/g, "") + .replace(/(?:\x1B\[|\x9B)[0-?]*[ -/]*[@-~]/g, "") + .replace(/\x1B[@-_]/g, ""); + const diagnostic = Buffer.from(encoded, "latin1").toString("utf8") + .replace(/[\u0000-\u001F\u007F-\u009F]/g, " "); + process.stdout.write(diagnostic); + ' 2>/dev/null; then + return 0 + fi + printf "" + } + stop_agent_forward_if_owned() { local forward_list owner status "$openshell_bin" forward stop "$port" "$sandbox_name" >/dev/null 2>&1 && return 0 @@ -578,12 +609,36 @@ restore_onboard_forward_after_post_checks() { fi } + start_diagnostic="" + diagnostic_file="$(mktemp "${TMPDIR:-/tmp}/nemoclaw-forward-start-XXXXXX" 2>/dev/null)" \ + || diagnostic_file="" + for attempt in 1 2 3; do stop_agent_forward_if_owned if [ "$attempt" -gt 1 ]; then sleep 2 fi - "$openshell_bin" forward start --background "$port" "$sandbox_name" >/dev/null 2>&1 || true + if [[ -n "$diagnostic_file" ]]; then + "$openshell_bin" forward start --background "$port" "$sandbox_name" \ + >"$diagnostic_file" 2>&1 || true + start_diagnostic="$(sanitize_forward_start_diagnostic <"$diagnostic_file" | awk ' + { + gsub(/[[:space:]]+/, " ") + sub(/^ /, "") + sub(/ $/, "") + if ($0 != "") joined = (joined == "" ? $0 : joined "; " $0) + } + END { printf "%s", joined } + ' 2>/dev/null || true)" + start_diagnostic="$( + printf "%s" "$start_diagnostic" | redact_forward_start_diagnostic + )" + if [[ "${#start_diagnostic}" -gt 300 ]]; then + start_diagnostic="${start_diagnostic:0:300} [truncated]" + fi + else + "$openshell_bin" forward start --background "$port" "$sandbox_name" >/dev/null 2>&1 || true + fi watcher_pid="" if [[ "${NEMOCLAW_SKIP_FORWARD_WATCHER:-}" != "1" ]] && command_exists node; then watcher_script="${pid_file}.js" @@ -598,10 +653,27 @@ function healthy() { stdio: "ignore", }).status === 0; } +function listedStatus() { + const listed = spawnSync(openshellBin, ["forward", "list"], { encoding: "utf-8" }); + if (listed.status !== 0 || typeof listed.stdout !== "string") return null; + for (const line of listed.stdout.split("\n")) { + const columns = line.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, "").trim().split(/\s+/); + if (columns[0] === sandboxName && columns[2] === port) { + return (columns[4] || "").toLowerCase(); + } + } + return ""; +} function tick() { if (healthy()) return; - run(["forward", "stop", port, sandboxName]); - run(["forward", "start", "--background", port, sandboxName]); + const status = listedStatus(); + if (status === null) return; + if (status === "dead") { + run(["forward", "stop", port, sandboxName]); + run(["forward", "start", "--background", port, sandboxName]); + return; + } + if (status === "") run(["forward", "start", "--background", port, sandboxName]); } tick(); setInterval(tick, 10_000); @@ -622,10 +694,12 @@ NODE sleep 4 if command_exists curl \ && curl -sf --max-time 3 "http://127.0.0.1:${port}/health" >/dev/null 2>&1; then + [[ -n "$diagnostic_file" ]] && rm -f "$diagnostic_file" return 0 fi watcher_pid="$(cat "$pid_file" 2>/dev/null || true)" if ! command_exists curl && [[ -n "$watcher_pid" ]] && kill -0 "$watcher_pid" >/dev/null 2>&1; then + [[ -n "$diagnostic_file" ]] && rm -f "$diagnostic_file" return 0 fi if [[ -n "$watcher_pid" ]]; then @@ -633,8 +707,12 @@ NODE fi rm -f "$pid_file" done + [[ -n "$diagnostic_file" ]] && rm -f "$diagnostic_file" warn "Could not restore ${agent_display} host forward on port ${port}." + if [[ -n "$start_diagnostic" ]]; then + warn "OpenShell reported: ${start_diagnostic}" + fi warn "Run: openshell forward start --background ${port} ${sandbox_name}" return 1 } diff --git a/test/install-forward-restore-diagnostics.test.ts b/test/install-forward-restore-diagnostics.test.ts new file mode 100644 index 00000000000..2f33b954306 --- /dev/null +++ b/test/install-forward-restore-diagnostics.test.ts @@ -0,0 +1,382 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +const REPOSITORY_ROOT = path.resolve(import.meta.dirname, ".."); +const INSTALLER = path.join(REPOSITORY_ROOT, "scripts", "install.sh"); +const SANDBOX = "created-by-onboard"; +const PORT = "8642"; +const LISTENER_FAILURE = [ + "ssh process started but local forward listener was not reachable", + `local forward listener did not open on 127.0.0.1:${PORT} within 10000ms`, + "last probe failed with Connection refused (os error 111)", +]; + +function writeExecutable(target: string, contents: string): void { + fs.writeFileSync(target, contents, { mode: 0o755 }); +} + +function prepareCheckout(prefix: string): { root: string; binDir: string } { + const root = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const binDir = path.join(root, "bin"); + const stateDir = path.join(root, ".nemoclaw"); + fs.mkdirSync(binDir, { recursive: true }); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "onboard-session.json"), + JSON.stringify({ sandboxName: SANDBOX, agent: "hermes" }), + ); + fs.writeFileSync( + path.join(stateDir, "sandboxes.json"), + JSON.stringify({ sandboxes: { [SANDBOX]: { hermesApiPort: Number(PORT) } } }), + ); + for (const command of ["sleep"]) { + writeExecutable(path.join(binDir, command), "#!/usr/bin/env bash\nexit 0\n"); + } + return { root, binDir }; +} + +function runRestore(root: string, binDir: string, env: Record = {}) { + return spawnSync( + "bash", + ["-c", 'source "$INSTALLER" 2>/dev/null; restore_onboard_forward_after_post_checks'], + { + cwd: REPOSITORY_ROOT, + encoding: "utf-8", + env: { + ...process.env, + HOME: root, + INSTALLER, + PATH: `${binDir}:${path.dirname(process.execPath)}:/usr/bin:/bin`, + ...env, + }, + }, + ); +} + +function findWatcherScript(root: string): string { + const [watcherScript] = fs.globSync(".nemoclaw/**/*.forward.pid.js", { cwd: root }); + expect(watcherScript).toBeDefined(); + return path.join(root, watcherScript as string); +} + +function runWatcherTick(watcherScript: string, binDir: string, openshell: string): string { + const log = `${watcherScript}.calls`; + spawnSync(process.execPath, [watcherScript, openshell, PORT, SANDBOX], { + encoding: "utf-8", + env: { ...process.env, OPENSHELL_LOG: log, PATH: `${binDir}:/usr/bin:/bin` }, + killSignal: "SIGKILL", + timeout: 4_000, + }); + return fs.existsSync(log) ? fs.readFileSync(log, "utf-8") : ""; +} + +function watcherScriptForListing( + prefix: string, + listing: string, + listExitStatus = 0, +): { + binDir: string; + openshell: string; + root: string; + watcherScript: string; +} { + const { root, binDir } = prepareCheckout(prefix); + const openshell = path.join(binDir, "openshell"); + writeExecutable( + openshell, + `#!/usr/bin/env bash +if [ -n "\${OPENSHELL_LOG:-}" ]; then printf '%s\\n' "$*" >> "$OPENSHELL_LOG"; fi +if [ "$1" = "forward" ] && [ "$2" = "list" ]; then + echo "SANDBOX BIND PORT PID STATUS" +${listing} + exit ${String(listExitStatus)} +fi +exit 0 +`, + ); + writeExecutable(path.join(binDir, "curl"), "#!/usr/bin/env bash\nexit 7\n"); + writeExecutable( + path.join(binDir, "node"), + `#!/usr/bin/env bash +if [ "\${1:-}" = "-e" ] && [[ "\${2:-}" == *"const { spawn }"* ]]; then exit 0; fi +exec ${JSON.stringify(process.execPath)} "$@" +`, + ); + const result = runRestore(root, binDir); + expect(result.status).toBe(1); + return { binDir, openshell, root, watcherScript: findWatcherScript(root) }; +} + +describe("Hermes host forward restore diagnostics", () => { + it("reports why the OpenShell forward start failed (#8884)", () => { + const { root, binDir } = prepareCheckout("nemohermes-forward-diagnostic-"); + try { + writeExecutable( + path.join(binDir, "openshell"), + `#!/usr/bin/env bash +if [ "$1" = "forward" ] && [ "$2" = "list" ]; then + echo "SANDBOX BIND PORT PID STATUS" + exit 0 +fi +if [ "$1" = "forward" ] && [ "$2" = "start" ]; then +${LISTENER_FAILURE.map((line) => ` echo ${JSON.stringify(line)} >&2`).join("\n")} + exit 1 +fi +exit 0 +`, + ); + writeExecutable(path.join(binDir, "curl"), "#!/usr/bin/env bash\nexit 7\n"); + + const result = runRestore(root, binDir, { NEMOCLAW_SKIP_FORWARD_WATCHER: "1" }); + + expect(result.status).toBe(1); + expect(result.stdout).toContain(`Could not restore Hermes host forward on port ${PORT}.`); + expect(result.stdout).toContain("OpenShell reported:"); + for (const line of LISTENER_FAILURE) { + expect(result.stdout).toContain(line); + } + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("omits the `OpenShell reported:` warning when the failed start printed no output (#8884)", () => { + const { root, binDir } = prepareCheckout("nemohermes-forward-silent-"); + try { + writeExecutable(path.join(binDir, "openshell"), "#!/usr/bin/env bash\nexit 1\n"); + writeExecutable(path.join(binDir, "curl"), "#!/usr/bin/env bash\nexit 7\n"); + + const result = runRestore(root, binDir, { NEMOCLAW_SKIP_FORWARD_WATCHER: "1" }); + + expect(result.status).toBe(1); + expect(result.stdout).toContain(`Could not restore Hermes host forward on port ${PORT}.`); + expect(result.stdout).not.toContain("OpenShell reported:"); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("redacts a credential from an OpenShell forward-start diagnostic (#8884)", () => { + const { root, binDir } = prepareCheckout("nemohermes-forward-secret-diagnostic-"); + const token = "nvapi-forward-start-secret-1234567890"; + try { + writeExecutable( + path.join(binDir, "openshell"), + `#!/usr/bin/env bash +if [ "$1" = "forward" ] && [ "$2" = "start" ]; then + echo "listener rejected token ${token}" >&2 + exit 1 +fi +exit 0 +`, + ); + writeExecutable(path.join(binDir, "curl"), "#!/usr/bin/env bash\nexit 7\n"); + + const result = runRestore(root, binDir, { NEMOCLAW_SKIP_FORWARD_WATCHER: "1" }); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("OpenShell reported: listener rejected token "); + expect(result.stdout).not.toContain(token); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("removes terminal controls from an OpenShell forward-start diagnostic (#8884)", () => { + const { root, binDir } = prepareCheckout("nemohermes-forward-control-diagnostic-"); + try { + writeExecutable( + path.join(binDir, "openshell"), + `#!/usr/bin/env bash +if [ "$1" = "forward" ] && [ "$2" = "start" ]; then + printf '\\033]0;changed title\\033\\listener \\235changed C1 title\\234rejected \\23331mdiagnostic\\2330m\\n' >&2 + exit 1 +fi +exit 0 +`, + ); + writeExecutable(path.join(binDir, "curl"), "#!/usr/bin/env bash\nexit 7\n"); + + const result = runRestore(root, binDir, { NEMOCLAW_SKIP_FORWARD_WATCHER: "1" }); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("OpenShell reported: listener rejected diagnostic"); + expect(result.stdout).not.toContain("\u001b"); + expect(result.stdout).not.toContain("\u0007"); + expect(result.stdout).not.toContain("\u009b"); + expect(result.stdout).not.toContain("\u009d"); + expect(result.stdout).not.toContain("changed title"); + expect(result.stdout).not.toContain("changed C1 title"); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("redacts the complete diagnostic when the compiled redactor is unavailable (#8884)", () => { + const { root, binDir } = prepareCheckout("nemohermes-forward-redactor-fallback-"); + const token = "nvapi-forward-fallback-secret-1234567890"; + try { + writeExecutable( + path.join(binDir, "openshell"), + `#!/usr/bin/env bash +if [ "$1" = "forward" ] && [ "$2" = "start" ]; then + echo "listener rejected token ${token}" >&2 + exit 1 +fi +exit 0 +`, + ); + writeExecutable(path.join(binDir, "curl"), "#!/usr/bin/env bash\nexit 7\n"); + + const result = runRestore(root, binDir, { + NEMOCLAW_REPO_ROOT: root, + NEMOCLAW_SKIP_FORWARD_WATCHER: "1", + }); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("OpenShell reported: "); + expect(result.stdout).not.toContain(token); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("bounds a long OpenShell forward-start diagnostic (#8884)", () => { + const { root, binDir } = prepareCheckout("nemohermes-forward-long-diagnostic-"); + const diagnostic = `listener failure: ${"a".repeat(320)} excluded suffix`; + try { + writeExecutable( + path.join(binDir, "openshell"), + `#!/usr/bin/env bash +if [ "$1" = "forward" ] && [ "$2" = "start" ]; then + echo ${JSON.stringify(diagnostic)} >&2 + exit 1 +fi +exit 0 +`, + ); + writeExecutable(path.join(binDir, "curl"), "#!/usr/bin/env bash\nexit 7\n"); + + const result = runRestore(root, binDir, { NEMOCLAW_SKIP_FORWARD_WATCHER: "1" }); + + expect(result.status).toBe(1); + expect(result.stdout).toContain( + `OpenShell reported: ${diagnostic.slice(0, 300)} [truncated]`, + ); + expect(result.stdout).not.toContain("excluded suffix"); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); +}); + +describe("Hermes host forward watcher", () => { + it.each([ + "running", + "active", + ])("does not replace a forward that OpenShell lists as %s when the health check fails (#8884)", (status) => { + const { root, binDir, openshell, watcherScript } = watcherScriptForListing( + `nemohermes-watcher-${status}-`, + ` echo "${SANDBOX} 127.0.0.1 ${PORT} 123 ${status}"`, + ); + try { + const calls = runWatcherTick(watcherScript, binDir, openshell); + + expect(calls).toContain("forward list"); + expect(calls).not.toContain(`forward stop ${PORT} ${SANDBOX}`); + expect(calls).not.toContain(`forward start --background ${PORT} ${SANDBOX}`); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it.each([ + "running", + "active", + ])("does not replace a forward that OpenShell colorizes as %s (#8884)", (status) => { + const { root, binDir, openshell, watcherScript } = watcherScriptForListing( + `nemohermes-watcher-color-${status}-`, + ` printf '${SANDBOX} 127.0.0.1 ${PORT} 123 \\033[32m${status}\\033[0m\\n'`, + ); + try { + const calls = runWatcherTick(watcherScript, binDir, openshell); + + expect(calls).toContain("forward list"); + expect(calls).not.toContain(`forward stop ${PORT} ${SANDBOX}`); + expect(calls).not.toContain(`forward start --background ${PORT} ${SANDBOX}`); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("does not change a forward when OpenShell cannot list forwards (#8884)", () => { + const { root, binDir, openshell, watcherScript } = watcherScriptForListing( + "nemohermes-watcher-unreadable-", + " :", + 1, + ); + try { + const calls = runWatcherTick(watcherScript, binDir, openshell); + + expect(calls).toContain("forward list"); + expect(calls).not.toContain(`forward stop ${PORT} ${SANDBOX}`); + expect(calls).not.toContain(`forward start --background ${PORT} ${SANDBOX}`); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("does not replace a forward with an unrecognized OpenShell status (#8884)", () => { + const { root, binDir, openshell, watcherScript } = watcherScriptForListing( + "nemohermes-watcher-pending-", + ` echo "${SANDBOX} 127.0.0.1 ${PORT} 123 pending"`, + ); + try { + const calls = runWatcherTick(watcherScript, binDir, openshell); + + expect(calls).toContain("forward list"); + expect(calls).not.toContain(`forward stop ${PORT} ${SANDBOX}`); + expect(calls).not.toContain(`forward start --background ${PORT} ${SANDBOX}`); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("does not run forward stop before starting a forward that OpenShell does not list (#8884)", () => { + const { root, binDir, openshell, watcherScript } = watcherScriptForListing( + "nemohermes-watcher-absent-", + " :", + ); + try { + const calls = runWatcherTick(watcherScript, binDir, openshell); + + expect(calls).toContain(`forward start --background ${PORT} ${SANDBOX}`); + expect(calls).not.toContain(`forward stop ${PORT} ${SANDBOX}`); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("restarts a forward OpenShell reports as dead (#8884)", () => { + const { root, binDir, openshell, watcherScript } = watcherScriptForListing( + "nemohermes-watcher-dead-", + ` echo "${SANDBOX} 127.0.0.1 ${PORT} 123 dead"`, + ); + try { + const calls = runWatcherTick(watcherScript, binDir, openshell); + + expect(calls).toContain(`forward stop ${PORT} ${SANDBOX}`); + expect(calls).toContain(`forward start --background ${PORT} ${SANDBOX}`); + expect(calls.indexOf("forward stop")).toBeLessThan(calls.indexOf("forward start")); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 9487d4ecedf..5b7f50b2bd6 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -163,6 +163,7 @@ export default defineConfig({ "test/install-station-vllm-continuation.test.ts", "test/install-build-dependency-preflight.test.ts", "test/install-clone-ref.test.ts", + "test/install-forward-restore-diagnostics.test.ts", "test/install-hermes-forward-restore.test.ts", "test/install-managed-cli-reuse.test.ts", "test/install-preflight.test.ts", @@ -194,6 +195,7 @@ export default defineConfig({ "test/install-station-vllm-continuation.test.ts", "test/install-build-dependency-preflight.test.ts", "test/install-clone-ref.test.ts", + "test/install-forward-restore-diagnostics.test.ts", "test/install-hermes-forward-restore.test.ts", "test/install-managed-cli-reuse.test.ts", "test/install-preflight.test.ts",