Carry "Message-ID" across email replies to prevent Gmail conversation splitting

Summary:
See T251. In Gmail, conversations split if you reply to them and the next email
does not "In-Reply-To" your message ID. When an action is triggered by an email,
carry its Message-ID through the stack and use it for "In-Reply-To" and
"References" on the subsequent message.

Test Plan:
Live-patched phabricator.com and replied to a Maniphest thread in Gmail without
disrupting the thread. Locally replied to Maniphest and Differential threads and
verified Message-ID was carried across the reply boundary.

Reviewed By: rm
Reviewers: tcook, jungejason, aran, tuomaspelkonen, rm
CC: aran, epriestley, rm
Differential Revision: 498

src/applications/differential/editor/comment/DifferentialCommentEditor.php

@@ -28,6 +28,8 @@ class DifferentialCommentEditor {
   protected $changedByCommit;
   protected $addedReviewers = array();
 
+  private $parentMessageID;
+
   public function __construct(
     DifferentialRevision $revision,
     $actor_phid,
@@ -38,6 +40,11 @@ class DifferentialCommentEditor {
     $this->action   = $action;
   }
 
+  public function setParentMessageID($parent_message_id) {
+    $this->parentMessageID = $parent_message_id;
+    return $this;
+  }
+
   public function setMessage($message) {
     $this->message = $message;
     return $this;
@@ -318,6 +325,7 @@ class DifferentialCommentEditor {
       ->setCCPHIDs($revision->getCCPHIDs())
       ->setChangedByCommit($this->getChangedByCommit())
       ->setXHeraldRulesHeader($xherald_header)
+      ->setParentMessageID($this->parentMessageID)
       ->send();
 
     $event_data = array(

src/applications/differential/mail/base/DifferentialMail.php

@@ -32,6 +32,7 @@ abstract class DifferentialMail {
   protected $heraldTranscriptURI;
   protected $heraldRulesHeader;
   protected $replyHandler;
+  protected $parentMessageID;
 
   abstract protected function renderSubject();
   abstract protected function renderBody();
@@ -53,6 +54,11 @@ abstract class DifferentialMail {
     return '???';
   }
 
+  public function setParentMessageID($parent_message_id) {
+    $this->parentMessageID = $parent_message_id;
+    return $this;
+  }
+
   public function setXHeraldRulesHeader($header) {
     $this->heraldRulesHeader = $header;
     return $this;
@@ -80,6 +86,7 @@ abstract class DifferentialMail {
       ->setSubject($subject)
       ->setBody($body)
       ->setIsHTML($this->shouldMarkMailAsHTML())
+      ->setParentMessageID($this->parentMessageID)
       ->addHeader('Thread-Topic', $this->getRevision()->getTitle());
 
     $template->setThreadID(

src/applications/differential/replyhandler/DifferentialReplyHandler.php

@@ -18,6 +18,8 @@
 
 class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
 
+  private $receivedMail;
+
   public function validateMailReceiver($mail_receiver) {
     if (!($mail_receiver instanceof DifferentialRevision)) {
       throw new Exception("Receiver is not a DifferentialRevision!");
@@ -94,6 +96,7 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
   }
 
   public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
+    $this->receivedMail = $mail;
     $this->handleAction($mail->getCleanTextBody());
   }
 
@@ -128,6 +131,12 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
         $actor->getPHID(),
         $command);
 
+      // NOTE: We have to be careful about this because Facebook's
+      // implementation jumps straight into handleAction() and will not have
+      // a PhabricatorMetaMTAReceivedMail object.
+      if ($this->receivedMail) {
+        $editor->setParentMessageID($this->receivedMail->getMessageID());
+      }
       $editor->setMessage($body);
       $editor->setAddCC(($command != DifferentialAction::ACTION_RESIGN));
       $comment = $editor->save();

src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php

@@ -18,6 +18,13 @@
 
 class ManiphestTransactionEditor {
 
+  private $parentMessageID;
+
+  public function setParentMessageID($parent_message_id) {
+    $this->parentMessageID = $parent_message_id;
+    return $this;
+  }
+
   public function applyTransactions($task, array $transactions) {
 
     $email_cc = $task->getCCPHIDs();
@@ -202,6 +209,7 @@ class ManiphestTransactionEditor {
     $template = id(new PhabricatorMetaMTAMail())
       ->setSubject($subject)
       ->setFrom($transaction->getAuthorPHID())
+      ->setParentMessageID($this->parentMessageID)
       ->addHeader('Thread-Topic', 'Maniphest Task '.$task->getID())
       ->setThreadID($thread_id, $is_create)
       ->setRelatedPHID($task->getPHID())

src/applications/maniphest/replyhandler/ManiphestReplyHandler.php

@@ -114,8 +114,8 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
     $xactions[] = $xaction;
 
     $editor = new ManiphestTransactionEditor();
+    $editor->setParentMessageID($mail->getMessageID());
     $editor->applyTransactions($task, $xactions);
-
   }
 
 }

src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php

@@ -62,6 +62,27 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
     return idx($this->parameters, $param);
   }
 
+  /**
+   * In Gmail, conversations will be broken if you reply to a thread and the
+   * server sends back a response without referencing your Message-ID, even if
+   * it references a Message-ID earlier in the thread. To avoid this, use the
+   * parent email's message ID explicitly if it's available. This overwrites the
+   * "In-Reply-To" and "References" headers we would otherwise generate. This
+   * needs to be set whenever an action is triggered by an email message. See
+   * T251 for more details.
+   *
+   * @param   string The "Message-ID" of the email which precedes this one.
+   * @return  this
+   */
+  public function setParentMessageID($id) {
+    $this->setParam('parent-message-id', $id);
+    return $this;
+  }
+
+  public function getParentMessageID() {
+    return $this->getParam('parent-message-id');
+  }
+
   public function getSubject() {
     return $this->getParam('subject');
   }
@@ -276,8 +297,13 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
             if ($is_first && $mailer->supportsMessageIDHeader()) {
               $mailer->addHeader('Message-ID',  $value);
             } else {
-              $mailer->addHeader('In-Reply-To', $value);
-              $mailer->addHeader('References',  $value);
+              $in_reply_to = $value;
+              $parent_id = $this->getParentMessageID();
+              if ($parent_id) {
+                $in_reply_to = $parent_id;
+              }
+              $mailer->addHeader('In-Reply-To', $in_reply_to);
+              $mailer->addHeader('References',  $in_reply_to);
             }
             $thread_index = $this->generateThreadIndex($value, $is_first);
             $mailer->addHeader('Thread-Index', $thread_index);

src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php

@@ -46,6 +46,10 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
     return $this;
   }
 
+  public function getMessageID() {
+    return idx($this->headers, 'message-id');
+  }
+
   public function processReceivedMail() {
     $to = idx($this->headers, 'to');