| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Item 123, built over 2026-08-20 to 2026-08-23 in thirteen tasks against
docs/superpowers/specs/2026-08-20-compose-and-send-design.md.
The application writes mail now. A composer window per message, markdown as
the body, drafts autosaving into the account's Maildir, and sending through a
per-account command on stdin rather than any network protocol of this
program's own. A countdown with an Undo stands between pressing Send and the
command running.
Two things came in alongside it. The notmuch auto-tagging hooks moved here
from the retiring `mailctl` project and learned that mail this application
files itself never arrived, so sent mail and drafts stop appearing in the
inbox. And the v1/v2 language is retired: semver on the user-visible surface
is the rule, and those labels described a split that composing made obsolete.
Hand tested against a fake send command rather than a real one, deliberately:
New, Reply and Forward all produce correct messages, a forwarded attachment
survives intact, and the sent copy is filed. That testing found the two
defects fixed on this branch, and both were invisible to the suite: a composer
orphaned by quitting the main window, and every sent message tagged `inbox`.
Twenty-two defects were found in the plan document's own draft code while
building it, which is why CLAUDE.md says to treat every code block in a plan
as a draft.
|
|
Task 13 of item 123, the documentation that was left when the code was
finished.
The README claimed "sending is not implemented. Compose, reply, forward and
send are planned for v2 and need a companion send script that does not exist
yet", which had been false for three days. It also documented none of
`trash`, `send_command` or the `[compose]` section: `trash` has been effectively
mandatory since item 103 and its absence breaks Delete, so a user reading only
the README could not configure a working install. Both are item 122, which
closes with this.
Every default in the new [compose] block was read out of config.h rather than
from the prose around it, which was worth doing: send_html defaults to TRUE
and the first draft of this documented it as false, and the key is
autosave_interval_ms rather than the autosave_delay_ms that reads more
naturally.
The v1/v2 language is retired rather than updated. The project follows semver
on its user-visible surface and those labels described a scope split that
stopped being true when compose shipped. CLAUDE.md says so explicitly rather
than deleting the sentence, since the older spec and plan documents still use
the phrase and are historical records: read it there as "before compose".
CLAUDE.md's architecture diagram listed none of the compose units. It does
now, and it distinguishes the classes from the namespaces, because most of
them are namespaces of free functions over values: the markdown, the MIME
assembly and the account-picking are all testable without a widget. There is
no FormatToolbar class, which the first draft of this diagram invented, and
that is the exact mistake the paragraph under it already warns about for
QueryBar and AttachmentBar.
Also records item 148 from the notes: Ctrl+W does not close the composer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
The 2026-08-23 pass over the user's notes. Ten rows, eight sections: two of
the ten are duplicates of items already open and get a row saying so rather
than a second entry.
Six of the eight are the compose interface, which had never been looked at by
a human until this week. They are presentation rather than defect, and every
cause is verified in the code rather than copied from the note: one toolbar
carrying formatting, Attach and Send together (142), text buttons where an
editor uses icons (143), a checkbox label that describes a mechanism without
naming it (144), and Cc and Bcc as unconditional form rows (145).
The other two are the message actions. Forward has no button anywhere (139),
and the user's own answer to that is bigger than a missing button: Compose,
Reply and Forward belong over the message pane rather than on a toolbar that
otherwise acts on the list (140), which needs a bar that does not exist yet
(141). 139 closes inside 140 if that is built.
138 is the closest thing to a defect here: every account already configures a
drafts folder and the query row has no generator for it, so the one folder the
composer writes to is the one folder with no button.
Item 21 gains a pointer rather than a plan. The user is drafting the shortcut
table in their own notes and it is the specification this item has been
waiting for, so the entry says to read it and not to propose one. It records
what the draft settles (Ctrl+Return for Send, which closes an open question
from item 123 task 11), the two collisions its proposal creates (Ctrl+R is
restore today, Ctrl+F is coupled to Find moving to `/`), and the trap under
that last one: a `/` registered as a shortcut is dispatched before any
editable widget sees it, so it needs the event filter Return already uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
A composer is deliberately parentless, so that it appears in the task
switcher and stays usable while the main window is. Qt therefore does not
take it down with that window, and being a live top-level it kept the process
alive: the main window vanished, the composer stayed on screen with nothing
behind it, and closing it then raised the unsaved-edits dialog for a session
the user had already ended.
The quit path already ASKED about those edits and saved them. What it never
did was close the windows afterwards.
Closing rather than deleting: WA_DeleteOnClose is set on every composer, so
close() is what frees them, and it lets ComposeWindow::closeEvent() run its
own draft handling on the way out. Iterating a copy of the list, since
closing runs the `closed` handler and that mutates m_composers.
Placed last, after every route that turns back has returned: reaching it means
the application really is quitting.
The test opens TWO composers, so a fix that closed only the last one cannot
pass it. Mutation-checked by removing the loop.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
The post-new hook and its rule store move here from the companion mailctl
project, which is being retired. Nothing else was shared between the two, so
this is a plain move: mailrules.py is stdlib-only and post-new imports only
it.
With that in hand, the hook learns the one thing it could not know before.
notmuch's new.tags applies `inbox` to every file it indexes, and it cannot
tell an arrival from the copy this application files into a sent folder after
a send, so sent mail turned up in the inbox view and in any hand-typed
tag:inbox search. Drafts arrived the same way, through the composer's
autosave. 786 messages were affected on the developer's own index.
qtmaildirconf.py reads the sent and drafts folders out of qtmaildir.conf, so
adding an account fixes itself. Reading the application's own config is not
the cross-repo coupling it would have been last week: this repo owns the hook
now.
Three properties are load-bearing:
- it is NOT a relaxation of PROTECTED_REMOVALS, which is about a rule
removing `inbox` from mail whose provenance the hook cannot judge. Here
the provenance is the file's own path, and `inbox` was never true of it.
- only `inbox`. maildir.synchronize_flags is true, so removing `unread`
would rewrite Maildir filenames and reach the server on the next mbsync.
- an empty folder list means NOTHING, never an empty query, which notmuch
reads as "match everything". A system with no qtmaildir config must be
left alone rather than have every new message stripped.
Trash is deliberately not in the list: Delete leaves `inbox` on a trashed
message so Restore can put it back where it came from.
The three Python suites run under ctest rather than beside it as scripts
someone remembers to run, since this code tags real mail unattended on every
sync.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
The reply family is disabled on mail that arrived at an account with no
send_command, behind a ribbon in MessageView naming the account and the key
to add. save_message is deliberately never disabled: it is the escape hatch
for exactly that case.
The ribbon is a WIDGET in the pane's layout, never markup inside the web
view. Composing HTML from configuration into the one document that renders
input from strangers is the wrong direction, and the header row is already a
widget for the same reason.
Compose itself is disabled only when NO account can send, and that state is
not warned about at startup: an installation with no send_command anywhere
is a valid read-only installation.
Every reply resolves through messageScopeFor(), not threadFor(): a thread
row means the one message its card shows. Replying to a thread is
meaningless; a reply answers a message. The context is built from the
DATABASE rather than the model, the rule Restore already follows, because a
row whose state has not been re-queried carries stale values and a reply
built from one would carry the wrong recipients.
The mail root crosses from the worker as its own signal. There was no route
for it at all: mailRootOf() is file-static in notmuchworker.cpp, and item
124 records that composing a destination from database.path writes into the
Xapian tree under a split index. The test uses NotmuchFixture::splitIndex(),
the only layout where the two accessors disagree.
A thread row's path is RELATIVE to the mail root while a message row's is
absolute, so the account lookup matched nothing and the reply family was
dead on mail from an account that could send. Found by the positive guard
test rather than the negative one, which passed throughout for the wrong
reason.
The quit path checks the failed-save case FIRST. In the ordinary case
nothing is lost by saving; there, saving is what is already not working, so
the dialog says plainly that quitting loses that text rather than offering a
save that will fail again. Both dialogs name the composers, and the ordinary
one asks once whatever the count, because three modals in a row is worse
than a coarse answer. Its wording says drafts already saved stay in the
folder, so Discard cannot read as 'delete my three messages'.
The Save loop holds QPointers, not raw pointers. A deleteLater() posted
while a nested exec() runs IS processed by that nested loop, measured in a
standalone program: the guard nulls before the modal returns. Closing a
composer while the quit dialog is up therefore freed a window the loop then
called saveDraftNow() on, crashing at the exact moment the application
promised to preserve that text.
A compose request that matches nothing clears itself and says so. It was
cleared only on a match, so a message deleted between selection and Reply
left the request armed for the session: Reply did nothing, and the next
ordinary click on that message opened a composer nobody asked for while the
pane stayed blank.
Forward carries the original's attachments, which the context has always had
a field for and nothing ever filled, and seeds its HTML toggle from
[compose] send_html. Only Reply seeds that from the original.
save_message keeps its filename inside the chosen directory and no longer
overwrites a file already there. The check was correct and untested: the
test asserted through Attachment's helpers rather than through the function
production calls, so deleting the containment check outright left it green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvwDptMWxjqhbCmjxwcSZ2
|
|
A separate top-level QMainWindow, one per draft, several open at once. A
modal dialog cannot consult another message while writing, which is most of
what replying is, and taking over the message pane fights the pane that
exists to show what is being replied to.
No geometry save and no restore, deliberately. Under a tiling compositor
saveGeometry stores normalGeometry while the compositor owns the tile, so
the restore is correct and looks broken; a whole session went into that
once.
Autosave is a debounce AND a dirty check: an unchanged message writes no
file and provokes no sync. The check is on a fingerprint of the
OutgoingMessage, NOT on the built bytes as the plan drafted. GMime is given
a fresh Date and Message-ID on every build, so two builds of an unchanged
message never compare equal; a check on the bytes would have read as
working while writing a file, and an mbsync upload, on every debounce.
Checking before the build also skips the blocking build for the no-change
case, which is the common one.
closeEvent writes the draft when the buffer is dirty. Without it the
debounce is a hole rather than a delay: typing a paragraph and pressing the
window manager's X inside the interval loses it silently, since
WA_DeleteOnClose destroys the window immediately afterwards. A failed save
there does NOT refuse the close, because a window that will not close
because it cannot save is worse than one that closes having raised the
banner, which is what the quit path reads.
One flag covers a send, countdown included. An earlier revision had two,
and the narrower "committed and running" one reads as the honest thing to
guard a live SMTP conversation with. It is not: a close during the
countdown destroys the parented SendDialog, committed() never fires, and
the user pressed Send, watched a countdown, and believes the mail went. The
narrow flag was also written in three places and read in none.
A failed draft write raises a persistent banner rather than a modal or a
fading status line. A modal mid-sentence is hostile while the user is
typing, but the warning must survive until it is dealt with, because the
quit path escalates exactly this state to a dialog on the way out. An
account with no drafts folder reports success rather than failure: nothing
was written and nothing failed, and a false there would make the quit path
offer a retry no retry can change.
A failed send saves the draft before reporting. send() builds from the
widgets without saving, so the revision on disk is whatever the last
debounce wrote: edit, send, fail, close, and the user gets the older text
back, having watched their correction be sent.
A failed sent copy after a successful send is a modal, and never a send
failure: the message went, and reporting otherwise makes someone send it
twice. It is the one failure here that silently diverges what the recipient
received from what the local archive shows, and nobody discovers a missing
sent copy by noticing a line that appeared for a few seconds.
The formatting toolbar applies its edits through a QTextCursor document
replacement inside one edit block, NOT setPlainText as the plan drafted.
Measured against a real widget: setPlainText destroys the document's undo
stack and resets the cursor to 0, so every toolbar press would throw away
everything the user could undo. The cursor route leaves undo available,
collapses to a single undo step, and emits textChanged once. The seeded
quote is cleared off the undo stack afterwards, since it is not an edit the
user made and one Ctrl+Z on a fresh composer must not wipe it.
The per-send connect carries Qt::SingleShotConnection. MessageSender is a
long-lived member, so a bare connect accumulates a permanent receiver per
send and the second result runs both lambdas, the first still holding the
first message's bytes: it files a sent copy of the wrong message and acts
on a dialog it already destroyed. Covered by a test that sends, fails,
corrects and sends again; without the flag it segfaults in QLabel::setText
on the destroyed dialog. Its companion disconnect takes the specific
connection handle rather than every finished receiver on this object, so a
later observer cannot be killed silently.
The attachment warning states sizes with a decimal and a stepped unit.
Integer MB division read as "'x' is 0 MB. Many mail servers refuse messages
above about 0 MB." for any attachment_warn_bytes below a megabyte, in both
halves of one sentence.
The autosave timer is created before buildUi(), which is load-bearing:
buildUi connects every field to markDirty and seeding then fills those
fields, so markDirty runs during construction. Created afterwards it is a
null dereference on the first seeded field, which is every composer.
Twenty-six cases in test_mainwindow, each mutation-checked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Found while running the suite during item 123 task 10, and checked rather
than assumed to be unrelated: with the branch's work stashed out, on a clean
tree, it still fails 1 run in 6. A failure that appears during unrelated work
gets blamed on the change in front of it unless someone measures.
Sized ? deliberately. The race is either in the test's wait or in the
Maildir move Delete performs and Undo reverses, and the two have very
different consequences: a test that waits wrongly is noise, while a move
that races is mail landing in the wrong folder, which this document already
records as reaching the mail server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5
|
|
Three rows in every state so nothing reflows and the window never jumps. The
bar changes MODE rather than place: determinate while the countdown drains,
because a countdown has measurable progress, and indeterminate once the
command starts, because a send does not. That is the pairing item 134's
widget was extracted to serve.
The delay is where cancelling is safe and it is the only place it is.
Nothing has reached a server during the countdown, so Undo means genuinely
nothing happened; killing send_command once it runs leaves an UNKNOWN send,
which is worse than either clean outcome. Undo therefore disables itself the
moment the command starts, and stays visible while disabled: a control that
vanishes re-lays out the popup mid-operation, and a greyed Undo says why
cancelling is no longer possible where an absent one looks like it was never
offered.
The test for this asserts the NEGATIVE property, that committed() never
fires after Undo, including after the original countdown would have elapsed.
Asserting only that undone() fired would pass against a design that ran the
command and threw the result away, which is the whole failure the delay
exists to prevent.
Removing the close BUTTON is not the same as closing the code path, and the
first draft did only the former while its comments claimed otherwise. Escape
still reached QDialog::reject(), and close() during the countdown hid the
window while leaving the timer running, so the send committed with nothing
on screen and the only cancel control destroyed: measured, committed=1 on a
dialog the user had dismissed. A never-shown dialog did the same, since
close() returns early without reaching done(). That is CLAUDE.md's done(int)
trap in the one place it costs mail rather than state.
Dismissal is REFUSED before commit rather than treated as an implicit Undo,
at the user's decision: a close that silently means cancel overloads one
gesture with two meanings, while a refusal leaves Undo as the only way out,
which is what the popup's single control already says. done(int) refuses
pre-commit and forces Accepted after, closeEvent covers the never-shown
route done() cannot see, and Undo passes through both. Task 12 needs no
special entry point, since it closes after the send finishes and that is
post-commit by definition.
A refusal must not read as a hang, so the label says how to leave. Making
the hint silent was a mutation that SURVIVED, because the text was written
in two places and neutering one was masked by the other; extracting it to
one function exposed a real defect behind the wrong green, in that the next
tick overwrote the hint 100ms later and the refusal was effectively silent
anyway. It is held for 1500ms now, with a test that it survives a tick and
still releases.
setStage is public and Task 12 passes values into it, so it refuses to wind
back to CountingDown after commit rather than trusting its caller with an
invariant this class documents as inviolable; the label read "Sending in
0..." and the bar returned to determinate. Both m_committed guards carry
tests: removing them left the suite green, so two deliberate safety
additions rested on reasoning alone.
Every route out is asserted, per the rule that a test using close() while
the user uses Cancel covers one route of three: close() shown, close()
never-shown, Escape bare and with Shift and Ctrl, reject() direct, and Undo,
which must still work or the popup is a trap.
The status label is sized to the longest string it can hold in the current
language rather than to its content: Italian 'Rimozione della bozza...' is
longer than 'Removing draft...', and a label sized to content resizes the
popup between stages.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5
|
|
Handlers are empty for now; this commit is the registration, so the three
coverage tests guard every later task rather than being satisfied at the end.
Two corrections to the spec, both found in the code rather than assumed. It
calls for a new top-level Message menu and one already exists, so these join
it; two menus named Message would be a defect. And it says every action
needs a binding, which item 132 changed while this was being planned:
save_message ships with no chord, since it is the rarely-used escape hatch
and menu reachability is now the rule that must hold.
reply_no_quote shares reply's icon and is added to the no-duplicate-icons
exception list for the same reason the five thread actions are: it never
reaches the toolbar, and a menu entry always carries its text. That list is
renamed menuOnlySharedIconActions, after the property that earns the
exemption rather than the tier that first needed it.
Bindings are provisional. The user intends to rework them, and Ctrl+Alt+R
for reply_no_quote is an imperfect fit since that tier elsewhere means a
wider scope rather than a variant.
The six labels went through a mnemonic pass that nothing enforced before.
Four of them collided inside the Message menu on first writing, and the
whole class was invisible to a green suite: Qt does not error on a duplicate
mnemonic, it cycles the highlight instead of activating, so the key simply
stops working. Item 57 had already decided this rule by rejecting a label
that would have collided, but it lived in prose and in one test's comment,
which is precisely why it was broken again here.
noMenuHasTwoEntriesSharingAMnemonic() enforces it now, scoped per menu since
a mnemonic resolves among the open menu's entries, and keyed on
QKeySequence::mnemonic() rather than on parsing & by hand, because && is a
literal ampersand and only Qt answers which key it will dispatch. Three
pre-existing collisions are a named freeze list rather than a silent fix or
a narrowed test: Alt+R three ways and Alt+S twice in Message, Alt+O in View.
Renaming entries a user has had in their fingers since 0.1.0 belongs to the
shortcuts rework, and the freeze is written as exact groups so a new entry
joining any of them still fails.
Two of the test's own design choices came from mutation checks that failed
for the right reason while reporting the wrong thing. Reporting collisions
as pairs was order-dependent, so a new colliding entry re-keyed a frozen
pair and the fresh defect read as "a frozen collision no longer happens";
matching frozen entries by whole string broke the same way, since a growing
group stopped matching its frozen text. It reports whole groups and matches
on menu plus key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5
|
|
MarkdownFormat, task 8 of the compose-and-send plan. Three free functions
over (text, selection start, selection end) returning the new text and the
selection that follows it, so the grammar is tested without a widget.
Three gaps in the plan's draft, each now pinned by a test checked against the
mutation that breaks it:
- QString::lastIndexOf INCLUDES the position it is given, so quoting with the
cursor at the end of a line found that line's newline and quoted the
FOLLOWING one. The draft's fixtures never placed a cursor there.
- A backwards selection was normalised but never tested, so the swap was
unguarded; a right-to-left drag is an ordinary gesture and Qt reports the
anchor after the cursor. normalise() now swaps and clamps in one place.
- A blank line inside a quoted range produced "> " with trailing whitespace,
which editors and mail clients strip anyway. It is written bare.
Two further defects came out of review:
- quote()'s selectionStart was unasserted for any block not starting at line
zero. Hardcoding it to 0 passed all nineteen tests, because the one test
naming the property quoted the first line, where right and wrong coincide.
A wrong selection there means a second press quotes a line the user never
selected, and a following Bold bolds the wrong text.
- A selection splitting a surrogate pair split the character across the
inserted tokens, leaving invalid UTF-16. Not reachable from the toolbar,
where arrow keys and mouse hit-testing both move in whole clusters, but
reachable by any code computing a position arithmetically. normalise()
nudges off a low surrogate; a collapsed cursor moves back on both ends,
since widening would turn "insert an empty pair here" into "wrap the emoji".
The buttons stack rather than toggle: a second Bold press gives ****this****,
and a second Quote press nests. That is what the spec specifies, and the
preserved selection exists so a second press can apply a SECOND token. A
toggle was built during this task at the user's request and reverted on
finding it contradicts the spec at two sites; it is recorded as backlog item
135, where the unanswered question is what replaces bold-then-italic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L
|
|
ComposeContext, task 7 of the compose-and-send plan. Address parsing,
recipient derivation, the References chain, subject prefixing and account
resolution, as free functions over values so they test without a painter.
Recipient derivation was designed from the spec rather than transcribed: the
plan's draft omitted it and its tests could not compile, calling
QVERIFY(config.load(path)) against a void return.
Six defects found in review, each pinned by a test checked against the
mutation that breaks it:
- Message-ids reached GMime bare, and GMime writes an EMPTY header for a bare
addr-spec rather than complaining. In-Reply-To and References both shipped
blank, so every reply would have arrived as an orphan thread with nothing
wrong to see locally. MessageBuilder now brackets on write, in the one place
that composes those headers rather than in each caller.
- internet_address_to_string was called with FALSE for the encode flag, so a
display name carrying a raw newline rendered with the newline intact. That
is a header-injection primitive.
- A reply to the user's own message addressed the user. It now goes to that
message's original recipients, mirroring their To/Cc split, which is what
the Sent view and a follow-up on unanswered mail need.
- A From parsing to no mailbox left To empty, reachable from real mail
("From: Mailer Daemon"). MessageBuilder treats an empty recipient list as
success, so the message would have been handed to the send command with
nobody to deliver to and filed in Sent looking sent.
- The References header was split on whitespace alone, so a client's
non-conformant "<a@x>,<b@y>" became one token and the bracket strip produced
the fabricated id "a@x>,<b@y".
- Reply and forward prefixes were recognised in English only, doubling every
AW:, SV:, WG: and Re[2]: a mixed-locale mailbox receives. Single-letter
spellings are deliberately excluded: with R: recognised, "R: report on Q3"
reads as a prefix and a genuine first reply threads nowhere.
The mailbox-only guard in parseAddressHeader survived its first mutation
check, because removing it still yields no recipients: the invalid GObject
cast makes GMime's own assertion return NULL. That is undefined behaviour
papered over by an assertion G_DISABLE_CHECKS compiles out, so the test now
asserts on the emitted critical rather than on the count. Registering the log
handler on a NULL domain catches nothing; the criticals carry "GLib-GObject"
and "gmime".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L
|
|
MessageSender runs the configured command with the message on stdin and
judges the result by its exit status alone. Nothing here waits on the
event loop, so a send does not block the GUI thread; a 1.6MB payload was
probed through a reading stub without deadlocking the pipe buffer.
The command is split and passed to QProcess as a program and an argument
list, never through a shell. A test asserts that by giving the command
shell metacharacters and checking that the marker file a shell would have
created does not exist, so the property fails a mutation rather than
resting on a comment.
Four corrections to the plan's draft. splitCommand handles double quotes
only, so a single-quoted argument splits wrongly and the header now says
so. A crashing command delivers finished(11, CrashExit) and would have
been reported as "exited with status 11", so a crash branch was added. A
command that exits without draining a large stdin emits WriteError before
finished(), which the draft handled correctly and by luck, untested. And
an empty send_command is checked after trimming.
Two contract gaps found in review, both about what this class promises
rather than what it does.
The exactly-once guarantee covers the EMIT, not what a caller receives: a
long-lived sender plus a connect() inside each send accumulates
receivers, and the second result then runs the first send's lambda too,
filing a sent copy of the wrong message. The header now scopes the
promise and requires Qt::SingleShotConnection. The plan's Task 11 call
site already had that flag, sixty-nine lines below the connect and
outside anything a reader would see, so the plan gained a note where
someone retyping it will read it.
And destruction mid-send killed the command with no report, announced
only by a Qt warning: a live SMTP conversation abandoned, possibly
partially delivered, while the user believes it was cancelled. The
destructor now closes stdin, waits a bounded five seconds, and only then
kills. It emits nothing either way, because the outcome after a kill is
genuinely unknown and reporting "not sent" for a message that may have
gone out is the mailsync.sh mistake pointing the other way. Claiming
m_reported before kill() is what makes that true, since kill() delivers
finished(CrashExit), which would otherwise emit exactly that untruth.
No timeout on the send itself: killing a slow but working send is worse
than waiting. Task 10 owns the popup, and deliberately offers no cancel
after commit, so this class promises none either.
Also refreshes the translations Task 5 left out. That gap was invisible
because test_translations builds its rows from the .ts file, so a string
that never entered it is never asserted on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QP2g3b3kuLx6AYFCNEz6UR
|
|
DraftStore::write() renders a built message into <folder>/cur with the
given Maildir flags, writing through QSaveFile and unlinking the previous
revision only after the new file is in place.
Two orderings here are load-bearing and both are covered by a test that
was checked against the mutation that breaks it.
The unlink runs only after the write has succeeded, so a failed save
leaves the previous revision intact rather than losing both. Provoking
that failure needs care: the plan's version used an unwritable path where
mkpath() fails and the function returns before reaching either the write
or the unlink, so a mutation moving the unlink up survived it. The test
uses an existing but read-only cur/ instead, where the failure lands at
the write.
And the size comparison stays ahead of commit() in the condition, because
QSaveFile::commit() returns true after a short write and renames the
truncated bytes into place: measured, write 4096 of 65536 with commit
reporting true and the file left in the listing. What leaves the
directory empty is the short-circuit returning before commit() is
reached, after which ~QSaveFile() discards the uncommitted scratch file.
Reducing the condition to !file.commit() looks like a simplification and
writes a truncated draft into cur/, where notmuch would index it and
mbsync would upload it.
Resolving the mail root stays the caller's job, per item 124; this takes
an absolute folder path and composes nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QP2g3b3kuLx6AYFCNEz6UR
|
|
Two silent failures on the path that produces bytes for other people.
A directory passed the attachment guard, because QFileInfo reports a
directory as existing and readable, and opening one read-only is legal. GMime's
base64 encoder then looped on read() returning EISDIR without advancing:
measured at 2.1 million failed reads in twenty seconds and still going. Since
build() runs synchronously from autosave on the GUI thread, dragging a folder
into a composer froze the whole application with the draft unrecoverable.
isFile() also excludes device nodes and FIFOs, which block the same way.
An unparseable recipient was dropped rather than reported. The old code
skipped anything that failed to parse and then only wrote the header if what
survived was non-empty, so a message whose only recipient was mistyped was
built with no To: header at all and reported success. With msmtp -t taking its
recipients from the headers, that is a message handed to the send command with
nobody to deliver to, and a copy filed in Sent that looks sent and reached no
one. A recipient the user typed and this cannot understand now stops the send,
the way a missing attachment already does.
The directory test carries a timeout deliberately: a regression there hangs
the binary rather than failing it. Two details make that work and the first
draft had neither. It must not join the worker, since a thread stuck in the
defect never returns and the join reproduces the hang instead of reporting it,
verified by reverting the fix: with the join the binary had to be killed at
150s with no verdict, without it it reports a FAIL and exits in 15s. The
result is shared through a shared_ptr so the leaked thread cannot write into a
returned stack frame.
Also: the no-address error names the account, since it matters once several
exist; messageId is assigned once on the success path rather than set early
and cleared on each failure, which is an invariant the next early return would
forget; and the Bcc comment now records that keeping the header stores the
blind list in plaintext in the sent copy and any draft, which mbsync syncs to
the server. That is accepted knowingly, and saying so stops a later reader
"fixing" it and silently breaking blind delivery.
One correction to the review that prompted this. The claim that
internet_address_list_parse returns a zero-length list rather than NULL did not
reproduce: measured on GMime 3.2 with a standalone probe, every garbage input
tried returned NULL, and no input was found producing a non-null empty list.
The length check is kept as defensive code and is documented as such rather
than as observed behaviour, since no fixture reaches it and a mutation on it
survives the suite. The defect itself was real and is what the test kills.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
Task 4's code review measured it: a large attachment is read and base64
encoded on the calling thread, and autosave calls build() from a GUI-thread
timer. The directory hang that review found is fixed, but the blocking read
is by design and will be felt in the composer.
Recorded against Task 11 rather than fixed, because nothing in the composer
crosses the worker boundary and adding a second threading model for one call
is worse than the stall.
|
|
One built message serves three consumers: the autosaved draft, the bytes on
the send command's stdin, and the sent copy. A draft is therefore
byte-identical to what would be sent.
Three GMime defaults are wrong for this application and each is corrected
explicitly, because all three fail only on accented text and this user
writes Italian:
GMime encodes as iso-8859-1 unless told otherwise, so the subject carries an
explicit utf-8 argument. g_mime_text_part_set_text() encodes with whatever
charset is set when it is CALLED, so setting the charset afterwards produces
a part labelled utf-8 carrying latin-1 bytes; the content stream is built
directly instead. And neither Date nor Message-ID is generated unless asked
for, and a message without a Message-ID cannot be threaded by anything that
receives it.
Attachments are checked at build time rather than at attach time: a file can
vanish in between, and a message missing the thing it was written to carry
must never reach the send command.
An account with no address fails the build rather than producing a message
with an empty From. Config::account() returns a default-constructed Account
for an unknown key rather than failing, so without that guard a bad key
would produce silently malformed mail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
toInt() and toLongLong() return 0 on failure rather than the default, so a
typo in autosave_interval_ms produced a zero-interval timer. That timer is
restarted on every keystroke, so it would fire on the next event-loop pass
and turn a 30 second debounce into a Maildir write per keystroke, each one
uploaded by mbsync: exactly the behaviour the debounce exists to prevent.
This file already had the right shape in five places, a checked parse that
reports the bad value and keeps the default. The [compose] keys were the
only numerics skipping it. The interval is also clamped, since nothing
assigns a meaning to a zero or negative autosave.
quote_position now warns on an unrecognised value, matching sync_on_exit,
language and date_format; the only silent fallbacks in this file are for
absent keys rather than malformed ones. And a missing `sent` folder is a
notice rather than a problem, because the spec blesses that configuration
and a modal on every launch for a permanently correct setup is how users
learn to dismiss dialogs unread.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
Task 2's code review found that the README's sample config documents every
other key, including recently added ones, and has nothing for send_command
or the [compose] section. Without it those keys ship undiscoverable: a user
has no way to learn that sending exists at all.
That is a gap in the plan rather than a deviation by the task, since no task
claimed the README, so it becomes a step in the close-out where the rest of
the documentation is written.
|
|
DraftStore needs the same filename generation moveMessages() already has,
and duplicating it would duplicate a correctness property rather than a
convenience: the comment records that carrying mbsync's ,U= infix across a
folder boundary produced 'Maildir error: duplicate UID' on real mail.
A pure move with no behaviour change, committed on its own so a bisect can
tell it apart from the feature that needed it. The function gains its own
tests, including the UID-infix case that previously had none.
|
|
The header still credited CMARK_OPT_SAFE after the .cpp comment and the test
were corrected, which left the wrong mechanism named in the file
MessageBuilder's author will actually read.
Three test weaknesses, each measured rather than assumed. The accented-text
test survived a SYMMETRIC latin-1 mutation, since the round trip cancels for
codepoints under U+0100, so it now carries a character latin-1 cannot
represent. The tasklist test asserted on the bare word "checked", which
ordinary prose would satisfy, and now asserts the attribute. And the
extension registration is wrapped in a function-local static: cmark-gfm's
registry has no once-guard, and this project has a worker thread, so the
first call racing itself would tear the registry rather than crash cleanly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
An account's ability to send IS its send_command's presence. Not a separate
receive_only key: with one key there is nothing to keep in step and nothing
to contradict, and a receive-only account is expressed by omission, which is
how one real account here is meant to work.
Startup validation follows the startup_query pattern, and is deliberately
asymmetric. A default_account that cannot send is warned about, because the
user named an account and expects mail to come from it. An installation
where NO account can send is not: that is a valid read-only installation,
and warning about it would train the user to ignore warnings.
Every [compose] key reads through value(key, default) rather than testing
contains(), because send_delay_ms = 0 is a real setting meaning 'send at
once' that a zero-test would mistake for unset.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
CMARK_OPT_SAFE has had no effect since cmark-gfm made safe mode the
default; the flag is retained for API compatibility and the real
protection is that CMARK_OPT_UNSAFE is never set. Measured against
0.29.0.gfm.13: rendering with OPT_DEFAULT alone, with OPT_SAFE, and with
OPT_UNSAFE shows the first two suppress a script element and a
javascript: link while the third leaks both.
The comment credited the flag, which would have sent the next reader to
the wrong place, and the test could not tell the two apart: it would
have passed just as well with the flag deleted. What it has to guard
against is OPT_UNSAFE being introduced, so it now also asserts that
unsafe links are stripped, which is a protection this gets for free and
previously asserted nothing about.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
The composer's body is markdown and the text/html part is generated from it.
cmark-gfm rather than plain cmark for autolink: under CommonMark a bare URL
in a mail body is not a link, and in mail it is expected to be clickable.
Three extensions are enabled and tables are deliberately not, since they
render badly across mail clients whoever generates them. Raw HTML in the
input is suppressed with CMARK_OPT_SAFE: the body is the user's own text,
but a body that can inject markup into its own generated HTML part is a
sharp edge with no upside.
The build needs TWO lookups. Only the core library ships a pkg-config file;
libcmark-gfm-extensions has none and is located with find_library, the way
notmuch already is. All three extensions live in that second library, so
finding only the first produces a build that compiles and silently renders
plain CommonMark.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
Thirteen tasks, ninety-nine steps, against the spec committed earlier on this
branch. Written on master so it is readable from either branch; the
implementation goes on compose-and-send.
Every API assumption was verified against this machine rather than written
from memory, which found five things the spec had wrong or unstated:
libcmark-gfm-extensions ships NO pkg-config file although libcmark-gfm does,
so CMake needs find_library beside pkg_check_modules. All three enabled
extensions live in that second library, so finding only the first yields a
build that compiles and silently renders plain CommonMark.
GMime defaults to iso-8859-1, emits no Date or Message-ID unless asked, and
g_mime_text_part_set_text() encodes with whatever charset is set when it is
called, so setting the charset afterwards produces a part labelled utf-8
carrying latin-1 bytes. All three fail only on accented text, which for this
user is every message. The plan builds the content stream directly and
carries a working probe's output as evidence.
MessageNode has no body or date field, so quoting takes a ParsedMessage.
ThreadListModel::messageScopeFor() takes a QModelIndexList rather than a
single index. There is no Config::maildirPath(): the mail root comes from
notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT) via a file-static helper in the
worker, and item 124 records that composing a destination from the wrong
root would write into the Xapian tree.
Two spec statements are corrected in the plan rather than followed. It calls
for a new top-level Message menu and one already exists at
mainwindow.cpp:1156. And it requires a shortcut per action, which item 132
changed while this was being planned, so save_message ships without one.
|
|
everyActionHasAShortcut() was written when the action list was short and
every action plausibly deserved a chord. Item 123 adds six more, and under
that rule each one consumes a key sequence whether or not anyone would ever
press it. Rarely-used actions were being given chords to satisfy a test
rather than because a user wanted them.
everyActionIsReachableFromAMenu() is the rule that actually matters, and it
already has the right shape: it is what stops an action shipping invisible,
which is the defect item 103 found when `restore` was reachable by a chord
and by nothing a user could see. Discoverability comes from the menu. A
shortcut is an accelerator for the things done often.
Nothing replaces the deleted test and nothing else needed changing:
showShortcutReference() already prints `(unbound)` for an empty sequence, so
the code anticipated this and only the test forbade it.
Verified rather than assumed: with `tag_rules` unbound in defaultBindings(),
an action that is registered, menu-reachable and carries an icon but has no
chord at all, the full suite passes. Before this commit it failed.
CLAUDE.md's "adding an action is FIVE places" paragraph is updated, including
its count of how many are test-enforced, which drops from four to three.
|
|
The seven brainstorm commits are documentation of decisions already taken:
the spec at docs/superpowers/specs/2026-08-20-compose-and-send-design.md,
the rewritten item 123 pointing at it, and the seven backlog items the
brainstorm opened (128 to 134). No implementation of 123 exists yet, so
nothing premature reaches master.
They are merged now because item 134 shipped to master as af902e0 while the
row defining it sat on the branch, which left master carrying code for an
item its own backlog had no record of. Item 132 is the same shape and is
next.
|
|
The status bar's sync bar was a bare QProgressBar configured inline in
MainWindow, and item 123's send popup needs the same thing again. It also
needs the half MainWindow does not use: the popup drains a determinate bar
through its cancellable countdown and switches THE SAME widget to
indeterminate when send_command starts and the duration stops being
knowable. Building that inline a second time is what this removes.
BusyIndicator carries both modes. setBusy() resets the range as well as the
visibility, so the switch out of the countdown cannot leave the bar drawing
its last fraction, and setProgress() treats a total of zero as busy rather
than passing it through: setRange(0, 0) IS the indeterminate range, so a
zero total would otherwise hand the caller an animating bar while it
believed it had drawn an empty one.
Only the bar is extracted, not the status label the backlog row mentions
beside it. m_statusLabel has 34 uses across MainWindow for transient
messages, selection counts and sync phases; it belongs to the window rather
than to the indicator, and the send popup owns its own phase text.
The hidden-on-construction test needs a shown parent, which cost a mutation
to find. Measured against a standalone Qt program: a parentless widget
reports isVisible() false and isHidden() true whether or not hide() was ever
called, so both obvious assertions passed against a constructor with the
hide() deleted. What differs is WA_WState_ExplicitShowHide, and the
behaviour it produces appears only once a parent is shown, which is how the
status bar holds this widget.
All five mutations checked and killed: the zero-total guard, the range reset
in setBusy(), the value clamp, the show() in setProgress() and the hide() in
the constructor.
|
|
Three rows in every state, so nothing reflows: status label, bar,
right-aligned Undo. The bar changes mode rather than place, determinate
and draining during the countdown because that has measurable progress,
indeterminate once send_command starts because a send does not.
Undo stays visible after it disables. A control that vanishes re-lays
out the popup mid-operation, and a greyed one says why cancelling is no
longer possible where an absent one looks like it was never offered.
The status label sizes from the longest string it can hold in the
current language rather than from its content: Italian "Rimozione della
bozza..." is longer than "Removing draft...", so a content-sized label
resizes the popup between stages, which is the jumping the fixed layout
exists to prevent.
Item 134 gains a requirement from this: the extracted widget must expose
both bar modes, not only the indeterminate one MainWindow happens to
need today.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The user asked for Gmail's undo-send, and it answers a question the
spec had left open: what Cancel means during a send.
It means nothing, if offered while send_command is running. Killing an
SMTP client mid-transaction leaves an unknown send, since the message
may have reached the server in full before the kill, and that is worse
than either clean outcome. Moving the cancel window before the command
starts makes Undo mean genuinely nothing happened.
The popup owns the whole operation, countdown through completion,
rather than a countdown popup handing over to a status bar. One widget
changing state in one place, and it keeps the eye-catching surface the
user asked for. Modal to the composer only, so a second composer and
the main window stay usable. No close button and no Escape: during the
countdown a dismissal cannot say whether it means cancel or send now.
send_delay_ms defaults to 5000, and zero skips it.
The test for this asserts a negative: Undo leaves the stub command never
run. A test asserting only that the composer reopened would pass against
a design that ran the command and discarded the result, which is exactly
what the delay exists to prevent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Two corrections from the user, both of which the spec had wrong.
A failed sent-copy write was put in the main window's status bar, on
the reasoning that the composer closes so the message needs somewhere
persistent. Wrong instinct: the fix for "the window is gone" is a
dialog, not a quieter surface. It is the one failure here that produces
a silent divergence between what the recipient received and what the
local archive holds, and nobody discovers that from a line that showed
for a few seconds. It gets a modal.
A failed autosave stays in the composer but as a persistent banner
rather than a status-area line, since the quit path already escalates
that state to a dialog and depends on it surviving.
Stated as a rule at the head of the section, because the user's point
was general: modal for silent divergence, banner for mid-task, status
bar only for what is already obvious.
Second correction: the composer's busy indicator is not built inline. A
second instance of MainWindow's progress-bar-plus-label pairing is
where a widget class earns itself, and "this codebase builds small UI
inline" describes what the code does rather than justifying repeating
it. Item 134 extracts it and converts MainWindow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The spec said "disabled with a spinner" without saying where, which
leaves a popup as a reasonable reading of it. A popup is wrong here: it
would be modal over a window that is already disabled, and it can be
dismissed while the operation continues, which is the indicator
ambiguity items 18, 19, 28 and 54 each closed once.
Progress goes in the composer's own status bar, through the three
stages the operation actually has, since a failure filing the sent copy
means something different from a failure sending. The window closing is
the success message.
The indicator is an indeterminate QProgressBar built inline, matching
MainWindow's m_syncProgress rather than factoring out a shared widget:
this codebase builds small UI inline, and two progress bars do not
justify a third class.
Also settles what the staged display implies for a sent-copy write that
fails after a successful send: the composer still closes, because
holding it open for a message already sent invites sending it twice,
and the warning goes to the main window's status bar instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The spec said "the editor is plain text" and left it there, which reads
as "you are on your own with the syntax". Storage format and editing
affordances are separate decisions and only the first was stated.
The toolbar is text transformation over the markdown source, not
rich-text editing: bold, italic, code, strikethrough, link and quote,
selection-aware, with the cursor landing between the tokens when there
is no selection.
Its shortcuts belong to the composer window's own scope and do not
touch KeyMap, which matters for item 132: the two namespaces should not
be conflated when that rule is revisited.
Live syntax highlighting is a follow-up (item 133) rather than part of
this: agreeing with the grammar about nesting and about code spans is
the expensive half, and it is better judged after living with the
toolbar.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The spec called it a new dependency needing a SlackBuild REQUIRES entry.
It is a new dependency, but /var/log/packages/ shows
cmark-gfm-0.29.0.gfm.13-x86_64-3 with no _danix tag, so it is stock and
REQUIRES lists only non-stock dependencies.
Also records the staleness cost accepted with it: cmark-gfm tracks an
older CommonMark base (0.29 era) than the stock plain cmark (0.31.2).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Brainstormed with the user. Design only, no code, which is what the
item's #plan-only tag asked for.
The decision that shaped everything: there is no MTA on the machine, so
"an external script on the same model as mailsync.sh" had no model to
copy. Send becomes a per-account send_command taking the message on
stdin, exactly as [sync] command already works, which keeps the
no-network-protocol rule intact without naming an MTA.
An account with no send_command is receive-only by construction, which
is how one of the five accounts is meant to work. Reply, reply-all and
forward are disabled on its mail behind a ribbon that says why.
The body is markdown parsed by cmark-gfm rather than a hand-written
parser for a limited set: the two share no code, so the small one is
deleted wholesale the moment the set widens.
Four new units, three of them widget-free and testable without a
painter. MessageSender is deliberately a separate unit rather than a
method on the composer, so a future outbox wraps the funnel instead of
reworking it.
Item 123's section is replaced by a pointer to the spec, per this
document's own rule for a fully specified item. The brainstorm opened
items 128 to 132, including a review of the every-action-has-a-shortcut
rule, which the user raised: six more actions takes it past the point
where a chord for everything is useful.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The entry said sending should be an external script on the same model as
mailsync.sh. Measured on the machine, there is no such model to copy: the
fetch side has mbsync and the send side has nothing.
No MTA is installed at all, msmtp and sendmail are both absent, and neomutt
sends over its own built-in SMTP configured per account in
~/.config/neomutt/accounts/*.rc. So the working setup this application
mirrors has no external send path either.
That makes the first question a non-UX one, ahead of everything the note
lists: sending needs either an MTA the user chooses to install and
configure, which is the only shape that keeps the no-network-protocol rule
intact, or a decision to relax that rule. Recorded so the brainstorm does
not assume the tidier answer, since installing and configuring an MTA is
work he has not asked for and the credentials already exist elsewhere.
Also notes that all five accounts already configure a drafts folder, so the
draft half has somewhere to live before anything is decided.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Not a regression and not something we removed: Save image is item 114,
still open. Item 127 removed Save LINK and deliberately left this one.
The circumstances the user reported sharpen it twice. The image had already
had its remote content loaded, so m_allowRemote was still true at the click.
That flag is live on the shared interceptor and cleared by the next
showThread(), so a download handler is subject to whatever it says at the
moment of the click rather than at render time. A naive handler therefore
looks perfect in exactly this case and fails once the grant is gone, which
makes "it worked when I tried it" worthless as evidence. The entry records
that both cases must be tested against a message whose grant has been
cleared.
The second is a corollary of item 127. downloadRequested is per-profile, so
connecting it lights up every download entry Chromium offers at once,
including the Save link just removed from the menu. An entry being absent
from a menu is not the same as the capability being absent, so the handler
must decide per request rather than merely exist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Reported by hand after the item 127 fix: right-clicking a link still
offered Save link.
It had been deferred to item 114 alongside Save image, on the grounds that
both are inert without a downloadRequested handler. That is true and it was
the wrong conclusion, because the two are not the same question.
Save image is content the message already carries, and item 114 is about
making it work. Save link fetches a remote URL chosen by the sender,
through the pane's profile, which is the one profile in this application
that must never fetch remote content: that is what m_allowRemote and the
interceptor exist to prevent. Answering it with a download handler would
put a network fetch of attacker-controlled content behind one context-menu
entry.
Saving what the user actually wants already has a path that never touches
the network: saveAttachment(), which writes a MIME part already parsed into
memory and sanitises the filename.
So it is removed rather than implemented, and the test asserts its absence.
Item 114 now carries the constraint that follows: a downloadRequested
handler added to make Save image work must not make Save link reachable
again, which the natural per-profile implementation would do by default.
Mutation checked: dropping the entry from the filter fails the test with
"a link action survived: Save link".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Items 126 and 127, in one sitting because the second is only safe after
the first.
126: an anchor carrying target="_blank" did nothing when clicked, with no
error and nothing on screen. Chromium routes such a click to
QWebEnginePage::createWindow() rather than to acceptNavigationRequest, and
MessagePage did not override it, so the base implementation returned
nullptr and the URL was discarded before any of our code saw it. Plain
anchors were unaffected and already worked, which is why this presented as
"HTML mail is broken" while a text mail's links opened: marketing HTML sets
_blank on practically every anchor.
createWindow() receives a WebWindowType and no URL, so an override cannot
simply read the target: it arrives afterwards as a navigation on whatever
page is returned. LinkRelayPage is that page. It has no view, hands the URL
to the same handler the plain-link path uses, refuses the navigation, and
deletes itself. Nothing is ever fetched and no second QWebEngineView is
created.
127: OpenLinkInNewTab, OpenLinkInNewWindow and OpenLinkInThisWindow join
removeBrowserActions()'s list. Item 100's list is the PAGE actions and was
tested by right-clicking the page; these appear only over a link, so it
never saw them. CopyLinkToClipboard stays, being the fallback for any link
that will not open. The order matters: 126 gives the page a working
createWindow(), so those entries would have stopped being dead and started
opening links into a tab that does not exist.
Testing needed two seams. The click cannot be synthesised, since JavaScript
is off in this profile (measured: runJavaScript returns an invalid QVariant)
and a synthetic press would depend on the anchor's rect and the desktop's
fonts; setUrl() is no substitute because it arrives as NavigationTypeTyped.
clickLinkForTest() and relayBlankTargetForTest() drive the real overrides on
the real page, and setLinkOpener() substitutes a recorder for
QDesktopServices::openUrl.
Both routes are asserted rather than only the broken one, since they share a
handler now. Three mutations checked and caught, including the filter also
removing CopyLinkToClipboard, which a later sweep of "dead link actions"
would otherwise take silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
The first diagnosis was wrong and the user's own follow-up disproved it: a
plain-text GitHub mail opens its links correctly while an HTML newsletter
does not. If RequestInterceptor blocking https were the cause, neither
would work.
Verified against the two messages named. The difference is target="_blank".
An anchor with no target navigates the main frame and reaches
acceptNavigationRequest, which hands it to QDesktopServices::openUrl; that
path works today. An anchor asking for a new window is routed by Chromium
to QWebEnginePage::createWindow(), which MessagePage does not override, so
the base implementation returns nullptr and the click is discarded before
any existing code observes it. Marketing HTML uses _blank almost
universally, which is what makes it read as "HTML mail is broken".
Both messages render HTML, so this was never a text-versus-HTML
distinction: the GitHub mail is multipart/alternative and its HTML part is
what the pane shows.
The entry also drops the proposal to let main-frame navigations through the
interceptor. That would have weakened the remote-content protection to fix
something it was not causing. Nothing here needs m_allowRemote relaxed: the
URL goes to an external browser and the pane fetches nothing.
Records the trap that decides the fix's shape: createWindow() receives no
URL, only a WebWindowType, so an override returning nullptr discards the
target before it can be read.
Item 127 is updated to match. OpenLinkInNewTab and OpenLinkInNewWindow fail
through the same missing createWindow(), so fixing 126 may make them start
working, which is worse rather than better.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Two defects found by hand, related but separate.
126: clicking a link in a message does nothing. The handler is already
there and correct, calling QDesktopServices::openUrl from
acceptNavigationRequest, and it has presumably never run. RequestInterceptor
denies http and https whenever m_allowRemote is false, which is the default
for every message, and it runs on the request before the page is asked
whether to accept the navigation. The click is dropped at the network layer
with no error, no navigation and no browser. That is the remote-content
protection working as designed; the bug is that a deliberate click is
indistinguishable from a resource the document fetched itself, at the layer
where the decision is currently made.
127: a link's context menu still offers Open in new tab, Open in new
window, Save link and Copy link. Item 100 removed the page-level actions
and its list names four of them; the link actions are different WebAction
values that Chromium adds only over a link, so item 100 never saw them.
Two are dead (there are no tabs, and a second view is deliberately never
created), one belongs with item 114's missing download handler, and Copy
link works and is currently the user's whole workaround for 126. It
follows 126, since a working click makes one "Open link" entry the right
answer rather than a removal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Item 124 shipped and is proven: the index moved from a 7200rpm platter to
NVMe with the mail staying at /data/Mail. Cold start went from 38618 ms to
668 ms for a complete walk, and 2008 ms to 50 ms for the first rows.
Counts held at 49174 messages / 5594 inbox / 100 tags at every step, and
Delete then Restore round-tripped through the account's trash by hand.
Item 121 stays open, and its entry now says why. The measured platter
figures are the evidence FOR building the indicator, not against it: a
mechanical disk is the cheap configuration, not an exotic one, and a user
with a large Maildir on spinning rust has nowhere to migrate to. Fixing
one developer's hardware is not fixing the application. The constraint
that pointed at item 124 as the answer is replaced by one saying the
opposite, and prefaulting stays rejected on its own merits since it is
worst on the low-memory machines most likely to have a slow disk.
Item 125 is new, found by hand during the migration. mailsync.sh exits 75
(EX_TEMPFAIL) when another run holds the lock, and the sync indicator
never clears; because an edit made during a sync is held until the sync
ends, a Delete sat queued for a completion that could not arrive and
looked like it had done nothing. Nothing was lost, since held edits reach
the disk, but the user cannot tell that.
Also records in CLAUDE.md that notmuch_database_get_path() is not the mail
root, that database.hook_dir defaults into the index directory and
silently stops post-new under a split config, and that the ordinary
fixture layout cannot tell the two accessors apart.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
notmuch can be configured with `mail_root` and `path` as separate keys,
which puts the Xapian index outside the Maildir. Under that layout
notmuch_database_get_path() returns the INDEX directory, and the worker
treated it as the mail root at four sites.
The consequences are not symmetric. Message paths resolved to `../..`
escapes that match no account prefix, which is a display defect. But
moveMessages() composes its destination from the same root, so Delete
would have written into the Xapian tree: outside the Maildir, invisible
to mbsync, and gone from every other client. That is the stranded-mail
failure of item 103 with a new cause.
notmuch_config_get(NOTMUCH_CONFIG_MAIL_ROOT) is correct under both
layouts, so no conditional is needed. Verified against the live database:
with only `path` set it returns the same string as get_path(), making
this a no-op for the current configuration.
The fixture gains an opt-in splitIndex(). That is load-bearing rather
than convenience: in the ordinary layout the index lives inside the mail
root and both accessors return the same string, so a test written
against it passes whichever one the code uses. All three new tests fail
against the old accessor, confirmed by mutation.
Also records the finding as backlog item 124, and corrects item 121's
timings, which had been copied from item 74 rather than measured. A cold
run seven minutes after boot, with the index verifiably unread, gives
2008 ms to the first rows and 38618 ms to a complete list, against the
642 ms and 5714 ms recorded there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
|
|
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 115's closed section, which the previous commit failed to update: its
python edit asserted on wording that did not match the file and aborted, so the
code shipped and the record did not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The user's preference after seeing item 115 ship: a small transient in the
bottom right of the pane with a checkmark, rather than a status bar message at
the far end of the window. A copy happens in the pane, so the confirmation
belongs there.
Three properties are load-bearing and each has a mutation that fails. The toast
is a hand-placed CHILD rather than a layout item, because it floats over the
message instead of taking a strip away from it: nothing reflows when it appears
and the text just copied does not jump. That is why resizeEvent() is overridden,
since a hand-placed child does not follow its parent. It is autoFillBackground
and painted from the theme's ToolTipBase/ToolTipText, so it stays readable over
a rendered message and follows the desktop theme the way the document already
does. And its timer is restarted rather than started, so a second copy gets its
own full reading time instead of inheriting what is left of the first.
The resize test was wrong on its first draft and passed against the mutation it
exists to catch. It grew the pane, which moves the right and bottom edges away,
so a toast left at its old position still satisfied "inside the pane"; measured
green with the reposition deleted. It shrinks now, where a stale position lands
outside the new rect, which is also what the user would see.
No new strings: the four messages are unchanged, only where they appear.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
mbsync's manual, under "the more efficient default UID mapping scheme": "it is
important that the MUA renames files when moving them between Maildir folders",
and "the general expectation is that a completely new filename is generated as
if the message was new".
qtmaildir is that MUA and did not rename. moveMessages() kept
QFileInfo(from).fileName() verbatim, `,U=<n>` included. That infix is mbsync's
per-folder IMAP UID, so carrying it across a folder boundary makes it a claim
about a folder the file is no longer in; moving a message out and back then
reinserts a UID the server has since reassigned. Reported by the user as
`Maildir error: duplicate UID 1`, and measured on the real Maildir: four
collisions in one folder, eight distinct messages, none lost.
freshMaildirName() regenerates the unique part and keeps ONLY the `:2,<flags>`
suffix. Keeping the flags is not a contradiction of "as if the message was new":
they record seen, flagged and replied, and maildir.synchronize_flags is true, so
dropping them would mark every deleted message unread and lose Important on the
way to the trash.
Two things fell out of the change and both were defects waiting to happen. The
already-in-the-destination guard compared full PATHS, which worked only because
the name was carried across; with a fresh name it can never be true, so a
message already in the destination would be renamed on every move. It compares
directories now. And test_mainwindow's folderHasMessageFile() matched on the
filename stem, so all fifty-odd assertions using it began reporting "the file is
not there" about files that were there. It reads the Message-ID out of each file
instead, which is what those assertions always meant.
Three mutations fail: the old name carried across, the flags dropped, and the
uniqueness counter frozen so two messages moved in one batch collide.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Items 115 and 117, both from the user's notes.
Select all was never in Chromium's menu for this pane, measured by hand with a
selection active and against a build with removeBrowserActions() reverted, so
the filter is not what removed it. MessageView::addPaneActions() supplies it,
static and taking the menu, mirroring removeBrowserActions() beside it. Two
comments claiming the standard menu already offered it are corrected; either
would have sent the next reader down the same three wrong theories the item
records.
The copy entries all worked and none of them said so. Four now report through
the pane's existing statusMessage, each naming what it copied rather than saying
"Copied", which is the item's own constraint when three of them sit together in
one menu. Connected to the page's own QActions, so the report follows the entry
wherever it is triggered from.
The two differ in what can be tested, and the tests say so rather than papering
over it. The copy path is fully covered: triggering the action runs the
production path, and mutations for a duplicated message and an unwired entry
both fail. addPaneActions() is covered, but showBodyContextMenu() CALLING it is
not and cannot be, since createStandardContextMenu() returns nothing outside a
real context-menu event; a mutation deleting that call leaves the suite green,
measured. The call site is a hand test and the test file records that so nobody
adds an assertion that appears to cover it.
The copy strings are QT_TR_NOOP inside an array, which CLAUDE.md warns extracts
nothing at file scope. Verified rather than assumed: lupdate found all four
under the MessageView context, because the array sits inside a member function.
387 finished, 0 unfinished.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Two entries in the notes had no item here. Both causes verified in the code
rather than copied from the note.
Item 119, the unsynced-changes count being clickable, is bigger than it reads.
pendingEditCount() sums four sources and one of them, m_unnettablePendingEdits,
is a bare int by design: it counts confirmed changes carrying no message ids, so
a dialog built from what is currently kept can list three groups and then owes
the user a remainder it cannot describe.
Item 120 goes to the deferred table, matching where the user filed it. It is not
plannable as it stands: nothing records which rule tagged a message, so the
information does not exist to display, and creating it means the post-new hook
storing something per message, which is a shared-format change across both
repos.
Everything else in the notes maps to an existing item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
