diff options
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 1146 |
1 files changed, 1146 insertions, 0 deletions
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 f2b977b..77d4726 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 @@ -7942,3 +7942,1149 @@ 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. + +## 104. Mail visible in Thunderbird never reaches qtmaildir + +**Observed (user, from the notes):** "sync doesn't work compared to thunderbird. +New mail received on thunderbird did not appear in qtmaildir. Need to investigate +further." + +**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. + +## 167. No way to tell one build of an unreleased version from another + +**Observed (user, from the notes):** "we should add a dev build number to be +pushed everytime we rebuild after a fix, so that I can verify if I'm in the +correct app version." The note has sat unrecorded through several sessions; +the 2026-08-25 reconciliation is the first to pick it up. + +**Cause (verified in the code, 2026-08-25.)** The version lives in exactly one +place, `project(qtmaildir VERSION ...)`, and `src/version.h.in` interpolates +`@PROJECT_VERSION@` and nothing else. That is correct for a release and says +nothing between two of them: the string moves only when the release procedure +bumps it, so every rebuild of `0.27.0` reports `0.27.0`. The status table above +shows why it bites in practice, since most closed items since 0.27.0 read +"unreleased" and the user hand-tests each one against a binary they rebuilt +themselves. + +Both surfaces that show the version take it from the same macro, so whatever is +added reaches them at once: the window title (`mainwindow.cpp:939`), the About +dialog (`mainwindow.cpp:2449`), the placeholder pane (`messageview.cpp:547`), +`--version` and `--help` (`main.cpp`). + +**Approach.** Needs a DECISION before any code, because the two candidates fail +in opposite directions. + +A git description (`git describe --always --dirty`, or the short hash) is +accurate and self-explaining: it names the commit the binary was built from, and +a reviewer can check out exactly that. Its cost is that CMake computes it at +CONFIGURE time, so a build after a new commit reports the previous hash unless +the configure step is made to re-run, which is a custom command with a dependency +on `.git/HEAD` and the packed refs, and is the part that usually ships subtly +wrong. + +A monotonic counter always moves and needs no git, but it means nothing on its +own: build 412 does not say which fix is in it, and it differs between the user's +machine and any other, so it cannot be quoted in a report. + +**Constraints.** A release build must keep printing a clean `X.Y.Z`, since the +SlackBuild in the `my-slackbuilds` repo builds from the release tarball where +there is no git checkout at all, and the release procedure checks +`./build/src/qtmaildir --version`. Whatever is added is therefore an addition to +the string in a dev build and absent in a release one, not a change to the +version itself. + +**Decision (user, 2026-08-25): the counter.** The git description was +offered as the recommendation and was not chosen; what the user wants is to +know a rebuild happened, not which commit it was. + +**Built 2026-08-25, unreleased.** `QTMAILDIR_BUILD_NUMBER`, a cmake option ON +by default, runs `cmake/BuildNumber.cmake` as a build step: it increments a +counter and writes `buildnumber.h`, which `version.h` includes. +`QTMAILDIR_VERSION_DISPLAY` is `X.Y.Z build N` when that macro is defined and +plain `X.Y.Z` when it is not. + +Two macros, not one, and the split is the load-bearing part. +`QTMAILDIR_VERSION` stays clean and keeps the window title, `applicationVersion` +and anything that might ever compare versions; `QTMAILDIR_VERSION_DISPLAY` goes +to the three surfaces the user picked: `--version`, `--help`, the About dialog +and the placeholder pane. The window title was offered and declined, since the +number would then sit in every screenshot. + +**The counter had to be a BUILD step, not `configure_file`.** That is the whole +reason this is not two lines: `configure_file` runs once per cmake run, so a +counter interpolated into `version.h.in` sits still across exactly the rebuilds +this item exists to distinguish. `version.h.in` therefore includes a second +generated header rather than carrying the number itself. + +The counter file lives in the build directory and is not tracked, so it cannot +conflict on a pull or dirty the tree; a fresh build directory restarts at 1, +which is honest, because it is a different build tree. A release build passes +`-DQTMAILDIR_BUILD_NUMBER=OFF` and the header is written empty. + +**Verified by running it**, since none of this is reachable from a C++ test: +three consecutive builds reported `build 2`, `build 3`, `build 4`, and a +separate Release configure with the option OFF reported a clean `0.27.0`. The +suite is 37 of 38, the one failure being item 136 on an unrelated path. + +**Size: XS**, as sized. + +## 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.** + +- **The hook is this repo's**, `assets/hooks/post-new`, which the live + `database.hook_dir` symlinks to. It has its own suites beside it; run + `./test_post_new.py` and `./test_mailrules.py` from `assets/hooks/`. +- 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. + +**Fixed 2026-08-25, option 1**, the one the entry named: strip only when +every file is in a sent folder. + +`sent_only()` in `assets/hooks/post-new` filters the matches and the tag is +then applied per id. **It is a loop because no query can express it**, and both +plausible query forms were measured against a real two-file message before the +loop was written: `not path:"Inbox/**"` does NOT exclude the message, and +`notmuch count --output=files` on a path query reports every file of every +matching message rather than the files that matched. Both read as if they +worked and are wrong for one reason, that a notmuch term is a predicate over a +MESSAGE while the distinction here is between its FILES. + +The root comes from `database.mail_root`, not `database.path`, since this index +is split and no message file sits under the index directory. The mutation +putting `database.path` back passes every pre-existing test, because the +ordinary fixture keeps the index inside the mail root and both keys return the +same string; `setup_accounts(split_index=True)` is what catches it, and is the +Python counterpart to `NotmuchFixture::splitIndex()`. + +Two mutations fail: `all` to `any` loses `inbox` on the self-addressed message, +`mail_root` to `path` silently stops stripping anything. + +**Verified read-only against the live index**, tagging nothing: of 807 messages +matching a sent path, 780 are still stripped and 27 are spared, every one of +them two files with one in another account's Inbox. No arrival is affected. + +**Size: S.** Done. + +## 112. Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread + +**Observed (user, 2026-08-17):** clicking a thread root and asking to mark the +whole thread unread does not do it. On a seven-message thread with two unread +replies, the result is that every message is toggled unread **except those +two**, which are left as they were. The user asks for an explicit "mark whole +thread read/unread" rather than a toggle. + +**Cause (verified in code):** the action exists, and its direction is the +defect. `toggle_unread_thread` (`src/mainwindow.cpp:931`, `Ctrl+Alt+U`) chooses +between adding and removing by asking +`everySelectedRowHasTag("unread", TagScope::Thread)`, which reads +`ThreadListModel::threadFor(index).tags`. That is notmuch's **union over the +thread** (`CLAUDE.md`, item 110), so a thread containing even one unread message +answers "unread" and the action picks *Mark thread read*. There is no input a +user can give that reaches *Mark thread unread* on a mixed thread: the only +threads that take that branch are the ones already entirely read, and the only +threads reporting "not unread" are the ones the user does not need the action +for. + +The write itself is absolute and correct. `tagSelected` with `TagScope::Thread` +adds or removes `unread` across every message, so the two unread replies in the +report are not skipped by the write. They are the reason the write ran in the +opposite direction from the one the user wanted. + +**A union is not a state, and a toggle needs a state.** This is the same class +as item 110 and the third time the union has produced a defect. Items 105 and 88 +fixed *which object* a toggle resolved; this one is about a thread having no +single answer to give. `everySelectedRowHasTag` is a two-valued predicate over a +three-valued reality: all read, all unread, or mixed. The mixed case is the one +that has no correct toggle direction, and picking either one silently is what +ships as "the action does the wrong thing". + +**Approach.** The user has already named it: stop toggling at thread scope. + +- Split `toggle_unread_thread` into two explicit actions, **Mark thread read** + and **Mark thread unread**, each with a fixed direction. Both appear in the + "Whole thread" submenu, where an entry always carries text, so a fixed label + is honest in a way a toggle's cannot be. +- The message-scoped `toggle_unread` stays a toggle. One message has a real + two-valued state, so the trap does not exist there. Do not "unify" the two: + the asymmetry is the point. + +**Constraints.** + +- **Adding an action is four places**, all enforced by tests that fail + confusingly: `KeyMap::knownActions()`, `defaultBindings()`, the icon table, + and the no-duplicate-icons exception list. See `CLAUDE.md`. Splitting one + action into two means one new entry in each, and the pair shares the twin's + icon under the existing named exemption for thread actions. +- **`Ctrl+Alt+U` is taken by the action being split**, and the whole-thread + bindings are already one modifier out from their twins because `Ctrl+Shift+U` + was claimed. Two directions need two sequences; if a second chord cannot be + found that is not worse than the menu, bind one and leave the other to the + submenu rather than inventing a three-modifier chord nobody will press. +- **This interacts with items 98 and 99**, which is the reason to decide all + three together. 99 asks for a dynamic label on the message-scoped toggle, + which is the opposite move: keep the toggle, make the label tell the truth. + A thread cannot do that, because on a mixed thread there is no true label to + show. Deciding 99 first will produce the wrong answer here by analogy. +- The undo entry must name the direction that ran (`Mark thread unread`), not + the action. `tagSelected` already takes the text, so this comes free from + splitting. +- **The test needs a MIXED thread**, which is the whole defect: a thread whose + messages are all in one state answers identically whichever way the direction + is computed, so a fixture built from a uniformly-unread thread passes against + the bug. Same trap as item 88's opposite-states requirement, recorded in + `CLAUDE.md`. + +**Built 2026-08-25 to the USER'S NOTE, not to the approach above**, which had +this half right and was shipped that way first. The approach proposed splitting +the thread toggle and explicitly said to leave the message-scoped one alone, +deciding 99 separately. The user's note is ONE design across both, and the +entry's own constraint said so ("this interacts with items 98 and 99, which is +the reason to decide all three together") without following it. The half-built +version was handed over, corrected by the user, and rebuilt. + +Four parts, all of them the note's: + +- The thread toggle splits into `mark_thread_read` and `mark_thread_unread`, + both absolute. **Neither carries a default chord**, at the user's choice: + since item 132 a shortcut is a chosen subset, and `Ctrl+Alt+U` meant + whichever direction the union happened to pick, which is what made it wrong. + It is now unbound. +- The message-scoped `toggle_unread` STAYS a toggle, because one message has a + real two-valued state, and gains a label naming the direction it will go. +- On a selection with no single state that entry is **hidden**, chosen over + disabled by the user. There is no honest label for a mixed selection, and + the thread submenu is the route the note points at. +- The label follows a WRITE as well as a selection change, keyed on the + model's `dataChanged` rather than on the six call sites that apply an + optimistic update, so a new one cannot forget. Without it, marking the + current row read left the entry offering to do it again. + +`selectionTagPresence()` is the three-valued predicate this needed; +`everySelectedRowHasTag()` now delegates to it and keeps its two-valued +answer, which is all a DIRECTION needs. A label needs the third value, and +asking a two-valued predicate a three-valued question is what this item was. + +**Three mutations fail:** restoring the union predicate reports "wrong +direction on a mixed thread: Mark thread read", which is the user's original +symptom; showing the action on a mixed selection; and dropping the +`dataChanged` refresh. The suite is 37 of 38, the one failure being item 136 on +an unrelated path, and the four new strings are translated with `lrelease` +reporting 0 unfinished. + +**Closes 99 and 147 with it**, which were the same note recorded twice. + +**Size: S.** Done, at roughly twice the entry's scope because the entry's scope +was wrong. + +## 118. No way to empty the trash from inside the app + +**Observed (user, 2026-08-17):** raised while reviewing item 103's spec, as +something that had been forgotten rather than newly noticed: "we could add +'Empty Trash' to the backlog as a future item. I forgot it existed, but I don't +want to squeeze it in this spec." + +**Blocked on 103**, which creates the trash folder this would empty. Until that +ships there is nothing to empty: Delete writes a tag and moves no file, so no +account has a populated trash folder except through another client. + +**Deliberately excluded from 103's spec**, at the user's request and recorded in +its "Out of scope" section. Worth keeping separate for a reason beyond scope +control: emptying the trash is the first action in this application that would +destroy mail with no undo. Every mutation so far is a tag or, after 103, a move, +and both are reversible. A purge is not. + +**Approach, unspecified.** The shape depends on decisions not yet made, and the +spec for 103 answers none of them: + +- **Local or remote.** Deleting the files locally and letting `Expunge Both` + carry it to the server is one thing; asking the provider to empty its own + trash is another, and mbsync offers no verb for the latter. The first is + probably what "Empty Trash" should mean here. +- **Whether the no-confirmation rule survives it.** It does not, on the face of + it. `CLAUDE.md` grants undo in place of confirmation dialogs, and this is the + action where undo cannot exist. That makes it the second item, after 103, that + re-examines the rule rather than assuming it, and unlike 103 it will probably + have to break it. +- **Per-account or all-accounts**, which should follow whatever the Trash filter + does once 103 ships rather than being decided independently. + +**Built 2026-08-25**, unblocked by 103. The three questions the entry left open +were put to the user and answered: + +- **Local, and let the sync carry it.** The files go, and a channel with + `Expunge Both` propagates that to the server. mbsync offers no verb for + asking a provider to empty its own trash, so the alternative was to delete + locally and not care, which brings the mail back on the next sync and reads + as the action having silently failed. +- **It confirms**, naming the count and the account, defaulting to Cancel, with + no default shortcut. CLAUDE.md now records this as the ONE exception to the + no-confirmation rule, in the same paragraph that states the rule, so the next + reader meets both together. +- **Scoped to the account selector**, like every other account-aware surface, + which is what the entry asked for. + +`NotmuchWorker::purgeMessages()` is a separate entry point from +`moveMessages()` rather than a flag on it, because the two look alike and only +one can be undone. It takes named ids only, never a folder sweep, so the blast +radius is what the dialog enumerated and the user confirmed. It deletes EVERY +file of a message: notmuch deduplicates by Message-ID, and leaving one behind +would leave the message alive in the folder the user emptied, which is the same +one-message-many-files property item 166 turned on. + +`resolveQueryMessages()` is a four-line wrapper over the existing private +`resolveQuery()`, so enumerating what is about to be destroyed needed no new +walk. The count in the dialog comes from the DATABASE rather than the model, +which holds whatever the current view is showing and is usually not the trash. + +**A defect surfaced while writing the tests**, and it is the one worth +remembering: the first version counted a message whose file was already gone as +destroyed, so the number reported for an irreversible action overstated it. An +absent file is correctly not an ERROR, since the index can name a path a sync +has removed; the mistake was treating "not an error" as "destroyed". The +mutation that restores it now fails. + +**A second defect was found by the user's own hand test**: the mail was +destroyed correctly and the LIST went on showing it until they re-ran the query +themselves. A purge is the one mutation with no optimistic update available, +because it removes rows rather than changing them, so `messagesPurged` re-runs +the current query. Nothing was connected to that signal at all, which is the +kind of gap a green suite is happy to keep. + +Verified against the live index after the user emptied one real account's +trash: zero files on disk, zero in the index. + +**Item 168 was filed from the same hand test**, on Delete being offered on mail +already in the trash. + +**Size: S.** Done. + +## 168. Delete is offered on mail already in the trash, and does nothing + +**Observed (user, 2026-08-25, while hand-testing item 118):** "I noticed I can +hit delete via context menu on a message already in the trash. Seems like a +bug, unless that action doesn't do for one message what Empty trash does for +the whole view." + +It does not, and the guess in the second half is worth recording as the reason +this matters: the user's mental model was that Delete on already-trashed mail +might PURGE it. It does not, and nothing about the menu says so. + +**Cause (verified in code, 2026-08-25.)** `moveMessages()` compares the file's +directory against the destination and takes an early-return branch when they +match (`notmuchworker.cpp`, the "already where it was asked to go" branch, +added when a fresh Maildir name made a path comparison useless). That branch +appends the id to `moved` and records an origin, so the message is reported as +having moved when nothing happened. The UI counts an unsynced change for it. + +Nothing is destroyed and nothing is corrupted; the cost is a menu entry that +lies about having done something, and a pending-changes count that overstates +what a sync has to carry. + +**The mirror of the same defect is already shipped beside it.** `restore` is +added unconditionally to both the Message menu (`mainwindow.cpp:1956`) and the +thread context menu (`mainwindow.cpp:2119`), so it is offered on mail that was +never deleted, where it has as little meaning as Delete has in the trash. + +**Approach.** The user chose to hide each action where it has no meaning, +which is the principle item 112 established for the unread entry: an action +with no honest meaning for the selection is absent rather than present and +inert. + +- Delete is hidden when every selected row is already in a trash folder. +- Restore is hidden when no selected row is. +- The test for both needs a MIXED selection as well as uniform ones, for the + reason item 112 records: a selection whose rows agree answers identically + whichever way the predicate is computed. + +**Constraints.** + +- **The question is about the PATH, not the tag.** A message trashed by + another client carries no `deleted` tag at all, which is why item 103 made + the trash view path-based. Asking `tags.contains("deleted")` here would + offer Delete on exactly the mail the user is most likely to be looking at + in a trash view. +- **`selectionTagPresence()` is the wrong instrument** for the same reason, + though it is the right shape. A path predicate needs the row's path, which + `MessageNode` carries. +- Deciding this does not require deciding item 118's relationship to it: a + purge stays an explicit whole-view action, and hiding Delete does not make + Delete a purge. + +**A second request, from the same tangent (user, 2026-08-25):** "messages moved +to the trash should be automatically marked `-unread`." Deleting is a decision +about the message, so leaving it bold and unread in the trash is noise; the +count of unread mail should not include what the user threw away. + +It is one line where Delete already composes its tag change, and it carries a +constraint worth stating rather than discovering. `maildir.synchronize_flags` +is true, so removing `unread` REWRITES the Maildir filename and reaches the +server on the next mbsync. That is acceptable here and is a deliberate +exception: it is the same mechanism the `post-new` hook refuses to touch on +arriving mail, for the good reason that the hook acts unattended on mail the +user has not seen. A Delete is an explicit gesture on a message in front of +them, which is the difference. + +Undo must put it back. `TagChange::inverted()` already does, provided the +removal travels as part of the SAME change rather than as a second write, so +one undo returns both the folder and the tag. + +**Built 2026-08-25**, both halves, to the user's own choice of "hide each +where it has no meaning". + +`everySelectedRowIsInATrashFolder()` asks each row about its own file, a reply +row's message and a thread row's displayed message, the same rule +`everySelectedRowHasTag()` follows. `refreshTrashActions()` runs beside +`refreshUnreadAction()` on both the selection change and the model's +`dataChanged`, so the entries follow a write as well as a selection. + +The `unread` removal travels inside the SAME `sendMove()` call rather than as a +second write, which is what makes one undo return the folder and the tag +together. + +**A mutation survived the first round and is worth recording**: comparing the +prefix WITHOUT its trailing separator passed every test, because no fixture had +a folder whose name starts with the trash folder's. `acct/trash-old` is a +different folder, and under that mutation Delete silently disappeared from mail +that had never been trashed, which is the quiet half of the same mistake. The +fixture carries that row now and the mutation fails. + +All three properties are mutation-checked: the separator, Restore's visibility, +and the `unread` removal. The suite is 37 of 38, the failure being item 136 on +an unrelated path, and no new user-facing strings were added. + +**Size: S** for the visibility half, XS for the `unread` half. Done. + + +## 68. A forwarded subject gets no `passed` tag + +**Observed (user, from the notes):** "passed tag should appear when subject is +`Fwd:` and `Fw:`." Refined in session on 2026-08-11: the user had noticed +`passed` appearing on messages whose subject carried `Fwd:` and not on `Fw:`, +and asked to expand the rule to both. + +**Cause:** there is no rule to expand. `passed` is the Maildir `P` flag in the +message filename, translated into a tag by notmuch because +`maildir.synchronize_flags=true`. The flag is written by whichever client +forwarded the message, or by the server over IMAP; nothing reads a subject line +anywhere in the chain. qtmaildir only ever colours the tag +(`src/tagcolors.cpp:36-37`) and the database's `post-new` hook does not mention +it either. + +**Measured against the real database (2026-08-11):** + +| Query | Count | +|---|---| +| `tag:passed` | 6 | +| `tag:passed and subject:"Fwd:"` | 1 | +| `tag:passed and subject:"Fw:"` | 0 | +| `subject:"Fwd:" and not tag:passed` | 194 | +| `subject:"Fw:" and not tag:passed` | 28 | + +Six tagged messages in the whole database, and every one of them carries `P` in +its filename flags. The single overlap with `Fwd:` is a message that was +forwarded and whose subject was already a forward, not evidence of a rule: 194 +`Fwd:` subjects carry no tag at all. The correlation the observation rests on +does not exist. + +**Approach and the decision it needs first.** Two different features, and the +measurements above decide how far apart they are. + +*Display only.* The card shows a forwarded mark when the subject matches. Touches +no mail, changes no flag, reversible by deleting the rule. XS. + +*Write the tag.* qtmaildir sets `P` from a subject heuristic. With +`maildir.synchronize_flags=true` that flag is a filename change that mbsync +carries out to the server, on 222 existing messages, on a guess about a string. +Not cleanly undoable, and it asserts a meaning for a flag this application did +not define. Recommended against; recorded so the choice is deliberate rather than +forgotten. + +**Constraints:** localised clients use their own prefixes, and `Fwd:` can appear +inside a subject rather than at its head, so whatever matches must be anchored. +If the tag is ever written, it must not be re-applied on every sync in a way that +produces pending edits the user never made, item 28 is the record of a count +going wrong. The display-only route avoids that entirely, since it derives the +mark at paint time and stores nothing. + +**Size: S** as written, XS if it is display only. Most of it is the decision, not +the code. + +**Status:** left open deliberately on 2026-08-11. The cause is settled and the +options are costed; the user has not chosen, and no code was written. + +**Built 2026-08-26, and the observation was wrong in a way worth recording.** +The note asked for one thing (expand a subject rule to `Fw:`) and the +measurement above had already shown there was no subject rule and no +correlation to expand. Taken literally the item was unbuildable; taken as what +the user actually wanted ("I want to know visually if someone has forwarded a +message to me") it split into three, and the user chose all three. + +**1. `replied` on a reply, `passed` on a forward.** The gap the item was +really sitting on, and it was never reported. Measured 2026-08-26 against the +developer's own index: 317 `replied` and 6 `passed`, spread over five +accounts, every one of them written by another client or the server. Nothing +in qtmaildir has ever written either flag. `ComposeWindow` emits +`sourceMessageAnswered` after a SUCCESSFUL send; `MainWindow` routes it +through `sendMessageTagChange`, message-scoped, off the undo stack for the +reason `markCurrentThreadRead` gives (the flag records that the mail went, and +the send cannot be undone, so an undo that retracted only the flag would leave +the two disagreeing). + +**Two traps here, one of which was caught only by reading.** `inReplyTo` is +deliberately EMPTY on a forward (carrying In-Reply-To would file the forward +under the thread it left, in the recipient's client), so keying the emit on it +made the `passed` half dead code that compiled and never fired. `ComposeContext` +carries `sourceMessageId` instead, set for all three kinds. And a resumed +`Kind::Draft` is excluded: its kind records how the FILE was opened, not what +the user is doing, so a draft that began as a reply cannot be told from one +that began as a new message. The cost is a missing flag on a reply finished in +two sittings, which is the safe direction, since `maildir.synchronize_flags` is +on and a wrong flag reaches the server. + +**2. A received-forward mark, display only.** `Marks::Mark::ReceivedForward`, +a seventh SVG, drawn from `ThreadListModel::IsReceivedForwardRole` in BOTH the +thread and the message branch per CLAUDE.md's rule. It is a different mark +from `passed` on purpose: `P` means "I forwarded this", which is a different +fact about a different person, and setting it from a subject guess would +assert something false on 222 existing messages and propagate it to the +server. Derived at paint time, stores nothing. + +**3. `[general] forward_prefixes`.** `subjectIsForwarded()` lives beside +`forwardSubject()` and shares its prefix table, so "do not double the prefix" +and "this is a forward" cannot drift apart. The config key EXTENDS that table +rather than replacing it, so adding a locale does not lose the measured +English/German/Iberian/French spellings. A `Re:` chain is stripped first +(bounded to 8, since the subject is input from a stranger and this runs per row +per repaint), so `Re: Fwd: x` is recognised. + +**A mutation survived the first round and corrected a claim in the code.** The +word-validation guard on a configured prefix was commented, and tested, as +protecting against an invalid pattern from an unescaped `(`. Measured with a +standalone probe: `QRegularExpression::escape` already makes punctuation inert +rather than invalid, so that test passed against the guard being removed. What +the guard actually buys is narrower and real: a configured `-` would match +`-: x` and a digit would match `2: x`. The comment and the test now assert +that instead. + +**Not built, and left as the item's own recommendation:** writing `P` from a +subject heuristic. Rejected on the same grounds the entry gave before the +work started. + + +## 119. The unsynced-changes count cannot be opened to see what it counts + +**Observed (user, from the notes):** "the bottom left statusbar message needs to +be clickable and show what 'N unsynced changes' are in a modal window". + +**Cause (verified in the code).** `m_pendingLabel` is a plain `QLabel` added to +the status bar with `addPermanentWidget` (`src/mainwindow.cpp:502-505`). A +`QLabel` has no clicked signal and none is installed, so there is nothing to +click and no route to a list. It carries a tooltip and nothing else. + +**The count is a SUM OVER FOUR SOURCES, and that is what makes this bigger than +it looks.** `pendingEditCount()` returns +`m_pendingTagEdits.size() + m_unnettablePendingEdits + held + heldMoves`. +Three of those can name what they hold: `m_pendingTagEdits` is a +`QHash<QString, bool>` keyed by message id, `m_heldEdits` and `m_heldMoves` are +queues of edits waiting for a sync to end. **`m_unnettablePendingEdits` is a +bare `int`** (`src/mainwindow.h:1248`), deliberately so: it counts confirmed +changes that carry no message ids and therefore cannot be netted against +anything. + +So a dialog built from what is currently kept would list three of the four +groups and then have to account for a remainder it cannot describe. Showing "and +3 more" is worse than the tooltip, because the user opened the window +specifically to find out what those were. + +**Approach.** Two halves, and the second is the real work. + +- The clickable half is small: a label that emits on click (an event filter, or + a flat `QToolButton` styled as a label), plus a dialog listing what the three + describable groups hold. The message pane already resolves an id to a subject. +- The complete half needs `m_unnettablePendingEdits` to become something that + can name its entries. Its comment says why it is an int: understating the + indicator is the direction that costs the user work, so it counts what it + cannot identify rather than dropping it. Making it describable means finding + out what those changes actually are and whether they can carry an id. + +**Constraints.** + +- **The count is deliberately conservative and must stay so.** Item 28 and item + 54 both landed on this indicator being wrong in the direction that made the + user think their work was safe. A dialog that lists fewer changes than the + count claims is the same failure in a new place: reconcile the two, or state + the remainder honestly rather than hiding it. +- **An external `notmuch` run can clear pending changes without this count + noticing**, which the tooltip already admits. A dialog makes that staleness + much more visible, since a listed change may no longer exist. Worth deciding + whether the dialog re-verifies against the database before showing. +- Read-only. This is an information window, not a place to retry or discard a + change; either would be a new mutation path with its own undo question. + +**Size: S** for the clickable half over the three describable groups. **Unknown** +for the fourth, and the item is not complete without it. + +**Closed 2026-08-26.** The blocker above was investigated first and did not +survive: `m_unnettablePendingEdits` counted confirmed changes carrying no +message ids, and `NotmuchWorker::applyTags()` (the only emitter of +`tagsApplied`) returns early on an empty id list, which is that exact +condition. `applyTagsToThreads()` resolves through a query and errors out on +an empty result, so it cannot hand `applyTags()` an empty list either. + +**Measured rather than read**, twice, because reading is what produced the +wrong answer the first time: a `qFatal` in the branch fired in 4 of 70 +`test_mainwindow` cases, all four building a `TagChange` by hand and invoking +the slot directly with no worker, and a `Q_ASSERT` before the worker's own +emit never fired across the whole suite. The counter was deleted and the +guard it shadowed is pinned where it lives, by +`applyTagsWithNoIdsDoesNothing()` in `test_notmuchworker`. + +Built in four commits: the snapshot, the subject resolve, the dialog and the +click, then a sizing fix after a hand test. + +Three decisions the user made, each of which shapes the code: + +- **Scope follows the ACTION, not the storage.** A thread action shows one + thread row with the count of messages it covered; a message action shows + its message. The three queues already encoded this, so nothing is expanded + and nothing is escalated: `HeldEdit` is thread-scoped because a `*_thread` + action made it, and everything else carries message ids. +- **A snapshot, frozen.** Taken at the click and never refreshed under the + user, who asked for exactly this: "if I keep the popup open for 20 minutes, + I don't want the popup to keep updating the info it's showing me." +- **Subjects resolved, stale rows kept.** An id the index no longer holds + still gets a row saying its subject is unknown, because the count the user + clicked has to equal the list they are shown. + +The re-verify question this item worried about mostly dissolved: item 54 +already clears the count when an external sync carries the edits, so a change +applied by cron does not survive to be clicked on. + +`resolvePendingSubjects()` answers POSITIONALLY, one subject per input row, +because one id can legitimately appear on several rows and a combined query +returns a set. `PendingChangeRow::startsMessage` is carried rather than +inferred from a non-empty subject, so an unresolved id still opens a run of +its own instead of folding its actions under the message above it. + +Read-only, per the constraint above. The dialog's height is sized to its +content; that is a hand test, since the offscreen platform returns an +identical frame either way. + + +## 169. A card shows the account only as a bar, with no fade and no avatar + +**Observed (user, from the notes):** "the left border of a card expresses the +account the mail belongs to. the background color of the card should fade left +to right from the account color to the current background color we are using (or +to transparent to work both in light and dark themes). On the left we should +leave room for an account avatar (a squircle), for now it could be extracted +from the sender name "From: john doe" becomes "JD" in the avatar. As soon as we +include khard (or some other vcard provider/manager) we will switch to images if +the corresponding vCard has one." + +**Cause (verified in the code):** not a defect. Half of it shipped. The account +colour is drawn as a solid bar down the left edge, `CardLayout::accentRect` +placed by `CardLayout`, filled by `CardDelegate::paint()` with +`CardDelegate::accentLineColour()`. There is no gradient anywhere on a card, and +nothing draws an avatar: `CardLayout` reserves no rect for one, so the geometry +would have to grow before the painting could. + +**Approach.** Two separable pieces, and the avatar is the one that changes the +layout. + +- The fade is a `QLinearGradient` fill over the card rect, from the accent + colour to the pane's background. `accentLineColour()` already records why + blending toward the background is wrong for a CHIP; a card's background is + exactly where such a blend belongs, so the constraint does not carry over. + Both themes come free if the far stop is the palette's own base rather than + a literal. +- The avatar needs a rect in `CardLayout`, which is where it becomes testable + without a painter, and it shifts `contentLeft` for every card. The initials + come from the display name already carried on the summary; a sender with no + display name (an address only) needs an answer before this is built. + +**Constraints.** + +- The vCard half is blocked on item 72, which is itself unspecified. Build the + initials only; do not design the image path in advance. +- A gradient behind the text has to keep the text readable at the left edge in + both themes, which is the same failure mode `accentLineColour()` guards + against on a dark palette. +- This is a looks question, so it is settled by the user looking at it rather + than by a test: assert the geometry in `CardLayout`, and hand the appearance + over per `tests-only-for-measurable-things`. + +## 172. A draft this application writes is tagged `unread` + +**Observed (user, 2026-08-27):** a draft they had edited was sitting in the +Unread view. Reported first as "in the inbox view", corrected to Unread. + +**Cause (measured, 2026-08-27).** `ComposeWindow::saveDraftNow()` called +`DraftStore::write(folder, bytes, "D", ...)`, and `DraftStore::write()` uses +the flag string verbatim, so every draft this application wrote landed as +`:2,D`. `maildir.synchronize_flags` is on, and notmuch tags any message +lacking the `S` (seen) flag `unread`. A draft the user authored is seen by +definition, so the tag was wrong the moment the file was written. + +**Why it looked intermittent, which is the part worth keeping.** The symptom +heals itself: the next mbsync of that folder round-trips the file, adds `S`, +and the tag goes away. On the developer's own mail two drafts written two +minutes apart differed only in whether their folder had synced afterwards: +one account's drafts folder had synced the next morning and its file read +`,DS`, while the other's had last synced two minutes after the write and read +`,D`. So only the newest draft in a folder that has not synced since shows +it, and an investigation that measures an older draft finds nothing wrong. + +**A measurement trap sat in front of this and cost the first answer.** +`notmuch search --output=tags` reports the union over a THREAD. A reply-draft +attached to an inbox message therefore reads `draft inbox unread` while no +single message carries both, which is the same union recorded for +`ThreadSummary::tags` under item 110. The first pass here read that union as +a draft carrying `inbox` and concluded there was no defect at all. Measure +drafts with `--output=messages`; item 164's evidence is a thread-level +reading and should be re-measured before it is worked on. + +**Fixed** by passing `"DS"`. `TestComposeWindow::aSavedDraftIsFlaggedSeen()` +asserts both flags on the written filename, verified failing first (`got D`). + +`TestMainWindow::anAutosaveWritesADraftAndClearsTheDirtyFlag()` had to be +repaired in the same commit: it asserted `endsWith(":2,D")`, pinning the whole +flag set when its own comment said the point was the draft flag "not left +bare". It therefore failed against the corrected behaviour. An +over-specified assertion of this shape blocks the fix rather than the bug. + +## 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.** + +- **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. +- 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. + +--- + +**RE-MEASURED 2026-08-27, and the item is DROPPED: there was never an `inbox` +tag on a draft.** Everything above this line is the investigation of a defect +that did not exist, and it is kept because the way it went wrong is worth more +than the conclusion. + +The premise came from `notmuch search --output=tags`, which reports the union +over a THREAD. A draft replying to an arrived message sits in that message's +thread, so the union reads `draft inbox unread` while the two tags live on two +different messages. Measured today on the thread that produced the original +report: + +- the arrived mail: `['account-<acct>', 'inbox']` +- the draft reply: `['draft', 'unread']` + +Neither carries both. Across the whole index, `notmuch count --output=messages +'tag:draft and tag:inbox'` is **0** against 12 drafts, nine of which were +written on or before 2026-08-25 and so were present when this was filed. + +**The trap has a second half that makes it much easier to fall into.** A +thread-level `notmuch count 'tag:draft and tag:inbox'` ALSO returns 0, because +search terms match per message even in a thread query. So the count and the +displayed tag list disagree, and the displayed list is the one that looks like +evidence. Use `--output=messages` and `notmuch show` when asking what tags a +message carries; `--output=tags` answers a different question than it appears +to. + +This is the same union recorded for `ThreadSummary::tags` under item 110, where +it made a card claim a tag its message did not have. It cost this item a week +open, two wrong causes, and a seven-variant reproducer built to explain an end +state that a union produces for free. It also caught a fresh reader of this +backlog on 2026-08-27, who read the same union and reported that drafts were +carrying `inbox` before measuring at message level. + +**The `unread` half of the original observation WAS real** and is item 172: the +app wrote drafts as `:2,D`, and notmuch tags anything without `S` as `unread`. +That is fixed. The reported tag set `draft inbox unread` is fully explained: +`unread` from the missing `S` flag on the draft, `inbox` from the arrived +message sharing its thread. + +## 171. A forwarded HTML message reaches the recipient as plain text + +**Observed (user, from the notes):** "forwarding an html message doesn't +maintain the html formatting of the original message. #bug" + +**Cause (verified in the code, 2026-08-27).** The forward path builds its body +through `ComposeContextBuilder::quoteBody()` (`src/composecontext.cpp`), which +reads `message.plainBody` and nothing else. `MimeParser` parses both halves and +`ParsedMessage` carries `htmlBody` beside `plainBody` (`src/mimeparser.h:150`), +so the HTML is available and simply never asked for. + +Two consequences follow, and they are not the same severity: + +- An original with both parts forwards its text/plain alternative, losing the + sender's formatting. Recoverable-looking, since the words survive. +- An original with an HTML part ONLY has an empty `plainBody`, so the forward + carries the attribution line and an empty quote. The message's content is + gone, and nothing says so. + +`MainWindow::composeReply()` already treats the two kinds differently for the +composer's own HTML state: `context.seedHtml` is the CONFIG's `sendHtml` for a +forward and `original.hasHtml()` for a reply, on the stated reasoning that an +HTML part is a fact about the sender's software. That reasoning is sound for +how the user WRITES and does not decide what the forward CARRIES, which is the +question here. + +**Approach.** The decision comes first; this is not a changed call site. + +A forward is a different act from a reply: the point is to hand somebody else +what arrived, and quoting is the wrong shape for it. Three candidates, in +increasing fidelity: + +- Render `htmlBody` down to text when `plainBody` is empty, so nothing is + silently lost. The smallest fix, and it does not answer the note: formatting + is still gone. +- Carry the original as a `message/rfc822` part, which is what item 130 already + describes and what GMime builds natively. Perfect fidelity, and every + attachment comes with it, but the recipient sees an attached message rather + than a body. +- Build the forward as `multipart/alternative` with the original's HTML nested + in the HTML half, which is what Thunderbird's inline forward does. + +**Constraints.** + +- **The HTML is input from a stranger and the composer is not the message + pane.** The pane's protections (off-the-record profile, JavaScript off, the + interceptor blocking every request) are `MessageView`'s, not + `ComposeWindow`'s. Any route that puts the original's markup into an outgoing + message must decide what it strips, and remote references in particular: + forwarding a tracking pixel forwards the tracking to the new recipient. +- Item 130 overlaps and may subsume this. Decide the two together rather than + building `message/rfc822` twice. +- The markdown body is the composer's source of truth, and markdown has no + syntax for arbitrary HTML the user can then edit. A route that keeps the + original's markup has to keep it OUTSIDE the editable buffer, which is the + same nesting problem item 129 carries. +- `quoteBody()` is shared with Reply. A change there reaches both; the + behaviour asked for is the forward's alone. + +--- + +**BUILT 2026-08-27.** Design in +`docs/superpowers/specs/2026-08-27-forward-html-design.md`, which is the +document to read; this entry records only what changed and what was learned. + +The user chose **carrying the original's markup inline** over attaching the +original as `message/rfc822` (item 130's mechanism, still open for its own +sake) and over a text-only fallback, and chose **strip remote content by +default with a per-forward opt-out** over always stripping and over keeping +everything. + +**Amended the same day, after the first build**: a forward sends ONE part +rather than a `multipart/alternative`, chosen by the Send-as-HTML toggle. The +first build sent both halves and also FORCED html on when there was markup to +carry; both were reversed. A forward's shape is something the user has already +decided by flipping that toggle, and sending both hands the choice to the +recipient's client. The consequence was put to the user explicitly and +accepted: with the toggle off, an HTML-only original forwards as the text +fallback and its formatting is lost. + +Four parts, each independently useful: + +1. **`HtmlSanitiser`** (`src/htmlsanitiser.h/.cpp`), a namespace of free + functions so the security property is testable without a widget. +2. **`quoteBody()`'s empty-plain fallback**, via + `QTextDocumentFragment::fromHtml().toPlainText()`. Closes the silent half + on its own. +3. **The MIME nesting** in `MessageBuilder`, asserted by parsing the result + back through `MimeParser` rather than by reading the RFC. +4. **The composer control**, created only when the original actually carries + remote content. + +**The allow-list rule is the part to preserve.** `HtmlBuilder::namespaceCids()` +is a block-list and documents scoping `srcset=` out; that trade is right for +rewriting and wrong for stripping, because a missed rewrite is a broken image +and a missed strip is a beacon reaching the recipient. `HtmlSanitiser` judges +every attribute by its VALUE, so `srcset`, `poster`, `data-*` and whatever HTML +adds next are handled by the default, which is removal. + +**A real bug was caught by the tests and is worth recording**: the walk used +`QRegularExpression::globalMatch()` while also advancing `pos` past a removed +element's content. `globalMatch` iterates over matches found against the +ORIGINAL string, so it handed back tags from inside the region just skipped; +the output duplicated content and an `<iframe>` survived. It matches by hand +from `pos` now. A tidy test would not have found this: it needed a removed +element with content, mid-document, followed by more markup. + +**Measured, not assumed:** 30 of 342 sampled inbox messages (~9%) declare +`text/html` with no `text/plain`, so the silent-loss half was not an edge case. + +**Hand-tested 2026-08-27, and it found two defects, the second of which +changed the design.** + +1. The original shipped TWICE inside the one HTML part: the composer seeds the + text quote into the editable body, so `markdownBody` already carried a + flattened copy, and the markup was appended to it. It read as two messages + stacked, the first with its URLs naked and mangled. A screenshot of a real + forward is what showed it; no test had covered the composer and the builder + together. + +2. The first fix subtracted the quote in `MessageBuilder`. **The user rejected + it**: the quote was still shown in the composer and no longer sent, so it + could be edited and the edits silently discarded. "If it is in the composer + but it's not sent, is worse." That is the right objection and the general + principle behind it, **what the composer shows must be what gets sent**, is + what the design now follows. + +So an HTML forward **does not seed a text quote at all**. The buffer holds the +user's own note alone, and the forwarded message appears in a read-only pane +BESIDE the editor, a `QSplitter` at 60/40 with a toggle in the Format menu (the +user's chosen arrangement). A plain forward is untouched: its quote is in the +buffer, where it is both editable and sent, so WYSIWYG already held there. + +The pane is a `QTextBrowser`, not a `QWebEngineView`: a web view would mean a +second Chromium render process per composer and a second copy of MessageView's +protections. The cost is that Qt's HTML subset is narrower than a mail +client's, so the pane shows the original ROUGHLY. Its label says the message +is sent as it arrived, so the pane is not mistaken for what travels. + +**The user asked for a real rich-text composer as the proper answer**, recorded +as item 173, which supersedes this middle ground and subsumes item 133. + +Two test traps hit while building it, both already in CLAUDE.md and both hit +anyway: the offscreen platform gives a `QSplitter` no width, so `sizes()` +reports 49/49 whatever the code asks and a pixel assertion fails against +correct code (assert the stretch factors, which land in the child's size +policy since `QSplitter` has no getter); and moving the editor into a splitter +broke `theComposerSplitsItsToolbarByScope`, which looked for the body directly +in the composer's column. + +Still to do: the hand test of the new arrangement. Forward a real HTML message +with the box checked and unchecked and confirm what arrives. + |