Files
openclaw/src/cli/ports.test.ts
Tak Hoffman 1be39d4250 fix(gateway): synthesize lifecycle robustness for restart and startup probes (#33831)
* fix(gateway): correct launchctl command sequence for gateway restart (closes #20030)

* fix(restart): expand HOME and escape label in launchctl plist path

* fix(restart): poll port free after SIGKILL to prevent EADDRINUSE restart loop

When cleanStaleGatewayProcessesSync() kills a stale gateway process,
the kernel may not immediately release the TCP port. Previously the
function returned after a fixed 500ms sleep (300ms SIGTERM + 200ms
SIGKILL), allowing triggerOpenClawRestart() to hand off to systemd
before the port was actually free. The new systemd process then raced
the dying socket for port 18789, hit EADDRINUSE, and exited with
status 1, causing systemd to retry indefinitely — the zombie restart
loop reported in #33103.

Fix: add waitForPortFreeSync() that polls lsof at 50ms intervals for
up to 2 seconds after SIGKILL. cleanStaleGatewayProcessesSync() now
blocks until the port is confirmed free (or the budget expires with a
warning) before returning. The increased SIGTERM/SIGKILL wait budgets
(600ms / 400ms) also give slow processes more time to exit cleanly.

Fixes #33103
Related: #28134

* fix: add EADDRINUSE retry and TIME_WAIT port-bind checks for gateway startup

* fix(ports): treat EADDRNOTAVAIL as non-retryable and fix flaky test

* fix(gateway): hot-reload agents.defaults.models allowlist changes

The reload plan had a rule for `agents.defaults.model` (singular) but
not `agents.defaults.models` (plural — the allowlist array).  Because
`agents.defaults.models` does not prefix-match `agents.defaults.model.`,
it fell through to the catch-all `agents` tail rule (kind=none), so
allowlist edits in openclaw.json were silently ignored at runtime.

Add a dedicated reload rule so changes to the models allowlist trigger
a heartbeat restart, which re-reads the config and serves the updated
list to clients.

Fixes #33600

Co-authored-by: HCL <chenglunhu@gmail.com>
Signed-off-by: HCL <chenglunhu@gmail.com>

* test(restart): 100% branch coverage — audit round 2

Audit findings fixed:
- remove dead guard: terminateStaleProcessesSync pids.length===0 check was
  unreachable (only caller cleanStaleGatewayProcessesSync already guards)
- expose __testing.callSleepSyncRaw so sleepSync's real Atomics.wait path
  can be unit-tested directly without going through the override
- fix broken sleepSync Atomics.wait test: previous test set override=null
  but cleanStaleGatewayProcessesSync returned before calling sleepSync —
  replaced with direct callSleepSyncRaw calls that actually exercise L36/L42-47
- fix pid collision: two tests used process.pid+304 (EPERM + dead-at-SIGTERM);
  EPERM test changed to process.pid+305
- fix misindented tests: 'deduplicates pids' and 'lsof status 1 container
  edge case' were outside their intended describe blocks; moved to correct
  scopes (findGatewayPidsOnPortSync and pollPortOnce respectively)
- add missing branch tests:
  - status 1 + non-empty stdout with zero openclaw pids → free:true (L145)
  - mid-loop non-openclaw cmd in &&-chain (L67)
  - consecutive p-lines without c-line between them (L67)
  - invalid PID in p-line (p0 / pNaN) — ternary false branch (L67)
  - unknown lsof output line (else-if false branch L69)

Coverage: 100% stmts / 100% branch / 100% funcs / 100% lines (36 tests)

* test(restart): fix stale-pid test typing for tsgo

* fix(gateway): address lifecycle review findings

* test(update): make restart-helper path assertions windows-safe

---------

Signed-off-by: HCL <chenglunhu@gmail.com>
Co-authored-by: Glucksberg <markuscontasul@gmail.com>
Co-authored-by: Efe Büken <efe@arven.digital>
Co-authored-by: Riccardo Marino <rmarino@apple.com>
Co-authored-by: HCL <chenglunhu@gmail.com>
2026-03-03 21:31:12 -06:00

125 lines
5.1 KiB
TypeScript

import { EventEmitter } from "node:events";
import net from "node:net";
import { describe, expect, it, vi } from "vitest";
// Hoist the factory so vi.mock can access it.
const mockCreateServer = vi.hoisted(() => vi.fn());
vi.mock("node:net", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:net")>();
return { ...actual, createServer: mockCreateServer };
});
import { probePortFree, waitForPortBindable } from "./ports.js";
/** Build a minimal fake net.Server that emits a given error code on listen(). */
function makeErrServer(code: string): net.Server {
const err = Object.assign(new Error(`bind error: ${code}`), {
code,
}) as NodeJS.ErrnoException;
const fake = new EventEmitter() as unknown as net.Server;
(fake as unknown as { close: (cb?: () => void) => net.Server }).close = (cb?: () => void) => {
cb?.();
return fake;
};
(fake as unknown as { unref: () => net.Server }).unref = () => fake;
(fake as unknown as { listen: (...args: unknown[]) => net.Server }).listen = (
..._args: unknown[]
) => {
setImmediate(() => fake.emit("error", err));
return fake;
};
return fake;
}
describe("probePortFree", () => {
it("resolves false (not rejects) when bind returns EADDRINUSE", async () => {
mockCreateServer.mockReturnValue(makeErrServer("EADDRINUSE"));
await expect(probePortFree(9999, "127.0.0.1")).resolves.toBe(false);
});
it("rejects immediately for EADDRNOTAVAIL (non-retryable: host address not on any interface)", async () => {
mockCreateServer.mockReturnValue(makeErrServer("EADDRNOTAVAIL"));
await expect(probePortFree(9999, "192.0.2.1")).rejects.toMatchObject({ code: "EADDRNOTAVAIL" });
});
it("rejects immediately for EACCES (non-retryable bind error)", async () => {
mockCreateServer.mockReturnValue(makeErrServer("EACCES"));
await expect(probePortFree(80, "0.0.0.0")).rejects.toMatchObject({ code: "EACCES" });
});
it("rejects immediately for other non-retryable errors", async () => {
mockCreateServer.mockReturnValue(makeErrServer("EINVAL"));
await expect(probePortFree(9999, "0.0.0.0")).rejects.toMatchObject({ code: "EINVAL" });
});
it("resolves true when the port is free", async () => {
// Mock a successful bind: the "listening" event fires immediately without
// acquiring a real socket, making this deterministic and avoiding TOCTOU races.
// (A real-socket approach would bind to :0, release, then reprobe — the OS can
// reassign the ephemeral port in between, causing a flaky EADDRINUSE failure.)
const fakeServer = new EventEmitter() as unknown as net.Server;
(fakeServer as unknown as { close: (cb?: () => void) => net.Server }).close = (
cb?: () => void,
) => {
cb?.();
return fakeServer;
};
(fakeServer as unknown as { unref: () => net.Server }).unref = () => fakeServer;
(fakeServer as unknown as { listen: (...args: unknown[]) => net.Server }).listen = (
..._args: unknown[]
) => {
// Simulate a successful bind by firing the "listening" callback.
const callback = _args.find((a) => typeof a === "function") as (() => void) | undefined;
setImmediate(() => callback?.());
return fakeServer;
};
mockCreateServer.mockReturnValue(fakeServer);
const result = await probePortFree(9999, "127.0.0.1");
expect(result).toBe(true);
});
});
describe("waitForPortBindable", () => {
it("probes the provided host when waiting for bindability", async () => {
const listenCalls: Array<{ port: number; host: string }> = [];
const fakeServer = new EventEmitter() as unknown as net.Server;
(fakeServer as unknown as { close: (cb?: () => void) => net.Server }).close = (
cb?: () => void,
) => {
cb?.();
return fakeServer;
};
(fakeServer as unknown as { unref: () => net.Server }).unref = () => fakeServer;
(fakeServer as unknown as { listen: (...args: unknown[]) => net.Server }).listen = (
...args: unknown[]
) => {
const [port, host] = args as [number, string];
listenCalls.push({ port, host });
const callback = args.find((a) => typeof a === "function") as (() => void) | undefined;
setImmediate(() => callback?.());
return fakeServer;
};
mockCreateServer.mockReturnValue(fakeServer);
await expect(
waitForPortBindable(9999, { timeoutMs: 100, intervalMs: 10, host: "127.0.0.1" }),
).resolves.toBe(0);
expect(listenCalls[0]).toEqual({ port: 9999, host: "127.0.0.1" });
});
it("propagates EACCES rejection immediately without retrying", async () => {
// Every call to createServer will emit EACCES — so if waitForPortBindable retried,
// mockCreateServer would be called many times. We assert it's called exactly once.
mockCreateServer.mockClear();
mockCreateServer.mockReturnValue(makeErrServer("EACCES"));
await expect(
waitForPortBindable(80, { timeoutMs: 5000, intervalMs: 50 }),
).rejects.toMatchObject({ code: "EACCES" });
// Only one probe should have been attempted — no spinning through the retry loop.
expect(mockCreateServer).toHaveBeenCalledTimes(1);
});
});