From 2fc260aa09584d8a4b9d97eabb6415c2e3e565af Mon Sep 17 00:00:00 2001 From: sunlit-deng Date: Wed, 24 Jun 2026 10:45:06 +0800 Subject: [PATCH] fix(ports): route isPortBusy through checkPortInUse to catch IPv4-only occupants (#94949) * fix(ports): route isPortBusy through checkPortInUse to catch IPv4-only occupants * fix(ports): treat PortUsageStatus unknown as busy in isPortBusy Per ClawSweeper review: checkPortInUse returns 'unknown' when every host probe fails for a non-EADDRINUSE reason. Treating unknown as 'not busy' could cause forceFreePortAndWait to exit before lsof/fuser inspects the port. Conservative fix: only 'free' means not busy; everything else (busy or unknown) triggers further inspection. * fix(ports): reuse canonical multi-address probe * fix(ports): reuse canonical multi-address probe --------- Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com> --- src/cli/ports.ts | 18 +++++++----------- src/cli/program.force.test.ts | 31 ++++++++++++++----------------- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/cli/ports.ts b/src/cli/ports.ts index cfd72f68442..47563665ab4 100644 --- a/src/cli/ports.ts +++ b/src/cli/ports.ts @@ -3,7 +3,7 @@ import { execFileSync } from "node:child_process"; import { createServer } from "node:net"; import { formatErrorMessage } from "../infra/errors.js"; import { resolveLsofCommandSync } from "../infra/ports-lsof.js"; -import { tryListenOnPort } from "../infra/ports-probe.js"; +import { probePortUsage } from "../infra/ports-probe.js"; import { getWindowsSystem32ExePath } from "../infra/windows-install-roots.js"; import { resolvePositiveTimerTimeoutMs, resolveTimerTimeoutMs } from "../shared/number-coercion.js"; import { sleep } from "../utils.js"; @@ -131,16 +131,12 @@ function killPortWithFuser(port: number, signal: "SIGTERM" | "SIGKILL"): PortPro } async function isPortBusy(port: number): Promise { - try { - await tryListenOnPort({ port, exclusive: true }); - return false; - } catch (err: unknown) { - const code = (err as NodeJS.ErrnoException).code; - if (code === "EADDRINUSE") { - return true; - } - throw err instanceof Error ? err : new Error(String(err)); - } + // Route through probePortUsage which probes all four endpoints + // (127.0.0.1, 0.0.0.0, ::1, ::) instead of a single hostless bind + // that defaults to IPv6 wildcard and misses IPv4-only occupants. + // Treat "unknown" as busy — inconclusive probe failures must not cause + // forceFreePortAndWait to exit early before lsof/fuser can inspect. + return (await probePortUsage(port)) !== "free"; } export function parseLsofOutput(output: string): PortProcess[] { diff --git a/src/cli/program.force.test.ts b/src/cli/program.force.test.ts index 53d44174dda..74ee37c2fd7 100644 --- a/src/cli/program.force.test.ts +++ b/src/cli/program.force.test.ts @@ -9,10 +9,10 @@ vi.mock("node:child_process", async () => { }; }); -const tryListenOnPortMock = vi.hoisted(() => vi.fn()); +const probePortUsageMock = vi.hoisted(() => vi.fn()); vi.mock("../infra/ports-probe.js", () => ({ - tryListenOnPort: (...args: unknown[]) => tryListenOnPortMock(...args), + probePortUsage: (...args: unknown[]) => probePortUsageMock(...args), })); import { execFileSync } from "node:child_process"; @@ -33,10 +33,8 @@ describe("gateway --force helpers", () => { vi.clearAllMocks(); originalKill = process.kill.bind(process); originalPlatform = process.platform; - tryListenOnPortMock.mockReset(); - tryListenOnPortMock.mockRejectedValue( - Object.assign(new Error("in use"), { code: "EADDRINUSE" }), - ); + probePortUsageMock.mockReset(); + probePortUsageMock.mockResolvedValue("busy"); // Pin to linux so all lsof tests are platform-invariant. Object.defineProperty(process, "platform", { value: "linux", configurable: true }); }); @@ -65,7 +63,7 @@ describe("gateway --force helpers", () => { }); it("skips lsof when the port is already bindable", async () => { - tryListenOnPortMock.mockResolvedValue(undefined); + probePortUsageMock.mockResolvedValue("free"); const result = await forceFreePortAndWait(18789, { timeoutMs: 500, intervalMs: 100 }); @@ -199,9 +197,7 @@ describe("gateway --force helpers", () => { } return "18789/tcp: 4242\n"; }); - tryListenOnPortMock - .mockRejectedValueOnce(Object.assign(new Error("in use"), { code: "EADDRINUSE" })) - .mockResolvedValue(undefined); + probePortUsageMock.mockResolvedValueOnce("busy").mockResolvedValue("free"); const result = await forceFreePortAndWait(18789, { timeoutMs: 500, intervalMs: 100 }); @@ -231,13 +227,12 @@ describe("gateway --force helpers", () => { return ""; }); - const busyErr = Object.assign(new Error("in use"), { code: "EADDRINUSE" }); - tryListenOnPortMock - .mockRejectedValueOnce(busyErr) - .mockRejectedValueOnce(busyErr) - .mockRejectedValueOnce(busyErr) - .mockRejectedValueOnce(busyErr) - .mockResolvedValueOnce(undefined); + probePortUsageMock + .mockResolvedValueOnce("busy") + .mockResolvedValueOnce("busy") + .mockResolvedValueOnce("busy") + .mockResolvedValueOnce("busy") + .mockResolvedValue("free"); const promise = forceFreePortAndWait(18789, { timeoutMs: 300, @@ -258,6 +253,8 @@ describe("gateway --force helpers", () => { }); it("throws when lsof is unavailable and fuser is missing", async () => { + // An inconclusive four-host probe must continue into the cleanup tools. + probePortUsageMock.mockResolvedValue("unknown"); (execFileSync as unknown as Mock).mockImplementation((cmd: string) => { const err = new Error(`spawnSync ${cmd} ENOENT`) as NodeJS.ErrnoException; err.code = "ENOENT";