diff options
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 303 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 222 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-09-13-cli-selectors.md | 2044 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-09-13-spam-view.md | 498 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-09-13-cli-selectors-design.md | 253 |
5 files changed, 3105 insertions, 215 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 945aacd..84685c6 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 @@ -10375,3 +10375,306 @@ repository.** different thing in a different place. + + +## 187. There is no Spam view beside Trash + +**Observed.** The user asks for a Spam view next to Trash. Mail can be marked +spam today and there is no filter that lists it. + +**Cause.** `kQueryGenerators` (`config.cpp:62`) is a closed set of six: +`unread`, `inbox`, `flagged`, `sent`, `drafts`, `trash`. There is no `spam`. +The `spam` action has existed since the first toolbar and writes the tag +(`mainwindow.cpp:1770`, adds `spam`, removes `inbox`), so the write half is +built and the read half is missing. + +**Two wrong premises were corrected before any design, and both are worth +keeping.** This entry first said no account names a spam folder, so a tag +generator was the only option. Wrong: the accounts synced with `Patterns *` +had a spam folder all along. It then said the accounts with an explicit +`Patterns` list could never have one. Also wrong, and the cause was local +rather than remote: the provider exposes the folder over IMAP and mbsync was +simply never asked for it. Adding it to those three channels on 2026-08-29 +took one line each, verified against `mbsync --list` rather than guessed, +which matters because `Create Both` turns a wrong folder name into a folder +created on the server (item 103). + +**So every account can now reach a spam folder, and the design is Trash's.** +The user settled three things on 2026-08-29: + +- **Path-based, exactly like Trash.** Not a tag generator. A tag query finds + only what this application marked and misses everything the server filed, + which is most of what those folders hold. +- **Mark spam MOVES the file**, as Delete does. This is a change to an + existing action, not only a new view, and it is the part that makes the + path-based view honest. +- **`Junk` is out of scope.** One account has a `Junk` folder beside its + `Spam`; it is not used and the key names one folder. + +**Approach.** Follow item 103's implementation rather than inventing one. + +1. A mandatory per-account `spam` key beside `trash`, an `Account::spamQuery()` + beside `trashQuery()`, and `Config::allSpamQuery()` beside + `allTrashQuery()`. +2. `spam` added to `kQueryGenerators` and to `builtinFilter()`, threaded like + Trash rather than flat, composing with the account selector through the same + path in `resolvedQuery()`. +3. The `spam` action moves the file instead of only writing tags, through + `moveMessages()`, with an origin tag so it can come back. Restore already + reads `deleted-from:`; this needs the same for spam, or one shared origin + scheme. +4. A cleanup pass for mail tagged `spam` that never moved, which is every + message the action has ever touched. + +**The cleanup pass has a precedent and should copy it.** +`showStrandedDeletedMail()` (item 103) is the same problem one version earlier: +mail tagged `deleted` whose file never left its folder. It builds +`tag:deleted and not (<all trash folders>)`, puts it in the query bar, and +REPORTS, moving nothing, leaving the user to select and act. Do the same with +`tag:spam and not (<all spam folders>)`. Two details of it are load-bearing: +an empty folder list must never be written as `not ()`, which notmuch parses +happily and matches nothing, reporting a clean database; and it runs +`AlreadyScoped` so the account dropdown does not narrow it and hide other +accounts' stranded mail. + +**Constraints.** + +- **A mandatory key breaks every existing config on upgrade**, exactly as + `trash` did under item 103. That needs an `### Upgrading` note in the + changelog, and the same treatment `trash` got: name the missing key rather + than failing silently. +- **Naming a folder that does not exist reaches the server.** Item 103's + lesson, and the reason the three Gmail patterns were verified against + `mbsync --list` before being written. A default value is not safe here; the + key is named by the user or the account has no spam view. +- **`Config::matchNothingQuery()` for an account with no spam folder**, never + an empty string: notmuch reads an empty query as "match everything", so the + Spam button would show the whole Maildir. +- **The trash view's own predicate must not be confused by this.** + `everySelectedRowIsInATrashFolder()` decides which actions the message bar + and menus offer (items 185, 186). A spam folder is not a trash folder and + must not satisfy it, or Restore and the purges appear on spam. +- **Mark spam removing `inbox` stays.** The tag half is still what makes the + message leave the Inbox view; the move is in addition to it, not instead. +- **The label is translated, the generator is not.** `spam` is stored in + `queries.json` and matched against a closed set, so it is wire format; see + the `flagged`/"Important" note in `builtinFilter()`. +- **Adding a generator changes queries.json's readable set**, so an older build + reading a file that names `spam` reports an unknown generator and KEEPS the + row. Existing behaviour, no version bump. +## 190. Mark spam is not on the message bar, and its icon was never chosen for one + +**Observed (user, from the notes):** "add \"mark as spam\" to the message pane +toolbar. Use a bug as the icon (or a skull, or something that signifies +bad/evil)." + +**Cause, verified in the code.** Two independent halves, and neither is a +regression. + +The action exists and has since the first toolbar: `addAction("spam", tr("Mark +&spam"), ...)` at `mainwindow.cpp:1768` writes `spam` and removes `inbox` +through `tagSelected()`. It is reachable from the Message menu +(`mainwindow.cpp:2062`) and the thread context menu (`:2223`), and it carries a +shortcut, `Ctrl+Shift+S` (`keymap.cpp:151`). What it has never been on is the +message pane's own bar: `refreshBarActions()` fills the ordinary branch with +exactly `reply`, `forward`, `flag`, `archive`, `delete` (`mainwindow.cpp:2387`), +and item 189 added Star and Archive there without raising spam. + +It meets the bar's rule as it stands. The bar carries selection-scoped actions +with an undo behind them, which is why `mark_all_read` was kept off it under +item 189 and why Star and Archive were let on. `spam` is a `tagSelected()` call +like those two, so it qualifies on both counts today. + +The icon is the second half and is the same latent wrong choice item 189 found +in `flag`. `{ "spam", "mail-mark-junk" }` (`mainwindow.cpp:2144`) was chosen for +a MENU, where the label carries the meaning and the icon only decorates it. On +an icon-only bar the icon IS the control, which is what made Breeze's +exclamation-mark rendering of `mail-mark-important` a defect rather than a +preference. Whether `mail-mark-junk` reads as "bad/evil" on the user's theme is +a question only the user can answer by looking, and the note suggests it does +not. + +**Approach.** Add `spam` to the ordinary branch of `refreshBarActions()`. Order +is a decision, not a detail: the bar reads answer, then file, then destroy, and +spam is a filing act whose destination is hostile, so it belongs with Archive +rather than beside Delete or before Star. For the icon, offer the user the +theme names that exist rather than picking one unseen; a shipped SVG under +`assets/icons/marks/` is the fallback if no theme name reads right, but that is +the panes' convention and the bar is chrome (item 70), so it is a last resort +rather than a first move. + +**Constraints.** + +- **The trash branch must not gain it.** `everySelectedRowIsInATrashFolder()` + swaps the bar to Restore, Delete permanently and Empty trash (item 185); + marking already-trashed mail as spam is not an act the user asked for, and the + same question item 187 flags applies here from the other side. +- **The icon table forbids duplicates** for any action that can reach the + toolbar, by the test item 140 established. `mail-mark-junk` is unique today + and any replacement must stay so. +- **Item 187 changes what this action does**, from a tag write to a file move + with an origin tag. Doing 190 first puts a button on the bar whose behaviour + then changes underneath it; doing 187 first means the button arrives already + correct. Neither ordering is wrong and the user chooses, but they should not + be built in ignorance of each other. + +**Verification.** The bar's contents are a list in one function, so an assertion +on it is measurable and belongs in the same test item 189 corrected. The icon is +a visual judgement and belongs to the user, per the rule in `CLAUDE.md`: hand it +over and let them look. +## 195. Mark spam leaves the message unread + +**Observed (user, from the notes):** "marking a message as spam without reading +it doesn't remove the unread tag." + +**Cause.** Verified, not assumed. The `spam` action at `mainwindow.cpp:1786` +calls `tagSelected({ "spam" }, { "inbox" }, ...)`: it names exactly two tags, +so `unread` is untouched by construction. The message leaves the inbox and +keeps counting toward every unread view. + +**Approach.** Add `unread` to the removal list of that one call. It is a +two-word change and the surrounding machinery already covers it: the write goes +through `applyTags`, which reports only the ids whose tags actually moved (item +176), so a spam mark on an already-read message pushes no bogus undo, and +`syncViewMembership()` evicts it from Unread on the same funnel as any other +read. + +**Constraints.** Item 187 rewrites this action into a file move, so the cheapest +path is to fold this in there rather than shipping a separate commit that 187 +then rewrites. Doing it alone is still fine and costs nothing. + +**One question for the user.** Whether marking spam should mark READ, or whether +the right answer is that a spam message stops matching the unread views at all +once 187 makes those views path-based. The first is what the note literally +asks for; the second falls out of 187 for free and means an unread spam message +is still honestly unread if it is ever restored. They are not the same and the +choice is theirs. +## 197. No way to say a message is not spam + +**Observed.** Split out of the item 187 design on 2026-09-10, at the user's +decision, rather than built into it: "maybe we could already provision for a +future 'unmark spam' action so that we can revert a filter decision". + +**What already covers half of it.** Restore handles every message this +application moved. Mark spam writes `moved-from:<folder>` and Restore reads it +back, so unmarking is the existing gesture under a different name. + +**The real gap is the provider's filter, not ours.** Mail the provider caught +was never in an inbox, arrived directly in the spam folder, and carries no +origin tag. Restore falls back to the account's inbox for exactly this case, +which is a documented guess rather than a recorded destination. + +**Two questions decide the shape, and neither is answerable from the code.** + +1. Where does a message with no origin go? The account's inbox is the obvious + answer and is still a guess; a user who wants it filed somewhere else has no + way to say so. +2. Should anything tell the PROVIDER its filter was wrong, so it learns? That + is outbound network work, which this application does not do by design. It + would belong in a sidecar, like item 194's. + +**No seam is needed in the meantime.** `sendMove()` already takes any +destination and any tag lists, so a Not-spam action is a caller rather than a +capability. Provisioning for it now would be a hook with one hypothetical +caller, which is what YAGNI names. + +**Corrected 2026-09-14:** the interface half is item 201. Restore's visibility +is coupled to `everySelectedRowIsInATrashFolder()`, which by design never +answers for spam, so "Restore already covers what qtmaildir moved" was a +capability claim that no surface offered. Read item 201 for the narrower, +decision-free half. +## 201. A message in the Spam view cannot be un-spammed, even one qtmaildir put there + +**Observed (user, 2026-09-14, testing the `spam-view` branch).** "If a message +is in spam, how do I unmark it spam?" The only built-in answer found was Ctrl+Z +immediately after the move, before any other action; an edit of the `spam` tag +alone leaves the file in the spam folder. The user asked for this to be built on +`spam-view` before that branch merges. + +**Cause.** Verified in `src/`. Three facts together: + +1. `spam` is one-way, not a toggle. `MainWindow::spamSelected()` always resolves + the selection and calls `spamMessages()`/`spamThreads()`; unlike `delete` + (`mainwindow.cpp:1745`) and `flag` (`mainwindow.cpp:1832`) it never asks + `everySelectedRowHasTag("spam")`, so pressing Mark spam again moves the file + toward the folder it is already in rather than reverting it. +2. Restore is hidden outside the trash. `refreshTrashActions()` + (`mainwindow.cpp:3936`) sets `restore`'s visibility to + `(!haveSelection || inTrash) && !m_replySelectionHidesDelete`, where `inTrash` + is `everySelectedRowIsInATrashFolder()`, which compares `account.trash` only + BY DESIGN so the predicate never answers for spam (the spec requires that, so + Delete is not hidden and Purge not offered there). Restore is coupled to the + same predicate, which is what hid it in the Spam view. +3. The write side already works. `restoreSelectedFromTrash()` + (`mainwindow.cpp:6907`) resolves the origin from the DATABASE and calls + `sendMove()`, which takes any destination and any tag lists. Only the entry + point and the gating are missing. + +So item 197's "Restore already covers what qtmaildir moved" is true of the +function and false of the interface. Its stated gap was the PROVIDER-caught +message with no origin; this item is the narrower half, mail qtmaildir itself +moved and can put back, and it needs no new decision. + +**Approach.** One of two, and the difference is what the user should decide: + +- **Widen Restore into the Spam view**, decoupling Restore's visibility from + `everySelectedRowIsInATrashFolder()` so a message carrying a `moved-from:` + origin can be restored from spam too. Smallest change; makes one action serve + both folders. +- **Add a distinct `not_spam` action** shown on a spam-folder selection, doing + the existing restore and also stripping `spam`. Clearer on the bar and in the + menu, and costs the five registration places the rule names + (`KeyMap::knownActions()`, `defaultBindings()` optional, the icon table, the + action, a menu). + +Either reuses `sendMove()` and resolves the origin from the database, never the +model (the message-scoped `restoreSelected()` reads the model, the path item 176 +and the spam-view final review warn against). + +**Constraints.** A move's origin is resolved by the worker, never the model +(`CLAUDE.md`). An undo covers what the write CHANGED, not what it asked for +(item 176). No confirmation: this is a move and it is undoable. Item 197's open +questions, where PROVIDER-caught mail with no origin goes (the inbox guess) and +whether to tell the provider its filter was wrong, stay out of scope here. + +**Verification.** A `WorkerBackedWindow` test using `QTRY_VERIFY_WITH_TIMEOUT`: +mark a message spam, then un-spam it through the new path, asserting the file +returns to the folder it came from, `spam` and `moved-from:` are gone, and the +row leaves the Spam view. Because it is a move, assert the undo as well. +## 202. Mail in a spam folder keeps `inbox`, so it appears in the Inbox view + +**Observed (user, 2026-09-14, testing `spam-view`).** "Not spam is available in +the inbox view; it should appear only when viewing the spam view." The action is +not at fault: its predicate is folder-based (`everySelectedRowIsInAFolder()`, +the same rule Restore uses), so it correctly appears on mail whose FILE is in a +spam folder. Those rows should not have been in the Inbox view at all. + +**Cause.** Verified, and not in the application. notmuch's `new.tags` is +`new;unread;inbox`, so every newly indexed FILE gets `inbox` regardless of the +folder it sits in. The Inbox built-in filter is `tag:inbox` +(`Config::generatorTag("inbox")`), not path-scoped, so any file carrying `inbox` +appears there. The `post-new` hook already corrects this for mail that did not +ARRIVE: `NOT_ARRIVALS` in `assets/hooks/qtmaildirconf.py:84` is +`("sent", "drafts")`, and `strip_inbox_from_non_arrivals()` removes `inbox` from +a message whose files are ALL inside one of those folders. `spam` is missing +because the hook predates the per-account `spam` key this branch adds. Measured +live: 49 files tagged `inbox` sitting in a `[Gmail]/Spam` folder; 0 in trash, +drafts or sent. + +**Approach.** Add `spam` to `NOT_ARRIVALS` so the existing carve-out covers it +too, correct the function/doc prose (its names say "sent" while the list means +"not an arrival"), and extend the hook's tests with the spam cases. Then a +ONE-TIME cleanup of the existing messages, using the same all-files rule. + +**Constraints.** The all-files rule is load-bearing (`not_arrival_only()`): notmuch +deduplicates by Message-ID, so a message with one file in spam and one in an +inbox DID genuinely arrive and must keep `inbox`. Only `inbox` is removed; +`unread` is untouched because `maildir.synchronize_flags` is true. The cleanup +is a write to the live index and is confirmed with the user before running (it +was, 2026-09-14). Trash is deliberately out of scope: measured 0 such files, and +qtmaildir's own Delete strips `inbox` (item 168). + +**Verification.** `assets/hooks/test_post_new.py` (and the other hook suites) +green with a spam-folder case and a two-file (spam + inbox) case; after the +cleanup, `notmuch search --output=files 'tag:inbox' | grep -i '/spam/'` is +empty. 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 6792353..2a73673 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 @@ -260,20 +260,22 @@ taking that too literally. | 184 | New mail waits up to ten minutes, because sync is a fixed cron tick | workflow | ? | **done 2026-09-13**, outside this repo and confirmed running on this machine (PID watching, `~/bin/mail-watcher.sh --config ~/.config/mail-watcher/config.ini`). Built as `mail-watcher`, its own repository at `~/Programming/GIT/mail-watcher`, designed in its own `docs/superpowers/specs/2026-09-13-mail-watcher-design.md`. It took the shape this entry argued for and settled the three decisions it listed: a watcher of ours rather than a third-party daemon, so no new SlackBuild; one Python file, standard library only; one thread per watched folder with its own reconnect, one trigger loop owning every `mailsync.sh` invocation with a debounce, default-watch with an explicit exclude list. Both constraints held: the cron tick stays as a backstop, and `/tmp/mbsync.lock` is still the shared mutex. Nothing in `src/` changed, which is the point, the no-network-protocol rule is intact. Section in the closed file. Original entry: open, 2026-08-29, from the user: the 10 minute tick "has always bothered me", and it is already a compromise down from 30. Outgoing edits are immediate (`auto_sync_delay_ms`), so this is the INCOMING half only. Polling faster is not the answer; IMAP IDLE is, and it lives in a watcher that triggers `mailsync.sh`, NOT in qtmaildir, which does no network protocol work. Needs decisions first: which watcher, whether it packages on Slackware, and what the server supports. **Blocked on 174**, whose status file is the reporting channel this needs anyway | | 185 | The message-pane bar offers Reply and Forward on a trashed message | presentation | S | **done 2026-08-29**, unreleased, with 186. The bar has a third branch keyed on the SELECTION being in a trash folder, the same predicate the menus use: Restore, Delete permanently and Empty trash replace the reply pair, and Restore alone is tinted. Added `purge`, the selection-scoped sibling of `empty_trash`, which inherits both its safeguards. Refilled from the digest as well as from the selection, since a conversation's trash-ness is not known until every path is reported. Section in the closed file. Original entry: `MainWindow::refreshMessageBarActions()` (`mainwindow.cpp:2311`) swaps the bar's message half for a DRAFT and for nothing else, so the trash view shows the two actions that make least sense there. The notes ask for Restore and Delete permanently in their place, and for Delete to move here from the main toolbar (item 186). The visibility rules already exist in `refreshTrashActions()`; what is missing is the bar consulting them | | 186 | Delete sits on the main toolbar rather than beside Reply and Forward | presentation | XS | **done 2026-08-29**, unreleased, with 185. Moved to the message bar's ordinary branch; still in the Message and context menus. Section in the closed file. Original entry: `toolBar->addAction(... "delete")` at `mainwindow.cpp:2251`. The user places it with the message actions, so this rides with item 185 rather than being done alone: moving it before the bar is trash-aware leaves Delete in a bar that still offers Reply on trashed mail | -| 187 | There is no Spam view beside Trash | workflow | M | open, **specified 2026-09-10** in `specs/2026-09-10-spam-view-design.md`, which covers 190 and 195 too; read that rather than this row. Grew again: the user added Empty Spam (a MOVE to the trash, per account) and the `deleted-from:` -> `moved-from:` rename. 2026-08-29, from the notes; **shape settled 2026-08-29** after two corrections and three decisions from the user. Spam works like Trash: path-based, a mandatory per-account `spam` key, and Mark spam MOVES the file. Every account can now reach a spam folder, the three Gmail ones having gained `[Gmail]/Spam` in `.mbsyncrc` this session. Grew from S to M: the move path, the origin tag and a cleanup pass are three parts, and it changes what an existing action does. See the entry | +| 187 | There is no Spam view beside Trash | workflow | M | **done 2026-09-14**, released in 0.29.0, on `spam-view`. Spec: `specs/2026-09-10-spam-view-design.md`. A per-account `spam` key with `spamQuery()`/`allSpamQuery()`, a threaded path-based Spam filter, and Mark spam now MOVES the file (origin tag rewritten to `moved-from:`) with Undo and Restore. Also shipped Empty Spam (per account, into the trash, no confirmation) and Find stranded spam. Covers 190 and 195 too. Section in the closed file | | 188 | Does Empty trash respect the account selector? | question | XS | **answered 2026-08-29** by reading the code, no work needed. It does: `MainWindow::emptyTrash()` (`mainwindow.cpp:6567`) reads `m_accountBox->currentData()` and uses `allTrashQuery()` only for All accounts, and the confirmation names which. Recorded so the notes' question has an answer rather than sitting open | | 189 | The message bar carries only Reply, Forward and Delete | presentation | S | **done 2026-08-29**, unreleased. Star and Archive joined the bar's ordinary branch, Archive leaving the main toolbar as Delete did. `mark_all_read` deliberately did NOT move, at the user's decision: it is the one action that ignores the selection. Item 140's toolbar test listed `archive` as a list-wide action and had to be corrected, which is the classification this item changed. Section in the closed file. Original entry: Asks for Star (`flag`) and Archive on the bar, and raises Mark all read as a question. Two of the three are selection-scoped and fit the bar's rule as it stands; **`mark_all_read` does not**, since it deliberately ignores the selection and acts on every row in the view, which is the one action in the window that does. Needs a decision from the user on that one and on whether Archive LEAVES the main toolbar the way Delete did | -| 190 | Mark spam is not on the message bar, and its icon was never chosen for one | presentation | XS | open, 2026-09-06, from the notes. The bar's ordinary branch carries Reply, Forward, Star, Archive, Delete after item 189 and `spam` is not among them, though it meets the bar's rule (selection-scoped, undoable). Two halves: put it on the bar, and settle the icon, which the note asks to be "a bug, or a skull, or something that signifies bad/evil" and which is `mail-mark-junk` today, chosen for a menu where the label carries the meaning. **Paired with 187**, which changes what the action DOES (moves the file); ordering is the user's call | +| 190 | Mark spam is not on the message bar, and its icon was never chosen for one | presentation | XS | **done 2026-09-14**, released in 0.29.0, with 187. `spam` joined the bar's ordinary branch between Archive and Delete; the icon is `bug` with a `mail-mark-junk` fallback (the icon table gained primary+fallback support). Hidden on a reply row and in the trash. Section in the closed file | | 191 | The Sent view collapses two messages you sent in one conversation into one row | defect | S | **done 2026-09-06**, unreleased, from a hand test. The Sent and Drafts views are flat, but the worker emitted one summary per THREAD and picked a single matched message to stand for it, oldest-first. A conversation replied to twice showed one row, dated by the thread and opening the OLDER message, and the newer one was reachable nowhere. Also a data-safety defect: `firstMessagePath` named the wrong file, so Delete would have moved it. A second half, found by hand once the rows appeared: the sort notmuch applies is a THREAD sort, so both rows took their thread's position and an older reply drew above a newer one. Flat rows are now sorted as one list. Section in the closed file | | 192 | A sent message does not appear in the Sent view until the next sync | defect | XS | **done 2026-09-06**, unreleased. The sent copy was filed correctly and never announced, so the index did not know it and the Sent view, a path query, could not show it. Measured as 65 files against 64 indexed. One signal to the worker, mirroring what drafts have had since item 158. The open question, whether the view should also refresh, was answered yes by the user on 2026-09-07 and built: `indexChanged()` to `refreshCurrentQuery()`. Section in the closed file | | 193 | The composer has no headings control | v2 | S | open, 2026-09-08, from the notes: "headers dropdown in the editor, H1 to H6 translating to #, ## ... already supported by the html render". The note is right about the renderer: cmark-gfm parses ATX headings in the core grammar, so `## x` already renders. The gap is composer-side. A heading is a LINE PREFIX, not a wrap, so it cannot go through `applyFormat()`/`MarkdownFormat::wrap()`; it is `quote()`'s shape, and unlike quote it must REPLACE an existing prefix rather than stack one, or a second press gives `## ## x`. That makes it the first formatting control that has to read the line's current state, which is item 135's question arriving early on one control | | 194 | No abuse reporting from a flagged message | workflow | L, split | open, 2026-09-08, from the notes and **confirmed by the user the same day as a feature they want and will build**. Parse a flagged `.eml`, extract IOCs, resolve abuse contacts via RDAP, generate X-ARF (RFC 5965), fan out to AbuseIPDB/URLhaus/VirusTotal and to abuse desks, backed by MISP via PyMISP. **One gesture here, the engine in a sidecar**: the split is architectural (four outbound protocols, which `src/` does not do) and not a judgement on the feature. qtmaildir's half is a message-bar button that marks spam and offers to report, with a confirmation; it is S and buildable before the sidecar exists. The user is a security consultant filling a phishing database, so the sidecar is the point rather than an accessory. Needs a spec for the sidecar; the qtmaildir half needs only 187/190 settled. Two of the user's constraints are safety properties: redact recipient identifiers before submission, and never fetch remote content during parsing | -| 195 | Mark spam leaves the message unread | defect | XS | open, 2026-09-10, from the notes ("marking a message as spam without reading it doesn't remove the unread tag"). Verified: the action at `mainwindow.cpp:1788` adds `spam` and removes `inbox`, and names no other tag, so an unmarked message keeps `unread` and every unread count keeps counting it. Small on its own; it touches the same action item 187 rewrites into a move, so doing it inside 187 costs nothing and doing it alone is a two-word change to one `tagSelected()` call. One question for the user: whether marking spam should mark read, or whether the tag should simply not be part of the unread views once 187 makes the view path-based | +| 195 | Mark spam leaves the message unread | defect | XS | **done 2026-09-14**, released in 0.29.0, folded into 187's move as the note asked: Mark spam now strips `unread` (and `inbox`) in the same move, so it leaves the Unread views. Section in the closed file | | 196 | Spam is never tagged automatically | workflow | ? | open, 2026-09-10, from the notes ("the app should be able to tag spam automatically leveraging intel from abusectl"). Depends on 194's sidecar existing: `~/Programming/GIT/abusectl` is a repo but nothing is on `PATH`, so the intel this would read does not yet have a shape to read. Also unspecified in direction: the natural home is the `post-new` hook rather than `src/`, since tagging at sync time is what `assets/hooks/mailrules.py` already does, and a rule sourced from an external database is a format question for both readers (see "Changing the rule format"). Ask the user what abusectl would expose before designing anything | | 198 | The unsynced-changes list never says which account a message belongs to | presentation | S | open, 2026-09-13, from the notes ("when clicking on the bottom right status bar, there's no way to discriminate what message belongs to what account"). The click opens `PendingChangesDialog` (item 119). Verified: `PendingChangeRow` (`pendingchangesdialog.h:32-50`) carries subject, action, `startsMessage` and `messageCount` and no account, so a list of five subjects across five accounts reads as one undifferentiated run. The data is reachable rather than missing: `accountForMessagePath()` (`mainwindow.cpp:6219`) resolves an account from a path, and `resolvePendingSubjects()` already walks every id in the worker and answers positionally, so the account is one more field on an existing round trip. One asymmetry to settle first: a held THREAD edit carries a thread id rather than a message id (`pendingChangeSnapshot()`, `mainwindow.cpp:5699`), and a thread can in principle span accounts, so the thread rows need a rule of their own rather than the message answer | | 199 | The window chrome uses the system icon theme, and the user wants a shipped set | presentation | M-L | open, 2026-09-13, from the notes ("we should ship our own icons, color themeable to be consistent in every theme a user may implement, since icons are a brand identity"). This deliberately REVERSES item 70, which drew the split as "panes are ours, chrome is the system's" and shipped `Marks` for the panes only; the note asks for the other half too, so it is a decision to revisit rather than a defect. Verified: the `themeIcons` table at `mainwindow.cpp:2211` and six `QIcon::fromTheme` sites in `composewindow.cpp` are every chrome icon, all resolved from the desktop theme. The mechanism already exists and is proven, `Marks::pixmap` compositing `fill="currentColor"` with `CompositionMode_SourceIn` so one asset serves a light and a dark palette, and `src/marks.h` records why it is compiled-in string literals rather than a `.qrc`. The size is the ARTWORK, not the code: item 70's six marks are shipped, this is roughly forty actions, each needing a drawing. Needs a decision from the user on scope before it can be sized honestly, and on whether the system theme stays as a fallback for an action with no shipped icon | | 200 | qtmaildir cannot be launched at a given account, thread or message | workflow | M | open, **specified 2026-09-13** in `specs/2026-09-13-cli-selectors-design.md`; read that rather than this row. The user settled three things: a second launch STEERS the running window over a `QLocalServer` rather than opening a second one, the selectors are `--account`/`--thread`/`--message` (`--query` dropped as the one with no caller), and a selector matching nothing opens the window normally and says so in the status bar. The design shrank on one side and grew on the other: `recoverStaleThread()` already runs `thread:<id>` with a deferred selection and is reused as a third caller, so the selectors are the small half, while the socket (connect-first ordering, stale-socket recovery, a degrade path when no socket is possible) is the real work and adds `Qt6::Network` to the component list. Original entry: open, 2026-09-13, from the notes ("the program should accept cli parameters like `--account` or `--thread`/`--message`, so that another app can launch qtmaildir opening that account's inbox or a certain message/thread"). Verified: `main.cpp:38-66` hand-rolls a `strcmp` loop over `argv` for `--version` and `--help` only, both answering before `QApplication` exists, which is deliberate and documented. Parsing is the small half and `QCommandLineParser` covers it; the item is bigger than it looks for two reasons. There is NO single-instance mechanism (no `QLocalServer` anywhere in `src/`), so a second launch opens a second window against the same notmuch database rather than steering the running one, and notmuch permits only one open handle per process. And the selector has to reach a query the startup path does not currently take, since `--thread` names a row that may not be in the configured startup view at all. Needs a decision from the user first: whether a second launch should focus the running window (which is the useful behaviour for "another app launches qtmaildir" and is the whole cost of the item) or simply start with a different query | -| 197 | No way to say a message is not spam | workflow | S | open, 2026-09-10, split out of the 187 design at the user's decision rather than built into it. Restore already covers what qtmaildir moved: a message it marked carries `moved-from:` and goes back where it came from. The gap is mail the PROVIDER's filter caught, which was never in an inbox and carries no origin tag, so "not spam" has no recorded destination to return it to. Needs two answers before it can be planned: where such a message goes (the account's inbox is the obvious guess and is a guess), and whether anything should tell the provider its filter was wrong, which is network work this application does not do and would belong in a sidecar like item 194's. No seam is needed in the meantime: `sendMove()` already takes any destination and any tags | +| 197 | No way to say a message is not spam | workflow | S | **done 2026-09-14**, released in 0.29.0. A `Not spam` action now exists (item 201): it returns each message to its `moved-from:` origin, strips `spam`, and for provider-caught mail with no origin falls back to the account's inbox, reported in the status bar (the destination question answered as the inbox guess). The provider-notification half stays out of scope as network work. Section in the closed file | +| 201 | A message in the Spam view cannot be un-spammed, even one qtmaildir put there | defect | S | **done 2026-09-14**, released in 0.29.0. Built as the distinct `not_spam` action (the second option): shown on a spam-folder selection, hidden on a reply row and in the trash, worker-resolved origin, undoable. Section in the closed file | +| 202 | Mail in a spam folder keeps `inbox`, so it appears in the Inbox view | defect | XS | **done 2026-09-14**, released in 0.29.0. Root cause was the `post-new` hook's non-arrival set, not the UI: `spam` added to `NOT_ARRIVALS`, helpers renamed `not_arrival_*`, tests added. Live one-time cleanup stripped `inbox` from the 49 affected messages (`tag:inbox` 5904 -> 5855). Section in the closed file | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -427,7 +429,6 @@ diff; the user decides what goes. **Size: still `?` until the list exists.** The sweep that produces the list is S to M; what it finds is the work. - ## 72. No khard/khal integration **Observed (user, from the notes):** "investigate khard/khal integration (light @@ -874,70 +875,6 @@ composer window and do not touch `KeyMap`, so nothing here interacts with item `quotingAnAlreadyQuotedLineNestsIt` in `tests/test_formattoolbar.cpp` both assert the current spec behaviour and would be replaced rather than extended. - ---- - -## 190. Mark spam is not on the message bar, and its icon was never chosen for one - -**Observed (user, from the notes):** "add \"mark as spam\" to the message pane -toolbar. Use a bug as the icon (or a skull, or something that signifies -bad/evil)." - -**Cause, verified in the code.** Two independent halves, and neither is a -regression. - -The action exists and has since the first toolbar: `addAction("spam", tr("Mark -&spam"), ...)` at `mainwindow.cpp:1768` writes `spam` and removes `inbox` -through `tagSelected()`. It is reachable from the Message menu -(`mainwindow.cpp:2062`) and the thread context menu (`:2223`), and it carries a -shortcut, `Ctrl+Shift+S` (`keymap.cpp:151`). What it has never been on is the -message pane's own bar: `refreshBarActions()` fills the ordinary branch with -exactly `reply`, `forward`, `flag`, `archive`, `delete` (`mainwindow.cpp:2387`), -and item 189 added Star and Archive there without raising spam. - -It meets the bar's rule as it stands. The bar carries selection-scoped actions -with an undo behind them, which is why `mark_all_read` was kept off it under -item 189 and why Star and Archive were let on. `spam` is a `tagSelected()` call -like those two, so it qualifies on both counts today. - -The icon is the second half and is the same latent wrong choice item 189 found -in `flag`. `{ "spam", "mail-mark-junk" }` (`mainwindow.cpp:2144`) was chosen for -a MENU, where the label carries the meaning and the icon only decorates it. On -an icon-only bar the icon IS the control, which is what made Breeze's -exclamation-mark rendering of `mail-mark-important` a defect rather than a -preference. Whether `mail-mark-junk` reads as "bad/evil" on the user's theme is -a question only the user can answer by looking, and the note suggests it does -not. - -**Approach.** Add `spam` to the ordinary branch of `refreshBarActions()`. Order -is a decision, not a detail: the bar reads answer, then file, then destroy, and -spam is a filing act whose destination is hostile, so it belongs with Archive -rather than beside Delete or before Star. For the icon, offer the user the -theme names that exist rather than picking one unseen; a shipped SVG under -`assets/icons/marks/` is the fallback if no theme name reads right, but that is -the panes' convention and the bar is chrome (item 70), so it is a last resort -rather than a first move. - -**Constraints.** - -- **The trash branch must not gain it.** `everySelectedRowIsInATrashFolder()` - swaps the bar to Restore, Delete permanently and Empty trash (item 185); - marking already-trashed mail as spam is not an act the user asked for, and the - same question item 187 flags applies here from the other side. -- **The icon table forbids duplicates** for any action that can reach the - toolbar, by the test item 140 established. `mail-mark-junk` is unique today - and any replacement must stay so. -- **Item 187 changes what this action does**, from a tag write to a file move - with an origin tag. Doing 190 first puts a button on the bar whose behaviour - then changes underneath it; doing 187 first means the button arrives already - correct. Neither ordering is wrong and the user chooses, but they should not - be built in ignorance of each other. - -**Verification.** The bar's contents are a list in one function, so an assertion -on it is measurable and belongs in the same test item 189 corrected. The icon is -a visual judgement and belongs to the user, per the rule in `CLAUDE.md`: hand it -over and let them look. - --- ## Deferred, unsized, or split out @@ -1023,7 +960,6 @@ what needs covering, so the two accounts must have different addresses and the message must be addressed to one of them, or either answer is correct and the test proves nothing. - ## 136. `undoMovesTheMessageBack` fails about one run in six **Observed.** `test_mainwindow` failed during a full-suite run while item 123 @@ -1320,93 +1256,6 @@ same discipline applies here. - The suite baseline is currently ONE known failure. Anything that makes it two intermittently costs the property that a red suite means something. - -## 187. There is no Spam view beside Trash - -**Observed.** The user asks for a Spam view next to Trash. Mail can be marked -spam today and there is no filter that lists it. - -**Cause.** `kQueryGenerators` (`config.cpp:62`) is a closed set of six: -`unread`, `inbox`, `flagged`, `sent`, `drafts`, `trash`. There is no `spam`. -The `spam` action has existed since the first toolbar and writes the tag -(`mainwindow.cpp:1770`, adds `spam`, removes `inbox`), so the write half is -built and the read half is missing. - -**Two wrong premises were corrected before any design, and both are worth -keeping.** This entry first said no account names a spam folder, so a tag -generator was the only option. Wrong: the accounts synced with `Patterns *` -had a spam folder all along. It then said the accounts with an explicit -`Patterns` list could never have one. Also wrong, and the cause was local -rather than remote: the provider exposes the folder over IMAP and mbsync was -simply never asked for it. Adding it to those three channels on 2026-08-29 -took one line each, verified against `mbsync --list` rather than guessed, -which matters because `Create Both` turns a wrong folder name into a folder -created on the server (item 103). - -**So every account can now reach a spam folder, and the design is Trash's.** -The user settled three things on 2026-08-29: - -- **Path-based, exactly like Trash.** Not a tag generator. A tag query finds - only what this application marked and misses everything the server filed, - which is most of what those folders hold. -- **Mark spam MOVES the file**, as Delete does. This is a change to an - existing action, not only a new view, and it is the part that makes the - path-based view honest. -- **`Junk` is out of scope.** One account has a `Junk` folder beside its - `Spam`; it is not used and the key names one folder. - -**Approach.** Follow item 103's implementation rather than inventing one. - -1. A mandatory per-account `spam` key beside `trash`, an `Account::spamQuery()` - beside `trashQuery()`, and `Config::allSpamQuery()` beside - `allTrashQuery()`. -2. `spam` added to `kQueryGenerators` and to `builtinFilter()`, threaded like - Trash rather than flat, composing with the account selector through the same - path in `resolvedQuery()`. -3. The `spam` action moves the file instead of only writing tags, through - `moveMessages()`, with an origin tag so it can come back. Restore already - reads `deleted-from:`; this needs the same for spam, or one shared origin - scheme. -4. A cleanup pass for mail tagged `spam` that never moved, which is every - message the action has ever touched. - -**The cleanup pass has a precedent and should copy it.** -`showStrandedDeletedMail()` (item 103) is the same problem one version earlier: -mail tagged `deleted` whose file never left its folder. It builds -`tag:deleted and not (<all trash folders>)`, puts it in the query bar, and -REPORTS, moving nothing, leaving the user to select and act. Do the same with -`tag:spam and not (<all spam folders>)`. Two details of it are load-bearing: -an empty folder list must never be written as `not ()`, which notmuch parses -happily and matches nothing, reporting a clean database; and it runs -`AlreadyScoped` so the account dropdown does not narrow it and hide other -accounts' stranded mail. - -**Constraints.** - -- **A mandatory key breaks every existing config on upgrade**, exactly as - `trash` did under item 103. That needs an `### Upgrading` note in the - changelog, and the same treatment `trash` got: name the missing key rather - than failing silently. -- **Naming a folder that does not exist reaches the server.** Item 103's - lesson, and the reason the three Gmail patterns were verified against - `mbsync --list` before being written. A default value is not safe here; the - key is named by the user or the account has no spam view. -- **`Config::matchNothingQuery()` for an account with no spam folder**, never - an empty string: notmuch reads an empty query as "match everything", so the - Spam button would show the whole Maildir. -- **The trash view's own predicate must not be confused by this.** - `everySelectedRowIsInATrashFolder()` decides which actions the message bar - and menus offer (items 185, 186). A spam folder is not a trash folder and - must not satisfy it, or Restore and the purges appear on spam. -- **Mark spam removing `inbox` stays.** The tag half is still what makes the - message leave the Inbox view; the move is in addition to it, not instead. -- **The label is translated, the generator is not.** `spam` is stored in - `queries.json` and matched against a closed set, so it is wire format; see - the `flagged`/"Important" note in `builtinFilter()`. -- **Adding a generator changes queries.json's readable set**, so an older build - reading a file that names `spam` reports an unknown generator and KEEPS the - row. Existing behaviour, no version bump. - ## 188. Does Empty trash respect the account selector? **Answered on 2026-08-29 by reading the code; no work follows from it.** It @@ -1542,35 +1391,6 @@ with the exit status and stdout as the report); and what qtmaildir shows when a report succeeds, partially succeeds, or fails, since a fan-out to four destinations can do all three at once. - -## 195. Mark spam leaves the message unread - -**Observed (user, from the notes):** "marking a message as spam without reading -it doesn't remove the unread tag." - -**Cause.** Verified, not assumed. The `spam` action at `mainwindow.cpp:1786` -calls `tagSelected({ "spam" }, { "inbox" }, ...)`: it names exactly two tags, -so `unread` is untouched by construction. The message leaves the inbox and -keeps counting toward every unread view. - -**Approach.** Add `unread` to the removal list of that one call. It is a -two-word change and the surrounding machinery already covers it: the write goes -through `applyTags`, which reports only the ids whose tags actually moved (item -176), so a spam mark on an already-read message pushes no bogus undo, and -`syncViewMembership()` evicts it from Unread on the same funnel as any other -read. - -**Constraints.** Item 187 rewrites this action into a file move, so the cheapest -path is to fold this in there rather than shipping a separate commit that 187 -then rewrites. Doing it alone is still fine and costs nothing. - -**One question for the user.** Whether marking spam should mark READ, or whether -the right answer is that a spam message stops matching the unread views at all -once 187 makes those views path-based. The first is what the note literally -asks for; the second falls out of 187 for free and means an unread spam message -is still honestly unread if it is ever restored. They are not the same and the -choice is theirs. - ## 196. Spam is never tagged automatically **Observed (user, from the notes):** "the app should be able to tag spam @@ -1603,35 +1423,6 @@ that currently runs offline against the local index. database queried per message, a periodically refreshed blocklist file, and a callable command are three different items sharing one sentence in the notes. -## 197. No way to say a message is not spam - -**Observed.** Split out of the item 187 design on 2026-09-10, at the user's -decision, rather than built into it: "maybe we could already provision for a -future 'unmark spam' action so that we can revert a filter decision". - -**What already covers half of it.** Restore handles every message this -application moved. Mark spam writes `moved-from:<folder>` and Restore reads it -back, so unmarking is the existing gesture under a different name. - -**The real gap is the provider's filter, not ours.** Mail the provider caught -was never in an inbox, arrived directly in the spam folder, and carries no -origin tag. Restore falls back to the account's inbox for exactly this case, -which is a documented guess rather than a recorded destination. - -**Two questions decide the shape, and neither is answerable from the code.** - -1. Where does a message with no origin go? The account's inbox is the obvious - answer and is still a guess; a user who wants it filed somewhere else has no - way to say so. -2. Should anything tell the PROVIDER its filter was wrong, so it learns? That - is outbound network work, which this application does not do by design. It - would belong in a sidecar, like item 194's. - -**No seam is needed in the meantime.** `sendMove()` already takes any -destination and any tag lists, so a Not-spam action is a caller rather than a -capability. Provisioning for it now would be a hook with one hypothetical -caller, which is what YAGNI names. - ## 198. The unsynced-changes list never says which account a message belongs to **Observed (user, from the notes):** "when clicking on the bottom right status @@ -1766,3 +1557,4 @@ in the notes. `--thread` value reaches a notmuch query, so it goes through `SearchTerm`'s quoting like every other query this application builds, rather than being concatenated at the call site. + diff --git a/docs/superpowers/plans/2026-09-13-cli-selectors.md b/docs/superpowers/plans/2026-09-13-cli-selectors.md new file mode 100644 index 0000000..6e01cc4 --- /dev/null +++ b/docs/superpowers/plans/2026-09-13-cli-selectors.md @@ -0,0 +1,2044 @@ +# CLI Selectors and Single-Instance Launch Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let another program launch `qtmaildir --account KEY --thread ID --message ID` and have the already-running window apply those selectors and raise itself, instead of a second process opening a second window. + +**Architecture:** `QCommandLineParser` replaces the hand-rolled `strcmp` loop in `main.cpp`. A `QLocalServer` under the state directory makes the first process the server; a later launch connects, sends its selectors as one payload, and exits. Both the local startup path and the socket handler call one `MainWindow::applySelectors()`, which reuses the existing `recoverStaleThread()` for the thread and message cases. Resolving a Message-ID to a thread id is one new worker round trip. + +**Tech Stack:** Qt 6.11 (Widgets, **Network** is new), libnotmuch 5, CMake 3.21+/Ninja, QtTest. + +**Spec:** `docs/superpowers/specs/2026-09-13-cli-selectors-design.md`. Backlog item 200. + +--- + +## Required reading before Task 1 + +Read these before writing any code. Each records a trap this plan walks past. + +- `AGENTS.md`, the whole file. In particular **"Adding an action is FIVE places"** (this plan adds none, and the reason is stated in the spec), the `tr()` rules, and the test-writing rules under "Rendering probes lie". +- `docs/superpowers/specs/2026-09-13-cli-selectors-design.md`, the spec this implements. +- `src/searchterm.h:30-35` on why a malformed notmuch query cannot be detected by asking notmuch. + +Three facts that will otherwise cost a session each: + +1. **Never run a test binary without `QT_QPA_PLATFORM=offscreen`**, and never launch `./build/src/qtmaildir` yourself. `tests/CMakeLists.txt` sets that variable for ctest only. One direct run of `test_mainwindow` throws a hundred windows onto the user's screen. +2. **`recoverStaleThread()` is a PRIVATE SLOT** (`src/mainwindow.h:605`). Tests reach private slots by name through `QMetaObject::invokeMethod`, which is the established pattern in `tests/test_mainwindow.cpp` (see line 2060). +3. **`QTRY_VERIFY_WITH_TIMEOUT`, never `qWait(n)`.** A fixed sleep passes when the result never arrives. + +## File Structure + +**Created:** + +- `src/singleinstance.h` / `src/singleinstance.cpp` — a `SingleInstance` QObject owning the `QLocalServer`. One responsibility: decide whether this process is the first, hand a later process's payload to whoever is listening, and emit what arrived. Knows nothing about queries or mail. +- `src/launchselectors.h` / `src/launchselectors.cpp` — a `LaunchSelectors` struct and the parse/serialise functions over it. Pure over values, no Qt GUI, no widget: this is what makes the parse and the payload testable without a window. +- `tests/test_launchselectors.cpp` — the parse and the payload round trip. +- `tests/test_singleinstance.cpp` — server/client behaviour against a `QTemporaryDir`. + +**Modified:** + +- `CMakeLists.txt:20` — add `Network` to `QTMAILDIR_QT_COMPONENTS`. +- `src/CMakeLists.txt:58` — link `Qt6::Network`, add the two new `.cpp` files to `qtmaildir_lib`. +- `src/main.cpp:38-66` — replace the `strcmp` loop; add the connect-or-listen step. +- `src/mainwindow.h` / `src/mainwindow.cpp` — `applySelectors()`, a state-path helper, and the Message-ID round trip. +- `src/notmuchworker.h` / `src/notmuchworker.cpp` — `resolveThreadForMessage()` slot and `threadForMessageResolved()` signal. +- `tests/CMakeLists.txt` — register the two new tests. +- `tests/test_mainwindow.cpp` — the applied-selector cases. +- `tests/test_notmuchworker.cpp` — the Message-ID lookup cases. +- `README.md` — a Usage section. +- `CHANGELOG.md` — an `[Unreleased]` entry. + +**Why two new units rather than code in `main.cpp`:** `main.cpp` is not in `qtmaildir_lib` (only the executable compiles it, see `src/CMakeLists.txt:67`), so anything written there cannot be tested at all. The parse and the socket both need tests, so both live in the library. + +--- + +## Task 1: The Qt6::Network component + +**Files:** +- Modify: `CMakeLists.txt:20` +- Modify: `src/CMakeLists.txt:58-61` + +- [ ] **Step 1: Add Network to the component list** + +In `CMakeLists.txt`, change line 20 from: + +```cmake +set(QTMAILDIR_QT_COMPONENTS Widgets Svg WebEngineWidgets) +``` + +to: + +```cmake +# Network is for QLocalServer/QLocalSocket only, which is a unix domain socket +# between two copies of this program (item 200). It is NOT network protocol +# work: the rule in AGENTS.md is about IMAP and SMTP, and nothing here speaks +# either. Slackware ships it inside the monolithic qt6 package, so this adds no +# new build dependency. +set(QTMAILDIR_QT_COMPONENTS Widgets Svg WebEngineWidgets Network) +``` + +- [ ] **Step 2: Link it** + +In `src/CMakeLists.txt`, change the `target_link_libraries(qtmaildir_lib ...)` call at line 58 so the `PUBLIC` list reads: + +```cmake +target_link_libraries(qtmaildir_lib + PUBLIC Qt6::Widgets Qt6::Svg Qt6::WebEngineWidgets Qt6::Network + PkgConfig::GMIME + ${NOTMUCH_LIBRARY} PkgConfig::CMARK_GFM + ${CMARK_GFM_EXTENSIONS_LIBRARY}) +``` + +- [ ] **Step 3: Reconfigure and build** + +Run: + +```bash +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && cmake --build build +``` + +Expected: configures and builds clean. If `find_package` cannot find `Qt6Network`, stop and report it rather than working around it — it means the assumption that Slackware's `qt6` package carries it is wrong, and that changes the SlackBuild too. + +- [ ] **Step 4: Commit** + +```bash +git add CMakeLists.txt src/CMakeLists.txt +git commit -S -m "build: link Qt6::Network for the single-instance socket + +A unix domain socket between two copies of this program, for item 200. Not +network protocol work: the rule in AGENTS.md is about IMAP and SMTP. + +Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> +Claude-Session: https://claude.ai/code/session_01P3HQXLauwQgzxR4YfJBB3x" +``` + +--- + +## Task 2: LaunchSelectors, the value type and its parse + +**Files:** +- Create: `src/launchselectors.h` +- Create: `src/launchselectors.cpp` +- Create: `tests/test_launchselectors.cpp` +- Modify: `src/CMakeLists.txt` +- Modify: `tests/CMakeLists.txt` + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_launchselectors.cpp`: + +```cpp +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <QtTest> + +#include "launchselectors.h" + +/// The command line and the socket payload, over values. No window, no +/// QApplication: this is the half of item 200 that can be asserted exactly, +/// which is why it is its own unit rather than code inside main.cpp. +class TestLaunchSelectors : public QObject +{ + Q_OBJECT + +private slots: + void anEmptyCommandLineSelectsNothing(); + void eachSelectorIsParsed(); + void theThreeSelectorsCompose(); + void anUnknownOptionIsReportedNotFatal(); + void aPayloadRoundTrips(); + void anEmptyPayloadRoundTripsToNothing(); + void aTruncatedPayloadIsRejected(); + void anOversizedPayloadIsRejected(); +}; + +void TestLaunchSelectors::anEmptyCommandLineSelectsNothing() +{ + QString error; + const LaunchSelectors selectors = + LaunchSelectors::parse({ QStringLiteral("qtmaildir") }, &error); + + QVERIFY(error.isEmpty()); + QVERIFY(selectors.isEmpty()); + QVERIFY(selectors.account.isEmpty()); + QVERIFY(selectors.threadId.isEmpty()); + QVERIFY(selectors.messageId.isEmpty()); +} + +void TestLaunchSelectors::eachSelectorIsParsed() +{ + QString error; + + const LaunchSelectors account = LaunchSelectors::parse( + { QStringLiteral("qtmaildir"), QStringLiteral("--account"), + QStringLiteral("work") }, &error); + QVERIFY(error.isEmpty()); + QCOMPARE(account.account, QStringLiteral("work")); + QVERIFY(!account.isEmpty()); + + const LaunchSelectors thread = LaunchSelectors::parse( + { QStringLiteral("qtmaildir"), QStringLiteral("--thread"), + QStringLiteral("0000000000001a2b") }, &error); + QVERIFY(error.isEmpty()); + QCOMPARE(thread.threadId, QStringLiteral("0000000000001a2b")); + + const LaunchSelectors message = LaunchSelectors::parse( + { QStringLiteral("qtmaildir"), QStringLiteral("--message"), + QStringLiteral("<abc@example.org>") }, &error); + QVERIFY(error.isEmpty()); + QCOMPARE(message.messageId, QStringLiteral("<abc@example.org>")); +} + +void TestLaunchSelectors::theThreeSelectorsCompose() +{ + // They are not exclusive: "open this message, in this account's view" is + // one sensible request, and the spec says they compose. + QString error; + const LaunchSelectors selectors = LaunchSelectors::parse( + { QStringLiteral("qtmaildir"), + QStringLiteral("--account"), QStringLiteral("work"), + QStringLiteral("--thread"), QStringLiteral("00001a2b"), + QStringLiteral("--message"), QStringLiteral("<abc@example.org>") }, + &error); + + QVERIFY(error.isEmpty()); + QCOMPARE(selectors.account, QStringLiteral("work")); + QCOMPARE(selectors.threadId, QStringLiteral("00001a2b")); + QCOMPARE(selectors.messageId, QStringLiteral("<abc@example.org>")); +} + +void TestLaunchSelectors::anUnknownOptionIsReportedNotFatal() +{ + // Reported so main() can print it, and NOT a crash or a silent ignore. + // Today's strcmp loop ignores everything it does not know, which is how a + // typo currently produces a normal window and no clue. + QString error; + const LaunchSelectors selectors = LaunchSelectors::parse( + { QStringLiteral("qtmaildir"), QStringLiteral("--nonsense") }, &error); + + QVERIFY(!error.isEmpty()); + QVERIFY(selectors.isEmpty()); +} + +void TestLaunchSelectors::aPayloadRoundTrips() +{ + // What crosses the socket. A round trip is the whole contract: the values + // that go in are the values that come out, including one with an embedded + // newline, which is what defeats a line-based format. + LaunchSelectors original; + original.account = QStringLiteral("work"); + original.threadId = QStringLiteral("00001a2b"); + original.messageId = QStringLiteral("<a\nb@example.org>"); + + QString error; + const LaunchSelectors parsed = + LaunchSelectors::fromPayload(original.toPayload(), &error); + + QVERIFY(error.isEmpty()); + QCOMPARE(parsed.account, original.account); + QCOMPARE(parsed.threadId, original.threadId); + QCOMPARE(parsed.messageId, original.messageId); +} + +void TestLaunchSelectors::anEmptyPayloadRoundTripsToNothing() +{ + // A bare `qtmaildir` with a running instance still sends a payload: it + // means "raise yourself", which is a real request and not an error. + QString error; + const LaunchSelectors parsed = + LaunchSelectors::fromPayload(LaunchSelectors().toPayload(), &error); + + QVERIFY(error.isEmpty()); + QVERIFY(parsed.isEmpty()); +} + +void TestLaunchSelectors::aTruncatedPayloadIsRejected() +{ + // The socket hands over whatever it is given. A half-written payload must + // be refused rather than half-applied. + LaunchSelectors original; + original.account = QStringLiteral("work"); + const QByteArray payload = original.toPayload(); + QVERIFY(payload.size() > 4); + + QString error; + const LaunchSelectors parsed = + LaunchSelectors::fromPayload(payload.left(payload.size() - 2), &error); + + QVERIFY(!error.isEmpty()); + QVERIFY(parsed.isEmpty()); +} + +void TestLaunchSelectors::anOversizedPayloadIsRejected() +{ + // A cap, because a local socket will hand over as much as the peer sends. + // The peer is the user's own process, so this is not a hostile-input + // defence; it is what stops a confused writer from being read as a + // gigabyte of selector. + QString error; + const LaunchSelectors parsed = LaunchSelectors::fromPayload( + QByteArray(LaunchSelectors::kMaxPayloadBytes + 1, 'x'), &error); + + QVERIFY(!error.isEmpty()); + QVERIFY(parsed.isEmpty()); +} + +QTEST_MAIN(TestLaunchSelectors) +#include "test_launchselectors.moc" +``` + +- [ ] **Step 2: Register the test and run it to verify it fails** + +Add to `tests/CMakeLists.txt`, beside the other `add_qtmaildir_test` lines: + +```cmake +add_qtmaildir_test(launchselectors) +``` + +Run: + +```bash +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && cmake --build build +``` + +Expected: FAILS to compile, with `launchselectors.h: No such file or directory`. + +- [ ] **Step 3: Write the header** + +Create `src/launchselectors.h`: + +```cpp +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include <QByteArray> +#include <QString> +#include <QStringList> + +/// What a launch asked the window to show (item 200). +/// +/// A value type with no Qt GUI dependency, deliberately: it is parsed before +/// QApplication exists, it crosses a socket, and both halves need tests that +/// no window has to be built for. +/// +/// The three selectors COMPOSE rather than excluding each other. "Open this +/// message, in this account's view" is one request, and nothing about it is +/// contradictory. +struct LaunchSelectors +{ + /// An account key as written in the config, e.g. `work` from + /// `[account.work]`. Validated against the configured accounts by the + /// window, not here: this unit knows nothing about a Config. + QString account; + + /// A notmuch thread id. + QString threadId; + + /// A Message-ID, with or without the angle brackets. + QString messageId; + + bool isEmpty() const + { + return account.isEmpty() && threadId.isEmpty() && messageId.isEmpty(); + } + + /// Longest payload accepted off the socket. + /// + /// A local socket hands over whatever the peer sends, and the peer here is + /// another copy of this program running as the same user, so this is not a + /// defence against an attacker. It is what stops a confused or truncated + /// writer from being read as an unbounded selector. Three ids and an + /// account key are a few hundred bytes; 64 KiB is room to spare. + static constexpr int kMaxPayloadBytes = 64 * 1024; + + /// Parses a command line, `arguments[0]` being the program name. + /// + /// Takes a QStringList rather than argc/argv so it can be called before + /// QCoreApplication exists, which is what lets --version keep answering on + /// a machine where the GUI cannot open. + /// + /// On an unknown option, returns an empty result and sets \p error. The + /// caller prints it; it is NOT fatal to the window, since a typo should + /// not cost the user their mail client. + static LaunchSelectors parse(const QStringList &arguments, QString *error); + + /// The help text, for `--help`. Translatable prose; the option NAMES are + /// wire format and are never translated. + static QString helpText(const QString &versionDisplay); + + /// Serialises for the socket. The inverse of fromPayload(). + QByteArray toPayload() const; + + /// Parses a socket payload. On anything malformed, oversized or truncated, + /// returns an empty result and sets \p error. + static LaunchSelectors fromPayload(const QByteArray &payload, + QString *error); +}; + +/// Declared in the header that DEFINES the type, as types.h and threaddigest.h +/// do for theirs. A consumer declaring it instead would leave any other +/// consumer without it, and QSignalSpy needs it to carry the type. +Q_DECLARE_METATYPE(LaunchSelectors) +``` + +- [ ] **Step 4: Write the implementation** + +Create `src/launchselectors.cpp`: + +```cpp +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "launchselectors.h" + +#include <QCommandLineOption> +#include <QCommandLineParser> +#include <QCoreApplication> +#include <QDataStream> +#include <QIODevice> + +namespace { + +/// Bumped only if the payload's shape changes incompatibly. Both ends of the +/// socket are the same binary in the ordinary case, but an upgrade can leave an +/// old instance running while a new one is launched, and a version the reader +/// does not know is refused rather than misread. +constexpr quint16 kPayloadVersion = 1; + +} // namespace + +LaunchSelectors LaunchSelectors::parse(const QStringList &arguments, + QString *error) +{ + if (error) + error->clear(); + + QCommandLineParser parser; + // No addHelpOption()/addVersionOption(): those are handled in main() before + // QApplication exists, and Qt's own versions call exit() through + // QCoreApplication, which is not constructed at that point. + QCommandLineOption accountOption( + QStringLiteral("account"), + QCoreApplication::translate( + "LaunchSelectors", "Open this account's view."), + QStringLiteral("key")); + QCommandLineOption threadOption( + QStringLiteral("thread"), + QCoreApplication::translate( + "LaunchSelectors", "Open this thread."), + QStringLiteral("id")); + QCommandLineOption messageOption( + QStringLiteral("message"), + QCoreApplication::translate( + "LaunchSelectors", "Open this message, inside its thread."), + QStringLiteral("id")); + + parser.addOption(accountOption); + parser.addOption(threadOption); + parser.addOption(messageOption); + + // parse(), not process(): process() prints to stderr and calls exit() on an + // error, which would take the window down over a typo. The error is + // returned instead and main() decides. + if (!parser.parse(arguments)) { + if (error) + *error = parser.errorText(); + return {}; + } + + // --version and --help are consumed in main() before this runs, so they + // never reach the parser. An unknown option does, and is an error rather + // than something to ignore: today's strcmp loop ignores everything it does + // not recognise, so a typo silently produces an ordinary window. + const QStringList unknown = parser.unknownOptionNames(); + if (!unknown.isEmpty()) { + if (error) { + *error = QCoreApplication::translate( + "LaunchSelectors", "Unknown option: %1") + .arg(unknown.join(QStringLiteral(", "))); + } + return {}; + } + + LaunchSelectors selectors; + selectors.account = parser.value(accountOption); + selectors.threadId = parser.value(threadOption); + selectors.messageId = parser.value(messageOption); + return selectors; +} + +QString LaunchSelectors::helpText(const QString &versionDisplay) +{ + // The option NAMES are wire format and are never translated; the prose + // beside them is. Kept as one block rather than assembled from pieces so a + // translator sees the layout they are translating. + return QCoreApplication::translate( + "LaunchSelectors", + "qtmaildir %1 - a Qt6 mail client for notmuch-indexed Maildirs\n" + "\n" + "Usage: qtmaildir [options]\n" + "\n" + " -h, --help Show this help and exit\n" + " -v, --version Show the version and exit\n" + " --account <key> Open this account's view\n" + " --thread <id> Open this thread\n" + " --message <id> Open this message, inside its thread\n" + "\n" + "The three selectors combine. When qtmaildir is already " + "running,\n" + "a second launch hands its selectors to that window and exits " + "rather\n" + "than opening a second one.\n" + "\n" + "Configuration: ~/.config/qtmaildir/qtmaildir.conf\n" + "qtmaildir reads a notmuch-indexed Maildir. It does no network\n" + "protocol work: fetching and sending are external commands.\n") + .arg(versionDisplay); +} + +QByteArray LaunchSelectors::toPayload() const +{ + // QDataStream rather than a line-based format: a Message-ID can contain + // almost anything, a newline included, and a length-prefixed encoding does + // not care. The round-trip test carries an embedded newline for exactly + // this reason. + QByteArray payload; + QDataStream stream(&payload, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_6_0); + stream << kPayloadVersion << account << threadId << messageId; + return payload; +} + +LaunchSelectors LaunchSelectors::fromPayload(const QByteArray &payload, + QString *error) +{ + if (error) + error->clear(); + + if (payload.size() > kMaxPayloadBytes) { + if (error) { + *error = QCoreApplication::translate( + "LaunchSelectors", "Launch payload too large"); + } + return {}; + } + + QDataStream stream(payload); + stream.setVersion(QDataStream::Qt_6_0); + + quint16 version = 0; + stream >> version; + if (stream.status() != QDataStream::Ok || version != kPayloadVersion) { + if (error) { + *error = QCoreApplication::translate( + "LaunchSelectors", "Unrecognised launch payload"); + } + return {}; + } + + LaunchSelectors selectors; + stream >> selectors.account >> selectors.threadId >> selectors.messageId; + + // Checked AFTER every read, which is what catches a truncated payload: a + // short read leaves the stream in ReadPastEnd and the fields + // default-constructed, so without this a half-written message id would be + // applied as an empty one. + if (stream.status() != QDataStream::Ok) { + if (error) { + *error = QCoreApplication::translate( + "LaunchSelectors", "Truncated launch payload"); + } + return {}; + } + + return selectors; +} +``` + +- [ ] **Step 5: Add the source to the library** + +In `src/CMakeLists.txt`, add `launchselectors.cpp` to the `qtmaildir_lib` source list, keeping the list's existing order convention. + +- [ ] **Step 6: Run the test to verify it passes** + +```bash +cmake --build build && ctest --test-dir build -R launchselectors --output-on-failure +``` + +Expected: PASS, 8 tests. + +- [ ] **Step 7: Commit** + +```bash +git add src/launchselectors.h src/launchselectors.cpp \ + tests/test_launchselectors.cpp src/CMakeLists.txt tests/CMakeLists.txt +git commit -S -m "feat: parse the launch selectors as a value type + +--account, --thread and --message, plus the payload that crosses the socket. +A value type with no GUI dependency: it is parsed before QApplication exists +and both halves need tests no window has to be built for. + +QDataStream rather than a line-based payload, because a Message-ID may contain +a newline. Every read is status-checked, which is what catches a truncated +payload: a short read otherwise leaves the fields default-constructed and a +half-written id would be applied as an empty one. + +Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> +Claude-Session: https://claude.ai/code/session_01P3HQXLauwQgzxR4YfJBB3x" +``` + +--- + +## Task 3: SingleInstance, the socket + +**Files:** +- Create: `src/singleinstance.h` +- Create: `src/singleinstance.cpp` +- Create: `tests/test_singleinstance.cpp` +- Modify: `src/CMakeLists.txt` +- Modify: `tests/CMakeLists.txt` + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_singleinstance.cpp`: + +```cpp +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <QLocalServer> +#include <QSignalSpy> +#include <QTemporaryDir> +#include <QtTest> + +#include "launchselectors.h" +#include "singleinstance.h" + +/// The socket half of item 200. Every case runs against a socket name of its +/// own inside a QTemporaryDir, so nothing here can reach a real running +/// qtmaildir, and two cases cannot collide. +class TestSingleInstance : public QObject +{ + Q_OBJECT + +private slots: + void init(); + + void theFirstInstanceBecomesTheServer(); + void aSecondInstanceHandsOverItsSelectors(); + void aSecondInstanceWithNoSelectorsStillArrives(); + void aStaleSocketFileIsReclaimed(); + void anUncreatableSocketDoesNotStopStartup(); + +private: + QTemporaryDir m_dir; + QString m_socketPath; +}; + +void TestSingleInstance::init() +{ + QVERIFY(m_dir.isValid()); + // A name per test function, so a socket left behind by one case cannot + // decide the next one's result. + m_socketPath = m_dir.filePath( + QStringLiteral("sock-%1").arg(QTest::currentTestFunction())); +} + +void TestSingleInstance::theFirstInstanceBecomesTheServer() +{ + SingleInstance first(m_socketPath); + QVERIFY(first.tryBecomeServer()); + QVERIFY(first.isServer()); +} + +void TestSingleInstance::aSecondInstanceHandsOverItsSelectors() +{ + // The whole point of the feature: the second process does not open a + // window, it hands its request to the first and exits. + SingleInstance first(m_socketPath); + QVERIFY(first.tryBecomeServer()); + + QSignalSpy arrived(&first, &SingleInstance::selectorsReceived); + + LaunchSelectors selectors; + selectors.account = QStringLiteral("work"); + selectors.messageId = QStringLiteral("<abc@example.org>"); + + SingleInstance second(m_socketPath); + QVERIFY(!second.tryBecomeServer()); + QVERIFY(second.sendToRunningInstance(selectors)); + + QTRY_VERIFY_WITH_TIMEOUT(arrived.count() == 1, 5000); + const auto received = + arrived.first().at(0).value<LaunchSelectors>(); + QCOMPARE(received.account, QStringLiteral("work")); + QCOMPARE(received.messageId, QStringLiteral("<abc@example.org>")); +} + +void TestSingleInstance::aSecondInstanceWithNoSelectorsStillArrives() +{ + // A bare `qtmaildir` against a running instance means "raise yourself". + // That is a real request, so it must arrive rather than being dropped as + // an empty message. + SingleInstance first(m_socketPath); + QVERIFY(first.tryBecomeServer()); + + QSignalSpy arrived(&first, &SingleInstance::selectorsReceived); + + SingleInstance second(m_socketPath); + QVERIFY(!second.tryBecomeServer()); + QVERIFY(second.sendToRunningInstance(LaunchSelectors())); + + QTRY_VERIFY_WITH_TIMEOUT(arrived.count() == 1, 5000); + QVERIFY(arrived.first().at(0).value<LaunchSelectors>().isEmpty()); +} + +void TestSingleInstance::aStaleSocketFileIsReclaimed() +{ + // A crash or a kill leaves the socket file behind, and listen() then fails + // with AddressInUse on a file nothing is serving. Without recovery the + // application would never start again until someone deleted it by hand. + // + // The file is created by a server that is then destroyed WITHOUT removing + // it, which QLocalServer does on an abrupt exit. + // A plain file at the socket path, which is what a killed process leaves + // behind: a filesystem entry with no process serving it. listen() then + // fails with AddressInUse, and nothing answers a connection. + QFile stale(m_socketPath); + QVERIFY(stale.open(QIODevice::WriteOnly)); + stale.close(); + QVERIFY(QFile::exists(m_socketPath)); + + SingleInstance fresh(m_socketPath); + QVERIFY2(fresh.tryBecomeServer(), + "a stale socket file must not stop the application starting"); + QVERIFY(fresh.isServer()); +} + +void TestSingleInstance::anUncreatableSocketDoesNotStopStartup() +{ + // A read-only state directory must degrade to today's behaviour, a window + // that opens and works, rather than to no mail client at all. The caller + // reads isServer() as false and carries on. + const QString impossible = + m_dir.filePath(QStringLiteral("no/such/directory/sock")); + + SingleInstance instance(impossible); + QVERIFY(!instance.tryBecomeServer()); + QVERIFY(!instance.isServer()); + // And it cannot reach a running instance either, since there is none. + QVERIFY(!instance.sendToRunningInstance(LaunchSelectors())); +} + +QTEST_MAIN(TestSingleInstance) +#include "test_singleinstance.moc" +``` + +- [ ] **Step 2: Register the test and run it to verify it fails** + +Add to `tests/CMakeLists.txt`: + +```cmake +add_qtmaildir_test(singleinstance) +``` + +Run: + +```bash +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && cmake --build build +``` + +Expected: FAILS to compile, `singleinstance.h: No such file or directory`. + +- [ ] **Step 3: Write the header** + +Create `src/singleinstance.h`: + +```cpp +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include <QObject> +#include <QString> + +#include "launchselectors.h" + +class QLocalServer; + +/// Makes a launch either the running instance or a messenger to it (item 200). +/// +/// **This is not network protocol work.** A QLocalServer is a unix domain +/// socket between two copies of this program, owned by the user, in the user's +/// own state directory. The rule in AGENTS.md is about IMAP and SMTP. +/// +/// Knows nothing about queries, accounts or mail: it carries a LaunchSelectors +/// from one process to another and emits what arrived. +class SingleInstance : public QObject +{ + Q_OBJECT + +public: + /// \p socketPath is a filesystem path, so a test can point it inside a + /// QTemporaryDir and never touch the user's real one. + explicit SingleInstance(const QString &socketPath, + QObject *parent = nullptr); + ~SingleInstance() override; + + /// Tries to become the instance others talk to. + /// + /// Returns true when this process is now listening, false when another + /// instance already is OR when no socket could be created at all. The + /// caller treats both falses the same way for the second case: **a socket + /// that cannot be created must not stop the window opening**, or a + /// read-only state directory costs the user their mail client. + /// + /// Handles the stale socket file, which is the ordinary aftermath of a + /// crash: it attempts a CONNECTION first, and a refused connection on an + /// existing file proves nothing is serving it, so the file is removed and + /// the listen retried. Connecting first is what stops a live instance + /// being removed out from under itself. + bool tryBecomeServer(); + + /// True when tryBecomeServer() succeeded and this process is listening. + bool isServer() const; + + /// Sends \p selectors to the running instance. Returns false when there is + /// none, or when the write could not be completed. + /// + /// An EMPTY selector set is still sent: a bare `qtmaildir` against a + /// running window means "raise yourself", which is a request and not a + /// no-op. + bool sendToRunningInstance(const LaunchSelectors &selectors); + +signals: + /// A later launch handed these over. Emitted on the server side only. + void selectorsReceived(const LaunchSelectors &selectors); + +private: + QString m_socketPath; + QLocalServer *m_server = nullptr; +}; +``` + +`Q_DECLARE_METATYPE(LaunchSelectors)` is **not** repeated here: it belongs in +`launchselectors.h`, which defines the type, matching `types.h` and +`threaddigest.h`. Declaring it in a consumer instead would leave every other +consumer without it. + +- [ ] **Step 4: Write the implementation** + +Create `src/singleinstance.cpp`: + +```cpp +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "singleinstance.h" + +#include <QDebug> +#include <QDir> +#include <QFileInfo> +#include <QLocalServer> +#include <QLocalSocket> + +namespace { + +/// How long a client waits for the running instance, and how long the server +/// waits for a client's payload. Short: both processes are on this machine and +/// the peer is either there or it is not. A long wait would stall a launch +/// behind a wedged instance, which is worse than opening a window. +constexpr int kTimeoutMs = 2000; + +} // namespace + +SingleInstance::SingleInstance(const QString &socketPath, QObject *parent) + : QObject(parent), m_socketPath(socketPath) +{ + // Registered here rather than at a call site so any connection carrying + // this type works, including a queued one a future caller might add. A + // Q_DECLARE_METATYPE alone gives the type a metatype but does not register + // it under the name a queued invoke resolves, which is the trap AGENTS.md + // records for Q_ENUM. + qRegisterMetaType<LaunchSelectors>("LaunchSelectors"); +} + +SingleInstance::~SingleInstance() +{ + if (m_server) { + m_server->close(); + // Removes the filesystem entry, so an orderly exit leaves nothing for + // the next launch to reclaim. + QLocalServer::removeServer(m_socketPath); + } +} + +bool SingleInstance::isServer() const +{ + return m_server != nullptr && m_server->isListening(); +} + +bool SingleInstance::tryBecomeServer() +{ + if (isServer()) + return true; + + // CONNECT FIRST, and the order is the design. A successful connection means + // a live instance owns this socket and this process is the messenger. A + // refused connection on an EXISTING file means the file is stale, left by a + // crash, and can be removed; doing it in this order is what stops a live + // instance being removed out from under itself. + { + QLocalSocket probe; + probe.connectToServer(m_socketPath); + if (probe.waitForConnected(kTimeoutMs)) { + probe.disconnectFromServer(); + return false; + } + } + + auto *server = new QLocalServer(this); + // The socket is the user's own, in their own state directory. Nothing else + // has any business connecting to it. + server->setSocketOptions(QLocalServer::UserAccessOption); + + if (!server->listen(m_socketPath)) { + if (server->serverError() == QAbstractSocket::AddressInUseError + && QFileInfo::exists(m_socketPath)) { + // Nothing answered the probe above, so this file is stale. + QLocalServer::removeServer(m_socketPath); + server->listen(m_socketPath); + } + } + + if (!server->isListening()) { + // A read-only state directory, a filesystem that has no unix sockets, + // or a path whose parent does not exist. Report and carry on: the + // window must still open. Losing single-instance behaviour is a + // degradation; losing the mail client is not acceptable. + qWarning() << "qtmaildir: cannot create the single-instance socket at" + << m_socketPath << ":" << server->errorString() + << "- continuing without it"; + delete server; + return false; + } + + m_server = server; + connect(m_server, &QLocalServer::newConnection, this, [this]() { + while (QLocalSocket *socket = m_server->nextPendingConnection()) { + // Deleted when the peer goes away, which it does immediately after + // writing: the client's whole life is one payload. + connect(socket, &QLocalSocket::disconnected, + socket, &QLocalSocket::deleteLater); + + if (!socket->waitForReadyRead(kTimeoutMs)) { + socket->disconnectFromServer(); + continue; + } + + // readAll() rather than a sized read: the payload is one short + // write and the cap inside fromPayload() is what bounds it. + const QByteArray payload = socket->readAll(); + QString error; + const LaunchSelectors selectors = + LaunchSelectors::fromPayload(payload, &error); + if (!error.isEmpty()) { + qWarning() << "qtmaildir: ignoring a launch payload:" << error; + socket->disconnectFromServer(); + continue; + } + + // Emitted even when empty: a bare launch means "raise yourself". + emit selectorsReceived(selectors); + socket->disconnectFromServer(); + } + }); + + return true; +} + +bool SingleInstance::sendToRunningInstance(const LaunchSelectors &selectors) +{ + QLocalSocket socket; + socket.connectToServer(m_socketPath); + if (!socket.waitForConnected(kTimeoutMs)) + return false; + + socket.write(selectors.toPayload()); + // Flushed before returning, because the caller exits immediately + // afterwards and an unflushed write would be lost with the process. + if (!socket.waitForBytesWritten(kTimeoutMs)) + return false; + + socket.disconnectFromServer(); + return true; +} +``` + +- [ ] **Step 5: Add the source to the library** + +In `src/CMakeLists.txt`, add `singleinstance.cpp` to the `qtmaildir_lib` source list. + +- [ ] **Step 6: Run the test to verify it passes** + +```bash +cmake --build build && ctest --test-dir build -R singleinstance --output-on-failure +``` + +Expected: PASS, 5 tests. + +- [ ] **Step 7: Commit** + +```bash +git add src/singleinstance.h src/singleinstance.cpp \ + tests/test_singleinstance.cpp src/CMakeLists.txt tests/CMakeLists.txt +git commit -S -m "feat: add the single-instance socket + +A QLocalServer under the state directory. The first launch listens; a later one +connects, hands over its selectors and exits. + +Connect-first ordering, which is also how a stale socket file is detected: a +refused connection on an existing file proves nothing is serving it. Doing it +the other way round would remove a live instance's socket out from under it. + +A socket that cannot be created does NOT stop the window opening. A read-only +state directory costs single-instance behaviour, which is a degradation; it +must not cost the user their mail client. + +Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> +Claude-Session: https://claude.ai/code/session_01P3HQXLauwQgzxR4YfJBB3x" +``` + +--- + +## Task 4: Resolving a Message-ID to its thread + +**Files:** +- Modify: `src/notmuchworker.h:130-141` +- Modify: `src/notmuchworker.cpp` (beside `threadIdForTesting`, around line 764) +- Test: `tests/test_notmuchworker.cpp` + +**Context you need:** `NotmuchWorker::threadIdForTesting(const QString &query)` already exists at `src/notmuchworker.cpp:764` and does exactly this lookup, but it is a plain method named for tests and explicitly documented as "Not a slot, so it cannot be reached across the thread boundary by accident". This task adds a real slot beside it rather than promoting it, because the slot must be asynchronous (it answers by signal) while the existing helper returns a value synchronously, and four tests already depend on the synchronous form. + +- [ ] **Step 1: Write the failing test** + +Add to the `private slots:` list in `tests/test_notmuchworker.cpp`, after `loadMessageOnAnUnknownIdReturnsNothing()`: + +```cpp + void resolvingAMessageIdAnswersItsThreadId(); + void resolvingAnUnknownMessageIdAnswersEmpty(); + void resolvingAMessageIdQuotesTheId(); +``` + +Add the bodies, after `TestNotmuchWorker::loadMessageOnAnUnknownIdReturnsNothing()`: + +```cpp +void TestNotmuchWorker::resolvingAMessageIdAnswersItsThreadId() +{ + // What --message needs (item 200): the CLI knows a Message-ID and the + // window needs the thread id, because opening the message means opening + // its conversation with that message selected. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy resolved(&worker, &NotmuchWorker::threadForMessageResolved); + + worker.resolveThreadForMessage(QStringLiteral("a2@example.org")); + + QCOMPARE(resolved.count(), 1); + QCOMPARE(resolved.first().at(0).toString(), QStringLiteral("a2@example.org")); + + // a2 is a REPLY, so its thread id is the thread's, not its own. Asserted + // against the thread a1 resolves to: the two must agree, which is the + // whole point of resolving through the thread rather than the message. + const QString threadId = resolved.first().at(1).toString(); + QVERIFY(!threadId.isEmpty()); + QCOMPARE(threadId, + worker.threadIdForTesting(QStringLiteral("id:a1@example.org"))); +} + +void TestNotmuchWorker::resolvingAnUnknownMessageIdAnswersEmpty() +{ + // Answers rather than staying silent: the window shows the miss in the + // status bar, and a slot that never replies would leave it waiting. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy resolved(&worker, &NotmuchWorker::threadForMessageResolved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.resolveThreadForMessage(QStringLiteral("nonexistent@example.org")); + + QCOMPARE(resolved.count(), 1); + QCOMPARE(resolved.first().at(0).toString(), + QStringLiteral("nonexistent@example.org")); + QVERIFY(resolved.first().at(1).toString().isEmpty()); + + // Not an error: a stale id from another program is an ordinary miss, the + // same class as a stale row after a reindex. + QCOMPARE(errors.count(), 0); +} + +void TestNotmuchWorker::resolvingAMessageIdQuotesTheId() +{ + // The security-relevant case. This id comes from argv, not from notmuch, + // and notmuch's parser rejects almost nothing: an unquoted id carrying + // query syntax would be PARSED as syntax, matching something else or + // nothing, with no error anywhere (searchterm.h:30-35). + // + // Asserted as a miss that stays a miss: the id cannot match, and it must + // not blow up, error, or resolve to some unrelated thread. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy resolved(&worker, &NotmuchWorker::threadForMessageResolved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.resolveThreadForMessage( + QStringLiteral("a2@example.org\" or from:alice or \"x")); + + QCOMPARE(resolved.count(), 1); + QVERIFY2(resolved.first().at(1).toString().isEmpty(), + "an id carrying query syntax resolved to a thread: it was not quoted"); + QCOMPARE(errors.count(), 0); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAILS to compile, `'threadForMessageResolved' is not a member of 'NotmuchWorker'`. + +- [ ] **Step 3: Declare the slot and the signal** + +In `src/notmuchworker.h`, add to the `public slots:` section that begins at line 141, after the `loadMessage` declaration: + +```cpp + /// Answers which thread a Message-ID belongs to (item 200). + /// + /// For `--message`, which knows an id and needs the conversation: opening + /// a message means opening its thread with that message selected, never an + /// `id:` query showing one card out of a conversation (item 91). + /// + /// Answers with an EMPTY thread id when the message is unknown rather than + /// staying silent, since the window reports the miss and a slot that never + /// replies would leave it waiting forever. + /// + /// **The id is quoted before it reaches notmuch.** Unlike every other id in + /// this class, this one came from argv rather than from notmuch itself, and + /// notmuch parses garbage happily while matching nothing. + void resolveThreadForMessage(const QString &messageId); +``` + +And to the `signals:` section, after `messageLoaded`: + +```cpp + /// The answer to resolveThreadForMessage(). The message id is echoed back + /// so a caller can tell which request this answers; the thread id is empty + /// when nothing matched. + void threadForMessageResolved(const QString &messageId, + const QString &threadId); +``` + +- [ ] **Step 4: Write the implementation** + +In `src/notmuchworker.cpp`, add after `threadIdForTesting()` (which ends around line 786): + +```cpp +void NotmuchWorker::resolveThreadForMessage(const QString &messageId) +{ + if (messageId.isEmpty()) { + emit threadForMessageResolved(messageId, QString()); + return; + } + + // SearchTerm::quote(), not the QStringLiteral("id:\"%1\"") this file uses + // elsewhere. Every other id here came out of notmuch; this one came off the + // command line of another program, so it is untrusted in the ordinary + // sense. quote() escapes backslashes before quotes, which is the order that + // matters, and caps the length. + const QString term = SearchTerm::field(QStringLiteral("id"), messageId); + if (term.isEmpty()) { + emit threadForMessageResolved(messageId, QString()); + return; + } + + // threadIdForTesting() is the same lookup and is deliberately NOT reused + // by name: it is documented as not being a slot so it cannot cross the + // thread boundary by accident, and renaming it would rewrite four existing + // tests for no gain. The shared part is one query, which is small enough + // that a helper would be more indirection than it saves. + emit threadForMessageResolved(messageId, firstThreadIdMatching(term)); +} +``` + +Rename the body of `threadIdForTesting` to a private helper both call. Replace the existing definition at line 764 with: + +```cpp +QString NotmuchWorker::firstThreadIdMatching(const QString &query) +{ + if (!openReadOnly()) + return QString(); + + NmQuery nmQuery(notmuch_query_create(m_db, query.toUtf8().constData())); + if (!nmQuery) + return QString(); + + notmuch_threads_t *rawThreads = nullptr; + if (notmuch_query_search_threads(nmQuery.get(), &rawThreads) + != NOTMUCH_STATUS_SUCCESS) { + return QString(); + } + NmThreads threads(rawThreads); + if (!notmuch_threads_valid(threads.get())) + return QString(); + + NmThread thread(notmuch_threads_get(threads.get())); + if (!thread) + return QString(); + return QString::fromUtf8(notmuch_thread_get_thread_id(thread.get())); +} + +QString NotmuchWorker::threadIdForTesting(const QString &query) +{ + return firstThreadIdMatching(query); +} +``` + +Declare the helper in `src/notmuchworker.h`, in the `private:` section: + +```cpp + /// The first thread id matching \p query, or empty. + /// + /// Shared by threadIdForTesting() and resolveThreadForMessage(). The + /// callers differ in what they do with it and in whether they are slots; + /// the lookup is the same. + QString firstThreadIdMatching(const QString &query); +``` + +Add the include at the top of `src/notmuchworker.cpp`, with the other project includes: + +```cpp +#include "searchterm.h" +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +```bash +cmake --build build && ctest --test-dir build -R notmuchworker --output-on-failure +``` + +Expected: PASS. The whole `notmuchworker` suite, not only the three new cases — the refactor of `threadIdForTesting` touches four existing tests. + +- [ ] **Step 6: Commit** + +```bash +git add src/notmuchworker.h src/notmuchworker.cpp tests/test_notmuchworker.cpp +git commit -S -m "feat: resolve a Message-ID to its thread id + +For --message (item 200), which knows an id and needs the conversation: +opening a message means opening its thread with that message selected, never +an id: query showing one card out of a conversation (item 91). + +The id is quoted through SearchTerm, unlike every other id in this class. +Those came out of notmuch; this one comes off another program's command line, +and notmuch parses garbage happily while matching nothing, so an unquoted id +carrying query syntax would be read AS syntax with no error anywhere. + +threadIdForTesting() keeps its name and gains a shared helper rather than being +promoted: it is documented as not being a slot, and the new entry point has to +answer asynchronously. + +Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> +Claude-Session: https://claude.ai/code/session_01P3HQXLauwQgzxR4YfJBB3x" +``` + +--- + +## Task 5: MainWindow::applySelectors() + +**Files:** +- Modify: `src/mainwindow.h` (public section around line 100, private slots around line 605) +- Modify: `src/mainwindow.cpp` (near `uiStatePath()` at line 92, the worker wiring at line 2681) +- Test: `tests/test_mainwindow.cpp` + +- [ ] **Step 1: Write the failing test** + +Add to the `private slots:` list in `tests/test_mainwindow.cpp`, after `aGeneratedStartupQueryActuallyRuns()`: + +```cpp + void theSocketPathIsUnderTheStateDirectory(); + void anAccountSelectorMovesTheDropdown(); + void anUnknownAccountSelectorLeavesTheDropdownAlone(); + void aThreadSelectorOpensThatThread(); + void anEmptySelectorSetChangesNothing(); +``` + +Add the bodies at the end of the file, before the `QTEST_MAIN` line: + +```cpp +void TestMainWindow::theSocketPathIsUnderTheStateDirectory() +{ + // Beside uistate.conf, and built the same way: GenericStateLocation, not + // StateLocation, because the latter appends both the organization and the + // application name and both are "qtmaildir". + const QString socket = MainWindow::singleInstanceSocketPath(); + const QString state = MainWindow::uiStatePath(); + + QVERIFY(!socket.isEmpty()); + QCOMPARE(QFileInfo(socket).absolutePath(), + QFileInfo(state).absolutePath()); + QVERIFY2(!socket.endsWith(QStringLiteral("/qtmaildir/qtmaildir")), + "StateLocation was used: the path doubles the application name"); +} + +void TestMainWindow::anAccountSelectorMovesTheDropdown() +{ + // --account work, with the config's own startup account being something + // else. The selector wins, which is what "open this account's view" means. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + // [general] keys are read WITHOUT the prefix: QSettings' INI backend + // treats a section literally named [general] as its own fallback. + s.setValue(QStringLiteral("startup_query"), QStringLiteral("Inbox")); + s.setValue(QStringLiteral("startup_account"), + QStringLiteral("personal")); + s.beginGroup(QStringLiteral("account.work")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("work")); + s.endGroup(); + s.beginGroup(QStringLiteral("account.personal")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("personal")); + s.endGroup(); + s.sync(); + } + Config config; + config.load(path); + + MainWindow window(config); + QCOMPARE(window.selectedAccountForTesting(), QStringLiteral("personal")); + + LaunchSelectors selectors; + selectors.account = QStringLiteral("work"); + window.applySelectors(selectors); + + QCOMPARE(window.selectedAccountForTesting(), QStringLiteral("work")); +} + +void TestMainWindow::anUnknownAccountSelectorLeavesTheDropdownAlone() +{ + // The miss path. The window opens on its configured view and says so in + // the status bar; it does not clear the dropdown, and it does not refuse + // to start. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.setValue(QStringLiteral("startup_account"), QStringLiteral("work")); + s.beginGroup(QStringLiteral("account.work")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("work")); + s.endGroup(); + s.sync(); + } + Config config; + config.load(path); + + MainWindow window(config); + + LaunchSelectors selectors; + selectors.account = QStringLiteral("nosuchaccount"); + window.applySelectors(selectors); + + QCOMPARE(window.selectedAccountForTesting(), QStringLiteral("work")); + + // "statusMessage", which is the object name buildUi() sets at + // mainwindow.cpp:674. There is no widget named "statusLabel", and + // findChild would return null and assert nothing. + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + QVERIFY2(status->text().contains(QStringLiteral("nosuchaccount")), + "the miss must name the value, so a stale caller can be debugged"); +} + +void TestMainWindow::aThreadSelectorOpensThatThread() +{ + // --thread, against a real database. The row the selector names is the row + // that ends up current. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("inbox"), QStringLiteral("one@example.org"), + QStringLiteral("First subject"), QStringLiteral("a@example.org"), + // Friday, verified with `date -d 2026-08-14 +%A`. Qt::RFC2822Date + // validates the weekday against the date. + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body one."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("inbox"), QStringLiteral("two@example.org"), + QStringLiteral("Second subject"), QStringLiteral("b@example.org"), + // Saturday, verified with `date -d 2026-08-15 +%A`. + QStringLiteral("Sat, 15 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body two."))); + QVERIFY2(backed.build(), qPrintable(backed.error())); + + // The thread id is not knowable in advance, so it is read back from the + // index the same way the CLI's caller would have obtained it. + NotmuchWorker probe(backed.config().notmuchConfig()); + const QString threadId = + probe.threadIdForTesting(QStringLiteral("id:two@example.org")); + QVERIFY(!threadId.isEmpty()); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<ThreadListView *>(); + QVERIFY(view); + + LaunchSelectors selectors; + selectors.threadId = threadId; + window.applySelectors(selectors); + + // QTRY, never qWait: the worker is on another thread and a fixed sleep + // passes when the result never arrives at all. + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + // Asserted on the thread the row STANDS FOR, through threadFor(), never + // threadAt(index.row()): a tree numbers rows per parent. + QTRY_VERIFY_WITH_TIMEOUT(view->currentIndex().isValid(), 15000); + QCOMPARE(model->threadFor(view->currentIndex()).threadId, threadId); +} + +void TestMainWindow::anEmptySelectorSetChangesNothing() +{ + // A bare `qtmaildir` against a running window means "raise yourself". It + // must not re-run a query or move the selection: the user is looking at + // something, and a raise is not a navigation. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.setValue(QStringLiteral("startup_account"), QStringLiteral("work")); + s.beginGroup(QStringLiteral("account.work")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("work")); + s.endGroup(); + s.sync(); + } + Config config; + config.load(path); + + MainWindow window(config); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + queryEdit->setText(QStringLiteral("tag:flagged")); + + window.applySelectors(LaunchSelectors()); + + QCOMPARE(window.selectedAccountForTesting(), QStringLiteral("work")); + QCOMPARE(queryEdit->text(), QStringLiteral("tag:flagged")); +} +``` + +Add the include at the top of `tests/test_mainwindow.cpp`, with the other project includes: + +```cpp +#include "launchselectors.h" +``` + +Nothing else needs adding: `QLabel`, `QFileInfo` (via `QDir`/`QtTest`), `threadlistmodel.h` and `threadlistview.h` are already included in that file. + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAILS to compile, `'applySelectors' is not a member of 'MainWindow'`. + +- [ ] **Step 3: Declare the new members** + +In `src/mainwindow.h`, add to the `public:` section, right after the `uiStatePath()` declaration (around line 165): + +```cpp + /// Path of the single-instance socket (item 200). + /// + /// Beside uiStatePath() and built the same way, so the two cannot drift. + /// GenericStateLocation, not StateLocation: the latter appends both the + /// organization and the application name, and both are "qtmaildir". + static QString singleInstanceSocketPath(); +``` + +And to the `public:` section around line 100, after `pendingChangeSnapshot()`: + +```cpp + /// Applies what a launch asked for (item 200). + /// + /// ONE entry point, called both by main() at startup and by the socket + /// handler when a later launch arrives. Two paths through separate code + /// would drift, which is the lesson this file has already learned from + /// every other pair. + /// + /// An EMPTY selector set deliberately changes nothing: a bare launch + /// against a running window means "raise yourself", and a raise is not a + /// navigation. The user is looking at something. + /// + /// A selector that matches nothing leaves the window on its configured + /// view and names the miss in the status bar. Not an empty result, which + /// makes a stale link look like a broken client; not a refusal, which is + /// right for a script and wrong for a desktop launch. + void applySelectors(const LaunchSelectors &selectors); +``` + +Add the include at the top of `src/mainwindow.h`: + +```cpp +#include "launchselectors.h" +``` + +And in the `private slots:` section, beside `recoverStaleThread`: + +```cpp + /// The worker's answer to a --message selector. + void onThreadForMessageResolved(const QString &messageId, + const QString &threadId); +``` + +- [ ] **Step 4: Write the implementation** + +In `src/mainwindow.cpp`, add after `uiStatePath()` (which ends at line 101): + +```cpp +QString MainWindow::singleInstanceSocketPath() +{ + // Built exactly like uiStatePath(), including the GenericStateLocation + // choice and the reason for it. A socket is machine-written state, so it + // belongs beside the UI state and never in the hand-edited config + // directory. + const QString base = + QStandardPaths::writableLocation(QStandardPaths::GenericStateLocation); + return base + QStringLiteral("/qtmaildir/qtmaildir.sock"); +} +``` + +Add the method body, near the other public methods: + +```cpp +void MainWindow::applySelectors(const LaunchSelectors &selectors) +{ + // Nothing asked for. A bare launch against a running window means "raise + // yourself", which main() and the socket handler do around this call; from + // here there is nothing to change, and re-running a query would take the + // user off whatever they were reading. + if (selectors.isEmpty()) + return; + + // The account FIRST, and the order matters: a built-in filter composes + // with the dropdown, so a query run before the account moved would carry + // the old scope. This is the same ordering the startup path uses. + if (!selectors.account.isEmpty()) { + const int index = m_accountBox->findData(selectors.account); + if (index >= 0) { + m_accountBox->setCurrentIndex(index); + } else { + // Named, so a caller passing a stale key can be debugged from the + // client rather than from the caller. + showTransientStatus( + tr("No account named '%1'.").arg(selectors.account)); + } + } + + // A message id names a message INSIDE a conversation, so it has to be + // resolved to its thread before anything can be opened. Asked of the + // worker, which owns the only database handle; the answer arrives in + // onThreadForMessageResolved(). + if (!selectors.messageId.isEmpty()) { + if (m_worker) { + QMetaObject::invokeMethod( + m_worker, "resolveThreadForMessage", Qt::QueuedConnection, + Q_ARG(QString, selectors.messageId)); + } + // The thread selector, if any, is deliberately NOT also applied here: + // the message's own thread is what will open, and running a second + // query underneath it would race the one the resolve is about to + // start. + return; + } + + if (!selectors.threadId.isEmpty()) { + // recoverStaleThread() is reused whole. It runs thread:<id>, remembers + // the target across the two queued round trips the load takes, expands + // the thread when its row arrives and selects the message once the + // replies land. Item 91's double-click already reuses it; this is the + // third caller. + // + // The empty message id is meaningful to it: land on the ROOT row, + // which is the thread's first message. + recoverStaleThread(selectors.threadId, QString()); + } +} + +void MainWindow::onThreadForMessageResolved(const QString &messageId, + const QString &threadId) +{ + if (threadId.isEmpty()) { + // The miss path, and the window stays where it is. A message id from + // another program can be stale for every ordinary reason: the mail was + // deleted, moved by another client, or never indexed here. + showTransientStatus(tr("No message matched '%1'.").arg(messageId)); + return; + } + + // The THREAD, with that message selected. An id: query on the message + // alone would show one card out of its conversation, which item 91 settled + // is the wrong reading of "open this message". + recoverStaleThread(threadId, messageId); +} +``` + +In `wireWorker()`, add beside the other worker connections (after the `messageLoaded` connect at line 2691): + +```cpp + connect(m_worker, &NotmuchWorker::threadForMessageResolved, + this, &MainWindow::onThreadForMessageResolved); +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +```bash +cmake --build build && ctest --test-dir build -R mainwindow --output-on-failure +``` + +Expected: PASS. The whole `mainwindow` suite, not only the five new cases. + +- [ ] **Step 6: Commit** + +```bash +git add src/mainwindow.h src/mainwindow.cpp tests/test_mainwindow.cpp +git commit -S -m "feat: apply the launch selectors to the window + +One entry point, called both at startup and by the socket handler when a later +launch arrives. Two paths would drift, which is the lesson this file has +already learned from every other pair. + +The account moves first, because a built-in filter composes with the dropdown +and a query run before it would carry the old scope. The thread case reuses +recoverStaleThread() whole, as item 91's double-click already does. A message +id resolves to its thread first: opening a message means opening its +conversation with that message selected. + +An empty selector set changes nothing. A bare launch against a running window +means raise yourself, and a raise is not a navigation. + +Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> +Claude-Session: https://claude.ai/code/session_01P3HQXLauwQgzxR4YfJBB3x" +``` + +--- + +## Task 6: Wiring it into main() + +**Files:** +- Modify: `src/main.cpp:38-66` and `src/main.cpp:138-152` + +**No test for this task.** `main.cpp` is not compiled into `qtmaildir_lib` (see `src/CMakeLists.txt:67`, only the executable compiles it), so no test binary can reach it. That is why Tasks 2, 3 and 5 put every decidable thing in the library: what is left here is wiring, and it is verified by the hand test in Task 8. + +- [ ] **Step 1: Replace the argument loop** + +In `src/main.cpp`, replace the whole `for (int i = 1; i < argc; ++i)` block (lines 44-66) with: + +```cpp + // Answered before anything heavier starts: registering web engine schemes + // and constructing a QApplication to print one line would be absurd, and + // --version has to work on a machine where the GUI cannot open at all. + // + // Still hand-checked rather than left to QCommandLineParser: Qt's own + // addVersionOption()/addHelpOption() exit through QCoreApplication, which + // does not exist yet at this point. + QStringList arguments; + arguments.reserve(argc); + for (int i = 0; i < argc; ++i) + arguments.append(QString::fromLocal8Bit(argv[i])); + + for (int i = 1; i < argc; ++i) { + if (std::strcmp(argv[i], "--version") == 0 + || std::strcmp(argv[i], "-v") == 0) { + std::printf("qtmaildir %s\n", QTMAILDIR_VERSION_DISPLAY); + return 0; + } + if (std::strcmp(argv[i], "--help") == 0 + || std::strcmp(argv[i], "-h") == 0) { + // The text lives with the parser, so the options and their + // descriptions cannot drift apart. + std::printf("%s", + LaunchSelectors::helpText( + QStringLiteral(QTMAILDIR_VERSION_DISPLAY)) + .toLocal8Bit() + .constData()); + return 0; + } + } + + QString selectorError; + const LaunchSelectors selectors = + LaunchSelectors::parse(arguments, &selectorError); + if (!selectorError.isEmpty()) { + std::fprintf(stderr, "qtmaildir: %s\n", + selectorError.toLocal8Bit().constData()); + return 2; + } +``` + +Add the includes at the top of `src/main.cpp`, with the other project includes: + +```cpp +#include "launchselectors.h" +#include "singleinstance.h" +``` + +- [ ] **Step 2: Add the connect-or-listen step** + +In `src/main.cpp`, immediately after the `QApplication app(argc, argv);` block and its `setApplicationName`/`setOrganizationName`/`setApplicationVersion` calls (which end around line 83), insert: + +```cpp + // Connect first, become the server only if that fails. A live instance is + // handed the selectors and this process exits without ever opening a + // database: notmuch permits one handle per process, so two windows are two + // handles, which this avoids as a side effect of the feature. + // + // On its own stack frame in main(), like the QTranslator below: it owns the + // socket for the life of the process and must outlive exec(). + SingleInstance instance(MainWindow::singleInstanceSocketPath()); + if (!instance.tryBecomeServer()) { + if (instance.sendToRunningInstance(selectors)) + return 0; + // No running instance answered and no socket could be created either. + // Carry on and open a window: losing single-instance behaviour is a + // degradation, and losing the mail client is not acceptable. + } +``` + +- [ ] **Step 3: Apply the selectors and handle later launches** + +In `src/main.cpp`, replace the `MainWindow window(config); window.show();` pair and what follows (lines 138-152) with: + +```cpp + MainWindow window(config); + window.show(); + + // What this launch asked for. After show(), so the window is up before a + // query starts running against it. + window.applySelectors(selectors); + + // A later launch. The selectors arrive on the socket and go through the + // same applySelectors() this startup path just used. + QObject::connect(&instance, &SingleInstance::selectorsReceived, &window, + [&window](const LaunchSelectors &arrived) { + // Raised whatever the selectors say, an empty set + // included: a bare launch against a running window + // means "show me the window". + // + // Under Wayland this is a REQUEST, not a command. The + // compositor may honour it as a focus hint or ignore + // it by policy, which is its decision and not a defect + // to work around: the selectors still apply and the + // window still shows the right thing. + window.setWindowState(window.windowState() + & ~Qt::WindowMinimized); + window.show(); + window.raise(); + window.activateWindow(); + window.applySelectors(arrived); + }); + + // After show(), and out here rather than inside the constructor. A modal + // raised from the constructor cannot be dismissed under the offscreen + // platform, so it hung the test suite with no output (item 84). Showing it + // here also gives the dialog a visible parent to sit on. + const QStringList problems = window.configProblems(); + if (!problems.isEmpty()) { + QMessageBox::warning(&window, QObject::tr("Configuration problems"), + problems.join(QLatin1Char('\n'))); + } + + return app.exec(); +``` + +- [ ] **Step 4: Build and run the whole suite** + +```bash +cmake --build build && ctest --test-dir build --output-on-failure +``` + +Expected: every test passes. A failure in a suite this task did not touch means the new library sources broke something; fix it before committing. + +- [ ] **Step 5: Commit** + +```bash +git add src/main.cpp +git commit -S -m "feat: accept the launch selectors on the command line + +Connect first, become the server only if that fails. A live instance is handed +the selectors and this process exits without ever opening a database, so two +launches no longer mean two notmuch handles. + +--version and --help stay hand-checked rather than going through +QCommandLineParser: Qt's own versions exit through QCoreApplication, which does +not exist at that point, and --version has to work where the GUI cannot open. + +Raising under Wayland is a request rather than a command. The compositor may +honour it as a focus hint or ignore it by policy; the selectors apply either +way, which is the half that has to work. + +Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> +Claude-Session: https://claude.ai/code/session_01P3HQXLauwQgzxR4YfJBB3x" +``` + +--- + +## Task 7: Translations and documentation + +**Files:** +- Modify: `translations/qtmaildir_it_IT.ts` +- Modify: `README.md` (a new section after `## Requirements`, line 30) +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: Refresh the translation source** + +```bash +lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations none +``` + +Expected: a clean run reporting **zero context warnings**. A warning saying `tr() cannot be called without context` means a literal needs `QT_TRANSLATE_NOOP("TheClass", "Text")` rather than a bare `tr()`; the strings added by this plan are all inside classes or use `QCoreApplication::translate` with an explicit context, so a warning here is a real defect to fix. + +- [ ] **Step 2: Translate the new strings** + +Fill in every `<translation type="unfinished">` that this change introduced. The new strings and their Italian: + +| Source | Italian | +|---|---| +| `Open this account's view.` | `Apre la vista di questo account.` | +| `Open this thread.` | `Apre questa conversazione.` | +| `Open this message, inside its thread.` | `Apre questo messaggio, nella sua conversazione.` | +| `Unknown option: %1` | `Opzione sconosciuta: %1` | +| `Launch payload too large` | `Dati di avvio troppo grandi` | +| `Unrecognised launch payload` | `Dati di avvio non riconosciuti` | +| `Truncated launch payload` | `Dati di avvio troncati` | +| `No account named '%1'.` | `Nessun account chiamato '%1'.` | +| `No message matched '%1'.` | `Nessun messaggio corrisponde a '%1'.` | + +The `helpText()` block is one long string; translate the prose lines and **leave the option names (`--account`, `--thread`, `--message`, `-h`, `-v`) exactly as they are**. They are wire format: a translated option name is an option the user cannot type. + +- [ ] **Step 3: Verify the translations compile and the suite agrees** + +```bash +cmake --build build && ctest --test-dir build -R translations --output-on-failure +``` + +Expected: PASS. `lrelease` must report **0 unfinished**: it silently DROPS an unfinished string and ships it as English inside an otherwise Italian UI. + +- [ ] **Step 4: Add the README section** + +In `README.md`, insert a new section after `## Requirements` (which begins at line 30) and before `## Building`: + +```markdown +## Usage + +``` +qtmaildir [options] + + -h, --help Show this help and exit + -v, --version Show the version and exit + --account <key> Open this account's view + --thread <id> Open this thread + --message <id> Open this message, inside its thread +``` + +The three selectors combine: `--account work --message '<abc@example.org>'` +opens that message in the work account's view. + +**A second launch does not open a second window.** When qtmaildir is already +running, a launch hands its selectors to the running window, asks it to raise +itself, and exits. This is what lets another program, a notification or a +script open a particular message in the client the user already has open. It +also means one process, and so one notmuch database handle. + +Under a Wayland compositor, raising a window is a request rather than a +command: the compositor may honour it, or apply its own focus policy. The +selectors are applied either way. + +A selector that matches nothing, a stale thread id or an account key that is +not configured, leaves the window on its normal startup view and says what +missed in the status bar. It is never a reason to refuse to start. +``` + +- [ ] **Step 5: Add the changelog entry** + +In `CHANGELOG.md`, under `## [Unreleased]`, add: + +```markdown +### Added + +- `--account`, `--thread` and `--message` on the command line, so another + program can open qtmaildir at a particular account's view, conversation or + message. The three combine. +- A second launch now hands its selectors to the already-running window and + asks it to raise itself, rather than opening a second window. One process, + and so one notmuch database handle. +``` + +- [ ] **Step 6: Commit** + +```bash +git add translations/qtmaildir_it_IT.ts README.md CHANGELOG.md +git commit -S -m "docs: document the launch selectors and translate them + +The option names stay untranslated in the Italian help text: they are wire +format, and a translated option name is an option the user cannot type. + +Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> +Claude-Session: https://claude.ai/code/session_01P3HQXLauwQgzxR4YfJBB3x" +``` + +--- + +## Task 8: Hand over for the hand test + +**Files:** none. + +Three properties of this feature cannot be tested here, for reasons `AGENTS.md` records, and they are handed to the user rather than covered by a test that would pass whatever the code does. + +- [ ] **Step 1: Run the full suite one more time** + +```bash +ctest --test-dir build --output-on-failure +``` + +Expected: all tests pass. Report the actual count rather than asserting success. + +- [ ] **Step 2: Verify the clean version string** + +```bash +cmake -S . -B /tmp/qtmaildir-clean -G Ninja -DCMAKE_BUILD_TYPE=Debug \ + -DQTMAILDIR_BUILD_NUMBER=OFF && cmake --build /tmp/qtmaildir-clean +/tmp/qtmaildir-clean/src/qtmaildir --version +``` + +Expected: a bare `qtmaildir X.Y.Z`, with no build number. This confirms `--version` still answers before `QApplication` exists, which is the one property the parser rewrite could quietly have broken. + +Note this is `--version` only, which prints and exits. **Do not launch the GUI.** + +- [ ] **Step 3: Hand it over** + +Tell the user the branch is ready and what to look at. Do not launch the application; running it is theirs. + +What to ask them to check: + +1. **With qtmaildir closed**, run `qtmaildir --account <one of their keys>` and confirm the window opens on that account's view. +2. **With qtmaildir already open**, run the same command from another terminal and confirm **no second window appears**, the existing window switches account, and it comes to the front. Whether it takes focus is Hyprland's decision; the switch is the part that must work. +3. **A stale selector**: `qtmaildir --thread 0000000000000000` against the running window. Expect the normal view and a status-bar line naming the id. +4. **`qtmaildir --message '<some real Message-ID>'`**, taken from a message they can see, and confirm the conversation opens with that message selected rather than one card on its own. +5. **`qtmaildir --nonsense`**, and confirm it prints the error and exits 2 rather than opening a window. + +--- + +## Self-review notes + +**Spec coverage.** Every section of `2026-09-13-cli-selectors-design.md` maps to a task: the command line to Task 2, single instance to Task 3, the Message-ID round trip to Task 4, applying the selectors and the miss path to Task 5, raising and the startup order to Task 6, translations/docs to Task 7, and the hand test to Task 8. The `Qt6::Network` constraint is Task 1. + +**One deviation from the spec, deliberately.** The spec says `QCommandLineParser` "replaces the `strcmp` loop". It replaces it for the three selectors; `--version` and `--help` keep their hand-check, because Qt's `addVersionOption()`/`addHelpOption()` exit through `QCoreApplication`, which does not exist at that point in `main()`. The spec's real constraint, that those two must answer without a `QApplication`, is met. + +**One thing the spec did not know.** `NotmuchWorker::threadIdForTesting()` already performs the Message-ID lookup synchronously and has four existing callers. Task 4 adds a slot beside it over a shared private helper rather than promoting it, because the new entry point must answer by signal while the existing one returns a value. diff --git a/docs/superpowers/plans/2026-09-13-spam-view.md b/docs/superpowers/plans/2026-09-13-spam-view.md new file mode 100644 index 0000000..6c33edc --- /dev/null +++ b/docs/superpowers/plans/2026-09-13-spam-view.md @@ -0,0 +1,498 @@ +# Mark spam moves mail, and there is a Spam view — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Turn `Mark spam` into a real move into a per-account spam folder, add a path-based Spam filter, rename the origin tag `deleted-from:` to `moved-from:`, put `spam` on the message bar, and add Empty Spam plus a stranded-spam cleanup. + +**Architecture:** Spam copies Trash everywhere: a mandatory per-account `spam` key, `Account::spamQuery()` and `Config::allSpamQuery()`, a seventh generated filter, and a `sendMove()` that writes `spam` plus the origin tag and strips `unread`/`inbox`. The origin-tag rename centralises the prefix into one constant and adds worker-side overwrite semantics (one origin tag ever). + +**Tech Stack:** Qt6 Widgets, libnotmuch, CMake/Ninja, ctest. + +**Spec:** `docs/superpowers/specs/2026-09-10-spam-view-design.md` — the plan argues from it; read both. + +## Global Constraints + +- `tr()` on every user-facing string. `lupdate` must report zero context warnings and `lrelease` zero unfinished. +- The origin-tag prefix is wire format: never translated, never matched against a translated string. +- Never run a test binary without `QT_QPA_PLATFORM=offscreen`. Never launch `./build/src/qtmaildir`. +- `QTRY_VERIFY_WITH_TIMEOUT`, never a fixed `qWait`. +- A move's origin is resolved by the worker, never read from the model. +- No confirmation dialogs for tag mutations. Empty Spam is a move (undoable), so it gets no dialog. +- Adding an action touches FIVE places: `KeyMap::knownActions()`, `defaultBindings()` (optional), the icon table, the action itself, and a menu. `everyActionIsReachableFromAMenu()` enforces the last. +- Build/test: `cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && cmake --build build && ctest --test-dir build --output-on-failure`. A single test: `ctest --test-dir build -R <name>`. + +--- + +### Task 1: Config — the `spam` key and its queries + +**Files:** +- Modify: `src/config.h` (Account field + method decls) +- Modify: `src/config.cpp` (parse, validate, `spamQuery()`, `allSpamQuery()`) +- Test: `tests/test_config.cpp` + +**Interfaces:** +- Produces: `Account::spam` (QString), `Account::spamQuery() -> QString`, `Config::allSpamQuery() -> QString`. + +- [ ] **Step 1: Write the failing tests.** Add these four methods to `tests/test_config.cpp`, and register their names in the `private slots:` block (near the trash tests around line 120). + +```cpp +void TestConfig::anAccountCarriesItsSpamFolder() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\nspam=Spam\n"))); + const Account account = config.account(QStringLiteral("work")); + QCOMPARE(account.spam, QStringLiteral("Spam")); + QCOMPARE(account.spamQuery(), QStringLiteral("path:\"work/Spam/**\"")); +} + +void TestConfig::aBracketedSpamFolderIsQuoted() +{ + Account account; + account.maildir = QStringLiteral("provider-a"); + account.spam = QStringLiteral("[Provider]/Spam"); + QCOMPARE(account.spamQuery(), + QStringLiteral("path:\"provider-a/[Provider]/Spam/**\"")); +} + +void TestConfig::anAccountWithoutASpamFolderWarns() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\n"))); + QVERIFY(config.account(QStringLiteral("work")).isValid()); + const QString joined = config.warnings().join(QLatin1Char('\n')); + QVERIFY(joined.contains(QStringLiteral("work"))); + QVERIFY(joined.contains(QStringLiteral("spam"))); +} + +void TestConfig::allSpamQueryJoinsAndSkips() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\nspam=Spam\n" + "\n[account.personal]\nmaildir=personal\nspam=[Provider]/Spam\n" + "\n[account.none]\nmaildir=none\n"))); + const QString all = config.allSpamQuery(); + QVERIFY(all.contains(QStringLiteral("path:\"work/Spam/**\""))); + QVERIFY(all.contains(QStringLiteral("path:\"personal/[Provider]/Spam/**\""))); + QVERIFY(!all.contains(QStringLiteral("none"))); +} +``` + +- [ ] **Step 2: Run and confirm failure.** `ctest --test-dir build -R config` fails to compile: `spam` is not a member of `Account`. + +- [ ] **Step 3: Implement.** In `src/config.h`, add a `QString spam;` field beside `trash` (line ~81) with a doc comment mirroring `trash`'s (mandatory; a missing one is a config error reported by `Config::load()`). Add `QString spamQuery() const;` near `trashQuery()` and `QString allSpamQuery() const;` near `allTrashQuery()`. In `src/config.cpp`: + - In `load()`, read the key beside `account.trash` (line ~528): `account.spam = settings.value(QStringLiteral("spam")).toString().trimmed();` + - Beside the trash-missing warning (line ~580), add a matching block naming the `spam` key and warning that Mark spam will not work for that account. + - After `Account::trashQuery()` (line ~156): + +```cpp +QString Account::spamQuery() const +{ + return folderQuery(maildir, spam); +} +``` + + - After `Config::allTrashQuery()` (line ~190): + +```cpp +QString Config::allSpamQuery() const +{ + return joinAccountQueries(m_accounts, &Account::spamQuery); +} +``` + +- [ ] **Step 4: Run.** `ctest --test-dir build -R config` passes. +- [ ] **Step 5: Commit.** `git add src/config.h src/config.cpp tests/test_config.cpp && git commit -S -m "feat(config): per-account spam folder and queries"` + +--- + +### Task 2: The Spam filter + +**Files:** +- Modify: `src/config.cpp` (`kQueryGenerators`, `builtinFilter()`, both `resolvedQuery()` overloads) +- Modify: `src/mainwindow.cpp` (`filterIcons`) +- Test: `tests/test_config.cpp` + +**Interfaces:** +- Consumes: `Account::spamQuery()`, `Config::allSpamQuery()` (Task 1). +- Produces: generator `"spam"`, `Config::builtinFilter("spam")` labelled `tr("Spam")` and threaded. + +- [ ] **Step 1: Write the failing tests** in `tests/test_config.cpp` (mirror the trash pair at lines 1099–1145), and register the names in `private slots:`. + +```cpp +void TestConfig::theSpamFilterComposesPerAccount() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\nspam=Spam\n" + "\n[account.personal]\nmaildir=personal\nspam=[Provider]/Spam\n"))); + const SavedQuery spam = Config::builtinFilter(QStringLiteral("spam")); + QVERIFY(spam.isGenerated()); + QVERIFY2(!spam.flat, "spam must be threaded, like trash"); + const QString all = config.resolvedQuery(spam, QString()); + QVERIFY(all.contains(QStringLiteral("path:\"work/Spam/**\""))); + QVERIFY(all.contains(QStringLiteral("path:\"personal/[Provider]/Spam/**\""))); + const QString scoped = config.resolvedQuery(spam, QStringLiteral("work")); + QCOMPARE(scoped, QStringLiteral("path:\"work/Spam/**\"")); + QVERIFY(!scoped.contains(QStringLiteral("personal"))); +} + +void TestConfig::theSpamFilterMatchesNothingWithoutAFolder() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\n"))); + const SavedQuery spam = Config::builtinFilter(QStringLiteral("spam")); + QCOMPARE(config.resolvedQuery(spam, QString()), Config::matchNothingQuery()); +} +``` + +- [ ] **Step 2: Run.** `ctest --test-dir build -R config` fails: the spam generator is unknown. + +- [ ] **Step 3: Implement** in `src/config.cpp`: + - Add `QStringLiteral("spam")` to `kQueryGenerators` (line 62). + - In `builtinFilter()` (line ~1035), add: + +```cpp +} else if (generator == QStringLiteral("spam")) { + filter.name = tr("Spam"); + // NOT flat, like trash: a spam message still belongs to its conversation. +} +``` + + - In `resolvedQuery(const SavedQuery &)` (line ~982), add beside the trash case: `if (query.generated == QStringLiteral("spam")) return allSpamQuery();` + - In `resolvedQuery(const SavedQuery &, const QString &accountKey)`: + - all-accounts branch (~line 1097): add spam, returning `allSpamQuery()` or `matchNothingQuery()` when empty. + - per-account branch (~line 1119): add spam, returning `scope.spamQuery()` or `matchNothingQuery()` when empty. Never the all-accounts query wrapped in the account path. + +- [ ] **Step 4: Run.** `ctest --test-dir build -R config` passes. +- [ ] **Step 5: Filter icon.** In `src/mainwindow.cpp` `filterIcons` (line ~3115), add `{ QStringLiteral("spam"), QStringLiteral("mail-mark-junk") },`. +- [ ] **Step 6: Commit.** `git add src/config.cpp src/mainwindow.cpp tests/test_config.cpp && git commit -S -m "feat(config): a threaded, path-based spam filter"` + +--- + +### Task 3: Rename the origin tag `deleted-from:` to `moved-from:` + +**Files:** +- Modify: `src/types.h` (shared prefix constant) +- Modify: `src/mainwindow.cpp` (composer + three readers) +- Modify: `src/notmuchworker.cpp` (`applyTags()` overwrite rule) +- Modify: `tests/test_mainwindow.cpp`, `tests/test_tagdialog.cpp` (all literals and comments) + +**Interfaces:** +- Produces: `kOriginTagPrefix` (in `types.h`) used by both `MainWindow` and `NotmuchWorker`. + +- [ ] **Step 1: Centralise the prefix** in `src/types.h` (after the includes): + +```cpp +/// Prefix of the origin tag a move writes. One origin tag per message, +/// overwritten on each move, so a reader cannot be handed two and forced to +/// pick one silently. Not translated, not user-facing. +inline constexpr auto kOriginTagPrefix = "moved-from:"; +``` + +- [ ] **Step 2: Composer.** In `MainWindow::originTagFor()` (`src/mainwindow.cpp:6384`), change the return to `return QString(kOriginTagPrefix) + accountRelative;` and update its doc prose from `deleted-from:` to `moved-from:`. + +- [ ] **Step 3: Readers.** In `src/mainwindow.cpp`, replace the three `const QString prefix = QStringLiteral("deleted-from:");` literals (lines ~6480, ~6594, ~6902) with `const QString prefix = QString::fromLatin1(kOriginTagPrefix);`. Update the surrounding comments' prose the same way. + +- [ ] **Step 4: Worker overwrite rule.** In `NotmuchWorker::applyTags()` (`src/notmuchworker.cpp:1060`), after `const QStringList before = tagsOf(message.get());` (line ~1123) and before the add/remove loop, insert: + +```cpp + // One origin tag ever: writing a `moved-from:` tag strips any other + // tag with that prefix the message still carries, so a message that + // travelled inbox -> spam -> trash ends with exactly one origin and + // Restore has one answer. Without this the reader's first-match + // break() picks silently. + const bool writingOrigin = + std::any_of(change.added.cbegin(), change.added.cend(), + [](const QString &t) { + return t.startsWith(QLatin1String(kOriginTagPrefix)); + }); + if (writingOrigin) { + for (const QString &tag : std::as_const(before)) { + if (tag.startsWith(QLatin1String(kOriginTagPrefix)) + && !change.added.contains(tag)) { + moves = moves + || notmuch_message_remove_tag( + message.get(), tag.toUtf8().constData()); + } + } + } +``` + + Ensure `#include <algorithm>` and `<utility>` (for `std::as_const`) are present in `notmuchworker.cpp`; add if missing. Include `types.h` if not already included. + +- [ ] **Step 5: Update tests.** In `tests/test_mainwindow.cpp` and `tests/test_tagdialog.cpp`, replace every `deleted-from:` literal and query string with `moved-from:` (`deleted-from:inbox` -> `moved-from:inbox`, `deleted-from:Trash` -> `moved-from:Trash`, `deleted-from:Inbox/SlackBuilds users` likewise). Update the surrounding comment prose. Use `grep -rn "deleted-from"` to find all sites; there must be none left anywhere except the spec/plan documents. + +- [ ] **Step 6: Run.** `ctest --test-dir build -R 'mainwindow|tagdialog|notmuchworker'` passes. +- [ ] **Step 7: Commit.** `git add -A && git commit -S -m "refactor: rename the origin tag to moved-from: with overwrite semantics"` + +--- + +### Task 4: Mark spam moves the file + +**Files:** +- Modify: `src/mainwindow.h` (three method decls) +- Modify: `src/mainwindow.cpp` (action body, `spamSelected()`, `spamMessages()`, `spamThreads()`, `onThreadMessagesResolved()` branch) +- Test: `tests/test_notmuchworker.cpp`, `tests/test_mainwindow.cpp` + +**Interfaces:** +- Consumes: `Account::spam`, `kOriginTagPlaceholder()`, `sendMove()`, `everySelectedRowIsInATrashFolder()`. +- Produces: `MainWindow::spamSelected()`, `spamMessages(const QStringList &, const QHash<QString,QString> &, int, const QStringList & = {})`, `spamThreads(const QStringList &)`. + +- [ ] **Step 1: Declare** in `src/mainwindow.h` beside the trash methods (near line 1205): + +```cpp + /// Moves each selected row's message to its account's spam folder, tagging + /// it `spam` and recording where it came from. Delete's sibling. + void spamSelected(); + void spamMessages(const QStringList &messageIds, + const QHash<QString, QString> &pathById, + int messageCount, + const QStringList &wholeThreadIds = {}); + void spamThreads(const QStringList &threadIds); +``` + +- [ ] **Step 2: Rewrite the action body** (`src/mainwindow.cpp:1786`): + +```cpp + addAction(QStringLiteral("spam"), tr("Mark &spam"), + tr("Move the selected messages to the spam folder"), [this]() { + spamSelected(); + }); +``` + +- [ ] **Step 3: Implement** the three methods as exact mirrors of `trashSelected()`/`trashThreads()`/`trashMessages()` (publicly at lines 6260–6413), changing only: + - the grouping key: `account.maildir + QLatin1Char('/') + account.spam` (skip and report when `account.spam.isEmpty()`), + - the `sendMove` call: + +```cpp + sendMove(it.value(), it.key(), + { QStringLiteral("spam"), kOriginTagPlaceholder() }, + { QStringLiteral("unread"), QStringLiteral("inbox") }, + tr("Mark spam"), false, wholeThreadIds); +``` + + - `spamThreads()` optimistically repaints `m_model->applyTagChange(threadId, { QStringLiteral("spam") }, { QStringLiteral("inbox") })` and requests `"spam_thread"` via `resolveThreadMessages`. + +- [ ] **Step 4: Handle the resolution.** In `onThreadMessagesResolved()` (the branch block at lines 6449–6475), add beside `delete_thread`: + +```cpp + if (requestTag == QStringLiteral("spam_thread")) { + spamMessages(messageIds, pathById, messageIds.size(), threadScope); + return; + } +``` + +- [ ] **Step 5: Worker-level test** in `tests/test_notmuchworker.cpp` (mirror `moveMessagesRelocatesTheFile` at line 1693). Add `addMovableMessage` for a message in `inbox`, move it to `spam` with `worker.moveMessages({ id }, QStringLiteral("spam"))`, assert the file is under `<maildir>/spam/cur` and gone from the origin. Then apply the tag half with `worker.applyTags(TagChange{ {id}, { QStringLiteral("spam"), QStringLiteral("moved-from:inbox") }, { QStringLiteral("unread"), QStringLiteral("inbox") }, QStringLiteral("Mark spam") })` and assert via a query that the message carries `spam` and `moved-from:inbox` and not `unread`. Add a second case starting from a message already carrying `moved-from:inbox` and being moved with `moved-from:Spam`, asserting exactly one `moved-from:` remains (the overwrite rule). + +- [ ] **Step 6: UI-level tests** in `tests/test_mainwindow.cpp`, using `WorkerBackedWindow` and mirroring the delete-move tests around line 12000 (`QTRY_VERIFY_WITH_TIMEOUT`, never `qWait`): + - mark a thread spam from the inbox; assert the file is at the account's spam folder, carries `spam` + `moved-from:inbox`, and no longer carries `unread`; + - undo it; assert the file returns to its exact original path and both `spam` and `moved-from:inbox` are gone; + - assert `everySelectedRowIsInATrashFolder()` returns false for a message whose path is under `account.spam` (the trash predicate must not answer for spam). + +- [ ] **Step 7: Run.** `ctest --test-dir build -R 'notmuchworker|mainwindow'` passes. +- [ ] **Step 8: Commit.** `git add src/mainwindow.h src/mainwindow.cpp tests/test_notmuchworker.cpp tests/test_mainwindow.cpp && git commit -S -m "feat: mark spam moves mail to the account's spam folder"` + +--- + +### Task 5: Message bar and icon fallback + +**Files:** +- Modify: `src/mainwindow.cpp` (`populateMessageBar()`, icon table and its application loop) +- Test: `tests/test_mainwindow.cpp` + +**Interfaces:** +- Consumes: the `spam` action (Task 4). + +- [ ] **Step 1: Put spam on the bar.** In `populateMessageBar()`, ordinary branch (line ~2405), insert `m_actions.value(QStringLiteral("spam"))` between `archive` and `delete` (a filing act whose destination is hostile, ordered before the destructive one). + +- [ ] **Step 2: Icon fallback.** Change the icon table (line ~2141) so a value carries a primary name and an optional fallback. Simplest shape that keeps the existing lookup: change `QHash<QString, QString>` to `QHash<QString, QPair<QString, QString>>` (primary, fallback; empty fallback means none) and update the application loop (line ~2220) to try `QIcon::fromTheme(primary)` and, when null and fallback is non-empty, `QIcon::fromTheme(fallback)`. Set the spam entry to `{ QStringLiteral("bug"), QStringLiteral("mail-mark-junk") }` and keep every other entry's fallback empty. Update the nearby comments to explain that `bug` is not a freedesktop standard name and the fallback keeps every standard theme rendering a junk icon. + +- [ ] **Step 3: Test.** In `tests/test_mainwindow.cpp`, add `theSpamActionCarriesTheBugIconWithAFallback()`: find the `spam` action, assert its icon is non-null, and assert the icon-table primary name for spam is `bug` with fallback `mail-mark-junk`. `everyActionCarriesAnIcon()` and `noTwoActionsShareAnIcon()` must still pass (they compare the primary name). + +- [ ] **Step 4: Run.** `ctest --test-dir build -R mainwindow` passes. +- [ ] **Step 5: Commit.** `git add src/mainwindow.cpp tests/test_mainwindow.cpp && git commit -S -m "feat: spam on the message bar, with a bug icon and junk fallback"` + +--- + +### Task 6: Empty Spam + +**Files:** +- Modify: `src/keymap.cpp` (`knownActions()`) +- Modify: `src/mainwindow.h` (`emptySpam()`, `isShowingSpam()`) +- Modify: `src/mainwindow.cpp` (action, menu, icon, `emptySpam()`, `onThreadMessagesResolved()` `"empty_spam"` branch, `isShowingSpam()`, refresh gate) +- Test: `tests/test_mainwindow.cpp` + +**Interfaces:** +- Consumes: `Config::allSpamQuery()`, `sendMove()`, `kOriginTagPlaceholder()`, `accountForMessagePath()`. +- Produces: `MainWindow::emptySpam()`, `bool MainWindow::isShowingSpam() const`. + +- [ ] **Step 1: Register the action.** In `src/keymap.cpp` `knownActions()` (line ~25), add `QStringLiteral("empty_spam"),` with a comment that it is Empty Trash's sibling but carries no confirmation because it is a MOVE (undoable). Add NO `defaultBindings()` entry (unbound). In `src/mainwindow.cpp` icon table (line ~2141), add an `empty_spam` entry (use `user-trash`, distinct from `empty_trash`'s `edit-delete-shred`). + +- [ ] **Step 2: The action + menu.** Add the action beside `empty_trash` (line ~1774): + +```cpp + addAction(QStringLiteral("empty_spam"), tr("Empty s&pam..."), + tr("Move every message in the spam folder to the trash"), + [this]() { + emptySpam(); + }); +``` + + Add `messageMenu->addAction(m_actions.value(QStringLiteral("empty_spam")));` beside the `empty_trash` entry (line ~2094). Pick an accelerator that does not collide in the Message menu: `Mark &spam` already owns `&s`, so use none or a free letter; `noMenuHasTwoEntriesSharingAMnemonic()` must stay green. + +- [ ] **Step 3: Implement `emptySpam()`** mirroring `emptyTrash()` (line 6742): resolve `m_accountBox->currentData().toString().isEmpty() ? m_config.allSpamQuery() : m_config.account(key).spamQuery()`; when empty, `showTransientStatus(tr("No spam folder is configured"))` and return (an empty notmuch query matches everything); otherwise `QMetaObject::invokeMethod(m_worker, "resolveQueryMessages", Qt::QueuedConnection, Q_ARG(QString, query), Q_ARG(QString, QStringLiteral("empty_spam")));` + +- [ ] **Step 4: The `"empty_spam"` branch** in `onThreadMessagesResolved()` beside the `empty_trash` branch (line ~6449). It groups per account and moves each group to that account's own trash: + +```cpp + if (requestTag == QStringLiteral("empty_spam")) { + QHash<QString, QStringList> byTrash; + for (int i = 0; i < messageIds.size(); ++i) { + const Account account = accountForMessagePath(paths.at(i)); + if (account.maildir.isEmpty() || account.trash.isEmpty()) + continue; + byTrash[account.maildir + QLatin1Char('/') + account.trash] + .append(messageIds.at(i)); + } + for (auto it = byTrash.cbegin(); it != byTrash.cend(); ++it) { + sendMove(it.value(), it.key(), + { QStringLiteral("deleted"), kOriginTagPlaceholder() }, + { QStringLiteral("spam"), QStringLiteral("unread"), + QStringLiteral("inbox") }, + tr("Empty spam")); + } + return; + } +``` + + (The user confirmed `unread` is stripped alongside `spam` and `inbox`, matching Delete's item 168 precedent. The placeholder resolves per message to `moved-from:<the spam folder it is leaving>`, and Task 3's overwrite rule keeps exactly one origin.) + +- [ ] **Step 5: `isShowingSpam()`** mirroring `isShowingTrash()` (line 3673), comparing `m_lastQuery` against `m_config.allSpamQuery()` and each `account.spamQuery()`. Then change the refresh gate in `onMessagesMoved()` (line ~7365) to `if (isShowingTrash() || isShowingSpam()) refreshCurrentQuery();` so emptying spam while the Spam filter is open drops the rows. + +- [ ] **Step 6: Tests** in `tests/test_mainwindow.cpp` (WorkerBackedWindow): + - *groups per account*: seed two accounts each with mail in its spam folder, Empty Spam, assert each message lands under its OWN account's trash; + - *rewrites the origin*: move inbox -> spam, then Empty Spam, assert exactly one `moved-from:` remains and it names the spam folder (`tag:"moved-from:Spam"` count is 1, `tag:"moved-from:inbox"` is 0); + - *refuses an empty query*: with no spam folder configured, assert no worker round trip and a status message naming the cause. + +- [ ] **Step 7: Run.** `ctest --test-dir build -R 'keymap|mainwindow'` passes. +- [ ] **Step 8: Commit.** `git add src/keymap.cpp src/mainwindow.h src/mainwindow.cpp tests/test_mainwindow.cpp && git commit -S -m "feat: empty spam moves mail to the trash, per account"` + +--- + +### Task 7: Stranded spam cleanup + +**Files:** +- Modify: `src/keymap.cpp` (`knownActions()`) +- Modify: `src/mainwindow.h` (`showStrandedSpamMail()`) +- Modify: `src/mainwindow.cpp` (action, menu, icon, `showStrandedSpamMail()`) +- Test: `tests/test_mainwindow.cpp` + +**Interfaces:** +- Consumes: `Config::allSpamQuery()`, `runQuery()`, `m_queryEdit`, `m_statusLabel`. +- Produces: `MainWindow::showStrandedSpamMail()`. + +- [ ] **Step 1: Register.** In `knownActions()` add `QStringLiteral("cleanup_stranded_spam"),` (no default binding). Icon table: `system-search`. Menu: beside `cleanup_stranded` (line ~2093). Choose an accelerator that does not collide in the Message menu. + +- [ ] **Step 2: The action** calling `showStrandedSpamMail()`, label `tr("Find stranded s&pam")` (or another free accelerator), tip `tr("Show mail tagged spam that is not in a spam folder")`. + +- [ ] **Step 3: Implement `showStrandedSpamMail()`** mirroring `showStrandedDeletedMail()` (line 6852), copying both load-bearing details: + +```cpp +void MainWindow::showStrandedSpamMail() +{ + const QString spam = m_config.allSpamQuery(); + // Never written as `not ()`: notmuch parses that happily and matches + // nothing, reporting a clean database. + const QString query = + spam.isEmpty() + ? QStringLiteral("tag:spam") + : QStringLiteral("tag:spam and not (%1)").arg(spam); + + m_queryEdit->setText(query); + runQuery(FlatResult::No, AccountScope::AlreadyScoped); + + m_statusLabel->setText(tr("Mail tagged spam but not in a spam folder. " + "Select what should go and press Mark spam.")); +} +``` + + (`AlreadyScoped` so the account dropdown does not hide another account's stranded mail, and the status is set after `runQuery()`, which overwrites it.) + +- [ ] **Step 4: Tests** in `tests/test_mainwindow.cpp`: assert the composed query excludes `path:"work/Spam/**"` when a spam folder is configured, and is exactly `tag:spam` when none is. Mirror the existing stranded-deleted test. + +- [ ] **Step 5: Run.** `ctest --test-dir build -R 'keymap|mainwindow'` passes. +- [ ] **Step 6: Commit.** `git add src/keymap.cpp src/mainwindow.h src/mainwindow.cpp tests/test_mainwindow.cpp && git commit -S -m "feat: find stranded spam mail"` + +--- + +### Task 8: Translations, changelog, README + +**Files:** +- Modify: `translations/qtmaildir_it_IT.ts` (and the generated `.qm`) +- Modify: the changelog's `[Unreleased]` section, `README.md` if it documents the config keys. + +- [ ] **Step 1:** Refresh the catalogue: + +```bash +lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations none +``` + + Expect zero context warnings (`tr() cannot be called without context`). + +- [ ] **Step 2:** Translate the new strings in the `.ts`: the `Spam` filter label, `Empty spam...`, the stranded-spam status, the spam-shortcut/tooltip strings, and the missing-`spam`-key warning. + +- [ ] **Step 3:** `lrelease translations/qtmaildir_it_IT.ts` reports 0 unfinished (an unfinished string is silently dropped and ships as English). + +- [ ] **Step 4:** `ctest --test-dir build -R translations` passes. + +- [ ] **Step 5:** Add the changelog and an `### Upgrading` note: every account now needs a `spam` key beside `trash`; the origin tag is renamed `deleted-from:` -> `moved-from:`. Note that the one-time notmuch tag rename on live mail is the user's own step, not part of the code change. + +- [ ] **Step 6: Commit.** `git add translations/ CHANGELOG.md README.md && git commit -S -m "docs(i18n): translate the spam strings"` + +--- + +### Task 9: Not spam + +**Files:** +- Modify: `src/keymap.cpp` (`knownActions()`) +- Modify: `src/mainwindow.h` (`notSpamSelected()`, `notSpamMessages()`, `notSpamThreads()`, `everySelectedRowIsInASpamFolder()`) +- Modify: `src/mainwindow.cpp` (action, icon table, both menus, message-bar spam branch, gating, the three methods, the `not_spam_*` resolution branches) +- Modify: `tests/test_mainwindow.cpp` +- Modify: `CHANGELOG.md`, `README.md`, `translations/qtmaildir_it_IT.ts` + +**Interfaces:** +- Consumes: `Account::spam`, `accountForMessagePath()`, `originTagFor()`, `sendMove()`, the `resolveMessages`/`resolveThreadMessages` worker calls, `kOriginTagPrefix`, `m_replySelectionHidesDelete`. +- Produces: `MainWindow::notSpamSelected()`, `notSpamMessages(const QStringList &, const QStringList &, const QStringList &)`, `notSpamThreads(const QStringList &)`, `bool MainWindow::everySelectedRowIsInASpamFolder() const`. + +**Behaviour (approved 2026-09-14).** Shown when the selection is in a spam folder, hidden on reply rows and in the trash, like Delete/Restore. Moves each message back to the folder its `moved-from:` tag names, strips `spam` + that origin, adds `inbox` when the destination is the account's inbox. A message with NO origin (provider-caught) falls back to the account's inbox and is reported in the status bar, exactly as `restoreResolvedMessages()` already does for trash. A conversation row acts on the whole conversation, a message row on that message. Undoable, no confirmation, no default shortcut. + +- [x] **Step 1: Register.** Add `QStringLiteral("not_spam")` to `KeyMap::knownActions()`, no `defaultBindings()` entry. Icon table: `{ QStringLiteral("not_spam"), { QStringLiteral("mail-mark-notjunk"), QString() } }` (`mail-mark-notjunk` ships in Breeze and Adwaita and is unused in the table, so no icon exception is needed). +- [x] **Step 2: Action + menus.** `addAction(QStringLiteral("not_spam"), tr("Not s&pam"), ...)` with tip `tr("Move the selected messages out of the spam folder")`; choose a mnemonic free in the Message menu (`&p` is `Re&ply`, `&s` is `Mark &spam`; `noMenuHasTwoEntriesSharingAMnemonic()` must stay green). Add the action to `messageMenu` and `m_threadContextMenu`. +- [x] **Step 3: Predicate + gating.** Add `bool MainWindow::everySelectedRowIsInASpamFolder() const` mirroring `everySelectedRowIsInATrashFolder()` but comparing against `account.spam`. In `refreshTrashActions()` compute `const bool inSpam = everySelectedRowIsInASpamFolder();` and set `not_spam` visible with `haveSelection && inSpam && !m_replySelectionHidesDelete`, so it is hidden on a reply row and everywhere outside the spam folder. +- [x] **Step 4: Message bar.** In `populateMessageBar()`, add a branch keyed on `everySelectedRowIsInASpamFolder() && !selection empty`, between the trash branch and the draft branch, whose list is `{ not_spam }`. +- [x] **Step 5: The three methods.** `notSpamSelected()` mirrors `restoreSelectedFromTrash()` but resolves through the worker with request tag `"not_spam_messages"`; `notSpamThreads()` mirrors `untrashThreads()` with `"not_spam_thread"`. Handle both in `onThreadMessagesResolved()` beside `"restore_messages"` and `"undelete_thread"`, clearing `spam` instead of `deleted`. **Prefer parameterising the existing `restoreResolvedMessages()` and the `undelete_thread` branch with the cleared tag and the undo description over copying them**, so the two scopes cannot drift; if you copy instead, say why. +- [x] **Step 6: Tests** in `tests/test_mainwindow.cpp`, `WorkerBackedWindow`, `QTRY_VERIFY_WITH_TIMEOUT`, assertions against the database: + - mark a message spam, then Not spam: the file returns to its exact original folder, `spam` and `moved-from:` are gone, and `inbox` is back when the origin is the inbox; + - a provider-caught message (in the spam folder, no `moved-from:`) goes to the account's inbox and the status reports that it had no origin; + - visibility: offered in the spam view, hidden on a reply row and in the trash view; + - undo restores it. +- [x] **Step 7: Docs + i18n.** Add a clause to the changelog's `[Unreleased]` Added entry, mention the action in `README.md`, run `lupdate-qt6`/`lrelease-qt6`, translate the new strings, and keep `ctest -R translations` green. +- [x] **Step 8: Run + commit.** `ctest --test-dir build -R 'keymap|mainwindow|translations'`, then `git commit -S -m "feat: a Not spam action"`. + +--- + +## Self-review + +- **Spec coverage:** config key + queries (Task 1), the Spam filter (Task 2), origin rename (Task 3), the move action (Task 4), message bar + icon (Task 5), Empty Spam (Task 6), stranded cleanup (Task 7), translations/docs (Task 8), Not spam (Task 9, added 2026-09-14 from backlog item 201 after the branch's final review). The trash-predicate test is in Task 4 Step 6. Item 196 (abusectl auto-tagging) and item 197's provider-notification half are out of scope by the spec; item 197's destination question is settled as the inbox fallback in Task 9. +- **Type consistency:** `spamQuery`/`allSpamQuery`/`isShowingSpam`/`spamSelected`/`spamMessages`/`spamThreads`/`emptySpam`/`showStrandedSpamMail`/`notSpamSelected`/`notSpamMessages`/`notSpamThreads`/`everySelectedRowIsInASpamFolder`/`kOriginTagPrefix` are used with one spelling throughout. +- **Decided:** Empty Spam removes `spam`, `unread` and `inbox` (user confirmed `unread`), and adds `deleted` + the origin placeholder. It carries no confirmation dialog and no default shortcut; it is a move and is undoable. Not spam falls back to the account's inbox for provider-caught mail, at the user's decision on 2026-09-14. +- **Known blast radius:** Task 3 renames a string literal in ~35 test sites; `grep -rn "deleted-from" src tests` must be empty afterwards. diff --git a/docs/superpowers/specs/2026-09-13-cli-selectors-design.md b/docs/superpowers/specs/2026-09-13-cli-selectors-design.md new file mode 100644 index 0000000..88cb381 --- /dev/null +++ b/docs/superpowers/specs/2026-09-13-cli-selectors-design.md @@ -0,0 +1,253 @@ +# Launching qtmaildir at an account, thread or message + +Date: 2026-09-13 +Status: approved, not yet implemented +Backlog: item 200 + +## Problem + +`qtmaildir` accepts no arguments beyond `--version` and `--help`. Another +program that knows which message it cares about, a notification, a script, a +sidecar like item 194's, has no way to say so: it can launch the client and +that is all. + +The user's note asks for `--account`, `--thread` and `--message` "so that +another app can launch qtmaildir opening that account's inbox or a certain +message/thread". + +## Verified context + +Established by reading the code on 2026-09-13, not assumed. + +- **Argument handling is a `strcmp` loop.** `main.cpp:38-66` walks `argv` for + `--version`/`-v` and `--help`/`-h`. Both answer and `return` BEFORE + `QApplication` is constructed, which is deliberate and documented there: + `--version` must work on a machine where the GUI cannot open. +- **There is no single-instance mechanism.** No `QLocalServer` or + `QLocalSocket` appears anywhere in `src/`. Two launches are two processes + against one notmuch database, and notmuch permits one open handle per + process. +- **`Qt6::Network` is not linked.** `CMakeLists.txt:20` sets the component list + to `Widgets Svg WebEngineWidgets` (plus `Test` for the suite), and + `src/CMakeLists.txt:58` links exactly those. A socket adds a component, which + is a packaging fact for the SlackBuild in `my-slackbuilds`, not just a CMake + line. +- **The startup path already composes an account with a query**, at + `mainwindow.cpp:624-657`, including the distinction this design must not + break: a generated filter comes back from `resolvedQuery()` ALREADY scoped and + takes `AccountScope::AlreadyScoped`, while a saved query states its own scope + and takes `AccountScope::Apply`. Getting it wrong is silent in both + directions, double-scoping one and leaving the other unscoped. +- **`recoverStaleThread(threadId, messageId)` already does the whole of what + `--thread` and `--message` need**, at `mainwindow.cpp:5002`. It runs + `thread:<id>`, remembers the target across the two queued round trips the + load takes, expands the thread when its row arrives + (`applyPendingRecovery()`, `mainwindow.cpp:5148`, called from the + query-complete handler at `mainwindow.cpp:3670`), selects the message once + the replies land, and falls + back to the root row when the message has gone. Item 91's double-click reuses + it outright and says so in a comment. **A CLI selector is a third caller.** +- **`announceAction()` / `showTransientStatus()`** (`mainwindow.cpp:5403`) is + how the window says something happened, with a timer that falls back to the + query's own result line. + +## Decisions + +Taken by the user on 2026-09-13. + +1. **A second launch steers the running window.** The second process hands its + arguments over a socket and exits; the running window applies them and + raises itself. This is what makes the feature useful to the caller the note + describes, which will usually find qtmaildir already running, and it fixes + the two-notmuch-handles problem that exists today as a side effect. +2. **Three selectors: `--account`, `--thread`, `--message`.** `--query` was + considered and dropped: it is the most general and the cheapest, and it is + also the one with no caller. The query bar already accepts an arbitrary + query from the only party who would type one. +3. **A selector that matches nothing opens the window normally and says so in + the status bar.** Not an empty result, which makes a stale link look like a + broken client; not a refusal to start, which is right for a script and wrong + for a desktop launch where the user gets no window and no reason. + +## Design + +### The command line + +`QCommandLineParser` replaces the `strcmp` loop, with one constraint that is +not negotiable: **`--version` and `--help` must keep answering without a +`QApplication`.** `QCommandLineParser` itself needs only `QCoreApplication`'s +argument list, which `QCommandLineParser::process(QStringList)` accepts, so the +parse can happen against `argv` before the GUI object exists. The two +early-exit options keep their current place and their current behaviour; the +parser gains the three selectors and nothing else. + +``` +qtmaildir [--account KEY] [--thread ID] [--message ID] +``` + +The three compose. `--account work --message '<abc@example.org>'` means what it +says, and the account half is applied before the query runs, exactly as +`startup_account` is. + +### Single instance + +A `QLocalServer` named under `QStandardPaths::GenericStateLocation`, beside the +UI state file, reached through a helper alongside `MainWindow::uiStatePath()` +so the two paths cannot drift. `GenericStateLocation`, not `StateLocation`, for +the reason already recorded: the latter appends both the organization and the +application name and both are `qtmaildir`. + +Startup order, and the order is the design: + +1. Parse the arguments. `--version`/`--help` answer and exit here, as now. +2. Try to CONNECT to the socket. If a connection succeeds, write the selectors + as one payload, wait briefly for the write to flush, and exit 0. This + process never constructs a `MainWindow` and never opens notmuch. +3. If the connection fails, this process becomes the server: listen, then build + the window and apply the selectors locally. + +**A stale socket file is the failure mode to handle, not to hope about.** A +crash or a kill leaves the socket file behind, and `listen()` then fails with +`AddressInUse` on a file nothing is serving. The recovery is to attempt a +connection first, which step 2 already does: a refused connection on an +existing file proves it is stale, and `QLocalServer::removeServer()` clears it +before listening. Doing it in this order means a live instance is never +removed out from under itself. + +**If the socket cannot be created at all, start anyway.** A read-only state +directory or a platform without local sockets must degrade to today's +behaviour, a window that opens and works, rather than to no mail client. Log +it; do not fail. + +### Applying the selectors + +One entry point on `MainWindow`, taking the three values, called from two +places: the local path at startup, and the socket handler when a second launch +arrives. The two must go through the same function or they will drift, which is +the lesson every other pair of paths in this file has already taught. + +- **`--account KEY`** sets the account dropdown, through the same + `findData()`/`setCurrentIndex()` the startup account uses. `Config` already + validates the key against the configured accounts and clears an invalid one, + so the miss is detectable rather than silent. +- **`--thread ID`** calls `recoverStaleThread(id, QString())`. The empty + message id is already meaningful to that function: land on the root row, + which IS the thread's first message. +- **`--message ID`** resolves the message's thread and calls + `recoverStaleThread(threadId, messageId)`. The thread is what the user wants + to see, with that message selected: an `id:` query on the message alone shows + one card out of its conversation, which item 91 settled is the wrong reading + of "open this message". + +**Resolving a Message-ID to a thread id is the one piece that does not exist.** +It is a worker question, since only the worker touches notmuch, and +`NotmuchWorker` already runs `id:"<...>"` at `notmuchworker.cpp:985`. It is one +more query with a queued reply, and the reply is what calls +`recoverStaleThread()`. Nothing about it is novel; it is listed here because it +is the only new round trip in the design. + +**Every id goes through `SearchTerm::quote()`**, like every other query this +application builds. A Message-ID comes from outside the process, so it is +untrusted input in the ordinary sense. Note the existing code at +`mainwindow.cpp:5011` and `:6798` interpolates ids unquoted because they came +from notmuch itself; an id from `argv` did not, and the difference is the whole +reason `SearchTerm` exists. + +### Raising the window + +`show()`, `raise()` and `activateWindow()`, plus `setWindowState()` clearing +`Qt::WindowMinimized` so a minimized window comes back. **Under Wayland this is +a request, not a command**, and Hyprland may honour it as a focus hint or +ignore it by policy. That is the compositor's decision and not a defect to +chase: the selectors still apply and the window still shows the right thing, +which is the part that must work. Do not add workarounds for focus stealing +prevention. + +### When a selector matches nothing + +The window opens on its configured startup view and +`showTransientStatus()` names what missed: + +- an account key naming no configured account, +- a thread id matching no thread, +- a Message-ID matching no message. + +One message per miss, naming the value, so a caller passing a stale id can be +debugged from the client rather than from the caller. The account case is +detectable before any query runs; the other two are known when the query comes +back empty. + +**A second launch that matches nothing must still raise the window.** The user +asked for it and something went wrong with the selector; showing them nothing +at all is the worst of both. + +## Testing + +What has a right answer and is worth building, per `AGENTS.md`. + +- **The parse**: each selector, the three composed, the two early-exit options + still answering, and an unknown option not crashing. Pure over `QStringList`, + so it is testable without a window. +- **The quoting**: a Message-ID containing a quote, a backslash, whitespace or + a `)` produces the query `SearchTerm` promises, not a malformed one. This is + the security-relevant assertion and notmuch cannot check it for us, since it + parses garbage happily and matches zero (`searchterm.h:30-35`). +- **Message-ID to thread id**, in `test_notmuchworker`, against the throwaway + database: a known id resolves to its thread, an unknown one resolves to + empty. +- **Applying the selectors**, in `test_mainwindow` with `WorkerBackedWindow`: an + account key moves the dropdown, a thread id lands on that thread's row, a + message id lands on that message's row inside its thread. Wait on observable + state with `QTRY_VERIFY_WITH_TIMEOUT`, never on a fixed `qWait`, and remember + `rowCount()` on a thread row is 0 until expanded. +- **The miss path**: an unknown account key leaves the dropdown on its + configured value and the window on its startup query rather than on nothing. +- **Stale socket recovery**: a socket file with nothing behind it does not stop + the window from starting. Testable against a `QTemporaryDir` state path. + +**Not tested, handed to the user to look at**: whether the window actually +raises and takes focus under Hyprland. It is compositor policy, the offscreen +platform cannot see it, and a test there would pass whatever the code does. + +## Constraints + +- **`Qt6::Network` joins the component list**, in the root `CMakeLists.txt` and + in `src/CMakeLists.txt`. It is already an installed part of Slackware's + monolithic `qt6` package, so the SlackBuild's dependencies do not change, but + the `.SlackBuild` lives in `my-slackbuilds` and the addition should be + verified there rather than assumed here. +- **No network protocol work.** A `QLocalServer` is a unix domain socket + between two copies of this program, not a network client. The rule + `AGENTS.md` states is about IMAP and SMTP; this does not touch it. +- **The socket is a trust boundary, and a small one.** It is owned by the user + and lives in their own state directory, so the payload comes from a process + running as them. That is not a reason to skip validation: the payload is + parsed as three optional strings, anything else is ignored, and every id + still goes through `SearchTerm::quote()` on the way to a query. A payload + size cap belongs here too, since a local socket will hand over whatever it is + given. +- **Five places for an action still applies if any of this gains one.** As + designed it does not: the selectors are startup input, not user-triggered + actions, and nothing appears in a menu. If a "raise" action is ever added it + takes the full treatment in `KeyMap::knownActions()`, the icon table and a + menu. +- **Every user-facing string is translatable**, including the status-bar misses + and the `--help` text. The option NAMES are wire format and are never + translated; their descriptions are prose and are. +- **`--help` gains three lines** and must stay accurate, since it is the only + documentation a caller will read. The README's usage section gains the same. + +## Sizing + +**M**, and the halves are uneven. + +The selectors are the small half, because `recoverStaleThread()` already exists +and is already proven by two callers. The single-instance socket is the real +work: the connect-first ordering, stale socket recovery, the degrade path when +no socket is possible, and one new worker round trip for the Message-ID lookup. + +They could ship separately, the parser and startup selectors first and the +socket after, and the spec is written so that split is available. It is not +recommended: the selectors without the socket answer the note's actual use case +("another app can launch qtmaildir") with a second window, which is the +behaviour the user rejected. |