docs: spec the saved-query file, and reduce item 23 to a pointer

Item 23 had grown past what a backlog entry should hold: a storage format, a
migration, a dialog and a layout change. This document's own rule says a fully
specified item moves to specs/ and leaves behind the two or three things that
decide whether it can be picked up, the way items 53, 63 and 76 went. The entry
now carries the observation, the three deciding constraints and the relation to
item 10, and points at the spec for the rest.

The spec pins what was still loose. The JSON is an ordered array, since the
ordering is the whole reason for moving off [queries], and nothing may sort it
on load. A query's account scope stores the account KEY, the INI group suffix,
rather than the maildir path, so it does not duplicate config that already lives
in one place and go stale when the user edits it; the scope then composes
through Account::scopedQuery(), whose parenthesisation is load-bearing for the
same reason it is in the rules hook, an unparenthesised disjunction escapes its
scope and matches every account.

Migrated entries are pinned, so the query row does not silently empty on the
first run after upgrade, and migration order is alphabetical because that is
genuinely all the INI knows. Sent stays out of the file: it is generated from
allSentQuery() rather than stored, and folding it in would mean writing a
per-account path query into stored config, which is the duplication the
account-key decision just rejected.

The testing section is written against the traps already recorded in CLAUDE.md.
The migration test asserts the INI file is byte-identical rather than re-reading
it through QSettings, which would pass against a rewrite that preserved values
while dropping comments; the round-trip test asserts order, which is the
property the INI could not provide; and the dialog is left to a hand test,
because the offscreen platform cannot assert sizing at all and a Cancel goes
through done(int) rather than closeEvent.

Every code reference in the spec was checked against the files rather than
copied from the entry.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

1 files changed, 200 insertions, 0 deletions

