fix(ssh): route PortInUseError to ephemeral-port fallback

wanglu241 · claude · wanglu241 · commit 04810b917861 · 2026-06-18T23:31:40.000+08:00
The preferred-port preflight calls ensurePortAvailable, which raises the
domain PortInUseError (no errno `code`) on a busy port. The catch only
matched `isErrno(err) &amp;&amp; err.code === "EADDRINUSE"`, so PortInUseError
fell through to the rethrow branch and the ephemeral-port fallback never
fired. Scoping the probe to 127.0.0.1 made this reachable. Match on the
domain error so a busy preferred port falls back as intended.

Co-Authored-By: Claude &lt;noreply@anthropic.com&gt;
diff --git a/src/infra/ssh-tunnel.test.ts b/src/infra/ssh-tunnel.test.ts
@@ -1,15 +1,24 @@
 // Covers SSH target parsing and tunnel startup preflight behavior.
-import { describe, expect, it, vi } from "vitest";
+import { EventEmitter } from "node:events";
+import net from "node:net";
+import { afterEach, describe, expect, it, vi } from "vitest";
 
 const mocks = vi.hoisted(() => ({
   ensurePortAvailable: vi.fn<(port: number, host?: string) => Promise<void>>(),
+  spawn: vi.fn(),
 }));
 
 vi.mock("./ports.js", async (importOriginal) => ({
   ...(await importOriginal<typeof import("./ports.js")>()),
   ensurePortAvailable: mocks.ensurePortAvailable,
 }));
 
+vi.mock("node:child_process", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("node:child_process")>()),
+  spawn: mocks.spawn,
+}));
+
+import { PortInUseError } from "./ports.js";
 import { parseSshTarget, startSshPortForward } from "./ssh-tunnel.js";
 
 describe("parseSshTarget", () => {
@@ -42,6 +51,51 @@ describe("parseSshTarget", () => {
 });
 
 describe("startSshPortForward", () => {
+  const openServers: net.Server[] = [];
+
+  afterEach(async () => {
+    while (openServers.length > 0) {
+      const server = openServers.pop();
+      await new Promise<void>((resolve) => {
+        server?.close(() => resolve());
+      });
+    }
+    mocks.ensurePortAvailable.mockReset();
+    mocks.spawn.mockReset();
+  });
+
+  // Fake ssh child that, when spawned, parses the -L forward spec and starts a
+  // real IPv4-loopback listener on the chosen local port so waitForLocalListener
+  // resolves without launching a real ssh process.
+  function spawnFakeSshListening() {
+    mocks.spawn.mockImplementation((_cmd: string, args: string[]) => {
+      const forwardSpec = args[args.indexOf("-L") + 1] ?? "";
+      const localPort = Number(forwardSpec.split(":")[0]);
+      const server = net.createServer();
+      server.on("error", () => {});
+      openServers.push(server);
+      server.listen(localPort, "127.0.0.1");
+
+      const child = new EventEmitter() as EventEmitter & {
+        killed: boolean;
+        pid: number;
+        stderr: EventEmitter & { setEncoding: (enc: string) => void };
+        kill: (signal?: string) => boolean;
+      };
+      child.killed = false;
+      child.pid = 4242;
+      const stderr = new EventEmitter() as EventEmitter & { setEncoding: (enc: string) => void };
+      stderr.setEncoding = () => {};
+      child.stderr = stderr;
+      child.kill = (signal?: string) => {
+        child.killed = true;
+        queueMicrotask(() => child.emit("exit", 0, signal ?? null));
+        return true;
+      };
+      return child;
+    });
+  }
+
   it("scopes the preferred-port preflight to the IPv4 loopback interface", async () => {
     const sentinel = new Error("stop before spawning ssh");
     mocks.ensurePortAvailable.mockRejectedValueOnce(sentinel);
@@ -57,4 +111,45 @@ describe("startSshPortForward", () => {
 
     expect(mocks.ensurePortAvailable).toHaveBeenCalledWith(43210, "127.0.0.1");
   });
+
+  it("falls back to an ephemeral port when the preferred port is in use", async () => {
+    // ensurePortAvailable raises the domain PortInUseError (no errno `code`),
+    // which the catch must treat as "busy" and route to pickEphemeralPort.
+    // Reserve a real port so pickEphemeralPort (listen(0)) cannot hand the same
+    // number back and make the assertion flaky.
+    const occupied = net.createServer();
+    await new Promise<void>((resolve, reject) => {
+      occupied.once("error", reject);
+      occupied.listen(0, "127.0.0.1", () => {
+        occupied.off("error", reject);
+        resolve();
+      });
+    });
+    openServers.push(occupied);
+    const addr = occupied.address();
+    if (!addr || typeof addr === "string") {
+      throw new Error("failed to reserve preferred port");
+    }
+    const preferredPort = addr.port;
+
+    mocks.ensurePortAvailable.mockRejectedValueOnce(new PortInUseError(preferredPort));
+    spawnFakeSshListening();
+
+    const tunnel = await startSshPortForward({
+      target: "me@example.com:2222",
+      localPortPreferred: preferredPort,
+      remotePort: 18789,
+      timeoutMs: 1000,
+    });
+
+    expect(tunnel.localPort).not.toBe(preferredPort);
+    expect(tunnel.localPort).toBeGreaterThan(0);
+    expect(mocks.spawn).toHaveBeenCalledWith(
+      "/usr/bin/ssh",
+      expect.arrayContaining(["-L", `${tunnel.localPort}:127.0.0.1:18789`]),
+      expect.anything(),
+    );
+
+    await tunnel.stop();
+  });
 });
diff --git a/src/infra/ssh-tunnel.ts b/src/infra/ssh-tunnel.ts
@@ -4,7 +4,7 @@ import net from "node:net";
 import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization";
 import { formatErrorMessage, isErrno } from "./errors.js";
 import { parseStrictPositiveInteger } from "./parse-finite-number.js";
-import { ensurePortAvailable } from "./ports.js";
+import { ensurePortAvailable, PortInUseError } from "./ports.js";
 
 export type SshParsedTarget = {
   user?: string;
@@ -121,7 +121,7 @@ export async function startSshPortForward(opts: {
   try {
     await ensurePortAvailable(localPort, "127.0.0.1");
   } catch (err) {
-    if (isErrno(err) && err.code === "EADDRINUSE") {
+    if (err instanceof PortInUseError || (isErrno(err) && err.code === "EADDRINUSE")) {
       localPort = await pickEphemeralPort();
     } else {
       throw err;
