fix(wifi): restore safe AP-enable trigger; decouple internet check from AP logic

The previous commit introduced _check_internet_connectivity() into
check_and_manage_ap_mode(), which shared the same _disconnected_checks counter
that triggers AP enable. This created a false-positive risk: 90 seconds of
packet loss on working WiFi would enable AP mode and kick off the connection.

Fix: restore nmcli association state as the sole AP-enable trigger (original,
safe behaviour). The internet connectivity check is now used only in the daemon
watchdog for the NM-restart escalation — matching how adsb-feeder-image actually
structures the two concerns (initial setup detection vs. ongoing monitoring).

Also clarify daemon comment: the connectivity check runs once per cycle in the
watchdog block, not inside check_and_manage_ap_mode, so there is no double-call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

scripts/utils/wifi_monitor_daemon.py

@@ -43,7 +43,11 @@ class WiFiMonitorDaemon:
         self.wifi_manager = WiFiManager()
         self.running = True
         self.last_state = None
-        
+        # Counts consecutive checks where nmcli says "connected" but internet is unreachable.
+        # After _nm_restart_threshold failures, NetworkManager is restarted as a recovery step.
+        self._consecutive_internet_failures = 0
+        self._nm_restart_threshold = 5  # ~2.5 min at 30s interval
+
         # Register signal handlers for graceful shutdown
         signal.signal(signal.SIGINT, self._signal_handler)
         signal.signal(signal.SIGTERM, self._signal_handler)
@@ -122,6 +126,29 @@ class WiFiMonitorDaemon:
                     else:
                         logger.debug(f"Status check: WiFi=disconnected, Ethernet={updated_ethernet}, AP={updated_status.ap_mode_active}")
                 
+                # Escalating recovery: if nmcli reports connected but actual internet
+                # is unreachable for several consecutive checks, restart NetworkManager.
+                # This is done HERE (not inside check_and_manage_ap_mode) to keep the
+                # AP-enable trigger clean and avoid false-positive AP enables from
+                # transient packet loss on otherwise working WiFi.
+                if updated_status.connected and not updated_status.ap_mode_active:
+                    if not self.wifi_manager._check_internet_connectivity():
+                        self._consecutive_internet_failures += 1
+                        logger.warning(
+                            f"Internet unreachable despite nmcli connection "
+                            f"({self._consecutive_internet_failures}/{self._nm_restart_threshold})"
+                        )
+                        if self._consecutive_internet_failures >= self._nm_restart_threshold:
+                            logger.warning("Restarting NetworkManager to recover internet connectivity")
+                            import subprocess as _sp
+                            _sp.run(["sudo", "systemctl", "restart", "NetworkManager"],
+                                    capture_output=True, timeout=20)
+                            self._consecutive_internet_failures = 0
+                    else:
+                        self._consecutive_internet_failures = 0
+                else:
+                    self._consecutive_internet_failures = 0
+
                 # Sleep until next check
                 time.sleep(self.check_interval)
                 

src/wifi_manager.py

@@ -60,6 +60,11 @@ def get_wifi_config_path():
 
 HOSTAPD_CONFIG_PATH = Path("/etc/hostapd/hostapd.conf")
 DNSMASQ_CONFIG_PATH = Path("/etc/dnsmasq.d/ledmatrix-captive.conf")
+# Drop-in config for NetworkManager's built-in dnsmasq (ipv4.method=shared).
+# Writing address=/#/<ap_ip> here causes NM to resolve every hostname to the AP,
+# triggering the OS captive-portal popup automatically on iOS/Android/Windows/macOS.
+NM_DNSMASQ_SHARED_DIR = Path("/etc/NetworkManager/dnsmasq-shared.d")
+NM_DNSMASQ_SHARED_CONF = NM_DNSMASQ_SHARED_DIR / "ledmatrix-captive.conf"
 HOSTAPD_SERVICE = "hostapd"
 DNSMASQ_SERVICE = "dnsmasq"
 
