3 files changed, 184 insertions, 0 deletions

diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp
index d95d55d..49bb390 100644
--- a/tests/test_composewindow.cpp
+++ b/tests/test_composewindow.cpp
@@ -66,6 +66,8 @@ private slots:
     void theMenuBarReachesEveryComposerAction();
     void saveDraftWritesAndReports();
     void aSavedDraftIsFlaggedSeen();
+    void everyRevisionOfADraftKeepsOneMessageId();
+    void aSentMessageDoesNotInheritTheDraftsMessageId();
     void aForwardCarriesTheOriginalHtmlAndStripsRemoteContent();
     void anHtmlForwardPreviewsTheOriginalInsteadOfQuotingIt();
     void theMenusReuseTheToolbarActions();
@@ -752,6 +754,97 @@ void TestComposeWindow::saveDraftWritesAndReports()
     QVERIFY2(!window.isWindowModified(), "a manual save must clear the marker");
 }
 
+/// Item 165. Two saves of one draft must produce ONE message, not two.
+///
+/// Every save used to mint a fresh Message-ID, and mbsync uploads each
+/// revision to the drafts folder before the next save removes the local file,
+/// so the server kept one message per revision: measured on the user's own
+/// mail as four independent messages for a single reply, which then threaded
+/// into the conversation and put a `draft` tag on a Sent row. Deleting a local
+/// file does not retract an uploaded one, so the local cleanup, which is
+/// correct, could never fix this.
+///
+/// Asserted on the FILE's header rather than on a notmuch tag, for the reason
+/// the test below gives: the bytes are what the code here controls.
+void TestComposeWindow::everyRevisionOfADraftKeepsOneMessageId()
+{
+    const Config config = configWithDrafts();
+
+    ComposeContext context;
+    context.kind = ComposeContext::Kind::New;
+    context.accountKey = QStringLiteral("work");
+
+    ComposeWindow window(context, config, m_dir->path());
+    auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body"));
+    QVERIFY(body);
+    auto *save = window.findChild<QAction *>(QStringLiteral("compose_save"));
+    QVERIFY(save);
+
+    const auto messageIdIn = [](const QString &path) {
+        QFile file(path);
+        if (!file.open(QIODevice::ReadOnly))
+            return QString();
+        const QString text = QString::fromUtf8(file.readAll());
+        for (const QString &line : text.split(QLatin1Char('\n'))) {
+            if (line.startsWith(QStringLiteral("Message-Id:"), Qt::CaseInsensitive))
+                return line.section(QLatin1Char(':'), 1).trimmed();
+            if (line.trimmed().isEmpty())
+                break;  // end of headers
+        }
+        return QString();
+    };
+
+    QSignalSpy saved(&window, &ComposeWindow::draftSaved);
+
+    body->setPlainText(QStringLiteral("First revision."));
+    save->trigger();
+    QCOMPARE(saved.size(), 1);
+    const QString firstPath = saved.at(0).at(0).toString();
+    const QString firstId = messageIdIn(firstPath);
+    QVERIFY2(!firstId.isEmpty(), "the first revision carries no Message-ID");
+
+    // A REAL change, or the dirty check short-circuits and no second file is
+    // written at all, which would pass this test while proving nothing.
+    body->setPlainText(QStringLiteral("Second revision, genuinely different."));
+    save->trigger();
+    QCOMPARE(saved.size(), 2);
+    const QString secondPath = saved.at(1).at(0).toString();
+    QVERIFY2(secondPath != firstPath,
+             "the second save wrote no new file, so the ids cannot be compared");
+
+    QCOMPARE(messageIdIn(secondPath), firstId);
+}
+
+/// The constraint that makes item 165 safe, and the one a future edit is most
+/// likely to break: the SEND path must mint its own id.
+///
+/// The user's decision was a stable id while drafting, DISCARDED at send, so
+/// the sent copy is a different item from the draft. Two sent messages sharing
+/// a Message-ID would be far worse than the defect this fixed, and a
+/// Message-ID reaches the server and every recipient, so it is not a local
+/// matter. currentMessage() leaves the field empty and the send path passes it
+/// straight to build(); this pins that.
+void TestComposeWindow::aSentMessageDoesNotInheritTheDraftsMessageId()
+{
+    const Config config = configWithDrafts();
+
+    ComposeContext context;
+    context.kind = ComposeContext::Kind::Draft;
+    context.accountKey = QStringLiteral("work");
+    context.draftMessageId = QStringLiteral("the-draft-id@example.org");
+
+    ComposeWindow window(context, config, m_dir->path());
+    auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body"));
+    QVERIFY(body);
+    body->setPlainText(QStringLiteral("About to send."));
+
+    // The message the SEND path builds from, which is what decides the id.
+    const OutgoingMessage outgoing = window.currentMessage();
+    QVERIFY2(outgoing.messageId.isEmpty(),
+             "the send path carried the draft's Message-ID, so the sent copy "
+             "would share an id with a message already on the server");
+}
+
 /// A draft is authored by the user, so it is SEEN by definition and must never
 /// be tagged `unread`.
 ///
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 5817ac4..f20701e 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -518,6 +518,7 @@ private slots:
     void doubleClickingADraftOpensTheComposer();
     void aResumedDraftReplacesItsFileRatherThanAddingOne();
     void aResumedDraftKeepsItsBlindRecipients();
+    void aResumedDraftKeepsTheMessageIdItWasSavedUnder();
     void aDraftRenamedByASyncStillReopensAndReplacesItsFile();
     void theComposerSplitsItsToolbarByScope();
     void ccAndBccHideBehindADisclosure();
@@ -14313,6 +14314,53 @@ void TestMainWindow::aDraftRenamedByASyncStillReopensAndReplacesItsFile()
              "into two files and both would reach the server");
 }
 
