Settings Wi-Fi + Bluetooth panels: ignore results from a cleaned-up effect or a superseded load (CodeRabbit Minor on #4017)

changelog.d/tsk-w4432q-wifi-bluetooth-race-fix.md

@@ -0,0 +1,2 @@
+### Fixed
+- Fixed race condition in Wi-Fi and Bluetooth settings panels where stale API responses could overwrite newer data. Under React StrictMode or when a slow request overlapped with the 10s refresh interval, an older response could restore old access points, adapters, devices, or errors. Both panels now use a sequence counter (`seq`) and an `active` flag to ignore results from superseded or cleaned-up effect runs.

desktop/src/apps/SettingsApp/BluetoothPanel.test.tsx

@@ -96,6 +96,38 @@ describe("BluetoothPanel", () => {
     expect(fetchSpy).toHaveBeenCalledTimes(2);
     vi.useRealTimers();
   }, 15000);
+
+  it("ignores stale response when a newer load starts before the first resolves", async () => {
+    vi.useFakeTimers({ toFake: ["setInterval", "clearInterval"] });
+    let resolveFirst: (value: Response) => void;
+    let resolveSecond: (value: Response) => void;
+    const firstPromise = new Promise<Response>((resolve) => {
+      resolveFirst = resolve;
+    });
+    const secondPromise = new Promise<Response>((resolve) => {
+      resolveSecond = resolve;
+    });
+    const fetchSpy = vi.spyOn(globalThis, "fetch");
+    fetchSpy.mockReturnValueOnce(firstPromise);
+    fetchSpy.mockReturnValueOnce(secondPromise);
+    render(<BluetoothPanel />);
+    // Advance timers to trigger the interval (second fetch)
+    await act(async () => {
+      vi.advanceTimersByTime(10_000);
+    });
+    // Now resolve the second fetch first (newer load)
+    await act(async () => {
+      resolveSecond!(json({ available: true, adapters: [{ path: "/org/bluez/hci0", address: "AA:BB:CC:DD:EE:FF", powered: true }], devices: [{ path: "/org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF", address: "AA:BB:CC:DD:EE:FF", name: "SecondDevice", paired: true, connected: true, trusted: true }] }));
+    });
+    await waitFor(() => expect(screen.getByText("SecondDevice")).toBeInTheDocument());
+    // Then resolve the first fetch (older load)
+    await act(async () => {
+      resolveFirst!(json({ available: true, adapters: [{ path: "/org/bluez/hci0", address: "AA:BB:CC:DD:EE:FF", powered: true }], devices: [{ path: "/org/bluez/hci0/dev_11_22_33_44_55_66", address: "11:22:33:44:55:66", name: "FirstDevice", paired: true, connected: true, trusted: true }] }));
+    });
+    expect(screen.getByText("SecondDevice")).toBeInTheDocument();
+    expect(screen.queryByText("FirstDevice")).not.toBeInTheDocument();
+    vi.useRealTimers();
+  });
 });
 
 describe("BluetoothPanel StrictMode", () => {

desktop/src/apps/SettingsApp/BluetoothPanel.tsx

@@ -1,40 +1,48 @@
-import { useState, useEffect, useRef } from "react";
+import { useState, useEffect } from "react";
 import { Card } from "@/components/ui";
 
 export function BluetoothPanel() {
   const [available, setAvailable] = useState<boolean | null>(null);
   const [adapters, setAdapters] = useState<{ path: string; address: string; powered: boolean }[]>([]);
   const [devices, setDevices] = useState<{ path: string; address: string; name: string | null; paired: boolean; connected: boolean; trusted: boolean }[]>([]);
   const [error, setError] = useState<string | null>(null);
-  const mounted = useRef(true);
 
   useEffect(() => {
-    mounted.current = true;
+    let active = true;
+    let seq = 0;
     const load = async () => {
+      const mine = ++seq;
       try {
         const res = await fetch("/api/hardware/bluetooth", { credentials: "include" });
+        if (!active || mine !== seq) return;
         const body = await res.json().catch(() => null);
-        if (!mounted.current) return;
+        if (!active || mine !== seq) return;
         if (!res.ok || body === null || typeof body.available !== "boolean") {
+          if (!active || mine !== seq) return;
           setError("Could not load Bluetooth state.");
           return;
         }
+        if (!active || mine !== seq) return;
         setAvailable(body.available);
         if (Array.isArray(body.adapters)) {
+          if (!active || mine !== seq) return;
           setAdapters(body.adapters);
         }
         if (Array.isArray(body.devices)) {
+          if (!active || mine !== seq) return;
           setDevices(body.devices);
         }
+        if (!active || mine !== seq) return;
         setError(null);
       } catch {
-        if (mounted.current) setError("Could not reach taOS.");
+        if (!active || mine !== seq) return;
+        setError("Could not reach taOS.");
       }
     };
     load();
     const id = setInterval(load, 10_000);
     return () => {
-      mounted.current = false;
+      active = false;
       clearInterval(id);
     };
   }, []);

desktop/src/apps/SettingsApp/WifiPanel.test.tsx

@@ -93,6 +93,38 @@ it("refreshes access points every 10s and clears interval on unmount", async ()
     expect(fetchSpy).toHaveBeenCalledTimes(2);
     vi.useRealTimers();
   }, 15000);
+
+  it("ignores stale response when a newer load starts before the first resolves", async () => {
+    vi.useFakeTimers({ toFake: ["setInterval", "clearInterval"] });
+    let resolveFirst: (value: Response) => void;
+    let resolveSecond: (value: Response) => void;
+    const firstPromise = new Promise<Response>((resolve) => {
+      resolveFirst = resolve;
+    });
+    const secondPromise = new Promise<Response>((resolve) => {
+      resolveSecond = resolve;
+    });
+    const fetchSpy = vi.spyOn(globalThis, "fetch");
+    fetchSpy.mockReturnValueOnce(firstPromise);
+    fetchSpy.mockReturnValueOnce(secondPromise);
+    render(<WifiPanel />);
+    // Advance timers to trigger the interval (second fetch)
+    await act(async () => {
+      vi.advanceTimersByTime(10_000);
+    });
+    // Now resolve the second fetch first (newer load)
+    await act(async () => {
+      resolveSecond!(json({ available: true, wifi_enabled: true, access_points: [{ ssid: "SecondAP", strength: 90, frequency: 2437, secured: true, active: true }] }));
+    });
+    await waitFor(() => expect(screen.getByText("SecondAP")).toBeInTheDocument());
+    // Then resolve the first fetch (older load)
+    await act(async () => {
+      resolveFirst!(json({ available: true, wifi_enabled: true, access_points: [{ ssid: "FirstAP", strength: 50, frequency: 2412, secured: true, active: false }] }));
+    });
+    expect(screen.getByText("SecondAP")).toBeInTheDocument();
+    expect(screen.queryByText("FirstAP")).not.toBeInTheDocument();
+    vi.useRealTimers();
+  });
 });
 
 describe("WifiPanel StrictMode", () => {

desktop/src/apps/SettingsApp/WifiPanel.tsx

@@ -1,4 +1,4 @@
-import { useState, useEffect, useRef } from "react";
+import { useState, useEffect } from "react";
 import { AlertCircle, Wifi as WifiIcon } from "lucide-react";
 import { Card } from "@/components/ui";
 
@@ -7,33 +7,41 @@ export function WifiPanel() {
   const [wifiEnabled, setWifiEnabled] = useState<boolean | null>(null);
   const [accessPoints, setAccessPoints] = useState<{ ssid: string; strength: number; frequency: number; secured: boolean; active: boolean }[]>([]);
   const [error, setError] = useState<string | null>(null);
-  const mounted = useRef(true);
 
   useEffect(() => {
-    mounted.current = true;
+    let active = true;
+    let seq = 0;
     const load = async () => {
+      const mine = ++seq;
       try {
         const res = await fetch("/api/hardware/wifi", { credentials: "include" });
+        if (!active || mine !== seq) return;
         const body = await res.json().catch(() => null);
-        if (!mounted.current) return;
+        if (!active || mine !== seq) return;
         if (!res.ok || body === null || typeof body.available !== "boolean") {
+          if (!active || mine !== seq) return;
           setError("Could not load Wi-Fi state.");
           return;
         }
+        if (!active || mine !== seq) return;
         setAvailable(body.available);
+        if (!active || mine !== seq) return;
         setWifiEnabled(body.wifi_enabled ?? null);
         if (body.available && body.wifi_enabled && Array.isArray(body.access_points)) {
+          if (!active || mine !== seq) return;
           setAccessPoints(body.access_points);
         }
+        if (!active || mine !== seq) return;
         setError(null);
       } catch {
-        if (mounted.current) setError("Could not reach taOS.");
+        if (!active || mine !== seq) return;
+        setError("Could not reach taOS.");
       }
     };
     load();
     const id = setInterval(load, 10_000);
     return () => {
-      mounted.current = false;
+      active = false;
       clearInterval(id);
     };
   }, []);