@@ -137,6 +142,9 @@ class WiFiManager:
         self._disconnected_checks = 0
         self._disconnected_checks_required = 3  # Require 3 consecutive disconnected checks (90 seconds at 30s interval)
 
+        # Timestamp set when AP mode is enabled; used for the idle-timeout check
+        self._ap_enabled_at: Optional[float] = None
+
         logger.info(f"WiFi Manager initialized - nmcli: {self.has_nmcli}, iwlist: {self.has_iwlist}, "
                    f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
                    f"interface: {self._wifi_interface}, trixie: {self._is_trixie}")
@@ -788,6 +796,10 @@ class WiFiManager:
             return True
         except Exception as e:
             logger.warning(f"Could not set up iptables redirect: {e}")
+            try:
+                self._teardown_iptables_redirect()
+            except Exception as cleanup_e:
+                logger.warning(f"Cleanup after iptables redirect exception also failed: {cleanup_e}")
             return False
 
     def _teardown_iptables_redirect(self) -> None:
@@ -833,6 +845,88 @@ class WiFiManager:
         except Exception as e:
             logger.warning(f"Could not tear down iptables redirect: {e}")
 
+    def _write_nm_dnsmasq_captive_conf(self, ap_ip: str = "192.168.4.1") -> None:
+        """
+        Write the NM dnsmasq-shared.d drop-in that makes NM's built-in dnsmasq
+        resolve every hostname to the AP IP.  This triggers the OS captive-portal
+        popup automatically on iOS / Android / Windows / macOS as soon as the
+        device connects — no manual navigation required.
+
+        NetworkManager reads /etc/NetworkManager/dnsmasq-shared.d/*.conf when it
+        starts the dnsmasq instance for ipv4.method=shared connections.
+        """
+        try:
+            content = f"# LEDMatrix captive portal: resolve all hostnames to AP\naddress=/#/{ap_ip}\n"
+            with open("/tmp/ledmatrix-nm-dnsmasq.conf", "w") as f:
+                f.write(content)
+            subprocess.run(
+                ["sudo", "mkdir", "-p", str(NM_DNSMASQ_SHARED_DIR)],
+                capture_output=True, timeout=5
+            )
+            subprocess.run(
+                ["sudo", "cp", "/tmp/ledmatrix-nm-dnsmasq.conf", str(NM_DNSMASQ_SHARED_CONF)],
+                capture_output=True, timeout=5
+            )
+            logger.info(f"Wrote NM dnsmasq captive-portal config: {NM_DNSMASQ_SHARED_CONF}")
+        except Exception as e:
+            logger.warning(f"Could not write NM dnsmasq captive config: {e}")
+
+    def _remove_nm_dnsmasq_captive_conf(self) -> None:
+        """Remove the NM dnsmasq-shared.d drop-in written by _write_nm_dnsmasq_captive_conf."""
+        try:
+            subprocess.run(
+                ["sudo", "rm", "-f", str(NM_DNSMASQ_SHARED_CONF)],
+                capture_output=True, timeout=5
+            )
+            logger.info("Removed NM dnsmasq captive-portal config")
+        except Exception as e:
+            logger.warning(f"Could not remove NM dnsmasq captive config: {e}")
+
+    def _check_internet_connectivity(self, timeout: int = 5) -> bool:
+        """
+        Test actual internet reachability — not just nmcli association state.
+
+        A device can be 'connected' in nmcli (associated with an AP) while the
+        router has no WAN link.  This check catches that case so the daemon can
+        auto-enable AP mode even when nmcli reports a connection.
+
+        Returns True if at least one reachability method succeeds.
+        """
+        try:
+            r = subprocess.run(
+                ["ping", "-c", "1", "-W", str(timeout), "8.8.8.8"],
+                capture_output=True, timeout=timeout + 1
+            )
+            if r.returncode == 0:
+                logger.debug("Internet connectivity confirmed via ping 8.8.8.8")
+                return True
+        except Exception:
+            pass
+        try:
+            import urllib.request as _ureq
+            _ureq.urlopen("http://connectivity-check.ubuntu.com/", timeout=timeout)
+            logger.debug("Internet connectivity confirmed via HTTP check")
+            return True
+        except Exception:
+            pass
+        logger.debug("Internet connectivity check failed (both ping and HTTP)")
+        return False
+
+    def _has_ap_clients(self) -> bool:
+        """
+        Return True if at least one client is associated with the AP.
+        Uses 'iw dev <iface> station dump' which works for both hostapd and
+        nmcli AP modes.
+        """
+        try:
+            result = subprocess.run(
+                ["iw", "dev", self._wifi_interface, "station", "dump"],
+                capture_output=True, text=True, timeout=5
+            )
+            return bool(result.stdout.strip())
+        except Exception:
+            return False
+
     def scan_networks(self, allow_cached: bool = True) -> Tuple[List[WiFiNetwork], bool]:
         """
         Scan for available WiFi networks.
@@ -1467,12 +1561,27 @@ class WiFiManager:
                 error_msg = result.stderr.strip() or result.stdout.strip()
                 logger.error(f"Failed to connect to {ssid}: {error_msg}")
                 self._show_led_message("Connection failed", duration=5)
+                if self._is_wrong_password_error(error_msg):
+                    return False, f"wrong_password: {error_msg}"
                 return False, error_msg
         except Exception as e:
             logger.error(f"Error connecting with nmcli: {e}")
             self._show_led_message("Connection error", duration=5)
             return False, str(e)
-    
+
+    @staticmethod
+    def _is_wrong_password_error(error_msg: str) -> bool:
+        """Return True when nmcli's error output indicates an authentication failure."""
+        indicators = [
+            "secrets were required",
+            "no secret agent",
+            "802-11-wireless-security.psk",
+            "authentication rejected",
+            "association rejected",
+        ]
+        lower = error_msg.lower()
+        return any(ind in lower for ind in indicators)
+
     def _connect_wpa_supplicant(self, ssid: str, password: str) -> Tuple[bool, str]:
         """Connect using wpa_supplicant (fallback)"""
         try:
@@ -1744,14 +1853,18 @@ class WiFiManager:
             if self.has_hostapd and self.has_dnsmasq:
                 result = self._enable_ap_mode_hostapd()
                 if result[0]:
+                    self._ap_enabled_at = time.time()
                     return result
-            
+
             # Fallback to nmcli hotspot (simpler, no captive portal)
             if self.has_nmcli:
                 logger.info("hostapd/dnsmasq failed or unavailable, trying nmcli hotspot fallback...")
                 self._show_led_message("Setup Mode", duration=5)
-                return self._enable_ap_mode_nmcli_hotspot()
-            
+                result = self._enable_ap_mode_nmcli_hotspot()
+                if result[0]:
+                    self._ap_enabled_at = time.time()
+                return result
+
             return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)"
         except Exception as e:
             logger.error(f"Error in enable_ap_mode: {e}")
@@ -1824,10 +1937,17 @@ class WiFiManager:
                     return False, f"Failed to start dnsmasq: {result.stderr}"
                 
                 # Set up iptables port forwarding (port 80 → 5000) and save ip_forward state
-                self._setup_iptables_redirect()
-                
+                if not self._setup_iptables_redirect():
+                    logger.error("Captive-portal redirect setup failed; stopping AP services")
+                    subprocess.run(["sudo", "systemctl", "stop", HOSTAPD_SERVICE],
+                                   capture_output=True, timeout=10)
+                    subprocess.run(["sudo", "systemctl", "stop", DNSMASQ_SERVICE],
+                                   capture_output=True, timeout=10)
+                    return False, "AP started but captive-portal redirect setup failed"
+
                 logger.info("AP mode enabled successfully")
-                ap_ssid = self.config.get("ap_ssid", DEFAULT_AP_SSID)
+                # Use the validated SSID so the displayed name matches what hostapd broadcast
+                ap_ssid, _ = self._validate_ap_config()
                 self._show_led_message(
                     f"WiFi Setup\n{ap_ssid}\nNo password\n192.168.4.1:5000", duration=10
                 )
