fix(sandbox): clear gateway marker when the start supervisor exits

docs/reference/architecture.mdx

@@ -87,7 +87,8 @@ Per-user units, partial units, and user-manager or bus outages do not take over
 That compatibility fallback remains until supported upgrade paths no longer include pre-service OpenShell installs and the package-managed handoff has direct nightly coverage.
 On Apple Silicon macOS, NemoClaw starts the OpenShell Docker-driver gateway and creates the sandbox as a Docker container.
 In both Docker-driver modes, the sandbox is a Docker container, not a Kubernetes pod.
-The in-container `/tmp/nemoclaw-gateway-local` marker is written only by entrypoint paths that actually launch an in-container gateway.
+Entrypoint supervisors create the in-container `/tmp/nemoclaw-gateway-local` marker only when they actually launch an in-container gateway, and they normally keep it present while that supervisor is active.
+On normal exits, handled `SIGTERM`/`SIGINT`, startup failures, and shell `errexit` termination through the `EXIT` trap, the supervisor removes the marker on a best-effort basis so the Docker health check does not keep trusting a stale gateway PID.
 Terminal runtimes may not write it.
 NemoClaw does not treat sandbox environment hints such as `OPENSHELL_DRIVERS` as authoritative for gateway ownership.
 Legacy non-Docker-driver installs still use the k3s-based gateway path.

scripts/nemoclaw-start.sh

@@ -350,6 +350,22 @@ mark_in_container_gateway() {
   _nemoclaw_safe_create_tmp_file /tmp/nemoclaw-gateway-local 600 "" best-effort 2>/dev/null || true
 }
 
+# Drop the in-container gateway marker (#4952). The HEALTHCHECK's pidfile
+# fallback trusts /tmp/nemoclaw-gateway.pid, which is refreshed *only* by
+# record_gateway_pid inside this supervisor's launch/respawn paths. On
+# OpenShell docker-driver sandboxes this script is NOT PID 1 -- OpenShell's
+# `sleep infinity` keeps the container alive as a sibling -- so when the
+# supervise loop exits, the container lives on but nothing refreshes the
+# pidfile. The marker would otherwise stay in place, leaving the healthcheck
+# trusting a stale PID forever (permanent false `unhealthy`). Tying marker
+# removal to supervisor exit completes the #4710 marker semantics: the marker
+# means "a supervisor is actively managing the gateway and keeping the pidfile
+# fresh". Once it is gone the healthcheck takes the marker-absent -> healthy
+# branch (#4503) instead. Best-effort: failure must never block teardown.
+clear_in_container_gateway_marker() {
+  rm -f /tmp/nemoclaw-gateway-local 2>/dev/null || true
+}
+
 # Record the PID/starttime identity of the live in-container gateway so the
 # Docker HEALTHCHECK
 # can confirm the actual gateway process (not merely *some* `openclaw`
@@ -4049,7 +4065,9 @@ GUARDENVEOF
 # cleanup_on_signal is provided by sandbox-init.sh. It reads
 # SANDBOX_CHILD_PIDS (array of all PIDs) and SANDBOX_WAIT_PID (the
 # primary process whose exit status is returned).
-# Each code path below sets these before registering the trap.
+# Each code path arms the trap before launching the gateway. These values are
+# populated as children start; cleanup refreshes and validates them before
+# signaling anything.
 
 # Keep per-user rc files out of runtime proxy wiring. Older images and prior
 # entrypoint versions wrote a two-line shim into .bashrc/.profile; remove that
@@ -4872,7 +4890,24 @@ wait_for_openclaw_gateway_internal() {
   return 1
 }
 
