feat(compose): derive a reply's recipients and headers, item 123

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

1 files changed, 51 insertions, 4 deletions

diff --git a/src/messagebuilder.cpp b/src/messagebuilder.cpp
index f27c3c2..42a0e31 100644
--- a/src/messagebuilder.cpp
+++ b/src/messagebuilder.cpp
@@ -110,6 +110,35 @@ GMimePart *makeTextPart(const char *subtype, const QString &text)
 /// rejects what GMime cannot parse at all; it is not an address validator, and
 /// a typo that happens to be parseable still goes out.
 bool setAddressHeader(GMimeMessage *message, const char *header, const QStringList &addresses,
+                      QString *badEntry);
+
+/// A message-id in the angle brackets the wire format requires, added if the
+/// caller did not supply them.
+///
+/// **The brackets are syntax, not decoration, and GMime enforces it by writing
+/// an EMPTY HEADER for a bare addr-spec rather than by complaining.** Measured
+/// 2026-08-21: `In-Reply-To: current@example.org` emits `In-Reply-To:` with no
+/// value, so the reply arrives as an orphan thread in the recipient's client
+/// while nothing looks wrong locally.
+///
+/// Bracketing lives HERE, in the one function that composes these headers,
+/// rather than in each caller. Every source of a message-id in this application
+/// hands over a bare one: GMime strips the brackets when MimeParser reads
+/// `Message-ID`, and `ComposeContextBuilder::referencesForReply` strips them
+/// again from the References chain so the two agree. A convention spread across
+/// callers is one a later caller gets wrong, and the failure is invisible
+/// without inspecting a sent message.
+QString bracketed(const QString &messageId)
+{
+    const QString id = messageId.trimmed();
+    if (id.isEmpty())
+        return {};
+    if (id.startsWith(QLatin1Char('<')) && id.endsWith(QLatin1Char('>')))
+        return id;
+    return QLatin1Char('<') + id + QLatin1Char('>');
+}
+
+bool setAddressHeader(GMimeMessage *message, const char *header, const QStringList &addresses,
                       QString *badEntry)
 {
     if (addresses.isEmpty())
@@ -238,12 +267,30 @@ Result build(const OutgoingMessage &message, const Account &account)
     const QByteArray subject = message.subject.toUtf8();
     g_mime_message_set_subject(mime, subject.constData(), "utf-8");
 
-    if (!message.inReplyTo.trimmed().isEmpty()) {
-        const QByteArray value = message.inReplyTo.trimmed().toUtf8();
+    // Both headers are bracketed HERE rather than by the caller. See bracketed()
+    // for why, and for what a bare id costs.
+    const QString inReplyTo = bracketed(message.inReplyTo);
+    if (!inReplyTo.isEmpty()) {
+        const QByteArray value = inReplyTo.toUtf8();
         g_mime_object_set_header(GMIME_OBJECT(mime), "In-Reply-To", value.constData(), "utf-8");
     }
-    if (!message.references.isEmpty()) {
-        const QByteArray value = message.references.join(QLatin1Char(' ')).toUtf8();
+    QStringList references;
+    for (const QString &id : message.references) {
+        const QString bracketedId = bracketed(id);
+        // An empty entry contributes nothing rather than a stray "<>": the
+        // References header is a run of ids, and one malformed entry is enough
+        // for a strict parser to discard the whole chain.
+        //
+        // Defensive rather than a path with a fixture behind it, like the
+        // length check in setAddressHeader above: referencesForReply() already
+        // drops empty ids, so a mutation on this line SURVIVES the suite.
+        // Measured 2026-08-21. Kept because it costs one comparison and the
+        // failure it covers is a silently broken thread.
+        if (!bracketedId.isEmpty())
+            references.append(bracketedId);
+    }
+    if (!references.isEmpty()) {
+        const QByteArray value = references.join(QLatin1Char(' ')).toUtf8();
         g_mime_object_set_header(GMIME_OBJECT(mime), "References", value.constData(), "utf-8");
     }