fix(network): enforce one active link and persist ethernet-off intent

BlocksScreen/lib/network/worker.py

@@ -8,7 +8,7 @@
 import socket as _socket
 import struct
 import threading
-from typing import Callable
+from collections.abc import Callable
 from uuid import uuid4
 
 import sdbus
@@ -284,11 +284,13 @@ def hotspot_password(self) -> str:
     async def _async_initialize(self) -> None:
         """Bootstrap the worker on the asyncio thread.
 
-        Detects network interfaces, enforces the boot-time ethernet/Wi-Fi
-        mutual exclusion, activates any saved VLANs if ethernet is present,
-        triggers an initial Wi-Fi scan, and starts all D-Bus signal listeners.
-        Emits ``initialized`` when done (even on failure, so the manager can
-        unblock its caller).
+        Detects network interfaces, activates any saved VLANs if ethernet is
+        present, triggers an initial Wi-Fi scan, and starts all D-Bus signal
+        listeners.  Emits ``initialized`` when done (even on failure, so the
+        manager can unblock its caller).
+
+        Wired autoconnect is deliberately not re-armed here: NM's latch is what
+        persists the user's "ethernet off" choice across reboots.
         """
         try:
             if not self._system_bus:
@@ -297,7 +299,6 @@ async def _async_initialize(self) -> None:
 
             self._running = True
             await self._detect_interfaces()
-            await self._enforce_boot_mutual_exclusion()
 
             if await self._is_ethernet_connected():
                 await self._activate_saved_vlans()
@@ -367,31 +368,39 @@ async def _detect_interfaces(self) -> None:
             # Ethernet-only or Wi-Fi driver still loading — log but don't alarm.
             logger.warning("No Wi-Fi interface detected; ethernet-only mode")
 
-    async def _enforce_boot_mutual_exclusion(self) -> None:
-        """Disable Wi-Fi at boot if ethernet is already connected.
+    async def _set_wired_profiles_autoconnect(self, enabled: bool) -> None:
+        """Persist autoconnect on every wired profile; Device.Autoconnect dies on NM restart."""
+        try:
+            paths = await self._nm_settings().list_connections()
+            for path, settings in await self._gather_settings(list(paths)):
+                conn = settings.get("connection", {})
+                if conn.get("type", (None, ""))[1] != "802-3-ethernet":
+                    continue
+                if bool(conn.get("autoconnect", ("b", True))[1]) == enabled:
+                    continue
+                props = {k: dict(v) for k, v in settings.items()}
+                props["connection"]["autoconnect"] = ("b", enabled)
+                props["connection"].pop("timestamp", None)
+                await self._conn_settings(path).update(props)
+                logger.info("Wired profile %s autoconnect -> %s", path, enabled)
+        except Exception as exc:
+            logger.warning("Wired profile autoconnect (%s) failed: %s", enabled, exc)
+
+    async def _ensure_wired_autoconnect(self) -> None:
+        """Re-arm wired autoconnect on both the device and the saved profiles.
 
-        Prevents the device from simultaneously using both interfaces at
-        startup.  If ethernet is active and the Wi-Fi radio is on, the Wi-Fi
-        device is disconnected and the radio is disabled, then we wait up to
-        8 s for the radio to confirm it is off.  Failures are logged but not
-        propagated — a non-fatal best-effort action at boot.
+        Called only when the user asks for ethernet, so autoconnect staying off
+        keeps meaning "user turned it off". Best-effort: never propagates.
         """
+        if not self._primary_wired_path:
+            return
+        await self._set_wired_profiles_autoconnect(True)
         try:
-            if not await self._is_ethernet_connected():
-                return
-            if not await self._nm().wireless_enabled:
-                return
-            logger.info("Boot: ethernet active + Wi-Fi enabled — disabling Wi-Fi")
-            if self._primary_wifi_path:
-                try:
-                    await self._wifi().disconnect()
-                except Exception as exc:
-                    logger.debug("Pre-radio-disable disconnect ignored: %s", exc)
-            await self._nm().wireless_enabled.set_async(False)
-            await self._wait_for_wifi_radio(False, timeout=8.0)
-            self._is_hotspot_active = False
+            wired = self._generic(self._primary_wired_path)
+            if not await wired.autoconnect:
+                await wired.autoconnect.set_async(True)
         except Exception as exc:
-            logger.warning("Boot mutual exclusion failed (non-fatal): %s", exc)
+            logger.debug("Device autoconnect re-arm ignored: %s", exc)
 
     async def _start_signal_listeners(self) -> None:
         """Create persistent proxies and spawn all D-Bus signal listeners.
@@ -749,31 +758,12 @@ async def _wait_for_wifi_device_ready(self, timeout: float = 8.0) -> bool:
         return False
 
     async def _async_get_current_state(self) -> None:
