fix(gateway): reuse ws ping for connection health

trialanderrorstudios · trialanderrorstudios · commit 1cd9292605bd · 2026-05-03T13:38:52.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -526,7 +526,7 @@ Docs: https://docs.openclaw.ai
 - macOS app: move recent session context rows into a Context submenu while keeping usage and cost details root-level, so the menu bar companion stays compact with many active sessions. Thanks @guti.
 - Gateway/SDK: add SDK-facing tools.invoke RPC with shared HTTP policy, typed approval/refusal results, and SDK helper support. Refs #74705. Thanks @BunsDev and @ai-hpc.
 - Discord: keep active buttons, selects, and forms working across Gateway restarts until they expire, so multi-step Discord interactions are less likely to break during upgrades or restarts. Thanks @amknight.
-- Gateway/health: add per-client `health.connection` telemetry with WebSocket ping/pong RTT, last-heartbeat, and stale-connection reporting while keeping cached service health shared. (#70230) Thanks @trialanderrorstudios.
+- Gateway/health: add per-client `health.connection` telemetry with authenticated WebSocket keepalive ping/pong RTT, last-heartbeat, and stale-connection reporting while keeping cached service health shared. (#70230) Thanks @trialanderrorstudios.
 - Messages/docs: clarify that `BodyForAgent` is the primary inbound model text while `Body` is the legacy envelope fallback, and add Signal coverage so channel hardening patches target the real prompt path. Refs #66198. Thanks @defonota3box.
 - Slack: publish a safe default App Home tab view on `app_home_opened` and include the Home tab event in setup manifests. Fixes #11655; refs #52020. Thanks @TinyTb.
 - Slack: keep track of bot-participated threads across restarts, so ongoing threaded conversations can continue auto-replying after the Gateway is restarted. Thanks @amknight.
diff --git a/docs/cli/health.md b/docs/cli/health.md
@@ -33,7 +33,8 @@ Notes:
   refresh in the background.
 - JSON output from an authenticated WebSocket health response includes a per-client
   `connection` block with `connected`, `rttMs`, and `lastHeartbeatAt`. Those fields
-  describe the current Gateway connection, not the cached service snapshot.
+  describe the current Gateway connection, not the cached service snapshot. RTT is
+  sampled from the authenticated WebSocket keepalive ping/pong.
 - `--verbose` forces a live probe, prints gateway connection details, and expands the
   human-readable output across all configured accounts and agents.
 - Output includes per-agent session stores when multiple agents are configured.
diff --git a/docs/gateway/health.md b/docs/gateway/health.md
@@ -76,7 +76,7 @@ The health snapshot includes: `ok` (boolean), `ts` (timestamp), `durationMs` (pr
 }
 ```
 
-`connection` is not stored in the shared cached health snapshot. It is overlaid for the requesting WebSocket client so separate clients keep independent liveness and RTT state. Before the first successful WebSocket ping/pong, `connected` can be `true` while `rttMs` and `lastHeartbeatAt` are `null`.
+`connection` is not stored in the shared cached health snapshot. It is overlaid for the requesting WebSocket client so separate clients keep independent liveness and RTT state. RTT samples come from the authenticated WebSocket keepalive ping/pong. Before the first successful WebSocket ping/pong, `connected` can be `true` while `rttMs` and `lastHeartbeatAt` are `null`.
 
 ## Related
 
diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md
@@ -320,7 +320,7 @@ enumeration of `src/gateway/server-methods/*.ts`.
 
 <AccordionGroup>
   <Accordion title="System and identity">
-    - `health` returns the cached or freshly probed gateway health snapshot. Authenticated WebSocket responses include a per-client `connection` block with `connected`, WebSocket ping/pong `rttMs`, and `lastHeartbeatAt`; this block is overlaid per response and is not part of the shared cached service snapshot.
+    - `health` returns the cached or freshly probed gateway health snapshot. Authenticated WebSocket responses include a per-client `connection` block with `connected`, authenticated keepalive ping/pong `rttMs`, and `lastHeartbeatAt`; this block is overlaid per response and is not part of the shared cached service snapshot.
     - `diagnostics.stability` returns the recent bounded diagnostic stability recorder. It keeps operational metadata such as event names, counts, byte sizes, memory readings, queue/session state, channel/plugin names, and session ids. It does not keep chat text, webhook bodies, tool outputs, raw request or response bodies, tokens, cookies, or secret values. Operator read scope is required.
     - `status` returns the `/status`-style gateway summary; sensitive fields are included only for admin-scoped operator clients.
     - `gateway.identity.get` returns the gateway device identity used by relay and pairing flows.
diff --git a/src/gateway/server-close.test.ts b/src/gateway/server-close.test.ts
@@ -85,7 +85,6 @@ function createGatewayCloseTestDeps(
     nodePresenceTimers: new Map(),
     broadcast: vi.fn(),
     tickInterval: setInterval(() => undefined, 60_000),
-    connectionPingInterval: setInterval(() => undefined, 60_000),
     healthInterval: setInterval(() => undefined, 60_000),
     dedupeCleanup: setInterval(() => undefined, 60_000),
     mediaCleanup: null,
diff --git a/src/gateway/server-close.ts b/src/gateway/server-close.ts
@@ -186,7 +186,6 @@ export function createGatewayCloseHandler(params: {
   nodePresenceTimers: Map<string, ReturnType<typeof setInterval>>;
   broadcast: (event: string, payload: unknown, opts?: { dropIfSlow?: boolean }) => void;
   tickInterval: ReturnType<typeof setInterval>;
-  connectionPingInterval: ReturnType<typeof setInterval>;
   healthInterval: ReturnType<typeof setInterval>;
   dedupeCleanup: ReturnType<typeof setInterval>;
   mediaCleanup: ReturnType<typeof setInterval> | null;
@@ -311,7 +310,6 @@ export function createGatewayCloseHandler(params: {
         restartExpectedMs,
       });
       clearInterval(params.tickInterval);
-      clearInterval(params.connectionPingInterval);
       clearInterval(params.healthInterval);
       clearInterval(params.dedupeCleanup);
       if (params.mediaCleanup) {
diff --git a/src/gateway/server-maintenance.test.ts b/src/gateway/server-maintenance.test.ts
@@ -1,8 +1,6 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { WebSocket } from "ws";
 import type { HealthSummary } from "../commands/health.js";
 import type { ChatAbortControllerEntry } from "./chat-abort.js";
-import type { GatewayWsClient } from "./server/ws-types.js";
 
 const cleanOldMediaMock = vi.fn(async () => {});
 
@@ -30,7 +28,6 @@ function createActiveRun(sessionKey: string): ChatAbortControllerEntry {
 
 function createMaintenanceTimerDeps() {
   return {
-    clients: new Set<GatewayWsClient>(),
     broadcast: () => {},
     nodeSendToAllSubscribed: () => {},
     getPresenceVersion: () => 1,
@@ -51,13 +48,11 @@ function createMaintenanceTimerDeps() {
 
 function stopMaintenanceTimers(timers: {
   tickInterval: NodeJS.Timeout;
-  connectionPingInterval: NodeJS.Timeout;
   healthInterval: NodeJS.Timeout;
   dedupeCleanup: NodeJS.Timeout;
   mediaCleanup: NodeJS.Timeout | null;
 }) {
   clearInterval(timers.tickInterval);
-  clearInterval(timers.connectionPingInterval);
   clearInterval(timers.healthInterval);
   clearInterval(timers.dedupeCleanup);
   if (timers.mediaCleanup) {
@@ -128,32 +123,6 @@ describe("startGatewayMaintenanceTimers", () => {
     stopMaintenanceTimers(timers);
   });
 
-  it("pings authenticated open clients on the connection health cadence", async () => {
-    vi.useFakeTimers();
-    vi.setSystemTime(new Date("2026-04-12T00:00:00Z"));
-    const { CONNECTION_PING_INTERVAL_MS } = await import("./server/connection-health.js");
-    const { startGatewayMaintenanceTimers } = await import("./server-maintenance.js");
-    const ping = vi.fn();
-    const deps = createMaintenanceTimerDeps();
-    const client = {
-      socket: { readyState: WebSocket.OPEN, ping },
-      connectionHealth: { connectedAtMs: Date.now() },
-    } as unknown as GatewayWsClient;
-    deps.clients.add(client);
-
-    const timers = startGatewayMaintenanceTimers(deps);
-
-    await vi.advanceTimersByTimeAsync(CONNECTION_PING_INTERVAL_MS);
-
-    expect(ping).toHaveBeenCalledWith(String(Date.now()));
-    expect(
-      (client as { connectionHealth: { lastPingSentAtMs?: number } }).connectionHealth
-        .lastPingSentAtMs,
-    ).toBe(Date.now());
-
-    stopMaintenanceTimers(timers);
-  });
-
   it("skips overlapping media cleanup runs", async () => {
     vi.useFakeTimers();
     let resolveCleanup = () => {};
diff --git a/src/gateway/server-maintenance.ts b/src/gateway/server-maintenance.ts
@@ -12,12 +12,9 @@ import {
 } from "./server-constants.js";
 import type { DedupeEntry } from "./server-shared.js";
 import { formatError } from "./server-utils.js";
-import { CONNECTION_PING_INTERVAL_MS, pingGatewayClient } from "./server/connection-health.js";
 import { setBroadcastHealthUpdate } from "./server/health-state.js";
-import type { GatewayWsClient } from "./server/ws-types.js";
 
 export function startGatewayMaintenanceTimers(params: {
-  clients: Set<GatewayWsClient>;
   broadcast: (
     event: string,
     payload: unknown,
@@ -50,7 +47,6 @@ export function startGatewayMaintenanceTimers(params: {
   mediaCleanupTtlMs?: number;
 }): {
   tickInterval: ReturnType<typeof setInterval>;
-  connectionPingInterval: ReturnType<typeof setInterval>;
   healthInterval: ReturnType<typeof setInterval>;
   dedupeCleanup: ReturnType<typeof setInterval>;
   mediaCleanup: ReturnType<typeof setInterval> | null;
@@ -72,13 +68,6 @@ export function startGatewayMaintenanceTimers(params: {
     params.nodeSendToAllSubscribed("tick", payload);
   }, TICK_INTERVAL_MS);
 
-  const connectionPingInterval = setInterval(() => {
-    const now = Date.now();
-    for (const client of params.clients) {
-      pingGatewayClient(client, now);
-    }
-  }, CONNECTION_PING_INTERVAL_MS);
-
   // periodic health refresh to keep cached snapshot warm
   const healthInterval = setInterval(() => {
     void params
@@ -178,7 +167,6 @@ export function startGatewayMaintenanceTimers(params: {
   if (typeof params.mediaCleanupTtlMs !== "number") {
     return {
       tickInterval,
-      connectionPingInterval,
       healthInterval,
       dedupeCleanup,
       mediaCleanup: null,
@@ -209,5 +197,5 @@ export function startGatewayMaintenanceTimers(params: {
 
   void runMediaCleanup();
 
-  return { tickInterval, connectionPingInterval, healthInterval, dedupeCleanup, mediaCleanup };
+  return { tickInterval, healthInterval, dedupeCleanup, mediaCleanup };
 }
diff --git a/src/gateway/server-methods/health.ts b/src/gateway/server-methods/health.ts
@@ -119,10 +119,7 @@ export const healthHandlers: GatewayRequestHandlers = {
     }
     try {
       const snap = await refreshHealthSnapshot({ probe: wantsProbe, includeSensitive });
-      const payload = context.getEventLoopHealth
-        ? { ...snap, eventLoop: context.getEventLoopHealth() }
-        : snap;
-      respond(true, withConnectionHealth(payload, client), undefined);
+      respond(true, withConnectionHealth(snap, client), undefined);
     } catch (err) {
       respond(false, undefined, errorShape(ErrorCodes.UNAVAILABLE, formatForLog(err)));
     }
diff --git a/src/gateway/server-runtime-handles.ts b/src/gateway/server-runtime-handles.ts
@@ -9,7 +9,6 @@ type GatewayConfigReloaderHandle = {
 export type GatewayServerMutableState = {
   bonjourStop: (() => Promise<void>) | null;
   tickInterval: ReturnType<typeof setInterval>;
-  connectionPingInterval: ReturnType<typeof setInterval>;
   healthInterval: ReturnType<typeof setInterval>;
   dedupeCleanup: ReturnType<typeof setInterval>;
   mediaCleanup: ReturnType<typeof setInterval> | null;
@@ -38,7 +37,6 @@ export function createGatewayServerMutableState(): GatewayServerMutableState {
   return {
     bonjourStop: null as (() => Promise<void>) | null,
     tickInterval: noopInterval(),
-    connectionPingInterval: noopInterval(),
     healthInterval: noopInterval(),
     dedupeCleanup: noopInterval(),
     mediaCleanup: null as ReturnType<typeof setInterval> | null,
diff --git a/src/gateway/server-startup-early.test.ts b/src/gateway/server-startup-early.test.ts
@@ -38,7 +38,6 @@ describe("startGatewayEarlyRuntime", () => {
         warn: () => {},
       },
       nodeRegistry: {} as never,
-      clients: new Set(),
       broadcast: () => {},
       nodeSendToAllSubscribed: () => {},
       getPresenceVersion: () => 0,
diff --git a/src/gateway/server/connection-health.ts b/src/gateway/server/connection-health.ts
@@ -2,7 +2,7 @@ import { WebSocket } from "ws";
 import type { HealthSummary } from "../../commands/health.js";
 import type { GatewayConnectionHealthState, GatewayWsClient } from "./ws-types.js";
 
-export const CONNECTION_PING_INTERVAL_MS = 5_000;
+export const CONNECTION_PING_INTERVAL_MS = 25_000;
 // 4x stale tolerance: misses up to 3 pongs before reporting disconnected.
 export const CONNECTION_STALE_MS = CONNECTION_PING_INTERVAL_MS * 4;
 const MAX_PENDING_PING_TIMESTAMPS =
diff --git a/src/gateway/server/ws-connection.test.ts b/src/gateway/server/ws-connection.test.ts
@@ -1,6 +1,6 @@
 import { EventEmitter } from "node:events";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
-import type { WebSocketServer } from "ws";
+import { WebSocket, type WebSocketServer } from "ws";
 import type { ResolvedGatewayAuth } from "../auth.js";
 
 const { attachGatewayWsMessageHandlerMock } = vi.hoisted(() => ({
@@ -11,6 +11,7 @@ vi.mock("./ws-connection/message-handler.js", () => ({
   attachGatewayWsMessageHandler: attachGatewayWsMessageHandlerMock,
 }));
 
+import { CONNECTION_PING_INTERVAL_MS } from "./connection-health.js";
 import { attachGatewayWsConnectionHandler } from "./ws-connection.js";
 import { resolveSharedGatewaySessionGeneration } from "./ws-shared-generation.js";
 
@@ -179,8 +180,9 @@ describe("attachGatewayWsConnectionHandler", () => {
     expect(clients.size).toBe(0);
   });
 
-  it("sends protocol pings until the connection closes", async () => {
+  it("sends connection-health pings from the authenticated socket timer until close", async () => {
     vi.useFakeTimers();
+    vi.setSystemTime(new Date("2026-04-12T00:00:00Z"));
     const listeners = new Map<string, (...args: unknown[]) => void>();
     const wss = {
       on: vi.fn((event: string, handler: (...args: unknown[]) => void) => {
@@ -196,6 +198,7 @@ describe("attachGatewayWsConnectionHandler", () => {
       },
       send: vi.fn(),
       ping: vi.fn(),
+      readyState: WebSocket.OPEN,
       close: vi.fn(),
     });
     const upgradeReq = {
@@ -235,20 +238,29 @@ describe("attachGatewayWsConnectionHandler", () => {
     const passed = attachGatewayWsMessageHandlerMock.mock.calls[0]?.[0] as {
       setClient: (client: unknown) => boolean;
     };
-    expect(
-      passed.setClient({
-        socket,
-        connect: { client: { id: "openclaw-control-ui", mode: "webchat" } },
-        connId: "ping-client",
-        usesSharedGatewayAuth: false,
-      }),
-    ).toBe(true);
+    const connectionHealth: { connectedAtMs: number; lastPingSentAtMs?: number } = {
+      connectedAtMs: Date.now(),
+    };
+    const client = {
+      socket,
+      connect: { client: { id: "openclaw-control-ui", mode: "webchat" } },
+      connId: "ping-client",
+      connectionHealth,
+      usesSharedGatewayAuth: false,
+    };
+    expect(passed.setClient(client)).toBe(true);
 
-    vi.advanceTimersByTime(25_000);
     expect(socket.ping).toHaveBeenCalledTimes(1);
+    expect(socket.ping).toHaveBeenCalledWith(String(Date.now()));
+    expect(connectionHealth.lastPingSentAtMs).toBe(Date.now());
+
+    vi.advanceTimersByTime(CONNECTION_PING_INTERVAL_MS);
+    expect(socket.ping).toHaveBeenCalledTimes(2);
+    expect(socket.ping).toHaveBeenLastCalledWith(String(Date.now()));
+    expect(connectionHealth.lastPingSentAtMs).toBe(Date.now());
 
     socket.emit("close", 1000, Buffer.from("done"));
-    vi.advanceTimersByTime(25_000);
-    expect(socket.ping).toHaveBeenCalledTimes(1);
+    vi.advanceTimersByTime(CONNECTION_PING_INTERVAL_MS);
+    expect(socket.ping).toHaveBeenCalledTimes(2);
   });
 });
diff --git a/src/gateway/server/ws-connection.ts b/src/gateway/server/ws-connection.ts
@@ -18,7 +18,11 @@ import { clearNodeWakeState } from "../server-methods/nodes-wake-state.js";
 import type { GatewayRequestContext, GatewayRequestHandlers } from "../server-methods/types.js";
 import { formatError } from "../server-utils.js";
 import { logWs } from "../ws-log.js";
-import { recordConnectionPong } from "./connection-health.js";
+import {
+  CONNECTION_PING_INTERVAL_MS,
+  pingGatewayClient,
+  recordConnectionPong,
+} from "./connection-health.js";
 import { getHealthVersion, incrementPresenceVersion } from "./health-state.js";
 import type { PreauthConnectionBudget } from "./preauth-connection-budget.js";
 import { broadcastPresenceSnapshot } from "./presence-events.js";
@@ -470,13 +474,10 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti
         releasePreauthBudget();
         client = next;
         clients.add(next);
+        pingGatewayClient(next);
         pingTimer = setInterval(() => {
-          try {
-            socket.ping();
-          } catch {
-            // close() clears the timer; ping can race with a socket already entering CLOSING.
-          }
-        }, 25_000);
+          pingGatewayClient(next);
+        }, CONNECTION_PING_INTERVAL_MS);
         return true;
       },
       setHandshakeState: (next) => {

Original file line number	Diff line number	Diff line change
@@ -76,7 +76,7 @@ The health snapshot includes: `ok` (boolean), `ts` (timestamp), `durationMs` (pr
`76`	`76`	`}`
`77`	`77`	```
`78`	`78`
`79`		-`connection` is not stored in the shared cached health snapshot. It is overlaid for the requesting WebSocket client so separate clients keep independent liveness and RTT state. Before the first successful WebSocket ping/pong, `connected` can be `true` while `rttMs` and `lastHeartbeatAt` are `null`.
	`79`	+`connection` is not stored in the shared cached health snapshot. It is overlaid for the requesting WebSocket client so separate clients keep independent liveness and RTT state. RTT samples come from the authenticated WebSocket keepalive ping/pong. Before the first successful WebSocket ping/pong, `connected` can be `true` while `rttMs` and `lastHeartbeatAt` are `null`.
`80`	`80`
`81`	`81`	`## Related`
`82`	`82`
Original file line number	Diff line number	Diff line change
`@@ -119,10 +119,7 @@ export const healthHandlers: GatewayRequestHandlers = {`
`119`	`119`	`}`
`120`	`120`	`try {`
`121`	`121`	`const snap = await refreshHealthSnapshot({ probe: wantsProbe, includeSensitive });`
`122`		`- const payload = context.getEventLoopHealth`
`123`		`- ? { ...snap, eventLoop: context.getEventLoopHealth() }`
`124`		`- : snap;`
`125`		`- respond(true, withConnectionHealth(payload, client), undefined);`
	`122`	`+ respond(true, withConnectionHealth(snap, client), undefined);`
`126`	`123`	`} catch (err) {`
`127`	`124`	`respond(false, undefined, errorShape(ErrorCodes.UNAVAILABLE, formatForLog(err)));`
`128`	`125`	`}`