fix(network): bound link transitions and restore each medium's DNS

Two defects found by an adversarial review of the merge.

A link transition no longer stops a slot whose connect attempt is in flight.
Stopping one means esp_mqtt_client_stop(), which waits on the SDK's API mutex
and its task's stopped event with no bound; the client task notices only when
it returns from whatever transport call it is in. During teardown the bridge's
StopUnproven timeout contains that, but during a transition nothing does, so a
routine WiFi flap could freeze the sole MQTT worker for far longer than the 2 s
this path advertises, with the bridge's own stop handshake queued behind it.
The attempt is left to resolve instead: unlike a reconfigure, a transition
changes neither endpoint nor credentials, so an attempt that completes is
credited to the broker it actually reached and drops with the old route.

Failing back to Ethernet left the node using the WiFi network's DNS server.
lwIP keeps one global server list and IDF 4.4 has no per-interface retention,
so the medium that leased last owns DNS for every socket. Each link now
remembers the resolver its own DHCP lease installed and puts it back when it is
selected again; a medium that never held a lease leaves the current resolver
alone. Where the two networks are on different subnets this was a silent
outage: the link read as connected while every broker and NTP hostname failed
to resolve, until Ethernet's own DHCP renewal happened to fix it.

MQTT_IMPLEMENTATION.md

@@ -880,6 +880,9 @@ the radio actually performs in that case.
   report a connection that no longer exists. When the same link returns, each slot gets one
   immediate attempt at its current backoff rung (a tripped circuit breaker gets one immediate
   probe); a switch to the other medium also clears backoff and breakers
+- Each medium's DHCP DNS servers are remembered when it gets its lease and restored when it is
+  selected again. lwIP keeps one global resolver list, so without this a node that fell back to
+  WiFi and then failed back to Ethernet would keep asking the WiFi network's DNS server
 - WiFi credentials changed at runtime (`set wifi.ssid` / `set wifi.pwd`) are used on the next
   reconnect attempt without a reboot
 - Packets are queued while a slot is disconnected and flushed when it recovers

src/helpers/NetworkLink.cpp

@@ -11,6 +11,7 @@
 
 #include <WiFi.h>
 #include <esp_wifi.h>
+#include <lwip/dns.h>
 
 #if defined(NETWORK_PREFER_ETHERNET)
 #include "ethernet/ch390/CH390Config.h"
