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
52 changes: 27 additions & 25 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ RUN if [ "$NEMOCLAW_DARWIN_VM_COMPAT" = "1" ]; then \
# can detect and restart unhealthy containers in standalone deployments.
# Ref: https://github.com/NVIDIA/NemoClaw/issues/1430
#
# Two-stage probe so Docker health does not contradict the NemoClaw delivery
# Layered probe so Docker health does not contradict the NemoClaw delivery
# chain on runtimes where the dashboard port lives in a different network
# namespace (e.g. DGX Spark / aarch64 with OpenShell-managed forwarding).
# The reporter saw `nemoclaw status` Ready + the host forward succeed while
Expand All @@ -846,27 +846,31 @@ RUN if [ "$NEMOCLAW_DARWIN_VM_COMPAT" = "1" ]; then \
#
# 1. Direct in-container probe (HTTP 200) — definitive when it works,
# preserves the original Compose/standalone health signal.
# 2. ONLY on curl exit 7 ("Couldn't connect"), fall back to verifying
# ALL of:
# - the OpenClaw gateway process started by nemoclaw-start is
# still alive (via pgrep --ignore-ancestors), AND
# - the gateway log file exists and is non-empty (proves the
# process started and emitted its banner; rules out cases
# where the gateway never came up).
# Exit 7 means the in-container TCP connect was refused by the
# kernel because nothing is bound to the dashboard port inside
# this network namespace — the namespace-mismatch shape reported
# in #3975. A connect timeout (curl exit 28) is treated as a real
# failure: a listener exists but is not responding (wedged HTTP
# server), and we want Docker to restart in that case.
#
# Tradeoff: this fallback also fires in a standalone deployment where the
# gateway process is alive but the configured dashboard port is wrong or
# the listener never came up. We accept that residual risk because it
# requires a misconfiguration the start-period (45s) already gives the
# wizard a chance to fix, and the existing host-side delivery chain
# probes (verify-deployment.ts, host port forward, sandbox status) still
# catch it from outside.
# 2. A connect timeout (curl exit 28) or HTTP 4xx/5xx (curl exit 22) is a
# real bad signal: a listener exists but is wedged or answered with a
# failure inside this container, so Docker should restart it.
# 3. ONLY on curl exit 7 ("Couldn't connect" — the kernel refused the
# in-container TCP connect because nothing is bound to the dashboard
# port in THIS network namespace) the meaning depends on whether this
# container is the one running the OpenClaw gateway:
# a. If nemoclaw-start launched the gateway in this container it
# drops the /tmp/nemoclaw-gateway-local marker (see
# scripts/nemoclaw-start.sh). The gateway is local but its port
# may be forwarded out of this namespace (#3975), so confirm the
# gateway came up: the process is still alive (pgrep
# --ignore-ancestors) AND the gateway log is non-empty. A
# standalone deployment whose gateway never started fails here so
# Docker restarts it (#1430).
# b. If the marker is ABSENT the OpenClaw gateway is delivered
# outside this container (OpenShell docker-driver deployments run
# it on the host / in a host-side process chain — #4503). An
# in-container curl/pgrep cannot observe an out-of-namespace
# gateway, so a process-name fallback here produced false
# "unhealthy" while `nemoclaw status` and OpenShell reported the
# sandbox Ready. We must not drive Docker health off a signal we
# cannot prove: report healthy and defer to NemoClaw/OpenShell's
# host-side delivery-chain monitoring (verify-deployment.ts, host
# port forward, sandbox status).
#
# The process pattern matches both `openclaw gateway run` (the launcher
# command nemoclaw-start runs) and `openclaw-gateway` (the re-execed
Expand All @@ -879,9 +883,6 @@ RUN if [ "$NEMOCLAW_DARWIN_VM_COMPAT" = "1" ]; then \
# without --ignore-ancestors `pgrep -f` would happily report it as the
# live gateway even after the real process exited (procps 4.0+ supports
# this flag; the base image pins procps to 2:4.0.4-9).
#
# We deliberately do not fall back on HTTP 4xx/5xx — those mean the gateway
# answered with a failure inside this container, which is a real bad signal.
HEALTHCHECK --interval=30s --timeout=5s --start-period=45s --retries=3 \
CMD port="${NEMOCLAW_DASHBOARD_PORT:-${OPENCLAW_GATEWAY_PORT:-}}"; \
if [ -z "$port" ]; then \
Expand All @@ -891,6 +892,7 @@ HEALTHCHECK --interval=30s --timeout=5s --start-period=45s --retries=3 \
curl -sf --max-time 3 "http://127.0.0.1:${port}/health" > /dev/null 2>&1 || rc=$?; \
if [ "$rc" = 0 ]; then exit 0; fi; \
if [ "$rc" != 7 ]; then exit 1; fi; \
[ -f /tmp/nemoclaw-gateway-local ] || exit 0; \
pgrep --ignore-ancestors -f 'openclaw[ -]gateway' > /dev/null 2>&1 || exit 1; \
[ -s /tmp/gateway.log ]