+arm_openclaw_gateway_supervisor_cleanup() {
+  # Bash does not run an EXIT trap when an untrapped SIGTERM/SIGINT terminates
+  # the shell, so both traps must be live before the marker is written.
+  trap cleanup_openclaw_on_signal SIGTERM SIGINT
+  trap clear_in_container_gateway_marker EXIT
+}
+
 launch_openclaw_gateway() {
+  # Drop the gateway marker whenever this supervisor exits -- clean gateway
+  # exit (`exit 0` below), a forwarded signal (cleanup_openclaw_on_signal ends
+  # in `cleanup_on_signal` -> `exit`), or errexit. This is the #4952 fix: on
+  # docker-driver sandboxes this script is not PID 1, so it can exit while the
+  # container lives on; a surviving marker would leave the HEALTHCHECK trusting
+  # a stale pidfile. Arm this before marking so early launch failures cannot
+  # leave the marker behind. The marker is re-dropped at each launch
+  # (mark_in_container_gateway), so the respawn loop -- which never exits the
+  # script -- keeps it in place.
+  arm_openclaw_gateway_supervisor_cleanup
   mark_in_container_gateway
   nohup "${STEP_DOWN_PREFIX_GATEWAY[@]}" sh -c \
     'umask 0007; exec "$@" >>/tmp/gateway.log 2>&1' sh \
@@ -4894,6 +4929,16 @@ launch_openclaw_gateway() {
   echo "[gateway] openclaw gateway launched as 'gateway' user (pid $GATEWAY_PID)" >&2
 }
 
+launch_openclaw_gateway_non_root() {
+  arm_openclaw_gateway_supervisor_cleanup
+  mark_in_container_gateway
+  nohup "$OPENCLAW" gateway run --port "${_DASHBOARD_PORT}" >/tmp/gateway.log 2>&1 &
+  GATEWAY_PID=$!
+  capture_openclaw_pid_start_identity "$GATEWAY_PID" GATEWAY_PID_START_IDENTITY || exit 1
+  record_gateway_pid "$GATEWAY_PID" "$GATEWAY_PID_START_IDENTITY"
+  echo "[gateway] openclaw gateway launched (pid $GATEWAY_PID)" >&2
+}
+
 openclaw_supervised_aux_pid_is_live() {
   local pid="$1"
   local expected_identity="$2"
@@ -5492,12 +5537,7 @@ if [ "$(id -u)" -ne 0 ]; then
   # to healthy — see the mark_in_container_gateway comment near the top of this
   # file for the #4710 rationale (why the marker is tied to the launch site
   # rather than an env-var conditional at startup).
-  mark_in_container_gateway
-  nohup "$OPENCLAW" gateway run --port "${_DASHBOARD_PORT}" >/tmp/gateway.log 2>&1 &
-  GATEWAY_PID=$!
-  capture_openclaw_pid_start_identity "$GATEWAY_PID" GATEWAY_PID_START_IDENTITY || exit 1
-  record_gateway_pid "$GATEWAY_PID" "$GATEWAY_PID_START_IDENTITY"
-  echo "[gateway] openclaw gateway launched (pid $GATEWAY_PID)" >&2
+  launch_openclaw_gateway_non_root
   # Diagnostic: mirror gateway log to PID 1's stderr — see root-mode block
   # below for rationale (NVIDIA/NemoClaw#2484).
   { tail -n +1 -F /tmp/gateway.log 2>/dev/null | sed -u 's/^/[gateway-log:] /' >&2; } &
@@ -5515,7 +5555,6 @@ if [ "$(id -u)" -ne 0 ]; then
   refresh_openclaw_supervised_child_pids
   # shellcheck disable=SC2034  # read by cleanup_on_signal from sandbox-init.sh
   SANDBOX_WAIT_PID="$GATEWAY_PID"
-  trap cleanup_openclaw_on_signal SIGTERM SIGINT
   print_dashboard_urls
 
   # Auto-respawn gateway on unexpected death (NVIDIA/NemoClaw#2757). Without
@@ -5720,6 +5759,7 @@ validate_nemoclaw_tmp_permissions
 # Marking, privilege step-down, log redirection, and PID recording are kept in
 # one reusable launch primitive so PID 1 owns initial start, crash respawn, and
 # host-requested restart identically.
+# The launch primitive arms signal and EXIT cleanup before writing the marker.
 launch_openclaw_gateway
 
 # Diagnostic: mirror gateway log to PID 1's stderr so its content surfaces in
@@ -5769,7 +5809,6 @@ start_gateway_serving_watchdog
 refresh_openclaw_supervised_child_pids
 # shellcheck disable=SC2034  # read by cleanup_on_signal from sandbox-init.sh
 SANDBOX_WAIT_PID="$GATEWAY_PID"
-trap cleanup_openclaw_on_signal SIGTERM SIGINT
 if ! gateway_control_init; then
   echo "[gateway-control] privileged gateway control unavailable" >&2
 fi

test/nemoclaw-start-gateway-health.test.ts

@@ -47,6 +47,10 @@ function extractShellFunction(src: string, name: string): string {
   return `${name}() {${body.slice(0, closing?.index ?? 0)}\n}`;
 }
 
+function gatewayMarkerFunction(src: string, name: string, markerPath: string): string {
+  return extractShellFunction(src, name).replaceAll("/tmp/nemoclaw-gateway-local", markerPath);
+}
+
 function extractGatewayLogAppendFunction(src: string, gatewayLog: string): string {
   const functionSource = extractShellFunction(src, "append_openclaw_gateway_log_line");
   const marker = '  local log_file="/tmp/gateway.log"';
@@ -101,6 +105,7 @@ function watchdogFunctions(gatewayLog: string): string {
 function rootGatewayLifecycleFunctions(src: string, gatewayLog: string): string {
   return [
     pidIdentityFunctions(src),
+    extractShellFunction(src, "arm_openclaw_gateway_supervisor_cleanup"),
     extractShellFunction(src, "launch_openclaw_gateway").replaceAll("/tmp/gateway.log", gatewayLog),
     extractShellFunction(src, "openclaw_supervised_aux_pid_is_live"),
     extractShellFunction(src, "stop_openclaw_supervised_gateway"),
@@ -111,6 +116,18 @@ function rootGatewayLifecycleFunctions(src: string, gatewayLog: string): string
   ].join("\n");
 }
 
+function gatewayLaunchBlock(src: string, kind: "non-root" | "root", gatewayLog: string): string {
+  const startMarker =
+    kind === "non-root"
+      ? "# Start gateway in background, auto-pair, then wait"
+      : "# Start the gateway as the 'gateway' user.";
+  const start = src.indexOf(startMarker);
+  const end = src.indexOf('SANDBOX_WAIT_PID="$GATEWAY_PID"', start);
+  expect(start, `Expected ${kind} gateway launch block in scripts/nemoclaw-start.sh`).not.toBe(-1);
+  expect(end, `Expected ${kind} gateway launch block in scripts/nemoclaw-start.sh`).not.toBe(-1);
+  return src.slice(start, src.indexOf("\n", end)).replaceAll("/tmp/gateway.log", gatewayLog);
+}
+
 // Drive the watchdog end-to-end against a real background process standing in
 // for the gateway. `curlPlan` is the sequence of curl exit codes the stubbed
 // probe returns, one per watchdog cycle; the last entry repeats forever.
@@ -994,12 +1011,8 @@ describe("healthcheck marker (#4503, #4710)", () => {
   });
 });
 
-// Behavioral wiring coverage: run the real launch block of each entrypoint
-// mode with the real marker/pidfile/watchdog helpers and assert their
-// runtime effects. This replaces source-text assertions (banned by
-// ci/source-shape-test-budget.json) and locks the #4748 regression
-// behaviorally: OPENSHELL_DRIVERS is exported during the run and must have
-// no influence on whether the marker is dropped.
+// Run both real launch paths with their marker, pidfile, and watchdog helpers.
+// This behaviorally covers the driver-env marker regression (#4748).
 describe("gateway launch wiring (#4710)", () => {
   it("exits PID 1 without signaling when gateway identity capture fails", () => {
     const src = fs.readFileSync(START_SCRIPT, "utf-8");
@@ -1021,10 +1034,12 @@ describe("gateway launch wiring (#4710)", () => {
           "GATEWAY_PID=0",
           "GATEWAY_PID_START_IDENTITY=",
           "mark_in_container_gateway() { :; }",
+          "clear_in_container_gateway_marker() { :; }\ncleanup_openclaw_on_signal() { :; }",
           "capture_openclaw_pid_start_identity() { return 1; }",
           'clear_gateway_pid_record() { printf "clear\\n" >>"$EVENT_LOG"; }',
           'kill() { printf "unexpected-kill:%s\\n" "$*" >>"$EVENT_LOG"; }',
           'wait() { printf "unexpected-wait:%s\\n" "$*" >>"$EVENT_LOG"; }',
+          extractShellFunction(src, "arm_openclaw_gateway_supervisor_cleanup"),
           launch,
           "launch_openclaw_gateway",
         ].join("\n"),
@@ -1038,20 +1053,6 @@ describe("gateway launch wiring (#4710)", () => {
     fs.rmSync(tmpDir, { recursive: true, force: true });
   });
 
-  function launchBlock(src: string, kind: "non-root" | "root"): string {
-    const startMarker =
-      kind === "non-root"
-        ? "# Start gateway in background, auto-pair, then wait"
-        : "# Start the gateway as the 'gateway' user.";
-    const start = src.indexOf(startMarker);
-    const trap = src.indexOf("trap cleanup_openclaw_on_signal SIGTERM SIGINT", start);
-    expect(start, `Expected ${kind} gateway launch block in scripts/nemoclaw-start.sh`).not.toBe(
-      -1,
-    );
-    expect(trap, `Expected ${kind} gateway launch block in scripts/nemoclaw-start.sh`).not.toBe(-1);
-    return src.slice(start, src.indexOf("\n", trap));
-  }
-
   function runLaunchWiring(kind: "non-root" | "root") {
     const src = fs.readFileSync(START_SCRIPT, "utf-8");
     const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `nemoclaw-launch-wiring-${kind}-`));
@@ -1074,9 +1075,11 @@ describe("gateway launch wiring (#4710)", () => {
 
     const realFunctions = [
       safeTmpHelpers(src),
-      extractShellFunction(src, "mark_in_container_gateway").replaceAll(
-        "/tmp/nemoclaw-gateway-local",
-        markerPath,
+      gatewayMarkerFunction(src, "mark_in_container_gateway", markerPath),
+      gatewayMarkerFunction(src, "clear_in_container_gateway_marker", markerPath),
+      extractShellFunction(src, "launch_openclaw_gateway_non_root").replaceAll(
+        "/tmp/gateway.log",
+        gatewayLog,
       ),
       extractShellFunction(src, "record_gateway_pid"),
       extractShellFunction(src, "gateway_pid_is_openclaw_gateway"),
@@ -1106,7 +1109,8 @@ describe("gateway launch wiring (#4710)", () => {
         "STEP_DOWN_PREFIX_SANDBOX=(gosu sandbox)",
         "STEP_DOWN_PREFIX_GATEWAY=(gosu gateway)",
         realFunctions,
-        launchBlock(src, kind).replaceAll("/tmp/gateway.log", gatewayLog),
+        gatewayLaunchBlock(src, kind, gatewayLog),
+        `if [ -f ${JSON.stringify(markerPath)} ]; then printf "MARKER_PRESENT=1\\n"; fi`,
         `for _ in $(command seq 1 100); do [ -s ${JSON.stringify(openclawLog)} ] && break; command sleep 0.1; done`,
         'printf "GATEWAY_PID=%s\\n" "$GATEWAY_PID"',
         'printf "WATCHDOG_PID=%s\\n" "${GATEWAY_WATCHDOG_PID:-}"',
@@ -1124,20 +1128,31 @@ describe("gateway launch wiring (#4710)", () => {
     const watchdogPid = stdout.match(/^WATCHDOG_PID=(\d+)$/m)?.[1];
     const childPids = (stdout.match(/^CHILD_PIDS=(.+)$/m)?.[1] ?? "").split(/\s+/);
     const pidFileContent = readFileIfPresent(pidFile)?.trim() ?? null;
+    const markerPresent = stdout.includes("MARKER_PRESENT=1");
     const markerExists = readFileIfPresent(markerPath) !== null;
     fs.rmSync(tmpDir, { recursive: true, force: true });
-    return { result, stdout, gatewayPid, watchdogPid, childPids, pidFileContent, markerExists };
+    return {
+      result,
+      stdout,
+      gatewayPid,
+      watchdogPid,
+      childPids,
+      pidFileContent,
+      markerPresent,
+      markerExists,
+    };
   }
 
   it.each([
     "non-root",
     "root",
-  ] as const)("%s launch drops the marker, records the gateway PID, and starts the tracked watchdog", (kind) => {
+  ] as const)("%s launch clears the marker on supervisor exit after recording the gateway PID", (kind) => {
     const run = runLaunchWiring(kind);
     expect(run.result.status, `script failed: ${run.result.stderr}`).toBe(0);
-    // Marker dropped by the launch site, even with OPENSHELL_DRIVERS=docker
-    // exported — env hints must not gate it (#4748 was a no-op for this).
-    expect(run.markerExists).toBe(true);
+    expect(run.markerPresent).toBe(true);
+    // The supervisor EXIT trap clears the in-container marker when this fixture
+    // exits, returning healthchecks to the marker-absent branch (#4952).
+    expect(run.markerExists).toBe(false);
     // The watchdog reads the gateway PID from the pidfile each cycle.
     expect(run.gatewayPid).toBeDefined();
     expect(run.pidFileContent?.split(" ")[0]).toBe(run.gatewayPid);
@@ -1356,27 +1371,11 @@ describe("respawn loop pidfile refresh (#4710)", () => {
   });
 });
 
-// Launch-path signal handling and child-PID tracking for both entrypoint
-// modes. Moved from test/nemoclaw-start.test.ts so the legacy file stays
-// under its ratcheted size budget; this file owns gateway-launch coverage.
+// Launch-path signal handling and child-PID tracking for both entrypoint modes.
+// This file owns gateway launch coverage to keep the legacy test within budget.
 describe("nemoclaw-start gateway launch signal handling", () => {
   const src = fs.readFileSync(START_SCRIPT, "utf-8");
 
-  function launchBlock(kind: "non-root" | "root", gatewayLog: string): string {
-    const startMarker =
-      kind === "non-root"
-        ? "# Start gateway in background, auto-pair, then wait"
-        : "# Start the gateway as the 'gateway' user.";
-    const start = src.indexOf(startMarker);
-    const trap = src.indexOf("trap cleanup_openclaw_on_signal SIGTERM SIGINT", start);
-    expect(start, `Expected ${kind} gateway launch block in scripts/nemoclaw-start.sh`).not.toBe(
-      -1,
-    );
-    expect(trap, `Expected ${kind} gateway launch block in scripts/nemoclaw-start.sh`).not.toBe(-1);
-    const lineEnd = src.indexOf("\n", trap);
-    return src.slice(start, lineEnd).replaceAll("/tmp/gateway.log", gatewayLog);
-  }
-
   function runLaunchBlock(kind: "non-root" | "root") {
     const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `nemoclaw-launch-${kind}-`));
     const fakeBin = path.join(tmpDir, "bin");
@@ -1417,20 +1416,20 @@ describe("nemoclaw-start gateway launch signal handling", () => {
         "start_plugin_registry_refresh() { :; }",
         "cleanup_on_signal() { :; }",
         safeTmpHelpers(src),
-        extractShellFunction(src, "mark_in_container_gateway").replaceAll(
-          "/tmp/nemoclaw-gateway-local",
-          markerPath,
-        ),
-        // #4710: the launch block also records the gateway PID for the
-        // serving watchdog and starts the watchdog alongside the other
-        // background services. Stub both — watchdog behavior has its own
-        // suite in test/nemoclaw-start-gateway-health.test.ts.
+        gatewayMarkerFunction(src, "mark_in_container_gateway", markerPath),
+        gatewayMarkerFunction(src, "clear_in_container_gateway_marker", markerPath),
+        // Stub PID recording and the serving watchdog; each has focused tests
+        // elsewhere in this suite (#4710).
         "record_gateway_pid() { :; }",
         'start_gateway_serving_watchdog() { sleep 30 & GATEWAY_WATCHDOG_PID=$!; capture_openclaw_pid_start_identity "$GATEWAY_WATCHDOG_PID" GATEWAY_WATCHDOG_PID_START_IDENTITY; }',
+        extractShellFunction(src, "launch_openclaw_gateway_non_root").replaceAll(
+          "/tmp/gateway.log",
+          gatewayLog,
+        ),
         rootGatewayLifecycleFunctions(src, gatewayLog),
         "STEP_DOWN_PREFIX_SANDBOX=(gosu sandbox)",
         "STEP_DOWN_PREFIX_GATEWAY=(gosu gateway)",
-        launchBlock(kind, gatewayLog),
+        gatewayLaunchBlock(src, kind, gatewayLog),
         kind === "root"
           ? `for _ in ${waitForLaunchLogIterations}; do [ -s ${JSON.stringify(gosuLog)} ] && [ -s ${JSON.stringify(openclawLog)} ] && break; sleep 0.1; done`
           : `for _ in ${waitForLaunchLogIterations}; do [ -s ${JSON.stringify(openclawLog)} ] && break; sleep 0.1; done`,
@@ -1457,7 +1456,7 @@ describe("nemoclaw-start gateway launch signal handling", () => {
 
   it("registers child PIDs, redirects gateway output, and traps signals in non-root mode", () => {
     const { result, openclaw, gateway } = runLaunchBlock("non-root");
-    expect(result.status).toBe(0);
+    expect(result.status, result.stderr).toBe(0);
     expect(openclaw).toContain("gateway run --port 19000");
     expect(openclaw).toContain("marker=present");
     expect(openclaw).not.toContain("marker=absent");