fix(mobile): avoid duplicate WebSocket recovery and reject invalid frames (#8690)

apps/mobile/data/realtime/realtime-provider.test.ts

@@ -0,0 +1,76 @@
+// @vitest-environment node
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+const lifecycle = vi.hoisted(() => ({
+  setup: undefined as (() => () => void) | undefined,
+  onAppState: undefined as ((state: string) => void) | undefined,
+}));
+
+vi.mock("react", async (importOriginal) => ({
+  ...await importOriginal<typeof import("react")>(),
+  useEffect: (setup: () => () => void) => { lifecycle.setup = setup; },
+  useRef: (current: unknown) => ({ current }),
+  useState: () => [null, vi.fn()],
+}));
+vi.mock("react-native", () => ({
+  AppState: {
+    addEventListener: (_event: string, handler: (state: string) => void) => {
+      lifecycle.onAppState = handler;
+      return { remove: vi.fn() };
+    },
+  },
+}));
+vi.mock("@react-native-community/netinfo", () => ({
+  default: { addEventListener: () => vi.fn() },
+}));
+vi.mock("@/data/auth-store", () => ({ useAuthStore: () => "user-1" }));
+vi.mock("@/data/workspace-store", () => ({ useWorkspaceStore: () => "workspace" }));
+vi.mock("@/data/secure-storage", () => ({ getToken: async () => "token" }));
+vi.mock("@/data/api", () => ({ api: { getToken: () => "token" } }));
+
+// Run the provider's effect and its actual WSClient, without rendering RN.
+class MockWebSocket {
+  static instances: MockWebSocket[] = [];
+  constructor() { MockWebSocket.instances.push(this); }
+  close() {}
+}
+
+describe("RealtimeProvider foreground recovery", () => {
+  let cleanup: (() => void) | undefined;
+
+  beforeEach(() => {
+    lifecycle.setup = undefined;
+    lifecycle.onAppState = undefined;
+    MockWebSocket.instances = [];
+    vi.stubGlobal("WebSocket", MockWebSocket);
+    vi.stubEnv("EXPO_PUBLIC_API_URL", "https://example.test");
+    vi.spyOn(console, "info").mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    cleanup?.();
+    cleanup = undefined;
+    vi.unstubAllGlobals();
+    vi.unstubAllEnvs();
+    vi.restoreAllMocks();
+  });
+
+  it("opens exactly one socket per background or inactive return", async () => {
+    const { RealtimeProvider } = await import("./realtime-provider");
+    RealtimeProvider({ children: null });
+    cleanup = lifecycle.setup!();
+    // Let the effect finish reading the token and install native listeners.
+    await Promise.resolve();
+    expect(MockWebSocket.instances).toHaveLength(1);
+
+    lifecycle.onAppState!("background");
+    expect(MockWebSocket.instances).toHaveLength(1);
+    lifecycle.onAppState!("active");
+    expect(MockWebSocket.instances).toHaveLength(2);
+
+    lifecycle.onAppState!("inactive");
+    expect(MockWebSocket.instances).toHaveLength(2);
+    lifecycle.onAppState!("active");
+    expect(MockWebSocket.instances).toHaveLength(3);
+  });
+});

apps/mobile/data/realtime/realtime-provider.tsx

@@ -108,9 +108,8 @@ export function RealtimeProvider({ children }: { children: React.ReactNode }) {
           if (status === "active") {
             // Foreground. The socket may have been paused (we put it
             // there on background) or it may be a zombie (iOS killed
-            // it silently). Either way: resume / force-reconnect.
+            // it silently). resume() handles both with one fresh socket.
             ws?.resume();
-            ws?.forceReconnect();
           } else if (status === "background") {
             ws?.pause();
           }

apps/mobile/data/realtime/ws-client.test.ts

@@ -1,3 +1,4 @@
+// @vitest-environment node
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { WSClient } from "./ws-client";
 
@@ -149,3 +150,111 @@ describe("WSClient session renewal", () => {
     });
   });
 });
