diff --git a/system/ui/lib/tests/test_handle_state_change.py b/system/ui/lib/tests/test_handle_state_change.py index eefd83c628..9e7c39be55 100644 --- a/system/ui/lib/tests/test_handle_state_change.py +++ b/system/ui/lib/tests/test_handle_state_change.py @@ -221,17 +221,12 @@ class TestNeedAuth: assert len(wm._callback_queue) == 0 - @pytest.mark.xfail(reason="TODO: interrupted auth (prev=DISCONNECTED) should be ignored") def test_interrupted_auth_ignored(self, mocker): """Switching A->B: NEED_AUTH from A (prev=DISCONNECTED) must not fire callback. Reproduced on device: rapidly switching between two saved networks can trigger a rare false "wrong password" dialog for the previous network, even though both have correct passwords. The stale NEED_AUTH has prev_state=DISCONNECTED (not CONFIG). - - Fix: the handler's second param is currently `_` (unused). Rename to `prev_state` - and only fire the NEED_AUTH callback when prev_state indicates a real auth failure - (e.g. prev_state == CONFIG), not a stale signal from a prior connection. """ wm = _make_wm(mocker) cb = mocker.MagicMock() @@ -246,26 +241,6 @@ class TestNeedAuth: assert wm._wifi_state.status == ConnectStatus.CONNECTING assert len(wm._callback_queue) == 0 - def test_need_auth_targets_previous_ssid_via_prev_ssid(self, mocker): - """Switch A→B, late NEED_AUTH arrives: prev_ssid mechanism fires callback for A. - - This tests current prev_ssid behavior which we plan to remove. - Migration: (1) add prev_state guard to NEED_AUTH (see test_interrupted_auth_ignored), - (2) remove prev_ssid from WifiState and handler, (3) delete this test — it becomes - redundant with test_interrupted_auth_ignored once prev_state is the guard mechanism. - """ - wm = _make_wm(mocker) - cb = mocker.MagicMock() - wm.add_callbacks(need_auth=cb) - wm._set_connecting("A") - wm._set_connecting("B") - - fire(wm, NMDeviceState.NEED_AUTH, reason=NMDeviceStateReason.SUPPLICANT_DISCONNECT) - - assert len(wm._callback_queue) == 1 - wm.process_callbacks() - cb.assert_called_once_with("A") - class TestPassthroughStates: """NEED_AUTH (generic), IP_CONFIG, IP_CHECK, SECONDARIES, FAILED (generic) are no-ops.""" @@ -335,7 +310,6 @@ class TestActivated: # on NEED_AUTH) also shrink these race windows to near-zero. If races are still # visible after, make WifiState frozen (replace() + single atomic assignment) and/or # add a narrow lock around _wifi_state reads/writes (not around DBus calls). -# The NEED_AUTH prev_ssid mutation race is eliminated by removing prev_ssid entirely. class TestThreadRaces: @pytest.mark.xfail(reason="TODO: PREPARE overwrites _set_connecting via stale DBus lookup") @@ -482,7 +456,6 @@ class TestFullSequences: assert wm._wifi_state.status == ConnectStatus.CONNECTED assert wm._wifi_state.ssid == "B" - @pytest.mark.xfail(reason="TODO: interrupted auth from switching should not fire need_auth") def test_rapid_switch_no_false_wrong_password(self, mocker): """Switch A→B quickly: A's interrupted NEED_AUTH must NOT show wrong password. diff --git a/system/ui/lib/wifi_manager.py b/system/ui/lib/wifi_manager.py index 5d6cd8a785..4b5711428b 100644 --- a/system/ui/lib/wifi_manager.py +++ b/system/ui/lib/wifi_manager.py @@ -148,7 +148,6 @@ class ConnectStatus(IntEnum): @dataclass class WifiState: ssid: str | None = None - prev_ssid: str | None = None status: ConnectStatus = ConnectStatus.DISCONNECTED @@ -292,10 +291,7 @@ class WifiManager: return self._tethering_password def _set_connecting(self, ssid: str | None): - # Track prev ssid so late NEED_AUTH signals target the right network - self._wifi_state = WifiState(ssid=ssid, - prev_ssid=self.connecting_to_ssid if ssid is not None else None, - status=ConnectStatus.DISCONNECTED if ssid is None else ConnectStatus.CONNECTING) + self._wifi_state = WifiState(ssid=ssid, status=ConnectStatus.DISCONNECTED if ssid is None else ConnectStatus.CONNECTING) def _enqueue_callbacks(self, cbs: list[Callable], *args): for cb in cbs: @@ -377,7 +373,7 @@ class WifiManager: self._handle_state_change(new_state, previous_state, change_reason) - def _handle_state_change(self, new_state: int, _: int, change_reason: int): + def _handle_state_change(self, new_state: int, prev_state: int, change_reason: int): # TODO: known race conditions when switching networks (e.g. forget A, connect to B): # 1. DEACTIVATING/DISCONNECTED + CONNECTION_REMOVED: fires before NewConnection for B # arrives, so _set_connecting(None) clears B's CONNECTING state causing UI flicker. @@ -398,8 +394,6 @@ class WifiManager: # deterministic fix (skip DBus lookup when ssid is set) also shrinks the race # window to near-zero since the read/write happen back-to-back under the GIL. # ACTIVATED: same read-then-write pattern with a DBus call in between. - # NEED_AUTH: mutates _wifi_state.prev_ssid in place, which can corrupt a new - # WifiState created by _set_connecting on the main thread. # The deterministic fixes (skip DBus lookup when ssid set, prev_state guard) shrink # the race windows significantly. If still visible, add a narrow lock around # _wifi_state reads/writes (not around DBus calls, to avoid blocking the UI thread). @@ -424,18 +418,19 @@ class WifiManager: self._wifi_state = wifi_state - # BAD PASSWORD - use prev if current has already moved on to a new connection + # BAD PASSWORD # - strong network rejects with NEED_AUTH+SUPPLICANT_DISCONNECT # - weak/gone network fails with FAILED+NO_SECRETS - elif ((new_state == NMDeviceState.NEED_AUTH and change_reason == NMDeviceStateReason.SUPPLICANT_DISCONNECT) or + # prev_state guard: real auth failures come from CONFIG (supplicant handshake). + # Stale NEED_AUTH from a prior connection during network switching arrives with + # prev_state=DISCONNECTED and must be ignored to avoid a false wrong-password callback. + elif ((new_state == NMDeviceState.NEED_AUTH and change_reason == NMDeviceStateReason.SUPPLICANT_DISCONNECT + and prev_state == NMDeviceState.CONFIG) or (new_state == NMDeviceState.FAILED and change_reason == NMDeviceStateReason.NO_SECRETS)): - failed_ssid = self._wifi_state.prev_ssid or self._wifi_state.ssid - if failed_ssid: - self._enqueue_callbacks(self._need_auth, failed_ssid) - self._wifi_state.prev_ssid = None - if self._wifi_state.ssid == failed_ssid: - self._set_connecting(None) + if self._wifi_state.ssid: + self._enqueue_callbacks(self._need_auth, self._wifi_state.ssid) + self._set_connecting(None) elif new_state in (NMDeviceState.NEED_AUTH, NMDeviceState.IP_CONFIG, NMDeviceState.IP_CHECK, NMDeviceState.SECONDARIES, NMDeviceState.FAILED): @@ -443,7 +438,7 @@ class WifiManager: elif new_state == NMDeviceState.ACTIVATED: # Note that IP address from Ip4Config may not be propagated immediately and could take until the next scan results - wifi_state = replace(self._wifi_state, prev_ssid=None, status=ConnectStatus.CONNECTED) + wifi_state = replace(self._wifi_state, status=ConnectStatus.CONNECTED) conn_path, _ = self._get_active_wifi_connection(self._conn_monitor) if conn_path is None: