fix: fast-fail dev scripts when ports are already in use (#939)

* fix: fast-fail dev scripts when ports are already in use

Add `assertPortsFree` to `scripts/dev-safe.mjs` that checks each
preferred port and throws a descriptive error if any are already bound,
rather than silently binding to an alternative port.

- `buildSafeDevConfigAsync` now calls `assertPortsFree` before
  returning config, so `npm run dev:minimal` exits immediately with a
  clear message when the agent-server port is occupied.
- `dev-with-automation.mjs`'s `buildConfig` does the same for all four
  service ports (ingress, agent-server, automation, vite), covering
  `npm run dev` and `npm run dev:static`.
- `buildConfig` gains env-var overrides for internal service ports
  (`OH_CANVAS_SAFE_BACKEND_PORT`, `OH_CANVAS_SAFE_AUTOMATION_PORT`,
  `OH_CANVAS_SAFE_VITE_PORT`) so tests and advanced users can redirect
  ports without touching production defaults.

Tests updated accordingly:
- Old "falls back when port is busy" tests replaced with "throws when
  port is busy" equivalents.
- New `assertPortsFree` describe block with three targeted cases.
- `envWithIsolatedKeyPath` in `dev-with-automation.test.ts` now seeds
  high free ports so the pre-flight check passes when a real dev stack
  is running during local test execution.

Closes #934

Co-authored-by: openhands <openhands@all-hands.dev>

* refactor: address review bot suggestions on assertPortsFree

- Check ports in parallel with Promise.all instead of sequentially
  (each check is independent I/O, so parallel is faster and more idiomatic)
- Include vscode port in the buildSafeDevConfigAsync pre-flight check
  alongside the agent-server port, so a conflict on that internal port
  is also caught with a helpful message rather than a cryptic spawn error

Co-authored-by: openhands <openhands@all-hands.dev>

---------

Co-authored-by: openhands <openhands@all-hands.dev>

__tests__/scripts/dev-safe.test.ts

@@ -15,6 +15,7 @@ import { setTimeout as delay } from "node:timers/promises";
 import { fileURLToPath } from "node:url";
 import { describe, expect, it, afterEach } from "vitest";
 import {
+  assertPortsFree,
   buildSafeDevConfig,
   buildSafeDevConfigAsync,
   buildNpmScriptCommand,
@@ -192,7 +193,10 @@ describe("buildSafeDevConfigAsync", () => {
   }
 
   it("returns config with dynamically allocated ports", async () => {
+    // Use a high port so the assertPortsFree check passes even when a real
+    // dev stack is running on the default port (18000).
     const config = await buildSafeDevConfigAsync(repoRoot, {
+      OH_CANVAS_SAFE_BACKEND_PORT: "19800",
       OH_SESSION_API_KEY_PATH: tempKeyPath(),
     });
 
@@ -204,7 +208,7 @@ describe("buildSafeDevConfigAsync", () => {
     expect(config.backendPort).not.toBe(config.vscodePort);
   });
 
-  it("falls back when preferred ports are busy", async () => {
+  it("throws when preferred port is busy", async () => {
     // Block a specific high port we'll request
     const busyPort = 19600;
     const server = net.createServer();
@@ -216,16 +220,82 @@ describe("buildSafeDevConfigAsync", () => {
       server.on("error", reject);
     });
 
-    // Request the busy port via env var
-    const config = await buildSafeDevConfigAsync(repoRoot, {
-      OH_CANVAS_SAFE_BACKEND_PORT: busyPort.toString(),
-      OH_SESSION_API_KEY_PATH: tempKeyPath(),
+    // Request the busy port via env var — should throw instead of falling back
+    await expect(
+      buildSafeDevConfigAsync(repoRoot, {
+        OH_CANVAS_SAFE_BACKEND_PORT: busyPort.toString(),
+        OH_SESSION_API_KEY_PATH: tempKeyPath(),
+      }),
+    ).rejects.toThrow(/agent-server.*port 19600/i);
+  });
+});
+
+describe("assertPortsFree", () => {
+  const servers: net.Server[] = [];
+
+  afterEach(() => {
+    for (const server of servers) {
+      server.close();
+    }
+    servers.length = 0;
+  });
+
+  it("resolves when all ports are free", async () => {
+    // High ports unlikely to be in use
+    await expect(
+      assertPortsFree([
+        { name: "svc-a", port: 19700 },
+        { name: "svc-b", port: 19701 },
+      ]),
+    ).resolves.toBeUndefined();
+  });
+
+  it("throws when a single port is busy", async () => {
+    const busyPort = await new Promise<number>((resolve, reject) => {
+      const server = net.createServer();
+      server.listen(0, "127.0.0.1", () => {
+        const addr = server.address();
+        if (addr && typeof addr === "object") {
+          servers.push(server);
+          resolve(addr.port);
+        } else {
+          server.close();
+          reject(new Error("Failed to get address"));
+        }
+      });
     });
 
-    // Backend port should NOT be busyPort since it's taken
-    expect(config.backendPort).not.toBe(busyPort);
-    expect(typeof config.backendPort).toBe("number");
-    expect(config.backendPort).toBeGreaterThan(0);
+    await expect(
+      assertPortsFree([{ name: "agent-server", port: busyPort }]),
+    ).rejects.toThrow(/agent-server.*port/i);
+  });
+
+  it("names all busy ports in the error message", async () => {
+    const [portA, portB] = await Promise.all(
+      [0, 0].map(
+        () =>
+          new Promise<number>((resolve, reject) => {
+            const server = net.createServer();
+            server.listen(0, "127.0.0.1", () => {
+              const addr = server.address();
+              if (addr && typeof addr === "object") {
+                servers.push(server);
+                resolve(addr.port);
+              } else {
+                server.close();
+                reject(new Error("Failed to get address"));
+              }
+            });
+          }),
+      ),
+    );
+
+    await expect(
+      assertPortsFree([
+        { name: "ingress", port: portA },
+        { name: "vite", port: portB },
+      ]),
+    ).rejects.toThrow(/ingress.*vite|vite.*ingress/is);
   });
 });
 
@@ -734,8 +804,13 @@ describe("dev-safe CLI startup", () => {
     const child = spawn(process.execPath, ["scripts/dev-safe.mjs"], {
       cwd: repoRoot,
       env: {
-        // Use empty PATH to ensure uvx is not found
+        // Use empty PATH to ensure uvx is not found.
         PATH: "",
+        // Redirect the agent-server port to a high free port so the
+        // assertPortsFree pre-flight check passes when a real dev stack is
+        // running on the default port (18000) — the test is about uvx, not
+        // port detection.
+        OH_CANVAS_SAFE_BACKEND_PORT: "19810",
       },
       stdio: ["ignore", "pipe", "pipe"],
     });

__tests__/scripts/dev-with-automation.test.ts

@@ -145,6 +145,10 @@ describe("buildConfig", () => {
   /**
    * Build an env that points persisted dev API key files at a fresh temp dir,
    * so tests don't write to the user's real ~/.openhands/agent-canvas files.
+   *
+   * Also redirects all service ports to high port numbers so that buildConfig's
+   * assertPortsFree check passes even when a real dev stack is running on the
+   * default ports (18000, 18001, 3001, 8000).
    */
   function envWithIsolatedKeyPath(
     extra: Record<string, string> = {},
@@ -153,6 +157,11 @@ describe("buildConfig", () => {
     keyDirs.push(dir);
     return {
       OH_SESSION_API_KEY_PATH: path.join(dir, "session-api-key.txt"),
+      // High ports that are almost certainly free, so assertPortsFree passes.
+      PORT: "19902",
+      OH_CANVAS_SAFE_BACKEND_PORT: "19900",
+      OH_CANVAS_SAFE_AUTOMATION_PORT: "19901",
+      OH_CANVAS_SAFE_VITE_PORT: "19903",
       ...extra,
     };
   }
@@ -192,7 +201,7 @@ describe("buildConfig", () => {
     expect(config.ingressPort).toBe(preferredPort);
   });
 
-  it("falls back to alternative port when ingress port is busy", async () => {
+  it("throws when ingress port is busy", async () => {
     const busyPort = 8100;
 
     // Block port 8100
@@ -205,15 +214,10 @@ describe("buildConfig", () => {
       server.on("error", reject);
     });
 
-    // Request the busy port
-    const config = await buildConfig(
-      { port: busyPort },
-      envWithIsolatedKeyPath(),
-    );
-
-    // Should get a different port since busyPort is taken
-    expect(config.ingressPort).not.toBe(busyPort);
-    expect(config.ingressPort).toBeGreaterThan(0);
+    // Should throw instead of falling back to a different port
+    await expect(
+      buildConfig({ port: busyPort }, envWithIsolatedKeyPath()),
+    ).rejects.toThrow(/ingress.*port 8100/i);
   });
 
   it("allocates valid ports for all services", async () => {
@@ -315,7 +319,10 @@ describe("buildConfig", () => {
   });
 
   it("reads sessionApiKey from SESSION_API_KEY", async () => {
-    const config = await buildConfig({}, { SESSION_API_KEY: "my-session-key" });
+    const config = await buildConfig(
+      {},
+      { ...envWithIsolatedKeyPath(), SESSION_API_KEY: "my-session-key" },
+    );
 
     expect(config.sessionApiKey).toBe("my-session-key");
   });
@@ -323,7 +330,7 @@ describe("buildConfig", () => {
   it("reads sessionApiKey from VITE_SESSION_API_KEY as fallback", async () => {
     const config = await buildConfig(
       {},
-      { VITE_SESSION_API_KEY: "vite-session-key" },
+      { ...envWithIsolatedKeyPath(), VITE_SESSION_API_KEY: "vite-session-key" },
     );
 
     expect(config.sessionApiKey).toBe("vite-session-key");
@@ -333,6 +340,7 @@ describe("buildConfig", () => {
     const config = await buildConfig(
       {},
       {
+        ...envWithIsolatedKeyPath(),
         SESSION_API_KEY: "session-key",
         VITE_SESSION_API_KEY: "vite-key",
       },
@@ -344,7 +352,7 @@ describe("buildConfig", () => {
   it("reads sessionApiKey from OH_SESSION_API_KEYS_0 (agent-server V1 env)", async () => {
     const config = await buildConfig(
       {},
-      { OH_SESSION_API_KEYS_0: "v1-session-key" },
+      { ...envWithIsolatedKeyPath(), OH_SESSION_API_KEYS_0: "v1-session-key" },
     );
 
     expect(config.sessionApiKey).toBe("v1-session-key");
@@ -354,6 +362,7 @@ describe("buildConfig", () => {
     const config = await buildConfig(
       {},
       {
+        ...envWithIsolatedKeyPath(),
         SESSION_API_KEY: "v0-key",
         OH_SESSION_API_KEYS_0: "v1-key",
       },
@@ -366,6 +375,7 @@ describe("buildConfig", () => {
     const config = await buildConfig(
       {},
       {
+        ...envWithIsolatedKeyPath(),
         SESSION_API_KEY: "v0-key",
         OH_SESSION_API_KEYS_0: "v1-key",
         VITE_SESSION_API_KEY: "vite-key",

scripts/dev-safe.mjs

@@ -210,6 +210,36 @@ function tryPort(port, host = "127.0.0.1") {
   });
 }
 
+/**
+ * Assert that all listed ports are available, throwing a descriptive error if
+ * any are already in use.
+ *
+ * Intended as a pre-flight check before spawning services so that a concurrent
+ * agent-canvas instance is detected immediately rather than silently starting
+ * on a different port.
+ *
+ * @param {Array<{name: string, port: number}>} portConfigs - Named port list
+ * @param {string} [host]
+ */
+export async function assertPortsFree(portConfigs, host = "127.0.0.1") {
+  const results = await Promise.all(
+    portConfigs.map(async ({ name, port }) => ({
+      name,
+      port,
+      free: await tryPort(port, host),
+    })),
+  );
+  const busy = results.filter(({ free }) => !free);
+  if (busy.length === 0) return;
+
+  const lines = busy.map(({ name, port }) => `   • ${name}: port ${port}`).join("\n");
+  throw new Error(
+    `Cannot start: the following ports are already in use:\n\n${lines}\n\n` +
+      `Another agent-canvas instance may already be running.\n` +
+      `Stop it first, or override the port via environment variables (e.g. PORT=<other>).`,
+  );
+}
+
 /**
  * Find multiple free ports at once, each preferring its specified default.
  *
@@ -491,26 +521,14 @@ export async function buildSafeDevConfigAsync(
     preferredBackendPort + 1,
   );
 
-  // Find available ports, preferring the defaults
-  const ports = await findFreePorts([
-    { name: "backend", preferred: preferredBackendPort },
-    { name: "vscode", preferred: preferredVscodePort },
+  // Fail fast if any required port is already in use.
+  await assertPortsFree([
+    { name: "agent-server", port: preferredBackendPort },
+    { name: "vscode", port: preferredVscodePort },
   ]);
 
-  // Log if we're using non-default ports
-  if (ports.backend !== preferredBackendPort) {
-    console.log(
-      `  ℹ Port ${preferredBackendPort} busy, using ${ports.backend} for agent-server`,
-    );
-  }
-  if (ports.vscode !== preferredVscodePort) {
-    console.log(
-      `  ℹ Port ${preferredVscodePort} busy, using ${ports.vscode} for vscode`,
-    );
-  }
-
   return buildConfigFromPorts(
-    { backendPort: ports.backend, vscodePort: ports.vscode },
+    { backendPort: preferredBackendPort, vscodePort: preferredVscodePort },
     cwd,
     env,
   );

scripts/dev-with-automation.mjs

@@ -51,13 +51,13 @@ import { setTimeout as delay } from "node:timers/promises";
 import process from "node:process";
 
 import {
+  assertPortsFree,
   buildAgentServerCommand,
   buildSafeDevConfig,
   buildAgentServerEnv,
   buildNpmScriptCommand,
   buildRuntimeServicesInfo,
   formatMissingUvxGuidance,
-  findFreePorts,
   getOrCreatePersistedApiKey,
   validateFrontendDependencies,
   validateLocalAgentServerPath,
@@ -281,52 +281,27 @@ async function buildConfig(args, env = process.env) {
     env.OH_AUTOMATION_REPO = args.automationRepo;
   }
 
-  // Preferred ports (from env or defaults)
+  // Preferred ports (from env or defaults).
+  // OH_CANVAS_SAFE_BACKEND_PORT / OH_CANVAS_SAFE_AUTOMATION_PORT /
+  // OH_CANVAS_SAFE_VITE_PORT allow tests (and advanced users) to redirect
+  // internal service ports without affecting the production default.
   const preferredIngressPort = args.port || parseInt(env.PORT, 10) || 8000;
-  const preferredBackendPort = DEFAULT_BACKEND_PORT;
-  const preferredAutomationPort = DEFAULT_AUTOMATION_PORT;
-  const preferredVitePort = 3001;
+  const preferredBackendPort =
+    parseInt(env.OH_CANVAS_SAFE_BACKEND_PORT, 10) || DEFAULT_BACKEND_PORT;
+  const preferredAutomationPort =
+    parseInt(env.OH_CANVAS_SAFE_AUTOMATION_PORT, 10) || DEFAULT_AUTOMATION_PORT;
+  const preferredVitePort = parseInt(env.OH_CANVAS_SAFE_VITE_PORT, 10) || 3001;
 
-  // Find available ports, preferring the defaults
-  logStep("ports", "Allocating ports...");
-  const ports = await findFreePorts([
-    { name: "ingress", preferred: preferredIngressPort },
-    { name: "backend", preferred: preferredBackendPort },
-    { name: "automation", preferred: preferredAutomationPort },
-    { name: "vite", preferred: preferredVitePort },
+  // Fail fast if any preferred port is already in use.
+  logStep("ports", "Checking ports...");
+  await assertPortsFree([
+    { name: "ingress", port: preferredIngressPort },
+    { name: "agent-server", port: preferredBackendPort },
+    { name: "automation", port: preferredAutomationPort },
+    { name: "vite", port: preferredVitePort },
   ]);
 
-  // Log any port changes
-  if (ports.ingress !== preferredIngressPort) {
-    logService(
-      "ports",
-      `Port ${preferredIngressPort} busy, using ${ports.ingress} for ingress`,
-      c.yellow,
-    );
-  }
-  if (ports.backend !== preferredBackendPort) {
-    logService(
-      "ports",
-      `Port ${preferredBackendPort} busy, using ${ports.backend} for agent-server`,
-      c.yellow,
-    );
-  }
-  if (ports.automation !== preferredAutomationPort) {
-    logService(
-      "ports",
-      `Port ${preferredAutomationPort} busy, using ${ports.automation} for automation`,
-      c.yellow,
-    );
-  }
-  if (ports.vite !== preferredVitePort) {
-    logService(
-      "ports",
-      `Port ${preferredVitePort} busy, using ${ports.vite} for vite`,
-      c.yellow,
-    );
-  }
-
-  const vscodePort = ports.backend + 1000;
+  const vscodePort = preferredBackendPort + 1000;
 
   // Session API key — shared by both agent-server and automation backend.
   // Both validate it via the `X-Session-API-Key` header.
@@ -336,19 +311,19 @@ async function buildConfig(args, env = process.env) {
   const safeConfig = buildSafeDevConfig(projectRoot, {
     ...env,
     OH_CANVAS_SAFE_STATE_DIR: stateDir,
-    OH_CANVAS_SAFE_BACKEND_PORT: ports.backend.toString(),
+    OH_CANVAS_SAFE_BACKEND_PORT: preferredBackendPort.toString(),
     OH_CANVAS_SAFE_VSCODE_PORT: vscodePort.toString(),
   });
   const sessionApiKey = safeConfig.sessionApiKey;
 
   return {
     // Ingress port (main entry point)
-    ingressPort: ports.ingress,
+    ingressPort: preferredIngressPort,
 
     // Service ports (internal)
-    agentServerPort: ports.backend,
-    autoBackendPort: ports.automation,
-    vitePort: ports.vite,
+    agentServerPort: preferredBackendPort,
+    autoBackendPort: preferredAutomationPort,
+    vitePort: preferredVitePort,
     vscodePort,
 
     // Paths