fix: credit a tag confirmation to the command whose tags it names

onTagsApplied() handed each confirmation to the head of the waiting
queue. The worker stays silent for a write that moved nothing, so when
one operation of a rule found nothing to do (another writer got there
between the count and the write), the next operation's ids were
credited to it, and its undo inverted the wrong tag on the wrong
messages: item 176 through a race.

A confirmation now goes to the first waiting command whose add and
remove sets equal the change's, and the commands ahead of it are
dropped with no ids, so their undo stays a no-op. Equal-tag commands,
the two halves of a mixed selection, still match in send order. No
match credits nobody. TagCommand carries its own add and remove to make
the comparison.

Items 176 and 212.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

3 files changed, 100 insertions, 15 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 8b1c537..9b1b874 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -2889,6 +2889,23 @@ void MainWindow::applyRuleToExistingMail(const TagRule &rule)
         Q_ARG(quint64, m_pendingApplyGeneration));
 }
 
+QList<const TagCommand *> MainWindow::armTagCommandsForTesting(
+    const QList<QPair<QStringList, QStringList>> &ops)
+{
+    QList<const TagCommand *> commands;
+    m_undoStack.beginMacro(QStringLiteral("test"));
+    for (int i = 0; i < ops.size(); ++i) {
+        auto *command = new MessageTagCommand(this, {}, ops.at(i).first,
+                                              ops.at(i).second,
+                                              QStringLiteral("test"));
+        awaitTagConfirmation(command, i == 0);
+        m_undoStack.push(command);
+        commands.append(command);
+    }
+    m_undoStack.endMacro();
+    return commands;
+}
+
 void MainWindow::confirmRuleApply(const QVector<int> &counts)
 {
     const TagRule rule = m_pendingApplyRule;
@@ -5305,9 +5322,18 @@ void MainWindow::onTagsApplied(const TagChange &change)
     // instead turned the undo of "mark 44 read" into "mark 44 unread": 2
     // unread became 43, on real mail, and maildir.synchronize_flags carried it
     // into the filenames.
-    if (!m_awaitingTagConfirmation.isEmpty()) {
-        TagCommand *command = m_awaitingTagConfirmation.takeFirst();
-        command->addChangedMessageIds(change.messageIds);
+    //
+    // Matched on the tags, and the commands ahead of the match dropped: they
+    // were writes the worker found nothing to do for, and crediting them this
+    // write's ids would make their undo invert the wrong tag on the wrong
+    // messages. See awaitTagConfirmation().
+    for (int i = 0; i < m_awaitingTagConfirmation.size(); ++i) {
+        TagCommand *command = m_awaitingTagConfirmation.at(i);
+        if (command->confirmedBy(change)) {
+            m_awaitingTagConfirmation.remove(0, i + 1);
+            command->addChangedMessageIds(change.messageIds);
+            break;
+        }
     }
 
     // Recorded here, where a write is CONFIRMED, rather than where one is sent:
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 5c9e87b..94b129b 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -167,6 +167,12 @@ public:
     /// the worker resolves its query.
     void applyRuleToExistingMail(const TagRule &rule);
 
+    /// Arms one MessageTagCommand per (add, remove) pair inside one undo macro,
+    /// the way confirmRuleApply() does, without sending anything. Returns them,
+    /// owned by the undo stack, so a test can read what each was credited.
+    QList<const TagCommand *> armTagCommandsForTesting(
+        const QList<QPair<QStringList, QStringList>> &ops);
+
     /// Whether the undo stack still holds anything. Exposed so a test can show
     /// that a rejected write did not take unrelated history down with it.
     bool canUndo() const { return m_undoStack.canUndo(); }
@@ -1272,6 +1278,17 @@ private:
     /// case for every write that never goes on the stack (the automatic
     /// mark-read, a move's tags).
     ///
+    /// A confirmation goes to the first waiting command whose TAGS it names
+    /// (TagCommand::confirmedBy()), not blindly to the head of the queue, and
+    /// the commands ahead of that match are dropped. The worker stays silent
+    /// for a write that moved nothing, so a head-of-queue rule credited the
+    /// NEXT write's ids to the silent one: a rule applied one tag at a time
+    /// (item 212) whose `+x` found nothing to do took the `-y` confirmation,
+    /// and its undo stripped `x` from messages that only lost `y`, which is
+    /// item 176 through a race. A dropped command keeps no ids, so its undo is
+    /// the no-op it should be. Commands with equal tags (the two halves of a
+    /// mixed selection) still match in send order. No match credits nobody.
+    ///
     /// `first` says this command opens a new gesture, which discards anything
     /// a previous one stranded.
     void awaitTagConfirmation(TagCommand *command, bool first = true)
@@ -2144,8 +2161,8 @@ private:
     /// first. See awaitTagConfirmation().
     ///
     /// Raw pointers, and the undo stack owns them. Cleared whenever the stack
-    /// is, and drained in order by onTagsApplied(). An entry can be left
-    /// stranded by a write that never confirms (one held for a sync, or one
+    /// is, and drained by onTagsApplied(), matched on tags. An entry can be
+    /// left stranded by a write that never confirms (one held for a sync, or one
     /// the worker found nothing to change), so a gesture clears whatever is
     /// left before arming its own: crediting a fresh confirmation to a dead
     /// command would put the wrong ids on the wrong undo entry, which is the
@@ -2187,7 +2204,21 @@ private:
 class TagCommand : public QUndoCommand
 {
 public:
-    using QUndoCommand::QUndoCommand;
+    TagCommand(const QString &description, const QStringList &add,
+               const QStringList &remove)
+        : QUndoCommand(description), m_add(add), m_remove(remove) {}
+
+    /// Whether `change` is the confirmation of THIS command's write, judged
+    /// on the tags. As sets: the worker echoes the lists it was sent, and the
+    /// order of tags in one write means nothing.
+    bool confirmedBy(const TagChange &change) const
+    {
+        return QSet<QString>(m_add.cbegin(), m_add.cend())
+                   == QSet<QString>(change.added.cbegin(), change.added.cend())
+            && QSet<QString>(m_remove.cbegin(), m_remove.cend())
+                   == QSet<QString>(change.removed.cbegin(),
+                                    change.removed.cend());
+    }
 
     /// Records the messages whose tags the write really moved.
     ///
@@ -2209,6 +2240,9 @@ public:
     QStringList changedMessageIds() const { return m_changedMessageIds; }
 
 protected:
+    QStringList m_add;
+    QStringList m_remove;
+
     /// Cleared before each re-send, so a redo learns what IT moved rather than
     /// undoing the effect of the run before it.
     void forgetChangedMessageIds()
@@ -2235,8 +2269,8 @@ public:
     ThreadTagCommand(MainWindow *window, const QStringList &threadIds,
                      const QStringList &add, const QStringList &remove,
                      const QString &description)
-        : TagCommand(description), m_window(window), m_threadIds(threadIds),
-          m_add(add), m_remove(remove), m_description(description) {}
+        : TagCommand(description, add, remove), m_window(window),
+          m_threadIds(threadIds), m_description(description) {}
 
     /// The stack calls redo() when the command is pushed. The change has
     /// already been sent by that point, so the first call is skipped.
@@ -2273,8 +2307,6 @@ public:
 private:
     MainWindow *m_window;
     QStringList m_threadIds;
-    QStringList m_add;
-    QStringList m_remove;
     QString m_description;
     bool m_firstRedo = true;
 };
@@ -2291,9 +2323,8 @@ public:
     MessageTagCommand(MainWindow *window, const QStringList &messageIds,
                       const QStringList &add, const QStringList &remove,
                       const QString &description)
-        : TagCommand(description), m_window(window),
-          m_messageIds(messageIds), m_add(add), m_remove(remove),
-          m_description(description) {}
+        : TagCommand(description, add, remove), m_window(window),
+          m_messageIds(messageIds), m_description(description) {}
 
     /// The stack calls redo() when the command is pushed, by which point the
     /// change has already been sent, so the first call is skipped.
@@ -2325,8 +2356,6 @@ public:
 private:
     MainWindow *m_window;
     QStringList m_messageIds;
-    QStringList m_add;
-    QStringList m_remove;
     QString m_description;
     bool m_firstRedo = true;
 };
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 4887647..918fe00 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -299,6 +299,7 @@ private slots:
     void aWorkerBackedWindowReturnsRealThreads();
     void aWriteSaysItIsApplyingUntilTheWorkerHasRunIt();
     void applyingARuleConfirmsWithTheCountAndUndoesExactly();
+    void aSilentWriteDoesNotStealTheNextConfirmation();
     void aPurgeTakesTheRowsOutOfTheViewWithoutARefresh();
     void theDashboardFollowsAWriteToTheConversationItShows();
     void anEditHeldByASyncSaysSoInsteadOfClaimingItLanded();
@@ -10020,6 +10021,35 @@ void TestMainWindow::applyingARuleConfirmsWithTheCountAndUndoesExactly()
              QStringLiteral("1"));
 }
 
