branch_worker: implement "always_reply_to_author" flag

kernel_patches_daemon/branch_worker.py

@@ -11,6 +11,7 @@
 import email
 import email.parser
 import email.policy
+import email.utils
 import hashlib
 import logging
 import os
@@ -378,6 +379,7 @@ def reply_email_recipients(
     msg: EmailMessage,
     allowlist: Optional[Sequence[re.Pattern]] = None,
     denylist: Optional[Sequence[re.Pattern]] = None,
+    always_reply_to_author: bool = False,
 ) -> Tuple[List[str], List[str]]:
     """
     Extracts response recipients from the `msg`, applying allowlist/denylist.
@@ -386,18 +388,27 @@ def reply_email_recipients(
         msg: the EmailMessage we will be replying to
         allowlist: list of email address regexes to allow, ignored if empty
         denylist: list of email address regexes to deny, ignored if empty
+        always_reply_to_author: if True, the patch author (message sender)
+            will be added to the to_list even if filtered out by allowlist/denylist
 
     Returns:
         (to_list, cc_list) - recipients of the reply email
     """
+
+    sender_address = None
+    to_list = []
+
+    sender = msg.get("From")
+    if sender is not None:
+        (_, sender_address) = email.utils.parseaddr(sender)
+        if sender_address:  # parseaddr might return an emptystring
+            to_list.append(sender_address)
+
     tos = msg.get_all("To", [])
-    ccs = msg.get_all("Cc", [])
-    # pyrefly: ignore  # implicit-import
-    cc_list = [a for (_, a) in email.utils.getaddresses(tos + ccs)]
+    to_list += [a for (_, a) in email.utils.getaddresses(tos)]
 
-    # pyrefly: ignore  # implicit-import, bad-argument-type
-    (_, sender_address) = email.utils.parseaddr(msg.get("From"))
-    to_list = [sender_address]
+    ccs = msg.get_all("Cc", [])
+    cc_list = [a for (_, a) in email.utils.getaddresses(ccs)]
 
     if allowlist:
         cc_list = [a for a in cc_list if email_matches_any(a, allowlist)]
