fix(delete): repair seven defects in the move-to-trash path

Item 103's implementation was committed unreviewed and never hand-tested.
Reviewing it, and then hand-testing it against real mail, found seven
defects. Six of them lose or corrupt state and none was caught by the
suite, which was green throughout.

**Undo pushed a command instead of consuming one.** onMessagesMoved()
pushed a MoveCommand for every confirmed move, including the move an undo
had just made, so undoText went "Delete", "Undo Delete", "Undo Undo
Delete". A second press of undo re-deleted the message the first had
rescued. PendingMove carries a fromUndo flag, which has to survive the
queued round trip and so cannot be a window-wide "am I undoing" flag.

**Held moves were invisible to the quit guard.** pendingEditCount() summed
the held tag edits and not the held moves, so a Delete pressed during a
sync left the count at zero: the indicator stayed hidden and closeEvent()'s
guard never fired, discarding the move on quit with no prompt. That is item
106's data loss with a worse shape, because a dropped move leaves the file
in the folder the user asked it out of.

**Two moves to one folder dropped the second's tags.** m_pendingMoves was
keyed on the destination, so two Deletes in one account before the first
confirmation both named `acct/Trash` and the second insert overwrote the
first. That file reached the trash carrying neither `deleted` nor
`deleted-from:`, unrestorable and invisible to a `tag:deleted` query. It is
a FIFO now: the worker moves one batch at a time and emits in request
order, so position alone matches a confirmation to its request.

**Second Delete left the origin tag behind.** The restore passed the origin
PLACEHOLDER in its removal list, and onMessagesMoved() resolves that from
the folder the worker reports, which on a restore is the trash. It asked to
remove `deleted-from:Trash`, a tag never written, while the real
`deleted-from:inbox` was never named. A restore does not need the
placeholder: it already read the origin to decide where to send the file.
originTagFor() is now the one derivation both sides use.

**Ctrl+Z left it behind too**, for a different reason: MoveCommand was
constructed with the unresolved pending.add. The command carries the
resolved tags now, and is pushed per origin group rather than once per
batch, because the placeholder resolves to a different tag per origin.

**A thread root re-deleted itself.** everySelectedRowHasTag() asked a
thread row about its THREAD's tags, which notmuch gives as a union. Delete
the root of a three-message thread and the replies are untouched, so the
union carries no `deleted` and a second press ran Delete again: the message
moved trash-to-trash and came out with `deleted`, `deleted-from:inbox` AND
`deleted-from:Trash`, with no way back. The union was a documented
approximation, called bounded because the worst case for a TAG toggle was
re-applying a tag the message already had. A MOVE re-applies the move.

Resolved through messageById(), NOT through ThreadSummary::firstMessageTags,
which is the value the query delivered and is never refreshed by an
optimistic update: after a delete the node reads `deleted` while the
summary still reads `unread`.

**Delete thread never moved anything.** It was left calling tagSelected()
when Delete became a move, so a whole conversation sat in the inbox wearing
a `deleted` chip. It moves every message now, each with its own origin, so
a thread spanning folders reassembles on restore. A reply row resolves to
its own thread through selectedThreadIds(): scopeFor() reports a reply
under messageIds and leaves threadIds empty, which made a thread action on
a reply row do nothing at all.

**And the root card did not repaint** until it was clicked, while its
replies did. sendMove() had no optimistic update at all, so nothing moved
until the worker answered; and applyMessageTagChange() deliberately leaves
a multi-message thread's SUMMARY alone, which is correct for a one-message
edit and wrong for a thread-scoped one. The replies have nodes and
repainted; the root card reads the summary. The thread paths repaint
synchronously with applyTagChange() before the worker is asked, which also
keeps the toggle's direction readable for the next press.

Every fix carries a test and every test was mutation-checked. Three false
greens were found while writing them and are recorded at their assertions:
a disjunction that emptied on the wrong term, a QTRY_VERIFY(rowCount() == 0)
satisfied by the interval before the worker answers, and a query issued
before the confirming write had landed. Absence is asked of notmuch
directly through a new notmuchCount() helper for that reason.