@@ -40,6 +41,8 @@ class NetworkLinkBase : public NetworkLink {
   bool _status_initialized = false;
   bool _last_connected = false;
   unsigned long _last_status_check = 0;
+  static constexpr int kDnsServers = 2;
+  std::atomic<uint32_t> _dns_snapshot[kDnsServers] = {{0}, {0}};
 
   AlertFaultPolicy::OutageSnapshot outage() const {
     return AlertFaultPolicy::unpackOutageSnapshot(
@@ -65,6 +68,7 @@ class NetworkLinkBase : public NetworkLink {
         (uint32_t)now_ms, reason, outage()));
   }
 
+
  public:
   unsigned long connectedAtMillis() const override {
     return _connected_at.load(std::memory_order_relaxed);
@@ -81,6 +85,39 @@ class NetworkLinkBase : public NetworkLink {
   AlertFaultPolicy::OutageSnapshot outageSnapshot() const override {
     return outage();
   }
+
+  // Remember the resolver this link's DHCP lease installed, so switching back
+  // to it can put that resolver back.
+  //
+  // lwIP keeps ONE global server list, not one per interface: a DHCP lease on
+  // any medium calls dns_setserver() and overwrites whatever the other medium
+  // had. IDF 4.4 has no per-interface retention either — esp_netif_get_dns_info()
+  // reads the same globals — so the medium that leased last owns DNS for every
+  // socket. Without this, an Ethernet node that falls back to Wi-Fi and then
+  // fails back keeps asking the Wi-Fi network's DNS server. When the two are on
+  // different subnets that server is unreachable from Ethernet, and the node
+  // reads as connected while every broker and NTP hostname fails to resolve,
+  // until Ethernet's own DHCP renewal happens to fix it hours later.
+  void snapshotDns() {
+    for (int i = 0; i < kDnsServers; i++) {
+      const ip_addr_t* server = dns_getserver(i);
+      _dns_snapshot[i].store(
+          (server != nullptr && !ip_addr_isany(server))
+              ? ip4_addr_get_u32(ip_2_ip4(server)) : 0,
+          std::memory_order_relaxed);
+    }
+  }
+
+  // Only servers this link actually leased are restored; a medium that has
+  // never held a lease leaves the current resolver alone rather than blanking it.
+  void restoreDns() const {
+    for (int i = 0; i < kDnsServers; i++) {
+      const uint32_t addr = _dns_snapshot[i].load(std::memory_order_relaxed);
+      if (addr == 0) continue;
+      const ip_addr_t server = IPADDR4_INIT(addr);
+      dns_setserver(i, &server);
+    }
+  }
 };
 
 class WiFiNetworkLink final : public NetworkLinkBase {
@@ -199,7 +236,10 @@ class WiFiNetworkLink final : public NetworkLinkBase {
       // Already associated when the link started (end()/begin() leaves STA up):
       // there is no connect transition below to carry the setting, so apply it
       // here or the node keeps running whatever mode was set before.
-      if (connected) applyPowerPrefs(wifi_power_save);
+      if (connected) {
+        applyPowerPrefs(wifi_power_save);
+        snapshotDns();
+      }
     }
 
     if ((uint32_t)(now_ms - _last_status_check) <= 10000) {
@@ -216,6 +256,7 @@ class WiFiNetworkLink final : public NetworkLinkBase {
         _connected_at.store(now_ms, std::memory_order_relaxed);
         _reconnect_backoff_attempt = 0;
         applyPowerPrefs(wifi_power_save);
+        snapshotDns();
       }
       _last_connected = true;
       return transitioned ? NetworkTransition::Up : NetworkTransition::None;
@@ -308,6 +349,10 @@ class EthernetNetworkLink final : public NetworkLinkBase {
             _event_state.store(static_cast<uint8_t>(EventState::GotIp),
                                std::memory_order_relaxed);
             noteConnected(millis());
+            // Taken here rather than at the maintain() edge: this is the moment
+            // the lease installed the resolver, before a Wi-Fi fallback lease
+            // can overwrite it.
+            snapshotDns();
             break;
           case ARDUINO_EVENT_ETH_DISCONNECTED:
             _event_state.store(static_cast<uint8_t>(EventState::LinkDown),
@@ -350,7 +395,10 @@ class EthernetNetworkLink final : public NetworkLinkBase {
       _status_initialized = true;
       setOutage(AlertFaultPolicy::applyWifiStatus(
           now_ms, connected, outage(), false));
-      if (connected) noteConnected(now_ms);
+      if (connected) {
+        noteConnected(now_ms);
+        snapshotDns();
+      }
       return NetworkTransition::None;
     }
 
@@ -364,6 +412,7 @@ class EthernetNetworkLink final : public NetworkLinkBase {
       _connected_at.store(now_ms, std::memory_order_relaxed);
       setOutage(AlertFaultPolicy::applyWifiStatus(
           now_ms, true, outage(), true));
+      snapshotDns();
       return NetworkTransition::Up;
     }
 
@@ -525,6 +574,13 @@ class AutomaticNetworkLink final : public NetworkLink {
     }
     _selected_down_since.store(0, std::memory_order_relaxed);
     _selected.store(medium, std::memory_order_release);
+    // Put back the resolver this medium leased. lwIP's server list is global,
+    // so whichever medium leased last still owns it here (see snapshotDns()).
+    if (medium == NetworkMedium::Ethernet) {
+      _ethernet.restoreDns();
+    } else if (medium == NetworkMedium::WiFi) {
+      _wifi.restoreDns();
+    }
   }
 
   bool startOrRetryEthernet(uint32_t now_ms, bool restart) {

src/helpers/bridges/MQTTBridge.cpp

@@ -2335,31 +2335,38 @@ static constexpr unsigned long kNetworkTransitionDisconnectMs = 2000;
 
 // Close a slot's transport because the route under it changed.
 //
-// Same shape as closeLiveClientForReconfigure(), and for the same F04 reason: a
-// client that is still Starting has an attempt aimed at the OLD route, and
-// softDisconnect() returns immediately on one that is not yet connected, so the
-// attempt would run to completion and deliver a CONNECTED event that is
-// indistinguishable from the new route's. On a medium switch the old route can
-// still be briefly usable, so that event is not hypothetical. Cancelling it
-// needs a real stop; reconnectSlotClient() starts a stopped client again.
+// Deliberately NOT the reconfigure path, even though both close a live client.
+// A reconfigure gives the slot a new endpoint and new credentials, so an
+// in-flight attempt that completes afterwards reports a connection to the OLD
+// broker under the new configuration, and cancelling it is worth a real stop
+// (F04). A link transition changes neither: the attempt is aimed at the same
+// broker with the same credentials, so if it completes it is credited to the
+// endpoint it actually reached. It is stale only in that its socket sits on a
+// route that is going away, and the ordinary DISCONNECTED path handles that.
 //
-// A connected client takes the cheap path instead — bounded, and the esp-mqtt
-// task stays, which is what keeps a Wi-Fi flap from recreating every slot's
-// task and refragmenting internal heap. The wait is short because the old route
-// is already gone; a client whose event is late is aborted by keepalive and
-// picked up by the normal backoff retry.
+// So an attempt in flight is left alone here. Stopping it would mean calling
+// esp_mqtt_client_stop(), which waits on the SDK's API mutex and its task's
+// stopped event with no bound (see stopSlotClient()) — the client task notices
+// only when it returns from whatever transport call it is in, up to
+// network_timeout_ms for a TLS connect. During teardown that is contained by
+// the bridge's StopUnproven timeout; during a transition nothing contains it,
+// so paying it serially for up to five slots would freeze the sole MQTT worker
+// far past the bound this path advertises, and would block the bridge's own
+// stop handshake behind it. A routine Wi-Fi flap must not cost that.
 //
-// Nothing here touches a client that is not live, so a Quarantined one (its SDK
-// task was never joined) is left alone, as it must be for the rest of the boot.
+// A connected client takes the cheap bounded path, and the esp-mqtt task stays,
+// which is what keeps a flap from recreating every slot's task and
+// refragmenting internal heap. Nothing here touches a client that is not live,
+// so a Quarantined one (its SDK task was never joined) is left alone, as it
+// must be for the rest of the boot.
 void MQTTBridge::closeLiveClientForLinkTransition(int index) {
   if (index < 0 || index >= RUNTIME_MQTT_SLOTS) return;
   MQTTSlot& slot = _slots[index];
   if (slot.client == nullptr || !clientStateIsLive(slot.client_state)) return;
 
   if (mqttClientStateHasAttemptInFlight(slot.client_state)) {
-    MQTT_DEBUG_PRINTLN("MQTT%d link transition during connect - stopping to cancel the attempt",
+    MQTT_DEBUG_PRINTLN("MQTT%d link transition during connect - leaving the attempt to resolve",
                        index + 1);
-    stopSlotClient(index);
     return;
   }
 

test/test_mqtt_client_state/test_mqtt_client_state.cpp

@@ -76,26 +76,31 @@ TEST(MqttClientState, StopIsAcknowledgedOnlyWhenEveryClientIsProvenStopped) {
 }
 
 // A link transition closes each slot's transport because the route under it
-// changed. The two predicates above are what decide how, and getting the
-// Starting case wrong is not cosmetic: softDisconnect() returns immediately on
-// a client that is not yet connected, so that attempt would run to completion
-// against the OLD route and deliver a CONNECTED event the new attempt cannot be
-// told apart from.
-TEST(MqttClientState, LinkTransitionStopsOnlyClientsWithAnAttemptInFlight) {
+// changed. These two predicates are what decide which clients it may touch.
+//
+// The distinction from a reconfigure matters: a reconfigure must cancel an
+// in-flight attempt, because that attempt would report the OLD endpoint under
+// the new configuration. A transition changes no configuration, so the attempt
+// is left to resolve — cancelling it would mean an unbounded
+// esp_mqtt_client_stop() on the sole MQTT worker during a routine link flap.
+TEST(MqttClientState, LinkTransitionOnlyDisconnectsClientsWithNoAttemptInFlight) {
   for (MqttClientState s : kAllStates) {
-    const bool touched = mqttClientStateIsLive(s);
-    const bool needs_stop = touched && mqttClientStateHasAttemptInFlight(s);
+    const bool live = mqttClientStateIsLive(s);
+    const bool disconnected_here = live && !mqttClientStateHasAttemptInFlight(s);
 
     // Quarantined is never touched: its SDK task was not joined, so it stays
     // out of service for the rest of the boot.
-    if (s == MqttClientState::Quarantined) EXPECT_FALSE(touched) << mqttClientStateName(s);
+    if (s == MqttClientState::Quarantined) EXPECT_FALSE(live) << mqttClientStateName(s);
     // Nothing that is already proven stopped is worth a disconnect.
-    if (mqttClientStateIsProvenStopped(s)) EXPECT_FALSE(touched) << mqttClientStateName(s);
+    if (mqttClientStateIsProvenStopped(s)) EXPECT_FALSE(live) << mqttClientStateName(s);
 
-    EXPECT_EQ(s == MqttClientState::Starting, needs_stop) << mqttClientStateName(s);
-    // The cheap bounded path, which keeps the esp-mqtt task alive.
     EXPECT_EQ(s == MqttClientState::Connected || s == MqttClientState::Disconnected,
-              touched && !needs_stop) << mqttClientStateName(s);
+              disconnected_here) << mqttClientStateName(s);
+    // Starting is live, and still must not be disconnected here.
+    if (s == MqttClientState::Starting) {
+      EXPECT_TRUE(live);
+      EXPECT_FALSE(disconnected_here);
+    }
   }
 }