From 20342f2172cb41b8829e61872d3fcf0739fab64e Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:22:14 +0200 Subject: [PATCH 01/10] mesh-mail: mail_search's Query gains from, to, cc and a date range, so a sender, a recipient or a date can filter mails without query words (issue 1228) --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 43 +++- .../mesh-mail/cmd/mesh-mail/search_test.go | 191 ++++++++++++++++++ modules/mesh-mail/cmd/mesh-mail/store.go | 37 ++++ 3 files changed, 268 insertions(+), 3 deletions(-) create mode 100644 modules/mesh-mail/cmd/mesh-mail/search_test.go diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index 0c63fec9..f91f257d 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -395,14 +395,51 @@ func (s *PgStore) Get(ctx context.Context, account, folder string, uid uint32) ( return m, err == nil, err } +// addressFilterSQL is 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc): an EXISTS over the jsonb array column (sender, +// recipients or copied), matched against one already-lowercased parameter either as a whole address or, with no +// "@", as a domain (every address under it). column names a fixed set the Go code chooses, never caller input. +func addressFilterSQL(column, param string) string { + return fmt.Sprintf("exists (select 1 from jsonb_array_elements(%s) el where lower(el->>'address') = %s or "+ + "lower(el->>'address') like '%%@' || %s)", column, param, param) +} + func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { limit := q.Limit if limit <= 0 { limit = 20 } - rows, err := s.pool.Query(ctx, `select `+messageColumns+` from message - where account = any($1) and ($2 = '' or folder = $2) and search @@ plainto_tsquery('simple'::regconfig, $3) - order by date desc limit $4`, q.Accounts, q.Folder, q.Text, limit) + var args []any + // param appends one value and answers its placeholder; every value from the query reaches SQL this way, never + // concatenated (only fixed column names and SQL text are built as strings). + param := func(v any) string { + args = append(args, v) + return fmt.Sprintf("$%d", len(args)) + } + where := []string{"account = any(" + param(q.Accounts) + ")"} + if q.Folder != "" { + where = append(where, "folder = "+param(q.Folder)) + } + if q.Text != "" { + where = append(where, "search @@ plainto_tsquery('simple'::regconfig, "+param(q.Text)+")") + } + if q.From != "" { + where = append(where, addressFilterSQL("sender", param(strings.ToLower(q.From)))) + } + if q.To != "" { + where = append(where, addressFilterSQL("recipients", param(strings.ToLower(q.To)))) + } + if q.Cc != "" { + where = append(where, addressFilterSQL("copied", param(strings.ToLower(q.Cc)))) + } + if !q.OnOrAfter.IsZero() { + where = append(where, "date >= "+param(q.OnOrAfter)) + } + if !q.Before.IsZero() { + where = append(where, "date < "+param(q.Before)) + } + sqlText := "select " + messageColumns + " from message where " + strings.Join(where, " and ") + + " order by date desc limit " + param(limit) + rows, err := s.pool.Query(ctx, sqlText, args...) if err != nil { return nil, err } diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go new file mode 100644 index 00000000..1fdbe4b5 --- /dev/null +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -0,0 +1,191 @@ +package main + +// Fixtures and store-level tests for mail_search's filters (issues 1228-1232, novox/hq mesh-catalog.mail-search). +// Built once as fixture Messages (store.go's own type) across two accounts (mesh, gmail) and the folders INBOX, +// Sent, Junk, then reused by every issue's tests, run through eachStore (the memory store and Postgres, +// kit_test.go:170-254). "today" for these fixtures is fixed at searchNow, never time.Now(), so a run on any day +// answers the same thing. + +import ( + "context" + "sort" + "testing" + "time" +) + +// searchNow is "today" for the fixtures below (AC2's "on-or-after today", AC7's "last month", AC9's "this week's +// Monday", AC11's "90 days ago"): a fixed instant, not time.Now(), so the tests are deterministic. +var searchNow = time.Date(2026, 10, 19, 12, 0, 0, 0, time.UTC) + +// searchFixtures is one small mailbox: two accounts, three folders each. Every field's shape is store.go's own +// Message (store.go:33-50): From/To/Cc are []Person{Name, Address} (store.go:17-20, as imap.go's "people" builds +// them from an envelope, imap.go:507-515); Attachments are []AttachmentRef{Bucket, Key, Size, Type, Name} +// (store.go:24-30); Flags are []string of IMAP flag names such as "\Seen" (store.go:46, imap.go:517-527); Date is +// the parsed Date header (store.go:40, imap.go:483-485); MessageID/InReplyTo are the Message-Id and In-Reply-To +// headers verbatim (store.go:38-39). +func searchFixtures() []Message { + at := func(h int, m int) time.Time { return time.Date(2026, 10, 19, h, m, 0, 0, time.UTC) } + return []Message{ + // mesh/INBOX ------------------------------------------------------------------------------------------------ + {Account: "mesh", Folder: "INBOX", UID: 1, MessageID: "", Subject: "October invoice", + Date: at(9, 0), From: []Person{{Name: "Bob", Address: "bob@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a1", Size: 1200, Type: "application/pdf", Name: "invoice.pdf"}}, + Body: "please settle the invoice, due at month end", Size: 1500}, + {Account: "mesh", Folder: "INBOX", UID: 2, MessageID: "", Subject: "renovatie plannen", + Date: time.Date(2026, 9, 1, 8, 0, 0, 0, time.UTC), From: []Person{{Name: "Alice", Address: "alice@example.org"}}, + To: []Person{{Address: "bob@example.org"}}, Cc: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, Body: "over de renovatie van de keuken", Size: 900}, + {Account: "mesh", Folder: "INBOX", UID: 3, MessageID: "", Subject: "renovatie herinnering", + Date: time.Date(2026, 9, 2, 8, 0, 0, 0, time.UTC), From: []Person{{Name: "Alice", Address: "alice@example.org"}}, + To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, + Body: "nog een vraag over de renovatie", Size: 800}, + {Account: "mesh", Folder: "INBOX", UID: 4, MessageID: "", Subject: "security alert", + Date: time.Date(2026, 10, 15, 7, 0, 0, 0, time.UTC), From: []Person{{Name: "Carol", Address: "carol@google.com"}}, + To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, + Body: "someone logged into your account from a new device", Size: 600}, + {Account: "mesh", Folder: "INBOX", UID: 5, MessageID: "", Subject: "team meeting invite", + Date: at(8, 0), From: []Person{{Name: "Dave", Address: "dave@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a2", Size: 300, Type: "text/calendar", Name: "meeting.ics"}}, + Body: "please join the planning meeting", Size: 700}, + {Account: "mesh", Folder: "INBOX", UID: 6, MessageID: "", InReplyTo: "", Subject: "Re: quick note", + Date: at(12, 30), From: []Person{{Name: "Bob", Address: "bob@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{}, Attachments: []AttachmentRef{}, Body: "sounds good, thanks for the note", Size: 400}, + + // mesh/Sent --------------------------------------------------------------------------------------------------- + {Account: "mesh", Folder: "Sent", UID: 1, MessageID: "", InReplyTo: "", Subject: "Re: October invoice", + Date: at(10, 0), From: []Person{{Address: "jschoubben@novox.be"}}, To: []Person{{Address: "bob@example.org"}}, + Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, Body: "paid, thank you", Size: 300}, + {Account: "mesh", Folder: "Sent", UID: 2, MessageID: "", Subject: "quick note", + Date: time.Date(2026, 10, 18, 9, 0, 0, 0, time.UTC), From: []Person{{Address: "jschoubben@novox.be"}}, + To: []Person{{Address: "bob@example.org"}}, Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, Body: "just checking in", Size: 250}, + + // mesh/Junk --------------------------------------------------------------------------------------------------- + {Account: "mesh", Folder: "Junk", UID: 1, MessageID: "", Subject: "special offer just for you", + Date: time.Date(2026, 10, 10, 6, 0, 0, 0, time.UTC), From: []Person{{Address: "bob@example.org"}}, + To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "buy now and save big", Size: 500}, + {Account: "mesh", Folder: "Junk", UID: 2, MessageID: "", Subject: "you won the lottery", + Date: time.Date(2026, 10, 10, 6, 5, 0, 0, time.UTC), From: []Person{{Address: "spam@unknown.com"}}, + To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "claim your prize now", Size: 500}, + + // gmail/INBOX ------------------------------------------------------------------------------------------------- + {Account: "gmail", Folder: "INBOX", UID: 1, MessageID: "", Subject: "security alert for your account", + Date: time.Date(2026, 10, 16, 7, 0, 0, 0, time.UTC), From: []Person{{Name: "Google", Address: "security@google.com"}}, + To: []Person{{Address: "me@gmail.example"}}, Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, + Body: "new sign-in detected from a new device", Size: 650}, + {Account: "gmail", Folder: "INBOX", UID: 2, MessageID: "", Subject: "schade melding woning", + Date: time.Date(2026, 8, 1, 7, 0, 0, 0, time.UTC), From: []Person{{Name: "Someone", Address: "someone@example.org"}}, + 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", + 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}, + } +} + +// addFixtures indexes every fixture message into the store. +func addFixtures(t *testing.T, ctx context.Context, st Store) { + t.Helper() + for _, m := range searchFixtures() { + if err := st.Add(ctx, m, nil); err != nil { + t.Fatal(err) + } + } +} + +func uids(ms []Message) []string { + out := make([]string, 0, len(ms)) + for _, m := range ms { + out = append(out, m.MessageID) + } + sort.Strings(out) + return out +} + +func sameSet(t *testing.T, got []Message, want ...string) { + t.Helper() + sort.Strings(want) + g := uids(got) + if len(g) != len(want) { + t.Fatalf("got %v, want %v", g, want) + } + for i := range g { + if g[i] != want[i] { + t.Fatalf("got %v, want %v", g, want) + } + } +} + +// 1228/AC2: account, folder and on-or-after combine (unread is 1229's, so this covers only the date/account/folder +// part this issue carries). +func TestSearchFiltersByAccountFolderAndOnOrAfter(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{"mesh"}, Folder: "INBOX", OnOrAfter: time.Date(2026, 10, 19, 0, 0, 0, 0, time.UTC), Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "", "", "") + }) +} + +// 1228/AC3: from
and on-or-after 1 January answers only that sender's mails of this year. +func TestSearchFiltersByFromAndOnOrAfter(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{"mesh"}, From: "bob@example.org", + OnOrAfter: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "", "", "") + }) +} + +// 1228/AC4: folder Sent, to
and a date range; a domain matches every address of it. +func TestSearchFiltersByFolderToAndDateRange(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + byAddress, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "Sent", To: "bob@example.org", + OnOrAfter: time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC), Before: time.Date(2026, 11, 1, 0, 0, 0, 0, time.UTC), Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, byAddress, "", "") + + byDomain, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "Sent", To: "example.org", + OnOrAfter: time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC), Before: time.Date(2026, 11, 1, 0, 0, 0, 0, time.UTC), Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, byDomain, "", "") + + // Before Oct 1 excludes both, proving the range's ends are honoured, not only the address. + none, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "Sent", To: "bob@example.org", + Before: time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC), Limit: 10}) + if err != nil { + t.Fatal(err) + } + if len(none) != 0 { + t.Fatalf("before the range: %v", none) + } + }) +} + +// 1228/AC6 (cc part): cc answers only the mail with that address in cc, not the one with it only in +// to (words are 1229's; this covers the cc field alone). +func TestSearchFiltersByCcNotTo(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{"mesh"}, Cc: "jschoubben@novox.be", 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 7444ddf9..a24a4eda 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -90,6 +90,17 @@ type Query struct { Accounts []string Folder string Limit int + + // From, To and Cc (issue 1228, AC2-AC4, AC6's cc part) are each an address or a domain, case-insensitive; a + // domain (no "@") matches every address of it. Empty means no filter on that field. + From string + To string + Cc string + + // OnOrAfter and Before (issue 1228, AC2-AC4) bound Date (the Date header, imap.go:483-485): on-or-after is + // inclusive, before is exclusive. A zero time means no bound. + OnOrAfter time.Time + Before time.Time } // Store is the index. @@ -239,6 +250,16 @@ func (s *memStore) Search(_ context.Context, q Query) ([]Message, error) { if !contains(q.Accounts, m.Account) || (q.Folder != "" && q.Folder != m.Folder) { continue } + // 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc): the address and date-range filters, mirroring PgStore.Search. + if !addressMatches(m.From, q.From) || !addressMatches(m.To, q.To) || !addressMatches(m.Cc, q.Cc) { + continue + } + if !q.OnOrAfter.IsZero() && m.Date.Before(q.OnOrAfter) { + continue + } + if !q.Before.IsZero() && !m.Date.Before(q.Before) { + continue + } hay := strings.ToLower(m.Subject + " " + m.Body + " " + peopleText(m.From) + " " + peopleText(m.To) + " " + peopleText(m.Cc)) all := true for _, w := range words { @@ -321,6 +342,22 @@ func contains(list []string, s string) bool { return false } +// addressMatches is 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc): filter is an address (matched exactly) or a domain +// (matched as the suffix after "@"), case-insensitive; empty filter matches every message. +func addressMatches(ps []Person, filter string) bool { + if filter == "" { + return true + } + f := strings.ToLower(filter) + for _, p := range ps { + addr := strings.ToLower(p.Address) + if addr == f || strings.HasSuffix(addr, "@"+f) { + return true + } + } + return false +} + func peopleText(ps []Person) string { var parts []string for _, p := range ps { From 2985122856f8414a0fa655b9ba70e6c0163482e3 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:29:15 +0200 Subject: [PATCH 02/10] mesh-mail: mail_search's Query gains subject/body words, attachment type/name and read/unread, so a word in one field, an attachment or a flag can filter mails (issue 1229) --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 70 +++++++++ .../mesh-mail/cmd/mesh-mail/search_test.go | 144 +++++++++++++++++- modules/mesh-mail/cmd/mesh-mail/store.go | 97 ++++++++++++ 3 files changed, 310 insertions(+), 1 deletion(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index f91f257d..f81fda04 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -403,6 +403,54 @@ func addressFilterSQL(column, param string) string { "lower(el->>'address') like '%%@' || %s)", column, param, param) } +// wordFilterSQL is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15: one condition per word of the list, each a +// whole-word, case-insensitive match (Postgres's \y, its own word boundary) against column, so a hyphenated word +// such as "4711-882" is never matched by only its digits (AC12); joined with "and" ("all", the default) or "or" +// ("any"). Empty answers "" (no filter). The word itself always reaches SQL as param's parameter, never +// concatenated; only its \y anchors are fixed SQL text. +func wordFilterSQL(column string, words []string, any bool, param func(any) string) string { + if len(words) == 0 { + return "" + } + parts := make([]string, 0, len(words)) + for _, w := range words { + parts = append(parts, fmt.Sprintf(`%s ~* ('\y' || %s || '\y')`, column, param(quoteRegex(w)))) + } + op := " and " + if any { + op = " or " + } + return "(" + strings.Join(parts, op) + ")" +} + +// quoteRegex escapes a word for Postgres's regex operators (ARE, a superset of POSIX ERE): the word is always +// matched literally, never as a pattern of its own, mirroring Go's regexp.QuoteMeta used by memStore's own match. +var quoteRegexEscaper = strings.NewReplacer( + `\`, `\\`, ".", `\.`, "+", `\+`, "*", `\*`, "?", `\?`, "(", `\(`, ")", `\)`, + "[", `\[`, "]", `\]`, "{", `\{`, "}", `\}`, "^", `\^`, "$", `\$`, "|", `\|`) + +func quoteRegex(word string) string { return quoteRegexEscaper.Replace(word) } + +// globToLike turns a shell glob ("*" and "?") into a SQL LIKE pattern, escaping LIKE's own metacharacters first +// (issue 1229, AC5, AC9: an attachment name pattern such as "*.pdf"). +func globToLike(pattern string) string { + var b strings.Builder + for _, r := range pattern { + switch r { + case '*': + b.WriteByte('%') + case '?': + b.WriteByte('_') + case '%', '_', '\\': + b.WriteByte('\\') + b.WriteRune(r) + default: + b.WriteRune(r) + } + } + return b.String() +} + func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { limit := q.Limit if limit <= 0 { @@ -437,6 +485,28 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { if !q.Before.IsZero() { where = append(where, "date < "+param(q.Before)) } + if c := wordFilterSQL("subject", q.SubjectWords, q.SubjectWordsAny, param); c != "" { + where = append(where, c) + } + if c := wordFilterSQL("body", q.BodyWords, q.BodyWordsAny, param); c != "" { + where = append(where, c) + } + if q.HasAttachment { + where = append(where, "jsonb_array_length(attachments) > 0") + } + if q.AttachmentType != "" || q.AttachmentNamePattern != "" { + var conds []string + if q.AttachmentType != "" { + conds = append(conds, "lower(el->>'type') = "+param(strings.ToLower(q.AttachmentType))) + } + if q.AttachmentNamePattern != "" { + conds = append(conds, "lower(el->>'name') like "+param(globToLike(strings.ToLower(q.AttachmentNamePattern)))) + } + where = append(where, "exists (select 1 from jsonb_array_elements(attachments) el where "+strings.Join(conds, " or ")+")") + } + if q.Unread != nil { + where = append(where, fmt.Sprintf("(%s = any(flags)) = %s", param(`\Seen`), param(!*q.Unread))) + } sqlText := "select " + messageColumns + " from message where " + strings.Join(where, " and ") + " order by date desc limit " + param(limit) rows, err := s.pool.Query(ctx, sqlText, args...) diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go index 1fdbe4b5..331ff044 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -50,6 +50,24 @@ func searchFixtures() []Message { {Account: "mesh", Folder: "INBOX", UID: 6, MessageID: "", InReplyTo: "", Subject: "Re: quick note", Date: at(12, 30), From: []Person{{Name: "Bob", Address: "bob@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "sounds good, thanks for the note", Size: 400}, + // 1229/AC5: from a domain (google.com), has-attachment, name *.pdf and the word "factuur". + {Account: "mesh", Folder: "INBOX", UID: 7, MessageID: "", Subject: "the factuur for Q3", + Date: at(13, 0), From: []Person{{Name: "Erik", Address: "erik@google.com"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a4", Size: 200, Type: "application/pdf", Name: "factuur.pdf"}}, + Body: "see the attached factuur, due on receipt", Size: 850}, + // 1229/AC12: a word with digits and a dash ("4711-882"), matched whole. + {Account: "mesh", Folder: "INBOX", UID: 8, MessageID: "", Subject: "order 4711-882 confirmed", + Date: at(14, 0), From: []Person{{Address: "ops@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{}, Attachments: []AttachmentRef{}, Body: "your order 4711-882 has shipped", Size: 300}, + // Control for 1229/AC12: "4711" and "882" appear as separate tokens, never hyphenated together. + {Account: "mesh", Folder: "INBOX", UID: 9, MessageID: "", Subject: "order 4711 shipped, item 882 noted", + Date: at(14, 5), From: []Person{{Address: "ops@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{}, Attachments: []AttachmentRef{}, Body: "item 4711 and item 882 are unrelated", Size: 300}, + // 1229/AC15 (the "any" word list): only "verzekering" is present, not "schade", with an attachment. + {Account: "mesh", Folder: "INBOX", UID: 10, MessageID: "", Subject: "verzekering aanvraag", + Date: at(14, 10), From: []Person{{Address: "broker@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a5", Size: 150, Type: "application/pdf", Name: "polis.pdf"}}, + Body: "nieuwe polis aanvraag for verzekering", Size: 450}, // mesh/Sent --------------------------------------------------------------------------------------------------- {Account: "mesh", Folder: "Sent", UID: 1, MessageID: "", InReplyTo: "", Subject: "Re: October invoice", @@ -127,7 +145,7 @@ func TestSearchFiltersByAccountFolderAndOnOrAfter(t *testing.T) { if err != nil { t.Fatal(err) } - sameSet(t, got, "", "", "") + sameSet(t, got, "", "", "", "", "", "", "") }) } @@ -189,3 +207,127 @@ func TestSearchFiltersByCcNotTo(t *testing.T) { sameSet(t, got, "") }) } + +// 1229/AC6 (words part): cc and the word "renovatie" in subject or body, combined with the cc filter +// from 1228: the mail with it only in to is still excluded. +func TestSearchFiltersByCcAndSubjectOrBodyWord(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{"mesh"}, Cc: "jschoubben@novox.be", BodyWords: []string{"renovatie"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "") + }) +} + +// 1229/AC5: from a domain, has-attachment, an attachment name pattern and a word in subject or body all combine. +func TestSearchFiltersByDomainAttachmentAndWord(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{"mesh"}, From: "google.com", HasAttachment: true, + AttachmentNamePattern: "*.pdf", BodyWords: []string{"factuur"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + // carol@google.com's security-alert mail (uid 4) is from the same domain but has neither an attachment nor + // the word, proving every one of the four conditions is enforced, not just one. + sameSet(t, got, "") + }) +} + +// 1229/AC9: on-or-after a date and an attachment type or name pattern (either suffices) answers a calendar invite. +func TestSearchFiltersByDateAndAttachmentTypeOrName(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{"mesh"}, OnOrAfter: time.Date(2026, 10, 19, 0, 0, 0, 0, time.UTC), + AttachmentType: "text/calendar", AttachmentNamePattern: "*.ics", Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "") + }) +} + +// 1229/AC12: a word with digits and a dash is matched whole, not as its separate parts. +func TestSearchFiltersBySubjectOrBodyWordWhole(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{"mesh", "gmail"}, BodyWords: []string{"4711-882"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "") + }) +} + +// 1229/AC13: from a domain and the subject words (default "all": every word of the list), across every granted +// account, each answer naming its own account. +func TestSearchFiltersByDomainAndSubjectWordsAcrossAccounts(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{"mesh", "gmail"}, From: "google.com", + SubjectWords: []string{"security", "alert"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "", "") + accounts := map[string]bool{} + for _, m := range got { + accounts[m.Account] = true + } + if !accounts["mesh"] || !accounts["gmail"] { + t.Fatalf("each answer should name its own account: %+v", got) + } + }) +} + +// 1229/AC15: a word list marked "any" (schade or verzekering) combined with has-attachment; the word list's "any" +// mode matches a mail holding only one of the words, not both. +func TestSearchFiltersByAnyWordsAndHasAttachment(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{"mesh", "gmail"}, BodyWords: []string{"schade", "verzekering"}, + BodyWordsAny: true, HasAttachment: true, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "", "") + + // Without "any" (the default, "all"), only the mail holding both words together matches: polis1@mesh has + // "verzekering" alone, so it drops out. + all, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, BodyWords: []string{"schade", "verzekering"}, + HasAttachment: true, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, all, "") + }) +} + +// 1229 (unread/read, from 1228/AC2): the flag filter, read or unread, independent of the other filters. +func TestSearchFiltersByUnreadFlag(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + unread := true + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "INBOX", Unread: &unread, Limit: 20}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "", "", "", "", "", "") + + read := false + gotRead, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "INBOX", Unread: &read, Limit: 20}) + if err != nil { + t.Fatal(err) + } + sameSet(t, gotRead, "", "", "", "") + }) +} diff --git a/modules/mesh-mail/cmd/mesh-mail/store.go b/modules/mesh-mail/cmd/mesh-mail/store.go index a24a4eda..f4e53990 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -6,6 +6,8 @@ package main import ( "context" + "path" + "regexp" "sort" "strconv" "strings" @@ -101,6 +103,28 @@ type Query struct { // inclusive, before is exclusive. A zero time means no bound. OnOrAfter time.Time Before time.Time + + // SubjectWords and BodyWords (issue 1229, AC5, AC6's words part, AC12, AC13, AC15) are each a whole word, + // matched case-insensitively against Subject or Body respectively (a word with digits and a dash, such as + // "4711-882", is matched whole, never by its separate parts, AC12). By default every word of the list must be + // found ("all"); the *WordsAny flag switches that list to "any of them" (AC15). Empty means no filter. + SubjectWords []string + SubjectWordsAny bool + BodyWords []string + BodyWordsAny bool + + // HasAttachment, AttachmentType and AttachmentNamePattern (issue 1229, AC5, AC9) filter by attachment: + // HasAttachment requires at least one; AttachmentType (an exact media type, case-insensitive) and + // AttachmentNamePattern (a shell glob such as "*.pdf", case-insensitive) each require one attachment matching + // it, but when both are given, either suffices for the same attachment (AC9: "type text/calendar or name + // *.ics", since a calendar invite may carry either). Empty/false means no filter. + HasAttachment bool + AttachmentType string + AttachmentNamePattern string + + // Unread (issue 1229, carrying 1228/AC2's flag part) is nil for no filter; true answers only messages without + // the \Seen flag, false only those with it. + Unread *bool } // Store is the index. @@ -260,6 +284,17 @@ func (s *memStore) Search(_ context.Context, q Query) ([]Message, error) { if !q.Before.IsZero() && !m.Date.Before(q.Before) { continue } + // 1229/AC5, 1229/AC6(words), 1229/AC9, 1229/AC12, 1229/AC13, 1229/AC15: the word, attachment and + // read/unread filters, mirroring PgStore.Search. + if !wordMatches(m.Subject, q.SubjectWords, q.SubjectWordsAny) || !wordMatches(m.Body, q.BodyWords, q.BodyWordsAny) { + continue + } + if !attachmentMatches(m.Attachments, q.HasAttachment, q.AttachmentType, q.AttachmentNamePattern) { + continue + } + if q.Unread != nil && hasSeenFlag(m.Flags) == *q.Unread { + continue + } hay := strings.ToLower(m.Subject + " " + m.Body + " " + peopleText(m.From) + " " + peopleText(m.To) + " " + peopleText(m.Cc)) all := true for _, w := range words { @@ -358,6 +393,68 @@ func addressMatches(ps []Person, filter string) bool { return false } +// wordRegexp is a whole-word, case-insensitive match (issue 1229, AC12): \b, Go's RE2 word boundary, treats a +// dash as a non-word character, so a hyphenated word such as "4711-882" is bounded at both ends and never matched +// by only its digits. +func wordRegexp(word string) *regexp.Regexp { + return regexp.MustCompile(`(?i)\b` + regexp.QuoteMeta(word) + `\b`) +} + +// wordMatches is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15: an empty list matches every message; otherwise +// every word of the list must be found in text ("all"), or at least one when any is true. +func wordMatches(text string, words []string, any bool) bool { + if len(words) == 0 { + return true + } + for _, w := range words { + found := wordRegexp(w).MatchString(text) + if any && found { + return true + } + if !any && !found { + return false + } + } + return !any +} + +// attachmentMatches is 1229/AC5, AC9: hasAttachment requires at least one attachment; typ and namePattern, when +// either is given, each require one attachment matching it, but when both are given either suffices for the same +// attachment (AC9's "type text/calendar or name *.ics"). +func attachmentMatches(atts []AttachmentRef, hasAttachment bool, typ, namePattern string) bool { + if hasAttachment && len(atts) == 0 { + return false + } + if typ == "" && namePattern == "" { + return true + } + for _, a := range atts { + if typ != "" && strings.EqualFold(a.Type, typ) { + return true + } + if namePattern != "" && globMatches(namePattern, a.Name) { + return true + } + } + return false +} + +// globMatches is 1229/AC5, AC9: a shell glob such as "*.pdf" or "*.ics", case-insensitive. +func globMatches(pattern, name string) bool { + ok, _ := path.Match(strings.ToLower(pattern), strings.ToLower(name)) + return ok +} + +// hasSeenFlag is 1229 (carrying 1228/AC2's flag part): whether the message is read, by IMAP's own \Seen flag. +func hasSeenFlag(flags []string) bool { + for _, f := range flags { + if strings.EqualFold(f, `\Seen`) { + return true + } + } + return false +} + func peopleText(ps []Person) string { var parts []string for _, p := range ps { From fc10031d225369cc191cc80d102e5a1479a5e185 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:36:53 +0200 Subject: [PATCH 03/10] mesh-mail: mail_search orders by date, sender, subject or size and groups by sender, so the newest, the largest or who writes most can be answered (issue 1230) Query's From becomes a list, OR'd (1228/AC11's several domains), since 1230's order-by needs it to test the ascending-order criterion end to end; To and Cc stay single, no criterion asking for more. --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 111 +++++++-- .../mesh-mail/cmd/mesh-mail/search_test.go | 107 ++++++++- modules/mesh-mail/cmd/mesh-mail/store.go | 221 +++++++++++++++--- 3 files changed, 386 insertions(+), 53 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index f81fda04..7e8e8139 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -395,14 +395,27 @@ func (s *PgStore) Get(ctx context.Context, account, folder string, uid uint32) ( return m, err == nil, err } -// addressFilterSQL is 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc): an EXISTS over the jsonb array column (sender, -// recipients or copied), matched against one already-lowercased parameter either as a whole address or, with no -// "@", as a domain (every address under it). column names a fixed set the Go code chooses, never caller input. +// addressFilterSQL is 1228/AC2, 1228/AC4, 1228/AC6(cc): an EXISTS over the jsonb array column (recipients or +// copied), matched against one already-lowercased parameter either as a whole address or, with no "@", as a +// domain (every address under it). column names a fixed set the Go code chooses, never caller input. func addressFilterSQL(column, param string) string { return fmt.Sprintf("exists (select 1 from jsonb_array_elements(%s) el where lower(el->>'address') = %s or "+ "lower(el->>'address') like '%%@' || %s)", column, param, param) } +// addressFilterSQLAny is 1228/AC3; 1230/AC11 ("several domains"): an EXISTS over column (sender), matched +// against any of filters, OR'd (each already-lowercased, each either a whole address or, with no "@", a domain). +// The whole list reaches SQL as one parameter, a text[]; never concatenated. +func addressFilterSQLAny(column string, filters []string, param func(any) string) string { + lowered := make([]string, len(filters)) + for i, f := range filters { + lowered[i] = strings.ToLower(f) + } + p := param(lowered) + return fmt.Sprintf("exists (select 1 from jsonb_array_elements(%s) el, unnest(%s::text[]) f where "+ + "lower(el->>'address') = f or lower(el->>'address') like '%%@' || f)", column, p) +} + // wordFilterSQL is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15: one condition per word of the list, each a // whole-word, case-insensitive match (Postgres's \y, its own word boundary) against column, so a hyphenated word // such as "4711-882" is never matched by only its digits (AC12); joined with "and" ("all", the default) or "or" @@ -451,14 +464,10 @@ func globToLike(pattern string) string { return b.String() } -func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { - limit := q.Limit - if limit <= 0 { - limit = 20 - } - var args []any - // param appends one value and answers its placeholder; every value from the query reaches SQL this way, never - // concatenated (only fixed column names and SQL text are built as strings). +// 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). +func searchWhere(q Query) (whereSQL string, args []any) { param := func(v any) string { args = append(args, v) return fmt.Sprintf("$%d", len(args)) @@ -470,8 +479,8 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { if q.Text != "" { where = append(where, "search @@ plainto_tsquery('simple'::regconfig, "+param(q.Text)+")") } - if q.From != "" { - where = append(where, addressFilterSQL("sender", param(strings.ToLower(q.From)))) + if len(q.From) > 0 { + where = append(where, addressFilterSQLAny("sender", q.From, param)) } if q.To != "" { where = append(where, addressFilterSQL("recipients", param(strings.ToLower(q.To)))) @@ -507,8 +516,44 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { if q.Unread != nil { where = append(where, fmt.Sprintf("(%s = any(flags)) = %s", param(`\Seen`), param(!*q.Unread))) } - sqlText := "select " + messageColumns + " from message where " + strings.Join(where, " and ") + - " order by date desc limit " + param(limit) + return strings.Join(where, " and "), args +} + +// orderColumns is 1230's fixed Go map from an order-by name to its (safe, never caller-built) SQL expression; +// unknown or empty answers "date" — the tool refuses an unknown order-by before the store is ever asked (AC16). +var orderColumns = map[string]string{ + "date": "date", + "sender": "coalesce(sender->0->>'address', '')", + "subject": "subject", + "size": "size", +} + +func orderExpr(orderBy string) string { + if e, ok := orderColumns[orderBy]; ok { + return e + } + return orderColumns["date"] +} + +func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { + limit := q.Limit + if limit <= 0 { + limit = 20 + } + whereSQL, args := searchWhere(q) + param := func(v any) string { + args = append(args, v) + return fmt.Sprintf("$%d", len(args)) + } + // 1230 (AC7, AC11, AC17's schema): order by the chosen expression, ascending unless desc; an empty OrderBy + // keeps the newest-first default (AC1) regardless of OrderDesc. Ties always break ascending on + // (account, folder, uid), so the order is stable. + dir := "asc" + if q.OrderBy == "" || q.OrderDesc { + dir = "desc" + } + sqlText := "select " + messageColumns + " from message where " + whereSQL + + " order by " + orderExpr(q.OrderBy) + " " + dir + ", account, folder, uid limit " + param(limit) rows, err := s.pool.Query(ctx, sqlText, args...) if err != nil { return nil, err @@ -525,6 +570,42 @@ func (s *PgStore) Search(ctx context.Context, q Query) ([]Message, error) { return out, rows.Err() } +// GroupBySender is 1230/AC10: one line per sender among the messages searchWhere's filters match, by count +// descending (address ascending to break ties), Limit bounding the number of lines. +func (s *PgStore) GroupBySender(ctx context.Context, q Query) ([]SenderGroup, error) { + limit := q.Limit + if limit <= 0 { + limit = 20 + } + whereSQL, args := searchWhere(q) + param := func(v any) string { + args = append(args, v) + return fmt.Sprintf("$%d", len(args)) + } + sqlText := `select el->>'address' as address, max(el->>'name') as name, count(*) as count, max(date) as newest + from message, jsonb_array_elements(sender) el where ` + whereSQL + ` group by el->>'address' + order by count desc, address asc limit ` + param(limit) + rows, err := s.pool.Query(ctx, sqlText, args...) + if err != nil { + return nil, err + } + defer rows.Close() + var out []SenderGroup + for rows.Next() { + var g SenderGroup + var name *string + if err := rows.Scan(&g.Address, &name, &g.Count, &g.Newest); err != nil { + return nil, err + } + if name != nil { + g.Name = *name + } + g.Newest = g.Newest.UTC() + out = append(out, g) + } + return out, rows.Err() +} + func (s *PgStore) Folders(ctx context.Context, account string) ([]Folder, error) { rows, err := s.pool.Query(ctx, `select name, uidvalidity, messages, synced from folder where account = $1 order by name`, account) 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 331ff044..a76856c6 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -154,7 +154,7 @@ func TestSearchFiltersByFromAndOnOrAfter(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{"mesh"}, From: "bob@example.org", + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: []string{"bob@example.org"}, OnOrAfter: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), Limit: 10}) if err != nil { t.Fatal(err) @@ -227,7 +227,7 @@ func TestSearchFiltersByDomainAttachmentAndWord(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{"mesh"}, From: "google.com", HasAttachment: true, + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, From: []string{"google.com"}, HasAttachment: true, AttachmentNamePattern: "*.pdf", BodyWords: []string{"factuur"}, Limit: 10}) if err != nil { t.Fatal(err) @@ -271,7 +271,7 @@ func TestSearchFiltersByDomainAndSubjectWordsAcrossAccounts(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{"mesh", "gmail"}, From: "google.com", + got, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, From: []string{"google.com"}, SubjectWords: []string{"security", "alert"}, Limit: 10}) if err != nil { t.Fatal(err) @@ -331,3 +331,104 @@ func TestSearchFiltersByUnreadFlag(t *testing.T) { sameSet(t, gotRead, "", "", "", "") }) } + +// sameOrder asserts got's message-ids equal want, in that exact order (unlike sameSet, which ignores order). +func sameOrder(t *testing.T, got []Message, want ...string) { + t.Helper() + g := make([]string, 0, len(got)) + for _, m := range got { + g = append(g, m.MessageID) + } + if len(g) != len(want) { + t.Fatalf("got %v, want %v", g, want) + } + for i := range g { + if g[i] != want[i] { + t.Fatalf("got %v, want %v", g, want) + } + } +} + +// 1230/AC7: a date range ("last month") ordered by size descending, limit bounding the answer. +func TestSearchOrdersBySizeDescending(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{"mesh"}, OnOrAfter: time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC), + Before: time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC), OrderBy: "size", OrderDesc: true, Limit: 5}) + if err != nil { + t.Fatal(err) + } + sameOrder(t, got, "", "") + }) +} + +// 1230/AC11: from several domains (OR'd) and on-or-after a date, order by date ascending, oldest first; limit +// bounds the answer. +func TestSearchFiltersBySeveralDomainsOrderedAscending(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{"mesh"}, From: []string{"example.org", "google.com"}, + OnOrAfter: searchNow.AddDate(0, 0, -90), OrderBy: "date", OrderDesc: false, Limit: 4}) + 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, "", "", "", "") + }) +} + +// 1230 (the order-by mechanism beyond date and size, not tied to one AC): sender and subject each sort, and the +// tie-break on (account, folder, uid) is stable when the chosen field is equal. +func TestSearchOrdersBySenderAndSubject(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + bySender, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "Sent", OrderBy: "sender", Limit: 10}) + if err != nil { + t.Fatal(err) + } + // Both Sent messages share the same sender (jschoubben@novox.be): the tie-break (account, folder, uid) + // orders them by uid ascending. + sameOrder(t, bySender, "", "") + + bySubject, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "Junk", OrderBy: "subject", Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameOrder(t, bySubject, "", "") // "special offer..." < "you won..." + }) +} + +// 1230/AC10: group by sender, folder INBOX, by count descending (address ascending to break ties); limit bounds +// the number of lines, not the number of messages. +func TestGroupBySenderOrdersByCountDescending(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{"mesh"}, Folder: "INBOX", Limit: 3}) + if err != nil { + t.Fatal(err) + } + if len(got) != 3 { + t.Fatalf("got %+v", got) + } + want := []struct { + address string + count int + }{{"alice@example.org", 2}, {"bob@example.org", 2}, {"ops@example.org", 2}} + for i, w := range want { + if got[i].Address != w.address || got[i].Count != w.count { + t.Fatalf("line %d: got %+v, want %+v", i, got[i], w) + } + } + // bob's two INBOX mails are invoice1 (09:00) and reply1 (12:30): the newest is reply1's. + for _, g := range got { + if g.Address == "bob@example.org" && !g.Newest.Equal(time.Date(2026, 10, 19, 12, 30, 0, 0, time.UTC)) { + t.Fatalf("bob's newest: %v", g.Newest) + } + } + }) +} diff --git a/modules/mesh-mail/cmd/mesh-mail/store.go b/modules/mesh-mail/cmd/mesh-mail/store.go index f4e53990..5253c4ee 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -93,9 +93,11 @@ type Query struct { Folder string Limit int - // From, To and Cc (issue 1228, AC2-AC4, AC6's cc part) are each an address or a domain, case-insensitive; a - // domain (no "@") matches every address of it. Empty means no filter on that field. - From string + // From (issue 1228, AC2, AC3, AC6's cc part; issue 1230, AC11's "several domains") is a list of addresses or + // domains, case-insensitive, OR'd together: a message matches when at least one entry matches (a domain, no + // "@", matching every address of it). To and Cc (issue 1228, AC4, AC6's cc part) are each a single address or + // domain, the same way; no criterion asks for more than one. Empty/nil means no filter on that field. + From []string To string Cc string @@ -125,6 +127,22 @@ type Query struct { // Unread (issue 1229, carrying 1228/AC2's flag part) is nil for no filter; true answers only messages without // the \Seen flag, false only those with it. Unread *bool + + // OrderBy and OrderDesc (issue 1230, AC7, AC11, AC17's schema) choose Search's sort: empty OrderBy means + // "date" with OrderDesc ignored, kept descending (today's newest-first default, AC1); given explicitly, one + // of "date", "sender" (by its first address), "subject" or "size", ascending unless OrderDesc. Ties always + // break ascending on (account, folder, uid). GroupBySender ignores both: its own order is fixed (AC10). + OrderBy string + OrderDesc bool +} + +// SenderGroup is one line of mail_search's group-by-sender answer (issue 1230, AC10): the sender's address and +// name (as last seen on a matching message), how many matching messages it sent, and the newest one's date. +type SenderGroup struct { + Address string `json:"address"` + Name string `json:"name,omitempty"` + Count int `json:"count"` + Newest time.Time `json:"newest"` } // Store is the index. @@ -147,8 +165,11 @@ type Store interface { Remove(ctx context.Context, account, folder string, uid uint32, e *Event) error // Get is one message, with its body. Get(ctx context.Context, account, folder string, uid uint32) (Message, bool, error) - // Search finds messages, newest first, without their bodies. + // Search finds messages, newest first by default (OrderBy, OrderDesc choose otherwise), without their bodies. Search(ctx context.Context, q Query) ([]Message, error) + // GroupBySender is 1230/AC10: one line per sender among the messages Query's other filters match, by count + // descending, Limit bounding the number of lines. + GroupBySender(ctx context.Context, q Query) ([]SenderGroup, error) // Folders is every folder the index knows of an account. Folders(ctx context.Context, account string) ([]Folder, error) // Pending is the events not yet said, oldest first; Said removes those said. @@ -265,47 +286,163 @@ func (s *memStore) Get(_ context.Context, account, folder string, uid uint32) (M return m, ok, nil } +// matchesQuery is every filter of Query but its order and its limit, shared by Search and GroupBySender so the +// two never drift apart (mirrored, condition for condition, by PgStore's own WHERE builder, pgstore.go). +func matchesQuery(m Message, q Query, words []string) bool { + if !contains(q.Accounts, m.Account) || (q.Folder != "" && q.Folder != m.Folder) { + return false + } + // 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc); 1230/AC11 (From's several domains): the address and date-range + // filters, mirroring PgStore.Search. + if !addressMatchesAny(m.From, q.From) || !addressMatches(m.To, q.To) || !addressMatches(m.Cc, q.Cc) { + return false + } + if !q.OnOrAfter.IsZero() && m.Date.Before(q.OnOrAfter) { + return false + } + if !q.Before.IsZero() && !m.Date.Before(q.Before) { + return false + } + // 1229/AC5, 1229/AC6(words), 1229/AC9, 1229/AC12, 1229/AC13, 1229/AC15: the word, attachment and + // read/unread filters, mirroring PgStore.Search. + if !wordMatches(m.Subject, q.SubjectWords, q.SubjectWordsAny) || !wordMatches(m.Body, q.BodyWords, q.BodyWordsAny) { + return false + } + if !attachmentMatches(m.Attachments, q.HasAttachment, q.AttachmentType, q.AttachmentNamePattern) { + return false + } + if q.Unread != nil && hasSeenFlag(m.Flags) == *q.Unread { + return false + } + hay := strings.ToLower(m.Subject + " " + m.Body + " " + peopleText(m.From) + " " + peopleText(m.To) + " " + peopleText(m.Cc)) + for _, w := range words { + if !strings.Contains(hay, w) { + return false + } + } + return true +} + func (s *memStore) Search(_ context.Context, q Query) ([]Message, error) { s.mu.Lock() defer s.mu.Unlock() words := strings.Fields(strings.ToLower(q.Text)) var out []Message for _, m := range s.messages { - if !contains(q.Accounts, m.Account) || (q.Folder != "" && q.Folder != m.Folder) { - continue - } - // 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc): the address and date-range filters, mirroring PgStore.Search. - if !addressMatches(m.From, q.From) || !addressMatches(m.To, q.To) || !addressMatches(m.Cc, q.Cc) { - continue - } - if !q.OnOrAfter.IsZero() && m.Date.Before(q.OnOrAfter) { - continue - } - if !q.Before.IsZero() && !m.Date.Before(q.Before) { - continue - } - // 1229/AC5, 1229/AC6(words), 1229/AC9, 1229/AC12, 1229/AC13, 1229/AC15: the word, attachment and - // read/unread filters, mirroring PgStore.Search. - if !wordMatches(m.Subject, q.SubjectWords, q.SubjectWordsAny) || !wordMatches(m.Body, q.BodyWords, q.BodyWordsAny) { - continue - } - if !attachmentMatches(m.Attachments, q.HasAttachment, q.AttachmentType, q.AttachmentNamePattern) { - continue - } - if q.Unread != nil && hasSeenFlag(m.Flags) == *q.Unread { - continue - } - hay := strings.ToLower(m.Subject + " " + m.Body + " " + peopleText(m.From) + " " + peopleText(m.To) + " " + peopleText(m.Cc)) - all := true - for _, w := range words { - all = all && strings.Contains(hay, w) - } - if all { + if matchesQuery(m, q, words) { m.Body = "" out = append(out, m) } } - sort.Slice(out, func(i, j int) bool { return out[i].Date.After(out[j].Date) }) + // 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) }) + if q.Limit > 0 && len(out) > q.Limit { + out = out[:q.Limit] + } + return out, nil +} + +// orderLess is 1230 (AC7, AC11, AC17's schema): date (the default when orderBy is empty), sender (by its first +// address), subject or size, ascending unless desc; ties break on (account, folder, uid), always ascending, so +// the order is stable regardless of the map order Go would otherwise give equal messages. +func orderLess(a, b Message, orderBy string, desc bool) bool { + if orderBy == "" { + // Unspecified order keeps the newest-first default (AC1), regardless of OrderDesc. + desc = true + } + var cmp int + switch orderBy { + case "sender": + cmp = strings.Compare(firstAddress(a.From), firstAddress(b.From)) + case "subject": + cmp = strings.Compare(a.Subject, b.Subject) + case "size": + cmp = compareInt64(a.Size, b.Size) + default: // "date", or empty (the default) + cmp = compareTime(a.Date, b.Date) + } + if desc { + cmp = -cmp + } + if cmp != 0 { + return cmp < 0 + } + if a.Account != b.Account { + return a.Account < b.Account + } + if a.Folder != b.Folder { + return a.Folder < b.Folder + } + return a.UID < b.UID +} + +func firstAddress(ps []Person) string { + if len(ps) == 0 { + return "" + } + return strings.ToLower(ps[0].Address) +} + +func compareInt64(a, b int64) int { + switch { + case a < b: + return -1 + case a > b: + return 1 + default: + return 0 + } +} + +func compareTime(a, b time.Time) int { + switch { + case a.Before(b): + return -1 + case a.After(b): + return 1 + default: + return 0 + } +} + +// GroupBySender is 1230 (AC10): one line per sender among the messages Query's other filters match (its OrderBy +// and OrderDesc are not used; the lines are always ordered by count descending, address ascending to break ties), +// Limit bounding the number of lines, not the number of messages. +func (s *memStore) GroupBySender(_ context.Context, q Query) ([]SenderGroup, error) { + s.mu.Lock() + defer s.mu.Unlock() + words := strings.Fields(strings.ToLower(q.Text)) + byAddress := map[string]*SenderGroup{} + var order []string + for _, m := range s.messages { + if !matchesQuery(m, q, words) { + continue + } + for _, p := range m.From { + addr := strings.ToLower(p.Address) + g, ok := byAddress[addr] + if !ok { + g = &SenderGroup{Address: p.Address, Name: p.Name} + byAddress[addr] = g + order = append(order, addr) + } + g.Count++ + if m.Date.After(g.Newest) { + g.Newest = m.Date + } + } + } + out := make([]SenderGroup, 0, len(order)) + for _, addr := range order { + out = append(out, *byAddress[addr]) + } + sort.Slice(out, func(i, j int) bool { + if out[i].Count != out[j].Count { + return out[i].Count > out[j].Count + } + return out[i].Address < out[j].Address + }) if q.Limit > 0 && len(out) > q.Limit { out = out[:q.Limit] } @@ -393,6 +530,20 @@ func addressMatches(ps []Person, filter string) bool { return false } +// addressMatchesAny is 1230/AC11: From's several domains (or addresses), OR'd together; empty/nil matches every +// message, otherwise at least one entry (each matched as addressMatches does) must match. +func addressMatchesAny(ps []Person, filters []string) bool { + if len(filters) == 0 { + return true + } + for _, f := range filters { + if addressMatches(ps, f) { + return true + } + } + return false +} + // wordRegexp is a whole-word, case-insensitive match (issue 1229, AC12): \b, Go's RE2 word boundary, treats a // dash as a non-word character, so a hyphenated word such as "4711-882" is bounded at both ends and never matched // by only its digits. From 4a60deb4a8e7a0634496d1d2088cab90349b01b0 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:39:59 +0200 Subject: [PATCH 04/10] mesh-mail: mail_search finds a thread's replies and mail from someone the operator wrote to, so a reply or a known correspondent's junk mail can be found (issue 1231) --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 12 ++++ .../mesh-mail/cmd/mesh-mail/search_test.go | 42 ++++++++++++ modules/mesh-mail/cmd/mesh-mail/store.go | 65 +++++++++++++++++-- 3 files changed, 115 insertions(+), 4 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index 7e8e8139..a02cf46c 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -516,6 +516,18 @@ func searchWhere(q Query) (whereSQL string, args []any) { if q.Unread != nil { where = append(where, fmt.Sprintf("(%s = any(flags)) = %s", param(`\Seen`), param(!*q.Unread))) } + if q.InReplyTo != "" { + 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 ( + 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')))`) + } 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 a76856c6..9c69879e 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -432,3 +432,45 @@ func TestGroupBySenderOrdersByCountDescending(t *testing.T) { } }) } + +// 1231/AC8: in-reply-to a mail I sent, combined with from (its recipient), answers the replies to it; alone, it +// answers every reply in the thread's direct answer set. +func TestSearchFiltersByInReplyTo(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + withFrom, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, InReplyTo: "", + From: []string{"bob@example.org"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, withFrom, "") + + alone, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, InReplyTo: "", Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, alone, "") + + // replies to , a different mail, so it never answers here. + other, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, InReplyTo: "", Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, other, "") + }) +} + +// 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. +func TestSearchFiltersByFromSomeoneIWroteTo(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{"mesh"}, 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 5253c4ee..dd548749 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -134,6 +134,15 @@ type Query struct { // break ascending on (account, folder, uid). GroupBySender ignores both: its own order is fixed (AC10). OrderBy string OrderDesc bool + + // InReplyTo (issue 1231, AC8) matches the In-Reply-To header (in_reply_to, schema.sql) exactly; empty means + // no filter. Combined with From, it finds the replies from one recipient; alone, every reply in the thread's + // direct answer set. + InReplyTo string + + // FromSomeoneIWroteTo (issue 1231, AC14) is true when the sender's address is a recipient (to or cc) of some + // mail in the same account's Sent folder; false (the default) means no filter. + FromSomeoneIWroteTo bool } // SenderGroup is one line of mail_search's group-by-sender answer (issue 1230, AC10): the sender's address and @@ -287,11 +296,20 @@ func (s *memStore) Get(_ context.Context, account, folder string, uid uint32) (M } // matchesQuery is every filter of Query but its order and its limit, shared by Search and GroupBySender so the -// two never drift apart (mirrored, condition for condition, by PgStore's own WHERE builder, pgstore.go). -func matchesQuery(m Message, q Query, words []string) bool { +// two never drift apart (mirrored, condition for condition, by PgStore's own WHERE builder, pgstore.go). wroteTo +// is wroteToByAccount's answer, needed only when q.FromSomeoneIWroteTo is set. +func matchesQuery(m Message, q Query, words []string, wroteTo map[string]map[string]bool) bool { if !contains(q.Accounts, m.Account) || (q.Folder != "" && q.Folder != m.Folder) { return false } + // 1231/AC8: the In-Reply-To header, matched exactly. + if q.InReplyTo != "" && m.InReplyTo != q.InReplyTo { + return false + } + // 1231/AC14: the sender's address is a recipient (to or cc) of some mail in the same account's Sent folder. + if q.FromSomeoneIWroteTo && !anyAddressIn(m.From, wroteTo[m.Account]) { + return false + } // 1228/AC2, 1228/AC3, 1228/AC4, 1228/AC6(cc); 1230/AC11 (From's several domains): the address and date-range // filters, mirroring PgStore.Search. if !addressMatchesAny(m.From, q.From) || !addressMatches(m.To, q.To) || !addressMatches(m.Cc, q.Cc) { @@ -327,9 +345,10 @@ func (s *memStore) Search(_ context.Context, q Query) ([]Message, error) { s.mu.Lock() defer s.mu.Unlock() words := strings.Fields(strings.ToLower(q.Text)) + wroteTo := s.wroteToByAccount(q.FromSomeoneIWroteTo) var out []Message for _, m := range s.messages { - if matchesQuery(m, q, words) { + if matchesQuery(m, q, words, wroteTo) { m.Body = "" out = append(out, m) } @@ -413,10 +432,11 @@ func (s *memStore) GroupBySender(_ context.Context, q Query) ([]SenderGroup, err s.mu.Lock() defer s.mu.Unlock() words := strings.Fields(strings.ToLower(q.Text)) + wroteTo := s.wroteToByAccount(q.FromSomeoneIWroteTo) byAddress := map[string]*SenderGroup{} var order []string for _, m := range s.messages { - if !matchesQuery(m, q, words) { + if !matchesQuery(m, q, words, wroteTo) { continue } for _, p := range m.From { @@ -544,6 +564,43 @@ func addressMatchesAny(ps []Person, filters []string) bool { 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. +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") { + continue + } + set, ok := out[m.Account] + if !ok { + set = map[string]bool{} + out[m.Account] = set + } + for _, p := range append(append([]Person{}, m.To...), m.Cc...) { + set[strings.ToLower(p.Address)] = true + } + } + return out +} + +// anyAddressIn is 1231/AC14: whether any of ps's addresses (case-insensitive) is in set; an unset or empty set +// (the account wrote to nobody, or wroteTo was never asked for) matches nobody. +func anyAddressIn(ps []Person, set map[string]bool) bool { + for _, p := range ps { + if set[strings.ToLower(p.Address)] { + return true + } + } + return false +} + // wordRegexp is a whole-word, case-insensitive match (issue 1229, AC12): \b, Go's RE2 word boundary, treats a // dash as a non-word character, so a hyphenated word such as "4711-882" is bounded at both ends and never matched // by only its digits. From b60ee706d40fbd89e7dd810e25c48ce83ab2396a Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:46:58 +0200 Subject: [PATCH 05/10] mesh-mail: mail_search's tool makes the query optional and offers every filter and order-by (one schema for mail_search and mail-store.search), validated before the index is ever asked (issue 1232) --- modules/mesh-mail/README.md | 2 +- modules/mesh-mail/cmd/mesh-mail/tools.go | 280 ++++++++++++++++-- modules/mesh-mail/cmd/mesh-mail/tools_test.go | 122 ++++++++ 3 files changed, 380 insertions(+), 24 deletions(-) diff --git a/modules/mesh-mail/README.md b/modules/mesh-mail/README.md index f7b77338..142579c0 100644 --- a/modules/mesh-mail/README.md +++ b/modules/mesh-mail/README.md @@ -61,7 +61,7 @@ The password of an account is the own secret `mail-credential-`, given | Tool | Answers | |---|---| | `mail_accounts` | each account's kind, state and why, when its server last answered, its folders; the address only to a caller granted it | -| `mail_search` | messages of the accounts the caller is granted, by every word of a query, newest first, no body | +| `mail_search` | messages of the accounts the caller is granted, no body: optional query words, and every filter combined (account, folder, from/to/cc, a date range, subject/body words, attachment, read/unread, in-reply-to, from-someone-i-wrote-to); ordered by date (default), sender, subject or size, ascending or descending; group-by-sender answers one line per sender instead. The newest mails of the granted accounts with no filter at all | | `mail_read` | one message's metadata and text, or one attachment's bytes, to a caller granted its account | | `mail_health` | healthy; unhealthy naming the account and why (connection down, or no answer for five minutes); waits naming `mail-credential-` while its password is not given | | `mail_send` | raises an ask on the operator channel (the account, the recipients, the subject, the first lines) for a caller granted the account; answers at once with the ask's id. Sends nothing itself | diff --git a/modules/mesh-mail/cmd/mesh-mail/tools.go b/modules/mesh-mail/cmd/mesh-mail/tools.go index 51c434df..ef6596b6 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools.go @@ -103,6 +103,51 @@ func (a access) refuse(caller, account string) error { return refused(caller, account) } +// searchArgs names mail_search's own arguments, used both for its schema (issue 1232, AC17) and for only's +// guard, so the two can never drift apart. +var searchArgs = []string{ + "query", "account", "folder", "limit", "from", "to", "cc", "on-or-after", "before", "subject-words", + "subject-words-any", "body-words", "body-words-any", "has-attachment", "attachment-type", "attachment-name", + "unread", "order-by", "order", "group-by-sender", "in-reply-to", "from-someone-i-wrote-to", +} + +// searchToolSchema is mail_search's one schema (issue 1232: SearchTool and SeatSearchTool share it, not two +// written by hand): query is optional, and every filter and order-by names its own form (AC17). +func searchToolSchema() map[string]any { + return schema(nil, map[string]any{ + "query": text("words to find across subject, text, sender or recipient, every one required; optional — " + + "with no filter at all, the newest mails of your granted accounts"), + "account": text("one account by its name in mail_accounts; every account you are granted when left out"), + "folder": text("one folder, such as INBOX; every folder when left out"), + "limit": integer("how many to answer (lines, with group-by-sender), at most 50 (20 when left out)"), + "from": stringArray("filter by sender: one or more addresses or domains (a domain, no \"@\", matches every " + + "address of it), case-insensitive; any one of them matching is enough"), + "to": text("filter by recipient: an address or a domain, case-insensitive"), + "cc": text("filter by a cc recipient: an address or a domain, case-insensitive"), + "on-or-after": text("only mails dated on or after this (RFC3339, or YYYY-MM-DD for the whole day)"), + "before": text("only mails dated before this (RFC3339, or YYYY-MM-DD for the whole day)"), + "subject-words": stringArray("every one of these whole words (case-insensitive) must be in the subject, " + + "unless subject-words-any"), + "subject-words-any": boolean("subject-words matches when any one of them is found, not every one"), + "body-words": stringArray("every one of these whole words (case-insensitive) must be in the text, unless " + + "body-words-any"), + "body-words-any": boolean("body-words matches when any one of them is found, not every one"), + "has-attachment": boolean("only mails carrying at least one attachment"), + "attachment-type": text("only mails carrying an attachment of this exact media type, case-insensitive; " + + "combined with attachment-name, either suffices for the same attachment"), + "attachment-name": text("only mails carrying an attachment whose name matches this glob (such as \"*.pdf\"), " + + "case-insensitive; combined with attachment-type, either suffices for the same attachment"), + "unread": boolean("true for only unread mail (no \\Seen flag), false for only read mail; every mail when left out"), + "order-by": enumText("sort by this field (date, the default, unless order-by is itself left out)", "date", "sender", "subject", "size"), + "order": enumText("ascending or descending; desc when left out (newest or largest first)", "asc", "desc"), + "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"), + }) +} + // Tools are the module's tools. func (b *Bundle) Tools() []stdio.Tool { return []stdio.Tool{ @@ -114,17 +159,18 @@ func (b *Bundle) Tools() []stdio.Tool { Input: schema(nil, map[string]any{}), Run: b.accountsTool}, {Name: SearchTool, - Description: "Search the mail of the accounts you are granted: every word of the query must be in the subject, the " + - "text, a sender or a recipient. Answers the newest first, at most 50: account, folder, uid, message id, date, " + - "sender, recipients, subject, size, flags and attachment references, never a body. Refused for an account you " + - "are not granted, and when you are granted none. Read-only.", - Input: schema([]string{"query"}, map[string]any{ - "query": text("the words to find, at most 200 characters"), - "account": text("one account by its name in mail_accounts; every account you are granted when left out"), - "folder": text("one folder, such as INBOX; every folder when left out"), - "limit": integer("how many to answer, at most 50 (20 when left out)"), - }), - Run: b.searchTool}, + Description: "Search the mail of the accounts you are granted. Query is optional: with no filter at all, the " + + "newest mails of your granted accounts. Every filter is optional and combines with the others (and): account, " + + "folder, from/to/cc (an address or a domain; from takes several, any one matching), a date range, " + + "subject-words/body-words (each all unless its own -any), has-attachment, attachment-type/attachment-name " + + "(either matching an attachment suffices), unread, in-reply-to, from-someone-i-wrote-to. Order by date (the " + + "default), sender, subject or size, asc or desc; group-by-sender answers one line per sender (address, name, " + + "count, newest) instead. At most 50 (20 default). Answers account, folder, uid, message id, date, sender, " + + "recipients, subject, size, flags and attachment references, never a body. Refused, naming the argument, for " + + "an unknown order-by, a limit over 50, a date that does not parse, or an account you are not granted — before " + + "the index is ever asked. Read-only.", + Input: searchToolSchema(), + Run: b.searchTool}, {Name: ReadTool, Description: "Read one message of an account you are granted, as an event or mail_search names it (account, folder, " + "uid): its metadata and its text (the plain part, or the HTML part as text, at most 64 KiB here). With " + @@ -171,13 +217,8 @@ func (b *Bundle) Tools() []stdio.Tool { Run: b.accountsTool}, {Name: SeatSearchTool, Description: "The mail-store seat's search verb (novox/hq ADR 0378 §2): the same as mail_search.", - Input: schema([]string{"query"}, map[string]any{ - "query": text("the words to find, at most 200 characters"), - "account": text("one account by its name in mail_accounts; every account you are granted when left out"), - "folder": text("one folder, such as INBOX; every folder when left out"), - "limit": integer("how many to answer, at most 50 (20 when left out)"), - }), - Run: b.searchTool}, + Input: searchToolSchema(), + Run: b.searchTool}, {Name: SeatReadTool, Description: "The mail-store seat's read verb (novox/hq ADR 0378 §2): the same as mail_read.", Input: schema([]string{"account", "folder", "uid"}, map[string]any{ @@ -274,13 +315,16 @@ func (b *Bundle) accountsTool(args map[string]any) (any, error) { return answer, nil } +// searchTool is mail_search and mail-store.search (issue 1232): query is optional (AC1), every filter and +// order-by issues 1228-1231 built is offered, and every argument is parsed and validated — naming it — before +// the index is ever asked (AC16). func (b *Bundle) searchTool(args map[string]any) (any, error) { - if err := only(args, "query", "account", "folder", "limit"); err != nil { + if err := only(args, searchArgs...); err != nil { return nil, err } query, _ := args["query"].(string) - if strings.TrimSpace(query) == "" || len(query) > MostQuery { - return nil, fmt.Errorf("%s needs a query of 1 to %d characters", SearchTool, MostQuery) + if len(query) > MostQuery { + return nil, fmt.Errorf("query is at most %d characters", MostQuery) } account, _ := args["account"].(string) folder, _ := args["folder"].(string) @@ -291,6 +335,64 @@ func (b *Bundle) searchTool(args map[string]any) (any, error) { if limit < 1 || limit > MostResults { return nil, fmt.Errorf("limit is 1 to %d", MostResults) } + from, err := stringsArg(args, "from") + if err != nil { + return nil, err + } + to, _ := args["to"].(string) + cc, _ := args["cc"].(string) + onOrAfter, err := dateArg(args, "on-or-after") + if err != nil { + return nil, err + } + before, err := dateArg(args, "before") + if err != nil { + return nil, err + } + subjectWords, err := stringsArg(args, "subject-words") + if err != nil { + return nil, err + } + subjectWordsAny, err := flag(args, "subject-words-any") + if err != nil { + return nil, err + } + bodyWords, err := stringsArg(args, "body-words") + if err != nil { + return nil, err + } + bodyWordsAny, err := flag(args, "body-words-any") + if err != nil { + return nil, err + } + hasAttachment, err := flag(args, "has-attachment") + if err != nil { + return nil, err + } + attachmentType, _ := args["attachment-type"].(string) + attachmentName, _ := args["attachment-name"].(string) + unread, err := triBool(args, "unread") + if err != nil { + return nil, err + } + orderBy, err := orderByArg(args) + if err != nil { + return nil, err + } + orderDesc, err := orderArg(args) + if err != nil { + return nil, err + } + groupBySender, err := flag(args, "group-by-sender") + if err != nil { + return nil, err + } + inReplyTo, _ := args["in-reply-to"].(string) + fromSomeoneIWroteTo, err := flag(args, "from-someone-i-wrote-to") + if err != nil { + return nil, err + } + accounts, grants, err := b.settings() if err != nil { return nil, err @@ -315,7 +417,23 @@ func (b *Bundle) searchTool(args map[string]any) (any, error) { if err != nil { return nil, fmt.Errorf("the index cannot be reached: %v", err) } - found, err := st.Search(ctx, Query{Text: query, Accounts: granted, Folder: folder, Limit: limit}) + q := Query{ + Text: query, Accounts: granted, Folder: folder, Limit: limit, + From: from, To: to, Cc: cc, OnOrAfter: onOrAfter, Before: before, + SubjectWords: subjectWords, SubjectWordsAny: subjectWordsAny, BodyWords: bodyWords, BodyWordsAny: bodyWordsAny, + HasAttachment: hasAttachment, AttachmentType: attachmentType, AttachmentNamePattern: attachmentName, + Unread: unread, OrderBy: orderBy, OrderDesc: orderDesc, + InReplyTo: inReplyTo, FromSomeoneIWroteTo: fromSomeoneIWroteTo, + } + // 1230/AC10: group-by-sender answers a different shape, one line per sender, not the mails themselves. + if groupBySender { + groups, err := st.GroupBySender(ctx, q) + if err != nil { + return nil, err + } + return map[string]any{"searched": granted, "senders": groups}, nil + } + found, err := st.Search(ctx, q) if err != nil { return nil, err } @@ -508,8 +626,124 @@ func number(args map[string]any, key string, otherwise int) (int, error) { return int(f), nil } -func text(d string) map[string]any { return map[string]any{"type": "string", "description": d} } +// flag is 1232: a plain optional boolean filter, false (no filter) when left out. +func flag(args map[string]any, key string) (bool, error) { + v, ok := args[key] + if !ok || v == nil { + return false, nil + } + b, ok := v.(bool) + if !ok { + return false, fmt.Errorf("%s is true or false", key) + } + return b, nil +} + +// triBool is 1232 (mail_search's unread): nil when left out (no filter), else the given true or false. +func triBool(args map[string]any, key string) (*bool, error) { + v, ok := args[key] + if !ok || v == nil { + return nil, nil + } + b, ok := v.(bool) + if !ok { + return nil, fmt.Errorf("%s is true or false", key) + } + return &b, nil +} + +// stringsArg is 1232: an optional list of strings (mail_search's from, subject-words, body-words), refusing, +// naming key, anything that is not a list of strings, before the index is ever asked (AC16). +func stringsArg(args map[string]any, key string) ([]string, error) { + v, ok := args[key] + if !ok || v == nil { + return nil, nil + } + list, ok := v.([]any) + if !ok { + return nil, fmt.Errorf("%s is a list of words", key) + } + out := make([]string, 0, len(list)) + for _, item := range list { + s, ok := item.(string) + if !ok { + return nil, fmt.Errorf("%s is a list of words", key) + } + out = append(out, s) + } + return out, nil +} + +// dateLayouts is 1232/AC16: the forms on-or-after and before parse, RFC3339 first, then a bare day (the whole +// day, UTC). +var dateLayouts = []string{time.RFC3339, "2006-01-02"} + +// dateArg is 1232/AC16: a date that does not parse is refused naming key, before the index is ever asked. +func dateArg(args map[string]any, key string) (time.Time, error) { + v, ok := args[key] + if !ok || v == nil { + return time.Time{}, nil + } + s, ok := v.(string) + if !ok { + return time.Time{}, fmt.Errorf("%s is a date", key) + } + for _, layout := range dateLayouts { + if t, err := time.Parse(layout, s); err == nil { + return t.UTC(), nil + } + } + return time.Time{}, fmt.Errorf("%s is not a date (RFC3339, or YYYY-MM-DD): %q", key, s) +} + +// orderByNames is 1230/AC17's fixed set; order-by outside it is refused by name (AC16), before the index is +// ever asked. +var orderByNames = map[string]bool{"date": true, "sender": true, "subject": true, "size": true} + +func orderByArg(args map[string]any) (string, error) { + v, ok := args["order-by"] + if !ok || v == nil { + return "", nil + } + s, ok := v.(string) + if !ok || !orderByNames[s] { + return "", errors.New("order-by is one of date, sender, subject, size") + } + return s, nil +} + +// orderArg is 1230: asc or desc; anything else is refused (AC16), before the index is ever asked. +func orderArg(args map[string]any) (bool, error) { + v, ok := args["order"] + if !ok || v == nil { + return false, nil + } + s, ok := v.(string) + if !ok { + return false, errors.New("order is asc or desc") + } + switch s { + case "asc": + return false, nil + case "desc": + return true, nil + default: + return false, errors.New("order is asc or desc") + } +} + +func text(d string) map[string]any { return map[string]any{"type": "string", "description": d} } + +// enumText is 1232/AC17: a string argument whose schema itself names the values it accepts (order-by, order). +func enumText(d string, values ...string) map[string]any { + return map[string]any{"type": "string", "description": d, "enum": values} +} + func integer(d string) map[string]any { return map[string]any{"type": "integer", "description": d} } +func boolean(d string) map[string]any { return map[string]any{"type": "boolean", "description": d} } +func stringArray(d string) map[string]any { + return map[string]any{"type": "array", "items": map[string]any{"type": "string"}, "description": d} +} func schema(required []string, props map[string]any) map[string]any { s := map[string]any{"type": "object", "properties": props, "additionalProperties": false} diff --git a/modules/mesh-mail/cmd/mesh-mail/tools_test.go b/modules/mesh-mail/cmd/mesh-mail/tools_test.go index fbc9c6c7..e81c1f2d 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools_test.go @@ -273,3 +273,125 @@ func TestARefusalSaysWhyTheGrantsCouldNotBeRead(t *testing.T) { t.Fatalf("accounts: %s", text) } } + +// 1228/AC1, 1232: with no filter at all (not even query), mail_search answers the newest mails of the granted +// accounts, not a refusal demanding a query. +func TestMailSearchWithNoFilterAnswersTheNewestGrantedMail(t *testing.T) { + b, _, _ := aBundle(t) + rt := startBundle(t, b.Tools()) + text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{}) + if isErr || !strings.Contains(text, `"message-id":"att@example.org"`) { + t.Fatalf("%v %s", isErr, text) + } +} + +// 1232/AC16: an unknown order-by, a limit over 50, a date that does not parse, or an account not granted are +// each refused naming the argument, and the index is never asked. +func TestMailSearchValidatesBeforeAskingTheIndex(t *testing.T) { + b, _, _ := aBundle(t) + calls := 0 + index := b.Index + b.Index = func(ctx context.Context) (Store, error) { calls++; return index(ctx) } + rt := startBundle(t, b.Tools()) + + cases := []struct { + name string + args map[string]any + want string + }{ + {"order-by", map[string]any{"order-by": "popularity"}, "order-by"}, + {"limit", map[string]any{"limit": 51}, "limit"}, + {"on-or-after", map[string]any{"on-or-after": "not a date"}, "on-or-after"}, + {"account", map[string]any{"account": "no-such-account"}, "no-such-account"}, + } + for _, c := range cases { + text, isErr := rt.call(SearchTool, "novox.triage", c.args) + if !isErr || !strings.Contains(text, c.want) { + t.Fatalf("%s: %v %s", c.name, isErr, text) + } + } + if calls != 0 { + t.Fatalf("the index was asked %d time(s) before a refusal", calls) + } +} + +// 1232/AC17: the schema describes every filter and order-by's own form (date, sender, subject, size, each +// ascending or descending). +func TestMailSearchSchemaOffersEveryFilterAndOrderBy(t *testing.T) { + b := &Bundle{} + var input map[string]any + for _, tool := range b.Tools() { + if tool.Name == SearchTool { + input = tool.Input + } + } + if input == nil { + t.Fatal("no mail_search tool") + } + props, _ := input["properties"].(map[string]any) + for _, name := range searchArgs { + if _, ok := props[name]; !ok { + t.Fatalf("mail_search's schema does not offer %q", name) + } + } + orderBy, _ := props["order-by"].(map[string]any) + if got, _ := orderBy["enum"].([]string); !sameStrings(got, []string{"date", "sender", "subject", "size"}) { + t.Fatalf("order-by's enum: %v", orderBy["enum"]) + } + order, _ := props["order"].(map[string]any) + if got, _ := order["enum"].([]string); !sameStrings(got, []string{"asc", "desc"}) { + t.Fatalf("order's enum: %v", order["enum"]) + } + // mail-store.search carries the very same schema (issue 1232: one schema, not two). + var seatInput map[string]any + for _, tool := range b.Tools() { + if tool.Name == SeatSearchTool { + seatInput = tool.Input + } + } + seatProps, _ := seatInput["properties"].(map[string]any) + if len(seatProps) != len(props) { + t.Fatalf("mail-store.search's schema has %d properties, mail_search's has %d", len(seatProps), len(props)) + } +} + +// 1232 (the end-to-end path of the others): from, unread, order-by/order and group-by-sender, called the way a +// caller calls them, through the MCP tool and its JSON arguments, not Go's Query directly. +func TestMailSearchEndToEndFiltersAndGroupBySender(t *testing.T) { + dir := t.TempDir() + env := Env{Accounts: filepath.Join(dir, "accounts.json"), Grants: filepath.Join(dir, "grants.json"), Secrets: dir, + Status: filepath.Join(dir, "health.json")} + writeFile(t, env.Accounts, stageOne) + writeFile(t, env.Grants, `{"grants": {"mesh": ["triage"]}}`) + st := newMemStore() + ctx := context.Background() + older := judgedAt.Add(-time.Hour) + if err := st.Add(ctx, Message{Account: "mesh", Folder: "INBOX", UID: 1, MessageID: "", Subject: "from alice", + Date: older, From: []Person{{Address: "alice@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{}, Attachments: []AttachmentRef{}, Body: "hi"}, nil); err != nil { + t.Fatal(err) + } + if err := st.Add(ctx, Message{Account: "mesh", Folder: "INBOX", UID: 2, MessageID: "", Subject: "from alice again", + Date: judgedAt, From: []Person{{Address: "alice@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, + Flags: []string{"\\Seen"}, Attachments: []AttachmentRef{}, Body: "hi again"}, nil); err != nil { + t.Fatal(err) + } + b := &Bundle{Env: env, Index: func(context.Context) (Store, error) { return st, nil }, Now: func() time.Time { return judgedAt }} + rt := startBundle(t, b.Tools()) + + unread := true + text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{"from": []any{"example.org"}, "unread": unread, + "order-by": "date", "order": "asc"}) + if isErr || !strings.Contains(text, "a@mesh") || strings.Contains(text, "b@mesh") { + t.Fatalf("%v %s", isErr, text) + } + + text, isErr = rt.call(SearchTool, "novox.triage", map[string]any{"group-by-sender": true}) + var grouped struct { + Senders []SenderGroup `json:"senders"` + } + if isErr || json.Unmarshal([]byte(text), &grouped) != nil || len(grouped.Senders) != 1 || + grouped.Senders[0].Address != "alice@example.org" || grouped.Senders[0].Count != 2 { + t.Fatalf("%v %s", isErr, text) + } +} From 1c3158e098f812e556497f119e4628955a8f9efc Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:52:36 +0200 Subject: [PATCH 06/10] mesh-mail: mail_search's address filter never treats % or _ as a wildcard, so a literal domain filter does not leak every mail in Postgres (issue 1228, review finding 4) --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 14 +++++++---- .../mesh-mail/cmd/mesh-mail/search_test.go | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index a02cf46c..bd06a046 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -397,15 +397,19 @@ func (s *PgStore) Get(ctx context.Context, account, folder string, uid uint32) ( // addressFilterSQL is 1228/AC2, 1228/AC4, 1228/AC6(cc): an EXISTS over the jsonb array column (recipients or // copied), matched against one already-lowercased parameter either as a whole address or, with no "@", as a -// domain (every address under it). column names a fixed set the Go code chooses, never caller input. +// domain (every address under it): right(address, length(domain)+1) = '@'||domain, an exact substring compare, +// not LIKE, so a filter holding "%" or "_" is never a wildcard (issue 1228, review round 1 finding 4: a filter +// from:["%"] matched every mail in Postgres and none in memStore, since memStore's strings.HasSuffix never +// treats "%" specially). column names a fixed set the Go code chooses, never caller input. func addressFilterSQL(column, param string) string { return fmt.Sprintf("exists (select 1 from jsonb_array_elements(%s) el where lower(el->>'address') = %s or "+ - "lower(el->>'address') like '%%@' || %s)", column, param, param) + "right(lower(el->>'address'), length(%s) + 1) = '@' || %s)", column, param, param, param) } // addressFilterSQLAny is 1228/AC3; 1230/AC11 ("several domains"): an EXISTS over column (sender), matched -// against any of filters, OR'd (each already-lowercased, each either a whole address or, with no "@", a domain). -// The whole list reaches SQL as one parameter, a text[]; never concatenated. +// against any of filters, OR'd (each already-lowercased, each either a whole address or, with no "@", a domain), +// the same exact substring compare as addressFilterSQL, never LIKE. The whole list reaches SQL as one +// parameter, a text[]; never concatenated. func addressFilterSQLAny(column string, filters []string, param func(any) string) string { lowered := make([]string, len(filters)) for i, f := range filters { @@ -413,7 +417,7 @@ func addressFilterSQLAny(column string, filters []string, param func(any) string } p := param(lowered) return fmt.Sprintf("exists (select 1 from jsonb_array_elements(%s) el, unnest(%s::text[]) f where "+ - "lower(el->>'address') = f or lower(el->>'address') like '%%@' || f)", column, p) + "lower(el->>'address') = f or right(lower(el->>'address'), length(f) + 1) = '@' || f)", column, p) } // wordFilterSQL is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15: one condition per word of the list, each a diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go index 9c69879e..8f7c21bf 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -208,6 +208,30 @@ func TestSearchFiltersByCcNotTo(t *testing.T) { }) } +// 1228 (review round 1, finding 4): a domain filter of "%" or "_" is never a LIKE wildcard; it matches nobody, +// the same in both stores (a Postgres-only bug matched every mail in Postgres, since '%@' || '%' is the +// wildcard '%@%', while memStore's strings.HasSuffix never treats '%' specially). +func TestSearchFiltersByDomainNeverTreatsPercentAsAWildcard(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{"mesh", "gmail"}, From: []string{"%"}, Limit: 50}) + if err != nil { + t.Fatal(err) + } + if len(got) != 0 { + t.Fatalf("from:[%%] matched %d mail(s): %v", len(got), got) + } + toUnderscore, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, To: "_", Limit: 50}) + if err != nil { + t.Fatal(err) + } + if len(toUnderscore) != 0 { + t.Fatalf("to:_ matched %d mail(s): %v", len(toUnderscore), toUnderscore) + } + }) +} + // 1229/AC6 (words part): cc and the word "renovatie" in subject or body, combined with the cc filter // from 1228: the mail with it only in to is still excluded. func TestSearchFiltersByCcAndSubjectOrBodyWord(t *testing.T) { From a3476a7bf26bbadce56885102c7cd79e458573a9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 14:57:26 +0200 Subject: [PATCH 07/10] 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)"), }) } From d1a9056a1c73db317deb792e3c9f62c5ddb4bf73 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 15:02:40 +0200 Subject: [PATCH 08/10] mesh-mail: mail_search gains words/words-any, a word found in subject or body, since subject-words and body-words together only ever meant and (issue 1229, review finding 3; also proves AC9's type-or-name independently and AC2+unread combined) --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 29 ++++-- .../mesh-mail/cmd/mesh-mail/search_test.go | 95 +++++++++++++++---- modules/mesh-mail/cmd/mesh-mail/store.go | 44 ++++++--- modules/mesh-mail/cmd/mesh-mail/tools.go | 27 ++++-- 4 files changed, 151 insertions(+), 44 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index 630121fe..66272106 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -420,18 +420,25 @@ func addressFilterSQLAny(column string, filters []string, param func(any) string "lower(el->>'address') = f or right(lower(el->>'address'), length(f) + 1) = '@' || f)", column, p) } -// wordFilterSQL is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15: one condition per word of the list, each a -// whole-word, case-insensitive match (Postgres's \y, its own word boundary) against column, so a hyphenated word -// such as "4711-882" is never matched by only its digits (AC12); joined with "and" ("all", the default) or "or" -// ("any"). Empty answers "" (no filter). The word itself always reaches SQL as param's parameter, never -// concatenated; only its \y anchors are fixed SQL text. -func wordFilterSQL(column string, words []string, any bool, param func(any) string) string { +// wordFilterSQL is 1229/AC13 (one column: SubjectWords or BodyWords); AC5, AC6(words), AC12, AC15 (review round +// 1 finding 3: columns holds both subject and body for Words, so a word counts in either — "the word in subject +// or body"). One condition per word of the list, each a whole-word, case-insensitive match (Postgres's \y, its +// own word boundary) against any one of columns, so a hyphenated word such as "4711-882" is never matched by +// only its digits (AC12); the per-word conditions join with "and" ("all", the default) or "or" ("any"). Empty +// answers "" (no filter). The word itself always reaches SQL as param's parameter, never concatenated; only its +// \y anchors and the column names (a fixed set the Go code chooses) are fixed SQL text. +func wordFilterSQL(columns []string, words []string, any bool, param func(any) string) string { if len(words) == 0 { return "" } parts := make([]string, 0, len(words)) for _, w := range words { - parts = append(parts, fmt.Sprintf(`%s ~* ('\y' || %s || '\y')`, column, param(quoteRegex(w)))) + p := param(quoteRegex(w)) + cols := make([]string, len(columns)) + for i, col := range columns { + cols[i] = fmt.Sprintf(`%s ~* ('\y' || %s || '\y')`, col, p) + } + parts = append(parts, "("+strings.Join(cols, " or ")+")") } op := " and " if any { @@ -508,10 +515,14 @@ func searchWhere(q Query) (whereSQL string, args []any) { if !q.Before.IsZero() { where = append(where, "date < "+param(q.Before)) } - if c := wordFilterSQL("subject", q.SubjectWords, q.SubjectWordsAny, param); c != "" { + if c := wordFilterSQL([]string{"subject"}, q.SubjectWords, q.SubjectWordsAny, param); c != "" { where = append(where, c) } - if c := wordFilterSQL("body", q.BodyWords, q.BodyWordsAny, param); c != "" { + if c := wordFilterSQL([]string{"body"}, q.BodyWords, q.BodyWordsAny, param); c != "" { + where = append(where, c) + } + // 1229/AC5, AC6(words), AC12, AC15 (review round 1 finding 3): a word found in subject or body. + if c := wordFilterSQL([]string{"subject", "body"}, q.Words, q.WordsAny, param); c != "" { where = append(where, c) } if q.HasAttachment { diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go index 625813dd..f31015a4 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -50,24 +50,28 @@ func searchFixtures() []Message { {Account: "mesh", Folder: "INBOX", UID: 6, MessageID: "", InReplyTo: "", Subject: "Re: quick note", Date: at(12, 30), From: []Person{{Name: "Bob", Address: "bob@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "sounds good, thanks for the note", Size: 400}, - // 1229/AC5: from a domain (google.com), has-attachment, name *.pdf and the word "factuur". + // 1229/AC5: from a domain (google.com), has-attachment, name *.pdf and the word "factuur" — in the + // subject only (review round 1 finding 3), so Words's "subject or body" is genuinely exercised, not + // merely a body match that would pass even with a subject-only bug. {Account: "mesh", Folder: "INBOX", UID: 7, MessageID: "", Subject: "the factuur for Q3", Date: at(13, 0), From: []Person{{Name: "Erik", Address: "erik@google.com"}}, To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a4", Size: 200, Type: "application/pdf", Name: "factuur.pdf"}}, - Body: "see the attached factuur, due on receipt", Size: 850}, - // 1229/AC12: a word with digits and a dash ("4711-882"), matched whole. + Body: "see the attached file, due on receipt", Size: 850}, + // 1229/AC12: a word with digits and a dash ("4711-882"), matched whole, in the subject only (review + // round 1 finding 3). {Account: "mesh", Folder: "INBOX", UID: 8, MessageID: "", Subject: "order 4711-882 confirmed", Date: at(14, 0), From: []Person{{Address: "ops@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, - Flags: []string{}, Attachments: []AttachmentRef{}, Body: "your order 4711-882 has shipped", Size: 300}, + Flags: []string{}, Attachments: []AttachmentRef{}, Body: "your order has shipped", Size: 300}, // Control for 1229/AC12: "4711" and "882" appear as separate tokens, never hyphenated together. {Account: "mesh", Folder: "INBOX", UID: 9, MessageID: "", Subject: "order 4711 shipped, item 882 noted", Date: at(14, 5), From: []Person{{Address: "ops@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{}, Body: "item 4711 and item 882 are unrelated", Size: 300}, - // 1229/AC15 (the "any" word list): only "verzekering" is present, not "schade", with an attachment. + // 1229/AC15 (the "any" word list): only "verzekering" is present, not "schade" — in the subject only + // (review round 1 finding 3) — with an attachment. {Account: "mesh", Folder: "INBOX", UID: 10, MessageID: "", Subject: "verzekering aanvraag", Date: at(14, 10), From: []Person{{Address: "broker@example.org"}}, To: []Person{{Address: "jschoubben@novox.be"}}, Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a5", Size: 150, Type: "application/pdf", Name: "polis.pdf"}}, - Body: "nieuwe polis aanvraag for verzekering", Size: 450}, + Body: "nieuwe polis aanvraag", Size: 450}, // mesh/Sent --------------------------------------------------------------------------------------------------- {Account: "mesh", Folder: "Sent", UID: 1, MessageID: "", InReplyTo: "", Subject: "Re: October invoice", @@ -94,6 +98,17 @@ func searchFixtures() []Message { Date: time.Date(2026, 8, 1, 7, 0, 0, 0, time.UTC), From: []Person{{Name: "Someone", Address: "someone@example.org"}}, 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}, + // 1229/AC9 (review round 1, "also in this pass"): matches the type-or-name attachment filter by its + // type alone — its name does not match the glob. + {Account: "gmail", Folder: "INBOX", UID: 3, MessageID: "", Subject: "another invite", + Date: time.Date(2026, 10, 19, 9, 0, 0, 0, time.UTC), From: []Person{{Address: "dave@example.org"}}, + To: []Person{{Address: "me@gmail.example"}}, Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a6", Size: 300, Type: "text/calendar", Name: "event.bin"}}, + Body: "a second invite", Size: 700}, + // 1229/AC9: matches the same filter by its name alone — its type is not text/calendar. + {Account: "gmail", Folder: "INBOX", UID: 4, MessageID: "", Subject: "yet another invite", + Date: time.Date(2026, 10, 19, 9, 5, 0, 0, time.UTC), From: []Person{{Address: "dave@example.org"}}, + To: []Person{{Address: "me@gmail.example"}}, Flags: []string{}, Attachments: []AttachmentRef{{Bucket: "mail", Key: "a7", Size: 300, Type: "application/octet-stream", Name: "invite.ics"}}, + Body: "a third invite", Size: 700}, // 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", @@ -249,7 +264,7 @@ func TestSearchFiltersByCcAndSubjectOrBodyWord(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{"mesh"}, Cc: "jschoubben@novox.be", BodyWords: []string{"renovatie"}, Limit: 10}) + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Cc: "jschoubben@novox.be", Words: []string{"renovatie"}, Limit: 10}) if err != nil { t.Fatal(err) } @@ -258,12 +273,14 @@ func TestSearchFiltersByCcAndSubjectOrBodyWord(t *testing.T) { } // 1229/AC5: from a domain, has-attachment, an attachment name pattern and a word in subject or body all combine. +// The word ("factuur") lives only in the subject of the matching mail (review round 1 finding 3), so this +// genuinely exercises Words's "or body", not merely a body match. func TestSearchFiltersByDomainAttachmentAndWord(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{"mesh"}, From: []string{"google.com"}, HasAttachment: true, - AttachmentNamePattern: "*.pdf", BodyWords: []string{"factuur"}, Limit: 10}) + AttachmentNamePattern: "*.pdf", Words: []string{"factuur"}, Limit: 10}) if err != nil { t.Fatal(err) } @@ -273,26 +290,53 @@ func TestSearchFiltersByDomainAttachmentAndWord(t *testing.T) { }) } -// 1229/AC9: on-or-after a date and an attachment type or name pattern (either suffices) answers a calendar invite. +// Review round 1, finding 3: SubjectWords and BodyWords together mean "and" (a word in each field, possibly two +// different fields at once) — never a stand-in for "the word in subject or body". factuur1@mesh's "factuur" is +// only in its subject, so the two together find nothing; Words finds it. +func TestWordsIsNotSubjectWordsAndBodyWordsTogether(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + asAnd, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, SubjectWords: []string{"factuur"}, + BodyWords: []string{"factuur"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + if len(asAnd) != 0 { + t.Fatalf("subject-words and body-words together: %v", asAnd) + } + asWords, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Words: []string{"factuur"}, Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameSet(t, asWords, "") + }) +} + +// 1229/AC9: on-or-after a date and an attachment type or name pattern (either suffices) answers a calendar +// invite. cal2@gmail matches by its type alone (its name does not match the glob) and cal3@gmail by its name +// alone (its type is not text/calendar) — review round 1, "also in this pass": proving each half of the "or" +// independently, not only cal1@mesh, which matches both at once and so would pass even if one half were broken. func TestSearchFiltersByDateAndAttachmentTypeOrName(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{"mesh"}, OnOrAfter: time.Date(2026, 10, 19, 0, 0, 0, 0, time.UTC), + got, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, OnOrAfter: time.Date(2026, 10, 19, 0, 0, 0, 0, time.UTC), AttachmentType: "text/calendar", AttachmentNamePattern: "*.ics", Limit: 10}) if err != nil { t.Fatal(err) } - sameSet(t, got, "") + sameSet(t, got, "", "", "") }) } -// 1229/AC12: a word with digits and a dash is matched whole, not as its separate parts. +// 1229/AC12: a word with digits and a dash is matched whole, not as its separate parts. It lives only in the +// subject of the matching mail (review round 1 finding 3), so Words's "subject or body" is genuinely exercised. func TestSearchFiltersBySubjectOrBodyWordWhole(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{"mesh", "gmail"}, BodyWords: []string{"4711-882"}, Limit: 10}) + got, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, Words: []string{"4711-882"}, Limit: 10}) if err != nil { t.Fatal(err) } @@ -323,13 +367,14 @@ func TestSearchFiltersByDomainAndSubjectWordsAcrossAccounts(t *testing.T) { } // 1229/AC15: a word list marked "any" (schade or verzekering) combined with has-attachment; the word list's "any" -// mode matches a mail holding only one of the words, not both. +// mode matches a mail holding only one of the words, not both. polis1@mesh's "verzekering" lives only in its +// subject (review round 1 finding 3), so Words's "subject or body" is genuinely exercised. func TestSearchFiltersByAnyWordsAndHasAttachment(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{"mesh", "gmail"}, BodyWords: []string{"schade", "verzekering"}, - BodyWordsAny: true, HasAttachment: true, Limit: 10}) + got, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, Words: []string{"schade", "verzekering"}, + WordsAny: true, HasAttachment: true, Limit: 10}) if err != nil { t.Fatal(err) } @@ -337,7 +382,7 @@ func TestSearchFiltersByAnyWordsAndHasAttachment(t *testing.T) { // Without "any" (the default, "all"), only the mail holding both words together matches: polis1@mesh has // "verzekering" alone, so it drops out. - all, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, BodyWords: []string{"schade", "verzekering"}, + all, err := st.Search(ctx, Query{Accounts: []string{"mesh", "gmail"}, Words: []string{"schade", "verzekering"}, HasAttachment: true, Limit: 10}) if err != nil { t.Fatal(err) @@ -367,6 +412,22 @@ func TestSearchFiltersByUnreadFlag(t *testing.T) { }) } +// 1228/AC2 combined with 1229's unread, in one query (review round 1, "also in this pass"): on-or-after today +// and unread together exclude cal1@mesh, dated today but read (\Seen), which either filter alone would include. +func TestSearchFiltersByOnOrAfterAndUnreadTogether(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + addFixtures(t, ctx, st) + unread := true + got, err := st.Search(ctx, Query{Accounts: []string{"mesh"}, Folder: "INBOX", + OnOrAfter: time.Date(2026, 10, 19, 0, 0, 0, 0, time.UTC), Unread: &unread, Limit: 20}) + if err != nil { + t.Fatal(err) + } + sameSet(t, got, "", "", "", "", "", "") + }) +} + // sameOrder asserts got's message-ids equal want, in that exact order (unlike sameSet, which ignores order). func sameOrder(t *testing.T, got []Message, want ...string) { t.Helper() diff --git a/modules/mesh-mail/cmd/mesh-mail/store.go b/modules/mesh-mail/cmd/mesh-mail/store.go index 2adf3d3f..ae44c3d4 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -106,15 +106,22 @@ type Query struct { OnOrAfter time.Time Before time.Time - // SubjectWords and BodyWords (issue 1229, AC5, AC6's words part, AC12, AC13, AC15) are each a whole word, - // matched case-insensitively against Subject or Body respectively (a word with digits and a dash, such as - // "4711-882", is matched whole, never by its separate parts, AC12). By default every word of the list must be - // found ("all"); the *WordsAny flag switches that list to "any of them" (AC15). Empty means no filter. + // SubjectWords and BodyWords (issue 1229, AC13) are each a whole word, matched case-insensitively against + // Subject or Body respectively, and only that field (a word with digits and a dash, such as "4711-882", is + // matched whole, never by its separate parts, AC12). By default every word of the list must be found ("all"); + // the *WordsAny flag switches that list to "any of them" (AC15). Empty means no filter. SubjectWords []string SubjectWordsAny bool BodyWords []string BodyWordsAny bool + // Words (issue 1229, AC5, AC6's words part, AC12, AC15; review round 1 finding 3) is the same whole-word, + // case-insensitive match as SubjectWords/BodyWords, but each word may be found in either field, Subject or + // Body: "the word in subject or body", as the acceptance criteria ask it, where SubjectWords/BodyWords alone + // would wrongly need it in one specific field. WordsAny is the same "any" switch. Empty means no filter. + Words []string + WordsAny bool + // HasAttachment, AttachmentType and AttachmentNamePattern (issue 1229, AC5, AC9) filter by attachment: // HasAttachment requires at least one; AttachmentType (an exact media type, case-insensitive) and // AttachmentNamePattern (a shell glob such as "*.pdf", case-insensitive) each require one attachment matching @@ -321,11 +328,16 @@ func matchesQuery(m Message, q Query, words []string, wroteTo map[string]map[str if !q.Before.IsZero() && !m.Date.Before(q.Before) { return false } - // 1229/AC5, 1229/AC6(words), 1229/AC9, 1229/AC12, 1229/AC13, 1229/AC15: the word, attachment and - // read/unread filters, mirroring PgStore.Search. - if !wordMatches(m.Subject, q.SubjectWords, q.SubjectWordsAny) || !wordMatches(m.Body, q.BodyWords, q.BodyWordsAny) { + // 1229/AC13: the subject-only and body-only word filters, mirroring PgStore.Search. + if !wordMatches([]string{m.Subject}, q.SubjectWords, q.SubjectWordsAny) || + !wordMatches([]string{m.Body}, q.BodyWords, q.BodyWordsAny) { return false } + // 1229/AC5, AC6(words), AC12, AC15 (review round 1 finding 3): a word found in subject or body. + if !wordMatches([]string{m.Subject, m.Body}, q.Words, q.WordsAny) { + return false + } + // 1229/AC9: the attachment and read/unread filters, mirroring PgStore.Search. if !attachmentMatches(m.Attachments, q.HasAttachment, q.AttachmentType, q.AttachmentNamePattern) { return false } @@ -621,14 +633,24 @@ func wordRegexp(word string) *regexp.Regexp { return regexp.MustCompile(`(?i)\b` + regexp.QuoteMeta(word) + `\b`) } -// wordMatches is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15: an empty list matches every message; otherwise -// every word of the list must be found in text ("all"), or at least one when any is true. -func wordMatches(text string, words []string, any bool) bool { +// wordMatches is 1229/AC5, AC6(words), AC9, AC12, AC13, AC15; review round 1 finding 3: an empty list matches +// every message; otherwise every word of the list must be found in at least one of texts ("all"), or at least +// one word found in at least one text when any is true. SubjectWords/BodyWords call it with one text (their own +// field, and only that field); Words calls it with both Subject and Body, so a word counts in either — "the +// word in subject or body", as the acceptance criteria ask it. +func wordMatches(texts []string, words []string, any bool) bool { if len(words) == 0 { return true } for _, w := range words { - found := wordRegexp(w).MatchString(text) + re := wordRegexp(w) + found := false + for _, t := range texts { + if re.MatchString(t) { + found = true + break + } + } if any && found { return true } diff --git a/modules/mesh-mail/cmd/mesh-mail/tools.go b/modules/mesh-mail/cmd/mesh-mail/tools.go index 9930730c..667c7fbd 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools.go @@ -106,8 +106,8 @@ func (a access) refuse(caller, account string) error { // searchArgs names mail_search's own arguments, used both for its schema (issue 1232, AC17) and for only's // guard, so the two can never drift apart. var searchArgs = []string{ - "query", "account", "folder", "limit", "from", "to", "cc", "on-or-after", "before", "subject-words", - "subject-words-any", "body-words", "body-words-any", "has-attachment", "attachment-type", "attachment-name", + "query", "account", "folder", "limit", "from", "to", "cc", "on-or-after", "before", "words", "words-any", + "subject-words", "subject-words-any", "body-words", "body-words-any", "has-attachment", "attachment-type", "attachment-name", "unread", "order-by", "order", "group-by-sender", "in-reply-to", "from-someone-i-wrote-to", } @@ -126,11 +126,14 @@ func searchToolSchema() map[string]any { "cc": text("filter by a cc recipient: an address or a domain, case-insensitive"), "on-or-after": text("only mails dated on or after this (RFC3339, or YYYY-MM-DD for the whole day)"), "before": text("only mails dated before this (RFC3339, or YYYY-MM-DD for the whole day)"), - "subject-words": stringArray("every one of these whole words (case-insensitive) must be in the subject, " + - "unless subject-words-any"), + "words": stringArray("every one of these whole words (case-insensitive) must be in the subject or the " + + "body (either counts), unless words-any"), + "words-any": boolean("words matches when any one of them is found, not every one"), + "subject-words": stringArray("every one of these whole words (case-insensitive) must be in the subject " + + "(only the subject; words, above, takes the subject or the body), unless subject-words-any"), "subject-words-any": boolean("subject-words matches when any one of them is found, not every one"), - "body-words": stringArray("every one of these whole words (case-insensitive) must be in the text, unless " + - "body-words-any"), + "body-words": stringArray("every one of these whole words (case-insensitive) must be in the text (only " + + "the body; words, above, takes the subject or the body), unless body-words-any"), "body-words-any": boolean("body-words matches when any one of them is found, not every one"), "has-attachment": boolean("only mails carrying at least one attachment"), "attachment-type": text("only mails carrying an attachment of this exact media type, case-insensitive; " + @@ -162,7 +165,8 @@ func (b *Bundle) Tools() []stdio.Tool { Description: "Search the mail of the accounts you are granted. Query is optional: with no filter at all, the " + "newest mails of your granted accounts. Every filter is optional and combines with the others (and): account, " + "folder, from/to/cc (an address or a domain; from takes several, any one matching), a date range, " + - "subject-words/body-words (each all unless its own -any), has-attachment, attachment-type/attachment-name " + + "words (a whole word in the subject or the body, either counts), subject-words/body-words (only that one " + + "field; each all unless its own -any), has-attachment, attachment-type/attachment-name " + "(either matching an attachment suffices), unread, in-reply-to, from-someone-i-wrote-to. Order by date (the " + "default), sender, subject or size, asc or desc; group-by-sender answers one line per sender (address, name, " + "count, newest) instead. At most 50 (20 default). Answers account, folder, uid, message id, date, sender, " + @@ -349,6 +353,14 @@ func (b *Bundle) searchTool(args map[string]any) (any, error) { if err != nil { return nil, err } + words, err := stringsArg(args, "words") + if err != nil { + return nil, err + } + wordsAny, err := flag(args, "words-any") + if err != nil { + return nil, err + } subjectWords, err := stringsArg(args, "subject-words") if err != nil { return nil, err @@ -420,6 +432,7 @@ func (b *Bundle) searchTool(args map[string]any) (any, error) { q := Query{ Text: query, Accounts: granted, Folder: folder, Limit: limit, From: from, To: to, Cc: cc, OnOrAfter: onOrAfter, Before: before, + Words: words, WordsAny: wordsAny, SubjectWords: subjectWords, SubjectWordsAny: subjectWordsAny, BodyWords: bodyWords, BodyWordsAny: bodyWordsAny, HasAttachment: hasAttachment, AttachmentType: attachmentType, AttachmentNamePattern: attachmentName, Unread: unread, OrderBy: orderBy, OrderDesc: orderDesc, From 8dd2a42448a12a8d72d8edefd8cc76bc6f74f386 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 15:05:23 +0200 Subject: [PATCH 09/10] mesh-mail: mail_search's order defaults to descending when left out, matching its own schema text, and AC1's no-filter test proves ordering, limit and grant isolation together (issue 1232, review findings 5 and 7) --- modules/mesh-mail/cmd/mesh-mail/tools.go | 6 +- modules/mesh-mail/cmd/mesh-mail/tools_test.go | 62 ++++++++++++++++++- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/tools.go b/modules/mesh-mail/cmd/mesh-mail/tools.go index 667c7fbd..14c9e9d3 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools.go @@ -725,11 +725,13 @@ func orderByArg(args map[string]any) (string, error) { return s, nil } -// orderArg is 1230: asc or desc; anything else is refused (AC16), before the index is ever asked. +// orderArg is 1230: asc or desc; desc (newest, largest, ...) when left out, as the schema says (review round 1 +// finding 5: this returned ascending on nothing given, contradicting its own description); anything else is +// refused (AC16), before the index is ever asked. func orderArg(args map[string]any) (bool, error) { v, ok := args["order"] if !ok || v == nil { - return false, nil + return true, nil } s, ok := v.(string) if !ok { diff --git a/modules/mesh-mail/cmd/mesh-mail/tools_test.go b/modules/mesh-mail/cmd/mesh-mail/tools_test.go index e81c1f2d..747b1d30 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools_test.go @@ -276,13 +276,58 @@ func TestARefusalSaysWhyTheGrantsCouldNotBeRead(t *testing.T) { // 1228/AC1, 1232: with no filter at all (not even query), mail_search answers the newest mails of the granted // accounts, not a refusal demanding a query. +// 1228/AC1, 1232 (review round 1 finding 7): several mails over two granted accounts and folders, one mail of a +// third account the caller is not granted, and a limit below the granted count — the newest granted mails +// answer, in that exact order, never the ungranted account's, even though its mail is the newest of all. func TestMailSearchWithNoFilterAnswersTheNewestGrantedMail(t *testing.T) { - b, _, _ := aBundle(t) + dir := t.TempDir() + env := Env{Accounts: filepath.Join(dir, "accounts.json"), Grants: filepath.Join(dir, "grants.json"), Secrets: dir, + Status: filepath.Join(dir, "health.json")} + writeFile(t, env.Accounts, `{"accounts": { + "mesh": {"kind": "imap", "address": "jschoubben@novox.be", "incoming": {"host": "mail.novox.be", "port": 993, "security": "tls"}}, + "gmail": {"kind": "imap", "address": "me@gmail.example", "incoming": {"host": "imap.gmail.com", "port": 993, "security": "tls"}}, + "secret": {"kind": "imap", "address": "other@example.org", "incoming": {"host": "mail.example.org", "port": 993, "security": "tls"}}}}`) + writeFile(t, env.Grants, `{"grants": {"mesh": ["triage"], "gmail": ["triage"], "secret": ["other"]}}`) + st := newMemStore() + ctx := context.Background() + at := func(h int) time.Time { return judgedAt.Add(time.Duration(h) * time.Hour) } + for _, m := range []Message{ + {Account: "mesh", Folder: "INBOX", UID: 1, MessageID: "", Subject: "m1", Date: at(1), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mesh", Folder: "INBOX", UID: 2, MessageID: "", Subject: "m2", Date: at(2), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mesh", Folder: "Sent", UID: 1, MessageID: "", Subject: "m3", Date: at(3), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "gmail", Folder: "INBOX", UID: 1, MessageID: "", Subject: "m4", Date: at(4), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "gmail", Folder: "Sent", UID: 1, MessageID: "", Subject: "m5", Date: at(5), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mesh", Folder: "Junk", UID: 1, MessageID: "", Subject: "m6", Date: at(6), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + // The newest of all, but in an account triage is not granted: it must never answer. + {Account: "secret", Folder: "INBOX", UID: 1, MessageID: "", Subject: "s1", Date: at(7), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + } { + if err := st.Add(ctx, m, nil); err != nil { + t.Fatal(err) + } + } + b := &Bundle{Env: env, Index: func(context.Context) (Store, error) { return st, nil }, Now: func() time.Time { return judgedAt }} rt := startBundle(t, b.Tools()) - text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{}) - if isErr || !strings.Contains(text, `"message-id":"att@example.org"`) { + text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{"limit": 3}) + if isErr { t.Fatalf("%v %s", isErr, text) } + if strings.Contains(text, "secret") || strings.Contains(text, "s1@secret") { + t.Fatalf("the ungranted account's mail answered: %s", text) + } + i6, i5, i4 := strings.Index(text, "m6@mesh"), strings.Index(text, "m5@gmail"), strings.Index(text, "m4@gmail") + if i6 < 0 || i5 < 0 || i4 < 0 || !(i6 < i5 && i5 < i4) { + t.Fatalf("not exactly m6, m5, m4 in that (newest-first) order: %s", text) + } + if strings.Contains(text, "m3@mesh") || strings.Contains(text, "m2@mesh") || strings.Contains(text, "m1@mesh") { + t.Fatalf("limit 3 answered more than the 3 newest: %s", text) + } } // 1232/AC16: an unknown order-by, a limit over 50, a date that does not parse, or an account not granted are @@ -394,4 +439,15 @@ func TestMailSearchEndToEndFiltersAndGroupBySender(t *testing.T) { grouped.Senders[0].Address != "alice@example.org" || grouped.Senders[0].Count != 2 { t.Fatalf("%v %s", isErr, text) } + + // 1232/AC17 (review round 1 finding 5): order-by given, order left out, defaults to desc, as the schema's + // own description promises — not asc. "from alice" is a prefix of "from alice again", so ascending would put + // first; descending puts first. + text, isErr = rt.call(SearchTool, "novox.triage", map[string]any{"order-by": "subject"}) + if isErr { + t.Fatalf("%v %s", isErr, text) + } + if first, second := strings.Index(text, "b@mesh"), strings.Index(text, "a@mesh"); first < 0 || second < 0 || first > second { + t.Fatalf("order-by with no order was not desc by default: %s", text) + } } From 920199862e181ebff560f29c23bfc2985da4e845 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 15:09:44 +0200 Subject: [PATCH 10/10] mesh-mail: group-by-sender and order-by sender compare addresses case-insensitively in Postgres too, and a sender's shown name and address are its newest mail's, deterministically (issue 1230, review finding 6) --- modules/mesh-mail/cmd/mesh-mail/pgstore.go | 15 ++++-- .../mesh-mail/cmd/mesh-mail/search_test.go | 47 +++++++++++++++++++ modules/mesh-mail/cmd/mesh-mail/store.go | 10 ++-- 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/pgstore.go b/modules/mesh-mail/cmd/mesh-mail/pgstore.go index 66272106..9d0c1d6b 100644 --- a/modules/mesh-mail/cmd/mesh-mail/pgstore.go +++ b/modules/mesh-mail/cmd/mesh-mail/pgstore.go @@ -561,7 +561,7 @@ func searchWhere(q Query) (whereSQL string, args []any) { // unknown or empty answers "date" — the tool refuses an unknown order-by before the store is ever asked (AC16). var orderColumns = map[string]string{ "date": "date", - "sender": "coalesce(sender->0->>'address', '')", + "sender": "lower(coalesce(sender->0->>'address', ''))", "subject": "subject", "size": "size", } @@ -620,9 +620,16 @@ func (s *PgStore) GroupBySender(ctx context.Context, q Query) ([]SenderGroup, er args = append(args, v) return fmt.Sprintf("$%d", len(args)) } - sqlText := `select el->>'address' as address, max(el->>'name') as name, count(*) as count, max(date) as newest - from message, jsonb_array_elements(sender) el where ` + whereSQL + ` group by el->>'address' - order by count desc, address asc limit ` + param(limit) + // 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. + // The outer select re-sorts by lower(address): Postgres resolves a SELECT alias used bare in order by, but + // not wrapped in a function, so the grouping is wrapped in its own subquery first. + 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') + ) grouped order by count desc, lower(address) asc limit ` + param(limit) rows, err := s.pool.Query(ctx, sqlText, args...) if err != nil { return nil, err diff --git a/modules/mesh-mail/cmd/mesh-mail/search_test.go b/modules/mesh-mail/cmd/mesh-mail/search_test.go index f31015a4..bc8bdde8 100644 --- a/modules/mesh-mail/cmd/mesh-mail/search_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/search_test.go @@ -9,6 +9,7 @@ package main import ( "context" "sort" + "strings" "testing" "time" ) @@ -529,6 +530,52 @@ func TestGroupBySenderOrdersByCountDescending(t *testing.T) { }) } +// 1230 (review round 1 finding 6): the same address, written differently across two mails, is one sender, not +// two — grouped case-insensitively, in both stores. Its displayed address and name are the newest mail's own +// (not max()'s lexicographically largest, and not whichever memStore's map iteration visited first), and +// order-by sender compares case-insensitively too. +func TestGroupBySenderAndOrderBySenderAreCaseInsensitive(t *testing.T) { + eachStore(t, func(t *testing.T, st Store) { + ctx := context.Background() + older := time.Date(2026, 10, 1, 9, 0, 0, 0, time.UTC) + newer := time.Date(2026, 10, 2, 9, 0, 0, 0, time.UTC) + for _, m := range []Message{ + {Account: "mixedcase", Folder: "INBOX", UID: 1, MessageID: "", Subject: "first", Date: older, + From: []Person{{Name: "Bob", Address: "Bob@Example.ORG"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mixedcase", Folder: "INBOX", UID: 2, MessageID: "", Subject: "second", Date: newer, + From: []Person{{Name: "Bobby", Address: "bob@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mixedcase", Folder: "INBOX", UID: 3, MessageID: "", Subject: "third", Date: older.Add(time.Hour), + From: []Person{{Name: "Alice", Address: "alice@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + } { + if err := st.Add(ctx, m, nil); err != nil { + t.Fatal(err) + } + } + groups, err := st.GroupBySender(ctx, Query{Accounts: []string{"mixedcase"}, Folder: "INBOX", Limit: 10}) + if err != nil { + t.Fatal(err) + } + if len(groups) != 2 { + t.Fatalf("Bob@Example.ORG and bob@example.org were not merged into one sender: %+v", groups) + } + var bob *SenderGroup + for i := range groups { + if strings.EqualFold(groups[i].Address, "bob@example.org") { + bob = &groups[i] + } + } + if bob == nil || bob.Count != 2 || bob.Address != "bob@example.org" || bob.Name != "Bobby" || !bob.Newest.Equal(newer) { + t.Fatalf("bob's merged line: %+v", bob) + } + + bySender, err := st.Search(ctx, Query{Accounts: []string{"mixedcase"}, Folder: "INBOX", OrderBy: "sender", Limit: 10}) + if err != nil { + t.Fatal(err) + } + sameOrder(t, bySender, "", "", "") // alice@... before Bob@.../bob@... (case-insensitive) + }) +} + // 1231/AC8: in-reply-to a mail I sent, combined with from (its recipient), answers the replies to it; alone, it // answers every reply in the thread's direct answer set. func TestSearchFiltersByInReplyTo(t *testing.T) { diff --git a/modules/mesh-mail/cmd/mesh-mail/store.go b/modules/mesh-mail/cmd/mesh-mail/store.go index ae44c3d4..4483dc8b 100644 --- a/modules/mesh-mail/cmd/mesh-mail/store.go +++ b/modules/mesh-mail/cmd/mesh-mail/store.go @@ -455,13 +455,17 @@ func (s *memStore) GroupBySender(_ context.Context, q Query) ([]SenderGroup, err addr := strings.ToLower(p.Address) g, ok := byAddress[addr] if !ok { - g = &SenderGroup{Address: p.Address, Name: p.Name} + g = &SenderGroup{} byAddress[addr] = g order = append(order, addr) } g.Count++ - if m.Date.After(g.Newest) { + // Review round 1 finding 6: Address and Name are the newest mail's own, never whichever message a + // Go map's (randomised) iteration order happened to visit first. + if g.Newest.IsZero() || m.Date.After(g.Newest) { g.Newest = m.Date + g.Address = p.Address + g.Name = p.Name } } } @@ -473,7 +477,7 @@ func (s *memStore) GroupBySender(_ context.Context, q Query) ([]SenderGroup, err if out[i].Count != out[j].Count { return out[i].Count > out[j].Count } - return out[i].Address < out[j].Address + return strings.ToLower(out[i].Address) < strings.ToLower(out[j].Address) }) if q.Limit > 0 && len(out) > q.Limit { out = out[:q.Limit]