Address review round: backoff fairness, watchdog integrity, silent failures

esphome/components/api/api_connection.cpp

@@ -1784,6 +1784,9 @@ void APIConnection::complete_authentication_() {
     zwave_proxy::global_zwave_proxy->api_connection_authenticated(this);
   }
 #endif
+#ifdef USE_API_OUTGOING_CONNECTION
+  this->parent_->on_client_authenticated();
+#endif
 }
 
 bool APIConnection::send_hello_response_(const HelloRequest &msg) {

esphome/components/api/api_outgoing_connection.cpp

@@ -35,7 +35,8 @@ void OutgoingConnectionManager::loop(APIServer *server) {
     return;  // on_target_client() already reset the dial state
   }
   if (this->dialed_conn_ != nullptr) {
-    // Dialed peer still connected but unproven; its own timeouts free the slot
+    // A live dialed session (flagged or not, e.g. a host: peer) is the
+    // target; a silent one dies on the handshake timeout
     return;
   }
   const uint32_t now = App.get_loop_component_start_time();
@@ -58,12 +59,33 @@ void OutgoingConnectionManager::loop(APIServer *server) {
 }
 
 void OutgoingConnectionManager::try_dial_(APIServer *server, uint32_t now) {
+  if (!network::is_connected()) {
+    // Flips within seconds of boot; recheck fast so a deep sleep wake
+    // window is not spent waiting
+    this->schedule_wait_(now, NETWORK_RETRY_MS);
+    return;
+  }
   const char *host = this->target_host_();
-  if (host == nullptr || !network::is_connected() || server->at_client_limit_() || !server->noise_ctx_.has_psk()) {
-    // Not a dial failure; retry soon without escalating the backoff
+  if (host == nullptr || server->at_client_limit_() || !server->noise_ctx_.has_psk()) {
+    ESP_LOGD(TAG, "Not dialing: %s",
+             host == nullptr              ? "no target"
+             : server->at_client_limit_() ? "max connections"
+                                          : "no key");
+    // Not a dial failure; retry without escalating the backoff
     this->schedule_wait_(now, PRECONDITION_RETRY_MS);
     return;
   }
+#if !defined(USE_NETWORK_IPV6) && !defined(API_OUTGOING_CONNECTION_HOST)
+  if (strchr(host, ':') != nullptr) {
+    // Remembered by an earlier IPv6 build; set_sockaddr() would silently
+    // turn it into 255.255.255.255 here
+    ESP_LOGW(TAG, "Clearing unusable IPv6 target %s", host);
+    this->saved_.host[0] = '\0';
+    this->target_pref_.save(&this->saved_);
+    this->schedule_wait_(now, PRECONDITION_RETRY_MS);
+    return;
+  }
+#endif
   struct sockaddr_storage addr;
   socklen_t addr_len =
       socket::set_sockaddr((struct sockaddr *) &addr, sizeof(addr), host, API_OUTGOING_CONNECTION_PORT);
@@ -86,6 +108,7 @@ void OutgoingConnectionManager::try_dial_(APIServer *server, uint32_t now) {
     if (this->dialed_conn_ == nullptr) {
       this->schedule_retry_(now);
     } else {
+      this->dial_handoff_ts_ = now;
       this->schedule_wait_(now, PRECONDITION_RETRY_MS);
     }
     return;
@@ -138,7 +161,12 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) {
   }
   int error = 0;
   socklen_t len = sizeof(error);
-  if (this->dial_socket_->getsockopt(SOL_SOCKET, SO_ERROR, &error, &len) != 0 || error != 0) {
+  if (this->dial_socket_->getsockopt(SOL_SOCKET, SO_ERROR, &error, &len) != 0) {
+    ESP_LOGW(TAG, "Connect status check failed: errno %d", errno);
+    this->schedule_retry_(now);
+    return;
+  }
+  if (error != 0) {
     ESP_LOGW(TAG, "Connect failed: %d", error);
     this->schedule_retry_(now);
     return;
@@ -148,6 +176,7 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) {
     this->schedule_retry_(now);
     return;
   }
+  this->dial_handoff_ts_ = now;
   // Hold unescalated until the peer proves itself or dies unproven
   this->schedule_wait_(now, PRECONDITION_RETRY_MS);
 }
@@ -163,23 +192,40 @@ void OutgoingConnectionManager::schedule_retry_(uint32_t now) {
 }
 
 void OutgoingConnectionManager::on_client_removed(APIConnection *conn) {
-  if (conn == this->dialed_conn_) {
-    this->dialed_conn_ = nullptr;
-    this->schedule_retry_(App.get_loop_component_start_time());
+  if (conn != this->dialed_conn_) {
+    return;
+  }
+  this->dialed_conn_ = nullptr;
+  const uint32_t now = App.get_loop_component_start_time();
+  if (now - this->dial_handoff_ts_ >= DIAL_PROVEN_MS) {
+    // Outlived the handshake timeout, so it authenticated: a working peer
+    // (e.g. a host: target that never sends the flag) disconnected normally
+    this->backoff_ = BACKOFF_MIN_MS;
+    this->schedule_wait_(now, API_OUTGOING_CONNECTION_DELAY);
+  } else {
+    this->schedule_retry_(now);
   }
 }
 
 void OutgoingConnectionManager::on_target_client(APIConnection *conn) {
   // The target is connected; stop any dial in flight and reset the backoff.
+  // A dialed connection stays tracked unless it is this one: an inbound
+  // target must not orphan a still-open dial.
   this->dial_socket_.reset();
-  this->dialed_conn_ = nullptr;
+  if (conn == this->dialed_conn_) {
+    this->dialed_conn_ = nullptr;
+  }
   this->state_ = DialState::DIAL_STATE_IDLE;
   this->backoff_ = BACKOFF_MIN_MS;
 #ifndef API_OUTGOING_CONNECTION_HOST
   SavedOutgoingTarget target{};
   conn->get_peername_to(target.host);
-  if (target.host[0] == '\0' || strcmp(target.host, this->saved_.host) == 0) {
-    return;  // unknown peer or unchanged; avoid flash wear
+  if (target.host[0] == '\0') {
+    ESP_LOGW(TAG, "Could not read peer address; not remembering target");
+    return;
+  }
+  if (strcmp(target.host, this->saved_.host) == 0) {
+    return;  // unchanged; avoid flash wear
   }
   if (!this->target_pref_.save(&target) || !global_preferences->sync()) {
     // Keep the old value so the save is retried on the next flagged hello

esphome/components/api/api_outgoing_connection.h

@@ -54,6 +54,10 @@ class OutgoingConnectionManager {
   static constexpr uint32_t BACKOFF_MAX_MS = 300000;
   static constexpr uint32_t CONNECT_TIMEOUT_MS = 10000;
   static constexpr uint32_t CONNECT_POLL_INTERVAL_MS = 250;
+  static constexpr uint32_t NETWORK_RETRY_MS = 500;
+  // Longer than the 60s handshake timeout: a dialed session still alive past
+  // this authenticated, so its death is a normal disconnect, not a bad dial
+  static constexpr uint32_t DIAL_PROVEN_MS = 65000;
 
   void try_dial_(APIServer *server, uint32_t now);
   void poll_connect_(APIServer *server, uint32_t now);
@@ -94,6 +98,7 @@ class OutgoingConnectionManager {
 #endif
   uint32_t state_ts_{0};
   uint32_t last_poll_{0};
+  uint32_t dial_handoff_ts_{0};
 
   // Byte-aligned types last
 #ifndef API_OUTGOING_CONNECTION_HOST

esphome/components/api/api_server.cpp

@@ -217,6 +217,8 @@ void APIServer::remove_client_(uint8_t client_index) {
     this->outgoing_target_count_--;
   }
   this->outgoing_conn_.on_client_removed(client.get());
+  // Read before the swap-and-reset below destroys the connection
+  const bool was_authenticated = client->is_authenticated();
 #endif
 
   // Close socket now (was deferred from on_fatal_error to allow getpeername)
@@ -239,7 +241,12 @@ void APIServer::remove_client_(uint8_t client_index) {
   // (suppressed while provisioning is pending - see loop()).
   if (this->api_connection_count_ == 0 && this->reboot_timeout_ != 0 && !this->provisioning_pending_()) {
     this->status_set_warning(LOG_STR("waiting for client connection"));
-    this->last_connected_ = App.get_loop_component_start_time();
+#ifdef USE_API_OUTGOING_CONNECTION
+    // An unauthenticated session (e.g. a dial to a host that accepts TCP but
+    // never speaks the API) must not keep resetting the reboot watchdog
+    if (was_authenticated)
+#endif
+      this->last_connected_ = App.get_loop_component_start_time();
   }
 
 #ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER
@@ -295,10 +302,20 @@ APIConnection *APIServer::add_outgoing_client_(std::unique_ptr<socket::Socket> s
   }
   auto *conn = new APIConnection(std::move(sock), this);
   conn->mark_outgoing();
-  this->add_client_(conn);
+  // Unlike add_client_, no watchdog refresh: a dial that never authenticates
+  // must not keep petting the no-client reboot timeout
+  this->clients_[this->api_connection_count_++].reset(conn);
+  conn->start();
   return conn;
 }
 
+void APIServer::on_client_authenticated() {
+  if (this->reboot_timeout_ != 0 && !this->provisioning_pending_()) {
+    this->status_clear_warning();
+    this->last_connected_ = App.get_loop_component_start_time();
+  }
+}
+
 void APIServer::on_outgoing_target_client(APIConnection *conn) {
   this->outgoing_target_count_++;
   this->outgoing_conn_.on_target_client(conn);

esphome/components/api/api_server.h

@@ -85,6 +85,9 @@ class APIServer final : public Component,
 #ifdef USE_API_OUTGOING_CONNECTION
   // Called by APIConnection when a client declares itself a dial-back target in its hello
   void on_outgoing_target_client(APIConnection *conn);
+  // Called by APIConnection on authentication so dialed sessions feed the
+  // reboot watchdog only once they are real
+  void on_client_authenticated();
 #endif
 
   void handle_disconnect(APIConnection *conn);

tests/component_tests/api/test_outgoing_connection.py

@@ -5,7 +5,7 @@ from pathlib import Path
 
 import pytest
 
-from esphome.components.api import CONFIG_SCHEMA
+from esphome.components.api import CONFIG_SCHEMA, _validate_outgoing_host_ipv6
 from esphome.components.esp32 import KEY_BOARD, KEY_VARIANT, VARIANT_ESP32
 import esphome.config_validation as cv
 from esphome.const import PlatformFramework
@@ -74,3 +74,33 @@ def test_outgoing_connection_rejects_hostnames(
     set_core_config(PlatformFramework.ESP32_IDF, platform_data=ESP32_PLATFORM_DATA)
     with pytest.raises(cv.Invalid, match="not a valid IP address"):
         CONFIG_SCHEMA(_api_config({"host": "homeassistant.local"}))
+
+
+def test_outgoing_connection_ipv6_host_requires_ipv6(
+    set_core_config: SetCoreConfigCallable,
+) -> None:
+    set_core_config(PlatformFramework.ESP32_IDF, platform_data=ESP32_PLATFORM_DATA)
+    config = CONFIG_SCHEMA(_api_config({"host": "fd00::1"}))
+    with pytest.raises(cv.Invalid, match="IPv6 is not"):
+        _validate_outgoing_host_ipv6(config)
+
+
+def test_outgoing_connection_ipv6_host_passes_with_ipv6_enabled(
+    set_core_config: SetCoreConfigCallable,
+) -> None:
+    set_core_config(
+        PlatformFramework.ESP32_IDF,
+        platform_data=ESP32_PLATFORM_DATA,
+        full_config={"network": {"enable_ipv6": True}},
+    )
+    config = CONFIG_SCHEMA(_api_config({"host": "fd00::1"}))
+    assert _validate_outgoing_host_ipv6(config) is config
+
+
+def test_outgoing_connection_ipv6_host_with_ipv6(
+    generate_main: Callable[[str | Path], str],
+) -> None:
+    generate_main("tests/component_tests/api/test_outgoing_connection_ipv6.yaml")
+
+    defines = {define.name: define.value for define in CORE.defines}
+    assert str(defines["API_OUTGOING_CONNECTION_HOST"]) == '"fd00::1"'

tests/component_tests/api/test_outgoing_connection_ipv6.yaml

@@ -0,0 +1,20 @@
+esphome:
+  name: test
+
+esp32:
+  board: esp32dev
+
+wifi:
+  ssid: SomeNetwork
+  password: SomePassword
+
+network:
+  enable_ipv6: true
+
+logger:
+
+api:
+  encryption:
+    key: bOFFzzvfpg5DB94DuBGLXD/hMnhpDKgP9UQyBulwWVU=
+  outgoing_connection:
+    host: fd00::1