diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-25 11:44:48 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-25 11:44:48 +0200 |
| commit | 66c36f0a7f869eac10f7cfa552f23cf076ba7548 (patch) | |
| tree | 0059adb0a70be1604c0eada37c5bf128c3b6c36f /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | |
| parent | 82588d51c4ad0c9e46bbd41658785eb0ba77f48b (diff) | |
| download | qtmaildir-66c36f0a7f869eac10f7cfa552f23cf076ba7548.tar.gz qtmaildir-66c36f0a7f869eac10f7cfa552f23cf076ba7548.zip | |
docs(backlog): close item 163
Its section moves to the closed file on this commit, with the outcome
recorded: MaildirName::resolveRenamed() wired into all three read sites,
the two deliberate refusals (ambiguous match, genuinely missing file)
and why each has a test, and the note that forDraft() must seed
draftPath from the resolved path or the fork simply arrives one step
later.
The stable-Message-ID question is recorded as left undecided rather than
quietly dropped: it is what turns a stale path into two server-side
messages rather than one replaced file, and a draft's id is not yet the
sent message's id, so it wants its own item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
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 | 112 |
1 files changed, 112 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 f537385..f2b977b 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 @@ -7830,3 +7830,115 @@ bounded half. The test renames without reindexing, which is the window mbsync opens, and asserts the database still names the old path so it cannot pass against a fixture that quietly reindexed. Mutation-checked. +## 163. The message pane shows a stale path, and the composer forks the draft + +**Observed (user, 2026-08-25):** selecting a draft filled the pane with +`(unreadable message)` and `This message could not be parsed.`, naming a file +under the account's drafts folder. + +**Cause (verified against the live Maildir):** the path in the pane ended +`.dnx:2,D`, and the only file on disk ended `.dnx,U=4:2,D`. Same mechanism as +item 162: mbsync renames an uploaded file to record its server UID, and the +name the application is holding stops existing. + +**The site is different, and so is the fix.** Item 162 is the WRITE path, +`moveMessages()` reading a filename from notmuch. This is the READ path, and +by the time it fires notmuch is already CORRECT: measured, the index named the +`,U=4` file while the pane still named the pre-`U=` one. The stale path is the +MODEL's, cached when the row was loaded, so refusing to act while a sync runs +(162's likely fix) would not help here at all. + +**The report is honest, which is why it is confusing.** `MimeParser` opened a +path that did not exist and said so. Nothing is lost and the next query +repairs it. + +**Approach.** The read path should RECOVER rather than refuse: on a failed +parse, re-resolve the message id through notmuch and retry once before +reporting. `recoverStaleThread()` already exists for the neighbouring problem +(item 91 reuses it) and is the shape to follow. + +**A SECOND site, found 2026-08-25 while hand-testing item 164, and its +consequence is worse than the pane's.** `MainWindow::openComposerFor()` passes +`ref.filePath` to `ComposeContextBuilder::forDraft()`, and `MessageRef::filePath` +is `notmuch_message_get_filename()` captured when the QUERY ran. After mbsync +renames an uploaded draft, a row loaded before that sync names a file that no +longer exists, `MimeParser::parse` fails, `forDraft` returns an empty context +and the composer reports "That draft could not be read". + +Measured, in order: draft written 10:42, sync at 10:50:43 logged "Detected 2 +file renames" and the file became `,U=5`, the user reopened it at 10:52 and +saw the error. + +**The pane's version is cosmetic and self-repairing. This one silently forks +the draft.** The refusal happens BEFORE `openComposer()`, so the user is left +with no composer for that draft. Composing again opens a FRESH one with +`m_draftPath` empty, so its first autosave has no previous path to unlink: +it writes a new file, `DraftStore` unlinks nothing, and the old revision +stays. Both are then uploaded and both reach the server. + +Two properties make this worse than it first reads. Each save mints a NEW +Message-ID (four saves in the hand test produced four ids), so the revisions +are distinct MESSAGES to notmuch rather than two files of one, and no dedup +anywhere will collapse them. And the unlink machinery is entirely correct +throughout, which is why the code reads fine: `forDraft` sets `draftPath`, +the composer seeds `m_draftPath`, the autosave passes it as `previousPath`, +and `DraftStore::write` unlinks it. The comment at `composecontext.cpp:501` +already names this exact failure ("one message becomes two"). None of it runs, +because the reopen was refused before any of it was reached. + +So the fix below covers this site too, and fixing only the pane would leave +the draft-forking half in place. + +**Constraints.** + +- **A retry must be bounded.** A message that genuinely cannot be parsed + (item 41's territory) must still report, or a real defect becomes an + infinite loop. +- Re-resolving by id is what makes this safe; re-scanning the folder is not, + since two files can carry the same id. +- The placeholder wording is correct and should stay for the genuine case. +- **The composer site must recover, not merely report better.** A clearer + error still leaves the user composing a second copy of their own draft. +- Whether a draft should keep a STABLE Message-ID across revisions is a + separate question this entry does not decide. It is what turns a stale path + into two server-side messages rather than one replaced file, and it wants + its own item; note that a draft's id is not yet the sent message's id, so + changing it is not obviously free. + +**Fixed 2026-08-25.** `MaildirName::resolveRenamed()` answers the filesystem +question: the path unchanged when it still exists, otherwise the file in that +same directory whose unique stem matches. mbsync preserves the stem +(`<stem>:2,D` becomes `<stem>,U=5:2,D`), which is what makes resolving by +filename safe here at all. It never recurses and never crosses a folder +boundary; a file that changed FOLDERS is a different question that only the +message id can answer, and `moveMessages()` re-resolves that way for item 162. + +Two refusals in it are deliberate and both have tests. An ambiguous match (two +files sharing a stem, which a correct Maildir cannot produce) yields nothing +rather than a guess, because opening or moving the wrong message is worse than +reporting none. And a genuinely missing file yields nothing too, so a real +deletion still reports instead of becoming a wrong answer. + +**Three call sites, not the one this was filed for.** The pane +(`renderMessages`), Reply and Forward (`openComposerFor`), and the draft reopen +(`forDraft`). The third is the one that cost data, and its shape is worth +keeping: the refusal happened BEFORE any composer existed, so the user composed +again into a fresh window whose autosave had no `previousPath` to unlink. The +unlink machinery was correct at every step and simply never ran. + +`forDraft()` seeds `draftPath` from the RESOLVED path. Seeding the caller's +would let the reopen succeed and the unlink still miss, which is the same fork +arriving one step later; the integration test asserts on `draftPath` for +exactly that reason. + +Covered by five unit tests on the resolver and by +`aDraftRenamedByASyncStillReopensAndReplacesItsFile()`, which renames the draft +the way mbsync does and asserts the file COUNT, the shape the fork actually +takes. Mutation-checked: removing the resolution fails it with the reported +symptom. + +**Left undecided, deliberately:** each save mints a NEW Message-ID, which is +what turns a stale path into two server-side MESSAGES rather than one replaced +file. That wants its own item. A draft's id is not yet the sent message's id, +so changing it is not obviously free. + |
