mirror of
https://github.com/warmbly/warmbly.git
synced 2026-10-28 00:02:31 +00:00
Ralf KleinandClaude Opus 5
3cef4e8268
imapStore fell back to Reply-To as the thread parent whenever a message carried no In-Reply-To. Reply-To is an address header -- send replies to this mailbox -- not a message identifier, so the thread key became a person's address and every message that sender ever sent collapsed into one strand. Measured on a production instance with 38 IMAP mailboxes: 484 of 1431 stored messages sat in 62 threads. One reporter's 76 unrelated DMARC aggregate reports arrived as a single 76-message conversation, and one mailbox's sequence tests, compose tests and live outreach merged into another. The remaining 947 messages, which did carry In-Reply-To, were threaded correctly. Sequence safety was never affected: stop_on_reply matches In-Reply-To against the task rather than the thread. The damage is to the unibox, where replies are triaged. A message that answers nothing now has no parent and roots its thread on its own Message-ID, which is what the existing else branch already does. The test fails if the fallback is reintroduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
81 lines
2.3 KiB
Go
81 lines
2.3 KiB
Go
package wmail
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"github.com/warmbly/warmbly/internal/models"
|
|
)
|
|
|
|
// threadParentID decides which conversation an inbound message joins. A wrong
|
|
// answer does not lose the message, it files it under the wrong conversation,
|
|
// which is harder to notice and worse to work in.
|
|
//
|
|
// The case this pins: Reply-To as a fallback parent. It is an address header,
|
|
// not a message identifier, so keying on it puts every message a sender ever
|
|
// sent into one thread.
|
|
func TestThreadParentIDIgnoresReplyTo(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
msg *models.EmailMessageData
|
|
want string
|
|
}{
|
|
{
|
|
name: "a genuine reply threads on In-Reply-To",
|
|
msg: &models.EmailMessageData{InReplyTo: []string{"<parent@example.com>"}},
|
|
want: "<parent@example.com>",
|
|
},
|
|
{
|
|
name: "the last In-Reply-To wins on a deep chain",
|
|
msg: &models.EmailMessageData{InReplyTo: []string{
|
|
"<root@example.com>", "<latest@example.com>",
|
|
}},
|
|
want: "<latest@example.com>",
|
|
},
|
|
{
|
|
name: "Reply-To alone is not a parent",
|
|
msg: &models.EmailMessageData{ReplyTo: []string{"reports@example.net"}},
|
|
want: "",
|
|
},
|
|
{
|
|
name: "Reply-To never overrides In-Reply-To",
|
|
msg: &models.EmailMessageData{
|
|
InReplyTo: []string{"<parent@example.com>"},
|
|
ReplyTo: []string{"someone@example.net"},
|
|
},
|
|
want: "<parent@example.com>",
|
|
},
|
|
{
|
|
name: "a message that answers nothing has no parent",
|
|
msg: &models.EmailMessageData{},
|
|
want: "",
|
|
},
|
|
{
|
|
name: "nil is not a panic",
|
|
msg: nil,
|
|
want: "",
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
if got := threadParentID(tc.msg); got != tc.want {
|
|
t.Errorf("threadParentID() = %q, want %q", got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// Two unrelated messages from one sender must not share a parent, which is the
|
|
// shape of the bug: same Reply-To, different conversations.
|
|
func TestThreadParentIDKeepsUnrelatedMessagesApart(t *testing.T) {
|
|
sender := []string{"Aggregate Reports <reports@example.net>"}
|
|
first := &models.EmailMessageData{MessageID: "<a@example.net>", ReplyTo: sender}
|
|
second := &models.EmailMessageData{MessageID: "<b@example.net>", ReplyTo: sender}
|
|
|
|
if p := threadParentID(first); p != "" {
|
|
t.Fatalf("first message got parent %q, want none", p)
|
|
}
|
|
if p := threadParentID(second); p != "" {
|
|
t.Fatalf("second message got parent %q, want none", p)
|
|
}
|
|
}
|