-        """Rebuild and emit the full NetworkState, enforcing runtime mutual exclusion."""
+        """Rebuild and emit the full NetworkState. Read-only: never mutates NM."""
         try:
             if not await self._ensure_dbus_connection():
                 self.state_changed.emit(NetworkState())
                 return
-            state = await self._build_current_state()
-            if (
-                state.ethernet_connected
-                and state.wifi_enabled
-                and not state.hotspot_enabled
-                and not self._is_hotspot_active
-            ):
-                logger.info(
-                    "Runtime mutual exclusion: ethernet active + "
-                    "Wi-Fi — disabling Wi-Fi"
-                )
-                if self._primary_wifi_path:
-                    try:
-                        await self._wifi().disconnect()
-                    except Exception as exc:
-                        logger.debug("Disconnect before Wi-Fi disable ignored: %s", exc)
-                await self._nm().wireless_enabled.set_async(False)
-                await asyncio.sleep(0.5)
-                state = await self._build_current_state()
-            self.state_changed.emit(state)
+            self.state_changed.emit(await self._build_current_state())
         except Exception as exc:
             logger.error("Failed to get current state: %s", exc)
             self.error_occurred.emit("get_current_state", str(exc))
@@ -1289,17 +1279,14 @@ async def _add_network_impl(
     ) -> ConnectionResult:
         """Scan for the SSID, build a connection profile, add it to NM, and activate it.
 
-        Deletes any pre-existing profile for the same SSID before adding.
+        Any pre-existing profile for the same SSID is backed up before being
+        replaced, and restored if the new credentials fail to activate.
         Returns a failed ConnectionResult if the SSID is not visible, the
         security type is unsupported, or the 20-second activation wait times out.
         """
         if not self._primary_wifi_path or not self._system_bus:
             return ConnectionResult(False, "No Wi-Fi interface", "no_interface")
 
-        if await self._is_known(ssid):
-            await self._delete_network_impl(ssid)
-            self._invalidate_saved_cache()
-
         try:
             await self._wifi().request_scan({})
         except Exception as exc:
@@ -1320,10 +1307,14 @@ async def _add_network_impl(
                 "unsupported_security",
             )
 
+        # Drop the old profile only once the new one is buildable, so early exits keep it.
+        backup = await self._backup_and_drop_existing(ssid)
         try:
             nm_settings = self._nm_settings()
             conn_path = await nm_settings.add_connection(conn_props)
         except Exception as exc:
