fix: undo only what the write actually changed

Closes item 176. applyTags reports the messages whose tags really moved, and
a command stores that rather than what it asked for, so undoing a mark-read
no longer marks the whole conversation unread.

2 files changed, 153 insertions, 0 deletions

diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 2d15fe8..d406b18 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -478,6 +478,7 @@ private slots:
     void twoDeletesToOneTrashBothGetTheirTags();
     void deletingTwiceLeavesNoOriginTagBehind();
     void undoOfADeleteRemovesTheOriginTagToo();
+    void undoingAMarkReadRestoresOnlyWhatWasUnread();
     void deletingALoneMessageRemovesItFromTheInboxAndUndoReturnsIt();
     void deleteThreadMovesEveryMessageAndRepaintsTheRootCard();
     void aFolderNameWithASpaceSurvivesTheRoundTrip();
@@ -2080,6 +2081,19 @@ void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep()
     // recover on the first.
     QCOMPARE(window.undoDepthForTesting(), 1);
 
+    // The confirmation the real worker sends back, which since item 176 is
+    // what tells the command WHICH messages it moved. There is no worker in
+    // this window, so it is delivered by hand; without it the command knows
+    // of no change and correctly undoes nothing.
+    const TagChange confirmed{ { QStringLiteral("t1-first@example.org"),
+                                 QStringLiteral("t2-first@example.org"),
+                                 QStringLiteral("t3-first@example.org") },
+                               {},
+                               { QStringLiteral("unread") },
+                               QStringLiteral("Mark all read") };
+    QMetaObject::invokeMethod(&window, "onTagsApplied",
+                              Q_ARG(TagChange, confirmed));
+
     auto *undo = window.findChild<QAction *>(QStringLiteral("undo"));
     QVERIFY(undo);
     undo->trigger();
@@ -15405,4 +15419,78 @@ void TestMainWindow::aConversationLeavesWhenItsUnionEmpties()
     QCOMPARE(model->threadAt(0).threadId, QStringLiteral("t1"));
 }
 
+void TestMainWindow::undoingAMarkReadRestoresOnlyWhatWasUnread()
+{
+    // Item 176, end to end. The thread has messages in DISAGREEING states:
+    // two in the same state answer identically whichever way the code
+    // resolves them, so a test built on agreement passes against the bug.
+    //
+    // Measured on the user's real mail before the fix: a thread of 44 with 2
+    // unread was marked read, undone, and came back with 43 unread. Because
+    // maildir.synchronize_flags is on, that rewrote the files and would have
+    // reached the server.
+    WorkerBackedWindow backed;
+    QVERIFY(backed.fixture().addMessage(
+        QStringLiteral("acct/inbox"), QStringLiteral("ur0@example.org"),
+        QStringLiteral("UR root"), QStringLiteral("sender@example.org"),
+        QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+        QStringLiteral("Root body."), false));
+    QVERIFY(backed.fixture().addMessage(
+        QStringLiteral("acct/inbox"), QStringLiteral("ur1@example.org"),
+        QStringLiteral("Re: UR root"), QStringLiteral("other@example.org"),
+        QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"),
+        QStringLiteral("Already read."), false,
+        QStringLiteral("ur0@example.org")));
+    QVERIFY(backed.fixture().addMessage(
+        QStringLiteral("acct/inbox"), QStringLiteral("ur2@example.org"),
+        QStringLiteral("Re: UR root"), QStringLiteral("third@example.org"),
+        QStringLiteral("Fri, 14 Aug 2026 12:00:00 +0200"),
+        QStringLiteral("The only unread one."), true,
+        QStringLiteral("ur0@example.org")));
+    QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"),
+                          QStringLiteral("Trash")),
+             qPrintable(backed.error()));
+
+    MainWindow window(backed.config());
+    auto *model = window.findChild<ThreadListModel *>();
+    auto *view = window.findChild<ThreadListView *>();
+    auto *queryEdit =
+        window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+    QVERIFY(model && view && queryEdit);
+
+    const QString cfg = backed.fixture().configPath();
+    const QString thread = QStringLiteral("thread:{id:ur0@example.org}");
+
+    // A `thread:` query, not `tag:unread`: the row must survive the write for
+    // the undo to be driven through the interface at all.
+    queryEdit->setText(thread);
+    queryEdit->returnPressed();
+    QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+    QCOMPARE(notmuchCount(cfg, thread), 3);
+    QCOMPARE(notmuchCount(cfg, thread + QStringLiteral(" and tag:unread")), 1);
+
+    // A conversation row, so this is the thread-scoped write.
+    view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+    QVERIFY(model->isConversationRow(model->index(0, 0, QModelIndex())));
+    window.findChild<QAction *>(QStringLiteral("toggle_unread"))->trigger();
+
+    QTRY_VERIFY_WITH_TIMEOUT(
+        notmuchCount(cfg, thread + QStringLiteral(" and tag:unread")) == 0,
+        15000);
+
+    window.findChild<QAction *>(QStringLiteral("undo"))->trigger();
+
+    // ONE message unread again, the one that was. Before the fix this was 3.
+    QTRY_VERIFY_WITH_TIMEOUT(
+        notmuchCount(cfg, thread + QStringLiteral(" and tag:unread")) == 1,
+        15000);
+    QCOMPARE(notmuchCount(cfg, QStringLiteral("id:ur2@example.org and "
+                                              "tag:unread")),
+             1);
+    QCOMPARE(notmuchCount(cfg, QStringLiteral("id:ur0@example.org and "
+                                              "tag:unread")),
+             0);
+}
+
 #include "test_mainwindow.moc"
diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp
index 45dc9a0..f70a8f5 100644
--- a/tests/test_notmuchworker.cpp
+++ b/tests/test_notmuchworker.cpp
@@ -51,6 +51,8 @@ private slots:
     void applyTagsEmitsTheChange();
     void applyTagsIgnoresUnknownMessageIds();
     void applyTagsWithNoIdsDoesNothing();
+    void applyTagsReportsOnlyTheMessagesItChanged();
+    void applyTagsThatChangeNothingEmitNothing();
     void queryStillWorksAfterWrite();
     void aThreadCarriesItsCardMessagesOwnTags();
 
@@ -886,6 +888,69 @@ void TestNotmuchWorker::applyTagsWithNoIdsDoesNothing()
     QVERIFY(errors.isEmpty());
 }
 
+void TestNotmuchWorker::applyTagsReportsOnlyTheMessagesItChanged()
+{
+    // Item 176. Undo inverts the tags and keeps the scope, so the inverse of
+    // "remove unread from 44 messages" was "add unread to 44 messages",
+    // whether or not they carried it. Measured on real mail: a thread of 44
+    // with 2 unread came back with 43 unread.
+    NotmuchFixture fixture;
+    QVERIFY(fixture.addMessage(QStringLiteral("inbox"),
+                               QStringLiteral("u0@example.org"),
+                               QStringLiteral("Read one"),
+                               QStringLiteral("alice@example.org"),
+                               QStringLiteral("Mon, 24 Aug 2026 10:00:00 +0200"),
+                               QStringLiteral("Body."), false));
+    QVERIFY(fixture.addMessage(QStringLiteral("inbox"),
+                               QStringLiteral("u1@example.org"),
+                               QStringLiteral("Unread one"),
+                               QStringLiteral("bob@example.org"),
+                               QStringLiteral("Tue, 25 Aug 2026 10:00:00 +0200"),
+                               QStringLiteral("Body."), true));
+    QVERIFY(fixture.index());
+
+    NotmuchWorker worker(fixture.configPath());
+    QSignalSpy spy(&worker, &NotmuchWorker::tagsApplied);
+
+    worker.applyTags(TagChange{
+        { QStringLiteral("u0@example.org"), QStringLiteral("u1@example.org") },
+        {}, { QStringLiteral("unread") }, QStringLiteral("Mark read") });
+
+    QCOMPARE(spy.count(), 1);
+    const TagChange applied = spy.at(0).at(0).value<TagChange>();
+    QCOMPARE(applied.messageIds,
+             QStringList{ QStringLiteral("u1@example.org") });
+}
+
+void TestNotmuchWorker::applyTagsThatChangeNothingEmitNothing()
+{
+    // The companion to the case above: when the reduced list is EMPTY the
+    // write moved nothing, so there is nothing to record as pending, nothing
+    // to sync and nothing to undo. Emitting an empty change would push an
+    // undo entry whose inverse would then add a tag no message ever had,
+    // which is the same defect one step later.
+    NotmuchFixture fixture;
+    QVERIFY(fixture.addMessage(QStringLiteral("inbox"),
+                               QStringLiteral("n0@example.org"),
+                               QStringLiteral("Already read"),
+                               QStringLiteral("alice@example.org"),
+                               QStringLiteral("Mon, 24 Aug 2026 10:00:00 +0200"),
+                               QStringLiteral("Body."), false));
+    QVERIFY(fixture.index());
+
+    NotmuchWorker worker(fixture.configPath());
+    QSignalSpy spy(&worker, &NotmuchWorker::tagsApplied);
+    QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred);
+
+    worker.applyTags(TagChange{ { QStringLiteral("n0@example.org") },
+                                {},
+                                { QStringLiteral("unread") },
+                                QStringLiteral("Mark read") });
+
+    QVERIFY(spy.isEmpty());
+    QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString()));
+}
+
 void TestNotmuchWorker::queryStillWorksAfterWrite()
 {
     // applyTags closes the read-only handle to take the write lock. The same