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:35:11 +0200
committerDanilo M. <danix@danix.xyz>2026-08-25 11:35:11 +0200
commitcd56144ba0304247812cf61c8b6e435779df29d0 (patch)
tree7d00eef972056c6fe55b0890b85806af7b088d0d /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
parent7a810640b2fabafd8da524b275cb4453a1953df6 (diff)
downloadqtmaildir-cd56144ba0304247812cf61c8b6e435779df29d0.tar.gz
qtmaildir-cd56144ba0304247812cf61c8b6e435779df29d0.zip
docs(backlog): close item 162, record a second site for 163
Item 162 is done and its section moves to the closed file on this commit, with the outcome recorded on it: moveMessages() re-resolves by message id when the recorded path is gone, and the hold candidate was investigated and rejected because aSyncHoldsTheWriteLock() guards notmuch's lock while the window sits between mbsync's rename and that sync's notmuch new. Item 163 gains a second site, found while hand-testing 164 and worse than the one it was filed for. openComposerFor() passes ref.filePath to forDraft(); after mbsync renames the file the parse fails and the reopen is refused BEFORE any composer exists, so composing again starts fresh with no previous path to unlink. The old revision survives, each save mints a new Message-ID, and both revisions reach the server. The unlink machinery is entirely correct and never runs. Its heading and row now name both sites, and the note that 162's fix would cover it is removed: that fix re-resolves inside the worker, while these hold a stale path in the UI. Item 164 gains the reproducer's findings. Seven variants in throwaway databases establish that index_file applies no tags, that the carve-out strips inbox correctly in every filename shape and ordering tried, and that the only reproduction is a pass applying inbox while tag:new is already spent. The trigger is still not established, and the entry says so: the live log shows the hook ran on the affected pass and logged success, which the new match-count instrumentation will disambiguate on the next occurrence. 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.md79
1 files changed, 79 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 2e941ec..f537385 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
@@ -7751,3 +7751,82 @@ new action and the existing ones gathered under it rather than duplicated.
- **Item 160 unblocked this** on 2026-08-25: the status bar exists, so a
manual save reports through `refreshDraftStatus()` like an autosave. Route
`Ctrl+S` through `saveDraftNow()` and the reporting is already done.
+
+## 162. Delete fails while a sync is renaming the file underneath it
+
+**Observed (user, 2026-08-25):** deleting a draft reported `Cannot move
+<file> to <account>/Trash`.
+
+**Cause (verified against the live Maildir, not read):** a stale path, and
+neither Delete nor item 158 is at fault.
+
+1. The composer autosaves a draft as `<name>:2,D` and item 158 indexes it
+ under exactly that filename.
+2. **mbsync uploads it and RENAMES it** to `<name>,U=<uid>:2,D`, recording the
+ server UID in the filename.
+3. notmuch still holds the pre-`U=` name until that sync's `notmuch new` runs.
+4. `moveMessages()` reads the filename from notmuch and calls
+ `QFile::rename()` on a path that no longer exists. It fails, the error is
+ emitted, and the message is skipped.
+
+Measured, in this order: `notmuch search --output=files` named a file that was
+not on disk while `Background sync running...` was up, and the same query was
+clean once the sync finished, with the file present under its new `,U=4` name.
+That is why it reads as intermittent, and why it heals itself.
+
+**It is truthful and it loses nothing.** The move is skipped, no wrong folder
+is created, no file is destroyed, and the next sync reconciles. The defect is
+that the message blames a folder for a timing problem, and that the action
+silently does nothing when the user asked for something.
+
+**This is `CLAUDE.md`'s `,U=` trap from the other side.** `MaildirName::fresh()`
+exists because CARRYING that infix across a folder boundary produced
+`Maildir error: duplicate UID` on real mail. Here mbsync is ADDING it and the
+index lags; the same infix, the opposite direction.
+
+**Approach, and it needs a decision.** Two candidates:
+
+- **Refuse the move while a sync holds the lock.** `SyncMonitor` already
+ reports this, and the held-edit machinery from items 97 and 106 already
+ exists for exactly this shape: a tag edit made during a sync is held and
+ flushed when it ends. Delete would join it rather than inventing anything.
+ This is the likelier right answer, since it matches what every other
+ mutation already does.
+- **Re-resolve the filename** from notmuch immediately before the rename and
+ re-query the message if the path is gone. Smaller, but it races the same
+ window it is trying to close, and a second lookup can be stale by the time
+ it is used.
+
+**Constraints.**
+
+- **Delete reaches the real mail server.** Read `CLAUDE.md`'s item 103 notes
+ before touching `moveMessages()`: a wrong folder name is created, adopted by
+ mbsync, and propagated to every other client.
+- Whatever is built, **the message must say a sync is running**, not name a
+ folder. The current wording sent the user looking for a broken folder
+ configuration, which was correct and configured.
+- A test cannot see this in the ordinary fixture layout, where nothing renames
+ a file underneath the index. Driving it means renaming the file between the
+ index write and the move, which is what the reproducer has to do.
+
+**Fixed 2026-08-25.** `moveMessages()` re-resolves by MESSAGE ID when the
+recorded path is gone: one `reindexFolder()` of that directory, then the
+filename from `notmuch_message_get_filenames()` that exists on disk. Bounded
+to a single retry, so a genuinely missing file still reports rather than
+becoming a silent no-op.
+
+**The hold candidate above was investigated and rejected**, and the reason
+matters more than the fix. `sendMove()` ALREADY refuses while a sync holds the
+lock and queues onto `m_heldMoves` (items 97 and 106 built it), yet the defect
+still fired. `aSyncHoldsTheWriteLock()` tracks notmuch's write lock, while this
+window sits between mbsync's RENAME and that sync's `notmuch new`; mbsync
+renames throughout its run without touching that lock, so the damaging window
+is open when there is nothing to observe. Refusing on the lock guards the wrong
+resource. That refusal is correct for its own purpose and was left alone.
+
+Covered by `moveMessagesRecoversWhenASyncRenamedTheFile()` and
+`moveMessagesStillReportsAMessageThatIsReallyGone()`, the second pinning the
+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.
+