diff options
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 100 |
1 files changed, 100 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index e7ef7db..4a4b1ff 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 @@ -5040,6 +5040,106 @@ when the write is moved above the guard. The cold-cache cost measured above was not touched and should not be. Nothing about the timing changed. +## 124. The worker reads the index directory as the mail root + +**Observed (measured, 2026-08-20):** not reported from use. Found while +measuring item 121's cold-start cost, which established that the 1.1 GB notmuch +index sits on a 7200rpm platter (`/dev/sda1`, `rotational: 1`) while an NVMe SSD +sits idle in the same machine. Moving the index to the SSD is a notmuch +configuration change and needs no code, but qtmaildir does not survive it. + +**Cause (verified against notmuch 0.39 and a throwaway database).** notmuch +supports splitting the index from the mail with two keys: + +```ini +[database] +mail_root=/data/Mail +path=/home/you/.local/share/notmuch +``` + +Under that layout `notmuch_database_get_path()` returns the **index** +directory, not the mail root. Measured on a split test database: + +| accessor | legacy (`path` only) | split | +|---|---|---| +| `notmuch_database_get_path()` | mail root | **index dir** | +| `notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT)` | mail root | mail root | + +`notmuchworker.cpp` calls `get_path()` at six sites and treats every one as the +mail root: lines 297, 755, 900, 901, 1062, and the `relativeFilePath()` calls +at 364, 380 and 918 that consume them. + +**One of those six moves mail, and it is the dangerous one.** +`moveMessages` composes its destination as +`root + "/" + destFolder + "/cur"` (`src/notmuchworker.cpp:755`). Under a split +config `root` is the index directory, so Delete would move the message into +`<index>/Trash/cur`: outside the Maildir, invisible to mbsync, and gone from +every other client. That is item 103's stranded-mail failure with a new cause, +and this repo has already shipped that class of bug once. + +The rest degrade rather than destroy. `relativeFilePath()` against the wrong +root yields `../../../data/Mail/account/cur/...` instead of `account/cur/...`, +so no path matches an account prefix and every row resolves to no account, +which is exactly what the comment at line 294 already warns about. + +**Approach.** Replace `notmuch_database_get_path()` with +`notmuch_config_get(m_db, NOTMUCH_CONFIG_MAIL_ROOT)` at the six sites. + +**No conditional is needed, and that is the point.** `MAIL_ROOT` returns the +mail root under BOTH layouts, verified above: under a legacy `path`-only config +it equals `get_path()`, so the change is a no-op against the current +configuration and correct against the split one. A fallback to `get_path()` when +`MAIL_ROOT` is NULL is the tempting belt-and-braces addition and should be +resisted unless a NULL is actually observed, since it reintroduces the wrong +answer on the path where it matters. + +**Constraints.** + +- **`moveMessages` is the site to test hardest.** A wrong root there reaches the + mail server, per the "Delete MOVES the file" note in `CLAUDE.md`. +- The test fixture must build a database whose index is NOT inside the mail + root, or it cannot tell the two accessors apart: under the ordinary fixture + layout both return the same string and a mutation stays green. +- `notmuch_config_get` returns a string owned by notmuch (`notmuch.h:2585`); + do not free it. +- Nothing about the legacy layout may change. The migration is the user's to + perform, separately, once this ships. + +**Size: S.** Six call sites and a fixture that can tell them apart. + +**Closed 2026-08-20, unreleased.** `mailRootOf()` in the anonymous namespace of +`notmuchworker.cpp` wraps `notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT)`, and +the four `notmuch_database_get_path()` call sites use it. No conditional: the +accessor is correct under both layouts, verified against the live database where +it returned the same string as `get_path()` before the migration. + +Three tests in `test_notmuchworker.cpp`, all requiring the fixture's new opt-in +`splitIndex()`. That flag is load-bearing rather than convenient: in the +ordinary layout the index sits inside the mail root and both accessors return +the same string, so a test written against it passes whichever one the code +uses. All three fail against the old accessor, confirmed by mutation, with the +move test failing outright rather than subtly. + +**Proven in production the same day.** The developer's own index moved from a +7200rpm platter to NVMe, mail staying at `/data/Mail`: + +| phase | platter, cold | NVMe, cold | +|---|---|---| +| `search_threads` | 673 ms | 12 ms | +| first 200 rows | 2008 ms | 50 ms | +| complete walk, 4634 threads | 38618 ms | 668 ms | + +Counts held at 49174 messages / 5594 inbox / 100 tags at every step, and Delete +then Restore round-tripped through `[Gmail]/Cestino` by hand, which is the +behaviour this item existed to protect. + +**Two things the migration taught that are not in the code.** `database.hook_dir` +defaults to `<database.path>/.notmuch/hooks`, so a split config silently stops +running `post-new`: `notmuch new` reports success and tags nothing. It must be +set explicitly. And the procedure's own first step, holding `/tmp/mbsync.lock`, +conflicts with hand-testing Delete, which auto-syncs; that surfaced item 125. +The procedure lives outside this repo, in the user's own documents. + ## 90. A saved-query button clears the account selection **Observed (user, notes):** "select an account and hit the 'unread' button, the |