From a3476a7bf26bbadce56885102c7cd79e458573a9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:57:26 +0200 Subject: [PATCH] mesh-mail: from-someone-i-wrote-to recognises gmail's Sent folder name too, and stays scoped to one account (issue 1231, review findings 1 and 2) --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 23 +++++++--- .../mesh-mail/cmd/mesh-mail/search_test.go | 43 +++++++++++++++---- modules/mesh-mail/cmd/mesh-mail/store.go | 23 +++++++--- modules/mesh-mail/cmd/mesh-mail/tools.go | 4 +- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index bd06a046..630121fe 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -468,6 +468,16 @@ func globToLike(pattern string) string { return b.String() } +// lowerStrings is 1231/AC14 (review round 1 finding 1): sentFolderNames, lowercased once, to bind as a single +// text[] parameter against lower(m2.folder) = any(...). +func lowerStrings(ss []string) []string { + out := make([]string, len(ss)) + for i, s := range ss { + out[i] = strings.ToLower(s) + } + return out +} + // searchWhere is every filter of Query (everything but its order and its limit), shared by Search and // GroupBySender so the two can never drift apart; every value from the query reaches SQL as one of args' // parameters, never concatenated (only fixed column names and SQL text are built as strings). @@ -524,13 +534,14 @@ func searchWhere(q Query) (whereSQL string, args []any) { where = append(where, "in_reply_to = "+param(q.InReplyTo)) } if q.FromSomeoneIWroteTo { - // 1231/AC14: some sender address of this message is a recipient (to or cc) of a mail in the same - // account's Sent folder (matched case-insensitively by name — "Sent" is the only name any account holds - // today; a future account naming it otherwise is the tool layer's "say what you chose", not this query). - // No value here needs a parameter: the fixed string 'sent' is structural SQL, like the column names. - where = append(where, `exists (select 1 from jsonb_array_elements(sender) s where exists ( + // 1231/AC14 (review round 1 finding 1): some sender address of this message is a recipient (to or cc) of + // a mail, in the SAME account (m2.account = message.account), from a folder named one of sentFolderNames + // ("Sent" for mesh and mediahuis/mailu, "[Gmail]/Sent Mail" for gmail; a future account naming it a third + // way is the tool layer's "say what you chose", not this query). The list reaches SQL as one parameter. + where = append(where, fmt.Sprintf(`exists (select 1 from jsonb_array_elements(sender) s where exists ( select 1 from message m2, jsonb_array_elements(m2.recipients || m2.copied) r - where m2.account = message.account and lower(m2.folder) = 'sent' and lower(r->>'address') = lower(s->>'address')))`) + where m2.account = message.account and lower(m2.folder) = any(%s) and lower(r->>'address') = lower(s->>'address')))`, + param(lowerStrings(sentFolderNames)))) } return strings.Join(where, " and "), args } diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go index 8f7c21bf..625813dd 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -95,10 +95,21 @@ func searchFixtures() []Message { To: []Person{{Address: "me@gmail.example"}}, Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a3", Size: 400, Type: "application/pdf", Name: "schade.pdf"}}, Body: "ik heb verzekering nodig voor de schade aan het dak", Size: 1100}, - // gmail/Sent ---------------------------------------------------------------------------------------------------- - {Account: "gmail", Folder: "Sent", UID: 1, MessageID: "", InReplyTo: "", Subject: "Re: schade melding woning", + // gmail/[Gmail]/Sent Mail (review round 1 finding 1: gmail's live folder name for Sent, not "Sent") ----------- + {Account: "gmail", Folder: "[Gmail]/Sent Mail", UID: 1, MessageID: "", InReplyTo: "", Subject: "Re: schade melding woning", 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}, + + // 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"}}, + To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "unrelated offer", Size: 500}, + + // gmail/Junk (review round 1 finding 1): the same someone@example.org, but here it is gmail's own + // account, whose "[Gmail]/Sent Mail" did write to this address — so this one must match. + {Account: "gmail", Folder: "Junk", UID: 1, MessageID: "", Subject: "a junk mail from a gmail contact", + Date: time.Date(2026, 10, 11, 6, 0, 0, 0, time.UTC), From: []Person{{Address: "someone@example.org"}}, + To: []Person{{Address: "me@gmail.example"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "unrelated offer", Size: 500}, } } @@ -394,13 +405,13 @@ func TestSearchFiltersBySeveralDomainsOrderedAscending(t *testing.T) { ctx := context.Background() addFixtures(t, ctx, st) got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: []string{"example.org", "google.com"}, - OnOrAfter: searchNow.AddDate(0, 0, -90), OrderBy: "date", OrderDesc: false, Limit: 4}) + OnOrAfter: searchNow.AddDate(0, 0, -90), OrderBy: "date", OrderDesc: false, Limit: 5}) if err != nil { t.Fatal(err) } - // junk1 (example.org, Junk) and sec1 (google.com, INBOX) prove both domains and every folder are reached; - // the ascending order (oldest first) proves OrderDesc: false, not merely the absence of a filter. - sameOrder(t, got, "", "", "", "") + // junk1/junk3 (example.org, Junk) and sec1 (google.com, INBOX) prove both domains and every folder are + // reached; the ascending order (oldest first) proves OrderDesc: false, not merely the absence of a filter. + sameOrder(t, got, "", "", "", "", "") }) } @@ -422,7 +433,7 @@ func TestSearchOrdersBySenderAndSubject(t *testing.T) { if err != nil { t.Fatal(err) } - sameOrder(t, bySubject, "", "") // "special offer..." < "you won..." + sameOrder(t, bySubject, "", "", "") // "a gmail..." < "special offer..." < "you won..." }) } @@ -486,7 +497,9 @@ func TestSearchFiltersByInReplyTo(t *testing.T) { } // 1231/AC14: folder Junk and from-someone-I-wrote-to answers only the junk mail whose sender is a recipient of a -// mail in the same account's Sent folder; spam@unknown.com, never written to, is excluded. +// mail in the same account's Sent folder. spam@unknown.com, never written to, is excluded; so is junk3@mesh, +// from someone@example.org, whom only gmail wrote to (gmail/[Gmail]/Sent Mail) and never mesh (review round 1 +// finding 2: the subquery must stay scoped to the same account, not leak another account's Sent folder). func TestSearchFiltersByFromSomeoneIWroteTo(t *testing.T) { eachStore(t, func(t *testing.T, st Store) { ctx := context.Background() @@ -498,3 +511,17 @@ func TestSearchFiltersByFromSomeoneIWroteTo(t *testing.T) { sameSet(t, got, "") }) } + +// 1231/AC14 (review round 1 finding 1): gmail's own Sent folder is named "[Gmail]/Sent Mail", not "Sent"; the +// filter must still recognise it as a Sent folder. +func TestSearchFiltersByFromSomeoneIWroteToRecognisesGmailsSentFolderName(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: "Junk", FromSomeoneIWroteTo: true, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "") + }) +} diff --git a/modules/mesh-mail/cmd/mesh-mail/store.go b/modules/mesh-mail/cmd/mesh-mail/store.go index dd548749..2adf3d3f 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -564,18 +564,31 @@ func addressMatchesAny(ps []Person, filters []string) bool { return false } +// sentFolderNames (issue 1231, AC14; review round 1 finding 1) are the names the live index holds a Sent folder +// under, matched case-insensitively: "Sent" for mesh and mediahuis (mailu), "[Gmail]/Sent Mail" for gmail. A +// future account naming it a third way is for the tool layer to surface (the context's "say what you chose"), +// not this store. +var sentFolderNames = []string{"Sent", "[Gmail]/Sent Mail"} + +func isSentFolder(folder string) bool { + for _, n := range sentFolderNames { + if strings.EqualFold(folder, n) { + return true + } + } + return false +} + // 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 "Sent", case-insensitively — the only name any account -// holds today; a future account naming its sent folder otherwise is for the tool layer to surface (the context's -// "say what you chose"), not this store. Computed only when needed (asked is false otherwise), since it walks -// every message. +// 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. func (s *memStore) wroteToByAccount(asked bool) map[string]map[string]bool { if !asked { return nil } out := map[string]map[string]bool{} for _, m := range s.messages { - if !strings.EqualFold(m.Folder, "Sent") { + if !isSentFolder(m.Folder) { continue } set, ok := out[m.Account] diff --git a/modules/mesh-mail/cmd/mesh-mail/tools.go b/modules/mesh-mail/cmd/mesh-mail/tools.go index ef6596b6..9930730c 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools.go @@ -143,8 +143,8 @@ func searchToolSchema() map[string]any { "group-by-sender": boolean("answer one line per sender (address, name, count, newest date) instead of the " + "mails themselves, by count descending; limit bounds the lines, not the mails"), "in-reply-to": text("only replies to this exact Message-Id (its In-Reply-To header)"), - "from-someone-i-wrote-to": boolean("only mail whose sender is a recipient of a mail you sent, in the same " + - "account's Sent folder"), + "from-someone-i-wrote-to": boolean("only mail whose sender is a recipient of a mail you sent, from the " + + "same account's own Sent folder (\"Sent\", or \"[Gmail]/Sent Mail\" for a gmail account)"), }) }