@@ -1900,6 +2020,12 @@ class WiFiManager:
                 self._show_led_message("AP mode failed", duration=5)
                 return False, f"Failed to create AP profile: {error_msg}"
 
+            # Write the NM dnsmasq-shared.d captive-portal config BEFORE bringing up
+            # the connection so NM's dnsmasq picks it up at start time.
+            # This causes every hostname DNS query from a connected device to resolve
+            # to 192.168.4.1, automatically triggering the OS captive-portal popup.
+            self._write_nm_dnsmasq_captive_conf()
+
             logger.info("AP connection profile created, bringing it up...")
             up_result = subprocess.run(
                 ["nmcli", "connection", "up", "LEDMatrix-Setup-AP"],
@@ -1917,7 +2043,14 @@ class WiFiManager:
 
             # NM's ipv4.method=shared manages ip_forward automatically, so we only
             # need to add the iptables port-redirect rules for the captive portal.
-            self._setup_iptables_redirect()
+            if not self._setup_iptables_redirect():
+                logger.error("Captive-portal redirect setup failed; rolling back AP profile")
+                subprocess.run(["nmcli", "connection", "down", "LEDMatrix-Setup-AP"],
+                               capture_output=True, timeout=10)
+                subprocess.run(["nmcli", "connection", "delete", "LEDMatrix-Setup-AP"],
+                               capture_output=True, timeout=10)
+                self._clear_led_message()
+                return False, "AP started but captive-portal redirect setup failed"
 
             # Verify the AP is actually running
             status = self._get_ap_status_nmcli()
@@ -2109,11 +2242,13 @@ class WiFiManager:
                     # so we only need to remove the iptables redirect rules we added.
                     logger.info("Skipping NetworkManager restart (nmcli AP mode, restart not needed)")
                     self._teardown_iptables_redirect()
+                    self._remove_nm_dnsmasq_captive_conf()
                     # Ensure WiFi radio is enabled after nmcli operations
                     wifi_enabled = self._ensure_wifi_radio_enabled(max_retries=3)
                     if not wifi_enabled:
                         logger.warning("WiFi radio may be disabled after nmcli AP cleanup")
-                
+
+                self._ap_enabled_at = None
                 logger.info("AP mode disabled successfully")
                 return True, "AP mode disabled"
             except Exception as e:
@@ -2260,22 +2395,21 @@ address=/detectportal.firefox.com/192.168.4.1
                         f"Ethernet={ethernet_connected}, AP_active={ap_active}, "
                         f"auto_enable={auto_enable}, disconnected_checks={self._disconnected_checks}")
             
-            # Determine if we should have AP mode active
-            # AP mode should only be auto-enabled if:
-            # - auto_enable_ap_mode is True AND
-            # - WiFi is NOT connected AND
-            # - Ethernet is NOT connected AND
-            # - We've had multiple consecutive disconnected checks (grace period)
+            # Determine if we should have AP mode active.
+            # AP-enable uses only the nmcli association state (fast, no network calls).
+            # This keeps the same reliable behaviour as before: momentary packet loss
+            # while on working WiFi does NOT trigger AP mode.  The internet-reachability
+            # check is performed separately in the daemon watchdog for NM recovery.
             is_disconnected = not status.connected and not ethernet_connected
-            
+
             if is_disconnected:
                 # Increment disconnected check counter
                 self._disconnected_checks += 1
                 logger.debug(f"Network disconnected (check {self._disconnected_checks}/{self._disconnected_checks_required})")
             else:
-                # Reset counter if we're connected
+                # Reset counter if we're associated
                 if self._disconnected_checks > 0:
-                    logger.debug(f"Network connected, resetting disconnected check counter")
+                    logger.debug("Network connected, resetting disconnected check counter")
                 self._disconnected_checks = 0
             
             # Only enable AP if we've had enough consecutive disconnected checks
