diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-29 11:25:02 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-29 11:25:02 +0200 |
| commit | 3fd999907ae8344f76ee4e5be1ac278a26f452ca (patch) | |
| tree | 17c01d44daee0147ecaca6cccd393471e6379cf1 /docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | |
| parent | 8c78dd139a77e896c72b7fc8b799af3c0df34344 (diff) | |
| download | qtmaildir-3fd999907ae8344f76ee4e5be1ac278a26f452ca.tar.gz qtmaildir-3fd999907ae8344f76ee4e5be1ac278a26f452ca.zip | |
feat: have the sync script report what it did
Item 174, and half of item 125.
The premise was corrected before any code. The note asks for an external
`notmuch new` to clear the pending count; it must not. That count means
tag mutations not yet known to have reached the MAIL STORE, which is the
server: an edit is in notmuch the moment it is made, and what is
outstanding is mbsync pushing the renamed Maildir files. `notmuch new`
re-indexes local files and pushes nothing, so clearing on it would tell
the user their work was safe to quit on while it was still local. The
entry's own proposal to watch notmuch_database_get_revision() was
rejected for the same reason: a revision moves when mail ARRIVES too, and
in neither case does it say anything about the server.
What was actually wrong was the reporting channel. The application
inferred a finished run from an inode in /proc/locks and from grepping
the log for its RUN END banner, which made a human-readable line into
wire format and could not say WHICH channels a run carried. The local
sync path has always narrowed its clear to the accounts it carried; the
external path could not, and cleared everything, so an edit to an
account a run never touched was reported as delivered.
So the script reports instead of leaving evidence to be inferred. It
writes ~/.local/state/qtmaildir/syncstatus.json atomically at the end of
every run, including a skip, naming the channels, both exit statuses and
a state of ok, failed or skipped. MailSync::readStatus() reads it,
MainWindow prefers it over the log banner and narrows the clear through
Account::syncChannel(). A skipped run clears nothing, which is item 125's
first half: the application can now see that a run happened and carried
nothing. The log banner and lastRunOutcome() stay as the fallback for a
missing file, which is what a first run after upgrading looks like.
This is the user's own framing of the scope: the script was written for
another system and adapted, and is now qtmaildir's only consumer, so it
serves the application rather than the reverse. Two facts made it safe to
act on: their crontab runs mailsync.sh and nothing else touches mail, and
~/bin/mailsync.sh is a symlink into this repo, so an edit is live on the
next tick.
Two bugs found while wiring it in, both recorded in the closed item.
A test read the developer's real sync state, twice: a [sync] section
naming only `log` leaves syncStatus() defaulting to the real file, so two
tests asserting that a FAILED run leaves the count alone read the last
real cron run, found ok, and cleared. Pinning only `status` has the
mirror problem. noSyncTestReadsTheRealSyncState() is the guard, modelled
on noTestCanSeeTheRealLockTable().
And Qt::ISODate carries no milliseconds. The status file is preferred
only when it describes THIS run, compared against when the lock appeared,
so a stale success cannot outrank a fresh failure; but the script writes
date -Iseconds, and a round trip of "now" comes back 329 ms behind,
measured. A fast sync's own file therefore parsed as stale and fell back
to the log, with nothing failing to say so. One second of slack matches
the precision the format carries.
Design: docs/superpowers/specs/2026-08-29-sync-status-file-design.md
Suite: 43 of 44, with undoMovesTheMessageBack failing as it does on
master (item 136).
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 113 |
1 files changed, 69 insertions, 44 deletions
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 8edc6bd..2cc3b5c 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 @@ -193,7 +193,7 @@ taking that too literally. | 124 | The worker reads the index directory as the mail root | defect | S | **done** 2026-08-20, unreleased. `mailRootOf()` over `NOTMUCH_CONFIG_MAIL_ROOT`, correct under both layouts. Verified by migrating the developer's own index to NVMe the same day: cold start 38.6 s to 0.67 s | -| 125 | A skipped sync leaves the spinner running for ever | defect | S | open, 2026-08-20, found by hand. `mailsync.sh` exits 75 (EX_TEMPFAIL) when another run holds the lock; the indicator never clears, and a held edit waits for a completion that never comes | +| 125 | A skipped sync leaves the spinner running for ever | defect | S | open, 2026-08-20, **half closed 2026-08-29 by item 174**: a skipped run now writes `state: skipped` to the status file, so the application can see it happened and clears the spinner without clearing the count. What remains is whether `SyncMonitor` should also time out an observation it never saw end, which covers a run that dies without writing anything at all | | 126 | A link with `target="_blank"` does nothing when clicked | defect | S | **done** 2026-08-20, unreleased. `createWindow()` returns a relay page that receives the navigation, hands the URL to the browser and refuses. The URL cannot be read in `createWindow()` itself, which is why a relay rather than a lookup | | 127 | A link's context menu offers four browser actions that cannot work | defect | XS | **done** 2026-08-20, unreleased. Three Open-in actions removed, `CopyLinkToClipboard` kept. Item 126 made them more dangerous rather than less: with a real `createWindow()` they would have started working | @@ -250,13 +250,14 @@ taking that too literally. | 176 | Undoing a thread-scoped action applies its inverse to messages it never changed | defect | S | **done 2026-08-28**, unreleased, on `thread-row-identity`. `NotmuchWorker::applyTags()` reads each message's tags before writing and reports only the ids whose tags actually MOVED; a `TagCommand` base carries that effective set for both `ThreadTagCommand` and `MessageTagCommand`, which had the same defect on a multi-row selection. `tagsApplied` does NOT fire on an empty effective list, since an empty change would push an undo entry whose inverse adds a tag no message ever carried, the same bug one step later. `sendThreadTagChange` gained `onlyMessageIds` so it keeps its thread-scoped REPAINT while restricting the WRITE: the card that changed on screen and the messages that changed on disk are different sets on purpose. **The spec's own plan said item 177 would make a thread undo honest and shrink this to the multi-row case; that was wrong and is corrected in the spec**, an undo inverts an EFFECT, not a scope | | 177 | A thread row means both a message and a conversation, and neither consistently | design | L | **done 2026-08-28**, unreleased, on `thread-row-identity`, eleven commits. Spec: `specs/2026-08-28-thread-row-identity-design.md`. `ThreadListModel::isConversationRow()` is the single predicate and `scopeForSelection()` the single resolver, replacing the `scopeFor()`/`messageScopeFor()` pair that made the CALLER choose. A summary with `totalCount == 1` is unchanged. **Reverses items 108, 110 and 111**, and the user confirmed they are happy to lose the two-tier chips; the `*_thread` submenu and its five action names are deleted with an `### Upgrading` note. Item 112's hiding rule is reversed too: with the absolute entries gone, hiding the toggle on a mixed selection leaves no way to act, so it is a catch-all and the write direction moves with the label. Membership is the union, with two user decisions kept (never evict the current row; an asked-for write evicts at once, an automatic one defers) and one documented lag (a long thread's summary is not updated by a message write, so reading its last unread message waits for the next query). Dashboard from a `ThreadDigest` read by its own worker walk. Two traps found while building: a `QStackedWidget` takes the LARGEST minimum width of its pages and the hidden dashboard was raising the pane's minimum to 395px over MainWindow's 300px floor, caught by an existing resize test; and the pane now holds two `TagStrip`s, so both are named | | 178 | Delete and Restore judge a conversation on one message | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`. `ThreadDigest` carries every message's path, collected by the walk it already makes, so the predicate tests the whole conversation. Known for the SINGLE selected conversation row the digest was requested for; any other selection falls back to the summary's one path, which is the pre-177 answer, deliberately left no worse rather than given a second differently-wrong rule. Section in the closed file | -| 174 | An external `notmuch new` reaches the index without the pending count noticing | defect | S | open, 2026-08-28, from the notes. Item 54 cleared the count for a sync run by `mailsync.sh`, which is what `SyncMonitor` watches; a bare `notmuch new` (a hand run, or a cron entry that is not the script) takes notmuch's own write lock and touches `/tmp/mbsync.lock` not at all, so nothing observes it. The user's framing is the approach: we own `mailsync.sh` and the whole process | +| 174 | An external sync's outcome can only be inferred, and never names what it carried | defect | S | **done 2026-08-29**, unreleased. The premise was corrected first: a bare `notmuch new` must NOT clear the count, since the edits are in the index but not on the server, and the item's own proposal to watch `notmuch_database_get_revision()` was rejected for that reason. `mailsync.sh` writes a JSON status file instead, naming the channels a run carried; the external path now narrows its clear the way the local one always has, and a `skipped` run clears nothing. Log fallback kept. Section in the closed file | | 175 | The send countdown says Undo, and cannot be skipped | presentation | XS | open, 2026-08-28, from the notes. Two changes in one control: the button reads Abort, and a second button sends immediately rather than waiting the countdown out | | 179 | Undo is one level deep in practice, and there is no Redo | workflow | ? | open, 2026-08-29, from the notes. The `QUndoStack` is real and multi-level; what is missing is a `redo` action (absent from `knownActions()`, never called) and an answer to the stack being CLEARED on every new query (`mainwindow.cpp:3458`), which is what makes a deep stack behave like a shallow one. The clear has a correct reason and cannot simply be removed. Redo re-applies a write to real mail, so item 176's rule binds it too | | 180 | The repaint rules are discovered one hole at a time | maintenance | S-L | open, 2026-08-29, from the notes, and a QUESTION rather than a defect. Items 105, 107, 109, 110 and 170 are each one hole in the same surface, all found by hand. Three mechanisms (optimistic repaint, `syncViewMembership()`, revert) agree by documentation rather than by code. Cheapest answer is one invariant test, not a rewrite; the user decides which, and that decides the size | | 181 | The thread dashboard does not follow a write to the conversation it shows | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`, from the notes. The dashboard draws a `ThreadDigest` built by the worker from the INDEX, which arrived only on selection, so a tag write moved the model and the card and left the pane reporting the count the conversation had when it was opened. Reachable from the dashboard's OWN Mark all read button. Re-requested from `onTagsApplied()`, where the write is confirmed: queued beside the write it races it and answers from the state before it, which is how the first fix passed review and failed the test. Section in the closed file | | 182 | An edit made during a sync is announced twice and never says it is waiting | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`, found by hand. The hold branches set a deliberately NON-transient label; all three callers overwrote it a line later with the bare action, so the user was told the write had landed and then told again when it really did. `announceAction()` adds the wait to the action rather than replacing it, since that announcement is what stands in for the confirmation dialog this project rules out. Section in the closed file | | 183 | `undoingAMarkReadRestoresOnlyWhatWasUnread` fails about 1 run in 9 under the full suite | testing | ? | open, 2026-08-29, measured. Item 176's regression test, which guards the undo that rewrote 44 messages of real mail. Nine runs on master: 4 standalone, 3 under `ctest -R mainwindow`, 3 under the FULL parallel suite, and the single failure was in the last group. Not a regression, the base commit behaves the same. Probably the same root cause as item 136 and worth solving with it | +| 184 | New mail waits up to ten minutes, because sync is a fixed cron tick | workflow | ? | 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 | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1223,48 +1224,6 @@ rich-text composer with the original inside it, editable. same want ("show me what I am writing"). If this is built, 133 is moot; if this is deferred, 133 is the thing to do instead. Do not build both. -## 174. An external `notmuch new` reaches the index without the pending count noticing - -**Observed (user, from the notes):** "the statusbar still reads that an -external notmuch run can clear the pending edits without the bar noticing. -Since we own mailsync and the whole process, we should fix that." - -**Cause (verified in the code, 2026-08-28).** Item 54 built exactly one -external path and it is narrower than the note assumes. `SyncMonitor` watches -the inode of `/tmp/mbsync.lock` in `/proc/locks` -(`syncmonitor.cpp:64`), which is the file `assets/mailsync.sh` flocks, and -`onExternalSyncStateChanged()` clears `m_pendingTagEdits` only when -`MailSync::lastRunOutcome()` reads an OK from that script's log -(`mainwindow.cpp:4812-4840`). - -A `notmuch new` that is not that script takes notmuch's own write lock inside -the Xapian directory and never touches `/tmp/mbsync.lock`. Nothing observes -it: no state change, no log line to read an outcome from, so the count keeps -reporting work that has already shipped and the exit prompt offers to sync for -it. This is item 54's symptom surviving through the one route item 54 did not -cover. - -**Approach (not decided).** The note names the lever: the process is ours, so -the honest fix is to stop inferring a sync from a lock file and observe the -INDEX instead. `notmuch_database_get_revision()` gives a monotonic uuid plus -revision that moves whenever anything is committed, whoever committed it; the -worker already holds a handle and reopens it. Watching that would cover the -script, a hand run and a cron entry alike, and would make the outcome question -moot: a revision that moved is evidence the write landed, where a log line is -a report about it. - -**Constraints.** -- Clearing the count on an observed revision bump is NOT sound on its own. A - revision moves for mail arriving as well as for our edits landing, so the - bump has to be read as "the index changed, re-check" rather than "our edits - are in". What settles it is the per-message check the pending map can - already do: every entry names a message and a tag. -- `SyncMonitor` stays whatever this becomes. It answers a different question, - "is a sync running", which drives the spinner and the write hold, and a - revision counter cannot answer it. -- Item 125 is open on the same monitor and should be read alongside: a - `mailsync.sh` that exits 75 leaves the spinner running for ever. - ## 175. The send countdown says Undo, and cannot be skipped **Observed (user, from the notes):** "the countdown popup has a 'undo' button @@ -1447,3 +1406,69 @@ same discipline applies here. the suite with a failure that masks the next real one. - The suite baseline is currently ONE known failure. Anything that makes it two intermittently costs the property that a red suite means something. + +## 184. New mail waits up to ten minutes, because sync is a fixed cron tick + +**Observed (user, 2026-08-29):** "the 10 minutes fixed tick has always bothered +me, I want the changes to my mail to be immediate, the 10 minutes mark is a +compromise, it had started at 30 min and was awful." + +**Split the want in two, because only half of it is open.** + +- **Outgoing is already immediate.** An edit arms `auto_sync_delay_ms`, 2 + seconds by default (item 71), so marking a message read reaches the server + without waiting for the tick. +- **Incoming is the gap.** Mail that arrives is invisible until the next cron + run of `mailsync.sh`, so the wait is uniform on [0, 10] minutes with a mean + of five. + +**Why a faster tick is not the fix.** Polling every minute is ten times the +connections and the server load for a mean wait of thirty seconds, and it is +still a poll: the wait is bounded by the interval however small it gets. The +answer to "tell me when something arrives" is IMAP IDLE, where the server holds +the connection and speaks first. + +**Where IDLE may live, and where it must not.** `AGENTS.md` states this +application does NO network protocol work at all: fetching and sending are +external commands, which is what keeps a mail client out of TLS, authentication +and an IMAP state machine. IDLE inside qtmaildir would break that rule outright +and is not on the table. + +It does not need to be inside. mbsync has no IDLE mode, being a batch syncer +that runs and exits; the tools that hold a connection and TRIGGER a sync are +separate programs (`goimapnotify` and the older `imapnotify` are the usual +ones). That shape fits the architecture exactly: the watcher replaces the cron +line, runs `mailsync.sh <channel>` on activity, and qtmaildir stays as ignorant +of IMAP as it is today. The script's flock still serialises a triggered sync +against a manual one. + +**Blocked on item 174**, and not merely sequenced after it. A watcher makes +syncs arrive at unpredictable times rather than on a known tick, which makes +the application's current guesswork about external runs worse. Item 174's +status file is the reporting channel this needs, and building it first means +the watcher has a tested one to write into. + +**Decisions needed before any code, none of which can be made by reading this +repository.** + +- **Which watcher**, or a small one of ours. A third-party daemon means a + SlackBuild in the `my-slackbuilds` repo and a package to maintain. +- **What the mail server supports.** IDLE is optional in IMAP, and a server + without it leaves polling as the only mechanism. +- **How many connections.** One per account per watched folder, held open + indefinitely; some servers cap concurrent connections. + +**Constraints.** + +- **Keep a slow cron tick as a backstop.** A held connection drops on network + sleep, a server restart or a laptop suspend, and a watcher that dies silently + stops mail altogether, which is worse than a ten minute wait. Belt and + braces: the watcher for latency, a slow tick so a dead watcher is survivable. +- **The lock is the shared mutex** and stays so. A triggered sync, a cron sync + and a click in qtmaildir must continue to serialise through + `/tmp/mbsync.lock`, or two mbsync runs corrupt Maildir UID state. +- **This is not a qtmaildir daemon.** A long-running qtmaildir service was + considered and rejected in the same conversation: it answers none of items + 174 or 125 better than a file does, and it adds a process that can wedge and + take mail delivery with it. What the user wants is a watcher, which is a + different thing in a different place. |