+void TestMainWindow::aSilentWriteDoesNotStealTheNextConfirmation()
+{
+    // Items 176 and 212. A rule is written one tag at a time, each operation
+    // its own armed command. If the first moves nothing (another writer got
+    // there between the count and the write), the worker stays silent for it,
+    // and the second operation's confirmation must not be credited to the
+    // first: its undo would then invert `+x` on messages that only lost `y`.
+    const Config config;
+    MainWindow window(config);
+
+    const QList<const TagCommand *> commands = window.armTagCommandsForTesting(
+        { { { QStringLiteral("x") }, {} },
+          { {}, { QStringLiteral("y") } } });
+    QCOMPARE(commands.size(), 2);
+
+    // Only the SECOND operation's confirmation arrives.
+    TagChange change;
+    change.messageIds = { QStringLiteral("moved@example.org") };
+    change.removed = { QStringLiteral("y") };
+    change.description = QStringLiteral("Apply rule");
+    QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied",
+                                      Q_ARG(TagChange, change)));
+
+    QVERIFY2(commands.at(0)->changedMessageIds().isEmpty(),
+             qPrintable(commands.at(0)->changedMessageIds().join(u',')));
+    QCOMPARE(commands.at(1)->changedMessageIds(),
+             QStringList{ QStringLiteral("moved@example.org") });
+}
+
 /// Item 181, from the user: "the thread dashboard doesn't update live with the
 /// modifications applied to the list pane. If I mark the thread as read, the
 /// dash still reports N unread".