Two bare-window tests moved off assertions about synchronous pending writes
onto the model, since the thread actions now round-trip through the worker.

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

1 files changed, 395 insertions, 59 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index d3f2dc7..361c0d4 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -214,7 +214,7 @@ void MainWindow::closeEvent(QCloseEvent *event)
             // Degrade to a warning rather than offering a sync that cannot run.
             const auto answer = QMessageBox::warning(
                 this, tr("Unsynced changes"),
-                tr("%n tag change(s) have not been synced, and no sync command "
+                tr("%n change(s) have not been synced, and no sync command "
                    "is configured. Quit anyway?", "", pendingEditCount()),
                 QMessageBox::Discard | QMessageBox::Cancel,
                 QMessageBox::Cancel);
@@ -228,7 +228,7 @@ void MainWindow::closeEvent(QCloseEvent *event)
             QMessageBox box(this);
             box.setIcon(QMessageBox::Question);
             box.setWindowTitle(tr("Unsynced changes"));
-            box.setText(tr("%n tag change(s) have not been synced.", "",
+            box.setText(tr("%n change(s) have not been synced.", "",
                            pendingEditCount()));
             box.setInformativeText(tr("Sync before quitting?"));
             QPushButton *sync =
@@ -933,12 +933,19 @@ void MainWindow::registerActions()
     });
     addAction(QStringLiteral("delete_thread"), tr("&Delete thread"),
               tr("Add or remove the deleted tag on whole threads"), [this]() {
+        // A MOVE now, like its message-scoped twin. It tagged and moved
+        // nothing until item 103's follow-up, so "Delete thread" left a whole
+        // conversation sitting in the inbox wearing a `deleted` chip: exactly
+        // the half-deleted state Delete stopped producing.
+        //
+        // The direction is read per MESSAGE, not from the thread's tag union.
+        // A thread whose root was deleted on its own carries `deleted` in the
+        // union while its replies do not, and asking the union there ran
+        // Delete a second time on messages already in the trash.
         if (everySelectedRowHasTag(QStringLiteral("deleted"), TagScope::Thread)) {
-            tagSelected({}, { QStringLiteral("deleted") },
-                        tr("Undelete thread"), TagScope::Thread);
+            restoreSelectedThreads();
         } else {
-            tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete thread"),
-                        TagScope::Thread);
+            trashSelectedThreads();
         }
     });
     addAction(QStringLiteral("spam_thread"), tr("Mark thread as &spam"),
@@ -1613,6 +1620,9 @@ void MainWindow::wireWorker()
     connect(m_worker, &NotmuchWorker::messagesMovedFrom,
             this, &MainWindow::onMessagesMoved);
 
+    connect(m_worker, &NotmuchWorker::threadMessagesResolved,
+            this, &MainWindow::onThreadMessagesResolved);
+
     m_workerThread.start();
 
     // Queued behind the thread start, so the completer has real tags as soon
@@ -2941,7 +2951,7 @@ void MainWindow::flushHeldEdits()
         m_heldMoves.clear();
         for (const HeldMove &move : moves) {
             sendMove(move.messageIds, move.destFolder, move.add, move.remove,
-                     move.description);
+                     move.description, move.fromUndo);
         }
         updatePendingIndicator();
     }
@@ -3756,7 +3766,15 @@ int MainWindow::pendingEditCount() const
     // Each held edit counts as one whatever its size, since it carries thread
     // ids rather than message ids and cannot be netted against the map.
     const int held = int(m_heldEdits.size());