Expand Down
23 changes: 23 additions & 0 deletions scripts/nemoclaw-start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,29 @@ case "${1:-}" in
esac
NEMOCLAW_CMD=("$@")

# Drop the marker the Docker HEALTHCHECK reads to decide whether an
# in-container gateway liveness check is meaningful. We write it as early as
# possible on the gateway-serving path — before the long startup work below —
# so a slow or hung boot is governed by the strict local liveness check
# (pgrep + gateway log) instead of being masked as healthy. Its presence means
# this container runs the OpenClaw gateway (standalone deployments and the
# #3975 forwarded-port shape). Its absence means the gateway is delivered out
# of this container's namespace (OpenShell docker-driver sandboxes run it on
# the host — #4503); an in-container probe cannot observe it, so the HEALTHCHECK
# reports healthy and defers to NemoClaw/OpenShell host-side delivery-chain
# monitoring. See the HEALTHCHECK block in the Dockerfile.
# Best-effort: a write failure must never block startup.
mark_in_container_gateway() {
: >/tmp/nemoclaw-gateway-local 2>/dev/null || true
}
# A non-empty NEMOCLAW_CMD means this container only runs a one-shot command
# (e.g. `openclaw agent ...`) and never serves the gateway, so leave the marker
# absent. Both the root and non-root entrypoint paths gate gateway startup on
# the same emptiness check further below.
if [ ${#NEMOCLAW_CMD[@]} -eq 0 ]; then
mark_in_container_gateway
fi

_chat_ui_url_port() {
[ -n "${CHAT_UI_URL:-}" ] || return 1
python3 - "$CHAT_UI_URL" <<'PYPORT'
Expand Down
72 changes: 71 additions & 1 deletion src/lib/actions/sandbox/docker-health.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,27 @@

import { describe, expect, it } from "vitest";

import { getSandboxDockerHealth } from "../../../../dist/lib/actions/sandbox/docker-health";
import {
getSandboxDockerHealth,
getSandboxDockerRuntime,
} from "../../../../dist/lib/actions/sandbox/docker-health";
import type { SandboxEntry } from "../../../../dist/lib/state/registry";

function fixture({
driver = "docker",
psNames = "openshell-cluster-nemoclaw\nopenshell-my-assistant-12ab\nopenshell-other-aa11",
healthRaw = "unhealthy\n",
pausedRaw = "false\n",
throwOnInspect = false,
throwOnPaused = false,
knownSandboxes = ["my-assistant"],
}: {
driver?: string | null;
psNames?: string;
healthRaw?: string;
pausedRaw?: string;
throwOnInspect?: boolean;
throwOnPaused?: boolean;
knownSandboxes?: string[];
} = {}) {
const sandbox: Partial<SandboxEntry> = { name: "my-assistant", openshellDriver: driver };
Expand All @@ -28,6 +35,10 @@ function fixture({
if (throwOnInspect) throw new Error("docker inspect crashed");
return healthRaw;
},
dockerInspectPaused: () => {
if (throwOnPaused) throw new Error("docker inspect paused crashed");
return pausedRaw;
},
};
}

Expand Down Expand Up @@ -141,3 +152,62 @@ describe("getSandboxDockerHealth", () => {
expect(result.containerName).toBe("openshell-my-assistant-12ab");
});
});

describe("getSandboxDockerRuntime (#4495)", () => {
it("reports paused=true when the resolved docker-driver container is paused", () => {
const deps = fixture({ healthRaw: "healthy\n", pausedRaw: "true\n" });
expect(getSandboxDockerRuntime("my-assistant", deps)).toEqual({
health: "healthy",
paused: true,
containerName: "openshell-my-assistant-12ab",
});
});

it("reports paused=false for a running container", () => {
const deps = fixture({ healthRaw: "healthy\n", pausedRaw: "false\n" });
expect(getSandboxDockerRuntime("my-assistant", deps).paused).toBe(false);
});

it("normalizes whitespace and case in the .State.Paused value", () => {
const deps = fixture({ pausedRaw: " True \n" });
expect(getSandboxDockerRuntime("my-assistant", deps).paused).toBe(true);
});

it("treats a non-boolean / empty paused value as not paused", () => {
const deps = fixture({ pausedRaw: "<no value>\n" });
expect(getSandboxDockerRuntime("my-assistant", deps).paused).toBe(false);
});

it("reports paused=false and does not throw when the paused inspect fails", () => {
const deps = fixture({ healthRaw: "unhealthy\n", throwOnPaused: true });
const result = getSandboxDockerRuntime("my-assistant", deps);
expect(result.paused).toBe(false);
expect(result.health).toBe("unhealthy");
expect(result.containerName).toBe("openshell-my-assistant-12ab");
});

it("returns health 'none', paused false for non-docker-driver sandboxes", () => {
const deps = fixture({ driver: "kubernetes" });
expect(getSandboxDockerRuntime("my-assistant", deps)).toEqual({
health: "none",
paused: false,
containerName: null,
});
});

it("returns health 'none', paused false when no container is found", () => {
const deps = fixture({ psNames: "openshell-cluster-nemoclaw\n" });
expect(getSandboxDockerRuntime("my-assistant", deps)).toEqual({
health: "none",
paused: false,
containerName: null,
});
});

it("reports health 'unknown' but still resolves paused when the health inspect throws", () => {
const deps = fixture({ throwOnInspect: true, pausedRaw: "true\n" });
const result = getSandboxDockerRuntime("my-assistant", deps);
expect(result.health).toBe("unknown");
expect(result.paused).toBe(true);
});
});
56 changes: 56 additions & 0 deletions src/lib/actions/sandbox/docker-health.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,25 @@ export interface SandboxDockerHealth {
containerName: string | null;
}

/**
* Combined Docker runtime view for a docker-driver sandbox container: the
* HEALTHCHECK signal plus whether the container is paused (`docker pause`).
* A paused container can surface upstream as `Phase: Error` even though the
* sandbox is intact, so `status` reads `paused` to print a recovery hint
* without rewriting the authoritative phase. See #4495.
*/
export interface SandboxDockerRuntime {
health: DockerHealthState;
paused: boolean;
containerName: string | null;
}

interface ResolveDeps {
getSandbox: (name: string) => registry.SandboxEntry | null;
listSandboxNames: () => string[];
dockerPsNames: () => string;
dockerInspectHealth: (containerName: string) => string;
dockerInspectPaused: (containerName: string) => string;
}

const defaultDeps: ResolveDeps = {
Expand All @@ -35,6 +49,12 @@ const defaultDeps: ResolveDeps = {
containerName,
{ ignoreError: true },
),
dockerInspectPaused: (containerName) =>
dockerContainerInspectFormat(
"{{if .State}}{{.State.Paused}}{{else}}false{{end}}",
containerName,
{ ignoreError: true },
),
};

function resolveDockerDriverSandboxContainer(
Expand Down Expand Up @@ -123,3 +143,39 @@ export function getSandboxDockerHealth(
}
return { state: normalizeHealthState(raw), containerName };
}

function normalizePausedState(raw: string): boolean {
// `docker inspect --format {{.State.Paused}}` prints `true`/`false`. Treat
// anything else (empty output, inspect failure surfaced as a string, older
// engines without the field) as not paused so we never invent a paused hint.
return raw.trim().toLowerCase() === "true";
}

/**
* Resolve a docker-driver sandbox container once and read both its HEALTHCHECK
* state and `.State.Paused` flag. Returns `health: "none", paused: false` when
* the sandbox is not on the docker driver or no container is found — same
* resolution contract as {@link getSandboxDockerHealth}. A paused container is
* still listed by `docker ps`, so the existing resolver finds it. See #4495.
*/
export function getSandboxDockerRuntime(
sandboxName: string,
depsOverride: Partial<ResolveDeps> = {},
): SandboxDockerRuntime {
const deps: ResolveDeps = { ...defaultDeps, ...depsOverride };
const containerName = resolveDockerDriverSandboxContainer(sandboxName, deps);
if (!containerName) return { health: "none", paused: false, containerName: null };
let health: DockerHealthState;
try {
health = normalizeHealthState(deps.dockerInspectHealth(containerName));
} catch {
health = "unknown";
}
let paused = false;
try {
paused = normalizePausedState(deps.dockerInspectPaused(containerName));
} catch {
paused = false;
}
return { health, paused, containerName };
}
Loading
Loading