@@ -2320,6 +2454,21 @@ address=/detectportal.firefox.com/192.168.4.1
                     # AP is active but auto_enable is disabled - this means it was manually enabled
                     # Don't disable it automatically, let it stay active
                     logger.debug("AP mode is active (manually enabled), keeping active")
+
+            # Idle-timeout check: disable AP if no client has connected within the window.
+            # Only applies when AP is active and we haven't just decided to enable/disable it.
+            if ap_active and self._ap_enabled_at is not None:
+                idle_timeout_min = self.config.get("ap_idle_timeout_minutes", 15)
+                elapsed = time.time() - self._ap_enabled_at
+                if elapsed > idle_timeout_min * 60 and not self._has_ap_clients():
+                    logger.info(
+                        f"AP idle timeout ({idle_timeout_min} min, no clients) — disabling AP"
+                    )
+                    success, message = self.disable_ap_mode()
+                    if success:
+                        return True
+                    else:
+                        logger.warning(f"Failed to disable AP on idle timeout: {message}")
             
             return False
         except Exception as e:

test/test_wifi_manager_ap.py

@@ -0,0 +1,324 @@
+"""
+Unit tests for WiFi AP mode — src.wifi_manager.
+
+Each test exercises logic that can be verified through the subprocess calls the
+manager emits, without requiring root access, hardware, or a running Pi.
+
+Scenarios covered:
+1. nmcli AP profile is created with no security parameters (open/passwordless).
+2. iptables PREROUTING and INPUT rules are added when the nmcli AP starts.
+3. iptables rules and ip_forward are reverted when the AP is torn down.
+4. LED matrix message includes the SSID, 'No password', and the setup URL.
+5. Known AP profile names are deleted before the new profile is created.
+"""
+from __future__ import annotations
+
+import json
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from src.wifi_manager import WiFiManager
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+def _ok(stdout: str = "", stderr: str = "") -> MagicMock:
+    r = MagicMock()
+    r.returncode = 0
+    r.stdout = stdout
+    r.stderr = stderr
+    return r
+
+
+def _fail(stdout: str = "", stderr: str = "error") -> MagicMock:
+    r = MagicMock()
+    r.returncode = 1
+    r.stdout = stdout
+    r.stderr = stderr
+    return r
+
+
+def _find_path_side_effect(name: str) -> str:
+    """Deterministic fake for _find_command_path."""
+    return {"iptables": "/usr/sbin/iptables", "sysctl": "/usr/sbin/sysctl"}.get(
+        name, f"/usr/bin/{name}"
+    )
+
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+@pytest.fixture()
+def wifi_config(tmp_path: Path) -> Path:
+    """Minimal wifi_config.json in a temporary directory."""
+    cfg_dir = tmp_path / "config"
+    cfg_dir.mkdir()
+    cfg = {
+        "ap_ssid": "LEDMatrix-Setup",
+        "ap_channel": 7,
+        "auto_enable_ap_mode": True,
+        "saved_networks": [],
+    }
+    p = cfg_dir / "wifi_config.json"
+    p.write_text(json.dumps(cfg))
+    return p
+
+
+@pytest.fixture()
+def manager(wifi_config: Path, tmp_path: Path) -> WiFiManager:
+    """
+    WiFiManager with all system calls stubbed out during construction and the
+    ip_forward save file redirected to a per-test temporary path.
+    """
+    with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")), \
+         patch.object(WiFiManager, "_detect_trixie", return_value=False):
+        mgr = WiFiManager(config_path=wifi_config)
+
+    # Force clean, deterministic state regardless of what __init__ inferred
+    mgr._wifi_interface = "wlan0"
+    mgr.has_nmcli = True
+    mgr.has_hostapd = False
+    mgr.has_dnsmasq = False
+    mgr.has_iwlist = False
+    mgr._is_trixie = False
+    # Redirect the ip_forward save file to tmp so tests never share state
+    mgr._IP_FORWARD_SAVE_PATH = tmp_path / "ip_fwd_saved"
+    return mgr
+
+
+# ---------------------------------------------------------------------------
+# 1. AP profile is open (no password)
+# ---------------------------------------------------------------------------
+
+@pytest.mark.unit
+def test_nmcli_ap_profile_has_no_security_params(manager: WiFiManager) -> None:
+    """
+    The 'nmcli connection add' command must not include key-mgmt, psk, or any
+    WPA-related parameter.  On Bookworm/Trixie, NM creates a WPA2-protected
+    hotspot even when those values are set to 'none'/empty via a later
+    'connection modify', so the profile must be created without a security
+    section from the start.
+    """
+    captured: list[list[str]] = []
+
+    def _run(cmd, **kw):
+        captured.append(list(cmd))
+        return _ok()
+
+    with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
+         patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
+         patch.object(manager, "_setup_iptables_redirect", return_value=True), \
+         patch.object(manager, "_get_ap_status_nmcli",
+                      return_value={"active": True, "ip": "192.168.4.1"}), \
+         patch.object(manager, "_show_led_message"):
+
+        success, _ = manager._enable_ap_mode_nmcli_hotspot()
+
+    assert success, "AP enable should report success"
+
+    add_calls = [c for c in captured if "nmcli" in c and "connection" in c and "add" in c]
+    assert add_calls, "Expected at least one 'nmcli connection add' invocation"
+
+    add_str = " ".join(add_calls[0])
+    assert "key-mgmt" not in add_str, "AP profile must not set key-mgmt"
+    assert "psk" not in add_str, "AP profile must not include a PSK/password"
+    assert "wpa" not in add_str.lower(), "AP profile must not reference WPA"
+    assert "802-11-wireless.mode" in add_str, "AP profile must declare wireless mode"
+    assert "ap" in add_calls[0], "Wireless mode value must be 'ap'"
+
+
+# ---------------------------------------------------------------------------
+# 2. iptables NAT rules are added when the AP starts
+# ---------------------------------------------------------------------------
+
+@pytest.mark.unit
+def test_iptables_nat_rules_added_on_ap_start(manager: WiFiManager) -> None:
+    """
+    _setup_iptables_redirect must add:
+    - a PREROUTING REDIRECT rule that maps incoming TCP port 80 to port 5000, and
+    - an INPUT ACCEPT rule for port 5000 (the post-redirect destination port,
+      NOT port 80 which never hits the INPUT chain after PREROUTING rewrites it).
+    """
+    captured: list[list[str]] = []
+
+    def _run(cmd, **kw):
+        captured.append(list(cmd))
+        # iptables -C (check) → rc=1 so the -A (add) branch executes
+        if "iptables" in " ".join(str(x) for x in cmd) and "-C" in cmd:
+            return _fail()
+        return _ok()
+
+    # Patch Path.read_text so /proc/sys/net/ipv4/ip_forward is readable on any OS
+    with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
+         patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect), \
+         patch("pathlib.Path.read_text", return_value="0\n"):
+
+        result = manager._setup_iptables_redirect()
+
+    assert result, "_setup_iptables_redirect must return True on success"
+
+    prerouting_adds = [c for c in captured if "iptables" in " ".join(c) and "-A" in c and "PREROUTING" in c]
+    assert prerouting_adds, "Expected 'iptables -A PREROUTING' invocation"
+    pr_str = " ".join(prerouting_adds[0])
+    assert "--dport" in pr_str and "80" in pr_str, "PREROUTING rule must match dport 80"
+    assert "5000" in pr_str, "PREROUTING rule must redirect to port 5000"
+    assert "REDIRECT" in pr_str, "PREROUTING rule must use REDIRECT target"
+
+    input_adds = [c for c in captured if "iptables" in " ".join(c) and "-A" in c and "INPUT" in c]
+    assert input_adds, "Expected 'iptables -A INPUT' invocation"
+    in_str = " ".join(input_adds[0])
+    assert "5000" in in_str, "INPUT rule must accept port 5000 (post-PREROUTING destination)"
+    assert "ACCEPT" in in_str, "INPUT rule must use ACCEPT target"
+
+    # Port 80 must NOT be used in the INPUT rule (it is already redirected by PREROUTING)
+    input_80 = [c for c in captured if "iptables" in " ".join(c) and "INPUT" in c and "--dport" in c and "80" in c]
+    assert not input_80, "INPUT rule must target port 5000, not port 80"
+
+
+# ---------------------------------------------------------------------------
+# 3a. iptables rules and ip_forward reverted on teardown
+# ---------------------------------------------------------------------------
+
+@pytest.mark.unit
+def test_iptables_rules_and_ip_forward_reverted_on_teardown(manager: WiFiManager) -> None:
+    """
+    _teardown_iptables_redirect must:
+    - remove the PREROUTING and INPUT iptables rules, and
+    - restore ip_forward to the exact value recorded in the save file.
+    """
+    original_fwd = "0"
+    manager._IP_FORWARD_SAVE_PATH.write_text(original_fwd)
+
+    captured: list[list[str]] = []
+
+    with patch("src.wifi_manager.subprocess.run",
+               side_effect=lambda cmd, **kw: (captured.append(list(cmd)) or _ok())), \
+         patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect):
+
+        manager._teardown_iptables_redirect()
+
+    assert [c for c in captured if "iptables" in " ".join(c) and "-D" in c and "PREROUTING" in c], \
+        "Expected 'iptables -D PREROUTING' invocation"
+
+    assert [c for c in captured if "iptables" in " ".join(c) and "-D" in c and "INPUT" in c], \
+        "Expected 'iptables -D INPUT' invocation"
+
+    restore_calls = [
+        c for c in captured
+        if "sysctl" in " ".join(c) and f"ip_forward={original_fwd}" in " ".join(c)
+    ]
+    assert restore_calls, f"Expected sysctl to restore ip_forward to {original_fwd!r}"
+
+    assert not manager._IP_FORWARD_SAVE_PATH.exists(), \
+        "Save file must be removed after successful teardown"
+
+
+# ---------------------------------------------------------------------------
+# 3b. ip_forward untouched when no save file exists
+# ---------------------------------------------------------------------------
+
+@pytest.mark.unit
+def test_ip_forward_not_restored_when_save_file_absent(manager: WiFiManager) -> None:
+    """
+    When the save file is missing (setup never wrote it, e.g. because /proc was
+    unreadable or the write failed), teardown must NOT call sysctl so it does not
+    accidentally clobber ip_forward state owned by a VPN or NetworkManager.
+    """
+    assert not manager._IP_FORWARD_SAVE_PATH.exists()
+
+    captured: list[list[str]] = []
+
+    with patch("src.wifi_manager.subprocess.run",
+               side_effect=lambda cmd, **kw: (captured.append(list(cmd)) or _ok())), \
+         patch.object(manager, "_find_command_path", side_effect=_find_path_side_effect):
+
+        manager._teardown_iptables_redirect()
+
+    sysctl_calls = [
+        c for c in captured
+        if "sysctl" in " ".join(c) and "ip_forward" in " ".join(c)
+    ]
+    assert not sysctl_calls, \
+        "sysctl must not be called when no ip_forward save file exists"
+
+
+# ---------------------------------------------------------------------------
+# 4. LED message content
+# ---------------------------------------------------------------------------
+
+@pytest.mark.unit
+def test_led_message_shows_ssid_no_password_and_url(manager: WiFiManager) -> None:
+    """
+    When the nmcli AP activates, the LED message must include:
+    - the AP SSID ('LEDMatrix-Setup')
+    - the string 'No password'
+    - the AP IP address (192.168.4.1) and Flask port (5000)
+    """
+    led_messages: list[str] = []
+
+    with patch("src.wifi_manager.subprocess.run", return_value=_ok()), \
+         patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
+         patch.object(manager, "_setup_iptables_redirect", return_value=True), \
+         patch.object(manager, "_get_ap_status_nmcli",
+                      return_value={"active": True, "ip": "192.168.4.1"}), \
+         patch.object(manager, "_show_led_message",
+                      side_effect=lambda msg, **kw: led_messages.append(msg)):
+
+        success, _ = manager._enable_ap_mode_nmcli_hotspot()
+
+    assert success, "AP enable should report success"
+    assert led_messages, "Expected at least one _show_led_message call"
+
+    combined = "\n".join(led_messages)
+    assert "No password" in combined, "LED message must say 'No password'"
+    assert "LEDMatrix-Setup" in combined, "LED message must include the AP SSID"
+    assert "192.168.4.1" in combined, "LED message must include the AP IP address"
+    assert "5000" in combined, "LED message must include the Flask port"
+
+
+# ---------------------------------------------------------------------------
+# 5. Stale AP profiles deleted before the new one is created
+# ---------------------------------------------------------------------------
+
+@pytest.mark.unit
+def test_existing_ap_profiles_deleted_before_new_profile_created(manager: WiFiManager) -> None:
+    """
+    Before 'nmcli connection add', the manager must issue
+    'nmcli connection down/delete' for every known AP profile name so stale
+    profiles (from a previous crash or partial setup) cannot block the new one.
+    """
+    captured: list[list[str]] = []
+
+    def _run(cmd, **kw):
+        captured.append(list(cmd))
+        return _ok()
+
+    with patch("src.wifi_manager.subprocess.run", side_effect=_run), \
+         patch.object(manager, "disconnect_from_network", return_value=(True, "ok")), \
+         patch.object(manager, "_setup_iptables_redirect", return_value=True), \
+         patch.object(manager, "_get_ap_status_nmcli",
+                      return_value={"active": True, "ip": "192.168.4.1"}), \
+         patch.object(manager, "_show_led_message"):
+
+        success, _ = manager._enable_ap_mode_nmcli_hotspot()
+
+    assert success
+
+    cmd_strs = [" ".join(c) for c in captured]
+
+    for profile in ("LEDMatrix-Setup-AP", "Hotspot", "TickerSetup-AP"):
+        assert any("connection delete" in s and profile in s for s in cmd_strs), \
+            f"Expected 'nmcli connection delete {profile}' before creating the new profile"
+
+    add_indices = [i for i, s in enumerate(cmd_strs) if "connection add" in s]
+    del_indices = [i for i, s in enumerate(cmd_strs) if "connection delete" in s]
+
+    assert add_indices, "Expected 'nmcli connection add' call"
+    assert del_indices, "Expected 'nmcli connection delete' calls"
+    assert max(del_indices) < min(add_indices), \
+        "All connection deletions must complete before the new profile is created"

web_interface/blueprints/api_v3.py

@@ -7133,9 +7133,14 @@ def connect_wifi():
                 'message': message
             })
         else:
+            # Propagate structured error type so the captive portal UI can show
+            # "Wrong password — try again" instead of a generic failure message.
+            error_type = "wrong_password" if (message or "").startswith("wrong_password:") else "connection_failed"
+            clean_message = (message or "").removeprefix("wrong_password: ") or "Failed to connect to network"
             return jsonify({
                 'status': 'error',
-                'message': message or 'Failed to connect to network'
+                'message': clean_message,
+                'error_type': error_type
             }), 400
     except Exception as e:
         logger.exception("[WiFi] Failed connecting to WiFi network")

web_interface/templates/v3/captive_setup.html

@@ -191,7 +191,10 @@ function doConnect() {
       // Poll for the new IP
       setTimeout(function() { checkNewIP(ssid); }, 3000);
     } else {
-      showMsg(data.message || 'Connection failed', 'err');
+      var msg = data.error_type === 'wrong_password'
+        ? 'Incorrect password — please try again'
+        : (data.message || 'Connection failed');
+      showMsg(msg, 'err');
       connecting = false;
       btn.disabled = false;
       btn.innerHTML = 'Connect';