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

This reverts commit a30b4c2. A backfill will stop arming commands on the
shared confirmation queue altogether, so matching confirmations by tags
is no longer needed, and its drop-ahead rule could discard a live
command when an unarmed write's tags matched a later one.

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

3 files changed, 15 insertions, 100 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 9b1b874..8b1c537 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -2889,23 +2889,6 @@ 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;
@@ -5322,18 +5305,9 @@ 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.
-    //
-    // 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;
-        }
+    if (!m_awaitingTagConfirmation.isEmpty()) {
+        TagCommand *command = m_awaitingTagConfirmation.takeFirst();
+        command->addChangedMessageIds(change.messageIds);
     }
 
     // Recorded here, where a write is CONFIRMED, rather than where one is sent:
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 94b129b..5c9e87b 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -167,12 +167,6 @@ 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(); }
@@ -1278,17 +1272,6 @@ 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)
@@ -2161,8 +2144,8 @@ private:
     /// first. See awaitTagConfirmation().
     ///
     /// Raw pointers, and the undo stack owns them. Cleared whenever the stack
-    /// 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
+    /// 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
     /// 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
@@ -2204,21 +2187,7 @@ private:
 class TagCommand : public QUndoCommand
 {
 public:
-    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());
-    }
+    using QUndoCommand::QUndoCommand;
 
     /// Records the messages whose tags the write really moved.
     ///
@@ -2240,9 +2209,6 @@ 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()
@@ -2269,8 +2235,8 @@ public:
     ThreadTagCommand(MainWindow *window, const QStringList &threadIds,
                      const QStringList &add, const QStringList &remove,
                      const QString &description)
-        : TagCommand(description, add, remove), m_window(window),
-          m_threadIds(threadIds), m_description(description) {}
+        : TagCommand(description), m_window(window), m_threadIds(threadIds),
+          m_add(add), m_remove(remove), 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.
@@ -2307,6 +2273,8 @@ public:
 private:
     MainWindow *m_window;
     QStringList m_threadIds;
+    QStringList m_add;
+    QStringList m_remove;
     QString m_description;
     bool m_firstRedo = true;
 };
@@ -2323,8 +2291,9 @@ public:
     MessageTagCommand(MainWindow *window, const QStringList &messageIds,
                       const QStringList &add, const QStringList &remove,
                       const QString &description)
-        : TagCommand(description, add, remove), m_window(window),
-          m_messageIds(messageIds), m_description(description) {}
+        : TagCommand(description), m_window(window),
+          m_messageIds(messageIds), m_add(add), m_remove(remove),
+          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.
@@ -2356,6 +2325,8 @@ 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 918fe00..4887647 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -299,7 +299,6 @@ private slots:
     void aWorkerBackedWindowReturnsRealThreads();
     void aWriteSaysItIsApplyingUntilTheWorkerHasRunIt();
     void applyingARuleConfirmsWithTheCountAndUndoesExactly();
-    void aSilentWriteDoesNotStealTheNextConfirmation();
     void aPurgeTakesTheRowsOutOfTheViewWithoutARefresh();
     void theDashboardFollowsAWriteToTheConversationItShows();
     void anEditHeldByASyncSaysSoInsteadOfClaimingItLanded();
@@ -10021,35 +10020,6 @@ 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".