@@ -407,6 +418,9 @@ def reply_email_recipients(
         cc_list = [a for a in cc_list if not email_matches_any(a, denylist)]
         to_list = [a for a in to_list if not email_matches_any(a, denylist)]
 
+    if always_reply_to_author and sender_address and sender_address not in to_list:
+        to_list.append(sender_address)
+
     return (to_list, cc_list)
 
 
@@ -433,7 +447,10 @@ async def send_pr_comment_email(
         return
 
     (to_list, cc_list) = reply_email_recipients(
-        msg, allowlist=cfg.recipient_allowlist, denylist=cfg.recipient_denylist
+        msg,
+        allowlist=cfg.recipient_allowlist,
+        denylist=cfg.recipient_denylist,
+        always_reply_to_author=cfg.always_reply_to_author,
     )
     cc_list += cfg.always_cc
 

kernel_patches_daemon/config.py

@@ -108,6 +108,7 @@ def from_json(cls, json: Dict) -> "BranchConfig":
 class PRCommentsForwardingConfig:
     enabled: bool
     always_cc: List[str]
+    always_reply_to_author: bool
     commenter_allowlist: List[str]
     recipient_allowlist: List[re.Pattern]
     recipient_denylist: List[re.Pattern]
@@ -117,6 +118,7 @@ def from_json(cls, json: Dict) -> "PRCommentsForwardingConfig":
         return cls(
             enabled=json.get("enabled", False),
             always_cc=json.get("always_cc", []),
+            always_reply_to_author=json.get("always_reply_to_author", False),
             recipient_allowlist=[
                 re.compile(pattern) for pattern in json.get("recipient_allowlist", [])
             ],

tests/test_branch_worker.py

@@ -7,6 +7,8 @@
 # pyre-unsafe
 
 import email
+import email.parser
+import email.policy
 import json
 import os
 import random
@@ -42,7 +44,6 @@
     prs_for_the_same_series,
     reply_email_recipients,
     same_series_different_target,
-    send_pr_comment_email,
     temporary_patch_file,
     UPSTREAM_REMOTE_NAME,
 )
@@ -1480,26 +1481,21 @@ def test_email_submitter_not_in_allowlist_and_allowlist_disabled(self):
         self.assertEqual(expected_email, email)
 
     def test_reply_email_recipients(self):
-        kpd_config_json = json.loads(read_fixture("kpd_config.json"))
-        kpd_config = KPDConfig.from_json(kpd_config_json)
-        self.assertIsNotNone(kpd_config)
-
         mbox = read_test_data_file(
             "test_sync_patches_pr_summary_success/series-970926.mbox"
         )
-        # pyrefly: ignore  # implicit-import
         parser = email.parser.BytesParser(policy=email.policy.default)
         msg = parser.parsebytes(mbox.encode("utf-8"), headersonly=True)
         self.assertIsNotNone(mbox)
-        # pyrefly: ignore  # missing-attribute
-        denylist = kpd_config.email.pr_comments_forwarding.recipient_denylist
+        denylist = [re.compile(".*@vger.kernel.org")]
         (to_list, cc_list) = reply_email_recipients(msg, denylist=denylist)
 
-        self.assertEqual(to_list, ["[email protected]"])
-        self.assertEqual(len(cc_list), 17)
+        self.assertIn("[email protected]", to_list)
+        self.assertEqual(len(to_list), 17)
+        self.assertEqual(len(cc_list), 1)
 
-        # test allowlist by using the same denylist
-        (to_list, cc_list) = reply_email_recipients(msg, allowlist=denylist)
+        allowlist = [re.compile(".*@vger.kernel.org")]
+        (to_list, cc_list) = reply_email_recipients(msg, allowlist=allowlist)
         self.assertEqual(to_list, [])
         self.assertEqual(len(cc_list), 3)
 
@@ -1509,8 +1505,23 @@ def test_reply_email_recipients(self):
         (to_list, cc_list) = reply_email_recipients(
             msg, allowlist=allowlist, denylist=denylist
         )
+        self.assertIn("[email protected]", to_list)
+        self.assertEqual(len(to_list), 3)
+        self.assertEqual(len(cc_list), 1)
+
+        # test denylist all
+        denylist = [re.compile(".*")]
+        (to_list, cc_list) = reply_email_recipients(msg, denylist=denylist)
+        self.assertEqual(to_list, [])
+        self.assertEqual(cc_list, [])
+
+        # test denylist all, but the sender
+        denylist = [re.compile(".*")]
+        (to_list, cc_list) = reply_email_recipients(
+            msg, denylist=denylist, always_reply_to_author=True
+        )
         self.assertEqual(to_list, ["[email protected]"])
-        self.assertEqual(len(cc_list), 3)
+        self.assertEqual(cc_list, [])
 
 
 class TestParsePrRef(unittest.TestCase):
@@ -1655,6 +1666,7 @@ async def test_forward_pr_comments(self, m: aioresponses) -> None:
         self._bw.email_config = MagicMock(
             pr_comments_forwarding=PRCommentsForwardingConfig(
                 enabled=True,
+                always_reply_to_author=False,
                 always_cc=["[email protected]"],
                 commenter_allowlist=["test_user"],
                 recipient_denylist=[re.compile(".*@vger.kernel.org")],
@@ -1702,9 +1714,10 @@ async def test_forward_pr_comments(self, m: aioresponses) -> None:
                 mock_send_email.call_args[0]
             )
 
-            self.assertEqual(to_list, ["[email protected]"])
-            self.assertEqual(len(cc_list), 18)
+            self.assertIn("[email protected]", to_list)
+            self.assertEqual(len(to_list), 17)
             self.assertIn("[email protected]", cc_list)
+            self.assertEqual(len(cc_list), 2)
             self.assertEqual(
                 "Re: [PATCH bpf-next] bpf: clear user buf when bpf_d_path failed",
                 subject,

tests/test_config.py

@@ -175,6 +175,7 @@ def test_valid(self) -> None:
                 pr_comments_forwarding=PRCommentsForwardingConfig(
                     enabled=True,
                     always_cc=["[email protected]"],
+                    always_reply_to_author=False,
                     commenter_allowlist=["kpd-bot[bot]"],
                     recipient_denylist=[re.compile(".*@vger.kernel.org")],
                     recipient_allowlist=[],