From c0ecc9ff7144b08f450a46d3f25408a6ee397b3d Mon Sep 17 00:00:00 2001
From: smoghe-bw <smoghe-bw>
Date: Mon, 3 Aug 2026 10:35:31 -0400
Subject: [PATCH] fix(signaling): disable auto-reconnect before closing on
 disconnect
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The rpc-websockets client is constructed with `reconnect: true` and
`max_reconnects: 0` (unlimited), and the library only skips reconnecting
when the close code is exactly 1000. Any other code reconnects forever —
including 1001/StatusGoingAway, which pv-gateway sends on every close
where it wants the device to come back (heartbeat timeout, and Handle's
teardown backstop).

_disconnect() removed all listeners and closed the socket but never
disabled reconnect, so an explicit disconnect could leave a reconnect
loop running behind a client that no longer has an "open" handler. The
reconnected socket therefore never calls setMediaPreferences, never
creates peer connections and never answers the heartbeat — an inert
connection on the gateway with no liveness check on either side, which
accumulates for as long as the process lives.

Reconnect stays enabled for the lifetime of a live connection; it is only
disabled in _disconnect(), where the caller has asked to tear this client
down for good. Failing to disable it no longer prevents the close.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/v1/signaling.test.ts | 34 ++++++++++++++++++++++++++++++++++
 src/v1/signaling.ts      | 19 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/src/v1/signaling.test.ts b/src/v1/signaling.test.ts
index c34f038..88d03c0 100644
--- a/src/v1/signaling.test.ts
+++ b/src/v1/signaling.test.ts
@@ -197,6 +197,40 @@ describe("Signaling disconnect", () => {
     expect((signaling as any).isReady).toBe(false);
   });
 
+  // The client is built with `reconnect: true, max_reconnects: 0` and
+  // rpc-websockets only skips reconnecting on close code exactly 1000. Without
+  // this, a gateway-initiated 1001 close leaves an endless reconnect loop
+  // behind a client whose listeners have already been removed — an inert
+  // connection on the gateway that neither side ever reaps.
+  test("should disable auto-reconnect before closing on disconnect", async () => {
+    const signaling = new Signaling();
+    await signaling.connect({ endpointToken: "test-token" });
+
+    const ws = (signaling as any).ws;
+    const callOrder: string[] = [];
+    ws.setAutoReconnect.mockImplementation(() => callOrder.push("setAutoReconnect"));
+    ws.close.mockImplementation(() => callOrder.push("close"));
+
+    signaling.disconnect();
+
+    expect(ws.setAutoReconnect).toHaveBeenCalledWith(false);
+    expect(callOrder).toEqual(["setAutoReconnect", "close"]);
+  });
+
+  test("should still close when disabling auto-reconnect throws", async () => {
+    const signaling = new Signaling();
+    await signaling.connect({ endpointToken: "test-token" });
+
+    const ws = (signaling as any).ws;
+    ws.setAutoReconnect.mockImplementation(() => {
+      throw new Error("socket already gone");
+    });
+
+    expect(() => signaling.disconnect()).not.toThrow();
+    expect(ws.close).toHaveBeenCalled();
+    expect((signaling as any).ws).toBeNull();
+  });
+
   test("should handle disconnect with diagnosticsBatcher", async () => {
     const diagnosticsBatcher = new DiagnosticsBatcher();
     const shutdownSpy = jest.spyOn(diagnosticsBatcher, "shutdown");
diff --git a/src/v1/signaling.ts b/src/v1/signaling.ts
index 6333d21..4c2f34c 100644
--- a/src/v1/signaling.ts
+++ b/src/v1/signaling.ts
@@ -143,6 +143,25 @@ class Signaling extends EventEmitter {
           logger.error("Error sending diagnostics... websocket may be disconnected", err);
         }
       }
+      // Stop auto-reconnect BEFORE closing. The client is constructed with
+      // `reconnect: true, max_reconnects: 0` (unlimited), and rpc-websockets
+      // only skips reconnecting when the close code is exactly 1000 — so any
+      // other code (notably 1001/StatusGoingAway, which the gateway sends
+      // whenever it wants the device to come back) puts the client into an
+      // endless reconnect loop. That loop outlives this disconnect: because
+      // removeAllListeners() has already run, the reconnected socket has no
+      // "open" handler, so it never calls setMediaPreferences, never creates
+      // peer connections, and never answers the heartbeat — an inert
+      // connection on the gateway that nothing on either side reaps.
+      //
+      // Reconnect stays enabled for the lifetime of a live connection (that is
+      // the point of it); it is only disabled here, where the caller has asked
+      // to disconnect and we are tearing this client down for good.
+      try {
+        this.ws.setAutoReconnect(false);
+      } catch (err) {
+        logger.error("Error disabling auto-reconnect", err);
+      }
       this.ws.removeAllListeners();
       try {
         this.ws.close();