diff --git a/agents/hermes/start.sh b/agents/hermes/start.sh index 0a218689be5..992a18c441a 100755 --- a/agents/hermes/start.sh +++ b/agents/hermes/start.sh @@ -161,6 +161,79 @@ retry_tirith_marker_if_needed() { fi } +cmdline_is_hermes_gateway() { + local cmdline=" $1 " + + case "$cmdline" in + *"/hermes gateway run "* | *" hermes gateway run "*) return 0 ;; + esac + return 1 +} + +has_live_hermes_gateway() { + local proc_root="${NEMOCLAW_PROC_ROOT:-/proc}" + local cmdline_file cmdline + + for cmdline_file in "${proc_root}"/[0-9]*/cmdline; do + [ -r "$cmdline_file" ] || continue + cmdline="$(tr '\0' ' ' <"$cmdline_file" 2>/dev/null || true)" + if cmdline_is_hermes_gateway "$cmdline"; then + return 0 + fi + done + return 1 +} + +cleanup_orphan_socat_forwarders() { + local proc_root="${NEMOCLAW_PROC_ROOT:-/proc}" + local cmdline_file pid cmdline + + for cmdline_file in "${proc_root}"/[0-9]*/cmdline; do + [ -r "$cmdline_file" ] || continue + pid="$(basename "$(dirname "$cmdline_file")")" + cmdline="$(tr '\0' ' ' <"$cmdline_file" 2>/dev/null || true)" + case "$cmdline" in + *socat*"TCP-LISTEN:${PUBLIC_PORT}"*"TCP:127.0.0.1:${INTERNAL_PORT}"*) + echo "[gateway] Removing orphaned socat forwarder for ${PUBLIC_PORT}->${INTERNAL_PORT} (pid ${pid})" >&2 + kill "$pid" 2>/dev/null || true + ;; + esac + done +} + +remove_stale_gateway_file() { + local path="$1" + local label="$2" + + if [ -L "$path" ]; then + echo "[gateway] Removing unsafe stale Hermes ${label} symlink: ${path}" >&2 + rm -f "$path" 2>/dev/null || echo "[gateway] WARNING: could not remove stale ${label}: ${path}" >&2 + return + fi + if [ -f "$path" ]; then + echo "[gateway] Removing stale Hermes ${label}: ${path}" >&2 + rm -f "$path" 2>/dev/null || echo "[gateway] WARNING: could not remove stale ${label}: ${path}" >&2 + fi +} + +cleanup_stale_hermes_gateway_runtime() { + local runtime_dir="${HERMES_DIR}/runtime" + + if has_live_hermes_gateway; then + echo "[gateway] Existing Hermes gateway process detected; preserving runtime lock state" >&2 + return 0 + fi + + # Hermes can leave gateway.lock behind after Docker GPU recreation kills the + # old process namespace. Clear it only after confirming no gateway is alive. + remove_stale_gateway_file "${runtime_dir}/gateway.pid" "PID file" + if [ ! -L "${HERMES_DIR}/gateway.pid" ]; then + remove_stale_gateway_file "${HERMES_DIR}/gateway.pid" "legacy PID file" + fi + remove_stale_gateway_file "${runtime_dir}/gateway.lock" "lock file" + cleanup_orphan_socat_forwarders +} + # ── socat forwarder ────────────────────────────────────────────── # Hermes API server binds to 127.0.0.1 regardless of config (upstream bug). # OpenShell needs the port accessible on 0.0.0.0 for port forwarding. @@ -559,6 +632,8 @@ if [ "$(id -u)" -ne 0 ]; then retry_tirith_marker_if_needed + cleanup_stale_hermes_gateway_runtime + prepare_restricted_log /tmp/gateway.log "" 600 # Defence-in-depth: verify /tmp file permissions before launching services. @@ -601,6 +676,8 @@ fi retry_tirith_marker_if_needed +cleanup_stale_hermes_gateway_runtime + # SECURITY: Protect gateway log from sandbox user tampering prepare_restricted_log /tmp/gateway.log gateway:gateway 600 diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index bcf606e74b3..0971d9f6168 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -5498,19 +5498,19 @@ async function createSandbox( // from openshell because bash returns the status of the last pipeline // command (awk, always 0) unless pipefail is set. Removing the pipe // lets the real exit code flow through to run(). + const sandboxStartupCommand = ["env", ...envArgs, "nemoclaw-start"]; const createCommand = `${openshellShellCommand([ "sandbox", "create", ...createArgs, "--", - "env", - ...envArgs, - "nemoclaw-start", + ...sandboxStartupCommand, ])} 2>&1`; const dockerGpuCreatePatch = dockerGpuSandboxCreate.createDockerGpuSandboxCreatePatch({ enabled: useDockerGpuPatch, sandboxName, gpuDevice: effectiveSandboxGpuConfig.sandboxGpuDevice, + openshellSandboxCommand: sandboxStartupCommand, timeoutSecs: sandboxReadyTimeoutSecs, deps: { runOpenshell, runCaptureOpenshell, sleep }, }); diff --git a/src/lib/onboard/docker-gpu-patch.test.ts b/src/lib/onboard/docker-gpu-patch.test.ts index 77c91b9284d..016467bcb1e 100644 --- a/src/lib/onboard/docker-gpu-patch.test.ts +++ b/src/lib/onboard/docker-gpu-patch.test.ts @@ -34,6 +34,7 @@ function inspectFixture(): DockerContainerInspect { "A=1", "OPENSHELL_ENDPOINT=http://host.openshell.internal:8080/", "OPENSHELL_TEST=1", + "OPENSHELL_SANDBOX_COMMAND=sleep infinity", "NVIDIA_VISIBLE_DEVICES=void", ], Labels: { @@ -143,6 +144,51 @@ describe("docker-gpu-patch", () => { expect(args).not.toEqual(expect.arrayContaining(["--env", "NVIDIA_VISIBLE_DEVICES=void"])); }); + it("replaces OpenShell's idle sandbox command when recreating a managed container", () => { + const sandboxCommand = [ + "env", + "CHAT_UI_URL=http://127.0.0.1:8642", + "NEMOCLAW_DASHBOARD_PORT=8642", + "nemoclaw-start", + ]; + + const args = buildDockerGpuCloneRunArgs(inspectFixture(), buildDockerGpuMode("gpus"), { + openshellSandboxCommand: sandboxCommand, + }); + + expect(args).toEqual( + expect.arrayContaining([ + "--env", + "OPENSHELL_SANDBOX_COMMAND=env CHAT_UI_URL=http://127.0.0.1:8642 NEMOCLAW_DASHBOARD_PORT=8642 nemoclaw-start", + ]), + ); + expect(args).not.toEqual( + expect.arrayContaining(["--env", "OPENSHELL_SANDBOX_COMMAND=sleep infinity"]), + ); + expect(args.slice(args.indexOf("openshell/sandbox:abc"))).toEqual([ + "openshell/sandbox:abc", + ...sandboxCommand, + ]); + }); + + it("adds OpenShell's sandbox command env when the inspected container lacks one", () => { + const inspect = inspectFixture(); + inspect.Config!.Env = inspect.Config!.Env!.filter( + (entry) => !entry.startsWith("OPENSHELL_SANDBOX_COMMAND="), + ); + + const args = buildDockerGpuCloneRunArgs(inspect, buildDockerGpuMode("gpus"), { + openshellSandboxCommand: ["env", "CHAT_UI_URL=http://127.0.0.1:8642", "nemoclaw-start"], + }); + + expect(args).toEqual( + expect.arrayContaining([ + "--env", + "OPENSHELL_SANDBOX_COMMAND=env CHAT_UI_URL=http://127.0.0.1:8642 nemoclaw-start", + ]), + ); + }); + it("adds SYS_PTRACE to the GPU clone when the baseline container lacks it", () => { const inspect = inspectFixture(); inspect.HostConfig!.CapAdd = ["SYS_ADMIN", "NET_ADMIN"]; @@ -427,14 +473,20 @@ describe("docker-gpu-patch", () => { if (args[0] === "info") return ""; return ""; }); + const dockerRunDetached = vi.fn(() => ({ status: 0, stdout: "new-container-id\n" })); const runOpenshell = vi.fn(() => ({ status: 1, stderr: "phase: Provisioning" })); const result = recreateOpenShellDockerSandboxWithGpu( - { sandboxName: "alpha", timeoutSecs: 1, waitForSupervisor: false }, + { + sandboxName: "alpha", + timeoutSecs: 1, + waitForSupervisor: false, + openshellSandboxCommand: ["env", "CHAT_UI_URL=http://127.0.0.1:8642", "nemoclaw-start"], + }, { dockerCapture, dockerRun: vi.fn(() => ({ status: 0, stdout: "probe-id\n" })), - dockerRunDetached: vi.fn(() => ({ status: 0, stdout: "new-container-id\n" })), + dockerRunDetached, dockerRename: vi.fn(() => ({ status: 0 })), dockerStop: vi.fn(() => ({ status: 0 })), dockerRm: vi.fn(() => ({ status: 0 })), @@ -446,6 +498,17 @@ describe("docker-gpu-patch", () => { expect(result.newContainerId).toBe("new-container-id"); expect(runOpenshell).not.toHaveBeenCalled(); + expect(dockerRunDetached).toHaveBeenCalledWith( + expect.arrayContaining([ + "--env", + "OPENSHELL_SANDBOX_COMMAND=env CHAT_UI_URL=http://127.0.0.1:8642 nemoclaw-start", + "openshell/sandbox:abc", + "env", + "CHAT_UI_URL=http://127.0.0.1:8642", + "nemoclaw-start", + ]), + expect.objectContaining({ ignoreError: true }), + ); }); }); diff --git a/src/lib/onboard/docker-gpu-patch.ts b/src/lib/onboard/docker-gpu-patch.ts index 6b62185fade..90abea45021 100644 --- a/src/lib/onboard/docker-gpu-patch.ts +++ b/src/lib/onboard/docker-gpu-patch.ts @@ -19,6 +19,7 @@ import { envInt } from "./env"; export const OPENSHELL_MANAGED_BY_LABEL = "openshell.ai/managed-by"; export const OPENSHELL_MANAGED_BY_VALUE = "openshell"; export const OPENSHELL_SANDBOX_NAME_LABEL = "openshell.ai/sandbox-name"; +const OPENSHELL_SANDBOX_COMMAND_ENV = "OPENSHELL_SANDBOX_COMMAND"; const DOCKER_GPU_PATCH_TIMEOUT_MS = 30_000; const DOCKER_GPU_PATCH_WAIT_SECS = 180; @@ -103,6 +104,7 @@ export type DockerGpuCloneRunOptions = { networkMode?: string | null; openshellEndpoint?: string | null; sandboxFallbackDns?: string | null; + openshellSandboxCommand?: readonly string[] | null; }; export type DockerGpuPatchDiagnostics = { @@ -246,6 +248,11 @@ function replaceEnvValue(entry: string, key: string, value: string | null | unde return `${key}=${value}`; } +function openshellSandboxCommandEnvValue(command: readonly string[] | null | undefined): string | null { + const parts = (command || []).map((part) => String(part)).filter((part) => part.length > 0); + return parts.length > 0 ? parts.join(" ") : null; +} + function dockerGpuHostEndpointFromOpenShellEndpoint(endpoint: string): string | null { try { const url = new URL(endpoint); @@ -443,9 +450,22 @@ export function buildDockerGpuCloneRunArgs( if (config.Tty) args.push("--tty"); if (config.OpenStdin) args.push("--interactive"); + const openshellSandboxCommandEnv = openshellSandboxCommandEnvValue( + options.openshellSandboxCommand, + ); + let sawOpenShellSandboxCommandEnv = false; for (const env of stringArray(config.Env).filter((entry) => !GPU_ENV_KEYS.has(envKey(entry)))) { + const key = envKey(env); + if (key === OPENSHELL_SANDBOX_COMMAND_ENV && openshellSandboxCommandEnv) { + sawOpenShellSandboxCommandEnv = true; + args.push("--env", `${OPENSHELL_SANDBOX_COMMAND_ENV}=${openshellSandboxCommandEnv}`); + continue; + } args.push("--env", replaceEnvValue(env, "OPENSHELL_ENDPOINT", options.openshellEndpoint)); } + if (openshellSandboxCommandEnv && !sawOpenShellSandboxCommandEnv) { + args.push("--env", `${OPENSHELL_SANDBOX_COMMAND_ENV}=${openshellSandboxCommandEnv}`); + } const labels = config.Labels || {}; for (const key of Object.keys(labels).sort()) { @@ -527,7 +547,10 @@ export function buildDockerGpuCloneRunArgs( const entrypoint = stringArray(config.Entrypoint); if (entrypoint.length > 0) args.push("--entrypoint", entrypoint[0]); - const commandArgs = [...entrypoint.slice(1), ...stringArray(config.Cmd)]; + const commandArgs = + options.openshellSandboxCommand && options.openshellSandboxCommand.length > 0 + ? [...options.openshellSandboxCommand] + : [...entrypoint.slice(1), ...stringArray(config.Cmd)]; args.push(image, ...commandArgs); return args; } @@ -760,6 +783,7 @@ export function recreateOpenShellDockerSandboxWithGpu( gpuDevice?: string | null; timeoutSecs?: number; waitForSupervisor?: boolean; + openshellSandboxCommand?: readonly string[] | null; }, deps: DockerGpuPatchDeps = {}, ): DockerGpuPatchResult { @@ -811,6 +835,7 @@ export function recreateOpenShellDockerSandboxWithGpu( } const cloneOptions = buildDockerGpuCloneRunOptions(inspect); + cloneOptions.openshellSandboxCommand = options.openshellSandboxCommand ?? null; const sandboxFallbackDns = d.detectSandboxFallbackDns(); if (sandboxFallbackDns) cloneOptions.sandboxFallbackDns = sandboxFallbackDns; const cloneArgs = buildDockerGpuCloneRunArgs(inspect, selection.mode, cloneOptions); diff --git a/src/lib/onboard/docker-gpu-sandbox-create.ts b/src/lib/onboard/docker-gpu-sandbox-create.ts index 9857aea56f5..a63538413c8 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create.ts @@ -25,6 +25,7 @@ type DockerGpuSandboxCreatePatchOptions = { enabled: boolean; sandboxName: string; gpuDevice?: string | null; + openshellSandboxCommand?: readonly string[] | null; timeoutSecs: number; deps: DockerGpuSandboxCreateDeps; }; @@ -58,6 +59,7 @@ export function createDockerGpuSandboxCreatePatch( const applyOptions = { sandboxName: options.sandboxName, gpuDevice: options.gpuDevice, + openshellSandboxCommand: options.openshellSandboxCommand ?? null, timeoutSecs: options.timeoutSecs, }; diff --git a/test/hermes-start.test.ts b/test/hermes-start.test.ts index 8734d2a6214..1e143e28bc8 100644 --- a/test/hermes-start.test.ts +++ b/test/hermes-start.test.ts @@ -96,6 +96,84 @@ function runTirithMarkerBootstrap(opts: { } } +function writeFakeProcCmdline(procRoot: string, pid: number, argv: string[]) { + const pidDir = path.join(procRoot, String(pid)); + fs.mkdirSync(pidDir, { recursive: true }); + fs.writeFileSync(path.join(pidDir, "cmdline"), Buffer.from(`${argv.join("\0")}\0`)); +} + +function runHermesGatewayRuntimeCleanup(opts: { + liveGateway?: boolean; + orphanSocat?: boolean; + staleLock?: boolean; + stalePid?: boolean; +}) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-hermes-runtime-cleanup-")); + const hermesHome = path.join(tmpDir, ".hermes"); + const runtimeDir = path.join(hermesHome, "runtime"); + const procRoot = path.join(tmpDir, "proc"); + const killLog = path.join(tmpDir, "kill.log"); + const scriptPath = path.join(tmpDir, "run.sh"); + const legacyPid = path.join(hermesHome, "gateway.pid"); + const runtimePid = path.join(runtimeDir, "gateway.pid"); + const runtimeLock = path.join(runtimeDir, "gateway.lock"); + + fs.mkdirSync(runtimeDir, { recursive: true }); + fs.mkdirSync(procRoot, { recursive: true }); + fs.symlinkSync("runtime/gateway.pid", legacyPid); + if (opts.stalePid !== false) fs.writeFileSync(runtimePid, "999999\n"); + if (opts.staleLock !== false) fs.writeFileSync(runtimeLock, "stale lock"); + if (opts.liveGateway) { + writeFakeProcCmdline(procRoot, 123, ["/usr/local/bin/hermes", "gateway", "run"]); + } + if (opts.orphanSocat) { + writeFakeProcCmdline(procRoot, 456, [ + "socat", + "TCP-LISTEN:8642,bind=0.0.0.0,fork,reuseaddr", + "TCP:127.0.0.1:18642", + ]); + } + + const src = fs.readFileSync(START_SCRIPT, "utf-8"); + fs.writeFileSync( + scriptPath, + [ + "#!/usr/bin/env bash", + "set -euo pipefail", + extractShellFunctionFromSource(src, "cmdline_is_hermes_gateway"), + extractShellFunctionFromSource(src, "has_live_hermes_gateway"), + extractShellFunctionFromSource(src, "cleanup_orphan_socat_forwarders"), + extractShellFunctionFromSource(src, "remove_stale_gateway_file"), + extractShellFunctionFromSource(src, "cleanup_stale_hermes_gateway_runtime"), + `KILL_LOG=${shellQuote(killLog)}`, + 'kill() { printf "%s\\n" "$*" >>"$KILL_LOG"; return 0; }', + `HERMES_DIR=${shellQuote(hermesHome)}`, + `NEMOCLAW_PROC_ROOT=${shellQuote(procRoot)}`, + "PUBLIC_PORT=8642", + "INTERNAL_PORT=18642", + "cleanup_stale_hermes_gateway_runtime", + ].join("\n"), + { mode: 0o700 }, + ); + + try { + const result = spawnSync("bash", [scriptPath], { + encoding: "utf-8", + timeout: 5000, + env: process.env, + }); + return { + result, + killLog: fs.existsSync(killLog) ? fs.readFileSync(killLog, "utf-8") : "", + runtimePidExists: fs.existsSync(runtimePid), + runtimeLockExists: fs.existsSync(runtimeLock), + legacyPidIsSymlink: fs.lstatSync(legacyPid).isSymbolicLink(), + }; + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + function runRuntimeShellEnvBootstrap() { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-hermes-runtime-env-")); const envFile = path.join(tmpDir, "nemoclaw-proxy-env.sh"); @@ -180,6 +258,37 @@ describe("agents/hermes/start.sh runtime shell env", () => { }); +describe("agents/hermes/start.sh gateway runtime cleanup", () => { + it("removes stale Hermes pid and lock files while preserving the compatibility pid symlink", () => { + const run = runHermesGatewayRuntimeCleanup({}); + + expect(run.result.status).toBe(0); + expect(run.runtimePidExists).toBe(false); + expect(run.runtimeLockExists).toBe(false); + expect(run.legacyPidIsSymlink).toBe(true); + expect(run.result.stderr).toContain("Removing stale Hermes PID file"); + expect(run.result.stderr).toContain("Removing stale Hermes lock file"); + }); + + it("kills orphaned socat forwarders when no Hermes gateway is alive", () => { + const run = runHermesGatewayRuntimeCleanup({ orphanSocat: true, staleLock: false, stalePid: false }); + + expect(run.result.status).toBe(0); + expect(run.killLog.trim()).toBe("456"); + expect(run.result.stderr).toContain("Removing orphaned socat forwarder"); + }); + + it("preserves Hermes runtime state when a gateway process is alive", () => { + const run = runHermesGatewayRuntimeCleanup({ liveGateway: true, orphanSocat: true }); + + expect(run.result.status).toBe(0); + expect(run.runtimePidExists).toBe(true); + expect(run.runtimeLockExists).toBe(true); + expect(run.killLog).toBe(""); + expect(run.result.stderr).toContain("Existing Hermes gateway process detected"); + }); +}); + describe("agents/hermes/start.sh Tirith marker bootstrap", () => { it("removes a retryable download_failed marker so Hermes runtime fallback can retry", () => { const run = runTirithMarkerBootstrap({ markerReason: "download_failed" });