diff options
Diffstat (limited to 'docs/superpowers')
5 files changed, 809 insertions, 208 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 39026cb..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 @@ -8607,3 +8607,484 @@ 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. + 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 5e95804..9cb8ba9 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 @@ -185,7 +185,7 @@ taking that too literally. | 116 | Copy image copies markup instead of the image | defect | XS | **dropped** 2026-08-17, same day. NOT A DEFECT: `wl-paste --list-types` run immediately after a copy reports `image/png`, `application/x-qt-image` and 30 more image flavours. The clipboard is correct and Chromium is behaving. The earlier "text only" reading was taken minutes late off a clipboard that had been overwritten, and a whole cause was theorised on it | | 117 | The message pane offers no Select all | workflow | XS | **done** 2026-08-19, unreleased. `addPaneActions()` supplies it. The call site is NOT covered by a test and cannot be: the production menu needs a real context-menu event. Stated in the test rather than faked | | 118 | No way to empty the trash from inside the app | workflow | S | **done 2026-08-25**, unreleased. Unblocked by 103. `Message > Empty trash...`, scoped to the account selector, no shortcut. The one confirmation in this application, and CLAUDE.md now records it as the single exception rather than leaving it to be discovered. Found a defect while testing: the count claimed messages whose files were already gone | -| 119 | The unsynced-changes count cannot be opened to see what it counts | information | S | open, 2026-08-19, from the notes. One of the four things it sums carries no message ids at all, so a list cannot be complete without a change to how the count is kept | +| 119 | The unsynced-changes count cannot be opened to see what it counts | information | S | **done** 2026-08-26, unreleased. **The stated blocker was not real**: the fourth term counted confirmed changes with no message ids, and `applyTags()` returns early on exactly that condition, so it could never fire. Measured before removing it, not read. The label opens a read-only list, grouped as the user asked: subject once, actions beneath. Scope follows the ACTION, so a held thread edit stays one thread row and reports its message count. A snapshot, frozen once open | | 121 | The thread list shows nothing while a query is running | feedback | S | open, 2026-08-20, from the notes. Follows item 74, which fixed the status-bar half and left the list itself blank | | 122 | The README documents a version of the app that no longer exists | documentation | M | **done** 2026-08-23, unreleased, inside item 123 task 13. `trash`, `send_command` and the whole `[compose]` section were undocumented; a Composing section is added and "sending is not implemented" removed. Every default was read from `config.h` rather than from the prose, which caught `send_html` documented as false when it defaults to true | @@ -202,7 +202,7 @@ taking that too literally. | 130 | A message cannot be attached to another message directly | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `message/rfc822` part, which GMime builds natively. The manual route exists from 123's first commit: `save_message` writes the `.eml` and it is attached as a file | | 131 | The markdown dialect and extensions are fixed | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** Configurable in the shape Hugo's config uses. Deliberately fixed initially: CommonMark plus autolink, strikethrough and tasklist | | 132 | Every action must have a shortcut, and that no longer serves | policy | S | done, 2026-08-20. `everyActionHasAShortcut` is deleted and nothing replaces it: `everyActionIsReachableFromAMenu()` is the required rule and a shortcut is now a chosen subset. Nothing else needed changing, since `showShortcutReference()` already printed `(unbound)` for an empty sequence. Verified by unbinding `tag_rules` and running the suite green, which would have failed before | -| 133 | The composer shows no markdown syntax highlighting | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `QSyntaxHighlighter` over the composer's editor, so `**bold**` reads as bold while the buffer stays plain markdown. Standard Qt, no dependency. Deliberately after 123's formatting toolbar: agreeing with the grammar about nesting and about code spans suppressing what is inside them is the expensive part, and the toolbar is what makes the feature usable | +| 133 | The composer shows no markdown syntax highlighting | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `QSyntaxHighlighter` over the composer's editor, so `**bold**` reads as bold while the buffer stays plain markdown. Standard Qt, no dependency. Deliberately after 123's formatting toolbar: agreeing with the grammar about nesting and about code spans suppressing what is inside them is the expensive part, and the toolbar is what makes the feature usable . **Subsumed by item 173** 2026-08-27: that is a rich-text editor, this is the cheap answer to the same want. Build one or the other, never both | | 134 | The busy indicator is built inline and is about to be built twice | maintenance | S | done, 2026-08-20, af902e0. `BusyIndicator` (`src/busyindicator.h`) carries both modes: `MainWindow` uses the indeterminate one, and item 123's send popup takes the determinate half for its undo countdown, switching the same widget over when the command starts. Only the BAR was extracted, not the status label this row paired with it. `m_statusLabel` has 34 uses across `MainWindow` for transient messages, selection counts and sync phases, so it belongs to the window rather than to the indicator, and the send popup owns its own phase text | | 135 | The formatting toolbar's buttons stack rather than toggle | v2 | S | open, 2026-08-21, asked for by the user during item 123 task 8 and reverted the same session. **A spec change, not a defect**: it conflicts with spec:236 ("deliberately no live toggle") and spec:187-190. Both sites need amending FIRST, and the amendment must resolve what replaces bold-then-italic, which is the gesture spec:187's preserved selection exists to serve and which a toggle makes unreachable. That question is the work; the state machine is understood and written up in the section | | 136 | `undoMovesTheMessageBack` fails when run ALONE, passes in the full suite | defect | ? | open, 2026-08-21, re-measured 2026-08-24 and it is not what the row said. Filed as an intermittent race (1 in 6); it is in fact **deterministic on the selection**: 6 failures in 6 when named on the command line, and, as of 2026-08-24, it fails in the FULL run too: measured at 58f13ad with the day's work stashed out, 274 passed and this one failed. The "passes in the suite" half of this row is therefore no longer true, and the selection-dependence it was named for may not be either. Re-measure before theorising. All three of its 15s `QTRY` timeouts expire, giving 45s against a 25s whole-suite run, so undo never moves the file rather than losing a race. A test that needs its predecessors is the likely shape (the `init()` lock-table fixture of item 61 is one candidate), which makes it a TEST defect until shown otherwise. Not caused by item 149, and re-confirmed 2026-08-24 as not caused by item 152 either, by running the test at the preceding commit in a throwaway worktree. The assertion that fails names the real question: the restored file is in NEITHER `cur` nor `new` of the account inbox, so establish where it went before theorising about a race | @@ -215,7 +215,7 @@ taking that too literally. | 143 | The formatting buttons are text, where every editor uses icons | presentation | XS | **done** 2026-08-24, unreleased, inside 142. `QIcon::fromTheme` per CLAUDE.md's chrome rule, the words kept as the tooltip, and an action whose theme lacks the name keeps its text rather than rendering an empty button | | 144 | "Also send a formatted copy" is prominent and does not say what it does | presentation | XS | **done** 2026-08-24, unreleased, inside 142. "Send as HTML", icon and text, alone at the right end of the editor bar where it reads as a control of the editor rather than as a formatting button. The Italian entry was refreshed with it, and `lrelease` reports 477 finished, 0 unfinished | | 145 | Cc and Bcc are permanent rows on every composer | presentation | S | **done** 2026-08-24, unreleased, inside 142. A `QToolButton` disclosure beside To:. `revealCcBccIfUsed()` is the load-bearing half the entry called for: it only ever SHOWS, never hides, so nothing but the user's own click can make a field holding an address invisible. `ComposeContext` carries no `bcc` at all, so the seeded-Bcc case can only arrive from a reopened draft, which is what its test drives. The LABEL is hidden with each field: a `QFormLayout` holds the two as separate items, so hiding the line edit alone strands a `Cc:` over empty space | -| 146 | The unsynced-changes count cannot be opened to see what it counts | information | S | **duplicate of 119**, recorded 2026-08-23 from the notes. Same request, and 119 already carries the blocker: one of the four things the count sums holds no message ids, so a list cannot be complete without changing how the count is kept | +| 146 | The unsynced-changes count cannot be opened to see what it counts | information | S | **done as 119** 2026-08-26. Duplicate, recorded 2026-08-23 from the notes; closed by the same work | | 147 | Toggle unread reads the same whichever way it will go | presentation | S | **duplicate of 99**, recorded 2026-08-23 from the notes, and closed with it on 2026-08-25 | | 148 | Ctrl+W does not close the composer | discoverability | XS | **done** 2026-08-24, unreleased. A `QAction` parented to the composer, so it is a WindowShortcut dispatched to the active composer only and the main window's namespace is untouched, exactly like the formatting shortcuts. It calls `close()` rather than doing anything of its own: `closeEvent()` already decides whether the draft is saved, and a second route out that skipped it would lose the message. Not registered in `KeyMap`, so item 132's rules do not apply | | 149 | A reply's cursor lands on the attribution line, not on blank space | defect | XS | **done** 2026-08-24, unreleased, in TWO passes. The first fixed the cursor within each branch (`End` under Above, `Start` under Below) and the user still saw the old layout, because the branches were already right and the DEFAULT was wrong: `above` shipped, and the layout asked for is what `below` produces. Default flipped, and the composer now focuses the body whenever To: is already filled, which a Reply and a Forward always are. Both halves were invisible to the existing `theQuotePositionDecidesWhereTheQuoteLands`, which asserts the quote's position and never the cursor's | @@ -237,13 +237,16 @@ taking that too literally. | 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 | +| 164 | A draft this application saved keeps `inbox` | defect | S | **dropped** 2026-08-27, NOT A DEFECT. The premise was a measurement artifact: its evidence was `notmuch search --output=tags`, which DISPLAYS the union over a thread, and a reply-draft under an arrived message reads `draft inbox unread` while no message carries both. Re-measured at message level: 0 of 12 drafts carry `inbox`, including nine written on or before 2026-08-25. The `unread` half was real and is item 172 | | 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 | **done 2026-08-25**, unreleased. `sent_only()` keeps a message only when EVERY file is inside a sent folder, which is what the carve-out's docstring already claimed. No query can express it, measured; the root comes from `database.mail_root`, with a split-index fixture the ordinary layout cannot provide. Verified read-only against the live index: 780 of 807 still stripped, 27 spared, no arrival affected | | 167 | No way to tell one build of an unreleased version from another | enhancement | XS | **done 2026-08-25**, unreleased. The user chose a counter over a git description: `QTMAILDIR_BUILD_NUMBER`, a cmake option ON by default, increments a counter in the BUILD directory on every build and writes `buildnumber.h`. `QTMAILDIR_VERSION_DISPLAY` carries it; `QTMAILDIR_VERSION` stays clean and is what the window title, `applicationVersion` and the release procedure use | | 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | **done 2026-08-25**, unreleased. Delete is hidden when every selected row is already in its account's trash, Restore when none is, both keyed on the PATH rather than the `deleted` tag. Delete also drops `unread` now, in the same TagChange so one undo returns the folder and the tag together | -| 169 | A card shows the account only as a bar, with no fade and no avatar | presentation | M | open, 2026-08-26, from the notes. The accent bar exists (`CardLayout::accentRect`, `CardDelegate::accentLineColour()`); the gradient fade and the sender avatar do not. The avatar's initials source is decided, the vCard half is blocked on item 72 | +| 169 | A card shows the account only as a bar, with no fade and no avatar | presentation | M | **done** 2026-08-26, unreleased, on `card-avatars`, merged fast-forward. Both halves: a `QLinearGradient` from the account colour to the pane's base across the card, and a squircle avatar with initials, given a rect in `CardLayout` so the geometry is asserted without a painter. **Hand-testing found four defects**, all fixed in 9ae43f9: `Avatar::initialsFor()` normalises the display name first (drops the angle-addr, takes the first comma-separated author, unwraps quotes, treats a bare address as no name, requires a word to carry a letter or digit); the two-tone gradient axis spans the DIAMETER rather than a radius, which was letting one hue fill the whole face; the account fade runs right to left, anchored opaque at the card's right edge; and a flat view hashes `ThreadSummary::firstMessageRecipient` rather than the user's own address. The vCard half stays blocked on item 72 | | 170 | A row that stops matching the view only leaves it on the Delete path | defect | S | open, 2026-08-26, from the notes, **cause found the same day and the premise is NOT stale**. The optimistic REPAINT is universal; the optimistic MEMBERSHIP is not. `removeThreadsWithoutTag()` has exactly one caller, on the move path, so marking a message read in the Unread view repaints the row and leaves it in a list it no longer belongs to | +| 171 | A forwarded HTML message reaches the recipient as plain text | defect | M | **done** 2026-08-27, unreleased. Design in `specs/2026-08-27-forward-html-design.md`. The user chose inline `multipart/alternative` over attaching the original, and remote content stripped by default with a per-forward opt-out. Four parts: `HtmlSanitiser` (an ALLOW-LIST, unlike `namespaceCids()`, because a missed strip is a beacon where a missed rewrite is a broken image), a text fallback for the ~9% of mail with no plain part, the MIME nesting, and the composer control | +| 172 | A draft this application writes is tagged `unread` | defect | XS | **done** 2026-08-27, unreleased. `DraftStore::write()` was called with `"D"`, and `maildir.synchronize_flags` makes notmuch tag anything without `S` as `unread`. Self-healing on the next sync of that folder, which is what made it look intermittent | +| 173 | The composer is a plain-text editor, not WYSIWYG | v2 | L | open, 2026-08-27, **asked for by the user** while hand-testing 171. This is a GUI mail client and should edit rich text the way one does: the forwarded original, and the user's own formatting, visible and editable in place. Supersedes the preview 171 shipped as a middle ground, and **subsumes item 133** (markdown syntax highlighting), which is the same want answered cheaply. See the entry: the draft format and the markdown-as-source-of-truth model both change | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -640,60 +643,6 @@ make Save image work must not make Save link reachable again. The test fails if it does, which is the point: the handler is per-profile, so the natural implementation would light up both entries at once. -## 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. - - ## 121. The thread list shows nothing while a query is running **Observed (user, from the notes):** "can we show a spinner in the left panel @@ -1144,109 +1093,6 @@ 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. -## 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. - ## 165. A draft gets a new Message-ID on every autosave **Observed (developer, 2026-08-25), while hand-testing items 163 and 164.** @@ -1315,49 +1161,6 @@ id and a draft of a reply carries both. replaced correctly now, so the fork this would have mitigated no longer happens by that route. -## 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`. - ## 170. A row that stops matching the view only leaves it on the Delete path **Observed (user, from the notes):** "should we refactor the list UI to be @@ -1407,3 +1210,55 @@ every tag write passes. Move it, or call it from both send paths. thread the query never returned. - Undo goes back through the same funnel, so a removal must not make an undone mark-read invisible in the view it was undone in. + +## 173. The composer is a plain-text editor, not WYSIWYG + +**Observed (user, 2026-08-27):** asked for directly while hand-testing item +171. "This is a GUI mail client and should have a wysiwyg editor." + +**Where it came from.** Item 171 forwards an HTML message by carrying the +original's markup, and the markup cannot be shown in a `QPlainTextEdit`. The +first build seeded a text quote into the editable buffer and then dropped it +when building the HTML part, so the user could edit a quote whose edits were +silently discarded. The user's objection is the right one and is more general +than that bug: **what the composer shows should be what gets sent.** + +171 shipped the middle ground, a read-only preview beside the editor. This +item is the real answer. + +**Cause (verified in the code).** The composer is a `QPlainTextEdit` over +markdown, deliberately: `OutgoingMessage::markdownBody` is "the source text, +exactly as typed", `MarkdownRenderer` turns it into HTML at build time, and +`DraftStore` autosaves that same markdown. There is nowhere in that model for +a stranger's markup, or for the user's own rich text, to live and be edited. + +**Approach.** A rich-text editor for the body, which is what every graphical +mail client does. Thunderbird is the reference: forwarding HTML opens a +rich-text composer with the original inside it, editable. + +**This is not a widget swap, and the constraints are why it is L.** + +- **The draft format changes.** A draft currently round-trips as markdown; a + rich-text composer means storing HTML, and a draft written by one and read + by the other loses formatting silently. Items 163 and 165 are already about + draft identity and are worth settling first. +- **`QTextEdit`'s HTML subset is narrow.** It is not a browser: real + newsletter markup (tables, modern CSS) degrades in it. So a naive swap makes + the FIDELITY of a forward worse than what 171 currently sends, while making + the editing better. Measure before committing to it. +- **The formatting toolbar has two masters.** `MarkdownFormat` answers "what + does this button do to a selection" over markdown text. A rich-text editor + has its own notion, and the toolbar must drive whichever is active without + the two disagreeing. +- **Plain text must stay reachable.** Not every message should be HTML, and + the `send_html` config default plus the per-message toggle both already say + so. A rich-text composer that cannot produce clean plain text regresses the + common case. +- **The security work does not go away.** A forwarded original is still input + from a stranger. Whatever renders it for editing must not fetch remote + content, and `HtmlSanitiser` (item 171) is what already answers that. + +**It subsumes item 133**, markdown syntax highlighting: that is a +`QSyntaxHighlighter` over the plain editor, which is the cheap answer to the +same want ("show me what I am writing"). If this is built, 133 is moot; if +this is deferred, 133 is the thing to do instead. Do not build both. diff --git a/docs/superpowers/plans/2026-08-26-card-avatars.md b/docs/superpowers/plans/2026-08-26-card-avatars.md index 0fba38f..03ea78a 100644 --- a/docs/superpowers/plans/2026-08-26-card-avatars.md +++ b/docs/superpowers/plans/2026-08-26-card-avatars.md @@ -988,7 +988,41 @@ void TestBusinessSenders::onlyBulkLookingLocalPartsAreProposed() } ``` -Declare all three in `private slots:` and add `#include <QFileInfo>` to the test's includes. +```cpp +void TestBusinessSenders::theFirstRunScansEverything() +{ + QTemporaryDir dir; + const QString missing = dir.filePath(QStringLiteral("business-senders")); + + // No file at all: a week of mail would propose almost nothing and the + // list would take months to become useful, so the first run pays for a + // full scan once. + QCOMPARE(BusinessSenders::scanQuery(missing), QStringLiteral("*")); + + // A file holding ONLY rejected candidates is still a first run: nothing + // has been accepted yet. Rescanning re-proposes none of them, since + // appendCandidates skips anything already mentioned. + QFile rejected(missing); + QVERIFY(rejected.open(QIODevice::WriteOnly | QIODevice::Text)); + rejected.write("# noreply@cofidis.it (47 messages)\n"); + rejected.close(); + QCOMPARE(BusinessSenders::scanQuery(missing), QStringLiteral("*")); +} + +void TestBusinessSenders::alaterRunScansOnlyRecentMail() +{ + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("business-senders")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("billing@example.org\n"); + file.close(); + + QCOMPARE(BusinessSenders::scanQuery(path), QStringLiteral("date:1week..")); +} +``` + +Declare all five in `private slots:` and add `#include <QFileInfo>` to the test's includes. - [ ] **Step 2: Run test to verify it fails** @@ -1019,6 +1053,18 @@ bool looksLikeBulk(const QString &address); /// rejected is never re-proposed, and one they deleted only returns if that /// sender writes again. void appendCandidates(const QString &path, const QHash<QString, int> &counts); + +/// The query the candidate scan should run. +/// +/// A week of mail once the file exists, so the step stays incremental and +/// cheap. EVERYTHING when the file is missing or holds no entries, because +/// that is the first run: a week's mail proposes almost nothing, and the file +/// would then take months to become useful. The whole-database scan is +/// affordable precisely because it happens once, measured at 76 ms over 5105 +/// messages. +/// +/// Returns notmuch query syntax, which is wire format and is never translated. +QString scanQuery(const QString &path); ``` Add `#include <QHash>` to the header. @@ -1097,15 +1143,35 @@ void appendCandidates(const QString &path, const QHash<QString, int> &counts) } ``` +```cpp +QString scanQuery(const QString &path) +{ + // "*" is notmuch's match-everything. An EMPTY string would also match + // everything, which is why Config::matchNothingQuery() exists elsewhere in + // this codebase; being explicit here means a reader never has to wonder + // which of the two an empty return meant. + const List existing = load(path); + if (existing.addresses.isEmpty() && existing.domains.isEmpty()) + return QStringLiteral("*"); + return QStringLiteral("date:1week.."); +} +``` + Add `#include <QFileInfo>` to `src/businesssenders.cpp`. +Note what the emptiness test is deliberately NOT: it asks whether the file holds +any usable ENTRY, not whether the file exists or has bytes. A file holding only +rejected candidates, every line commented out, is still a first run as far as +this is concerned, and rescanning it costs 76 ms and re-proposes nothing, since +`appendCandidates` skips everything already mentioned. + - [ ] **Step 5: Run test to verify it passes** ```bash cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R businesssenders --output-on-failure ``` -Expected: PASS, 9 tests. +Expected: PASS, 11 tests. - [ ] **Step 6: Commit** @@ -1785,7 +1851,11 @@ In `MainWindow`, where a sync completes (search for where the unsynced count is }); ``` -Request the counts scoped to recently indexed mail rather than the whole database, so the step stays incremental: `countSenders(QStringLiteral("date:1week.."))`. +Scope the request with `BusinessSenders::scanQuery()`, added below: a week of mail once the file exists, and everything on the first run. + +```cpp + countSenders(BusinessSenders::scanQuery(BusinessSenders::defaultPath())); +``` - [ ] **Step 5: Run test to verify it passes** @@ -1853,6 +1923,10 @@ After each sync the application appends addresses that look like bulk mail, it. Anything already in the file, commented or not, is never proposed again: commenting a line out is therefore the permanent way to reject it, while deleting it lets that sender be proposed again if they write to you. + +The first scan, when the file does not exist or holds no active entry, covers +the whole database so the list is useful straight away. Afterwards it covers +the last week's mail. ``` - [ ] **Step 4: Update the changelog** diff --git a/docs/superpowers/specs/2026-08-26-card-avatars-design.md b/docs/superpowers/specs/2026-08-26-card-avatars-design.md index 14829c0..e396a34 100644 --- a/docs/superpowers/specs/2026-08-26-card-avatars-design.md +++ b/docs/superpowers/specs/2026-08-26-card-avatars-design.md @@ -217,6 +217,16 @@ of the newly arrived mail and appends CANDIDATES, commented out: # noreply@cofidis.it (47 messages) ``` +**How much mail the scan covers** depends on whether the list has ever been +used. With no file, or a file holding no active entry, it scans the WHOLE +database; afterwards it scans the last week. The first run is exactly when a +full scan earns its cost: a week of mail proposes almost nothing, so a +week-only rule would leave the list taking months to become useful. It is +affordable because it happens once, measured at 76 ms over 5105 messages. + +A file holding only rejected candidates still counts as unused. Rescanning it +re-proposes none of them, since anything already mentioned is skipped. + A candidate is an address whose local part is in a small built-in word list (`noreply`, `no-reply`, `donotreply`, `info`, `support`, `billing`, `newsletter`, `notifications`, `mailer-daemon`), or one that recurs with no diff --git a/docs/superpowers/specs/2026-08-27-forward-html-design.md b/docs/superpowers/specs/2026-08-27-forward-html-design.md new file mode 100644 index 0000000..a71cc54 --- /dev/null +++ b/docs/superpowers/specs/2026-08-27-forward-html-design.md @@ -0,0 +1,181 @@ +# Forwarding an HTML message with its formatting + +Item 171. Design, 2026-08-27. **Read this before the backlog row**, which +records only the cause. + +## The defect + +`ComposeContextBuilder::quoteBody()` reads `ParsedMessage::plainBody` and +nothing else, so the forward path drops the original's `htmlBody` entirely. +Two consequences, and they are not the same severity: + +- An original with both parts forwards its text/plain alternative. The words + survive, the sender's formatting does not. This is what the user reported. +- **An original with an HTML part ONLY has an empty `plainBody`**, so the + forward carries an attribution line and an empty quote. The content is gone + and nothing says so. + +Measured on the developer's own inbox 2026-08-27: **30 of 342 sampled +messages** (~9%) declare `text/html` with no `text/plain` part. The silent +half is not an edge case. + +## What was decided, and by whom + +The user chose, 2026-08-27, from three routes put to them: + +1. **Inline, carrying the original's markup** — CHOSEN. The forward carries + the original's own HTML rather than a flattened quote. Highest inline + fidelity, and the most work, because it is the only route that puts a + stranger's markup into an outgoing message. + + **Amended 2026-08-27, after the first build**: a forward sends ONE part, + not a `multipart/alternative`. The Send-as-HTML toggle chooses which — the + original's markup when on, the text quote when off. The user's reasoning is + that a forward's shape is something they have already decided by flipping + that toggle, and sending both halves hands the choice to the recipient's + client instead. An ordinary (non-forward) message still sends the + alternative as before; only the forward path is single-part. +2. Attach the original as `message/rfc822` (item 130's mechanism). Rejected + here, though item 130 may still build it for its own sake: the user wants + the content inline, not as an attachment. +3. Text fallback only. Rejected: it fixes the silent-loss half and does not + answer the note at all, since formatting is still lost on every forward. + +**Remote content is STRIPPED BY DEFAULT, with a per-forward opt-out**, also +the user's choice against "always strip" and "keep everything". A control in +the composer, checked by default, reading roughly "Strip remote content from +the forwarded message". + +## Why this is the security-critical item in the backlog + +Every other HTML path in this application renders a stranger's markup *to the +user*, behind protections that live in `MessageView`: an off-the-record +profile, JavaScript disabled, and `RequestInterceptor` blocking every request +by default and failing closed. + +**None of those protections apply here.** The markup leaves this process and +is rendered by somebody else's mail client, under their policy, on their +machine. The interceptor cannot help: it intercepts requests *we* would make. +So the sanitising has to happen to the bytes, before they are handed to +`MessageSender`, and there is no second line of defence behind it. + +The concrete harm, in the user's own words when the decision was put to them: +forwarding a tracking pixel forwards the tracking. The original sender learns +that the forwarded copy was opened, by whom, and how many times, and the +user's recipient never consented to that. + +## The allow-list rule, which is not negotiable + +`HtmlBuilder::namespaceCids()` is the closest prior art and it is a +**block-list**: it names the attributes that can carry a `cid:` and rewrites +those. It documents scoping `srcset=` out, on the reasoning that its quoting +grammar differs and `cid:` in `srcset` is not seen in the wild. + +**That trade is correct for rewriting and WRONG for stripping**, and the +asymmetry is the whole design: + +| | a missed reference means | +|---|---| +| `namespaceCids` (rewrite) | one broken image | +| this sanitiser (strip) | a tracking beacon reaching the recipient | + +So the sanitiser must **allow-list what may remain**, not block-list what must +go. Anything not recognised is removed. A new HTML attribute, a quoting form +not anticipated, a `srcset`, a CSS `image-set()`, an `@import` — each is +handled by the default, which is removal, rather than by having been +enumerated in advance. + +Stated as the invariant to test against: **after sanitising, no attribute +value and no CSS construct in the output may contain a URL whose scheme is +anything other than `cid:`, and no element that fetches may remain without +one.** + +## What is kept and what goes + +Kept: + +- `cid:` references. They travel inside the message, fetch nothing, and are + what makes an inline logo survive. Item 129 will need the same machinery. +- Structural and presentational markup: tables, lists, headings, spans, + `style=""` attributes with their remote constructs removed. + +Removed: + +- Any `src`, `href`, `background`, `poster`, `srcset`, `data-*` or other + attribute value carrying a non-`cid:` URL scheme. `http:`, `https:`, + `//host/path`, `data:` (which can carry markup), `file:` above all. +- `<link>`, `<script>`, `<iframe>`, `<object>`, `<embed>`, `<base>`, and + `<meta http-equiv="refresh">`. +- CSS `url()`, `@import` and `image-set()` naming anything but a `cid:`, + whether in a `style=""` attribute or a `<style>` block. +- Event-handler attributes (`onload`, `onerror`, …). The recipient's client + most likely disables scripting, but that is their policy and not ours to + assume. + +A stripped `<img>` leaves a gap where the image was. That is the correct +outcome and should not be papered over with a placeholder that itself fetches. + +## Shape of the change + +- **`HtmlSanitiser`, a new namespace of free functions over values** + (`src/htmlsanitiser.h/.cpp`), matching `MarkdownRenderer` and + `MessageBuilder`. No widget, so the security property is testable without a + painter or a web engine. This is where the allow-list lives. +- **`OutgoingMessage` gains the forwarded HTML and the strip flag.** It + currently carries `markdownBody` and `sendHtml` only, so there is nowhere to + put a second HTML source; both are new fields rather than a changed call + site. +- **`MessageBuilder::build()` chooses the part.** On a forward with the toggle + ON, the body is one `text/html` part: the user's rendered markdown, a rule, + then the sanitised original. With the toggle OFF it is one `text/plain` part + carrying the markdown and the text quote. No alternative either way. + + **The toggle is honoured even when the original had no plain-text part**, so + an HTML-only original forwarded with the toggle off goes out as the text + fallback and its formatting is lost. Chosen deliberately 2026-08-27 over + forcing HTML for those messages, so that the toggle means what it says; the + first build forced it and that was reversed. +- **`quoteBody()` is not the fix and must not become it.** It is shared with + Reply, and the behaviour asked for is the forward's alone. It keeps + producing the text quote for the plain half. Its one change is the + silent-loss case: when `plainBody` is empty it should render `htmlBody` down + to text rather than emitting an empty quote, so the plain half is never + blank. +- **The composer control** sits with the existing per-message toggles, checked + by default, and only appears on a Forward carrying HTML. + +## Traps recorded in advance + +- **`quoteBody()` is shared with Reply.** A change there reaches both paths. +- **`QString::arg()` does not collapse `%%`.** CLAUDE.md records the 0.11.0 + placeholder losing its mask, its glow and both gradients this way while + still painting a plausible pane. Any generated CSS here is subject to it. +- **A geometry or rendering probe cannot see this.** The property under test + is "no remote URL survives", which is a property of the STRING. Assert on + the generated bytes. CLAUDE.md's "Rendering probes lie" section is the + general warning; here a probe that renders the result and looks at it would + endorse a tracking pixel it cannot see, because a 1x1 transparent image is + invisible by design. +- **Test with real hostile shapes**, not tidy markup: unquoted attribute + values (`<img src=http://x/p>`), mixed quoting, whitespace and newlines + around `=`, uppercase tags and attributes, a `cid:` whose id contains a + URL-looking substring, and a `style` attribute carrying `url(...)` with no + quotes. `namespaceCids()` documents each of these forms as real. +- **`MimeParser` must parse what we build.** A round trip through + `MessageBuilder` and back is the check that the nesting is right, and it is + cheaper than reading the RFC. + +## Order of work + +1. `HtmlSanitiser` with the allow-list, tested against the hostile shapes + above. Nothing else depends on decisions inside it. +2. `quoteBody()`'s empty-plain fallback, which closes the silent-loss half on + its own and is independently useful. +3. `OutgoingMessage` fields and the `MessageBuilder` nesting, with a + parse-back round trip. +4. The composer control, defaulting to strip. +5. Hand test: forward a real HTML message to yourself with the box checked and + unchecked, and confirm what arrives. + +Steps 1 and 2 are separable and each ship a real improvement, so this does not +have to land as one commit. |