Skip to content

Commit 5ea03ef

Browse files
steipetesteipete
committed
fix: harden windows gateway lifecycle
1 parent 84a2a28 commit 5ea03ef

11 files changed

Lines changed: 680 additions & 171 deletions

‎src/cli/daemon-cli/lifecycle.test.ts‎

Lines changed: 25 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
22

3-
const mockReadFileSync = vi.hoisted(() => vi.fn());
4-
const mockSpawnSync = vi.hoisted(() => vi.fn());
5-
63
type RestartHealthSnapshot = {
74
healthy: boolean;
85
staleGatewayPids: number[];
@@ -35,7 +32,9 @@ const terminateStaleGatewayPids = vi.fn();
3532
const renderGatewayPortHealthDiagnostics = vi.fn(() => ["diag: unhealthy port"]);
3633
const renderRestartDiagnostics = vi.fn(() => ["diag: unhealthy runtime"]);
3734
const resolveGatewayPort = vi.fn(() => 18789);
38-
const findGatewayPidsOnPortSync = vi.fn<(port: number) => number[]>(() => []);
35+
const findVerifiedGatewayListenerPidsOnPortSync = vi.fn<(port: number) => number[]>(() => []);
36+
const signalVerifiedGatewayPidSync = vi.fn<(pid: number, signal: "SIGTERM" | "SIGUSR1") => void>();
37+
const formatGatewayPidList = vi.fn<(pids: number[]) => string>((pids) => pids.join(", "));
3938
const probeGateway = vi.fn<
4039
(opts: {
4140
url: string;
@@ -49,24 +48,18 @@ const probeGateway = vi.fn<
4948
const isRestartEnabled = vi.fn<(config?: { commands?: unknown }) => boolean>(() => true);
5049
const loadConfig = vi.fn(() => ({}));
5150

52-
vi.mock("node:fs", () => ({
53-
default: {
54-
readFileSync: (...args: unknown[]) => mockReadFileSync(...args),
55-
},
56-
}));
57-
58-
vi.mock("node:child_process", () => ({
59-
spawnSync: (...args: unknown[]) => mockSpawnSync(...args),
60-
}));
61-
6251
vi.mock("../../config/config.js", () => ({
6352
loadConfig: () => loadConfig(),
6453
readBestEffortConfig: async () => loadConfig(),
6554
resolveGatewayPort,
6655
}));
6756

68-
vi.mock("../../infra/restart.js", () => ({
69-
findGatewayPidsOnPortSync: (port: number) => findGatewayPidsOnPortSync(port),
57+
vi.mock("../../infra/gateway-processes.js", () => ({
58+
findVerifiedGatewayListenerPidsOnPortSync: (port: number) =>
59+
findVerifiedGatewayListenerPidsOnPortSync(port),
60+
signalVerifiedGatewayPidSync: (pid: number, signal: "SIGTERM" | "SIGUSR1") =>
61+
signalVerifiedGatewayPidSync(pid, signal),
62+
formatGatewayPidList: (pids: number[]) => formatGatewayPidList(pids),
7063
}));
7164

7265
vi.mock("../../gateway/probe.js", () => ({
@@ -121,12 +114,12 @@ describe("runDaemonRestart health checks", () => {
121114
renderGatewayPortHealthDiagnostics.mockReset();
122115
renderRestartDiagnostics.mockReset();
123116
resolveGatewayPort.mockReset();
124-
findGatewayPidsOnPortSync.mockReset();
117+
findVerifiedGatewayListenerPidsOnPortSync.mockReset();
118+
signalVerifiedGatewayPidSync.mockReset();
119+
formatGatewayPidList.mockReset();
125120
probeGateway.mockReset();
126121
isRestartEnabled.mockReset();
127122
loadConfig.mockReset();
128-
mockReadFileSync.mockReset();
129-
mockSpawnSync.mockReset();
130123

131124
service.readCommand.mockResolvedValue({
132125
programArguments: ["openclaw", "gateway", "--port", "18789"],
@@ -158,23 +151,8 @@ describe("runDaemonRestart health checks", () => {
158151
configSnapshot: { commands: { restart: true } },
159152
});
160153
isRestartEnabled.mockReturnValue(true);
161-
mockReadFileSync.mockImplementation((path: string) => {
162-
const match = path.match(/\/proc\/(\d+)\/cmdline$/);
163-
if (!match) {
164-
throw new Error(`unexpected path ${path}`);
165-
}
166-
const pid = Number.parseInt(match[1] ?? "", 10);
167-
if ([4200, 4300].includes(pid)) {
168-
return ["openclaw", "gateway", "--port", "18789", ""].join("\0");
169-
}
170-
throw new Error(`unknown pid ${pid}`);
171-
});
172-
mockSpawnSync.mockReturnValue({
173-
error: null,
174-
status: 0,
175-
stdout: "openclaw gateway --port 18789",
176-
stderr: "",
177-
});
154+
signalVerifiedGatewayPidSync.mockImplementation(() => {});
155+
formatGatewayPidList.mockImplementation((pids) => pids.join(", "));
178156
});
179157

180158
afterEach(() => {
@@ -242,38 +220,20 @@ describe("runDaemonRestart health checks", () => {
242220
});
243221

244222
it("signals an unmanaged gateway process on stop", async () => {
245-
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
246-
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true);
247-
findGatewayPidsOnPortSync.mockReturnValue([4200, 4200, 4300]);
248-
mockSpawnSync.mockReturnValue({
249-
error: null,
250-
status: 0,
251-
stdout:
252-
'CommandLine="C:\\\\Program Files\\\\OpenClaw\\\\openclaw.exe" gateway --port 18789\r\n',
253-
stderr: "",
254-
});
223+
findVerifiedGatewayListenerPidsOnPortSync.mockReturnValue([4200, 4200, 4300]);
255224
runServiceStop.mockImplementation(async (params: { onNotLoaded?: () => Promise<unknown> }) => {
256225
await params.onNotLoaded?.();
257226
});
258227

259228
await runDaemonStop({ json: true });
260229

261-
expect(findGatewayPidsOnPortSync).toHaveBeenCalledWith(18789);
262-
expect(killSpy).toHaveBeenCalledWith(4200, "SIGTERM");
263-
expect(killSpy).toHaveBeenCalledWith(4300, "SIGTERM");
230+
expect(findVerifiedGatewayListenerPidsOnPortSync).toHaveBeenCalledWith(18789);
231+
expect(signalVerifiedGatewayPidSync).toHaveBeenCalledWith(4200, "SIGTERM");
232+
expect(signalVerifiedGatewayPidSync).toHaveBeenCalledWith(4300, "SIGTERM");
264233
});
265234

266235
it("signals a single unmanaged gateway process on restart", async () => {
267-
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
268-
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true);
269-
findGatewayPidsOnPortSync.mockReturnValue([4200]);
270-
mockSpawnSync.mockReturnValue({
271-
error: null,
272-
status: 0,
273-
stdout:
274-
'CommandLine="C:\\\\Program Files\\\\OpenClaw\\\\openclaw.exe" gateway --port 18789\r\n',
275-
stderr: "",
276-
});
236+
findVerifiedGatewayListenerPidsOnPortSync.mockReturnValue([4200]);
277237
runServiceRestart.mockImplementation(
278238
async (params: RestartParams & { onNotLoaded?: () => Promise<unknown> }) => {
279239
await params.onNotLoaded?.();
@@ -291,8 +251,8 @@ describe("runDaemonRestart health checks", () => {
291251

292252
await runDaemonRestart({ json: true });
293253

294-
expect(findGatewayPidsOnPortSync).toHaveBeenCalledWith(18789);
295-
expect(killSpy).toHaveBeenCalledWith(4200, "SIGUSR1");
254+
expect(findVerifiedGatewayListenerPidsOnPortSync).toHaveBeenCalledWith(18789);
255+
expect(signalVerifiedGatewayPidSync).toHaveBeenCalledWith(4200, "SIGUSR1");
296256
expect(probeGateway).toHaveBeenCalledTimes(1);
297257
expect(waitForGatewayHealthyListener).toHaveBeenCalledTimes(1);
298258
expect(waitForGatewayHealthyRestart).not.toHaveBeenCalled();
@@ -301,15 +261,7 @@ describe("runDaemonRestart health checks", () => {
301261
});
302262

303263
it("fails unmanaged restart when multiple gateway listeners are present", async () => {
304-
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
305-
findGatewayPidsOnPortSync.mockReturnValue([4200, 4300]);
306-
mockSpawnSync.mockReturnValue({
307-
error: null,
308-
status: 0,
309-
stdout:
310-
'CommandLine="C:\\\\Program Files\\\\OpenClaw\\\\openclaw.exe" gateway --port 18789\r\n',
311-
stderr: "",
312-
});
264+
findVerifiedGatewayListenerPidsOnPortSync.mockReturnValue([4200, 4300]);
313265
runServiceRestart.mockImplementation(
314266
async (params: RestartParams & { onNotLoaded?: () => Promise<unknown> }) => {
315267
await params.onNotLoaded?.();
@@ -323,7 +275,7 @@ describe("runDaemonRestart health checks", () => {
323275
});
324276

325277
it("fails unmanaged restart when the running gateway has commands.restart disabled", async () => {
326-
findGatewayPidsOnPortSync.mockReturnValue([4200]);
278+
findVerifiedGatewayListenerPidsOnPortSync.mockReturnValue([4200]);
327279
probeGateway.mockResolvedValue({
328280
ok: true,
329281
configSnapshot: { commands: { restart: false } },
@@ -342,21 +294,13 @@ describe("runDaemonRestart health checks", () => {
342294
});
343295

344296
it("skips unmanaged signaling for pids that are not live gateway processes", async () => {
345-
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true);
346-
findGatewayPidsOnPortSync.mockReturnValue([4200]);
347-
mockReadFileSync.mockReturnValue(["python", "-m", "http.server", ""].join("\0"));
348-
mockSpawnSync.mockReturnValue({
349-
error: null,
350-
status: 0,
351-
stdout: "python -m http.server",
352-
stderr: "",
353-
});
297+
findVerifiedGatewayListenerPidsOnPortSync.mockReturnValue([]);
354298
runServiceStop.mockImplementation(async (params: { onNotLoaded?: () => Promise<unknown> }) => {
355299
await params.onNotLoaded?.();
356300
});
357301

358302
await runDaemonStop({ json: true });
359303

360-
expect(killSpy).not.toHaveBeenCalled();
304+
expect(signalVerifiedGatewayPidSync).not.toHaveBeenCalled();
361305
});
362306
});

‎src/cli/daemon-cli/lifecycle.ts‎

Lines changed: 8 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { spawnSync } from "node:child_process";
2-
import fsSync from "node:fs";
31
import { isRestartEnabled } from "../../config/commands.js";
42
import { readBestEffortConfig, resolveGatewayPort } from "../../config/config.js";
5-
import { parseCmdScriptCommandLine } from "../../daemon/cmd-argv.js";
63
import { resolveGatewayService } from "../../daemon/service.js";
74
import { probeGateway } from "../../gateway/probe.js";
8-
import { isGatewayArgv, parseProcCmdline } from "../../infra/gateway-process-argv.js";
9-
import { findGatewayPidsOnPortSync } from "../../infra/restart.js";
5+
import {
6+
findVerifiedGatewayListenerPidsOnPortSync,
7+
formatGatewayPidList,
8+
signalVerifiedGatewayPidSync,
9+
} from "../../infra/gateway-processes.js";
1010
import { defaultRuntime } from "../../runtime.js";
1111
import { theme } from "../../terminal/theme.js";
1212
import { formatCliCommand } from "../command-format.js";
@@ -43,85 +43,12 @@ async function resolveGatewayLifecyclePort(service = resolveGatewayService()) {
4343
return portFromArgs ?? resolveGatewayPort(await readBestEffortConfig(), mergedEnv);
4444
}
4545

46-
function extractWindowsCommandLine(raw: string): string | null {
47-
const lines = raw
48-
.split(/\r?\n/)
49-
.map((line) => line.trim())
50-
.filter(Boolean);
51-
for (const line of lines) {
52-
if (!line.toLowerCase().startsWith("commandline=")) {
53-
continue;
54-
}
55-
const value = line.slice("commandline=".length).trim();
56-
return value || null;
57-
}
58-
return lines.find((line) => line.toLowerCase() !== "commandline") ?? null;
59-
}
60-
61-
function readGatewayProcessArgsSync(pid: number): string[] | null {
62-
if (process.platform === "linux") {
63-
try {
64-
return parseProcCmdline(fsSync.readFileSync(`/proc/${pid}/cmdline`, "utf8"));
65-
} catch {
66-
return null;
67-
}
68-
}
69-
if (process.platform === "darwin") {
70-
const ps = spawnSync("ps", ["-o", "command=", "-p", String(pid)], {
71-
encoding: "utf8",
72-
timeout: 1000,
73-
});
74-
if (ps.error || ps.status !== 0) {
75-
return null;
76-
}
77-
const command = ps.stdout.trim();
78-
return command ? command.split(/\s+/) : null;
79-
}
80-
if (process.platform === "win32") {
81-
const wmic = spawnSync(
82-
"wmic",
83-
["process", "where", `ProcessId=${pid}`, "get", "CommandLine", "/value"],
84-
{
85-
encoding: "utf8",
86-
timeout: 1000,
87-
},
88-
);
89-
if (wmic.error || wmic.status !== 0) {
90-
return null;
91-
}
92-
const command = extractWindowsCommandLine(wmic.stdout);
93-
return command ? parseCmdScriptCommandLine(command) : null;
94-
}
95-
return null;
96-
}
97-
98-
function resolveGatewayListenerPids(port: number): number[] {
99-
return Array.from(new Set(findGatewayPidsOnPortSync(port)))
100-
.filter((pid): pid is number => Number.isFinite(pid) && pid > 0)
101-
.filter((pid) => {
102-
const args = readGatewayProcessArgsSync(pid);
103-
return args != null && isGatewayArgv(args, { allowGatewayBinary: true });
104-
});
105-
}
106-
10746
function resolveGatewayPortFallback(): Promise<number> {
10847
return readBestEffortConfig()
10948
.then((cfg) => resolveGatewayPort(cfg, process.env))
11049
.catch(() => resolveGatewayPort(undefined, process.env));
11150
}
11251

113-
function signalGatewayPid(pid: number, signal: "SIGTERM" | "SIGUSR1") {
114-
const args = readGatewayProcessArgsSync(pid);
115-
if (!args || !isGatewayArgv(args, { allowGatewayBinary: true })) {
116-
throw new Error(`refusing to signal non-gateway process pid ${pid}`);
117-
}
118-
process.kill(pid, signal);
119-
}
120-
121-
function formatGatewayPidList(pids: number[]): string {
122-
return pids.join(", ");
123-
}
124-
12552
async function assertUnmanagedGatewayRestartEnabled(port: number): Promise<void> {
12653
const probe = await probeGateway({
12754
url: `ws://127.0.0.1:${port}`,
@@ -143,7 +70,7 @@ async function assertUnmanagedGatewayRestartEnabled(port: number): Promise<void>
14370
}
14471

14572
function resolveVerifiedGatewayListenerPids(port: number): number[] {
146-
return resolveGatewayListenerPids(port).filter(
73+
return findVerifiedGatewayListenerPidsOnPortSync(port).filter(
14774
(pid): pid is number => Number.isFinite(pid) && pid > 0,
14875
);
14976
}
@@ -154,7 +81,7 @@ async function stopGatewayWithoutServiceManager(port: number) {
15481
return null;
15582
}
15683
for (const pid of pids) {
157-
signalGatewayPid(pid, "SIGTERM");
84+
signalVerifiedGatewayPidSync(pid, "SIGTERM");
15885
}
15986
return {
16087
result: "stopped" as const,
@@ -173,7 +100,7 @@ async function restartGatewayWithoutServiceManager(port: number) {
173100
`multiple gateway processes are listening on port ${port}: ${formatGatewayPidList(pids)}; use "openclaw gateway status --deep" before retrying restart`,
174101
);
175102
}
176-
signalGatewayPid(pids[0], "SIGUSR1");
103+
signalVerifiedGatewayPidSync(pids[0], "SIGUSR1");
177104
return {
178105
result: "restarted" as const,
179106
message: `Gateway restart signal sent to unmanaged process on port ${port}: ${pids[0]}.`,

‎src/cli/daemon-cli/restart-health.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,32 @@ describe("inspectGatewayRestart", () => {
190190
);
191191
});
192192

193+
it("treats a busy port as healthy when runtime status lags but the probe succeeds", async () => {
194+
Object.defineProperty(process, "platform", { value: "win32", configurable: true });
195+
196+
const service = {
197+
readRuntime: vi.fn(async () => ({ status: "stopped" })),
198+
} as unknown as GatewayService;
199+
200+
inspectPortUsage.mockResolvedValue({
201+
port: 18789,
202+
status: "busy",
203+
listeners: [{ pid: 9100, commandLine: "openclaw-gateway" }],
204+
hints: [],
205+
});
206+
classifyPortListener.mockReturnValue("gateway");
207+
probeGateway.mockResolvedValue({
208+
ok: true,
209+
close: null,
210+
});
211+
212+
const { inspectGatewayRestart } = await import("./restart-health.js");
213+
const snapshot = await inspectGatewayRestart({ service, port: 18789 });
214+
215+
expect(snapshot.healthy).toBe(true);
216+
expect(snapshot.staleGatewayPids).toEqual([]);
217+
});
218+
193219
it("treats auth-closed probe as healthy gateway reachability", async () => {
194220
const snapshot = await inspectAmbiguousOwnershipWithProbe({
195221
ok: false,

‎src/cli/daemon-cli/restart-health.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ async function confirmGatewayReachable(port: number): Promise<boolean> {
6565
const probe = await probeGateway({
6666
url: `ws://127.0.0.1:${port}`,
6767
auth: token || password ? { token, password } : undefined,
68-
timeoutMs: 1_000,
68+
timeoutMs: 3_000,
69+
includeDetails: false,
6970
});
7071
return probe.ok || looksLikeAuthClose(probe.close?.code, probe.close?.reason);
7172
}
@@ -123,6 +124,22 @@ export async function inspectGatewayRestart(params: {
123124
};
124125
}
125126

127+
if (portUsage.status === "busy" && runtime.status !== "running") {
128+
try {
129+
const reachable = await confirmGatewayReachable(params.port);
130+
if (reachable) {
131+
return {
132+
runtime,
133+
portUsage,
134+
healthy: true,
135+
staleGatewayPids: [],
136+
};
137+
}
138+
} catch {
139+
// Probe is best-effort; keep the ownership-based diagnostics.
140+
}
141+
}
142+
126143
const gatewayListeners =
127144
portUsage.status === "busy"
128145
? portUsage.listeners.filter(

0 commit comments

Comments
 (0)