+
+describe("WSClient frame validation", () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    vi.stubGlobal("WebSocket", MockWebSocket);
+    MockWebSocket.instances = [];
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.unstubAllGlobals();
+  });
+
+  it.each([
+    null,
+    false,
+    42,
+    "event",
+    [],
+    {},
+    { error: "unauthorized" },
+    { type: null },
+    { type: 42 },
+    { type: {} },
+    { type: "" },
+  ])("ignores invalid envelopes and still delivers the next event: %j", (frame) => {
+    const { client, socket } = connectAuthenticatedClient();
+    const onAny = vi.fn();
+    const onRunning = vi.fn();
+    client.onAny(onAny);
+    client.on("task:running", onRunning);
+
+    expect(() => socket.receive(frame)).not.toThrow();
+    expect(onAny).not.toHaveBeenCalled();
+    expect(onRunning).not.toHaveBeenCalled();
+
+    const payload = { task_id: "task-1", agent_id: "agent-1", issue_id: "issue-1" };
+    const message = { type: "task:running", payload, actor_id: "actor-1" };
+    socket.receive(message);
+
+    expect(onRunning).toHaveBeenCalledExactlyOnceWith(payload, "actor-1");
+    expect(onAny).toHaveBeenCalledExactlyOnceWith(message);
+    client.disconnect();
+  });
+});
+
+describe("WSClient foreground recovery", () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    vi.stubGlobal("WebSocket", MockWebSocket);
+    MockWebSocket.instances = [];
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.unstubAllGlobals();
+    vi.restoreAllMocks();
+  });
+
+  it.each(["paused", "active"])("opens one fresh socket on returning from %s", (state) => {
+    const { client, socket } = connectAuthenticatedClient();
+    const onReconnect = vi.fn();
+    client.onReconnect(onReconnect);
+    if (state === "paused") client.pause();
+
+    client.resume();
+
+    expect(MockWebSocket.instances).toHaveLength(2);
+    expect(socket.readyState).toBe(MockWebSocket.CLOSED);
+    expect(onReconnect).not.toHaveBeenCalled();
+    const resumed = MockWebSocket.instances[1];
+    resumed.open();
+    resumed.receive({ type: "auth_ack" });
+    resumed.receive({ type: "pong" });
+    expect(onReconnect).toHaveBeenCalledTimes(1);
+
+    vi.advanceTimersByTime(10_001);
+    expect(MockWebSocket.instances).toHaveLength(2);
+    client.disconnect();
+  });
+
+  it("cancels a pending reconnect when returning to the foreground", () => {
+    vi.spyOn(Math, "random").mockReturnValue(0.5);
+    const { client, socket } = connectAuthenticatedClient();
+    socket.close();
+
+    client.resume();
+
+    expect(MockWebSocket.instances).toHaveLength(2);
+    vi.advanceTimersByTime(2_000);
+    expect(MockWebSocket.instances).toHaveLength(2);
+    client.disconnect();
+  });
+
+  it("does not resume after disconnect or wake a paused client on a network edge", () => {
+    const { client } = connectAuthenticatedClient();
+    client.pause();
+    client.forceReconnect();
+    vi.advanceTimersByTime(60_000);
+    expect(MockWebSocket.instances).toHaveLength(1);
+
+    client.disconnect();
+    client.resume();
+    client.forceReconnect();
+    vi.advanceTimersByTime(60_000);
+    expect(MockWebSocket.instances).toHaveLength(1);
+  });
+});

apps/mobile/data/realtime/ws-client.ts

@@ -136,12 +136,12 @@ export class WSClient {
     this.teardownSocket();
   }
 
-  /** Paused → active. Used by the provider when AppState=active. */
+  /** Return to the foreground with one fresh socket. An active socket may
+   *  also need replacing after iOS silently drops the connection. */
   resume() {
-    if (this.state !== "paused") return;
+    if (this.state === "idle") return;
     this.state = "active";
-    this.reconnectAttempt = 0;
-    this.openSocket();
+    this.forceReconnect();
   }
 
   /** Force a fresh socket without going through paused. Used when NetInfo
@@ -232,7 +232,16 @@ export class WSClient {
         return;
       }
 
-      const type = (msg as { type?: string }).type;
+      // Validate the envelope before transport frames or business dispatch.
+      // JSON primitives (including null) and non-string types are not events.
+      const type = (msg as { type?: unknown } | null)?.type;
+      if (typeof type !== "string" || !type) {
+        // Server-side error frames have shape {error: "..."}; log and drop.
+        // Reconnect loop is bounded by auth-store's 401 handler eventually
+        // tearing this client down via disconnect().
+        this.logger.warn("[ws] frame without a string type", event.data);
+        return;
+      }
       if (type === "auth_ack") {
         this.onAuthenticated();
         return;
@@ -241,14 +250,6 @@ export class WSClient {
         this.onPong();
         return;
       }
-      if (!type) {
-        // Server-side error frames have shape {error: "..."}; log and drop.
-        // Reconnect loop is bounded by auth-store's 401 handler eventually
-        // tearing this client down via disconnect().
-        this.logger.warn("[ws] frame without type", event.data);
-        return;
-      }
-
       this.logger.debug("[ws] event", type);
       const set = this.handlers.get(msg.type);
       if (set) {