+            if backup:
+                await self._restore_profile(ssid, backup)
             err_str = str(exc).lower()
             if "psk" in err_str and ("invalid" in err_str or "property" in err_str):
                 return ConnectionResult(
@@ -1338,15 +1329,7 @@ async def _add_network_impl(
         try:
             await self._nm().activate_connection(conn_path)
             if not await self._wait_for_connection(ssid, timeout=_WIFI_CONNECT_TIMEOUT):
-                await self._delete_network_impl(ssid)
-                self._invalidate_saved_cache()
-                return ConnectionResult(
-                    False,
-                    f"Authentication failed for '{ssid}'.\n"
-                    "The saved profile has been removed.\n"
-                    "Please check the password and try again.",
-                    "auth_failed",
-                )
+                return await self._rollback_failed_add(ssid, backup)
             return ConnectionResult(True, f"Network '{ssid}' added and connecting")
         except Exception as act_err:
             logger.warning("Activate after add failed: %s", act_err)
@@ -1361,6 +1344,66 @@ async def _reload_connections(self) -> None:
         except Exception as reload_err:
             logger.debug("reload_connections non-fatal: %s", reload_err)
 
+    async def _backup_profile(self, ssid: str) -> dict | None:
+        """Snapshot a saved profile's settings plus secrets so it can be re-added."""
+        conn_path = await self._get_connection_path(ssid)
+        if not conn_path:
+            return None
+        try:
+            cs = self._conn_settings(conn_path)
+            settings = dict(await cs.get_settings())
+            await self._merge_wifi_secrets(cs, settings)
+            settings.get("connection", {}).pop("timestamp", None)
+            logger.debug("backup_profile: '%s' sections=%s", ssid, sorted(settings))
+            return settings
+        except Exception as exc:
+            logger.warning("backup_profile: could not snapshot '%s': %s", ssid, exc)
+            return None
+
+    async def _restore_profile(self, ssid: str, settings: dict) -> bool:
+        """Re-add a backed-up profile after a failed replacement; True when restored."""
+        try:
+            await self._nm_settings().add_connection(settings)
+            self._invalidate_saved_cache()
+            logger.info("restore_profile: '%s' restored after failed add", ssid)
+            return True
+        except Exception as exc:
+            logger.error("restore_profile: could not restore '%s': %s", ssid, exc)
+            return False
+
+    async def _rollback_failed_add(
+        self, ssid: str, backup: dict | None
+    ) -> ConnectionResult:
+        """Delete the profile that never activated and restore *backup* if there is one."""
+        logger.warning("add_network: '%s' never activated, rolling back", ssid)
+        await self._delete_network_impl(ssid)
+        self._invalidate_saved_cache()
+        if backup and await self._restore_profile(ssid, backup):
+            return ConnectionResult(
+                False,
+                f"Could not connect to '{ssid}'.\n"
+                "The previously saved password was kept.\n"
+                "Please check the password and try again.",
+                "auth_failed",
+            )
+        return ConnectionResult(
+            False,
+            f"Authentication failed for '{ssid}'.\n"
+            "The saved profile has been removed.\n"
+            "Please check the password and try again.",
+            "auth_failed",
+        )
+
+    async def _backup_and_drop_existing(self, ssid: str) -> dict | None:
+        """Back up and delete a saved profile for *ssid* so it can be re-added cleanly."""
+        if not await self._is_known(ssid):
+            return None
+        backup = await self._backup_profile(ssid)
+        logger.info("add_network: replacing saved '%s' (backup=%s)", ssid, bool(backup))
+        await self._delete_network_impl(ssid)
+        self._invalidate_saved_cache()
+        return backup
+
     async def _async_connect_network(self, ssid: str) -> None:
         """Activate an existing saved Wi-Fi profile and emit connection_result."""
         try:
@@ -1585,16 +1628,13 @@ async def _update_network_impl(
             return ConnectionResult(False, str(exc), "update_failed")
 
     async def _async_set_wifi_enabled(self, enabled: bool) -> None:
-        """Enable or disable the Wi-Fi radio, handling ethernet mutual exclusion."""
+        """Enable or disable the Wi-Fi radio. Ethernet is left untouched."""
         try:
             if not self._system_bus:
                 return
             if not enabled:
                 self._is_hotspot_active = False
 
-            if enabled and await self._is_ethernet_connected():
-                await self._async_disconnect_ethernet()
-
             current = await self._nm().wireless_enabled
             if current != enabled:
                 if not enabled:
@@ -1632,7 +1672,13 @@ async def _async_disconnect_ethernet(self) -> None:
             return
         try:
             await self._deactivate_all_vlans()
-            await self._wired().disconnect()
+            try:
+                await self._wired().disconnect()
+            except Exception as exc:
+                # Already inactive is the goal state, not a failure.
+                if "not active" not in str(exc).lower():
+                    raise
+                logger.debug("Ethernet already inactive: %s", exc)
             loop = asyncio.get_running_loop()
             deadline = loop.time() + 4.0
             while loop.time() < deadline:
@@ -1642,26 +1688,23 @@ async def _async_disconnect_ethernet(self) -> None:
             logger.info("Ethernet disconnected")
         except Exception as exc:
             logger.error("Failed to disconnect ethernet: %s", exc)
+        finally:
+            # Only user toggles reach here, so record intent even if teardown failed.
+            await self._set_wired_profiles_autoconnect(False)
 
     async def _async_connect_ethernet(self) -> None:
-        """Disable Wi-Fi/hotspot, activate the wired device, and restore saved VLANs."""
+        """Activate the wired device and restore saved VLANs.
+
+        Mechanism only: the one-link-at-a-time policy lives in the UI toggles.
+        """
         if not self._primary_wired_path:
             self.error_occurred.emit("connect_ethernet", "No wired device found")
             return
         try:
             if self._is_hotspot_active:
                 await self._async_toggle_hotspot(False)
 
-            if self._primary_wifi_path:
-                try:
-                    await self._wifi().disconnect()
-                except Exception as exc:
-                    logger.debug("Pre-VLAN disconnect ignored: %s", exc)
-                await asyncio.sleep(0.5)
-
-            if await self._nm().wireless_enabled:
-                await self._nm().wireless_enabled.set_async(False)
-                await self._wait_for_wifi_radio(False, timeout=8.0)
+            await self._ensure_wired_autoconnect()
 
             await self._nm().activate_connection("/", self._primary_wired_path, "/")
             await asyncio.sleep(1.5)
@@ -1695,18 +1738,9 @@ async def _async_create_vlan(
             if self._is_hotspot_active:
                 await self._async_toggle_hotspot(False)
 
-            if self._primary_wifi_path:
-                try:
-                    await self._wifi().disconnect()
-                except Exception as exc:
-                    logger.debug("Pre-VLAN disconnect ignored: %s", exc)
-                await asyncio.sleep(0.5)
-
-            if await self._nm().wireless_enabled:
-                await self._nm().wireless_enabled.set_async(False)
-                await self._wait_for_wifi_radio(False, timeout=8.0)
-
+            # A VLAN rides on eth0; Wi-Fi is orthogonal and stays up.
             if not await self._is_ethernet_connected():
+                await self._ensure_wired_autoconnect()
                 await self._nm().activate_connection("/", self._primary_wired_path, "/")
                 await asyncio.sleep(1.5)