fix(connect): use TCP probe to verify forward health

src/lib/actions/sandbox/process-recovery.ts

@@ -310,16 +310,26 @@ function ensureSandboxPortForward(sandboxName: string): boolean {
  * The in-sandbox gateway and the host-side forward are independent
  * dimensions: the forward can die (host SSH session dropped, list shows
  * STATUS=dead) while the gateway keeps listening on 127.0.0.1:<port>.
+ *
+ * Also falls back to a local TCP/HTTP probe of 127.0.0.1:<port> when
+ * `forward list` would classify the entry as not-running. openshell's
+ * STATUS column lags real state — it can show "dead" for an entry that
+ * is still serving traffic, or hide an entry whose SSH session was just
+ * recycled (#3334). Trusting the column verbatim made every `connect`
+ * print a "missing or dead" preamble followed by a "Failed to
+ * re-establish" line even though the forward worked.
  */
 function isSandboxForwardHealthy(sandboxName: string): SandboxForwardHealth {
-  const port = String(resolveSandboxDashboardPort(sandboxName));
+  const port = resolveSandboxDashboardPort(sandboxName);
   const result = captureOpenshell(["forward", "list"], {
     ignoreError: true,
     timeout: OPENSHELL_PROBE_TIMEOUT_MS,
   });
   if (!result || isCommandTimeout(result) || result.status !== 0) return null;
   const entries = parseForwardList(result.output) as SandboxForwardListEntry[];
-  return classifySandboxForwardHealth(entries, sandboxName, port);
+  return classifyForwardHealthWithReachability(entries, sandboxName, String(port), () =>
+    isLocalForwardReachable(port),
+  );
 }
 
 export function classifySandboxForwardHealth(
@@ -333,6 +343,50 @@ export function classifySandboxForwardHealth(
   return match.status === "running";
 }
 
+/**
+ * Like {@link classifySandboxForwardHealth} but accepts a reachability
+ * callback that probes whether the local forwarded port actually answers.
+ * When the entry-based classification would return `false`, the
+ * reachability check overrides it: a port that answers is healthy
+ * regardless of what `forward list` reports. The "occupied" verdict is
+ * preserved — we never silently take over a forward owned by another
+ * sandbox, even if that forward happens to be reachable.
+ */
+export function classifyForwardHealthWithReachability(
+  entries: SandboxForwardListEntry[],
+  sandboxName: string,
+  port: string,
+  isReachable: () => boolean,
+): Exclude<SandboxForwardHealth, null> {
+  const verdict = classifySandboxForwardHealth(entries, sandboxName, port);
+  if (verdict !== false) return verdict;
+  return isReachable() ? true : false;
+}
+
+/**
+ * Synchronous reachability check for a local port. Used to override a
+ * negative `openshell forward list` verdict when the forward is actually
+ * still serving traffic — see {@link classifyForwardHealthWithReachability}.
+ * Returns false on any error so the existing recovery path stays intact
+ * when Node can't probe (e.g., restrictive sandbox).
+ */
+function isLocalForwardReachable(port: number): boolean {
+  const script =
+    "const net=require('node:net');" +
+    `const s=net.createConnection({host:'127.0.0.1',port:${port}});` +
+    "s.setTimeout(1000);" +
+    "s.on('connect',()=>{s.destroy();process.exit(0)});" +
+    "s.on('error',()=>process.exit(1));" +
+    "s.on('timeout',()=>{s.destroy();process.exit(1)});";
+  const result = spawnSync(process.execPath, ["-e", script], {
+    encoding: "utf-8",
+    stdio: ["ignore", "ignore", "ignore"],
+    timeout: 2000,
+  });
+  if (result.error) return false;
+  return result.status === 0;
+}
+
 /**
  * Detect and recover from a sandbox that survived a gateway restart but
  * whose OpenClaw processes are not running. Also re-establishes the

test/process-recovery.test.ts

@@ -4,8 +4,10 @@
 import { describe, expect, it } from "vitest";
 
 import {
+  classifyForwardHealthWithReachability,
   classifySandboxForwardHealth,
   resolveSandboxDashboardPort,
+  type SandboxForwardListEntry,
 } from "../dist/lib/actions/sandbox/process-recovery.js";
 
 describe("resolveSandboxDashboardPort", () => {
@@ -81,3 +83,63 @@ describe("classifySandboxForwardHealth", () => {
     ).toBe(false);
   });
 });
+
+describe("classifyForwardHealthWithReachability", () => {
+  // Regression coverage for #3334: `openshell forward list` STATUS can lag the
+  // real state of the forward. When it shows a non-running entry but the
+  // local port still answers, the forward is functionally healthy and the
+  // probe must not trigger spurious "missing or dead" + "Failed to
+  // re-establish" log pairs.
+  it("treats a non-running entry as healthy when the local port answers", () => {
+    // Covers both branches that produce `false` from the underlying classifier:
+    // a missing entry, and an entry whose status is anything but "running".
+    const inputs: SandboxForwardListEntry[][] = [
+      [],
+      [{ sandboxName: "beta", port: "18790", status: "dead" }],
+    ];
+    for (const entries of inputs) {
+      expect(
+        classifyForwardHealthWithReachability(entries, "beta", "18790", () => true),
+      ).toBe(true);
+    }
+  });
+
+  it("returns false when forward list says dead and the port does not answer", () => {
+    expect(
+      classifyForwardHealthWithReachability(
+        [{ sandboxName: "beta", port: "18790", status: "dead" }],
+        "beta",
+        "18790",
+        () => false,
+      ),
+    ).toBe(false);
+  });
+
+  it("returns true without probing when forward list already reports running", () => {
+    let probed = false;
+    const result = classifyForwardHealthWithReachability(
+      [{ sandboxName: "beta", port: "18790", status: "running" }],
+      "beta",
+      "18790",
+      () => {
+        probed = true;
+        return false;
+      },
+    );
+    expect(result).toBe(true);
+    expect(probed).toBe(false);
+  });
+
+  it("returns occupied even when the port answers if another sandbox owns it", () => {
+    // Reachability says yes, but the entry belongs to a different sandbox —
+    // we must not silently take over someone else's forward.
+    expect(
+      classifyForwardHealthWithReachability(
+        [{ sandboxName: "alpha", port: "18790", status: "running" }],
+        "beta",
+        "18790",
+        () => true,
+      ),
+    ).toBe("occupied");
+  });
+});

test/recover-port-forward.test.ts

@@ -11,6 +11,14 @@ import { execTimeout, testTimeoutOptions } from "./helpers/timeouts";
 
 const tmpFixtures: string[] = [];
 
+// Each fixture grabs a unique high port. Sharing port 18789 across tests
+// collides with real nemoclaw installs on the developer's machine: the
+// post-#3334 reachability probe sees the real forward answering and
+// (correctly) classifies the dead-list entry as healthy, skipping recovery.
+// Seed the base with the worker PID so parallel vitest workers (if ever
+// enabled for this file) can't reuse the same ports across processes.
+let nextFixturePort = 47000 + (process.pid % 10000);
+
 afterEach(() => {
   for (const dir of tmpFixtures.splice(0)) {
     fs.rmSync(dir, { recursive: true, force: true });
@@ -33,7 +41,7 @@ function setupFixture(opts: {
   port?: string;
 }): Fixture {
   const sandboxName = opts.sandboxName;
-  const port = opts.port ?? "18789";
+  const port = opts.port ?? String(nextFixturePort++);
   const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-recover-"));
   tmpFixtures.push(tmpDir);
   const homeLocalBin = path.join(tmpDir, ".local", "bin");