From e2e9227431bb101ce17698704db9584ebb780367 Mon Sep 17 00:00:00 2001 From: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com> Date: Tue, 23 Jun 2026 01:31:43 -0600 Subject: [PATCH] fix: harden launchd port release polling --- src/daemon/launchd.test.ts | 53 ++++++++++++++++++++++++----------- src/daemon/launchd.ts | 28 +++++++++++------- src/infra/ports-inspect.ts | 35 ++--------------------- src/infra/ports-probe.test.ts | 10 ++++++- src/infra/ports-probe.ts | 34 ++++++++++++++++++++++ 5 files changed, 100 insertions(+), 60 deletions(-) diff --git a/src/daemon/launchd.test.ts b/src/daemon/launchd.test.ts index 5b5e77049a06..e7415811f3a8 100644 --- a/src/daemon/launchd.test.ts +++ b/src/daemon/launchd.test.ts @@ -65,6 +65,9 @@ const cleanStaleGatewayProcessesSync = vi.hoisted(() => const inspectPortUsage = vi.hoisted(() => vi.fn(async () => ({ port: 18789, status: "free", listeners: [], hints: [] })), ); +const probePortUsage = vi.hoisted(() => + vi.fn(async () => "free"), +); const formatPortDiagnostics = vi.hoisted(() => vi.fn(() => ["Port 18789 is already in use."])); const defaultProgramArguments = ["node", "-e", "process.exit(0)"]; @@ -262,6 +265,10 @@ vi.mock("../infra/ports.js", () => ({ formatPortDiagnostics, })); +vi.mock("../infra/ports-probe.js", () => ({ + probePortUsage, +})); + vi.mock("node:fs/promises", async () => { const actual = await vi.importActual("node:fs/promises"); const wrapped = { @@ -354,6 +361,8 @@ beforeEach(() => { cleanStaleGatewayProcessesSync.mockReturnValue([]); inspectPortUsage.mockReset(); inspectPortUsage.mockResolvedValue({ port: 18789, status: "free", listeners: [], hints: [] }); + probePortUsage.mockReset(); + probePortUsage.mockResolvedValue("free"); formatPortDiagnostics.mockReset(); formatPortDiagnostics.mockReturnValue(["Port 18789 is already in use."]); launchdRestartHandoffState.scheduleDetachedLaunchdRestartHandoff.mockReset(); @@ -1117,25 +1126,35 @@ describe("launchd install", () => { ...createDefaultLaunchdEnv(), OPENCLAW_GATEWAY_PORT: "19009", }; - inspectPortUsage - .mockResolvedValueOnce({ - port: 19009, - status: "busy", - listeners: [], - hints: [], - }) - .mockResolvedValueOnce({ - port: 19009, - status: "free", - listeners: [], - hints: [], - }); + inspectPortUsage.mockResolvedValueOnce({ + port: 19009, + status: "busy", + listeners: [], + hints: [], + }); await runStopLaunchAgentWithFakeTimers({ env, stdout: new PassThrough() }); - expect(inspectPortUsage).toHaveBeenCalledTimes(2); - expect(inspectPortUsage).toHaveBeenNthCalledWith(1, 19009); - expect(inspectPortUsage).toHaveBeenNthCalledWith(2, 19009); + expect(inspectPortUsage).toHaveBeenCalledTimes(1); + expect(probePortUsage).toHaveBeenCalledWith(19009); + }); + + it("keeps waiting until a bind probe explicitly confirms port release", async () => { + const env = { + ...createDefaultLaunchdEnv(), + OPENCLAW_GATEWAY_PORT: "19010", + }; + inspectPortUsage.mockResolvedValueOnce({ + port: 19010, + status: "busy", + listeners: [], + hints: [], + }); + probePortUsage.mockResolvedValueOnce("busy").mockResolvedValueOnce("unknown"); + + await runStopLaunchAgentWithFakeTimers({ env, stdout: new PassThrough() }); + + expect(probePortUsage).toHaveBeenCalledTimes(3); }); it("resolves the stop postcondition port from the stored LaunchAgent environment", async () => { @@ -1170,6 +1189,7 @@ describe("launchd install", () => { listeners: [], hints: [], }); + probePortUsage.mockResolvedValue("busy"); formatPortDiagnostics.mockReturnValue(["Port 19004 is held by pid 4242."]); await expect(runStopLaunchAgentWithFakeTimers({ env, stdout })).rejects.toThrow( @@ -1315,6 +1335,7 @@ describe("launchd install", () => { listeners: [], hints: [], }); + probePortUsage.mockResolvedValue("busy"); formatPortDiagnostics.mockReturnValue(["Port 19008 is held by pid 4242."]); await expect(runStopLaunchAgentWithFakeTimers({ env, stdout, disable: true })).rejects.toThrow( diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index 3deb298665b5..7922cca64601 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -5,6 +5,7 @@ import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/st import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js"; import { normalizeEnvVarKey } from "../infra/host-env-security.js"; import { parseStrictInteger, parseStrictPositiveInteger } from "../infra/parse-finite-number.js"; +import { probePortUsage } from "../infra/ports-probe.js"; import { formatPortDiagnostics, inspectPortUsage } from "../infra/ports.js"; import { cleanStaleGatewayProcessesSync } from "../infra/restart-stale-pids.js"; import { parseTcpPort } from "../infra/tcp-port.js"; @@ -49,7 +50,7 @@ const LAUNCH_AGENT_ENV_DIR_NAME = "service-env"; const LAUNCH_AGENT_STDERR_PATH = "/dev/null"; const OPENCLAW_UPDATE_LAUNCHD_LABEL_PREFIX = "ai.openclaw.update."; const OPENCLAW_MANUAL_UPDATE_LAUNCHD_LABEL_PATTERN = /^ai\.openclaw\.manual-update\.\d+$/; -const LAUNCH_AGENT_STOP_PORT_RELEASE_ATTEMPTS = 20; +const LAUNCH_AGENT_STOP_PORT_RELEASE_TIMEOUT_MS = 2_000; const LAUNCH_AGENT_STOP_PORT_RELEASE_POLL_MS = 100; export type StaleOpenClawUpdateLaunchdJob = { @@ -773,24 +774,31 @@ async function waitForLaunchAgentStopped(serviceTarget: string): Promise { + const deadline = Date.now() + LAUNCH_AGENT_STOP_PORT_RELEASE_TIMEOUT_MS; + while (Date.now() < deadline) { + await sleep(Math.min(LAUNCH_AGENT_STOP_PORT_RELEASE_POLL_MS, deadline - Date.now())); + const status = await probePortUsage(port); + if (status === "free") { + return true; + } + } + return false; +} + async function assertGatewayPortReleasedAfterStop(env: GatewayServiceEnv): Promise { const port = await resolveLaunchAgentGatewayPort(env); if (port === null) { return; } cleanStaleGatewayProcessesSync(port); - let diagnostics = await inspectPortUsage(port).catch(() => null); - for ( - let attempt = 1; - diagnostics?.status === "busy" && attempt < LAUNCH_AGENT_STOP_PORT_RELEASE_ATTEMPTS; - attempt += 1 - ) { - await sleep(LAUNCH_AGENT_STOP_PORT_RELEASE_POLL_MS); - diagnostics = await inspectPortUsage(port).catch(() => null); - } + const diagnostics = await inspectPortUsage(port).catch(() => null); if (diagnostics?.status !== "busy") { return; } + if (await waitForGatewayPortRelease(port)) { + return; + } throw new Error( [ `gateway port ${port} is still busy after LaunchAgent stop`, diff --git a/src/infra/ports-inspect.ts b/src/infra/ports-inspect.ts index 4419ed7e83dd..4395e80e2f1a 100644 --- a/src/infra/ports-inspect.ts +++ b/src/infra/ports-inspect.ts @@ -2,11 +2,10 @@ import os from "node:os"; import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import { runCommandWithTimeout } from "../process/exec.js"; -import { isErrno } from "./errors.js"; import { parseStrictPositiveInteger } from "./parse-finite-number.js"; import { buildPortHints } from "./ports-format.js"; import { resolveLsofCommand } from "./ports-lsof.js"; -import { tryListenOnPort } from "./ports-probe.js"; +import { probePortUsage } from "./ports-probe.js"; import type { PortConnection, PortConnectionDirection, @@ -610,36 +609,6 @@ async function readWindowsEstablishedConnections( return { connections: result.entries, detail: result.detail, errors: result.errors }; } -async function tryListenOnHost(port: number, host: string): Promise { - try { - await tryListenOnPort({ port, host, exclusive: true }); - return "free"; - } catch (err) { - if (isErrno(err) && err.code === "EADDRINUSE") { - return "busy"; - } - if (isErrno(err) && (err.code === "EADDRNOTAVAIL" || err.code === "EAFNOSUPPORT")) { - return "skip"; - } - return "unknown"; - } -} - -async function checkPortInUse(port: number): Promise { - const hosts = ["127.0.0.1", "0.0.0.0", "::1", "::"]; - let sawUnknown = false; - for (const host of hosts) { - const result = await tryListenOnHost(port, host); - if (result === "busy") { - return "busy"; - } - if (result === "unknown") { - sawUnknown = true; - } - } - return sawUnknown ? "unknown" : "free"; -} - export async function inspectPortUsage(port: number): Promise { const errors: string[] = []; const result = @@ -648,7 +617,7 @@ export async function inspectPortUsage(port: number): Promise { let listeners = result.listeners; let status: PortUsageStatus = listeners.length > 0 ? "busy" : "unknown"; if (listeners.length === 0) { - status = await checkPortInUse(port); + status = await probePortUsage(port); } if (status !== "busy") { listeners = []; diff --git a/src/infra/ports-probe.test.ts b/src/infra/ports-probe.test.ts index 4844462d27f4..54685b576b00 100644 --- a/src/infra/ports-probe.test.ts +++ b/src/infra/ports-probe.test.ts @@ -1,7 +1,7 @@ // Tests local port probing and availability detection. import net from "node:net"; import { describe, expect, it } from "vitest"; -import { tryListenOnPort } from "./ports-probe.js"; +import { probePortUsage, tryListenOnPort } from "./ports-probe.js"; async function withListeningServer(cb: (address: net.AddressInfo) => Promise): Promise { const server = net.createServer(); @@ -65,3 +65,11 @@ describe("tryListenOnPort", () => { }); }); }); + +describe("probePortUsage", () => { + it("reports an IPv4-only loopback listener as busy", async () => { + await withListeningServer(async (address) => { + await expect(probePortUsage(address.port)).resolves.toBe("busy"); + }); + }); +}); diff --git a/src/infra/ports-probe.ts b/src/infra/ports-probe.ts index b9c3e8fee0c2..6284f8c83289 100644 --- a/src/infra/ports-probe.ts +++ b/src/infra/ports-probe.ts @@ -1,5 +1,9 @@ // Probes local ports and reports listener availability. import net from "node:net"; +import { isErrno } from "./errors.js"; +import type { PortUsageStatus } from "./ports-types.js"; + +const PORT_PROBE_HOSTS = ["127.0.0.1", "0.0.0.0", "::1", "::"]; /** Opens and closes a temporary listener to verify that a port can be bound. */ export async function tryListenOnPort(params: { @@ -28,3 +32,33 @@ export async function tryListenOnPort(params: { .listen(listenOptions); }); } + +async function probePortOnHost(port: number, host: string): Promise { + try { + await tryListenOnPort({ port, host, exclusive: true }); + return "free"; + } catch (err) { + if (isErrno(err) && err.code === "EADDRINUSE") { + return "busy"; + } + if (isErrno(err) && (err.code === "EADDRNOTAVAIL" || err.code === "EAFNOSUPPORT")) { + return "skip"; + } + return "unknown"; + } +} + +/** Checks all supported local address families without resolving listener diagnostics. */ +export async function probePortUsage(port: number): Promise { + let sawUnknown = false; + for (const host of PORT_PROBE_HOSTS) { + const result = await probePortOnHost(port, host); + if (result === "busy") { + return "busy"; + } + if (result === "unknown") { + sawUnknown = true; + } + } + return sawUnknown ? "unknown" : "free"; +}