# Delete Moves Mail To Trash Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Make Delete move a message into its account's trash folder instead of only tagging it, with a Trash filter, a Restore action, and a repeatable cleanup for mail stranded by the old behaviour. **Architecture:** A new per-account `trash` config key (mandatory), a `moveMessages(ids, destFolder)` mutation on `NotmuchWorker` shaped for reuse by v2's Send, a fifth built-in query generator composed exactly as `sent` is, and UI actions that route through the existing undo stack. **Tech Stack:** C++17, Qt 6.11, libnotmuch, QtTest. Build with CMake + Ninja. **Spec:** `docs/superpowers/specs/2026-08-17-delete-to-trash-design.md`. Read it before starting; it records the measurements behind these decisions. --- ## Before you start Run the suite once so you know it is green before you touch anything: ```bash cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug cmake --build build ctest --test-dir build --output-on-failure ``` Expected: 24 tests, all passing. **Never run a test binary without `QT_QPA_PLATFORM=offscreen`, and never launch `./build/src/qtmaildir`.** The project's `CLAUDE.md` explains why: a direct run throws real windows onto the user's desktop, and `test_mainwindow` alone flashes over a hundred. ## File structure | File | Responsibility | Change | |---|---|---| | `src/config.h` / `src/config.cpp` | The `trash` account key, the `trash` generator, its queries | Modify | | `src/notmuchworker.h` / `src/notmuchworker.cpp` | `moveMessages()`, the first non-tag mutation | Modify | | `src/mainwindow.h` / `src/mainwindow.cpp` | Delete rewired, Restore, cleanup menu entry, undo commands | Modify | | `tests/test_config.cpp` | Key parsing, generator composition, the missing-key error | Modify | | `tests/test_notmuchworker.cpp` | The move, its ordering, its failure modes | Modify | | `tests/test_mainwindow.cpp` | Delete's new behaviour, Restore's visibility, undo | Modify | | `CHANGELOG.md` | The `### Upgrading` note for the mandatory key | Modify | No new files. Every change lands in a file that already owns that responsibility. --- ## Task 1: The `trash` account key **Files:** - Modify: `src/config.h` (the `Account` struct, near the `sent` member around line 58) - Modify: `src/config.cpp` (`Account::trashQuery()` beside `sentQuery()` at line 130, and the account parser) - Test: `tests/test_config.cpp` - [x] **Step 1: Write the failing test** Add to `tests/test_config.cpp`, and declare both slots in the `private slots:` block: ```cpp void TestConfig::anAccountCarriesItsTrashFolder() { QTemporaryDir dir; QVERIFY(dir.isValid()); const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); { QFile file(path); QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); QTextStream out(&file); out << "[account.work]\n" << "maildir=work\n" << "trash=Trash\n"; } Config config; config.load(path); const Account account = config.account(QStringLiteral("work")); QCOMPARE(account.trash, QStringLiteral("Trash")); // Quoted and globbed exactly as sentQuery() does it, so a folder with a // space or a bracket cannot break the query. QCOMPARE(account.trashQuery(), QStringLiteral("path:\"work/Trash/**\"")); } void TestConfig::aBracketedTrashFolderIsQuoted() { // The real setup nests a localised trash folder under a bracketed parent. // The brackets are not notmuch syntax, but the quoting has to survive them. Account account; account.maildir = QStringLiteral("provider-a"); account.trash = QStringLiteral("[Provider]/Cestino"); QCOMPARE(account.trashQuery(), QStringLiteral("path:\"provider-a/[Provider]/Cestino/**\"")); } ``` - [x] **Step 2: Run test to verify it fails** ```bash cmake --build build 2>&1 | tail -5 ``` Expected: FAIL to compile, `'trash' is not a member of 'Account'`. - [x] **Step 3: Write minimal implementation** In `src/config.h`, add to `Account` immediately after the `sent` member: ```cpp /// The account's trash folder, relative to maildir. /// /// MANDATORY, unlike `sent` and `drafts`. Delete moves a file into this /// folder, so an account without one cannot delete at all, and the user /// chose a config error over a per-account disabled state: "it is /// mandatory for the program to function properly". Config::load() /// reports a missing key through the warnings path. QString trash; ``` And beside `sentQuery()`: ```cpp /// Matches this account's trash, or empty when `trash` is unset. /// /// Empty is a config error rather than a legitimate state, unlike /// sentQuery(). The query helper still returns empty so callers compose /// uniformly; it is Config::load() that reports the problem. QString trashQuery() const; ``` In `src/config.cpp`, beside `Account::sentQuery()` at line 130: ```cpp QString Account::trashQuery() const { return folderQuery(maildir, trash); } ``` Then find where the account parser reads `sent` (search for `QStringLiteral("sent")` inside the account-loading loop) and add the sibling line, following whatever form the neighbouring keys use: ```cpp account.trash = settings.value(QStringLiteral("trash")).toString(); ``` - [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config ``` Expected: PASS, all tests. - [x] **Step 5: Commit** ```bash git add src/config.h src/config.cpp tests/test_config.cpp git commit -m "feat(config): read a per-account trash folder" ``` --- ## Task 2: A missing `trash` key is a config error **Files:** - Modify: `src/config.cpp` (the account-loading loop, beside the existing warnings) - Test: `tests/test_config.cpp` First read how warnings are currently raised: search `src/config.cpp` for `m_warnings` and copy the surrounding form exactly. Do not invent a new mechanism. - [x] **Step 1: Write the failing test** ```cpp void TestConfig::anAccountWithoutATrashFolderWarns() { QTemporaryDir dir; QVERIFY(dir.isValid()); const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); { QFile file(path); QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); QTextStream out(&file); out << "[account.work]\n" << "maildir=work\n"; } Config config; config.load(path); // The account still loads. A missing trash folder disables Delete, it does // not invalidate the account: the user can still read mail. QVERIFY(config.account(QStringLiteral("work")).isValid()); const QStringList warnings = config.warnings(); QVERIFY(!warnings.isEmpty()); // Names the account and the key, so the warning is actionable. A warning // the user cannot act on teaches them to ignore warnings, which item 83 // recorded the hard way. const QString joined = warnings.join(QLatin1Char('\n')); QVERIFY(joined.contains(QStringLiteral("work"))); QVERIFY(joined.contains(QStringLiteral("trash"))); } ``` - [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config -functions | grep -i trash QT_QPA_PLATFORM=offscreen ./build/tests/test_config anAccountWithoutATrashFolderWarns ``` Expected: FAIL, `warnings` is empty. - [x] **Step 3: Write minimal implementation** In the account-loading loop in `src/config.cpp`, after the account is parsed and before it is appended: ```cpp if (account.trash.isEmpty()) { // Mandatory, unlike `sent`. Delete moves a file into this folder, // so without it the action cannot work at all. Reported rather // than defaulted to "Trash": that folder does not exist under // every provider, and a default pointing at a folder mbsync does // not sync would move mail somewhere the server never sees. m_warnings.append( tr("Account \"%1\" has no trash folder. Add a `trash` key to " "qtmaildir.conf; Delete cannot work without it.") .arg(account.key)); } ``` - [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config ``` Expected: PASS. - [x] **Step 5: Commit** ```bash git add src/config.cpp tests/test_config.cpp git commit -m "feat(config): warn when an account configures no trash folder" ``` --- ## Task 3: The `trash` query generator **Files:** - Modify: `src/config.h` (declare `allTrashQuery()` beside `allSentQuery()`) - Modify: `src/config.cpp` (`kQueryGenerators` line 60, `builtinFilter()` line 757, `resolvedQuery()` line 790, `allTrashQuery()` beside line 140) - Test: `tests/test_config.cpp` - [x] **Step 1: Write the failing test** ```cpp void TestConfig::theTrashFilterComposesPerAccount() { QTemporaryDir dir; QVERIFY(dir.isValid()); const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); { QFile file(path); QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); QTextStream out(&file); out << "[account.work]\n" << "maildir=work\n" << "trash=Trash\n" << "[account.personal]\n" << "maildir=personal\n" << "trash=[Provider]/Cestino\n"; } Config config; config.load(path); const SavedQuery trash = Config::builtinFilter(QStringLiteral("trash")); QVERIFY(trash.isGenerated()); // All accounts: the union, never a bare path that would match one account. const QString all = config.resolvedQuery(trash, QString()); QVERIFY(all.contains(QStringLiteral("path:\"work/Trash/**\""))); QVERIFY(all.contains(QStringLiteral("path:\"personal/[Provider]/Cestino/**\""))); // One account: that account's OWN query. Asserting on the STRING, not on a // row count: the all-accounts query wrapped in this account's path returns // exactly the right rows, because path: is hierarchical, so a count passes // against the wrong thing. Config::resolvedQuery documents this trap. const QString scoped = config.resolvedQuery(trash, QStringLiteral("work")); QCOMPARE(scoped, QStringLiteral("path:\"work/Trash/**\"")); QVERIFY(!scoped.contains(QStringLiteral("personal"))); } void TestConfig::theTrashFilterMatchesNothingWithoutAFolder() { // An empty query means "match everything" to notmuch, so a filter with // nothing to match must say so explicitly. A button labelled Trash that // showed the whole Maildir is the failure this prevents. Config config; const SavedQuery trash = Config::builtinFilter(QStringLiteral("trash")); QCOMPARE(config.resolvedQuery(trash, QString()), Config::matchNothingQuery()); } ``` - [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config theTrashFilterComposesPerAccount ``` Expected: FAIL, `trash.isGenerated()` is false because the generator is unknown. - [x] **Step 3: Write minimal implementation** In `src/config.cpp`, add to `kQueryGenerators` at line 60, last so it sits rightmost on the query row: ```cpp const QStringList kQueryGenerators = { QStringLiteral("unread"), QStringLiteral("inbox"), QStringLiteral("flagged"), QStringLiteral("sent"), QStringLiteral("trash") }; ``` In `Config::builtinFilter()`, add a branch beside the `sent` one: ```cpp } else if (generator == QStringLiteral("trash")) { filter.name = tr("Trash"); // NOT flat, unlike Sent. A deleted message still belongs to its // conversation, and folding it back is what Sent had to avoid rather // than something every folder filter wants. } ``` In `src/config.h`, beside `allSentQuery()`: ```cpp /// The union of every account's trashQuery(), empty when none has one. QString allTrashQuery() const; ``` In `src/config.cpp`, beside `allSentQuery()` at line 140: ```cpp QString Config::allTrashQuery() const { return joinAccountQueries(m_accounts, &Account::trashQuery); } ``` In `Config::resolvedQuery()`, extend both branches. In the all-accounts branch, beside the `sent` case: ```cpp if (query.generated == QStringLiteral("trash")) { const QString all = allTrashQuery(); return all.isEmpty() ? matchNothingQuery() : all; } ``` And in the per-account branch, beside the `sent` case: ```cpp if (query.generated == QStringLiteral("trash")) { // The account's OWN query, for the reason spelled out above the sent // case: wrapping the all-accounts query in this account's path works // by accident of path: being hierarchical. const QString trash = scope.trashQuery(); return trash.isEmpty() ? matchNothingQuery() : trash; } ``` - [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_config ``` Expected: PASS. - [x] **Step 5: Verify the mutation fails** Temporarily change the per-account branch to `return allTrashQuery();` and rebuild. `theTrashFilterComposesPerAccount` must FAIL on the `QCOMPARE`. Revert the mutation. This proves the test asserts on the string rather than on rows. - [x] **Step 6: Commit** ```bash git add src/config.h src/config.cpp tests/test_config.cpp git commit -m "feat(config): add the trash query generator" ``` --- ## Task 4: `moveMessages()` on the worker **Files:** - Modify: `src/notmuchworker.h` (declare the slot and its signal) - Modify: `src/notmuchworker.cpp` (implement beside `applyTags()` at line 550) - Test: `tests/test_notmuchworker.cpp` This is the first mutation in the project that is not a notmuch tag. Read `applyTags()` at `src/notmuchworker.cpp:550` first: the close-first ordering it uses is required, not stylistic, because notmuch permits one open handle per process. - [x] **Step 1: Write the failing test** Add to `tests/test_notmuchworker.cpp`, declaring each slot in `private slots:`: ```cpp void TestNotmuchWorker::moveMessagesRelocatesTheFile() { NotmuchFixture fixture; QVERIFY(fixture.isValid()); QVERIFY(fixture.addMessage(QStringLiteral("Inbox"), QStringLiteral("m1@example.org"), QStringLiteral("Movable"), QStringLiteral("Alice "), QStringLiteral("Mon, 1 Jun 2026 10:00:00 +0000"), QStringLiteral("body"), false)); NotmuchWorker worker; worker.setConfigPath(fixture.configPath()); QSignalSpy spy(&worker, &NotmuchWorker::messagesMoved); worker.moveMessages({ QStringLiteral("m1@example.org") }, QStringLiteral("Trash")); QCOMPARE(spy.count(), 1); QCOMPARE(spy.at(0).at(0).toStringList(), QStringList{ QStringLiteral("m1@example.org") }); // The file is where it was sent, and no longer where it was. const QDir trash(fixture.maildirPath() + QStringLiteral("/Trash/cur")); QCOMPARE(trash.entryList(QDir::Files).count(), 1); const QDir inbox(fixture.maildirPath() + QStringLiteral("/Inbox/cur")); QCOMPARE(inbox.entryList(QDir::Files).count(), 0); } void TestNotmuchWorker::moveMessagesReindexesAtTheNewPath() { NotmuchFixture fixture; QVERIFY(fixture.isValid()); QVERIFY(fixture.addMessage(QStringLiteral("Inbox"), QStringLiteral("m2@example.org"), QStringLiteral("Reindexed"), QStringLiteral("Alice "), QStringLiteral("Mon, 1 Jun 2026 10:00:00 +0000"), QStringLiteral("body"), false)); NotmuchWorker worker; worker.setConfigPath(fixture.configPath()); worker.moveMessages({ QStringLiteral("m2@example.org") }, QStringLiteral("Trash")); // notmuch knows the message at its new path and not at its old one. This // is the half a filesystem check cannot see: a moved file with a stale // index entry looks correct on disk and is invisible to every query. NotmuchWorker reader; reader.setConfigPath(fixture.configPath()); QSignalSpy spy(&reader, &NotmuchWorker::threadsReady); reader.runQuery(QStringLiteral("path:\"Trash/**\""), 1, NotmuchWorker::NewestFirst, false); QVERIFY(spy.count() > 0); const auto threads = spy.at(0).at(0).value>(); QCOMPARE(threads.count(), 1); QSignalSpy old(&reader, &NotmuchWorker::threadsReady); reader.runQuery(QStringLiteral("path:\"Inbox/**\""), 2, NotmuchWorker::NewestFirst, false); QVERIFY(old.count() > 0); QCOMPARE(old.at(0).at(0).value>().count(), 0); } void TestNotmuchWorker::moveMessagesKeepsTheMessagesTags() { NotmuchFixture fixture; QVERIFY(fixture.isValid()); QVERIFY(fixture.addMessage(QStringLiteral("Inbox"), QStringLiteral("m3@example.org"), QStringLiteral("Tagged"), QStringLiteral("Alice "), QStringLiteral("Mon, 1 Jun 2026 10:00:00 +0000"), QStringLiteral("body"), true)); NotmuchWorker worker; worker.setConfigPath(fixture.configPath()); worker.applyTags(TagChange{ { QStringLiteral("m3@example.org") }, { QStringLiteral("keepme") }, {}, QString() }); worker.moveMessages({ QStringLiteral("m3@example.org") }, QStringLiteral("Trash")); // Indexing the new path BEFORE removing the old one is what preserves // these. The other order removes the last filename for the message id, // which destroys the database entry and every tag on it. NotmuchWorker reader; reader.setConfigPath(fixture.configPath()); QSignalSpy spy(&reader, &NotmuchWorker::messageLoaded); reader.loadMessage(QStringLiteral("m3@example.org"), 1); QVERIFY(spy.count() > 0); const auto ref = spy.at(0).at(0).value(); QVERIFY(ref.tags.contains(QStringLiteral("keepme"))); } void TestNotmuchWorker::moveMessagesReportsOnlyWhatMoved() { NotmuchFixture fixture; QVERIFY(fixture.isValid()); QVERIFY(fixture.addMessage(QStringLiteral("Inbox"), QStringLiteral("m4@example.org"), QStringLiteral("Real"), QStringLiteral("Alice "), QStringLiteral("Mon, 1 Jun 2026 10:00:00 +0000"), QStringLiteral("body"), false)); NotmuchWorker worker; worker.setConfigPath(fixture.configPath()); QSignalSpy spy(&worker, &NotmuchWorker::messagesMoved); // A stale id alongside a live one must not abort the batch, exactly as // applyTags() documents for the same case. worker.moveMessages({ QStringLiteral("m4@example.org"), QStringLiteral("gone@example.org") }, QStringLiteral("Trash")); QCOMPARE(spy.count(), 1); // Reports what MOVED, not what was asked for. A caller that assumed the // request succeeded would show a delete that never happened. QCOMPARE(spy.at(0).at(0).toStringList(), QStringList{ QStringLiteral("m4@example.org") }); } ``` - [x] **Step 2: Run test to verify it fails** ```bash cmake --build build 2>&1 | tail -5 ``` Expected: FAIL to compile, `'moveMessages' is not a member of 'NotmuchWorker'`. - [x] **Step 3: Write minimal implementation** In `src/notmuchworker.h`, beside `applyTags()`: ```cpp /// Moves messages into `destFolder`, relative to the database path. /// /// A folder NAME rather than a "move to trash" call, because v2's Send /// needs exactly this operation for Drafts and Sent. Nothing /// trash-specific belongs here. /// /// The first mutation in this class that is not a notmuch tag: a rename on /// disk plus a reindex. Ordering is rename, index the new path, drop the /// old one. Indexing first is required, not stylistic: removing the last /// filename for a message id deletes the database entry and every tag on /// it, so removing before indexing loses the message's tags. void moveMessages(const QStringList &messageIds, const QString &destFolder); ``` And in the signals block: ```cpp /// Carries the ids that ACTUALLY moved, which may be fewer than requested. /// A stale id, a missing folder or a failed rename drops out here rather /// than aborting the batch. void messagesMoved(const QStringList &messageIds, const QString &destFolder); ``` In `src/notmuchworker.cpp`, beside `applyTags()`. Add `#include ` and `#include ` at the top if they are not already there: ```cpp void NotmuchWorker::moveMessages(const QStringList &messageIds, const QString &destFolder) { if (messageIds.isEmpty() || destFolder.isEmpty()) return; // Close first: notmuch permits one open handle per process, so the // read-only handle has to go before a read-write one opens. Same // constraint applyTags() documents. close(); const QByteArray configPath = configPathArg(); notmuch_database_t *db = nullptr; char *error = nullptr; const notmuch_status_t status = notmuch_database_open_with_config( nullptr, NOTMUCH_DATABASE_MODE_READ_WRITE, configPath.isEmpty() ? nullptr : configPath.constData(), nullptr, &db, &error); if (status != NOTMUCH_STATUS_SUCCESS) { emit errorOccurred( QStringLiteral("Cannot open database for writing: %1") .arg(QString::fromUtf8(error ? error : notmuch_status_to_string(status)))); free(error); return; } const QString root = QString::fromUtf8(notmuch_database_get_path(db)); QStringList moved; for (const QString &id : messageIds) { notmuch_message_t *raw = nullptr; if (notmuch_database_find_message(db, id.toUtf8().constData(), &raw) != NOTMUCH_STATUS_SUCCESS || !raw) { continue; } NmMessage message(raw); const QString from = QString::fromUtf8(notmuch_message_get_filename(message.get())); if (from.isEmpty()) continue; // cur/, never new/. A message that has been seen by any client lives // in cur, and a file dropped into new/ would be re-announced as fresh // mail by every reader of this Maildir. const QString destDir = root + QLatin1Char('/') + destFolder + QStringLiteral("/cur"); if (!QDir().mkpath(destDir)) { emit errorOccurred( QStringLiteral("Cannot create folder: %1").arg(destDir)); continue; } const QString to = destDir + QLatin1Char('/') + QFileInfo(from).fileName(); if (from == to) continue; if (!QFile::rename(from, to)) { emit errorOccurred( QStringLiteral("Cannot move message to %1").arg(destFolder)); continue; } // Index the NEW path before dropping the old one. The reverse order // removes the last filename for this message id, and notmuch then // deletes the database entry outright, taking every tag with it. notmuch_message_t *indexed = nullptr; const notmuch_status_t added = notmuch_database_index_file( db, to.toUtf8().constData(), nullptr, &indexed); if (indexed) notmuch_message_destroy(indexed); // DUPLICATE_MESSAGE_ID is success here: it means the id was already // known, which is exactly the case for a file we just moved. if (added != NOTMUCH_STATUS_SUCCESS && added != NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) { // Put it back rather than leaving a file notmuch cannot see. QFile::rename(to, from); emit errorOccurred( QStringLiteral("Cannot index moved message in %1").arg(destFolder)); continue; } notmuch_database_remove_message(db, from.toUtf8().constData()); moved.append(id); } notmuch_database_close(db); notmuch_database_destroy(db); emit messagesMoved(moved, destFolder); } ``` - [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_notmuchworker ``` Expected: PASS, all tests. - [x] **Step 5: Verify the ordering mutation fails** Move the `notmuch_database_remove_message` call to immediately BEFORE the `notmuch_database_index_file` call and rebuild. `moveMessagesKeepsTheMessagesTags` must FAIL, because the tags are gone. Revert the mutation. This is the single most important check in the task: the wrong order silently destroys user tags and every other test still passes. - [x] **Step 6: Commit** ```bash git add src/notmuchworker.h src/notmuchworker.cpp tests/test_notmuchworker.cpp git commit -m "feat(worker): move messages between maildir folders The first mutation here that is not a notmuch tag. Indexes the new path before dropping the old one, since removing the last filename for a message id deletes the database entry and every tag on it." ``` --- ## Task 5: Route Delete through the move **Files:** - Modify: `src/mainwindow.h` (a `MoveCommand` beside the existing tag commands around line 1130) - Modify: `src/mainwindow.cpp` (the `delete` action at line 825) - Test: `tests/test_mainwindow.cpp` Read the existing `delete` action first. It is a toggle over the whole selection, resolved through `everySelectedRowHasTag()`, and `CLAUDE.md` records two separate bugs that lived in those three lines. Preserve the toggle: Delete twice still means "put it back". - [x] **Step 1: Write the failing test** ```cpp void TestMainWindow::deleteMovesTheMessageToTrash() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); const QString id = window.currentMessageId(); QVERIFY(!id.isEmpty()); window.triggerAction(QStringLiteral("delete")); // The file lands in the account's trash folder, and notmuch knows it // there. Asserting on the trash QUERY rather than on the tag: the tag is // the record of who deleted it, the folder is where it is. QTRY_VERIFY_WITH_TIMEOUT( window.countMatching(QStringLiteral("path:\"work/Trash/**\"")) == 1, 5000); QCOMPARE(window.countMatching(QStringLiteral("path:\"work/Inbox/**\"")), 0); } void TestMainWindow::deleteRecordsWhereTheMessageCameFrom() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); const QString id = window.currentMessageId(); window.triggerAction(QStringLiteral("delete")); // Restore needs a destination, and a Maildir filename does not record one. // Without this tag a restore three days later can only guess. QTRY_VERIFY_WITH_TIMEOUT( window.tagsOf(id).contains(QStringLiteral("deleted-from:Inbox")), 5000); QVERIFY(window.tagsOf(id).contains(QStringLiteral("deleted"))); } void TestMainWindow::undoMovesTheMessageBack() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); const QString id = window.currentMessageId(); window.triggerAction(QStringLiteral("delete")); QTRY_VERIFY_WITH_TIMEOUT( window.countMatching(QStringLiteral("path:\"work/Trash/**\"")) == 1, 5000); window.triggerAction(QStringLiteral("undo")); // Back to the exact folder it came from, not merely out of the trash, and // both tags stripped so the row does not still read as deleted. QTRY_VERIFY_WITH_TIMEOUT( window.countMatching(QStringLiteral("path:\"work/Inbox/**\"")) == 1, 5000); QCOMPARE(window.countMatching(QStringLiteral("path:\"work/Trash/**\"")), 0); QVERIFY(!window.tagsOf(id).contains(QStringLiteral("deleted"))); QVERIFY(!window.tagsOf(id).contains(QStringLiteral("deleted-from:Inbox"))); } ``` `WorkerBackedWindow` exists but may lack `countMatching()`, `tagsOf()` and `currentMessageId()`. Read its definition in `tests/test_mainwindow.cpp` and add whichever helpers are missing, following the form of the ones already there. `CLAUDE.md` records that the worker is unreachable by `findChild`, so wait on observable state with `QTRY_VERIFY_WITH_TIMEOUT` and never on worker signals or a fixed `qWait(n)`. - [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow deleteMovesTheMessageToTrash ``` Expected: FAIL, the message is still in Inbox because Delete only tags. - [x] **Step 3: Write minimal implementation** Add a `MoveCommand` in `src/mainwindow.h`, beside `MessageTagCommand` around line 1176, following its shape exactly: ```cpp /// Undo entry for a folder move. Separate from MessageTagCommand because the /// inverse of a move is another move, and the destination has to be carried /// rather than derived: a Maildir filename does not record where a message /// came from. class MoveCommand : public QUndoCommand { public: MoveCommand(MainWindow *window, const QStringList &messageIds, const QString &from, const QString &to, const QStringList &addedTags, const QString &text) : QUndoCommand(text) , m_window(window) , m_messageIds(messageIds) , m_from(from) , m_to(to) , m_addedTags(addedTags) { } void redo() override { m_window->sendMove(m_messageIds, m_to, m_addedTags, {}); } void undo() override { // Strips the tags the move added, so an undone delete does not leave a // row still reading as deleted. m_window->sendMove(m_messageIds, m_from, {}, m_addedTags); } private: MainWindow *m_window; QStringList m_messageIds; QString m_from; QString m_to; QStringList m_addedTags; }; ``` Declare `sendMove()` in `MainWindow`, near `sendMessageTagChange()`: ```cpp /// Moves messages to `destFolder`, applying `add` and `remove` in the same /// gesture. Invokable for the same reason sendThreadTagChange() is. Q_INVOKABLE void sendMove(const QStringList &messageIds, const QString &destFolder, const QStringList &add, const QStringList &remove); ``` Implement it in `src/mainwindow.cpp` beside `sendMessageTagChange()`, routing to the worker with the same queued-invocation form the neighbouring senders use, and applying the tags after the move reports success. Then rewire the `delete` action at line 825. Keep the toggle and the whole-selection direction; change only what each direction DOES: ```cpp addAction(QStringLiteral("delete"), tr("&Delete"), tr("Move to trash, or restore when already there"), [this]() { // Still a toggle over the whole selection, for the reasons the old // implementation recorded: one keystroke must not leave the selection // in two states. What changed is that each direction now MOVES the // file, and the tag is the record rather than the action. const bool everyRowDeleted = everySelectedRowHasTag(QStringLiteral("deleted")); if (everyRowDeleted) restoreSelected(); else trashSelected(); }); ``` Write `trashSelected()` and `restoreSelected()` as private helpers. `trashSelected()` resolves each selected row to its message id and its account (from the message's path prefix), reads the account's `trash` key, computes the origin folder from the current path, and pushes one `MoveCommand` carrying `deleted` and `deleted-from:`. An account with no `trash` key contributes nothing and reports through `statusMessage`, since Task 2 already warned at load. - [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow ``` Expected: PASS, all tests. - [x] **Step 5: Commit** ```bash git add src/mainwindow.h src/mainwindow.cpp tests/test_mainwindow.cpp git commit -m "feat: Delete moves mail to the account's trash folder" ``` --- ## Task 6: Restore from trash **Files:** - Modify: `src/mainwindow.cpp` (a `restore` action beside `delete`, and the icon table) - Modify: `src/keymap.cpp` (`knownActions()` and `defaultBindings()`) - Test: `tests/test_mainwindow.cpp` **Adding an action is four places**, and `CLAUDE.md` records that three of them are enforced by tests that fail confusingly: `KeyMap::knownActions()` (a `Q_ASSERT` fires otherwise, in whichever suite builds a `MainWindow` first), `defaultBindings()` (every action must be keyboard-reachable), and the icon table (every action must carry one). - [x] **Step 1: Write the failing test** ```cpp void TestMainWindow::restoreIsOnlyEnabledInTheTrashView() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); // Not in an ordinary view: restoring a message that is not in the trash // has no meaning, and an enabled action that does nothing is worse than an // absent one. QVERIFY(!window.action(QStringLiteral("restore"))->isEnabled()); window.runQuery(QStringLiteral("path:\"work/Trash/**\"")); QTRY_VERIFY_WITH_TIMEOUT(window.isShowingTrash(), 5000); QVERIFY(window.action(QStringLiteral("restore"))->isEnabled()); } void TestMainWindow::restoreReturnsAMessageToItsOriginFolder() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); const QString id = window.currentMessageId(); window.triggerAction(QStringLiteral("delete")); QTRY_VERIFY_WITH_TIMEOUT( window.countMatching(QStringLiteral("path:\"work/Trash/**\"")) == 1, 5000); window.runQuery(QStringLiteral("path:\"work/Trash/**\"")); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() == 1, 5000); window.selectFirstThread(); window.triggerAction(QStringLiteral("restore")); QTRY_VERIFY_WITH_TIMEOUT( window.countMatching(QStringLiteral("path:\"work/Inbox/**\"")) == 1, 5000); QVERIFY(!window.tagsOf(id).contains(QStringLiteral("deleted"))); } void TestMainWindow::restoreFallsBackToInboxWithoutAnOriginTag() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); // A message trashed by another client: in the trash folder, carrying no // deleted-from: tag, because nothing here put it there. There is one such // message in the real Maildir, so this is not a hypothetical. QVERIFY(window.addMessageInFolder(QStringLiteral("work/Trash"), QStringLiteral("foreign@example.org"))); window.runQuery(QStringLiteral("path:\"work/Trash/**\"")); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); window.triggerAction(QStringLiteral("restore")); QTRY_VERIFY_WITH_TIMEOUT( window.countMatching(QStringLiteral("path:\"work/Inbox/**\"")) == 1, 5000); } ``` - [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow restoreIsOnlyEnabledInTheTrashView ``` Expected: FAIL, there is no `restore` action. - [x] **Step 3: Write minimal implementation** Add `"restore"` to `KeyMap::knownActions()` and give it a binding in `defaultBindings()`. Check what is free first: ```bash grep -n 'defaultBindings' -A40 src/keymap.cpp ``` Add an entry to the icon table in `src/mainwindow.cpp` (search for `QIcon::fromTheme` near the other actions), using a theme icon as the toolbar and menus must keep doing. Then the action itself, beside `delete`: ```cpp addAction(QStringLiteral("restore"), tr("&Restore from trash"), tr("Move the selected messages out of the trash"), [this]() { restoreSelected(); }); ``` Enable it from wherever the current query is applied, so it follows the view: ```cpp // Only meaningful on mail that is actually in a trash folder. Enabled from // the QUERY rather than from the selection's tags: a message trashed by // another client carries no tag of ours and must still be restorable. m_actions[QStringLiteral("restore")]->setEnabled(isShowingTrash()); ``` `isShowingTrash()` compares the current resolved query against the `trash` generator's, for the current account selection. `restoreSelected()` reads each row's `deleted-from:` tag for its destination and falls back to `Inbox`, reporting which through `statusMessage`. - [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow ctest --test-dir build --output-on-failure ``` Expected: PASS, 24 of 24. The keymap and icon-table tests fail loudly if a place was missed. - [x] **Step 5: Commit** ```bash git add src/mainwindow.h src/mainwindow.cpp src/keymap.cpp tests/test_mainwindow.cpp git commit -m "feat: restore mail from the trash view" ``` --- ## Task 7: The cleanup menu entry **Files:** - Modify: `src/mainwindow.cpp` (a `cleanup_stranded` action, added to a menu and NOT to the query row) - Modify: `src/keymap.cpp` (`knownActions()` and `defaultBindings()`) - Test: `tests/test_mainwindow.cpp` The user's constraint, verbatim: "the cleanup should be a menu entry only, not to be confused with the filter Trash". Do not add a sixth button. - [x] **Step 1: Write the failing test** ```cpp void TestMainWindow::theCleanupQueryFindsStrandedMail() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); const QString id = window.currentMessageId(); // Tag without moving: exactly the state the old Delete left mail in, and // the state 848 real messages are in today. window.sendMessageTagChangeForTesting({ id }, { QStringLiteral("deleted") }, {}, QStringLiteral("Strand")); QTRY_VERIFY_WITH_TIMEOUT( window.tagsOf(id).contains(QStringLiteral("deleted")), 5000); window.triggerAction(QStringLiteral("cleanup_stranded")); // The stranded message is in the list; mail already in the trash is not, // since it needs no cleanup. QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() == 1, 5000); QVERIFY(window.currentQuery().contains(QStringLiteral("tag:deleted"))); QVERIFY(window.currentQuery().contains(QStringLiteral("not"))); } void TestMainWindow::theCleanupQueryExcludesMailAlreadyInTrash() { WorkerBackedWindow window; QVERIFY(window.isValid()); window.show(); QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() > 0, 5000); window.selectFirstThread(); window.triggerAction(QStringLiteral("delete")); QTRY_VERIFY_WITH_TIMEOUT( window.countMatching(QStringLiteral("path:\"work/Trash/**\"")) == 1, 5000); window.triggerAction(QStringLiteral("cleanup_stranded")); // Properly trashed mail carries the tag AND sits in the folder, so it must // not appear here. Without the exclusion this reports every deleted // message ever, which makes the action useless the moment it works. QTRY_VERIFY_WITH_TIMEOUT(window.threadCount() == 0, 5000); } ``` - [x] **Step 2: Run test to verify it fails** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow theCleanupQueryFindsStrandedMail ``` Expected: FAIL, there is no `cleanup_stranded` action. - [x] **Step 3: Write minimal implementation** Register the action in `knownActions()`, `defaultBindings()` and the icon table as in Task 6, then: ```cpp addAction(QStringLiteral("cleanup_stranded"), tr("Find &stranded deleted mail"), tr("Show mail tagged deleted that is not in a trash folder"), [this]() { // Repeatable, never a startup migration: the user asked for something // they could come back to. It reports what it finds and moves nothing. const QString trash = m_config.allTrashQuery(); const QString query = trash.isEmpty() ? QStringLiteral("tag:deleted") : QStringLiteral("tag:deleted and not (%1)").arg(trash); runQuery(query); statusMessage(tr("Mail tagged deleted but not in a trash folder. " "Select what should go and press Delete.")); }); ``` Add it to a menu, not to the query row. Find where the other menu entries are built and follow that form. - [x] **Step 4: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ./build/tests/test_mainwindow ``` Expected: PASS. - [x] **Step 5: Commit** ```bash git add src/mainwindow.h src/mainwindow.cpp src/keymap.cpp tests/test_mainwindow.cpp git commit -m "feat: find mail tagged deleted but never moved to trash" ``` --- ## Task 8: Translations and changelog **Files:** - Modify: `translations/qtmaildir_it_IT.ts` - Modify: `CHANGELOG.md` Every user-facing string added above needs a translation, and `lrelease` silently DROPS an unfinished string and ships it as English inside an otherwise Italian UI. Item 108 shipped fifteen strings that way. - [x] **Step 1: Refresh the translation source** ```bash lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations none ``` Expected: a clean run reporting zero context warnings. A "tr() cannot be called without context" warning means a literal needs `QT_TRANSLATE_NOOP("TheClass", "Text")` rather than `tr()`. - [x] **Step 2: Translate every new string** Open `translations/qtmaildir_it_IT.ts` and fill in each ``. The new strings are the Trash filter name, the Restore and cleanup action texts and tooltips, the trash config warning, and the status messages. Do NOT translate notmuch query syntax. `tag:deleted`, `path:` and `deleted-from:` are wire format. - [x] **Step 3: Verify nothing is unfinished** ```bash lrelease-qt6 translations/qtmaildir_it_IT.ts ``` Expected: "Generated N translation(s) (N finished, 0 unfinished)". A nonzero unfinished count means a string will ship as English. - [x] **Step 4: Run the translations test** ```bash ctest --test-dir build -R translations --output-on-failure ``` Expected: PASS. - [x] **Step 5: Write the changelog entry** Under `## [Unreleased]` in `CHANGELOG.md`, with an `### Upgrading` section, since a working config now warns until five keys are added: ```markdown ### Added - Delete now moves mail into the account's trash folder instead of only tagging it. A `Trash` filter sits beside Unread, Inbox, Important and Sent, and composes with the account selector like the others. - `Restore from trash`, available while the trash view is showing. A message this application deleted returns to the folder it came from; one trashed by another client returns to the inbox. - `Find stranded deleted mail`, in the menu, listing mail tagged `deleted` that never moved. Run it whenever you like; it moves nothing on its own. ### Upgrading Every account now needs a `trash` key in `qtmaildir.conf`, naming its trash folder relative to `maildir`: [account.work] maildir = work trash = Trash The folder must be one your `mbsync` configuration actually syncs, or the move will never reach the server. Accounts without the key still load and still read mail, but Delete cannot work on them and a warning says so at startup. Mail deleted by earlier versions carries the `deleted` tag and sits wherever it was. It is not migrated automatically. Use `Find stranded deleted mail` to review it and Delete to move what should really go. Note that Delete's reversibility depends on your provider: a trash folder that the provider purges on a timer will eventually remove the mail for good. ``` - [x] **Step 6: Commit** ```bash git add translations/qtmaildir_it_IT.ts CHANGELOG.md git commit -m "i18n: translate the trash strings, and document the trash key" ``` --- ## Task 9: Full verification - [x] **Step 1: Clean build** ```bash rm -rf build cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug cmake --build build ``` Expected: no warnings from the changed files. - [x] **Step 2: Full suite** ```bash ctest --test-dir build --output-on-failure ``` Expected: 24 of 24 passing. - [x] **Step 3: Confirm the spec's claims hold** Re-read `docs/superpowers/specs/2026-08-17-delete-to-trash-design.md` and check each testing bullet has a test. The spec lists six; all six are covered by Tasks 1, 3, 4, 5, 6 and 7. - [x] **Step 4: Hand the build to the user** Do NOT run `./build/src/qtmaildir`. Report what to look at: - Delete on a message, then look in the trash view for it. - Undo, and confirm it returns to the folder it came from. - The trash view with the account dropdown on each account and on All accounts. - `Find stranded deleted mail`, which should list a large number of messages on the account that has them, and nothing on the others. - Delete on a message whose account has no `trash` key, which should report rather than silently doing nothing. The startup warning about missing `trash` keys will appear until the config is updated; that is Task 8's `### Upgrading` note in action.