Build separate mail for each recipient, honoring recipient access levels

Summary:
Ref T6367. Removes `multiplexMail()`!

We can't pass a single body into a function which splits it anymore: we need to split recipients first, then build bodies for each recipient list. This lets us build separate bodies for each recipient's individual translation/access levels.

The new logic does this:

  - First, split recipients into groups called "targets".
    - Each target corresponds to one actual mail we're going to build.
    - Each target has a viewer (whose translation / access levels will be used to generate the mail).
    - Each target has a to/cc list (the users who we'll ultimately send the mail to).
  - For each target, build a custom mail body based on the viewer's access levels and settings (language prefs not actually implemented).
  - Then, deliver the mail.

Test Plan:
  - Read new config help.

Then did a bunch of testing, primarily with `bin/mail list-outbound` and `bin/mail show-outbound` (to review generated mail), `bin/phd debug taskmaster` (to run daemons freely) and `bin/worker execute --id <id>` (to repeatedly test a specific piece of code after identifying an issue).

With `one-mail-per-recipient` on (default):

  - Sent mail to multiple users.
  - Verified mail showed up in `mail list-outbound`.
  - Examined mail with `mail show-outbound`.
  - Added a project that a subscriber could not see.
    - Verified it was not present in `X-Phabricator-Projects`.
    - Verified it was rendered as "Restricted Project" for the non-permissioned viewer.
  - Added a subscriber, then changed the object policy so they could not see it and sent mail.
    - Verified I received mail but the other user did not.
  - Enabled public replies and verified mail generated with public addresses.
  - Disabld public replies and verified mail generated with private addresses.

With `one-mail-per-recipient` off:

  - Verified that one mail is sent to all recipients.
  - Verified users who can not see the object are still filtered.
  - Verified that partially-visible projects are completely visible in the mail (this violates policies, as documented, as the best available compromise).
  - Enabled public replies and verified the mail generated with "Reply To".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: carlsverre, epriestley

Maniphest Tasks: T6367

Differential Revision: https://secure.phabricator.com/D13131

src/__phutil_library_map__.php

@@ -2021,6 +2021,7 @@ phutil_register_library_map(array(
     'PhabricatorMailReceiverTestCase' => 'applications/metamta/receiver/__tests__/PhabricatorMailReceiverTestCase.php',
     'PhabricatorMailReplyHandler' => 'applications/metamta/replyhandler/PhabricatorMailReplyHandler.php',
     'PhabricatorMailSetupCheck' => 'applications/config/check/PhabricatorMailSetupCheck.php',
+    'PhabricatorMailTarget' => 'applications/metamta/replyhandler/PhabricatorMailTarget.php',
     'PhabricatorMailgunConfigOptions' => 'applications/config/option/PhabricatorMailgunConfigOptions.php',
     'PhabricatorMainMenuSearchView' => 'view/page/menu/PhabricatorMainMenuSearchView.php',
     'PhabricatorMainMenuView' => 'view/page/menu/PhabricatorMainMenuView.php',
@@ -5429,6 +5430,7 @@ phutil_register_library_map(array(
     'PhabricatorMailManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'PhabricatorMailReceiverTestCase' => 'PhabricatorTestCase',
     'PhabricatorMailSetupCheck' => 'PhabricatorSetupCheck',
+    'PhabricatorMailTarget' => 'Phobject',
     'PhabricatorMailgunConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorMainMenuSearchView' => 'AphrontView',
     'PhabricatorMainMenuView' => 'AphrontView',

src/applications/config/option/PhabricatorMetaMTAConfigOptions.php

@@ -54,6 +54,8 @@ alincoln", "To: usgrant", "To: htaft"). The major advantages and disadvantages
 of each approach are:
 
   - One mail to everyone:
+    - This violates policy controls. The body of the mail is generated without
+      respect for object policies.
     - Recipients can see To/Cc at a glance.
     - If you use mailing lists, you won't get duplicate mail if you're
       a normal recipient and also Cc'd on a mailing list.
@@ -65,6 +67,7 @@ of each approach are:
     - Not supported with a private reply-to address.
     - Mails are sent in the server default translation.
   - One mail to each user:
+    - Policy controls work correctly and are enforced per-user.
     - Recipients need to look in the mail body to see To/Cc.
     - If you use mailing lists, recipients may sometimes get duplicate
       mail.
@@ -74,8 +77,6 @@ of each approach are:
     - Required if private reply-to addresses are configured.
     - Mails are sent in the language of user preference.
 
-In the code, splitting one outbound email into one-per-recipient is sometimes
-referred to as "multiplexing".
 EODOC
 ));
 
@@ -222,6 +223,7 @@ EODOC
         'metamta.one-mail-per-recipient',
         'bool',
         true)
+        ->setLocked(true)
         ->setBoolOptions(
           array(
             pht('Send Mail To Each Recipient'),

src/applications/conpherence/mail/ConpherenceReplyHandler.php

@@ -21,9 +21,8 @@ final class ConpherenceReplyHandler extends PhabricatorMailReplyHandler {
     }
   }
 
-  public function getPrivateReplyHandlerEmailAddress(
+  public function getPrivateReplyHandlerEmailAddress(PhabricatorUser $user) {
-    PhabricatorObjectHandle $handle) {
+    return $this->getDefaultPrivateReplyHandlerEmailAddress($user, 'Z');
-    return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'Z');
   }
 
   public function getPublicReplyHandlerEmailAddress() {

src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php

@@ -47,7 +47,7 @@ abstract class PhabricatorMailReplyHandler {
 
   abstract public function validateMailReceiver($mail_receiver);
   abstract public function getPrivateReplyHandlerEmailAddress(
-    PhabricatorObjectHandle $handle);
+    PhabricatorUser $user);
 
   public function getReplyHandlerDomain() {
     return PhabricatorEnv::getEnvConfig('metamta.reply-handler-domain');
@@ -117,151 +117,6 @@ abstract class PhabricatorMailReplyHandler {
     return null;
   }
 
-  final public function getRecipientsSummary(
-    array $to_handles,
-    array $cc_handles) {
-    assert_instances_of($to_handles, 'PhabricatorObjectHandle');
-    assert_instances_of($cc_handles, 'PhabricatorObjectHandle');
-
-    $body = '';
-
-    if (PhabricatorEnv::getEnvConfig('metamta.recipients.show-hints')) {
-      if ($to_handles) {
-        $body .= "To: ".implode(', ', mpull($to_handles, 'getName'))."\n";
-      }
-      if ($cc_handles) {
-        $body .= "Cc: ".implode(', ', mpull($cc_handles, 'getName'))."\n";
-      }
-    }
-
-    return $body;
-  }
-
-  final public function getRecipientsSummaryHTML(
-    array $to_handles,
-    array $cc_handles) {
-    assert_instances_of($to_handles, 'PhabricatorObjectHandle');
-    assert_instances_of($cc_handles, 'PhabricatorObjectHandle');
-
-    if (PhabricatorEnv::getEnvConfig('metamta.recipients.show-hints')) {
-      $body = array();
-      if ($to_handles) {
-        $body[] = phutil_tag('strong', array(), 'To: ');
-        $body[] = phutil_implode_html(', ', mpull($to_handles, 'getName'));
-        $body[] = phutil_tag('br');
-      }
-      if ($cc_handles) {
-        $body[] = phutil_tag('strong', array(), 'Cc: ');
-        $body[] = phutil_implode_html(', ', mpull($cc_handles, 'getName'));
-        $body[] = phutil_tag('br');
-      }
-      return phutil_tag('div', array(), $body);
-    } else {
-      return '';
-    }
-
-  }
-
-  final public function multiplexMail(
-    PhabricatorMetaMTAMail $mail_template,
-    array $to_handles,
-    array $cc_handles) {
-    assert_instances_of($to_handles, 'PhabricatorObjectHandle');
-    assert_instances_of($cc_handles, 'PhabricatorObjectHandle');
-
-    $result = array();
-
-    // If MetaMTA is configured to always multiplex, skip the single-email
-    // case.
-    if (!PhabricatorMetaMTAMail::shouldMultiplexAllMail()) {
-      // If private replies are not supported, simply send one email to all
-      // recipients and CCs. This covers cases where we have no reply handler,
-      // or we have a public reply handler.
-      if (!$this->supportsPrivateReplies()) {
-        $mail = clone $mail_template;
-        $mail->addTos(mpull($to_handles, 'getPHID'));
-        $mail->addCCs(mpull($cc_handles, 'getPHID'));
-
-        if ($this->supportsPublicReplies()) {
-          $reply_to = $this->getPublicReplyHandlerEmailAddress();
-          $mail->setReplyTo($reply_to);
-        }
-
-        $result[] = $mail;
-
-        return $result;
-      }
-    }
-
-    // TODO: This is pretty messy. We should really be doing all of this
-    // multiplexing in the task queue, but that requires significant rewriting
-    // in the general case. ApplicationTransactions can do it fairly easily,
-    // but other mail sites currently can not, so we need to support this
-    // junky version until they catch up and we can swap things over.
-
-    $to_handles = $this->expandRecipientHandles($to_handles);
-    $cc_handles = $this->expandRecipientHandles($cc_handles);
-
-    $tos = mpull($to_handles, null, 'getPHID');
-    $ccs = mpull($cc_handles, null, 'getPHID');
-
-    // Merge all the recipients together. TODO: We could keep the CCs as real
-    // CCs and send to a "noreply@domain.com" type address, but keep it simple
-    // for now.
-    $recipients = $tos + $ccs;
-
-    // When multiplexing mail, explicitly include To/Cc information in the
-    // message body and headers.
-
-    $mail_template = clone $mail_template;
-
-    $mail_template->addPHIDHeaders('X-Phabricator-To', array_keys($tos));
-    $mail_template->addPHIDHeaders('X-Phabricator-Cc', array_keys($ccs));
-
-    $body = $mail_template->getBody();
-    $body .= "\n";
-    $body .= $this->getRecipientsSummary($to_handles, $cc_handles);
-
-    $html_body = $mail_template->getHTMLBody();
-    if (strlen($html_body)) {
-      $html_body .= hsprintf('%s',
-        $this->getRecipientsSummaryHTML($to_handles, $cc_handles));
-    }
-
-    foreach ($recipients as $phid => $recipient) {
-
-      $mail = clone $mail_template;
-      if (isset($to_handles[$phid])) {
-        $mail->addTos(array($phid));
-      } else if (isset($cc_handles[$phid])) {
-        $mail->addCCs(array($phid));
-      } else {
-        // not good - they should be a to or a cc
-        continue;
-      }
-
-      $mail->setBody($body);
-      $mail->setHTMLBody($html_body);
-
-      $reply_to = null;
-      if (!$reply_to && $this->supportsPrivateReplies()) {
-        $reply_to = $this->getPrivateReplyHandlerEmailAddress($recipient);
-      }
-
-      if (!$reply_to && $this->supportsPublicReplies()) {
-        $reply_to = $this->getPublicReplyHandlerEmailAddress();
-      }
-
-      if ($reply_to) {
-        $mail->setReplyTo($reply_to);
-      }
-
-      $result[] = $mail;
-    }
-
-    return $result;
-  }
-
   protected function getDefaultPublicReplyHandlerEmailAddress($prefix) {
 
     $receiver = $this->getMailReceiver();
@@ -288,31 +143,15 @@ abstract class PhabricatorMailReplyHandler {
   }
 
   protected function getDefaultPrivateReplyHandlerEmailAddress(
-    PhabricatorObjectHandle $handle,
+    PhabricatorUser $user,
     $prefix) {
 
-    if ($handle->getType() != PhabricatorPeopleUserPHIDType::TYPECONST) {
-      // You must be a real user to get a private reply handler address.
-      return null;
-    }
-
-    $user = id(new PhabricatorPeopleQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withPHIDs(array($handle->getPHID()))
-      ->executeOne();
-
-    if (!$user) {
-      // This may happen if a user was subscribed to something, and was then
-      // deleted.
-      return null;
-    }
-
     $receiver = $this->getMailReceiver();
     $receiver_id = $receiver->getID();
     $user_id = $user->getID();
     $hash = PhabricatorObjectMailReceiver::computeMailHash(
       $receiver->getMailKey(),
-      $handle->getPHID());
+      $user->getPHID());
     $domain = $this->getReplyHandlerDomain();
 
     $address = "{$prefix}{$receiver_id}+{$user_id}+{$hash}@{$domain}";
@@ -368,21 +207,188 @@ abstract class PhabricatorMailReplyHandler {
     return rtrim($output);
   }
 
-  private function expandRecipientHandles(array $handles) {
+
-    if (!$handles) {
+  /**
+   * Produce a list of mail targets for a given to/cc list.
+   *
+   * Each target should be sent a separate email, and contains the information
+   * required to generate it with appropriate permissions and configuration.
+   *
+   * @param list<phid> List of "To" PHIDs.
+   * @param list<phid> List of "CC" PHIDs.
+   * @return list<PhabricatorMailTarget> List of targets.
+   */
+  final public function getMailTargets(array $raw_to, array $raw_cc) {
+    list($to, $cc) = $this->expandRecipientPHIDs($raw_to, $raw_cc);
+    list($to, $cc) = $this->loadRecipientUsers($to, $cc);
+    list($to, $cc) = $this->filterRecipientUsers($to, $cc);
+
+    if (!$to && !$cc) {
       return array();
     }
 
-    $phids = mpull($handles, 'getPHID');
+    $template = id(new PhabricatorMailTarget())
-    $results = id(new PhabricatorMetaMTAMemberQuery())
+      ->setRawToPHIDs($raw_to)
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->setRawCCPHIDs($raw_cc);
-      ->withPHIDs($phids)
-      ->executeExpansion();
 
-    return id(new PhabricatorHandleQuery())
+    // Set the public reply address as the default, if one exists. We
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+    // might replace this with a private address later.
-      ->withPHIDs($results)
+    if ($this->supportsPublicReplies()) {
-      ->execute();
+      $reply_to = $this->getPublicReplyHandlerEmailAddress();
+      if ($reply_to) {
+        $template->setReplyTo($reply_to);
+      }
+    }
+
+    $supports_private_replies = $this->supportsPrivateReplies();
+    $mail_all = !PhabricatorEnv::getEnvConfig('metamta.one-mail-per-recipient');
+    $targets = array();
+    if ($mail_all) {
+      $target = id(clone $template)
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->setToMap($to)
+        ->setCCMap($cc);
+
+      $targets[] = $target;
+    } else {
+      $map = $to + $cc;
+
+      foreach ($map as $phid => $user) {
+        $target = id(clone $template)
+          ->setViewer($user)
+          ->setToMap(array($phid => $user))
+          ->setCCMap(array());
+
+        if ($supports_private_replies) {
+          $reply_to = $this->getPrivateReplyHandlerEmailAddress($user);
+          if ($reply_to) {
+            $target->setReplyTo($reply_to);
+          }
+        }
+
+        $targets[] = $target;
+      }
+    }
+
+    return $targets;
+  }
+
+
+  /**
+   * Expand lists of recipient PHIDs.
+   *
+   * This takes any compound recipients (like projects) and looks up all their
+   * members.
+   *
+   * @param list<phid> List of To PHIDs.
+   * @param list<phid> List of CC PHIDs.
+   * @return pair<list<phid>, list<phid>> Expanded PHID lists.
+   */
+  private function expandRecipientPHIDs(array $to, array $cc) {
+    $to_result = array();
+    $cc_result = array();
+
+    $all_phids = array_merge($to, $cc);
+    if ($all_phids) {
+      $map = id(new PhabricatorMetaMTAMemberQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs($all_phids)
+        ->execute();
+      foreach ($to as $phid) {
+        foreach ($map[$phid] as $expanded) {
+          $to_result[$expanded] = $expanded;
+        }
+      }
+      foreach ($cc as $phid) {
+        foreach ($map[$phid] as $expanded) {
+          $cc_result[$expanded] = $expanded;
+        }
+      }
+    }
+
+    // Remove recipients from "CC" if they're also present in "To".
+    $cc_result = array_diff_key($cc_result, $to_result);
+
+    return array(array_values($to_result), array_values($cc_result));
+  }
+
+
+  /**
+   * Load @{class:PhabricatorUser} objects for each recipient.
+   *
+   * Invalid recipients are dropped from the results.
+   *
+   * @param list<phid> List of To PHIDs.
+   * @param list<phid> List of CC PHIDs.
+   * @return pair<wild, wild> Maps from PHIDs to users.
+   */
+  private function loadRecipientUsers(array $to, array $cc) {
+    $to_result = array();
+    $cc_result = array();
+
+    $all_phids = array_merge($to, $cc);
+    if ($all_phids) {
+      $users = id(new PhabricatorPeopleQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs($all_phids)
+        ->execute();
+      $users = mpull($users, null, 'getPHID');
+
+      foreach ($to as $phid) {
+        if (isset($users[$phid])) {
+          $to_result[$phid] = $users[$phid];
+        }
+      }
+      foreach ($cc as $phid) {
+        if (isset($users[$phid])) {
+          $cc_result[$phid] = $users[$phid];
+        }
+      }
+    }
+
+    return array($to_result, $cc_result);
+  }
+
+
+  /**
+   * Remove recipients who do not have permission to view the mail receiver.
+   *
+   * @param map<string, PhabricatorUser> Map of "To" users.
+   * @param map<string, PhabricatorUser> Map of "CC" users.
+   * @return pair<wild, wild> Filtered user maps.
+   */
+  private function filterRecipientUsers(array $to, array $cc) {
+    $to_result = array();
+    $cc_result = array();
+
+    $all_users = $to + $cc;
+    if ($all_users) {
+      $can_see = array();
+      $object = $this->getMailReceiver();
+      foreach ($all_users as $phid => $user) {
+        $visible = PhabricatorPolicyFilter::hasCapability(
+          $user,
+          $object,
+          PhabricatorPolicyCapability::CAN_VIEW);
+        if ($visible) {
+          $can_see[$phid] = true;
+        }
+      }
+
+      foreach ($to as $phid => $user) {
+        if (!empty($can_see[$phid])) {
+          $to_result[$phid] = $all_users[$phid];
+        }
+      }
+
+      foreach ($cc as $phid => $user) {
+        if (!empty($can_see[$phid])) {
+          $cc_result[$phid] = $all_users[$phid];
+        }
+      }
+    }
+
+    return array($to_result, $cc_result);
   }
 
 }

src/applications/metamta/replyhandler/PhabricatorMailTarget.php

@@ -0,0 +1,146 @@
+<?php
+
+final class PhabricatorMailTarget extends Phobject {
+
+  private $viewer;
+  private $replyTo;
+  private $toMap = array();
+  private $ccMap = array();
+  private $rawToPHIDs;
+  private $rawCCPHIDs;
+
+  public function setRawToPHIDs(array $to_phids) {
+    $this->rawToPHIDs = $to_phids;
+    return $this;
+  }
+
+  public function setRawCCPHIDs(array $cc_phids) {
+    $this->rawCCPHIDs = $cc_phids;
+    return $this;
+  }
+
+  public function setCCMap(array $cc_map) {
+    $this->ccMap = $cc_map;
+    return $this;
+  }
+
+  public function getCCMap() {
+    return $this->ccMap;
+  }
+
+  public function setToMap(array $to_map) {
+    $this->toMap = $to_map;
+    return $this;
+  }
+
+  public function getToMap() {
+    return $this->toMap;
+  }
+
+  public function setReplyTo($reply_to) {
+    $this->replyTo = $reply_to;
+    return $this;
+  }
+
+  public function getReplyTo() {
+    return $this->replyTo;
+  }
+
+  public function setViewer($viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  public function getViewer() {
+    return $this->viewer;
+  }
+
+  public function sendMail(PhabricatorMetaMTAMail $mail) {
+    $viewer = $this->getViewer();
+
+    $mail->addPHIDHeaders('X-Phabricator-To', $this->rawToPHIDs);
+    $mail->addPHIDHeaders('X-Phabricator-Cc', $this->rawCCPHIDs);
+
+    $to_handles = $viewer->loadHandles($this->rawToPHIDs);
+    $cc_handles = $viewer->loadHandles($this->rawCCPHIDs);
+
+    $body = $mail->getBody();
+    $body .= "\n";
+    $body .= $this->getRecipientsSummary($to_handles, $cc_handles);
+    $mail->setBody($body);
+
+    $html_body = $mail->getHTMLBody();
+    if (strlen($html_body)) {
+      $html_body .= hsprintf(
+        '%s',
+        $this->getRecipientsSummaryHTML($to_handles, $cc_handles));
+    }
+    $mail->setHTMLBody($html_body);
+
+    $reply_to = $this->getReplyTo();
+    if ($reply_to) {
+      $mail->setReplyTo($reply_to);
+    }
+
+    $to = array_keys($this->getToMap());
+    if ($to) {
+      $mail->addTos($to);
+    }
+
+    $cc = array_keys($this->getCCMap());
+    if ($cc) {
+      $mail->addCCs($cc);
+    }
+
+    return $mail->save();
+  }
+
+  private function getRecipientsSummary(
+    PhabricatorHandleList $to_handles,
+    PhabricatorHandleList $cc_handles) {
+
+    if (!PhabricatorEnv::getEnvConfig('metamta.recipients.show-hints')) {
+      return '';
+    }
+
+    $to_handles = iterator_to_array($to_handles);
+    $cc_handles = iterator_to_array($cc_handles);
+
+    $body = '';
+    if ($to_handles) {
+      $body .= "To: ".implode(', ', mpull($to_handles, 'getName'))."\n";
+    }
+    if ($cc_handles) {
+      $body .= "Cc: ".implode(', ', mpull($cc_handles, 'getName'))."\n";
+    }
+
+    return $body;
+  }
+
+  private function getRecipientsSummaryHTML(
+    PhabricatorHandleList $to_handles,
+    PhabricatorHandleList $cc_handles) {
+
+    if (!PhabricatorEnv::getEnvConfig('metamta.recipients.show-hints')) {
+      return '';
+    }
+
+    $to_handles = iterator_to_array($to_handles);
+    $cc_handles = iterator_to_array($cc_handles);
+
+    $body = array();
+    if ($to_handles) {
+      $body[] = phutil_tag('strong', array(), 'To: ');
+      $body[] = phutil_implode_html(', ', mpull($to_handles, 'getName'));
+      $body[] = phutil_tag('br');
+    }
+    if ($cc_handles) {
+      $body[] = phutil_tag('strong', array(), 'Cc: ');
+      $body[] = phutil_implode_html(', ', mpull($cc_handles, 'getName'));
+      $body[] = phutil_tag('br');
+    }
+    return phutil_tag('div', array(), $body);
+  }
+
+
+}

src/applications/owners/mail/OwnersPackageReplyHandler.php

@@ -11,7 +11,7 @@ final class OwnersPackageReplyHandler extends PhabricatorMailReplyHandler {
   }
 
   public function getPrivateReplyHandlerEmailAddress(
-    PhabricatorObjectHandle $handle) {
+    PhabricatorUser $user) {
     return null;
   }
 

src/applications/repository/mail/PhabricatorRepositoryPushReplyHandler.php

@@ -8,7 +8,7 @@ final class PhabricatorRepositoryPushReplyHandler
   }
 
   public function getPrivateReplyHandlerEmailAddress(
-    PhabricatorObjectHandle $handle) {
+    PhabricatorUser $user) {
     return null;
   }
 

src/applications/repository/worker/PhabricatorRepositoryPushMailWorker.php

@@ -27,6 +27,23 @@ final class PhabricatorRepositoryPushMailWorker
       return;
     }
 
+    $targets = id(new PhabricatorRepositoryPushReplyHandler())
+      ->setMailReceiver($repository)
+      ->getMailTargets($email_phids, array());
+    foreach ($targets as $target) {
+      $this->sendMail($target, $repository, $event);
+    }
+  }
+
+  private function sendMail(
+    PhabricatorMailTarget $target,
+    PhabricatorRepository $repository,
+    PhabricatorRepositoryPushEvent $event) {
+
+    $task_data = $this->getTaskData();
+    $viewer = $target->getViewer();
+    // TODO: Swap locale to viewer locale.
+
     $logs = $event->getLogs();
 
     list($ref_lines, $ref_list) = $this->renderRefs($logs);
@@ -103,20 +120,7 @@ final class PhabricatorRepositoryPushMailWorker
       ->addHeader('Thread-Topic', $subject)
       ->setIsBulk(true);
 
-    $to_handles = id(new PhabricatorHandleQuery())
+    $target->sendMail($mail);
-      ->setViewer($viewer)
-      ->withPHIDs($email_phids)
-      ->execute();
-
-    $reply_handler = new PhabricatorRepositoryPushReplyHandler();
-    $mails = $reply_handler->multiplexMail(
-      $mail,
-      $to_handles,
-      array());
-
-    foreach ($mails as $mail) {
-      $mail->saveAndSend();
-    }
   }
 
   public function renderForDisplay(PhabricatorUser $viewer) {

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -992,12 +992,10 @@ abstract class PhabricatorApplicationTransactionEditor
     // Hook for other edges that may need (re-)loading
     $object = $this->willPublish($object, $xactions);
 
-    $this->loadHandles($xactions);
+    $mailed = array();
-
-    $mail = null;
     if (!$this->getDisableEmail()) {
       if ($this->shouldSendMail($object, $xactions)) {
-        $mail = $this->sendMail($object, $xactions);
+        $mailed = $this->sendMail($object, $xactions);
       }
     }
 
@@ -1009,10 +1007,6 @@ abstract class PhabricatorApplicationTransactionEditor
     }
 
     if ($this->shouldPublishFeedStory($object, $xactions)) {
-      $mailed = array();
-      if ($mail) {
-        $mailed = $mail->buildRecipientList();
-      }
       $this->publishFeedStory(
         $object,
         $xactions,
@@ -2128,30 +2122,60 @@ abstract class PhabricatorApplicationTransactionEditor
     }
 
     if (!$any_visible) {
-      return;
+      return array();
     }
 
-    $email_force = array();
     $email_to = $this->mailToPHIDs;
     $email_cc = $this->mailCCPHIDs;
-
     $email_cc = array_merge($email_cc, $this->heraldEmailPHIDs);
-    $email_force = $this->heraldForcedEmailPHIDs;
 
-    $phids = array_merge($email_to, $email_cc);
+    $targets = $this->buildReplyHandler($object)
-    $handles = id(new PhabricatorHandleQuery())
+      ->getMailTargets($email_to, $email_cc);
-      ->setViewer($this->requireActor())
-      ->withPHIDs($phids)
-      ->execute();
 
-    $template = $this->buildMailTemplate($object);
+    // Set this explicitly before we start swapping out the effective actor.
+    $this->setActingAsPHID($this->getActingAsPHID());
+
+
+    $mailed = array();
+    foreach ($targets as $target) {
+      $original_actor = $this->getActor();
+      $this->setActor($target->getViewer());
+      // TODO: Swap locale to viewer locale.
+
+      $caught = null;
+      try {
+        // Reload handles for the new viewer.
+        $this->loadHandles($xactions);
+
+        $mail = $this->sendMailToTarget($object, $xactions, $target);
+      } catch (Exception $ex) {
+        $caught = $ex;
+      }
+
+      $this->setActor($original_actor);
+      if ($caught) {
+        throw $ex;
+      }
+
+      foreach ($mail->buildRecipientList() as $phid) {
+        $mailed[$phid] = true;
+      }
+    }
+
+    return array_keys($mailed);
+  }
+
+  private function sendMailToTarget(
+    PhabricatorLiskDAO $object,
+    array $xactions,
+    PhabricatorMailTarget $target) {
+
+    $mail = $this->buildMailTemplate($object);
     $body = $this->buildMailBody($object, $xactions);
 
     $mail_tags = $this->getMailTags($object, $xactions);
     $action = $this->getMailAction($object, $xactions);
 
-    $reply_handler = $this->buildReplyHandler($object);
-
     if (PhabricatorEnv::getEnvConfig('metamta.email-preferences')) {
       $this->addEmailPreferenceSectionToMailBody(
         $body,
@@ -2159,48 +2183,36 @@ abstract class PhabricatorApplicationTransactionEditor
         $xactions);
     }
 
-    $template
+    $mail
       ->setFrom($this->getActingAsPHID())
       ->setSubjectPrefix($this->getMailSubjectPrefix())
       ->setVarySubjectPrefix('['.$action.']')
       ->setThreadID($this->getMailThreadID($object), $this->getIsNewObject())
       ->setRelatedPHID($object->getPHID())
       ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
-      ->setForceHeraldMailRecipientPHIDs($email_force)
+      ->setForceHeraldMailRecipientPHIDs($this->heraldForcedEmailPHIDs)
       ->setMailTags($mail_tags)
       ->setIsBulk(true)
       ->setBody($body->render())
       ->setHTMLBody($body->renderHTML());
 
     foreach ($body->getAttachments() as $attachment) {
-      $template->addAttachment($attachment);
+      $mail->addAttachment($attachment);
     }
 
     if ($this->heraldHeader) {
-      $template->addHeader('X-Herald-Rules', $this->heraldHeader);
+      $mail->addHeader('X-Herald-Rules', $this->heraldHeader);
     }
 
     if ($object instanceof PhabricatorProjectInterface) {
-      $this->addMailProjectMetadata($object, $template);
+      $this->addMailProjectMetadata($object, $mail);
     }
 
     if ($this->getParentMessageID()) {
-      $template->setParentMessageID($this->getParentMessageID());
+      $mail->setParentMessageID($this->getParentMessageID());
     }
 
-    $mails = $reply_handler->multiplexMail(
+    return $target->sendMail($mail);
-      $template,
-      array_select_keys($handles, $email_to),
-      array_select_keys($handles, $email_cc));
-
-    foreach ($mails as $mail) {
-      $mail->saveAndSend();
-    }
-
-    $template->addTos($email_to);
-    $template->addCCs($email_cc);
-
-    return $template;
   }
 
   private function addMailProjectMetadata(

src/applications/transactions/replyhandler/PhabricatorApplicationTransactionReplyHandler.php

@@ -6,9 +6,9 @@ abstract class PhabricatorApplicationTransactionReplyHandler
   abstract public function getObjectPrefix();
 
   public function getPrivateReplyHandlerEmailAddress(
-    PhabricatorObjectHandle $handle) {
+    PhabricatorUser $user) {
     return $this->getDefaultPrivateReplyHandlerEmailAddress(
-      $handle,
+      $user,
       $this->getObjectPrefix());
   }