+/// Item 165, the third link in the chain and the one a composer-only test
+/// cannot reach: reopening a draft must keep the identity the FILE already
+/// has, or the next autosave starts a second one and the server ends up with
+/// two messages for one draft after all.
+void TestMainWindow::aResumedDraftKeepsTheMessageIdItWasSavedUnder()
+{
+    ComposeFixture fixture;
+    QVERIFY(fixture.build());
+
+    OutgoingMessage message;
+    message.accountKey = QStringLiteral("acct");
+    message.to = { QStringLiteral("someone@example.org") };
+    message.subject = QStringLiteral("Resumed");
+    message.markdownBody = QStringLiteral("Body.");
+    message.messageId = QStringLiteral("already-saved-under@example.org");
+
+    const QString folder = fixture.mailRoot() + QStringLiteral("/acct/Drafts");
+    const QString path = writeDraftFile(folder, message,
+                                        fixture.config().account(
+                                            QStringLiteral("acct")));
+    QVERIFY(!path.isEmpty());
+
+    const ComposeContext context =
+        ComposeContextBuilder::forDraft(fixture.config(), path);
+    QCOMPARE(context.draftMessageId,
+             QStringLiteral("already-saved-under@example.org"));
+
+    // And it reaches the next revision, which is the property that matters:
+    // the context carrying it is only half the chain.
+    ComposeWindow window(context, fixture.config(), fixture.mailRoot());
+    auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body"));
+    QVERIFY(body);
+    body->setPlainText(QStringLiteral("Edited after reopening."));
+
+    QSignalSpy saved(&window, &ComposeWindow::draftSaved);
+    auto *save = window.findChild<QAction *>(QStringLiteral("compose_save"));
+    QVERIFY(save);
+    save->trigger();
+    QCOMPARE(saved.size(), 1);
+
+    QFile written(saved.first().first().toString());
+    QVERIFY(written.open(QIODevice::ReadOnly));
+    const QString text = QString::fromUtf8(written.readAll());
+    QVERIFY2(text.contains(QStringLiteral("<already-saved-under@example.org>")),
+             "the revision written after a reopen carries a different id");
+}
+
 void TestMainWindow::aResumedDraftKeepsItsBlindRecipients()
 {
     // MessageBuilder writes Bcc into the draft file deliberately, and says
diff --git a/tests/test_messagebuilder.cpp b/tests/test_messagebuilder.cpp
index 2ea14f0..56cf6a9 100644
--- a/tests/test_messagebuilder.cpp
+++ b/tests/test_messagebuilder.cpp
@@ -56,6 +56,8 @@ private slots:
     void aDirectoryAttachmentFailsRatherThanHangingTheProcess();
     void anUnparseableRecipientFailsRatherThanVanishing();
     void everyMessageCarriesADateAndMessageId();
+    void aSuppliedMessageIdIsUsedRatherThanAFreshOne();
+    void twoBuildsWithNoSuppliedIdStillDiffer();
     void aForwardSendsOnePartChosenByTheHtmlToggle();
     void recipientsAppearInTheirOwnHeaders();
     void anAccountWithNoAddressFailsRatherThanBuildingHeaderlessMail();
@@ -419,6 +421,47 @@ void TestMessageBuilder::everyMessageCarriesADateAndMessageId()
     QVERIFY(!r.messageId.isEmpty());
 }
 
+/// Item 165. A draft keeps ONE identity across its revisions, so an autosave
+/// replaces the message it wrote last time rather than adding another. Without
+/// this every save minted a new Message-ID, and mbsync uploaded each revision
+/// to the drafts folder before the next save removed the local file: measured
+/// on real mail as four distinct messages on the server for one reply.
+void TestMessageBuilder::aSuppliedMessageIdIsUsedRatherThanAFreshOne()
+{
+    OutgoingMessage message = baseMessage();
+    message.messageId = QStringLiteral("kept-across-revisions@example.org");
+
+    const MessageBuilder::Result r = MessageBuilder::build(message, m_account);
+    QVERIFY2(r.ok(), qPrintable(r.error));
+
+    QCOMPARE(r.messageId, QStringLiteral("kept-across-revisions@example.org"));
+    QVERIFY2(QString::fromUtf8(r.bytes).contains(
+                 QStringLiteral("<kept-across-revisions@example.org>")),
+             "the supplied id did not reach the headers");
+
+    // Twice, because the point is that a SECOND save keeps it. A test building
+    // once cannot tell a reused id from a freshly generated one.
+    const MessageBuilder::Result again = MessageBuilder::build(message, m_account);
+    QVERIFY2(again.ok(), qPrintable(again.error));
+    QCOMPARE(again.messageId, r.messageId);
+}
+
+/// The other half, and the constraint that makes the change safe: the SEND
+/// path supplies no id, and two sent messages must never share one. A field
+/// that defaulted to some fixed value would pass the test above and break
+/// this.
+void TestMessageBuilder::twoBuildsWithNoSuppliedIdStillDiffer()
+{
+    const MessageBuilder::Result first = MessageBuilder::build(baseMessage(), m_account);
+    const MessageBuilder::Result second = MessageBuilder::build(baseMessage(), m_account);
+    QVERIFY2(first.ok() && second.ok(), "a build failed");
+
+    QVERIFY(!first.messageId.isEmpty());
+    QVERIFY(!second.messageId.isEmpty());
+    QVERIFY2(first.messageId != second.messageId,
+             "two messages built with no supplied id share a Message-ID");
+}
+
 /// Bcc must be PRESENT in the bytes. The documented send command is `msmtp -t`,
 /// which reads its recipients FROM the headers and strips Bcc itself before
 /// transmission. Removing it here would mean blind recipients never receive the