-    return m_pendingTagEdits.size() + m_unnettablePendingEdits + held;
+    // Held MOVES count for exactly the same reason, and were missed. With no
+    // tag edit queued the count was 0, so the indicator stayed hidden and
+    // closeEvent()'s `pendingEditCount() > 0` guard never fired: a Delete
+    // pressed during a sync was discarded on quit with no prompt at all. That
+    // is item 106's data loss, and worse here, because a dropped move leaves
+    // the file in the folder the user asked it out of.
+    const int heldMoves = int(m_heldMoves.size());
+    return m_pendingTagEdits.size() + m_unnettablePendingEdits + held
+           + heldMoves;
 }
 
 void MainWindow::updatePendingIndicator()
@@ -3771,7 +3789,7 @@ void MainWindow::updatePendingIndicator()
     // they did, not the writes it became.
     m_pendingLabel->setText(tr("%n unsynced change(s)", "", pending));
     m_pendingLabel->setToolTip(
-        tr("Tag changes made here that a sync has not yet carried to the mail "
+        tr("Changes made here that a sync has not yet carried to the mail "
            "store. An external notmuch run can clear them without this count "
            "noticing."));
     m_pendingLabel->show();
@@ -3895,22 +3913,43 @@ bool MainWindow::everySelectedRowHasTag(const QString &tag,
         } else if (m_model->isMessageRow(index)) {
             tags = m_model->messageAt(index).tags;
         } else {
-            // The thread's summary, and this is a KNOWN approximation rather
-            // than an oversight. A thread row acts on the message its card
-            // displays, but that message's own tags are never in the model:
-            // setThreadMessages drops depth 0 because the root row stands for
-            // it, so there is no node to read and messageById() cannot find
-            // one. The summary is a union over the thread, so it answers
-            // "unread" while ANY message is.
+            // A thread row answers about the MESSAGE ITS CARD DISPLAYS, which
+            // is what it acts on. threadFor() already substitutes that
+            // message's own tags for the thread's union when they are known
+            // (item 110), so this reads the row's real state rather than a
+            // union over messages it does not stand for.
             //
-            // The consequence is bounded and only affects the DIRECTION a
-            // toggle picks, never what it writes: on a thread whose first
-            // message is read while a later one is not, Toggle unread reads
-            // the thread as unread and marks the first message read again, a
-            // no-op. Fixing it properly needs per-message state in
-            // ThreadSummary, which is the same thing item 87 needs; leave it
-            // for that item rather than guessing here.
-            tags = m_model->threadFor(index).tags;
+            // This used to read the union deliberately, with a comment
+            // calling the imprecision bounded because no per-message tags
+            // existed in the model. They do now: ThreadSummary carries
+            // firstMessageTags from the query, so an UNEXPANDED row already
+            // knows its own tags, and the comment outlived the fact.
+            //
+            // The cost of the union was not bounded once Delete became a
+            // MOVE. Deleting the root of a three-message thread left the two
+            // replies undeleted, so the union carried no `deleted`, so a
+            // second press read the row as not-deleted and deleted it AGAIN:
+            // the message was moved trash-to-trash and came out carrying
+            // `deleted`, `deleted-from:inbox` and `deleted-from:Trash` at
+            // once, with no way back. A tag toggle merely re-applied a tag it
+            // already had; a move re-applies the MOVE.
+            //
+            // Resolved through messageById() on the row's own message, which
+            // is the id messageScopeFor() will act on. Asking the same
+            // question the write asks is what keeps the direction and the
+            // write from disagreeing; the union answered a question about a
+            // conversation when the row stands for one message.
+            const ThreadSummary summary = m_model->threadFor(index);
+            const MessageNode own =
+                m_model->messageById(summary.firstMessageId);
+            // messageById() and NOT summary.firstMessageTags, which is the
+            // value the QUERY delivered and is not refreshed by an optimistic
+            // update: applyMessageTagChange() writes the row's node, so after
+            // a delete the node reads `deleted, deleted-from:inbox` while the
+            // summary still reads `inbox, unread`. Measured, and preferring
+            // the summary left this defect exactly as it was.
+            tags = own.messageId.isEmpty() ? summary.firstMessageTags
+                                           : own.tags;
         }
         if (!tags.contains(tag))
             return false;
@@ -4173,13 +4212,33 @@ void MainWindow::trashSelected()
     if (scope.messageIds.isEmpty())
         return;
 
+    QHash<QString, QString> pathById;
+    for (const QString &messageId : scope.messageIds)
+        pathById.insert(messageId, m_model->messageById(messageId).filePath);
+
+    trashMessages(scope.messageIds, pathById, scope.messageCount);
+}
+
+void MainWindow::trashMessages(const QStringList &messageIds,
+                               const QHash<QString, QString> &pathById,
+                               int messageCount,
+                               const QStringList &wholeThreadIds)
+{
+    if (messageIds.isEmpty())
+        return;
+
     // Grouped by destination, because moveMessages() takes one folder per call
     // and a selection can span accounts with different trash folders.
+    //
+    // Paths are passed IN rather than read from the model, because the thread
+    // path arrives with messages the model has never seen: a thread the user
+    // never expanded holds no node for its replies, so a lookup there returns
+    // nothing and every message resolves to no account.
     QHash<QString, QStringList> byTrash;
     QStringList unconfigured;
-    for (const QString &messageId : scope.messageIds) {
-        const QString path = m_model->messageById(messageId).filePath;
-        const Account account = accountForMessagePath(path);
+    for (const QString &messageId : messageIds) {
+        const Account account =
+            accountForMessagePath(pathById.value(messageId));
         if (account.trash.isEmpty()) {
             unconfigured.append(messageId);
             continue;
@@ -4204,11 +4263,199 @@ void MainWindow::trashSelected()
     for (auto it = byTrash.cbegin(); it != byTrash.cend(); ++it) {
         sendMove(it.value(), it.key(),
                  { QStringLiteral("deleted"), kOriginTagPlaceholder() }, {},
-                 tr("Delete"));
+                 tr("Delete"), false, wholeThreadIds);
     }
 
     showTransientStatus(
-        tr("%1: %n message(s)", "", scope.messageCount).arg(tr("Delete")));
+        tr("%1: %n message(s)", "", messageCount).arg(tr("Delete")));
+}
+
+QString MainWindow::originTagFor(const QString &dbRelativeFolder) const
+{
+    // `acct/inbox` becomes `deleted-from:inbox`. The tag stores the folder
+    // relative to the ACCOUNT, never to the database: the account prefix is
+    // recomposed from the message's own path when it is read back, so storing
+    // it would duplicate it and would go stale the day the user renames a
+    // maildir.
+    //
+    // Shared by the two sites that need the tag, rather than derived twice.
+    // They disagreed once already: onMessagesMoved() resolved a placeholder
+    // from the folder the worker reported, which on a RESTORE is the trash
+    // rather than the origin, so the restore stripped `deleted-from:Trash`
+    // and left the real tag in place.
+    const Account account =
+        accountForMessagePath(dbRelativeFolder + QLatin1Char('/'));
+    QString accountRelative = dbRelativeFolder;
+    if (!account.maildir.isEmpty()
+        && dbRelativeFolder.startsWith(account.maildir + QLatin1Char('/'))) {
+        accountRelative = dbRelativeFolder.mid(account.maildir.length() + 1);
+    }
+    if (accountRelative.isEmpty())
+        return QString();
+    return QStringLiteral("deleted-from:%1").arg(accountRelative);
+}
+
+QStringList MainWindow::selectedThreadIds() const
+{
+    // A THREAD action on a reply row means that reply's conversation.
+    //
+    // scopeFor() reports a reply under messageIds and leaves threadIds empty,
+    // which is right for the mixed selections it was built for and wrong as
+    // the only input to a thread-scoped action: the early return on an empty
+    // threadIds made Delete thread do nothing at all when the selected row
+    // happened to be a reply. threadFor() resolves either kind of row.
+    const QModelIndexList rows =
+        m_threadView->selectionModel()->selectedRows();
+    QStringList threadIds;
+    for (const QModelIndex &index : rows) {
+        const QString threadId = m_model->threadFor(index).threadId;
+        if (!threadId.isEmpty() && !threadIds.contains(threadId))
+            threadIds.append(threadId);
+    }
+    return threadIds;
+}
+
+void MainWindow::trashSelectedThreads()
+{
+    const QModelIndexList rows =
+        m_threadView->selectionModel()->selectedRows();
+    if (rows.isEmpty())
+        return;
+
+    const QStringList threadIds = selectedThreadIds();
+    if (threadIds.isEmpty())
+        return;
+
+    // Asked of the WORKER rather than resolved here. A thread the user never
+    // expanded has no nodes in the model for its replies, so the ids and the
+    // paths a move needs exist only in the database. applyTagsToThreads()
+    // solves the same problem the same way, for the same reason.
+    //
+    // Repainted HERE, synchronously, before the worker is asked.
+    //
+    // The move needs message ids and paths that only the database holds for an
+    // unexpanded thread, so the move itself is asynchronous. The DISPLAY must
+    // not wait for that round trip: the card is what the user watches, and
+    // holding it back is what made a deleted thread sit unchanged until it was
+    // clicked. It also keeps the toggle's direction readable immediately, so a
+    // second press restores rather than deleting again.
+    for (const QString &threadId : threadIds)
+        m_model->applyTagChange(threadId, { QStringLiteral("deleted") }, {});
+
+    m_pendingThreadScope = threadIds;
+    QMetaObject::invokeMethod(m_worker, "resolveThreadMessages",
+                              Qt::QueuedConnection,
+                              Q_ARG(QStringList, threadIds),
+                              Q_ARG(QString, QStringLiteral("delete_thread")));
+}
+
+void MainWindow::onThreadMessagesResolved(const QStringList &messageIds,
+                                          const QStringList &paths,
+                                          const QStringList &tags,
+                                          const QString &requestTag)
+{
+    if (messageIds.size() != paths.size() || messageIds.size() != tags.size())
+        return;
+
+    QHash<QString, QString> pathById;
+    for (int i = 0; i < messageIds.size(); ++i)
+        pathById.insert(messageIds.at(i), paths.at(i));
+
+    const QStringList threadScope = m_pendingThreadScope;
+    m_pendingThreadScope.clear();
+
+    if (requestTag == QStringLiteral("delete_thread")) {
+        trashMessages(messageIds, pathById, messageIds.size(), threadScope);
+        return;
+    }
+
+    if (requestTag != QStringLiteral("undelete_thread"))
+        return;
+
+    // Restore, resolved per message: each one goes back to the folder its own
+    // `deleted-from:` tag names, so a thread whose messages were deleted from
+    // different folders reassembles correctly rather than collapsing into one.
+    const QString prefix = QStringLiteral("deleted-from:");
+    QHash<QString, QStringList> byOrigin;
+    QStringList unknown;
+    for (int i = 0; i < messageIds.size(); ++i) {
+        // Split on TAB, matching resolveThreadMessages(). A space is not a
+        // safe separator: a folder name containing one produces a tag
+        // containing one, and splitting there silently truncates the origin
+        // to its first word.
+        const QStringList messageTags =
+            tags.at(i).split(QLatin1Char('\t'), Qt::SkipEmptyParts);
+        QString origin;
+        for (const QString &tag : messageTags) {
+            if (tag.startsWith(prefix)) {
+                origin = tag.mid(prefix.length());
+                break;
+            }
+        }
+        // A message with no `deleted` tag is not in the trash and has nothing
+        // to come back from. A thread-scoped restore reaches every message,
+        // including ones the user never deleted, and moving those would drag
+        // untouched mail out of whatever folder it legitimately sits in.
+        if (!messageTags.contains(QStringLiteral("deleted")))
+            continue;
+        const Account account =
+            accountForMessagePath(paths.at(i));
+        if (origin.isEmpty() || account.maildir.isEmpty()) {
+            unknown.append(messageIds.at(i));
+            continue;
+        }
+        byOrigin[account.maildir + QLatin1Char('/') + origin]
+            .append(messageIds.at(i));
+    }
+
+    if (!unknown.isEmpty()) {
+        // No origin recorded: deleted by an older version or tagged by hand.
+        // The tag comes off so the row stops claiming to be deleted, but no
+        // file moves, since guessing a folder would put the message somewhere
+        // the user never had it.
+        sendMessageTagChange(unknown, {}, { QStringLiteral("deleted") },
+                             tr("Undelete thread"));
+        m_undoStack.push(new MessageTagCommand(this, unknown, {},
+                                               { QStringLiteral("deleted") },
+                                               tr("Undelete thread")));
+    }
+
+    for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) {
+        // The origin tag is named here, not left as the placeholder: on a
+        // restore the placeholder would resolve to the folder the message is
+        // coming FROM, which is the trash, and strip a tag never written.
+        const QString origin = originTagFor(it.key());
+        QStringList remove{ QStringLiteral("deleted") };
+        if (!origin.isEmpty())
+            remove.append(origin);
+        sendMove(it.value(), it.key(), {}, remove, tr("Undelete thread"),
+                 false, threadScope);
+    }
+
+    showTransientStatus(tr("%1: %n message(s)", "", messageIds.size())
+                            .arg(tr("Undelete thread")));
+}
+
+void MainWindow::restoreSelectedThreads()
+{
+    const QModelIndexList rows =
+        m_threadView->selectionModel()->selectedRows();
+    if (rows.isEmpty())
+        return;
+
+    const QStringList threadIds = selectedThreadIds();
+    if (threadIds.isEmpty())
+        return;
+
+    // Repainted synchronously, as the delete direction is.
+    for (const QString &threadId : threadIds)
+        m_model->applyTagChange(threadId, {}, { QStringLiteral("deleted") });
+
+    m_pendingThreadScope = threadIds;
+    QMetaObject::invokeMethod(
+        m_worker, "resolveThreadMessages", Qt::QueuedConnection,
+        Q_ARG(QStringList, threadIds),
+        Q_ARG(QString, QStringLiteral("undelete_thread")));
 }
 
 void MainWindow::restoreSelected()
