diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index 9d0c1d6b..6bee5550 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -573,6 +573,23 @@ func orderExpr(orderBy string) string { return orderColumns["date"] } +// dedupedMessagesSQL is 1248/AC1, AC2, AC4-AC7: a FROM-clause subquery, aliased "message" so the caller needs no +// further change, that answers each (account, message-id) once among the rows whereSQL matches: grouped by +// account and message_id, except an empty message_id (AC5) is never grouped with anything else — each such row +// gets its own one-row group, keyed by its own (account, folder, uid) instead, which is already unique. Within a +// group the copy outside archiveFolderNames wins (AC1); only when every copy is archived does the archive copy +// answer (AC2). Callers skip this (keep the plain "message where ...") when a folder is named (AC3): every mail +// of that folder answers, never merged with anything else. +func dedupedMessagesSQL(whereSQL string, param func(any) string) string { + archiveParam := param(lowerStrings(archiveFolderNames)) + return `(select distinct on (account, dedup_key) ` + messageColumns + ` from ( + select *, + case when message_id = '' then 'u:' || folder || ':' || uid::text else message_id end as dedup_key, + (lower(folder) = any(` + archiveParam + `)) as dedup_archived + from message where ` + whereSQL + ` + ) m order by account, dedup_key, dedup_archived asc, uid asc) message` +} + func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { limit := q.Limit if limit <= 0 { @@ -590,7 +607,13 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { if q.OrderBy == "" || q.OrderDesc { dir = "desc" } - sqlText := "select " + messageColumns + " from message where " + whereSQL + + // 1248/AC1-AC7: with no folder named, order/limit/group-by apply to the de-duplicated set (dedupedMessagesSQL); + // a named folder (AC3) keeps the plain scan. + fromSQL := "message where " + whereSQL + if q.Folder == "" { + fromSQL = dedupedMessagesSQL(whereSQL, param) + } + sqlText := "select " + messageColumns + " from " + fromSQL + " order by " + orderExpr(q.OrderBy) + " " + dir + ", account, folder, uid limit " + param(limit) rows, err := s.pool.Query(ctx, sqlText, args...) if err != nil { @@ -620,6 +643,12 @@ func (s *PgStore) GroupBySender(ctx context.Context, q Query) ([]SenderGroup, er args = append(args, v) return fmt.Sprintf("$%d", len(args)) } + // 1248/AC4: with no folder named, group by sender over the de-duplicated set (dedupedMessagesSQL), so a mail + // held in two folders counts once; a named folder keeps the plain scan, unchanged. + fromSQL, groupWhere := "message", whereSQL + if q.Folder == "" { + fromSQL, groupWhere = dedupedMessagesSQL(whereSQL, param), "true" + } // Review round 1 finding 6: grouped by lower(address) (not case-sensitively, as a mixed-case repeat of the // same address would otherwise split into two lines); name and address are the newest mail's own // (array_agg ordered by date, its first element), never max()'s lexicographically largest, which is neither. @@ -628,7 +657,7 @@ func (s *PgStore) GroupBySender(ctx context.Context, q Query) ([]SenderGroup, er sqlText := `select address, name, count, newest from ( select (array_agg(el->>'address' order by date desc))[1] as address, (array_agg(el->>'name' order by date desc))[1] as name, count(*) as count, max(date) as newest - from message, jsonb_array_elements(sender) el where ` + whereSQL + ` group by lower(el->>'address') + from ` + fromSQL + `, jsonb_array_elements(sender) el where ` + groupWhere + ` group by lower(el->>'address') ) grouped order by count desc, lower(address) asc limit ` + param(limit) rows, err := s.pool.Query(ctx, sqlText, args...) if err != nil { diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go index bc8bdde8..5b19dbfb 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -116,6 +116,21 @@ func searchFixtures() []Message { Date: time.Date(2026, 8, 2, 7, 0, 0, 0, time.UTC), From: []Person{{Address: "me@gmail.example"}}, To: []Person{{Address: "someone@example.org"}}, Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, Body: "will send the claim form soon", Size: 350}, + // gmail/INBOX, a mail gmail also keeps a copy of in [Gmail]/All Mail (1248/AC1, AC4) ---------------------- + {Account: "gmail", Folder: "INBOX", UID: 5, MessageID: "", Subject: "dashboard digest", + Date: time.Date(2026, 10, 17, 7, 0, 0, 0, time.UTC), From: []Person{{Name: "Dashboard", Address: "dash@example.org"}}, + To: []Person{{Address: "me@gmail.example"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "weekly digest", Size: 500}, + + // gmail/[Gmail]/All Mail (1248/AC1-AC4): gmail's archive, holding a second copy of every mail. UID 100 is + // gdup1@gmail's own second copy (same message-id as the INBOX one above); UID 101 is a mail archived out + // of the inbox, living only here. + {Account: "gmail", Folder: "[Gmail]/All Mail", UID: 100, MessageID: "", Subject: "dashboard digest", + Date: time.Date(2026, 10, 17, 7, 0, 0, 0, time.UTC), From: []Person{{Name: "Dashboard", Address: "dash@example.org"}}, + To: []Person{{Address: "me@gmail.example"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "weekly digest", Size: 500}, + {Account: "gmail", Folder: "[Gmail]/All Mail", UID: 101, MessageID: "", Subject: "old newsletter", + Date: time.Date(2026, 7, 1, 7, 0, 0, 0, time.UTC), From: []Person{{Name: "Newsletter", Address: "news@example.org"}}, + To: []Person{{Address: "me@gmail.example"}}, Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, Body: "monthly digest", Size: 400}, + // mesh/Junk (review round 1 finding 2): someone@example.org, written to only by gmail's Sent, not mesh's. {Account: "mesh", Folder: "Junk", UID: 3, MessageID: "", Subject: "a gmail contact's junk mail", Date: time.Date(2026, 10, 11, 6, 0, 0, 0, time.UTC), From: []Person{{Address: "someone@example.org"}}, @@ -633,3 +648,174 @@ func TestSearchFiltersByFromSomeoneIWroteToRecognisesGmailsSentFolderName(t *tes sameSet(t, got, "") }) } + +// 1248/AC1: gdup1@gmail lives in both INBOX and [Gmail]/All Mail with the same message-id. Searched with no +// folder, it is answered once, from INBOX, never from the archive. +func TestSearchWithNoFolderAnswersAGmailMailOnceFromInbox(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + got, err := st.Search(ctx, Query{Accounts: []string{"gmail"}, Words: []string{"digest"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + var found []Message + for _, m := range got { + if m.MessageID == "" { + found = append(found, m) + } + } + if len(found) != 1 { + t.Fatalf("gdup1@gmail answered %d time(s): %+v", len(found), found) + } + if found[0].Folder != "INBOX" { + t.Fatalf("gdup1@gmail answered from %q, want INBOX", found[0].Folder) + } + }) +} + +// 1248/AC2: garchived1@gmail lives only in [Gmail]/All Mail (archived out of the inbox). Searched with no +// folder, it is still answered once, from the archive, since no other copy exists. +func TestSearchWithNoFolderAnswersAnArchivedOnlyGmailMailFromAllMail(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + got, err := st.Search(ctx, Query{Accounts: []string{"gmail"}, Words: []string{"newsletter"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 { + t.Fatalf("garchived1@gmail answered %d time(s): %+v", len(got), got) + } + if got[0].MessageID != "" || got[0].Folder != "[Gmail]/All Mail" { + t.Fatalf("got %+v, want from [Gmail]/All Mail", got[0]) + } + }) +} + +// 1248/AC3: naming the folder [Gmail]/All Mail answers every mail of that folder, never merging it with the +// INBOX copy of the same message-id: gdup1's All Mail copy (uid 100) and garchived1 (uid 101) both come back. +func TestSearchByAllMailFolderNamedAnswersEveryMailOfItUnmerged(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + got, err := st.Search(ctx, Query{Accounts: []string{"gmail"}, Folder: "[Gmail]/All Mail", Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "", "") + }) +} + +// 1248/AC4: group-by-sender with no folder named counts dash@example.org once, not twice, even though gdup1@gmail +// (its only mail) sits in both INBOX and [Gmail]/All Mail. +func TestGroupBySenderWithNoFolderCountsAGmailMailOnce(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + got, err := st.GroupBySender(ctx, Query{Accounts: []string{"gmail"}, From: []string{"dash@example.org"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].Count != 1 { + t.Fatalf("got %+v, want one line with count 1", got) + } + }) +} + +// 1248/AC5: two mails with an empty message-id are never merged with each other or with anything, even though a +// search with no folder is otherwise asked to answer each message-id once. +func TestSearchWithNoFolderNeverMergesEmptyMessageIDs(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + for _, m := range []Message{ + {Account: "noid", Folder: "INBOX", UID: 1, MessageID: "", Subject: "no id one", Date: at1248(9, 0), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "noid", Folder: "INBOX", UID: 2, MessageID: "", Subject: "no id two", Date: at1248(9, 5), + From: []Person{{Address: "b@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + } { + if err := st.Add(ctx, m, nil); err != nil { + t.Fatal(err) + } + } + got, err := st.Search(ctx, Query{Accounts: []string{"noid"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + if len(got) != 2 { + t.Fatalf("two empty-message-id mails merged: %+v", got) + } + }) +} + +// 1248/AC6: the same message-id in two accounts (a mail sent from mesh to gmail, each account indexing its own +// copy) is answered once per account: twice in all, never merged across accounts. +func TestSearchWithNoFolderAnswersTheSameMessageIDOncePerAccount(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + for _, m := range []Message{ + {Account: "mesh", Folder: "Sent", UID: 50, MessageID: "", Subject: "cross-account", + Date: at1248(9, 0), From: []Person{{Address: "jschoubben@novox.be"}}, To: []Person{{Address: "me@gmail.example"}}, + Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "gmail", Folder: "INBOX", UID: 50, MessageID: "", Subject: "cross-account", + Date: at1248(9, 1), From: []Person{{Address: "jschoubben@novox.be"}}, To: []Person{{Address: "me@gmail.example"}}, + Flags: []string{}, Attachments: []AttachmentRef{}}, + } { + if err := st.Add(ctx, m, nil); err != nil { + t.Fatal(err) + } + } + got, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + var meshCount, gmailCount int + for _, m := range got { + if m.MessageID != "" { + continue + } + if m.Account == "mesh" { + meshCount++ + } + if m.Account == "gmail" { + gmailCount++ + } + } + if meshCount != 1 || gmailCount != 1 { + t.Fatalf("mesh: %d, gmail: %d, want 1 each: %+v", meshCount, gmailCount, got) + } + }) +} + +// at1248 is a fixed time for 1248's own dedicated fixtures (AC5-AC7), separate from searchFixtures' at() helper. +func at1248(h, m int) time.Time { return time.Date(2026, 10, 19, h, m, 0, 0, time.UTC) } + +// 1248/AC7: limit 5 over mails that each sit in two folders (INBOX and [Gmail]/All Mail) answers 5 different +// mails, in the asked (newest-first default) order — never fewer because duplicates were counted against the +// limit, and never a duplicate taking one of the 5 slots. +func TestSearchWithNoFolderAndLimitAnswersDistinctMailsInOrder(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + ids := []string{"", "", "", "", "", ""} + for i, id := range ids { + date := at1248(8, 0).Add(time.Duration(i) * time.Hour) + for _, folder := range []string{"INBOX", "[Gmail]/All Mail"} { + uid := uint32(200 + i) + if folder == "[Gmail]/All Mail" { + uid += 1000 + } + m := Message{Account: "manyfolders", Folder: folder, UID: uid, MessageID: id, Subject: "dup " + id, + Date: date, From: []Person{{Address: "dup@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}} + if err := st.Add(ctx, m, nil); err != nil { + t.Fatal(err) + } + } + } + got, err := st.Search(ctx, Query{Accounts: []string{"manyfolders"}, Limit: 5}) + if err != nil { + t.Fatal(err) + } + // Newest first (the default): many6, many5, many4, many3, many2 — many1 is excluded by the limit. + sameOrder(t, got, "", "", "", "", "") + }) +} diff --git a/modules/mesh-mail/cmd/mesh-mail/store.go b/modules/mesh-mail/cmd/mesh-mail/store.go index 4483dc8b..8e9c6d24 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -365,6 +365,11 @@ func (s *memStore) Search(_ context.Context, q Query) ([]Message, error) { out = append(out, m) } } + // 1248/AC1, AC2, AC4-AC7: no folder named, so answer each (account, message-id) once, before order and limit + // are applied to the de-duplicated set; a named folder (AC3) skips this. + if q.Folder == "" { + out = dedupeArchive(out) + } // 1230: order by date (the default), sender, subject or size, ascending or descending, tied-broken on // (account, folder, uid) for a stable order. sort.Slice(out, func(i, j int) bool { return orderLess(out[i], out[j], q.OrderBy, q.OrderDesc) }) @@ -445,12 +450,19 @@ func (s *memStore) GroupBySender(_ context.Context, q Query) ([]SenderGroup, err defer s.mu.Unlock() words := strings.Fields(strings.ToLower(q.Text)) wroteTo := s.wroteToByAccount(q.FromSomeoneIWroteTo) + var matched []Message + for _, m := range s.messages { + if matchesQuery(m, q, words, wroteTo) { + matched = append(matched, m) + } + } + // 1248/AC4: no folder named, so a mail held in two folders (one of them the archive) counts once. + if q.Folder == "" { + matched = dedupeArchive(matched) + } byAddress := map[string]*SenderGroup{} var order []string - for _, m := range s.messages { - if !matchesQuery(m, q, words, wroteTo) { - continue - } + for _, m := range matched { for _, p := range m.From { addr := strings.ToLower(p.Address) g, ok := byAddress[addr] @@ -595,6 +607,51 @@ func isSentFolder(folder string) bool { return false } +// archiveFolderNames (issue 1248, AC1-AC7) are the folders where an account keeps a second copy of every mail +// (gmail's special-use \All, called "[Gmail]/All Mail" in its UI): searched only when named (AC3). A search +// naming no folder answers each (account, message-id) once, preferring the copy outside this list (AC1), and +// falling back to the archive copy only when that is the only one (AC2). Matched case-insensitively, like +// sentFolderNames; a future account naming its archive a third way is for the tool layer to surface, not this +// store. +var archiveFolderNames = []string{"[Gmail]/All Mail"} + +func isArchiveFolder(folder string) bool { + for _, n := range archiveFolderNames { + if strings.EqualFold(folder, n) { + return true + } + } + return false +} + +// dedupeArchive is 1248/AC1, AC2, AC4-AC7: ms (already matching a Query with no Folder named) with each +// (account, message-id) kept once — the copy outside archiveFolderNames, or the archive copy when that is the +// only copy. An empty message-id (AC5) is never merged with anything: every such mail is its own group, keyed by +// its own (account, folder, uid), which msgKey already makes unique. The order of ms is otherwise kept +// (Search/GroupBySender sort or group afterwards), so which duplicate "wins" a tie between two non-archive +// copies (never asked for by an acceptance criterion) is decided by content, not by position. +func dedupeArchive(ms []Message) []Message { + type groupKey struct{ account, id string } + kept := map[groupKey]int{} // groupKey -> index into out + var out []Message + for _, m := range ms { + id := m.MessageID + if id == "" { + id = "\x00" + m.Folder + "\x00" + itoa(int64(m.UID)) + } + k := groupKey{m.Account, id} + if i, ok := kept[k]; ok { + if isArchiveFolder(out[i].Folder) && !isArchiveFolder(m.Folder) { + out[i] = m + } + continue + } + kept[k] = len(out) + out = append(out, m) + } + return out +} + // wroteToByAccount is 1231/AC14's own data: for each account, every address (lowercased) that account has // written to (as a "to" or a "cc") from a folder named one of sentFolderNames. Computed only when needed (asked // is false otherwise), since it walks every message.