fix(windows): stabilize gateway restart and avoid false stale cleanup

CharlesHappy · CharlesHappy · commit d8520412d5b0 · 2026-03-19T09:25:50.000+08:00
diff --git a/src/cli/daemon-cli/lifecycle.test.ts b/src/cli/daemon-cli/lifecycle.test.ts
@@ -3,8 +3,9 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vite
 type RestartHealthSnapshot = {
   healthy: boolean;
   staleGatewayPids: number[];
-  runtime: { status?: string };
+  runtime: { status?: string; pid?: number };
   portUsage: { port: number; status: string; listeners: []; hints: []; errors?: string[] };
+  listenerKinds?: { gateway: number[]; unknown: number[]; other: number[] };
 };
 
 type RestartPostCheckContext = {
@@ -21,6 +22,7 @@ type RestartParams = {
 
 const service = {
   readCommand: vi.fn(),
+  readRuntime: vi.fn(),
   restart: vi.fn(),
 };
 
@@ -128,6 +130,7 @@ describe("runDaemonRestart health checks", () => {
 
   beforeEach(() => {
     service.readCommand.mockReset();
+    service.readRuntime.mockReset();
     service.restart.mockReset();
     runServiceRestart.mockReset();
     runServiceStop.mockReset();
@@ -148,6 +151,7 @@ describe("runDaemonRestart health checks", () => {
       programArguments: ["openclaw", "gateway", "--port", "18789"],
       environment: {},
     });
+    service.readRuntime.mockResolvedValue({ status: "running", pid: 1500 });
     service.restart.mockResolvedValue({ outcome: "completed" });
 
     runServiceRestart.mockImplementation(async (params: RestartParams) => {
@@ -194,6 +198,7 @@ describe("runDaemonRestart health checks", () => {
       staleGatewayPids: [],
       runtime: { status: "running" },
       portUsage: { port: 18789, status: "busy", listeners: [], hints: [] },
+      listenerKinds: { gateway: [], unknown: [], other: [] },
     };
     waitForGatewayHealthyRestart.mockResolvedValueOnce(unhealthy).mockResolvedValueOnce(healthy);
     terminateStaleGatewayPids.mockResolvedValue([1993]);
@@ -204,6 +209,9 @@ describe("runDaemonRestart health checks", () => {
     expect(terminateStaleGatewayPids).toHaveBeenCalledWith([1993]);
     expect(service.restart).toHaveBeenCalledTimes(1);
     expect(waitForGatewayHealthyRestart).toHaveBeenCalledTimes(2);
+    for (const [params] of waitForGatewayHealthyRestart.mock.calls) {
+      expect(params).not.toHaveProperty("includeUnknownListenersAsStale");
+    }
   });
 
   it("skips stale-pid retry health checks when the retry restart is only scheduled", async () => {
@@ -242,6 +250,47 @@ describe("runDaemonRestart health checks", () => {
     expect(renderRestartDiagnostics).toHaveBeenCalledTimes(1);
   });
 
+  it("warns on Windows when restart health recovers without a clear pid change", async () => {
+    const originalPlatform = process.platform;
+    Object.defineProperty(process, "platform", { value: "win32", configurable: true });
+    const healthy: RestartHealthSnapshot & {
+      listenerKinds: { gateway: number[]; unknown: number[]; other: number[] };
+    } = {
+      healthy: true,
+      staleGatewayPids: [],
+      runtime: { status: "running", pid: 1500 },
+      portUsage: { port: 18789, status: "busy", listeners: [], hints: [] },
+      listenerKinds: { gateway: [1500], unknown: [], other: [] },
+    };
+    waitForGatewayHealthyRestart.mockResolvedValue(healthy);
+    const capturedWarnings: string[][] = [];
+    runServiceRestart.mockImplementationOnce(async (params: RestartParams) => {
+      const warnings: string[] = [];
+      capturedWarnings.push(warnings);
+      await params.postRestartCheck?.({
+        json: true,
+        stdout: process.stdout,
+        warnings,
+        fail: (message: string, hints?: string[]) => {
+          const err = new Error(message) as Error & { hints?: string[] };
+          err.hints = hints;
+          throw err;
+        },
+      });
+      return true;
+    });
+
+    try {
+      await expect(runDaemonRestart({ json: true })).resolves.toBe(true);
+    } finally {
+      Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true });
+    }
+
+    expect(capturedWarnings[0]).toContain(
+      "Gateway restart became healthy, but process identity did not clearly change from pid 1500; runtime may be lagging or the old process may still own the listener.",
+    );
+  });
+
   it("signals an unmanaged gateway process on stop", async () => {
     findVerifiedGatewayListenerPidsOnPortSync.mockReturnValue([4200, 4200, 4300]);
     runServiceStop.mockImplementation(async (params: { onNotLoaded?: () => Promise<unknown> }) => {
diff --git a/src/cli/daemon-cli/lifecycle.ts b/src/cli/daemon-cli/lifecycle.ts
@@ -151,11 +151,18 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi
   const json = Boolean(opts.json);
   const service = resolveGatewayService();
   let restartedWithoutServiceManager = false;
+  let preRestartRuntimePid: number | undefined;
   const restartPort = await resolveGatewayLifecyclePort(service).catch(() =>
     resolveGatewayPortFallback(),
   );
   const restartWaitMs = POST_RESTART_HEALTH_ATTEMPTS * POST_RESTART_HEALTH_DELAY_MS;
   const restartWaitSeconds = Math.round(restartWaitMs / 1000);
+  if (process.platform === "win32") {
+    preRestartRuntimePid = await service
+      .readRuntime(process.env)
+      .then((runtime) => runtime.pid)
+      .catch(() => undefined);
+  }
 
   return await runServiceRestart({
     serviceNoun: "Gateway",
@@ -204,14 +211,18 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi
         port: restartPort,
         attempts: POST_RESTART_HEALTH_ATTEMPTS,
         delayMs: POST_RESTART_HEALTH_DELAY_MS,
-        includeUnknownListenersAsStale: process.platform === "win32",
       });
 
       if (!health.healthy && health.staleGatewayPids.length > 0) {
-        const staleMsg = `Found stale gateway process(es): ${health.staleGatewayPids.join(", ")}.`;
+        const staleMsg = `Found positively identified stale gateway process(es): ${health.staleGatewayPids.join(", ")}.`;
+        const diagnostics = renderRestartDiagnostics(health);
         warnings.push(staleMsg);
+        warnings.push(...diagnostics);
         if (!json) {
           defaultRuntime.log(theme.warn(staleMsg));
+          for (const line of diagnostics) {
+            defaultRuntime.log(theme.muted(line));
+          }
           defaultRuntime.log(theme.muted("Stopping stale process(es) and retrying restart..."));
         }
 
@@ -225,11 +236,23 @@ export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promi
           port: restartPort,
           attempts: POST_RESTART_HEALTH_ATTEMPTS,
           delayMs: POST_RESTART_HEALTH_DELAY_MS,
-          includeUnknownListenersAsStale: process.platform === "win32",
         });
       }
 
       if (health.healthy) {
+        if (
+          process.platform === "win32" &&
+          preRestartRuntimePid != null &&
+          (health.runtime.pid === preRestartRuntimePid ||
+            health.listenerKinds.gateway.includes(preRestartRuntimePid))
+        ) {
+          const identityWarning = `Gateway restart became healthy, but process identity did not clearly change from pid ${preRestartRuntimePid}; runtime may be lagging or the old process may still own the listener.`;
+          if (!json) {
+            defaultRuntime.log(theme.warn(identityWarning));
+          } else {
+            warnings.push(identityWarning);
+          }
+        }
         return;
       }
 
diff --git a/src/cli/daemon-cli/restart-health.test.ts b/src/cli/daemon-cli/restart-health.test.ts
@@ -210,6 +210,30 @@ describe("inspectGatewayRestart", () => {
     expect(snapshot.staleGatewayPids).toEqual([]);
   });
 
+  it("treats a busy unknown Windows listener as healthy when the probe succeeds", async () => {
+    Object.defineProperty(process, "platform", { value: "win32", configurable: true });
+    classifyPortListener.mockReturnValue("unknown");
+    probeGateway.mockResolvedValue({
+      ok: true,
+      close: null,
+    });
+
+    const snapshot = await inspectGatewayRestartWithSnapshot({
+      runtime: { status: "stopped" },
+      portUsage: {
+        port: 18789,
+        status: "busy",
+        listeners: [{ pid: 9100, commandLine: "" }],
+        hints: [],
+      },
+      includeUnknownListenersAsStale: true,
+    });
+
+    expect(snapshot.healthy).toBe(true);
+    expect(snapshot.staleGatewayPids).toEqual([]);
+    expect(snapshot.probeResult).toBe("ok");
+  });
+
   it("treats auth-closed probe as healthy gateway reachability", async () => {
     const snapshot = await inspectAmbiguousOwnershipWithProbe({
       ok: false,
@@ -240,4 +264,35 @@ describe("inspectGatewayRestart", () => {
     expect(snapshot.healthy).toBe(true);
     expect(probeGateway).not.toHaveBeenCalled();
   });
+
+  it("reports probe result, listener classification, and failure reason in diagnostics", async () => {
+    Object.defineProperty(process, "platform", { value: "win32", configurable: true });
+    classifyPortListener.mockImplementation((listener) =>
+      "command" in (listener as Record<string, unknown>) ? "unknown" : "gateway",
+    );
+    probeGateway.mockResolvedValue({
+      ok: false,
+      close: null,
+    });
+
+    const { inspectGatewayRestart, renderRestartDiagnostics } = await import("./restart-health.js");
+    const snapshot = await inspectGatewayRestart({
+      service: makeGatewayService({ status: "stopped" }),
+      port: 18789,
+      includeUnknownListenersAsStale: false,
+    });
+
+    const lines = renderRestartDiagnostics({
+      ...snapshot,
+      listenerKinds: { gateway: [9000], unknown: [9100], other: [] },
+      staleGatewayPids: [9000],
+      failureReason: "stale-gateway-listener",
+      probeResult: "failed",
+    });
+
+    expect(lines).toContain("Restart probe: failed.");
+    expect(lines).toContain("Listener classification: gateway=9000; unknown=9100; other=none.");
+    expect(lines).toContain("Stale gateway termination candidates: 9000.");
+    expect(lines).toContain("Restart health failure reason: stale-gateway-listener.");
+  });
 });
diff --git a/src/cli/daemon-cli/restart-health.ts b/src/cli/daemon-cli/restart-health.ts
@@ -21,6 +21,13 @@ export type GatewayRestartSnapshot = {
   portUsage: PortUsage;
   healthy: boolean;
   staleGatewayPids: number[];
+  probeResult: "ok" | "failed" | "not-run";
+  listenerKinds: {
+    gateway: number[];
+    unknown: number[];
+    other: number[];
+  };
+  failureReason?: "runtime-lag" | "probe-failed" | "stale-gateway-listener";
 };
 
 export type GatewayPortHealthSnapshot = {
@@ -124,35 +131,49 @@ export async function inspectGatewayRestart(params: {
     };
   }
 
+  let probeResult: GatewayRestartSnapshot["probeResult"] = "not-run";
   if (portUsage.status === "busy" && runtime.status !== "running") {
     try {
       const reachable = await confirmGatewayReachable(params.port);
+      probeResult = reachable ? "ok" : "failed";
       if (reachable) {
         return {
           runtime,
           portUsage,
           healthy: true,
           staleGatewayPids: [],
+          probeResult,
+          listenerKinds: { gateway: [], unknown: [], other: [] },
         };
       }
     } catch {
+      probeResult = "failed";
       // Probe is best-effort; keep the ownership-based diagnostics.
     }
   }
 
-  const gatewayListeners =
+  const classifiedListeners =
     portUsage.status === "busy"
-      ? portUsage.listeners.filter(
-          (listener) => classifyPortListener(listener, params.port) === "gateway",
-        )
+      ? portUsage.listeners.map((listener) => ({
+          listener,
+          kind: classifyPortListener(listener, params.port),
+        }))
       : [];
+  const gatewayListeners = classifiedListeners
+    .filter(({ kind }) => kind === "gateway")
+    .map(({ listener }) => listener);
+  const unknownListeners = classifiedListeners
+    .filter(({ kind }) => kind === "unknown")
+    .map(({ listener }) => listener);
+  const otherListeners = classifiedListeners
+    .filter(({ kind }) => kind !== "gateway" && kind !== "unknown")
+    .map(({ listener }) => listener);
   const fallbackListenerPids =
     params.includeUnknownListenersAsStale &&
     process.platform === "win32" &&
     runtime.status !== "running" &&
     portUsage.status === "busy"
-      ? portUsage.listeners
-          .filter((listener) => classifyPortListener(listener, params.port) === "unknown")
+      ? unknownListeners
           .map((listener) => listener.pid)
           .filter((pid): pid is number => Number.isFinite(pid))
       : [];
@@ -169,7 +190,9 @@ export async function inspectGatewayRestart(params: {
   if (!healthy && running && portUsage.status === "busy") {
     try {
       healthy = await confirmGatewayReachable(params.port);
+      probeResult = healthy ? "ok" : "failed";
     } catch {
+      probeResult = "failed";
       // best-effort probe
     }
   }
@@ -192,12 +215,34 @@ export async function inspectGatewayRestart(params: {
       ),
     ]),
   );
+  const failureReason = healthy
+    ? undefined
+    : staleGatewayPids.length > 0
+      ? "stale-gateway-listener"
+      : portUsage.status === "busy" && probeResult === "failed" && runtime.status !== "running"
+        ? "probe-failed"
+        : portUsage.status === "busy" && runtime.status !== "running"
+          ? "runtime-lag"
+          : undefined;
 
   return {
     runtime,
     portUsage,
     healthy,
     staleGatewayPids,
+    probeResult,
+    listenerKinds: {
+      gateway: gatewayListeners
+        .map((listener) => listener.pid)
+        .filter((pid): pid is number => Number.isFinite(pid)),
+      unknown: unknownListeners
+        .map((listener) => listener.pid)
+        .filter((pid): pid is number => Number.isFinite(pid)),
+      other: otherListeners
+        .map((listener) => listener.pid)
+        .filter((pid): pid is number => Number.isFinite(pid)),
+    },
+    failureReason,
   };
 }
 
@@ -290,6 +335,26 @@ export function renderRestartDiagnostics(snapshot: GatewayRestartSnapshot): stri
     lines.push(`Service runtime: ${runtimeSummary}`);
   }
 
+  lines.push(`Restart probe: ${snapshot.probeResult}.`);
+
+  const formatPidSummary = (label: string, pids: number[]) =>
+    `${label}=${pids.length > 0 ? pids.join(", ") : "none"}`;
+  lines.push(
+    `Listener classification: ${[
+      formatPidSummary("gateway", snapshot.listenerKinds.gateway),
+      formatPidSummary("unknown", snapshot.listenerKinds.unknown),
+      formatPidSummary("other", snapshot.listenerKinds.other),
+    ].join("; ")}.`,
+  );
+
+  if (snapshot.staleGatewayPids.length > 0) {
+    lines.push(`Stale gateway termination candidates: ${snapshot.staleGatewayPids.join(", ")}.`);
+  }
+
+  if (snapshot.failureReason) {
+    lines.push(`Restart health failure reason: ${snapshot.failureReason}.`);
+  }
+
   lines.push(...renderPortUsageDiagnostics(snapshot));
 
   return lines;
diff --git a/src/cli/daemon-cli/status.gather.test.ts b/src/cli/daemon-cli/status.gather.test.ts
@@ -31,6 +31,8 @@ const inspectGatewayRestart = vi.fn<(opts?: unknown) => Promise<GatewayRestartSn
     portUsage: { port: 19001, status: "busy", listeners: [], hints: [] },
     healthy: true,
     staleGatewayPids: [],
+    probeResult: "ok",
+    listenerKinds: { gateway: [1234], unknown: [], other: [] },
   }),
 );
 const serviceReadCommand = vi.fn<
@@ -459,6 +461,8 @@ describe("gatherDaemonStatus", () => {
       },
       healthy: false,
       staleGatewayPids: [9000],
+      probeResult: "failed",
+      listenerKinds: { gateway: [9000], unknown: [], other: [] },
     });
 
     const status = await gatherDaemonStatus({
diff --git a/src/infra/ports-format.test.ts b/src/infra/ports-format.test.ts
diff --git a/src/infra/ports-format.ts b/src/infra/ports-format.ts
