diff options
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 112 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 77 |
2 files changed, 113 insertions, 76 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. + 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 f60ee7f..5e3df58 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 @@ -236,7 +236,7 @@ taking that too literally. | 161 | The composer has no menu bar | discoverability | S | **done** 2026-08-25, unreleased. File / Edit / Format, to the user's own chosen scope. **Save draft (`Ctrl+S`) is the only NEW action**; everything else is gathered, and the menus show the toolbar's own `QAction` objects rather than copies, per item 140's rule. Two needed hand-building: the HTML toggle is a `QToolButton` and cannot go in a menu, so a checkable twin mirrors it BOTH ways; and the signature entry takes the switch's own `QMenu` pointer, since that menu is rebuilt when the signatures change and copied entries would go stale. Edit's entries follow the editor's own `undoAvailable`/`copyAvailable`. `theMenuBarReachesEveryComposerAction()` is item 132's rule applied to the composer, walking the real menu bar and finding actions by `findChildren`, so a future action added to the toolbar and forgotten in the menus fails without touching the test | | 162 | Delete fails while a sync is renaming the file underneath it | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix and notmuch keeps the pre-`U=` name until that sync's `notmuch new` runs, so `moveMessages` renamed a path that no longer existed and Delete silently did nothing while blaming the destination folder. `moveMessages` now re-resolves by MESSAGE ID when the recorded path is gone: one reindex of that directory, then the filename that exists on disk. Bounded to one retry, so a file genuinely gone still reports. Holding the move during a sync was the other candidate and is NOT the fix: `sendMove` already refuses on notmuch's write lock, but this window sits between mbsync's rename and that sync's `notmuch new`, which touches no lock | -| 163 | The message pane shows a stale path, and the composer forks the draft | defect | S | open, 2026-08-25, found by hand. The model keeps the filename a row was loaded with, mbsync renames the file to add `,U=<uid>`, and `MimeParser` then opens a path that no longer exists. notmuch is CORRECT by then; the MODEL is behind. **A SECOND site found 2026-08-25 and it is the worse half:** `openComposerFor` passes `ref.filePath` to `forDraft`, the parse fails, and the reopen is REFUSED before any composer exists, so the user composes a fresh draft whose autosave has no previous path to unlink. The old revision survives, each save mints a new Message-ID, and both reach the server. Item 162's fix does not touch either site: that one re-resolves inside the worker, these hold a stale path in the UI | +| 163 | The message pane shows a stale path, and the composer forks the draft | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix while the model still holds the name the query returned. `MaildirName::resolveRenamed()` returns the path unchanged when it exists, else finds the file in that one directory whose unique stem matches; it refuses an ambiguous match and yields nothing for a genuinely missing file. Wired into all THREE read sites: the pane, Reply/Forward, and the draft reopen. The reopen was the one that cost data, forking a draft into two files with two Message-IDs, both reaching the server | | 164 | A draft this application saved keeps `inbox` | defect | S | open, 2026-08-25, **cause corrected 2026-08-25**. The first diagnosis blamed a missing drafts helper and was WRONG: `NOT_ARRIVALS` in `qtmaildirconf.py` is `("sent", "drafts")`, the folder list includes every account's drafts folder, and `notmuch count` confirms the carve-out query MATCHES the affected draft. The carve-out is scoped to `tag:new`, and the draft carries `inbox` while `tag:new` is 0, so it was never in scope when the hook ran. Measured separately: an mbsync-style rename does NOT re-add `new.tags`, so the retag theory is out too. What remains unestablished is WHICH pass tagged it; establish that before writing code | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1341,81 +1341,6 @@ The 70-second duration recorded above fits a `QTRY_*` waiting for a file that is never going to appear, which is consistent with a wrong destination rather than a slow one. -## 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. - ## 164. A draft this application saved keeps `inbox` **Observed (developer, 2026-08-25):** `notmuch search --output=tags` on a |