diff --git a/docs/superpowers/specs/2026-08-13-saved-queries-design.md b/docs/superpowers/specs/2026-08-13-saved-queries-design.md
new file mode 100644
index 0000000..6d0019f
--- /dev/null
+++ b/docs/superpowers/specs/2026-08-13-saved-queries-design.md
@@ -0,0 +1,200 @@
+# Saved queries in a file of their own: design
+
+Backlog item 23, "No way to save a search query from the UI", and item 81, "No
+way to turn a saved query into a tagging rule", which depends on it.
+
+**Status:** design approved 2026-08-13, not implemented.
+
+## The problem this solves
+
+A saved query can only be created by hand-editing `qtmaildir.conf`. There is no
+way to keep a query you have just written and are looking at the results of,
+which is exactly the moment you know it is worth keeping.
+
+Underneath that sits a second problem the user named separately: every saved
+query becomes a button in the query row (`src/mainwindow.cpp:557`), so the row
+grows without bound and cannot distinguish "Inbox", which belongs on screen
+permanently, from "all messages from one correspondent", which belongs in a
+menu.
+
+## Why the storage moves, and what that decides
+
+The obvious implementation writes back into `[queries]`. It was rejected, and
+the reason shapes the rest of this document.
+
+**`[queries]` cannot express order.** It is read through
+`QSettings::childKeys()`, which returns keys alphabetically and never in file
+order. `src/config.cpp:401` already carries a comment saying a hand-rolled
+parser would be needed to change that, and the README documents the alphabetical
+button order as though it were a feature. A row of pinned buttons that cannot be
+arranged is not worth building.
+
+**QSettings would also destroy the file it wrote to.** It preserves neither
+comments nor key order, so the first save from the dialog reformats a
+hand-edited config wholesale. Item 1 established that machine-written state stays
+out of `qtmaildir.conf` for precisely this reason.
+
+So saved queries move to **`~/.config/qtmaildir/queries.json`**, and gain three
+things the INI could not hold: an order, a `pinned` flag, and a per-query account
+scope.
+
+**In `~/.config/`, not `~/.local/state/`.** A named, scoped query is user intent,
+hand-editable, and belongs in a config backup; a state directory is commonly
+excluded from one. `uistate.conf` keeps window blobs and column widths, which is
+a different kind of thing.
+
+### The rules.json analogy, and where it stops
+
+The user proposed this by analogy with `~/.config/mailrules/rules.json`. The
+analogy is right about the shape and wrong about the machinery, and the
+difference is worth stating because copying too much of it would be a real cost.
+
+`rules.json` carries unknown-field preservation and a `kFormatVersion` handshake
+because it has **two independent implementations** that agree by test rather than
+by shared code: `src/tagrules.cpp` here and `mailrules.py` in `../mailctl`. All
+of that exists to stop two programs destroying each other's writes.
+
+`queries.json` has exactly one reader. What carries across:
+
+- a versioned document, so a future breaking change has a way to announce itself
+- unknown fields preserved through a load/save round trip, so a field added by a
+  later build is not stripped by an older one
+
+What does not carry across: any notion of a second implementation, a shared
+contract, or a two-repo change. This is a single-repo change and must stay one.
+
+## The format
+
+```json
+{
+  "version": 1,
+  "queries": [
+    { "name": "Inbox",  "query": "tag:inbox",   "pinned": true },
+    { "name": "Unread", "query": "tag:unread",  "pinned": true },
+    { "name": "Work invoices", "query": "from:billing", "account": "work" }
+  ]
+}
+```
+
+`queries` is an **array**, and its order is the display order. That is the whole
+point of the change; nothing may sort it on load.
+
+| field | required | meaning |
+|---|---|---|
+| `name` | yes | Display name, and what `startup_query` matches against |
+| `query` | yes | notmuch query, wire format, never translated |
+| `pinned` | no, default `false` | Renders as a button; otherwise it lives in the menu |
+| `account` | no | An account **key**, the INI group suffix (`work` from `[account.work]`), not a path or a display name |
+
+**`account` stores the key, not the maildir path.** `Account::scopedQuery()`
+(`src/config.cpp:42`) composes `path:"<maildir>/**"` from the account, so storing
+the path would duplicate config that already exists in one place and go stale
+when the user edits it. A key naming an account that no longer exists resolves to
+no scope, with a config problem reported the way an unmatched `startup_query`
+already is.
+
+**Scope composes, it does not replace.** A query with an `account` runs as
+`Account::scopedQuery(query)`, which yields `path:"<maildir>/**" and (<query>)`.
+The parenthesisation is load-bearing for the same reason it is in the rules hook:
+`path:... and a or b` binds as `(path:... and a) or b`, so an unparenthesised
+disjunction escapes its scope and matches every account. `scopedQuery` already
+does this correctly; the requirement is to use it rather than concatenating.
+
+## Migration
+
+On a run where `queries.json` does not exist and `[queries]` does: read the INI
+section, write the JSON, and **leave `[queries]` in place, untouched**. From then
+on the JSON is the only source and `[queries]` is ignored entirely.
+
+**Do not strip the section.** Removing it means rewriting the hand-edited file
+with QSettings, which drops comments and key order across the whole file, not
+just the part being removed. That is the loss this design exists to avoid.
+Leaving it costs a few stale lines the user can delete by hand whenever they
+like, and keeps a downgrade to an older build working.
+
+Migrated entries get `pinned: true`, since they are buttons today and the
+migration must not silently empty the query row. Order is alphabetical on
+migration because that is genuinely all the INI knows; the user reorders once, by
+hand or in the dialog, and it sticks from then on.
+
+**Rejected: reading both forever.** Two sources of truth for one thing, with no
+answer for a name colliding across them and no obvious place for the dialog to
+write.
+
+## Consequences for `startup_query`
+
+`[general] startup_query` names a saved query and resolves case-insensitively,
+falling back to the first entry (`Config::startupSavedQuery()`,
+`src/config.cpp:422`). It keeps working unchanged, but **its fallback quietly
+changes meaning**: "the first entry" stops being "alphabetically first" and
+becomes "first in the user's own order".
+
+That is an improvement, and it is still a user-visible behaviour change for a
+config whose `startup_query` matches nothing. It belongs in the changelog, and
+it makes this a minor version bump rather than a patch.
+
+## The UI
+
+### A Save query button beside the search bar
+
+Opens a dialog on the current query bar contents, holding:
+
+- **name**, required, and unique. A name that already exists offers to overwrite
+  rather than silently creating a duplicate the menu would render twice.
+- **query**, editable, prefilled from the query bar.
+- **account scope**, a dropdown of configured accounts plus "All accounts".
+- **pinned**, a checkbox: button or menu.
+
+### The button row moves to a row of its own
+
+Today the query row holds the account box, the sort dropdown, the query bar and
+every saved-query button (`src/mainwindow.cpp:554-587`). Pinned queries move to
+their own row beneath it, and the unpinned ones go into a menu.
+
+**There is no `SavedQueryBar` class**, whatever older revisions of the backlog
+said. These buttons are built inline in `MainWindow` and this is a change to that
+layout code.
+
+Sent stays where it is. It is not a saved query, it is built from
+`allSentQuery()` (`src/mainwindow.cpp:579`), and folding it into `queries.json`
+would mean generating a per-account path query into stored config, which is the
+duplication the `account`-key decision just rejected.
+
+## What is deliberately not here
+
+**Item 81, saving a query as a tagging rule.** A saved query is a view and costs
+nothing if it is wrong. A rule is applied to real mail by the `post-new` hook
+every ten minutes and lives in the file shared with mailctl. Keeping it out means
+item 23 ships as a single-repo change; folding it in would give a presentation
+change a two-repo commitment and a live blast radius.
+
+**Item 10's account persistence.** Per-query account scope answers item 10's
+remaining complaint as a side effect, which is noted in both entries. Item 10
+itself stays postponed and is not reopened here: the user asked that the rest of
+it not be proposed unprompted.
+
+## Testing
+
+`Config` is already unit-tested, and this is mostly a format with an ordering
+guarantee, so most of it is testable without a window.
+
+- Round trip: load, save, load, and assert the document is unchanged **including
+  order**, which is the property the INI could not provide.
+- Unknown fields survive a round trip. Write a field no build knows, load, save,
+  assert it is still there.
+- Migration: an `[queries]` section and no JSON produces a JSON file with every
+  entry `pinned`, and leaves the INI file **byte-identical**. Assert on the file
+  bytes, not on a re-read through QSettings, which would pass against a rewrite
+  that happened to preserve values while dropping comments.
+- A malformed or unreadable `queries.json` reports a config problem and does not
+  take the query row down with it.
+- `startup_query` still resolves by name, and its fallback now returns the first
+  entry in document order.
+- Scope composition yields `path:"<maildir>/**" and (<query>)` with the
+  parentheses present. Assert on the composed string for a query containing
+  `or`, which is the case that breaks without them.
+
+**The dialog itself needs a hand test**, per the offscreen-platform constraints
+already recorded in CLAUDE.md: sizing cannot be asserted there at all, and a
+dialog's Cancel path goes through `done(int)` rather than `closeEvent`, so a test
+that only exercises `close()` proves less than it appears to.