Merge branch 'integration' into memory_api

.github/workflows/codeowner-review-request.yml

@@ -178,6 +178,51 @@ jobs:
                 reviewedUsers.add(review.user.login);
               });
 
+              // Check for previous comments from this workflow to avoid duplicate pings
+              const { data: comments } = await github.rest.issues.listComments({
+                owner,
+                repo,
+                issue_number: pr_number
+              });
+
+              const previouslyPingedUsers = new Set();
+              const previouslyPingedTeams = new Set();
+
+              // Look for comments from github-actions bot that contain codeowner pings
+              const workflowComments = comments.filter(comment =>
+                comment.user.type === 'Bot' &&
+                comment.user.login === 'github-actions[bot]' &&
+                comment.body.includes("I've automatically requested reviews from codeowners")
+              );
+
+              // Extract previously mentioned users and teams from workflow comments
+              for (const comment of workflowComments) {
+                // Match @username patterns (not team mentions)
+                const userMentions = comment.body.match(/@([a-zA-Z0-9_.-]+)(?![/])/g) || [];
+                userMentions.forEach(mention => {
+                  const username = mention.slice(1); // remove @
+                  previouslyPingedUsers.add(username);
+                });
+
+                // Match @org/team patterns
+                const teamMentions = comment.body.match(/@[a-zA-Z0-9_.-]+\/([a-zA-Z0-9_.-]+)/g) || [];
+                teamMentions.forEach(mention => {
+                  const teamName = mention.split('/')[1];
+                  previouslyPingedTeams.add(teamName);
+                });
+              }
+
+              console.log(`Found ${previouslyPingedUsers.size} previously pinged users and ${previouslyPingedTeams.size} previously pinged teams`);
+
+              // Remove users who have already been pinged in previous workflow comments
+              previouslyPingedUsers.forEach(user => {
+                matchedOwners.delete(user);
+              });
+
+              previouslyPingedTeams.forEach(team => {
+                matchedTeams.delete(team);
+              });
+
               // Remove only users who have already submitted reviews (not just requested reviewers)
               reviewedUsers.forEach(reviewer => {
                 matchedOwners.delete(reviewer);
@@ -192,7 +237,7 @@ jobs:
               const teamsList = Array.from(matchedTeams);
 
               if (reviewersList.length === 0 && teamsList.length === 0) {
-                console.log('No eligible reviewers found (all may already be requested or reviewed)');
+                console.log('No eligible reviewers found (all may already be requested, reviewed, or previously pinged)');
                 return;
               }
 
@@ -227,31 +272,41 @@ jobs:
                   console.log('All codeowners are already requested reviewers or have reviewed');
                 }
 
-                // Add a comment to the PR mentioning what happened (include all matched codeowners)
-                const commentBody = createCommentBody(reviewersList, teamsList, fileMatches.size, true);
+                // Only add a comment if there are new codeowners to mention (not previously pinged)
+                if (reviewersList.length > 0 || teamsList.length > 0) {
+                  const commentBody = createCommentBody(reviewersList, teamsList, fileMatches.size, true);
 
-                await github.rest.issues.createComment({
-                  owner,
-                  repo,
-                  issue_number: pr_number,
-                  body: commentBody
-                });
+                  await github.rest.issues.createComment({
+                    owner,
+                    repo,
+                    issue_number: pr_number,
+                    body: commentBody
+                  });
+                  console.log(`Added comment mentioning ${reviewersList.length} users and ${teamsList.length} teams`);
+                } else {
+                  console.log('No new codeowners to mention in comment (all previously pinged)');
+                }
               } catch (error) {
                 if (error.status === 422) {
                   console.log('Some reviewers may already be requested or unavailable:', error.message);
 
-                  // Try to add a comment even if review request failed
-                  const commentBody = createCommentBody(reviewersList, teamsList, fileMatches.size, false);
+                  // Only try to add a comment if there are new codeowners to mention
+                  if (reviewersList.length > 0 || teamsList.length > 0) {
+                    const commentBody = createCommentBody(reviewersList, teamsList, fileMatches.size, false);
 
-                  try {
-                    await github.rest.issues.createComment({
-                      owner,
-                      repo,
-                      issue_number: pr_number,
-                      body: commentBody
-                    });
-                  } catch (commentError) {
-                    console.log('Failed to add comment:', commentError.message);
+                    try {
+                      await github.rest.issues.createComment({
+                        owner,
+                        repo,
+                        issue_number: pr_number,
+                        body: commentBody
+                      });
+                      console.log(`Added fallback comment mentioning ${reviewersList.length} users and ${teamsList.length} teams`);
+                    } catch (commentError) {
+                      console.log('Failed to add comment:', commentError.message);
+                    }
+                  } else {
+                    console.log('No new codeowners to mention in fallback comment');
                   }
                 } else {
                   throw error;

.github/workflows/issue-codeowner-notify.yml

@@ -92,10 +92,49 @@ jobs:
                 mention !== `@${issueAuthor}`
               );
 
-              const allMentions = [...filteredUserOwners, ...teamOwners];
+              // Check for previous comments from this workflow to avoid duplicate pings
+              const { data: comments } = await github.rest.issues.listComments({
+                owner,
+                repo,
+                issue_number: issue_number
+              });
+
+              const previouslyPingedUsers = new Set();
+              const previouslyPingedTeams = new Set();
+
+              // Look for comments from github-actions bot that contain codeowner pings for this component
+              const workflowComments = comments.filter(comment =>
+                comment.user.type === 'Bot' &&
+                comment.user.login === 'github-actions[bot]' &&
+                comment.body.includes(`component: ${componentName}`) &&
+                comment.body.includes("you've been identified as a codeowner")
+              );
+
+              // Extract previously mentioned users and teams from workflow comments
+              for (const comment of workflowComments) {
+                // Match @username patterns (not team mentions)
+                const userMentions = comment.body.match(/@([a-zA-Z0-9_.-]+)(?![/])/g) || [];
+                userMentions.forEach(mention => {
+                  previouslyPingedUsers.add(mention); // Keep @ prefix for easy comparison
+                });
+
+                // Match @org/team patterns
+                const teamMentions = comment.body.match(/@[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+/g) || [];
+                teamMentions.forEach(mention => {
+                  previouslyPingedTeams.add(mention);
+                });
+              }
+
+              console.log(`Found ${previouslyPingedUsers.size} previously pinged users and ${previouslyPingedTeams.size} previously pinged teams for component ${componentName}`);
+
+              // Remove previously pinged users and teams
+              const newUserOwners = filteredUserOwners.filter(mention => !previouslyPingedUsers.has(mention));
+              const newTeamOwners = teamOwners.filter(mention => !previouslyPingedTeams.has(mention));
+
+              const allMentions = [...newUserOwners, ...newTeamOwners];
 
               if (allMentions.length === 0) {
-                console.log('No codeowners to notify (issue author is the only codeowner)');
+                console.log('No new codeowners to notify (all previously pinged or issue author is the only codeowner)');
                 return;
               }
 
@@ -111,7 +150,7 @@ jobs:
                 body: commentBody
               });
 
-              console.log(`Successfully notified codeowners: ${mentionString}`);
+              console.log(`Successfully notified new codeowners: ${mentionString}`);
 
             } catch (error) {
               console.log('Failed to process codeowner notifications:', error.message);

esphome/components/api/api_connection.cpp

@@ -79,14 +79,16 @@ APIConnection::APIConnection(std::unique_ptr<socket::Socket> sock, APIServer *pa
 #if defined(USE_API_PLAINTEXT) && defined(USE_API_NOISE)
   auto noise_ctx = parent->get_noise_ctx();
   if (noise_ctx->has_psk()) {
-    this->helper_ = std::unique_ptr<APIFrameHelper>{new APINoiseFrameHelper(std::move(sock), noise_ctx)};
+    this->helper_ =
+        std::unique_ptr<APIFrameHelper>{new APINoiseFrameHelper(std::move(sock), noise_ctx, &this->client_info_)};
   } else {
-    this->helper_ = std::unique_ptr<APIFrameHelper>{new APIPlaintextFrameHelper(std::move(sock))};
+    this->helper_ = std::unique_ptr<APIFrameHelper>{new APIPlaintextFrameHelper(std::move(sock), &this->client_info_)};
   }
 #elif defined(USE_API_PLAINTEXT)
-  this->helper_ = std::unique_ptr<APIFrameHelper>{new APIPlaintextFrameHelper(std::move(sock))};
+  this->helper_ = std::unique_ptr<APIFrameHelper>{new APIPlaintextFrameHelper(std::move(sock), &this->client_info_)};
 #elif defined(USE_API_NOISE)
-  this->helper_ = std::unique_ptr<APIFrameHelper>{new APINoiseFrameHelper(std::move(sock), parent->get_noise_ctx())};
+  this->helper_ = std::unique_ptr<APIFrameHelper>{
+      new APINoiseFrameHelper(std::move(sock), parent->get_noise_ctx(), &this->client_info_)};
 #else
 #error "No frame helper defined"
 #endif
@@ -109,9 +111,8 @@ void APIConnection::start() {
              errno);
     return;
   }
-  this->client_info_ = helper_->getpeername();
-  this->client_peername_ = this->client_info_;
-  this->helper_->set_log_info(this->client_info_);
+  this->client_info_.peername = helper_->getpeername();
+  this->client_info_.name = this->client_info_.peername;
 }
 
 APIConnection::~APIConnection() {
@@ -1374,7 +1375,7 @@ void APIConnection::complete_authentication_() {
   this->flags_.connection_state = static_cast<uint8_t>(ConnectionState::AUTHENTICATED);
   ESP_LOGD(TAG, "%s connected", this->get_client_combined_info().c_str());
 #ifdef USE_API_CLIENT_CONNECTED_TRIGGER
-  this->parent_->get_client_connected_trigger()->trigger(this->client_info_, this->client_peername_);
+  this->parent_->get_client_connected_trigger()->trigger(this->client_info_.name, this->client_info_.peername);
 #endif
 #ifdef USE_HOMEASSISTANT_TIME
   if (homeassistant::global_homeassistant_time != nullptr) {
@@ -1384,13 +1385,12 @@ void APIConnection::complete_authentication_() {
 }
 
 HelloResponse APIConnection::hello(const HelloRequest &msg) {
-  this->client_info_ = msg.client_info;
-  this->client_peername_ = this->helper_->getpeername();
-  this->helper_->set_log_info(this->get_client_combined_info());
+  this->client_info_.name = msg.client_info;
+  this->client_info_.peername = this->helper_->getpeername();
   this->client_api_version_major_ = msg.api_version_major;
   this->client_api_version_minor_ = msg.api_version_minor;
-  ESP_LOGV(TAG, "Hello from client: '%s' | %s | API Version %" PRIu32 ".%" PRIu32, this->client_info_.c_str(),
-           this->client_peername_.c_str(), this->client_api_version_major_, this->client_api_version_minor_);
+  ESP_LOGV(TAG, "Hello from client: '%s' | %s | API Version %" PRIu32 ".%" PRIu32, this->client_info_.name.c_str(),
+           this->client_info_.peername.c_str(), this->client_api_version_major_, this->client_api_version_minor_);
 
   HelloResponse resp;
   resp.api_version_major = 1;

esphome/components/api/api_connection.h

@@ -16,6 +16,20 @@
 namespace esphome {
 namespace api {
 
+// Client information structure
+struct ClientInfo {
+  std::string name;      // Client name from Hello message
+  std::string peername;  // IP:port from socket
+
+  std::string get_combined_info() const {
+    if (name == peername) {
+      // Before Hello message, both are the same
+      return name;
+    }
+    return name + " (" + peername + ")";
+  }
+};
+
 // Keepalive timeout in milliseconds
 static constexpr uint32_t KEEPALIVE_TIMEOUT_MS = 60000;
 // Maximum number of entities to process in a single batch during initial state/info sending
@@ -270,13 +284,7 @@ class APIConnection : public APIServerConnection {
   bool try_to_clear_buffer(bool log_out_of_space);
   bool send_buffer(ProtoWriteBuffer buffer, uint8_t message_type) override;
 
-  std::string get_client_combined_info() const {
-    if (this->client_info_ == this->client_peername_) {
-      // Before Hello message, both are the same (just IP:port)
-      return this->client_info_;
-    }
-    return this->client_info_ + " (" + this->client_peername_ + ")";
-  }
+  std::string get_client_combined_info() const { return this->client_info_.get_combined_info(); }
 
   // Buffer allocator methods for batch processing
   ProtoWriteBuffer allocate_single_message_buffer(uint16_t size);
@@ -482,9 +490,8 @@ class APIConnection : public APIServerConnection {
   std::unique_ptr<camera::CameraImageReader> image_reader_;
 #endif
 
-  // Group 3: Strings (12 bytes each on 32-bit, 4-byte aligned)
-  std::string client_info_;
-  std::string client_peername_;
+  // Group 3: Client info struct (24 bytes on 32-bit: 2 strings × 12 bytes each)
+  ClientInfo client_info_;
 
   // Group 4: 4-byte types
   uint32_t last_traffic_;

esphome/components/api/api_frame_helper.cpp

@@ -1,5 +1,6 @@
 #include "api_frame_helper.h"
 #ifdef USE_API
+#include "api_connection.h"  // For ClientInfo struct
 #include "esphome/core/application.h"
 #include "esphome/core/hal.h"
 #include "esphome/core/helpers.h"
@@ -13,6 +14,8 @@ namespace api {
 
 static const char *const TAG = "api.socket";
 
+#define HELPER_LOG(msg, ...) ESP_LOGVV(TAG, "%s: " msg, this->client_info_->get_combined_info().c_str(), ##__VA_ARGS__)
+
 const char *api_error_to_str(APIError err) {
   // not using switch to ensure compiler doesn't try to build a big table out of it
   if (err == APIError::OK) {
@@ -81,7 +84,7 @@ APIError APIFrameHelper::handle_socket_write_error_() {
   if (errno == EWOULDBLOCK || errno == EAGAIN) {
     return APIError::WOULD_BLOCK;
   }
-  ESP_LOGVV(TAG, "%s: Socket write failed with errno %d", this->info_.c_str(), errno);
+  HELPER_LOG("Socket write failed with errno %d", errno);
   this->state_ = State::FAILED;
   return APIError::SOCKET_WRITE_FAILED;
 }
@@ -198,13 +201,13 @@ APIError APIFrameHelper::try_send_tx_buf_() {
 
 APIError APIFrameHelper::init_common_() {
   if (state_ != State::INITIALIZE || this->socket_ == nullptr) {
-    ESP_LOGVV(TAG, "%s: Bad state for init %d", this->info_.c_str(), (int) state_);
+    HELPER_LOG("Bad state for init %d", (int) state_);
     return APIError::BAD_STATE;
   }
   int err = this->socket_->setblocking(false);
   if (err != 0) {
     state_ = State::FAILED;
-    ESP_LOGVV(TAG, "%s: Setting nonblocking failed with errno %d", this->info_.c_str(), errno);
+    HELPER_LOG("Setting nonblocking failed with errno %d", errno);
     return APIError::TCP_NONBLOCKING_FAILED;
   }
 
@@ -212,14 +215,12 @@ APIError APIFrameHelper::init_common_() {
   err = this->socket_->setsockopt(IPPROTO_TCP, TCP_NODELAY, &enable, sizeof(int));
   if (err != 0) {
     state_ = State::FAILED;
-    ESP_LOGVV(TAG, "%s: Setting nodelay failed with errno %d", this->info_.c_str(), errno);
+    HELPER_LOG("Setting nodelay failed with errno %d", errno);
     return APIError::TCP_NODELAY_FAILED;
   }
   return APIError::OK;
 }
 
-#define HELPER_LOG(msg, ...) ESP_LOGVV(TAG, "%s: " msg, this->info_.c_str(), ##__VA_ARGS__)
-
 APIError APIFrameHelper::handle_socket_read_result_(ssize_t received) {
   if (received == -1) {
     if (errno == EWOULDBLOCK || errno == EAGAIN) {

esphome/components/api/api_frame_helper.h

@@ -19,6 +19,9 @@
 namespace esphome {
 namespace api {
 
+// Forward declaration
+struct ClientInfo;
+
 class ProtoWriteBuffer;
 
 struct ReadPacketBuffer {
@@ -68,7 +71,8 @@ const char *api_error_to_str(APIError err);
 class APIFrameHelper {
  public:
   APIFrameHelper() = default;
-  explicit APIFrameHelper(std::unique_ptr<socket::Socket> socket) : socket_owned_(std::move(socket)) {
+  explicit APIFrameHelper(std::unique_ptr<socket::Socket> socket, const ClientInfo *client_info)
+      : socket_owned_(std::move(socket)), client_info_(client_info) {
     socket_ = socket_owned_.get();
   }
   virtual ~APIFrameHelper() = default;
@@ -94,8 +98,6 @@ class APIFrameHelper {
     }
     return APIError::OK;
   }
-  // Give this helper a name for logging
-  void set_log_info(std::string info) { info_ = std::move(info); }
   virtual APIError write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) = 0;
   // Write multiple protobuf packets in a single operation
   // packets contains (message_type, offset, length) for each message in the buffer
@@ -160,10 +162,13 @@ class APIFrameHelper {
 
   // Containers (size varies, but typically 12+ bytes on 32-bit)
   std::deque<SendBuffer> tx_buf_;
-  std::string info_;
   std::vector<struct iovec> reusable_iovs_;
   std::vector<uint8_t> rx_buf_;
 
+  // Pointer to client info (4 bytes on 32-bit)
+  // Note: The pointed-to ClientInfo object must outlive this APIFrameHelper instance.
+  const ClientInfo *client_info_{nullptr};
+
   // Group smaller types together
   uint16_t rx_buf_len_ = 0;
   State state_{State::INITIALIZE};
@@ -181,8 +186,9 @@ class APIFrameHelper {
 #ifdef USE_API_NOISE
 class APINoiseFrameHelper : public APIFrameHelper {
  public:
-  APINoiseFrameHelper(std::unique_ptr<socket::Socket> socket, std::shared_ptr<APINoiseContext> ctx)
-      : APIFrameHelper(std::move(socket)), ctx_(std::move(ctx)) {
+  APINoiseFrameHelper(std::unique_ptr<socket::Socket> socket, std::shared_ptr<APINoiseContext> ctx,
+                      const ClientInfo *client_info)
+      : APIFrameHelper(std::move(socket), client_info), ctx_(std::move(ctx)) {
     // Noise header structure:
     // Pos 0: indicator (0x01)
     // Pos 1-2: encrypted payload size (16-bit big-endian)
@@ -238,7 +244,8 @@ class APINoiseFrameHelper : public APIFrameHelper {
 #ifdef USE_API_PLAINTEXT
 class APIPlaintextFrameHelper : public APIFrameHelper {
  public:
-  APIPlaintextFrameHelper(std::unique_ptr<socket::Socket> socket) : APIFrameHelper(std::move(socket)) {
+  APIPlaintextFrameHelper(std::unique_ptr<socket::Socket> socket, const ClientInfo *client_info)
+      : APIFrameHelper(std::move(socket), client_info) {
     // Plaintext header structure (worst case):
     // Pos 0: indicator (0x00)
     // Pos 1-3: payload size varint (up to 3 bytes)

esphome/components/api/api_server.cpp

@@ -184,9 +184,9 @@ void APIServer::loop() {
 
     // Rare case: handle disconnection
 #ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER
-    this->client_disconnected_trigger_->trigger(client->client_info_, client->client_peername_);
+    this->client_disconnected_trigger_->trigger(client->client_info_.name, client->client_info_.peername);
 #endif
-    ESP_LOGV(TAG, "Remove connection %s", client->client_info_.c_str());
+    ESP_LOGV(TAG, "Remove connection %s", client->client_info_.name.c_str());
 
     // Swap with the last element and pop (avoids expensive vector shifts)
     if (client_index < this->clients_.size() - 1) {