aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-25 11:44:48 +0200
committerDanilo M. <danix@danix.xyz>2026-08-25 11:44:48 +0200
commit66c36f0a7f869eac10f7cfa552f23cf076ba7548 (patch)
tree0059adb0a70be1604c0eada37c5bf128c3b6c36f /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
parent82588d51c4ad0c9e46bbd41658785eb0ba77f48b (diff)
downloadqtmaildir-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.md112
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.
+