diff options
| -rwxr-xr-x | assets/hooks/post-new | 27 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 191 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 498 | ||||
| -rw-r--r-- | src/composecontext.cpp | 23 | ||||
| -rw-r--r-- | src/maildirname.cpp | 61 | ||||
| -rw-r--r-- | src/maildirname.h | 24 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 24 | ||||
| -rw-r--r-- | src/nmraii.h | 2 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 104 | ||||
| -rw-r--r-- | tests/test_maildirname.cpp | 96 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 78 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 133 |
12 files changed, 1071 insertions, 190 deletions
diff --git a/assets/hooks/post-new b/assets/hooks/post-new index 5102103..428ec29 100755 --- a/assets/hooks/post-new +++ b/assets/hooks/post-new @@ -104,10 +104,21 @@ def strip_inbox_from_sent(run): return True query = f"{SCOPE} and ({qtmaildirconf.sent_query(folders)})" + + # Counted BEFORE the tag, because the tag is what makes the count zero. + # A `notmuch tag` that matches nothing SUCCEEDS, so the old log line said + # "applied" whether it stripped four messages or none, and item 164 is + # exactly the case where that distinction is the whole question: a draft + # kept `inbox` on a pass whose log claimed the carve-out had run. The + # count is the only thing that separates "the tag ran and something + # re-added inbox afterwards" from "the message was never in scope". + matched = count(query) + if not run(["-inbox"], query): return False - log(f"sent-folder carve-out applied over {len(folders)} folder(s)") + log(f"sent-folder carve-out applied over {len(folders)} folder(s), " + f"{matched} message(s)") return True @@ -116,6 +127,20 @@ def protected_removals(rule): return sorted(PROTECTED_REMOVALS.intersection(rule.remove)) +def count(query): + """How many messages a query matches, or `?` if the count itself failed. + + Diagnostic only: nothing branches on this. A failure here must not fail + the sync, because the carve-out's own tag is what matters and it reports + its own status separately. + """ + result = subprocess.run(["notmuch", "count", "--", query], + capture_output=True, text=True) + if result.returncode != 0: + return "?" + return result.stdout.strip() or "?" + + def run_tag(arguments, query): result = subprocess.run(["notmuch", "tag"] + arguments + ["--", query], capture_output=True, text=True) diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 2e941ec..f2b977b 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -7751,3 +7751,194 @@ new action and the existing ones gathered under it rather than duplicated. - **Item 160 unblocked this** on 2026-08-25: the status bar exists, so a manual save reports through `refreshDraftStatus()` like an autosave. Route `Ctrl+S` through `saveDraftNow()` and the reporting is already done. + +## 162. Delete fails while a sync is renaming the file underneath it + +**Observed (user, 2026-08-25):** deleting a draft reported `Cannot move +<file> to <account>/Trash`. + +**Cause (verified against the live Maildir, not read):** a stale path, and +neither Delete nor item 158 is at fault. + +1. The composer autosaves a draft as `<name>:2,D` and item 158 indexes it + under exactly that filename. +2. **mbsync uploads it and RENAMES it** to `<name>,U=<uid>:2,D`, recording the + server UID in the filename. +3. notmuch still holds the pre-`U=` name until that sync's `notmuch new` runs. +4. `moveMessages()` reads the filename from notmuch and calls + `QFile::rename()` on a path that no longer exists. It fails, the error is + emitted, and the message is skipped. + +Measured, in this order: `notmuch search --output=files` named a file that was +not on disk while `Background sync running...` was up, and the same query was +clean once the sync finished, with the file present under its new `,U=4` name. +That is why it reads as intermittent, and why it heals itself. + +**It is truthful and it loses nothing.** The move is skipped, no wrong folder +is created, no file is destroyed, and the next sync reconciles. The defect is +that the message blames a folder for a timing problem, and that the action +silently does nothing when the user asked for something. + +**This is `CLAUDE.md`'s `,U=` trap from the other side.** `MaildirName::fresh()` +exists because CARRYING that infix across a folder boundary produced +`Maildir error: duplicate UID` on real mail. Here mbsync is ADDING it and the +index lags; the same infix, the opposite direction. + +**Approach, and it needs a decision.** Two candidates: + +- **Refuse the move while a sync holds the lock.** `SyncMonitor` already + reports this, and the held-edit machinery from items 97 and 106 already + exists for exactly this shape: a tag edit made during a sync is held and + flushed when it ends. Delete would join it rather than inventing anything. + This is the likelier right answer, since it matches what every other + mutation already does. +- **Re-resolve the filename** from notmuch immediately before the rename and + re-query the message if the path is gone. Smaller, but it races the same + window it is trying to close, and a second lookup can be stale by the time + it is used. + +**Constraints.** + +- **Delete reaches the real mail server.** Read `CLAUDE.md`'s item 103 notes + before touching `moveMessages()`: a wrong folder name is created, adopted by + mbsync, and propagated to every other client. +- Whatever is built, **the message must say a sync is running**, not name a + folder. The current wording sent the user looking for a broken folder + configuration, which was correct and configured. +- A test cannot see this in the ordinary fixture layout, where nothing renames + a file underneath the index. Driving it means renaming the file between the + index write and the move, which is what the reproducer has to do. + +**Fixed 2026-08-25.** `moveMessages()` re-resolves by MESSAGE ID when the +recorded path is gone: one `reindexFolder()` of that directory, then the +filename from `notmuch_message_get_filenames()` that exists on disk. Bounded +to a single retry, so a genuinely missing file still reports rather than +becoming a silent no-op. + +**The hold candidate above was investigated and rejected**, and the reason +matters more than the fix. `sendMove()` ALREADY refuses while a sync holds the +lock and queues onto `m_heldMoves` (items 97 and 106 built it), yet the defect +still fired. `aSyncHoldsTheWriteLock()` tracks notmuch's write lock, while this +window sits between mbsync's RENAME and that sync's `notmuch new`; mbsync +renames throughout its run without touching that lock, so the damaging window +is open when there is nothing to observe. Refusing on the lock guards the wrong +resource. That refusal is correct for its own purpose and was left alone. + +Covered by `moveMessagesRecoversWhenASyncRenamedTheFile()` and +`moveMessagesStillReportsAMessageThatIsReallyGone()`, the second pinning the +bounded half. The test renames without reindexing, which is the window mbsync +opens, and asserts the database still names the old path so it cannot pass +against a fixture that quietly reindexed. Mutation-checked. + +## 163. The message pane shows a stale path, and the composer forks the draft + +**Observed (user, 2026-08-25):** selecting a draft filled the pane with +`(unreadable message)` and `This message could not be parsed.`, naming a file +under the account's drafts folder. + +**Cause (verified against the live Maildir):** the path in the pane ended +`.dnx:2,D`, and the only file on disk ended `.dnx,U=4:2,D`. Same mechanism as +item 162: mbsync renames an uploaded file to record its server UID, and the +name the application is holding stops existing. + +**The site is different, and so is the fix.** Item 162 is the WRITE path, +`moveMessages()` reading a filename from notmuch. This is the READ path, and +by the time it fires notmuch is already CORRECT: measured, the index named the +`,U=4` file while the pane still named the pre-`U=` one. The stale path is the +MODEL's, cached when the row was loaded, so refusing to act while a sync runs +(162's likely fix) would not help here at all. + +**The report is honest, which is why it is confusing.** `MimeParser` opened a +path that did not exist and said so. Nothing is lost and the next query +repairs it. + +**Approach.** The read path should RECOVER rather than refuse: on a failed +parse, re-resolve the message id through notmuch and retry once before +reporting. `recoverStaleThread()` already exists for the neighbouring problem +(item 91 reuses it) and is the shape to follow. + +**A SECOND site, found 2026-08-25 while hand-testing item 164, and its +consequence is worse than the pane's.** `MainWindow::openComposerFor()` passes +`ref.filePath` to `ComposeContextBuilder::forDraft()`, and `MessageRef::filePath` +is `notmuch_message_get_filename()` captured when the QUERY ran. After mbsync +renames an uploaded draft, a row loaded before that sync names a file that no +longer exists, `MimeParser::parse` fails, `forDraft` returns an empty context +and the composer reports "That draft could not be read". + +Measured, in order: draft written 10:42, sync at 10:50:43 logged "Detected 2 +file renames" and the file became `,U=5`, the user reopened it at 10:52 and +saw the error. + +**The pane's version is cosmetic and self-repairing. This one silently forks +the draft.** The refusal happens BEFORE `openComposer()`, so the user is left +with no composer for that draft. Composing again opens a FRESH one with +`m_draftPath` empty, so its first autosave has no previous path to unlink: +it writes a new file, `DraftStore` unlinks nothing, and the old revision +stays. Both are then uploaded and both reach the server. + +Two properties make this worse than it first reads. Each save mints a NEW +Message-ID (four saves in the hand test produced four ids), so the revisions +are distinct MESSAGES to notmuch rather than two files of one, and no dedup +anywhere will collapse them. And the unlink machinery is entirely correct +throughout, which is why the code reads fine: `forDraft` sets `draftPath`, +the composer seeds `m_draftPath`, the autosave passes it as `previousPath`, +and `DraftStore::write` unlinks it. The comment at `composecontext.cpp:501` +already names this exact failure ("one message becomes two"). None of it runs, +because the reopen was refused before any of it was reached. + +So the fix below covers this site too, and fixing only the pane would leave +the draft-forking half in place. + +**Constraints.** + +- **A retry must be bounded.** A message that genuinely cannot be parsed + (item 41's territory) must still report, or a real defect becomes an + infinite loop. +- Re-resolving by id is what makes this safe; re-scanning the folder is not, + since two files can carry the same id. +- The placeholder wording is correct and should stay for the genuine case. +- **The composer site must recover, not merely report better.** A clearer + error still leaves the user composing a second copy of their own draft. +- Whether a draft should keep a STABLE Message-ID across revisions is a + separate question this entry does not decide. It is what turns a stale path + into two server-side messages rather than one replaced file, and it wants + its own item; note that a draft's id is not yet the sent message's id, so + changing it is not obviously free. + +**Fixed 2026-08-25.** `MaildirName::resolveRenamed()` answers the filesystem +question: the path unchanged when it still exists, otherwise the file in that +same directory whose unique stem matches. mbsync preserves the stem +(`<stem>:2,D` becomes `<stem>,U=5:2,D`), which is what makes resolving by +filename safe here at all. It never recurses and never crosses a folder +boundary; a file that changed FOLDERS is a different question that only the +message id can answer, and `moveMessages()` re-resolves that way for item 162. + +Two refusals in it are deliberate and both have tests. An ambiguous match (two +files sharing a stem, which a correct Maildir cannot produce) yields nothing +rather than a guess, because opening or moving the wrong message is worse than +reporting none. And a genuinely missing file yields nothing too, so a real +deletion still reports instead of becoming a wrong answer. + +**Three call sites, not the one this was filed for.** The pane +(`renderMessages`), Reply and Forward (`openComposerFor`), and the draft reopen +(`forDraft`). The third is the one that cost data, and its shape is worth +keeping: the refusal happened BEFORE any composer existed, so the user composed +again into a fresh window whose autosave had no `previousPath` to unlink. The +unlink machinery was correct at every step and simply never ran. + +`forDraft()` seeds `draftPath` from the RESOLVED path. Seeding the caller's +would let the reopen succeed and the unlink still miss, which is the same fork +arriving one step later; the integration test asserts on `draftPath` for +exactly that reason. + +Covered by five unit tests on the resolver and by +`aDraftRenamedByASyncStillReopensAndReplacesItsFile()`, which renames the draft +the way mbsync does and asserts the file COUNT, the shape the fork actually +takes. Mutation-checked: removing the resolution fails it with the reported +symptom. + +**Left undecided, deliberately:** each save mints a NEW Message-ID, which is +what turns a stale path into two server-side MESSAGES rather than one replaced +file. That wants its own item. A draft's id is not yet the sent message's id, +so changing it is not obviously free. + diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 6041e28..f1db28a 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -170,7 +170,7 @@ taking that too literally. | 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | open; item 49 built the edit half deliberately. Needs a decision, see the entry | | 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | **done** 2026-08-17, unreleased. A Note column before `ColumnCount`, so the appended Matches column stays last. Found a second defect on the way: `restoreState` REFUSES a header state with a different column count, and the sized flags were being set regardless | | 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S+M | done; Delete moves to the account trash, with Restore and a stranded-mail cleanup. Section in the closed file | -| 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | ? | open, reported 2026-08-16, cause NOT established. Most likely outside this repo; see the entry before writing code | +| 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | XS | **fixed 2026-08-25**, awaiting hand test. The worker never reopened its read-only notmuch handle, so no query saw mail indexed after startup | | 109 | A root card's own message is invisible to a message-scoped write | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 108. `applyMessageTagChange` and `messageById` searched only the loaded replies, and a root's message is never among them, so the ORDINARY gesture repainted nothing and wiped the pane's chip row | | 110 | A card and the message pane show tags belonging to a message's siblings | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 109 against a real 4-message thread. `ThreadSummary::tags` is notmuch's UNION; a card standing for one message drew it. Also the reason a root card could not repaint at all | | 111 | A card should show its siblings' tags smaller, not drop them | presentation | S | **done** 2026-08-16, unreleased. The user's own design, from looking at 110's result: own tags full size, the thread's others smaller and muted, so nothing appears to vanish on selection | @@ -235,9 +235,11 @@ taking that too literally. | 160 | The composer never says a draft was autosaved | feedback | S | **done** 2026-08-25, unreleased. A status bar on the composer: the age line left, the `○ unsaved content` cue beside it. **The fix is a funnel, not a label.** `m_dirty` had SEVEN writers and four of them clear it, only two of which are a save, so a cue hung off the save path silently missed the constructor and the send; `setDirty()` is the one writer now and refreshes both cues plus `setWindowModified()`. Presentation was **reworked after the user looked at it**: it first reused item 151's yellow ribbon treatment, which reads as a misplaced widget on a bare status label, and the cue sat in the permanent (right-hand) tray. Two defects found by probing rather than by reading, see the section | | 161 | The composer has no menu bar | discoverability | S | **done** 2026-08-25, unreleased. File / Edit / Format, to the user's own chosen scope. **Save draft (`Ctrl+S`) is the only NEW action**; everything else is gathered, and the menus show the toolbar's own `QAction` objects rather than copies, per item 140's rule. Two needed hand-building: the HTML toggle is a `QToolButton` and cannot go in a menu, so a checkable twin mirrors it BOTH ways; and the signature entry takes the switch's own `QMenu` pointer, since that menu is rebuilt when the signatures change and copied entries would go stale. Edit's entries follow the editor's own `undoAvailable`/`copyAvailable`. `theMenuBarReachesEveryComposerAction()` is item 132's rule applied to the composer, walking the real menu bar and finding actions by `findChildren`, so a future action added to the toolbar and forgotten in the menus fails without touching the test | -| 162 | Delete fails while a sync is renaming the file underneath it | defect | S | open, 2026-08-25, found by hand. `Cannot move <file> to <folder>`. NOT a Delete defect and not item 158's: mbsync renames an uploaded file to add its `,U=<uid>` infix, and notmuch keeps the pre-`U=` name until that sync's `notmuch new` runs, so `moveMessages` renames a path that no longer exists. Truthful, harmless and SELF-HEALING, which is why it reads as intermittent: verified a ghost present mid-sync and gone after. The message names a folder as though the folder were the problem. Delete reaches the real mail server, so read `CLAUDE.md` before touching it | -| 163 | The message pane shows a stale path and reports the message unreadable | defect | S | open, 2026-08-25, found by hand. Same root as 162 and a DIFFERENT site: the model keeps the filename a row was loaded with, mbsync renames the file to add `,U=<uid>`, and `MimeParser` then opens a path that no longer exists and honestly reports "could not be parsed". notmuch is CORRECT by then; the UI is behind, so 162's fix (refuse the write while a sync runs) does not touch this. The read path needs to recover rather than refuse | -| 164 | Every newly synced draft carries `inbox` | defect | S | open, 2026-08-25, found by hand. Measured `draft inbox unread` on a draft this application wrote. `strip_inbox_from_sent()` in `assets/hooks/post-new` reads `qtmaildirconf.sent_folders()` only, and `qtmaildirconf.py` has no drafts equivalent, so the carve-out never covers a drafts folder. **Contradicts a shipped 0.27.0 changelog entry** claiming both are kept out of the inbox, so it is a documentation defect as well. Item 158's measurement was correct and did not cover this: `index_file` assigns no tags, but mbsync's upload and the next `notmuch new` re-tag the file | +| 162 | Delete fails while a sync is renaming the file underneath it | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix and notmuch keeps the pre-`U=` name until that sync's `notmuch new` runs, so `moveMessages` renamed a path that no longer existed and Delete silently did nothing while blaming the destination folder. `moveMessages` now re-resolves by MESSAGE ID when the recorded path is gone: one reindex of that directory, then the filename that exists on disk. Bounded to one retry, so a file genuinely gone still reports. Holding the move during a sync was the other candidate and is NOT the fix: `sendMove` already refuses on notmuch's write lock, but this window sits between mbsync's rename and that sync's `notmuch new`, which touches no lock | +| 163 | The message pane shows a stale path, and the composer forks the draft | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix while the model still holds the name the query returned. `MaildirName::resolveRenamed()` returns the path unchanged when it exists, else finds the file in that one directory whose unique stem matches; it refuses an ambiguous match and yields nothing for a genuinely missing file. Wired into all THREE read sites: the pane, Reply/Forward, and the draft reopen. The reopen was the one that cost data, forking a draft into two files with two Message-IDs, both reaching the server | +| 164 | A draft this application saved keeps `inbox` | defect | S | open, 2026-08-25, **cause corrected 2026-08-25**. The first diagnosis blamed a missing drafts helper and was WRONG: `NOT_ARRIVALS` in `qtmaildirconf.py` is `("sent", "drafts")`, the folder list includes every account's drafts folder, and `notmuch count` confirms the carve-out query MATCHES the affected draft. The carve-out is scoped to `tag:new`, and the draft carries `inbox` while `tag:new` is 0, so it was never in scope when the hook ran. Measured separately: an mbsync-style rename does NOT re-add `new.tags`, so the retag theory is out too. What remains unestablished is WHICH pass tagged it; establish that before writing code | +| 165 | A draft gets a new Message-ID on every autosave | enhancement | ? | open, 2026-08-25, found while hand-testing 163 and 164. `MessageBuilder::build()` generates an id unconditionally and every autosave calls it, so each revision is a distinct MESSAGE to notmuch and to the server rather than a new version of one. Invisible while the file is replaced correctly, which item 163's fix restores; it is what turned that fork into two messages rather than one duplicated file. Needs a DECISION on what a draft's identity is before any code: a stable id reused at send, a stable id discarded at send, or the status quo. Neither `ComposeContext` nor `OutgoingMessage` has a field to carry an id, so it is not a changed call site | +| 166 | Mail you send to your own other account loses `inbox` | defect | S | open, found 2026-08-25. Two-repo change: the carve-out lives in the `post-new` hook, mirrored in mailctl's `mailrules.py` | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -540,58 +542,157 @@ reaches it (item 42), so most of this exists. New mail received on thunderbird did not appear in qtmaildir. Need to investigate further." -**Cause: NOT established.** Recorded because it is a defect report about mail -going missing, which is the most serious kind this backlog carries, and it has -been sitting in the notes unrecorded. What follows is one measured mechanism that -would produce exactly this symptom, not a diagnosis. - -**qtmaildir cannot show what mbsync did not fetch, and mbsync fetches folders by -pattern.** Three of the five channels in the user's `~/.mbsyncrc` name their -folders explicitly: - -``` -Patterns "INBOX" "[Gmail]/Posta inviata" "[Gmail]/Bozze" "[Gmail]/Speciali" -``` - -and one names only `"INBOX"`. The two non-Gmail channels use `Patterns *`. -Gmail applies labels, and a message whose label is not one of those four is in a -folder mbsync never asks for. Thunderbird speaks IMAP directly and sees every -folder, so the same message is visible there and absent locally. This is a -configuration property of the user's mbsyncrc, outside this repository entirely. - -**One inconsistency worth reporting regardless**, found while checking the -above: one of the Gmail accounts is configured in `qtmaildir.conf` with -`sent = [Gmail]/Posta inviata` and `drafts = [Gmail]/Bozze`, while its mbsync -channel has `Patterns "INBOX"` and fetches neither. The Sent and Drafts filters -for that account can therefore only ever be empty. That is real, and it is -independent of whatever this item turns out to be. - -**Approach.** Reproduce before anything else, and the reproduction has to -distinguish three layers, because the fix lives in a different place for each: - -1. Is the message on disk? `find` in the Maildir, or `notmuch count` on a term - from it. If not, this is mbsync or `.mbsyncrc`, and there is nothing to - change here. -2. If it is on disk, is it indexed? `notmuch new` and count again. If not, this - is notmuch config, `new.ignore` or the hook. -3. Only if it is indexed and still not shown is this qtmaildir's defect, and - then the question is which query hid it: the account scope, the built-in - filter, or a rule that tagged it out of the inbox. +**Cause: ESTABLISHED 2026-08-25, and it was this repository after all.** This +entry previously named mbsync's folder `Patterns` as the leading theory and +concluded "most likely not a code change here at all". That was wrong; the +superseded reasoning is kept at the bottom because the reproduction is what +overturned it. + +**`NotmuchWorker` opened one read-only notmuch handle and kept it for the +process lifetime.** A read-only handle is a Xapian SNAPSHOT taken when it is +opened; it never observes a write made by another process afterwards. The sync +script's `notmuch new` is exactly such a process, so every query the worker +answered after startup was served from the index as it stood when the +application launched. `openReadOnly()` returned early on `if (m_db) return +true;` and there was no `notmuch_database_reopen` anywhere in the tree. + +This accounts for every symptom, including the ones that defeated three earlier +theories during the diagnosis: + +- The post-sync refresh found nothing, so `refreshCurrentQuery()` and + `ThreadListModel::reconcile()` were each suspected in turn. Both are correct. +- A query the user typed BY HAND also found nothing. That is what rules out the + model, the generation counter and the account scope together: a fresh query + clears the model and re-runs from scratch, and it still hits the same stale + handle. +- A restart showed the mail instantly, with no sync in between. +- Tag WRITES were never affected, which is why the defect reads as "reading is + broken" rather than "notmuch is broken". `applyTags` opens its own read-write + handle per call, so it always sees current data. + +**Why it survived from 2026-08-16 to 2026-08-25.** The symptom needs mail to +arrive from outside the process while the window stays open, which is the +ordinary way this application is used and the one thing no test did: every +fixture opens a worker, queries it, and drops it. `TestNotmuchWorker::runQuery()` +builds a FRESH worker per call, so the suite was structurally incapable of +reproducing it, and a test written through that helper passes against the bug. + +**Fixed** in `NotmuchWorker::openReadOnly()`: when a handle already exists, +`notmuch_database_reopen(m_db, NOTMUCH_DATABASE_MODE_READ_ONLY)` before +returning it. Every read path begins by asking for the handle, so one call +covers all of them; putting it at the call sites instead would be one more +place to forget. A reopen failure is deliberately NOT fatal, since the existing +handle is still usable and answering from a slightly stale index beats refusing +to answer at all. + +Covered by `aQuerySeesMailIndexedAfterTheWorkerOpened`, which holds ONE worker +across two queries and runs `notmuch new` in a second process between them. +Mutation-checked: `after.size()` is 0 without the fix, 1 with it. The first +query asserts zero results before the message is written, so "found nothing" +cannot mean "the query was malformed". + +**The reproduction, kept because this entry's Approach section asked for exactly +this and it took three wrong turns to get there.** Four messages sent to one +account on 2026-08-25, viewed in that account's Inbox, synced with the app's own +Sync button. The three layers resolved as: on disk (yes), indexed (yes), shown +(no), which is layer 3 and therefore this repository. Two of the four matched +the running view's exact query (`path:"<account>/**" and (tag:inbox)`, 2 results +from the shell) and were absent from a window that had been open across the +sync. + +Two measurement errors made during that diagnosis, both worth repeating because +each produced a confident wrong answer: + +- `notmuch count 'inbox and path:...'` was used to check the view's contents. A + bare `inbox` is a FREE-TEXT term, not a tag term; the app generates + `tag:inbox`. The bare form returned 0 where the real query returns 2, which + briefly made the defect look like a tagging problem. +- The messages' tags were first read across every file matching the subject, + including the sender-side Sent copies in other accounts. That mixed three + accounts' messages into one answer. + +**Superseded theory, kept for the record.** mbsync fetches Gmail folders by +pattern and three of the five channels name their folders explicitly, so a +message labelled anything else is in a folder mbsync never asks for while +Thunderbird, speaking IMAP directly, sees it. That mechanism is real and would +produce a similar symptom, but it is not what was happening here: the mail was +on disk and indexed. It remains a plausible cause of any FUTURE report of this +shape, so check layer 1 before assuming this fix covers it. + +**One inconsistency worth reporting regardless**, found while checking the above +and still true: one of the Gmail accounts is configured in `qtmaildir.conf` with +a sent and a drafts folder, while its mbsync channel has `Patterns "INBOX"` and +fetches neither. The Sent and Drafts filters for that account can therefore only +ever be empty. That is real, independent of this item, and outside this +repository. + +**Size: XS.** Done. + + + +## 166. Mail you send to your own other account loses `inbox` + +**Observed (agent, 2026-08-25, while setting up msmtp.)** Four test messages +were sent to one of the user's own accounts, one from each configured sending +account. All four were delivered and indexed. The two sent from accounts whose +Sent folder is fetched locally arrived in the recipient account's Inbox +**without the `inbox` tag**, so they were absent from that account's Inbox view. +The two sent from an account whose Sent folder is not fetched kept `inbox` +normally. + +**Cause: established, and it is the `post-new` hook, not this binary.** +`strip_inbox_from_sent()` in `assets/hooks/post-new` removes `inbox` from any +message matching a configured sent folder's PATH. Its docstring states the +assumption exactly: "the provenance is the file's own path: a message inside a +configured sent folder is one this system sent, and `inbox` was never true of +it." + +That holds for one file. It fails for one MESSAGE, because **notmuch +deduplicates by Message-ID and a message can have several files**. When the +sender and the recipient are both the user's own accounts, mbsync fetches two +copies: the sender's Sent copy and the recipient's Inbox copy. notmuch stores +them as ONE message with two filenames. The carve-out's query matches via the +Sent filename and strips `inbox` from the message object, which is the same +object the recipient's Inbox copy belongs to. + +Measured: one message, two paths, one in the sender account's sent folder and +one in the recipient account's `Inbox/cur`. + +The assumption is not merely incomplete, it is false in this case: the message +was genuinely sent AND genuinely received. There is no single right answer for +"was `inbox` ever true of this message", because it was true of one file and +false of another. + +**Approach.** Not settled, and the choice matters more than the code: + +1. **Strip only when EVERY file is in a sent folder.** Closest to the existing + intent, and it makes the predicate match the docstring's claim. A + self-addressed message keeps `inbox`, which is right: it did arrive. +2. **Strip only when the message has exactly one file.** Simpler to express, + but it silently stops protecting any sent message that happens to be + duplicated for an unrelated reason. +3. **Leave it.** Self-addressed mail is rare outside testing. The cost is that + it is invisible when it happens, and it looks exactly like the sync defect + item 104 turned out to be, which is how this was found. + +Option 1 is the one that makes the code true to what it already says it does. **Constraints.** -- Ask the user for one concrete example before investigating: which account, - roughly when, and what Thunderbird shows for it. A general "sync doesn't work" - cannot be reproduced, and the last four defects in this backlog were all found - from a specific message. -- The `post-new` hook from mailctl tags mail unattended. A rule that removes - `inbox` would make a correctly fetched, correctly indexed message vanish from - the default view, which looks identical to a sync failure from the outside. - `notmuch search` without a filter is what tells them apart. -- Do not change `.mbsyncrc` as part of this. It is the user's, it is outside the - repo, and a Patterns change refetches folders. +- **This is a two-repo change.** The carve-out is in this repo's `post-new`, + but the shared rule machinery is mirrored in `../mailctl`. Read "Changing the + shared rule format" in CLAUDE.md before touching it, and run both suites. +- The hook **tags real mail unattended, every ten minutes, on the user's live + index.** A predicate that is wrong in the other direction would strip `inbox` + from arriving mail, which is the failure mode PROTECTED_REMOVALS exists to + prevent. Test against a throwaway database first. +- `notmuch tag` matching zero messages SUCCEEDS, so a log line saying the + carve-out ran is not evidence it matched anything. The count added for item + 164 is what distinguishes them; use it. +- Do not fix this by narrowing the query to exclude the recipient account. The + bug is in the per-file predicate, not in which folders are configured. -**Size: `?`** until reproduced. Most likely not a code change here at all. +**Size: S.** ## 112. Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread @@ -1341,138 +1442,173 @@ The 70-second duration recorded above fits a `QTRY_*` waiting for a file that is never going to appear, which is consistent with a wrong destination rather than a slow one. -## 162. Delete fails while a sync is renaming the file underneath it - -**Observed (user, 2026-08-25):** deleting a draft reported `Cannot move -<file> to <account>/Trash`. - -**Cause (verified against the live Maildir, not read):** a stale path, and -neither Delete nor item 158 is at fault. - -1. The composer autosaves a draft as `<name>:2,D` and item 158 indexes it - under exactly that filename. -2. **mbsync uploads it and RENAMES it** to `<name>,U=<uid>:2,D`, recording the - server UID in the filename. -3. notmuch still holds the pre-`U=` name until that sync's `notmuch new` runs. -4. `moveMessages()` reads the filename from notmuch and calls - `QFile::rename()` on a path that no longer exists. It fails, the error is - emitted, and the message is skipped. - -Measured, in this order: `notmuch search --output=files` named a file that was -not on disk while `Background sync running...` was up, and the same query was -clean once the sync finished, with the file present under its new `,U=4` name. -That is why it reads as intermittent, and why it heals itself. - -**It is truthful and it loses nothing.** The move is skipped, no wrong folder -is created, no file is destroyed, and the next sync reconciles. The defect is -that the message blames a folder for a timing problem, and that the action -silently does nothing when the user asked for something. - -**This is `CLAUDE.md`'s `,U=` trap from the other side.** `MaildirName::fresh()` -exists because CARRYING that infix across a folder boundary produced -`Maildir error: duplicate UID` on real mail. Here mbsync is ADDING it and the -index lags; the same infix, the opposite direction. - -**Approach, and it needs a decision.** Two candidates: - -- **Refuse the move while a sync holds the lock.** `SyncMonitor` already - reports this, and the held-edit machinery from items 97 and 106 already - exists for exactly this shape: a tag edit made during a sync is held and - flushed when it ends. Delete would join it rather than inventing anything. - This is the likelier right answer, since it matches what every other - mutation already does. -- **Re-resolve the filename** from notmuch immediately before the rename and - re-query the message if the path is gone. Smaller, but it races the same - window it is trying to close, and a second lookup can be stale by the time - it is used. +## 164. A draft this application saved keeps `inbox` + +**Observed (developer, 2026-08-25):** `notmuch search --output=tags` on a +draft this application had just written reported `draft inbox unread`. + +**The first cause recorded here was WRONG, and the correction is the useful +part.** It said `strip_inbox_from_sent()` reads a sent-only folder list and +that `qtmaildirconf.py` has no drafts equivalent. Neither is true: + +- `NOT_ARRIVALS` is `("sent", "drafts")`, so `sent_folders()` already returns + both. The name says "sent" and the contents do not, which is what made the + wrong reading plausible. +- Run against the real config it returns every account's drafts folder. +- `notmuch count "(<carve-out query>) and id:<the draft>"` returns **1**. The + query the hook builds MATCHES the affected message. + +So the folder list and the query are correct, and the fix is not there. + +**What is actually established.** + +- The carve-out is scoped to `SCOPE = "tag:new"` (`post-new:106`). +- The affected draft carries `inbox`, and `notmuch count tag:new` is **0**. +- The installed hooks are SYMLINKS into this repository, so the code read is + the code that runs. Verified rather than assumed. +- An mbsync-style rename does **not** re-apply `new.tags`: measured in a + throwaway database, a file renamed to add `,U=4` and reindexed kept the tags + it had. The "the rename retags it" theory is therefore also out. + +**What is NOT established, and must be before any code is written:** which +pass put `inbox` on this file, and why it was not carrying `tag:new` when the +hook's carve-out ran. The likely shape is an ordering one, since item 158 +indexes a draft from the application itself, outside `notmuch new`, and a file +already known to the database is not a new file on the next pass. But that is +a hypothesis and the last two hypotheses here were both wrong. + +**The reproducer was built (2026-08-25) and it settles the mechanism.** Seven +variants were driven in throwaway databases, modelling `indexDraftFile()` with +a real `notmuch_database_index_file` call rather than the CLI, because no CLI +command indexes an untracked path without applying `new.tags`. + +What the sweep established, each measured rather than reasoned: + +- `index_file` applies **no tags at all**. A draft the application indexes is + therefore never in `tag:new` scope, and the hook has nothing to carve out. +- Whenever the file IS in `tag:new` scope, the carve-out strips `inbox` + correctly, in every filename shape tried: `:2,DS`, `:2,D`, no info suffix, + in `cur/` and in `new/`, with and without the `,U=4` infix. The real file's + shape (`,U=4:2,D`) is among them. +- It survives the orderings too: `notmuch new` first then the app's index, + the app's index first then the rename, an autosave landing between + `notmuch new` and the hook, and the stale-path `remove_message` that makes + the renamed file arrive as new mail. All six left the draft clean. +- The `D` flag is what puts `draft` on the message (`synchronize_flags`), and + the `S` flag is what removes `unread`. The affected file is `:2,D`, which is + why it carries `unread`, and that matches the reported tag set exactly. + +**The one variant that reproduces it** is the general shape rather than a +filename detail: a pass where `inbox` is applied while `tag:new` has ALREADY +been consumed. Modelled as a file indexed at a path the carve-out does not +cover and moved into the drafts folder afterwards, it ends in precisely the +live end state, `draft inbox unread` in Drafts with `,U=4` and `tag:new` at 0. +Nothing revisits a message once the marker is gone, so the tag is permanent. + +**What is still NOT established, and the next step.** The affected account +writes drafts straight to `<account>/Drafts`, which the carve-out +covers (verified against the live config and the live query, which matches the +message by id today), so the reproducing variant's premise does not hold for +it as written. The live log for the pass that added it reads + + 10:10:52 Added 1 new message to the database. Detected 9 file renames. + 10:10:52 post-new: sent-folder carve-out applied over 9 folder(s) + +so the hook DID run on that pass, over a path the query covers, and logged +success. The remaining candidates are all about what the path or the marker +looked like at that instant, not about the query text: the carve-out logs +"applied" on a `notmuch tag` that matched zero messages, so a successful log +line is not evidence the message was in scope. Instrumenting the hook to log +the carve-out's MATCH COUNT, and leaving it to run until the next draft, is +the cheapest way to close it, and is a log-only change to code that tags real +mail unattended. + +The filename also rules one thing in: `1787645266.M802P16149Q3.<host>` is +exactly `MaildirName::fresh()` output, so the application wrote this file. It +is not a draft another client left behind. + +The reproducer scripts are throwaway and were not kept; `indexfile.c` is +fifteen lines around one `notmuch_database_index_file` call and is trivial to +rebuild from this entry if the instrumentation points back at the hook. **Constraints.** -- **Delete reaches the real mail server.** Read `CLAUDE.md`'s item 103 notes - before touching `moveMessages()`: a wrong folder name is created, adopted by - mbsync, and propagated to every other client. -- Whatever is built, **the message must say a sync is running**, not name a - folder. The current wording sent the user looking for a broken folder - configuration, which was correct and configured. -- A test cannot see this in the ordinary fixture layout, where nothing renames - a file underneath the index. Driving it means renaming the file between the - index write and the move, which is what the reproducer has to do. - -## 163. The message pane shows a stale path and reports the message unreadable - -**Observed (user, 2026-08-25):** selecting a draft filled the pane with -`(unreadable message)` and `This message could not be parsed.`, naming a file -under the account's drafts folder. - -**Cause (verified against the live Maildir):** the path in the pane ended -`.dnx:2,D`, and the only file on disk ended `.dnx,U=4:2,D`. Same mechanism as -item 162: mbsync renames an uploaded file to record its server UID, and the -name the application is holding stops existing. - -**The site is different, and so is the fix.** Item 162 is the WRITE path, -`moveMessages()` reading a filename from notmuch. This is the READ path, and -by the time it fires notmuch is already CORRECT: measured, the index named the -`,U=4` file while the pane still named the pre-`U=` one. The stale path is the -MODEL's, cached when the row was loaded, so refusing to act while a sync runs -(162's likely fix) would not help here at all. - -**The report is honest, which is why it is confusing.** `MimeParser` opened a -path that did not exist and said so. Nothing is lost and the next query -repairs it. - -**Approach.** The read path should RECOVER rather than refuse: on a failed -parse, re-resolve the message id through notmuch and retry once before -reporting. `recoverStaleThread()` already exists for the neighbouring problem -(item 91 reuses it) and is the shape to follow. - -**Constraints.** - -- **A retry must be bounded.** A message that genuinely cannot be parsed - (item 41's territory) must still report, or a real defect becomes an - infinite loop. -- Re-resolving by id is what makes this safe; re-scanning the folder is not, - since two files can carry the same id. -- The placeholder wording is correct and should stay for the genuine case. - -## 164. Every newly synced draft carries `inbox` - -**Observed (developer, 2026-08-25):** `notmuch search --output=tags` on a draft -this application had just written reported `draft inbox unread`. - -**Cause (verified in the hook):** `strip_inbox_from_sent()` in -`assets/hooks/post-new` builds its query from -`qtmaildirconf.sent_folders()`, and `assets/hooks/qtmaildirconf.py` exposes -`sent_folders()` and `sent_query()` and **no drafts equivalent**. The -carve-out therefore never covers a drafts folder, and every draft that -completes a sync round trip is tagged `inbox` by `notmuch new` like any other -newly indexed file. - -**Item 158's measurement was right and did not cover this.** That item -measured `notmuch_database_index_file` assigning NO tags, which is true and is -why a freshly autosaved draft is clean. The tagging happens later: mbsync -uploads the file, renames it, and the next `notmuch new` indexes it as new -mail. - -**This contradicts shipped documentation.** The 0.27.0 changelog says "Sent -mail and drafts no longer appear in the inbox", and the drafts half has never -been true. Fixing the hook and correcting the entry are one item. - -**Approach.** A `drafts_folders()` / `drafts_query()` pair beside the sent -ones, and one more carve-out call, or a single function taking the folder kind -so the two cannot drift. - -**Constraints.** - -- **This is a two-repo change in spirit but not in fact.** The hooks moved - into this repository in 0.27.0, so `mailrules.py` is not involved and the - shared-format procedure does not apply. Check that before assuming - otherwise: `CLAUDE.md` still describes the hook as shipping from `mailctl` - in places. -- The hook must keep refusing to consume `tag:new` when a carve-out fails. - Clearing the marker while the rules did not run orphans that mail - permanently, which the sent half already gets right. +- **The hook tags real mail unattended every ten minutes.** Nothing here is + worth a speculative change. +- The 0.27.0 changelog claims sent mail and drafts both stay out of the inbox. + Whatever the cause, that claim is currently false for drafts and the entry + needs correcting with the fix. - Only `inbox` may be touched. A draft legitimately carries `draft` and `unread`, and `maildir.synchronize_flags` means removing `unread` rewrites the filename and reaches the server. -- `test_post_new.py` and `test_qtmaildirconf.py` are in the same directory and - must both be extended; the sent carve-out has tests to copy. +- The hook must keep refusing to consume `tag:new` when a carve-out fails. +- `test_post_new.py` and `test_qtmaildirconf.py` both live beside the hook and + have sent-carve-out tests to copy. + +## 165. A draft gets a new Message-ID on every autosave + +**Observed (developer, 2026-08-25), while hand-testing items 163 and 164.** +Four saved drafts produced four distinct Message-IDs, and one reopen-and-edit +turned one id into another. A draft therefore has no stable identity across +its own revisions. + +**Cause (verified in the code, not inferred).** `MessageBuilder::build()` +calls `g_mime_utils_generate_message_id()` unconditionally on every call +(`messagebuilder.cpp:311`), and every autosave calls `build()`. +`OutgoingMessage` has no field to carry an existing id in, and +`ComposeContext` has no field for the draft's OWN id either: it carries +`inReplyTo` and `references`, which are the ORIGINAL's id when replying, and +`ComposeContextBuilder::forDraft()` never reads the draft's Message-ID back +out of the file it parses. So this is not a changed call site; it needs a +field that does not exist yet, threaded from `forDraft()` through +`ComposeContext` and `OutgoingMessage` into `build()`. + +**Why it matters, and why it is NOT urgent.** To notmuch and to the server, +each revision is a different MESSAGE, not a new version of one. While the +file is replaced correctly this is invisible: one file in, one file out. It +becomes visible whenever a revision is NOT replaced, and item 163 is the +proof, where a stale path forked a draft into two files that were also two +messages and that nothing will ever collapse. Item 163's fix removes the +known way to reach that state; this entry is about the property that turned a +one-file mistake into a two-message one. + +An interrupted save is the remaining route: `DraftStore::write()` unlinks the +previous revision only AFTER the new file is safely in place (deliberately, +so a failed write cannot lose the draft), so a crash between the two leaves +two files, and with two ids they are two drafts rather than one duplicated. + +**Approach, and it needs a decision rather than an implementation.** The +question is what a draft's identity IS, and it is not obviously "the id it +will be sent under": + +- A stable id reused at send time makes the draft and the sent message one + message, which is what a user means by "my draft became this email". It + also means the id was in a file mbsync uploaded to the drafts folder before + the message was ever sent, and the server has seen it. +- A stable id DISCARDED at send time keeps revisions collapsed while drafting + and mints a fresh id for the sent copy. Two identities, and the sent one is + the one that threads. +- The current behaviour is a third position, and its only virtue is that no + id is ever reused for two different things. + +Whichever is chosen must be checked against `In-Reply-To`/`References` on the +eventual send, since `referencesForReply()` builds those from the ORIGINAL's +id and a draft of a reply carries both. + +**Constraints.** + +- **A Message-ID reaches the server and every recipient**, so a reused id is + not a local matter. Two different messages sharing an id is worse than two + ids for one draft, which is what makes the current behaviour defensible as + a default rather than simply wrong. +- `MessageBuilder::build()` is on the SEND path as well as the autosave path. + A change that makes ids stable must not make two different sent messages + share one. +- The comment at `messagebuilder.cpp:297` records that GMime generates + neither Date nor Message-ID unless asked, and that a message without one + cannot be threaded by anything receiving it, this application's own index + of the sent copy included. Any "just omit it while drafting" variant has to + answer that. +- Item 163's fix stands on its own and this does not block it: the file is + replaced correctly now, so the fork this would have mitigated no longer + happens by that route. diff --git a/src/composecontext.cpp b/src/composecontext.cpp index d0406fc..7233330 100644 --- a/src/composecontext.cpp +++ b/src/composecontext.cpp @@ -24,6 +24,7 @@ #include "composecontext.h" #include "config.h" +#include "maildirname.h" #include "mimeparser.h" #include <QDir> @@ -491,16 +492,32 @@ ComposeContext ComposeContextBuilder::forDraft(const Config &config, { ComposeContext context; + // Item 163. The caller's path comes from the model, captured when the + // query ran, and mbsync renames an uploaded draft to add its `,U=<uid>` + // infix. Resolving first is what stops a rename from refusing the reopen: + // the refusal happens BEFORE any composer exists, so the user composes + // again into a FRESH window whose autosave has no previous path to unlink, + // and the draft is silently forked into two files with two Message-IDs, + // both of which reach the server. + // + // Returns the path unchanged when nothing was renamed, and empty when the + // file is genuinely gone, which still fails below exactly as before. + const QString resolved = MaildirName::resolveRenamed(path); + MimeParser parser; - const ParsedMessage draft = parser.parse(path); + const ParsedMessage draft = parser.parse(resolved); if (!draft.ok) return context; // Kind::New and empty: the caller reports the failure. context.kind = ComposeContext::Kind::Draft; - context.originalPath = path; + context.originalPath = resolved; // The file this composer OWNS. Without it the first autosave writes a // second draft and leaves this one behind, so one message becomes two. - context.draftPath = path; + // + // The RESOLVED path, never the caller's: seeding the stale one would let + // the reopen succeed and the unlink still miss, which is the same fork + // arriving one step later. + context.draftPath = resolved; const auto addresses = [](const QString &header) { QStringList out; diff --git a/src/maildirname.cpp b/src/maildirname.cpp index 6263aec..9f827fc 100644 --- a/src/maildirname.cpp +++ b/src/maildirname.cpp @@ -20,6 +20,8 @@ #include <QCoreApplication> #include <QDateTime> +#include <QDir> +#include <QFileInfo> #include <QHostInfo> namespace MaildirName { @@ -77,4 +79,63 @@ QString fresh(const QString &oldName) .arg(info); } +QString resolveRenamed(const QString &path) +{ + if (path.isEmpty()) + return QString(); + + // The ordinary case, and the overwhelmingly common one: nothing was + // renamed. One stat, then out. + if (QFileInfo::exists(path)) + return path; + + const QFileInfo info(path); + const QString name = info.fileName(); + + // The unique part mbsync preserves. `<stem>:2,D` becomes + // `<stem>,U=5:2,D`, so the stem ends at whichever of `,` or `:` comes + // first. A name carrying neither is all stem. + int cut = name.size(); + for (const QChar separator : { QLatin1Char(','), QLatin1Char(':') }) { + const int at = name.indexOf(separator); + if (at >= 0 && at < cut) + cut = at; + } + const QString stem = name.left(cut); + if (stem.isEmpty()) + return QString(); + + // One directory, never a recursive walk: a rename keeps the file where it + // was, and a file that changed FOLDERS is a different question that only + // the message id can answer (see NotmuchWorker::moveMessages(), item 162). + const QDir dir(info.absolutePath()); + if (!dir.exists()) + return QString(); + + QString found; + const QFileInfoList entries = + dir.entryInfoList(QDir::Files | QDir::NoDotAndDotDot); + for (const QFileInfo &entry : entries) { + const QString candidate = entry.fileName(); + // Anchored on the stem AND on what follows it, so `...Q2` cannot match + // `...Q23`: the next character must begin the infix or the flags. + if (!candidate.startsWith(stem)) + continue; + const QString rest = candidate.mid(stem.size()); + if (!rest.isEmpty() && !rest.startsWith(QLatin1Char(',')) + && !rest.startsWith(QLatin1Char(':'))) { + continue; + } + + // Two files sharing a stem cannot happen in a correct Maildir. Refuse + // rather than guess: the caller reports "gone", which is honest, where + // a guess could open, move or delete the wrong message. + if (!found.isEmpty()) + return QString(); + found = entry.absoluteFilePath(); + } + + return found; +} + } // namespace MaildirName diff --git a/src/maildirname.h b/src/maildirname.h index f24bc71..255517d 100644 --- a/src/maildirname.h +++ b/src/maildirname.h @@ -38,4 +38,28 @@ namespace MaildirName { /// what a newly composed draft is. QString fresh(const QString &oldName); +/// The file \p path names, or the renamed file that replaced it. +/// +/// Item 163. mbsync renames an uploaded file to add its `,U=<uid>` infix, and +/// anything holding the previous name (the model's `MessageRef::filePath`, a +/// draft's `ComposeContext::draftPath`) then points at a path that no longer +/// exists. Returns \p path unchanged when it is still there, so the ordinary +/// case costs one stat and nothing else. +/// +/// Matched on the UNIQUE STEM, the part before the first `,` or `:`, which +/// mbsync preserves: `<stem>:2,D` becomes `<stem>,U=5:2,D`. That is what makes +/// this safe to do by filename at all. The search is confined to the file's +/// own directory and never recurses, and an ambiguous match (more than one +/// candidate, which a correct Maildir cannot produce) yields nothing rather +/// than guessing. +/// +/// Empty when there is no such file, which every caller must treat as the +/// genuine "it is gone" it is: recovering silently from a real deletion would +/// turn a reportable defect into a wrong answer. +/// +/// This resolves a RENAME, not a MOVE. A file that changed folders is a +/// different question and belongs to whoever knows the message id; +/// `NotmuchWorker::moveMessages()` re-resolves that way for item 162. +QString resolveRenamed(const QString &path); + } // namespace MaildirName diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index af3b817..5845922 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -18,6 +18,8 @@ #include "mainwindow.h" +#include "maildirname.h" + #include <QAction> #include <QApplication> #include <QCloseEvent> @@ -1043,8 +1045,13 @@ void MainWindow::openComposerFor(const MessageRef &ref, return; } + // Item 163, the same stale path the pane and the draft reopen hit. Here it + // refuses a Reply or a Forward outright, so the user cannot answer a + // message that is sitting on disk and readable. + const QString originalPath = MaildirName::resolveRenamed(ref.filePath); + MimeParser parser; - const ParsedMessage original = parser.parse(ref.filePath); + const ParsedMessage original = parser.parse(originalPath); if (!original.ok) { showTransientStatus(tr("That message could not be read")); return; @@ -1052,7 +1059,7 @@ void MainWindow::openComposerFor(const MessageRef &ref, ComposeContext context; context.kind = kind; - context.originalPath = ref.filePath; + context.originalPath = originalPath; const bool replyAll = kind == ComposeContext::Kind::ReplyAll; const bool forwarding = kind == ComposeContext::Kind::Forward; @@ -3849,11 +3856,22 @@ void MainWindow::renderMessages(const QVector<MessageRef> &messages) const MessageRef &ref = messages.at(i); ThreadRenderItem item; - item.message = parser.parse(ref.filePath); + // Item 163. The model's path was captured when the query ran, and + // mbsync renames an uploaded file to add its `,U=<uid>` infix, so a row + // loaded before that sync names a file that no longer exists. The pane + // then reported the message unreadable while nothing was wrong with it. + // Unchanged when nothing was renamed; empty when the file is genuinely + // gone, which still reports below. + const QString path = MaildirName::resolveRenamed(ref.filePath); + item.message = parser.parse(path); if (!item.message.ok) { // One unreadable message must not lose the rest of the thread, so // it becomes an inline note rather than replacing the whole pane. + // + // Named by the path the model HOLDS, not by the resolved one: when + // resolution failed there is no resolved path, and the stale name + // is what the user can act on. item.message = {}; item.message.ok = true; item.message.from = tr("(unreadable message)"); diff --git a/src/nmraii.h b/src/nmraii.h index 1c3e8f1..925a8e7 100644 --- a/src/nmraii.h +++ b/src/nmraii.h @@ -67,3 +67,5 @@ using NmMessages = NmHandle<notmuch_messages_t, notmuch_messages_destroy>; using NmThread = NmHandle<notmuch_thread_t, notmuch_thread_destroy>; using NmMessage = NmHandle<notmuch_message_t, notmuch_message_destroy>; using NmTags = NmHandle<notmuch_tags_t, notmuch_tags_destroy>; +using NmFilenames = + NmHandle<notmuch_filenames_t, notmuch_filenames_destroy>; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 16df4ed..f3a76ea 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -184,6 +184,39 @@ void walkReplies(notmuch_messages_t *messages, int depth, } } +/// Teach the database the current filenames in one Maildir directory. +/// +/// Item 162's recovery step. mbsync renames a file to add its `,U=<uid>` infix +/// and notmuch does not learn the new name until a `notmuch new` runs; this +/// indexes just the one directory rather than waiting for that sweep. +/// +/// Deliberately NOT a full `notmuch new`: that walks the entire Maildir and +/// runs the post-new hook, which tags real mail. This must stay a read of one +/// folder with no side effects beyond the filenames it records. +/// +/// Indexing a file already known under another name ADDS a filename to the +/// same message rather than creating a second message, which is what lets the +/// caller pick the surviving path out of get_filenames(). Errors are ignored +/// on purpose: this is a best-effort repair whose caller reports the failure +/// if the path is still missing afterwards. +void reindexFolder(notmuch_database_t *db, const QString &folder) +{ + const QDir dir(folder); + if (!dir.exists()) + return; + + const QFileInfoList entries = + dir.entryInfoList(QDir::Files | QDir::NoDotAndDotDot); + for (const QFileInfo &entry : entries) { + notmuch_message_t *indexed = nullptr; + notmuch_database_index_file( + db, entry.absoluteFilePath().toUtf8().constData(), nullptr, + &indexed); + if (indexed) + notmuch_message_destroy(indexed); + } +} + /// The Maildir FOLDER a message file sits in, relative to the database root. /// /// `<root>/acct/inbox/cur/12345` becomes `acct/inbox`: the `cur`/`new` segment @@ -255,8 +288,28 @@ QByteArray NotmuchWorker::configPathArg() const bool NotmuchWorker::openReadOnly() { - if (m_db) + if (m_db) { + // A read-only handle is a Xapian SNAPSHOT taken when it was opened, so + // it never observes a write made by another process afterwards. The + // sync script's `notmuch new` is exactly that, which made mail arriving + // while the window was open invisible until the application restarted: + // not only to the post-sync refresh, but to any query the user typed by + // hand, since all of them are answered from the same handle. Item 104. + // + // Reopening here rather than at each call site covers every read path, + // which all begin by asking for the handle. It is cheap and it is what + // notmuch provides the call for; a failure is deliberately NOT fatal, + // since the existing handle is still usable and serving slightly stale + // results beats refusing to answer at all. + const notmuch_status_t status = + notmuch_database_reopen(m_db, NOTMUCH_DATABASE_MODE_READ_ONLY); + if (status != NOTMUCH_STATUS_SUCCESS) { + emit errorOccurred( + QStringLiteral("Cannot refresh notmuch database: %1") + .arg(QString::fromUtf8(notmuch_status_to_string(status)))); + } return true; + } const QByteArray configPath = configPathArg(); char *error = nullptr; @@ -747,7 +800,54 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds, const char *rawName = notmuch_message_get_filename(message.get()); if (!rawName) continue; - const QString from = QString::fromUtf8(rawName); + QString from = QString::fromUtf8(rawName); + + // Item 162. mbsync renames an uploaded file to record the server UID + // (`<name>,U=<uid>:2,<flags>`), and notmuch keeps the pre-`U=` name + // until that sync's `notmuch new` runs. Renaming a path that no longer + // exists fails, and Delete silently does nothing while blaming the + // destination folder for a timing problem. + // + // Refusing while a sync holds the write lock does NOT close this: + // mbsync renames throughout its run without touching notmuch's lock, + // so the damaging window is open when there is nothing to observe. + // Re-resolving is what closes it. + // + // Recovery is by MESSAGE ID, never by scanning the folder: two files + // can carry the same id, and picking the wrong one moves the wrong + // file. notmuch_message_get_filenames() lists every path the database + // holds for this id, so a file that was renamed rather than removed is + // found among them once the folder is reindexed. + if (!QFileInfo::exists(from)) { + // One reindex of the containing folder, which is what teaches + // notmuch the new name. Bounded deliberately: a single attempt, + // and a message that is still missing afterwards falls through to + // the error below, so a genuinely deleted file is still reported + // (item 41's territory) rather than becoming a silent no-op. + const QString folder = QFileInfo(from).absolutePath(); + message.reset(); + reindexFolder(db, folder); + + notmuch_message_t *again = nullptr; + if (notmuch_database_find_message(db, id.toUtf8().constData(), + &again) + == NOTMUCH_STATUS_SUCCESS + && again) { + message.reset(again); + for (NmFilenames names( + notmuch_message_get_filenames(message.get())); + notmuch_filenames_valid(names.get()); + notmuch_filenames_move_to_next(names.get())) { + const QString candidate = QString::fromUtf8( + notmuch_filenames_get(names.get())); + if (QFileInfo::exists(candidate)) { + from = candidate; + break; + } + } + } + } + // The handle is released before the file moves under it. message.reset(); diff --git a/tests/test_maildirname.cpp b/tests/test_maildirname.cpp index dcc8fab..a8e4c01 100644 --- a/tests/test_maildirname.cpp +++ b/tests/test_maildirname.cpp @@ -18,7 +18,10 @@ #include "maildirname.h" +#include <QDir> +#include <QFile> #include <QSet> +#include <QTemporaryDir> #include <QTest> class TestMaildirName : public QObject @@ -31,6 +34,11 @@ private slots: void anEmptyFlagSuffixIsPreserved(); void aNameWithNoSuffixGetsNone(); void theUidInfixIsNotCarriedAcross(); + void resolveRenamedReturnsAPathThatStillExists(); + void resolveRenamedFindsTheFileMbsyncRenamed(); + void resolveRenamedIsEmptyWhenTheFileIsReallyGone(); + void resolveRenamedDoesNotMatchADifferentMessage(); + void resolveRenamedRefusesAnAmbiguousMatch(); }; // Two messages written in the same second must not collide, which a @@ -89,5 +97,93 @@ void TestMaildirName::theUidInfixIsNotCarriedAcross() .arg(name))); } +namespace { + +/// One empty file, so a test can assert on which PATH is chosen rather than on +/// content. resolveRenamed() answers a filesystem question and never opens the +/// file. +bool touch(const QString &path) +{ + QFile file(path); + if (!file.open(QIODevice::WriteOnly)) + return false; + file.close(); + return true; +} + +} // namespace + +void TestMaildirName::resolveRenamedReturnsAPathThatStillExists() +{ + // The ordinary case, and the one that must stay cheap: nothing was + // renamed, so the answer is the question. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString path = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(touch(path)); + + QCOMPARE(MaildirName::resolveRenamed(path), path); +} + +void TestMaildirName::resolveRenamedFindsTheFileMbsyncRenamed() +{ + // Item 163. mbsync uploads the file and inserts its `,U=<uid>` infix + // before the flag suffix, leaving the unique stem alone. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString stale = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + const QString renamed = + dir.filePath(QStringLiteral("1787647354.M369Q2.host,U=5:2,D")); + QVERIFY(touch(renamed)); + QVERIFY2(!QFile::exists(stale), "the stale path must not exist"); + + QCOMPARE(MaildirName::resolveRenamed(stale), renamed); +} + +void TestMaildirName::resolveRenamedIsEmptyWhenTheFileIsReallyGone() +{ + // The bounded half. A deleted file must NOT be recovered from, or a + // reportable defect becomes a wrong answer. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString gone = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(!QFile::exists(gone)); + + QVERIFY(MaildirName::resolveRenamed(gone).isEmpty()); +} + +void TestMaildirName::resolveRenamedDoesNotMatchADifferentMessage() +{ + // A neighbouring file in the same folder is not this message. Matching on + // anything looser than the whole stem would return it, and the caller + // would then open, display or MOVE the wrong mail. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString stale = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(touch(dir.filePath(QStringLiteral("1787647354.M369Q3.host,U=5:2,D")))); + QVERIFY(touch(dir.filePath(QStringLiteral("9999999999.M111Q1.host,U=6:2,D")))); + + QVERIFY(MaildirName::resolveRenamed(stale).isEmpty()); +} + +void TestMaildirName::resolveRenamedRefusesAnAmbiguousMatch() +{ + // Two files sharing one stem cannot happen in a correct Maildir, so this + // is a "the world is not what I assumed" case. Guessing between them could + // move or delete the wrong file, and the caller reports honestly instead. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString stale = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(touch(dir.filePath(QStringLiteral("1787647354.M369Q2.host,U=5:2,D")))); + QVERIFY(touch(dir.filePath(QStringLiteral("1787647354.M369Q2.host,U=6:2,S")))); + + QVERIFY(MaildirName::resolveRenamed(stale).isEmpty()); +} + QTEST_MAIN(TestMaildirName) #include "test_maildirname.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index f935cac..21c3661 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -490,6 +490,7 @@ private slots: void doubleClickingADraftOpensTheComposer(); void aResumedDraftReplacesItsFileRatherThanAddingOne(); void aResumedDraftKeepsItsBlindRecipients(); + void aDraftRenamedByASyncStillReopensAndReplacesItsFile(); void theComposerSplitsItsToolbarByScope(); void ccAndBccHideBehindADisclosure(); void ccAndBccAreRevealedWhenTheyCarryAValue(); @@ -12861,6 +12862,83 @@ void TestMainWindow::aResumedDraftReplacesItsFileRatherThanAddingOne() "now exists twice"); } +void TestMainWindow::aDraftRenamedByASyncStillReopensAndReplacesItsFile() +{ + // Item 163, the composer site, and the one that costs data rather than + // display. mbsync uploads a draft and renames it to add its `,U=<uid>` + // infix; the model's path was captured when the query ran, so the reopen + // is handed a name that no longer exists. + // + // The refusal happens BEFORE any composer exists, so the user composes + // again into a FRESH window whose autosave has no previous path to unlink. + // The old revision survives, each save mints a new Message-ID, and both + // files reach the server. Asserted as the file COUNT, which is the shape + // the fork actually takes. + ComposeFixture fixture; + QVERIFY(fixture.build()); + + OutgoingMessage message; + message.accountKey = QStringLiteral("acct"); + message.to = { QStringLiteral("someone@example.org") }; + message.subject = QStringLiteral("Written before a sync"); + message.markdownBody = QStringLiteral("The first half."); + + const QString folder = fixture.mailRoot() + QStringLiteral("/acct/Drafts"); + const QString path = writeDraftFile(folder, message, + fixture.config().account( + QStringLiteral("acct"))); + QVERIFY(!path.isEmpty()); + + // mbsync's rename: same directory, same unique stem, `,U=<uid>` inserted + // before the flag suffix. Nothing reindexes, so the caller below still + // holds the pre-rename name, which is the whole precondition. + const QFileInfo before(path); + const QString base = before.fileName(); + const int suffix = base.indexOf(QStringLiteral(":2,")); + QVERIFY2(suffix > 0, "the draft fixture has no maildir flag suffix"); + const QString renamed = before.absolutePath() + QLatin1Char('/') + + base.left(suffix) + QStringLiteral(",U=7") + + base.mid(suffix); + QVERIFY2(QFile::rename(path, renamed), "could not stage the sync rename"); + + // The guard that proves this test can fail: without it, a fixture that + // quietly left the original in place would pass against the bug. + QVERIFY2(!QFile::exists(path), "the stale path should no longer exist"); + + const auto draftCount = [&folder]() { + return QDir(folder + QStringLiteral("/cur")) + .entryList(QDir::Files).size(); + }; + QCOMPARE(draftCount(), 1); + + // The STALE path, exactly as openComposerFor() passes MessageRef::filePath. + const ComposeContext context = + ComposeContextBuilder::forDraft(fixture.config(), path); + QVERIFY2(context.kind == ComposeContext::Kind::Draft, + "the reopen was refused, so the user would compose a second draft"); + // Resolved, not the caller's: seeding the stale path would let the reopen + // succeed and the unlink still miss, forking the draft one step later. + QCOMPARE(context.draftPath, renamed); + + ComposeWindow window(context, fixture.config(), fixture.mailRoot()); + auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("The second half.")); + + auto *timer = window.findChild<QTimer *>(QStringLiteral("autosave")); + QVERIFY2(timer, "the composer has no autosave timer"); + QVERIFY2(timer->isActive(), "editing the body did not arm the autosave"); + timer->setInterval(0); + QTRY_VERIFY_WITH_TIMEOUT(!timer->isActive(), 5000); + + // Still ONE draft: the autosave replaced the renamed file rather than + // leaving it behind beside a new one. + QCOMPARE(draftCount(), 1); + QVERIFY2(!QFile::exists(renamed), + "the renamed draft survived the autosave, so the draft was forked " + "into two files and both would reach the server"); +} + void TestMainWindow::aResumedDraftKeepsItsBlindRecipients() { // MessageBuilder writes Bcc into the draft file deliberately, and says diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index d02f8bd..e1a21cd 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -92,6 +92,8 @@ private slots: void moveMessagesReportsOnlyWhatMoved(); void moveMessagesGivesTheFileAFreshMaildirName(); void moveMessagesKeepsTheMaildirFlags(); + void moveMessagesRecoversWhenASyncRenamedTheFile(); + void moveMessagesStillReportsAMessageThatIsReallyGone(); void indexDraftFileMakesAFileFindable(); void indexDraftFileRemovesThePreviousFile(); @@ -102,6 +104,8 @@ private slots: void aSplitIndexListsTheMaildirsFolders(); void twoMessagesMovedTogetherGetDistinctNames(); + void aQuerySeesMailIndexedAfterTheWorkerOpened(); + private: /// Adds one read message in `folder` and reindexes, for the move tests. /// Each of those takes its own message, because a move is destructive and @@ -184,6 +188,59 @@ void TestNotmuchWorker::initTestCase() QVERIFY2(m_fixture.index(), qPrintable(m_fixture.error())); } +void TestNotmuchWorker::aQuerySeesMailIndexedAfterTheWorkerOpened() +{ + // The defect this covers is item 104, and it is the reason mail arriving + // while the window is open was invisible until the application restarted. + // + // A read-only notmuch handle is a Xapian SNAPSHOT taken when it is opened. + // `notmuch new` runs in a separate process, so nothing it writes is visible + // to a handle already open, however long it is held and however many + // queries are run through it. The worker opens once and keeps that handle + // for the process lifetime, so every query after the first sync answered + // from a stale index: a refresh missed the mail, and so did a query the + // user typed by hand, which is what ruled out the model and the generation + // counter when this was diagnosed. + // + // ONE worker across both queries is the whole point. The runQuery() helper + // builds a fresh worker per call, which opens a fresh handle and therefore + // cannot reproduce this at all: a test written through it passes against + // the bug. + NotmuchWorker worker(m_fixture.configPath()); + + const QString query = QStringLiteral("subject:\"Arrived mid-session\""); + + { + QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); + worker.runQuery(query, 1); + QVector<ThreadSummary> before; + for (const QList<QVariant> &args : ready) + before += args.at(0).value<QVector<ThreadSummary>>(); + // Establishes that the handle is open and the query is well-formed, + // rather than leaving "found nothing" to mean either. + QCOMPARE(before.size(), 0); + } + + // A second process writes to the index, exactly as the sync script's + // `notmuch new` does while the window is open. + QVERIFY(m_fixture.addMessage(QStringLiteral("inbox"), + QStringLiteral("mid@example.org"), + QStringLiteral("Arrived mid-session"), + QStringLiteral("Carol <carol@example.org>"), + QStringLiteral("Tue, 9 Jun 2026 10:00:00 +0000"), + QStringLiteral("new mail"))); + QVERIFY2(m_fixture.index(), qPrintable(m_fixture.error())); + + QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); + worker.runQuery(query, 2); + QVector<ThreadSummary> after; + for (const QList<QVariant> &args : ready) + after += args.at(0).value<QVector<ThreadSummary>>(); + + QCOMPARE(after.size(), 1); + QCOMPARE(after.first().subject, QStringLiteral("Arrived mid-session")); +} + QVector<ThreadSummary> TestNotmuchWorker::runQuery( const QString &query, NotmuchWorker::SortOrder sort, bool withRecipients) { @@ -1365,6 +1422,82 @@ void TestNotmuchWorker::moveMessagesKeepsTheMaildirFlags() QVERIFY(!name.contains(QStringLiteral(",U="))); } +void TestNotmuchWorker::moveMessagesRecoversWhenASyncRenamedTheFile() +{ + // Item 162. mbsync uploads a file and RENAMES it to record the server UID, + // and notmuch keeps the pre-`U=` name until that sync's `notmuch new` + // runs. moveMessages() then renames a path that no longer exists, reports + // "Cannot move <file> to <folder>", and silently does nothing. + // + // The ordinary fixture layout cannot see this: nothing renames a file + // underneath the index. Driving it means renaming the file WITHOUT + // reindexing, which is exactly the window mbsync opens. + const QString id = QStringLiteral("move-stale@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + const QString indexed = fileOf(id); + QVERIFY(!indexed.isEmpty()); + + // mbsync's rename, and deliberately NO m_fixture.index() afterwards: the + // database must still name the old path, which is the whole precondition. + const QString renamed = QFileInfo(indexed).absolutePath() + + QStringLiteral("/move-stale.example.org,U=7:2,D"); + QVERIFY2(QFile::rename(indexed, renamed), "could not stage the sync rename"); + + // The guard that proves this test can fail: without it, a fixture that + // quietly reindexed would make the assertions below pass against the bug. + QCOMPARE(fileOf(id), indexed); + QVERIFY2(!QFile::exists(indexed), "the stale path should no longer exist"); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.moveMessages({ id }, QStringLiteral("trash")); + + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + QCOMPARE(moved.size(), 1); + QCOMPARE(moved.first().at(0).toStringList(), QStringList{ id }); + + // The file really moved, and the database followed it. + const QString after = fileOf(id); + QVERIFY2(!after.isEmpty(), "the message is not in the database after the move"); + QCOMPARE(QFileInfo(after).absolutePath(), + m_fixture.maildirPath() + QStringLiteral("/trash/cur")); + QVERIFY2(QFile::exists(after), qPrintable(after)); + QVERIFY(!QFile::exists(renamed)); + + // The `,U=` infix must not be carried across the folder boundary: that is + // what produced `Maildir error: duplicate UID` on real mail. + QVERIFY(!QFileInfo(after).fileName().contains(QStringLiteral(",U="))); +} + +void TestNotmuchWorker::moveMessagesStillReportsAMessageThatIsReallyGone() +{ + // The bounded half of the recovery above. A file that is genuinely absent, + // rather than merely renamed, must still be REPORTED: recovering silently + // from every missing path would turn a real defect into a move that + // claims success and does nothing. + const QString id = QStringLiteral("move-gone@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + const QString indexed = fileOf(id); + QVERIFY(!indexed.isEmpty()); + QVERIFY2(QFile::remove(indexed), "could not remove the file"); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.moveMessages({ id }, QStringLiteral("trash")); + + QCOMPARE(errors.size(), 1); + // Nothing is claimed to have moved. + QVERIFY(moved.isEmpty() || moved.first().at(0).toStringList().isEmpty()); +} + void TestNotmuchWorker::twoMessagesMovedTogetherGetDistinctNames() { // The generated name must be unique, since a collision is the entire class |