summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-18 12:13:51 +0200
committerDanilo M. <danix@danix.xyz>2026-08-18 12:13:51 +0200
commit4583de009571aaa674e7d161d31ec640860787e1 (patch)
tree149f880e57dd1259f4b3ed6f98c8f3c97a688381
parentec8c2d7492b479846270f702541a26be5643a376 (diff)
downloadqtmaildir-4583de009571aaa674e7d161d31ec640860787e1.tar.gz
qtmaildir-4583de009571aaa674e7d161d31ec640860787e1.zip
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>
-rw-r--r--src/mainwindow.cpp454
-rw-r--r--src/mainwindow.h85
-rw-r--r--tests/test_mainwindow.cpp779
-rw-r--r--translations/qtmaildir_it_IT.ts40
4 files changed, 1258 insertions, 100 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,
diff --git a/src/mainwindow.h b/src/mainwindow.h
index ada4845..ae87868 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -21,6 +21,7 @@
#include <QHash>
#include <QSet>
#include <QMainWindow>
+#include <QQueue>
#include <QPointer>
#include <QThread>
#include <QUndoCommand>
@@ -754,18 +755,69 @@ private:
/// onMessagesMoved() replaces with `deleted-from:<origin>` per message.
/// The origin is not known until the worker reports it, and it differs per
/// message in a multi-row selection.
+ /// `fromUndo` marks a move the undo stack itself started, which must NOT
+ /// push a command of its own when it is confirmed. See onMessagesMoved().
+ /// `wholeThreadIds`, when non-empty, says this move covers every message
+ /// of those threads, so the optimistic repaint updates each thread's
+ /// SUMMARY rather than each message's node. A thread row's card reads the
+ /// summary, so a thread-scoped move that updated only nodes repainted the
+ /// replies and left the root card stale until the next query.
void sendMove(const QStringList &messageIds, const QString &destFolder,
const QStringList &add, const QStringList &remove,
- const QString &description);
+ const QString &description, bool fromUndo = false,
+ const QStringList &wholeThreadIds = {});
/// Moves each selected row's message to its account's trash, tagging it
/// `deleted` and recording where it came from.
void trashSelected();
+ /// The half of trashSelected() that does the work, given the messages and
+ /// their paths.
+ ///
+ /// Paths are passed in rather than looked up, because the thread-scoped
+ /// caller has messages the MODEL has never seen: an unexpanded thread
+ /// holds no node for its replies, so a model lookup resolves them to no
+ /// account and the move is silently dropped. The worker supplies them.
+ void trashMessages(const QStringList &messageIds,
+ const QHash<QString, QString> &pathById,
+ int messageCount,
+ const QStringList &wholeThreadIds = {});
+
+ /// Moves every message of each selected THREAD to its account's trash.
+ ///
+ /// Asynchronous, unlike its message-scoped twin: the ids and paths of an
+ /// unexpanded thread's messages live only in the database, so this asks
+ /// the worker and finishes in onThreadMessagesResolved().
+ void trashSelectedThreads();
+
+ /// The thread ids the selection covers, resolving a reply row to its own
+ /// thread. scopeFor() reports a reply under messageIds instead, which left
+ /// a thread action on a reply row doing nothing at all.
+ QStringList selectedThreadIds() const;
+
+ /// The inverse of trashSelectedThreads(): moves every message of each
+ /// selected thread back where it came from.
+ void restoreSelectedThreads();
+
+ /// Runs the thread-scoped delete once the worker has resolved the
+ /// threads to messages.
+ void onThreadMessagesResolved(const QStringList &messageIds,
+ const QStringList &paths,
+ const QStringList &tags,
+ const QString &requestTag);
+
/// The inverse: moves each selected row's message back to the folder its
/// `deleted-from:` tag names, stripping both tags.
void restoreSelected();
+ /// The `deleted-from:` tag naming `dbRelativeFolder`, or empty when no
+ /// account owns it.
+ ///
+ /// One rule for both sites that need the tag: the delete that writes it
+ /// and the restore that strips it. Deriving it twice let them disagree,
+ /// and a restore stripped a tag that had never been written.
+ QString originTagFor(const QString &dbRelativeFolder) const;
+
/// The account whose maildir contains `path`, or an invalid account when
/// no configured maildir does.
///
@@ -781,13 +833,27 @@ private:
const QString &destFolder);
/// What a move asked to be tagged, held until the worker confirms it.
- /// Keyed by destination folder so two moves in flight cannot be confused.
+ ///
+ /// A FIFO and not a map keyed on the destination: two Deletes in one
+ /// account before the first confirmation arrives name the same folder, so
+ /// a keyed map dropped the first entry and left the second confirmation
+ /// with nothing to apply. That file reached the trash carrying neither
+ /// `deleted` nor `deleted-from:`, unrestorable and invisible to a
+ /// `tag:deleted` query. The worker moves one batch at a time and emits in
+ /// request order, so position alone matches a confirmation to its request.
struct PendingMove {
QStringList add;
QStringList remove;
QString description;
+ /// Set for a move the undo stack started, which must not push again.
+ bool fromUndo = false;
};
- QHash<QString, PendingMove> m_pendingMoves;
+ QQueue<PendingMove> m_pendingMoves;
+
+ /// The threads a resolveThreadMessages() request was made for, held until
+ /// the answer arrives so the optimistic repaint knows the move is
+ /// thread-scoped.
+ QStringList m_pendingThreadScope;
/// Undoes the optimistic model update for a write the worker rejected.
void revertPendingTagChange();
@@ -856,6 +922,9 @@ private:
QStringList add;
QStringList remove;
QString description;
+ /// Carried through the hold, or a move undone during a sync would
+ /// push a command when it is finally flushed.
+ bool fromUndo = false;
};
QVector<HeldMove> m_heldMoves;
@@ -1302,8 +1371,10 @@ public:
m_firstRedo = false;
return;
}
+ // Also fromUndo: a redo replays a command that is ALREADY on the
+ // stack, so confirming it must not push a duplicate either.
m_window->sendMove(m_origins.keys(), m_dest, m_add, m_remove,
- m_description);
+ m_description, true);
}
void undo() override
@@ -1317,8 +1388,12 @@ public:
byOrigin[it.value()].append(it.key());
}
for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) {
+ // fromUndo: this move is the undo, so its confirmation must not
+ // push a command of its own. Without it the stack grew on every
+ // press and a second undo re-deleted the message.
m_window->sendMove(it.value(), it.key(), m_remove, m_add,
- QStringLiteral("Undo %1").arg(m_description));
+ QStringLiteral("Undo %1").arg(m_description),
+ true);
}
}
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index f4ad6a8..a17eba1 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -16,6 +16,7 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
+#include <QProcess>
#include <QtTest>
#include <QAction>
@@ -363,6 +364,14 @@ private slots:
void undoMovesTheMessageBack();
void deleteOnAReplyMovesThatReplyOnly();
void deleteWithoutATrashFolderSaysSoRatherThanDoingNothing();
+ void undoingADeleteConsumesItsCommandRatherThanPushingAnother();
+ void aDeleteHeldDuringASyncCountsAsUnsyncedWork();
+ void twoDeletesToOneTrashBothGetTheirTags();
+ void deletingTwiceLeavesNoOriginTagBehind();
+ void undoOfADeleteRemovesTheOriginTagToo();
+ void deletingAThreadRootTwiceRestoresItRatherThanRedeleting();
+ void deleteThreadMovesEveryMessageAndRepaintsTheRootCard();
+ void aFolderNameWithASpaceSurvivesTheRoundTrip();
private:
/// Owns the throwaway lock table init() points every test at. A pointer
@@ -4782,15 +4791,27 @@ void TestMainWindow::deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList()
"the fixture did not produce a reply row at row 0, so this test "
"would assert nothing about item 88's trap");
+ // t2 is the reply's thread and is NOT deleted, so the correct direction
+ // is Delete. Reading t1's state instead would choose Undelete.
+ QVERIFY2(!model->threadAt(1).isDeleted(),
+ "the fixture's second thread is already deleted, so both "
+ "directions would look alike and this test would assert nothing");
+
action->trigger();
- QCOMPARE(window.undoDepthForTesting(), 1);
- QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Delete")),
- qPrintable(QStringLiteral(
- "Delete on a reply of an undeleted thread chose "
- "the wrong direction: %1. It read the FIRST "
- "thread's state, which is deleted.")
- .arg(window.undoTextForTesting())));
+ // Asserted on the MODEL, not on the undo stack. Delete thread MOVES since
+ // item 103's follow-up, and the undo entry is pushed once the worker
+ // confirms the move, which this bare window has no database to perform.
+ // The DIRECTION is chosen synchronously and is what item 88's trap was
+ // about: the repaint below happens only on the delete direction.
+ QVERIFY2(model->threadAt(1).isDeleted(),
+ "Delete on a reply of an undeleted thread chose the wrong "
+ "direction: it read the FIRST thread's state, which is deleted");
+ // And the OTHER thread is untouched: the action must act on the reply's
+ // own conversation, not on both.
+ QVERIFY2(model->threadAt(0).isDeleted(),
+ "the fixture's first thread stopped being deleted, which means "
+ "the action reached a thread it was never pointed at");
}
void TestMainWindow::toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList()
@@ -5356,18 +5377,24 @@ void TestMainWindow::anActionOnAThreadRowActsOnTheMessageItDisplays()
// The thread action is how the conversation is reached, and it must still
// work from the same selection.
+ //
+ // Asserted on the MODEL rather than on a pending write. Delete thread
+ // MOVES every message since item 103's follow-up, and a move needs ids and
+ // paths that only the database holds for a thread this bare window never
+ // expanded, so the write is issued after a worker round trip that never
+ // completes here. What is synchronous, and what this test is about, is the
+ // scope: the whole thread is marked, not the one message its card shows.
auto *deleteThread =
window.findChild<QAction *>(QStringLiteral("delete_thread"));
QVERIFY(deleteThread);
+ QVERIFY2(!model->threadAt(0).isDeleted(),
+ "the thread already read as deleted, so the check below would "
+ "pass without the action doing anything");
deleteThread->trigger();
- QCOMPARE(window.pendingThreadIdsForTesting(),
- QStringList{ QStringLiteral("t1") });
-
- // Two commands, one per gesture, each recording the scope it used: a thread
- // action that pushed the message command would undo a fraction of what it
- // did.
- QCOMPARE(window.undoDepthForTesting(), 2);
+ QVERIFY2(model->threadAt(0).isDeleted(),
+ "Delete thread did not mark the whole thread, so the card paints "
+ "undeleted until the row is clicked");
}
void TestMainWindow::theThreadSubmenuIsReachableFromBothMenus()
@@ -8753,6 +8780,29 @@ void TestMainWindow::aSingleMessageIdQuerysCardOpensInTheMessagePane()
/// as `cur/del1.x:2,S`. Asserting on the exact basename therefore fails
/// against a move that worked perfectly, which is how three of these tests
/// first "failed".
+/// Counts messages matching `query` in the fixture's database, by running
+/// notmuch itself.
+///
+/// Asked directly rather than through the query bar because the UI's
+/// rowCount() reads 0 for the whole interval before the worker answers, so an
+/// assertion that a tag is ABSENT is satisfied by the gap before any answer
+/// arrives and passes against a database that still carries the tag.
+static int notmuchCount(const QString &configPath, const QString &query)
+{
+ QProcess process;
+ QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+ env.insert(QStringLiteral("NOTMUCH_CONFIG"), configPath);
+ process.setProcessEnvironment(env);
+ process.start(QStringLiteral("notmuch"),
+ { QStringLiteral("count"), query });
+ if (!process.waitForFinished(15000))
+ return -1;
+ bool ok = false;
+ const int count =
+ QString::fromUtf8(process.readAllStandardOutput()).trimmed().toInt(&ok);
+ return ok ? count : -1;
+}
+
static bool folderHasMessageFile(const QString &dir, const QString &stem)
{
QDir directory(dir);
@@ -8882,6 +8932,531 @@ void TestMainWindow::deleteRecordsWhereTheMessageCameFrom()
QLatin1Char(' ')))));
}
+void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind()
+{
+ // Delete twice is the ordinary way back: the action toggles, so a second
+ // press on a deleted message restores it. That path is NOT the undo path
+ // and had its own defect.
+ //
+ // onMessagesMoved() resolved the origin placeholder from the folder the
+ // WORKER reported, which is where the message came FROM. On a delete that
+ // is the inbox and correct. On a restore it is the TRASH, so the restore
+ // asked to remove `deleted-from:Trash`, a tag that had never been written,
+ // while the real `deleted-from:inbox` was never named and stayed on the
+ // message. It came home still claiming to have been deleted from
+ // somewhere, which makes Restore offer to move a message already at home.
+ //
+ // Reported from a hand test. The undo test passed throughout, because undo
+ // carries its tags on the command and never resolves a placeholder.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("twice@example.org"),
+ QStringLiteral("Delete me twice"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ 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);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString stem = QStringLiteral("twice.example.org");
+ const QString trash = root + QStringLiteral("/acct/Trash/cur");
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000);
+
+ // The origin tag really was written, so the assertion after the second
+ // delete is about it being REMOVED rather than never having existed.
+ queryEdit->setText(QStringLiteral(
+ "id:twice@example.org and tag:\"deleted-from:inbox\""));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ // Second press on the same message, which restores it.
+ queryEdit->setText(QStringLiteral("id:twice@example.org"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem)
+ || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"),
+ stem),
+ 15000);
+
+ // The file arriving is NOT the end of the restore. The tags are written
+ // only once the worker confirms the move, so the writes land after the
+ // rename the assertion above waits for. Querying in that gap reads the
+ // state before the restore finished tagging, which is how an earlier
+ // version of this test passed against the bug it exists to catch.
+ //
+ // Waited on the `deleted` tag, which the restore removes on every code
+ // path, rather than on a fixed sleep.
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(backed.fixture().configPath(),
+ QStringLiteral("id:twice@example.org and tag:deleted"))
+ == 0,
+ 15000);
+
+ // BOTH tags gone, asked of the database. `deleted-from:` left behind is
+ // the defect this covers, and it survived a green suite before.
+ // The origin tag specifically, asserted on its OWN query.
+ //
+ // A combined `tag:deleted or tag:"deleted-from:inbox"` query is NOT
+ // equivalent and passed against the bug: `deleted` is removed correctly
+ // and promptly, so the disjunction went to zero on that term alone while
+ // the origin tag was still on the message. Split, so the assertion can
+ // only be satisfied by the tag it names.
+ // Asked of notmuch DIRECTLY, not through the query bar.
+ //
+ // A UI query cannot answer this reliably: rowCount() is 0 for the whole
+ // interval before the worker replies, so QTRY_VERIFY(rowCount() == 0) is
+ // satisfied instantly by the empty pre-result and passes against any
+ // state of the database. Measured while building this test: 0 right after
+ // returnPressed(), 1 once the answer actually landed. The database is the
+ // thing under test here, so it is asked directly.
+ const QString cfg = backed.fixture().configPath();
+
+ // The message still exists: an assertion that a tag is absent would be
+ // satisfied just as well by the message having vanished.
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org")), 1);
+
+ // The origin tag is gone. This is the defect: it used to survive the
+ // restore, because the placeholder resolved to `deleted-from:Trash`, the
+ // folder the message was coming FROM, and stripped a tag that had never
+ // been written.
+ QCOMPARE(notmuchCount(cfg,
+ QStringLiteral("id:twice@example.org and "
+ "tag:\"deleted-from:inbox\"")),
+ 0);
+
+ // And no tag naming the trash was invented in its place.
+ QCOMPARE(notmuchCount(cfg,
+ QStringLiteral("id:twice@example.org and "
+ "tag:\"deleted-from:Trash\"")),
+ 0);
+
+ // `deleted` itself, so a fix that dropped this one instead cannot hide.
+ QCOMPARE(notmuchCount(
+ cfg, QStringLiteral("id:twice@example.org and tag:deleted")),
+ 0);
+
+}
+
+void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo()
+{
+ // Ctrl+Z is a THIRD way back, beside the second Delete, and it had the
+ // same defect for a different reason.
+ //
+ // MoveCommand was constructed with pending.add, which still holds the
+ // unresolved origin PLACEHOLDER: onMessagesMoved() resolved the
+ // placeholder for the tags it wrote to the database, but handed the undo
+ // command the raw list. Undo then asked to remove a tag by the
+ // placeholder's literal name, which no message carries, so the removal
+ // was a silent no-op and `deleted-from:inbox` survived. The message came
+ // home still claiming to have been deleted from somewhere, which makes
+ // Restore offer to move a message that is already at home.
+ //
+ // Reported from a hand test after the second-Delete path was fixed: that
+ // fix did not touch this one, and the existing undo test asserted on the
+ // file's location rather than on its tags.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("undotag@example.org"),
+ QStringLiteral("Undo my tags"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ 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);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString stem = QStringLiteral("undotag.example.org");
+ const QString cfg = backed.fixture().configPath();
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem),
+ 15000);
+
+ // The origin tag really was written, so the assertion after the undo is
+ // about it being REMOVED rather than never having existed.
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg, QStringLiteral("id:undotag@example.org and "
+ "tag:\"deleted-from:inbox\"")) == 1,
+ 15000);
+
+ window.findChild<QAction *>(QStringLiteral("undo"))->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem)
+ || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"),
+ stem),
+ 15000);
+
+ // The file arriving is not the end of the undo: the tags are written only
+ // once the worker confirms the move, so they land after the rename.
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg,
+ QStringLiteral("id:undotag@example.org and tag:deleted"))
+ == 0,
+ 15000);
+
+ // Asked of notmuch directly. A UI query cannot answer this: rowCount() is
+ // 0 for the whole interval before the worker replies, so an assertion
+ // that a tag is absent is satisfied by the gap before any answer arrives.
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org")), 1);
+ QCOMPARE(notmuchCount(cfg,
+ QStringLiteral("id:undotag@example.org and "
+ "tag:\"deleted-from:inbox\"")),
+ 0);
+ QCOMPARE(notmuchCount(cfg,
+ QStringLiteral("id:undotag@example.org and "
+ "tag:\"deleted-from:Trash\"")),
+ 0);
+}
+
+void TestMainWindow::deletingAThreadRootTwiceRestoresItRatherThanRedeleting()
+{
+ // The toggle asked a THREAD ROW about its thread's tags, which notmuch
+ // gives as a UNION over the conversation. Delete the root of a
+ // three-message thread and the two replies are untouched, so the union
+ // carries no `deleted`, so a second press read the row as not-deleted and
+ // ran Delete AGAIN: the message was moved trash-to-trash and came out
+ // carrying `deleted`, `deleted-from:inbox` AND `deleted-from:Trash`, with
+ // no way back, since a later restore would send it to the trash it now
+ // claims to have come from.
+ //
+ // 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, which is a no-op. A MOVE re-applies the move. The comment outlived
+ // the code it described.
+ //
+ // The row must be left ALONE between the two presses: a re-query rebuilds
+ // it from the database and hides the defect, which is why an earlier
+ // version of this probe passed. The user's gesture is two presses on the
+ // list as it stands.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("troot@example.org"),
+ QStringLiteral("Thread root"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Root body.")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("trep1@example.org"),
+ QStringLiteral("Re: Thread root"), QStringLiteral("other@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"),
+ QStringLiteral("Reply one."), true,
+ QStringLiteral("troot@example.org")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("trep2@example.org"),
+ QStringLiteral("Re: Thread root"), QStringLiteral("third@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 12:00:00 +0200"),
+ QStringLiteral("Reply two."), true,
+ QStringLiteral("troot@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);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString cfg = backed.fixture().configPath();
+ const QString stem = QStringLiteral("troot.example.org");
+ const QString trash = root + QStringLiteral("/acct/Trash/cur");
+
+ // Three messages, so the union genuinely differs from the root's own
+ // tags. With one message the two are identical and the defect cannot
+ // appear at all.
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("thread:{id:troot@example.org}")),
+ 3);
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000);
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg, QStringLiteral("id:troot@example.org and "
+ "tag:\"deleted-from:inbox\"")) == 1,
+ 15000);
+
+ // Only the root moved. The replies are what make the union disagree, so
+ // this is also the guard the rest of the test depends on.
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:trep1@example.org and "
+ "tag:deleted")),
+ 0);
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:trep2@example.org and "
+ "tag:deleted")),
+ 0);
+
+ // Second press on the row as it stands, no re-query.
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem)
+ || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"),
+ stem),
+ 15000);
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg,
+ QStringLiteral("id:troot@example.org and tag:deleted"))
+ == 0,
+ 15000);
+
+ // Asked of notmuch directly: a UI query reads 0 rows for the whole
+ // interval before the worker answers, so an absence assertion through the
+ // query bar passes against any state of the database.
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:troot@example.org")), 1);
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:troot@example.org and "
+ "tag:\"deleted-from:inbox\"")),
+ 0);
+ // The tag the re-delete invented. Its presence is the signature of this
+ // defect rather than a variation on the origin-tag ones.
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:troot@example.org and "
+ "tag:\"deleted-from:Trash\"")),
+ 0);
+ QVERIFY2(!folderHasMessageFile(trash, stem),
+ "the second press left the message in the trash");
+}
+
+void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard()
+{
+ // Two defects in one gesture, both reported from a hand test.
+ //
+ // Delete thread never moved anything: it was left calling tagSelected()
+ // when Delete became a move, so a whole conversation stayed in the inbox
+ // wearing a `deleted` chip, which is the half-deleted state item 103
+ // existed to remove. It moves every message now, each carrying its own
+ // `deleted-from:` origin so a thread spanning folders reassembles.
+ //
+ // And the ROOT card did not repaint until it was clicked, while its
+ // replies did. A thread-scoped move updated each message's node;
+ // applyMessageTagChange() deliberately leaves a multi-message thread's
+ // SUMMARY alone, because one message's edit does not describe the
+ // conversation. The replies have nodes and repainted; the root card reads
+ // the summary and did not. A thread-scoped move DID change every message,
+ // so the summary genuinely moves and applyTagChange() is the right update.
+ //
+ // The stale summary was also why a second press did nothing: the toggle
+ // asks the summary for its direction and kept reading "not deleted".
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("dt0@example.org"),
+ QStringLiteral("DT root"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Root body.")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("dt1@example.org"),
+ QStringLiteral("Re: DT root"), QStringLiteral("other@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"),
+ QStringLiteral("Reply one."), true, QStringLiteral("dt0@example.org")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("dt2@example.org"),
+ QStringLiteral("Re: DT root"), QStringLiteral("third@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 12:00:00 +0200"),
+ QStringLiteral("Reply two."), true, QStringLiteral("dt0@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);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString cfg = backed.fixture().configPath();
+ const QString trash = root + QStringLiteral("/acct/Trash/cur");
+ const QString thread = QStringLiteral("thread:{id:dt0@example.org}");
+
+ // Three messages, so a thread-scoped action is distinguishable from a
+ // message-scoped one at all.
+ QCOMPARE(notmuchCount(cfg, thread), 3);
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ view->expand(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger();
+
+ // Every message MOVED, not merely tagged. This is the half that was
+ // missing entirely: the action tagged and moved nothing.
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(trash, QStringLiteral("dt0.example.org"))
+ && folderHasMessageFile(trash, QStringLiteral("dt1.example.org"))
+ && folderHasMessageFile(trash, QStringLiteral("dt2.example.org")),
+ 15000);
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 3,
+ 15000);
+ // Each with its own origin, which is what makes the move reversible.
+ QCOMPARE(notmuchCount(cfg, thread
+ + QStringLiteral(" and "
+ "tag:\"deleted-from:inbox\"")),
+ 3);
+
+ // The ROOT CARD's own state, which is what the user watches. Read from the
+ // summary because that is what a thread row draws, and it is the value
+ // that stayed stale: the replies repainted and the root did not.
+ QVERIFY2(model->threadAt(0).tags.contains(QStringLiteral("deleted")),
+ "the root card still reads as not deleted, so it paints "
+ "undeleted until the row is clicked");
+
+ // Second press restores the whole thread, which only works if the toggle
+ // can see the state the first press produced.
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger();
+
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 0,
+ 15000);
+
+ // Home, and nothing left behind in the trash.
+ QCOMPARE(notmuchCount(cfg, thread), 3);
+ QCOMPARE(notmuchCount(cfg, thread
+ + QStringLiteral(" and "
+ "tag:\"deleted-from:inbox\"")),
+ 0);
+ QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt0.example.org")));
+ QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt1.example.org")));
+ QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt2.example.org")));
+}
+
+void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip()
+{
+ // A notmuch tag MAY contain a space, and a Maildir folder name may too.
+ // The worker reported each message's tags as one space-joined string, so
+ // `deleted-from:Inbox/SlackBuilds users` was split back into
+ // "deleted-from:Inbox/SlackBuilds" and "users", and Restore moved the
+ // messages to the truncated folder, CREATING it. On the user's real
+ // Maildir that put four messages into a directory mbsync does not sync,
+ // beside the real folder of 808, and they read as missing.
+ //
+ // The leftover origin tag was the visible half: the restore stripped the
+ // truncated name, which no message carried, so the real tag stayed on.
+ //
+ // Separator is a TAB now. A tag cannot contain one, since notmuch's own
+ // dump format is line-based and whitespace-delimited.
+ WorkerBackedWindow backed;
+ const QString folder = QStringLiteral("acct/Inbox/SlackBuilds users");
+ QVERIFY(backed.fixture().addMessage(
+ folder, QStringLiteral("sp0@example.org"), QStringLiteral("SP root"),
+ QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Root body.")));
+ QVERIFY(backed.fixture().addMessage(
+ folder, QStringLiteral("sp1@example.org"),
+ QStringLiteral("Re: SP root"), QStringLiteral("other@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"),
+ QStringLiteral("Reply."), true, QStringLiteral("sp0@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);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString cfg = backed.fixture().configPath();
+ const QString thread = QStringLiteral("thread:{id:sp0@example.org}");
+ const QString home = root + QLatin1Char('/') + folder;
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger();
+
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 2,
+ 15000);
+
+ // The origin tag carries the WHOLE folder name, space included.
+ QCOMPARE(notmuchCount(cfg,
+ thread
+ + QStringLiteral(" and tag:\"deleted-from:"
+ "Inbox/SlackBuilds users\"")),
+ 2);
+
+ // Back again.
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete_thread"))->trigger();
+
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg, thread + QStringLiteral(" and tag:deleted")) == 0,
+ 15000);
+
+ // No origin tag left behind. This is the half the user saw: a tag they
+ // could see, could not type, and could not remove.
+ QCOMPARE(notmuchCount(cfg,
+ thread
+ + QStringLiteral(" and tag:\"deleted-from:"
+ "Inbox/SlackBuilds users\"")),
+ 0);
+ // Nor a truncated one, which is what a space-split would have written.
+ QCOMPARE(notmuchCount(cfg,
+ thread
+ + QStringLiteral(" and tag:\"deleted-from:"
+ "Inbox/SlackBuilds\"")),
+ 0);
+
+ // Home, in the folder with the space in its name.
+ QVERIFY2(folderHasMessageFile(home + QStringLiteral("/cur"),
+ QStringLiteral("sp0.example.org")),
+ "the root did not come back to the folder it was deleted from");
+ QVERIFY2(folderHasMessageFile(home + QStringLiteral("/cur"),
+ QStringLiteral("sp1.example.org")),
+ "the reply did not come back to the folder it was deleted from");
+
+ // And the truncated folder was never created. Its existence is the defect
+ // that hid four real messages from the user and from mbsync.
+ QVERIFY2(!QDir(root + QStringLiteral("/acct/Inbox/SlackBuilds")).exists(),
+ "a folder named after the truncated origin was created, so the "
+ "messages are somewhere mbsync will never sync");
+}
+
void TestMainWindow::undoMovesTheMessageBack()
{
// Undo is this project's answer to the confirmation dialog it rules out,
@@ -9024,6 +9599,182 @@ void TestMainWindow::deleteOnAReplyMovesThatReplyOnly()
"deleting a reply moved its thread's root as well");
}
+void TestMainWindow::undoingADeleteConsumesItsCommandRatherThanPushingAnother()
+{
+ // A move is confirmed through onMessagesMoved(), and so is the move an
+ // UNDO makes. Pushing a command there unconditionally meant undo left a
+ // fresh command on the stack instead of consuming the one it undid, so
+ // the stack grew on every press: "Delete", "Undo Delete", "Undo Undo
+ // Delete". A user pressing undo twice to be sure re-deleted the mail they
+ // had just rescued, which is the opposite of what undo is for here, undo
+ // being this project's stand-in for a confirmation dialog.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("undo2@example.org"),
+ QStringLiteral("Undo twice"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ 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);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString stem = QStringLiteral("undo2.example.org");
+ const QString trash = root + QStringLiteral("/acct/Trash/cur");
+ const auto inInbox = [&] {
+ return folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"),
+ stem)
+ || folderHasMessageFile(
+ root + QStringLiteral("/acct/inbox/new"), stem);
+ };
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000);
+
+ // The guard the assertions below need: one command, from the delete.
+ QTRY_VERIFY_WITH_TIMEOUT(window.undoDepthForTesting() == 1, 15000);
+
+ window.findChild<QAction *>(QStringLiteral("undo"))->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(inInbox(), 15000);
+
+ // The stack is spent. Asserted on undoText rather than depth alone
+ // because a command that is merely marked done still reports its text,
+ // and it is the text the user reads off the Edit menu.
+ QTRY_VERIFY_WITH_TIMEOUT(window.undoTextForTesting().isEmpty(), 15000);
+
+ // And the real point: pressing undo again must not move the message
+ // anywhere. Before the fix this put it straight back in the trash.
+ window.findChild<QAction *>(QStringLiteral("undo"))->trigger();
+ QTest::qWait(1500);
+ QVERIFY2(!folderHasMessageFile(trash, stem),
+ "a second undo re-deleted the message the first one restored");
+ QVERIFY2(inInbox(), "a second undo moved the message out of the inbox");
+}
+
+void TestMainWindow::aDeleteHeldDuringASyncCountsAsUnsyncedWork()
+{
+ // 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 `pendingEditCount() > 0` guard never
+ // fired, discarding the move on quit with no prompt. That is item 106's
+ // data loss with a worse shape, since a dropped move leaves the file in
+ // the folder the user asked it out of.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("held1@example.org"),
+ QStringLiteral("Held by a sync"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ 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"));
+ auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits"));
+ QVERIFY(model && view && queryEdit && label);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+
+ // Hidden before the gesture, so the assertion after it means something.
+ QVERIFY2(label->isHidden(), "the pending indicator was already showing");
+
+ // A sync now holds the write lock, which is what makes the move held
+ // rather than sent.
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Running));
+
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+
+ QVERIFY2(!label->isHidden(),
+ "a Delete held by a sync did not count as unsynced work, so "
+ "quitting would have discarded it with no prompt");
+
+ // The file really is still where it was: this is a HELD move, not a
+ // failed one, and the indicator would be meaningless otherwise.
+ const QString root = backed.fixture().maildirPath();
+ QVERIFY(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"),
+ QStringLiteral("held1.example.org")));
+}
+
+void TestMainWindow::twoDeletesToOneTrashBothGetTheirTags()
+{
+ // The pending-move table was keyed on the destination folder, so two
+ // Deletes in one account before the first confirmation arrived both named
+ // `acct/Trash`: the second insert overwrote the first and the second
+ // confirmation took an empty entry. That file reached the trash carrying
+ // neither `deleted` nor `deleted-from:`, which makes it unrestorable by
+ // Restore and invisible to a `tag:deleted` query.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("two1@example.org"),
+ QStringLiteral("First"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("two2@example.org"),
+ QStringLiteral("Second"), QStringLiteral("other@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ 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);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000);
+
+ // Both Deletes issued back to back, WITHOUT waiting for the first to be
+ // confirmed. That is the whole point: waiting would serialise them and
+ // the keyed table would have coped.
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+ view->setCurrentIndex(model->index(1, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+
+ const QString root = backed.fixture().maildirPath();
+ const QString trash = root + QStringLiteral("/acct/Trash/cur");
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(trash, QStringLiteral("two1.example.org"))
+ && folderHasMessageFile(trash, QStringLiteral("two2.example.org")),
+ 15000);
+
+ // Both carry BOTH tags, asked of the database rather than of the model:
+ // the defect was a write that never happened, and the model would have
+ // shown the optimistic state either way.
+ queryEdit->setText(QStringLiteral(
+ "tag:deleted and tag:\"deleted-from:inbox\" and "
+ "(id:two1@example.org or id:two2@example.org)"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000);
+}
+
void TestMainWindow::deleteWithoutATrashFolderSaysSoRatherThanDoingNothing()
{
// Task 2 warns at config load. This is the second line of defence: a key
diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts
index 89886d0..cc854b2 100644
--- a/translations/qtmaildir_it_IT.ts
+++ b/translations/qtmaildir_it_IT.ts
@@ -132,20 +132,6 @@
<source>Unsynced changes</source>
<translation>Modifiche non sincronizzate</translation>
</message>
- <message numerus="yes">
- <source>%n tag change(s) have not been synced, and no sync command is configured. Quit anyway?</source>
- <translation>
- <numerusform>%n modifica alle etichette non è stata sincronizzata e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform>
- <numerusform>%n modifiche alle etichette non sono state sincronizzate e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform>
- </translation>
- </message>
- <message numerus="yes">
- <source>%n tag change(s) have not been synced.</source>
- <translation>
- <numerusform>%n modifica alle etichette non è stata sincronizzata.</numerusform>
- <numerusform>%n modifiche alle etichette non sono state sincronizzate.</numerusform>
- </translation>
- </message>
<message>
<source>Sync before quitting?</source>
<translation>Sincronizzare prima di uscire?</translation>
@@ -262,6 +248,10 @@
<source>Add or remove the deleted tag</source>
<translation>Aggiunge o rimuove l&apos;etichetta deleted</translation>
</message>
+ <message>
+ <source>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.</source>
+ <translation>Modifiche fatte qui che nessuna sincronizzazione ha ancora trasferito all&apos;archivio di posta. Un&apos;esecuzione esterna di notmuch può azzerarle senza che questo conteggio se ne accorga.</translation>
+ </message>
<message numerus="yes">
<source>%n message(s) could not be deleted: no trash folder is configured for their account.</source>
<translation>
@@ -277,6 +267,20 @@
<source>Delete</source>
<translation>Elimina</translation>
</message>
+ <message numerus="yes">
+ <source>%n change(s) have not been synced, and no sync command is configured. Quit anyway?</source>
+ <translation>
+ <numerusform>%n modifica non è stata sincronizzata e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform>
+ <numerusform>%n modifiche non sono state sincronizzate e non è configurato alcun comando di sincronizzazione. Uscire comunque?</numerusform>
+ </translation>
+ </message>
+ <message numerus="yes">
+ <source>%n change(s) have not been synced.</source>
+ <translation>
+ <numerusform>%n modifica non è stata sincronizzata.</numerusform>
+ <numerusform>%n modifiche non sono state sincronizzate.</numerusform>
+ </translation>
+ </message>
<message>
<source>Mark &amp;spam</source>
<translation>Segna come &amp;spam</translation>
@@ -362,10 +366,6 @@
<translation>Ripristina conversazione</translation>
</message>
<message>
- <source>Delete thread</source>
- <translation>Elimina conversazione</translation>
- </message>
- <message>
<source>Mark thread as &amp;spam</source>
<translation>Segna conversazione come &amp;spam</translation>
</message>
@@ -851,10 +851,6 @@
</translation>
</message>
<message>
- <source>Tag 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.</source>
- <translation>Modifiche alle etichette fatte qui che nessuna sincronizzazione ha ancora trasferito all&apos;archivio di posta. Un&apos;esecuzione esterna di notmuch può azzerarle senza che questo conteggio se ne accorga.</translation>
- </message>
- <message>
<source>&amp;Whole thread</source>
<translation>&amp;Intera conversazione</translation>
</message>