@@ -4260,9 +4507,26 @@ void MainWindow::restoreSelected()
     }
 
     for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) {
-        sendMove(it.value(), it.key(), {},
-                 { QStringLiteral("deleted"), kOriginTagPlaceholder() },
-                 tr("Undelete"));
+        // The origin tag is named HERE, not left as the placeholder.
+        //
+        // onMessagesMoved() resolves the placeholder from the origin the
+        // WORKER reports, which is where the message is coming FROM. On a
+        // delete that is the inbox and correct; on a restore it is the trash,
+        // so the placeholder resolved to `deleted-from:Trash` and asked to
+        // remove a tag that never existed, while the real `deleted-from:inbox`
+        // was never named. The message came home still claiming to have been
+        // deleted from somewhere, which then made Restore offer to move a
+        // message that was already back.
+        //
+        // A restore does not need the placeholder at all: the origin was just
+        // read off the message's own tag to decide where to send it, so the
+        // exact tag to strip is already known. Recomposed from the same
+        // account-relative form it was stored in.
+        const QString origin = originTagFor(it.key());
+        QStringList remove{ QStringLiteral("deleted") };
+        if (!origin.isEmpty())
+            remove.append(origin);
+        sendMove(it.value(), it.key(), {}, remove, tr("Undelete"));
     }
 
     showTransientStatus(
@@ -4272,7 +4536,8 @@ void MainWindow::restoreSelected()
 void MainWindow::sendMove(const QStringList &messageIds,
                           const QString &destFolder, const QStringList &add,
                           const QStringList &remove,
-                          const QString &description)
+                          const QString &description, bool fromUndo,
+                          const QStringList &wholeThreadIds)
 {
     if (messageIds.isEmpty() || destFolder.isEmpty())
         return;
@@ -4286,8 +4551,8 @@ void MainWindow::sendMove(const QStringList &messageIds,
     // would apply the tags and never move the file, which is worse than
     // waiting: the message would read as deleted and still be in the inbox.
     if (aSyncHoldsTheWriteLock()) {
-        m_heldMoves.append(
-            HeldMove{ messageIds, destFolder, add, remove, description });
+        m_heldMoves.append(HeldMove{ messageIds, destFolder, add, remove,
+                                     description, fromUndo });
         m_statusLabel->setText(
             tr("A sync is running; your change will be applied when it "
                "finishes."));
@@ -4295,9 +4560,61 @@ void MainWindow::sendMove(const QStringList &messageIds,
         return;
     }
 
+    // Repainted NOW, before the worker is asked.
+    //
+    // The write itself waits for the move to be confirmed, and must: tagging
+    // the database first would leave a message marked deleted in a folder it
+    // never left if the rename failed. The DISPLAY has no such constraint, and
+    // holding it back until the round trip finished is what made a deleted row
+    // sit there unchanged until the user clicked it. The reply rows repainted
+    // and the root did not, because the replies were separately tagged while
+    // the root's card reads its thread's summary.
+    //
+    // Reverted by revertPendingTagChange() if the write is rejected, exactly
+    // as the tag path's optimistic update is.
+    //
+    // The placeholder is dropped rather than displayed: the real origin is not
+    // known until the worker answers, and a chip reading the placeholder's
+    // literal name would be worse than one chip arriving a moment late.
+    QStringList displayAdd;
+    for (const QString &tag : add) {
+        if (tag != kOriginTagPlaceholder())
+            displayAdd.append(tag);
+    }
+    QStringList displayRemove;
+    for (const QString &tag : remove) {
+        if (tag != kOriginTagPlaceholder())
+            displayRemove.append(tag);
+    }
+    // A thread-scoped move already repainted its rows in
+    // trashSelectedThreads() / restoreSelectedThreads(), synchronously, before
+    // the worker was asked to resolve the threads at all. Repeating it here
+    // would be harmless but redundant; more importantly the caller there needs
+    // the repaint to happen WITHOUT a worker round trip, which is the whole
+    // reason it is not done from this function.
+    //
+    // applyTagChange() is what those callers use, and applyMessageTagChange()
+    // is what this one uses, and the difference is not a style choice: the
+    // former moves the thread's SUMMARY, which a thread row's card draws from,
+    // while the latter deliberately leaves a multi-message thread's summary
+    // alone because one message's edit does not describe the conversation.
+    if (wholeThreadIds.isEmpty()) {
+        for (const QString &messageId : messageIds)
+            m_model->applyMessageTagChange(messageId, displayAdd, displayRemove);
+    }
+
     // What to tag once the move is CONFIRMED. Tagging now would leave a
     // message marked deleted in a folder it never left if the rename failed.
-    m_pendingMoves.insert(destFolder, PendingMove{ add, remove, description });
+    //
+    // A QUEUE, not a map keyed on the destination: two Deletes in the same
+    // account before the first confirmation arrives both name `acct/Trash`,
+    // so the second insert overwrote the first and the second confirmation
+    // took an empty PendingMove. That file landed in the trash carrying
+    // neither `deleted` nor `deleted-from:`, which makes it unrestorable and
+    // invisible to a `tag:deleted` query. The worker handles one move at a
+    // time on its own thread and emits in the order it was asked, so a plain
+    // FIFO matches confirmations to requests without needing a key at all.
+    m_pendingMoves.enqueue(PendingMove{ add, remove, description, fromUndo });
 
     QMetaObject::invokeMethod(m_worker, "moveMessages", Qt::QueuedConnection,
                               Q_ARG(QStringList, messageIds),
@@ -4307,7 +4624,9 @@ void MainWindow::sendMove(const QStringList &messageIds,
 void MainWindow::onMessagesMoved(const QMap<QString, QString> &originByMessageId,
                                  const QString &destFolder)
 {
-    const PendingMove pending = m_pendingMoves.take(destFolder);
+    if (m_pendingMoves.isEmpty())
+        return;
+    const PendingMove pending = m_pendingMoves.dequeue();
     if (originByMessageId.isEmpty())
         return;
 
@@ -4332,16 +4651,7 @@ void MainWindow::onMessagesMoved(const QMap<QString, QString> &originByMessageId
         // `acct`, so the stored tag is `inbox`. Resolved through the first
         // message's path, which is still the account's whichever folder it
         // sits in now.
-        const QString dbRelativeOrigin = it.key();
-        const Account account = accountForMessagePath(dbRelativeOrigin
-                                                      + QLatin1Char('/'));
-        QString accountRelative = dbRelativeOrigin;
-        if (!account.maildir.isEmpty()
-            && dbRelativeOrigin.startsWith(account.maildir
-                                           + QLatin1Char('/'))) {
-            accountRelative =
-                dbRelativeOrigin.mid(account.maildir.length() + 1);
-        }
+        const QString originTag = originTagFor(it.key());
 
         auto resolve = [&](const QStringList &tags) {
             QStringList out;
@@ -4350,24 +4660,50 @@ void MainWindow::onMessagesMoved(const QMap<QString, QString> &originByMessageId
                     out.append(tag);
                     continue;
                 }
-                if (!accountRelative.isEmpty()) {
-                    out.append(QStringLiteral("deleted-from:%1")
-                                   .arg(accountRelative));
-                }
+                if (!originTag.isEmpty())
+                    out.append(originTag);
             }
             return out;
         };
 
-        sendMessageTagChange(it.value(), resolve(pending.add),
-                             resolve(pending.remove), pending.description);
+        const QStringList resolvedAdd = resolve(pending.add);
+        const QStringList resolvedRemove = resolve(pending.remove);
+        sendMessageTagChange(it.value(), resolvedAdd, resolvedRemove,
+                             pending.description);
+
+        // The undo entry carries the RESOLVED tags, and is pushed per origin
+        // group rather than once for the batch.
+        //
+        // It used to be handed pending.add straight, which still holds the
+        // unresolved placeholder: undo then asked to remove a tag by that
+        // literal name, which no message carries, so the removal was a silent
+        // no-op and `deleted-from:inbox` survived the undo. The file came home
+        // still claiming to have been deleted from somewhere. Same defect as
+        // the one the second-Delete path had, reached through Ctrl+Z instead.
+        //
+        // Per group because the placeholder resolves to a DIFFERENT tag per
+        // origin: one command for a batch spanning two folders could only
+        // carry one of them, so the other would be the wrong tag rather than
+        // merely an unresolved one.
+        if (!pending.fromUndo) {
+            QMap<QString, QString> groupOrigins;
+            for (const QString &messageId : it.value())
+                groupOrigins.insert(messageId, originByMessageId.value(messageId));
+            m_undoStack.push(new MoveCommand(this, groupOrigins, destFolder,
+                                             resolvedAdd, resolvedRemove,
+                                             pending.description));
+        }
     }
 
-    // Pushed only now, because the origins are what makes the command
-    // reversible and they do not exist until the worker reports them. See
-    // MoveCommand: the destination has to be carried rather than derived.
-    m_undoStack.push(new MoveCommand(this, originByMessageId, destFolder,
-                                     pending.add, pending.remove,
-                                     pending.description));
+    // The undo entries are pushed inside the loop above, one per origin
+    // group, because the placeholder resolves per origin. Nothing is pushed
+    // for a move the undo stack itself started: a MoveCommand is confirmed
+    // through this same slot, so pushing unconditionally left the undo of a
+    // Delete putting a fresh command on the stack instead of consuming the
+    // one it undid, and a second press of undo re-deleted the message. The
+    // flag rides on PendingMove because the answer has to survive the queued
+    // round trip; a window-wide "am I undoing" flag would long since have
+    // been cleared by the time the worker replies.
 }
 
 void MainWindow::sendThreadTagChange(const QStringList &threadIds,